dsmiley commented on code in PR #4101:
URL: https://github.com/apache/solr/pull/4101#discussion_r2801358604


##########
solr/test-framework/src/java/org/apache/solr/util/TestHarness.java:
##########
@@ -400,42 +399,41 @@ public class LocalRequestFactory {
     public LocalRequestFactory() {}
 
     /**
-     * Creates a LocalSolrQueryRequest based on variable args; for historical 
reasons, this method
+     * Creates a SolrQueryRequestBase based on variable args; for historical 
reasons, this method
      * has some peculiar behavior:
      *
      * <ul>
      *   <li>If there is a single arg, then it is treated as the "q" param, 
and the
-     *       LocalSolrQueryRequest consists of that query string along with 
"qt", "start", and
-     *       "rows" params (based on the qtype, start, and limit properties of 
this factory) along
-     *       with any other default "args" set on this factory.
+     *       SolrQueryRequestBase consists of that query string along with 
"qt", "start", and "rows"
+     *       params (based on the qtype, start, and limit properties of this 
factory) along with any
+     *       other default "args" set on this factory.
      *   <li>If there are multiple args, then there must be an even number of 
them, and each pair of
-     *       args is used as a key=value param in the LocalSolrQueryRequest. 
<b>NOTE: In this usage,
+     *       args is used as a key=value param in the SolrQueryRequestBase. 
<b>NOTE: In this usage,
      *       the "qtype", "start", "limit", and "args" properties of this 
factory are ignored.</b>
      * </ul>
      *
      * TODO: this isn't really safe in the presence of core reloads! Perhaps 
the best we could do is
      * increment the core reference count and decrement it in the request 
close() method?
      */
     @SuppressWarnings({"unchecked"})
-    public LocalSolrQueryRequest makeRequest(String... q) {
+    public SolrQueryRequestBase makeRequest(String... q) {
+      args.computeIfAbsent("wt", k -> "xml");
       if (q.length == 1) {
-        args.computeIfAbsent("wt", k -> "xml");
-        return new LocalSolrQueryRequest(
-            TestHarness.this.getCore(), q[0], qtype, start, limit, args);
+        Map<String, String[]> map = new HashMap<>();
+        for (Map.Entry<String, String> e : args.entrySet()) {
+          map.put(e.getKey(), new String[] {e.getValue()});
+        }
+        if (q[0] != null) map.put(CommonParams.Q, new String[] {q[0]});
+        if (qtype != null) map.put(CommonParams.QT, new String[] {qtype});
+        map.put(CommonParams.START, new String[] {Integer.toString(start)});
+        map.put(CommonParams.ROWS, new String[] {Integer.toString(limit)});
+        return new SolrQueryRequestBase(TestHarness.this.getCore(), new 
MultiMapSolrParams(map));

Review Comment:
   ugh; no.  You can probably simply tell Claude to use ModifiableSolrParams 
without creating a new HashMap, and it can probably simplify it a lot.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to