Bug 4383 - Change JobOperator parameters from primitive types to Objects
Change JobOperator parameters from primitive types to Objects
Status: CLOSED FIXED
Product: jbatch
Classification: Unclassified
Component: source
1
All All
: P5 normal
: ---
Assigned To: cvignola
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-30 21:40 UTC by kmukher
Modified: 2013-01-16 15:27 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kmukher 2012-11-30 21:40:49 UTC
I'd like to propose changing most of the primitive types used in the JobOperator API to Objects to ensure type safety. This will help to avoid any type of problems where instance id's are confused with execution id's or vice versa (We've already hit this issue several times in the RI developement). I believe this conforms more closely with Java best practices and will allow the compiler to catch these types of simple errors in a batch application before they get into the runtime. I've included a refactored version of the JobOperator here with a few other minor corrections with exceptions, getRunningExecution(), and getStepExecution().

package javax.batch.operations;

import java.util.List;
import java.util.Set;
import java.util.Properties;
import javax.batch.operations.exception.JobExecutionIsRunningException;
import javax.batch.operations.exception.JobExecutionNotRunningException;
import javax.batch.operations.exception.JobInstanceAlreadyCompleteException;
import javax.batch.operations.exception.JobRestartException;
import javax.batch.operations.exception.JobStartException;
import javax.batch.operations.exception.NoSuchJobException;
import javax.batch.operations.exception.NoSuchJobExecutionException;
import javax.batch.operations.exception.NoSuchJobInstanceException;
import javax.batch.runtime.JobExecution;
import javax.batch.runtime.JobInstance;
import javax.batch.runtime.StepExecution;

public interface JobOperator {
	/**
	 * Returns a set of all job names known to the batch runtime.
	 * 
	 * @return a set of job names.
	 */
	Set<String> getJobNames();

	/**
	 * Returns number of instances of a job with a particular name (id).
	 * 
	 * @param jobName
	 *            specifies the name of the job.
	 * @return count of instances of the named job.
	 * @throws NoSuchJobException
	 */
	int getJobInstanceCount(String jobName) throws NoSuchJobException;

	/**
	 * Returns all instanceIds belonging to a job with a particular name.
	 * 
	 * @param jobName
	 *            identifies the job name.
	 * @param start
	 *            identifies the relative starting number to return from the
	 *            maximal list of job instances.
	 * @param count
	 *            identifies the number of instance ids to return from the
	 *            starting position of the maximal list of job instances.
	 * @return list of job instances
	 * @throws NoSuchJobException
	 */
	List<JobInstance> getJobInstances(String jobName, int start, int count)
			throws NoSuchJobException;

	/**
	 * Returns instanceIds for all running jobs across all instances of a job
	 * with a particular name.
	 * 
	 * @param jobName
	 *            identifies the job name.
	 * @return a Set of job instances
	 * @throws NoSuchJobException
	 */
	Set<JobInstance> getRunningInstanceIds(String jobName) throws NoSuchJobException;

	/**
	 * Returns all executionIds belonging to a particular job instance.
	 * 
	 * @param jobInstance
	 *            identifies the job instance
	 * @return List of Job Executions
	 * @throws NoSuchJobInstanceException
	 */
	List<JobExecution> getExecutions(JobInstance jobInstance) throws NoSuchJobInstanceException;

	/**
	 * Returns job parameters for specified execution. These are the key/value
	 * pairs specified when the instance was started or restarted.
	 * 
	 * @param jobExecution
	 *            identifies the execution.
	 * @return a Properties object containing the key/value job parameter pairs.
	 * @throws NoSuchJobExecutionException
	 */
	Properties getParameters(JobExecution jobExecution)
			throws NoSuchJobExecutionException;

	/**
	 * Creates a new job instance and starts the first execution of that
	 * instance.
	 * 
	 * @param job
	 *            specifies the Job XML describing the job.
	 * @param jobParameters
	 *            specifies the keyword/value pairs for property override and
	 *            substitution in Job XML.
	 * @return JobInstance of the new job execution.
	 * @throws JobStartException
	 */
	JobInstance start(String job, Properties jobParameters) throws JobStartException;

	
	/**
	 * Returns the currently running execution of the given jobInstance
	 * 
	 * @param jobInstance
	 * @return The currently running
	 */
	JobExecution getRunningExecution(JobInstance jobInstance)throws JobExecutionNotRunningException;
	
