Skip to content

Commit 6a5a14b

Browse files
Fabrice BibonneFBibonne
Fabrice Bibonne
authored and
FBibonne
committed
feat(step name unicity) spring-projects#3757
the names of different steps in a job must be different Signed-off-by: Fabrice Bibonne <[email protected]>
1 parent 9c6c392 commit 6a5a14b

File tree

6 files changed

+91
-33
lines changed

6 files changed

+91
-33
lines changed

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@
1717
package org.springframework.batch.core.job;
1818

1919
import java.time.LocalDateTime;
20-
import java.util.Collection;
21-
import java.util.List;
20+
import java.util.*;
2221
import java.util.stream.Collectors;
2322

2423
import io.micrometer.core.instrument.LongTaskTimer;
@@ -43,6 +42,7 @@
4342
import org.springframework.batch.core.StartLimitExceededException;
4443
import org.springframework.batch.core.Step;
4544
import org.springframework.batch.core.StepExecution;
45+
import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
4646
import org.springframework.batch.core.launch.NoSuchJobException;
4747
import org.springframework.batch.core.launch.support.ExitCodeMapper;
4848
import org.springframework.batch.core.listener.CompositeJobExecutionListener;
@@ -300,6 +300,7 @@ public final void execute(JobExecution execution) {
300300

301301
execution.setStartTime(LocalDateTime.now());
302302
updateStatus(execution, BatchStatus.STARTED);
303+
checkStepNamesUnicity();
303304

304305
listener.beforeJob(execution);
305306

@@ -368,9 +369,23 @@ public final void execute(JobExecution execution) {
368369
finally {
369370
JobSynchronizationManager.release();
370371
}
371-
372372
}
373+
}
374+
375+
protected abstract void checkStepNamesUnicity() throws AlreadyUsedStepNameException ;
373376

377+
private Optional<String> findFirstDoubleElementInList(List<String> strings) {
378+
if (strings==null){
379+
return Optional.empty();
380+
}
381+
Set<String> alreadyChecked=new HashSet<>();
382+
for (String value:strings){
383+
if (alreadyChecked.contains(value)){
384+
return Optional.of(value);
385+
}
386+
alreadyChecked.add(value);
387+
}
388+
return Optional.empty();
374389
}
375390

376391
private void stopObservation(JobExecution execution, Observation observation) {
@@ -430,6 +445,15 @@ else if (ex instanceof NoSuchJobException || ex.getCause() instanceof NoSuchJobE
430445
return exitStatus;
431446
}
432447

448+
protected static void addToMapCheckingUnicity(Map<String, Step> map, Step step, String name) throws AlreadyUsedStepNameException {
449+
map.merge(name, step, (old, value)->{
450+
if (!old.equals(value)){
451+
throw new AlreadyUsedStepNameException(name);
452+
}
453+
return old;
454+
});
455+
}
456+
433457
private void updateStatus(JobExecution jobExecution, BatchStatus status) {
434458
jobExecution.setStatus(status);
435459
jobRepository.update(jobExecution);

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616

1717
package org.springframework.batch.core.job;
1818

19-
import java.util.ArrayList;
20-
import java.util.Collection;
21-
import java.util.List;
19+
import java.util.*;
2220

2321
import org.springframework.batch.core.BatchStatus;
2422
import org.springframework.batch.core.Job;
@@ -27,6 +25,7 @@
2725
import org.springframework.batch.core.StartLimitExceededException;
2826
import org.springframework.batch.core.Step;
2927
import org.springframework.batch.core.StepExecution;
28+
import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
3029
import org.springframework.batch.core.repository.JobRestartException;
3130
import org.springframework.batch.core.step.StepLocator;
3231

@@ -145,4 +144,10 @@ protected void doExecute(JobExecution execution)
145144
}
146145
}
147146

147+
@Override
148+
protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
149+
Map<String, Step> map = new HashMap<>();
150+
steps.forEach(step->{addToMapCheckingUnicity(map, step, step.getName());});
151+
}
152+
148153
}

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

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,20 @@
1515
*/
1616
package org.springframework.batch.core.job.flow;
1717

