Not unique steps names in a FlowJob should raise an exception or at least log a warning

[FBibonne]

When running a job with conditional flow, if steps in this job have the same name, the job doesn't behavior as expected (infinite loop on one step ...) because transitions in the job rely on step names. So in order to avoid such behavior, steps names in a flow job should be unique : spring batch should raise an exception or at least should log a warning.
Furthermore, in xml config, as step names are defined as ids of <step>, they must be unique : it is an xsd constraint.

Here an example :

// Job definition :
public Job flowjob() {
	return jobBuilderFactory.get("flowJob")
			.start(step1())
			.on("ODD").to(step2())
			.from(step1()).on("EVEN").to(step3()
			.from(step3()).next(step4())
			.end()
			.build();
}

// step1 definition with the name of step1 :
private Step step1() {
	return stepBuilderFactory.get("step1").tasklet(new Tasklet() {
		@Override
		public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
			String exitStatus = (System.currentTimeMillis() % 2 == 0) ? "EVEN" : "ODD";
			contribution.setExitStatus(new ExitStatus(exitStatus));
			return RepeatStatus.FINISHED;
		}
	})
        .build();
}

//step2 definition with name of **step1** :
private Step step2() {
	return stepBuilderFactory.get("step1") /*Big mistake !*/
        .tasklet((contrib, chunk)->RepeatStatus.FINISHED)
        .build();
}

// and so on : step3 and step4 are defined as step2 with always the same name (step1)

When running the job, spring batch loops infinitely on step3.

[FBibonne]

Step names are used as identifiers (keys in maps in spring batch internal code) so they should be unique and the piece of code above is a non sense.
But as a new spring batch developer I remember I did the mistake and it took a lot of time to correct it. I saw recently the same mistake by an other new spring btach developer. So I submit this issue mainly thinking about new spring batch developers

[fmbenhassine]

Thank you for raising this issue. That's a valid concern. Step names must be unique within a flow definition and indeed, this is not enforced currently (as of v5.0.3).

The samples in documentation were updated in dde5455 (with a warning about the importance of unique step names in flow definitions), but it is better to check this programmatically and raise and exception or log a warning when this constraint is violated. Adding name-based equals and hashCode to AbstractStep might improve things, but probably not much after this commit: edc197a. That fix should probably be reviewed/reverted and the original issue should be fixed differently (like detecting if the step is a proxy and get its name from the target object).

This will be a breaking change, so I will plan it for 5.2.

[FBibonne]

I created the PR to indicate work in progress but it is still in draft because 5 tests still fail

[FBibonne]

PR #4544 is ready for review