dsmiley commented on code in PR #4011:
URL: https://github.com/apache/solr/pull/4011#discussion_r2701818301
##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -951,20 +946,35 @@ public int getFormUploadLimitKB() {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> result) {
- if (znodeVersion > -1) result.put(ZNODEVER, znodeVersion);
- if (luceneMatchVersion != null)
- result.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM,
luceneMatchVersion.toString());
- result.put("updateHandler", getUpdateHandlerInfo());
- Map<String, Object> m = new LinkedHashMap<>();
- result.put("query", m);
- m.put("useFilterForSortedQuery", useFilterForSortedQuery);
- m.put("queryResultWindowSize", queryResultWindowSize);
- m.put("queryResultMaxDocsCached", queryResultMaxDocsCached);
- m.put("enableLazyFieldLoading", enableLazyFieldLoading);
- m.put("maxBooleanClauses", booleanQueryMaxClauseCount);
- m.put(MIN_PREFIX_QUERY_TERM_LENGTH, prefixQueryMinPrefixLength);
+ public void writeMap(EntryWriter ew) throws IOException {
+ if (znodeVersion > -1) {
+ ew.put(ZNODEVER, znodeVersion);
+ }
+ if (luceneMatchVersion != null) {
+ ew.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM,
luceneMatchVersion.toString());
+ }
+ ew.put("updateHandler", new SimpleOrderedMap<>(getUpdateHandlerInfo()));
+ ew.put(
+ "query",
+ new SimpleOrderedMap<>(
+ (MapWriter)
+ m -> {
Review Comment:
I suspect you could pass the MapWriter as a value to `put("query", ...)`
without the SimpleOrderedMap wrapper. Try that please -- I suspect this works
(or we could enhance MapWriter to make it work if it doesn't). I just did some
looking and I think we should go this route.
##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -981,45 +991,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);
+ }));
Review Comment:
Can't this be `eq.put(tag, items)` ? If that's basically what it was before
then not sure why we add double intermediation of SOM & NL.
##########
solr/core/src/java/org/apache/solr/core/PluginInfo.java:
##########
@@ -193,27 +195,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);
Review Comment:
Implementations of `writeMap` should strongly avoid this pattern --
creating a SimpleOrderedMap (or any data structure) merely to call writeMap /
write it. The very point of `MapWriter` being introduced, over
`MapSerializable` was to avoid this.
I'll see if I can help here
##########
solr/core/src/java/org/apache/solr/core/SolrConfig.java:
##########
@@ -951,20 +946,35 @@ public int getFormUploadLimitKB() {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> result) {
- if (znodeVersion > -1) result.put(ZNODEVER, znodeVersion);
- if (luceneMatchVersion != null)
- result.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM,
luceneMatchVersion.toString());
- result.put("updateHandler", getUpdateHandlerInfo());
- Map<String, Object> m = new LinkedHashMap<>();
- result.put("query", m);
- m.put("useFilterForSortedQuery", useFilterForSortedQuery);
- m.put("queryResultWindowSize", queryResultWindowSize);
- m.put("queryResultMaxDocsCached", queryResultMaxDocsCached);
- m.put("enableLazyFieldLoading", enableLazyFieldLoading);
- m.put("maxBooleanClauses", booleanQueryMaxClauseCount);
- m.put(MIN_PREFIX_QUERY_TERM_LENGTH, prefixQueryMinPrefixLength);
+ public void writeMap(EntryWriter ew) throws IOException {
+ if (znodeVersion > -1) {
+ ew.put(ZNODEVER, znodeVersion);
+ }
+ if (luceneMatchVersion != null) {
+ ew.put(IndexSchema.LUCENE_MATCH_VERSION_PARAM,
luceneMatchVersion.toString());
+ }
+ ew.put("updateHandler", new SimpleOrderedMap<>(getUpdateHandlerInfo()));
Review Comment:
previously there was no SimpleOrderedMap wrapper added around
`getUpdateHandlerInfo`... so I'm wondering why add it now
--
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]
