Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No Panache session in REST endpoints defined by an interface #36236

Closed
danielbobbert opened this issue Oct 2, 2023 · 13 comments · Fixed by #36432
Closed

No Panache session in REST endpoints defined by an interface #36236

danielbobbert opened this issue Oct 2, 2023 · 13 comments · Fixed by #36432
Labels
area/panache env/m1 Impacts Apple M1 machines kind/bug Something isn't working
Milestone

Comments

@danielbobbert
Copy link
Contributor

Describe the bug

When a REST resource is implemented as a CDI bean (without any Jax-RS annotations) that implements an interface (which carries the REST annotations), then invoking a method on this services fails because of a missing Panache session. Error message:

- no reactive session was found in the context and the context was not marked to open a new session lazily
- you might need to annotate the business method with @WithSession
at io.quarkus.hibernate.reactive.panache.common.runtime.SessionOperations.getSession(SessionOperations.java:163)

Expected behavior

The REST services should be executed with an active Panache session, just as if the REST annotations were present on the resource implementation itself.
This used to work in Quarkus 2 without problem.

Actual behavior

If the resource implementation has REST annotations itself, the execution works correctly. If the REST annotations are provided by an interface, invocation fails.

How to Reproduce?

See attached sample project and execute the tests. It contains two resources:

  • one with REST annotations on the resource itself
  • one which implements an interface that provides the REST annotations

The first resource works as expected, the second resource fails.

Output of uname -a or ver

Darwin dbb-m1.local 22.6.0 Darwin Kernel Version 22.6.0: Fri Sep 15 13:41:28 PDT 2023; root:xnu-8796.141.3.700.8~1/RELEASE_ARM64_T6000 arm64

Output of java -version

17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.4.1

Build tool (ie. output of mvnw --version or gradlew --version)

mvn 3.9.4

Additional information

reactive-session-bug.zip

@danielbobbert danielbobbert added the kind/bug Something isn't working label Oct 2, 2023
@quarkus-bot quarkus-bot bot added area/panache env/m1 Impacts Apple M1 machines labels Oct 2, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 2, 2023

/cc @FroMage (panache), @gastaldi (m1), @loicmathieu (panache)

@danielbobbert
Copy link
Contributor Author

I double checked with Quarkus 3.3, 3.2 and 3.0.4, and they all exhibit the same problem. So it seems to have been introduced with the switch to Quarkus 3.

@gsmet
Copy link
Member

gsmet commented Oct 6, 2023

@DavideD @mkouba wondering if it could be either related to Hibernate Reactive changes or to CDI changes?

@mkouba
Copy link
Contributor

mkouba commented Oct 9, 2023

@DavideD @mkouba wondering if it could be either related to Hibernate Reactive changes or to CDI changes?

Yes, this is definitely a consequence of HR Panache refactoring introduced in quarkus 3.0. We do not open the reactive session automatically when the resource annotations are declared on an interface. The details are explained in the migration guide.

@danielbobbert you can just annotate the resource methods/class with @WithSession or @WithTransaction.

@danielbobbert
Copy link
Contributor Author

Yes, that's what I am doing right now and it's certainly not a big deal. I still find it kind of inconsistent that you are automatically opening a session when the jax-rs annotations are on the bean itself, but you don't if the annotations are on the interface. From my understanding, there is no semantic difference between the two, and they should behave the same way.

@mkouba
Copy link
Contributor

mkouba commented Oct 9, 2023

From my understanding, there is no semantic difference between the two, and they should behave the same way.

Yes, that's my understanding too. However, we only cover the basic use cases. Also keep in mind that @WithSession and @WithTransaction are CDI interceptor bindings and cannot be used on interfaces (CDI inheritance rules are different compared to JAX-RS).

@danielbobbert
Copy link
Contributor Author

danielbobbert commented Oct 9, 2023

Yes, definitely. And since session management is certainly an implementation-detail I would put those annotations on the service (impl) anyways and not on the interface.
But the question whether you automatically open a session, is a JAX-RS question and not a CDI question, right?! Obviously, putting JAX-RS annotation on the interface (instead of the bean itself) doesn't make a difference in terms of JAX-RS. It's automatically recognized as an endpoint with routing, serialization, exception mapping, etc. all in place.
And your docs state: "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".
There's no addition "but only, if you happen to put your JAX-RS annotations in the right place". How do you actually check whether "a Panache entity method is invoked in a JAX-RS resource method"? Is this explicitly checking for some annotations (then it should be easy to collect those annotations from interfaces as well)? Or is there some other subtle difference at runtime, depending on how the JAX-RS resource is defined?
Anyways: as we already agreed, it's easy to fix (by putting @WithSession on the bean), it's just a bit counter-intuitive that it behaves differently. So feel free to fix it, or just close the issue.

@FroMage
Copy link
Member

FroMage commented Oct 10, 2023

How hard would it be to support this, @mkouba ?

@mkouba
Copy link
Contributor

mkouba commented Oct 10, 2023

How do you actually check whether "a Panache entity method is invoked in a JAX-RS resource method"? Is this explicitly checking for some annotations (then it should be easy to collect those annotations from interfaces as well)? Or is there some other subtle difference at runtime, depending on how the JAX-RS resource is defined?

@danielbobbert We add @WithSessionOnDemand to a method that

  • is not static,
  • is not synthetic,
  • returns Uni,
  • is declared in a class that uses a panache entity/repository,
  • is annotated with an HTTP method designator (@GET, @POST, @PUT, @DELETE ,@PATCH ,@HEAD or @OPTIONS),
  • and is not annotated with @ReactiveTransactional, @WithSession, @WithSessionOnDemand, or @WithTransaction.

https://github.com/quarkusio/quarkus/blob/main/extensions/panache/hibernate-reactive-panache-common/deployment/src/main/java/io/quarkus/hibernate/reactive/panache/common/deployment/PanacheJpaCommonResourceProcessor.java#L104-L182

How hard would it be to support this, @mkouba ?

@FroMage I'm not sure. In theory, we could modify the logic and instead of transforming method annotations we could transform annotations of a resource class. In any case, we would also need to scan the class hierarchy for interfaces with JAX-RS annotations...

@danielbobbert
Copy link
Contributor Author

Am I correct that your code currently only catches "direct" usage of Panache? If the JAX-RS resource uses an injected service bean, which in turn uses an injected Panache repository (but is not a Panache repository itself), then this case would not be detected by the current code either, right?!

@mkouba
Copy link
Contributor

mkouba commented Oct 10, 2023

Am I correct that your code currently only catches "direct" usage of Panache? If the JAX-RS resource uses an injected service bean, which in turn uses an injected Panache repository (but is not a Panache repository itself), then this case would not be detected by the current code either, right?!

Yes, that would be a non-trivial task to analyse all the injected services or detect any indirect usage of panache.

And it was not a goal to cover these scenarios.

@FroMage
Copy link
Member

FroMage commented Oct 11, 2023

In any case, we would also need to scan the class hierarchy for interfaces with JAX-RS annotations.

Not just scan, also find out what method we're overriding, which includes resolving type parameters and all…

Perhaps the easiest is to add this message to the exception:

- This automatically works from JAX-RS annotated methods (not via an interface)

So at least, we make it clear where it works and not?

@mkouba
Copy link
Contributor

mkouba commented Oct 12, 2023

In any case, we would also need to scan the class hierarchy for interfaces with JAX-RS annotations.

Not just scan, also find out what method we're overriding, which includes resolving type parameters and all…

Well, I was thinking that we could add @WithSessionOnDemand to the class, not specific methods, i.e. if a class implements a JAX-RS resource then just annotate the class with @WithSessionOnDemand and all methods returning Uni would have a session opened automatically.

The downside of this approach is that if a method is annotated e.g. with @WithSession then we would intercept the method twice which is suboptimal.

Perhaps the easiest is to add this message to the exception:

Yes, that makes sense. I'll send a PR.

mkouba added a commit to mkouba/quarkus that referenced this issue Oct 12, 2023
- when no current reactive session is found
- resolves quarkusio#36236
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Oct 12, 2023
@gsmet gsmet modified the milestones: 3.6 - main, 3.5.0 Oct 17, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue Oct 17, 2023
- when no current reactive session is found
- resolves quarkusio#36236

