SPR-6094: PostConstruct now processed top-down (base class first); @PreDestroy...

SPR-6094: PostConstruct now processed top-down (base class first); @PreDestroy processed bottom-up (subclass first)

org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java

@@ -21,10 +21,10 @@ import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.Collection;
 import java.util.Iterator;
-import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.logging.Log;
@@ -119,18 +119,18 @@ public class InitDestroyAnnotationBeanPostProcessor
 		if (beanType != null) {
 			LifecycleMetadata metadata = findLifecycleMetadata(beanType);
 			for (Iterator<LifecycleElement> it = metadata.getInitMethods().iterator(); it.hasNext();) {
-				String methodName = calculateMethodIdentifierInHierarchy(it.next().getMethod());
-				if (!beanDefinition.isExternallyManagedInitMethod(methodName)) {
-					beanDefinition.registerExternallyManagedInitMethod(methodName);
+				String methodIdentifier = it.next().getIdentifier();
+				if (!beanDefinition.isExternallyManagedInitMethod(methodIdentifier)) {
+					beanDefinition.registerExternallyManagedInitMethod(methodIdentifier);
 				}
 				else {
 					it.remove();
 				}
 			}
 			for (Iterator<LifecycleElement> it = metadata.getDestroyMethods().iterator(); it.hasNext();) {
-				String methodName = calculateMethodIdentifierInHierarchy(it.next().getMethod());
-				if (!beanDefinition.isExternallyManagedDestroyMethod(methodName)) {
-					beanDefinition.registerExternallyManagedDestroyMethod(methodName);
+				String methodIdentifier = it.next().getIdentifier();
+				if (!beanDefinition.isExternallyManagedDestroyMethod(methodIdentifier)) {
+					beanDefinition.registerExternallyManagedDestroyMethod(methodIdentifier);
 				}
 				else {
 					it.remove();
@@ -177,15 +177,6 @@ public class InitDestroyAnnotationBeanPostProcessor
 	}
 
 
-	private String calculateMethodIdentifierInHierarchy(Method method) {
-		if (Modifier.isPrivate(method.getModifiers())) {
-			return method.getDeclaringClass() + "." + method.getName();
-		}
-		else {
-			return method.getName();
-		}
-	}
-
 	private LifecycleMetadata findLifecycleMetadata(Class clazz) {
 		if (this.lifecycleMetadataCache == null) {
 			// Happens after deserialization, during destruction...
@@ -238,15 +229,18 @@ public class InitDestroyAnnotationBeanPostProcessor
 	 */
 	private class LifecycleMetadata {
 
-		private final Set<LifecycleElement> initMethods = new LinkedHashSet<LifecycleElement>();
+		private final LinkedList<LifecycleElement> initMethods = new LinkedList<LifecycleElement>();
 
-		private final Set<LifecycleElement> destroyMethods = new LinkedHashSet<LifecycleElement>();
+		private final LinkedList<LifecycleElement> destroyMethods = new LinkedList<LifecycleElement>();
 
 		public void addInitMethod(Method method) {
-			this.initMethods.add(new LifecycleElement(method));
+			LifecycleElement element = new LifecycleElement(method);
+			if (!this.initMethods.contains(element)) {
+				this.initMethods.addFirst(element);
+			}
 		}
 
-		public Set<LifecycleElement> getInitMethods() {
+		public Collection<LifecycleElement> getInitMethods() {
 			return this.initMethods;
 		}
 
@@ -263,10 +257,13 @@ public class InitDestroyAnnotationBeanPostProcessor
 		}
 
 		public void addDestroyMethod(Method method) {
-			this.destroyMethods.add(new LifecycleElement(method));
+			LifecycleElement element = new LifecycleElement(method);
+			if (!this.destroyMethods.contains(element)) {
+				this.destroyMethods.addLast(element);
+			}
 		}
 
-		public Set<LifecycleElement> getDestroyMethods() {
+		public Collection<LifecycleElement> getDestroyMethods() {
 			return this.destroyMethods;
 		}
 
@@ -291,17 +288,25 @@ public class InitDestroyAnnotationBeanPostProcessor
 
 		private final Method method;
 
+		private final String identifier;
+
 		public LifecycleElement(Method method) {
 			if (method.getParameterTypes().length != 0) {
 				throw new IllegalStateException("Lifecycle method annotation requires a no-arg method: " + method);
 			}
 			this.method = method;
+			this.identifier = (Modifier.isPrivate(method.getModifiers()) ?
+					method.getDeclaringClass() + "." + method.getName() : method.getName());
 		}
 
 		public Method getMethod() {
 			return this.method;
 		}
 
+		public String getIdentifier() {
+			return this.identifier;
+		}
+
 		public void invoke(Object target) throws Throwable {
 			ReflectionUtils.makeAccessible(this.method);
 			this.method.invoke(target, (Object[]) null);
@@ -316,13 +321,12 @@ public class InitDestroyAnnotationBeanPostProcessor
 				return false;
 			}
 			LifecycleElement otherElement = (LifecycleElement) other;
-			return (this.method.getName().equals(otherElement.method.getName()) &&
-					this.method.getDeclaringClass().equals(otherElement.method.getDeclaringClass()));
+			return (this.identifier.equals(otherElement.identifier));
 		}
 
 		@Override
 		public int hashCode() {
-			return this.method.getName().hashCode();
+			return this.identifier.hashCode();
 		}
 	}
 

org.springframework.context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java

@@ -17,15 +17,14 @@
 package org.springframework.context.annotation;
 
 import java.util.Properties;
-
-import static org.junit.Assert.*;
-
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.annotation.Resource;
 import javax.ejb.EJB;
 
+import static org.junit.Assert.*;
 import org.junit.Test;
+
 import org.springframework.beans.INestedTestBean;
 import org.springframework.beans.ITestBean;
 import org.springframework.beans.NestedTestBean;
@@ -33,8 +32,8 @@ import org.springframework.beans.TestBean;
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
-import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer;
 import org.springframework.beans.factory.annotation.InitDestroyAnnotationBeanPostProcessor;
+import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.jndi.support.SimpleJndiBeanFactory;
@@ -356,6 +355,9 @@ public class CommonAnnotationBeanPostProcessorTests {
 			if (this.testBean == null || this.testBean2 == null) {
 				throw new IllegalStateException("Resources not injected");
 			}
+			if (!this.initCalled) {
+				throw new IllegalStateException("Superclass init method not called yet");
+			}
 			if (this.init2Called) {
 				throw new IllegalStateException("Already called");
 			}
@@ -372,6 +374,9 @@ public class CommonAnnotationBeanPostProcessorTests {
 
 		@PreDestroy
 		protected void destroy2() {
+			if (this.destroyCalled) {
+				throw new IllegalStateException("Superclass destroy called too soon");
+			}
 			if (this.destroy2Called) {
 				throw new IllegalStateException("Already called");
 			}
@@ -380,6 +385,9 @@ public class CommonAnnotationBeanPostProcessorTests {
 
 		@PreDestroy
 		private void destroy() {
+			if (this.destroyCalled) {
+				throw new IllegalStateException("Superclass destroy called too soon");
+			}
 			if (this.destroy3Called) {
 				throw new IllegalStateException("Already called");
 			}