Enable ComparableComparisonStrategy for more types

So far this strategy only applied to BigDecimals. It has now been
extended to automatically apply to all simple types that implement
`java.lang.Comparable`.

The list of types that qualify as _simple_ has also been extended. It
now looks as follows:

* all primitive types (int, short, byte, etc.)
* all primitive wrapper types (Integer, Short, Byte, etc.)
* enums
* subclasses of:
    * `java.math.BigDecimal`
    * `java.math.BigInteger`
    * `java.lang.CharSequence`
    * `java.util.Calendar`
    * `java.util.Date`
* `java.lang.Class`
* `java.net.URI`
* `java.net.URL`
* `java.util.Locale`
* `java.util.UUID`

Of course this behaviour can be overriden via configuration API or
property annotations.

To make comparison via compareTo more reliable from now on compareTo
will be invoked on the working and the base object and objects will be
considered equal if either one of this comparisons returns true.

Fixes #85 and fixes #128

Author: SQiShER

CHANGELOG.md

@@ -1,5 +1,40 @@
 # Changelog
 
+## 0.91.2
+
+### Improvements
+
+- **Comparison:** Enable ComparableComparisonStrategy for more types
+
+	So far this strategy only applied to BigDecimals. It has now been extended
+	to automatically apply to all simple types that implement `java.lang.Comparable`.
+
+	The list of types that qualify as _simple_ has also been extended. It now
+	looks as follows:
+
+	* all primitive types (int, short, byte, etc.)
+	* all primitive wrapper types (Integer, Short, Byte, etc.)
+	* enums
+	* subclasses of:
+		* `java.math.BigDecimal`
+		* `java.math.BigInteger`
+		* `java.lang.CharSequence`
+		* `java.util.Calendar`
+		* `java.util.Date`
+	* `java.lang.Class`
+	* `java.net.URI`
+	* `java.net.URL`
+	* `java.util.Locale`
+	* `java.util.UUID`
+
+	Of course this behaviour can be overriden via configuration API or
+	property annotations.
+
+    To make comparison via `compareTo` more reliable, from now on `compareTo`
+    will be invoked on the `working` and the `base` object and both will be
+    considered equal if either one of this comparisons returns `true`.
+- **Comparison:** Dates are now compared via compareTo method to workaround the strictness of java.util.Date's equals method, which only returns true for other java.util.Dates, but not for extending classes like java.sql.Date. [#85]
+
 ## 0.91.1
 
 ### Features

src/integration-test/java/de/danielbechler/diff/comparison/DateComparisonIT.groovy

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2015 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.comparison
+
+import de.danielbechler.diff.ObjectDifferBuilder
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import java.sql.Time
+import java.sql.Timestamp
+
+class DateComparisonIT extends Specification {
+
+	@Unroll
+	def 'should consider java.util.Date and #subclass equal when they represent the same date'() {
+		def now = new Timestamp(System.currentTimeMillis()).time
+
+		given:
+		  def utilDate = new Date(now)
+		  def sqlDate = subclass.newInstance([now] as Object[])
+		expect:
+		  ObjectDifferBuilder.buildDefault().compare(utilDate, sqlDate).untouched
+		where:
+		  subclass << [java.sql.Date, Time, Timestamp]
+	}
+
+	def 'should consider java.sql.Time and java.util.Date equal when they represent the same date'() {
+		def now = System.currentTimeMillis()
+
+		given:
+		  def utilDate = new Date(now)
+		  def time = new Time(now)
+		expect:
+		  ObjectDifferBuilder.buildDefault().compare(utilDate, time).untouched
+	}
+}

src/main/java/de/danielbechler/diff/comparison/ComparisonService.java

@@ -27,11 +27,11 @@
 import java.util.HashMap;
 import java.util.Map;
 
