You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2023/08/23 21:48:01 UTC
[solr] branch branch_9x updated: SOLR-16933: Include full query response in API tool (#1863)
This is an automated email from the ASF dual-hosted git repository.
houston pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 5daca6a3a5e SOLR-16933: Include full query response in API tool (#1863)
5daca6a3a5e is described below
commit 5daca6a3a5e53c11910119cd26a8d5392045ed14
Author: Houston Putman <ho...@apache.org>
AuthorDate: Wed Aug 23 15:40:27 2023 -0400
SOLR-16933: Include full query response in API tool (#1863)
(cherry picked from commit f5b0d22f06363a06db25368f70e0d429a0e3b192)
---
solr/CHANGES.txt | 2 +
.../core/src/java/org/apache/solr/cli/ApiTool.java | 54 +++++++++++-------
.../org/apache/solr/cli/CreateCollectionTool.java | 7 +++
.../src/java/org/apache/solr/cli/DeleteTool.java | 12 ++--
.../src/test/org/apache/solr/cli/ApiToolTest.java | 64 +++++++++++++++++++++-
.../solr/client/solrj/impl/NodeValueFetcher.java | 2 +-
.../solrj/impl/SolrClientNodeStateProvider.java | 5 +-
.../solr/client/solrj/impl/Http2SolrClient.java | 8 ++-
.../org/apache/solr/common/IteratorWriter.java | 2 +-
.../org/apache/solr/common/SolrDocumentList.java | 21 ++++++-
.../java/org/apache/solr/common/util/Utils.java | 10 +++-
solr/solrj/src/java/org/noggit/JSONWriter.java | 21 ++++++-
12 files changed, 171 insertions(+), 37 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 7ff58891334..663e8665eb8 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -78,6 +78,8 @@ Bug Fixes
* SOLR-16946: Updated Cluster Singleton plugins are stopped correctly when the Overseer is closed. (Paul McArthur)
+* SOLR-16933: Include the full query response when using the API tool, and fix serialization issues for SolrDocumentList. (Houston Putman)
+
Dependency Upgrades
---------------------
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 c4ecce4a639..bf12b98d443 100644
--- a/solr/core/src/java/org/apache/solr/cli/ApiTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/ApiTool.java
@@ -23,6 +23,7 @@ import java.util.List;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.impl.JsonMapResponseParser;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.NamedList;
@@ -57,29 +58,42 @@ public class ApiTool extends ToolBase {
@Override
public void runImpl(CommandLine cli) throws Exception {
+ String response = null;
String getUrl = cli.getOptionValue("get");
if (getUrl != null) {
- getUrl = getUrl.replace("+", "%20");
- URI uri = new URI(getUrl);
- String solrUrl = getSolrUrlFromUri(uri);
- String path = uri.getPath();
- try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
- NamedList<Object> response =
- solrClient.request(
- // For path parameter we need the path without the root so from the second / char
- // (because root can be configured)
- // E.g URL is http://localhost:8983/solr/admin/info/system path is
- // /solr/admin/info/system and the path without root is /admin/info/system
- new GenericSolrRequest(
- SolrRequest.METHOD.GET,
- path.substring(path.indexOf("/", path.indexOf("/") + 1)),
- getSolrParamsFromUri(uri)));
+ response = callGet(getUrl);
+ }
+ if (response != null) {
+ // pretty-print the response to stdout
+ echo(response);
+ }
+ }
- // pretty-print the response to stdout
- CharArr arr = new CharArr();
- new JSONWriter(arr, 2).write(response.asMap());
- echo(arr.toString());
- }
+ protected String callGet(String url) throws Exception {
+ URI uri = new URI(url.replace("+", "%20"));
+ String solrUrl = getSolrUrlFromUri(uri);
+ String path = uri.getPath();
+ try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
+ // For path parameter we need the path without the root so from the second / char
+ // (because root can be configured)
+ // E.g URL is http://localhost:8983/solr/admin/info/system path is
+ // /solr/admin/info/system and the path without root is /admin/info/system
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ path.substring(path.indexOf("/", path.indexOf("/") + 1)),
+ getSolrParamsFromUri(uri) // .add("indent", "true")
+ );
+ // Using the "smart" solr parsers won't work, because they decode into Solr objects.
+ // When trying to re-write into JSON, the JSONWriter doesn't have the right info to print it
+ // correctly.
+ // All we want to do is pass the JSON response to the user, so do that.
+ req.setResponseParser(new JsonMapResponseParser());
+ NamedList<Object> response = solrClient.request(req);
+ // pretty-print the response to stdout
+ CharArr arr = new CharArr();
+ new JSONWriter(arr, 2).write(response.asMap());
+ return arr.toString();
}
}
diff --git a/solr/core/src/java/org/apache/solr/cli/CreateCollectionTool.java b/solr/core/src/java/org/apache/solr/cli/CreateCollectionTool.java
index 610e94a231a..17252204c39 100644
--- a/solr/core/src/java/org/apache/solr/cli/CreateCollectionTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/CreateCollectionTool.java
@@ -31,6 +31,7 @@ import org.apache.commons.cli.Option;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.JsonMapResponseParser;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.cloud.ZkStateReader;
@@ -153,6 +154,10 @@ public class CreateCollectionTool extends ToolBase {
NamedList<Object> response;
try {
+ var req =
+ CollectionAdminRequest.createCollection(
+ collectionName, confname, numShards, replicationFactor);
+ req.setResponseParser(new JsonMapResponseParser());
response =
cloudSolrClient.request(
CollectionAdminRequest.createCollection(
@@ -163,9 +168,11 @@ public class CreateCollectionTool extends ToolBase {
}
if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+ // pretty-print the response to stdout
CharArr arr = new CharArr();
new JSONWriter(arr, 2).write(response.asMap());
echo(arr.toString());
+ echo("\n");
} else {
String endMessage =
String.format(
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 c8debf07173..2acdd11559c 100644
--- a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
@@ -35,6 +35,8 @@ import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
+import org.apache.solr.client.solrj.impl.JsonMapResponseParser;
+import org.apache.solr.client.solrj.impl.NoOpResponseParser;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.CoreAdminRequest;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
@@ -180,7 +182,9 @@ public class DeleteTool extends ToolBase {
NamedList<Object> response;
try {
- response = cloudSolrClient.request(CollectionAdminRequest.deleteCollection(collectionName));
+ var req = CollectionAdminRequest.deleteCollection(collectionName);
+ req.setResponseParser(new JsonMapResponseParser());
+ response = cloudSolrClient.request(req);
} catch (SolrServerException sse) {
throw new Exception(
"Failed to delete collection '" + collectionName + "' due to: " + sse.getMessage());
@@ -201,6 +205,7 @@ public class DeleteTool extends ToolBase {
}
if (response != null) {
+ // pretty-print the response to stdout
CharArr arr = new CharArr();
new JSONWriter(arr, 2).write(response.asMap());
echo(arr.toString());
@@ -222,15 +227,14 @@ public class DeleteTool extends ToolBase {
unloadRequest.setDeleteDataDir(true);
unloadRequest.setDeleteInstanceDir(true);
unloadRequest.setCoreName(coreName);
+ unloadRequest.setResponseParser(new NoOpResponseParser("json"));
response = solrClient.request(unloadRequest);
} catch (SolrServerException sse) {
throw new Exception("Failed to delete core '" + coreName + "' due to: " + sse.getMessage());
}
if (response != null) {
- CharArr arr = new CharArr();
- new JSONWriter(arr, 2).write(response.asMap());
- echoIfVerbose(arr.toString(), cli);
+ echoIfVerbose((String) response.get("response"), cli);
echoIfVerbose("\n", cli);
}
}
diff --git a/solr/core/src/test/org/apache/solr/cli/ApiToolTest.java b/solr/core/src/test/org/apache/solr/cli/ApiToolTest.java
index 82e9b99a473..d817845854a 100644
--- a/solr/core/src/test/org/apache/solr/cli/ApiToolTest.java
+++ b/solr/core/src/test/org/apache/solr/cli/ApiToolTest.java
@@ -16,14 +16,30 @@
*/
package org.apache.solr.cli;
+import java.io.File;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
-import org.apache.solr.SolrTestCase;
+import java.util.Locale;
+import org.apache.lucene.tests.util.TestUtil;
+import org.apache.solr.client.solrj.request.AbstractUpdateRequest;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.params.ModifiableSolrParams;
+import org.junit.BeforeClass;
import org.junit.Test;
-public class ApiToolTest extends SolrTestCase {
+public class ApiToolTest extends SolrCloudTestCase {
+ static String COLLECTION_NAME = "globalLoaderColl";
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1)
+ .addConfig(
+ "config", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
+ .configure();
+ }
@Test
public void testParsingGetUrl() throws URISyntaxException {
@@ -42,4 +58,48 @@ public class ApiToolTest extends SolrTestCase {
assertEquals("select id from COLL_NAME limit 10", params.get("stmt"));
}
}
+
+ @Test
+ public void testQueryResponse() throws Exception {
+ int docCount = 1000;
+ CollectionAdminRequest.createCollection(COLLECTION_NAME, "config", 2, 1)
+ .process(cluster.getSolrClient());
+ cluster.waitForActiveCollection(COLLECTION_NAME, 2, 2);
+
+ String tmpFileLoc =
+ new File(cluster.getBaseDir().toFile().getAbsolutePath() + File.separator).getPath();
+
+ UpdateRequest ur = new UpdateRequest();
+ ur.setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true);
+
+ for (int i = 0; i < docCount; i++) {
+ ur.add(
+ "id",
+ String.valueOf(i),
+ "desc_s",
+ TestUtil.randomSimpleString(random(), 10, 50),
+ "a_dt",
+ "2019-09-30T05:58:03Z");
+ }
+ cluster.getSolrClient().request(ur, COLLECTION_NAME);
+
+ ApiTool tool = new ApiTool();
+
+ String response =
+ tool.callGet(
+ cluster.getJettySolrRunner(0).getBaseUrl()
+ + "/"
+ + COLLECTION_NAME
+ + "/select?q=*:*&rows=1&fl=id&sort=id+asc");
+ // Fields that could be missed because of serialization
+ assertFindInJson(response, "\"numFound\":1000,");
+ // Correct formatting
+ assertFindInJson(response, "\"docs\":[{");
+ }
+
+ private void assertFindInJson(String json, String find) {
+ assertTrue(
+ String.format(Locale.ROOT, "Could not find string %s in response: \n%s", find, json),
+ json.contains(find));
+ }
}
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
index c5832aa1432..c6e122aae50 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
@@ -139,7 +139,7 @@ public class NodeValueFetcher {
try {
SimpleSolrResponse rsp = ctx.invokeWithRetry(solrNode, CommonParams.METRICS_PATH, params);
- NamedList<?> metrics = (NamedList<?>) rsp.nl.get("metrics");
+ NamedList<?> metrics = (NamedList<?>) rsp.getResponse().get("metrics");
if (metrics != null) {
for (Tags t : Tags.values()) {
ctx.tags.put(t.tagName, t.extractResult(metrics));
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
index 1bc2aed24bc..4d7f087426a 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
@@ -194,7 +194,8 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter
SimpleSolrResponse rsp = ctx.invokeWithRetry(solrNode, CommonParams.METRICS_PATH, params);
metricsKeyVsTag.forEach(
(key, tags) -> {
- Object v = Utils.getObjectByPath(rsp.nl, true, Arrays.asList("metrics", key));
+ Object v =
+ Utils.getObjectByPath(rsp.getResponse(), true, Arrays.asList("metrics", key));
for (Object tag : tags) {
if (tag instanceof Function) {
@SuppressWarnings({"unchecked"})
@@ -296,7 +297,7 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter
.withResponseParser(new BinaryResponseParser())
.build()) {
NamedList<Object> rsp = client.request(request);
- request.response.nl = rsp;
+ request.response.setResponse(rsp);
return request.response;
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index 68d319e079f..ec80be332a3 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -871,6 +871,9 @@ public class Http2SolrClient extends SolrClient {
}
Object error = rsp == null ? null : rsp.get("error");
+ if (rsp != null && error == null && processor instanceof NoOpResponseParser) {
+ error = rsp.get("response");
+ }
if (error != null
&& (String.valueOf(getObjectByPath(error, true, errPath))
.endsWith("ExceptionWithErrObject"))) {
@@ -907,9 +910,12 @@ public class Http2SolrClient extends SolrClient {
if (reason == null) {
StringBuilder msg = new StringBuilder();
msg.append(response.getReason())
- .append("\n\n")
+ .append("\n")
.append("request: ")
.append(response.getRequest().getMethod());
+ if (error != null) {
+ msg.append("\n\nError returned:\n").append(error);
+ }
reason = java.net.URLDecoder.decode(msg.toString(), FALLBACK_CHARSET);
}
RemoteSolrException rss =
diff --git a/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java b/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java
index f321646a5c6..1c2274b7c0f 100644
--- a/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java
@@ -104,7 +104,7 @@ public interface IteratorWriter extends JSONWriter.Writable {
writer.writeValueSeparator();
}
writer.indent();
- write(writer);
+ writer.write(o);
return this;
}
});
diff --git a/solr/solrj/src/java/org/apache/solr/common/SolrDocumentList.java b/solr/solrj/src/java/org/apache/solr/common/SolrDocumentList.java
index 4204fc2e61b..cbc7f8dd2b5 100644
--- a/solr/solrj/src/java/org/apache/solr/common/SolrDocumentList.java
+++ b/solr/solrj/src/java/org/apache/solr/common/SolrDocumentList.java
@@ -16,7 +16,9 @@
*/
package org.apache.solr.common;
+import java.io.IOException;
import java.util.ArrayList;
+import java.util.Iterator;
/**
* Represent a list of SolrDocuments returned from a search. This includes position and offset
@@ -24,7 +26,24 @@ import java.util.ArrayList;
*
* @since solr 1.3
*/
-public class SolrDocumentList extends ArrayList<SolrDocument> {
+public class SolrDocumentList extends ArrayList<SolrDocument> implements MapWriter {
+
+ @Override
+ public void writeMap(EntryWriter ew) throws IOException {
+ ew.put("numFound", numFound);
+ ew.put("start", start);
+
+ if (maxScore != null) {
+ ew.put("maxScore", maxScore);
+ }
+
+ if (numFoundExact != null) {
+ ew.put("numFoundExact", numFoundExact);
+ }
+ final Iterator<SolrDocument> docs = iterator();
+ ew.put("docs", (IteratorWriter) iw -> docs.forEachRemaining(iw::addNoEx));
+ }
+
private long numFound = 0;
private long start = 0;
private Float maxScore = null;
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 a796872a825..c72a0956e19 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
@@ -480,15 +480,19 @@ public class Utils {
Object o = getVal(obj, s, -1);
if (o == null) return null;
if (idx > -1) {
- if (o instanceof MapWriter) {
+ if (o instanceof List) {
+ List<?> l = (List<?>) o;
+ o = idx < l.size() ? l.get(idx) : null;
+ } else if (o instanceof IteratorWriter) {
+ o = getValueAt((IteratorWriter) o, idx);
+ } else if (o instanceof MapWriter) {
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 {
- List<?> l = (List<?>) o;
- o = idx < l.size() ? l.get(idx) : null;
+ return null;
}
}
if (!isMapLike(o)) return null;
diff --git a/solr/solrj/src/java/org/noggit/JSONWriter.java b/solr/solrj/src/java/org/noggit/JSONWriter.java
index 0782717b8ae..349c1fecf02 100644
--- a/solr/solrj/src/java/org/noggit/JSONWriter.java
+++ b/solr/solrj/src/java/org/noggit/JSONWriter.java
@@ -21,6 +21,7 @@ package org.noggit;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Iterator;
import java.util.Map;
public class JSONWriter {
@@ -70,6 +71,8 @@ public class JSONWriter {
// perfect hashing.
if (o == null) {
writeNull();
+ } else if (o instanceof Writable) {
+ ((Writable) o).write(this);
} else if (o instanceof String) {
writeString((String) o);
} else if (o instanceof Number) {
@@ -86,12 +89,12 @@ public class JSONWriter {
write((Map<?, ?>) o);
} else if (o instanceof Collection) {
write((Collection<?>) o);
+ } else if (o instanceof Iterator) {
+ write((Iterator<?>) o);
} else if (o instanceof Boolean) {
write(((Boolean) o).booleanValue());
} else if (o instanceof CharSequence) {
writeString((CharSequence) o);
- } else if (o instanceof Writable) {
- ((Writable) o).write(this);
} else if (o instanceof Object[]) {
write(Arrays.asList((Object[]) o));
} else if (o instanceof int[]) {
@@ -156,6 +159,20 @@ public class JSONWriter {
endArray();
}
+ public void write(Iterator<?> val) {
+ startArray();
+ boolean first = true;
+ while (val.hasNext()) {
+ if (first) {
+ first = false;
+ } else {
+ writeValueSeparator();
+ }
+ write(val.next());
+ }
+ endArray();
+ }
+
/**
* A byte[] may be either a single logical value, or a list of small integers. It's up to the
* implementation to decide.