This is an automated email from the ASF dual-hosted git repository.

danhaywood pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git

commit 2fc93a23eff85b8f5530bbf6dee0e0f7d07a19f6
Author: Andi Huber <ahu...@apache.org>
AuthorDate: Mon Jan 15 14:06:44 2018 +0100

    ISIS-1740 Refactoring and consolidating invocation exception handling
    + introducing MethodHandles to speed up reflective invocation
---
 .../isis/core/commons/lang/MethodExtensions.java   |  14 +--
 .../core/commons/lang/ThrowableExtensions.java     |  62 ++++++++---
 .../isis/core/commons/reflection/Reflect.java      |  44 ++++++--
 .../isis/core/metamodel/facets/Annotations.java    | 114 +++++++++++++++------
 ...ctionInvocationFacetForDomainEventAbstract.java |  54 +++++-----
 5 files changed, 196 insertions(+), 92 deletions(-)

diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/MethodExtensions.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/MethodExtensions.java
index fb95bf9..7215a47 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/MethodExtensions.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/MethodExtensions.java
@@ -19,12 +19,9 @@
 
 package org.apache.isis.core.commons.lang;
 
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 
-import org.apache.isis.core.metamodel.exceptions.MetaModelException;
-
 public class MethodExtensions {
 
     private MethodExtensions() {
@@ -45,14 +42,9 @@ public class MethodExtensions {
         try {
             Object[] defaultAnyPrimitive = 
defaultAnyPrimitive(method.getParameterTypes(), arguments);
             return method.invoke(object, defaultAnyPrimitive);
-        } catch (final IllegalArgumentException e) {
-            throw e;
-        } catch (final InvocationTargetException e) {
-            ThrowableExtensions.throwWithinIsisException(e, "Exception 
executing " + method);
-            return null;
-        } catch (final IllegalAccessException e) {
-            throw new MetaModelException("illegal access of " + method, e);
-        }
+        } catch (Exception e) {
+               return ThrowableExtensions.handleInvocationException(e, 
method.getName());
+               } 
     }
 
     private static Object[] defaultAnyPrimitive(Class<?>[] parameterTypes, 
Object[] arguments) {
diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ThrowableExtensions.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ThrowableExtensions.java
index 70cf875..855ba1d 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ThrowableExtensions.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ThrowableExtensions.java
@@ -19,26 +19,64 @@
 
 package org.apache.isis.core.commons.lang;
 
+import java.lang.invoke.WrongMethodTypeException;
 import java.lang.reflect.InvocationTargetException;
+import java.util.function.Consumer;
 
 import org.apache.isis.applib.RecoverableException;
 import org.apache.isis.core.commons.exceptions.IsisApplicationException;
 import org.apache.isis.core.metamodel.exceptions.MetaModelException;
+import org.apache.isis.core.metamodel.specloader.ReflectiveActionException;
 
 public final class ThrowableExtensions {
 
-    public static void throwWithinIsisException(final 
InvocationTargetException e, final String error) {
-        final Throwable targetException = e.getTargetException();
-        if (targetException instanceof RecoverableException) {
-            // an application exception from the domain code is re-thrown as an
-            // IsisException with same semantics
-            // TODO: should probably be using ApplicationException here
-            throw new IsisApplicationException(error, targetException);
+       public static Object handleInvocationException(
+               final Throwable e, 
+               final String memberName) {
+               return handleInvocationException(e, memberName, null);
+       }
+       
+    public static Object handleInvocationException(
+               final Throwable e, 
+               final String memberName, 
+               final Consumer<RecoverableException> recovery) {
+       
+       if(e instanceof InvocationTargetException) {
+                       return 
handleInvocationException(((InvocationTargetException) e).getTargetException(), 
memberName, recovery);
+               }
+       if(e instanceof WrongMethodTypeException) {
+                       throw new MetaModelException("Wrong method type access 
of " + memberName, e);
+               }
+       if(e instanceof IllegalAccessException) {
+                       throw new ReflectiveActionException("Illegal access of 
" + memberName, e);
+           }
+       if(e instanceof IllegalStateException) {
+            throw new ReflectiveActionException( String.format(
+                    "IllegalStateException thrown while invoking %s %s",
+                    memberName, e.getMessage()), e);
         }
-        if (targetException instanceof RuntimeException) {
-            throw (RuntimeException) targetException;
-        } else {
-            throw new MetaModelException(targetException);
+               if(e instanceof RecoverableException) {
+                       return 
handleRecoverableException((RecoverableException)e, memberName, recovery);
+           }
+               if (e instanceof RuntimeException) {
+            throw (RuntimeException) e;
         }
-    }
+        throw new MetaModelException("Exception invoking " + memberName, e);
+       }
+
+
+       private static Object handleRecoverableException(
+                       final RecoverableException e, 
+               final String memberName, 
+               final Consumer<RecoverableException> recovery) {
+               
+               if(recovery!=null)
+                       recovery.accept(e); 
+               
+               // an application exception from the domain code is re-thrown 
as an
+        // IsisException with same semantics
+        // TODO: should probably be using ApplicationException here
+        throw new IsisApplicationException("Exception invoking " + memberName, 
e);
+       }
+    
 }
diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/commons/reflection/Reflect.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/commons/reflection/Reflect.java
index 59bc1c8..44e9431 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/commons/reflection/Reflect.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/commons/reflection/Reflect.java
@@ -22,6 +22,8 @@ import java.beans.BeanInfo;
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -46,9 +48,9 @@ public class Reflect {
 
        public static Object[] emptyObjects = {};
        public static Class<?>[] emptyClasses = {};
-       
+
        // -- CLASS REFLECTION
-       
+
        /**
         * Returns declared methods of this class/interface and all super 
classes/interfaces.
         * @param type
@@ -76,7 +78,7 @@ public class Reflect {
                
visitSuperclassesOf(type,c->Stream.of(c.getDeclaredFields()).forEach(fields::add));
                return fields;
        }
-       
+
        public static void visitSuperclassesOf(final Class<?> clazz, final 
Consumer<Class<?>> visitor){
                final Class<?> superclass = clazz.getSuperclass();
                if(superclass!=null){
@@ -92,7 +94,7 @@ public class Reflect {
                for(Class<?> interf : clazz.getInterfaces())
                        visitor.accept(interf);
        }
-       
+
        public static Method getGetter(Class<?> cls, String propertyName) 
throws IntrospectionException {
                final BeanInfo beanInfo = Introspector.getBeanInfo(cls);
                for(PropertyDescriptor pd:beanInfo.getPropertyDescriptors()){
@@ -102,14 +104,35 @@ public class Reflect {
                }
                return null;    
        }
-       
+
        public static Method getGetter(Object bean, String propertyName) throws 
IntrospectionException {
                if(bean==null)
                        return null;
                return getGetter(bean, propertyName);   
        }
-       
-       
+
+       // -- METHOD HANDLES
+
+       public static MethodHandle handleOf(Method method) throws 
IllegalAccessException {
+               if(!method.isAccessible()) {
+                       method.setAccessible(true);
+                       MethodHandle mh = 
MethodHandles.publicLookup().unreflect(method);
+                       method.setAccessible(false);
+                       return mh;      
+               }
+               return MethodHandles.publicLookup().unreflect(method);
+       }
+
+       public static MethodHandle handleOf(Field field) throws 
IllegalAccessException {
+               if(!field.isAccessible()) {
+                       field.setAccessible(true);
+                       MethodHandle mh = 
MethodHandles.lookup().unreflectGetter(field);
+                       field.setAccessible(false);
+                       return mh;      
+               }
+               return MethodHandles.lookup().unreflectGetter(field);
+       }
+
        // -- PRIMITIVE TYPES
 
        private static final Set<Class<?>> primitives = new 
HashSet<>(Arrays.asList(
@@ -135,7 +158,7 @@ public class Reflect {
                        Short.class
                        //Void.class //separated out into its own predicate: 
isVoid(...)
                        ));
-       
+
        // -- TYPE PREDICATES
 
        public static boolean isVoid(Class<?> c) {
@@ -171,4 +194,9 @@ public class Reflect {
                return isVoid(m.getReturnType());
        }
 
+
+
+
+
+
 }
diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/Annotations.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/Annotations.java
index 3d24c77..381df00 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/Annotations.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/Annotations.java
@@ -19,13 +19,15 @@
 
 package org.apache.isis.core.metamodel.facets;
 
+import java.beans.IntrospectionException;
 import java.lang.annotation.Annotation;
+import java.lang.invoke.MethodHandle;
 import java.lang.reflect.Field;
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -40,6 +42,7 @@ import org.apache.isis.applib.annotation.Property;
 import org.apache.isis.applib.annotation.PropertyLayout;
 import org.apache.isis.applib.annotation.Title;
 import org.apache.isis.core.commons.lang.ThrowableExtensions;
+import org.apache.isis.core.commons.reflection.Reflect;
 import org.apache.isis.core.metamodel.exceptions.MetaModelException;
 import org.apache.isis.core.metamodel.methodutils.MethodScope;
 
@@ -381,11 +384,11 @@ public final class Annotations  {
      * @param cls
      * @param annotationClass
      * @return list of {@link Evaluator} that wraps each annotated member 
found on the class where 
-     * the search stopped, null otherwise
+     * the search stopped, or an empty list if no such {@code annotationClass} 
annotation found.
      * 
      * @since 2.0.0
      */
-    public static <T extends Annotation> List<Evaluator<T>> 
findFirstInHierarchyHaving(
+    public static <T extends Annotation> List<Evaluator<T>> 
firstEvaluatorsInHierarchyHaving(
             final Class<?> cls,
             final Class<T> annotationClass) {
        
@@ -411,8 +414,8 @@ public final class Annotations  {
        if(!filter.test(cls))
                return; // stop visitation
        
-       collectMethodEvaluators(cls, annotationClass, visitor);
-       collectFieldEvaluators(cls, annotationClass, visitor);
+       visitMethodEvaluators(cls, annotationClass, visitor);
+       visitFieldEvaluators(cls, annotationClass, visitor);
         
         // search super-classes
         final Class<?> superclass = cls.getSuperclass();
@@ -423,39 +426,39 @@ public final class Annotations  {
     }
     
     @SuppressWarnings({ "rawtypes", "unchecked" })
-       private static <T extends Annotation> void collectMethodEvaluators(
+       private static <T extends Annotation> void visitMethodEvaluators(
             final Class<?> cls,
             final Class<T> annotationClass,
-            final Consumer<Evaluator<T>> action) {
+            final Consumer<Evaluator<T>> visitor) {
        
        for (Method method : cls.getDeclaredMethods()) {
             if(MethodScope.OBJECT.matchesScopeOf(method) &&
                     method.getParameterTypes().length == 0) {
                 final Annotation annotation = 
method.getAnnotation(annotationClass);
                 if(annotation != null) {
-                       action.accept(new MethodEvaluator(method, annotation));
+                       visitor.accept(new MethodEvaluator(method, annotation));
                 }
             }
         }
     }
     
     @SuppressWarnings({ "rawtypes", "unchecked" })
-       private static <T extends Annotation> void collectFieldEvaluators(
+       private static <T extends Annotation> void visitFieldEvaluators(
             final Class<?> cls,
             final Class<T> annotationClass,
-            final Consumer<Evaluator<T>> action) {
+            final Consumer<Evaluator<T>> visitor) {
        
        for (final Field field: cls.getDeclaredFields()) {
             final Annotation annotation = field.getAnnotation(annotationClass);
             if(annotation != null) {
-               action.accept(new FieldEvaluator(field, annotation));
+               visitor.accept(new FieldEvaluator(field, annotation));
             }
         }
     }
 
     public static abstract class Evaluator<T extends Annotation> {
-
         private final T annotation;
+       private MethodHandle mh;  
 
         protected Evaluator(final T annotation) {
             this.annotation = annotation;
@@ -465,7 +468,25 @@ public final class Annotations  {
             return annotation;
         }
 
-        public abstract Object value(final Object obj) ;
+        protected abstract MethodHandle createMethodHandle() throws 
IllegalAccessException;
+        protected abstract String name();
+        
+        public Object value(final Object obj) {
+               if(mh==null) {
+                       try {
+                                       mh = createMethodHandle();
+                               } catch (IllegalAccessException e) {
+                                       throw new MetaModelException("illegal 
access of " + name(), e);
+                               }
+               }
+               
+               try {
+                               return mh.invoke(obj);
+                       } catch (Throwable e) {
+                               return 
ThrowableExtensions.handleInvocationException(e, name());
+                       }
+ 
+        }
     }
 
     public static class MethodEvaluator<T extends Annotation> extends 
Evaluator<T> {
@@ -475,21 +496,31 @@ public final class Annotations  {
             super(annotation);
             this.method = method;
         }
-
-        public Object value(final Object obj)  {
-            try {
-                return method.invoke(obj);
-            } catch (final InvocationTargetException e) {
-                ThrowableExtensions.throwWithinIsisException(e, "Exception 
executing " + method);
-                return null;
-            } catch (final IllegalAccessException e) {
-                throw new MetaModelException("illegal access of " + method, e);
-            }
+        
+        @Override
+        protected String name() {
+               return method.getName();
         }
 
+//        public Object value(final Object obj)  {
+//            try {
+//                return method.invoke(obj);
+//            } catch (final InvocationTargetException e) {
+//                ThrowableExtensions.throwWithinIsisException(e, "Exception 
executing " + method);
+//                return null;
+//            } catch (final IllegalAccessException e) {
+//                throw new MetaModelException("illegal access of " + method, 
e);
+//            }
+//        }
+
         public Method getMethod() {
             return method;
         }
+
+               @Override
+               protected MethodHandle createMethodHandle() throws 
IllegalAccessException {
+                       return Reflect.handleOf(method);
+               }
     }
 
     public static class FieldEvaluator<T extends Annotation> extends 
Evaluator<T> {
@@ -499,19 +530,40 @@ public final class Annotations  {
             super(annotation);
             this.field = field;
         }
-
-        public Object value(final Object obj)  {
-            try {
-                field.setAccessible(true);
-                return field.get(obj);
-            } catch (final IllegalAccessException e) {
-                throw new MetaModelException("illegal access of " + field, e);
-            }
+        
+        @Override
+        protected String name() {
+               return field.getName();
         }
 
+        @Override
+               protected MethodHandle createMethodHandle() throws 
IllegalAccessException {
+                       return Reflect.handleOf(field);
+               }
+        
+//        public Object value(final Object obj)  {
+//            try {
+//                field.setAccessible(true);
+//                return field.get(obj);
+//            } catch (final IllegalAccessException e) {
+//                throw new MetaModelException("illegal access of " + field, 
e);
+//            }
+//        }
+
         public Field getField() {
             return field;
         }
+        
+        public Optional<Method> getGetter(Class<?> originatingClass) {
+                       try {
+                       return Optional.ofNullable(
+                                       Reflect.getGetter(originatingClass, 
field.getName())    );
+                       } catch (IntrospectionException e) {
+                               e.printStackTrace();
+                       }
+               return Optional.empty();
+        }
+        
     }
 
     private static List<Class<?>> fieldAnnotationClasses = 
Collections.unmodifiableList(
diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/invocation/ActionInvocationFacetForDomainEventAbstract.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/invocation/ActionInvocationFacetForDomainEventAbstract.java
index f6683fd..9053e47 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/invocation/ActionInvocationFacetForDomainEventAbstract.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/invocation/ActionInvocationFacetForDomainEventAbstract.java
@@ -27,6 +27,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Callable;
+import java.util.function.Consumer;
 
 import org.apache.isis.applib.NonRecoverableException;
 import org.apache.isis.applib.RecoverableException;
@@ -273,36 +274,29 @@ public abstract class 
ActionInvocationFacetForDomainEventAbstract
 
                         return 
ObjectAdapter.Util.unwrap(resultAdapterPossiblyCloned);
 
-                    } catch (IllegalAccessException ex) {
-                        throw new ReflectiveActionException("Illegal access of 
" + method, ex);
-                    } catch (InvocationTargetException ex) {
-
-                        final Throwable targetException = 
ex.getTargetException();
-                        if (targetException instanceof IllegalStateException) {
-                            throw new ReflectiveActionException( String.format(
-                                    "IllegalStateException thrown while 
executing %s %s",
-                                    method, targetException.getMessage()), 
targetException);
-                        }
-
-                        if(targetException instanceof RecoverableException) {
-                            if (!getTransactionState().canCommit()) {
-                                // something severe has happened to the 
underlying transaction;
-                                // so escalate this exception to be 
non-recoverable
-                                final Throwable targetExceptionCause = 
targetException.getCause();
-                                Throwable nonRecoverableCause = 
targetExceptionCause != null
-                                        ? targetExceptionCause
-                                        : targetException;
-
-                                // trim to first 300 chars
-                                final String message = 
trim(nonRecoverableCause.getMessage(), 300);
-
-                                throw new NonRecoverableException(message, 
nonRecoverableCause);
-                            }
-                        }
-
-                        ThrowableExtensions.throwWithinIsisException(ex, 
"Exception executing " + method);
-                        return null; // never executed, previous line throws
-                    }
+                    } catch (Exception e) {
+                       
+                       final Consumer<RecoverableException> recovery = 
recoverableException->{
+                               
+                               if (!getTransactionState().canCommit()) {
+                                       // something severe has happened to the 
underlying transaction;
+                                       // so escalate this exception to be 
non-recoverable
+                                       final Throwable 
recoverableExceptionCause = recoverableException.getCause();
+                                       Throwable nonRecoverableCause = 
recoverableExceptionCause != null
+                                               ? recoverableExceptionCause
+                                               : recoverableException;
+               
+                                       // trim to first 300 chars
+                                       final String message = 
trim(nonRecoverableCause.getMessage(), 300);
+               
+                                       throw new 
NonRecoverableException(message, nonRecoverableCause);
+                               }
+                        };
+                       
+                       return ThrowableExtensions.handleInvocationException(e, 
method.getName(), recovery);
+                                       }
+                    
+                    
                 }
             };
 

-- 
To stop receiving notification emails like this one, please contact
danhayw...@apache.org.

Reply via email to