Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate Job/Step builder factories #4188

Closed
fmbenhassine opened this issue Sep 7, 2022 · 0 comments
Closed

Deprecate Job/Step builder factories #4188

fmbenhassine opened this issue Sep 7, 2022 · 0 comments

Comments

@fmbenhassine
Copy link
Contributor

fmbenhassine commented Sep 7, 2022

JobBuilderFactory was reported to be of no real added value compared to JobBuilder. In fact, it only sets a single property on the builder it creates, which is the job repository. While this is not a problem in itself, I think there are a few issues with this factory:

Implicit configuration

This factory (when used with @EnableBatchProcessing, which is the most common use case) hides the fact that a job repository is created and set on the builder, unless users notice that by reading the docs:

@EnableBatchProcessing
public class MyJobConfig {

    @Autowired
    private JobBuilderFactory jobBuilderFactory;

    @Bean
    public Job job(Step step) {
        return this.jobBuilderFactory.get("myJob")
                .start(step)
                .build();
    }

}

In my experience, this kind of snippets was misleading to many users (especially newcomers) as it implicitly sets a job repository (that was also created implicitly behind the scene) on the job builder. Explicit is better than implicit, so here is a more explicit version (without the JobBuilderFactory):

@EnableBatchProcessing
public class MyJobConfig {

    @Bean
    public Job job(JobRepository jobRepository, Step step) {
        return new JobBuilder("myJob")
                .repository(jobRepository)
                .start(step)
                .build();
    }

}

I personally don't see any added value of the former sample over the latter.

Inconvenient configuration

It is not uncommon to have multiple job repositories in the same application (see references). In this case, the job repository that should be injected in the JobBuilderFactory bean created by @EnableBatchProcessing should be configured by a custom BatchConfigurer. This means, as a developer, I have to write code to customize the behaviour of an annotation. This, to me, and according to community feedback, is incorrect. If I go the declarative way with annotations, I am not expecting to write code to customize the behaviour of that annotation. I should be able to customize things with attributes, nested annotations, etc but not by writing code (ie writing a custom configurer).

As a side note, when multiple jobs are defined in the same configuration class (which I guess is the case for which this factory was created), there are several options to choose from like 1) autowiring the JobRepository as an instance variable and set on each job, or 2) creating a method that returns a JobBuilder and use it for jobs configuration, etc.


The same is true for StepBuilderFactory (after resolving #4130) . So this utility class should be deprecated as well.


References:


Related issues:

@fmbenhassine fmbenhassine added this to the 5.0.0-M6 milestone Sep 7, 2022
@fmbenhassine fmbenhassine self-assigned this Sep 7, 2022
@fmbenhassine fmbenhassine changed the title Deprecate JobBuilderFactory Deprecate Job/Step builder factories Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant