roryqi commented on code in PR #12549:
URL: https://github.com/apache/gravitino/pull/12549#discussion_r3850845356


##########
api/src/main/java/org/apache/gravitino/policy/PolicyOperations.java:
##########
@@ -106,4 +106,27 @@ Policy alterPolicy(String name, PolicyChange... changes)
    * @return True if the policy is deleted, false if the policy does not exist.
    */
   boolean deletePolicy(String name);
+
+  /**
+   * Lists tag names directly associated with a policy.
+   *
+   * @param policyName The policy name.
+   * @return The directly associated tag names.
+   * @throws UnsupportedOperationException If listing policy-to-tag 
associations is not supported.
+   */
+  default String[] listTagsForPolicy(String policyName) {
+    throw new UnsupportedOperationException("Listing tags for a policy is not 
supported");
+  }
+
+  /**
+   * Lists detailed tag associations for a policy.
+   *
+   * @param policyName The policy name.
+   * @return The tag associations including selectors.
+   * @throws UnsupportedOperationException If listing policy-to-tag 
associations is not supported.
+   */
+  default PolicyTagAssociation[] listTagAssociationsForPolicy(String 
policyName) {
+    throw new UnsupportedOperationException(
+        "Listing tag associations for a policy is not supported");

Review Comment:
   No. This API returns all direct policy-to-tag associations and does not 
evaluate selectors because there is no metadata object context. I clarified the 
Javadoc in b25ac7066.



##########
api/src/main/java/org/apache/gravitino/policy/PolicyOperations.java:
##########
@@ -106,4 +106,27 @@ Policy alterPolicy(String name, PolicyChange... changes)
    * @return True if the policy is deleted, false if the policy does not exist.
    */
   boolean deletePolicy(String name);
+
+  /**
+   * Lists tag names directly associated with a policy.
+   *
+   * @param policyName The policy name.
+   * @return The directly associated tag names.
+   * @throws UnsupportedOperationException If listing policy-to-tag 
associations is not supported.
+   */
+  default String[] listTagsForPolicy(String policyName) {
+    throw new UnsupportedOperationException("Listing tags for a policy is not 
supported");
+  }

Review Comment:
   Agreed. I removed listTagsForPolicy and the symmetric listPoliciesForTag. 
Callers now use the association APIs, which return PolicyTagAssociation entries 
including their selectors. Updated in b25ac7066.



##########
api/src/main/java/org/apache/gravitino/tag/TagOperations.java:
##########
@@ -115,4 +119,69 @@ Tag alterTag(String name, TagChange... changes)
    * @return True if the tag is deleted, false if the tag does not exist.
    */
   boolean deleteTag(String name);
+
+  /**
+   * Lists policy names directly associated with a tag.
+   *
+   * @param tagName The tag name.
+   * @return The directly associated policy names.
+   * @throws UnsupportedOperationException If listing policy-to-tag 
associations is not supported.
+   */
+  default String[] listPoliciesForTag(String tagName) {
+    throw new UnsupportedOperationException("Listing policies for a tag is not 
supported");
+  }
+
+  /**
+   * Lists detailed policy associations for a tag.
+   *
+   * @param tagName The tag name.
+   * @return The policy associations including selectors.
+   * @throws UnsupportedOperationException If listing policy-to-tag 
associations is not supported.
+   */
+  default PolicyTagAssociation[] listPolicyAssociationsForTag(String tagName) {
+    throw new UnsupportedOperationException(
+        "Listing policy associations for a tag is not supported");
+  }
+
+  /**
+   * Adds one policy association for a tag using {@link AllValuesSelector}. It 
matches by tag
+   * presence regardless of assignment values and does not replace an existing 
association.
+   *
+   * @param tagName The tag name.
+   * @param policyName The policy name.
+   * @return The resulting association.
+   * @throws PolicyAlreadyAssociatedException If the policy is already 
associated with the tag.
+   * @throws UnsupportedOperationException If adding policy-to-tag 
associations is not supported.
+   */
+  default PolicyTagAssociation addPolicyForTag(String tagName, String 
policyName)
+      throws PolicyAlreadyAssociatedException {
+    return addPolicyForTag(tagName, policyName, AllValuesSelector.get());
+  }
+
+  /**
+   * Adds one policy association for a tag. It does not replace an existing 
association.
+   *
+   * @param tagName The tag name.
+   * @param policyName The policy name.
+   * @param selector The non-null policy association selector.
+   * @return The resulting association.
+   * @throws PolicyAlreadyAssociatedException If the policy is already 
associated with the tag.
+   * @throws UnsupportedOperationException If adding policy-to-tag 
associations is not supported.
+   */
+  default PolicyTagAssociation addPolicyForTag(
+      String tagName, String policyName, PolicyAssociationSelector selector)
+      throws PolicyAlreadyAssociatedException {
+    throw new UnsupportedOperationException("Adding a policy for a tag is not 
supported");
+  }
+
+  /**
+   * Removes one policy association from a tag.
+   *
+   * @param tagName The tag name.
+   * @param policyName The policy name.
+   * @throws UnsupportedOperationException If removing policy-to-tag 
associations is not supported.
+   */
+  default void removePolicyFromTag(String tagName, String policyName) {
+    throw new UnsupportedOperationException("Removing a policy from a tag is 
not supported");
+  }

Review Comment:
   No. Each policy-to-tag pair has at most one active association. The selector 
is an attribute of that relation, so tagName and policyName uniquely identify 
the association to remove.



##########
api/src/main/java/org/apache/gravitino/policy/PolicyTagAssociation.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.gravitino.policy;
+
+import org.apache.gravitino.annotation.Evolving;
+import org.apache.gravitino.tag.Tag;
+
+/** A policy-to-tag association and its required assignment selector. */
+@Evolving
+public interface PolicyTagAssociation {
+
+  /**
+   * @return The associated policy.
+   */
+  Policy policy();
+
+  /**
+   * @return The associated tag.
+   */
+  Tag tag();

Review Comment:
   No. This association has no metadata object context, so the returned Tag 
does not carry an assignment. The effective assignment is loaded separately 
when resolving policies for a specific object.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to