From b6840efaeada99d8056802566e557f57f0c8b4e4 Mon Sep 17 00:00:00 2001 From: Arnaud MONCEL Date: Thu, 20 Jul 2023 16:09:15 +0200 Subject: [PATCH] fix(add field decorator): throw when put space inside fieldName (#767) --- .../src/decorators/computed/collection.ts | 2 + .../src/decorators/rename-field/collection.ts | 9 +--- .../test/collection-customizer.test.ts | 51 ++++++++++++++++--- .../decorators/computed/collection.test.ts | 18 +++++++ .../rename-field/collection.test.ts | 2 +- .../src/validation/field.ts | 11 ++++ .../test/validation/field/field.test.ts | 14 +++++ 7 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 packages/datasource-toolkit/test/validation/field/field.test.ts diff --git a/packages/datasource-customizer/src/decorators/computed/collection.ts b/packages/datasource-customizer/src/decorators/computed/collection.ts index 8108648454..c6805499da 100644 --- a/packages/datasource-customizer/src/decorators/computed/collection.ts +++ b/packages/datasource-customizer/src/decorators/computed/collection.ts @@ -35,6 +35,8 @@ export default class ComputedCollection extends CollectionDecorator { } registerComputed(name: string, computed: ComputedDefinition): void { + FieldValidator.validateName(this.name, name); + // Check that all dependencies exist and are columns for (const field of computed.dependencies) { FieldValidator.validate(this, field); diff --git a/packages/datasource-customizer/src/decorators/rename-field/collection.ts b/packages/datasource-customizer/src/decorators/rename-field/collection.ts index ef28df59e6..bc1c67c6f3 100644 --- a/packages/datasource-customizer/src/decorators/rename-field/collection.ts +++ b/packages/datasource-customizer/src/decorators/rename-field/collection.ts @@ -6,6 +6,7 @@ import { CollectionSchema, DataSourceDecorator, FieldSchema, + FieldValidator, Filter, PaginatedFilter, Projection, @@ -34,13 +35,7 @@ export default class RenameFieldCollectionDecorator extends CollectionDecorator let initialName = currentName; - if (/ /.test(newName)) { - const sanitizedName = newName.replace(/ (.)/g, (_, s) => s.toUpperCase()); - throw new Error( - `The renaming of field '${currentName}' must not contain space.` + - ` Something like '${sanitizedName}' should work has expected.`, - ); - } + FieldValidator.validateName(this.name, newName); // Revert previous renaming (avoids conflicts and need to recurse on this.toSubCollection). if (this.toChildCollection[currentName]) { diff --git a/packages/datasource-customizer/test/collection-customizer.test.ts b/packages/datasource-customizer/test/collection-customizer.test.ts index 6482df4baf..91ab0bb4a9 100644 --- a/packages/datasource-customizer/test/collection-customizer.test.ts +++ b/packages/datasource-customizer/test/collection-customizer.test.ts @@ -128,6 +128,17 @@ describe('Builder > Collection', () => { }); describe('renameField', () => { + it('should throw when renaming with a name including space', async () => { + const { customizer, dsc } = await setup(); + + customizer.renameField('firstName', 'renamed field'); + + await expect(() => dsc.getDataSource(logger)).rejects.toThrow( + `The name of field 'renamed field' you configured on 'authors' must not contain space.` + + ` Something like 'renamedField' should work has expected.`, + ); + }); + it('should rename a field', async () => { const { dsc, customizer, stack } = await setup(); const spy = jest.spyOn(stack.renameField.getCollection('authors'), 'renameField'); @@ -212,6 +223,17 @@ describe('Builder > Collection', () => { }); describe('importField', () => { + it('should throw when importing with a name including space', async () => { + const { customizer, dsc } = await setup(); + + customizer.importField('first name copy', { path: 'firstName' }); + + await expect(() => dsc.getDataSource(logger)).rejects.toThrow( + `The name of field 'first name copy' you configured on 'authors' must not contain space.` + + ` Something like 'firstNameCopy' should work has expected.`, + ); + }); + it('should call addField', async () => { const { dsc, customizer } = await setup(); @@ -307,6 +329,23 @@ describe('Builder > Collection', () => { }); describe('addField', () => { + it('should throw when adding field with a name including space', async () => { + const { customizer, dsc } = await setup(); + + const fieldDefinition: ComputedDefinition = { + columnType: 'String', + dependencies: ['firstName'], + getValues: records => records.map(() => 'aaa'), + }; + + customizer.addField('new field', fieldDefinition); + + await expect(() => dsc.getDataSource(logger)).rejects.toThrow( + `The name of field 'new field' you configured on 'authors' must not contain space.` + + ` Something like 'newField' should work has expected.`, + ); + }); + it('should add a field to early collection', async () => { const { dsc, customizer, stack } = await setup(); const spy = jest.spyOn(stack.earlyComputed.getCollection('authors'), 'registerComputed'); @@ -317,12 +356,12 @@ describe('Builder > Collection', () => { getValues: records => records.map(() => 'aaa'), }; - const self = customizer.addField('new field', fieldDefinition); + const self = customizer.addField('newField', fieldDefinition); await dsc.getDataSource(logger); expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith('new field', fieldDefinition); - expect(self.schema.fields['new field']).toBeDefined(); + expect(spy).toHaveBeenCalledWith('newField', fieldDefinition); + expect(self.schema.fields.newField).toBeDefined(); expect(self).toEqual(customizer); const { getValues } = spy.mock.calls[0][1]; @@ -347,12 +386,12 @@ describe('Builder > Collection', () => { getValues: records => records.map(() => 'aaa'), }; - const self = customizer.addField('new field', fieldDefinition); + const self = customizer.addField('newField', fieldDefinition); await dsc.getDataSource(logger); expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith('new field', fieldDefinition); - expect(self.schema.fields['new field']).toBeDefined(); + expect(spy).toHaveBeenCalledWith('newField', fieldDefinition); + expect(self.schema.fields.newField).toBeDefined(); expect(self).toEqual(customizer); const { getValues } = spy.mock.calls[0][1]; diff --git a/packages/datasource-customizer/test/decorators/computed/collection.test.ts b/packages/datasource-customizer/test/decorators/computed/collection.test.ts index 1e3b7dbba3..8c1690cbfa 100644 --- a/packages/datasource-customizer/test/decorators/computed/collection.test.ts +++ b/packages/datasource-customizer/test/decorators/computed/collection.test.ts @@ -114,6 +114,24 @@ describe('ComputedDecorator', () => { }).toThrow("Unexpected field type: 'books.author' (found 'ManyToOne' expected 'Column')"); }); + test('should throw when adding field with name including space', () => { + expect(() => + newPersons.registerComputed('full name', { + columnType: 'String', + dependencies: ['firstName', 'lastName'], + getValues: records => { + return new Promise(resolve => { + const result = records.map(record => `${record.firstName} ${record.lastName}`); + setTimeout(() => resolve(result)); + }); + }, + }), + ).toThrow( + `The name of field 'full name' you configured on 'persons' must not contain space.` + + ` Something like 'fullName' should work has expected.`, + ); + }); + describe('With a computed', () => { beforeEach(() => { newPersons.registerComputed('fullName', { diff --git a/packages/datasource-customizer/test/decorators/rename-field/collection.test.ts b/packages/datasource-customizer/test/decorators/rename-field/collection.test.ts index 71b3ff5b95..326da77e9b 100644 --- a/packages/datasource-customizer/test/decorators/rename-field/collection.test.ts +++ b/packages/datasource-customizer/test/decorators/rename-field/collection.test.ts @@ -142,7 +142,7 @@ describe('RenameFieldCollectionDecorator', () => { test('should throw when renaming with a name including space', () => { expect(() => newPersons.renameField('id', 'the key')).toThrow( - `The renaming of field 'id' must not contain space.` + + `The name of field 'the key' you configured on 'persons' must not contain space.` + ` Something like 'theKey' should work has expected.`, ); }); diff --git a/packages/datasource-toolkit/src/validation/field.ts b/packages/datasource-toolkit/src/validation/field.ts index bf699b50d4..907e7ac6ba 100644 --- a/packages/datasource-toolkit/src/validation/field.ts +++ b/packages/datasource-toolkit/src/validation/field.ts @@ -87,4 +87,15 @@ export default class FieldValidator { ); } } + + static validateName(collectionName: string, name: string) { + if (/ /.test(name)) { + const sanitizedName = name.replace(/ (.)/g, (_, s) => s.toUpperCase()); + throw new Error( + `The name of field '${name}' you configured on` + + ` '${collectionName}' must not contain space.` + + ` Something like '${sanitizedName}' should work has expected.`, + ); + } + } } diff --git a/packages/datasource-toolkit/test/validation/field/field.test.ts b/packages/datasource-toolkit/test/validation/field/field.test.ts new file mode 100644 index 0000000000..10af6b54f8 --- /dev/null +++ b/packages/datasource-toolkit/test/validation/field/field.test.ts @@ -0,0 +1,14 @@ +import FieldValidator from '../../../src/validation/field'; + +describe('FieldValidator', () => { + test('should fail with name conataining space', () => { + expect(() => FieldValidator.validateName('collectionName', 'field name')).toThrow( + `The name of field 'field name' you configured on 'collectionName' must not contain space.` + + ` Something like 'fieldName' should work has expected.`, + ); + }); + + test('should not fail with a valid name', () => { + expect(() => FieldValidator.validateName('collectionName', 'fieldName')).not.toThrow(); + }); +});