SOLR-17623: SimpleOrderedMap implements Map (#3048)

Also added NamedList.indexOf(name) convenience method. Does *not* adjust asShallowMap() yet. Coming soon. --------- Co-authored-by: David Smiley <[email protected]>
apache · Jan 31, 2025 · dac5fe1 · dac5fe1
1 parent e81bc32
commit dac5fe1
Show file tree

Hide file tree

Showing 5 changed files with 305 additions and 16 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -162,7 +162,7 @@ Improvements
   other v2 APIs.  SolrJ now offers (experimental) SolrRequest implementations for all v2 configset APIs in
   `org.apache.solr.client.solrj.request.ConfigsetsApi`. (Jason Gerlowski)
 
-Optimizations
+Optimizations                                           
 ---------------------
 * SOLR-17578: Remove ZkController internal core supplier, for slightly faster reconnection after Zookeeper session loss. (Pierre Salagnac)
 
@@ -209,6 +209,8 @@ Other Changes
 * SOLR-17581: Introduce new test variant of waitForState(), that does not wait on live node changes when we're only
  interested in the collection state. (Pierre Salagnac)
 
+* SOLR-17623: SimpleOrderedMap (a NamedList) now implements java.util.Map. (Renato Haeberli, David Smiley)
+
 ==================  9.8.0 ==================
 New Features
 ---------------------

diff --git a/solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java b/solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java
@@ -168,12 +168,14 @@ public SimpleOrderedMap<List<NamedList<Object>>> process(String[] pivots) throws
 
         for (String refinements : refinementValuesByField) {
           pivotResponse.addAll(
-              processSingle(
-                  pivotFields, refinements, statsFields, parsed, facetQueries, facetRanges));
+              (Map<String, List<NamedList<Object>>>)
+                  processSingle(
+                      pivotFields, refinements, statsFields, parsed, facetQueries, facetRanges));
         }
       } else {
         pivotResponse.addAll(
-            processSingle(pivotFields, null, statsFields, parsed, facetQueries, facetRanges));
+            (Map<String, List<NamedList<Object>>>)
+                processSingle(pivotFields, null, statsFields, parsed, facetQueries, facetRanges));
       }
     }
     return pivotResponse;

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
@@ -238,6 +238,15 @@ public int indexOf(String name, int start) {
     return -1;
   }
 
