You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/02/01 14:22:43 UTC

[GitHub] [lucene-solr] gerlowskija opened a new pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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


   # Description
   
   Solr supports two different ways to write v2 APIs: a JSON spec based
   approach, and one based on annotated POJOs.  The POJO method is now
   preferred.
   
   This commit switches the /v2/collections APIs over to the
   annotation-based approach.
   
   # Solution
   
   This commit introduces a relatively rote migration of the JSON spec over to annotated POJOs.  There's little of note here, except for cases where the v2 JSON spec was missing support for params supported by the v1 APIs.  Here I had to make a few decisions as to whether property names should be retained, etc.  A good example of this is the v2 backup API, which had several params not covered by the existing v2 spec.
   
   # Tests
   
   Adds a new test: `V2CollectionsAPIMappingTest.java` to test the v2->v1 conversion.
   
   # 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 `master` branch.
   - [ ] I have run `./gradlew check`.
   - [x] I have added tests for my changes.
   - [x] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
   


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

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



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


[GitHub] [lucene-solr] noblepaul commented on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#issuecomment-773232908


   Thanks for picking this up @gerlowskija 
   
   Please let me know if you need any help


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

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



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


[GitHub] [lucene-solr] sigram commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570137942



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateBody.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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 CreateBody implements ReflectMapWriter {
+    @JsonProperty(required = true)
+    public String name;
+
+    @JsonProperty
+    public String config;
+
+    @JsonProperty
+    public RouterInfo router;
+
+    @JsonProperty
+    public Integer numShards;
+
+    @JsonProperty
+    public String shards;
+
+    @JsonProperty
+    public Integer replicationFactor;
+
+    @JsonProperty
+    public Integer nrtReplicas;
+
+    @JsonProperty
+    public Integer tlogReplicas;
+
+    @JsonProperty
+    public Integer pullReplicas;
+
+    @JsonProperty
+    public List<String> nodeSet;
+
+    @JsonProperty
+    public Boolean shuffleNodes;
+
+    @JsonProperty
+    public Map<String, Object> properties;
+
+    @JsonProperty
+    public String async;
+
+    @JsonProperty
+    public Boolean waitForFinalState;
+
+    @JsonProperty
+    public Boolean perReplicaState;
+
+    public static class RouterInfo implements ReflectMapWriter {

Review comment:
       Router spec may in fact contain arbitrary properties in addition to these two - see `DocRouter.getRouterSpec`. Currently only `name` and `field` are used in the routers that Solr provides but other users may use some other properties in their implementations.




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

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



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


[GitHub] [lucene-solr] sigram commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570134139



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/BackupCollectionBody.java
##########
@@ -0,0 +1,51 @@
+/*
+ * 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;
+
+/**
+ * V2 API POJO for the /v2/collections 'backup-collection' command.
+ *
+ * Analogous to the request parameters for v1 /admin/collections?action=BACKUP API.
+ */
+public class BackupCollectionBody implements ReflectMapWriter {

Review comment:
       What do you think about renaming all of these to use `*Payload` instead of `*Body`?




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

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



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


[GitHub] [lucene-solr] gerlowskija edited a comment on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija edited a comment on pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#issuecomment-773515236


   Hey, thanks for offering @noblepaul  .  I do have one thing I was puzzling over:
   
   How is SolrJ's `getV2Request`/`V2RequestSupport` feature intended to work for annotation-based v2 APIs?  It looks like `V1ToV2ApiMapper` is really dependent on `CollectionApiMapping.Meta`, which the annotation-based APIs can't be found in obviously.
   
   There are some annotation-based v2 APIs whose SolrJ request classes implement V2RequestSupport (e.g. CollectionAdminRequest.ClusterProp), so there should be logic somewhere to handle that I imagine?  Unless getV2Request will blow up now if called on certain CollectionAdminRequest objects?
   
   More broadly I wonder whether there's any value in getV2Request going forward.  It never really "took off" to cover APIs other than CollectionAdmin, and there might be better ways to let SolrJ users dip their toes into the v2 waters (e.g. dedicated request classes that better align with the v2 paths/concepts? idk).
   
   ----
   
   If we don't have an approach for V2RequestSupport + annotation-based APIs worked out, and there's not strong feeling about carrying V2RequestSupport forward, I'd lean towards making this PR a master-only change, and deprecating getV2Request in 8.9 so it can be removed by this PR on master.   


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

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



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


[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -50,11 +81,144 @@ public void getCollections(SolrQueryRequest req, SolrQueryResponse rsp) throws E
     CollectionsHandler.CollectionOperation.LIST_OP.execute(req, rsp, collectionsHandler);
   }
 
+    @EndPoint(
+            path = {"/c", "/collections"},
+            method = POST,
+            permission = COLL_EDIT_PERM)
+    public class CollectionsCommands {
+
+        @Command(name = V2_BACKUP_CMD)
+        @SuppressWarnings("unchecked")
+        public void backupCollection(PayloadObj<BackupCollectionBody> obj) throws Exception {
+            final Map<String, Object> v1Params = obj.get().toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.BACKUP.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_RESTORE_CMD)
+        @SuppressWarnings("unchecked")
+        public void restoreBackup(PayloadObj<RestoreCollectionBody> obj) throws Exception {
+            final RestoreCollectionBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.RESTORE.toLower());
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollParams = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollParams);
+                v1Params.putAll(createCollParams);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void createAlias(PayloadObj<CreateAliasBody> obj) throws Exception {
+            final CreateAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATEALIAS.toLower());
+            if (! CollectionUtils.isEmpty(v2Body.collections)) {
+                final String collectionsStr = String.join(",", v2Body.collections);
+                v1Params.remove(V2ApiConstants.COLLECTIONS);
+                v1Params.put(V2ApiConstants.COLLECTIONS, collectionsStr);
+            }
+            if (v2Body.router != null) {
+                Map<String, Object> routerProperties = (Map<String, Object>) v1Params.remove(V2ApiConstants.ROUTER_KEY);
+                flattenMapWithPrefix(routerProperties, v1Params, ROUTER_PREFIX);
+            }
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollectionMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollectionMap);
+                flattenMapWithPrefix(createCollectionMap, v1Params, CREATE_COLLECTION_PREFIX);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_SET_ALIAS_PROP_CMD)
+        @SuppressWarnings("unchecked")
+        public void setAliasProperty(PayloadObj<SetAliasPropertyBody> obj) throws Exception {
+            final SetAliasPropertyBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.ALIASPROP.toLower());
+            // Flatten "properties" map into individual prefixed params
+            final Map<String, Object> propertiesMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.PROPERTIES_KEY);
+            flattenMapWithPrefix(propertiesMap, v1Params, PROPERTY_PREFIX);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_DELETE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void deleteAlias(PayloadObj<DeleteAliasBody> obj) throws Exception {
+            final DeleteAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.DELETEALIAS.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_COLLECTION_CMD)
+        @SuppressWarnings("unchecked")
+        public void create(PayloadObj<CreateBody> obj) throws Exception {
+            final CreateBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATE.toLower());
+            convertV2CreateCollectionMapToV1ParamMap(v1Params);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @SuppressWarnings("unchecked")
+        private void convertV2CreateCollectionMapToV1ParamMap(Map<String, Object> v2MapVals) {
+            // Keys are copied so that map can be modified as keys are looped through.
+            final Set<String> v2Keys = v2MapVals.keySet().stream().collect(Collectors.toSet());
+            for (String key : v2Keys) {
+                switch (key) {
+                    case V2ApiConstants.PROPERTIES_KEY:
+                        final Map<String, Object> propertiesMap = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.PROPERTIES_KEY);
+                        flattenMapWithPrefix(propertiesMap, v2MapVals, PROPERTY_PREFIX);
+                        break;
+                    case ROUTER_KEY:
+                        final Map<String, Object> routerProperties = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.ROUTER_KEY);
+                        flattenMapWithPrefix(routerProperties, v2MapVals, CollectionAdminParams.ROUTER_PREFIX);
+                        break;
+                    case V2ApiConstants.CONFIG:
+                        v2MapVals.put(CollectionAdminParams.COLL_CONF, v2MapVals.remove(V2ApiConstants.CONFIG));
+                        break;
+                    case V2ApiConstants.SHUFFLE_NODES:
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM, v2MapVals.remove(V2ApiConstants.SHUFFLE_NODES));
+                        break;
+                    case V2ApiConstants.NODE_SET:
+                        final List<String> nodeSetList = (List<String>) v2MapVals.remove(V2ApiConstants.NODE_SET);
+                        final String nodeSetStr = String.join(",", nodeSetList);
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetStr);
+                        break;
+                    default:
+                        break;
+                }
+            }
+        }
+
+        private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,
+                                          String additionalPrefix) {
+            if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
+                return;
+            }
+
+            toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));

