From 5795f896a0ef6fcdd4b1ee7e02277a5f775779bc Mon Sep 17 00:00:00 2001 From: zonghaishang Date: Tue, 16 Oct 2018 19:15:53 +0800 Subject: [PATCH] fix issue: https://github.com/apache/incubator-dubbo/issues/2522 & https://github.com/apache/incubator-dubbo/issues/2194 --- .../ReferenceAnnotationBeanPostProcessor.java | 107 ++++++++++++++++-- ...erenceAnnotationBeanPostProcessorTest.java | 72 ++++++++++++ 2 files changed, 170 insertions(+), 9 deletions(-) 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 index f89378620ff..ed0fb5c4205 100644 --- 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> referenceBeansCache = new ConcurrentHashMap>(); + private static final Map, List> annotationMethodsCache = + new ConcurrentReferenceHashMap, List>(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 getFieldValue(Object object, String fieldName, Class 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 annotations) { + Iterator> annotationVisitor = annotations.entrySet().iterator(); + StringBuilder builder = new StringBuilder(); + while (annotationVisitor.hasNext()) { + Map.Entry 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 annotationValues(Annotation annotation) { + Map 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 getAnnotationMethods(Class annotationType) { + List methods = annotationMethodsCache.get(annotationType); + if (methods != null) { + return methods; + } + + methods = new ArrayList(); + 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 index e45b0255632..f77f3a8610c 100644 --- 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 @@ -197,6 +197,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 +282,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 +306,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; + } } } \ No newline at end of file