+  /***
+   * Scans the names of the list sequentially beginning at index 0 and returns the index of the first
+   * pair with the specified name.
+   * @see #indexOf(String, int)
+   */
+  public int indexOf(String name) {
+    return indexOf(name, 0);
+  }
+
   /**
    * Gets the value for the first instance of the specified name found.
    *
@@ -403,8 +412,12 @@ public Map<String, T> asShallowMap() {
     return asShallowMap(false);
   }
 
+  /**
+   * @deprecated use {@link SimpleOrderedMap} instead of NamedList when a Map is required.
+   */
+  @Deprecated
   public Map<String, T> asShallowMap(boolean allowDps) {
-    return new Map<String, T>() {
+    return new Map<>() {
       @Override
       public int size() {
         return NamedList.this.size();
@@ -799,7 +812,7 @@ public Collection<String> removeConfigArgs(final String name) throws SolrExcepti
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, err + o.getClass());
     }
 
-    if (collection.size() > 0) {
+    if (!collection.isEmpty()) {
       killAll(name);
     }
 
@@ -845,7 +858,7 @@ public void forEachKey(Consumer<String> fun) {
     }
   }
 
-  public void forEach(BiConsumer<String, ? super T> action) {
+  public void forEach(BiConsumer<? super String, ? super T> action) {
     int sz = size();
     for (int i = 0; i < sz; i++) {
       action.accept(getName(i), getVal(i));

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
@@ -16,9 +16,14 @@
  */
 package org.apache.solr.common.util;
 
+import java.util.AbstractMap;
+import java.util.AbstractSet;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * <code>SimpleOrderedMap</code> is a {@link NamedList} where access by key is more important than
@@ -30,10 +35,12 @@
  * {"foo":10,"bar":20} and may choose to write a NamedList as ["foo",10,"bar",20]. An XML response
  * writer may choose to render both the same way.
  *
- * <p>This class does not provide efficient lookup by key, its main purpose is to hold data to be
- * serialized. It aims to minimize overhead and to be efficient at adding new elements.
+ * <p>This class does not provide efficient lookup by key. The lookup performance is only O(N), and
+ * not O(1) or O(Log N) as it is for the most common Map-implementations. Its main purpose is to
+ * hold data to be serialized. It aims to minimize overhead and to be efficient at adding new
+ * elements.
  */
-public class SimpleOrderedMap<T> extends NamedList<T> {
+public class SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T> {
 
   private static final SimpleOrderedMap<Object> EMPTY = new SimpleOrderedMap<>(List.of());
 
@@ -64,28 +71,124 @@ public SimpleOrderedMap(Map.Entry<String, T>[] nameValuePairs) {
     super(nameValuePairs);
   }
 
+  // TODO override asShallowMap in Solr 10
+
   @Override
   public SimpleOrderedMap<T> clone() {
     ArrayList<Object> newList = new ArrayList<>(nvPairs.size());
     newList.addAll(nvPairs);
     return new SimpleOrderedMap<>(newList);
   }
 
+  @Override
+  public boolean isEmpty() {
+    return nvPairs.isEmpty();
+  }
+
+  @Override
+  public boolean containsKey(final Object key) {
+    return this.indexOf((String) key) >= 0;
+  }
+
+  @Override
+  public boolean containsValue(final Object value) {
+    return values().contains(value);
+  }
+
   /**
-   * Returns a shared, empty, and immutable instance of SimpleOrderedMap.
+   * {@inheritDoc}
+   *
+   * <p>Has linear lookup time O(N)
    *
-   * @return Empty SimpleOrderedMap (immutable)
+   * @see NamedList#get(String)
    */
-  public static SimpleOrderedMap<Object> of() {
-    return EMPTY;
+  @Override
+  public T get(final Object key) {
+    return super.get((String) key);
+  }
+
+  @Override
+  public T put(String key, T value) {
+    int idx = indexOf(key);
+    if (idx == -1) {
+      add(key, value);
+      return null;
+    } else {
+      T t = get(key);
+      setVal(idx, value);
+      return t;
+    }
   }
 
   /**
-   * Returns an immutable instance of SimpleOrderedMap with a single key-value pair.
+   * @see NamedList#remove(String)
+   */
+  @Override
+  public T remove(final Object key) {
+    return super.remove((String) key);
+  }
+
+  @Override
+  public void putAll(final Map<? extends String, ? extends T> m) {
+    if (isEmpty()) {
+      m.forEach(this::add);
+    } else {
+      m.forEach(this::put);
+    }
+  }
+
+  @Override
+  public Set<String> keySet() {
+    return new InnerMap().keySet();
+  }
+
+  @Override
+  public Collection<T> values() {
+    return new InnerMap().values();
+  }
+
+  @Override
+  public Set<Entry<String, T>> entrySet() {
+
+    return new AbstractSet<>() {
+      @Override
+      public Iterator<Entry<String, T>> iterator() {
+        return SimpleOrderedMap.this.iterator();
+      }
+
+      @Override
+      public int size() {
+        return SimpleOrderedMap.this.size();
+      }
+    };
+  }
+
+  /**
+   * Returns an immutable instance of {@link SimpleOrderedMap} with a single key-value pair.
    *
-   * @return SimpleOrderedMap containing one key-value pair
+   * @return {@link SimpleOrderedMap} containing one key-value pair
    */
   public static <T> SimpleOrderedMap<T> of(String name, T val) {
     return new SimpleOrderedMap<>(List.of(name, val));
   }
+
+  /**
+   * Returns a shared, empty, and immutable instance of {@link SimpleOrderedMap}.
+   *
+   * @return Empty {@link SimpleOrderedMap} (immutable)
+   */
+  public static SimpleOrderedMap<Object> of() {
+    return EMPTY;
+  }
+
+  /**
+   * {@link SimpleOrderedMap} extending {@link NamedList}, we are not able to extend {@link
+   * AbstractMap}. With the help of InnerMap we can still use {@link AbstractMap} methods.
+   */
+  private class InnerMap extends AbstractMap<String, T> {
+    @Override
+    public Set<Entry<String, T>> entrySet() {
+      return SimpleOrderedMap.this.entrySet();
+    }
+  }
 }