From 09816fdf579f71c8eb9cfe3bd7302c89c47d68e6 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 28 Aug 2014 12:27:32 +0200 Subject: [PATCH] Validate create index requests' number of primary/replica shards Fixes #7495 --- .../indices/create/CreateIndexRequest.java | 8 +++ .../cluster/metadata/IndexMetaData.java | 4 +- .../admin/indices/CreateIndexTests.java | 56 --------------- .../indices/create/CreateIndexTests.java | 72 ++++++++++++++++++- 4 files changed, 81 insertions(+), 59 deletions(-) delete mode 100644 src/test/java/org/elasticsearch/action/admin/indices/CreateIndexTests.java diff --git a/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java b/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java index f3ddb6c1d9a..8c32eec67f4 100644 --- a/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java +++ b/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java @@ -107,6 +107,14 @@ public class CreateIndexRequest extends AcknowledgedRequest 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; } diff --git a/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index bc3632b0f28..b6d5d661392 100644 --- a/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/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 mappings, ImmutableOpenMap aliases, ImmutableOpenMap customs) { - Preconditions.checkArgument(settings.getAsInt(SETTING_NUMBER_OF_SHARDS, -1) != -1, "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_SHARDS, null) != null, "must specify numberOfShards 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; diff --git a/src/test/java/org/elasticsearch/action/admin/indices/CreateIndexTests.java b/src/test/java/org/elasticsearch/action/admin/indices/CreateIndexTests.java deleted file mode 100644 index ac6408dd9d3..00000000000 --- a/src/test/java/org/elasticsearch/action/admin/indices/CreateIndexTests.java +++ /dev/null @@ -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()) - .addMapping("type1", new HashMap()); - 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"); - } - } -} diff --git a/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexTests.java b/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexTests.java index 11bce27d8f5..638c9a61fb4 100644 --- a/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexTests.java +++ b/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()) + .addMapping("type1", new HashMap()); + 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)); + } + } }