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

[Guillaume Smet]

Thanks Gail!

On Tue, Sep 12, 2017 at 8:10 AM, Gail Badner <gbadner at redhat.com> wrote:

> 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
>>>
>>
>>
>