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


##########
common/src/main/java/org/apache/gravitino/dto/requests/TagValuesAssociateRequest.java:
##########
@@ -82,32 +85,28 @@ public TagValue[] tagValuesToRemove() {
   @Override
   public void validate() throws IllegalArgumentException {
     Preconditions.checkArgument(
-        tagsToAdd != null || tagsToRemove != null,
-        "tagsToAdd and tagsToRemove cannot both be null");
+        tagsToAdd.length > 0 || tagsToRemove.length > 0,
+        "tagsToAdd and tagsToRemove cannot both be empty");
 
     validateTagValues(tagsToAdd, "tagsToAdd");

Review Comment:
   Fixed in e5aaed4e5. `TagValuesAssociateRequest.validate()` now rejects exact 
`(name, value)` overlap across add/remove arrays, with DTO-level regression 
coverage for valued and valueless pairs. Different values for the same tag 
remain valid.



##########
server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociateTagAuthorizationExecutor.java:
##########
@@ -63,37 +66,51 @@ public boolean execute(AuthorizationRequestContext context) 
throws Exception {
     context.setOriginalAuthorizationExpression(expression);
     Entity.EntityType targetType =
         Entity.EntityType.TAG; // Tags are the only supported batch target here
+
     Preconditions.checkArgument(
-        request instanceof TagsAssociateRequest,
+        request instanceof TagsAssociateRequest || request instanceof 
TagValuesAssociateRequest,
         "Only tag can use AssociateTagAuthorizationExecutor, please contact 
the administrator.");
-    TagsAssociateRequest tagsAssociateRequest = (TagsAssociateRequest) request;
-    tagsAssociateRequest.validate();
+
+    TagValue[] tagsToAdd;
+    TagValue[] tagsToRemove;
+    if (request instanceof TagsAssociateRequest) {
+      TagsAssociateRequest tagsAssociateRequest = (TagsAssociateRequest) 
request;
+      tagsAssociateRequest.validate();
+      tagsToAdd = toNoValue(tagsAssociateRequest.getTagsToAdd());
+      tagsToRemove = toNoValue(tagsAssociateRequest.getTagsToRemove());
+    } else {
+      TagValuesAssociateRequest tagValuesAssociateRequest = 
(TagValuesAssociateRequest) request;
+      tagValuesAssociateRequest.validate();

Review Comment:
   Fixed in e5aaed4e5. The authorization executor now extracts tag names 
without performing full request validation, leaving invalid V2 body validation 
to the resource layer. Added executor coverage and an interception/resource 
regression that returns 400 instead of authorization 500.



##########
server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java:
##########
@@ -260,6 +257,35 @@ public Response associateTagsForObject(
       @PathParam("fullName") @AuthorizationFullName String fullName,
       @AuthorizationRequest(type = 
AuthorizationRequest.RequestType.ASSOCIATE_TAG)
           TagsAssociateRequest request) {
+    return associateTagsForObjectInternal(metalake, type, fullName, request);
+  }
+
+  /**
+   * Associates tag values with a metadata object using the v2 request 
representation.
+   *
+   * @param metalake The metalake name.
+   * @param type The metadata object type.
+   * @param fullName The metadata object full name.
+   * @param request The tag values association request.
+   * @return The response containing associated tag names.
+   */
+  @POST
+  @Produces("application/vnd.gravitino.v2+json")
+  @Timed(name = "associate-object-tags." + MetricNames.HTTP_PROCESS_DURATION, 
absolute = true)
+  @ResponseMetered(name = "associate-object-tags", absolute = true)
+  @AuthorizationExpression(expression = CAN_ACCESS_METADATA_AND_TAG)
+  public Response associateTagValuesForObject(
+      @PathParam("metalake") @AuthorizationMetadata(type = 
Entity.EntityType.METALAKE)
+          String metalake,
+      @PathParam("type") @AuthorizationObjectType String type,
+      @PathParam("fullName") @AuthorizationFullName String fullName,
+      @AuthorizationRequest(type = 
AuthorizationRequest.RequestType.ASSOCIATE_TAG)
+          TagValuesAssociateRequest request) {
+    return associateTagsForObjectInternal(metalake, type, fullName, request);

Review Comment:
   Fixed in e5aaed4e5. V2 association now consumes 
`application/vnd.gravitino.v2+json`, the Java client sends that content type 
plus Accept, OpenAPI documents V1 and V2 under separate request media types, 
and V2 success responses are returned with the negotiated V2 vendor media type. 
Added REST/client tests for the media-type behavior.



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