Spring/spring-data-cassandra
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATACASS-661 - Fix findAllById(…) repository queries using MapId.

Browse Source 
We now detect the Id property repositories using MapId with single keys. Cassandra supports only single keys with IN queries so we reject composite keys using imperative repositories with MapId in findAllById(findAllById).

The reactive findAllById(…) repository method fetched rows one-by-one. We now optimize the query using IN queries with a single roundtrip if possible. Composite MapId keys are fetched one-by-one as previously done as fallback.
This commit is contained in:
Mark Paluch
2019-06-05 13:01:57 +02:00
parent 664ba968f0
commit 9a62942861
9 changed files with 410 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/CassandraRepository.java
View File

@@ -54,8 +54,14 @@ public interface CassandraRepository<T, ID> extends CrudRepository<T, ID> {
@Override
List<T> findAll();
/* (non-Javadoc)
* @see org.springframework.data.repository.CrudRepository#findAllById(java.lang.Iterable)
/**
* {@inheritDoc}
* <p/>
* Note: Cassandra supports single-field {@code IN} queries only. When using {@link MapId} with multiple components,
* use {@link #findById(Object)}.
*
* @throws org.springframework.dao.InvalidDataAccessApiUsageException thrown when using {@link MapId} with multiple
* key components.
*/
@Override
List<T> findAllById(Iterable<ID> ids);

17
spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/ReactiveCassandraRepository.java
View File

@@ -60,4 +60,21 @@ public interface ReactiveCassandraRepository<T, ID> extends ReactiveCrudReposito
* @return the saved entity
*/
<S extends T> Flux<S> insert(Publisher<S> entities);
/**
* {@inheritDoc}
* <p/>
* Note: Cassandra supports single-field {@code IN} queries only. Fetches each row individually when using
* {@link MapId} with multiple components.
*/
@Override
Flux<T> findAllById(Iterable<ID> iterable);
/**
* {@inheritDoc}
* <p/>
* Fetches each row individually.
*/
@Override
Flux<T> findAllById(Publisher<ID> publisher);
}

118
spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/FindByIdQuery.java Normal file
View File

@@ -0,0 +1,118 @@
/*
* Copyright 2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* 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.cassandra.repository.support;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.cassandra.core.mapping.MapId;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
/**
* Value object representing a Find by ID query supporting also {@link MapId}.
*
* @author Mark Paluch
* @since 2.2
*/
class FindByIdQuery {
private final @Nullable String idProperty;
private final List<Object> idCollection;
private FindByIdQuery(@Nullable String idProperty, List<Object> idCollection) {
this.idProperty = idProperty;
this.idCollection = idCollection;
}
/**
* Create a new {@link FindByIdQuery} given {@link Iterable} of {@code ID}s. Id's can be either scalar values or
* {@link MapId}s. In case of the latter, this method discovers the {@link #getIdProperty() Id property name}.
*
* @param ids
* @return
*/
static FindByIdQuery forIds(Iterable<?> ids) {
Assert.notNull(ids, "The given Iterable of ids must not be null");
List<Object> idCollection = new ArrayList<>();
String idField = null;
for (Object id : ids) {
if (id instanceof MapId) {
MapId mapId = (MapId) id;
Iterator<String> iterator = mapId.keySet().iterator();
if (mapId.size() > 1) {
throw new InvalidDataAccessApiUsageException("MapId with multiple keys are not supported");
}
if (!iterator.hasNext()) {
throw new InvalidDataAccessApiUsageException("MapId is empty");
} else {
idField = iterator.next();
idCollection.add(mapId.get(idField));
}
} else {
idCollection.add(id);
}
}
return new FindByIdQuery(idField, idCollection);
}
/**
* Check if the {@link Iterable} of {@code ID}s contains composite keys.
*
* @param ids
* @return
*/
static boolean hasCompositeKeys(Iterable<?> ids) {
Assert.notNull(ids, "The given Iterable of ids must not be null");
for (Object id : ids) {
if (id instanceof MapId) {
MapId mapId = (MapId) id;
Iterator<String> iterator = mapId.keySet().iterator();
if (mapId.size() > 1) {
return true;
}
}
}
return false;
}
@Nullable
String getIdProperty() {
return idProperty;
}
List<Object> getIdCollection() {
return idCollection;
}
}

26
spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepository.java
View File

