From ec2c66885dba4999e5e1cab17ecd129bc78fae30 Mon Sep 17 00:00:00 2001 From: dsyer Date: Fri, 14 Mar 2008 17:09:26 +0000 Subject: [PATCH] RESOLVED - issue BATCH-464: Throw exception if restart is attempted on job instance that was already completed successfully --- .../batch/core/launch/JobLauncher.java | 5 ++- .../support/SimpleExportedJobLauncher.java | 9 +--- .../launch/support/SimpleJobLauncher.java | 4 +- .../JobInstanceAlreadyCompleteException.java | 44 +++++++++++++++++++ .../batch/core/repository/JobRepository.java | 33 +++++++------- .../support/SimpleJobRepository.java | 21 ++++++--- ...InstanceAlreadyCompleteExceptionTests.java | 40 +++++++++++++++++ .../sample/quartz/JobLauncherDetails.java | 12 +---- .../batch/sample/RestartFunctionalTests.java | 30 +++++++------ 9 files changed, 144 insertions(+), 54 deletions(-) create mode 100644 spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteException.java create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteExceptionTests.java diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobLauncher.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobLauncher.java index eb333dc1b..81709f533 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobLauncher.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobLauncher.java @@ -18,6 +18,7 @@ package org.springframework.batch.core.launch; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobParameters; +import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException; import org.springframework.batch.core.repository.JobRestartException; import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; @@ -47,8 +48,10 @@ public interface JobLauncher { * null. * @throws JobRestartException if the job has been run before and * circumstances that preclude a re-start. + * @throws JobInstanceAlreadyCompleteException if the job has been run + * before with the same parameters and completed successfully */ public JobExecution run(Job job, JobParameters jobParameters) throws JobExecutionAlreadyRunningException, - JobRestartException; + JobRestartException, JobInstanceAlreadyCompleteException; } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleExportedJobLauncher.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleExportedJobLauncher.java index 28f81bddb..6f7a9ee43 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleExportedJobLauncher.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleExportedJobLauncher.java @@ -22,14 +22,13 @@ import java.util.Properties; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; +import org.springframework.batch.core.JobExecutionException; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.StepExecution; import org.springframework.batch.core.configuration.JobLocator; import org.springframework.batch.core.converter.DefaultJobParametersConverter; import org.springframework.batch.core.converter.JobParametersConverter; import org.springframework.batch.core.launch.JobLauncher; -import org.springframework.batch.core.repository.JobRestartException; -import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; import org.springframework.batch.core.repository.NoSuchJobException; import org.springframework.batch.support.PropertiesConverter; import org.springframework.beans.factory.InitializingBean; @@ -160,13 +159,9 @@ public class SimpleExportedJobLauncher implements ExportedJobLauncher, Initializ try { execution = launcher.run(job, jobParameters); } - catch (JobExecutionAlreadyRunningException e) { + catch (JobExecutionException e) { return e.getClass().getName() + ": " + e.getMessage(); } - catch (JobRestartException e) { - return e.getClass().getName() + ": " + e.getMessage(); - } - registry.put(name + params, execution); return execution.toString(); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java index 8b6a9bdeb..73f9ae3c6 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java @@ -22,6 +22,7 @@ import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobInstance; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.launch.JobLauncher; +import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException; import org.springframework.batch.core.repository.JobRestartException; import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; import org.springframework.batch.core.repository.JobRepository; @@ -71,9 +72,10 @@ public class SimpleJobLauncher implements JobLauncher, InitializingBean { * exists and has an execution already running. * @throws JobRestartException if the execution would be a re-start, but a * re-start is either not allowed or not needed. + * @throws JobInstanceAlreadyCompleteException */ public JobExecution run(final Job job, final JobParameters jobParameters) - throws JobExecutionAlreadyRunningException, JobRestartException { + throws JobExecutionAlreadyRunningException, JobRestartException, JobInstanceAlreadyCompleteException { Assert.notNull(job, "The Job must not be null."); Assert.notNull(jobParameters, "The JobParameters must not be null."); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteException.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteException.java new file mode 100644 index 000000000..d92132987 --- /dev/null +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteException.java @@ -0,0 +1,44 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.core.repository; + +import org.springframework.batch.core.JobExecutionException; + +/** + * An exception indicating an illegal attempt to restart a job that was already + * completed successfully. + * + * @author Dave Syer + * + */ +public class JobInstanceAlreadyCompleteException extends JobExecutionException { + + /** + * @param string the message + */ + public JobInstanceAlreadyCompleteException(String string) { + super(string); + } + + /** + * @param msg the cause + * @param t the message + */ + public JobInstanceAlreadyCompleteException(String msg, Throwable t) { + super(msg, t); + } + +} diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java index 552442094..330346f7e 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java @@ -26,10 +26,10 @@ import org.springframework.batch.item.ExecutionContext; /** *

- * Repository for storing batch {@link JobExecution} and {@link StepExecution}s. - * Before using any methods, a {@link JobExecution} - * must first be obtained using the createJobExecution method. Once a {@link JobExecution} - * is obtained, they can be updated. + * Repository for storing batch {@link JobExecution} and {@link StepExecution}s. + * Before using any methods, a {@link JobExecution} must first be obtained using + * the createJobExecution method. Once a {@link JobExecution} is obtained, they + * can be updated. *

* * @author Lucas Ward @@ -39,25 +39,28 @@ public interface JobRepository { /** * Find or create a {@link JobExecution} for a given {@link Job} and - * {@link JobParameters}. If the {@link Job} that is uniquely identified by - * {@link JobParameters} already exists, its persisted values (including ID) - * will be returned in a new {@link JobInstance} object, associated with the - * returned {@link JobExecution}. If no previous run - * is found, the execution will be associated with a new {@link JobInstance} + * {@link JobParameters}. If the {@link Job} was already executed with + * these {@link JobParameters}, its persisted values (including ID) will be + * returned in a new {@link JobInstance}, associated with the + * {@link JobExecution}. If no previous instance is found, the execution + * will be associated with a new {@link JobInstance} * * @param jobParameters the runtime parameters for the job * @param job the job the execution should be associated with. * * @return a valid job {@link JobExecution} for the arguments provided * @throws JobExecutionAlreadyRunningException if there is a - * {@link JobExecution} alrady running for the job instance that would - * otherwise be returned - * @throws JobRestartException if more than one JobInstance if found or if - * JobInstance.getJobExecutionCount() is greater than Job.getStartLimit() + * {@link JobExecution} already running for the job instance with the + * provided job and parameters. + * @throws JobRestartException if one or more existing {@link JobInstance}s + * is found with the same parameters and {@link Job#isRestartable()} is + * false. + * @throws JobInstanceAlreadyCompleteException if a {@link JobInstance} is + * found and was already completed successfully. * */ - JobExecution createJobExecution(Job job, JobParameters jobParameters) - throws JobExecutionAlreadyRunningException, JobRestartException; + JobExecution createJobExecution(Job job, JobParameters jobParameters) throws JobExecutionAlreadyRunningException, + JobRestartException, JobInstanceAlreadyCompleteException; /** * Save or Update a {@link JobExecution}. If no ID is found a new instance diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java index 8f7f18574..44f2ebc26 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java @@ -20,15 +20,17 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobInstance; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.Step; import org.springframework.batch.core.StepExecution; -import org.springframework.batch.core.repository.JobRestartException; import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; +import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException; import org.springframework.batch.core.repository.JobRepository; +import org.springframework.batch.core.repository.JobRestartException; import org.springframework.batch.core.repository.dao.JobExecutionDao; import org.springframework.batch.core.repository.dao.JobInstanceDao; import org.springframework.batch.core.repository.dao.StepExecutionDao; @@ -134,7 +136,7 @@ public class SimpleJobRepository implements JobRepository { * */ public JobExecution createJobExecution(Job job, JobParameters jobParameters) - throws JobExecutionAlreadyRunningException, JobRestartException { + throws JobExecutionAlreadyRunningException, JobRestartException, JobInstanceAlreadyCompleteException { Assert.notNull(job, "Job must not be null."); Assert.notNull(jobParameters, "JobParameters must not be null."); @@ -157,7 +159,7 @@ public class SimpleJobRepository implements JobRepository { } List executions = jobExecutionDao.findJobExecutions(jobInstance); - + // check for running executions and find the last started for (Iterator iterator = executions.iterator(); iterator.hasNext();) { JobExecution execution = (JobExecution) iterator.next(); @@ -165,19 +167,24 @@ public class SimpleJobRepository implements JobRepository { throw new JobExecutionAlreadyRunningException("A job execution for this job is already running: " + jobInstance); } + if (execution.getStatus() == BatchStatus.COMPLETED) { + throw new JobInstanceAlreadyCompleteException( + "A job instance already exists is complete for parameters=" + jobParameters + + ". If you want to run this job again, change the parameters."); + } } } else { // no job found, create one jobInstance = jobInstanceDao.createJobInstance(job, jobParameters); } - + JobExecution jobExecution = new JobExecution(jobInstance); - + // Save the JobExecution so that it picks up an ID (useful for clients // monitoring asynchronous executions): saveOrUpdate(jobExecution); - + return jobExecution; } @@ -222,7 +229,7 @@ public class SimpleJobRepository implements JobRepository { Assert.notNull(stepExecution, "StepExecution cannot be null."); Assert.notNull(stepExecution.getStepName(), "StepExecution's step name cannot be null."); Assert.notNull(stepExecution.getJobExecutionId(), "StepExecution must belong to persisted JobExecution"); - + if (stepExecution.getId() == null) { stepExecutionDao.saveStepExecution(stepExecution); } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteExceptionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteExceptionTests.java new file mode 100644 index 000000000..1d443372f --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/JobInstanceAlreadyCompleteExceptionTests.java @@ -0,0 +1,40 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.core.repository; + +import org.springframework.batch.core.AbstractExceptionTests; + +/** + * @author Dave Syer + * + */ +public class JobInstanceAlreadyCompleteExceptionTests extends AbstractExceptionTests { + + /* (non-Javadoc) + * @see org.springframework.batch.io.exception.AbstractExceptionTests#getException(java.lang.String) + */ + public Exception getException(String msg) throws Exception { + return new JobInstanceAlreadyCompleteException(msg); + } + + /* (non-Javadoc) + * @see org.springframework.batch.io.exception.AbstractExceptionTests#getException(java.lang.String, java.lang.Throwable) + */ + public Exception getException(String msg, Throwable t) throws Exception { + return new JobInstanceAlreadyCompleteException(msg, t); + } + +} diff --git a/spring-batch-samples/src/main/java/org/springframework/batch/sample/quartz/JobLauncherDetails.java b/spring-batch-samples/src/main/java/org/springframework/batch/sample/quartz/JobLauncherDetails.java index 86f3a5fe7..2a544bc9c 100644 --- a/spring-batch-samples/src/main/java/org/springframework/batch/sample/quartz/JobLauncherDetails.java +++ b/spring-batch-samples/src/main/java/org/springframework/batch/sample/quartz/JobLauncherDetails.java @@ -23,13 +23,11 @@ import java.util.Map.Entry; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.quartz.JobExecutionContext; +import org.springframework.batch.core.JobExecutionException; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersBuilder; import org.springframework.batch.core.configuration.JobLocator; import org.springframework.batch.core.launch.JobLauncher; -import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; -import org.springframework.batch.core.repository.JobRestartException; -import org.springframework.batch.core.repository.NoSuchJobException; import org.springframework.scheduling.quartz.QuartzJobBean; /** @@ -73,13 +71,7 @@ public class JobLauncherDetails extends QuartzJobBean { try { jobLauncher.run(jobLocator.getJob(jobName), jobParameters); } - catch (JobExecutionAlreadyRunningException e) { - log.error("Could not execute job.", e); - } - catch (JobRestartException e) { - log.error("Could not execute job.", e); - } - catch (NoSuchJobException e) { + catch (JobExecutionException e) { log.error("Could not execute job.", e); } } diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java index 728f48c57..886c68d49 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java @@ -17,7 +17,8 @@ package org.springframework.batch.sample; import org.springframework.batch.core.UnexpectedJobExecutionException; -import org.springframework.batch.core.JobParameters; +import org.springframework.batch.core.converter.DefaultJobParametersConverter; +import org.springframework.batch.support.PropertiesConverter; import org.springframework.jdbc.core.JdbcOperations; /** @@ -28,35 +29,36 @@ import org.springframework.jdbc.core.JdbcOperations; */ public class RestartFunctionalTests extends AbstractBatchLauncherTests { - //auto-injected attributes + // auto-injected attributes private JdbcOperations jdbcTemplate; /** * Public setter for the jdbcTemplate. - * + * * @param jdbcTemplate the jdbcTemplate to set */ public void setJdbcTemplate(JdbcOperations jdbcTemplate) { this.jdbcTemplate = jdbcTemplate; } - - /* (non-Javadoc) + + /* + * (non-Javadoc) * @see org.springframework.test.AbstractSingleSpringContextTests#onTearDown() */ protected void onTearDown() throws Exception { jdbcTemplate.update("DELETE FROM TRADE"); } - + /** * Job fails on first run, because the module throws exception after * processing more than half of the input. On the second run, the job should * finish successfully, because it continues execution where the previous * run stopped (module throws exception after fixed number of processed * records). - * @throws Exception + * @throws Exception */ public void testRestart() throws Exception { - + int before = jdbcTemplate.queryForInt("SELECT COUNT(*) FROM TRADE"); try { @@ -64,24 +66,26 @@ public class RestartFunctionalTests extends AbstractBatchLauncherTests { fail("First run of the job is expected to fail."); } catch (UnexpectedJobExecutionException expected) { - //expected - assertTrue("Not planned exception: "+expected.getMessage(), expected.getMessage().toLowerCase().indexOf("planned")>=0); + // expected + assertTrue("Not planned exception: " + expected.getMessage(), expected.getMessage().toLowerCase().indexOf( + "planned") >= 0); } int medium = jdbcTemplate.queryForInt("SELECT COUNT(*) FROM TRADE"); // assert based on commit interval = 2 - assertEquals(before+2, medium); + assertEquals(before + 2, medium); runJob(); int after = jdbcTemplate.queryForInt("SELECT COUNT(*) FROM TRADE"); - assertEquals(before+5, after); + assertEquals(before + 5, after); } // load the application context and launch the job private void runJob() throws Exception { - launcher.run(getJob(), new JobParameters()); + launcher.run(getJob(), new DefaultJobParametersConverter().getJobParameters(PropertiesConverter + .stringToProperties("restart=true"))); } }