Skip to content

Commit

Permalink
Transaction annotations on interface methods with CGLIB proxies
Browse files Browse the repository at this point in the history
Issue: SPR-14322
  • Loading branch information
jhoeller committed Apr 26, 2017
1 parent 4a8c99c commit 42d6d7e
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,10 @@

package org.springframework.transaction.aspectj;

import java.lang.reflect.Method;

import org.junit.Before;
import org.junit.Test;

import org.springframework.tests.transaction.CallCountingTransactionManager;
import org.springframework.transaction.annotation.AnnotationTransactionAttributeSource;
import org.springframework.transaction.interceptor.TransactionAttribute;

import static org.junit.Assert.*;

Expand Down Expand Up @@ -65,55 +61,50 @@ public void testCommitOnAnnotatedClass() throws Throwable {
}

@Test
public void testCommitOnAnnotatedProtectedMethod() throws Throwable {
public void commitOnAnnotatedProtectedMethod() throws Throwable {
txManager.clear();
assertEquals(0, txManager.begun);
beanWithAnnotatedProtectedMethod.doInTransaction();
assertEquals(1, txManager.commits);
}

@Test
public void testCommitOnAnnotatedPrivateMethod() throws Throwable {
public void commitOnAnnotatedPrivateMethod() throws Throwable {
txManager.clear();
assertEquals(0, txManager.begun);
beanWithAnnotatedPrivateMethod.doSomething();
assertEquals(1, txManager.commits);
}

@Test
public void testNoCommitOnNonAnnotatedNonPublicMethodInTransactionalType() throws Throwable {
public void commitOnNonAnnotatedNonPublicMethodInTransactionalType() throws Throwable {
txManager.clear();
assertEquals(0,txManager.begun);
assertEquals(0, txManager.begun);
annotationOnlyOnClassWithNoInterface.nonTransactionalMethod();
assertEquals(0,txManager.begun);
assertEquals(0, txManager.begun);
}

@Test
public void testCommitOnAnnotatedMethod() throws Throwable {
public void commitOnAnnotatedMethod() throws Throwable {
txManager.clear();
assertEquals(0, txManager.begun);
methodAnnotationOnly.echo(null);
assertEquals(1, txManager.commits);
}

@Test
public void testNotTransactional() throws Throwable {
public void notTransactional() throws Throwable {
txManager.clear();
assertEquals(0, txManager.begun);
new NotTransactional().noop();
assertEquals(0, txManager.begun);
}

@Test
public void testDefaultCommitOnAnnotatedClass() throws Throwable {
public void defaultCommitOnAnnotatedClass() throws Throwable {
final Exception ex = new Exception();
try {
testRollback(new TransactionOperationCallback() {
@Override
public Object performTransactionalOperation() throws Throwable {
return annotationOnlyOnClassWithNoInterface.echo(ex);
}
}, false);
testRollback(() -> annotationOnlyOnClassWithNoInterface.echo(ex), false);
fail("Should have thrown Exception");
}
catch (Exception ex2) {
Expand All @@ -122,15 +113,10 @@ public Object performTransactionalOperation() throws Throwable {
}

@Test
public void testDefaultRollbackOnAnnotatedClass() throws Throwable {
public void defaultRollbackOnAnnotatedClass() throws Throwable {
final RuntimeException ex = new RuntimeException();
try {
testRollback(new TransactionOperationCallback() {
@Override
public Object performTransactionalOperation() throws Throwable {
return annotationOnlyOnClassWithNoInterface.echo(ex);
}
}, true);
testRollback(() -> annotationOnlyOnClassWithNoInterface.echo(ex), true);
fail("Should have thrown RuntimeException");
}
catch (RuntimeException ex2) {
Expand All @@ -139,15 +125,10 @@ public Object performTransactionalOperation() throws Throwable {
}

@Test
public void testDefaultCommitOnSubclassOfAnnotatedClass() throws Throwable {
public void defaultCommitOnSubclassOfAnnotatedClass() throws Throwable {
final Exception ex = new Exception();
try {
testRollback(new TransactionOperationCallback() {
@Override
public Object performTransactionalOperation() throws Throwable {
return new SubclassOfClassWithTransactionalAnnotation().echo(ex);
}
}, false);
testRollback(() -> new SubclassOfClassWithTransactionalAnnotation().echo(ex), false);
fail("Should have thrown Exception");
}
catch (Exception ex2) {
Expand All @@ -156,15 +137,10 @@ public Object performTransactionalOperation() throws Throwable {
}

@Test
public void testDefaultCommitOnSubclassOfClassWithTransactionalMethodAnnotated() throws Throwable {
public void defaultCommitOnSubclassOfClassWithTransactionalMethodAnnotated() throws Throwable {
final Exception ex = new Exception();
try {
testRollback(new TransactionOperationCallback() {
@Override
public Object performTransactionalOperation() throws Throwable {
return new SubclassOfClassWithTransactionalMethodAnnotation().echo(ex);
}
}, false);
testRollback(() -> new SubclassOfClassWithTransactionalMethodAnnotation().echo(ex), false);
fail("Should have thrown Exception");
}
catch (Exception ex2) {
Expand All @@ -173,41 +149,19 @@ public Object performTransactionalOperation() throws Throwable {
}

@Test
public void testDefaultCommitOnImplementationOfAnnotatedInterface() throws Throwable {
public void noCommitOnImplementationOfAnnotatedInterface() throws Throwable {
final Exception ex = new Exception();
testNotTransactional(new TransactionOperationCallback() {
@Override
public Object performTransactionalOperation() throws Throwable {
return new ImplementsAnnotatedInterface().echo(ex);
}
}, ex);
testNotTransactional(() -> new ImplementsAnnotatedInterface().echo(ex), ex);
}

/**
* Note: resolution does not occur. Thus we can't make a class transactional if
* it implements a transactionally annotated interface. This behavior could only
* be changed in AbstractFallbackTransactionAttributeSource in Spring proper.
* See SPR-14322.
*/
@Test
public void testDoesNotResolveTxAnnotationOnMethodFromClassImplementingAnnotatedInterface() throws Exception {
AnnotationTransactionAttributeSource atas = new AnnotationTransactionAttributeSource();
Method method = ImplementsAnnotatedInterface.class.getMethod("echo", Throwable.class);
TransactionAttribute ta = atas.getTransactionAttribute(method, ImplementsAnnotatedInterface.class);
assertNull(ta);
}

@Test
public void testDefaultRollbackOnImplementationOfAnnotatedInterface() throws Throwable {
public void noRollbackOnImplementationOfAnnotatedInterface() throws Throwable {
final Exception rollbackProvokingException = new RuntimeException();
testNotTransactional(new TransactionOperationCallback() {
@Override
public Object performTransactionalOperation() throws Throwable {
return new ImplementsAnnotatedInterface().echo(rollbackProvokingException);
}
}, rollbackProvokingException);
testNotTransactional(() -> new ImplementsAnnotatedInterface().echo(rollbackProvokingException),
rollbackProvokingException);
}


protected void testRollback(TransactionOperationCallback toc, boolean rollback) throws Throwable {
txManager.clear();
assertEquals(0, txManager.begun);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -150,12 +150,10 @@ protected TransactionAttribute findTransactionAttribute(Class<?> clazz) {
* or {@code null} if none was found
*/
protected TransactionAttribute determineTransactionAttribute(AnnotatedElement ae) {
if (ae.getAnnotations().length > 0) {
for (TransactionAnnotationParser annotationParser : this.annotationParsers) {
TransactionAttribute attr = annotationParser.parseTransactionAnnotation(ae);
if (attr != null) {
return attr;
}
for (TransactionAnnotationParser annotationParser : this.annotationParsers) {
TransactionAttribute attr = annotationParser.parseTransactionAnnotation(ae);
if (attr != null) {
return attr;
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -39,7 +39,8 @@ public class SpringTransactionAnnotationParser implements TransactionAnnotationP

@Override
public TransactionAttribute parseTransactionAnnotation(AnnotatedElement ae) {
AnnotationAttributes attributes = AnnotatedElementUtils.getMergedAnnotationAttributes(ae, Transactional.class);
AnnotationAttributes attributes = AnnotatedElementUtils.findMergedAnnotationAttributes(
ae, Transactional.class, false, false);
if (attributes != null) {
return parseTransactionAnnotation(attributes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.util.Collection;
import java.util.Map;

import org.junit.Ignore;
import org.junit.Test;

import org.springframework.aop.support.AopUtils;
Expand Down Expand Up @@ -166,7 +165,7 @@ public void spr14322FindsOnInterfaceWithInterfaceProxy() {
ctx.close();
}

@Test @Ignore // TODO
@Test
public void spr14322FindsOnInterfaceWithCglibProxy() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Spr14322ConfigB.class);
TransactionalTestInterface bean = ctx.getBean(TransactionalTestInterface.class);
Expand Down

0 comments on commit 42d6d7e

Please sign in to comment.