From 55f8409ea024dfdeb624f012ad314c14be510bb8 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Mon, 1 Feb 2010 20:13:08 +0000 Subject: [PATCH] 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 --- .../spel/ast/ConstructorReference.java | 10 ++++-- .../expression/spel/ast/MethodReference.java | 6 +++- .../spel/ConstructorInvocationTests.java | 31 ++++++++++++++++--- .../spel/MethodInvocationTests.java | 23 ++++++++++++-- .../spel/testresources/Inventor.java | 7 ++++- 5 files changed, 66 insertions(+), 11 deletions(-) diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java index f75d9d6f1a4..4bff9417230 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java +++ b/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(); - throw new SpelEvaluationException(getStartPosition(), rootCause, SpelMessage.CONSTRUCTOR_INVOCATION_PROBLEM, - typename,FormatHelper.formatMethodForMessage("", argumentTypes)); + if (rootCause instanceof RuntimeException) { + throw (RuntimeException)rootCause; + } 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 diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index a7c899511bf..98cda1ec2cc 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/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 diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java index d1a62f14d41..2e6adf3b374 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/ConstructorInvocationTests.java +++ b/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)); - - // Now cause it to throw an exception: - eContext.setVariable("bar",1); + // 4 will make it throw a checked exception - this will be wrapped by spel on the way out + eContext.setVariable("bar",4); 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 diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 07bbb20be98..1bae4c43c12 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/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() { diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index 747b09f744f..a7a7039d841 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/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(); }