Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

SPR-9335: catch exceptions explicitly where possible #162

Open
wants to merge 1 commit into from

2 participants

Dave Syer Chris Beams
Dave Syer

This change restores the old behaviour in Spring 3.0, as near
as I can tell, without breaking the Groovy aspect changes
introduced in 3.1.

Since no-one has come up with a test case for all the catch blocks
and there is no test project for the WAS case (I tried) we have to
rely on the users in the JIRA thread to test this change.

Dave Syer SPR-9335: catch exceptions explicitly where possible
This change restores the old behaviour in Spring 3.0, as near
as I can tell, without breaking the Groovy aspect changes
introduced in 3.1.
4e1552c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Oct 09, 2012
Dave Syer SPR-9335: catch exceptions explicitly where possible
This change restores the old behaviour in Spring 3.0, as near
as I can tell, without breaking the Groovy aspect changes
introduced in 3.1.
4e1552c
This page is out of date. Refresh to see the latest.
28  spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java
@@ -29,7 +29,6 @@
29 29
 import org.apache.commons.logging.LogFactory;
30 30
 import org.aspectj.weaver.BCException;
31 31
 import org.aspectj.weaver.patterns.NamePattern;
32  
-import org.aspectj.weaver.reflect.ReflectionWorld;
33 32
 import org.aspectj.weaver.reflect.ReflectionWorld.ReflectionWorldException;
34 33
 import org.aspectj.weaver.reflect.ShadowMatchImpl;
35 34
 import org.aspectj.weaver.tools.ContextBasedMatcher;
