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


##########
solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java:
##########
@@ -360,71 +353,160 @@ public void onReconnect() {
   @SuppressWarnings({"unchecked"})
   public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) 
throws Exception {
     final SolrParams params = req.getParams();
-    Map<String, String> map = Map.of(WT, "raw", OMIT_HEADER, "true");
+
+    // Force JSON response and omit header for cleaner output
+    Map<String, String> map = Map.of(WT, "json", OMIT_HEADER, "true");
     req.setParams(SolrParams.wrapDefaults(new MapSolrParams(map), params));
+
+    // Ensure paging support is initialized
+    ensurePagingSupportInitialized();
+
+    // Validate parameters
+    validateParameters(params);
+
+    // Determine request type and handle accordingly
+    boolean isGraphView = "graph".equals(params.get("view"));
+    ZkBasePrinter printer =
+        isGraphView ? handleGraphViewRequest(params) : 
handlePathViewRequest(params);
+
+    try {
+      printer.print();
+    } finally {
+      printer.close();
+    }
+
+    addJsonToResponse(printer.getJsonString(), rsp);
+  }
+
+  /** Ensures the paging support is initialized (thread-safe lazy 
initialization). */
+  private void ensurePagingSupportInitialized() {
     synchronized (this) {
       if (pagingSupport == null) {
         pagingSupport = new PagedCollectionSupport();
         ZkController zkController = cores.getZkController();
         if (zkController != null) {
-          // get notified when the ZK session expires (so we can clear the 
cached collections and
-          // rebuild)
+          // Get notified when the ZK session expires (so we can clear cached 
collections)
           zkController.addOnReconnectListener(pagingSupport);
         }
       }
     }
+  }
 
-    String path = params.get(PATH);
-
+  /**
+   * Validates incoming parameters.
+   *
+   * @param params Request parameters to validate
+   * @throws SolrException if validation fails
+   */
+  private void validateParameters(SolrParams params) {
     if (params.get("addr") != null) {
       throw new SolrException(ErrorCode.BAD_REQUEST, "Illegal parameter 
\"addr\"");
     }
+  }
 
-    String detailS = params.get(PARAM_DETAIL);
-    boolean detail = detailS != null && detailS.equals("true");
+  /**
+   * Handles the graph view request with paginated collections.
+   *
+   * @param params Request parameters including pagination settings
+   * @return JSON string representing paginated collection data
+   */
+  private ZkBasePrinter handleGraphViewRequest(SolrParams params) {
+    // Extract pagination parameters
+    int start = params.getInt("start", 0);
+    int rows = params.getInt("rows", -1);
 
-    String dumpS = params.get("dump");
-    boolean dump = dumpS != null && dumpS.equals("true");
+    // Extract filter parameters
+    FilterType filterType = extractFilterType(params);
+    String filter = extractFilter(params, filterType);
+
+    // Extract display options (applicable to graph view)
+    boolean detail = params.getBool(PARAM_DETAIL, false);
+    boolean dump = params.getBool("dump", false);
+
+    // Create printer for paginated collections
+    return new ZkGraphPrinter(
+        cores.getZkController(),
+        new PageOfCollections(start, rows, filterType, filter),
+        pagingSupport,
+        detail,
+        dump);
+  }
 
-    int start = params.getInt("start", 0); // Note start ignored if rows not 
specified
-    int rows = params.getInt("rows", -1);
+  /**
+   * Handles the path view request for a specific ZooKeeper path.
+   *
+   * @param params Request parameters including the path to display
+   * @return JSON string representing the ZooKeeper path data
+   */
+  private ZkBasePrinter handlePathViewRequest(SolrParams params) {
+    // Extract path parameter
+    String path = params.get(PATH);
+
+    // Extract display options
+    boolean detail = params.getBool(PARAM_DETAIL, false);
+    boolean dump = params.getBool("dump", false);
 
+    // Create printer for specific path
+    return new ZkPathPrinter(cores.getZkController(), path, detail, dump);
+  }
+
+  /**
+   * Extracts and normalizes the filter type from request parameters.
+   *
+   * @param params Request parameters
+   * @return The filter type (defaults to FilterType.none if not specified)
+   */
+  private FilterType extractFilterType(SolrParams params) {
     String filterType = params.get("filterType");
     if (filterType != null) {
       filterType = filterType.trim().toLowerCase(Locale.ROOT);
-      if (filterType.length() == 0) filterType = null;
+      if (filterType.length() == 0) {
+        return FilterType.none;
+      }
+      return FilterType.valueOf(filterType);
+    }
+    return FilterType.none;
+  }
+
+  /**
+   * Extracts and normalizes the filter value from request parameters.
+   *
+   * @param params Request parameters
+   * @param filterType The filter type being used
+   * @return The filter string, or null if not applicable
+   */
+  private String extractFilter(SolrParams params, FilterType filterType) {
+    if (filterType == FilterType.none) {
+      return null;
     }
-    FilterType type = (filterType != null) ? FilterType.valueOf(filterType) : 
FilterType.none;
 
-    String filter = (type != FilterType.none) ? params.get("filter") : null;
+    String filter = params.get("filter");
     if (filter != null) {
       filter = filter.trim();
-      if (filter.length() == 0) filter = null;
+      if (filter.length() > 0) {
+        return filter;
+      }
     }
+    return null;
+  }
 
-    ZKPrinter printer = new ZKPrinter(cores.getZkController());
-    printer.detail = detail;
-    printer.dump = dump;
-    boolean isGraphView = "graph".equals(params.get("view"));
-    // There is no znode /clusterstate.json (removed in Solr 9), but we do as 
if there's one and
-    // return collection listing. Need to change services.js if cleaning up 
here, collection list is
-    // used from Admin UI Cloud - Graph
-    boolean paginateCollections = (isGraphView && 
"/clusterstate.json".equals(path));
-    printer.page = paginateCollections ? new PageOfCollections(start, rows, 
type, filter) : null;
-    printer.pagingSupport = pagingSupport;
+  /**
+   * Converts JSON string to structured response objects and adds to 
SolrQueryResponse.
+   *
+   * @param jsonString The JSON string to parse
+   * @param rsp The response object to populate
+   */
+  private void addJsonToResponse(String jsonString, SolrQueryResponse rsp) {
+    // Parse the JSON we built and return as structured data
+    // This allows any response writer (json, xml, etc.) to serialize it 
properly
+    // The JSON is always a Map since both printPath() and 
printPaginatedCollections()
+    // start with json.startObject() and end with json.endObject()
+    @SuppressWarnings("unchecked")
+    Map<String, Object> jsonMap = (Map<String, Object>) 
Utils.fromJSONString(jsonString);
 
-    try {
-      if (paginateCollections) {
-        // List collections and allow pagination, but no specific znode info 
like when looking at a
-        // normal ZK path
-        printer.printPaginatedCollections();
-      } else {
-        printer.print(path);
-      }
-    } finally {
-      printer.close();
+    for (Map.Entry<String, Object> entry : jsonMap.entrySet()) {

Review Comment:
   Copilot is so right here.  "Raw" was being used to avoid this very thing.   
Let's not serialize & decode needlessly!
   
   Another way to avoid this is a potentially large refactor of ZkBasePrinter 
to instead either produce plain Maps & arrays & such that Solr will serialize 
(to either Json or Xml even).  I think you/LLM will find that easiest to 
understand.  Or embrace Solr's `MapWriter` that allows very efficient 
just-in-time streaming.  See https://issues.apache.org/jira/browse/SOLR-17582 
#2916 for an example of that.



-- 
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