Validate create index requests' number of primary/replica shards

Fixes #7495

src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java

@@ -107,6 +107,14 @@ public class CreateIndexRequest extends AcknowledgedRequest<CreateIndexRequest>
         if (index == null) {
             validationException = addValidationError("index is missing", validationException);
         }
+        Integer number_of_primaries = settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_SHARDS, null);
+        Integer number_of_replicas = settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, null);
+        if (number_of_primaries != null && number_of_primaries <= 0) {
+            validationException = addValidationError("index must have 1 or more primary shards", validationException);
+        }
+        if (number_of_replicas != null && number_of_replicas < 0) {
+            validationException = addValidationError("index must have 0 or more replica shards", validationException);
+        }
         return validationException;
     }
 

src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

@@ -184,8 +184,8 @@ public class IndexMetaData {
     private final DiscoveryNodeFilters excludeFilters;
 
     private IndexMetaData(String index, long version, State state, Settings settings, ImmutableOpenMap<String, MappingMetaData> mappings, ImmutableOpenMap<String, AliasMetaData> aliases, ImmutableOpenMap<String, Custom> customs) {
-        Preconditions.checkArgument(settings.getAsInt(SETTING_NUMBER_OF_SHARDS, -1) != -1, "must specify numberOfShards for index [" + index + "]");
+        Preconditions.checkArgument(settings.getAsInt(SETTING_NUMBER_OF_SHARDS, null) != null, "must specify numberOfShards for index [" + index + "]");
-        Preconditions.checkArgument(settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, -1) != -1, "must specify numberOfReplicas for index [" + index + "]");
+        Preconditions.checkArgument(settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, null) != null, "must specify numberOfReplicas for index [" + index + "]");
         this.index = index;
         this.version = version;
         this.state = state;

src/test/java/org/elasticsearch/action/admin/indices/CreateIndexTests.java

@@ -1,56 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch 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.elasticsearch.action.admin.indices;
-
-import java.util.HashMap;
-
-import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
-import org.elasticsearch.test.ElasticsearchIntegrationTest;
-
-import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-
-public class CreateIndexTests extends ElasticsearchIntegrationTest {
-    public void testDoubleAddMapping() throws Exception {
-        try {
-            prepareCreate("test")
-                .addMapping("type1", "date", "type=date")
-                .addMapping("type1", "num", "type=integer");    
-            fail("did not hit expected exception");
-        } catch (IllegalStateException ise) {
-            // expected
-        }
-        try {
-            prepareCreate("test")
-                .addMapping("type1", new HashMap<String,Object>())
-                .addMapping("type1", new HashMap<String,Object>());
-            fail("did not hit expected exception");
-        } catch (IllegalStateException ise) {
-            // expected
-        }
-        try {
-            prepareCreate("test")
-                .addMapping("type1", jsonBuilder())
-                .addMapping("type1", jsonBuilder());
-            fail("did not hit expected exception");
-        } catch (IllegalStateException ise) {
-            // expected
-            System.out.println("HERE");
-        }
-    }
-}

src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.admin.indices.create;
 
+import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -30,6 +31,9 @@ import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
 import org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
 import org.junit.Test;
 
+import java.util.HashMap;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.*;
 import static org.hamcrest.core.IsNull.notNullValue;
 
@@ -51,7 +55,7 @@ public class CreateIndexTests extends ElasticsearchIntegrationTest{
         assertThat(index, notNullValue());
         assertThat(index.creationDate(), equalTo(4l));
     }
-    
+
     @Test
     public void testCreationDate_Generated() {
         long timeBeforeRequest = System.currentTimeMillis();
@@ -70,4 +74,70 @@ public class CreateIndexTests extends ElasticsearchIntegrationTest{
         assertThat(index.creationDate(), allOf(lessThanOrEqualTo(timeAfterRequest), greaterThanOrEqualTo(timeBeforeRequest)));
     }
 
+    @Test
+    public void testDoubleAddMapping() throws Exception {
+        try {
+            prepareCreate("test")
+                    .addMapping("type1", "date", "type=date")
+                    .addMapping("type1", "num", "type=integer");
+            fail("did not hit expected exception");
+        } catch (IllegalStateException ise) {
+            // expected
+        }
+        try {
+            prepareCreate("test")
+                    .addMapping("type1", new HashMap<String,Object>())
+                    .addMapping("type1", new HashMap<String,Object>());
+            fail("did not hit expected exception");
+        } catch (IllegalStateException ise) {
+            // expected
+        }
+        try {
+            prepareCreate("test")
+                    .addMapping("type1", jsonBuilder())
+                    .addMapping("type1", jsonBuilder());
+            fail("did not hit expected exception");
+        } catch (IllegalStateException ise) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testInvalidShardCountSettings() throws Exception {
+        try {
+            prepareCreate("test").setSettings(ImmutableSettings.builder()
+                    .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, randomIntBetween(-10, 0))
+                    .build())
+            .get();
+            fail("should have thrown an exception about the primary shard count");
+        } catch (ActionRequestValidationException e) {
+            assertThat("message contains error about shard count: " + e.getMessage(),
+                    e.getMessage().contains("index must have 1 or more primary shards"), equalTo(true));
+        }
+
+        try {
+            prepareCreate("test").setSettings(ImmutableSettings.builder()
+                    .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, randomIntBetween(-10, -1))
+                    .build())
+                    .get();
+            fail("should have thrown an exception about the replica shard count");
+        } catch (ActionRequestValidationException e) {
+            assertThat("message contains error about shard count: " + e.getMessage(),
+                    e.getMessage().contains("index must have 0 or more replica shards"), equalTo(true));
+        }
+
+        try {
+            prepareCreate("test").setSettings(ImmutableSettings.builder()
+                    .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, randomIntBetween(-10, 0))
+                    .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, randomIntBetween(-10, -1))
+                    .build())
+                    .get();
+            fail("should have thrown an exception about the shard count");
+        } catch (ActionRequestValidationException e) {
+            assertThat("message contains error about shard count: " + e.getMessage(),
+                    e.getMessage().contains("index must have 1 or more primary shards"), equalTo(true));
+            assertThat("message contains error about shard count: " + e.getMessage(),
+                    e.getMessage().contains("index must have 0 or more replica shards"), equalTo(true));
+        }
+    }
 }