yifan-c commented on code in PR #239: URL: https://github.com/apache/cassandra-sidecar/pull/239#discussion_r2257772546
########## server/src/main/java/org/apache/cassandra/sidecar/handlers/OpenApiHandler.java: ########## @@ -0,0 +1,164 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Singleton; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; + +/** + * Handler that serves the OpenAPI specification + * + * This handler serves the OpenAPI specification generated by the Gradle openApiGenerate task Review Comment: 👍 on mentioning the gradle task. However, I think the task name is wrong. Is it `generateOpenApiSpec`? ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/OpenApiHandler.java: ########## @@ -0,0 +1,164 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Singleton; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; + +/** + * Handler that serves the OpenAPI specification + * + * This handler serves the OpenAPI specification generated by the Gradle openApiGenerate task + * which processes JAX-RS/MicroProfile annotations to create comprehensive API documentation. + */ +@Singleton +public class OpenApiHandler implements Handler<RoutingContext> +{ + private static final Logger logger = LoggerFactory.getLogger(OpenApiHandler.class); + + @Override + public void handle(RoutingContext context) + { + try + { + // Determine format from request path or Accept header + boolean isYaml = isYamlRequest(context); + String openApiContent = loadGeneratedOpenApiSpec(isYaml); + String contentType = isYaml ? "application/yaml" : "application/json"; + + context.response() + .putHeader("Content-Type", contentType) + .end(openApiContent); + } + catch (Exception e) + { + logger.warn("Failed to load generated OpenAPI specification, falling back to basic config", e); + context.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()) + .putHeader("Content-Type", "application/json") + .end(); + } + } + + /** + * Determines if the request is for YAML format based on path or Accept header + */ + private boolean isYamlRequest(RoutingContext context) + { + String path = context.request().path(); + if (path != null && path.endsWith(".yaml")) + { + return true; + } + + String acceptHeader = context.request().getHeader("Accept"); + return acceptHeader != null && acceptHeader.contains("application/yaml"); + } + + /** + * Loads the generated OpenAPI specification from resources or build output + */ + private String loadGeneratedOpenApiSpec(boolean isYaml) throws IOException + { + // First try to load from classpath resources (for packaged deployments) + String content = loadFromClasspathResource(isYaml); + if (content != null) + { + return content; + } + + // Fallback to file system paths (for development) + return loadFromFileSystem(isYaml); + } + + /** + * Attempts to load the OpenAPI spec from classpath resources + */ + private String loadFromClasspathResource(boolean isYaml) throws IOException + { + String fileName = isYaml ? "openapi.yaml" : "openapi.json"; + String resourcePath = "/openapi/" + fileName; + + InputStream inputStream = getClass().getResourceAsStream(resourcePath); + if (inputStream == null) + { + return null; + } + + logger.debug("Loading OpenAPI specification from classpath: {}", resourcePath); + try + { + byte[] bytes = inputStream.readAllBytes(); + String content = new String(bytes, StandardCharsets.UTF_8); + logger.info("Loaded OpenAPI specification from classpath resource"); + return content; + } + finally + { + inputStream.close(); + } + } + + /** + * Attempts to load the OpenAPI spec from file system (development mode) + */ + private String loadFromFileSystem(boolean isYaml) throws IOException + { + String fileName = isYaml ? "openapi.yaml" : "openapi.json"; + String[] possiblePaths = { + "server/build/generated/openapi/" + fileName, // From project root + "build/generated/openapi/" + fileName, // From server directory + "../server/build/generated/openapi/" + fileName, // Alternative relative path + }; + + Path foundPath = null; + for (String pathStr : possiblePaths) + { + Path path = Paths.get(pathStr); + if (Files.exists(path)) + { + foundPath = path; + break; + } + } + + if (foundPath == null) + { + throw new IOException("Generated OpenAPI specification not found in classpath resources or file system. " + + "Please run './gradlew openApiGenerate' and ensure the generated file is included in the build."); Review Comment: Please update the gradle task name here as well. Or just mention that read the document on generating the openAPI specs w/o mention a specific gradle task name. Would defer to your choice. ########## server/src/test/java/org/apache/cassandra/sidecar/handlers/OpenApiHandlerTest.java: ########## @@ -0,0 +1,259 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.util.Modules; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.Vertx; +import io.vertx.core.buffer.Buffer; +import io.vertx.ext.web.client.HttpResponse; +import io.vertx.ext.web.client.WebClient; +import io.vertx.ext.web.codec.BodyCodec; +import io.vertx.junit5.VertxExtension; +import io.vertx.junit5.VertxTestContext; +import org.apache.cassandra.sidecar.TestModule; +import org.apache.cassandra.sidecar.modules.SidecarModules; +import org.apache.cassandra.sidecar.server.Server; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for the {@link OpenApiHandler} class + */ +@ExtendWith(VertxExtension.class) +class OpenApiHandlerTest +{ + private static final Logger LOGGER = LoggerFactory.getLogger(OpenApiHandlerTest.class); + private static final ObjectMapper JSON_MAPPER = new ObjectMapper(); + private static final ObjectMapper YAML_MAPPER = new ObjectMapper(new YAMLFactory()); + + private Vertx vertx; + private Server server; + + @BeforeEach + void setUp() throws InterruptedException + { + Injector injector = Guice.createInjector(Modules.override(SidecarModules.all()).with(new TestModule())); + server = injector.getInstance(Server.class); + vertx = injector.getInstance(Vertx.class); + + VertxTestContext context = new VertxTestContext(); + server.start() + .onSuccess(s -> context.completeNow()) + .onFailure(context::failNow); + + context.awaitCompletion(5, TimeUnit.SECONDS); + } + + @AfterEach + void tearDown() throws InterruptedException + { + if (server != null) + { + CountDownLatch closeLatch = new CountDownLatch(1); + server.close().onSuccess(res -> closeLatch.countDown()); + if (closeLatch.await(60, TimeUnit.SECONDS)) + LOGGER.info("Close event received before timeout."); + else + LOGGER.error("Close event timed out."); + } + } + + @Test + void testJsonRequestByPath(VertxTestContext testContext) + { + WebClient client = WebClient.create(vertx); + client.get(server.actualPort(), "localhost", "/openapi.json") + .as(BodyCodec.buffer()) + .send(resp -> { + HttpResponse<Buffer> response = resp.result(); + assertThat(response.statusCode()).isEqualTo(HttpResponseStatus.OK.code()); + assertThat(response.getHeader("Content-Type")).isEqualTo("application/json"); + + String body = response.bodyAsString(); + assertThat(body).isNotEmpty(); + + // Validate that the response is valid JSON + try + { + JSON_MAPPER.readTree(body); + testContext.completeNow(); + } + catch (Exception e) + { + testContext.failNow(new AssertionError("Response is not valid JSON: " + e.getMessage())); + } + }); + } + + @Test + void testYamlRequestByPath(VertxTestContext testContext) + { + WebClient client = WebClient.create(vertx); + client.get(server.actualPort(), "localhost", "/openapi.yaml") + .as(BodyCodec.buffer()) + .send(resp -> { + HttpResponse<Buffer> response = resp.result(); + assertThat(response.statusCode()).isEqualTo(HttpResponseStatus.OK.code()); + assertThat(response.getHeader("Content-Type")).isEqualTo("application/yaml"); + + String body = response.bodyAsString(); + assertThat(body).isNotEmpty(); + + // Validate that the response is valid YAML + try + { + YAML_MAPPER.readTree(body); + testContext.completeNow(); + } + catch (Exception e) + { + testContext.failNow(new AssertionError("Response is not valid YAML: " + e.getMessage())); + } + }); + } + + @Test + void testJsonRequestByAcceptHeader(VertxTestContext testContext) + { + WebClient client = WebClient.create(vertx); + client.get(server.actualPort(), "localhost", "/openapi") + .putHeader("Accept", "application/json") + .as(BodyCodec.buffer()) + .send(resp -> { + HttpResponse<Buffer> response = resp.result(); + assertThat(response.statusCode()).isEqualTo(HttpResponseStatus.OK.code()); + assertThat(response.getHeader("Content-Type")).isEqualTo("application/json"); + + String body = response.bodyAsString(); + assertThat(body).isNotEmpty(); + + // Validate that the response is valid JSON + try + { + JSON_MAPPER.readTree(body); + testContext.completeNow(); + } + catch (Exception e) + { + testContext.failNow(new AssertionError("Response is not valid JSON: " + e.getMessage())); + } + }); + } + + @Test + void testYamlRequestByAcceptHeader(VertxTestContext testContext) + { + WebClient client = WebClient.create(vertx); + client.get(server.actualPort(), "localhost", "/openapi") + .putHeader("Accept", "application/yaml") + .as(BodyCodec.buffer()) + .send(resp -> { + HttpResponse<Buffer> response = resp.result(); + assertThat(response.statusCode()).isEqualTo(HttpResponseStatus.OK.code()); + assertThat(response.getHeader("Content-Type")).isEqualTo("application/yaml"); + + String body = response.bodyAsString(); + assertThat(body).isNotEmpty(); + + // Validate that the response is valid YAML + try + { + YAML_MAPPER.readTree(body); + testContext.completeNow(); + } + catch (Exception e) + { + testContext.failNow(new AssertionError("Response is not valid YAML: " + e.getMessage())); + } + }); + } + + @ParameterizedTest + @ValueSource(strings = {"application/json, application/yaml", "text/html, application/yaml", "application/yaml; q=0.9"}) Review Comment: nice ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/OpenApiHandler.java: ########## @@ -0,0 +1,164 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Singleton; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.Handler; +import io.vertx.ext.web.RoutingContext; + +/** + * Handler that serves the OpenAPI specification + * + * This handler serves the OpenAPI specification generated by the Gradle openApiGenerate task + * which processes JAX-RS/MicroProfile annotations to create comprehensive API documentation. + */ +@Singleton +public class OpenApiHandler implements Handler<RoutingContext> +{ + private static final Logger logger = LoggerFactory.getLogger(OpenApiHandler.class); Review Comment: nit: static variable are in uppercase, so `LOGGER` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org