@@ -355,7 +354,17 @@ protected String getCurrentProxiedBeanName() {
355 354
 	private PointcutExpression getFallbackPointcutExpression(
356 355
 			Class<?> targetClass) {
357 356
 		ClassLoader classLoader = targetClass.getClassLoader();
358  
-		return classLoader == null ? this.pointcutExpression : buildPointcutExpression(classLoader);
  357
+		PointcutExpression result = null;
  358
+		try {
  359
+			result = classLoader == null ? this.pointcutExpression : buildPointcutExpression(classLoader);
  360
+		}
  361
+		catch (BCException e) {
  362
+			throw e;
  363
+		}
  364
+		catch (Exception e) {
  365
+			logger.debug("Failed to build fallback pointcut", e);
  366
+		}
  367
+		return result==null ? this.pointcutExpression : result;
359 368
 	}
360 369
 
361 370
 	/**
@@ -402,13 +411,18 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
402 411
 					try {
403 412
 						shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod);
404 413
 					}
405  
-					catch (ReflectionWorld.ReflectionWorldException ex) {
  414
+					catch (ReflectionWorldException ex) {
406 415
 						// Failed to introspect target method, probably because it has been loaded
407 416
 						// in a special ClassLoader. Let's try the original method instead...
408 417
 						try {
409  
-							fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
  418
+							try {
  419
+								fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
  420
+							} catch (BCException e) {
  421
+								// In #matches() we catch this explicitly so we should do the same to be safe
  422
+								throw new ReflectionWorldException("Failed to build fallback: " + e.getMessage());
  423
+							} 
410 424
 							shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
411  
-						} catch (ReflectionWorld.ReflectionWorldException e) {
  425
+						} catch (ReflectionWorldException e) {
412 426
 							if (targetMethod == originalMethod) {
413 427
 								shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
414 428
 							}
@@ -416,14 +430,14 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
416 430
 								try {
417 431
 									shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
418 432
 								}
419  
-								catch (ReflectionWorld.ReflectionWorldException ex2) {
  433
+								catch (ReflectionWorldException ex2) {
420 434
 									// Could neither introspect the target class nor the proxy class ->
421 435
 									// let's simply consider this method as non-matching.
422 436
 									methodToMatch = originalMethod;
423 437
 									fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
424 438
 									try {
425 439
 										shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
426  
-									} catch (ReflectionWorld.ReflectionWorldException e2) {
  440
+									} catch (ReflectionWorldException e2) {
427 441
 										shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
428 442
 									}
429 443
 								}
52  spring-aop/src/test/java/org/springframework/aop/aspectj/TrickyAspectJPointcutExpressionTests.java
@@ -67,10 +67,10 @@ public void testManualProxyJavaWithStaticPointcutAndTwoClassLoaders() throws Exc
67 67
 
68 68
 		// Test with default class loader first...
69 69
 		testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, new TestServiceImpl(), "TestServiceImpl");
70  
-		
  70
+
71 71
 		// Then try again with a different class loader on the target...
72  
-		SimpleThrowawayClassLoader loader = new SimpleThrowawayClassLoader(new TestServiceImpl().getClass().getClassLoader());
73  
-		// Make sure the interface is loaded from the  parent class loader
  72
+		SimpleThrowawayClassLoader loader = new SimpleThrowawayClassLoader(getClass().getClassLoader());
  73
+		// Make sure the interface is loaded from the parent class loader
74 74
 		loader.excludeClass(TestService.class.getName());
75 75
 		loader.excludeClass(TestException.class.getName());
76 76
 		TestService other = (TestService) loader.loadClass(TestServiceImpl.class.getName()).newInstance();
@@ -83,26 +83,32 @@ private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService ta
83 83
 		testAdvice(advisor, logAdvice, target, message, false);
84 84
 	}
85 85
 
86  
-	private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message,
  86
+	private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, Object target, String message,
87 87
 			boolean proxyTargetClass) throws Exception {
  88
+		testAdvice(advisor, logAdvice, target, message, proxyTargetClass, getClass().getClassLoader());
  89
+	}
  90
+
  91
+	private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, Object target, String message,
  92
+			boolean proxyTargetClass, ClassLoader loader) throws Exception {
88 93
 
89 94
 		logAdvice.reset();
90 95
 
91 96
 		ProxyFactory factory = new ProxyFactory(target);
92 97
 		factory.setProxyTargetClass(proxyTargetClass);
93 98
 		factory.addAdvisor(advisor);
94  
-		TestService bean = (TestService) factory.getProxy();
  99
+		TestService bean = (TestService) factory.getProxy(loader);
95 100
 
96 101
 		assertEquals(0, logAdvice.getCountThrows());
97 102
 		try {
98 103
 			bean.sayHello();
99 104
 			fail("Expected exception");
100  
-		} catch (TestException e) {
  105
+		}
  106
+		catch (TestException e) {
101 107
 			assertEquals(message, e.getMessage());
102 108
 		}
103 109
 		assertEquals(1, logAdvice.getCountThrows());
104 110
 	}
105  
-	
  111
+
106 112
 	public static class SimpleThrowawayClassLoader extends OverridingClassLoader {
107 113
 
108 114
 		/**
@@ -114,7 +120,7 @@ public SimpleThrowawayClassLoader(ClassLoader parent) {
114 120
 		}
115 121
 
116 122
 	}
117  
-	
  123
+
118 124
 	public static class TestException extends RuntimeException {
119 125
 
120 126
 		public TestException(String string) {
@@ -129,32 +135,32 @@ public TestException(String string) {
129 135
 	@Inherited
130 136
 	public static @interface Log {
131 137
 	}
132  
-	
  138
+
133 139
 	public static interface TestService {
134  
-	    public String sayHello();
  140
+		public String sayHello();
135 141
 	}
136  
-	
  142
+
137 143
 	@Log
138  
-	public static class TestServiceImpl implements TestService{
139  
-	    public String sayHello() {
140  
-	        throw new TestException("TestServiceImpl");
141  
-	    }
  144
+	public static class TestServiceImpl implements TestService {
  145
+		public String sayHello() {
  146
+			throw new TestException("TestServiceImpl");
  147
+		}
142 148
 	}
143 149
 
144 150
 	public class LogUserAdvice implements MethodBeforeAdvice, ThrowsAdvice {
145  
-		
  151
+
146 152
 		private int countBefore = 0;
147  
-		
  153
+
148 154
 		private int countThrows = 0;
149  
-		
  155
+
150 156
 		public void before(Method method, Object[] objects, Object o) throws Throwable {
151 157
 			countBefore++;
152  
-	    }
  158
+		}
153 159
 
154 160
 		public void afterThrowing(Exception e) throws Throwable {
155 161
 			countThrows++;
156  
-	        throw e;
157  
-	    }
  162
+			throw e;
  163
+		}
158 164
 
159 165
 		public int getCountBefore() {
160 166
 			return countBefore;
@@ -163,12 +169,12 @@ public int getCountBefore() {
163 169
 		public int getCountThrows() {
164 170
 			return countThrows;
165 171
 		}
166  
-		
  172
+
167 173
 		public void reset() {
168 174
 			countThrows = 0;
169 175
 			countBefore = 0;
170 176
 		}
171 177
 
172 178
 	}
173  
-	
  179
+
174 180
 }
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.