Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions changelog/unreleased/solr-18072-refactor-admin-cmd-apis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
title: Refactor CollectionApiCommands to add easily expandable AdminCmdContext argument
type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other
authors:
- name: Houston Putman
nick: HoustonPutman
links:
- name: SOLR-18072
url: https://issues.apache.org/jira/browse/SOLR-18072
2 changes: 1 addition & 1 deletion gradle/validation/validate-source-patterns.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class ValidateSourcePatternsTask extends DefaultTask {
}

def checkMockitoAssume = {f, text ->
if (text.contains("mockito") && !text.contains("assumeWorkingMockito()")) {
if (text.contains("mockito") && !(text.contains("assumeWorkingMockito()") || text.contains("extends MockAPITest"))) {
reportViolation(f, 'File uses Mockito but has no assumeWorkingMockito() call')
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import org.apache.solr.client.api.model.AddReplicaPropertyRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

@Path("/collections/{collName}/shards/{shardName}/replicas/{replicaName}/properties/{propName}")
public interface AddReplicaPropertyApi {
Expand All @@ -33,7 +33,7 @@ public interface AddReplicaPropertyApi {
@Operation(
summary = "Adds a property to the specified replica",
tags = {"replica-properties"})
public SolrJerseyResponse addReplicaProperty(
public SubResponseAccumulatingJerseyResponse addReplicaProperty(
@Parameter(
description = "The name of the collection the replica belongs to.",
required = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import jakarta.ws.rs.PathParam;
import org.apache.solr.client.api.model.GetAliasPropertyResponse;
import org.apache.solr.client.api.model.GetAllAliasPropertiesResponse;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
import org.apache.solr.client.api.model.UpdateAliasPropertiesRequestBody;
import org.apache.solr.client.api.model.UpdateAliasPropertyRequestBody;

Expand Down Expand Up @@ -56,7 +56,7 @@ GetAliasPropertyResponse getAliasProperty(
@Operation(
summary = "Update properties for a collection alias.",
tags = {"alias-properties"})
SolrJerseyResponse updateAliasProperties(
SubResponseAccumulatingJerseyResponse updateAliasProperties(
@Parameter(description = "Alias Name") @PathParam("aliasName") String aliasName,
@RequestBody(description = "Properties that need to be updated", required = true)
UpdateAliasPropertiesRequestBody requestBody)
Expand All @@ -67,7 +67,7 @@ SolrJerseyResponse updateAliasProperties(
@Operation(
summary = "Update a specific property for a collection alias.",
tags = {"alias-properties"})
SolrJerseyResponse createOrUpdateAliasProperty(
SubResponseAccumulatingJerseyResponse createOrUpdateAliasProperty(
@Parameter(description = "Alias Name") @PathParam("aliasName") String aliasName,
@Parameter(description = "Property Name") @PathParam("propName") String propName,
@RequestBody(description = "Property value that needs to be updated", required = true)
Expand All @@ -79,7 +79,7 @@ SolrJerseyResponse createOrUpdateAliasProperty(
@Operation(
summary = "Delete a specific property for a collection alias.",
tags = {"alias-properties"})
SolrJerseyResponse deleteAliasProperty(
SubResponseAccumulatingJerseyResponse deleteAliasProperty(
@Parameter(description = "Alias Name") @PathParam("aliasName") String aliasName,
@Parameter(description = "Property Name") @PathParam("propName") String propName)
throws Exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import org.apache.solr.client.api.model.BalanceReplicasRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

@Path("cluster/replicas/balance")
public interface BalanceReplicasApi {
@POST
@Operation(
summary = "Balance Replicas across the given set of Nodes.",
tags = {"cluster"})
SolrJerseyResponse balanceReplicas(
SubResponseAccumulatingJerseyResponse balanceReplicas(
@RequestBody(description = "Contains user provided parameters")
BalanceReplicasRequestBody requestBody)
throws Exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import org.apache.solr.client.api.model.CreateAliasRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

@Path("/aliases")
public interface CreateAliasApi {
@POST
@Operation(
summary = "Create a traditional or 'routed' alias",
tags = {"aliases"})
SolrJerseyResponse createAlias(CreateAliasRequestBody requestBody) throws Exception;
SubResponseAccumulatingJerseyResponse createAlias(CreateAliasRequestBody requestBody)
throws Exception;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.QueryParam;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

@Path("/aliases/{aliasName}")
public interface DeleteAliasApi {
Expand All @@ -32,7 +32,7 @@ public interface DeleteAliasApi {
@Operation(
summary = "Deletes an alias by its name",
tags = {"aliases"})
SolrJerseyResponse deleteAlias(
SubResponseAccumulatingJerseyResponse deleteAlias(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Q] Why are we narrowing the response schema of this API?

Typically "SubResponseAccumulatingJerseyResponse" gets used for those admin APIs where the overseer sends out requests to individual replicas, etc. and includes each of those sub-responses in what gets returned to the original caller. But looking at DeleteAliasCmd, it doesn't seem to be doing anything like that?

There's a (minor?) downside to doing this in that our OpenAPI spec (and everything that gets generated from it) will now look like the additional fields will be present in the API response.

(Note: this question applies to a few of the files above as well, but just leaving it in this one place to avoid duplication.)

@Parameter(description = "The name of the alias to delete", required = true)
@PathParam("aliasName")
String aliasName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import org.apache.solr.client.api.model.DeleteNodeRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

@Path("cluster/nodes/{nodeName}/clear/")
public interface DeleteNodeApi {
Expand All @@ -33,7 +33,7 @@ public interface DeleteNodeApi {
@Operation(
summary = "Delete all replicas off of the specified SolrCloud node",
tags = {"node"})
SolrJerseyResponse deleteNode(
SubResponseAccumulatingJerseyResponse deleteNode(
@Parameter(
description =
"The name of the node to be cleared. Usually of the form 'host:1234_solr'.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import org.apache.solr.client.api.model.InstallShardDataRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

/** V2 API definition allowing users to import offline-constructed index into a shard. */
@Path("/collections/{collName}/shards/{shardName}/install")
Expand All @@ -30,7 +30,7 @@ public interface InstallShardDataApi {
@Operation(
summary = "Install offline index into an existing shard",
tags = {"shards"})
SolrJerseyResponse installShardData(
SubResponseAccumulatingJerseyResponse installShardData(
@PathParam("collName") String collName,
@PathParam("shardName") String shardName,
InstallShardDataRequestBody requestBody)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import org.apache.solr.client.api.model.MigrateReplicasRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

/** V2 API definition for migrating replicas from a set of nodes to another set of nodes. */
@Path("cluster/replicas/migrate")
Expand All @@ -30,7 +30,7 @@ public interface MigrateReplicasApi {
@Operation(
summary = "Migrate Replicas from a given set of nodes.",
tags = {"cluster"})
SolrJerseyResponse migrateReplicas(
SubResponseAccumulatingJerseyResponse migrateReplicas(
@RequestBody(description = "Contains user provided parameters", required = true)
MigrateReplicasRequestBody requestBody)
throws Exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import org.apache.solr.client.api.model.ReplaceNodeRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;

/**
* V2 API definition for recreating replicas in one node (the source) on another node(s) (the
Expand All @@ -35,7 +35,7 @@ public interface ReplaceNodeApi {
@Operation(
summary = "'Replace' a specified node by moving all replicas elsewhere",
tags = {"node"})
SolrJerseyResponse replaceNode(
SubResponseAccumulatingJerseyResponse replaceNode(
@Parameter(description = "The name of the node to be replaced.", required = true)
@PathParam("sourceNodeName")
String sourceNodeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import io.swagger.v3.oas.annotations.media.Schema;

public class CreateCollectionSnapshotResponse extends AsyncJerseyResponse {
public class CreateCollectionSnapshotResponse extends SubResponseAccumulatingJerseyResponse {
@Schema(description = "The name of the collection.")
public String collection;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import io.swagger.v3.oas.annotations.media.Schema;

/** The Response for {@link org.apache.solr.client.api.endpoint.CollectionSnapshotApis.Delete} */
public class DeleteCollectionSnapshotResponse extends AsyncJerseyResponse {
public class DeleteCollectionSnapshotResponse extends SubResponseAccumulatingJerseyResponse {
@Schema(description = "The name of the collection.")
@JsonProperty(COLLECTION)
public String collection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
import static org.apache.solr.common.params.CommonAdminParams.TIMEOUT;
import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;

Expand Down Expand Up @@ -76,13 +75,13 @@ public AddReplicaCmd(CollectionCommandContext ccc) {
}

@Override
public void call(ClusterState state, ZkNodeProps message, NamedList<Object> results)
public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList<Object> results)
throws Exception {
addReplica(state, message, results, null);
addReplica(adminCmdContext, message, results, null);
}

List<ZkNodeProps> addReplica(
ClusterState clusterState,
AdminCmdContext adminCmdContext,
ZkNodeProps message,
NamedList<Object> results,
Runnable onComplete)
Expand All @@ -103,7 +102,7 @@ List<ZkNodeProps> addReplica(
collectionName = extCollectionName;
}

DocCollection coll = clusterState.getCollection(collectionName);
DocCollection coll = adminCmdContext.getClusterState().getCollection(collectionName);
if (coll == null) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Collection: " + collectionName + " does not exist");
Expand All @@ -117,7 +116,6 @@ List<ZkNodeProps> addReplica(
boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, false);
boolean skipCreateReplicaInClusterState =
message.getBool(SKIP_CREATE_REPLICA_IN_CLUSTER_STATE, false);
final String asyncId = message.getStr(ASYNC);

String node = message.getStr(CoreAdminParams.NODE);
String createNodeSetStr = message.getStr(CREATE_NODE_SET);
Expand Down Expand Up @@ -152,7 +150,7 @@ List<ZkNodeProps> addReplica(
List<CreateReplica> createReplicas =
buildReplicaPositions(
ccc.getSolrCloudManager(),
clusterState,
adminCmdContext.getClusterState(),
collectionName,
message,
numReplicas,
Expand All @@ -161,14 +159,17 @@ List<ZkNodeProps> addReplica(
.map(
replicaPosition ->
assignReplicaDetails(
ccc.getSolrCloudManager(), clusterState, message, replicaPosition))
ccc.getSolrCloudManager(),
adminCmdContext.getClusterState(),
message,
replicaPosition))
.collect(Collectors.toList());

ShardHandler shardHandler = ccc.newShardHandler();
ZkStateReader zkStateReader = ccc.getZkStateReader();

final ShardRequestTracker shardRequestTracker =
CollectionHandlingUtils.asyncRequestTracker(asyncId, ccc);
CollectionHandlingUtils.asyncRequestTracker(adminCmdContext, ccc);
for (CreateReplica createReplica : createReplicas) {
assert createReplica.coreName != null;
ModifiableSolrParams params =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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.cloud.api.collections;

import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.params.CollectionParams;

public class AdminCmdContext {
private final CollectionParams.CollectionAction action;
private final String asyncId;
private ClusterState clusterState;

public AdminCmdContext(CollectionParams.CollectionAction action) {
this(action, null);
}

public AdminCmdContext(CollectionParams.CollectionAction action, String asyncId) {
this.action = action;
this.asyncId = asyncId;
}

public CollectionParams.CollectionAction getAction() {
return action;
}

public String getAsyncId() {
return asyncId;
}

public ClusterState getClusterState() {
return clusterState;
}

public AdminCmdContext withClusterState(ClusterState clusterState) {
this.clusterState = clusterState;
return this;
}

public AdminCmdContext subRequestContext(CollectionParams.CollectionAction action) {
return subRequestContext(action, asyncId);
}

public AdminCmdContext subRequestContext(
CollectionParams.CollectionAction action, String asyncId) {
AdminCmdContext nextContext = new AdminCmdContext(action, asyncId);
return nextContext.withClusterState(clusterState);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import java.util.Map;
import org.apache.solr.cloud.OverseerSolrResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.CollectionProperties;
import org.apache.solr.common.cloud.ZkNodeProps;
import org.apache.solr.common.params.CollectionParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.handler.admin.CollectionsHandler;
Expand All @@ -52,7 +52,7 @@ protected AliasCmd(CollectionCommandContext ccc) {
* If the collection already exists then this is not an error.
*/
static NamedList<Object> createCollectionAndWait(
ClusterState clusterState,
AdminCmdContext adminCmdContext,
String aliasName,
Map<String, String> aliasMetadata,
String createCollName,
Expand Down Expand Up @@ -83,7 +83,11 @@ static NamedList<Object> createCollectionAndWait(
// CreateCollectionCmd.
// note: there's doesn't seem to be any point in locking on the collection name, so we don't.
// We currently should already have a lock on the alias name which should be sufficient.
new CreateCollectionCmd(ccc).call(clusterState, createMessage, results);
new CreateCollectionCmd(ccc)
.call(
adminCmdContext.subRequestContext(CollectionParams.CollectionAction.CREATE),
createMessage,
results);
} catch (SolrException e) {
// The collection might already exist, and that's okay -- we can adopt it.
if (!e.getMessage().contains("collection already exists")) {
Expand Down
Loading