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.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 void testAllNodes() throws Exception { 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.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 @@ private void executeReq( 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) o); - throw new RuntimeException("Unknown response"); - }, + () -> + NavigableObject.wrap( + Utils.executeGET(client.getHttpClient(), jetty.getBaseUrl() + uri, parser)), expected); } } @@ -634,10 +630,9 @@ public void testAPI() throws Exception { TestDistribFileStore.assertResponseValues( 1, () -> - new MapWriterMap( - (Map) - 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 void testAPI() throws Exception { TestDistribFileStore.assertResponseValues( 1, () -> - new MapWriterMap( - (Map) - 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 void testAPI() throws Exception { TestDistribFileStore.assertResponseValues( 1, () -> - new MapWriterMap( - (Map) - 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 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 @@ private Map fetchProps(String nodeName, Collection tags) 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 ed9da582fdc..d0e52f6892f 100644 --- a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java +++ b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java @@ -152,5 +152,6 @@ default BiConsumer getBiConsumer() { } } + @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.util.List; import java.util.Map; +@Deprecated // see NavigableMap.wrap. May keep but use package scope. public class MapWriterMap implements MapWriter { private final Map 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..9fa468fa1cf 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 @@ default int _size() { _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 navObj) return navObj; + if (obj instanceof Map m) return new MapWriterMap((Map) m); + 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 253c97f275b..584fcadccc1 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 @@ -80,7 +80,6 @@ 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; @@ -533,12 +532,8 @@ public static Object getObjectByPath(Object root, boolean onlyPrimitive, List) { o = getVal(o, null, idx); - } else if (o instanceof Map) { - @SuppressWarnings("unchecked") - Map map = (Map) o; - o = getVal(new MapWriterMap(map), null, idx); } else { return null; } @@ -604,32 +599,41 @@ private static boolean isMapLike(Object o) { 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 m) { + 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 mapWriter) { Object[] result = new Object[1]; try { - ((MapWriter) obj) - .writeMap( - new MapWriter.EntryWriter() { - int count = -1; - - @Override - public MapWriter.EntryWriter put(CharSequence k, Object v) { - if (result[0] != null) return this; - if (idx < 0) { - if (key.contentEquals(k)) result[0] = v; - } else { - if (++count == idx) result[0] = new MapWriterEntry<>(k, v); - } - return this; - } - }); + mapWriter.writeMap( + new MapWriter.EntryWriter() { + int count = -1; + + @Override + public MapWriter.EntryWriter put(CharSequence k, Object v) { + if (result[0] != null) return this; + if (idx < 0) { + if (key.contentEquals(k)) result[0] = v; + } else { + if (++count == idx) result[0] = new MapWriterEntry<>(k, v); + } + return this; + } + }); } catch (IOException e) { 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"); } /** 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 22d11e2f0b0..25e2653a2f8 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 @@ -22,7 +22,6 @@ import java.lang.invoke.MethodHandles; import java.util.Collections; -import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.solr.client.solrj.SolrClient; @@ -32,7 +31,6 @@ import org.apache.solr.client.solrj.response.SolrPingResponse; 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.util.Utils; import org.apache.solr.embedded.JettySolrRunner; @@ -190,17 +188,13 @@ public void testRestart() throws Exception { PerReplicaStates.State st = prs.get(replicaName); assertNotEquals(Replica.State.ACTIVE, st.state); @SuppressWarnings("unchecked") - NavigableObject rsp = - new MapWriterMap( - (Map) - Utils.fromJSON( - SolrCloudTestCase.cluster - .getZkClient() - .getData( - DocCollection.getCollectionPath(testCollection), - null, - null, - true))); + var rsp = + NavigableObject.wrap( + Utils.fromJSON( + SolrCloudTestCase.cluster + .getZkClient() + .getData( + DocCollection.getCollectionPath(testCollection), null, null, true))); assertNull( rsp._get(