fix: require @PrimaryKey sort fields to be non-nullable (#8562)

Co-authored-by: Colin Ihrig <colihrig@amazon.com>
aws-amplify · Oct 27, 2021 · bf4fb0e · bf4fb0e
1 parent 4bfe636
commit bf4fb0e
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 5 deletions.
diff --git a/...y-graphql-index-transformer/src/__tests__/amplify-graphql-primary-key-transformer.test.ts b/...y-graphql-index-transformer/src/__tests__/amplify-graphql-primary-key-transformer.test.ts
@@ -20,7 +20,7 @@ test('throws if multiple primary keys are defined on an object', () => {
   }).toThrow(`You may only supply one primary key on type 'Test'.`);
 });
 
-test('throws if primary key is nullable', () => {
+test('throws if partition key is nullable', () => {
   const schema = `
     type Test @model {
       id: ID @primaryKey
@@ -36,6 +36,22 @@ test('throws if primary key is nullable', () => {
   }).toThrow(`The primary key on type 'Test' must reference non-null fields.`);
 });
 
+test('throws if sort key is nullable', () => {
+  const schema = `
+    type Test @model {
+      id: ID @primaryKey(sortKeyFields: ["email"])
+      email: String
+    }`;
+
+  const transformer = new GraphQLTransform({
+    transformers: [new ModelTransformer(), new PrimaryKeyTransformer()],
+  });
+
+  expect(() => {
+    transformer.transform(schema);
+  }).toThrow(`The primary key on type 'Test' must reference non-null fields.`);
+});
+
 test('throws if @primaryKey is used in a non-@model type', () => {
   const schema = `
     type Test {
@@ -213,7 +229,7 @@ test('a primary key with a composite sort key is properly configured', () => {
     type Test @model {
       email: String! @primaryKey(sortKeyFields: ["kind", "other"])
       kind: Int!
-      other: AWSDateTime
+      other: AWSDateTime!
       yetAnother: String
       andAnother: String!
     }`;
@@ -265,7 +281,7 @@ test('a primary key with a composite sort key is properly configured', () => {
   expect(createInput).toBeDefined();
   expect(createInput.fields.find((f: any) => f.name.value === 'email' && f.type.kind === Kind.NON_NULL_TYPE)).toBeDefined();
   expect(createInput.fields.find((f: any) => f.name.value === 'kind' && f.type.kind === Kind.NON_NULL_TYPE)).toBeDefined();
-  expect(createInput.fields.find((f: any) => f.name.value === 'other' && f.type.kind === Kind.NAMED_TYPE)).toBeDefined();
+  expect(createInput.fields.find((f: any) => f.name.value === 'other' && f.type.kind === Kind.NON_NULL_TYPE)).toBeDefined();
   expect(createInput.fields.find((f: any) => f.name.value === 'yetAnother' && f.type.kind === Kind.NAMED_TYPE)).toBeDefined();
   expect(createInput.fields.find((f: any) => f.name.value === 'andAnother' && f.type.kind === Kind.NON_NULL_TYPE)).toBeDefined();
   expect(createInput.fields.find((f: any) => f.name.value === 'id')).toBeUndefined();
@@ -427,7 +443,7 @@ test('resolvers can be renamed by @model', () => {
       mutations: { create: "testCreate", delete: "testDelete", update: "testUpdate" }
     ) {
       id: ID! @primaryKey(sortKeyFields: ["email"])
-      email: String
+      email: String!
     }`;
 
   const transformer = new GraphQLTransform({
@@ -471,7 +487,7 @@ test('individual resolvers can be made null by @model', () => {
   const inputSchema = `
     type Test @model(queries: { get: "testGet", list: null }) {
       id: ID! @primaryKey(sortKeyFields: ["email"])
-      email: String
+      email: String!
     }`;
 
   const transformer = new GraphQLTransform({

diff --git a/packages/amplify-graphql-index-transformer/src/graphql-primary-key-transformer.ts b/packages/amplify-graphql-index-transformer/src/graphql-primary-key-transformer.ts
@@ -148,6 +148,10 @@ function validate(config: PrimaryKeyDirectiveConfiguration, ctx: TransformerCont
       );
     }
 
+    if (!isNonNullType(sortField.type)) {
+      throw new InvalidDirectiveError(`The primary key on type '${object.name.value}' must reference non-null fields.`);
+    }
+
     config.sortKey.push(sortField);
   }
 }