On 14 September 2016 at 17:16, Steve Ebersole <st...@hibernate.org> wrote:
> To be clear, I mean "never end the transaction locally". It's like any
> resource handling... if you start/begin/open something you should
> stop/end/close it. IMHO.
Thanks for the clarifications. I agree on the "who opens close"
principle.. I probably shouldn't be doing the commit then, if I skip
the begin?
>
>
> On Wed, Sep 14, 2016 at 11:15 AM Steve Ebersole <st...@hibernate.org> wrote:
>>
>> Yes, it was intentional. As you say it is totally reasonable.
>>
>> The problem is "matching". Like it was no problem to call begin() before
>> but what happened if the txn was commited multiple times was wonky. In fact
>> JPA explicitly forbids it multiple calls to commit (as well as multiple
>> calls to begin).
>>
>> I personally think code like this is just awful:
>>
>> Session s = ...;
>> s.accessTransaction().begin();
>> // do some stuff
>> // but never end the txn
>>
>>
>>
>> On Wed, Sep 14, 2016 at 10:59 AM Sanne Grinovero <sa...@hibernate.org>
>> wrote:
>>>
>>> Hi Steve,
>>> as a follow up of migrating #getTransaction() usage to
>>> session.accessTransaction() I now noticed a difference:
>>>
>>> we had previous code like this:
>>>
>>> Transaction transaction = session.getTransaction();
>>> transaction.begin();
>>>
>>> Which worked fine even though the user is actually using an
>>> EntityManager (the Hibernate Search integration code consistently uses
>>> the underlying Session so that it works in either case..).
>>> It also worked fine in both JTA and other transaction modes; in case
>>> of JTA w'd previously have started the transaction on the
>>> TransactionManager (as well).
>>>
>>> But the new code now requires a bit more care:
>>>
>>> Transaction transaction = session.accessTransaction();
>>> if ( transaction.isActive() == false ) {
>>> transaction.begin();
>>> }
>>>
>>> as otherwise the transaction.begin() would trigger this
>>> IllegalStateException:
>>> -
>>> https://github.com/hibernate/hibernate-orm/blob/cf0fb8d262ec725a4d0692e13d0a56d149d84584/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java#L50-L53
>>>
>>> I'll say that the new behaviour doesn't look unreasonable, but I'd
>>> like to hear from you if this was intentional as you seemed to suggest
>>> over chat that the accessTransaction() method was to be a drop-in
>>> replacement for the previous semantics of getTransaction.
>>>
>>> Secondarily, when it comes to the "transaction.commit()" I'm having no
>>> exception and it seems to work fine... should I need to check for the
>>> state?
>>>
>>> Thanks,
>>> Sanne
>>>
>>>
>>> On 13 September 2016 at 15:54, Steve Ebersole <st...@hibernate.org>
>>> wrote:
>>> > NIce! I never knew of this plugin, but there is a Gradle plugin for it
>>> > as
>>> > well.
>>> >
>>> > On Tue, Sep 13, 2016 at 9:33 AM Sanne Grinovero <sa...@hibernate.org>
>>> > wrote:
>>> >>
>>> >> Since Hibernate ORM 5.2, the method getTransaction() on Session needs
>>> >> to behave according to EntityManager spec, which implies that it has
>>> >> to throw an exception in certain circumstances which depend on the
>>> >> configuration.
>>> >>
>>> >> Hibernate Search used this method in various places, for example to
>>> >> integrate with the current transaction's events, or even to control
>>> >> the transaction explicitly in the case of the MassIndexer.
>>> >>
>>> >> Since we want Hibernate Search to work fine with Hibernate ORM no
>>> >> matter what configuration is being used, we need to avoid invoking
>>> >> this method.
>>> >> The solution is extremely simple: use its SPI level replacement, which
>>> >> is SessionImplementor#accessTransaction().
>>> >>
>>> >> Unfortunately most of our Search/ORM tests happen to run without a
>>> >> Transaction Manager so if you happen to use the old method, the tests
>>> >> would pass and everything would seem fine - however your shiny new
>>> >> feature would not work in certain configurations.
>>> >>
>>> >> One solution to verify we're not using it, is to ban this method using
>>> >> the "forbiddenapi" plugin:
>>> >> -
>>> >>
>>> >> https://github.com/Sanne/hibernate-search/commit/a980ee5dca0c7a58dd79ba98acd8a354bc5601e6#diff-600376dffeb79835ede4a0b285078036R1036
>>> >>
>>> >> A more comprehensive integration test would be to re-run all tests
>>> >> from the Search/ORM using a proper JTA configuration; not rushing to
>>> >> refactor our testsuite now since we have the forbidden-apis plugin but
>>> >> opening a JIRA task for 5.7, as this version will support ORM 5.7:
>>> >> - https://hibernate.atlassian.net/browse/HSEARCH-2344
>>> >>
>>> >> Thanks,
>>> >> Sanne
>>> >> _______________________________________________
>>> >> hibernate-dev mailing list
>>> >> hibernate-dev@lists.jboss.org
>>> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
