dsmiley commented on code in PR #4011:
URL: https://github.com/apache/solr/pull/4011#discussion_r2678317636
##########
solr/core/src/java/org/apache/solr/handler/admin/api/InstallShardData.java:
##########
@@ -129,6 +130,6 @@ public static ZkNodeProps createRemoteMessage(
}
messageTyped.validate();
- return new ZkNodeProps(messageTyped.toMap(new HashMap<>()));
+ return new ZkNodeProps((Map<String, Object>) new
SimpleOrderedMap<>(messageTyped));
Review Comment:
generally, we should avoid creating a SimpleOrderedMap. Use
Utils.convertToMap instead.
A SimpleOrderedMap is okay to add to a MapWritable entry though, or any
similar place for a response-writing use-case. A SimpleOrderedMap does not
hash its data, so has O(N) access. It's designed for writing out data that
will unlikely have its keys looked up. It's not a general replacement for a
Map.
##########
changelog/unreleased/SOLR-17600.yml:
##########
@@ -0,0 +1,7 @@
+title: SOLR 17600 - Replace MapSerializable with MapWriter
+type: changed
Review Comment:
refactorings are "other".
I'm not happy with the logchange plugin constraining the enum choices here
##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -981,45 +991,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:
not sure what I'm looking at here
--
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]
