Copilot commented on code in PR #12503:
URL: https://github.com/apache/gravitino/pull/12503#discussion_r3840512235
##########
docs/open-api/policies.yaml:
##########
@@ -497,6 +497,8 @@ components:
- "FILESET"
- "TOPIC"
- "MODEL"
+ - "VIEW"
+ - "FUNCTION"
- "COLUMN"
Review Comment:
OpenAPI now includes VIEW and FUNCTION in `MetadataObject.type`, but
`PolicyContentBase.supportedObjectTypes.items.enum` (earlier in this file)
still only lists CATALOG/SCHEMA/TABLE/FILESET/TOPIC/MODEL. This makes the spec
inconsistent and will reject creating policy contents that declare supported
object types VIEW/FUNCTION.
##########
clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/PolicyIT.java:
##########
@@ -759,6 +827,380 @@ public void testAssociatePoliciesToModel() {
MetadataObject.Type.MODEL,
policy3.associatedObjects().objects()[0].type());
}
+ @Test
+ public void testAssociatePoliciesToView() {
+ PolicyContent content =
+ PolicyContents.custom(
+ ImmutableMap.of("rule1", "value1"),
ImmutableSet.of(MetadataObject.Type.VIEW), null);
+ Policy policy1 =
+ metalake.createPolicy(
+ GravitinoITUtils.genRandomName("policy_it_view_policy1"),
+ "custom",
+ null,
+ true,
+ content);
+ Policy policy2 =
+ metalake.createPolicy(
+ GravitinoITUtils.genRandomName("policy_it_view_policy2"),
+ "custom",
+ null,
+ true,
+ content);
+ Policy policy3 =
+ metalake.createPolicy(
+ GravitinoITUtils.genRandomName("policy_it_view_policy3"),
+ "custom",
+ null,
+ true,
+ content);
+
+ // Associate policies to catalog
+ relationalCatalog.supportsPolicies().associatePolicies(new String[]
{policy1.name()}, null);
+
+ // Associate policies to schema
+ schema.supportsPolicies().associatePolicies(new String[] {policy2.name()},
null);
+
+ // Test associate policies to view
+ String[] policies =
+ view.supportsPolicies().associatePolicies(new String[]
{policy3.name()}, null);
+
+ Assertions.assertEquals(1, policies.length);
+ Assertions.assertEquals(policy3.name(), policies[0]);
+
+ // Test list associated policies for view
+ String[] policies1 = view.supportsPolicies().listPolicies();
+ Assertions.assertEquals(3, policies1.length);
+ Set<String> policyNames = Sets.newHashSet(policies1);
+ Assertions.assertTrue(policyNames.contains(policy1.name()));
+ Assertions.assertTrue(policyNames.contains(policy2.name()));
+ Assertions.assertTrue(policyNames.contains(policy3.name()));
+
+ // Test list associated policies with details for view
+ Policy[] policies2 = view.supportsPolicies().listPolicyInfos();
+ Assertions.assertEquals(3, policies2.length);
+
+ Set<Policy> nonInheritedPolicies =
+ Arrays.stream(policies2)
+ .filter(policy -> !policy.inherited().get())
+ .collect(Collectors.toSet());
+ Set<Policy> inheritedPolicies =
+ Arrays.stream(policies2)
+ .filter(policy -> policy.inherited().get())
+ .collect(Collectors.toSet());
+
+ Assertions.assertEquals(1, nonInheritedPolicies.size());
+ Assertions.assertEquals(2, inheritedPolicies.size());
+ Assertions.assertTrue(nonInheritedPolicies.contains(policy3));
+ Assertions.assertTrue(inheritedPolicies.contains(policy1));
+ Assertions.assertTrue(inheritedPolicies.contains(policy2));
+
+ // Test get associated policy for view
+ Policy resultPolicy1 = view.supportsPolicies().getPolicy(policy1.name());
+ Assertions.assertEquals(policy1, resultPolicy1);
+ Assertions.assertTrue(resultPolicy1.inherited().get());
+
+ Policy resultPolicy2 = view.supportsPolicies().getPolicy(policy2.name());
+ Assertions.assertEquals(policy2, resultPolicy2);
+ Assertions.assertTrue(resultPolicy2.inherited().get());
+
+ Policy resultPolicy3 = view.supportsPolicies().getPolicy(policy3.name());
+ Assertions.assertEquals(policy3, resultPolicy3);
+ Assertions.assertFalse(resultPolicy3.inherited().get());
+
+ // Test get objects associated with policy
+ Assertions.assertEquals(1, policy1.associatedObjects().count());
+ Assertions.assertEquals(
+ relationalCatalog.name(),
policy1.associatedObjects().objects()[0].name());
+ Assertions.assertEquals(
+ MetadataObject.Type.CATALOG,
policy1.associatedObjects().objects()[0].type());
+
+ Assertions.assertEquals(1, policy2.associatedObjects().count());
+ Assertions.assertEquals(schema.name(),
policy2.associatedObjects().objects()[0].name());
+ Assertions.assertEquals(
+ MetadataObject.Type.SCHEMA,
policy2.associatedObjects().objects()[0].type());
+
+ Assertions.assertEquals(1, policy3.associatedObjects().count());
+ Assertions.assertEquals(view.name(),
policy3.associatedObjects().objects()[0].name());
+ Assertions.assertEquals(
+ MetadataObject.Type.VIEW,
policy3.associatedObjects().objects()[0].type());
+ }
+
+ @Test
+ public void testAssociatePoliciesToFunction() {
+ PolicyContent content =
+ PolicyContents.custom(
+ ImmutableMap.of("rule1", "value1"),
+ ImmutableSet.of(MetadataObject.Type.FUNCTION),
+ null);
+ Policy policy1 =
+ metalake.createPolicy(
+ GravitinoITUtils.genRandomName("policy_it_func_policy1"),
+ "custom",
+ null,
+ true,
+ content);
+ Policy policy2 =
+ metalake.createPolicy(
+ GravitinoITUtils.genRandomName("policy_it_func_policy2"),
+ "custom",
+ null,
+ true,
+ content);
+ Policy policy3 =
+ metalake.createPolicy(
+ GravitinoITUtils.genRandomName("policy_it_func_policy3"),
+ "custom",
+ null,
+ true,
+ content);
+
+ // Associate policies to catalog
+ relationalCatalog.supportsPolicies().associatePolicies(new String[]
{policy1.name()}, null);
+
+ // Associate policies to schema
+ schema.supportsPolicies().associatePolicies(new String[] {policy2.name()},
null);
+
+ // Test associate policies to function
+ String[] policies =
+ function.supportsPolicies().associatePolicies(new String[]
{policy3.name()}, null);
+
+ Assertions.assertEquals(1, policies.length);
+ Assertions.assertEquals(policy3.name(), policies[0]);
+
+ // Test list associated policies for function
+ String[] policies1 = function.supportsPolicies().listPolicies();
+ Assertions.assertEquals(3, policies1.length);
+ Set<String> policyNames = Sets.newHashSet(policies1);
+ Assertions.assertTrue(policyNames.contains(policy1.name()));
+ Assertions.assertTrue(policyNames.contains(policy2.name()));
+ Assertions.assertTrue(policyNames.contains(policy3.name()));
+
+ // Test list associated policies with details for function
+ Policy[] policies2 = function.supportsPolicies().listPolicyInfos();
+ Assertions.assertEquals(3, policies2.length);
+
+ Set<Policy> nonInheritedPolicies =
+ Arrays.stream(policies2)
+ .filter(policy -> !policy.inherited().get())
+ .collect(Collectors.toSet());
+ Set<Policy> inheritedPolicies =
+ Arrays.stream(policies2)
+ .filter(policy -> policy.inherited().get())
+ .collect(Collectors.toSet());
+
+ Assertions.assertEquals(1, nonInheritedPolicies.size());
+ Assertions.assertEquals(2, inheritedPolicies.size());
+ Assertions.assertTrue(nonInheritedPolicies.contains(policy3));
+ Assertions.assertTrue(inheritedPolicies.contains(policy1));
+ Assertions.assertTrue(inheritedPolicies.contains(policy2));
+
+ // Test get associated policy for function
+ Policy resultPolicy1 =
function.supportsPolicies().getPolicy(policy1.name());
+ Assertions.assertEquals(policy1, resultPolicy1);
+ Assertions.assertTrue(resultPolicy1.inherited().get());
+
+ Policy resultPolicy2 =
function.supportsPolicies().getPolicy(policy2.name());
+ Assertions.assertEquals(policy2, resultPolicy2);
+ Assertions.assertTrue(resultPolicy2.inherited().get());
+
+ Policy resultPolicy3 =
function.supportsPolicies().getPolicy(policy3.name());
+ Assertions.assertEquals(policy3, resultPolicy3);
+ Assertions.assertFalse(resultPolicy3.inherited().get());
+
+ // Test get objects associated with policy
+ Assertions.assertEquals(1, policy1.associatedObjects().count());
+ Assertions.assertEquals(
+ relationalCatalog.name(),
policy1.associatedObjects().objects()[0].name());
+ Assertions.assertEquals(
+ MetadataObject.Type.CATALOG,
policy1.associatedObjects().objects()[0].type());
+
+ Assertions.assertEquals(1, policy2.associatedObjects().count());
+ Assertions.assertEquals(schema.name(),
policy2.associatedObjects().objects()[0].name());
+ Assertions.assertEquals(
+ MetadataObject.Type.SCHEMA,
policy2.associatedObjects().objects()[0].type());
+
+ Assertions.assertEquals(1, policy3.associatedObjects().count());
+ Assertions.assertEquals(function.name(),
policy3.associatedObjects().objects()[0].name());
+ Assertions.assertEquals(
+ MetadataObject.Type.FUNCTION,
policy3.associatedObjects().objects()[0].type());
+ }
+
+ @Test
+ public void testCascadeDeleteView() {
+ // Create a policy that supports VIEW
+ Policy policy =
createCustomPolicy(GravitinoITUtils.genRandomName("policy_it_cascade_view"));
+
Review Comment:
`testCascadeDeleteView` says it creates a policy that supports VIEW, but it
calls `createCustomPolicy(...)` which currently creates a policy with
`supportedObjectTypes = {CATALOG}` (see helper at end of file). If policy
supported types are enforced now or later, this test will either be invalid or
will fail to associate the policy to a view; consider passing the intended
supported object types into the helper or constructing a VIEW-specific policy
here.
This issue also appears on line 1077 of the same file.
--
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]
