From 62fd532bae53521cdf3556397c114f1c660e37af Mon Sep 17 00:00:00 2001
From: Mark Robert Miller <markrmiller@apache.org>
Date: Fri, 15 Mar 2013 15:03:02 +0000
Subject: [PATCH] SOLR-4585: The Collections API validates numShards with < 0
 but should use  <= 0.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1456979 13f79535-47bb-0310-9956-ffa450edef68
---
 solr/CHANGES.txt                              |  5 ++-
 .../cloud/OverseerCollectionProcessor.java    | 12 ++++---
 .../CollectionsAPIDistributedZkTest.java      | 35 +++++++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3078be2e23e..4cf470ce720 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -98,7 +98,7 @@ Bug Fixes
   fields to null (Ben Pennell, Rob, shalin)
 
 * SOLR-4543: setting shardHandlerFactory in solr.xml/solr.properties does not work.
-  (Ryan Ernst, Robert Muir via Erick Ericson)
+  (Ryan Ernst, Robert Muir via Erick Erickson)
 
 * SOLR-4371: Admin UI - Analysis Screen shows empty result (steffkes)
 
@@ -123,6 +123,9 @@ Bug Fixes
   should still wait to see the shard id in it's current ClusterState.
   (Mark Miller)
 
+* SOLR-4585: The Collections API validates numShards with < 0 but should use 
+  <= 0. (Mark Miller)
+
 Optimizations
 ----------------------
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java
index 3bd1f0d91e0..fc9ddc4c017 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java
+++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java
@@ -316,17 +316,21 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread {
       // if it does not, find best nodes to create more cores
       
       int repFactor = msgStrToInt(message, REPLICATION_FACTOR, 1);
-      int numSlices = msgStrToInt(message, NUM_SLICES, 0);
+      Integer numSlices = msgStrToInt(message, NUM_SLICES, null);
+      
+      if (numSlices == null) {
+        throw new SolrException(ErrorCode.BAD_REQUEST, "collection already exists: " + collectionName);
+      }
+      
       int maxShardsPerNode = msgStrToInt(message, MAX_SHARDS_PER_NODE, 1);
       String createNodeSetStr; 
       List<String> createNodeList = ((createNodeSetStr = message.getStr(CREATE_NODE_SET)) == null)?null:StrUtils.splitSmart(createNodeSetStr, ",", true);
       
       if (repFactor <= 0) {
-        SolrException.log(log, REPLICATION_FACTOR + " must be > 0");
-        throw new SolrException(ErrorCode.BAD_REQUEST, "collection already exists: " + collectionName);
+        throw new SolrException(ErrorCode.BAD_REQUEST, NUM_SLICES + " is a required paramater");
       }
       
-      if (numSlices < 0) {
+      if (numSlices <= 0) {
         throw new SolrException(ErrorCode.BAD_REQUEST, NUM_SLICES + " must be > 0");
       }
       
diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java
index 7f768e0f878..3f8f41a6b30 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java
@@ -207,6 +207,41 @@ public class CollectionsAPIDistributedZkTest extends AbstractFullDistribZkTestBa
     }
     assertTrue(gotExp);
     
+    // No numShards should fail
+    params = new ModifiableSolrParams();
+    params.set("action", CollectionAction.CREATE.toString());
+    collectionName = "acollection";
+    params.set("name", collectionName);
+    params.set(OverseerCollectionProcessor.REPLICATION_FACTOR, 10);
+    request = new QueryRequest(params);
+    request.setPath("/admin/collections");
+    gotExp = false;
+    resp = null;
+    try {
+      resp = createNewSolrServer("", baseUrl).request(request);
+    } catch (SolrException e) {
+      gotExp = true;
+    }
+    assertTrue(gotExp);
+    
+    // 0 numShards should fail
+    params = new ModifiableSolrParams();
+    params.set("action", CollectionAction.CREATE.toString());
+    collectionName = "acollection";
+    params.set("name", collectionName);
+    params.set(OverseerCollectionProcessor.REPLICATION_FACTOR, 10);
+    params.set("numShards", 0);
+    request = new QueryRequest(params);
+    request.setPath("/admin/collections");
+    gotExp = false;
+    resp = null;
+    try {
+      resp = createNewSolrServer("", baseUrl).request(request);
+    } catch (SolrException e) {
+      gotExp = true;
+    }
+    assertTrue(gotExp);
+    
     // Fail on one node
     
     // first we make a core with the core name the collections api