From b7c513c4413ca88b2f25a5b267512815adbf0c15 Mon Sep 17 00:00:00 2001 From: Anton Okolnychyi Date: Sat, 16 Dec 2023 09:15:35 +0100 Subject: [PATCH] API: Fix equals and hashCode in CharSequenceSet (#9245) --- .../apache/iceberg/util/CharSequenceSet.java | 20 +++++++---- .../iceberg/util/TestCharSequenceSet.java | 33 +++++++++++++++++++ 2 files changed, 46 insertions(+), 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 index 7b97807157e6..5c25fc81919b 100644 --- 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 index 4840856252c3..9420548ca9aa 100644 --- 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 set3 = Collections.unmodifiableSet(set2); + + Set 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()); + } }