aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.41k stars 2.12k forks source link

DataStore ignoring foreign key Id in @hasMany - @belongsTo relationships so authorizations and subscriptions don't work #11124

Open ritxweb opened 1 year ago

ritxweb commented 1 year ago

Before opening, please confirm:

JavaScript Framework

Angular

Amplify APIs

Authentication, GraphQL API

Amplify Categories

auth, api

Environment information

``` # Put output below this line System: OS: Windows 10 10.0.19044 CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz Memory: 6.18 GB / 15.92 GB Binaries: Node: 18.13.0 - C:\Program Files\nodejs\node.EXE npm: 8.18.0 - C:\Program Files\nodejs\npm.CMD Browsers: Chrome: 111.0.5563.65 Edge: Spartan (44.19041.1266.0), Chromium (111.0.1661.44) Internet Explorer: 11.0.19041.1566 npmPackages: @angular-devkit/build-angular: ^14.2.6 => 14.2.10 @angular/animations: ^14.2.0 => 14.2.12 @angular/cdk: ^14.2.5 => 14.2.7 @angular/cli: ~14.2.6 => 14.2.10 @angular/common: ^14.2.0 => 14.2.12 @angular/compiler: ^14.2.0 => 14.2.12 @angular/compiler-cli: ^14.2.0 => 14.2.12 @angular/core: ^14.2.0 => 14.2.12 @angular/forms: ^14.2.0 => 14.2.12 @angular/localize: ^14.2.6 => 14.2.12 @angular/material: ^14.2.5 => 14.2.7 @angular/platform-browser: ^14.2.0 => 14.2.12 @angular/platform-browser-dynamic: ^14.2.0 => 14.2.12 @angular/platform-server: ^14.2.0 => 14.2.12 @angular/router: ^14.2.0 => 14.2.12 @nguniversal/builders: ^14.2.0 => 14.2.3 @nguniversal/express-engine: ^14.2.0 => 14.2.3 @types/express: ^4.17.0 => 4.17.17 @types/jasmine: ~4.0.0 => 4.0.3 @types/node: ^14.15.0 => 14.18.36 aws-amplify: ^5.0.21 => 5.0.21 express: ^4.15.2 => 4.18.2 jasmine-core: ~4.3.0 => 4.3.0 karma: ~6.4.0 => 6.4.1 karma-chrome-launcher: ~3.1.0 => 3.1.1 karma-coverage: ~2.2.0 => 2.2.0 karma-coverage-coffee-example: 1.0.0 karma-jasmine: ~5.1.0 => 5.1.0 karma-jasmine-html-reporter: ~2.0.0 => 2.0.0 rxjs: ~7.5.0 => 7.5.7 (6.6.7) rxjs/ajax: undefined () rxjs/fetch: undefined () rxjs/internal-compatibility: undefined () rxjs/operators: undefined () rxjs/testing: undefined () rxjs/webSocket: undefined () tslib: ^2.3.0 => 2.5.0 (1.14.1, 2.4.0) typescript: ~4.7.2 => 4.7.4 ulid: ^2.3.0 => 2.3.0 zone-mix: undefined () zone-node: undefined () zone-testing: undefined () zone.js: ~0.11.4 => 0.11.8 zone.js/async-stack-tagging: undefined () zone.js/async-stack-tagging.min: undefined () zone.js/async-test: undefined () zone.js/async-test.min: undefined () zone.js/fake-async-test: undefined () zone.js/fake-async-test.min: undefined () zone.js/jasmine-patch: undefined () zone.js/jasmine-patch.min: undefined () zone.js/long-stack-trace-zone: undefined () zone.js/long-stack-trace-zone.min: undefined () zone.js/mocha-patch: undefined () zone.js/mocha-patch.min: undefined () zone.js/proxy: undefined () zone.js/proxy.min: undefined () zone.js/sync-test: undefined () zone.js/sync-test.min: undefined () zone.js/task-tracking: undefined () zone.js/task-tracking.min: undefined () zone.js/webapis-media-query: undefined () zone.js/webapis-media-query.min: undefined () zone.js/webapis-notification: undefined () zone.js/webapis-notification.min: undefined () zone.js/webapis-rtc-peer-connection: undefined () zone.js/webapis-rtc-peer-connection.min: undefined () zone.js/webapis-shadydom: undefined () zone.js/webapis-shadydom.min: undefined () zone.js/wtf: undefined () zone.js/wtf.min: undefined () zone.js/zone-bluebird: undefined () zone.js/zone-bluebird.min: undefined () zone.js/zone-error: undefined () zone.js/zone-error.min: undefined () zone.js/zone-legacy: undefined () zone.js/zone-legacy.min: undefined () zone.js/zone-patch-canvas: undefined () zone.js/zone-patch-canvas.min: undefined () zone.js/zone-patch-cordova: undefined () zone.js/zone-patch-cordova.min: undefined () zone.js/zone-patch-electron: undefined () zone.js/zone-patch-electron.min: undefined () zone.js/zone-patch-fetch: undefined () zone.js/zone-patch-fetch.min: undefined () zone.js/zone-patch-jsonp: undefined () zone.js/zone-patch-jsonp.min: undefined () zone.js/zone-patch-message-port: undefined () zone.js/zone-patch-message-port.min: undefined () zone.js/zone-patch-promise-test: undefined () zone.js/zone-patch-promise-test.min: undefined () zone.js/zone-patch-resize-observer: undefined () zone.js/zone-patch-resize-observer.min: undefined () zone.js/zone-patch-rxjs: undefined () zone.js/zone-patch-rxjs-fake-async: undefined () zone.js/zone-patch-rxjs-fake-async.min: undefined () zone.js/zone-patch-rxjs.min: undefined () zone.js/zone-patch-socket-io: undefined () zone.js/zone-patch-socket-io.min: undefined () zone.js/zone-patch-user-media: undefined () zone.js/zone-patch-user-media.min: undefined () npmGlobalPackages: @angular-devkit/schematics-cli: 0.801.1 @angular/cli: 14.2.6 @aws-amplify/cli: 10.2.3 @ionic/cli: 6.6.0 angular: 1.7.8 ng2-custom-carousel: 0.0.30 npm: 8.18.0 ```

