Skip to content

Commit

Permalink
refactor: Clarify error when right_path exists (#1555)
Browse files Browse the repository at this point in the history
* refactor: Clarify error when right_path exists

Fixes #1552

This commit catches the assertion error when verifying the `right_path`
does not exist when a path has been identified on the left side of the
policy. This refactor was made to better clarify the cause of the error
for developers who may not be familiar with the technical limitations of
the current SQLAlchemy integration.

In my case, I had assumed that the use of foreign keys and paths on both
sides would be more performant, and thus the best choice for a policy,
when writing the authorization rules. The resulting error didn't include
any context about what was wrong with the policy I had written. This
commit aims to address this by providing a best effort guess to provide
a starting point for developers to debug the policy issue themselves.

Co-authored-by: gw <gwen@osohq.com>
Co-authored-by: Sam Scott <sam@osohq.com>
  • Loading branch information
3 people authored Jun 13, 2022
1 parent 81209b6 commit dd2482b
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions languages/python/sqlalchemy-oso/sqlalchemy_oso/partial.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
from polar.partial import dot_path
from polar.expression import Expression
from polar.variable import Variable
from polar.exceptions import UnsupportedError
from polar.exceptions import UnsupportedError, PolarRuntimeError

from sqlalchemy_oso.preprocess import preprocess

Expand Down Expand Up @@ -223,7 +223,11 @@ def translate_compare(expression: Expression, session: Session, model, get_model
# Dot operation is on the left hand side
if left_path[1:]:
assert left_path[0] == Variable("_this")
assert not right_path
if right_path:
raise PolarRuntimeError(
"Invalid comparison in policy. This may be caused by comparing the "
+ "foreign key field rather than the relationship property"
)
path, field_name = left_path[1:-1], left_path[-1]
return translate_dot(
path,
Expand Down

1 comment on commit dd2482b

@github-actions
Copy link

Choose a reason for hiding this comment

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

Rust Benchmark

Benchmark suite Current: dd2482b Previous: 81209b6 Ratio
rust_get_attribute 57447 ns/iter (± 5931) 44750 ns/iter (± 2062) 1.28
n_plus_one/100 2687505 ns/iter (± 149185) 2232888 ns/iter (± 6216) 1.20
n_plus_one/500 12865943 ns/iter (± 545244) 10756383 ns/iter (± 17303) 1.20
n_plus_one/1000 25725356 ns/iter (± 1024120) 21377826 ns/iter (± 142068) 1.20
unify_once 1163 ns/iter (± 271) 976 ns/iter (± 61) 1.19
unify_twice 2913 ns/iter (± 204) 2544 ns/iter (± 57) 1.15
many_rules 75457 ns/iter (± 8110) 61270 ns/iter (± 1284) 1.23
fib/5 632422 ns/iter (± 47390) 527356 ns/iter (± 9156) 1.20
prime/3 21655 ns/iter (± 1858) 17539 ns/iter (± 737) 1.23
prime/23 21985 ns/iter (± 2143) 17570 ns/iter (± 713) 1.25
prime/43 22189 ns/iter (± 2297) 17608 ns/iter (± 725) 1.26
prime/83 22316 ns/iter (± 1868) 17592 ns/iter (± 847) 1.27
prime/255 20032 ns/iter (± 1562) 16200 ns/iter (± 626) 1.24
indexed/100 7519 ns/iter (± 1630) 5814 ns/iter (± 626) 1.29
indexed/500 9641 ns/iter (± 3523) 7438 ns/iter (± 1794) 1.30
indexed/1000 12403 ns/iter (± 2832) 9479 ns/iter (± 463) 1.31
indexed/10000 25200 ns/iter (± 6152) 25300 ns/iter (± 3230) 1.00
not 7757 ns/iter (± 1615) 5851 ns/iter (± 87) 1.33
double_not 15698 ns/iter (± 1229) 12152 ns/iter (± 226) 1.29
De_Morgan_not 9585 ns/iter (± 665) 7863 ns/iter (± 168) 1.22
load_policy 1142125 ns/iter (± 91626) 891454 ns/iter (± 1276) 1.28
partial_and/1 37408 ns/iter (± 3289) 31104 ns/iter (± 1357) 1.20
partial_and/5 129038 ns/iter (± 9716) 110334 ns/iter (± 3141) 1.17
partial_and/10 250993 ns/iter (± 12386) 210319 ns/iter (± 4129) 1.19
partial_and/20 522288 ns/iter (± 31618) 427521 ns/iter (± 6136) 1.22
partial_and/40 1076474 ns/iter (± 66304) 909965 ns/iter (± 11766) 1.18
partial_and/80 2425668 ns/iter (± 133156) 2060197 ns/iter (± 3536) 1.18
partial_and/100 3077186 ns/iter (± 196769) 2716040 ns/iter (± 15891) 1.13
partial_rule_depth/1 110211 ns/iter (± 12499) 101056 ns/iter (± 4233) 1.09
partial_rule_depth/5 391814 ns/iter (± 276869) 332708 ns/iter (± 9628) 1.18
partial_rule_depth/10 856387 ns/iter (± 48770) 726516 ns/iter (± 11014) 1.18
partial_rule_depth/20 2506029 ns/iter (± 214064) 2045345 ns/iter (± 18124) 1.23
partial_rule_depth/40 9154942 ns/iter (± 934640) 7451087 ns/iter (± 114759) 1.23
partial_rule_depth/80 64287195 ns/iter (± 7174375) 43737276 ns/iter (± 730758) 1.47
partial_rule_depth/100 101889330 ns/iter (± 5935513) 80043113 ns/iter (± 586491) 1.27

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.