mchades commented on code in PR #12384:
URL: https://github.com/apache/gravitino/pull/12384#discussion_r3758506579
##########
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:
**[P2] Keep explicit `Accept` preferences and pre-resource V2 errors intact**
The new fallback still treats every `Accept` value without a Gravitino
vendor token as if it were absent or wildcard. For example, with `Content-Type:
application/vnd.gravitino.v2+json` and explicit `Accept: application/json`,
`VersioningFilter` replaces the client preference with V2 and returns a
representation the client did not accept instead of allowing normal 406
negotiation. The parser also still stops at the first vendor token and ignores
quality/fallback ranges (for example, `v3;q=0, v2`). In addition,
`withMediaType` only covers exceptions raised after the resource is invoked:
malformed JSON is handled earlier by the JSON exception mappers, which still
use `Utils.illegalArguments` and return `application/json`, although the
updated OpenAPI 400 response promises the V2 media type for malformed requests.
Please parse the actual Accept ranges and apply the negotiated media type in a
response/exception layer that also covers deserialization failures; add
explicit non-wildcard
Accept and malformed-JSON regressions.
##########
server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java:
##########
@@ -270,26 +299,65 @@ public Response associateTagsForObject(
httpRequest,
() -> {
request.validate();
- MetadataObject object =
- MetadataObjects.parse(
- fullName,
MetadataObject.Type.valueOf(type.toUpperCase(Locale.ROOT)));
+ MetadataObject object = parseMetadataObject(type, fullName);
String[] tagNames =
tagDispatcher.associateTagsForMetadataObject(
metalake, object, request.getTagsToAdd(),
request.getTagsToRemove());
tagNames = tagNames == null ? new String[0] : tagNames;
- LOG.info(
- "Associated tags: {} for object type: {}, full name: {} under
metalake: {}",
- Arrays.toString(tagNames),
- type,
- fullName,
- metalake);
+ logAssociatedTags(type, fullName, metalake, tagNames);
return Utils.ok(new NameListResponse(tagNames));
});
} catch (Exception e) {
return ExceptionHandlers.handleTagException(OperationType.ASSOCIATE, "",
fullName, e);
}
}
+ private Response associateTagValuesForObjectInternal(
+ String metalake, String type, String fullName, TagValuesAssociateRequest
request) {
+ LOG.info(
+ "Received associate tag values request for object type: {}, full name:
{} under metalake: {}",
+ type,
+ fullName,
+ metalake);
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ request.validate();
+ MetadataObject object = parseMetadataObject(type, fullName);
+ String[] tagNames =
+ tagDispatcher.associateTagValuesForMetadataObject(
Review Comment:
**[P2] Return 404 when a V2 pair references a missing tag**
This V2 path still inherits the legacy name-only “ignore missing tags”
behavior. `TagMetaService` resolves only existing tag definitions and simply
`continue`s when the requested name is absent, so `tagsToAdd:
[{"name":"typo","value":"finance"}]` (and the corresponding removal) returns
200 with no change. The tag-assignment-values contract requires a missing tag
referenced by either V2 array to return 404; silently succeeding makes client
typos look applied. Please validate all V2 pair names before updating while
preserving the V1 ignore-missing behavior, and add an endpoint regression.
--
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]