18-
import java.util.Collection;
19-
import java.util.Map;
20-
import java.util.concurrent.ConcurrentHashMap;
21-
2218
import org.springframework.batch.core.Job;
2319
import org.springframework.batch.core.JobExecution;
2420
import org.springframework.batch.core.JobExecutionException;
2521
import org.springframework.batch.core.Step;
2622
import org.springframework.batch.core.job.AbstractJob;
2723
import org.springframework.batch.core.job.SimpleStepHandler;
24+
import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
2825
import org.springframework.batch.core.step.StepHolder;
2926
import org.springframework.batch.core.step.StepLocator;
3027

28+
import java.util.Collection;
29+
import java.util.Map;
30+
import java.util.concurrent.ConcurrentHashMap;
31+
3132
/**
3233
* Implementation of the {@link Job} interface that allows for complex flows of steps,
3334
* rather than requiring sequential execution. In general, this job implementation was
@@ -74,50 +75,55 @@ public void setFlow(Flow flow) {
7475
*/
7576
@Override
7677
public Step getStep(String stepName) {
77-
if (!initialized) {
78-
init();
79-
}
78+
init();
8079
return stepMap.get(stepName);
8180
}
8281

82+
8383
/**
8484
* Initialize the step names
8585
*/
8686
private void init() {
87-
findSteps(flow, stepMap);
88-
initialized = true;
87+
if (!initialized) {
88+
findStepsThrowingIfNameNotUnique(flow, stepMap);
89+
initialized = true;
90+
}
8991
}
9092

