Skip to content

Commit f3bc0af

Browse files
schaudermp911de
schauder
authored and
mp911de
committed
Polishing.
Simplify ValueFunction mapping. Remove invariants of findBy SQL generation in favor of the Condition-based variant. Reduce visibility. Change return value of AggregateReader to List See #1601 Original pull request: #1617
1 parent 0fdeaeb commit f3bc0af

File tree

15 files changed

+109
-216
lines changed

15 files changed

+109
-216
lines changed

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java

+39-56
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@
1818
import java.sql.ResultSet;
1919
import java.sql.SQLException;
2020
import java.util.ArrayList;
21+
import java.util.Collection;
2122
import java.util.Iterator;
2223
import java.util.List;
23-
import java.util.Map;
2424
import java.util.Optional;
25-
import java.util.function.BiFunction;
2625

2726
import org.springframework.dao.IncorrectResultSizeDataAccessException;
2827
import org.springframework.data.relational.core.dialect.Dialect;
2928
import org.springframework.data.relational.core.mapping.AggregatePath;
3029
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
30+
import org.springframework.data.relational.core.query.Criteria;
3131
import org.springframework.data.relational.core.query.CriteriaDefinition;
3232
import org.springframework.data.relational.core.query.Query;
3333
import org.springframework.data.relational.core.sql.Condition;
@@ -36,14 +36,15 @@
3636
import org.springframework.data.relational.core.sqlgeneration.SingleQuerySqlGenerator;
3737
import org.springframework.data.relational.core.sqlgeneration.SqlGenerator;
3838
import org.springframework.data.relational.domain.RowDocument;
39+
import org.springframework.data.util.Streamable;
3940
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
4041
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
4142
import org.springframework.lang.Nullable;
4243
import org.springframework.util.Assert;
4344

