DATACASS-687 - Fix UDT creation order.

The ordering of UDT now considers all UDTs before creating the final creation order to consider dependencies between UDTs that refer to nested UDTs.

Previously, UDT ordering was already built from iteration over individual UDTs so that UDTs that had dependencies on already seen UDTs wheren't considered properly.
This commit is contained in:
Mark Paluch
2019-09-20 11:10:47 +02:00
parent 9b3d8bf0de
commit 15a7f0de0d
2 changed files with 146 additions and 32 deletions

View File

@@ -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<CreateUserTypeSpecification> specifications = new ArrayList<>();
Set<CqlIdentifier> created = new HashSet<>();
UserDefinedTypeSet udts = new UserDefinedTypeSet();
entities.forEach(entity -> {
Set<CqlIdentifier> seen = new LinkedHashSet<>();
seen.add(entity.getTableName());
visitUserTypes(entity, seen);
List<CqlIdentifier> 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<CqlIdentifier> 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<CqlIdentifier> {
private final Set<CqlIdentifier> seen = new HashSet<>();
private final List<CqlIdentifier> creationOrder = new ArrayList<>();
public boolean add(CqlIdentifier cqlIdentifier) {
if (seen.add(cqlIdentifier)) {
creationOrder.add(cqlIdentifier);
return true;
}
return false;
}
@NotNull
@Override
public Iterator<CqlIdentifier> iterator() {
List<CqlIdentifier> 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}.
* <p>
* 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);
}
}
}

View File

@@ -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<Class<?>> ordered = new ArrayList<>(Arrays.asList(Udt2.class, Udt1.class, RequiredByAll.class));
context = new CassandraMappingContext() {
@Override
public Collection<CassandraPersistentEntity<?>> 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<CreateUserTypeSpecification> userTypeSpecifications = schemaCreator.createUserTypeSpecifications(false);
List<CqlIdentifier> 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;
}
}