@Transactional commit issue

365 views
Skip to first unread message

Armin Bahramshahry

unread,
Nov 20, 2013, 6:35:40 PM11/20/13
to google...@googlegroups.com
Hi,

I've setup Guice with Jersey, and the PersistFilter. Trying to use @Transactional annotation, and it seems to be working if the object is binded in the Singleton scope, or Request scope (from within the servlet module binding); however, objects that are just binded with the basic binding (none) they seem to have problem with @Transactional. 

Getting the EntityManager seem to work fine, etc, but nothing gets committed. 

Simply moving the binding of the object from the AbstractModule to ServletModule makes the @Transactional to work as expected. The @Transactional seem to also work for all objects that are marked as Singleton. 

Was wondering if this is an intended behavior or not, and if not, what could be causing this?

Regards,
Armin 

Stephan Classen

unread,
Nov 21, 2013, 2:52:50 AM11/21/13
to google...@googlegroups.com
The @Transactional annotation uses aop method interception (see
JpaLocalTxnInterceptor).
This means the annotation works only when all of the following is true:
- The concrete instance was created by Guice and not by anybody else
(other framework or calling new)
- the annotated method is not private
- the annotated method is not final
- the annotated method is not static

In your case I would suspect that maybe the instance was created by
Jersey (and Jersey then passes the instance to Guice for member
injection) and not by Guice itself.

If this doesn't help you may need to post some code or reduce the
problem to a minimal setup for further exploration.

regards
Stephan
> --
> You received this message because you are subscribed to the Google
> Groups "google-guice" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to google-guice...@googlegroups.com.
> To post to this group, send email to google...@googlegroups.com.
> Visit this group at http://groups.google.com/group/google-guice.
> For more options, visit https://groups.google.com/groups/opt_out.

Robert Voliva

unread,
Nov 21, 2013, 9:43:00 AM11/21/13
to google...@googlegroups.com
With Guice/Jersey, we've always had to inject a proxy/delegate class that contains all our AOP annotated methods.  As Stephan said, Jersey resource classes are already created/instrumented and Guice then no longer has the opportunity to wrap it with its AOP functionality.


On Thu, Nov 21, 2013 at 1:52 AM, Stephan Classen <st.cl...@gmx.ch> wrote:
The @Transactional annotation uses aop method interception (see JpaLocalTxnInterceptor).
This means the annotation works only when all of the following is true:
  - The concrete instance was created by Guice and not by anybody else (other framework or calling new)
  - the annotated method is not private
  - the annotated method is not final
  - the annotated method is not static

In your case I would suspect that maybe the instance was created by Jersey (and Jersey then passes the instance to Guice for member injection) and not by Guice itself.

If this doesn't help you may need to post some code or reduce the problem to a minimal setup for further exploration.

regards
Stephan




On 11/21/2013 12:35 AM, Armin Bahramshahry wrote:
Hi,

I've setup Guice with Jersey, and the PersistFilter. Trying to use @Transactional annotation, and it seems to be working if the object is binded in the Singleton scope, or Request scope (from within the servlet module binding); however, objects that are just binded with the basic binding (none) they seem to have problem with @Transactional.

Getting the EntityManager seem to work fine, etc, but nothing gets committed.

Simply moving the binding of the object from the AbstractModule to ServletModule makes the @Transactional to work as expected. The @Transactional seem to also work for all objects that are marked as Singleton.

Was wondering if this is an intended behavior or not, and if not, what could be causing this?

Regards,
Armin
--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-guice+unsubscribe@googlegroups.com.

To post to this group, send email to google...@googlegroups.com.
Visit this group at http://groups.google.com/group/google-guice.
For more options, visit https://groups.google.com/groups/opt_out.
--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-guice+unsubscribe@googlegroups.com.

Armin Bahramshahry

unread,
Nov 21, 2013, 1:07:19 PM11/21/13
to google...@googlegroups.com
The object i'm referring to is definitely being created by Guice, in fact, its being created via a Guice factory binding..., as I said, moving the binding from the AbstractModule to ServletModule without ANY other changes, and it results in the correct behavior. My understanding is that that would only specify the Scope for the object to be Scopes.Request which is fine for my case, so this is sort of a solved problem, but I'm trying to understand why, and if that's some sort of intended behavior. I'll put together a sample to post to show what i'm doing. 


