You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/07/13 19:57:10 UTC

[GitHub] [solr] gerlowskija opened a new pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

gerlowskija opened a new pull request #218:
URL: https://github.com/apache/solr/pull/218


   # Description
   
   The community is in the process of migrating v2 API implementations over to an annotation-based framework that results in less duplication and easier to understand code.  But the /c/<coll>/shards APIs still need to use this. 
   
   # Solution
   
   This PR reimplements the /c/<coll>/shards APIs using the annotation framework.
   
   # Tests
   
   See V2ShardsAPIMappingTest
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [x] I have created a Jira issue and added the issue ID to my pull request title.
   - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [x] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [x] I have added tests for my changes.
   
   


-- 
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


[GitHub] [solr] epugh commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669115816



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/AddReplicaPayload.java
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class AddReplicaPayload implements ReflectMapWriter {
+  @JsonProperty
+  public String shard;
+
+  @JsonProperty
+  public String _route_;
+
+  // TODO Could 'node' be replaced (in v2 and v1) by just specifying a 'createNodeSet' of size 1?

Review comment:
       I just read the "tip" in the doc that warns you you have to pick one or the other, this would be a nice simplifying thing.   We have too many gotchas in the behaviors as is!




-- 
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


[GitHub] [solr] gerlowskija commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669898730



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/AddReplicaPayload.java
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class AddReplicaPayload implements ReflectMapWriter {
+  @JsonProperty
+  public String shard;
+
+  @JsonProperty
+  public String _route_;
+
+  // TODO Could 'node' be replaced (in v2 and v1) by just specifying a 'createNodeSet' of size 1?

Review comment:
       OK, thanks for the +1 on the idea.  I'll delegate it to a separate ticket, since changing the API brings in 8x deprecation and other concerns. (See SOLR-15542)




-- 
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


[GitHub] [solr] epugh commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669116222



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/AddReplicaPayload.java
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class AddReplicaPayload implements ReflectMapWriter {
+  @JsonProperty
+  public String shard;
+
+  @JsonProperty
+  public String _route_;
+
+  // TODO Could 'node' be replaced (in v2 and v1) by just specifying a 'createNodeSet' of size 1?
+  @JsonProperty
+  public String node;
+
+  // TODO Should this just be 'nodeSet' to match the name used by create-shard and other APIs?

Review comment:
       LIkewise seems like a good idea.   Would these be done in this PR, or in future tickets?  




-- 
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


[GitHub] [solr] gerlowskija commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669899451



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/AddReplicaPayload.java
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class AddReplicaPayload implements ReflectMapWriter {
+  @JsonProperty
+  public String shard;
+
+  @JsonProperty
+  public String _route_;
+
+  // TODO Could 'node' be replaced (in v2 and v1) by just specifying a 'createNodeSet' of size 1?
+  @JsonProperty
+  public String node;
+
+  // TODO Should this just be 'nodeSet' to match the name used by create-shard and other APIs?

Review comment:
       I think a separate ticket, since there's technically deprecation and other concerns that come up here.  Maybe I can handle it as a part of SOLR-15542, which I created based on your comment above?




-- 
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


[GitHub] [solr] gerlowskija commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669902144



##########
File path: solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java
##########
@@ -27,6 +27,8 @@
   String IN_PLACE_MOVE = "inPlaceMove";
   /** Method to use for shard splitting. */
   String SPLIT_METHOD = "splitMethod";
+  /** Key to use during shard splitting */
+  String SPLIT_KEY = "split.key";

Review comment:
       That'd be my preference, yep.  This constant only uses dot-delimiting since that's what the v1 API used up to this point.  And it's only used for the v1 API, so there's the convention doesn't spill into v2-land.




-- 
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


[GitHub] [solr] epugh commented on pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #218:
URL: https://github.com/apache/solr/pull/218#issuecomment-967063294


   I think I am slightly confused, I was expecting these "apispec.json" files to go away, but they are over in solrj????
   
   https://github.com/gerlowskija/solr/blob/SOLR_15536_shards_v2_apis_using_annotation_framework/solr/solrj/src/resources/apispec/collections.collection.shards.shard.Commands.json
   
   Do we define the apis on both places?  Or am I jumping the gun and you just haven't gotton to deleting the related .json file?   Great to see progress...


-- 
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


[GitHub] [solr] epugh commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669904361



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/AddReplicaPayload.java
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.util.ReflectMapWriter;
+
+import java.util.List;
+import java.util.Map;
+
+public class AddReplicaPayload implements ReflectMapWriter {
+  @JsonProperty
+  public String shard;
+
+  @JsonProperty
+  public String _route_;
+
+  // TODO Could 'node' be replaced (in v2 and v1) by just specifying a 'createNodeSet' of size 1?
+  @JsonProperty
+  public String node;
+
+  // TODO Should this just be 'nodeSet' to match the name used by create-shard and other APIs?

Review comment:
       I like doing it as part of SOLR-15542 as well.  




-- 
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


[GitHub] [solr] epugh commented on pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #218:
URL: https://github.com/apache/solr/pull/218#issuecomment-967072572


   Okay, one more morning thought....   Do we actually need the `captureConvertedV1Params` and the mapping between V1 and V2 tests?   Do we care that V2 is different then V1????    Especially now that it's "experimental" and we might make a LOT of changes...     DO we limit ourselves by having this mapping check?    I'm curious how having these tests (which also look complex and a LOT of work) help us build a better V2 API?   
   
   A part of me wants to pick a common use case for the API, and draw out the steps and the related API calls, and see where we are overly verbose or overly terse to inform our V2 structure ;-)


-- 
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


[GitHub] [solr] gerlowskija commented on pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #218:
URL: https://github.com/apache/solr/pull/218#issuecomment-968089083


   > Do we actually need the captureConvertedV1Params and the mapping between V1 and V2 tests? Do we care that V2 is different then V1????
   
   I think so, but maybe you can correct me.  Here's the case I'd make for them:
   
   Right now our v2 APIs are mostly written as a thin interface that calls out to existing v1 functionality.  e.g. `AddReplicaAPI` doesn't really have replica-adding logic - it just shuffles some parameters around and calls the v1 logic that already exists in `CollectionsHandler.handleRequestBody`.
   
   So to have confidence in our v2 API, we need to verify two things:
   1. That `CollectionsHandler.handleRequestBody` works as expected.
   2. That the "parameter shuffling" AddReplicaAPI does before calling `handleRequestBody` is correct.
   
   The first is already verified by many tests throughout the codebase.  So that box is checked.  We should already have lots of confidence in `/solr/admin/collections?action=ADDREPLICA`.
   
   That just leaves (2).
   
   To trust AddReplicaAPIs replica-property support, we need to verify that AddReplicaAPI's JSON map of properties gets flattened and prefixed with `property.`.  To trust AddReplicaAPI's handling of nodeset values we need to verify that they are converted from the nice JSON list that AddReplicaAPI offers to the comma-delimited string that v1 requires. And so on.
   
   That's what the tests in this PR are doing.  They're not ensuring that v1 and v2 are the same - they don't have to be.  They're ensuring that when a v2 API makes use of v1 code, it's invoking that code correctly.


-- 
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


[GitHub] [solr] epugh commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669116901



##########
File path: solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java
##########
@@ -27,6 +27,8 @@
   String IN_PLACE_MOVE = "inPlaceMove";
   /** Method to use for shard splitting. */
   String SPLIT_METHOD = "splitMethod";
+  /** Key to use during shard splitting */
+  String SPLIT_KEY = "split.key";

Review comment:
       Should this be `splitKey` in a perfect world?




-- 
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


[GitHub] [solr] gerlowskija commented on pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #218:
URL: https://github.com/apache/solr/pull/218#issuecomment-879360543


   FYI @epugh since you've been kind enough to review similar patches in the past.
   


-- 
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


[GitHub] [solr] gerlowskija commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669893442



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/api/SplitShardAPI.java
##########
@@ -0,0 +1,87 @@
+/*
+ * 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 org.apache.commons.collections.MapUtils;
+import org.apache.commons.lang3.StringUtils;
+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.AddReplicaPropertyPayload;
+import org.apache.solr.client.solrj.request.beans.SplitShardPayload;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CommonAdminParams;
+import org.apache.solr.handler.admin.CollectionsHandler;
+
+import java.util.HashMap;
+import java.util.Map;
+
+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.PROPERTY_PREFIX;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.handler.ClusterAPI.wrapParams;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+/**
+ * V2 API for splitting an existing shard up into multiple pieces.
+ *
+ * This API (POST /v2/collections/collectionName/shards {'split': {...}}) is analogous to the v1
+ * /admin/collections?action=SPLITSHARD command.
+ *
+ * @see SplitShardPayload
+ */
+@EndPoint(
+        path = {"/c/{collection}/shards", "/collections/{collection}/shards"},
+        method = POST,
+        permission = COLL_EDIT_PERM)
+public class SplitShardAPI {
+  private static final String V2_SPLIT_CMD = "split";
+
+  private final CollectionsHandler collectionsHandler;
+
+  public SplitShardAPI(CollectionsHandler collectionsHandler) {
+    this.collectionsHandler = collectionsHandler;
+  }
+
+  @Command(name = V2_SPLIT_CMD)
+  public void splitShard(PayloadObj<SplitShardPayload> obj) throws Exception {
+    final SplitShardPayload v2Body = obj.get();
+    final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+    v1Params.put(ACTION, CollectionParams.CollectionAction.SPLITSHARD.toLower());
+    v1Params.put(COLLECTION, obj.getRequest().getPathTemplateValues().get(COLLECTION));
+
+    if (StringUtils.isNotEmpty(v2Body.splitKey)) {
+      v1Params.put(CommonAdminParams.SPLIT_KEY, v2Body.splitKey);
+    }
+    if (MapUtils.isNotEmpty(v2Body.coreProperties)) {
+      flattenMapWithPrefix(v2Body.coreProperties, v1Params, PROPERTY_PREFIX);
+    }
+    collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+  }
+
+  private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,

Review comment:
       Sometimes laziness or obliviousness gets in the way, but I'm always for DRY in theory.  Will fix.




-- 
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


[GitHub] [solr] epugh commented on a change in pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #218:
URL: https://github.com/apache/solr/pull/218#discussion_r669079557



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/api/SplitShardAPI.java
##########
@@ -0,0 +1,87 @@
+/*
+ * 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 org.apache.commons.collections.MapUtils;
+import org.apache.commons.lang3.StringUtils;
+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.AddReplicaPropertyPayload;
+import org.apache.solr.client.solrj.request.beans.SplitShardPayload;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CommonAdminParams;
+import org.apache.solr.handler.admin.CollectionsHandler;
+
+import java.util.HashMap;
+import java.util.Map;
+
+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.PROPERTY_PREFIX;
+import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.handler.ClusterAPI.wrapParams;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+/**
+ * V2 API for splitting an existing shard up into multiple pieces.
+ *
+ * This API (POST /v2/collections/collectionName/shards {'split': {...}}) is analogous to the v1
+ * /admin/collections?action=SPLITSHARD command.
+ *
+ * @see SplitShardPayload
+ */
+@EndPoint(
+        path = {"/c/{collection}/shards", "/collections/{collection}/shards"},
+        method = POST,
+        permission = COLL_EDIT_PERM)
+public class SplitShardAPI {
+  private static final String V2_SPLIT_CMD = "split";
+
+  private final CollectionsHandler collectionsHandler;
+
+  public SplitShardAPI(CollectionsHandler collectionsHandler) {
+    this.collectionsHandler = collectionsHandler;
+  }
+
+  @Command(name = V2_SPLIT_CMD)
+  public void splitShard(PayloadObj<SplitShardPayload> obj) throws Exception {
+    final SplitShardPayload v2Body = obj.get();
+    final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+    v1Params.put(ACTION, CollectionParams.CollectionAction.SPLITSHARD.toLower());
+    v1Params.put(COLLECTION, obj.getRequest().getPathTemplateValues().get(COLLECTION));
+
+    if (StringUtils.isNotEmpty(v2Body.splitKey)) {
+      v1Params.put(CommonAdminParams.SPLIT_KEY, v2Body.splitKey);
+    }
+    if (MapUtils.isNotEmpty(v2Body.coreProperties)) {
+      flattenMapWithPrefix(v2Body.coreProperties, v1Params, PROPERTY_PREFIX);
+    }
+    collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+  }
+
+  private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,

Review comment:
       Should this method `flattenMapWithPrefix` be shared?    I know some projects have a really strong "DRY" feeling, and others, it's okay to share. 




-- 
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


[GitHub] [solr] epugh commented on pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #218:
URL: https://github.com/apache/solr/pull/218#issuecomment-967070550


   > I think I am slightly confused, I was expecting these "apispec.json" files to go away, but they are over in solrj????
   > 
   > https://github.com/gerlowskija/solr/blob/SOLR_15536_shards_v2_apis_using_annotation_framework/solr/solrj/src/resources/apispec/collections.collection.shards.shard.Commands.json
   > 
   > Do we define the apis on both places? Or am I jumping the gun and you just haven't gotton to deleting the related .json file? Great to see progress...
   
   Never mind, I didn't scroll far enough down...?


-- 
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


[GitHub] [solr] gerlowskija merged pull request #218: SOLR-15536: Rewrite /c//shards v2 APIs using annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija merged pull request #218:
URL: https://github.com/apache/solr/pull/218


   


-- 
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