diff --git a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx index 7b8a39e9745..13d6fa16228 100644 --- a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx +++ b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx @@ -63,6 +63,20 @@ async function comboboxSelectItem( }); } +async function multiComboboxToggleItem( + label: string, + value: string, + visibleLabel = value +) { + userEvent.click(screen.getByRole('textbox', { name: label })); + await waitFor(() => { + const listbox = screen.getByRole('listbox'); + expect(listbox).to.be.visible; + const option = within(listbox).getByRole('option', { name: visibleLabel }); + userEvent.click(option); + }); +} + function getMultiComboboxValues(testId: string) { const combobox = screen.getByTestId(testId); expect(combobox).to.be.visible; @@ -166,15 +180,15 @@ describe('DiagramEditorSidePanel', function () { it('should render a nested field context drawer', async function () { const result = renderDrawer(); result.plugin.store.dispatch( - selectField('flights.routes', ['airline', 'id']) + selectField('flights.routes', ['airline', '_id']) ); await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline.id')).to.be.visible; + expect(screen.getByTitle('routes.airline._id')).to.be.visible; const nameInput = screen.getByLabelText('Field name'); expect(nameInput).to.be.visible; - expect(nameInput).to.have.value('id'); + expect(nameInput).to.have.value('_id'); const selectedTypes = getMultiComboboxValues('lg-combobox-datatype'); expect(selectedTypes).to.have.lengthOf(1); @@ -184,16 +198,16 @@ describe('DiagramEditorSidePanel', function () { it('should delete a field', async function () { const result = renderDrawer(); result.plugin.store.dispatch( - selectField('flights.routes', ['airline', 'id']) + selectField('flights.routes', ['airline', '_id']) ); await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline.id')).to.be.visible; + expect(screen.getByTitle('routes.airline._id')).to.be.visible; userEvent.click(screen.getByLabelText(/delete field/i)); await waitFor(() => { - expect(screen.queryByText('routes.airline.id')).not.to.exist; + expect(screen.queryByText('routes.airline._id')).not.to.exist; }); expect(screen.queryByLabelText('Name')).to.not.exist; @@ -205,7 +219,7 @@ describe('DiagramEditorSidePanel', function () { expect( modifiedCollection?.jsonSchema.properties?.airline.properties - ).to.not.have.property('id'); // deleted field + ).to.not.have.property('_id'); // deleted field expect( modifiedCollection?.jsonSchema.properties?.airline.properties ).to.have.property('name'); // sibling field remains @@ -254,13 +268,139 @@ describe('DiagramEditorSidePanel', function () { await waitForDrawerToOpen(); expect(screen.getByTitle('routes.airline.name')).to.be.visible; - updateInputWithBlur('Field name', 'id'); + updateInputWithBlur('Field name', '_id'); await waitFor(() => { expect(screen.queryByText('Field already exists.')).to.exist; expect(screen.queryByText('routes.airline.name')).to.exist; }); }); + + it('should change the field type', async function () { + const result = renderDrawer(); + result.plugin.store.dispatch( + selectField('flights.routes', ['airline', 'name']) + ); + + await waitForDrawerToOpen(); + expect(screen.getByTitle('routes.airline.name')).to.be.visible; + + // before - string + const selectedTypesBefore = getMultiComboboxValues( + 'lg-combobox-datatype' + ); + expect(selectedTypesBefore).to.have.members(['string']); + + // add int and bool and remove string + await multiComboboxToggleItem('Datatype', 'int'); + await multiComboboxToggleItem('Datatype', 'bool'); + await multiComboboxToggleItem('Datatype', 'string'); + + const modifiedCollection = selectCurrentModelFromState( + result.plugin.store.getState() + ).collections.find((coll) => { + return coll.ns === 'flights.routes'; + }); + expect( + modifiedCollection?.jsonSchema.properties?.airline?.properties?.name + .bsonType + ).to.have.members(['int', 'bool']); + }); + + it('should not completely remove the type', async function () { + const result = renderDrawer(); + result.plugin.store.dispatch( + selectField('flights.routes', ['airline', 'name']) + ); + + await waitForDrawerToOpen(); + expect(screen.getByTitle('routes.airline.name')).to.be.visible; + + // before - string + const selectedTypesBefore = getMultiComboboxValues( + 'lg-combobox-datatype' + ); + expect(selectedTypesBefore).to.have.members(['string']); + + // remove string without adding anything else + await multiComboboxToggleItem('Datatype', 'string'); + + await waitFor(() => { + // error message shown + expect(screen.queryByText('Field must have a type.')).to.exist; + const modifiedCollection = selectCurrentModelFromState( + result.plugin.store.getState() + ).collections.find((coll) => { + return coll.ns === 'flights.routes'; + }); + // type remains unchanged + expect( + modifiedCollection?.jsonSchema.properties?.airline?.properties?.name + .bsonType + ).to.equal('string'); + }); + + // finally, add some types + await multiComboboxToggleItem('Datatype', 'bool'); + await multiComboboxToggleItem('Datatype', 'int'); + + await waitFor(() => { + // error goes away + expect(screen.queryByText('Field must have a type.')).not.to.exist; + const modifiedCollection = selectCurrentModelFromState( + result.plugin.store.getState() + ).collections.find((coll) => { + return coll.ns === 'flights.routes'; + }); + // new type applied + expect( + modifiedCollection?.jsonSchema.properties?.airline?.properties?.name + .bsonType + ).to.have.members(['bool', 'int']); + }); + }); + + it('top level _id field is treated as readonly', async function () { + const result = renderDrawer(); + result.plugin.store.dispatch(selectField('flights.routes', ['_id'])); + + await waitForDrawerToOpen(); + expect(screen.getByTitle('routes._id')).to.be.visible; + + expect(screen.queryByLabelText(/delete field/i)).not.to.exist; + expect(screen.getByLabelText('Field name')).to.have.attribute( + 'aria-disabled', + 'true' + ); + expect(screen.getByLabelText('Datatype')).to.have.attribute( + 'aria-disabled', + 'true' + ); + }); + + it('nested _id field is not treated as readonly', async function () { + const result = renderDrawer(); + result.plugin.store.dispatch( + selectField('flights.routes', ['airline', '_id']) + ); + + await waitForDrawerToOpen(); + expect(screen.getByTitle('routes.airline._id')).to.be.visible; + + expect(screen.queryByLabelText(/delete field/i)).to.exist; + expect(screen.queryByLabelText(/delete field/i)).to.have.attribute( + 'aria-disabled', + 'false' + ); + expect(screen.getByLabelText('Field name')).to.have.attribute( + 'aria-disabled', + 'false' + ); + expect(screen.getByLabelText('Datatype')).to.have.attribute( + 'aria-disabled', + 'false' + ); + }); }); it('should change the content of the drawer when selecting different items', async function () { diff --git a/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx b/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx index 7adea901509..226e6937d2b 100644 --- a/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx +++ b/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react'; +import React, { useMemo, useState } from 'react'; import { connect } from 'react-redux'; import type { FieldPath, @@ -11,6 +11,7 @@ import { } from '@mongodb-js/compass-components'; import { BSONType } from 'mongodb'; import { + changeFieldType, createNewRelationship, deleteRelationship, getCurrentDiagramFromState, @@ -56,8 +57,7 @@ type FieldDrawerContentProps = { onChangeFieldType: ( namespace: string, fieldPath: FieldPath, - fromBsonType: string | string[], - toBsonType: string | string[] + newTypes: string[] ) => void; }; @@ -117,6 +117,11 @@ const FieldDrawerContent: React.FunctionComponent = ({ onRenameField, onChangeFieldType, }) => { + const [fieldTypeEditErrorMessage, setFieldTypeEditErrorMessage] = useState< + string | undefined + >(); + const [fieldTypes, setFieldTypes] = useState(types); + const { value: fieldName, ...nameInputProps } = useChangeOnBlur( fieldPath[fieldPath.length - 1], (fieldName) => { @@ -137,17 +142,25 @@ const FieldDrawerContent: React.FunctionComponent = ({ [fieldPath, fieldPaths, fieldName] ); - const handleTypeChange = (newTypes: string | string[]) => { - onChangeFieldType(namespace, fieldPath, types, newTypes); + const handleTypeChange = (newTypes: string[]) => { + setFieldTypes(newTypes); + if (newTypes.length === 0) { + setFieldTypeEditErrorMessage('Field must have a type.'); + return; + } + setFieldTypeEditErrorMessage(undefined); + onChangeFieldType(namespace, fieldPath, newTypes); }; + const isReadOnly = useMemo(() => isIdField(fieldPath), [fieldPath]); + return ( <> = ({ data-testid="lg-combobox-datatype" label="Datatype" aria-label="Datatype" - disabled={true} // TODO(COMPASS-9659): enable when field type change is implemented - value={types} + disabled={isReadOnly} + value={fieldTypes} size="small" multiselect={true} clearable={false} onChange={handleTypeChange} + state={fieldTypeEditErrorMessage ? 'error' : undefined} + errorMessage={fieldTypeEditErrorMessage} > {BSON_TYPES.map((type) => ( @@ -228,6 +243,6 @@ export default connect( onEditRelationshipClick: selectRelationship, onDeleteRelationshipClick: deleteRelationship, onRenameField: renameField, - onChangeFieldType: () => {}, // TODO(COMPASS-9659): updateFieldSchema, + onChangeFieldType: changeFieldType, } )(FieldDrawerContent); diff --git a/packages/compass-data-modeling/src/store/apply-edit.ts b/packages/compass-data-modeling/src/store/apply-edit.ts index 3e9d4c1d2aa..7577484b90a 100644 --- a/packages/compass-data-modeling/src/store/apply-edit.ts +++ b/packages/compass-data-modeling/src/store/apply-edit.ts @@ -167,7 +167,7 @@ export function applyEdit(edit: Edit, model?: StaticModel): StaticModel { jsonSchema: updateSchema({ jsonSchema: collection.jsonSchema, fieldPath: edit.field, - update: 'removeField', + updateParameters: { update: 'removeField' }, }), }; }), @@ -198,8 +198,29 @@ export function applyEdit(edit: Edit, model?: StaticModel): StaticModel { jsonSchema: updateSchema({ jsonSchema: collection.jsonSchema, fieldPath: edit.field, - update: 'renameField', - newFieldName: edit.newName, + updateParameters: { + update: 'renameField', + newFieldName: edit.newName, + }, + }), + }; + }), + }; + } + case 'ChangeFieldType': { + return { + ...model, + collections: model.collections.map((collection) => { + if (collection.ns !== edit.ns) return collection; + return { + ...collection, + jsonSchema: updateSchema({ + jsonSchema: collection.jsonSchema, + fieldPath: edit.field, + updateParameters: { + update: 'changeFieldSchema', + newFieldSchema: edit.to, + }, }), }; }), diff --git a/packages/compass-data-modeling/src/store/diagram.ts b/packages/compass-data-modeling/src/store/diagram.ts index 883328112ff..52aa40eb736 100644 --- a/packages/compass-data-modeling/src/store/diagram.ts +++ b/packages/compass-data-modeling/src/store/diagram.ts @@ -29,7 +29,11 @@ import type { MongoDBJSONSchema } from 'mongodb-schema'; import { getCoordinatesForNewNode } from '@mongodb-js/diagramming'; import { collectionToDiagramNode } from '../utils/nodes-and-edges'; import toNS from 'mongodb-ns'; -import { traverseSchema } from '../utils/schema-traversal'; +import { + getFieldFromSchema, + getSchemaWithNewTypes, + traverseSchema, +} from '../utils/schema-traversal'; import { applyEdit as _applyEdit } from './apply-edit'; function isNonEmptyArray(arr: T[]): arr is [T, ...T[]] { @@ -684,6 +688,34 @@ export function renameField( return applyEdit({ type: 'RenameField', ns, field, newName }); } +export function changeFieldType( + ns: string, + fieldPath: FieldPath, + newTypes: string[] +): DataModelingThunkAction { + return (dispatch, getState) => { + const collectionSchema = selectCurrentModelFromState( + getState() + ).collections.find((collection) => collection.ns === ns)?.jsonSchema; + if (!collectionSchema) throw new Error('Collection not found in model'); + const field = getFieldFromSchema({ + jsonSchema: collectionSchema, + fieldPath: fieldPath, + }); + if (!field) throw new Error('Field not found in schema'); + const to = getSchemaWithNewTypes(field.jsonSchema, newTypes); + dispatch( + applyEdit({ + type: 'ChangeFieldType', + ns, + field: fieldPath, + from: field.jsonSchema, + to, + }) + ); + }; +} + function getPositionForNewCollection( existingCollections: DataModelCollection[], newCollection: Omit diff --git a/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx b/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx index 252869cf8d5..89bb4146e51 100644 --- a/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx +++ b/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx @@ -3,6 +3,7 @@ import { traverseSchema, getFieldFromSchema, updateSchema, + getSchemaWithNewTypes, } from './schema-traversal'; import Sinon from 'sinon'; @@ -463,7 +464,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: {}, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({}); }); @@ -491,7 +494,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['address', 'age'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal(schema); }); @@ -509,7 +514,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({ ...schema, @@ -543,7 +550,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['person', 'address'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({ ...schema, @@ -580,7 +589,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['person', 'address', 'city'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({ ...schema, @@ -624,7 +635,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['names', 'first'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({ ...schema, @@ -664,7 +677,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['addresses', 'streetNumber'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({ ...schema, @@ -707,7 +722,9 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['todos', 'completed'], jsonSchema: schema, - update: 'removeField', + updateParameters: { + update: 'removeField', + }, }); expect(result).to.deep.equal({ ...schema, @@ -738,8 +755,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: {}, - update: 'renameField', - newFieldName: 'newName', + updateParameters: { + update: 'renameField', + newFieldName: 'newName', + }, }); expect(result).to.deep.equal({}); }); @@ -767,8 +786,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['address', 'age'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'newName', + updateParameters: { + update: 'renameField', + newFieldName: 'newName', + }, }); expect(result).to.deep.equal(schema); }); @@ -786,8 +807,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'newName', + updateParameters: { + update: 'renameField', + newFieldName: 'newName', + }, }); expect(result).to.deep.equal({ ...schema, @@ -822,8 +845,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['person', 'address'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'location', + updateParameters: { + update: 'renameField', + newFieldName: 'location', + }, }); expect(result).to.deep.equal({ ...schema, @@ -861,8 +886,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['person', 'address', 'city'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'town', + updateParameters: { + update: 'renameField', + newFieldName: 'town', + }, }); expect(result).to.deep.equal({ ...schema, @@ -908,8 +935,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['names', 'first'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'given', + updateParameters: { + update: 'renameField', + newFieldName: 'given', + }, }); expect(result).to.deep.equal({ ...schema, @@ -950,8 +979,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['addresses', 'streetNumber'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'street_num', + updateParameters: { + update: 'renameField', + newFieldName: 'street_num', + }, }); expect(result).to.deep.equal({ ...schema, @@ -995,8 +1026,10 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['todos', 'completed'], jsonSchema: schema, - update: 'renameField', - newFieldName: 'done', + updateParameters: { + update: 'renameField', + newFieldName: 'done', + }, }); expect(result).to.deep.equal({ ...schema, @@ -1021,3 +1054,336 @@ describe('renameField', function () { }); }); }); + +describe('getSchemaWithNewTypes', function () { + describe('basic types', function () { + it('updates a single type', function () { + const newTypes = ['string', 'int']; + const result = getSchemaWithNewTypes({ bsonType: 'string' }, newTypes); + expect(result).to.deep.equal({ bsonType: newTypes }); + }); + + it('updates an array of types', function () { + const newTypes = ['bool', 'int']; + const result = getSchemaWithNewTypes( + { bsonType: ['string', 'bool'] }, + newTypes + ); + expect(result).to.deep.equal({ bsonType: newTypes }); + }); + }); + + describe('complex types', function () { + describe('cleans up the root schema', function () { + it('changes an object to a string', function () { + const newTypes = ['string']; + const result = getSchemaWithNewTypes( + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + newTypes + ); + expect(result).to.deep.equal({ bsonType: newTypes }); + }); + + it('changes an array to a string', function () { + const newTypes = ['string']; + const result = getSchemaWithNewTypes( + { + bsonType: 'array', + items: { + bsonType: 'int', + }, + }, + newTypes + ); + expect(result).to.deep.equal({ bsonType: newTypes }); + }); + }); + + describe('cleans up parts of anyOf', function () { + it('removes object but keeps array', function () { + const newTypes = ['array']; + const oldSchema = { + anyOf: [ + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + { + bsonType: 'array', + items: { + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + // array is no longer part of anyOf, now it is the only type and so the root schema + expect(result).to.deep.equal(oldSchema.anyOf[1]); + }); + + it('removes array but keeps object', function () { + const newTypes = ['object']; + const oldSchema = { + anyOf: [ + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + { + bsonType: 'array', + items: { + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + // object is no longer part of anyOf, now it is the only type and so the root schema + expect(result).to.deep.equal(oldSchema.anyOf[0]); + }); + + it('removes one of many types', function () { + const newTypes = ['object', 'array', 'string', 'bool']; // removes int + const oldSchema = { + anyOf: [ + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + { + bsonType: 'array', + items: { + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + }, + { + bsonType: 'string', + }, + { + bsonType: 'int', + }, + { + bsonType: 'bool', + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).to.not.to.have.property('bsonType'); + expect(result.anyOf).to.have.lengthOf(4); + expect(result.anyOf).to.have.deep.members([ + oldSchema.anyOf[0], + oldSchema.anyOf[1], + oldSchema.anyOf[2], + // int - is missing + oldSchema.anyOf[4], + ]); + expect(result.anyOf).to.not.have.deep.members([ + oldSchema.anyOf[3], // int - is missing + ]); + }); + }); + + describe('uses anyOf for a mixture of simple and complex types', function () { + it('adds another type on top of object and array', function () { + const newTypes = ['object', 'array', 'bool']; + const oldSchema = { + anyOf: [ + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + { + bsonType: 'array', + items: { + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('bsonType'); + expect(result.anyOf).to.have.lengthOf(3); + expect(result.anyOf).to.have.deep.members([ + oldSchema.anyOf[0], + oldSchema.anyOf[1], + { + bsonType: 'bool', + }, + ]); + }); + + it('adds object alongside a string', function () { + const newTypes = ['string', 'object']; + const oldSchema = { + bsonType: 'string', + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('bsonType'); + expect(result.anyOf).to.have.lengthOf(2); + expect(result.anyOf).to.deep.include({ + bsonType: 'string', + }); + expect(result.anyOf).to.deep.include({ + bsonType: 'object', + properties: {}, + required: [], + }); + }); + + it('adds array alongside a string', function () { + const newTypes = ['string', 'array']; + const oldSchema = { + bsonType: 'string', + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('bsonType'); + expect(result.anyOf).to.have.lengthOf(2); + expect(result.anyOf).to.deep.include({ + bsonType: 'string', + }); + expect(result.anyOf).to.deep.include({ + bsonType: 'array', + items: {}, + }); + }); + + it('adds string alongside an object', function () { + const newTypes = ['string', 'object']; + const oldSchema = { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('bsonType'); + expect(result).not.to.have.property('properties'); + expect(result).not.to.have.property('required'); + expect(result.anyOf).to.have.lengthOf(2); + expect(result.anyOf).to.have.deep.members([ + { + bsonType: 'string', + }, + oldSchema, + ]); + }); + + it('adds string alongside an array', function () { + const newTypes = ['string', 'array']; + const oldSchema = { + bsonType: 'array', + items: { bsonType: 'int' }, + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('bsonType'); + expect(result).not.to.have.property('items'); + expect(result.anyOf).to.have.lengthOf(2); + expect(result.anyOf).to.have.deep.members([ + { + bsonType: 'string', + }, + oldSchema, + ]); + }); + }); + + describe('cleans up anyOf when it is no longer needed', function () { + it('removes array from a mixed type', function () { + const newTypes = ['int', 'double']; + const oldSchema = { + anyOf: [ + { + bsonType: 'array', + items: [{ bsonType: 'string' }], + }, + { + bsonType: 'int', + }, + { + bsonType: 'double', + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('anyOf'); + expect(result).to.deep.equal({ bsonType: newTypes }); + }); + + it('removes object from a mixed type', function () { + const newTypes = ['int', 'bool']; + const oldSchema = { + anyOf: [ + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + { + bsonType: 'int', + }, + { + bsonType: 'bool', + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('anyOf'); + expect(result).to.deep.equal({ bsonType: newTypes }); + }); + + it('removes string from a mixed type, leaving object', function () { + const newTypes = ['object']; + const oldSchema = { + anyOf: [ + { + bsonType: 'object', + properties: { + name: { bsonType: 'string' }, + }, + required: ['name'], + }, + { + bsonType: 'string', + }, + ], + }; + const result = getSchemaWithNewTypes(oldSchema, newTypes); + expect(result).not.to.have.property('anyOf'); + expect(result).to.deep.equal(oldSchema.anyOf[0]); + }); + }); + }); +}); diff --git a/packages/compass-data-modeling/src/utils/schema-traversal.tsx b/packages/compass-data-modeling/src/utils/schema-traversal.tsx index 80358124260..90fde1ae24d 100644 --- a/packages/compass-data-modeling/src/utils/schema-traversal.tsx +++ b/packages/compass-data-modeling/src/utils/schema-traversal.tsx @@ -1,4 +1,4 @@ -import type { MongoDBJSONSchema } from 'mongodb-schema'; +import type { JSONSchema, MongoDBJSONSchema } from 'mongodb-schema'; import type { FieldPath } from '../services/data-model-storage'; /** @@ -96,6 +96,23 @@ function searchItemsForChild( return undefined; } +function getFieldTypes(jsonSchema: MongoDBJSONSchema): string[] { + const types: string[] = []; + if (jsonSchema.bsonType) { + if (Array.isArray(jsonSchema.bsonType)) { + types.push(...jsonSchema.bsonType); + } else { + types.push(jsonSchema.bsonType); + } + } + if (jsonSchema.anyOf) { + types.push( + ...jsonSchema.anyOf.flatMap((variant) => variant.bsonType || []) + ); + } + return types; +} + /** * Finds a single field in a MongoDB JSON schema. */ @@ -140,21 +157,8 @@ export const getFieldFromSchema = ({ // Reached the end of path, return the field information if (fieldPath.length === 1) { - const types: string[] = []; - if (nextStep.bsonType) { - if (Array.isArray(nextStep.bsonType)) { - types.push(...nextStep.bsonType); - } else { - types.push(nextStep.bsonType); - } - } - if (nextStep.anyOf) { - types.push( - ...nextStep.anyOf.flatMap((variant) => variant.bsonType || []) - ); - } return { - fieldTypes: types, + fieldTypes: getFieldTypes(nextStep), jsonSchema: nextStep, }; } @@ -167,14 +171,16 @@ export const getFieldFromSchema = ({ }; type UpdateOperationParameters = { - update: 'removeField' | 'renameField'; + update: 'removeField' | 'renameField' | 'changeFieldSchema'; newFieldName?: string; + newFieldSchema?: MongoDBJSONSchema; }; const applySchemaUpdate = ({ schema, fieldName, newFieldName, + newFieldSchema, update, }: { schema: MongoDBJSONSchema; @@ -205,6 +211,21 @@ const applySchemaUpdate = ({ ), }; } + case 'changeFieldSchema': { + if (!schema.properties || !schema.properties[fieldName]) + throw new Error('Field to change type does not exist'); + if (!newFieldSchema) + throw new Error( + 'New field schema is required for the change operation' + ); + return { + ...schema, + properties: { + ...schema.properties, + [fieldName]: newFieldSchema, + }, + }; + } default: return schema; } @@ -216,12 +237,12 @@ const applySchemaUpdate = ({ export const updateSchema = ({ jsonSchema, fieldPath, - update, - newFieldName, + updateParameters, }: { jsonSchema: MongoDBJSONSchema; fieldPath: FieldPath; -} & UpdateOperationParameters): MongoDBJSONSchema => { + updateParameters: UpdateOperationParameters; +}): MongoDBJSONSchema => { const newSchema = { ...jsonSchema, }; @@ -234,8 +255,7 @@ export const updateSchema = ({ return applySchemaUpdate({ schema: newSchema, fieldName: nextInPath, - update, - newFieldName, + ...updateParameters, }); } newSchema.properties = { @@ -243,8 +263,7 @@ export const updateSchema = ({ [nextInPath]: updateSchema({ jsonSchema: newSchema.properties[nextInPath], fieldPath: remainingFieldPath, - update, - newFieldName, + updateParameters, }), }; } @@ -253,8 +272,7 @@ export const updateSchema = ({ updateSchema({ jsonSchema: variant, fieldPath: fieldPath, - update, - newFieldName, + updateParameters, }) ); } @@ -263,16 +281,14 @@ export const updateSchema = ({ newSchema.items = updateSchema({ jsonSchema: newSchema.items, fieldPath: fieldPath, - update, - newFieldName, + updateParameters, }); } else { newSchema.items = newSchema.items.map((item) => updateSchema({ jsonSchema: item, fieldPath: fieldPath, - update, - newFieldName, + updateParameters, }) ); } @@ -280,3 +296,104 @@ export const updateSchema = ({ return newSchema; }; + +const getMin1ArrayVariants = (oldSchema: JSONSchema) => { + const arrayVariants = oldSchema.anyOf?.filter( + (variant) => variant.bsonType === 'array' + ); + if (arrayVariants && arrayVariants.length > 0) { + return arrayVariants as [MongoDBJSONSchema, ...MongoDBJSONSchema[]]; + } + return [ + { + bsonType: 'array', + items: oldSchema.items || {}, + }, + ]; +}; + +const getMin1ObjectVariants = ( + oldSchema: JSONSchema +): [MongoDBJSONSchema, ...MongoDBJSONSchema[]] => { + const objectVariants = oldSchema.anyOf?.filter( + (variant) => variant.bsonType === 'object' + ); + if (objectVariants && objectVariants.length > 0) { + return objectVariants as [MongoDBJSONSchema, ...MongoDBJSONSchema[]]; + } + return [ + { + bsonType: 'object', + properties: oldSchema.properties || {}, + required: oldSchema.required || [], + }, + ]; +}; + +const getOtherVariants = ( + oldSchema: MongoDBJSONSchema, + newTypes: string[] +): MongoDBJSONSchema[] => { + const existingAnyOfVariants = + oldSchema.anyOf?.filter( + (variant) => + typeof variant.bsonType === 'string' && + variant.bsonType !== 'object' && + variant.bsonType !== 'array' && + newTypes.includes(variant.bsonType) + ) || []; + const existingAnyOfTypes = existingAnyOfVariants + .map((v) => v.bsonType) + .flat(); + const existingBasicTypes = oldSchema.bsonType + ? Array.isArray(oldSchema.bsonType) + ? oldSchema.bsonType + : [oldSchema.bsonType] + : []; + const existingBasicVariants = existingBasicTypes + .filter( + (type) => type !== 'object' && type !== 'array' && newTypes.includes(type) + ) + .map((type) => ({ bsonType: type })); + const newVariants = newTypes + .filter( + (type) => + type !== 'object' && + type !== 'array' && + !existingAnyOfTypes.includes(type) && + !existingBasicTypes.includes(type) + ) + .map((type) => ({ bsonType: type })); + return [...existingAnyOfVariants, ...existingBasicVariants, ...newVariants]; +}; + +export function getSchemaWithNewTypes( + oldSchema: MongoDBJSONSchema, + newTypes: string[] +): MongoDBJSONSchema { + const oldTypes = getFieldTypes(oldSchema); + if (oldTypes.join(',') === newTypes.join(',')) return oldSchema; + + // Simple schema - new type does includes neither object nor array + if (!newTypes.some((t) => t === 'object' || t === 'array')) { + return { bsonType: newTypes }; + } + + // Complex schema + const arrayVariants: MongoDBJSONSchema[] = newTypes.includes('array') + ? getMin1ArrayVariants(oldSchema) + : []; + const objectVariants: MongoDBJSONSchema[] = newTypes.includes('object') + ? getMin1ObjectVariants(oldSchema) + : []; + const otherVariants: MongoDBJSONSchema[] = getOtherVariants( + oldSchema, + newTypes + ); + + const newVariants = [...arrayVariants, ...objectVariants, ...otherVariants]; + if (newVariants.length === 1) { + return newVariants[0]; + } + return { anyOf: newVariants }; +} diff --git a/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json b/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json index b205fec74ae..23a672f1c8c 100644 --- a/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json +++ b/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json @@ -204,7 +204,7 @@ "name": { "bsonType": "string" }, - "id": { + "_id": { "bsonType": "string" } }