Liquibase extension #6334

andrejpetras · 2019-12-23T19:51:42Z

Liquibase extension

Add the liquibase-core and snakeyml (it is used for liquibase json, yaml format) dependencies.
Because Liquibase object requires a JDBC connection I created a QLiquibase wrapper with datasource instance and common liquibase methods to open and close connection after each liquibase method.
Multiple datasources (Datasoruce annotation) are supported (like flyway)
Integration tests are for xml, json, yaml and sql format
Liquibase has own implementation of the class-path scanner to load the Services (Interface implementation). For this I generated txt files (list of implementation classes) at build time which are used in the native run time.

I am really not sure if the QLiquibase (wrapper class) is the right name.

Update:
I renamed the QLiquibase class in Liquibase
Add the org.osgi dependency

gsmet

Woot! Thanks for this work, very cool.

I added some suggestions and questions.

extensions/liquibase/deployment/pom.xml

...iquibase/deployment/src/main/java/io/quarkus/liquibase/LiquibaseDatasourceBeanGenerator.java

extensions/liquibase/deployment/src/main/java/io/quarkus/liquibase/LiquibaseProcessor.java

extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/runtime/LiquibaseConfig.java

...quibase/runtime/src/main/java/io/quarkus/liquibase/runtime/graal/LiquibaseServiceLoader.java

extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/runtime/graal/SubstituteKp.java

...ibase/runtime/src/main/java/io/quarkus/liquibase/runtime/graal/SubstituteServiceLocator.java

integration-tests/pom.xml

gastaldi · 2019-12-30T16:07:26Z

@andrejpetras looks like you need to rebase this branch, as it has some unrelated commits. Here are the steps in case you need some help:

git checkout master
git pull
git checkout 5957-liquibase-extension
git rebase master
git push -f

andrejpetras · 2020-01-01T14:01:26Z

@andrejpetras looks like you need to rebase this branch, as it has some unrelated commits. Here are the steps in case you need some help:

git checkout master

git pull

git checkout 5957-liquibase-extension

git rebase master

git push -f

@gastaldi thanks for the hint. I will do that.

gastaldi · 2020-01-07T15:23:14Z

Looks like it needs to be rebased again :)

extensions/liquibase/deployment/src/main/java/io/quarkus/liquibase/LiquibaseProcessor.java

gsmet

I added a couple of small questions and remarks.

I'm a bit worried about the instantiation of a Liquibase instance for each operation. I think that's the one remaining issue but I'm not sure what we can do about it.

bom/runtime/pom.xml

extensions/liquibase/deployment/pom.xml

gsmet · 2020-01-07T22:18:07Z

extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/Liquibase.java

+        config.currentDateTimeFunction.ifPresent(database::setCurrentDateTimeFunction);
+        config.liquibaseCatalogName.ifPresent(database::setLiquibaseCatalogName);
+        config.liquibaseSchemaName.ifPresent(database::setLiquibaseSchemaName);
+        config.liquibaseTablespaceName.ifPresent(database::setLiquibaseTablespaceName);


Is it a good idea to create a Liquibase instance for each operation?

I'm not familiar with Liquibase but it doesn't seem right to me. I'm not sure I see another solution though as I suppose we need to close it somehow but I thought I should raise the issue.

@gsmet I am not sure but I found another option which could be the one

I am using the data-source driver to create a liquibase database and liquibase instance is created only once.

Then we have in each Liquibase bean method this logic:
Open JDBC connection
Execute liquibase.Liquibase method
Close JDBC connection

https://github.com/andrejpetras/quarkus/blob/de19e6e3de27ce9ede78069addce066dfb8c483e/extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/Liquibase.java#L65

Like hibernate-orm or quartz extension I have guess method for the liquibase database:
https://github.com/andrejpetras/quarkus/blob/de19e6e3de27ce9ede78069addce066dfb8c483e/extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/Liquibase.java#L105

I am not sure if it is possible to move this part in the LiquibaseProcessor but I found this:

quarkus/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java

Line 113 in f8ccf73

dataSourceDriver.produce(new DataSourceDriverBuildItem(agroalBuildTimeConfig.defaultDataSource.driver.get()));

It looks like DataSourceDriverBuildItem currently supports only a default datasource

Commit:
andrejpetras@de19e6e

@gsmet could you please check the solution in this commit? andrejpetras@de19e6e
Looks like this is the last open point in this PR

Only the JDBC connection is created for each method execution

Let's not mark conversation as resolved if we are discussing it :). Sorry, I was away for a bit, will have a look at your commit.

That looks cool

Should I merge it into PR?

@gsmet please have a look at the solution.

PR update for liquibase 3.8.7 version

@Inject LiquibaseFactory liquibaseFactory; try (Liquibase liquibase = liquibaseFactory.createLiquibase()) { liquibase.update(liquibaseFactory.createContexts(),liquibaseFactory.createLabels()); }

The liquibase version 3.8.7 will be release in the next few weeks
Liquibase PR: liquibase/liquibase#987

extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/Liquibase.java

extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/runtime/graal/SubstituteKp.java

...ibase/runtime/src/main/java/io/quarkus/liquibase/runtime/graal/SubstituteServiceLocator.java

andrejpetras · 2020-01-09T08:13:21Z

@gsmet @gastaldi

How can I fix the problem with an Azure job?
Error: The job running on agent Azure Pipelines 12 ran longer than the maximum time of 30 minutes.

gastaldi · 2020-01-09T08:31:21Z

I usually.hit the "Re-run failed checks" button in the Checks tab. You probably need special permissions if that is not available for you. Anyway I triggered that for you in this PR

andrejpetras · 2020-01-09T09:02:00Z

I usually.hit the "Re-run failed checks" button in the Checks tab. You probably need special permissions if that is not available for you. Anyway I triggered that for you in this PR

Oh, thank you.

andrejpetras · 2020-01-15T14:05:20Z

@gastaldi @gsmet Can I also create the documentation page?

gastaldi · 2020-01-15T14:07:53Z

@andrejpetras yes please, that would be great

andrejpetras · 2020-01-15T14:08:59Z

@gastaldi should I create separate PR for it?

gastaldi · 2020-01-15T14:10:05Z

No, I'd prefer if you added to this PR

andrejpetras · 2020-01-17T16:49:09Z

@gsmet @gastaldi
I added the documentation page.
The Liquibase object is now created only once, and only the JDBC connection is created for each Liquibase method.

Could you please review this PR again.

docs/src/main/asciidoc/liquibase.adoc

andrejpetras · 2020-01-20T14:07:34Z

@gsmet @gastaldi do we need to fix any other task or issue in this PR?

docs/src/main/asciidoc/liquibase.adoc

gastaldi · 2020-01-20T14:15:33Z

...base/deployment/src/test/java/io/quarkus/liquibase/test/LiquibaseExtensionConfigFixture.java

+ * This fixture provides access to read the expected and the actual configuration of liquibase.
+ * It also provides a method combining all assertions to be reused for multiple tests.
+ */
+@Disabled


Why this class needs a Disabled annotation?

It's copy & paste from the Flyway extension. My understanding is that we do not use this class as a test class, but only as a test help class.

@gastaldi should I remove it?

If it is not necessary I believe it's better to remove it 😉

gastaldi · 2020-01-20T14:45:08Z

docs/src/main/asciidoc/liquibase.adoc

+}
+--
+
+<1> Inject the Liquibase object if you want to use it directly


Better move the deep technical explanation here

docs/src/main/asciidoc/liquibase.adoc

gsmet

@andrejpetras I pushed a rebase with an upgrade to 3.8.7.

I added a small question about the osgi dependency. Could you have a look?

gsmet · 2020-02-26T22:02:03Z

bom/runtime/pom.xml

+            <dependency>
+                <groupId>org.osgi</groupId>
+                <artifactId>org.osgi.core</artifactId>
+                <version>${osgi.version}</version>


Is this strictly needed or can we use an exclusion instead?

@gsmet cool, thanks

I believe that I add this dependency for native build because of this class:
https://github.com/liquibase/liquibase/blob/9626494719cf3f6e698bcdd76ee97b9eee1ff717/liquibase-core/src/main/java/liquibase/servicelocator/ServiceLocator.java#L7

The OSGI is used in the static block of the class

static { try { Class<?> scanner = Class.forName("Liquibase.ServiceLocator.ClrServiceLocator, Liquibase"); instance = (ServiceLocator) scanner.newInstance(); } catch (Exception e) { try { if (OSGiUtil.isLiquibaseLoadedAsOSGiBundle()) { Bundle liquibaseBundle = FrameworkUtil.getBundle(ServiceLocator.class); instance = new ServiceLocator(new OSGiPackageScanClassResolver(liquibaseBundle), new OSGiResourceAccessor(liquibaseBundle)); } else { instance = new ServiceLocator(); } } catch (Throwable e1) { LogService.getLog(ServiceLocator.class).severe(LogType.LOG, "Cannot build ServiceLocator", e1); } } }

We are not using this in the quarkus at all.
I have no idea how it is possible to substitute the static block without replacing the whole class for the native build.

@andrejpetras we would need to move that code to a static method and substitute that method. Maybe you can prepare another PR for Liquibase?

Oh wait silly me, we would still need the dependency anyway so let's leave it at that.

@gsmet I check the 4.0.x branch of the liquibase and the class does not have osgi packages anymore. We can check the liquibase extension again after liquibase release 4.0.0 version. (This will happen in few months or more ...) But this change should not affect the liquibase extension API for developer. We can change it later in background and clean up the dependencies and also fix the native resources with a regex. We can have new task for it and I can take it to synchronize the liquibase changes in the extension.

gsmet · 2020-02-27T11:02:50Z

Yes, the idea is to merge it today but it's in conflict with my big datasource config PR so I would like to get this one in and then update the Liquibase PR.

Also include the jaxb-deployment dependency to rely on this extension to provide everything related to JAXB/JAXP.

