[hibernate-dev] Usage of Session#getTransaction() being banned from the Hibernate Search code

[Sanne Grinovero]

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 <steve at 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 <sanne at 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 at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev