-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Description
Expected Behavior
StepExecution should expose a single, obvious way to access its enclosing JobExecution and related data (ID, parameters, etc): via getJobExecution() and then standard getters on JobExecution:
stepExecution.getJobExecution().getId();
stepExecution.getJobExecution().getJobParameters();The following convenience methods on StepExecution should be deprecated and scheduled for removal in the next major version (7.x):
// StepExecution
public long getJobExecutionId();
public JobParameters getJobParameters();This would make the public API more consistent and encourage users to always navigate the aggregate in a single, canonical way.
Current Behavior
StepExecution currently exposes the following methods:
/**
* Accessor for the job execution ID.
* @return the {@code jobExecutionId}.
*/
// TODO What is the added value of that?
public long getJobExecutionId() {
return this.jobExecution.getId();
}
/**
* Convenience method to get the current job parameters.
* @return the {@link JobParameters} from the enclosing job or empty if that is
* {@code null}.
*/
// TODO What is the added value of that?
public JobParameters getJobParameters() {
return this.jobExecution.getJobParameters();
}These methods are very thin wrappers around jobExecution, and the code itself already contains TODOs questioning their added value.
At the same time, many parts of the framework already access the same information via the JobExecution/JobInstance aggregate directly, for example:
// StepExecutionUtils.execute(..)
StepExecutionEvent stepExecutionEvent = new StepExecutionEvent(
stepExecution.getStepName(),
stepExecution.getJobExecution().getJobInstance().getJobName(),
stepExecution.getId(),
stepExecution.getJobExecution().getId()
);
// MessageChannelPartitionHandler#doHandle(..)
long jobExecutionId = managerStepExecution.getJobExecution().getId();New code that needs the job execution id or job parameters can (and already does) use:
stepExecution.getJobExecution().getId();
stepExecution.getJobExecution().getJobParameters();So the convenience methods do not enable anything that cannot already be expressed in a straightforward way with the existing model API.
Context
- Version: I’m looking at the 6.0.x line.
- Use case: While working in the
integration/chunkarea (for exampleChunkTaskExecutorItemWriterand related code), I noticed that we now have several ways of reaching the same data onJobExecutionandJobInstance:stepExecution.getJobExecutionId()stepExecution.getJobExecution().getId()stepExecution.getJobExecution().getJobInstance().getId()
- Problem: Having multiple public entry points for the same concept (especially with TODO comments that already question their value) makes the API surface larger than necessary and invites inconsistent usage patterns across the codebase and user applications.
- Proposal:
- Deprecate on
StepExecution:/** * @deprecated use {@link #getJobExecution().getId()} instead */ @Deprecated(since = "6.0", forRemoval = true) public long getJobExecutionId() { ... } /** * @deprecated use {@link #getJobExecution().getJobParameters()} instead */ @Deprecated(since = "6.0", forRemoval = true) public JobParameters getJobParameters() { ... }
- Update framework code to consistently use
getJobExecution().getId()andgetJobExecution().getJobParameters()where needed. - Remove both methods in 7.x as part of the next major cleanup step.
- Deprecate on
This would keep the model API focused and consistent, while still providing a proper deprecation window for any external users who might currently rely on these convenience methods.
This request is related to the refactoring in 90d8959, which introduced the immutable domain model entities.