From 139351a89674f2fa90767c83599b86b4d935df1e Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Tue, 12 Nov 2024 09:44:49 -0800 Subject: [PATCH] SOLR-17256: Nuke SolrRequest set/getBasePath in 10 (#2852) 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. --- solr/CHANGES.txt | 3 +++ .../modules/deployment-guide/pages/solrj.adoc | 4 ++-- .../apache/solr/client/solrj/SolrRequest.java | 12 ----------- .../client/solrj/impl/HttpSolrClient.java | 3 +-- .../client/solrj/impl/HttpSolrClientBase.java | 2 +- .../client/solrj/request/RequestWriter.java | 5 ----- .../solr/client/solrj/util/ClientUtils.java | 21 +++++-------------- .../client/solrj/SolrExampleCborTest.java | 12 ----------- .../client/solrj/util/ClientUtilsTest.java | 15 +++---------- 9 files changed, 15 insertions(+), 62 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b16fea65387..874adfcc4d7 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -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) diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/solrj.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/solrj.adoc index bcb2486ab34..e2936efa9a3 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/solrj.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/solrj.adoc @@ -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. diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java index 58a1c239683..a122cf5bda1 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java @@ -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<>(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index 3977316abc1..9c1ed46d7b5 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -339,8 +339,7 @@ protected HttpRequestBase createMethod(SolrRequest request, String collection Collection 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) { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java index 244fcb4c7b9..27e56375c2d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java @@ -112,7 +112,7 @@ protected HttpSolrClientBase(String serverBaseUrl, HttpSolrClientBuilderBase solrRequest, String collection) throws MalformedURLException { - return ClientUtils.buildRequestUrl(solrRequest, requestWriter, serverBaseUrl, collection); + return ClientUtils.buildRequestUrl(solrRequest, serverBaseUrl, collection); } protected ResponseParser responseParser(SolrRequest solrRequest) { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/RequestWriter.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/RequestWriter.java index e15c750f308..bfc7141648d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/RequestWriter.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/RequestWriter.java @@ -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; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java b/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java index 81453b079cd..0dab0c402aa 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java @@ -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; @@ -66,32 +64,23 @@ public static Collection 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; } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java index f0d420a69ec..41cfd596fed 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java @@ -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 docs = updateRequest.getDocuments(); - if (docs == null || docs.isEmpty()) return super.getPath(req); - return "/update/cbor"; - } else { - return super.getPath(req); - } - } }; } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java index 3322b10fbdc..cb7f9f7e93e 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java @@ -59,7 +59,7 @@ 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); } @@ -67,24 +67,15 @@ public void testUrlBuilding() throws Exception { { 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); } }