From 0ed9ca097bb499b46c67d34d59e08a1337758097 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 15 May 2015 11:22:29 +0200 Subject: [PATCH] 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 --- .../factory/support/AbstractBeanDefinition.java | 4 ++-- .../factory/support/DisposableBeanAdapter.java | 13 ++++++++----- .../factory/xml/BeanDefinitionParserDelegate.java | 4 +--- .../ConfigurationClassBeanDefinitionReader.java | 2 +- .../config/ScriptBeanDefinitionParser.java | 4 ++-- .../support/ScriptFactoryPostProcessor.java | 2 +- .../annotation/DestroyMethodInferenceTests.java | 14 +++++++++++++- .../DestroyMethodInferenceTests-context.xml | 4 ++++ 8 files changed, 32 insertions(+), 15 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java index 1a0258679f6..34b8d72d12a 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java +++ b/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()); } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java index d6533455144..dedc75cf49a 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java +++ b/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); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java b/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java index 9761263d5d5..290ce5f0c4b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java +++ b/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) { diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index 570df5ac555..033d27e58eb 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/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); } diff --git a/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java b/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java index 67c9a3fe0b9..533fd21c193 100644 --- a/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java +++ b/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) { diff --git a/spring-context/src/main/java/org/springframework/scripting/support/ScriptFactoryPostProcessor.java b/spring-context/src/main/java/org/springframework/scripting/support/ScriptFactoryPostProcessor.java index 156e322e025..d91573d03aa 100644 --- a/spring-context/src/main/java/org/springframework/scripting/support/ScriptFactoryPostProcessor.java +++ b/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]); } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/DestroyMethodInferenceTests.java b/spring-context/src/test/java/org/springframework/context/annotation/DestroyMethodInferenceTests.java index 87999283a31..a9b1bf88297 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/DestroyMethodInferenceTests.java +++ b/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(); + } } diff --git a/spring-context/src/test/resources/org/springframework/context/annotation/DestroyMethodInferenceTests-context.xml b/spring-context/src/test/resources/org/springframework/context/annotation/DestroyMethodInferenceTests-context.xml index 08b84f7c513..ce9902c5dd5 100644 --- a/spring-context/src/test/resources/org/springframework/context/annotation/DestroyMethodInferenceTests-context.xml +++ b/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)"/> + +