	/**
	 * Restarts a failed or stopped job instance.
	 * 
	 * @param instanceId
	 *            belonging to the instance to restart. The execution that
	 *            restarts is the most recent execution to run.
	 * @param jobParameters
	 *            specify replacement job parameters for the job restart. The
	 *            replacement add to and/or override the original job parameters
	 *            that were specified when the instance was originally started.
	 * @return new job execution
	 * @throws JobInstanceAlreadyCompleteException
	 * @throws NoSuchJobExecutionException
	 * @throws NoSuchJobException
	 * @throws JobRestartException
	 */
	JobExecution restart(JobInstance jobInstance, Properties jobParameters)
			throws JobInstanceAlreadyCompleteException,
			NoSuchJobInstanceException, NoSuchJobException,
			JobRestartException;

	/**
	 * Request a running execution stops. *
	 * 
	 * @param instanceId
	 *            specifies the instance to stop (the currently running
	 *            execution is stopped)
	 * @throws NoSuchJobExecutionException
	 * @throws JobExecutionNotRunningException
	 */
	void stop(JobInstance jobInstance) throws NoSuchJobInstanceException,
			JobExecutionNotRunningException;

	/**
	 * Set batch status to ABANDONED. The instance must not be running.
	 * 
	 * @param instanceId
	 *            specifies the instance to mark abandoned
	 * @throws NoSuchJobExecutionException
	 *             * @throws JobExecutionIsRunningException
	 */
	void abandon(long instanceId) throws NoSuchJobExecutionException,
			JobExecutionIsRunningException;

	/**
	 * Return the job instance for the specified job instance id
	 * 
	 * @param instanceId
	 *            specifies the requested job instance
	 * @return job instance
	 */
	JobInstance getJobInstance(long instanceId);

	/**
	 * Return all job executions belonging to the specified job instance
	 * 
	 * @param jobInstance
	 *            specifies the job instance
	 * @return list of job executions
	 */
	List<JobExecution> getJobExecutions(long instanceId);

	/**
	 * Return job execution for specified execution id
	 * 
	 * @param executionId
	 *            specifies the requested job execution
	 * @return job execution
	 */
	JobExecution getJobExecution(long executionId);

	/**
	 * Return step execution for specified execution
	 * 
	 * @param jobInstance
	 *            specifies the JobInstance
	 * @param jobExecution
	 *            specifies the JobExecution
	 * @param stepId
	 *            specifies the unique step id within a job
	 * @return step execution
	 */
	StepExecution getStepExecution(JobInstance jobInstance, JobExecution jobExecution, String stepId);

	/**
	 * Returns a list of all the executed steps in a job execution. 
	 * 
	 * @param jobExecution
	 * @return
	 */
	List<StepExecution> getJobSteps(JobExecution jobExecution);
}
Comment 1 kmukher 2012-11-30 21:54:39 UTC
Small correction to the getStepExecution() method. Removed the JobInstance parameter, since a JobExecution object is already associated with a JobInstance.

Also, please note that we need to change the last parameter from 'long stepExecutionId' to 'String stepId' and add a getStepId() method to StepExecution interface. This is because currently there is no way to map from a step id to a StepExecution. For example, if I had job with id=job1 with three steps, id=step1, id=step2, id=step3, and I wanted to get the end time for step2, there is no way to input that to the JobOperator.

/**
 * Return step execution for specified execution
 * 
 * @param jobExecution
 *            specifies the JobExecution
 * @param stepId
 *            specifies the unique step id within a job
 * @return step execution
 */
StepExecution getStepExecution(JobExecution jobExecution, String stepId);



public interface StepExecution<P> {

    
	/**
	 * Get the step id of the executed step.
	 * 
	 * @return
	 */
	public String getStepId();
Comment 2 geofjamg 2012-11-30 22:22:38 UTC
In that case, why not removing all the 'long' ids from JobOperator interface and also getId() method in JobInstance, JobExecution and StepExecution so that ids are not exposed to the api?
Comment 3 ScottKurz 2012-12-04 16:39:08 UTC
This might be a choice between performance and clarity to the developer, since for a persistent impl, we might typically go to the database to conjure up a full JobExecution/JobInstance object.

We wouldn't want the JobOperator client to have to new up the JobInstance/JobExecution himself I don't thik.

With this change, for code like: 

JobExecution exec =  jobOperator.restart(...)
exec.getStartTime(); 

This might then typically be called before the job has (re)started.   

So we end up with a 'null' (I doubt we'd want to specify a default value).

OTOH, the batch status is well-defined (STARTING).

I don't think that's a big deal....  and like Kaushik said it is a source of confusion to the developer...
Comment 4 cvignola 2012-12-20 16:15:13 UTC
The general concept raised by this bug will be factored into the spec.  You will have to look to the Final Proposed spec for details.