RTU.setField() shouldn't call toString() on target

ReflectionTestUtils.setField() implicitly calls toString() on the target
object when arguments for a call to Assert.notNull() are built. This can
have undesirable side effects, for example if the toString() invocation
results in a thrown exception or access to an external system (e.g., a
database).

This commit addresses this issue by inlining the Assert.notNull() code,
thereby avoiding accidental invocation of toString() on a non-null
target.

Issue: SPR-9571

spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -104,10 +104,18 @@ public class ReflectionTestUtils {
 	public static void setField(Object target, String name, Object value, Class<?> type) {
 		Assert.notNull(target, "Target object must not be null");
 		Field field = ReflectionUtils.findField(target.getClass(), name, type);
-		Assert.notNull(field, "Could not find field [" + name + "] on target [" + target + "]");
+
+		// SPR-9571: inline Assert.notNull() in order to avoid accidentally invoking
+		// toString() on a non-null target.
+		if (field == null) {
+			throw new IllegalArgumentException(String.format("Could not find field [%s] of type [%s] on target [%s]",
+				name, type, target));
+		}
 
 		if (logger.isDebugEnabled()) {
-			logger.debug("Setting field [" + name + "] on target [" + target + "]");
+			logger.debug(String.format("Setting field [%s] of type [%s] on target [%s] to value [%s]", name, type,
+				target,
+				value));
 		}
 		ReflectionUtils.makeAccessible(field);
 		ReflectionUtils.setField(field, target, value);

spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -16,18 +16,15 @@
 
 package org.springframework.test.util;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.springframework.test.util.ReflectionTestUtils.getField;
-import static org.springframework.test.util.ReflectionTestUtils.invokeGetterMethod;
-import static org.springframework.test.util.ReflectionTestUtils.invokeMethod;
-import static org.springframework.test.util.ReflectionTestUtils.invokeSetterMethod;
-import static org.springframework.test.util.ReflectionTestUtils.setField;
+import static org.junit.Assert.*;
+import static org.springframework.test.util.ReflectionTestUtils.*;
 
 import org.junit.Ignore;
 import org.junit.Test;
+
 import org.springframework.test.AssertThrows;
 import org.springframework.test.util.subpackage.Component;
+import org.springframework.test.util.subpackage.LegacyEntity;
 import org.springframework.test.util.subpackage.Person;
 
 /**
@@ -111,6 +108,17 @@ public class ReflectionTestUtilsTests {
 		}.runTest();
 	}
 
+	/**
+	 * Verifies behavior requested in <a href="https://jira.springsource.org/browse/SPR-9571">SPR-9571</a>.
+	 */
+	@Test
+	public void setFieldOnLegacyEntityWithSideEffectsInToString() {
+		String testCollaborator = "test collaborator";
+		LegacyEntity entity = new LegacyEntity();
+		setField(entity, "collaborator", testCollaborator, Object.class);
+		assertTrue(entity.toString().contains(testCollaborator));
+	}
+
 	@Test
 	public void invokeSetterAndMethods() throws Exception {
 

spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntity.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2002-2012 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.test.util.subpackage;
+
+import org.springframework.core.style.ToStringCreator;
+
+/**
+ * A <em>legacy entity</em> whose {@link #toString()} method has side effects;
+ * intended for use in unit tests.
+ * 
+ * @author Sam Brannen
+ * @since 3.2
+ */
+public class LegacyEntity {
+
+	private Object collaborator = new Object() {
+
+		public String toString() {
+			throw new RuntimeException(
+				"Invoking toString() on the default collaborator causes an undesirable side effect");
+		};
+	};
+
+
+	public String toString() {
+		return new ToStringCreator(this)//
+		.append("collaborator", this.collaborator)//
+		.toString();
+	}
+
+}