Skip to content

Commit aaa894f

Browse files
committed
DATAJDBC-326 - Separated construction of Identifiers from PropertyPath analysis.
1 parent 2473b32 commit aaa894f

File tree

8 files changed

+210
-92
lines changed

8 files changed

+210
-92
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import org.springframework.dao.DataRetrievalFailureException;
3030
import org.springframework.dao.EmptyResultDataAccessException;
3131
import org.springframework.dao.InvalidDataAccessApiUsageException;
32-
import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
32+
import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder;
3333
import org.springframework.data.jdbc.support.JdbcUtil;
3434
import org.springframework.data.mapping.PersistentPropertyAccessor;
3535
import org.springframework.data.mapping.PersistentPropertyPath;
@@ -89,7 +89,7 @@ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, Relation
8989
*/
9090
@Override
9191
public <T> Object insert(T instance, Class<T> domainType, Map<String, Object> additionalParameters) {
92-
return insert(instance, domainType, BasicJdbcConverter.fromNamedValues(additionalParameters));
92+
return insert(instance, domainType, JdbcIdentifierBuilder.from(additionalParameters).build());
9393
}
9494

9595
/*

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import java.util.Collections;
2121
import java.util.Map;
2222

23-
import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
23+
import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder;
2424
import org.springframework.data.relational.domain.Identifier;
2525
import org.springframework.data.mapping.PersistentPropertyPath;
2626
import org.springframework.data.relational.core.conversion.DbAction;
@@ -149,14 +149,14 @@ private <T> Identifier getParentKeys(DbAction.WithDependingOn<?> action) {
149149
RelationalPersistentEntity<?> persistentEntity = context.getRequiredPersistentEntity(dependingOn.getEntityType());
150150

151151
Object id = getIdFromEntityDependingOn(dependingOn, persistentEntity);
152-
Identifier identifier = BasicJdbcConverter //
152+
JdbcIdentifierBuilder identifier = JdbcIdentifierBuilder //
153153
.forBackReferences(action.getPropertyPath(), id);
154154

155155
for (Map.Entry<PersistentPropertyPath<RelationalPersistentProperty>, Object> qualifier : action.getQualifiers().entrySet()) {
156156
identifier = identifier.withQualifier(qualifier.getKey(), qualifier.getValue());
157157
}
158158

159-
return identifier;
159+
return identifier.build();
160160
}
161161

162162
@Nullable

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

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -76,34 +76,6 @@ public BasicJdbcConverter(
7676
super(context, conversions);
7777
}
7878

79-
/**
80-
* Only provided as a bridge to the old way of passing additional parameters to an insert statement.
81-
*
82-
* @param additionalParameters
83-
*/
84-
public static Identifier fromNamedValues(Map<String, Object> additionalParameters) {
85-
86-
List<Identifier.SingleIdentifierValue> keys = new ArrayList<>();
87-
additionalParameters.forEach((k, v) -> keys.add(new Identifier.SingleIdentifierValue(k, v, v == null ? Object.class : v.getClass())));
88-
89-
Identifier identifier = new Identifier(keys);
90-
return identifier;
91-
}
92-
93-
/**
94-
* Creates ParentKeys with backreference for the given path and value of the parents id.
95-
*/
96-
public static Identifier forBackReferences(PersistentPropertyPath<RelationalPersistentProperty> path, @Nullable Object value) {
97-
98-
Identifier.SingleIdentifierValue singleIdentifierValue = new Identifier.SingleIdentifierValue( //
99-
path.getRequiredLeafProperty().getReverseColumnName(), //
100-
value, //
101-
getLastIdProperty(path).getColumnType() //
102-
);
103-
104-
return new Identifier(Collections.singletonList(singleIdentifierValue));
105-
}
106-
10779
/*
10880
* (non-Javadoc)
10981
* @see org.springframework.data.relational.core.conversion.RelationalConverter#readValue(java.lang.Object, org.springframework.data.util.TypeInformation)
@@ -158,16 +130,4 @@ public Object writeValue(@Nullable Object value, TypeInformation<?> type) {
158130
}
159131

160132

161-
private static RelationalPersistentProperty getLastIdProperty(
162-
PersistentPropertyPath<RelationalPersistentProperty> path) {
163-
164-
RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty();
165-
166-
if (idProperty != null) {
167-
return idProperty;
168-
}
169-
170-
return getLastIdProperty(path.getParentPath());
171-
}
172-
173133
}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright 2019 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+
* http://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.jdbc.core.convert;
17+
18+
import java.util.Map;
19+
20+
import org.springframework.data.mapping.PersistentPropertyPath;
21+
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
22+
import org.springframework.data.relational.domain.Identifier;
23+
import org.springframework.lang.Nullable;
24+
25+
/**
26+
* @author Jens Schauder
27+
* @since 1.1
28+
*/
29+
public class JdbcIdentifierBuilder {
30+
31+
private Identifier identifier;
32+
33+
private JdbcIdentifierBuilder(Identifier identifier) {
34+
this.identifier = identifier;
35+
}
36+
37+
public static JdbcIdentifierBuilder empty() {
38+
return new JdbcIdentifierBuilder(Identifier.empty());
39+
}
40+
41+
public static JdbcIdentifierBuilder from(Map<String, Object> additionalParameters) {
42+
43+
Identifier[] identifier = new Identifier[] { Identifier.empty() };
44+
45+
additionalParameters
46+
.forEach((k, v) -> identifier[0] = identifier[0].add(k, v, v == null ? Object.class : v.getClass()));
47+
48+
return new JdbcIdentifierBuilder(identifier[0]);
49+
}
50+
51+
/**
52+
* Creates ParentKeys with backreference for the given path and value of the parents id.
53+
*/
54+
public static JdbcIdentifierBuilder forBackReferences(PersistentPropertyPath<RelationalPersistentProperty> path,
55+
@Nullable Object value) {
56+
57+
Identifier identifier = Identifier.simple( //
58+
path.getRequiredLeafProperty().getReverseColumnName(), //
59+
value, //
60+
getLastIdProperty(path).getColumnType() //
61+
);
62+
63+
return new JdbcIdentifierBuilder(identifier);
64+
}
65+
66+
public JdbcIdentifierBuilder withQualifier(PersistentPropertyPath<RelationalPersistentProperty> path, Object value) {
67+
68+
RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty();
69+
identifier = identifier.add(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType());
70+
71+
return this;
72+
}
73+
74+
public Identifier build() {
75+
return identifier;
76+
}
77+
78+
private static RelationalPersistentProperty getLastIdProperty(
79+
PersistentPropertyPath<RelationalPersistentProperty> path) {
80+
81+
RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty();
82+
83+
if (idProperty != null) {
84+
return idProperty;
85+
}
86+
87+
return getLastIdProperty(path.getParentPath());
88+
}
89+
}

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,19 @@
1616
package org.springframework.data.jdbc.core;
1717

