SOLR-17562: Unify JAX-RS "raw response" endpoints (#3032)

Creates InputStreamResponse and corresponding tests to wrap the
NamedList produced by InputStreamResponseParser.  Modifies the codegen
mustache template to use this response type for any v2 APIs that are
tagged as producing 'rawOutput', including the zk-read and fetch-index-file
endpoints.

Author: gerlowskija

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.

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 {

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,

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(

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:.*}")

solr/api/src/java/org/apache/solr/client/api/model/ZooKeeperFileResponse.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";

solr/api/src/java/org/apache/solr/client/api/util/Constants.java

@@ -308,10 +308,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti
 
     if (responseParser instanceof InputStreamResponseParser) {
       // SPECIAL CASE
-      NamedList<Object> 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

solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.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,15 +66,15 @@ 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);
   }
 
   /** 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 */

solr/core/src/java/org/apache/solr/handler/admin/ZookeeperRead.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,42 +118,37 @@ 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());
     }
   }
 
   @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<String, ZooKeeperStat> childStatsByPath =
-          response.unknownProperties().get("/configs/_default");
+          listDefaultFilesResponse.unknownProperties().get("/configs/_default");
       assertEquals(6, childStatsByPath.size());
       assertThat(
           childStatsByPath.keySet(),