diff --git a/dev-docs/v2-api-conventions.adoc b/dev-docs/v2-api-conventions.adoc index 74a760c2bfd..75e2896560b 100644 --- a/dev-docs/v2-api-conventions.adoc +++ b/dev-docs/v2-api-conventions.adoc @@ -1,3 +1,4 @@ += API Design == HTTP Paths Where possible, each v2 API is given an HTTP path that reflects the resource type and/or name most relevant to its functionality. @@ -69,8 +70,8 @@ For use within the v2 API, the four "popular" HTTP methods have the following se == Errors v2 APIs should be consistent in how they report errors. Throwing a `SolrException` will convey -1.the error code as the HTTP response status code, as `responseHeader.status` and as `error.code`, and -1.the error message as `error.msg`. +1. The error code as the HTTP response status code, as `responseHeader.status` and as `error.code`, and +2. The error message as `error.msg`. API calls that reference a specific resource (e.g. `specificCollName`, `specificAliasName`, `specificPropertyName` and others per the above list) that do not exist should return `SolrException.ErrorCode.NOT_FOUND` (HTTP 404). @@ -82,3 +83,23 @@ Often these operations were initially conceived of as procedural "commands" and Solr's v2 API currently accommodates these "command" APIs by appending the command name (often a verb like "unload", "reload", or "split") onto the otherwise "resource"-based path. For example: Solr's core "unload" command uses the API `POST /api/cores/specificCoreName/unload`. + += JAX-RS Implementation Conventions + +== Streaming + +Solr has a number of APIs that return binary file data or other arbitrary content, such as the "replication" APIs used to pull index files from other cores. +Please use the following conventions when implementing similar endpoints: +1. `@Operation` annotations use an "extension property" to indicate to codegen tools that the API output is "raw" or untyped. For example: ++ +``` + @Operation( + summary = "Return the data stored in a specified ZooKeeper node", + tags = {"zookeeper-read"}, + extensions = { + @Extension(properties = {@ExtensionProperty(name = RAW_OUTPUT_PROPERTY, value = "true")}) + }) +``` +2. Interface methods should return a type that implements the JAX-RS `StreamingOutput` interface. + +See the `fetchFile()` method in `ReplicationApis.java` for a concrete example. diff --git a/solr/api/build.gradle b/solr/api/build.gradle index adad0302602..a5878bb06ac 100644 --- a/solr/api/build.gradle +++ b/solr/api/build.gradle @@ -55,6 +55,8 @@ resolve { outputDir = file(project.openApiSpecDir) outputFileName = "solr-openapi-${version}" prettyPrint = true +// Ignore resources not annotated with 'Operation', useful for omitting endpoints from OAS + readAllResources = false } dependencies { diff --git a/solr/api/src/java/org/apache/solr/client/api/endpoint/ConfigsetsApi.java b/solr/api/src/java/org/apache/solr/client/api/endpoint/ConfigsetsApi.java index 9961b4c9f28..4bc812043e9 100644 --- a/solr/api/src/java/org/apache/solr/client/api/endpoint/ConfigsetsApi.java +++ b/solr/api/src/java/org/apache/solr/client/api/endpoint/ConfigsetsApi.java @@ -89,6 +89,7 @@ SolrJerseyResponse uploadConfigSet( @PUT @Path("{filePath:.+}") + @Operation(summary = "Create a new configset.", tags = "configsets") SolrJerseyResponse uploadConfigSetFile( @PathParam("configSetName") String configSetName, @PathParam("filePath") String filePath, diff --git a/solr/api/src/java/org/apache/solr/client/api/endpoint/ReplicationApis.java b/solr/api/src/java/org/apache/solr/client/api/endpoint/ReplicationApis.java index 3fe5ac14f45..1f0476f8661 100644 --- a/solr/api/src/java/org/apache/solr/client/api/endpoint/ReplicationApis.java +++ b/solr/api/src/java/org/apache/solr/client/api/endpoint/ReplicationApis.java @@ -16,7 +16,7 @@ */ package org.apache.solr.client.api.endpoint; -import static org.apache.solr.client.api.util.Constants.OMIT_FROM_CODEGEN_PROPERTY; +import static org.apache.solr.client.api.util.Constants.RAW_OUTPUT_PROPERTY; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; @@ -59,10 +59,9 @@ FileListResponse fetchFileList( @CoreApiParameters @Operation( summary = "Get a stream of a specific file path of a core", - tags = {"core-replication"}, - extensions = { // TODO Remove as a part of SOLR-17562 - @Extension( - properties = {@ExtensionProperty(name = OMIT_FROM_CODEGEN_PROPERTY, value = "true")}) + tags = {"replication"}, + extensions = { + @Extension(properties = {@ExtensionProperty(name = RAW_OUTPUT_PROPERTY, value = "true")}) }) @Path("/files/{filePath}") StreamingOutput fetchFile( diff --git a/solr/api/src/java/org/apache/solr/client/api/endpoint/ZooKeeperReadApis.java b/solr/api/src/java/org/apache/solr/client/api/endpoint/ZooKeeperReadApis.java index f41e8de3d63..58c5091fe42 100644 --- a/solr/api/src/java/org/apache/solr/client/api/endpoint/ZooKeeperReadApis.java +++ b/solr/api/src/java/org/apache/solr/client/api/endpoint/ZooKeeperReadApis.java @@ -16,15 +16,19 @@ */ package org.apache.solr.client.api.endpoint; +import static org.apache.solr.client.api.util.Constants.RAW_OUTPUT_PROPERTY; + import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.extensions.Extension; +import io.swagger.v3.oas.annotations.extensions.ExtensionProperty; import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.MediaType; -import org.apache.solr.client.api.model.ZooKeeperFileResponse; +import jakarta.ws.rs.core.StreamingOutput; import org.apache.solr.client.api.model.ZooKeeperListChildrenResponse; /** V2 API definitions for Solr's ZooKeeper ready-proxy endpoint */ @@ -35,9 +39,12 @@ public interface ZooKeeperReadApis { @Path("/data{zkPath:.+}") @Operation( summary = "Return the data stored in a specified ZooKeeper node", - tags = {"zookeeper-read"}) + tags = {"zookeeper-read"}, + extensions = { + @Extension(properties = {@ExtensionProperty(name = RAW_OUTPUT_PROPERTY, value = "true")}) + }) @Produces({"application/vnd.apache.solr.raw", MediaType.APPLICATION_JSON}) - ZooKeeperFileResponse readNode( + StreamingOutput readNode( @Parameter(description = "The path of the node to read from ZooKeeper") @PathParam("zkPath") String zkPath); @@ -48,7 +55,7 @@ ZooKeeperFileResponse readNode( @GET @Path("/data/security.json") @Produces({"application/vnd.apache.solr.raw", MediaType.APPLICATION_JSON}) - ZooKeeperFileResponse readSecurityJsonNode(); + StreamingOutput readSecurityJsonNode(); @GET @Path("/children{zkPath:.*}") diff --git a/solr/api/src/java/org/apache/solr/client/api/model/ZooKeeperFileResponse.java b/solr/api/src/java/org/apache/solr/client/api/model/ZooKeeperFileResponse.java deleted file mode 100644 index d09302fa168..00000000000 --- a/solr/api/src/java/org/apache/solr/client/api/model/ZooKeeperFileResponse.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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.api.model; - -import com.fasterxml.jackson.annotation.JsonProperty; - -public class ZooKeeperFileResponse extends SolrJerseyResponse { - // TODO Should be switched over to using StreamingOutput as a part of SOLR-17562 - @JsonProperty("content") // A flag value that RawResponseWriter handles specially - public Object output; - - @JsonProperty("zkData") - public String zkData; -} diff --git a/solr/api/src/java/org/apache/solr/client/api/util/Constants.java b/solr/api/src/java/org/apache/solr/client/api/util/Constants.java index 49e69d37fac..eab0b6f1020 100644 --- a/solr/api/src/java/org/apache/solr/client/api/util/Constants.java +++ b/solr/api/src/java/org/apache/solr/client/api/util/Constants.java @@ -29,7 +29,12 @@ private Constants() { public static final String CORE_NAME_PATH_PARAMETER = "coreName"; + // Annotation used on endpoints that should be skipped by code-generation public static final String OMIT_FROM_CODEGEN_PROPERTY = "omitFromCodegen"; + // Annotation used to indicate that the specified API can return arbitrary, unparseable content + // such as ZK or filestore files + public static final String RAW_OUTPUT_PROPERTY = "rawOutput"; + public static final String GENERIC_ENTITY_PROPERTY = "genericEntity"; public static final String BINARY_CONTENT_TYPE_V2 = "application/vnd.apache.solr.javabin"; diff --git a/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java b/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java index a15195f6228..c1bb7116d72 100644 --- a/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java +++ b/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java @@ -308,10 +308,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti if (responseParser instanceof InputStreamResponseParser) { // SPECIAL CASE - NamedList namedList = new NamedList<>(); - namedList.add("stream", byteBuffer.toInputStream()); - namedList.add("responseStatus", 200); // always by this point - return namedList; + return InputStreamResponseParser.createInputStreamNamedList(200, byteBuffer.toInputStream()); } // note: don't bother using the Reader variant; it often throws UnsupportedOperationException diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperRead.java b/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperRead.java index 8ded06a7a01..bf0db85fcdb 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperRead.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperRead.java @@ -21,19 +21,21 @@ import static org.apache.solr.security.PermissionNameProvider.Name.ZK_READ_PERM; import jakarta.inject.Inject; +import jakarta.ws.rs.WebApplicationException; +import jakarta.ws.rs.core.StreamingOutput; +import java.io.IOException; +import java.io.OutputStream; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.apache.solr.client.api.endpoint.ZooKeeperReadApis; -import org.apache.solr.client.api.model.ZooKeeperFileResponse; import org.apache.solr.client.api.model.ZooKeeperListChildrenResponse; import org.apache.solr.client.api.model.ZooKeeperStat; import org.apache.solr.client.solrj.impl.BinaryResponseParser; import org.apache.solr.client.solrj.impl.XMLResponseParser; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.util.ContentStreamBase; import org.apache.solr.core.CoreContainer; import org.apache.solr.handler.admin.api.AdminAPIBase; import org.apache.solr.jersey.PermissionName; @@ -64,7 +66,7 @@ public ZookeeperRead(CoreContainer coreContainer, SolrQueryRequest req, SolrQuer /** Request contents of a znode, except security.json */ @Override @PermissionName(ZK_READ_PERM) - public ZooKeeperFileResponse readNode(String zkPath) { + public StreamingOutput readNode(String zkPath) { zkPath = sanitizeZkPath(zkPath); return readNodeAndAddToResponse(zkPath); } @@ -72,7 +74,7 @@ public ZooKeeperFileResponse readNode(String zkPath) { /** Request contents of the security.json node */ @Override @PermissionName(SECURITY_READ_PERM) - public ZooKeeperFileResponse readSecurityJsonNode() { + public StreamingOutput readSecurityJsonNode() { return readNodeAndAddToResponse("/security.json"); } @@ -141,18 +143,19 @@ private String guessMime(byte firstByte) { } /** Reads content of a znode */ - private ZooKeeperFileResponse readNodeAndAddToResponse(String zkPath) { - final ZooKeeperFileResponse zkFileResponse = - instantiateJerseyResponse(ZooKeeperFileResponse.class); - + private StreamingOutput readNodeAndAddToResponse(String zkPath) { byte[] d = readPathFromZookeeper(zkPath); if (d == null || d.length == 0) { - zkFileResponse.zkData = EMPTY; - return zkFileResponse; + d = new byte[0]; } - zkFileResponse.output = new ContentStreamBase.ByteArrayStream(d, null, guessMime(d[0])); - return zkFileResponse; + final var bytesToWrite = d; + return new StreamingOutput() { + @Override + public void write(OutputStream output) throws IOException, WebApplicationException { + output.write(bytesToWrite); + } + }; } /** Reads a single node from zookeeper and return as byte array */ diff --git a/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java index fd87fe340bc..928109df2e0 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java @@ -17,20 +17,18 @@ package org.apache.solr.handler.admin; -import static org.apache.solr.common.util.StrUtils.split; -import static org.apache.solr.common.util.Utils.getObjectByPath; import static org.hamcrest.Matchers.containsInAnyOrder; -import com.fasterxml.jackson.databind.ObjectMapper; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.stream.Collectors; -import org.apache.solr.client.api.model.ZooKeeperListChildrenResponse; +import org.apache.commons.io.IOUtils; import org.apache.solr.client.api.model.ZooKeeperStat; import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.request.ZookeeperReadApi; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrException; -import org.apache.solr.common.util.Utils; import org.apache.zookeeper.CreateMode; import org.junit.After; import org.junit.Before; @@ -69,20 +67,29 @@ public void tearDown() throws Exception { @Test public void testZkread() throws Exception { try (HttpSolrClient client = new HttpSolrClient.Builder(baseUrl.toString()).build()) { - Object o = - Utils.executeGET(client.getHttpClient(), basezk + "/security.json", Utils.JSONCONSUMER); - assertNotNull(o); - o = Utils.executeGET(client.getHttpClient(), basezkls + "/configs", Utils.JSONCONSUMER); + final var securityJsonRequest = new ZookeeperReadApi.ReadNode("/security.json"); + final var securityJsonResponse = securityJsonRequest.process(client); + assertEquals(200, securityJsonResponse.getHttpStatus()); + try (final var stream = securityJsonResponse.getResponseStream()) { + final var securityJsonContent = IOUtils.toString(stream, StandardCharsets.UTF_8); + assertNotNull(securityJsonContent); + } + + final var configListRequest = new ZookeeperReadApi.ListNodes("/configs"); + final var configListResponse = configListRequest.process(client).getParsed(); assertEquals( - "16", - String.valueOf(getObjectByPath(o, true, split(":/configs:_default:dataLength", ':')))); + 16, configListResponse.unknownProperties().get("/configs").get("_default").dataLength); assertEquals( - "16", String.valueOf(getObjectByPath(o, true, split(":/configs:conf:dataLength", ':')))); - assertEquals("0", String.valueOf(getObjectByPath(o, true, split("/stat/version", '/')))); - - o = Utils.executeGET(client.getHttpClient(), basezk + "/configs", Utils.JSONCONSUMER); - assertTrue(((Map) o).containsKey("zkData")); - assertEquals("empty", ((Map) o).get("zkData")); + 16, configListResponse.unknownProperties().get("/configs").get("conf").dataLength); + assertEquals(0, configListResponse.stat.version); + + final var configDataRequest = new ZookeeperReadApi.ReadNode("/configs"); + final var configDataResponse = configDataRequest.process(client); + // /configs exists but has no data, so API returns '200 OK' with empty response body + assertEquals(200, configDataResponse.getHttpStatus()); + try (final var stream = configDataResponse.getResponseStream()) { + assertEquals("", IOUtils.toString(stream, StandardCharsets.UTF_8)); + } byte[] bytes = new byte[1024 * 5]; for (int i = 0; i < bytes.length; i++) { @@ -92,17 +99,16 @@ public void testZkread() throws Exception { cluster .getZkClient() .create("/configs/_default/testdata", bytes, CreateMode.PERSISTENT, true); - Utils.executeGET( - client.getHttpClient(), - basezk + "/configs/_default/testdata", - is -> { - byte[] newBytes = new byte[bytes.length]; - is.read(newBytes); - for (int i = 0; i < newBytes.length; i++) { - assertEquals(bytes[i], newBytes[i]); - } - return null; - }); + + final var testDataRequest = new ZookeeperReadApi.ReadNode("/configs/_default/testdata"); + final var testDataResponse = testDataRequest.process(client); + assertEquals(200, testDataResponse.getHttpStatus()); + try (final var stream = testDataResponse.getResponseStream()) { + final var foundContents = stream.readAllBytes(); + for (int i = 0; i < foundContents.length; i++) { + assertEquals(foundContents[i], bytes[i]); + } + } } finally { cluster.getZkClient().delete("/configs/_default/testdata", -1, true); } @@ -112,15 +118,13 @@ public void testZkread() throws Exception { @Test public void testRequestingDataFromNonexistentNodeReturnsAnError() throws Exception { try (HttpSolrClient client = new HttpSolrClient.Builder(baseUrl.toString()).build()) { - final SolrException expected = + final var missingNodeReq = new ZookeeperReadApi.ReadNode("/configs/_default/nonexistentnode"); + final var missingNodeResponse = missingNodeReq.process(client); + assertEquals(404, missingNodeResponse.getHttpStatus()); + + final var expected = expectThrows( - SolrException.class, - () -> { - Utils.executeGET( - client.getHttpClient(), - basezk + "/configs/_default/nonexistentnode", - Utils.JSONCONSUMER); - }); + SolrException.class, () -> missingNodeResponse.getResponseStreamIfSuccessful()); assertEquals(404, expected.code()); } } @@ -128,26 +132,23 @@ public void testRequestingDataFromNonexistentNodeReturnsAnError() throws Excepti @Test public void testCanListChildNodes() throws Exception { try (HttpSolrClient client = new HttpSolrClient.Builder(baseUrl.toString()).build()) { - final ZooKeeperListChildrenResponse response = - Utils.executeGET( - client.getHttpClient(), - basezkls + "/configs/_default", - is -> { - return new ObjectMapper().readValue(is, ZooKeeperListChildrenResponse.class); - }); + final var listDefaultFilesReq = new ZookeeperReadApi.ListNodes("/configs/_default"); + final var listDefaultFilesResponse = listDefaultFilesReq.process(client).getParsed(); // At the top level, the response contains a key with the value of the specified zkPath - assertEquals(1, response.unknownProperties().size()); + assertEquals(1, listDefaultFilesResponse.unknownProperties().size()); assertEquals( "/configs/_default", - response.unknownProperties().keySet().stream().collect(Collectors.toList()).get(0)); + listDefaultFilesResponse.unknownProperties().keySet().stream() + .collect(Collectors.toList()) + .get(0)); // Under the specified zkPath is a key for each child, with values being that stat for that // node. // The actual stat values vary a good bit so aren't very useful to assert on, so let's just // make sure all of the expected child nodes were found. final Map childStatsByPath = - response.unknownProperties().get("/configs/_default"); + listDefaultFilesResponse.unknownProperties().get("/configs/_default"); assertEquals(6, childStatsByPath.size()); assertThat( childStatsByPath.keySet(), diff --git a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java index 35a7b021d64..f03ca147e75 100644 --- a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java +++ b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java @@ -64,7 +64,7 @@ public static JSONTupleStream create(SolrClient server, SolrParams requestParams query.setResponseParser(new InputStreamResponseParser("json")); query.setMethod(SolrRequest.METHOD.POST); NamedList genericResponse = server.request(query); - InputStream stream = (InputStream) genericResponse.get("stream"); + InputStream stream = (InputStream) genericResponse.get(InputStreamResponseParser.STREAM_KEY); InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8); return new JSONTupleStream(reader); } diff --git a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java index 1f06c852fd1..72425040b18 100644 --- a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java +++ b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java @@ -302,7 +302,7 @@ private TupleStreamParser constructParser(SolrParams requestParams) var client = clientCache.getHttpSolrClient(baseUrl); NamedList genericResponse = client.request(query); - InputStream stream = (InputStream) genericResponse.get("stream"); + InputStream stream = (InputStream) genericResponse.get(InputStreamResponseParser.STREAM_KEY); CloseableHttpResponse httpResponse = (CloseableHttpResponse) genericResponse.get("closeableResponse"); diff --git a/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/graph/GraphExpressionTest.java b/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/graph/GraphExpressionTest.java index 487dec09b6f..90a47751395 100644 --- a/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/graph/GraphExpressionTest.java +++ b/solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/graph/GraphExpressionTest.java @@ -1250,7 +1250,7 @@ public void testGraphHandler() throws Exception { NamedList genericResponse = client.request(query); - InputStream stream = (InputStream) genericResponse.get("stream"); + InputStream stream = (InputStream) genericResponse.get(InputStreamResponseParser.STREAM_KEY); InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8); String xml = readString(reader); // Validate the nodes diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/InputStreamResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/InputStreamResponse.java new file mode 100644 index 00000000000..b86e76f13ac --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/InputStreamResponse.java @@ -0,0 +1,132 @@ +/* + * 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; + +import static org.apache.solr.client.solrj.impl.InputStreamResponseParser.HTTP_STATUS_KEY; +import static org.apache.solr.client.solrj.impl.InputStreamResponseParser.STREAM_KEY; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.invoke.MethodHandles; +import java.nio.charset.Charset; +import java.util.function.Function; +import org.apache.solr.client.solrj.impl.InputStreamResponseParser; +import org.apache.solr.client.solrj.response.SimpleSolrResponse; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.IOUtils; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.ObjectReleaseTracker; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Represents the NamedList response format created by {@link InputStreamResponseParser}. + * + *

Particularly useful when targeting APIs that return arbitrary or binary data (e.g. replication + * APIs for fetching index files) + */ +public class InputStreamResponse extends SimpleSolrResponse { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + // True if the HTTP status is '200 OK', false otherwise + public static final Function HTTP_OK_VALIDATOR = (status) -> 200 == status; + // True if the HTTP status is neither a 4xx or a 5xx error. + public static final Function NON_ERROR_CODE_VALIDATOR = + (status) -> status < 399; + + @Override + public void setResponse(NamedList rsp) { + if (rsp.get(STREAM_KEY) == null) { + throw new IllegalArgumentException( + "Missing key '" + + STREAM_KEY + + "'; " + + getClass().getSimpleName() + + " can only be used with requests or clients configured to use " + + InputStreamResponseParser.class.getSimpleName()); + } + super.setResponse(rsp); + } + + public int getHttpStatus() { + return (int) getResponse().get(HTTP_STATUS_KEY); + } + + /** + * Access the server response as an {@link InputStream}, regardless of the HTTP status code + * + *

Caller is responsible for consuming and closing the stream, and releasing it from the + * tracking done by {@link ObjectReleaseTracker}. No validation is done on the HTTP status code. + */ + public InputStream getResponseStream() { + final NamedList resp = getResponse(); + + return (InputStream) resp.get(STREAM_KEY); + } + + /** + * Access the server response as an {@link InputStream}, after ensuring that the HTTP status code + * is 200 ('OK') + * + *

Caller is responsible for consuming and closing the stream, and releasing it from the + * tracking done by {@link ObjectReleaseTracker}. + */ + public InputStream getResponseStreamIfSuccessful() { + return getResponseStreamIfSuccessful(HTTP_OK_VALIDATOR); + } + + /** + * Access the server response as an {@link InputStream}, after ensuring the HTTP status code + * passes a provided validator. + * + * @param statusValidator a function that returns true iff the response body should be returned + */ + public InputStream getResponseStreamIfSuccessful(Function statusValidator) { + validateExpectedStatus(statusValidator); + return getResponseStream(); + } + + private void validateExpectedStatus(Function statusChecker) { + final var httpStatus = getHttpStatus(); + if (!statusChecker.apply(httpStatus)) { + try { + log.error( + "Request returned unexpected HTTP status code {}; response content: {}", + httpStatus, + consumeAndStringifyForLogging(getResponseStream())); + } catch (IOException e) { + log.error("could not print error", e); + } + throw new SolrException( + SolrException.ErrorCode.getErrorCode(httpStatus), + "Unexpected status code [" + httpStatus + "] on response."); + } + } + + private String consumeAndStringifyForLogging(InputStream inputStream) throws IOException { + final var baos = new ByteArrayOutputStream(); + try { + inputStream.transferTo(baos); + return baos.toString(Charset.defaultCharset()); + } finally { + ObjectReleaseTracker.release(inputStream); + IOUtils.closeQuietly(baos); + IOUtils.closeQuietly(inputStream); + } + } +} 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 e1bfd69ef74..277ab627684 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 @@ -611,12 +611,11 @@ protected NamedList executeMethod( } } if (processor == null || processor instanceof InputStreamResponseParser) { - // no processor specified, return raw stream - NamedList rsp = new NamedList<>(); - rsp.add("stream", respBody); + final var rsp = + InputStreamResponseParser.createInputStreamNamedList( + response.getStatusLine().getStatusCode(), respBody); rsp.add("closeableResponse", response); - rsp.add("responseStatus", response.getStatusLine().getStatusCode()); // Only case where stream should not be closed shouldClose = false; return rsp; 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 8683b965ad0..adf655cb2c6 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 @@ -217,13 +217,10 @@ protected NamedList processErrorsAndResponse( } if (wantStream(processor)) { - // no processor specified, return raw stream - NamedList rsp = new NamedList<>(); - rsp.add("stream", is); - rsp.add("responseStatus", httpStatus); // Only case where stream should not be closed shouldClose = false; - return rsp; + // no processor specified, return raw stream + return InputStreamResponseParser.createInputStreamNamedList(httpStatus, is); } checkContentType(processor, is, mimeType, encoding, httpStatus, urlExceptionMessage); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/InputStreamResponseParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/InputStreamResponseParser.java index 530851c3fa8..50ea192dea7 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/InputStreamResponseParser.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/InputStreamResponseParser.java @@ -20,10 +20,18 @@ import java.io.Reader; import org.apache.solr.client.solrj.ResponseParser; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; -/** Simply puts the InputStream into an entry in a NamedList named "stream". */ +/** + * Simply puts the InputStream into an entry in a NamedList named "stream". + * + * @see org.apache.solr.client.solrj.InputStreamResponse + */ public class InputStreamResponseParser extends ResponseParser { + public static String STREAM_KEY = "stream"; + public static String HTTP_STATUS_KEY = "responseStatus"; + private final String writerType; public InputStreamResponseParser(String writerType) { @@ -44,4 +52,12 @@ public NamedList processResponse(Reader reader) { public NamedList processResponse(InputStream body, String encoding) { throw new UnsupportedOperationException(); } + + public static NamedList createInputStreamNamedList( + int httpStatus, InputStream inputStream) { + final var nl = new SimpleOrderedMap<>(); + nl.add(STREAM_KEY, inputStream); + nl.add(HTTP_STATUS_KEY, httpStatus); + return nl; + } } diff --git a/solr/solrj/src/resources/java-template/api.mustache b/solr/solrj/src/resources/java-template/api.mustache index c4ef7f9c59b..f276f3f0a72 100644 --- a/solr/solrj/src/resources/java-template/api.mustache +++ b/solr/solrj/src/resources/java-template/api.mustache @@ -36,6 +36,7 @@ import org.apache.solr.client.solrj.JacksonParsingResponse; import org.apache.solr.client.solrj.JacksonContentWriter; import org.apache.solr.client.solrj.request.RequestWriter.ContentWriter; import org.apache.solr.client.solrj.impl.InputStreamResponseParser; +import org.apache.solr.client.solrj.InputStreamResponse; import org.apache.solr.client.solrj.ResponseParser; {{! Covers all top-level request/response model classes, but not necessarily any types nested in those classes }} @@ -86,11 +87,16 @@ public class {{classname}} { {{#operation}} {{^vendorExtensions.x-omitFromCodegen}} - public static class {{operationIdCamelCase}}Response extends JacksonParsingResponse<{{modelPackage}}.{{returnType}}> { - public {{operationIdCamelCase}}Response() { - super({{modelPackage}}.{{returnType}}.class); - } + {{#vendorExtensions.x-rawOutput}} + public static class {{operationIdCamelCase}}Response extends InputStreamResponse {} + {{/vendorExtensions.x-rawOutput}} + {{^vendorExtensions.x-rawOutput}} + public static class {{operationIdCamelCase}}Response extends JacksonParsingResponse<{{modelPackage}}.{{returnType}}> { + public {{operationIdCamelCase}}Response() { + super({{modelPackage}}.{{returnType}}.class); + } } + {{/vendorExtensions.x-rawOutput}} public static class {{operationIdCamelCase}} extends SolrRequest<{{operationIdCamelCase}}Response> { {{#bodyParam}} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/InputStreamResponseTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/InputStreamResponseTest.java new file mode 100644 index 00000000000..6737c310832 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/InputStreamResponseTest.java @@ -0,0 +1,80 @@ +/* + * 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; + +import static org.hamcrest.Matchers.containsString; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.impl.InputStreamResponseParser; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.NamedList; +import org.junit.Test; + +/** Unit tests for {@link InputStreamResponse} */ +public class InputStreamResponseTest extends SolrTestCase { + + @Test + public void testDetectsWhenResponseDoesntMatchExpectedFormat() { + final var inputStreamResponse = new InputStreamResponse(); + final var rawNl = new NamedList(); + rawNl.add("someKey", "someVal"); + + final var thrown = + expectThrows( + IllegalArgumentException.class, + () -> { + inputStreamResponse.setResponse(rawNl); + }); + assertThat(thrown.getMessage(), containsString("Missing key 'stream'")); + } + + @Test + public void testAllowsAccessToStatusAndStream() throws IOException { + final var inputStreamResponse = new InputStreamResponse(); + final var rawNl = + InputStreamResponseParser.createInputStreamNamedList( + 200, new ByteArrayInputStream(new byte[] {'h', 'e', 'l', 'l', 'o'})); + inputStreamResponse.setResponse(rawNl); + + assertEquals(200, inputStreamResponse.getHttpStatus()); + try (final var is = inputStreamResponse.getResponseStream()) { + final var streamVal = new String(is.readAllBytes(), StandardCharsets.UTF_8); + assertEquals("hello", streamVal); + } + } + + @Test + public void testThrowsErrorIfUnexpectedResponseEncountered() { + final var inputStreamResponse = new InputStreamResponse(); + final var rawNl = + InputStreamResponseParser.createInputStreamNamedList( + 500, new ByteArrayInputStream(new byte[] {'h', 'e', 'l', 'l', 'o'})); + inputStreamResponse.setResponse(rawNl); + + final var thrown = + expectThrows( + SolrException.class, + () -> { + inputStreamResponse.getResponseStreamIfSuccessful(); + }); + assertEquals(500, thrown.code()); // Matches that status of the HTTP response + assertThat(thrown.getMessage(), containsString("Unexpected status code")); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java index 17ce59e1cc4..694d808a9ce 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java @@ -459,7 +459,7 @@ protected void testGetRawStream(HttpSolrClientBase client) throws Exception { final var req = new QueryRequest(params("q", "*:*")); req.setResponseParser(new InputStreamResponseParser("xml")); final var rsp = req.process(client); - Object stream = rsp.getResponse().get("stream"); + Object stream = rsp.getResponse().get(InputStreamResponseParser.STREAM_KEY); assertNotNull(stream); assertThat(stream, instanceOf(InputStream.class)); InputStream is = (InputStream) stream;