Review comment:
       I actually tried this initially but found the result to be a lot like the code in `CollectionsApiMapping.Meta`, which I personally find tough to read with all the map-literals for renaming properties and prefixing properties.  I guessed it was more readable to just call this non-recursive version twice.  That wouldn't work as well on APIs with deeper nesting, but seemed feasible here.
   
   I'm open to going back that direction though if you prefer it.  Did you have a particular signature in mind?

##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -36,7 +58,16 @@
  */
 public class CollectionsAPI {
 
-  private final CollectionsHandler collectionsHandler;
+    public static final String V2_CREATE_COLLECTION_CMD = "create";
+    public static final String V2_BACKUP_CMD = "backup-collection";
+    public static final String V2_RESTORE_CMD = "restore-collection";
+    public static final String V2_CREATE_ALIAS_CMD = "create-alias";
+    public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
+    public static final String V2_DELETE_ALIAS_CMD = "delete-alias";

Review comment:
       They should definitely exist somewhere, but they're not `/v2/collections` APIs, so I didn't do that in this PR.  If you guys really want them, I'm not against adding those constants in this PR.
   
   But even then they should probably live somewhere else, shouldn't they? (since this class is for `/v2/collections` APIs, and those commands are for other APIs?  AFAIK this file is only for those APIs that specifically live under `/v2/collections`, right?)

##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateBody.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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 CreateBody implements ReflectMapWriter {
+    @JsonProperty(required = true)
+    public String name;
+
+    @JsonProperty
+    public String config;
+
+    @JsonProperty
+    public RouterInfo router;
+
+    @JsonProperty
+    public Integer numShards;
+
+    @JsonProperty
+    public String shards;
+
+    @JsonProperty
+    public Integer replicationFactor;
+
+    @JsonProperty
+    public Integer nrtReplicas;
+
+    @JsonProperty
+    public Integer tlogReplicas;
+
+    @JsonProperty
+    public Integer pullReplicas;
+
+    @JsonProperty
+    public List<String> nodeSet;
+
+    @JsonProperty
+    public Boolean shuffleNodes;
+
+    @JsonProperty
+    public Map<String, Object> properties;
+
+    @JsonProperty
+    public String async;
+
+    @JsonProperty
+    public Boolean waitForFinalState;
+
+    @JsonProperty
+    public Boolean perReplicaState;
+
+    public static class RouterInfo implements ReflectMapWriter {

Review comment:
       Good catch.  There's a Jackson annotation made for this case (`JsonAnyGetter`) - named properties will be mapped as usual but "extra" ones are put in a Map or whatever.  But it looks like we're not actually using Jackson here to avoid SolrJ needing to depend on Jackson.
   
   I could create clone JsonAnyGetter the way we've done with JsonProperty, but idt it's worth it.  I'll just remove this RouterInfo class and handle all the properties as a Map for the translation.
   
   Might be worth considering whether a Jackson-dep is worth it after all going forward though.

##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateBody.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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 CreateBody implements ReflectMapWriter {
+    @JsonProperty(required = true)
+    public String name;
+
+    @JsonProperty
+    public String config;
+
+    @JsonProperty
+    public RouterInfo router;
+
+    @JsonProperty
+    public Integer numShards;
+
+    @JsonProperty
+    public String shards;
+
+    @JsonProperty
+    public Integer replicationFactor;
+
+    @JsonProperty
+    public Integer nrtReplicas;
+
+    @JsonProperty
+    public Integer tlogReplicas;
+
+    @JsonProperty
+    public Integer pullReplicas;
+
+    @JsonProperty
+    public List<String> nodeSet;
+
+    @JsonProperty
+    public Boolean shuffleNodes;
+
+    @JsonProperty
+    public Map<String, Object> properties;
+
+    @JsonProperty
+    public String async;
+
+    @JsonProperty
+    public Boolean waitForFinalState;
+
+    @JsonProperty
+    public Boolean perReplicaState;
+
+    public static class RouterInfo implements ReflectMapWriter {

Review comment:
       Good catch.  There's a Jackson annotation made for this case (`JsonAnyGetter`) - named properties will be mapped as usual but "extra" ones are put in the catchall Map.  But it looks like we're not actually using Jackson here to avoid SolrJ needing to depend on it.  The JsonProperty annotation is a copy  in our own code to avoid the dep.
   
   I could create clone JsonAnyGetter the way we've done with JsonProperty, but idt it's worth it.  I'll just remove this RouterInfo class and handle all the properties as a Map for the translation.
   
   Might be worth considering whether a Jackson-dep is worth it after all going forward though.




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

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



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


[GitHub] [lucene-solr] gerlowskija commented on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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


   Hey, thanks for offering @noblepaul  .  I do have one thing I was puzzling over:
   
   How is SolrJ's `getV2Request`/`V2RequestSupport` feature intended to work for annotation-based v2 APIs?  It looks like `V1ToV2ApiMapper` is really dependent on `CollectionApiMapping.Meta`, which the annotation-based APIs can't be found in obviously.
   
   There are some annotation-based v2 APIs whose SolrJ request classes implement V2RequestSupport (e.g. CollectionAdminRequest.ClusterProp), so there should be logic somewhere to handle that I imagine?  Unless getV2Request will blow up now if called on certain CollectionAdminRequest objects?
   
   More broadly I wonder whether there's any value in getV2Request going forward.  It never really "took off" to cover APIs other than CollectionAdmin, and there might be better ways to let SolrJ users dip their toes into the v2 waters (e.g. dedicated request classes that better align with the v2 paths/concepts? idk). 


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

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



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


[GitHub] [lucene-solr] gerlowskija merged pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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


   


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

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



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


[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateBody.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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 CreateBody implements ReflectMapWriter {
+    @JsonProperty(required = true)
+    public String name;
+
+    @JsonProperty
+    public String config;
+
+    @JsonProperty
+    public RouterInfo router;
+
+    @JsonProperty
+    public Integer numShards;
+
+    @JsonProperty
+    public String shards;
+
+    @JsonProperty
+    public Integer replicationFactor;
+
+    @JsonProperty
+    public Integer nrtReplicas;
+
+    @JsonProperty
+    public Integer tlogReplicas;
+
+    @JsonProperty
+    public Integer pullReplicas;
+
+    @JsonProperty
+    public List<String> nodeSet;
+
+    @JsonProperty
+    public Boolean shuffleNodes;
+
+    @JsonProperty
+    public Map<String, Object> properties;
+
+    @JsonProperty
+    public String async;
+
+    @JsonProperty
+    public Boolean waitForFinalState;
+
+    @JsonProperty
+    public Boolean perReplicaState;
+
+    public static class RouterInfo implements ReflectMapWriter {

Review comment:
       Good catch.  There's a Jackson annotation made for this case (`JsonAnyGetter`) - named properties will be mapped as usual but "extra" ones are put in the catchall Map.  But it looks like we're not actually using Jackson here to avoid SolrJ needing to depend on it.  The JsonProperty annotation is a copy  in our own code to avoid the dep.
   
   I could create clone JsonAnyGetter the way we've done with JsonProperty, but idt it's worth it.  I'll just remove this RouterInfo class and handle all the properties as a Map for the translation.
   
   Might be worth considering whether a Jackson-dep is worth it after all going forward though.




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

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



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


[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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



##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateBody.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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 CreateBody implements ReflectMapWriter {
+    @JsonProperty(required = true)
+    public String name;
+
+    @JsonProperty
+    public String config;
+
+    @JsonProperty
+    public RouterInfo router;
+
+    @JsonProperty
+    public Integer numShards;
+
+    @JsonProperty
+    public String shards;
+
+    @JsonProperty
+    public Integer replicationFactor;
+
+    @JsonProperty
+    public Integer nrtReplicas;
+
+    @JsonProperty
+    public Integer tlogReplicas;
+
+    @JsonProperty
+    public Integer pullReplicas;
+
+    @JsonProperty
+    public List<String> nodeSet;
+
+    @JsonProperty
+    public Boolean shuffleNodes;
+
+    @JsonProperty
+    public Map<String, Object> properties;
+
+    @JsonProperty
+    public String async;
+
+    @JsonProperty
+    public Boolean waitForFinalState;
+
+    @JsonProperty
+    public Boolean perReplicaState;
+
+    public static class RouterInfo implements ReflectMapWriter {

Review comment:
       Good catch.  There's a Jackson annotation made for this case (`JsonAnyGetter`) - named properties will be mapped as usual but "extra" ones are put in a Map or whatever.  But it looks like we're not actually using Jackson here to avoid SolrJ needing to depend on Jackson.
   
   I could create clone JsonAnyGetter the way we've done with JsonProperty, but idt it's worth it.  I'll just remove this RouterInfo class and handle all the properties as a Map for the translation.
   
   Might be worth considering whether a Jackson-dep is worth it after all going forward though.




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

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



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


[GitHub] [lucene-solr] noblepaul commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570150592



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -36,7 +58,16 @@
  */
 public class CollectionsAPI {
 
-  private final CollectionsHandler collectionsHandler;
+    public static final String V2_CREATE_COLLECTION_CMD = "create";
+    public static final String V2_BACKUP_CMD = "backup-collection";
+    public static final String V2_RESTORE_CMD = "restore-collection";
+    public static final String V2_CREATE_ALIAS_CMD = "create-alias";
+    public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
+    public static final String V2_DELETE_ALIAS_CMD = "delete-alias";

Review comment:
       Yes, please




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

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



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


[GitHub] [lucene-solr] sigram commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570133109



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -50,11 +81,144 @@ public void getCollections(SolrQueryRequest req, SolrQueryResponse rsp) throws E
     CollectionsHandler.CollectionOperation.LIST_OP.execute(req, rsp, collectionsHandler);
   }
 
+    @EndPoint(
+            path = {"/c", "/collections"},
+            method = POST,
+            permission = COLL_EDIT_PERM)
+    public class CollectionsCommands {
+
+        @Command(name = V2_BACKUP_CMD)
+        @SuppressWarnings("unchecked")
+        public void backupCollection(PayloadObj<BackupCollectionBody> obj) throws Exception {
+            final Map<String, Object> v1Params = obj.get().toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.BACKUP.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_RESTORE_CMD)
+        @SuppressWarnings("unchecked")
+        public void restoreBackup(PayloadObj<RestoreCollectionBody> obj) throws Exception {
+            final RestoreCollectionBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.RESTORE.toLower());
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollParams = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollParams);
+                v1Params.putAll(createCollParams);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void createAlias(PayloadObj<CreateAliasBody> obj) throws Exception {
+            final CreateAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATEALIAS.toLower());
+            if (! CollectionUtils.isEmpty(v2Body.collections)) {
+                final String collectionsStr = String.join(",", v2Body.collections);
+                v1Params.remove(V2ApiConstants.COLLECTIONS);
+                v1Params.put(V2ApiConstants.COLLECTIONS, collectionsStr);
+            }
+            if (v2Body.router != null) {
+                Map<String, Object> routerProperties = (Map<String, Object>) v1Params.remove(V2ApiConstants.ROUTER_KEY);
+                flattenMapWithPrefix(routerProperties, v1Params, ROUTER_PREFIX);
+            }
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollectionMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollectionMap);
+                flattenMapWithPrefix(createCollectionMap, v1Params, CREATE_COLLECTION_PREFIX);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_SET_ALIAS_PROP_CMD)
+        @SuppressWarnings("unchecked")
+        public void setAliasProperty(PayloadObj<SetAliasPropertyBody> obj) throws Exception {
+            final SetAliasPropertyBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.ALIASPROP.toLower());
+            // Flatten "properties" map into individual prefixed params
+            final Map<String, Object> propertiesMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.PROPERTIES_KEY);
+            flattenMapWithPrefix(propertiesMap, v1Params, PROPERTY_PREFIX);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_DELETE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void deleteAlias(PayloadObj<DeleteAliasBody> obj) throws Exception {
+            final DeleteAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.DELETEALIAS.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_COLLECTION_CMD)
+        @SuppressWarnings("unchecked")
+        public void create(PayloadObj<CreateBody> obj) throws Exception {
+            final CreateBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATE.toLower());
+            convertV2CreateCollectionMapToV1ParamMap(v1Params);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @SuppressWarnings("unchecked")
+        private void convertV2CreateCollectionMapToV1ParamMap(Map<String, Object> v2MapVals) {
+            // Keys are copied so that map can be modified as keys are looped through.
+            final Set<String> v2Keys = v2MapVals.keySet().stream().collect(Collectors.toSet());
+            for (String key : v2Keys) {
+                switch (key) {
+                    case V2ApiConstants.PROPERTIES_KEY:
+                        final Map<String, Object> propertiesMap = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.PROPERTIES_KEY);
+                        flattenMapWithPrefix(propertiesMap, v2MapVals, PROPERTY_PREFIX);
+                        break;
+                    case ROUTER_KEY:
+                        final Map<String, Object> routerProperties = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.ROUTER_KEY);
+                        flattenMapWithPrefix(routerProperties, v2MapVals, CollectionAdminParams.ROUTER_PREFIX);
+                        break;
+                    case V2ApiConstants.CONFIG:
+                        v2MapVals.put(CollectionAdminParams.COLL_CONF, v2MapVals.remove(V2ApiConstants.CONFIG));
+                        break;
+                    case V2ApiConstants.SHUFFLE_NODES:
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM, v2MapVals.remove(V2ApiConstants.SHUFFLE_NODES));
+                        break;
+                    case V2ApiConstants.NODE_SET:
+                        final List<String> nodeSetList = (List<String>) v2MapVals.remove(V2ApiConstants.NODE_SET);
+                        final String nodeSetStr = String.join(",", nodeSetList);
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetStr);
+                        break;
+                    default:
+                        break;
+                }
+            }
+        }
+
+        private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,
+                                          String additionalPrefix) {
+            if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
+                return;
+            }
+
+            toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));

