Bugfix

mercyblitz · mercyblitz · commit a982a1d90ce1 · 2021-09-09T11:34:33.000+08:00
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/main/java/org/geektimes/interceptor/ChainableInvocationContext.java b/projects/stage-1/middleware-frameworks/my-interceptor/src/main/java/org/geektimes/interceptor/ChainableInvocationContext.java
@@ -21,6 +21,7 @@
 import javax.interceptor.InvocationContext;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
@@ -92,8 +93,12 @@ public Object proceed() throws Exception {
         if (pos < size) {
             int currentPos = pos++;
             Object interceptor = interceptors.get(currentPos);
-            Method interceptionMethod = resolveInterceptionMethod(interceptor);
-            return interceptionMethod.invoke(interceptor, this);
+            Collection<Method> interceptionMethods = resolveInterceptionMethods(interceptor);
+            Object result = null;
+            for (Method interceptionMethod : interceptionMethods) {
+                result = interceptionMethod.invoke(interceptor, this);
+            }
+            return result;
         } else {
             return delegateContext.proceed();
         }
@@ -113,26 +118,30 @@ private List<Object> resolveInterceptors(Object[] defaultInterceptors) {
     }
 
 
-    private Method resolveInterceptionMethod(Object interceptor) {
+    private Collection<Method> resolveInterceptionMethods(Object interceptor) {
         InterceptorInfo interceptorInfo = interceptorManager.getInterceptorInfo(interceptor.getClass());
 
-        final Method interceptionMethod;  // nerver null
+        if (interceptorInfo == null) { // interceptor may be a default(external) Interceptor
+            interceptorInfo = new InterceptorInfo(interceptor.getClass());
+        }
+
+        final Collection<Method> interceptionMethods;  // nerver null
 
         if (getTimer() != null) { // If the "Timer" is present
-            interceptionMethod = interceptorInfo.getAroundTimeoutMethod();
+            interceptionMethods = interceptorInfo.getAroundTimeoutMethods();
         } else if (getConstructor() != null) { // If the "Constructor" should be intercepted
-            interceptionMethod = interceptorInfo.getAroundConstructMethod();
+            interceptionMethods = interceptorInfo.getAroundConstructMethods();
         } else {
             Method method = getMethod();
             if (method.isAnnotationPresent(PostConstruct.class)) {
-                interceptionMethod = interceptorInfo.getPostConstructMethod();
+                interceptionMethods = interceptorInfo.getPostConstructMethods();
             } else if (method.isAnnotationPresent(PreDestroy.class)) {
-                interceptionMethod = interceptorInfo.getPreDestroyMethod();
+                interceptionMethods = interceptorInfo.getPreDestroyMethods();
             } else {
-                interceptionMethod = interceptorInfo.getAroundInvokeMethod();
+                interceptionMethods = interceptorInfo.getAroundInvokeMethods();
             }
         }
 
-        return interceptionMethod;
+        return interceptionMethods;
     }
 }
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/main/java/org/geektimes/interceptor/DefaultInterceptorManager.java b/projects/stage-1/middleware-frameworks/my-interceptor/src/main/java/org/geektimes/interceptor/DefaultInterceptorManager.java
@@ -83,7 +83,35 @@ public void registerInterceptorClass(Class<?> interceptorClass) {
     public void registerInterceptor(Object interceptor) {
         Class<?> interceptorClass = interceptor.getClass();
         registerInterceptorClass(interceptorClass);
-        InterceptorBindings interceptorBindings = getInterceptorBindings(interceptorClass);
+        InterceptorInfo interceptorInfo = getInterceptorInfo(interceptorClass);
+        registerRegularInterceptor(interceptorInfo, interceptor);
+        registerLifecycleEventInterceptor(interceptorInfo, interceptor);
+    }
+
+    private void registerRegularInterceptor(InterceptorInfo interceptorInfo, Object interceptor) {
+        InterceptorBindings interceptorBindings = interceptorInfo.getInterceptorBindings();
+        registerInterceptor(interceptorBindings, interceptor);
+    }
+
+    private void registerLifecycleEventInterceptor(InterceptorInfo interceptorInfo, Object interceptor) {
+        for (Method method : interceptorInfo.getPostConstructMethods()) {
+            registerLifecycleEventInterceptor(method, PostConstruct.class, interceptor);
+        }
+
+        for (Method method : interceptorInfo.getPreDestroyMethods()) {
+            registerLifecycleEventInterceptor(method, PreDestroy.class, interceptor);
+        }
+    }
+
+    private void registerLifecycleEventInterceptor(Method method, Class<? extends Annotation> lifecycleAnnotationType, Object interceptor) {
+        Annotation lifecycleAnnotation = method.getAnnotation(lifecycleAnnotationType);
+        if (lifecycleAnnotation != null) {
+            InterceptorBindings interceptorBindings = new InterceptorBindings(singleton(lifecycleAnnotation));
+            registerInterceptor(interceptorBindings, interceptor);
+        }
+    }
+
+    private void registerInterceptor(InterceptorBindings interceptorBindings, Object interceptor) {
         SortedSet<Object> interceptors = bindingInterceptors.computeIfAbsent(interceptorBindings, t -> new TreeSet<>(PriorityComparator.INSTANCE));
         interceptors.add(interceptor);
     }
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/main/java/org/geektimes/interceptor/InterceptorInfo.java b/projects/stage-1/middleware-frameworks/my-interceptor/src/main/java/org/geektimes/interceptor/InterceptorInfo.java
@@ -25,15 +25,14 @@
 import javax.interceptor.AroundTimeout;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 
 import static java.lang.String.format;
-import static org.geektimes.commons.collection.util.CollectionUtils.asSet;
+import static java.util.Collections.unmodifiableCollection;
 import static org.geektimes.commons.lang.util.AnnotationUtils.getAllDeclaredAnnotations;
-import static org.geektimes.commons.reflect.util.MethodUtils.getAllDeclaredMethods;
+import static org.geektimes.commons.reflect.util.ClassUtils.getAllClasses;
 import static org.geektimes.interceptor.util.InterceptorUtils.validatorInterceptorClass;
 
 /**
@@ -48,59 +47,80 @@ public class InterceptorInfo {
 
     private final Class<?> interceptorClass;
 
-    private final Method aroundInvokeMethod;
+    /**
+     * If an interceptor class declared using interceptor bindings has superclasses,
+     * interceptor methods declared in the interceptor class’s superclasses are
+     * invoked before the interceptor method declared in the interceptor class itself,
+     * most general superclass first.
+     */
+    private final Collection<Method> aroundInvokeMethods;
 
-    private final Method aroundTimeoutMethod;
+    private final Collection<Method> aroundTimeoutMethods;
 
-    private final Method aroundConstructMethod;
+    private final Collection<Method> aroundConstructMethods;
 
-    private final Method postConstructMethod;
+    private final Collection<Method> postConstructMethods;
 
-    private final Method preDestroyMethod;
+    private final Collection<Method> preDestroyMethods;
 
     private final InterceptorBindings interceptorBindings;
 
     public InterceptorInfo(Class<?> interceptorClass) {
         validatorInterceptorClass(interceptorClass);
         this.interceptorManager = InterceptorManager.getInstance(interceptorClass.getClassLoader());
         this.interceptorClass = interceptorClass;
-        Map<Class<? extends Annotation>, Method> interceptionMethods = resolveInterceptionMethods();
-        this.aroundInvokeMethod = interceptionMethods.remove(AroundInvoke.class);
-        this.aroundTimeoutMethod = interceptionMethods.remove(AroundTimeout.class);
-        this.aroundConstructMethod = interceptionMethods.remove(AroundConstruct.class);
-        this.postConstructMethod = interceptionMethods.remove(PostConstruct.class);
-        this.preDestroyMethod = interceptionMethods.remove(PreDestroy.class);
+        this.aroundInvokeMethods = new LinkedList<>();
+        this.aroundTimeoutMethods = new LinkedList<>();
+        this.aroundConstructMethods = new LinkedList<>();
+        this.postConstructMethods = new LinkedList<>();
+        this.preDestroyMethods = new LinkedList<>();
+        resolveInterceptionMethods();
         this.interceptorBindings = resolveInterceptorBindings();
     }
 
-    private Map<Class<? extends Annotation>, Method> resolveInterceptionMethods() throws IllegalStateException {
-        Set<Method> methods = getAllDeclaredMethods(interceptorClass, method -> !Object.class.equals(method.getDeclaringClass()));
-        Map<Class<? extends Annotation>, Method> interceptionMethods = new HashMap<>();
+    private void resolveInterceptionMethods() throws IllegalStateException {
+        Set<Class<?>> allClasses = getAllClasses(interceptorClass, true, t -> !Object.class.equals(t));
 
-        for (Method method : methods) {
-            resolveInterceptionMethod(method, AroundInvoke.class, InterceptorUtils::isAroundInvokeMethod, interceptionMethods);
-            resolveInterceptionMethod(method, AroundTimeout.class, InterceptorUtils::isAroundTimeoutMethod, interceptionMethods);
-            resolveInterceptionMethod(method, AroundConstruct.class, InterceptorUtils::isAroundConstructMethod, interceptionMethods);
-            resolveInterceptionMethod(method, PostConstruct.class, InterceptorUtils::isPostConstructMethod, interceptionMethods);
-            resolveInterceptionMethod(method, PreDestroy.class, InterceptorUtils::isPreDestroyMethod, interceptionMethods);
+        for (Class<?> declaringClass : allClasses) {
+            Map<Class<? extends Annotation>, Method> interceptionMethods = new HashMap<>();
+            for (Method method : declaringClass.getDeclaredMethods()) {
+                resolveInterceptionMethod(method, AroundInvoke.class, InterceptorUtils::isAroundInvokeMethod,
+                        interceptionMethods, aroundInvokeMethods::add);
+
+                resolveInterceptionMethod(method, AroundTimeout.class, InterceptorUtils::isAroundTimeoutMethod,
+                        interceptionMethods, aroundTimeoutMethods::add);
+
+                resolveInterceptionMethod(method, AroundConstruct.class, InterceptorUtils::isAroundConstructMethod,
+                        interceptionMethods, aroundConstructMethods::add);
+
+                resolveInterceptionMethod(method, PostConstruct.class, InterceptorUtils::isPostConstructMethod,
+                        interceptionMethods, postConstructMethods::add);
+
+                resolveInterceptionMethod(method, PreDestroy.class, InterceptorUtils::isPreDestroyMethod,
+                        interceptionMethods, preDestroyMethods::add);
+            }
+            interceptionMethods.clear();
         }
 
-        return interceptionMethods;
     }
 
-    private void resolveInterceptionMethod(Method method, Class<? extends Annotation> annotationType,
+    private void resolveInterceptionMethod(Method method,
+                                           Class<? extends Annotation> annotationType,
                                            Predicate<Method> isInterceptionMethod,
-                                           Map<Class<? extends Annotation>, Method> interceptionMethods) {
+                                           Map<Class<? extends Annotation>, Method> interceptionMethods,
+                                           Consumer<Method> interceptionMethodConsumer) {
         if (isInterceptionMethod.test(method)) {
-            if (interceptionMethods.putIfAbsent(annotationType, method) != null) {
-                throw interceptionMethodDefinitionException(annotationType);
+            if (interceptionMethods.putIfAbsent(annotationType, method) == null) {
+                interceptionMethodConsumer.accept(method);
+            } else {
+                throw interceptionMethodDefinitionException(method, annotationType);
             }
         }
     }
 
-    private IllegalStateException interceptionMethodDefinitionException(Class<? extends Annotation> annotationType) {
-        throw new IllegalStateException(format("There is only one @%s method is declared in the interceptor class[%s]",
-                annotationType.getName(), interceptorClass.getName()));
+    private IllegalStateException interceptionMethodDefinitionException(Method method, Class<? extends Annotation> annotationType) {
+        throw new IllegalStateException(format("There is only one @%s method[%s] is declared in the interceptor class[%s]",
+                annotationType.getName(), method.toString(), method.getDeclaringClass().getName()));
     }
 
     private InterceptorBindings resolveInterceptorBindings() {
@@ -111,27 +131,27 @@ public Class<?> getInterceptorClass() {
         return interceptorClass;
     }
 
-    public Method getAroundInvokeMethod() {
-        return aroundInvokeMethod;
+    Collection<Method> getAroundInvokeMethods() {
+        return aroundInvokeMethods;
     }
 
-    public Method getAroundTimeoutMethod() {
-        return aroundTimeoutMethod;
+    Collection<Method> getAroundTimeoutMethods() {
+        return aroundTimeoutMethods;
     }
 
-    public Method getAroundConstructMethod() {
-        return aroundConstructMethod;
+    Collection<Method> getAroundConstructMethods() {
+        return aroundConstructMethods;
     }
 
-    public Method getPostConstructMethod() {
-        return postConstructMethod;
+    Collection<Method> getPostConstructMethods() {
+        return postConstructMethods;
     }
 
-    public Method getPreDestroyMethod() {
-        return preDestroyMethod;
+    Collection<Method> getPreDestroyMethods() {
+        return preDestroyMethods;
     }
 
-    public InterceptorBindings getInterceptorBindings() {
+    InterceptorBindings getInterceptorBindings() {
         return interceptorBindings;
     }
 
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/test/java/org/geektimes/interceptor/DefaultComponentEnhancerTest.java b/projects/stage-1/middleware-frameworks/my-interceptor/src/test/java/org/geektimes/interceptor/DefaultComponentEnhancerTest.java
@@ -18,6 +18,9 @@
 
 import org.junit.Test;
 
+import static org.geektimes.commons.collection.util.CollectionUtils.asSet;
+import static org.junit.Assert.assertEquals;
+
 /**
  * {@link DefaultComponentEnhancer} Test
  *
@@ -31,8 +34,11 @@ public class DefaultComponentEnhancerTest {
     @Test
     public void testInterface() {
         EchoService echoService = new EchoService();
-        echoService = interceptorEnhancer.enhance(echoService);
+        ExternalInterceptor interceptor = new ExternalInterceptor();
+        echoService = interceptorEnhancer.enhance(echoService, interceptor);
         echoService.init();
         echoService.echo("Hello,World");
+
+        assertEquals(asSet("init", "echo"), interceptor.getMethodNames());
     }
 }
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/test/java/org/geektimes/interceptor/ExtLoggingInterceptor.java b/projects/stage-1/middleware-frameworks/my-interceptor/src/test/java/org/geektimes/interceptor/ExtLoggingInterceptor.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.geektimes.interceptor;
+
+import org.geektimes.interceptor.Logging;
+import org.geektimes.interceptor.LoggingInterceptor;
+
+import javax.annotation.PostConstruct;
+import javax.interceptor.Interceptor;
+import javax.interceptor.InvocationContext;
+import java.util.logging.Logger;
+
+/**
+ * @author <a href="mailto:mercyblitz@gmail.com">Mercy</a>
+ * @since
+ */
+@Logging
+@Interceptor
+public class ExtLoggingInterceptor extends LoggingInterceptor {
+
+    @PostConstruct
+    public void postConstruct(InvocationContext context) throws Exception {
+        Logger logger = Logger.getLogger(getClass().getName());
+        logger.info("postConstruct");
+    }
+}
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/test/java/org/geektimes/interceptor/ExternalInterceptor.java b/projects/stage-1/middleware-frameworks/my-interceptor/src/test/java/org/geektimes/interceptor/ExternalInterceptor.java
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.geektimes.interceptor;
+
+import javax.annotation.PostConstruct;
+import javax.interceptor.AroundInvoke;
+import javax.interceptor.Interceptor;
+import javax.interceptor.InvocationContext;
+import java.util.LinkedHashSet;
+import java.util.Set;
+import java.util.logging.Logger;
+
+/**
+ * External {@link Interceptor @Interceptor}
+ *
+ * @author <a href="mailto:mercyblitz@gmail.com">Mercy</a>
+ * @since 1.0.0
+ */
+@Interceptor
+public class ExternalInterceptor {
+
+    private final Logger logger = Logger.getLogger(getClass().getName());
+
+    private Set<String> methodNames = new LinkedHashSet<>();
+
+    @AroundInvoke
+    public Object intercept(InvocationContext context) throws Throwable {
+        String methodName = context.getMethod().getName();
+        methodNames.add(methodName);
+        logger.info("Interception Method : " + methodName);
+        return context.proceed();
+    }
+
+    @PostConstruct
+    public void postConstruct(InvocationContext context) {
+        String methodName = context.getMethod().getName();
+        methodNames.add(methodName);
+        logger.info("Post Construct : " + context.getMethod().getName());
+    }
+
+    public Set<String> getMethodNames() {
+        return methodNames;
+    }
+}
diff --git a/projects/stage-1/middleware-frameworks/my-interceptor/src/test/resources/META-INF/services/org.geektimes.interceptor.Interceptor b/projects/stage-1/middleware-frameworks/my-interceptor/src/test/resources/META-INF/services/org.geektimes.interceptor.Interceptor
@@ -1 +1 @@
-org.geektimes.interceptor.LoggingInterceptor
+org.geektimes.interceptor.ExtLoggingInterceptor

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-org.geektimes.interceptor.LoggingInterceptor`
	`1`	`+org.geektimes.interceptor.ExtLoggingInterceptor`