Quarkus Superheroes Quarkus 2 -> 3 upgrade issues #31400
Comments
|
/cc @gastaldi (m1) |
|
https://github.com/quarkiverse/quarkiverse/wiki/Migrating-to-Quarkus-3.x#its-all-gone-wrong-help says to mention @maxandersen and @holly-cummins, so there, I did it :) Although maybe that was only for extension authors. Sorry If I pinged you and shouldn't have. |
|
@mkouba could you have a look at this one? It looks related to Hibernate Reactive and Vert.x. Not exactly sure how the Jakarta changes could have broken it tbh. |
I'll take a look next Monday. I don't think it's related to the Jakarta changes. We did some breaking changes in Hibernate Reactive Panache... It's documented in the migration guide by the way. |
Yes I read the guide and attempted to follow it. If you see from my original note
As an additional troubleshooting step, I added so I'm not quite sure where to go for from there. Issues 2 & 3 are most likely related. I think once one is solved the other will be solved too. Issues 1 & 4 aren't related I don't think. Maybe issue 1 is related too, but I'm not sure. Also, I'm curious about this statement in the migration guide:
In the case of these apps (as I would presume would be the case for MANY apps out there in the wild), the panache methods (using a Panache repository class in this particular instance) is NOT invoked directly from the JAX-RS methods. Instead, they are accessed from the "business tier", which is its own CDI bean. We could debate whether or not a JAX-RS layer even belongs talking directly to a persistence layer in an application, but regardless of what side of that debate you're on, there will be lots and lots of apps that don't, so this is going to come and bite them. Is there a particular reason for this change in behavior? It just seems like lots of unnecessary boilerplate stuff that a user has to go through just to write a method that accesses data. |
Hm, I think that this should work because the
This will need more investigation but
Well, you can't call any
You're right - for these apps an explicit session/transaction demarcation is required ATM. We could in theory add
Well, the main reason was consistency and alignment with Hibernate Reactive (and reactive in general). We don't store the current I'm all for improving the docs, migration guide and helping the users to overcome those limitations. |
|
@mkouba I'll have a look at that this week and see what the problem is |
|
Thank you for taking a look @mkouba . Two questions on the
|
Hm, you're right, the
Sorry, I meant - it's not legal to use a reactive session concurrently. So in this case, it should be ok. |
@edeandrea Hibernate Reactive does yet work with Quarkus 3, we are working on it. |
@geoand What do you mean? Which version of HR? I do use quarkus 3.0.0.Alpha4 and HR Panache (based on |
|
I am talking about |
|
So @geoand guess that means I just leave it for now? The 1st 3 issues are probably related to hibernate reactive. The last one (mocking/spying/etc) is not. The fights service is reactive but uses panache with mongodb. Not HR. |
|
Yes, hopefully we can pick it up next week |
|
Got it. Thank you both for looking into things! |
|
Thanks for reporting the issue :) Let's circle back next week and see if we can address it |
|
Hi folks. FYI I upgraded things to Alpha5. As expected, the things involving hibernate reactive I can't even take the alpha5 upgrade because of https://quarkus.io/blog/quarkus-3-0-0-alpha5-released/#hibernate-reactive-temporarily-disabled But the last issue (# 4 in my original list) (Mocking/stubbing/spying not working right) is still broken. |
|
Any chance you can provide a standalone simplified sample that shows the mocking/spying problem? |
|
I can try to take this app and strip it down as bare as it can get while still showing the problem. |
|
Thanks a lot |
|
@geoand I created https://github.com/edeandrea/quarkus3-supes-mocking which is as stripped down as I can get it (no Kafka/stork/rest client/rest endpoints/mapstruct/removing timeout fault tolerance/etc) to still show the problem. The repo has 2 identical versions of the stripped down app, 1 with Quarkus 2 & 1 with Quarkus 3. All tests pass in the quarkus 2 version yet there are failures in the Quarkus 3 version. The README file has specific details. |
|
Thanks a lot!
…On Thu, Mar 9, 2023, 16:35 Eric Deandrea ***@***.***> wrote:
@geoand <https://github.com/geoand> I created
https://github.com/edeandrea/quarkus3-supes-mocking which is as stripped
down as I can get it to still show the problem. It has 2 identical versions
of the stripped down app, 1 with Quarkus 2 & 1 with Quarkus 3. All tests
pass in the quarkus 2 version yet there are failures in the Quarkus 3
version.
The README file
<https://github.com/edeandrea/quarkus3-supes-mocking/blob/main/README.md>
has specific details.
—
Reply to this email directly, view it on GitHub
<#31400 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBMDP64ALPN7U2GIGAGS3TW3HTCTANCNFSM6AAAAAAVG5I7CI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Let me know if you need anything else |
|
There seems to be something funky going on with the Bean subclasses. @mkouba @Ladicek could this be due to the changes in handling synthetic methods we were talking about earlier? @edeandrea reproducer is super useful here as it has both a Quarkus 2 version that works as expected and a failing Quarkus 3 version. |
|
I don't ever want to hear "you write too many tests" 😸 |
|
@geoand I'll update the superheroes and the mock reproducer I built to 3.0.0.Beta1 tomorrow and report back of anything gets fixed (or gets broken further :) ) |
|
ok so I couldn't wait :) The mocking issue still exists in 3.0.0.Beta1. I updated https://github.com/edeandrea/quarkus3-supes-mocking to 2.16.5.Final & 3.0.0.Beta1. 3.0.0.Beta is still broken. The other stuff with Hibernate reactive (no Vertx context found, etc) is still broken, even after adding all the aforementioned session controls to the methods. I updated https://github.com/edeandrea/quarkus-super-heroes/tree/quarkus3 to 3.0.0.Beta1. You can see the latest build result at https://github.com/edeandrea/quarkus-super-heroes/actions/runs/4506826311 |
|
Thanks |
|
The problem is that here the exception is thrown "outside" the pipeline. In theory, HV could return |
|
yes, it should returned a failed Uni in this case. I'm wondering how it worked before. |
Yup, I had tried something (I think I had only posted it in a comment of an issue) and you had mentioned that it was more complex |
Right, now I found your proposal and my answer explaining some problems and edge cases. Interestingly, Clément supposedly implemented validation for Reactive Routes, except for Back to our topic... no, it doesn't seem like validation of reactive methods ever worked in RestEasy Reactive. |
I certainly never did anything to make it work :) |
So is it a bug or a feature that it works in 2.x 😁 |
|
An accident :) |
|
Maybe one thing I should mention. The methods in question here are NOT on a rest resource method. They're in another class entirely (business method on another bean). Validation of a request body in a jax-rs method works as it should. |
|
@geoand FYI the mocking issue still exists in 3.0.0.CR1. I updated https://github.com/edeandrea/quarkus3-supes-mocking to 3.0.0.CR1. |
|
Thanks for the update |
|
Did we have a dedicated issues for that? |
|
No, it was part of this thread. I can create one though if you'd like. |
|
Let's do that as this thread has too many discussions. |
|
Thank you everyone that has helped out so far! I took CR2 and the mocking issue (from #32300) is now resolved. I'm now stuck on 2 things
|
@edeandrea Hm, it's not a mock but a spy, i.e. you only mock the |
|
Nope no other method on the repository is called in that flow. The repository class itself is also annotated with |
|
Should I open a separate issue on the constraint validation issue mentioned in #31400 (comment)? Seems like a regression between Quarkus 2 & 3. @cescoffier mentioned that the exception should not bubble up outside of the reactive pipeline. |
|
Yes, it's very hard to use a single issue to track all different problems. I would also close this one to TBH as some things have been fixed and as such the issue has outlived its usefulness. |
|
Will do that later today. |
|
I'll even build as small a reproducer as possible :) |
|
💪🏼 |
|
Thanks |
|
Thought I'd let folks know I got the 1st successful build of the superheroes with Quarkus 3 today! https://github.com/edeandrea/quarkus-super-heroes/actions/runs/4692776058 |
Describe the bug
I'm in process of upgrading the Quarkus Superheroes to use Quarkus 3. I'm working on my own personal fork for the moment (https://github.com/edeandrea/quarkus-super-heroes on the
quarkus3branch - https://github.com/edeandrea/quarkus-super-heroes/tree/quarkus3).I've run into a few things that I can't seem to figure out how to solve and could use some help/guidance
io.quarkus.test.TestReactiveTransactionannotation at the class level causes lots of errorsMutiny.SessionfoundUsing the
io.quarkus.test.TestReactiveTransactionannotation at the class level causes lots of errorsRemoving the annotation from the class and instead placing it on each & every test method in the class removes the errors. I would still call it a bug, though, but at least I can get past it.
This is the error I was seeing when
@TestReactiveTransactionwas placed at the class level:Hibernate reactive not finding Vertx context
In the
rest-heroesapplication there's some issue with hibernate reactive where its not finding a Vertx context (see https://github.com/edeandrea/quarkus-super-heroes/actions/runs/4257167883/jobs/7406959691 the full test execution, failures, and errors). It seems to be the same problem causing all the failures.What's somewhat interesting is that the tests that fail are tests that mutate data. Tests which only read data all run fine.
I did replace all instances of
@ReactiveTransactionalwith@WithTransactionbecause@ReactiveTransactionalis deprecated. Changing back to@ReactiveTransactionaldoesn't change anything. I still see the same failures.Here's an excerpt of one of them:
No current
Mutiny.SessionfoundWhat's a little more interesting is if I run the
rest-heroesapp in dev mode and then try to hit the/api/heroes/randomendpoint (or any other endpoint) I get the following error, even though the tests which call these same operations pass.If I add the
@WithSessionor@WithSessionOnDemandannotation to theHeroServiceclass, then this error goes away. but then ALL the tests inHeroServiceTestsfail (not just the ones that mutate data) with theNo current Vertx context founderror mentioned above.As an additional troubleshooting step, I added
@WithSessionto theHeroServiceclass, then added@RunOnVertxContextto theHeroServiceTestsclass and got lots of these errors:Mocking/stubbing/spying not working right
In the
rest-fightsapplication there's some issue with mocking/stubbing/spying within tests. See https://github.com/edeandrea/quarkus-super-heroes/actions/runs/4257167883/jobs/7406959585 for full the full test execution, failures, and errors.For example, if you look at the test failure for
io.quarkus.sample.superheroes.fight.service.FightServiceTests.findRandomFightersNoneFoundit sayswhich isn't at all true. If you look at the code in
io.quarkus.sample.superheroes.fight.service.FightService:You can see that
findRandomFightersdefinitely callsfindRandomHeroandfindRandomVillain. You can even see this in the test log output:In the test in
io.quarkus.sample.superheroes.fight.service.FIghtServiceTests.findRandomFightersNoneFound:you can see that it is verifying that the
findRandomHeroandfindRandomVillainmethods should have been called once, which the logs clearly show they did, but yet the mocking/spying doesn't see that they did.Expected behavior
No response
Actual behavior
No response
How to Reproduce?
Reproducer: https://github.com/edeandrea/quarkus-super-heroes/tree/quarkus3 (
quarkus3branch from https://github.com/edeandrea/quarkus-super-heroes)For seeing the issues in
rest-heroes:cdintorest-heroes./mvnw clean verifyFor seeing the issues in
rest-fights:cdintorest-fights./mvnw clean verifyOutput of
uname -aorverDoesn't really matter. My local machine is a mac m1, but the GH action (https://github.com/edeandrea/quarkus-super-heroes/actions/runs/4257167883) uses ubuntu.
Output of
java -versionTemurin 11 or 17. The GH action (https://github.com/edeandrea/quarkus-super-heroes/actions/runs/4257167883) tests using both.
GraalVM version (if different from Java)
22.3.1
Quarkus version or git rev
3.0.0.Alpha4
Build tool (ie. output of
mvnw --versionorgradlew --version)No response
Additional information
No response
The text was updated successfully, but these errors were encountered: