Skip to content

Commit

Permalink
[apache#6254]Improve: Rename tag to a name already exists in the curr…
Browse files Browse the repository at this point in the history
…ent metalake, it should return 409 instead of 500

### What changes were proposed in this pull request?
1. Rename tag to a name already exists in the current metalake, it should return 409 instead of 500
2. The error message should show the correct conflict name
### Why are the changes needed?
### Fix: apache#6254

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
ut
  • Loading branch information
cool9850311 committed Jan 16, 2025
1 parent 393609b commit 89cbe73
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
13 changes: 11 additions & 2 deletions core/src/main/java/org/apache/gravitino/tag/TagManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ public Tag alterTag(String metalake, String name, TagChange... changes)
throw new NoSuchTagException(
"Tag with name %s under metalake %s does not exist", name, metalake);
} catch (EntityAlreadyExistsException e) {
throw new RuntimeException(
"Tag with name " + name + " under metalake " + metalake + " already exists");
throw new TagAlreadyExistsException(
"Tag with name %s under metalake %s already exists", getNewName(changes), metalake);
} catch (IOException ioe) {
LOG.error("Failed to alter tag {} under metalake {}", name, metalake, ioe);
throw new RuntimeException(ioe);
Expand Down Expand Up @@ -352,6 +352,15 @@ public String[] associateTagsForMetadataObject(
}));
}

private String getNewName(TagChange... changes) {
for (TagChange change : changes) {
if (change instanceof TagChange.RenameTag) {
return ((TagChange.RenameTag) change).getNewName();
}
}
return null;
}

private TagEntity updateTagEntity(TagEntity tagEntity, TagChange... changes) {
Map<String, String> props =
tagEntity.properties() == null
Expand Down
12 changes: 12 additions & 0 deletions core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,18 @@ public void testAlterTag() {
Assertions.assertEquals("new comment", removedPropTag.comment());
Map<String, String> expectedProp2 = ImmutableMap.of("k2", "v2");
Assertions.assertEquals(expectedProp2, removedPropTag.properties());

// Test rename tag to an already-existing tag
tagManager.createTag(METALAKE, "tagA", "existing comment", null);
tagManager.createTag(METALAKE, "tagB", "some comment", null);

TagChange renameToExisting = TagChange.rename("tagB");
Exception e =
Assertions.assertThrows(
TagAlreadyExistsException.class,
() -> tagManager.alterTag(METALAKE, "tagA", renameToExisting));
Assertions.assertEquals(
"Tag with name tagB under metalake " + METALAKE + " already exists", e.getMessage());
}

@Test
Expand Down

0 comments on commit 89cbe73

Please sign in to comment.