yifan-c commented on code in PR #239: URL: https://github.com/apache/cassandra-sidecar/pull/239#discussion_r2240622285
########## OPENAPI.md: ########## @@ -0,0 +1,96 @@ +<!-- +# +# 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. +# +--> + +# OpenAPI Documentation Generation + +This project includes Gradle tasks for generating comprehensive OpenAPI documentation for the Cassandra Sidecar APIs. + +## Available Tasks + +### `generateOpenApiDocs` +Generates OpenAPI documentation files in multiple formats: +- **JSON**: `server/build/docs/openapi/openapi.json` - OpenAPI 3.0 specification in JSON format +- **YAML**: `server/build/docs/openapi/openapi.yaml` - OpenAPI 3.0 specification in YAML format +- **HTML**: `server/build/docs/openapi/api-docs.html` - Interactive HTML documentation with Swagger UI + +```bash +# Generate documentation files +./gradlew generateOpenApiDocs +``` + +### `openApiDocs` +Generates the documentation and automatically opens the HTML version in your default browser. + +```bash +# Generate and open documentation +./gradlew openApiDocs +``` + +## Generated Documentation Features + +The generated documentation includes: + +- ✅ **Complete API Coverage** - All endpoints with proper HTTP methods +- ✅ **Interactive UI** - Test APIs directly from the documentation +- ✅ **Response Examples** - Sample JSON responses for each endpoint +- ✅ **Error Documentation** - HTTP status codes and error descriptions +- ✅ **Tag Organization** - Endpoints grouped by functionality (Health, Schema, Snapshots, etc.) +- ✅ **Server Information** - API version, description, and license details + +## Runtime API Documentation + +In addition to static documentation generation, the running Sidecar server provides: + +- **OpenAPI Spec**: `GET http://localhost:9043/api/v1/openapi.json` +- **Interactive Docs**: `GET http://localhost:9043/api/v1/docs` Review Comment: I do not think it is supposed to be used as API. I would rephrase it as the following ```suggestion - **Interactive Docs**: Open `http://localhost:9043/api/v1/docs` in browser ``` ########## .gitignore: ########## @@ -57,6 +57,7 @@ target/ *.bak *.sw[o,p] *.tmp +*.class Review Comment: The class files should only appear inside the build folders and are ignored by git already. Does this PR somehow introduces a different `.class` file output location? ########## server/src/main/java/org/apache/cassandra/sidecar/config/yaml/OpenApiConfigurationImpl.java: ########## @@ -0,0 +1,203 @@ +/* + * 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.config.yaml; + +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.cassandra.sidecar.config.OpenApiConfiguration; + +/** + * Implementation of {@link OpenApiConfiguration} + */ +public class OpenApiConfigurationImpl implements OpenApiConfiguration +{ + public static final boolean DEFAULT_ENABLED = true; + public static final String DEFAULT_TITLE = "Cassandra Sidecar API"; + public static final String DEFAULT_DESCRIPTION = "REST API for managing Apache Cassandra operations"; + public static final String DEFAULT_VERSION = "1.0.0"; Review Comment: The version should be sourced from Sidecar's build version. The version file can be found at `/sidecar.version` ########## server/src/main/java/org/apache/cassandra/sidecar/docs/OpenApiDocumentationGenerator.java: ########## Review Comment: It is concerning to have to maintain all those of html generation code. Instead of making project-specific annotations, could we leverage existing tools like https://github.com/microprofile/microprofile-open-api/ for annotation and https://github.com/smallrye/smallrye-open-api/tree/main/tools/gradle-plugin for doc generation? ########## conf/sidecar.yaml: ########## @@ -398,3 +398,14 @@ live_migration: - glob:${DATA_FILE_DIR}/*/*/snapshots # Excludes snapshot directories in data folder to copy to destination migration_map: # Map of source and destination Cassandra instances # localhost1: localhost4 # This entry says that localhost1 will be migrated to localhost4 + +# OpenAPI documentation configuration +openapi: + enabled: true + title: "Cassandra Sidecar API" + description: "REST API for managing Apache Cassandra operations" + version: "1.0.0" + license_name: "Apache License 2.0" + license_url: "https://www.apache.org/licenses/LICENSE-2.0"; + server_url: "http://localhost:9043/api/v1"; + server_description: "Development server" Review Comment: I do not think making this configurable is necessary. ########## server/src/main/java/org/apache/cassandra/sidecar/config/OpenApiConfiguration.java: ########## @@ -0,0 +1,65 @@ +/* + * 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.config; + +/** + * Configuration for OpenAPI documentation + */ +public interface OpenApiConfiguration Review Comment: We can remove the config if the openAPI configuration is removed from yaml. ########## server/build.gradle: ########## @@ -128,6 +128,17 @@ dependencies { // DataHub client is used to convert and report Cassandra schema on a periodic basis implementation(group: 'io.acryl', name: 'datahub-client', version: '0.15.0-3') + + // OpenAPI support + implementation('org.eclipse.microprofile.openapi:microprofile-openapi-api:3.1.1') + implementation('io.swagger.core.v3:swagger-core:2.2.21') + implementation('io.swagger.core.v3:swagger-annotations:2.2.21') + implementation('io.swagger.core.v3:swagger-models:2.2.21') + implementation('io.swagger.core.v3:swagger-jaxrs2:2.2.21') + implementation('org.webjars:swagger-ui:5.17.14') + + // For HTML documentation generation + compileOnly('io.swagger.codegen.v3:swagger-codegen-cli:3.0.46') Review Comment: Wondering if we want to create a separate sub-project for docs-gen and introduce those dependencies in that sub-project? ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java: ########## @@ -157,6 +157,10 @@ public final class ApiEndpointsV1 public static final String LIVE_MIGRATION_FILE_TRANSFER_API = LIVE_MIGRATION_FILES_API + "/:" + DIR_TYPE_PARAM + "/:" + DIR_INDEX_PARAM + "/*"; + public static final String OPENAPI_ROUTE = API_V1 + "/openapi.json"; + public static final String SWAGGER_UI_ROUTE = API_V1 + "/docs"; + public static final String WEBJARS_ROUTE = "/webjars/*"; Review Comment: Can we remove the webjars? The patch is only to introduce openAPI spec docs ########## server/src/main/java/org/apache/cassandra/sidecar/cluster/locator/CachedLocalTokenRanges.java: ########## @@ -205,22 +205,26 @@ private synchronized Map<Integer, Set<TokenRange>> getCacheOrReload(Metadata met if (isClusterTheSame && localTokenRangesCache != null && localTokenRangesCache.containsKey(ks.getName())) { // we don't need to rebuild if already cached - perKeyspaceBuilder.put(ks.getName(), localTokenRangesCache.get(ks.getName())); + Map<Integer, Set<TokenRange>> cachedRanges = localTokenRangesCache.get(ks.getName()); + if (cachedRanges != null) + { + perKeyspaceBuilder.put(ks.getName(), cachedRanges); + continue; + } } - else + + // Build token ranges for this keyspace + ImmutableMap.Builder<Integer, Set<TokenRange>> resultBuilder = ImmutableMap.builder(); + for (InstanceMetadata instance : localInstances) { - ImmutableMap.Builder<Integer, Set<TokenRange>> resultBuilder = ImmutableMap.builder(); - for (InstanceMetadata instance : localInstances) + Pair<Host, Set<TokenRange>> pair = tokenRangesOfHost(metadata, keyspace, instance, allHosts); + if (pair != null) { - Pair<Host, Set<TokenRange>> pair = tokenRangesOfHost(metadata, keyspace, instance, allHosts); - if (pair != null) - { - hostBuilder.add(pair.getKey()); - resultBuilder.put(instance.id(), Collections.unmodifiableSet(pair.getValue())); - } + hostBuilder.add(pair.getKey()); + resultBuilder.put(instance.id(), Collections.unmodifiableSet(pair.getValue())); } - perKeyspaceBuilder.put(ks.getName(), resultBuilder.build()); Review Comment: The changes are unrelated to the patch. Is there any bug in this code? -- 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
