Skip to content

Commit

Permalink
fix(add field decorator): throw when put space inside fieldName (#767)
Browse files Browse the repository at this point in the history
  • Loading branch information
arnaud-moncel authored Jul 20, 2023
1 parent fa804d6 commit b6840ef
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
CollectionSchema,
DataSourceDecorator,
FieldSchema,
FieldValidator,
Filter,
PaginatedFilter,
Projection,
Expand Down Expand Up @@ -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]) {
Expand Down
51 changes: 45 additions & 6 deletions packages/datasource-customizer/test/collection-customizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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');
Expand All @@ -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];
Expand All @@ -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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
);
});
Expand Down
11 changes: 11 additions & 0 deletions packages/datasource-toolkit/src/validation/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
);
}
}
}
14 changes: 14 additions & 0 deletions packages/datasource-toolkit/test/validation/field/field.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});

0 comments on commit b6840ef

Please sign in to comment.