sfc-gh-ygu commented on code in PR #1104:
URL: https://github.com/apache/polaris/pull/1104#discussion_r2015128453


##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java:
##########
@@ -1821,4 +1824,176 @@ public Map<String, String> getInternalPropertyMap(
     // return the result
     return new ResolvedEntityResult(entity, 
entityVersions.getGrantRecordsVersion(), grantRecords);
   }
+
+  @Override
+  public @Nonnull AttachmentResult attachPolicyToEntity(
+      @Nonnull PolarisCallContext callCtx,
+      @Nonnull List<PolarisEntityCore> targetCatalogPath,
+      @Nonnull PolarisEntityCore target,
+      @Nonnull List<PolarisEntityCore> policyCatalogPath,
+      @Nonnull PolicyEntity policy,
+      Map<String, String> parameters) {
+    // get metastore we should be using
+    BasePersistence ms = callCtx.getMetaStore();
+
+    PolicyType policyType = PolicyType.fromCode(policy.getPolicyTypeCode());
+    if (policyType == null) {
+      // This should never happen
+      return new AttachmentResult(
+          BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unknown policy 
type");
+    }
+
+    // TODO: this may make it necessary to push down this logic to policy 
persistence layer
+    if (policyType.isInheritable()) {
+      // Verify that there is no other mapping of the same type but different 
entity
+      List<PolarisPolicyMappingRecord> existingRecordsOfSameType =
+          ms.loadPoliciesOnTargetByType(
+              callCtx, target.getCatalogId(), target.getId(), 
policy.getPolicyTypeCode());
+      if (existingRecordsOfSameType.size() > 1) {
+        return new AttachmentResult(
+            
BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null);
+      } else if (existingRecordsOfSameType.size() == 1) {
+        PolarisPolicyMappingRecord existingRecord = 
existingRecordsOfSameType.get(0);
+        if (existingRecord.getPolicyId() != policy.getId()
+            || existingRecord.getPolicyCatalogId() != policy.getCatalogId()) {
+          return new AttachmentResult(
+              
BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS, null);
+        }
+      }
+    }
+
+    PolarisPolicyMappingRecord mappingRecord =
+        this.persistNewPolicyMappingRecord(callCtx, ms, target, policy, 
parameters);
+    return new AttachmentResult(mappingRecord);
+  }
+
+  @Override
+  public @Nonnull AttachmentResult detachPolicyFromEntity(
+      @Nonnull PolarisCallContext callCtx,
+      @Nonnull List<PolarisEntityCore> catalogPath,
+      @Nonnull PolarisEntityCore target,
+      @Nonnull List<PolarisEntityCore> policyCatalogPath,
+      @Nonnull PolicyEntity policy) {
+    // get metastore we should be using
+    BasePersistence ms = callCtx.getMetaStore();
+
+    PolarisPolicyMappingRecord mappingRecord =
+        ms.lookupPolicyMappingRecord(
+            callCtx,
+            target.getCatalogId(),
+            target.getId(),
+            policy.getPolicyTypeCode(),
+            policy.getCatalogId(),
+            policy.getId());
+    if (mappingRecord == null) {
+      return new 
AttachmentResult(BaseResult.ReturnStatus.POLICY_MAPPING_NOT_FOUND, null);
+    }
+
+    ms.deleteFromPolicyMappingRecords(callCtx, mappingRecord);
+
+    return new AttachmentResult(mappingRecord);
+  }
+
+  @Override
+  public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntity(
+      @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore target) {
+    // get metastore we should be using
+    BasePersistence ms = callCtx.getMetaStore();
+
+    int grantRecordVersion =
+        ms.lookupEntityGrantRecordsVersion(callCtx, target.getCatalogId(), 
target.getId());
+    if (grantRecordVersion == 0) {
+      // Target entity does not exists
+      return new 
LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null);
+    }
+
+    final List<PolarisPolicyMappingRecord> policyMappingRecords =
+        ms.loadAllPoliciesOnTarget(callCtx, target.getCatalogId(), 
target.getId());
+
+    List<PolicyEntity> policyEntities =
+        loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords);
+    return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities);
+  }
+
+  @Override
+  public @Nonnull LoadPolicyMappingsResult loadPoliciesOnEntityByType(
+      @Nonnull PolarisCallContext callCtx,
+      @Nonnull PolarisEntityCore target,
+      @Nonnull PolicyType policyType) {
+    // get metastore we should be using
+    BasePersistence ms = callCtx.getMetaStore();
+
+    int grantRecordVersion =
+        ms.lookupEntityGrantRecordsVersion(callCtx, target.getCatalogId(), 
target.getId());
+    if (grantRecordVersion == 0) {
+      // Target entity does not exists
+      return new 
LoadPolicyMappingsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null);
+    }
+
+    final List<PolarisPolicyMappingRecord> policyMappingRecords =
+        ms.loadAllPoliciesOnTarget(callCtx, target.getCatalogId(), 
target.getId());
+
+    List<PolicyEntity> policyEntities =
+        loadPoliciesFromMappingRecords(callCtx, ms, policyMappingRecords);
+    return new LoadPolicyMappingsResult(policyMappingRecords, policyEntities);
+  }
+
+  /**
+   * Create and persist a new policy mapping record
+   *
+   * @param callCtx call context
+   * @param ms meta store in read/write mode
+   * @param target target
+   * @param policy policy
+   * @param parameters optional parameters
+   * @return new policy mapping record which was created and persisted
+   */

Review Comment:
   I'm OK with keeping them if it helps for code reader. My general take on 
private methods is that we can be more relaxed about JavaDocs—it's totally fine 
to add one if it adds clarity, but it's also fine to skip it if the method is 
simple and self-explanatory.



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