Re: [hibernate-dev] Session and carrying 3rd party state

Gunnar, can you lead the work on getting that set of "fleshed out" use cases. With a bit of luck we could have them by tomorrow for the meeting and discuss them there.

BTW, as I said in my proposal the delegate does not work as there is not always a delegate object created.

Emmanuel On Mon 2014-03-31 9:28, Steve Ebersole wrote: > 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. > > 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. > > > [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&gt;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&gt; 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 > >