An EntityPersister describes each entity you defined in your model. Its one-to-one between @Entity and EntityPersister. And really, it is totally reasonable to have the EntityPersister determine the "EntityEntry builder" to use. That also eliminates needing to develop yet-another-extension-point because if we expose it from EntityPersister OGM already swaps EntityPersister impls so this would JustWork.
On Fri, Mar 27, 2015 at 6:04 AM, John O'Hara <joh...@redhat.com> wrote: > On 26/03/15 16:22, Steve Ebersole wrote: > > The contract I suggested btw is missing one use case I know we have > discussed in regards to OGM a few times... the ability to store references > to datastore specific locators (I think the use case was to efficiently > load collections through that datastore reference). But that (obviously) > requires passing extra, specific state into the createEntityEntry method. > Not really sure the best way to handle such things tbh. > > If we did have a "creation Context" parameter passed to the > createEntityEntry, we would be instantiating more objects, however if we > went with Sanne' suggestion if caching the EnityEntryFactory with the > EntityPersister, how many objects would be actually created? I am not sure > about the relationship between Entity, EntityEntry and Session but could > this a possibility if there is a 1-1 relationship between EntityPersister > and no. of different Entities definitions? > > > On Thu, Mar 26, 2015 at 11:16 AM, Steve Ebersole <st...@hibernate.org> > wrote: > >> Apparently I hit some key combo that means send :) To finish up... >> >> On Thu, Mar 26, 2015 at 10:58 AM, Steve Ebersole <st...@hibernate.org> >> wrote: >> >>> >>> On Thu, Mar 26, 2015 at 10:20 AM, Sanne Grinovero <sa...@hibernate.org> >>> wrote: >>>> >>>> >>>> Concurrency >>>> Since the goal is to share the ImmutableEntityEntry instance among >>>> multiple threads reading it, I'd rather make sure that it is really >>>> immutable. For example it now holds onto potentially lazy initialized >>>> fields, such as getEntityKey(). >>>> If it's not possible to make it really immutable (all final fields), >>>> we'll need to make it threadsafe and question the name I'm proposing. >>>> >>> >>> The specific use-case John is interested in does indeed need to be >>> completely thread-safe and fully concurrent. >>> >>> >>>> LockMode >>>> From a logical perspective of users, one might think that an entity >>>> being "immutable" doesn't necessarily imply I can't lock on it.. >>>> right? I'm not sure how far it could be a valid use case, but these >>>> patches are implying that one can't lock an immutable entity anymore, >>>> as the lock state would be as immutable as anything else on the >>>> EntityEntry. >>>> Are we good with that? Alternatively one might need to think to >>>> separate the lock state handling from the EntityEntry. >>>> >>> >>> Conceptually there is nothing wrong with requesting a READ lock on an >>> immutable entity. But like you said, what is the logical expectation >>> there? IMO there should be none. But if we decide that it is OK to req >>> >> >> But if we decide that it is OK to request a lock on an immutable >> entity, that is problematic for the idea of a "SharedEntityEntry" or an >> "ImmutableEntityEntry" because a lock is associated with a Session which is >> what the EntityEntry is meant to model... an entity's information in >> relation to a Session. Aka, it now needs to hold state. >> >> >> >>> Notice I said immutable here and not READ_ONLY which is a specific >>> distinction which is important to other parts of your email that I will >>> address in a second email. >>> >>> >>> >>>> >>>> On smaller details: >>>> # org.hibernate.engine.internal.SharedEntityEntry is hosted in an >>>> .internal package, I don't think it's right to refactor all the public >>>> API javadoc which was referring to EntityEntry to now refer to the >>>> internal implementation. >>>> # things like EntityEntryExtraState should probably get moved to >>>> .internal packages as well now - we couldn't do that before without >>>> breaking either encapsulation or APIs. >>>> >>> >> +1 >> >> >>> >>>> In terms of git patches, the complexity of the changeset risks to get >>>> a bit our of hand. What about we focus on creating a clean pull >>>> request which focuses exclusively on making EntityEntry an interface, >>>> and move things to the right packages and javadoc? >>>> >>> >> Agree 100% >> >> >>> You'd have a trivial EntitEntryFactory, and we can then build the >>>> evolution on top of that, not least maybe helping out by challenging >>>> some points in parallel work. >>>> These are the things I'd leave for a second iteration: >>>> - add various implementations of EntityEntry iteratively, as needed >>>> - the strategy such a Factory would use the pick an implementation >>>> - ultimately, make it possible for an integrator to override such a >>>> Factory >>>> >>> >> This all seems reasonable. For 5.0 I think the most important thing >> is to nail down the idea of EntityEntry as an interface, introduce a >> Factory for building them and agree on the signature for building them. >> Granted we may need iteration to finalize the actual Factory signature, >> especially as OGM finally starts to use it, but I think that in general we >> can get it pretty close. Worst case we just pass high-level constructs >> like the EntityPersister (which OGM supplies custom impls) and the Session >> and all the "EntityEntry state". For the purpose of starting a discussion: >> >> public interface EntityEntryFactory { >> public EntityEntry createEntityEntry( >> PersistenceContext persistenceContext, >> EntityPersister persister, >> Status status, >> Serializable id, >> Object version, >> Object[] loadedState, >> Object rowId, >> LockMode lockMode, >> boolean existsInDatabase, >> boolean disableVersionIncrement, >> boolean lazyPropertiesAreUnfetched); >> } >> >> I would suggest a "creation context" method param object to pass in >> here, but seeing as how we are trying to stop instantiations... >> >> I would prefer the definition of the EntityEntryFactory to use be >> defined via SessionFactoryBuilder interface. I already have plans to have >> a auto-loaded hook for integrators to be able to adjust the >> SessionFactoryBuilder. But as a Service is fine too. >> >> >>>> For example with Hibernate OGM we might want to override / re >>>> configure the factories to use custom EntityEntry implementations - >>>> requirements are not fully clear at this point but it seems likely. >>>> >>>> The priority being to define the API as that would be a blocker for >>>> 5.0, we have then better choices to leave more smarter and advanced >>>> EntityEntry implementations for the future; we'd still need to >>>> implement at least the essential ones to make sure the API of the >>>> EntityEntryFactory has all the context it needs. >>>> >>>> Thanks, >>>> Sanne >>>> >>>> >>>> On 24 March 2015 at 09:27, John O'Hara <joh...@redhat.com> wrote: >>>> > Steve, >>>> > >>>> > Have you had chance to look at this? Do you have any >>>> comments/observations? >>>> > >>>> > Thanks >>>> > >>>> > John >>>> > >>>> > >>>> > On 17/03/15 09:24, John O'Hara wrote: >>>> > >>>> > Steve, >>>> > >>>> > I have been having a think about the EntityEntry interface, and have >>>> forked >>>> > a branch here: >>>> > >>>> > >>>> https://github.com/johnaoahra80/hibernate-orm/tree/EntityEntryInterface >>>> > >>>> > I know it is nowhere near complete, but was this the sort of idea you >>>> had in >>>> > mind? >>>> > >>>> > Thanks >>>> > >>>> > John >>>> > >>>> > >>>> > On 13/03/15 09:44, John O'Hara wrote: >>>> > >>>> > EntityEntry retains a reference to a persistenceContext internally >>>> that >>>> > org.hibernate.engine.spi.EntityEntry#setReadOnly makes calls to, is >>>> this >>>> > where the session reference is kept? As >>>> > org.hibernate.engine.spi.PersistenceConext is an interface could we >>>> have a >>>> > different implementation for this use case? e.g. an >>>> > ImmutablePersistenceContext that could be shared across sessions? >>>> > >>>> > For the bytecode enhancement, could we change the enhancer so that it >>>> adds >>>> > an EntityEntry interface with javassist. >>>> > ClassPool.javassist.ClassPool.makeInterface()() as opposed to adding >>>> a class >>>> > javassist.ClassPool.makeClass()? I need to have a look at javassit to >>>> > confirm what javassist.ClassPool.makeInterface() does. >>>> > >>>> > Thanks >>>> > >>>> > John >>>> > >>>> > On 12/03/15 18:52, Steve Ebersole wrote: >>>> > >>>> > It is possible. Although some of the changes are particularly >>>> painful. >>>> > Most of EntityEntry, if it is an interface, can be made to work with >>>> your >>>> > use case. org.hibernate.engine.spi.EntityEntry#setReadOnly I think >>>> is the >>>> > one exception, because: >>>> > 1) your use case needs it >>>> > 2) it expects the Session to be available internally (its not passed) >>>> > >>>> > The bigger thing I am worried about for you is the bytecode stuff, as >>>> that >>>> > ties very tightly with EntityEntry. >>>> > >>>> _______________________________________________ >>>> hibernate-dev mailing list >>>> hibernate-dev@lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> >>> >>> >> > > > -- > John O'harajoh...@redhat.com > > JBoss, by Red Hat > Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, > Windsor, Berkshire, SI4 1TE, United Kingdom. > Registered in UK and Wales under Company Registration No. 3798903 Directors: > Michael Cunningham (USA), Charlie Peters (USA), Matt Parsons (USA) and > Michael O'Neill (Ireland). > > > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev