Copilot commented on code in PR #7898:
URL: https://github.com/apache/gravitino/pull/7898#discussion_r2250361982
##########
server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java:
##########
@@ -70,6 +76,22 @@ public Response listRoles(
httpRequest,
() -> {
String[] names =
accessControlDispatcher.listRoleNamesByObject(metalake, object);
+ names =
+ Arrays.stream(names)
+ .filter(
+ role -> {
+ NameIdentifier[] nameIdentifiers =
+ new NameIdentifier[]
{NameIdentifierUtil.ofRole(metalake, role)};
+ return MetadataFilterHelper.filterByExpression(
+ metalake,
+ "METALAKE::OWNER || ROLE::OWNER ||
ROLE::SELF",
Review Comment:
[nitpick] The authorization expression is hardcoded. Consider extracting
this to a constant or configuration to improve maintainability and make it
easier to modify access control rules.
```suggestion
ROLE_LIST_AUTHORIZATION_EXPRESSION,
```
##########
server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java:
##########
@@ -70,6 +76,22 @@ public Response listRoles(
httpRequest,
() -> {
String[] names =
accessControlDispatcher.listRoleNamesByObject(metalake, object);
+ names =
+ Arrays.stream(names)
+ .filter(
+ role -> {
+ NameIdentifier[] nameIdentifiers =
+ new NameIdentifier[]
{NameIdentifierUtil.ofRole(metalake, role)};
Review Comment:
Creating a new array for each role name in the filter is inefficient.
Consider reusing the array or using a more efficient approach for single
element operations.
```suggestion
// Reuse a single-element array to avoid repeated allocations
final NameIdentifier[] nameIdentifiers = new NameIdentifier[1];
names =
Arrays.stream(names)
.filter(
role -> {
nameIdentifiers[0] =
NameIdentifierUtil.ofRole(metalake, role);
```
##########
server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java:
##########
@@ -70,6 +76,22 @@ public Response listRoles(
httpRequest,
() -> {
String[] names =
accessControlDispatcher.listRoleNamesByObject(metalake, object);
+ names =
+ Arrays.stream(names)
+ .filter(
+ role -> {
+ NameIdentifier[] nameIdentifiers =
+ new NameIdentifier[]
{NameIdentifierUtil.ofRole(metalake, role)};
+ return MetadataFilterHelper.filterByExpression(
+ metalake,
+ "METALAKE::OWNER || ROLE::OWNER ||
ROLE::SELF",
+ Entity.EntityType.ROLE,
+ nameIdentifiers)
+ .length
+ > 0;
+ })
+ .collect(Collectors.toList())
+ .toArray(new String[0]);
Review Comment:
Converting to List and then back to array is inefficient. Consider using
`toArray(String[]::new)` or collecting directly to an array to avoid
unnecessary intermediate collection.
```suggestion
.toArray(String[]::new);
```
--
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]
