This is an automated email from the ASF dual-hosted git repository. mercyblitz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/master by this push: new 95bd350 Polish /apache/dubbo#5446 : With @Reference, Aop does not work (#5717) 95bd350 is described below commit 95bd350cae95c0f738eade5ba78ad9827586135d Author: Mercy Ma <mercybl...@gmail.com> AuthorDate: Fri Feb 7 12:19:35 2020 +0800 Polish /apache/dubbo#5446 : With @Reference, Aop does not work (#5717) * Polish /apache/dubbo#5446 : With @Reference, Aop does not work * Polish /apache/dubbo#5718 : [Infrastructure] Uprade com.alibaba.spring:spring-context-support 1.0.6 * Polish /apache/dubbo#5446 : With @Reference, Aop does not work --- dubbo-config/dubbo-config-spring/pom.xml | 7 + .../ReferenceAnnotationBeanPostProcessor.java | 147 +++++++++++++-------- .../ReferenceAnnotationBeanPostProcessorTest.java | 37 ++++-- dubbo-dependencies-bom/pom.xml | 2 +- 4 files changed, 127 insertions(+), 66 deletions(-) diff --git a/dubbo-config/dubbo-config-spring/pom.xml b/dubbo-config/dubbo-config-spring/pom.xml index e88fc2d..74cc929 100644 --- a/dubbo-config/dubbo-config-spring/pom.xml +++ b/dubbo-config/dubbo-config-spring/pom.xml @@ -57,6 +57,13 @@ <artifactId>spring-context-support</artifactId> </dependency> + <!-- Testing Dependencies --> + <dependency> + <groupId>org.aspectj</groupId> + <artifactId>aspectjweaver</artifactId> + <version>1.9.5</version> + <scope>test</scope> + </dependency> <dependency> <groupId>org.apache.dubbo</groupId> <artifactId>dubbo-registry-default</artifactId> 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 e3f23ec..3fea6ec 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 @@ -30,7 +30,6 @@ import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.core.annotation.AnnotationAttributes; @@ -57,7 +56,7 @@ import static org.springframework.util.StringUtils.hasText; * @since 2.5.7 */ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBeanPostProcessor implements - ApplicationContextAware, ApplicationListener { + ApplicationContextAware, ApplicationListener<ServiceBeanExportedEvent> { /** * The bean name of {@link ReferenceAnnotationBeanPostProcessor} @@ -72,15 +71,15 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean private final ConcurrentMap<String, ReferenceBean<?>> referenceBeanCache = new ConcurrentHashMap<>(CACHE_SIZE); - private final ConcurrentHashMap<String, ReferenceBeanInvocationHandler> localReferenceBeanInvocationHandlerCache = - new ConcurrentHashMap<>(CACHE_SIZE); - private final ConcurrentMap<InjectionMetadata.InjectedElement, ReferenceBean<?>> injectedFieldReferenceBeanCache = new ConcurrentHashMap<>(CACHE_SIZE); private final ConcurrentMap<InjectionMetadata.InjectedElement, ReferenceBean<?>> injectedMethodReferenceBeanCache = new ConcurrentHashMap<>(CACHE_SIZE); + private final ConcurrentMap<String, ReferencedBeanInvocationHandler> referencedBeanInvocationHandlersCache = + new ConcurrentHashMap<>(); + private ApplicationContext applicationContext; /** @@ -135,11 +134,13 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean ReferenceBean referenceBean = buildReferenceBeanIfAbsent(referenceBeanName, attributes, injectedType); - registerReferenceBean(referencedBeanName, referenceBean, attributes, injectedType); + boolean localServiceBean = isLocalServiceBean(referencedBeanName, referenceBean, attributes); + + registerReferenceBean(referencedBeanName, referenceBean, attributes, localServiceBean, injectedType); cacheInjectedReferenceBean(referenceBean, injectedElement); - return getOrCreateProxy(referencedBeanName, referenceBeanName, attributes, injectedType); + return getOrCreateProxy(referencedBeanName, referenceBean, localServiceBean, injectedType); } /** @@ -148,18 +149,19 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean * @param referencedBeanName The name of bean that annotated Dubbo's {@link Service @Service} in the Spring {@link ApplicationContext} * @param referenceBean the instance of {@link ReferenceBean} is about to register into the Spring {@link ApplicationContext} * @param attributes the {@link AnnotationAttributes attributes} of {@link Reference @Reference} + * @param localServiceBean Is Local Service bean or not * @param interfaceClass the {@link Class class} of Service interface * @since 2.7.3 */ private void registerReferenceBean(String referencedBeanName, ReferenceBean referenceBean, AnnotationAttributes attributes, - Class<?> interfaceClass) { + boolean localServiceBean, Class<?> interfaceClass) { ConfigurableListableBeanFactory beanFactory = getBeanFactory(); String beanName = getReferenceBeanName(attributes, interfaceClass); - if (existsServiceBean(referencedBeanName)) { // If @Service bean is local one + if (localServiceBean) { // If @Service bean is local one /** * Get the @Service's BeanDefinition from {@link BeanFactory} * Refer to {@link ServiceAnnotationBeanPostProcessor#buildServiceBeanDefinition} @@ -223,63 +225,108 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean return beanNameBuilder.toString(); } + /** + * Is Local Service bean or not? + * + * @param referencedBeanName the bean name to the referenced bean + * @return If the target referenced bean is existed, return <code>true</code>, or <code>false</code> + * @since 2.7.6 + */ + private boolean isLocalServiceBean(String referencedBeanName, ReferenceBean referenceBean, AnnotationAttributes attributes) { + return existsServiceBean(referencedBeanName) && !isRemoteReferenceBean(referenceBean, attributes); + } + + /** + * Check the {@link ServiceBean} is exited or not + * + * @param referencedBeanName the bean name to the referenced bean + * @return if exists, return <code>true</code>, or <code>false</code> + * @revised 2.7.6 + */ private boolean existsServiceBean(String referencedBeanName) { - return applicationContext.containsBean(referencedBeanName); + return applicationContext.containsBean(referencedBeanName) && + applicationContext.isTypeMatch(referencedBeanName, ServiceBean.class); + + } + + private boolean isRemoteReferenceBean(ReferenceBean referenceBean, AnnotationAttributes attributes) { + boolean remote = Boolean.FALSE.equals(referenceBean.isInjvm()) || Boolean.FALSE.equals(attributes.get("injvm")); + return remote; } /** * Get or Create a proxy of {@link ReferenceBean} for the specified the type of Dubbo service interface * * @param referencedBeanName The name of bean that annotated Dubbo's {@link Service @Service} in the Spring {@link ApplicationContext} - * @param referenceBeanName the bean name of {@link ReferenceBean} - * @param attributes the {@link AnnotationAttributes attributes} of {@link Reference @Reference} + * @param referenceBean the instance of {@link ReferenceBean} + * @param localServiceBean Is Local Service bean or not * @param serviceInterfaceType the type of Dubbo service interface * @return non-null * @since 2.7.4 */ - private Object getOrCreateProxy(String referencedBeanName, String referenceBeanName, AnnotationAttributes attributes, Class<?> serviceInterfaceType) { - - String beanName = getReferenceBeanName(attributes, serviceInterfaceType); + private Object getOrCreateProxy(String referencedBeanName, ReferenceBean referenceBean, boolean localServiceBean, + Class<?> serviceInterfaceType) { + if (localServiceBean) { // If the local @Service Bean exists, build a proxy of Service + return newProxyInstance(getClassLoader(), new Class[]{serviceInterfaceType}, + newReferencedBeanInvocationHandler(referencedBeanName)); + } else { + exportServiceBeanIfNecessary(referencedBeanName); // If the referenced ServiceBean exits, export it immediately + return referenceBean.get(); + } + } - ConfigurableListableBeanFactory beanFactory = getBeanFactory(); - // Get remote or local @Service Bean - Object bean = beanFactory.getBean(beanName); + private void exportServiceBeanIfNecessary(String referencedBeanName) { if (existsServiceBean(referencedBeanName)) { - // If the local @Service Bean exists, build a proxy of ReferenceBean - return newProxyInstance(getClassLoader(), new Class[]{serviceInterfaceType}, - wrapInvocationHandler(referenceBeanName, bean)); - } else { - return bean; + ServiceBean serviceBean = getServiceBean(referencedBeanName); + if (!serviceBean.isExported()) { + serviceBean.export(); + } } } + private ServiceBean getServiceBean(String referencedBeanName) { + return applicationContext.getBean(referencedBeanName, ServiceBean.class); + } + + private InvocationHandler newReferencedBeanInvocationHandler(String referencedBeanName) { + return referencedBeanInvocationHandlersCache.computeIfAbsent(referencedBeanName, + ReferencedBeanInvocationHandler::new); + } + /** - * Wrap an instance of {@link InvocationHandler} that is used to get the proxy of {@link ReferenceBean} after - * the specified local referenced bean that annotated {@link @Service} exported. - * - * @param referenceBeanName the bean name of {@link ReferenceBean} - * @param referenceBean the instance of {@link ReferenceBean} - * @return non-null - * @since 2.7.4 + * The {@link InvocationHandler} class for the referenced Bean */ - private InvocationHandler wrapInvocationHandler(String referenceBeanName, Object referenceBean) { - return localReferenceBeanInvocationHandlerCache.computeIfAbsent(referenceBeanName, name -> - new ReferenceBeanInvocationHandler(referenceBean)); + @Override + public void onApplicationEvent(ServiceBeanExportedEvent event) { + initReferencedBeanInvocationHandler(event.getServiceBean()); + } + + private void initReferencedBeanInvocationHandler(ServiceBean serviceBean) { + String serviceBeanName = serviceBean.getBeanName(); + referencedBeanInvocationHandlersCache.computeIfPresent(serviceBeanName, (name, handler) -> { + handler.init(); + return null; + }); } - private static class ReferenceBeanInvocationHandler implements InvocationHandler { + private class ReferencedBeanInvocationHandler implements InvocationHandler { + + private final String referencedBeanName; private Object bean; - private ReferenceBeanInvocationHandler(Object bean) { - this.bean = bean; + private ReferencedBeanInvocationHandler(String referencedBeanName) { + this.referencedBeanName = referencedBeanName; } @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { - Object result; + Object result = null; try { + if (bean == null) { + init(); + } result = method.invoke(bean, args); } catch (InvocationTargetException e) { // re-throws the actual Exception. @@ -287,6 +334,11 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean } return result; } + + private void init() { + ServiceBean serviceBean = applicationContext.getBean(referencedBeanName, ServiceBean.class); + this.bean = serviceBean.getRef(); + } } @Override @@ -341,29 +393,10 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean } @Override - public void onApplicationEvent(ApplicationEvent event) { - if (event instanceof ServiceBeanExportedEvent) { - onServiceBeanExportEvent((ServiceBeanExportedEvent) event); - } - } - - private void onServiceBeanExportEvent(ServiceBeanExportedEvent event) { - ServiceBean serviceBean = event.getServiceBean(); - initReferenceBeanInvocationHandler(serviceBean); - } - - private void initReferenceBeanInvocationHandler(ServiceBean serviceBean) { - String serviceBeanName = serviceBean.getBeanName(); - // Remove ServiceBean when it's exported - localReferenceBeanInvocationHandlerCache.remove(serviceBeanName); - } - - - @Override public void destroy() throws Exception { super.destroy(); this.referenceBeanCache.clear(); - this.localReferenceBeanInvocationHandlerCache.clear(); + this.referencedBeanInvocationHandlersCache.clear(); this.injectedFieldReferenceBeanCache.clear(); this.injectedMethodReferenceBeanCache.clear(); } 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 c213d78..679d46d 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 @@ -23,6 +23,9 @@ import org.apache.dubbo.config.spring.api.DemoService; import org.apache.dubbo.config.spring.api.HelloService; import org.apache.dubbo.config.utils.ReferenceConfigCache; +import org.aspectj.lang.ProceedingJoinPoint; +import org.aspectj.lang.annotation.Around; +import org.aspectj.lang.annotation.Aspect; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,6 +35,8 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.stereotype.Component; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; @@ -51,15 +56,29 @@ import static org.junit.Assert.assertTrue; @ContextConfiguration( classes = { ServiceAnnotationTestConfiguration.class, - ReferenceAnnotationBeanPostProcessorTest.class + ReferenceAnnotationBeanPostProcessorTest.class, + ReferenceAnnotationBeanPostProcessorTest.TestAspect.class }) @TestPropertySource(properties = { "packagesToScan = org.apache.dubbo.config.spring.context.annotation.provider", "consumer.version = ${demo.service.version}", "consumer.url = dubbo://127.0.0.1:12345?version=2.5.7", }) +@EnableAspectJAutoProxy(proxyTargetClass = true, exposeProxy = true) public class ReferenceAnnotationBeanPostProcessorTest { + private static final String AOP_SUFFIX = "(based on AOP)"; + + @Aspect + @Component + public static class TestAspect { + + @Around("execution(* org.apache.dubbo.config.spring.context.annotation.provider.DemoServiceImpl.*(..))") + public Object aroundApi(ProceedingJoinPoint pjp) throws Throwable { + return pjp.proceed() + AOP_SUFFIX; + } + } + @Bean public TestBean testBean() { return new TestBean(); @@ -105,27 +124,29 @@ public class ReferenceAnnotationBeanPostProcessorTest { Assert.assertNotNull(testBean.autowiredDemoService); Assert.assertEquals(1, demoServicesMap.size()); - Assert.assertEquals("Hello,Mercy", demoService.sayName("Mercy")); + String expectedResult = "Hello,Mercy" + AOP_SUFFIX; + + Assert.assertEquals(expectedResult, testBean.autowiredDemoService.sayName("Mercy")); + Assert.assertEquals(expectedResult, demoService.sayName("Mercy")); Assert.assertEquals("Greeting, Mercy", defaultHelloService.sayHello("Mercy")); Assert.assertEquals("Hello, Mercy", helloServiceImpl.sayHello("Mercy")); Assert.assertEquals("Greeting, Mercy", helloService.sayHello("Mercy")); - Assert.assertEquals("Hello,Mercy", testBean.getDemoServiceFromAncestor().sayName("Mercy")); - Assert.assertEquals("Hello,Mercy", testBean.getDemoServiceFromParent().sayName("Mercy")); - Assert.assertEquals("Hello,Mercy", testBean.getDemoService().sayName("Mercy")); - Assert.assertEquals("Hello,Mercy", testBean.autowiredDemoService.sayName("Mercy")); + Assert.assertEquals(expectedResult, testBean.getDemoServiceFromAncestor().sayName("Mercy")); + Assert.assertEquals(expectedResult, testBean.getDemoServiceFromParent().sayName("Mercy")); + Assert.assertEquals(expectedResult, testBean.getDemoService().sayName("Mercy")); DemoService myDemoService = context.getBean("my-reference-bean", DemoService.class); - Assert.assertEquals("Hello,Mercy", myDemoService.sayName("Mercy")); + Assert.assertEquals(expectedResult, myDemoService.sayName("Mercy")); for (DemoService demoService1 : demoServicesMap.values()) { Assert.assertEquals(myDemoService, demoService1); - Assert.assertEquals("Hello,Mercy", demoService1.sayName("Mercy")); + Assert.assertEquals(expectedResult, demoService1.sayName("Mercy")); } } diff --git a/dubbo-dependencies-bom/pom.xml b/dubbo-dependencies-bom/pom.xml index 261b180..5b0b323 100644 --- a/dubbo-dependencies-bom/pom.xml +++ b/dubbo-dependencies-bom/pom.xml @@ -145,7 +145,7 @@ <eureka.version>1.9.12</eureka.version> <!-- Alibaba --> - <alibaba_spring_context_support_version>1.0.5</alibaba_spring_context_support_version> + <alibaba_spring_context_support_version>1.0.6</alibaba_spring_context_support_version> <jaxb_version>2.2.7</jaxb_version> <activation_version>1.2.0</activation_version>