This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 64cb57422a [#10659] fix(core): Correctly handle tag rename conflict in
TagManager (#10661)
64cb57422a is described below
commit 64cb57422a67882db012473e245fcf9bef9ad406
Author: Chisom Uma <[email protected]>
AuthorDate: Thu Apr 23 13:36:23 2026 +0100
[#10659] fix(core): Correctly handle tag rename conflict in TagManager
(#10661)
### What changes were proposed in this pull request?
This PR improves the error handling in `TagManager.alterTag` when a tag
is renamed to a name that already exists in the same metalake.
Specifically:
- Caught `EntityAlreadyExistsException` during the [alterTag]
TagManager.java operation.
- Added logic to extract the intended `newName` from the [TagChange]
TagChange.java array.
- Now throws a domain-specific `TagAlreadyExistsException` with a clear
message, ensuring the server returns an HTTP 409 (Conflict) instead of
an HTTP 500 (Internal Server Error).
### Why are the changes needed?
Previously, renaming a tag to an existing name caused the server to
return a generic `RuntimeException`, which surfaced as an HTTP 500 error
to the client. This is a valid user mistake that should be handled as a
conflict (HTTP 409) with a descriptive error message.
Fix: #10659
### Does this PR introduce _any_ user-facing change?
Yes, it changes the error response when a tag rename conflict occurs:
- **Old behavior**: Returns HTTP 500 (Internal Server Error).
- **New behavior**: Returns HTTP 409 (Conflict) with the message: `"Tag
with name <newName> under metalake <metalake> already exists"`.
### How was this patch tested?
Added a new unit test [testAlterTagRenameToExistingTag]
TestTagManager.java
`core/src/test/java/org/apache/gravitino/tag/TestTagManager.java` that
specifically covers this scenario:
1. Creates two tags.
2. Attempts to rename the first tag to the second's name.
3. Asserts that `TagAlreadyExistsException` is thrown with the correct
message.
4. Verifies that the state of both tags remains unchanged.
Verified the fix by running:
`./gradlew :core:test --tests "org.apache.gravitino.tag.TestTagManager"
-PskipITs`
---------
Co-authored-by: Jerry Shao <[email protected]>
---
.../java/org/apache/gravitino/tag/TagOperations.java | 1 +
.../org/apache/gravitino/client/GravitinoClient.java | 2 +-
.../org/apache/gravitino/client/GravitinoMetalake.java | 2 +-
.../client-python/gravitino/api/tag/tag_operations.py | 2 ++
.../client-python/gravitino/client/gravitino_client.py | 2 ++
.../gravitino/client/gravitino_metalake.py | 2 ++
.../org/apache/gravitino/hook/TagHookDispatcher.java | 4 +++-
.../apache/gravitino/listener/TagEventDispatcher.java | 4 +++-
.../java/org/apache/gravitino/tag/TagDispatcher.java | 3 +++
.../main/java/org/apache/gravitino/tag/TagManager.java | 12 ++++++++----
.../java/org/apache/gravitino/tag/TestTagManager.java | 18 ++++++++++++++++++
design-docs/gravitino-function-privilege.md | 1 +
12 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/tag/TagOperations.java
b/api/src/main/java/org/apache/gravitino/tag/TagOperations.java
index 1745efb236..c58ceeda8e 100644
--- a/api/src/main/java/org/apache/gravitino/tag/TagOperations.java
+++ b/api/src/main/java/org/apache/gravitino/tag/TagOperations.java
@@ -78,6 +78,7 @@ public interface TagOperations {
* @return The altered tag.
* @throws NoSuchTagException If the tag does not exist.
* @throws IllegalArgumentException If the changes cannot be applied to the
tag.
+ * @throws TagAlreadyExistsException If a tag with the new name already
exists.
*/
Tag alterTag(String name, TagChange... changes)
throws NoSuchTagException, IllegalArgumentException;
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
index d47c88882a..c7e19ba73c 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
@@ -565,7 +565,7 @@ public class GravitinoClient extends GravitinoClientBase
@Override
public Tag alterTag(String name, TagChange... changes)
- throws NoSuchTagException, IllegalArgumentException {
+ throws NoSuchTagException, IllegalArgumentException,
TagAlreadyExistsException {
return getMetalake().alterTag(name, changes);
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
index 1d3c9c2a23..2be2b88d92 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
@@ -537,7 +537,7 @@ public class GravitinoMetalake extends MetalakeDTO
*/
@Override
public Tag alterTag(String name, TagChange... changes)
- throws NoSuchTagException, IllegalArgumentException {
+ throws NoSuchTagException, IllegalArgumentException,
TagAlreadyExistsException {
Preconditions.checkArgument(StringUtils.isNotBlank(name), "tag name must
not be null or empty");
List<TagUpdateRequest> updates =
Arrays.stream(changes).map(DTOConverters::toTagUpdateRequest).collect(Collectors.toList());
diff --git a/clients/client-python/gravitino/api/tag/tag_operations.py
b/clients/client-python/gravitino/api/tag/tag_operations.py
index 58bbce2657..7089564705 100644
--- a/clients/client-python/gravitino/api/tag/tag_operations.py
+++ b/clients/client-python/gravitino/api/tag/tag_operations.py
@@ -114,6 +114,8 @@ class TagOperations(ABC):
Raises:
NoSuchTagException: If the tag does not exist.
NoSuchMetalakeException: If the metalake does not exist.
+ IllegalArgumentException: If the changes cannot be applied to the
tag.
+ TagAlreadyExistsException: If a tag with the new name already
exists.
"""
pass
diff --git a/clients/client-python/gravitino/client/gravitino_client.py
b/clients/client-python/gravitino/client/gravitino_client.py
index 862b4bce33..29c36a8937 100644
--- a/clients/client-python/gravitino/client/gravitino_client.py
+++ b/clients/client-python/gravitino/client/gravitino_client.py
@@ -312,6 +312,8 @@ class GravitinoClient(GravitinoClientBase, SupportsJobs,
TagOperations):
Raises:
NoSuchTagException: If the tag does not exist.
NoSuchMetalakeException: If the metalake does not exist.
+ IllegalArgumentException: If the changes cannot be applied to the
tag.
+ TagAlreadyExistsException: If a tag with the new name already
exists.
"""
return self.get_metalake().alter_tag(tag_name, *changes)
diff --git a/clients/client-python/gravitino/client/gravitino_metalake.py
b/clients/client-python/gravitino/client/gravitino_metalake.py
index 431bfdd23b..3bd3d2fff2 100644
--- a/clients/client-python/gravitino/client/gravitino_metalake.py
+++ b/clients/client-python/gravitino/client/gravitino_metalake.py
@@ -644,6 +644,8 @@ class GravitinoMetalake(
Raises:
NoSuchTagException: If the tag does not exist.
NoSuchMetalakeException: If the metalake does not exist.
+ IllegalArgumentException: If the changes cannot be applied to the
tag.
+ TagAlreadyExistsException: If a tag with the new name already
exists.
"""
Precondition.check_argument(
StringUtils.is_not_blank(tag_name),
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
index 1db84fdff8..444f597fe4 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
@@ -24,6 +24,7 @@ import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.NoSuchTagException;
+import org.apache.gravitino.exceptions.TagAlreadyExistsException;
import org.apache.gravitino.tag.Tag;
import org.apache.gravitino.tag.TagChange;
import org.apache.gravitino.tag.TagDispatcher;
@@ -73,7 +74,8 @@ public class TagHookDispatcher implements TagDispatcher {
}
@Override
- public Tag alterTag(String metalake, String name, TagChange... changes) {
+ public Tag alterTag(String metalake, String name, TagChange... changes)
+ throws IllegalArgumentException, TagAlreadyExistsException {
return dispatcher.alterTag(metalake, name, changes);
}
diff --git
a/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java
b/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java
index 2dd81cde3f..8b06f10ddb 100644
--- a/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.listener;
import java.util.Map;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.exceptions.NoSuchTagException;
+import org.apache.gravitino.exceptions.TagAlreadyExistsException;
import org.apache.gravitino.listener.api.event.AlterTagEvent;
import org.apache.gravitino.listener.api.event.AlterTagFailureEvent;
import org.apache.gravitino.listener.api.event.AlterTagPreEvent;
@@ -141,7 +142,8 @@ public class TagEventDispatcher implements TagDispatcher {
}
@Override
- public Tag alterTag(String metalake, String name, TagChange... changes) {
+ public Tag alterTag(String metalake, String name, TagChange... changes)
+ throws IllegalArgumentException, TagAlreadyExistsException {
AlterTagPreEvent preEvent =
new AlterTagPreEvent(PrincipalUtils.getCurrentUserName(), metalake,
name, changes);
diff --git a/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java
b/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java
index 6e0276703d..e48113dfdf 100644
--- a/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.tag;
import java.util.Map;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.exceptions.NoSuchTagException;
+import org.apache.gravitino.exceptions.TagAlreadyExistsException;
/**
* {@code TagDispatcher} interface provides functionalities for managing tags
within a metalake. It
@@ -72,6 +73,8 @@ public interface TagDispatcher {
* @param name The name of the tag.
* @param changes The changes to apply to the tag.
* @return The updated tag.
+ * @throws IllegalArgumentException If the changes cannot be applied to the
tag.
+ * @throws TagAlreadyExistsException If a tag with the new name already
exists.
*/
Tag alterTag(String metalake, String name, TagChange... changes);
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 38c3f5bab7..eb26cf37e9 100644
--- a/core/src/main/java/org/apache/gravitino/tag/TagManager.java
+++ b/core/src/main/java/org/apache/gravitino/tag/TagManager.java
@@ -173,10 +173,14 @@ public class TagManager implements TagDispatcher {
throw new NoSuchTagException(
"Tag with name %s under metalake %s does not exist", name,
metalake);
} catch (EntityAlreadyExistsException e) {
- throw new RuntimeException(
- String.format(
- "Trying to alter tag %s under metalake %s, but the new
name already exists",
- name, metalake));
+ String newName =
+ Arrays.stream(changes)
+ .filter(c -> c instanceof TagChange.RenameTag)
+ .map(c -> ((TagChange.RenameTag) c).getNewName())
+ .findFirst()
+ .orElse(name);
+ throw new TagAlreadyExistsException(
+ e, "Tag with name %s under metalake %s already exists",
newName, metalake);
} catch (IOException ioe) {
LOG.error("Failed to alter tag {} under metalake {}", name,
metalake, ioe);
throw new RuntimeException(ioe);
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 c1ca78e8cf..d7e86abba3 100644
--- a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
+++ b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
@@ -356,6 +356,24 @@ public class TestTagManager {
Assertions.assertEquals(expectedProp2, removedPropTag.properties());
}
+ @Test
+ public void testAlterTagRenameToExistingTag() {
+ tagManager.createTag(METALAKE, "tag1", null, null);
+ tagManager.createTag(METALAKE, "tag2", null, null);
+
+ TagAlreadyExistsException exception =
+ Assertions.assertThrows(
+ TagAlreadyExistsException.class,
+ () -> tagManager.alterTag(METALAKE, "tag1",
TagChange.rename("tag2")));
+
+ Assertions.assertEquals(
+ "Tag with name tag2 under metalake metalake_for_tag_test already
exists",
+ exception.getMessage());
+
+ Assertions.assertEquals("tag1", tagManager.getTag(METALAKE,
"tag1").name());
+ Assertions.assertEquals("tag2", tagManager.getTag(METALAKE,
"tag2").name());
+ }
+
@Test
public void testDeleteTag() {
tagManager.createTag(METALAKE, "tag1", null, null);
diff --git a/design-docs/gravitino-function-privilege.md
b/design-docs/gravitino-function-privilege.md
index c6c703b577..8ad3c01932 100644
--- a/design-docs/gravitino-function-privilege.md
+++ b/design-docs/gravitino-function-privilege.md
@@ -16,6 +16,7 @@
specific language governing permissions and limitations
under the License.
-->
+
# Design of Function Privilege Control in Gravitino
## Background
