JobOperator creates its own MapJobRegistry ignoring explicitly defined JobRegistry Spring bean in application

[andrianov17]

Bug description
We use dynamically defined Spring Batch jobs, i.e., jobs defined dynamically via JobBuilder with subsequent jobRegistry.register(Job job), not as Job Spring beans which are loaded upon application startup.

After upgrade to Spring Batch 6, we define JobRegistry as in:

    @Bean
    JobRegistry springBatchJobRegistry() {
        return new MapJobRegistry();
    }

to be able to register jobs dynamically because JobRegistry Spring bean is not available from Spring Batch 6 anymore (which is OK).

The problem comes when we try to stop job via jobOperator.stop().

Because JobOperator now has only its own MapJobRegistry created via org.springframework.batch.core.launch.support.JobOperatorFactoryBean#afterPropertiesSet() or via org.springframework.batch.core.configuration.support.DefaultBatchConfiguration#getJobRegistry, the registry has no dynamically job defined, hence JobOperator doesn't call stoppableStep.stop().

Solution could be to check whether a JobRegistry Spring bean is defined before creating MapJobRegistry and use it accordingly.

Currently, to solve the problem, we extended JobOperator, define our custom JobOperator Spring bean and explicitly pass our MapJobRegistry to it.

Environment
Spring Batch 6.0.1, SQL Server 2022

Steps to reproduce

1. Defined JobRegistry Spring bean in the app
2. Create a job using JobBuilder and the JobRegistry above
3. Start the job
4. Stop the job using jobOperator bean (available from Spring Batch configuration) - stoppableStep.stop() is not called.

Expected behavior
stoppableStep.stop() is called as part of jobOperator.stop()

[quaff]

I think it's fixed by #5200.

[andrianov17]

I agree - looks like the fix you mentioned should be enough.

Thank you!

[fmbenhassine]

This change is related to #4971, which states:

The job operator can be initialised with an internal job registry and customized with a user-defined one if needed.

However, this does not seem to be the case. So this is a bug in Spring Batch.

@quaff Thank you for the PR, but the way to fix that is by looking for a registry from the context before creating a default one (similar to what we do for the datasource and the transaction manager in the JDBC config). What #5200 suggests is a breaking change and is not consistent with how other optional components are customized (why would we use an ObjectProvider for the registry but not the for task executor or the job parameters converter?). I will add a note about this in the PR.

[chgr0s]

I believe that loading the JobRegistry as a bean from the ApplicationContext is only a temporary workaround. In my opinion, the real issue is that the JobOperator (SimpleJobOperator) still uses the JobRegistry internally in some places, even though the JobOperator API suggests that it should be able to work with any job instance (since it accepts objects directly instead of, for example, job names).

This creates an indirect dependency along the lines of: "You may only pass jobs into this interface if they have already been registered in the background via the JobRegistry."

This also breaks the migration example for @EnableBatchProcessing(modular = true). When jobs are registered that way, they cannot be stopped properly. Just like dynamically generated jobs.

I think that by Spring Batch 6.2, when the old APIs are removed, the dependency from JobOperator to JobRegistry should also be eliminated. That would make the JobRegistry completely optional, since the JobOperator would then operate solely on job objects.

Right now, however, there is a clear inconsistency: @EnableBatchProcessing uses a bean named jobRegistry from the context, whereas DefaultBatchConfiguration does not.

EDIT: If the JobOperator were truly independent of the JobRegistry, it would also make #5197 obsolete. In that case, it would not matter for job execution whether the jobs come from a JobRegistry or are created on the fly. This would also mean that the MapJobRegistry or any other component (such as the deprecated JobRegistrySmartInitializingSingleton) would not need to know that the job originates from a different ApplicationContext.

[quaff]

the real issue is that the JobOperator (SimpleJobOperator) still uses the JobRegistry internally in some places, even though the JobOperator API suggests that it should be able to work with any job instance (since it accepts objects directly instead of, for example, job names).

How to eliminate such usage?

spring-batch/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java

Line 352 in 2e9755d

Job job = jobRegistry.getJob(jobExecution.getJobInstance().getJobName());

[fmbenhassine]

@chgr0s The job operator is not designed to be independent from the job registry (see SimpleJobOperator#afterPropertiesSet). There are some operations that require job executions as input and not the job itself (like stop or abandon) , so we need a way to get the job reference from a given execution. That's where the registry is required, as shown by @quaff .

What has changed in v6 is that, for configuration simplification purposes, if no registry is defined as a bean, the default would be to create an internal one in the operator and pre-populate it with the jobs defined in the context.

The issue here is that when a user defines a custom registry as a bean, that custom registry is not used by the operator (which is a bug in Spring Batch, and which will be fixed with #5200).