[Dubbo -fix annotation bug] Fix @reference bug (#2649)

zonghaishang · mercyblitz · commit d8282fe1b807 · 2018-10-17T10:14:45.000+08:00
It's fine.
diff --git a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessor.java b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessor.java
@@ -16,10 +16,12 @@
  */
 package org.apache.dubbo.config.spring.beans.factory.annotation;
 
-import org.apache.dubbo.config.annotation.Reference;
-import org.apache.dubbo.config.spring.ReferenceBean;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.dubbo.common.Constants;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.config.annotation.Reference;
+import org.apache.dubbo.config.spring.ReferenceBean;
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.PropertyValues;
@@ -35,15 +37,18 @@
 import org.springframework.core.PriorityOrdered;
 import org.springframework.core.env.Environment;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.ConcurrentReferenceHashMap;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
-import org.springframework.util.StringUtils;
 
 import java.beans.PropertyDescriptor;
+import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -83,6 +88,9 @@ public class ReferenceAnnotationBeanPostProcessor extends InstantiationAwareBean
     private final ConcurrentMap<String, ReferenceBean<?>> referenceBeansCache =
             new ConcurrentHashMap<String, ReferenceBean<?>>();
 
+    private static final Map<Class<? extends Annotation>, List<Method>> annotationMethodsCache =
+            new ConcurrentReferenceHashMap<Class<? extends Annotation>, List<Method>>(256);
+
     @Override
     public PropertyValues postProcessPropertyValues(
             PropertyValues pvs, PropertyDescriptor[] pds, Object bean, String beanName) throws BeanCreationException {
@@ -193,7 +201,7 @@ private ReferenceInjectionMetadata buildReferenceMetadata(final Class<?> beanCla
 
     private InjectionMetadata findReferenceMetadata(String beanName, Class<?> clazz, PropertyValues pvs) {
         // Fall back to class name as cache key, for backwards compatibility with custom callers.
-        String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName());
+        String cacheKey = (StringUtils.isNotEmpty(beanName) ? beanName : clazz.getName());
         // Quick check on the concurrent map first, with minimal locking.
         ReferenceInjectionMetadata metadata = this.injectionMetadataCache.get(cacheKey);
         if (InjectionMetadata.needsRefresh(metadata, clazz)) {
@@ -402,11 +410,7 @@ private ReferenceBean<?> buildReferenceBean(Reference reference, Class<?> refere
      */
     private String generateReferenceBeanCacheKey(Reference reference, Class<?> beanClass) {
 
-        String interfaceName = resolveInterfaceName(reference, beanClass);
-
-        String key = reference.url() + "/" + interfaceName +
-                "/" + reference.version() +
-                "/" + reference.group();
+        String key = resolveReferenceKey(annotationValues(reference));
 
         Environment environment = applicationContext.getEnvironment();
 
@@ -501,5 +505,90 @@ private <T> T getFieldValue(Object object, String fieldName, Class<T> fieldType)
 
     }
 
+    /**
+     * Generate a key based on the annotation.
+     *
+     * @param annotations annotatoin value
+     * @return unique key, never null will be returned.
+     * @since 2.7.0
+     */
+    private String resolveReferenceKey(Map<String, Object> annotations) {
+        Iterator<Map.Entry<String, Object>> annotationVisitor = annotations.entrySet().iterator();
+        StringBuilder builder = new StringBuilder();
+        while (annotationVisitor.hasNext()) {
+            Map.Entry<String, Object> attribute = annotationVisitor.next();
+            String attributeValue = null;
+            if (attribute.getValue() instanceof String[]) {
+                attributeValue = toPlainString((String[]) attribute.getValue());
+            } else {
+                attributeValue = attribute.getValue() == null ? "" : attribute.getValue().toString();
+            }
+
+            if (StringUtils.isNotEmpty(attributeValue)) {
+                if (builder.length() > 0) {
+                    builder.append(Constants.PATH_SEPARATOR);
+                }
+                builder.append(attributeValue);
+            }
+        }
+        return builder.toString();
+    }
+
+    private Map<String, Object> annotationValues(Annotation annotation) {
+        Map<String, Object> annotations = new LinkedHashMap<>();
 
+        for (Method method : getAnnotationMethods(annotation.annotationType())) {
+            try {
+                Object attributeValue = method.invoke(annotation);
+                Object defaultValue = method.getDefaultValue();
+                if (nullSafeEquals(attributeValue, defaultValue)) {
+                    continue;
+                }
+                annotations.put(method.getName(), attributeValue);
+            } catch (Throwable e) {
+                throw new IllegalStateException("Failed to obtain annotation attribute value for " + method, e);
+            }
+        }
+        return annotations;
+    }
+
+    private static List<Method> getAnnotationMethods(Class<? extends Annotation> annotationType) {
+        List<Method> methods = annotationMethodsCache.get(annotationType);
+        if (methods != null) {
+            return methods;
+        }
+
+        methods = new ArrayList<Method>();
+        for (Method method : annotationType.getDeclaredMethods()) {
+            if (isAnnotationMethod(method)) {
+                ReflectionUtils.makeAccessible(method);
+                methods.add(method);
+            }
+        }
+
+        annotationMethodsCache.put(annotationType, methods);
+        return methods;
+    }
+
+    private static boolean isAnnotationMethod(Method method) {
+        return (method != null
+                && method.getParameterTypes().length == 0
+                && method.getReturnType() != void.class);
+    }
+
+    private static boolean nullSafeEquals(Object first, Object another) {
+        return ObjectUtils.nullSafeEquals(first, another);
+    }
+
+    private String toPlainString(String[] array) {
+        if (array == null || array.length == 0) return "";
+        StringBuilder buffer = new StringBuilder();
+        for (int i = 0; i < array.length; i++) {
+            if (i > 0) {
+                buffer.append(Constants.COMMA_SEPARATOR);
+            }
+            buffer.append(array[i]);
+        }
+        return buffer.toString();
+    }
 }
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
@@ -106,7 +106,12 @@ public void testGetReferenceBeans() {
 
         Collection<ReferenceBean<?>> referenceBeans = beanPostProcessor.getReferenceBeans();
 
-        Assert.assertEquals(1, referenceBeans.size());
+        /**
+         * 1 -> demoService、demoServiceShouldBeSame
+         * 1 -> demoServiceShouldNotBeSame
+         * 1 -> demoServiceWithArray、demoServiceWithArrayShouldBeSame
+         */
+        Assert.assertEquals(3, referenceBeans.size());
 
         ReferenceBean<?> referenceBean = referenceBeans.iterator().next();
 
@@ -130,7 +135,10 @@ public void testGetInjectedFieldReferenceBeanMap() {
         Map<InjectionMetadata.InjectedElement, ReferenceBean<?>> referenceBeanMap =
                 beanPostProcessor.getInjectedFieldReferenceBeanMap();
 
-        Assert.assertEquals(1, referenceBeanMap.size());
+        /**
+         * contains 5 fields.
+         */
+        Assert.assertEquals(5, referenceBeanMap.size());
 
         for (Map.Entry<InjectionMetadata.InjectedElement, ReferenceBean<?>> entry : referenceBeanMap.entrySet()) {
 
@@ -197,6 +205,49 @@ public void testModuleInfo() {
         }
     }
 
+    @Test
+    public void testReferenceCache() throws Exception {
+
+        AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestBean.class);
+
+        TestBean testBean = context.getBean(TestBean.class);
+
+        Assert.assertNotNull(testBean.getDemoServiceFromAncestor());
+        Assert.assertNotNull(testBean.getDemoServiceFromParent());
+        Assert.assertNotNull(testBean.getDemoService());
+
+        Assert.assertEquals(testBean.getDemoServiceFromAncestor(), testBean.getDemoServiceFromParent());
+        Assert.assertEquals(testBean.getDemoService(), testBean.getDemoServiceFromParent());
+
+        DemoService demoService = testBean.getDemoService();
+
+        Assert.assertEquals(demoService, testBean.getDemoServiceShouldBeSame());
+        Assert.assertNotEquals(demoService, testBean.getDemoServiceShouldNotBeSame());
+
+        context.close();
+
+    }
+
+    @Test
+    public void testReferenceCacheWithArray() throws Exception {
+
+        AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestBean.class);
+
+        TestBean testBean = context.getBean(TestBean.class);
+
+        Assert.assertNotNull(testBean.getDemoServiceFromAncestor());
+        Assert.assertNotNull(testBean.getDemoServiceFromParent());
+        Assert.assertNotNull(testBean.getDemoService());
+
+        Assert.assertEquals(testBean.getDemoServiceFromAncestor(), testBean.getDemoServiceFromParent());
+        Assert.assertEquals(testBean.getDemoService(), testBean.getDemoServiceFromParent());
+
+        Assert.assertEquals(testBean.getDemoServiceWithArray(), testBean.getDemoServiceWithArrayShouldBeSame());
+
+        context.close();
+
+    }
+
     private static class AncestorBean {
 
 
@@ -239,6 +290,19 @@ static class TestBean extends ParentBean {
 
         private DemoService demoService;
 
+        @Reference(version = "1.2", url = "dubbo://127.0.0.1:12345")
+        private DemoService demoServiceShouldBeSame;
+
+        @Reference(version = "1.2", url = "dubbo://127.0.0.1:12345", async = true)
+        private DemoService demoServiceShouldNotBeSame;
+
+
+        @Reference(version = "1.2", url = "dubbo://127.0.0.1:12345", parameters = { "key1", "value1"})
+        private DemoService demoServiceWithArray;
+
+        @Reference(version = "1.2", url = "dubbo://127.0.0.1:12345", parameters = { "key1", "value1"})
+        private DemoService demoServiceWithArrayShouldBeSame;
+
         @Autowired
         private ApplicationContext applicationContext;
 
@@ -250,6 +314,22 @@ public DemoService getDemoService() {
         public void setDemoService(DemoService demoService) {
             this.demoService = demoService;
         }
+
+        public DemoService getDemoServiceShouldNotBeSame() {
+            return demoServiceShouldNotBeSame;
+        }
+
+        public DemoService getDemoServiceShouldBeSame() {
+            return demoServiceShouldBeSame;
+        }
+
+        public DemoService getDemoServiceWithArray() {
+            return demoServiceWithArray;
+        }
+
+        public DemoService getDemoServiceWithArrayShouldBeSame() {
+            return demoServiceWithArrayShouldBeSame;
+        }
     }
 
 }
