Polishing and improved test coverage of introspection package

Author: SQiShER

src/main/java/de/danielbechler/diff/introspection/DefaultPropertyAccessExceptionHandler.java

@@ -22,13 +22,11 @@
 
 public class DefaultPropertyAccessExceptionHandler implements PropertyAccessExceptionHandler
 {
-	private static final Logger logger = LoggerFactory.getLogger(DefaultPropertyAccessExceptionHandler.class);
+	Logger logger = LoggerFactory.getLogger(DefaultPropertyAccessExceptionHandler.class);
 
 	public DiffNode onPropertyWriteException(final PropertyWriteException exception, final DiffNode node)
 	{
-		final Object newValue = exception.getNewValue();
-		final String propertyName = exception.getPropertyName();
-		logger.info("Couldn't set new value '{}' for property '{}'", newValue, propertyName);
+		logger.info(exception.getMessage());
 		throw exception;
 	}
 

src/main/java/de/danielbechler/diff/introspection/PropertyAccessException.java

@@ -23,37 +23,23 @@ public abstract class PropertyAccessException extends RuntimeException
 {
 	private static final long serialVersionUID = 1L;
 
-	private String propertyName;
-	private Class<?> targetType;
+	private final String propertyName;
+	private final Class<?> targetType;
 
-	public PropertyAccessException(final Throwable cause)
+	protected PropertyAccessException(final String propertyName, final Class<?> targetType, final Throwable cause)
 	{
 		super(cause);
-	}
-
-	@Override
-	public String getMessage()
-	{
-		return String.format("Property '%s' on target of type %s.", propertyName, targetType);
+		this.propertyName = propertyName;
+		this.targetType = targetType;
 	}
 
 	public String getPropertyName()
 	{
 		return propertyName;
 	}
 
-	public void setPropertyName(final String propertyName)
-	{
-		this.propertyName = propertyName;
-	}
-
 	public Class<?> getTargetType()
 	{
 		return targetType;
 	}
-
-	public void setTargetType(final Class<?> targetType)
-	{
-		this.targetType = targetType;
-	}
 }

src/main/java/de/danielbechler/diff/introspection/PropertyAccessor.java

@@ -124,12 +124,9 @@ public Object get(final Object target)
 		{
 			return readMethod.invoke(target);
 		}
-		catch (final Exception e)
+		catch (final Exception cause)
 		{
-			final PropertyReadException ex = new PropertyReadException(e);
-			ex.setPropertyName(propertyName);
-			ex.setTargetType(target.getClass());
-			throw ex;
+			throw new PropertyReadException(propertyName, target.getClass(), cause);
 		}
 	}
 
@@ -161,22 +158,23 @@ private void logFailedSet(final Object value)
 		logger.info("Couldn't set new value '{}' for property '{}'", value, propertyName);
 	}
 
+	@SuppressWarnings("unchecked")
 	private void tryToReplaceContentOfCollectionTypes(final Object target, final Object value)
 	{
 		if (Collection.class.isAssignableFrom(readMethod.getReturnType()))
 		{
-			//noinspection unchecked
-			tryToReplaceCollectionContent((Collection<Object>) get(target), (Collection<Object>) value);
-			return;
+			if (tryToReplaceCollectionContent((Collection<Object>) get(target), (Collection<Object>) value))
+			{
+				return;
+			}
 		}
-
 		if (Map.class.isAssignableFrom(readMethod.getReturnType()))
 		{
-			//noinspection unchecked
-			tryToReplaceMapContent((Map<Object, Object>) get(target), (Map<Object, Object>) value);
-			return;
+			if (tryToReplaceMapContent((Map<Object, Object>) get(target), (Map<Object, Object>) value))
+			{
+				return;
+			}
 		}
-
 		logFailedSet(value);
 	}
 
@@ -186,12 +184,9 @@ private void invokeWriteMethod(final Object target, final Object value)
 		{
 			writeMethod.invoke(target, value);
 		}
-		catch (final Exception e)
+		catch (final Exception cause)
 		{
-			final PropertyWriteException ex = new PropertyWriteException(e, value);
-			ex.setPropertyName(propertyName);
-			ex.setTargetType(getType());
-			throw ex;
+			throw new PropertyWriteException(propertyName, getType(), value, cause);
 		}
 	}
 
@@ -243,6 +238,12 @@ public Class<?> getType()
 	@Override
 	public String toString()
 	{
-		return "property '" + propertyName + "'";
+		final StringBuilder sb = new StringBuilder("PropertyAccessor{");
+		sb.append("propertyName='").append(propertyName).append('\'');
+		sb.append(", type=").append(type.getCanonicalName());
+		sb.append(", source=").append(readMethod.getDeclaringClass().getCanonicalName());
+		sb.append(", hasWriteMethod=").append(writeMethod != null);
+		sb.append('}');
+		return sb.toString();
 	}
 }

