This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 8ea927ad7d [Cherry-pick to branch-1.3] [#14017] fix(core): Return
JobTemplateAlreadyExistsException when renaming a job template to an existing
name (#14018) (#14044)
8ea927ad7d is described below
commit 8ea927ad7d9c03a6c2fddcbfd1e938c02685862f
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Wed Oct 21 14:45:55 2026 +0800
[Cherry-pick to branch-1.3] [#14017] fix(core): Return
JobTemplateAlreadyExistsException when renaming a job template to an existing
name (#14018) (#14044)
### What changes were proposed in this pull request?
Cherry-pick of #14018 to branch-1.3. `JobManager.alterJobTemplate`
converts `EntityAlreadyExistsException` into
`JobTemplateAlreadyExistsException` instead of a plain
`RuntimeException`, and unwraps the new name so the message no longer
prints `Optional[...]`.
The automatic cherry-pick conflicted only in the tests: around the new
cases in `TestJobManager` and `TestJobOperations`, main has tests from
other commits that are not on branch-1.3. This PR keeps only the tests
added by #14018; the main code applies unchanged.
### Why are the changes needed?
Backport the fix for #14017 to branch-1.3, which has the same
`alterJobTemplate` code. Renaming a job template to an existing name
returns 500 there instead of 409.
Fix: #14017
### Does this PR introduce _any_ user-facing change?
Yes, as in #14018. A rename collision returns HTTP 409
(`JobTemplateAlreadyExistsException`) instead of 500.
### How was this patch tested?
`TestJobManager#testAlterJobTemplateRenameToExistingName` and the 409
case in `TestJobOperations#testAlterJobTemplate`, both from #14018;
`TestJobManager` and `TestJobOperations` pass on branch-1.3.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Jerry Shao <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
Co-authored-by: Jerry Shao <[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 b5ca1698e2..c2e977e29b 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 bd6e8ecc8e..021648f0c4 100644
--- a/core/src/main/java/org/apache/gravitino/job/JobManager.java
+++ b/core/src/main/java/org/apache/gravitino/job/JobManager.java
@@ -309,7 +309,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 =
@@ -343,12 +344,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 795ae6aeae..f22114491a 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 1d73004281..b9e5c4d4c3 100644
--- a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
+++ b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
@@ -174,6 +174,34 @@ public class TestJobManager {
}
}
+ @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 da79ca5bb7..49790d5655 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
@@ -648,6 +648,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 RuntimeException
doThrow(new RuntimeException("mock error"))
.when(jobOperationDispatcher)
