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


##########
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java:
##########
@@ -845,7 +844,7 @@ public void testSchemaDiffEndpoint() throws Exception {
         idFieldMapUpdated.indexOf("omitTermFreqAndPositions", 0), 
Boolean.FALSE);
 
     SolrParams solrParams = idFieldMapUpdated.toSolrParams();
-    Map<String, Object> mapParams = solrParams.toMap(new HashMap<>());
+    Map<String, Object> mapParams = new SimpleOrderedMap<>(solrParams);

Review Comment:
   `SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T>` 
(SimpleOrderedMap.java:46). Assignment to `Map<String, Object>` compiles and 
behaves correctly.



##########
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:
   `SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T>` 
(SimpleOrderedMap.java:46), so it is a `Map`. That said, this call site was 
switched to `Utils.convertToMap(payload.get(), new HashMap<>())` per @dsmiley's 
comment.



##########
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:
   `SimpleOrderedMap<T> extends NamedList<T> implements Map<String, T>` 
(SimpleOrderedMap.java:46), so it is a `Map`. @dsmiley also confirmed SOM is 
appropriate here as a response structure.



##########
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:
   `SimpleOrderedMap` implements `Map<String, T>`, so the type is compatible. 
This call site was further simplified — the `new SimpleOrderedMap<>(p)` wrapper 
is dropped and `p` (a `MapWriter`) is passed directly.



##########
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:
   `SimpleOrderedMap` implements `Map<String, T>`, so the type is compatible. 
The SOM stays here because the resulting map is mutated downstream 
(`computeIfAbsent`, `put`).



##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -320,10 +323,18 @@ private void handleGET() {
       }
     }
 
+    @SuppressWarnings({"unchecked"})
+    private Map<String, Object> getComponentInfo(
+        Map<String, Object> pluginInfo, String componentName) {
+      return pluginInfo.containsKey("class")
+          ? pluginInfo
+          : (Map<String, Object>) pluginInfo.getOrDefault(componentName, new 
HashMap<>());
+    }
+
     private Map<String, Object> getConfigDetails(String componentType, 
SolrQueryRequest req) {
       String componentName = componentType == null ? null : 
req.getParams().get("componentName");
       boolean showParams = req.getParams().getBool("expandParams", false);
-      Map<String, Object> map = this.req.getCore().getSolrConfig().toMap(new 
LinkedHashMap<>());
+      Map<String, Object> map = new 
SimpleOrderedMap<>(this.req.getCore().getSolrConfig());

Review Comment:
   `SimpleOrderedMap` implements `Map<String, T>`, so the type is compatible. 
That said, this line was reverted to `.toMap(new LinkedHashMap<>())` because 
`SolrConfig` does not implement `MapWriter` until Part 2 — the SOM constructor 
call was premature.



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