src/main/java/de/danielbechler/diff/introspection/PropertyReadException.java

@@ -23,14 +23,14 @@ public class PropertyReadException extends PropertyAccessException
 {
 	private static final long serialVersionUID = 1L;
 
-	public PropertyReadException(final Throwable cause)
+	public PropertyReadException(final String propertyName, final Class<?> targetType, final Throwable cause)
 	{
-		super(cause);
+		super(propertyName, targetType, cause);
 	}
 
 	@Override
 	public String getMessage()
 	{
-		return "Failed to invoke read method. " + super.getMessage();
+		return String.format("Failed to read value from property '%s' of type '%s'", getPropertyName(), getTargetType().getCanonicalName());
 	}
 }

src/main/java/de/danielbechler/diff/introspection/PropertyWriteException.java

@@ -22,18 +22,20 @@
 public class PropertyWriteException extends PropertyAccessException
 {
 	private static final long serialVersionUID = 1L;
-	private Object newValue;
 
-	public PropertyWriteException(final Throwable cause, final Object newValue)
+	@SuppressWarnings("TransientFieldNotInitialized")
+	private final transient Object newValue;
+
+	public PropertyWriteException(final String propertyName, final Class<?> targetType, final Object newValue, final Throwable cause)
 	{
-		super(cause);
+		super(propertyName, targetType, cause);
 		this.newValue = newValue;
 	}
 
 	@Override
 	public String getMessage()
 	{
-		return "Error while invoking write method. " + super.getMessage();
+		return String.format("Failed to write new value '%s' to property '%s' of type '%s'", newValue, getPropertyName(), getTargetType().getCanonicalName());
 	}
 
 	public Object getNewValue()

src/test/java/de/danielbechler/diff/access/InstancesSpec.groovy

@@ -76,13 +76,13 @@ class InstancesSpec extends Specification {
 
 	def "getType() returns most specific shared type for non-primitive objects (except for Map and Collection)"() {
 		given:
-		  def instances = Instances.of(beanPropertyAccessor, working, base);
+		  def instances = Instances.of(propertyAccessor, working, base);
 
 		expect:
 		  instances.getType() == resultType
 
 		where:
-		  beanPropertyAccessor  | working                                    | base                                     | resultType
+		  propertyAccessor  | working                                    | base                                     | resultType
 		  RootAccessor.instance | BigInteger.ONE                             | BigDecimal.ONE                           | Number
 		  RootAccessor.instance | new LineNumberReader(new StringReader("")) | new BufferedReader(new StringReader("")) | BufferedReader
 		  RootAccessor.instance | new StringReader("")                       | new BufferedReader(new StringReader("")) | Reader

src/test/java/de/danielbechler/diff/introspection/DefaultPropertyAccessExceptionHandlerTest.groovy

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2014 Daniel Bechler
+ *
+ * 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 de.danielbechler.diff.introspection
+
+import de.danielbechler.diff.node.DiffNode
+import org.slf4j.Logger
+import spock.lang.Specification
+
+/**
+ * Created by Daniel Bechler.
+ */
+class DefaultPropertyAccessExceptionHandlerTest extends Specification {
+	def exceptionHandler
+	def diffNode = Mock(DiffNode)
+	def logger = Mock(Logger)
+
+	def setup() {
+		exceptionHandler = new DefaultPropertyAccessExceptionHandler()
+		exceptionHandler.logger = logger
+	}
+
+	def 'onPropertyReadException should rethrow the PropertyReadException'() {
+		given:
+		  def readException = new PropertyReadException('foo', Object, null)
+		when:
+		  exceptionHandler.onPropertyReadException(readException, diffNode)
+		then:
+		  def thrownException = thrown(PropertyReadException)
+		  thrownException.is readException
+	}
+
+	def 'onPropertyWriteException should rethrow the PropertyWriteException'() {
+		given:
+		  def writeException = new PropertyWriteException('foo', Object, 'bar', null)
+		when:
+		  exceptionHandler.onPropertyWriteException(writeException, diffNode)
+		then:
+		  def thrownException = thrown(PropertyWriteException)
+		  thrownException.is writeException
+	}
+
+	def 'onPropertyWriteException should log exception message'() {
+		given:
+		  def writeException = new PropertyWriteException('foo', Object, 'bar', null)
+		when:
+		  exceptionHandler.onPropertyWriteException(writeException, diffNode)
+		then:
+		  1 * logger.info(writeException.message);
+		and:
+		  thrown(PropertyWriteException)
+	}
+}