2014-03-31 17:09 GMT+02:00 Emmanuel Bernard <emman...@hibernate.org>:
> 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. > I've started a document with use cases at https://docs.google.com/document/d/13EOuMe8Ma6gacEyFUbH_KiNW0Yq1XKF26j-K7I6lHow/edit?usp=sharing. Any comments welcome. > 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
