Skip to content

Commit a909ff3

Browse files
author
Dane Pilcher
authored
fix: validate references on belongsTo when cpk is disabled (#822)
1 parent 9a8d048 commit a909ff3

File tree

5 files changed

+101
-68
lines changed

5 files changed

+101
-68
lines changed

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

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ describe('custom references', () => {
523523
}
524524
`;
525525

526-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
526+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
527527
expect(visitor.generate()).toMatchSnapshot();
528528
});
529529

@@ -543,7 +543,7 @@ describe('custom references', () => {
543543
}
544544
`;
545545

546-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
546+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
547547
expect(visitor.generate()).toMatchSnapshot();
548548
});
549549

@@ -567,7 +567,7 @@ describe('custom references', () => {
567567
primary: Primary @belongsTo(references: ["primaryId"])
568568
}
569569
`;
570-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
570+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
571571
expect(visitor.generate()).toMatchSnapshot();
572572
});
573573

@@ -588,7 +588,7 @@ describe('custom references', () => {
588588
}
589589
`;
590590

591-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
591+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
592592
expect(visitor.generate()).toMatchSnapshot();
593593
});
594594

@@ -611,7 +611,7 @@ describe('custom references', () => {
611611
}
612612
`;
613613

614-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
614+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
615615
expect(visitor.generate()).toMatchSnapshot();
616616
});
617617

@@ -633,7 +633,7 @@ describe('custom references', () => {
633633
primary: Primary @belongsTo(references: ["primaryTenantId", "primaryInstanceId", "primaryRecordId"])
634634
}
635635
`;
636-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
636+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
637637
expect(visitor.generate()).toMatchSnapshot();
638638
});
639639

@@ -653,7 +653,7 @@ describe('custom references', () => {
653653
}
654654
`;
655655

656-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
656+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
657657
expect(() => visitor.generate())
658658
.toThrowError(`'fields' and 'references' cannot be used together.`);
659659
});
@@ -674,7 +674,7 @@ describe('custom references', () => {
674674
}
675675
`;
676676

677-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
677+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
678678
expect(() => visitor.generate())
679679
.toThrowError(`'fields' and 'references' cannot be used together.`);
680680
});
@@ -695,7 +695,7 @@ describe('custom references', () => {
695695
}
696696
`;
697697

698-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
698+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
699699
expect(() => visitor.generate())
700700
.toThrowError(`'fields' and 'references' cannot be used together.`);
701701
});
@@ -716,7 +716,7 @@ describe('custom references', () => {
716716
}
717717
`;
718718

719-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
719+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
720720
expect(() => visitor.generate())
721721
.toThrowError(`Error processing @hasOne directive on SqlPrimary.related. @belongsTo directive with references ["primaryId"] was not found in connected model SqlRelated`);
722722
});
@@ -737,11 +737,33 @@ describe('custom references', () => {
737737
}
738738
`;
739739

740-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
740+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
741741
expect(() => visitor.generate())
742742
.toThrowError(`Error processing @belongsTo directive on SqlRelated.primary. @hasOne or @hasMany directive with references ["primaryId"] was not found in connected model SqlPrimary`);
743743
});
744744

745+
test('throws error when missing references on hasMany related model when custom pk is disabled', () => {
746+
const schema = /* GraphQL */ `
747+
type SqlPrimary @refersTo(name: "sql_primary") @model {
748+
id: Int! @primaryKey
749+
content: String
750+
related: [SqlRelated] @hasMany(references: ["primaryId"])
751+
}
752+
753+
type SqlRelated @refersTo(name: "sql_related") @model {
754+
id: Int! @primaryKey
755+
content: String
756+
primaryId: Int! @refersTo(name: "primary_id") @index(name: "primary_id")
757+
primary: SqlPrimary @belongsTo
758+
}
759+
`;
760+
761+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: false });
762+
expect(() => visitor.generate())
763+
.toThrowError(`Error processing @hasMany directive on SqlPrimary.related. @belongsTo directive with references ["primaryId"] was not found in connected model SqlRelated`);
764+
});
765+
766+
745767
test('throws error when missing references on hasMany related model', () => {
746768
const schema = /* GraphQL */ `
747769
type SqlPrimary @refersTo(name: "sql_primary") @model {
@@ -758,7 +780,7 @@ describe('custom references', () => {
758780
}
759781
`;
760782

761-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
783+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
762784
expect(() => visitor.generate())
763785
.toThrowError(`Error processing @hasMany directive on SqlPrimary.related. @belongsTo directive with references ["primaryId"] was not found in connected model SqlRelated`);
764786
});
@@ -779,7 +801,7 @@ describe('custom references', () => {
779801
}
780802
`;
781803

782-
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema);
804+
const visitor: AppSyncModelIntrospectionVisitor = getVisitor(schema, { respectPrimaryKeyAttributesOnConnectionField: true });
783805
expect(() => visitor.generate())
784806
.toThrowError(`Error processing @belongsTo directive on SqlRelated.primary. @hasOne or @hasMany directive with references ["primaryId"] was not found in connected model SqlPrimary`);
785807
});

