diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/DefaultSqlTypeMapping.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/DefaultSqlTypeMapping.java index 526ad5a5fa..cda9811f51 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/DefaultSqlTypeMapping.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/DefaultSqlTypeMapping.java @@ -63,4 +63,9 @@ public DefaultSqlTypeMapping() { public String getColumnType(RelationalPersistentProperty property) { return typeMap.get(ClassUtils.resolvePrimitiveIfNecessary(property.getActualType())); } + + @Override + public String getColumnTypeByClass(Class clazz) { + return typeMap.get(clazz); + } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/ForeignKey.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/ForeignKey.java new file mode 100644 index 0000000000..638c78ddac --- /dev/null +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/ForeignKey.java @@ -0,0 +1,30 @@ +package org.springframework.data.jdbc.core.mapping.schema; + +import java.util.List; +import java.util.Objects; + +/** + * Models a Foreign Key for generating SQL for Schema generation. + * + * @author Evgenii Koba + * @since 3.2 + */ +record ForeignKey(String name, String tableName, List columnNames, String referencedTableName, + List referencedColumnNames) { + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + ForeignKey that = (ForeignKey) o; + return Objects.equals(tableName, that.tableName) && Objects.equals(columnNames, that.columnNames) && Objects.equals( + referencedTableName, that.referencedTableName) && Objects.equals(referencedColumnNames, + that.referencedColumnNames); + } + + @Override + public int hashCode() { + return Objects.hash(tableName, columnNames, referencedTableName, referencedColumnNames); + } +} diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriter.java index b935127547..d835c4f186 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriter.java @@ -21,8 +21,10 @@ import liquibase.change.ColumnConfig; import liquibase.change.ConstraintsConfig; import liquibase.change.core.AddColumnChange; +import liquibase.change.core.AddForeignKeyConstraintChange; import liquibase.change.core.CreateTableChange; import liquibase.change.core.DropColumnChange; +import liquibase.change.core.DropForeignKeyConstraintChange; import liquibase.change.core.DropTableChange; import liquibase.changelog.ChangeLogChild; import liquibase.changelog.ChangeLogParameters; @@ -52,6 +54,7 @@ import java.util.Set; import java.util.function.BiPredicate; import java.util.function.Predicate; +import java.util.stream.Collectors; import org.springframework.core.io.Resource; import org.springframework.data.mapping.context.MappingContext; @@ -321,7 +324,7 @@ private ChangeSet createChangeSet(ChangeSetMetadata metadata, SchemaDiff differe private SchemaDiff initial() { Tables mappedEntities = Tables.from(mappingContext.getPersistentEntities().stream().filter(schemaFilter), - sqlTypeMapping, null); + sqlTypeMapping, null, mappingContext); return SchemaDiff.diff(mappedEntities, Tables.empty(), nameComparator); } @@ -329,7 +332,7 @@ private SchemaDiff differenceOf(Database database) throws LiquibaseException { Tables existingTables = getLiquibaseModel(database); Tables mappedEntities = Tables.from(mappingContext.getPersistentEntities().stream().filter(schemaFilter), - sqlTypeMapping, database.getDefaultCatalogName()); + sqlTypeMapping, database.getDefaultCatalogName(), mappingContext); return SchemaDiff.diff(mappedEntities, existingTables, nameComparator); } @@ -362,6 +365,13 @@ private DatabaseChangeLog getDatabaseChangeLog(File changeLogFile, @Nullable Dat private void generateTableAdditionsDeletions(ChangeSet changeSet, SchemaDiff difference) { + for (Table table : difference.tableDeletions()) { + for (ForeignKey foreignKey : table.foreignKeys()) { + DropForeignKeyConstraintChange dropForeignKey = dropForeignKey(foreignKey); + changeSet.addChange(dropForeignKey); + } + } + for (Table table : difference.tableAdditions()) { CreateTableChange newTable = changeTable(table); changeSet.addChange(newTable); @@ -373,12 +383,24 @@ private void generateTableAdditionsDeletions(ChangeSet changeSet, SchemaDiff dif changeSet.addChange(dropTable(table)); } } + + for (Table table : difference.tableAdditions()) { + for (ForeignKey foreignKey : table.foreignKeys()) { + AddForeignKeyConstraintChange addForeignKey = addForeignKey(foreignKey); + changeSet.addChange(addForeignKey); + } + } } private void generateTableModifications(ChangeSet changeSet, SchemaDiff difference) { for (TableDiff table : difference.tableDiffs()) { + for (ForeignKey foreignKey : table.fkToDrop()) { + DropForeignKeyConstraintChange dropForeignKey = dropForeignKey(foreignKey); + changeSet.addChange(dropForeignKey); + } + if (!table.columnsToAdd().isEmpty()) { changeSet.addChange(addColumns(table)); } @@ -388,6 +410,11 @@ private void generateTableModifications(ChangeSet changeSet, SchemaDiff differen if (!deletedColumns.isEmpty()) { changeSet.addChange(dropColumns(table, deletedColumns)); } + + for (ForeignKey foreignKey : table.fkToAdd()) { + AddForeignKeyConstraintChange addForeignKey = addForeignKey(foreignKey); + changeSet.addChange(addForeignKey); + } } } @@ -444,12 +471,27 @@ private Tables getLiquibaseModel(Database targetDatabase) throws LiquibaseExcept tableModel.columns().add(columnModel); } + tableModel.foreignKeys().addAll(extractForeignKeys(table)); + existingTables.add(tableModel); } return new Tables(existingTables); } + private static List extractForeignKeys(liquibase.structure.core.Table table) { + + return table.getOutgoingForeignKeys().stream().map(foreignKey -> { + String tableName = foreignKey.getForeignKeyTable().getName(); + List columnNames = foreignKey.getForeignKeyColumns().stream() + .map(liquibase.structure.core.Column::getName).toList(); + String referencedTableName = foreignKey.getPrimaryKeyTable().getName(); + List referencedColumnNames = foreignKey.getPrimaryKeyColumns().stream() + .map(liquibase.structure.core.Column::getName).toList(); + return new ForeignKey(foreignKey.getName(), tableName, columnNames, referencedTableName, referencedColumnNames); + }).collect(Collectors.toList()); + } + private static AddColumnChange addColumns(TableDiff table) { AddColumnChange addColumnChange = new AddColumnChange(); @@ -532,6 +574,25 @@ private static DropTableChange dropTable(Table table) { return change; } + private static AddForeignKeyConstraintChange addForeignKey(ForeignKey foreignKey) { + + AddForeignKeyConstraintChange change = new AddForeignKeyConstraintChange(); + change.setConstraintName(foreignKey.name()); + change.setBaseTableName(foreignKey.tableName()); + change.setBaseColumnNames(String.join(",", foreignKey.columnNames())); + change.setReferencedTableName(foreignKey.referencedTableName()); + change.setReferencedColumnNames(String.join(",", foreignKey.referencedColumnNames())); + return change; + } + + private static DropForeignKeyConstraintChange dropForeignKey(ForeignKey foreignKey) { + + DropForeignKeyConstraintChange change = new DropForeignKeyConstraintChange(); + change.setConstraintName(foreignKey.name()); + change.setBaseTableName(foreignKey.tableName()); + return change; + } + /** * Metadata for a ChangeSet. */ diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SchemaDiff.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SchemaDiff.java index 079c40dde1..ffa7032af5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SchemaDiff.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SchemaDiff.java @@ -16,6 +16,7 @@ package org.springframework.data.jdbc.core.mapping.schema; import java.util.ArrayList; +import java.util.Collection; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -91,43 +92,40 @@ private static List diffTable(Tables mappedEntities, Map mappedColumns = createMapping(mappedEntity.columns(), Column::name, nameComparator); - mappedEntity.keyColumns().forEach(it -> mappedColumns.put(it.name(), it)); - Map existingColumns = createMapping(existingTable.columns(), Column::name, nameComparator); - existingTable.keyColumns().forEach(it -> existingColumns.put(it.name(), it)); - // Identify deleted columns - Map toDelete = new TreeMap<>(nameComparator); - toDelete.putAll(existingColumns); - mappedColumns.keySet().forEach(toDelete::remove); - - tableDiff.columnsToDrop().addAll(toDelete.values()); - - // Identify added columns - Map addedColumns = new TreeMap<>(nameComparator); - addedColumns.putAll(mappedColumns); - - existingColumns.keySet().forEach(addedColumns::remove); - - // Add columns in order. This order can interleave with existing columns. - for (Column column : mappedEntity.keyColumns()) { - if (addedColumns.containsKey(column.name())) { - tableDiff.columnsToAdd().add(column); - } - } - + tableDiff.columnsToDrop().addAll(findDiffs(mappedColumns, existingColumns, nameComparator)); + // Identify added columns and add columns in order. This order can interleave with existing columns. + Collection addedColumns = findDiffs(existingColumns, mappedColumns, nameComparator); for (Column column : mappedEntity.columns()) { - if (addedColumns.containsKey(column.name())) { + if (addedColumns.contains(column)) { tableDiff.columnsToAdd().add(column); } } + Map mappedForeignKeys = createMapping(mappedEntity.foreignKeys(), ForeignKey::name, + nameComparator); + Map existingForeignKeys = createMapping(existingTable.foreignKeys(), ForeignKey::name, + nameComparator); + // Identify deleted foreign keys + tableDiff.fkToDrop().addAll(findDiffs(mappedForeignKeys, existingForeignKeys, nameComparator)); + // Identify added foreign keys + tableDiff.fkToAdd().addAll(findDiffs(existingForeignKeys, mappedForeignKeys, nameComparator)); + tableDiffs.add(tableDiff); } return tableDiffs; } + private static Collection findDiffs(Map baseMapping, Map toCompareMapping, + Comparator nameComparator) { + Map diff = new TreeMap<>(nameComparator); + diff.putAll(toCompareMapping); + baseMapping.keySet().forEach(diff::remove); + return diff.values(); + } + private static SortedMap createMapping(List items, Function keyFunction, Comparator nameComparator) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SqlTypeMapping.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SqlTypeMapping.java index d66f932ca4..982d739945 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SqlTypeMapping.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/SqlTypeMapping.java @@ -40,6 +40,18 @@ public interface SqlTypeMapping { @Nullable String getColumnType(RelationalPersistentProperty property); + /** + * Determines a column type for Class. + * + * @param clazz class for which the type should be determined. + * @return the SQL type to use, such as {@code VARCHAR} or {@code NUMERIC}. Can be {@literal null} if the strategy + * cannot provide a column type. + */ + @Nullable + default String getColumnTypeByClass(Class clazz) { + return null; + } + /** * Returns the required column type for a persistent property or throws {@link IllegalArgumentException} if the type * cannot be determined. diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Table.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Table.java index 43b465d9a7..908d87be9a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Table.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Table.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; import org.springframework.lang.Nullable; import org.springframework.util.ObjectUtils; @@ -27,7 +28,7 @@ * @author Kurt Niemi * @since 3.2 */ -record Table(@Nullable String schema, String name, List keyColumns, List columns) { +record Table(@Nullable String schema, String name, List columns, List foreignKeys) { public Table(@Nullable String schema, String name) { this(schema, name, new ArrayList<>(), new ArrayList<>()); @@ -37,6 +38,10 @@ public Table(String name) { this(null, name); } + public List getIdColumns() { + return columns().stream().filter(Column::identity).collect(Collectors.toList()); + } + @Override public boolean equals(Object o) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/TableDiff.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/TableDiff.java index 5ff5e01e71..189a9edd9d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/TableDiff.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/TableDiff.java @@ -25,10 +25,11 @@ * @author Kurt Niemi * @since 3.2 */ -record TableDiff(Table table, List columnsToAdd, List columnsToDrop) { +record TableDiff(Table table, List columnsToAdd, List columnsToDrop, List fkToAdd, + List fkToDrop) { public TableDiff(Table table) { - this(table, new ArrayList<>(), new ArrayList<>()); + this(table, new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Tables.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Tables.java index 12a35ce535..2a9421d49a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Tables.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/schema/Tables.java @@ -15,17 +15,24 @@ */ package org.springframework.data.jdbc.core.mapping.schema; +import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; - import org.springframework.data.annotation.Id; +import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.relational.core.mapping.MappedCollection; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.lang.Nullable; /** @@ -37,15 +44,16 @@ record Tables(List tables) { public static Tables from(RelationalMappingContext context) { - return from(context.getPersistentEntities().stream(), new DefaultSqlTypeMapping(), null); + return from(context.getPersistentEntities().stream(), new DefaultSqlTypeMapping(), null, context); } - // TODO: Add support (i.e. create tickets) to support mapped collections, entities, embedded properties, and aggregate - // references. + // TODO: Add support (i.e. create tickets) to support entities, embedded properties, and aggregate references. public static Tables from(Stream> persistentEntities, - SqlTypeMapping sqlTypeMapping, @Nullable String defaultSchema) { + SqlTypeMapping sqlTypeMapping, @Nullable String defaultSchema, + MappingContext, ? extends RelationalPersistentProperty> context) { + List foreignKeyMetadataList = new ArrayList<>(); List
tables = persistentEntities .filter(it -> it.isAnnotationPresent(org.springframework.data.relational.core.mapping.Table.class)) // .map(entity -> { @@ -58,22 +66,146 @@ public static Tables from(Stream> persis for (RelationalPersistentProperty property : entity) { if (property.isEntity() && !property.isEmbedded()) { + foreignKeyMetadataList.add(createForeignKeyMetadata(entity, property, context, sqlTypeMapping)); continue; } - String columnType = sqlTypeMapping.getRequiredColumnType(property); - Column column = new Column(property.getColumnName().getReference(), sqlTypeMapping.getColumnType(property), - sqlTypeMapping.isNullable(property), identifierColumns.contains(property)); + sqlTypeMapping.isNullable(property), identifierColumns.contains(property)); table.columns().add(column); } return table; }).collect(Collectors.toList()); + applyForeignKeyMetadata(tables, foreignKeyMetadataList); + return new Tables(tables); } public static Tables empty() { return new Tables(Collections.emptyList()); } + + /** + * Apply all information we know about foreign keys to correctly create foreign and primary keys + */ + private static void applyForeignKeyMetadata(List
tables, List foreignKeyMetadataList) { + + foreignKeyMetadataList.forEach(foreignKeyMetadata -> { + Table table = tables.stream().filter(t -> t.name().equals(foreignKeyMetadata.tableName)).findAny().get(); + + List parentIdColumns = collectParentIdentityColumns(foreignKeyMetadata, foreignKeyMetadataList, tables); + List parentIdColumnNames = parentIdColumns.stream().map(Column::name).toList(); + + String foreignKeyName = getForeignKeyName(foreignKeyMetadata.parentTableName, parentIdColumnNames); + if(parentIdColumnNames.size() == 1) { + addIfAbsent(table.columns(), new Column(foreignKeyMetadata.referencingColumnName(), parentIdColumns.get(0).type(), + false, table.getIdColumns().isEmpty())); + if(foreignKeyMetadata.keyColumnName() != null) { + addIfAbsent(table.columns(), new Column(foreignKeyMetadata.keyColumnName(), foreignKeyMetadata.keyColumnType(), + false, true)); + } + addIfAbsent(table.foreignKeys(), new ForeignKey(foreignKeyName, foreignKeyMetadata.tableName(), + List.of(foreignKeyMetadata.referencingColumnName()), foreignKeyMetadata.parentTableName(), parentIdColumnNames)); + } else { + addIfAbsent(table.columns(), parentIdColumns.toArray(new Column[0])); + addIfAbsent(table.columns(), new Column(foreignKeyMetadata.keyColumnName(), foreignKeyMetadata.keyColumnType(), + false, true)); + addIfAbsent(table.foreignKeys(), new ForeignKey(foreignKeyName, foreignKeyMetadata.tableName(), parentIdColumnNames, + foreignKeyMetadata.parentTableName(), parentIdColumnNames)); + } + + }); + } + + private static void addIfAbsent(List list, E... elements) { + for(E element : elements) { + if (!list.contains(element)) { + list.add(element); + } + } + } + + private static List collectParentIdentityColumns(ForeignKeyMetadata child, + List foreignKeyMetadataList, List
tables) { + return collectParentIdentityColumns(child, foreignKeyMetadataList, tables, new HashSet<>()); + } + + private static List collectParentIdentityColumns(ForeignKeyMetadata child, + List foreignKeyMetadataList, List
tables, Set excludeTables) { + + excludeTables.add(child.tableName()); + + Table parentTable = findTableByName(tables, child.parentTableName()); + ForeignKeyMetadata parentMetadata = findMetadataByTableName(foreignKeyMetadataList, child.parentTableName(), excludeTables); + List parentIdColumns = parentTable.getIdColumns(); + + if (!parentIdColumns.isEmpty()) { + return new ArrayList<>(parentIdColumns); + } else if(parentMetadata == null) { + //mustn't happen, probably wrong entity declaration + return new ArrayList<>(); + } else { + List parentParentIdColumns = collectParentIdentityColumns(parentMetadata, foreignKeyMetadataList, tables); + if (parentParentIdColumns.size() == 1) { + Column parentParentIdColumn = parentParentIdColumns.get(0); + Column withChangedName = new Column(parentMetadata.referencingColumnName, parentParentIdColumn.type(), false, true); + parentParentIdColumns = new LinkedList<>(List.of(withChangedName)); + } + if (parentMetadata.keyColumnName() != null) { + parentParentIdColumns.add(new Column(parentMetadata.keyColumnName(), parentMetadata.keyColumnType(), false, true)); + } + return parentParentIdColumns; + } + } + + @Nullable + private static Table findTableByName(List
tables, String tableName) { + return tables.stream().filter(table -> table.name().equals(tableName)).findAny().orElse(null); + } + + @Nullable + private static ForeignKeyMetadata findMetadataByTableName(List metadata, String tableName, + Set excludeTables) { + return metadata.stream() + .filter(m -> m.tableName().equals(tableName) && !excludeTables.contains(m.parentTableName())) + .findAny() + .orElse(null); + } + + private static ForeignKeyMetadata createForeignKeyMetadata( + RelationalPersistentEntity entity, + RelationalPersistentProperty property, + MappingContext, ? extends RelationalPersistentProperty> context, + SqlTypeMapping sqlTypeMapping) { + + RelationalPersistentEntity childEntity = context.getRequiredPersistentEntity(property.getActualType()); + + String referencedKeyColumnType = null; + if(property.isAnnotationPresent(MappedCollection.class)) { + if (property.getType() == List.class) { + referencedKeyColumnType = sqlTypeMapping.getColumnTypeByClass(Integer.class); + } else if (property.getType() == Map.class) { + referencedKeyColumnType = sqlTypeMapping.getColumnTypeByClass(property.getComponentType()); + } + } + + return new ForeignKeyMetadata( + childEntity.getTableName().getReference(), + property.getReverseColumnName(entity).getReference(), + Optional.ofNullable(property.getKeyColumn()).map(SqlIdentifier::getReference).orElse(null), + referencedKeyColumnType, + entity.getTableName().getReference() + ); + } + + //TODO should we place it in BasicRelationalPersistentProperty/BasicRelationalPersistentEntity and generate using NamingStrategy? + private static String getForeignKeyName(String referencedTableName, List referencedColumnNames) { + return String.format("%s_%s_fk", referencedTableName, String.join("_", referencedColumnNames)); + } + + private record ForeignKeyMetadata(String tableName, String referencingColumnName, @Nullable String keyColumnName, + @Nullable String keyColumnType, String parentTableName) { + + } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterIntegrationTests.java index d27e59a37e..805cf85256 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterIntegrationTests.java @@ -20,7 +20,9 @@ import liquibase.change.AddColumnConfig; import liquibase.change.ColumnConfig; import liquibase.change.core.AddColumnChange; +import liquibase.change.core.AddForeignKeyConstraintChange; import liquibase.change.core.DropColumnChange; +import liquibase.change.core.DropForeignKeyConstraintChange; import liquibase.change.core.DropTableChange; import liquibase.changelog.ChangeSet; import liquibase.changelog.DatabaseChangeLog; @@ -41,6 +43,7 @@ import org.springframework.core.io.FileSystemResource; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.schema.LiquibaseChangeSetWriter.ChangeSetMetadata; +import org.springframework.data.relational.core.mapping.MappedCollection; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.Table; import org.springframework.data.util.Predicates; @@ -202,6 +205,93 @@ void shouldAppendToChangeLog(@TempDir File tempDir) { }); } + @Test // GH-1599 + void dropAndCreateTableWithRightOrderOfFkChanges() { + + withEmbeddedDatabase("drop-and-create-table-with-fk.sql", c -> { + + H2Database h2Database = new H2Database(); + h2Database.setConnection(new JdbcConnection(c)); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(contextOf(GroupOfPersons.class)); + writer.setDropTableFilter(Predicates.isTrue()); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), h2Database, new DatabaseChangeLog()); + + assertThat(changeSet.getChanges()).hasSize(4); + assertThat(changeSet.getChanges().get(0)).isInstanceOf(DropForeignKeyConstraintChange.class); + assertThat(changeSet.getChanges().get(3)).isInstanceOf(AddForeignKeyConstraintChange.class); + + DropForeignKeyConstraintChange dropForeignKey = (DropForeignKeyConstraintChange) changeSet.getChanges().get(0); + assertThat(dropForeignKey.getConstraintName()).isEqualToIgnoringCase("fk_to_drop"); + assertThat(dropForeignKey.getBaseTableName()).isEqualToIgnoringCase("table_to_drop"); + + AddForeignKeyConstraintChange addForeignKey = (AddForeignKeyConstraintChange) changeSet.getChanges().get(3); + assertThat(addForeignKey.getBaseTableName()).isEqualToIgnoringCase("person"); + assertThat(addForeignKey.getBaseColumnNames()).isEqualToIgnoringCase("group_id"); + assertThat(addForeignKey.getReferencedTableName()).isEqualToIgnoringCase("group_of_persons"); + assertThat(addForeignKey.getReferencedColumnNames()).isEqualToIgnoringCase("id"); + }); + } + + @Test // GH-1599 + void dropAndCreateFkInRightOrder() { + + withEmbeddedDatabase("drop-and-create-fk.sql", c -> { + + H2Database h2Database = new H2Database(); + h2Database.setConnection(new JdbcConnection(c)); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(contextOf(GroupOfPersons.class)); + writer.setDropColumnFilter((s, s2) -> true); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), h2Database, new DatabaseChangeLog()); + + assertThat(changeSet.getChanges()).hasSize(3); + assertThat(changeSet.getChanges().get(0)).isInstanceOf(DropForeignKeyConstraintChange.class); + assertThat(changeSet.getChanges().get(2)).isInstanceOf(AddForeignKeyConstraintChange.class); + + DropForeignKeyConstraintChange dropForeignKey = (DropForeignKeyConstraintChange) changeSet.getChanges().get(0); + assertThat(dropForeignKey.getConstraintName()).isEqualToIgnoringCase("fk_to_drop"); + assertThat(dropForeignKey.getBaseTableName()).isEqualToIgnoringCase("person"); + + AddForeignKeyConstraintChange addForeignKey = (AddForeignKeyConstraintChange) changeSet.getChanges().get(2); + assertThat(addForeignKey.getBaseTableName()).isEqualToIgnoringCase("person"); + assertThat(addForeignKey.getBaseColumnNames()).isEqualToIgnoringCase("group_id"); + assertThat(addForeignKey.getReferencedTableName()).isEqualToIgnoringCase("group_of_persons"); + assertThat(addForeignKey.getReferencedColumnNames()).isEqualToIgnoringCase("id"); + }); + } + + @Test // GH-1599 + void fieldForFkWillBeCreated() { + + withEmbeddedDatabase("create-fk-with-field.sql", c -> { + + H2Database h2Database = new H2Database(); + h2Database.setConnection(new JdbcConnection(c)); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(contextOf(GroupOfPersons.class)); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), h2Database, new DatabaseChangeLog()); + + assertThat(changeSet.getChanges()).hasSize(2); + assertThat(changeSet.getChanges().get(0)).isInstanceOf(AddColumnChange.class); + assertThat(changeSet.getChanges().get(1)).isInstanceOf(AddForeignKeyConstraintChange.class); + + AddColumnChange addColumn = (AddColumnChange) changeSet.getChanges().get(0); + assertThat(addColumn.getTableName()).isEqualToIgnoringCase("person"); + assertThat(addColumn.getColumns()).hasSize(1); + assertThat(addColumn.getColumns()).extracting(AddColumnConfig::getName).containsExactly("group_id"); + + AddForeignKeyConstraintChange addForeignKey = (AddForeignKeyConstraintChange) changeSet.getChanges().get(1); + assertThat(addForeignKey.getBaseTableName()).isEqualToIgnoringCase("person"); + assertThat(addForeignKey.getBaseColumnNames()).isEqualToIgnoringCase("group_id"); + assertThat(addForeignKey.getReferencedTableName()).isEqualToIgnoringCase("group_of_persons"); + assertThat(addForeignKey.getReferencedColumnNames()).isEqualToIgnoringCase("id"); + }); + } + RelationalMappingContext contextOf(Class... classes) { RelationalMappingContext context = new RelationalMappingContext(); @@ -246,4 +336,10 @@ static class DifferentPerson { String hello; } + @Table + static class GroupOfPersons { + @Id int id; + @MappedCollection(idColumn = "group_id") Set persons; + } + } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterUnitTests.java index 314bbea8f4..3329dd0f2c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/schema/LiquibaseChangeSetWriterUnitTests.java @@ -15,17 +15,31 @@ */ package org.springframework.data.jdbc.core.mapping.schema; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + import static org.assertj.core.api.Assertions.*; +import java.util.stream.Collectors; +import liquibase.change.Change; import liquibase.change.ColumnConfig; +import liquibase.change.core.AddForeignKeyConstraintChange; import liquibase.change.core.CreateTableChange; import liquibase.changelog.ChangeSet; import liquibase.changelog.DatabaseChangeLog; +import org.assertj.core.groups.Tuple; import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.schema.LiquibaseChangeSetWriter.ChangeSetMetadata; +import org.springframework.data.relational.core.mapping.Column; +import org.springframework.data.relational.core.mapping.MappedCollection; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; +import org.springframework.data.util.Optionals; /** * Unit tests for {@link LiquibaseChangeSetWriter}. @@ -73,6 +87,181 @@ void shouldApplySchemaFilter() { assertThat(createTable.getTableName()).isEqualTo("other_table"); } + @Test // GH-1599 + void createForeignKeyWithNewTable() { + + RelationalMappingContext context = new RelationalMappingContext(); + context.getRequiredPersistentEntity(Tables.class); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), new DatabaseChangeLog()); + + AddForeignKeyConstraintChange addForeignKey = (AddForeignKeyConstraintChange) changeSet.getChanges().get(2); + + assertThat(addForeignKey.getBaseTableName()).isEqualTo("other_table"); + assertThat(addForeignKey.getBaseColumnNames()).isEqualTo("tables"); + assertThat(addForeignKey.getReferencedTableName()).isEqualTo("tables"); + assertThat(addForeignKey.getReferencedColumnNames()).isEqualTo("id"); + + } + + @Test // GH-1599 + void fieldForFkShouldNotBeCreatedTwice() { + + RelationalMappingContext context = new RelationalMappingContext(); + context.getRequiredPersistentEntity(DifferentTables.class); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), new DatabaseChangeLog()); + + Optional tableWithFk = changeSet.getChanges().stream().filter(change -> { + return change instanceof CreateTableChange && ((CreateTableChange) change).getTableName() + .equals("table_with_fk_field"); + }).findFirst(); + assertThat(tableWithFk.isPresent()).isEqualTo(true); + + List columns = ((CreateTableChange) tableWithFk.get()).getColumns(); + assertThat(columns).extracting(ColumnConfig::getName).containsExactly("id", "tables_id"); + } + + @Test // GH-1599 + void createForeignKeyForNestedEntities() { + + RelationalMappingContext context = new RelationalMappingContext(); + context.getRequiredPersistentEntity(ListOfMapOfNoIdTables.class); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), new DatabaseChangeLog()); + + assertCreateTable(changeSet, "no_id_table", + Tuple.tuple("field", "VARCHAR(255 BYTE)", null), + Tuple.tuple("list_id", "INT", true), + Tuple.tuple("list_of_map_of_no_id_tables_key", "INT", true), + Tuple.tuple("map_of_no_id_tables_key", "VARCHAR(255 BYTE)", true)); + + assertCreateTable(changeSet, "map_of_no_id_tables", + Tuple.tuple("list_id", "INT", true), + Tuple.tuple("list_of_map_of_no_id_tables_key", "INT", true)); + + assertCreateTable(changeSet, "list_of_map_of_no_id_tables", Tuple.tuple("id", "INT", true)); + + assertAddForeignKey(changeSet, "no_id_table", "list_id,list_of_map_of_no_id_tables_key", + "map_of_no_id_tables", "list_id,list_of_map_of_no_id_tables_key"); + + assertAddForeignKey(changeSet, "map_of_no_id_tables", "list_id", + "list_of_map_of_no_id_tables", "id"); + } + + @Test // GH-1599 + void createForeignKeyForOneToOneWithMultipleChildren() { + + RelationalMappingContext context = new RelationalMappingContext(); + context.getRequiredPersistentEntity(OneToOneLevel1.class); + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), new DatabaseChangeLog()); + + assertCreateTable(changeSet, "other_table", + Tuple.tuple("id", "BIGINT", true), Tuple.tuple("one_to_one_level1", "INT", null)); + + assertCreateTable(changeSet, "one_to_one_level2", Tuple.tuple("one_to_one_level1", "INT", true)); + + assertCreateTable(changeSet, "no_id_table", Tuple.tuple("field", "VARCHAR(255 BYTE)", null), + Tuple.tuple("one_to_one_level2", "INT", true), Tuple.tuple("additional_one_to_one_level2", "INT", null)); + + assertAddForeignKey(changeSet, "other_table", "one_to_one_level1", + "one_to_one_level1", "id"); + + assertAddForeignKey(changeSet, "one_to_one_level2", "one_to_one_level1", + "one_to_one_level1", "id"); + + assertAddForeignKey(changeSet, "no_id_table", "one_to_one_level2", + "one_to_one_level2", "one_to_one_level1"); + + assertAddForeignKey(changeSet, "no_id_table", "additional_one_to_one_level2", + "one_to_one_level2", "one_to_one_level1"); + + } + + @Test // GH-1599 + void createForeignKeyForCircularWithId() { + RelationalMappingContext context = new RelationalMappingContext() { + @Override + public Collection> getPersistentEntities() { + return List.of(getPersistentEntity(CircularWithId.class), getPersistentEntity(ParentOfCircularWithId.class)); + } + }; + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), new DatabaseChangeLog()); + + assertCreateTable(changeSet, "circular_with_id", + Tuple.tuple("id", "INT", true), + Tuple.tuple("circular_with_id", "INT", null), + Tuple.tuple("parent_of_circular_with_id", "INT", null)); + + assertAddForeignKey(changeSet, "circular_with_id", "parent_of_circular_with_id", + "parent_of_circular_with_id", "id"); + + assertAddForeignKey(changeSet, "circular_with_id", "circular_with_id", + "circular_with_id", "id"); + } + + @Test // GH-1599 + void createForeignKeyForCircularNoId() { + RelationalMappingContext context = new RelationalMappingContext() { + @Override + public Collection> getPersistentEntities() { + return List.of(getPersistentEntity(CircularNoId.class), getPersistentEntity(ParentOfCircularNoId.class)); + } + }; + + LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context); + + ChangeSet changeSet = writer.createChangeSet(ChangeSetMetadata.create(), new DatabaseChangeLog()); + + assertCreateTable(changeSet, "circular_no_id", + Tuple.tuple("circular_no_id", "INT", true), + Tuple.tuple("parent_of_circular_no_id", "INT", null)); + + assertAddForeignKey(changeSet, "circular_no_id", "parent_of_circular_no_id", + "parent_of_circular_no_id", "id"); + + assertAddForeignKey(changeSet, "circular_no_id", "circular_no_id", + "circular_no_id", "parent_of_circular_no_id"); + } + + void assertCreateTable(ChangeSet changeSet, String tableName, Tuple... columnTuples) { + Optional createTableOptional = changeSet.getChanges().stream().filter(change -> { + return change instanceof CreateTableChange && ((CreateTableChange) change).getTableName().equals(tableName); + }).findFirst(); + assertThat(createTableOptional.isPresent()).isTrue(); + CreateTableChange createTable = (CreateTableChange) createTableOptional.get(); + assertThat(createTable.getColumns()) + .extracting(ColumnConfig::getName, ColumnConfig::getType, column -> column.getConstraints().isPrimaryKey()) + .containsExactly(columnTuples); + } + + void assertAddForeignKey(ChangeSet changeSet, String baseTableName, String baseColumnNames, String referencedTableName, + String referencedColumnNames) { + Optional addFkOptional = changeSet.getChanges().stream().filter(change -> { + return change instanceof AddForeignKeyConstraintChange + && ((AddForeignKeyConstraintChange) change).getBaseTableName().equals(baseTableName) + && ((AddForeignKeyConstraintChange) change).getBaseColumnNames().equals(baseColumnNames); + }).findFirst(); + assertThat(addFkOptional.isPresent()).isTrue(); + AddForeignKeyConstraintChange addFk = (AddForeignKeyConstraintChange) addFkOptional.get(); + assertThat(addFk.getBaseTableName()).isEqualTo(baseTableName); + assertThat(addFk.getBaseColumnNames()).isEqualTo(baseColumnNames); + assertThat(addFk.getReferencedTableName()).isEqualTo(referencedTableName); + assertThat(addFk.getReferencedColumnNames()).isEqualTo(referencedColumnNames); + } + @org.springframework.data.relational.core.mapping.Table static class VariousTypes { @Id long id; @@ -88,4 +277,88 @@ static class OtherTable { @Id long id; } + @org.springframework.data.relational.core.mapping.Table + static class Tables { + @Id int id; + @MappedCollection + Set tables; + } + + @org.springframework.data.relational.core.mapping.Table + static class SetOfTables { + @Id int id; + @MappedCollection(idColumn = "set_id") + Set setOfTables; + } + + @org.springframework.data.relational.core.mapping.Table + static class DifferentTables { + @Id int id; + @MappedCollection(idColumn = "tables_id") + Set tables; + } + + @org.springframework.data.relational.core.mapping.Table + static class TableWithFkField { + @Id int id; + int tablesId; + } + + @org.springframework.data.relational.core.mapping.Table + static class NoIdTable { + String field; + } + + @org.springframework.data.relational.core.mapping.Table + static class MapOfNoIdTables { + @MappedCollection + Map tables; + } + + @org.springframework.data.relational.core.mapping.Table + static class ListOfMapOfNoIdTables { + @Id int id; + @MappedCollection(idColumn = "list_id") + List listOfTables; + } + + @org.springframework.data.relational.core.mapping.Table + static class OneToOneLevel1 { + @Id int id; + OneToOneLevel2 oneToOneLevel2; + OtherTable otherTable; + } + + @org.springframework.data.relational.core.mapping.Table + static class OneToOneLevel2 { + NoIdTable table1; + @Column("additional_one_to_one_level2") + NoIdTable table2; + } + + @org.springframework.data.relational.core.mapping.Table + static class ParentOfCircularWithId { + @Id int id; + CircularWithId circularWithId; + + } + + @org.springframework.data.relational.core.mapping.Table + static class CircularWithId { + @Id int id; + CircularWithId circularWithId; + } + + @org.springframework.data.relational.core.mapping.Table + static class ParentOfCircularNoId { + @Id int id; + CircularNoId CircularNoId; + + } + + @org.springframework.data.relational.core.mapping.Table + static class CircularNoId { + CircularNoId CircularNoId; + } + } diff --git a/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/create-fk-with-field.sql b/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/create-fk-with-field.sql new file mode 100644 index 0000000000..15b912bed9 --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/create-fk-with-field.sql @@ -0,0 +1,11 @@ +CREATE TABLE group_of_persons +( + id int primary key +); + +CREATE TABLE person +( + id int, + first_name varchar(255), + last_name varchar(255) +); diff --git a/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/drop-and-create-fk.sql b/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/drop-and-create-fk.sql new file mode 100644 index 0000000000..030599f566 --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/drop-and-create-fk.sql @@ -0,0 +1,14 @@ +CREATE TABLE group_of_persons +( + id int primary key +); + +CREATE TABLE person +( + id int, + first_name varchar(255), + last_name varchar(255), + group_id int, + group_id_to_drop int, + constraint fk_to_drop foreign key (group_id_to_drop) references group_of_persons(id) +); diff --git a/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/drop-and-create-table-with-fk.sql b/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/drop-and-create-table-with-fk.sql new file mode 100644 index 0000000000..9a81131180 --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org/springframework/data/jdbc/core/mapping/schema/drop-and-create-table-with-fk.sql @@ -0,0 +1,11 @@ +CREATE TABLE group_of_persons +( + id int primary key +); + +CREATE TABLE table_to_drop +( + id int primary key, + persons int, + constraint fk_to_drop foreign key (persons) references group_of_persons(id) +);