From e118086bd11f96ba4c36c3ce668867faeea13431 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 29 Dec 2014 18:56:44 +0100 Subject: [PATCH] Annotation post-processors clear old InjectionMetadata registrations on refresh Issue: SPR-12526 (cherry picked from commit 809ee0d) --- .../beans/MutablePropertyValues.java | 12 +- .../AutowiredAnnotationBeanPostProcessor.java | 11 +- .../factory/annotation/InjectionMetadata.java | 36 +++- .../CommonAnnotationBeanPostProcessor.java | 10 +- .../configuration/Spr12526Tests.java | 155 ++++++++++++++++++ ...ersistenceAnnotationBeanPostProcessor.java | 10 +- 6 files changed, 219 insertions(+), 15 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/configuration/Spr12526Tests.java diff --git a/spring-beans/src/main/java/org/springframework/beans/MutablePropertyValues.java b/spring-beans/src/main/java/org/springframework/beans/MutablePropertyValues.java index 693dcffbc1..d5653a3261 100644 --- a/spring-beans/src/main/java/org/springframework/beans/MutablePropertyValues.java +++ b/spring-beans/src/main/java/org/springframework/beans/MutablePropertyValues.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -320,6 +320,16 @@ public class MutablePropertyValues implements PropertyValues, Serializable { this.processedProperties.add(propertyName); } + /** + * Clear the "processed" registration of the given property, if any. + * @since 3.2.13 + */ + public void clearProcessedProperty(String propertyName) { + if (this.processedProperties != null) { + this.processedProperties.remove(propertyName); + } + } + /** * Mark this holder as containing converted values only * (i.e. no runtime resolution needed anymore). diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java index 49a0b88f42..6a9bbcf09b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java @@ -218,7 +218,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean @Override public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class beanType, String beanName) { if (beanType != null) { - InjectionMetadata metadata = findAutowiringMetadata(beanName, beanType); + InjectionMetadata metadata = findAutowiringMetadata(beanName, beanType, null); metadata.checkConfigMembers(beanDefinition); } } @@ -293,7 +293,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean public PropertyValues postProcessPropertyValues( PropertyValues pvs, PropertyDescriptor[] pds, Object bean, String beanName) throws BeansException { - InjectionMetadata metadata = findAutowiringMetadata(beanName, bean.getClass()); + InjectionMetadata metadata = findAutowiringMetadata(beanName, bean.getClass(), pvs); try { metadata.inject(bean, beanName, pvs); } @@ -311,7 +311,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean */ public void processInjection(Object bean) throws BeansException { Class clazz = bean.getClass(); - InjectionMetadata metadata = findAutowiringMetadata(clazz.getName(), clazz); + InjectionMetadata metadata = findAutowiringMetadata(clazz.getName(), clazz, null); try { metadata.inject(bean, null, null); } @@ -321,7 +321,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean } - private InjectionMetadata findAutowiringMetadata(String beanName, Class clazz) { + private InjectionMetadata findAutowiringMetadata(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()); // Quick check on the concurrent map first, with minimal locking. @@ -330,6 +330,9 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean synchronized (this.injectionMetadataCache) { metadata = this.injectionMetadataCache.get(cacheKey); if (InjectionMetadata.needsRefresh(metadata, clazz)) { + if (metadata != null) { + metadata.clear(pvs); + } metadata = buildAutowiringMetadata(clazz); this.injectionMetadataCache.put(cacheKey, metadata); } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java index 636b103764..9cb34e62ee 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -46,7 +46,7 @@ import org.springframework.util.ReflectionUtils; */ public class InjectionMetadata { - private final Log logger = LogFactory.getLog(InjectionMetadata.class); + private static final Log logger = LogFactory.getLog(InjectionMetadata.class); private final Class targetClass; @@ -60,6 +60,7 @@ public class InjectionMetadata { this.injectedElements = elements; } + public void checkConfigMembers(RootBeanDefinition beanDefinition) { Set checkedElements = new LinkedHashSet(this.injectedElements.size()); for (InjectedElement element : this.injectedElements) { @@ -82,13 +83,26 @@ public class InjectionMetadata { boolean debug = logger.isDebugEnabled(); for (InjectedElement element : elementsToIterate) { if (debug) { - logger.debug("Processing injected method of bean '" + beanName + "': " + element); + logger.debug("Processing injected element of bean '" + beanName + "': " + element); } element.inject(target, beanName, pvs); } } } + /** + * @since 3.2.13 + */ + public void clear(PropertyValues pvs) { + Collection elementsToIterate = + (this.checkedElements != null ? this.checkedElements : this.injectedElements); + if (!elementsToIterate.isEmpty()) { + for (InjectedElement element : elementsToIterate) { + element.clearPropertySkipping(pvs); + } + } + } + public static boolean needsRefresh(InjectionMetadata metadata, Class clazz) { return (metadata == null || !metadata.targetClass.equals(clazz)); @@ -170,7 +184,7 @@ public class InjectionMetadata { } /** - * Checks whether this injector's property needs to be skipped due to + * Check whether this injector's property needs to be skipped due to * an explicit property value having been specified. Also marks the * affected property as processed for other processors to ignore it. */ @@ -201,6 +215,20 @@ public class InjectionMetadata { } } + /** + * @since 3.2.13 + */ + protected void clearPropertySkipping(PropertyValues pvs) { + if (pvs == null) { + return; + } + synchronized (pvs) { + if (Boolean.FALSE.equals(this.skip) && this.pd != null && pvs instanceof MutablePropertyValues) { + ((MutablePropertyValues) pvs).clearProcessedProperty(this.pd.getName()); + } + } + } + /** * Either this or {@link #inject} needs to be overridden. */ diff --git a/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java index 63ce5c90fa..70956b5a0f 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java @@ -281,7 +281,7 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class beanType, String beanName) { super.postProcessMergedBeanDefinition(beanDefinition, beanType, beanName); if (beanType != null) { - InjectionMetadata metadata = findResourceMetadata(beanName, beanType); + InjectionMetadata metadata = findResourceMetadata(beanName, beanType, null); metadata.checkConfigMembers(beanDefinition); } } @@ -300,7 +300,7 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean public PropertyValues postProcessPropertyValues( PropertyValues pvs, PropertyDescriptor[] pds, Object bean, String beanName) throws BeansException { - InjectionMetadata metadata = findResourceMetadata(beanName, bean.getClass()); + InjectionMetadata metadata = findResourceMetadata(beanName, bean.getClass(), pvs); try { metadata.inject(bean, beanName, pvs); } @@ -311,7 +311,7 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean } - private InjectionMetadata findResourceMetadata(String beanName, final Class clazz) { + private InjectionMetadata findResourceMetadata(String beanName, final 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()); // Quick check on the concurrent map first, with minimal locking. @@ -320,6 +320,10 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean synchronized (this.injectionMetadataCache) { metadata = this.injectionMetadataCache.get(cacheKey); if (InjectionMetadata.needsRefresh(metadata, clazz)) { + if (metadata != null) { + metadata.clear(pvs); + } + LinkedList elements = new LinkedList(); Class targetClass = clazz; diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/Spr12526Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/Spr12526Tests.java new file mode 100644 index 0000000000..7c4982032a --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/Spr12526Tests.java @@ -0,0 +1,155 @@ +/* + * Copyright 2002-2014 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.context.annotation.configuration; + +import javax.annotation.Resource; + +import org.junit.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Scope; + +import static org.junit.Assert.*; +import static org.springframework.beans.factory.config.BeanDefinition.*; + +/** + * @author Marcin Piela + * @author Juergen Hoeller + */ +public class Spr12526Tests { + + @Test + public void testInjection() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(TestContext.class); + CustomCondition condition = ctx.getBean(CustomCondition.class); + + condition.setCondition(true); + FirstService firstService = (FirstService) ctx.getBean(Service.class); + assertNotNull("FirstService.dependency is null", firstService.getDependency()); + + condition.setCondition(false); + SecondService secondService = (SecondService) ctx.getBean(Service.class); + assertNotNull("SecondService.dependency is null", secondService.getDependency()); + } + + + @Configuration + public static class TestContext { + + @Bean + @Scope(SCOPE_SINGLETON) + public CustomCondition condition() { + return new CustomCondition(); + } + + + @Bean + @Scope(SCOPE_PROTOTYPE) + public Service service(CustomCondition condition) { + return (condition.check() ? new FirstService() : new SecondService()); + } + + @Bean + public DependencyOne dependencyOne() { + return new DependencyOne(); + } + + + @Bean + public DependencyTwo dependencyTwo() { + return new DependencyTwo(); + } + } + + + public static class CustomCondition { + + private boolean condition; + + public boolean check() { + return condition; + } + + public void setCondition(boolean value) { + this.condition = value; + } + } + + + public interface Service { + + void doStuff(); + } + + + public static class FirstService implements Service { + + private DependencyOne dependency; + + + @Override + public void doStuff() { + if (dependency == null) { + throw new IllegalStateException("FirstService: dependency is null"); + } + } + + @Resource(name = "dependencyOne") + public void setDependency(DependencyOne dependency) { + this.dependency = dependency; + } + + + public DependencyOne getDependency() { + return dependency; + } + } + + + public static class SecondService implements Service { + + private DependencyTwo dependency; + + @Override + public void doStuff() { + if (dependency == null) { + throw new IllegalStateException("SecondService: dependency is null"); + } + } + + @Resource(name = "dependencyTwo") + public void setDependency(DependencyTwo dependency) { + this.dependency = dependency; + } + + + public DependencyTwo getDependency() { + return dependency; + } + } + + + public static class DependencyOne { + } + + + public static class DependencyTwo { + } + +} diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java index 6ccaeebc28..b970c78a08 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java @@ -330,7 +330,7 @@ public class PersistenceAnnotationBeanPostProcessor @Override public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class beanType, String beanName) { if (beanType != null) { - InjectionMetadata metadata = findPersistenceMetadata(beanName, beanType); + InjectionMetadata metadata = findPersistenceMetadata(beanName, beanType, null); metadata.checkConfigMembers(beanDefinition); } } @@ -349,7 +349,7 @@ public class PersistenceAnnotationBeanPostProcessor public PropertyValues postProcessPropertyValues( PropertyValues pvs, PropertyDescriptor[] pds, Object bean, String beanName) throws BeansException { - InjectionMetadata metadata = findPersistenceMetadata(beanName, bean.getClass()); + InjectionMetadata metadata = findPersistenceMetadata(beanName, bean.getClass(), pvs); try { metadata.inject(bean, beanName, pvs); } @@ -376,7 +376,7 @@ public class PersistenceAnnotationBeanPostProcessor } - private InjectionMetadata findPersistenceMetadata(String beanName, final Class clazz) { + private InjectionMetadata findPersistenceMetadata(String beanName, final 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()); // Quick check on the concurrent map first, with minimal locking. @@ -385,6 +385,10 @@ public class PersistenceAnnotationBeanPostProcessor synchronized (this.injectionMetadataCache) { metadata = this.injectionMetadataCache.get(cacheKey); if (InjectionMetadata.needsRefresh(metadata, clazz)) { + if (metadata != null) { + metadata.clear(pvs); + } + LinkedList elements = new LinkedList(); Class targetClass = clazz;