Skip to content

Commit

Permalink
CXM-5384 Fix fk validation with arrays of file names (#467)
Browse files Browse the repository at this point in the history 
It was recently discovered that even though we can specify an array of
file names in the `foreignKey` property of our mvom schemas to validate
the foreign key against, it is only ever validating against the first
file. This PR changes the file name/entity name separator character from
a `,` to a `#` since when an array of file names gets transformed to a
string it inserts commas between each of them. Thus, when performing
splitting logic later on in the process flow, it was not working
correctly. After making this change the file names, were correctly split
from the entity name, but additional splitting was needed to split each
of the file names back into an array to pass to the u2 subroutine.

---------

Co-authored-by: Ken Thompson <kat@storis.com>
  • Loading branch information
@rmattos500 @kthompson23
rmattos500 and kthompson23 committed May 5, 2023
1 parent e392306 commit fa8ab31
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 18 deletions.
13 changes: 8 additions & 5 deletions src/Document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface DocumentConstructorOptions {
}

export interface BuildForeignKeyDefinitionsResult {
filename: string;
filename: string[];
entityName: string;
entityIds: string[];
}
Expand Down Expand Up @@ -130,8 +130,8 @@ class Document {
return [];
}

// U2 does not allow commas in filenames so we can use it to separate filename/entityName combinations
const separator = ',';
// U2 does not allow pound signs in filenames so we can use it to separate filename/entityName combinations
const separator = '#';
const definitionMap = Array.from(this.#schema.paths).reduce(
(foreignKeyDefinitions, [keyPath, schemaType]) => {
const value = getIn(this, keyPath, null);
Expand Down Expand Up @@ -165,8 +165,11 @@ class Document {
return Array.from(definitionMap).reduce<BuildForeignKeyDefinitionsResult[]>(
(acc, [key, value]) => {
const keyParts = key.split(separator);
const filename = keyParts[0];
// Just incase the entity name included a comma, rejoin
const fileName = keyParts[0];
// If an array of filenames was provided, when we transformed the array into a string above, commas
// would have been inserted between each filename. Split the string back into an array.
const filename = fileName.split(',');
// Just incase the entity name included a pound sign, rejoin
const entityName = keyParts.slice(1).join(separator);
acc.push({ filename, entityName, entityIds: Array.from(value) });
return acc;
Expand Down
34 changes: 26 additions & 8 deletions src/__tests__/Document.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ describe('buildForeignKeyDefinitions', () => {
const document = new DocumentSubclass(schema, { data: { prop1: 'foo' } });

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: 'FILE', entityName: 'entityName', entityIds: ['foo'] },
{ filename: ['FILE'], entityName: 'entityName', entityIds: ['foo'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});
Expand All @@ -675,8 +675,8 @@ describe('buildForeignKeyDefinitions', () => {
const document = new DocumentSubclass(schema, { data: { prop1: 'foo', prop2: 'bar' } });

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: 'FILE1', entityName: 'entityName1', entityIds: ['foo'] },
{ filename: 'FILE2', entityName: 'entityName2', entityIds: ['bar'] },
{ filename: ['FILE1'], entityName: 'entityName1', entityIds: ['foo'] },
{ filename: ['FILE2'], entityName: 'entityName2', entityIds: ['bar'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});
Expand All @@ -691,7 +691,7 @@ describe('buildForeignKeyDefinitions', () => {
const document = new DocumentSubclass(schema, { data: { prop1: 'foo', prop2: 'bar' } });

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: 'FILE', entityName: 'entityName', entityIds: ['foo', 'bar'] },
{ filename: ['FILE'], entityName: 'entityName', entityIds: ['foo', 'bar'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});
Expand All @@ -707,7 +707,7 @@ describe('buildForeignKeyDefinitions', () => {
const document = new DocumentSubclass(schema, { data: { prop1: ['foo', 'bar'] } });

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: 'FILE', entityName: 'entityName', entityIds: ['foo', 'bar'] },
{ filename: ['FILE'], entityName: 'entityName', entityIds: ['foo', 'bar'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});
Expand Down Expand Up @@ -741,8 +741,8 @@ describe('buildForeignKeyDefinitions', () => {
});

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: 'FILE1', entityName: 'entityName', entityIds: ['foo', 'baz'] },
{ filename: 'FILE2', entityName: 'entityName', entityIds: ['bar', 'qux'] },
{ filename: ['FILE1'], entityName: 'entityName', entityIds: ['foo', 'baz'] },
{ filename: ['FILE2'], entityName: 'entityName', entityIds: ['bar', 'qux'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});
Expand All @@ -758,7 +758,25 @@ describe('buildForeignKeyDefinitions', () => {
const document = new DocumentSubclass(schema, { data: { _id: 'id', prop1: 'foo' } });

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: 'FILE', entityName: 'entityName', entityIds: ['id'] },
{ filename: ['FILE'], entityName: 'entityName', entityIds: ['id'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});

test('should build foreign key definitions with multiple filenames', () => {
const definition: SchemaDefinition = {
prop1: {
type: 'string',
path: '1',
foreignKey: { entityName: 'entityName', file: ['FILE1', 'FILE2'] },
},
};
const schema = new Schema(definition);

const document = new DocumentSubclass(schema, { data: { prop1: 'foo' } });

const expected: BuildForeignKeyDefinitionsResult[] = [
{ filename: ['FILE1', 'FILE2'], entityName: 'entityName', entityIds: ['foo'] },
];
expect(document.buildForeignKeyDefinitions()).toEqual(expected);
});
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/compileModel.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ describe('save', () => {
__v: null,
record: `prop1-value${am}123`,
foreignKeyDefinitions: [
{ entityIds: ['prop1-value'], entityName: 'prop1', filename: 'FK_FILE' },
{ entityIds: ['prop1-value'], entityName: 'prop1', filename: ['FK_FILE'] },
],
},
{},
Expand Down Expand Up @@ -723,7 +723,7 @@ describe('save', () => {
__v: null,
record: `prop1-value${am}123`,
foreignKeyDefinitions: [
{ entityIds: ['prop1-value'], entityName: 'prop1', filename: 'FK_FILE' },
{ entityIds: ['prop1-value'], entityName: 'prop1', filename: ['FK_FILE'] },
],
},
{},
Expand Down Expand Up @@ -934,7 +934,7 @@ describe('save', () => {
__v: null,
record: `prop1-value${am}123`,
foreignKeyDefinitions: [
{ entityIds: ['prop1-value'], entityName: 'prop1', filename: 'FK_FILE' },
{ entityIds: ['prop1-value'], entityName: 'prop1', filename: ['FK_FILE'] },
],
},
{ maxReturnPayloadSize, userDefined, requestId },
Expand Down
4 changes: 2 additions & 2 deletions src/schemaType/__tests__/DocumentArrayType.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ describe('transformForeignKeyDefinitionsToDb', () => {
value2.prop2 = 4.56;

const expected: ForeignKeyDbDefinition[] = [
{ filename: 'FILE', entityId: 'foo', entityName: 'FK_ENTITY' },
{ filename: 'FILE', entityId: 'bar', entityName: 'FK_ENTITY' },
{ filename: ['FILE'], entityId: 'foo', entityName: 'FK_ENTITY' },
{ filename: ['FILE'], entityId: 'bar', entityName: 'FK_ENTITY' },
];
expect(documentArrayType.transformForeignKeyDefinitionsToDb([value1, value2])).toEqual(
expected,
Expand Down

0 comments on commit fa8ab31

Please sign in to comment.