This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 05effa82a90adf527a8113aa3807ea7fb7e434ba Author: David Smiley <[email protected]> AuthorDate: Wed Feb 12 17:12:42 2025 -0500 Add NavigableObject.wrap. Deprecate MapWriter.EMPTY and MapWriterMap. (#3148) Introduce NavigableObject.wrap to coerce its arg to a NavigableObject. Deprecate MapWriter.EMPTY; don't need it anymore (due to the above); was only used once. Enhance Utils to not need MapWriterMap for its functions, and to slightly optimize for SimpleOrderedMap now being a Map. Deprecate MapWriterMap. Ultimately may simply make package scope and maybe rename to orient itself around its NavigableObject-ness, which is what it's only used for, after the changes here. (cherry picked from commit d911c1a844a1e3cb45d43af8d6c622b507118e9c) --- .../org/apache/solr/cloud/BalanceReplicasTest.java | 6 ++--- .../src/test/org/apache/solr/pkg/TestPackages.java | 31 +++++++++------------- .../solr/common/cloud/NodesSysPropsCacher.java | 3 +-- .../src/java/org/apache/solr/common/MapWriter.java | 1 + .../java/org/apache/solr/common/MapWriterMap.java | 1 + .../org/apache/solr/common/NavigableObject.java | 11 ++++++++ .../java/org/apache/solr/common/util/Utils.java | 28 +++++++++++-------- 7 files changed, 45 insertions(+), 36 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java b/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java index a14b372df2e..b93d5bc3f34 100644 --- a/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java @@ -37,9 +37,9 @@ import org.apache.solr.client.api.model.BalanceReplicasRequestBody; import org.apache.solr.client.solrj.impl.CloudLegacySolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; -import org.apache.solr.common.MapWriterMap; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; import org.apache.solr.common.util.Utils; import org.junit.Before; @@ -100,9 +100,7 @@ public class BalanceReplicasTest extends SolrCloudTestCase { log.debug("### Before balancing: {}", collection); postDataAndGetResponse( - cluster.getSolrClient(), - "/api/cluster/replicas/balance", - new MapWriterMap(Collections.emptyMap())); + cluster.getSolrClient(), "/api/cluster/replicas/balance", SimpleOrderedMap.of()); collection = cloudClient.getClusterState().getCollectionOrNull(coll, false); log.debug("### After balancing: {}", collection); diff --git a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java index 7c6069f2220..78de0ee5199 100644 --- a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java +++ b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java @@ -57,7 +57,6 @@ import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.util.ClientUtils; import org.apache.solr.cloud.MiniSolrCloudCluster; import org.apache.solr.cloud.SolrCloudTestCase; -import org.apache.solr.common.MapWriterMap; import org.apache.solr.common.NavigableObject; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.annotation.JsonProperty; @@ -540,12 +539,9 @@ public class TestPackages extends SolrCloudTestCase { try (HttpSolrClient client = (HttpSolrClient) jetty.newClient()) { TestDistribFileStore.assertResponseValues( 10, - () -> { - Object o = Utils.executeGET(client.getHttpClient(), jetty.getBaseUrl() + uri, parser); - if (o instanceof NavigableObject) return (NavigableObject) o; - if (o instanceof Map) return new MapWriterMap((Map<String, Object>) o); - throw new RuntimeException("Unknown response"); - }, + () -> + NavigableObject.wrap( + Utils.executeGET(client.getHttpClient(), jetty.getBaseUrl() + uri, parser)), expected); } } @@ -634,10 +630,9 @@ public class TestPackages extends SolrCloudTestCase { TestDistribFileStore.assertResponseValues( 1, () -> - new MapWriterMap( - (Map<String, Object>) - Utils.fromJSON( - cluster.getZkClient().getData(SOLR_PKGS_PATH, null, new Stat(), true))), + NavigableObject.wrap( + Utils.fromJSON( + cluster.getZkClient().getData(SOLR_PKGS_PATH, null, new Stat(), true))), Map.of(":packages:test_pkg[0]:version", "0.12", ":packages:test_pkg[0]:files[0]", FILE2)); // post a new jar with a proper signature @@ -658,10 +653,9 @@ public class TestPackages extends SolrCloudTestCase { TestDistribFileStore.assertResponseValues( 1, () -> - new MapWriterMap( - (Map<String, Object>) - Utils.fromJSON( - cluster.getZkClient().getData(SOLR_PKGS_PATH, null, new Stat(), true))), + NavigableObject.wrap( + Utils.fromJSON( + cluster.getZkClient().getData(SOLR_PKGS_PATH, null, new Stat(), true))), Map.of(":packages:test_pkg[1]:version", "0.13", ":packages:test_pkg[1]:files[0]", FILE3)); // Now we will just delete one version @@ -684,10 +678,9 @@ public class TestPackages extends SolrCloudTestCase { TestDistribFileStore.assertResponseValues( 1, () -> - new MapWriterMap( - (Map<String, Object>) - Utils.fromJSON( - cluster.getZkClient().getData(SOLR_PKGS_PATH, null, new Stat(), true))), + NavigableObject.wrap( + Utils.fromJSON( + cluster.getZkClient().getData(SOLR_PKGS_PATH, null, new Stat(), true))), Map.of(":packages:test_pkg[0]:version", "0.13", ":packages:test_pkg[0]:files[0]", FILE3)); // So far we have been verifying the details with ZK directly diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java index 37699901323..575c9edcf2a 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java @@ -24,7 +24,6 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.request.GenericSolrRequest; -import org.apache.solr.common.MapWriter; import org.apache.solr.common.NavigableObject; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; @@ -92,7 +91,7 @@ public class NodesSysPropsCacher implements NodesSysProps, AutoCloseable { solrClient .requestWithBaseUrl(zkStateReader.getBaseUrlForNodeName(nodeName), null, req) .getResponse(); - NavigableObject metrics = (NavigableObject) response._get("metrics", MapWriter.EMPTY); + var metrics = NavigableObject.wrap(response._get("metrics", null)); keys.forEach((tag, key) -> result.put(tag, metrics._get(key, null))); return result; } catch (Exception e) { diff --git a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java index 247460bbc89..b2d79018c76 100644 --- a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java +++ b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java @@ -159,5 +159,6 @@ public interface MapWriter extends MapSerializable, NavigableObject, JSONWriter. } } + @Deprecated // use SimpleOrderedMap.of() MapWriter EMPTY = new MapWriterMap(Collections.emptyMap()); } diff --git a/solr/solrj/src/java/org/apache/solr/common/MapWriterMap.java b/solr/solrj/src/java/org/apache/solr/common/MapWriterMap.java index 2f3735e81df..e8528bfd51b 100644 --- a/solr/solrj/src/java/org/apache/solr/common/MapWriterMap.java +++ b/solr/solrj/src/java/org/apache/solr/common/MapWriterMap.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +@Deprecated // see NavigableMap.wrap. May keep but use package scope. public class MapWriterMap implements MapWriter { private final Map<String, Object> delegate; diff --git a/solr/solrj/src/java/org/apache/solr/common/NavigableObject.java b/solr/solrj/src/java/org/apache/solr/common/NavigableObject.java index 7241f792aaa..492ec9c14c6 100644 --- a/solr/solrj/src/java/org/apache/solr/common/NavigableObject.java +++ b/solr/solrj/src/java/org/apache/solr/common/NavigableObject.java @@ -18,7 +18,9 @@ package org.apache.solr.common; import java.util.List; +import java.util.Map; import java.util.function.BiConsumer; +import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.Utils; /** @@ -96,4 +98,13 @@ public interface NavigableObject { _forEachEntry((k, v) -> size[0]++); return size[0]; } + + /** Casts or wraps the argument into a NavigableObject if possible, never returning null. */ + @SuppressWarnings("unchecked") + static NavigableObject wrap(Object obj) { + if (obj == null) return SimpleOrderedMap.of(); + if (obj instanceof NavigableObject) return (NavigableObject) obj; + if (obj instanceof Map<?, ?>) return new MapWriterMap((Map<String, Object>) obj); + throw new IllegalArgumentException("Cannot wrap " + obj.getClass()); + } } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index 9d69b9d9fb0..552d62d9a08 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -75,7 +75,6 @@ import org.apache.http.util.EntityUtils; import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.LinkedHashMapWriter; import org.apache.solr.common.MapWriter; -import org.apache.solr.common.MapWriterMap; import org.apache.solr.common.SolrException; import org.apache.solr.common.SpecProvider; import org.apache.solr.common.annotation.JsonProperty; @@ -518,12 +517,8 @@ public class Utils { o = idx < l.size() ? l.get(idx) : null; } else if (o instanceof IteratorWriter) { o = getValueAt((IteratorWriter) o, idx); - } else if (o instanceof MapWriter) { + } else if (o instanceof MapWriter || o instanceof Map<?, ?>) { o = getVal(o, null, idx); - } else if (o instanceof Map) { - @SuppressWarnings("unchecked") - Map<String, Object> map = (Map<String, Object>) o; - o = getVal(new MapWriterMap(map), null, idx); } else { return null; } @@ -589,11 +584,23 @@ public class Utils { return o instanceof Map || o instanceof NamedList || o instanceof MapWriter; } - private static Object getVal(Object obj, String key, int idx) { - if (obj instanceof MapWriter) { + /** Extract either the key or index from mapLike. */ + private static Object getVal(Object mapLike, String key, int idx) { + assert (key == null && idx >= 0) || (key != null && idx == -1); + if (mapLike instanceof Map<?, ?>) { + var m = (Map<?, ?>) mapLike; + if (key != null) { + return m.get(key); + } else { + var optEntry = m.entrySet().stream().skip(idx).findFirst(); + return optEntry + .map(entry -> new MapWriterEntry<>(entry.getKey().toString(), entry.getValue())) + .orElse(null); + } + } else if (mapLike instanceof MapWriter) { Object[] result = new Object[1]; try { - ((MapWriter) obj) + ((MapWriter) mapLike) .writeMap( new MapWriter.EntryWriter() { int count = -1; @@ -613,8 +620,7 @@ public class Utils { throw new RuntimeException(e); } return result[0]; - } else if (obj instanceof Map) return ((Map<?, ?>) obj).get(key); - else throw new RuntimeException("must be a NamedList or Map"); + } else throw new RuntimeException("must be a NamedList or Map"); } /**
