[GitHub] [solr] madrob commented on a diff in pull request #875: SOLR-9359: Make Config API work for warming queries

[GitBox <gi...@apache.org>]

madrob commented on code in PR #875:
URL: https://github.com/apache/solr/pull/875#discussion_r885819640


##########
solr/core/src/java/org/apache/solr/core/QuerySenderListener.java:
##########
@@ -102,4 +103,48 @@ public void close() {}
     }
     log.info("QuerySenderListener done.");
   }
+
+  private ArrayList<NamedList<Object>> queries2list(ArrayList<Object> queries) {
+
+    ArrayList<NamedList<Object>> allLists = new ArrayList<NamedList<Object>>();
+
+    for (Object o : queries) {
+      switch (o.getClass().getSimpleName()) {
+
+          // XML takes this path
+        case "ArrayList":

Review Comment:
   this has the potential to be a little bit trappy, because there are several classes called `ArrayList` even in the idk, for example java.util.ArrayList (the one we are all familiar with) but also calling `Arrays.asList` returns a `java.util.Arrays.ArrayList` which is a different class but will have the same name.



##########
solr/core/src/java/org/apache/solr/core/QuerySenderListener.java:
##########
@@ -102,4 +103,48 @@ public void close() {}
     }
     log.info("QuerySenderListener done.");
   }
+
+  private ArrayList<NamedList<Object>> queries2list(ArrayList<Object> queries) {

Review Comment:
   I'm somewhat confused about the general structure of this method. Trying to read what's going on before vs after, it seems like the behavior for json would remain unchanged but we're fixing a bug specific to xml?



##########
solr/core/src/java/org/apache/solr/core/QuerySenderListener.java:
##########
@@ -46,7 +46,8 @@ public void newSearcher(SolrIndexSearcher newSearcher, SolrIndexSearcher current
     final SolrIndexSearcher searcher = newSearcher;
     log.debug("QuerySenderListener sending requests to {}", newSearcher);
     @SuppressWarnings("unchecked")
-    List<NamedList<Object>> allLists = (List<NamedList<Object>>) getArgs().get("queries");
+    ArrayList<NamedList<Object>> allLists =

Review Comment:
   nit: I think this can still be a `List` declaration, I don't think it needed to change?



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org