From 6a8247dfac61f99f0cf0a26e16e9ef7198307977 Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 6 Mar 2025 19:23:15 -0800 Subject: [PATCH 01/15] PolicyEntity and PolicyType add more tests use plural form fix test fix test Throw error instead of -1 add policyMapping persistence --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 86 ++++++ .../eclipselink/PolarisEclipseLinkStore.java | 117 +++++++++ .../main/resources/META-INF/persistence.xml | 1 + .../jpa/models/ModelPolicyMappingRecord.java | 153 +++++++++++ .../AtomicOperationMetaStoreManager.java | 176 +++++++++++++ .../core/persistence/BasePersistence.java | 3 +- .../persistence/PolarisMetaStoreManager.java | 6 +- .../TransactionWorkspaceMetaStoreManager.java | 50 ++++ .../persistence/dao/entity/BaseResult.java | 5 + .../AbstractTransactionalPersistence.java | 83 ++++++ .../TransactionalMetaStoreManagerImpl.java | 246 ++++++++++++++++++ .../TransactionalPersistence.java | 4 +- .../transactional/TreeMapMetaStore.java | 42 +++ .../TreeMapTransactionalPersistenceImpl.java | 83 ++++++ .../policy/PolarisPolicyMappingManager.java | 211 +++++++++++++++ .../policy/PolarisPolicyMappingRecord.java | 215 +++++++++++++++ .../core/policy/PolicyMappingPersistence.java | 151 +++++++++++ ...TransactionalPolicyMappingPersistence.java | 84 ++++++ .../BasePolarisMetaStoreManagerTest.java | 6 + .../PolarisTestMetaStoreManager.java | 239 +++++++++++++++++ 20 files changed, 1958 insertions(+), 3 deletions(-) create mode 100644 extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingRecord.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 7005d5cfea..a00aedb706 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -53,6 +53,7 @@ import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; @@ -60,6 +61,7 @@ import org.apache.polaris.jpa.models.ModelEntityActive; import org.apache.polaris.jpa.models.ModelEntityChangeTracking; import org.apache.polaris.jpa.models.ModelGrantRecord; +import org.apache.polaris.jpa.models.ModelPolicyMappingRecord; import org.apache.polaris.jpa.models.ModelPrincipalSecrets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -664,6 +666,90 @@ PolarisStorageIntegration loadPolarisStorageIntegrationInCurrentTxn( return storageIntegrationProvider.getStorageIntegrationForConfig(storageConfig); } + /** {@inheritDoc} */ + @Override + public void writeToPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + + this.store.writeToPolicyMappingRecords(localSession.get(), record); + } + + /** {@inheritDoc} */ + @Override + public void deleteFromPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + this.store.deleteFromPolicyMappingRecords(localSession.get(), record); + } + + /** {@inheritDoc} */ + @Override + public void deleteAllEntityPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore entity, + @Nonnull List mappingOnTarget, + @Nonnull List mappingOnPolicy) { + this.store.deleteAllEntityPolicyMappingRecords(localSession.get(), entity); + } + + /** {@inheritDoc} */ + @Nullable + @Override + public PolarisPolicyMappingRecord lookupPolicyMappingRecordInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode, + long policyCatalogId, + long policyId) { + return ModelPolicyMappingRecord.toPolicyMappingRecord( + this.store.lookupPolicyMappingRecord( + localSession.get(), + targetCatalogId, + targetId, + policyTypeCode, + policyCatalogId, + policyId)); + } + + /** {@inheritDoc} */ + @Nonnull + @Override + public List loadPoliciesOnTargetByTypeInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode) { + return this.store + .loadPoliciesOnTargetByType(localSession.get(), targetCatalogId, targetId, policyTypeCode) + .stream() + .map(ModelPolicyMappingRecord::toPolicyMappingRecord) + .toList(); + } + + /** {@inheritDoc} */ + @Nonnull + @Override + public List loadAllPoliciesOnTargetInCurrentTxn( + @Nonnull PolarisCallContext callCtx, long targetCatalogId, long targetId) { + return this.store + .loadAllPoliciesOnTarget(localSession.get(), targetCatalogId, targetId) + .stream() + .map(ModelPolicyMappingRecord::toPolicyMappingRecord) + .toList(); + } + + /** {@inheritDoc} */ + @Nonnull + @Override + public List loadAllPoliciesOnPolicyInCurrentTxn( + @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + return this.store + .loadAllPoliciesOnPolicy(localSession.get(), policyCatalogId, policyId) + .stream() + .map(ModelPolicyMappingRecord::toPolicyMappingRecord) + .toList(); + } + @Override public void rollback() { EntityManager session = localSession.get(); diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index bfc83ae373..ab1d8c91f8 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -35,10 +35,12 @@ import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.jpa.models.ModelEntity; import org.apache.polaris.jpa.models.ModelEntityActive; import org.apache.polaris.jpa.models.ModelEntityChangeTracking; import org.apache.polaris.jpa.models.ModelGrantRecord; +import org.apache.polaris.jpa.models.ModelPolicyMappingRecord; import org.apache.polaris.jpa.models.ModelPrincipalSecrets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -411,6 +413,121 @@ void deletePrincipalSecrets(EntityManager session, String clientId) { session.remove(modelPrincipalSecrets); } + void writeToPolicyMappingRecords( + EntityManager session, PolarisPolicyMappingRecord mappingRecord) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + session.persist(ModelPolicyMappingRecord.fromPolicyMappingRecord(mappingRecord)); + } + + void deleteFromPolicyMappingRecords( + EntityManager session, PolarisPolicyMappingRecord mappingRecord) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + ModelPolicyMappingRecord lookupPolicyMappingRecord = + lookupPolicyMappingRecord( + session, + mappingRecord.getTargetCatalogId(), + mappingRecord.getTargetId(), + mappingRecord.getPolicyTypeCode(), + mappingRecord.getPolicyCatalogId(), + mappingRecord.getPolicyId()); + + diagnosticServices.check(lookupPolicyMappingRecord != null, "policy_mapping_record_not_found"); + session.remove(lookupPolicyMappingRecord); + } + + void deleteAllEntityPolicyMappingRecords(EntityManager session, PolarisEntityCore entity) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + loadAllPoliciesOnPolicy(session, entity.getCatalogId(), entity.getId()) + .forEach(session::remove); + loadAllPoliciesOnTarget(session, entity.getCatalogId(), entity.getId()) + .forEach(session::remove); + } + + ModelPolicyMappingRecord lookupPolicyMappingRecord( + EntityManager session, + long targetCatalogId, + long targetId, + long policyTypeCode, + long policyCatalogId, + long policyId) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + return session + .createQuery( + "SELECT m from ModelPolicyMappingRecord m " + + "where m.targetCatalogId=:targetCatalogId " + + "and m.targetId=:targetId " + + "and m.policyTypeCode=:policyTypeCode " + + "and m.policyCatalogId=:policyCatalogId " + + "and m.policyId=:policyId", + ModelPolicyMappingRecord.class) + .setParameter("targetCatalogId", targetCatalogId) + .setParameter("targetId", targetId) + .setParameter("policyTypeCode", policyTypeCode) + .setParameter("policyCatalogId", policyCatalogId) + .setParameter("policyId", policyId) + .getResultStream() + .findFirst() + .orElse(null); + } + + List loadPoliciesOnTargetByType( + EntityManager session, long targetCatalogId, long targetId, int policyTypeCode) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + return session + .createQuery( + "SELECT m from ModelPolicyMappingRecord m " + + "where m.targetCatalogId=:targetCatalogId " + + "and m.targetId=:targetId " + + "and m.policyTypeCode=:policyTypeCode", + ModelPolicyMappingRecord.class) + .setParameter("targetCatalogId", targetCatalogId) + .setParameter("targetId", targetId) + .setParameter("policyTypeCode", policyTypeCode) + .getResultList(); + } + + List loadAllPoliciesOnTarget( + EntityManager session, long targetCatalogId, long targetId) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + return session + .createQuery( + "SELECT m from ModelPolicyMappingRecord m " + + " where m.targetCatalogId=:targetCatalogId " + + "and m.targetId=:targetId", + ModelPolicyMappingRecord.class) + .setParameter("targetCatalogId", targetCatalogId) + .setParameter("targetId", targetId) + .getResultList(); + } + + List loadAllPoliciesOnPolicy( + EntityManager session, long policyCatalogId, long policyId) { + diagnosticServices.check(session != null, "session_is_null"); + checkInitialized(); + + return session + .createQuery( + "SELECT m from ModelPolicyMappingRecord m " + + "where m.policyCatalogId=:policyCatalogId " + + "and m.policyId=:policyId", + ModelPolicyMappingRecord.class) + .setParameter("policyCatalogId", policyCatalogId) + .setParameter("policyId", policyId) + .getResultList(); + } + private void checkInitialized() { diagnosticServices.check(this.initialized.get(), "store_not_initialized"); } diff --git a/extension/persistence/eclipselink/src/main/resources/META-INF/persistence.xml b/extension/persistence/eclipselink/src/main/resources/META-INF/persistence.xml index d435b3d334..cd86105990 100644 --- a/extension/persistence/eclipselink/src/main/resources/META-INF/persistence.xml +++ b/extension/persistence/eclipselink/src/main/resources/META-INF/persistence.xml @@ -29,6 +29,7 @@ org.apache.polaris.jpa.models.ModelEntityActive org.apache.polaris.jpa.models.ModelEntityChangeTracking org.apache.polaris.jpa.models.ModelGrantRecord + org.apache.polaris.jpa.models.ModelPolicyMappingRecord org.apache.polaris.jpa.models.ModelPrincipalSecrets org.apache.polaris.jpa.models.ModelSequenceId NONE diff --git a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java new file mode 100644 index 0000000000..122eeadb88 --- /dev/null +++ b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPolicyMappingRecord.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.jpa.models; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.Table; +import jakarta.persistence.Version; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; + +@Entity +@Table( + name = "POLICY_MAPPING_RECORDS", + indexes = { + @Index( + name = "POLICY_MAPPING_RECORDS_BY_POLICY_INDEX", + columnList = "policyCatalogId,policyId,targetCatalogId,targetId") + }) +public class ModelPolicyMappingRecord { + // id of the catalog where target entity resides + @Id private long targetCatalogId; + + // id of the target entity + @Id private long targetId; + + // id associated to the policy type + @Id private int policyTypeCode; + + // id of the catalog where the policy entity resides + @Id private long policyCatalogId; + + // id of the policy + @Id private long policyId; + + // additional parameters of the mapping + private String parameters; + + // Used for Optimistic Locking to handle concurrent reads and updates + @Version private long version; + + public long getTargetCatalogId() { + return targetCatalogId; + } + + public long getTargetId() { + return targetId; + } + + public int getPolicyTypeCode() { + return policyTypeCode; + } + + public long getPolicyCatalogId() { + return policyCatalogId; + } + + public long getPolicyId() { + return policyId; + } + + public String getParameters() { + return parameters; + } + + public static ModelPolicyMappingRecord.Builder builder() { + return new ModelPolicyMappingRecord.Builder(); + } + + public static final class Builder { + private final ModelPolicyMappingRecord policyMappingRecord; + + private Builder() { + policyMappingRecord = new ModelPolicyMappingRecord(); + } + + public Builder targetCatalogId(long targetCatalogId) { + policyMappingRecord.targetCatalogId = targetCatalogId; + return this; + } + + public Builder targetId(long targetId) { + policyMappingRecord.targetId = targetId; + return this; + } + + public Builder policyTypeCode(int policyTypeCode) { + policyMappingRecord.policyTypeCode = policyTypeCode; + return this; + } + + public Builder policyCatalogId(long policyCatalogId) { + policyMappingRecord.policyCatalogId = policyCatalogId; + return this; + } + + public Builder policyId(long policyId) { + policyMappingRecord.policyId = policyId; + return this; + } + + public Builder parameters(String parameters) { + policyMappingRecord.parameters = parameters; + return this; + } + + public ModelPolicyMappingRecord build() { + return policyMappingRecord; + } + } + + public static ModelPolicyMappingRecord fromPolicyMappingRecord( + PolarisPolicyMappingRecord record) { + if (record == null) return null; + + return ModelPolicyMappingRecord.builder() + .targetCatalogId(record.getTargetCatalogId()) + .targetId(record.getTargetId()) + .policyTypeCode(record.getPolicyTypeCode()) + .policyCatalogId(record.getPolicyCatalogId()) + .policyId(record.getPolicyId()) + .parameters(record.getParameters()) + .build(); + } + + public static PolarisPolicyMappingRecord toPolicyMappingRecord(ModelPolicyMappingRecord model) { + if (model == null) return null; + + return new PolarisPolicyMappingRecord( + model.getTargetCatalogId(), + model.getTargetId(), + model.getPolicyCatalogId(), + model.getPolicyId(), + model.getPolicyTypeCode(), + model.getParameters()); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 7dcc9f0ac6..fcbd1cc4fa 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -61,10 +61,14 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -1821,4 +1825,176 @@ public Map getInternalPropertyMap( // return the result return new ResolvedEntityResult(entity, entityVersions.getGrantRecordsVersion(), grantRecords); } + + @Override + public @NotNull AttachmentResult attachPolicyToEntity( + @NotNull PolarisCallContext callCtx, + @NotNull List targetCatalogPath, + @NotNull PolarisEntityCore target, + @NotNull List policyCatalogPath, + @NotNull PolicyEntity policy, + Map parameters) { + // get metastore we should be using + BasePersistence ms = callCtx.getMetaStore(); + + PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode()); + if (policyType == null) { + // This should never happen + return new AttachmentResult( + BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); + } + + // TODO: this may make it necessary to push down this logic to policy persistence layer + if (policyType.isInheritable()) { + // Verify that there is no other mapping of the same type but different entity + List existingRecordsOfSameType = + ms.loadPoliciesOnTargetByType( + callCtx, target.getCatalogId(), target.getId(), policy.getPolicyTypeCode()); + if (existingRecordsOfSameType.size() > 1) { + return new AttachmentResult( + BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); + } else if (existingRecordsOfSameType.size() == 1) { + PolarisPolicyMappingRecord existingRecord = existingRecordsOfSameType.get(0); + if (existingRecord.getPolicyId() != policy.getId() + || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) { + return new AttachmentResult( + BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); + } + } + } + + PolarisPolicyMappingRecord mappingRecord = + this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); + return new AttachmentResult(mappingRecord); + } + + @Override + public @NotNull AttachmentResult detachPolicyFromEntity( + @NotNull PolarisCallContext callCtx, + @NotNull List catalogPath, + @NotNull PolarisEntityCore target, + @NotNull List policyCatalogPath, + @NotNull PolicyEntity policy) { + // get metastore we should be using + BasePersistence ms = callCtx.getMetaStore(); + + PolarisPolicyMappingRecord mappingRecord = + ms.lookupPolicyMappingRecord( + callCtx, + target.getCatalogId(), + target.getId(), + policy.getPolicyTypeCode(), + policy.getCatalogId(), + policy.getId()); + if (mappingRecord == null) { + return new AttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null); + } + + ms.deleteFromPolicyMappingRecords(callCtx, mappingRecord); + + return new AttachmentResult(mappingRecord); + } + + @Override + public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntity( + @NotNull PolarisCallContext callCtx, @NotNull PolarisEntityCore target) { + // get metastore we should be using + BasePersistence ms = callCtx.getMetaStore(); + + int grantRecordVersion = + ms.lookupEntityGrantRecordsVersion(callCtx, target.getCatalogId(), target.getId()); + if (grantRecordVersion == 0) { + // Target entity does not exists + return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + } + + final List policyMappingRecords = + ms.loadAllPoliciesOnTarget(callCtx, target.getCatalogId(), target.getId()); + + List policyEntities = + loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); + return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); + } + + @Override + public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntityByType( + @NotNull PolarisCallContext callCtx, + @NotNull PolarisEntityCore target, + @NotNull PolicyType policyType) { + // get metastore we should be using + BasePersistence ms = callCtx.getMetaStore(); + + int grantRecordVersion = + ms.lookupEntityGrantRecordsVersion(callCtx, target.getCatalogId(), target.getId()); + if (grantRecordVersion == 0) { + // Target entity does not exists + return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + } + + final List policyMappingRecords = + ms.loadAllPoliciesOnTarget(callCtx, target.getCatalogId(), target.getId()); + + List policyEntities = + loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); + return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); + } + + /** + * Create and persist a new policy mapping record + * + * @param callCtx call context + * @param ms meta store in read/write mode + * @param target target + * @param policy policy + * @param parameters optional parameters + * @return new policy mapping record which was created and persisted + */ + private @Nonnull PolarisPolicyMappingRecord persistNewPolicyMappingRecord( + @Nonnull PolarisCallContext callCtx, + @Nonnull BasePersistence ms, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyEntity policy, + Map parameters) { + callCtx.getDiagServices().checkNotNull(target, "unexpected_null_target"); + callCtx.getDiagServices().checkNotNull(policy, "unexpected_null_policy"); + + PolarisPolicyMappingRecord mappingRecord = + new PolarisPolicyMappingRecord( + target.getCatalogId(), + target.getId(), + policy.getCatalogId(), + policy.getId(), + policy.getPolicyTypeCode(), + parameters); + + ms.writeToPolicyMappingRecords(callCtx, mappingRecord); + + return mappingRecord; + } + + /** + * Load policies from a list of policy mapping records + * + * @param callCtx call context + * @param ms meta store + * @param policyMappingRecords a list of policy mapping records + * @return a list of policy entities + */ + private List loadPoliciesFromMappingRecords( + @Nonnull PolarisCallContext callCtx, + @Nonnull BasePersistence ms, + @Nonnull List policyMappingRecords) { + List policyEntityIds = + policyMappingRecords.stream() + .map( + policyMappingRecord -> + new PolarisEntityId( + policyMappingRecord.getPolicyCatalogId(), + policyMappingRecord.getPolicyId())) + .distinct() + .collect(Collectors.toList()); + return ms.lookupEntities(callCtx, policyEntityIds).stream() + .map(PolicyEntity::of) + .collect(Collectors.toList()); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index 88e5143cf8..45460eb46f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -31,6 +31,7 @@ import org.apache.polaris.core.entity.PolarisEntityId; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; +import org.apache.polaris.core.policy.PolicyMappingPersistence; /** * Interface to the Polaris persistence backend, with which to persist and retrieve all the data @@ -41,7 +42,7 @@ * the underlying data store. The goal is to make it really easy to back this using databases like * Postgres or simpler KV store. */ -public interface BasePersistence { +public interface BasePersistence extends PolicyMappingPersistence { /** * The returned id must be fully unique within a realm and never reused once generated, whether or * not anything ends up committing an entity with the generated id. diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java index cc082bc27b..da2ab521e1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java @@ -42,6 +42,7 @@ import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; +import org.apache.polaris.core.policy.PolarisPolicyMappingManager; import org.apache.polaris.core.storage.PolarisCredentialVendor; /** @@ -49,7 +50,10 @@ * authorization. It uses the underlying persistent metastore to store and retrieve Polaris metadata */ public interface PolarisMetaStoreManager - extends PolarisSecretsManager, PolarisGrantManager, PolarisCredentialVendor { + extends PolarisSecretsManager, + PolarisGrantManager, + PolarisCredentialVendor, + PolarisPolicyMappingManager { /** * Bootstrap the Polaris service, creating the root catalog, root principal, and associated diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 4f26c51fa6..2d8d7cdbd9 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -49,7 +49,10 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisStorageActions; +import org.jetbrains.annotations.NotNull; /** * Wraps an existing impl of PolarisMetaStoreManager and delegates expected "read" operations @@ -394,4 +397,51 @@ public ResolvedEntityResult refreshResolvedEntity( .fail("illegal_method_in_transaction_workspace", "refreshResolvedEntity"); return null; } + + @Override + public @NotNull AttachmentResult attachPolicyToEntity( + @NotNull PolarisCallContext callCtx, + @NotNull List targetCatalogPath, + @NotNull PolarisEntityCore target, + @NotNull List policyCatalogPath, + @NotNull PolicyEntity policy, + Map parameters) { + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "attachPolicyToEntity"); + return null; + } + + @Override + public @NotNull AttachmentResult detachPolicyFromEntity( + @NotNull PolarisCallContext callCtx, + @NotNull List catalogPath, + @NotNull PolarisEntityCore target, + @NotNull List policyCatalogPath, + @NotNull PolicyEntity policy) { + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "detachPolicyFromEntity"); + return null; + } + + @Override + public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntity( + @NotNull PolarisCallContext callCtx, @NotNull PolarisEntityCore target) { + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "loadPoliciesOnEntity"); + return null; + } + + @Override + public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntityByType( + @NotNull PolarisCallContext callCtx, + @NotNull PolarisEntityCore target, + @NotNull PolicyType policyType) { + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "loadPoliciesOnEntityByType"); + return null; + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java index b4e1757de5..d540be49ea 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java @@ -111,6 +111,11 @@ public enum ReturnStatus { // error caught while sub-scoping credentials. Error message will be returned SUBSCOPE_CREDS_ERROR(13), + // policy mapping not found + POLICY_MAPPING_NOT_FOUND(14), + + // policy mapping of same type already exists + POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS(15), ; // code for the enum diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java index c673c48499..28670b8a87 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java @@ -35,6 +35,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.EntityAlreadyExistsException; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -660,4 +661,86 @@ public EntityNameLookupRecord lookupEntityIdAndSubTypeByNameInCurrentTxn( new PolarisEntitiesActiveKey(catalogId, parentId, typeCode, name); return this.lookupEntityActiveInCurrentTxn(callCtx, entityActiveKey); } + + /** {@inheritDoc} */ + @Override + public void writeToPolicyMappingRecords( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + this.runActionInTransaction( + callCtx, () -> this.writeToPolicyMappingRecordsInCurrentTxn(callCtx, record)); + } + + /** {@inheritDoc} */ + @Override + public void deleteFromPolicyMappingRecords( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + this.runActionInTransaction( + callCtx, () -> this.deleteFromPolicyMappingRecordsInCurrentTxn(callCtx, record)); + } + + /** {@inheritDoc} */ + @Override + public void deleteAllEntityPolicyMappingRecords( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore entity, + @Nonnull List mappingOnTarget, + @Nonnull List mappingOnPolicy) { + this.runActionInTransaction( + callCtx, + () -> + this.deleteAllEntityPolicyMappingRecordsInCurrentTxn( + callCtx, entity, mappingOnTarget, mappingOnPolicy)); + } + + /** {@inheritDoc} */ + @Override + @Nullable + public PolarisPolicyMappingRecord lookupPolicyMappingRecord( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode, + long policyCatalogId, + long policyId) { + return this.runInReadTransaction( + callCtx, + () -> + this.lookupPolicyMappingRecordInCurrentTxn( + callCtx, targetCatalogId, targetId, policyTypeCode, policyCatalogId, policyId)); + } + + /** {@inheritDoc} */ + @Override + @Nonnull + public List loadPoliciesOnTargetByType( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode) { + return this.runInReadTransaction( + callCtx, + () -> + this.loadPoliciesOnTargetByTypeInCurrentTxn( + callCtx, targetCatalogId, targetId, policyTypeCode)); + } + + /** {@inheritDoc} */ + @Override + @Nonnull + public List loadAllPoliciesOnTarget( + @Nonnull PolarisCallContext callCtx, long targetCatalogId, long targetId) { + return this.runInReadTransaction( + callCtx, + () -> this.loadAllPoliciesOnTargetInCurrentTxn(callCtx, targetCatalogId, targetId)); + } + + /** {@inheritDoc} */ + @Override + @Nonnull + public List loadAllPoliciesOnPolicy( + @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + return this.runInReadTransaction( + callCtx, + () -> this.loadAllPoliciesOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId)); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index cf1e54b535..9836dae57c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -62,10 +62,14 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -2316,4 +2320,246 @@ public Map getInternalPropertyMap( entityCatalogId, entityId)); } + + /** {@inheritDoc} */ + @Override + public @NotNull AttachmentResult attachPolicyToEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List targetCatalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy, + Map parameters) { + // get metastore we should be using + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + + return ms.runInTransaction( + callCtx, + () -> + this.doAttachPolicyToEntity( + callCtx, ms, target, targetCatalogPath, policy, policyCatalogPath, parameters)); + } + + /** + * See {@link #attachPolicyToEntity(PolarisCallContext, List, PolarisEntityCore, List, + * PolicyEntity, Map)} + */ + private @Nonnull AttachmentResult doAttachPolicyToEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull PolarisEntityCore target, + @Nonnull List targetCatalogPath, + @Nonnull PolicyEntity policy, + @Nonnull List policyCatalogPath, + Map parameters) { + PolarisEntityResolver targetResolver = + new PolarisEntityResolver(callCtx, ms, targetCatalogPath, target); + PolarisEntityResolver policyResolver = + new PolarisEntityResolver(callCtx, ms, policyCatalogPath, policy); + if (targetResolver.isFailure() || policyResolver.isFailure()) { + return new AttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); + } + + PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode()); + if (policyType == null) { + // This should never happen + return new AttachmentResult( + BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); + } + + if (policyType.isInheritable()) { + // Verify that there is no other mapping of the same type but different entity + List existingRecordsOfSameType = + ms.loadPoliciesOnTargetByTypeInCurrentTxn( + callCtx, target.getCatalogId(), target.getId(), policy.getPolicyTypeCode()); + if (existingRecordsOfSameType.size() > 1) { + return new AttachmentResult( + BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); + } else if (existingRecordsOfSameType.size() == 1) { + PolarisPolicyMappingRecord existingRecord = existingRecordsOfSameType.get(0); + if (existingRecord.getPolicyId() != policy.getId() + || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) { + return new AttachmentResult( + BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); + } + } + } + + PolarisPolicyMappingRecord mappingRecord = + this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); + return new AttachmentResult(mappingRecord); + } + + /** {@inheritDoc} */ + @Override + public @Nonnull AttachmentResult detachPolicyFromEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List targetCatalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy) { + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + return ms.runInTransaction( + callCtx, + () -> + this.doDetachPolicyFromEntity( + callCtx, ms, target, targetCatalogPath, policy, policyCatalogPath)); + } + + /** + * See {@link #detachPolicyFromEntity(PolarisCallContext, List, PolarisEntityCore, + * List,PolicyEntity)} + */ + private AttachmentResult doDetachPolicyFromEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull PolarisEntityCore target, + @Nonnull List targetCatalogPath, + @Nonnull PolicyEntity policy, + @Nonnull List policyCatalogPath) { + PolarisEntityResolver targetResolver = + new PolarisEntityResolver(callCtx, ms, targetCatalogPath, target); + PolarisEntityResolver policyResolver = + new PolarisEntityResolver(callCtx, ms, policyCatalogPath, policy); + if (targetResolver.isFailure() || policyResolver.isFailure()) { + return new AttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); + } + + PolarisPolicyMappingRecord mappingRecord = + ms.lookupPolicyMappingRecordInCurrentTxn( + callCtx, + target.getCatalogId(), + target.getId(), + policy.getPolicyTypeCode(), + policy.getCatalogId(), + policy.getId()); + if (mappingRecord == null) { + return new AttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null); + } + + ms.deleteFromPolicyMappingRecordsInCurrentTxn(callCtx, mappingRecord); + + return new AttachmentResult(mappingRecord); + } + + /** {@inheritDoc} */ + @Override + public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntity( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target) { + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + return ms.runInReadTransaction(callCtx, () -> this.doLoadPoliciesOnEntity(callCtx, ms, target)); + } + + /** See {@link #loadPoliciesOnEntity(PolarisCallContext, PolarisEntityCore)} */ + private LoadPolicyMappingsResult doLoadPoliciesOnEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull PolarisEntityCore target) { + int grantRecordVersion = + ms.lookupEntityGrantRecordsVersionInCurrentTxn( + callCtx, target.getCatalogId(), target.getId()); + if (grantRecordVersion == 0) { + // Target entity does not exists + return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + } + + final List policyMappingRecords = + ms.loadAllPoliciesOnTargetInCurrentTxn(callCtx, target.getCatalogId(), target.getId()); + + List policyEntities = + loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); + return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); + } + + /** {@inheritDoc} */ + @Override + public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntityByType( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyType policyType) { + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + return ms.runInReadTransaction( + callCtx, () -> this.doLoadPoliciesOnEntityByType(callCtx, ms, target, policyType)); + } + + /** See {@link #loadPoliciesOnEntityByType(PolarisCallContext, PolarisEntityCore, PolicyType)} */ + public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyType policyType) { + int grantRecordVersion = + ms.lookupEntityGrantRecordsVersionInCurrentTxn( + callCtx, target.getCatalogId(), target.getId()); + if (grantRecordVersion == 0) { + // Target entity does not exists + return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + } + + final List policyMappingRecords = + ms.loadPoliciesOnTargetByTypeInCurrentTxn( + callCtx, target.getCatalogId(), target.getId(), policyType.getCode()); + List policyEntities = + loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); + return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); + } + + /** + * Create and persist a new policy mapping record + * + * @param callCtx call context + * @param ms meta store in read/write mode + * @param target target + * @param policy policy + * @param parameters optional parameters + * @return new policy mapping record which was created and persisted + */ + private @Nonnull PolarisPolicyMappingRecord persistNewPolicyMappingRecord( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyEntity policy, + Map parameters) { + callCtx.getDiagServices().checkNotNull(target, "unexpected_null_target"); + callCtx.getDiagServices().checkNotNull(policy, "unexpected_null_policy"); + + PolarisPolicyMappingRecord mappingRecord = + new PolarisPolicyMappingRecord( + target.getCatalogId(), + target.getId(), + policy.getCatalogId(), + policy.getId(), + policy.getPolicyTypeCode(), + parameters); + + ms.writeToPolicyMappingRecordsInCurrentTxn(callCtx, mappingRecord); + + return mappingRecord; + } + + /** + * Load policies from a list of policy mapping records + * + * @param callCtx call context + * @param ms meta store + * @param policyMappingRecords a list of policy mapping records + * @return a list of policy entities + */ + private List loadPoliciesFromMappingRecords( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull List policyMappingRecords) { + List policyEntityIds = + policyMappingRecords.stream() + .map( + policyMappingRecord -> + new PolarisEntityId( + policyMappingRecord.getPolicyCatalogId(), + policyMappingRecord.getPolicyId())) + .distinct() + .collect(Collectors.toList()); + return ms.lookupEntitiesInCurrentTxn(callCtx, policyEntityIds).stream() + .map(PolicyEntity::of) + .collect(Collectors.toList()); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java index f3d3842bb2..2057991db0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java @@ -36,6 +36,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.IntegrationPersistence; +import org.apache.polaris.core.policy.TransactionalPolicyMappingPersistence; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -44,7 +45,8 @@ * which can support a runInTransaction semantic, while providing default implementations of some of * the BasePersistence methods in terms of lower-level methods that subclasses must implement. */ -public interface TransactionalPersistence extends BasePersistence, IntegrationPersistence { +public interface TransactionalPersistence + extends BasePersistence, IntegrationPersistence, TransactionalPolicyMappingPersistence { /** * Run the specified transaction code (a Supplier lambda type) in a database read/write diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java index b37f6e840f..c914140407 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapMetaStore.java @@ -31,6 +31,7 @@ import org.apache.polaris.core.entity.PolarisEntityCore; import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; /** Implements a simple in-memory store for Polaris, using tree-map */ public class TreeMapMetaStore { @@ -209,6 +210,10 @@ public boolean isWrite() { // slice to store principal secrets private final Slice slicePrincipalSecrets; + private final Slice slicePolicyMappingRecords; + + private final Slice slicePolicyMappingRecordsByPolicy; + // next id generator private final AtomicLong nextId = new AtomicLong(); @@ -266,6 +271,29 @@ public TreeMapMetaStore(@Nonnull PolarisDiagnostics diagnostics) { principalSecrets -> String.format("%s", principalSecrets.getPrincipalClientId()), PolarisPrincipalSecrets::new); + this.slicePolicyMappingRecords = + new Slice<>( + policyMappingRecord -> + String.format( + "%d::%d::%d::%d::%d", + policyMappingRecord.getTargetCatalogId(), + policyMappingRecord.getTargetId(), + policyMappingRecord.getPolicyTypeCode(), + policyMappingRecord.getPolicyCatalogId(), + policyMappingRecord.getPolicyId()), + PolarisPolicyMappingRecord::new); + + this.slicePolicyMappingRecordsByPolicy = + new Slice<>( + policyMappingRecord -> + String.format( + "%d::%d::%d::%d", + policyMappingRecord.getPolicyCatalogId(), + policyMappingRecord.getPolicyId(), + policyMappingRecord.getTargetCatalogId(), + policyMappingRecord.getTargetId()), + PolarisPolicyMappingRecord::new); + // no transaction open yet this.diagnosticServices = diagnostics; this.tr = null; @@ -345,6 +373,8 @@ private void startWriteTransaction() { this.sliceGrantRecords.startWriteTransaction(); this.sliceGrantRecordsByGrantee.startWriteTransaction(); this.slicePrincipalSecrets.startWriteTransaction(); + this.slicePolicyMappingRecords.startWriteTransaction(); + this.slicePolicyMappingRecordsByPolicy.startWriteTransaction(); } /** Rollback transaction */ @@ -355,6 +385,8 @@ void rollback() { this.sliceGrantRecords.rollback(); this.sliceGrantRecordsByGrantee.rollback(); this.slicePrincipalSecrets.rollback(); + this.slicePolicyMappingRecords.rollback(); + this.slicePolicyMappingRecordsByPolicy.rollback(); } /** Ensure that a read/write FDB transaction has been started */ @@ -497,6 +529,14 @@ public Slice getSlicePrincipalSecrets() { return slicePrincipalSecrets; } + public Slice getSlicePolicyMappingRecords() { + return slicePolicyMappingRecords; + } + + public Slice getSlicePolicyMappingRecordsByPolicy() { + return slicePolicyMappingRecordsByPolicy; + } + /** * Next sequence number generator * @@ -516,5 +556,7 @@ void deleteAll() { this.sliceGrantRecordsByGrantee.deleteAll(); this.sliceGrantRecords.deleteAll(); this.slicePrincipalSecrets.deleteAll(); + this.slicePolicyMappingRecords.deleteAll(); + this.slicePolicyMappingRecordsByPolicy.deleteAll(); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 8d2e59c963..3a48215496 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -38,6 +38,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BaseMetaStoreManager; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; @@ -551,4 +552,86 @@ PolarisStorageIntegration loadPolarisStorageIntegrationInCurrentTxn( public void rollback() { this.store.rollback(); } + + /** {@inheritDoc} */ + @Override + public void writeToPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + this.store.getSlicePolicyMappingRecords().write(record); + this.store.getSlicePolicyMappingRecordsByPolicy().write(record); + } + + /** {@inheritDoc} */ + @Override + public void deleteFromPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + this.store.getSlicePolicyMappingRecords().delete(record); + this.store.getSlicePolicyMappingRecordsByPolicy().delete(record); + } + + /** {@inheritDoc} */ + @Override + public void deleteAllEntityPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore entity, + @Nonnull List mappingOnTarget, + @Nonnull List mappingOnPolicy) { + // build composite prefix key and delete policy mapping records on the indexed side of each + // mapping table + String prefix = this.store.buildPrefixKeyComposite(entity.getCatalogId(), entity.getId()); + this.store.getSlicePolicyMappingRecords().deleteRange(prefix); + this.store.getSlicePolicyMappingRecordsByPolicy().deleteRange(prefix); + + // also delete the other side. We need to delete these mapping one at a time versus doing a + // range delete + mappingOnTarget.forEach(record -> this.store.getSlicePolicyMappingRecords().delete(record)); + mappingOnPolicy.forEach( + record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); + } + + /** {@inheritDoc} */ + @Override + public @Nullable PolarisPolicyMappingRecord lookupPolicyMappingRecordInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode, + long policyCatalogId, + long policyId) { + return this.store + .getSlicePolicyMappingRecords() + .read( + this.store.buildKeyComposite( + targetCatalogId, targetId, policyTypeCode, policyCatalogId, policyId)); + } + + /** {@inheritDoc} */ + @Override + public @Nonnull List loadPoliciesOnTargetByTypeInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode) { + return this.store + .getSlicePolicyMappingRecords() + .readRange(this.store.buildPrefixKeyComposite(targetCatalogId, targetId, policyTypeCode)); + } + + /** {@inheritDoc} */ + @Override + public @Nonnull List loadAllPoliciesOnTargetInCurrentTxn( + @Nonnull PolarisCallContext callCtx, long targetCatalogId, long targetId) { + return this.store + .getSlicePolicyMappingRecords() + .readRange(this.store.buildPrefixKeyComposite(targetCatalogId, targetId)); + } + + /** {@inheritDoc} */ + @Override + public @Nonnull List loadAllPoliciesOnPolicyInCurrentTxn( + @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + return this.store + .getSlicePolicyMappingRecordsByPolicy() + .readRange(this.store.buildPrefixKeyComposite(policyCatalogId, policyId)); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java new file mode 100644 index 0000000000..fc4cb6a063 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java @@ -0,0 +1,211 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.policy; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.entity.PolarisEntityCore; +import org.apache.polaris.core.persistence.dao.entity.BaseResult; + +public interface PolarisPolicyMappingManager { + + /** + * Attach a policy to a target entity, for example attach a policy to a table. + * + *

For inheritable policy, only one policy of the same type can be attached to the target. For + * non-inheritable policy, multiple policies of the same type can be attached to the target. + * + * @param callCtx call context + * @param targetCatalogPath path to the target entity + * @param target target entity + * @param policyCatalogPath path to the policy entity + * @param policy policy entity + * @param parameters additional parameters for the attachment + * @return The policy mapping record we created for this attachment. Will return ENTITY_NOT_FOUND + * if the specified target or policy does not exist. Will return + * POLICY_OF_SAME_TYPE_ALREADY_ATTACHED if the target already has a policy of the same type + * attached and the policy is inheritable. + */ + @Nonnull + AttachmentResult attachPolicyToEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List targetCatalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy, + Map parameters); + + /** + * Detach a policy from a target entity + * + * @param callCtx call context + * @param catalogPath path to the target entity + * @param target target entity + * @param policyCatalogPath path to the policy entity + * @param policy policy entity + * @return The policy mapping record we detached. Will return ENTITY_NOT_FOUND if the specified + * target or policy does not exist. Will return POLICY_MAPPING_NOT_FOUND if the mapping cannot + * be found + */ + @Nonnull + AttachmentResult detachPolicyFromEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List catalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy); + + /** + * Load all policies attached to a target entity + * + * @param callCtx call context + * @param target target entity + * @return the list of policy mapping records on the target entity. Will return ENTITY_NOT_FOUND + * if the specified target does not exist. + */ + @Nonnull + LoadPolicyMappingsResult loadPoliciesOnEntity( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target); + + /** + * Load all policies of a specific type attached to a target entity + * + * @param callCtx call context + * @param target target entity + * @param policyType the type of policy + * @return the list of policy mapping records on the target entity. Will return ENTITY_NOT_FOUND + * if the specified target does not exist. + */ + @Nonnull + LoadPolicyMappingsResult loadPoliciesOnEntityByType( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyType policyType); + + /** result of an attach/detach operation */ + class AttachmentResult extends BaseResult { + // null if not success + private final PolarisPolicyMappingRecord mappingRecord; + + /** + * Constructor for an error + * + * @param errorCode error code, cannot be SUCCESS + * @param extraInformation extra information + */ + public AttachmentResult( + @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { + super(errorCode, extraInformation); + this.mappingRecord = null; + } + + /** + * Constructor for success + * + * @param mappingRecord policy mapping record being attached/detached + */ + public AttachmentResult(@Nonnull PolarisPolicyMappingRecord mappingRecord) { + super(BaseResult.ReturnStatus.SUCCESS); + this.mappingRecord = mappingRecord; + } + + @JsonCreator + private AttachmentResult( + @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, + @JsonProperty("extraInformation") String extraInformation, + @JsonProperty("policyMappingRecord") PolarisPolicyMappingRecord mappingRecord) { + super(returnStatus, extraInformation); + this.mappingRecord = mappingRecord; + } + + public PolarisPolicyMappingRecord getPolicyMappingRecord() { + return mappingRecord; + } + } + + /** result of a load policy mapping call */ + class LoadPolicyMappingsResult extends BaseResult { + // null if not success. Else set of policy mapping records on a target or from a policy + private final List mappingRecords; + + // null if not success. Else set of policy entities in the mapping records. + private final List policyEntities; + + /** + * Constructor for an error + * + * @param errorCode error code, cannot be SUCCESS + * @param extraInformation extra information + */ + public LoadPolicyMappingsResult( + @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { + super(errorCode, extraInformation); + this.mappingRecords = null; + this.policyEntities = null; + } + + /** + * Constructor for success + * + * @param mappingRecords policy mapping records + * @param policyEntities policy entities + */ + public LoadPolicyMappingsResult( + @Nonnull List mappingRecords, + @Nonnull List policyEntities) { + super(BaseResult.ReturnStatus.SUCCESS); + this.mappingRecords = mappingRecords; + this.policyEntities = policyEntities; + } + + @JsonCreator + private LoadPolicyMappingsResult( + @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, + @JsonProperty("extraInformation") String extraInformation, + @JsonProperty("policyMappingRecords") List mappingRecords, + @JsonProperty("policyEntities") List policyEntities) { + super(returnStatus, extraInformation); + this.mappingRecords = mappingRecords; + this.policyEntities = policyEntities; + } + + public List getPolicyMappingRecords() { + return mappingRecords; + } + + public List getPolicyEntities() { + return policyEntities; + } + + @JsonIgnore + public Map getPolicyEntitiesAsMap() { + return policyEntities == null + ? null + : policyEntities.stream() + .collect(Collectors.toMap(PolicyEntity::getId, entity -> entity)); + } + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingRecord.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingRecord.java new file mode 100644 index 0000000000..d4bf118b6b --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingRecord.java @@ -0,0 +1,215 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.policy; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +public class PolarisPolicyMappingRecord { + // to serialize/deserialize properties + public static final String EMPTY_MAP_STRING = "{}"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + // id of the catalog where target entity resides + private long targetCatalogId; + + // id of the target entity + private long targetId; + + // id of the catalog where the policy entity resides + private long policyCatalogId; + + // id of the policy + private long policyId; + + // id associated to the policy type + private int policyTypeCode; + + // additional parameters of the mapping + private String parameters; + + public PolarisPolicyMappingRecord() {} + + public long getTargetCatalogId() { + return targetCatalogId; + } + + public void setTargetCatalogId(long targetCatalogId) { + this.targetCatalogId = targetCatalogId; + } + + public long getTargetId() { + return targetId; + } + + public void setTargetId(long targetId) { + this.targetId = targetId; + } + + public long getPolicyId() { + return policyId; + } + + public void setPolicyId(long policyId) { + this.policyId = policyId; + } + + public int getPolicyTypeCode() { + return policyTypeCode; + } + + public void setPolicyTypeCode(int policyTypeCode) { + this.policyTypeCode = policyTypeCode; + } + + public long getPolicyCatalogId() { + return policyCatalogId; + } + + public void setPolicyCatalogId(long policyCatalogId) { + this.policyCatalogId = policyCatalogId; + } + + public String getParameters() { + return parameters; + } + + public void setParameters(String parameters) { + this.parameters = parameters; + } + + public Map getParametersAsMap() { + if (parameters == null) { + return new HashMap<>(); + } + try { + return MAPPER.readValue(parameters, new TypeReference<>() {}); + } catch (JsonProcessingException ex) { + throw new IllegalStateException( + String.format("Failed to deserialize json. parameters %s", parameters), ex); + } + } + + public void setParametersAsMap(Map parameters) { + try { + this.parameters = + parameters == null ? EMPTY_MAP_STRING : MAPPER.writeValueAsString(parameters); + } catch (JsonProcessingException ex) { + throw new IllegalStateException( + String.format("Failed to serialize json. properties %s", parameters), ex); + } + } + + /** + * Constructor + * + * @param targetCatalogId id of the catalog where target entity resides + * @param targetId id of the target entity + * @param policyCatalogId id of the catalog where the policy entity resides + * @param policyId id of the policy + * @param policyTypeCode id associated to the policy type + * @param parameters additional parameters of the mapping + */ + @JsonCreator + public PolarisPolicyMappingRecord( + @JsonProperty("targetCatalogId") long targetCatalogId, + @JsonProperty("targetId") long targetId, + @JsonProperty("policyCatalogId") long policyCatalogId, + @JsonProperty("policyId") long policyId, + @JsonProperty("policyTypeCode") int policyTypeCode, + @JsonProperty("parameters") String parameters) { + this.targetCatalogId = targetCatalogId; + this.targetId = targetId; + this.policyCatalogId = policyCatalogId; + this.policyId = policyId; + this.policyTypeCode = policyTypeCode; + this.parameters = parameters; + } + + public PolarisPolicyMappingRecord( + long targetCatalogId, + long targetId, + long policyCatalogId, + long policyId, + int policyTypeCode, + Map parameters) { + this.targetCatalogId = targetCatalogId; + this.targetId = targetId; + this.policyCatalogId = policyCatalogId; + this.policyId = policyId; + this.policyTypeCode = policyTypeCode; + this.setParametersAsMap(parameters); + } + + /** + * Copy constructor + * + * @param policyMappingRecord policy mapping rec to copy + */ + public PolarisPolicyMappingRecord(PolarisPolicyMappingRecord policyMappingRecord) { + this.targetCatalogId = policyMappingRecord.getTargetCatalogId(); + this.targetId = policyMappingRecord.getTargetId(); + this.policyCatalogId = policyMappingRecord.getPolicyCatalogId(); + this.policyId = policyMappingRecord.getPolicyId(); + this.policyTypeCode = policyMappingRecord.getPolicyTypeCode(); + this.parameters = policyMappingRecord.getParameters(); + } + + @Override + public String toString() { + return "PolarisPolicyMappingRec{" + + "targetCatalogId=" + + targetCatalogId + + ", targetId=" + + targetId + + ", policyCatalogId=" + + policyCatalogId + + ", policyId=" + + policyId + + ", policyType='" + + policyTypeCode + + ", parameters='" + + parameters + + "}"; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PolarisPolicyMappingRecord that = (PolarisPolicyMappingRecord) o; + return targetCatalogId == that.targetCatalogId + && targetId == that.targetId + && policyCatalogId == that.policyCatalogId + && policyId == that.policyId + && policyTypeCode == that.policyTypeCode + && Objects.equals(parameters, that.parameters); + } + + @Override + public int hashCode() { + return java.util.Objects.hash(targetId, policyId, policyCatalogId, policyTypeCode, parameters); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java new file mode 100644 index 0000000000..c6ca31213f --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.policy; + +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.util.List; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.entity.PolarisEntityCore; + +/** + * Interface for interacting with the Polaris persistence backend for Policy Mapping operations. + * This interface provides methods to persist and retrieve policy mapping records, which define the + * relationships between policies and target entities in Polaris. + * + *

Note that APIs to the actual persistence store are very basic, often point read or write to + * the underlying data store. The goal is to make it really easy to back this using databases like + * Postgres or simpler KV store. + */ +public interface PolicyMappingPersistence { + + /** + * Write the specified policyMappingRecord to the policy_mapping_records table. If there is a + * conflict (existing record with the same PK), all attributes of the new record will replace the + * existing one. + * + * @param callCtx call context + * @param record policy mapping record to write, potentially replacing an existing policy mapping + * with the same key + */ + default void writeToPolicyMappingRecords( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** + * Delete the specified policyMappingRecord to the policy_mapping_records table. + * + * @param callCtx call context + * @param record policy mapping record to delete. + */ + default void deleteFromPolicyMappingRecords( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** + * Delete the all policy mapping records in the policy_mapping_records table for the specified + * entity. This method will delete all policy mapping records on the entity + * + * @param callCtx call context + * @param entity entity whose policy mapping records should be deleted + * @param mappingOnTarget all mappings on that target entity. Empty list if that entity is not a + * target + * @param mappingOnPolicy all mappings on that policy entity. Empty list if that entity is not a + * policy + */ + default void deleteAllEntityPolicyMappingRecords( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore entity, + @Nonnull List mappingOnTarget, + @Nonnull List mappingOnPolicy) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** + * Look up the specified policy mapping record from the policy_mapping_records table. Return NULL + * if not found + * + * @param callCtx call context + * @param targetCatalogId catalog id of the target entity, NULL_ID if the entity is top-level + * @param targetId id of the target entity + * @param policyTypeCode type code of the policy entity + * @param policyCatalogId catalog id of the policy entity + * @param policyId id of the policy entity + * @return the policy mapping record if found, NULL if not found + */ + @Nullable + default PolarisPolicyMappingRecord lookupPolicyMappingRecord( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode, + long policyCatalogId, + long policyId) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** + * Get all policies on the specified target entity with specified policy type. + * + * @param callCtx call context + * @param targetCatalogId catalog id of the target entity, NULL_ID if the entity is top-level + * @param targetId id of the target entity + * @param policyTypeCode type code of the policy entity + * @return the list of policy mapping records for the specified target entity with the specified + * policy type + */ + @Nonnull + default List loadPoliciesOnTargetByType( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** + * Get all policies on the specified target entity. + * + * @param callCtx call context + * @param targetCatalogId catalog id of the target entity, NULL_ID if the entity is top-level + * @param targetId id of the target entity + * @return the list of policy mapping records for the specified target entity + */ + @Nonnull + default List loadAllPoliciesOnTarget( + @Nonnull PolarisCallContext callCtx, long targetCatalogId, long targetId) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** + * Get all targets of the specified policy entity + * + * @param callCtx call context + * @param policyCatalogId catalog id of the policy entity, NULL_ID if the entity is top-level + * @param policyId id of the policy entity + * @return the list of policy mapping records for the specified policy entity + */ + @Nonnull + default List loadAllPoliciesOnPolicy( + @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + throw new UnsupportedOperationException("Not Implemented"); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java new file mode 100644 index 0000000000..f5c1c99541 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.policy; + +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.util.List; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.entity.PolarisEntityCore; + +public interface TransactionalPolicyMappingPersistence extends PolicyMappingPersistence { + /** See {@link PolicyMappingPersistence#writeToPolicyMappingRecords} */ + default void writeToPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** See {@link PolicyMappingPersistence#deleteFromPolicyMappingRecords} */ + default void deleteFromPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** See {@link PolicyMappingPersistence#deleteAllEntityPolicyMappingRecords} */ + default void deleteAllEntityPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore entity, + @Nonnull List mappingOnTarget, + @Nonnull List mappingOnPolicy) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** See {@link PolicyMappingPersistence#lookupPolicyMappingRecord} */ + @Nullable + default PolarisPolicyMappingRecord lookupPolicyMappingRecordInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode, + long policyCatalogId, + long policyId) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** See {@link PolicyMappingPersistence#loadPoliciesOnTargetByType} */ + @Nonnull + default List loadPoliciesOnTargetByTypeInCurrentTxn( + @Nonnull PolarisCallContext callCtx, + long targetCatalogId, + long targetId, + int policyTypeCode) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** See {@link PolicyMappingPersistence#loadAllPoliciesOnTarget} */ + @Nonnull + default List loadAllPoliciesOnTargetInCurrentTxn( + @Nonnull PolarisCallContext callCtx, long targetCatalogId, long targetId) { + throw new UnsupportedOperationException("Not Implemented"); + } + + /** See {@link PolicyMappingPersistence#loadAllPoliciesOnPolicy} */ + @Nonnull + default List loadAllPoliciesOnPolicyInCurrentTxn( + @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { + throw new UnsupportedOperationException("Not Implemented"); + } +} diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java index 81f233825b..0f9824fcd0 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java @@ -280,6 +280,12 @@ void testEntityCache() { polarisTestMetaStoreManager.testEntityCache(); } + /** Test that attaching/detaching policies works well */ + @Test + void testPolicyMapping() { + polarisTestMetaStoreManager.testPolicyMapping(); + } + @Test void testLoadTasks() { for (int i = 0; i < 20; i++) { diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 591c657d13..2627c05e33 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -49,6 +49,8 @@ import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; +import org.apache.polaris.core.policy.PolarisPolicyMappingManager; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.policy.PredefinedPolicyTypes; @@ -956,6 +958,196 @@ void revokeToGrantee( this.ensureGrantRecordRemoved(granted, grantee, priv); } + /** attach a policy to a target */ + void attachPolicyToTarget( + List targetCatalogPath, + PolarisBaseEntity target, + List policyCatalogPath, + PolicyEntity policy) { + polarisMetaStoreManager.attachPolicyToEntity( + polarisCallContext, targetCatalogPath, target, policyCatalogPath, policy, null); + ensurePolicyMappingRecordExists(target, policy); + } + + /** detach a policy from a target */ + void detachPolicyFromTarget( + List targetCatalogPath, + PolarisBaseEntity target, + List policyCatalogPath, + PolicyEntity policy) { + polarisMetaStoreManager.detachPolicyFromEntity( + polarisCallContext, targetCatalogPath, target, policyCatalogPath, policy); + ensurePolicyMappingRecordRemoved(target, policy); + } + + /** + * Ensure that the specified policy mapping record exists + * + * @param target the target + * @param policy the policy + */ + void ensurePolicyMappingRecordExists(PolarisBaseEntity target, PolicyEntity policy) { + target = + polarisMetaStoreManager + .loadEntity( + this.polarisCallContext, target.getCatalogId(), target.getId(), target.getType()) + .getEntity(); + Assertions.assertThat(target).isNotNull(); + + policy = + PolicyEntity.of( + polarisMetaStoreManager + .loadEntity( + this.polarisCallContext, + policy.getCatalogId(), + policy.getId(), + PolarisEntityType.POLICY) + .getEntity()); + Assertions.assertThat(policy).isNotNull(); + + PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResult = + polarisMetaStoreManager.loadPoliciesOnEntity(this.polarisCallContext, target); + + validateLoadedPolicyMappings(loadPolicyMappingsResult); + + checkPolicyMappingRecordExists( + loadPolicyMappingsResult.getPolicyMappingRecords(), target, policy); + + // also try load by specific type + PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResultByType = + polarisMetaStoreManager.loadPoliciesOnEntityByType( + this.polarisCallContext, target, policy.getPolicyType()); + validateLoadedPolicyMappings(loadPolicyMappingsResultByType); + checkPolicyMappingRecordExists( + loadPolicyMappingsResultByType.getPolicyMappingRecords(), target, policy); + } + + /** + * Ensure that the specified policy mapping record has been removed + * + * @param target the target + * @param policy the policy + */ + void ensurePolicyMappingRecordRemoved(PolarisBaseEntity target, PolicyEntity policy) { + target = + polarisMetaStoreManager + .loadEntity( + this.polarisCallContext, target.getCatalogId(), target.getId(), target.getType()) + .getEntity(); + Assertions.assertThat(target).isNotNull(); + + policy = + PolicyEntity.of( + polarisMetaStoreManager + .loadEntity( + this.polarisCallContext, + policy.getCatalogId(), + policy.getId(), + PolarisEntityType.POLICY) + .getEntity()); + Assertions.assertThat(policy).isNotNull(); + + PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResult = + polarisMetaStoreManager.loadPoliciesOnEntity(this.polarisCallContext, target); + + validateLoadedPolicyMappings(loadPolicyMappingsResult); + + checkPolicyMappingRecordRemoved( + loadPolicyMappingsResult.getPolicyMappingRecords(), target, policy); + + // also try load by specific type + PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResultByType = + polarisMetaStoreManager.loadPoliciesOnEntityByType( + this.polarisCallContext, target, policy.getPolicyType()); + validateLoadedPolicyMappings(loadPolicyMappingsResultByType); + checkPolicyMappingRecordRemoved( + loadPolicyMappingsResultByType.getPolicyMappingRecords(), target, policy); + } + + /** + * Validate the return of loadPoliciesOnEntity() or LoadPoliciesOnEntityByType() + * + * @param loadPolicyMappingRecords return from calling + * loadPoliciesOnEntity()/LoadPoliciesOnEntityByType() + */ + void validateLoadedPolicyMappings( + PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingRecords) { + Assertions.assertThat(loadPolicyMappingRecords).isNotNull(); + + Map policyEntities = loadPolicyMappingRecords.getPolicyEntitiesAsMap(); + Assertions.assertThat(policyEntities).isNotNull(); + + for (PolarisPolicyMappingRecord policyMappingRecord : + loadPolicyMappingRecords.getPolicyMappingRecords()) { + PolicyEntity entity = + PolicyEntity.of( + polarisMetaStoreManager + .loadEntity( + this.polarisCallContext, + policyMappingRecord.getPolicyCatalogId(), + policyMappingRecord.getPolicyId(), + PolarisEntityType.POLICY) + .getEntity()); + + Assertions.assertThat(entity).isNotNull(); + Assertions.assertThat(policyEntities.get(entity.getId())).isEqualTo(entity); + } + } + + /** + * Check that the policy mapping record exists + * + * @param policyMappingRecords list of policy mapping records + * @param target the target + * @param policy the policy + */ + void checkPolicyMappingRecordExists( + List policyMappingRecords, + PolarisBaseEntity target, + PolicyEntity policy) { + boolean exists = isPolicyMappingRecordExists(policyMappingRecords, target, policy); + Assertions.assertThat(exists).isTrue(); + } + + /** + * Check that the policy mapping record has been removed + * + * @param policyMappingRecords list of policy mapping records + * @param target the target + * @param policy the policy + */ + void checkPolicyMappingRecordRemoved( + List policyMappingRecords, + PolarisBaseEntity target, + PolicyEntity policy) { + boolean exists = isPolicyMappingRecordExists(policyMappingRecords, target, policy); + Assertions.assertThat(exists).isFalse(); + } + + /** + * Check if the policy mapping record exists + * + * @param policyMappingRecords list of policy mapping records + * @param target the target + * @param policy the policy + */ + boolean isPolicyMappingRecordExists( + List policyMappingRecords, + PolarisBaseEntity target, + PolicyEntity policy) { + long policyMappingCount = + policyMappingRecords.stream() + .filter( + record -> + record.getPolicyCatalogId() == policy.getCatalogId() + && record.getPolicyId() == policy.getId() + && record.getTargetCatalogId() == target.getCatalogId() + && record.getTargetId() == target.getId() + && record.getPolicyTypeCode() == policy.getPolicyTypeCode()) + .count(); + return policyMappingCount == 1; + } + /** * Create a test catalog. This is a new catalog which will have the following objects (N is for a * namespace, T for a table, V for a view, R for a role, P for a principal, POL for a policy): @@ -2443,4 +2635,51 @@ public void testEntityCache() { this.refreshCacheEntry( 1, 1, PolarisEntityType.TABLE_LIKE, N1.getCatalogId() + 1000, N1.getId(), false); } + + void testPolicyMapping() { + PolarisBaseEntity catalog = this.createTestCatalog("test"); + Assertions.assertThat(catalog).isNotNull(); + + PolarisBaseEntity N1 = + this.ensureExistsByName(List.of(catalog), PolarisEntityType.NAMESPACE, "N1"); + PolarisBaseEntity N1_N2 = + this.ensureExistsByName(List.of(catalog, N1), PolarisEntityType.NAMESPACE, "N2"); + PolarisBaseEntity N5 = + this.ensureExistsByName(List.of(catalog), PolarisEntityType.NAMESPACE, "N5"); + + PolarisBaseEntity N1_N2_T1 = + this.ensureExistsByName( + List.of(catalog, N1, N1_N2), + PolarisEntityType.TABLE_LIKE, + PolarisEntitySubType.ANY_SUBTYPE, + "T1"); + + PolicyEntity N1_P1 = + this.createPolicy(List.of(catalog, N1), "P1", PredefinedPolicyTypes.DATA_COMPACTION); + + PolicyEntity N1_P2 = + this.createPolicy(List.of(catalog, N1), "P2", PredefinedPolicyTypes.DATA_COMPACTION); + + PolicyEntity N5_P3 = + this.createPolicy(List.of(catalog, N5), "P3", PredefinedPolicyTypes.METADATA_COMPACTION); + attachPolicyToTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N1), N1_P1); + // attach a different policy of different inheritable type to the same target, should succeed + attachPolicyToTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N5), N5_P3); + + // attach a different policy of same inheritable type to the same target, should fail + PolarisPolicyMappingManager.AttachmentResult attachmentResult = + polarisMetaStoreManager.attachPolicyToEntity( + polarisCallContext, + List.of(catalog, N1, N1_N2), + N1_N2_T1, + List.of(catalog, N1), + N1_P2, + null); + + Assertions.assertThat(attachmentResult.isSuccess()).isFalse(); + Assertions.assertThat(attachmentResult.getReturnStatus()) + .isEqualTo(BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS); + detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N1), N1_P1); + detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N5), N5_P3); + } } From c8f8a88054804364717f3e67166af05c0e0e1ec4 Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 14 Mar 2025 16:44:32 -0700 Subject: [PATCH 02/15] Remove @NotNull from jetbrains --- .../AtomicOperationMetaStoreManager.java | 37 +++++++++---------- .../TransactionWorkspaceMetaStoreManager.java | 37 +++++++++---------- .../TransactionalMetaStoreManagerImpl.java | 7 ++-- 3 files changed, 39 insertions(+), 42 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index fcbd1cc4fa..6c9df6443a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -68,7 +68,6 @@ import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; -import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -1827,12 +1826,12 @@ public Map getInternalPropertyMap( } @Override - public @NotNull AttachmentResult attachPolicyToEntity( - @NotNull PolarisCallContext callCtx, - @NotNull List targetCatalogPath, - @NotNull PolarisEntityCore target, - @NotNull List policyCatalogPath, - @NotNull PolicyEntity policy, + public @Nonnull AttachmentResult attachPolicyToEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List targetCatalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy, Map parameters) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -1869,12 +1868,12 @@ public Map getInternalPropertyMap( } @Override - public @NotNull AttachmentResult detachPolicyFromEntity( - @NotNull PolarisCallContext callCtx, - @NotNull List catalogPath, - @NotNull PolarisEntityCore target, - @NotNull List policyCatalogPath, - @NotNull PolicyEntity policy) { + public @Nonnull AttachmentResult detachPolicyFromEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List catalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -1896,8 +1895,8 @@ public Map getInternalPropertyMap( } @Override - public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntity( - @NotNull PolarisCallContext callCtx, @NotNull PolarisEntityCore target) { + public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntity( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -1917,10 +1916,10 @@ public Map getInternalPropertyMap( } @Override - public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntityByType( - @NotNull PolarisCallContext callCtx, - @NotNull PolarisEntityCore target, - @NotNull PolicyType policyType) { + public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntityByType( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyType policyType) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 2d8d7cdbd9..63c7dbb8fa 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -52,7 +52,6 @@ import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisStorageActions; -import org.jetbrains.annotations.NotNull; /** * Wraps an existing impl of PolarisMetaStoreManager and delegates expected "read" operations @@ -399,12 +398,12 @@ public ResolvedEntityResult refreshResolvedEntity( } @Override - public @NotNull AttachmentResult attachPolicyToEntity( - @NotNull PolarisCallContext callCtx, - @NotNull List targetCatalogPath, - @NotNull PolarisEntityCore target, - @NotNull List policyCatalogPath, - @NotNull PolicyEntity policy, + public @Nonnull AttachmentResult attachPolicyToEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List targetCatalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy, Map parameters) { callCtx .getDiagServices() @@ -413,12 +412,12 @@ public ResolvedEntityResult refreshResolvedEntity( } @Override - public @NotNull AttachmentResult detachPolicyFromEntity( - @NotNull PolarisCallContext callCtx, - @NotNull List catalogPath, - @NotNull PolarisEntityCore target, - @NotNull List policyCatalogPath, - @NotNull PolicyEntity policy) { + public @Nonnull AttachmentResult detachPolicyFromEntity( + @Nonnull PolarisCallContext callCtx, + @Nonnull List catalogPath, + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy) { callCtx .getDiagServices() .fail("illegal_method_in_transaction_workspace", "detachPolicyFromEntity"); @@ -426,8 +425,8 @@ public ResolvedEntityResult refreshResolvedEntity( } @Override - public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntity( - @NotNull PolarisCallContext callCtx, @NotNull PolarisEntityCore target) { + public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntity( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target) { callCtx .getDiagServices() .fail("illegal_method_in_transaction_workspace", "loadPoliciesOnEntity"); @@ -435,10 +434,10 @@ public ResolvedEntityResult refreshResolvedEntity( } @Override - public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntityByType( - @NotNull PolarisCallContext callCtx, - @NotNull PolarisEntityCore target, - @NotNull PolicyType policyType) { + public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntityByType( + @Nonnull PolarisCallContext callCtx, + @Nonnull PolarisEntityCore target, + @Nonnull PolicyType policyType) { callCtx .getDiagServices() .fail("illegal_method_in_transaction_workspace", "loadPoliciesOnEntityByType"); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 9836dae57c..06048fda77 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -69,7 +69,6 @@ import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; -import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -2323,7 +2322,7 @@ public Map getInternalPropertyMap( /** {@inheritDoc} */ @Override - public @NotNull AttachmentResult attachPolicyToEntity( + public @Nonnull AttachmentResult attachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List targetCatalogPath, @Nonnull PolarisEntityCore target, @@ -2444,7 +2443,7 @@ private AttachmentResult doDetachPolicyFromEntity( /** {@inheritDoc} */ @Override - public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntity( + public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntity( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target) { TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); return ms.runInReadTransaction(callCtx, () -> this.doLoadPoliciesOnEntity(callCtx, ms, target)); @@ -2473,7 +2472,7 @@ private LoadPolicyMappingsResult doLoadPoliciesOnEntity( /** {@inheritDoc} */ @Override - public @NotNull LoadPolicyMappingsResult loadPoliciesOnEntityByType( + public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntityByType( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target, @Nonnull PolicyType policyType) { From f70f6cc949651dbeef72c1939f76d4b97136e2be Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 18 Mar 2025 13:43:09 -0700 Subject: [PATCH 03/15] reorder method signatures --- .../TransactionalMetaStoreManagerImpl.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 06048fda77..9099f91a86 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -2336,7 +2336,7 @@ public Map getInternalPropertyMap( callCtx, () -> this.doAttachPolicyToEntity( - callCtx, ms, target, targetCatalogPath, policy, policyCatalogPath, parameters)); + callCtx, ms, targetCatalogPath, target, policyCatalogPath, policy, parameters)); } /** @@ -2346,10 +2346,10 @@ public Map getInternalPropertyMap( private @Nonnull AttachmentResult doAttachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, - @Nonnull PolarisEntityCore target, @Nonnull List targetCatalogPath, - @Nonnull PolicyEntity policy, + @Nonnull PolarisEntityCore target, @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy, Map parameters) { PolarisEntityResolver targetResolver = new PolarisEntityResolver(callCtx, ms, targetCatalogPath, target); @@ -2402,7 +2402,7 @@ public Map getInternalPropertyMap( callCtx, () -> this.doDetachPolicyFromEntity( - callCtx, ms, target, targetCatalogPath, policy, policyCatalogPath)); + callCtx, ms, targetCatalogPath, target, policyCatalogPath, policy)); } /** @@ -2412,10 +2412,10 @@ public Map getInternalPropertyMap( private AttachmentResult doDetachPolicyFromEntity( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, - @Nonnull PolarisEntityCore target, @Nonnull List targetCatalogPath, - @Nonnull PolicyEntity policy, - @Nonnull List policyCatalogPath) { + @Nonnull PolarisEntityCore target, + @Nonnull List policyCatalogPath, + @Nonnull PolicyEntity policy) { PolarisEntityResolver targetResolver = new PolarisEntityResolver(callCtx, ms, targetCatalogPath, target); PolarisEntityResolver policyResolver = From cd340662600c509f671f04038b593c6772c827f3 Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 20 Mar 2025 08:35:05 -0700 Subject: [PATCH 04/15] Fix CI --- .../polaris/core/persistence/PolarisTestMetaStoreManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 2627c05e33..e36c33ca0c 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -2650,7 +2650,7 @@ void testPolicyMapping() { PolarisBaseEntity N1_N2_T1 = this.ensureExistsByName( List.of(catalog, N1, N1_N2), - PolarisEntityType.TABLE_LIKE, + PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, "T1"); From 6ef1d9724aa887fcd137b6121d548d9eadd97f3e Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 11:57:50 -0700 Subject: [PATCH 05/15] Move result to upper-level class --- .../AtomicOperationMetaStoreManager.java | 18 +-- .../TransactionWorkspaceMetaStoreManager.java | 6 +- .../dao/entity/LoadPolicyMappingsResult.java | 92 ++++++++++++++ .../dao/entity/PolicyAttachmentResult.java | 66 ++++++++++ .../TransactionalMetaStoreManagerImpl.java | 26 ++-- .../policy/PolarisPolicyMappingManager.java | 116 +----------------- .../PolarisTestMetaStoreManager.java | 20 +-- 7 files changed, 200 insertions(+), 144 deletions(-) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 6c9df6443a..e9de41a270 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -56,6 +56,8 @@ import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; +import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; @@ -1826,7 +1828,7 @@ public Map getInternalPropertyMap( } @Override - public @Nonnull AttachmentResult attachPolicyToEntity( + public @Nonnull PolicyAttachmentResult attachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List targetCatalogPath, @Nonnull PolarisEntityCore target, @@ -1839,7 +1841,7 @@ public Map getInternalPropertyMap( PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode()); if (policyType == null) { // This should never happen - return new AttachmentResult( + return new PolicyAttachmentResult( BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); } @@ -1850,13 +1852,13 @@ public Map getInternalPropertyMap( ms.loadPoliciesOnTargetByType( callCtx, target.getCatalogId(), target.getId(), policy.getPolicyTypeCode()); if (existingRecordsOfSameType.size() > 1) { - return new AttachmentResult( + return new PolicyAttachmentResult( BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); } else if (existingRecordsOfSameType.size() == 1) { PolarisPolicyMappingRecord existingRecord = existingRecordsOfSameType.get(0); if (existingRecord.getPolicyId() != policy.getId() || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) { - return new AttachmentResult( + return new PolicyAttachmentResult( BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); } } @@ -1864,11 +1866,11 @@ public Map getInternalPropertyMap( PolarisPolicyMappingRecord mappingRecord = this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); - return new AttachmentResult(mappingRecord); + return new PolicyAttachmentResult(mappingRecord); } @Override - public @Nonnull AttachmentResult detachPolicyFromEntity( + public @Nonnull PolicyAttachmentResult detachPolicyFromEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List catalogPath, @Nonnull PolarisEntityCore target, @@ -1886,12 +1888,12 @@ public Map getInternalPropertyMap( policy.getCatalogId(), policy.getId()); if (mappingRecord == null) { - return new AttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null); + return new PolicyAttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null); } ms.deleteFromPolicyMappingRecords(callCtx, mappingRecord); - return new AttachmentResult(mappingRecord); + return new PolicyAttachmentResult(mappingRecord); } @Override diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 63c7dbb8fa..4c3778e980 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -44,6 +44,8 @@ import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; +import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; @@ -398,7 +400,7 @@ public ResolvedEntityResult refreshResolvedEntity( } @Override - public @Nonnull AttachmentResult attachPolicyToEntity( + public @Nonnull PolicyAttachmentResult attachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List targetCatalogPath, @Nonnull PolarisEntityCore target, @@ -412,7 +414,7 @@ public ResolvedEntityResult refreshResolvedEntity( } @Override - public @Nonnull AttachmentResult detachPolicyFromEntity( + public @Nonnull PolicyAttachmentResult detachPolicyFromEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List catalogPath, @Nonnull PolarisEntityCore target, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java new file mode 100644 index 0000000000..906b6727d8 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence.dao.entity; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyEntity; + +/** result of a load policy mapping call */ +public class LoadPolicyMappingsResult extends BaseResult { + // null if not success. Else set of policy mapping records on a target or from a policy + private final List mappingRecords; + + // null if not success. Else set of policy entities in the mapping records. + private final List policyEntities; + + /** + * Constructor for an error + * + * @param errorCode error code, cannot be SUCCESS + * @param extraInformation extra information + */ + public LoadPolicyMappingsResult( + @Nonnull ReturnStatus errorCode, @Nullable String extraInformation) { + super(errorCode, extraInformation); + this.mappingRecords = null; + this.policyEntities = null; + } + + /** + * Constructor for success + * + * @param mappingRecords policy mapping records + * @param policyEntities policy entities + */ + public LoadPolicyMappingsResult( + @Nonnull List mappingRecords, + @Nonnull List policyEntities) { + super(ReturnStatus.SUCCESS); + this.mappingRecords = mappingRecords; + this.policyEntities = policyEntities; + } + + @JsonCreator + private LoadPolicyMappingsResult( + @JsonProperty("returnStatus") @Nonnull ReturnStatus returnStatus, + @JsonProperty("extraInformation") String extraInformation, + @JsonProperty("policyMappingRecords") List mappingRecords, + @JsonProperty("policyEntities") List policyEntities) { + super(returnStatus, extraInformation); + this.mappingRecords = mappingRecords; + this.policyEntities = policyEntities; + } + + public List getPolicyMappingRecords() { + return mappingRecords; + } + + public List getPolicyEntities() { + return policyEntities; + } + + @JsonIgnore + public Map getPolicyEntitiesAsMap() { + return policyEntities == null + ? null + : policyEntities.stream().collect(Collectors.toMap(PolicyEntity::getId, entity -> entity)); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java new file mode 100644 index 0000000000..f95c9884ce --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence.dao.entity; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; + +/** result of an attach/detach operation */ +public class PolicyAttachmentResult extends BaseResult { + // null if not success + private final PolarisPolicyMappingRecord mappingRecord; + + /** + * Constructor for an error + * + * @param errorCode error code, cannot be SUCCESS + * @param extraInformation extra information + */ + public PolicyAttachmentResult( + @Nonnull ReturnStatus errorCode, @Nullable String extraInformation) { + super(errorCode, extraInformation); + this.mappingRecord = null; + } + + /** + * Constructor for success + * + * @param mappingRecord policy mapping record being attached/detached + */ + public PolicyAttachmentResult(@Nonnull PolarisPolicyMappingRecord mappingRecord) { + super(ReturnStatus.SUCCESS); + this.mappingRecord = mappingRecord; + } + + @JsonCreator + private PolicyAttachmentResult( + @JsonProperty("returnStatus") @Nonnull ReturnStatus returnStatus, + @JsonProperty("extraInformation") String extraInformation, + @JsonProperty("policyMappingRecord") PolarisPolicyMappingRecord mappingRecord) { + super(returnStatus, extraInformation); + this.mappingRecord = mappingRecord; + } + + public PolarisPolicyMappingRecord getPolicyMappingRecord() { + return mappingRecord; + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 9099f91a86..0f7aa654d0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -57,6 +57,8 @@ import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; +import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; @@ -2322,7 +2324,7 @@ public Map getInternalPropertyMap( /** {@inheritDoc} */ @Override - public @Nonnull AttachmentResult attachPolicyToEntity( + public @Nonnull PolicyAttachmentResult attachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List targetCatalogPath, @Nonnull PolarisEntityCore target, @@ -2343,7 +2345,7 @@ public Map getInternalPropertyMap( * See {@link #attachPolicyToEntity(PolarisCallContext, List, PolarisEntityCore, List, * PolicyEntity, Map)} */ - private @Nonnull AttachmentResult doAttachPolicyToEntity( + private @Nonnull PolicyAttachmentResult doAttachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nonnull List targetCatalogPath, @@ -2356,13 +2358,13 @@ public Map getInternalPropertyMap( PolarisEntityResolver policyResolver = new PolarisEntityResolver(callCtx, ms, policyCatalogPath, policy); if (targetResolver.isFailure() || policyResolver.isFailure()) { - return new AttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); + return new PolicyAttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); } PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode()); if (policyType == null) { // This should never happen - return new AttachmentResult( + return new PolicyAttachmentResult( BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); } @@ -2372,13 +2374,13 @@ public Map getInternalPropertyMap( ms.loadPoliciesOnTargetByTypeInCurrentTxn( callCtx, target.getCatalogId(), target.getId(), policy.getPolicyTypeCode()); if (existingRecordsOfSameType.size() > 1) { - return new AttachmentResult( + return new PolicyAttachmentResult( BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); } else if (existingRecordsOfSameType.size() == 1) { PolarisPolicyMappingRecord existingRecord = existingRecordsOfSameType.get(0); if (existingRecord.getPolicyId() != policy.getId() || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) { - return new AttachmentResult( + return new PolicyAttachmentResult( BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); } } @@ -2386,12 +2388,12 @@ public Map getInternalPropertyMap( PolarisPolicyMappingRecord mappingRecord = this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); - return new AttachmentResult(mappingRecord); + return new PolicyAttachmentResult(mappingRecord); } /** {@inheritDoc} */ @Override - public @Nonnull AttachmentResult detachPolicyFromEntity( + public @Nonnull PolicyAttachmentResult detachPolicyFromEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List targetCatalogPath, @Nonnull PolarisEntityCore target, @@ -2409,7 +2411,7 @@ public Map getInternalPropertyMap( * See {@link #detachPolicyFromEntity(PolarisCallContext, List, PolarisEntityCore, * List,PolicyEntity)} */ - private AttachmentResult doDetachPolicyFromEntity( + private PolicyAttachmentResult doDetachPolicyFromEntity( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nonnull List targetCatalogPath, @@ -2421,7 +2423,7 @@ private AttachmentResult doDetachPolicyFromEntity( PolarisEntityResolver policyResolver = new PolarisEntityResolver(callCtx, ms, policyCatalogPath, policy); if (targetResolver.isFailure() || policyResolver.isFailure()) { - return new AttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); + return new PolicyAttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); } PolarisPolicyMappingRecord mappingRecord = @@ -2433,12 +2435,12 @@ private AttachmentResult doDetachPolicyFromEntity( policy.getCatalogId(), policy.getId()); if (mappingRecord == null) { - return new AttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null); + return new PolicyAttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null); } ms.deleteFromPolicyMappingRecordsInCurrentTxn(callCtx, mappingRecord); - return new AttachmentResult(mappingRecord); + return new PolicyAttachmentResult(mappingRecord); } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java index fc4cb6a063..266d5477ea 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingManager.java @@ -18,17 +18,13 @@ */ package org.apache.polaris.core.policy; -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.annotation.Nonnull; -import jakarta.annotation.Nullable; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.entity.PolarisEntityCore; -import org.apache.polaris.core.persistence.dao.entity.BaseResult; +import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; +import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; public interface PolarisPolicyMappingManager { @@ -50,7 +46,7 @@ public interface PolarisPolicyMappingManager { * attached and the policy is inheritable. */ @Nonnull - AttachmentResult attachPolicyToEntity( + PolicyAttachmentResult attachPolicyToEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List targetCatalogPath, @Nonnull PolarisEntityCore target, @@ -71,7 +67,7 @@ AttachmentResult attachPolicyToEntity( * be found */ @Nonnull - AttachmentResult detachPolicyFromEntity( + PolicyAttachmentResult detachPolicyFromEntity( @Nonnull PolarisCallContext callCtx, @Nonnull List catalogPath, @Nonnull PolarisEntityCore target, @@ -104,108 +100,4 @@ LoadPolicyMappingsResult loadPoliciesOnEntityByType( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target, @Nonnull PolicyType policyType); - - /** result of an attach/detach operation */ - class AttachmentResult extends BaseResult { - // null if not success - private final PolarisPolicyMappingRecord mappingRecord; - - /** - * Constructor for an error - * - * @param errorCode error code, cannot be SUCCESS - * @param extraInformation extra information - */ - public AttachmentResult( - @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { - super(errorCode, extraInformation); - this.mappingRecord = null; - } - - /** - * Constructor for success - * - * @param mappingRecord policy mapping record being attached/detached - */ - public AttachmentResult(@Nonnull PolarisPolicyMappingRecord mappingRecord) { - super(BaseResult.ReturnStatus.SUCCESS); - this.mappingRecord = mappingRecord; - } - - @JsonCreator - private AttachmentResult( - @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, - @JsonProperty("extraInformation") String extraInformation, - @JsonProperty("policyMappingRecord") PolarisPolicyMappingRecord mappingRecord) { - super(returnStatus, extraInformation); - this.mappingRecord = mappingRecord; - } - - public PolarisPolicyMappingRecord getPolicyMappingRecord() { - return mappingRecord; - } - } - - /** result of a load policy mapping call */ - class LoadPolicyMappingsResult extends BaseResult { - // null if not success. Else set of policy mapping records on a target or from a policy - private final List mappingRecords; - - // null if not success. Else set of policy entities in the mapping records. - private final List policyEntities; - - /** - * Constructor for an error - * - * @param errorCode error code, cannot be SUCCESS - * @param extraInformation extra information - */ - public LoadPolicyMappingsResult( - @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { - super(errorCode, extraInformation); - this.mappingRecords = null; - this.policyEntities = null; - } - - /** - * Constructor for success - * - * @param mappingRecords policy mapping records - * @param policyEntities policy entities - */ - public LoadPolicyMappingsResult( - @Nonnull List mappingRecords, - @Nonnull List policyEntities) { - super(BaseResult.ReturnStatus.SUCCESS); - this.mappingRecords = mappingRecords; - this.policyEntities = policyEntities; - } - - @JsonCreator - private LoadPolicyMappingsResult( - @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, - @JsonProperty("extraInformation") String extraInformation, - @JsonProperty("policyMappingRecords") List mappingRecords, - @JsonProperty("policyEntities") List policyEntities) { - super(returnStatus, extraInformation); - this.mappingRecords = mappingRecords; - this.policyEntities = policyEntities; - } - - public List getPolicyMappingRecords() { - return mappingRecords; - } - - public List getPolicyEntities() { - return policyEntities; - } - - @JsonIgnore - public Map getPolicyEntitiesAsMap() { - return policyEntities == null - ? null - : policyEntities.stream() - .collect(Collectors.toMap(PolicyEntity::getId, entity -> entity)); - } - } } diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index e36c33ca0c..2261483c68 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -48,8 +48,9 @@ import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; +import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; -import org.apache.polaris.core.policy.PolarisPolicyMappingManager; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; @@ -1005,7 +1006,7 @@ void ensurePolicyMappingRecordExists(PolarisBaseEntity target, PolicyEntity poli .getEntity()); Assertions.assertThat(policy).isNotNull(); - PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResult = + LoadPolicyMappingsResult loadPolicyMappingsResult = polarisMetaStoreManager.loadPoliciesOnEntity(this.polarisCallContext, target); validateLoadedPolicyMappings(loadPolicyMappingsResult); @@ -1014,7 +1015,7 @@ void ensurePolicyMappingRecordExists(PolarisBaseEntity target, PolicyEntity poli loadPolicyMappingsResult.getPolicyMappingRecords(), target, policy); // also try load by specific type - PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResultByType = + LoadPolicyMappingsResult loadPolicyMappingsResultByType = polarisMetaStoreManager.loadPoliciesOnEntityByType( this.polarisCallContext, target, policy.getPolicyType()); validateLoadedPolicyMappings(loadPolicyMappingsResultByType); @@ -1047,7 +1048,7 @@ void ensurePolicyMappingRecordRemoved(PolarisBaseEntity target, PolicyEntity pol .getEntity()); Assertions.assertThat(policy).isNotNull(); - PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResult = + LoadPolicyMappingsResult loadPolicyMappingsResult = polarisMetaStoreManager.loadPoliciesOnEntity(this.polarisCallContext, target); validateLoadedPolicyMappings(loadPolicyMappingsResult); @@ -1056,7 +1057,7 @@ void ensurePolicyMappingRecordRemoved(PolarisBaseEntity target, PolicyEntity pol loadPolicyMappingsResult.getPolicyMappingRecords(), target, policy); // also try load by specific type - PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingsResultByType = + LoadPolicyMappingsResult loadPolicyMappingsResultByType = polarisMetaStoreManager.loadPoliciesOnEntityByType( this.polarisCallContext, target, policy.getPolicyType()); validateLoadedPolicyMappings(loadPolicyMappingsResultByType); @@ -1070,8 +1071,7 @@ void ensurePolicyMappingRecordRemoved(PolarisBaseEntity target, PolicyEntity pol * @param loadPolicyMappingRecords return from calling * loadPoliciesOnEntity()/LoadPoliciesOnEntityByType() */ - void validateLoadedPolicyMappings( - PolarisPolicyMappingManager.LoadPolicyMappingsResult loadPolicyMappingRecords) { + void validateLoadedPolicyMappings(LoadPolicyMappingsResult loadPolicyMappingRecords) { Assertions.assertThat(loadPolicyMappingRecords).isNotNull(); Map policyEntities = loadPolicyMappingRecords.getPolicyEntitiesAsMap(); @@ -2667,7 +2667,7 @@ void testPolicyMapping() { attachPolicyToTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N5), N5_P3); // attach a different policy of same inheritable type to the same target, should fail - PolarisPolicyMappingManager.AttachmentResult attachmentResult = + PolicyAttachmentResult policyAttachmentResult = polarisMetaStoreManager.attachPolicyToEntity( polarisCallContext, List.of(catalog, N1, N1_N2), @@ -2676,8 +2676,8 @@ void testPolicyMapping() { N1_P2, null); - Assertions.assertThat(attachmentResult.isSuccess()).isFalse(); - Assertions.assertThat(attachmentResult.getReturnStatus()) + Assertions.assertThat(policyAttachmentResult.isSuccess()).isFalse(); + Assertions.assertThat(policyAttachmentResult.getReturnStatus()) .isEqualTo(BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS); detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N1), N1_P1); detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N5), N5_P3); From d6fae34b6bae8157d4a9e593cd0a6cf1897cb070 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 12:05:22 -0700 Subject: [PATCH 06/15] TransactionalPolicyMappingPersistence does not need to inherit PolicyMappingPersistence --- .../core/policy/TransactionalPolicyMappingPersistence.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java index f5c1c99541..85944b12eb 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java @@ -24,7 +24,7 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.entity.PolarisEntityCore; -public interface TransactionalPolicyMappingPersistence extends PolicyMappingPersistence { +public interface TransactionalPolicyMappingPersistence { /** See {@link PolicyMappingPersistence#writeToPolicyMappingRecords} */ default void writeToPolicyMappingRecordsInCurrentTxn( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { From 191694f57e88b21abefea8b6d72cf5bc9e9a10a5 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 12:06:59 -0700 Subject: [PATCH 07/15] A small nit change to add a blank line --- .../apache/polaris/core/persistence/dao/entity/BaseResult.java | 1 + 1 file changed, 1 insertion(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java index d540be49ea..a4eee22cc4 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java @@ -111,6 +111,7 @@ public enum ReturnStatus { // error caught while sub-scoping credentials. Error message will be returned SUBSCOPE_CREDS_ERROR(13), + // policy mapping not found POLICY_MAPPING_NOT_FOUND(14), From 30e628c9e64d9707aac7a431273cdf98e24a9594 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 13:36:10 -0700 Subject: [PATCH 08/15] Fix a bug that does not filter by policy type --- .../persistence/AtomicOperationMetaStoreManager.java | 3 ++- .../core/persistence/PolarisTestMetaStoreManager.java | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index e9de41a270..c849a5ee77 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1933,7 +1933,8 @@ public Map getInternalPropertyMap( } final List policyMappingRecords = - ms.loadAllPoliciesOnTarget(callCtx, target.getCatalogId(), target.getId()); + ms.loadPoliciesOnTargetByType( + callCtx, target.getCatalogId(), target.getId(), policyType.getCode()); List policyEntities = loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 2261483c68..3075e141b3 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -2679,6 +2679,17 @@ void testPolicyMapping() { Assertions.assertThat(policyAttachmentResult.isSuccess()).isFalse(); Assertions.assertThat(policyAttachmentResult.getReturnStatus()) .isEqualTo(BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS); + + LoadPolicyMappingsResult loadPolicyMappingsResult = + polarisMetaStoreManager.loadPoliciesOnEntityByType( + polarisCallContext, N1_N2_T1, PredefinedPolicyTypes.DATA_COMPACTION); + Assertions.assertThat(loadPolicyMappingsResult.isSuccess()).isTrue(); + Assertions.assertThat(loadPolicyMappingsResult.getPolicyEntities()).hasSize(1); + PolicyEntity policyEntity = loadPolicyMappingsResult.getPolicyEntities().get(0); + Assertions.assertThat(policyEntity.getId()).isEqualTo(N1_P1.getId()); + Assertions.assertThat(policyEntity.getPolicyType()) + .isEqualTo(PredefinedPolicyTypes.DATA_COMPACTION); + detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N1), N1_P1); detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog, N5), N5_P3); } From a2a537cd889a6153bd83ffa33ff1edfc1b6ba6db Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 17:46:36 -0700 Subject: [PATCH 09/15] First attempt to push down logic to persistence layer --- .../AtomicOperationMetaStoreManager.java | 46 ++++++------------- .../PolicyMappingAlreadyExistsException.java | 41 +++++++++++++++++ .../dao/entity/PolicyAttachmentResult.java | 17 +++++-- .../AbstractTransactionalPersistence.java | 36 ++++++++++++++- .../TransactionalMetaStoreManagerImpl.java | 45 ++++++------------ ...TransactionalPolicyMappingPersistence.java | 14 ++++++ 6 files changed, 131 insertions(+), 68 deletions(-) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/PolicyMappingAlreadyExistsException.java diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index c849a5ee77..effaec2096 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1838,35 +1838,7 @@ public Map getInternalPropertyMap( // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); - PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode()); - if (policyType == null) { - // This should never happen - return new PolicyAttachmentResult( - BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); - } - - // TODO: this may make it necessary to push down this logic to policy persistence layer - if (policyType.isInheritable()) { - // Verify that there is no other mapping of the same type but different entity - List existingRecordsOfSameType = - ms.loadPoliciesOnTargetByType( - callCtx, target.getCatalogId(), target.getId(), policy.getPolicyTypeCode()); - if (existingRecordsOfSameType.size() > 1) { - return new PolicyAttachmentResult( - BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); - } else if (existingRecordsOfSameType.size() == 1) { - PolarisPolicyMappingRecord existingRecord = existingRecordsOfSameType.get(0); - if (existingRecord.getPolicyId() != policy.getId() - || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) { - return new PolicyAttachmentResult( - BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); - } - } - } - - PolarisPolicyMappingRecord mappingRecord = - this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); - return new PolicyAttachmentResult(mappingRecord); + return this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); } @Override @@ -1951,7 +1923,7 @@ public Map getInternalPropertyMap( * @param parameters optional parameters * @return new policy mapping record which was created and persisted */ - private @Nonnull PolarisPolicyMappingRecord persistNewPolicyMappingRecord( + private @Nonnull PolicyAttachmentResult persistNewPolicyMappingRecord( @Nonnull PolarisCallContext callCtx, @Nonnull BasePersistence ms, @Nonnull PolarisEntityCore target, @@ -1968,10 +1940,18 @@ public Map getInternalPropertyMap( policy.getId(), policy.getPolicyTypeCode(), parameters); + try { + ms.writeToPolicyMappingRecords(callCtx, mappingRecord); + } catch (IllegalArgumentException e) { + return new PolicyAttachmentResult( + BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); + } catch (PolicyMappingAlreadyExistsException e) { + return new PolicyAttachmentResult( + BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, + e.getExistingRecord().getPolicyTypeCode()); + } - ms.writeToPolicyMappingRecords(callCtx, mappingRecord); - - return mappingRecord; + return new PolicyAttachmentResult(mappingRecord); } /** diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolicyMappingAlreadyExistsException.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolicyMappingAlreadyExistsException.java new file mode 100644 index 0000000000..2cd714f25d --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolicyMappingAlreadyExistsException.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence; + +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; + +/** + * Exception raised when an existing policy mapping preveents the attempted creation of a new policy + * mapping record. + */ +public class PolicyMappingAlreadyExistsException extends RuntimeException { + private PolarisPolicyMappingRecord existingRecord; + + /** + * @param existingRecord The conflicting record that caused creation to fail. + */ + public PolicyMappingAlreadyExistsException(PolarisPolicyMappingRecord existingRecord) { + super("Existing Policy Mapping Record: " + existingRecord); + this.existingRecord = existingRecord; + } + + public PolarisPolicyMappingRecord getExistingRecord() { + return this.existingRecord; + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java index f95c9884ce..ffc7eabdd3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/PolicyAttachmentResult.java @@ -32,12 +32,23 @@ public class PolicyAttachmentResult extends BaseResult { /** * Constructor for an error * - * @param errorCode error code, cannot be SUCCESS + * @param errorStatus error code, cannot be SUCCESS * @param extraInformation extra information */ public PolicyAttachmentResult( - @Nonnull ReturnStatus errorCode, @Nullable String extraInformation) { - super(errorCode, extraInformation); + @Nonnull ReturnStatus errorStatus, @Nullable String extraInformation) { + super(errorStatus, extraInformation); + this.mappingRecord = null; + } + + /** + * Constructor for an error + * + * @param errorStatus error code, cannot be SUCCESS + * @param policyTypeCode existing policy mapping record's policy type code + */ + public PolicyAttachmentResult(@Nonnull ReturnStatus errorStatus, int policyTypeCode) { + super(errorStatus, Integer.toString(policyTypeCode)); this.mappingRecord = null; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java index 28670b8a87..06ad023ad8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java @@ -18,6 +18,7 @@ */ package org.apache.polaris.core.persistence.transactional; +import com.google.common.base.Preconditions; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.List; @@ -34,8 +35,10 @@ import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.EntityAlreadyExistsException; +import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -667,7 +670,38 @@ public EntityNameLookupRecord lookupEntityIdAndSubTypeByNameInCurrentTxn( public void writeToPolicyMappingRecords( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { this.runActionInTransaction( - callCtx, () -> this.writeToPolicyMappingRecordsInCurrentTxn(callCtx, record)); + callCtx, + () -> { + this.checkConditionsForWriteToPolicyMappingRecordsInCurrentTxn(callCtx, record); + this.writeToPolicyMappingRecordsInCurrentTxn(callCtx, record); + }); + } + + /** {@inheritDoc} */ + @Override + public void checkConditionsForWriteToPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + + PolicyType policyType = PolicyType.fromCode(record.getPolicyTypeCode()); + Preconditions.checkArgument( + policyType != null, "Invalid policy type code: %s", record.getPolicyTypeCode()); + + if (!policyType.isInheritable()) { + return; + } + + List existingRecords = + this.loadPoliciesOnTargetByTypeInCurrentTxn( + callCtx, record.getTargetCatalogId(), record.getTargetId(), record.getPolicyTypeCode()); + if (existingRecords.size() > 1) { + throw new PolicyMappingAlreadyExistsException(existingRecords.get(0)); + } else if (existingRecords.size() == 1) { + PolarisPolicyMappingRecord existingRecord = existingRecords.get(0); + if (existingRecord.getPolicyCatalogId() != record.getPolicyCatalogId() + || existingRecord.getPolicyId() != record.getPolicyId()) { + throw new PolicyMappingAlreadyExistsException(existingRecord); + } + } } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 0f7aa654d0..a1bbe363fa 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -2361,34 +2361,7 @@ public Map getInternalPropertyMap( return new PolicyAttachmentResult(BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED, null); } - PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode()); - if (policyType == null) { - // This should never happen - return new PolicyAttachmentResult( - BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); - } - - if (policyType.isInheritable()) { - // Verify that there is no other mapping of the same type but different entity - List existingRecordsOfSameType = - ms.loadPoliciesOnTargetByTypeInCurrentTxn( - callCtx, target.getCatalogId(), target.getId(), policy.getPolicyTypeCode()); - if (existingRecordsOfSameType.size() > 1) { - return new PolicyAttachmentResult( - BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); - } else if (existingRecordsOfSameType.size() == 1) { - PolarisPolicyMappingRecord existingRecord = existingRecordsOfSameType.get(0); - if (existingRecord.getPolicyId() != policy.getId() - || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) { - return new PolicyAttachmentResult( - BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null); - } - } - } - - PolarisPolicyMappingRecord mappingRecord = - this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); - return new PolicyAttachmentResult(mappingRecord); + return this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, parameters); } /** {@inheritDoc} */ @@ -2515,7 +2488,7 @@ public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( * @param parameters optional parameters * @return new policy mapping record which was created and persisted */ - private @Nonnull PolarisPolicyMappingRecord persistNewPolicyMappingRecord( + private @Nonnull PolicyAttachmentResult persistNewPolicyMappingRecord( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nonnull PolarisEntityCore target, @@ -2533,9 +2506,19 @@ public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( policy.getPolicyTypeCode(), parameters); - ms.writeToPolicyMappingRecordsInCurrentTxn(callCtx, mappingRecord); + try { + ms.checkConditionsForWriteToPolicyMappingRecordsInCurrentTxn(callCtx, mappingRecord); + ms.writeToPolicyMappingRecordsInCurrentTxn(callCtx, mappingRecord); + } catch (IllegalArgumentException e) { + return new PolicyAttachmentResult( + BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy type"); + } catch (PolicyMappingAlreadyExistsException e) { + return new PolicyAttachmentResult( + BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, + e.getExistingRecord().getPolicyTypeCode()); + } - return mappingRecord; + return new PolicyAttachmentResult(mappingRecord); } /** diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java index 85944b12eb..498a4367a2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java @@ -31,6 +31,20 @@ default void writeToPolicyMappingRecordsInCurrentTxn( throw new UnsupportedOperationException("Not Implemented"); } + /** + * Helpers to check conditions for writing new PolicyMappingRecords in current transaction. + * + *

It should throw a PolicyMappingAlreadyExistsException if the new record conflicts with an + * exising record with same policy type but different policy. + * + * @param callCtx call context + * @param record policy mapping record to write. + */ + default void checkConditionsForWriteToPolicyMappingRecordsInCurrentTxn( + @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { + throw new UnsupportedOperationException("Not Implemented"); + } + /** See {@link PolicyMappingPersistence#deleteFromPolicyMappingRecords} */ default void deleteFromPolicyMappingRecordsInCurrentTxn( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisPolicyMappingRecord record) { From 7da52870e17ebaf2d1eb14d7e75d7d3276b47620 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 20:40:52 -0700 Subject: [PATCH 10/15] Use lookupEntities --- .../AtomicOperationMetaStoreManager.java | 12 ++++++------ .../TransactionalMetaStoreManagerImpl.java | 16 ++++++++-------- .../core/policy/PolicyMappingPersistence.java | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index effaec2096..13f515d98a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1874,9 +1874,9 @@ public Map getInternalPropertyMap( // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); - int grantRecordVersion = - ms.lookupEntityGrantRecordsVersion(callCtx, target.getCatalogId(), target.getId()); - if (grantRecordVersion == 0) { + PolarisBaseEntity entity = + ms.lookupEntity(callCtx, target.getCatalogId(), target.getId(), target.getTypeCode()); + if (entity == null) { // Target entity does not exists return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } @@ -1897,9 +1897,9 @@ public Map getInternalPropertyMap( // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); - int grantRecordVersion = - ms.lookupEntityGrantRecordsVersion(callCtx, target.getCatalogId(), target.getId()); - if (grantRecordVersion == 0) { + PolarisBaseEntity entity = + ms.lookupEntity(callCtx, target.getCatalogId(), target.getId(), target.getTypeCode()); + if (entity == null) { // Target entity does not exists return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index a1bbe363fa..5261e844cd 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -2429,10 +2429,10 @@ private LoadPolicyMappingsResult doLoadPoliciesOnEntity( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nonnull PolarisEntityCore target) { - int grantRecordVersion = - ms.lookupEntityGrantRecordsVersionInCurrentTxn( - callCtx, target.getCatalogId(), target.getId()); - if (grantRecordVersion == 0) { + PolarisBaseEntity entity = + ms.lookupEntityInCurrentTxn( + callCtx, target.getCatalogId(), target.getId(), target.getTypeCode()); + if (entity == null) { // Target entity does not exists return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } @@ -2462,10 +2462,10 @@ public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( @Nonnull TransactionalPersistence ms, @Nonnull PolarisEntityCore target, @Nonnull PolicyType policyType) { - int grantRecordVersion = - ms.lookupEntityGrantRecordsVersionInCurrentTxn( - callCtx, target.getCatalogId(), target.getId()); - if (grantRecordVersion == 0) { + PolarisBaseEntity entity = + ms.lookupEntityInCurrentTxn( + callCtx, target.getCatalogId(), target.getId(), target.getTypeCode()); + if (entity == null) { // Target entity does not exists return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java index c6ca31213f..d4093f753f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java @@ -31,7 +31,7 @@ * *

Note that APIs to the actual persistence store are very basic, often point read or write to * the underlying data store. The goal is to make it really easy to back this using databases like - * Postgres or simpler KV store. + * Postgres or simpler KV store. Each API in this interface need to be atomic. */ public interface PolicyMappingPersistence { From 2f181df3685e3f33690f5e58a78d5b883aa6c072 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 25 Mar 2025 21:01:02 -0700 Subject: [PATCH 11/15] LoadPolicyMappingResult need to be general in the return type. --- .../AtomicOperationMetaStoreManager.java | 10 ++--- .../dao/entity/LoadPolicyMappingsResult.java | 38 ++++++++++++------- .../TransactionalMetaStoreManagerImpl.java | 10 ++--- .../PolarisTestMetaStoreManager.java | 6 +-- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 13f515d98a..1dcc52db09 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1884,7 +1884,7 @@ public Map getInternalPropertyMap( final List policyMappingRecords = ms.loadAllPoliciesOnTarget(callCtx, target.getCatalogId(), target.getId()); - List policyEntities = + List policyEntities = loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); } @@ -1908,7 +1908,7 @@ public Map getInternalPropertyMap( ms.loadPoliciesOnTargetByType( callCtx, target.getCatalogId(), target.getId(), policyType.getCode()); - List policyEntities = + List policyEntities = loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); } @@ -1962,7 +1962,7 @@ public Map getInternalPropertyMap( * @param policyMappingRecords a list of policy mapping records * @return a list of policy entities */ - private List loadPoliciesFromMappingRecords( + private List loadPoliciesFromMappingRecords( @Nonnull PolarisCallContext callCtx, @Nonnull BasePersistence ms, @Nonnull List policyMappingRecords) { @@ -1975,8 +1975,6 @@ private List loadPoliciesFromMappingRecords( policyMappingRecord.getPolicyId())) .distinct() .collect(Collectors.toList()); - return ms.lookupEntities(callCtx, policyEntityIds).stream() - .map(PolicyEntity::of) - .collect(Collectors.toList()); + return ms.lookupEntities(callCtx, policyEntityIds); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java index 906b6727d8..48a0277dd9 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/LoadPolicyMappingsResult.java @@ -26,16 +26,16 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; -import org.apache.polaris.core.policy.PolicyEntity; /** result of a load policy mapping call */ public class LoadPolicyMappingsResult extends BaseResult { // null if not success. Else set of policy mapping records on a target or from a policy private final List mappingRecords; - // null if not success. Else set of policy entities in the mapping records. - private final List policyEntities; + // null if not success. Else, for each policy mapping record, list of target or policy entities + private final List entities; /** * Constructor for an error @@ -47,21 +47,21 @@ public LoadPolicyMappingsResult( @Nonnull ReturnStatus errorCode, @Nullable String extraInformation) { super(errorCode, extraInformation); this.mappingRecords = null; - this.policyEntities = null; + this.entities = null; } /** * Constructor for success * * @param mappingRecords policy mapping records - * @param policyEntities policy entities + * @param entities policy entities */ public LoadPolicyMappingsResult( @Nonnull List mappingRecords, - @Nonnull List policyEntities) { + @Nonnull List entities) { super(ReturnStatus.SUCCESS); this.mappingRecords = mappingRecords; - this.policyEntities = policyEntities; + this.entities = entities; } @JsonCreator @@ -69,24 +69,34 @@ private LoadPolicyMappingsResult( @JsonProperty("returnStatus") @Nonnull ReturnStatus returnStatus, @JsonProperty("extraInformation") String extraInformation, @JsonProperty("policyMappingRecords") List mappingRecords, - @JsonProperty("policyEntities") List policyEntities) { + @JsonProperty("policyEntities") List entities) { super(returnStatus, extraInformation); this.mappingRecords = mappingRecords; - this.policyEntities = policyEntities; + this.entities = entities; } public List getPolicyMappingRecords() { return mappingRecords; } - public List getPolicyEntities() { - return policyEntities; + public List getEntities() { + return entities; } @JsonIgnore - public Map getPolicyEntitiesAsMap() { - return policyEntities == null + public Map getEntitiesAsMap() { + return entities == null ? null - : policyEntities.stream().collect(Collectors.toMap(PolicyEntity::getId, entity -> entity)); + : entities.stream().collect(Collectors.toMap(PolarisBaseEntity::getId, entity -> entity)); + } + + @Override + public String toString() { + return "LoadPolicyMappingsResult{" + + "mappingRecords=" + + mappingRecords + + ", entities=" + + entities + + '}'; } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 5261e844cd..27b049e917 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -2440,7 +2440,7 @@ private LoadPolicyMappingsResult doLoadPoliciesOnEntity( final List policyMappingRecords = ms.loadAllPoliciesOnTargetInCurrentTxn(callCtx, target.getCatalogId(), target.getId()); - List policyEntities = + List policyEntities = loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); } @@ -2473,7 +2473,7 @@ public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( final List policyMappingRecords = ms.loadPoliciesOnTargetByTypeInCurrentTxn( callCtx, target.getCatalogId(), target.getId(), policyType.getCode()); - List policyEntities = + List policyEntities = loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords); return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities); } @@ -2529,7 +2529,7 @@ public LoadPolicyMappingsResult doLoadPoliciesOnEntityByType( * @param policyMappingRecords a list of policy mapping records * @return a list of policy entities */ - private List loadPoliciesFromMappingRecords( + private List loadPoliciesFromMappingRecords( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nonnull List policyMappingRecords) { @@ -2542,8 +2542,6 @@ private List loadPoliciesFromMappingRecords( policyMappingRecord.getPolicyId())) .distinct() .collect(Collectors.toList()); - return ms.lookupEntitiesInCurrentTxn(callCtx, policyEntityIds).stream() - .map(PolicyEntity::of) - .collect(Collectors.toList()); + return ms.lookupEntitiesInCurrentTxn(callCtx, policyEntityIds); } } diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 3075e141b3..f4290b2676 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -1074,7 +1074,7 @@ void ensurePolicyMappingRecordRemoved(PolarisBaseEntity target, PolicyEntity pol void validateLoadedPolicyMappings(LoadPolicyMappingsResult loadPolicyMappingRecords) { Assertions.assertThat(loadPolicyMappingRecords).isNotNull(); - Map policyEntities = loadPolicyMappingRecords.getPolicyEntitiesAsMap(); + Map policyEntities = loadPolicyMappingRecords.getEntitiesAsMap(); Assertions.assertThat(policyEntities).isNotNull(); for (PolarisPolicyMappingRecord policyMappingRecord : @@ -2684,8 +2684,8 @@ void testPolicyMapping() { polarisMetaStoreManager.loadPoliciesOnEntityByType( polarisCallContext, N1_N2_T1, PredefinedPolicyTypes.DATA_COMPACTION); Assertions.assertThat(loadPolicyMappingsResult.isSuccess()).isTrue(); - Assertions.assertThat(loadPolicyMappingsResult.getPolicyEntities()).hasSize(1); - PolicyEntity policyEntity = loadPolicyMappingsResult.getPolicyEntities().get(0); + Assertions.assertThat(loadPolicyMappingsResult.getEntities()).hasSize(1); + PolicyEntity policyEntity = PolicyEntity.of(loadPolicyMappingsResult.getEntities().get(0)); Assertions.assertThat(policyEntity.getId()).isEqualTo(N1_P1.getId()); Assertions.assertThat(policyEntity.getPolicyType()) .isEqualTo(PredefinedPolicyTypes.DATA_COMPACTION); From 83fdf613f3506c909bea7a57df1cd439a3fd01f0 Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 27 Mar 2025 16:13:33 -0500 Subject: [PATCH 12/15] fix nit --- .../core/persistence/AtomicOperationMetaStoreManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 1dcc52db09..9796073767 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1877,7 +1877,7 @@ public Map getInternalPropertyMap( PolarisBaseEntity entity = ms.lookupEntity(callCtx, target.getCatalogId(), target.getId(), target.getTypeCode()); if (entity == null) { - // Target entity does not exists + // Target entity does not exist return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } @@ -1900,7 +1900,7 @@ public Map getInternalPropertyMap( PolarisBaseEntity entity = ms.lookupEntity(callCtx, target.getCatalogId(), target.getId(), target.getTypeCode()); if (entity == null) { - // Target entity does not exists + // Target entity does not exist return new LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } From f4a56f4455dbd3c75918c56828893840c81d93a4 Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 27 Mar 2025 16:29:34 -0500 Subject: [PATCH 13/15] fix merge conflict --- .../polaris/core/persistence/PolarisTestMetaStoreManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index f4290b2676..3e7a0fb2ac 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -2650,7 +2650,7 @@ void testPolicyMapping() { PolarisBaseEntity N1_N2_T1 = this.ensureExistsByName( List.of(catalog, N1, N1_N2), - PolarisEntityType.ICEBERG_TABLE_LIKE, + PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, "T1"); From 6dc0e569708fbb4a67f4e3509980cc902866f7d7 Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 28 Mar 2025 16:26:43 -0500 Subject: [PATCH 14/15] Update to make naming reasonable --- .../eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java | 4 ++-- .../persistence/impl/eclipselink/PolarisEclipseLinkStore.java | 4 ++-- .../transactional/AbstractTransactionalPersistence.java | 4 ++-- .../transactional/TreeMapTransactionalPersistenceImpl.java | 2 +- .../apache/polaris/core/policy/PolicyMappingPersistence.java | 2 +- .../core/policy/TransactionalPolicyMappingPersistence.java | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index a00aedb706..b20f18e56f 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -741,10 +741,10 @@ public List loadAllPoliciesOnTargetInCurrentTxn( /** {@inheritDoc} */ @Nonnull @Override - public List loadAllPoliciesOnPolicyInCurrentTxn( + public List loadAllTargetsOnPolicyInCurrentTxn( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { return this.store - .loadAllPoliciesOnPolicy(localSession.get(), policyCatalogId, policyId) + .loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId) .stream() .map(ModelPolicyMappingRecord::toPolicyMappingRecord) .toList(); diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index ab1d8c91f8..6628f0fe21 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -443,7 +443,7 @@ void deleteAllEntityPolicyMappingRecords(EntityManager session, PolarisEntityCor diagnosticServices.check(session != null, "session_is_null"); checkInitialized(); - loadAllPoliciesOnPolicy(session, entity.getCatalogId(), entity.getId()) + loadAllTargetsOnPolicy(session, entity.getCatalogId(), entity.getId()) .forEach(session::remove); loadAllPoliciesOnTarget(session, entity.getCatalogId(), entity.getId()) .forEach(session::remove); @@ -512,7 +512,7 @@ List loadAllPoliciesOnTarget( .getResultList(); } - List loadAllPoliciesOnPolicy( + List loadAllTargetsOnPolicy( EntityManager session, long policyCatalogId, long policyId) { diagnosticServices.check(session != null, "session_is_null"); checkInitialized(); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java index 06ad023ad8..04da04d14a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java @@ -771,10 +771,10 @@ public List loadAllPoliciesOnTarget( /** {@inheritDoc} */ @Override @Nonnull - public List loadAllPoliciesOnPolicy( + public List loadAllTargetsOnPolicy( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { return this.runInReadTransaction( callCtx, - () -> this.loadAllPoliciesOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId)); + () -> this.loadAllTargetsOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId)); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 3a48215496..8220456737 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -628,7 +628,7 @@ record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); /** {@inheritDoc} */ @Override - public @Nonnull List loadAllPoliciesOnPolicyInCurrentTxn( + public @Nonnull List loadAllTargetsOnPolicyInCurrentTxn( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { return this.store .getSlicePolicyMappingRecordsByPolicy() diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java index d4093f753f..33a7546689 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingPersistence.java @@ -144,7 +144,7 @@ default List loadAllPoliciesOnTarget( * @return the list of policy mapping records for the specified policy entity */ @Nonnull - default List loadAllPoliciesOnPolicy( + default List loadAllTargetsOnPolicy( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { throw new UnsupportedOperationException("Not Implemented"); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java index 498a4367a2..6ad1ac80c8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/policy/TransactionalPolicyMappingPersistence.java @@ -89,9 +89,9 @@ default List loadAllPoliciesOnTargetInCurrentTxn( throw new UnsupportedOperationException("Not Implemented"); } - /** See {@link PolicyMappingPersistence#loadAllPoliciesOnPolicy} */ + /** See {@link PolicyMappingPersistence#loadAllTargetsOnPolicy} */ @Nonnull - default List loadAllPoliciesOnPolicyInCurrentTxn( + default List loadAllTargetsOnPolicyInCurrentTxn( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { throw new UnsupportedOperationException("Not Implemented"); } From d0864ee973a34d84779f9dc6d4d8e53ad7a17551 Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 28 Mar 2025 16:32:29 -0500 Subject: [PATCH 15/15] Fix CI --- .../eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java | 4 +--- .../persistence/impl/eclipselink/PolarisEclipseLinkStore.java | 3 +-- .../transactional/AbstractTransactionalPersistence.java | 3 +-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index b20f18e56f..a8492384eb 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -743,9 +743,7 @@ public List loadAllPoliciesOnTargetInCurrentTxn( @Override public List loadAllTargetsOnPolicyInCurrentTxn( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { - return this.store - .loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId) - .stream() + return this.store.loadAllTargetsOnPolicy(localSession.get(), policyCatalogId, policyId).stream() .map(ModelPolicyMappingRecord::toPolicyMappingRecord) .toList(); } diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index 6628f0fe21..fc889656b5 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -443,8 +443,7 @@ void deleteAllEntityPolicyMappingRecords(EntityManager session, PolarisEntityCor diagnosticServices.check(session != null, "session_is_null"); checkInitialized(); - loadAllTargetsOnPolicy(session, entity.getCatalogId(), entity.getId()) - .forEach(session::remove); + loadAllTargetsOnPolicy(session, entity.getCatalogId(), entity.getId()).forEach(session::remove); loadAllPoliciesOnTarget(session, entity.getCatalogId(), entity.getId()) .forEach(session::remove); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java index 04da04d14a..e949b33fe2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java @@ -774,7 +774,6 @@ public List loadAllPoliciesOnTarget( public List loadAllTargetsOnPolicy( @Nonnull PolarisCallContext callCtx, long policyCatalogId, long policyId) { return this.runInReadTransaction( - callCtx, - () -> this.loadAllTargetsOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId)); + callCtx, () -> this.loadAllTargetsOnPolicyInCurrentTxn(callCtx, policyCatalogId, policyId)); } }