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

feat: add limited support for mixed operators #107

Merged
merged 3 commits into from
Dec 11, 2023
Merged

Conversation

rhamzeh
Copy link
Member

@rhamzeh rhamzeh commented Nov 13, 2023

This PR adds initial support for mixed operators

Description

Cases to be supported (both from and to DSL)
(with no direct assignment):

  • define relation: (rel1 and rel2) but not rel3
  • define relation: ((rel1 and rel2) but not rel3) # transformation back to DSL will not keep the extraneous surrounding brackets
  • define relation: rel1 and (rel2 but not rel3)
  • define relation: rel1 and ((rel2 but not rel3) or (rel4 and (rel5 but not rel6 from rel7)))

(with direct assignment, only if there is a single reference to the assignment and only if it is the first entry):

  • define relation: [user, user:*, user#follower with condition] or ((rel1 and rel2) but not rel3)
  • define relation: ([user] or ((rel1 and rel2)) but not rel3 # Note we can de-prioritize this as pending discussion

Cases NOT planned to supported yet, pending discussion:
Anything involving multiple references to direct assignment, or the presence of direct assignment not as the first entry, or references to self/this:

  • define relation: [user] as self | rel1 but not self
  • define relation: [user] as self | this but not rel1
  • define relation: rel1 but not [user]
  • define relation: [user] as self | (self or ((rel1 and rel2)) but not rel3
  • define relation: (rel3 or ((rel1 and rel2)) but not [user]
  • define relation: (rel3 or (([user] and rel2)) but not self
  • define relation: ([user] and allowed) or ([user] but not blocked]

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@rhamzeh rhamzeh changed the title chore(mixed-operators): add two tests for mixed operators feat(mixed-operators): add two tests for mixed operators Nov 13, 2023
@d-jeffery
Copy link
Contributor

d-jeffery commented Nov 29, 2023

Sadly this grammar, while pretty robust doesnt catch define relation: rel1 and ([user] or thing) as expected. Might have to validate it during transformation, unless we can spot a way to catch this with the grammar rules.

Nevermind, think i got it.

@d-jeffery d-jeffery marked this pull request as ready for review December 2, 2023 00:27
@d-jeffery d-jeffery requested a review from a team as a code owner December 2, 2023 00:27
pkg/js/transformer/dsltojson.ts Outdated Show resolved Hide resolved
pkg/js/validator/validate-dsl.ts Show resolved Hide resolved
@rhamzeh rhamzeh changed the title feat(mixed-operators): add two tests for mixed operators feat: add limited support for mixed operators Dec 11, 2023
@adriantam adriantam added this pull request to the merge queue Dec 11, 2023
Merged via the queue into main with commit b996e0f Dec 11, 2023
10 of 11 checks passed
@adriantam adriantam deleted the feat/mixed-operators branch December 11, 2023 17:10
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.

feat: support nested userset rewrite operands in the DSL
3 participants