From c33addd934c873678b2f12a7e957ddecd365d4d7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 23 Aug 2022 15:21:45 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20deleteAllById(=E2=80=A6)=20using=20compos?= =?UTF-8?q?ite=20keys.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now correctly delete all elements when using composite keys (MapId, Primary Key classes). Previously, we did not support that case or removed only the first element. Closes #1298 --- .../repository/support/FindByIdQuery.java | 14 +++- .../support/SimpleCassandraRepository.java | 6 +- .../SimpleReactiveCassandraRepository.java | 6 +- .../data/cassandra/domain/CompositeKey.java | 4 ++ ...epositoryCompositeIdIntegrationTests.java} | 58 +++++++++++++-- ...epositoryCompositeIdIntegrationTests.java} | 70 +++++++++++++++++-- 6 files changed, 139 insertions(+), 19 deletions(-) rename spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/{SimpleCassandraRepositoryMapIdIntegrationTests.java => SimpleCassandraRepositoryCompositeIdIntegrationTests.java} (57%) rename spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/{SimpleReactiveCassandraRepositoryMapIdIntegrationTests.java => SimpleReactiveCassandraRepositoryCompositeIdIntegrationTests.java} (58%) diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/FindByIdQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/FindByIdQuery.java index 22b6bac33..6435500be 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/FindByIdQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/FindByIdQuery.java @@ -20,7 +20,10 @@ import java.util.Iterator; import java.util.List; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.cassandra.core.mapping.BasicCassandraPersistentEntity; +import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty; import org.springframework.data.cassandra.core.mapping.MapId; +import org.springframework.data.mapping.context.MappingContext; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -85,9 +88,11 @@ class FindByIdQuery { * Check if the {@link Iterable} of {@code ID}s contains composite keys. * * @param ids + * @param mappingContext * @return */ - static boolean hasCompositeKeys(Iterable ids) { + static boolean hasCompositeKeys(Iterable ids, + MappingContext, CassandraPersistentProperty> mappingContext) { Assert.notNull(ids, "The given Iterable of ids must not be null"); @@ -100,6 +105,13 @@ class FindByIdQuery { if (mapId.size() > 1) { return true; } + } else { + + BasicCassandraPersistentEntity persistentEntity = mappingContext.getPersistentEntity(id.getClass()); + + if (persistentEntity != null && persistentEntity.isCompositePrimaryKey()) { + return true; + } } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepository.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepository.java index f9d75cc1c..11b8990d6 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepository.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepository.java @@ -200,7 +200,11 @@ public class SimpleCassandraRepository implements CassandraRepository implements ReactiveCassand Assert.notNull(ids, "The given Iterable of ids must not be null"); - if (FindByIdQuery.hasCompositeKeys(ids)) { + if (FindByIdQuery.hasCompositeKeys(ids, this.mappingContext)) { return findAllById(Flux.fromIterable(ids)); } @@ -257,8 +257,8 @@ public class SimpleReactiveCassandraRepository implements ReactiveCassand Assert.notNull(ids, "The given Iterable of ids must not be null"); - if (FindByIdQuery.hasCompositeKeys(ids)) { - return deleteById(Flux.fromIterable(ids)); + if (FindByIdQuery.hasCompositeKeys(ids, this.mappingContext)) { + return Flux.fromIterable(ids).flatMap(this::deleteById).then(); } if (!ids.iterator().hasNext()) { diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/CompositeKey.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/CompositeKey.java index 9b707cc30..6f2817913 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/CompositeKey.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/CompositeKey.java @@ -15,7 +15,9 @@ */ package org.springframework.data.cassandra.domain; +import lombok.AllArgsConstructor; import lombok.Data; +import lombok.NoArgsConstructor; import java.io.Serializable; @@ -28,6 +30,8 @@ import org.springframework.data.cassandra.core.mapping.PrimaryKeyColumn; */ @PrimaryKeyClass @Data +@AllArgsConstructor +@NoArgsConstructor public class CompositeKey implements Serializable { @PrimaryKeyColumn(type = PrimaryKeyType.PARTITIONED, ordinal = 1, name = "first_name") private String firstname; diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryMapIdIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryCompositeIdIntegrationTests.java similarity index 57% rename from spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryMapIdIntegrationTests.java rename to spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryCompositeIdIntegrationTests.java index 2459e2bf3..44189dda2 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryMapIdIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryCompositeIdIntegrationTests.java @@ -17,6 +17,7 @@ package org.springframework.data.cassandra.repository.support; import static org.assertj.core.api.Assertions.*; +import java.util.Arrays; import java.util.Collections; import org.junit.jupiter.api.BeforeEach; @@ -26,20 +27,24 @@ import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.cassandra.core.CassandraTemplate; import org.springframework.data.cassandra.core.mapping.BasicMapId; import org.springframework.data.cassandra.core.mapping.MapId; +import org.springframework.data.cassandra.domain.CompositeKey; +import org.springframework.data.cassandra.domain.TypeWithKeyClass; import org.springframework.data.cassandra.domain.TypeWithMapId; import org.springframework.data.cassandra.domain.User; import org.springframework.data.cassandra.test.util.AbstractKeyspaceCreatingIntegrationTests; /** - * Integration tests for {@link SimpleCassandraRepository} using MapId. + * Integration tests for {@link SimpleCassandraRepository} using MapId and primary key classes. * * @author Mark Paluch */ -class SimpleCassandraRepositoryMapIdIntegrationTests extends AbstractKeyspaceCreatingIntegrationTests { +class SimpleCassandraRepositoryCompositeIdIntegrationTests extends AbstractKeyspaceCreatingIntegrationTests { private SimpleCassandraRepository simple; - private SimpleCassandraRepository composite; + private SimpleCassandraRepository mapId; + + private SimpleCassandraRepository primaryKeyClass; @BeforeEach @SuppressWarnings("unchecked") @@ -48,17 +53,22 @@ class SimpleCassandraRepositoryMapIdIntegrationTests extends AbstractKeyspaceCre CassandraTemplate template = new CassandraTemplate(this.session); SchemaTestUtils.potentiallyCreateTableFor(User.class, template); SchemaTestUtils.potentiallyCreateTableFor(TypeWithMapId.class, template); + SchemaTestUtils.potentiallyCreateTableFor(TypeWithKeyClass.class, template); SchemaTestUtils.truncate(TypeWithMapId.class, template); - SchemaTestUtils.truncate(TypeWithMapId.class, template); + SchemaTestUtils.truncate(TypeWithKeyClass.class, template); simple = new SimpleCassandraRepository<>(new MappingCassandraEntityInformation( template.getConverter().getMappingContext().getRequiredPersistentEntity(User.class), template.getConverter()), template); - composite = new SimpleCassandraRepository<>(new MappingCassandraEntityInformation( + mapId = new SimpleCassandraRepository<>(new MappingCassandraEntityInformation( template.getConverter().getMappingContext().getRequiredPersistentEntity(TypeWithMapId.class), template.getConverter()), template); + + primaryKeyClass = new SimpleCassandraRepository<>(new MappingCassandraEntityInformation( + template.getConverter().getMappingContext().getRequiredPersistentEntity(TypeWithKeyClass.class), + template.getConverter()), template); } @Test // DATACASS-661 @@ -81,9 +91,43 @@ class SimpleCassandraRepositoryMapIdIntegrationTests extends AbstractKeyspaceCre withMapId.setFirstname("Walter"); withMapId.setLastname("White"); - composite.save(withMapId); + mapId.save(withMapId); - assertThatThrownBy(() -> composite.findAllById(Collections.singletonList(withMapId.getMapId()))) + assertThatThrownBy(() -> mapId.findAllById(Collections.singletonList(withMapId.getMapId()))) .isInstanceOf(InvalidDataAccessApiUsageException.class); } + + @Test // GH-1298 + void shouldDeleteAllByMapId() { + + TypeWithMapId withMapId1 = new TypeWithMapId(); + withMapId1.setFirstname("Walter"); + withMapId1.setLastname("White"); + + TypeWithMapId withMapId2 = new TypeWithMapId(); + withMapId2.setFirstname("Skyler"); + withMapId2.setLastname("White"); + + mapId.saveAll(Arrays.asList(withMapId1, withMapId2)); + + mapId.deleteAllById(Arrays.asList(withMapId1.getMapId(), withMapId2.getMapId())); + + assertThat(mapId.findAll()).isEmpty(); + } + + @Test // GH-1298 + void shouldDeleteAllByCompositeId() { + + TypeWithKeyClass composite1 = new TypeWithKeyClass(); + composite1.setKey(new CompositeKey("Walter", "White")); + + TypeWithKeyClass composite2 = new TypeWithKeyClass(); + composite2.setKey(new CompositeKey("Skyler", "White")); + + primaryKeyClass.saveAll(Arrays.asList(composite1, composite2)); + + primaryKeyClass.deleteAllById(Arrays.asList(composite1.getKey(), composite2.getKey())); + + assertThat(primaryKeyClass.findAll()).isEmpty(); + } } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryMapIdIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryCompositeIdIntegrationTests.java similarity index 58% rename from spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryMapIdIntegrationTests.java rename to spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryCompositeIdIntegrationTests.java index cce8b9edb..4fcd4f09f 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryMapIdIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryCompositeIdIntegrationTests.java @@ -17,6 +17,7 @@ package org.springframework.data.cassandra.repository.support; import reactor.test.StepVerifier; +import java.util.Arrays; import java.util.Collections; import org.junit.jupiter.api.BeforeEach; @@ -27,20 +28,24 @@ import org.springframework.data.cassandra.core.ReactiveCassandraTemplate; import org.springframework.data.cassandra.core.cql.session.DefaultBridgedReactiveSession; import org.springframework.data.cassandra.core.mapping.BasicMapId; import org.springframework.data.cassandra.core.mapping.MapId; +import org.springframework.data.cassandra.domain.CompositeKey; +import org.springframework.data.cassandra.domain.TypeWithKeyClass; import org.springframework.data.cassandra.domain.TypeWithMapId; import org.springframework.data.cassandra.domain.User; import org.springframework.data.cassandra.test.util.AbstractKeyspaceCreatingIntegrationTests; /** - * Integration tests for {@link SimpleReactiveCassandraRepository} using MapId. + * Integration tests for {@link SimpleReactiveCassandraRepository} using MapId and primary key classes. * * @author Mark Paluch */ -class SimpleReactiveCassandraRepositoryMapIdIntegrationTests extends AbstractKeyspaceCreatingIntegrationTests { +class SimpleReactiveCassandraRepositoryCompositeIdIntegrationTests extends AbstractKeyspaceCreatingIntegrationTests { private SimpleReactiveCassandraRepository simple; - private SimpleReactiveCassandraRepository composite; + private SimpleReactiveCassandraRepository mapId; + + private SimpleReactiveCassandraRepository primaryKeyClass; @BeforeEach @SuppressWarnings("unchecked") @@ -49,9 +54,10 @@ class SimpleReactiveCassandraRepositoryMapIdIntegrationTests extends AbstractKey CassandraTemplate template = new CassandraTemplate(this.session); SchemaTestUtils.potentiallyCreateTableFor(User.class, template); SchemaTestUtils.potentiallyCreateTableFor(TypeWithMapId.class, template); + SchemaTestUtils.potentiallyCreateTableFor(TypeWithKeyClass.class, template); SchemaTestUtils.truncate(TypeWithMapId.class, template); - SchemaTestUtils.truncate(TypeWithMapId.class, template); + SchemaTestUtils.truncate(TypeWithKeyClass.class, template); ReactiveCassandraTemplate reactiveTemplate = new ReactiveCassandraTemplate( new DefaultBridgedReactiveSession(this.session)); @@ -60,9 +66,13 @@ class SimpleReactiveCassandraRepositoryMapIdIntegrationTests extends AbstractKey template.getConverter().getMappingContext().getRequiredPersistentEntity(User.class), template.getConverter()), reactiveTemplate); - composite = new SimpleReactiveCassandraRepository<>(new MappingCassandraEntityInformation( + mapId = new SimpleReactiveCassandraRepository<>(new MappingCassandraEntityInformation( template.getConverter().getMappingContext().getRequiredPersistentEntity(TypeWithMapId.class), template.getConverter()), reactiveTemplate); + + primaryKeyClass = new SimpleReactiveCassandraRepository<>(new MappingCassandraEntityInformation( + template.getConverter().getMappingContext().getRequiredPersistentEntity(TypeWithKeyClass.class), + template.getConverter()), reactiveTemplate); } @Test // DATACASS-661 @@ -91,14 +101,60 @@ class SimpleReactiveCassandraRepositoryMapIdIntegrationTests extends AbstractKey withMapId.setFirstname("Walter"); withMapId.setLastname("White"); - composite.save(withMapId) // + mapId.save(withMapId) // .as(StepVerifier::create) // .expectNextCount(1) // .verifyComplete(); - composite.findAllById(Collections.singletonList(withMapId.getMapId())) // + mapId.findAllById(Collections.singletonList(withMapId.getMapId())) // .as(StepVerifier::create) // .expectNextCount(1) // .verifyComplete(); } + + @Test // GH-1298 + void shouldDeleteAllByMapId() { + + TypeWithMapId withMapId1 = new TypeWithMapId(); + withMapId1.setFirstname("Walter"); + withMapId1.setLastname("White"); + + TypeWithMapId withMapId2 = new TypeWithMapId(); + withMapId2.setFirstname("Skyler"); + withMapId2.setLastname("White"); + + mapId.saveAll(Arrays.asList(withMapId1, withMapId2)).then() // + .as(StepVerifier::create) // + .verifyComplete(); + + mapId.deleteAllById(Arrays.asList(withMapId1.getMapId(), withMapId2.getMapId())) // + .as(StepVerifier::create) // + .verifyComplete(); + + mapId.findAll() // + .as(StepVerifier::create) // + .verifyComplete(); + } + + @Test // GH-1298 + void shouldDeleteAllByCompositeId() { + + TypeWithKeyClass composite1 = new TypeWithKeyClass(); + composite1.setKey(new CompositeKey("Walter", "White")); + + TypeWithKeyClass composite2 = new TypeWithKeyClass(); + composite2.setKey(new CompositeKey("Skyler", "White")); + + primaryKeyClass.saveAll(Arrays.asList(composite1, composite2)).then() // + .as(StepVerifier::create) // + .verifyComplete(); + + primaryKeyClass.deleteAllById(Arrays.asList(composite1.getKey(), composite2.getKey())) // + .as(StepVerifier::create) // + .verifyComplete(); + + primaryKeyClass.findAll() // + .as(StepVerifier::create) // + .verifyComplete(); + } }