Skip to content

Commit

Permalink
Added alias action validation
Browse files Browse the repository at this point in the history
Moved alias action validation to IndicesAliasesRequest, so that Java api and RestIndexPutAliasAction can benefit from it too.
Added check in MetaDataIndexAliasesService too.

Closes elastic#3363
  • Loading branch information
javanna committed Jul 22, 2013
1 parent 585756f commit 7beccf3
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 7 deletions.
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.master.MasterNodeOperationRequest;
import org.elasticsearch.cluster.metadata.AliasAction;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.unit.TimeValue;
Expand All @@ -35,6 +36,7 @@

import java.io.IOException;
import java.util.List;
import java.util.Locale;
import java.util.Map;

import static org.elasticsearch.action.ValidateActions.addValidationError;
Expand Down Expand Up @@ -170,7 +172,15 @@ public IndicesAliasesRequest timeout(String timeout) {
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (aliasActions.isEmpty()) {
validationException = addValidationError("Must specify at least one alias action", validationException);
return addValidationError("Must specify at least one alias action", validationException);
}
for (AliasAction aliasAction : aliasActions) {
if (!Strings.hasText(aliasAction.alias())) {
validationException = addValidationError("Alias action [" + aliasAction.actionType().name().toLowerCase(Locale.ENGLISH) + "] requires an [alias] to be set", validationException);
}
if (!Strings.hasText(aliasAction.index())) {
validationException = addValidationError("Alias action [" + aliasAction.actionType().name().toLowerCase(Locale.ENGLISH) + "] requires an [index] to be set", validationException);
}
}
return validationException;
}
Expand Down
Expand Up @@ -77,6 +77,10 @@ public ClusterState execute(final ClusterState currentState) {
Map<String, IndexService> indices = Maps.newHashMap();
try {
for (AliasAction aliasAction : request.actions) {
if (!Strings.hasText(aliasAction.alias()) || !Strings.hasText(aliasAction.index())) {
listener.onFailure(new ElasticSearchIllegalArgumentException("Index name and alias name are required"));
return currentState;
}
if (!currentState.metaData().hasIndex(aliasAction.index())) {
listener.onFailure(new IndexMissingException(new Index(aliasAction.index())));
return currentState;
Expand Down
Expand Up @@ -117,12 +117,7 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
}
}
}
if (index == null) {
throw new ElasticSearchIllegalArgumentException("Alias action [" + action + "] requires an [index] to be set");
}
if (alias == null) {
throw new ElasticSearchIllegalArgumentException("Alias action [" + action + "] requires an [alias] to be set");
}

if (type == AliasAction.Type.ADD) {
AliasAction aliasAction = newAddAliasAction(index, alias).filter(filter);
if (routingSet) {
Expand Down
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.test.integration.aliases;

import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthStatus;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse;
Expand Down Expand Up @@ -57,6 +58,7 @@
import static org.elasticsearch.index.query.FilterBuilders.termFilter;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;
import static org.testng.AssertJUnit.assertTrue;
import static org.testng.AssertJUnit.fail;

/**
Expand Down Expand Up @@ -795,6 +797,106 @@ public void testIndicesGetAliases() throws Exception {
assertThat(existsResponse.exists(), equalTo(false));
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void testAddAliasNullIndex() {

client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, "alias1"))
.execute().actionGet();
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void testAddAliasEmptyIndex() {

client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction("", "alias1"))
.execute().actionGet();
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void testAddAliasNullAlias() {

client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction("index1", null))
.execute().actionGet();
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void testAddAliasEmptyAlias() {

client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction("index1", ""))
.execute().actionGet();
}

@Test
public void testAddAliasNullAliasNullIndex() {
try {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, null))
.execute().actionGet();
assertTrue("Should throw " + ActionRequestValidationException.class.getSimpleName(), false);
} catch(ActionRequestValidationException e) {
assertThat(e.validationErrors(), notNullValue());
assertThat(e.validationErrors().size(), equalTo(2));
}
}

@Test
public void testAddAliasEmptyAliasEmptyIndex() {
try {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction("", ""))
.execute().actionGet();
assertTrue("Should throw " + ActionRequestValidationException.class.getSimpleName(), false);
} catch(ActionRequestValidationException e) {
assertThat(e.validationErrors(), notNullValue());
assertThat(e.validationErrors().size(), equalTo(2));
}
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void tesRemoveAliasNullIndex() {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newRemoveAliasAction(null, "alias1"))
.execute().actionGet();
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void tesRemoveAliasEmptyIndex() {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newRemoveAliasAction("", "alias1"))
.execute().actionGet();
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void tesRemoveAliasNullAlias() {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newRemoveAliasAction("index1", null))
.execute().actionGet();
}

@Test(expectedExceptions = ActionRequestValidationException.class)
public void tesRemoveAliasEmptyAlias() {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newRemoveAliasAction("index1", ""))
.execute().actionGet();
}

@Test
public void testRemoveAliasNullAliasNullIndex() {
try {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, null))
.execute().actionGet();
assertTrue("Should throw " + ActionRequestValidationException.class.getSimpleName(), false);
} catch(ActionRequestValidationException e) {
assertThat(e.validationErrors(), notNullValue());
assertThat(e.validationErrors().size(), equalTo(2));
}
}

@Test
public void testRemoveAliasEmptyAliasEmptyIndex() {
try {
client().admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction("", ""))
.execute().actionGet();
assertTrue("Should throw " + ActionRequestValidationException.class.getSimpleName(), false);
} catch(ActionRequestValidationException e) {
assertThat(e.validationErrors(), notNullValue());
assertThat(e.validationErrors().size(), equalTo(2));
}
}

private void assertHits(SearchHits hits, String... ids) {
assertThat(hits.totalHits(), equalTo((long) ids.length));
Set<String> hitIds = newHashSet();
Expand Down

0 comments on commit 7beccf3

Please sign in to comment.