This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 046677d10b3 SOLR-17623: SimpleOrderedMap equals, hashCode (and for
Map.Entry) (#3214)
046677d10b3 is described below
commit 046677d10b384dd399da53b89d19ab457ab5e6b1
Author: David Smiley <[email protected]>
AuthorDate: Mon Mar 3 01:33:02 2025 -0500
SOLR-17623: SimpleOrderedMap equals, hashCode (and for Map.Entry) (#3214)
And ensure a NamedList is never equal to a SimpleOrderedMap
---
.../org/apache/solr/common/util/NamedList.java | 113 ++++++++++-----------
.../apache/solr/common/util/SimpleOrderedMap.java | 17 +++-
.../solr/common/util/SimpleOrderedMapTest.java | 32 ++++++
3 files changed, 97 insertions(+), 65 deletions(-)
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java
b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java
index 98b4efd42bd..32ea838a18e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java
@@ -18,6 +18,7 @@ package org.apache.solr.common.util;
import java.io.IOException;
import java.io.Serializable;
+import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -566,35 +567,11 @@ public class NamedList<T>
* Helper class implementing Map.Entry<String, T> to store the
key-value relationship in
* NamedList (the keys of which are String-s)
*/
- public static final class NamedListEntry<T> implements Map.Entry<String, T> {
-
- public NamedListEntry() {}
-
+ @Deprecated // use AbstractMap.SimpleEntry or Map.entry() (albeit no nulls)
+ public static final class NamedListEntry<T> extends
AbstractMap.SimpleEntry<String, T> {
public NamedListEntry(String _key, T _value) {
- key = _key;
- value = _value;
- }
-
- @Override
- public String getKey() {
- return key;
- }
-
- @Override
- public T getValue() {
- return value;
- }
-
- @Override
- public T setValue(T _value) {
- T oldValue = value;
- value = _value;
- return oldValue;
+ super(_key, _value);
}
-
- private String key;
-
- private T value;
}
/** Iterates over the Map and sequentially adds its key/value pairs */
@@ -627,53 +604,63 @@ public class NamedList<T>
/** Support the Iterable interface */
@Override
public Iterator<Map.Entry<String, T>> iterator() {
+ return new Iterator<>() {
- final NamedList<T> list = this;
+ int idx = 0;
- Iterator<Map.Entry<String, T>> iter =
- new Iterator<Map.Entry<String, T>>() {
+ @Override
+ public boolean hasNext() {
+ return idx < NamedList.this.size();
+ }
- int idx = 0;
+ @Override
+ public Map.Entry<String, T> next() {
+ return new Map.Entry<>() {
+ final int index = idx++;
@Override
- public boolean hasNext() {
- return idx < list.size();
+ public String getKey() {
+ return NamedList.this.getName(index);
}
@Override
- public Map.Entry<String, T> next() {
- final int index = idx++;
- Map.Entry<String, T> nv =
- new Map.Entry<String, T>() {
- @Override
- public String getKey() {
- return list.getName(index);
- }
-
- @Override
- public T getValue() {
- return list.getVal(index);
- }
-
- @Override
- public String toString() {
- return getKey() + "=" + getValue();
- }
-
- @Override
- public T setValue(T value) {
- return list.setVal(index, value);
- }
- };
- return nv;
+ public T getValue() {
+ return NamedList.this.getVal(index);
}
@Override
- public void remove() {
- throw new UnsupportedOperationException();
+ public T setValue(T value) {
+ return NamedList.this.setVal(index, value);
+ }
+
+ // The following implement the Map.Entry specification:
+
+ @Override
+ public boolean equals(Object o) {
+ return o instanceof Map.Entry<?, ?> e
+ && Objects.equals(getKey(), e.getKey())
+ && Objects.equals(getValue(), e.getValue());
+ }
+
+ @Override
+ public int hashCode() {
+ var key = getKey();
+ var value = getValue();
+ return (key == null ? 0 : key.hashCode()) ^ (value == null ? 0 :
value.hashCode());
+ }
+
+ @Override
+ public String toString() {
+ return getKey() + "=" + getValue();
}
};
- return iter;
+ }
+
+ @Override
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
+ };
}
/**
@@ -830,7 +817,11 @@ public class NamedList<T>
@Override
public boolean equals(Object obj) {
+ if (obj == this) return true;
if (!(obj instanceof NamedList<?> nl)) return false;
+ if (obj instanceof SimpleOrderedMap<?>) {
+ return false;
+ }
return this.nvPairs.equals(nl.nvPairs);
}
diff --git
a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java
b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java
index cdd12b97749..a30324b8719 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java
@@ -56,12 +56,8 @@ public class SimpleOrderedMap<T> extends NamedList<T>
implements Map<String, T>
/**
* Creates an instance backed by an explicitly specified list of pairwise
names/values.
*
- * <p>TODO: this method was formerly public, now that it's not we can change
the impl details of
- * this class to be based on a Map.Entry[]
- *
* @param nameValuePairs underlying List which should be used to implement a
SimpleOrderedMap;
* modifying this List will affect the SimpleOrderedMap.
- * @lucene.internal
*/
private SimpleOrderedMap(List<Object> nameValuePairs) {
super(nameValuePairs);
@@ -80,6 +76,19 @@ public class SimpleOrderedMap<T> extends NamedList<T>
implements Map<String, T>
return new SimpleOrderedMap<>(newList);
}
+ @SuppressWarnings("EqualsDoesntCheckParameterClass")
+ @Override
+ public boolean equals(Object obj) {
+ return obj == this || new InnerMap().equals(obj); // Use AbstractMap's code
+ }
+
+ @Override
+ public int hashCode() {
+ return new InnerMap().hashCode(); // Use AbstractMap's code
+ }
+
+ // toString is inherited and implements the Map contract (and this is tested)
+
@Override
public boolean isEmpty() {
return nvPairs.isEmpty();
diff --git
a/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java
b/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java
index 78038083b11..b91f991251c 100644
--- a/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java
@@ -17,6 +17,7 @@
package org.apache.solr.common.util;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import org.apache.solr.SolrTestCase;
@@ -38,6 +39,37 @@ public class SimpleOrderedMapTest extends SolrTestCase {
assertEquals(2, map.nvPairs.get(3));
}
+ @Test
+ public void testEquals() {
+ var map2 = new SimpleOrderedMap<Integer>();
+ map2.put("one", 1);
+ map2.put("two", 2);
+
+ var map3 = map2.clone();
+ assertNotSame(map2, map3);
+ assertEquals(map2, map3);
+ map3.put("three", 3);
+ assertNotEquals(map2, map3);
+ map3.remove("one");
+ map3.remove("three");
+ map3.add("one", 1); // now it's a different order
+ assertNotEquals(map2.toString(), map3.toString());
+ assertEquals(map2, map3); // but still equals despite different order
+ assertEquals(map2.hashCode(), map3.hashCode());
+
+ var nl2 = new NamedList<Object>(map2);
+ assertNotEquals(map2, nl2);
+ assertNotEquals(nl2, map2);
+ }
+
+ public void testToString() {
+ SimpleOrderedMap<Object> map = new SimpleOrderedMap<>();
+ map.add("one", 1);
+ map.add("two", 2);
+ map.add("aNull", null);
+ assertEquals(new LinkedHashMap<>(map).toString(), map.toString());
+ }
+
public void testPutReturnOldValue() {
map.put("one", 1);
int oldValue = map.put("one", 2);
