From bd4078621f69beae46ab5e7a495f45c4d3ca3f72 Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 11 May 2016 11:47:50 -0700 Subject: [PATCH] DATACASS-219 - Polish. Refactored code based on @Mark Paluch's code review here (https://github.com/spring-projects/spring-data-cassandra/pull/55). (cherry picked from commit 27019f3218636ab7506a4b78015c1109e54a1359) Signed-off-by: John Blum --- .../CassandraCqlClusterFactoryBean.java | 74 +++++++++++++++---- .../CassandraCqlSessionFactoryBean.java | 2 +- ...ssandraCqlSessionFactoryBeanUnitTests.java | 34 ++------- .../core/util/CollectionUtilsUnitTests.java | 8 +- .../config/CassandraClusterFactoryBean.java | 7 +- .../config/CassandraSessionFactoryBean.java | 4 +- .../config/xml/CassandraNamespaceHandler.java | 3 +- .../mapping/BasicCassandraMappingContext.java | 42 +++++++---- .../CassandraSessionFactoryBeanUnitTests.java | 32 ++------ 9 files changed, 111 insertions(+), 95 deletions(-) diff --git a/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlClusterFactoryBean.java b/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlClusterFactoryBean.java index de33f72a0..497b72a00 100644 --- a/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlClusterFactoryBean.java +++ b/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlClusterFactoryBean.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.cassandra.config; import java.util.ArrayList; @@ -32,6 +33,7 @@ import org.springframework.cassandra.core.cql.generator.DropKeyspaceCqlGenerator import org.springframework.cassandra.core.keyspace.CreateKeyspaceSpecification; import org.springframework.cassandra.core.keyspace.DropKeyspaceSpecification; import org.springframework.cassandra.core.keyspace.KeyspaceActionSpecification; +import org.springframework.cassandra.core.util.CollectionUtils; import org.springframework.cassandra.support.CassandraExceptionTranslator; import org.springframework.dao.DataAccessException; import org.springframework.dao.support.PersistenceExceptionTranslator; @@ -57,6 +59,13 @@ import com.datastax.driver.core.policies.RetryPolicy; * @author Matthew T. Adams * @author David Webb * @author Mark Paluch + * @author Kirk Clemens + * @author Jorge Davison + * @author John Blum + * @see org.springframework.beans.factory.InitializingBean + * @see org.springframework.beans.factory.DisposableBean + * @see org.springframework.beans.factory.FactoryBean + * @see com.datastax.driver.core.Cluster */ public class CassandraCqlClusterFactoryBean implements FactoryBean, InitializingBean, DisposableBean, PersistenceExceptionTranslator { @@ -214,12 +223,12 @@ public class CassandraCqlClusterFactoryBean implements FactoryBean, Ini protected void executeSpecsAndScripts(@SuppressWarnings("rawtypes") List specs, List scripts) { Session system = null; - CqlTemplate template = null; try { - if (specs != null) { - system = specs.size() == 0 ? null : cluster.connect(); - template = system == null ? null : new CqlTemplate(system); + if (!CollectionUtils.isEmpty(specs)) { + system = cluster.connect(); + + CqlTemplate template = new CqlTemplate(system); Iterator i = specs.iterator(); while (i.hasNext()) { @@ -232,18 +241,12 @@ public class CassandraCqlClusterFactoryBean implements FactoryBean, Ini } } - if (scripts != null) { + if (!CollectionUtils.isEmpty(scripts)) { + system = (system != null ? system : cluster.connect()); - if (system == null) { - system = scripts.size() == 0 ? null : cluster.connect(); - } - - if (template == null) { - template = system == null ? null : new CqlTemplate(system); - } + CqlTemplate template = new CqlTemplate(system); for (String script : scripts) { - if (log.isDebugEnabled()) { log.debug("executing raw CQL [{}]", script); } @@ -252,7 +255,6 @@ public class CassandraCqlClusterFactoryBean implements FactoryBean, Ini } } } finally { - if (system != null) { system.close(); } @@ -273,54 +275,98 @@ public class CassandraCqlClusterFactoryBean implements FactoryBean, Ini this.contactPoints = contactPoints; } + /** + * Set the port for the contact points. Default is {@code 9042}, see {@link #DEFAULT_PORT}. + */ public void setPort(int port) { this.port = port; } + /** + * Set the {@link CompressionType}. Default is uncompressed. + */ public void setCompressionType(CompressionType compressionType) { this.compressionType = compressionType; } + /** + * Set the {@link PoolingOptions}. + */ public void setPoolingOptions(PoolingOptions poolingOptions) { this.poolingOptions = poolingOptions; } + /** + * Set the {@link SocketOptions} containing low-level socket options. + */ public void setSocketOptions(SocketOptions socketOptions) { this.socketOptions = socketOptions; } + /** + * Set the {@link AuthProvider}. Default is unauthenticated. + */ public void setAuthProvider(AuthProvider authProvider) { this.authProvider = authProvider; } + /** + * Set the {@link LoadBalancingPolicy}. + */ public void setLoadBalancingPolicy(LoadBalancingPolicy loadBalancingPolicy) { this.loadBalancingPolicy = loadBalancingPolicy; } + /** + * Set the {@link ReconnectionPolicy}. + */ public void setReconnectionPolicy(ReconnectionPolicy reconnectionPolicy) { this.reconnectionPolicy = reconnectionPolicy; } + /** + * Set the {@link RetryPolicy}. + */ public void setRetryPolicy(RetryPolicy retryPolicy) { this.retryPolicy = retryPolicy; } + /** + * Set whether metrics are enabled. Default is {@literal true}, see {@link #DEFAULT_METRICS_ENABLED}. + */ public void setMetricsEnabled(boolean metricsEnabled) { this.metricsEnabled = metricsEnabled; } + /** + * Set a {@link List} of {@link CreateKeyspaceSpecification create keyspace specifications} that are executed when + * this factory is {@link #afterPropertiesSet() initialized}. {@link CreateKeyspaceSpecification Create keyspace + * specifications} are executed on a system session with no keyspace set, before executing + * {@link #setStartupScripts(List)}. + */ public void setKeyspaceCreations(List specifications) { this.keyspaceCreations = specifications; } + /** + * Return a {@link List} of {@link CreateKeyspaceSpecification create keyspace specifications}. + */ public List getKeyspaceCreations() { return keyspaceCreations; } + /** + * Set a {@link List} of {@link DropKeyspaceSpecification drop keyspace specifications} that are executed when this + * factory is {@link #destroy() destroyed}. {@link DropKeyspaceSpecification Drop keyspace specifications} are + * executed on a system session with no keyspace set, before executing {@link #setShutdownScripts(List)}. + */ public void setKeyspaceDrops(List specifications) { this.keyspaceDrops = specifications; } + /** + * Reurn the {@link List} of {@link DropKeyspaceSpecification drop keyspace specifications}. + */ public List getKeyspaceDrops() { return keyspaceDrops; } diff --git a/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBean.java b/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBean.java index b41f0c135..1e4f5bcf1 100644 --- a/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBean.java +++ b/spring-cql/src/main/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBean.java @@ -40,7 +40,7 @@ import com.datastax.driver.core.Session; /** * Factory for creating and configuring a Cassandra {@link Session}, which is a thread-safe singleton. * As such, it is sufficient to have one {@link Session} per application and keyspace. - * + * * @author Alex Shvid * @author Matthew T. Adams * @author John Blum diff --git a/spring-cql/src/test/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBeanUnitTests.java b/spring-cql/src/test/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBeanUnitTests.java index b1b66b360..1c0ec18d6 100644 --- a/spring-cql/src/test/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBeanUnitTests.java +++ b/spring-cql/src/test/java/org/springframework/cassandra/config/CassandraCqlSessionFactoryBeanUnitTests.java @@ -16,24 +16,11 @@ package org.springframework.cassandra.config; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.hamcrest.Matchers.sameInstance; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import java.util.ArrayList; import java.util.Arrays; @@ -58,14 +45,8 @@ import com.datastax.driver.core.Session; * and functionality of the {@link CassandraCqlSessionFactoryBean} class. * * @author John Blum - * @see org.junit.Rule - * @see org.junit.Test - * @see org.junit.rules.ExpectedException - * @see org.junit.runner.RunWith - * @see org.mockito.Mock - * @see org.mockito.Mockito - * @see org.mockito.runners.MockitoJUnitRunner * @see org.springframework.cassandra.config.CassandraCqlSessionFactoryBean + * @see , CassandraPersistentProperty> implements - CassandraMappingContext, ApplicationContextAware { +public class BasicCassandraMappingContext extends AbstractMappingContext, CassandraPersistentProperty> + implements CassandraMappingContext, ApplicationContextAware { protected ApplicationContext context; - protected Mapping mapping = new Mapping(); + protected ClassLoader beanClassLoader; + protected CassandraPersistentEntityMetadataVerifier verifier = new BasicCassandraPersistentEntityMetadataVerifier(); + protected Mapping mapping = new Mapping(); + // useful caches + protected Map, CassandraPersistentEntity> entitiesByType = new HashMap, CassandraPersistentEntity>(); protected Map>> entitySetsByTableName = new HashMap>>(); + protected Set> nonPrimaryKeyEntities = new HashSet>(); protected Set> primaryKeyEntities = new HashSet>(); - protected Map, CassandraPersistentEntity> entitiesByType = new HashMap, CassandraPersistentEntity>(); /** * Creates a new {@link BasicCassandraMappingContext}. @@ -75,9 +79,7 @@ public class BasicCassandraMappingContext extends @Override public void initialize() { - super.initialize(); - processMappingOverrides(); } @@ -129,9 +131,11 @@ public class BasicCassandraMappingContext extends // now do some caching of the entity Set> entities = entitySetsByTableName.get(entity.getTableName()); + if (entities == null) { entities = new HashSet>(); } + entities.add(entity); entitySetsByTableName.put(entity.getTableName(), entities); @@ -180,13 +184,13 @@ public class BasicCassandraMappingContext extends if (pkProp.isPartitionKeyColumn()) { spec.partitionKeyColumn(pkProp.getColumnName(), pkProp.getDataType()); } else { // it's a cluster column - spec.clusteredKeyColumn(pkProp.getColumnName(), pkProp.getDataType(), pkProp.getPrimaryKeyOrdering()); + spec.clusteredKeyColumn(pkProp.getColumnName(), pkProp.getDataType(), + pkProp.getPrimaryKeyOrdering()); } } }); } else { - if (prop.isIdProperty() || prop.isPartitionKeyColumn()) { spec.partitionKeyColumn(prop.getColumnName(), prop.getDataType()); } else if (prop.isClusterKeyColumn()) { @@ -226,20 +230,25 @@ public class BasicCassandraMappingContext extends } String entityClassName = entityMapping.getEntityClassName(); + Class entityClass; + try { entityClass = ClassUtils.forName(entityClassName, beanClassLoader); } catch (ClassNotFoundException e) { - throw new IllegalStateException(String.format("unknown persistent entity name [%s]", entityClassName), e); + throw new IllegalStateException(String.format("unknown persistent entity name [%s]", + entityClassName), e); } CassandraPersistentEntity entity = getPersistentEntity(entityClass); if (entity == null) { - throw new IllegalStateException(String.format("unknown persistent entity class name [%s]", entityClassName)); + throw new IllegalStateException(String.format("unknown persistent entity class name [%s]", + entityClassName)); } String tableName = entityMapping.getTableName(); + if (StringUtils.hasText(tableName)) { entity.setTableName(cqlId(tableName, Boolean.valueOf(entityMapping.getForceQuote()))); } @@ -258,13 +267,16 @@ public class BasicCassandraMappingContext extends protected void processMappingOverride(CassandraPersistentEntity entity, PropertyMapping mapping) { CassandraPersistentProperty property = entity.getPersistentProperty(mapping.getPropertyName()); + if (property == null) { throw new IllegalArgumentException(String.format("entity class [%s] has no persistent property named [%s]", - entity.getType().getName(), mapping.getPropertyName())); + entity.getType().getName(), mapping.getPropertyName())); } boolean forceQuote = false; + String value = mapping.getForceQuote(); + if (StringUtils.hasText(value)) { property.setForceQuote(forceQuote = Boolean.valueOf(value)); } @@ -274,7 +286,6 @@ public class BasicCassandraMappingContext extends if (StringUtils.hasText(value)) { property.setColumnName(cqlId(value, forceQuote)); } - } public void setBeanClassLoader(ClassLoader beanClassLoader) { @@ -285,6 +296,7 @@ public class BasicCassandraMappingContext extends public CassandraPersistentEntity getExistingPersistentEntity(Class type) { CassandraPersistentEntity entity = entitiesByType.get(type); + if (entity != null) { return entity; } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/config/CassandraSessionFactoryBeanUnitTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/config/CassandraSessionFactoryBeanUnitTests.java index 91bab047f..ff47a3b42 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/config/CassandraSessionFactoryBeanUnitTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/config/CassandraSessionFactoryBeanUnitTests.java @@ -16,28 +16,14 @@ package org.springframework.data.cassandra.config; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isNull; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; -import static org.springframework.data.cassandra.config.CassandraSessionFactoryBean.DEFAULT_CREATE_IF_NOT_EXISTS; -import static org.springframework.data.cassandra.config.CassandraSessionFactoryBean.DEFAULT_DROP_TABLES; -import static org.springframework.data.cassandra.config.CassandraSessionFactoryBean.DEFAULT_DROP_UNUSED_TABLES; +import static org.mockito.Mockito.*; +import static org.springframework.data.cassandra.config.CassandraSessionFactoryBean.*; import java.util.Collections; import java.util.Map; @@ -68,14 +54,8 @@ import com.datastax.driver.core.TableMetadata; * of the {@link CassandraSessionFactoryBean} class. * * @author John Blum - * @see org.junit.Rule - * @see org.junit.Test - * @see org.junit.rules.ExpectedException - * @see org.junit.runner.RunWith - * @see org.mockito.Mock - * @see org.mockito.Mockito - * @see org.mockito.runners.MockitoJUnitRunner * @see org.springframework.data.cassandra.config.CassandraSessionFactoryBean + * @see