amplify-codegen icon indicating copy to clipboard operation
amplify-codegen copied to clipboard

fix(amplify-codegen): swift - add has-many associatedFields for CPK use case

Open lawmicha opened this issue 2 years ago • 1 comments

Table of contents

  • Library Changes data-dev-preview https://github.com/aws-amplify/amplify-swift/pull/2730
  • Library Changes v1 - https://github.com/aws-amplify/amplify-swift/pull/2734
  • Library Changes main - https://github.com/aws-amplify/amplify-swift/pull/2735
  • Codegen Changes - https://github.com/aws-amplify/amplify-codegen/pull/538 (You are here)

Tagged release tagged-release-without-e2e-tests/cpk-has-many

Description of changes

This PR is a bug fix for CPK feature to add the foreign key fields of a has-many relationship as the "associatedFields" of the field. For uni-directional has-many relationships, with the parent having a CPK use case:

# LL.7. Implicit Uni-directional Has Many
# CLI.4. Implicit Uni-directional Has Many

type Post4 @model {
  postId: ID! @primaryKey(sortKeyFields:["title"])
  title: String!
  comments: [Comment4] @hasMany
}
type Comment4 @model {
  commentId: ID! @primaryKey(sortKeyFields:["content"])
  content: String!
}

# LL.11. Explicit Uni-directional Has Many
# CLI.8. Explicit Uni-directional Has Many

type Post8 @model {
  postId: ID! @primaryKey(sortKeyFields:["title"])
  title: String!
  comments: [Comment8] @hasMany(indexName:"byPost", fields:["postId", "title"])
}
type Comment8 @model {
  commentId: ID! @primaryKey(sortKeyFields:["content"])
  content: String!
  postId: ID @index(name: "byPost", sortKeyFields:["postTitle"]) # customized foreign key for parent primary key
  postTitle: String # customized foreign key for parent sort key
}
  • The generated swift schema file for Comment does not have information about which fields are foreign keys to the Post. "postId" and "postTitle" are string fields.
  • The parent model, Post, has a reference to the hasMany Comments. This reference has an associatedWith field which points to the primary key of the Post, ie.
.hasMany(post4.comments, is: .optional, ofType: Comment4.self, associatedWith: Comment4.keys.post4CommentsPostId),

The issue here is that it's missing the information about the entire CPK of Post, the library only knows that the comment is associated with the post by the field Comment4.keys.post4CommentsPostId on Comment.

By replacing associatedWith with associatedFields hasMany schema information to include all the foreign key composite key, we support CPK uni-directional has-many use cases. The final generated schema information will appear like this:

.hasMany(post4.comments, is: .optional, ofType: Comment4.self, associatedFields: [Comment4.keys.post4CommentsPostId, Comment4.keys.post4CommentsTitle]),

This is useful for querying for Comments by using associatedFields, done by the library's lazy loading list functionality. For more details on the library issue, see https://github.com/aws-amplify/amplify-swift/pull/2730

Since this codegen change is behind the CPK feature flag and generates "associatedFields" on the hasMany, this means the library needs to define and release first, followed by the codegen changes. Since CPK feature is available for both v1 and main (v2) of Amplify Swift library. We need to introduce the changes to v1, main, and not just data-dev-preview (lazy reference + custom selecion set feature branch) of the library.

Issue #, if available

Description of how you validated changes

Checklist

  • [ ] PR description included
  • [ ] yarn test passes
  • [ ] Tests are changed or added
  • [ ] Relevant documentation is changed or added (and PR referenced)
  • [ ] Breaking changes to existing customers are released behind a feature flag or major version update
  • [ ] Changes are tested using sample applications for all relevant platforms (iOS/android/flutter/Javascript) that use the feature added/modified

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

lawmicha avatar Feb 02 '23 20:02 lawmicha

Codecov Report

Merging #538 (1cbf6b1) into main (cf7f167) will increase coverage by 0.00%. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #538   +/-   ##
=======================================
  Coverage   85.69%   85.70%           
=======================================
  Files         148      148           
  Lines        7380     7385    +5     
  Branches     1962     1965    +3     
=======================================
+ Hits         6324     6329    +5     
  Misses        959      959           
  Partials       97       97           
Impacted Files Coverage Δ
...elgen-plugin/src/visitors/appsync-swift-visitor.ts 97.00% <100.00%> (+0.03%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Feb 02 '23 20:02 codecov-commenter