epugh commented on code in PR #4178: URL: https://github.com/apache/solr/pull/4178#discussion_r4524445257
########## solr/api/src/java/org/apache/solr/client/api/endpoint/PackageApis.java: ########## @@ -0,0 +1,116 @@ +/* + * 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.solr.client.api.endpoint; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.parameters.RequestBody; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.QueryParam; +import org.apache.solr.client.api.model.AddPackageVersionRequestBody; +import org.apache.solr.client.api.model.GetPackageResponse; +import org.apache.solr.client.api.model.ListPackagesResponse; +import org.apache.solr.client.api.model.SolrJerseyResponse; + +/** V2 API definitions for managing Solr packages. */ +@Path("/cluster/packages") +public interface PackageApis { + + @GET + @Operation( + summary = "List all packages registered in this Solr cluster.", + tags = {"package"}) + ListPackagesResponse listPackages( + @Parameter( + description = + "If provided, the node waits until its package data matches this ZooKeeper version.") + @QueryParam("expectedVersion") + Integer expectedVersion); + + @GET + @Path("/{packageName}") + @Operation( + summary = "Get information about a specific package in this Solr cluster.", + tags = {"package"}) + GetPackageResponse getPackage( + @Parameter(description = "The name of the package.", required = true) + @PathParam("packageName") + String packageName, + @Parameter( + description = + "If provided, the node waits until its package data matches this ZooKeeper version before responding.") + @QueryParam("expectedVersion") + Integer expectedVersion); + + @POST + @Path("/{packageName}/versions") + @Operation( + summary = "Add a version of a package to this Solr cluster.", + tags = {"package"}) + SolrJerseyResponse addPackageVersion( + @Parameter(description = "The name of the package.", required = true) + @PathParam("packageName") + String packageName, + @RequestBody(description = "Details of the package version to add.", required = true) + AddPackageVersionRequestBody requestBody); + + @DELETE + @Path("/{packageName}/versions/{version}") + @Operation( + summary = "Delete a specific version of a package from this Solr cluster.", + tags = {"package"}) + SolrJerseyResponse deletePackageVersion( + @Parameter(description = "The name of the package.", required = true) + @PathParam("packageName") + String packageName, + @Parameter(description = "The version of the package to delete.", required = true) + @PathParam("version") + String version); + + @POST + @Path("/{packageName}/refresh") + @Operation( + summary = "Refresh a package on all nodes in this Solr cluster.", + tags = {"package"}) + SolrJerseyResponse refreshPackage( + @Parameter(description = "The name of the package to refresh.", required = true) + @PathParam("packageName") + String packageName, + @Parameter( + description = + "Indicates whether the refresh should only be done on the receiving node. For internal use only.") + @QueryParam("localOnly") + Boolean localOnly); + + @POST + @Path("/sync") Review Comment: Per `dev-docs/apis-v2-conventions.adoc`'s "Unproxied APIs" section: paths like `/api/node/key` exist specifically to mark operations that only affect the *receiving* node and must never be proxied. `syncPackages()` fits that exactly -- it only ever touches the local `PackageStore` (see `ClusterPackage.syncToVersion`), and is invoked exclusively via `broadcastToLiveNodes()`/`processWithBaseUrl()` targeting one specific node's base URL. The `@Operation` summary here says as much: "Used internally for inter-node fan-out after a package change; not normally invoked directly." Recommend `POST /api/node/packages/sync` instead of `/cluster/packages/sync`, following the existing `/node/{resource}` pattern (`NodeSystemInfoApi` → `/node/system`, `NodeHealthApi` → `/node/health`, and especially `GetNodeCommandStatusApi` → `/node/commands/`, the closest precedent as another receiving-node-only, not-cluster-tracked concept) combined with the "command API" convention already used elsewhere in this PR (`refreshPackage` → `.../refresh`): `packages` as the resource, `sync` as the appended command verb. Only this one endpoint would need to move -- `listPackages`/`getPackage` are genuinely cluster-wide, proxy-friendly reads and should stay under `/cluster/packages`. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