-/**
- *
- */
 public class ComparisonService implements ComparisonConfigurer, ComparisonStrategyResolver, PrimitiveDefaultValueModeResolver
 {
+	private static final ComparisonStrategy COMPARABLE_COMPARISON_STRATEGY = new ComparableComparisonStrategy();
+	private static final ComparisonStrategy EQUALS_ONLY_COMPARISON_STRATEGY = new EqualsOnlyComparisonStrategy();
+
 	private final NodePathValueHolder<ComparisonStrategy> nodePathComparisonStrategies = NodePathValueHolder.of(ComparisonStrategy.class);
 	private final Map<Class<?>, ComparisonStrategy> typeComparisonStrategyMap = new HashMap<Class<?>, ComparisonStrategy>();
 	private final ObjectDifferBuilder objectDifferBuilder;
@@ -57,14 +57,18 @@ public ComparisonStrategy resolveComparisonStrategy(final DiffNode node)
 			return typeComparisonStrategyMap.get(valueType);
 		}
 
-		if (Classes.isComparableType(valueType))
-		{
-			return new ComparableComparisonStrategy();
-		}
-
 		if (Classes.isSimpleType(valueType))
 		{
-			return new EqualsOnlyComparisonStrategy();
+			// if the simple type implements comparable we use that, since its contract
+			// dictates that compareTo == zero carries the same semantics as equals
+			if (Classes.isComparableType(valueType))
+			{
+				return COMPARABLE_COMPARISON_STRATEGY;
+			}
+			else
+			{
+				return EQUALS_ONLY_COMPARISON_STRATEGY;
+			}
 		}
 
 		final ObjectDiffPropertyComparisonStrategyResolver comparisonStrategyResolver = ObjectDiffPropertyComparisonStrategyResolver.instance;

src/main/java/de/danielbechler/util/Classes.java

@@ -22,31 +22,67 @@
 import java.io.Serializable;
 import java.lang.reflect.Constructor;
 import java.math.BigDecimal;
+import java.math.BigInteger;
 import java.net.URI;
 import java.net.URL;
+import java.util.Calendar;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.UUID;
+
+import static java.util.Arrays.asList;
 
-/**
- * @author Daniel Bechler
- */
 public final class Classes
 {
 	private static final Logger logger = LoggerFactory.getLogger(Classes.class);
-
-	private static final Set<Class<?>> WRAPPER_TYPES = getWrapperTypes();
+	private static final Set<Class<?>> PRIMITIVE_WRAPPER_TYPES = getPrimitiveWrapperTypes();
 	private static final Collection<Class<?>> PRIMITIVE_NUMERIC_TYPES = getPrimitiveNumericTypes();
+	//	private static final List<?> COMPARABLE_TYPES = Arrays.asList(
+	//			// The Extendables
+	//			// since Java 1.8
+	//			Duration.class,
+	//			Instant.class,
+	//			LocalDate.class,
+	//			LocalDateTime.class,
+	//			LocalTime.class,
+	//			MonthDay.class,
+	//			OffsetDateTime.class,
+	//			OffsetTime.class,
+	//			Year.class,
+	//			YearMonth.class,
+	//			ZonedDateTime.class,
+	//			ZoneOffset.class
+	//	);
+	@SuppressWarnings("unchecked")
+	private static final Collection<Class<?>> EXTENDABLE_SIMPLE_TYPES = asList(
+			BigDecimal.class,
+			BigInteger.class,
+			CharSequence.class, // String, StringBuilder, etc.
+			Calendar.class, // GregorianCalendar, etc.
+			Date.class, // java.sql.Date, java.util.Date, java.util.Time, etc.
+			Enum.class // enums... duh
+
+	);
+	@SuppressWarnings("unchecked")
+	private static final List<Class<? extends Serializable>> FINAL_SIMPLE_TYPES = asList(
+			Class.class,
+			URI.class,
+			URL.class,
+			Locale.class,
+			UUID.class
+	);
 
 	private Classes()
 	{
 	}
 
-	private static Set<Class<?>> getWrapperTypes()
+	private static Set<Class<?>> getPrimitiveWrapperTypes()
 	{
 		final Set<Class<?>> wrapperTypes = new HashSet<Class<?>>();
 		wrapperTypes.add(Boolean.class);
@@ -76,50 +112,44 @@ private static Collection<Class<?>> getPrimitiveNumericTypes()
 		numericTypes.add(long.class);
 		numericTypes.add(float.class);
 		numericTypes.add(double.class);
+//
+// The following are not numeric types and tests fail when they are commented in. Feels weird...
+//
+//		numericTypes.add(boolean.class);
+//		numericTypes.add(void.class);
+//
 		return numericTypes;
 	}
 
+	public static boolean isComparableType(final Class<?> clazz)
+	{
+		return Comparable.class.isAssignableFrom(clazz);
+	}
+
 	public static boolean isSimpleType(final Class<?> clazz)
 	{
-		if (clazz != null)
+		if (clazz == null)
 		{
-			if (isPrimitiveType(clazz))
-			{
-				return true;
-			}
-			else if (isPrimitiveWrapperType(clazz))
-			{
-				return true;
-			}
-			else if (clazz.isEnum())
-			{
-				return true;
-			}
-			else if (CharSequence.class.isAssignableFrom(clazz))
-			{
-				return true;
-			}
-			else if (Number.class.isAssignableFrom(clazz))
-			{
-				return true;
-			}
-			else if (Date.class.isAssignableFrom(clazz))
-			{
-				return true;
-			}
-			else if (URI.class.equals(clazz))
-			{
-				return true;
-			}
-			else if (URL.class.equals(clazz))
-			{
-				return true;
-			}
-			else if (Locale.class.equals(clazz))
+			return false;
+		}
+		else if (isPrimitiveType(clazz))
+		{
+			return true;
+		}
+		else if (isPrimitiveWrapperType(clazz))
+		{
+			return true;
+		}
+		for (final Class<?> type : FINAL_SIMPLE_TYPES)
+		{
+			if (type.equals(clazz))
 			{
 				return true;
 			}
-			else if (Class.class.equals(clazz))
+		}
+		for (final Class<?> type : EXTENDABLE_SIMPLE_TYPES)
+		{
+			if (type.isAssignableFrom(clazz))
 			{
 				return true;
 			}
@@ -134,12 +164,7 @@ public static boolean isPrimitiveType(final Class<?> clazz)
 
 	public static boolean isPrimitiveWrapperType(final Class<?> clazz)
 	{
-		return clazz != null && WRAPPER_TYPES.contains(clazz);
-	}
-
-	public static boolean isComparableType(final Class<?> clazz)
-	{
-		return BigDecimal.class.equals(clazz);
+		return clazz != null && PRIMITIVE_WRAPPER_TYPES.contains(clazz);
 	}
 
 	public static <T> T freshInstanceOf(final Class<T> clazz)
@@ -272,4 +297,49 @@ else if (o2.isAssignableFrom(o1))
 			}
 		}
 	}
