Skip to content

Commit c7b5b01

Browse files
committed
Fix inconsistent state validation rules in RepositoryItemReader
Before this commit, state validation rules in RepositoryItemReader were not consistent with those applied in its builder. This commit makes validation rules consistent between the two ways of creating a RepositoryItemReader. This commit also adds a getter for the component name in ExecutionContextUserSupport to be able to assert on it where appropriate down the hierarchy. Resolves #4276
1 parent 6697683 commit c7b5b01

File tree

6 files changed

+53
-6
lines changed

6 files changed

+53
-6
lines changed

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/ItemStreamSupport.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2006-2013 the original author or authors.
2+
* Copyright 2006-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
*
2323
* @author Dave Syer
2424
* @author Dean de Bree
25+
* @author Mahmoud Ben Hassine
2526
*
2627
*/
2728
public abstract class ItemStreamSupport implements ItemStream {
@@ -63,6 +64,14 @@ public void setName(String name) {
6364
this.setExecutionContextName(name);
6465
}
6566

67+
/**
68+
* Get the name of the component
69+
* @return the name of the component
70+
*/
71+
public String getName() {
72+
return executionContextUserSupport.getName();
73+
}
74+
6675
protected void setExecutionContextName(String name) {
6776
executionContextUserSupport.setName(name);
6877
}

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/data/RepositoryItemReader.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.springframework.util.Assert;
3838
import org.springframework.util.ClassUtils;
3939
import org.springframework.util.MethodInvoker;
40+
import org.springframework.util.StringUtils;
4041

4142
/**
4243
* <p>
@@ -72,6 +73,7 @@
7273
*
7374
* @author Michael Minella
7475
* @author Antoine Kapps
76+
* @author Mahmoud Ben Hassine
7577
* @since 2.2
7678
*/
7779
public class RepositoryItemReader<T> extends AbstractItemCountingItemStreamItemReader<T> implements InitializingBean {
@@ -119,7 +121,7 @@ public void setSort(Map<String, Sort.Direction> sorts) {
119121
}
120122

121123
/**
122-
* @param pageSize The number of items to retrieve per page.
124+
* @param pageSize The number of items to retrieve per page. Must be greater than 0.
123125
*/
124126
public void setPageSize(int pageSize) {
125127
this.pageSize = pageSize;
@@ -150,6 +152,10 @@ public void afterPropertiesSet() throws Exception {
150152
Assert.state(repository != null, "A PagingAndSortingRepository is required");
151153
Assert.state(pageSize > 0, "Page size must be greater than 0");
152154
Assert.state(sort != null, "A sort is required");
155+
Assert.state(this.methodName != null && !this.methodName.isEmpty(), "methodName is required.");
156+
if (isSaveState()) {
157+
Assert.state(StringUtils.hasText(getName()), "A name is required when saveState is set to true.");
158+
}
153159
}
154160

155161
@Nullable

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/data/builder/RepositoryItemReaderBuilder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017-2018 the original author or authors.
2+
* Copyright 2017-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -158,7 +158,7 @@ public RepositoryItemReaderBuilder<T> sorts(Map<String, Sort.Direction> sorts) {
158158
/**
159159
* Establish the pageSize for the generated RepositoryItemReader.
160160
*
161-
* @param pageSize The number of items to retrieve per page.
161+
* @param pageSize The number of items to retrieve per page. Must be greater than 0.
162162
* @return The current instance of the builder.
163163
* @see RepositoryItemReader#setPageSize(int)
164164
*/
@@ -236,6 +236,7 @@ public RepositoryItemReader<T> build() {
236236

237237
Assert.notNull(this.sorts, "sorts map is required.");
238238
Assert.notNull(this.repository, "repository is required.");
239+
Assert.isTrue(this.pageSize > 0, "Page size must be greater than 0");
239240
Assert.hasText(this.methodName, "methodName is required.");
240241
if (this.saveState) {
241242
Assert.state(StringUtils.hasText(this.name), "A name is required when saveState is set to true.");

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/util/ExecutionContextUserSupport.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2006-2007 the original author or authors.
2+
* Copyright 2006-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
2323
* {@link ExecutionContext} based on the name.
2424
*
2525
* @author Robert Kasanicky
26+
* @author Mahmoud Ben Hassine
2627
*/
2728
public class ExecutionContextUserSupport {
2829

@@ -40,7 +41,7 @@ public ExecutionContextUserSupport(String name) {
4041
/**
4142
* @return name used to uniquely identify this instance's entries in shared context.
4243
*/
43-
protected String getName() {
44+
public String getName() {
4445
return this.name;
4546
}
4647

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/data/RepositoryItemReaderTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,22 @@ public void testAfterPropertiesSet() throws Exception {
108108
// expected
109109
}
110110

111+
try {
112+
reader = new RepositoryItemReader<>();
113+
reader.setRepository(repository);
114+
reader.setPageSize(1);
115+
reader.setSort(sorts);
116+
reader.afterPropertiesSet();
117+
fail();
118+
} catch (IllegalStateException iae) {
119+
// expected
120+
}
121+
111122
reader = new RepositoryItemReader<>();
112123
reader.setRepository(repository);
113124
reader.setPageSize(1);
114125
reader.setSort(sorts);
126+
reader.setMethodName("findAll");
115127
reader.afterPropertiesSet();
116128
}
117129

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/data/builder/RepositoryItemReaderBuilderTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
/**
4242
* @author Glenn Renfro
4343
* @author Drummond Dawson
44+
* @author Mahmoud Ben Hassine
4445
*/
4546
public class RepositoryItemReaderBuilderTests {
4647

@@ -234,6 +235,23 @@ public void testNoRepository() throws Exception {
234235
}
235236
}
236237

238+
@Test
239+
public void testInvalidPageSize() {
240+
try {
241+
new RepositoryItemReaderBuilder<>()
242+
.repository(repository)
243+
.sorts(this.sorts)
244+
.pageSize(-1)
245+
.build();
246+
247+
fail("IllegalArgumentException should have been thrown");
248+
}
249+
catch (IllegalArgumentException iae) {
250+
assertEquals("IllegalArgumentException message did not match the expected result.",
251+
"Page size must be greater than 0", iae.getMessage());
252+
}
253+
}
254+
237255
@Test
238256
public void testArguments() throws Exception {
239257
List<String> args = new ArrayList<>(3);

0 commit comments

Comments
 (0)