isaric commented on code in PR #4465:
URL: https://github.com/apache/solr/pull/4465#discussion_r3406020774
##########
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java:
##########
@@ -99,7 +100,7 @@ public TikaServerExtractionBackend(
this.maxCharsLimit = maxCharsLimit;
if (initArgs != null) {
- initArgs.toMap(this.initArgsMap);
+ initArgsMap.putAll(new SimpleOrderedMap<>(initArgs));
Review Comment:
Applied — `initArgs.forEach(initArgsMap::put);` directly. SOM import removed.
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java:
##########
@@ -42,7 +43,7 @@ public ZkNodeProps(Map<String, Object> propMap) {
}
public ZkNodeProps(MapWriter mw) {
- propMap = mw.toMap(new HashMap<>());
+ propMap = new SimpleOrderedMap<>(mw);
Review Comment:
Switched to `Utils.convertToMap(mw, new HashMap<>())`. SOM import removed.
##########
solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java:
##########
@@ -428,11 +427,6 @@ public boolean writeKnownType(Object val) throws
IOException {
writeMapEntry((Map.Entry) val);
return true;
}
- if (val instanceof MapSerializable) {
Review Comment:
Good catch — reverted in `4aae25c4a56`. The `MapSerializable` branch still
needs to handle classes that implement only `MapSerializable` (not `MapWriter`)
until the interface itself is deleted, so I will move this removal into Part 4.
##########
solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java:
##########
@@ -89,9 +87,6 @@ default void writeVal(String name, Object val, boolean raw)
throws IOException {
writeMap(name, (MapWriter) val);
} else if (val instanceof ReflectWritable) {
writeVal(name, Utils.getReflectWriter(val));
- } else if (val instanceof MapSerializable) {
Review Comment:
Same — reverted in `4aae25c4a56`; moving the removal to Part 4 alongside the
`MapSerializable` interface deletion.
##########
solr/solrj/src/java/org/apache/solr/common/util/Utils.java:
##########
@@ -178,7 +178,7 @@ public MapWriter.EntryWriter put(CharSequence k, Object v) {
private static Object makeDeepCopy(Object v, int maxDepth, boolean mutable,
boolean sorted) {
if (v instanceof MapWriter && maxDepth > 1) {
- v = ((MapWriter) v).toMap(new LinkedHashMap<>());
+ v = new SimpleOrderedMap<>((MapWriter) v);
Review Comment:
Switched to `convertToMap((MapWriter) v, new LinkedHashMap<>())`. This
restores the pre-Part-3 behavior exactly — the removed `MapWriter.toMap`
default also called `Utils.convertToMap` under the hood — while avoiding SOM in
this non-request path.
##########
solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java:
##########
@@ -76,7 +76,7 @@ default List<Object> toList(List<Object> l) {
new ItemWriter() {
@Override
public ItemWriter add(Object o) throws IOException {
- if (o instanceof MapWriter) o = ((MapWriter) o).toMap(new
LinkedHashMap<>());
+ if (o instanceof MapWriter) o = new
SimpleOrderedMap<>((MapWriter) o);
Review Comment:
Deferring per your suggestion — happy to add a
`SimpleOrderedMap.from(MapWriter)` helper later as a small follow-up if you
want; not blocking this PR.
##########
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/expr/Explanation.java:
##########
@@ -139,35 +138,14 @@ public void addHelper(Explanation helper) {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> map) {
- if (null != expressionNodeId) {
- map.put("expressionNodeId", expressionNodeId);
- }
- if (null != expressionType) {
- map.put("expressionType", expressionType);
- }
- if (null != functionName) {
- map.put("functionName", functionName);
- }
- if (null != implementingClass) {
- map.put("implementingClass", implementingClass);
- }
- if (null != expression) {
- map.put("expression", expression);
- }
- if (null != note) {
- map.put("note", note);
- }
-
- if (null != helpers && 0 != helpers.size()) {
- List<Map<String, Object>> helperMaps = new ArrayList<>();
- for (Explanation helper : helpers) {
- helperMaps.add(helper.toMap(new LinkedHashMap<>()));
- }
- map.put("helpers", helperMaps);
- }
-
- return map;
+ public void writeMap(EntryWriter ew) throws IOException {
Review Comment:
Thanks!
##########
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExplanation.java:
##########
@@ -58,17 +57,12 @@ public void addChild(Explanation child) {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> map) {
- map = super.toMap(map);
-
- if (null != children && 0 != children.size()) {
- List<Map<String, Object>> childrenMaps = new ArrayList<>();
+ public void writeMap(EntryWriter ew) throws IOException {
Review Comment:
Good catch — done. `writeMap` is now `super.writeMap(ew);
ew.putIfNotNull("children", children);` matching the `helpers` pattern in
`Explanation`.
--
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]
