From 89cbe730142acb56756e893f2571deb11723bf87 Mon Sep 17 00:00:00 2001 From: Harry Shih Date: Thu, 16 Jan 2025 16:55:59 +0800 Subject: [PATCH] [#6254]Improve: Rename tag to a name already exists in the current 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: #6254 ### Does this PR introduce any user-facing change? No ### How was this patch tested? ut --- .../java/org/apache/gravitino/tag/TagManager.java | 13 +++++++++++-- .../org/apache/gravitino/tag/TestTagManager.java | 12 ++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/tag/TagManager.java b/core/src/main/java/org/apache/gravitino/tag/TagManager.java index c03f6392fb8..767b056064b 100644 --- a/core/src/main/java/org/apache/gravitino/tag/TagManager.java +++ b/core/src/main/java/org/apache/gravitino/tag/TagManager.java @@ -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); @@ -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 props = tagEntity.properties() == null diff --git a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java index 27b4fa84b6c..3ca8626af18 100644 --- a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java +++ b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java @@ -343,6 +343,18 @@ public void testAlterTag() { Assertions.assertEquals("new comment", removedPropTag.comment()); Map 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