gsmet · 2020-03-06T16:46:22Z

I just force-pushed an update with a migration to the new datasource config stuff and some fixes.

Let's see how it goes with CI.

gsmet · 2020-03-06T18:06:58Z

@andrejpetras I'm going to merge that one and include it in CR2. The code is totally isolated so it's not a problem.

Thanks a lot for your work and your tenacity.

…urceSchemaReadyBuildItem a multiple build item The liquibase extension was merged in quarkusio#6334 which means that we have multiple possibilities of making a DB schema ready. This makes the build item that controls whether DB schemas are ready a mutliple build item so that it can be produced by several extensions. Without this we have the following extensions when we try to cohabite Flyway and Liquibase ``` io.quarkus.builder.ChainBuildException: Multiple producers of item class io.quarkus.agroal.deployment.JdbcDataSourceSchemaReadyBuildItem(io.quarkus.flyway.FlywayProcessor#configureRuntimeProperties) ``` Also avoid the usage of star imports in liquibase integration tests

gsmet requested changes Dec 23, 2019

View reviewed changes

gastaldi added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Dec 30, 2019

gastaldi removed the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Jan 1, 2020

andrejpetras requested a review from gsmet January 6, 2020 18:48

andrejpetras mentioned this pull request Jan 7, 2020

Support for Liquibase #5957

Closed

gastaldi added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Jan 7, 2020

gastaldi reviewed Jan 7, 2020

View reviewed changes

extensions/liquibase/deployment/src/main/java/io/quarkus/liquibase/LiquibaseProcessor.java Outdated Show resolved Hide resolved

gsmet requested changes Jan 7, 2020

View reviewed changes

andrejpetras requested a review from gsmet January 9, 2020 09:02

gastaldi removed the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Jan 9, 2020

gastaldi reviewed Jan 17, 2020

View reviewed changes

docs/src/main/asciidoc/liquibase.adoc Outdated Show resolved Hide resolved

andrejpetras requested a review from gastaldi January 18, 2020 12:15

gastaldi reviewed Jan 20, 2020

View reviewed changes

docs/src/main/asciidoc/liquibase.adoc Outdated Show resolved Hide resolved

gastaldi reviewed Jan 20, 2020

View reviewed changes

andrejpetras mentioned this pull request Jan 27, 2020

Make Liquibase AutoCloseable liquibase/liquibase#978

Merged

boring-cyborg bot added area/core area/dependencies Pull requests that update a dependency file labels Jan 30, 2020

boring-cyborg bot added the area/documentation label Jan 30, 2020

andrejpetras mentioned this pull request Feb 6, 2020

Regex support for the NativeImageResourceBuildItem #7033

Closed

gsmet requested changes Feb 26, 2020

View reviewed changes

gsmet added the release/noteworthy-feature label Feb 27, 2020

Andrej Petras and others added 3 commits March 6, 2020 16:42

Add a Liquibase extension

0503581

Adjust a few things in the Liquibase documentation

bf481f5

Migrate the Liquibase extension to the new datasource config

a99fafe

Also include the jaxb-deployment dependency to rely on this extension to provide everything related to JAXB/JAXP.

boring-cyborg bot added the area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure label Mar 6, 2020

gsmet added the triage/backport? label Mar 6, 2020

gsmet added this to the 1.4.0 milestone Mar 6, 2020

gsmet approved these changes Mar 6, 2020

View reviewed changes

gsmet merged commit 8cda477 into quarkusio:master Mar 6, 2020

machi1990 mentioned this pull request Mar 6, 2020

fix: allow Flyway and Liquibase to play together by making JdbcDataSourceSchemaReadyBuildItem a multiple build item #7658

Merged

gsmet modified the milestones: 1.4.0, 1.3.0.Final, 1.3.0.CR2 Mar 8, 2020

gsmet removed the triage/backport? label Mar 8, 2020

Liquibase extension #6334

Liquibase extension #6334

Conversation

andrejpetras commented Dec 23, 2019 • edited Loading

gsmet left a comment

Choose a reason for hiding this comment

gastaldi commented Dec 30, 2019

andrejpetras commented Jan 1, 2020

gastaldi commented Jan 7, 2020

gsmet 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

andrejpetras Feb 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrejpetras commented Jan 9, 2020

gastaldi commented Jan 9, 2020

andrejpetras commented Jan 9, 2020

andrejpetras commented Jan 15, 2020

gastaldi commented Jan 15, 2020

andrejpetras commented Jan 15, 2020

gastaldi commented Jan 15, 2020

andrejpetras commented Jan 17, 2020 • edited Loading

andrejpetras commented Jan 20, 2020

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

gsmet left a comment • edited Loading

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

gsmet commented Feb 27, 2020

gsmet commented Mar 6, 2020

gsmet commented Mar 6, 2020

andrejpetras commented Dec 23, 2019 •

edited

Loading

andrejpetras Feb 6, 2020 •

edited

Loading

andrejpetras commented Jan 17, 2020 •

edited

Loading

gsmet left a comment •

edited

Loading