Skip to content

Commit

Permalink
fix: allow required uni-directional hasOne with fields
Browse files Browse the repository at this point in the history
  • Loading branch information
dpilch committed Oct 2, 2024
1 parent 8801359 commit 2e74506
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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]);
Expand All @@ -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);
Expand All @@ -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)', () => {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -462,4 +527,4 @@ describe('process connection', () => {
expect(getConnectedField(subordinateField, employeeModel, employeeModel)).toEqual(supervisorField);
});
});
});
});
30 changes: 27 additions & 3 deletions packages/appsync-modelgen-plugin/src/utils/process-connections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2e74506

Please sign in to comment.