diff --git a/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java index 0a622b25..9afb2fe6 100644 --- a/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/CascadingDataAccessStrategy.java @@ -43,7 +43,7 @@ public class CascadingDataAccessStrategy implements DataAccessStrategy { * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) */ @Override - public T insert(T instance, Class domainType, Map additionalParameters) { + public Object insert(T instance, Class domainType, Map additionalParameters) { return collect(das -> das.insert(instance, domainType, additionalParameters)); } 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 0be96173..98dbf364 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java @@ -38,10 +38,9 @@ public interface DataAccessStrategy { * @param additionalParameters name-value pairs of additional parameters. Especially ids of parent entities that need * to get referenced are contained in this map. Must not be {@code null}. * @param the type of the instance. - * @return the instance after insert into. The returned instance may be different to the given {@code instance} as - * this method can apply changes to the return object. + * @return the id generated by the database if any. */ - T insert(T instance, Class domainType, Map additionalParameters); + Object insert(T instance, Class domainType, Map additionalParameters); /** * Updates the data of a single entity in the database. Referenced entities don't get handled. 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 931e8418..14e11f12 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -21,13 +21,11 @@ import lombok.RequiredArgsConstructor; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; -import org.springframework.dao.NonTransientDataAccessException; import org.springframework.data.jdbc.support.JdbcUtil; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; @@ -83,7 +81,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) */ @Override - public T insert(T instance, Class domainType, Map additionalParameters) { + public Object insert(T instance, Class domainType, Map additionalParameters) { KeyHolder holder = new GeneratedKeyHolder(); RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); @@ -110,16 +108,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { holder // ); - instance = setIdFromJdbc(instance, holder, persistentEntity); - - // if there is an id property and it was null before the save - // The database should have created an id and provided it. - - if (idProperty != null && idValue == null && persistentEntity.isNew(instance)) { - throw new IllegalStateException(String.format(ENTITY_NEW_AFTER_INSERT, persistentEntity)); - } - - return instance; + return getIdFromHolder(holder, persistentEntity); } /* @@ -327,41 +316,20 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { || (idProperty.getType() == long.class && idValue.equals(0L)); } - private S setIdFromJdbc(S instance, KeyHolder holder, RelationalPersistentEntity persistentEntity) { - - try { - - PersistentPropertyAccessor accessor = converter.getPropertyAccessor(persistentEntity, instance); - - getIdFromHolder(holder, persistentEntity).ifPresent(it -> { - - RelationalPersistentProperty idProperty = persistentEntity.getRequiredIdProperty(); - - accessor.setProperty(idProperty, it); - - }); - - return accessor.getBean(); - - } catch (NonTransientDataAccessException e) { - throw new UnableToSetId("Unable to set id of " + instance, e); - } - } - - private Optional getIdFromHolder(KeyHolder holder, RelationalPersistentEntity persistentEntity) { + private Object getIdFromHolder(KeyHolder holder, RelationalPersistentEntity persistentEntity) { try { // MySQL just returns one value with a special name - return Optional.ofNullable(holder.getKey()); + return holder.getKey(); } catch (InvalidDataAccessApiUsageException e) { // Postgres returns a value for each column Map keys = holder.getKeys(); if (keys == null || persistentEntity.getIdProperty() == null) { - return Optional.empty(); + return null; } - return Optional.ofNullable(keys.get(persistentEntity.getIdColumn())); + return keys.get(persistentEntity.getIdColumn()); } } 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 1fac5284..dbcef36b 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -58,8 +58,9 @@ class DefaultJdbcInterpreter implements Interpreter { @Override public void interpret(Insert insert) { - T entity = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), createAdditionalColumnValues(insert)); - insert.setResultingEntity(entity); + Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), createAdditionalColumnValues(insert)); + + insert.setGeneratedId(id); } /* @@ -69,8 +70,8 @@ class DefaultJdbcInterpreter implements Interpreter { @Override public void interpret(InsertRoot insert) { - T entity = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Collections.emptyMap()); - insert.setResultingEntity(entity); + Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Collections.emptyMap()); + insert.setGeneratedId(id); } /* @@ -78,7 +79,7 @@ class DefaultJdbcInterpreter implements Interpreter { * @see org.springframework.data.relational.core.conversion.Interpreter#interpret(org.springframework.data.relational.core.conversion.DbAction.Update) */ @Override - public void interpret(Update update) { + public void interpret(Update update ) { accessStrategy.update(update.getEntity(), update.getEntityType()); } @@ -169,8 +170,8 @@ class DefaultJdbcInterpreter implements Interpreter { Object entity = dependingOn.getEntity(); - if (dependingOn instanceof DbAction.WithResultEntity) { - entity = ((DbAction.WithResultEntity) dependingOn).getResultingEntity(); + if (dependingOn instanceof DbAction.WithGeneratedId) { + return ((DbAction.WithGeneratedId) dependingOn).getGeneratedId(); } return persistentEntity.getIdentifierAccessor(entity).getIdentifier(); diff --git a/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java index 7665196e..abb90395 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DelegatingDataAccessStrategy.java @@ -36,7 +36,7 @@ public class DelegatingDataAccessStrategy implements DataAccessStrategy { * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) */ @Override - public T insert(T instance, Class domainType, Map additionalParameters) { + public Object insert(T instance, Class domainType, Map additionalParameters) { return delegate.insert(instance, domainType, additionalParameters); } diff --git a/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java b/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java index c28aa135..2abff0db 100644 --- a/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java +++ b/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java @@ -22,6 +22,7 @@ import org.springframework.data.mapping.IdentifierAccessor; import org.springframework.data.relational.core.conversion.AggregateChange; import org.springframework.data.relational.core.conversion.AggregateChange.Kind; import org.springframework.data.relational.core.conversion.Interpreter; +import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.conversion.RelationalEntityDeleteWriter; import org.springframework.data.relational.core.conversion.RelationalEntityWriter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; @@ -46,6 +47,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { private final ApplicationEventPublisher publisher; private final RelationalMappingContext context; + private final RelationalConverter converter; private final Interpreter interpreter; private final RelationalEntityWriter jdbcEntityWriter; @@ -62,14 +64,16 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { * @param dataAccessStrategy must not be {@literal null}. */ public JdbcAggregateTemplate(ApplicationEventPublisher publisher, RelationalMappingContext context, - DataAccessStrategy dataAccessStrategy) { + RelationalConverter converter, DataAccessStrategy dataAccessStrategy) { Assert.notNull(publisher, "ApplicationEventPublisher must not be null!"); Assert.notNull(context, "RelationalMappingContext must not be null!"); + Assert.notNull(converter, "RelationalConverter must not be null!"); Assert.notNull(dataAccessStrategy, "DataAccessStrategy must not be null!"); this.publisher = publisher; this.context = context; + this.converter = converter; this.accessStrategy = dataAccessStrategy; this.jdbcEntityWriter = new RelationalEntityWriter(context); @@ -86,8 +90,8 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { Assert.notNull(instance, "Aggregate instance must not be null!"); - RelationalPersistentEntity entity = context.getRequiredPersistentEntity(instance.getClass()); - IdentifierAccessor identifierAccessor = entity.getIdentifierAccessor(instance); + RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(instance.getClass()); + IdentifierAccessor identifierAccessor = persistentEntity.getIdentifierAccessor(instance); AggregateChange change = createChange(instance); @@ -97,9 +101,9 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { change // )); - change.executeWith(interpreter); + change.executeWith(interpreter, context, converter); - Object identifier = entity.getIdentifierAccessor(change.getEntity()).getIdentifier(); + Object identifier = persistentEntity.getIdentifierAccessor(change.getEntity()).getIdentifier(); Assert.notNull(identifier, "After saving the identifier must not be null"); @@ -198,7 +202,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { public void deleteAll(Class domainType) { AggregateChange change = createDeletingChange(domainType); - change.executeWith(interpreter); + change.executeWith(interpreter, context, converter); } private void deleteTree(Object id, @Nullable Object entity, Class domainType) { @@ -209,7 +213,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { Optional optionalEntity = Optional.ofNullable(entity); publisher.publishEvent(new BeforeDeleteEvent(specifiedId, optionalEntity, change)); - change.executeWith(interpreter); + change.executeWith(interpreter, context, converter); publisher.publishEvent(new AfterDeleteEvent(specifiedId, optionalEntity, change)); } diff --git a/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 9dd23e61..2a6109a2 100644 --- a/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -128,12 +128,13 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { * @see org.springframework.data.jdbc.core.DataAccessStrategy#insert(java.lang.Object, java.lang.Class, java.util.Map) */ @Override - public T insert(T instance, Class domainType, Map additionalParameters) { - - sqlSession().insert(namespace(domainType) + ".insert", - new MyBatisContext(null, instance, domainType, additionalParameters)); + public Object insert(T instance, Class domainType, Map additionalParameters) { - return instance; + MyBatisContext myBatisContext = new MyBatisContext(null, instance, domainType, additionalParameters); + sqlSession().insert(namespace(domainType) + ".insert", + myBatisContext); + + return myBatisContext.getId(); } /* diff --git a/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java b/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java index db5d196d..e4eb0fed 100644 --- a/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java +++ b/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java @@ -104,7 +104,7 @@ public class JdbcRepositoryFactory extends RepositoryFactorySupport { @Override protected Object getTargetRepository(RepositoryInformation repositoryInformation) { - JdbcAggregateTemplate template = new JdbcAggregateTemplate(publisher, context, accessStrategy); + JdbcAggregateTemplate template = new JdbcAggregateTemplate(publisher, context, converter, accessStrategy); return new SimpleJdbcRepository<>(template, context.getPersistentEntity(repositoryInformation.getDomainType())); } diff --git a/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java b/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java index 3b66c3e7..ed14500f 100644 --- a/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java +++ b/src/main/java/org/springframework/data/relational/core/conversion/AggregateChange.java @@ -16,10 +16,19 @@ package org.springframework.data.relational.core.conversion; import lombok.Getter; -import lombok.RequiredArgsConstructor; import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.PersistentPropertyPath; +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.lang.Nullable; +import org.springframework.util.Assert; /** * Represents the change happening to the aggregate (as used in the context of Domain Driven Design) as a whole. @@ -36,11 +45,11 @@ public class AggregateChange { private final Class entityType; /** Aggregate root, to which the change applies, if available */ - private T entity; + @Nullable private T entity; private final List> actions = new ArrayList<>(); - public AggregateChange(Kind kind, Class entityType, T entity) { + public AggregateChange(Kind kind, Class entityType, @Nullable T entity) { this.kind = kind; this.entityType = entityType; @@ -48,22 +57,144 @@ public class AggregateChange { } @SuppressWarnings("unchecked") - public void executeWith(Interpreter interpreter) { + public void executeWith(Interpreter interpreter, RelationalMappingContext context, RelationalConverter converter) { + + RelationalPersistentEntity persistentEntity = entity != null + ? (RelationalPersistentEntity) context.getRequiredPersistentEntity(entity.getClass()) + : null; + + PersistentPropertyAccessor propertyAccessor = // + persistentEntity != null // + ? converter.getPropertyAccessor(persistentEntity, entity) // + : null; actions.forEach(a -> { a.executeWith(interpreter); - if (a instanceof DbAction.InsertRoot && a.getEntityType().equals(entityType)) { - entity = (T) ((DbAction.InsertRoot) a).getResultingEntity(); + if (a instanceof DbAction.WithGeneratedId) { + + Assert.notNull(persistentEntity, + "For statements triggering database side id generation a RelationalPersistentEntity must be provided."); + Assert.notNull(propertyAccessor, "propertyAccessor must not be null"); + + Object generatedId = ((DbAction.WithGeneratedId) a).getGeneratedId(); + + if (generatedId != null) { + + if (a instanceof DbAction.InsertRoot && a.getEntityType().equals(entityType)) { + propertyAccessor.setProperty(persistentEntity.getRequiredIdProperty(), generatedId); + } else if (a instanceof DbAction.WithDependingOn) { + + setId(context, converter, propertyAccessor, (DbAction.WithDependingOn) a, generatedId); + } + } } }); + + if (propertyAccessor != null) { + entity = propertyAccessor.getBean(); + } } public void addAction(DbAction action) { actions.add(action); } + @SuppressWarnings("unchecked") + static void setId(RelationalMappingContext context, RelationalConverter converter, + PersistentPropertyAccessor propertyAccessor, DbAction.WithDependingOn action, Object generatedId) { + + PersistentPropertyPath propertyPathToEntity = action.getPropertyPath(); + + RelationalPersistentProperty requiredIdProperty = context + .getRequiredPersistentEntity(propertyPathToEntity.getRequiredLeafProperty().getActualType()) + .getRequiredIdProperty(); + + PersistentPropertyPath pathToId = context.getPersistentPropertyPath( + propertyPathToEntity.toDotPath() + '.' + requiredIdProperty.getName(), + propertyPathToEntity.getBaseProperty().getOwner().getType()); + + RelationalPersistentProperty leafProperty = propertyPathToEntity.getRequiredLeafProperty(); + + Object currentPropertyValue = propertyAccessor.getProperty(propertyPathToEntity); + Assert.notNull(currentPropertyValue, "Trying to set an ID for an element that does not exist"); + + if (leafProperty.isQualified()) { + + String keyColumn = leafProperty.getKeyColumn(); + Object keyObject = action.getAdditionalValues().get(keyColumn); + + if (List.class.isAssignableFrom(leafProperty.getType())) { + setIdInElementOfList(converter, action, generatedId, (List) currentPropertyValue, (int) keyObject); + } else if (Map.class.isAssignableFrom(leafProperty.getType())) { + setIdInElementOfMap(converter, action, generatedId, (Map) currentPropertyValue, keyObject); + } else { + throw new IllegalStateException("Can't handle " + currentPropertyValue); + } + } else if (leafProperty.isCollectionLike()) { + + if (Set.class.isAssignableFrom(leafProperty.getType())) { + setIdInElementOfSet(converter, action, generatedId, (Set) currentPropertyValue); + } else { + throw new IllegalStateException("Can't handle " + currentPropertyValue); + } + } else { + propertyAccessor.setProperty(pathToId, generatedId); + } + } + + @SuppressWarnings("unchecked") + private static void setIdInElementOfSet(RelationalConverter converter, DbAction.WithDependingOn action, + Object generatedId, Set set) { + + PersistentPropertyAccessor intermediateAccessor = setId(converter, action, generatedId); + + // this currently only works on the standard collections + // no support for immutable collections, nor specialized ones. + set.remove((T) action.getEntity()); + set.add((T) intermediateAccessor.getBean()); + } + + @SuppressWarnings("unchecked") + private static void setIdInElementOfMap(RelationalConverter converter, DbAction.WithDependingOn action, + Object generatedId, Map map, K keyObject) { + + PersistentPropertyAccessor intermediateAccessor = setId(converter, action, generatedId); + + // this currently only works on the standard collections + // no support for immutable collections, nor specialized ones. + map.put(keyObject, (V) intermediateAccessor.getBean()); + } + + @SuppressWarnings("unchecked") + private static void setIdInElementOfList(RelationalConverter converter, DbAction.WithDependingOn action, + Object generatedId, List list, int index) { + + PersistentPropertyAccessor intermediateAccessor = setId(converter, action, generatedId); + + // this currently only works on the standard collections + // no support for immutable collections, nor specialized ones. + list.set(index, (T) intermediateAccessor.getBean()); + } + + /** + * Sets the id of the entity referenced in the action and uses the {@link PersistentPropertyAccessor} used for that. + */ + private static PersistentPropertyAccessor setId(RelationalConverter converter, + DbAction.WithDependingOn action, Object generatedId) { + + Object originalElement = action.getEntity(); + + RelationalPersistentEntity persistentEntity = (RelationalPersistentEntity) converter.getMappingContext() + .getRequiredPersistentEntity(action.getEntityType()); + PersistentPropertyAccessor intermediateAccessor = converter.getPropertyAccessor(persistentEntity, + (T) originalElement); + + intermediateAccessor.setProperty(persistentEntity.getRequiredIdProperty(), generatedId); + return intermediateAccessor; + } + /** * The kind of action to be performed on an aggregate. */ diff --git a/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index d69843a9..d8b6d9f4 100644 --- a/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -25,6 +25,9 @@ import java.util.Map; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.util.Pair; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * An instance of this interface represents a (conceptual) single interaction with a database, e.g. a single update, @@ -68,16 +71,15 @@ public interface DbAction { * @param type of the entity for which this represents a database interaction. */ @Data - @RequiredArgsConstructor - class Insert implements WithDependingOn, WithEntity, WithResultEntity { + class Insert implements WithGeneratedId, WithDependingOn { - @NonNull private final T entity; - @NonNull private final PersistentPropertyPath propertyPath; - @NonNull private final WithEntity dependingOn; + @NonNull final T entity; + @NonNull final PersistentPropertyPath propertyPath; + @NonNull final WithEntity dependingOn; Map additionalValues = new HashMap<>(); - private T resultingEntity; + private Object generatedId; @Override public void doExecuteWith(Interpreter interpreter) { @@ -97,11 +99,11 @@ public interface DbAction { */ @Data @RequiredArgsConstructor - class InsertRoot implements WithEntity, WithResultEntity { + class InsertRoot implements WithEntity, WithGeneratedId { @NonNull private final T entity; - private T resultingEntity; + private Object generatedId; @Override public void doExecuteWith(Interpreter interpreter) { @@ -240,7 +242,7 @@ public interface DbAction { * * @author Jens Schauder */ - interface WithDependingOn extends WithPropertyPath { + interface WithDependingOn extends WithPropertyPath, WithEntity{ /** * The {@link DbAction} of a parent entity, possibly the aggregate root. This is used to obtain values needed to @@ -260,6 +262,11 @@ public interface DbAction { * @return Guaranteed to be not {@code null}. */ Map getAdditionalValues(); + + @Override + default Class getEntityType() { + return WithEntity.super.getEntityType(); + } } /** @@ -287,12 +294,13 @@ public interface DbAction { * * @author Jens Schauder */ - interface WithResultEntity extends WithEntity { + interface WithGeneratedId extends WithEntity { /** * @return the entity to persist. Guaranteed to be not {@code null}. */ - T getResultingEntity(); + @Nullable + Object getGeneratedId(); @SuppressWarnings("unchecked") @Override diff --git a/src/main/java/org/springframework/data/relational/core/conversion/Interpreter.java b/src/main/java/org/springframework/data/relational/core/conversion/Interpreter.java index a668146b..5013db75 100644 --- a/src/main/java/org/springframework/data/relational/core/conversion/Interpreter.java +++ b/src/main/java/org/springframework/data/relational/core/conversion/Interpreter.java @@ -42,8 +42,8 @@ public interface Interpreter { /** * Interpret an {@link Update}. Interpreting normally means "executing". * - * @param update the {@link Update} to be executed * @param the type of entity to work on. + * @param update the {@link Update} to be executed */ void interpret(Update update); diff --git a/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityWriter.java b/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityWriter.java index e8f6d8d8..1b6b01c5 100644 --- a/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityWriter.java +++ b/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityWriter.java @@ -15,9 +15,6 @@ */ package org.springframework.data.relational.core.conversion; -import lombok.Getter; -import lombok.RequiredArgsConstructor; - import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -25,6 +22,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import lombok.Value; import org.springframework.data.convert.EntityWriter; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyPath; @@ -164,8 +162,13 @@ public class RelationalEntityWriter implements EntityWriter) action; } @@ -176,9 +179,9 @@ public class RelationalEntityWriter implements EntityWriter from(PersistentPropertyPath path) { + private List from(PersistentPropertyPath path) { - List nodes = new ArrayList<>(); + List nodes = new ArrayList<>(); if (path.getLength() == 1) { @@ -194,7 +197,7 @@ public class RelationalEntityWriter implements EntityWriter pathNodes = nodesCache.get(path.getParentPath()); pathNodes.forEach(parentNode -> { - Object value = path.getRequiredLeafProperty().getOwner().getPropertyAccessor(parentNode.value) + Object value = path.getRequiredLeafProperty().getOwner().getPropertyAccessor(parentNode.getValue()) .getProperty(path.getRequiredLeafProperty()); nodes.addAll(createNodes(path, parentNode, value)); @@ -213,7 +216,7 @@ public class RelationalEntityWriter implements EntityWriter nodes = new ArrayList<>(); + List nodes = new ArrayList<>(); if (path.getRequiredLeafProperty().isQualified()) { @@ -235,17 +238,26 @@ public class RelationalEntityWriter implements EntityWriter path; - private final @Nullable PathNode parent; - private final Object value; - } + /** + * Represents a single entity in an aggregate along with its property path from the root entity and the chain of objects + * to traverse a long this path. + */ + @Value + static class PathNode { + + /** The path to this entity */ + PersistentPropertyPath path; + + /** + * The parent {@link PathNode}. This is {@code null} if this is + * the root entity. + */ + @Nullable + PathNode parent; + + /** The value of the entity. */ + Object value; } } diff --git a/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java b/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java index 6ddb2714..8c916733 100644 --- a/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java +++ b/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java @@ -129,15 +129,6 @@ class BasicRelationalPersistentProperty extends AnnotationBasedPersistentPropert return isListLike(); } - /* - * (non-Javadoc) - * @see org.springframework.data.mapping.model.AbstractPersistentProperty#isImmutable() - */ - @Override - public boolean isImmutable() { - return false; - } - private boolean isListLike() { return isCollectionLike() && !Set.class.isAssignableFrom(this.getType()); } diff --git a/src/test/java/org/springframework/data/jdbc/core/JdbcEntityTemplateIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java similarity index 92% rename from src/test/java/org/springframework/data/jdbc/core/JdbcEntityTemplateIntegrationTests.java rename to src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java index 5d023d7e..22d46b1f 100644 --- a/src/test/java/org/springframework/data/jdbc/core/JdbcEntityTemplateIntegrationTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java @@ -15,9 +15,8 @@ */ package org.springframework.data.jdbc.core; -import static java.util.Collections.singletonList; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.tuple; +import static java.util.Collections.*; +import static org.assertj.core.api.Assertions.*; import lombok.Data; @@ -32,6 +31,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.testing.TestConfiguration; +import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.rules.SpringClassRule; @@ -45,12 +45,11 @@ import org.springframework.transaction.annotation.Transactional; */ @ContextConfiguration @Transactional -public class JdbcEntityTemplateIntegrationTests { +public class AggregateTemplateIntegrationTests { @ClassRule public static final SpringClassRule classRule = new SpringClassRule(); @Rule public SpringMethodRule methodRule = new SpringMethodRule(); - @Autowired - JdbcAggregateOperations template; + @Autowired JdbcAggregateOperations template; LegoSet legoSet = createLegoSet(); @@ -248,12 +247,13 @@ public class JdbcEntityTemplateIntegrationTests { @Bean Class testClass() { - return JdbcEntityTemplateIntegrationTests.class; + return AggregateTemplateIntegrationTests.class; } @Bean - JdbcAggregateOperations operations(ApplicationEventPublisher publisher, RelationalMappingContext context, DataAccessStrategy dataAccessStrategy) { - return new JdbcAggregateTemplate(publisher, context, dataAccessStrategy); + JdbcAggregateOperations operations(ApplicationEventPublisher publisher, RelationalMappingContext context, + DataAccessStrategy dataAccessStrategy, RelationalConverter converter) { + return new JdbcAggregateTemplate(publisher, context, converter, dataAccessStrategy); } } } diff --git a/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java index c636dcd4..3db867d2 100644 --- a/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java @@ -60,9 +60,10 @@ public class DefaultJdbcInterpreterUnitTests { Element element = new Element(); InsertRoot containerInsert = new InsertRoot<>(container); - containerInsert.setResultingEntity(container); + containerInsert.setGeneratedId(CONTAINER_ID); - Insert insert = new Insert<>(element, PropertyPathUtils.toPath("element", Container.class, context), containerInsert); + Insert insert = new Insert<>(element, PropertyPathUtils.toPath("element", Container.class, context), + containerInsert); interpreter.interpret(insert); diff --git a/src/test/java/org/springframework/data/jdbc/core/ImmutableAggregateTemplateHsqlIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/core/ImmutableAggregateTemplateHsqlIntegrationTests.java new file mode 100644 index 00000000..01395161 --- /dev/null +++ b/src/test/java/org/springframework/data/jdbc/core/ImmutableAggregateTemplateHsqlIntegrationTests.java @@ -0,0 +1,272 @@ +/* + * Copyright 2017-2018 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.Collections.*; +import static org.assertj.core.api.Assertions.*; + +import lombok.Value; +import lombok.experimental.Wither; + +import org.assertj.core.api.SoftAssertions; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; +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.testing.TestConfiguration; +import org.springframework.data.relational.core.conversion.RelationalConverter; +import org.springframework.data.relational.core.mapping.RelationalMappingContext; +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; + +/** + * Integration tests for {@link JdbcAggregateTemplate} and it's handling of immutable entities. + * + * @author Jens Schauder + */ +@ContextConfiguration +@Transactional +public class ImmutableAggregateTemplateHsqlIntegrationTests { + + @ClassRule public static final SpringClassRule classRule = new SpringClassRule(); + @Rule public SpringMethodRule methodRule = new SpringMethodRule(); + @Autowired JdbcAggregateOperations template; + + @Test // DATAJDBC-241 + public void saveWithGeneratedIdCreatesNewInstance() { + + LegoSet legoSet = createLegoSet(createManual()); + + LegoSet saved = template.save(legoSet); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(legoSet).isNotSameAs(saved); + softly.assertThat(legoSet.getId()).isNull(); + + softly.assertThat(saved.getId()).isNotNull(); + softly.assertThat(saved.name).isNotNull(); + softly.assertThat(saved.manual).isNotNull(); + softly.assertThat(saved.manual.content).isNotNull(); + + softly.assertAll(); + } + + @Test // DATAJDBC-241 + public void saveAndLoadAnEntityWithReferencedEntityById() { + + LegoSet saved = template.save(createLegoSet(createManual())); + + assertThat(saved.manual.id).describedAs("id of stored manual").isNotNull(); + + LegoSet reloadedLegoSet = template.findById(saved.getId(), LegoSet.class); + + assertThat(reloadedLegoSet.manual).isNotNull(); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(reloadedLegoSet.manual.getId()) // + .isEqualTo(saved.getManual().getId()) // + .isNotNull(); + softly.assertThat(reloadedLegoSet.manual.getContent()).isEqualTo(saved.getManual().getContent()); + + softly.assertAll(); + } + + @Test // DATAJDBC-241 + public void saveAndLoadManyEntitiesWithReferencedEntity() { + + LegoSet legoSet = createLegoSet(createManual()); + + LegoSet savedLegoSet = template.save(legoSet); + + Iterable reloadedLegoSets = template.findAll(LegoSet.class); + + assertThat(reloadedLegoSets).hasSize(1).extracting("id", "manual.id", "manual.content") + .contains(tuple(savedLegoSet.getId(), savedLegoSet.getManual().getId(), savedLegoSet.getManual().getContent())); + } + + @Test // DATAJDBC-241 + public void saveAndLoadManyEntitiesByIdWithReferencedEntity() { + + LegoSet saved = template.save(createLegoSet(createManual())); + + Iterable reloadedLegoSets = template.findAllById(singletonList(saved.getId()), LegoSet.class); + + assertThat(reloadedLegoSets).hasSize(1).extracting("id", "manual.id", "manual.content") + .contains(tuple(saved.getId(), saved.getManual().getId(), saved.getManual().getContent())); + } + + @Test // DATAJDBC-241 + public void saveAndLoadAnEntityWithReferencedNullEntity() { + + LegoSet saved = template.save(createLegoSet(null)); + + LegoSet reloadedLegoSet = template.findById(saved.getId(), LegoSet.class); + + assertThat(reloadedLegoSet.manual).isNull(); + } + + @Test // DATAJDBC-241 + public void saveAndDeleteAnEntityWithReferencedEntity() { + + LegoSet legoSet = createLegoSet(createManual()); + + LegoSet saved = template.save(legoSet); + + template.delete(saved, LegoSet.class); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(template.findAll(LegoSet.class)).isEmpty(); + softly.assertThat(template.findAll(Manual.class)).isEmpty(); + + softly.assertAll(); + } + + @Test // DATAJDBC-241 + public void saveAndDeleteAllWithReferencedEntity() { + + template.save(createLegoSet(createManual())); + + template.deleteAll(LegoSet.class); + + SoftAssertions softly = new SoftAssertions(); + + assertThat(template.findAll(LegoSet.class)).isEmpty(); + assertThat(template.findAll(Manual.class)).isEmpty(); + + softly.assertAll(); + } + + @Test // DATAJDBC-241 + public void updateReferencedEntityFromNull() { + + LegoSet saved = template.save(createLegoSet(null)); + + LegoSet changedLegoSet = new LegoSet(saved.id, saved.name, new Manual(23L, "Some content")); + + template.save(changedLegoSet); + + LegoSet reloadedLegoSet = template.findById(saved.getId(), LegoSet.class); + + assertThat(reloadedLegoSet.manual.content).isEqualTo("Some content"); + } + + @Test // DATAJDBC-241 + public void updateReferencedEntityToNull() { + + LegoSet saved = template.save(createLegoSet(null)); + + LegoSet changedLegoSet = new LegoSet(saved.id, saved.name, null); + + template.save(changedLegoSet); + + LegoSet reloadedLegoSet = template.findById(saved.getId(), LegoSet.class); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(reloadedLegoSet.manual).isNull(); + softly.assertThat(template.findAll(Manual.class)).describedAs("Manuals failed to delete").isEmpty(); + + softly.assertAll(); + } + + @Test // DATAJDBC-241 + public void replaceReferencedEntity() { + + LegoSet saved = template.save(createLegoSet(null)); + + LegoSet changedLegoSet = new LegoSet(saved.id, saved.name, new Manual(null, "other content")); + + template.save(changedLegoSet); + + LegoSet reloadedLegoSet = template.findById(saved.getId(), LegoSet.class); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(reloadedLegoSet.manual.content).isEqualTo("other content"); + softly.assertThat(template.findAll(Manual.class)).describedAs("There should be only one manual").hasSize(1); + + softly.assertAll(); + } + + @Test // DATAJDBC-241 + public void changeReferencedEntity() { + + LegoSet saved = template.save(createLegoSet(createManual())); + + LegoSet changedLegoSet = saved.withManual(saved.manual.withContent("new content")); + + template.save(changedLegoSet); + + LegoSet reloadedLegoSet = template.findById(saved.getId(), LegoSet.class); + + Manual manual = reloadedLegoSet.manual; + assertThat(manual).isNotNull(); + assertThat(manual.content).isEqualTo("new content"); + } + + private static LegoSet createLegoSet(Manual manual) { + + return new LegoSet(null, "Star Destroyer", manual); + } + + private static Manual createManual() { + return new Manual(null, + "Accelerates to 99% of light speed. Destroys almost everything. See https://what-if.xkcd.com/1/"); + } + + @Value + @Wither + static class LegoSet { + + @Id Long id; + String name; + Manual manual; + } + + @Value + @Wither + static class Manual { + + @Id Long id; + String content; + } + + @Configuration + @Import(TestConfiguration.class) + static class Config { + + @Bean + Class testClass() { + return ImmutableAggregateTemplateHsqlIntegrationTests.class; + } + + @Bean + JdbcAggregateOperations operations(ApplicationEventPublisher publisher, RelationalMappingContext context, + DataAccessStrategy dataAccessStrategy, RelationalConverter converter) { + return new JdbcAggregateTemplate(publisher, context, converter, dataAccessStrategy); + } + } +} diff --git a/src/test/java/org/springframework/data/jdbc/mybatis/DummyEntity.java b/src/test/java/org/springframework/data/jdbc/mybatis/DummyEntity.java index 796fd6b2..8b08a36c 100644 --- a/src/test/java/org/springframework/data/jdbc/mybatis/DummyEntity.java +++ b/src/test/java/org/springframework/data/jdbc/mybatis/DummyEntity.java @@ -15,6 +15,7 @@ */ package org.springframework.data.jdbc.mybatis; +import lombok.experimental.Wither; import org.apache.ibatis.type.Alias; import org.springframework.data.annotation.Id; @@ -24,6 +25,7 @@ import org.springframework.data.annotation.Id; @Alias("DummyEntity") class DummyEntity { + @Wither @Id final Long id; final String name; diff --git a/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java b/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java new file mode 100644 index 00000000..c5a60605 --- /dev/null +++ b/src/test/java/org/springframework/data/relational/core/conversion/AggregateChangeUnitTests.java @@ -0,0 +1,132 @@ +/* + * Copyright 2018 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.core.conversion; + +import static org.assertj.core.api.Assertions.*; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.junit.Test; +import org.springframework.data.annotation.Id; +import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.relational.core.mapping.RelationalMappingContext; + +/** + * Unit tests for the {@link AggregateChange}. + * + * @author Jens Schauder + */ +public class AggregateChangeUnitTests { + + DummyEntity entity = new DummyEntity(); + Content content = new Content(); + + RelationalMappingContext context = new RelationalMappingContext(); + RelationalConverter converter = new BasicRelationalConverter(context); + + PersistentPropertyAccessor propertyAccessor = context.getRequiredPersistentEntity(DummyEntity.class) + .getPropertyAccessor(entity); + Object id = 23; + + DbAction.WithEntity rootInsert = new DbAction.InsertRoot<>(entity); + + 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); + + return insert; + } + + @Test // DATAJDBC-241 + public void setIdForSimpleReference() { + + entity.single = content; + + DbAction.Insert insert = createInsert("single", content, null); + + AggregateChange.setId(context, converter, propertyAccessor, insert, id); + + DummyEntity result = propertyAccessor.getBean(); + + assertThat(result.single.id).isEqualTo(id); + } + + @Test // DATAJDBC-241 + public void setIdForSingleElementSet() { + + entity.contentSet.add(content); + + DbAction.Insert insert = createInsert("contentSet", content, null); + + AggregateChange.setId(context, converter, propertyAccessor, insert, id); + + DummyEntity result = propertyAccessor.getBean(); + assertThat(result.contentSet).isNotNull(); + assertThat(result.contentSet).extracting(c -> c == null ? "null" : c.id).containsExactlyInAnyOrder(23); + } + + @Test // DATAJDBC-241 + public void setIdForSingleElementList() { + + entity.contentList.add(content); + + DbAction.Insert insert = createInsert("contentList", content, 0); + + AggregateChange.setId(context, converter, propertyAccessor, insert, id); + + DummyEntity result = propertyAccessor.getBean(); + assertThat(result.contentList).extracting(c -> c.id).containsExactlyInAnyOrder(23); + } + + @Test // DATAJDBC-241 + public void setIdForSingleElementMap() { + + entity.contentMap.put("one", content); + + DbAction.Insert insert = createInsert("contentMap", content, "one"); + + AggregateChange.setId(context, converter, propertyAccessor, insert, id); + + DummyEntity result = propertyAccessor.getBean(); + assertThat(result.contentMap.entrySet()).extracting(e -> e.getKey(), e -> e.getValue().id) + .containsExactlyInAnyOrder(tuple("one", 23)); + } + + private static class DummyEntity { + + @Id Integer rootId; + + Content single; + + Set contentSet = new HashSet<>(); + + List contentList = new ArrayList<>(); + + Map contentMap = new HashMap<>(); + } + + private static class Content { + + @Id Integer id; + } +} diff --git a/src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-hsql.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-hsql.sql similarity index 100% rename from src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-hsql.sql rename to src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-hsql.sql diff --git a/src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-mariadb.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mariadb.sql similarity index 100% rename from src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-mariadb.sql rename to src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mariadb.sql diff --git a/src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-mysql.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mysql.sql similarity index 100% rename from src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-mysql.sql rename to src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mysql.sql diff --git a/src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-postgres.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-postgres.sql similarity index 100% rename from src/test/resources/org.springframework.data.jdbc.core/JdbcEntityTemplateIntegrationTests-postgres.sql rename to src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-postgres.sql diff --git a/src/test/resources/org.springframework.data.jdbc.core/ImmutableAggregateTemplateHsqlIntegrationTests-hsql.sql b/src/test/resources/org.springframework.data.jdbc.core/ImmutableAggregateTemplateHsqlIntegrationTests-hsql.sql new file mode 100644 index 00000000..20a0a290 --- /dev/null +++ b/src/test/resources/org.springframework.data.jdbc.core/ImmutableAggregateTemplateHsqlIntegrationTests-hsql.sql @@ -0,0 +1,5 @@ +CREATE TABLE LEGO_SET ( id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) PRIMARY KEY, NAME VARCHAR(30)); +CREATE TABLE MANUAL ( id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) PRIMARY KEY, LEGO_SET BIGINT, CONTENT VARCHAR(2000)); + +ALTER TABLE MANUAL ADD FOREIGN KEY (LEGO_SET) +REFERENCES LEGO_SET(id);