aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
81 stars 71 forks source link

SQLSchema - Optional belongsTo relationship breaks query/mutations #2580

Closed kekami closed 1 month ago

kekami commented 1 month ago

Environment information

System:
  OS: macOS 14.4.1
  CPU: (10) arm64 Apple M1 Max
  Memory: 157.84 MB / 32.00 GB
  Shell: /bin/zsh
Binaries:
  Node: 20.12.2 - ~/.nvm/versions/node/v20.12.2/bin/node
  Yarn: 3.8.2 - ~/.nvm/versions/node/v20.12.2/bin/yarn
  npm: 10.5.0 - ~/.nvm/versions/node/v20.12.2/bin/npm
  pnpm: undefined - undefined
NPM Packages:
  @aws-amplify/backend: 1.0.2
  @aws-amplify/backend-cli: 1.0.3
  aws-amplify: 6.3.2
  aws-cdk: 2.142.1
  aws-cdk-lib: 2.142.1
  typescript: 5.4.5
AWS environment variables:
  AWS_SDK_LOAD_CONFIG = 1
  AWS_STS_REGIONAL_ENDPOINTS = regional
  AWS_NODEJS_CONNECTION_REUSE_ENABLED = 1
No CDK environment variables

Description

Schema:

    models.Contract.relationships({
      organisation: a.belongsTo('Organisation', 'organisation_id'),
    }),

    models.Organisation.relationships({
      contracts: a.hasMany('Contract', 'organisation_id'),
    }),

On running createContract mutation, the following error is received.

2024-05-28T20:08:54.908Z    a6df0c55-d6bc-4407-b261-eff561811370    INFO    error: select * from "organisations" where "id" = $1 - invalid input syntax for type uuid: ""
    at Parser.parseErrorMessage (/opt/nodejs/node_modules/pg-protocol/dist/parser.js:287:98)
    at Parser.handlePacket (/opt/nodejs/node_modules/pg-protocol/dist/parser.js:126:29)
    at Parser.parse (/opt/nodejs/node_modules/pg-protocol/dist/parser.js:39:38)
    at TLSSocket.<anonymous> (/opt/nodejs/node_modules/pg-protocol/dist/index.js:11:42)
    at TLSSocket.emit (node:events:517:28)
    at addChunk (node:internal/streams/readable:368:12)
    at readableAddChunk (node:internal/streams/readable:341:9)
    at Readable.push (node:internal/streams/readable:278:10)
    at TLSWrap.onStreamRead (node:internal/stream_base_commons:190:23) {
  length: 129,
  severity: 'ERROR',
  code: '22P02',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: "unnamed portal parameter $1 = ''",
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'uuid.c',
  line: '133',
  routine: 'string_to_uuid'
}

Presumably due to the following line in the ContractOrganisationDataResolverFn where it defaults to "" if the created Contract has no organisation_id (optional relationship)

$util.qr($lambdaInput.args.input.put("id", $util.defaultIfNull($ctx.source.organisation_id, "")))
{
  "version": "2018-05-29",
  "operation": "Invoke",
  "payload":   $util.toJson($lambdaInput)
}

Same error is also received when querying Contract with no organisation_id, e.g.

query MyQuery {
  listContracts {
    items {
      id
        organisation {
          id
        }
    }
  }
}

Ping me if you have any further questions.

ykethan commented 1 month ago

Hey👋 thanks for raising this! I'm going to transfer this over to our API repository for better assistance 🙂

palpatim commented 1 month ago

@kekami Can you share the SQL table definitions for Contract and Organisation, as well as the generated source schema.sql.ts? Connection fields (that is, the fields decorated with hasMany, hasOne, or belongsTo, must always be optional, in order to prevent potential schema validation errors on nested queries.

The reference fields (that is, the fields on the related model that comprise the primary key of the primary model), may be either optional or required.

We have tested relationships with optional required fields in SQL data sources (e.g., see https://github.com/aws-amplify/amplify-category-api/blob/main/packages/amplify-graphql-api-construct-tests/src/__tests__/relationships/references/references-sqlprimary-sqlrelated.test.ts) generated from the CDK construct. It's possible we didn't cover your particular configuration, and it's also possible that there's something lost in translation between the schema builder (the Gen2 typescript definition) and the GraphQL schema it generates, but we'll need more info to know for sure.

kekami commented 1 month ago

Hi @palpatim, I've created a smaller schema that this reproduces the problem.

Here is the table definition used:

CREATE TABLE organisations (
    id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
    owners TEXT[],
    created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
);

CREATE TABLE contracts (
    id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
    organisation_id UUID,
    owners TEXT[],
    created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
    FOREIGN KEY (organisation_id) REFERENCES organisations(id)
);

And the generated schema.sql.ts

/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
import { a } from "@aws-amplify/data-schema";
import { configure } from "@aws-amplify/data-schema/internals";
import { secret } from "@aws-amplify/backend";

export const schema = configure({
    database: {
        identifier: "IDLkPsFEcNeUVt6GqrbYTxYg",
        engine: "postgresql",
        connectionUri: secret("SQL_CONNECTION_STRING_REPRO")
    }
}).schema({
    "contracts": a.model({
        id: a.id().required(),
        organisation_id: a.id(),
        owners: a.string().array(),
        created_at: a.datetime(),
        updated_at: a.datetime()
    }).identifier([
        "id"
    ]),
    "organisations": a.model({
        id: a.id().required(),
        owners: a.string().array(),
        created_at: a.datetime(),
        updated_at: a.datetime()
    }).identifier([
        "id"
    ])
});

And resource.ts

import { type ClientSchema, a, defineData } from "@aws-amplify/backend";

import { schema as generatedSqlSchema } from "./schema.sql.js";

const sqlSchema = generatedSqlSchema
  .renameModels(() => [
    ["contracts", "Contract"],
    ["organisations", "Organisation"],
  ])
  .setRelationships((models) => [
    models.Contract.relationships({
      organisation: a.belongsTo("Organisation", "organisation_id"),
    }),

    models.Organisation.relationships({
      contracts: a.hasMany("Contract", "organisation_id"),
    }),
  ])
  .setAuthorization((models) => [
    models.Contract.authorization((allow) => [
      allow.ownersDefinedIn("owners"),
      allow.groups(["admin"]),
    ]),
    models.Organisation.authorization((allow) => [
      allow.ownersDefinedIn("owners"),
      allow.groups(["admin"]),
    ]),
  ]);

export type Schema = ClientSchema<typeof sqlSchema>;

export const data = defineData({
  schema: sqlSchema,
  authorizationModes: {
    defaultAuthorizationMode: "userPool",
    apiKeyAuthorizationMode: {
      expiresInDays: 30,
    },
  },
});
palpatim commented 1 month ago

Hi @kekami, thanks for the repro case. I've confirmed the issue on my side and am looking at the fix for this.

palpatim commented 1 month ago

Fixed in https://github.com/aws-amplify/amplify-category-api/pull/2587, pending next release of this package.

palpatim commented 1 month ago

This has been released on constructs:

You should be able to do an npm update in your project to update to the latest dependencies and consume this fix.

github-actions[bot] commented 1 month ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.