Fix split MongoDB Panache packages #18803

loicmathieu · 2021-07-19T11:10:35Z

Related to #18732

Will fix these warnings:

    "io.quarkus.mongodb.panache" found in [io.quarkus:quarkus-mongodb-panache-common, io.quarkus:quarkus-mongodb-panache]
    "io.quarkus.mongodb.panache.reactive" found in [io.quarkus:quarkus-mongodb-panache-common, io.quarkus:quarkus-mongodb-panache]
    "io.quarkus.mongodb.panache.reactive.runtime" found in [io.quarkus:quarkus-mongodb-panache, io.quarkus:quarkus-mongodb-panache-common]
    "io.quarkus.mongodb.panache.runtime" found in [io.quarkus:quarkus-mongodb-panache-common, io.quarkus:quarkus-mongodb-panache]

loicmathieu · 2021-07-19T11:12:36Z

@gsmet the current impl remove the split packages on the runtime and deployment packages.

To remove the split packages on the io.quarkus.mongodb.panache and io.quarkus.mongodb.panache.reactive packages, I need to duplicate the annotations (easy, already done) and to move some interfaces.
This will create a breaking change on a lot of code as I would need to move the PanacheQuery interface that almost all user of the extension use ! Is it worth it ?

gsmet

This will create a breaking change on a lot of code as I would need to move the PanacheQuery interface that almost all user of the extension use ! Is it worth it ?

Well, that's not a question of "is it worth it?" but more a question of "can we do without it?". Split packages come with all sorts of problem so we really need to avoid them.

I think it's good enough if people have one version to move from one class to the other.

We need to add a paragraph in the migration guide about these changes.

gsmet · 2021-07-19T11:34:33Z

...mon/runtime/src/test/java/io/quarkus/mongodb/panache/common/runtime/MongoOperationsTest.java


-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.*;


Please avoid star imports.

loicmathieu · 2021-07-19T11:51:44Z

I think it's good enough if people have one version to move from one class to the other.

This is the issue, they will not have one version to move from a class to the other, they will need to do it immediatly as I need to move some interfaces so I cannot duplicate them as I did for the annotations.

gsmet · 2021-07-19T15:47:17Z

Why can't you keep the original interfaces and have the old one extending the new one? That's how we do things when moving interfaces around.

loicmathieu · 2021-07-19T16:09:11Z

I was thinking something similar today. It should work. I'll try this tomorrow

loicmathieu · 2021-07-20T10:58:43Z

@gsmet it's the other way around, the new interfaces need to extends the old one to make old code compile.
Can you have a quick look at the current impl of this PR ? It should works (test still runs).

I still need to:

Polish the code
Add the package exclusion to avoid the WARNING
Make both @MongoEntity annotation works
Add some test
Check the documentation
Add a note on the upgrading page

fixup! annotations fixup! annotations

loicmathieu · 2021-07-20T16:31:04Z

@gsmet I finished the PR, this has been a little more complicated than expected because of the usage of the MongoEntity annotation.

Maybe @FroMage or @evanchooly can also have a look as it's a pretty huge PR.

Now there is still a bunch of packages inside mongodb-panache-common that are not child of the io.quarkus.mongodb.panache.common package. There is currently no issue with them so I'll move them inside the common package on a followup PR to have a cleaner package hierarchy.

I'll write something in the upgrade page when the PR will be merged.

quarkus-bot · 2021-07-20T19:58:31Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `2201393`

Status	Name	Step	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 16

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/panache/mongodb-panache/deployment

✖ io.quarkus.mongodb.panache.mongoentity.MongoEntityTest. - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.mongodb.deployment.DevServicesMongoProcessor#startMongo threw an exception: java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.lambda$getFirstValidStrategy$7(DockerClientProviderStrategy.java:215)
	at java.base/java.util.Optional.orElseThrow(Optional.java:408)
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.getFirstValidStrategy(DockerClientProviderStrategy.java:207)
	at org.testcontainers.DockerClientFactory.getOrInitializeStrategy(DockerClientFactory.java:136)
	at org.testcontainers.DockerClientFactory.client(DockerClientFactory.java:178)
	at org.testcontainers.LazyDockerClient.getDockerClient(LazyDockerClient.java:14)
	at org.testcontainers.LazyDockerClient.authConfig(LazyDockerClient.java:12)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:310)
	at io.quarkus.mongodb.deployment.DevServicesMongoProcessor.startMongo(DevServicesMongoProcessor.java:175)
	at io.quarkus.mongodb.deployment.DevServicesMongoProcessor.startMongo(DevServicesMongoProcessor.java:95)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:6...

evanchooly · 2021-07-20T20:28:54Z

Overall it looks fine to me. Largely just a package move and concomitant code support changes. I'm curious why old-extending-new didn't work as that would be how I would've done it, too. But otherwise looks like a fairly mechanical rearrangement to me.

loicmathieu · 2021-07-21T07:16:59Z

@evanchooly the other way around didn't compile in Kotlin:

[ERROR] /data/git/quarkus/extensions/panache/mongodb-panache-kotlin/deployment/src/test/kotlin/io/quarkus/mongodb/panache/kotlin/deployment/Book.kt:[158,15] Type mismatch: inferred type is io.quarkus.mongodb.panache.common.PanacheUpdate! but io.quarkus.mongodb.panache.PanacheUpdate was expected

quarkus-bot · 2021-07-21T12:55:47Z

Failing Jobs - Building `2201393`

Status	Name	Step	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 16

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/panache/mongodb-panache/deployment

