Skip to content

Commit

Permalink
SOLR-17066: Add GenericSolrRequest.setRequiresCollection (#2229)
Browse files Browse the repository at this point in the history
SOLR-17066 added a 'defaultCollection' field to each SolrClient
implementation, similar to the one that has long been in use for SolrJ's
"cloud" clients.  This default collection (or core) is used on a
request-by-request basis to build the complete HTTP path, based on the
value of SolrRequest.requiresCollection().

This is a particular challenge for GenericSolrRequest though, which can
be used to make both collection-agnostic and collection-aware requests.
This commit adds a GenericSolrRequest setter,
`setRequiresCollection(boolean)`, to allow GSR users to specify whether
the client-level default collection should be used on their request or not.
  • Loading branch information
gerlowskija authored Jan 30, 2024
1 parent 4d22f0c commit d5d7e55
Show file tree
Hide file tree
Showing 15 changed files with 147 additions and 61 deletions.
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ Other Changes
---------------------
* SOLR-17126: Cut over System.getProperty to EnvUtils.getProperty (janhoy)

* SOLR-17066: GenericSolrRequest now has a `setRequiresCollection` setter that allows it to specify whether
it should make use of the client-level default collection/core. (Jason Gerlowski)

================== 9.5.0 ==================
New Features
---------------------
Expand Down
21 changes: 10 additions & 11 deletions solr/core/src/java/org/apache/solr/cli/ExportTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.impl.StreamingBinaryResponseParser;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.cloud.DocCollection;
Expand Down Expand Up @@ -219,9 +220,10 @@ void fetchUniqueKey() throws SolrServerException, IOException {
NamedList<Object> response =
solrClient.request(
new GenericSolrRequest(
SolrRequest.METHOD.GET,
"/schema/uniquekey",
new MapSolrParams(Collections.singletonMap("collection", coll))));
SolrRequest.METHOD.GET,
"/schema/uniquekey",
new MapSolrParams(Collections.singletonMap("collection", coll)))
.setRequiresCollection(true));
uniqueKey = (String) response.get("uniqueKey");
}

