improve BeanComparator test coverage

Author: stevebosman

src/main/java/org/apache/commons/beanutils2/BeanComparator.java

@@ -24,7 +24,6 @@
  * <p>
  * This comparator compares two beans by the specified bean property. It is also possible to compare beans based on nested, indexed, combined, mapped bean
  * properties. Please see the {@link PropertyUtilsBean} documentation for all property name possibilities.
- *
  * </p>
  * <p>
  * <strong>Note:</strong> The BeanComparator passes the values of the specified bean property to an internal natural order {@link Comparator}, if no comparator
@@ -42,49 +41,12 @@ public class BeanComparator<T, V> implements Comparator<T> {
     /**
      * A {@link Comparator Comparator} that compares {@link Comparable Comparable} objects.
      * <p>
-     * This Comparator is useful, for example, for enforcing the natural order in custom implementations of {@link java.util.SortedSet SortedSet} and
-     * {@link java.util.SortedMap SortedMap}.
+     * This Comparator is useful, for example, for enforcing the natural order in custom implementations
+     * of {@link java.util.SortedSet SortedSet} and {@link java.util.SortedMap SortedMap}.
      * </p>
-     *
-     * @param <E> the type of objects compared by this comparator
-     * @see java.util.Collections#reverseOrder()
      */
-    private static final class NaturalOrderComparator<E extends Comparable<? super E>> implements Comparator<E> {
-
-        /** The singleton instance. */
-        @SuppressWarnings("rawtypes")
-        public static final NaturalOrderComparator INSTANCE = new NaturalOrderComparator();
-
-        /**
-         * Private constructor to prevent instantiation. Only use INSTANCE.
-         */
-        private NaturalOrderComparator() {
-        }
-
-        /**
-         * Compare the two {@link Comparable Comparable} arguments. This method is equivalent to:
-         *
-         * <pre>
-         * ((Comparable) obj1).compareTo(obj2)
-         * </pre>
-         */
-        @Override
-        public int compare(final E obj1, final E obj2) {
-            return obj1.compareTo(obj2);
-        }
-
-        @Override
-        public boolean equals(final Object object) {
-            return this == object || null != object && object.getClass().equals(this.getClass());
-        }
-
-        @Override
-        public int hashCode() {
-            return "NaturalOrderComparator".hashCode();
-        }
-    }
-
-    private static final long serialVersionUID = 1L;
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    private static final Comparator NATURAL_ORDER_COMPARATOR = (obj1, obj2) -> ((Comparable) obj1).compareTo(obj2);
 
     /** Property. */
     private String property;
@@ -120,8 +82,9 @@ public BeanComparator() {
      * @param property String Name of a bean property, which may contain the name of a simple, nested, indexed, mapped, or combined property. See
      *                 {@link PropertyUtilsBean} for property query language syntax. If the property passed in is null then the actual objects will be compared
      */
+    @SuppressWarnings("unchecked")
     public BeanComparator(final String property) {
-        this(property, NaturalOrderComparator.INSTANCE);
+        this(property, NATURAL_ORDER_COMPARATOR);
     }
 
     /**
@@ -135,7 +98,7 @@ public BeanComparator(final String property) {
      */
     public BeanComparator(final String property, final Comparator<V> comparator) {
         setProperty(property);
-        this.comparator = comparator != null ? comparator : NaturalOrderComparator.INSTANCE;
+        this.comparator = comparator != null ? comparator : NATURAL_ORDER_COMPARATOR;
     }
 
     /**
@@ -158,7 +121,7 @@ public int compare(final T o1, final T o2) {
             final Object value2 = PropertyUtils.getProperty(o2, property);
             return internalCompare(value1, value2);
         } catch (final NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
-            throw new RuntimeException(e.getClass().getSimpleName() + ": " + e.toString());
+            throw new RuntimeException(e.getClass().getSimpleName() + ": " + e);
         }
     }
 

src/test/java/org/apache/commons/beanutils2/BeanComparatorTest.java

@@ -17,10 +17,14 @@
 
 package org.apache.commons.beanutils2;
 
+import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.Comparator;
+
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -168,4 +172,96 @@ public void testSimpleCompareInverse() {
         final int result = beanComparator.compare(alphaBean2, alphaBean1);
         assertEquals(1, result, () -> "Comparator did not sort properly.  Result:" + result);
     }
+
+    /**
+     * Tests comparing two beans via their name using the default natural order Comparator
+     */
+    @Test
+    public void testSimpleCompareWithDefaultNaturalComparator() {
+        final BeanComparator<AlphaBean, String> beanComparator = new BeanComparator<>("name", null);
+        final int result = beanComparator.compare(alphaBean1, alphaBean2);
+        assertEquals(-1, result, () -> "Comparator did not sort properly.  Result:" + result);
+    }
+
+    /**
+     * Tests comparing two beans via their name using the default natural order Comparator, but the inverse
+     */
+    @Test
+    public void testSimpleCompareInverseWithDefaultNaturalComparator() {
+        final BeanComparator<AlphaBean, String> beanComparator = new BeanComparator<>("name", null);
+        final int result = beanComparator.compare(alphaBean2, alphaBean1);
+        assertEquals(1, result, () -> "Comparator did not sort properly.  Result:" + result);
+    }
+
+    /**
+     * Tests comparing two comparable beans using the default natural order Comparator
+     */
+    @Test
+    public void testNaturalCompare() {
+        final BeanComparator<String, ?> beanComparator = new BeanComparator<>();
+        final int result = beanComparator.compare("string1", "string2");
+        assertEquals(-1, result, () -> "Comparator did not sort properly.  Result:" + result);
+    }
+
+    /**
+     * Tests comparing two comparable beans using the default natural order Comparator, but the inverse
+     */
+    @Test
+    public void testNaturalCompareInverse() {
+        final BeanComparator<String, ?> beanComparator = new BeanComparator<>();
+        final int result = beanComparator.compare("string2", "string1");
+        assertEquals(1, result, () -> "Comparator did not sort properly.  Result:" + result);
+    }
+
+    /**
+     * Tests comparing two beans via their name using the default Comparator
+     */
+    @Test
+    public void testWithCustomComparator() {
+        final Comparator<AlphaBean> comparator = Comparator.comparing(AlphaBean::getName);
+
+        final BeanComparator<AlphaBean, ?> beanComparator = new BeanComparator<>(null, comparator);
+        final int result = beanComparator.compare(alphaBean1, alphaBean2);
+        assertEquals(-1, result, () -> "Comparator did not sort properly.  Result:" + result);
+    }
+
+    /**
+     * Tests comparing two beans via their name using the default Comparator
+     */
+    @Test
+    public void testWithCustomComparatorInverse() {
+        final Comparator<AlphaBean> comparator = Comparator.comparing(AlphaBean::getName);
+
+        final BeanComparator<AlphaBean, ?> beanComparator = new BeanComparator<>(null, comparator);
+        final int result = beanComparator.compare(alphaBean2, alphaBean1);
+        assertEquals(1, result, () -> "Comparator did not sort properly.  Result:" + result);
+    }
+
+    @Test
+    public void testEquals() {
+        final Comparator<AlphaBean> comparator = Comparator.comparing(AlphaBean::getName);
+        final BeanComparator<AlphaBean, String> nameComparator = new BeanComparator<>("name");
+        final BeanComparator<AlphaBean, String> nameComparator2 = new BeanComparator<>("name");
+        final BeanComparator<AlphaBean, String> nullPropertyComparator1 = new BeanComparator<>(null);
+        final BeanComparator<AlphaBean, String> nullPropertyComparator2 = new BeanComparator<>(null);
+        final BeanComparator<AlphaBean, String> nameComparatorDifferentComparator = new BeanComparator<>("name",
+                                                                                             String::compareTo);
+        final BeanComparator<AlphaBean, Boolean> booleanComparator = new BeanComparator<>("booleanProperty");
+        assertAll(
+                () -> assertEquals(nameComparator, nameComparator,
+                                   "an instance should be equal to itself"),
+                () -> assertNotEquals(nameComparator, comparator,
+                                      "an instance should not be equal to a non-BeanComparator"),
+                () -> assertNotEquals(nameComparator, nameComparatorDifferentComparator,
+                                   "an instance should not be equal to a BeanComparator using different comparator"),
+                () -> assertNotEquals(nameComparator, booleanComparator,
+                                      "an instance should not be equal to a BeanComparator using different property"),
+                () -> assertEquals(nameComparator, nameComparator2,
+                                   "an instance should be equal to a BeanComparator with same comparator and property"),
+                () -> assertNotEquals(nullPropertyComparator1, booleanComparator,
+                                      "an instance with null property comparator should not be equal to a BeanComparator using a property"),
+                () -> assertEquals(nullPropertyComparator1, nullPropertyComparator2,
+                                   "an instance with null property comparator should be equal to a BeanComparator with same comparator and a null property")
+        );
+    }
 }