isaric commented on code in PR #4464:
URL: https://github.com/apache/solr/pull/4464#discussion_r3381613026
##########
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)));
+ ew.put(tag, writers);
} else {
- result.put(tag, infos.get(0));
+ ew.put(tag, new SimpleOrderedMap<>(m ->
infos.getFirst().writeMap(m)));
Review Comment:
Yes — restored. `requestParsers` is now `Map.of("multipartUploadLimitKB",
..., "formUploadLimitKB", ...)` without intermediate copies.
##########
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)));
+ ew.put(tag, writers);
} else {
- result.put(tag, infos.get(0));
+ ew.put(tag, new SimpleOrderedMap<>(m ->
infos.getFirst().writeMap(m)));
}
}
}
- addCacheConfig(
- m,
- filterCacheConfig,
- queryResultCacheConfig,
- documentCacheConfig,
- fieldValueCacheConfig,
- featureVectorCacheConfig);
- m = new LinkedHashMap<>();
- result.put("requestDispatcher", m);
- if (httpCachingConfig != null) m.put("httpCaching", httpCachingConfig);
- m.put(
- "requestParsers",
- Map.of(
- "multipartUploadLimitKB",
- multipartUploadLimitKB,
- "formUploadLimitKB",
- formUploadLimitKB));
- if (indexConfig != null) result.put("indexConfig", indexConfig);
-
- // TODO there is more to add
-
- return result;
+ ew.put(
+ "requestDispatcher",
+ new SimpleOrderedMap<>(
Review Comment:
Understood — applied across this PR. `MapWriter` instances flow into
`EntryWriter` without an intermediate `SimpleOrderedMap` / `NamedList` / `Map`
copy wherever the value isn't mutated afterwards. Where a real mutable `Map`
was needed (e.g., `SolrConfigHandler`'s response shaping that then calls
`computeIfAbsent` / `put` / `setValue`), the SOM stays.
##########
solr/core/src/java/org/apache/solr/schema/IndexSchema.java:
##########
@@ -1699,7 +1698,7 @@ public Map<String, Object> toMap(Map<String, Object> map)
{
SchemaProps.Handler::getNameLower,
SchemaProps.Handler::getRealName));
public Map<String, Object> getNamedPropertyValues(String name, SolrParams
params) {
- return new SchemaProps(name, params, this).toMap(new LinkedHashMap<>());
+ return new SimpleOrderedMap<>(new SchemaProps(name, params, this));
Review Comment:
Done — `getNamedPropertyValues` now returns `Utils.convertToMap(new
SchemaProps(...), new LinkedHashMap<>())`.
##########
solr/core/src/java/org/apache/solr/search/CacheConfig.java:
##########
@@ -175,9 +177,8 @@ public SolrCache newInstance() {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> argsMap) {
- // TODO: Should not create new HashMap?
- return new HashMap<>(args);
+ public void writeMap(EntryWriter ew) throws IOException {
+ new NamedList<>(args).writeMap(ew);
Review Comment:
Done — `CacheConfig.writeMap` now iterates `args.entrySet()` directly. No
`NamedList` allocation.
##########
solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java:
##########
@@ -195,28 +195,19 @@ public SolrIndexConfig(ConfigNode cfg, SolrIndexConfig
def) {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> map) {
- map.put("useCompoundFile", useCompoundFile);
- map.put("maxBufferedDocs", maxBufferedDocs);
- map.put("ramBufferSizeMB", ramBufferSizeMB);
- map.put("ramPerThreadHardLimitMB", ramPerThreadHardLimitMB);
- map.put("maxCommitMergeWaitTime", maxCommitMergeWaitMillis);
- map.put("writeLockTimeout", writeLockTimeout);
- map.put("lockType", lockType);
- map.put("infoStreamEnabled", infoStream != InfoStream.NO_OUTPUT);
- if (mergeSchedulerInfo != null) {
- map.put("mergeScheduler", mergeSchedulerInfo);
- }
- if (metricsInfo != null) {
- map.put("metrics", metricsInfo);
- }
- if (mergePolicyFactoryInfo != null) {
- map.put("mergePolicyFactory", mergePolicyFactoryInfo);
- }
- if (mergedSegmentWarmerInfo != null) {
- map.put("mergedSegmentWarmer", mergedSegmentWarmerInfo);
- }
- return map;
+ public void writeMap(EntryWriter ew) throws IOException {
+ ew.put("useCompoundFile", useCompoundFile)
+ .put("maxBufferedDocs", maxBufferedDocs)
+ .put("ramBufferSizeMB", ramBufferSizeMB)
+ .put("ramPerThreadHardLimitMB", ramPerThreadHardLimitMB)
+ .put("maxCommitMergeWaitTime", maxCommitMergeWaitMillis)
+ .put("writeLockTimeout", writeLockTimeout)
+ .put("lockType", lockType)
+ .put("infoStreamEnabled", infoStream != InfoStream.NO_OUTPUT)
+ .putIfNotNull("mergeScheduler", mergeSchedulerInfo)
+ .putIfNotNull("metrics", metricsInfo)
+ .putIfNotNull("mergePolicyFactory", mergePolicyFactoryInfo)
+ .putIfNotNull("mergedSegmentWarmer", mergedSegmentWarmerInfo);
Review Comment:
Thanks!
##########
solr/core/src/java/org/apache/solr/core/PluginInfo.java:
##########
@@ -192,27 +194,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);
+ }
+ if (children == null || children.isEmpty()) {
+ return;
+ }
+
+ for (PluginInfo child : children) {
+ child.writeMap(ew);
}
Review Comment:
Children grouping is preserved — the implementation collects them into a
`childrenGrouped` `LinkedHashMap` keyed by `child.name`, aggregating duplicates
into a `List`, then writes each entry. `initArgs` are written via
`initArgs.asMap(3).forEach(...)` which preserves the multi-value flattening
semantics. (Compile errors from the earlier `Map.Entry<String, Object>` and
`instanceof List<Object> list` patterns were fixed in the fixup commit —
`NamedList.asMap` returns a raw `Map`, and parameterized `instanceof` on
`Object` is rejected by javac.)
--
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]
