From 364c0afb5bc98460713e6ede071d375058e8de9c Mon Sep 17 00:00:00 2001 From: David Smiley <dsmiley@apache.org> Date: Tue, 10 Dec 2024 10:52:18 -0500 Subject: [PATCH] SOLR-14680: Deprecating org.apache.solr.cluster.api, includes NamedList methods (#2856) NamedList: deprecated methods: forEachEntry, forEachKey, abortableForEachKey, abortableForEach, asMap (no-arg only), get(key, default) -- all come from the SimpleMap interface. Added getOrDefault. Deprecated the SimpleMap interface as well as the entirety of the SolrJ package org.apache.solr.cluster.api, which wasn't used except for SimpleMap. Includes some trivial refactorings to not call those deprecated methods. Includes internal changes to ConfigNode and friends so as to not use SimpleMap. (cherry picked from commit c80d41597c83c8c73634144ab8c55f8002dd101d) --- solr/CHANGES.txt | 4 + .../src/java/org/apache/solr/cli/ApiTool.java | 2 +- .../java/org/apache/solr/cli/CreateTool.java | 10 +- .../java/org/apache/solr/cli/DeleteTool.java | 2 +- .../java/org/apache/solr/cli/StatusTool.java | 5 +- .../apache/solr/core/OverlaidConfigNode.java | 4 +- .../java/org/apache/solr/core/PluginInfo.java | 2 +- .../org/apache/solr/core/SolrXmlConfig.java | 7 +- .../org/apache/solr/schema/IndexSchema.java | 2 +- .../org/apache/solr/search/CacheConfig.java | 4 +- .../org/apache/solr/update/UpdateLog.java | 3 +- .../org/apache/solr/util/DOMConfigNode.java | 11 +- .../org/apache/solr/util/DataConfigNode.java | 116 +++++++++++++----- .../apache/solr/core/TestCodecSupport.java | 3 +- .../handler/RequestHandlerMetricsTest.java | 7 +- .../admin/CoreAdminHandlerActionTest.java | 5 +- .../facet/SpatialHeatmapFacetsTest.java | 8 +- .../apache/solr/common/LazySolrCluster.java | 1 + .../org/apache/solr/common/SimpleZkMap.java | 1 + .../common/cloud/PerReplicaStatesOps.java | 2 +- .../org/apache/solr/cluster/api/ApiType.java | 1 + .../solr/cluster/api/CollectionConfig.java | 1 + .../apache/solr/cluster/api/HashRange.java | 1 + .../org/apache/solr/cluster/api/Resource.java | 1 + .../org/apache/solr/cluster/api/Router.java | 1 + .../org/apache/solr/cluster/api/Shard.java | 1 + .../apache/solr/cluster/api/ShardReplica.java | 1 + .../apache/solr/cluster/api/SimpleMap.java | 8 ++ .../apache/solr/cluster/api/SolrCluster.java | 1 + .../solr/cluster/api/SolrCollection.java | 1 + .../org/apache/solr/cluster/api/SolrNode.java | 1 + .../apache/solr/cluster/api/package-info.java | 1 + .../org/apache/solr/common/ConfigNode.java | 11 +- .../apache/solr/common/cloud/DocRouter.java | 6 +- .../solr/common/cloud/PerReplicaStates.java | 26 ++-- .../org/apache/solr/common/util/DOMUtil.java | 2 +- .../apache/solr/common/util/NamedList.java | 8 +- .../solr/common/util/WrappedSimpleMap.java | 1 + .../PerReplicaStatesIntegrationTest.java | 6 +- 39 files changed, 174 insertions(+), 104 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f08bd159fe2..7b9a36db61c 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -173,6 +173,10 @@ led to the suppression of exceptions. (Andrey Bozhko) * SOLR-16781: "<lib/>" tags in solrconfig.xml are now quietly ignored by default unless explicitly enabled with the `SOLR_CONFIG_LIB_ENABLED=true` enviroment variable (or corresponding sysprop). These tags are now considered deprecated and will be removed in Solr 10. +* SOLR-14680: NamedList: deprecating methods: forEachEntry, forEachKey, abortableForEachKey, abortableForEach, + asMap (no-arg only), get(key, default). Added getOrDefault. Deprecated the SimpleMap interface as well as the + entirety of the SolrJ package org.apache.solr.cluster.api, which wasn't used except for SimpleMap. (David Smiley) + ================== 9.7.1 ================== Bug Fixes --------------------- diff --git a/solr/core/src/java/org/apache/solr/cli/ApiTool.java b/solr/core/src/java/org/apache/solr/cli/ApiTool.java index 1c412062ec8..f4fd3724032 100644 --- a/solr/core/src/java/org/apache/solr/cli/ApiTool.java +++ b/solr/core/src/java/org/apache/solr/cli/ApiTool.java @@ -108,7 +108,7 @@ protected String callGet(String url) throws Exception { NamedList<Object> response = solrClient.request(req); // pretty-print the response to stdout CharArr arr = new CharArr(); - new JSONWriter(arr, 2).write(response.asMap()); + new JSONWriter(arr, 2).write(response.asMap(10)); return arr.toString(); } } diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java index b2e7e063baa..f4d04fa45c9 100644 --- a/solr/core/src/java/org/apache/solr/cli/CreateTool.java +++ b/solr/core/src/java/org/apache/solr/cli/CreateTool.java @@ -23,7 +23,6 @@ import java.nio.file.Paths; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.commons.cli.CommandLine; @@ -219,10 +218,9 @@ protected void createCore(CommandLine cli, SolrClient solrClient) throws Excepti String coreRootDirectory; // usually same as solr home, but not always - Map<String, Object> systemInfo = - solrClient - .request(new GenericSolrRequest(SolrRequest.METHOD.GET, CommonParams.SYSTEM_INFO_PATH)) - .asMap(); + NamedList<?> systemInfo = + solrClient.request( + new GenericSolrRequest(SolrRequest.METHOD.GET, CommonParams.SYSTEM_INFO_PATH)); // convert raw JSON into user-friendly output coreRootDirectory = (String) systemInfo.get("core_root"); @@ -375,7 +373,7 @@ protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine cli if (verbose) { // pretty-print the response to stdout CharArr arr = new CharArr(); - new JSONWriter(arr, 2).write(response.asMap()); + new JSONWriter(arr, 2).write(response.asMap(10)); echo(arr.toString()); } String endMessage = diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java index 535b87ce1ff..fa44fe14b04 100644 --- a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java +++ b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java @@ -296,7 +296,7 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli if (response != null) { // pretty-print the response to stdout CharArr arr = new CharArr(); - new JSONWriter(arr, 2).write(response.asMap()); + new JSONWriter(arr, 2).write(response.asMap(10)); echo(arr.toString()); echo("\n"); } diff --git a/solr/core/src/java/org/apache/solr/cli/StatusTool.java b/solr/core/src/java/org/apache/solr/cli/StatusTool.java index 4a8322b1c54..3e2368a8d6a 100644 --- a/solr/core/src/java/org/apache/solr/cli/StatusTool.java +++ b/solr/core/src/java/org/apache/solr/cli/StatusTool.java @@ -365,13 +365,14 @@ protected Map<String, String> getCloudStatus(SolrClient solrClient, String zkHos Map<String, String> cloudStatus = new LinkedHashMap<>(); cloudStatus.put("ZooKeeper", (zkHost != null) ? zkHost : "?"); + // TODO add booleans to request just what we want; not everything NamedList<Object> json = solrClient.request(new CollectionAdminRequest.ClusterStatus()); List<String> liveNodes = (List<String>) json.findRecursive("cluster", "live_nodes"); cloudStatus.put("liveNodes", String.valueOf(liveNodes.size())); - Map<String, Object> collections = - ((NamedList<Object>) json.findRecursive("cluster", "collections")).asMap(); + // TODO get this as a metric from the metrics API instead, or something else. + var collections = (NamedList<Object>) json.findRecursive("cluster", "collections"); cloudStatus.put("collections", String.valueOf(collections.size())); return cloudStatus; diff --git a/solr/core/src/java/org/apache/solr/core/OverlaidConfigNode.java b/solr/core/src/java/org/apache/solr/core/OverlaidConfigNode.java index 30561311dfc..c6fab13fef7 100644 --- a/solr/core/src/java/org/apache/solr/core/OverlaidConfigNode.java +++ b/solr/core/src/java/org/apache/solr/core/OverlaidConfigNode.java @@ -19,9 +19,9 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.function.Function; import java.util.function.Predicate; -import org.apache.solr.cluster.api.SimpleMap; import org.apache.solr.common.ConfigNode; /** A config node impl which has an overlay */ @@ -80,7 +80,7 @@ public String name() { } @Override - public SimpleMap<String> attributes() { + public Map<String, String> attributes() { return delegate.attributes(); } diff --git a/solr/core/src/java/org/apache/solr/core/PluginInfo.java b/solr/core/src/java/org/apache/solr/core/PluginInfo.java index 5c24ee9a8f4..b55907f1e2f 100644 --- a/solr/core/src/java/org/apache/solr/core/PluginInfo.java +++ b/solr/core/src/java/org/apache/solr/core/PluginInfo.java @@ -118,7 +118,7 @@ public PluginInfo(ConfigNode node, String err, boolean requireName, boolean requ className = cName.className; pkgName = cName.pkg; initArgs = DOMUtil.childNodesToNamedList(node); - attributes = node.attributes().asMap(); + attributes = node.attributes(); children = loadSubPlugins(node); isFromSolrConfig = true; } 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 cb326e734ea..270ae18d32a 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java @@ -31,6 +31,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Properties; import java.util.Set; import java.util.function.Consumer; @@ -141,10 +142,10 @@ public static NodeConfig fromConfig( // since it is arranged as a separate section it is placed here Map<String, String> coreAdminHandlerActions = readNodeListAsNamedList(root.get("coreAdminHandlerActions"), "<coreAdminHandlerActions>") - .asMap() + .asShallowMap() .entrySet() .stream() - .collect(Collectors.toMap(item -> item.getKey(), item -> item.getValue().toString())); + .collect(Collectors.toMap(Entry::getKey, item -> item.getValue().toString())); UpdateShardHandlerConfig updateConfig; if (deprecatedUpdateConfig == null) { @@ -752,7 +753,7 @@ private static MetricsConfig getMetricsConfig(ConfigNode metrics) { ConfigNode caching = metrics.get("solr/metrics/caching"); if (caching != null) { Object threadsCachingIntervalSeconds = - DOMUtil.childNodesToNamedList(caching).get("threadsIntervalSeconds", null); + DOMUtil.childNodesToNamedList(caching).get("threadsIntervalSeconds"); builder.setCacheConfig( new MetricsConfig.CacheConfig( threadsCachingIntervalSeconds == null diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 06dad3175dc..b3f59eab4e7 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -526,7 +526,7 @@ protected void readSchema(ConfigSetService.ConfigResource is) { log.info("{}", sb); } - version = Float.parseFloat(rootNode.attributes().get("version", "1.0f")); + version = Float.parseFloat(rootNode.attributes().getOrDefault("version", "1.0f")); // load the Field Types final FieldTypePluginLoader typeLoader = diff --git a/solr/core/src/java/org/apache/solr/search/CacheConfig.java b/solr/core/src/java/org/apache/solr/search/CacheConfig.java index 83ce72af836..da05152a86d 100644 --- a/solr/core/src/java/org/apache/solr/search/CacheConfig.java +++ b/solr/core/src/java/org/apache/solr/search/CacheConfig.java @@ -87,7 +87,7 @@ public static Map<String, CacheConfig> getMultipleConfigs( for (ConfigNode node : nodes) { if (node.boolAttr("enabled", true)) { CacheConfig config = - getConfig(loader, solrConfig, node.name(), node.attributes().asMap(), configPath); + getConfig(loader, solrConfig, node.name(), node.attributes(), configPath); result.put(config.args.get(NAME), config); } } @@ -98,7 +98,7 @@ public static CacheConfig getConfig(SolrConfig solrConfig, ConfigNode node, Stri if (!node.boolAttr("enabled", true) || !node.exists()) { return null; } - return getConfig(solrConfig, node.name(), node.attributes().asMap(), xpath); + return getConfig(solrConfig, node.name(), node.attributes(), xpath); } public static CacheConfig getConfig( diff --git a/solr/core/src/java/org/apache/solr/update/UpdateLog.java b/solr/core/src/java/org/apache/solr/update/UpdateLog.java index d1c430bceee..4d148ed0af4 100644 --- a/solr/core/src/java/org/apache/solr/update/UpdateLog.java +++ b/solr/core/src/java/org/apache/solr/update/UpdateLog.java @@ -391,7 +391,8 @@ public void init(PluginInfo info) { } int timeoutMs = objToInt( - info.initArgs.get("docLockTimeoutMs", info.initArgs.get("versionBucketLockTimeoutMs")), + info.initArgs.getOrDefault( + "docLockTimeoutMs", info.initArgs.get("versionBucketLockTimeoutMs")), EnvUtils.getPropertyAsLong("solr.update.docLockTimeoutMs", 0L).intValue()); updateLocks = new UpdateLocks(timeoutMs); diff --git a/solr/core/src/java/org/apache/solr/util/DOMConfigNode.java b/solr/core/src/java/org/apache/solr/util/DOMConfigNode.java index f7a47b5cc93..d35cd4bccc5 100644 --- a/solr/core/src/java/org/apache/solr/util/DOMConfigNode.java +++ b/solr/core/src/java/org/apache/solr/util/DOMConfigNode.java @@ -18,14 +18,11 @@ package org.apache.solr.util; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Function; -import org.apache.solr.cluster.api.SimpleMap; import org.apache.solr.common.ConfigNode; import org.apache.solr.common.util.DOMUtil; -import org.apache.solr.common.util.WrappedSimpleMap; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -33,7 +30,7 @@ public class DOMConfigNode implements ConfigNode { private final Node node; - SimpleMap<String> attrs; + Map<String, String> attrs; @Override public String name() { @@ -50,10 +47,10 @@ public DOMConfigNode(Node node) { } @Override - public SimpleMap<String> attributes() { + public Map<String, String> attributes() { if (attrs != null) return attrs; Map<String, String> attrs = DOMUtil.toMap(node.getAttributes()); - return this.attrs = attrs.size() == 0 ? EMPTY : new WrappedSimpleMap<>(attrs); + return this.attrs = attrs.isEmpty() ? Map.of() : attrs; } @Override @@ -85,6 +82,4 @@ public void forEachChild(Function<ConfigNode, Boolean> fun) { if (Boolean.FALSE.equals(toContinue)) break; } } - - private static final SimpleMap<String> EMPTY = new WrappedSimpleMap<>(Collections.emptyMap()); } diff --git a/solr/core/src/java/org/apache/solr/util/DataConfigNode.java b/solr/core/src/java/org/apache/solr/util/DataConfigNode.java index e8a00075031..4711e644cb4 100644 --- a/solr/core/src/java/org/apache/solr/util/DataConfigNode.java +++ b/solr/core/src/java/org/apache/solr/util/DataConfigNode.java @@ -17,8 +17,10 @@ package org.apache.solr.util; +import java.util.AbstractMap; +import java.util.AbstractSet; import java.util.ArrayList; -import java.util.Collections; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -26,22 +28,20 @@ import java.util.function.BiConsumer; import java.util.function.Function; import java.util.function.Predicate; -import org.apache.solr.cluster.api.SimpleMap; import org.apache.solr.common.ConfigNode; import org.apache.solr.common.util.PropertiesUtil; -import org.apache.solr.common.util.WrappedSimpleMap; /** ConfigNode impl that copies and maintains data internally from DOM */ public class DataConfigNode implements ConfigNode { public final String name; - public final SimpleMap<String> attributes; - public final SimpleMap<List<ConfigNode>> kids; + public final Map<String, String> attributes; + public final Map<String, List<ConfigNode>> kids; public final String textData; public DataConfigNode(ConfigNode root) { Map<String, List<ConfigNode>> kids = new LinkedHashMap<>(); name = root.name(); - attributes = wrap(root.attributes()); + attributes = wrapSubstituting(root.attributes()); textData = root.txt(); root.forEachChild( it -> { @@ -54,31 +54,17 @@ public DataConfigNode(ConfigNode root) { e.setValue(List.copyOf(e.getValue())); } } - this.kids = kids.isEmpty() ? EMPTY : new WrappedSimpleMap<>(Map.copyOf(kids)); + this.kids = Map.copyOf(kids); } - public String subtituteVal(String s) { + private static String substituteVal(String s) { return PropertiesUtil.substitute(s, SUBSTITUTES.get()); } - private SimpleMap<String> wrap(SimpleMap<String> delegate) { + /** provides a substitute view, and read-only */ + private static Map<String, String> wrapSubstituting(Map<String, String> delegate) { if (delegate.size() == 0) return delegate; // avoid unnecessary object creation - return new SimpleMap<>() { - @Override - public String get(String key) { - return subtituteVal(delegate.get(key)); - } - - @Override - public void forEachEntry(BiConsumer<String, ? super String> fun) { - delegate.forEachEntry((k, v) -> fun.accept(k, subtituteVal(v))); - } - - @Override - public int size() { - return delegate.size(); - } - }; + return new SubstitutingMap(delegate); } @Override @@ -88,11 +74,11 @@ public String name() { @Override public String txt() { - return subtituteVal(textData); + return substituteVal(textData); } @Override - public SimpleMap<String> attributes() { + public Map<String, String> attributes() { return attributes; } @@ -104,7 +90,7 @@ public ConfigNode child(String name) { @Override public List<ConfigNode> getAll(String name) { - return kids.get(name, Collections.emptyList()); + return kids.getOrDefault(name, List.of()); } @Override @@ -126,7 +112,7 @@ public List<ConfigNode> getAll(Predicate<ConfigNode> test, Set<String> matchName @Override public void forEachChild(Function<ConfigNode, Boolean> fun) { - kids.forEachEntry( + kids.forEach( (s, configNodes) -> { if (configNodes != null) { configNodes.forEach(fun::apply); @@ -134,6 +120,74 @@ public void forEachChild(Function<ConfigNode, Boolean> fun) { }); } - public static final SimpleMap<List<ConfigNode>> EMPTY = - new WrappedSimpleMap<>(Collections.emptyMap()); + private static class SubstitutingMap extends AbstractMap<String, String> { + + private final Map<String, String> delegate; + + SubstitutingMap(Map<String, String> delegate) { + this.delegate = delegate; + } + + @Override + public String get(Object key) { + return substituteVal(delegate.get(key)); + } + + @Override + public int size() { + return delegate.size(); + } + + @Override + public Set<String> keySet() { + return delegate.keySet(); + } + + @Override + public void forEach(BiConsumer<? super String, ? super String> action) { + delegate.forEach((k, v) -> action.accept(k, substituteVal(v))); + } + + @Override + public Set<Entry<String, String>> entrySet() { + return new AbstractSet<>() { + @Override + public Iterator<Entry<String, String>> iterator() { + // using delegate, return an iterator using Streams + return delegate.entrySet().stream() + .map(entry -> (Entry<String, String>) new SubstitutingEntry(entry)) + .iterator(); + } + + @Override + public int size() { + return delegate.size(); + } + }; + } + + private static class SubstitutingEntry implements Entry<String, String> { + + private final Entry<String, String> delegateEntry; + + SubstitutingEntry(Entry<String, String> delegateEntry) { + this.delegateEntry = delegateEntry; + } + + @Override + public String getKey() { + return delegateEntry.getKey(); + } + + @Override + public String getValue() { + return substituteVal(delegateEntry.getValue()); + } + + @Override + public String setValue(String value) { + throw new UnsupportedOperationException(); + } + } + } } diff --git a/solr/core/src/test/org/apache/solr/core/TestCodecSupport.java b/solr/core/src/test/org/apache/solr/core/TestCodecSupport.java index 8fd60619122..54f7406635d 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCodecSupport.java +++ b/solr/core/src/test/org/apache/solr/core/TestCodecSupport.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.Map; +import java.util.Set; import org.apache.lucene.codecs.Codec; import org.apache.lucene.codecs.lucene99.Lucene99Codec.Mode; import org.apache.lucene.codecs.perfield.PerFieldDocValuesFormat; @@ -238,7 +239,7 @@ public void testCompressionModeDefault() throws IOException { config.get("codecFactory").attr("class")); assertTrue( "Unexpected configuration of codec factory for this test. Expecting empty element", - config.get("codecFactory").getAll(null, (String) null).isEmpty()); + config.get("codecFactory").getAll(null, Set.of()).isEmpty()); IndexSchema schema = IndexSchemaFactory.buildIndexSchema("schema_codec.xml", config); CoreContainer coreContainer = h.getCoreContainer(); diff --git a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java index e3eecfc8cb3..b5b73de2597 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -90,11 +90,10 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept final double[] minUpdateTime = {Double.MAX_VALUE}; final double[] maxUpdateTime = {-1.0}; Set<NamedList<Object>> coreMetrics = new HashSet<>(); - metrics.forEachKey( - (key) -> { + metrics.forEach( + (key, coreMetric) -> { if (key.startsWith("solr.core.testRequestHandlerMetrics")) { - NamedList<Object> coreMetric = (NamedList<Object>) metrics.get(key); - coreMetrics.add(coreMetric); + coreMetrics.add((NamedList<Object>) coreMetric); } }); assertEquals(2, coreMetrics.size()); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerActionTest.java b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerActionTest.java index 235db7af21e..6b5daa5642d 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerActionTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerActionTest.java @@ -17,7 +17,6 @@ package org.apache.solr.handler.admin; import java.util.Locale; -import java.util.Map; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CoreAdminParams; @@ -82,11 +81,11 @@ private void testAction(String action, String propertyName, String propertyValue admin.handleRequestBody(req(CoreAdminParams.ACTION, action), response); - Map<String, Object> actionResponse = ((NamedList<Object>) response.getResponse()).asMap(); + var actionResponse = (NamedList<Object>) response.getResponse(); assertTrue( String.format(Locale.ROOT, "Action response should contain %s property", propertyName), - actionResponse.containsKey(propertyName)); + actionResponse.get(propertyName) != null); assertEquals( String.format( Locale.ROOT, diff --git a/solr/core/src/test/org/apache/solr/search/facet/SpatialHeatmapFacetsTest.java b/solr/core/src/test/org/apache/solr/search/facet/SpatialHeatmapFacetsTest.java index 185e9693860..a385eeeb498 100644 --- a/solr/core/src/test/org/apache/solr/search/facet/SpatialHeatmapFacetsTest.java +++ b/solr/core/src/test/org/apache/solr/search/facet/SpatialHeatmapFacetsTest.java @@ -155,9 +155,11 @@ public void testClassicFacets() throws Exception { // AKA SimpleFacets .findRecursive("facet_counts", "facet_heatmaps", "course", "gridLevel")); assertTrue( ((NamedList<Object>) - response.getResponse().findRecursive("facet_counts", "facet_heatmaps", "course")) - .asMap(0) - .containsKey("counts_" + courseFormat)); + response + .getResponse() + .findRecursive("facet_counts", "facet_heatmaps", "course")) + .indexOf("counts_" + courseFormat, 0) + >= 0); } // ------ Index data diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/LazySolrCluster.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/LazySolrCluster.java index 9b09d063ab2..6dc597ad646 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/LazySolrCluster.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/LazySolrCluster.java @@ -53,6 +53,7 @@ * the value of anything can change any moment Creating an instance is a low cost operation. It does * not result in a network call or large object creation */ +@Deprecated public class LazySolrCluster implements SolrCluster { final ZkStateReader zkStateReader; diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/SimpleZkMap.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/SimpleZkMap.java index 39051da857c..f72d487256d 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/SimpleZkMap.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/SimpleZkMap.java @@ -35,6 +35,7 @@ * #abortableForEach(BiFunction)} to traverse DO not use the {@link #size()} method. It always * return 0 because it is very expensive to compute that */ +@Deprecated public class SimpleZkMap implements SimpleMap<Resource> { private final ZkStateReader zkStateReader; private final String basePath; diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java index dd0b14fd3e1..06903a65a0e 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java @@ -214,7 +214,7 @@ public static PerReplicaStatesOps disable(PerReplicaStates rs) { new PerReplicaStatesOps( prs -> { List<PerReplicaStates.Operation> result = new ArrayList<>(); - prs.states.forEachEntry( + prs.states.forEach( (s, state) -> result.add( new PerReplicaStates.Operation( diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/ApiType.java b/solr/solrj/src/java/org/apache/solr/cluster/api/ApiType.java index 1d4502cd3ce..1945a2039a1 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/ApiType.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/ApiType.java @@ -18,6 +18,7 @@ package org.apache.solr.cluster.api; /** Types of API calls */ +@Deprecated public enum ApiType { V1("solr"), V2("api"); diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/CollectionConfig.java b/solr/solrj/src/java/org/apache/solr/cluster/api/CollectionConfig.java index 020c57fc055..c3043ab50dc 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/CollectionConfig.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/CollectionConfig.java @@ -17,6 +17,7 @@ package org.apache.solr.cluster.api; +@Deprecated public interface CollectionConfig { String name(); diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/HashRange.java b/solr/solrj/src/java/org/apache/solr/cluster/api/HashRange.java index 23d8fab1325..3bfa66eaab6 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/HashRange.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/HashRange.java @@ -18,6 +18,7 @@ package org.apache.solr.cluster.api; /** A range of hash that is stored in a shard */ +@Deprecated public interface HashRange { /** minimum value (inclusive) */ diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/Resource.java b/solr/solrj/src/java/org/apache/solr/cluster/api/Resource.java index 13548f6a822..faa5207838b 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/Resource.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/Resource.java @@ -22,6 +22,7 @@ import org.apache.solr.common.SolrException; /** A binary resource. The impl is agnostic of the content type */ +@Deprecated public interface Resource { /** This is a full path. e.g schema.xml, solrconfig.xml, lang/stopwords.txt etc */ String name(); diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/Router.java b/solr/solrj/src/java/org/apache/solr/cluster/api/Router.java index ae6f6336ba4..ecea98572f2 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/Router.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/Router.java @@ -18,6 +18,7 @@ package org.apache.solr.cluster.api; /** identify shards for a given routing key or document id */ +@Deprecated public interface Router { /** shard name for a given routing key */ diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/Shard.java b/solr/solrj/src/java/org/apache/solr/cluster/api/Shard.java index 0b3cfc15abd..896cf8dd6b4 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/Shard.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/Shard.java @@ -18,6 +18,7 @@ package org.apache.solr.cluster.api; /** A shard of a collection */ +@Deprecated public interface Shard { /** name of the shard */ diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/ShardReplica.java b/solr/solrj/src/java/org/apache/solr/cluster/api/ShardReplica.java index 57b133a0ad6..0e85e51cea7 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/ShardReplica.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/ShardReplica.java @@ -20,6 +20,7 @@ import org.apache.solr.common.cloud.Replica; /** replica of a shard */ +@Deprecated public interface ShardReplica { /** Name of this replica */ String name(); diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/SimpleMap.java b/solr/solrj/src/java/org/apache/solr/cluster/api/SimpleMap.java index 4398d12742f..3684a2862df 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/SimpleMap.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/SimpleMap.java @@ -32,23 +32,27 @@ * efficient to implement and consume. The keys are always {@link CharSequence} objects, The values * can be of any type */ +@Deprecated public interface SimpleMap<T> extends MapWriter { /** get a value by key. If not present , null is returned */ T get(String key); + @Deprecated default T get(String key, T def) { T val = get(key); return val == null ? def : val; } /** Navigate through all keys and values */ + @Deprecated void forEachEntry(BiConsumer<String, ? super T> fun); /** * iterate through all keys The default impl is suboptimal. Proper implementations must do it more * efficiently */ + @Deprecated default void forEachKey(Consumer<String> fun) { forEachEntry((k, t) -> fun.accept(k)); } @@ -62,6 +66,7 @@ default void forEachKey(Consumer<String> fun) { * @param fun Consume each key and return a boolean to signal whether to proceed or not. If true, * continue. If false stop */ + @Deprecated default void abortableForEachKey(Function<String, Boolean> fun) { abortableForEach((key, t) -> fun.apply(key)); } @@ -73,6 +78,7 @@ default void abortableForEachKey(Function<String, Boolean> fun) { * @param fun Consume each entry and return a boolean to signal whether to proceed or not. If * true, continue, if false stop */ + @Deprecated default void abortableForEach(BiFunction<String, ? super T, Boolean> fun) { forEachEntry( new BiConsumer<>() { @@ -91,11 +97,13 @@ default void writeMap(EntryWriter ew) throws IOException { forEachEntry(ew::putNoEx); } + @Deprecated default Map<String, T> asMap(Map<String, T> sink) { forEachEntry(sink::put); return sink; } + @Deprecated default Map<String, T> asMap() { return asMap(new LinkedHashMap<>()); } diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCluster.java b/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCluster.java index 301cd57655b..1fa77d24c8d 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCluster.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCluster.java @@ -20,6 +20,7 @@ import org.apache.solr.common.SolrException; /** Represents a Solr cluster */ +@Deprecated public interface SolrCluster { /** collections in the cluster */ diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCollection.java b/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCollection.java index df9007e3b7f..c0dd86d047a 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCollection.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/SolrCollection.java @@ -18,6 +18,7 @@ package org.apache.solr.cluster.api; /** Represents a collection in Solr */ +@Deprecated public interface SolrCollection { String name(); diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/SolrNode.java b/solr/solrj/src/java/org/apache/solr/cluster/api/SolrNode.java index 13115643c12..e6db68a1470 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/SolrNode.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/SolrNode.java @@ -18,6 +18,7 @@ package org.apache.solr.cluster.api; /** A read only view of a Solr node */ +@Deprecated public interface SolrNode { /** The node name */ diff --git a/solr/solrj/src/java/org/apache/solr/cluster/api/package-info.java b/solr/solrj/src/java/org/apache/solr/cluster/api/package-info.java index 9c4183d49da..84e7f43b356 100644 --- a/solr/solrj/src/java/org/apache/solr/cluster/api/package-info.java +++ b/solr/solrj/src/java/org/apache/solr/cluster/api/package-info.java @@ -16,4 +16,5 @@ */ /** API interfaces for core SolrCloud classes */ +@Deprecated package org.apache.solr.cluster.api; diff --git a/solr/solrj/src/java/org/apache/solr/common/ConfigNode.java b/solr/solrj/src/java/org/apache/solr/common/ConfigNode.java index be86c1e5c7b..18022605f69 100644 --- a/solr/solrj/src/java/org/apache/solr/common/ConfigNode.java +++ b/solr/solrj/src/java/org/apache/solr/common/ConfigNode.java @@ -26,12 +26,11 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; -import org.apache.solr.cluster.api.SimpleMap; -import org.apache.solr.common.util.WrappedSimpleMap; /** * A generic interface that represents a config file, mostly XML Please note that this is an @@ -44,7 +43,7 @@ public interface ConfigNode { String name(); /** Attributes */ - SimpleMap<String> attributes(); + Map<String, String> attributes(); /** Child by name */ default ConfigNode child(String name) { @@ -195,8 +194,8 @@ public String txt() { } @Override - public SimpleMap<String> attributes() { - return empty_attrs; + public Map<String, String> attributes() { + return Map.of(); } @Override @@ -233,8 +232,6 @@ public boolean isNull() { public void forEachChild(Function<ConfigNode, Boolean> fun) {} }; - SimpleMap<String> empty_attrs = new WrappedSimpleMap<>(Collections.emptyMap()); - class Helpers { static boolean _bool(Object v, boolean def) { return v == null ? def : Boolean.parseBoolean(v.toString()); diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java b/solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java index 3c8ff93b816..82265675224 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java @@ -24,7 +24,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import org.apache.solr.cluster.api.HashRange; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.cloud.DocCollection.CollectionStateProps; @@ -89,7 +88,7 @@ public static Map<String, Object> getRouterSpec(ZkNodeProps props) { // Hash ranges can't currently "wrap" - i.e. max must be greater or equal to min. // TODO: ranges may not be all contiguous in the future (either that or we will // need an extra class to model a collection of ranges) - public static class Range implements JSONWriter.Writable, Comparable<Range>, HashRange { + public static class Range implements JSONWriter.Writable, Comparable<Range> { public int min; // inclusive public int max; // inclusive @@ -99,17 +98,14 @@ public Range(int min, int max) { this.max = max; } - @Override public int min() { return min; } - @Override public int max() { return max; } - @Override public boolean includes(int hash) { return hash >= min && hash <= max; } diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java b/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java index 60616d8fd70..961f3efc50d 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java @@ -31,14 +31,12 @@ import java.util.Objects; import java.util.Set; import java.util.function.BiConsumer; -import org.apache.solr.cluster.api.SimpleMap; import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.MapWriter; import org.apache.solr.common.annotation.JsonProperty; import org.apache.solr.common.cloud.Replica.ReplicaStateProps; import org.apache.solr.common.util.ReflectMapWriter; import org.apache.solr.common.util.StrUtils; -import org.apache.solr.common.util.WrappedSimpleMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,7 +57,7 @@ public class PerReplicaStates implements ReflectMapWriter { @JsonProperty public final int cversion; // states of individual replicas - @JsonProperty public final SimpleMap<State> states; + @JsonProperty public final Map<String, State> states; private volatile Boolean allActive; @@ -85,7 +83,7 @@ public PerReplicaStates(String path, int cversion, List<String> states) { tmp.put(rs.replica, rs.insert(existing)); } } - this.states = new WrappedSimpleMap<>(tmp); + this.states = tmp; } public static PerReplicaStates empty(String collectionName) { @@ -95,27 +93,23 @@ public static PerReplicaStates empty(String collectionName) { /** Check and return if all replicas are ACTIVE */ public boolean allActive() { if (this.allActive != null) return allActive; - boolean[] result = new boolean[] {true}; - states.forEachEntry( - (r, s) -> { - if (s.state != Replica.State.ACTIVE) result[0] = false; - }); - return this.allActive = result[0]; + this.allActive = states.values().stream().allMatch(s -> s.state == Replica.State.ACTIVE); + return allActive; } /** Get the changed replicas */ public static Set<String> findModifiedReplicas(PerReplicaStates old, PerReplicaStates fresh) { Set<String> result = new HashSet<>(); if (fresh == null) { - old.states.forEachKey(result::add); + result.addAll(old.states.keySet()); return result; } - old.states.forEachEntry( + old.states.forEach( (s, state) -> { // the state is modified or missing if (!Objects.equals(fresh.get(s), state)) result.add(s); }); - fresh.states.forEachEntry( + fresh.states.forEach( (s, state) -> { if (old.get(s) == null) result.add(s); }); @@ -165,8 +159,8 @@ public String toString() { } private StringBuilder appendStates(StringBuilder sb) { - states.forEachEntry( - new BiConsumer<String, State>() { + states.forEach( + new BiConsumer<>() { int count = 0; @Override @@ -318,7 +312,7 @@ public EntryWriter put(CharSequence k, Object v) throws IOException { ew.put( "states", (IteratorWriter) - iw -> states.forEachEntry((s, state) -> iw.addNoEx(state.toString()))); + iw -> states.forEach((s, state) -> iw.addNoEx(state.toString()))); } else { ew.put(k, v); } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/DOMUtil.java b/solr/solrj/src/java/org/apache/solr/common/util/DOMUtil.java index 840c64b5d87..0ef6bfbe538 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/DOMUtil.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/DOMUtil.java @@ -50,7 +50,7 @@ public static Map<String, String> toMap(ConfigNode node) { public static Map<String, String> toMapExcept(ConfigNode node, String... exclusions) { Map<String, String> args = new HashMap<>(); node.attributes() - .forEachEntry( + .forEach( (k, v) -> { for (String ex : exclusions) if (ex.equals(k)) return; args.put(k, v); 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 aef0edaa4cf..6a34479fe16 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 @@ -253,6 +253,12 @@ public T get(String name) { return get(name, 0); } + /** Like {@link #get(String)} but returns a default value if it would be null. */ + public T getOrDefault(String name, T def) { + T val = get(name); + return val == null ? def : val; + } + /** * Gets the value for the first instance of the specified name found starting at the specified * index. @@ -486,7 +492,7 @@ public Set<Entry<String, T>> entrySet() { @Override public void forEach(BiConsumer action) { - NamedList.this.forEachEntry(action); + NamedList.this.forEach(action); } }; } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/WrappedSimpleMap.java b/solr/solrj/src/java/org/apache/solr/common/util/WrappedSimpleMap.java index 417e8d3c968..768fef788b9 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/WrappedSimpleMap.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/WrappedSimpleMap.java @@ -22,6 +22,7 @@ import java.util.function.BiConsumer; import org.apache.solr.cluster.api.SimpleMap; +@Deprecated public class WrappedSimpleMap<T> implements SimpleMap<T> { private final Map<String, T> delegate; diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java index 3512e74c6f3..feeab8ab198 100644 --- a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java @@ -159,7 +159,7 @@ public void testRestart() throws Exception { PerReplicaStatesOps.fetch(collectionPath, SolrCloudTestCase.cluster.getZkClient(), null); assertEquals(2, prs.states.size()); c = cluster.getZkStateReader().getCollection(testCollection); - prs.states.forEachEntry((s, state) -> assertEquals(Replica.State.ACTIVE, state.state)); + prs.states.forEach((s, state) -> assertEquals(Replica.State.ACTIVE, state.state)); String replicaName = null; for (Replica r : c.getSlice("shard1").getReplicas()) { @@ -215,7 +215,7 @@ public void testRestart() throws Exception { prs = PerReplicaStatesOps.fetch( collectionPath, SolrCloudTestCase.cluster.getZkClient(), null); - prs.states.forEachEntry((s, state) -> assertEquals(Replica.State.ACTIVE, state.state)); + prs.states.forEach((s, state) -> assertEquals(Replica.State.ACTIVE, state.state)); } } finally { @@ -275,7 +275,7 @@ public void testMultipleTransitions() throws Exception { PerReplicaStates prs2 = PerReplicaStatesOps.fetch( DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); - prs2.states.forEachEntry( + prs2.states.forEach( (r, newState) -> { if (newState.getDuplicate() != null) anyFail.set(true); });