Expand Down Expand Up @@ -620,7 +622,7 @@ boolean exportDocsFromCore() throws IOException, SolrServerException {

try (SolrClient client = SolrCLI.getSolrClient(baseurl, credentials)) {
expectedDocs = getDocCount(replica.getCoreName(), client, query);
GenericSolrRequest request;
QueryRequest request;
ModifiableSolrParams params = new ModifiableSolrParams();
params.add(Q, query);
if (fields != null) params.add(FL, fields);
Expand All @@ -644,12 +646,10 @@ boolean exportDocsFromCore() throws IOException, SolrServerException {
if (failed) return false;
if (docsWritten.get() > limit) return true;
params.set(CursorMarkParams.CURSOR_MARK_PARAM, cursorMark);
request =
new GenericSolrRequest(
SolrRequest.METHOD.GET, "/" + replica.getCoreName() + "/select", params);
request = new QueryRequest(params);
request.setResponseParser(responseParser);
try {
NamedList<Object> rsp = client.request(request);
NamedList<Object> rsp = client.request(request, replica.getCoreName());
String nextCursorMark = (String) rsp.get(CursorMarkParams.CURSOR_MARK_NEXT);
if (nextCursorMark == null || Objects.equals(cursorMark, nextCursorMark)) {
if (output != null) {
Expand Down Expand Up @@ -692,9 +692,8 @@ static long getDocCount(String coreName, SolrClient client, String query)
SolrQuery q = new SolrQuery(query);
q.setRows(0);
q.add("distrib", "false");
GenericSolrRequest request =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/" + coreName + "/select", q);
NamedList<Object> res = client.request(request);
final var request = new QueryRequest(q);
NamedList<Object> res = client.request(request, coreName);
SolrDocumentList sdl = (SolrDocumentList) res.get("response");
return sdl.getNumFound();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,10 @@ public Map<String, SolrPackageInstance> getPackagesDeployed(String collection) {
NamedList<Object> result =
solrClient.request(
new GenericSolrRequest(
SolrRequest.METHOD.GET,
PackageUtils.getCollectionParamsPath(collection) + "/PKG_VERSIONS"));
SolrRequest.METHOD.GET,
PackageUtils.getCollectionParamsPath(collection) + "/PKG_VERSIONS")
.setRequiresCollection(
false) /* Making a collection request, but already baked into path */);
packages =
(Map<String, String>)
result._get("/response/params/PKG_VERSIONS", Collections.emptyMap());
Expand Down Expand Up @@ -420,8 +422,10 @@ private Pair<List<String>, List<String>> deployCollectionPackage(
solrClient
.request(
new GenericSolrRequest(
SolrRequest.METHOD.GET,
PackageUtils.getCollectionParamsPath(collection) + "/packages"))
SolrRequest.METHOD.GET,
PackageUtils.getCollectionParamsPath(collection) + "/packages")
.setRequiresCollection(
false) /* Making a collection-request, but already baked into path */)
.asShallowMap()
.containsKey("params");
SolrCLI.postJsonToSolr(
Expand Down Expand Up @@ -722,8 +726,10 @@ Map<String, String> getPackageParams(String packageName, String collection) {
NamedList<Object> response =
solrClient.request(
new GenericSolrRequest(
SolrRequest.METHOD.GET,
PackageUtils.getCollectionParamsPath(collection) + "/packages"));
SolrRequest.METHOD.GET,
PackageUtils.getCollectionParamsPath(collection) + "/packages")
.setRequiresCollection(
false) /* Making a collection-request, but already baked into path */);
return (Map<String, String>)
response._get("/response/params/packages/" + packageName, Collections.emptyMap());
} catch (Exception ex) {
Expand Down Expand Up @@ -766,7 +772,8 @@ public boolean verify(

if ("GET".equalsIgnoreCase(cmd.method)) {
String response =
PackageUtils.getJsonStringFromUrl(solrClient, path, new ModifiableSolrParams());
PackageUtils.getJsonStringFromNonCollectionApi(
solrClient, path, new ModifiableSolrParams());
PackageUtils.printGreen(response);
String actualValue = null;
try {
Expand Down Expand Up @@ -816,7 +823,8 @@ public boolean verify(

if ("GET".equalsIgnoreCase(cmd.method)) {
String response =
PackageUtils.getJsonStringFromUrl(solrClient, path, new ModifiableSolrParams());
PackageUtils.getJsonStringFromCollectionApi(
solrClient, path, new ModifiableSolrParams());
PackageUtils.printGreen(response);
String actualValue = null;
try {
Expand Down Expand Up @@ -1098,7 +1106,7 @@ public Map<String, String> getDeployedCollections(String packageName) {
for (String collection : allCollections) {
// Check package version installed
String paramsJson =
PackageUtils.getJsonStringFromUrl(
PackageUtils.getJsonStringFromCollectionApi(
solrClient,
PackageUtils.getCollectionParamsPath(collection) + "/PKG_VERSIONS",
new ModifiableSolrParams().add("omitHeader", "true"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ public String getContentType() {
public static <T> T getJson(SolrClient client, String path, Class<T> klass) {
try {
return getMapper()
.readValue(getJsonStringFromUrl(client, path, new ModifiableSolrParams()), klass);
.readValue(
getJsonStringFromNonCollectionApi(client, path, new ModifiableSolrParams()), klass);
} catch (IOException e) {
throw new RuntimeException(e);
}
Expand All @@ -154,10 +155,39 @@ public static String getFileFromJarsAsString(List<Path> jars, String filename) {
return null;
}

/**
* Returns the response of a collection or core API call as string-ified JSON
*
* @param client the SolrClient used to make the request
* @param path the HTTP path of the Solr API to hit, starting after the collection or core name
* (i.e. omitting '/solr/techproducts')
* @param params query parameters to include when making the request
*/
public static String getJsonStringFromCollectionApi(
SolrClient client, String path, SolrParams params) {
return getJsonStringFromUrl(client, path, params, true);
}

/**
* Returns the response of a collection-agnostic API call as string-ified JSON
*
* @param client the SolrClient used to make the request
* @param path the HTTP path of the Solr API to hit, starting after '/solr' (or '/api' for v2
* requests)
* @param params query parameters to include when making the request
*/
public static String getJsonStringFromNonCollectionApi(
SolrClient client, String path, SolrParams params) {
return getJsonStringFromUrl(client, path, params, false);
}

/** Returns JSON string from a given Solr URL */
public static String getJsonStringFromUrl(SolrClient client, String path, SolrParams params) {
private static String getJsonStringFromUrl(
SolrClient client, String path, SolrParams params, boolean isCollectionApi) {
try {
GenericSolrRequest request = new GenericSolrRequest(SolrRequest.METHOD.GET, path, params);
GenericSolrRequest request =
new GenericSolrRequest(SolrRequest.METHOD.GET, path, params)
.setRequiresCollection(isCollectionApi);
request.setResponseParser(new JsonMapResponseParser());
NamedList<Object> response = client.request(request);
return response.jsonStr();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public void addKey(byte[] key, String destinationKeyFilename) throws Exception {
// put the public key into package store's trusted key store and request a sync.
String path = FileStoreAPI.KEYS_DIR + "/" + destinationKeyFilename;
PackageUtils.uploadKey(key, path, Paths.get(solrHome));
PackageUtils.getJsonStringFromUrl(
PackageUtils.getJsonStringFromNonCollectionApi(
solrClient, "/api/node/files" + path, new ModifiableSolrParams().add("sync", "true"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,8 @@ private UpdateCommand fetchFullUpdateFromLeader(AddUpdateCommand inplaceAdd, lon
params.set(DISTRIB, false);
params.set("getInputDocument", id);
params.set("onlyIfActive", true);
SolrRequest<SimpleSolrResponse> ur = new GenericSolrRequest(METHOD.GET, "/get", params);
SolrRequest<SimpleSolrResponse> ur =
new GenericSolrRequest(METHOD.GET, "/get", params).setRequiresCollection(true);

String leaderUrl = getLeaderUrl(id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1617,15 +1617,16 @@ public void testEmptyBackups() throws Exception {
final String backupName = "empty_backup1";
final GenericSolrRequest req =
new GenericSolrRequest(
SolrRequest.METHOD.GET,
"/replication",
params(
"command",
"backup",
"location",
backupDir.getAbsolutePath(),
"name",
backupName));
SolrRequest.METHOD.GET,
"/replication",
params(
"command",
"backup",
"location",
backupDir.getAbsolutePath(),
"name",
backupName))
.setRequiresCollection(true);
final TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME);
final SimpleSolrResponse rsp = req.process(leaderClient);

Expand All @@ -1645,15 +1646,16 @@ public void testEmptyBackups() throws Exception {
final String backupName = "empty_backup2";
final GenericSolrRequest req =
new GenericSolrRequest(
SolrRequest.METHOD.GET,
"/replication",
params(
"command",
"backup",
"location",
backupDir.getAbsolutePath(),
"name",
backupName));
SolrRequest.METHOD.GET,
"/replication",
params(
"command",
"backup",
"location",
backupDir.getAbsolutePath(),
"name",
backupName))
.setRequiresCollection(true);
final TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME);
final SimpleSolrResponse rsp = req.process(leaderClient);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ public void testReplicationHandler() throws Exception {

/** no solrj API for ReplicationHandler */
private GenericSolrRequest makeReplicationReq(SolrParams p) {
return new GenericSolrRequest(GenericSolrRequest.METHOD.GET, "/replication", p);
return new GenericSolrRequest(GenericSolrRequest.METHOD.GET, "/replication", p)
.setRequiresCollection(true);
}

/**
Expand Down
15 changes: 9 additions & 6 deletions solr/core/src/test/org/apache/solr/pkg/TestPackages.java
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,11 @@ public void testPluginLoading() throws Exception {
10,
cluster.getSolrClient(),
new GenericSolrRequest(
SolrRequest.METHOD.GET,
"/stream",
new MapSolrParams(
Map.of("collection", COLLECTION_NAME, WT, JAVABIN, "action", "plugins"))),
SolrRequest.METHOD.GET,
"/stream",
new MapSolrParams(
Map.of("collection", COLLECTION_NAME, WT, JAVABIN, "action", "plugins")))
.setRequiresCollection(true),
Map.of(":plugins:mincopy", "org.apache.solr.client.solrj.io.stream.metrics.MinCopyMetric"));

UpdateRequest ur = new UpdateRequest();
Expand Down Expand Up @@ -558,7 +559,8 @@ private void verifyComponent(
"true"));

GenericSolrRequest req1 =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/config/" + componentType, params);
new GenericSolrRequest(SolrRequest.METHOD.GET, "/config/" + componentType, params)
.setRequiresCollection(true);
TestDistribFileStore.assertResponseValues(
10,
client,
Expand Down Expand Up @@ -843,7 +845,8 @@ private void verifySchemaComponent(
SolrParams params =
new MapSolrParams(Map.of("collection", COLLECTION_NAME, WT, JAVABIN, "meta", "true"));

GenericSolrRequest req = new GenericSolrRequest(SolrRequest.METHOD.GET, path, params);
GenericSolrRequest req =
new GenericSolrRequest(SolrRequest.METHOD.GET, path, params).setRequiresCollection(true);
TestDistribFileStore.assertResponseValues(10, client, req, expected);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ public void testNonExistentQuery() throws Exception {
params.set("queryUUID", "foobar");

GenericSolrRequest request =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/cancel", params);
new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/cancel", params)
.setRequiresCollection(true);
NamedList<Object> queryResponse = cluster.getSolrClient(COLLECTION_NAME).request(request);

assertEquals("Query with queryID foobar not found", queryResponse.get("status"));
Expand Down Expand Up @@ -185,7 +186,9 @@ private NamedList<String> listTasks() throws SolrServerException, IOException {
NamedList<Object> response =
cluster
.getSolrClient(COLLECTION_NAME)
.request(new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/list"));
.request(
new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/list")
.setRequiresCollection(true));
return (NamedList<String>) response.get("taskList");
}

Expand All @@ -195,7 +198,8 @@ public void testCheckSpecificQueryStatus() throws Exception {
params.set("taskUUID", "25");

GenericSolrRequest request =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/list", params);
new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/list", params)
.setRequiresCollection(true);
NamedList<Object> queryResponse = cluster.getSolrClient(COLLECTION_NAME).request(request);

String result = (String) queryResponse.get("taskStatus");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ public void testBasicAuth() throws Exception {
assertPkiAuthMetricsMinimums(3, 3, 0, 0, 0, 0);

// Query that succeeds
GenericSolrRequest req = new GenericSolrRequest(SolrRequest.METHOD.GET, "/select", params);
final var req = new QueryRequest(params);
req.setBasicAuthCredentials("harry", "HarryIsUberCool");
cluster.getSolrClient().request(req, COLLECTION);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ private void modifySchema(String testCollection, CloudSolrClient client)
throws SolrServerException, IOException {
GenericSolrRequest req =
new GenericSolrRequest(SolrRequest.METHOD.POST, "/schema")
.setRequiresCollection(true)
.setContentWriter(
new RequestWriter.StringPayloadContentWriter(
"{\n"
Expand All @@ -180,6 +181,7 @@ private GenericSolrRequest createJsonReq(byte[] b) {
SolrRequest.METHOD.POST,
"/update/json/docs",
new MapSolrParams(Map.of("commit", "true")))
.setRequiresCollection(true)
.withContent(b, "application/json");
}

Expand All @@ -191,13 +193,15 @@ private GenericSolrRequest createJavabinReq(byte[] b) throws IOException {

return new GenericSolrRequest(
SolrRequest.METHOD.POST, "/update", new MapSolrParams(Map.of("commit", "true")))
.withContent(baos.toByteArray(), "application/javabin");
.withContent(baos.toByteArray(), "application/javabin")
.setRequiresCollection(true);
}

private GenericSolrRequest createCborReq(byte[] b) throws IOException {
return new GenericSolrRequest(
SolrRequest.METHOD.POST, "/update/cbor", new MapSolrParams(Map.of("commit", "true")))
.withContent(serializeToCbor(b), "application/cbor");
.withContent(serializeToCbor(b), "application/cbor")
.setRequiresCollection(true);
}

@SuppressWarnings("unchecked")
Expand Down
Loading

0 comments on commit d5d7e55

Please sign in to comment.