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