✖ io.quarkus.mongodb.panache.mongoentity.MongoEntityTest. - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.mongodb.deployment.DevServicesMongoProcessor#startMongo threw an exception: java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.lambda$getFirstValidStrategy$7(DockerClientProviderStrategy.java:215)
	at java.base/java.util.Optional.orElseThrow(Optional.java:408)
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.getFirstValidStrategy(DockerClientProviderStrategy.java:207)
	at org.testcontainers.DockerClientFactory.getOrInitializeStrategy(DockerClientFactory.java:136)
	at org.testcontainers.DockerClientFactory.client(DockerClientFactory.java:178)
	at org.testcontainers.LazyDockerClient.getDockerClient(LazyDockerClient.java:14)
	at org.testcontainers.LazyDockerClient.authConfig(LazyDockerClient.java:12)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:310)
	at io.quarkus.mongodb.deployment.DevServicesMongoProcessor.startMongo(DevServicesMongoProcessor.java:175)
	at io.quarkus.mongodb.deployment.DevServicesMongoProcessor.startMongo(DevServicesMongoProcessor.java:95)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:6...

gsmet

OK, let's make progress on this.

I will backport it as I want 2.2 to be clean on this side so we need to push it to 2.1 first.

gsmet · 2021-07-21T16:03:01Z

@loicmathieu can you add your classes there: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-2.1#panache-split-packages ? Thanks!

loicmathieu · 2021-07-21T17:06:12Z

@gsmet I'll do it tomorrow.
But I didn't merged it because one test didn't pass and I don't understand why. Seems to be linked to devservices.
I didn't had the time to check it today so I fear it will fail the CI.
Feel free to ignore the failing test I'l have a look tomorrow

loicmathieu · 2021-07-22T07:47:14Z

@gsmet I updated the wiki page.
I think the old calsses should be kept more than one release as I don't think everybody upgrade Quarkus version on each release so we should remove them in 3 or 4 releases.

quarkus-bot bot added area/documentation area/mongodb area/panache labels Jul 19, 2021

gsmet reviewed Jul 19, 2021

View reviewed changes

loicmathieu force-pushed the fix/duplicate-mongodb-panache-packages branch from 404db88 to c07020a Compare July 20, 2021 10:55

loicmathieu force-pushed the fix/duplicate-mongodb-panache-packages branch 2 times, most recently from 9d4ccc5 to e0ffb54 Compare July 20, 2021 11:20

loicmathieu added 4 commits July 20, 2021 18:22

Avoid split runtime packages on MongoDB with Panache

11899b5

Duplicate annotations

db73e41

fixup! annotations fixup! annotations

Duplicate interfaces

e901359

Silence split package as long as the deprecated components still exists

2201393

loicmathieu force-pushed the fix/duplicate-mongodb-panache-packages branch from e0ffb54 to 2201393 Compare July 20, 2021 16:26

loicmathieu marked this pull request as ready for review July 20, 2021 16:26

quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Jul 20, 2021

gsmet approved these changes Jul 21, 2021

View reviewed changes

gsmet merged commit d0555c7 into quarkusio:main Jul 21, 2021

quarkus-bot bot added this to the 2.2 - main milestone Jul 21, 2021

gsmet added the triage/backport? label Jul 21, 2021

gsmet changed the title ~~Fix/duplicate mongodb panache packages~~ Fix split MongoDB Panache packages Jul 21, 2021

gsmet modified the milestones: 2.2 - main, 2.1.0.Final Jul 21, 2021

gsmet removed the triage/backport? label Jul 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix split MongoDB Panache packages #18803

Fix split MongoDB Panache packages #18803

loicmathieu commented Jul 19, 2021

loicmathieu commented Jul 19, 2021

gsmet left a comment

gsmet Jul 19, 2021

loicmathieu commented Jul 19, 2021

gsmet commented Jul 19, 2021

loicmathieu commented Jul 19, 2021

loicmathieu commented Jul 20, 2021 •

edited

Loading

loicmathieu commented Jul 20, 2021

quarkus-bot bot commented Jul 20, 2021 •

edited

Loading

Failing Jobs - Building `2201393`

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/panache/mongodb-panache/deployment

evanchooly commented Jul 20, 2021

loicmathieu commented Jul 21, 2021

quarkus-bot bot commented Jul 21, 2021

gsmet left a comment

gsmet commented Jul 21, 2021

loicmathieu commented Jul 21, 2021

loicmathieu commented Jul 22, 2021


		import static org.junit.jupiter.api.Assertions.assertEquals;
		import static org.junit.jupiter.api.Assertions.*;

Fix split MongoDB Panache packages #18803

Fix split MongoDB Panache packages #18803

Conversation

loicmathieu commented Jul 19, 2021

loicmathieu commented Jul 19, 2021

gsmet left a comment

Choose a reason for hiding this comment

gsmet Jul 19, 2021

Choose a reason for hiding this comment

loicmathieu commented Jul 19, 2021

gsmet commented Jul 19, 2021

loicmathieu commented Jul 19, 2021

loicmathieu commented Jul 20, 2021 • edited Loading

loicmathieu commented Jul 20, 2021

quarkus-bot bot commented Jul 20, 2021 • edited Loading

Failing Jobs - Building 2201393

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/panache/mongodb-panache/deployment

evanchooly commented Jul 20, 2021

loicmathieu commented Jul 21, 2021

quarkus-bot bot commented Jul 21, 2021

Failing Jobs - Building 2201393

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/panache/mongodb-panache/deployment

gsmet left a comment

Choose a reason for hiding this comment

gsmet commented Jul 21, 2021

loicmathieu commented Jul 21, 2021

loicmathieu commented Jul 22, 2021

loicmathieu commented Jul 20, 2021 •

edited

Loading

quarkus-bot bot commented Jul 20, 2021 •

edited

Loading

Failing Jobs - Building `2201393`

Failing Jobs - Building `2201393`