Review comment:
       Maybe we should make this recursive to support potentially nested v2 payloads?




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

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



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


[GitHub] [lucene-solr] sigram commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570144961



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -36,7 +58,16 @@
  */
 public class CollectionsAPI {
 
-  private final CollectionsHandler collectionsHandler;
+    public static final String V2_CREATE_COLLECTION_CMD = "create";
+    public static final String V2_BACKUP_CMD = "backup-collection";
+    public static final String V2_RESTORE_CMD = "restore-collection";
+    public static final String V2_CREATE_ALIAS_CMD = "create-alias";
+    public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
+    public static final String V2_DELETE_ALIAS_CMD = "delete-alias";

Review comment:
       Should we also add other collection commands here, such as: COLLECTIONPROP, CREATE / DELETESHARD, ADD / MOVE / DELETEREPLICA, MODIFYCOLLECTION, REINDEXCOLLECTION, RENAME, SPLITSHARD ?




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

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



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


[GitHub] [lucene-solr] gerlowskija commented on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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


   Going to merge this with the strategy for dealing with V2RequestSupport that I mentioned in my last comment.
   
   If you eventually get a chance to reply to that comment above @noblepaul , lmk if you prefer some other approach and I can pick it back up then.


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

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



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


[GitHub] [lucene-solr] noblepaul commented on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#issuecomment-773232908


   Thanks for picking this up @gerlowskija 
   
   Please let me know if you need any help


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

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



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


[GitHub] [lucene-solr] noblepaul commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570150592



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -36,7 +58,16 @@
  */
 public class CollectionsAPI {
 
-  private final CollectionsHandler collectionsHandler;
+    public static final String V2_CREATE_COLLECTION_CMD = "create";
+    public static final String V2_BACKUP_CMD = "backup-collection";
+    public static final String V2_RESTORE_CMD = "restore-collection";
+    public static final String V2_CREATE_ALIAS_CMD = "create-alias";
+    public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
+    public static final String V2_DELETE_ALIAS_CMD = "delete-alias";

Review comment:
       Yes, please




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

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



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


[GitHub] [lucene-solr] sigram commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#discussion_r570133109



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -50,11 +81,144 @@ public void getCollections(SolrQueryRequest req, SolrQueryResponse rsp) throws E
     CollectionsHandler.CollectionOperation.LIST_OP.execute(req, rsp, collectionsHandler);
   }
 
+    @EndPoint(
+            path = {"/c", "/collections"},
+            method = POST,
+            permission = COLL_EDIT_PERM)
+    public class CollectionsCommands {
+
+        @Command(name = V2_BACKUP_CMD)
+        @SuppressWarnings("unchecked")
+        public void backupCollection(PayloadObj<BackupCollectionBody> obj) throws Exception {
+            final Map<String, Object> v1Params = obj.get().toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.BACKUP.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_RESTORE_CMD)
+        @SuppressWarnings("unchecked")
+        public void restoreBackup(PayloadObj<RestoreCollectionBody> obj) throws Exception {
+            final RestoreCollectionBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.RESTORE.toLower());
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollParams = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollParams);
+                v1Params.putAll(createCollParams);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void createAlias(PayloadObj<CreateAliasBody> obj) throws Exception {
+            final CreateAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATEALIAS.toLower());
+            if (! CollectionUtils.isEmpty(v2Body.collections)) {
+                final String collectionsStr = String.join(",", v2Body.collections);
+                v1Params.remove(V2ApiConstants.COLLECTIONS);
+                v1Params.put(V2ApiConstants.COLLECTIONS, collectionsStr);
+            }
+            if (v2Body.router != null) {
+                Map<String, Object> routerProperties = (Map<String, Object>) v1Params.remove(V2ApiConstants.ROUTER_KEY);
+                flattenMapWithPrefix(routerProperties, v1Params, ROUTER_PREFIX);
+            }
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollectionMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollectionMap);
+                flattenMapWithPrefix(createCollectionMap, v1Params, CREATE_COLLECTION_PREFIX);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_SET_ALIAS_PROP_CMD)
+        @SuppressWarnings("unchecked")
+        public void setAliasProperty(PayloadObj<SetAliasPropertyBody> obj) throws Exception {
+            final SetAliasPropertyBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.ALIASPROP.toLower());
+            // Flatten "properties" map into individual prefixed params
+            final Map<String, Object> propertiesMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.PROPERTIES_KEY);
+            flattenMapWithPrefix(propertiesMap, v1Params, PROPERTY_PREFIX);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_DELETE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void deleteAlias(PayloadObj<DeleteAliasBody> obj) throws Exception {
+            final DeleteAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.DELETEALIAS.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_COLLECTION_CMD)
+        @SuppressWarnings("unchecked")
+        public void create(PayloadObj<CreateBody> obj) throws Exception {
+            final CreateBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATE.toLower());
+            convertV2CreateCollectionMapToV1ParamMap(v1Params);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @SuppressWarnings("unchecked")
+        private void convertV2CreateCollectionMapToV1ParamMap(Map<String, Object> v2MapVals) {
+            // Keys are copied so that map can be modified as keys are looped through.
+            final Set<String> v2Keys = v2MapVals.keySet().stream().collect(Collectors.toSet());
+            for (String key : v2Keys) {
+                switch (key) {
+                    case V2ApiConstants.PROPERTIES_KEY:
+                        final Map<String, Object> propertiesMap = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.PROPERTIES_KEY);
+                        flattenMapWithPrefix(propertiesMap, v2MapVals, PROPERTY_PREFIX);
+                        break;
+                    case ROUTER_KEY:
+                        final Map<String, Object> routerProperties = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.ROUTER_KEY);
+                        flattenMapWithPrefix(routerProperties, v2MapVals, CollectionAdminParams.ROUTER_PREFIX);
+                        break;
+                    case V2ApiConstants.CONFIG:
+                        v2MapVals.put(CollectionAdminParams.COLL_CONF, v2MapVals.remove(V2ApiConstants.CONFIG));
+                        break;
+                    case V2ApiConstants.SHUFFLE_NODES:
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM, v2MapVals.remove(V2ApiConstants.SHUFFLE_NODES));
+                        break;
+                    case V2ApiConstants.NODE_SET:
+                        final List<String> nodeSetList = (List<String>) v2MapVals.remove(V2ApiConstants.NODE_SET);
+                        final String nodeSetStr = String.join(",", nodeSetList);
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetStr);
+                        break;
+                    default:
+                        break;
+                }
+            }
+        }
+
+        private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,
+                                          String additionalPrefix) {
+            if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
+                return;
+            }
+
+            toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));

