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

[Steve Ebersole]

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