Conditionnel Flow

[elfelli]

Bug description
In Spring Batch, when defining a Flow with multiple transitions based on a JobExecutionDecider, the order of .from(decider).on(...) declarations is impacting the flow execution unexpectedly.

Environment

Spring Batch version: 5.0.3
Java version: 17
Database (if any): postgresql
Any other relevant framework/tool versions

Steps to reproduce

Configure a Job with a Flow as defined in the provided example.
Use a JobExecutionDecider to determine the flow execution based on a job parameter.
Notice that the execution path depends incorrectly on the order of the .from(routeDecider).on(...) method calls.

Expected behavior
The flow execution decision should solely depend on the output of the JobExecutionDecider and not on the order of the .from().on() method calls in the flow definition. Each condition ("node", "can", "both") should correctly lead to its corresponding flow regardless of the order these conditions are defined in the flow builder except the * case

Minimal Complete Reproducible example
A simple setup can be provided that includes:

public class RouteDecider implements JobExecutionDecider {

    @Override
    public FlowExecutionStatus decide(JobExecution jobExecution, StepExecution stepExecution) {
        String route = jobExecution.getJobParameters().getString("route").trim(); 
        log.info("Route decision : {}",route);
        return new FlowExecutionStatus(route);
    }

}

My flow is configured like bellow ;

    @Bean
    public Flow testFlow(Step archiveCanStep, Step archiveNodeStep, Step archiveAllStep, Step next, Step expStep, Step canStep, RouteDecider routeDecider) {
        return new FlowBuilder<SimpleFlow>("testFlow")
                .start(routeDecider).on("node").to(expStep).next(nodeStep).next(archiveNodeStep)
                .from(routeDecider).on("can").to(canStep).next(archiveCanStep)
                .from(routeDecider).on("both").to(expStep).next(nodeStep).next(canStep).next(archiveBothStep)
                .from(routeDecider).on("*").end()
                .end();
    }

Case of "node" -> OK
Case of "can" -> OK
Case of "both" -> execute always (node) (expStep).next(nodeStep).next(archiveNodeStep)
Case "*" -> OK

if i change order definition like this

    @Bean
    public Flow testFlow(Step archiveCanStep, Step archiveNoeStep, Step archiveAllStep, Step next, Step expStep, Step canStep, RouteDecider routeDecider) {
        return new FlowBuilder<SimpleFlow>("testFlow")
                .start(routeDecider).on("both").to(expStep).next(nodeStep).next(canStep).next(archiveBothStep)
                .from(routeDecider).on("can").to(canStep).next(archiveCanStep)
                .from(routeDecider).on("node").to(expStep).next(nodeStep).next(archiveNodeStep)
                .from(routeDecider).on("*").end()
                .end();
    }

Case of "node" -> KO execute always (both) .to(expStep).next(nodeStep).next(canalisationStep).next(archiveBothStep)
Case of "can" -> OK
Case of "both" -> OK
Case "*" -> OK

So I tried something else to bypass this issue by defining single flow for each case like this now so I can simplify the decider flow ...

       @Bean
    public Flow routeDeciderFlow(Flow NodeFlow, Flow canFlow, Flow AllFlow,routeDecider routeDecider) {
        return new FlowBuilder<SimpleFlow>(FLOW_JOB_BERG_DECIDER)
                .start(routeDecider).on("can").to(canFlow)
                .from(routeDecider).on("node").to(NodeFlow)
                .from(routeDecider).on("*").to(AllFlow)
                .end();
    }

    @Bean
    public Flow NodeFlow(Step nodeStep, Step expStep, Step archiveNoeStep) {
        return new FlowBuilder<SimpleFlow>(FLOW_JOB_BERG_DECIDER)
                .start(expStep)
                .next(nodeStep)
                .next(archiveNoeStep)
                .end();
    }

    @Bean
    public Flow canFlow(Step canStep, Step archiveCanStep) {
        return new FlowBuilder<SimpleFlow>(FLOW_JOB_BERG_DECIDER)
                .start(canStep)
                .next(archiveCanStep)
                .end();
    }

    @Bean
    public Flow AllFlow(Step archiveAllStep, Step nodeStep, Step expStep, Step canStep) {
        return new FlowBuilder<SimpleFlow>(FLOW_JOB_BERG_DECIDER)
                .start(expStep)
                .next(nodeStep)
                .next(canStep)
                .next(archiveAllStep)
                .end();
    }

[rojyates]

Sounds like an issue I encountered using 5.1.2 when adding a decider to a Flow - unless I'm misunderstanding how deciders should work.
I have a decider that checks for the presence of a file. It has two possible outcomes defined as constants:

• NO_FILE
• FILE_EXISTS

The outcome of the Job when the file is present unexpectedly depends on the order in which I specify the .on conditions in this Flow:

I originally wrote the check with the NO_FILE check last:
Option 1. 'fail condition checked last':

               .next(fileExistsDecider)
                .on(FILE_EXISTS).to(continueFlow)
                .on(NO_FILE).fail()
                .build();

When this is executed with no file (result NO_FILE), it behaves as expected and the job fails.
When the file is present, however, the job continues to execute as expected with all steps finishing as 'COMPLETED', but the overall Job finishes with 'FAILED'.

Reversing the order of these .on conditions, changes the behaviour of the Job.

Option 2. 'success condition checked last':

               .next(fileExistsDecider)
                .on(NO_FILE).fail()
                .on(FILE_EXISTS).to(continueFlow)
                .build();

Now we have the fail condition checked first, and the success condition last.
When this is executed with no file (result NO_FILE), it behaves as expected and the job fails.
When the file is present, the job continues to execute as expected with all steps finishing as 'COMPLETED', and the overall job finishes with 'COMPLETED', as expected.

I would expect the order of these checks not to impact the final result of the job.
Is this a bug, or am I missing something?

[fmbenhassine]

The order of flow definition should NOT matter. If it's the case, then this is a bug in Spring Batch.

Thank you for reporting this! Can you please provide a failing test or package a minimal sample that we can use to reproduce the issue? I will try to include the fix in the upcoming 5.2.0 release planned for November, otherwise it will be in a subsequent patch release. Many thanks upfront.

[rojyates]

Thanks @fmbenhassine. I'll put together a minimized codebase & test to illustrate.

[rojyates]

Hi @fmbenhassine, I've put together a set of tests with various permutations, including one that fails (test 3).
I've attached a zip with details including a README.md
github-decider-issue.zip
In a nutshell, I think I was missing a .from, but test #3 in the zip still surprises me with its result.
Let me know if you need anything else?