fix(step name unicity WITHIN A FLOW)

Author: Fabrice Bibonne

spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java

@@ -374,20 +374,6 @@ public final void execute(JobExecution execution) {
 
 	protected abstract void checkStepNamesUnicity() throws AlreadyUsedStepNameException ;
 
-	private Optional<String> findFirstDoubleElementInList(List<String> strings) {
-		if (strings==null){
-			return Optional.empty();
-		}
-		Set<String> alreadyChecked=new HashSet<>();
-		for (String value:strings){
-			if (alreadyChecked.contains(value)){
-				return Optional.of(value);
-			}
-			alreadyChecked.add(value);
-		}
-		return Optional.empty();
-	}
-
 	private void stopObservation(JobExecution execution, Observation observation) {
 		List<Throwable> throwables = execution.getFailureExceptions();
 		if (!throwables.isEmpty()) {

spring-batch-core/src/main/java/org/springframework/batch/core/job/SimpleJob.java

@@ -25,7 +25,6 @@
 import org.springframework.batch.core.StartLimitExceededException;
 import org.springframework.batch.core.Step;
 import org.springframework.batch.core.StepExecution;
-import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
 import org.springframework.batch.core.repository.JobRestartException;
 import org.springframework.batch.core.step.StepLocator;
 
@@ -145,9 +144,8 @@ protected void doExecute(JobExecution execution)
 	}
 
 	@Override
-	protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
-		Map<String, Step> map = new HashMap<>();
-		steps.forEach(step->{addToMapCheckingUnicity(map, step, step.getName());});
+	protected void checkStepNamesUnicity() {
+		//noop : steps of SimpleJob can share the same name
 	}
 
 }

spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/FlowJob.java

@@ -85,20 +85,20 @@ public Step getStep(String stepName) {
 	 */
 	private void init() {
 		if (!initialized) {
-			findStepsThrowingIfNameNotUnique(flow, stepMap);
+			findStepsThrowingIfNameNotUnique(flow);
 			initialized = true;
 		}
 	}
 
-	private void findStepsThrowingIfNameNotUnique(Flow flow, Map<String, Step> map) {
+	private void findStepsThrowingIfNameNotUnique(Flow flow) {
 
 		for (State state : flow.getStates()) {
 			if (state instanceof StepLocator locator) {
 				for (String name : locator.getStepNames()) {
-					addToMapCheckingUnicity(map, locator.getStep(name), name);
+					addToMapCheckingUnicity(this.stepMap, locator.getStep(name), name);
 				}
 			}
-			//TODO remove this else bock ? not executed during tests : the only State wich implements StepHolder is StepState which implements also StepLocator
+			//TODO remove this else bock ? not executed during tests : the only State which implements StepHolder is StepState which already implements StepLocator
 			/*
 			Tests Coverage
 			Hits : 30
@@ -108,11 +108,11 @@ private void findStepsThrowingIfNameNotUnique(Flow flow, Map<String, Step> map)
 			*/
 			else if (state instanceof StepHolder stepHolder) {
 				Step step = stepHolder.getStep();
-				addToMapCheckingUnicity(map, step, step.getName());
+				addToMapCheckingUnicity(this.stepMap, step, step.getName());
 			}
 			else if (state instanceof FlowHolder flowHolder) {
 				for (Flow subflow : flowHolder.getFlows()) {
-					findStepsThrowingIfNameNotUnique(subflow, map);
+					findStepsThrowingIfNameNotUnique(subflow);
 				}
 			}
 		}

spring-batch-core/src/test/java/org/springframework/batch/core/job/ExtendedAbstractJobTests.java

@@ -17,33 +17,20 @@
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.springframework.batch.core.BatchStatus;
-import org.springframework.batch.core.JobExecution;
-import org.springframework.batch.core.JobExecutionException;
-import org.springframework.batch.core.JobInterruptedException;
-import org.springframework.batch.core.JobParameters;
-import org.springframework.batch.core.JobParametersInvalidException;
-import org.springframework.batch.core.Step;
-import org.springframework.batch.core.StepExecution;
-import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
+import org.springframework.batch.core.*;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean;
 import org.springframework.batch.core.step.StepSupport;
-import org.springframework.jdbc.support.JdbcTransactionManager;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
+import org.springframework.jdbc.support.JdbcTransactionManager;
 import org.springframework.lang.Nullable;
 
 import java.time.LocalDateTime;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.List;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 /**
  * @author Dave Syer
@@ -218,8 +205,7 @@ protected void doExecute(JobExecution execution) throws JobExecutionException {
 		}
 
 		@Override
-		protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
-
+		protected void checkStepNamesUnicity(){
 		}
 
 		@Override

spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java

@@ -513,6 +513,25 @@ void testGetMultipleJobParameters() throws Exception {
 
 	}
 
+	@Test
+	public void testMultipleStepsWithSameName(){
+		job.setName("MultipleStepsWithSameName");
+		String sharedName="stepName";
+		final List<String> executionsCallbacks=new ArrayList<>();
+		StubStep sharedNameStep1=new StubStep(sharedName, jobRepository);
+		sharedNameStep1.setCallback(()->executionsCallbacks.add("step1"));
+		job.addStep(sharedNameStep1);
+		StubStep sharedNameStep2=new StubStep(sharedName, jobRepository);
+		sharedNameStep2.setCallback(()->executionsCallbacks.add("step2"));
+		job.addStep(sharedNameStep2);
+		StubStep sharedNameStep3=new StubStep(sharedName, jobRepository);
+		sharedNameStep3.setCallback(()->executionsCallbacks.add("step3"));
+		job.addStep(sharedNameStep3);
+		job.execute(jobExecution);
+		assertEquals(List.of("step1", "step2", "step3"), executionsCallbacks);
+		assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus());
+	}
+
 	/*
 	 * Check JobRepository to ensure status is being saved.
 	 */

spring-batch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java

@@ -55,7 +55,6 @@
 
 import static org.junit.Assert.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * @author Dave Syer

spring-batch-core/src/test/java/org/springframework/batch/core/launch/support/SimpleJobOperatorTests.java

@@ -43,7 +43,6 @@
 import org.springframework.batch.core.explore.JobExplorer;
 import org.springframework.batch.core.job.AbstractJob;
 import org.springframework.batch.core.job.JobSupport;
-import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
 import org.springframework.batch.core.launch.JobInstanceAlreadyExistsException;
 import org.springframework.batch.core.launch.NoSuchJobException;
 import org.springframework.batch.core.launch.NoSuchJobExecutionException;
@@ -457,7 +456,7 @@ protected void doExecute(JobExecution execution) throws JobExecutionException {
 		}
 
 		@Override
-		protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
+		protected void checkStepNamesUnicity() {
 		}
 
 	}