diff --git a/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java b/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java index 87400ff5419..4f508511f51 100644 --- a/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java +++ b/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java @@ -173,14 +173,14 @@ public class ReflectionTestUtils { Field field = ReflectionUtils.findField(targetClass, name, type); if (field == null) { throw new IllegalArgumentException(String.format( - "Could not find field '%s' of type [%s] on target object [%s] or target class [%s]", name, type, - ultimateTarget, targetClass)); + "Could not find field '%s' of type [%s] on %s or target class [%s]", name, type, + safeToString(ultimateTarget), targetClass)); } if (logger.isDebugEnabled()) { logger.debug(String.format( - "Setting field '%s' of type [%s] on target object [%s] or target class [%s] to value [%s]", name, type, - ultimateTarget, targetClass, value)); + "Setting field '%s' of type [%s] on %s or target class [%s] to value [%s]", name, type, + safeToString(ultimateTarget), targetClass, value)); } ReflectionUtils.makeAccessible(field); ReflectionUtils.setField(field, ultimateTarget, value); @@ -253,14 +253,13 @@ public class ReflectionTestUtils { Field field = ReflectionUtils.findField(targetClass, name); if (field == null) { - throw new IllegalArgumentException( - String.format("Could not find field '%s' on target object [%s] or target class [%s]", name, - ultimateTarget, targetClass)); + throw new IllegalArgumentException(String.format("Could not find field '%s' on %s or target class [%s]", + name, safeToString(ultimateTarget), targetClass)); } if (logger.isDebugEnabled()) { - logger.debug(String.format("Getting field '%s' from target object [%s] or target class [%s]", name, - ultimateTarget, targetClass)); + logger.debug(String.format("Getting field '%s' from %s or target class [%s]", name, + safeToString(ultimateTarget), targetClass)); } ReflectionUtils.makeAccessible(field); return ReflectionUtils.getField(field, ultimateTarget); @@ -327,13 +326,16 @@ public class ReflectionTestUtils { method = ReflectionUtils.findMethod(target.getClass(), setterMethodName, paramTypes); } if (method == null) { - throw new IllegalArgumentException("Could not find setter method '" + setterMethodName + - "' on target [" + target + "] with parameter type [" + type + "]"); + throw new IllegalArgumentException(String.format( + "Could not find setter method '%s' on %s with parameter type [%s]", setterMethodName, + safeToString(target), type)); } if (logger.isDebugEnabled()) { - logger.debug("Invoking setter method '" + setterMethodName + "' on target [" + target + "]"); + logger.debug(String.format("Invoking setter method '%s' on %s with value [%s]", setterMethodName, + safeToString(target), value)); } + ReflectionUtils.makeAccessible(method); ReflectionUtils.invokeMethod(method, target, value); } @@ -372,12 +374,12 @@ public class ReflectionTestUtils { method = ReflectionUtils.findMethod(target.getClass(), getterMethodName); } if (method == null) { - throw new IllegalArgumentException("Could not find getter method '" + getterMethodName + - "' on target [" + target + "]"); + throw new IllegalArgumentException(String.format( + "Could not find getter method '%s' on %s", getterMethodName, safeToString(target))); } if (logger.isDebugEnabled()) { - logger.debug("Invoking getter method '" + getterMethodName + "' on target [" + target + "]"); + logger.debug(String.format("Invoking getter method '%s' on %s", getterMethodName, safeToString(target))); } ReflectionUtils.makeAccessible(method); return ReflectionUtils.invokeMethod(method, target); @@ -412,8 +414,8 @@ public class ReflectionTestUtils { methodInvoker.prepare(); if (logger.isDebugEnabled()) { - logger.debug("Invoking method '" + name + "' on target [" + target + "] with arguments [" + - ObjectUtils.nullSafeToString(args) + "]"); + logger.debug(String.format("Invoking method '%s' on %s with arguments %s", name, safeToString(target), + ObjectUtils.nullSafeToString(args))); } return (T) methodInvoker.invoke(); @@ -424,4 +426,14 @@ public class ReflectionTestUtils { } } + private static String safeToString(Object target) { + try { + return String.format("target object [%s]", target); + } + catch (Exception ex) { + return String.format("target of type [%s] whose toString() method threw [%s]", + (target != null ? target.getClass().getName() : "unknown"), ex); + } + } + } diff --git a/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java b/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java index 0216e362f75..3ea39f9b8b4 100644 --- a/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java +++ b/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java @@ -48,6 +48,8 @@ public class ReflectionTestUtilsTests { private final Component component = new Component(); + private final LegacyEntity entity = new LegacyEntity(); + @Rule public final ExpectedException exception = ExpectedException.none(); @@ -202,17 +204,6 @@ public class ReflectionTestUtilsTests { setField(person, "likesPets", null, boolean.class); } - /** - * Verifies behavior requested in SPR-9571. - */ - @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 setStaticFieldViaClass() throws Exception { setField(StaticFields.class, "publicField", "xxx"); @@ -398,4 +389,37 @@ public class ReflectionTestUtilsTests { invokeMethod(component, "configure", new Integer(42), "enigma", "baz", "quux"); } + @Test // SPR-14363 + public void getFieldOnLegacyEntityWithSideEffectsInToString() { + Object collaborator = getField(entity, "collaborator"); + assertNotNull(collaborator); + } + + @Test // SPR-9571 and SPR-14363 + public void setFieldOnLegacyEntityWithSideEffectsInToString() { + String testCollaborator = "test collaborator"; + setField(entity, "collaborator", testCollaborator, Object.class); + assertTrue(entity.toString().contains(testCollaborator)); + } + + @Test // SPR-14363 + public void invokeMethodOnLegacyEntityWithSideEffectsInToString() { + invokeMethod(entity, "configure", new Integer(42), "enigma"); + assertEquals("number should have been configured", new Integer(42), entity.getNumber()); + assertEquals("text should have been configured", "enigma", entity.getText()); + } + + @Test // SPR-14363 + public void invokeGetterMethodOnLegacyEntityWithSideEffectsInToString() { + Object collaborator = invokeGetterMethod(entity, "collaborator"); + assertNotNull(collaborator); + } + + @Test // SPR-14363 + public void invokeSetterMethodOnLegacyEntityWithSideEffectsInToString() { + String testCollaborator = "test collaborator"; + invokeSetterMethod(entity, "collaborator", testCollaborator); + assertTrue(entity.toString().contains(testCollaborator)); + } + } diff --git a/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntity.java b/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntity.java index 8c8ddfd4af1..72d4633f098 100644 --- a/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntity.java +++ b/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntity.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2016 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. @@ -31,17 +31,41 @@ public class LegacyEntity { @Override public String toString() { - throw new RuntimeException( + throw new LegacyEntityException( "Invoking toString() on the default collaborator causes an undesirable side effect"); - }; + } }; + private Integer number; + private String text; + + + public void configure(Integer number, String text) { + this.number = number; + this.text = text; + } + + public Integer getNumber() { + return this.number; + } + + public String getText() { + return this.text; + } + + public Object getCollaborator() { + return this.collaborator; + } + + public void setCollaborator(Object collaborator) { + this.collaborator = collaborator; + } @Override public String toString() { return new ToStringCreator(this)// - .append("collaborator", this.collaborator)// - .toString(); + .append("collaborator", this.collaborator)// + .toString(); } } diff --git a/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntityException.java b/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntityException.java new file mode 100644 index 00000000000..2318e90a5f5 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/util/subpackage/LegacyEntityException.java @@ -0,0 +1,32 @@ +/* + * Copyright 2002-2016 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; + +/** + * Exception thrown by a {@link LegacyEntity}. + * + * @author Sam Brannen + * @since 4.3.1 + */ +@SuppressWarnings("serial") +public class LegacyEntityException extends RuntimeException { + + public LegacyEntityException(String message) { + super(message); + } + +} diff --git a/spring-test/src/test/resources/log4j.properties b/spring-test/src/test/resources/log4j.properties index 8b88c2e0798..258e7b8425c 100644 --- a/spring-test/src/test/resources/log4j.properties +++ b/spring-test/src/test/resources/log4j.properties @@ -25,6 +25,9 @@ log4j.logger.org.springframework.test.context.web=WARN #log4j.logger.org.springframework.test.context.support.AbstractGenericContextLoader=INFO #log4j.logger.org.springframework.test.context.support.AnnotationConfigContextLoader=INFO +# The following must be kept at DEBUG in order to test SPR-14363. +log4j.logger.org.springframework.test.util=DEBUG + log4j.logger.org.springframework.test.web.servlet.result=DEBUG #log4j.logger.org.springframework.test=TRACE