From 3b0c140ebb6545cdaa12666d224f4dd2106fbdc6 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 9 Oct 2017 07:17:23 +0200 Subject: [PATCH 1/3] DATAJDBC-131 - Prepare branch --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7fc8f36b75..dd40bf4559 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jdbc - 1.0.0.BUILD-SNAPSHOT + 1.0.0.DATAJDBC-131-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. From 1bc8bfd44d9accbecedd979519a38b0f5862d64e Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 9 Oct 2017 07:55:58 +0200 Subject: [PATCH 2/3] DATAJDBC-131 - Basic support for Maps Aggregate roots with properties of type java.util.Map get properly inserted, updated and deleted. Known limitations: - Naming strategy does not allow for multiple references via Set, Map or directly to the same entity. - The table for the referenced Entity contains the column for the map key. A workaround for that would be to manipulate the DbActions in the AggregateChange yourself. --- .../data/jdbc/core/DataAccessStrategy.java | 2 +- .../jdbc/core/DefaultDataAccessStrategy.java | 20 +- .../jdbc/core/DefaultJdbcInterpreter.java | 9 +- .../data/jdbc/core/EntityRowMapper.java | 6 + .../core/IterableOfEntryToMapConverter.java | 74 ++++++ .../data/jdbc/core/MapEntityRowMapper.java | 49 ++++ .../data/jdbc/core/SqlGenerator.java | 32 ++- .../data/jdbc/core/conversion/DbAction.java | 10 + .../core/conversion/JdbcEntityWriter.java | 87 +++++-- .../model/BasicJdbcPersistentProperty.java | 9 +- .../mapping/model/DefaultNamingStrategy.java | 10 + .../mapping/model/JdbcPersistentProperty.java | 2 + .../jdbc/mapping/model/NamingStrategy.java | 13 + .../jdbc/core/EntityRowMapperUnitTests.java | 58 ++++- ...terableOfEntryToMapConverterUnitTests.java | 73 ++++++ .../data/jdbc/core/SqlGeneratorUnitTests.java | 27 +++ .../conversion/JdbcEntityWriterUnitTests.java | 75 +++++- ...dbcRepositoryWithMapsIntegrationTests.java | 228 ++++++++++++++++++ ...epositoryWithMapsIntegrationTests-hsql.sql | 2 + ...positoryWithMapsIntegrationTests-mysql.sql | 2 + ...itoryWithMapsIntegrationTests-postgres.sql | 4 + 21 files changed, 744 insertions(+), 48 deletions(-) create mode 100644 src/main/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverter.java create mode 100644 src/main/java/org/springframework/data/jdbc/core/MapEntityRowMapper.java create mode 100644 src/test/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverterUnitTests.java create mode 100644 src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java create mode 100644 src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql create mode 100644 src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-mysql.sql create mode 100644 src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-postgres.sql diff --git a/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java index 61f9b90ec7..dba854a348 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java @@ -29,7 +29,7 @@ public interface DataAccessStrategy { void insert(T instance, Class domainType, Map additionalParameters); - void update(S instance, Class domainType); + void update(T instance, Class domainType); void delete(Object id, Class domainType); diff --git a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 81ebcac45e..5c13619fc1 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -37,6 +37,7 @@ import org.springframework.data.mapping.PropertyHandler; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.repository.core.EntityInformation; +import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.jdbc.support.GeneratedKeyHolder; @@ -71,7 +72,6 @@ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, NamedPar /** * Creates a {@link DefaultDataAccessStrategy} which references it self for resolution of recursive data accesses. - * * Only suitable if this is the only access strategy in use. */ public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, NamedParameterJdbcOperations operations, @@ -149,7 +149,7 @@ public void deleteAll(Class domainType) { } @Override - public void deleteAll(PropertyPath propertyPath) { + public void deleteAll(PropertyPath propertyPath) { operations.getJdbcOperations().update(sql(propertyPath.getOwningType().getType()).createDeleteAllSql(propertyPath)); } @@ -193,14 +193,18 @@ public Iterable findAllById(Iterable ids, Class domainType) { } @Override - public Iterable findAllByProperty(Object rootId, JdbcPersistentProperty property) { + public Iterable findAllByProperty(Object rootId, JdbcPersistentProperty property) { Class actualType = property.getActualType(); - String findAllByProperty = sql(actualType).getFindAllByProperty(property.getReverseColumnName()); + boolean isMap = property.getTypeInformation().isMap(); + String findAllByProperty = sql(actualType).getFindAllByProperty(property.getReverseColumnName(), + property.getKeyColumn()); MapSqlParameterSource parameter = new MapSqlParameterSource(property.getReverseColumnName(), rootId); - return (Iterable) operations.query(findAllByProperty, parameter, getEntityRowMapper(actualType)); + return operations.query(findAllByProperty, parameter, isMap // + ? getMapEntityRowMapper(property) // + : getEntityRowMapper(actualType)); } @Override @@ -287,7 +291,12 @@ private EntityRowMapper getEntityRowMapper(Class domainType) { return new EntityRowMapper<>(getRequiredPersistentEntity(domainType), conversions, context, accessStrategy); } + private RowMapper getMapEntityRowMapper(JdbcPersistentProperty property) { + return new MapEntityRowMapper(getEntityRowMapper(property.getActualType()), property.getKeyColumn()); + } + private MapSqlParameterSource createIdParameterSource(Object id, Class domainType) { + return new MapSqlParameterSource("id", convert(id, getRequiredPersistentEntity(domainType).getRequiredIdProperty().getColumnType())); } @@ -313,5 +322,4 @@ private V convert(Object from, Class to) { private SqlGenerator sql(Class domainType) { return sqlGeneratorSource.getSqlGenerator(domainType); } - } diff --git a/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index 7d876d6c0a..3109078899 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -77,6 +77,13 @@ public void interpret(DeleteAll delete) { private Map createAdditionalColumnValues(Insert insert) { Map additionalColumnValues = new HashMap<>(); + addDependingOnInformation(insert, additionalColumnValues); + additionalColumnValues.putAll(insert.getAdditionalValues()); + + return additionalColumnValues; + } + + private void addDependingOnInformation(Insert insert, Map additionalColumnValues) { DbAction dependingOn = insert.getDependingOn(); if (dependingOn != null) { @@ -88,7 +95,5 @@ private Map createAdditionalColumnValues(Insert insert) { additionalColumnValues.put(columnName, identifier); } - return additionalColumnValues; } - } diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index fe5a513529..5100b5ae7e 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -19,6 +19,7 @@ import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Map; import java.util.Set; import org.springframework.core.convert.ConversionService; @@ -80,6 +81,11 @@ public T mapRow(ResultSet resultSet, int rowNumber) throws SQLException { if (Set.class.isAssignableFrom(property.getType())) { propertyAccessor.setProperty(property, accessStrategy.findAllByProperty(id, property)); + } else if (Map.class.isAssignableFrom(property.getType())) { + + Iterable allByProperty = accessStrategy.findAllByProperty(id, property); + IterableOfEntryToMapConverter converter = new IterableOfEntryToMapConverter(); + propertyAccessor.setProperty(property, converter.convert(allByProperty)); } else { propertyAccessor.setProperty(property, readFrom(resultSet, property, "")); } diff --git a/src/main/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverter.java b/src/main/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverter.java new file mode 100644 index 0000000000..d596637f52 --- /dev/null +++ b/src/main/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverter.java @@ -0,0 +1,74 @@ +/* + * Copyright 2017 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 java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; + +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalConverter; +import org.springframework.core.convert.converter.Converter; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +/** + * A converter for creating a {@link Map} from an {@link Iterable}. + * + * @author Jens Schauder + */ +class IterableOfEntryToMapConverter implements ConditionalConverter, Converter { + + @Nullable + @Override + public Map convert(Iterable source) { + + HashMap result = new HashMap(); + + source.forEach(element -> { + + if (!(element instanceof Entry)) { + throw new IllegalArgumentException(String.format("Cannot convert %s to Map.Entry", element.getClass())); + } + + Entry entry = (Entry) element; + result.put(entry.getKey(), entry.getValue()); + }); + + return result; + } + + /** + * Tries to determine if the {@literal sourceType} can be converted to a {@link Map}. If this can not be determined, + * because the sourceTyppe does not contain information about the element type it returns {@literal true}. + * + * @param sourceType {@link TypeDescriptor} to convert from. + * @param targetType {@link TypeDescriptor} to convert to. + * @return + */ + @Override + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + + Assert.notNull(sourceType, "Source type must not be null."); + Assert.notNull(targetType, "Target type must not be null."); + + if (!sourceType.isAssignableTo(TypeDescriptor.valueOf(Iterable.class))) + return false; + + TypeDescriptor elementDescriptor = sourceType.getElementTypeDescriptor(); + return elementDescriptor == null || elementDescriptor.isAssignableTo(TypeDescriptor.valueOf(Entry.class)); + } +} diff --git a/src/main/java/org/springframework/data/jdbc/core/MapEntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/MapEntityRowMapper.java new file mode 100644 index 0000000000..4849e71eac --- /dev/null +++ b/src/main/java/org/springframework/data/jdbc/core/MapEntityRowMapper.java @@ -0,0 +1,49 @@ +/* + * Copyright 2017 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 java.sql.ResultSet; +import java.sql.SQLException; +import java.util.HashMap; +import java.util.Map; + +import org.springframework.jdbc.core.RowMapper; +import org.springframework.lang.Nullable; + +/** + * A {@link RowMapper} that maps a row to a {@link Map.Entry} so an {@link Iterable} of those can be converted to a + * {@link Map} using an {@link IterableOfEntryToMapConverter}. Creation of the {@literal value} part of the resulting + * {@link Map.Entry} is delegated to a {@link RowMapper} provided in the constructor. + * + * @author Jens Schauder + */ +class MapEntityRowMapper implements RowMapper> { + + private final RowMapper delegate; + private final String keyColumn; + + MapEntityRowMapper(RowMapper delegate, String keyColumn) { + + this.delegate = delegate; + this.keyColumn = keyColumn; + } + + @Nullable + @Override + public Map.Entry mapRow(ResultSet rs, int rowNum) throws SQLException { + return new HashMap.SimpleEntry<>(rs.getObject(keyColumn), delegate.mapRow(rs, rowNum)); + } +} diff --git a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java index d3dd178940..37ef48d422 100644 --- a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java +++ b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -88,8 +89,24 @@ String getFindAll() { return findAllSql.get(); } - String getFindAllByProperty(String columnName) { - return String.format("%s WHERE %s = :%s", findAllSql.get(), columnName, columnName); + /** + * Returns a query for selecting all simple properties of an entity, including those for one-to-one relationships. + * Results are limited to those rows referencing some other entity using the column specified by + * {@literal columnName}. This is used to select values for a complex property ({@link Set}, {@link Map} ...) based on + * a referencing entity. + * + * @param columnName name of the column of the FK back to the referencing entity. + * @param keyColumn if the property is of type {@link Map} this column contains the map key. + * @return a SQL String. + */ + String getFindAllByProperty(String columnName, String keyColumn) { + + String baseSelect = (keyColumn != null) // + ? createSelectBuilder().column(cb -> cb.tableAlias(entity.getTableName()).column(keyColumn).as(keyColumn)) + .build() + : getFindAll(); + + return String.format("%s WHERE %s = :%s", baseSelect, columnName, columnName); } String getExists() { @@ -136,10 +153,19 @@ private SelectBuilder createSelectBuilder() { return builder; } + /** + * Adds the columns to the provided {@link SelectBuilder} representing simplem properties, including those from + * one-to-one relationships. + * + * @param builder The {@link SelectBuilder} to be modified. + */ private void addColumnsAndJoinsForOneToOneReferences(SelectBuilder builder) { for (JdbcPersistentProperty property : entity) { - if (!property.isEntity() || Collection.class.isAssignableFrom(property.getType())) { + if (!property.isEntity() // + || Collection.class.isAssignableFrom(property.getType()) // + || Map.class.isAssignableFrom(property.getType()) // + ) { continue; } diff --git a/src/main/java/org/springframework/data/jdbc/core/conversion/DbAction.java b/src/main/java/org/springframework/data/jdbc/core/conversion/DbAction.java index 341370bd8f..f25d031ed7 100644 --- a/src/main/java/org/springframework/data/jdbc/core/conversion/DbAction.java +++ b/src/main/java/org/springframework/data/jdbc/core/conversion/DbAction.java @@ -18,6 +18,9 @@ import lombok.Getter; import lombok.ToString; +import java.util.HashMap; +import java.util.Map; + import org.springframework.data.mapping.PropertyPath; import org.springframework.util.Assert; @@ -40,6 +43,13 @@ public abstract class DbAction { */ private final T entity; + /** + * Key-value-pairs to specify additional values to be used with the statement which can't be obtained from the entity, + * nor from {@link DbAction}s {@literal this} depends on. A used case are map keys, which need to be persisted with + * the map value but aren't part of the value. + */ + private final Map additionalValues = new HashMap<>(); + /** * Another action, this action depends on. For example the insert for one entity might need the id of another entity, * which gets insert before this one. That action would be referenced by this property, so that the id becomes diff --git a/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java b/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java index 925f4de811..8115652a03 100644 --- a/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java +++ b/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java @@ -15,7 +15,11 @@ */ package org.springframework.data.jdbc.core.conversion; +import lombok.RequiredArgsConstructor; + import java.util.Collection; +import java.util.Map; +import java.util.Map.Entry; import java.util.stream.Stream; import org.springframework.data.jdbc.core.conversion.DbAction.Insert; @@ -56,7 +60,7 @@ private void write(Object o, AggregateChange aggregateChange, DbAction depending Insert insert = DbAction.insert(o, dependingOn); aggregateChange.addAction(insert); - referencedEntities(o).forEach(e -> saveReferencedEntities(e, aggregateChange, insert)); + referencedEntities(o).forEach(propertyAndValue -> saveReferencedEntities(propertyAndValue, aggregateChange, insert)); } else { deleteReferencedEntities(entityInformation.getRequiredId(o), aggregateChange); @@ -64,32 +68,35 @@ private void write(Object o, AggregateChange aggregateChange, DbAction depending Update update = DbAction.update(o, dependingOn); aggregateChange.addAction(update); - referencedEntities(o).forEach(e -> insertReferencedEntities(e, aggregateChange, update)); + referencedEntities(o).forEach(propertyAndValue -> insertReferencedEntities(propertyAndValue, aggregateChange, update)); } } - private void saveReferencedEntities(Object o, AggregateChange aggregateChange, DbAction dependingOn) { + private void saveReferencedEntities(PropertyAndValue propertyAndValue, AggregateChange aggregateChange, DbAction dependingOn) { - saveActions(o, dependingOn).forEach(a -> { + saveActions(propertyAndValue, dependingOn).forEach(a -> { aggregateChange.addAction(a); - referencedEntities(o).forEach(e -> saveReferencedEntities(e, aggregateChange, a)); + referencedEntities(propertyAndValue.value).forEach(pav -> saveReferencedEntities(pav, aggregateChange, a)); }); - } - private Stream saveActions(T t, DbAction dependingOn) { + private Stream saveActions(PropertyAndValue propertyAndValue, DbAction dependingOn) { - if (Collection.class.isAssignableFrom(ClassUtils.getUserClass(t))) { - return collectionSaveAction((Collection) t, dependingOn); + if (Map.Entry.class.isAssignableFrom(ClassUtils.getUserClass(propertyAndValue.value))) { + return mapEntrySaveAction(propertyAndValue, dependingOn); } - return Stream.of(singleSaveAction(t, dependingOn)); + return Stream.of(singleSaveAction(propertyAndValue.value, dependingOn)); } - private Stream collectionSaveAction(Collection collection, DbAction dependingOn) { + private Stream mapEntrySaveAction(PropertyAndValue propertyAndValue, DbAction dependingOn) { + + Map.Entry entry = (Map.Entry) propertyAndValue.value; - return collection.stream().map(e -> singleSaveAction(e, dependingOn)); + DbAction action = singleSaveAction(entry.getValue(), dependingOn); + action.getAdditionalValues().put(propertyAndValue.property.getKeyColumn(), entry.getKey()); + return Stream.of(action); } private DbAction singleSaveAction(T t, DbAction dependingOn) { @@ -100,22 +107,36 @@ private DbAction singleSaveAction(T t, DbAction dependingOn) { return entityInformation.isNew(t) ? DbAction.insert(t, dependingOn) : DbAction.update(t, dependingOn); } - private void insertReferencedEntities(Object o, AggregateChange aggregateChange, DbAction dependingOn) { + private void insertReferencedEntities(PropertyAndValue propertyAndValue, AggregateChange aggregateChange, DbAction dependingOn) { - aggregateChange.addAction(DbAction.insert(o, dependingOn)); - referencedEntities(o).forEach(e -> insertReferencedEntities(e, aggregateChange, dependingOn)); + Insert insert; + if (propertyAndValue.property.isMap()) { + + Entry valueAsEntry = (Entry) propertyAndValue.value; + insert = DbAction.insert(valueAsEntry.getValue(), dependingOn); + insert.getAdditionalValues().put(propertyAndValue.property.getKeyColumn(), valueAsEntry.getKey()); + } else { + insert = DbAction.insert(propertyAndValue.value, dependingOn); + } + + aggregateChange.addAction(insert); + referencedEntities(insert.getEntity()) + .forEach(pav -> insertReferencedEntities(pav, aggregateChange, dependingOn)); } - private Stream referencedEntities(Object o) { + private Stream referencedEntities(Object o) { JdbcPersistentEntity persistentEntity = context.getRequiredPersistentEntity(o.getClass()); return StreamUtils.createStreamFromIterator(persistentEntity.iterator()) // - .filter(PersistentProperty::isEntity) - .flatMap(p -> referencedEntity(p, persistentEntity.getPropertyAccessor(o))); + .filter(PersistentProperty::isEntity) // + .flatMap( // + p -> referencedEntity(p, persistentEntity.getPropertyAccessor(o)) // + .map(e -> new PropertyAndValue(p, e)) // + ); } - private Stream referencedEntity(JdbcPersistentProperty p, PersistentPropertyAccessor propertyAccessor) { + private Stream referencedEntity(JdbcPersistentProperty p, PersistentPropertyAccessor propertyAccessor) { Class actualType = p.getActualType(); JdbcPersistentEntity persistentEntity = context // @@ -127,9 +148,15 @@ private Stream referencedEntity(JdbcPersistentProperty p, PersistentPropertyA Class type = p.getType(); - return Collection.class.isAssignableFrom(type) // - ? collectionPropertyAsStream(p, propertyAccessor) // - : singlePropertyAsStream(p, propertyAccessor); + if (Collection.class.isAssignableFrom(type)) { + return collectionPropertyAsStream(p, propertyAccessor); + } + + if (Map.class.isAssignableFrom(type)) { + return mapPropertyAsStream(p, propertyAccessor); + } + + return singlePropertyAsStream(p, propertyAccessor); } private Stream collectionPropertyAsStream(JdbcPersistentProperty p, @@ -142,6 +169,15 @@ private Stream collectionPropertyAsStream(JdbcPersistentProperty p, : ((Collection) property).stream(); } + private Stream mapPropertyAsStream(JdbcPersistentProperty p, PersistentPropertyAccessor propertyAccessor) { + + Object property = propertyAccessor.getProperty(p); + + return property == null // + ? Stream.empty() // + : ((Map) property).entrySet().stream().map(e -> (Object) e); + } + private Stream singlePropertyAsStream(JdbcPersistentProperty p, PersistentPropertyAccessor propertyAccessor) { Object property = propertyAccessor.getProperty(p); @@ -151,4 +187,11 @@ private Stream singlePropertyAsStream(JdbcPersistentProperty p, Persiste return Stream.of(property); } + + @RequiredArgsConstructor + private static class PropertyAndValue { + + private final JdbcPersistentProperty property; + private final Object value; + } } diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java b/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java index b5c54b90b2..ea4c31b0cd 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java @@ -80,7 +80,7 @@ protected Association createAssociation() { * @see org.springframework.data.jdbc.mapping.model.JdbcPersistentProperty#getColumnName() */ public String getColumnName() { - return this.context.getNamingStrategy().getColumnName(this); + return context.getNamingStrategy().getColumnName(this); } /** @@ -104,7 +104,12 @@ public JdbcPersistentEntity getOwner() { @Override public String getReverseColumnName() { - return getOwner().getTableName(); + return context.getNamingStrategy().getReverseColumnName(this); + } + + @Override + public String getKeyColumn() { + return isMap() ? context.getNamingStrategy().getKeyColumn(this) : null; } private Class columnTypeIfEntity(Class type) { diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/DefaultNamingStrategy.java b/src/main/java/org/springframework/data/jdbc/mapping/model/DefaultNamingStrategy.java index 5bad582468..dde37726de 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/DefaultNamingStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/DefaultNamingStrategy.java @@ -50,4 +50,14 @@ public String getTableName(Class type) { public String getColumnName(JdbcPersistentProperty property) { return property.getName(); } + + @Override + public String getReverseColumnName(JdbcPersistentProperty property) { + return property.getOwner().getTableName(); + } + + @Override + public String getKeyColumn(JdbcPersistentProperty property) { + return getReverseColumnName(property) + "_key"; + } } diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java index c3f756af1d..3d97da72c9 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java @@ -44,4 +44,6 @@ public interface JdbcPersistentProperty extends PersistentProperty getOwner(); String getReverseColumnName(); + + String getKeyColumn(); } diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java b/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java index f5e7f44522..3f034006f0 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/NamingStrategy.java @@ -30,4 +30,17 @@ default String getQualifiedTableName(Class type) { return this.getSchema() + (this.getSchema().equals("") ? "" : ".") + this.getTableName(type); } + /** + * For a reference A -> B this is the name in the table for B which references A. + * + * @return a column name. + */ + String getReverseColumnName(JdbcPersistentProperty property); + + /** + * For a map valued reference A -> Map>X,B< this is the name of the column in the tabel for B holding the key of the map. + * @return + */ + String getKeyColumn(JdbcPersistentProperty property); + } diff --git a/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java index 8f370af77a..0385c15aef 100644 --- a/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java @@ -23,6 +23,7 @@ import java.sql.ResultSet; import java.sql.SQLException; +import java.util.AbstractMap.SimpleEntry; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -36,7 +37,9 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.annotation.Id; +import org.springframework.data.convert.Jsr310Converters; import org.springframework.data.jdbc.mapping.model.DefaultNamingStrategy; import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; import org.springframework.data.jdbc.mapping.model.JdbcPersistentEntity; @@ -50,11 +53,14 @@ */ public class EntityRowMapperUnitTests { + public static final long ID_FOR_ENTITY_REFERENCING_MAP = 42L; + public static final long ID_FOR_ENTITY_NOT_REFERENCING_MAP = 23L; + @Test // DATAJDBC-113 public void simpleEntitiesGetProperlyExtracted() throws SQLException { ResultSet rs = mockResultSet(asList("id", "name"), // - 23L, "alpha"); + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha"); rs.next(); Trivial extracted = createRowMapper(Trivial.class).mapRow(rs, 1); @@ -62,15 +68,14 @@ public void simpleEntitiesGetProperlyExtracted() throws SQLException { assertThat(extracted) // .isNotNull() // .extracting(e -> e.id, e -> e.name) // - .containsExactly(23L, "alpha"); - + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha"); } @Test // DATAJDBC-113 public void simpleOneToOneGetsProperlyExtracted() throws SQLException { ResultSet rs = mockResultSet(asList("id", "name", "child_id", "child_name"), // - 23L, "alpha", 42L, "beta"); + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 24L, "beta"); rs.next(); OneToOne extracted = createRowMapper(OneToOne.class).mapRow(rs, 1); @@ -78,14 +83,14 @@ public void simpleOneToOneGetsProperlyExtracted() throws SQLException { assertThat(extracted) // .isNotNull() // .extracting(e -> e.id, e -> e.name, e -> e.child.id, e -> e.child.name) // - .containsExactly(23L, "alpha", 42L, "beta"); + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 24L, "beta"); } @Test // DATAJDBC-113 public void collectionReferenceGetsLoadedWithAdditionalSelect() throws SQLException { ResultSet rs = mockResultSet(asList("id", "name"), // - 23L, "alpha"); + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha"); rs.next(); OneToSet extracted = createRowMapper(OneToSet.class).mapRow(rs, 1); @@ -93,19 +98,45 @@ public void collectionReferenceGetsLoadedWithAdditionalSelect() throws SQLExcept assertThat(extracted) // .isNotNull() // .extracting(e -> e.id, e -> e.name, e -> e.children.size()) // - .containsExactly(23L, "alpha", 2); + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 2); + } + + @Test // DATAJDBC-131 + public void mapReferenceGetsLoadedWithAdditionalSelect() throws SQLException { + + ResultSet rs = mockResultSet(asList("id", "name"), // + ID_FOR_ENTITY_REFERENCING_MAP, "alpha"); + rs.next(); + + OneToMap extracted = createRowMapper(OneToMap.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.name, e -> e.children.size()) // + .containsExactly(ID_FOR_ENTITY_REFERENCING_MAP, "alpha", 2); } private EntityRowMapper createRowMapper(Class type) { JdbcMappingContext context = new JdbcMappingContext(new DefaultNamingStrategy()); - DataAccessStrategy template = mock(DataAccessStrategy.class); + DataAccessStrategy accessStrategy = mock(DataAccessStrategy.class); - doReturn(new HashSet<>(asList(new Trivial(), new Trivial()))).when(template).findAllByProperty(eq(23L), + // the ID of the entity is used to determin what kind of resultset is needed for subsequent selects. + doReturn(new HashSet<>(asList(new Trivial(), new Trivial()))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_NOT_REFERENCING_MAP), any(JdbcPersistentProperty.class)); + doReturn(new HashSet<>(asList( // + new SimpleEntry("one", new Trivial()), // + new SimpleEntry("two", new Trivial()) // + ))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_MAP), any(JdbcPersistentProperty.class)); + + GenericConversionService conversionService = new GenericConversionService(); + conversionService.addConverter(new IterableOfEntryToMapConverter()); + DefaultConversionService.addDefaultConverters(conversionService); + Jsr310Converters.getConvertersToRegister().forEach(conversionService::addConverter); + return new EntityRowMapper<>((JdbcPersistentEntity) context.getRequiredPersistentEntity(type), - new DefaultConversionService(), context, template); + conversionService, context, accessStrategy); } private static ResultSet mockResultSet(List columns, Object... values) { @@ -223,4 +254,11 @@ static class OneToSet { Set children; } + @RequiredArgsConstructor + static class OneToMap { + + @Id Long id; + String name; + Map children; + } } diff --git a/src/test/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverterUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverterUnitTests.java new file mode 100644 index 0000000000..915a7107a0 --- /dev/null +++ b/src/test/java/org/springframework/data/jdbc/core/IterableOfEntryToMapConverterUnitTests.java @@ -0,0 +1,73 @@ +/* + * Copyright 2017 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 java.util.Arrays.*; +import static java.util.Collections.*; + +import java.util.AbstractMap.SimpleEntry; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.assertj.core.api.SoftAssertions; +import org.junit.Test; +import org.springframework.core.convert.TypeDescriptor; + +/** + * Unit tests for {@link IterableOfEntryToMapConverter}. + * + * @author Jens Schauder + */ +public class IterableOfEntryToMapConverterUnitTests { + + IterableOfEntryToMapConverter converter = new IterableOfEntryToMapConverter(); + + @Test + public void testConversions() { + + HashMap map = new HashMap<>(); + map.put("key", "value"); + List testValues = asList( // + new Object[] { emptySet(), emptyMap() }, // + new Object[] { emptyList(), emptyMap() }, // + new Object[] { "string", false }, // + new Object[] { asList(new SimpleEntry<>("key", "value")), map }, // + new Object[] { asList("string"), IllegalArgumentException.class } // + ); + + SoftAssertions softly = new SoftAssertions(); + testValues.forEach(array -> softly.assertThat(tryToConvert(array[0])).isEqualTo(array[1])); + softly.assertAll(); + } + + private Object tryToConvert(Object input) { + + try { + if (converter.matches( // + TypeDescriptor.valueOf(input.getClass()), // + TypeDescriptor.valueOf(Map.class)) // + ) { + return converter.convert((Iterable) input); + } else { + return false; + } + + } catch (Exception e) { + return e.getClass(); + } + } +} diff --git a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java index c76ce9aad0..48c6202c13 100644 --- a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.*; +import java.util.Map; import java.util.Set; import org.assertj.core.api.SoftAssertions; @@ -108,6 +109,31 @@ public void cascadingDeleteSecondLevel() { "DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT x_l1id FROM ReferencedEntity WHERE DummyEntity IS NOT NULL)"); } + @Test // DATAJDBC-131 + public void findAllByProperty() { + + // this would get called when DummyEntity is the element type of a Set + String sql = sqlGenerator.getFindAllByProperty("back-ref", null); + + assertThat(sql).isEqualTo("SELECT DummyEntity.id AS id, DummyEntity.name AS name, " + + "ref.l1id AS ref_l1id, ref.content AS ref_content, ref.further AS ref_further " + + "FROM DummyEntity LEFT OUTER JOIN ReferencedEntity AS ref ON ref.DummyEntity = DummyEntity.id " + + "WHERE back-ref = :back-ref"); + } + + @Test // DATAJDBC-131 + public void findAllByPropertyWithKey() { + + // this would get called when DummyEntity is th element type of a Map + String sql = sqlGenerator.getFindAllByProperty("back-ref", "key-column"); + + assertThat(sql).isEqualTo("SELECT DummyEntity.id AS id, DummyEntity.name AS name, " + + "ref.l1id AS ref_l1id, ref.content AS ref_content, ref.further AS ref_further, " + + "DummyEntity.key-column AS key-column " + + "FROM DummyEntity LEFT OUTER JOIN ReferencedEntity AS ref ON ref.DummyEntity = DummyEntity.id " + + "WHERE back-ref = :back-ref"); + } + @SuppressWarnings("unused") static class DummyEntity { @@ -115,6 +141,7 @@ static class DummyEntity { String name; ReferencedEntity ref; Set elements; + Map mappedElements; } @SuppressWarnings("unused") diff --git a/src/test/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriterUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriterUnitTests.java index 2843a0b8bd..64b3ff5a37 100644 --- a/src/test/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriterUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriterUnitTests.java @@ -19,7 +19,9 @@ import lombok.RequiredArgsConstructor; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.junit.Test; @@ -41,6 +43,7 @@ @RunWith(MockitoJUnitRunner.class) public class JdbcEntityWriterUnitTests { + public static final long SOME_ENTITY_ID = 23L; JdbcEntityWriter converter = new JdbcEntityWriter(new JdbcMappingContext(new DefaultNamingStrategy())); @Test // DATAJDBC-112 @@ -62,7 +65,7 @@ public void newEntityGetsConvertedToOneInsert() { @Test // DATAJDBC-112 public void existingEntityGetsConvertedToUpdate() { - SingleReferenceEntity entity = new SingleReferenceEntity(23L); + SingleReferenceEntity entity = new SingleReferenceEntity(SOME_ENTITY_ID); AggregateChange aggregateChange = // new AggregateChange(Kind.SAVE, SingleReferenceEntity.class, entity); @@ -79,7 +82,7 @@ public void existingEntityGetsConvertedToUpdate() { @Test // DATAJDBC-112 public void referenceTriggersDeletePlusInsert() { - SingleReferenceEntity entity = new SingleReferenceEntity(23L); + SingleReferenceEntity entity = new SingleReferenceEntity(SOME_ENTITY_ID); entity.other = new Element(null); AggregateChange aggregateChange = new AggregateChange(Kind.SAVE, SingleReferenceEntity.class, @@ -158,6 +161,63 @@ public void cascadingReferencesTriggerCascadingActions() { ); } + @Test // DATAJDBC-131 + public void newEntityWithEmptyMapResultsInSingleInsert() { + + MapContainer entity = new MapContainer(null); + AggregateChange aggregateChange = new AggregateChange(Kind.SAVE, MapContainer.class, entity); + + converter.write(entity, aggregateChange); + + assertThat(aggregateChange.getActions()).extracting(DbAction::getClass, DbAction::getEntityType) // + .containsExactly( // + tuple(Insert.class, MapContainer.class)); + } + + @Test // DATAJDBC-131 + public void newEntityWithMapResultsInAdditionalInsertPerElement() { + + MapContainer entity = new MapContainer(null); + entity.elements.put("one", new Element(null)); + entity.elements.put("two", new Element(null)); + + AggregateChange aggregateChange = new AggregateChange(Kind.SAVE, SetContainer.class, entity); + converter.write(entity, aggregateChange); + + assertThat(aggregateChange.getActions()) + .extracting(DbAction::getClass, DbAction::getEntityType, JdbcEntityWriterUnitTests::getMapKey) // + .containsExactlyInAnyOrder( // + tuple(Insert.class, MapContainer.class, null), // + tuple(Insert.class, Element.class, "one"), // + tuple(Insert.class, Element.class, "two") // + ).containsSubsequence( // container comes before the elements + tuple(Insert.class, MapContainer.class, null), // + tuple(Insert.class, Element.class, "two") // + ).containsSubsequence( // container comes before the elements + tuple(Insert.class, MapContainer.class, null), // + tuple(Insert.class, Element.class, "one") // + ); + } + + @Test // DATAJDBC-112 + public void mapTriggersDeletePlusInsert() { + + MapContainer entity = new MapContainer(SOME_ENTITY_ID); + entity.elements.put("one", new Element(null)); + + AggregateChange aggregateChange = new AggregateChange(Kind.SAVE, MapContainer.class, entity); + + converter.write(entity, aggregateChange); + + assertThat(aggregateChange.getActions()) // + .extracting(DbAction::getClass, DbAction::getEntityType, JdbcEntityWriterUnitTests::getMapKey) // + .containsExactly( // + tuple(Delete.class, Element.class, null), // + tuple(Update.class, MapContainer.class, null), // + tuple(Insert.class, Element.class, "one") // + ); + } + private CascadingReferenceMiddleElement createMiddleElement(Element first, Element second) { CascadingReferenceMiddleElement middleElement1 = new CascadingReferenceMiddleElement(null); @@ -166,6 +226,10 @@ private CascadingReferenceMiddleElement createMiddleElement(Element first, Eleme return middleElement1; } + private static Object getMapKey(DbAction a) { + return a.getAdditionalValues().get("MapContainer_key"); + } + @RequiredArgsConstructor static class SingleReferenceEntity { @@ -196,6 +260,13 @@ private static class SetContainer { Set elements = new HashSet<>(); } + @RequiredArgsConstructor + private static class MapContainer { + + @Id final Long id; + Map elements = new HashMap<>(); + } + @RequiredArgsConstructor private static class Element { @Id final Long id; diff --git a/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java new file mode 100644 index 0000000000..013524aea2 --- /dev/null +++ b/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryWithMapsIntegrationTests.java @@ -0,0 +1,228 @@ +/* + * Copyright 2017 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.repository; + +import static org.assertj.core.api.Assertions.*; + +import junit.framework.AssertionFailedError; +import lombok.Data; +import lombok.RequiredArgsConstructor; + +import java.util.HashMap; +import java.util.Map; + +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.data.annotation.Id; +import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; +import org.springframework.data.jdbc.testing.TestConfiguration; +import org.springframework.data.repository.CrudRepository; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.rules.SpringClassRule; +import org.springframework.test.context.junit4.rules.SpringMethodRule; +import org.springframework.transaction.annotation.Transactional; + +/** + * Very simple use cases for creation and usage of JdbcRepositories for Entities that contain {@link java.util.Map}s. + * + * @author Jens Schauder + */ +@ContextConfiguration +@Transactional +public class JdbcRepositoryWithMapsIntegrationTests { + + @Configuration + @Import(TestConfiguration.class) + static class Config { + + @Autowired JdbcRepositoryFactory factory; + + @Bean + Class testClass() { + return JdbcRepositoryWithMapsIntegrationTests.class; + } + + @Bean + DummyEntityRepository dummyEntityRepository() { + return factory.getRepository(DummyEntityRepository.class); + } + } + + @ClassRule public static final SpringClassRule classRule = new SpringClassRule(); + @Rule public SpringMethodRule methodRule = new SpringMethodRule(); + + @Autowired NamedParameterJdbcTemplate template; + @Autowired DummyEntityRepository repository; + + @Test // DATAJDBC-131 + public void saveAndLoadEmptyMap() { + + DummyEntity entity = repository.save(createDummyEntity()); + + assertThat(entity.id).isNotNull(); + + DummyEntity reloaded = repository.findById(entity.id).orElseThrow(AssertionFailedError::new); + + assertThat(reloaded.content) // + .isNotNull() // + .isEmpty(); + } + + @Test // DATAJDBC-131 + public void saveAndLoadNonEmptyMap() { + + Element element1 = new Element(); + Element element2 = new Element(); + + DummyEntity entity = createDummyEntity(); + entity.content.put("one", element1); + entity.content.put("two", element2); + + entity = repository.save(entity); + + assertThat(entity.id).isNotNull(); + assertThat(entity.content.values()).allMatch(v -> v.id != null); + + DummyEntity reloaded = repository.findById(entity.id).orElseThrow(AssertionFailedError::new); + + assertThat(reloaded.content.values()) // + .isNotNull() // + .extracting(e -> e.id) // + .containsExactlyInAnyOrder(element1.id, element2.id); + } + + @Test // DATAJDBC-131 + public void findAllLoadsMap() { + + Element element1 = new Element(); + Element element2 = new Element(); + + DummyEntity entity = createDummyEntity(); + entity.content.put("one", element1); + entity.content.put("two", element2); + + entity = repository.save(entity); + + assertThat(entity.id).isNotNull(); + assertThat(entity.content.values()).allMatch(v -> v.id != null); + + Iterable reloaded = repository.findAll(); + + reloaded.forEach(de -> System.out.println("id " + de.id + " content " + de.content.values().iterator().next().content)); + + assertThat(reloaded) // + .extracting(e -> e.id, e -> e.content.size()) // + .containsExactly(tuple(entity.id, entity.content.size())); + } + + @Test // DATAJDBC-131 + public void updateMap() { + + Element element1 = createElement("one"); + Element element2 = createElement("two"); + Element element3 = createElement("three"); + + DummyEntity entity = createDummyEntity(); + entity.content.put("one", element1); + entity.content.put("two", element2); + + entity = repository.save(entity); + + entity.content.remove("one"); + element2.content = "two changed"; + entity.content.put("three", element3); + + entity = repository.save(entity); + + assertThat(entity.id).isNotNull(); + assertThat(entity.content.values()).allMatch(v -> v.id != null); + + DummyEntity reloaded = repository.findById(entity.id).orElseThrow(AssertionFailedError::new); + + // the elements got properly updated and reloaded + assertThat(reloaded.content) // + .isNotNull(); + + assertThat(reloaded.content.entrySet()) // + .extracting(e -> e.getKey(), e -> e.getValue().id, e -> e.getValue().content) // + .containsExactlyInAnyOrder( // + tuple("two", element2.id, "two changed"), // + tuple("three", element3.id, "three") // + ); + + Long count = template.queryForObject("select count(1) from Element", new HashMap<>(), Long.class); + assertThat(count).isEqualTo(2); + } + + @Test // DATAJDBC-131 + public void deletingWithMap() { + + Element element1 = createElement("one"); + Element element2 = createElement("two"); + + DummyEntity entity = createDummyEntity(); + entity.content.put("one", element1); + entity.content.put("two", element2); + + entity = repository.save(entity); + + repository.deleteById(entity.id); + + assertThat(repository.findById(entity.id)).isEmpty(); + + Long count = template.queryForObject("select count(1) from Element", new HashMap<>(), Long.class); + assertThat(count).isEqualTo(0); + } + + private Element createElement(String content) { + + Element element = new Element(); + element.content = content; + return element; + } + + private static DummyEntity createDummyEntity() { + + DummyEntity entity = new DummyEntity(); + entity.setName("Entity Name"); + return entity; + } + + interface DummyEntityRepository extends CrudRepository {} + + @Data + static class DummyEntity { + + @Id private Long id; + String name; + Map content = new HashMap<>(); + + } + + @RequiredArgsConstructor + static class Element { + + @Id private Long id; + String content; + } + +} diff --git a/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql b/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql new file mode 100644 index 0000000000..9e813b3003 --- /dev/null +++ b/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-hsql.sql @@ -0,0 +1,2 @@ +CREATE TABLE dummyentity ( 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), DummyEntity_key VARCHAR(100), dummyentity BIGINT); diff --git a/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-mysql.sql b/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-mysql.sql new file mode 100644 index 0000000000..f30df0af7f --- /dev/null +++ b/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-mysql.sql @@ -0,0 +1,2 @@ +CREATE TABLE dummyentity ( id BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id BIGINT AUTO_INCREMENT PRIMARY KEY, content VARCHAR(100), DummyEntity_key VARCHAR(100),dummyentity BIGINT); diff --git a/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-postgres.sql b/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-postgres.sql new file mode 100644 index 0000000000..25562c6550 --- /dev/null +++ b/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryWithMapsIntegrationTests-postgres.sql @@ -0,0 +1,4 @@ +DROP TABLE element; +DROP TABLE dummyentity; +CREATE TABLE dummyentity ( id SERIAL PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id SERIAL PRIMARY KEY, content VARCHAR(100),dummyentity_key VARCHAR(100), dummyentity BIGINT); From 1579a3cefe8b40b255bd283addbc761cd6b7e836 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 26 Oct 2017 09:57:21 +0200 Subject: [PATCH 3/3] DATAJDBC-131 - Polishing after review. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Put generics back in and added @SupressWarnings where this caused warnings. Extracted the test “isMap” into a “isQualified” method on the property. --- .../data/jdbc/core/DefaultDataAccessStrategy.java | 8 ++++---- .../data/jdbc/core/conversion/JdbcEntityWriter.java | 2 +- .../jdbc/mapping/model/BasicJdbcPersistentProperty.java | 7 ++++++- .../data/jdbc/mapping/model/JdbcPersistentProperty.java | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 5c13619fc1..9cba047827 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -149,7 +149,7 @@ public void deleteAll(Class domainType) { } @Override - public void deleteAll(PropertyPath propertyPath) { + public void deleteAll(PropertyPath propertyPath) { operations.getJdbcOperations().update(sql(propertyPath.getOwningType().getType()).createDeleteAllSql(propertyPath)); } @@ -192,17 +192,17 @@ public Iterable findAllById(Iterable ids, Class domainType) { return operations.query(findAllInListSql, parameter, getEntityRowMapper(domainType)); } + @SuppressWarnings("unchecked") @Override - public Iterable findAllByProperty(Object rootId, JdbcPersistentProperty property) { + public Iterable findAllByProperty(Object rootId, JdbcPersistentProperty property) { Class actualType = property.getActualType(); - boolean isMap = property.getTypeInformation().isMap(); String findAllByProperty = sql(actualType).getFindAllByProperty(property.getReverseColumnName(), property.getKeyColumn()); MapSqlParameterSource parameter = new MapSqlParameterSource(property.getReverseColumnName(), rootId); - return operations.query(findAllByProperty, parameter, isMap // + return (Iterable)operations.query(findAllByProperty, parameter, property.isQualified() // ? getMapEntityRowMapper(property) // : getEntityRowMapper(actualType)); } diff --git a/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java b/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java index 8115652a03..478ce68b33 100644 --- a/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java +++ b/src/main/java/org/springframework/data/jdbc/core/conversion/JdbcEntityWriter.java @@ -110,7 +110,7 @@ private DbAction singleSaveAction(T t, DbAction dependingOn) { private void insertReferencedEntities(PropertyAndValue propertyAndValue, AggregateChange aggregateChange, DbAction dependingOn) { Insert insert; - if (propertyAndValue.property.isMap()) { + if (propertyAndValue.property.isQualified()) { Entry valueAsEntry = (Entry) propertyAndValue.value; insert = DbAction.insert(valueAsEntry.getValue(), dependingOn); diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java b/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java index ea4c31b0cd..e092f93132 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/BasicJdbcPersistentProperty.java @@ -109,7 +109,12 @@ public String getReverseColumnName() { @Override public String getKeyColumn() { - return isMap() ? context.getNamingStrategy().getKeyColumn(this) : null; + return isQualified() ? context.getNamingStrategy().getKeyColumn(this) : null; + } + + @Override + public boolean isQualified() { + return isMap(); } private Class columnTypeIfEntity(Class type) { diff --git a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java index 3d97da72c9..6d1adcb738 100644 --- a/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java +++ b/src/main/java/org/springframework/data/jdbc/mapping/model/JdbcPersistentProperty.java @@ -46,4 +46,9 @@ public interface JdbcPersistentProperty extends PersistentProperty