Describe the bug

When creating two models with a one to many relationship acording to the example on the documentation https://docs.amplify.aws/lib/datastore/relational/q/platform/js/:

type Post @model {
  id: ID!
  title: String!
  comments: [Comment] @hasMany
}

type Comment @model {
  id: ID!
  post: Post @belongsTo
  content: String!
}

It creates under the hood an attribute in the Comment model called postCommentsId where it stores the Id referencing to the Post it belongs to. It even creates a GSI with that field as primary key to make even easier to search for this attribute. This attribute can be used for instance from the query editor in the aws console for appsync, to query, sync or subscribe for Comments that belong to a Post with a particular post Id.

query MyQuery {
  listComments(filter: {postCommentsId: {eq: "YOUR_POST_ID"}}) {
    items {
      id
    }
  }
}

This is the ideal behaviour I think it should had. But the problem comes when I try to use this attribute with DataStore query or subscribe since it is not pressent on the javascript implementation that Amplify creates when generating models. In the documentation it says that you can query for the comments of a post by querying for the post and getting the comments field or directly querying for the comments with a post with that id:

const comments = await DataStore.query(Comment, c => c.post.id.eq('YOUR_POST_ID'));

But there is no option for querying directly for the Comments with a postCommentsId equal to the Post Id you want, even if this GSI actually exists. This is annoying with queries although it can be saved using the options in the documentation, but it becomes a big problem with subscriptions and authorizations. DataStore javascript classes do not allow to subscribe to fields in relationships:

    DataStore.configure({
      syncExpressions: [
        syncExpression(Comment, c => c.post.id.eq('YOUR_POST_ID'))
      ]
    });

This doesn't work and as the field postCommentsId does not exists in the javascript classes created, we can also not filter for that field so filter and only syncronize Comments that belong to a Post in a system with thousands of Comments becomes impossible.

It also create problems with authoization, if I use a custom claim to store the list of Posts a User has and I want to allow access to the Comments only to users that has the postCommentsId field among this Posts from the claim it doesn't work since mutations do not return this field among the attributes so the Auth resolver fails to "unauthorized".

type Comment @model
  @auth(
    rules: [
      {
        allow: groups
        groupsField: "postCommentsId"
        groupClaim: "userPosts"
      }
    ]
  ) {
  id: ID!
  post: Post @belongsTo
  content: String!
}

I have tried by creating the field manually with the model:

type Comment @model {
  id: ID!
  postCommentsId: ID! @index
  post: Post @belongsTo
  content: String!
}

Also creating the index like appsync says in the documentation here https://docs.amplify.aws/cli/graphql/data-modeling/#belongs-to-relationship:

type Post @model {
  id: ID!
  title: String!
  comments: [Comment] @hasMany(indexName: "byPost", fields: ["id"])
}

type Comment @model {
  id: ID!
  postID: ID! @index(name: "byPost", sortKeyFields: ["content"])
  content: String!
  post: Post @belongsTo(fields: ["postID"])
}

But none of this works. This is a continuation of the issue #8285 that was closed and forgotten almost two years ago but I think this is very necessary.

Expected behavior

Datastore includes the foreign key Id attributes in one to many relationships in javascript generated classes so queries and subscriptions can use it.

Reproduction steps

Follow example in documentation.

Code Snippet

// Put your code below this line.

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

iartemiev commented 1 year ago

For this schema

type Post @model {
  id: ID!
  title: String!
  comments: [Comment] @hasMany
}

type Comment @model {
  id: ID!
  post: Post @belongsTo
  content: String!
}

You can filter comments by the post instance. For example:

const post = await DataStore.query(Post, 'YOUR_POST_ID');
const comments = await DataStore.query(Comment, c => c.post.eq(post));

If you want to be able to specify the foreign key specifically, try

