[hibernate-dev] ORM 5.2 issue with getInterceptor() not being a true getter

[Gail Badner]

I created https://hibernate.atlassian.net/browse/HHH-11982.

I just committed the one-line change without a unit test. Unless I hear
otherwise, I'll assume that's OK and I'll plan to release on Wednesday,
9/13.

Regards,
Gail

On Mon, Sep 11, 2017 at 10:52 PM, Gail Badner <gbadner at redhat.com> wrote:

> OK, I'll fix it for 5.2.11.
>
> Thanks,
> Gail
>
> On Mon, Sep 11, 2017 at 12:20 PM, Steve Ebersole <steve at hibernate.org>
> wrote:
>
>> I'm fine with that.  That method really does not fit with the paradigm
>> which needs that pulse.
>>
>>
>> On Mon, Sep 11, 2017 at 1:36 PM Sanne Grinovero <sanne at hibernate.org>
>> wrote:
>>
>> > On 11 September 2017 at 16:14, Guillaume Smet <guillaume.smet at gmail.com
>> >
>> > wrote:
>> > > Hi!
>> > >
>> > > Any comment on this?
>> > >
>> > > After reading the javadoc of SharedSessionContractImplementor, I
>> think we
>> > > should probably just get rid of the `checkTransactionSynchStatus();`
>> call
>> > > in getInterceptor().
>> > >
>> > > I don't think getInterceptor() should be responsible for joining the
>> > > transaction.
>> > >
>> > > What do you all think?
>> >
>> > +1
>> >
>> > >
>> > > --
>> > > Guillaume
>> > >
>> > > On Fri, Sep 8, 2017 at 6:26 PM, Guillaume Smet <
>> guillaume.smet at gmail.com
>> > >
>> > > wrote:
>> > >
>> > >> Hi,
>> > >>
>> > >> Note to Gail: this is potentially a blocking issue for 5.2.11 for the
>> > OGM
>> > >> upgrade (I thought it was a bug in OGM but apparently, it's an issue
>> > with
>> > >> ORM).
>> > >>
>> > >> In 5.2 SessionImpl, we now use getInterceptor() instead of accessing
>> the
>> > >> interceptor field directly because the field has been moved to
>> > >> AbstractSharedSessionContract.
>> > >>
>> > >> See for instance the change made here in
>> afterTransactionCompletion():
>> > >> https://github.com/hibernate/hibernate-orm/blame/master/
>> > >>
>> > hibernate-core/src/main/java/org/hibernate/internal/SessionI
>> mpl.java#L2443
>> > >>
>> > >> I think this is an issue as getInterceptor() is not a simple getter
>> but
>> > is:
>> > >> @Override
>> > >> public Interceptor getInterceptor() {
>> > >>     checkTransactionSynchStatus();
>> > >>     return interceptor;
>> > >> }
>> > >>
>> > >> protected void checkTransactionSynchStatus() {
>> > >>     pulseTransactionCoordinator();
>> > >>     delayedAfterCompletion();
>> > >> }
>> > >>
>> > >> Thus calling the pulse() method of the TransactionCoordinator,
>> > triggering
>> > >> an implicit join whereas we're in the afterTransactionCompletion()
>> > phase.
>> > >>
>> > >> This is an issue for us as the pulse() method of our
>> > >> TransactionCoordinator creates Neo4j transactions so when the
>> > >> getInterceptor() method is called in afterTransactionCompletion(), we
>> > >> create a new Neo4j transaction.
>> > >>
>> > >> So 2 questions:
>> > >> - should we really call checkTransactionSynchStatus(); in
>> > >> getInterceptor()? If feels a bit weird.
>> > >> - if so, I think we should have a true protected getter
>> (interceptor()
>> > >> following Steve's convention?) to avoid SessionImpl "pulsing" the
>> > >> transaction coordinator when accessing the interceptor.
>> > >>
>> > >> Thanks for your feedback!
>> > >>
>> > >> --
>> > >> Guillaume
>> > >>
>> > > _______________________________________________
>> > > hibernate-dev mailing list
>> > > hibernate-dev at lists.jboss.org
>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> > _______________________________________________
>> > hibernate-dev mailing list
>> > hibernate-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>
>