DRAFT: Switch from the Search session holder to an ORM session extension #4893
Conversation
| // Only do this as a fallback: if somehow the session holder was added to the session properties | ||
| // before the session was closed, we definitely want to use it and avoid the static map. | ||
| holder = holderPerClosedSessionTransaction.get( session.accessTransaction() ); | ||
| } |
There was a problem hiding this comment.
Hi @yrodiere 👋🏻 🙂
I've tried to integrate the ORM extension, and it made me wonder if this ^ could even happen with holder being attached to a session as soon as it (session) is created.
The tests passed without this static map.. but ... I'm not sure whether we actualy have any tests to hit this scenario, or we do?
There was a problem hiding this comment.
Hey. It's been a long time... but I don't see a reason this would still be needed if the holder is added when creating the session.
Also, the PR that added this code seems to be specifically about closed sessions: #2450
So if the tests from that PR are still around, and still pass... +1 to remove all this code?
There was a problem hiding this comment.
ahh so the tests are in Spring-JTA ... but I haven't run those particular ones as I was running on JDK 25 ... let me give it another try with 21 to double check
There was a problem hiding this comment.
ok ... so I did aaaand it works but we need to figure out the synchronisation part so that it doesn't need extra casts/unwraps. 🙂
marko-bekhta
force-pushed
the
wip/orm-session-extension
branch
from
3fc04b3 to
c219d34
Compare
| // TODO: won't be able to use these (toEntityManager / toOrmSession / session) in a stateless session case... | ||
| // options ? | ||
| // = deprecate and introduce some shared session interface instead ? | ||
| // = keep two search session interfaces one for stateless one for stateful ? | ||
| @Override | ||
| public EntityManager toEntityManager() { | ||
| return sessionImplementor; | ||
| } | ||
|
|
||
| @Override | ||
| public Session toOrmSession() { | ||
| return sessionImplementor; | ||
| } | ||
|
|
||
| @Override | ||
| public SessionImplementor session() { | ||
| return sessionImplementor; | ||
| } | ||
|
|
There was a problem hiding this comment.
we have these publicly exposed ... so I wonder what should we do about them ... (once we start the stateless session work...)
| // TODO: There's no action queue with stateless session ... | ||
| // Hence it would be great if we could just have an SPI in ORM to deal with this instead? | ||
| // TODO: Have a closer look at this part to figure the best way to deal with stateless sessions.. |
There was a problem hiding this comment.
we won't have a queue in stateless session so we'd need to deal with this one way or the other 😃,
but an ORM SPI would be nice 🙂
2 participants
Based on hibernate/hibernate-orm#11093
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.