From df34cefec6288bb88ddfc6dfd20748981cba6156 Mon Sep 17 00:00:00 2001 From: Jason Gerlowski Date: Fri, 6 Sep 2024 10:15:02 -0700 Subject: [PATCH] SOLR-17044: Expose api "version" on SolrRequest objects (#2456) Solr uses different API roots for its v1 and v2 APIs: "/solr" for v1, and "/api" for v2. Adding a method for each SolrRequest to indicate which API version it belongs to will allow SolrJ to build the full request path more reliably without relying on fragile 'instanceof' checks. --- .../solr/packagemanager/PackageManager.java | 9 +-- .../solr/packagemanager/PackageUtils.java | 52 +++++------------ .../packagemanager/RepositoryManager.java | 3 +- .../apache/solr/client/solrj/SolrRequest.java | 24 ++++++++ .../client/solrj/impl/CloudSolrClient.java | 5 +- .../client/solrj/impl/HttpSolrClient.java | 3 +- .../client/solrj/impl/HttpSolrClientBase.java | 3 +- .../solrj/request/GenericV2SolrRequest.java | 58 +++++++++++++++++++ .../solr/client/solrj/request/V2Request.java | 5 ++ .../solr/client/solrj/util/ClientUtils.java | 28 ++++++--- .../src/resources/java-template/api.mustache | 5 ++ 11 files changed, 139 insertions(+), 56 deletions(-) create mode 100644 solr/solrj/src/java/org/apache/solr/client/solrj/request/GenericV2SolrRequest.java diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java index 15b6def6ecb..a751d20472f 100644 --- a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java +++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java @@ -47,6 +47,7 @@ import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.SolrZkClientTimeout; import org.apache.solr.client.solrj.request.GenericSolrRequest; +import org.apache.solr.client.solrj.request.GenericV2SolrRequest; import org.apache.solr.client.solrj.request.V2Request; import org.apache.solr.client.solrj.request.beans.PackagePayload; import org.apache.solr.client.solrj.request.beans.PluginMeta; @@ -237,7 +238,7 @@ public Map getPackagesDeployed(String collection) { try { NamedList result = solrClient.request( - new GenericSolrRequest( + new GenericV2SolrRequest( SolrRequest.METHOD.GET, PackageUtils.getCollectionParamsPath(collection) + "/PKG_VERSIONS") .setRequiresCollection( @@ -278,7 +279,7 @@ public Map getPackagesDeployedAsClusterLevelPlugins try { NamedList response = solrClient.request( - new GenericSolrRequest(SolrRequest.METHOD.GET, PackageUtils.CLUSTERPROPS_PATH)); + new GenericV2SolrRequest(SolrRequest.METHOD.GET, PackageUtils.CLUSTERPROPS_PATH)); Integer statusCode = (Integer) response.findRecursive("responseHeader", "status"); if (statusCode == null || statusCode == ErrorCode.NOT_FOUND.code) { // Cluster props doesn't exist, that means there are no cluster level plugins installed. @@ -421,7 +422,7 @@ private Pair, List> deployCollectionPackage( boolean packageParamsExist = solrClient .request( - new GenericSolrRequest( + new GenericV2SolrRequest( SolrRequest.METHOD.GET, PackageUtils.getCollectionParamsPath(collection) + "/packages") .setRequiresCollection( @@ -723,7 +724,7 @@ Map getPackageParams(String packageName, String collection) { try { NamedList response = solrClient.request( - new GenericSolrRequest( + new GenericV2SolrRequest( SolrRequest.METHOD.GET, PackageUtils.getCollectionParamsPath(collection) + "/packages") .setRequiresCollection( diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java index 6e9919adde2..a90fc0b0b30 100644 --- a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java +++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java @@ -25,9 +25,7 @@ import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider; import com.jayway.jsonpath.spi.mapper.MappingProvider; import java.io.IOException; -import java.io.OutputStream; import java.nio.ByteBuffer; -import java.nio.channels.Channels; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.List; @@ -40,11 +38,11 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.JsonMapResponseParser; +import org.apache.solr.client.solrj.request.FileStoreApi; import org.apache.solr.client.solrj.request.GenericSolrRequest; -import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.client.solrj.request.GenericV2SolrRequest; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; -import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; @@ -55,6 +53,7 @@ import org.apache.solr.filestore.FileStoreAPI; import org.apache.solr.packagemanager.SolrPackage.Manifest; import org.apache.solr.util.SolrJacksonAnnotationInspector; +import org.apache.zookeeper.server.ByteBufferInputStream; public class PackageUtils { @@ -92,39 +91,18 @@ public static ObjectMapper getMapper() { */ public static void postFile(SolrClient client, ByteBuffer buffer, String name, String sig) throws SolrServerException, IOException { - String resource = "/api/cluster/files" + name; - ModifiableSolrParams params = new ModifiableSolrParams(); - if (sig != null) { - params.add("sig", sig); - } - GenericSolrRequest request = - new GenericSolrRequest(SolrRequest.METHOD.PUT, resource, params) { - @Override - public RequestWriter.ContentWriter getContentWriter(String expectedType) { - return new RequestWriter.ContentWriter() { - public final ByteBuffer payload = buffer; - - @Override - public void write(OutputStream os) throws IOException { - if (payload == null) return; - Channels.newChannel(os).write(payload); - } + try (final var stream = new ByteBufferInputStream(buffer)) { + final var uploadReq = new FileStoreApi.UploadFile(name, stream); + if (sig != null) { + uploadReq.setSig(List.of(sig)); + } - @Override - public String getContentType() { - return "application/octet-stream"; - } - }; - } - }; - NamedList rsp = client.request(request); - if (!name.equals(rsp.get(CommonParams.FILE))) { - throw new SolrException( - ErrorCode.BAD_REQUEST, - "Mismatch in file uploaded. Uploaded: " - + rsp.get(CommonParams.FILE) - + ", Original: " - + name); + final var uploadRsp = uploadReq.process(client).getParsed(); + if (!name.equals(uploadRsp.file)) { + throw new SolrException( + ErrorCode.BAD_REQUEST, + "Mismatch in file uploaded. Uploaded: " + uploadRsp.file + ", Original: " + name); + } } } @@ -206,7 +184,7 @@ public static Manifest fetchManifest( SolrClient solrClient, String manifestFilePath, String expectedSHA512) throws IOException, SolrServerException { GenericSolrRequest request = - new GenericSolrRequest(SolrRequest.METHOD.GET, "/api/node/files" + manifestFilePath); + new GenericV2SolrRequest(SolrRequest.METHOD.GET, "/api/node/files" + manifestFilePath); request.setResponseParser(new JsonMapResponseParser()); NamedList response = solrClient.request(request); String manifestJson = (String) response.get("response"); diff --git a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java index a203ce1ee39..3ba2bc76cfb 100644 --- a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java +++ b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java @@ -45,6 +45,7 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.GenericSolrRequest; +import org.apache.solr.client.solrj.request.GenericV2SolrRequest; import org.apache.solr.client.solrj.request.RequestWriter; import org.apache.solr.client.solrj.request.beans.PackagePayload; import org.apache.solr.common.SolrException; @@ -238,7 +239,7 @@ private boolean installPackage(String packageName, String version) throws SolrEx add.manifestSHA512 = manifestSHA512; GenericSolrRequest request = - new GenericSolrRequest(SolrRequest.METHOD.POST, PackageUtils.PACKAGE_PATH) { + new GenericV2SolrRequest(SolrRequest.METHOD.POST, PackageUtils.PACKAGE_PATH) { @Override public RequestWriter.ContentWriter getContentWriter(String expectedType) { return new RequestWriter.StringPayloadContentWriter( 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 e00ad8376dd..bb7955173b3 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 @@ -52,6 +52,21 @@ public enum METHOD { DELETE }; + public enum ApiVersion { + V1("/solr"), + V2("/api"); + + private final String apiPrefix; + + ApiVersion(String apiPrefix) { + this.apiPrefix = apiPrefix; + } + + public String getApiPrefix() { + return apiPrefix; + } + } + public enum SolrRequestType { QUERY, UPDATE, @@ -214,6 +229,15 @@ public boolean requiresCollection() { return false; } + /** + * Indicates which API version this request will make + * + *

Defaults implementation returns 'V1'. + */ + public ApiVersion getApiVersion() { + return ApiVersion.V1; + } + /** * @deprecated Please use {@link SolrRequest#getContentWriter(String)} instead. */ diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index ffc86f321ce..e6602f47545 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -911,7 +911,8 @@ protected NamedList requestWithRetryOnStaleState( // or request is v2 api and its method is not GET if (inputCollections.isEmpty() || isAdmin - || (request instanceof V2Request && request.getMethod() != SolrRequest.METHOD.GET)) { + || (request.getApiVersion() == SolrRequest.ApiVersion.V2 + && request.getMethod() != SolrRequest.METHOD.GET)) { if (exc instanceof SolrServerException) { throw (SolrServerException) exc; } else if (exc instanceof IOException) { @@ -1084,7 +1085,7 @@ protected NamedList sendRequest(SolrRequest request, List inp final List theUrlList = new ArrayList<>(); // we populate this as follows... - if (request instanceof V2Request) { + if (request.getApiVersion() == SolrRequest.ApiVersion.V2) { if (!liveNodes.isEmpty()) { List liveNodesList = new ArrayList<>(liveNodes); Collections.shuffle(liveNodesList, rand); 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 900400c8059..a6907565cc2 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 @@ -77,7 +77,6 @@ import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.V2RequestSupport; import org.apache.solr.client.solrj.request.RequestWriter; -import org.apache.solr.client.solrj.request.V2Request; import org.apache.solr.client.solrj.util.ClientUtils; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; @@ -266,7 +265,7 @@ public NamedList request( } private boolean isV2ApiRequest(final SolrRequest request) { - return request instanceof V2Request || request.getPath().contains("/____v2"); + return request.getApiVersion() == SolrRequest.ApiVersion.V2; } private void setBasicAuthHeader(SolrRequest request, HttpRequestBase method) 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 e423634fe3d..8092003aa5c 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 @@ -40,7 +40,6 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.RequestWriter; -import org.apache.solr.client.solrj.request.V2Request; import org.apache.solr.client.solrj.util.AsyncListener; import org.apache.solr.client.solrj.util.Cancellable; import org.apache.solr.client.solrj.util.ClientUtils; @@ -404,7 +403,7 @@ public CompletableFuture> requestAsync(final SolrRequest re } public boolean isV2ApiRequest(final SolrRequest request) { - return request instanceof V2Request || request.getPath().contains("/____v2"); + return request.getApiVersion() == SolrRequest.ApiVersion.V2; } public String getBaseURL() { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/GenericV2SolrRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/GenericV2SolrRequest.java new file mode 100644 index 00000000000..f4267ab7815 --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/GenericV2SolrRequest.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.client.solrj.request; + +import org.apache.solr.common.params.SolrParams; + +/** A {@link GenericSolrRequest} implementation intended for v2 APIs */ +public class GenericV2SolrRequest extends GenericSolrRequest { + + /** + * @param m the HTTP method to use for this request + * @param path the HTTP path to use for this request. Path may include the v2 API root path (i.e. + * "/api"), but does not need to. If users are making a collection-aware request (i.e. {@link + * #setRequiresCollection(boolean)} is called with 'true'), only the section of the API path + * following the collection or core should be provided here. + */ + public GenericV2SolrRequest(METHOD m, String path) { + super(m, removeLeadingApiRoot(path)); + } + + /** + * @param m the HTTP method to use for this request + * @param path the HTTP path to use for this request. If users are making a collection-aware + * request (i.e. {@link #setRequiresCollection(boolean)} is called with 'true'), only the + * section of the API path following the collection or core should be provided here. + * @param params query parameter names and values for making this request. + */ + public GenericV2SolrRequest(METHOD m, String path, SolrParams params) { + super(m, path); + this.params = params; + } + + @Override + public ApiVersion getApiVersion() { + return ApiVersion.V2; + } + + private static String removeLeadingApiRoot(String path) { + if (path.startsWith("/api")) { + return path.replaceFirst("/api", ""); + } + return path; + } +} diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/V2Request.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/V2Request.java index 242b24591f3..bcef4488560 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/V2Request.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/V2Request.java @@ -111,6 +111,11 @@ public String getCollection() { return collection; } + @Override + public ApiVersion getApiVersion() { + return ApiVersion.V2; + } + @Override protected V2Response createResponse(SolrClient client) { return new V2Response(); 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 78f369fb995..ac2c5f76ed2 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 @@ -84,11 +84,11 @@ public static String buildRequestUrl( throws MalformedURLException { String basePath = solrRequest.getBasePath() == null ? serverRootUrl : solrRequest.getBasePath(); - if (solrRequest instanceof V2Request) { - if (System.getProperty("solr.v2RealPath") == null) { - basePath = changeV2RequestEndpoint(basePath); - } else { + if (solrRequest.getApiVersion() == SolrRequest.ApiVersion.V2) { + if (solrRequest instanceof V2Request && System.getProperty("solr.v2RealPath") != null) { basePath = serverRootUrl + "/____v2"; + } else { + basePath = addNormalV2ApiRoot(basePath); } } @@ -102,10 +102,22 @@ public static String buildRequestUrl( return basePath + path; } - private static String changeV2RequestEndpoint(String basePath) throws MalformedURLException { - URI oldURI = URI.create(basePath); - String newPath = oldURI.getPath().replaceFirst("/solr", "/api"); - return oldURI.resolve(newPath).toString(); + private static String addNormalV2ApiRoot(String basePath) throws MalformedURLException { + final var oldURI = URI.create(basePath); + final var revisedPath = buildReplacementV2Path(oldURI.getPath()); + return oldURI.resolve(revisedPath).toString(); + } + + private static String buildReplacementV2Path(String existingPath) { + final var v1Root = SolrRequest.ApiVersion.V1.getApiPrefix(); + final var v2Root = SolrRequest.ApiVersion.V2.getApiPrefix(); + if (existingPath.contains(v1Root)) { + return existingPath.replaceFirst(v1Root, v2Root); + } else if (!existingPath.contains(v2Root)) { + return existingPath + v2Root; + } else { + return existingPath; + } } // ------------------------------------------------------------------------ diff --git a/solr/solrj/src/resources/java-template/api.mustache b/solr/solrj/src/resources/java-template/api.mustache index bbb300023ed..c3961cf50b1 100644 --- a/solr/solrj/src/resources/java-template/api.mustache +++ b/solr/solrj/src/resources/java-template/api.mustache @@ -185,6 +185,11 @@ public class {{classname}} { return SolrRequestType.ADMIN.toString(); } + @Override + public ApiVersion getApiVersion() { + return ApiVersion.V2; + } + @Override public SolrParams getParams() { final ModifiableSolrParams params = new ModifiableSolrParams();