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/hibernate-dev.2014-03-11-15.46.log.html
>
>
>
> On Mon, Mar 31, 2014 at 7:31 AM, Emmanuel Bernard
> <emman...@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 <gun...@hibernate.org> wrote:
> > >
> > >
> > >
> > >
> > > 2014-03-20 23:05 GMT+01:00 Emmanuel Bernard <emman...@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@lists.jboss.org
> > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> > >
> > _______________________________________________
> > hibernate-dev mailing list
> > hibernate-dev@lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
