2014-03-31 16:28 GMT+02:00 Steve Ebersole <st...@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/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
