Skip to content

Commit

Permalink
fix(graphql-relational-transformer): nullability enforcement for refe…
Browse files Browse the repository at this point in the history 
…rences 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 <schmelte@amazon.com>
  • Loading branch information
@atierian @palpatim
atierian and palpatim committed Apr 29, 2024
1 parent 53665c0 commit d540097
Show file tree
Hide file tree
Showing 15 changed files with 397 additions and 21 deletions.
109 changes: 108 additions & 1 deletion packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,7 @@ describe('ModelTransformer:', () => {
tags: [String!]
attachments: Attachment
}
type Attachment {
report: String!
image: String!
Expand Down Expand Up @@ -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');
});
});
});
4 changes: 2 additions & 2 deletions ...ansformer/src/__tests__/__snapshots__/amplify-graphql-belongs-to-transformer.test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -255,7 +255,7 @@ input ModelProfileConditionInput {
input CreateProfileInput {
profileId: String!
userFirstName: String
userLastName: String!
userLastName: String
}
input UpdateProfileInput {
Expand Down
4 changes: 2 additions & 2 deletions ...-transformer/src/__tests__/__snapshots__/amplify-graphql-has-one-transformer.test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -255,7 +255,7 @@ input ModelProfileConditionInput {
input CreateProfileInput {
profileId: String!
userFirstName: String
userLastName: String!
userLastName: String
}
input UpdateProfileInput {
Expand Down
4 changes: 2 additions & 2 deletions ...aphql-relational-transformer/src/__tests__/amplify-graphql-belongs-to-transformer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down Expand Up @@ -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"])
}
`;
Expand Down
116 changes: 114 additions & 2 deletions ...ational-transformer/src/__tests__/amplify-graphql-has-many-references-transformer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(() =>
Expand All @@ -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();

Expand All @@ -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")
}
`;
Expand Down
73 changes: 73 additions & 0 deletions ...lational-transformer/src/__tests__/amplify-graphql-has-one-references-transformer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit d540097

Please sign in to comment.