diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index 22399bb9..aa9d7fc3 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -50,6 +50,7 @@ import org.springframework.util.Assert; * @author Myeonghyeon Lee * @author Chirag Tailor */ +@SuppressWarnings("rawtypes") class JdbcAggregateChangeExecutionContext { private static final String UPDATE_FAILED = "Failed to update entity [%s]. Id [%s] not found in database."; @@ -192,7 +193,7 @@ class JdbcAggregateChangeExecutionContext { private DbAction.WithEntity getIdOwningAction(DbAction.WithEntity action, PersistentPropertyPathExtension idPath) { - if (!(action instanceof DbAction.WithDependingOn)) { + if (!(action instanceof DbAction.WithDependingOn withDependingOn)) { Assert.state(idPath.getLength() == 0, "When the id path is not empty the id providing action should be of type WithDependingOn"); @@ -200,8 +201,6 @@ class JdbcAggregateChangeExecutionContext { return action; } - DbAction.WithDependingOn withDependingOn = (DbAction.WithDependingOn) action; - if (idPath.matches(withDependingOn.getPropertyPath())) { return action; } @@ -257,9 +256,8 @@ class JdbcAggregateChangeExecutionContext { roots.add((T) newEntity); } - // the id property was immutable so we have to propagate changes up the tree - if (newEntity != action.getEntity() && action instanceof DbAction.Insert) { - DbAction.Insert insert = (DbAction.Insert) action; + // the id property was immutable, so we have to propagate changes up the tree + if (newEntity != action.getEntity() && action instanceof DbAction.Insert insert) { Pair qualifier = insert.getQualifier(); @@ -463,6 +461,7 @@ class JdbcAggregateChangeExecutionContext { public List add(@Nullable List list, @Nullable Object qualifier, Object value) { Assert.notNull(list, "List must not be null."); + Assert.notNull(qualifier, "ListAggregator can't handle a null qualifier."); int index = (int) qualifier; if (index >= list.size()) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java index d79b8d9d..0f88b00f 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java @@ -21,7 +21,7 @@ import org.springframework.data.domain.Sort; import org.springframework.lang.Nullable; /** - * Specifies a operations one can perform on a database, based on an Domain Type. + * Specifies operations one can perform on a database, based on an Domain Type. * * @author Jens Schauder * @author Thomas Lang @@ -90,7 +90,7 @@ public interface JdbcAggregateOperations { void deleteAllById(Iterable ids, Class domainType); /** - * Delete an aggregate identified by it's aggregate root. + * Delete an aggregate identified by its aggregate root. * * @param aggregateRoot to delete. Must not be {@code null}. * @param domainType the type of the aggregate root. Must not be {@code null}. diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java index 426a1883..fc0494e4 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java @@ -287,6 +287,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { .forDelete(domainType); ids.forEach(id -> { + DeleteAggregateChange change = createDeletingChange(id, null, domainType); triggerBeforeDelete(null, id, change); batchingAggregateChange.add(change); @@ -316,6 +317,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { Map instancesBeforeExecute = new LinkedHashMap<>(); instances.forEach(instance -> { + Object id = context.getRequiredPersistentEntity(domainType).getIdentifierAccessor(instance) .getRequiredIdentifier(); DeleteAggregateChange change = createDeletingChange(id, instance, domainType); @@ -384,6 +386,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { for (T instance : instances) { if (batchingAggregateChange == null) { + // noinspection unchecked batchingAggregateChange = BatchingAggregateChange.forSave((Class) ClassUtils.getUserClass(instance)); } batchingAggregateChange.add(beforeExecute(instance, changeCreatorSelectorForSave(instance))); @@ -540,15 +543,6 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { return null; } - private static class EntityAndPreviousVersion { - - private final T entity; - private final Number version; - - EntityAndPreviousVersion(T entity, @Nullable Number version) { - - this.entity = entity; - this.version = version; - } + private record EntityAndPreviousVersion (T entity, @Nullable Number version) { } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java index f790d67c..c652fb4e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java @@ -27,6 +27,8 @@ import org.springframework.data.relational.core.conversion.IdValueSource; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.sql.LockMode; +import static java.lang.Boolean.*; + /** * Delegates each methods to the {@link DataAccessStrategy}s passed to the constructor in turn until the first that does * not throw an exception. @@ -155,7 +157,6 @@ public class CascadingDataAccessStrategy implements DataAccessStrategy { private T collect(Function function) { - // Keep as Eclipse fails to compile if <> is used. return strategies.stream().collect(new FunctionCollector<>(function)); } @@ -163,7 +164,7 @@ public class CascadingDataAccessStrategy implements DataAccessStrategy { collect(das -> { consumer.accept(das); - return null; + return TRUE; }); } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 3652bbd5..605352e2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -183,8 +183,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { @Override public void delete(Object rootId, PersistentPropertyPath propertyPath) { - RelationalPersistentEntity rootEntity = context - .getRequiredPersistentEntity(propertyPath.getBaseProperty().getOwner().getType()); + RelationalPersistentEntity rootEntity = context.getRequiredPersistentEntity(getBaseType(propertyPath)); RelationalPersistentProperty referencingProperty = propertyPath.getLeafProperty(); Assert.notNull(referencingProperty, "No property found matching the PropertyPath " + propertyPath); @@ -199,8 +198,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { @Override public void delete(Iterable rootIds, PersistentPropertyPath propertyPath) { - RelationalPersistentEntity rootEntity = context - .getRequiredPersistentEntity(propertyPath.getBaseProperty().getOwner().getType()); + RelationalPersistentEntity rootEntity = context.getRequiredPersistentEntity(getBaseType(propertyPath)); RelationalPersistentProperty referencingProperty = propertyPath.getLeafProperty(); @@ -220,8 +218,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { @Override public void deleteAll(PersistentPropertyPath propertyPath) { - operations.getJdbcOperations() - .update(sql(propertyPath.getBaseProperty().getOwner().getType()).createDeleteAllSql(propertyPath)); + operations.getJdbcOperations().update(sql(getBaseType(propertyPath)).createDeleteAllSql(propertyPath)); } @Override @@ -365,4 +362,13 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { return Optional.ofNullable(context.getRequiredPersistentEntity(domainType).getIdProperty()) .map(RelationalPersistentProperty::getColumnName).orElse(null); } + + private Class getBaseType(PersistentPropertyPath propertyPath) { + + RelationalPersistentProperty baseProperty = propertyPath.getBaseProperty(); + + Assert.notNull(baseProperty, "The base property must not be null"); + + return baseProperty.getOwner().getType(); + } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index b84c1923..ba03abd2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -103,7 +103,7 @@ class SqlGenerator { } /** - * Construct a IN-condition based on a {@link Select Sub-Select} which selects the ids (or stand ins for ids) of the + * Construct an IN-condition based on a {@link Select Sub-Select} which selects the ids (or stand-ins for ids) of the * given {@literal path} to those that reference the root entities specified by the {@literal rootCondition}. * * @param path specifies the table and id to select @@ -135,7 +135,7 @@ class SqlGenerator { innerCondition = rootCondition.apply(selectFilterColumn); } else { - // otherwise we need another layer of subselect + // otherwise, we need another layer of subselect innerCondition = getSubselectCondition(parentPath, rootCondition, selectFilterColumn); } @@ -502,7 +502,7 @@ class SqlGenerator { @Nullable Column getColumn(PersistentPropertyPathExtension path) { - // an embedded itself doesn't give an column, its members will though. + // an embedded itself doesn't give a column, its members will though. // if there is a collection or map on the path it won't get selected at all, but it will get loaded with a separate // select // only the parent path is considered in order to handle arrays that get stored as BINARY properly @@ -512,7 +512,7 @@ class SqlGenerator { if (path.isEntity()) { - // Simple entities without id include there backreference as an synthetic id in order to distinguish null entities + // Simple entities without id include there backreference as a synthetic id in order to distinguish null entities // from entities with only null values. if (path.isQualified() // @@ -622,7 +622,7 @@ class SqlGenerator { Table table = getTable(); - List assignments = columns.getUpdateableColumns() // + List assignments = columns.getUpdatableColumns() // .stream() // .map(columnName -> Assignments.value( // table.column(columnName), // @@ -807,7 +807,7 @@ class SqlGenerator { private final List nonIdColumnNames = new ArrayList<>(); private final Set readOnlyColumnNames = new HashSet<>(); private final Set insertableColumns; - private final Set updateableColumns; + private final Set updatableColumns; Columns(RelationalPersistentEntity entity, MappingContext, RelationalPersistentProperty> mappingContext, @@ -823,12 +823,12 @@ class SqlGenerator { this.insertableColumns = Collections.unmodifiableSet(insertable); - Set updateable = new LinkedHashSet<>(columnNames); + Set updatable = new LinkedHashSet<>(columnNames); - updateable.removeAll(idColumnNames); - updateable.removeAll(readOnlyColumnNames); + updatable.removeAll(idColumnNames); + updatable.removeAll(readOnlyColumnNames); - this.updateableColumns = Collections.unmodifiableSet(updateable); + this.updatableColumns = Collections.unmodifiableSet(updatable); } private void populateColumnNameCache(RelationalPersistentEntity entity, String prefix) { @@ -881,8 +881,8 @@ class SqlGenerator { /** * @return Column names that can be used for {@code UPDATE}. */ - Set getUpdateableColumns() { - return updateableColumns; + Set getUpdatableColumns() { + return updatableColumns; } } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 67fee8a9..53ecb788 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -21,10 +21,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.ibatis.session.SqlSession; import org.mybatis.spring.SqlSessionTemplate; import org.springframework.dao.EmptyResultDataAccessException; @@ -63,11 +60,9 @@ import org.springframework.util.Assert; */ public class MyBatisDataAccessStrategy implements DataAccessStrategy { - private static final Log LOG = LogFactory.getLog(MyBatisDataAccessStrategy.class); private static final String VERSION_SQL_PARAMETER_NAME_OLD = "___oldOptimisticLockingVersion"; private final SqlSession sqlSession; - private final IdentifierProcessing identifierProcessing; private NamespaceStrategy namespaceStrategy = NamespaceStrategy.DEFAULT_INSTANCE; /** @@ -133,7 +128,6 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { public MyBatisDataAccessStrategy(SqlSession sqlSession, IdentifierProcessing identifierProcessing) { this.sqlSession = sqlSession; - this.identifierProcessing = identifierProcessing; } /** @@ -210,7 +204,7 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { @Override public void delete(Object rootId, PersistentPropertyPath propertyPath) { - Class ownerType = propertyPath.getBaseProperty().getOwner().getType(); + Class ownerType = getOwnerTyp(propertyPath); String statement = namespace(ownerType) + ".delete-" + toDashPath(propertyPath); Class leafType = propertyPath.getRequiredLeafProperty().getTypeInformation().getType(); MyBatisContext parameter = new MyBatisContext(rootId, null, leafType, Collections.emptyMap()); @@ -234,10 +228,9 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { @Override public void deleteAll(PersistentPropertyPath propertyPath) { - Class baseType = propertyPath.getBaseProperty().getOwner().getType(); Class leafType = propertyPath.getRequiredLeafProperty().getTypeInformation().getType(); - String statement = namespace(baseType) + ".deleteAll-" + toDashPath(propertyPath); + String statement = namespace(getOwnerTyp(propertyPath)) + ".deleteAll-" + toDashPath(propertyPath); MyBatisContext parameter = new MyBatisContext(null, null, leafType, Collections.emptyMap()); sqlSession().delete(statement, parameter); } @@ -291,8 +284,7 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { public Iterable findAllByPath(Identifier identifier, PersistentPropertyPath path) { - String statementName = namespace(path.getBaseProperty().getOwner().getType()) + ".findAllByPath-" - + path.toDotPath(); + String statementName = namespace(getOwnerTyp(path)) + ".findAllByPath-" + path.toDotPath(); return sqlSession().selectList(statementName, new MyBatisContext(identifier, null, path.getRequiredLeafProperty().getType())); @@ -333,12 +325,6 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { return sqlSession().selectOne(statement, parameter); } - private Map convertToParameterMap(Map additionalParameters) { - - return additionalParameters.entrySet().stream() // - .collect(Collectors.toMap(e -> e.getKey().toSql(identifierProcessing), Map.Entry::getValue)); - } - private String namespace(Class domainType) { return this.namespaceStrategy.getNamespace(domainType); } @@ -348,6 +334,20 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { } private static String toDashPath(PersistentPropertyPath propertyPath) { - return propertyPath.toDotPath().replaceAll("\\.", "-"); + + String dotPath = propertyPath.toDotPath(); + if (dotPath == null) { + return ""; + } + return dotPath.replaceAll("\\.", "-"); + } + + private Class getOwnerTyp(PersistentPropertyPath propertyPath) { + + RelationalPersistentProperty baseProperty = propertyPath.getBaseProperty(); + + Assert.notNull(baseProperty, "BaseProperty must not be null."); + + return baseProperty.getOwner().getType(); } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationImmutableUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationImmutableUnitTests.java index 937c8b7f..1a38cc80 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationImmutableUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationImmutableUnitTests.java @@ -406,11 +406,9 @@ public class AggregateChangeIdGenerationImmutableUnitTests { DbAction.Insert createInsert(String propertyName, Object value, @Nullable Object key) { - DbAction.Insert insert = new DbAction.Insert<>(value, + return new DbAction.Insert<>(value, context.getPersistentPropertyPath(propertyName, DummyEntity.class), rootInsert, singletonMap(toPath(propertyName), key), IdValueSource.GENERATED); - - return insert; } DbAction.Insert createDeepInsert(String propertyName, Object value, Object key, @@ -418,10 +416,9 @@ public class AggregateChangeIdGenerationImmutableUnitTests { PersistentPropertyPath propertyPath = toPath( parentInsert.getPropertyPath().toDotPath() + "." + propertyName); - DbAction.Insert insert = new DbAction.Insert<>(value, propertyPath, parentInsert, - singletonMap(propertyPath, key), IdValueSource.GENERATED); - return insert; + return new DbAction.Insert<>(value, propertyPath, parentInsert, + singletonMap(propertyPath, key), IdValueSource.GENERATED); } PersistentPropertyPath toPath(String path) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationUnitTests.java index 734e27e9..e83ca63b 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationUnitTests.java @@ -380,11 +380,11 @@ public class AggregateChangeIdGenerationUnitTests { @Id Integer id; } - private static class IncrementingIds implements Answer { + private static class IncrementingIds implements Answer { long id = 1; @Override - public Object answer(InvocationOnMock invocation) throws Throwable { + public Object answer(InvocationOnMock invocation) { if (!invocation.getMethod().getReturnType().equals(Object.class)) { throw new UnsupportedOperationException("This mock does not support this invocation: " + invocation); @@ -392,16 +392,5 @@ public class AggregateChangeIdGenerationUnitTests { return id++; } - - private DbAction findAction(Object[] arguments) { - - for (Object argument : arguments) { - - if (argument instanceof DbAction) { - return (DbAction) argument; - } - } - return null; - } } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java index 88bd07f9..cc5aed74 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java @@ -84,8 +84,8 @@ public class JdbcAggregateChangeExecutorContextImmutableUnitTests { when(accessStrategy.insert(any(DummyEntity.class), eq(DummyEntity.class), eq(Identifier.empty()), eq(IdValueSource.GENERATED))).thenReturn(23L); - when(accessStrategy.insert(any(Content.class), eq(Content.class), eq(createBackRef(23L)), eq(IdValueSource.GENERATED))) - .thenReturn(24L); + when(accessStrategy.insert(any(Content.class), eq(Content.class), eq(createBackRef(23L)), + eq(IdValueSource.GENERATED))).thenReturn(24L); DbAction.InsertRoot rootInsert = new DbAction.InsertRoot<>(root, IdValueSource.GENERATED); executionContext.executeInsertRoot(rootInsert); @@ -134,7 +134,6 @@ public class JdbcAggregateChangeExecutorContextImmutableUnitTests { when(accessStrategy.insert(content1, Content.class, createBackRef(123L), IdValueSource.GENERATED)).thenReturn(11L); executionContext.executeInsert(createInsert(rootUpdate1, "content", content1, null)); - DummyEntity root2 = new DummyEntity(); DbAction.InsertRoot rootInsert2 = new DbAction.InsertRoot<>(root2, IdValueSource.GENERATED); when(accessStrategy.insert(root2, DummyEntity.class, Identifier.empty(), IdValueSource.GENERATED)).thenReturn(456L); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java index 62a7df91..0187bb84 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java @@ -167,9 +167,10 @@ public class JdbcAggregateChangeExecutorContextUnitTests { @Test // GH-537 void batchInsertRootOperation_withGeneratedIds() { - when(accessStrategy.insert(singletonList(InsertSubject.describedBy(root, Identifier.empty())), DummyEntity.class, IdValueSource.GENERATED)) - .thenReturn(new Object[] { 123L }); - executionContext.executeBatchInsertRoot(new DbAction.BatchInsertRoot<>(singletonList(new DbAction.InsertRoot<>(root, IdValueSource.GENERATED)))); + when(accessStrategy.insert(singletonList(InsertSubject.describedBy(root, Identifier.empty())), DummyEntity.class, + IdValueSource.GENERATED)).thenReturn(new Object[] { 123L }); + executionContext.executeBatchInsertRoot( + new DbAction.BatchInsertRoot<>(singletonList(new DbAction.InsertRoot<>(root, IdValueSource.GENERATED)))); List newRoots = executionContext.populateIdsIfNecessary(); @@ -180,9 +181,10 @@ public class JdbcAggregateChangeExecutorContextUnitTests { @Test // GH-537 void batchInsertRootOperation_withoutGeneratedIds() { - when(accessStrategy.insert(singletonList(InsertSubject.describedBy(root, Identifier.empty())), DummyEntity.class, IdValueSource.PROVIDED)) - .thenReturn(new Object[] { null }); - executionContext.executeBatchInsertRoot(new DbAction.BatchInsertRoot<>(singletonList(new DbAction.InsertRoot<>(root, IdValueSource.PROVIDED)))); + when(accessStrategy.insert(singletonList(InsertSubject.describedBy(root, Identifier.empty())), DummyEntity.class, + IdValueSource.PROVIDED)).thenReturn(new Object[] { null }); + executionContext.executeBatchInsertRoot( + new DbAction.BatchInsertRoot<>(singletonList(new DbAction.InsertRoot<>(root, IdValueSource.PROVIDED)))); List newRoots = executionContext.populateIdsIfNecessary(); @@ -242,7 +244,7 @@ public class JdbcAggregateChangeExecutorContextUnitTests { } DbAction.Insert createInsert(DbAction.WithEntity parent, String propertyName, Object value, - @Nullable Object key, IdValueSource idValueSource) { + @Nullable Object key, IdValueSource idValueSource) { return new DbAction.Insert<>(value, getPersistentPropertyPath(propertyName), parent, key == null ? emptyMap() : singletonMap(toPath(propertyName), key), idValueSource); @@ -269,6 +271,7 @@ public class JdbcAggregateChangeExecutorContextUnitTests { .orElseThrow(() -> new IllegalArgumentException("No matching path found")); } + @SuppressWarnings("unused") private static class DummyEntity { @Id Long id; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index 13c07a50..cfc60e81 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -49,7 +49,7 @@ import org.springframework.context.annotation.Import; import org.springframework.dao.IncorrectUpdateSemanticsDataAccessException; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.annotation.Id; -import org.springframework.data.annotation.PersistenceConstructor; +import org.springframework.data.annotation.PersistenceCreator; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Version; import org.springframework.data.domain.PageRequest; @@ -205,14 +205,13 @@ class JdbcAggregateTemplateIntegrationTests { assertThat(reloadedLegoSet.manual).isNotNull(); - SoftAssertions softly = new SoftAssertions(); + assertSoftly(softly -> { + softly.assertThat(reloadedLegoSet.manual.getId()) // + .isEqualTo(legoSet.getManual().getId()) // + .isNotNull(); + softly.assertThat(reloadedLegoSet.manual.getContent()).isEqualTo(legoSet.getManual().getContent()); + }); - softly.assertThat(reloadedLegoSet.manual.getId()) // - .isEqualTo(legoSet.getManual().getId()) // - .isNotNull(); - softly.assertThat(reloadedLegoSet.manual.getContent()).isEqualTo(legoSet.getManual().getContent()); - - softly.assertAll(); } @Test // DATAJDBC-112 @@ -307,12 +306,11 @@ class JdbcAggregateTemplateIntegrationTests { template.delete(legoSet, LegoSet.class); - SoftAssertions softly = new SoftAssertions(); + assertSoftly(softly -> { - softly.assertThat(template.findAll(LegoSet.class)).isEmpty(); - softly.assertThat(template.findAll(Manual.class)).isEmpty(); - - softly.assertAll(); + softly.assertThat(template.findAll(LegoSet.class)).isEmpty(); + softly.assertThat(template.findAll(Manual.class)).isEmpty(); + }); } @Test // DATAJDBC-112 @@ -323,44 +321,46 @@ class JdbcAggregateTemplateIntegrationTests { template.deleteAll(LegoSet.class); - SoftAssertions softly = new SoftAssertions(); + assertSoftly(softly -> { - assertThat(template.findAll(LegoSet.class)).isEmpty(); - assertThat(template.findAll(Manual.class)).isEmpty(); + softly.assertThat(template.findAll(LegoSet.class)).isEmpty(); + softly.assertThat(template.findAll(Manual.class)).isEmpty(); + }); - softly.assertAll(); } @Test // GH-537 @EnabledOnFeature(SUPPORTS_QUOTED_IDS) void saveAndDeleteAllByAggregateRootsWithReferencedEntity() { + LegoSet legoSet1 = template.save(legoSet); LegoSet legoSet2 = template.save(createLegoSet("Some Name")); + template.save(createLegoSet("Some other Name")); template.deleteAll(List.of(legoSet1, legoSet2), LegoSet.class); - SoftAssertions softly = new SoftAssertions(); + assertSoftly(softly -> { - assertThat(template.findAll(LegoSet.class)).isEmpty(); - assertThat(template.findAll(Manual.class)).isEmpty(); - - softly.assertAll(); + softly.assertThat(template.findAll(LegoSet.class)).extracting(l -> l.name).containsExactly("Some other Name"); + softly.assertThat(template.findAll(Manual.class)).hasSize(1); + }); } @Test // GH-537 @EnabledOnFeature(SUPPORTS_QUOTED_IDS) void saveAndDeleteAllByIdsWithReferencedEntity() { + LegoSet legoSet1 = template.save(legoSet); LegoSet legoSet2 = template.save(createLegoSet("Some Name")); + template.save(createLegoSet("Some other Name")); template.deleteAllById(List.of(legoSet1.id, legoSet2.id), LegoSet.class); - SoftAssertions softly = new SoftAssertions(); + assertSoftly(softly -> { - assertThat(template.findAll(LegoSet.class)).isEmpty(); - assertThat(template.findAll(Manual.class)).isEmpty(); - - softly.assertAll(); + softly.assertThat(template.findAll(LegoSet.class)).extracting(l -> l.name).containsExactly("Some other Name"); + softly.assertThat(template.findAll(Manual.class)).hasSize(1); + }); } @Test // DATAJDBC-112 @@ -427,12 +427,11 @@ class JdbcAggregateTemplateIntegrationTests { LegoSet reloadedLegoSet = template.findById(legoSet.getId(), LegoSet.class); - SoftAssertions softly = new SoftAssertions(); + assertSoftly(softly -> { - 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(); + softly.assertThat(reloadedLegoSet.manual.content).isEqualTo("other content"); + softly.assertThat(template.findAll(Manual.class)).describedAs("There should be only one manual").hasSize(1); + }); } @Test // DATAJDBC-112 @@ -524,14 +523,14 @@ class JdbcAggregateTemplateIntegrationTests { LegoSet reloadedLegoSet = template.findById(legoSet.getId(), LegoSet.class); - SoftAssertions softly = new SoftAssertions(); - softly.assertThat(reloadedLegoSet.alternativeInstructions).isNotNull(); - softly.assertThat(reloadedLegoSet.alternativeInstructions.id).isNotNull(); - softly.assertThat(reloadedLegoSet.alternativeInstructions.id).isNotEqualTo(reloadedLegoSet.manual.id); - softly.assertThat(reloadedLegoSet.alternativeInstructions.content) - .isEqualTo(reloadedLegoSet.alternativeInstructions.content); + assertSoftly(softly -> { - softly.assertAll(); + softly.assertThat(reloadedLegoSet.alternativeInstructions).isNotNull(); + softly.assertThat(reloadedLegoSet.alternativeInstructions.id).isNotNull(); + softly.assertThat(reloadedLegoSet.alternativeInstructions.id).isNotEqualTo(reloadedLegoSet.manual.id); + softly.assertThat(reloadedLegoSet.alternativeInstructions.content) + .isEqualTo(reloadedLegoSet.alternativeInstructions.content); + }); } @Test // DATAJDBC-276 @@ -559,7 +558,7 @@ class JdbcAggregateTemplateIntegrationTests { ElementNoId element = new ElementNoId(); element.content = "content"; - ListParentAllArgs entity = new ListParentAllArgs("name", asList(element)); + ListParentAllArgs entity = new ListParentAllArgs("name", singletonList(element)); entity = template.save(entity); @@ -1103,6 +1102,7 @@ class JdbcAggregateTemplateIntegrationTests { } + @SuppressWarnings("unused") static class OneToOneParent { @Column("id3") @Id private Long id; @@ -1116,6 +1116,7 @@ class JdbcAggregateTemplateIntegrationTests { } @Table("LIST_PARENT") + @SuppressWarnings("unused") static class ListParent { @Column("id4") @Id private Long id; @@ -1130,7 +1131,7 @@ class JdbcAggregateTemplateIntegrationTests { private final String name; @MappedCollection(idColumn = "LIST_PARENT") private final List content = new ArrayList<>(); - @PersistenceConstructor + @PersistenceCreator ListParentAllArgs(Long id, String name, List content) { this.id = id; @@ -1150,23 +1151,27 @@ class JdbcAggregateTemplateIntegrationTests { /** * One may think of ChainN as a chain with N further elements */ + @SuppressWarnings("unused") static class Chain0 { @Id Long zero; String zeroValue; } + @SuppressWarnings("unused") static class Chain1 { @Id Long one; String oneValue; Chain0 chain0; } + @SuppressWarnings("unused") static class Chain2 { @Id Long two; String twoValue; Chain1 chain1; } + @SuppressWarnings("unused") static class Chain3 { @Id Long three; String threeValue; @@ -1273,6 +1278,7 @@ class JdbcAggregateTemplateIntegrationTests { Map chain3 = new HashMap<>(); } + @SuppressWarnings("unused") static class WithReadOnly { @Id Long id; String name; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 37cbdfdb..86b97f6e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -50,6 +50,7 @@ import org.springframework.data.relational.core.sql.Aliased; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; +import org.springframework.lang.Nullable; /** * Unit tests for the {@link SqlGenerator}. @@ -65,16 +66,17 @@ import org.springframework.data.relational.core.sql.Table; * @author Mikhail Polivakha * @author Chirag Tailor */ +@SuppressWarnings("Convert2MethodRef") class SqlGeneratorUnitTests { private static final Identifier BACKREF = Identifier.of(unquoted("backref"), "some-value", String.class); - private SqlGenerator sqlGenerator; - private NamingStrategy namingStrategy = new PrefixingNamingStrategy(); - private RelationalMappingContext context = new JdbcMappingContext(namingStrategy); - private JdbcConverter converter = new BasicJdbcConverter(context, (identifier, path) -> { + private final NamingStrategy namingStrategy = new PrefixingNamingStrategy(); + private final RelationalMappingContext context = new JdbcMappingContext(namingStrategy); + private final JdbcConverter converter = new BasicJdbcConverter(context, (identifier, path) -> { throw new UnsupportedOperationException(); }); + private SqlGenerator sqlGenerator; @BeforeEach void setUp() { @@ -267,7 +269,8 @@ class SqlGeneratorUnitTests { SqlGenerator sqlGenerator = createSqlGenerator(DummyEntity.class, PostgresDialect.INSTANCE); - String sql = sqlGenerator.getFindAll(Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); + String sql = sqlGenerator + .getFindAll(Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); assertThat(sql).contains("ORDER BY \"dummy_entity\".\"x_name\" ASC NULLS LAST"); } @@ -277,7 +280,8 @@ class SqlGeneratorUnitTests { SqlGenerator sqlGenerator = createSqlGenerator(DummyEntity.class, SqlServerDialect.INSTANCE); - String sql = sqlGenerator.getFindAll(Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); + String sql = sqlGenerator + .getFindAll(Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); assertThat(sql).endsWith("ORDER BY dummy_entity.x_name ASC"); } @@ -695,6 +699,7 @@ class SqlGeneratorUnitTests { }); } + @Nullable private SqlGenerator.Join generateJoin(String path, Class type) { return createSqlGenerator(type, AnsiDialect.INSTANCE) .getJoin(new PersistentPropertyPathExtension(context, PropertyPathTestingUtils.toPath(path, type, context))); @@ -733,6 +738,7 @@ class SqlGeneratorUnitTests { SqlIdentifier.quoted("child"), SqlIdentifier.quoted("CHILD_PARENT_OF_NO_ID_CHILD")); } + @Nullable private SqlIdentifier getAlias(Object maybeAliased) { if (maybeAliased instanceof Aliased) { @@ -741,6 +747,7 @@ class SqlGeneratorUnitTests { return null; } + @Nullable private org.springframework.data.relational.core.sql.Column generatedColumn(String path, Class type) { return createSqlGenerator(type, AnsiDialect.INSTANCE) @@ -762,6 +769,7 @@ class SqlGeneratorUnitTests { AggregateReference other; } + @SuppressWarnings("unused") static class VersionedEntity extends DummyEntity { @Version Integer version; } @@ -781,6 +789,7 @@ class SqlGeneratorUnitTests { String something; } + @SuppressWarnings("unused") static class Element { @Id Long id; String content; @@ -795,6 +804,7 @@ class SqlGeneratorUnitTests { private static class NoIdChild {} + @SuppressWarnings("unused") static class OtherAggregate { @Id Long id; String name; @@ -823,6 +833,7 @@ class SqlGeneratorUnitTests { @ReadOnlyProperty String readOnlyValue; } + @SuppressWarnings("unused") static class EntityWithQuotedColumnName { // these column names behave like single double quote in the name since the get quoted and then doubling the double @@ -865,36 +876,43 @@ class SqlGeneratorUnitTests { Chain3 chain3; } + @SuppressWarnings("unused") static class NoIdChain0 { String zeroValue; } + @SuppressWarnings("unused") static class NoIdChain1 { String oneValue; NoIdChain0 chain0; } + @SuppressWarnings("unused") static class NoIdChain2 { String twoValue; NoIdChain1 chain1; } + @SuppressWarnings("unused") static class NoIdChain3 { String threeValue; NoIdChain2 chain2; } + @SuppressWarnings("unused") static class NoIdChain4 { @Id Long four; String fourValue; NoIdChain3 chain3; } + @SuppressWarnings("unused") static class IdNoIdChain { @Id Long id; NoIdChain4 chain4; } + @SuppressWarnings("unused") static class IdIdNoIdChain { @Id Long id; IdNoIdChain idNoIdChain; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java index f89e94bd..005477c8 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java @@ -21,6 +21,10 @@ import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.SoftAssertions.*; import static org.springframework.test.context.TestExecutionListeners.MergeMode.*; +import lombok.Data; +import lombok.NoArgsConstructor; +import lombok.Value; + import java.io.IOException; import java.sql.ResultSet; import java.time.Instant; @@ -48,8 +52,6 @@ import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.jdbc.core.mapping.AggregateReference; -import org.springframework.data.relational.core.mapping.MappedCollection; -import org.springframework.data.relational.repository.Lock; import org.springframework.data.jdbc.repository.query.Modifying; import org.springframework.data.jdbc.repository.query.Query; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; @@ -57,6 +59,7 @@ import org.springframework.data.jdbc.testing.AssumeFeatureTestExecutionListener; import org.springframework.data.jdbc.testing.EnabledOnFeature; import org.springframework.data.jdbc.testing.TestConfiguration; import org.springframework.data.jdbc.testing.TestDatabaseFeatures; +import org.springframework.data.relational.core.mapping.MappedCollection; import org.springframework.data.relational.core.mapping.event.AbstractRelationalEvent; import org.springframework.data.relational.core.mapping.event.AfterConvertEvent; import org.springframework.data.relational.core.sql.LockMode; @@ -75,10 +78,6 @@ import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.jdbc.JdbcTestUtils; import org.springframework.transaction.annotation.Transactional; -import lombok.Data; -import lombok.NoArgsConstructor; -import lombok.Value; - /** * Very simple use cases for creation and usage of JdbcRepositories. * @@ -607,8 +606,7 @@ public class JdbcRepositoryIntegrationTests { repository.saveAll(asList(dummyA, dummyB, dummyC)); assertThat(repository.findByEnumTypeIn(Set.of(Direction.LEFT, Direction.RIGHT))) - .extracting(DummyEntity::getDirection) - .containsExactlyInAnyOrder(Direction.LEFT, Direction.RIGHT); + .extracting(DummyEntity::getDirection).containsExactlyInAnyOrder(Direction.LEFT, Direction.RIGHT); } @Test // GH-1212 @@ -622,13 +620,13 @@ public class JdbcRepositoryIntegrationTests { dummyC.setDirection(Direction.RIGHT); repository.saveAll(asList(dummyA, dummyB, dummyC)); - assertThat(repository.findByEnumType(Direction.CENTER)) - .extracting(DummyEntity::getDirection) + assertThat(repository.findByEnumType(Direction.CENTER)).extracting(DummyEntity::getDirection) .containsExactlyInAnyOrder(Direction.CENTER); } @Test // GH-537 void manyInsertsWithNestedEntities() { + Root root1 = createRoot("root1"); Root root2 = createRoot("root2"); @@ -644,6 +642,7 @@ public class JdbcRepositoryIntegrationTests { @Test // GH-537 @EnabledOnFeature(TestDatabaseFeatures.Feature.SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES) void manyUpdatesWithNestedEntities() { + Root root1 = createRoot("root1"); Root root2 = createRoot("root2"); List roots = rootRepository.saveAll(asList(root1, root2)); @@ -669,6 +668,7 @@ public class JdbcRepositoryIntegrationTests { @Test // GH-537 @EnabledOnFeature(TestDatabaseFeatures.Feature.SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES) void manyInsertsAndUpdatesWithNestedEntities() { + Root root1 = createRoot("root1"); Root savedRoot1 = rootRepository.save(root1); Root updatedRoot1 = new Root(savedRoot1.id, "updated" + savedRoot1.name, @@ -685,6 +685,7 @@ public class JdbcRepositoryIntegrationTests { } private Root createRoot(String namePrefix) { + return new Root(null, namePrefix, new Intermediate(null, namePrefix + "Intermediate", new Leaf(null, namePrefix + "Leaf"), emptyList()), singletonList(new Intermediate(null, namePrefix + "QualifiedIntermediate", null, @@ -692,6 +693,7 @@ public class JdbcRepositoryIntegrationTests { } private void assertIsEqualToWithNonNullIds(Root reloadedRoot1, Root root1) { + assertThat(reloadedRoot1.id).isNotNull(); assertThat(reloadedRoot1.name).isEqualTo(root1.name); assertThat(reloadedRoot1.intermediate.id).isNotNull(); @@ -840,6 +842,7 @@ public class JdbcRepositoryIntegrationTests { @Value static class Root { + @Id Long id; String name; Intermediate intermediate; @@ -848,6 +851,7 @@ public class JdbcRepositoryIntegrationTests { @Value static class Intermediate { + @Id Long id; String name; Leaf leaf; @@ -856,13 +860,14 @@ public class JdbcRepositoryIntegrationTests { @Value static class Leaf { + @Id Long id; String name; } static class MyEventListener implements ApplicationListener> { - private List> events = new ArrayList<>(); + private final List> events = new ArrayList<>(); @Override public void onApplicationEvent(AbstractRelationalEvent event) { @@ -892,13 +897,11 @@ public class JdbcRepositoryIntegrationTests { } interface DummyProjection { - String getName(); } @Value static class DtoProjection { - String name; } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java index 418f1b57..9939313f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java @@ -17,6 +17,7 @@ package org.springframework.data.jdbc.repository; import static java.util.Arrays.*; import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.groups.Tuple.tuple; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -28,7 +29,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import org.assertj.core.groups.Tuple; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.stubbing.Answer; @@ -45,8 +45,15 @@ import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.dialect.H2Dialect; import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.mapping.RelationalMappingContext; -import org.springframework.data.relational.core.mapping.event.*; +import org.springframework.data.relational.core.mapping.event.AfterConvertEvent; +import org.springframework.data.relational.core.mapping.event.AfterDeleteEvent; +import org.springframework.data.relational.core.mapping.event.AfterSaveEvent; +import org.springframework.data.relational.core.mapping.event.BeforeConvertEvent; +import org.springframework.data.relational.core.mapping.event.BeforeDeleteEvent; +import org.springframework.data.relational.core.mapping.event.BeforeSaveEvent; import org.springframework.data.relational.core.mapping.event.Identifier; +import org.springframework.data.relational.core.mapping.event.RelationalEvent; +import org.springframework.data.relational.core.mapping.event.WithId; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.jdbc.core.JdbcOperations; @@ -129,12 +136,12 @@ public class SimpleJdbcRepositoryEventsUnitTests { assertThat(publisher.events) // .extracting(RelationalEvent::getClass, e -> ((DummyEntity) e.getEntity()).getId()) // .containsExactly( // - Tuple.tuple(BeforeConvertEvent.class, null), // - Tuple.tuple(BeforeSaveEvent.class, null), // - Tuple.tuple(BeforeConvertEvent.class, 23L), // - Tuple.tuple(BeforeSaveEvent.class, 23L), // - Tuple.tuple(AfterSaveEvent.class, generatedId), // - Tuple.tuple(AfterSaveEvent.class, 23L) // + tuple(BeforeConvertEvent.class, null), // + tuple(BeforeSaveEvent.class, null), // + tuple(BeforeConvertEvent.class, 23L), // + tuple(BeforeSaveEvent.class, 23L), // + tuple(AfterSaveEvent.class, generatedId), // + tuple(AfterSaveEvent.class, 23L) // ); } @@ -150,8 +157,8 @@ public class SimpleJdbcRepositoryEventsUnitTests { this::getEntity, // this::getId // ).containsExactly( // - Tuple.tuple(BeforeDeleteEvent.class, entity, Identifier.of(23L)), // - Tuple.tuple(AfterDeleteEvent.class, entity, Identifier.of(23L)) // + tuple(BeforeDeleteEvent.class, entity, Identifier.of(23L)), // + tuple(AfterDeleteEvent.class, entity, Identifier.of(23L)) // ); } diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-db2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-db2.sql index ae2ebe02..4916d64b 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-db2.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-db2.sql @@ -19,6 +19,7 @@ CREATE TABLE ROOT ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, @@ -27,6 +28,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID BIGINT, ROOT_KEY INTEGER ); + CREATE TABLE LEAF ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-h2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-h2.sql index 0358d5db..c9eedd6b 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-h2.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-h2.sql @@ -14,6 +14,7 @@ CREATE TABLE ROOT ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, @@ -22,6 +23,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID BIGINT, ROOT_KEY INTEGER ); + CREATE TABLE LEAF ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-hsql.sql index 0358d5db..c9eedd6b 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-hsql.sql @@ -14,6 +14,7 @@ CREATE TABLE ROOT ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, @@ -22,6 +23,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID BIGINT, ROOT_KEY INTEGER ); + CREATE TABLE LEAF ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql index a652b3ba..5a4a83d6 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql @@ -14,6 +14,7 @@ CREATE TABLE ROOT ID BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, @@ -22,6 +23,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID BIGINT, ROOT_KEY INTEGER ); + CREATE TABLE LEAF ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mssql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mssql.sql index e0a307e7..5f2069c6 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mssql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mssql.sql @@ -19,6 +19,7 @@ CREATE TABLE ROOT ID BIGINT IDENTITY PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID BIGINT IDENTITY PRIMARY KEY, @@ -27,6 +28,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID BIGINT, ROOT_KEY INTEGER ); + CREATE TABLE LEAF ( ID BIGINT IDENTITY PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mysql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mysql.sql index fdae0af0..09995864 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mysql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mysql.sql @@ -17,6 +17,7 @@ CREATE TABLE ROOT ID BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, @@ -25,6 +26,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID BIGINT, ROOT_KEY INTEGER ); + CREATE TABLE LEAF ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql index 3cf241c2..518e667c 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql @@ -19,6 +19,7 @@ CREATE TABLE ROOT ID NUMBER GENERATED BY DEFAULT ON NULL AS IDENTITY PRIMARY KEY, NAME VARCHAR2(100) ); + CREATE TABLE INTERMEDIATE ( ID NUMBER GENERATED BY DEFAULT ON NULL AS IDENTITY PRIMARY KEY, @@ -27,6 +28,7 @@ CREATE TABLE INTERMEDIATE ROOT_ID NUMBER, ROOT_KEY NUMBER ); + CREATE TABLE LEAF ( ID NUMBER GENERATED BY DEFAULT ON NULL AS IDENTITY PRIMARY KEY, diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql index 4fd42572..8bcd1735 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql @@ -19,6 +19,7 @@ CREATE TABLE ROOT ID SERIAL PRIMARY KEY, NAME VARCHAR(100) ); + CREATE TABLE INTERMEDIATE ( ID SERIAL PRIMARY KEY, @@ -27,6 +28,7 @@ CREATE TABLE INTERMEDIATE "ROOT_ID" BIGINT, "ROOT_KEY" INTEGER ); + CREATE TABLE LEAF ( ID SERIAL PRIMARY KEY, diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BatchedActions.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BatchedActions.java new file mode 100644 index 00000000..359b039c --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BatchedActions.java @@ -0,0 +1,173 @@ +/* + * Copyright 2022 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 + * + * https://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 java.util.Collections.*; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; +import java.util.stream.Stream; + +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; + +/** + * Collects actions of a certain type and allows to consume them in a batched fashion, i.e. "similar" actions get + * combined into a batched action variant. + * + * @param type of the singular action. + * @param type of the batched action. + * @param type of the container used for gathering singular actions. + * @author Jens Schauder + * @since 3.0 + */ +class BatchedActions { + + private static final Comparator> PATH_LENGTH_COMPARATOR = // + Comparator.comparing(PersistentPropertyPath::getLength); + private static final Comparator> REVERSE_PATH_LENGTH_COMPARATOR = // + PATH_LENGTH_COMPARATOR.reversed(); + + private final Map, C> actionMap = new HashMap<>(); + + private final Combiner combiner; + private final Comparator> sorting; + + static BatchedActions> batchedDeletes() { + return new BatchedActions<>(DeleteCombiner.INSTANCE, REVERSE_PATH_LENGTH_COMPARATOR); + } + + static BatchedActions>> batchedInserts() { + return new BatchedActions<>(InsertCombiner.INSTANCE, PATH_LENGTH_COMPARATOR); + } + + private BatchedActions(Combiner combiner, + Comparator> sorting) { + + this.combiner = combiner; + this.sorting = sorting; + } + + /** + * Adds an action that might get combined with other actions into a batch. + * + * @param action the action to combine with other actions. + */ + void add(S action) { + combiner.merge(actionMap, action.getPropertyPath(), action); + } + + void forEach(Consumer consumer) { + + combiner.forEach( // + actionMap.entrySet().stream() // + .sorted(Map.Entry.comparingByKey(sorting)), // + consumer); + } + + interface Combiner { + + /** + * Merges an additional entry into the map of actions, which groups the actions by property path. + * + * @param actionMap the map of actions into which the new action is to be merged. + * @param propertyPath the property map under which to add the action. + * @param action the action to be merged. + */ + void merge(Map, C> actionMap, + PersistentPropertyPath propertyPath, S action); + + /** + * Invokes the consumer for the actions in the sorted stream. Before passed to the consumer compatible actions will + * get combined into batched actions. + * + * @param sorted + * @param consumer + */ + void forEach(Stream, C>> sorted, + Consumer consumer); + } + + enum DeleteCombiner implements Combiner, DbAction.BatchDelete> { + INSTANCE; + + @Override + public void merge(Map, List> actionMap, + PersistentPropertyPath propertyPath, DbAction.Delete action) { + + actionMap.merge( // + propertyPath, // + new ArrayList<>(singletonList(action)), // + (actions, defaultValue) -> { + actions.add(action); + return actions; + }); + } + + @Override + public void forEach( + Stream, List>> sorted, + Consumer consumer) { + + sorted.forEach((entry) -> { + + List actions = entry.getValue(); + if (actions.size() > 1) { + singletonList(new DbAction.BatchDelete(actions)).forEach(consumer); + } else { + actions.forEach(consumer); + } + }); + } + + } + + enum InsertCombiner + implements Combiner>, DbAction.BatchInsert> { + INSTANCE; + + @Override + public void merge( + Map, Map>> actionMap, + PersistentPropertyPath propertyPath, DbAction.Insert action) { + + actionMap.merge( // + propertyPath, // + new HashMap<>(singletonMap(action.getIdValueSource(), new ArrayList<>(singletonList(action)))), // + (map, mapDefaultValue) -> { + map.merge(action.getIdValueSource(), new ArrayList<>(singletonList(action)), + (actions, listDefaultValue) -> { + actions.add(action); + return actions; + }); + return map; + }); + } + + @Override + public void forEach( + Stream, Map>>> sorted, + Consumer consumer) { + + sorted.forEach((entry) -> entry.getValue() // + .forEach((idValueSource, inserts) -> consumer.accept(new DbAction.BatchInsert(inserts)))); + } + } +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index b65ad2d1..15b72f3c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -15,8 +15,6 @@ */ package org.springframework.data.relational.core.conversion; -import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -64,7 +62,7 @@ public interface DbAction { this.entity = entity; this.propertyPath = propertyPath; this.dependingOn = dependingOn; - this.qualifiers = Collections.unmodifiableMap(new HashMap<>(qualifiers)); + this.qualifiers = Map.copyOf(qualifiers); this.idValueSource = idValueSource; } @@ -358,9 +356,9 @@ public interface DbAction { private final B batchValue; /** - * Creates a {@link BatchWithValue} instance from the given actions and the value which can be extracted by applying #batchValueExtractor on any of the actions. - * - * All actions must result in the same value when #batchValueExtractor is applied. + * Creates a {@link BatchWithValue} instance from the given actions and the value which can be extracted by applying + * #batchValueExtractor on any of the actions. All actions must result in the same value when #batchValueExtractor + * is applied. * * @param actions the actions forming the batch. * @param batchValueExtractor function for extracting the {@link #batchValue} from an action. @@ -371,10 +369,8 @@ public interface DbAction { Iterator actionIterator = actions.iterator(); this.batchValue = batchValueExtractor.apply(actionIterator.next()); - actionIterator.forEachRemaining(action -> { - Assert.isTrue(batchValueExtractor.apply(action).equals(batchValue), - "All actions in the batch must have matching batchValue"); - }); + actionIterator.forEachRemaining(action -> Assert.isTrue(batchValueExtractor.apply(action).equals(batchValue), + "All actions in the batch must have matching batchValue")); this.actions = actions; } @@ -423,12 +419,14 @@ public interface DbAction { } /** - * Represents a batch delete statement for multiple entities that are reachable via a given path from the aggregate root. + * Represents a batch delete statement for multiple entities that are reachable via a given path from the aggregate + * root. * * @param type of the entity for which this represents a database interaction. * @since 3.0 */ - final class BatchDelete extends BatchWithValue, PersistentPropertyPath> { + final class BatchDelete + extends BatchWithValue, PersistentPropertyPath> { public BatchDelete(List> actions) { super(actions, Delete::getPropertyPath); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultRootAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultRootAggregateChange.java index b42a4ff7..93551300 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultRootAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultRootAggregateChange.java @@ -42,7 +42,7 @@ class DefaultRootAggregateChange implements RootAggregateChange { /** The previous version assigned to the instance being changed, if available */ @Nullable private final Number previousVersion; - public DefaultRootAggregateChange(Kind kind, Class entityType, @Nullable Number previousVersion) { + DefaultRootAggregateChange(Kind kind, Class entityType, @Nullable Number previousVersion) { this.kind = kind; this.entityType = entityType; diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteAggregateChange.java index ff7c3da3..0dd4832d 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteAggregateChange.java @@ -40,7 +40,7 @@ public class DeleteAggregateChange implements MutableAggregateChange { /** The previous version assigned to the instance being changed, if available */ @Nullable private final Number previousVersion; - public DeleteAggregateChange(Class entityType, @Nullable Number previousVersion) { + DeleteAggregateChange(Class entityType, @Nullable Number previousVersion) { this.entityType = entityType; this.previousVersion = previousVersion; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java index 49e0ea51..a3104e41 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java @@ -1,16 +1,12 @@ package org.springframework.data.relational.core.conversion; -import org.springframework.data.mapping.PersistentPropertyPath; -import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; - import java.util.ArrayList; import java.util.Comparator; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.function.Consumer; -import static java.util.Collections.*; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; /** * A {@link BatchingAggregateChange} implementation for delete changes that can contain actions for one or more delete @@ -29,10 +25,9 @@ public class DeleteBatchingAggregateChange implements BatchingAggregateChange private final Class entityType; private final List> rootActions = new ArrayList<>(); private final List> lockActions = new ArrayList<>(); - private final Map, List>> deleteActions = // - new HashMap<>(); + private final BatchedActions deleteActions = BatchedActions.batchedDeletes(); - public DeleteBatchingAggregateChange(Class entityType) { + DeleteBatchingAggregateChange(Class entityType) { this.entityType = entityType; } @@ -50,15 +45,7 @@ public class DeleteBatchingAggregateChange implements BatchingAggregateChange public void forEachAction(Consumer> consumer) { lockActions.forEach(consumer); - deleteActions.entrySet().stream().sorted(Map.Entry.comparingByKey(pathLengthComparator.reversed())) - .forEach((entry) -> { - List> deletes = entry.getValue(); - if (deletes.size() > 1) { - consumer.accept(new DbAction.BatchDelete<>(deletes)); - } else { - deletes.forEach(consumer); - } - }); + deleteActions.forEach(consumer); rootActions.forEach(consumer); } @@ -67,23 +54,12 @@ public class DeleteBatchingAggregateChange implements BatchingAggregateChange aggregateChange.forEachAction(action -> { if (action instanceof DbAction.DeleteRoot deleteRootAction) { - //noinspection unchecked rootActions.add((DbAction.DeleteRoot) deleteRootAction); } else if (action instanceof DbAction.Delete deleteAction) { - // noinspection unchecked - addDelete((DbAction.Delete) deleteAction); + deleteActions.add(deleteAction); } else if (action instanceof DbAction.AcquireLockRoot lockRootAction) { lockActions.add(lockRootAction); } }); } - - private void addDelete(DbAction.Delete action) { - - PersistentPropertyPath propertyPath = action.getPropertyPath(); - deleteActions.merge(propertyPath, new ArrayList<>(singletonList(action)), (actions, defaultValue) -> { - actions.add(action); - return actions; - }); - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java index 64713437..0ee4bc9a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java @@ -15,13 +15,9 @@ */ package org.springframework.data.relational.core.conversion; -import static java.util.Collections.*; - import java.util.ArrayList; import java.util.Comparator; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.function.Consumer; import org.springframework.data.mapping.PersistentPropertyPath; @@ -49,9 +45,8 @@ public class SaveBatchingAggregateChange implements BatchingAggregateChange> insertRootBatchCandidates = new ArrayList<>(); - private final Map, Map>>> insertActions = // - new HashMap<>(); - private final Map, List>> deleteActions = new HashMap<>(); + private final BatchedActions insertActions = BatchedActions.batchedInserts(); + private final BatchedActions deleteActions = BatchedActions.batchedDeletes(); SaveBatchingAggregateChange(Class entityType) { this.entityType = entityType; @@ -70,7 +65,7 @@ public class SaveBatchingAggregateChange implements BatchingAggregateChange> consumer) { - Assert.notNull(consumer, "Consumer must not be null."); + Assert.notNull(consumer, "Consumer must not be null"); rootActions.forEach(consumer); if (insertRootBatchCandidates.size() > 1) { @@ -78,17 +73,8 @@ public class SaveBatchingAggregateChange implements BatchingAggregateChange { - List> deletes = entry.getValue(); - if (deletes.size() > 1) { - consumer.accept(new DbAction.BatchDelete<>(deletes)); - } else { - deletes.forEach(consumer); - } - }); - insertActions.entrySet().stream().sorted(Map.Entry.comparingByKey(pathLengthComparator)).forEach((entry) -> entry - .getValue().forEach((idValueSource, inserts) -> consumer.accept(new DbAction.BatchInsert<>(inserts)))); + deleteActions.forEach(consumer); + insertActions.forEach(consumer); } @Override @@ -109,12 +95,9 @@ public class SaveBatchingAggregateChange implements BatchingAggregateChange) rootAction); } else if (action instanceof DbAction.Insert insertAction) { - - // noinspection unchecked - addInsert((DbAction.Insert) insertAction); + insertActions.add(insertAction); } else if (action instanceof DbAction.Delete deleteAction) { - // noinspection unchecked - addDelete((DbAction.Delete) deleteAction); + deleteActions.add(deleteAction); } }); } @@ -133,26 +116,4 @@ public class SaveBatchingAggregateChange implements BatchingAggregateChange action) { - - PersistentPropertyPath propertyPath = action.getPropertyPath(); - insertActions.merge(propertyPath, - new HashMap<>(singletonMap(action.getIdValueSource(), new ArrayList<>(singletonList(action)))), - (map, mapDefaultValue) -> { - map.merge(action.getIdValueSource(), new ArrayList<>(singletonList(action)), (actions, listDefaultValue) -> { - actions.add(action); - return actions; - }); - return map; - }); - } - - private void addDelete(DbAction.Delete action) { - - PersistentPropertyPath propertyPath = action.getPropertyPath(); - deleteActions.merge(propertyPath, new ArrayList<>(singletonList(action)), (actions, defaultValue) -> { - actions.add(action); - return actions; - }); - } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/BatchedActionsUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/BatchedActionsUnitTests.java new file mode 100644 index 00000000..b5896980 --- /dev/null +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/BatchedActionsUnitTests.java @@ -0,0 +1,105 @@ +/* + * Copyright 2022 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 + * + * https://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.List; +import java.util.function.Consumer; + +import org.junit.jupiter.api.Test; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.conversion.DbAction.BatchDelete; +import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; + +public class BatchedActionsUnitTests { + + BatchedActions deletes = BatchedActions.batchedDeletes(); + + LoggingConsumer consumer = new LoggingConsumer(); + RelationalMappingContext context = new RelationalMappingContext(); + + DbAction.Delete firstOneDelete = new DbAction.Delete(23L, path("one")); + DbAction.Delete secondOneDelete = new DbAction.Delete(24L, path("one")); + DbAction.Delete firstTwoDelete = new DbAction.Delete(25L, path("two")); + DbAction.Delete secondTwoDelete = new DbAction.Delete(26L, path("two")); + + @Test // GH-537 + void emptyBatchedDeletesDoesNotInvokeConsumer() { + + deletes.forEach(consumer); + + assertThat(consumer.log).isEmpty(); + } + + @Test // GH-537 + void singleActionGetsPassedToConsumer() { + + deletes.add(firstOneDelete); + + deletes.forEach(consumer); + + assertThat(consumer.log).containsExactly(firstOneDelete); + } + + @Test // GH-537 + void multipleUnbatchableActionsGetsPassedToConsumerIndividually() { + + deletes.add(firstOneDelete); + deletes.add(firstTwoDelete); + + deletes.forEach(consumer); + + assertThat(consumer.log).containsExactlyInAnyOrder(firstOneDelete, firstTwoDelete); + } + + @Test // GH-537 + void batchableActionsGetPassedToConsumerAsOne() { + + deletes.add(firstOneDelete); + deletes.add(secondOneDelete); + + deletes.forEach(consumer); + + assertThat(consumer.log).extracting(a -> ((Class)a.getClass())).containsExactly(BatchDelete.class); + } + + private PersistentPropertyPath path(String path) { + return context.getPersistentPropertyPath(path, DummyEntity.class); + } + + private static class LoggingConsumer implements Consumer> { + List> log = new ArrayList<>(); + + @Override + public void accept(DbAction dbAction) { + log.add(dbAction); + } + } + + private static class DummyEntity { + OtherEntity one; + OtherEntity two; + } + + private static class OtherEntity { + String one; + String two; + } + +} diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DbActionTestSupport.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DbActionTestSupport.java index 5f0e8280..30cfcd71 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DbActionTestSupport.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DbActionTestSupport.java @@ -28,7 +28,7 @@ import org.springframework.lang.Nullable; @UtilityClass class DbActionTestSupport { - static String extractPath(DbAction action) { + static String extractPath(DbAction action) { if (action instanceof DbAction.WithPropertyPath) { return ((DbAction.WithPropertyPath) action).getPropertyPath().toDotPath(); @@ -37,14 +37,15 @@ class DbActionTestSupport { return ""; } - static boolean isWithDependsOn(DbAction dbAction) { + static boolean isWithDependsOn(DbAction dbAction) { return dbAction instanceof DbAction.WithDependingOn; } - static Class actualEntityType(DbAction a) { + @Nullable + static Class actualEntityType(DbAction a) { if (a instanceof DbAction.WithEntity) { - return ((DbAction.WithEntity) a).getEntity().getClass(); + return ((DbAction.WithEntity) a).getEntity().getClass(); } return null; } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java index c91edd7b..cdacbe76 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java @@ -22,7 +22,7 @@ class DeleteBatchingAggregateChangeTest { RelationalMappingContext context = new RelationalMappingContext(); - @Test + @Test // GH-537 void yieldsDeleteActions() { Root root = new Root(1L, null); @@ -37,7 +37,7 @@ class DeleteBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(intermediateDelete); } - @Test + @Test // GH-537 void yieldsNestedDeleteActionsInTreeOrderFromLeavesToRoot() { Root root = new Root(2L, null); @@ -45,6 +45,7 @@ class DeleteBatchingAggregateChangeTest { DbAction.Delete intermediateDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate", Root.class)); aggregateChange.addAction(intermediateDelete); + DbAction.Delete leafDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate.leaf", Root.class)); aggregateChange.addAction(leafDelete); @@ -56,16 +57,18 @@ class DeleteBatchingAggregateChangeTest { assertThat(actions).containsExactly(leafDelete, intermediateDelete); } - @Test + @Test // GH-537 void yieldsDeleteActionsAsBatchDeletes_groupedByPath_whenGroupContainsMultipleDeletes() { Root root = new Root(1L, null); DeleteAggregateChange aggregateChange = MutableAggregateChange.forDelete(root); + DbAction.Delete intermediateDelete1 = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate", Root.class)); + aggregateChange.addAction(intermediateDelete1); + DbAction.Delete intermediateDelete2 = new DbAction.Delete<>(2L, context.getPersistentPropertyPath("intermediate", Root.class)); - aggregateChange.addAction(intermediateDelete1); aggregateChange.addAction(intermediateDelete2); BatchingAggregateChange> change = BatchingAggregateChange.forDelete(Root.class); @@ -78,7 +81,7 @@ class DeleteBatchingAggregateChangeTest { .containsExactly(intermediateDelete1, intermediateDelete2); } - @Test + @Test // GH-537 void yieldsDeleteRootActions() { DeleteAggregateChange aggregateChange = MutableAggregateChange.forDelete(new Root(null, null)); @@ -91,12 +94,14 @@ class DeleteBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(deleteRoot); } - @Test + @Test // GH-537 void yieldsDeleteRootActionsAfterDeleteActions() { DeleteAggregateChange aggregateChange = MutableAggregateChange.forDelete(new Root(null, null)); + DbAction.DeleteRoot deleteRoot = new DbAction.DeleteRoot<>(1L, Root.class, null); aggregateChange.addAction(deleteRoot); + DbAction.Delete intermediateDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate", Root.class)); aggregateChange.addAction(intermediateDelete); @@ -107,10 +112,11 @@ class DeleteBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(intermediateDelete, deleteRoot); } - @Test + @Test // GH-537 void yieldsLockRootActions() { DeleteAggregateChange aggregateChange = MutableAggregateChange.forDelete(new Root(null, null)); + DbAction.AcquireLockRoot lockRootAction = new DbAction.AcquireLockRoot<>(1L, Root.class); aggregateChange.addAction(lockRootAction); @@ -120,13 +126,15 @@ class DeleteBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(lockRootAction); } - @Test + @Test // GH-537 void yieldsLockRootActionsBeforeDeleteActions() { DeleteAggregateChange aggregateChange = MutableAggregateChange.forDelete(new Root(null, null)); + DbAction.Delete intermediateDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate", Root.class)); aggregateChange.addAction(intermediateDelete); + DbAction.AcquireLockRoot lockRootAction = new DbAction.AcquireLockRoot<>(1L, Root.class); aggregateChange.addAction(lockRootAction); @@ -150,14 +158,6 @@ class DeleteBatchingAggregateChangeTest { .orElseThrow(() -> new RuntimeException("No BatchWithValue action found!")); } - private DbAction.BatchWithValue, Object> getBatchWithValueAction(List> actions, - Class entityType, Class batchActionType, Object batchValue) { - - return getBatchWithValueActions(actions, entityType, batchActionType).stream() - .filter(batchWithValue -> batchWithValue.getBatchValue() == batchValue).findFirst().orElseThrow( - () -> new RuntimeException(String.format("No BatchWithValue with batch value '%s' found!", batchValue))); - } - @SuppressWarnings("unchecked") private List, Object>> getBatchWithValueActions( List> actions, Class entityType, Class batchActionType) { @@ -170,12 +170,14 @@ class DeleteBatchingAggregateChangeTest { @Value static class Root { + @Id Long id; Intermediate intermediate; } @Value static class Intermediate { + @Id Long id; String name; Leaf leaf; @@ -183,6 +185,7 @@ class DeleteBatchingAggregateChangeTest { @Value static class Leaf { + @Id Long id; String name; } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java index 265b2a4a..63debaef 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java @@ -98,7 +98,7 @@ public class RelationalEntityWriterUnitTests { ); } - @Test + @Test // GH-1159 void newEntityWithPrimitiveLongId_insertDoesNotIncludeId_whenIdValueIsZero() { PrimitiveLongIdEntity entity = new PrimitiveLongIdEntity(); @@ -119,7 +119,7 @@ public class RelationalEntityWriterUnitTests { ); } - @Test + @Test // GH-1159 void newEntityWithPrimitiveIntId_insertDoesNotIncludeId_whenIdValueIsZero() { PrimitiveIntIdEntity entity = new PrimitiveIntIdEntity(); @@ -187,7 +187,7 @@ public class RelationalEntityWriterUnitTests { ); } - @Test + @Test // GH-1159 void newEntityWithReference_whenReferenceHasPrimitiveId_insertDoesNotIncludeId_whenIdValueIsZero() { EntityWithReferencesToPrimitiveIdEntity entity = new EntityWithReferencesToPrimitiveIdEntity(null); @@ -665,7 +665,7 @@ public class RelationalEntityWriterUnitTests { ); } - @Test + @Test // GH-1159 void newEntityWithCollection_whenElementHasPrimitiveId_doesNotIncludeId_whenIdValueIsZero() { EntityWithReferencesToPrimitiveIdEntity entity = new EntityWithReferencesToPrimitiveIdEntity(null); diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java index 22d81b76..f829df5c 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java @@ -39,7 +39,7 @@ class SaveBatchingAggregateChangeTest { RelationalMappingContext context = new RelationalMappingContext(); - @Test + @Test // GH-537 void startsWithNoActions() { BatchingAggregateChange> change = BatchingAggregateChange.forSave(Root.class); @@ -49,7 +49,7 @@ class SaveBatchingAggregateChangeTest { @Nested class RootActionsTests { - @Test + @Test // GH-537 void yieldsUpdateRoot() { Root root = new Root(1L, null); @@ -63,7 +63,7 @@ class SaveBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(rootUpdate); } - @Test + @Test // GH-537 void yieldsSingleInsertRoot_followedByUpdateRoot_asIndividualActions() { Root root1 = new Root(1L, null); @@ -87,7 +87,7 @@ class SaveBatchingAggregateChangeTest { Tuple.tuple(DbAction.UpdateRoot.class, Root.class, IdValueSource.PROVIDED)); } - @Test + @Test // GH-537 void yieldsMultipleMatchingInsertRoot_followedByUpdateRoot_asBatchInsertRootAction() { Root root1 = new Root(1L, null); @@ -120,7 +120,7 @@ class SaveBatchingAggregateChangeTest { .containsExactly(root1Insert, root2Insert); } - @Test + @Test // GH-537 void yieldsInsertRoot() { Root root = new Root(1L, null); @@ -134,7 +134,7 @@ class SaveBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(rootInsert); } - @Test + @Test // GH-537 void yieldsSingleInsertRoot_followedByNonMatchingInsertRoot_asIndividualActions() { Root root1 = new Root(1L, null); @@ -154,7 +154,7 @@ class SaveBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(root1Insert, root2Insert); } - @Test + @Test // GH-537 void yieldsMultipleMatchingInsertRoot_followedByNonMatchingInsertRoot_asBatchInsertRootAction() { Root root1 = new Root(1L, null); @@ -187,7 +187,7 @@ class SaveBatchingAggregateChangeTest { .containsExactly(root1Insert, root2Insert); } - @Test + @Test // GH-537 void yieldsMultipleMatchingInsertRoot_asBatchInsertRootAction() { Root root1 = new Root(1L, null); @@ -212,7 +212,7 @@ class SaveBatchingAggregateChangeTest { .containsExactly(root1Insert, root2Insert); } - @Test + @Test // GH-537 void yieldsPreviouslyYieldedInsertRoot_asBatchInsertRootAction_whenAdditionalMatchingInsertRootIsAdded() { Root root1 = new Root(1L, null); @@ -244,7 +244,7 @@ class SaveBatchingAggregateChangeTest { } } - @Test + @Test // GH-537 void yieldsRootActionsBeforeDeleteActions() { Root root1 = new Root(null, null); @@ -271,7 +271,7 @@ class SaveBatchingAggregateChangeTest { Tuple.tuple(DbAction.Delete.class, Intermediate.class)); } - @Test + @Test // GH-537 void yieldsNestedDeleteActionsInTreeOrderFromLeavesToRoot() { Root root1 = new Root(1L, null); @@ -305,7 +305,7 @@ class SaveBatchingAggregateChangeTest { .containsExactly(root1IntermediateDelete, root2IntermediateDelete); } - @Test + @Test // GH-537 void yieldsDeleteActionsAsBatchDeletes_groupedByPath_whenGroupContainsMultipleDeletes() { Root root = new Root(1L, null); @@ -331,7 +331,7 @@ class SaveBatchingAggregateChangeTest { .containsExactly(intermediateDelete1, intermediateDelete2); } - @Test + @Test // GH-537 void yieldsDeleteActionsBeforeInsertActions() { Root root1 = new Root(null, null); @@ -361,7 +361,7 @@ class SaveBatchingAggregateChangeTest { Tuple.tuple(DbAction.BatchInsert.class, Intermediate.class)); } - @Test + @Test // GH-537 void yieldsInsertActionsAsBatchInserts_groupedByIdValueSource() { Root root = new Root(null, null); @@ -398,7 +398,7 @@ class SaveBatchingAggregateChangeTest { .getActions()).containsExactly(intermediateInsertProvidedId); } - @Test + @Test // GH-537 void yieldsNestedInsertActionsInTreeOrderFromRootToLeaves() { Root root1 = new Root(null, null); @@ -445,7 +445,7 @@ class SaveBatchingAggregateChangeTest { .containsExactly(root1LeafInsert); } - @Test + @Test // GH-537 void yieldsInsertsWithSameLengthReferences_asSeparateInserts() { RootWithSameLengthReferences root = new RootWithSameLengthReferences(null, null, null);