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 daea2d230..a4146e067 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 @@ -19,12 +19,14 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedHashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import org.jetbrains.annotations.NotNull; + import org.springframework.data.cassandra.core.cql.CqlIdentifier; import org.springframework.data.cassandra.core.cql.generator.CreateIndexCqlGenerator; import org.springframework.data.cassandra.core.cql.generator.CreateTableCqlGenerator; @@ -36,6 +38,7 @@ import org.springframework.data.cassandra.core.mapping.BasicCassandraPersistentE import org.springframework.data.cassandra.core.mapping.CassandraMappingContext; import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity; import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty; +import org.springframework.data.util.Streamable; import org.springframework.util.Assert; /** @@ -152,41 +155,89 @@ public class CassandraPersistentEntitySchemaCreator { .collect(Collectors.toMap(CassandraPersistentEntity::getTableName, entity -> entity)); List specifications = new ArrayList<>(); - - Set created = new HashSet<>(); + UserDefinedTypeSet udts = new UserDefinedTypeSet(); entities.forEach(entity -> { - Set seen = new LinkedHashSet<>(); - - seen.add(entity.getTableName()); - visitUserTypes(entity, seen); - - List ordered = new ArrayList<>(seen); - - Collections.reverse(ordered); - - specifications.addAll(ordered - .stream().filter(created::add).map(identifier -> this.mappingContext - .getCreateUserTypeSpecificationFor(byTableName.get(identifier)).ifNotExists(ifNotExists)) - .collect(Collectors.toList())); + udts.add(entity.getTableName()); + visitUserTypes(entity, udts); }); + specifications + .addAll(udts + .stream().map(identifier -> this.mappingContext + .getCreateUserTypeSpecificationFor(byTableName.get(identifier)).ifNotExists(ifNotExists)) + .collect(Collectors.toList())); + return specifications; } - private void visitUserTypes(CassandraPersistentEntity entity, final Set seen) { + private void visitUserTypes(CassandraPersistentEntity entity, UserDefinedTypeSet udts) { for (CassandraPersistentProperty property : entity) { - BasicCassandraPersistentEntity persistentEntity = this.mappingContext.getPersistentEntity(property); + BasicCassandraPersistentEntity propertyType = this.mappingContext.getPersistentEntity(property); - if (persistentEntity == null) { + if (propertyType == null) { continue; } - if (persistentEntity.isUserDefinedType() && seen.add(persistentEntity.getTableName())) { - visitUserTypes(persistentEntity, seen); + if (propertyType.isUserDefinedType()) { + if (udts.add(propertyType.getTableName())) { + visitUserTypes(propertyType, udts); + } else { + udts.updateDependency(entity.getTableName(), propertyType.getTableName()); + } + } + } + } + + /** + * Object to record dependencies and report them in the order of creation. + */ + static class UserDefinedTypeSet implements Streamable { + + private final Set seen = new HashSet<>(); + private final List creationOrder = new ArrayList<>(); + + public boolean add(CqlIdentifier cqlIdentifier) { + + if (seen.add(cqlIdentifier)) { + creationOrder.add(cqlIdentifier); + return true; + } + + return false; + } + + @NotNull + @Override + public Iterator iterator() { + + List reverseCreationOrder = new ArrayList<>(creationOrder); + Collections.reverse(reverseCreationOrder); + + return reverseCreationOrder.iterator(); + } + + /** + * Checks the dependency order. {@code referent} depends on {@code reference} and we need to make sure that + * {@code referent} gets created after {@code reference}. + *

+ * This method therefore updates the dependency order. + * + * @param referent the client of {@code reference}. + * @param reference the dependency required by {@code referent}. + */ + void updateDependency(CqlIdentifier referent, CqlIdentifier reference) { + + int referentIndex = creationOrder.indexOf(referent); + int referenceIndex = creationOrder.indexOf(reference); + + if (referentIndex > referenceIndex) { + + creationOrder.remove(referent); + creationOrder.add(referenceIndex, referent); } } } 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 074fb9d9b..b07441b83 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 @@ -15,9 +15,16 @@ */ package org.springframework.data.cassandra.core; -import static org.mockito.ArgumentMatchers.matches; +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -25,8 +32,14 @@ import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; + +import org.springframework.data.cassandra.core.cql.CqlIdentifier; import org.springframework.data.cassandra.core.cql.CqlOperations; +import org.springframework.data.cassandra.core.cql.keyspace.CreateUserTypeSpecification; +import org.springframework.data.cassandra.core.cql.keyspace.UserTypeNameSpecification; import org.springframework.data.cassandra.core.mapping.CassandraMappingContext; +import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity; +import org.springframework.data.cassandra.core.mapping.UserDefinedType; /** * Unit tests for {@link CassandraPersistentEntitySchemaCreator}. @@ -54,14 +67,43 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests extends CassandraPe when(adminOperations.getCqlOperations()).thenReturn(operations); } + @Test // DATACASS-687 + public void shouldConsiderProperUdtOrdering() { + + List> ordered = new ArrayList<>(Arrays.asList(Udt2.class, Udt1.class, RequiredByAll.class)); + + context = new CassandraMappingContext() { + @Override + public Collection> getUserDefinedTypeEntities() { + return ordered.stream().map(this::getRequiredPersistentEntity).collect(Collectors.toList()); + } + }; + + context.setUserTypeResolver(typeName -> { + // 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)); + }); + + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); + + List userTypeSpecifications = schemaCreator.createUserTypeSpecifications(false); + + List collect = userTypeSpecifications.stream().map(UserTypeNameSpecification::getName) + .collect(Collectors.toList()); + + assertThat(collect).hasSize(3).startsWith(CqlIdentifier.of("requiredbyall")); + } + @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); @@ -73,8 +115,8 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests extends CassandraPe context.getPersistentEntity(PlanetType.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createUserTypes(false); @@ -88,8 +130,8 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests extends CassandraPe context.getPersistentEntity(SpaceAgencyType.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createUserTypes(false); @@ -103,8 +145,8 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests extends CassandraPe context.getPersistentEntity(PlanetType.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createUserTypes(false); @@ -116,8 +158,8 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests extends CassandraPe context.getPersistentEntity(IndexedEntity.class); - CassandraPersistentEntitySchemaCreator schemaCreator = - new CassandraPersistentEntitySchemaCreator(context, adminOperations); + CassandraPersistentEntitySchemaCreator schemaCreator = new CassandraPersistentEntitySchemaCreator(context, + adminOperations); schemaCreator.createIndexes(false); @@ -132,4 +174,25 @@ public class CassandraPersistentEntitySchemaCreatorUnitTests extends CassandraPe inOrder.verify(operations).execute(Mockito.contains("CREATE TYPE " + typename)); } } + + abstract static class AbstractModel { + private RequiredByAll attachments; + } + + @UserDefinedType + static class RequiredByAll { + private String name; + } + + @UserDefinedType + static class Udt1 { + + private RequiredByAll attachment; + } + + @UserDefinedType + static class Udt2 extends AbstractModel { + + private Udt1 u1; + } }