SimpleOrderedMap MapWriter constructor (#3235)

* SimpleOrderedMap MapWriter constructor
* new SimpleOrderedMap(MapWriter)   ex: a SolrParams
* deprecate SolrParams.toNamedList
* optimize SolrParams.writeMap

Bigger picture is avoiding NamedList.

* comment for the array side

Author: dsmiley

solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java

@@ -461,7 +461,7 @@ public IndexFetchResult doFetch(SolrParams solrParams, boolean forceReplication)
         if (currentIndexFetcher != null && currentIndexFetcher != pollingIndexFetcher) {
           currentIndexFetcher.destroy();
         }
-        currentIndexFetcher = new IndexFetcher(solrParams.toNamedList(), this, core);
+        currentIndexFetcher = new IndexFetcher(new SimpleOrderedMap<>(solrParams), this, core);
       } else {
         currentIndexFetcher = pollingIndexFetcher;
       }

solr/core/src/test/org/apache/solr/search/TestMinHashQParser.java

@@ -22,6 +22,7 @@
 import org.apache.lucene.search.ConstantScoreQuery;
 import org.apache.lucene.search.Query;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.request.SolrQueryRequest;
@@ -431,13 +432,12 @@ public void testBandsWrap() throws SyntaxError {
 
   private SolrQueryRequest createRequest(String query) {
     SolrQueryRequest qr = req(query);
-    NamedList<Object> par = qr.getParams().toNamedList();
-    par.add("debug", "false");
-    par.add("rows", "30");
-    par.add("fl", "id,score");
+    ModifiableSolrParams par = ModifiableSolrParams.of(qr.getParams());
+    par.set("debug", "false");
+    par.set("rows", "30");
+    par.set("fl", "id,score");
     par.remove("qt");
-    SolrParams newp = par.toSolrParams();
-    qr.setParams(newp);
+    qr.setParams(par);
     return qr;
   }
 }

solr/core/src/test/org/apache/solr/update/processor/RegexBoostProcessorTest.java

@@ -19,6 +19,7 @@
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.response.SolrQueryResponse;
@@ -32,7 +33,7 @@
 public class RegexBoostProcessorTest extends SolrTestCaseJ4 {
   private static RegexpBoostProcessor reProcessor;
   protected static SolrRequestParsers _parser;
-  protected static ModifiableSolrParams parameters;
+  protected static SimpleOrderedMap<String> config;
   private static RegexpBoostProcessorFactory factory;
   private SolrInputDocument document;
 
@@ -43,13 +44,13 @@ public static void setUpBeforeClass() throws Exception {
     SolrCore core = h.getCore();
     _parser = new SolrRequestParsers(null);
     SolrQueryResponse resp = null;
-    parameters = new ModifiableSolrParams();
-    parameters.set(RegexpBoostProcessor.BOOST_FILENAME_PARAM, "regex-boost-processor-test.txt");
-    parameters.set(RegexpBoostProcessor.INPUT_FIELD_PARAM, "url");
-    parameters.set(RegexpBoostProcessor.BOOST_FIELD_PARAM, "urlboost");
+    config = new SimpleOrderedMap<>();
+    config.add(RegexpBoostProcessor.BOOST_FILENAME_PARAM, "regex-boost-processor-test.txt");
+    config.add(RegexpBoostProcessor.INPUT_FIELD_PARAM, "url");
+    config.add(RegexpBoostProcessor.BOOST_FIELD_PARAM, "urlboost");
     SolrQueryRequest req = _parser.buildRequestFrom(core, new ModifiableSolrParams(), null);
     factory = new RegexpBoostProcessorFactory();
-    factory.init(parameters.toNamedList());
+    factory.init(config);
     reProcessor = (RegexpBoostProcessor) factory.getInstance(req, resp, null);
   }
 
@@ -58,7 +59,7 @@ public static void tearDownAfterClass() {
     // null static members for gc
     reProcessor = null;
     _parser = null;
-    parameters = null;
+    config = null;
     factory = null;
   }
 

solr/modules/langid/src/test/org/apache/solr/update/processor/OpenNLPLangDetectUpdateProcessorFactoryTest.java

@@ -20,6 +20,7 @@
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.request.SolrQueryRequest;
 import org.junit.Test;
 
@@ -41,7 +42,7 @@ protected OpenNLPLangDetectUpdateProcessor createLangIdProcessor(ModifiableSolrP
     }
     SolrQueryRequest req = _parser.buildRequestFrom(h.getCore(), new ModifiableSolrParams(), null);
     OpenNLPLangDetectUpdateProcessorFactory factory = new OpenNLPLangDetectUpdateProcessorFactory();
-    factory.init(parameters.toNamedList());
+    factory.init(new SimpleOrderedMap<>(parameters));
     factory.inform(h.getCore());
     return (OpenNLPLangDetectUpdateProcessor) factory.getInstance(req, resp, null);
   }

solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java

@@ -69,18 +69,17 @@ public String get(String param, String def) {
 
   @Override
   public void writeMap(EntryWriter ew) throws IOException {
-    // TODO don't call toNamedList; more efficiently implement here
-    // note: multiple values, if present, are a String[] under 1 key
-    toNamedList()
-        .forEach(
-            (k, v) -> {
-              if (v == null || "".equals(v)) return;
-              try {
-                ew.put(k, v);
-              } catch (IOException e) {
-                throw new RuntimeException("Error serializing", e);
-              }
-            });
+    for (Entry<String, String[]> entry : this) {
+      String[] value = entry.getValue();
+      // if only one value, don't wrap in an array
+      if (value.length == 1) {
+        assert value[0] != null;
+        ew.put(entry.getKey(), value[0]);
+      } else if (value.length > 1) {
+        // values shouldn't be null; not bothering to assert it
+        ew.put(entry.getKey(), value);
+      }
+    }
   }
 
   /** Returns an Iterator of {@code Map.Entry} providing a multi-map view. Treat it as read-only. */
@@ -434,7 +433,10 @@ public static SolrParams wrapAppended(SolrParams params, SolrParams defaults) {
   /**
    * Convert this to a NamedList of unique keys with either String or String[] values depending on
    * how many values there are for the parameter.
+   *
+   * @deprecated see {@link SimpleOrderedMap#SimpleOrderedMap(MapWriter)}
    */
+  @Deprecated
   public NamedList<Object> toNamedList() {
     final SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
 

solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java

@@ -16,6 +16,7 @@
  */
 package org.apache.solr.common.util;
 
+import java.io.IOException;
 import java.util.AbstractMap;
 import java.util.AbstractSet;
 import java.util.ArrayList;
@@ -24,6 +25,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.params.SolrParams;
 
 /**
  * <code>SimpleOrderedMap</code> is a {@link NamedList} where access by key is more important than
@@ -67,7 +70,22 @@ public SimpleOrderedMap(Map.Entry<String, T>[] nameValuePairs) {
     super(nameValuePairs);
   }
 
-  // TODO override asShallowMap in Solr 10
+  /** Can convert a {@link SolrParams} and other things. */
+  public SimpleOrderedMap(MapWriter mapWriter) {
+    try {
+      mapWriter.writeMap(
+          new EntryWriter() {
+            @SuppressWarnings("unchecked")
+            @Override
+            public EntryWriter put(CharSequence k, Object v) throws IOException {
+              SimpleOrderedMap.this.add(k.toString(), (T) v);
+              return this;
+            }
+          });
+    } catch (IOException e) {
+      throw new RuntimeException(e); // impossible?
+    }
+  }
 
   @Override
   public SimpleOrderedMap<T> clone() {