[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1687: SOLR-14658: Fix colstatus solrj interfaces to return one or all collections

[GitBox <gi...@apache.org>]

dsmiley commented on a change in pull request #1687:
URL: https://github.com/apache/lucene-solr/pull/1687#discussion_r474312456



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
##########
@@ -517,10 +517,7 @@ private static void addStatusToResponse(NamedList<Object> results, RequestStatus
           ColStatus.RAW_SIZE_DETAILS_PROP,
           ColStatus.RAW_SIZE_SAMPLING_PERCENT_PROP,
           ColStatus.SIZE_INFO_PROP);
-      // make sure we can get the name if there's "name" but not "collection"
-      if (props.containsKey(CoreAdminParams.NAME) && !props.containsKey(COLLECTION_PROP)) {

Review comment:
       CC @sigram I believe you added the lines of code here that Andy thinks should be removed.

##########
File path: solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
##########
@@ -662,6 +662,44 @@ public void testColStatus() throws Exception {
     Number down = (Number) rsp.getResponse().findRecursive(collectionName, "shards", "shard1", "replicas", "down");
     assertTrue("should be some down replicas, but there were none in shard1:" + rsp, down.intValue() > 0);
   }
+  
+  @Test
+  public void testColStatusCollectionName() throws Exception {
+    final String[] collectionNames = {"collectionStatusTest_1", "collectionStatusTest_2"};
+    for (String collectionName : collectionNames) {
+      CollectionAdminRequest.createCollection(collectionName, "conf2", 1, 1)
+      .process(cluster.getSolrClient());
+      cluster.waitForActiveCollection(collectionName, 1, 1);
+    }
+    // assert only one collection is returned using the solrj colstatus interface
+    CollectionAdminRequest.ColStatus req = CollectionAdminRequest.collectionStatus(collectionNames[0]);
+    CollectionAdminResponse rsp = req.process(cluster.getSolrClient());
+    assertNotNull(rsp.getResponse().get(collectionNames[0]));
+    assertNull(rsp.getResponse().get(collectionNames[1]));
+   
+    req = CollectionAdminRequest.collectionStatus(collectionNames[1]);
+    rsp = req.process(cluster.getSolrClient());
+    assertNotNull(rsp.getResponse().get(collectionNames[1]));
+    assertNull(rsp.getResponse().get(collectionNames[0]));
+    
+    // assert passing null collection fails
+    try {

Review comment:
       please convert to expectThrows




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