type Post @model {
  id: ID!
  title: String!
  comments: [Comment] @hasMany(indexName: "byPost", fields: ["id"])
}

type Comment @model {
  id: ID!
  postID: ID! @index(name: "byPost")
  post: Post @belongsTo(fields: ["postID"])
  content: String!
}

Note: the @index doesn't need a sortKeyFields attribute. Then you can you filter by postID:

const comments = await DataStore.query(Comment, c => c.postID.eq('YOUR_POST_ID'));

Regarding your authorization question a couple points:

  1. The groupClaim value would need to match the name of the custom claim in your Cognito identity token. How are you adding the custom claim and injecting values into it? Are you leveraging a pre token generation lambda trigger?
  2. When you perform a save such as
const post = await DataStore.save(new Post({ title: "test" }));
await DataStore.save(
  new Comment({
    post: post,
    content: "test comment",
  })
);

The postCommentsId in the actual mutation query that gets sent to AppSync will be automatically populated with the id field of the post. You can see this if you inspect the network requests in dev tools. This is also true for queries and subscriptions - the postCommentsId is included in the selection set. Therefore the auth resolver in AppSync will also have access to this field if you’re using it for auth. If you enable Logs with verbose content in your AppSync API settings, you should see the postCommentsId in the resolver evaluation logs (in $ctx.args or $ctx.result for create and update/delete mutations respectively).

ritxweb commented 1 year ago

Thanks @iartemiev also for formating my question.

Sorry if maybe I was not clear enough. I have already tried all the possible forms to cretate the field with the foreign key, including the one you mentioned that I also mentioned as the last way (the AppSync way) of creating it. With or without the "indexName" or "fields" attributes in the @hasMany declaration, the "fields" attribute in the @belongsTo declaration, or the "sortKeyFields" attribute in the @index declaration, it doesn't matter, it does not work anyway. Please check it because this failure has been around for a long time. It is as simple as that the javascript models generated do not contain the foreign key field, I don't know if it is the same for Android or IOs libraries but in javascript it works that way and that is wrong because subscriptions filtering by this relation are not possible and I think it is really important and a key concept.

And about the authorization sorry again if I was not clear enough. Yes, I use the pre token generator lambda trigger but that is not the problem. Probably it works with creation since you send the post Id, but in mutations it does not work. If instead of creating a Comment you just edit the "content" field of the Comment and then save it to DataStore, you will see that the field with the post Id is not sent in the request and, what is really important, it does not appear in the requested return fields and since subscriptions are based on this fields, this value is never compared with the values in the custom claim, therefore never sending the corresponding notifications to subscribers to changes on Comments for a Post Id or allowed to read them.

Please let me know if there is something not clear yet. As I said, this has been around for quite long time and is as simple as include this field in the javascript models (created automatically or manually with the "fields" attribute in the @belongsTo declaration) and I think it is really important for regular use of DataStore and Amplify.

ritxweb commented 1 year ago

I have discovered that even the input for the subscriptions to changes on this model Comment generated by the trasnformer does include this foreign key field to subscribe to changes on Comments that belong to a Post Id:

input ModelSubscriptionCommentFilterInput {
    postId: ModelSubscriptionIDInput
    and: [ModelSubscriptionCommentFilterInput]
    or: [ModelSubscriptionCommentFilterInput]
}

So this is clearly an error in the Datastore model generation for javascript that should include this foreign key field. Please fix it as soon as possible so we can continue with the development.

As a temporary workaround I have included a new field with an index in this model Comment to include this foreign key with the Id of the Post:

type Comment @model {
  id: ID!
  post: Post @belongsTo
  postIdWA: ID! @index(name: "byPostWA")
  content: String!
}

This is not ideal because everytime I create a Comment from the DataStore javascript code I have to fill both fields, the post and the postIdWA and check in the backend resolver that both Ids are the same, but at least now it allows me to subscribe to Comments filtering for this field:

DataStore.configure({
  syncExpressions: [
    syncExpression(Comment, c => c.postIdWA.eq('YOUR_POST_ID'))
  ]
});

But what is surprising now (I don't know if this is the normal behavior) is that the ModelSubscriptionCommentFilterInput mentioned before does not include this field so, even if now the subscriptions are working, I am receiving warnings in the javascript console and in the socket messages to this subscriptions:

Connection failed: {"errors":[{"message":"The variables input contains a field that is not defined for input object type 'ModelSubscriptionCommentFilterInput' "}]}

If someone could explain me if this is normal and the way to get rid of those messages I would also apreciate it.

ritxweb commented 1 year ago

any news about this? I can see more people affected by this issue: https://stackoverflow.com/questions/70772111/amplify-id-for-a-belongto-to-one-relationship-not-in-the-generated-typescript

ritxweb commented 1 year ago

Hello. Any news about this? Should we abandon Amplify for real projects till it become stable?

ritxweb commented 10 months ago

Hello. Any news about this?

ritxweb commented 6 months ago

Hello. Any news about this?