Changes to support generating Liquibase change-sets #1520

kurtn718 · 2023-05-22T12:43:13Z

You have read the Spring Data contribution guidelines.
You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
You submit test cases (unit or integration tests) that back your changes.
You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Initial Data Model for Schema SQL Generation that can generate SQL for a simple entity. Closes #1478

…ween current and previous state

… deletions.

…s own class

kurtn718 · 2023-05-22T13:11:30Z

...nal/src/main/java/org/springframework/data/relational/core/mapping/DerivedSqlIdentifier.java

@@ -32,12 +32,12 @@
 * @author Kurt Niemi
 * @since 2.0
 */
-class DerivedSqlIdentifier implements SqlIdentifier {
+public class DerivedSqlIdentifier implements SqlIdentifier {



Open to alternatives if we don't want to make this public.

The TableModel and ColumnModel classes take in a SQLIdentifier in their constructor - and the LiquibaseChangeSetGenerator class creates a SchemaSQLGenerationModel (which I think I am going to rename/refactor to SchemaModel)

mp911de

Did a first review pass.

mp911de · 2023-05-22T13:29:44Z

spring-data-jdbc/pom.xml

@@ -70,6 +70,17 @@
 			</roles>
 			<timezone>+1</timezone>
 		</developer>
+		<developer>


We should clean up the developer sections as these information are inherited from parent pom's.

So, all of the developers should go into the parent spring-data-relational.pom? (and we remove from the spring-data-jdbc and spring-data-r2dbc) and also make sure that nobody gets deleted :-)

Yes, exactly. I assume that we missed the cleanup when we split modules and added the R2DBC module during the course of our repository rearrangement one or two years ago.

mp911de · 2023-05-22T13:33:28Z

...gframework/data/relational/core/mapping/schemasqlgeneration/LiquibaseChangeSetGenerator.java

+                continue;
+            }
+
+            SqlIdentifier tableName = new DerivedSqlIdentifier(table.getName(), true);


The usage of DerivedSqlIdentifier isn't appropriate. DerivedSqlIdentifier is intended to be used when creating a SQL model from Java classes. In this case, we have already a proper SQL model. I'm also not quite sure whether quoted should be part of the identifier handling as we're working with plain table and column names.

mp911de · 2023-05-22T13:33:50Z

...gframework/data/relational/core/mapping/schemasqlgeneration/LiquibaseChangeSetGenerator.java

