serhiy-bzhezytskyy commented on code in PR #4761:
URL: https://github.com/apache/solr/pull/4761#discussion_r3838130211
##########
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##########
Review Comment:
Applied the same fix here ahead of SOLR-18398: `((SimpleOrderedMap<Object>)
rb.rsp.getResponseHeader()).put(...)`. Once SOLR-18398 is merged and
`getResponseHeader()` returns `SimpleOrderedMap` directly, the cast can be
dropped.
##########
solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java:
##########
@@ -127,11 +128,11 @@ public static NodeConfig fromConfig(
// It should go inside the fillSolrSection method but
// since it is arranged as a separate section it is placed here
Map<String, String> coreAdminHandlerActions =
- readNodeListAsNamedList(root.get("coreAdminHandlerActions"),
"<coreAdminHandlerActions>")
- .asShallowMap()
- .entrySet()
- .stream()
- .collect(Collectors.toMap(Entry::getKey, item ->
item.getValue().toString()));
+ new SimpleOrderedMap<>(
Review Comment:
Switched to a plain loop over the NamedList into a `LinkedHashMap`, no
`SimpleOrderedMap` involved.
##########
solr/core/src/java/org/apache/solr/jersey/SolrJacksonMapper.java:
##########
@@ -70,7 +70,8 @@ public NamedListSerializer(Class<NamedList> nlClazz) {
@Override
public void serialize(NamedList value, JsonGenerator gen,
SerializerProvider provider)
throws IOException {
- gen.writeObject(value.asShallowMap());
+ // Not SimpleOrderedMap: it IS a NamedList, so this serializer would
recurse on it.
+ gen.writeObject(value.asMap(0));
Review Comment:
Expanded: `SimpleOrderedMap` extends `NamedList`, so this serializer would
recurse into it infinitely if used here; `asMap(0)` returns a plain
`LinkedHashMap` at the top level while leaving any nested `NamedList` values
untouched.
##########
solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java:
##########
@@ -421,16 +422,14 @@ private Pair<List<String>, List<String>>
deployCollectionPackage(
// Get package params
try {
- boolean packageParamsExist =
- solrClient
- .request(
- new GenericV2SolrRequest(
- SolrRequest.METHOD.GET,
- PackageUtils.getCollectionParamsPath(collection) +
"/packages")
- .setRequiresCollection(
- false) /* Making a collection-request, but already
baked into path */)
- .asShallowMap()
- .containsKey("params");
+ NamedList<Object> collectionParams =
+ solrClient.request(
+ new GenericV2SolrRequest(
+ SolrRequest.METHOD.GET,
+ PackageUtils.getCollectionParamsPath(collection) +
"/packages")
+ .setRequiresCollection(
+ false) /* Making a collection-request, but already
baked into path */);
+ boolean packageParamsExist = new
SimpleOrderedMap<>(collectionParams).containsKey("params");
Review Comment:
Dropped the `SimpleOrderedMap` wrapper: `collectionParams` is already a
`NamedList`, so `get("params") != null` works directly on it -- went with this
per your second suggestion.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java:
##########
@@ -56,7 +57,8 @@ public class JavaBinUpdateRequestCodec {
public void marshal(UpdateRequest updateRequest, OutputStream os) throws
IOException {
NamedList<Object> nl = new NamedList<>();
- NamedList<Object> params = updateRequest.getParams().toNamedList();
+ // SimpleOrderedMap serializes with the same JavaBin ORDERED_MAP tag as
before.
Review Comment:
Reworded: `JavaBinCodec` picks the wire tag (`ORDERED_MAP` vs `NAMED_LST`)
from the runtime type, and receivers expect `ORDERED_MAP` here -- so it has to
stay a `SimpleOrderedMap`, not a plain `NamedList`.
--
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]