4445
/**
4546
* Reads complete Aggregates from the database, by generating appropriate SQL using a {@link SingleQuerySqlGenerator}
46-
* through {@link org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate}. Results are converterd into an
47+
* through {@link org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate}. Results are converted into an
4748
* intermediate {@link RowDocumentResultSetExtractor RowDocument} and mapped via
4849
* {@link org.springframework.data.relational.core.conversion.RelationalConverter#read(Class, RowDocument)}.
4950
*
@@ -55,7 +56,8 @@
5556
class AggregateReader<T> {
5657

5758
private final RelationalPersistentEntity<T> aggregate;
58-
private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator sqlGenerator;
59+
private final Table table;
60+
private final SqlGenerator sqlGenerator;
5961
private final JdbcConverter converter;
6062
private final NamedParameterJdbcOperations jdbcTemplate;
6163
private final RowDocumentResultSetExtractor extractor;
@@ -66,6 +68,7 @@ class AggregateReader<T> {
6668
this.converter = converter;
6769
this.aggregate = aggregate;
6870
this.jdbcTemplate = jdbcTemplate;
71+
this.table = Table.create(aggregate.getQualifiedTableName());
6972

7073
this.sqlGenerator = new CachingSqlGenerator(
7174
new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, aggregate));
@@ -74,62 +77,58 @@ class AggregateReader<T> {
7477
createPathToColumnMapping(aliasFactory));
7578
}
7679

77-
public List<T> findAll() {
78-
return jdbcTemplate.query(sqlGenerator.findAll(), this::extractAll);
79-
}
80-
8180
@Nullable
8281
public T findById(Object id) {
8382

84-
id = converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation());
83+
Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).is(id)).limit(1);
8584

86-
return jdbcTemplate.query(sqlGenerator.findById(), Map.of("id", id), this::extractZeroOrOne);
85+
return findOne(query);
8786
}
8887

89-
public Iterable<T> findAllById(Iterable<?> ids) {
88+
@Nullable
89+
public T findOne(Query query) {
9090

91-
List<Object> convertedIds = new ArrayList<>();
92-
for (Object id : ids) {
93-
convertedIds.add(converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation()));
94-
}
91+
MapSqlParameterSource parameterSource = new MapSqlParameterSource();
92+
Condition condition = createCondition(query, parameterSource);
9593

96-
return jdbcTemplate.query(sqlGenerator.findAllById(), Map.of("ids", convertedIds), this::extractAll);
94+
return jdbcTemplate.query(sqlGenerator.findAll(condition), parameterSource, this::extractZeroOrOne);
9795
}
9896

99-
public Iterable<T> findAllBy(Query query) {
97+
public List<T> findAll() {
98+
return jdbcTemplate.query(sqlGenerator.findAll(), this::extractAll);
99+
}
100100

101-
MapSqlParameterSource parameterSource = new MapSqlParameterSource();
102-
BiFunction<Table, RelationalPersistentEntity, Condition> condition = createConditionSource(query, parameterSource);
103-
return jdbcTemplate.query(sqlGenerator.findAllByCondition(condition), parameterSource, this::extractAll);
101+
public List<T> findAllById(Iterable<?> ids) {
102+
103+
Collection<?> identifiers = ids instanceof Collection<?> idl ? idl : Streamable.of(ids).toList();
104+
Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).in(identifiers)).limit(1);
105+
106+
return findAll(query);
104107
}
105108

106-
public Optional<T> findOneByQuery(Query query) {
107-
108-
MapSqlParameterSource parameterSource = new MapSqlParameterSource();
109-
BiFunction<Table, RelationalPersistentEntity, Condition> condition = createConditionSource(query, parameterSource);
109+
public List<T> findAll(Query query) {
110110

111-
return Optional.ofNullable(
112-
jdbcTemplate.query(sqlGenerator.findAllByCondition(condition), parameterSource, this::extractZeroOrOne));
111+
MapSqlParameterSource parameterSource = new MapSqlParameterSource();
112+
Condition condition = createCondition(query, parameterSource);
113+
return jdbcTemplate.query(sqlGenerator.findAll(condition), parameterSource, this::extractAll);
113114
}
114115

115-
private BiFunction<Table, RelationalPersistentEntity, Condition> createConditionSource(Query query, MapSqlParameterSource parameterSource) {
116+
@Nullable
117+
private Condition createCondition(Query query, MapSqlParameterSource parameterSource) {
116118

117119
QueryMapper queryMapper = new QueryMapper(converter);
118120

119-
BiFunction<Table, RelationalPersistentEntity, Condition> condition = (table, aggregate) -> {
120-
Optional<CriteriaDefinition> criteria = query.getCriteria();
121-
return criteria
122-
.map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition, table, aggregate))
123-
.orElse(null);
124-
};
125-
return condition;
121+
Optional<CriteriaDefinition> criteria = query.getCriteria();
122+
return criteria
123+
.map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition, table, aggregate))
124+
.orElse(null);
126125
}
127126

128127
/**
129128
* Extracts a list of aggregates from the given {@link ResultSet} by utilizing the
130129
* {@link RowDocumentResultSetExtractor} and the {@link JdbcConverter}. When used as a method reference this conforms
131130
* to the {@link org.springframework.jdbc.core.ResultSetExtractor} contract.
132-
*
131+
*
133132
* @param rs the {@link ResultSet} from which to extract the data. Must not be {(}@literal null}.
134133
* @return a {@code List} of aggregates, fully converted.
135134
* @throws SQLException
@@ -195,21 +194,15 @@ public String keyColumn(AggregatePath path) {
195194
* @author Jens Schauder
196195
* @since 3.2
197196
*/
198-
static class CachingSqlGenerator implements org.springframework.data.relational.core.sqlgeneration.SqlGenerator {
199-
200-
private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator delegate;
197+
static class CachingSqlGenerator implements SqlGenerator {
201198

199+
private final SqlGenerator delegate;
202200
private final String findAll;
203-
private final String findById;
204-
private final String findAllById;
205201

206202
public CachingSqlGenerator(SqlGenerator delegate) {
207203

208204
this.delegate = delegate;
209-
210-
findAll = delegate.findAll();
211-
findById = delegate.findById();
212-
findAllById = delegate.findAllById();
205+
this.findAll = delegate.findAll();
213206
}
214207

215208
@Override
@@ -218,18 +211,8 @@ public String findAll() {
218211
}
219212

220213
@Override
221-
public String findById() {
222-
return findById;
223-
}
224-
225-
@Override
226-
public String findAllById() {
227-
return findAllById;
228-
}
229-
230-
@Override
231-
public String findAllByCondition(BiFunction<Table, RelationalPersistentEntity, Condition> conditionSource) {
232-
return delegate.findAllByCondition(conditionSource);
214+
public String findAll(@Nullable Condition condition) {
215+
return delegate.findAll(condition);
233216
}
234217

235218
@Override

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java

+2-5
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import java.util.HashMap;
2323
import java.util.List;
2424
import java.util.Map;
25-
import java.util.Objects;
26-
import java.util.function.Function;
2725

2826
import org.springframework.data.domain.Sort;
2927
import org.springframework.data.jdbc.core.mapping.JdbcValue;
@@ -35,7 +33,6 @@
3533
import org.springframework.data.mapping.context.InvalidPersistentPropertyPath;
3634
import org.springframework.data.mapping.context.MappingContext;
3735
import org.springframework.data.relational.core.dialect.Dialect;
38-
import org.springframework.data.relational.core.dialect.Escaper;
3936
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
4037
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
4138
import org.springframework.data.relational.core.query.CriteriaDefinition;
@@ -77,7 +74,7 @@ public QueryMapper(Dialect dialect, JdbcConverter converter) {
7774
Assert.notNull(converter, "JdbcConverter must not be null");
7875

7976
this.converter = converter;
80-
this.mappingContext = (MappingContext) converter.getMappingContext();
77+
this.mappingContext = converter.getMappingContext();
8178
}
8279

8380
/**
@@ -310,7 +307,7 @@ private Condition mapCondition(CriteriaDefinition criteria, MapSqlParameterSourc
310307
sqlType = getTypeHint(mappedValue, actualType.getType(), settableValue);
311308
} else if (criteria.getValue() instanceof ValueFunction valueFunction) {
312309

313-
mappedValue = valueFunction.transform(v -> convertValue(comparator, v, propertyField.getTypeHint()));
310+
mappedValue = valueFunction.map(v -> convertValue(comparator, v, propertyField.getTypeHint()));
314311
sqlType = propertyField.getSqlType();
315312

316313
} else if (propertyField instanceof MetadataBackedField metadataBackedField //

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryDataAccessStrategy.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,12 @@ public <T> Iterable<T> findAll(Class<T> domainType, Pageable pageable) {
7777

7878
@Override
7979
public <T> Optional<T> findOne(Query query, Class<T> domainType) {
80-
return getReader(domainType).findOneByQuery(query);
80+
return Optional.ofNullable(getReader(domainType).findOne(query));
8181
}
8282

8383
@Override
8484
public <T> Iterable<T> findAll(Query query, Class<T> domainType) {
85-
86-
return getReader(domainType).findAllBy(query);
85+
return getReader(domainType).findAll(query);
8786
}
8887

8988
@Override

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryFallbackDataAccessStrategy.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
8787
return super.findAllById(ids, domainType);
8888
}
8989

90+
@Override
9091
public <T> Optional<T> findOne(Query query, Class<T> domainType) {
9192

9293
if (isSingleSelectQuerySupported(domainType) && isSingleSelectQuerySupported(query)) {
@@ -137,11 +138,6 @@ private boolean entityQualifiesForSingleQueryLoading(Class<?> entityType) {
137138

138139
referenceFound = true;
139140
}
140-
141-
// AggregateReferences aren't supported yet
142-
// if (property.isAssociation()) {
143-
// return false;
144-
// }
145141
}
146142
return true;
147143

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/EscapingParameterSource.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,13 @@
2121
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
2222

2323
/**
24-
* This {@link SqlParameterSource} will apply escaping to it's values.
25-
*
24+
* This {@link SqlParameterSource} will apply escaping to its values.
25+
*
2626
* @author Jens Schauder
2727
* @since 3.2
2828
*/
29-
public class EscapingParameterSource implements SqlParameterSource {
29+
class EscapingParameterSource implements SqlParameterSource {
30+
3031
private final SqlParameterSource parameterSource;
3132
private final Escaper escaper;
3233

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/ParametrizedQuery.java

+6-7
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
*/
1616
package org.springframework.data.jdbc.repository.query;
1717

18-
import org.springframework.data.relational.core.dialect.Dialect;
1918
import org.springframework.data.relational.core.dialect.Escaper;
2019
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
2120

2221
/**
23-
* Value object encapsulating a query containing named parameters and a{@link SqlParameterSource} to bind the parameters.
22+
* Value object encapsulating a query containing named parameters and a{@link SqlParameterSource} to bind the
23+
* parameters.
2424
*
2525
* @author Mark Paluch
2626
* @author Jens Schauder
@@ -41,13 +41,12 @@ String getQuery() {
4141
return query;
4242
}
4343

44+
SqlParameterSource getParameterSource(Escaper escaper) {
45+
return new EscapingParameterSource(parameterSource, escaper);
46+
}
47+
4448
@Override
4549
public String toString() {
4650
return this.query;
4751
}
48-
49-
public SqlParameterSource getParameterSource(Escaper escaper) {
50-
51-
return new EscapingParameterSource(parameterSource, escaper);
52-
}
5352
}

‎spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java

+5-9
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,16 @@ public Expression getMappedObject(Expression expression, @Nullable RelationalPer
176176
return expression;
177177
}
178178

179-
if (expression instanceof Column) {
179+
if (expression instanceof Column column) {
180180

181-
Column column = (Column) expression;
182181
Field field = createPropertyField(entity, column.getName());
183182
TableLike table = column.getTable();
184183

185184
Column columnFromTable = table.column(field.getMappedColumnName());
186185
return column instanceof Aliased ? columnFromTable.as(((Aliased) column).getAlias()) : columnFromTable;
187186
}
188187

189-
if (expression instanceof SimpleFunction) {
190-
191-
SimpleFunction function = (SimpleFunction) expression;
188+
if (expression instanceof SimpleFunction function) {
192189

193190
List<Expression> arguments = function.getExpressions();
194191
List<Expression> mappedArguments = new ArrayList<>(arguments.size());
@@ -367,15 +364,14 @@ private Condition mapCondition(CriteriaDefinition criteria, MutableBindings bind
367364
Class<?> typeHint;
368365

369366
Comparator comparator = criteria.getComparator();
370-
if (criteria.getValue() instanceof Parameter) {
371-
372-
Parameter parameter = (Parameter) criteria.getValue();
367+
if (criteria.getValue()instanceof Parameter parameter) {
373368

374369
mappedValue = convertValue(comparator, parameter.getValue(), propertyField.getTypeHint());
375370
typeHint = getTypeHint(mappedValue, actualType.getType(), parameter);
376371
} else if (criteria.getValue() instanceof ValueFunction<?> valueFunction) {
377372

378-
mappedValue = valueFunction.transform(v -> convertValue(comparator, v, propertyField.getTypeHint())).apply(getEscaper(comparator));
373+
mappedValue = valueFunction.map(v -> convertValue(comparator, v, propertyField.getTypeHint()))
374+
.apply(getEscaper(comparator));
379375

380376
typeHint = actualType.getType();
381377
} else {

‎spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/UpdateMapper.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private Assignment getAssignment(SqlIdentifier columnName, Object value, Mutable
118118

119119
} else if (value instanceof ValueFunction<?> valueFunction) {
120120

121-
mappedValue = valueFunction.transform(v -> convertValue(v, propertyField.getTypeHint())).apply(Escaper.DEFAULT);
121+
mappedValue = valueFunction.map(v -> convertValue(v, propertyField.getTypeHint())).apply(Escaper.DEFAULT);
122122

123123
if (mappedValue == null) {
124124
return Assignments.value(column, SQL.nullLiteral());

‎spring-data-relational/src/main/java/org/springframework/data/relational/core/query/ValueFunction.java

+10-6
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,18 @@ default Supplier<T> toSupplier(Escaper escaper) {
5858
}
5959

6060
/**
61-
* Transforms the inner value of the ValueFunction using the profided transformation.
61+
* Return a new ValueFunction applying the given mapping {@link Function}. The mapping function is applied after
62+
* applying {@link Escaper}.
6263
*
63-
* The default implementation just return the current {@literal ValueFunction}.
64-
* This is not a valid implementation and serves just to maintain backward compatibility.
65-
*
66-
* @param transformation to be applied to the underlying value.
64+
* @param mapper the mapping function to apply to the value.
65+
* @param <R> the type of the value returned from the mapping function.
6766
* @return a new {@literal ValueFunction}.
6867
* @since 3.2
6968
*/
70-
default ValueFunction<T> transform(Function<Object, Object> transformation) {return this;};
69+
default <R> ValueFunction<R> map(Function<T, R> mapper) {
70+
71+
Assert.notNull(mapper, "Mapping function must not be null");
72+
73+
return escaper -> mapper.apply(this.apply(escaper));
74+
}
7175
}

0 commit comments

Comments
 (0)