+        for (liquibase.structure.core.Table table : tables) {
+
+            // Exclude internal Liquibase tables from comparison
+            if (table.getName().startsWith("DATABASECHANGELOG")) {


Let's have a table filter Predicate to keep things extensible.

mp911de · 2023-05-22T13:35:16Z

...gframework/data/relational/core/mapping/schemasqlgeneration/LiquibaseChangeSetGenerator.java

+        this.targetDatabase = targetDatabase;
+    }
+
+    public void generateLiquibaseChangeset(String changeLogFilePath) throws InvalidExampleException, DatabaseException, IOException, ChangeLogParseException {


Let's use Spring's Resource abstraction instead of Strings. With string values, we can never know, whether these could be URL's, resource paths or file paths.

mp911de · 2023-05-22T13:38:26Z

...n/java/org/springframework/data/relational/core/mapping/schemasqlgeneration/SchemaModel.java

+                setIdentifierColumns.add(p);
+            }
+
+            iter =


By iterating over persistent properties, we should get all properties. We would need to ensure that properties aren't associations or entities. We should not require @Column.

mp911de · 2023-05-22T13:39:17Z

...ain/java/org/springframework/data/relational/core/mapping/schemasqlgeneration/TableDiff.java

+import java.util.ArrayList;
+import java.util.List;
+
+public class TableDiff {


I think we should be able to use Java records for all pure data objects.

Converted all of the pure data objects to use records. It's definitely cleaner - removing the getters/setters.

mp911de · 2023-05-22T14:29:42Z

...n/java/org/springframework/data/relational/core/mapping/schemasqlgeneration/SchemaModel.java

+
+        // Identify deleted tables
+        Set<TableModel> deletedTables = new HashSet<TableModel>(sourceTableData);
+        deletedTables.removeAll(targetTableData);


We need to be careful. Databases may contain tables that are not mapped. If we remove those tables, then we might delete their data. Another safeguard (predicate) would be good, to either have a boolean flag or a predicate to identify the tables, that can be deleted.

We still have the safe-guard in that we don't automatically apply the change set, but having user(s) modify the changeet to remove the deletions every time would be annoying :-) and also error prone.

I like the predicate idea - with the default implementation we don't delete anything, and then it's on the developer if they supply a custom predicate.

@mp911de - Hi, I think I've pushed up changes for everything that you had mentioned.

…SchemaModel

…rform any difference/comparison)

…bles / columns (i.e. only perform additions - unless the user implements a predicate telling us otherwise)

…en querying DB state is the Reference Name.

…bsolute file name

… ones

Reformat code, switch to tabs. Accept property in DatabaseTypeMapping to provide more context to the type mapping component. Rename LiquibaseChangeSetGenerator to …Writer as we're writing a changeset and computing the contents is a consequence of writing a changeset. Refine naming to express what we're actually doing. Introduce setters for enhanced configuration of predicates. Reduce visibility of types to avoid unwanted public API where public access is not needed. Remove usused code, move methods around for improved grouping of code.

Rename package to schema as the schema is being created and updated and not generated. Rename …Model classes to just their name as types are package-private and not visible externally. Refactor SchemaDiff to Java record. Use different overloads to write schema changes to avoid LiquibaseException leaking into cases where no diff is being used. Introduce SchemaFilter to filter unwanted mapped entities. Add tests for changeset-creation.

Move code to JDBC module. Introduce comparator strategy to customize how table and column names are compared.

We now support schema creation and schema migration by generating Liquibase changesets from mapped entities. We also support evolution of schema by comparing existing tables with mapped entities to compute differential changesets. Closes #1478 Original pull request: #1520

Reformat code, switch to tabs. Accept property in DatabaseTypeMapping to provide more context to the type mapping component. Rename LiquibaseChangeSetGenerator to …Writer as we're writing a changeset and computing the contents is a consequence of writing a changeset. Refine naming to express what we're actually doing. Introduce setters for enhanced configuration of predicates. Reduce visibility of types to avoid unwanted public API where public access is not needed. Remove usused code, move methods around for improved grouping of code. Rename package to schema as the schema is being created and updated and not generated. Rename …Model classes to just their name as types are package-private and not visible externally. Refactor SchemaDiff to Java record. Use different overloads to write schema changes to avoid LiquibaseException leaking into cases where no diff is being used. Introduce SchemaFilter to filter unwanted mapped entities. Move code to JDBC module. Introduce comparator strategy to customize how table and column names are compared. See #1478 Original pull request: #1520

See #1478 Original pull request: #1520

mp911de · 2023-06-06T06:50:18Z

That's merged and polished now.

We now support schema creation and schema migration by generating Liquibase changesets from mapped entities. We also support evolution of schema by comparing existing tables with mapped entities to compute differential changesets. Closes #756 Original pull request: #1520

Reformat code, switch to tabs. Accept property in DatabaseTypeMapping to provide more context to the type mapping component. Rename LiquibaseChangeSetGenerator to …Writer as we're writing a changeset and computing the contents is a consequence of writing a changeset. Refine naming to express what we're actually doing. Introduce setters for enhanced configuration of predicates. Reduce visibility of types to avoid unwanted public API where public access is not needed. Remove usused code, move methods around for improved grouping of code. Rename package to schema as the schema is being created and updated and not generated. Rename …Model classes to just their name as types are package-private and not visible externally. Refactor SchemaDiff to Java record. Use different overloads to write schema changes to avoid LiquibaseException leaking into cases where no diff is being used. Introduce SchemaFilter to filter unwanted mapped entities. Move code to JDBC module. Introduce comparator strategy to customize how table and column names are compared. See #756 Original pull request: #1520

See #756 Original pull request: #1520

kurtn718 added 14 commits May 19, 2023 07:10

Initial Data Model for Schema SQL Generation (#1481)

f5fa502

Initial Data Model for Schema SQL Generation that can generate SQL for a simple entity. Closes #1478

Initial changes for SQLGenerationDataModel to perform differences bet…

201ba55

…ween current and previous state

Remove Serializable

e5e993f

Add liquibase as an optional dependency

ebde99f

Initial changes for Generating Liquibase changeset

4d08588

Remove classes not used for generating Liquibase change set

f7a5ead

Make DerivedSqlIdentifier public - as we need to create one

989d4b2

Fix Unit Test

7e21cfc

Generate Liquibase changesets for table additions, modifications, and…

046d916

… deletions.

Re-run new-issue-branch script after rebase

33e3308

Re-add things lost in rebase

2335a4b

cleanup - remove unnecessary changes

a22eccc

Refactoring - move logic for generating Liquibase Change Sets into it…

d2d7845

…s own class

Additional refactoring on LiquibaseChangeSetGenerator

023b34f

kurtn718 requested review from mp911de and schauder May 22, 2023 12:44

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 22, 2023

kurtn718 commented May 22, 2023

View reviewed changes

Refactor rename SchemaSQLGenerationDataModel to SchemaModel

03597b9

mp911de reviewed May 22, 2023

View reviewed changes

kurtn718 added 10 commits May 23, 2023 21:08

Refactor Database type mapping and provide Default implementation to …

86c52f1

…SchemaModel

Add additional Javadocs

c68584b

Additional Javadoc's

c9efa82

Refactoring - make SchemaModel class purely a model class (and not pe…

9757b71

…rform any difference/comparison)

Change to use Records for pure data types

0c01a9c

Use predicate to define all Liquibase specific tables

e3e123d

Add predicate with default implementation to not drop any external ta…

cf21d29

…bles / columns (i.e. only perform additions - unless the user implements a predicate telling us otherwise)

Remove usage of DerivedSqlIdentifier as what we get from Liquibase wh…

c9841c5

…en querying DB state is the Reference Name.

Fix unit tests

1693d24

Changes for not requiring @table annotation

212d07a

kurtn718 added 2 commits May 26, 2023 12:32

For Liquibase changeset generation - Use Spring Resource instead of a…

9749dcf

…bsolute file name

Ensure all author(s) are listed in parent pom - and remove from other…

0ea5833

… ones

mp911de self-requested a review May 30, 2023 14:29

Cleanup - remove un-needed imports (fix build errors)

187e55d

kurtn718 marked this pull request as ready for review May 31, 2023 17:19

mp911de added 2 commits June 1, 2023 11:45

mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 2, 2023

mp911de self-assigned this Jun 2, 2023

mp911de added 2 commits June 5, 2023 15:25

Polishing.

e85ab13

Move code to JDBC module. Introduce comparator strategy to customize how table and column names are compared.

Add documentation.

cd8d041

mp911de added a commit that referenced this pull request Jun 6, 2023

Add documentation.

6c032fb

See #1478 Original pull request: #1520

mp911de added this to the 3.2 M1 (2023.1.0) milestone Jun 6, 2023

mp911de closed this Jun 6, 2023

mp911de deleted the issue-1480 branch June 6, 2023 06:50

mp911de linked an issue Jun 6, 2023 that may be closed by this pull request

Integrating with Liquibase - Generating Changesets #1480

Closed

mp911de added a commit that referenced this pull request Jun 6, 2023

Add documentation.

b35c7cb

See #756 Original pull request: #1520

mp911de linked an issue Jun 6, 2023 that may be closed by this pull request

Schema generation [DATAJDBC-536] #756

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to support generating Liquibase change-sets #1520

Changes to support generating Liquibase change-sets #1520

kurtn718 commented May 22, 2023

kurtn718 May 22, 2023

mp911de left a comment

mp911de May 22, 2023

kurtn718 May 25, 2023

mp911de May 26, 2023

mp911de May 22, 2023

mp911de May 22, 2023

mp911de May 22, 2023

mp911de May 22, 2023

mp911de May 22, 2023

kurtn718 May 25, 2023

mp911de May 22, 2023

kurtn718 May 25, 2023

kurtn718 May 30, 2023

mp911de commented Jun 6, 2023

Changes to support generating Liquibase change-sets #1520

Changes to support generating Liquibase change-sets #1520

Conversation

kurtn718 commented May 22, 2023

Choose a reason for hiding this comment

mp911de left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mp911de commented Jun 6, 2023