David_W (70) [Avatar] Offline
#1
It is unfortunate that you have the txAttribute.rollbackOn(ex) call.

Otherwise, you could just do
boolean ok = false;
try {
Object ret = pjp.proceed();
transactionManager.commit(status);
ok = true;
return ret;
} finally {
if (!ok) {
if (txAttribute.rollbackOn()) {
transactionManager.rollback(status);
} else {
transactionManager.commit(status);
}
}

with a little more work required to deal with an exception in the rollback or commit methods in the finally block.
ramnivas (171) [Avatar] Offline
#2
Re: Listing 14.2 forcing throws Throwable on all transactional methods
Well, rollbackOn(ex) requires the exception to decide if the transaction should be rolled back on that exception. Therefore, a catch block is needed.
David_W (70) [Avatar] Offline
#3
Re: Listing 14.2 forcing throws Throwable on all transactional methods
What I meant to say is that, since the around advice declares that it throws a Throwable, that should require that the advice can only be applied to methods whose signature also declares that it throws Throwable.
ramnivas (171) [Avatar] Offline
#4
Re: Listing 14.2 forcing throws Throwable on all transactional methods
This arrangement won't force that advised methods throw Thowable or any other exception. Advice, although declared to throw Throwable, should not throw any incompatible exception. For example, it would be wrong for the transaction management advice to throw IOException, unless all advised methods also declare that they throw that exception. In our case, we throw only runtime exceptions, so that won't pose any problem.
David_W (70) [Avatar] Offline
#5
Re: Listing 14.2 forcing throws Throwable on all transactional methods
Using AJDT in Eclipse, create this class and aspect:

public class Work {
public void doWork(String str) {
System.out.println("doWork:"+str);
}
}

public aspect TestThrow {
public pointcut inWork():execution(* Work.*(..));

Object around() throws Throwable:inWork() {
try {
return proceed();
} catch(Throwable ex) {
ex.printStackTrace();
throw ex;
}
}
}

AJDT won't compile this - error message is
can't throw checked exception 'java.lang.Throwable' at this join point 'method-execution(void com.david.Work.doWork(java.lang.String))'

If you remove the 'throws Throwable' in the advice, the error message in the aspect is
Unhandled exception type Throwable
ramnivas (171) [Avatar] Offline
#6
Re: Listing 14.2 forcing throws Throwable on all transactional methods
That is (an additional) reason I am using @AspectJ (besides Spring AOP compatiblity). @AspectJ is lenient in this regard. It won't have any issue with the code equivalent to yours.

As for traditional syntax, there is a feature request to allow the "rethrow" construct that will bridge the gap. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=240608 and http://dev.eclipse.org/mhonarc/lists/aspectj-dev/msg02387.html for some discussion (which not coincidentally uses the transaction management as an example).
David_W (70) [Avatar] Offline
#7
Re: Listing 14.2 forcing throws Throwable on all transactional methods
To avoid this inconsistency, why not use before, after returning, and after throwing advice?
ramnivas (171) [Avatar] Offline
#8
Re: Listing 14.2 forcing throws Throwable on all transactional methods
I did consider that. In fact, that is how I do it in org.springframework.transaction.aspectj.AbstractTransactionAspect. However, in that case (and if I were to do the same in the book), I need a thread local to pass the TransactionStatus obtained in before to after returning/after throwing. Further that thread local will need a stack to consider nested transactions. In the end, I decided to go for simpler @AspectJ along with around().
David_W (70) [Avatar] Offline
#9
Re: Listing 14.2 forcing throws Throwable on all transactional methods
I found something interesting.

public class App {
public static void main(String[] args) {
Work w = new Work();
try {
String val = w.doWork("abc");
System.out.println("returned "+val);
} catch (Exception ex) {
System.out.println("caught it: "+ex);
}
System.out.println("ran it");
}
}
package com.david;

public class Work {
public String doWork(String str) {
System.out.println("doWork:"+str);
if (str==null) {
throw new IllegalArgumentException("I did it");
}
return str+"aaa";
}
}
package com.david.aspects;

import java.io.IOException;

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;

@Aspect
public class TestAnnAspect {

@Pointcut("execution(* com.david.Work.*(..))")
public void inWork() {}

@Around("inWork()")
public String doingWork(ProceedingJoinPoint jp) throws Throwable {
System.out.println("in doing work");
String value = (String)jp.proceed();
if (value.equals("abcaaa")) {
throw new IOException("fake error");
}
return value;
}
}

Output is:
in doing work
doWork:abc
caught it: java.io.IOException: fake error
ran it


This shows that the around advice applied to a method that declares no checked exceptions is able to throw an IOException. Sounds like a bug.
ramnivas (171) [Avatar] Offline
#10
Re: Listing 14.2 forcing throws Throwable on all transactional methods
Yep, it is a known bug in AspectJ (https://bugs.eclipse.org/bugs/show_bug.cgi?id=284301).