From 5306f31bd3b37cd0a8fbb03051d9a55b0d936176 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Tue, 18 Dec 2018 11:13:53 +0100 Subject: [PATCH 1/2] Only wrap exceptions when necessary in ArC subclassing This way, the proxy has an exception handling as similar as possible to the original class. Fixes #329. --- .../arc/processor/SubclassGenerator.java | 35 +++++++- .../ExceptionHandlingBean.java | 53 +++++++++++ .../ExceptionHandlingCase.java | 23 +++++ .../ExceptionHandlingInterceptor.java | 36 ++++++++ .../ExceptionHandlingInterceptorBinding.java | 34 ++++++++ .../InterceptorExceptionHandlingTest.java | 87 +++++++++++++++++++ .../MyDeclaredException.java | 20 +++++ .../exceptionhandling/MyOtherException.java | 20 +++++ .../exceptionhandling/MyRuntimeException.java | 20 +++++ 9 files changed, 324 insertions(+), 4 deletions(-) create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingBean.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingCase.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptor.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptorBinding.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/InterceptorExceptionHandlingTest.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyDeclaredException.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyOtherException.java create mode 100644 ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyRuntimeException.java diff --git a/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java b/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java index 41f4b86d9cde2..dcd39223117aa 100644 --- a/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java +++ b/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java @@ -65,6 +65,9 @@ */ public class SubclassGenerator extends AbstractGenerator { + private static final DotName JAVA_LANG_EXCEPTION = DotName.createSimple(Exception.class.getName()); + private static final DotName JAVA_LANG_RUNTIME_EXCEPTION = DotName.createSimple(RuntimeException.class.getName()); + static final String SUBCLASS_SUFFIX = "_Subclass"; private final Predicate applicationClassPredicate; @@ -264,14 +267,38 @@ private void createForwardingMethod(ClassOutput classOutput, BeanInfo bean, Meth method.parameters().stream().map(p -> p.name().toString()).collect(Collectors.toList()).toArray(new String[0])), forwardMethod.getThis(), superParamHandles); funcBytecode.returnValue(superResult != null ? superResult : funcBytecode.loadNull()); + for (Type declaredException : method.exceptions()) { + forwardMethod.addException(declaredException.name().toString()); + } // InvocationContext // (java.lang.String) InvocationContextImpl.aroundInvoke(this, methods.get("m1"), params, interceptorChains.get("m1"), forward).proceed() TryBlock tryCatch = forwardMethod.tryBlock(); - // catch (Exception e) - CatchBlockCreator exception = tryCatch.addCatch(Exception.class); - // throw new RuntimeException(e) - exception.throwException(RuntimeException.class, "Error invoking subclass", exception.getCaughtException()); + // catch exceptions declared on the original method + boolean addCatchRuntimeException = true; + boolean addCatchException = true; + for (Type declaredException : method.exceptions()) { + CatchBlockCreator catchDeclaredException = tryCatch.addCatch(declaredException.name().toString()); + catchDeclaredException.throwException(catchDeclaredException.getCaughtException()); + + if (JAVA_LANG_RUNTIME_EXCEPTION.equals(declaredException.name())) { + addCatchRuntimeException = false; + } + if (JAVA_LANG_EXCEPTION.equals(declaredException.name())) { + addCatchException = false; + } + } + // catch (RuntimeException e) if not already caught + if (addCatchRuntimeException) { + CatchBlockCreator catchRuntimeException = tryCatch.addCatch(RuntimeException.class); + catchRuntimeException.throwException(catchRuntimeException.getCaughtException()); + } + // now catch the rest (Exception e) if not already caught + if (addCatchException) { + CatchBlockCreator catchOtherExceptions = tryCatch.addCatch(Exception.class); + // and wrap them in a new RuntimeException(e) + catchOtherExceptions.throwException(RuntimeException.class, "Error invoking subclass method", catchOtherExceptions.getCaughtException()); + } // InvocationContextImpl.aroundInvoke(this, methods.get("m1"), params, interceptorChains.get("m1"), forward) ResultHandle methodIdHandle = tryCatch.load(methodId); ResultHandle interceptedMethodHandle = tryCatch.invokeInterfaceMethod(MethodDescriptors.MAP_GET, diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingBean.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingBean.java new file mode 100644 index 0000000000000..a5b3241ec99f5 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingBean.java @@ -0,0 +1,53 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +import javax.enterprise.context.Dependent; + +@Dependent +public class ExceptionHandlingBean { + + public ExceptionHandlingBean() { + } + + @ExceptionHandlingInterceptorBinding + void foo(ExceptionHandlingCase exceptionHandlingCase) throws MyDeclaredException { + switch (exceptionHandlingCase) { + case DECLARED_EXCEPTION: + throw new MyDeclaredException(); + case RUNTIME_EXCEPTION: + throw new MyRuntimeException(); + case OTHER_EXCEPTIONS: + // this case should be handled by the interceptor + break; + } + } + + @ExceptionHandlingInterceptorBinding + void bar() throws Exception { + throw new Exception(); + } + + @ExceptionHandlingInterceptorBinding + void baz() throws RuntimeException { + throw new RuntimeException(); + } + + Integer fooNotIntercepted() { + return 1; + } +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingCase.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingCase.java new file mode 100644 index 0000000000000..ac6b1b846b969 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingCase.java @@ -0,0 +1,23 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +public enum ExceptionHandlingCase { + DECLARED_EXCEPTION, + RUNTIME_EXCEPTION, + OTHER_EXCEPTIONS; +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptor.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptor.java new file mode 100644 index 0000000000000..eab7c28146f01 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptor.java @@ -0,0 +1,36 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +import javax.annotation.Priority; +import javax.interceptor.AroundInvoke; +import javax.interceptor.Interceptor; +import javax.interceptor.InvocationContext; + +@ExceptionHandlingInterceptorBinding +@Priority(1) +@Interceptor +public class ExceptionHandlingInterceptor { + + @AroundInvoke + Object intercept(InvocationContext ctx) throws Exception { + if (ctx.getParameters()[0] == ExceptionHandlingCase.OTHER_EXCEPTIONS) { + throw new MyOtherException(); + } + return ctx.proceed(); + } +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptorBinding.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptorBinding.java new file mode 100644 index 0000000000000..2360b8369f59d --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/ExceptionHandlingInterceptorBinding.java @@ -0,0 +1,34 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import javax.interceptor.InterceptorBinding; + +@Target({ TYPE, METHOD }) +@Retention(RUNTIME) +@Documented +@InterceptorBinding +public @interface ExceptionHandlingInterceptorBinding { +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/InterceptorExceptionHandlingTest.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/InterceptorExceptionHandlingTest.java new file mode 100644 index 0000000000000..4a9b078d14319 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/InterceptorExceptionHandlingTest.java @@ -0,0 +1,87 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import org.jboss.protean.arc.Arc; +import org.jboss.protean.arc.Subclass; +import org.jboss.protean.arc.test.ArcTestContainer; +import org.junit.Rule; +import org.junit.Test; + +public class InterceptorExceptionHandlingTest { + + @Rule + public ArcTestContainer container = new ArcTestContainer(ExceptionHandlingBean.class, + ExceptionHandlingInterceptor.class, ExceptionHandlingInterceptorBinding.class); + + @Test(expected = MyDeclaredException.class) + public void testProperlyThrowsDeclaredExceptions() throws MyDeclaredException { + ExceptionHandlingBean exceptionHandlingBean = Arc.container().instance(ExceptionHandlingBean.class).get(); + + assertTrue(exceptionHandlingBean instanceof Subclass); + + exceptionHandlingBean.foo(ExceptionHandlingCase.DECLARED_EXCEPTION); + } + + @Test(expected = MyRuntimeException.class) + public void testProperlyThrowsRuntimeExceptions() throws MyDeclaredException { + ExceptionHandlingBean exceptionHandlingBean = Arc.container().instance(ExceptionHandlingBean.class).get(); + + assertTrue(exceptionHandlingBean instanceof Subclass); + + exceptionHandlingBean.foo(ExceptionHandlingCase.RUNTIME_EXCEPTION); + } + + @Test + public void testWrapsOtherExceptions() throws MyDeclaredException { + try { + ExceptionHandlingBean exceptionHandlingBean = Arc.container().instance(ExceptionHandlingBean.class).get(); + + assertTrue(exceptionHandlingBean instanceof Subclass); + + exceptionHandlingBean.foo(ExceptionHandlingCase.OTHER_EXCEPTIONS); + fail("The method should have thrown a RuntimeException wrapping a MyOtherException but didn't throw any exception."); + } catch (RuntimeException e) { + // Let's check the cause is consistent with what we except. + assertEquals(MyOtherException.class, e.getCause().getClass()); + } catch (Exception e) { + fail("The method should have thrown a RuntimeException wrapping a MyOtherException but threw: " + e); + } + } + + @Test(expected = Exception.class) + public void testThrowsException() throws Exception { + ExceptionHandlingBean exceptionHandlingBean = Arc.container().instance(ExceptionHandlingBean.class).get(); + + assertTrue(exceptionHandlingBean instanceof Subclass); + + exceptionHandlingBean.bar(); + } + + @Test(expected = RuntimeException.class) + public void testThrowsRuntimeException() { + ExceptionHandlingBean exceptionHandlingBean = Arc.container().instance(ExceptionHandlingBean.class).get(); + + assertTrue(exceptionHandlingBean instanceof Subclass); + + exceptionHandlingBean.baz(); + } +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyDeclaredException.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyDeclaredException.java new file mode 100644 index 0000000000000..737d77efbccb9 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyDeclaredException.java @@ -0,0 +1,20 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +public class MyDeclaredException extends Exception { +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyOtherException.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyOtherException.java new file mode 100644 index 0000000000000..e17a1cccefc23 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyOtherException.java @@ -0,0 +1,20 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +public class MyOtherException extends Exception { +} diff --git a/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyRuntimeException.java b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyRuntimeException.java new file mode 100644 index 0000000000000..a64459df9c8b1 --- /dev/null +++ b/ext/arc/tests/src/test/java/org/jboss/protean/arc/test/interceptors/exceptionhandling/MyRuntimeException.java @@ -0,0 +1,20 @@ +/* + * Copyright 2018 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.protean.arc.test.interceptors.exceptionhandling; + +public class MyRuntimeException extends RuntimeException { +} From f02cff988f27cfe8a2221a3a3fc6256cfe3ff069 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Tue, 18 Dec 2018 12:31:46 +0100 Subject: [PATCH 2/2] Use DotNames.create() instead of DotName.createSimple() Also take into account Throwable, even if not critical - it works OK without it. --- .../protean/arc/processor/SubclassGenerator.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java b/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java index dcd39223117aa..8ca8c6722a270 100644 --- a/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java +++ b/ext/arc/processor/src/main/java/org/jboss/protean/arc/processor/SubclassGenerator.java @@ -65,8 +65,9 @@ */ public class SubclassGenerator extends AbstractGenerator { - private static final DotName JAVA_LANG_EXCEPTION = DotName.createSimple(Exception.class.getName()); - private static final DotName JAVA_LANG_RUNTIME_EXCEPTION = DotName.createSimple(RuntimeException.class.getName()); + private static final DotName JAVA_LANG_THROWABLE = DotNames.create(Throwable.class.getName()); + private static final DotName JAVA_LANG_EXCEPTION = DotNames.create(Exception.class.getName()); + private static final DotName JAVA_LANG_RUNTIME_EXCEPTION = DotNames.create(RuntimeException.class.getName()); static final String SUBCLASS_SUFFIX = "_Subclass"; @@ -281,10 +282,12 @@ private void createForwardingMethod(ClassOutput classOutput, BeanInfo bean, Meth CatchBlockCreator catchDeclaredException = tryCatch.addCatch(declaredException.name().toString()); catchDeclaredException.throwException(catchDeclaredException.getCaughtException()); - if (JAVA_LANG_RUNTIME_EXCEPTION.equals(declaredException.name())) { + if (JAVA_LANG_RUNTIME_EXCEPTION.equals(declaredException.name()) || + JAVA_LANG_THROWABLE.equals(declaredException.name())) { addCatchRuntimeException = false; } - if (JAVA_LANG_EXCEPTION.equals(declaredException.name())) { + if (JAVA_LANG_EXCEPTION.equals(declaredException.name()) || + JAVA_LANG_THROWABLE.equals(declaredException.name())) { addCatchException = false; } }