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


##########
solr/core/src/java/org/apache/solr/core/RequestParams.java:
##########
@@ -129,7 +130,9 @@ public RequestParams setParams(String name, ParamSet 
paramSet) {
     Map p = (Map) deepCopy.get(NAME);
     if (p == null) deepCopy.put(NAME, p = new LinkedHashMap<>());
     if (paramSet == null) p.remove(name);
-    else p.put(name, paramSet.toMap(new LinkedHashMap<>()));
+    else {
+      p.put(name, Utils.convertToMap(paramSet, new HashMap<>()));

Review Comment:
   lets continue to use LinkedHashMap here



##########
solr/core/src/java/org/apache/solr/core/PluginInfo.java:
##########
@@ -193,26 +195,32 @@ 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);
-        }
+  public void writeMap(EntryWriter ew) throws IOException {
+    new NamedList<>(attributes).writeMap(ew);

Review Comment:
   let's not create a new data structure just to write out our data structures. 
 That's the overall theme of MapWritable -- don't do this.
   
   The type here is simple so no multi-value concern.  
   
   Just need to call `attributes.forEach(ew::putNoEx);` here I think



##########
solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java:
##########
@@ -269,7 +270,7 @@ private static FileStoreEntryMetadata 
convertToResponse(FileStore.FileDetails de
     entryMetadata.size = size;
     entryMetadata.timestamp = timestamp;
     if (details.getMetaData() != null) {
-      details.getMetaData().toMap(entryMetadata.unknownProperties());
+      entryMetadata.unknownProperties().putAll(new 
SimpleOrderedMap<>(details.getMetaData()));

Review Comment:
   why did this change not use `details.getMetadata()` as before?  After all, 
we just checked it for not null so... 



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