Skip to content

Commit 84f1377

Browse files
authored
Merge pull request #530 from aws-amplify/bidirectional-mipr-fix
fix(appsync-modelgen-plugin): associatedWith fields in HasOne bi-directional with CPK enabled
2 parents 48e1b43 + 273ad18 commit 84f1377

10 files changed

+117
-15
lines changed

packages/appsync-modelgen-plugin/src/__tests__/utils/process-connections-v2.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { buildSchema, parse, visit } from 'graphql';
1010
import { directives, scalars } from '../../scalars/supported-directives';
1111
import { AppSyncModelVisitor, CodeGenGenerateEnum } from '../../visitors/appsync-visitor';
12+
1213
describe('GraphQL V2 process connections tests', () => {
1314
describe('GraphQL vNext getConnectedField tests with @primaryKey and @index', () => {
1415
let hasOneWithFieldsModelMap: CodeGenModelMap;
@@ -625,6 +626,7 @@ describe('GraphQL V2 process connections tests', () => {
625626
});
626627
});
627628
});
629+
628630
describe('Connection process with custom Primary Key support tests', () => {
629631
const createBaseVisitorWithCustomPrimaryKeyEnabled = (schema: string) => {
630632
const visitorConfig = {
@@ -901,6 +903,7 @@ describe('Connection process with custom Primary Key support tests', () => {
901903
});
902904
});
903905
});
906+
904907
describe('belongsTo special case tests', () => {
905908
it('should return correct connection info in multiple hasOne defined in parent', () => {
906909
const schema = /* GraphQL */ `
@@ -951,5 +954,83 @@ describe('Connection process with custom Primary Key support tests', () => {
951954
isConnectingFieldAutoCreated: false,
952955
});
953956
});
957+
958+
it('With shouldUseFieldsInAssociatedWithInHasOne and CPK enabled, should return correct connection info for bi-directional has-one with composite primary key', () => {
959+
const schema = /* GraphQL */ `
960+
type CompositeOwner @model {
961+
lastName: ID! @primaryKey(sortKeyFields: ["firstName"])
962+
firstName: String!
963+
compositeDog: CompositeDog @hasOne
964+
}
965+
type CompositeDog @model {
966+
name: ID! @primaryKey(sortKeyFields: ["description"])
967+
description: String!
968+
compositeOwner: CompositeOwner @belongsTo
969+
}
970+
`;
971+
const modelMap: CodeGenModelMap = createBaseVisitorWithCustomPrimaryKeyEnabled(schema).models;
972+
const compositeOwner: CodeGenModel = modelMap.CompositeOwner;
973+
const compositeDog: CodeGenModel = modelMap.CompositeDog;
974+
const hasOneField = compositeOwner.fields.find(f => f.name === 'compositeDog')!;
975+
const belongsToField = compositeDog.fields.find(f => f.name === 'compositeOwner')!;
976+
const hasOneAssociatedWithFields = compositeDog.fields.filter(f => f.name === 'name' || f.name === 'description')!;
977+
const hasOneRelationInfo = processConnectionsV2(hasOneField, compositeOwner, modelMap, false, true, true);
978+
const belongsToRelationInfo = processConnectionsV2(belongsToField, compositeDog, modelMap, false, true, true);
979+
expect(hasOneRelationInfo).toEqual({
980+
kind: CodeGenConnectionType.HAS_ONE,
981+
associatedWith: hasOneAssociatedWithFields[0],
982+
associatedWithFields: hasOneAssociatedWithFields,
983+
targetName: 'compositeOwnerCompositeDogName',
984+
targetNames: ['compositeOwnerCompositeDogName', 'compositeOwnerCompositeDogDescription'],
985+
connectedModel: compositeDog,
986+
isConnectingFieldAutoCreated: true,
987+
});
988+
expect(belongsToRelationInfo).toEqual({
989+
kind: CodeGenConnectionType.BELONGS_TO,
990+
targetName: 'compositeDogCompositeOwnerLastName',
991+
targetNames: ['compositeDogCompositeOwnerLastName', 'compositeDogCompositeOwnerFirstName'],
992+
connectedModel: compositeOwner,
993+
isConnectingFieldAutoCreated: false,
994+
});
995+
});
996+
997+
it('With shouldUseFieldsInAssociatedWithInHasOne and CPK enabled, should return correct connection info for bi-directional has-one without Composite primary key', () => {
998+
const schema = /* GraphQL */ `
999+
type BoringOwner @model {
1000+
id: ID!
1001+
name: String
1002+
boringDog: BoringDog @hasOne
1003+
}
1004+
type BoringDog @model {
1005+
id: ID!
1006+
name: String
1007+
boringOwner: BoringOwner @belongsTo
1008+
}
1009+
`;
1010+
const modelMap: CodeGenModelMap = createBaseVisitorWithCustomPrimaryKeyEnabled(schema).models;
1011+
const boringOwner: CodeGenModel = modelMap.BoringOwner;
1012+
const boringDog: CodeGenModel = modelMap.BoringDog;
1013+
const hasOneField = boringOwner.fields.find(f => f.name === 'boringDog')!;
1014+
const belongsToField = boringDog.fields.find(f => f.name === 'boringOwner')!;
1015+
const hasOneAssociatedWithFields = boringDog.fields.filter(f => f.name === 'id')!;
1016+
const hasOneRelationInfo = processConnectionsV2(hasOneField, boringOwner, modelMap, false, true, true);
1017+
const belongsToRelationInfo = processConnectionsV2(belongsToField, boringDog, modelMap, false, true, true);
1018+
expect(hasOneRelationInfo).toEqual({
1019+
kind: CodeGenConnectionType.HAS_ONE,
1020+
associatedWith: hasOneAssociatedWithFields[0],
1021+
associatedWithFields: hasOneAssociatedWithFields,
1022+
targetName: 'boringOwnerBoringDogId',
1023+
targetNames: ['boringOwnerBoringDogId'],
1024+
connectedModel: boringDog,
1025+
isConnectingFieldAutoCreated: true,
1026+
});
1027+
expect(belongsToRelationInfo).toEqual({
1028+
kind: CodeGenConnectionType.BELONGS_TO,
1029+
targetName: 'boringDogBoringOwnerId',
1030+
targetNames: ['boringDogBoringOwnerId'],
1031+
connectedModel: boringOwner,
1032+
isConnectingFieldAutoCreated: false,
1033+
});
1034+
});
9541035
})
9551036
});

packages/appsync-modelgen-plugin/src/__tests__/visitors/__snapshots__/appsync-json-metadata-visitor.test.ts.snap

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,8 @@ exports[`Metadata visitor for custom PK support relation metadata for hasOne/bel
615615
\\"association\\": {
616616
\\"connectionType\\": \\"HAS_ONE\\",
617617
\\"associatedWith\\": [
618-
\\"project\\"
618+
\\"id\\",
619+
\\"name\\"
619620
],
620621
\\"targetNames\\": [
621622
\\"projectTeamId\\",
@@ -795,7 +796,8 @@ export const schema: Schema = {
795796
\\"association\\": {
796797
\\"connectionType\\": \\"HAS_ONE\\",
797798
\\"associatedWith\\": [
798-
\\"project\\"
799+
\\"id\\",
800+
\\"name\\"
799801
],
800802
\\"targetNames\\": [
801803
\\"projectTeamId\\",

packages/appsync-modelgen-plugin/src/__tests__/visitors/__snapshots__/appsync-model-introspection-visitor.test.ts.snap

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ exports[`Custom primary key tests should generate correct model intropection fil
3232
\\"association\\": {
3333
\\"connectionType\\": \\"HAS_ONE\\",
3434
\\"associatedWith\\": [
35-
\\"project\\"
35+
\\"teamId\\",
36+
\\"name\\"
3637
],
3738
\\"targetNames\\": [
3839
\\"project1TeamTeamId\\",
@@ -365,7 +366,8 @@ exports[`Custom primary key tests should generate correct model intropection fil
365366
\\"association\\": {
366367
\\"connectionType\\": \\"HAS_ONE\\",
367368
\\"associatedWith\\": [
368-
\\"project\\"
369+
\\"teamId\\",
370+
\\"name\\"
369371
],
370372
\\"targetNames\\": [
371373
\\"teamId\\",

packages/appsync-modelgen-plugin/src/utils/process-connections-v2.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,15 @@ export function processConnectionsV2(
129129
model: CodeGenModel,
130130
modelMap: CodeGenModelMap,
131131
shouldUseModelNameFieldInHasManyAndBelongsTo: boolean = false,
132-
isCustomPKEnabled: boolean = false
132+
isCustomPKEnabled: boolean = false,
133+
shouldUseFieldsInAssociatedWithInHasOne: boolean = false,
133134
): CodeGenFieldConnection | undefined {
134135
const connectionDirective = field.directives.find(d => d.name === 'hasOne' || d.name === 'hasMany' || d.name === 'belongsTo');
135136

136137
if (connectionDirective) {
137138
switch (connectionDirective.name) {
138139
case 'hasOne':
139-
return processHasOneConnection(field, model, modelMap, connectionDirective, isCustomPKEnabled);
140+
return processHasOneConnection(field, model, modelMap, connectionDirective, isCustomPKEnabled, shouldUseFieldsInAssociatedWithInHasOne);
140141
case 'belongsTo':
141142
return processBelongsToConnection(field, model, modelMap, connectionDirective, isCustomPKEnabled);
142143
case 'hasMany':

packages/appsync-modelgen-plugin/src/utils/process-has-one.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ export function processHasOneConnection(
1313
model: CodeGenModel,
1414
modelMap: CodeGenModelMap,
1515
connectionDirective: CodeGenDirective,
16-
isCustomPKEnabled: boolean = false
16+
isCustomPKEnabled: boolean = false,
17+
shouldUseFieldsInAssociatedWithInHasOne:boolean = false
1718
): CodeGenFieldConnection | undefined {
1819
const otherSide = modelMap[field.type];
1920
// Find other side belongsTo field when in bi direction connection
@@ -23,7 +24,7 @@ export function processHasOneConnection(
2324
}
2425
let associatedWithFields;
2526
if (isCustomPKEnabled) {
26-
associatedWithFields = getConnectedFieldsForHasOne(otherSideBelongsToField, otherSide);
27+
associatedWithFields = getConnectedFieldsForHasOne(otherSideBelongsToField, otherSide, shouldUseFieldsInAssociatedWithInHasOne);
2728
} else {
2829
const otherSideField = getConnectedFieldV2(field, model, otherSide, connectionDirective.name);
2930
associatedWithFields = [otherSideField];
@@ -57,8 +58,9 @@ export function processHasOneConnection(
5758
* Get connected fields for hasOne relation
5859
* @param otherSideBelongsToField belongsTo field on other side
5960
* @param otherSide other side model of hasOne connection
61+
* @param shouldUseFieldsInAssociatedWithInHasOne Return the connected fields instead of the connected model
6062
* @returns Array of connected fields. Return belongsTo field when in bi direction connenction. Otherwise return child pk and sk fields
6163
*/
62-
export function getConnectedFieldsForHasOne(otherSideBelongsToField: CodeGenField | undefined, otherSide: CodeGenModel): CodeGenField[] {
63-
return otherSideBelongsToField ? [otherSideBelongsToField] : getModelPrimaryKeyComponentFields(otherSide);
64+
export function getConnectedFieldsForHasOne(otherSideBelongsToField: CodeGenField | undefined, otherSide: CodeGenModel, shouldUseFieldsInAssociatedWithInHasOne: boolean = false): CodeGenField[] {
65+
return (!shouldUseFieldsInAssociatedWithInHasOne && otherSideBelongsToField) ? [otherSideBelongsToField] : getModelPrimaryKeyComponentFields(otherSide);
6466
}

packages/appsync-modelgen-plugin/src/visitors/appsync-javascript-visitor.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,11 @@ export class AppSyncModelJavascriptVisitor<
4747
const shouldUseModelNameFieldInHasManyAndBelongsTo = false;
4848
// This flag is going to be used to tight-trigger on JS implementations only.
4949
const shouldImputeKeyForUniDirectionalHasMany = true;
50+
const shouldUseFieldsInAssociatedWithInHasOne = true;
5051
this.processDirectives(
5152
shouldUseModelNameFieldInHasManyAndBelongsTo,
52-
shouldImputeKeyForUniDirectionalHasMany
53+
shouldImputeKeyForUniDirectionalHasMany,
54+
shouldUseFieldsInAssociatedWithInHasOne
5355
);
5456

5557
if (this._parsedConfig.isDeclaration) {

packages/appsync-modelgen-plugin/src/visitors/appsync-json-metadata-visitor.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,12 @@ export class AppSyncJSONVisitor<
114114
const shouldUseModelNameFieldInHasManyAndBelongsTo = false;
115115
// This flag is going to be used to tight-trigger on JS implementations only.
116116
const shouldImputeKeyForUniDirectionalHasMany = true;
117+
const shouldUseFieldsInAssociatedWithInHasOne = true;
118+
117119
this.processDirectives(
118120
shouldUseModelNameFieldInHasManyAndBelongsTo,
119-
shouldImputeKeyForUniDirectionalHasMany
121+
shouldImputeKeyForUniDirectionalHasMany,
122+
shouldUseFieldsInAssociatedWithInHasOne
120123
);
121124

122125
if (this._parsedConfig.metadataTarget === 'typescript') {

packages/appsync-modelgen-plugin/src/visitors/appsync-model-introspection-visitor.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ export class AppSyncModelIntrospectionVisitor<
3131
const shouldUseModelNameFieldInHasManyAndBelongsTo = false;
3232
// This flag is going to be used to tight-trigger on JS implementations only.
3333
const shouldImputeKeyForUniDirectionalHasMany = true;
34+
const shouldUseFieldsInAssociatedWithInHasOne = true;
3435
this.processDirectives(
3536
shouldUseModelNameFieldInHasManyAndBelongsTo,
36-
shouldImputeKeyForUniDirectionalHasMany
37+
shouldImputeKeyForUniDirectionalHasMany,
38+
shouldUseFieldsInAssociatedWithInHasOne
3739
);
3840

3941
const modelIntrosepctionSchema = this.generateModelIntrospectionSchema();

packages/appsync-modelgen-plugin/src/visitors/appsync-typescript-visitor.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ export class AppSyncModelTypeScriptVisitor<
4444
const shouldUseModelNameFieldInHasManyAndBelongsTo = false;
4545
// This flag is going to be used to tight-trigger on JS implementations only.
4646
const shouldImputeKeyForUniDirectionalHasMany = true;
47+
const shouldUseFieldsInAssociatedWithInHasOne = true;
4748
this.processDirectives(
4849
shouldUseModelNameFieldInHasManyAndBelongsTo,
49-
shouldImputeKeyForUniDirectionalHasMany
50+
shouldImputeKeyForUniDirectionalHasMany,
51+
shouldUseFieldsInAssociatedWithInHasOne
5052
);
5153
const imports = this.generateImports();
5254
const enumDeclarations = Object.values(this.enumMap)

packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,12 +328,15 @@ export class AppSyncModelVisitor<
328328
shouldUseModelNameFieldInHasManyAndBelongsTo: boolean,
329329
// This flag is going to be used to tight-trigger on JS implementations only.
330330
shouldImputeKeyForUniDirectionalHasMany: boolean,
331+
// This flag is currently used in JS/TS and Model introspection generation only.
332+
shouldUseFieldsInAssociatedWithInHasOne: boolean = false
331333
) {
332334
if (this.config.usePipelinedTransformer || this.config.transformerVersion === 2) {
333335
this.processV2KeyDirectives();
334336
this.processConnectionDirectivesV2(
335337
shouldUseModelNameFieldInHasManyAndBelongsTo,
336-
shouldImputeKeyForUniDirectionalHasMany
338+
shouldImputeKeyForUniDirectionalHasMany,
339+
shouldUseFieldsInAssociatedWithInHasOne
337340
);
338341
} else {
339342
this.processConnectionDirective();
@@ -898,6 +901,7 @@ export class AppSyncModelVisitor<
898901
shouldUseModelNameFieldInHasManyAndBelongsTo: boolean,
899902
// This flag is going to be used to tight-trigger on JS implementations only.
900903
shouldImputeKeyForUniDirectionalHasMany: boolean,
904+
shouldUseFieldsInAssociatedWithInHasOne: boolean
901905
): void {
902906
this.processManyToManyDirectives();
903907

@@ -911,6 +915,7 @@ export class AppSyncModelVisitor<
911915
this.modelMap,
912916
shouldUseModelNameFieldInHasManyAndBelongsTo,
913917
isCustomPKEnabled,
918+
shouldUseFieldsInAssociatedWithInHasOne
914919
);
915920
if (connectionInfo) {
916921
if (connectionInfo.kind === CodeGenConnectionType.HAS_MANY) {

0 commit comments

Comments
 (0)