You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "gerlowskija (via GitHub)" <gi...@apache.org> on 2023/06/26 16:16:33 UTC

[GitHub] [solr] gerlowskija commented on a diff in pull request #1679: SOLR-16392: Tweak v2 ADDREPLICA to be more REST-ful

gerlowskija commented on code in PR #1679:
URL: https://github.com/apache/solr/pull/1679#discussion_r1242393959


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -970,27 +965,17 @@ public Map<String, Object> execute(
     ADDREPLICA_OP(
         ADDREPLICA,
         (req, rsp, h) -> {
-          Map<String, Object> props =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  "node",
-                  SHARD_ID_PROP,
-                  _ROUTE_,
-                  CoreAdminParams.NAME,
-                  INSTANCE_DIR,
-                  DATA_DIR,
-                  ULOG_DIR,
-                  REPLICA_TYPE,
-                  WAIT_FOR_FINAL_STATE,
-                  NRT_REPLICAS,
-                  TLOG_REPLICAS,
-                  PULL_REPLICAS,
-                  CREATE_NODE_SET,
-                  FOLLOW_ALIASES,
-                  SKIP_NODE_ASSIGNMENT);
-          return copyPropertiesWithPrefix(req.getParams(), props, PROPERTY_PREFIX);
+          final var params = req.getParams();

Review Comment:
   [I] Application logic for Solr's APIs has historically lived in the `handleRequestBody` method of a "RequestHandler" class.  Often, a single RequestHandler will cover many logically distinct operations, one of which will be selected by users at request time via a specific query parameter.
   
   `CollectionsHandler.handleRequestBody` (which corresponds to the `/admin/collections` v1 API) looks for a query parameter called `action` and then uses a switch-case to execute different functionality based on the provided value.
   
   The application logic for this particular API is relatively simple - Solr is taking the user-provided parameters and formatting them into a message that can be stored in a work queue and processed later by a worker called the "overseer".  It is this work-queue processing that actually creates the replica (see AddReplicaCmd for the "real" replica-creation code).
   
   In any case, the logic behind the API itself "just" formats the message and stores it in ZK.
   
   ----
   
   Note that the v1 codepath (i.e. this RequestHandler) calls the v2 codepath (i.e. the API class) under the hood.  Structuring the code this way ensures that the v2 codepath is kept up to date if there are any changes to the v1 or v2 APIs.  It also ensures that the v2 codepath is tested implicitly in Solr's tests, even when users are making v1 requests. 



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateReplicaAPI.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * 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.handler.admin.api;
+
+import static org.apache.solr.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.PULL_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_TYPE;
+import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
+import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.NAME;
+import static org.apache.solr.common.params.CoreAdminParams.NODE;
+import static org.apache.solr.common.params.CoreAdminParams.ULOG_DIR;
+import static org.apache.solr.common.params.ShardParams._ROUTE_;
+import static org.apache.solr.handler.admin.api.CreateCollectionAPI.copyPrefixedPropertiesWithoutPrefix;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CollectionUtil;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SubResponseAccumulatingJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for adding a new replica to an existing shard.
+ *
+ * <p>This API (POST /v2/collections/cName/shards/sName/replicas {...}) is analogous to the v1
+ * /admin/collections?action=ADDREPLICA command.
+ */
+@Path("/collections/{collectionName}/shards/{shardName}/replicas")
+public class CreateReplicaAPI extends AdminAPIBase {

Review Comment:
   [I] This class extends 'AdminAPIBase', which abstracts out a lot of functionality shared among `CollectionsHandler` APIs, which usually operate by formatting a "message" about the work to be done and storing it on a work queue.  (See [this comment](https://github.com/apache/solr/pull/1679/files#diff-582348d44491dcb0ce1dfb169fb544e9e95620b2d0448eb1a1744f4e8dd5a349R968) in CollectionsHandler for more details.)



##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1543,7 +1529,6 @@ public Collection<Class<? extends JerseyResource>> getJerseyResources() {
   public Collection<Api> getApis() {
     final List<Api> apis = new ArrayList<>();
     apis.addAll(AnnotatedApi.getApis(new SplitShardAPI(this)));
-    apis.addAll(AnnotatedApi.getApis(new AddReplicaAPI(this)));

Review Comment:
   [I] Solr uses the `getApis` method to register v2 APIs that used our older "homegrown" API framework.  Since we're switching the add-replica functionality from the "homegrown" framework to the JAX-RS framework with this PR, we delete the registration line here.



##########
solr/solr-ref-guide/modules/deployment-guide/pages/replica-management.adoc:
##########
@@ -73,12 +73,9 @@ http://localhost:8983/solr/admin/collections?action=ADDREPLICA&collection=techpr
 
 [source,bash]
 ----
-curl -X POST http://localhost:8983/api/collections/techproducts/shards -H 'Content-Type: application/json' -d '
+curl -X POST http://localhost:8983/api/collections/techproducts/shards/shard1/replicas -H 'Content-Type: application/json' -d '

Review Comment:
   [I] Of course, if the v2 API is changing, the ref-guide docs should be updated to reflect its new form.  This often involves updating the path in the v2 example (if there is one).  If no v2 API example exists, it'd be ideal (but I guess not strictly required) to add one using syntax similar to that found here.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/AddReplicaAPI.java:
##########
@@ -1,72 +0,0 @@
-/*
- * 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.handler.admin.api;
-
-import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
-import static org.apache.solr.common.params.CollectionAdminParams.COLLECTION;
-import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
-import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
-import static org.apache.solr.common.params.CommonParams.ACTION;
-import static org.apache.solr.handler.ClusterAPI.wrapParams;
-import static org.apache.solr.handler.api.V2ApiUtils.flattenMapWithPrefix;
-import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
-
-import java.util.HashMap;
-import java.util.Map;
-import org.apache.solr.api.Command;
-import org.apache.solr.api.EndPoint;
-import org.apache.solr.api.PayloadObj;
-import org.apache.solr.client.solrj.request.beans.AddReplicaPayload;
-import org.apache.solr.common.params.CollectionParams;
-import org.apache.solr.handler.admin.CollectionsHandler;
-
-/**
- * V2 API for adding a new replica to an existing shard.
- *
- * <p>This API (POST /v2/collections/collectionName/shards {'add-replica': {...}}) is analogous to
- * the v1 /admin/collections?action=ADDREPLICA command.
- *
- * @see AddReplicaPayload
- */
-@EndPoint(
-    path = {"/c/{collection}/shards", "/collections/{collection}/shards"},
-    method = POST,
-    permission = COLL_EDIT_PERM)
-public class AddReplicaAPI {

Review Comment:
   [I] Github shows this file as "deleted", but in reality it has been renamed to CreateReplicaAPI.  The rename isn't strictly necessary and makes the PR a little more confusing, but CreateSomethingAPI fits a little better with the naming convention used in the rest of our JAX-RS APIs.



##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1514,6 +1499,7 @@ public Boolean registerV2() {
   @Override
   public Collection<Class<? extends JerseyResource>> getJerseyResources() {
     return List.of(
+        CreateReplicaAPI.class,

Review Comment:
   [I] Solr uses the `getJerseyResources` method to register JAX-RS APIs, so we add our "CreateReplicaAPI" class here.



##########
solr/CHANGES.txt:
##########
@@ -150,6 +150,10 @@ Improvements
 * SOLR-16392: The v2 "create shard" API has been tweaked to be more intuitive, by removing the top-level "create"
   command specifier.  The rest of the API remains unchanged. (Jason Gerlowski)
 
+* SOLR-16392: The v2 "add-replica" API has been tweaked to be more intuitive, by removing the top-level command specifier and

Review Comment:
   [I] This CHANGES.txt file is used by the project both as a way to credit contributors, and as a way for Solr users to learn about changes that might impact them as they prepare to upgrade.
   
   Entries are often written by the merging committer close to merge-time.  (Merge conflicts with this file are a common problem, so doing this last often saves headache.). But contributors are also welcome to write the CHANGES.txt entry for their change, if they'd wish.
   
   v2 API changes usually fit best in the "Improvements" section.  The trailing parenthesis section is intended to credit anyone who contributed on the ticket (with code, as a reviewer, etc.).  These are formatted as a single name in parenthesis (in the case of committers who merged their own code), or as `(Contributor1, Contributor2 via MergingCommitter)` in the more normal case of multiple contributors whose code was merged by a committer. 



##########
solr/core/src/test/org/apache/solr/handler/admin/api/CreateReplicaAPITest.java:
##########
@@ -0,0 +1,182 @@
+/*
+ * 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.handler.admin.api;
+
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.PULL_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_TYPE;
+import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.COLLECTION;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.NAME;
+import static org.apache.solr.common.params.CoreAdminParams.NODE;
+import static org.apache.solr.common.params.CoreAdminParams.ULOG_DIR;
+import static org.apache.solr.common.params.ShardParams._ROUTE_;
+
+import java.util.List;
+import java.util.Map;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.junit.Test;
+
+/** Unit tests for {@link CreateReplicaAPI} */
+public class CreateReplicaAPITest extends SolrTestCaseJ4 {

Review Comment:
   [I] Testing for v2 APIs is a bit complicated.
   
   Using integration tests would be ideal, but it's not really feasible to create a v2 test for each case exercised by our existing test suite (which uses the v1 APIs in almost all cases, as there is currently no Java client for making v2 API requests).
   
   Instead, most of our v2 APIs aim at good-enough coverage by:
   
   1. Writing unit tests to test as much of the application and API logic as possible at a "low" level. (As seen in this class)
   2. Structuring the v1 code to use the v2 code implicitly (see the [PR comment here](https://github.com/apache/solr/pull/1679/files#diff-582348d44491dcb0ce1dfb169fb544e9e95620b2d0448eb1a1744f4e8dd5a349R968) for more specifics.). This structural approach gives us at least some confidence that the v2 code is correct, since existing v1 tests are ultimately relying on it as well.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateReplicaAPI.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * 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.handler.admin.api;
+
+import static org.apache.solr.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.PULL_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_TYPE;
+import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
+import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.NAME;
+import static org.apache.solr.common.params.CoreAdminParams.NODE;
+import static org.apache.solr.common.params.CoreAdminParams.ULOG_DIR;
+import static org.apache.solr.common.params.ShardParams._ROUTE_;
+import static org.apache.solr.handler.admin.api.CreateCollectionAPI.copyPrefixedPropertiesWithoutPrefix;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CollectionUtil;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SubResponseAccumulatingJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for adding a new replica to an existing shard.
+ *
+ * <p>This API (POST /v2/collections/cName/shards/sName/replicas {...}) is analogous to the v1
+ * /admin/collections?action=ADDREPLICA command.
+ */
+@Path("/collections/{collectionName}/shards/{shardName}/replicas")
+public class CreateReplicaAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateReplicaAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  @POST
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public SubResponseAccumulatingJerseyResponse createReplica(
+      @PathParam("collectionName") String collectionName,
+      @PathParam("shardName") String shardName,
+      AddReplicaRequestBody requestBody)
+      throws Exception {
+    final var response = instantiateJerseyResponse(SubResponseAccumulatingJerseyResponse.class);
+    if (requestBody == null) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST, "Required request-body is missing");
+    }
+    ensureRequiredParameterProvided(COLLECTION_PROP, collectionName);
+    ensureRequiredParameterProvided(SHARD_ID_PROP, shardName);
+    final String resolvedCollectionName =
+        resolveAndValidateAliasIfEnabled(
+            collectionName, Boolean.TRUE.equals(requestBody.followAliases));
+
+    final ZkNodeProps remoteMessage =
+        createRemoteMessage(resolvedCollectionName, shardName, requestBody);
+    submitRemoteMessageAndHandleResponse(
+        response, CollectionParams.CollectionAction.ADDREPLICA, remoteMessage, requestBody.asyncId);
+    return response;
+  }
+
+  public static ZkNodeProps createRemoteMessage(
+      String collectionName, String shardName, AddReplicaRequestBody requestBody) {
+    final Map<String, Object> remoteMessage = new HashMap<>();
+    remoteMessage.put(QUEUE_OPERATION, CollectionParams.CollectionAction.ADDREPLICA.toLower());
+    remoteMessage.put(COLLECTION_PROP, collectionName);
+    remoteMessage.put(SHARD_ID_PROP, shardName);
+    insertIfNotNull(remoteMessage, CoreAdminParams.NAME, requestBody.name);
+    insertIfNotNull(remoteMessage, _ROUTE_, requestBody.route);
+    insertIfNotNull(remoteMessage, NODE, requestBody.node);
+    if (CollectionUtil.isNotEmpty(requestBody.nodeSet)) {
+      remoteMessage.put(CREATE_NODE_SET_PARAM, String.join(",", requestBody.nodeSet));
+    }
+    insertIfNotNull(remoteMessage, SKIP_NODE_ASSIGNMENT, requestBody.skipNodeAssignment);
+    insertIfNotNull(remoteMessage, INSTANCE_DIR, requestBody.instanceDir);
+    insertIfNotNull(remoteMessage, DATA_DIR, requestBody.dataDir);
+    insertIfNotNull(remoteMessage, ULOG_DIR, requestBody.ulogDir);
+    insertIfNotNull(remoteMessage, REPLICA_TYPE, requestBody.type);
+    insertIfNotNull(remoteMessage, WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
+    insertIfNotNull(remoteMessage, NRT_REPLICAS, requestBody.nrtReplicas);
+    insertIfNotNull(remoteMessage, TLOG_REPLICAS, requestBody.tlogReplicas);
+    insertIfNotNull(remoteMessage, PULL_REPLICAS, requestBody.pullReplicas);
+    insertIfNotNull(remoteMessage, FOLLOW_ALIASES, requestBody.followAliases);
+    insertIfNotNull(remoteMessage, ASYNC, requestBody.asyncId);
+
+    if (requestBody.properties != null) {
+      requestBody
+          .properties
+          .entrySet()
+          .forEach(
+              entry -> {
+                remoteMessage.put(PROPERTY_PREFIX + entry.getKey(), entry.getValue());
+              });
+    }
+
+    return new ZkNodeProps(remoteMessage);
+  }
+
+  public static class AddReplicaRequestBody implements JacksonReflectMapWriter {

Review Comment:
   [I] The RequestBody object here implements "JacksonReflectMapWriter", which allows Solr to deserialize it from a number of different input formats (particularly XML, JSON, and "javabin" - a custom binary format supported by Solr)
   
   Jackson annotations are used to designate properties that can be serialized/deserialized.  An annotation value only needs specified when the property name is expected to differ from the variable name.
   
   This class is roughly analogous with the "Payload" class used in v2 POST/PUT APIs the use the old "homegrown" framework.  If a Payload class exists, it can usually be copied here with only minor modification.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateReplicaAPI.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * 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.handler.admin.api;
+
+import static org.apache.solr.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.PULL_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_TYPE;
+import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
+import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR;
+import static org.apache.solr.common.params.CoreAdminParams.NAME;
+import static org.apache.solr.common.params.CoreAdminParams.NODE;
+import static org.apache.solr.common.params.CoreAdminParams.ULOG_DIR;
+import static org.apache.solr.common.params.ShardParams._ROUTE_;
+import static org.apache.solr.handler.admin.api.CreateCollectionAPI.copyPrefixedPropertiesWithoutPrefix;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.CollectionUtil;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SubResponseAccumulatingJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for adding a new replica to an existing shard.
+ *
+ * <p>This API (POST /v2/collections/cName/shards/sName/replicas {...}) is analogous to the v1
+ * /admin/collections?action=ADDREPLICA command.
+ */
+@Path("/collections/{collectionName}/shards/{shardName}/replicas")
+public class CreateReplicaAPI extends AdminAPIBase {
+
+  @Inject

Review Comment:
   [I] Solr's integration with the JAX-RS framework allows us to inject objects into the class ctor.  (While this can be changed - the framework currently instantiates a new CreateReplicaAPI instance for each incoming request.)
   
   Currently the types available for injection are relatively limited, but this can grow over time as needed.  See `InjectionFactories` and `JerseyApplications` for a better sense on the types currently available for injection, and for ideas on how to support injection of additional types.



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/AddReplicaPayload.java:
##########
@@ -1,56 +0,0 @@
-/*
- * 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.solrj.request.beans;
-
-import java.util.List;
-import java.util.Map;
-import org.apache.solr.common.annotation.JsonProperty;
-import org.apache.solr.common.util.ReflectMapWriter;
-
-public class AddReplicaPayload implements ReflectMapWriter {

Review Comment:
   [I] Payload classes were used by the "homegrown" annotation framework that we're in the process of moving away from, and have largely been superceded by the (very similar) "RequestBody" classes used by JAX-RS APIs.
   
   Very few changes are needed to affect this; see CreateReplicaAPI.CreateReplicaAPIRequestBody for more info. 



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org