This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit eb22e11471fadd788bae6bcc6e777f6c6fbe3715 Author: Serhiy Bzhezytskyy <[email protected]> AuthorDate: Fri Aug 28 06:05:55 2026 +0300 SOLR-18373: remove NamedList.asShallowMap/get(String,int) and SolrParams.toNamedList (#4761) And improve performance of the V2 API for serializing SimpleOrderedMap (custom Jackson serializer). (cherry picked from commit 55e3b8838b43c6da6e6f26f8a663f46abffe4ed7) --- .../src/java/org/apache/solr/core/SolrCore.java | 4 +- .../java/org/apache/solr/core/SolrXmlConfig.java | 12 +- .../apache/solr/handler/DumpRequestHandler.java | 2 +- .../handler/component/CombinedQueryComponent.java | 18 ++- .../solr/handler/component/QueryComponent.java | 25 ++-- .../handler/designer/DefaultSchemaSuggester.java | 4 +- .../org/apache/solr/jersey/SolrJacksonMapper.java | 38 +++++- .../apache/solr/packagemanager/PackageManager.java | 26 ++-- .../org/apache/solr/update/IndexFingerprint.java | 2 +- .../java/org/apache/solr/util/PivotListEntry.java | 3 +- .../handler/component/MockResponseBuilder.java | 4 +- .../apache/solr/jersey/SolrJacksonMapperTest.java | 80 ++++++++++++ .../solr/search/facet/TestCloudJSONFacetSKG.java | 3 +- .../src/test/org/apache/solr/util/TestUtils.java | 4 +- .../java/org/apache/solr/ltr/LTRThreadModule.java | 3 +- .../solr/client/solrj/impl/CloudSolrClient.java | 6 +- .../solrj/request/JavaBinUpdateRequestCodec.java | 5 +- .../solrj/response/schema/SchemaResponse.java | 15 +-- .../org/apache/solr/common/params/SolrParams.java | 25 ---- .../org/apache/solr/common/util/NamedList.java | 136 +-------------------- .../solrj/impl/CloudHttp2SolrClientTest.java | 6 +- .../org/apache/solr/common/util/NamedListTest.java | 18 --- .../solr/common/util/SimpleOrderedMapTest.java | 18 +++ 23 files changed, 212 insertions(+), 245 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 901531c8e51..9a350fb0b2a 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -3040,9 +3040,9 @@ public class SolrCore implements SolrInfoBean, Closeable { + "'"); } if (echoParams == EchoParamStyle.EXPLICIT) { - responseHeader.add("params", req.getOriginalParams().toNamedList()); + responseHeader.add("params", new SimpleOrderedMap<>(req.getOriginalParams())); } else if (echoParams == EchoParamStyle.ALL) { - responseHeader.add("params", req.getParams().toNamedList()); + responseHeader.add("params", new SimpleOrderedMap<>(req.getParams())); } } } diff --git a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java index aa9ba040bf9..de58f5f50aa 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -126,12 +127,11 @@ public class SolrXmlConfig { // 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())); + Map<String, String> coreAdminHandlerActions = new LinkedHashMap<>(); + for (Entry<String, Object> entry : + readNodeListAsNamedList(root.get("coreAdminHandlerActions"), "<coreAdminHandlerActions>")) { + coreAdminHandlerActions.put(entry.getKey(), entry.getValue().toString()); + } UpdateShardHandlerConfig updateConfig; if (deprecatedUpdateConfig == null) { diff --git a/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java index d96b33517ca..c0e43985a7e 100644 --- a/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java @@ -43,7 +43,7 @@ public class DumpRequestHandler extends RequestHandlerBase { @SuppressWarnings({"unchecked"}) public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException { // Show params - rsp.add("params", req.getParams().toNamedList()); + rsp.add("params", new SimpleOrderedMap<>(req.getParams())); String[] parts = req.getParams().getParams("urlTemplateValues"); if (parts != null && parts.length > 0) { Map<String, String> map = new LinkedHashMap<>(); diff --git a/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java index 28b4a80bb6e..54a73256e28 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java @@ -455,10 +455,10 @@ public class CombinedQueryComponent extends QueryComponent implements SolrCoreAw populateNextCursorMarkFromMergedShards(rb); if (thereArePartialResults) { - rb.rsp - .getResponseHeader() - .asShallowMap() - .put(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE); + updateResponseHeader( + rb.rsp.getResponseHeader(), + SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, + Boolean.TRUE); } if (segmentTerminatedEarly != null) { final Object existingSegmentTerminatedEarly = @@ -472,12 +472,10 @@ public class CombinedQueryComponent extends QueryComponent implements SolrCoreAw SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY, segmentTerminatedEarly); } else if (!Boolean.TRUE.equals(existingSegmentTerminatedEarly) && segmentTerminatedEarly) { - rb.rsp - .getResponseHeader() - .remove(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY); - rb.rsp - .getResponseHeader() - .add(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY, true); + updateResponseHeader( + rb.rsp.getResponseHeader(), + SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY, + true); } } if (maxHitsTerminatedEarly) { diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java index 3c8c2a7a5bd..9d51160bba2 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java @@ -1237,10 +1237,10 @@ public class QueryComponent extends SearchComponent { populateNextCursorMarkFromMergedShards(rb); if (thereArePartialResults) { - rb.rsp - .getResponseHeader() - .asShallowMap() - .put(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE); + updateResponseHeader( + rb.rsp.getResponseHeader(), + SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, + Boolean.TRUE); } if (segmentTerminatedEarly != null) { final Object existingSegmentTerminatedEarly = @@ -1255,14 +1255,10 @@ public class QueryComponent extends SearchComponent { segmentTerminatedEarly); } else if (!Boolean.TRUE.equals(existingSegmentTerminatedEarly) && Boolean.TRUE.equals(segmentTerminatedEarly)) { - rb.rsp - .getResponseHeader() - .remove(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY); - rb.rsp - .getResponseHeader() - .add( - SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY, - segmentTerminatedEarly); + updateResponseHeader( + rb.rsp.getResponseHeader(), + SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY, + segmentTerminatedEarly); } } if (maxHitsTerminatedEarly) { @@ -1284,6 +1280,11 @@ public class QueryComponent extends SearchComponent { } } + @SuppressWarnings("unchecked") + protected static void updateResponseHeader(NamedList<Object> header, String key, Object value) { + ((SimpleOrderedMap<Object>) header).put(key, value); + } + protected void setResultIdsAndResponseDocs( ResponseBuilder rb, ShardDocQueue shardDocQueue, diff --git a/solr/core/src/java/org/apache/solr/handler/designer/DefaultSchemaSuggester.java b/solr/core/src/java/org/apache/solr/handler/designer/DefaultSchemaSuggester.java index 963cae66283..c212d5b9359 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/DefaultSchemaSuggester.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/DefaultSchemaSuggester.java @@ -185,9 +185,7 @@ public class DefaultSchemaSuggester implements SchemaSuggester { fieldProps.add("multiValued", true); fieldProps.remove("name"); fieldProps.remove("type"); - schema = - schema.replaceField( - schemaField.getName(), schemaField.getType(), fieldProps.asShallowMap()); + schema = schema.replaceField(schemaField.getName(), schemaField.getType(), fieldProps); } // TODO: other "healing" type operations here ... but we have to be careful about overriding // explicit user changes such as a user making a text field a string field, we wouldn't want to diff --git a/solr/core/src/java/org/apache/solr/jersey/SolrJacksonMapper.java b/solr/core/src/java/org/apache/solr/jersey/SolrJacksonMapper.java index 7f57715993c..f72d541a5bf 100644 --- a/solr/core/src/java/org/apache/solr/jersey/SolrJacksonMapper.java +++ b/solr/core/src/java/org/apache/solr/jersey/SolrJacksonMapper.java @@ -27,7 +27,9 @@ import com.fasterxml.jackson.databind.ser.std.StdSerializer; import jakarta.ws.rs.ext.ContextResolver; import jakarta.ws.rs.ext.Provider; import java.io.IOException; +import java.util.Map; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; /** Customizes the ObjectMapper settings used for serialization/deserialization in Jersey */ @SuppressWarnings("rawtypes") @@ -48,6 +50,7 @@ public class SolrJacksonMapper implements ContextResolver<ObjectMapper> { private static ObjectMapper createObjectMapper() { final SimpleModule customTypeModule = new SimpleModule(); customTypeModule.addSerializer(new NamedListSerializer(NamedList.class)); + customTypeModule.addSerializer(new SimpleOrderedMapSerializer(SimpleOrderedMap.class)); return new ObjectMapper() // TODO Should failOnUnknown=false be made available on a "permissive" object mapper instead @@ -70,7 +73,40 @@ public class SolrJacksonMapper implements ContextResolver<ObjectMapper> { @Override public void serialize(NamedList value, JsonGenerator gen, SerializerProvider provider) throws IOException { - gen.writeObject(value.asShallowMap()); + // SimpleOrderedMap goes through SimpleOrderedMapSerializer below instead. asMap(0) avoids + // recursing into this same serializer for plain NamedLists. + gen.writeObject(value.asMap(0)); + } + } + + /** + * Writes a {@link SimpleOrderedMap} out directly via its {@link Map} entries, without the copy + * {@link NamedListSerializer} needs to dodge infinite recursion -- {@link SimpleOrderedMap} is + * already a {@link Map}, so there is nothing to convert. + */ + public static class SimpleOrderedMapSerializer extends StdSerializer<SimpleOrderedMap> { + + public SimpleOrderedMapSerializer() { + this(null); + } + + public SimpleOrderedMapSerializer(Class<SimpleOrderedMap> somClazz) { + super(somClazz); + } + + @Override + @SuppressWarnings("unchecked") + public void serialize(SimpleOrderedMap value, JsonGenerator gen, SerializerProvider provider) + throws IOException { + final Map<String, Object> map = (Map<String, Object>) value; + gen.writeStartObject(); + for (Map.Entry<String, Object> entry : map.entrySet()) { + // defaultSerializeField() doesn't honor NON_NULL inclusion itself -- skip nulls here. + if (entry.getValue() != null) { + provider.defaultSerializeField(entry.getKey(), entry.getValue(), gen); + } + } + gen.writeEndObject(); } } } diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java index 128538e643b..b8c38274f8d 100644 --- a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java +++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java @@ -278,7 +278,7 @@ public class PackageManager implements Closeable { Map<String, String> packageVersions = new HashMap<>(); // map of package name to multiple values of pluginMeta(Map<String, String>) Map<String, Set<PluginMeta>> packagePlugins = new HashMap<>(); - Map<String, Object> result; + Object pluginsValue; try { NamedList<Object> response = solrClient.request( @@ -286,16 +286,16 @@ public class PackageManager implements Closeable { Integer statusCode = (Integer) response._get(List.of("responseHeader", "status"), null); if (statusCode == null || statusCode == ErrorCode.NOT_FOUND.code) { // Cluster props doesn't exist, that means there are no cluster level plugins installed. - result = Map.of(); + pluginsValue = null; } else { - result = response.asShallowMap(); + pluginsValue = response.get(ContainerPluginsApi.PLUGIN); } } catch (SolrServerException | IOException ex) { throw new SolrException(ErrorCode.SERVER_ERROR, ex); } @SuppressWarnings({"unchecked"}) Map<String, Object> clusterPlugins = - (Map<String, Object>) result.getOrDefault(ContainerPluginsApi.PLUGIN, Map.of()); + pluginsValue != null ? (Map<String, Object>) pluginsValue : Map.of(); for (Map.Entry<String, Object> entry : clusterPlugins.entrySet()) { PluginMeta pluginMeta; try { @@ -421,16 +421,14 @@ public class PackageManager implements Closeable { // 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 = collectionParams.get("params") != null; SolrCLI.postJsonToSolr( solrClient, PackageUtils.getCollectionParamsPath(collection), diff --git a/solr/core/src/java/org/apache/solr/update/IndexFingerprint.java b/solr/core/src/java/org/apache/solr/update/IndexFingerprint.java index 1323c9eb083..4d4ee4664fe 100644 --- a/solr/core/src/java/org/apache/solr/update/IndexFingerprint.java +++ b/solr/core/src/java/org/apache/solr/update/IndexFingerprint.java @@ -200,7 +200,7 @@ public class IndexFingerprint implements MapWriter { if (o instanceof Map) { map = (Map<String, Object>) o; } else if (o instanceof NamedList) { - map = ((NamedList<Object>) o).asShallowMap(); + map = new SimpleOrderedMap<>((NamedList<Object>) o); } else { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown type " + o); } diff --git a/solr/core/src/java/org/apache/solr/util/PivotListEntry.java b/solr/core/src/java/org/apache/solr/util/PivotListEntry.java index 74457def798..3238dbad7f5 100644 --- a/solr/core/src/java/org/apache/solr/util/PivotListEntry.java +++ b/solr/core/src/java/org/apache/solr/util/PivotListEntry.java @@ -80,6 +80,7 @@ public enum PivotListEntry { } // otherwise... // scan starting at the min/optional index - return pivotList.get(this.getName(), this.minIndex); + final int idx = pivotList.indexOf(this.getName(), this.minIndex); + return idx == -1 ? null : pivotList.getVal(idx); } } diff --git a/solr/core/src/test/org/apache/solr/handler/component/MockResponseBuilder.java b/solr/core/src/test/org/apache/solr/handler/component/MockResponseBuilder.java index 241ff703fb5..fa8bca48727 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/MockResponseBuilder.java +++ b/solr/core/src/test/org/apache/solr/handler/component/MockResponseBuilder.java @@ -20,7 +20,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; -import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.schema.IndexSchema; @@ -48,7 +48,7 @@ public class MockResponseBuilder extends ResponseBuilder { SchemaField uniqueIdField = new SchemaField("id", new StrField()); // we need this because QueryComponent adds a property to it. - NamedList<Object> responseHeader = new NamedList<>(); + SimpleOrderedMap<Object> responseHeader = new SimpleOrderedMap<>(); // the mock implementations Mockito.when(request.getSchema()).thenReturn(indexSchema); diff --git a/solr/core/src/test/org/apache/solr/jersey/SolrJacksonMapperTest.java b/solr/core/src/test/org/apache/solr/jersey/SolrJacksonMapperTest.java new file mode 100644 index 00000000000..a43fa36082f --- /dev/null +++ b/solr/core/src/test/org/apache/solr/jersey/SolrJacksonMapperTest.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.jersey; + +import static org.hamcrest.Matchers.equalTo; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; +import org.junit.Test; + +/** Unit tests for {@link SolrJacksonMapper}'s NamedList/SimpleOrderedMap serialization. */ +public class SolrJacksonMapperTest extends SolrTestCaseJ4 { + + @Test + public void testSimpleOrderedMapSerializesDirectlyWithoutRecursing() throws Exception { + final SimpleOrderedMap<Object> top = new SimpleOrderedMap<>(); + top.add("status", 0); + + final NamedList<Object> nestedPlainNamedList = new NamedList<>(); + nestedPlainNamedList.add("nestedKey", "nestedVal"); + top.add("nested_plain_namedlist", nestedPlainNamedList); + + final SimpleOrderedMap<Object> nestedSimpleOrderedMap = new SimpleOrderedMap<>(); + nestedSimpleOrderedMap.add("innerKey", 42); + top.add("nested_simple_ordered_map", nestedSimpleOrderedMap); + + final ObjectMapper mapper = SolrJacksonMapper.getObjectMapper(); + final String json = mapper.writeValueAsString(top); + + assertThat( + json, + equalTo( + "{\"status\":0," + + "\"nested_plain_namedlist\":{\"nestedKey\":\"nestedVal\"}," + + "\"nested_simple_ordered_map\":{\"innerKey\":42}}")); + } + + @Test + public void testPlainNamedListStillSerializesViaAsMap() throws Exception { + final NamedList<Object> namedList = new NamedList<>(); + namedList.add("key", "value"); + + final ObjectMapper mapper = SolrJacksonMapper.getObjectMapper(); + final String json = mapper.writeValueAsString(namedList); + + assertThat(json, equalTo("{\"key\":\"value\"}")); + } + + @Test + public void testSimpleOrderedMapOmitsNullValuesLikeNamedListDoes() throws Exception { + final NamedList<Object> namedListWithNull = new NamedList<>(); + namedListWithNull.add("present", "val"); + namedListWithNull.add("absent", null); + + final SimpleOrderedMap<Object> somWithNull = new SimpleOrderedMap<>(); + somWithNull.add("present", "val"); + somWithNull.add("absent", null); + + final ObjectMapper mapper = SolrJacksonMapper.getObjectMapper(); + assertThat(mapper.writeValueAsString(namedListWithNull), equalTo("{\"present\":\"val\"}")); + assertThat(mapper.writeValueAsString(somWithNull), equalTo("{\"present\":\"val\"}")); + } +} diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java b/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java index 48ccb4a133f..3695c732e50 100644 --- a/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java +++ b/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java @@ -44,6 +44,7 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.embedded.JettySolrRunner; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -499,7 +500,7 @@ public class TestCloudJSONFacetSKG extends SolrCloudTestCase { assertEquals( "Unexpected keys in facet response", expectedKeys, - actualFacetResponse.asShallowMap().keySet()); + new SimpleOrderedMap<>(actualFacetResponse).keySet()); } } diff --git a/solr/core/src/test/org/apache/solr/util/TestUtils.java b/solr/core/src/test/org/apache/solr/util/TestUtils.java index 1228ff0c8bd..d0da76dc419 100644 --- a/solr/core/src/test/org/apache/solr/util/TestUtils.java +++ b/solr/core/src/test/org/apache/solr/util/TestUtils.java @@ -70,9 +70,9 @@ public class TestUtils extends SolrTestCaseJ4 { assertEquals("one", map.getName(0)); map.setName(0, "ONE"); assertEquals("ONE", map.getName(0)); - assertEquals(Integer.valueOf(100), map.get("one", 1)); + assertEquals(Integer.valueOf(100), map.getVal(map.indexOf("one", 1))); assertEquals(4, map.indexOf(null, 1)); - assertNull(map.get(null, 1)); + assertNull(map.getVal(map.indexOf(null, 1))); map = new SimpleOrderedMap<>(); map.add("one", 1); diff --git a/solr/modules/ltr/src/java/org/apache/solr/ltr/LTRThreadModule.java b/solr/modules/ltr/src/java/org/apache/solr/ltr/LTRThreadModule.java index dccae8bb319..43dad63e2a1 100644 --- a/solr/modules/ltr/src/java/org/apache/solr/ltr/LTRThreadModule.java +++ b/solr/modules/ltr/src/java/org/apache/solr/ltr/LTRThreadModule.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Semaphore; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.util.SolrPluginUtils; import org.apache.solr.util.plugin.NamedListInitializedPlugin; @@ -88,7 +89,7 @@ public final class LTRThreadModule implements NamedListInitializedPlugin { // remove consumed keys only once iteration is complete // since NamedList iterator does not support 'remove' - for (Object key : extractedArgs.asShallowMap().keySet()) { + for (Object key : new SimpleOrderedMap<>(extractedArgs).keySet()) { args.remove(CONFIG_PREFIX + key); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 376ee50ce60..58577187cf9 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -683,7 +683,11 @@ public abstract class CloudSolrClient extends SolrClient { resp = sendRequest(request, inputCollections); // to avoid an O(n) operation we always add STATE_VERSION to the last and try to read it from // there - Object o = resp == null || resp.size() == 0 ? null : resp.get(STATE_VERSION, resp.size() - 1); + Object o = null; + if (resp != null && resp.size() > 0) { + final int stateVersionIdx = resp.indexOf(STATE_VERSION, resp.size() - 1); + o = stateVersionIdx == -1 ? null : resp.getVal(stateVersionIdx); + } if (o != null && o instanceof Map<?, ?> invalidStates) { // remove this because no one else needs this and tests would fail if they are comparing // responses diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java index 6ce10ef381b..5da32870e04 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java @@ -35,6 +35,7 @@ import org.apache.solr.common.util.CollectionUtil; import org.apache.solr.common.util.DataInputInputStream; import org.apache.solr.common.util.JavaBinCodec; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; /** * Provides methods for marshalling an UpdateRequest to a NamedList which can be serialized in the @@ -56,7 +57,9 @@ public class JavaBinUpdateRequestCodec { public void marshal(UpdateRequest updateRequest, OutputStream os) throws IOException { NamedList<Object> nl = new NamedList<>(); - NamedList<Object> params = updateRequest.getParams().toNamedList(); + // Must be SimpleOrderedMap, not a plain NamedList: JavaBinCodec picks the wire tag + // (ORDERED_MAP vs NAMED_LST) from the runtime type, and receivers expect ORDERED_MAP here. + NamedList<Object> params = new SimpleOrderedMap<>(updateRequest.getParams()); if (updateRequest.getCommitWithin() != -1) { params.add("commitWithin", updateRequest.getCommitWithin()); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java index 7f34859f0a3..e2ca9d835f1 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java @@ -24,6 +24,7 @@ import org.apache.solr.client.solrj.request.schema.AnalyzerDefinition; import org.apache.solr.client.solrj.request.schema.FieldTypeDefinition; import org.apache.solr.client.solrj.response.SolrResponseBase; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; /** * This class is used to wrap the response messages retrieved from Solr Schema API. @@ -267,7 +268,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - schemaName = SchemaResponse.getSchemaName(response.asShallowMap()); + schemaName = SchemaResponse.getSchemaName(new SimpleOrderedMap<>(response)); } public String getSchemaName() { @@ -282,7 +283,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - schemaVersion = SchemaResponse.getSchemaVersion(response.asShallowMap()); + schemaVersion = SchemaResponse.getSchemaVersion(new SimpleOrderedMap<>(response)); } public float getSchemaVersion() { @@ -314,7 +315,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - fields = SchemaResponse.getFields(response.asShallowMap()); + fields = SchemaResponse.getFields(new SimpleOrderedMap<>(response)); } public List<Map<String, Object>> getFields() { @@ -361,7 +362,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - uniqueKey = SchemaResponse.getSchemaUniqueKey(response.asShallowMap()); + uniqueKey = SchemaResponse.getSchemaUniqueKey(new SimpleOrderedMap<>(response)); } public String getUniqueKey() { @@ -376,7 +377,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - similarity = SchemaResponse.getSimilarity(response.asShallowMap()); + similarity = SchemaResponse.getSimilarity(new SimpleOrderedMap<>(response)); } public Map<String, Object> getSimilarity() { @@ -391,7 +392,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - copyFields = SchemaResponse.getCopyFields(response.asShallowMap()); + copyFields = SchemaResponse.getCopyFields(new SimpleOrderedMap<>(response)); } public List<Map<String, Object>> getCopyFields() { @@ -423,7 +424,7 @@ public class SchemaResponse extends SolrResponseBase { public void setResponse(NamedList<Object> response) { super.setResponse(response); - fieldTypes = SchemaResponse.getFieldTypeRepresentations(response.asShallowMap()); + fieldTypes = SchemaResponse.getFieldTypeRepresentations(new SimpleOrderedMap<>(response)); } public List<FieldTypeRepresentation> getFieldTypes() { diff --git a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java index 9243350766e..7116f4a5db9 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java @@ -31,8 +31,6 @@ import java.util.stream.StreamSupport; import org.apache.solr.client.solrj.util.ClientUtils; import org.apache.solr.common.MapWriter; import org.apache.solr.common.SolrException; -import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; /** @@ -411,29 +409,6 @@ public abstract class SolrParams return AppendedSolrParams.wrapAppended(params, defaults); } - /** - * Convert this to a NamedList of unique keys with either String or String[] values depending on - * how many values there are for the parameter. - * - * @deprecated see {@link SimpleOrderedMap#SimpleOrderedMap(MapWriter)} - */ - @Deprecated - public NamedList<Object> toNamedList() { - final SimpleOrderedMap<Object> result = new SimpleOrderedMap<>(); - - for (Iterator<String> it = getParameterNamesIterator(); it.hasNext(); ) { - final String name = it.next(); - final String[] values = getParams(name); - if (values.length == 1) { - result.add(name, values[0]); - } else { - // currently, no reason not to use the same array - result.add(name, values); - } - } - return result; - } - /** * Returns this SolrParams as a proper URL encoded string, starting with {@code "?"}, if not * empty. diff --git a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java index f13a71f8723..13afd38e428 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java @@ -28,7 +28,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.function.BiConsumer; import org.apache.solr.common.MapWriter; import org.apache.solr.common.SolrException; @@ -250,10 +249,10 @@ public class NamedList<T> * * @return null if not found or if the value stored was null. * @see #indexOf - * @see #get(String,int) */ public T get(String name) { - return get(name, 0); + final int idx = indexOf(name); + return idx == -1 ? null : getVal(idx); } /** Like {@link #get(String)} but returns a default value if it would be null. */ @@ -262,31 +261,6 @@ public class NamedList<T> return val == null ? def : val; } - /** - * Gets the value for the first instance of the specified name found starting at the specified - * index. - * - * <p>NOTE: this runs in linear time (it scans starting at the specified position until it finds - * the first pair with the specified name). - * - * @return null if not found or if the value stored was null. - * @see #indexOf - * @deprecated Use {@link #indexOf(String, int)} then {@link #getVal(int)}. - */ - @Deprecated - public T get(String name, int start) { - int sz = size(); - for (int i = start; i < sz; i++) { - String n = getName(i); - if (name == null) { - if (n == null) return getVal(i); - } else if (name.equals(n)) { - return getVal(i); - } - } - return null; - } - /** * Gets the values for the specified name * @@ -342,112 +316,6 @@ public class NamedList<T> return new NamedList<>(Collections.unmodifiableList(copy.nvPairs)); } - /** - * @deprecated Use {@link SimpleOrderedMap} instead. - */ - @Deprecated - public Map<String, T> asShallowMap() { - return asShallowMap(false); - } - - /** - * @deprecated use {@link SimpleOrderedMap} instead of NamedList when a Map is required. - */ - @Deprecated - public Map<String, T> asShallowMap(boolean allowDps) { - return new Map<>() { - @Override - public int size() { - return NamedList.this.size(); - } - - @Override - public boolean isEmpty() { - return size() == 0; - } - - @Override - public boolean containsKey(Object key) { - return NamedList.this.get((String) key) != null; - } - - @Override - public boolean containsValue(Object value) { - return false; - } - - @Override - public T get(Object key) { - return NamedList.this.get((String) key); - } - - @Override - public T put(String key, T value) { - if (allowDps) { - NamedList.this.add(key, value); - return null; - } - int idx = NamedList.this.indexOf(key, 0); - if (idx == -1) { - NamedList.this.add(key, value); - } else { - NamedList.this.setVal(idx, value); - } - return null; - } - - @Override - public T remove(Object key) { - return NamedList.this.remove((String) key); - } - - @Override - @SuppressWarnings({"unchecked"}) - public void putAll(Map m) { - boolean isEmpty = isEmpty(); - for (Object o : m.entrySet()) { - @SuppressWarnings({"rawtypes"}) - Map.Entry e = (Entry) o; - if (isEmpty) { // we know that there are no duplicates - add((String) e.getKey(), (T) e.getValue()); - } else { - put(e.getKey() == null ? null : e.getKey().toString(), (T) e.getValue()); - } - } - } - - @Override - public void clear() { - NamedList.this.clear(); - } - - @Override - @SuppressWarnings({"unchecked"}) - public Set<String> keySet() { - // TODO implement more efficiently - return NamedList.this.asMap(1).keySet(); - } - - @Override - @SuppressWarnings({"unchecked", "rawtypes"}) - public Collection values() { - // TODO implement more efficiently - return NamedList.this.asMap(1).values(); - } - - @Override - public Set<Entry<String, T>> entrySet() { - // TODO implement more efficiently - return NamedList.this.asMap(1).entrySet(); - } - - @Override - public void forEach(BiConsumer action) { - NamedList.this.forEach(action); - } - }; - } - @SuppressWarnings("rawtypes") public Map asMap(int maxDepth) { LinkedHashMap result = new LinkedHashMap<>(); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index bb8ca9898e4..94ca36f2b15 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -853,9 +853,11 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { COLLECTION + ":" + (coll.getZNodeVersion() - 1)); // an older version expect error QueryResponse rsp = solrClient.query(q); + final NamedList<Object> response = rsp.getResponse(); + final int stateVersionIdx = + response.indexOf(CloudSolrClient.STATE_VERSION, response.size() - 1); @SuppressWarnings({"rawtypes"}) - Map m = - (Map) rsp.getResponse().get(CloudSolrClient.STATE_VERSION, rsp.getResponse().size() - 1); + Map m = stateVersionIdx == -1 ? null : (Map) response.getVal(stateVersionIdx); assertNotNull( "Expected an extra information from server with the list of invalid collection states", m); diff --git a/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java b/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java index cadc88ed890..57bfffdf057 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java @@ -18,7 +18,6 @@ package org.apache.solr.common.util; import java.util.ArrayList; import java.util.List; -import java.util.Map; import org.apache.solr.SolrTestCase; import org.apache.solr.common.SolrException; import org.junit.Test; @@ -192,21 +191,4 @@ public class NamedListTest extends SolrTestCase { Object enltest4 = enl._get(List.of("key2"), null); assertNull(enltest4); } - - @Test - public void testShallowMap() { - NamedList<String> nl = new NamedList<>(); - nl.add("key1", "Val1"); - Map<String, String> m = nl.asShallowMap(); - m.put("key1", "Val1_"); - assertEquals("Val1_", nl.get("key1")); - assertEquals("Val1_", m.get("key1")); - assertEquals(0, nl.indexOf("key1", 0)); - m.putAll(Map.of("key1", "Val1__", "key2", "Val2")); - assertEquals("Val1__", nl.get("key1")); - assertEquals("Val1__", m.get("key1")); - assertEquals(0, nl.indexOf("key1", 0)); - assertEquals("Val2", nl.get("key2")); - assertEquals("Val2", m.get("key2")); - } } diff --git a/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java b/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java index b91f991251c..f50216f4848 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java @@ -193,6 +193,24 @@ public class SimpleOrderedMapTest extends SolrTestCase { assertFalse(map.containsKey("two")); } + /** The MapWriter constructor copies rather than returning a live view. */ + @Test + public void testMapWriterConstructorCopiesRatherThanViewing() { + final NamedList<Integer> source = new NamedList<>(); + source.add("one", 1); + + final SimpleOrderedMap<Integer> copy = new SimpleOrderedMap<>(source); + assertEquals(Integer.valueOf(1), copy.get("one")); + + copy.put("one", 11); + assertEquals( + "mutating the copy must not reach the source", Integer.valueOf(1), source.get("one")); + assertEquals(Integer.valueOf(11), copy.get("one")); + + source.add("two", 2); + assertNull("adding to the source must not reach the copy", copy.get("two")); + } + private void setupData() { map.add("one", 1); map.add("two", 2);
