Skip to content

Commit bcf6f23

Browse files
jhoellerjhoeller
committed
declarative destroy-method="..." specifications get validated at bean creation time (SPR-5602)
1 parent d14cc0d commit bcf6f23

File tree

4 files changed

+98
-81
lines changed

4 files changed

+98
-81
lines changed

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,12 @@ else if (!this.allowRawInjectionDespiteWrapping && hasDependentBean(beanName)) {
512512
}
513513

514514
// Register bean as disposable.
515-
registerDisposableBeanIfNecessary(beanName, bean, mbd);
515+
try {
516+
registerDisposableBeanIfNecessary(beanName, bean, mbd);
517+
}
518+
catch (BeanDefinitionValidationException ex) {
519+
throw new BeanCreationException(mbd.getResourceDescription(), beanName, "Invalid destruction signature", ex);
520+
}
516521

517522
return exposedObject;
518523
}
@@ -1387,8 +1392,8 @@ protected void invokeCustomInitMethod(
13871392
Method initMethod = BeanUtils.findMethod(bean.getClass(), initMethodName, null);
13881393
if (initMethod == null) {
13891394
if (enforceInitMethod) {
1390-
throw new NoSuchMethodException("Couldn't find an init method named '" + initMethodName +
1391-
"' on bean with name '" + beanName + "'");
1395+
throw new BeanDefinitionValidationException("Couldn't find an init method named '" +
1396+
initMethodName + "' on bean with name '" + beanName + "'");
13921397
}
13931398
else {
13941399
if (logger.isDebugEnabled()) {

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java

+68-73
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2008 the original author or authors.
2+
* Copyright 2002-2009 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -58,9 +58,9 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
5858

5959
private final boolean invokeDisposableBean;
6060

61-
private final String destroyMethodName;
61+
private String destroyMethodName;
6262

63-
private final boolean enforceDestroyMethod;
63+
private transient Method destroyMethod;
6464

6565
private List<DestructionAwareBeanPostProcessor> beanPostProcessors;
6666

@@ -79,35 +79,55 @@ public DisposableBeanAdapter(Object bean, String beanName, RootBeanDefinition be
7979
Assert.notNull(bean, "Bean must not be null");
8080
this.bean = bean;
8181
this.beanName = beanName;
82-
this.invokeDisposableBean = !beanDefinition.isExternallyManagedDestroyMethod("destroy");
83-
this.destroyMethodName =
84-
(!beanDefinition.isExternallyManagedDestroyMethod(beanDefinition.getDestroyMethodName()) ?
85-
beanDefinition.getDestroyMethodName() : null);
86-
this.enforceDestroyMethod = beanDefinition.isEnforceDestroyMethod();
82+
this.invokeDisposableBean =
83+
(this.bean instanceof DisposableBean && !beanDefinition.isExternallyManagedDestroyMethod("destroy"));
84+
String destroyMethodName = beanDefinition.getDestroyMethodName();
85+
if (destroyMethodName != null && !(this.invokeDisposableBean && "destroy".equals(destroyMethodName)) &&
86+
!beanDefinition.isExternallyManagedDestroyMethod(destroyMethodName)) {
87+
this.destroyMethodName = destroyMethodName;
88+
try {
89+
this.destroyMethod = BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName);
90+
}
91+
catch (IllegalArgumentException ex) {
92+
throw new BeanDefinitionValidationException("Couldn't find a unique destroy method on bean with name '" +
93+
this.beanName + ": " + ex.getMessage());
94+
}
95+
if (this.destroyMethod == null) {
96+
if (beanDefinition.isEnforceDestroyMethod()) {
97+
throw new BeanDefinitionValidationException("Couldn't find a destroy method named '" +
98+
destroyMethodName + "' on bean with name '" + beanName + "'");
99+
}
100+
}
101+
else {
102+
Class[] paramTypes = this.destroyMethod.getParameterTypes();
103+
if (paramTypes.length > 1) {
104+
throw new BeanDefinitionValidationException("Method '" + destroyMethodName + "' of bean '" +
105+
beanName + "' has more than one parameter - not supported as destroy method");
106+
}
107+
else if (paramTypes.length == 1 && !paramTypes[0].equals(boolean.class)) {
108+
throw new BeanDefinitionValidationException("Method '" + destroyMethodName + "' of bean '" +
109+
beanName + "' has a non-boolean parameter - not supported as destroy method");
110+
}
111+
}
112+
}
87113
this.beanPostProcessors = filterPostProcessors(postProcessors);
88114
}
89115

90116
/**
91117
* Create a new DisposableBeanAdapter for the given bean.
92118
* @param bean the bean instance (never <code>null</code>)
93119
* @param beanName the name of the bean
94-
* @param invokeDisposableBean whether to actually invoke
95-
* DisposableBean's destroy method here
96-
* @param destroyMethodName the name of the custom destroy method
97-
* (<code>null</code> if there is none)
98-
* @param enforceDestroyMethod whether to the specified custom
99-
* destroy method (if any) has to be present on the bean object
120+
* @param invokeDisposableBean whether to actually invoke DisposableBean's destroy method here
121+
* @param destroyMethodName the name of the custom destroy method (<code>null</code> if there is none)
100122
* @param postProcessors the List of DestructionAwareBeanPostProcessors, if any
101123
*/
102124
private DisposableBeanAdapter(Object bean, String beanName, boolean invokeDisposableBean,
103-
String destroyMethodName, boolean enforceDestroyMethod,
104-
List<DestructionAwareBeanPostProcessor> postProcessors) {
125+
String destroyMethodName, List<DestructionAwareBeanPostProcessor> postProcessors) {
105126

106127
this.bean = bean;
107128
this.beanName = beanName;
108129
this.invokeDisposableBean = invokeDisposableBean;
109130
this.destroyMethodName = destroyMethodName;
110-
this.enforceDestroyMethod = enforceDestroyMethod;
111131
this.beanPostProcessors = postProcessors;
112132
}
113133

@@ -142,8 +162,7 @@ public void destroy() {
142162
}
143163
}
144164

145-
boolean isDisposableBean = (this.bean instanceof DisposableBean);
146-
if (isDisposableBean && this.invokeDisposableBean) {
165+
if (this.invokeDisposableBean) {
147166
if (logger.isDebugEnabled()) {
148167
logger.debug("Invoking destroy() on bean with name '" + this.beanName + "'");
149168
}
@@ -161,8 +180,13 @@ public void destroy() {
161180
}
162181
}
163182

164-
if (this.destroyMethodName != null && !(isDisposableBean && "destroy".equals(this.destroyMethodName))) {
165-
invokeCustomDestroyMethod();
183+
if (this.destroyMethod != null) {
184+
invokeCustomDestroyMethod(this.destroyMethod);
185+
}
186+
else if (this.destroyMethodName != null) {
187+
Method destroyMethod =
188+
BeanUtils.findMethodWithMinimalParameters(this.bean.getClass(), this.destroyMethodName);
189+
invokeCustomDestroyMethod(destroyMethod);
166190
}
167191
}
168192

@@ -172,62 +196,33 @@ public void destroy() {
172196
* for a method with a single boolean argument (passing in "true",
173197
* assuming a "force" parameter), else logging an error.
174198
*/
175-
private void invokeCustomDestroyMethod() {
199+
private void invokeCustomDestroyMethod(Method destroyMethod) {
200+
Class[] paramTypes = destroyMethod.getParameterTypes();
201+
Object[] args = new Object[paramTypes.length];
202+
if (paramTypes.length == 1) {
203+
args[0] = Boolean.TRUE;
204+
}
205+
if (logger.isDebugEnabled()) {
206+
logger.debug("Invoking destroy method '" + this.destroyMethodName +
207+
"' on bean with name '" + this.beanName + "'");
208+
}
209+
ReflectionUtils.makeAccessible(destroyMethod);
176210
try {
177-
Method destroyMethod =
178-
BeanUtils.findMethodWithMinimalParameters(this.bean.getClass(), this.destroyMethodName);
179-
if (destroyMethod == null) {
180-
if (this.enforceDestroyMethod) {
181-
logger.warn("Couldn't find a destroy method named '" + this.destroyMethodName +
182-
"' on bean with name '" + this.beanName + "'");
183-
}
211+
destroyMethod.invoke(this.bean, args);
212+
}
213+
catch (InvocationTargetException ex) {
214+
String msg = "Invocation of destroy method '" + this.destroyMethodName +
215+
"' failed on bean with name '" + this.beanName + "'";
216+
if (logger.isDebugEnabled()) {
217+
logger.warn(msg, ex.getTargetException());
184218
}
185-
186219
else {
187-
Class[] paramTypes = destroyMethod.getParameterTypes();
188-
if (paramTypes.length > 1) {
189-
logger.error("Method '" + this.destroyMethodName + "' of bean '" + this.beanName +
190-
"' has more than one parameter - not supported as destroy method");
191-
}
192-
else if (paramTypes.length == 1 && !paramTypes[0].equals(boolean.class)) {
193-
logger.error("Method '" + this.destroyMethodName + "' of bean '" + this.beanName +
194-
"' has a non-boolean parameter - not supported as destroy method");
195-
}
196-
197-
else {
198-
Object[] args = new Object[paramTypes.length];
199-
if (paramTypes.length == 1) {
200-
args[0] = Boolean.TRUE;
201-
}
202-
if (logger.isDebugEnabled()) {
203-
logger.debug("Invoking destroy method '" + this.destroyMethodName +
204-
"' on bean with name '" + this.beanName + "'");
205-
}
206-
ReflectionUtils.makeAccessible(destroyMethod);
207-
try {
208-
destroyMethod.invoke(this.bean, args);
209-
}
210-
catch (InvocationTargetException ex) {
211-
String msg = "Invocation of destroy method '" + this.destroyMethodName +
212-
"' failed on bean with name '" + this.beanName + "'";
213-
if (logger.isDebugEnabled()) {
214-
logger.warn(msg, ex.getTargetException());
215-
}
216-
else {
217-
logger.warn(msg + ": " + ex.getTargetException());
218-
}
219-
}
220-
catch (Throwable ex) {
221-
logger.error("Couldn't invoke destroy method '" + this.destroyMethodName +
222-
"' on bean with name '" + this.beanName + "'", ex);
223-
}
224-
}
220+
logger.warn(msg + ": " + ex.getTargetException());
225221
}
226222
}
227-
catch (IllegalArgumentException ex) {
228-
// thrown from findMethodWithMinimalParameters
229-
logger.error("Couldn't find a unique destroy method on bean with name '" +
230-
this.beanName + ": " + ex.getMessage());
223+
catch (Throwable ex) {
224+
logger.error("Couldn't invoke destroy method '" + this.destroyMethodName +
225+
"' on bean with name '" + this.beanName + "'", ex);
231226
}
232227
}
233228

@@ -247,7 +242,7 @@ protected Object writeReplace() {
247242
}
248243
}
249244
return new DisposableBeanAdapter(this.bean, this.beanName, this.invokeDisposableBean,
250-
this.destroyMethodName, this.enforceDestroyMethod, serializablePostProcessors);
245+
this.destroyMethodName, serializablePostProcessors);
251246
}
252247

253248
}

‎org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/FactoryMethodTests.java

+19-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2008 the original author or authors.
2+
* Copyright 2002-2009 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,21 +16,20 @@
1616

1717
package org.springframework.beans.factory.xml;
1818

19-
import static org.junit.Assert.*;
20-
2119
import java.util.Arrays;
2220
import java.util.Collections;
2321
import java.util.List;
2422
import java.util.Properties;
2523

24+
import static org.junit.Assert.*;
2625
import org.junit.Test;
26+
import test.beans.TestBean;
27+
2728
import org.springframework.beans.factory.BeanCreationException;
2829
import org.springframework.beans.factory.BeanDefinitionStoreException;
2930
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
3031
import org.springframework.core.io.ClassPathResource;
3132

32-
import test.beans.TestBean;
33-
3433
/**
3534
* @author Juergen Hoeller
3635
* @author Chris Beams
@@ -71,6 +70,21 @@ public void testFactoryMethodsSingletonOnTargetClass() {
7170
assertTrue(tb.wasDestroyed());
7271
}
7372

73+
@Test
74+
public void testFactoryMethodsWithInvalidDestroyMethod() {
75+
DefaultListableBeanFactory xbf = new DefaultListableBeanFactory();
76+
XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(xbf);
77+
reader.loadBeanDefinitions(new ClassPathResource("factory-methods.xml", getClass()));
78+
79+
try {
80+
xbf.getBean("defaultTestBeanWithInvalidDestroyMethod");
81+
fail("Should have thrown BeanCreationException");
82+
}
83+
catch (BeanCreationException ex) {
84+
// expected
85+
}
86+
}
87+
7488
@Test
7589
public void testFactoryMethodsWithNullInstance() {
7690
DefaultListableBeanFactory xbf = new DefaultListableBeanFactory();

‎org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/factory-methods.xml

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
<bean id="defaultTestBean" factory-bean="default" factory-method="getTestBean"
1313
init-method="haveBirthday" destroy-method="destroy"/>
1414

15+
<bean id="defaultTestBeanWithInvalidDestroyMethod" factory-bean="default" factory-method="getTestBean"
16+
init-method="haveBirthday" destroy-method="xxx" lazy-init="true"/>
17+
1518
<bean id="defaultTestBean.protected" factory-bean="default" factory-method="protectedGetTestBean"
1619
init-method="haveBirthday" destroy-method="destroy"/>
1720

0 commit comments

Comments
 (0)