gh-yzou commented on code in PR #1294: URL: https://github.com/apache/polaris/pull/1294#discussion_r2027589160
########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -0,0 +1,300 @@ +/* + * 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.service.catalog.policy; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.CatalogEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException; +import org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException; +import org.apache.polaris.core.policy.validator.PolicyValidators; +import org.apache.polaris.service.types.Policy; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PolicyCatalog { + private static final Logger LOGGER = LoggerFactory.getLogger(PolicyCatalog.class); + + private final CallContext callContext; + private final PolarisResolutionManifestCatalogView resolvedEntityView; + private final CatalogEntity catalogEntity; + private long catalogId = -1; + private PolarisMetaStoreManager metaStoreManager; + + public PolicyCatalog( + PolarisMetaStoreManager metaStoreManager, + CallContext callContext, + PolarisResolutionManifestCatalogView resolvedEntityView) { + this.callContext = callContext; + this.resolvedEntityView = resolvedEntityView; + this.catalogEntity = + CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + this.catalogId = catalogEntity.getId(); + this.metaStoreManager = metaStoreManager; + } + + public Policy createPolicy( + PolicyIdentifier policyIdentifier, String type, String description, String content) { + PolarisResolvedPathWrapper resolvedPolicyEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity entity = + PolicyEntity.of( + resolvedPolicyEntities == null ? null : resolvedPolicyEntities.getRawLeafEntity()); + + if (entity == null) { + PolicyType policyType = PolicyType.fromName(type); + if (policyType == null) { + throw new BadRequestException("Unknown policy type: %s", type); + } + + entity = + new PolicyEntity.Builder( + policyIdentifier.namespace(), policyIdentifier.name(), policyType) + .setCatalogId(catalogId) + .setDescription(description) + .setContent(content) + .setId(getMetaStoreManager().generateNewEntityId(getCurrentPolarisContext()).getId()) + .build(); + + PolicyValidators.validate(entity); + + } else { + throw new AlreadyExistsException("Policy already exists %s", policyIdentifier); + } + + return constructPolicy(createPolicyEntity(policyIdentifier, entity)); + } + + public List<PolicyIdentifier> listPolicies(Namespace namespace, PolicyType policyType) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved namespace '%s'", namespace)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath(); + List<PolicyEntity> policyEntities = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.POLICY, + PolarisEntitySubType.ANY_SUBTYPE) + .getEntities() + .stream() + .map( + polarisEntityActiveRecord -> + PolicyEntity.of( + getMetaStoreManager() + .loadEntity( + getCurrentPolarisContext(), + polarisEntityActiveRecord.getCatalogId(), + polarisEntityActiveRecord.getId(), + polarisEntityActiveRecord.getType()) + .getEntity())) + .filter( + policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) + .toList(); + + List<PolarisEntity.NameAndId> entities = + policyEntities.stream().map(PolarisEntity::nameAndId).toList(); + + return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities).stream() + .map(PolicyIdentifier::fromTableIdentifier) + .toList(); + } + + public Policy loadPolicy(PolicyIdentifier policyIdentifier) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + return constructPolicy(policy); + } + + public Policy updatePolicy( + PolicyIdentifier policyIdentifier, + String newDescription, + String newContent, + int currentPolicyVersion) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy); + int policyVersion = policy.getPolicyVersion(); + if (currentPolicyVersion != policyVersion) { + throw new PolicyVersionMismatchException( + String.format("Policy version mismatch. Current version is %d", policyVersion)); + } + boolean hasUpdate = false; + if (newContent != null) { + newPolicyBuilder.setContent(newContent); + hasUpdate = true; + } + + if (newDescription != null) { + newPolicyBuilder.setDescription(newDescription); + hasUpdate = true; + } + + if (!hasUpdate) { + return constructPolicy(policy); + } + + newPolicyBuilder.setPolicyVersion(policyVersion + 1); + PolicyEntity newPolicyEntity = newPolicyBuilder.build(); + PolicyValidators.validate(newPolicyEntity); + newPolicyEntity = PolicyEntity.of(updatePolicy(policyIdentifier, newPolicyEntity)); + + return constructPolicy(newPolicyEntity); + } + + public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean detachAll) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + if (resolvedEntities == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath(); + PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); + + DropEntityResult dropEntityResult = + getMetaStoreManager() + .dropEntityIfExists( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + leafEntity, + Map.of(), + false); + + return dropEntityResult.isSuccess(); + } + + private PolicyEntity createPolicyEntity(PolicyIdentifier identifier, PolarisEntity entity) { + PolarisResolvedPathWrapper resolvedParent = + resolvedEntityView.getResolvedPath(identifier.namespace()); + if (resolvedParent == null) { + // Illegal state because the namespace should've already been in the static resolution set. + throw new IllegalStateException( + String.format("Failed to fetch resolved parent for Policy '%s'", identifier)); + } + + List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath(); + if (entity.getParentId() <= 0) { + entity = + new PolarisEntity.Builder(entity) + .setParentId(resolvedParent.getRawLeafEntity().getId()) + .build(); + } + + entity = + new PolarisEntity.Builder(entity).setCreateTimestamp(System.currentTimeMillis()).build(); + + PolarisEntity returnedEntity = + PolarisEntity.of( + getMetaStoreManager() + .createEntityIfNotExists( + getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), entity)); + + LOGGER.debug("Created Policy entity {} with Identifier {}", entity, identifier); + if (returnedEntity == null) { + throw new IllegalStateException("Failed to create Policy entity"); + } + + return PolicyEntity.of(returnedEntity); + } + + private PolarisEntity updatePolicy(PolicyIdentifier identifier, PolarisEntity entity) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getResolvedPath(identifier, entity.getType(), entity.getSubType()); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved PolicyIdentifier '%s'", identifier)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath(); + PolarisEntity returnedEntity = + Optional.ofNullable( + getMetaStoreManager() + .updateEntityPropertiesIfNotChanged( + getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), entity) + .getEntity()) + .map(PolarisEntity::new) + .orElse(null); + if (returnedEntity == null) { + throw new IllegalStateException("Failed to update Policy entity"); + } + + return returnedEntity; + } + + private PolarisMetaStoreManager getMetaStoreManager() { + return metaStoreManager; Review Comment: do we nee those two functions? it seems just a direct return there ########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -0,0 +1,300 @@ +/* + * 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.service.catalog.policy; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.CatalogEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException; +import org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException; +import org.apache.polaris.core.policy.validator.PolicyValidators; +import org.apache.polaris.service.types.Policy; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PolicyCatalog { + private static final Logger LOGGER = LoggerFactory.getLogger(PolicyCatalog.class); + + private final CallContext callContext; + private final PolarisResolutionManifestCatalogView resolvedEntityView; + private final CatalogEntity catalogEntity; + private long catalogId = -1; + private PolarisMetaStoreManager metaStoreManager; + + public PolicyCatalog( + PolarisMetaStoreManager metaStoreManager, + CallContext callContext, + PolarisResolutionManifestCatalogView resolvedEntityView) { + this.callContext = callContext; + this.resolvedEntityView = resolvedEntityView; + this.catalogEntity = + CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + this.catalogId = catalogEntity.getId(); + this.metaStoreManager = metaStoreManager; + } + + public Policy createPolicy( + PolicyIdentifier policyIdentifier, String type, String description, String content) { + PolarisResolvedPathWrapper resolvedPolicyEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity entity = + PolicyEntity.of( + resolvedPolicyEntities == null ? null : resolvedPolicyEntities.getRawLeafEntity()); + + if (entity == null) { + PolicyType policyType = PolicyType.fromName(type); + if (policyType == null) { + throw new BadRequestException("Unknown policy type: %s", type); + } + + entity = + new PolicyEntity.Builder( + policyIdentifier.namespace(), policyIdentifier.name(), policyType) + .setCatalogId(catalogId) + .setDescription(description) + .setContent(content) + .setId(getMetaStoreManager().generateNewEntityId(getCurrentPolarisContext()).getId()) + .build(); + + PolicyValidators.validate(entity); + + } else { + throw new AlreadyExistsException("Policy already exists %s", policyIdentifier); + } + + return constructPolicy(createPolicyEntity(policyIdentifier, entity)); + } + + public List<PolicyIdentifier> listPolicies(Namespace namespace, PolicyType policyType) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved namespace '%s'", namespace)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath(); + List<PolicyEntity> policyEntities = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.POLICY, + PolarisEntitySubType.ANY_SUBTYPE) + .getEntities() + .stream() + .map( + polarisEntityActiveRecord -> + PolicyEntity.of( + getMetaStoreManager() + .loadEntity( + getCurrentPolarisContext(), + polarisEntityActiveRecord.getCatalogId(), + polarisEntityActiveRecord.getId(), + polarisEntityActiveRecord.getType()) + .getEntity())) + .filter( + policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) + .toList(); + + List<PolarisEntity.NameAndId> entities = + policyEntities.stream().map(PolarisEntity::nameAndId).toList(); + + return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities).stream() + .map(PolicyIdentifier::fromTableIdentifier) + .toList(); + } + + public Policy loadPolicy(PolicyIdentifier policyIdentifier) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + return constructPolicy(policy); + } + + public Policy updatePolicy( + PolicyIdentifier policyIdentifier, + String newDescription, + String newContent, + int currentPolicyVersion) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy); + int policyVersion = policy.getPolicyVersion(); + if (currentPolicyVersion != policyVersion) { + throw new PolicyVersionMismatchException( + String.format("Policy version mismatch. Current version is %d", policyVersion)); + } + boolean hasUpdate = false; + if (newContent != null) { + newPolicyBuilder.setContent(newContent); + hasUpdate = true; + } + + if (newDescription != null) { + newPolicyBuilder.setDescription(newDescription); + hasUpdate = true; + } + + if (!hasUpdate) { + return constructPolicy(policy); + } + + newPolicyBuilder.setPolicyVersion(policyVersion + 1); + PolicyEntity newPolicyEntity = newPolicyBuilder.build(); + PolicyValidators.validate(newPolicyEntity); + newPolicyEntity = PolicyEntity.of(updatePolicy(policyIdentifier, newPolicyEntity)); + + return constructPolicy(newPolicyEntity); + } + + public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean detachAll) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + if (resolvedEntities == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath(); + PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); + + DropEntityResult dropEntityResult = + getMetaStoreManager() + .dropEntityIfExists( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + leafEntity, + Map.of(), + false); + + return dropEntityResult.isSuccess(); + } + + private PolicyEntity createPolicyEntity(PolicyIdentifier identifier, PolarisEntity entity) { + PolarisResolvedPathWrapper resolvedParent = + resolvedEntityView.getResolvedPath(identifier.namespace()); + if (resolvedParent == null) { + // Illegal state because the namespace should've already been in the static resolution set. + throw new IllegalStateException( + String.format("Failed to fetch resolved parent for Policy '%s'", identifier)); + } + + List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath(); + if (entity.getParentId() <= 0) { + entity = + new PolarisEntity.Builder(entity) + .setParentId(resolvedParent.getRawLeafEntity().getId()) + .build(); + } + + entity = + new PolarisEntity.Builder(entity).setCreateTimestamp(System.currentTimeMillis()).build(); + + PolarisEntity returnedEntity = + PolarisEntity.of( + getMetaStoreManager() + .createEntityIfNotExists( + getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), entity)); + + LOGGER.debug("Created Policy entity {} with Identifier {}", entity, identifier); + if (returnedEntity == null) { + throw new IllegalStateException("Failed to create Policy entity"); + } + + return PolicyEntity.of(returnedEntity); + } + + private PolarisEntity updatePolicy(PolicyIdentifier identifier, PolarisEntity entity) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getResolvedPath(identifier, entity.getType(), entity.getSubType()); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved PolicyIdentifier '%s'", identifier)); Review Comment: Does this mean no Policy found? should an exception like NoSuchPolicy with a clear message "Failed to find Policy %s" better? ########## service/common/src/testFixtures/java/org/apache/polaris/service/catalog/PolarisPassthroughResolutionView.java: ########## @@ -94,6 +95,14 @@ public PolarisResolvedPathWrapper getResolvedPath( identifier); manifest.resolveAll(); return manifest.getResolvedPath(identifier, entityType, subType); + } else if (key instanceof PolicyIdentifier identifier) { + manifest.addPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier.toTableIdentifier()), Review Comment: That seems a very hacky way, where we are converting between table identifiers and policy identifier? could you provide some details about why are we doing thing like this here, maybe we can see if there is other better ways to address this. ########## api/polaris-catalog-service/src/main/java/org/apache/polaris/service/types/PolicyIdentifier.java: ########## @@ -0,0 +1,122 @@ +/* + * 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.service.types; + +import com.google.common.base.Preconditions; +import com.google.common.base.Splitter; +import com.google.common.collect.Iterables; +import java.util.Arrays; +import java.util.Locale; +import java.util.Objects; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; + +public class PolicyIdentifier { + + private static final Splitter DOT = Splitter.on('.'); + + private final Namespace namespace; + private final String name; + + public static PolicyIdentifier of(String... names) { Review Comment: i don't think we should reuse Table Identifier as PolicyIdentifier also, even though they have a lot of similarity. However seems you have a PolicyIdentifier definition in the spec, why do we need to have a class override here? ########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -0,0 +1,300 @@ +/* + * 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.service.catalog.policy; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.CatalogEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException; +import org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException; +import org.apache.polaris.core.policy.validator.PolicyValidators; +import org.apache.polaris.service.types.Policy; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PolicyCatalog { + private static final Logger LOGGER = LoggerFactory.getLogger(PolicyCatalog.class); + + private final CallContext callContext; + private final PolarisResolutionManifestCatalogView resolvedEntityView; + private final CatalogEntity catalogEntity; + private long catalogId = -1; + private PolarisMetaStoreManager metaStoreManager; + + public PolicyCatalog( + PolarisMetaStoreManager metaStoreManager, + CallContext callContext, + PolarisResolutionManifestCatalogView resolvedEntityView) { + this.callContext = callContext; + this.resolvedEntityView = resolvedEntityView; + this.catalogEntity = + CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + this.catalogId = catalogEntity.getId(); + this.metaStoreManager = metaStoreManager; + } + + public Policy createPolicy( + PolicyIdentifier policyIdentifier, String type, String description, String content) { + PolarisResolvedPathWrapper resolvedPolicyEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity entity = + PolicyEntity.of( + resolvedPolicyEntities == null ? null : resolvedPolicyEntities.getRawLeafEntity()); + + if (entity == null) { + PolicyType policyType = PolicyType.fromName(type); + if (policyType == null) { + throw new BadRequestException("Unknown policy type: %s", type); + } + + entity = + new PolicyEntity.Builder( + policyIdentifier.namespace(), policyIdentifier.name(), policyType) + .setCatalogId(catalogId) + .setDescription(description) + .setContent(content) + .setId(getMetaStoreManager().generateNewEntityId(getCurrentPolarisContext()).getId()) + .build(); + + PolicyValidators.validate(entity); + + } else { + throw new AlreadyExistsException("Policy already exists %s", policyIdentifier); + } + + return constructPolicy(createPolicyEntity(policyIdentifier, entity)); + } + + public List<PolicyIdentifier> listPolicies(Namespace namespace, PolicyType policyType) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved namespace '%s'", namespace)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath(); + List<PolicyEntity> policyEntities = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.POLICY, + PolarisEntitySubType.ANY_SUBTYPE) + .getEntities() + .stream() + .map( + polarisEntityActiveRecord -> + PolicyEntity.of( + getMetaStoreManager() + .loadEntity( + getCurrentPolarisContext(), + polarisEntityActiveRecord.getCatalogId(), + polarisEntityActiveRecord.getId(), + polarisEntityActiveRecord.getType()) + .getEntity())) + .filter( + policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) + .toList(); + + List<PolarisEntity.NameAndId> entities = + policyEntities.stream().map(PolarisEntity::nameAndId).toList(); + + return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities).stream() + .map(PolicyIdentifier::fromTableIdentifier) + .toList(); + } + + public Policy loadPolicy(PolicyIdentifier policyIdentifier) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + return constructPolicy(policy); + } + + public Policy updatePolicy( + PolicyIdentifier policyIdentifier, + String newDescription, + String newContent, + int currentPolicyVersion) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy); + int policyVersion = policy.getPolicyVersion(); + if (currentPolicyVersion != policyVersion) { + throw new PolicyVersionMismatchException( + String.format("Policy version mismatch. Current version is %d", policyVersion)); + } + boolean hasUpdate = false; + if (newContent != null) { + newPolicyBuilder.setContent(newContent); + hasUpdate = true; + } + + if (newDescription != null) { + newPolicyBuilder.setDescription(newDescription); + hasUpdate = true; + } + + if (!hasUpdate) { + return constructPolicy(policy); + } + + newPolicyBuilder.setPolicyVersion(policyVersion + 1); + PolicyEntity newPolicyEntity = newPolicyBuilder.build(); + PolicyValidators.validate(newPolicyEntity); + newPolicyEntity = PolicyEntity.of(updatePolicy(policyIdentifier, newPolicyEntity)); + + return constructPolicy(newPolicyEntity); + } + + public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean detachAll) { + PolarisResolvedPathWrapper resolvedEntities = Review Comment: Please add a TODO. here so that we know what will happen, ########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -0,0 +1,300 @@ +/* + * 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.service.catalog.policy; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.CatalogEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException; +import org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException; +import org.apache.polaris.core.policy.validator.PolicyValidators; +import org.apache.polaris.service.types.Policy; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PolicyCatalog { + private static final Logger LOGGER = LoggerFactory.getLogger(PolicyCatalog.class); + + private final CallContext callContext; + private final PolarisResolutionManifestCatalogView resolvedEntityView; + private final CatalogEntity catalogEntity; + private long catalogId = -1; + private PolarisMetaStoreManager metaStoreManager; + + public PolicyCatalog( + PolarisMetaStoreManager metaStoreManager, + CallContext callContext, + PolarisResolutionManifestCatalogView resolvedEntityView) { + this.callContext = callContext; + this.resolvedEntityView = resolvedEntityView; + this.catalogEntity = + CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + this.catalogId = catalogEntity.getId(); + this.metaStoreManager = metaStoreManager; + } + + public Policy createPolicy( + PolicyIdentifier policyIdentifier, String type, String description, String content) { + PolarisResolvedPathWrapper resolvedPolicyEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity entity = + PolicyEntity.of( + resolvedPolicyEntities == null ? null : resolvedPolicyEntities.getRawLeafEntity()); + + if (entity == null) { + PolicyType policyType = PolicyType.fromName(type); + if (policyType == null) { + throw new BadRequestException("Unknown policy type: %s", type); + } + + entity = + new PolicyEntity.Builder( + policyIdentifier.namespace(), policyIdentifier.name(), policyType) + .setCatalogId(catalogId) + .setDescription(description) + .setContent(content) + .setId(getMetaStoreManager().generateNewEntityId(getCurrentPolarisContext()).getId()) + .build(); + + PolicyValidators.validate(entity); + + } else { + throw new AlreadyExistsException("Policy already exists %s", policyIdentifier); + } + + return constructPolicy(createPolicyEntity(policyIdentifier, entity)); + } + + public List<PolicyIdentifier> listPolicies(Namespace namespace, PolicyType policyType) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved namespace '%s'", namespace)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath(); + List<PolicyEntity> policyEntities = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.POLICY, + PolarisEntitySubType.ANY_SUBTYPE) + .getEntities() + .stream() + .map( + polarisEntityActiveRecord -> + PolicyEntity.of( + getMetaStoreManager() + .loadEntity( + getCurrentPolarisContext(), + polarisEntityActiveRecord.getCatalogId(), + polarisEntityActiveRecord.getId(), + polarisEntityActiveRecord.getType()) + .getEntity())) + .filter( + policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) + .toList(); + + List<PolarisEntity.NameAndId> entities = + policyEntities.stream().map(PolarisEntity::nameAndId).toList(); + + return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities).stream() + .map(PolicyIdentifier::fromTableIdentifier) + .toList(); + } + + public Policy loadPolicy(PolicyIdentifier policyIdentifier) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + return constructPolicy(policy); + } + + public Policy updatePolicy( + PolicyIdentifier policyIdentifier, + String newDescription, + String newContent, + int currentPolicyVersion) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy); + int policyVersion = policy.getPolicyVersion(); + if (currentPolicyVersion != policyVersion) { + throw new PolicyVersionMismatchException( + String.format("Policy version mismatch. Current version is %d", policyVersion)); + } + boolean hasUpdate = false; Review Comment: could you add some comment here about how the algorithm about how updating is handled? it is not so straightforward from the code ########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -0,0 +1,300 @@ +/* + * 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.service.catalog.policy; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.CatalogEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; +import org.apache.polaris.core.policy.PolicyEntity; +import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException; +import org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException; +import org.apache.polaris.core.policy.validator.PolicyValidators; +import org.apache.polaris.service.types.Policy; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PolicyCatalog { + private static final Logger LOGGER = LoggerFactory.getLogger(PolicyCatalog.class); + + private final CallContext callContext; + private final PolarisResolutionManifestCatalogView resolvedEntityView; + private final CatalogEntity catalogEntity; + private long catalogId = -1; + private PolarisMetaStoreManager metaStoreManager; + + public PolicyCatalog( + PolarisMetaStoreManager metaStoreManager, + CallContext callContext, + PolarisResolutionManifestCatalogView resolvedEntityView) { + this.callContext = callContext; + this.resolvedEntityView = resolvedEntityView; + this.catalogEntity = + CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + this.catalogId = catalogEntity.getId(); + this.metaStoreManager = metaStoreManager; + } + + public Policy createPolicy( + PolicyIdentifier policyIdentifier, String type, String description, String content) { + PolarisResolvedPathWrapper resolvedPolicyEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity entity = + PolicyEntity.of( + resolvedPolicyEntities == null ? null : resolvedPolicyEntities.getRawLeafEntity()); + + if (entity == null) { + PolicyType policyType = PolicyType.fromName(type); + if (policyType == null) { + throw new BadRequestException("Unknown policy type: %s", type); + } + + entity = + new PolicyEntity.Builder( + policyIdentifier.namespace(), policyIdentifier.name(), policyType) + .setCatalogId(catalogId) + .setDescription(description) + .setContent(content) + .setId(getMetaStoreManager().generateNewEntityId(getCurrentPolarisContext()).getId()) + .build(); + + PolicyValidators.validate(entity); + + } else { + throw new AlreadyExistsException("Policy already exists %s", policyIdentifier); + } + + return constructPolicy(createPolicyEntity(policyIdentifier, entity)); + } + + public List<PolicyIdentifier> listPolicies(Namespace namespace, PolicyType policyType) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved namespace '%s'", namespace)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath(); + List<PolicyEntity> policyEntities = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.POLICY, + PolarisEntitySubType.ANY_SUBTYPE) + .getEntities() + .stream() + .map( + polarisEntityActiveRecord -> + PolicyEntity.of( + getMetaStoreManager() + .loadEntity( + getCurrentPolarisContext(), + polarisEntityActiveRecord.getCatalogId(), + polarisEntityActiveRecord.getId(), + polarisEntityActiveRecord.getType()) + .getEntity())) + .filter( + policyEntity -> policyType == null || policyEntity.getPolicyType() == policyType) + .toList(); + + List<PolarisEntity.NameAndId> entities = + policyEntities.stream().map(PolarisEntity::nameAndId).toList(); + + return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities).stream() + .map(PolicyIdentifier::fromTableIdentifier) + .toList(); + } + + public Policy loadPolicy(PolicyIdentifier policyIdentifier) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + return constructPolicy(policy); + } + + public Policy updatePolicy( + PolicyIdentifier policyIdentifier, + String newDescription, + String newContent, + int currentPolicyVersion) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + + PolicyEntity policy = + PolicyEntity.of(resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); + + if (policy == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy); + int policyVersion = policy.getPolicyVersion(); + if (currentPolicyVersion != policyVersion) { + throw new PolicyVersionMismatchException( + String.format("Policy version mismatch. Current version is %d", policyVersion)); + } + boolean hasUpdate = false; + if (newContent != null) { + newPolicyBuilder.setContent(newContent); + hasUpdate = true; + } + + if (newDescription != null) { + newPolicyBuilder.setDescription(newDescription); + hasUpdate = true; + } + + if (!hasUpdate) { + return constructPolicy(policy); + } + + newPolicyBuilder.setPolicyVersion(policyVersion + 1); + PolicyEntity newPolicyEntity = newPolicyBuilder.build(); + PolicyValidators.validate(newPolicyEntity); + newPolicyEntity = PolicyEntity.of(updatePolicy(policyIdentifier, newPolicyEntity)); + + return constructPolicy(newPolicyEntity); + } + + public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean detachAll) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + policyIdentifier, PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE); + if (resolvedEntities == null) { + throw new NoSuchPolicyException(String.format("Policy does not exist: %s", policyIdentifier)); + } + + List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath(); + PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); + + DropEntityResult dropEntityResult = + getMetaStoreManager() + .dropEntityIfExists( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + leafEntity, + Map.of(), + false); + + return dropEntityResult.isSuccess(); + } + + private PolicyEntity createPolicyEntity(PolicyIdentifier identifier, PolarisEntity entity) { + PolarisResolvedPathWrapper resolvedParent = + resolvedEntityView.getResolvedPath(identifier.namespace()); + if (resolvedParent == null) { + // Illegal state because the namespace should've already been in the static resolution set. + throw new IllegalStateException( + String.format("Failed to fetch resolved parent for Policy '%s'", identifier)); + } + + List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath(); + if (entity.getParentId() <= 0) { + entity = + new PolarisEntity.Builder(entity) + .setParentId(resolvedParent.getRawLeafEntity().getId()) + .build(); + } + + entity = + new PolarisEntity.Builder(entity).setCreateTimestamp(System.currentTimeMillis()).build(); + + PolarisEntity returnedEntity = + PolarisEntity.of( + getMetaStoreManager() + .createEntityIfNotExists( + getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), entity)); + + LOGGER.debug("Created Policy entity {} with Identifier {}", entity, identifier); + if (returnedEntity == null) { + throw new IllegalStateException("Failed to create Policy entity"); + } + + return PolicyEntity.of(returnedEntity); + } + + private PolarisEntity updatePolicy(PolicyIdentifier identifier, PolarisEntity entity) { + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getResolvedPath(identifier, entity.getType(), entity.getSubType()); + if (resolvedEntities == null) { + throw new IllegalStateException( + String.format("Failed to fetch resolved PolicyIdentifier '%s'", identifier)); Review Comment: Actually, that seems a duplicated logic as above? and it seems a function called just by updatePolicy, and the code seems fairly simple, i don't think we need a separate function for this maybe just merge it with the parent updatePolicy function -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org