From 8989d062cd9c90956a3c8a3c03445491f2c57453 Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 4 Aug 2014 12:33:02 +0200 Subject: [PATCH] MultiGet & MultiTermVector api: fail when using no routing and an alias to an index that has routing required (for that doc type) Made sure that the routing required check is performed against the concrete index, added use of aliases to existing routing tests. Taken the change to unify the failure message as well to this form: routing is required for [" + index + "]/[" + type + "]/[" + id + "] Closes #7145 --- .../action/get/TransportMultiGetAction.java | 12 +-- .../TransportMultiTermVectorsAction.java | 9 +- .../elasticsearch/mget/SimpleMgetTests.java | 2 +- .../routing/SimpleRoutingTests.java | 96 ++++++++++--------- 4 files changed, 61 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java b/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java index c619034b6e31c..04c7273cc63fd 100644 --- a/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java +++ b/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java @@ -23,7 +23,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.cluster.ClusterService; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockLevel; @@ -32,8 +31,6 @@ import org.elasticsearch.common.util.concurrent.AtomicArray; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.BaseTransportRequestHandler; -import org.elasticsearch.transport.TransportChannel; import org.elasticsearch.transport.TransportService; import java.util.HashMap; @@ -73,13 +70,14 @@ protected void doExecute(final MultiGetRequest request, final ActionListener indexing with id [1], and routing [0]"); - client().prepareIndex("test", "type1", "1").setRouting("0").setSource("field", "value1").setRefresh(true).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setRouting("0").setSource("field", "value1").setRefresh(true).execute().actionGet(); logger.info("--> verifying get with no routing, should fail"); logger.info("--> indexing with id [1], with no routing, should fail"); try { - client().prepareIndex("test", "type1", "1").setSource("field", "value1").setRefresh(true).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setSource("field", "value1").setRefresh(true).execute().actionGet(); fail(); } catch (ElasticsearchException e) { assertThat(e.unwrapCause(), instanceOf(RoutingMissingException.class)); @@ -195,44 +196,45 @@ public void testRequiredRoutingMapping() throws Exception { logger.info("--> verifying get with routing, should find"); for (int i = 0; i < 5; i++) { - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); } logger.info("--> deleting with no routing, should broadcast the delete since _routing is required"); - client().prepareDelete("test", "type1", "1").setRefresh(true).execute().actionGet(); + client().prepareDelete(indexOrAlias(), "type1", "1").setRefresh(true).execute().actionGet(); for (int i = 0; i < 5; i++) { try { - client().prepareGet("test", "type1", "1").execute().actionGet().isExists(); + client().prepareGet(indexOrAlias(), "type1", "1").execute().actionGet().isExists(); fail(); } catch (RoutingMissingException e) { assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST)); assertThat(e.getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); } - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(false)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(false)); } logger.info("--> indexing with id [1], and routing [0]"); - client().prepareIndex("test", "type1", "1").setRouting("0").setSource("field", "value1").setRefresh(true).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setRouting("0").setSource("field", "value1").setRefresh(true).execute().actionGet(); logger.info("--> verifying get with no routing, should not find anything"); logger.info("--> bulk deleting with no routing, should broadcast the delete since _routing is required"); - client().prepareBulk().add(Requests.deleteRequest("test").type("type1").id("1")).execute().actionGet(); + client().prepareBulk().add(Requests.deleteRequest(indexOrAlias()).type("type1").id("1")).execute().actionGet(); client().admin().indices().prepareRefresh().execute().actionGet(); for (int i = 0; i < 5; i++) { try { - client().prepareGet("test", "type1", "1").execute().actionGet().isExists(); + client().prepareGet(indexOrAlias(), "type1", "1").execute().actionGet().isExists(); fail(); } catch (RoutingMissingException e) { assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST)); assertThat(e.getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); } - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(false)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(false)); } } @Test public void testRequiredRoutingWithPathMapping() throws Exception { client().admin().indices().prepareCreate("test") + .addAlias(new Alias("alias")) .addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1") .startObject("_routing").field("required", true).field("path", "routing_field").endObject().startObject("properties") .startObject("routing_field").field("type", "string").field("index", randomBoolean() ? "no" : "not_analyzed").field("doc_values", randomBoolean() ? "yes" : "no").endObject().endObject() @@ -241,11 +243,11 @@ public void testRequiredRoutingWithPathMapping() throws Exception { ensureGreen(); logger.info("--> indexing with id [1], and routing [0]"); - client().prepareIndex("test", "type1", "1").setSource("field", "value1", "routing_field", "0").setRefresh(true).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setSource("field", "value1", "routing_field", "0").setRefresh(true).execute().actionGet(); logger.info("--> check failure with different routing"); try { - client().prepareIndex("test", "type1", "1").setRouting("1").setSource("field", "value1", "routing_field", "0").setRefresh(true).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setRouting("1").setSource("field", "value1", "routing_field", "0").setRefresh(true).execute().actionGet(); fail(); } catch (ElasticsearchException e) { assertThat(e.unwrapCause(), instanceOf(MapperParsingException.class)); @@ -255,7 +257,7 @@ public void testRequiredRoutingWithPathMapping() throws Exception { logger.info("--> verifying get with no routing, should fail"); for (int i = 0; i < 5; i++) { try { - client().prepareGet("test", "type1", "1").execute().actionGet().isExists(); + client().prepareGet(indexOrAlias(), "type1", "1").execute().actionGet().isExists(); fail(); } catch (RoutingMissingException e) { assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST)); @@ -264,13 +266,14 @@ public void testRequiredRoutingWithPathMapping() throws Exception { } logger.info("--> verifying get with routing, should find"); for (int i = 0; i < 5; i++) { - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); } } @Test public void testRequiredRoutingWithPathMappingBulk() throws Exception { client().admin().indices().prepareCreate("test") + .addAlias(new Alias("alias")) .addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1") .startObject("_routing").field("required", true).field("path", "routing_field").endObject() .endObject().endObject()) @@ -279,13 +282,13 @@ public void testRequiredRoutingWithPathMappingBulk() throws Exception { logger.info("--> indexing with id [1], and routing [0]"); client().prepareBulk().add( - client().prepareIndex("test", "type1", "1").setSource("field", "value1", "routing_field", "0")).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setSource("field", "value1", "routing_field", "0")).execute().actionGet(); client().admin().indices().prepareRefresh().execute().actionGet(); logger.info("--> verifying get with no routing, should fail"); for (int i = 0; i < 5; i++) { try { - client().prepareGet("test", "type1", "1").execute().actionGet().isExists(); + client().prepareGet(indexOrAlias(), "type1", "1").execute().actionGet().isExists(); fail(); } catch (RoutingMissingException e) { assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST)); @@ -294,7 +297,7 @@ public void testRequiredRoutingWithPathMappingBulk() throws Exception { } logger.info("--> verifying get with routing, should find"); for (int i = 0; i < 5; i++) { - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); } } @@ -302,6 +305,7 @@ public void testRequiredRoutingWithPathMappingBulk() throws Exception { public void testRequiredRoutingWithPathNumericType() throws Exception { client().admin().indices().prepareCreate("test") + .addAlias(new Alias("alias")) .addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1") .startObject("_routing").field("required", true).field("path", "routing_field").endObject() .endObject().endObject()) @@ -309,13 +313,13 @@ public void testRequiredRoutingWithPathNumericType() throws Exception { ensureGreen(); logger.info("--> indexing with id [1], and routing [0]"); - client().prepareIndex("test", "type1", "1").setSource("field", "value1", "routing_field", 0).execute().actionGet(); + client().prepareIndex(indexOrAlias(), "type1", "1").setSource("field", "value1", "routing_field", 0).execute().actionGet(); client().admin().indices().prepareRefresh().execute().actionGet(); logger.info("--> verifying get with no routing, should fail"); for (int i = 0; i < 5; i++) { try { - client().prepareGet("test", "type1", "1").execute().actionGet().isExists(); + client().prepareGet(indexOrAlias(), "type1", "1").execute().actionGet().isExists(); fail(); } catch (RoutingMissingException e) { assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST)); @@ -324,35 +328,35 @@ public void testRequiredRoutingWithPathNumericType() throws Exception { } logger.info("--> verifying get with routing, should find"); for (int i = 0; i < 5; i++) { - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); } } @Test public void testRequiredRoutingMapping_variousAPIs() throws Exception { - client().admin().indices().prepareCreate("test") + client().admin().indices().prepareCreate("test").addAlias(new Alias("alias")) .addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("_routing").field("required", true).endObject().endObject().endObject()) .execute().actionGet(); ensureGreen(); logger.info("--> indexing with id [1], and routing [0]"); - client().prepareIndex("test", "type1", "1").setRouting("0").setSource("field", "value1").get(); + client().prepareIndex(indexOrAlias(), "type1", "1").setRouting("0").setSource("field", "value1").get(); logger.info("--> indexing with id [2], and routing [0]"); - client().prepareIndex("test", "type1", "2").setRouting("0").setSource("field", "value2").setRefresh(true).get(); + client().prepareIndex(indexOrAlias(), "type1", "2").setRouting("0").setSource("field", "value2").setRefresh(true).get(); logger.info("--> verifying get with id [1] with routing [0], should succeed"); - assertThat(client().prepareGet("test", "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); + assertThat(client().prepareGet(indexOrAlias(), "type1", "1").setRouting("0").execute().actionGet().isExists(), equalTo(true)); logger.info("--> verifying get with id [1], with no routing, should fail"); try { - client().prepareGet("test", "type1", "1").get(); + client().prepareGet(indexOrAlias(), "type1", "1").get(); fail(); } catch (RoutingMissingException e) { assertThat(e.getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); } logger.info("--> verifying explain with id [2], with routing [0], should succeed"); - ExplainResponse explainResponse = client().prepareExplain("test", "type1", "2") + ExplainResponse explainResponse = client().prepareExplain(indexOrAlias(), "type1", "2") .setQuery(QueryBuilders.matchAllQuery()) .setRouting("0").get(); assertThat(explainResponse.isExists(), equalTo(true)); @@ -360,7 +364,7 @@ public void testRequiredRoutingMapping_variousAPIs() throws Exception { logger.info("--> verifying explain with id [2], with no routing, should fail"); try { - client().prepareExplain("test", "type1", "2") + client().prepareExplain(indexOrAlias(), "type1", "2") .setQuery(QueryBuilders.matchAllQuery()).get(); fail(); } catch (RoutingMissingException e) { @@ -368,24 +372,24 @@ public void testRequiredRoutingMapping_variousAPIs() throws Exception { } logger.info("--> verifying term vector with id [1], with routing [0], should succeed"); - TermVectorResponse termVectorResponse = client().prepareTermVector("test", "type1", "1").setRouting("0").get(); + TermVectorResponse termVectorResponse = client().prepareTermVector(indexOrAlias(), "type1", "1").setRouting("0").get(); assertThat(termVectorResponse.isExists(), equalTo(true)); assertThat(termVectorResponse.getId(), equalTo("1")); try { - client().prepareTermVector("test", "type1", "1").get(); + client().prepareTermVector(indexOrAlias(), "type1", "1").get(); fail(); } catch (RoutingMissingException e) { assertThat(e.getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); } - UpdateResponse updateResponse = client().prepareUpdate("test", "type1", "1").setRouting("0") + UpdateResponse updateResponse = client().prepareUpdate(indexOrAlias(), "type1", "1").setRouting("0") .setDoc("field1", "value1").get(); assertThat(updateResponse.getId(), equalTo("1")); assertThat(updateResponse.getVersion(), equalTo(2l)); try { - client().prepareUpdate("test", "type1", "1").setDoc("field1", "value1").get(); + client().prepareUpdate(indexOrAlias(), "type1", "1").setDoc("field1", "value1").get(); fail(); } catch (RoutingMissingException e) { assertThat(e.getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); @@ -393,8 +397,8 @@ public void testRequiredRoutingMapping_variousAPIs() throws Exception { logger.info("--> verifying mget with ids [1,2], with routing [0], should succeed"); MultiGetResponse multiGetResponse = client().prepareMultiGet() - .add(new MultiGetRequest.Item("test", "type1", "1").routing("0")) - .add(new MultiGetRequest.Item("test", "type1", "2").routing("0")).get(); + .add(new MultiGetRequest.Item(indexOrAlias(), "type1", "1").routing("0")) + .add(new MultiGetRequest.Item(indexOrAlias(), "type1", "2").routing("0")).get(); assertThat(multiGetResponse.getResponses().length, equalTo(2)); assertThat(multiGetResponse.getResponses()[0].isFailed(), equalTo(false)); assertThat(multiGetResponse.getResponses()[0].getResponse().getId(), equalTo("1")); @@ -403,19 +407,19 @@ public void testRequiredRoutingMapping_variousAPIs() throws Exception { logger.info("--> verifying mget with ids [1,2], with no routing, should fail"); multiGetResponse = client().prepareMultiGet() - .add(new MultiGetRequest.Item("test", "type1", "1")) - .add(new MultiGetRequest.Item("test", "type1", "2")).get(); + .add(new MultiGetRequest.Item(indexOrAlias(), "type1", "1")) + .add(new MultiGetRequest.Item(indexOrAlias(), "type1", "2")).get(); assertThat(multiGetResponse.getResponses().length, equalTo(2)); assertThat(multiGetResponse.getResponses()[0].isFailed(), equalTo(true)); assertThat(multiGetResponse.getResponses()[0].getFailure().getId(), equalTo("1")); - assertThat(multiGetResponse.getResponses()[0].getFailure().getMessage(), equalTo("routing is required, but hasn't been specified")); + assertThat(multiGetResponse.getResponses()[0].getFailure().getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); assertThat(multiGetResponse.getResponses()[1].isFailed(), equalTo(true)); assertThat(multiGetResponse.getResponses()[1].getFailure().getId(), equalTo("2")); - assertThat(multiGetResponse.getResponses()[1].getFailure().getMessage(), equalTo("routing is required, but hasn't been specified")); + assertThat(multiGetResponse.getResponses()[1].getFailure().getMessage(), equalTo("routing is required for [test]/[type1]/[2]")); MultiTermVectorsResponse multiTermVectorsResponse = client().prepareMultiTermVectors() - .add(new TermVectorRequest("test", "type1", "1").routing("0")) - .add(new TermVectorRequest("test", "type1", "2").routing("0")).get(); + .add(new TermVectorRequest(indexOrAlias(), "type1", "1").routing("0")) + .add(new TermVectorRequest(indexOrAlias(), "type1", "2").routing("0")).get(); assertThat(multiTermVectorsResponse.getResponses().length, equalTo(2)); assertThat(multiTermVectorsResponse.getResponses()[0].getId(), equalTo("1")); assertThat(multiTermVectorsResponse.getResponses()[0].isFailed(), equalTo(false)); @@ -427,16 +431,20 @@ public void testRequiredRoutingMapping_variousAPIs() throws Exception { assertThat(multiTermVectorsResponse.getResponses()[1].getResponse().isExists(), equalTo(true)); multiTermVectorsResponse = client().prepareMultiTermVectors() - .add(new TermVectorRequest("test", "type1", "1")) - .add(new TermVectorRequest("test", "type1", "2")).get(); + .add(new TermVectorRequest(indexOrAlias(), "type1", "1")) + .add(new TermVectorRequest(indexOrAlias(), "type1", "2")).get(); assertThat(multiTermVectorsResponse.getResponses().length, equalTo(2)); assertThat(multiTermVectorsResponse.getResponses()[0].getId(), equalTo("1")); assertThat(multiTermVectorsResponse.getResponses()[0].isFailed(), equalTo(true)); - assertThat(multiTermVectorsResponse.getResponses()[0].getFailure().getMessage(), equalTo("routing is required, but hasn't been specified")); + assertThat(multiTermVectorsResponse.getResponses()[0].getFailure().getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); assertThat(multiTermVectorsResponse.getResponses()[0].getResponse(), nullValue()); assertThat(multiTermVectorsResponse.getResponses()[1].getId(), equalTo("2")); assertThat(multiTermVectorsResponse.getResponses()[1].isFailed(), equalTo(true)); assertThat(multiTermVectorsResponse.getResponses()[1].getResponse(),nullValue()); - assertThat(multiTermVectorsResponse.getResponses()[1].getFailure().getMessage(), equalTo("routing is required, but hasn't been specified")); + assertThat(multiTermVectorsResponse.getResponses()[1].getFailure().getMessage(), equalTo("routing is required for [test]/[type1]/[2]")); + } + + private static String indexOrAlias() { + return randomBoolean() ? "test" : "alias"; } }