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(); + } }