Copilot commented on code in PR #4466:
URL: https://github.com/apache/solr/pull/4466#discussion_r3298393139
##########
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);
}
- return m;
}
Review Comment:
`PluginInfo.writeMap` currently serializes children by calling
`child.writeMap(ew)` on the parent writer, which flattens child fields into the
parent map and loses the previous semantics where children were nested under
`child.name` (and aggregated into lists when multiple children shared the same
name). To preserve the original structure, collect children grouped by
`child.name` and `ew.put(child.name, ...)` either a single child map or a list
of child maps for duplicates.
##########
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;
// make strings out of as many things as we can now to minimize
differences from
// the standard impls that pass through a
NamedList/SimpleOrderedMap...
Class<?> oClass = o.getClass();
if (oClass.isPrimitive()
|| Number.class.isAssignableFrom(oClass)
|| Character.class.isAssignableFrom(oClass)
|| Boolean.class.isAssignableFrom(oClass)) {
- suppliedMap.put(param, String.valueOf(o));
+ ew.put(param, String.valueOf(o));
} else if (List.class.isAssignableFrom(oClass)
+ && !((List) o).isEmpty()
&& ((List) o).get(0) instanceof String) {
Review Comment:
This changes behavior for empty `List<String>` values: previously an empty
list would have been treated as a string list and written as an empty
`String[]`, but now it falls through to the generic branch and is written as an
empty `List`. If the goal is to keep parity with the prior behavior / standard
param representations, handle the empty-list case by still emitting `new
String[0]` when `o` is an empty list that conceptually represents multi-valued
string params.
--
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]
