SPR-6610: don't wrap runtime exceptions thrown from methods invoked via an expression

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@2899 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java

@@ -106,9 +106,13 @@ public class ConstructorReference extends SpelNodeImpl {
 				Throwable rootCause = (causeOfAccessException==null?null:causeOfAccessException.getCause());
 				if (rootCause!=null) {
 					// User exception was the root cause - exit now
-					String typename = (String) children[0].getValueInternal(state).getValue();
+					if (rootCause instanceof RuntimeException) {
-					throw new SpelEvaluationException(getStartPosition(), rootCause, SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM,
+						throw (RuntimeException)rootCause;
-							typename,FormatHelper.formatMethodForMessage("", argumentTypes));
+					} else {
+						String typename = (String) children[0].getValueInternal(state).getValue();
+						throw new SpelEvaluationException(getStartPosition(), rootCause, SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM,
+								typename,FormatHelper.formatMethodForMessage("", argumentTypes));
+					}
 				}
 				
 				// at this point we know it wasn't a user problem so worth a retry if a better candidate can be found

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java

@@ -93,8 +93,12 @@ public class MethodReference extends SpelNodeImpl {
 				Throwable rootCause = (causeOfAccessException==null?null:causeOfAccessException.getCause());
 				if (rootCause!=null) {
 					// User exception was the root cause - exit now
-					throw new SpelEvaluationException( getStartPosition(), rootCause, SpelMessage.EXCEPTION_DURING_METHOD_INVOCATION,
+					if (rootCause instanceof RuntimeException) {
+						throw (RuntimeException)rootCause;
+					} else {
+						throw new SpelEvaluationException( getStartPosition(), rootCause, SpelMessage.EXCEPTION_DURING_METHOD_INVOCATION,
 							this.name, state.getActiveContextObject().getValue().getClass().getName(), rootCause.getMessage());
+					}
 				}
 				
 				// at this point we know it wasn't a user problem so worth a retry if a better candidate can be found

org.springframework.expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java

@@ -41,13 +41,17 @@ public class ConstructorInvocationTests extends ExpressionTestCase {
 		evaluateAndCheckError("new FooBar()",SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM);
 	}
 	
+	static class TestException extends Exception {
+		
+	}
+	
 	static class Tester {
 		public static int counter;
 		public int i;
 		
 		public Tester() {}
 		
-		public Tester(int i) {
+		public Tester(int i) throws Exception {
 			counter++;
 			if (i==1) {
 				throw new IllegalArgumentException("IllegalArgumentException for 1");
@@ -55,6 +59,9 @@ public class ConstructorInvocationTests extends ExpressionTestCase {
 			if (i==2) {
 				throw new RuntimeException("RuntimeException for 2");
 			}
+			if (i==4) {
+				throw new TestException();
+			}
 			this.i = i;
 		}
 		
@@ -100,11 +107,11 @@ public class ConstructorInvocationTests extends ExpressionTestCase {
 		Assert.assertEquals(3, o);
 		Assert.assertEquals(2,parser.parseExpression("counter").getValue(eContext));
 
-		
+		// 4 will make it throw a checked exception - this will be wrapped by spel on the way out
-		// Now cause it to throw an exception:
+		eContext.setVariable("bar",4);
-		eContext.setVariable("bar",1);
 		try {
 			o = expr.getValue(eContext);
+			Assert.fail("Should have failed");
 		} catch (Exception e) {
 			// A problem occurred whilst attempting to construct an object of type 'org.springframework.expression.spel.ConstructorInvocationTests$Tester' using arguments '(java.lang.Integer)'
 			int idx = e.getMessage().indexOf("Tester");
@@ -115,6 +122,22 @@ public class ConstructorInvocationTests extends ExpressionTestCase {
 		}
 		// If counter is 4 then the method got called twice!
 		Assert.assertEquals(3,parser.parseExpression("counter").getValue(eContext));
+		
+		
+		// 1 will make it throw a RuntimeException - SpEL will let this through
+		eContext.setVariable("bar",1);
+		try {
+			o = expr.getValue(eContext);
+			Assert.fail("Should have failed");
+		} catch (Exception e) {
+			// A problem occurred whilst attempting to construct an object of type 'org.springframework.expression.spel.ConstructorInvocationTests$Tester' using arguments '(java.lang.Integer)'
+			if (e instanceof SpelEvaluationException) {
+				e.printStackTrace();
+				Assert.fail("Should not have been wrapped");
+			}
+		}
+		// If counter is 5 then the method got called twice!
+		Assert.assertEquals(4,parser.parseExpression("counter").getValue(eContext));
 	}
 	
 	@Test

org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java

@@ -116,13 +116,32 @@ public class MethodInvocationTests extends ExpressionTestCase {
 		eContext.setVariable("bar",1);
 		try {
 			o = expr.getValue(eContext);
+			Assert.fail();
 		} catch (Exception e) {
-			e.printStackTrace();
+			if (e instanceof SpelEvaluationException) {
+				e.printStackTrace();
+				Assert.fail("Should not be a SpelEvaluationException");
+			}
 			// normal
 		}
 		// If counter is 4 then the method got called twice!
 		Assert.assertEquals(3,parser.parseExpression("counter").getValue(eContext));
-	}
+
+		eContext.setVariable("bar",4);
+		try {
+			o = expr.getValue(eContext);
+			Assert.fail();
+		} catch (Exception e) {
+			// 4 means it will throw a checked exception - this will be wrapped
+			if (!(e instanceof SpelEvaluationException)) {
+				e.printStackTrace();
+				Assert.fail("Should have been wrapped");
+			}
+			// normal
+		}
+		// If counter is 5 then the method got called twice!
+		Assert.assertEquals(4,parser.parseExpression("counter").getValue(eContext));
+}
 
 	@Test
 	public void testVarargsInvocation01() {

org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java

@@ -91,7 +91,7 @@ public class Inventor {
 		return placeOfBirth;
 	}
 	
-	public int throwException(int valueIn) {
+	public int throwException(int valueIn) throws Exception {
 		counter++;
 		if (valueIn==1) {
 			throw new IllegalArgumentException("IllegalArgumentException for 1");
@@ -99,9 +99,14 @@ public class Inventor {
 		if (valueIn==2) {
 			throw new RuntimeException("RuntimeException for 2");
 		}
+		if (valueIn==4) {
+			throw new TestException();
+		}
 		return valueIn;
 	}
 	
+	static class TestException extends Exception {}
+	
 	public String throwException(PlaceOfBirth pob) {
 		return pob.getCity();
 	}