roryqi commented on code in PR #12384: URL: https://github.com/apache/gravitino/pull/12384#discussion_r3755670867
########## server/src/test/java/org/apache/gravitino/server/web/filter/authorization/TestAssociateTagAuthorizationExecutor.java: ########## @@ -0,0 +1,98 @@ +/* + * 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.server.web.filter.authorization; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.authorization.AuthorizationRequestContext; +import org.apache.gravitino.dto.requests.TagValuesAssociateRequest; +import org.apache.gravitino.dto.requests.TagsAssociateRequest; +import org.apache.gravitino.server.authorization.annotations.AuthorizationRequest; +import org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator; +import org.apache.gravitino.tag.TagValue; +import org.junit.jupiter.api.Test; + +public class TestAssociateTagAuthorizationExecutor { + + @Test + public void testAuthorizesV1TagNames() throws Exception { + TagsAssociateRequest request = + new TagsAssociateRequest(new String[] {"pii", "data_domain"}, null); + assertAuthorizesAllTags("associateV1", TagsAssociateRequest.class, request); + } + + @Test + public void testAuthorizesV2TagValues() throws Exception { + TagValuesAssociateRequest request = + new TagValuesAssociateRequest( + new TagValue[] {TagValue.noValue("pii"), TagValue.of("data_domain", "finance")}, null); + assertAuthorizesAllTags("associateV2", TagValuesAssociateRequest.class, request); Review Comment: Fixed in 694053b1c. The V1 and V2 authorization tests now exercise one tag in tagsToAdd and one tag in tagsToRemove, so the test fails if the removal branch is not authorized. ########## server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java: ########## @@ -268,14 +272,20 @@ public Response deleteTag( public Response listMetadataObjectsForTag( @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, - @PathParam("tag") @AuthorizationMetadata(type = Entity.EntityType.TAG) String tagName) { - LOG.info("Received list objects for tag: {} under metalake: {}", tagName, metalake); + @PathParam("tag") @AuthorizationMetadata(type = Entity.EntityType.TAG) String tagName, + @QueryParam("value") String value) { Review Comment: Fixed in 694053b1c. The REST endpoint now validates a non-null value filter as nonblank and <= 256 characters before forwarding it, and TestTagOperations covers a valid value, value=, and an over-length value returning 400. ########## server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java: ########## @@ -157,7 +157,11 @@ public Response createTag( request.validate(); Tag tag = tagDispatcher.createTag( - metalake, request.getName(), request.getComment(), request.getProperties()); + metalake, + request.getName(), + request.getComment(), + request.getProperties(), + request.valueConstraint()); Review Comment: Fixed in 694053b1c. The OpenAPI Tag schema now documents allowedValues and assignmentValues, and TagCreateRequest documents allowedValues. ########## 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 694053b1c. The associateTags OpenAPI operation now documents both V1 TagsAssociateRequest and V2 TagValuesAssociateRequest shapes and includes the application/vnd.gravitino.v2+json 200 response media type. ########## server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java: ########## @@ -268,14 +272,20 @@ public Response deleteTag( public Response listMetadataObjectsForTag( @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, - @PathParam("tag") @AuthorizationMetadata(type = Entity.EntityType.TAG) String tagName) { - LOG.info("Received list objects for tag: {} under metalake: {}", tagName, metalake); + @PathParam("tag") @AuthorizationMetadata(type = Entity.EntityType.TAG) String tagName, + @QueryParam("value") String value) { + LOG.info( + "Received list objects for tag: {} and value: {} under metalake: {}", + tagName, + value, + metalake); try { return Utils.doAs( httpRequest, () -> { - MetadataObject[] objects = tagDispatcher.listMetadataObjectsForTag(metalake, tagName); + MetadataObject[] objects = + tagDispatcher.listMetadataObjectsForTag(metalake, tagName, value); Review Comment: Fixed in 694053b1c. The listTagObjects OpenAPI operation now documents the value query parameter with the 256-character limit. -- 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]
