I think you got it.
By the way, just to clarify... I am not pushing one or the other. I am
just asking that you realize that you are conceptually really wanting a
custom Session impl. The reason I think this is important is that I have
seen many times the problems that come about down the road when we
implement things in a way that does not accurately model the problem at
hand. It's important IMO to go into these decisions "eyes wide open".
Yes, (3) is still to be covered in depth. We left that for another day.
Assuming y'all go for (b) above, IMO "transaction scope" state very nicely
fits in Hibernate's local transaction Synchrnonization registry depending
on the state you need available. To help facilitate that, I could
implement a similar concept to #getListenerOfType on the
org.hibernate.engine.transaction.spi.SynchronizationRegistry.
Also understand that ORM does not have a nice encapsulation to hook "flush
cycle scoped" state to; flush-scoped state held on your SessionEventListener
is an option.
On Tue, Apr 1, 2014 at 12:32 PM, Gunnar Morling <gunnar(a)hibernate.org>wrote:
Ok, so here is what I took from today's meeting regarding the
management
of state in Hibernate OGM:
1) For maintaining state specific to an entity (e.g. the original
store-specific representation it was loaded from, so it can be re-used
later on instead of going to the store again), ORM could be extended to
allow for custom EntityEntry (and CollectionEntry) implementations created
by a customizable factory; OGM would plug in its own implementation of that
factory (e.g. via a service)
2) For maintaining state bound to a specific session (e.g. session-level
options), we may either
a) use a custom SessionEventListener which is registered by OGM and
maintains this state; This requires a new method
SessionEventsManager#getListenerOfType(Class<T> type) in ORM which can be
used in OGM persisters etc. to access that listener and the state it
maintains
OR
b) have a specialized SessionImpl in OGM which maintains that state and
exposes it to persisters etc. This requires ORM's SessionImpl to be made
non-final
3) We did not discuss in depth OGM information bound to the transaction or
single flush cycles
Sanne said he might take a look into a POC for 1).
We need to decide for 2 a) or b), with Emmanuel's preference being a) and
Steve suggesting b) :) Both have their pro's and con's. What I like about
listeners is that they provide a nice way for passing in the information
via custom events, but I'm open to both.
3) is open for another time.
If any of this is wrong or incomplete, please correct or add.
--Gunnar
2014-04-01 16:36 GMT+02:00 Steve Ebersole <steve(a)hibernate.org>:
In my opinion, any attempts to present a custom Session ought to
> inherently mean presenting a custom SessionFactory. I assume you also
> present a OgmSessionFactory, so you should have total control over how a
> Session creation happens. At some point do you think maybe part of the
> problem is "delegating too much"? Delegation is a great pattern, but it
> has limits. I realize this is a bit simplistic, but essentially you want
> the processing to go from OgmSession->Session->OgmSession. Inherently
> there is going to need to be some level of "circular dependency" here if
> using delegation (`getSessionOwner()` is in fact a form of circular
> dependency).
>
> Really it comes down to what you want. If it were me, I'd want my
> Session (OgmSession) being the thing that gets passed to listeners and
> persisters. Heck your persisters clearly expect to be able to access the
> OgmSession or we would not be having this conversation :) But that means
> not using delegating as the basis for OgmSession, instead having OgmSession
> extend "SessionImpl". SessionImpl is final as I said, so this would mean
> either:
> 1) removing final from SessionImpl
> 2) creating a new `AbstractEventSourceSession` (name pending) that both
> SessionImpl and OgmSession (etal) can extend from
>
>
>
>
> On Tue, Apr 1, 2014 at 6:46 AM, Gunnar Morling <gunnar(a)hibernate.org>wrote:
>
>> 2014-03-31 16:28 GMT+02:00 Steve Ebersole <steve(a)hibernate.org>:
>>
>> Wasn't just me that said -1...
>>>
>>> My concerns are 2-fold:
>>> 1) You want ORM to manage and expose "state storage" on Session
even
>>> though it does not use it.
>>> 2) You want to store state in there that isnt even Session-scoped.
>>> Rather you have state that is scoped to a flush cycle, or to a
>>> transaction, etc.
>>>
>>
>> We have needs for storage with different scopes, but we never meant to
>> store all those at the session level. This discussion is about the
>> session-scoped storage only; flush cycle and TX scoped storage are
>> different pairs of shoes.
>>
>>
>>> Really, as I said in the IRC meeting when we discussed this[1], I
>>> really just want you guys to think through the use cases and flesh them
>>> out. Then in my opinion we start to see natural places to hook in state
>>> storage. For example, in the course of that IRC discussion we saw the need
>>> to store things at the transaction-level become more clear and it seemed to
>>> me like storing the state related to these transaction-scoped use-cases is
>>> just a bad idea plain and simple. You keep it relative to the transaction.
>>>
>>> To be honest I still have not seen that "fleshed out" discussion of
use
>>> cases, so its hard for me to say how state storage SHOULD be done. But I
>>> can certainly see ways that it SHOULD NOT be done.
>>>
>>> BTW, in looking through the discussion of getSessionOwner() again, I
>>> still don't agree that was the right solution for what y'all
want/need.
>>> Ultimately the problem is that SessionImpl, not your delegate, gets passed
>>> into the listeners/persisters/etc. And that happens because SessionImpl
>>> passes `this` all the time. A simple solution would be to find all the
>>> places that SessionImpl passes itself (`this`) into the places of
>>> interest (creating events, calling persisters, etc) and to instead pass the
>>> delegate via an override-able method that your delegate overriddes. For
>>> example, we have this currently on SessionImpl:
>>>
>>>
>>> @Override
>>> public void persistOnFlush(String entityName, Object object, Map
>>> copiedAlready) {
>>> firePersistOnFlush( copiedAlready, new PersistEvent( entityName,
>>> object, this ) );
>>> }
>>>
>>> but IIUC the following would solve your problems in an even better way:
>>>
>>> public EventSource getEventSource() {
>>> return this;
>>> }
>>>
>>> @Override
>>> public void persistOnFlush(String entityName, Object object, Map
>>> copiedAlready) {
>>> firePersistOnFlush( copiedAlready, new PersistEvent( entityName,
>>> object, getEventSource() ) );
>>> }
>>>
>>>
>>> and then your delegate would override this `getEventSource()` method
>>> to return itself instead.
>>>
>>> The premise here is that ultimately we'd be better served actually
>>> getting the OgmSession passed along to listeners/persisters.
>>>
>>
>> That's an interesting idea; The problem is though, when creating an
>> OgmSession, the SessionImpl instance we delegate to has already been
>> created. So we'd need a setter for passing in the OgmSession as event
>> source. Or, we'd have to create a sub-class of SessionImpl:
>>
>> public class CustomEventSourceSessionImpl extends SessionImpl {
>>
>> private SessionImpl delegate;
>> private EventSource es;
>>
>> public CustomEventSourceSessionImpl(SessionImpl delegate,
>> EventSource es) { ... }
>>
>> public EventSource getEventSource() { return es; }
>>
>> //all other methods delegate
>> }
>>
>> And in OgmSession:
>>
>> public OgmSession(OgmSessionFactory factory, EventSource delegate) {
>> super( delegate, new CustomEventSourceSessionImpl( delegate,
>> this ) );
>> ...
>> }
>>
>> In the end it this forms a circular dependency between delegator
>> (OgmSession) and delegate (SessionImpl). Not sure whether that's a good
>> thing.
>>
>> --Gunnar
>>
>>
>>
>> [1]
>>>
http://transcripts.jboss.org/meeting/irc.freenode.org/hibernate-dev/2014/...
>>>
>>>
>>>
>>> On Mon, Mar 31, 2014 at 7:31 AM, Emmanuel Bernard <
>>> emmanuel(a)hibernate.org> wrote:
>>>
>>>> The thing is, the map approach had a big -1 from Steve hanging on its
>>>> head :)
>>>>
>>>> > On 31 mars 2014, at 12:07, Gunnar Morling
<gunnar(a)hibernate.org>
>>>> wrote:
>>>> >
>>>> >
>>>> >
>>>> >
>>>> > 2014-03-20 23:05 GMT+01:00 Emmanuel Bernard
<emmanuel(a)hibernate.org
>>>> >:
>>>> >> I took some more time to think about our conversation from 2
IRC
>>>> meeting ago
>>>> >> about offering the ability to carry session bound state not
related
>>>> to
>>>> >> ORM per se.
>>>> >> Below is a sum and a potential solution.
>>>> >> If you are short on time, read Goals, then the
>>>> SessionSessionEventListener
>>>> >> approach and ignore the rest.
>>>> >>
>>>> >> ## Goals
>>>> >>
>>>> >> The goal is to be able to carry session bound state for non-ORM
>>>> projects
>>>> >> like search and OGM.
>>>> >> We want to avoid ThreadLocal use, in particular when it cannot
be
>>>> >> protected by a try / catch for proper resource cleaning.
>>>> >> We want to avoid a structure that would be shared across
threads
>>>> concurrently
>>>> >> i.e. using ConcurrentHashMap with a Weak reference to the
session.
>>>> >> It needs to be informed of a call to session.clear()
>>>> >> It needs to be informed of a call to session.close()
>>>> >> The state needs to be accessed from event listener
implementations
>>>> and custom
>>>> >> persister / loader implementations i.e. SessionImplementor and
maybe
>>>> >> EventSource?
>>>> >>
>>>> >> ## Approaches
>>>> >>
>>>> >> I'll discuss the approaches we explored in the meeting and
then
>>>> offer an
>>>> >> alternative one that I think is pretty interesting and fit
better
>>>> with
>>>> >> the current Session model.
>>>> >>
>>>> >> ### Map
>>>> >>
>>>> >> This is essentially sticking a map on SessionImpl and use it to
>>>> carry
>>>> >> state.
>>>> >> The following is a pseudo implementation
>>>> >>
>>>> >>
>>>> >> /**
>>>> >> * interface implemented by SessionImpl and the like
>>>> >> */
>>>> >> interface SessionCompanion {
>>>> >> Object getCompanion(String key);
>>>> >> void addCompanion(String key, Object companion);
>>>> >> void removeCompanion(String key);
>>>> >> }
>>>> >>
>>>> >>
>>>> >> /**
>>>> >> * adds a map to the SessionImpl
>>>> >> */
>>>> >> SessionImpl {
>>>> >> private Map<String, Object> companions;
>>>> >> public Object getCompanion(String key) { return
>>>> companions.get(key); }
>>>> >> public void addCompanion(String key, Object value) {
>>>> companions.add(key, companion); }
>>>> >> public void removeCompanion(String key) {
>>>> companions.remove(key) }
>>>> >> }
>>>> >>
>>>> >> The persister or event listener would call
>>>> SessionCompation.*Companion method
>>>> >> to put and retrieve its state.
>>>> >>
>>>> >> There is no clear / close event listener loop and it would need
to
>>>> be added.
>>>> >>
>>>> >> ### Delegator
>>>> >>
>>>> >> Gunnar and teve discussed an approach where the delegator would
be
>>>> passed to
>>>> >> the underlying session and be accessible via an `unwrap`
method.
>>>> >> I have not followed the details but this approach has one major
>>>> flaw: the
>>>> >> delegator (OgmSession, FullTextSession etc) is not always
created
>>>> and thus
>>>> >> would not be necessarily available.
>>>> >> A somewhat similar idea involving passing the session owner has
the
>>>> same
>>>> >> drawback. And another one described by Gunnar in
>>>> >>
https://hibernate.atlassian.net/browse/OGM-469
>>>> >>
>>>> >> ### The type-safe map approach
>>>> >>
>>>> >> This approach is vaguely similar to the Map approach except
that
>>>> the payload is
>>>> >> represented and looked up by Class. This has the benefit of not
>>>> having
>>>> >> namespace problems and is generally less String-y.
>>>> >>
>>>> >>
>>>> >> /**
>>>> >> * interface implemented by SessionImpl and the like
>>>> >> */
>>>> >> interface SessionCompanion {
>>>> >> T getCompanion(Class<T> type);
>>>> >> void addCompanion(Object companion);
>>>> >> void removeCompanion(Class<?> type)
>>>> >>
>>>> >> }
>>>> >>
>>>> >>
>>>> >> SessionImpl {
>>>> >> //could also use an array or an ArrayList
>>>> >> private Map<Class<?>, Object> companions;
>>>> >> public T getCompanion(Class<T> type) { return (T)
>>>> companions.get(type); }
>>>> >> public void addCompanion(Object companion) {
>>>> companions.add(companion.getClass(), type); }
>>>> >> public void removeCompanion(Class<T> type) {
>>>> companions.remove(type); }
>>>> >> }
>>>> >>
>>>> >> Like in the Map approach, the persister or custom event
listener
>>>> would interact
>>>> >> with SessionCompanion.
>>>> >> There are open issues like what should be done when two objects
of
>>>> the same
>>>> >> type are added to the same session.
>>>> >> Likewise the clear / close hook issues need to be addressed.
>>>> >>
>>>> >> ### the SessionEventListener approach
>>>> >>
>>>> >> I did not know but there is a concept of `SessionEventListener`
>>>> which can be
>>>> >> added to a `SessionImplementor`. It has hooks that are
addressing
>>>> most of the
>>>> >> goals.
>>>> >>
>>>> >>
>>>> >> //interface already exists
>>>> >> interface SessionImplementor {
>>>> >> public SessionEventListenerManager
>>>> getEventListenerManager();
>>>> >> }
>>>> >>
>>>> >> //interface already exists
>>>> >> public interface SessionEventListenerManager extends
>>>> SessionEventListener {
>>>> >> // add this method to be able to retrieve a specific
>>>> listener holding some state for a 3rd party project
>>>> >> List<SessionEventListener>
getSessionEventListeners();
>>>> >> }
>>>> >>
>>>> >> OGM or Search would implement a `SessionEventListener` and
attach
>>>> an instance to a session via `Session.addEventListeners()`.
>>>> >> It would require to add a method to retrieve the list of
>>>> `SessionEventListener`s attached to a `SessionEventListenerManager`.
>>>> >>
>>>> >> List<SessionEventListeners> listeners =
>>>>
sessionImplementor.getSessionEventListenerManager().getEnlistedListeners();
>>>> >> OgmSessionEventListener ogmListener =
>>>> findOrAddOgmListener(sessionImplementor, listeners);
>>>> >> ogmListener.someStuff();
>>>> >>
>>>> >> ## What about clear and close?
>>>> >>
>>>> >> We have a few ways to react to these.
>>>> >> SessionEventListener is already called back when a flush begins
/
>>>> ends as well as when Session closes.
>>>> >> We need to either:
>>>> >>
>>>> >> - add a clear begins / ends callback
>>>> >> - have the third party project add a ClearEventListener which
would
>>>> access the SessionEventListeners and do some magic.
>>>> >>
>>>> >> The first approach has my preference and would do:
>>>> >>
>>>> >>
>>>> >> public interface SessionEventListener {
>>>> >> [...]
>>>> >> void clearStart();
>>>> >> void clearEnd();
>>>> >> }
>>>> >>
>>>> >> What do you guys think? The SessionEventListener approach feels
>>>> more natural.
>>>> >
>>>> > Tbh. I feel maintaining the state with a listener is a bit hacky.
>>>> How would we find "our" listener, I guess it would involve some
ugly
>>>> instanceof check?
>>>> >
>>>> > The map based approaches seem preferable to me (I dislike the
>>>> "companion" naming scheme though). The type-safe map approach
is nice, as
>>>> you say it may cause type collisions though and lead to a proliferation
of
>>>> key types. How about a combined approach which provides one such
typesafe
>>>> container per "client":
>>>> >
>>>> > public interface SessionAttributes {
>>>> >
>>>> > <T> T get(Class<T> type);
>>>> > <T> T put(Class<T> type, T value);
>>>> > <T> T remove(Class<T> type);
>>>> > }
>>>> >
>>>> > And
>>>> >
>>>> > public interface SessionImplementor {
>>>> > ...
>>>> > SessionAttributes getAttributes(String integratorId);
>>>> > }
>>>> >
>>>> > And in OGM:
>>>> >
>>>> > public static final String OGM_COMMON_ATTRIBUTES =
>>>> "org.hibernate.ogm.attributes.common";
>>>> > ...
>>>> > FooBar fb =
>>>> session.getAttributes(OGM_COMMON_ATTRIBUTES).get(FooBar.class);
>>>> >
>>>> > This avoids collisions of attributes of the same type between
>>>> different clients such as OGM and Search (there could even be several
>>>> attribute containers used by one client). The SessionAttributes could be
>>>> instantiated lazily upon the first getAttributes() call, avoiding any
>>>> overhead if the functionality is not used.
>>>> >
>>>> > The clean-up part (if it is required?) could then be done by an
>>>> accompanying SessionEventListener.
>>>> >
>>>> > --Gunnar
>>>> >
>>>> >>
>>>> >> Emmanuel
>>>> >> _______________________________________________
>>>> >> hibernate-dev mailing list
>>>> >> hibernate-dev(a)lists.jboss.org
>>>> >>
https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>> >
>>>> _______________________________________________
>>>> hibernate-dev mailing list
>>>> hibernate-dev(a)lists.jboss.org
>>>>
https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>
>>>
>>>
>>
>