From d0610b2812f91e1d47862d178ad68df204ba4328 Mon Sep 17 00:00:00 2001 From: John Blum Date: Sun, 24 Jul 2016 20:17:33 -0700 Subject: [PATCH] DATACASS-288 - Polish. Re-implemented CassandraBatchTemplate to take a vararg array of Object entities rather than a single entity and guarded against null. Original pull request: #78. --- .../core/CassandraBatchOperations.java | 73 +++++++------- .../core/CassandraBatchTemplate.java | 99 +++++++++---------- .../cassandra/core/CassandraTemplate.java | 7 +- ...assandraBatchTemplateIntegrationTests.java | 32 +++--- .../core/CassandraTemplateUnitTests.java | 3 +- 5 files changed, 109 insertions(+), 105 deletions(-) diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchOperations.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchOperations.java index c2962c937..59d37fd32 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchOperations.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchOperations.java @@ -36,8 +36,8 @@ public interface CassandraBatchOperations { /** * Execute the batch. The batch can be executed only once. - * - * @throws IllegalStateException if the batch is executed after it was executed once + * + * @throws IllegalStateException if the batch is executed after it was executed already. */ void execute(); @@ -45,62 +45,63 @@ public interface CassandraBatchOperations { * Apply a given {@code timestamp} to the whole batch. * * @param timestamp the timestamp to apply. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ CassandraBatchOperations withTimestamp(long timestamp); /** - * Add a single insert to the batch. + * Add an array of inserts to the batch. * - * @param entity the entity to insert, must not be {@literal null}. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * @param entities the entities to insert; must not be {@literal null}. + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ - CassandraBatchOperations insert(Object entity); + CassandraBatchOperations insert(Object... entities); /** * Add a collection of inserts to the batch. - * - * @param entities the entities to insert, must not be {@literal null}. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * + * @param entities the entities to insert; must not be {@literal null}. + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ - CassandraBatchOperations insert(Iterable entities); + CassandraBatchOperations insert(Iterable entities); /** - * Add a single update to the batch. - * - * @param entity the entity to update, must not be {@literal null}. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * Add an array of updates to the batch. + * + * @param entities the entities to update; must not be {@literal null}. + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ - CassandraBatchOperations update(Object entity); + CassandraBatchOperations update(Object... entities); /** * Add a collection of updates to the batch. - * - * @param entities the entities to insert, must not be {@literal null}. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * + * @param entities the entities to update; must not be {@literal null}. + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ - CassandraBatchOperations update(Iterable entities); + CassandraBatchOperations update(Iterable entities); /** - * Add a single delete to the batch. - * - * @param entity the entity to delete, must not be {@literal null}. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * Add an array of deletes to the batch. + * + * @param entities the entities to delete; must not be {@literal null}. + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ - CassandraBatchOperations delete(Object entity); + CassandraBatchOperations delete(Object... entities); /** * Add a collection of deletes to the batch. - * - * @param entities the entities to delete, must not be {@literal null}. - * @return {@code this} {@link CassandraBatchOperations} - * @throws IllegalStateException if the batch was already executed + * + * @param entities the entities to delete; must not be {@literal null}. + * @return {@code this} {@link CassandraBatchOperations}. + * @throws IllegalStateException if the batch was already executed. */ - CassandraBatchOperations delete(Iterable entities); + CassandraBatchOperations delete(Iterable entities); + } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchTemplate.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchTemplate.java index a7dd68c99..55c759f50 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchTemplate.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraBatchTemplate.java @@ -15,6 +15,8 @@ */ package org.springframework.data.cassandra.core; +import java.util.Arrays; +import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import org.springframework.util.Assert; @@ -26,14 +28,19 @@ import com.datastax.driver.core.querybuilder.QueryBuilder; * Default implementation for {@link CassandraBatchOperations}. * * @author Mark Paluch + * @author John Blum * @since 1.5 */ class CassandraBatchTemplate implements CassandraBatchOperations { - private final CassandraTemplate cassandraTemplate; - private final Batch batch; + static final Object[] EMPTY_ARRAY = new Object[0]; + private AtomicBoolean executed = new AtomicBoolean(); + private final Batch batch; + + private final CassandraTemplate cassandraTemplate; + public CassandraBatchTemplate(CassandraTemplate cassandraTemplate) { Assert.notNull(cassandraTemplate, "CassandraTemplate must not be null"); @@ -42,7 +49,7 @@ class CassandraBatchTemplate implements CassandraBatchOperations { this.batch = QueryBuilder.batch(); } - /* + /* * (non-Javadoc) * @see org.springframework.data.cassandra.core.CassandraBatchOperations#execute() */ @@ -54,48 +61,42 @@ class CassandraBatchTemplate implements CassandraBatchOperations { return; } - ensureNotExecuted(); + assertNotExecuted(); } - /* + /* * (non-Javadoc) * @see org.springframework.data.cassandra.core.CassandraBatchOperations#withTimestamp(long) */ @Override public CassandraBatchOperations withTimestamp(long timestamp) { - ensureNotExecuted(); + assertNotExecuted(); batch.using(QueryBuilder.timestamp(timestamp)); + return this; } - /* + /* * (non-Javadoc) - * @see org.springframework.data.cassandra.core.CassandraBatchOperations#insert(java.lang.Object) + * @see org.springframework.data.cassandra.core.CassandraBatchOperations#insert(Object...) */ @Override - public CassandraBatchOperations insert(Object entity) { - - ensureNotExecuted(); - Assert.notNull(entity, "Entity must not be null"); - - batch.add(cassandraTemplate.createInsertQuery(entity, null)); - return this; + public CassandraBatchOperations insert(Object... entities) { + return insert(nullSafeIterable(entities)); } - /* + /* * (non-Javadoc) * @see org.springframework.data.cassandra.core.CassandraBatchOperations#insert(java.lang.Iterable) */ @Override - public CassandraBatchOperations insert(Iterable entities) { + public CassandraBatchOperations insert(Iterable entities) { - ensureNotExecuted(); - Assert.notNull(entities, "Entities must not be null"); - - for (Object entity : entities) { + assertNotExecuted(); + for (Object entity : nullSafeIterable(entities)) { Assert.notNull(entity, "Entity must not be null"); batch.add(cassandraTemplate.createInsertQuery(entity, null)); } @@ -103,33 +104,25 @@ class CassandraBatchTemplate implements CassandraBatchOperations { return this; } - /* + /* * (non-Javadoc) - * @see org.springframework.data.cassandra.core.CassandraBatchOperations#update(java.lang.Object) + * @see org.springframework.data.cassandra.core.CassandraBatchOperations#update(Object...) */ @Override - public CassandraBatchOperations update(Object entity) { - - ensureNotExecuted(); - Assert.notNull(entity, "Entity must not be null"); - - batch.add(cassandraTemplate.createUpdateQuery(entity, null)); - - return this; + public CassandraBatchOperations update(Object... entities) { + return update(nullSafeIterable(entities)); } - /* + /* * (non-Javadoc) * @see org.springframework.data.cassandra.core.CassandraBatchOperations#update(java.lang.Iterable) */ @Override - public CassandraBatchOperations update(Iterable entities) { + public CassandraBatchOperations update(Iterable entities) { - ensureNotExecuted(); - Assert.notNull(entities, "Entities must not be null"); - - for (Object entity : entities) { + assertNotExecuted(); + for (Object entity : nullSafeIterable(entities)) { Assert.notNull(entity, "Entity must not be null"); batch.add(cassandraTemplate.createUpdateQuery(entity, null)); } @@ -137,19 +130,13 @@ class CassandraBatchTemplate implements CassandraBatchOperations { return this; } - /* + /* * (non-Javadoc) - * @see org.springframework.data.cassandra.core.CassandraBatchOperations#delete(java.lang.Object) + * @see org.springframework.data.cassandra.core.CassandraBatchOperations#delete(Object...) */ @Override - public CassandraBatchOperations delete(Object entity) { - - ensureNotExecuted(); - Assert.notNull(entity, "Entity must not be null"); - - batch.add(cassandraTemplate.createDeleteQuery(entity, null)); - - return this; + public CassandraBatchOperations delete(Object... entities) { + return delete(nullSafeIterable(entities)); } /* @@ -157,13 +144,11 @@ class CassandraBatchTemplate implements CassandraBatchOperations { * @see org.springframework.data.cassandra.core.CassandraBatchOperations#delete(java.lang.Iterable) */ @Override - public CassandraBatchOperations delete(Iterable entities) { + public CassandraBatchOperations delete(Iterable entities) { - ensureNotExecuted(); - Assert.notNull(entities, "Entities must not be null"); - - for (Object entity : entities) { + assertNotExecuted(); + for (Object entity : nullSafeIterable(entities)) { Assert.notNull(entity, "Entity must not be null"); batch.add(cassandraTemplate.createDeleteQuery(entity, null)); } @@ -171,7 +156,15 @@ class CassandraBatchTemplate implements CassandraBatchOperations { return this; } - private void ensureNotExecuted() { + private void assertNotExecuted() { Assert.state(!executed.get(), "This Cassandra Batch was already executed"); } + + private Iterable nullSafeIterable(T... array) { + return (array == null ? Collections.emptyList() : Arrays.asList(array)); + } + + private Iterable nullSafeIterable(Iterable iterable) { + return (iterable != null ? iterable : Collections.emptyList()); + } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraTemplate.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraTemplate.java index 93c02817f..58a9a5e2a 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraTemplate.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraTemplate.java @@ -704,8 +704,9 @@ public class CassandraTemplate extends CqlTemplate implements CassandraOperation protected T doInsert(T entity, WriteOptions options) { Assert.notNull(entity, "Entity must not be null"); - Insert insert = createInsertQuery(entity, options); - execute(insert); + + execute(createInsertQuery(entity, options)); + return entity; } @@ -917,7 +918,7 @@ public class CassandraTemplate extends CqlTemplate implements CassandraOperation return executeAsynchronously(createUpdateQuery(entity, options), queryListener); } - /* + /* * (non-Javadoc) * @see org.springframework.data.cassandra.core.CassandraOperations#batchOps(java.lang.Class) */ diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraBatchTemplateIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraBatchTemplateIntegrationTests.java index bd0768347..577d152eb 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraBatchTemplateIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraBatchTemplateIntegrationTests.java @@ -39,7 +39,7 @@ import com.datastax.driver.core.Row; /** * Integration tests for {@link CassandraBatchTemplate}. - * + * * @author Mark Paluch */ @RunWith(SpringJUnit4ClassRunner.class) @@ -63,7 +63,7 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldInsertEntities() { @@ -75,11 +75,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.insert(walter).insert(mike).execute(); Group loaded = cassandraTemplate.selectOneById(Group.class, walter.getId()); + assertThat(loaded.getId().getUsername(), is(equalTo(walter.getId().getUsername()))); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldInsertCollectionOfEntities() { @@ -91,11 +92,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.insert(Arrays.asList(walter, mike)).execute(); Group loaded = cassandraTemplate.selectOneById(Group.class, walter.getId()); + assertThat(loaded.getId().getUsername(), is(equalTo(walter.getId().getUsername()))); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldUpdateEntities() { @@ -110,11 +112,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.update(walter).update(mike).execute(); Group loaded = cassandraTemplate.selectOneById(Group.class, walter.getId()); + assertThat(loaded.getEmail(), is(equalTo(walter.getEmail()))); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldUpdateCollectionOfEntities() { @@ -129,11 +132,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.update(Arrays.asList(walter, mike)).execute(); Group loaded = cassandraTemplate.selectOneById(Group.class, walter.getId()); + assertThat(loaded.getEmail(), is(equalTo(walter.getEmail()))); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldUpdatesCollectionOfEntities() { @@ -148,11 +152,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.update(Arrays.asList(walter, mike)).execute(); FlatGroup loaded = cassandraTemplate.selectOneById(FlatGroup.class, walter); + assertThat(loaded.getEmail(), is(equalTo(walter.getEmail()))); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldDeleteEntities() { @@ -165,11 +170,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.delete(walter).delete(mike).execute(); Group loaded = cassandraTemplate.selectOneById(Group.class, walter.getId()); + assertThat(loaded, is(nullValue())); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldDeleteCollectionOfEntities() { @@ -182,11 +188,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.delete(Arrays.asList(walter, mike)).execute(); Group loaded = cassandraTemplate.selectOneById(Group.class, walter.getId()); + assertThat(loaded, is(nullValue())); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void shouldApplyTimestampToAllEntities() { @@ -205,13 +212,14 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm ResultSet resultSet = cassandraTemplate.query("SELECT writetime(email) FROM group;"); assertThat(resultSet.getAvailableWithoutFetching(), is(2)); + for (Row row : resultSet) { assertThat(row.getLong(0), is(timestamp)); } } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test(expected = IllegalStateException.class) public void shouldNotExecuteTwice() { @@ -220,11 +228,12 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.insert(new Group(new GroupKey("users", "0x1", "walter"))).execute(); batchOperations.execute(); + fail("Missing IllegalStateException"); } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test(expected = IllegalStateException.class) public void shouldNotAllowModificationAfterExecution() { @@ -233,6 +242,7 @@ public class CassandraBatchTemplateIntegrationTests extends AbstractSpringDataEm batchOperations.insert(new Group(new GroupKey("users", "0x1", "walter"))).execute(); batchOperations.update(new Group()); + fail("Missing IllegalStateException"); } } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateUnitTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateUnitTests.java index 458863e86..f394b4eae 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateUnitTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateUnitTests.java @@ -172,11 +172,10 @@ public class CassandraTemplateUnitTests { } /** - * @see DATACASS-288 + * @see DATACASS-288 */ @Test public void batchOperationsShouldCallSession() { - template.batchOps().insert(new Book()).execute(); verify(mockSession).execute(Mockito.any(Batch.class));