From a3161632dd71920efb769b3a80555e89949b7695 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Mon, 31 Dec 2012 14:16:38 +0100 Subject: [PATCH] Backport "Use ExtendedBeanInfo on an as-needed basis only" Prior to this change, CachedIntrospectionResults delegated to ExtendedBeanInfo by default in order to inspect JavaBean PropertyDescriptor information for bean classes. Originally introduced with SPR-8079, ExtendedBeanInfo was designed to go beyond the capabilities of the default JavaBeans Introspector in order to support non-void returning setter methods, principally to support use of builder-style APIs within Spring XML. This is a complex affair, and the non-trivial logic in ExtendedBeanInfo has led to various bugs including regressions for bean classes that do not declare non-void returning setters. Many of these issues were fixed when overhauling non-void JavaBean write method support in SPR-10029, however it appears that some extremely subtle issues may still remain. ExtendedBeanInfo was taken out of use completely in the 3.2.x line with SPR-9677 and the new BeanInfoFactory mechanism. This support was not backported to 3.1.x, however, in the interest of stability. This commit, then, inlines the conditional logic introduced by BeanInfoFactory directly into CachedIntrospectionResults. The net effect is that ExtendedBeanInfo is out of the code path entirely except for bean classes that actually contain 'candidate' methods, i.e. non-void and/or static write methods. This commit also includes the changes made in SPR-10115. Issue: SPR-9723, SPR-10029, SPR-9677, SPR-8079 --- .../beans/CachedIntrospectionResults.java | 12 ++++++++-- .../beans/ExtendedBeanInfo.java | 23 +++++++++++-------- .../beans/BeanWrapperTests.java | 18 +++++++++++++++ .../beans/ExtendedBeanInfoTests.java | 23 +++++++++++++++++++ 4 files changed, 64 insertions(+), 12 deletions(-) diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java b/org.springframework.beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java index dc03777392..850a034638 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java @@ -22,6 +22,7 @@ import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.ref.Reference; import java.lang.ref.WeakReference; +import java.lang.reflect.Method; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; @@ -32,7 +33,6 @@ import java.util.WeakHashMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -214,7 +214,15 @@ public class CachedIntrospectionResults { if (logger.isTraceEnabled()) { logger.trace("Getting BeanInfo for class [" + beanClass.getName() + "]"); } - this.beanInfo = new ExtendedBeanInfo(Introspector.getBeanInfo(beanClass)); + BeanInfo originalBeanInfo = Introspector.getBeanInfo(beanClass); + BeanInfo extendedBeanInfo = null; + for (Method method : beanClass.getMethods()) { + if (ExtendedBeanInfo.isCandidateWriteMethod(method)) { + extendedBeanInfo = new ExtendedBeanInfo(originalBeanInfo); + break; + } + } + this.beanInfo = extendedBeanInfo != null ? extendedBeanInfo : originalBeanInfo; // Immediately remove class from Introspector cache, to allow for proper // garbage collection on class loader shutdown - we cache it here anyway, diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java b/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java index 32d4b7e40b..432a5ec717 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -42,8 +42,8 @@ import static org.springframework.beans.PropertyDescriptorUtils.*; /** * Decorator for a standard {@link BeanInfo} object, e.g. as created by - * {@link Introspector#getBeanInfo(Class)}, designed to discover and register non-void - * returning setter methods. For example: + * {@link Introspector#getBeanInfo(Class)}, designed to discover and register static + * and/or non-void returning setter methods. For example: * 
{@code
  * public class Bean {
  *     private Foo foo;
@@ -102,17 +102,17 @@ class ExtendedBeanInfo implements BeanInfo {
 					new SimpleNonIndexedPropertyDescriptor(pd));
 		}
 
-		for (Method method : findNonVoidWriteMethods(delegate.getMethodDescriptors())) {
-			handleNonVoidWriteMethod(method);
+		for (Method method : findCandidateWriteMethods(delegate.getMethodDescriptors())) {
+			handleCandidateWriteMethod(method);
 		}
 	}
 
 
-	private List findNonVoidWriteMethods(MethodDescriptor[] methodDescriptors) {
+	private List findCandidateWriteMethods(MethodDescriptor[] methodDescriptors) {
 		List matches = new ArrayList();
 		for (MethodDescriptor methodDescriptor : methodDescriptors) {
 			Method method = methodDescriptor.getMethod();
-			if (isNonVoidWriteMethod(method)) {
+			if (isCandidateWriteMethod(method)) {
 				matches.add(method);
 			}
 		}
@@ -127,20 +127,23 @@ class ExtendedBeanInfo implements BeanInfo {
 		return matches;
 	}
 
-	public static boolean isNonVoidWriteMethod(Method method) {
+	public static boolean isCandidateWriteMethod(Method method) {
 		String methodName = method.getName();
 		Class[] parameterTypes = method.getParameterTypes();
 		int nParams = parameterTypes.length;
 		if (methodName.length() > 3 && methodName.startsWith("set") &&
 				Modifier.isPublic(method.getModifiers()) &&
-				!void.class.isAssignableFrom(method.getReturnType()) &&
+				(
+						!void.class.isAssignableFrom(method.getReturnType()) ||
+						Modifier.isStatic(method.getModifiers())
+				) &&
 				(nParams == 1 || (nParams == 2 && parameterTypes[0].equals(int.class)))) {
 			return true;
 		}
 		return false;
 	}
 
-	private void handleNonVoidWriteMethod(Method method) throws IntrospectionException {
+	private void handleCandidateWriteMethod(Method method) throws IntrospectionException {
 		int nParams = method.getParameterTypes().length;
 		String propertyName = propertyNameFor(method);
 		Class propertyType = method.getParameterTypes()[nParams-1];
diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperTests.java
index fc912c0be9..a2ae82c093 100644
--- a/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperTests.java
+++ b/org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperTests.java
@@ -1541,6 +1541,24 @@ public final class BeanWrapperTests {
 		assertEquals(TestEnum.TEST_VALUE, consumer.getEnumValue());
 	}
 
+	@Test
+	public void cornerSpr10115() {
+		Spr10115Bean foo = new Spr10115Bean();
+		BeanWrapperImpl bwi = new BeanWrapperImpl();
+		bwi.setWrappedInstance(foo);
+		bwi.setPropertyValue("prop1", "val1");
+		assertEquals("val1", Spr10115Bean.prop1);
+	}
+
+
+	static class Spr10115Bean {
+		private static String prop1;
+
+		public static void setProp1(String prop1) {
+			Spr10115Bean.prop1 = prop1;
+		}
+	}
+
 
 	private static class Foo {
 
diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java
index 4b6cc2ba90..12a392539f 100644
--- a/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java
+++ b/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java
@@ -959,4 +959,27 @@ public class ExtendedBeanInfoTests {
 		}
 	}
 
+	@Test
+	public void shouldSupportStaticWriteMethod() throws IntrospectionException {
+		{
+			BeanInfo bi = Introspector.getBeanInfo(WithStaticWriteMethod.class);
+			assertThat(hasReadMethodForProperty(bi, "prop1"), is(false));
+			assertThat(hasWriteMethodForProperty(bi, "prop1"), is(false));
+			assertThat(hasIndexedReadMethodForProperty(bi, "prop1"), is(false));
+			assertThat(hasIndexedWriteMethodForProperty(bi, "prop1"), is(false));
+		}
+		{
+			BeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(WithStaticWriteMethod.class));
+			assertThat(hasReadMethodForProperty(bi, "prop1"), is(false));
+			assertThat(hasWriteMethodForProperty(bi, "prop1"), is(true));
+			assertThat(hasIndexedReadMethodForProperty(bi, "prop1"), is(false));
+			assertThat(hasIndexedWriteMethodForProperty(bi, "prop1"), is(false));
+		}
+	}
+
+	static class WithStaticWriteMethod {
+		@SuppressWarnings("unused")
+		public static void setProp1(String prop1) {
+		}
+	}
 }