LuciferYang opened a new pull request, #12253:
URL: https://github.com/apache/gravitino/pull/12253

   ### What changes were proposed in this pull request?
   
   `AuthorizationExpressionEvaluator` stored the converted OGNL expression as a 
`String` and evaluated it via `Ognl.getValue(String, ctx)`, which re-parses the 
expression into an AST on every call. This PR parses the expression once at 
construction and caches the AST in a process-level `ConcurrentHashMap` keyed by 
the converted OGNL string, then evaluates with `Ognl.getValue(tree, ctx)`.
   
   - The cache keys all originate from compile-time constants in 
`AuthorizationExpressionConstants` and `@AuthorizationExpression` annotations, 
so the map is naturally bounded and needs no eviction.
   - Semantics are preserved: `getValue(String, ctx)` already parses and then 
evaluates against the same context-as-root, so passing the pre-parsed tree only 
skips the re-parse. Verified against the OGNL 3.4.7 bytecode.
   - The shared tree is evaluated concurrently (`MetadataAuthzHelper.doFilter` 
runs on a thread pool). This is safe: OGNL only memoizes context-independent 
constant nodes during evaluation, every thread writes the same immutable value, 
publication is guarded by a volatile flag, and each evaluation builds its own 
`OgnlContext`.
   - Invalid expressions now fail fast at construction instead of on first 
evaluation.
   
   ### Why are the changes needed?
   
   On every authorized request the evaluator is constructed and evaluated, and 
in the list-filter fallback (`MetadataAuthzHelper.doFilter`) once per listed 
object, so the repeated parse cost scales with request volume and namespace 
size. Caching the parsed AST removes the redundant parse from these hot paths.
   
   Fix: #11135
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   Added unit tests in `TestAuthorizationExpressionEvaluator`:
   - parsed AST is reused across evaluators for the same expression, and 
differs for different expressions;
   - concurrent evaluation on a shared AST (16 threads x 200 iterations) 
produces correct allow/deny results with no error;
   - an invalid expression fails fast at construction with the `OgnlException` 
cause preserved.
   
   Existing allow/deny/owner regression tests continue to guard evaluation 
semantics. Ran `./gradlew :server-common:test --tests 
"*AuthorizationExpressionEvaluator*"`, plus `spotlessApply` and `javadoc`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to