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(a)hibernate.org&gt; 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/a980ee5dca0c7a58dd79ba98... > > 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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev

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(a)hibernate.org&gt; 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/cf0fb8d262ec725a4d0692e13... > > 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(a)hibernate.org&gt; > 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(a)hibernate.org&gt; > 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/a980ee5dca0c7a58dd79ba98... > >> > >> 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(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >

On 14 September 2016 at 17:16, Steve Ebersole <steve(a)hibernate.org&gt; 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 <steve(a)hibernate.org&gt; 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 <sanne(a)hibernate.org&gt; >> 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/cf0fb8d262ec725a4d0692e13... >>> >>> 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(a)hibernate.org&gt; >>> 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(a)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/a980ee5dca0c7a58dd79ba98... >>> >> >>> >> 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(a)lists.jboss.org >>> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev