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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/OverseerOperationAPI.java:
##########
@@ -59,7 +59,7 @@ public OverseerOperationAPI(CoreAdminHandler 
coreAdminHandler) {
   @Command(name = OVERSEER_OP_CMD)
   public void joinOverseerLeaderElection(PayloadObj<OverseerOperationPayload> 
payload)
       throws Exception {
-    final Map<String, Object> v1Params = payload.get().toMap(new HashMap<>());
+    final Map<String, Object> v1Params = new SimpleOrderedMap<>(payload.get());

Review Comment:
   Lets standardize on use of `Utils.convertToMap` in these places (classes 
with suffix API... on the request dispatch code-path)



##########
solr/core/src/java/org/apache/solr/handler/admin/BaseHandlerApiSupport.java:
##########
@@ -195,26 +196,32 @@ public Map<String, Object> toMap(Map<String, Object> 
suppliedMap) {
                       : map.get(key);
               if (o == null) o = pathValues.get(key);
               if (o == null && useRequestParams) o = origParams.getParams(key);
+              if (o == null) {
+                ew.put(param, null);
+                continue;
+              }

Review Comment:
   can we skip putting 'null'?  Curious, what test or issue brought this change 
about?



##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -204,7 +205,7 @@ private void handleGET() {
             Map<String, Object> m = new LinkedHashMap<>();
             m.put(ZNODEVER, params.getZnodeVersion());
             if (p != null) {
-              m.put(RequestParams.NAME, Map.of(parts.get(2), p.toMap(new 
LinkedHashMap<>())));
+              m.put(RequestParams.NAME, Map.of(parts.get(2), new 
SimpleOrderedMap<>(p)));

Review Comment:
   SOM makes sense as it's a response structure here



##########
solr/core/src/java/org/apache/solr/handler/admin/IndexSizeEstimator.java:
##########
@@ -277,12 +278,10 @@ private void convert(Map<String, Object> result) {
     for (Map.Entry<String, Object> entry : result.entrySet()) {
       Object value = entry.getValue();
       if (value instanceof ItemPriorityQueue queue) {
-        Map<String, Object> map = new LinkedHashMap<>();
-        queue.toMap(map);
+        Map<String, Object> map = new SimpleOrderedMap<>(queue);
         entry.setValue(map);
       } else if (value instanceof MapWriterSummaryStatistics stats) {
-        Map<String, Object> map = new LinkedHashMap<>();
-        stats.toMap(map);
+        Map<String, Object> map = new SimpleOrderedMap<>(stats);

Review Comment:
   SOM is good here; this is a response-structure



##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -283,16 +284,16 @@ private void handleGET() {
               Map pluginNameVsPluginInfo = (Map) val.get(parts.get(1));
               if (pluginNameVsPluginInfo != null) {
                 Object o =
-                    pluginNameVsPluginInfo instanceof MapSerializable
+                    pluginNameVsPluginInfo instanceof MapWriter
                         ? pluginNameVsPluginInfo
                         : pluginNameVsPluginInfo.get(componentName);
                 Map<String, Object> pluginInfo =
-                    o instanceof MapSerializable
-                        ? ((MapSerializable) o).toMap(new LinkedHashMap<>())
+                    o instanceof MapWriter
+                        ? new SimpleOrderedMap<>((MapWriter) o)

Review Comment:
   SOM is okay for response



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