Ensure hashCode hashes the same properties as the equality.

Since HashFunctionIdentity is an interface there is no control over what
is hashed. Add a hash function to the HashFunctionValidator to ensure
the hash code is the same if two hash functions are equal according to
the hashFunctionIdentity.

Note: Since Shape is final we use the properties directly and not through the get methods.

src/main/java/org/apache/commons/collections4/bloomfilter/hasher/HashFunctionValidator.java

@@ -16,6 +16,9 @@
  */
 package org.apache.commons.collections4.bloomfilter.hasher;
 
+import java.util.Locale;
+import java.util.Objects;
+
 /**
  * Contains validation for hash functions.
  */
@@ -23,6 +26,33 @@ final class HashFunctionValidator {
     /** Do not instantiate. */
     private HashFunctionValidator() {}
 
+    /**
+     * Generates a hash code for the identity of the hash function. The hash code is
+     * generated using the same properties as those tested in
+     * {@link #areEqual(HashFunctionIdentity, HashFunctionIdentity)}, that is the
+     * signedness, process type and name. The name is not case specific and is converted
+     * to lower-case using the {@link Locale#ROOT root locale}.
+     *
+     * <p>The generated value is suitable for use in generation of a hash code that satisfies
+     * the contract of {@link Object#hashCode()} if the {@link Object#equals(Object)} method
+     * is implemented using {@link #areEqual(HashFunctionIdentity, HashFunctionIdentity)}. That
+     * is two objects considered equal will have the same hash code.
+     *
+     * <p>If the hash function identity is a field within a larger object the generated hash code
+     * should be incorporated into the entire hash, for example using
+     * {@link Objects#hash(Object...)}.
+     *
+     * @param a hash function.
+     * @return hash code
+     * @see String#toLowerCase(Locale)
+     * @see Locale#ROOT
+     */
+    static int hash(HashFunctionIdentity a) {
+        return Objects.hash(a.getSignedness(),
+                            a.getProcessType(),
+                            a.getName().toLowerCase(Locale.ROOT));
+    }
+
     /**
      * Compares the identity of the two hash functions. The functions are considered
      * equal if the signedness, process type and name are equal. The name is not

src/main/java/org/apache/commons/collections4/bloomfilter/hasher/Shape.java

@@ -338,17 +338,21 @@ public final class Shape {
     public boolean equals(final Object o) {
         if (o instanceof Shape) {
             final Shape other = (Shape) o;
-            return
+            return numberOfBits == other.numberOfBits &&
-                getNumberOfBits() == other.getNumberOfBits() &&
+                   numberOfHashFunctions == other.numberOfHashFunctions &&
-                getNumberOfHashFunctions() == other.getNumberOfHashFunctions() &&
+                   HashFunctionValidator.areEqual(hashFunctionIdentity,
-                HashFunctionValidator.areEqual(getHashFunctionIdentity(),
+                                                  other.hashFunctionIdentity);
-                                               other.getHashFunctionIdentity());
         }
         return false;
     }
 
+    @Override
+    public int hashCode() {
+        return hashCode;
+    }
+
     private int generateHashCode() {
-        return Objects.hash(hashFunctionIdentity, numberOfBits, numberOfHashFunctions);
+        return Objects.hash(numberOfBits, numberOfHashFunctions, HashFunctionValidator.hash(hashFunctionIdentity));
     }
 
     /**
@@ -410,11 +414,6 @@ public final class Shape {
             numberOfHashFunctions);
     }
 
-    @Override
-    public int hashCode() {
-        return hashCode;
-    }
-
     @Override
     public String toString() {
         return String.format("Shape[ %s n=%s m=%s k=%s ]",

src/test/java/org/apache/commons/collections4/bloomfilter/hasher/ShapeTest.java

@@ -20,7 +20,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.fail;
 
-import java.util.Objects;
+import org.apache.commons.collections4.bloomfilter.hasher.HashFunctionIdentity.ProcessType;
+import org.apache.commons.collections4.bloomfilter.hasher.HashFunctionIdentity.Signedness;
+
+import java.util.ArrayList;
 
 import org.junit.Test;
 
@@ -458,11 +461,44 @@ public class ShapeTest {
     }
 
     /**
-     * Test that hashCode equals hashCode of hashFunctionIdentity
+     * Test that hashCode satisfies the contract between {@link Object#hashCode()} and
+     * {@link Object#equals(Object)}. Equal shapes must have the same hash code.
      */
     @Test
     public void hashCodeTest() {
-        final int hashCode = Objects.hash(testFunction, 24, 3);
+        // Hash function equality is based on process type, signedness and name (case insensitive)
-        assertEquals(hashCode, shape.hashCode());
+        final ArrayList<HashFunctionIdentity> list = new ArrayList<>();
+        list.add(new HashFunctionIdentityImpl("Provider", "Name", Signedness.SIGNED, ProcessType.ITERATIVE, 0L));
+        // Provider changes
+        list.add(new HashFunctionIdentityImpl("PROVIDER", "Name", Signedness.SIGNED, ProcessType.ITERATIVE, 0L));
+        list.add(new HashFunctionIdentityImpl("Provider2", "Name", Signedness.SIGNED, ProcessType.ITERATIVE, 0L));
+        // Name changes
+        list.add(new HashFunctionIdentityImpl("Provider", "name", Signedness.SIGNED, ProcessType.ITERATIVE, 0L));
+        list.add(new HashFunctionIdentityImpl("Provider", "NAME", Signedness.SIGNED, ProcessType.ITERATIVE, 0L));
+        list.add(new HashFunctionIdentityImpl("Provider", "Other", Signedness.SIGNED, ProcessType.ITERATIVE, 0L));
+        // Signedness changes
+        list.add(new HashFunctionIdentityImpl("Provider", "Name", Signedness.UNSIGNED, ProcessType.ITERATIVE, 0L));
+        // ProcessType changes
+        list.add(new HashFunctionIdentityImpl("Provider", "Name", Signedness.SIGNED, ProcessType.CYCLIC, 0L));
+        // Signature changes
+        list.add(new HashFunctionIdentityImpl("Provider", "Name", Signedness.SIGNED, ProcessType.ITERATIVE, 1L));
+
+        // Create shapes that only differ in the hash function.
+        final int numberOfItems = 30;
+        final int numberOfBits = 3000;
+        final int numberOfHashFunctions = 10;
+        final Shape shape1 = new Shape(list.get(0), numberOfItems, numberOfBits, numberOfHashFunctions);
+        assertEquals(shape1, shape1);
+
+        // Try variations
+        for (int i = 1; i < list.size(); i++) {
+            final Shape shape2 = new Shape(list.get(i), numberOfItems, numberOfBits, numberOfHashFunctions);
+            assertEquals(shape2, shape2);
+
+            // Equal shapes must have the same hash code
+            if (shape1.equals(shape2)) {
+                assertEquals(shape1.hashCode(), shape2.hashCode());
+            }
+        }
     }
 }