Review comment:
       Maybe we should make this recursive to support potentially nested v2 payloads?

##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/BackupCollectionBody.java
##########
@@ -0,0 +1,51 @@
+/*
+ * 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;
+
+/**
+ * V2 API POJO for the /v2/collections 'backup-collection' command.
+ *
+ * Analogous to the request parameters for v1 /admin/collections?action=BACKUP API.
+ */
+public class BackupCollectionBody implements ReflectMapWriter {

Review comment:
       What do you think about renaming all of these to use `*Payload` instead of `*Body`?

##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/CreateBody.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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 CreateBody implements ReflectMapWriter {
+    @JsonProperty(required = true)
+    public String name;
+
+    @JsonProperty
+    public String config;
+
+    @JsonProperty
+    public RouterInfo router;
+
+    @JsonProperty
+    public Integer numShards;
+
+    @JsonProperty
+    public String shards;
+
+    @JsonProperty
+    public Integer replicationFactor;
+
+    @JsonProperty
+    public Integer nrtReplicas;
+
+    @JsonProperty
+    public Integer tlogReplicas;
+
+    @JsonProperty
+    public Integer pullReplicas;
+
+    @JsonProperty
+    public List<String> nodeSet;
+
+    @JsonProperty
+    public Boolean shuffleNodes;
+
+    @JsonProperty
+    public Map<String, Object> properties;
+
+    @JsonProperty
+    public String async;
+
+    @JsonProperty
+    public Boolean waitForFinalState;
+
+    @JsonProperty
+    public Boolean perReplicaState;
+
+    public static class RouterInfo implements ReflectMapWriter {

Review comment:
       Router spec may in fact contain arbitrary properties in addition to these two - see `DocRouter.getRouterSpec`. Currently only `name` and `field` are used in the routers that Solr provides but other users may use some other properties in their implementations.

##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -36,7 +58,16 @@
  */
 public class CollectionsAPI {
 
-  private final CollectionsHandler collectionsHandler;
+    public static final String V2_CREATE_COLLECTION_CMD = "create";
+    public static final String V2_BACKUP_CMD = "backup-collection";
+    public static final String V2_RESTORE_CMD = "restore-collection";
+    public static final String V2_CREATE_ALIAS_CMD = "create-alias";
+    public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
+    public static final String V2_DELETE_ALIAS_CMD = "delete-alias";

Review comment:
       Should we also add other collection commands here, such as: COLLECTIONPROP, CREATE / DELETESHARD, ADD / MOVE / DELETEREPLICA, MODIFYCOLLECTION, REINDEXCOLLECTION, RENAME, SPLITSHARD ?




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

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



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


[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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



##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -50,11 +81,144 @@ public void getCollections(SolrQueryRequest req, SolrQueryResponse rsp) throws E
     CollectionsHandler.CollectionOperation.LIST_OP.execute(req, rsp, collectionsHandler);
   }
 
+    @EndPoint(
+            path = {"/c", "/collections"},
+            method = POST,
+            permission = COLL_EDIT_PERM)
+    public class CollectionsCommands {
+
+        @Command(name = V2_BACKUP_CMD)
+        @SuppressWarnings("unchecked")
+        public void backupCollection(PayloadObj<BackupCollectionBody> obj) throws Exception {
+            final Map<String, Object> v1Params = obj.get().toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.BACKUP.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_RESTORE_CMD)
+        @SuppressWarnings("unchecked")
+        public void restoreBackup(PayloadObj<RestoreCollectionBody> obj) throws Exception {
+            final RestoreCollectionBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.RESTORE.toLower());
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollParams = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollParams);
+                v1Params.putAll(createCollParams);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void createAlias(PayloadObj<CreateAliasBody> obj) throws Exception {
+            final CreateAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATEALIAS.toLower());
+            if (! CollectionUtils.isEmpty(v2Body.collections)) {
+                final String collectionsStr = String.join(",", v2Body.collections);
+                v1Params.remove(V2ApiConstants.COLLECTIONS);
+                v1Params.put(V2ApiConstants.COLLECTIONS, collectionsStr);
+            }
+            if (v2Body.router != null) {
+                Map<String, Object> routerProperties = (Map<String, Object>) v1Params.remove(V2ApiConstants.ROUTER_KEY);
+                flattenMapWithPrefix(routerProperties, v1Params, ROUTER_PREFIX);
+            }
+            if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
+                final Map<String, Object> createCollectionMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
+                convertV2CreateCollectionMapToV1ParamMap(createCollectionMap);
+                flattenMapWithPrefix(createCollectionMap, v1Params, CREATE_COLLECTION_PREFIX);
+            }
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_SET_ALIAS_PROP_CMD)
+        @SuppressWarnings("unchecked")
+        public void setAliasProperty(PayloadObj<SetAliasPropertyBody> obj) throws Exception {
+            final SetAliasPropertyBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.ALIASPROP.toLower());
+            // Flatten "properties" map into individual prefixed params
+            final Map<String, Object> propertiesMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.PROPERTIES_KEY);
+            flattenMapWithPrefix(propertiesMap, v1Params, PROPERTY_PREFIX);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name= V2_DELETE_ALIAS_CMD)
+        @SuppressWarnings("unchecked")
+        public void deleteAlias(PayloadObj<DeleteAliasBody> obj) throws Exception {
+            final DeleteAliasBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+            v1Params.put(ACTION, CollectionAction.DELETEALIAS.toLower());
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @Command(name = V2_CREATE_COLLECTION_CMD)
+        @SuppressWarnings("unchecked")
+        public void create(PayloadObj<CreateBody> obj) throws Exception {
+            final CreateBody v2Body = obj.get();
+            final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
+
+            v1Params.put(ACTION, CollectionAction.CREATE.toLower());
+            convertV2CreateCollectionMapToV1ParamMap(v1Params);
+
+            collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+        }
+
+        @SuppressWarnings("unchecked")
+        private void convertV2CreateCollectionMapToV1ParamMap(Map<String, Object> v2MapVals) {
+            // Keys are copied so that map can be modified as keys are looped through.
+            final Set<String> v2Keys = v2MapVals.keySet().stream().collect(Collectors.toSet());
+            for (String key : v2Keys) {
+                switch (key) {
+                    case V2ApiConstants.PROPERTIES_KEY:
+                        final Map<String, Object> propertiesMap = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.PROPERTIES_KEY);
+                        flattenMapWithPrefix(propertiesMap, v2MapVals, PROPERTY_PREFIX);
+                        break;
+                    case ROUTER_KEY:
+                        final Map<String, Object> routerProperties = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.ROUTER_KEY);
+                        flattenMapWithPrefix(routerProperties, v2MapVals, CollectionAdminParams.ROUTER_PREFIX);
+                        break;
+                    case V2ApiConstants.CONFIG:
+                        v2MapVals.put(CollectionAdminParams.COLL_CONF, v2MapVals.remove(V2ApiConstants.CONFIG));
+                        break;
+                    case V2ApiConstants.SHUFFLE_NODES:
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM, v2MapVals.remove(V2ApiConstants.SHUFFLE_NODES));
+                        break;
+                    case V2ApiConstants.NODE_SET:
+                        final List<String> nodeSetList = (List<String>) v2MapVals.remove(V2ApiConstants.NODE_SET);
+                        final String nodeSetStr = String.join(",", nodeSetList);
+                        v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetStr);
+                        break;
+                    default:
+                        break;
+                }
+            }
+        }
+
+        private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,
+                                          String additionalPrefix) {
+            if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
+                return;
+            }
+
+            toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));

