From dbfb336bb9084319ce58a224c6a87a5fd35cd834 Mon Sep 17 00:00:00 2001 From: Michael Minella Date: Fri, 2 Jun 2017 17:17:53 -0500 Subject: [PATCH 1/2] Align XML and Java based configuration This commit aligns the XML and Java based validations. When using XML to configure a chunk oriented step both an ItemReader and ItemWriter are requied. However when using Java based configuration the ItemWriter is optional if an ItemProcessor is present. Having no ItemWriter and only an ItemProcessor lead to strange results in one of our batch jobs, which was accidentily configured without an ItemProcessor. Related: BATCH-1520 Fixes: BATCH-2624 --- .../builder/FaultTolerantStepBuilder.java | 4 +-- .../core/step/builder/SimpleStepBuilder.java | 4 +-- .../FaultTolerantStepFactoryBeanTests.java | 26 +---------------- .../step/item/SimpleStepFactoryBeanTests.java | 29 ++----------------- 4 files changed, 7 insertions(+), 56 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/FaultTolerantStepBuilder.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/FaultTolerantStepBuilder.java index cfe440fa05..8066943aef 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/FaultTolerantStepBuilder.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/FaultTolerantStepBuilder.java @@ -178,7 +178,7 @@ protected void registerStepListenerAsSkipListener() { @Override protected Tasklet createTasklet() { Assert.state(getReader() != null, "ItemReader must be provided"); - Assert.state(getProcessor() != null || getWriter() != null, "ItemWriter or ItemProcessor must be provided"); + Assert.state(getWriter() != null, "ItemWriter must be provided"); addSpecialExceptions(); registerSkipListeners(); ChunkProvider chunkProvider = createChunkProvider(); @@ -772,6 +772,6 @@ public boolean equals(Object obj) { } return chunkListener.equals(obj); } - + } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java index f25f454d9d..ca2a8ee433 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2014 the original author or authors. + * Copyright 2006-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -156,7 +156,7 @@ private void checkAndAddItemListener(StepListener stepListener) { @Override protected Tasklet createTasklet() { Assert.state(reader != null, "ItemReader must be provided"); - Assert.state(processor != null || writer != null, "ItemWriter or ItemProcessor must be provided"); + Assert.state(writer != null, "ItemWriter must be provided"); RepeatOperations repeatOperations = createChunkOperations(); SimpleChunkProvider chunkProvider = new SimpleChunkProvider<>(getReader(), repeatOperations); SimpleChunkProcessor chunkProcessor = new SimpleChunkProcessor<>(getProcessor(), getWriter()); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java index 1c1da512df..4e8b12d585 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -372,30 +372,6 @@ public void testProcessFilter() throws Exception { .getName())); } - @Test - public void testNullWriter() throws Exception { - - factory.setItemWriter(null); - Step step = factory.getObject(); - - step.execute(stepExecution); - - assertEquals(0, stepExecution.getSkipCount()); - assertEquals(0, stepExecution.getReadSkipCount()); - assertEquals(5, stepExecution.getReadCount()); - // Write count is incremented even if nothing happens - assertEquals(5, stepExecution.getWriteCount()); - assertEquals(0, stepExecution.getFilterCount()); - assertEquals(0, stepExecution.getRollbackCount()); - - // writer skips "4" - assertTrue(reader.getRead().contains("4")); - - assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus()); - assertStepExecutionsAreEqual(stepExecution, repository.getLastStepExecution(jobExecution.getJobInstance(), step - .getName())); - } - /** * Check items causing errors are skipped as expected. */ diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java index b272df36f6..46e2417711 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import java.util.List; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.ChunkListener; @@ -475,32 +476,6 @@ public void onWriteError(Exception exception, List items) { } - @Test - public void testNullWriter() throws Exception { - - SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); - factory.setItemWriter(null); - factory.setItemProcessor(new ItemProcessor() { - @Override - public String process(String item) throws Exception { - written.add(item); - return null; - } - }); - - Step step = factory.getObject(); - - job.setSteps(Collections.singletonList(step)); - - JobExecution jobExecution = repository.createJobExecution(job.getName(), new JobParameters()); - - job.execute(jobExecution); - - assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus()); - assertEquals("[foo, bar, spam]", written.toString()); - - } - private SimpleStepFactoryBean getStepFactory(String... args) throws Exception { SimpleStepFactoryBean factory = new SimpleStepFactoryBean(); From b8291d5a1f5926951a2bbfddf8bcd1369befd9a5 Mon Sep 17 00:00:00 2001 From: Mahmoud Ben Hassine Date: Thu, 5 Apr 2018 11:54:53 +0200 Subject: [PATCH 2/2] BATCH-2624: add tests on mandatory item reader/writer --- .../core/step/builder/SimpleStepBuilder.java | 2 +- .../FaultTolerantStepFactoryBeanTests.java | 29 ++++++++++++++++- .../step/item/SimpleStepFactoryBeanTests.java | 32 +++++++++++++++++-- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java index ca2a8ee433..0cc69f8cc3 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/SimpleStepBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2017 the original author or authors. + * Copyright 2006-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java index 4e8b12d585..fc9928e8ac 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2017 the original author or authors. + * Copyright 2008-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,8 +27,10 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.aop.framework.ProxyFactory; import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.ChunkListener; @@ -77,6 +79,9 @@ */ public class FaultTolerantStepFactoryBeanTests { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + protected final Log logger = LogFactory.getLog(getClass()); private FaultTolerantStepFactoryBean factory; @@ -135,6 +140,28 @@ public void setUp() throws Exception { repository.add(stepExecution); } + @Test + public void testMandatoryReader() throws Exception { + factory = new FaultTolerantStepFactoryBean<>(); + factory.setItemWriter(writer); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("ItemReader must be provided"); + + factory.getObject(); + } + + @Test + public void testMandatoryWriter() throws Exception { + factory = new FaultTolerantStepFactoryBean<>(); + factory.setItemReader(reader); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("ItemWriter must be provided"); + + factory.getObject(); + } + /** * Non-skippable (and non-fatal) exception causes failure immediately. * diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java index 46e2417711..a62ed6f0ba 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2017 the original author or authors. + * Copyright 2006-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,8 +27,9 @@ import java.util.List; import org.junit.Before; -import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.ChunkListener; import org.springframework.batch.core.ItemProcessListener; @@ -64,6 +65,9 @@ */ public class SimpleStepFactoryBeanTests { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + private List listened = new ArrayList(); private SimpleJobRepository repository = new SimpleJobRepository(new MapJobInstanceDao(), new MapJobExecutionDao(), @@ -78,7 +82,7 @@ public void write(List data) throws Exception { } }; - private ItemReader reader; + private ItemReader reader = new ListItemReader<>(Arrays.asList("a", "b", "c")); private SimpleJob job = new SimpleJob(); @@ -93,6 +97,28 @@ public void testMandatoryProperties() throws Exception { new SimpleStepFactoryBean().getObject(); } + @Test + public void testMandatoryReader() throws Exception { + SimpleStepFactoryBean factory = new SimpleStepFactoryBean<>(); + factory.setItemWriter(writer); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("ItemReader must be provided"); + + factory.getObject(); + } + + @Test + public void testMandatoryWriter() throws Exception { + SimpleStepFactoryBean factory = new SimpleStepFactoryBean<>(); + factory.setItemReader(reader); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("ItemWriter must be provided"); + + factory.getObject(); + } + @Test public void testSimpleJob() throws Exception {