From 5ad74053cd209cc02a0dff4d66992bafe196fc68 Mon Sep 17 00:00:00 2001 From: dsyer Date: Mon, 28 Jan 2008 09:46:21 +0000 Subject: [PATCH] Resolved some TODOs. --- .../batch/core/repository/JobRepository.java | 2 +- .../execution/repository/dao/StepDao.java | 2 - .../scope/StepSynchronizationManager.java | 2 +- .../simple/ThreadStepInterruptionPolicy.java | 1 - .../batch/io/file/FlatFileItemWriter.java | 6 +- .../file/DefaultFlatFileItemReaderTests.java | 45 +++++------- .../io/file/FlatFileItemWriterTests.java | 71 +++++++++---------- .../ExceptionClassifierRetryPolicyTests.java | 1 - 8 files changed, 58 insertions(+), 72 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java index 050a40f3a..edf60ab55 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java @@ -52,7 +52,7 @@ public interface JobRepository { * exists, its persisted values (including ID) will be returned in a new * {@link JobInstance} object. If no previous run is found, a new job will * be created and returned. - * @param jobParameters TODO + * @param jobParameters the runtime parameters for the job * @param jobConfiguration * describes the configuration for this job * diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/dao/StepDao.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/dao/StepDao.java index 0efd350c9..9c49cf6d7 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/dao/StepDao.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/dao/StepDao.java @@ -25,8 +25,6 @@ import org.springframework.batch.core.domain.StepInstance; /** * Data access object for steps. * - * TODO: Add java doc. - * * @author Lucas Ward * */ diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/scope/StepSynchronizationManager.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/scope/StepSynchronizationManager.java index c37de1580..3d05718fb 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/scope/StepSynchronizationManager.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/scope/StepSynchronizationManager.java @@ -37,7 +37,7 @@ public class StepSynchronizationManager { * Method for registering a context - should only be used by * {@link StepExecutor} implementations to ensure that {@link #getContext()} * always returns the correct value. - * @param context TODO + * @param context the step context to register */ public static void register(StepContext context) { StepSynchronizationManager.contextHolder.set(context); diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/simple/ThreadStepInterruptionPolicy.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/simple/ThreadStepInterruptionPolicy.java index bdbd24636..f82ef506f 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/simple/ThreadStepInterruptionPolicy.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/simple/ThreadStepInterruptionPolicy.java @@ -41,7 +41,6 @@ public class ThreadStepInterruptionPolicy implements StepInterruptionPolicy { } /** - * TODO: add more interruption policies: they should all check context.isTerminateOnly() * @param context the current context * @return true if the job has been interrupted */ diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/io/file/FlatFileItemWriter.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/io/file/FlatFileItemWriter.java index ba92a996a..443b4ec19 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/io/file/FlatFileItemWriter.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/io/file/FlatFileItemWriter.java @@ -76,11 +76,11 @@ public class FlatFileItemWriter extends AbstractTransactionalIoSource implements private static final String LINE_SEPARATOR = System.getProperty("line.separator"); - public static final String WRITTEN_STATISTICS_NAME = "Written"; + public static final String WRITTEN_STATISTICS_NAME = "written"; - public static final String RESTART_COUNT_STATISTICS_NAME = "Restart count"; + public static final String RESTART_COUNT_STATISTICS_NAME = "restart.count"; - public static final String RESTART_DATA_NAME = "flatfileoutputtemplate.currentLine"; + public static final String RESTART_DATA_NAME = "flatfileoutput.current.line"; private Resource resource; diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/DefaultFlatFileItemReaderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/DefaultFlatFileItemReaderTests.java index 49b498752..945e4c350 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/DefaultFlatFileItemReaderTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/DefaultFlatFileItemReaderTests.java @@ -17,6 +17,7 @@ package org.springframework.batch.io.file; import java.io.IOException; +import java.util.Properties; import junit.framework.TestCase; @@ -32,9 +33,9 @@ import org.springframework.transaction.support.TransactionSynchronization; /** * Tests for {@link DefaultFlatFileItemReader} - * + * * @author robert.kasanicky - * + * * TODO only regular reading is tested currently, add exception cases, restart, * skip, validation... */ @@ -49,11 +50,11 @@ public class DefaultFlatFileItemReaderTests extends TestCase { // simple stub instead of a realistic tokenizer private LineTokenizer tokenizer = new LineTokenizer() { public FieldSet tokenize(String line) { - return new DefaultFieldSet(new String[]{line}); + return new DefaultFieldSet(new String[] { line }); } }; - private FieldSetMapper fieldSetMapper = new FieldSetMapper(){ + private FieldSetMapper fieldSetMapper = new FieldSetMapper() { public Object mapLine(FieldSet fs) { return fs; } @@ -118,7 +119,7 @@ public class DefaultFlatFileItemReaderTests extends TestCase { // statistics.get(FlatFileInputTemplate.SKIPPED_STATISTICS_NAME)); } - + /** * Test skip and skipRollback functionality * @throws IOException @@ -135,17 +136,16 @@ public class DefaultFlatFileItemReaderTests extends TestCase { inputSource.read(); // #3 // mark record as skipped inputSource.skip(); - // rollback + // rollback inputSource.getTransactionSynchronization().afterCompletion(TransactionSynchronization.STATUS_ROLLED_BACK); // read next record inputSource.read(); // should be #1 - + // we should now process all records after first commit point, that are // not marked as skipped assertEquals("[testLine2]", inputSource.read().toString()); } - /** * Test skip and skipRollback functionality @@ -161,22 +161,17 @@ public class DefaultFlatFileItemReaderTests extends TestCase { inputSource.read(); inputSource.skip(); inputSource.read(); - // TODO - // statistics = template.getStatistics(); - // skipped = (String) - // statistics.get(FlatFileInputTemplate.SKIPPED_STATISTICS_NAME); - // read = (String) - // statistics.get(FlatFileInputTemplate.READ_STATISTICS_NAME); + + Properties statistics = inputSource.getStatistics(); + String skipped = statistics.getProperty(DefaultFlatFileItemReader.SKIPPED_STATISTICS_NAME); + String read = statistics.getProperty(DefaultFlatFileItemReader.READ_STATISTICS_NAME); // call unknown, which has no influence and therefore statistics should // be the same inputSource.getTransactionSynchronization().afterCompletion(TransactionSynchronization.STATUS_UNKNOWN); - // TODO - // statistics = template.getStatistics(); - // assertEquals(skipped, (String) - // statistics.get(FlatFileInputTemplate.SKIPPED_STATISTICS_NAME)); - // assertEquals(read, (String) - // statistics.get(FlatFileInputTemplate.READ_STATISTICS_NAME)); + statistics = inputSource.getStatistics(); + assertEquals(skipped, statistics.getProperty(DefaultFlatFileItemReader.SKIPPED_STATISTICS_NAME)); + assertEquals(read, statistics.getProperty(DefaultFlatFileItemReader.READ_STATISTICS_NAME)); } public void testRestartFromNullData() throws Exception { @@ -210,8 +205,8 @@ public class DefaultFlatFileItemReaderTests extends TestCase { // get restart data RestartData restartData = inputSource.getRestartData(); - // TODO - // assertEquals("4", (String) restartData); + assertEquals("4", (String) restartData.getProperties().getProperty( + DefaultFlatFileItemReader.READ_STATISTICS_NAME)); // close input inputSource.close(); @@ -225,10 +220,8 @@ public class DefaultFlatFileItemReaderTests extends TestCase { assertEquals("[testLine5]", inputSource.read().toString()); assertEquals("[testLine6]", inputSource.read().toString()); - // TODO - // Map statistics = template.getStatistics(); - // assertEquals("6", - // statistics.get(FlatFileInputTemplate.READ_STATISTICS_NAME)); + Properties statistics = inputSource.getStatistics(); + assertEquals("6", statistics.getProperty(DefaultFlatFileItemReader.READ_STATISTICS_NAME)); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/FlatFileItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/FlatFileItemWriterTests.java index 5018eaf68..3c35cc9cf 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/FlatFileItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/io/file/FlatFileItemWriterTests.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.FileReader; import java.io.IOException; import java.util.Collections; +import java.util.Properties; import junit.framework.TestCase; @@ -33,8 +34,8 @@ import org.springframework.transaction.support.TransactionSynchronizationManager import org.springframework.transaction.support.TransactionSynchronizationUtils; /** - * Tests of regular usage for {@link FlatFileItemWriter} Exception cases will - * be in separate TestCase classes with different setUp and + * Tests of regular usage for {@link FlatFileItemWriter} Exception cases will be + * in separate TestCase classes with different setUp and * tearDown methods * * @author robert.kasanicky @@ -56,16 +57,16 @@ public class FlatFileItemWriterTests extends TestCase { private BufferedReader reader; /** - * Create temporary output file, define mock behaviour, set dependencies - * and initialize the object under test + * Create temporary output file, define mock behaviour, set dependencies and + * initialize the object under test */ protected void setUp() throws Exception { - - if(TransactionSynchronizationManager.isSynchronizationActive()){ + + if (TransactionSynchronizationManager.isSynchronizationActive()) { TransactionSynchronizationManager.clearSynchronization(); } TransactionSynchronizationManager.initSynchronization(); - + outputFile = File.createTempFile("flatfile-output-", ".tmp"); inputSource.setResource(new FileSystemResource(outputFile)); @@ -79,25 +80,24 @@ public class FlatFileItemWriterTests extends TestCase { * Release resources and delete the temporary output file */ protected void tearDown() throws Exception { - if( reader != null){ + if (reader != null) { reader.close(); } inputSource.close(); outputFile.delete(); } - + /* - * Read a line from the output file, if the reader has not been - * created, recreate. This method is only necessary because - * running the tests in a UNIX environment locks the file - * if it's open for writing. + * Read a line from the output file, if the reader has not been created, + * recreate. This method is only necessary because running the tests in a + * UNIX environment locks the file if it's open for writing. */ - private String readLine() throws IOException{ - - if(reader == null){ + private String readLine() throws IOException { + + if (reader == null) { reader = new BufferedReader(new FileReader(outputFile)); } - + return reader.readLine(); } @@ -108,7 +108,7 @@ public class FlatFileItemWriterTests extends TestCase { inputSource.write(TEST_STRING); inputSource.close(); String lineFromFile = readLine(); - + assertEquals(TEST_STRING, lineFromFile); } @@ -166,7 +166,7 @@ public class FlatFileItemWriterTests extends TestCase { } }); Object data = new Object(); - inputSource.write(new Object[] {data, data}); + inputSource.write(new Object[] { data, data }); inputSource.close(); String lineFromFile = readLine(); assertEquals("FOO:" + data.toString(), lineFromFile); @@ -180,6 +180,7 @@ public class FlatFileItemWriterTests extends TestCase { public void testWriteWithConverterAndInfiniteLoopInConvertedCollection() throws IOException { inputSource.setConverter(new Converter() { boolean converted = false; + public Object convert(Object input) { if (converted) { return input; @@ -192,9 +193,10 @@ public class FlatFileItemWriterTests extends TestCase { try { inputSource.write(data); fail("Expected IllegalStateException"); - } catch (IllegalStateException e) { + } + catch (IllegalStateException e) { // expected - assertTrue("Wrong message: "+e, e.getMessage().toLowerCase().indexOf("infinite")>=0); + assertTrue("Wrong message: " + e, e.getMessage().toLowerCase().indexOf("infinite") >= 0); } inputSource.close(); String lineFromFile = readLine(); @@ -294,20 +296,20 @@ public class FlatFileItemWriterTests extends TestCase { // commit commit(); - + // this will be rolled back... inputSource.write("this will be rolled back"); // rollback rollback(); - + // write more lines inputSource.write("testLine4"); inputSource.write("testLine5"); // commit commit(); - + // get restart data RestartData restartData = inputSource.getRestartData(); // close template @@ -343,11 +345,9 @@ public class FlatFileItemWriterTests extends TestCase { } // get statistics - // Statistics statistics = template.getStatistics(); + Properties statistics = inputSource.getStatistics(); // 3 lines were written to the file after restart - // TODO - // assertEquals("3", - // statistics.get(FlatFileOutputTemplate.WRITTEN_STATISTICS_NAME)); + assertEquals("3", statistics.getProperty(FlatFileItemWriter.WRITTEN_STATISTICS_NAME)); } @@ -366,25 +366,22 @@ public class FlatFileItemWriterTests extends TestCase { inputSource = new FlatFileItemWriter(); RestartData restartData = inputSource.getRestartData(); assertNotNull(restartData); - // TODO: assert the properties of the default restart data assertEquals(1, restartData.getProperties().size()); + assertEquals("0", restartData.getProperties().getProperty(FlatFileItemWriter.RESTART_DATA_NAME)); } - + private void commit() { - TransactionSynchronizationUtils.invokeAfterCompletion( - TransactionSynchronizationManager.getSynchronizations(), + TransactionSynchronizationUtils.invokeAfterCompletion(TransactionSynchronizationManager.getSynchronizations(), TransactionSynchronization.STATUS_COMMITTED); } private void rollback() { - TransactionSynchronizationUtils.invokeAfterCompletion( - TransactionSynchronizationManager.getSynchronizations(), + TransactionSynchronizationUtils.invokeAfterCompletion(TransactionSynchronizationManager.getSynchronizations(), TransactionSynchronization.STATUS_ROLLED_BACK); } - + private void unknown() { - TransactionSynchronizationUtils.invokeAfterCompletion( - TransactionSynchronizationManager.getSynchronizations(), + TransactionSynchronizationUtils.invokeAfterCompletion(TransactionSynchronizationManager.getSynchronizations(), TransactionSynchronization.STATUS_UNKNOWN); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java index c6cb6aa8b..d72322be6 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java @@ -69,7 +69,6 @@ public class ExceptionClassifierRetryPolicyTests extends TestCase { policy.setExceptionClassifier(new ExceptionClassifierSupport() { public Object classify(Throwable throwable) { - //TODO probably (==null) and (!=null) branches are interchanged? if (throwable != null) { return "foo"; }