Skip to content

Commit 37d475e

Browse files
committed
Polishing.
Refactor BinaryOperation to InfixOperation, add static factory method for consistent creation, tweak Javadoc. See #2320 Original pull request: #2321
1 parent 37df7b1 commit 37d475e

8 files changed

Lines changed: 193 additions & 131 deletions

File tree

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616
package org.springframework.data.jdbc.core.convert;
1717

1818
import java.util.ArrayList;
19-
import java.util.Comparator;
2019
import java.util.LinkedHashMap;
2120
import java.util.List;
2221
import java.util.Map;
2322
import java.util.TreeMap;
2423

2524
import org.jspecify.annotations.Nullable;
25+
2626
import org.springframework.data.mapping.MappingException;
2727
import org.springframework.data.relational.core.mapping.AggregatePath;
2828
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
@@ -491,26 +491,34 @@ private abstract static class CollectionContainer {
491491

492492
private static class ListContainer extends CollectionContainer {
493493

494-
private final Map<Number, Object> list = new TreeMap<>(Comparator.comparing(Number::longValue));
494+
private final Map<Integer, @Nullable Object> list = new TreeMap<>();
495+
private @Nullable Exception addException;
495496

496497
@Override
497498
public void add(Object key, @Nullable Object value) {
498-
list.put(((Number) key).intValue(), value);
499+
int index = ((Number) key).intValue();
500+
if (index < 0 && addException == null) {
501+
addException = new MappingException("Can't add negative indices (%d)".formatted(index));
502+
}
503+
list.put(index, value);
499504
}
500505

501506
@Override
502507
public List<@Nullable Object> get() {
503508

504509
List<@Nullable Object> result = new ArrayList<>(list.size());
505-
506510
list.forEach((index, o) -> {
507511

508-
int intValue = index.intValue();
509-
if (intValue < 0) {
510-
throw new MappingException("Can't build a List with negativ index = " + intValue);
512+
if (index < 0) {
513+
514+
String message = "Can't build a List using negative indices (%d)".formatted(index);
515+
if (addException != null) {
516+
throw new MappingException(message, addException);
517+
}
518+
throw new MappingException(message);
511519
}
512520

513-
while (result.size() < intValue) {
521+
while (result.size() < index) {
514522
result.add(null);
515523
}
516524

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/RowDocumentResultSetExtractorUnitTests.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,11 +271,15 @@ void extractSingleUnorderedListReference() {
271271
@Test // GH-2320
272272
void failsOnNegativeListIndex() {
273273

274-
assertThatExceptionOfType(MappingException.class).isThrownBy(() -> //
274+
MappingException exception = catchThrowableOfType(MappingException.class, () -> //
275275
testerFor(WithList.class).resultSet(rsc -> {
276276
rsc.withPaths("id").withKey("withoutIds").withPath("withoutIds.name") //
277277
.withRow(1, -1, "Dummy Alfred");
278278
}).run(document -> {}));
279+
280+
assertThat(exception).hasMessage("Can't build a List using negative indices (-1)");
281+
assertThat(exception.getCause()).isInstanceOf(MappingException.class)
282+
.hasMessage("Can't add negative indices (-1)");
279283
}
280284
}
281285
}

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/BinaryOperation.java

Lines changed: 0 additions & 85 deletions
This file was deleted.

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Expressions.java

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public static Expression cast(Expression expression, String targetType) {
7070
* {@link Expression}. Otherwise, it creates and returns a {@link TupleExpression} that represents multiple columns as
7171
* a single expression.
7272
*
73-
* @param columns the list of {@link Column}s to include in the expression; must not be {@literal null}.
73+
* @param columns the list of {@link Column}s to include in the expression.
7474
* @return an {@link Expression} corresponding to the input columns: either a single column or a
7575
* {@link TupleExpression} for multiple columns.
7676
* @since 4.0
@@ -83,12 +83,30 @@ public static Expression of(List<Column> columns) {
8383
return new TupleExpression(columns);
8484
}
8585

86-
public static BinaryOperation plus(Expression left, Expression right) {
87-
return new BinaryOperation(left, "+", right);
86+
/**
87+
* Creates an {@link InfixOperation arithmetic operation} that adds {@code right} to {@code left} using the {@code +}
88+
* operator.
89+
*
90+
* @param left the left-hand operand.
91+
* @param right the right-hand operand.
92+
* @return the addition {@link InfixOperation}.
93+
* @since 4.1.1
94+
*/
95+
public static InfixOperation plus(Expression left, Expression right) {
96+
return InfixOperation.create(left, "+", right);
8897
}
8998

90-
public static BinaryOperation minus(Expression left, Expression right) {
91-
return new BinaryOperation(left, "-", right);
99+
/**
100+
* Creates an {@link InfixOperation arithmetic operation} that subtracts {@code right} from {@code left} using the
101+
* {@code -} operator.
102+
*
103+
* @param left the left-hand operand.
104+
* @param right the right-hand operand.
105+
* @return the subtraction {@link InfixOperation}.
106+
* @since 4.1.1
107+
*/
108+
public static InfixOperation minus(Expression left, Expression right) {
109+
return InfixOperation.create(left, "-", right);
92110
}
93111

94112
// Utility constructor.
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2026-present 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+
package org.springframework.data.relational.core.sql;
17+
18+
import org.springframework.util.Assert;
19+
20+
/**
21+
* Represents an infix (two expressions combined with an operator) operation between two {@link Expression}s such as
22+
* arithmetic operations {@code a + b} or concatenation {@code a || b}.
23+
* <p>
24+
* Results in a rendered expression: {@code <left> <operator> <right>} (e.g. {@code col + 4}). For conditional
25+
* expressions use {@link Comparison} instead of a raw {@code InfixOperation}.
26+
*
27+
* @author Jens Schauder
28+
* @author Mark Paluch
29+
* @since 4.1.1
30+
* @see Comparison
31+
*/
32+
public class InfixOperation extends AbstractSegment implements Expression {
33+
34+
private final Expression left;
35+
private final String operator;
36+
private final Expression right;
37+
38+
private InfixOperation(Expression left, String operator, Expression right) {
39+
super(left, right);
40+
this.left = left;
41+
this.operator = operator;
42+
this.right = right;
43+
}
44+
45+
/**
46+
* Create a new {@link InfixOperation} using {@link Expression}s and an operator.
47+
*
48+
* @param left the left {@link Expression}.
49+
* @param operator the operator.
50+
* @param right the right {@link Expression}.
51+
* @return the {@link InfixOperation} operation.
52+
*/
53+
public static InfixOperation create(Expression left, String operator, Expression right) {
54+
55+
Assert.notNull(left, "Left expression must not be null");
56+
Assert.notNull(operator, "Operator must not be null");
57+
Assert.notNull(right, "Right expression must not be null");
58+
59+
return new InfixOperation(left, operator, right);
60+
}
61+
62+
public Expression getLeft() {
63+
return left;
64+
}
65+
66+
public String getOperator() {
67+
return operator;
68+
}
69+
70+
public Expression getRight() {
71+
return right;
72+
}
73+
74+
/**
75+
* Creates a new {@code InfixOperation} aliased to {@code alias}.
76+
*
77+
* @param alias must not be {@literal null} or empty.
78+
* @return the new {@code InfixOperation} using the {@code alias}.
79+
* @since 4.1.1
80+
*/
81+
public InfixOperation as(String alias) {
82+
return new AliasedInfixOperation(this, SqlIdentifier.unquoted(alias));
83+
}
84+
85+
/**
86+
* Creates a new {@code InfixOperation} aliased to {@code alias}.
87+
*
88+
* @param alias must not be {@literal null} or empty.
89+
* @return the new {@code InfixOperation} using the {@code alias}.
90+
* @since 4.1.1
91+
*/
92+
public InfixOperation as(SqlIdentifier alias) {
93+
return new AliasedInfixOperation(this, alias);
94+
}
95+
96+
@Override
97+
public String toString() {
98+
return left + " " + operator + " " + right;
99+
}
100+
101+
private static class AliasedInfixOperation extends InfixOperation implements Aliased {
102+
103+
private final SqlIdentifier alias;
104+
105+
AliasedInfixOperation(InfixOperation operation, SqlIdentifier alias) {
106+
107+
super(operation.left, operation.operator, operation.right);
108+
this.alias = alias;
109+
}
110+
111+
@Override
112+
public SqlIdentifier getAlias() {
113+
return alias;
114+
}
115+
116+
}
117+
118+
}

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ExpressionVisitor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,9 @@ Delegation enterMatched(Expression segment) {
115115
CastVisitor visitor = new CastVisitor(context);
116116
partRenderer = visitor;
117117
return Delegation.delegateTo(visitor);
118-
} else if (segment instanceof BinaryOperation) {
118+
} else if (segment instanceof InfixOperation) {
119119

120-
BinaryOperationVisitor visitor = new BinaryOperationVisitor(context);
120+
InfixOperationVisitor visitor = new InfixOperationVisitor(context);
121121
partRenderer = visitor;
122122
return Delegation.delegateTo(visitor);
123123
} else if (segment instanceof CaseExpression) {

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/BinaryOperationVisitor.java renamed to spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InfixOperationVisitor.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,29 @@
1818
import java.util.StringJoiner;
1919

2020
import org.jspecify.annotations.Nullable;
21-
import org.springframework.data.relational.core.sql.BinaryOperation;
21+
import org.springframework.data.relational.core.sql.InfixOperation;
2222
import org.springframework.data.relational.core.sql.Visitable;
2323
import org.springframework.util.Assert;
2424

2525
/**
26-
* Renders a {@link BinaryOperation} by delegating to an {@link ExpressionVisitor} for each operand and joining the
26+
* Renders a {@link InfixOperation} by delegating to an {@link ExpressionVisitor} for each operand and joining the
2727
* rendered parts with the operator.
2828
*
2929
* @author Jens Schauder
30-
* @since 4.2
30+
* @since 4.1.1
3131
*/
32-
class BinaryOperationVisitor extends TypedSubtreeVisitor<BinaryOperation> implements PartRenderer {
32+
class InfixOperationVisitor extends TypedSubtreeVisitor<InfixOperation> implements PartRenderer {
3333

3434
private final RenderContext context;
3535
private @Nullable StringJoiner joiner;
3636
private @Nullable ExpressionVisitor expressionVisitor;
3737

38-
BinaryOperationVisitor(RenderContext context) {
38+
InfixOperationVisitor(RenderContext context) {
3939
this.context = context;
4040
}
4141

4242
@Override
43-
Delegation enterMatched(BinaryOperation operation) {
43+
Delegation enterMatched(InfixOperation operation) {
4444

4545
joiner = new StringJoiner(" " + operation.getOperator() + " ");
4646
return super.enterMatched(operation);

0 commit comments

Comments
 (0)