Skip to content

Commit 5d6ce18

Browse files
committed
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.
1 parent 41f72ca commit 5d6ce18

File tree

6 files changed

+193
-21
lines changed

6 files changed

+193
-21
lines changed

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractor.java

+70-10
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
* {@link ResultSet}-driven extractor to extract {@link RowDocument documents}.
3939
*
4040
* @author Mark Paluch
41+
* @author Jens Schauder
4142
* @since 3.2
4243
*/
4344
class RowDocumentResultSetExtractor {
@@ -152,18 +153,18 @@ private class RowDocumentIterator implements Iterator<RowDocument> {
152153
private final RelationalPersistentEntity<?> rootEntity;
153154
private final Integer identifierIndex;
154155
private final AggregateContext<ResultSet> aggregateContext;
155-
private boolean hasNext;
156+
157+
/**
158+
* Answers the question if the internal {@link ResultSet} points at an actual row. Since when not currently
159+
* extracting a document the {@link ResultSet} points at the next row to be read (or behind all rows), this is
160+
* equivalent to {@literal hasNext()} from the outside.
161+
*/
162+
private boolean pointsAtRow;
156163

157164
RowDocumentIterator(RelationalPersistentEntity<?> entity, ResultSet resultSet) throws SQLException {
158165

159166
ResultSetAdapter adapter = ResultSetAdapter.INSTANCE;
160167

161-
if (resultSet.isBeforeFirst()) {
162-
hasNext = resultSet.next();
163-
} else {
164-
hasNext = !resultSet.isAfterLast();
165-
}
166-
167168
this.rootPath = context.getAggregatePath(entity);
168169
this.rootEntity = entity;
169170

@@ -173,11 +174,70 @@ private class RowDocumentIterator implements Iterator<RowDocument> {
173174

174175
this.resultSet = resultSet;
175176
this.identifierIndex = columns.get(idColumn);
177+
178+
pointsAtRow = pointAtInitialRow();
179+
}
180+
181+
private boolean pointAtInitialRow() throws SQLException {
182+
183+
// If we are before the first row we need to advance to the first row.
184+
try {
185+
if (resultSet.isBeforeFirst()) {
186+
return resultSet.next();
187+
}
188+
} catch (SQLException e) {
189+
// seems that isBeforeFirst is not implemented
190+
}
191+
192+
// if we are after the last row we are done and not pointing a valid row and also can't advance to one.
193+
try {
194+
if (resultSet.isAfterLast()) {
195+
return false;
196+
}
197+
} catch (SQLException e) {
198+
// seems that isAfterLast is not implemented
199+
}
200+
201+
// if we arrived here we know almost nothing.
202+
// maybe isBeforeFirst or isBeforeLast aren't implemented
203+
// or the ResultSet is empty.
204+
205+
206+
boolean peek = peek(resultSet);
207+
if (peek) {
208+
// we can see actual data, so we are looking at a current row.
209+
return true;
210+
}
211+
212+
213+
try {
214+
return resultSet.next();
215+
} catch (SQLException e) {
216+
// we aren't looking at a row, but we can't advance either.
217+
// so it seems we are facing an empty ResultSet
218+
return false;
219+
}
220+
}
221+
222+
/**
223+
* Tries ot access values of the passed in {@link ResultSet} in order to check if it is pointing at an actual row.
224+
*
225+
* @param resultSet to check.
226+
* @return true if values of the {@literal ResultSet} can be accessed and it therefore points to an actual row.
227+
*/
228+
private boolean peek(ResultSet resultSet) {
229+
230+
try {
231+
resultSet.getObject(1);
232+
return true;
233+
} catch (SQLException e) {
234+
return false;
235+
}
176236
}
177237

178238
@Override
179239
public boolean hasNext() {
180-
return hasNext;
240+
return pointsAtRow;
181241
}
182242

183243
@Override
@@ -197,8 +257,8 @@ public RowDocument next() {
197257
}
198258

199259
reader.accept(resultSet);
200-
hasNext = resultSet.next();
201-
} while (hasNext);
260+
pointsAtRow = resultSet.next();
261+
} while (pointsAtRow);
202262
} catch (SQLException e) {
203263
throw new DataRetrievalFailureException("Cannot advance ResultSet", e);
204264
}

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import org.springframework.data.jdbc.core.convert.DataAccessStrategy;
5757
import org.springframework.data.jdbc.core.convert.JdbcConverter;
5858
import org.springframework.data.jdbc.testing.AssumeFeatureTestExecutionListener;
59+
import org.springframework.data.jdbc.testing.CombiningActiveProfileResolver;
5960
import org.springframework.data.jdbc.testing.EnabledOnFeature;
6061
import org.springframework.data.jdbc.testing.TestConfiguration;
6162
import org.springframework.data.jdbc.testing.TestDatabaseFeatures;
@@ -918,7 +919,7 @@ void readOnlyGetsLoadedButNotWritten() {
918919

919920
assertThat(
920921
jdbcTemplate.queryForObject("SELECT read_only FROM with_read_only", Collections.emptyMap(), String.class))
921-
.isEqualTo("from-db");
922+
.isEqualTo("from-db");
922923
}
923924

