[hibernate-dev] What should be the contract for PersistenceUnitInfo#addTransformer with regard to multiple persistence units mapping the same entity class?

[Scott Marlow]

On Tue, Jun 5, 2018 at 3:38 PM, Luis Barreiro <lbarreiro at redhat.com> wrote:

> My reading on that javadoc is that you only get the transformation once
> (the first time that a class is loaded) and after that, if it's used by
> other PUs, it may not get the transformations those PUs specify.
>

We added TRACE logging for WildFly 14 that shows when the transformation
occurs.  https://paste.fedoraproject.org/paste/gna7ww8aM59sXjnAUIsEJw shows
server output for ORM 5.1.14 + 5.3.1.  I also locally added a call to
Thread.printStack()
if org.hibernate.jpa.internal.enhance.EnhancingClassTransformerImpl returns
a non-null result (which means that the passed entity class was enhanced).

The server output, shows the persistence.xml for an invalid application, so
we can see what happens if the same entity class is mapped to two
persistence units, that both enable entity enhancing.

If there is interest in discussion this, we can meet up on hipchat or irc
(or discussing here is also fine).

It looks like EnhancingClassTransformerImpl doesn't return the correct
result, if no transformation is made.  Not a serious bug but I think
we EnhancingClassTransformerImpl
should return null if the class is already transformed.

Scott

> I would like to add that the enhancer is synchronous (it only enhance one
> class at a time), hence my question about synchronization, but you were
> obviously looking at a different level.
>
> Regards,
>
> Luis Barreiro
>
> Middleware Performance Team
> <https://red.ht/sig>
> On 06/05/2018 07:08 PM, Scott Marlow wrote:
>
> Thanks, I think they probably intended that the persistence provider would
> use a marker interface like org.hibernate.engine.spi.Managed to ensure
> that the enhancement is only done once.  Responses to your questions are
> inline below.
>
> On Tue, Jun 5, 2018 at 12:19 PM, Luis Barreiro <lbarreiro at redhat.com>
> wrote:
>
>> Hi Scott,
>>
>> In the particular case of the hibernate bytecode enhancer, it adds the
>> org.hibernate.engine.spi.Managed marker interface to the entity so that
>> it only performs the enhancement once. (that implies that you can't have
>> the same entity enhanced for different features in the same class loader)
>>
>> Also, in theory, you could have multiple transformations applied to the
>> same entity / PU. So what is it you really want to check ?!
>>
> Mostly, I'm trying to understand why the [1] javadoc requires that classes
> are only transformed only once, as I would expect that some persistence
> providers could want to register multiple transformers.
>
>> I don't get you last paragraph ... why ORM will be only capable of
>> registering one transformer,
>>
> If we update the WildFly JPA container  JPADelegatingClassFileTransformer
> [2] to only allow the application entity class to be enhanced at most once
> (at runtime), ORM would be able to register multiple transformers but the
> first ORM transformer that actually enhances the entity class (e.g.
> transformer returns non-null return value), would need to terminate the
> "for each transformer" loop in [2].
>
> Since, as you said, ORM is already using a marker interface to know when a
> class is already enhanced, it sounds like WildFly doesn't really need to
> enforce the [1] contract (with regard to only transforming the entity class
> once).
>
>
>> and why will you need synchronization ?!
>>
> If WildFly JPADelegatingClassFileTransformer [2] where modified to
> prevent more than one transformation of an entity class, that would include
> checking across (parallel) persistence unit deployment threads, which could
> be done with a Java synchronization lock.
>
> Scott
>
> [1] https://docs.oracle.com/javaee/7/api/javax/persistence/spi/
> PersistenceUnitInfo.html#addTransformer-javax.persistence.spi.
> ClassTransformer-
>
> [2] https://github.com/wildfly/wildfly/blob/master/
> jpa/subsystem/src/main/java/org/jboss/as/jpa/classloader/
> JPADelegatingClassFileTransformer.java#L44
>
>
>
>