Customize destruction callback for AutoCloseable beans

Previously, a Bean implementing `AutoCloseable` (or `Closeable`) was
always destroyed regardless of its bean definition. In particular, the
documented way of disabling the destruction callback via an empty String
did not work.

AutoCloseable beans are now treated pretty much as any other bean: we
still use the presence of the interface to optimize the check of a
destroy method and we only auto-discover the method name to invoke if
the inferred mode is enabled.

Issue: SPR-13022

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -303,7 +303,7 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 				setInitMethodName(otherAbd.getInitMethodName());
 				setEnforceInitMethod(otherAbd.isEnforceInitMethod());
 			}
-			if (StringUtils.hasLength(otherAbd.getDestroyMethodName())) {
+			if (otherAbd.getDestroyMethodName() != null) {
 				setDestroyMethodName(otherAbd.getDestroyMethodName());
 				setEnforceDestroyMethod(otherAbd.isEnforceDestroyMethod());
 			}

spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -38,6 +38,7 @@ import org.springframework.beans.factory.config.DestructionAwareBeanPostProcesso
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ReflectionUtils;
+import org.springframework.util.StringUtils;
 
 /**
  * Adapter that implements the {@link DisposableBean} and {@link Runnable} interfaces
@@ -50,6 +51,7 @@ import org.springframework.util.ReflectionUtils;
  *
  * @author Juergen Hoeller
  * @author Costin Leau
+ * @author Stephane Nicoll
  * @since 2.0
  * @see AbstractBeanFactory
  * @see org.springframework.beans.factory.DisposableBean
@@ -186,8 +188,9 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 	 * interfaces, reflectively calling the "close" method on implementing beans as well.
 	 */
 	private String inferDestroyMethodIfNecessary(Object bean, RootBeanDefinition beanDefinition) {
-		if (AbstractBeanDefinition.INFER_METHOD.equals(beanDefinition.getDestroyMethodName()) ||
-				(beanDefinition.getDestroyMethodName() == null && closeableInterface.isInstance(bean))) {
+		String destroyMethodName = beanDefinition.getDestroyMethodName();
+		if (AbstractBeanDefinition.INFER_METHOD.equals(destroyMethodName) ||
+				(destroyMethodName == null && closeableInterface.isInstance(bean))) {
 			// Only perform destroy method inference or Closeable detection
 			// in case of the bean not explicitly implementing DisposableBean
 			if (!(bean instanceof DisposableBean)) {
@@ -205,7 +208,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 			}
 			return null;
 		}
-		return beanDefinition.getDestroyMethodName();
+		return (StringUtils.hasLength(destroyMethodName) ? destroyMethodName : null);
 	}
 
 	/**
@@ -399,7 +402,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
 		if (AbstractBeanDefinition.INFER_METHOD.equals(destroyMethodName)) {
 			return ClassUtils.hasMethod(bean.getClass(), CLOSE_METHOD_NAME);
 		}
-		return (destroyMethodName != null);
+		return StringUtils.hasLength(destroyMethodName);
 	}
 
 }

spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java

@@ -641,9 +641,7 @@ public class BeanDefinitionParserDelegate {
 
 		if (ele.hasAttribute(DESTROY_METHOD_ATTRIBUTE)) {
 			String destroyMethodName = ele.getAttribute(DESTROY_METHOD_ATTRIBUTE);
-			if (!"".equals(destroyMethodName)) {
-				bd.setDestroyMethodName(destroyMethodName);
-			}
+			bd.setDestroyMethodName(destroyMethodName);
 		}
 		else {
 			if (this.defaults.getDestroyMethod() != null) {

spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java

@@ -236,7 +236,7 @@ class ConfigurationClassBeanDefinitionReader {
 		}
 
 		String destroyMethodName = bean.getString("destroyMethod");
-		if (StringUtils.hasText(destroyMethodName)) {
+		if (destroyMethodName != null) {
 			beanDef.setDestroyMethodName(destroyMethodName);
 		}
 

spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java

@@ -165,8 +165,8 @@ class ScriptBeanDefinitionParser extends AbstractBeanDefinitionParser {
 			bd.setInitMethodName(beanDefinitionDefaults.getInitMethodName());
 		}
 
-		String destroyMethod = element.getAttribute(DESTROY_METHOD_ATTRIBUTE);
-		if (StringUtils.hasLength(destroyMethod)) {
+		if (element.hasAttribute(DESTROY_METHOD_ATTRIBUTE)) {
+			String destroyMethod = element.getAttribute(DESTROY_METHOD_ATTRIBUTE);
 			bd.setDestroyMethodName(destroyMethod);
 		}
 		else if (beanDefinitionDefaults.getDestroyMethodName() != null) {

spring-context/src/main/java/org/springframework/scripting/support/ScriptFactoryPostProcessor.java

@@ -505,7 +505,7 @@ public class ScriptFactoryPostProcessor extends InstantiationAwareBeanPostProces
 				Signature signature = new Signature(abd.getInitMethodName(), Type.VOID_TYPE, new Type[0]);
 				maker.add(signature, new Type[0]);
 			}
-			if (abd.getDestroyMethodName() != null) {
+			if (StringUtils.hasText(abd.getDestroyMethodName())) {
 				Signature signature = new Signature(abd.getDestroyMethodName(), Type.VOID_TYPE, new Type[0]);
 				maker.add(signature, new Type[0]);
 			}

spring-context/src/test/java/org/springframework/context/annotation/DestroyMethodInferenceTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -30,6 +30,7 @@ import static org.junit.Assert.*;
 /**
  * @author Chris Beams
  * @author Juergen Hoeller
+ * @author Stephane Nicoll
  */
 public class DestroyMethodInferenceTests {
 
@@ -45,6 +46,7 @@ public class DestroyMethodInferenceTests {
 		WithInheritedCloseMethod c5 = ctx.getBean("c5", WithInheritedCloseMethod.class);
 		WithNoCloseMethod c6 = ctx.getBean("c6", WithNoCloseMethod.class);
 		WithLocalShutdownMethod c7 = ctx.getBean("c7", WithLocalShutdownMethod.class);
+		WithInheritedCloseMethod c8 = ctx.getBean("c8", WithInheritedCloseMethod.class);
 
 		assertThat(c0.closed, is(false));
 		assertThat(c1.closed, is(false));
@@ -54,6 +56,7 @@ public class DestroyMethodInferenceTests {
 		assertThat(c5.closed, is(false));
 		assertThat(c6.closed, is(false));
 		assertThat(c7.closed, is(false));
+		assertThat(c8.closed, is(false));
 		ctx.close();
 		assertThat("c0", c0.closed, is(true));
 		assertThat("c1", c1.closed, is(true));
@@ -63,6 +66,7 @@ public class DestroyMethodInferenceTests {
 		assertThat("c5", c5.closed, is(true));
 		assertThat("c6", c6.closed, is(false));
 		assertThat("c7", c7.closed, is(true));
+		assertThat("c8", c8.closed, is(false));
 	}
 
 	@Test
@@ -73,6 +77,8 @@ public class DestroyMethodInferenceTests {
 		WithLocalCloseMethod x2 = ctx.getBean("x2", WithLocalCloseMethod.class);
 		WithLocalCloseMethod x3 = ctx.getBean("x3", WithLocalCloseMethod.class);
 		WithNoCloseMethod x4 = ctx.getBean("x4", WithNoCloseMethod.class);
+		WithInheritedCloseMethod x8 = ctx.getBean("x8", WithInheritedCloseMethod.class);
+
 		assertThat(x1.closed, is(false));
 		assertThat(x2.closed, is(false));
 		assertThat(x3.closed, is(false));
@@ -82,6 +88,7 @@ public class DestroyMethodInferenceTests {
 		assertThat(x2.closed, is(true));
 		assertThat(x3.closed, is(true));
 		assertThat(x4.closed, is(false));
+		assertThat(x8.closed, is(false));
 	}
 
 	@Configuration
@@ -134,6 +141,11 @@ public class DestroyMethodInferenceTests {
 		public WithLocalShutdownMethod c7() {
 			return new WithLocalShutdownMethod();
 		}
+
+		@Bean(destroyMethod = "")
+		public WithInheritedCloseMethod c8() {
+			return new WithInheritedCloseMethod();
+		}
 	}
 
 

spring-context/src/test/resources/org/springframework/context/annotation/DestroyMethodInferenceTests-context.xml

@@ -11,6 +11,10 @@
 	      class="org.springframework.context.annotation.DestroyMethodInferenceTests$WithLocalCloseMethod"
 	      destroy-method="(inferred)"/>
 
+	<bean id="x8"
+		  class="org.springframework.context.annotation.DestroyMethodInferenceTests.WithInheritedCloseMethod"
+		  destroy-method=""/>
+
 	<beans default-destroy-method="(inferred)">
 		<bean id="x3"
 		      class="org.springframework.context.annotation.DestroyMethodInferenceTests$WithLocalCloseMethod"/>