DATACASS-420 - Do not insert null values.

We now no longer insert null values via CassandraOperations.insert(…). Inserting null values creates tombstones in Cassandra which are likely unwanted and impact performance. Omitting null values simply does not create values in Cassandra. Setting properties to null with UPDATE remains unchanged and updating an object containing null values will propagate all null values to Cassandra as part of the UPDATE clause.

SimpleCassandraRepository.save(…) needs to consider whether the entity is new or whether it consists only of primary key properties to propagate the optimization. New entities and entities consisting only of primary key columns are saved via INSERT. All other entities are saved via UPDATE.

Related ticket: DATACASS-428.
This commit is contained in:
Mark Paluch
2017-04-12 13:58:55 +02:00
parent d00ce554d0
commit 76ab62e2d3
8 changed files with 196 additions and 43 deletions

View File

@@ -358,6 +358,10 @@ public class MappingCassandraConverter extends AbstractCassandraConverter
return;
}
if (value == null) {
return;
}
if (log.isDebugEnabled()) {
log.debug("Adding insert.value [{}] - [{}]", property.getColumnName().toCql(), value);
}

View File

@@ -42,7 +42,7 @@ import com.datastax.driver.core.querybuilder.Select;
* <p>
* {@link CassandraOperations} mixes synchronous and asynchronous methods so asynchronous methods are subject to be
* moved into an asynchronous Cassandra template.
*
*
* @author Alex Shvid
* @author David Webb
* @author Matthew Adams
@@ -197,7 +197,7 @@ public interface CassandraOperations extends CqlOperations {
long count(Class<?> entityClass);
/**
* Insert the given entity.
* Insert the given entity without inserting {@literal null} values.
*
* @param entity The entity to insert
* @return The entity given
@@ -205,7 +205,7 @@ public interface CassandraOperations extends CqlOperations {
<T> T insert(T entity);
/**
* Insert the given entity.
* Insert the given entity without inserting {@literal null} values.
*
* @param entity The entity to insert
* @param options The {@link WriteOptions} to use.
@@ -525,7 +525,7 @@ public interface CassandraOperations extends CqlOperations {
/**
* Deletes all entities of a given class.
*
*
* @param entityClass The entity type must not be {@literal null}.
*/
<T> void deleteAll(Class<T> entityClass);

View File

@@ -1,12 +1,12 @@
/*
* Copyright 2013-2014 the original author or authors
*
* Copyright 2013-2017 the original author or authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -21,10 +21,20 @@ import org.springframework.data.repository.core.EntityInformation;
/**
* Cassandra specific {@link EntityInformation}.
*
*
* @author Alex Shvid
* @author Mark Paluch
*/
public interface CassandraEntityInformation<T, ID extends Serializable> extends EntityInformation<T, ID>,
CassandraEntityMetadata<T> {
public interface CassandraEntityInformation<T, ID extends Serializable>
extends EntityInformation<T, ID>, CassandraEntityMetadata<T> {
/**
* Return {@literal true} if the persistent entity consists entirely of primary key properties (a single Id property,
* composite primary key).
*
* @return {@literal true} if the persistent entity consists entirely of primary key properties (a single Id property,
* composite primary key).
* @since 1.5.2
*/
boolean isPrimaryKeyEntity();
}

View File

@@ -16,6 +16,7 @@
package org.springframework.data.cassandra.repository.support;
import java.io.Serializable;
import java.util.concurrent.atomic.AtomicReference;
import org.springframework.cassandra.core.cql.CqlIdentifier;
import org.springframework.data.cassandra.convert.CassandraConverter;
@@ -23,6 +24,7 @@ import org.springframework.data.cassandra.mapping.CassandraPersistentEntity;
import org.springframework.data.cassandra.mapping.CassandraPersistentProperty;
import org.springframework.data.cassandra.repository.MapId;
import org.springframework.data.cassandra.repository.query.CassandraEntityInformation;
import org.springframework.data.mapping.PropertyHandler;
import org.springframework.data.repository.core.support.AbstractEntityInformation;
import org.springframework.util.Assert;
@@ -33,12 +35,14 @@ import org.springframework.util.Assert;
*
* @author Alex Shvid
* @author Matthew T. Adams
* @author Mark Paluch
*/
public class MappingCassandraEntityInformation<T, ID extends Serializable> extends AbstractEntityInformation<T, ID>
implements CassandraEntityInformation<T, ID> {
private final CassandraPersistentEntity<T> entityMetadata;
private CassandraConverter converter;
private final CassandraConverter converter;
private final boolean isPrimaryKeyEntity;
/**
* Creates a new {@link MappingCassandraEntityInformation} for the given {@link CassandraPersistentEntity}.
@@ -51,6 +55,7 @@ public class MappingCassandraEntityInformation<T, ID extends Serializable> exten
this.entityMetadata = entity;
this.converter = converter;
this.isPrimaryKeyEntity = hasNonIdProperties(entity);
}
@SuppressWarnings("unchecked")
@@ -79,4 +84,28 @@ public class MappingCassandraEntityInformation<T, ID extends Serializable> exten
public CqlIdentifier getTableName() {
return entityMetadata.getTableName();
}
@Override
public boolean isPrimaryKeyEntity() {
return isPrimaryKeyEntity;
}
private static boolean hasNonIdProperties(CassandraPersistentEntity<?> entity) {
final AtomicReference<Boolean> hasPrimaryKeyOnlyProperties = new AtomicReference<Boolean>(true);
entity.doWithProperties(new PropertyHandler<CassandraPersistentProperty>() {
@Override
public void doWithPersistentProperty(CassandraPersistentProperty property) {
if (property.isCompositePrimaryKey() || property.isPrimaryKeyColumn() || property.isIdProperty()) {
return;
}
hasPrimaryKeyOnlyProperties.set(false);
}
});
return hasPrimaryKeyOnlyProperties.get();
}
}

View File

@@ -39,6 +39,8 @@ public class SimpleCassandraRepository<T, ID extends Serializable> implements Ty
protected CassandraOperations operations;
protected CassandraEntityInformation<T, ID> entityInformation;
private final boolean isPrimaryKeyEntity;
/**
* Creates a new {@link SimpleCassandraRepository} for the given {@link CassandraEntityInformation} and
* {@link CassandraTemplate}.
@@ -53,6 +55,7 @@ public class SimpleCassandraRepository<T, ID extends Serializable> implements Ty
this.entityInformation = metadata;
this.operations = operations;
this.isPrimaryKeyEntity = metadata.isPrimaryKeyEntity();
}
/* (non-Javadoc)
@@ -63,7 +66,7 @@ public class SimpleCassandraRepository<T, ID extends Serializable> implements Ty
Assert.notNull(entity, "Entity must not be null");
if (entityInformation.isNew(entity)) {
if (entityInformation.isNew(entity) || isPrimaryKeyEntity) {
return operations.insert(entity);
}

View File

@@ -156,8 +156,8 @@ public class MappingCassandraConverterUDTIntegrationTests extends AbstractSpring
Insert insert = QueryBuilder.insertInto("addressbook");
converter.write(addressBook, insert);
assertThat(insert.toString()).isEqualTo("INSERT INTO addressbook (alternate,currentaddress,id,previousaddresses) "
+ "VALUES (null,{zip:'69469',city:'Weinheim',streetlines:['Heckenpfad','14']},'1',null);");
assertThat(insert.toString()).isEqualTo("INSERT INTO addressbook (currentaddress,id) "
+ "VALUES ({zip:'69469',city:'Weinheim',streetlines:['Heckenpfad','14']},'1');");
}
@Test // DATACASS-172
@@ -193,8 +193,8 @@ public class MappingCassandraConverterUDTIntegrationTests extends AbstractSpring
Insert insert = QueryBuilder.insertInto("addressbook");
converter.write(addressBook, insert);
assertThat(insert.toString()).isEqualTo("INSERT INTO addressbook (alternate,currentaddress,id,previousaddresses) "
+ "VALUES (null,null,'1',[{zip:'69469',city:'Weinheim',streetlines:['Heckenpfad','14']}]);");
assertThat(insert.toString()).isEqualTo("INSERT INTO addressbook (id,previousaddresses) "
+ "VALUES ('1',[{zip:'69469',city:'Weinheim',streetlines:['Heckenpfad','14']}]);");
}
@Test // DATACASS-172
@@ -228,8 +228,8 @@ public class MappingCassandraConverterUDTIntegrationTests extends AbstractSpring
Insert insert = QueryBuilder.insertInto("addressbook");
converter.write(addressBook, insert);
assertThat(insert.toString()).isEqualTo("INSERT INTO addressbook (alternate,currentaddress,id,previousaddresses) "
+ "VALUES ({zip:'69469',city:'Weinheim',streetlines:['Heckenpfad','14']},null,'1',null);");
assertThat(insert.toString()).isEqualTo("INSERT INTO addressbook (alternate,id) "
+ "VALUES ({zip:'69469',city:'Weinheim',streetlines:['Heckenpfad','14']},'1');");
}
@Test // DATACASS-172
@@ -302,7 +302,7 @@ public class MappingCassandraConverterUDTIntegrationTests extends AbstractSpring
converter.write(bank, insert);
assertThat(insert.toString())
.isEqualTo("INSERT INTO bank (currency,id,othercurrencies) VALUES ({currency:'EUR'},null,null);");
.isEqualTo("INSERT INTO bank (currency) VALUES ({currency:'EUR'});");
}
@Test // DATACASS-172
@@ -361,7 +361,7 @@ public class MappingCassandraConverterUDTIntegrationTests extends AbstractSpring
converter.write(bank, insert);
assertThat(insert.toString())
.isEqualTo("INSERT INTO bank (currency,id,othercurrencies) VALUES (null,null,[{currency:'EUR'}]);");
.isEqualTo("INSERT INTO bank (othercurrencies) VALUES ([{currency:'EUR'}]);");
}
@Test // DATACASS-172

View File

@@ -0,0 +1,106 @@
/*
* Copyright 2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.cassandra.repository.support;
import static org.assertj.core.api.Assertions.*;
import lombok.Data;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.springframework.data.annotation.Id;
import org.springframework.data.cassandra.convert.CassandraConverter;
import org.springframework.data.cassandra.convert.MappingCassandraConverter;
import org.springframework.data.cassandra.domain.AllPossibleTypes;
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.mapping.BasicCassandraMappingContext;
import org.springframework.data.cassandra.mapping.UserTypeResolver;
/**
* Unit tests for {@link MappingCassandraEntityInformation}.
*
* @author Mark Paluch
*/
@RunWith(MockitoJUnitRunner.class)
@SuppressWarnings("unchecked")
public class MappingCassandraEntityInformationUnitTests {
BasicCassandraMappingContext context = new BasicCassandraMappingContext();
CassandraConverter converter = new MappingCassandraConverter(context);
@Mock UserTypeResolver userTypeResolver;
@Before
public void before() {
context.setUserTypeResolver(userTypeResolver);
}
@Test // DATACASS-420
public void shouldConsiderSimpleIdEntityAsPrimaryKeyOnly() {
MappingCassandraEntityInformation information = new MappingCassandraEntityInformation(
context.getPersistentEntity(PrimaryKeyOnly.class), converter);
assertThat(information.isPrimaryKeyEntity()).isTrue();
}
@Test // DATACASS-420
public void shouldConsiderCompositeIdEntityAsPrimaryKeyOnly() {
MappingCassandraEntityInformation information = new MappingCassandraEntityInformation(
context.getPersistentEntity(CompositeKey.class), converter);
assertThat(information.isPrimaryKeyEntity()).isTrue();
}
@Test // DATACASS-420
public void shouldConsiderCompositeKeyClassEntityAsPrimaryKeyOnly() {
MappingCassandraEntityInformation information = new MappingCassandraEntityInformation(
context.getPersistentEntity(TypeWithKeyClass.class), converter);
assertThat(information.isPrimaryKeyEntity()).isTrue();
}
@Test // DATACASS-420
public void shouldConsiderMapIdClassEntityAsPrimaryKeyOnly() {
MappingCassandraEntityInformation information = new MappingCassandraEntityInformation(
context.getPersistentEntity(TypeWithMapId.class), converter);
assertThat(information.isPrimaryKeyEntity()).isTrue();
}
@Test // DATACASS-420
public void shouldComplexEntityNotAsPrimaryKeyOnly() {
MappingCassandraEntityInformation information = new MappingCassandraEntityInformation(
context.getPersistentEntity(AllPossibleTypes.class), converter);
assertThat(information.isPrimaryKeyEntity()).isFalse();
}
@Data
static class PrimaryKeyOnly {
@Id String id;
}
}

View File

@@ -627,27 +627,8 @@ public class CassandraOperationsIntegrationTests extends AbstractKeyspaceCreatin
assertThat(template.count(Book.class)).isEqualTo(count);
}
@Test // DATACASS-182
public void updateShouldRemoveFields() {
Book book = new Book();
book.setIsbn("isbn");
book.setTitle("title");
book.setAuthor("author");
template.insert(book);
book.setTitle(null);
template.update(book);
Book loaded = template.selectOneById(Book.class, book.getIsbn());
assertThat(loaded.getTitle()).isNull();
assertThat(loaded.getAuthor()).isEqualTo("author");
}
@Test // DATACASS-182
public void insertShouldRemoveFields() {
@Test // DATACASS-182, DATACASS-420
public void insertShouldNotRemoveFields() {
Book book = new Book();
book.setIsbn("isbn");
@@ -662,7 +643,7 @@ public class CassandraOperationsIntegrationTests extends AbstractKeyspaceCreatin
Book loaded = template.selectOneById(Book.class, book.getIsbn());
assertThat(loaded.getTitle()).isNull();
assertThat(loaded.getTitle()).isEqualTo("title");
assertThat(loaded.getAuthor()).isEqualTo("author");
}
@@ -683,6 +664,26 @@ public class CassandraOperationsIntegrationTests extends AbstractKeyspaceCreatin
assertThat(loaded.getTitle()).isEqualTo("title");
}
@Test // DATACASS-182, DATACASS-420
public void updateShouldRemoveFields() {
Book book = new Book();
book.setIsbn("isbn");
book.setTitle("title");
book.setAuthor("author");
template.insert(book);
book.setTitle(null);
template.update(book);
Book loaded = template.selectOneById(Book.class, book.getIsbn());
assertThat(loaded.getTitle()).isNull();
assertThat(loaded.getAuthor()).isEqualTo("author");
}
@Test // DATACASS-182
public void insertAndUpdateToEmptyCollection() {