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


##########
solr/core/src/java/org/apache/solr/core/PluginInfo.java:
##########
@@ -192,27 +194,35 @@ public PluginInfo getChild(String type) {
   }
 
   @Override
-  @SuppressWarnings({"unchecked", "rawtypes"})
-  public Map<String, Object> toMap(Map<String, Object> map) {
-    map.putAll(attributes);
-    Map m = map;
-    if (initArgs != null) m.putAll(initArgs.asMap(3));
-    if (children != null) {
-      for (PluginInfo child : children) {
-        Object old = m.get(child.name);
-        if (old == null) {
-          m.put(child.name, child.toMap(new LinkedHashMap<>()));
-        } else if (old instanceof List list) {
-          list.add(child.toMap(new LinkedHashMap<>()));
-        } else {
-          ArrayList l = new ArrayList();
-          l.add(old);
-          l.add(child.toMap(new LinkedHashMap<>()));
-          m.put(child.name, l);
-        }
+  @SuppressWarnings("unchecked")
+  public void writeMap(EntryWriter ew) throws IOException {
+    new NamedList<>(attributes).writeMap(ew);
+    if (initArgs != null) {
+      for (Map.Entry<String, Object> entry : initArgs.asMap(3).entrySet()) {
+        ew.put(entry.getKey(), entry.getValue());
+      }
+    }
+    if (children == null || children.isEmpty()) {
+      return;
+    }
+
+    Map<String, Object> childrenGrouped = new LinkedHashMap<>();
+    for (PluginInfo child : children) {
+      Object old = childrenGrouped.get(child.name);
+      if (old == null) {
+        childrenGrouped.put(child.name, child);
+      } else if (old instanceof List) {
+        ((List<Object>) old).add(child);

Review Comment:
   Applied. Note: had to use raw `List list` rather than the parameterized 
`List<Object> list` — javac rejects the parameterized pattern as an unsafe cast 
from `Object`. Same shape as you suggested otherwise.



##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -952,20 +947,35 @@ public int getFormUploadLimitKB() {
   }
 
   @Override
-  public Map<String, Object> toMap(Map<String, Object> result) {
-    if (znodeVersion > -1) result.put(ZNODEVER, znodeVersion);
-    if (luceneMatchVersion != null)
-      result.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM, 
luceneMatchVersion.toString());
-    result.put("updateHandler", getUpdateHandlerInfo());
-    Map<String, Object> m = new LinkedHashMap<>();
-    result.put("query", m);
-    m.put("useFilterForSortedQuery", useFilterForSortedQuery);
-    m.put("queryResultWindowSize", queryResultWindowSize);
-    m.put("queryResultMaxDocsCached", queryResultMaxDocsCached);
-    m.put("enableLazyFieldLoading", enableLazyFieldLoading);
-    m.put("maxBooleanClauses", booleanQueryMaxClauseCount);
-    m.put(MIN_PREFIX_QUERY_TERM_LENGTH, prefixQueryMinPrefixLength);
+  public void writeMap(EntryWriter ew) throws IOException {
+    if (znodeVersion > -1) {
+      ew.put(ZNODEVER, znodeVersion);
+    }
+    if (luceneMatchVersion != null) {
+      ew.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM, 
luceneMatchVersion.toString());
+    }
 
+    ew.put("updateHandler", new SimpleOrderedMap<>(getUpdateHandlerInfo()));

Review Comment:
   Done — `getUpdateHandlerInfo()` is now passed directly as a `MapWriter` 
value, no SOM copy. Same for `indexConfig`, `httpCachingConfig`, the cache 
configs via `addCacheConfig`, and the `infos` / `infos.getFirst()` plugin 
branches.



##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -982,45 +992,47 @@ public Map<String, Object> toMap(Map<String, Object> 
result) {
             overlay.getNamedPlugins(plugin.tag).entrySet()) {
           items.put(e.getKey(), e.getValue());
         }
-        result.put(tag, items);
+        ew.put(
+            tag,
+            new SimpleOrderedMap<>(
+                m -> {
+                  new NamedList<>(items).writeMap(m);
+                }));

Review Comment:
   The double conversion is gone — the current block builds `items` as a 
`LinkedHashMap` (no intermediate `NamedList` or SOM) and writes it via a small 
`MapWriter` lambda. We need the explicit map because we merge two sources under 
the same name keys: `PluginInfo` entries from `infos` and `Map<String,Object>` 
entries from `overlay.getNamedPlugins(plugin.tag)`. Open to dropping the lambda 
and just doing `ew.put(tag, items)` directly if you'd prefer — the response 
writers handle a mixed-value `Map` the same way.



##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -982,45 +992,47 @@ public Map<String, Object> toMap(Map<String, Object> 
result) {
             overlay.getNamedPlugins(plugin.tag).entrySet()) {
           items.put(e.getKey(), e.getValue());
         }
-        result.put(tag, items);
+        ew.put(
+            tag,
+            new SimpleOrderedMap<>(
+                m -> {
+                  new NamedList<>(items).writeMap(m);
+                }));
       } else {
         if (plugin.options.contains(MULTI_OK)) {
-          ArrayList<MapSerializable> l = new ArrayList<>();
-          for (PluginInfo info : infos) l.add(info);
-          result.put(tag, l);
+          ArrayList<MapWriter> writers = new ArrayList<>();
+          infos.forEach(info -> writers.add(new SimpleOrderedMap<>(info)));

Review Comment:
   Dropped — both `infos` (multi-OK) and `infos.getFirst()` are now passed 
directly to `ew.put(tag, ...)`. No SOM copy on either branch.



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