1818
import static org.assertj.core.api.Assertions.*;
19-
import static org.mockito.ArgumentMatchers.eq;
19+
import static org.mockito.ArgumentMatchers.*;
2020
import static org.mockito.Mockito.*;
2121

2222
import org.junit.Test;
2323
import org.mockito.ArgumentCaptor;
2424
import org.springframework.data.annotation.Id;
25-
import org.springframework.data.relational.domain.Identifier;
26-
import org.springframework.data.relational.domain.Identifier.SingleIdentifierValue;
2725
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
2826
import org.springframework.data.relational.core.conversion.DbAction.Insert;
2927
import org.springframework.data.relational.core.conversion.DbAction.InsertRoot;
3028
import org.springframework.data.relational.core.mapping.NamingStrategy;
3129
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
3230
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
31+
import org.springframework.data.relational.domain.Identifier;
3332

3433
/**
3534
* Unit tests for {@link DefaultJdbcInterpreter}
@@ -69,7 +68,9 @@ public void insertDoesHonourNamingStrategyForBackReference() {
6968
ArgumentCaptor<Identifier> argumentCaptor = ArgumentCaptor.forClass(Identifier.class);
7069
verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture());
7170

72-
assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class));
71+
assertThat(argumentCaptor.getValue().getParameters()) //
72+
.extracting("name", "value", "targetType") //
73+
.containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class));
7374
}
7475

7576
@Test // DATAJDBC-251
@@ -82,7 +83,9 @@ public void idOfParentGetsPassedOnAsAdditionalParameterIfNoIdGotGenerated() {
8283
ArgumentCaptor<Identifier> argumentCaptor = ArgumentCaptor.forClass(Identifier.class);
8384
verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture());
8485

85-
assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class));
86+
assertThat(argumentCaptor.getValue().getParameters()) //
87+
.extracting("name", "value", "targetType") //
88+
.containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class));
8689
}
8790

8891
@Test // DATAJDBC-251
@@ -95,7 +98,9 @@ public void generatedIdOfParentGetsPassedOnAsAdditionalParameter() {
9598
ArgumentCaptor<Identifier> argumentCaptor = ArgumentCaptor.forClass(Identifier.class);
9699
verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture());
97100

98-
assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class));
101+
assertThat(argumentCaptor.getValue().getParameters()) //
102+
.extracting("name", "value", "targetType") //
103+
.containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class));
99104
}
100105

101106
static class Container {
Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,29 @@
1818
import static org.assertj.core.api.Assertions.*;
1919
import static org.springframework.data.jdbc.core.PropertyPathUtils.*;
2020

21+
import java.util.AbstractMap;
22+
import java.util.ArrayList;
2123
import java.util.HashMap;
2224
import java.util.List;
2325
import java.util.Map;
2426
import java.util.UUID;
2527

28+
import org.assertj.core.groups.Tuple;
2629
import org.jetbrains.annotations.NotNull;
2730
import org.junit.Test;
2831
import org.springframework.data.annotation.Id;
29-
import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
30-
import org.springframework.data.relational.domain.Identifier;
32+
import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder;
3133
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
3234
import org.springframework.data.mapping.PersistentPropertyPath;
3335
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
36+
import org.springframework.data.relational.domain.Identifier;
3437

3538
/**
36-
* Unit tests for the {@link Identifier} creating methods in the {@link BasicJdbcConverter}.
39+
* Unit tests for the {@link JdbcIdentifierBuilder}.
3740
*
3841
* @author Jens Schauder
3942
*/
40-
public class BasicJdbcConverterIdentifierUnitTests {
43+
public class JdbcIdentifierBuilderUnitTests {
4144

4245
JdbcMappingContext context = new JdbcMappingContext();
4346

@@ -48,11 +51,13 @@ public void parametersWithStringKeysUseTheValuesType() {
4851
parameters.put("one", "eins");
4952
parameters.put("two", 2L);
5053

51-
Identifier identifier = BasicJdbcConverter.fromNamedValues(parameters);
54+
Identifier identifier = JdbcIdentifierBuilder.from(parameters).build();
5255

53-
assertThat(identifier.getParameters()).containsExactlyInAnyOrder( //
54-
new Identifier.SingleIdentifierValue("one", "eins", String.class), //
55-
new Identifier.SingleIdentifierValue("two", 2L, Long.class) //
56+
assertThat(identifier.getParameters()) //
57+
.extracting("name", "value", "targetType") //
58+
.containsExactlyInAnyOrder( //
59+
tuple("one", "eins", String.class), //
60+
tuple("two", 2L, Long.class) //
5661
);
5762
}
5863

@@ -62,20 +67,24 @@ public void parametersWithStringKeysUseObjectAsTypeForNull() {
6267
HashMap<String, Object> parameters = new HashMap<>();
6368
parameters.put("one", null);
6469

65-
Identifier identifier = BasicJdbcConverter.fromNamedValues(parameters);
70+
Identifier identifier = JdbcIdentifierBuilder.from(parameters).build();
6671

67-
assertThat(identifier.getParameters()).containsExactly( //
68-
new Identifier.SingleIdentifierValue("one", null, Object.class) //
72+
assertThat(identifier.getParameters()) //
73+
.extracting("name", "value", "targetType") //
74+
.containsExactly( //
75+
tuple("one", null, Object.class) //
6976
);
7077
}
7178

7279
@Test // DATAJDBC-326
7380
public void parametersWithPropertyKeysUseTheParentPropertyJdbcType() {
7481

75-
Identifier identifier = BasicJdbcConverter.forBackReferences(getPath("child"), "eins");
82+
Identifier identifier = JdbcIdentifierBuilder.forBackReferences(getPath("child"), "eins").build();
7683

77-
assertThat(identifier.getParameters()).containsExactly( //
78-
new Identifier.SingleIdentifierValue("dummy_entity", "eins", UUID.class) //
84+
assertThat(identifier.getParameters()) //
85+
.extracting("name", "value", "targetType") //
86+
.containsExactly( //
87+
tuple("dummy_entity", "eins", UUID.class) //
7988
);
8089
}
8190

@@ -84,13 +93,16 @@ public void qualifiersForMaps() {
8493

8594
PersistentPropertyPath<RelationalPersistentProperty> path = getPath("children");
8695

87-
Identifier identifier = BasicJdbcConverter //
96+
Identifier identifier = JdbcIdentifierBuilder //
8897
.forBackReferences(path, "parent-eins") //
89-
.withQualifier(path, "map-key-eins");
90-
91-
assertThat(identifier.getParameters()).containsExactlyInAnyOrder( //
92-
new Identifier.SingleIdentifierValue("dummy_entity", "parent-eins", UUID.class), //
93-
new Identifier.SingleIdentifierValue("dummy_entity_key", "map-key-eins", String.class) //
98+
.withQualifier(path, "map-key-eins") //
99+
.build();
100+
101+
assertThat(identifier.getParameters()) //
102+
.extracting("name", "value", "targetType") //
103+
.containsExactlyInAnyOrder( //
104+
tuple("dummy_entity", "parent-eins", UUID.class), //
105+
tuple("dummy_entity_key", "map-key-eins", String.class) //
94106
);
95107
}
96108

@@ -99,23 +111,30 @@ public void qualifiersForLists() {
99111

100112
PersistentPropertyPath<RelationalPersistentProperty> path = getPath("moreChildren");
101113

102-
Identifier identifier = BasicJdbcConverter //
114+
Identifier identifier = JdbcIdentifierBuilder //
103115
.forBackReferences(path, "parent-eins") //
104-
.withQualifier(path, "list-index-eins");
105-
106-
assertThat(identifier.getParameters()).containsExactlyInAnyOrder( //
107-
new Identifier.SingleIdentifierValue("dummy_entity", "parent-eins", UUID.class), //
108-
new Identifier.SingleIdentifierValue("dummy_entity_key", "list-index-eins", Integer.class) //
116+
.withQualifier(path, "list-index-eins") //
117+
.build();
118+
119+
assertThat(identifier.getParameters()) //
120+
.extracting("name", "value", "targetType") //
121+
.containsExactlyInAnyOrder( //
122+
tuple("dummy_entity", "parent-eins", UUID.class), //
123+
tuple("dummy_entity_key", "list-index-eins", Integer.class) //
109124
);
110125
}
111126

112127
@Test // DATAJDBC-326
113128
public void backreferenceAcrossEmbeddable() {
114129

115-
Identifier identifier = BasicJdbcConverter.forBackReferences(getPath("embeddable.child"), "parent-eins");
130+
Identifier identifier = JdbcIdentifierBuilder //
131+
.forBackReferences(getPath("embeddable.child"), "parent-eins") //
132+
.build();
116133

117-
assertThat(identifier.getParameters()).containsExactly( //
118-
new Identifier.SingleIdentifierValue("embeddable", "parent-eins", UUID.class) //
134+
assertThat(identifier.getParameters()) //
135+
.extracting("name", "value", "targetType") //
136+
.containsExactly( //
137+
tuple("embeddable", "parent-eins", UUID.class) //
119138
);
120139
}
121140

0 commit comments

Comments
 (0)