mchades commented on code in PR #13863:
URL: https://github.com/apache/gravitino/pull/13863#discussion_r4280328056


##########
clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TagOperationsAuthorizationIT.java:
##########
@@ -281,6 +284,32 @@ public void testListTagByMetadata() {
     String[] tagsLoadByNormalUser = tableSupportTagByNormalUser.listTags();
     Arrays.sort(tagsLoadByNormalUser);
     assertArrayEquals(new String[] {"tag2", "tag3"}, tagsLoadByNormalUser);
+
+    RESTClient restClient = getNormalUserRestClient();
+    String path =
+        "api/metalakes/" + METALAKE + "/objects/table/" + CATALOG + "." + 
SCHEMA + ".table1/tags";
+    TagNameListResponse nameListResponse =
+        restClient.get(
+            path,
+            TagNameListResponse.class,
+            Map.of(),
+            error -> {
+              throw new AssertionError(error);
+            });
+    Assertions.assertEquals(2, nameListResponse.getNames().length);
+    Assertions.assertTrue(nameListResponse.hasInaccessibleTags());

Review Comment:
   Please add an untagged object and an object whose only tag is inherited and 
inaccessible, asserting empty arrays with `false` versus `true` for both 
response modes. The current assertions only cover mixed visibility, so the 
empty-array distinction in #13859 remains untested.



##########
server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectPolicyOperations.java:
##########
@@ -390,14 +395,18 @@ public void testListPoliciesFiltersResolvedResult() 
throws IllegalAccessExceptio
           operations.listPoliciesForMetadataObject(
               metalake, table.type().toString(), table.fullName(), true);
       Assertions.assertEquals(Response.Status.OK.getStatusCode(), 
detailsResponse.getStatus());
-      Assertions.assertEquals(
-          0, ((PolicyListResponse) 
detailsResponse.getEntity()).getPolicies().length);
+      MetadataObjectPolicyListResponse details =
+          (MetadataObjectPolicyListResponse) detailsResponse.getEntity();
+      Assertions.assertEquals(0, details.getPolicies().length);
+      Assertions.assertTrue(details.hasInaccessiblePolicies());

Review Comment:
   Please add an authorization integration test for this flag. Both the 
dispatcher and authorization filter are mocked here, and 
`PolicyAuthorizationIT` does not call the object policy-list endpoint. Verify 
all-hidden and mixed-visible tag-derived policies through real authorization, 
checking the flag and visible arrays in both response modes.



-- 
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