isaric commented on code in PR #4465:
URL: https://github.com/apache/solr/pull/4465#discussion_r3398809609
##########
solr/core/src/java/org/apache/solr/core/RequestParams.java:
##########
@@ -112,8 +112,9 @@ public int getZnodeVersion() {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> map) {
- return getMapWithVersion(data, znodeVersion);
+ public void writeMap(EntryWriter ew) throws IOException {
+ ew.put(ConfigOverlay.ZNODEVER, znodeVersion);
+ data.forEach(ew::putNoEx);
}
Review Comment:
The previous `getMapWithVersion(data, znodeVersion)` did not have
conditional logic — it unconditionally put `ZNODEVER` then `putAll(data)`. The
new `writeMap` preserves that same unconditional behavior. No semantic change.
##########
solr/core/src/java/org/apache/solr/handler/admin/BaseHandlerApiSupport.java:
##########
@@ -185,7 +186,7 @@ public Iterator<String> getParameterNamesIterator() {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> suppliedMap) {
+ public void writeMap(EntryWriter ew) throws IOException {
for (Iterator<String> it = getParameterNamesIterator();
it.hasNext(); ) {
Review Comment:
The `if (o == null) continue;` is a latent-bug fix — the original `toMap`
would NPE on `o.getClass()` two lines down when `o == null` (all three lookups
`map.get(key)` / `pathValues.get(key)` / `origParams.getParams(key)` returned
null). Downstream V1 handlers already treat absent and null-valued params the
same way. Discussed and confirmed by @dsmiley on Part 1.
##########
solr/core/src/java/org/apache/solr/handler/admin/BaseHandlerApiSupport.java:
##########
@@ -195,26 +196,27 @@ public Map<String, Object> toMap(Map<String, Object>
suppliedMap) {
: map.get(key);
if (o == null) o = pathValues.get(key);
if (o == null && useRequestParams) o = origParams.getParams(key);
+ if (o == null) continue;
Review Comment:
Same as the comment above at line 190 — NPE-prevention, no semantic change
for downstream V1 handlers. Resolving.
##########
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` and accumulates
duplicates into a `List`, then writes each entry. This was addressed in Part 2
(now merged), so on the rebased branch this code is on `main`.
--
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]
