Skip to content

Commit

Permalink
[Dubbo -fix annotation bug] Fix @reference bug (apache#2649)
Browse files Browse the repository at this point in the history
It's fine.
  • Loading branch information
zonghaishang authored and CrazyHZM committed Dec 6, 2018
1 parent 4be238c commit fd0f223
Show file tree
Hide file tree
Showing 2 changed files with 180 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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()) {

Expand Down Expand Up @@ -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 {


Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
}

}

0 comments on commit fd0f223

Please sign in to comment.