From b2ecc544bd2f243e9ad183331e556bf6c7e69792 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 4 Sep 2023 11:29:21 +0200 Subject: [PATCH 1/2] fix-RowDocumentResultSetExtractor - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-r2dbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 3097538048..86fe10838b 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 271486f02a..738f08166e 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index d385c2fc57..913794e58d 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index a60f8e183a..f46f5fbc29 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 74f350faa8..1741efa6c2 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.2.0-SNAPSHOT + 3.2.0-1601-where-clause-SNAPSHOT From 929c124ddea8700cf2710de1e6e281496caf0a49 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 5 Sep 2023 10:41:01 +0200 Subject: [PATCH 2/2] Fix RowDocumentIterator's hasNext implementation. This properly fixes the test setup, taking into account that @ActiveProfile by default replaces any profiles set via environment variable or system property. It turns out that the hasNext calculation in RowDocumentIterator was wrong because a) isBeforeFirst and isBeforeLast return both false when the ResultSet is empty. b) isBeforeFirst and isBeforeLast aren't necessarily implemented for all ResultSets and for example DB2s ResultSet implementation don't support it by default. --- .../RowDocumentResultSetExtractor.java | 80 ++++++++++++++++--- ...JdbcAggregateTemplateIntegrationTests.java | 10 ++- .../jdbc/core/convert/ResultSetTestUtil.java | 2 +- .../CombiningActiveProfileResolver.java | 76 ++++++++++++++++++ .../jdbc/testing/DataSourceConfiguration.java | 17 +++- ...egateTemplateIntegrationTests-postgres.sql | 29 +++++-- 6 files changed, 193 insertions(+), 21 deletions(-) create mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/CombiningActiveProfileResolver.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java index 2f558c71ac..db9afbdf11 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java @@ -38,6 +38,7 @@ * {@link ResultSet}-driven extractor to extract {@link RowDocument documents}. * * @author Mark Paluch + * @author Jens Schauder * @since 3.2 */ class RowDocumentResultSetExtractor { @@ -152,18 +153,18 @@ private class RowDocumentIterator implements Iterator { private final RelationalPersistentEntity rootEntity; private final Integer identifierIndex; private final AggregateContext aggregateContext; - private boolean hasNext; + + /** + * Answers the question if the internal {@link ResultSet} points at an actual row. Since when not currently + * extracting a document the {@link ResultSet} points at the next row to be read (or behind all rows), this is + * equivalent to {@literal hasNext()} from the outside. + */ + private boolean pointsAtRow; RowDocumentIterator(RelationalPersistentEntity entity, ResultSet resultSet) throws SQLException { ResultSetAdapter adapter = ResultSetAdapter.INSTANCE; - if (resultSet.isBeforeFirst()) { - hasNext = resultSet.next(); - } else { - hasNext = !resultSet.isAfterLast(); - } - this.rootPath = context.getAggregatePath(entity); this.rootEntity = entity; @@ -173,11 +174,70 @@ private class RowDocumentIterator implements Iterator { this.resultSet = resultSet; this.identifierIndex = columns.get(idColumn); + + pointsAtRow = pointAtInitialRow(); + } + + private boolean pointAtInitialRow() throws SQLException { + + // If we are before the first row we need to advance to the first row. + try { + if (resultSet.isBeforeFirst()) { + return resultSet.next(); + } + } catch (SQLException e) { + // seems that isBeforeFirst is not implemented + } + + // if we are after the last row we are done and not pointing a valid row and also can't advance to one. + try { + if (resultSet.isAfterLast()) { + return false; + } + } catch (SQLException e) { + // seems that isAfterLast is not implemented + } + + // if we arrived here we know almost nothing. + // maybe isBeforeFirst or isBeforeLast aren't implemented + // or the ResultSet is empty. + + + boolean peek = peek(resultSet); + if (peek) { + // we can see actual data, so we are looking at a current row. + return true; + } + + + try { + return resultSet.next(); + } catch (SQLException e) { + // we aren't looking at a row, but we can't advance either. + // so it seems we are facing an empty ResultSet + return false; + } + } + + /** + * Tries ot access values of the passed in {@link ResultSet} in order to check if it is pointing at an actual row. + * + * @param resultSet to check. + * @return true if values of the {@literal ResultSet} can be accessed and it therefore points to an actual row. + */ + private boolean peek(ResultSet resultSet) { + + try { + resultSet.getObject(1); + return true; + } catch (SQLException e) { + return false; + } } @Override public boolean hasNext() { - return hasNext; + return pointsAtRow; } @Override @@ -197,8 +257,8 @@ public RowDocument next() { } reader.accept(resultSet); - hasNext = resultSet.next(); - } while (hasNext); + pointsAtRow = resultSet.next(); + } while (pointsAtRow); } catch (SQLException e) { throw new DataRetrievalFailureException("Cannot advance ResultSet", e); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java index 8c2e2324c1..6c69b476ad 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java @@ -56,6 +56,7 @@ import org.springframework.data.jdbc.core.convert.DataAccessStrategy; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.jdbc.testing.AssumeFeatureTestExecutionListener; +import org.springframework.data.jdbc.testing.CombiningActiveProfileResolver; import org.springframework.data.jdbc.testing.EnabledOnFeature; import org.springframework.data.jdbc.testing.TestConfiguration; import org.springframework.data.jdbc.testing.TestDatabaseFeatures; @@ -918,7 +919,7 @@ void readOnlyGetsLoadedButNotWritten() { assertThat( jdbcTemplate.queryForObject("SELECT read_only FROM with_read_only", Collections.emptyMap(), String.class)) - .isEqualTo("from-db"); + .isEqualTo("from-db"); } @Test @@ -1873,10 +1874,11 @@ JdbcAggregateOperations operations(ApplicationEventPublisher publisher, Relation } } - static class JdbcAggregateTemplateIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests { } + static class JdbcAggregateTemplateIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests {} - @ActiveProfiles(PROFILE_SINGLE_QUERY_LOADING) - static class JdbcAggregateTemplateSingleQueryLoadingIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests { + @ActiveProfiles(value = PROFILE_SINGLE_QUERY_LOADING, resolver = CombiningActiveProfileResolver.class) + static class JdbcAggregateTemplateSingleQueryLoadingIntegrationTests + extends AbstractJdbcAggregateTemplateIntegrationTests { } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetTestUtil.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetTestUtil.java index 736d300270..370a680fb4 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetTestUtil.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetTestUtil.java @@ -136,7 +136,7 @@ private boolean isAfterLast() { } private boolean isBeforeFirst() { - return index < 0; + return index < 0 && !values.isEmpty(); } private Object getObject(String column) throws SQLException { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/CombiningActiveProfileResolver.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/CombiningActiveProfileResolver.java new file mode 100644 index 0000000000..dcafe9bb2a --- /dev/null +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/CombiningActiveProfileResolver.java @@ -0,0 +1,76 @@ +/* + * Copyright 2023 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.data.jdbc.testing; + +import java.util.ArrayList; + +import org.jetbrains.annotations.NotNull; +import org.springframework.test.context.ActiveProfilesResolver; +import org.springframework.test.context.support.DefaultActiveProfilesResolver; + +/** + * A {@link ActiveProfilesResolver} combining the profile configurations from environement, system properties and + * {@link org.springframework.test.context.ActiveProfiles} annotations. + * + * @author Jens Schauder + */ +public class CombiningActiveProfileResolver implements ActiveProfilesResolver { + + private static final String SPRING_PROFILES_ACTIVE = "spring.profiles.active"; + private final DefaultActiveProfilesResolver defaultActiveProfilesResolver = new DefaultActiveProfilesResolver(); + + @Override + public String[] resolve(Class testClass) { + + ArrayList combinedProfiles = new ArrayList<>(); + + for (String profile : defaultActiveProfilesResolver.resolve(testClass)) { + combinedProfiles.add(profile); + } + for (String profile : getSystemProfiles()) { + combinedProfiles.add(profile); + } + for (String profile : getEnvironmentProfiles()) { + combinedProfiles.add(profile); + } + + return combinedProfiles.toArray(new String[0]); + } + + @NotNull + private static String[] getSystemProfiles() { + + if (System.getProperties().containsKey(SPRING_PROFILES_ACTIVE)) { + + final String profiles = System.getProperty(SPRING_PROFILES_ACTIVE); + return profiles.split("\\s*,\\s*"); + } + return new String[0]; + } + + private String[] getEnvironmentProfiles() { + + if (System.getenv().containsKey(SPRING_PROFILES_ACTIVE)) { + + String profiles = System.getenv().get(SPRING_PROFILES_ACTIVE); + return profiles.split("\\s*,\\s*"); + } + return new String[0]; + + } + +} diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DataSourceConfiguration.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DataSourceConfiguration.java index ad0e5b0c2a..d8b2ee290f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DataSourceConfiguration.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DataSourceConfiguration.java @@ -18,6 +18,9 @@ import static org.awaitility.pollinterval.FibonacciPollInterval.*; import java.sql.Connection; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.concurrent.TimeUnit; import javax.sql.DataSource; @@ -64,7 +67,7 @@ DataSourceInitializer initializer() { initializer.setDataSource(dataSource()); String[] activeProfiles = environment.getActiveProfiles(); - String profile = activeProfiles.length == 0 ? "" : activeProfiles[0]; + String profile = getDatabaseProfile(activeProfiles); ClassPathResource script = new ClassPathResource(TestUtils.createScriptName(testClass, profile)); ResourceDatabasePopulator populator = new ResourceDatabasePopulator(script); @@ -74,6 +77,18 @@ DataSourceInitializer initializer() { return initializer; } + private static String getDatabaseProfile(String[] activeProfiles) { + + List validDbs = Arrays.asList("hsql", "h2", "mysql", "mariadb", "postgres", "db2", "oracle", "mssql"); + for (String profile : activeProfiles) { + if (validDbs.contains(profile)) { + return profile; + } + } + + return ""; + } + /** * Return the {@link DataSource} to be exposed as a Spring bean. * diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql index ed1fb9662d..0c77c88139 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql @@ -4,14 +4,33 @@ DROP TABLE ONE_TO_ONE_PARENT; DROP TABLE Child_No_Id; DROP TABLE element_no_id; DROP TABLE "LIST_PARENT"; -DROP TABLE ARRAY_OWNER; +DROP TABLE "ARRAY_OWNER"; +DROP TABLE DOUBLE_LIST_OWNER; +DROP TABLE FLOAT_LIST_OWNER; DROP TABLE BYTE_ARRAY_OWNER; -DROP TABLE CHAIN4; -DROP TABLE CHAIN3; -DROP TABLE CHAIN2; -DROP TABLE CHAIN1; DROP TABLE CHAIN0; +DROP TABLE CHAIN1; +DROP TABLE CHAIN2; +DROP TABLE CHAIN3; +DROP TABLE CHAIN4; +DROP TABLE NO_ID_CHAIN0; +DROP TABLE NO_ID_CHAIN1; +DROP TABLE NO_ID_CHAIN2; +DROP TABLE NO_ID_CHAIN3; +DROP TABLE NO_ID_CHAIN4; +DROP TABLE NO_ID_LIST_CHAIN0; +DROP TABLE NO_ID_LIST_CHAIN1; +DROP TABLE NO_ID_LIST_CHAIN2; +DROP TABLE NO_ID_LIST_CHAIN3; +DROP TABLE NO_ID_LIST_CHAIN4; +DROP TABLE NO_ID_MAP_CHAIN0; +DROP TABLE NO_ID_MAP_CHAIN1; +DROP TABLE NO_ID_MAP_CHAIN2; +DROP TABLE NO_ID_MAP_CHAIN3; +DROP TABLE NO_ID_MAP_CHAIN4; +DROP TABLE "VERSIONED_AGGREGATE"; DROP TABLE WITH_READ_ONLY; +DROP TABLE WITH_LOCAL_DATE_TIME; DROP TABLE WITH_ID_ONLY; DROP TABLE WITH_INSERT_ONLY;