roryqi commented on code in PR #12384:
URL: https://github.com/apache/gravitino/pull/12384#discussion_r3757298822
##########
server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectTagOperations.java:
##########
@@ -270,12 +296,24 @@ public Response associateTagsForObject(
httpRequest,
() -> {
request.validate();
+ TagValue[] tagsToAdd;
+ TagValue[] tagsToRemove;
+ if (request instanceof TagsAssociateRequest) {
+ TagsAssociateRequest tagsAssociateRequest =
(TagsAssociateRequest) request;
+ tagsToAdd = toNoValue(tagsAssociateRequest.getTagsToAdd());
+ tagsToRemove = toNoValue(tagsAssociateRequest.getTagsToRemove());
+ } else {
+ TagValuesAssociateRequest tagValuesAssociateRequest =
+ (TagValuesAssociateRequest) request;
+ tagsToAdd = tagValuesAssociateRequest.tagValuesToAdd();
+ tagsToRemove = tagValuesAssociateRequest.tagValuesToRemove();
+ }
MetadataObject object =
MetadataObjects.parse(
fullName,
MetadataObject.Type.valueOf(type.toUpperCase(Locale.ROOT)));
String[] tagNames =
- tagDispatcher.associateTagsForMetadataObject(
- metalake, object, request.getTagsToAdd(),
request.getTagsToRemove());
+ tagDispatcher.associateTagValuesForMetadataObject(
Review Comment:
Fixed in e5aaed4e5. V1 name-only removal and V2 pair-specific removal are
now separated; V2 valueless removal only removes the `(tag, null)` pair and is
a no-op when that pair is absent. Added a regression in `TestTagManager` for
valued assignments surviving `TagValue.noValue(tag)` removal.
##########
clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java:
##########
@@ -298,6 +301,42 @@ public void testCreateGetAndListTag() {
Assertions.assertEquals(tag2, tag3);
}
+ @Test
+ public void testAssignmentValues() {
+ String tagName = GravitinoITUtils.genRandomName("tag_it_assignment_value");
+ TagValueConstraint constraint =
TagValueConstraint.ofAllowedValues("finance", "risk");
+ Tag tag = metalake.createTag(tagName, "comment", Collections.emptyMap(),
constraint);
+ Assertions.assertEquals(constraint, tag.valueConstraint());
+
+ TagValue[] values =
+ new TagValue[] {TagValue.of(tagName, "finance"), TagValue.of(tagName,
"risk")};
+ Assertions.assertArrayEquals(
+ new String[] {tagName}, table.supportsTags().associateTags(values,
null));
Review Comment:
Fixed in e5aaed4e5. `RelationUpdate` now carries an explicit value-aware
flag, so V2 all-valueless requests still route through the idempotent
value-aware path while legacy V1 keeps duplicate-failing semantics. Added
repeated no-value coverage in both `TestTagManager` and `TagIT`.
--
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]
