diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/JobParametersBuilder.java b/spring-batch-core/src/main/java/org/springframework/batch/core/JobParametersBuilder.java index a0728c5fbe..02fdef9458 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/JobParametersBuilder.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/JobParametersBuilder.java @@ -29,7 +29,7 @@ /** * Helper class for creating {@link JobParameters}. Useful because all * {@link JobParameter} objects are immutable, and must be instantiated separately - * to ensure typesafety. Once created, it can be used in the + * to ensure type safety. Once created, it can be used in the * same was a java.lang.StringBuilder (except, order is irrelevant), by adding * various parameter types and creating a valid {@link JobParameters} once * finished.
@@ -239,6 +239,10 @@ public JobParametersBuilder addJobParameters(JobParameters jobParameters) { /** * Initializes the {@link JobParameters} based on the state of the {@link Job}. This * should be called after all parameters have been entered into the builder. + * All parameters already set on this builder instance will be appended to + * those retrieved from the job incrementer, overriding any with the same key (Same + * behaviour as {@link org.springframework.batch.core.launch.support.CommandLineJobRunner} + * with "-next" option and {@link org.springframework.batch.core.launch.JobOperator#startNextInstance(String)}) * * @param job the job for which the {@link JobParameters} are being constructed. * @return a reference to this object. @@ -246,59 +250,35 @@ public JobParametersBuilder addJobParameters(JobParameters jobParameters) { * @since 4.0 */ public JobParametersBuilder getNextJobParameters(Job job) { - if(this.jobExplorer == null) { - throw new IllegalStateException("A JobExplore is required to get next job parameters"); - } + Assert.state(this.jobExplorer != null, "A JobExplorer is required to get next job parameters"); + Assert.notNull(job, "Job must not be null"); + Assert.notNull(job.getJobParametersIncrementer(), "No job parameters incrementer found for job=" + job.getName()); String name = job.getName(); - JobParameters nextParameters = new JobParameters(); + JobParameters nextParameters; List lastInstances = this.jobExplorer.getJobInstances(name, 0, 1); JobParametersIncrementer incrementer = job.getJobParametersIncrementer(); if (lastInstances.isEmpty()) { // Start from a completely clean sheet - if (incrementer != null) { - nextParameters = incrementer.getNext(new JobParameters()); - } + nextParameters = incrementer.getNext(new JobParameters()); } else { - List previousExecutions = this.jobExplorer - .getJobExecutions(lastInstances.get(0)); - JobExecution previousExecution = previousExecutions.get(0); - if (previousExecution == null) { + List previousExecutions = this.jobExplorer.getJobExecutions(lastInstances.get(0)); + if (previousExecutions.isEmpty()) { // Normally this will not happen - an instance exists with no executions - if (incrementer != null) { - nextParameters = incrementer.getNext(new JobParameters()); - } - } - else if (isStoppedOrFailed(previousExecution) && job.isRestartable()) { - // Retry a failed or stopped execution - nextParameters = previousExecution.getJobParameters(); - // Non-identifying additional parameters can be removed to a retry - removeNonIdentifying(this.parameterMap); + nextParameters = incrementer.getNext(new JobParameters()); } - else if (incrementer != null) { - // New instance so increment the parameters if we can + else { + JobExecution previousExecution = previousExecutions.get(0); nextParameters = incrementer.getNext(previousExecution.getJobParameters()); } } - this.parameterMap = addJobParameters(nextParameters) - .toJobParameters() - .getParameters(); + // start with parameters from the incrementer + Map nextParametersMap = new HashMap<>(nextParameters.getParameters()); + // append new parameters (overriding those with the same key) + nextParametersMap.putAll(this.parameterMap); + this.parameterMap = nextParametersMap; return this; } - - private void removeNonIdentifying(Map parameters) { - HashMap copy = new HashMap<>(parameters); - for (Map.Entry parameter : copy.entrySet()) { - if (!parameter.getValue().isIdentifying()) { - parameters.remove(parameter.getKey()); - } - } - } - - private boolean isStoppedOrFailed(JobExecution execution) { - BatchStatus status = execution.getStatus(); - return (status == BatchStatus.STOPPED || status == BatchStatus.FAILED); - } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobRunner.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobRunner.java index 3bcf735e92..13a09b9c40 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobRunner.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobRunner.java @@ -21,10 +21,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.Set; @@ -36,8 +34,8 @@ import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobInstance; -import org.springframework.batch.core.JobParameter; import org.springframework.batch.core.JobParameters; +import org.springframework.batch.core.JobParametersBuilder; import org.springframework.batch.core.JobParametersIncrementer; import org.springframework.batch.core.configuration.JobLocator; import org.springframework.batch.core.converter.DefaultJobParametersConverter; @@ -47,7 +45,6 @@ import org.springframework.batch.core.launch.JobExecutionNotRunningException; import org.springframework.batch.core.launch.JobExecutionNotStoppedException; import org.springframework.batch.core.launch.JobLauncher; -import org.springframework.batch.core.launch.JobParametersNotFoundException; import org.springframework.batch.core.launch.NoSuchJobException; import org.springframework.batch.core.repository.JobRepository; import org.springframework.beans.factory.BeanDefinitionStoreException; @@ -356,10 +353,9 @@ int start(String jobPath, String jobIdentifier, String[] parameters, Set } if (opts.contains("-next")) { - JobParameters nextParameters = getNextJobParameters(job); - Map map = new HashMap(nextParameters.getParameters()); - map.putAll(jobParameters.getParameters()); - jobParameters = new JobParameters(map); + jobParameters = new JobParametersBuilder(jobParameters, jobExplorer) + .getNextJobParameters(job) + .toJobParameters(); } JobExecution jobExecution = launcher.run(job, jobParameters); @@ -469,35 +465,6 @@ private Long getLongIdentifier(String jobIdentifier) { } } - /** - * @param job the job that we need to find the next parameters for - * @return the next job parameters if they can be located - * @throws JobParametersNotFoundException if there is a problem - */ - private JobParameters getNextJobParameters(Job job) throws JobParametersNotFoundException { - String jobIdentifier = job.getName(); - JobParameters jobParameters; - List lastInstances = jobExplorer.getJobInstances(jobIdentifier, 0, 1); - - JobParametersIncrementer incrementer = job.getJobParametersIncrementer(); - if (incrementer == null) { - throw new JobParametersNotFoundException("No job parameters incrementer found for job=" + jobIdentifier); - } - - if (lastInstances.isEmpty()) { - jobParameters = incrementer.getNext(new JobParameters()); - if (jobParameters == null) { - throw new JobParametersNotFoundException("No bootstrap parameters found from incrementer for job=" - + jobIdentifier); - } - } - else { - List lastExecutions = jobExplorer.getJobExecutions(lastInstances.get(0)); - jobParameters = incrementer.getNext(lastExecutions.get(0).getJobParameters()); - } - return jobParameters; - } - /** * Launch a batch job using a {@link CommandLineJobRunner}. Creates a new * Spring context for the job execution, and uses a common parent for all @@ -515,7 +482,7 @@ private JobParameters getNextJobParameters(Job job) throws JobParametersNotFound * interpreted either as the name of the job or the id of the job execution * that failed. *
  • -next: (optional) if the job has a {@link JobParametersIncrementer} - * that can be used to launch the next in a sequence
    • + * that can be used to launch the next instance in a sequence *
  • jobPath: the xml application context containing a {@link Job} *
  • jobIdentifier: the bean id of the job or id of the failed execution * in the case of a restart. diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java index 08d94dc0cc..6bbadefd4c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-2018 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. @@ -31,7 +31,7 @@ import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobInstance; import org.springframework.batch.core.JobParameters; -import org.springframework.batch.core.JobParametersIncrementer; +import org.springframework.batch.core.JobParametersBuilder; import org.springframework.batch.core.JobParametersInvalidException; import org.springframework.batch.core.Step; import org.springframework.batch.core.StepExecution; @@ -45,7 +45,6 @@ import org.springframework.batch.core.launch.JobInstanceAlreadyExistsException; import org.springframework.batch.core.launch.JobLauncher; import org.springframework.batch.core.launch.JobOperator; -import org.springframework.batch.core.launch.JobParametersNotFoundException; import org.springframework.batch.core.launch.NoSuchJobException; import org.springframework.batch.core.launch.NoSuchJobExecutionException; import org.springframework.batch.core.launch.NoSuchJobInstanceException; @@ -79,6 +78,7 @@ * @author Dave Syer * @author Lucas Ward * @author Will Schipp + * @author Mahmoud Ben Hassine * @since 2.0 */ public class SimpleJobOperator implements JobOperator, InitializingBean { @@ -334,30 +334,15 @@ public Long start(String jobName, String parameters) throws NoSuchJobException, * @see JobOperator#startNextInstance(String ) */ @Override - public Long startNextInstance(String jobName) throws NoSuchJobException, JobParametersNotFoundException, + public Long startNextInstance(String jobName) throws NoSuchJobException, UnexpectedJobExecutionException, JobParametersInvalidException { logger.info("Locating parameters for next instance of job with name=" + jobName); Job job = jobRegistry.getJob(jobName); - List lastInstances = jobExplorer.getJobInstances(jobName, 0, 1); - - JobParametersIncrementer incrementer = job.getJobParametersIncrementer(); - if (incrementer == null) { - throw new JobParametersNotFoundException("No job parameters incrementer found for job=" + jobName); - } - - JobParameters parameters; - if (lastInstances.isEmpty()) { - parameters = incrementer.getNext(new JobParameters()); - if (parameters == null) { - throw new JobParametersNotFoundException("No bootstrap parameters found for job=" + jobName); - } - } - else { - List lastExecutions = jobExplorer.getJobExecutions(lastInstances.get(0)); - parameters = incrementer.getNext(lastExecutions.get(0).getJobParameters()); - } + JobParameters parameters = new JobParametersBuilder(jobExplorer) + .getNextJobParameters(job) + .toJobParameters(); logger.info(String.format("Attempting to launch job with name=%s and parameters=%s", jobName, parameters)); try { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParametersBuilderTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParametersBuilderTests.java index 9197d0a48b..2da832c0bc 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParametersBuilderTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParametersBuilderTests.java @@ -23,7 +23,9 @@ import java.util.Properties; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.batch.core.explore.JobExplorer; import org.springframework.batch.core.job.SimpleJob; @@ -56,6 +58,9 @@ public class JobParametersBuilderTests { private Date date = new Date(System.currentTimeMillis()); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Before public void initialize() { this.job = new SimpleJob("simpleJob"); @@ -198,9 +203,10 @@ public void testGetNextJobParametersFirstRun(){ @Test public void testGetNextJobParametersNoIncrementer(){ + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("No job parameters incrementer found for job=simpleJob"); initializeForNextJobParameters(); this.parametersBuilder.getNextJobParameters(this.job); - baseJobParametersVerify(this.parametersBuilder.toJobParameters(), 3); } @Test @@ -226,14 +232,13 @@ public void testGetNextJobParametersRestartable(){ initializeForNextJobParameters(); this.parametersBuilder.addLong("NON_IDENTIFYING_LONG", new Long(1), false); this.parametersBuilder.getNextJobParameters(this.job); - baseJobParametersVerify(this.parametersBuilder.toJobParameters(), 3); + baseJobParametersVerify(this.parametersBuilder.toJobParameters(), 5); } @Test public void testGetNextJobParametersNoPreviousExecution(){ this.job.setJobParametersIncrementer(new RunIdIncrementer()); this.jobInstanceList.add(new JobInstance(1L, "simpleJobInstance")); - this.jobExecutionList.add(null); when(this.jobExplorer.getJobInstances("simpleJob",0,1)).thenReturn(this.jobInstanceList); when(this.jobExplorer.getJobExecutions(any())).thenReturn(this.jobExecutionList); initializeForNextJobParameters(); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/launch/support/SimpleJobOperatorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/launch/support/SimpleJobOperatorTests.java index c53c708f21..f1613f25ec 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/launch/support/SimpleJobOperatorTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/launch/support/SimpleJobOperatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-2018 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. @@ -67,6 +67,7 @@ /** * @author Dave Syer * @author Will Schipp + * @author Mahmoud Ben Hassine * */ public class SimpleJobOperatorTests { @@ -153,6 +154,7 @@ public void testMandatoryProperties() throws Exception { */ @Test public void testStartNextInstanceSunnyDay() throws Exception { + jobParameters = new JobParameters(); JobInstance jobInstance = new JobInstance(321L, "foo"); when(jobExplorer.getJobInstances("foo", 0, 1)).thenReturn(Collections.singletonList(jobInstance)); when(jobExplorer.getJobExecutions(jobInstance)).thenReturn(Collections.singletonList(new JobExecution(jobInstance, new JobParameters())));