From fa85ba54a354ff95722b80f34d295c429fc04886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henning=20P=C3=B6ttker?= Date: Sat, 3 Jul 2021 00:37:25 +0200 Subject: [PATCH] Fix multi-threaded empty read for JdbcPagingItemReader On empty input, the JdbcPagingItemReader cannot derive a start value for the sort key to be used in further queries. For multi-threaded steps, it is thus necessary to prevent the reader from trying to read further pages if the first page is empty. Issue #3898 --- .../item/database/JdbcPagingItemReader.java | 23 ++++--- .../JdbcPagingItemReaderAsyncTests.java | 5 +- ...bcPagingItemReaderEmptyResultSetTests.java | 68 +++++++++++++++++++ ...dbcPagingItemReaderCommonTests-context.xml | 17 ++--- ...JpaPagingItemReaderCommonTests-context.xml | 17 ++--- .../item/database/data-source-context.xml | 18 +++-- 6 files changed, 103 insertions(+), 45 deletions(-) create mode 100644 spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java index 889cdcd7b4..5ba05e23fe 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2018 the original author or authors. + * Copyright 2006-2021 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. @@ -19,7 +19,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; -import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -179,7 +179,6 @@ public void afterPropertiesSet() throws Exception { } @Override - @SuppressWarnings("unchecked") protected void doReadPage() { if (results == null) { results = new CopyOnWriteArrayList<>(); @@ -190,7 +189,7 @@ protected void doReadPage() { PagingRowMapper rowCallback = new PagingRowMapper(); - List query; + List query; if (getPage() == 0) { if (logger.isDebugEnabled()) { @@ -202,8 +201,8 @@ protected void doReadPage() { getParameterMap(parameterValues, null), rowCallback); } else { - query = getJdbcTemplate().query(firstPageSql, - getParameterList(parameterValues, null).toArray(), rowCallback); + query = getJdbcTemplate().query(firstPageSql, rowCallback, + getParameterList(parameterValues, null).toArray()); } } else { @@ -211,7 +210,7 @@ protected void doReadPage() { } } - else { + else if (startAfterValues != null) { previousStartAfterValues = startAfterValues; if (logger.isDebugEnabled()) { logger.debug("SQL used for reading remaining pages: [" + remainingPagesSql + "]"); @@ -221,13 +220,15 @@ protected void doReadPage() { getParameterMap(parameterValues, startAfterValues), rowCallback); } else { - query = getJdbcTemplate().query(remainingPagesSql, - getParameterList(parameterValues, startAfterValues).toArray(), rowCallback); + query = getJdbcTemplate().query(remainingPagesSql, rowCallback, + getParameterList(parameterValues, startAfterValues).toArray()); } } + else { + query = Collections.emptyList(); + } - Collection result = (Collection) query; - results.addAll(result); + results.addAll(query); } @Override diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java index 9113c20dcc..607178a79a 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2014 the original author or authors. + * Copyright 2009-2021 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. @@ -80,7 +80,8 @@ public class JdbcPagingItemReaderAsyncTests { @Before public void init() { JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); - maxId = jdbcTemplate.queryForObject("SELECT MAX(ID) from T_FOOS", Integer.class); + Integer maxIdResult = jdbcTemplate.queryForObject("SELECT MAX(ID) from T_FOOS", Integer.class); + maxId = maxIdResult == null ? 0 : maxIdResult; for (int i = maxId + 1; i <= ITEM_COUNT; i++) { jdbcTemplate.update("INSERT into T_FOOS (ID,NAME,VALUE) values (?, ?, ?)", i, "foo" + i, i); } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java new file mode 100644 index 0000000000..ee821ad3fc --- /dev/null +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java @@ -0,0 +1,68 @@ +/* + * Copyright 2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.item.database; + +import static org.junit.Assert.assertNull; + +import java.util.Collections; + +import javax.sql.DataSource; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.batch.item.ItemReader; +import org.springframework.batch.item.database.support.HsqlPagingQueryProvider; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jdbc.core.SingleColumnRowMapper; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(locations = "JdbcPagingItemReaderCommonTests-context.xml") +public class JdbcPagingItemReaderEmptyResultSetTests { + + private static final int PAGE_SIZE = 2; + private static final int EMPTY_READS = PAGE_SIZE + 1; + + @Autowired + private DataSource dataSource; + + @Test + public void testMultiplePageReadsOnEmptyResultSet() throws Exception { + final ItemReader reader = getItemReader(); + for (int i = 0; i < EMPTY_READS; i++) { + assertNull(reader.read()); + } + } + + private ItemReader getItemReader() throws Exception { + JdbcPagingItemReader reader = new JdbcPagingItemReader<>(); + reader.setDataSource(dataSource); + HsqlPagingQueryProvider queryProvider = new HsqlPagingQueryProvider(); + queryProvider.setSelectClause("select ID"); + queryProvider.setFromClause("from T_FOOS"); + queryProvider.setWhereClause("1 = 0"); + queryProvider.setSortKeys(Collections.singletonMap("ID", Order.ASCENDING)); + reader.setQueryProvider(queryProvider); + reader.setRowMapper(new SingleColumnRowMapper<>()); + reader.setPageSize(PAGE_SIZE); + reader.afterPropertiesSet(); + reader.setSaveState(false); + + return reader; + } +} diff --git a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderCommonTests-context.xml b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderCommonTests-context.xml index e1efe4b169..f68f311eb3 100644 --- a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderCommonTests-context.xml +++ b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderCommonTests-context.xml @@ -1,17 +1,12 @@ + xmlns:jdbc="http://www.springframework.org/schema/jdbc" + xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd + http://www.springframework.org/schema/jdbc https://www.springframework.org/schema/jdbc/spring-jdbc.xsd"> - - - - - - - - + + + diff --git a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JpaPagingItemReaderCommonTests-context.xml b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JpaPagingItemReaderCommonTests-context.xml index 8c6caafc33..d1e2b23243 100644 --- a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JpaPagingItemReaderCommonTests-context.xml +++ b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JpaPagingItemReaderCommonTests-context.xml @@ -1,17 +1,12 @@ + xmlns:jdbc="http://www.springframework.org/schema/jdbc" + xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd + http://www.springframework.org/schema/jdbc https://www.springframework.org/schema/jdbc/spring-jdbc.xsd"> - - - - - - - - + + + diff --git a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/data-source-context.xml b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/data-source-context.xml index 46c32cdf9f..c62d54ff88 100644 --- a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/data-source-context.xml +++ b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/data-source-context.xml @@ -1,20 +1,18 @@ + xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd + http://www.springframework.org/schema/jdbc https://www.springframework.org/schema/jdbc/spring-jdbc.xsd + http://www.springframework.org/schema/tx https://www.springframework.org/schema/tx/spring-tx.xsd"> + + + + - - - - - - - -