SolrParams.equals implementation

solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java

@@ -46,7 +46,6 @@
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.common.util.IOUtils;
-import org.apache.solr.common.util.NamedList;
 import org.apache.solr.crossdc.common.CrossDcConf;
 import org.apache.solr.crossdc.common.IQueueHandler;
 import org.apache.solr.crossdc.common.KafkaCrossDcConf;
@@ -259,7 +258,6 @@ boolean pollAndProcessRequests() {
         partitionWork.partitionQueue.add(workUnit);
         try {
           ModifiableSolrParams lastUpdateParams = null;
-          NamedList<?> lastUpdateParamsAsNamedList = null;
           for (ConsumerRecord<String, MirroredSolrRequest<?>> requestRecord : partitionRecords) {
             if (log.isTraceEnabled()) {
               log.trace(
@@ -291,8 +289,7 @@ boolean pollAndProcessRequests() {
             if (type == MirroredSolrRequest.Type.UPDATE
                 && (
                 // different params
-                (lastUpdateParams != null
-                        && !lastUpdateParams.toNamedList().equals(params.toNamedList()))
+                (lastUpdateParams != null && !lastUpdateParams.equals(params))
                     ||
                     // no collapsing
                     (collapseUpdates == CrossDcConf.CollapseUpdates.NONE)
@@ -310,7 +307,6 @@ boolean pollAndProcessRequests() {
                 sendBatch(updateReqBatch, type, lastRecord, workUnit);
               }
               updateReqBatch = null;
-              lastUpdateParamsAsNamedList = null;
               currentCollapsed = 0;
               workUnit = new PartitionManager.WorkUnit(partition);
               workUnit.nextOffset = PartitionManager.getOffsetForPartition(partitionRecords);
@@ -334,9 +330,7 @@ boolean pollAndProcessRequests() {
               }
               UpdateRequest update = (UpdateRequest) solrReq;
               MirroredSolrRequest.setParams(updateReqBatch, params);
-              if (lastUpdateParamsAsNamedList == null) {
-                lastUpdateParamsAsNamedList = lastUpdateParams.toNamedList();
-              }
+
               // merge
               List<SolrInputDocument> docs = update.getDocuments();
               if (docs != null) {

solr/cross-dc-manager/src/test/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumerTest.java

@@ -300,10 +300,7 @@ public void testHandleValidMirroredSolrRequest() {
                   return ((UpdateRequest) solrRequest)
                           .getDocuments()
                           .equals(validRequest.getDocuments())
-                      && solrRequest
-                          .getParams()
-                          .toNamedList()
-                          .equals(validRequest.getParams().toNamedList());
+                      && solrRequest.getParams().equals(validRequest.getParams());
                 }),
             eq(MirroredSolrRequest.Type.UPDATE),
             eq(record),
@@ -349,10 +346,7 @@ public void testHandleValidAdminRequest() throws Exception {
             argThat(
                 solrRequest -> {
                   // Check if the SolrRequest has the same content as the original validRequest
-                  return solrRequest
-                      .getParams()
-                      .toNamedList()
-                      .equals(create.getParams().toNamedList());
+                  return solrRequest.getParams().equals(create.getParams());
                 }),
             eq(MirroredSolrRequest.Type.ADMIN),
             eq(record1),
@@ -479,10 +473,7 @@ protected KafkaMirroringSink createKafkaMirroringSink(KafkaCrossDcConf conf) {
                   System.out.println(Utils.toJSONString(solrRequest));
                   // Check if the UpdateRequest has the same content as the original invalidRequest
                   return ((UpdateRequest) solrRequest).getDocuments() == null
-                      && solrRequest
-                          .getParams()
-                          .toNamedList()
-                          .equals(invalidRequest.getParams().toNamedList());
+                      && solrRequest.getParams().equals(invalidRequest.getParams());
                 }),
             any(),
             eq(record),

solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java

@@ -253,8 +253,7 @@ public void writeTo(OutputStream out) throws IOException {
                       Update upd = update;
                       while (upd != null) {
                         UpdateRequest req = upd.getRequest();
-                        SolrParams currentParams = new ModifiableSolrParams(req.getParams());
-                        if (!origParams.toNamedList().equals(currentParams.toNamedList())
+                        if (!origParams.equals(req.getParams())
                             || !Objects.equals(origTargetCollection, upd.getCollection())) {
                           // Request has different params or destination core/collection, return to
                           // queue

solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java

@@ -350,8 +350,7 @@ public OutStream(
     }
 
     boolean belongToThisStream(SolrRequest<?> solrRequest, String collection) {
-      ModifiableSolrParams solrParams = new ModifiableSolrParams(solrRequest.getParams());
-      return origParams.toNamedList().equals(solrParams.toNamedList())
+      return origParams.equals(solrRequest.getParams())
           && Objects.equals(origCollection, collection);
     }
 

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

@@ -24,6 +24,7 @@
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 import org.apache.solr.client.solrj.util.ClientUtils;
@@ -37,7 +38,7 @@
  * SolrParams is designed to hold parameters to Solr, often from the request coming into Solr. It's
  * basically a MultiMap of String keys to one or more String values. Neither keys nor values may be
  * null. Unlike a general Map/MultiMap, the size is unknown without iterating over each parameter
- * name.
+ * name, if you want to count the different values for a key separately.
  */
 public abstract class SolrParams
     implements Serializable, MapWriter, Iterable<Map.Entry<String, String[]>> {
@@ -526,4 +527,36 @@ public String toString() {
     }
     return sb.toString();
   }
+
+  /**
+   * A SolrParams is equal to another if they have the same keys and values. The order of keys does
+   * not matter.
+   */
+  @Override
+  public boolean equals(Object obj) {
+    if (obj == this) return true;
+    if (!(obj instanceof SolrParams b)) return false;
+
+    // iterating this params, see if other has the same values for each key
+    int count = 0;
+    for (Entry<String, String[]> thisEntry : this) {
+      String name = thisEntry.getKey();
+      if (!Arrays.equals(thisEntry.getValue(), b.getParams(name))) return false;
+      count++;
+    }
+    // does other params have the same number of keys?  It might have more but not less.
+    Iterator<String> bNames = b.getParameterNamesIterator();
+    while (bNames.hasNext()) {
+      bNames.next();
+      count--;
+      if (count < 0) return false;
+    }
+    assert count == 0;
+    return true;
+  }
+
+  @Override
+  public int hashCode() {
+    throw new UnsupportedOperationException();
+  }
 }

solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java

@@ -30,12 +30,11 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/** */
 public class SolrParamTest extends SolrTestCase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   public void testLocalParamRoundTripParsing() throws Exception {
-    final SolrParams in =
+    final ModifiableSolrParams in =
         params(
             "simple", "xxx",
             "blank", "",
@@ -49,30 +48,42 @@ public void testLocalParamRoundTripParsing() throws Exception {
             "dollar", "x$y",
             "multi", "x",
             "multi", "y y",
-            "v", "$ref");
+            "v", ""); // we'll replace this in a moment
+    SolrParams out = QueryParsing.getLocalParams(in.toLocalParamsString(), null);
+    assertEquals(in, out);
+
+    // test resolve dollar in 'v'
+    in.set("v", "$ref");
+    assertNotEquals(in, out);
     final String toStr = in.toLocalParamsString();
-    final SolrParams out = QueryParsing.getLocalParams(toStr, params("ref", "ref value"));
-
-    assertEquals("xxx", out.get("simple"));
-    assertEquals("", out.get("blank"));
-    assertEquals("x y z", out.get("space"));
-    assertEquals(" x", out.get("lead_space"));
-    assertEquals("x}y", out.get("curly"));
-    assertEquals("x'y", out.get("quote"));
-    assertEquals("'x y'", out.get("quoted"));
-    assertEquals("x\"y", out.get("d_quote"));
-    assertEquals("\"x y\"", out.get("d_quoted"));
-    assertEquals("x$y", out.get("dollar"));
-
-    assertArrayEquals(new String[] {"x", "y y"}, out.getParams("multi"));
+    out = QueryParsing.getLocalParams(toStr, params("ref", "ref value"));
+    assertEquals("ref value", out.get("v"));
+
     // first one should win...
     assertEquals("x", out.get("multi"));
 
-    assertEquals("ref value", out.get("v"));
-
     assertIterSize(toStr, 12, out);
   }
 
+  public void testTrivialEquals() {
+    assertEquals(params(), params());
+    assertFalse(params().equals(null));
+
+    var pFoo = params("foo", "val");
+    assertNotEquals(params(), pFoo);
+    assertNotEquals(pFoo, params()); // reflexive
+
+    assertNotEquals(pFoo, params("foo", "something-else")); // diff vals
+
+    // order
+    var pAB = params("a", "1", "b", "2");
+    var pBA = params("b", "2", "a", "1");
+    assertEquals("order", pAB, pBA);
+    assertEquals("order", pBA, pAB); // reflexive
+
+    // some other tests also test equality
+  }
+
   public void testParamIterators() {
 
     ModifiableSolrParams aaa = new ModifiableSolrParams();

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -1031,8 +1031,7 @@ public static String assertJQ(SolrQueryRequest req, double delta, String... test
       }
       return response;
     } finally {
-      // restore the params
-      if (params != null && params != req.getParams()) req.setParams(params);
+      req.setParams(params); // restore in case we changed it
     }
   }