From 2e7450679e0f362b73ed5f02530c524c39d4f95e 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 | 30 ++++++- .../src/visitors/appsync-visitor.ts | 2 +- 3 files changed, 103 insertions(+), 14 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..6b2a84bd6 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('One:One connection has required field on both sides 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..bf290b48d 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) { @@ -156,7 +157,7 @@ export function processConnections( }; } else if (!field.isList && otherSideField.isList) { // One to Many - if (connectionFields.length > 1) { + if (isDataStoreEnabled && connectionFields.length > 1) { // Todo: Move to a common function and update the error message throw new Error('DataStore only support one key in field'); } @@ -209,15 +210,38 @@ 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 && !isDataStoreEnabled) { + /* + // other side + type User @model { + id: ID! + } + + type Session @model { + id: ID! + sessionUserId: ID! + // field + user: User! @connection(fields: ["sessionUserId"]) + } + */ + return { + kind: CodeGenConnectionType.HAS_ONE, + associatedWith: otherSideField, + associatedWithFields: [], // New attribute for v2 custom pk support. Not used in v1 so use empty array. + connectedModel: otherSide, + isConnectingFieldAutoCreated, + 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 { /* # model type License { # belongsTo - person: Person! + person: Person } # otherSide type Person { # hasOne - license: License; + license: License } */ throw new Error( diff --git a/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts b/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts index cb2cea965..926bc6d43 100644 --- a/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts +++ b/packages/appsync-modelgen-plugin/src/visitors/appsync-visitor.ts @@ -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