Skip to content

Commit 4ea5438

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 a547fe5 commit 4ea5438

File tree

6 files changed

+36
-12
lines changed

6 files changed

+36
-12
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2006-2022 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.
@@ -72,6 +72,14 @@ public void setName(String name) {
7272
this.setExecutionContextName(name);
7373
}
7474

75+
/**
76+
* Get the name of the component
77+
* @return the name of the component
78+
*/
79+
public String getName() {
80+
return executionContextUserSupport.getName();
81+
}
82+
7583
protected void setExecutionContextName(String name) {
7684
executionContextUserSupport.setName(name);
7785
}

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>
@@ -76,6 +77,7 @@
7677
*
7778
* @author Michael Minella
7879
* @author Antoine Kapps
80+
* @author Mahmoud Ben Hassine
7981
* @since 2.2
8082
*/
8183
public class RepositoryItemReader<T> extends AbstractItemCountingItemStreamItemReader<T> implements InitializingBean {
@@ -121,7 +123,7 @@ public void setSort(Map<String, Sort.Direction> sorts) {
121123
}
122124

123125
/**
124-
* @param pageSize The number of items to retrieve per page.
126+
* @param pageSize The number of items to retrieve per page. Must be greater than 0.
125127
*/
126128
public void setPageSize(int pageSize) {
127129
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 & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017-2022 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.
@@ -16,20 +16,14 @@
1616

1717
package org.springframework.batch.item.data.builder;
1818

19-
import java.lang.reflect.Method;
20-
import java.util.ArrayList;
2119
import java.util.Arrays;
2220
import java.util.List;
2321
import java.util.Map;
2422

2523
import org.springframework.batch.item.data.RepositoryItemReader;
26-
import org.springframework.cglib.proxy.Enhancer;
27-
import org.springframework.cglib.proxy.MethodInterceptor;
28-
import org.springframework.cglib.proxy.MethodProxy;
2924
import org.springframework.data.domain.Sort;
3025
import org.springframework.data.repository.PagingAndSortingRepository;
3126
import org.springframework.util.Assert;
32-
import org.springframework.util.CollectionUtils;
3327
import org.springframework.util.StringUtils;
3428

3529
/**
@@ -148,7 +142,7 @@ public RepositoryItemReaderBuilder<T> sorts(Map<String, Sort.Direction> sorts) {
148142

149143
/**
150144
* Establish the pageSize for the generated RepositoryItemReader.
151-
* @param pageSize The number of items to retrieve per page.
145+
* @param pageSize The number of items to retrieve per page. Must be greater than 0.
152146
* @return The current instance of the builder.
153147
* @see RepositoryItemReader#setPageSize(int)
154148
*/
@@ -191,6 +185,7 @@ public RepositoryItemReaderBuilder<T> methodName(String methodName) {
191185
public RepositoryItemReader<T> build() {
192186
Assert.notNull(this.sorts, "sorts map is required.");
193187
Assert.notNull(this.repository, "repository is required.");
188+
Assert.isTrue(this.pageSize > 0, "Page size must be greater than 0");
194189
Assert.hasText(this.methodName, "methodName is required.");
195190
if (this.saveState) {
196191
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
* generating keys for {@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: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,13 @@ void testAfterPropertiesSet() throws Exception {
9393
reader.setRepository(repository);
9494
reader.setPageSize(1);
9595
reader.setSort(sorts);
96+
assertThrows(IllegalStateException.class, reader::afterPropertiesSet);
97+
98+
reader = new RepositoryItemReader<>();
99+
reader.setRepository(repository);
100+
reader.setPageSize(1);
101+
reader.setSort(sorts);
102+
reader.setMethodName("findAll");
96103
reader.afterPropertiesSet();
97104
}
98105

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,13 @@ void testNoRepository() {
141141
assertEquals("repository is required.", exception.getMessage());
142142
}
143143

144+
@Test
145+
void testInvalidPageSize() {
146+
var builder = new RepositoryItemReaderBuilder<>().repository(repository).sorts(this.sorts).pageSize(-1);
147+
Exception exception = assertThrows(IllegalArgumentException.class, builder::build);
148+
assertEquals("Page size must be greater than 0", exception.getMessage());
149+
}
150+
144151
@Test
145152
void testArguments() throws Exception {
146153
List<String> args = new ArrayList<>(3);

0 commit comments

Comments
 (0)