SOLR-17562: Unify JAX-RS "raw response" endpoints

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/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";

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<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/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,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/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,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(),