From 37de0ac9efb4ec4bfeec0e72c16c1b34ced78b09 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 4 Feb 2019 08:21:18 +0100 Subject: [PATCH 1/7] DATAJDBC-326 - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index fe60487ec8..3181bc4eb1 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-326-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 1753776a73..d987c51da0 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 - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-326-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 9458230173..9a1158979d 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-jdbc - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-326-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-326-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 11e7db737c..46306c43a9 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-relational - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-326-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-326-SNAPSHOT From 45c4d3acd2c42fcba944aa96d9cc8591be07257a Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 12 Feb 2019 07:50:05 +0100 Subject: [PATCH 2/7] DATAJDBC-326 - Support conversion of backreferences. Ids used as backreferences now get properly converted. Introduced ParentKeys. ParentKeys hold information about data that needs to be considered for inserts or updates but is not part of the entity. Appart from column names and values they also hold information about the desired JdbcType in order to facilitate conversions. This replaces the Map handed around in the past. --- .../core/CascadingDataAccessStrategy.java | 11 +- .../data/jdbc/core/DataAccessStrategy.java | 18 +++ .../jdbc/core/DefaultDataAccessStrategy.java | 15 +- .../jdbc/core/DefaultJdbcInterpreter.java | 34 ++--- .../core/DelegatingDataAccessStrategy.java | 9 ++ .../data/jdbc/core/ParentKeys.java | 109 +++++++++++++ .../mybatis/MyBatisDataAccessStrategy.java | 14 ++ .../core/DefaultJdbcInterpreterUnitTests.java | 13 +- .../data/jdbc/core/ParentKeysUnitTests.java | 144 ++++++++++++++++++ ...dbcRepositoryWithMapsIntegrationTests.java | 2 + .../src/test/resources/logback.xml | 2 +- ...AggregateTemplateIntegrationTests-hsql.sql | 6 +- ...epositoryWithMapsIntegrationTests-hsql.sql | 4 + .../core/conversion/AggregateChange.java | 3 +- .../relational/core/conversion/DbAction.java | 8 +- .../core/conversion/WritingContext.java | 2 +- .../BasicRelationalPersistentProperty.java | 13 ++ .../mapping/RelationalPersistentProperty.java | 2 + .../conversion/AggregateChangeUnitTests.java | 13 +- .../RelationalEntityWriterUnitTests.java | 82 ++++++---- 20 files changed, 434 insertions(+), 70 deletions(-) create mode 100644 spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java create mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java index 7edb996ce6..c2ba944240 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java @@ -47,6 +47,15 @@ public Object insert(T instance, Class domainType, Map ad return collect(das -> das.insert(instance, domainType, additionalParameters)); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, org.springframework.data.jdbc.core.ParentKeys) + */ + @Override + public Object insert(T instance, Class domainType, ParentKeys parentKeys) { + return collect(das -> das.insert(instance, domainType, parentKeys)); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#update(java.lang.Object, java.lang.Class) @@ -149,7 +158,7 @@ public boolean existsById(Object id, Class domainType) { private T collect(Function function) { // Keep as Eclipse fails to compile if <> is used. - return strategies.stream().collect(new FunctionCollector(function)); + return strategies.stream().collect(new FunctionCollector<>(function)); } private void collectVoid(Consumer consumer) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java index a307b8b2d0..fe53315113 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java @@ -39,9 +39,27 @@ public interface DataAccessStrategy { * to get referenced are contained in this map. Must not be {@code null}. * @param the type of the instance. * @return the id generated by the database if any. + * + * @deprecated use {@link #insert(Object, Class, ParentKeys)} instead. */ + @Deprecated Object insert(T instance, Class domainType, Map additionalParameters); + + /** + * Inserts a the data of a single entity. Referenced entities don't get handled. + * + * @param instance the instance to be stored. Must not be {@code null}. + * @param domainType the type of the instance. Must not be {@code null}. + * @param parentKeys information about data that needs to be considered for the insert but which is not part of the entity. + * Namely references back to a parent entity and key/index columns for entities that are stored in a {@link Map} or {@link java.util.List}. + * @param the type of the instance. + * @return the id generated by the database if any. + */ + default Object insert(T instance, Class domainType, ParentKeys parentKeys){ + return insert(instance, domainType, parentKeys.getParametersByName()); + } + /** * Updates the data of a single entity in the database. Referenced entities don't get handled. * diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 7ee0cf8390..8d6251a3e3 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -87,10 +87,23 @@ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, Relation */ @Override public Object insert(T instance, Class domainType, Map additionalParameters) { + return insert(instance, domainType, ParentKeys.fromNamedValues(additionalParameters)); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) + */ + @Override + public Object insert(T instance, Class domainType, ParentKeys parentKeys) { KeyHolder holder = new GeneratedKeyHolder(); RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); - Map parameters = new LinkedHashMap<>(additionalParameters); + + Map parameters = new LinkedHashMap<>(); + for (ParentKeys.ParentKey parameter : parentKeys.getParameters()) { + parameters.put(parameter.getName(), converter.writeValue(parameter.getValue(), ClassTypeInformation.from(parameter.getTargetType()))); + } MapSqlParameterSource parameterSource = getPropertyMap(instance, persistentEntity, ""); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index ea6c6603f0..fa63204eb2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -16,9 +16,9 @@ package org.springframework.data.jdbc.core; import lombok.RequiredArgsConstructor; +import lombok.val; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import org.springframework.data.mapping.PersistentPropertyPath; @@ -37,6 +37,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * {@link Interpreter} for {@link DbAction}s using a {@link DataAccessStrategy} for performing actual database @@ -58,7 +59,7 @@ class DefaultJdbcInterpreter implements Interpreter { @Override public void interpret(Insert insert) { - Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), createAdditionalColumnValues(insert)); + Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), getParentKeys(insert)); insert.setGeneratedId(id); } @@ -101,7 +102,7 @@ public void interpret(Merge merge) { // temporary implementation if (!accessStrategy.update(merge.getEntity(), merge.getEntityType())) { - accessStrategy.insert(merge.getEntity(), merge.getEntityType(), createAdditionalColumnValues(merge)); + accessStrategy.insert(merge.getEntity(), merge.getEntityType(), getParentKeys(merge)); } } @@ -141,27 +142,21 @@ public void interpret(DeleteAllRoot deleteAllRoot) { accessStrategy.deleteAll(deleteAllRoot.getEntityType()); } - private Map createAdditionalColumnValues(DbAction.WithDependingOn action) { - - Map additionalColumnValues = new HashMap<>(); - addDependingOnInformation(action, additionalColumnValues); - additionalColumnValues.putAll(action.getAdditionalValues()); - - return additionalColumnValues; - } - - private void addDependingOnInformation(DbAction.WithDependingOn action, - Map additionalColumnValues) { + private ParentKeys getParentKeys(DbAction.WithDependingOn action) { DbAction.WithEntity dependingOn = action.getDependingOn(); RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(dependingOn.getEntityType()); - String columnName = getColumnNameForReverseColumn(action); - Object identifier = getIdFromEntityDependingOn(dependingOn, persistentEntity); + ParentKeys parentKeys = ParentKeys // + .forBackReferences(action.getPropertyPath(), identifier); - additionalColumnValues.put(columnName, identifier); + for (Map.Entry, Object> qualifier : action.getQualifiers().entrySet()) { + parentKeys = parentKeys.withQualifier(qualifier.getKey(), qualifier.getValue()); + } + + return parentKeys; } @Nullable @@ -182,9 +177,4 @@ private Object getIdFromEntityDependingOn(DbAction.WithEntity dependingOn, return persistentEntity.getIdentifierAccessor(entity).getIdentifier(); } - private String getColumnNameForReverseColumn(DbAction.WithPropertyPath action) { - - PersistentPropertyPath path = action.getPropertyPath(); - return path.getRequiredLeafProperty().getReverseColumnName(); - } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java index 0a833f30e4..a81a7d1c06 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java @@ -40,6 +40,15 @@ public Object insert(T instance, Class domainType, Map ad return delegate.insert(instance, domainType, additionalParameters); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, org.springframework.data.jdbc.core.ParentKeys) + */ + @Override + public Object insert(T instance, Class domainType, ParentKeys parentKeys) { + return delegate.insert(instance, domainType, parentKeys); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#update(java.lang.Object, java.lang.Class) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java new file mode 100644 index 0000000000..8f6899f6b7 --- /dev/null +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java @@ -0,0 +1,109 @@ +/* + * Copyright 2019 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 + * + * http://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.core; + +import lombok.Value; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.lang.Nullable; + +/** + * @author Jens Schauder + */ +public class ParentKeys { + + private final List keys; + + /** + * Only provided as a bridge to the old way of passing additional parameters to an insert statement. + * + * @param additionalParameters + */ + @Deprecated + static ParentKeys fromNamedValues(Map additionalParameters) { + + List keys = new ArrayList<>(); + additionalParameters.forEach((k, v) -> keys.add(new ParentKey(k, v, v == null ? Object.class : v.getClass()))); + + ParentKeys parentKeys = new ParentKeys(keys); + return parentKeys; + } + + /** + * Creates ParentKeys with backreference for the given path and value of the parents id. + */ + static ParentKeys forBackReferences(PersistentPropertyPath path, @Nullable Object value) { + + ParentKey parentKey = new ParentKey( // + path.getRequiredLeafProperty().getReverseColumnName(), // + value, // + getLastIdProperty(path).getColumnType() // + ); + + return new ParentKeys(Collections.singletonList(parentKey)); + } + + private static RelationalPersistentProperty getLastIdProperty( + PersistentPropertyPath path) { + + RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty(); + + if (idProperty != null) { + return idProperty; + } + + return getLastIdProperty(path.getParentPath()); + } + + private ParentKeys(List keys) { + + this.keys = Collections.unmodifiableList(keys); + } + + ParentKeys withQualifier(PersistentPropertyPath path, Object value) { + + List keys = new ArrayList<>(this.keys); + + RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); + keys.add(new ParentKey(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType())); + + return new ParentKeys(keys); + } + + @Deprecated + public Map getParametersByName() { + return new HashMap<>(); + } + + Collection getParameters() { + return keys; + } + + @Value + static class ParentKey { + String name; + Object value; + Class targetType; + } +} diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 9618455805..a056d6514e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -26,6 +26,7 @@ import org.springframework.data.jdbc.core.DataAccessStrategy; import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; import org.springframework.data.jdbc.core.DelegatingDataAccessStrategy; +import org.springframework.data.jdbc.core.ParentKeys; import org.springframework.data.jdbc.core.SqlGeneratorSource; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PropertyPath; @@ -136,6 +137,19 @@ public Object insert(T instance, Class domainType, Map ad return myBatisContext.getId(); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, ParentKeys) + */ + @Override + public Object insert(T instance, Class domainType, ParentKeys parentKeys) { + + MyBatisContext myBatisContext = new MyBatisContext(null, instance, domainType, parentKeys.getParametersByName()); + sqlSession().insert(namespace(domainType) + ".insert", myBatisContext); + + return myBatisContext.getId(); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#update(java.lang.Object, java.lang.Class) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java index 1029746cea..bffce4ed3c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java @@ -25,6 +25,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.springframework.data.annotation.Id; +import org.springframework.data.jdbc.core.ParentKeys.ParentKey; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.relational.core.conversion.DbAction.Insert; import org.springframework.data.relational.core.conversion.DbAction.InsertRoot; @@ -67,10 +68,10 @@ public void insertDoesHonourNamingStrategyForBackReference() { interpreter.interpret(insert); - ArgumentCaptor> argumentCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ParentKeys.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue()).containsExactly(new SimpleEntry(BACK_REFERENCE, CONTAINER_ID)); + assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new ParentKey(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @Test // DATAJDBC-251 @@ -80,10 +81,10 @@ public void idOfParentGetsPassedOnAsAdditionalParameterIfNoIdGotGenerated() { interpreter.interpret(insert); - ArgumentCaptor> argumentCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ParentKeys.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue()).containsExactly(new SimpleEntry(BACK_REFERENCE, CONTAINER_ID)); + assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new ParentKey(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @Test // DATAJDBC-251 @@ -93,10 +94,10 @@ public void generatedIdOfParentGetsPassedOnAsAdditionalParameter() { interpreter.interpret(insert); - ArgumentCaptor> argumentCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ParentKeys.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue()).containsExactly(new SimpleEntry(BACK_REFERENCE, CONTAINER_ID)); + assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new ParentKey(BACK_REFERENCE, CONTAINER_ID, Long.class)); } static class Container { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java new file mode 100644 index 0000000000..4ac8345478 --- /dev/null +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java @@ -0,0 +1,144 @@ +/* + * Copyright 2019 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 + * + * http://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.core; + +import static org.assertj.core.api.Assertions.*; +import static org.springframework.data.jdbc.core.PropertyPathUtils.*; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import org.jetbrains.annotations.NotNull; +import org.junit.Test; +import org.springframework.data.annotation.Id; +import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; + +/** + * @author Jens Schauder + */ +public class ParentKeysUnitTests { + + JdbcMappingContext context = new JdbcMappingContext(); + + @Test // DATAJDBC-326 + public void parametersWithStringKeysUseTheValuesType() { + + HashMap parameters = new HashMap<>(); + parameters.put("one", "eins"); + parameters.put("two", 2L); + + ParentKeys parentKeys = ParentKeys.fromNamedValues(parameters); + + assertThat(parentKeys.getParameters()).containsExactlyInAnyOrder( // + new ParentKeys.ParentKey("one", "eins", String.class), // + new ParentKeys.ParentKey("two", 2L, Long.class) // + ); + } + + @Test // DATAJDBC-326 + public void parametersWithStringKeysUseObjectAsTypeForNull() { + + HashMap parameters = new HashMap<>(); + parameters.put("one", null); + + ParentKeys parentKeys = ParentKeys.fromNamedValues(parameters); + + assertThat(parentKeys.getParameters()).containsExactly( // + new ParentKeys.ParentKey("one", null, Object.class) // + ); + } + + @Test // DATAJDBC-326 + public void parametersWithPropertyKeysUseTheParentPropertyJdbcType() { + + ParentKeys parentKeys = ParentKeys.forBackReferences(getPath("child"), "eins"); + + assertThat(parentKeys.getParameters()).containsExactly( // + new ParentKeys.ParentKey("dummy_entity", "eins", UUID.class) // + ); + } + + @Test // DATAJDBC-326 + public void qualifiersForMaps() { + + PersistentPropertyPath path = getPath("children"); + + ParentKeys parentKeys = ParentKeys // + .forBackReferences(path, "parent-eins") // + .withQualifier(path, "map-key-eins"); + + assertThat(parentKeys.getParameters()).containsExactlyInAnyOrder( // + new ParentKeys.ParentKey("dummy_entity", "parent-eins", UUID.class), // + new ParentKeys.ParentKey("dummy_entity_key", "map-key-eins", String.class) // + ); + } + + @Test // DATAJDBC-326 + public void qualifiersForLists() { + + PersistentPropertyPath path = getPath("moreChildren"); + + ParentKeys parentKeys = ParentKeys // + .forBackReferences(path, "parent-eins") // + .withQualifier(path, "list-index-eins"); + + assertThat(parentKeys.getParameters()).containsExactlyInAnyOrder( // + new ParentKeys.ParentKey("dummy_entity", "parent-eins", UUID.class), // + new ParentKeys.ParentKey("dummy_entity_key", "list-index-eins", Integer.class) // + ); + } + + @Test // DATAJDBC-326 + public void backreferenceAcrossEmbeddable() { + + ParentKeys parentKeys = ParentKeys.forBackReferences(getPath("embeddable.child"), "parent-eins"); + + assertThat(parentKeys.getParameters()).containsExactly( // + new ParentKeys.ParentKey("embeddable", "parent-eins", UUID.class) // + ); + } + + @NotNull + private PersistentPropertyPath getPath(String dotPath) { + return toPath(dotPath, DummyEntity.class, context); + } + + @SuppressWarnings("unused") + static class DummyEntity { + + @Id UUID id; + String one; + Long two; + Child child; + + Map children; + + List moreChildren; + + Embeddable embeddable; + } + + @SuppressWarnings("unused") + static class Embeddable { + Child child; + } + + static class Child {} +} diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java index 269a426c5f..0239595f13 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java @@ -96,7 +96,9 @@ public void saveAndLoadEmptyMap() { public void saveAndLoadNonEmptyMap() { Element element1 = new Element(); + element1.content = "element 1"; Element element2 = new Element(); + element2.content = "element 2"; DummyEntity entity = createDummyEntity(); entity.content.put("one", element1); diff --git a/spring-data-jdbc/src/test/resources/logback.xml b/spring-data-jdbc/src/test/resources/logback.xml index e310de95b6..f1bfdbaf39 100644 --- a/spring-data-jdbc/src/test/resources/logback.xml +++ b/spring-data-jdbc/src/test/resources/logback.xml @@ -12,7 +12,7 @@ - + \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql index a327aecad8..4edb29701d 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql @@ -8,6 +8,10 @@ CREATE TABLE ONE_TO_ONE_PARENT ( id3 BIGINT GENERATED BY DEFAULT AS IDENTITY(STA CREATE TABLE Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR(30)); CREATE TABLE LIST_PARENT ( id4 BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100)); -CREATE TABLE element_no_id ( content VARCHAR(100), LIST_PARENT_key BIGINT, LIST_PARENT BIGINT); +CREATE TABLE ELEMENT_NO_ID ( content VARCHAR(100), LIST_PARENT_KEY BIGINT, LIST_PARENT BIGINT); +ALTER TABLE ELEMENT_NO_ID + ADD FOREIGN KEY (LIST_PARENT) + REFERENCES LIST_PARENT(id4); + CREATE TABLE ARRAY_OWNER (ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, DIGITS VARCHAR(20) ARRAY[10] NOT NULL, MULTIDIMENSIONAL VARCHAR(20) ARRAY[10] NULL); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql index 33c747b6f0..15d39c175f 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql @@ -1,2 +1,6 @@ CREATE TABLE dummy_entity ( id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100)); CREATE TABLE element (id BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, content VARCHAR(100), Dummy_Entity_key VARCHAR(100), dummy_entity BIGINT); + +ALTER TABLE ELEMENT + ADD FOREIGN KEY (dummy_entity) + REFERENCES dummy_entity(id); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java index 86e9bc8acf..037d851930 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java @@ -114,8 +114,7 @@ static void setId(RelationalMappingContext context, RelationalConverter converte if (leafProperty.isQualified()) { - String keyColumn = leafProperty.getKeyColumn(); - Object keyObject = action.getAdditionalValues().get(keyColumn); + Object keyObject = action.getQualifiers().get(propertyPathToEntity); if (List.class.isAssignableFrom(leafProperty.getType())) { setIdInElementOfList(converter, action, generatedId, (List) currentPropertyValue, (int) keyObject); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index 171b392bf2..0daa1d445a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -75,7 +75,7 @@ class Insert implements WithGeneratedId, WithDependingOn { @NonNull final PersistentPropertyPath propertyPath; @NonNull final WithEntity dependingOn; - Map additionalValues = new HashMap<>(); + Map, Object> qualifiers = new HashMap<>(); private Object generatedId; @@ -154,7 +154,7 @@ class Merge implements WithDependingOn, WithPropertyPath { @NonNull PersistentPropertyPath propertyPath; @NonNull WithEntity dependingOn; - Map additionalValues = new HashMap<>(); + Map, Object> qualifiers = new HashMap<>(); @Override public void doExecuteWith(Interpreter interpreter) { @@ -248,7 +248,7 @@ interface WithDependingOn extends WithPropertyPath, WithEntity { * become available once the parent entity got persisted. * * @return Guaranteed to be not {@code null}. - * @see #getAdditionalValues() + * @see #getQualifiers() */ WithEntity getDependingOn(); @@ -259,7 +259,7 @@ interface WithDependingOn extends WithPropertyPath, WithEntity { * * @return Guaranteed to be not {@code null}. */ - Map getAdditionalValues(); + Map, Object> getQualifiers(); @Override default Class getEntityType() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java index 10b2874050..6de69781e7 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java @@ -127,7 +127,7 @@ private List> insertAll(PersistentPropertyPath value = (Pair) node.getValue(); insert = new DbAction.Insert<>(value.getSecond(), path, getAction(node.getParent())); - insert.getAdditionalValues().put(node.getPath().getRequiredLeafProperty().getKeyColumn(), value.getFirst()); + insert.getQualifiers().put(node.getPath(), value.getFirst()); } else { insert = new DbAction.Insert<>(node.getValue(), path, getAction(node.getParent())); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java index 001f7fb3ca..b9b70ea4af 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java @@ -195,6 +195,19 @@ public boolean isQualified() { return isMap() || isListLike(); } + @Override + public Class getQualifierColumnType() { + + Assert.isTrue(isQualified(), "The qualifier column type is only defined for properties that are qualified"); + + if (isMap()) { + return getTypeInformation().getComponentType().getType(); + } + + // for lists and arrays + return Integer.class; + } + @Override public boolean isOrdered() { return isListLike(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentProperty.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentProperty.java index 2653be60f3..4d0f60dbaa 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentProperty.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentProperty.java @@ -67,6 +67,8 @@ public interface RelationalPersistentProperty extends PersistentProperty getQualifierColumnType(); + /** * Returns whether this property is an ordered property. */ diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java index d894935b9e..ef07b70461 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java @@ -27,7 +27,10 @@ import org.junit.Test; import org.springframework.data.annotation.Id; import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.mapping.PersistentPropertyPaths; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; /** * Unit tests for the {@link AggregateChange}. @@ -52,7 +55,7 @@ DbAction.Insert createInsert(String propertyName, Object value, Object key) { DbAction.Insert insert = new DbAction.Insert<>(value, context.getPersistentPropertyPath(propertyName, DummyEntity.class), rootInsert); - insert.getAdditionalValues().put("dummy_entity_key", key); + insert.getQualifiers().put(toPath(propertyName, DummyEntity.class), key); return insert; } @@ -112,6 +115,14 @@ public void setIdForSingleElementMap() { .containsExactlyInAnyOrder(tuple("one", 23)); } + PersistentPropertyPath toPath(String path, Class source) { + + PersistentPropertyPaths persistentPropertyPaths = context + .findPersistentPropertyPaths(source, p -> true); + + return persistentPropertyPaths.filter(p -> p.toDotPath().equals(path)).stream().findFirst().orElse(null); + } + private static class DummyEntity { @Id Integer rootId; diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java index 1d143329c7..98e2b08f44 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java @@ -30,6 +30,8 @@ import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.data.annotation.Id; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.mapping.PersistentPropertyPaths; import org.springframework.data.relational.core.conversion.AggregateChange.Kind; import org.springframework.data.relational.core.conversion.DbAction.Delete; import org.springframework.data.relational.core.conversion.DbAction.Insert; @@ -37,6 +39,7 @@ import org.springframework.data.relational.core.conversion.DbAction.UpdateRoot; import org.springframework.data.relational.core.mapping.Embedded; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; /** * Unit tests for the {@link RelationalEntityWriter} @@ -47,8 +50,13 @@ @RunWith(MockitoJUnitRunner.class) public class RelationalEntityWriterUnitTests { - public static final long SOME_ENTITY_ID = 23L; - RelationalEntityWriter converter = new RelationalEntityWriter(new RelationalMappingContext()); + static final long SOME_ENTITY_ID = 23L; + final RelationalMappingContext context = new RelationalMappingContext(); + final RelationalEntityWriter converter = new RelationalEntityWriter(context); + + final PersistentPropertyPath listContainerElements = toPath("elements", ListContainer.class, context); + + private final PersistentPropertyPath mapContainerElements = toPath("elements", MapContainer.class, context); @Test // DATAJDBC-112 public void newEntityGetsConvertedToOneInsert() { @@ -60,8 +68,8 @@ public void newEntityGetsConvertedToOneInsert() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) // - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(InsertRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false) // ); @@ -79,8 +87,8 @@ public void newEntityGetsConvertedToOneInsertByEmbeddedEntities() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) // - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(InsertRoot.class, EmbeddedReferenceEntity.class, "", EmbeddedReferenceEntity.class, false) // ); @@ -98,8 +106,8 @@ public void newEntityWithReferenceGetsConvertedToTwoInserts() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) // - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(InsertRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false), // tuple(Insert.class, Element.class, "other", Element.class, true) // @@ -117,8 +125,8 @@ public void existingEntityGetsConvertedToDeletePlusUpdate() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) // - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(Delete.class, Element.class, "other", null, false), // tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false) // @@ -131,14 +139,14 @@ public void newReferenceTriggersDeletePlusInsert() { SingleReferenceEntity entity = new SingleReferenceEntity(SOME_ENTITY_ID); entity.other = new Element(null); - AggregateChange aggregateChange = new AggregateChange<>(Kind.SAVE, SingleReferenceEntity.class, - entity); + AggregateChange aggregateChange = new AggregateChange<>(Kind.SAVE, + SingleReferenceEntity.class, entity); converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) // - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(Delete.class, Element.class, "other", null, false), // tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false), // @@ -150,14 +158,14 @@ public void newReferenceTriggersDeletePlusInsert() { public void newEntityWithEmptySetResultsInSingleInsert() { SetContainer entity = new SetContainer(null); - AggregateChange aggregateChange = new AggregateChange<>( - Kind.SAVE, SetContainer.class, entity); + AggregateChange aggregateChange = new AggregateChange<>(Kind.SAVE, + SetContainer.class, entity); converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) // - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(InsertRoot.class, SetContainer.class, "", SetContainer.class, false)); } @@ -173,8 +181,8 @@ public void newEntityWithSetResultsInAdditionalInsertPerElement() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(InsertRoot.class, SetContainer.class, "", SetContainer.class, false), // tuple(Insert.class, Element.class, "elements", Element.class, true), // @@ -203,8 +211,8 @@ public void cascadingReferencesTriggerCascadingActions() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(InsertRoot.class, CascadingReferenceEntity.class, "", CascadingReferenceEntity.class, false), // tuple(Insert.class, CascadingReferenceMiddleElement.class, "other", CascadingReferenceMiddleElement.class, @@ -239,8 +247,8 @@ public void cascadingReferencesTriggerCascadingActionsForUpdate() { converter.write(entity, aggregateChange); assertThat(aggregateChange.getActions()) - .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, DbActionTestSupport::actualEntityType, - DbActionTestSupport::isWithDependsOn) // + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath, + DbActionTestSupport::actualEntityType, DbActionTestSupport::isWithDependsOn) // .containsExactly( // tuple(Delete.class, Element.class, "other.element", null, false), tuple(Delete.class, CascadingReferenceMiddleElement.class, "other", null, false), @@ -264,7 +272,8 @@ public void newEntityWithEmptyMapResultsInSingleInsert() { converter.write(entity, aggregateChange); - assertThat(aggregateChange.getActions()).extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // + assertThat(aggregateChange.getActions()) + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // .containsExactly( // tuple(InsertRoot.class, MapContainer.class, "")); } @@ -341,7 +350,8 @@ public void newEntityWithEmptyListResultsInSingleInsert() { converter.write(entity, aggregateChange); - assertThat(aggregateChange.getActions()).extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // + assertThat(aggregateChange.getActions()) + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // .containsExactly( // tuple(InsertRoot.class, ListContainer.class, "")); } @@ -418,17 +428,29 @@ private CascadingReferenceMiddleElement createMiddleElement(Element first, Eleme } private Object getMapKey(DbAction a) { - return a instanceof DbAction.WithDependingOn - ? ((DbAction.WithDependingOn) a).getAdditionalValues().get("map_container_key") + + return a instanceof DbAction.WithDependingOn // + ? ((DbAction.WithDependingOn) a).getQualifiers().get(mapContainerElements) // : null; } private Object getListKey(DbAction a) { - return a instanceof DbAction.WithDependingOn - ? ((DbAction.WithDependingOn) a).getAdditionalValues().get("list_container_key") + + return a instanceof DbAction.WithDependingOn // + ? ((DbAction.WithDependingOn) a).getQualifiers() + .get(listContainerElements) // : null; } + static PersistentPropertyPath toPath(String path, Class source, + RelationalMappingContext context) { + + PersistentPropertyPaths persistentPropertyPaths = context + .findPersistentPropertyPaths(source, p -> true); + + return persistentPropertyPaths.filter(p -> p.toDotPath().equals(path)).stream().findFirst().orElse(null); + } + @RequiredArgsConstructor static class SingleReferenceEntity { From 914a81a99ccb994551dfe8e4a8ad0b43614731e9 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 27 Feb 2019 14:41:52 +0100 Subject: [PATCH 3/7] DATAJDBC-326 - Applied feedback from review. ParentKeys gets renamed to Identifier and moved to spring-data-relational. This facilitates reuse beyond just references to parents. The factory methods moved to BasicJdbcConverter. --- .../core/CascadingDataAccessStrategy.java | 5 +- .../data/jdbc/core/DataAccessStrategy.java | 9 +- .../jdbc/core/DefaultDataAccessStrategy.java | 56 +++++---- .../jdbc/core/DefaultJdbcInterpreter.java | 8 +- .../core/DelegatingDataAccessStrategy.java | 5 +- .../data/jdbc/core/ParentKeys.java | 109 ------------------ .../jdbc/core/convert/BasicJdbcConverter.java | 48 ++++++++ .../mybatis/MyBatisDataAccessStrategy.java | 6 +- ...asicJdbcConverterIdentifierUnitTests.java} | 48 ++++---- .../core/DefaultJdbcInterpreterUnitTests.java | 18 ++- .../data/relational/domain/Identifier.java | 83 +++++++++++++ 11 files changed, 214 insertions(+), 181 deletions(-) delete mode 100644 spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java rename spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/{ParentKeysUnitTests.java => BasicJdbcConverterIdentifierUnitTests.java} (60%) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java index c2ba944240..f78dc5bd35 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java @@ -21,6 +21,7 @@ import java.util.function.Consumer; import java.util.function.Function; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; @@ -52,8 +53,8 @@ public Object insert(T instance, Class domainType, Map ad * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, org.springframework.data.jdbc.core.ParentKeys) */ @Override - public Object insert(T instance, Class domainType, ParentKeys parentKeys) { - return collect(das -> das.insert(instance, domainType, parentKeys)); + public Object insert(T instance, Class domainType, Identifier identifier) { + return collect(das -> das.insert(instance, domainType, identifier)); } /* diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java index fe53315113..ce731ce145 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java @@ -17,6 +17,7 @@ import java.util.Map; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.lang.Nullable; @@ -40,7 +41,7 @@ public interface DataAccessStrategy { * @param the type of the instance. * @return the id generated by the database if any. * - * @deprecated use {@link #insert(Object, Class, ParentKeys)} instead. + * @deprecated use {@link #insert(Object, Class, Identifier)} instead. */ @Deprecated Object insert(T instance, Class domainType, Map additionalParameters); @@ -51,13 +52,13 @@ public interface DataAccessStrategy { * * @param instance the instance to be stored. Must not be {@code null}. * @param domainType the type of the instance. Must not be {@code null}. - * @param parentKeys information about data that needs to be considered for the insert but which is not part of the entity. + * @param identifier information about data that needs to be considered for the insert but which is not part of the entity. * Namely references back to a parent entity and key/index columns for entities that are stored in a {@link Map} or {@link java.util.List}. * @param the type of the instance. * @return the id generated by the database if any. */ - default Object insert(T instance, Class domainType, ParentKeys parentKeys){ - return insert(instance, domainType, parentKeys.getParametersByName()); + default Object insert(T instance, Class domainType, Identifier identifier){ + return insert(instance, domainType, identifier.getParametersByName()); } /** diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 8d6251a3e3..ab3b80220e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -29,6 +29,7 @@ import org.springframework.dao.DataRetrievalFailureException; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; import org.springframework.data.jdbc.support.JdbcUtil; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; @@ -37,6 +38,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.util.ClassTypeInformation; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -87,7 +89,7 @@ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, Relation */ @Override public Object insert(T instance, Class domainType, Map additionalParameters) { - return insert(instance, domainType, ParentKeys.fromNamedValues(additionalParameters)); + return insert(instance, domainType, BasicJdbcConverter.fromNamedValues(additionalParameters)); } /* @@ -95,15 +97,16 @@ public Object insert(T instance, Class domainType, Map ad * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) */ @Override - public Object insert(T instance, Class domainType, ParentKeys parentKeys) { + public Object insert(T instance, Class domainType, Identifier identifier) { KeyHolder holder = new GeneratedKeyHolder(); RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); Map parameters = new LinkedHashMap<>(); - for (ParentKeys.ParentKey parameter : parentKeys.getParameters()) { - parameters.put(parameter.getName(), converter.writeValue(parameter.getValue(), ClassTypeInformation.from(parameter.getTargetType()))); - } + identifier.forEach(identifierValue -> { + parameters.put(identifierValue.getName(), + converter.writeValue(identifierValue.getValue(), ClassTypeInformation.from(identifierValue.getTargetType()))); + }); MapSqlParameterSource parameterSource = getPropertyMap(instance, persistentEntity, ""); @@ -295,7 +298,8 @@ public boolean existsById(Object id, Class domainType) { return result; } - private MapSqlParameterSource getPropertyMap(final S instance, RelationalPersistentEntity persistentEntity, String prefix) { + private MapSqlParameterSource getPropertyMap(final S instance, RelationalPersistentEntity persistentEntity, + String prefix) { MapSqlParameterSource parameters = new MapSqlParameterSource(); @@ -307,23 +311,26 @@ private MapSqlParameterSource getPropertyMap(final S instance, Relational return; } - if(property.isEmbedded()){ + if (property.isEmbedded()) { Object value = propertyAccessor.getProperty(property); - final RelationalPersistentEntity embeddedEntity = context.getPersistentEntity(property.getType()); - final MapSqlParameterSource additionalParameters = getPropertyMap((T)value, (RelationalPersistentEntity) embeddedEntity, prefix + property.getEmbeddedPrefix()); + final RelationalPersistentEntity embeddedEntity = context.getPersistentEntity(property.getType()); + final MapSqlParameterSource additionalParameters = getPropertyMap((T) value, + (RelationalPersistentEntity) embeddedEntity, prefix + property.getEmbeddedPrefix()); parameters.addValues(additionalParameters.getValues()); } else { Object value = propertyAccessor.getProperty(property); Object convertedValue = convertForWrite(property, value); - parameters.addValue(prefix + property.getColumnName(), convertedValue, JdbcUtil.sqlTypeFor(property.getColumnType())); + parameters.addValue(prefix + property.getColumnName(), convertedValue, + JdbcUtil.sqlTypeFor(property.getColumnType())); } }); return parameters; } + @Nullable private Object convertForWrite(RelationalPersistentProperty property, @Nullable Object value) { @@ -340,9 +347,8 @@ private Object convertForWrite(RelationalPersistentProperty property, @Nullable String typeName = JDBCType.valueOf(JdbcUtil.sqlTypeFor(componentType)).getName(); - return operations.getJdbcOperations().execute( - (Connection c) -> c.createArrayOf(typeName, (Object[]) convertedValue) - ); + return operations.getJdbcOperations() + .execute((Connection c) -> c.createArrayOf(typeName, (Object[]) convertedValue)); } @SuppressWarnings("unchecked") @@ -367,22 +373,22 @@ private static boolean isIdPropertyNullOrScalarZero(@Nullable ID idValue @Nullable private Object getIdFromHolder(KeyHolder holder, RelationalPersistentEntity persistentEntity) { - try { - // MySQL just returns one value with a special name - return holder.getKey(); - } catch (DataRetrievalFailureException | InvalidDataAccessApiUsageException e) { - // Postgres returns a value for each column + try { + // MySQL just returns one value with a special name + return holder.getKey(); + } catch (DataRetrievalFailureException | InvalidDataAccessApiUsageException e) { + // Postgres returns a value for each column // MS SQL Server returns a value that might be null. - Map keys = holder.getKeys(); + Map keys = holder.getKeys(); - if (keys == null || persistentEntity.getIdProperty() == null) { - return null; - } + if (keys == null || persistentEntity.getIdProperty() == null) { + return null; + } - return keys.get(persistentEntity.getIdColumn()); - } - } + return keys.get(persistentEntity.getIdColumn()); + } + } private EntityRowMapper getEntityRowMapper(Class domainType) { return new EntityRowMapper<>(getRequiredPersistentEntity(domainType), context, converter, accessStrategy); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index fa63204eb2..1679052df5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -16,11 +16,12 @@ package org.springframework.data.jdbc.core; import lombok.RequiredArgsConstructor; -import lombok.val; import java.util.Collections; import java.util.Map; +import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.conversion.DbAction; import org.springframework.data.relational.core.conversion.DbAction.Delete; @@ -37,7 +38,6 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.lang.Nullable; -import org.springframework.util.Assert; /** * {@link Interpreter} for {@link DbAction}s using a {@link DataAccessStrategy} for performing actual database @@ -142,14 +142,14 @@ public void interpret(DeleteAllRoot deleteAllRoot) { accessStrategy.deleteAll(deleteAllRoot.getEntityType()); } - private ParentKeys getParentKeys(DbAction.WithDependingOn action) { + private Identifier getParentKeys(DbAction.WithDependingOn action) { DbAction.WithEntity dependingOn = action.getDependingOn(); RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(dependingOn.getEntityType()); Object identifier = getIdFromEntityDependingOn(dependingOn, persistentEntity); - ParentKeys parentKeys = ParentKeys // + Identifier parentKeys = BasicJdbcConverter // .forBackReferences(action.getPropertyPath(), identifier); for (Map.Entry, Object> qualifier : action.getQualifiers().entrySet()) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java index a81a7d1c06..67382311eb 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java @@ -17,6 +17,7 @@ import java.util.Map; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.util.Assert; @@ -45,8 +46,8 @@ public Object insert(T instance, Class domainType, Map ad * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, org.springframework.data.jdbc.core.ParentKeys) */ @Override - public Object insert(T instance, Class domainType, ParentKeys parentKeys) { - return delegate.insert(instance, domainType, parentKeys); + public Object insert(T instance, Class domainType, Identifier identifier) { + return delegate.insert(instance, domainType, identifier); } /* diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java deleted file mode 100644 index 8f6899f6b7..0000000000 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/ParentKeys.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright 2019 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 - * - * http://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.core; - -import lombok.Value; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import org.springframework.data.mapping.PersistentPropertyPath; -import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; -import org.springframework.lang.Nullable; - -/** - * @author Jens Schauder - */ -public class ParentKeys { - - private final List keys; - - /** - * Only provided as a bridge to the old way of passing additional parameters to an insert statement. - * - * @param additionalParameters - */ - @Deprecated - static ParentKeys fromNamedValues(Map additionalParameters) { - - List keys = new ArrayList<>(); - additionalParameters.forEach((k, v) -> keys.add(new ParentKey(k, v, v == null ? Object.class : v.getClass()))); - - ParentKeys parentKeys = new ParentKeys(keys); - return parentKeys; - } - - /** - * Creates ParentKeys with backreference for the given path and value of the parents id. - */ - static ParentKeys forBackReferences(PersistentPropertyPath path, @Nullable Object value) { - - ParentKey parentKey = new ParentKey( // - path.getRequiredLeafProperty().getReverseColumnName(), // - value, // - getLastIdProperty(path).getColumnType() // - ); - - return new ParentKeys(Collections.singletonList(parentKey)); - } - - private static RelationalPersistentProperty getLastIdProperty( - PersistentPropertyPath path) { - - RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty(); - - if (idProperty != null) { - return idProperty; - } - - return getLastIdProperty(path.getParentPath()); - } - - private ParentKeys(List keys) { - - this.keys = Collections.unmodifiableList(keys); - } - - ParentKeys withQualifier(PersistentPropertyPath path, Object value) { - - List keys = new ArrayList<>(this.keys); - - RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); - keys.add(new ParentKey(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType())); - - return new ParentKeys(keys); - } - - @Deprecated - public Map getParametersByName() { - return new HashMap<>(); - } - - Collection getParameters() { - return keys; - } - - @Value - static class ParentKey { - String name; - Object value; - Class targetType; - } -} diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java index 5680a51ba2..b49c5adac9 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java @@ -20,17 +20,23 @@ import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; +import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.relational.core.conversion.BasicRelationalConverter; import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import java.sql.Array; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; /** * {@link RelationalConverter} that uses a {@link MappingContext} to apply basic conversion of relational values to @@ -70,6 +76,34 @@ public BasicJdbcConverter( super(context, conversions); } + /** + * Only provided as a bridge to the old way of passing additional parameters to an insert statement. + * + * @param additionalParameters + */ + public static Identifier fromNamedValues(Map additionalParameters) { + + List keys = new ArrayList<>(); + additionalParameters.forEach((k, v) -> keys.add(new Identifier.SingleIdentifierValue(k, v, v == null ? Object.class : v.getClass()))); + + Identifier identifier = new Identifier(keys); + return identifier; + } + + /** + * Creates ParentKeys with backreference for the given path and value of the parents id. + */ + public static Identifier forBackReferences(PersistentPropertyPath path, @Nullable Object value) { + + Identifier.SingleIdentifierValue singleIdentifierValue = new Identifier.SingleIdentifierValue( // + path.getRequiredLeafProperty().getReverseColumnName(), // + value, // + getLastIdProperty(path).getColumnType() // + ); + + return new Identifier(Collections.singletonList(singleIdentifierValue)); + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.conversion.RelationalConverter#readValue(java.lang.Object, org.springframework.data.util.TypeInformation) @@ -122,4 +156,18 @@ public Object writeValue(@Nullable Object value, TypeInformation type) { return super.writeValue(value, type); } + + + private static RelationalPersistentProperty getLastIdProperty( + PersistentPropertyPath path) { + + RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty(); + + if (idProperty != null) { + return idProperty; + } + + return getLastIdProperty(path.getParentPath()); + } + } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index a056d6514e..27c04a40f6 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -26,7 +26,7 @@ import org.springframework.data.jdbc.core.DataAccessStrategy; import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; import org.springframework.data.jdbc.core.DelegatingDataAccessStrategy; -import org.springframework.data.jdbc.core.ParentKeys; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.jdbc.core.SqlGeneratorSource; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PropertyPath; @@ -142,9 +142,9 @@ public Object insert(T instance, Class domainType, Map ad * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, ParentKeys) */ @Override - public Object insert(T instance, Class domainType, ParentKeys parentKeys) { + public Object insert(T instance, Class domainType, Identifier identifier) { - MyBatisContext myBatisContext = new MyBatisContext(null, instance, domainType, parentKeys.getParametersByName()); + MyBatisContext myBatisContext = new MyBatisContext(null, instance, domainType, identifier.getParametersByName()); sqlSession().insert(namespace(domainType) + ".insert", myBatisContext); return myBatisContext.getId(); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/BasicJdbcConverterIdentifierUnitTests.java similarity index 60% rename from spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java rename to spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/BasicJdbcConverterIdentifierUnitTests.java index 4ac8345478..0b8dc82ec3 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/ParentKeysUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/BasicJdbcConverterIdentifierUnitTests.java @@ -26,14 +26,18 @@ import org.jetbrains.annotations.NotNull; import org.junit.Test; import org.springframework.data.annotation.Id; +import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; +import org.springframework.data.relational.domain.Identifier; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; /** + * Unit tests for the {@link Identifier} creating methods in the {@link BasicJdbcConverter}. + * * @author Jens Schauder */ -public class ParentKeysUnitTests { +public class BasicJdbcConverterIdentifierUnitTests { JdbcMappingContext context = new JdbcMappingContext(); @@ -44,11 +48,11 @@ public void parametersWithStringKeysUseTheValuesType() { parameters.put("one", "eins"); parameters.put("two", 2L); - ParentKeys parentKeys = ParentKeys.fromNamedValues(parameters); + Identifier identifier = BasicJdbcConverter.fromNamedValues(parameters); - assertThat(parentKeys.getParameters()).containsExactlyInAnyOrder( // - new ParentKeys.ParentKey("one", "eins", String.class), // - new ParentKeys.ParentKey("two", 2L, Long.class) // + assertThat(identifier.getParameters()).containsExactlyInAnyOrder( // + new Identifier.SingleIdentifierValue("one", "eins", String.class), // + new Identifier.SingleIdentifierValue("two", 2L, Long.class) // ); } @@ -58,20 +62,20 @@ public void parametersWithStringKeysUseObjectAsTypeForNull() { HashMap parameters = new HashMap<>(); parameters.put("one", null); - ParentKeys parentKeys = ParentKeys.fromNamedValues(parameters); + Identifier identifier = BasicJdbcConverter.fromNamedValues(parameters); - assertThat(parentKeys.getParameters()).containsExactly( // - new ParentKeys.ParentKey("one", null, Object.class) // + assertThat(identifier.getParameters()).containsExactly( // + new Identifier.SingleIdentifierValue("one", null, Object.class) // ); } @Test // DATAJDBC-326 public void parametersWithPropertyKeysUseTheParentPropertyJdbcType() { - ParentKeys parentKeys = ParentKeys.forBackReferences(getPath("child"), "eins"); + Identifier identifier = BasicJdbcConverter.forBackReferences(getPath("child"), "eins"); - assertThat(parentKeys.getParameters()).containsExactly( // - new ParentKeys.ParentKey("dummy_entity", "eins", UUID.class) // + assertThat(identifier.getParameters()).containsExactly( // + new Identifier.SingleIdentifierValue("dummy_entity", "eins", UUID.class) // ); } @@ -80,13 +84,13 @@ public void qualifiersForMaps() { PersistentPropertyPath path = getPath("children"); - ParentKeys parentKeys = ParentKeys // + Identifier identifier = BasicJdbcConverter // .forBackReferences(path, "parent-eins") // .withQualifier(path, "map-key-eins"); - assertThat(parentKeys.getParameters()).containsExactlyInAnyOrder( // - new ParentKeys.ParentKey("dummy_entity", "parent-eins", UUID.class), // - new ParentKeys.ParentKey("dummy_entity_key", "map-key-eins", String.class) // + assertThat(identifier.getParameters()).containsExactlyInAnyOrder( // + new Identifier.SingleIdentifierValue("dummy_entity", "parent-eins", UUID.class), // + new Identifier.SingleIdentifierValue("dummy_entity_key", "map-key-eins", String.class) // ); } @@ -95,23 +99,23 @@ public void qualifiersForLists() { PersistentPropertyPath path = getPath("moreChildren"); - ParentKeys parentKeys = ParentKeys // + Identifier identifier = BasicJdbcConverter // .forBackReferences(path, "parent-eins") // .withQualifier(path, "list-index-eins"); - assertThat(parentKeys.getParameters()).containsExactlyInAnyOrder( // - new ParentKeys.ParentKey("dummy_entity", "parent-eins", UUID.class), // - new ParentKeys.ParentKey("dummy_entity_key", "list-index-eins", Integer.class) // + assertThat(identifier.getParameters()).containsExactlyInAnyOrder( // + new Identifier.SingleIdentifierValue("dummy_entity", "parent-eins", UUID.class), // + new Identifier.SingleIdentifierValue("dummy_entity_key", "list-index-eins", Integer.class) // ); } @Test // DATAJDBC-326 public void backreferenceAcrossEmbeddable() { - ParentKeys parentKeys = ParentKeys.forBackReferences(getPath("embeddable.child"), "parent-eins"); + Identifier identifier = BasicJdbcConverter.forBackReferences(getPath("embeddable.child"), "parent-eins"); - assertThat(parentKeys.getParameters()).containsExactly( // - new ParentKeys.ParentKey("embeddable", "parent-eins", UUID.class) // + assertThat(identifier.getParameters()).containsExactly( // + new Identifier.SingleIdentifierValue("embeddable", "parent-eins", UUID.class) // ); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java index bffce4ed3c..5f27c2ec9b 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java @@ -19,13 +19,11 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; -import java.util.AbstractMap.SimpleEntry; -import java.util.Map; - import org.junit.Test; import org.mockito.ArgumentCaptor; import org.springframework.data.annotation.Id; -import org.springframework.data.jdbc.core.ParentKeys.ParentKey; +import org.springframework.data.relational.domain.Identifier; +import org.springframework.data.relational.domain.Identifier.SingleIdentifierValue; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.relational.core.conversion.DbAction.Insert; import org.springframework.data.relational.core.conversion.DbAction.InsertRoot; @@ -68,10 +66,10 @@ public void insertDoesHonourNamingStrategyForBackReference() { interpreter.interpret(insert); - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ParentKeys.class); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new ParentKey(BACK_REFERENCE, CONTAINER_ID, Long.class)); + assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @Test // DATAJDBC-251 @@ -81,10 +79,10 @@ public void idOfParentGetsPassedOnAsAdditionalParameterIfNoIdGotGenerated() { interpreter.interpret(insert); - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ParentKeys.class); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new ParentKey(BACK_REFERENCE, CONTAINER_ID, Long.class)); + assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @Test // DATAJDBC-251 @@ -94,10 +92,10 @@ public void generatedIdOfParentGetsPassedOnAsAdditionalParameter() { interpreter.interpret(insert); - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ParentKeys.class); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new ParentKey(BACK_REFERENCE, CONTAINER_ID, Long.class)); + assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class)); } static class Container { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java new file mode 100644 index 0000000000..b48edda4bd --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java @@ -0,0 +1,83 @@ +/* + * Copyright 2019 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 + * + * http://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.relational.domain; + +import lombok.Value; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; + +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; + +/** + * {@literal Identifier} represents a multi part id of an entity. Parts or all of the entity might not have a + * representation as a property in the entity but might only be derived from other entities referencing it. + * + * @author Jens Schauder + * @since 1.1 + */ +public class Identifier { + + private final List keys; + + public Identifier(List keys) { + + this.keys = Collections.unmodifiableList(keys); + } + + public Identifier withQualifier(PersistentPropertyPath path, Object value) { + + List keys = new ArrayList<>(this.keys); + + RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); + keys.add(new SingleIdentifierValue(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType())); + + return new Identifier(keys); + } + + @Deprecated + public Map getParametersByName() { + return new HashMap<>(); + } + + public Collection getParameters() { + return keys; + } + + public void forEach(Consumer consumer) { + getParameters().forEach(consumer); + } + + /** + * A single value of an Identifier consisting of the column name, the value and the target type which is to be used to + * store the element in the database. + * + * @author Jens Schauder + * @since 1.1 + */ + @Value + public static class SingleIdentifierValue { + String name; + Object value; + Class targetType; + } +} From eadefaee83a59cd6922a85f73ac60dea09f00484 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 27 Feb 2019 15:09:57 +0100 Subject: [PATCH 4/7] DATAJDBC-326 - Polishing. Renamed test class to match default naming pattern for unit tests. --- .../event/{IdentifierTest.java => IdentifierUnitTests.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/{IdentifierTest.java => IdentifierUnitTests.java} (97%) diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/IdentifierTest.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/IdentifierUnitTests.java similarity index 97% rename from spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/IdentifierTest.java rename to spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/IdentifierUnitTests.java index 992d295663..a740d95334 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/IdentifierTest.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/event/IdentifierUnitTests.java @@ -26,7 +26,7 @@ * * @author Jens Schauder */ -public class IdentifierTest { +public class IdentifierUnitTests { @SuppressWarnings("unchecked") @Test From 2473b3292f1ef75bf735b26f9ced84f089f999db Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 28 Feb 2019 10:58:48 +0100 Subject: [PATCH 5/7] DATAJDBC-326 - Polishing. Variables renamed to match changed class names --- .../data/jdbc/core/DefaultJdbcInterpreter.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index 1679052df5..4210b6fbe2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -148,15 +148,15 @@ private Identifier getParentKeys(DbAction.WithDependingOn action) { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(dependingOn.getEntityType()); - Object identifier = getIdFromEntityDependingOn(dependingOn, persistentEntity); - Identifier parentKeys = BasicJdbcConverter // - .forBackReferences(action.getPropertyPath(), identifier); + Object id = getIdFromEntityDependingOn(dependingOn, persistentEntity); + Identifier identifier = BasicJdbcConverter // + .forBackReferences(action.getPropertyPath(), id); for (Map.Entry, Object> qualifier : action.getQualifiers().entrySet()) { - parentKeys = parentKeys.withQualifier(qualifier.getKey(), qualifier.getValue()); + identifier = identifier.withQualifier(qualifier.getKey(), qualifier.getValue()); } - return parentKeys; + return identifier; } @Nullable From aaa894f84cd81bc7b98216ac26cf2f9afdbdd44f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 28 Feb 2019 11:13:15 +0100 Subject: [PATCH 6/7] DATAJDBC-326 - Separated construction of Identifiers from PropertyPath analysis. --- .../jdbc/core/DefaultDataAccessStrategy.java | 4 +- .../jdbc/core/DefaultJdbcInterpreter.java | 6 +- .../jdbc/core/convert/BasicJdbcConverter.java | 40 --------- .../core/convert/JdbcIdentifierBuilder.java | 89 +++++++++++++++++++ .../core/DefaultJdbcInterpreterUnitTests.java | 17 ++-- ...va => JdbcIdentifierBuilderUnitTests.java} | 77 ++++++++++------ .../data/relational/domain/Identifier.java | 32 ++++--- .../domain/IdentifierUnitTests.java | 37 ++++++++ 8 files changed, 210 insertions(+), 92 deletions(-) create mode 100644 spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java rename spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/{BasicJdbcConverterIdentifierUnitTests.java => JdbcIdentifierBuilderUnitTests.java} (57%) create mode 100644 spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index ab3b80220e..2f30444150 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -29,7 +29,7 @@ import org.springframework.dao.DataRetrievalFailureException; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; -import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; +import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder; import org.springframework.data.jdbc.support.JdbcUtil; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; @@ -89,7 +89,7 @@ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, Relation */ @Override public Object insert(T instance, Class domainType, Map additionalParameters) { - return insert(instance, domainType, BasicJdbcConverter.fromNamedValues(additionalParameters)); + return insert(instance, domainType, JdbcIdentifierBuilder.from(additionalParameters).build()); } /* diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index 4210b6fbe2..b30ca91e40 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -20,7 +20,7 @@ import java.util.Collections; import java.util.Map; -import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; +import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder; import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.conversion.DbAction; @@ -149,14 +149,14 @@ private Identifier getParentKeys(DbAction.WithDependingOn action) { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(dependingOn.getEntityType()); Object id = getIdFromEntityDependingOn(dependingOn, persistentEntity); - Identifier identifier = BasicJdbcConverter // + JdbcIdentifierBuilder identifier = JdbcIdentifierBuilder // .forBackReferences(action.getPropertyPath(), id); for (Map.Entry, Object> qualifier : action.getQualifiers().entrySet()) { identifier = identifier.withQualifier(qualifier.getKey(), qualifier.getValue()); } - return identifier; + return identifier.build(); } @Nullable diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java index b49c5adac9..e94f91bf92 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java @@ -76,34 +76,6 @@ public BasicJdbcConverter( super(context, conversions); } - /** - * Only provided as a bridge to the old way of passing additional parameters to an insert statement. - * - * @param additionalParameters - */ - public static Identifier fromNamedValues(Map additionalParameters) { - - List keys = new ArrayList<>(); - additionalParameters.forEach((k, v) -> keys.add(new Identifier.SingleIdentifierValue(k, v, v == null ? Object.class : v.getClass()))); - - Identifier identifier = new Identifier(keys); - return identifier; - } - - /** - * Creates ParentKeys with backreference for the given path and value of the parents id. - */ - public static Identifier forBackReferences(PersistentPropertyPath path, @Nullable Object value) { - - Identifier.SingleIdentifierValue singleIdentifierValue = new Identifier.SingleIdentifierValue( // - path.getRequiredLeafProperty().getReverseColumnName(), // - value, // - getLastIdProperty(path).getColumnType() // - ); - - return new Identifier(Collections.singletonList(singleIdentifierValue)); - } - /* * (non-Javadoc) * @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) { } - private static RelationalPersistentProperty getLastIdProperty( - PersistentPropertyPath path) { - - RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty(); - - if (idProperty != null) { - return idProperty; - } - - return getLastIdProperty(path.getParentPath()); - } - } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java new file mode 100644 index 0000000000..181bddbbc4 --- /dev/null +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java @@ -0,0 +1,89 @@ +/* + * Copyright 2019 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 + * + * http://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.core.convert; + +import java.util.Map; + +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; +import org.springframework.lang.Nullable; + +/** + * @author Jens Schauder + * @since 1.1 + */ +public class JdbcIdentifierBuilder { + + private Identifier identifier; + + private JdbcIdentifierBuilder(Identifier identifier) { + this.identifier = identifier; + } + + public static JdbcIdentifierBuilder empty() { + return new JdbcIdentifierBuilder(Identifier.empty()); + } + + public static JdbcIdentifierBuilder from(Map additionalParameters) { + + Identifier[] identifier = new Identifier[] { Identifier.empty() }; + + additionalParameters + .forEach((k, v) -> identifier[0] = identifier[0].add(k, v, v == null ? Object.class : v.getClass())); + + return new JdbcIdentifierBuilder(identifier[0]); + } + + /** + * Creates ParentKeys with backreference for the given path and value of the parents id. + */ + public static JdbcIdentifierBuilder forBackReferences(PersistentPropertyPath path, + @Nullable Object value) { + + Identifier identifier = Identifier.simple( // + path.getRequiredLeafProperty().getReverseColumnName(), // + value, // + getLastIdProperty(path).getColumnType() // + ); + + return new JdbcIdentifierBuilder(identifier); + } + + public JdbcIdentifierBuilder withQualifier(PersistentPropertyPath path, Object value) { + + RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); + identifier = identifier.add(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType()); + + return this; + } + + public Identifier build() { + return identifier; + } + + private static RelationalPersistentProperty getLastIdProperty( + PersistentPropertyPath path) { + + RelationalPersistentProperty idProperty = path.getRequiredLeafProperty().getOwner().getIdProperty(); + + if (idProperty != null) { + return idProperty; + } + + return getLastIdProperty(path.getParentPath()); + } +} diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java index 5f27c2ec9b..be9a14ae90 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java @@ -16,20 +16,19 @@ package org.springframework.data.jdbc.core; import static org.assertj.core.api.Assertions.*; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.springframework.data.annotation.Id; -import org.springframework.data.relational.domain.Identifier; -import org.springframework.data.relational.domain.Identifier.SingleIdentifierValue; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.relational.core.conversion.DbAction.Insert; import org.springframework.data.relational.core.conversion.DbAction.InsertRoot; import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; /** * Unit tests for {@link DefaultJdbcInterpreter} @@ -69,7 +68,9 @@ public void insertDoesHonourNamingStrategyForBackReference() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class)); + assertThat(argumentCaptor.getValue().getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @Test // DATAJDBC-251 @@ -82,7 +83,9 @@ public void idOfParentGetsPassedOnAsAdditionalParameterIfNoIdGotGenerated() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class)); + assertThat(argumentCaptor.getValue().getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @Test // DATAJDBC-251 @@ -95,7 +98,9 @@ public void generatedIdOfParentGetsPassedOnAsAdditionalParameter() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()).containsExactly(new SingleIdentifierValue(BACK_REFERENCE, CONTAINER_ID, Long.class)); + assertThat(argumentCaptor.getValue().getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class)); } static class Container { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/BasicJdbcConverterIdentifierUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java similarity index 57% rename from spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/BasicJdbcConverterIdentifierUnitTests.java rename to spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java index 0b8dc82ec3..20c3454fec 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/BasicJdbcConverterIdentifierUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java @@ -18,26 +18,29 @@ import static org.assertj.core.api.Assertions.*; import static org.springframework.data.jdbc.core.PropertyPathUtils.*; +import java.util.AbstractMap; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; +import org.assertj.core.groups.Tuple; import org.jetbrains.annotations.NotNull; import org.junit.Test; import org.springframework.data.annotation.Id; -import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; -import org.springframework.data.relational.domain.Identifier; +import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; /** - * Unit tests for the {@link Identifier} creating methods in the {@link BasicJdbcConverter}. + * Unit tests for the {@link JdbcIdentifierBuilder}. * * @author Jens Schauder */ -public class BasicJdbcConverterIdentifierUnitTests { +public class JdbcIdentifierBuilderUnitTests { JdbcMappingContext context = new JdbcMappingContext(); @@ -48,11 +51,13 @@ public void parametersWithStringKeysUseTheValuesType() { parameters.put("one", "eins"); parameters.put("two", 2L); - Identifier identifier = BasicJdbcConverter.fromNamedValues(parameters); + Identifier identifier = JdbcIdentifierBuilder.from(parameters).build(); - assertThat(identifier.getParameters()).containsExactlyInAnyOrder( // - new Identifier.SingleIdentifierValue("one", "eins", String.class), // - new Identifier.SingleIdentifierValue("two", 2L, Long.class) // + assertThat(identifier.getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactlyInAnyOrder( // + tuple("one", "eins", String.class), // + tuple("two", 2L, Long.class) // ); } @@ -62,20 +67,24 @@ public void parametersWithStringKeysUseObjectAsTypeForNull() { HashMap parameters = new HashMap<>(); parameters.put("one", null); - Identifier identifier = BasicJdbcConverter.fromNamedValues(parameters); + Identifier identifier = JdbcIdentifierBuilder.from(parameters).build(); - assertThat(identifier.getParameters()).containsExactly( // - new Identifier.SingleIdentifierValue("one", null, Object.class) // + assertThat(identifier.getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactly( // + tuple("one", null, Object.class) // ); } @Test // DATAJDBC-326 public void parametersWithPropertyKeysUseTheParentPropertyJdbcType() { - Identifier identifier = BasicJdbcConverter.forBackReferences(getPath("child"), "eins"); + Identifier identifier = JdbcIdentifierBuilder.forBackReferences(getPath("child"), "eins").build(); - assertThat(identifier.getParameters()).containsExactly( // - new Identifier.SingleIdentifierValue("dummy_entity", "eins", UUID.class) // + assertThat(identifier.getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactly( // + tuple("dummy_entity", "eins", UUID.class) // ); } @@ -84,13 +93,16 @@ public void qualifiersForMaps() { PersistentPropertyPath path = getPath("children"); - Identifier identifier = BasicJdbcConverter // + Identifier identifier = JdbcIdentifierBuilder // .forBackReferences(path, "parent-eins") // - .withQualifier(path, "map-key-eins"); - - assertThat(identifier.getParameters()).containsExactlyInAnyOrder( // - new Identifier.SingleIdentifierValue("dummy_entity", "parent-eins", UUID.class), // - new Identifier.SingleIdentifierValue("dummy_entity_key", "map-key-eins", String.class) // + .withQualifier(path, "map-key-eins") // + .build(); + + assertThat(identifier.getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactlyInAnyOrder( // + tuple("dummy_entity", "parent-eins", UUID.class), // + tuple("dummy_entity_key", "map-key-eins", String.class) // ); } @@ -99,23 +111,30 @@ public void qualifiersForLists() { PersistentPropertyPath path = getPath("moreChildren"); - Identifier identifier = BasicJdbcConverter // + Identifier identifier = JdbcIdentifierBuilder // .forBackReferences(path, "parent-eins") // - .withQualifier(path, "list-index-eins"); - - assertThat(identifier.getParameters()).containsExactlyInAnyOrder( // - new Identifier.SingleIdentifierValue("dummy_entity", "parent-eins", UUID.class), // - new Identifier.SingleIdentifierValue("dummy_entity_key", "list-index-eins", Integer.class) // + .withQualifier(path, "list-index-eins") // + .build(); + + assertThat(identifier.getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactlyInAnyOrder( // + tuple("dummy_entity", "parent-eins", UUID.class), // + tuple("dummy_entity_key", "list-index-eins", Integer.class) // ); } @Test // DATAJDBC-326 public void backreferenceAcrossEmbeddable() { - Identifier identifier = BasicJdbcConverter.forBackReferences(getPath("embeddable.child"), "parent-eins"); + Identifier identifier = JdbcIdentifierBuilder // + .forBackReferences(getPath("embeddable.child"), "parent-eins") // + .build(); - assertThat(identifier.getParameters()).containsExactly( // - new Identifier.SingleIdentifierValue("embeddable", "parent-eins", UUID.class) // + assertThat(identifier.getParameters()) // + .extracting("name", "value", "targetType") // + .containsExactly( // + tuple("embeddable", "parent-eins", UUID.class) // ); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java index b48edda4bd..3afbeadc2c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java @@ -15,6 +15,8 @@ */ package org.springframework.data.relational.domain; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; import lombok.Value; import java.util.ArrayList; @@ -25,9 +27,6 @@ import java.util.Map; import java.util.function.Consumer; -import org.springframework.data.mapping.PersistentPropertyPath; -import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; - /** * {@literal Identifier} represents a multi part id of an entity. Parts or all of the entity might not have a * representation as a property in the entity but might only be derived from other entities referencing it. @@ -35,28 +34,35 @@ * @author Jens Schauder * @since 1.1 */ -public class Identifier { +public final class Identifier { private final List keys; - public Identifier(List keys) { - - this.keys = Collections.unmodifiableList(keys); + private Identifier(List keys) { + this.keys = keys; } - public Identifier withQualifier(PersistentPropertyPath path, Object value) { + static public Identifier empty() { + return new Identifier(Collections.emptyList()); + } - List keys = new ArrayList<>(this.keys); + static public Identifier simple(String name, Object value, Class targetType) { + return new Identifier(Collections.singletonList(new SingleIdentifierValue(name, value, targetType))); + } - RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); - keys.add(new SingleIdentifierValue(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType())); + public Identifier add(String name, Object value, Class targetType) { + List keys = new ArrayList<>(this.keys); + keys.add(new SingleIdentifierValue(name, value, targetType)); return new Identifier(keys); } @Deprecated public Map getParametersByName() { - return new HashMap<>(); + + HashMap result = new HashMap<>(); + forEach(v -> result.put(v.name, v.value)); + return result; } public Collection getParameters() { @@ -75,7 +81,9 @@ public void forEach(Consumer consumer) { * @since 1.1 */ @Value + @AllArgsConstructor(access = AccessLevel.PRIVATE) public static class SingleIdentifierValue { + String name; Object value; Class targetType; diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java new file mode 100644 index 0000000000..488b839a23 --- /dev/null +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java @@ -0,0 +1,37 @@ +/* + * Copyright 2019 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 + * + * http://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.relational.domain; + +import org.junit.Test; + +import java.util.AbstractMap; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Jens Schauder + */ +public class IdentifierUnitTests { + + @Test // DATAJDBC-326 + public void getParametersByName() { + + Identifier identifier = Identifier.simple("aName", "aValue", String.class);; + + assertThat(identifier.getParametersByName()) + .containsExactly(new AbstractMap.SimpleEntry<>("aName", "aValue")); + } +} \ No newline at end of file From bd05db9b12489717affa14929e56ed6147cc6077 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 5 Mar 2019 15:08:38 +0100 Subject: [PATCH 7/7] DATAJDBC-326 - Polishing. Add Javadoc. Tweak naming and factory methods. Add equals/hashcode to Identifier. Extend tests. --- .../data/jdbc/core/DataAccessStrategy.java | 14 +- .../jdbc/core/DefaultDataAccessStrategy.java | 20 +- .../jdbc/core/DefaultJdbcInterpreter.java | 7 +- .../core/convert/JdbcIdentifierBuilder.java | 18 +- .../mybatis/MyBatisDataAccessStrategy.java | 29 +-- .../core/DefaultJdbcInterpreterUnitTests.java | 7 +- .../core/JdbcIdentifierBuilderUnitTests.java | 45 +---- .../data/relational/domain/Identifier.java | 173 +++++++++++++++--- .../domain/IdentifierUnitTests.java | 81 +++++++- 9 files changed, 268 insertions(+), 126 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java index ce731ce145..1ff274c26d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java @@ -17,9 +17,9 @@ import java.util.Map; -import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; import org.springframework.lang.Nullable; /** @@ -40,25 +40,25 @@ public interface DataAccessStrategy { * to get referenced are contained in this map. Must not be {@code null}. * @param the type of the instance. * @return the id generated by the database if any. - * - * @deprecated use {@link #insert(Object, Class, Identifier)} instead. + * @deprecated since 1.1, use {@link #insert(Object, Class, Identifier)} instead. */ @Deprecated Object insert(T instance, Class domainType, Map additionalParameters); - /** * Inserts a the data of a single entity. Referenced entities don't get handled. * * @param instance the instance to be stored. Must not be {@code null}. * @param domainType the type of the instance. Must not be {@code null}. - * @param identifier information about data that needs to be considered for the insert but which is not part of the entity. - * Namely references back to a parent entity and key/index columns for entities that are stored in a {@link Map} or {@link java.util.List}. + * @param identifier information about data that needs to be considered for the insert but which is not part of the + * entity. Namely references back to a parent entity and key/index columns for entities that are stored in a + * {@link Map} or {@link java.util.List}. * @param the type of the instance. * @return the id generated by the database if any. + * @since 1.1 */ default Object insert(T instance, Class domainType, Identifier identifier){ - return insert(instance, domainType, identifier.getParametersByName()); + return insert(instance, domainType, identifier.toMap()); } /** diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 2f30444150..4b3a48cfc0 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -29,7 +29,6 @@ import org.springframework.dao.DataRetrievalFailureException; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; -import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder; import org.springframework.data.jdbc.support.JdbcUtil; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; @@ -59,10 +58,6 @@ @RequiredArgsConstructor public class DefaultDataAccessStrategy implements DataAccessStrategy { - private static final String ENTITY_NEW_AFTER_INSERT = "Entity [%s] still 'new' after insert. Please set either" - + " the id property in a BeforeInsert event handler, or ensure the database creates a value and your " - + "JDBC driver returns it."; - private final @NonNull SqlGeneratorSource sqlGeneratorSource; private final @NonNull RelationalMappingContext context; private final @NonNull RelationalConverter converter; @@ -89,7 +84,7 @@ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, Relation */ @Override public Object insert(T instance, Class domainType, Map additionalParameters) { - return insert(instance, domainType, JdbcIdentifierBuilder.from(additionalParameters).build()); + return insert(instance, domainType, Identifier.from(additionalParameters)); } /* @@ -102,10 +97,9 @@ public Object insert(T instance, Class domainType, Identifier identifier) KeyHolder holder = new GeneratedKeyHolder(); RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); - Map parameters = new LinkedHashMap<>(); - identifier.forEach(identifierValue -> { - parameters.put(identifierValue.getName(), - converter.writeValue(identifierValue.getValue(), ClassTypeInformation.from(identifierValue.getTargetType()))); + Map parameters = new LinkedHashMap<>(identifier.size()); + identifier.forEach((name, value, type) -> { + parameters.put(name, converter.writeValue(value, ClassTypeInformation.from(type))); }); MapSqlParameterSource parameterSource = getPropertyMap(instance, persistentEntity, ""); @@ -298,7 +292,7 @@ public boolean existsById(Object id, Class domainType) { return result; } - private MapSqlParameterSource getPropertyMap(final S instance, RelationalPersistentEntity persistentEntity, + private MapSqlParameterSource getPropertyMap(S instance, RelationalPersistentEntity persistentEntity, String prefix) { MapSqlParameterSource parameters = new MapSqlParameterSource(); @@ -314,8 +308,8 @@ private MapSqlParameterSource getPropertyMap(final S instance, Relational if (property.isEmbedded()) { Object value = propertyAccessor.getProperty(property); - final RelationalPersistentEntity embeddedEntity = context.getPersistentEntity(property.getType()); - final MapSqlParameterSource additionalParameters = getPropertyMap((T) value, + RelationalPersistentEntity embeddedEntity = context.getPersistentEntity(property.getType()); + MapSqlParameterSource additionalParameters = getPropertyMap((T) value, (RelationalPersistentEntity) embeddedEntity, prefix + property.getEmbeddedPrefix()); parameters.addValues(additionalParameters.getValues()); } else { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index b30ca91e40..ab15fd7129 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -21,7 +21,6 @@ import java.util.Map; import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder; -import org.springframework.data.relational.domain.Identifier; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.conversion.DbAction; import org.springframework.data.relational.core.conversion.DbAction.Delete; @@ -37,6 +36,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; import org.springframework.lang.Nullable; /** @@ -142,7 +142,7 @@ public void interpret(DeleteAllRoot deleteAllRoot) { accessStrategy.deleteAll(deleteAllRoot.getEntityType()); } - private Identifier getParentKeys(DbAction.WithDependingOn action) { + private Identifier getParentKeys(DbAction.WithDependingOn action) { DbAction.WithEntity dependingOn = action.getDependingOn(); @@ -152,7 +152,8 @@ private Identifier getParentKeys(DbAction.WithDependingOn action) { JdbcIdentifierBuilder identifier = JdbcIdentifierBuilder // .forBackReferences(action.getPropertyPath(), id); - for (Map.Entry, Object> qualifier : action.getQualifiers().entrySet()) { + for (Map.Entry, Object> qualifier : action.getQualifiers() + .entrySet()) { identifier = identifier.withQualifier(qualifier.getKey(), qualifier.getValue()); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java index 181bddbbc4..1757df32f1 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java @@ -15,14 +15,14 @@ */ package org.springframework.data.jdbc.core.convert; -import java.util.Map; - import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.domain.Identifier; import org.springframework.lang.Nullable; /** + * Builder for {@link Identifier}. Mainly for internal use within the framework + * * @author Jens Schauder * @since 1.1 */ @@ -38,23 +38,13 @@ public static JdbcIdentifierBuilder empty() { return new JdbcIdentifierBuilder(Identifier.empty()); } - public static JdbcIdentifierBuilder from(Map additionalParameters) { - - Identifier[] identifier = new Identifier[] { Identifier.empty() }; - - additionalParameters - .forEach((k, v) -> identifier[0] = identifier[0].add(k, v, v == null ? Object.class : v.getClass())); - - return new JdbcIdentifierBuilder(identifier[0]); - } - /** * Creates ParentKeys with backreference for the given path and value of the parents id. */ public static JdbcIdentifierBuilder forBackReferences(PersistentPropertyPath path, @Nullable Object value) { - Identifier identifier = Identifier.simple( // + Identifier identifier = Identifier.of( // path.getRequiredLeafProperty().getReverseColumnName(), // value, // getLastIdProperty(path).getColumnType() // @@ -66,7 +56,7 @@ public static JdbcIdentifierBuilder forBackReferences(PersistentPropertyPath path, Object value) { RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); - identifier = identifier.add(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType()); + identifier = identifier.withPart(leafProperty.getKeyColumn(), value, leafProperty.getQualifierColumnType()); return this; } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 27c04a40f6..0677a15e3e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -22,17 +22,18 @@ import org.apache.ibatis.session.SqlSession; import org.mybatis.spring.SqlSessionTemplate; + import org.springframework.data.jdbc.core.CascadingDataAccessStrategy; import org.springframework.data.jdbc.core.DataAccessStrategy; import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; import org.springframework.data.jdbc.core.DelegatingDataAccessStrategy; -import org.springframework.data.relational.domain.Identifier; import org.springframework.data.jdbc.core.SqlGeneratorSource; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.Identifier; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.util.Assert; @@ -124,7 +125,7 @@ public void setNamespaceStrategy(NamespaceStrategy namespaceStrategy) { this.namespaceStrategy = namespaceStrategy; } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) */ @@ -144,13 +145,13 @@ public Object insert(T instance, Class domainType, Map ad @Override public Object insert(T instance, Class domainType, Identifier identifier) { - MyBatisContext myBatisContext = new MyBatisContext(null, instance, domainType, identifier.getParametersByName()); + MyBatisContext myBatisContext = new MyBatisContext(null, instance, domainType, identifier.toMap()); sqlSession().insert(namespace(domainType) + ".insert", myBatisContext); return myBatisContext.getId(); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#update(java.lang.Object, java.lang.Class) */ @@ -161,7 +162,7 @@ public boolean update(S instance, Class domainType) { new MyBatisContext(null, instance, domainType, Collections.emptyMap())) != 0; } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#delete(java.lang.Object, java.lang.Class) */ @@ -172,7 +173,7 @@ public void delete(Object id, Class domainType) { new MyBatisContext(id, null, domainType, Collections.emptyMap())); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#delete(java.lang.Object, org.springframework.data.mapping.PersistentPropertyPath) */ @@ -185,7 +186,7 @@ public void delete(Object rootId, PersistentPropertyPath void deleteAll(Class domainType) { ); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteAll(org.springframework.data.mapping.PersistentPropertyPath) */ @@ -214,7 +215,7 @@ public void deleteAll(PersistentPropertyPath prope ); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) */ @@ -224,7 +225,7 @@ public T findById(Object id, Class domainType) { new MyBatisContext(id, null, domainType, Collections.emptyMap())); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class) */ @@ -234,7 +235,7 @@ public Iterable findAll(Class domainType) { new MyBatisContext(null, null, domainType, Collections.emptyMap())); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAllById(java.lang.Iterable, java.lang.Class) */ @@ -244,7 +245,7 @@ public Iterable findAllById(Iterable ids, Class domainType) { new MyBatisContext(ids, null, domainType, Collections.emptyMap())); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAllByProperty(java.lang.Object, org.springframework.data.relational.core.mapping.RelationalPersistentProperty) */ @@ -255,7 +256,7 @@ public Iterable findAllByProperty(Object rootId, RelationalPersistentProp new MyBatisContext(rootId, null, property.getType(), Collections.emptyMap())); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#existsById(java.lang.Object, java.lang.Class) */ @@ -265,7 +266,7 @@ public boolean existsById(Object id, Class domainType) { new MyBatisContext(id, null, domainType, Collections.emptyMap())); } - /* + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class) */ diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java index be9a14ae90..164a8b3355 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java @@ -21,6 +21,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; + import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.relational.core.conversion.DbAction.Insert; @@ -68,7 +69,7 @@ public void insertDoesHonourNamingStrategyForBackReference() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()) // + assertThat(argumentCaptor.getValue().getParts()) // .extracting("name", "value", "targetType") // .containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @@ -83,7 +84,7 @@ public void idOfParentGetsPassedOnAsAdditionalParameterIfNoIdGotGenerated() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()) // + assertThat(argumentCaptor.getValue().getParts()) // .extracting("name", "value", "targetType") // .containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class)); } @@ -98,7 +99,7 @@ public void generatedIdOfParentGetsPassedOnAsAdditionalParameter() { ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Identifier.class); verify(dataAccessStrategy).insert(eq(element), eq(Element.class), argumentCaptor.capture()); - assertThat(argumentCaptor.getValue().getParameters()) // + assertThat(argumentCaptor.getValue().getParts()) // .extracting("name", "value", "targetType") // .containsExactly(tuple(BACK_REFERENCE, CONTAINER_ID, Long.class)); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java index 20c3454fec..d7213940e9 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcIdentifierBuilderUnitTests.java @@ -18,16 +18,13 @@ import static org.assertj.core.api.Assertions.*; import static org.springframework.data.jdbc.core.PropertyPathUtils.*; -import java.util.AbstractMap; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; -import org.assertj.core.groups.Tuple; import org.jetbrains.annotations.NotNull; import org.junit.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.convert.JdbcIdentifierBuilder; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; @@ -44,44 +41,12 @@ public class JdbcIdentifierBuilderUnitTests { JdbcMappingContext context = new JdbcMappingContext(); - @Test // DATAJDBC-326 - public void parametersWithStringKeysUseTheValuesType() { - - HashMap parameters = new HashMap<>(); - parameters.put("one", "eins"); - parameters.put("two", 2L); - - Identifier identifier = JdbcIdentifierBuilder.from(parameters).build(); - - assertThat(identifier.getParameters()) // - .extracting("name", "value", "targetType") // - .containsExactlyInAnyOrder( // - tuple("one", "eins", String.class), // - tuple("two", 2L, Long.class) // - ); - } - - @Test // DATAJDBC-326 - public void parametersWithStringKeysUseObjectAsTypeForNull() { - - HashMap parameters = new HashMap<>(); - parameters.put("one", null); - - Identifier identifier = JdbcIdentifierBuilder.from(parameters).build(); - - assertThat(identifier.getParameters()) // - .extracting("name", "value", "targetType") // - .containsExactly( // - tuple("one", null, Object.class) // - ); - } - @Test // DATAJDBC-326 public void parametersWithPropertyKeysUseTheParentPropertyJdbcType() { Identifier identifier = JdbcIdentifierBuilder.forBackReferences(getPath("child"), "eins").build(); - assertThat(identifier.getParameters()) // + assertThat(identifier.getParts()) // .extracting("name", "value", "targetType") // .containsExactly( // tuple("dummy_entity", "eins", UUID.class) // @@ -98,7 +63,7 @@ public void qualifiersForMaps() { .withQualifier(path, "map-key-eins") // .build(); - assertThat(identifier.getParameters()) // + assertThat(identifier.getParts()) // .extracting("name", "value", "targetType") // .containsExactlyInAnyOrder( // tuple("dummy_entity", "parent-eins", UUID.class), // @@ -116,7 +81,7 @@ public void qualifiersForLists() { .withQualifier(path, "list-index-eins") // .build(); - assertThat(identifier.getParameters()) // + assertThat(identifier.getParts()) // .extracting("name", "value", "targetType") // .containsExactlyInAnyOrder( // tuple("dummy_entity", "parent-eins", UUID.class), // @@ -131,7 +96,7 @@ public void backreferenceAcrossEmbeddable() { .forBackReferences(getPath("embeddable.child"), "parent-eins") // .build(); - assertThat(identifier.getParameters()) // + assertThat(identifier.getParts()) // .extracting("name", "value", "targetType") // .containsExactly( // tuple("embeddable", "parent-eins", UUID.class) // diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java index 3afbeadc2c..18dc6c6b8d 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/domain/Identifier.java @@ -17,60 +17,165 @@ import lombok.AccessLevel; import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.ToString; import lombok.Value; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.function.Consumer; + +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; /** - * {@literal Identifier} represents a multi part id of an entity. Parts or all of the entity might not have a - * representation as a property in the entity but might only be derived from other entities referencing it. - * + * {@literal Identifier} represents a composite id of an entity that may be composed of one or many parts. Parts or all + * of the entity might not have a representation as a property in the entity but might only be derived from other + * entities referencing it. + * * @author Jens Schauder + * @author Mark Paluch * @since 1.1 */ +@EqualsAndHashCode +@ToString public final class Identifier { - private final List keys; + private static final Identifier EMPTY = new Identifier(Collections.emptyList()); - private Identifier(List keys) { - this.keys = keys; + private final List parts; + + private Identifier(List parts) { + this.parts = parts; } - static public Identifier empty() { - return new Identifier(Collections.emptyList()); + /** + * Returns an empty {@link Identifier}. + * + * @return an empty {@link Identifier}. + */ + public static Identifier empty() { + return EMPTY; } - static public Identifier simple(String name, Object value, Class targetType) { + /** + * Creates an {@link Identifier} from {@code name}, {@code value}, and a {@link Class target type}. + * + * @param name must not be {@literal null} or empty. + * @param value + * @param targetType must not be {@literal null}. + * @return the {@link Identifier} for {@code name}, {@code value}, and a {@link Class target type}. + */ + public static Identifier of(String name, Object value, Class targetType) { + + Assert.hasText(name, "Name must not be empty!"); + Assert.notNull(targetType, "Target type must not be null!"); + return new Identifier(Collections.singletonList(new SingleIdentifierValue(name, value, targetType))); } - public Identifier add(String name, Object value, Class targetType) { + /** + * Creates an {@link Identifier} from a {@link Map} of name to value tuples. + * + * @param map must not be {@literal null}. + * @return the {@link Identifier} from a {@link Map} of name to value tuples. + */ + public static Identifier from(Map map) { + + Assert.notNull(map, "Map must not be null!"); + + if (map.isEmpty()) { + return empty(); + } + + List values = new ArrayList<>(); - List keys = new ArrayList<>(this.keys); - keys.add(new SingleIdentifierValue(name, value, targetType)); - return new Identifier(keys); + map.forEach((k, v) -> { + + values.add(new SingleIdentifierValue(k, v, v != null ? ClassUtils.getUserClass(v) : Object.class)); + }); + + return new Identifier(Collections.unmodifiableList(values)); } - @Deprecated - public Map getParametersByName() { + /** + * Creates a new {@link Identifier} from the current instance and sets the value for {@code key}. Existing key + * definitions for {@code name} are overwritten if they already exist. + * + * @param name must not be {@literal null} or empty. + * @param value + * @param targetType must not be {@literal null}. + * @return the {@link Identifier} containing all existing keys and the key part for {@code name}, {@code value}, and a + * {@link Class target type}. + */ + public Identifier withPart(String name, Object value, Class targetType) { + + Assert.hasText(name, "Name must not be empty!"); + Assert.notNull(targetType, "Target type must not be null!"); + + boolean overwritten = false; + List keys = new ArrayList<>(this.parts.size() + 1); + + for (SingleIdentifierValue singleValue : this.parts) { + + if (singleValue.getName().equals(name)) { + overwritten = true; + keys.add(new SingleIdentifierValue(singleValue.getName(), value, targetType)); + } else { + keys.add(singleValue); + } + } - HashMap result = new HashMap<>(); - forEach(v -> result.put(v.name, v.value)); + if (!overwritten) { + keys.add(new SingleIdentifierValue(name, value, targetType)); + } + + return new Identifier(Collections.unmodifiableList(keys)); + } + + /** + * Returns a {@link Map} containing the identifier name to value tuples. + * + * @return a {@link Map} containing the identifier name to value tuples. + */ + public Map toMap() { + + Map result = new LinkedHashMap<>(); + forEach((name, value, type) -> result.put(name, value)); return result; } - public Collection getParameters() { - return keys; + /** + * @return the {@link SingleIdentifierValue key parts}. + */ + public Collection getParts() { + return this.parts; } - public void forEach(Consumer consumer) { - getParameters().forEach(consumer); + /** + * Performs the given action for each element of the {@link Identifier} until all elements have been processed or the + * action throws an exception. Unless otherwise specified by the implementing class, actions are performed in the + * order of iteration (if an iteration order is specified). Exceptions thrown by the action are relayed to the caller. + * + * @param consumer the action, must not be {@literal null}. + */ + public void forEach(IdentifierConsumer consumer) { + + Assert.notNull(consumer, "IdentifierConsumer must not be null"); + + getParts().forEach(it -> consumer.accept(it.name, it.value, it.targetType)); + } + + /** + * Returns the number of key parts in this collection. + * + * @return the number of key parts in this collection. + */ + public int size() { + return this.parts.size(); } /** @@ -78,14 +183,32 @@ public void forEach(Consumer consumer) { * store the element in the database. * * @author Jens Schauder - * @since 1.1 */ @Value @AllArgsConstructor(access = AccessLevel.PRIVATE) - public static class SingleIdentifierValue { + static class SingleIdentifierValue { String name; Object value; Class targetType; } + + /** + * Represents an operation that accepts identifier key parts (name, value and {@link Class target type}) defining a + * contract to consume {@link Identifier} values. + * + * @author Mark Paluch + */ + @FunctionalInterface + public interface IdentifierConsumer { + + /** + * Performs this operation on the given arguments. + * + * @param name + * @param value + * @param targetType + */ + void accept(String name, Object value, Class targetType); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java index 488b839a23..57f58f6a7f 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/domain/IdentifierUnitTests.java @@ -15,23 +15,90 @@ */ package org.springframework.data.relational.domain; -import org.junit.Test; +import static org.assertj.core.api.Assertions.*; -import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; -import static org.assertj.core.api.Assertions.assertThat; +import org.junit.Test; /** + * Unit tests for {@link Identifier}. + * * @author Jens Schauder + * @author Mark Paluch */ public class IdentifierUnitTests { @Test // DATAJDBC-326 public void getParametersByName() { - Identifier identifier = Identifier.simple("aName", "aValue", String.class);; + Identifier identifier = Identifier.of("aName", "aValue", String.class); + + assertThat(identifier.toMap()).hasSize(1).containsEntry("aName", "aValue"); + } + + @Test // DATAJDBC-326 + public void parametersWithStringKeysUseObjectAsTypeForNull() { + + HashMap parameters = new HashMap<>(); + parameters.put("one", null); + + Identifier identifier = Identifier.from(parameters); + + assertThat(identifier.getParts()) // + .extracting("name", "value", "targetType") // + .containsExactly( // + tuple("one", null, Object.class) // + ); + } + + @Test // DATAJDBC-326 + public void createsIdentifierFromMap() { + + Identifier identifier = Identifier.from(Collections.singletonMap("aName", "aValue")); + + assertThat(identifier.toMap()).hasSize(1).containsEntry("aName", "aValue"); + } + + @Test // DATAJDBC-326 + public void withAddsNewEntries() { + + Identifier identifier = Identifier.from(Collections.singletonMap("aName", "aValue")).withPart("foo", "bar", + String.class); + + assertThat(identifier.toMap()).hasSize(2).containsEntry("aName", "aValue").containsEntry("foo", "bar"); + } + + @Test // DATAJDBC-326 + public void withOverridesExistingEntries() { + + Identifier identifier = Identifier.from(Collections.singletonMap("aName", "aValue")).withPart("aName", "bar", + String.class); + + assertThat(identifier.toMap()).hasSize(1).containsEntry("aName", "bar"); + } + + @Test // DATAJDBC-326 + public void forEachIteratesOverKeys() { + + List keys = new ArrayList<>(); + + Identifier.from(Collections.singletonMap("aName", "aValue")).forEach((name, value, targetType) -> keys.add(name)); + + assertThat(keys).containsOnly("aName"); + } + + @Test // DATAJDBC-326 + public void equalsConsidersEquality() { + + Identifier one = Identifier.from(Collections.singletonMap("aName", "aValue")); + Identifier two = Identifier.from(Collections.singletonMap("aName", "aValue")); + Identifier three = Identifier.from(Collections.singletonMap("aName", "different")); - assertThat(identifier.getParametersByName()) - .containsExactly(new AbstractMap.SimpleEntry<>("aName", "aValue")); + assertThat(one).isEqualTo(two); + assertThat(one).isNotEqualTo(three); } -} \ No newline at end of file +}