iss-907: clean up

edydkim · Oct 13, 2015 · 958ec5d · 958ec5d
1 parent 3579733
commit 958ec5d
Show file tree

Hide file tree

Showing 14 changed files with 190 additions and 126 deletions.
diff --git a/hystrix-contrib/hystrix-javanica/build.gradle b/hystrix-contrib/hystrix-javanica/build.gradle
@@ -61,17 +61,16 @@ ext {
 
 
 dependencies {
-    //compile project(':hystrix-core')
+    compile project(':hystrix-core')
     ajtools "org.aspectj:aspectjtools:$aspectjVersion"
     testRuntime "org.aspectj:aspectjrt:$aspectjVersion"
-    compile 'com.netflix.hystrix:hystrix-core:1.4.15'
     compile "org.aspectj:aspectjweaver:$aspectjVersion"
     compile "org.aspectj:aspectjrt:$aspectjVersion"
-
     compile 'com.google.guava:guava:15.0'
     compile 'commons-collections:commons-collections:3.2.1'
     compile 'org.apache.commons:commons-lang3:3.1'
     compile 'com.google.code.findbugs:jsr305:2.0.0'
+
     testCompile 'junit:junit-dep:4.10'
     testCompile 'org.springframework:spring-core:4.0.0.RELEASE'
     testCompile 'org.springframework:spring-context:4.0.0.RELEASE'

diff --git a/...ca/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCacheAspect.java b/...ca/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCacheAspect.java
@@ -31,9 +31,8 @@
 import java.lang.reflect.Method;
 
 import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getMethodFromTarget;
-import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.getWeavingMode;
 import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.isCompileWeaving;
-import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.getOriginalMethod;
+import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.getAjcMethodAroundAdvice;
 
 /**
  * AspectJ aspect to process methods which annotated with annotations from
@@ -57,8 +56,7 @@ public Object methodsAnnotatedWithCacheRemove(final ProceedingJoinPoint joinPoin
         MetaHolder metaHolder = MetaHolder.builder()
                 .args(args).method(method).obj(obj)
                 .executionType(ExecutionType.SYNCHRONOUS)
-                .weavingMode(getWeavingMode())
-                .ajcMethod(isCompileWeaving() ? getOriginalMethod(obj.getClass(), method) : null)
+                .ajcMethod(isCompileWeaving() ? getAjcMethodAroundAdvice(obj.getClass(), method) : null)
                 .build();
         CacheInvocationContext<CacheRemove> context = CacheInvocationContextFactory
                 .createCacheRemoveInvocationContext(metaHolder);

diff --git a/.../src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java b/.../src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java
@@ -1,12 +1,12 @@
 /**
  * Copyright 2012 Netflix, Inc.
- *
+ * <p/>
  * 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
- *
+ * <p/>
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p/>
  * 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.
@@ -15,6 +15,7 @@
  */
 package com.netflix.hystrix.contrib.javanica.aop.aspectj;
 
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
 import com.netflix.hystrix.HystrixExecutable;
 import com.netflix.hystrix.contrib.javanica.annotation.HystrixCollapser;
@@ -26,21 +27,25 @@
 import com.netflix.hystrix.contrib.javanica.command.MetaHolder;
 import com.netflix.hystrix.exception.HystrixBadRequestException;
 import org.apache.commons.lang3.Validate;
+import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.annotation.Around;
 import org.aspectj.lang.annotation.Aspect;
 import org.aspectj.lang.annotation.Pointcut;
+import org.aspectj.lang.reflect.MethodSignature;
 
 import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Future;
+
 
-import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getAjcMethodFromTarget;
 import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getDeclaredMethod;
 import static com.netflix.hystrix.contrib.javanica.utils.AopUtils.getMethodFromTarget;
-import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.getWeavingMode;
 import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.isCompileWeaving;
-import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.getOriginalBatchMethod;
+import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.getAjcMethodAroundAdvice;
 
 /**
  * AspectJ aspect to process methods which annotated with {@link HystrixCommand} annotation.
@@ -108,11 +113,14 @@ public MetaHolder create(final ProceedingJoinPoint joinPoint) {
         public abstract MetaHolder create(Object proxy, Method method, Object obj, Object[] args, final ProceedingJoinPoint joinPoint);
 
         MetaHolder.Builder metaHolderBuilder(Object proxy, Method method, Object obj, Object[] args, final ProceedingJoinPoint joinPoint) {
-            return MetaHolder.builder()
+            MetaHolder.Builder builder = MetaHolder.builder()
                     .args(args).method(method).obj(obj).proxyObj(proxy)
                     .defaultGroupKey(obj.getClass().getSimpleName())
-                    .joinPoint(joinPoint).weavingMode(getWeavingMode())
-                    .ajcMethod(getAjcMethodFromTarget(joinPoint)); // todo: we don't need to call it everytime, only if weaving mode = compile
+                    .joinPoint(joinPoint);
+            if (isCompileWeaving()) {
+                builder.ajcMethod(getAjcMethodFromTarget(joinPoint));
+            }
+            return builder;
         }
     }
 
@@ -121,12 +129,40 @@ private static class CollapserMetaHolderFactory extends MetaHolderFactory {
         @Override
         public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Object[] args, final ProceedingJoinPoint joinPoint) {
             HystrixCollapser hystrixCollapser = collapserMethod.getAnnotation(HystrixCollapser.class);
+            if (collapserMethod.getParameterTypes().length > 1 || collapserMethod.getParameterTypes().length == 0) {
+                throw new IllegalStateException("Collapser method must have one argument: " + collapserMethod);
+            }
+
             Method batchCommandMethod = getDeclaredMethod(obj.getClass(), hystrixCollapser.batchMethod(), List.class);
             if (batchCommandMethod == null || !batchCommandMethod.getReturnType().equals(List.class)) {
                 throw new IllegalStateException("required batch method for collapser is absent: "
                         + "(java.util.List) " + obj.getClass().getCanonicalName() + "." +
                         hystrixCollapser.batchMethod() + "(java.util.List)");
             }
+
+            if (!collapserMethod.getParameterTypes()[0]
+                    .equals(getGenericParameter(batchCommandMethod.getGenericParameterTypes()[0]))) {
+                throw new IllegalStateException("required batch method for collapser is absent, wrong generic type: expected"
+                        + obj.getClass().getCanonicalName() + "." +
+                        hystrixCollapser.batchMethod() + "(java.util.List<" + collapserMethod.getParameterTypes()[0] + ">), but it's " +
+                        getGenericParameter(batchCommandMethod.getGenericParameterTypes()[0]));
+            }
+
+            Class<?> collapserMethodReturnType;
+            if (Future.class.isAssignableFrom(collapserMethod.getReturnType())) {
+                collapserMethodReturnType = getGenericParameter(collapserMethod.getGenericReturnType());
+            } else {
+                collapserMethodReturnType = collapserMethod.getReturnType();
+            }
+
+            if (!collapserMethodReturnType
+                    .equals(getGenericParameter(batchCommandMethod.getGenericReturnType()))) {
+                throw new IllegalStateException("Return type of batch method must be java.util.List parametrized with corresponding type: expected " +
+                        "(java.util.List<" + collapserMethodReturnType + ">)" + obj.getClass().getCanonicalName() + "." +
+                        hystrixCollapser.batchMethod() + "(java.util.List<" + collapserMethod.getParameterTypes()[0] + ">), but it's " +
+                        getGenericParameter(batchCommandMethod.getGenericReturnType()));
+            }
+
             HystrixCommand hystrixCommand = batchCommandMethod.getAnnotation(HystrixCommand.class);
             if (hystrixCommand == null) {
                 throw new IllegalStateException("batch method must be annotated with HystrixCommand annotation");
@@ -136,11 +172,10 @@ public MetaHolder create(Object proxy, Method collapserMethod, Object obj, Objec
             MetaHolder.Builder builder = MetaHolder.builder()
                     .args(args).method(batchCommandMethod).obj(obj).proxyObj(proxy)
                     .defaultGroupKey(obj.getClass().getSimpleName())
-                    .joinPoint(joinPoint).weavingMode(getWeavingMode());
-
+                    .joinPoint(joinPoint);
 
-            if(isCompileWeaving()){
-                builder.ajcMethod(getOriginalBatchMethod(obj.getClass(), batchCommandMethod.getName()));
+            if (isCompileWeaving()) {
+                builder.ajcMethod(getAjcMethodAroundAdvice(obj.getClass(), batchCommandMethod.getName(), List.class));
             }
 
             builder.hystrixCollapser(hystrixCollapser);
@@ -175,4 +210,19 @@ static HystrixPointcutType of(Method method) {
         }
     }
 
+    private static Method getAjcMethodFromTarget(JoinPoint joinPoint) {
+        return getAjcMethodAroundAdvice(joinPoint.getTarget().getClass(), (MethodSignature) joinPoint.getSignature());
+    }
+
+
+    private static Class<?> getGenericParameter(Type type) {
+        Type tType = ((ParameterizedType) type).getActualTypeArguments()[0];
+        String className = tType.toString().split(" ")[1];
+        try {
+            return Class.forName(className);
+        } catch (ClassNotFoundException e) {
+            throw Throwables.propagate(e);
+        }
+    }
+
 }
diff --git a/...-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/WeavingMode.java b/...-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/WeavingMode.java
@@ -1,5 +1,5 @@
 /**
- * Copyright 2012 Netflix, Inc.
+ * Copyright 2015 Netflix, Inc.
  * <p/>
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

diff --git a/...main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommandFactory.java b/...main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommandFactory.java
@@ -22,7 +22,6 @@
 import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand;
 import com.netflix.hystrix.contrib.javanica.annotation.HystrixProperty;
 import com.netflix.hystrix.contrib.javanica.conf.HystrixPropertiesManager;
-import com.netflix.hystrix.contrib.javanica.utils.EnvUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Validate;
 
@@ -34,7 +33,7 @@
 import static com.netflix.hystrix.contrib.javanica.cache.CacheInvocationContextFactory.createCacheRemoveInvocationContext;
 import static com.netflix.hystrix.contrib.javanica.cache.CacheInvocationContextFactory.createCacheResultInvocationContext;
 import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.isCompileWeaving;
-import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.getOriginalMethod;
+import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.getAjcMethodAroundAdvice;
 
 /**
  * Base implementation of {@link HystrixCommandFactory} interface.
@@ -95,7 +94,6 @@ CommandAction createFallbackAction(MetaHolder metaHolder,
                             .obj(metaHolder.getObj())
                             .method(fallbackMethod)
                             .ajcMethod(getAjcMethod(metaHolder.getObj(), fallbackMethod))
-                            .weavingMode(EnvUtils.getWeavingMode())
                             .args(metaHolder.getArgs())
                             .defaultCollapserKey(metaHolder.getDefaultCollapserKey())
                             .defaultCommandKey(fallbackMethod.getName())
@@ -104,12 +102,11 @@ CommandAction createFallbackAction(MetaHolder metaHolder,
                             .hystrixCommand(fallbackMethod.getAnnotation(HystrixCommand.class)).build();
                     fallbackAction = new LazyCommandExecutionAction(GenericHystrixCommandFactory.getInstance(), fmMetaHolder, collapsedRequests);
                 } else {
-                    // if falback methid isn't annotated with command annotation then we don't need to get ajc method for this
+
                     MetaHolder fmMetaHolder = MetaHolder.builder()
                             .obj(metaHolder.getObj())
                             .method(fallbackMethod)
-                            .ajcMethod(null)
-                            .weavingMode(EnvUtils.getWeavingMode())
+                            .ajcMethod(null) // if fallback method isn't annotated with command annotation then we don't need to get ajc method for this
                             .args(metaHolder.getArgs()).build();
 
                     fallbackAction = new MethodExecutionAction(fmMetaHolder.getObj(), fallbackMethod, fmMetaHolder.getArgs(), fmMetaHolder);
@@ -126,7 +123,7 @@ CommandAction createFallbackAction(MetaHolder metaHolder,
 
     private Method getAjcMethod(Object target, Method fallback) {
         if (isCompileWeaving()) {
-            return getOriginalMethod(target.getClass(), fallback);
+            return getAjcMethodAroundAdvice(target.getClass(), fallback);
         }
         return null;
     }

diff --git a/...rc/main/java/com/netflix/hystrix/contrib/javanica/command/LazyCommandExecutionAction.java b/...rc/main/java/com/netflix/hystrix/contrib/javanica/command/LazyCommandExecutionAction.java
@@ -74,7 +74,6 @@ private MetaHolder createHolder(ExecutionType executionType) {
                 .obj(metaHolder.getObj())
                 .method(metaHolder.getMethod())
                 .ajcMethod(metaHolder.getAjcMethod())
-                .weavingMode(metaHolder.getWeavingMode())
                 .executionType(executionType)
                 .args(metaHolder.getArgs())
                 .defaultCollapserKey(metaHolder.getDefaultCollapserKey())
@@ -90,7 +89,6 @@ private MetaHolder createHolder(ExecutionType executionType, Object[] args) {
                 .method(metaHolder.getMethod())
                 .executionType(executionType)
                 .ajcMethod(metaHolder.getAjcMethod())
-                .weavingMode(metaHolder.getWeavingMode())
                 .args(args)
                 .defaultCollapserKey(metaHolder.getDefaultCollapserKey())
                 .defaultCommandKey(metaHolder.getDefaultCommandKey())

diff --git a/...strix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java b/...strix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MetaHolder.java
@@ -46,7 +46,6 @@ public class MetaHolder {
     private final String defaultCollapserKey;
     private final ExecutionType executionType;
     private final ExecutionType collapserExecutionType;
-    private final WeavingMode weavingMode;
     private final JoinPoint joinPoint;
 
     private MetaHolder(Builder builder) {
@@ -64,7 +63,6 @@ private MetaHolder(Builder builder) {
         this.hystrixCollapser = builder.hystrixCollapser;
         this.executionType = builder.executionType;
         this.collapserExecutionType = builder.collapserExecutionType;
-        this.weavingMode = builder.weavingMode;
         this.joinPoint = builder.joinPoint;
     }
 
@@ -136,10 +134,6 @@ public boolean isCollapser(){
         return hystrixCollapser!=null;
     }
 
-    public WeavingMode getWeavingMode() {
-        return weavingMode;
-    }
-
     public JoinPoint getJoinPoint() {
         return joinPoint;
     }
@@ -160,7 +154,6 @@ public static final class Builder {
         private String defaultCollapserKey;
         private ExecutionType executionType;
         private ExecutionType collapserExecutionType;
-        private WeavingMode weavingMode;
         private JoinPoint joinPoint;
 
         public Builder hystrixCollapser(HystrixCollapser hystrixCollapser) {
@@ -233,11 +226,6 @@ public Builder defaultCollapserKey(String defCollapserKey) {
             return this;
         }
 
-        public Builder weavingMode(WeavingMode weavingMode) {
-            this.weavingMode = weavingMode;
-            return this;
-        }
-
         public Builder joinPoint(JoinPoint joinPoint) {
             this.joinPoint = joinPoint;
             return this;

diff --git a/...ica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MethodExecutionAction.java b/...ica/src/main/java/com/netflix/hystrix/contrib/javanica/command/MethodExecutionAction.java
@@ -16,16 +16,15 @@
 package com.netflix.hystrix.contrib.javanica.command;
 
 
-import com.netflix.hystrix.contrib.javanica.aop.aspectj.WeavingMode;
 import com.netflix.hystrix.contrib.javanica.command.closure.Closure;
 import com.netflix.hystrix.contrib.javanica.command.closure.ClosureFactoryRegistry;
 import com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException;
 import com.netflix.hystrix.contrib.javanica.exception.ExceptionUtils;
-import org.aspectj.lang.JoinPoint;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 
+import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.isCompileWeaving;
 import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.invokeAjcMethod;
 
 /**
@@ -57,8 +56,6 @@ public MethodExecutionAction(Object object, Method method, Object[] args, MetaHo
         this.metaHolder = metaHolder;
     }
 
-
-
     public Object getObject() {
         return object;
     }
@@ -108,7 +105,7 @@ private Object execute(Object o, Method m, Object... args) throws CommandActionE
         Object result = null;
         try {
             m.setAccessible(true); // suppress Java language access
-            if (WeavingMode.COMPILE == metaHolder.getWeavingMode() && metaHolder.getAjcMethod() != null) { // metaHolder.getAjcMethod() != null todo this is hack, refactor
+            if (isCompileWeaving() && metaHolder.getAjcMethod() != null) {
                 result = invokeAjcMethod(metaHolder.getAjcMethod(), o, metaHolder, args);
             } else {
                 result = m.invoke(o, args);

diff --git a/...ain/java/com/netflix/hystrix/contrib/javanica/command/closure/AbstractClosureFactory.java b/...ain/java/com/netflix/hystrix/contrib/javanica/command/closure/AbstractClosureFactory.java
@@ -16,13 +16,13 @@
 package com.netflix.hystrix.contrib.javanica.command.closure;
 
 import com.google.common.base.Throwables;
-import com.netflix.hystrix.contrib.javanica.aop.aspectj.WeavingMode;
 import com.netflix.hystrix.contrib.javanica.command.ClosureCommand;
 import com.netflix.hystrix.contrib.javanica.command.MetaHolder;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 
+import static com.netflix.hystrix.contrib.javanica.utils.EnvUtils.isCompileWeaving;
 import static com.netflix.hystrix.contrib.javanica.utils.ajc.AjcUtils.invokeAjcMethod;
 import static org.slf4j.helpers.MessageFormatter.format;
 
@@ -54,7 +54,7 @@ public Closure createClosure(MetaHolder metaHolder, Method method, Object o, Obj
         try {
             Object closureObj;
             method.setAccessible(true);
-            if (WeavingMode.COMPILE == metaHolder.getWeavingMode()) {
+            if (isCompileWeaving()) {
                 closureObj = invokeAjcMethod(metaHolder.getAjcMethod(), o, metaHolder, args);
             } else {
                 closureObj = method.invoke(o, args); // creates instance of an anonymous class