gh-yzou commented on code in PR #1294:
URL: https://github.com/apache/polaris/pull/1294#discussion_r2027954488


##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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);

Review Comment:
   can we add an UNKONW status in the PolicyType to make things more clear, 
instead of using null, so that we can make a clear distinguish that at there is 
no policyType or the policyType is unknown



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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.getNamespace(), policyIdentifier.getName(), 
policyType)
+              .setCatalogId(catalogId)
+              .setParentId(resolvedParent.getRawLeafEntity().getId())
+              .setDescription(description)
+              .setContent(content)
+              
.setId(metaStoreManager.generateNewEntityId(getCurrentPolarisContext()).getId())
+              .setCreateTimestamp(System.currentTimeMillis())
+              .build();
+
+      PolicyValidators.validate(entity);
+    } else {
+      throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+    }
+
+    EntityResult res =
+        metaStoreManager.createEntityIfNotExists(
+            getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), 
entity);
+
+    if (!res.isSuccess()) {
+      switch (res.getReturnStatus()) {
+        case BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS:
+          throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+
+        default:
+          throw new IllegalStateException(
+              String.format(
+                  "Unknown error status for identifier %s: %s with extraInfo: 
%s",
+                  policyIdentifier, res.getReturnStatus(), 
res.getExtraInformation()));
+      }
+    }
+
+    PolicyEntity resultEntity = PolicyEntity.of(res.getEntity());
+    LOGGER.debug(
+        "Created Policy entity {} with PolicyIdentifier {}", resultEntity, 
policyIdentifier);
+    return constructPolicy(resultEntity);
+  }
+
+  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 =
+        metaStoreManager
+            .listEntities(
+                getCurrentPolarisContext(),
+                PolarisEntity.toCoreList(catalogPath),
+                PolarisEntityType.POLICY,
+                PolarisEntitySubType.ANY_SUBTYPE)
+            .getEntities()
+            .stream()
+            .map(
+                polarisEntityActiveRecord ->
+                    PolicyEntity.of(
+                        metaStoreManager
+                            .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 entities.stream()
+        .map(
+            entity ->
+                PolicyIdentifier.builder()
+                    .setNamespace(namespace)
+                    .setName(entity.getName())
+                    .build())
+        .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));
+    }
+
+    // Verify that the current version of the policy matches the version that 
the user is trying to
+    // update
+    int policyVersion = policy.getPolicyVersion();
+    if (currentPolicyVersion != policyVersion) {
+      throw new PolicyVersionMismatchException(
+          String.format("Policy version mismatch. Current version is %d", 
policyVersion));

Review Comment:
   and let's also add the currentPolicyVersion in the message, like (expected 
Version is %d, currentPolicyVersion)



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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.getNamespace(), policyIdentifier.getName(), 
policyType)
+              .setCatalogId(catalogId)
+              .setParentId(resolvedParent.getRawLeafEntity().getId())
+              .setDescription(description)
+              .setContent(content)
+              
.setId(metaStoreManager.generateNewEntityId(getCurrentPolarisContext()).getId())
+              .setCreateTimestamp(System.currentTimeMillis())
+              .build();
+
+      PolicyValidators.validate(entity);
+    } else {
+      throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+    }
+
+    EntityResult res =
+        metaStoreManager.createEntityIfNotExists(
+            getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), 
entity);
+
+    if (!res.isSuccess()) {
+      switch (res.getReturnStatus()) {
+        case BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS:
+          throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+
+        default:
+          throw new IllegalStateException(
+              String.format(
+                  "Unknown error status for identifier %s: %s with extraInfo: 
%s",
+                  policyIdentifier, res.getReturnStatus(), 
res.getExtraInformation()));
+      }
+    }
+
+    PolicyEntity resultEntity = PolicyEntity.of(res.getEntity());
+    LOGGER.debug(
+        "Created Policy entity {} with PolicyIdentifier {}", resultEntity, 
policyIdentifier);
+    return constructPolicy(resultEntity);
+  }
+
+  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 =
+        metaStoreManager
+            .listEntities(
+                getCurrentPolarisContext(),
+                PolarisEntity.toCoreList(catalogPath),
+                PolarisEntityType.POLICY,
+                PolarisEntitySubType.ANY_SUBTYPE)
+            .getEntities()
+            .stream()
+            .map(
+                polarisEntityActiveRecord ->
+                    PolicyEntity.of(
+                        metaStoreManager
+                            .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 entities.stream()
+        .map(
+            entity ->
+                PolicyIdentifier.builder()
+                    .setNamespace(namespace)
+                    .setName(entity.getName())
+                    .build())
+        .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));
+    }
+
+    // Verify that the current version of the policy matches the version that 
the user is trying to
+    // update
+    int policyVersion = policy.getPolicyVersion();
+    if (currentPolicyVersion != policyVersion) {
+      throw new PolicyVersionMismatchException(
+          String.format("Policy version mismatch. Current version is %d", 
policyVersion));

Review Comment:
   same, the String.format seems uncessary here



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));