On Thursday, November 21, 2013 6:43:00 AM UTC-8, Robert Voliva wrote:
With Guice/Jersey, we've always had to inject a proxy/delegate class that contains all our AOP annotated methods.  As Stephan said, Jersey resource classes are already created/instrumented and Guice then no longer has the opportunity to wrap it with its AOP functionality.
On Thu, Nov 21, 2013 at 1:52 AM, Stephan Classen <st.cl...@gmx.ch> wrote:
The @Transactional annotation uses aop method interception (see JpaLocalTxnInterceptor).
This means the annotation works only when all of the following is true:
  - The concrete instance was created by Guice and not by anybody else (other framework or calling new)
  - the annotated method is not private
  - the annotated method is not final
  - the annotated method is not static

In your case I would suspect that maybe the instance was created by Jersey (and Jersey then passes the instance to Guice for member injection) and not by Guice itself.

If this doesn't help you may need to post some code or reduce the problem to a minimal setup for further exploration.

regards
Stephan




On 11/21/2013 12:35 AM, Armin Bahramshahry wrote:
Hi,

I've setup Guice with Jersey, and the PersistFilter. Trying to use @Transactional annotation, and it seems to be working if the object is binded in the Singleton scope, or Request scope (from within the servlet module binding); however, objects that are just binded with the basic binding (none) they seem to have problem with @Transactional.

Getting the EntityManager seem to work fine, etc, but nothing gets committed.

Simply moving the binding of the object from the AbstractModule to ServletModule makes the @Transactional to work as expected. The @Transactional seem to also work for all objects that are marked as Singleton.

Was wondering if this is an intended behavior or not, and if not, what could be causing this?

Regards,
Armin
--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-guice...@googlegroups.com.

To post to this group, send email to google...@googlegroups.com.
Visit this group at http://groups.google.com/group/google-guice.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-guice...@googlegroups.com.

Armin Bahramshahry

unread,
Nov 21, 2013, 1:29:58 PM11/21/13
to google...@googlegroups.com
It's something like this right now.

public class Module extends AbstractModule {
@Override
protected void configure() {
install(new ServletModule());
}
}
 
