[#9254,#9168] improvement(authz): Optimize metadata authorization in the list metalake and list role. #9255
Conversation
| .toArray(NameIdentifier[]::new); | ||
| } | ||
|
|
||
| public static Metalake[] filterMetalakes(Metalake[] metalakes, String expression) { |
There was a problem hiding this comment.
Why do we need to create a new method instead of reusing the method filterByExpression?
There was a problem hiding this comment.
Because this method requires passing a metalake.
There was a problem hiding this comment.
We can extract common part and reuse them instead of producing duplicated code.
There was a problem hiding this comment.
Pull request overview
This PR optimizes metadata authorization in the list metalakes operation by refactoring inline filtering logic into a reusable helper method. The change improves code maintainability by centralizing the authorization filtering logic and adds concurrent processing capabilities for better performance.
Key changes:
- Extracted metalake filtering logic from
MetalakeOperations.listMetalakes()into a dedicatedMetadataAuthzHelper.filterMetalakes()method - Implemented concurrent authorization evaluation using
CompletableFuturefor improved performance when filtering multiple metalakes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| MetalakeOperations.java | Simplified listMetalakes() by replacing inline filtering logic with a call to the new helper method |
| MetadataAuthzHelper.java | Added new filterMetalakes() method that performs concurrent authorization evaluation for metalakes |
server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
Outdated
Show resolved
Hide resolved
server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
Outdated
Show resolved
Hide resolved
| Map<Entity.EntityType, NameIdentifier> nameIdentifierMap = | ||
| spiltMetadataNames( |
There was a problem hiding this comment.
The method name 'spiltMetadataNames' appears to be a typo. It should likely be 'splitMetadataNames'. However, since this is an existing method being called (not introduced in this PR), this should be addressed separately if confirmed.
…ization/MetadataAuthzHelper.java Co-authored-by: Copilot <[email protected]>
…ization/MetadataAuthzHelper.java Co-authored-by: Copilot <[email protected]>
|
RoleOperations has the similar issue. Could u fix them together? |
fixed |
Could you modify the description to align to the new implementation? |
|
cc @danhuawang This will improve the performance of listing roles. |
hdygxsj
changed the title
hdygxsj
changed the title
2 participants
What changes were proposed in this pull request?
Optimize metadata authorization in the list metalake and list role.
Why are the changes needed?
Fix: #9254 #9168
Does this PR introduce any user-facing change?
None
How was this patch tested?
Existing test case in org.apache.gravitino.client.integration.test.authorization.MetalakeAuthorizationIT
Existing test case in org.apache.gravitino.client.integration.test.authorization.RoleAuthorizationIT