BTW, if we go this route this *cannot* be backported because we do allow
users to override EntityPersister; its an extension point. Adding a new
method to extension points in bugfixes is a no-no.
On Fri, Mar 27, 2015 at 6:35 AM, Steve Ebersole <st...@hibernate.org> wrote:
> 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