Review comment:
       I actually tried this initially but found the result to be a lot like the code in `CollectionsApiMapping.Meta`, which I personally find tough to read with all the map-literals for renaming properties and prefixing properties.  I guessed it was more readable to just call this non-recursive version twice.  That wouldn't work as well on APIs with deeper nesting, but seemed feasible here.
   
   I'm open to going back that direction though if you prefer it.  Did you have a particular signature in mind?

##########
File path: solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
##########
@@ -36,7 +58,16 @@
  */
 public class CollectionsAPI {
 
-  private final CollectionsHandler collectionsHandler;
+    public static final String V2_CREATE_COLLECTION_CMD = "create";
+    public static final String V2_BACKUP_CMD = "backup-collection";
+    public static final String V2_RESTORE_CMD = "restore-collection";
+    public static final String V2_CREATE_ALIAS_CMD = "create-alias";
+    public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
+    public static final String V2_DELETE_ALIAS_CMD = "delete-alias";

Review comment:
       They should definitely exist somewhere, but they're not `/v2/collections` APIs, so I didn't do that in this PR.  If you guys really want them, I'm not against adding those constants in this PR.
   
   But even then they should probably live somewhere else, shouldn't they? (since this class is for `/v2/collections` APIs, and those commands are for other APIs?  AFAIK this file is only for those APIs that specifically live under `/v2/collections`, right?)




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

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



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


[GitHub] [lucene-solr] gerlowskija commented on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

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


   Hey, thanks for offering @noblepaul  .  I do have one thing I was puzzling over:
   
   How is SolrJ's `getV2Request`/`V2RequestSupport` feature intended to work for annotation-based v2 APIs?  It looks like `V1ToV2ApiMapper` is really dependent on `CollectionApiMapping.Meta`, which the annotation-based APIs can't be found in obviously.
   
   There are some annotation-based v2 APIs whose SolrJ request classes implement V2RequestSupport (e.g. CollectionAdminRequest.ClusterProp), so there should be logic somewhere to handle that I imagine?  Unless getV2Request will blow up now if called on certain CollectionAdminRequest objects?
   
   More broadly I wonder whether there's any value in getV2Request going forward.  It never really "took off" to cover APIs other than CollectionAdmin, and there might be better ways to let SolrJ users dip their toes into the v2 waters (e.g. dedicated request classes that better align with the v2 paths/concepts? idk). 


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

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



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


[GitHub] [lucene-solr] gerlowskija edited a comment on pull request #2281: SOLR-15118: Convert /v2/collections APIs to annotations

Posted by GitBox <gi...@apache.org>.
gerlowskija edited a comment on pull request #2281:
URL: https://github.com/apache/lucene-solr/pull/2281#issuecomment-773515236


   Hey, thanks for offering @noblepaul  .  I do have one thing I was puzzling over:
   
   How is SolrJ's `getV2Request`/`V2RequestSupport` feature intended to work for annotation-based v2 APIs?  It looks like `V1ToV2ApiMapper` is really dependent on `CollectionApiMapping.Meta`, which the annotation-based APIs can't be found in obviously.
   
   There are some annotation-based v2 APIs whose SolrJ request classes implement V2RequestSupport (e.g. CollectionAdminRequest.ClusterProp), so there should be logic somewhere to handle that I imagine?  Unless getV2Request will blow up now if called on certain CollectionAdminRequest objects?
   
   More broadly I wonder whether there's any value in getV2Request going forward.  It never really "took off" to cover APIs other than CollectionAdmin, and there might be better ways to let SolrJ users dip their toes into the v2 waters (e.g. dedicated request classes that better align with the v2 paths/concepts? idk).
   
   ----
   
   If we don't have an approach for V2RequestSupport + annotation-based APIs worked out, and there's not strong feeling about carrying V2RequestSupport forward, I'd lean towards making this PR a master-only change, and deprecating getV2Request in 8.9 so it can be removed by this PR on master.   


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

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



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