From 8a44cf5fa5cb84e207699a80449ad985c785381d Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 2 Mar 2015 11:41:06 +0000 Subject: [PATCH] Simplfy RefreshScope By adding a proxyMode to the @RefreshScope annotation we no longer need to do all teh autoProxy stuff in GenericScope (apparently Spring does it for you now). See gh-96 --- .../config/annotation/RefreshScope.java | 2 +- .../cloud/context/scope/GenericScope.java | 131 +----------------- .../context/scope/refresh/RefreshScope.java | 3 +- .../refresh/RefreshScopeIntegrationTests.java | 6 +- 4 files changed, 7 insertions(+), 135 deletions(-) diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/config/annotation/RefreshScope.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/config/annotation/RefreshScope.java index 42bdee75..d46daa24 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/config/annotation/RefreshScope.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/config/annotation/RefreshScope.java @@ -42,6 +42,6 @@ public @interface RefreshScope { /** * @see Scope#proxyMode() */ - ScopedProxyMode proxyMode() default ScopedProxyMode.DEFAULT; + ScopedProxyMode proxyMode() default ScopedProxyMode.TARGET_CLASS; } diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/GenericScope.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/GenericScope.java index 719be07e..b28c72fe 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/GenericScope.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/GenericScope.java @@ -22,31 +22,22 @@ import java.util.UUID; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.aop.scope.ScopedProxyUtils; import org.springframework.beans.BeansException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.ObjectFactory; -import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.config.BeanDefinitionHolder; -import org.springframework.beans.factory.config.BeanDefinitionVisitor; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.Scope; -import org.springframework.beans.factory.support.BeanDefinitionReaderUtils; -import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.cloud.context.config.BeanLifecycleDecorator; import org.springframework.cloud.context.config.BeanLifecycleDecorator.Context; import org.springframework.cloud.context.config.StandardBeanLifecycleDecorator; -import org.springframework.context.expression.BeanFactoryAccessor; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.expression.ParseException; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import org.springframework.util.StringValueResolver; /** *

@@ -62,6 +53,8 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable private static final Log logger = LogFactory.getLog(GenericScope.class); + public static final String SCOPED_TARGET_PREFIX = "scopedTarget."; + private BeanLifecycleWrapperCache cache = new BeanLifecycleWrapperCache( new StandardScopeCache()); @@ -69,8 +62,6 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable private boolean proxyTargetClass = true; - private boolean autoProxy = true; - private ConfigurableListableBeanFactory beanFactory; private StandardEvaluationContext evaluationContext; @@ -108,21 +99,6 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable this.proxyTargetClass = proxyTargetClass; } - /** - * Flag to indicate that all scoped beans should automatically be proxied. If true - * then scoped beans can be injected as dependencies of another component and the - * concrete target will only be instantiated when it is used. Proxying is a huge - * advantage if the context storage for the scope cache is not available at - * configuration time (e.g. for thread-based, or other transient scopes). If this flag - * is false you can expect maybe to have to add extra meta-data to the bean - * definitions individually (e.g. <aop:scoped-proxy/> for an XML configuration). - * - * @param autoProxy the flag value to set, default is true - */ - public void setAutoProxy(boolean autoProxy) { - this.autoProxy = autoProxy; - } - /** * The cache implementation to use for bean instances in this scope. * @@ -219,36 +195,8 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { - beanFactory.registerScope(name, this); setSerializationId(beanFactory); - - this.beanFactory = beanFactory; - - evaluationContext = new StandardEvaluationContext(); - evaluationContext.addPropertyAccessor(new BeanFactoryAccessor()); - - if (!autoProxy) { - // No need to try and create proxies - return; - } - - Assert.state(beanFactory instanceof BeanDefinitionRegistry, - "BeanFactory was not a BeanDefinitionRegistry, so RefreshScope cannot be used."); - BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory; - - for (String beanName : beanFactory.getBeanDefinitionNames()) { - BeanDefinition definition = beanFactory.getBeanDefinition(beanName); - // Replace this or any of its inner beans with scoped proxy if it - // has this scope - boolean scoped = name.equals(definition.getScope()); - Scopifier scopifier = new Scopifier(registry, name, proxyTargetClass, scoped); - scopifier.visitBeanDefinition(definition); - if (scoped) { - createScopedProxy(beanName, definition, registry, proxyTargetClass); - } - } - } /** @@ -293,81 +241,6 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable return new IllegalStateException(throwable); } - private static BeanDefinitionHolder createScopedProxy(String beanName, - BeanDefinition definition, BeanDefinitionRegistry registry, - boolean proxyTargetClass) { - BeanDefinitionHolder proxyHolder = ScopedProxyUtils.createScopedProxy( - new BeanDefinitionHolder(definition, beanName), registry, - proxyTargetClass); - registry.registerBeanDefinition(beanName, proxyHolder.getBeanDefinition()); - return proxyHolder; - } - - /** - * Helper class to scan a bean definition hierarchy and force the use of auto-proxy - * for scoped beans. - * - * @author Dave Syer - * - */ - private static class Scopifier extends BeanDefinitionVisitor { - - private final boolean proxyTargetClass; - - private final BeanDefinitionRegistry registry; - - private final String scope; - - private final boolean scoped; - - public Scopifier(BeanDefinitionRegistry registry, String scope, - boolean proxyTargetClass, boolean scoped) { - super(new StringValueResolver() { - public String resolveStringValue(String value) { - return value; - } - }); - this.registry = registry; - this.proxyTargetClass = proxyTargetClass; - this.scope = scope; - this.scoped = scoped; - } - - @Override - protected Object resolveValue(Object value) { - - BeanDefinition definition = null; - String beanName = null; - if (value instanceof BeanDefinition) { - definition = (BeanDefinition) value; - beanName = BeanDefinitionReaderUtils.generateBeanName(definition, - registry); - } - else if (value instanceof BeanDefinitionHolder) { - BeanDefinitionHolder holder = (BeanDefinitionHolder) value; - definition = holder.getBeanDefinition(); - beanName = holder.getBeanName(); - } - - if (definition != null) { - boolean nestedScoped = scope.equals(definition.getScope()); - boolean scopeChangeRequiresProxy = !scoped && nestedScoped; - if (scopeChangeRequiresProxy) { - // Exit here so that nested inner bean definitions are not - // analysed - return createScopedProxy(beanName, definition, registry, - proxyTargetClass); - } - } - - // Nested inner bean definitions are recursively analysed here - value = super.resolveValue(value); - return value; - - } - - } - private static class BeanLifecycleWrapperCache { private final ScopeCache cache; diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java index b7429e5b..953e5103 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java @@ -16,11 +16,11 @@ package org.springframework.cloud.context.scope.refresh; import java.io.Serializable; import org.springframework.beans.BeansException; +import org.springframework.cloud.context.scope.GenericScope; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; -import org.springframework.cloud.context.scope.GenericScope; /** *

@@ -59,7 +59,6 @@ import org.springframework.cloud.context.scope.GenericScope; @ManagedResource public class RefreshScope extends GenericScope implements ApplicationContextAware { - protected static final String SCOPED_TARGET_PREFIX = "scopedTarget."; private ApplicationContext context; /** diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeIntegrationTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeIntegrationTests.java index 02886c47..e540a779 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeIntegrationTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeIntegrationTests.java @@ -30,6 +30,7 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; import org.springframework.cloud.context.config.annotation.RefreshScope; +import org.springframework.cloud.context.scope.GenericScope; import org.springframework.cloud.context.scope.refresh.RefreshScopeIntegrationTests.TestConfiguration; import org.springframework.context.ApplicationListener; import org.springframework.context.annotation.Bean; @@ -107,9 +108,8 @@ public class RefreshScopeIntegrationTests { assertEquals(1, ExampleService.getDestroyCount()); assertNotSame(id1, id2); assertNotNull(ExampleService.event); - assertEquals( - org.springframework.cloud.context.scope.refresh.RefreshScope.SCOPED_TARGET_PREFIX - + "service", ExampleService.event.getName()); + assertEquals(GenericScope.SCOPED_TARGET_PREFIX + "service", + ExampleService.event.getName()); } public static interface Service {