You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2018/02/14 15:15:25 UTC

[isis] 09/13: ISIS-1740 Refactoring and consolidating invocation exception handling + introducing MethodHandles to speed up reflective invocation

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 <ah...@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
danhaywood@apache.org.