dsmiley commented on code in PR #4011:
URL: https://github.com/apache/solr/pull/4011#discussion_r2674922297
##########
solr/core/src/java/org/apache/solr/core/RequestParams.java:
##########
@@ -130,7 +132,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, new SimpleOrderedMap<>(paramSet));
Review Comment:
This one's fine
##########
solr/core/src/java/org/apache/solr/core/PluginInfo.java:
##########
@@ -193,27 +195,19 @@ 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) {
+ new SimpleOrderedMap<>(initArgs).writeMap(ew);
+ }
Review Comment:
Let's not create a new Map just to ultimately write it!
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/InstallShardDataCmd.java:
##########
@@ -80,8 +80,7 @@ public void call(ClusterState state, ZkNodeProps message,
NamedList<Object> resu
final ModifiableSolrParams coreApiParams = new ModifiableSolrParams();
coreApiParams.set(
CoreAdminParams.ACTION,
CoreAdminParams.CoreAdminAction.INSTALLCOREDATA.toString());
- typedMessage.toMap(new HashMap<>()).forEach((k, v) -> coreApiParams.set(k,
v.toString()));
-
+ new SimpleOrderedMap<>(typedMessage).forEach((k, v) ->
coreApiParams.set(k, v.toString()));
Review Comment:
`typedMessage._forEachEntry` here instead
(avoiding senseless intermediate data structure creation).
##########
solr/core/src/java/org/apache/solr/api/NodeConfigClusterPluginsSource.java:
##########
@@ -79,7 +80,7 @@ private static Map<String, Object> readPlugins(final
NodeConfig cfg) {
pluginMap.put("class", p.className);
if (p.initArgs.size() > 0) {
- Map<String, Object> config = p.initArgs.toMap(new HashMap<>());
+ Map<String, Object> config = new SimpleOrderedMap<>(p.initArgs);
Review Comment:
Again, avoid NamedList/SimpleOrderedMap
How about `p.initArgs.asMap(0)`
--
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]
