From 879b9fcb127eabf17f198f40a98847001423ac4b Mon Sep 17 00:00:00 2001 From: robokaso Date: Mon, 12 May 2008 08:48:07 +0000 Subject: [PATCH] RESOLVED - BATCH-616: Possible overflow in exit description if a stream.open() throws exception exit description is now truncated also on save --- .../repository/dao/JdbcStepExecutionDao.java | 31 ++++++++++++----- .../dao/AbstractStepExecutionDaoTests.java | 12 +++---- .../dao/JdbcStepExecutionDaoTests.java | 33 ++++++++++++++++++- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDao.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDao.java index 2f5f69fa4..6632eaddd 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDao.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDao.java @@ -178,6 +178,8 @@ public class JdbcStepExecutionDao extends AbstractJdbcBatchMetadataDao implement "to-be-saved (not updated) StepExecution can't already have a version assigned"); validateStepExecution(stepExecution); + + String exitDescription = truncateExitDescription(stepExecution.getExitStatus().getExitDescription()); stepExecution.setId(new Long(stepExecutionIncrementer.nextLongValue())); stepExecution.incrementVersion(); // should be 0 now @@ -185,7 +187,7 @@ public class JdbcStepExecutionDao extends AbstractJdbcBatchMetadataDao implement stepExecution.getStepName(), stepExecution.getJobExecutionId(), stepExecution.getStartTime(), stepExecution.getEndTime(), stepExecution.getStatus().toString(), stepExecution.getCommitCount(), stepExecution.getItemCount(), stepExecution.getExitStatus().isContinuable() ? "Y" : "N", - stepExecution.getExitStatus().getExitCode(), stepExecution.getExitStatus().getExitDescription(), + stepExecution.getExitStatus().getExitCode(), exitDescription, stepExecution.getReadSkipCount(), stepExecution.getWriteSkipCount(), stepExecution.getRollbackCount() }; getJdbcTemplate().update( getQuery(SAVE_STEP_EXECUTION), @@ -306,11 +308,7 @@ public class JdbcStepExecutionDao extends AbstractJdbcBatchMetadataDao implement // Do not check for existence of step execution considering // it is saved at every commit point. - String exitDescription = stepExecution.getExitStatus().getExitDescription(); - if (exitDescription != null && exitDescription.length() > EXIT_MESSAGE_LENGTH) { - exitDescription = exitDescription.substring(0, EXIT_MESSAGE_LENGTH); - logger.debug("Truncating long message before update of StepExecution: " + stepExecution); - } + String exitDescription = truncateExitDescription(stepExecution.getExitStatus().getExitDescription()); // Attempt to prevent concurrent modification errors by blocking here if // someone is already trying to do it. @@ -321,9 +319,8 @@ public class JdbcStepExecutionDao extends AbstractJdbcBatchMetadataDao implement stepExecution.getStatus().toString(), stepExecution.getCommitCount(), stepExecution.getItemCount(), stepExecution.getExitStatus().isContinuable() ? "Y" : "N", stepExecution.getExitStatus().getExitCode(), exitDescription, version, - stepExecution.getReadSkipCount(), - stepExecution.getWriteSkipCount(), stepExecution.getRollbackCount(), - stepExecution.getId(), stepExecution.getVersion() }; + stepExecution.getReadSkipCount(), stepExecution.getWriteSkipCount(), + stepExecution.getRollbackCount(), stepExecution.getId(), stepExecution.getVersion() }; int count = getJdbcTemplate().update( getQuery(UPDATE_STEP_EXECUTION), parameters, @@ -345,6 +342,22 @@ public class JdbcStepExecutionDao extends AbstractJdbcBatchMetadataDao implement } } + /** + * Truncate the exit description if the length exceeds + * {@link #EXIT_MESSAGE_LENGTH}. + * @param description the string to truncate + * @return truncated description + */ + private String truncateExitDescription(String description) { + if (description != null && description.length() > EXIT_MESSAGE_LENGTH) { + logger.debug("Truncating long message before update of StepExecution, original message is: " + description); + return description.substring(0, EXIT_MESSAGE_LENGTH); + } + else { + return description; + } + } + private class StepExecutionRowMapper implements RowMapper { private final JobExecution jobExecution; diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractStepExecutionDaoTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractStepExecutionDaoTests.java index 82079108e..db6599ceb 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractStepExecutionDaoTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractStepExecutionDaoTests.java @@ -38,17 +38,17 @@ import org.springframework.test.AbstractTransactionalDataSourceSpringContextTest */ public abstract class AbstractStepExecutionDaoTests extends AbstractTransactionalDataSourceSpringContextTests { - private StepExecutionDao dao; + protected StepExecutionDao dao; - private JobInstance jobInstance; + protected JobInstance jobInstance; - private JobExecution jobExecution; + protected JobExecution jobExecution; - private Step step; + protected Step step; - private StepExecution stepExecution; + protected StepExecution stepExecution; - private JobRepository repository; + protected JobRepository repository; /** * @return {@link StepExecutionDao} implementation ready for use. diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDaoTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDaoTests.java index e50075925..611ceba7f 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDaoTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/JdbcStepExecutionDaoTests.java @@ -1,6 +1,8 @@ package org.springframework.batch.core.repository.dao; +import org.springframework.batch.core.StepExecution; import org.springframework.batch.core.repository.JobRepository; +import org.springframework.batch.repeat.ExitStatus; public class JdbcStepExecutionDaoTests extends AbstractStepExecutionDaoTests { @@ -17,5 +19,34 @@ public class JdbcStepExecutionDaoTests extends AbstractStepExecutionDaoTests { protected String[] getConfigLocations() { return new String[] { "sql-dao-test.xml" }; } - + + /** + * Long exit descriptions are truncated on both save and update. + */ + public void testTruncateExitDescription() { + + StringBuffer sb = new StringBuffer(); + for (int i = 0; i < 100; i++) { + sb.append("too long exit description"); + } + String longDescription = sb.toString(); + + ExitStatus exitStatus = ExitStatus.FAILED.addExitDescription(longDescription); + + stepExecution.setExitStatus(exitStatus); + + dao.saveStepExecution(stepExecution); + + StepExecution retrievedAfterSave = dao.getStepExecution(jobExecution, step); + + assertTrue("Exit description should be truncated", retrievedAfterSave.getExitStatus().getExitDescription() + .length() < stepExecution.getExitStatus().getExitDescription().length()); + + dao.updateStepExecution(stepExecution); + + StepExecution retrievedAfterUpdate = dao.getStepExecution(jobExecution, step); + + assertTrue("Exit description should be truncated", retrievedAfterUpdate.getExitStatus().getExitDescription() + .length() < stepExecution.getExitStatus().getExitDescription().length()); + } }