AVRO-4354: [java] Fix generated equals() and hashCode() semantics

lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java

@@ -1173,14 +1173,25 @@ public int hashCode(Object o, Schema s) {
           currentHashCode = this.hashCodeAdd(e, elementType);
         }
         return currentHashCode;
+      case MAP:
+        // Consistent with compareMaps(): independent of the entry order and of
+        // the String or Utf8 implementation of the keys and values.
+        int mapHashCode = 0;
+        Schema valueType = s.getValueType();
+        for (Map.Entry<?, ?> entry : ((Map<?, ?>) o).entrySet()) {
+          Object key = entry.getKey();
+          int keyHashCode = Utf8.hashCodeFor(key instanceof CharSequence ? (CharSequence) key : key.toString());
+          mapHashCode += keyHashCode ^ new HashCodeCalculator().hashCode(entry.getValue(), valueType);
+        }
+        return mapHashCode;
       case UNION:
         return hashCode(o, s.getTypes().get(GenericData.this.resolveUnion(s, o)));
       case ENUM:
         return s.getEnumOrdinal(o.toString());
       case NULL:
         return 0;
       case STRING:
-        return (o instanceof Utf8 ? o : new Utf8(o.toString())).hashCode();
+        return Utf8.hashCodeFor(o instanceof CharSequence ? (CharSequence) o : o.toString());
       default:
         return o.hashCode();
       }
