From c80d41597c83c8c73634144ab8c55f8002dd101d Mon Sep 17 00:00:00 2001 From: David Smiley 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. --- solr/CHANGES.txt | 4 + .../src/java/org/apache/solr/cli/ApiTool.java | 2 +- .../java/org/apache/solr/cli/CLIUtils.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 +- 40 files changed, 175 insertions(+), 105 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 40aca459802..7e057e943c4 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -300,6 +300,10 @@ led to the suppression of exceptions. (Andrey Bozhko) * SOLR-17577: Remove "solr.indexfetcher.sotimeout" system property that was for optimizing replication tests. It was disabled, but not removed. (Eric Pugh) +* 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 ede4de68971..c187360061f 100644 --- a/solr/core/src/java/org/apache/solr/cli/ApiTool.java +++ b/solr/core/src/java/org/apache/solr/cli/ApiTool.java @@ -98,7 +98,7 @@ protected String callGet(String url, String credentials) throws Exception { NamedList 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/CLIUtils.java b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java index 7f4c0dc648b..b3336eb2361 100644 --- a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java +++ b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java @@ -328,7 +328,7 @@ public static boolean safeCheckCoreExists(String solrUrl, String coreName, Strin Map failureStatus = (Map) existsCheckResult.get("initFailures"); String errorMsg = (String) failureStatus.get(coreName); - final boolean hasName = coreStatus != null && coreStatus.asMap().containsKey(NAME); + final boolean hasName = coreStatus != null && coreStatus.get(NAME) != null; exists = hasName || errorMsg != null; wait = hasName && errorMsg == null && "true".equals(coreStatus.get("isLoading")); } while (wait && System.nanoTime() - startWaitAt < MAX_WAIT_FOR_CORE_LOAD_NANOS); 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 b1607af071b..8c46bc13c4f 100644 --- a/solr/core/src/java/org/apache/solr/cli/CreateTool.java +++ b/solr/core/src/java/org/apache/solr/cli/CreateTool.java @@ -22,7 +22,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.commons.cli.CommandLine; @@ -167,10 +166,9 @@ protected void createCore(CommandLine cli, SolrClient solrClient) throws Excepti String coreRootDirectory; // usually same as solr home, but not always - Map 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"); @@ -321,7 +319,7 @@ protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine cli if (isVerbose()) { // 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 7fc94cb85d3..c06e5ddf63c 100644 --- a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java +++ b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java @@ -211,7 +211,7 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli if (isVerbose() && 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 45713fbc085..fa6c2cbf2bd 100644 --- a/solr/core/src/java/org/apache/solr/cli/StatusTool.java +++ b/solr/core/src/java/org/apache/solr/cli/StatusTool.java @@ -346,13 +346,14 @@ protected Map getCloudStatus(SolrClient solrClient, String zkHos Map cloudStatus = new LinkedHashMap<>(); cloudStatus.put("ZooKeeper", (zkHost != null) ? zkHost : "?"); + // TODO add booleans to request just what we want; not everything NamedList json = solrClient.request(new CollectionAdminRequest.ClusterStatus()); List liveNodes = (List) json.findRecursive("cluster", "live_nodes"); cloudStatus.put("liveNodes", String.valueOf(liveNodes.size())); - Map collections = - ((NamedList) json.findRecursive("cluster", "collections")).asMap(); + // TODO get this as a metric from the metrics API instead, or something else. + var collections = (NamedList) 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 attributes() { + public Map 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 c091beea08e..2e1991973df 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 da4df722c59..3525fd2cc95 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; @@ -131,10 +132,10 @@ public static NodeConfig fromConfig( // since it is arranged as a separate section it is placed here Map coreAdminHandlerActions = readNodeListAsNamedList(root.get("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) { @@ -733,7 +734,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 9cd757571fd..6f4f131de8f 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 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 8e0126073d2..60615a32f49 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 attrs; + Map attrs; @Override public String name() { @@ -50,10 +47,10 @@ public DOMConfigNode(Node node) { } @Override - public SimpleMap attributes() { + public Map attributes() { if (attrs != null) return attrs; Map 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 fun) { if (Boolean.FALSE.equals(toContinue)) break; } } - - private static final SimpleMap 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 attributes; - public final SimpleMap> kids; + public final Map attributes; + public final Map> kids; public final String textData; public DataConfigNode(ConfigNode root) { Map> 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 wrap(SimpleMap delegate) { + /** provides a substitute view, and read-only */ + private static Map wrapSubstituting(Map 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 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 attributes() { + public Map attributes() { return attributes; } @@ -104,7 +90,7 @@ public ConfigNode child(String name) { @Override public List getAll(String name) { - return kids.get(name, Collections.emptyList()); + return kids.getOrDefault(name, List.of()); } @Override @@ -126,7 +112,7 @@ public List getAll(Predicate test, Set matchName @Override public void forEachChild(Function fun) { - kids.forEachEntry( + kids.forEach( (s, configNodes) -> { if (configNodes != null) { configNodes.forEach(fun::apply); @@ -134,6 +120,74 @@ public void forEachChild(Function fun) { }); } - public static final SimpleMap> EMPTY = - new WrappedSimpleMap<>(Collections.emptyMap()); + private static class SubstitutingMap extends AbstractMap { + + private final Map delegate; + + SubstitutingMap(Map 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 keySet() { + return delegate.keySet(); + } + + @Override + public void forEach(BiConsumer action) { + delegate.forEach((k, v) -> action.accept(k, substituteVal(v))); + } + + @Override + public Set> entrySet() { + return new AbstractSet<>() { + @Override + public Iterator> iterator() { + // using delegate, return an iterator using Streams + return delegate.entrySet().stream() + .map(entry -> (Entry) new SubstitutingEntry(entry)) + .iterator(); + } + + @Override + public int size() { + return delegate.size(); + } + }; + } + + private static class SubstitutingEntry implements Entry { + + private final Entry delegateEntry; + + SubstitutingEntry(Entry 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> coreMetrics = new HashSet<>(); - metrics.forEachKey( - (key) -> { + metrics.forEach( + (key, coreMetric) -> { if (key.startsWith("solr.core.testRequestHandlerMetrics")) { - NamedList coreMetric = (NamedList) metrics.get(key); - coreMetrics.add(coreMetric); + coreMetrics.add((NamedList) 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 actionResponse = ((NamedList) response.getResponse()).asMap(); + var actionResponse = (NamedList) 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) - 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 { 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 85fcef00e83..781289df2f7 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 @@ -212,7 +212,7 @@ public static PerReplicaStatesOps disable(PerReplicaStates rs) { new PerReplicaStatesOps( prs -> { List 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 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 fun); /** * iterate through all keys The default impl is suboptimal. Proper implementations must do it more * efficiently */ + @Deprecated default void forEachKey(Consumer fun) { forEachEntry((k, t) -> fun.accept(k)); } @@ -62,6 +66,7 @@ default void forEachKey(Consumer 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 fun) { abortableForEach((key, t) -> fun.apply(key)); } @@ -73,6 +78,7 @@ default void abortableForEachKey(Function 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 fun) { forEachEntry( new BiConsumer<>() { @@ -91,11 +97,13 @@ default void writeMap(EntryWriter ew) throws IOException { forEachEntry(ew::putNoEx); } + @Deprecated default Map asMap(Map sink) { forEachEntry(sink::put); return sink; } + @Deprecated default Map 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 attributes(); + Map attributes(); /** Child by name */ default ConfigNode child(String name) { @@ -195,8 +194,8 @@ public String txt() { } @Override - public SimpleMap attributes() { - return empty_attrs; + public Map attributes() { + return Map.of(); } @Override @@ -233,8 +232,6 @@ public boolean isNull() { public void forEachChild(Function fun) {} }; - SimpleMap 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 2ac50b6b1f5..d3a533b07d0 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 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, HashRange { + public static class Range implements JSONWriter.Writable, Comparable { 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 e8e95d4e8ea..783729e2edf 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 states; + @JsonProperty public final Map states; private volatile Boolean allActive; @@ -85,7 +83,7 @@ public PerReplicaStates(String path, int cversion, List 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 findModifiedReplicas(PerReplicaStates old, PerReplicaStates fresh) { Set 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() { + states.forEach( + new BiConsumer<>() { int count = 0; @Override @@ -317,7 +311,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 toMap(ConfigNode node) { public static Map toMapExcept(ConfigNode node, String... exclusions) { Map 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 e443414c5b1..123a5e81d91 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> 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 implements SimpleMap { private final Map 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 21a5307eb67..71336a8088d 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 @@ -164,7 +164,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()) { @@ -226,7 +226,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 { @@ -286,7 +286,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); });