From d54009736092410b2d6e78ebf116a38298bf03ce Mon Sep 17 00:00:00 2001 From: Ian Saultz <52051793+atierian@users.noreply.github.com> Date: Mon, 29 Apr 2024 18:53:51 -0400 Subject: [PATCH] fix(graphql-relational-transformer): nullability enforcement for references relational fields (#2510) * improve error messaging for inconsistent nullability of reference fields * update hasOne / belongsTo test schema for consistent nullability * add no global auth model transformer tests --------- Co-authored-by: Tim Schmelter --- .../src/__tests__/model-transformer.test.ts | 109 +++++++++++++++- ...raphql-belongs-to-transformer.test.ts.snap | 4 +- ...y-graphql-has-one-transformer.test.ts.snap | 4 +- ...ify-graphql-belongs-to-transformer.test.ts | 4 +- ...ql-has-many-references-transformer.test.ts | 116 +++++++++++++++++- ...hql-has-one-references-transformer.test.ts | 73 +++++++++++ ...mplify-graphql-has-one-transformer.test.ts | 2 +- ...to-directive-ddb-references-transformer.ts | 2 + .../belongs-to-directive-sql-transformer.ts | 2 + ...ny-directive-ddb-references-transformer.ts | 3 +- .../has-many-directive-sql-transformer.ts | 9 +- ...ne-directive-ddb-references-transformer.ts | 2 + .../has-one-directive-sql-transformer.ts | 9 +- .../src/utils.ts | 77 ++++++++++-- .../src/index.ts | 2 +- 15 files changed, 397 insertions(+), 21 deletions(-) diff --git a/packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts b/packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts index 145d3b0a92..a84fc4d68c 100644 --- a/packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts +++ b/packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts @@ -1694,7 +1694,7 @@ describe('ModelTransformer:', () => { tags: [String!] attachments: Attachment } - + type Attachment { report: String! image: String! @@ -2209,4 +2209,111 @@ describe('ModelTransformer:', () => { }); }); }); + + describe('No global auth', () => { + it('sandbox mode + IAM access enabled', () => { + const schema = ` + type Post @model { + id: ID! @primaryKey + title: String! + } + `; + + const out = testTransform({ + schema, + transformers: [new ModelTransformer(), new PrimaryKeyTransformer()], + transformParameters: { + sandboxModeEnabled: true, + }, + synthParameters: { + enableIamAccess: true, + }, + }); + + const parsed = parse(out.schema); + validateModelSchema(parsed); + + const postType = getObjectType(parsed, 'Post')!; + expect(postType).toBeDefined(); + expect(postType.directives).toBeDefined(); + expect(postType.directives!.length).toEqual(2); + const directiveNames = postType.directives!.map((dir) => dir.name.value); + expect(directiveNames).toContain('aws_api_key'); + expect(directiveNames).toContain('aws_iam'); + }); + + it('sandbox mode + default authentication not API_KEY', () => { + const schema = ` + type Post @model { + id: ID! @primaryKey + title: String! + } + `; + + const out = testTransform({ + schema, + transformers: [new ModelTransformer(), new PrimaryKeyTransformer()], + transformParameters: { + sandboxModeEnabled: true, + }, + authConfig: { + defaultAuthentication: { + authenticationType: 'AMAZON_COGNITO_USER_POOLS', + }, + additionalAuthenticationProviders: [ + { + authenticationType: 'API_KEY', + }, + ], + }, + }); + + const parsed = parse(out.schema); + validateModelSchema(parsed); + + const postType = getObjectType(parsed, 'Post')!; + expect(postType).toBeDefined(); + expect(postType.directives).toBeDefined(); + expect(postType.directives!.length).toEqual(1); + const directiveNames = postType.directives!.map((dir) => dir.name.value); + expect(directiveNames).toContain('aws_api_key'); + }); + + it('IAM access enabled + default authentication not AWS_IAM', () => { + const schema = ` + type Post @model { + id: ID! @primaryKey + title: String! + } + `; + + const out = testTransform({ + schema, + transformers: [new ModelTransformer(), new PrimaryKeyTransformer()], + synthParameters: { + enableIamAccess: true, + }, + authConfig: { + defaultAuthentication: { + authenticationType: 'AMAZON_COGNITO_USER_POOLS', + }, + additionalAuthenticationProviders: [ + { + authenticationType: 'AWS_IAM', + }, + ], + }, + }); + + const parsed = parse(out.schema); + validateModelSchema(parsed); + + const postType = getObjectType(parsed, 'Post')!; + expect(postType).toBeDefined(); + expect(postType.directives).toBeDefined(); + expect(postType.directives!.length).toEqual(1); + const directiveNames = postType.directives!.map((dir) => dir.name.value); + expect(directiveNames).toContain('aws_iam'); + }); + }); }); diff --git a/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-belongs-to-transformer.test.ts.snap b/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-belongs-to-transformer.test.ts.snap index 8b66442452..bb5d249873 100644 --- a/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-belongs-to-transformer.test.ts.snap +++ b/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-belongs-to-transformer.test.ts.snap @@ -10,7 +10,7 @@ exports[`@belongsTo directive with RDS datasource composite key should generate type Profile { profileId: String! userFirstName: String - userLastName: String! + userLastName: String user: User } @@ -255,7 +255,7 @@ input ModelProfileConditionInput { input CreateProfileInput { profileId: String! userFirstName: String - userLastName: String! + userLastName: String } input UpdateProfileInput { diff --git a/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-has-one-transformer.test.ts.snap b/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-has-one-transformer.test.ts.snap index 1311dc7ae4..fed05a8ff8 100644 --- a/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-has-one-transformer.test.ts.snap +++ b/packages/amplify-graphql-relational-transformer/src/__tests__/__snapshots__/amplify-graphql-has-one-transformer.test.ts.snap @@ -10,7 +10,7 @@ exports[`@hasOne directive with RDS datasource composite key should generate cor type Profile { profileId: String! userFirstName: String - userLastName: String! + userLastName: String user: User } @@ -255,7 +255,7 @@ input ModelProfileConditionInput { input CreateProfileInput { profileId: String! userFirstName: String - userLastName: String! + userLastName: String } input UpdateProfileInput { diff --git a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-belongs-to-transformer.test.ts b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-belongs-to-transformer.test.ts index 7efdc9d706..8801b0e855 100644 --- a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-belongs-to-transformer.test.ts +++ b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-belongs-to-transformer.test.ts @@ -458,7 +458,7 @@ test('Should not resolve to secondary index of connected model if the index is d content: String! @index comments: [Comment] @hasMany(indexName:"byParent", fields:["customId", "content"]) } - + type Comment @model { childId: ID! @primaryKey(sortKeyFields:["content"]) content: String! @@ -783,7 +783,7 @@ describe('@belongsTo directive with RDS datasource', () => { type Profile @model { profileId: String! @primaryKey userFirstName: String - userLastName: String! + userLastName: String user: User @belongsTo(references: ["userFirstName", "userLastName"]) } `; diff --git a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-many-references-transformer.test.ts b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-many-references-transformer.test.ts index 212e359be3..ce9781087a 100644 --- a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-many-references-transformer.test.ts +++ b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-many-references-transformer.test.ts @@ -185,7 +185,7 @@ test('fails if used as a has one relationship', () => { type Member @model { id: ID! teamID: String - team: Team @belongsTo(fields: ["teamID"]) + team: Team @belongsTo(references: ["teamID"]) }`; expect(() => @@ -196,6 +196,118 @@ test('fails if used as a has one relationship', () => { ).toThrowError('@hasMany must be used with a list. Use @hasOne for non-list types.'); }); +test('fails if primary relational field list type is required', () => { + const inputSchema = ` + type Team @model { + id: ID! + name: String! + members: [Member]! @hasMany(references: ["teamID"]) + } + type Member @model { + id: ID! + teamID: String + team: Team @belongsTo(references: ["teamID"]) + }`; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new HasManyTransformer(), new BelongsToTransformer()], + }), + ).toThrowError("@hasMany fields must not be required. Change 'Team.members: [Member]!' to 'Team.members: [Member]'"); +}); + +test('fails if primary relational field element type required', () => { + const inputSchema = ` + type Team @model { + id: ID! + name: String! + members: [Member!] @hasMany(references: ["teamID"]) + } + type Member @model { + id: ID! + teamID: String + team: Team @belongsTo(references: ["teamID"]) + }`; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new HasManyTransformer(), new BelongsToTransformer()], + }), + ).toThrowError("@hasMany fields must not be required. Change 'Team.members: [Member!]' to 'Team.members: [Member]'"); +}); + +test('fails if primary relational field list type and element type are required', () => { + const inputSchema = ` + type Team @model { + id: ID! + name: String! + members: [Member!]! @hasMany(references: ["teamID"]) + } + type Member @model { + id: ID! + teamID: String + team: Team @belongsTo(references: ["teamID"]) + }`; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new HasManyTransformer(), new BelongsToTransformer()], + }), + ).toThrowError("@hasMany fields must not be required. Change 'Team.members: [Member]!' to 'Team.members: [Member]'"); +}); + +test('fails if related relational field is required', () => { + const inputSchema = ` + type Team @model { + id: ID! + name: String! + members: [Member] @hasMany(references: ["teamID"]) + } + type Member @model { + id: ID! + teamID: String + team: Team! @belongsTo(references: ["teamID"]) + }`; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new HasManyTransformer(), new BelongsToTransformer()], + }), + ).toThrowError("@belongsTo fields must not be required. Change 'Member.team: Team!' to 'Member.team: Team'"); +}); + +test('fails with inconsistent nullability of reference fields', () => { + const inputSchema = ` + type Member @model { + name: String + teamId: String! + teamMantra: String + team: Team @belongsTo(references: ["teamId", "teamMantra"]) + } + type Team @model { + id: String! @primaryKey(sortKeyFields: ["mantra"]) + mantra: String! + members: [Member] @hasMany(references: ["teamId", "teamMantra"]) + } + `; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new PrimaryKeyTransformer(), new HasManyTransformer(), new BelongsToTransformer()], + }), + ).toThrowError( + "Reference fields defined on related type: 'Member' for @hasMany(references: ['teamId', 'teamMantra']) Team.members relationship have inconsistent nullability." + + "\nRequired fields: 'teamId'" + + "\nNullable fields: 'teamMantra'" + + "\nUpdate reference fields on type 'Member' to have consistent nullability -- either all required or all nullable.", + ); +}); + test('hasMany / hasOne - belongsTo across data source type boundary', () => { const mockSqlStrategy = mockSqlDataSourceStrategy(); @@ -215,7 +327,7 @@ test('hasMany / hasOne - belongsTo across data source type boundary', () => { type Team @model { id: String! @primaryKey mantra: String - members: [Member!] @hasMany(references: "teamId") + members: [Member] @hasMany(references: "teamId") project: Project @hasOne(references: "teamId") } `; diff --git a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-references-transformer.test.ts b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-references-transformer.test.ts index 8bdc39362b..e9ef0bf059 100644 --- a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-references-transformer.test.ts +++ b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-references-transformer.test.ts @@ -277,6 +277,79 @@ test('fails if object type fields are provided', () => { ).toThrowError('All reference fields provided to @hasOne must be scalar or enum fields.'); }); +test('fails if primary relational field is required', () => { + const inputSchema = ` + type Project @model { + name: String + teamId: String + team: Team @belongsTo(references: "teamId") + } + + type Team @model { + id: String! + mantra: String + project: Project! @hasOne(references: "teamId") + }`; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new HasOneTransformer(), new BelongsToTransformer()], + }), + ).toThrowError("@hasOne fields must not be required. Change 'Team.project: Project!' to 'Team.project: Project'"); +}); + +test('fails if related relational field is required', () => { + const inputSchema = ` + type Project @model { + name: String + teamId: String + team: Team! @belongsTo(references: "teamId") + } + + type Team @model { + id: String! + mantra: String + project: Project @hasOne(references: "teamId") + }`; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new HasOneTransformer(), new BelongsToTransformer()], + }), + ).toThrowError("@belongsTo fields must not be required. Change 'Project.team: Team!' to 'Project.team: Team'"); +}); + +test('fails with inconsistent nullability of reference fields', () => { + const inputSchema = ` + type Project @model { + name: String + teamId: String! + teamMantra: String + team: Team @belongsTo(references: ["teamId", "teamMantra"]) + } + + type Team @model { + id: String! @primaryKey(sortKeyFields: ["mantra"]) + mantra: String! + project: Project @hasOne(references: ["teamId", "teamMantra"]) + } + `; + + expect(() => + testTransform({ + schema: inputSchema, + transformers: [new ModelTransformer(), new PrimaryKeyTransformer(), new HasOneTransformer(), new BelongsToTransformer()], + }), + ).toThrowError( + "Reference fields defined on related type: 'Project' for @hasOne(references: ['teamId', 'teamMantra']) Team.project relationship have inconsistent nullability." + + "\nRequired fields: 'teamId'" + + "\nNullable fields: 'teamMantra'" + + "\nUpdate reference fields on type 'Project' to have consistent nullability -- either all required or all nullable.", + ); +}); + test('has one references single partition key', () => { const inputSchema = ` type Project @model { diff --git a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-transformer.test.ts b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-transformer.test.ts index 86280af315..8fe2aae0b8 100644 --- a/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-transformer.test.ts +++ b/packages/amplify-graphql-relational-transformer/src/__tests__/amplify-graphql-has-one-transformer.test.ts @@ -797,7 +797,7 @@ describe('@hasOne directive with RDS datasource', () => { type Profile @model { profileId: String! @primaryKey userFirstName: String - userLastName: String! + userLastName: String user: User @belongsTo(references: ["userFirstName", "userLastName"]) } `; diff --git a/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-ddb-references-transformer.ts b/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-ddb-references-transformer.ts index b7b8593c87..6226ac59c1 100644 --- a/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-ddb-references-transformer.ts +++ b/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-ddb-references-transformer.ts @@ -14,6 +14,7 @@ import { registerHasOneForeignKeyMappings, validateChildReferencesFields, validateReferencesBidirectionality, + validateReferencesRelationalFieldNullability, } from '../utils'; /** @@ -54,6 +55,7 @@ export class BelongsToDirectiveDDBReferencesTransformer implements DataSourceBas validate = (context: TransformerContextProvider, config: BelongsToDirectiveConfiguration): void => { ensureReferencesArray(config); validateChildReferencesFields(config, context as TransformerContextProvider); + validateReferencesRelationalFieldNullability(config); config.referenceNodes = getBelongsToReferencesNodes(config, context); validateReferencesBidirectionality(config); }; diff --git a/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-sql-transformer.ts b/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-sql-transformer.ts index 858c7b5596..31dd3725a8 100644 --- a/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-sql-transformer.ts +++ b/packages/amplify-graphql-relational-transformer/src/belongs-to/belongs-to-directive-sql-transformer.ts @@ -13,6 +13,7 @@ import { getBelongsToReferencesNodes, validateChildReferencesFields, validateReferencesBidirectionality, + validateReferencesRelationalFieldNullability, } from '../utils'; /** @@ -39,6 +40,7 @@ export class BelongsToDirectiveSQLTransformer implements DataSourceBasedDirectiv validate = (context: TransformerContextProvider, config: BelongsToDirectiveConfiguration): void => { ensureReferencesArray(config); config.referenceNodes = getBelongsToReferencesNodes(config, context); + validateReferencesRelationalFieldNullability(config); validateReferencesBidirectionality(config); }; } diff --git a/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-ddb-references-transformer.ts b/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-ddb-references-transformer.ts index c8c2abd88e..5c7ff21f84 100644 --- a/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-ddb-references-transformer.ts +++ b/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-ddb-references-transformer.ts @@ -18,6 +18,7 @@ import { registerHasManyForeignKeyMappings, validateParentReferencesFields, validateReferencesBidirectionality, + validateReferencesRelationalFieldNullability, } from '../utils'; /** @@ -66,11 +67,11 @@ export class HasManyDirectiveDDBReferencesTransformer implements DataSourceBased } ensureReferencesArray(config); validateParentReferencesFields(config, context); + validateReferencesRelationalFieldNullability(config); const objectName = config.object.name.value; const fieldName = config.field.name.value; config.indexName = `gsi-${objectName}.${fieldName}`; config.referenceNodes = getReferencesNodes(config, context); - validateReferencesBidirectionality(config); }; } diff --git a/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-sql-transformer.ts b/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-sql-transformer.ts index 1bb6500b7c..4f02750aa5 100644 --- a/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-sql-transformer.ts +++ b/packages/amplify-graphql-relational-transformer/src/has-many/has-many-directive-sql-transformer.ts @@ -8,7 +8,13 @@ import { DataSourceBasedDirectiveTransformer } from '../data-source-based-direct import { getGenerator } from '../resolver/generator-factory'; import { setFieldMappingResolverReference } from '../resolvers'; import { HasManyDirectiveConfiguration } from '../types'; -import { ensureReferencesArray, getReferencesNodes, validateParentReferencesFields, validateReferencesBidirectionality } from '../utils'; +import { + ensureReferencesArray, + getReferencesNodes, + validateParentReferencesFields, + validateReferencesBidirectionality, + validateReferencesRelationalFieldNullability, +} from '../utils'; /** * HasManyDirectiveSQLTransformer executes transformations based on `@hasMany(references: [String!])` configurations @@ -34,6 +40,7 @@ export class HasManyDirectiveSQLTransformer implements DataSourceBasedDirectiveT validate = (context: TransformerContextProvider, config: HasManyDirectiveConfiguration): void => { ensureReferencesArray(config); config.referenceNodes = getReferencesNodes(config, context); + validateReferencesRelationalFieldNullability(config); validateReferencesBidirectionality(config); }; } diff --git a/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-ddb-references-transformer.ts b/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-ddb-references-transformer.ts index 283590b8d6..c06192e2fe 100644 --- a/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-ddb-references-transformer.ts +++ b/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-ddb-references-transformer.ts @@ -18,6 +18,7 @@ import { registerHasOneForeignKeyMappings, validateParentReferencesFields, validateReferencesBidirectionality, + validateReferencesRelationalFieldNullability, } from '../utils'; /** @@ -66,6 +67,7 @@ export class HasOneDirectiveDDBReferencesTransformer implements DataSourceBasedD } ensureReferencesArray(config); validateParentReferencesFields(config, context); + validateReferencesRelationalFieldNullability(config); const objectName = config.object.name.value; const fieldName = config.field.name.value; config.indexName = `gsi-${objectName}.${fieldName}`; diff --git a/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-sql-transformer.ts b/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-sql-transformer.ts index 870939d77b..79fc0258be 100644 --- a/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-sql-transformer.ts +++ b/packages/amplify-graphql-relational-transformer/src/has-one/has-one-directive-sql-transformer.ts @@ -7,7 +7,13 @@ import { import { DataSourceBasedDirectiveTransformer } from '../data-source-based-directive-transformer'; import { setFieldMappingResolverReference } from '../resolvers'; import { HasOneDirectiveConfiguration } from '../types'; -import { ensureReferencesArray, getReferencesNodes, validateParentReferencesFields, validateReferencesBidirectionality } from '../utils'; +import { + ensureReferencesArray, + getReferencesNodes, + validateParentReferencesFields, + validateReferencesBidirectionality, + validateReferencesRelationalFieldNullability, +} from '../utils'; import { getGenerator } from '../resolver/generator-factory'; /** @@ -34,6 +40,7 @@ export class HasOneDirectiveSQLTransformer implements DataSourceBasedDirectiveTr validate = (context: TransformerContextProvider, config: HasOneDirectiveConfiguration): void => { ensureReferencesArray(config); config.referenceNodes = getReferencesNodes(config, context); + validateReferencesRelationalFieldNullability(config); validateReferencesBidirectionality(config); }; } diff --git a/packages/amplify-graphql-relational-transformer/src/utils.ts b/packages/amplify-graphql-relational-transformer/src/utils.ts index 1e27af569e..fc790d7249 100644 --- a/packages/amplify-graphql-relational-transformer/src/utils.ts +++ b/packages/amplify-graphql-relational-transformer/src/utils.ts @@ -10,7 +10,6 @@ import { DirectiveNode, EnumTypeDefinitionNode, FieldDefinitionNode, - isNonNullType, Kind, ListValueNode, NamedTypeNode, @@ -304,7 +303,53 @@ const referenceFieldTypeMatchesPrimaryKey = (a: FieldDefinitionNode, b: FieldDef }; /** - * Validates the a {@link ReferencesRelationalDirectiveConfiguration} conforms to the following rules: + * Validates that a given {@link ReferencesRelationalDirectiveConfiguration} is decorated on a non-required (nullable) + * field. + * + * Valid: + * `members: [Member] @hasMany(references: 'teamId') + * `team: Team @belongsTo(references: 'teamId)` + * `project: Project @hasOne(references: 'teamId)` + * + * Invalid: + * `members: [Member]! @hasMany(references: 'teamId') + * `members: [Member!] @hasMany(references: 'teamId') + * `members: [Member!]! @hasMany(references: 'teamId') + * `team: Team! @belongsTo(references: 'teamId)` + * `project: Project! @hasOne(references: 'teamId)` + * @param config {@link ReferencesRelationalDirectiveConfiguration} + */ +export const validateReferencesRelationalFieldNullability = (config: ReferencesRelationalDirectiveConfiguration): void => { + const { field, object, relatedType } = config; + const fieldType = field.type; + const relatedTypeName = relatedType.name.value; + + if (fieldType.kind === Kind.NON_NULL_TYPE) { + const fieldDescription = + `${object.name.value}.${field.name.value}: ` + + `${config.directiveName === HasManyDirective.name ? `[${relatedTypeName}]` : relatedTypeName}`; + throw new InvalidDirectiveError( + `@${config.directiveName} fields must not be required. Change '${fieldDescription}!' to '${fieldDescription}'`, + ); + } + + if (fieldType.kind === Kind.LIST_TYPE && fieldType.type.kind === Kind.NON_NULL_TYPE) { + const fieldDescription = (typeDescription: string): string => { + return `${config.object.name.value}.${config.field.name.value}: ${typeDescription}`; + }; + const relatedTypeIs = config.directiveName === HasManyDirective.name ? `[${relatedTypeName}!]` : `${relatedTypeName}!`; + const relatedTypeShould = config.directiveName === HasManyDirective.name ? `[${relatedTypeName}]` : `${relatedTypeName}`; + + throw new InvalidDirectiveError( + `@${config.directiveName} fields must not be required. Change '${fieldDescription(relatedTypeIs)}' to '${fieldDescription( + relatedTypeShould, + )}'`, + ); + } +}; + +/** + * Validates that a given {@link ReferencesRelationalDirectiveConfiguration} conforms to the following rules: * - relationship is bidirectional * - hasOne and hasMany have a belongsTo counterpart * - belongsTo has a hasOne or hasMany counterpart @@ -445,11 +490,29 @@ export const getReferencesNodes = ( return fieldNode; }); - // Ensure that the reference fields have consistent nullability - const firstReferenceNodeIsNonNull = isNonNullType(referenceNodes[0]); - referenceNodes.slice(1).forEach((node) => { - if (isNonNullType(node) !== firstReferenceNodeIsNonNull) { - throw new InvalidDirectiveError('reference fields must have consistent nullability'); + // Validate that the reference fields have consistent nullability + const firstReferenceNodeIsNonNull = referenceNodes[0].type.kind === Kind.NON_NULL_TYPE; + referenceNodes.slice(1).forEach((referenceNode) => { + const isNonNull = referenceNode.type.kind === Kind.NON_NULL_TYPE; + if (isNonNull !== firstReferenceNodeIsNonNull) { + const nonNullReferenceFields = referenceNodes + .filter((node) => node.type.kind === Kind.NON_NULL_TYPE) + .map((node) => `'${node.name.value}'`) + .join(', '); + + const nullableReferenceFields = referenceNodes + .filter((node) => node.type.kind !== Kind.NON_NULL_TYPE) + .map((node) => `'${node.name.value}'`) + .join(', '); + + const referencesDescription = '[' + references.map((reference) => `'${reference}'`).join(', ') + ']'; + const fieldDescription = `@${directiveName}(references: ${referencesDescription}) ${config.object.name.value}.${config.field.name.value}`; + throw new InvalidDirectiveError( + `Reference fields defined on related type: '${relatedType.name.value}' for ${fieldDescription} relationship have inconsistent nullability.` + + `\nRequired fields: ${nonNullReferenceFields}` + + `\nNullable fields: ${nullableReferenceFields}` + + `\nUpdate reference fields on type '${relatedType.name.value}' to have consistent nullability -- either all required or all nullable.`, + ); } }); diff --git a/packages/amplify-graphql-schema-test-library/src/index.ts b/packages/amplify-graphql-schema-test-library/src/index.ts index 27fac718db..05f733c808 100644 --- a/packages/amplify-graphql-schema-test-library/src/index.ts +++ b/packages/amplify-graphql-schema-test-library/src/index.ts @@ -502,7 +502,7 @@ export const schemas: { [key: string]: TransformerSchema } = { instanceId: ID! recordId: ID! content: String - related: [Related!] @hasMany(references: ["primaryTenantId", "primaryInstanceId", "primaryRecordId"]) + related: [Related] @hasMany(references: ["primaryTenantId", "primaryInstanceId", "primaryRecordId"]) } type Related @model {