91-
private void findSteps(Flow flow, Map<String, Step> map) {
93+
private void findStepsThrowingIfNameNotUnique(Flow flow, Map<String, Step> map) {
9294

9395
for (State state : flow.getStates()) {
9496
if (state instanceof StepLocator locator) {
9597
for (String name : locator.getStepNames()) {
96-
map.put(name, locator.getStep(name));
98+
addToMapCheckingUnicity(map, locator.getStep(name), name);
9799
}
98100
}
99-
else if (state instanceof StepHolder) {
100-
Step step = ((StepHolder) state).getStep();
101-
String name = step.getName();
102-
stepMap.put(name, step);
101+
//TODO remove this else bock ? not executed during tests : the only State wich implements StepHolder is StepState which implements also StepLocator
102+
/*
103+
Tests Coverage
104+
Hits : 30
105+
state instanceof StepHolder
106+
true hits: 0
107+
false hits : 30
108+
*/
109+
else if (state instanceof StepHolder stepHolder) {
110+
Step step = stepHolder.getStep();
111+
addToMapCheckingUnicity(map, step, step.getName());
103112
}
104-
else if (state instanceof FlowHolder) {
105-
for (Flow subflow : ((FlowHolder) state).getFlows()) {
106-
findSteps(subflow, map);
113+
else if (state instanceof FlowHolder flowHolder) {
114+
for (Flow subflow : flowHolder.getFlows()) {
115+
findStepsThrowingIfNameNotUnique(subflow, map);
107116
}
108117
}
109118
}
110-
111119
}
112120

113121
/**
114122
* {@inheritDoc}
115123
*/
116124
@Override
117125
public Collection<String> getStepNames() {
118-
if (!initialized) {
119-
init();
120-
}
126+
init();
121127
return stepMap.keySet();
122128
}
123129

@@ -139,4 +145,9 @@ protected void doExecute(final JobExecution execution) throws JobExecutionExcept
139145
}
140146
}
141147

148+
@Override
149+
protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
150+
init();
151+
}
152+
142153
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.batch.core.JobParametersInvalidException;
2626
import org.springframework.batch.core.Step;
2727
import org.springframework.batch.core.StepExecution;
28+
import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
2829
import org.springframework.batch.core.repository.JobRepository;
2930
import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean;
3031
import org.springframework.batch.core.step.StepSupport;
@@ -36,6 +37,7 @@
3637
import java.time.LocalDateTime;
3738
import java.util.Collection;
3839
import java.util.Collections;
40+
import java.util.List;
3941

4042
import static org.junit.jupiter.api.Assertions.assertEquals;
4143
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -215,6 +217,11 @@ public StubJob() {
215217
protected void doExecute(JobExecution execution) throws JobExecutionException {
216218
}
217219

220+
@Override
221+
protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
222+
223+
}
224+
218225
@Override
219226
public Step getStep(String stepName) {
220227
return null;

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@
2828
import org.springframework.batch.core.job.flow.JobExecutionDecider;
2929
import org.springframework.batch.core.job.flow.support.SimpleFlow;
3030
import org.springframework.batch.core.launch.JobLauncher;
31+
import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException;
32+
import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException;
3133
import org.springframework.batch.core.repository.JobRepository;
34+
import org.springframework.batch.core.repository.JobRestartException;
3235
import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean;
3336
import org.springframework.batch.core.scope.context.ChunkContext;
3437
import org.springframework.batch.core.step.StepSupport;
@@ -54,8 +57,7 @@
5457
import java.util.concurrent.CountDownLatch;
5558
import java.util.concurrent.TimeUnit;
5659

57-
import static org.junit.jupiter.api.Assertions.assertEquals;
58-
import static org.junit.jupiter.api.Assertions.assertThrows;
60+
import static org.junit.jupiter.api.Assertions.*;
5961

6062
/**
6163
* @author Dave Syer
@@ -365,12 +367,16 @@ void testBuildWithJobScopedStep() throws Exception {
365367

366368
//https://github.com/spring-projects/spring-batch/issues/3757#issuecomment-1821593539
367369
@Test
368-
void testStepNamesMustBeUniqueWithinFlowDefinition() {
370+
void testStepNamesMustBeUniqueWithinFlowDefinition() throws JobInstanceAlreadyCompleteException, JobExecutionAlreadyRunningException, JobParametersInvalidException, JobRestartException {
369371
ApplicationContext context = new AnnotationConfigApplicationContext(JobConfigurationForStepNameUnique.class);
370372
JobLauncher jobLauncher = context.getBean(JobLauncher.class);
371373
Job job = context.getBean(Job.class);
372-
assertThrows(AlreadyUsedStepNameException.class, ()->jobLauncher.run(job, new JobParametersBuilder().addLong("random", 2L).addString("stepTwo.name", JobConfigurationForStepNameUnique.SHARED_NAME).toJobParameters()));
373-
assertThrows(AlreadyUsedStepNameException.class, ()->jobLauncher.run(job, new JobParametersBuilder().addLong("random", 1L).addString("stepTwo.name",JobConfigurationForStepNameUnique.SHARED_NAME).toJobParameters()));
374+
JobExecution jobExecution=jobLauncher.run(job, new JobParametersBuilder().addLong("random", 2L).addString("stepTwo.name", JobConfigurationForStepNameUnique.SHARED_NAME).toJobParameters());
375+
assertTrue(jobExecution.getAllFailureExceptions().stream().map(Object::getClass).anyMatch(AlreadyUsedStepNameException.class::equals));
376+
assertEquals(ExitStatus.FAILED.getExitCode(), jobExecution.getExitStatus().getExitCode());
377+
jobExecution=jobLauncher.run(job, new JobParametersBuilder().addLong("random", 1L).addString("stepTwo.name", JobConfigurationForStepNameUnique.SHARED_NAME).toJobParameters());
378+
assertTrue(jobExecution.getAllFailureExceptions().stream().map(Object::getClass).anyMatch(AlreadyUsedStepNameException.class::equals));
379+
assertEquals(ExitStatus.FAILED.getExitCode(), jobExecution.getExitStatus().getExitCode());
374380
}
375381

376382
@EnableBatchProcessing

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.springframework.batch.core.explore.JobExplorer;
4444
import org.springframework.batch.core.job.AbstractJob;
4545
import org.springframework.batch.core.job.JobSupport;
46+
import org.springframework.batch.core.job.builder.AlreadyUsedStepNameException;
4647
import org.springframework.batch.core.launch.JobInstanceAlreadyExistsException;
4748
import org.springframework.batch.core.launch.NoSuchJobException;
4849
import org.springframework.batch.core.launch.NoSuchJobExecutionException;
@@ -455,6 +456,10 @@ protected void doExecute(JobExecution execution) throws JobExecutionException {
455456

456457
}
457458

459+
@Override
460+
protected void checkStepNamesUnicity() throws AlreadyUsedStepNameException {
461+
}
462+
458463
}
459464

460465
}

0 commit comments

Comments
 (0)