feat(step name unicity) spring-projects#3757

the names of different steps in a job must be different

Author: Fabrice Bibonne

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

@@ -17,8 +17,7 @@
 package org.springframework.batch.core.job;
 
 import java.time.LocalDateTime;
-import java.util.Collection;
-import java.util.List;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import io.micrometer.core.instrument.LongTaskTimer;
@@ -43,6 +42,7 @@
 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.launch.NoSuchJobException;
 import org.springframework.batch.core.launch.support.ExitCodeMapper;
 import org.springframework.batch.core.listener.CompositeJobExecutionListener;
@@ -300,6 +300,7 @@ public final void execute(JobExecution execution) {
 
 				execution.setStartTime(LocalDateTime.now());
 				updateStatus(execution, BatchStatus.STARTED);
+				checkStepNamesUnicity();
 
 				listener.beforeJob(execution);
 
@@ -368,9 +369,23 @@ public final void execute(JobExecution execution) {
 			finally {
 				JobSynchronizationManager.release();
 			}
-
 		}
+	}
+
+	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) {
@@ -430,6 +445,15 @@ else if (ex instanceof NoSuchJobException || ex.getCause() instanceof NoSuchJobE
 		return exitStatus;
 	}
 
+	protected static void addToMapCheckingUnicity(Map<String, Step> map, Step step, String name) throws AlreadyUsedStepNameException {
+		map.merge(name, step, (old, value)->{
+			if (!old.equals(value)){
+				throw new AlreadyUsedStepNameException(name);
+			}
+			return old;
+		});
+	}
+
 	private void updateStatus(JobExecution jobExecution, BatchStatus status) {
 		jobExecution.setStatus(status);
 		jobRepository.update(jobExecution);

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

@@ -16,9 +16,7 @@
 
 package org.springframework.batch.core.job;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
+import java.util.*;
 
 import org.springframework.batch.core.BatchStatus;
 import org.springframework.batch.core.Job;
@@ -27,6 +25,7 @@
 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,4 +144,10 @@ 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());});
+	}
+
 }

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

@@ -15,19 +15,20 @@
  */
 package org.springframework.batch.core.job.flow;
 
-import java.util.Collection;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-
 import org.springframework.batch.core.Job;
 import org.springframework.batch.core.JobExecution;
 import org.springframework.batch.core.JobExecutionException;
 import org.springframework.batch.core.Step;
 import org.springframework.batch.core.job.AbstractJob;
 import org.springframework.batch.core.job.SimpleStepHandler;
+import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
 import org.springframework.batch.core.step.StepHolder;
 import org.springframework.batch.core.step.StepLocator;
 
+import java.util.Collection;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
 /**
  * Implementation of the {@link Job} interface that allows for complex flows of steps,
  * rather than requiring sequential execution. In general, this job implementation was
@@ -74,50 +75,55 @@ public void setFlow(Flow flow) {
 	 */
 	@Override
 	public Step getStep(String stepName) {
-		if (!initialized) {
-			init();
-		}
+		init();
 		return stepMap.get(stepName);
 	}
 
+
 	/**
 	 * Initialize the step names
 	 */
 	private void init() {
-		findSteps(flow, stepMap);
-		initialized = true;
+		if (!initialized) {
+			findStepsThrowingIfNameNotUnique(flow, stepMap);
+			initialized = true;
+		}
 	}
 
-	private void findSteps(Flow flow, Map<String, Step> map) {
+	private void findStepsThrowingIfNameNotUnique(Flow flow, Map<String, Step> map) {
 
 		for (State state : flow.getStates()) {
 			if (state instanceof StepLocator locator) {
 				for (String name : locator.getStepNames()) {
-					map.put(name, locator.getStep(name));
+					addToMapCheckingUnicity(map, locator.getStep(name), name);
 				}
 			}
-			else if (state instanceof StepHolder) {
-				Step step = ((StepHolder) state).getStep();
-				String name = step.getName();
-				stepMap.put(name, step);
+			//TODO remove this else bock ? not executed during tests : the only State wich implements StepHolder is StepState which implements also StepLocator
+			/*
+			Tests Coverage
+			Hits : 30
+				state instanceof StepHolder
+					true hits: 0
+					false hits : 30
+			*/
+			else if (state instanceof StepHolder stepHolder) {
+				Step step = stepHolder.getStep();
+				addToMapCheckingUnicity(map, step, step.getName());
 			}
-			else if (state instanceof FlowHolder) {
-				for (Flow subflow : ((FlowHolder) state).getFlows()) {
-					findSteps(subflow, map);
+			else if (state instanceof FlowHolder flowHolder) {
+				for (Flow subflow : flowHolder.getFlows()) {
+					findStepsThrowingIfNameNotUnique(subflow, map);
 				}
 			}
 		}
-
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	@Override
 	public Collection<String> getStepNames() {
-		if (!initialized) {
-			init();
-		}
+		init();
 		return stepMap.keySet();
 	}
 
@@ -139,4 +145,9 @@ protected void doExecute(final JobExecution execution) throws JobExecutionExcept
 		}
 	}
 
+	@Override
+	protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
+		init();
+	}
+
 }

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

@@ -25,6 +25,7 @@
 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.repository.JobRepository;
 import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean;
 import org.springframework.batch.core.step.StepSupport;
@@ -36,6 +37,7 @@
 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;
@@ -215,6 +217,11 @@ public StubJob() {
 		protected void doExecute(JobExecution execution) throws JobExecutionException {
 		}
 
+		@Override
+		protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
+
+		}
+
 		@Override
 		public Step getStep(String stepName) {
 			return null;