IN PROGRESS - issue BATCH-194: Incorrect exception handling when using Hibernate

http://opensource.atlassian.com/projects/spring/browse/BATCH-194

Implemented a pretty good workaround in HibernateCreditWriter, which can probably be tightened up and made into a base class HibernateItemWriter.  Also had to make HibernateCursorInputSource Skippable (and it isn't yet Restartable so it's kind of broken - work in progress).  Plus the samples were configured to use SimpleStepExecutor which doesn't skip by default (makes me wonder if the restart sample actually does anything sensible).
This commit is contained in:
dsyer
2007-11-22 13:51:53 +00:00
parent d7744aafb8
commit 3915aeb1e9
14 changed files with 255 additions and 140 deletions

View File

@@ -96,7 +96,7 @@ public class ItemProviderProcessTasklet implements Tasklet, Recoverable, Skippab
* item being processed. Needed to provide recoverable behavior if
* {@link RetryOperations} are not provided.
*/
private static final String ITEM_KEY = ItemProviderProcessTasklet.class + ".ITEM";
private static final String ITEM_KEY = ItemProviderProcessTasklet.class.getName() + ".ITEM";
private RetryPolicy retryPolicy = null;

View File

@@ -15,6 +15,8 @@
*/
package org.springframework.batch.io.cursor;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import org.hibernate.ScrollableResults;
@@ -22,6 +24,7 @@ import org.hibernate.Session;
import org.hibernate.SessionFactory;
import org.hibernate.StatelessSession;
import org.springframework.batch.io.InputSource;
import org.springframework.batch.io.Skippable;
import org.springframework.batch.item.ResourceLifecycle;
import org.springframework.batch.repeat.synch.BatchTransactionSynchronizationManager;
import org.springframework.batch.restart.GenericRestartData;
@@ -38,47 +41,66 @@ import org.springframework.util.ClassUtils;
* {@link InputSource} for reading database records built on top of Hibernate.
*
* It executes the HQL {@link #queryString} when initialized and iterates over
* the result set as {@link #read()} method is called, returning an object corresponding
* to current row.
* the result set as {@link #read()} method is called, returning an object
* corresponding to current row.
*
* Input source can be configured to use either {@link StatelessSession} sufficient
* for simple mappings without the need to cascade to associated objects or standard hibernate
* {@link Session} for more advanced mappings or when caching is desired.
* Input source can be configured to use either {@link StatelessSession}
* sufficient for simple mappings without the need to cascade to associated
* objects or standard hibernate {@link Session} for more advanced mappings or
* when caching is desired.
*
* When stateful session is used it will be cleared after successful commit *without* being flushed
* (no inserts or updates are expected).
* When stateful session is used it will be cleared after successful commit
* without being flushed (no inserts or updates are expected).
*
* @author Robert Kasanicky
*/
public class HibernateCursorInputSource implements InputSource, Restartable, InitializingBean, DisposableBean,
ResourceLifecycle {
private static final String RESTART_DATA_ROW_NUMBER_KEY = ClassUtils.getShortName(HibernateCursorInputSource.class)+".rowNumber";
public class HibernateCursorInputSource implements InputSource, Restartable,
Skippable, InitializingBean, DisposableBean, ResourceLifecycle {
private static final String RESTART_DATA_ROW_NUMBER_KEY = ClassUtils
.getShortName(HibernateCursorInputSource.class)
+ ".rowNumber";
private SessionFactory sessionFactory;
private StatelessSession statelessSession;
private Session statefulSession;
private ScrollableResults cursor;
private String queryString;
private boolean useStatelessSession = true;
private int lastCommitRowNumber = 0;
private final List skippedRows = new ArrayList();
private int skipCount = 0;
/* Current count of processed records. */
private int currentProcessedRow = 0;
private boolean initialized = false;
private TransactionSynchronization synchronization = new HibernateInputSourceTransactionSynchronization();
public Object read() {
if (!initialized) {
open();
}
if (cursor.next()) {
currentProcessedRow++;
if (!skippedRows.isEmpty()) {
// while is necessary to handle successive skips.
while (skippedRows.contains(new Integer(currentProcessedRow))) {
if (!cursor.next()) {
return null;
}
currentProcessedRow++;
}
}
Object data = cursor.get(0);
return data;
}
@@ -91,6 +113,9 @@ public class HibernateCursorInputSource implements InputSource, Restartable, Ini
public void close() {
initialized = false;
cursor.close();
currentProcessedRow = 0;
skippedRows.clear();
skipCount = 0;
if (useStatelessSession) {
statelessSession.close();
} else {
@@ -109,13 +134,15 @@ public class HibernateCursorInputSource implements InputSource, Restartable, Ini
statefulSession = sessionFactory.openSession();
cursor = statefulSession.createQuery(queryString).scroll();
}
BatchTransactionSynchronizationManager.registerSynchronization(synchronization );
BatchTransactionSynchronizationManager
.registerSynchronization(synchronization);
initialized = true;
}
/**
* @param sessionFactory hibernate session factory
* @param sessionFactory
* hibernate session factory
*/
public void setSessionFactory(SessionFactory sessionFactory) {
this.sessionFactory = sessionFactory;
@@ -131,7 +158,8 @@ public class HibernateCursorInputSource implements InputSource, Restartable, Ini
}
/**
* @param queryString HQL query string
* @param queryString
* HQL query string
*/
public void setQueryString(String queryString) {
this.queryString = queryString;
@@ -139,9 +167,10 @@ public class HibernateCursorInputSource implements InputSource, Restartable, Ini
/**
* Can be set only in uninitialized state.
* @param useStatelessSession
* <code>true</code> to use {@link StatelessSession}
* <code>false</code> to use standard hibernate {@link Session}
*
* @param useStatelessSession
* <code>true</code> to use {@link StatelessSession}
* <code>false</code> to use standard hibernate {@link Session}
*/
public void setUseStatelessSession(boolean useStatelessSession) {
Assert.state(!initialized);
@@ -153,8 +182,9 @@ public class HibernateCursorInputSource implements InputSource, Restartable, Ini
*/
public RestartData getRestartData() {
Properties props = new Properties();
props.setProperty(RESTART_DATA_ROW_NUMBER_KEY, String.valueOf(cursor.getRowNumber()));
props.setProperty(RESTART_DATA_ROW_NUMBER_KEY, String.valueOf(cursor
.getRowNumber()));
return new GenericRestartData(props);
}
@@ -162,28 +192,49 @@ public class HibernateCursorInputSource implements InputSource, Restartable, Ini
* Sets the cursor to the received row number.
*/
public void restoreFrom(RestartData data) {
Assert.state(!initialized,
"Cannot restore when already intialized. Call close() first before restore()");
Assert
.state(!initialized,
"Cannot restore when already intialized. Call close() first before restore()");
Properties props = data.getProperties();
if (props.getProperty(RESTART_DATA_ROW_NUMBER_KEY) == null) {
return;
}
int rowNumber = Integer.parseInt(props.getProperty(RESTART_DATA_ROW_NUMBER_KEY));
int rowNumber = Integer.parseInt(props
.getProperty(RESTART_DATA_ROW_NUMBER_KEY));
open();
cursor.setRowNumber(rowNumber);
}
/**
* Skip the current row. If the transaction is rolled back, this row will
* not be represented when read() is called. For example, if you read in row
* 2, find the data to be bad, and call skip(), then continue processing and
* find
*/
public void skip() {
skippedRows.add(new Integer(currentProcessedRow));
skipCount++;
}
/**
* Encapsulates transaction events handling.
*/
private class HibernateInputSourceTransactionSynchronization extends TransactionSynchronizationAdapter {
private class HibernateInputSourceTransactionSynchronization extends
TransactionSynchronizationAdapter {
public void afterCompletion(int status) {
if (status == TransactionSynchronization.STATUS_ROLLED_BACK) {
cursor.setRowNumber(lastCommitRowNumber);
currentProcessedRow = lastCommitRowNumber;
if (lastCommitRowNumber == 0) {
cursor.beforeFirst();
} else {
// Set the cursor so that next time it is advanced it will
// come back to the committed row.
cursor.setRowNumber(lastCommitRowNumber - 1);
}
} else if (status == TransactionSynchronization.STATUS_COMMITTED) {
lastCommitRowNumber = cursor.getRowNumber();
lastCommitRowNumber = currentProcessedRow;
if (!useStatelessSession) {
statefulSession.clear();
}

View File

@@ -129,7 +129,7 @@ public class RethrowOnThresholdExceptionHandler implements ExceptionHandler {
}
private RepeatContextCounter getCounter(RepeatContext context, Object key) {
String attribute = RethrowOnThresholdExceptionHandler.class + "."
String attribute = RethrowOnThresholdExceptionHandler.class.getName() + "."
+ key.toString();
// Creates a new counter and stores it in the correct context:
return new RepeatContextCounter(context, attribute, useParent);

View File

@@ -35,7 +35,7 @@ public abstract class CountingCompletionPolicy extends DefaultResultCompletionPo
/**
* Session key for global counter.
*/
public static final String COUNT = CountingCompletionPolicy.class + ".COUNT";
public static final String COUNT = CountingCompletionPolicy.class.getName() + ".COUNT";
private boolean useParent = false;

View File

@@ -64,7 +64,7 @@ public class BatchTransactionSynchronizationManager {
/**
* The key in the context attributes for the list of synchronizations.
*/
private static final String SYNCHS_ATTR_KEY = BatchTransactionSynchronizationManager.class + ".SYNCHRONIZATIONS";
private static final String SYNCHS_ATTR_KEY = BatchTransactionSynchronizationManager.class.getName() + ".SYNCHRONIZATIONS";
/**
* Static method to register synchronizations. A TransactionSyncrhonization

View File

@@ -42,7 +42,7 @@ public class ItemProviderRetryCallback implements RetryCallback {
private final static Log logger = LogFactory.getLog(ItemProviderRetryCallback.class);
public static final String ITEM = ItemProviderRetryCallback.class + ".ITEM";
public static final String ITEM = ItemProviderRetryCallback.class.getName() + ".ITEM";
private ItemProvider provider;

View File

@@ -44,7 +44,7 @@ public class ItemProviderRetryPolicy extends AbstractStatefulRetryPolicy {
protected Log logger = LogFactory.getLog(getClass());
public static final String EXHAUSTED = ItemProviderRetryPolicy.class + ".EXHAUSTED";
public static final String EXHAUSTED = ItemProviderRetryPolicy.class.getName() + ".EXHAUSTED";
private RetryPolicy delegate;

View File

@@ -59,7 +59,7 @@ public class RethrowOnThresholdExceptionHandlerTests extends TestCase {
handler.setThresholds(Collections.singletonMap("RuntimeException", new Integer(1)));
// No exception...
handler.handleException(context, new RuntimeException("Foo"));
RepeatContextCounter counter = new RepeatContextCounter(context, RethrowOnThresholdExceptionHandler.class + ".RuntimeException");
RepeatContextCounter counter = new RepeatContextCounter(context, RethrowOnThresholdExceptionHandler.class.getName() + ".RuntimeException");
assertNotNull(counter);
assertEquals(1, counter.getCount());
}

View File

@@ -33,12 +33,14 @@ import org.springframework.orm.hibernate3.support.HibernateDaoSupport;
public class HibernateCreditWriter extends HibernateDaoSupport implements
CustomerCreditWriter, RepeatInterceptor {
private boolean failOnFlush = false;
private boolean first = true;
private int failOnFlush = -1;
private List errors = new ArrayList();
private Set processed = new HashSet();
private Set failed = new HashSet();
// TODO: these need to be ThreadLocal (or a pure framework concern).
private RepeatContext context;
private Set processed = new HashSet();
/**
* Public accessor for the errors property.
*
@@ -54,16 +56,15 @@ public class HibernateCreditWriter extends HibernateDaoSupport implements
* @see org.springframework.batch.sample.dao.CustomerCreditWriter#write(org.springframework.batch.sample.domain.CustomerCredit)
*/
public void writeCredit(CustomerCredit customerCredit) {
if (!failOnFlush || !first) {
getHibernateTemplate().update(customerCredit);
} else {
if (customerCredit.getId() == failOnFlush) {
// try to insert one with a duplicate ID
CustomerCredit newCredit = new CustomerCredit();
newCredit.setId(customerCredit.getId());
newCredit.setName(customerCredit.getName());
newCredit.setCredit(customerCredit.getCredit());
getHibernateTemplate().save(newCredit);
first = false; // fail on the first record only
} else {
getHibernateTemplate().update(customerCredit);
}
}
@@ -75,23 +76,31 @@ public class HibernateCreditWriter extends HibernateDaoSupport implements
public void write(Object output) {
processed.add(output);
writeCredit((CustomerCredit) output);
if (failed.contains(output)) {
// Force early completion to commit aggressively if we encounter a
// failed item (from a failed chunk but we don't know which one was
// the problem).
context.setCompleteOnly();
// Flush now, so that if there is a failure this record will be
// skipped.
getHibernateTemplate().flush();
}
}
/**
* Public setter for the {@link boolean} property.
* Public setter for the failOnFlush property.
*
* @param failOnFlush
* true if you want to fail on flush (for testing)
*/
public void setFailOnFlush(boolean failOnFlush) {
public void setFailOnFlush(int failOnFlush) {
this.failOnFlush = failOnFlush;
}
public void after(RepeatContext context, ExitStatus result) {
//
public void before(RepeatContext context) {
}
public void before(RepeatContext context) {
public void after(RepeatContext context, ExitStatus result) {
}
/**
@@ -117,9 +126,9 @@ public class HibernateCreditWriter extends HibernateDaoSupport implements
}
public void open(RepeatContext context) {
this.context = context;
errors.clear();
processed.clear();
System.err.println(failed);
}
}

View File

@@ -13,8 +13,8 @@ public class SqlNflPlayerSummaryDao extends JdbcDaoSupport implements ItemWriter
public void write(Object output) {
Assert.isInstanceOf(NflPlayerSummary.class, output, SqlNflPlayerSummaryDao.class + " only " +
"supports outputing " + NflPlayerSummary.class + " instances.");
Assert.isInstanceOf(NflPlayerSummary.class, output, SqlNflPlayerSummaryDao.class.getName() + " only " +
"supports outputing " + NflPlayerSummary.class.getName() + " instances.");
NflPlayerSummary summary = (NflPlayerSummary)output;

View File

@@ -1,9 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:aop="http://www.springframework.org/schema/aop"
xmlns:tx="http://www.springframework.org/schema/tx"
xmlns:p="http://www.springframework.org/schema/p"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:aop="http://www.springframework.org/schema/aop"
xmlns:tx="http://www.springframework.org/schema/tx"
xmlns:p="http://www.springframework.org/schema/p"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="
http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd
http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop-2.0.xsd
@@ -16,75 +16,86 @@
<bean id="hibernateJob" parent="simpleJob">
<property name="steps">
<bean id="step1" class="org.springframework.batch.execution.step.RepeatOperationsStepConfiguration">
<property name="tasklet">
<bean
class="org.springframework.batch.execution.tasklet.RestartableItemProviderTasklet">
<property name="itemProvider">
<bean
class="org.springframework.batch.item.provider.InputSourceItemProvider">
<property name="inputSource"
ref="hibernateInputSource" />
</bean>
</property>
<property name="itemProcessor">
<bean
class="org.springframework.batch.sample.item.processor.CustomerCreditIncreaseProcessor">
<property name="outputSource" ref="hibernateOutputSource"/>
</bean>
</property>
</bean>
</property>
<property name="chunkOperations">
<bean class="org.springframework.batch.repeat.support.RepeatTemplate">
<property name="interceptor" ref="hibernateOutputSource"/>
<property name="completionPolicy">
<bean class="org.springframework.batch.repeat.policy.SimpleCompletionPolicy">
<property name="chunkSize" value="3"/>
</bean>
<bean id="step1"
class="org.springframework.batch.execution.step.RepeatOperationsStepConfiguration">
<property name="tasklet">
<bean
class="org.springframework.batch.execution.tasklet.RestartableItemProviderTasklet">
<property name="itemProvider">
<bean
class="org.springframework.batch.item.provider.InputSourceItemProvider">
<property name="inputSource"
ref="hibernateInputSource" />
</bean>
</property>
</bean>
</property>
<property name="stepOperations">
<bean class="org.springframework.batch.repeat.support.RepeatTemplate">
<property name="exceptionHandler">
<bean class="org.springframework.batch.repeat.exception.handler.SimpleLimitExceptionHandler"
p:limit="5" p:useParent="true" p:type="java.lang.Exception"/>
<property name="itemProcessor">
<bean
class="org.springframework.batch.sample.item.processor.CustomerCreditIncreaseProcessor">
<property name="outputSource"
ref="hibernateOutputSource" />
</bean>
</property>
</bean>
</property>
</bean>
</property>
</bean>
<bean id="hibernateOutputSource"
class="org.springframework.batch.sample.dao.HibernateCreditWriter">
<property name="sessionFactory" ref="sessionFactory" />
</bean>
<bean id="hibernateInputSource"
class="org.springframework.batch.io.cursor.HibernateCursorInputSource"
scope="step">
<aop:scoped-proxy />
<property name="queryString" value="from CustomerCredit" />
<property name="sessionFactory" ref="sessionFactory" />
</bean>
<bean id="sessionFactory"
class="org.springframework.orm.hibernate3.LocalSessionFactoryBean">
<property name="dataSource" ref="dataSource" />
<property name="mappingResources">
<list>
<value>CustomerCredit.hbm.xml</value>
</list>
</property>
<property name="hibernateProperties">
<value>
hibernate.dialect=org.hibernate.dialect.HSQLDialect
</value>
</property>
</bean>
<bean parent="customEditorConfigurer" />
<property name="chunkOperations">
<bean
class="org.springframework.batch.repeat.support.RepeatTemplate">
<property name="interceptor"
ref="hibernateOutputSource" />
<property name="completionPolicy">
<bean
class="org.springframework.batch.repeat.policy.SimpleCompletionPolicy">
<property name="chunkSize" value="3" />
</bean>
</property>
</bean>
</property>
<property name="stepOperations">
<bean
class="org.springframework.batch.repeat.support.RepeatTemplate">
<property name="exceptionHandler">
<bean
class="org.springframework.batch.repeat.exception.handler.SimpleLimitExceptionHandler"
p:limit="2" p:useParent="true" p:type="java.lang.Exception" />
</property>
</bean>
</property>
</bean>
</property>
</bean>
<bean id="hibernateOutputSource"
class="org.springframework.batch.sample.dao.HibernateCreditWriter">
<property name="sessionFactory" ref="sessionFactory" />
</bean>
<bean id="hibernateInputSource"
class="org.springframework.batch.io.cursor.HibernateCursorInputSource"
scope="step">
<aop:scoped-proxy />
<property name="queryString" value="from CustomerCredit" />
<property name="sessionFactory" ref="sessionFactory" />
</bean>
<bean id="sessionFactory"
class="org.springframework.orm.hibernate3.LocalSessionFactoryBean">
<property name="dataSource" ref="dataSource" />
<property name="mappingResources">
<list>
<value>CustomerCredit.hbm.xml</value>
</list>
</property>
<property name="hibernateProperties">
<value>
<![CDATA[
hibernate.show_sql=true
hibernate.format_sql=true
hibernate.dialect=org.hibernate.dialect.HSQLDialect
]]>
</value>
</property>
</bean>
<bean parent="customEditorConfigurer" />
</beans>

View File

@@ -57,7 +57,7 @@
</bean>
<bean id="stepExecutor"
class="org.springframework.batch.execution.step.simple.SimpleStepExecutor"
class="org.springframework.batch.execution.step.simple.DefaultStepExecutor"
scope="prototype">
<property name="transactionManager" ref="${batch.transaction.manager}" />
<property name="repository" ref="simpleJobRepository" />

View File

@@ -3,6 +3,7 @@ package org.springframework.batch.sample;
import java.math.BigDecimal;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import org.springframework.batch.sample.item.processor.CustomerCreditIncreaseProcessor;
@@ -27,6 +28,8 @@ public abstract class AbstractCustomerCreditIncreaseTests extends
private static final String CREDIT_COLUMN = "CREDIT";
protected static final String ID_COLUMN = "ID";
private List creditsBeforeUpdate;
public void setJdbcTemplate(JdbcOperations jdbcTemplate) {
@@ -49,6 +52,8 @@ public abstract class AbstractCustomerCreditIncreaseTests extends
* Credit was increased by CREDIT_INCREASE
*/
protected void validatePostConditions() throws Exception {
final List matches = new ArrayList();
jdbcTemplate.query(ALL_CUSTOMERS, new RowMapper() {
@@ -56,12 +61,32 @@ public abstract class AbstractCustomerCreditIncreaseTests extends
final BigDecimal creditBeforeUpdate = (BigDecimal) creditsBeforeUpdate.get(rowNum);
final BigDecimal expectedCredit = creditBeforeUpdate
.add(CREDIT_INCREASE);
assertEquals(expectedCredit, rs.getBigDecimal(CREDIT_COLUMN));
if (expectedCredit.equals(rs.getBigDecimal(CREDIT_COLUMN))) {
matches.add(rs.getBigDecimal(ID_COLUMN));
}
return null;
}
});
assertEquals(getExpectedMatches(), matches.size());
checkMatches(matches);
}
/**
* @param matches
*/
protected void checkMatches(List matches) {
// no-op...
}
/**
* @return the expected number of matches in the updated credits.
*/
protected int getExpectedMatches() {
return creditsBeforeUpdate.size();
}
}

View File

@@ -1,5 +1,8 @@
package org.springframework.batch.sample;
import java.math.BigDecimal;
import java.util.List;
import org.springframework.batch.sample.dao.HibernateCreditWriter;
import org.springframework.jdbc.UncategorizedSQLException;
import org.springframework.orm.hibernate3.HibernateJdbcException;
@@ -32,7 +35,7 @@ public class HibernateFailureJobFunctionalTests extends
*/
protected void onTearDown() throws Exception {
super.onTearDown();
writer.setFailOnFlush(false);
writer.setFailOnFlush(-1);
}
protected String[] getConfigLocations() {
@@ -45,7 +48,7 @@ public class HibernateFailureJobFunctionalTests extends
* @see org.springframework.batch.sample.AbstractValidatingBatchLauncherTests#testLaunchJob()
*/
public void testLaunchJob() throws Exception {
writer.setFailOnFlush(true);
writer.setFailOnFlush(2);
int before = jdbcTemplate.queryForInt("SELECT COUNT(*) from CUSTOMER");
assertTrue(before>0);
@@ -57,8 +60,9 @@ public class HibernateFailureJobFunctionalTests extends
} catch (UncategorizedSQLException e) {
// Expected, but check that the exception was registered:
assertEquals(1, writer.getErrors().size());
throw e;
// throw e;
}
validatePostConditions();
int after = jdbcTemplate.queryForInt("SELECT COUNT(*) from CUSTOMER");
assertEquals(before, after);
}
@@ -68,6 +72,21 @@ public class HibernateFailureJobFunctionalTests extends
*/
protected void validatePostConditions() throws Exception {
// TODO: fix so that the postconditions in super class are true
// super.validatePostConditions();
super.validatePostConditions();
}
/* (non-Javadoc)
* @see org.springframework.batch.sample.AbstractCustomerCreditIncreaseTests#checkMatches(java.util.List)
*/
protected void checkMatches(List matches) {
assertFalse(matches.contains(new BigDecimal(2)));
}
/* (non-Javadoc)
* @see org.springframework.batch.sample.AbstractCustomerCreditIncreaseTests#getExpectedMatches()
*/
protected int getExpectedMatches() {
// One record was skipped, so it won't be processed in the final state.
return super.getExpectedMatches()-1;
}
}