roryqi commented on code in PR #12855:
URL: https://github.com/apache/gravitino/pull/12855#discussion_r3930671907
##########
core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java:
##########
@@ -238,6 +249,71 @@ public MetadataObject[] listMetadataObjectsForTag(String
metalake, String name,
}
}
+ @Override
+ public RelationalEntity<?>[] listPolicyAssociationsForTag(String metalake,
String name) {
+ return dispatcher.listPolicyAssociationsForTag(metalake, name);
+ }
+
+ @Override
+ public void addPolicyForTag(
+ String metalake, String tagName, String policyName,
PolicyAssociationSelector selector) {
+ String user = PrincipalUtils.getCurrentUserName();
+ PolicyTagAssociationInfo previousAssociation;
+ try {
+ previousAssociation = findPolicyAssociation(metalake, tagName,
policyName);
Review Comment:
Fixed in ffabdc7d0. I removed the dispatcher-level association snapshot
lookup and deleted PolicyTagAssociationInfo. The add/remove event paths no
longer call listPolicyAssociationsForTag, and success events no longer claim a
previous or removed association. Tests now verify that mutations perform no
relation-list scan.
##########
core/src/main/java/org/apache/gravitino/audit/v2/CompatibilityUtils.java:
##########
@@ -75,6 +75,8 @@ public class CompatibilityUtils {
Operation.LIST_TAGS_INFO_FOR_METADATA_OBJECT)
.put(OperationType.LIST_METADATA_OBJECTS_FOR_TAG,
Operation.LIST_METADATA_OBJECTS_FOR_TAG)
.put(OperationType.LIST_TAGS_INFO, Operation.LIST_TAGS_INFO)
+ .put(OperationType.ADD_POLICY_FOR_TAG, Operation.ADD_POLICY_FOR_TAG)
Review Comment:
Fixed in ffabdc7d0. The v2 customInfo now includes policyName for add/remove
and the requested selector for add. TestSimpleAuditLogV2 verifies the actual
formatted output. The deprecated v1 schema has no custom-info field, so it
remains tag-identifier-only; this limitation is now documented in the design
and pinned in TestOperation.
##########
core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java:
##########
@@ -238,6 +249,71 @@ public MetadataObject[] listMetadataObjectsForTag(String
metalake, String name,
}
}
+ @Override
+ public RelationalEntity<?>[] listPolicyAssociationsForTag(String metalake,
String name) {
+ return dispatcher.listPolicyAssociationsForTag(metalake, name);
+ }
+
+ @Override
+ public void addPolicyForTag(
+ String metalake, String tagName, String policyName,
PolicyAssociationSelector selector) {
+ String user = PrincipalUtils.getCurrentUserName();
+ PolicyTagAssociationInfo previousAssociation;
+ try {
+ previousAssociation = findPolicyAssociation(metalake, tagName,
policyName);
+ } catch (Exception e) {
+ eventBus.dispatchEvent(
Review Comment:
Fixed in ffabdc7d0. Pre-events are now intent-only and dispatched before any
business access. The snapshot read has been removed entirely. Mutation
exceptions still produce failure events without being masked, and the new
ordering/veto tests verify PRE -> mutation -> SUCCESS and that a pre-listener
veto prevents all delegate access.
##########
core/src/main/java/org/apache/gravitino/listener/api/event/AddPolicyForTagEvent.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.listener.api.event;
+
+import java.util.Optional;
+import javax.annotation.Nullable;
+import org.apache.gravitino.annotation.DeveloperApi;
+import org.apache.gravitino.listener.api.info.PolicyTagAssociationInfo;
+import org.apache.gravitino.policy.PolicyAssociationSelector;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/** Represents an event triggered after successfully adding a policy to a tag.
*/
+@DeveloperApi
+public final class AddPolicyForTagEvent extends TagEvent {
+ @Nullable private final PolicyTagAssociationInfo previousAssociation;
Review Comment:
Fixed in ffabdc7d0. I chose the existing create-only contract: adding an
existing pair raises PolicyAlreadyAssociatedException regardless of whether the
selector is the same or different. The successful add event no longer exposes
previousAssociation, and the design document now specifies POST/create-only
behavior with 409 Conflict for duplicates. TestTagManager covers duplicate adds
with both the same and a different selector.
--
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]
