Call access control once per check #15131
Conversation
kokosing
force-pushed
the
origin/master/160_cache_access_control
branch
from
bcddf1a to
9d1413e
Compare
|
This is just an idea for discussion |
kokosing
force-pushed
the
origin/master/160_cache_access_control
branch
from
9d1413e to
6dc574f
Compare
Calling access check multiple times per same element not only needs more time to execute query but also may pollute audit information.
kokosing
force-pushed
the
origin/master/160_cache_access_control
branch
from
6dc574f to
51df95b
Compare
| this.accessControl = requireNonNull(accessControl, "accessControl is null"); | ||
| this.accessControl = newProxy( | ||
| AccessControl.class, | ||
| new OnlyOneInvocationHandler(requireNonNull(accessControl, "accessControl is null"))); |
There was a problem hiding this comment.
Can this be explicit Map here in this class?
|
What's the example query where this would be beneficial? |
|
BTW overall it makes sense to me. |
|
IMO, using reflection to override behavior is not a very sustainable design. In this case, I have no idea what would happen in a multi-threaded environment. If this is a behavior we want then we should add this behavior to the access control manager... but I'd have to think about if we want to handle caching here or in another layer. |
i think similar (#15131 (comment)) |
|
I agree, about not using reflections. Also this PR requires #15111 (proper |
Call access control once per check
Calling access check multiple times per same element not only
needs more time to execute query but also may pollute audit information.