‎packages/appsync-modelgen-plugin/src/utils/process-belongs-to.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
flattenFieldDirectives,
77
makeConnectionAttributeName,
88
} from './process-connections';
9-
import { getConnectedFieldV2, fieldsAndReferencesErrorMessage } from './process-connections-v2';
9+
import { getConnectedFieldV2, getConnectedFieldForReferences } from './process-connections-v2';
1010

1111

1212
export function processBelongsToConnection(
@@ -28,20 +28,21 @@ export function processBelongsToConnection(
2828
`A 'belongsTo' field should match to a corresponding 'hasMany' or 'hasOne' field`
2929
);
3030
}
31+
const references = connectionDirective.arguments.references || [];
32+
const isUsingReferences = references.length > 0;
33+
if (isUsingReferences) {
34+
// ensure there is a matching hasOne/hasMany field with references
35+
getConnectedFieldForReferences(field, model, otherSide, connectionDirective.name)
36+
}
37+
3138
const otherSideField = isCustomPKEnabled ? otherSideConnectedFields[0] : getConnectedFieldV2(field, model, otherSide, connectionDirective.name);
3239
const connectionFields = connectionDirective.arguments.fields || [];
3340

34-
const references = connectionDirective.arguments.references || [];
35-
36-
if (connectionFields.length > 0 && references.length > 0) {
37-
throw new Error(fieldsAndReferencesErrorMessage);
38-
}
3941
// if a type is connected using name, then amplify-graphql-relational-transformer adds a field to
4042
// track the connection and that field is not part of the selection set
4143
// but if the field are connected using fields argument in connection directive
4244
// we are reusing the field and it should be preserved in selection set
4345
const otherSideHasMany = otherSideField.isList;
44-
const isUsingReferences = references.length > 0;
4546
// New metada type introduced by custom PK v2 support
4647
let targetNames: string[] = [ ...connectionFields, ...references ];
4748
if (targetNames.length === 0) {

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

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,8 @@ export function getConnectedFieldV2(
1919
if (!connectionInfo) {
2020
throw new Error(`The ${field.name} on model ${model.name} is not connected`);
2121
}
22-
23-
const references = connectionInfo.arguments.references;
2422
if (connectionInfo.name === 'belongsTo') {
2523
let connectedFieldsBelongsTo = getBelongsToConnectedFields(model, connectedModel);
26-
if (references) {
27-
const connectedField = connectedFieldsBelongsTo.find((field) => {
28-
return field.directives.some((dir) => {
29-
return (dir.name === 'hasOne' || dir.name === 'hasMany')
30-
&& dir.arguments.references
31-
&& JSON.stringify(dir.arguments.references) === JSON.stringify(connectionInfo.arguments.references);
32-
});
33-
});
34-
if (!connectedField) {
35-
throw new Error(`Error processing @belongsTo directive on ${model.name}.${field.name}. @hasOne or @hasMany directive with references ${JSON.stringify(connectionInfo.arguments?.references)} was not found in connected model ${connectedModel.name}`);
36-
}
37-
return connectedField;
38-
}
3924

4025
if (connectedFieldsBelongsTo.length === 1) {
4126
return connectedFieldsBelongsTo[0];
@@ -44,25 +29,10 @@ export function getConnectedFieldV2(
4429

4530
const indexName = connectionInfo.arguments.indexName;
4631
const connectionFields = connectionInfo.arguments.fields;
47-
if (connectionFields && references) {
48-
throw new Error(fieldsAndReferencesErrorMessage);
49-
}
50-
if (references || connectionFields || directiveName === 'hasOne') {
32+
if (connectionFields || directiveName === 'hasOne') {
5133
let connectionDirective;
52-
if (references) {
53-
if (connectionInfo) {
54-
connectionDirective = flattenFieldDirectives(connectedModel).find((dir) => {
55-
return dir.arguments.references
56-
&& JSON.stringify(dir.arguments.references) === JSON.stringify(connectionInfo.arguments.references);
57-
});
58-
if (!connectionDirective) {
59-
throw new Error(`Error processing @${connectionInfo.name} directive on ${model.name}.${field.name}. @belongsTo directive with references ${JSON.stringify(connectionInfo.arguments?.references)} was not found in connected model ${connectedModel.name}`);
60-
}
61-
}
62-
}
63-
6434
// Find gsi on other side if index is defined
65-
else if (indexName) {
35+
if (indexName) {
6636
connectionDirective = flattenFieldDirectives(connectedModel).find(dir => {
6737
return dir.name === 'index' && dir.arguments.name === indexName;
6838
});
@@ -154,6 +124,56 @@ export function getConnectedFieldV2(
154124
};
155125
}
156126

127+
export function getConnectedFieldForReferences(
128+
field: CodeGenField,
129+
model: CodeGenModel,
130+
connectedModel: CodeGenModel,
131+
directiveName: string,
132+
): CodeGenField {
133+
const connectionInfo = getDirective(field)(directiveName);
134+
if (!connectionInfo) {
135+
throw new Error(`The ${field.name} on model ${model.name} is not connected`);
136+
}
137+
const references = connectionInfo.arguments.references;
138+
if (!references) {
139+
throw new Error(`The ${field.name} on model ${model.name} does not have references.`);
140+
}
141+
const connectionFields = connectionInfo.arguments.fields;
142+
if (connectionFields && references) {
143+
throw new Error(`'fields' and 'references' cannot be used together.`);
144+
}
145+
146+
if (connectionInfo.name === 'belongsTo') {
147+
let connectedFieldsBelongsTo = getBelongsToConnectedFields(model, connectedModel);
148+
const connectedField = connectedFieldsBelongsTo.find((field) => {
149+
return field.directives.some((dir) => {
150+
return (dir.name === 'hasOne' || dir.name === 'hasMany')
151+
&& dir.arguments.references
152+
&& JSON.stringify(dir.arguments.references) === JSON.stringify(connectionInfo.arguments.references);
153+
});
154+
});
155+
if (!connectedField) {
156+
throw new Error(`Error processing @belongsTo directive on ${model.name}.${field.name}. @hasOne or @hasMany directive with references ${JSON.stringify(connectionInfo.arguments?.references)} was not found in connected model ${connectedModel.name}`);
157+
}
158+
return connectedField;
159+
}
160+
161+
// hasOne and hasMany
162+
const connectionDirective = flattenFieldDirectives(connectedModel).find((dir) => {
163+
return dir.arguments.references
164+
&& JSON.stringify(dir.arguments.references) === JSON.stringify(connectionInfo.arguments.references);
165+
});
166+
if (!connectionDirective) {
167+
throw new Error(`Error processing @${connectionInfo.name} directive on ${model.name}.${field.name}. @belongsTo directive with references ${JSON.stringify(connectionInfo.arguments?.references)} was not found in connected model ${connectedModel.name}`);
168+
}
169+
const connectedFieldName = ((fieldDir: CodeGenFieldDirective) => {
170+
return fieldDir.fieldName;
171+
})(connectionDirective as CodeGenFieldDirective)
172+
173+
const connectedField = connectedModel.fields.find(f => f.name === connectedFieldName);
174+
return connectedField!;
175+
}
176+
157177
export function processConnectionsV2(
158178
field: CodeGenField,
159179
model: CodeGenModel,
@@ -177,5 +197,3 @@ export function processConnectionsV2(
177197
}
178198
}
179199
}
180-
181-
export const fieldsAndReferencesErrorMessage = `'fields' and 'references' cannot be used together.`;

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
flattenFieldDirectives,
1010
CodeGenFieldConnectionHasMany,
1111
} from './process-connections';
12-
import { getConnectedFieldV2, fieldsAndReferencesErrorMessage } from './process-connections-v2';
12+
import { getConnectedFieldV2, getConnectedFieldForReferences } from './process-connections-v2';
1313

1414

1515
export function processHasManyConnection(
@@ -27,15 +27,11 @@ export function processHasManyConnection(
2727
const connectionFields = connectionDirective.arguments.fields || [];
2828
const references = connectionDirective.arguments.references || [];
2929

30-
if (connectionFields.length > 0 && references.length > 0) {
31-
throw new Error(fieldsAndReferencesErrorMessage);
32-
}
33-
3430
if (references.length > 0) {
3531
// native uses the connected field instead of associatedWithFields
3632
// when using references associatedWithFields and associatedWithNative are not the same
37-
// getConnectedFieldV2 also ensures there is a matching belongsTo field with references
38-
const associatedWithNativeReferences = getConnectedFieldV2(field, model, otherSide, connectionDirective.name, shouldUseModelNameFieldInHasManyAndBelongsTo)
33+
// getConnectedFieldForRerences also ensures there is a matching belongsTo field with references
34+
const associatedWithNativeReferences = getConnectedFieldForReferences(field, model, otherSide, connectionDirective.name)
3935
const associatedWithFields = references.map((reference: string) => otherSide.fields.find((field) => reference === field.name))
4036
return {
4137
kind: CodeGenConnectionType.HAS_MANY,

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
CodeGenFieldConnection,
55
makeConnectionAttributeName,
66
} from './process-connections';
7-
import { getConnectedFieldV2, fieldsAndReferencesErrorMessage } from './process-connections-v2';
7+
import { getConnectedFieldV2, getConnectedFieldForReferences } from './process-connections-v2';
88
import { getModelPrimaryKeyComponentFields } from './fieldUtils';
99
import { getOtherSideBelongsToField } from './fieldUtils';
1010

@@ -26,16 +26,12 @@ export function processHasOneConnection(
2626
const connectionFields = connectionDirective.arguments.fields || [];
2727
const references = connectionDirective.arguments.references || [];
2828

29-
if (connectionFields.length > 0 && references.length > 0) {
30-
throw new Error(fieldsAndReferencesErrorMessage);
31-
}
32-
3329
let associatedWithFields;
3430
if (references.length > 0) {
3531
// native uses the connected field instead of associatedWithFields
3632
// when using references associatedWithFields and associatedWithNative are not the same
37-
// getConnectedFieldV2 also ensures there is a matching belongsTo field with references
38-
const associatedWithNativeReferences = getConnectedFieldV2(field, model, otherSide, connectionDirective.name);
33+
// getConnectedFieldForReferences also ensures there is a matching belongsTo field with references
34+
const associatedWithNativeReferences = getConnectedFieldForReferences(field, model, otherSide, connectionDirective.name)
3935
associatedWithFields = references.map((reference: string) => otherSide.fields.find((field) => reference === field.name))
4036
return {
4137
kind: CodeGenConnectionType.HAS_ONE,

0 commit comments

Comments
 (0)