From a913fc20413c3fe3c29437788811f7bad4d20e60 Mon Sep 17 00:00:00 2001 From: Bridger Howell Date: Thu, 26 Jan 2017 16:15:01 -0700 Subject: [PATCH 1/4] Add tests that paging queries that require table qualifiers don't break --- .../batch/item/AbstractItemReaderTests.java | 5 +- .../JdbcPagingItemReaderMultiTableTests.java | 69 +++++++++++++++++++ ...agingItemReaderMultiTableTests-context.xml | 30 ++++++++ .../item/database/init-bar-schema-hsqldb.sql | 12 ++++ 4 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java create mode 100644 spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests-context.xml create mode 100644 spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/init-bar-schema-hsqldb.sql diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java index d7058ec044..c6f1c3ba9f 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java @@ -71,8 +71,9 @@ public void testRead() throws Exception { @Test public void testEmptyInput() throws Exception { pointToEmptyInput(tested); - tested.read(); - assertNull(tested.read()); + + final Foo nullFoo = tested.read(); + assertNull(nullFoo); } /** diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java new file mode 100644 index 0000000000..05b9f36aaa --- /dev/null +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java @@ -0,0 +1,69 @@ +/* + * Copyright 2012 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 + * + * http://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 java.sql.ResultSet; +import java.sql.SQLException; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.springframework.batch.item.ItemReader; +import org.springframework.batch.item.database.support.HsqlPagingQueryProvider; +import org.springframework.batch.item.sample.Foo; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.test.context.ContextConfiguration; +/** + * Tests for {@link JpaPagingItemReader} where multiple tables are involved, requiring the sort key to + * be given table-qualified. + * + * @author Thomas Risberg + * @author Michael Minella + * @author Bridger Howell + */ +@ContextConfiguration(inheritLocations = false) +public class JdbcPagingItemReaderMultiTableTests extends AbstractGenericDataSourceItemReaderIntegrationTests { + + @Override + protected ItemReader createItemReader() throws Exception { + JdbcPagingItemReader inputSource = new JdbcPagingItemReader(); + inputSource.setDataSource(dataSource); + HsqlPagingQueryProvider queryProvider = new HsqlPagingQueryProvider(); + queryProvider.setSelectClause("select T_FOOS.ID, T_FOOS.NAME, T_FOOS.VALUE"); + queryProvider.setFromClause("from T_FOOS, T_BARS"); + queryProvider.setWhereClause("where T_BARS.ID = 1"); + Map sortKeys = new LinkedHashMap(); + sortKeys.put("T_FOOS.ID", Order.ASCENDING); + queryProvider.setSortKeys(sortKeys); + inputSource.setQueryProvider(queryProvider); + inputSource.setRowMapper( + new RowMapper() { + @Override + public Foo mapRow(ResultSet rs, int i) throws SQLException { + Foo foo = new Foo(); + foo.setId(rs.getInt(1)); + foo.setName(rs.getString(2)); + foo.setValue(rs.getInt(3)); + return foo; + } + } + ); + inputSource.setPageSize(3); + inputSource.afterPropertiesSet(); + inputSource.setSaveState(true); + + return inputSource; + } +} diff --git a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests-context.xml b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests-context.xml new file mode 100644 index 0000000000..d425aa9d03 --- /dev/null +++ b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests-context.xml @@ -0,0 +1,30 @@ + + + + + + + + classpath:org/springframework/batch/item/database/init-foo-schema-hsqldb.sql + classpath:org/springframework/batch/item/database/init-bar-schema-hsqldb.sql + + + + + + + + + + + + + + + + + diff --git a/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/init-bar-schema-hsqldb.sql b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/init-bar-schema-hsqldb.sql new file mode 100644 index 0000000000..86a747c96c --- /dev/null +++ b/spring-batch-infrastructure/src/test/resources/org/springframework/batch/item/database/init-bar-schema-hsqldb.sql @@ -0,0 +1,12 @@ +DROP TABLE T_BARS if exists; + +CREATE TABLE T_BARS ( + ID BIGINT NOT NULL, + NAME VARCHAR(45), + VALUE INTEGER +); + +ALTER TABLE T_BARS ADD PRIMARY KEY (ID); + +INSERT INTO t_bars (id, name, value) VALUES (1, 'baz1', 1); + From 63f920abbbbfc95906a198dfae0f473da90889f7 Mon Sep 17 00:00:00 2001 From: Bridger Howell Date: Thu, 2 Feb 2017 13:49:33 -0700 Subject: [PATCH 2/4] Use sortKeyResultNames to determine how to refer to sort keys in a ResultSet --- .../item/database/JdbcPagingItemReader.java | 6 +++- .../item/database/PagingQueryProvider.java | 9 ++++- .../AbstractSqlPagingQueryProvider.java | 36 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) 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 805e6af67a..7a329931f5 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 @@ -330,11 +330,15 @@ private class PagingRowMapper implements RowMapper { public T mapRow(ResultSet rs, int rowNum) throws SQLException { startAfterValues = new LinkedHashMap(); for (Map.Entry sortKey : queryProvider.getSortKeys().entrySet()) { - startAfterValues.put(sortKey.getKey(), rs.getObject(sortKey.getKey())); + startAfterValues.put(sortKey.getKey(), findObject(rs, sortKey.getKey())); } return rowMapper.mapRow(rs, rowNum); } + + private Object findObject(ResultSet rs, String columnName) throws SQLException{ + return rs.getObject(queryProvider.getSortKeyResultName(columnName)); + } } private JdbcTemplate getJdbcTemplate() { diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java index 91861c1eff..9000858b80 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java @@ -85,7 +85,14 @@ public interface PagingQueryProvider { * @return the sort keys used to order the query */ Map getSortKeys(); - + + /** + * Gives the name of the specified sort key, as it should be reffered in the context of a {@link java.sql.ResultSet}. + * + * @return the key's name, as it is used in a {@link java.sql.ResultSet}. + */ + String getSortKeyResultName(String key); + /** * Returns either a String to be used as the named placeholder for a sort key value (based on the column name) * or a ? for unnamed parameters. diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java index d71d6e0305..bd170535c3 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java @@ -59,6 +59,8 @@ public abstract class AbstractSqlPagingQueryProvider implements PagingQueryProvi private Map sortKeys = new LinkedHashMap(); + private Map sortKeyResultNames = new LinkedHashMap(); + private String groupClause; private int parameterCount; @@ -248,6 +250,40 @@ private String removeKeyWord(String keyWord, String clause) { } } + /** + * Set the names of sort keys as they are to be retrieved from a result set. + * + * Keys that are not included will be determined as described in {@link #getSortKeyResultName}. + */ + public void setSortKeyResultNames(Map sortKeyResultNames) { + this.sortKeyResultNames = sortKeyResultNames; + } + + /** + * Determines the name to be used for looking up the given key in a result set. + * + * First, if the key is set through {@link #setSortKeyResultNames}, the + * corresponding result will be returned. + * Second, if the key contains a table separator ('.'), then only the portion + * of the key after the table separator is returned. + * Otherwise, the key is returned as it was given. + * + * @return the name of the key, in the context of a {@link java.sql.ResultSet}. + */ + @Override + public String getSortKeyResultName(String key) { + if (sortKeyResultNames.containsKey(key)) { + return sortKeyResultNames.get(key); + } + + int tableSeparatorIndex = key.lastIndexOf('.'); + if (tableSeparatorIndex != -1) { + return key.substring(tableSeparatorIndex + 1); + } + + return key; + } + /** * * @return sortKey key to use to sort and limit page content (without alias) From 0a975e1831658a4b482adb899a3828c2f087f78e Mon Sep 17 00:00:00 2001 From: Bridger Howell Date: Thu, 9 Feb 2017 08:45:19 -0700 Subject: [PATCH 3/4] Add tests for getting queryProvider result names --- .../AbstractSqlPagingQueryProviderTests.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java index a82e5354b8..dad7cb81d1 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java @@ -103,7 +103,25 @@ public void testGenerateJumpToItemQueryForFirstPageWithMultipleSortKeys() { String s = pagingQueryProvider.generateJumpToItemQuery(45, pageSize); assertEquals(getJumpToItemQueryForFirstPageWithMultipleSortKeys(), s); } - + + @Test + public void testGetSortKeyResultNameWithoutMap() { + assertEquals("id", pagingQueryProvider.getSortKeyResultName("id")); + assertEquals("column", pagingQueryProvider.getSortKeyResultName("table.column")); + assertEquals("column", pagingQueryProvider.getSortKeyResultName("schema.table.column")); + } + + @Test + public void testGetSortKeyResultNameWithMap() { + Map sortKeyResultNames = new LinkedHashMap<>(); + sortKeyResultNames.put("foo.id", "foo_id"); + pagingQueryProvider.setSortKeyResultNames(sortKeyResultNames); + + assertEquals("foo_id", pagingQueryProvider.getSortKeyResultName("foo.id")); + assertEquals("column", pagingQueryProvider.getSortKeyResultName("column")); + assertEquals("id", pagingQueryProvider.getSortKeyResultName("bar.id")); + } + @Test public abstract void testGenerateFirstPageQuery(); From c9374c9de11f771c62adc889ffaa95bb98995f11 Mon Sep 17 00:00:00 2001 From: Bridger Howell Date: Thu, 9 Feb 2017 08:50:13 -0700 Subject: [PATCH 4/4] Update copyrights --- .../batch/item/database/JdbcPagingItemReader.java | 2 +- .../batch/item/database/PagingQueryProvider.java | 2 +- .../item/database/support/AbstractSqlPagingQueryProvider.java | 2 +- .../org/springframework/batch/item/AbstractItemReaderTests.java | 2 +- .../item/database/JdbcPagingItemReaderMultiTableTests.java | 2 +- .../database/support/AbstractSqlPagingQueryProviderTests.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) 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 7a329931f5..1a69fe4122 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-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. diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java index 9000858b80..d92c5fce25 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/PagingQueryProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2012 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. diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java index bd170535c3..65b7760872 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2012 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. diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java index c6f1c3ba9f..7ed565358d 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/AbstractItemReaderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2010 the original author or authors. + * Copyright 2009-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. diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java index 05b9f36aaa..045617d35b 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderMultiTableTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-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. diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java index dad7cb81d1..29a564b446 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/support/AbstractSqlPagingQueryProviderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2012 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.