diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/Job.java b/spring-batch-core/src/main/java/org/springframework/batch/core/Job.java index 583ed2911..507c2384b 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/Job.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/Job.java @@ -54,6 +54,4 @@ public interface Job { */ JobParametersIncrementer getJobParametersIncrementer(); - void validate(JobParameters parameters) throws JobParametersInvalidException; - } \ No newline at end of file diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/GroupAwareJob.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/GroupAwareJob.java index c96c0829c..e087b06e2 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/GroupAwareJob.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/GroupAwareJob.java @@ -17,9 +17,7 @@ package org.springframework.batch.core.configuration.support; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; -import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersIncrementer; -import org.springframework.batch.core.JobParametersInvalidException; /** * A {@link Job} that can optionally prepend a group name to another job's name, @@ -71,10 +69,6 @@ public class GroupAwareJob implements Job { delegate.execute(execution); } - public void validate(JobParameters parameters) throws JobParametersInvalidException { - delegate.validate(parameters); - } - /** * Concatenates the group name and the delegate job name (joining with a * "."). diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java index 5f04c6b91..9e9470337 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java @@ -16,6 +16,8 @@ package org.springframework.batch.core.job; +import java.io.PrintWriter; +import java.io.StringWriter; import java.util.Collection; import java.util.Date; @@ -29,12 +31,12 @@ import org.springframework.batch.core.JobExecutionException; import org.springframework.batch.core.JobExecutionListener; import org.springframework.batch.core.JobInstance; import org.springframework.batch.core.JobInterruptedException; -import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersIncrementer; -import org.springframework.batch.core.JobParametersInvalidException; import org.springframework.batch.core.StartLimitExceededException; import org.springframework.batch.core.Step; import org.springframework.batch.core.StepExecution; +import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.launch.support.ExitCodeMapper; import org.springframework.batch.core.listener.CompositeJobExecutionListener; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.repository.JobRestartException; @@ -99,16 +101,6 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In this.jobParametersValidator = jobParametersValidator; } - /** - * Delegates to the {@link #setJobParametersValidator validator} supplied - * (defaults to just checking for null parameters). - * - * @see Job#validate(JobParameters) - */ - public void validate(JobParameters parameters) throws JobParametersInvalidException { - jobParametersValidator.validate(parameters); - } - /** * Assert mandatory properties: {@link JobRepository}. * @@ -263,6 +255,8 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In try { + jobParametersValidator.validate(execution.getJobInstance().getJobParameters()); + if (execution.getStatus() != BatchStatus.STOPPING) { execution.setStartTime(new Date()); @@ -291,21 +285,23 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In } catch (JobInterruptedException e) { logger.error("Encountered interruption executing job", e); - execution.setExitStatus(ExitStatus.STOPPED); + execution.setExitStatus(getDefaultExitStatusForFailure(e)); execution.setStatus(BatchStatus.STOPPED); execution.addFailureException(e); } catch (Throwable t) { logger.error("Encountered fatal error executing job", t); - execution.setExitStatus(ExitStatus.FAILED); + execution.setExitStatus(getDefaultExitStatusForFailure(t)); execution.setStatus(BatchStatus.FAILED); execution.addFailureException(t); } finally { - if (execution.getStepExecutions().isEmpty()) { - execution.setExitStatus(ExitStatus.NOOP - .addExitDescription("All steps already completed or no steps configured for this job.")); + if (execution.getStatus().isLessThanOrEqualTo(BatchStatus.STOPPED) + && execution.getStepExecutions().isEmpty()) { + ExitStatus exitStatus = execution.getExitStatus(); + execution.setExitStatus(exitStatus.and(ExitStatus.NOOP + .addExitDescription("All steps already completed or no steps configured for this job."))); } execution.setEndTime(new Date()); @@ -458,6 +454,30 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In } } + /** + * Default mapping from throwable to {@link ExitStatus}. + * + * @param ex the cause of the failure + * @return an {@link ExitStatus} + */ + private ExitStatus getDefaultExitStatusForFailure(Throwable ex) { + ExitStatus exitStatus; + if (ex instanceof JobInterruptedException || ex.getCause() instanceof JobInterruptedException) { + exitStatus = ExitStatus.STOPPED.addExitDescription(JobInterruptedException.class.getName()); + } + else if (ex instanceof NoSuchJobException || ex.getCause() instanceof NoSuchJobException) { + exitStatus = new ExitStatus(ExitCodeMapper.NO_SUCH_JOB, ex.getClass().getName()); + } + else { + StringWriter writer = new StringWriter(); + ex.printStackTrace(new PrintWriter(writer)); + String message = writer.toString(); + exitStatus = ExitStatus.FAILED.addExitDescription(message); + } + + return exitStatus; + } + private void updateStatus(JobExecution jobExecution, BatchStatus status) { jobExecution.setStatus(status); jobRepository.update(jobExecution); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/DefaultJobParametersValidator.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/DefaultJobParametersValidator.java index f6015cb9b..2a2318f40 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/DefaultJobParametersValidator.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/job/DefaultJobParametersValidator.java @@ -3,6 +3,7 @@ package org.springframework.batch.core.job; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; +import java.util.Set; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersInvalidException; @@ -27,14 +28,19 @@ public class DefaultJobParametersValidator implements JobParametersValidator, In */ public void afterPropertiesSet() throws IllegalStateException { for (String key : requiredKeys) { - Assert.state(!optionalKeys.contains(key), "Optional keys canot be required: "+key); + Assert.state(!optionalKeys.contains(key), "Optional keys canot be required: " + key); } } /** - * Check the parameters meet the specification provided. + * Check the parameters meet the specification provided. If optional keys + * are explicitly specified then all keys must be in that list, or in the + * required list. Otherwise all keys that are specified as required must be + * present. * * @see JobParametersValidator#validate(JobParameters) + * + * @throws JobParametersInvalidException if the parameters are not valid */ public void validate(JobParameters parameters) throws JobParametersInvalidException { @@ -42,9 +48,28 @@ public class DefaultJobParametersValidator implements JobParametersValidator, In throw new JobParametersInvalidException("The JobParameters can not be null"); } + Set keys = parameters.getParameters().keySet(); + + // If there are explicit optional keys then all keys must be in that + // group, or in the required group. + if (!optionalKeys.isEmpty()) { + + Collection missingKeys = new HashSet(); + for (String key : keys) { + if (!optionalKeys.contains(key) && !requiredKeys.contains(key)) { + missingKeys.add(key); + } + } + if (!missingKeys.isEmpty()) { + throw new JobParametersInvalidException( + "The JobParameters contains keys that are not explicitly optional or required: " + missingKeys); + } + + } + Collection missingKeys = new HashSet(); for (String key : requiredKeys) { - if (!parameters.getParameters().containsKey(key)) { + if (!keys.contains(key)) { missingKeys.add(key); } } @@ -55,18 +80,27 @@ public class DefaultJobParametersValidator implements JobParametersValidator, In } /** - * The keys that are required in the parameters. + * The keys that are required in the parameters. The default is empty, + * meaning that all parameters are optional, unless optional keys are + * explicitly specified. * * @param requiredKeys the required key values + * + * @see #setOptionalKeys(String[]) */ public void setRequiredKeys(String[] requiredKeys) { this.requiredKeys = new HashSet(Arrays.asList(requiredKeys)); } /** - * The keys that are optional in the parameters. + * The keys that are optional in the parameters. If any keys are explicitly + * optional, then to be valid all other keys must be explicitly required. + * The default is empty, meaning that all parameters that are not required + * are optional. * * @param optionalKeys the optional key values + * + * @see #setRequiredKeys(String[]) */ public void setOptionalKeys(String[] optionalKeys) { this.optionalKeys = new HashSet(Arrays.asList(optionalKeys)); 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 0871f37ca..f3abe6e81 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 @@ -84,9 +84,6 @@ public class SimpleJobLauncher implements JobLauncher, InitializingBean { Assert.notNull(job, "The Job must not be null."); Assert.notNull(jobParameters, "The JobParameters must not be null."); - // Allow the job to veto the execution - job.validate(jobParameters); - final JobExecution jobExecution; JobExecution lastExecution = jobRepository.getLastJobExecution(job.getName(), jobParameters); if (lastExecution != null) { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/SplitInterruptedJobParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/SplitInterruptedJobParserTests.java index 84be5fa94..bb20ff2f2 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/SplitInterruptedJobParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/SplitInterruptedJobParserTests.java @@ -50,7 +50,7 @@ public class SplitInterruptedJobParserTests extends AbstractJobParserTests { Thread.sleep(200L); assertEquals(BatchStatus.STOPPED, jobExecution.getStatus()); - assertEquals(ExitStatus.STOPPED, jobExecution.getExitStatus()); + assertEquals(ExitStatus.STOPPED.getExitCode(), jobExecution.getExitStatus().getExitCode()); assertTrue(stepNamesList.contains("stop")); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/DefaultJobParametersValidatorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/DefaultJobParametersValidatorTests.java index bb3488b2f..68848a182 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/DefaultJobParametersValidatorTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/DefaultJobParametersValidatorTests.java @@ -14,6 +14,11 @@ public class DefaultJobParametersValidatorTests { validator.validate(null); } + @Test + public void testValidateNoRequiredValues() throws Exception { + validator.validate(new JobParametersBuilder().addString("name", "foo").toJobParameters()); + } + @Test public void testValidateRequiredValues() throws Exception { validator.setRequiredKeys(new String[] { "name", "value" }); @@ -33,6 +38,19 @@ public class DefaultJobParametersValidatorTests { validator.validate(new JobParameters()); } + @Test(expected = JobParametersInvalidException.class) + public void testValidateOptionalWithImplicitRequiredKey() throws Exception { + validator.setOptionalKeys(new String[] { "name", "value" }); + validator.validate(new JobParametersBuilder().addString("foo", "bar").toJobParameters()); + } + + @Test + public void testValidateOptionalWithExplicitRequiredKey() throws Exception { + validator.setOptionalKeys(new String[] { "name", "value" }); + validator.setRequiredKeys(new String[] { "foo" }); + validator.validate(new JobParametersBuilder().addString("foo", "bar").toJobParameters()); + } + @Test(expected = IllegalStateException.class) public void testOptionalValuesAlsoRequired() throws Exception { validator.setOptionalKeys(new String[] { "name", "value" }); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/ExtendedAbstractJobTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/ExtendedAbstractJobTests.java index e78d4502b..049ca1148 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/ExtendedAbstractJobTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/ExtendedAbstractJobTests.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; +import org.junit.Before; import org.junit.Test; import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.JobExecution; @@ -45,7 +46,16 @@ import org.springframework.batch.support.transaction.ResourcelessTransactionMana */ public class ExtendedAbstractJobTests { - AbstractJob job = new StubJob("job"); + private AbstractJob job; + private JobRepository jobRepository; + + @Before + public void setUp() throws Exception { + MapJobRepositoryFactoryBean factory = new MapJobRepositoryFactoryBean(); + factory.setTransactionManager(new ResourcelessTransactionManager()); + jobRepository = (JobRepository) factory.getObject(); + job = new StubJob("job", jobRepository); + } /** * Test method for @@ -75,7 +85,7 @@ public class ExtendedAbstractJobTests { */ @Test public void testSetBeanNameWithNullName() { - job = new StubJob(null); + job = new StubJob(null, null); assertEquals(null, job.getName()); job.setBeanName("foo"); assertEquals("foo", job.getName()); @@ -111,26 +121,27 @@ public class ExtendedAbstractJobTests { } } - @Test(expected=JobParametersInvalidException.class) - public void testValidatorWithNullParameters() throws Exception { - job.validate(null); - } - @Test public void testValidatorWithNotNullParameters() throws Exception { - job.validate(new JobParameters()); + JobExecution execution = jobRepository.createJobExecution("job", new JobParameters()); + job.execute(execution); // Should be free of side effects } - @Test(expected=JobParametersInvalidException.class) + @Test public void testSetValidator() throws Exception { job.setJobParametersValidator(new DefaultJobParametersValidator() { @Override public void validate(JobParameters parameters) throws JobParametersInvalidException { - throw new JobParametersInvalidException("Expected"); + throw new JobParametersInvalidException("FOO"); } }); - job.validate(new JobParameters()); + JobExecution execution = jobRepository.createJobExecution("job", new JobParameters()); + job.execute(execution); + assertEquals(BatchStatus.FAILED, execution.getStatus()); + assertEquals("FOO", execution.getFailureExceptions().get(0).getMessage()); + String description = execution.getExitStatus().getExitDescription(); + assertTrue("Wrong description: "+description, description.contains("FOO")); } /** @@ -183,9 +194,16 @@ public class ExtendedAbstractJobTests { private static class StubJob extends AbstractJob { /** * @param name + * @param jobRepository */ - private StubJob(String name) { + private StubJob(String name, JobRepository jobRepository) { super(name); + try { + setJobRepository(jobRepository); + } + catch (Exception e) { + throw new IllegalStateException(e); + } } /** diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java index 2e102a54c..9a894829d 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java @@ -21,9 +21,7 @@ import java.util.List; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; -import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersIncrementer; -import org.springframework.batch.core.JobParametersInvalidException; import org.springframework.batch.core.Step; import org.springframework.batch.core.UnexpectedJobExecutionException; import org.springframework.beans.factory.BeanNameAware; @@ -135,13 +133,6 @@ public class JobSupport implements BeanNameAware, Job { return restartable; } - /** - * @see Job#validate(JobParameters) - */ - public void validate(JobParameters parameters) throws JobParametersInvalidException { - - } - /* * (non-Javadoc) * diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/launch/SimpleJobLauncherTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/launch/SimpleJobLauncherTests.java index dfae24a1c..fa5eda747 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/launch/SimpleJobLauncherTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/launch/SimpleJobLauncherTests.java @@ -72,7 +72,10 @@ public class SimpleJobLauncherTests { @Test public void testRun() throws Exception { + run(ExitStatus.COMPLETED); + } + private void run(ExitStatus exitStatus) throws Exception { JobExecution jobExecution = new JobExecution(null, null); expect(jobRepository.getLastJobExecution(job.getName(), jobParameters)).andReturn(null); @@ -80,10 +83,13 @@ public class SimpleJobLauncherTests { replay(jobRepository); jobLauncher.afterPropertiesSet(); - jobLauncher.run(job, jobParameters); - assertEquals(ExitStatus.COMPLETED, jobExecution.getExitStatus()); - - verify(jobRepository); + try { + jobLauncher.run(job, jobParameters); + } + finally { + assertEquals(exitStatus, jobExecution.getExitStatus()); + verify(jobRepository); + } } /* @@ -142,7 +148,7 @@ public class SimpleJobLauncherTests { } }; try { - testRun(); + run(ExitStatus.FAILED); fail("Expected RuntimeException"); } catch (RuntimeException e) { @@ -159,7 +165,7 @@ public class SimpleJobLauncherTests { } }; try { - testRun(); + run(ExitStatus.FAILED); fail("Expected Error"); } catch (RuntimeException e) { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java index ff1e0cafd..e7e33df61 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java @@ -697,9 +697,7 @@ public class TaskletStepTests { step.execute(stepExecution); assertEquals(BatchStatus.UNKNOWN, stepExecution.getStatus()); - String msg = stepExecution.getExitStatus().getExitDescription(); Throwable ex = stepExecution.getFailureExceptions().get(0); - msg = ex.getMessage(); // The original rollback failed because of this one: assertEquals("Bar", ex.getMessage()); } @@ -890,16 +888,10 @@ public class TaskletStepTests { private boolean restoreFromCalled = false; - private boolean restoreFromCalledWithSomeContext = false; - public String read() throws Exception { return "item"; } - public boolean isRestoreFromCalledWithSomeContext() { - return restoreFromCalledWithSomeContext; - } - public void update(ExecutionContext executionContext) { getExecutionAttributesCalled = true; executionContext.putString("spam", "bucket"); @@ -920,10 +912,6 @@ public class TaskletStepTests { public void beforeStep(StepExecution stepExecution) { } - public ExitStatus onErrorInStep(StepExecution stepExecution, Throwable e) { - return null; - } - } } diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/support/JobSupport.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/support/JobSupport.java index 6815d4ceb..952692366 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/support/JobSupport.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/support/JobSupport.java @@ -21,9 +21,7 @@ import java.util.List; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; -import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersIncrementer; -import org.springframework.batch.core.JobParametersInvalidException; import org.springframework.batch.core.Step; import org.springframework.batch.core.UnexpectedJobExecutionException; import org.springframework.beans.factory.BeanNameAware; @@ -99,10 +97,6 @@ public class JobSupport implements BeanNameAware, Job { return name; } - public void validate(JobParameters parameters) throws JobParametersInvalidException { - // no-op - } - public void setSteps(List steps) { this.steps.clear(); this.steps.addAll(steps);