From 927bc7fd21385beb2862ea5c39f7204a2a617c34 Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Wed, 2 Oct 2024 15:47:10 -0600 Subject: [PATCH] fix: allow required uni-directional hasOne with fields --- .../utils/process-connections.test.ts | 85 ++++++++++++++++--- .../src/utils/process-connections.ts | 3 +- .../src/visitors/appsync-visitor.ts | 4 +- 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/packages/appsync-modelgen-plugin/src/__tests__/utils/process-connections.test.ts b/packages/appsync-modelgen-plugin/src/__tests__/utils/process-connections.test.ts index 29a7bb073..83d229228 100644 --- a/packages/appsync-modelgen-plugin/src/__tests__/utils/process-connections.test.ts +++ b/packages/appsync-modelgen-plugin/src/__tests__/utils/process-connections.test.ts @@ -56,7 +56,7 @@ describe('process connection', () => { it('should return HAS_MANY for Post.comments field connection info', () => { const commentsField = modelMap.Post.fields[0]; - const connectionInfo = (processConnections(commentsField, modelMap.Post, modelMap) as any) as CodeGenFieldConnectionHasMany; + const connectionInfo = (processConnections(commentsField, modelMap.Post, modelMap, true) as any) as CodeGenFieldConnectionHasMany; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.HAS_MANY); @@ -66,7 +66,7 @@ describe('process connection', () => { it('should return BELONGS_TO for Comment.post field connection info', () => { const postField = modelMap.Comment.fields[0]; - const connectionInfo = (processConnections(postField, modelMap.Comment, modelMap) as any) as CodeGenFieldConnectionBelongsTo; + const connectionInfo = (processConnections(postField, modelMap.Comment, modelMap, true) as any) as CodeGenFieldConnectionBelongsTo; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.BELONGS_TO); @@ -120,7 +120,7 @@ describe('process connection', () => { it('should return HAS_ONE Person.license field', () => { const licenseField = modelMap.Person.fields[0]; - const connectionInfo = (processConnections(licenseField, modelMap.Person, modelMap) as any) as CodeGenFieldConnectionHasOne; + const connectionInfo = (processConnections(licenseField, modelMap.Person, modelMap, true) as any) as CodeGenFieldConnectionHasOne; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.HAS_ONE); expect(connectionInfo.associatedWith).toEqual(modelMap.License.fields[0]); @@ -130,7 +130,7 @@ describe('process connection', () => { it('should return BELONGS_TO License.person field', () => { const personField = modelMap.License.fields[0]; - const connectionInfo = (processConnections(personField, modelMap.License, modelMap) as any) as CodeGenFieldConnectionBelongsTo; + const connectionInfo = (processConnections(personField, modelMap.License, modelMap, true) as any) as CodeGenFieldConnectionBelongsTo; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.BELONGS_TO); expect(connectionInfo.isConnectingFieldAutoCreated).toEqual(true); @@ -141,9 +141,74 @@ describe('process connection', () => { // Make person field optional personField.isNullable = true; expect(() => { - processConnections(personField, modelMap.License, modelMap); + processConnections(personField, modelMap.License, modelMap, true); }).toThrowError('DataStore does not support 1 to 1 connection with both sides of connection as optional field'); }); + + it('uni-directional One:One connection with required field and datastore is not enabled', () => { + const schema = /* GraphQL */ ` + type User @model { + id: ID! + } + type Session @model { + id: ID! + sessionUserId: ID! + user: User! @connection(fields: ["sessionUserId"]) + } + `; + + const modelMap: CodeGenModelMap = { + User: { + name: 'User', + type: 'model', + directives: [], + fields: [ + { + type: 'ID', + isNullable: false, + isList: false, + name: 'id', + directives: [], + }, + ], + }, + Session: { + name: 'Session', + type: 'model', + directives: [], + fields: [ + { + type: 'ID', + isNullable: false, + isList: false, + name: 'id', + directives: [], + }, + { + type: 'ID', + isNullable: false, + isList: false, + name: 'sessionUserId', + directives: [], + }, + { + type: 'User', + isNullable: false, + isList: false, + name: 'user', + directives: [{ name: 'connection', arguments: { fields: ['sessionUserId'] } }], + }, + ], + }, + }; + + modelMap.Session.fields[2] + + const connectionInfo = (processConnections(modelMap.Session.fields[2], modelMap.User, modelMap, false) as any) as CodeGenFieldConnectionBelongsTo; + expect(connectionInfo).toBeDefined(); + expect(connectionInfo.kind).toEqual(CodeGenConnectionType.HAS_ONE); + expect(connectionInfo.isConnectingFieldAutoCreated).toEqual(false); + }); }); }); describe('Uni-directional connection (unnamed connection)', () => { @@ -192,7 +257,7 @@ describe('process connection', () => { it('should return HAS_MANY for Post.comments', () => { const commentsField = modelMap.Post.fields[0]; - const connectionInfo = (processConnections(commentsField, modelMap.Post, modelMap) as any) as CodeGenFieldConnectionHasMany; + const connectionInfo = (processConnections(commentsField, modelMap.Post, modelMap, true) as any) as CodeGenFieldConnectionHasMany; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.HAS_MANY); @@ -208,7 +273,7 @@ describe('process connection', () => { it('should return BELONGS_TO for Comment.post', () => { const commentsField = modelMap.Comment.fields[0]; - const connectionInfo = (processConnections(commentsField, modelMap.Comment, modelMap) as any) as CodeGenFieldConnectionBelongsTo; + const connectionInfo = (processConnections(commentsField, modelMap.Comment, modelMap, true) as any) as CodeGenFieldConnectionBelongsTo; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.BELONGS_TO); @@ -280,12 +345,12 @@ describe('process connection', () => { it('should not throw error if connection directive has keyName', () => { const commentsField = modelMap.Post.fields[0]; - expect(() => processConnections(commentsField, modelMap.Post, modelMap)).not.toThrowError(); + expect(() => processConnections(commentsField, modelMap.Post, modelMap, true)).not.toThrowError(); }); it('should support connection with @key on BELONGS_TO side', () => { const postField = modelMap.Comment.fields[2]; - const connectionInfo = (processConnections(postField, modelMap.Post, modelMap) as any) as CodeGenFieldConnectionBelongsTo; + const connectionInfo = (processConnections(postField, modelMap.Post, modelMap, true) as any) as CodeGenFieldConnectionBelongsTo; expect(connectionInfo).toBeDefined(); expect(connectionInfo.kind).toEqual(CodeGenConnectionType.BELONGS_TO); expect(connectionInfo.targetName).toEqual(modelMap.Comment.fields[0].name); @@ -462,4 +527,4 @@ describe('process connection', () => { expect(getConnectedField(subordinateField, employeeModel, employeeModel)).toEqual(supervisorField); }); }); -}); \ No newline at end of file +}); diff --git a/packages/appsync-modelgen-plugin/src/utils/process-connections.ts b/packages/appsync-modelgen-plugin/src/utils/process-connections.ts index e2f33113e..94b2c8e0a 100644 --- a/packages/appsync-modelgen-plugin/src/utils/process-connections.ts +++ b/packages/appsync-modelgen-plugin/src/utils/process-connections.ts @@ -128,6 +128,7 @@ export function processConnections( field: CodeGenField, model: CodeGenModel, modelMap: CodeGenModelMap, + isDataStoreEnabled: boolean, ): CodeGenFieldConnection | undefined { const connectionDirective = field.directives.find(d => d.name === 'connection'); if (connectionDirective) { @@ -189,7 +190,7 @@ export function processConnections( targetName: connectionFields[0] || makeConnectionAttributeName(model.name, field.name), targetNames: [] // New attribute for v2 custom pk support. Not used in v1 so use empty array. }; - } else if (field.isNullable && !otherSideField.isNullable) { + } else if ((field.isNullable && !otherSideField.isNullable) || !isDataStoreEnabled) { /* # model type License { # belongsTo diff --git a/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts b/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts index cb2cea965..b1470e59b 100644 --- a/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts +++ b/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts @@ -169,7 +169,7 @@ export interface ParsedAppSyncModelConfig extends ParsedConfig { selectedType?: string; generate?: CodeGenGenerateEnum; target?: string; - isDataStoreEnabled?: string; + isDataStoreEnabled?: boolean; isTimestampFieldsAdded?: boolean; handleListNullabilityTransparently?: boolean; usePipelinedTransformer?: boolean; @@ -818,7 +818,7 @@ export class AppSyncModelVisitor< protected processConnectionDirective(): void { Object.values(this.modelMap).forEach(model => { model.fields.forEach(field => { - const connectionInfo = processConnections(field, model, this.modelMap); + const connectionInfo = processConnections(field, model, this.modelMap, !!this.config.isDataStoreEnabled); if (connectionInfo) { if (connectionInfo.kind === CodeGenConnectionType.HAS_MANY || connectionInfo.kind === CodeGenConnectionType.HAS_ONE) { // Need to update the other side of the connection even if there is no connection directive