quarkus-bot[bot]
commented
1 year ago /cc @gastaldi (m1)
Closed edeandrea closed 1 year ago
quarkus-bot[bot]
commented
1 year ago /cc @gastaldi (m1)
edeandrea
commented
1 year ago 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.
gsmet
commented
1 year ago @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.
mkouba
commented
1 year ago @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.
edeandrea
commented
1 year ago 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
What's a little more interesting is if I run the rest-heroes app 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 (or onto all of the methods), then this error goes away. but then ALL the tests inHeroServiceTestsfail (not just the ones that mutate data) with theNo current Vertx context found error mentioned above.
As an additional troubleshooting step, I added @WithSession to the HeroService class, then added @RunOnVertxContext to the HeroServiceTests class and got lots of these errors
16:30:34 WARN [io.ve.co.im.BlockedThreadChecker] (vertx-blocked-thread-checker) Thread Thread[vert.x-eventloop-thread-3,5,main] has been blocked for 2365 ms, time limit is 2000 ms: io.vertx.core.VertxException: Thread blockedso 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 some cases, the session is opened automatically on demand. For example, if a Panache entity method is invoked in a JAX-RS resource method in an application that includes the
quarkus-resteasy-reactiveextension.
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.
mkouba
commented
1 year ago
- Using the
io.quarkus.test.TestReactiveTransactionannotation at the class level causes lots of errors
Hm, I think that this should work because the io.quarkus.test.vertx.RunOnVertxContextTestMethodInvoker should be used if a test class is annotated with @TestReactiveTransaction. I'm not sure why it doesn't. @geoand do you have an idea?
- Hibernate reactive not finding Vertx context
HeroServiceTests.persistInvalidHero() is not a valid test case anymore, i.e. you can't call a HR Panache entity from a blocking thread. As explained in https://quarkus.io/version/main/guides/hibernate-reactive-panache#testing you'll need to use the @RunOnVertxContext annotation and UniAsserter, maybe also the convenient TransactionalUniAsserterInterceptor.
- No current
Mutiny.Sessionfound
@WithSession or @WithTransaction is the way to go for HeroResource.getRandomHero(). I'm not so sure about the but then ALL the tests in HeroServiceTests fail (not just the ones that mutate data) problem. But as I mentioned above the HeroServiceTests tests need to be rewritten anyway.
- Mocking/stubbing/spying not working right
This will need more investigation but Uni.combine().all().unis(hero, villain) is not legal within the context of a single Hibernate Reactive session.
As an additional troubleshooting step, I added @WithSession to the HeroService class, then added @RunOnVertxContext to the HeroServiceTests class and got lots of these errors
Well, you can't call any Uni.await() on the event loop thread. That's why I say the test should be rewritten using the UniAsserter.
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.
You're right - for these apps an explicit session/transaction demarcation is required ATM.
We could in theory add @WithSessionOnDemand on any method of a CDI bean that returns Uni and makes use of a HR Panache entity/repository, i.e. use the similar approach as we do for JAX-RS resources...
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.
Well, the main reason was consistency and alignment with Hibernate Reactive (and reactive in general). We don't store the current Mutiny.Session in the CDI context (the lifecycle of a reactive session does not fit the lifecycle of the CDI request context; note that we even removed the @RequestScoped Mutiny.Session bean!) and an execution of a HR panache entity is never offloaded on the current Vert.x context.
I'm all for improving the docs, migration guide and helping the users to overcome those limitations.
geoand
commented
1 year ago @mkouba I'll have a look at that this week and see what the problem is
edeandrea
commented
1 year ago Thank you for taking a look @mkouba . Two questions on the HeroService and it's tests...
1) The HeroServiceTests class completely mocks the HeroRepository via @InjectMock. In that case, why would the need for a session in the tests even matter?
Uni.combine().all().unis(hero, villain) not legal? How would someone make 2 simultaneous calls? These aren't calls to a database. These are calls using a jaxrs client.mkouba
commented
1 year ago Thank you for taking a look @mkouba . Two questions on the
HeroServiceand it's tests...1. The `HeroServiceTests` class completely mocks the `HeroRepository` via `@InjectMock`. In that case, why would the need for a session in the tests even matter?
Hm, you're right, the @InjectMock HeroRepository should not require a reactive session at all unless a method that is not mocked is called. I'll need to take a look at your branch.
2. Why is `Uni.combine().all().unis(hero, villain)` not legal? How would someone make 2 simultaneous calls? These aren't calls to a database. These are calls using a jaxrs client.
Sorry, I meant - it's not legal to use a reactive session concurrently. So in this case, it should be ok.
geoand
commented
1 year ago @mkouba I'll have a look at that this week and see what the problem is
@edeandrea Hibernate Reactive does yet work with Quarkus 3, we are working on it.
mkouba
commented
1 year ago @mkouba I'll have a look at that this week and see what the problem is
@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 hibernate-reactive-core-jakarta:hibernate-reactive-core-jakarta) in my test app and it works just fine.
geoand
commented
1 year ago I am talking about main. There is no point in trying to figure out what is going on with HR in the Alpha versions, as there are too many changes being made to support the move to ORM 6
edeandrea
commented
1 year ago 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.
geoand
commented
1 year ago Yes, hopefully we can pick it up next week
edeandrea
commented
1 year ago Got it. Thank you both for looking into things!
geoand
commented
1 year ago Thanks for reporting the issue :)
Let's circle back next week and see if we can address it
edeandrea
commented
1 year ago 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.
geoand
commented
1 year ago Any chance you can provide a standalone simplified sample that shows the mocking/spying problem?
edeandrea
commented
1 year ago I can try to take this app and strip it down as bare as it can get while still showing the problem.
geoand
commented
1 year ago Thanks a lot
edeandrea
commented
1 year ago @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.
geoand
commented
1 year ago 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 https://github.com/quarkusio/quarkus/issues/31400#issuecomment-1462167970, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBMDP64ALPN7U2GIGAGS3TW3HTCTANCNFSM6AAAAAAVG5I7CI . You are receiving this because you were mentioned.Message ID: @.***>
edeandrea
commented
1 year ago Let me know if you need anything else
geoand
commented
1 year ago 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.
edeandrea
commented
1 year ago I don't ever want to hear "you write too many tests" 😸
edeandrea
commented
1 year ago @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 :) )
edeandrea
commented
1 year ago 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
geoand
commented
1 year ago Thanks
mkouba
commented
1 year ago 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.
Could you point at the broken examples? I tried to explain in https://github.com/quarkusio/quarkus/issues/31400#issuecomment-1446180882 that tests like io.quarkus.sample.superheroes.hero.service.HeroServiceTests.persistHero() are not valid anymore and need to be rewritten...
edeandrea
commented
1 year ago And like I said before (https://github.com/quarkusio/quarkus/issues/31400#issuecomment-1446342375), the test completely mocks the repository via @InjectMock, so there shouldn't need to be a context.
mkouba
commented
1 year ago And like I said before (#31400 (comment)), the test completely mocks the repository via
@InjectMock, so there shouldn't need to be a context.
Ok, but then what is the "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." sentence related to?
edeandrea
commented
1 year ago Ok, but then what is the "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." sentence related to?
Thinking that I still needed to do that for some reason does not fix any of the tests is what I meant.
So let me ask this and this may help clarify my understanding. With these new Hibernate Reactive changes, it is now up to the developer to establish session boundaries within their application? So in my particular case, the class which uses the repository class (HeroService in my case) needs to control the session boundaries?
Which therefore means in my tests of that class, even though I am completely mocking the repository class and am never using Hibernate Reactive in the test, I still need to re-write my test because the method that I am testing (& completely mocking hibernate reactive underneath) is establishing a session boundary (which my test absolutely cares nothing about)?
mkouba
commented
1 year ago With these new Hibernate Reactive changes, it is now up to the developer to establish session boundaries within their application?
It's Hibernate Reactive Panache to be precise. But yes, a reactive session is not created automatically and stored in the CDI request context anymore. You can use @WithSession/@WithTransaction and Panache.withSession()/Panache.withTransaction() respectively. Since 3.0.0.Beta1 you can annotate a class and all methods that do not return a Uni are just ignored.
Which therefore means in my tests of that class, even though I am completely mocking the repository class and am never using Hibernate Reactive in the test, I still need to re-write my test because the method that I am testing (& completely mocking hibernate reactive underneath) is establishing a session boundary (which my test absolutely cares nothing about)?
Hm, now I finally understand the problem. The tested class is io.quarkus.sample.superheroes.hero.service.HeroService and the mocked repository is io.quarkus.sample.superheroes.hero.repository.HeroRepository, right?
Unfortunately, I don't have a solution in my sleeve. You can try to annotate the repository class with @WithTransaction (as I mentioned above - the methods that do not return Uni should be ignored since 3.0.0.Beta1).
edeandrea
commented
1 year ago given my last comment I made a few changes by pushing the transaction boundary into the repository (adding @WithTransaction to the repository class instead of marking methods in the service class). Doing so and seeing that the tests that were previously failing now succeed confirms my suspicions.
What I'm seeing now, however, is my tests for invalid input are failing.
For example, the service class has a method signature that looks like this:
public Uni<Hero> replaceHero(@SpanAttribute("arg.hero") @NotNull @Valid Hero hero)Notice the @NotNull and @Valid annotations on the parameter.
I have a test which tries to pass invalid input and assert that the right thing happens:
@Test
public void fullyUpdateInvalidHero() {
var hero = createDefaultHero();
hero.setName(null);
var cve = (ConstraintViolationException) this.heroService.replaceHero(hero)
.subscribe().withSubscriber(UniAssertSubscriber.create())
.assertSubscribed()
.awaitFailure(Duration.ofSeconds(5))
.assertFailedWith(ConstraintViolationException.class)
.getFailure();
assertThat(cve)
.isNotNull();
var violations = cve.getConstraintViolations();
assertThat(violations)
.isNotNull()
.hasSize(1);
assertThat(violations.stream().findFirst())
.isNotNull()
.isPresent()
.get()
.extracting(
ConstraintViolation::getInvalidValue,
ConstraintViolation::getMessage
)
.containsExactly(
null,
"must not be null"
);
verifyNoInteractions(this.heroRepository, this.heroFullUpdateMapper, this.heroPartialUpdateMapper);
}What I'm seeing though is that the ConstraintViolationException is being thrown but is breaking the reactive pipeline, almost like the pipeline isn't handling the error condition. It is terminating the pipeline and causing the test to fail with the following.
I can solve this using a try/catch (or assertj's assertions for an exception being thrown), but that doesn't seem to be the right thing to do? A thrown exception in a reactive pipeline shouldn't bubble out of the pipeline (I don't think).
jakarta.validation.ConstraintViolationException: 1 constraint violation(s) occurred during method validation.
Constructor or Method: public io.smallrye.mutiny.Uni io.quarkus.sample.superheroes.hero.service.HeroService.replaceHero(io.quarkus.sample.superheroes.hero.Hero)
Argument values: [Hero{id=1, name='null', otherName='Super Chocolatine chocolate in', level=42, picture='super_chocolatine.png', powers='does not eat pain au chocolat'}]
Constraint violations:
(1) Kind: PROPERTY
message: must not be null
root bean: io.quarkus.sample.superheroes.hero.service.HeroService_Subclass@6be7a271
property path: replaceHero.hero.name
constraint: @jakarta.validation.constraints.NotNull(message="{jakarta.validation.constraints.NotNull.message}", groups={}, payload={})
at io.quarkus.hibernate.validator.runtime.interceptor.AbstractMethodValidationInterceptor.validateMethodInvocation(AbstractMethodValidationInterceptor.java:67)
at io.quarkus.hibernate.validator.runtime.interceptor.MethodValidationInterceptor.validateMethodInvocation(MethodValidationInterceptor.java:17)
at io.quarkus.hibernate.validator.runtime.interceptor.MethodValidationInterceptor_Bean.intercept(Unknown Source)
at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:71)
at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:63)
at io.quarkus.opentelemetry.runtime.tracing.cdi.WithSpanInterceptor.span(WithSpanInterceptor.java:66)
at io.quarkus.opentelemetry.runtime.tracing.cdi.WithSpanInterceptor_Bean.intercept(Unknown Source)
at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:38)
at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:26)
at io.quarkus.sample.superheroes.hero.service.HeroService_Subclass.replaceHero(Unknown Source)
at io.quarkus.sample.superheroes.hero.service.HeroService_ClientProxy.replaceHero(Unknown Source)
at io.quarkus.sample.superheroes.hero.service.HeroServiceTests.fullyUpdateInvalidHero(HeroServiceTests.java:415)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1002)
at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:816)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:57)
at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)given my last comment I made a few changes by pushing the transaction boundary into the repository (adding @WithTransaction to the repository class instead of marking methods in the service class). Doing so and seeing that the tests that were previously failing now succeed confirms my suspicions.
:+1:
What I'm seeing though is that the ConstraintViolationException is being thrown but is breaking the reactive pipeline,...
It does not seem that Hibernate Validator interceptors support async return types...?
CC @gsmet @yrodiere
edeandrea
commented
1 year ago It does not seem that Hibernate Validator interceptors support async return types...?
They did in Quarkus 2. That same test in Quarkus 2 works fine (see https://github.com/quarkusio/quarkus-super-heroes/blob/main/rest-heroes/src/test/java/io/quarkus/sample/superheroes/hero/service/HeroServiceTests.java#L411-L445).
yrodiere
commented
1 year ago It does not seem that Hibernate Validator interceptors support async return types...?
They did in Quarkus 2. That same test in Quarkus 2 works fine (see https://github.com/quarkusio/quarkus-super-heroes/blob/main/rest-heroes/src/test/java/io/quarkus/sample/superheroes/hero/service/HeroServiceTests.java#L411-L445).
I also didn't think that was supported, and there's a standing issue about it: https://github.com/quarkusio/quarkus/issues/8821
I remember @geoand giving it a try but I thought there was a blocker. I can't find the PR anymore...
@gsmet do you remember what was the status of validation of Uni/Multi with Hibernate Validator?
edeandrea
commented
1 year ago This isn't about validating the return type. Its about validating an input parameter to a method. It works fine in Quarkus 2 but seems to be broken in 3.
mkouba
commented
1 year ago They did in Quarkus 2. That same test in Quarkus 2 works fine...
Well, the test could have passed by coincidence. The relevant code seems to be 5 years old - if a violation occurs during validation of params then a ConstraintViolationException is thrown directly from the interceptor.
edeandrea
commented
1 year ago Something further downstream must have caught that and forwarded to the pipeline as a failure.
I can certainly adjust my test to more of a try/catch, but if I do that on the Quarkus 2 version, the test fails because an exception is never thrown by calling the method. The exception is a failure in the pipeline.
So I guess my question is - how should it be? I would think that any failure at any point in the pipeline should propagate within the pipeline and not break it?
edeandrea
commented
1 year ago something else completely unrelated to the exception propagation question. I notice when I start the app its running importing the import.sql twice...
08:17:08 INFO [io.qu.da.de.de.DevServicesDatasourceProcessor] (build-37) Dev Services for the default datasource (postgresql) started - container ID is 6b3254ebe47e
__ ____ __ _____ ___ __ ____ ______
--/ __ \/ / / / _ | / _ \/ //_/ / / / __/
-/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/
08:17:09 WARN [io.qu.config] (Quarkus Main Thread) Unrecognized configuration key "quarkus.opentelemetry.tracer.resource-attributes" was provided; it will be ignored; verify that the dependency extension for this configuration is set or that you did not make a typo
08:17:09 WARN [io.qu.config] (Quarkus Main Thread) Unrecognized configuration key "quarkus.opentelemetry.tracer.exporter.otlp.endpoint" was provided; it will be ignored; verify that the dependency extension for this configuration is set or that you did not make a typo
08:17:09 WARN [io.qu.op.ru.ex.ot.LateBoundBatchSpanProcessor] (Quarkus Main Thread) No BatchSpanProcessor delegate specified, no action taken.
08:17:10 INFO [or.hi.re.pr.im.ReactiveIntegrator] (JPA Startup Thread) HR000001: Hibernate Reactive
08:17:10 INFO [or.hi.to.sc.in.SchemaCreatorImpl] (JPA Startup Thread) HHH000476: Executing import script 'file:/Users/edeandre/workspaces/quarkus/quarkus-super-heroes/rest-heroes/target/classes/import.sql'
08:17:10 WARN [io.ve.sq.im.SocketConnectionBase] (vert.x-eventloop-thread-0) Backend notice: severity='NOTICE', code='00000', message='table "hero" does not exist, skipping', detail='null', hint='null', position='null', internalPosition='null', internalQuery='null', where='null', file='tablecmds.c', line='1272', routine='DropErrorMsgNonExistent', schema='null', table='null', column='null', dataType='null', constraint='null'
08:17:10 WARN [io.ve.sq.im.SocketConnectionBase] (vert.x-eventloop-thread-0) Backend notice: severity='NOTICE', code='00000', message='sequence "hero_seq" does not exist, skipping', detail='null', hint='null', position='null', internalPosition='null', internalQuery='null', where='null', file='tablecmds.c', line='1272', routine='DropErrorMsgNonExistent', schema='null', table='null', column='null', dataType='null', constraint='null'
08:17:10 INFO [or.hi.to.sc.in.SchemaCreatorImpl] (JPA Startup Thread) HHH000476: Executing import script 'file:/Users/edeandre/workspaces/quarkus/quarkus-super-heroes/rest-heroes/target/classes/import.sql'
08:17:10 WARN [io.ve.sq.im.SocketConnectionBase] (vert.x-eventloop-thread-0) Backend notice: severity='NOTICE', code='00000', message='table "hero" does not exist, skipping', detail='null', hint='null', position='null', internalPosition='null', internalQuery='null', where='null', file='tablecmds.c', line='1272', routine='DropErrorMsgNonExistent', schema='null', table='null', column='null', dataType='null', constraint='null'
08:17:10 WARN [io.ve.sq.im.SocketConnectionBase] (vert.x-eventloop-thread-0) Backend notice: severity='NOTICE', code='00000', message='sequence "hero_seq" does not exist, skipping', detail='null', hint='null', position='null', internalPosition='null', internalQuery='null', where='null', file='tablecmds.c', line='1272', routine='DropErrorMsgNonExistent', schema='null', table='null', column='null', dataType='null', constraint='null'
08:17:10 INFO [io.quarkus] (Quarkus Main Thread) rest-heroes 1.0 on JVM (powered by Quarkus 3.0.0.Beta1) started in 5.207s. Listening on: http://localhost:8083
08:17:10 INFO [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
08:17:10 INFO [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, config-yaml, hibernate-orm, hibernate-reactive, hibernate-reactive-panache, hibernate-validator, kotlin, kubernetes, micrometer, opentelemetry, pact-provider, qute, reactive-pg-client, resteasy-reactive, resteasy-reactive-jackson, resteasy-reactive-qute, smallrye-context-propagation, smallrye-health, smallrye-openapi, swagger-ui, vertx]
``something else completely unrelated to the exception propagation question. I notice when I start the app its running importing the import.sql twice...
edeandrea
commented
1 year ago Thanks @yrodiere !
mkouba
commented
1 year ago I would think that any failure at any point in the pipeline should propagate within the pipeline and not break it?
The problem is that here the exception is thrown "outside" the pipeline.
In theory, HV could return Uni.createFrom().failure(new ConstraintViolationException(...)) instead of throw new ConstraintViolationException(...) in case of the method returns Uni...
cescoffier
commented
1 year ago yes, it should returned a failed Uni in this case. I'm wondering how it worked before.
geoand
commented
1 year ago I remember @geoand giving it a try but I thought there was a blocker. I can't find the PR anymore...
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
yrodiere
commented
1 year ago I remember @geoand giving it a try but I thought there was a blocker. I can't find the PR anymore...
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 Multi: https://github.com/quarkusio/quarkus/pull/11864. I'm guessing there must be some limitations there, though, since some things don't seem possible unless we introduce some way for Hibernate Validator to understand @Min(0) Uni<Integer> as @Min(0) Integer, for example.
Back to our topic... no, it doesn't seem like validation of reactive methods ever worked in RestEasy Reactive.
geoand
commented
1 year ago it doesn't seem like validation of reactive methods ever worked in RestEasy Reactive.
I certainly never did anything to make it work :)
edeandrea
commented
1 year ago I certainly never did anything to make it work :)
So is it a bug or a feature that it works in 2.x 😁
geoand
commented
1 year ago An accident :)
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