Skip to content

Commit

Permalink
SOLR-17562: Unify JAX-RS "raw response" endpoints (#3032)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gerlowskija committed Jan 20, 2025
1 parent 50b728e commit 70bcd76
Show file tree
Hide file tree
Showing 19 changed files with 334 additions and 117 deletions.
2 changes: 2 additions & 0 deletions solr/api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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);

Expand All @@ -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:.*}")
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 15 additions & 12 deletions solr/core/src/java/org/apache/solr/handler/admin/ZookeeperRead.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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++) {
Expand All @@ -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);
}
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public static JSONTupleStream create(SolrClient server, SolrParams requestParams
query.setResponseParser(new InputStreamResponseParser("json"));
query.setMethod(SolrRequest.METHOD.POST);
NamedList<Object> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ private TupleStreamParser constructParser(SolrParams requestParams)

var client = clientCache.getHttpSolrClient(baseUrl);
NamedList<Object> genericResponse = client.request(query);
InputStream stream = (InputStream) genericResponse.get("stream");
InputStream stream = (InputStream) genericResponse.get(InputStreamResponseParser.STREAM_KEY);

CloseableHttpResponse httpResponse =
(CloseableHttpResponse) genericResponse.get("closeableResponse");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ public void testGraphHandler() throws Exception {

NamedList<Object> 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
Expand Down
Loading

0 comments on commit 70bcd76

Please sign in to comment.