RESOLVED - issue BATCH-199: ExceptionHandler in SImpleStepConfiguration ignored by SimpleStepConfigurationFactory
http://opensource.atlassian.com/projects/spring/browse/BATCH-199 Moved ExecptionHandler to the step operations. It seemed to me that the responsibility for doing this should be encapsulated in the StepExecutor - also prevents a nasty package cycle. The only way to do this and keep everyone honest was to add an applyConfiguration() method to the StepExecutor interface. I am open to suggestion about how to do this more cleanly, but for now that is probably enough.
This commit is contained in:
@@ -25,8 +25,8 @@ import org.springframework.batch.repeat.ExitStatus;
|
||||
* Interface for processing a step. Implementations are free to process the step
|
||||
* and return when finished, or to schedule the step for processing
|
||||
* concurrently, or in the future. The status of the execution should be
|
||||
* trackable with the step execution context ({@see Step#getContext()}). The
|
||||
* configuration should be treated as immutable.<br/>
|
||||
* trackable with the step execution. The configuration should be treated as
|
||||
* immutable.<br/>
|
||||
*
|
||||
* Because step execution parameters and policies can vary from step to step, a
|
||||
* {@link StepExecutor} should be created by the caller using a
|
||||
@@ -39,17 +39,35 @@ import org.springframework.batch.repeat.ExitStatus;
|
||||
public interface StepExecutor {
|
||||
|
||||
/**
|
||||
* Process the step according to the given configuration.
|
||||
* Process the step according to the given configuration. Implementations
|
||||
* can be expected to modify their state when this method is called, to take
|
||||
* account of any policy information in the configuration. Thus it is not
|
||||
* safe to re-use an instance of {@link StepExecutor} to process multiple
|
||||
* concurrent executions.
|
||||
*
|
||||
* @param configuration the configuration to use when running the step.
|
||||
* Contains a recipe for the business logic of an individual processing
|
||||
* operation. Also used to determine policies for commit intervals and
|
||||
* exception handling, for instance.
|
||||
* @param stepExecution an entity representing the step to be executed
|
||||
* @throws StepInterruptedException if the step is interrupted externally
|
||||
* @throws BatchCriticalException if there is a problem that needs to be
|
||||
* signalled to the caller
|
||||
* @param configuration
|
||||
* the configuration to use when running the step. Contains a
|
||||
* recipe for the business logic of an individual processing
|
||||
* operation. Also used to determine policies for commit
|
||||
* intervals and exception handling, for instance.
|
||||
*
|
||||
* @param stepExecution
|
||||
* an entity representing the step to be executed
|
||||
* @throws StepInterruptedException
|
||||
* if the step is interrupted externally
|
||||
* @throws BatchCriticalException
|
||||
* if there is a problem that needs to be signalled to the
|
||||
* caller
|
||||
*/
|
||||
ExitStatus process(StepConfiguration configuration, StepExecution stepExecution) throws StepInterruptedException, BatchCriticalException;
|
||||
ExitStatus process(StepConfiguration configuration,
|
||||
StepExecution stepExecution) throws StepInterruptedException,
|
||||
BatchCriticalException;
|
||||
|
||||
/**
|
||||
* Apply the configuration by inspecting it to see if it has any relevant
|
||||
* policy information. Should be called before any calls to process.
|
||||
*
|
||||
* @param configuration
|
||||
*/
|
||||
void applyConfiguration(StepConfiguration configuration);
|
||||
}
|
||||
|
||||
@@ -20,8 +20,6 @@ import org.springframework.batch.core.executor.StepExecutor;
|
||||
import org.springframework.batch.core.executor.StepExecutorFactory;
|
||||
import org.springframework.batch.execution.step.simple.SimpleStepExecutor;
|
||||
import org.springframework.batch.repeat.RepeatOperations;
|
||||
import org.springframework.batch.repeat.policy.SimpleCompletionPolicy;
|
||||
import org.springframework.batch.repeat.support.RepeatTemplate;
|
||||
import org.springframework.beans.BeansException;
|
||||
import org.springframework.beans.factory.BeanFactory;
|
||||
import org.springframework.beans.factory.BeanFactoryAware;
|
||||
@@ -107,35 +105,9 @@ public class PrototypeBeanStepExecutorFactory implements StepExecutorFactory,
|
||||
* @see StepExecutorFactory#getExecutor(StepConfiguration)
|
||||
*/
|
||||
public StepExecutor getExecutor(StepConfiguration configuration) {
|
||||
|
||||
StepExecutor executor = getStepExecutor();
|
||||
|
||||
if (executor instanceof SimpleStepExecutor) {
|
||||
RepeatTemplate template = new RepeatTemplate();
|
||||
RepeatOperations chunkOperations = template;
|
||||
RepeatOperations stepOperations = null;
|
||||
if (configuration instanceof RepeatOperationsHolder) {
|
||||
RepeatOperationsHolder holder = (RepeatOperationsHolder) configuration;
|
||||
chunkOperations = holder.getChunkOperations();
|
||||
stepOperations = holder.getStepOperations();
|
||||
Assert
|
||||
.state(chunkOperations != null,
|
||||
"Chunk operations obtained from step configuration must be non-null.");
|
||||
} else if (configuration instanceof SimpleStepConfiguration) {
|
||||
template.setCompletionPolicy(new SimpleCompletionPolicy(
|
||||
((SimpleStepConfiguration) configuration)
|
||||
.getCommitInterval()));
|
||||
template.setExceptionHandler(((SimpleStepConfiguration)configuration).getExceptionHandler());
|
||||
}
|
||||
SimpleStepExecutor simpleExecutor = (SimpleStepExecutor) executor;
|
||||
simpleExecutor.setChunkOperations(chunkOperations);
|
||||
if (stepOperations!=null) {
|
||||
simpleExecutor.setStepOperations(stepOperations);
|
||||
}
|
||||
}
|
||||
|
||||
executor.applyConfiguration(configuration);
|
||||
return executor;
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -32,11 +32,15 @@ import org.springframework.batch.core.tasklet.Tasklet;
|
||||
import org.springframework.batch.execution.scope.SimpleStepContext;
|
||||
import org.springframework.batch.execution.scope.StepScope;
|
||||
import org.springframework.batch.execution.scope.StepSynchronizationManager;
|
||||
import org.springframework.batch.execution.step.RepeatOperationsHolder;
|
||||
import org.springframework.batch.execution.step.SimpleStepConfiguration;
|
||||
import org.springframework.batch.io.exception.BatchCriticalException;
|
||||
import org.springframework.batch.repeat.ExitStatus;
|
||||
import org.springframework.batch.repeat.RepeatCallback;
|
||||
import org.springframework.batch.repeat.RepeatContext;
|
||||
import org.springframework.batch.repeat.RepeatOperations;
|
||||
import org.springframework.batch.repeat.exception.handler.ExceptionHandler;
|
||||
import org.springframework.batch.repeat.policy.SimpleCompletionPolicy;
|
||||
import org.springframework.batch.repeat.support.RepeatTemplate;
|
||||
import org.springframework.batch.repeat.synch.BatchTransactionSynchronizationManager;
|
||||
import org.springframework.batch.restart.RestartData;
|
||||
@@ -138,7 +142,7 @@ public class SimpleStepExecutor implements StepExecutor {
|
||||
* is used to store the result. Various reporting information are also added
|
||||
* to the current context (the {@link RepeatContext} governing the step
|
||||
* execution, which would normally be available to the caller somehow
|
||||
* through the step's {@link JobExecutionContext}.
|
||||
* through the step's {@link JobExecutionContext}.<br/>
|
||||
*
|
||||
* @throws StepInterruptedException
|
||||
* if the step or a chunk is interrupted
|
||||
@@ -153,7 +157,7 @@ public class SimpleStepExecutor implements StepExecutor {
|
||||
final StepInstance step = stepExecution.getStep();
|
||||
boolean isRestart = step.getStepExecutionCount() > 0 ? true : false;
|
||||
Assert.notNull(step);
|
||||
|
||||
|
||||
final Tasklet module = configuration.getTasklet();
|
||||
|
||||
ExitStatus status = ExitStatus.FAILED;
|
||||
@@ -367,9 +371,14 @@ public class SimpleStepExecutor implements StepExecutor {
|
||||
return tasklet.execute();
|
||||
}
|
||||
|
||||
private RestartData getRestartData(Tasklet module) {
|
||||
if (module instanceof Restartable) {
|
||||
return ((Restartable) module).getRestartData();
|
||||
/**
|
||||
* @param tasklet
|
||||
* @return restart data from the {@link Tasklet} if it is
|
||||
* {@link Restartable}
|
||||
*/
|
||||
private RestartData getRestartData(Tasklet tasklet) {
|
||||
if (tasklet instanceof Restartable) {
|
||||
return ((Restartable) tasklet).getRestartData();
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
@@ -411,4 +420,57 @@ public class SimpleStepExecutor implements StepExecutor {
|
||||
ExitCodeExceptionClassifier exceptionClassifier) {
|
||||
this.exceptionClassifier = exceptionClassifier;
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply the configuration by inspecting it to see if it has any relevant
|
||||
* policy information.
|
||||
* <ul>
|
||||
* <li> If the configuration is a {@link RepeatOperationsHolder} then we use
|
||||
* the provided {@link RepeatOperations} instances for chunk and step. </li>
|
||||
* <li> If the configuration is a {@link SimpleStepConfiguration} then we
|
||||
* apply the commit interval at the chunk level and the exception handler at
|
||||
* the step level, provided the existing repeat operations are instances of
|
||||
* {@link RepeatTemplate}.</li>
|
||||
* </ul>
|
||||
*
|
||||
* @param configuration
|
||||
* a step configuration
|
||||
*/
|
||||
public void applyConfiguration(StepConfiguration configuration) {
|
||||
|
||||
if (configuration instanceof RepeatOperationsHolder) {
|
||||
|
||||
RepeatOperationsHolder holder = (RepeatOperationsHolder) configuration;
|
||||
RepeatOperations chunkOperations = holder.getChunkOperations();
|
||||
RepeatOperations stepOperations = holder.getStepOperations();
|
||||
Assert
|
||||
.state(chunkOperations != null,
|
||||
"Chunk operations obtained from step configuration must be non-null.");
|
||||
|
||||
|
||||
if (chunkOperations != null) {
|
||||
setChunkOperations(chunkOperations);
|
||||
}
|
||||
if (stepOperations != null) {
|
||||
setStepOperations(stepOperations);
|
||||
}
|
||||
|
||||
} else if (configuration instanceof SimpleStepConfiguration) {
|
||||
|
||||
SimpleStepConfiguration simpleConfiguation = (SimpleStepConfiguration) configuration;
|
||||
if (this.chunkOperations instanceof RepeatTemplate) {
|
||||
RepeatTemplate template = (RepeatTemplate) this.chunkOperations;
|
||||
template.setCompletionPolicy(new SimpleCompletionPolicy(
|
||||
simpleConfiguation.getCommitInterval()));
|
||||
}
|
||||
ExceptionHandler exceptionHandler = simpleConfiguation
|
||||
.getExceptionHandler();
|
||||
if (this.stepOperations instanceof RepeatTemplate && exceptionHandler!=null) {
|
||||
RepeatTemplate template = (RepeatTemplate) this.stepOperations;
|
||||
template.setExceptionHandler(exceptionHandler);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,12 +19,7 @@ import org.springframework.batch.core.configuration.StepConfiguration;
|
||||
import org.springframework.batch.core.executor.StepExecutor;
|
||||
import org.springframework.batch.core.executor.StepExecutorFactory;
|
||||
import org.springframework.batch.core.repository.JobRepository;
|
||||
import org.springframework.batch.execution.step.RepeatOperationsHolder;
|
||||
import org.springframework.batch.execution.step.SimpleStepConfiguration;
|
||||
import org.springframework.batch.repeat.RepeatOperations;
|
||||
import org.springframework.batch.repeat.exception.handler.ExceptionHandler;
|
||||
import org.springframework.batch.repeat.policy.SimpleCompletionPolicy;
|
||||
import org.springframework.batch.repeat.support.RepeatTemplate;
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
@@ -63,38 +58,8 @@ public class SimpleStepExecutorFactory implements StepExecutorFactory,
|
||||
|
||||
SimpleStepExecutor executor = new SimpleStepExecutor();
|
||||
executor.setRepository(jobRepository);
|
||||
executor.applyConfiguration(configuration);
|
||||
|
||||
RepeatTemplate template = new RepeatTemplate();
|
||||
RepeatOperations chunkOperations = template;
|
||||
RepeatOperations stepOperations = null;
|
||||
|
||||
if (configuration instanceof RepeatOperationsHolder) {
|
||||
|
||||
RepeatOperationsHolder holder = (RepeatOperationsHolder) configuration;
|
||||
chunkOperations = holder.getChunkOperations();
|
||||
stepOperations = holder.getStepOperations();
|
||||
Assert
|
||||
.state(chunkOperations != null,
|
||||
"Chunk operations obtained from step configuration must be non-null.");
|
||||
|
||||
} else {
|
||||
|
||||
SimpleStepConfiguration simpleConfiguration = (SimpleStepConfiguration) configuration;
|
||||
template.setCompletionPolicy(new SimpleCompletionPolicy(
|
||||
simpleConfiguration.getCommitInterval()));
|
||||
ExceptionHandler exceptionHandler = simpleConfiguration
|
||||
.getExceptionHandler();
|
||||
if (exceptionHandler != null) {
|
||||
template.setExceptionHandler(exceptionHandler);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
executor.setChunkOperations(chunkOperations);
|
||||
if (stepOperations!=null) {
|
||||
executor.setStepOperations(stepOperations);
|
||||
}
|
||||
|
||||
return executor;
|
||||
|
||||
}
|
||||
|
||||
@@ -20,17 +20,9 @@ import java.util.List;
|
||||
|
||||
import junit.framework.TestCase;
|
||||
|
||||
import org.springframework.batch.core.configuration.StepConfiguration;
|
||||
import org.springframework.batch.core.configuration.StepConfigurationSupport;
|
||||
import org.springframework.batch.core.domain.JobExecution;
|
||||
import org.springframework.batch.core.domain.JobInstance;
|
||||
import org.springframework.batch.core.domain.StepExecution;
|
||||
import org.springframework.batch.core.domain.StepInstance;
|
||||
import org.springframework.batch.core.executor.StepExecutor;
|
||||
import org.springframework.batch.core.executor.StepInterruptedException;
|
||||
import org.springframework.batch.execution.step.simple.SimpleStepExecutor;
|
||||
import org.springframework.batch.io.exception.BatchCriticalException;
|
||||
import org.springframework.batch.repeat.ExitStatus;
|
||||
import org.springframework.batch.repeat.RepeatOperations;
|
||||
import org.springframework.batch.repeat.support.RepeatTemplate;
|
||||
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
|
||||
@@ -95,11 +87,7 @@ public class PrototypeBeanStepExecutorFactoryTests extends TestCase {
|
||||
}
|
||||
|
||||
public void testSuccessfulStepExecutorWithSimpleConfigurationAndNotSimpleExecutor() throws Exception {
|
||||
StepExecutor executor = new StepExecutor() {
|
||||
public ExitStatus process(StepConfiguration configuration, StepExecution stepExecution) throws BatchCriticalException {
|
||||
return ExitStatus.FINISHED;
|
||||
}
|
||||
};
|
||||
StepExecutor executor = new SimpleStepExecutor();
|
||||
applicationContext.getBeanFactory().registerSingleton("foo", executor);
|
||||
factory.setStepExecutorName("foo");
|
||||
assertEquals(executor, factory.getExecutor(new SimpleStepConfiguration()));
|
||||
@@ -116,11 +104,6 @@ public class PrototypeBeanStepExecutorFactoryTests extends TestCase {
|
||||
public void testSuccessfulStepExecutorHolderStrategyWithStepOperations() throws Exception {
|
||||
final List list = new ArrayList();
|
||||
SimpleStepExecutor executor = new SimpleStepExecutor() {
|
||||
public ExitStatus process(StepConfiguration configuration,
|
||||
StepExecution stepExecution)
|
||||
throws StepInterruptedException, BatchCriticalException {
|
||||
return ExitStatus.FINISHED;
|
||||
}
|
||||
public void setChunkOperations(RepeatOperations chunkOperations) {
|
||||
list.add(chunkOperations);
|
||||
super.setChunkOperations(chunkOperations);
|
||||
@@ -134,14 +117,8 @@ public class PrototypeBeanStepExecutorFactoryTests extends TestCase {
|
||||
factory.setStepExecutorName("foo");
|
||||
RepeatTemplate chunkTemplate = new RepeatTemplate();
|
||||
RepeatTemplate stepTemplate = new RepeatTemplate();
|
||||
SimpleHolderStepConfiguration configuration = new SimpleHolderStepConfiguration(
|
||||
chunkTemplate, stepTemplate);
|
||||
StepExecutor product = factory.getExecutor(new SimpleHolderStepConfiguration(chunkTemplate, stepTemplate));
|
||||
SimpleStepExecutor product = (SimpleStepExecutor) factory.getExecutor(new SimpleHolderStepConfiguration(chunkTemplate, stepTemplate));
|
||||
assertEquals(executor, product);
|
||||
StepExecution stepExecution = new StepExecution(new StepInstance(
|
||||
new Long(11)), new JobExecution(new JobInstance(null),
|
||||
new Long(12)));
|
||||
executor.process(configuration, stepExecution);
|
||||
assertEquals(2, list.size());
|
||||
assertEquals(chunkTemplate, list.get(0));
|
||||
assertEquals(stepTemplate, list.get(1));
|
||||
|
||||
@@ -245,7 +245,7 @@ public class DefaultStepExecutorTests extends TestCase {
|
||||
|
||||
}
|
||||
|
||||
public void testExitCodeDefaultClassification() {
|
||||
public void testExitCodeDefaultClassification() throws Exception {
|
||||
|
||||
Tasklet tasklet = new Tasklet() {
|
||||
|
||||
|
||||
@@ -68,9 +68,11 @@ public class SimpleStepExecutorFactoryTests extends TestCase {
|
||||
new Long(11)), new JobExecution(new JobInstance(null),
|
||||
new Long(12)));
|
||||
try {
|
||||
executor.processChunk(configuration, stepExecution);
|
||||
executor.process(configuration, stepExecution);
|
||||
fail("Expected RuntimeException");
|
||||
} catch (RuntimeException e) {
|
||||
} catch (NullPointerException e) {
|
||||
throw e;
|
||||
}catch (RuntimeException e) {
|
||||
assertEquals("Oops", e.getMessage());
|
||||
}
|
||||
assertEquals(1, list.size());
|
||||
@@ -93,7 +95,7 @@ public class SimpleStepExecutorFactoryTests extends TestCase {
|
||||
new Long(11)), new JobExecution(new JobInstance(null),
|
||||
new Long(12)));
|
||||
try {
|
||||
executor.processChunk(configuration, stepExecution);
|
||||
executor.process(configuration, stepExecution);
|
||||
fail("Expected RuntimeException");
|
||||
} catch (NullPointerException e) {
|
||||
// expected
|
||||
|
||||
@@ -217,7 +217,6 @@ public class RepeatTemplate implements RepeatOperations {
|
||||
+ (i + 1) + " of "
|
||||
+ interceptors.length + ")", throwable);
|
||||
}
|
||||
|
||||
exceptionHandler
|
||||
.handleException(context, throwable);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user