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

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


The following commit(s) were added to refs/heads/main by this push:
     new 8bba06b8b Core: change to return ApplicablePolicies (#1415)
8bba06b8b is described below

commit 8bba06b8b6f3510e2514068b5a3a30faf0da0064
Author: Honah (Jonas) J. <hon...@apache.org>
AuthorDate: Mon Apr 21 13:22:34 2025 -0700

    Core: change to return ApplicablePolicies (#1415)
---
 .../apache/polaris/core/policy/PolicyEntity.java   |  9 ++++
 .../service/quarkus/catalog/PolicyCatalogTest.java | 25 ++++++++---
 .../service/catalog/policy/PolicyCatalog.java      | 51 +++++++++++++++++-----
 3 files changed, 68 insertions(+), 17 deletions(-)

diff --git 
a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyEntity.java 
b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyEntity.java
index 470822bb4..52dac8190 100644
--- 
a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyEntity.java
+++ 
b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyEntity.java
@@ -75,6 +75,15 @@ public class PolicyEntity extends PolarisEntity {
     return Integer.parseInt(getPropertiesAsMap().get(POLICY_VERSION_KEY));
   }
 
+  @JsonIgnore
+  public Namespace getParentNamespace() {
+    String parentNamespace = 
getInternalPropertiesAsMap().get(NamespaceEntity.PARENT_NAMESPACE_KEY);
+    if (parentNamespace != null) {
+      return RESTUtil.decodeNamespace(parentNamespace);
+    }
+    return null;
+  }
+
   public static class Builder extends PolarisEntity.BaseBuilder<PolicyEntity, 
Builder> {
     public Builder(Namespace namespace, String policyName, PolicyType 
policyType) {
       super();
diff --git 
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
 
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
index e2c3be891..1b4e616df 100644
--- 
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
+++ 
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
@@ -38,6 +38,7 @@ import jakarta.ws.rs.core.SecurityContext;
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.time.Clock;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -94,6 +95,7 @@ import 
org.apache.polaris.service.catalog.policy.PolicyCatalog;
 import org.apache.polaris.service.config.RealmEntityManagerFactory;
 import 
org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl;
 import org.apache.polaris.service.task.TaskExecutor;
+import org.apache.polaris.service.types.ApplicablePolicy;
 import org.apache.polaris.service.types.Policy;
 import org.apache.polaris.service.types.PolicyAttachmentTarget;
 import org.apache.polaris.service.types.PolicyIdentifier;
@@ -585,8 +587,8 @@ public class PolicyCatalogTest {
     policyCatalog.attachPolicy(POLICY2, POLICY_ATTACH_TARGET_NS, null);
     var applicablePolicies = policyCatalog.getApplicablePolicies(NS, null, 
null);
     assertThat(applicablePolicies.size()).isEqualTo(2);
-    assertThat(applicablePolicies.contains(p1)).isTrue();
-    assertThat(applicablePolicies.contains(p2)).isTrue();
+    assertThat(applicablePolicies.contains(policyToApplicablePolicy(p1, true, 
NS))).isTrue();
+    assertThat(applicablePolicies.contains(policyToApplicablePolicy(p2, false, 
NS))).isTrue();
 
     // attach policies to a table
     icebergCatalog.createTable(TABLE, SCHEMA);
@@ -605,8 +607,8 @@ public class PolicyCatalogTest {
     policyCatalog.attachPolicy(POLICY4, POLICY_ATTACH_TARGET_TBL, null);
     applicablePolicies = policyCatalog.getApplicablePolicies(NS, TABLE.name(), 
null);
     // p2 should be overwritten by p4, as they are the same type
-    assertThat(applicablePolicies.contains(p4)).isTrue();
-    assertThat(applicablePolicies.contains(p2)).isFalse();
+    assertThat(applicablePolicies.contains(policyToApplicablePolicy(p4, false, 
NS))).isTrue();
+    assertThat(applicablePolicies.contains(policyToApplicablePolicy(p2, true, 
NS))).isFalse();
   }
 
   @Test
@@ -626,6 +628,19 @@ public class PolicyCatalogTest {
     policyCatalog.attachPolicy(POLICY2, POLICY_ATTACH_TARGET_NS, null);
     var applicablePolicies = policyCatalog.getApplicablePolicies(NS, null, 
DATA_COMPACTION);
     // only p2 is with the type fetched
-    assertThat(applicablePolicies.contains(p2)).isTrue();
+    assertThat(applicablePolicies.contains(policyToApplicablePolicy(p2, false, 
NS))).isTrue();
+  }
+
+  private static ApplicablePolicy policyToApplicablePolicy(
+      Policy policy, boolean inherited, Namespace parent) {
+    return new ApplicablePolicy(
+        policy.getPolicyType(),
+        policy.getInheritable(),
+        policy.getName(),
+        policy.getDescription(),
+        policy.getContent(),
+        policy.getVersion(),
+        inherited,
+        Arrays.asList(parent.levels()));
   }
 }
diff --git 
a/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
 
b/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
index 195d0e128..45e5f17e6 100644
--- 
a/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
+++ 
b/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
@@ -24,11 +24,15 @@ import static 
org.apache.polaris.service.types.PolicyAttachmentTarget.TypeEnum.C
 import com.google.common.base.Strings;
 import jakarta.annotation.Nonnull;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Stream;
 import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.AlreadyExistsException;
@@ -52,6 +56,7 @@ import 
org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
 import org.apache.polaris.core.policy.exceptions.PolicyAttachException;
 import 
org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException;
 import org.apache.polaris.core.policy.validator.PolicyValidators;
+import org.apache.polaris.service.types.ApplicablePolicy;
 import org.apache.polaris.service.types.Policy;
 import org.apache.polaris.service.types.PolicyAttachmentTarget;
 import org.apache.polaris.service.types.PolicyIdentifier;
@@ -342,7 +347,7 @@ public class PolicyCatalog {
     return true;
   }
 
-  public List<Policy> getApplicablePolicies(
+  public List<ApplicablePolicy> getApplicablePolicies(
       Namespace namespace, String targetName, PolicyType policyType) {
     var targetFullPath = getFullPath(namespace, targetName);
     return getEffectivePolicies(targetFullPath, policyType);
@@ -369,14 +374,15 @@ public class PolicyCatalog {
    * @return a list of effective policies, combining inherited policies from 
upper levels and
    *     non-inheritable policies from the final entity
    */
-  private List<Policy> getEffectivePolicies(List<PolarisEntity> path, 
PolicyType policyType) {
+  private List<ApplicablePolicy> getEffectivePolicies(
+      List<PolarisEntity> path, PolicyType policyType) {
     if (path == null || path.isEmpty()) {
       return List.of();
     }
 
-    Map<String, PolicyEntity> inheritedPolicies = new LinkedHashMap<>();
-    // Final list of effective policies (inheritable + last-entity 
non-inheritable)
-    List<PolicyEntity> finalPolicies = new ArrayList<>();
+    Map<String, PolicyEntity> inheritablePolicies = new LinkedHashMap<>();
+    Set<Long> directAttachedInheritablePolicies = new HashSet<>();
+    List<PolicyEntity> nonInheritablePolicies = new ArrayList<>();
 
     // Process all entities except the last one
     for (int i = 0; i < path.size() - 1; i++) {
@@ -387,7 +393,7 @@ public class PolicyCatalog {
         // For non-last entities, we only carry forward inheritable policies
         if (policy.getPolicyType().isInheritable()) {
           // Put in map; overwrites by policyType if encountered again
-          inheritedPolicies.put(policy.getPolicyType().getName(), policy);
+          inheritablePolicies.put(policy.getPolicyType().getName(), policy);
         }
       }
     }
@@ -398,17 +404,22 @@ public class PolicyCatalog {
     for (var policy : lastPolicies) {
       if (policy.getPolicyType().isInheritable()) {
         // Overwrite anything by the same policyType in the inherited map
-        inheritedPolicies.put(policy.getPolicyType().getName(), policy);
+        inheritablePolicies.put(policy.getPolicyType().getName(), policy);
+        directAttachedInheritablePolicies.add(policy.getId());
       } else {
         // Non-inheritable => goes directly to final list
-        finalPolicies.add(policy);
+        nonInheritablePolicies.add(policy);
       }
     }
 
-    // Append all inherited policies at the end, preserving insertion order
-    finalPolicies.addAll(inheritedPolicies.values());
-
-    return finalPolicies.stream().map(PolicyCatalog::constructPolicy).toList();
+    return Stream.concat(
+            nonInheritablePolicies.stream().map(policy -> 
constructApplicablePolicy(policy, false)),
+            inheritablePolicies.values().stream()
+                .map(
+                    policy ->
+                        constructApplicablePolicy(
+                            policy, 
!directAttachedInheritablePolicies.contains(policy.getId()))))
+        .toList();
   }
 
   private List<PolicyEntity> getPolicies(PolarisEntity target, PolicyType 
policyType) {
@@ -506,4 +517,20 @@ public class PolicyCatalog {
         .setVersion(policyEntity.getPolicyVersion())
         .build();
   }
+
+  private static ApplicablePolicy constructApplicablePolicy(
+      PolicyEntity policyEntity, boolean inherited) {
+    Namespace parentNamespace = policyEntity.getParentNamespace();
+
+    return ApplicablePolicy.builder()
+        .setPolicyType(policyEntity.getPolicyType().getName())
+        .setInheritable(policyEntity.getPolicyType().isInheritable())
+        .setName(policyEntity.getName())
+        .setDescription(policyEntity.getDescription())
+        .setContent(policyEntity.getContent())
+        .setVersion(policyEntity.getPolicyVersion())
+        .setInherited(inherited)
+        .setNamespace(Arrays.asList(parentNamespace.levels()))
+        .build();
+  }
 }

Reply via email to