Review Comment:
   should it be NoSuchNamespaceException Error?



##########
api/polaris-catalog-service/src/main/java/org/apache/polaris/service/types/PolicyIdentifier.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.annotations.ApiModelProperty;
+import java.util.Objects;
+import org.apache.iceberg.catalog.Namespace;
+
+/**
+ * Represents a modified version of the PolicyIdentifier that is different 
from the one generated by
+ * the OpenAPI generator
+ *
+ * <p>This version uses {@link org.apache.iceberg.catalog.Namespace} instead a 
list of strings for
+ * namespace field.
+ */
+public class PolicyIdentifier {
+
+  private final Namespace namespace;

Review Comment:
   One way we could potentially fix the problem is to add a Namespace 
definition under schema that whose type is object, and then directly link to 
the iceberg namespace.
   
   And in your policyId, link to this newly defined Namespace, during the 
generation, map the namespace to Iceberg namespace class. That could 
potentially fix the generation problem.
   
   We can also try to do this in a follow up PR, but let's make sure we follow 
up on this.



##########
api/polaris-catalog-service/src/main/java/org/apache/polaris/service/types/PolicyIdentifier.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.annotations.ApiModelProperty;
+import java.util.Objects;
+import org.apache.iceberg.catalog.Namespace;
+
+/**
+ * Represents a modified version of the PolicyIdentifier that is different 
from the one generated by
+ * the OpenAPI generator
+ *
+ * <p>This version uses {@link org.apache.iceberg.catalog.Namespace} instead a 
list of strings for
+ * namespace field.

Review Comment:
   nit: also comment here about why are we doing this today, for example:
   ```
   the open api generation inlines the namespace definition, generates a 
list<String> directly, instead of generating a Namespace class.
   ```



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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.getNamespace(), policyIdentifier.getName(), 
policyType)
+              .setCatalogId(catalogId)
+              .setParentId(resolvedParent.getRawLeafEntity().getId())
+              .setDescription(description)
+              .setContent(content)
+              
.setId(metaStoreManager.generateNewEntityId(getCurrentPolarisContext()).getId())
+              .setCreateTimestamp(System.currentTimeMillis())
+              .build();
+
+      PolicyValidators.validate(entity);
+    } else {
+      throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+    }
+
+    EntityResult res =
+        metaStoreManager.createEntityIfNotExists(
+            getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), 
entity);
+
+    if (!res.isSuccess()) {
+      switch (res.getReturnStatus()) {
+        case BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS:
+          throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+
+        default:
+          throw new IllegalStateException(
+              String.format(
+                  "Unknown error status for identifier %s: %s with extraInfo: 
%s",
+                  policyIdentifier, res.getReturnStatus(), 
res.getExtraInformation()));
+      }
+    }
+
+    PolicyEntity resultEntity = PolicyEntity.of(res.getEntity());
+    LOGGER.debug(
+        "Created Policy entity {} with PolicyIdentifier {}", resultEntity, 
policyIdentifier);
+    return constructPolicy(resultEntity);
+  }
+
+  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 =
+        metaStoreManager
+            .listEntities(
+                getCurrentPolarisContext(),
+                PolarisEntity.toCoreList(catalogPath),
+                PolarisEntityType.POLICY,
+                PolarisEntitySubType.ANY_SUBTYPE)
+            .getEntities()
+            .stream()
+            .map(
+                polarisEntityActiveRecord ->
+                    PolicyEntity.of(
+                        metaStoreManager
+                            .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 entities.stream()
+        .map(
+            entity ->
+                PolicyIdentifier.builder()
+                    .setNamespace(namespace)
+                    .setName(entity.getName())
+                    .build())
+        .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));
+    }
+
+    // Verify that the current version of the policy matches the version that 
the user is trying to
+    // update
+    int policyVersion = policy.getPolicyVersion();
+    if (currentPolicyVersion != policyVersion) {
+      throw new PolicyVersionMismatchException(
+          String.format("Policy version mismatch. Current version is %d", 
policyVersion));
+    }
+
+    if (newDescription.equals(policy.getDescription()) && 
newContent.equals(policy.getContent())) {
+      // No need to update the policy if the new description and content are 
the same as the current
+      return constructPolicy(policy);
+    }
+
+    PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy);
+    newPolicyBuilder.setContent(newContent);
+    newPolicyBuilder.setDescription(newDescription);
+    newPolicyBuilder.setPolicyVersion(policyVersion + 1);
+    PolicyEntity newPolicyEntity = newPolicyBuilder.build();
+
+    PolicyValidators.validate(newPolicyEntity);
+
+    List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath();
+    newPolicyEntity =
+        Optional.ofNullable(
+                metaStoreManager
+                    .updateEntityPropertiesIfNotChanged(
+                        getCurrentPolarisContext(),
+                        PolarisEntity.toCoreList(catalogPath),
+                        newPolicyEntity)
+                    .getEntity())
+            .map(PolicyEntity::of)
+            .orElse(null);
+
+    if (newPolicyEntity == null) {
+      throw new IllegalStateException(
+          String.format("Failed to update policy %s", policyIdentifier));

Review Comment:
   no need for String.format



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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.getNamespace(), policyIdentifier.getName(), 
policyType)
+              .setCatalogId(catalogId)
+              .setParentId(resolvedParent.getRawLeafEntity().getId())
+              .setDescription(description)
+              .setContent(content)
+              
.setId(metaStoreManager.generateNewEntityId(getCurrentPolarisContext()).getId())
+              .setCreateTimestamp(System.currentTimeMillis())
+              .build();
+
+      PolicyValidators.validate(entity);
+    } else {
+      throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+    }
+
+    EntityResult res =
+        metaStoreManager.createEntityIfNotExists(
+            getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), 
entity);
+
+    if (!res.isSuccess()) {
+      switch (res.getReturnStatus()) {
+        case BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS:
+          throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+
+        default:
+          throw new IllegalStateException(
+              String.format(
+                  "Unknown error status for identifier %s: %s with extraInfo: 
%s",
+                  policyIdentifier, res.getReturnStatus(), 
res.getExtraInformation()));
+      }
+    }
+
+    PolicyEntity resultEntity = PolicyEntity.of(res.getEntity());
+    LOGGER.debug(
+        "Created Policy entity {} with PolicyIdentifier {}", resultEntity, 
policyIdentifier);
+    return constructPolicy(resultEntity);
+  }
+
+  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 =
+        metaStoreManager
+            .listEntities(
+                getCurrentPolarisContext(),
+                PolarisEntity.toCoreList(catalogPath),
+                PolarisEntityType.POLICY,
+                PolarisEntitySubType.ANY_SUBTYPE)
+            .getEntities()
+            .stream()
+            .map(
+                polarisEntityActiveRecord ->
+                    PolicyEntity.of(
+                        metaStoreManager
+                            .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 entities.stream()
+        .map(
+            entity ->
+                PolicyIdentifier.builder()
+                    .setNamespace(namespace)
+                    .setName(entity.getName())
+                    .build())
+        .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));
+    }
+
+    // Verify that the current version of the policy matches the version that 
the user is trying to
+    // update
+    int policyVersion = policy.getPolicyVersion();
+    if (currentPolicyVersion != policyVersion) {
+      throw new PolicyVersionMismatchException(
+          String.format("Policy version mismatch. Current version is %d", 
policyVersion));
+    }
+
+    if (newDescription.equals(policy.getDescription()) && 
newContent.equals(policy.getContent())) {
+      // No need to update the policy if the new description and content are 
the same as the current
+      return constructPolicy(policy);
+    }
+
+    PolicyEntity.Builder newPolicyBuilder = new PolicyEntity.Builder(policy);
+    newPolicyBuilder.setContent(newContent);
+    newPolicyBuilder.setDescription(newDescription);
+    newPolicyBuilder.setPolicyVersion(policyVersion + 1);
+    PolicyEntity newPolicyEntity = newPolicyBuilder.build();
+
+    PolicyValidators.validate(newPolicyEntity);
+
+    List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath();
+    newPolicyEntity =
+        Optional.ofNullable(
+                metaStoreManager
+                    .updateEntityPropertiesIfNotChanged(
+                        getCurrentPolarisContext(),
+                        PolarisEntity.toCoreList(catalogPath),
+                        newPolicyEntity)
+                    .getEntity())
+            .map(PolicyEntity::of)
+            .orElse(null);
+
+    if (newPolicyEntity == null) {
+      throw new IllegalStateException(
+          String.format("Failed to update policy %s", policyIdentifier));
+    }
+
+    return constructPolicy(newPolicyEntity);
+  }
+
+  public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean 
detachAll) {
+    // TODO: Implement detachAll when we support attach/detach policy
+    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 =
+        metaStoreManager.dropEntityIfExists(
+            getCurrentPolarisContext(),
+            PolarisEntity.toCoreList(catalogPath),
+            leafEntity,
+            Map.of(),
+            false);
+
+    return dropEntityResult.isSuccess();
+  }
+
+  private PolarisCallContext getCurrentPolarisContext() {
+    return callContext.getPolarisCallContext();

Review Comment:
   nit: that seems a simple one line utility function, if there is no strong 
preference, let's just inline the call to reduce unnecessary functions, which 
could reduce the code jumping when looking at the code.



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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.getNamespace(), policyIdentifier.getName(), 
policyType)
+              .setCatalogId(catalogId)
+              .setParentId(resolvedParent.getRawLeafEntity().getId())
+              .setDescription(description)
+              .setContent(content)
+              
.setId(metaStoreManager.generateNewEntityId(getCurrentPolarisContext()).getId())
+              .setCreateTimestamp(System.currentTimeMillis())
+              .build();
+
+      PolicyValidators.validate(entity);
+    } else {
+      throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+    }
+
+    EntityResult res =
+        metaStoreManager.createEntityIfNotExists(
+            getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), 
entity);
+
+    if (!res.isSuccess()) {
+      switch (res.getReturnStatus()) {
+        case BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS:
+          throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+
+        default:
+          throw new IllegalStateException(
+              String.format(
+                  "Unknown error status for identifier %s: %s with extraInfo: 
%s",
+                  policyIdentifier, res.getReturnStatus(), 
res.getExtraInformation()));
+      }
+    }
+
+    PolicyEntity resultEntity = PolicyEntity.of(res.getEntity());
+    LOGGER.debug(
+        "Created Policy entity {} with PolicyIdentifier {}", resultEntity, 
policyIdentifier);
+    return constructPolicy(resultEntity);
+  }
+
+  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 =
+        metaStoreManager
+            .listEntities(
+                getCurrentPolarisContext(),
+                PolarisEntity.toCoreList(catalogPath),
+                PolarisEntityType.POLICY,
+                PolarisEntitySubType.ANY_SUBTYPE)
+            .getEntities()
+            .stream()
+            .map(
+                polarisEntityActiveRecord ->
+                    PolicyEntity.of(
+                        metaStoreManager
+                            .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 entities.stream()
+        .map(
+            entity ->
+                PolicyIdentifier.builder()
+                    .setNamespace(namespace)
+                    .setName(entity.getName())
+                    .build())
+        .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));

Review Comment:
   you shouldn't need String.format here, most exception automatically formats, 
for example here 
https://github.com/apache/polaris/blob/512642319b5a0e4a377dcdab33c40bc1fa0b0c3d/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java#L224



##########
api/polaris-catalog-service/src/main/java/org/apache/polaris/service/types/PolicyIdentifier.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.annotations.ApiModelProperty;
+import java.util.Objects;
+import org.apache.iceberg.catalog.Namespace;
+
+/**
+ * Represents a modified version of the PolicyIdentifier that is different 
from the one generated by
+ * the OpenAPI generator
+ *
+ * <p>This version uses {@link org.apache.iceberg.catalog.Namespace} instead a 
list of strings for
+ * namespace field.

Review Comment:
   not: Add a TODO here to investigate make code generation to generate the 
namespace class



##########
service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * 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.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.BaseResult;
+import org.apache.polaris.core.persistence.dao.entity.DropEntityResult;
+import org.apache.polaris.core.persistence.dao.entity.EntityResult;
+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 resolvedParent =
+        resolvedEntityView.getResolvedPath(policyIdentifier.getNamespace());
+    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'", 
policyIdentifier));
+    }
+
+    List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
+
+    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.getNamespace(), policyIdentifier.getName(), 
policyType)
+              .setCatalogId(catalogId)
+              .setParentId(resolvedParent.getRawLeafEntity().getId())
+              .setDescription(description)
+              .setContent(content)
+              
.setId(metaStoreManager.generateNewEntityId(getCurrentPolarisContext()).getId())
+              .setCreateTimestamp(System.currentTimeMillis())
+              .build();
+
+      PolicyValidators.validate(entity);
+    } else {
+      throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+    }
+
+    EntityResult res =
+        metaStoreManager.createEntityIfNotExists(
+            getCurrentPolarisContext(), PolarisEntity.toCoreList(catalogPath), 
entity);
+
+    if (!res.isSuccess()) {
+      switch (res.getReturnStatus()) {
+        case BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS:
+          throw new AlreadyExistsException("Policy already exists %s", 
policyIdentifier);
+
+        default:
+          throw new IllegalStateException(
+              String.format(
+                  "Unknown error status for identifier %s: %s with extraInfo: 
%s",
+                  policyIdentifier, res.getReturnStatus(), 
res.getExtraInformation()));
+      }
+    }
+
+    PolicyEntity resultEntity = PolicyEntity.of(res.getEntity());
+    LOGGER.debug(
+        "Created Policy entity {} with PolicyIdentifier {}", resultEntity, 
policyIdentifier);
+    return constructPolicy(resultEntity);
+  }
+
+  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 =
+        metaStoreManager
+            .listEntities(
+                getCurrentPolarisContext(),
+                PolarisEntity.toCoreList(catalogPath),
+                PolarisEntityType.POLICY,
+                PolarisEntitySubType.ANY_SUBTYPE)

Review Comment:
   nit, that is just my side question, i see we use NULL_SUBTYPE when calling 
getPassthroughResolvedPath, why we do not also use ANY_SUBTYPE there?



-- 
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

Reply via email to