Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(graphql-relational-transformer): nullability enforcement for references relational fields #2510

Merged
merged 7 commits into from
Apr 29, 2024
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
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
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
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"])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated fix in test schema

}`;

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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
}
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
registerHasOneForeignKeyMappings,
validateChildReferencesFields,
validateReferencesBidirectionality,
validateReferencesRelationalFieldNullability,
} from '../utils';

/**
Expand Down Expand Up @@ -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);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
getBelongsToReferencesNodes,
validateChildReferencesFields,
validateReferencesBidirectionality,
validateReferencesRelationalFieldNullability,
} from '../utils';

/**
Expand All @@ -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);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
registerHasManyForeignKeyMappings,
validateParentReferencesFields,
validateReferencesBidirectionality,
validateReferencesRelationalFieldNullability,
} from '../utils';

/**
Expand Down Expand Up @@ -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);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
registerHasOneForeignKeyMappings,
validateParentReferencesFields,
validateReferencesBidirectionality,
validateReferencesRelationalFieldNullability,
} from '../utils';

/**
Expand Down Expand Up @@ -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}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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);
};
}
Loading
Loading