@@ -1206,6 +1217,24 @@ public int compare(Object o1, Object o2, Schema s) {
   }
 
   protected int compareMaps(final Map<?, ?> m1, final Map<?, ?> m2) {
+    return compareMaps(m1, m2, null);
+  }
+
+  /**
+   * Compares two maps for equality. With a value schema, the values are compared
+   * with {@link #compare(Object, Object, Schema, boolean)}, so that the
+   * conversions of this model and the equality of Avro, such as a {@link Utf8}
+   * equal to a {@link String} with the same content, also apply to values that
+   * are arrays or maps. Without one, the values are compared with
+   * {@link Object#equals(Object)}, treating a {@link Utf8} and a {@link String}
+   * with the same content as equal.
+   *
+   * @param m1          the first map
+   * @param m2          the second map
+   * @param valueSchema the schema of the values, or {@code null}
+   * @return 0 if the maps are equal, 1 otherwise
+   */
+  protected int compareMaps(final Map<?, ?> m1, final Map<?, ?> m2, final Schema valueSchema) {
     if (m1 == m2) {
       return 0;
     }
@@ -1248,7 +1277,11 @@ protected int compareMaps(final Map<?, ?> m1, final Map<?, ?> m2) {
           }
         } else {
           final Object value2 = m2.get(lookupKey);
-          if (value instanceof Utf8 && value2 instanceof String) {
+          if (valueSchema != null) {
+            if (value2 == null || compare(value, value2, valueSchema, true) != 0) {
+              return 1;
+            }
+          } else if (value instanceof Utf8 && value2 instanceof String) {
             if (!value.toString().equals(value2)) {
               return 1;
             }
@@ -1275,7 +1308,7 @@ protected int compareMaps(final Map<?, ?> m1, final Map<?, ?> m2) {
    * for order.
    */
   @SuppressWarnings(value = "unchecked")
-  protected int compare(Object o1, Object o2, Schema s, boolean equals) {
+  public int compare(Object o1, Object o2, Schema s, boolean equals) {
     if (o1 == o2)
       return 0;
     switch (s.getType()) {
@@ -1306,7 +1339,7 @@ protected int compare(Object o1, Object o2, Schema s, boolean equals) {
       return e1.hasNext() ? 1 : (e2.hasNext() ? -1 : 0);
     case MAP:
       if (equals)
-        return compareMaps((Map) o1, (Map) o2);
+        return compareMaps((Map) o1, (Map) o2, s.getValueType());
       throw new AvroRuntimeException("Can't compare maps!");
     case UNION:
       int i1 = resolveUnion(s, o1);

lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java

@@ -980,7 +980,7 @@ private Schema getSchema(Type type, Map<String, Schema> names) {
   }
 
   @Override
-  protected int compare(Object o1, Object o2, Schema s, boolean equals) {
+  public int compare(Object o1, Object o2, Schema s, boolean equals) {
     switch (s.getType()) {
     case ARRAY:
       if (!o1.getClass().isArray())

lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java

@@ -585,7 +585,7 @@ public Protocol getProtocol(Class iface) {
   }
 
   @Override
-  protected int compare(Object o1, Object o2, Schema s, boolean eq) {
+  public int compare(Object o1, Object o2, Schema s, boolean eq) {
     switch (s.getType()) {
     case ENUM:
       if (o1 instanceof Enum)

lang/java/avro/src/main/java/org/apache/avro/util/Utf8.java

@@ -232,6 +232,50 @@ public void readExternal(ObjectInput in) throws IOException, ClassNotFoundExcept
     in.readFully(bytes);
   }
 
+  /**
+   * Returns the hash code of the UTF-8 encoding of the given character sequence,
+   * that is the hash code of a {@link Utf8} with the same content, without
+   * encoding the sequence. A {@link String} and a {@link Utf8} with the same
+   * content, which are equal according to
+   * {@link #compareSequences(CharSequence, CharSequence)}, therefore have the
+   * same hash code.
+   *
+   * @param cs the character sequence to hash
+   * @return the hash code of the UTF-8 encoding of the character sequence
+   */
+  public static int hashCodeFor(CharSequence cs) {
+    if (cs instanceof Utf8) {
+      return cs.hashCode();
+    }
+    int h = 1;
+    int length = cs.length();
+    for (int i = 0; i < length; i++) {
+      char c = cs.charAt(i);
+      if (c < 0x80) {
+        h = 31 * h + c;
+      } else if (c < 0x800) {
+        h = 31 * h + (byte) (0xC0 | (c >> 6));
+        h = 31 * h + (byte) (0x80 | (c & 0x3F));
+      } else if (Character.isSurrogate(c)) {
+        if (Character.isHighSurrogate(c) && i + 1 < length && Character.isLowSurrogate(cs.charAt(i + 1))) {
+          int codePoint = Character.toCodePoint(c, cs.charAt(++i));
+          h = 31 * h + (byte) (0xF0 | (codePoint >> 18));
+          h = 31 * h + (byte) (0x80 | ((codePoint >> 12) & 0x3F));
+          h = 31 * h + (byte) (0x80 | ((codePoint >> 6) & 0x3F));
+          h = 31 * h + (byte) (0x80 | (codePoint & 0x3F));
+        } else {
+          // An unpaired surrogate is encoded as the replacement character.
+          h = 31 * h + '?';
+        }
+      } else {
+        h = 31 * h + (byte) (0xE0 | (c >> 12));
+        h = 31 * h + (byte) (0x80 | ((c >> 6) & 0x3F));
+        h = 31 * h + (byte) (0x80 | (c & 0x3F));
+      }
+    }
+    return h;
+  }
+
   public static int compareSequences(CharSequence cs1, CharSequence cs2) {
     if (cs1 == cs2) {
       return 0;

lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java

@@ -136,6 +136,25 @@ void testHashCode() {
     GenericData.get().hashCode(r, schema);
   }
 
+  /** AVRO-4354: the hash code of a map must be consistent with compareMaps(). */
+  @Test
+  void testMapHashCodeIsConsistentWithEquals() {
+    Schema s = Schema.createMap(Schema.create(Type.STRING));
+    Map<CharSequence, CharSequence> strings = new HashMap<>();
+    strings.put("a", "x");
+    strings.put("b", "y");
+    Map<CharSequence, CharSequence> utf8s = new HashMap<>();
+    utf8s.put(new Utf8("b"), new Utf8("y"));
+    utf8s.put(new Utf8("a"), new Utf8("x"));
+    Map<CharSequence, CharSequence> other = new HashMap<>();
+    other.put(new Utf8("a"), new Utf8("z"));
+
+    assertEquals(0, GenericData.get().compare(strings, utf8s, s, true));
+    assertEquals(GenericData.get().hashCode(utf8s, s), GenericData.get().hashCode(strings, s));
+    assertEquals(utf8s.hashCode(), GenericData.get().hashCode(utf8s, s));
+    assertNotEquals(GenericData.get().hashCode(other, s), GenericData.get().hashCode(strings, s));
+  }
+
   @Test
   void testEquals() {
     Schema s = recordSchema();

lang/java/avro/src/test/java/org/apache/avro/util/TestUtf8.java

@@ -27,6 +27,7 @@
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.Random;
 
 import org.apache.avro.SystemLimitException;
 import org.apache.avro.TestSystemLimitException;
@@ -162,4 +163,32 @@ void serialization() throws IOException, ClassNotFoundException {
     }
   }
 
+  @Test
+  void hashCodeForMatchesUtf8HashCode() {
+    String[] samples = { "", "a", "abc", "0123456789", "caf\u00e9", "\u20ac uro", "\u4e2d\u6587\u5b57\u7b26",
+        "smile \ud83d\ude00", "\ud83d\ude00\ud83d\ude00\ud83d\ude00", "lone high \ud83d", "lone low \ude00 x", "\ud83d",
+        "\ude00", "high then ascii \ud83da", "\ud83d\ud83d\ude00" };
+    for (String sample : samples) {
+      int expected = new Utf8(sample).hashCode();
+      assertEquals(expected, Utf8.hashCodeFor(sample), sample);
+      assertEquals(expected, Utf8.hashCodeFor(new StringBuilder(sample)), sample);
+      assertEquals(expected, Utf8.hashCodeFor(new Utf8(sample)), sample);
+    }
+  }
+
+  @Test
+  void hashCodeForMatchesUtf8HashCodeForRandomStrings() {
+    Random random = new Random(4354);
+    for (int i = 0; i < 1000; i++) {
+      StringBuilder sb = new StringBuilder();
+      int length = random.nextInt(40);
+      for (int j = 0; j < length; j++) {
+        // Includes unpaired surrogates.
+        sb.appendCodePoint(random.nextInt(Character.MAX_CODE_POINT + 1));
+      }
+      String sample = sb.toString();
+      assertEquals(new Utf8(sample).hashCode(), Utf8.hashCodeFor(sample), sample);
+    }
+  }
+
 }

lang/java/compiler/pom.xml

@@ -138,6 +138,7 @@
                 <argument>${project.basedir}/src/test/resources/full_record_v1.avsc</argument>
                 <argument>${project.basedir}/src/test/resources/full_record_v2.avsc</argument>
                 <argument>${project.basedir}/src/test/resources/regression_error_field_in_record.avsc</argument>
+                <argument>${project.basedir}/src/test/resources/generated_equals.avsc</argument>
                 <argument>${project.basedir}/target/generated-test-sources/javacc</argument>
               </arguments>
             </configuration>

lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java

@@ -1202,6 +1202,41 @@ public boolean isPrimitiveType(Schema schema) {
     return !isUnboxedJavaTypeNullable(schema) && getConvertedLogicalType(schema) == null;
   }
 
+  /**
+   * Utility for template use. Returns whether the values of a field with the
+   * given schema must be compared and hashed through the {@code SpecificData}
+   * model of the generated class, instead of {@code Object.equals()} and
+   * {@code Object.hashCode()}. That is the case for arrays and maps, whose
+   * runtime types differ between built and decoded records (for example
+   * {@code java.util.List} and {@code GenericData.Array}, or {@code String} and
+   * {@code Utf8} elements) and whose elements may need the logical type
+   * conversions of the model, and for unions that are not a nullable type.
+   */
+  public boolean requiresModelComparison(Schema schema) {
+    switch (schema.getType()) {
+    case ARRAY:
+    case MAP:
+      return true;
+    case UNION:
+      List<Schema> types = schema.getTypes();
+      boolean nullable = types.size() == 2 && types.contains(NULL_SCHEMA);
+      return !nullable || types.stream().anyMatch(this::requiresModelComparison);
+    default:
+      return false;
+    }
+  }
+
+  /**
+   * Utility for template use. Returns the expression computing the hash code of
+   * the given field, consistent with the generated {@code equals()}.
+   */
+  public String hashCodeFor(Field field, String name) {
+    if (requiresModelComparison(field.schema())) {
+      return "MODEL$.hashCode(" + name + ", SCHEMA$.getFields().get(" + field.pos() + ").schema())";
+    }
+    return hashCodeFor(field.schema(), name);
+  }
+
   public String hashCodeFor(Schema schema, String name) {
     switch (javaUnbox(schema, false)) {
     case "int":
@@ -1226,6 +1261,10 @@ public String hashCodeFor(Schema schema, String name) {
               + ").ordinal() : " + name + ".hashCode())";
         }
       }
+      if ("java.lang.CharSequence".equals(javaType(schema))) {
+        // Consistent with Utf8.compareSequences(), used by the generated equals().
+        return "(" + name + " == null ? 0 : Utf8.hashCodeFor(" + name + "))";
+      }
       return "(" + name + " == null ? 0 : " + name + ".hashCode())";
     }
   }

lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm

@@ -625,7 +625,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
 #foreach ($field in $schema.getFields())
 #if (!${this.ignoredField($field)})
 #set ($n = ${this.mangle($field.name(), $schema.isError())})
-    result = 31 * result + ${this.hashCodeFor($field.schema(), "this." + $n)};
+    result = 31 * result + ${this.hashCodeFor($field, "this." + $n)};
 #end
 #end
     return result;
@@ -648,6 +648,9 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
     if (this.$n != other.$n) {
 #elseif (${this.javaType($field.schema()).equals("java.lang.CharSequence")})
     if (Utf8.compareSequences(this.$n, other.$n) != 0) {
+#elseif (${this.requiresModelComparison($s)})
+    if (this.$n == null ? other.$n != null : other.$n == null
+        || MODEL$.compare(this.$n, other.$n, SCHEMA$.getFields().get(${field.pos()}).schema(), true) != 0) {
 #else
     if (!java.util.Objects.equals(this.$n, other.$n)) {
 #end