Skip to content

Commit

Permalink
SOLR-17256: Nuke SolrRequest set/getBasePath in 10 (apache#2852)
Browse files Browse the repository at this point in the history
Previous commits removed all usage of these methods, but held off on
actually removing the deprecated methods themselves.  This commit now
takes the final step of doing that removal.

Should not be backported to branch_9x, as the deprecated methods are
required to remain there for the remainder of the 9.x release line.
  • Loading branch information
gerlowskija authored Nov 12, 2024
1 parent c694258 commit 139351a
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 62 deletions.
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ Deprecation Removals

* SOLR-17352: Remove deprecated Solr CLI options. Run bin/solr yourcommand -h to see current options. (Eric Pugh, Christos Malliardis)

* SOLR-17256: Previously deprecated `SolrRequest` methods `setBasePath` and `getBasePath` have been removed. SolrJ users
wishing to temporarily override an HTTP client's base URL may use `Http2SolrClient.requestWithBaseUrl` instead. (Jason Gerlowski)

Dependency Upgrades
---------------------
(No changes)
Expand Down
4 changes: 2 additions & 2 deletions solr/solr-ref-guide/modules/deployment-guide/pages/solrj.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ Unless otherwise specified, SolrJ expects these URLs to point to the root Solr p

A few notable exceptions to this are described below:

- *Http2SolrClient* - Users of `Http2SolrClient` may choose to skip providing a root URL to their client, in favor of specifing the URL on a request-by-request basis using `SolrRequest.setBasePath`.
`Http2SolrClient` will throw an `IllegalArgumentException` if neither the client nor the request specify a URL.
- *Http2SolrClient* - Users of `Http2SolrClient` may choose to skip providing a root URL to their client, in favor of specifing the URL as a argument for the `Http2SolrClient.requestWithBaseUrl` method.
Calling any other `request` methods on a URL-less `Http2SolrClient` will result in an `IllegalArgumentException`.
- *LBHttpSolrClient* and *LBHttp2SolrClient* - Solr's "load balancing" clients are frequently used to round-robin requests across a set of replicas or cores.
URLs are still expected to point to the Solr root (i.e. "/solr"), but to support this use-case the URLs are often supplemented by an additional parameter to specify the targeted core.
Alternatively, some "load balancing" methods make use of an `Endpoint` abstraction to provide this URL and core information in a more structured way.
Expand Down
12 changes: 0 additions & 12 deletions solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -273,18 +273,6 @@ public String getCollection() {
return getParams() == null ? null : getParams().get("collection");
}

@Deprecated // SOLR-17256 Slated for removal in Solr 10; only used internally
public void setBasePath(String path) {
if (path.endsWith("/")) path = path.substring(0, path.length() - 1);

this.basePath = path;
}

@Deprecated // SOLR-17256 Slated for removal in Solr 10; only used internally
public String getBasePath() {
return basePath;
}

public void addHeader(String key, String value) {
if (headers == null) {
headers = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,7 @@ protected HttpRequestBase createMethod(SolrRequest<?> request, String collection
Collection<ContentStream> streams =
contentWriter == null ? requestWriter.getContentStreams(request) : null;

final String requestUrlBeforeParams =
ClientUtils.buildRequestUrl(request, requestWriter, baseUrl, collection);
final String requestUrlBeforeParams = ClientUtils.buildRequestUrl(request, baseUrl, collection);

ResponseParser parser = request.getResponseParser();
if (parser == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ protected HttpSolrClientBase(String serverBaseUrl, HttpSolrClientBuilderBase<?,

protected String getRequestUrl(SolrRequest<?> solrRequest, String collection)
throws MalformedURLException {
return ClientUtils.buildRequestUrl(solrRequest, requestWriter, serverBaseUrl, collection);
return ClientUtils.buildRequestUrl(solrRequest, serverBaseUrl, collection);
}

protected ResponseParser responseParser(SolrRequest<?> solrRequest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,6 @@ && isNull(updateRequest.getDeleteQuery())
&& updateRequest.getDocIterator() == null;
}

@Deprecated // SOLR-17256 Slated for removal in Solr 10; only used internally
public String getPath(SolrRequest<?> req) {
return req.getPath();
}

public void write(SolrRequest<?> request, OutputStream os) throws IOException {
if (request instanceof UpdateRequest) {
UpdateRequest updateRequest = (UpdateRequest) request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@
import java.util.Date;
import java.util.Map;
import java.util.Map.Entry;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.SolrInputField;
import org.apache.solr.common.cloud.Slice;
Expand Down Expand Up @@ -66,32 +64,23 @@ public static Collection<ContentStream> toContentStreams(
* Create the full URL for a SolrRequest (excepting query parameters) as a String
*
* @param solrRequest the {@link SolrRequest} to build the URL for
* @param requestWriter a {@link RequestWriter} from the {@link SolrClient} that will be sending
* the request
* @param serverRootUrl the root URL of the Solr server being targeted. May be overridden {@link
* SolrRequest#getBasePath()}, if present.
* @param serverRootUrl the root URL of the Solr server being targeted.
* @param collection the collection to send the request to. May be null if no collection is
* needed.
* @throws MalformedURLException if {@code serverRootUrl} or {@link SolrRequest#getBasePath()}
* contain a malformed URL string
* @throws MalformedURLException if {@code serverRootUrl} contains a malformed URL string
*/
public static String buildRequestUrl(
SolrRequest<?> solrRequest,
RequestWriter requestWriter,
String serverRootUrl,
String collection)
SolrRequest<?> solrRequest, String serverRootUrl, String collection)
throws MalformedURLException {

// TODO remove getBasePath support here prior to closing SOLR-17256
String basePath = solrRequest.getBasePath() == null ? serverRootUrl : solrRequest.getBasePath();

String basePath = serverRootUrl;
if (solrRequest.getApiVersion() == SolrRequest.ApiVersion.V2) {
basePath = addNormalV2ApiRoot(basePath);
}

if (solrRequest.requiresCollection() && collection != null) basePath += "/" + collection;

String path = requestWriter.getPath(solrRequest);
String path = solrRequest.getPath();
if (path == null || !path.startsWith("/")) {
path = DEFAULT_PATH;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,18 +278,6 @@ public String getContentType() {
public String getUpdateContentType() {
return "application/cbor";
}

@Override
public String getPath(SolrRequest<?> req) {
if (req instanceof UpdateRequest) {
UpdateRequest updateRequest = (UpdateRequest) req;
List<SolrInputDocument> docs = updateRequest.getDocuments();
if (docs == null || docs.isEmpty()) return super.getPath(req);
return "/update/cbor";
} else {
return super.getPath(req);
}
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,32 +59,23 @@ public void testUrlBuilding() throws Exception {
// Simple case, non-collection request
{
final var request = new HealthCheckRequest();
final var url = ClientUtils.buildRequestUrl(request, rw, "http://localhost:8983/solr", null);
final var url = ClientUtils.buildRequestUrl(request, "http://localhost:8983/solr", null);
assertEquals("http://localhost:8983/solr/admin/info/health", url);
}

// Simple case, collection request
{
final var request = new QueryRequest();
final var url =
ClientUtils.buildRequestUrl(request, rw, "http://localhost:8983/solr", "someColl");
ClientUtils.buildRequestUrl(request, "http://localhost:8983/solr", "someColl");
assertEquals("http://localhost:8983/solr/someColl/select", url);
}

// Uses SolrRequest.getBasePath() to override baseUrl
{
final var request = new HealthCheckRequest();
request.setBasePath("http://alternate-url:7574/solr");
final var url = ClientUtils.buildRequestUrl(request, rw, "http://localhost:8983/solr", null);
assertEquals("http://alternate-url:7574/solr/admin/info/health", url);
}

// Ignores collection when not needed (i.e. obeys SolrRequest.requiresCollection)
{
final var request = new HealthCheckRequest();
final var url =
ClientUtils.buildRequestUrl(
request, rw, "http://localhost:8983/solr", "unneededCollection");
ClientUtils.buildRequestUrl(request, "http://localhost:8983/solr", "unneededCollection");
assertEquals("http://localhost:8983/solr/admin/info/health", url);
}
}
Expand Down

0 comments on commit 139351a

Please sign in to comment.