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: fix reference-style relationship validation #2533

Merged
merged 5 commits into from
May 9, 2024

Conversation

palpatim
Copy link
Member

@palpatim palpatim commented May 7, 2024

Description of changes

This change fixes relationship validation for reference-style relationships to support two use cases:

1. Fixes support for multiple relationships between models.

Example:

type Person @model @auth(rules: [{allow: public, provider: iam}]) {
  name: String
  authoredPosts: [Post] @hasMany(references: ["authorId"])
  editedPosts: [Post] @hasMany(references: ["editorId"])
}

type Post @model @auth(rules: [{allow: public, provider: iam}]) {
  title: String!
  content: String!
  authorId: ID!
  author: Person @belongsTo(references: ["authorId"])
  editorId: ID
  editor: Person @belongsTo(references: ["editorId"])
}

2. Fixes support for recursive relationships

Example:

type TreeNode @model {
  value: String
  parentId: ID
  parent: TreeNode @belongsTo(references: ["parentId"])
  children: [TreeNode] @hasMany(references: ["parentId"])
}

Additional changes

  • Add unit tests for previously missing validation cases
  • Add utilities and associated unit tests for schema reflection

NOTE: New E2Es covering these cases will come in a followup PR

Checklist

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

@palpatim palpatim requested review from a team as code owners May 7, 2024 20:41
@palpatim palpatim changed the title fix: support multiple relationships between models fix: fix reference-style relationship validation May 8, 2024
if (!associatedRelationalDirective) {
throw new InvalidDirectiveError(`Uni-directional relationships are not supported. ${expectedBidirectionalErrorMessages()}`);
return [];
Copy link
Member Author

Choose a reason for hiding this comment

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

This is safe to do because we have validation elsewhere for empty associated fields. This check was causing an early return in recursive schemas.

Copy link
Contributor

@sundersc sundersc left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Do we have E2E tests with references style to make sure the runtime behavior works as expected for multiple and recursive relationships between types.

Update: Nvm, I missed your note on the E2E tests.

@palpatim palpatim merged commit 7b3cf0e into main May 9, 2024
6 of 7 checks passed
@palpatim palpatim deleted the palpatim.fix.reference-count-validation branch May 9, 2024 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants