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

check all / check unless behaviour #109

Closed
Geal opened this issue Sep 27, 2022 · 3 comments · Fixed by #113
Closed

check all / check unless behaviour #109

Geal opened this issue Sep 27, 2022 · 3 comments · Fixed by #113

Comments

@Geal
Copy link
Contributor

Geal commented Sep 27, 2022

the behaviour for check is to look for a set of matching facts, for which all expressions return true, and stop there. It can be a bit confusing sometimes because we might expect different behaviours.

One common case is the need for a check all behaviour: instead of stopping at the first set of facts that succeed, it would validate all of them. More precisely, for every set of fact that unifies, all the expressions should return true.

As an example, if we had the facts test("a"), test("b"), value("a", 1), value("a", 2):

  • with normal checks, check if test($c), value($c, $i), $i < 2 would succeed because it would select test("a"), value("a", 1) and stop there
  • with check all test($c), value($c, $i), $i < 2, it would accept test("a"), value("a", 1), ignore sets with test("b") because it won't unify with value, and ultimately fail because test("a"), value("a", 2) does not validate the expression

Anther way to write would be with a negative: since we have check if on one side, we could have check unless, that succeeds if it does not find any set o facts that unifies and validates the expressions. That previous example could be written as check unless test($c), value($c, $i), $i >= 2, but it does not encode the same behaviour: if there are no test or value fact, the check will succeed. We would like instead to have the facts, and that all of them match the condition.

I think check all could be a new feature (not replacing the old checks), that would give the token more capabilities (that right now are only possible in the authorizer with deny)

@Korbik
Copy link

Korbik commented Sep 28, 2022

If we add check all, we could also add a synonym for if with oneOf

check oneOf test($c), value($c, $i), $i < 2

@Geal
Copy link
Contributor Author

Geal commented Sep 29, 2022

I am not fond of adding aliases like this in the syntax if there's already one way to do it. But maybe that would be easier to understand

@Korbik
Copy link

Korbik commented Sep 29, 2022

I am not fond of adding aliases like this in the syntax if there's already one way to do it. But maybe that would be easier to understand

I'm exactly on the same position. I hate alias but I think it would make a clearer distinction than the if. I would keep the if more for a retro compatibility purpose. If it is only for me, then we shouldn't bother with it. The main purpose of the ticket is still improvement for me.

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 a pull request may close this issue.

2 participants