Skip to content

Commit

Permalink
NavigableObject.wrap. Deprecate MapWriter.Empty and MapWriterMap.
Browse files Browse the repository at this point in the history
  • Loading branch information
@dsmiley
dsmiley committed Jan 31, 2025
1 parent af7e9a9 commit c07e639
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 64 deletions.
6 changes: 2 additions & 4 deletions solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
31 changes: 12 additions & 19 deletions solr/core/src/test/org/apache/solr/pkg/TestPackages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, Object>) o);
throw new RuntimeException("Unknown response");
},
() ->
NavigableObject.wrap(
Utils.executeGET(client.getHttpClient(), jetty.getBaseUrl() + uri, parser)),
expected);
}
}
Expand Down Expand Up @@ -634,10 +630,9 @@ public void testAPI() throws Exception {
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
Expand All @@ -658,10 +653,9 @@ public void testAPI() throws Exception {
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
Expand All @@ -684,10 +678,9 @@ public void testAPI() throws Exception {
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
Expand Down
3 changes: 1 addition & 2 deletions solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -92,7 +91,7 @@ private Map<String, Object> fetchProps(String nodeName, Collection<String> 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) {
Expand Down
1 change: 1 addition & 0 deletions solr/solrj/src/java/org/apache/solr/common/MapWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,5 +152,6 @@ default BiConsumer<CharSequence, Object> getBiConsumer() {
}
}

@Deprecated // use SimpleOrderedMap.of()
MapWriter EMPTY = new MapWriterMap(Collections.emptyMap());
}
1 change: 1 addition & 0 deletions solr/solrj/src/java/org/apache/solr/common/MapWriterMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> delegate;

Expand Down
11 changes: 11 additions & 0 deletions solr/solrj/src/java/org/apache/solr/common/NavigableObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<String, Object>) m);
throw new IllegalArgumentException("Cannot wrap " + obj.getClass());
}
}
56 changes: 30 additions & 26 deletions solr/solrj/src/java/org/apache/solr/common/util/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -533,12 +532,8 @@ public static Object getObjectByPath(Object root, boolean onlyPrimitive, List<St
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;
}
Expand Down Expand Up @@ -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");
}

/**
Expand Down
20 changes: 7 additions & 13 deletions solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, Object>)
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(
Expand Down

0 comments on commit c07e639

Please sign in to comment.