API: Fix equals and hashCode in CharSequenceSet (apache#9245)

cdouglas · Apr 22, 2024 · b7c513c · b7c513c
1 parent f6ebd37
commit b7c513c
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 7 deletions.
diff --git a/api/src/main/java/org/apache/iceberg/util/CharSequenceSet.java b/api/src/main/java/org/apache/iceberg/util/CharSequenceSet.java
@@ -21,7 +21,6 @@
 import java.io.Serializable;
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
@@ -168,22 +167,29 @@ public void clear() {
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) {
+  public boolean equals(Object other) {
+    if (this == other) {
       return true;
+    } else if (!(other instanceof Set)) {
+      return false;
     }
 
-    if (o == null || getClass() != o.getClass()) {
+    Set<?> that = (Set<?>) other;
+
+    if (size() != that.size()) {
       return false;
     }
 
-    CharSequenceSet that = (CharSequenceSet) o;
-    return wrapperSet.equals(that.wrapperSet);
+    try {
+      return containsAll(that);
+    } catch (ClassCastException | NullPointerException unused) {
+      return false;
+    }
   }
 
   @Override
   public int hashCode() {
-    return Objects.hashCode(wrapperSet);
+    return wrapperSet.stream().mapToInt(CharSequenceWrapper::hashCode).sum();
   }
 
   @Override

diff --git a/api/src/test/java/org/apache/iceberg/util/TestCharSequenceSet.java b/api/src/test/java/org/apache/iceberg/util/TestCharSequenceSet.java
@@ -19,8 +19,10 @@
 package org.apache.iceberg.util;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Set;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -79,4 +81,35 @@ public void testRemoveAll() {
 
     Assertions.assertThat(set).isEmpty();
   }
+
+  @Test
+  public void testEqualsAndHashCode() {
+    CharSequenceSet set1 = CharSequenceSet.empty();
+    CharSequenceSet set2 = CharSequenceSet.empty();
+
+    Assertions.assertThat(set1).isEqualTo(set2);
+    Assertions.assertThat(set1.hashCode()).isEqualTo(set2.hashCode());
+
+    set1.add("v1");
+    set1.add("v2");
+    set1.add("v3");
+
+    set2.add(new StringBuilder("v1"));
+    set2.add(new StringBuffer("v2"));
+    set2.add("v3");
+
+    Set<CharSequence> set3 = Collections.unmodifiableSet(set2);
+
+    Set<CharSequenceWrapper> set4 =
+        ImmutableSet.of(
+            CharSequenceWrapper.wrap("v1"),
+            CharSequenceWrapper.wrap(new StringBuffer("v2")),
+            CharSequenceWrapper.wrap(new StringBuffer("v3")));
+
+    Assertions.assertThat(set1).isEqualTo(set2).isEqualTo(set3).isEqualTo(set4);
+    Assertions.assertThat(set1.hashCode())
+        .isEqualTo(set2.hashCode())
+        .isEqualTo(set3.hashCode())
+        .isEqualTo(set4.hashCode());
+  }
 }