Copilot commented on code in PR #4465:
URL: https://github.com/apache/solr/pull/4465#discussion_r3298389054
##########
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:
This changes the serialized structure of `PluginInfo` children: previously
children were grouped under `child.name` (and multiple children of the same
name were accumulated into a list). The new implementation flattens each child
into the parent output, which can overwrite parent keys and loses the
`child.name` grouping entirely. Reintroduce the prior semantics by emitting
each child under its `child.name` key (accumulating into a `List` when needed),
e.g., materialize `child` to a map and then `ew.put(child.name, ...)` with
list-merging when the key already exists.
##########
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:
This changes behavior by skipping parameters with `null` values entirely.
Previously the `toMap(...)` implementation would insert the key with a `null`
value, which can be semantically different for downstream consumers that
distinguish between \"missing\" vs \"present-but-null\". If callers rely on
prior semantics, remove the `continue` and explicitly emit the key with `null`
(or otherwise preserve the earlier behavior).
##########
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:
This changes behavior by skipping parameters with `null` values entirely.
Previously the `toMap(...)` implementation would insert the key with a `null`
value, which can be semantically different for downstream consumers that
distinguish between \"missing\" vs \"present-but-null\". If callers rely on
prior semantics, remove the `continue` and explicitly emit the key with `null`
(or otherwise preserve the earlier behavior).
##########
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 behavior used `getMapWithVersion(data, znodeVersion)`, which
may have had conditional logic around emitting the znode version (e.g.,
omitting sentinel values). This implementation always emits `ZNODEVER` even
when the version is unset/sentinel. To preserve prior behavior, mirror
`getMapWithVersion` logic here (e.g., only write `ZNODEVER` when it’s valid) or
inline that method’s conditional rules into `writeMap`.
--
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]