+
+	private static final class TypeConfig
+	{
+		private final Class<?> type;
+		private final boolean simple;
+		private final boolean boxed;
+		private final boolean comparable;
+		private final boolean extendable;
+
+		public TypeConfig(final Class<?> type, final boolean simple, final boolean boxed, final boolean comparable, final boolean extendable)
+		{
+			this.type = type;
+			this.simple = simple;
+			this.boxed = boxed;
+			this.comparable = comparable;
+			this.extendable = extendable;
+		}
+
+		public boolean isSimple()
+		{
+			return simple;
+		}
+
+		public boolean isBoxed()
+		{
+			return boxed;
+		}
+
+		public boolean isComparable()
+		{
+			return comparable;
+		}
+
+		public boolean appliesTo(final Class<?> otherType)
+		{
+			if (extendable)
+			{
+				return type.isAssignableFrom(otherType);
+			}
+			else
+			{
+				return type.equals(otherType);
+			}
+		}
+	}
 }

src/main/java/de/danielbechler/util/Comparables.java

@@ -34,7 +34,13 @@ public static <T extends Comparable<T>> boolean isEqualByComparison(final T a, f
 		}
 		else if (a != null && b != null)
 		{
-			return a.compareTo(b) == 0;
+			// When testing the comparison of java.util.Date and java.sql.Timestamp I noticed,
+			// that they were never considered equal, because java.util.Date was used as the
+			// working object. This way only its compareTo method was used, but never the one
+			// of java.sql.Date, although the subclass added some magic to make the objects
+			// compatible. To remedy this, this method tests both objects against each other and
+			// returns true when one of the comparisons returns true.
+			return a.compareTo(b) == 0 || b.compareTo(a) == 0;
 		}
 		return false;
 	}