@@ -15,9 +15,10 @@
*/
package org.springframework.data.cassandra.repository.support;
import static org.springframework.data.cassandra.core.query.Criteria.where;
import static org.springframework.data.cassandra.core.query.Criteria.*;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
@@ -32,8 +33,6 @@ import org.springframework.data.cassandra.repository.query.CassandraEntityInform
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Slice;
import org.springframework.data.mapping.context.AbstractMappingContext;
import org.springframework.data.util.StreamUtils;
import org.springframework.data.util.Streamable;
import org.springframework.util.Assert;
import com.datastax.driver.core.querybuilder.Insert;
@@ -161,7 +160,11 @@ public class SimpleCassandraRepository<T, ID> implements CassandraRepository<T,
Assert.notNull(id, "The given id must not be null");
return Optional.ofNullable(this.operations.selectOneById(id, this.entityInformation.getJavaType()));
return Optional.ofNullable(doFindOne(id));
}
private T doFindOne(ID id) {
return this.operations.selectOneById(id, this.entityInformation.getJavaType());
}
/* (non-Javadoc)
@@ -202,10 +205,19 @@ public class SimpleCassandraRepository<T, ID> implements CassandraRepository<T,
Assert.notNull(ids, "The given Iterable of id's must not be null");
List<ID> idCollection = Streamable.of(ids).stream().collect(StreamUtils.toUnmodifiableList());
FindByIdQuery mapIdQuery = FindByIdQuery.forIds(ids);
List<Object> idCollection = mapIdQuery.getIdCollection();
String idField = mapIdQuery.getIdProperty();
return this.operations.select(Query.query(where(this.entityInformation.getIdAttribute()).in(idCollection)),
this.entityInformation.getJavaType());
if (idCollection.isEmpty()) {
return Collections.emptyList();
}
if (idField == null) {
idField = this.entityInformation.getIdAttribute();
}
return this.operations.select(Query.query(where(idField).in(idCollection)), this.entityInformation.getJavaType());
}
/* (non-Javadoc)

28
spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepository.java
View File

@@ -15,15 +15,21 @@
*/
package org.springframework.data.cassandra.repository.support;
import static org.springframework.data.cassandra.core.query.Criteria.*;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import java.util.List;
import org.reactivestreams.Publisher;
import org.springframework.data.cassandra.core.EntityWriteResult;
import org.springframework.data.cassandra.core.InsertOptions;
import org.springframework.data.cassandra.core.ReactiveCassandraOperations;
import org.springframework.data.cassandra.core.mapping.BasicCassandraPersistentEntity;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
import org.springframework.data.cassandra.core.query.Query;
import org.springframework.data.cassandra.repository.ReactiveCassandraRepository;
import org.springframework.data.cassandra.repository.query.CassandraEntityInformation;
import org.springframework.data.mapping.context.AbstractMappingContext;
@@ -229,11 +235,27 @@ public class SimpleReactiveCassandraRepository<T, ID> implements ReactiveCassand
* @see org.springframework.data.repository.reactive.ReactiveCrudRepository#findAllById(java.lang.Iterable)
*/
@Override
public Flux<T> findAllById(Iterable<ID> iterable) {
public Flux<T> findAllById(Iterable<ID> ids) {
Assert.notNull(iterable, "The given Iterable of ids must not be null");
Assert.notNull(ids, "The given Iterable of ids must not be null");
return findAllById(Flux.fromIterable(iterable));
if (FindByIdQuery.hasCompositeKeys(ids)) {
return findAllById(Flux.fromIterable(ids));
}
FindByIdQuery query = FindByIdQuery.forIds(ids);
List<Object> idCollection = query.getIdCollection();
String idField = query.getIdProperty();
if (idCollection.isEmpty()) {
return Flux.empty();
}
if (idField == null) {
idField = this.entityInformation.getIdAttribute();
}
return this.operations.select(Query.query(where(idField).in(idCollection)), this.entityInformation.getJavaType());
}
/*

35
spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/mapid/RepositoryMapIdIntegrationTests.java
View File

@@ -18,12 +18,18 @@ package org.springframework.data.cassandra.repository.mapid;
import static org.assertj.core.api.Assertions.*;
import static org.springframework.data.cassandra.core.mapping.BasicMapId.*;
import java.util.Collections;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.cassandra.core.CassandraOperations;
import org.springframework.data.cassandra.core.mapping.BasicMapId;
import org.springframework.data.cassandra.core.mapping.MapId;
import org.springframework.data.cassandra.repository.config.EnableCassandraRepositories;
import org.springframework.data.cassandra.repository.support.AbstractSpringDataEmbeddedCassandraIntegrationTest;
@@ -52,13 +58,13 @@ public class RepositoryMapIdIntegrationTests extends AbstractSpringDataEmbeddedC
}
@Autowired CassandraOperations template;
@Autowired SinglePrimaryKecColumnRepository singlePrimaryKecColumnRepository;
@Autowired SinglePrimaryKecColumnRepository singlePrimaryKeyColumnRepository;
@Autowired MultiPrimaryKeyColumnsRepository multiPrimaryKeyColumnsRepository;
@Before
public void before() {
assertThat(template).isNotNull();
assertThat(singlePrimaryKecColumnRepository).isNotNull();
assertThat(singlePrimaryKeyColumnRepository).isNotNull();
assertThat(multiPrimaryKeyColumnsRepository).isNotNull();
}
@@ -68,28 +74,38 @@ public class RepositoryMapIdIntegrationTests extends AbstractSpringDataEmbeddedC
// insert
SinglePrimaryKeyColumn inserted = new SinglePrimaryKeyColumn(uuid());
inserted.setValue(uuid());
SinglePrimaryKeyColumn saved = singlePrimaryKecColumnRepository.save(inserted);
SinglePrimaryKeyColumn saved = singlePrimaryKeyColumnRepository.save(inserted);
assertThat(inserted).isSameAs(saved);
// select
MapId id = id("key", saved.getKey());
SinglePrimaryKeyColumn selected = singlePrimaryKecColumnRepository.findById(id).get();
SinglePrimaryKeyColumn selected = singlePrimaryKeyColumnRepository.findById(id).get();
assertThat(saved).isNotSameAs(selected);
assertThat(selected.getKey()).isEqualTo(saved.getKey());
assertThat(selected.getValue()).isEqualTo(saved.getValue());
List<SinglePrimaryKeyColumn> allById = singlePrimaryKeyColumnRepository.findAllById(Collections.singletonList(id));
assertThat(allById).containsOnly(saved);
// update
selected.setValue(uuid());
SinglePrimaryKeyColumn updated = singlePrimaryKecColumnRepository.save(selected);
SinglePrimaryKeyColumn updated = singlePrimaryKeyColumnRepository.save(selected);
assertThat(selected).isSameAs(updated);
selected = singlePrimaryKecColumnRepository.findById(id).get();
selected = singlePrimaryKeyColumnRepository.findById(id).get();
assertThat(updated).isNotSameAs(selected);
assertThat(selected.getValue()).isEqualTo(updated.getValue());
// delete
singlePrimaryKecColumnRepository.delete(selected);
assertThat(singlePrimaryKecColumnRepository.findById(id)).isEmpty();
singlePrimaryKeyColumnRepository.delete(selected);
assertThat(singlePrimaryKeyColumnRepository.findById(id)).isEmpty();
}
@Test // DATACASS-661
public void findAllByIdRejectsEmptyMapId() {
assertThatThrownBy(() -> multiPrimaryKeyColumnsRepository.findAllById(Collections.singletonList(BasicMapId.id())))
.isInstanceOf(InvalidDataAccessApiUsageException.class);
}
@Test
@@ -109,6 +125,9 @@ public class RepositoryMapIdIntegrationTests extends AbstractSpringDataEmbeddedC
assertThat(selected.getKey1()).isEqualTo(saved.getKey1());
assertThat(selected.getValue()).isEqualTo(saved.getValue());
assertThatThrownBy(() -> multiPrimaryKeyColumnsRepository.findAllById(Collections.singletonList(id)))
.isInstanceOf(InvalidDataAccessApiUsageException.class);
// update
selected.setValue(uuid());
MultiPrimaryKeyColumns updated = multiPrimaryKeyColumnsRepository.save(selected);

3
spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/mapid/SinglePrimaryKeyColumn.java
View File

@@ -15,6 +15,8 @@
*/
package org.springframework.data.cassandra.repository.mapid;
import lombok.EqualsAndHashCode;
import org.springframework.data.cassandra.core.cql.PrimaryKeyType;
import org.springframework.data.cassandra.core.mapping.Column;
import org.springframework.data.cassandra.core.mapping.PrimaryKeyColumn;
@@ -24,6 +26,7 @@ import org.springframework.data.cassandra.core.mapping.Table;
* @author Matthew T. Adams
*/
@Table
@EqualsAndHashCode
public class SinglePrimaryKeyColumn {
@PrimaryKeyColumn(ordinal = 0, type = PrimaryKeyType.PARTITIONED) String key;

89
spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleCassandraRepositoryMapIdIntegrationTests.java Normal file
View File

@@ -0,0 +1,89 @@
/*
* Copyright 2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* 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.cassandra.repository.support;
import static org.assertj.core.api.Assertions.*;
import java.util.Collections;
import org.junit.Before;
import org.junit.Test;
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.TypeWithMapId;
import org.springframework.data.cassandra.domain.User;
import org.springframework.data.cassandra.test.util.AbstractKeyspaceCreatingIntegrationTest;
/**
* Integration tests for {@link SimpleCassandraRepository} using MapId.
*
* @author Mark Paluch
*/
public class SimpleCassandraRepositoryMapIdIntegrationTests extends AbstractKeyspaceCreatingIntegrationTest {
private SimpleCassandraRepository<User, MapId> simple;
private SimpleCassandraRepository<TypeWithMapId, MapId> composite;
@Before
@SuppressWarnings("unchecked")
public void setUp() {
CassandraTemplate template = new CassandraTemplate(this.session);
SchemaTestUtils.potentiallyCreateTableFor(User.class, template);
SchemaTestUtils.potentiallyCreateTableFor(TypeWithMapId.class, template);
SchemaTestUtils.truncate(TypeWithMapId.class, template);
SchemaTestUtils.truncate(TypeWithMapId.class, template);
simple = new SimpleCassandraRepository<>(new MappingCassandraEntityInformation(
template.getConverter().getMappingContext().getRequiredPersistentEntity(User.class), template.getConverter()),
template);
composite = new SimpleCassandraRepository<>(new MappingCassandraEntityInformation(
template.getConverter().getMappingContext().getRequiredPersistentEntity(TypeWithMapId.class),
template.getConverter()), template);
}
@Test // DATACASS-661
public void shouldFindByIdWithSimpleKey() {
User user = new User();
user.setId("heisenberg");
user.setFirstname("Walter");
user.setLastname("White");
simple.save(user);
assertThat(simple.findAllById(Collections.singletonList(BasicMapId.id("id", user.getId())))).hasSize(1);
}
@Test // DATACASS-661
public void shouldFindByIdWithCompositeKey() {
TypeWithMapId withMapId = new TypeWithMapId();
withMapId.setFirstname("Walter");
withMapId.setLastname("White");
composite.save(withMapId);
assertThatThrownBy(() -> composite.findAllById(Collections.singletonList(withMapId.getMapId())))
.isInstanceOf(InvalidDataAccessApiUsageException.class);
}
}

104
spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/SimpleReactiveCassandraRepositoryMapIdIntegrationTests.java Normal file
View File

@@ -0,0 +1,104 @@
/*
* Copyright 2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* 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.cassandra.repository.support;
import reactor.test.StepVerifier;
import java.util.Collections;
import org.junit.Before;
import org.junit.Test;
import org.springframework.data.cassandra.core.CassandraTemplate;
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.TypeWithMapId;
import org.springframework.data.cassandra.domain.User;
import org.springframework.data.cassandra.test.util.AbstractKeyspaceCreatingIntegrationTest;
/**
* Integration tests for {@link SimpleReactiveCassandraRepository} using MapId.
*
* @author Mark Paluch
*/
public class SimpleReactiveCassandraRepositoryMapIdIntegrationTests extends AbstractKeyspaceCreatingIntegrationTest {
private SimpleReactiveCassandraRepository<User, MapId> simple;
private SimpleReactiveCassandraRepository<TypeWithMapId, MapId> composite;
@Before
@SuppressWarnings("unchecked")
public void setUp() {
CassandraTemplate template = new CassandraTemplate(this.session);
SchemaTestUtils.potentiallyCreateTableFor(User.class, template);
SchemaTestUtils.potentiallyCreateTableFor(TypeWithMapId.class, template);
SchemaTestUtils.truncate(TypeWithMapId.class, template);
SchemaTestUtils.truncate(TypeWithMapId.class, template);
ReactiveCassandraTemplate reactiveTemplate = new ReactiveCassandraTemplate(
new DefaultBridgedReactiveSession(this.session));
simple = new SimpleReactiveCassandraRepository<>(new MappingCassandraEntityInformation(
template.getConverter().getMappingContext().getRequiredPersistentEntity(User.class), template.getConverter()),
reactiveTemplate);
composite = new SimpleReactiveCassandraRepository<>(new MappingCassandraEntityInformation(
template.getConverter().getMappingContext().getRequiredPersistentEntity(TypeWithMapId.class),
template.getConverter()), reactiveTemplate);
}
@Test // DATACASS-661
public void shouldFindByIdWithSimpleKey() {
User user = new User();
user.setId("heisenberg");
user.setFirstname("Walter");
user.setLastname("White");
simple.save(user) //
.as(StepVerifier::create) //
.expectNextCount(1) //
.verifyComplete();
simple.findAllById(Collections.singletonList(BasicMapId.id("id", user.getId()))) //
.as(StepVerifier::create) //
.expectNextCount(1) //
.verifyComplete();
}
@Test // DATACASS-661
public void shouldFindByIdWithCompositeKey() {
TypeWithMapId withMapId = new TypeWithMapId();
withMapId.setFirstname("Walter");
withMapId.setLastname("White");
composite.save(withMapId) //
.as(StepVerifier::create) //
.expectNextCount(1) //
.verifyComplete();
composite.findAllById(Collections.singletonList(withMapId.getMapId())) //
.as(StepVerifier::create) //
.expectNextCount(1) //
.verifyComplete();
}
}