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 60cd039154 [#14017] fix(core): Return
JobTemplateAlreadyExistsException when renaming a job template to an existing
name (#14018)
60cd039154 is described below
commit 60cd039154c329999039b97f337a6baef3963527
Author: Jerry Shao <[email protected]>
AuthorDate: Wed Oct 21 10:41:27 2026 +0800
[#14017] fix(core): Return JobTemplateAlreadyExistsException when renaming
a job template to an existing name (#14018)
### What changes were proposed in this pull request?
- `JobManager.alterJobTemplate` now converts
`EntityAlreadyExistsException` into `JobTemplateAlreadyExistsException`
instead of a plain `RuntimeException`, and unwraps the new name so the
message no longer prints `Optional[...]`.
- Declare `JobTemplateAlreadyExistsException` on
`SupportsJobs#alterJobTemplate` and
`JobOperationDispatcher#alterJobTemplate`.
### Why are the changes needed?
Renaming a job template to an existing name returned 500 with a
misleading message. It should return 409 so clients can ask the user to
pick another name.
Fix: #14017
### Does this PR introduce _any_ user-facing change?
Yes. A rename collision now returns HTTP 409
(`JobTemplateAlreadyExistsException`) instead of 500. Java and Python
clients already map 409 to `JobTemplateAlreadyExistsException`.
### How was this patch tested?
- Added `TestJobManager#testAlterJobTemplateRenameToExistingName`.
- Added a 409 case to `TestJobOperations#testAlterJobTemplate`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../org/apache/gravitino/job/SupportsJobs.java | 5 +++-
.../java/org/apache/gravitino/job/JobManager.java | 22 +++++++++++------
.../gravitino/job/JobOperationDispatcher.java | 5 +++-
.../org/apache/gravitino/job/TestJobManager.java | 28 ++++++++++++++++++++++
.../server/web/rest/TestJobOperations.java | 18 ++++++++++++++
5 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/job/SupportsJobs.java
b/api/src/main/java/org/apache/gravitino/job/SupportsJobs.java
index 5bea40c5ab..ace73cc257 100644
--- a/api/src/main/java/org/apache/gravitino/job/SupportsJobs.java
+++ b/api/src/main/java/org/apache/gravitino/job/SupportsJobs.java
@@ -80,10 +80,13 @@ public interface SupportsJobs {
* @param changes the changes to apply to the job template
* @return the updated job template after applying the changes
* @throws NoSuchJobTemplateException if no job template with the specified
name exists
+ * @throws JobTemplateAlreadyExistsException if the job template is renamed
to a name that already
+ * exists
* @throws IllegalArgumentException if any of the changes cannot be applied
to the job template
*/
default JobTemplate alterJobTemplate(String jobTemplateName,
JobTemplateChange... changes)
- throws NoSuchJobTemplateException, IllegalArgumentException {
+ throws NoSuchJobTemplateException, JobTemplateAlreadyExistsException,
+ IllegalArgumentException {
throw new UnsupportedOperationException("Not implemented");
}
diff --git a/core/src/main/java/org/apache/gravitino/job/JobManager.java
b/core/src/main/java/org/apache/gravitino/job/JobManager.java
index 32e9966b02..20feaf1ca5 100644
--- a/core/src/main/java/org/apache/gravitino/job/JobManager.java
+++ b/core/src/main/java/org/apache/gravitino/job/JobManager.java
@@ -339,7 +339,8 @@ public class JobManager implements JobOperationDispatcher {
@Override
public JobTemplateEntity alterJobTemplate(
String metalake, String jobTemplateName, JobTemplateChange... changes)
- throws NoSuchJobTemplateException, IllegalArgumentException {
+ throws NoSuchJobTemplateException, JobTemplateAlreadyExistsException,
+ IllegalArgumentException {
checkMetalake(NameIdentifierUtil.ofMetalake(metalake), entityStore);
Optional<String> newName =
@@ -371,12 +372,19 @@ public class JobManager implements JobOperationDispatcher
{
} catch (EntityAlreadyExistsException e) {
// If the EntityAlreadyExistsException is thrown, it means the new
name already exists.
// So there should be a rename change, and the new name should be
present.
- throw new RuntimeException(
- String.format(
- "Failed to rename job template from %s to %s under
metalake %s, the new name "
- + "already exists",
- jobTemplateName, newName, metalake),
- e);
+ String conflictName =
+ newName.orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Job template already exists without a rename
change", e));
+ throw new JobTemplateAlreadyExistsException(
+ e,
+ "Failed to rename job template %s to %s under metalake %s, a
job template named "
+ + "%s already exists",
+ jobTemplateName,
+ conflictName,
+ metalake,
+ conflictName);
}
});
}
diff --git
a/core/src/main/java/org/apache/gravitino/job/JobOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/job/JobOperationDispatcher.java
index f0e349a8cc..7c2c340fef 100644
--- a/core/src/main/java/org/apache/gravitino/job/JobOperationDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/job/JobOperationDispatcher.java
@@ -81,11 +81,14 @@ public interface JobOperationDispatcher extends Closeable {
* @param changes the changes to apply to the job template
* @return the updated job template entity after applying the changes
* @throws NoSuchJobTemplateException if no job template with the specified
name exists
+ * @throws JobTemplateAlreadyExistsException if the job template is renamed
to a name that already
+ * exists
* @throws IllegalArgumentException if any of the changes cannot be applied
to the job template.
*/
JobTemplateEntity alterJobTemplate(
String metalake, String jobTemplateName, JobTemplateChange... changes)
- throws NoSuchJobTemplateException, IllegalArgumentException;
+ throws NoSuchJobTemplateException, JobTemplateAlreadyExistsException,
+ IllegalArgumentException;
/**
* List all the jobs. If the jobTemplateName is provided, it will list the
jobs associated with
diff --git a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
index 694bff92d9..1b2d7f79fa 100644
--- a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
+++ b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
@@ -275,6 +275,34 @@ public class TestJobManager {
() -> jobManager.alterJobTemplate(metalake, "shell_job")));
}
+ @Test
+ public void testAlterJobTemplateRenameToExistingName() throws IOException {
+ EntityAlreadyExistsException exists = new
EntityAlreadyExistsException("Name already exists");
+ doThrow(exists).when(entityStore).update(any(), any(), any(), any());
+
+ JobTemplateAlreadyExistsException failure =
+ Assertions.assertThrows(
+ JobTemplateAlreadyExistsException.class,
+ () ->
+ jobManager.alterJobTemplate(
+ metalake, "shell_job",
JobTemplateChange.rename("existing_job")));
+ Assertions.assertSame(exists, failure.getCause());
+ Assertions.assertEquals(
+ "Failed to rename job template shell_job to existing_job under
metalake "
+ + metalake
+ + ", a job template named existing_job already exists",
+ failure.getMessage());
+
+ // Without a rename change, a conflict is an invariant violation, not a
name conflict.
+ IllegalStateException unexpected =
+ Assertions.assertThrows(
+ IllegalStateException.class,
+ () ->
+ jobManager.alterJobTemplate(
+ metalake, "shell_job",
JobTemplateChange.updateComment("new comment")));
+ Assertions.assertSame(exists, unexpected.getCause());
+ }
+
@Test
public void testListJobTemplates() throws IOException {
mockedMetalake
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
index 04f057ab5d..de3ea2c4d1 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java
@@ -674,6 +674,24 @@ public class TestJobOperations extends JerseyTest {
Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
errorResp4.getCode());
Assertions.assertEquals(NoSuchJobTemplateException.class.getSimpleName(),
errorResp4.getType());
+ // Test throw JobTemplateAlreadyExistsException
+ doThrow(new JobTemplateAlreadyExistsException("mock error"))
+ .when(jobOperationDispatcher)
+ .alterJobTemplate(any(), any(), any());
+
+ Response respExists =
+ target(jobTemplatePath())
+ .path(templateName)
+ .request(APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(req, APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
respExists.getStatus());
+ ErrorResponse errorRespExists = respExists.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
errorRespExists.getCode());
+ Assertions.assertEquals(
+ JobTemplateAlreadyExistsException.class.getSimpleName(),
errorRespExists.getType());
+
// Test throw UnsupportedOperationException
doThrow(new UnsupportedOperationException("mock error"))
.when(jobOperationDispatcher)
