(gravitino) branch main updated: [#12685] fix(core): Fix built-in policy supportedObjectTypes validation and error message (#12686)

[mchades]

This is an automated email from the ASF dual-hosted git repository.

mchades pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git


The following commit(s) were added to refs/heads/main by this push:
     new cbadfbf9e9 [#12685] fix(core): Fix built-in policy 
supportedObjectTypes validation and error message (#12686)
cbadfbf9e9 is described below

commit cbadfbf9e98fd700e8ad27d1272ba9ff68e13640
Author: Tanay Paul <contact.tanayp...@gmail.com>
AuthorDate: Thu Sep 3 15:56:12 2026 +0530

    [#12685] fix(core): Fix built-in policy supportedObjectTypes validation and 
error message (#12686)
    
    ## What changes were proposed in this pull request?
    
    Fix the validation of `supportedObjectTypes` when updating built-in
    policies.
    
    `PolicyManager.updatePolicyEntity()` currently uses
    `Sets.difference(oldTypes, newTypes).isEmpty()`, which only detects
    removed object types. When the updated set contains additional object
    types, the validation incorrectly passes.
    
    This change:
    
    - validates that the old and new supported object type sets are exactly
    equal;
    - fixes the validation error message by supplying the expected and
    actual sets;
    - adds a regression test covering attempted supported object type
    changes for a built-in policy.
    
    ## Why are the changes needed?
    
    Built-in policies must not change their supported metadata object types
    during an update. The previous asymmetric set comparison did not enforce
    this invariant for additions.
    
    Fixes #12685
---
 .../org/apache/gravitino/policy/PolicyManager.java |  6 ++-
 .../apache/gravitino/policy/TestPolicyManager.java | 55 ++++++++++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java 
b/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
index ce91fe393e..e4dc65746c 100644
--- a/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
+++ b/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
@@ -527,11 +527,13 @@ public class PolicyManager implements PolicyDispatcher {
         if (policyType != Policy.BuiltInType.CUSTOM) {
           // cannot change the supported object types for built-in policies
           Preconditions.checkArgument(
-              Sets.difference(
+              Sets.symmetricDifference(
                       policyEntity.content().supportedObjectTypes(),
                       updateContent.getContent().supportedObjectTypes())
                   .isEmpty(),
-              "Policy content type mismatch: expected %s but got %s");
+              "Policy content type mismatch: expected %s but got %s",
+              policyEntity.content().supportedObjectTypes(),
+              updateContent.getContent().supportedObjectTypes());
         }
 
         newContent = updateContent.getContent();
diff --git 
a/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java 
b/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
index 5b80249042..5f69606220 100644
--- a/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
+++ b/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
@@ -354,6 +354,61 @@ public class TestPolicyManager {
     Assertions.assertTrue(enabledPolicy.enabled());
   }
 
+  @Test
+  public void testAlterBuiltInPolicyContent() {
+    String policyName = "policy_" + UUID.randomUUID().toString().replace("-", 
"");
+    policyManager.createPolicy(
+        METALAKE,
+        policyName,
+        Policy.BuiltInType.ICEBERG_COMPACTION,
+        null,
+        true,
+        PolicyContents.icebergDataCompaction());
+
+    // Adding a type not in the original set must be rejected.
+    Set<MetadataObject.Type> withExtra =
+        ImmutableSet.of(
+            MetadataObject.Type.CATALOG,
+            MetadataObject.Type.SCHEMA,
+            MetadataObject.Type.TABLE,
+            MetadataObject.Type.FILESET);
+    PolicyContent addedType = PolicyContents.custom(ImmutableMap.of(), 
withExtra, null);
+    IllegalArgumentException addEx =
+        Assertions.assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                policyManager.alterPolicy(
+                    METALAKE,
+                    policyName,
+                    PolicyChange.updateContent("system_iceberg_compaction", 
addedType)));
+    Assertions.assertTrue(
+        addEx.getMessage().contains("Policy content type mismatch"),
+        "expected mismatch message, got: " + addEx.getMessage());
+    // Format arguments must be substituted — neither placeholder should 
survive literally.
+    Assertions.assertFalse(
+        addEx.getMessage().contains("%s"),
+        "format args were not substituted: " + addEx.getMessage());
+
+    // Removing a type must equally be rejected.
+    Set<MetadataObject.Type> withFewer =
+        ImmutableSet.of(MetadataObject.Type.CATALOG, 
MetadataObject.Type.SCHEMA);
+    PolicyContent removedType = PolicyContents.custom(ImmutableMap.of(), 
withFewer, null);
+    IllegalArgumentException removeEx =
+        Assertions.assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                policyManager.alterPolicy(
+                    METALAKE,
+                    policyName,
+                    PolicyChange.updateContent("system_iceberg_compaction", 
removedType)));
+    Assertions.assertTrue(
+        removeEx.getMessage().contains("Policy content type mismatch"),
+        "expected mismatch message, got: " + removeEx.getMessage());
+    Assertions.assertFalse(
+        removeEx.getMessage().contains("%s"),
+        "format args were not substituted: " + removeEx.getMessage());
+  }
+
   @Test
   public void testDeletePolicy() {
     String policyName = "policy1" + UUID.randomUUID().toString().replace("-", 
"");