(cherry picked from commit 82c1323)
benkard pushed a commit to benkard/mulkcms2 that referenced this issue Nov 12, 2023
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.219.0` -> `^0.220.0`](https://renovatebot.com/diffs/npm/flow-bin/0.219.0/0.220.0) |
| [org.jsoup:jsoup](https://jsoup.org/) ([source](https://github.com/jhy/jsoup)) | compile | patch | `1.16.1` -> `1.16.2` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.4.3` -> `3.5.0` |
| [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | minor | `3.4.3` -> `3.5.0` |

---

### Release Notes

<details>
<summary>flowtype/flow-bin</summary>

### [`v0.220.0`](flow/flow-bin@f7f3f3f...030bfc6)

[Compare Source](flow/flow-bin@f7f3f3f...030bfc6)

### [`v0.219.5`](flow/flow-bin@f16a6c7...f7f3f3f)

[Compare Source](flow/flow-bin@f16a6c7...f7f3f3f)

### [`v0.219.4`](flow/flow-bin@9f67075...f16a6c7)

[Compare Source](flow/flow-bin@9f67075...f16a6c7)

### [`v0.219.3`](flow/flow-bin@80dcea5...9f67075)

[Compare Source](flow/flow-bin@80dcea5...9f67075)

### [`v0.219.2`](flow/flow-bin@c184c5d...80dcea5)

[Compare Source](flow/flow-bin@c184c5d...80dcea5)

</details>

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.5.0`](https://github.com/quarkusio/quarkus/releases/tag/3.5.0)

[Compare Source](quarkusio/quarkus@3.4.3...3.5.0)

##### Complete changelog

-   [#&#8203;36527](quarkusio/quarkus#36527) - Start MongoDB 4.4 instead of 4.0
-   [#&#8203;36523](quarkusio/quarkus#36523) - Minor OIDC Auth0 updates
-   [#&#8203;36518](quarkusio/quarkus#36518) - Allow for setting logging scope programmatically
-   [#&#8203;36517](quarkusio/quarkus#36517) - Use Mandrel 23.1 in windows CI
-   [#&#8203;36501](quarkusio/quarkus#36501) - Let custom OIDC token propagation filters customize the exchange status
-   [#&#8203;36495](quarkusio/quarkus#36495) - Support external OTel exporters in CDI
-   [#&#8203;36490](quarkusio/quarkus#36490) - Take ReaderInterceptor into account when reading SSE events
-   [#&#8203;36487](quarkusio/quarkus#36487) - Upgrade to Liquibase 4.24.0
-   [#&#8203;36485](quarkusio/quarkus#36485) - Fix typo in gradle-tooling.adoc
-   [#&#8203;36474](quarkusio/quarkus#36474) - Fix some issues in getting-started-dev-services
-   [#&#8203;36465](quarkusio/quarkus#36465) - Be more consistent in guides when creating projects/adding extensions
-   [#&#8203;36464](quarkusio/quarkus#36464) - HTTP reference guide - HTTP/2 section update, drop JDK 8 note
-   [#&#8203;36459](quarkusio/quarkus#36459) - Let custom OIDC token propagation filters provide client name
-   [#&#8203;36457](quarkusio/quarkus#36457) - Update builder images to jdk-21
-   [#&#8203;36453](quarkusio/quarkus#36453) - Upgrade Oracle JDBC driver to 23.3.0.23.09
-   [#&#8203;36452](quarkusio/quarkus#36452) - Fix doc extension-add.adoc
-   [#&#8203;36451](quarkusio/quarkus#36451) - Adjust extension name for consistency with rest of Quarkus
-   [#&#8203;36446](quarkusio/quarkus#36446) - Regression: Liquibase fails to migrate on Quarkus start, crashing the application
-   [#&#8203;36445](quarkusio/quarkus#36445) - Updates to Infinispan 14.0.19.Final
-   [#&#8203;36442](quarkusio/quarkus#36442) - Use default content type when X-SSE header not set
-   [#&#8203;36436](quarkusio/quarkus#36436) - Upgrade to Hibernate ORM 6.2.13.Final
-   [#&#8203;36432](quarkusio/quarkus#36432) - Hibernate Reactive Panache: improve error message
-   [#&#8203;36420](quarkusio/quarkus#36420) - Allow parallel execution of blocking health checks
-   [#&#8203;36419](quarkusio/quarkus#36419) - Blocking Health Checks should be executed in parallel, not sequentially/ordered
-   [#&#8203;36417](quarkusio/quarkus#36417) - Reduce timeout of the doc build to 60 minutes
-   [#&#8203;36413](quarkusio/quarkus#36413) - Simplify virtual threads guide by pushing users to 21
-   [#&#8203;36412](quarkusio/quarkus#36412) - Drop Optaplanner from the documentation
-   [#&#8203;36411](quarkusio/quarkus#36411) - Drop panache topic from Hibernate Reactive guide
-   [#&#8203;36410](quarkusio/quarkus#36410) - Add compatibility topic to Spring guides
-   [#&#8203;36407](quarkusio/quarkus#36407) - Register RuntimeOverrideConfigSource in STATIC_INIT
-   [#&#8203;36406](quarkusio/quarkus#36406) - AssembleDownstreamDocumentation - print guide name
-   [#&#8203;36400](quarkusio/quarkus#36400) - Add topics and extensions metadata to guides
-   [#&#8203;36367](quarkusio/quarkus#36367) - Bump org.wiremock:wiremock-standalone from 3.1.0 to 3.2.0
-   [#&#8203;36365](quarkusio/quarkus#36365) - Bump de.flapdoodle.embed:de.flapdoodle.embed.mongo from 4.7.0 to 4.9.2
-   [#&#8203;36360](quarkusio/quarkus#36360) - Drop the old Dev UI guide
-   [#&#8203;36337](quarkusio/quarkus#36337) - Upgrade maven to version 3.9.5
-   [#&#8203;36236](quarkusio/quarkus#36236) - No Panache session in REST endpoints defined by an interface
-   [#&#8203;35931](quarkusio/quarkus#35931) - Add OIDC Auth0 extended tutorial
-   [#&#8203;33548](quarkusio/quarkus#33548) - Pick random debug port when the configured one is taken
-   [#&#8203;33363](quarkusio/quarkus#33363) - allow quarkus dev to pick random debug port

</details>

<details>
<summary>quarkusio/quarkus-platform</summary>

### [`v3.5.0`](quarkusio/quarkus-platform@3.4.3...3.5.0)

[Compare Source](quarkusio/quarkus-platform@3.4.3...3.5.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Feb 8, 2024
- when no current reactive session is found
- resolves quarkusio#36236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/panache env/m1 Impacts Apple M1 machines kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants