[hibernate-dev] Breaking JBossStandAloneJtaPlatform backwards compatibility

[Scott Marlow]

On 05/29/2018 10:05 AM, Steve Ebersole wrote:
> On Tue, May 29, 2018 at 8:38 AM Scott Marlow <smarlow at redhat.com 
> <mailto:smarlow at redhat.com>> wrote:
> 
> 
>     On 05/29/2018 08:48 AM, Steve Ebersole wrote:
>      > By "non-jpa container managed deployments" you mean injecting
>     Hibernate
>      > Sessions?  Otherwise, I am not sure what this is supposed to
>     mean.  Nor
>      > why it is a differentiator in how we use JtaPlatform
>     / JtaPlatformInitiator.
> 
>     Applications that deploy on WF, will use the WildFly
>     JtaPlatformInitiator, unless they add a persistence unit hint
>     "wildfly.jpa.jtaplatform" set to "false", in which case the WildFly
>     JtaPlatformInitiator will not be added for the deployment.  We added
>     this for allowing applications to have more control over which
>     JtaPlatform is used (e.g. they can use an app configured JtaPlatform or
>     the determined correct ORM JtaPlatform).
> 
> 
> Well that may be how your jipijapa works.  That's not how mine works.  I 
> really wish we would keep this code in sync, or if you could use 
> hibernate-jipijapa.  Maybe you mean against 5.1?

I think we both agreed that for now, we should maintain the jipijapa 
code in WF, with a fork being kept in ORM, for testing and other 
purposes (like making a new ORM version available to already shipped WF 
versions).

I agree that we should sync our copies of this code, just not sure of 
how, other than discussing a comparison and picking which changes should 
be merged to the other copy.

> 
> Anyway, in hibernate-jipijapa I simply extend the normal 
> `JtaPlatformInitiator` and override `#getFallbackProvider`.  The way 
> that should work is that it will prefer any value explicitly set by the 
> user; but, if they do not specify anything explicitly, I use the 
> "fallback".  At least that is how it should work - if it does not, I 
> would consider that a bug and we should fix it.

Interesting, should RegionFactoryInitiator also support automatic 
"fallback" to the user configured region factory?  I assume not since we 
are enabling the second level cache on Hibernate ORM but currently are 
not able to do that in WildFly (as the Infinispan cache needs to be made 
available first).

> 
> Or if you mean against 5.1 or an earlier version... the only difference 
> is the inclusion of the explicit `#getFallbackProvider` hook - you can 
> still effect the same change simply by fully implementing 
> `JtaPlatformInitiator#initiateService`
> 
> 
>     I do see that Hibernate ORM will always succeed to use the
>     JBossAppServerJtaPlatform on WF, since we will only try using the
>     JBossStandalone if the JNDI lookup of the WF TM fails.
> 
> 
> Sure, because you are not telling it to do anything different with an 
> initiator.
> 
> 
>     What happens if the deployment, is configured to use
>     JBossStandAloneJtaPlatform directly on WF?  I assume they will need to
>     switch to use our new WildFlyStandAloneJtaPlatform class on ORM 5.3.2,
>     so that the app sees correct JTA TX status?
> 
> 
> Nope, on 5.3 hibernate-jipijapa simply works.  You have problems because 
> you are not following that paradigm.
> 
>     There is also now the problem that in 5.3.1, JBossStandAloneJtaPlatform
>     referred to the WildFly Transaction Client but is now reverted in
>     5.3.2,
>     to only refer to the Arjuna classes.  Perhaps instead in 5.3.2, we
>     should delete WildFlyStandAloneJtaPlatform and merge the logic into the
>     existing JBossStandAloneJtaPlatform (so that we first check for WildFly
>     Transaction Client classes, failing that, we then try the Arjuna
>     classes.)
> 
> 
> I agree that this is a problem and should not have been changed.  At 
> least without looking.  IMO JBossStandAloneJtaPlatform ought to look for 
> any of the classes it can use.

I reopened HHH-12640, so we can delete WildFlyStandAloneJtaPlatform and 
have JBossStandAloneJtaPlatform look for any of the classes it can use. 
https://github.com/hibernate/hibernate-orm/pull/2317 is the PR for this 
change.