BATCH-2711: Fix JobParameterBuilder not to overwrite new parameters with old ones

[peterminhk]

In a previous commit, when rerun a previously failed job with
new parameters they are replaced with previous ones.
It is somewhat critical because once failed, there is no way to run a job changing its parameter(s), unless change the status of the job manually from DB.
This problem has happened since commit
b2f8f7f.

So this commit fix the issue and, also keep the purpose of previous
commit providing public method to add JobParameters to the builder.

related JiRA ticket: https://jira.spring.io/browse/BATCH-2711

[pivotal-issuemaster]

@peterminhk Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@peterminhk Thank you for signing the Contributor License Agreement!

[Ankitg44]

Once this PR will merge, which version of Spring Batch core have these changes??

[peterminhk]

@Ankitg44
Since it looks critical I think it should be merged in 4.0.2 and should be released as soon as possible. But the people in Spring organization will make a decision even whether merge or not.

@mminella
Could you give the answer?

[Ankitg44]

@peterminhk
Thanks for quick response.
I do agree this is critical because even for last successful job execution of a Batch job it is replacing, new instance Job parameters with older job execution job parameters.

[fmbenhassine]

Hi,

This issue is planned to be fixed in 4.0.2 and 4.1.0.RELEASE.

Kr,
Mahmoud

[aritzbastida]

Please, before releasing this PR, take a look at my comments in this JIRA issue: https://jira.spring.io/browse/BATCH-2711

All in all, I think that the PR does not completely solve the problem, because it fails to implement some use cases. I argued it with full details in the issue above.

[fmbenhassine]

@aritzbastida ok thank you for all the details! I will take a look before merging the PR.

[aritzbastida]

Thanks! Keep me posted!

[fmbenhassine]

Hi,

As said in JIRA, the method addJobParametersIfAbsent is used in getNextJobParameters so it is supposed to be used in a "start next instance scenario" (equivalent to CommandLineJobRunner with -next option). This method is not correct because it says in the Javadoc: If the map previously contained a mapping for the key, the new value is ignored. while the new value should override the existing one to be consistent with CommandLineJobRunner which says in the javadoc: If the -next option is used the parameters on the command line (if any) are appended to those retrieved from the incrementer, overriding any with the same key.

For this reason, I'm closing this PR.

Thank you @peterminhk for your contribution anyway!

Br,
Mahmoud