From 414bb07ac83426d94205bc7576d5f88bc03f9fc5 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 1 Mar 2017 21:55:57 +0100 Subject: [PATCH] DATACASS-406 - Polishing. Extend year range in copyright headers. Reformat code. Add JavaDoc. Add ticket references to test methods. Remove unused code. Add guard to test to not call UserTypeResolver during type creation. Rename UserDataTypeProvider to DataTypeProvider and the individual constants to reflect what they provide. Original pull request: #100. --- ...assandraPersistentEntitySchemaCreator.java | 11 - .../mapping/BasicCassandraMappingContext.java | 219 +++++++++++------- ...ersistentEntitySchemaCreatorUnitTests.java | 68 ++---- 3 files changed, 159 insertions(+), 139 deletions(-) diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreator.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreator.java index 5b9c0f71c..9b47610d9 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreator.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreator.java @@ -122,7 +122,6 @@ public class CassandraPersistentEntitySchemaCreator { List specifications = new ArrayList<>(); - // TODO is this Set really needed? Set created = new HashSet<>(); for (CassandraPersistentEntity entity : entities) { @@ -144,16 +143,6 @@ public class CassandraPersistentEntitySchemaCreator { return specifications; } - private Map> getEntitiesByTableName(Collection> entities) { - // TODO simplify by using Java 8 Streams API in 2.0.x - Map> byTableName = new HashMap>(); - - for (CassandraPersistentEntity entity : entities) { - byTableName.put(entity.getTableName(), entity); - } - return byTableName; - } - private void visitUserTypes(CassandraPersistentEntity entity, final Set seen) { entity.doWithProperties(new PropertyHandler() { diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/mapping/BasicCassandraMappingContext.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/mapping/BasicCassandraMappingContext.java index e4d3c071a..191f57881 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/mapping/BasicCassandraMappingContext.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/mapping/BasicCassandraMappingContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2016 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. @@ -68,8 +68,7 @@ public class BasicCassandraMappingContext protected ApplicationContext context; - protected CassandraPersistentEntityMetadataVerifier verifier = - new CompositeCassandraPersistentEntityMetadataVerifier(); + protected CassandraPersistentEntityMetadataVerifier verifier = new CompositeCassandraPersistentEntityMetadataVerifier(); protected ClassLoader beanClassLoader; @@ -91,12 +90,13 @@ public class BasicCassandraMappingContext * Create a new {@link BasicCassandraMappingContext}. */ public BasicCassandraMappingContext() { + setCustomConversions(new CustomConversions(Collections.EMPTY_LIST)); setSimpleTypeHolder(CassandraSimpleTypeHolder.HOLDER); } - /** - * @inheritDoc + /* (non-Javadoc) + * @see org.springframework.data.mapping.context.AbstractMappingContext#initialize() */ @Override public void initialize() { @@ -104,45 +104,43 @@ public class BasicCassandraMappingContext processMappingOverrides(); } - /* (non-Javadoc) */ @SuppressWarnings("all") protected void processMappingOverrides() { if (mapping != null) { - mapping.getEntityMappings().stream().filter((entityMapping -> entityMapping != null)) - .forEach(entityMapping -> { - String entityClassName = entityMapping.getEntityClassName(); - try { - Class entityClass = ClassUtils.forName(entityClassName, beanClassLoader); + mapping.getEntityMappings().stream() // + .filter(entityMapping -> entityMapping != null) // + .forEach(entityMapping -> { - CassandraPersistentEntity entity = getPersistentEntity(entityClass); + String entityClassName = entityMapping.getEntityClassName(); - Assert.state(entity != null, - String.format("Unknown persistent entity class name [%s]", entityClassName)); + try { + Class entityClass = ClassUtils.forName(entityClassName, beanClassLoader); - String entityTableName = entityMapping.getTableName(); + CassandraPersistentEntity entity = getPersistentEntity(entityClass); - if (StringUtils.hasText(entityTableName)) { - entity.setTableName(cqlId(entityTableName, Boolean.valueOf(entityMapping.getForceQuote()))); + Assert.state(entity != null, String.format("Unknown persistent entity class name [%s]", entityClassName)); + + String entityTableName = entityMapping.getTableName(); + + if (StringUtils.hasText(entityTableName)) { + entity.setTableName(cqlId(entityTableName, Boolean.valueOf(entityMapping.getForceQuote()))); + } + + processMappingOverrides(entity, entityMapping); + } catch (ClassNotFoundException e) { + throw new IllegalStateException(String.format("Unknown persistent entity name [%s]", entityClassName), e); } - - processMappingOverrides(entity, entityMapping); - } catch (ClassNotFoundException e) { - throw new IllegalStateException( - String.format("Unknown persistent entity name [%s]", entityClassName), e); - } - }); + }); } } - /* (non-Javadoc) */ protected void processMappingOverrides(CassandraPersistentEntity entity, EntityMapping entityMapping) { - entityMapping.getPropertyMappings().forEach( - (key, propertyMapping) -> processMappingOverride(entity, propertyMapping)); + entityMapping.getPropertyMappings() + .forEach((key, propertyMapping) -> processMappingOverride(entity, propertyMapping)); } - /* (non-Javadoc) */ protected void processMappingOverride(CassandraPersistentEntity entity, PropertyMapping mapping) { CassandraPersistentProperty property = entity.getPersistentProperty(mapping.getPropertyName()); @@ -159,8 +157,7 @@ public class BasicCassandraMappingContext } } - /* - * (non-Javadoc) + /* (non-Javadoc) * @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext) */ @Override @@ -221,28 +218,39 @@ public class BasicCassandraMappingContext return verifier; } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getNonPrimaryKeyEntities() + */ @Override public Collection> getNonPrimaryKeyEntities() { return getTableEntities(); } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getPrimaryKeyEntities() + */ @Override public Collection> getPrimaryKeyEntities() { return Collections.unmodifiableSet(primaryKeyEntities); } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getTableEntities() + */ @Override public Collection> getTableEntities() { return Collections.unmodifiableCollection(tableEntities); } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getUserDefinedTypeEntities() + */ @Override public Collection> getUserDefinedTypeEntities() { return Collections.unmodifiableSet(userDefinedTypes); } - /* - * (non-Javadoc) + /* (non-Javadoc) * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getPersistentEntities(boolean) */ @Override @@ -255,11 +263,14 @@ public class BasicCassandraMappingContext return getTableEntities(); } + /* (non-Javadoc) + * @see org.springframework.data.mapping.context.AbstractMappingContext#createPersistentEntity(org.springframework.data.util.TypeInformation) + */ @Override protected CassandraPersistentEntity createPersistentEntity(TypeInformation typeInformation) { - UserDefinedType userDefinedType = AnnotatedElementUtils.findMergedAnnotation( - typeInformation.getType(), UserDefinedType.class); + UserDefinedType userDefinedType = AnnotatedElementUtils.findMergedAnnotation(typeInformation.getType(), + UserDefinedType.class); CassandraPersistentEntity entity; @@ -300,16 +311,17 @@ public class BasicCassandraMappingContext return entity; } + /* (non-Javadoc) + * @see org.springframework.data.mapping.context.AbstractMappingContext#createPersistentProperty(java.lang.reflect.Field, java.beans.PropertyDescriptor, org.springframework.data.mapping.model.MutablePersistentEntity, org.springframework.data.mapping.model.SimpleTypeHolder) + */ @Override public CassandraPersistentProperty createPersistentProperty(Field field, PropertyDescriptor descriptor, - CassandraPersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { - + CassandraPersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { return createPersistentProperty(field, descriptor, owner, (CassandraSimpleTypeHolder) simpleTypeHolder); } public CassandraPersistentProperty createPersistentProperty(Field field, PropertyDescriptor descriptor, - CassandraPersistentEntity owner, CassandraSimpleTypeHolder simpleTypeHolder) { - + CassandraPersistentEntity owner, CassandraSimpleTypeHolder simpleTypeHolder) { return new BasicCassandraPersistentProperty(field, descriptor, owner, simpleTypeHolder, userTypeResolver); } @@ -336,24 +348,24 @@ public class BasicCassandraMappingContext final AtomicBoolean foundReference = new AtomicBoolean(); - getPersistentEntities().forEach(entity -> entity.doWithProperties( - new PropertyHandler() { + getPersistentEntities() + .forEach(entity -> entity.doWithProperties(new PropertyHandler() { - @Override - public void doWithPersistentProperty(CassandraPersistentProperty persistentProperty) { + @Override + public void doWithPersistentProperty(CassandraPersistentProperty persistentProperty) { - CassandraType cassandraType = persistentProperty.findAnnotation(CassandraType.class); + CassandraType cassandraType = persistentProperty.findAnnotation(CassandraType.class); - if (cassandraType == null) { - return; + if (cassandraType == null) { + return; + } + + if (StringUtils.hasText(cassandraType.userTypeName()) + && CqlIdentifier.cqlId(cassandraType.userTypeName()).equals(identifier)) { + foundReference.set(true); + } } - - if (StringUtils.hasText(cassandraType.userTypeName()) - && CqlIdentifier.cqlId(cassandraType.userTypeName()).equals(identifier)) { - foundReference.set(true); - } - } - })); + })); return foundReference.get(); } @@ -369,6 +381,9 @@ public class BasicCassandraMappingContext return false; } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getCreateTableSpecificationFor(org.springframework.data.cassandra.mapping.CassandraPersistentEntity) + */ @Override public CreateTableSpecification getCreateTableSpecificationFor(CassandraPersistentEntity entity) { @@ -388,11 +403,10 @@ public class BasicCassandraMappingContext @Override public void doWithPersistentProperty(CassandraPersistentProperty primaryKeyProperty) { if (primaryKeyProperty.isPartitionKeyColumn()) { - specification.partitionKeyColumn(primaryKeyProperty.getColumnName(), - getDataType(primaryKeyProperty)); + specification.partitionKeyColumn(primaryKeyProperty.getColumnName(), getDataType(primaryKeyProperty)); } else { // it's a cluster column - specification.clusteredKeyColumn(primaryKeyProperty.getColumnName(), - getDataType(primaryKeyProperty), primaryKeyProperty.getPrimaryKeyOrdering()); + specification.clusteredKeyColumn(primaryKeyProperty.getColumnName(), getDataType(primaryKeyProperty), + primaryKeyProperty.getPrimaryKeyOrdering()); } } }); @@ -416,8 +430,7 @@ public class BasicCassandraMappingContext return specification; } - /* - * (non-Javadoc) + /* (non-Javadoc) * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getCreateUserTypeSpecificationFor(org.springframework.data.cassandra.mapping.CassandraPersistentEntity) */ @Override @@ -432,10 +445,10 @@ public class BasicCassandraMappingContext @Override public void doWithPersistentProperty(final CassandraPersistentProperty property) { - specification.field( - property.getColumnName(), - getDataTypeWithUserTypeFactory(property, UserDataTypeProvider.Fake) - ); + // Use frozen literal to not resolve types from Cassandra. + // At this stage, they might be not created yet. + specification.field(property.getColumnName(), + getDataTypeWithUserTypeFactory(property, DataTypeProvider.FrozenLiteral)); } }); @@ -447,8 +460,8 @@ public class BasicCassandraMappingContext } /* (non-Javadoc) - * @see org.springframework.data.mapping.context.AbstractMappingContext#shouldCreatePersistentEntityFor(org.springframework.data.util.TypeInformation) - */ + * @see org.springframework.data.mapping.context.AbstractMappingContext#shouldCreatePersistentEntityFor(org.springframework.data.util.TypeInformation) + */ @Override protected boolean shouldCreatePersistentEntityFor(TypeInformation typeInfo) { return (!customConversions.hasCustomWriteTarget(typeInfo.getType()) @@ -469,11 +482,12 @@ public class BasicCassandraMappingContext */ @Override public DataType getDataType(CassandraPersistentProperty property) { - - return getDataTypeWithUserTypeFactory(property, UserDataTypeProvider.Simple); + return getDataTypeWithUserTypeFactory(property, DataTypeProvider.EntityUserType); } - private DataType getDataTypeWithUserTypeFactory(CassandraPersistentProperty property, UserDataTypeProvider userDataTypeProvider) { + private DataType getDataTypeWithUserTypeFactory(CassandraPersistentProperty property, + DataTypeProvider dataTypeProvider) { + if (property.isCompositePrimaryKey()) { return property.getDataType(); } @@ -486,8 +500,11 @@ public class BasicCassandraMappingContext if (persistentEntity != null && persistentEntity.isUserDefinedType()) { - DataType elementType = getUserDataType(property, userDataTypeProvider, persistentEntity); - if (elementType != null) return elementType; + DataType elementType = getUserDataType(property, dataTypeProvider, persistentEntity); + + if (elementType != null) { + return elementType; + } } if (customConversions.hasCustomWriteTarget(property.getType())) { @@ -495,9 +512,11 @@ public class BasicCassandraMappingContext } if (customConversions.hasCustomWriteTarget(property.getActualType())) { + Class targetType = customConversions.getCustomWriteTarget(property.getActualType()); if (property.isCollectionLike()) { + if (List.class.isAssignableFrom(property.getType())) { return DataType.list(getDataTypeFor(targetType)); } @@ -513,8 +532,10 @@ public class BasicCassandraMappingContext return property.getDataType(); } - private DataType getUserDataType(CassandraPersistentProperty property, UserDataTypeProvider userDataTypeProvider, CassandraPersistentEntity persistentEntity) { - DataType elementType = userDataTypeProvider.get(persistentEntity); + private DataType getUserDataType(CassandraPersistentProperty property, DataTypeProvider dataTypeProvider, + CassandraPersistentEntity persistentEntity) { + + DataType elementType = dataTypeProvider.getDataType(persistentEntity); if (property.isCollectionLike()) { @@ -530,6 +551,7 @@ public class BasicCassandraMappingContext if (!property.isCollectionLike() && !property.isMapLike()) { return elementType; } + return null; } @@ -538,10 +560,13 @@ public class BasicCassandraMappingContext */ @Override public DataType getDataType(Class type) { - return (customConversions.hasCustomWriteTarget(type) - ? getDataTypeFor(customConversions.getCustomWriteTarget(type)) : getDataTypeFor(type)); + return (customConversions.hasCustomWriteTarget(type) ? getDataTypeFor(customConversions.getCustomWriteTarget(type)) + : getDataTypeFor(type)); } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#getExistingPersistentEntity(java.lang.Class) + */ @Override public CassandraPersistentEntity getExistingPersistentEntity(Class type) { @@ -552,45 +577,71 @@ public class BasicCassandraMappingContext return entity; } + /* (non-Javadoc) + * @see org.springframework.data.cassandra.mapping.CassandraMappingContext#contains(java.lang.Class) + */ @Override public boolean contains(Class type) { return entitiesByType.containsKey(type); } - enum UserDataTypeProvider { + /** + * @author Jens Schauder + * @since 1.5.1 + */ + enum DataTypeProvider { + + EntityUserType { - Simple { @Override - public DataType get(CassandraPersistentEntity entity) { + public DataType getDataType(CassandraPersistentEntity entity) { return entity.getUserType(); } }, - Fake { + FrozenLiteral { + @Override - public DataType get(CassandraPersistentEntity entity) { - return new FakeUserType(entity.getTableName()); + public DataType getDataType(CassandraPersistentEntity entity) { + return new FrozenLiteralDataType(entity.getTableName()); } }; - abstract DataType get(CassandraPersistentEntity entity); + /** + * Return the data type for the {@link CassandraPersistentEntity}. + * + * @param entity must not be {@literal null}. + * @return + */ + abstract DataType getDataType(CassandraPersistentEntity entity); } - - static class FakeUserType extends DataType { + /** + * @author Jens Schauder + * @since 1.5.1 + */ + static class FrozenLiteralDataType extends DataType { private final CqlIdentifier type; - protected FakeUserType(CqlIdentifier type) { + protected FrozenLiteralDataType(CqlIdentifier type) { + super(Name.UDT); + this.type = type; } + /* (non-Javadoc) + * @see com.datastax.driver.core.DataType#isFrozen() + */ @Override public boolean isFrozen() { - return false; + return true; } + /* (non-Javadoc) + * @see java.lang.Object#toString() + */ @Override public String toString() { return String.format("frozen<%s>", type.toCql()); diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreatorUnitTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreatorUnitTests.java index 3fc4eb589..1a7d4a8da 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreatorUnitTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraPersistentEntitySchemaCreatorUnitTests.java @@ -17,8 +17,6 @@ package org.springframework.data.cassandra.core; import static org.mockito.Mockito.*; -import lombok.Data; - import java.util.List; import java.util.Set; @@ -35,7 +33,6 @@ import org.springframework.data.cassandra.mapping.BasicCassandraMappingContext; import org.springframework.data.cassandra.mapping.UserDefinedType; import org.springframework.data.cassandra.mapping.UserTypeResolver; -import com.datastax.driver.core.KeyspaceMetadata; import com.datastax.driver.core.UserType; /** @@ -49,9 +46,6 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests { @Mock CassandraAdminOperations adminOperations; @Mock CqlOperations operations; - @Mock KeyspaceMetadata metadata; - @Mock UserType universetype; - @Mock UserType moontype; BasicCassandraMappingContext context = new BasicCassandraMappingContext(); @@ -61,36 +55,34 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests { context.setUserTypeResolver(new UserTypeResolver() { @Override public UserType resolveType(CqlIdentifier typeName) { - return metadata.getUserType(typeName.toCql()); + // make sure that calls to this method pop up. Calling UserTypeResolver while resolving + // to be created user types isn't a good idea because they do not exist at resolution time. + throw new IllegalArgumentException(String.format("Type %s not found", typeName)); } }); when(adminOperations.getCqlOperations()).thenReturn(operations); } - @Test - public void createsCorrectTypeForSimpleTypes(){ + @Test // DATACASS-172, DATACASS-406 + public void createsCorrectTypeForSimpleTypes() { context.getPersistentEntity(MoonType.class); + context.getPersistentEntity(PlanetType.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createUserTypes(false); - verifyTypesGetCreatedInOrderFor( - "universetype", - "moontype" - ); + verifyTypesGetCreatedInOrderFor("universetype", "moontype", "planettype"); } - @Test - public void createsCorrectTypeForSets(){ + @Test // DATACASS-406 + public void createsCorrectTypeForSets() { context.getPersistentEntity(PlanetType.class); - - CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, adminOperations); @@ -98,49 +90,38 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests { verify(operations).execute(matches("CREATE TYPE planettype .* set<.*moontype>.*")); - verifyTypesGetCreatedInOrderFor( - "universetype", - "moontype", - "planettype" - ); + verifyTypesGetCreatedInOrderFor("universetype", "moontype", "planettype"); } - @Test - public void createsCorrectTypeForLists(){ + @Test // DATACASS-406 + public void createsCorrectTypeForLists() { + context.getPersistentEntity(SpaceAgencyType.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createUserTypes(false); verify(operations).execute(matches("CREATE TYPE spaceagencytype .* list<.*astronauttype>.*")); - verifyTypesGetCreatedInOrderFor( - "astronauttype", - "spaceagencytype" - ); - + verifyTypesGetCreatedInOrderFor("astronauttype", "spaceagencytype"); } - @Test - public void createsCorrectTypesForNestedTypes(){ + @Test // DATACASS-406 + public void createsCorrectTypesForNestedTypes() { context.getPersistentEntity(PlanetType.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createUserTypes(false); - verifyTypesGetCreatedInOrderFor( - "universetype", - "moontype", - "planettype" - ); + verifyTypesGetCreatedInOrderFor("universetype", "moontype", "planettype"); } - private void verifyTypesGetCreatedInOrderFor(String ... typenames) { + private void verifyTypesGetCreatedInOrderFor(String... typenames) { InOrder inOrder = Mockito.inOrder(operations); for (String typename : typenames) { @@ -155,7 +136,6 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests { @UserDefinedType static class MoonType { - UniverseType universeType; }