public class ServletModule {
@Override
protected void configureServlets() {
install(new FactoryModuleBuilder().implement(H.class, X.class, XI.class)
.implement(H.class, A.class, AI.class)
.build(HFactory.class);
//... other bindings
}
 
Originally, the "install(new Factory...") section was in Module class (above "install(new ServletModule())").
 
Definitions:
public abstract class H extends Callable<Void> {
//...
@Transactional
@Override
public void call() {
//...
}
}
 
public class XI extends H
 
public class AI extends H
 
@BindingAnnotation @Target({ METHOD }) @Retention(RUNTIME)
public @interface X
 
@BindingAnnotation @Target({ METHOD }) @Retention(RUNTIME)
public @interface A

Paul Bryan

unread,
Nov 25, 2013, 12:34:51 PM11/25/13
to google...@googlegroups.com
Is moving its binding to the ServletModule implicitly changing the binding's scope to request?

Armin Bahramshahry

unread,
Dec 5, 2013, 11:36:27 AM12/5/13
to google...@googlegroups.com
So is this behavior not expected?

Stephan Classen

unread,
Dec 5, 2013, 3:56:35 PM12/5/13
to google...@googlegroups.com
So far I always bound the persistence in a ServletModule (don't knwo why, just never did it any other way).
But having read the entire persistence code at least twice I cannot remember anything about request scope.
The only piece of code related to the servlet extension is the PersistFilter which allows to have a unit of work span a request.
The transactional annotation should be independent of the unit of work (it starts its own if non is running at the time the annotated method is executed).

Also the unit tests in the persist extension are bound in abstract modules and not in servlet modules.
So I don't think what you are seeing is expected behavior.

What you could do is set a break point within the annotated method and check if the JpaLocalTxnInterceptor class is in the call stack.
If this is not the case then the aop interceptor has not picket up the annotation. Another breakpoint in the constructor would allow you to find out who is instantiating the object.

Good luck in finding the root of this issue
Stephan
--

Armin Bahramshahry

unread,
Dec 6, 2013, 2:37:51 PM12/6/13
to google...@googlegroups.com
Thanks Stephan!

Viraj Jasani

unread,
Jun 25, 2019, 1:23:07 PM6/25/19
to google-guice
Although this thread is old, I just happened to come across while trying to understands internals of guice @Transactional.

If class A has caller method that calls a public @Transactional method in it's own class, does Transactional really apply?
If class A has dependency injected of class B(Singleton), and if class A calls a public @Transactional method of class B, I believe Transaction is definitely applied. However, it seems that for above scenario(calling method of it's own class), I believe Transaction should not be applied. Could you please let me know if I am correct?

Thomas Broyer

unread,
Jun 26, 2019, 2:29:37 AM6/26/19
to google-guice


On Tuesday, June 25, 2019 at 7:23:07 PM UTC+2, Viraj Jasani wrote:
Although this thread is old, I just happened to come across while trying to understands internals of guice @Transactional.

If class A has caller method that calls a public @Transactional method in it's own class, does Transactional really apply?
If class A has dependency injected of class B(Singleton), and if class A calls a public @Transactional method of class B, I believe Transaction is definitely applied. However, it seems that for above scenario(calling method of it's own class), I believe Transaction should not be applied. Could you please let me know if I am correct?

Transactional uses Guice AOP: a subclass will be generated dynamically that overrides the annotated methods with something like:
@Override
public void foo() {
 
try {
   
// …begin transaction…
   
super.foo();
 
} catch (Exception e) {
   
if (shouldRollback(e)) { // depends on @Transactional(rollbackOn=)
     
// …rollback…
   
} else {
     
// …commit…
   
}
   
throw e;
 
}
 
// …commit…
}



Because this is a subclass, calling @Transactional methods from the same class will use a transaction. We use this to our advantage in our code using a package-private @Transactional method:
class UseCase {
 
public Result execute() {
   
// preconditions
   
try {
      doInTransaction
();
     
return Result.SUCCESS;
   
} catch (NoDataFoundException e) {
     
// …log…
     
return Result.NOT_FOUND;
   
} catch (Exception e) {
     
// …log…
     
return Result.ERROR
   
}
 
}


 
@Transactional
 
void doInTransaction() {
   
// …
 
}
}

though generally-speaking I avoid Guice Persist in new projects; if only because there's no validation that @Transactional is correctly applied and will have any effect (and we've had bugs because of that: @Transaction on private methods for example, or sometimes in classes that weren't instantiated by Guice).

Kasper Nielsen

unread,
Jun 26, 2019, 3:10:24 AM6/26/19
to google...@googlegroups.com
>
> though generally-speaking I avoid Guice Persist in new projects; if only because there's no validation that @Transactional is correctly applied and will have any effect (and we've had bugs because of that: @Transaction on private methods for example, or sometimes in classes that weren't instantiated by Guice).
Do you use an alternative implementation? Or do you manual handle the
transactions?

Best
Kasper

Thomas Broyer

unread,
Jun 26, 2019, 6:21:43 AM6/26/19
to google-guice
I'm using jOOQ (and was using guice-jooq-persist), so I just use jOOQ's transaction management methods (https://www.jooq.org/doc/3.11/manual/sql-execution/transaction-management/), that we wrap into a simple TransactionManager so the jOOQ types aren't directly exposed to our non-DAO code.
From:
doInTransaction();
// …
@Transactional void doInTransaction() { }

to:
transactionManager.transaction(this::doInTransaction);
// …
private void doInTransaction() { }

(or we could have inlined the doInTransaction method into a lambda)

Viraj Jasani

unread,
Jun 26, 2019, 6:39:19 AM6/26/19
to google-guice
This is quite surprising for me: calling @Transactional method of the same class should work. Because, I have usually observed that it doesn't work and usually I have to refactor the @Transactional method out of the class and put in some other class(which has guice @Singleton of course), and then injecting this new class to caller class just to make Transaction work. And yes, guice is giving tough time with @Transactional as there is no proper way of finding out if it is working fine(other than updating DB directly)
Reply all
Reply to author
Forward
0 new messages