924925
@Test
@@ -1873,10 +1874,11 @@ JdbcAggregateOperations operations(ApplicationEventPublisher publisher, Relation
18731874
}
18741875
}
18751876

1876-
static class JdbcAggregateTemplateIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests { }
1877+
static class JdbcAggregateTemplateIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests {}
18771878

1878-
@ActiveProfiles(PROFILE_SINGLE_QUERY_LOADING)
1879-
static class JdbcAggregateTemplateSingleQueryLoadingIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests {
1879+
@ActiveProfiles(value = PROFILE_SINGLE_QUERY_LOADING, resolver = CombiningActiveProfileResolver.class)
1880+
static class JdbcAggregateTemplateSingleQueryLoadingIntegrationTests
1881+
extends AbstractJdbcAggregateTemplateIntegrationTests {
18801882

18811883
}
18821884
}

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/ResultSetTestUtil.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ private boolean isAfterLast() {
136136
}
137137

138138
private boolean isBeforeFirst() {
139-
return index < 0;
139+
return index < 0 && !values.isEmpty();
140140
}
141141

142142
private Object getObject(String column) throws SQLException {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.data.jdbc.testing;
18+
19+
import java.util.ArrayList;
20+
21+
import org.jetbrains.annotations.NotNull;
22+
import org.springframework.test.context.ActiveProfilesResolver;
23+
import org.springframework.test.context.support.DefaultActiveProfilesResolver;
24+
25+
/**
26+
* A {@link ActiveProfilesResolver} combining the profile configurations from environement, system properties and
27+
* {@link org.springframework.test.context.ActiveProfiles} annotations.
28+
*
29+
* @author Jens Schauder
30+
*/
31+
public class CombiningActiveProfileResolver implements ActiveProfilesResolver {
32+
33+
private static final String SPRING_PROFILES_ACTIVE = "spring.profiles.active";
34+
private final DefaultActiveProfilesResolver defaultActiveProfilesResolver = new DefaultActiveProfilesResolver();
35+
36+
@Override
37+
public String[] resolve(Class<?> testClass) {
38+
39+
ArrayList<Object> combinedProfiles = new ArrayList<>();
40+
41+
for (String profile : defaultActiveProfilesResolver.resolve(testClass)) {
42+
combinedProfiles.add(profile);
43+
}
44+
for (String profile : getSystemProfiles()) {
45+
combinedProfiles.add(profile);
46+
}
47+
for (String profile : getEnvironmentProfiles()) {
48+
combinedProfiles.add(profile);
49+
}
50+
51+
return combinedProfiles.toArray(new String[0]);
52+
}
53+
54+
@NotNull
55+
private static String[] getSystemProfiles() {
56+
57+
if (System.getProperties().containsKey(SPRING_PROFILES_ACTIVE)) {
58+
59+
final String profiles = System.getProperty(SPRING_PROFILES_ACTIVE);
60+
return profiles.split("\\s*,\\s*");
61+
}
62+
return new String[0];
63+
}
64+
65+
private String[] getEnvironmentProfiles() {
66+
67+
if (System.getenv().containsKey(SPRING_PROFILES_ACTIVE)) {
68+
69+
String profiles = System.getenv().get(SPRING_PROFILES_ACTIVE);
70+
return profiles.split("\\s*,\\s*");
71+
}
72+
return new String[0];
73+
74+
}
75+
76+
}

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DataSourceConfiguration.java

+16-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
import static org.awaitility.pollinterval.FibonacciPollInterval.*;
1919

2020
import java.sql.Connection;
21+
import java.util.Arrays;
22+
import java.util.Collections;
23+
import java.util.List;
2124
import java.util.concurrent.TimeUnit;
2225

2326
import javax.sql.DataSource;
@@ -64,7 +67,7 @@ DataSourceInitializer initializer() {
6467
initializer.setDataSource(dataSource());
6568

6669
String[] activeProfiles = environment.getActiveProfiles();
67-
String profile = activeProfiles.length == 0 ? "" : activeProfiles[0];
70+
String profile = getDatabaseProfile(activeProfiles);
6871

6972
ClassPathResource script = new ClassPathResource(TestUtils.createScriptName(testClass, profile));
7073
ResourceDatabasePopulator populator = new ResourceDatabasePopulator(script);
@@ -74,6 +77,18 @@ DataSourceInitializer initializer() {
7477
return initializer;
7578
}
7679

80+
private static String getDatabaseProfile(String[] activeProfiles) {
81+
82+
List<String> validDbs = Arrays.asList("hsql", "h2", "mysql", "mariadb", "postgres", "db2", "oracle", "mssql");
83+
for (String profile : activeProfiles) {
84+
if (validDbs.contains(profile)) {
85+
return profile;
86+
}
87+
}
88+
89+
return "";
90+
}
91+
7792
/**
7893
* Return the {@link DataSource} to be exposed as a Spring bean.
7994
*

Diff for: spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql

+24-5
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,33 @@ DROP TABLE ONE_TO_ONE_PARENT;
44
DROP TABLE Child_No_Id;
55
DROP TABLE element_no_id;
66
DROP TABLE "LIST_PARENT";
7-
DROP TABLE ARRAY_OWNER;
7+
DROP TABLE "ARRAY_OWNER";
8+
DROP TABLE DOUBLE_LIST_OWNER;
9+
DROP TABLE FLOAT_LIST_OWNER;
810
DROP TABLE BYTE_ARRAY_OWNER;
9-
DROP TABLE CHAIN4;
10-
DROP TABLE CHAIN3;
11-
DROP TABLE CHAIN2;
12-
DROP TABLE CHAIN1;
1311
DROP TABLE CHAIN0;
12+
DROP TABLE CHAIN1;
13+
DROP TABLE CHAIN2;
14+
DROP TABLE CHAIN3;
15+
DROP TABLE CHAIN4;
16+
DROP TABLE NO_ID_CHAIN0;
17+
DROP TABLE NO_ID_CHAIN1;
18+
DROP TABLE NO_ID_CHAIN2;
19+
DROP TABLE NO_ID_CHAIN3;
20+
DROP TABLE NO_ID_CHAIN4;
21+
DROP TABLE NO_ID_LIST_CHAIN0;
22+
DROP TABLE NO_ID_LIST_CHAIN1;
23+
DROP TABLE NO_ID_LIST_CHAIN2;
24+
DROP TABLE NO_ID_LIST_CHAIN3;
25+
DROP TABLE NO_ID_LIST_CHAIN4;
26+
DROP TABLE NO_ID_MAP_CHAIN0;
27+
DROP TABLE NO_ID_MAP_CHAIN1;
28+
DROP TABLE NO_ID_MAP_CHAIN2;
29+
DROP TABLE NO_ID_MAP_CHAIN3;
30+
DROP TABLE NO_ID_MAP_CHAIN4;
31+
DROP TABLE "VERSIONED_AGGREGATE";
1432
DROP TABLE WITH_READ_ONLY;
33+
DROP TABLE WITH_LOCAL_DATE_TIME;
1534
DROP TABLE WITH_ID_ONLY;
1635
DROP TABLE WITH_INSERT_ONLY;
1736

0 commit comments

Comments
 (0)