Annotation post-processors clear old InjectionMetadata registrations on refresh

Issue: SPR-12526

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).

spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java

@@ -230,7 +230,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);
 		}
 	}
@@ -326,7 +326,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);
 		}
@@ -344,7 +344,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);
 		}
@@ -354,7 +354,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.
@@ -363,6 +363,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);
+					}
 					try {
 						metadata = buildAutowiringMetadata(clazz);
 						this.injectionMetadataCache.put(cacheKey, metadata);

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<InjectedElement> checkedElements = new LinkedHashSet<InjectedElement>(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<InjectedElement> 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.
 		 */

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,9 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean
 			synchronized (this.injectionMetadataCache) {
 				metadata = this.injectionMetadataCache.get(cacheKey);
 				if (InjectionMetadata.needsRefresh(metadata, clazz)) {
+					if (metadata != null) {
+						metadata.clear(pvs);
+					}
 					try {
 						metadata = buildResourceMetadata(clazz);
 						this.injectionMetadataCache.put(cacheKey, metadata);

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 {
+	}
+
+}

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,9 @@ public class PersistenceAnnotationBeanPostProcessor
 			synchronized (this.injectionMetadataCache) {
 				metadata = this.injectionMetadataCache.get(cacheKey);
 				if (InjectionMetadata.needsRefresh(metadata, clazz)) {
+					if (metadata != null) {
+						metadata.clear(pvs);
+					}
 					try {
 						metadata = buildPersistenceMetadata(clazz);
 						this.injectionMetadataCache.put(cacheKey, metadata);