MultiGet & MultiTermVector api: fail when using no routing and an ali…

…as 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
elastic · Aug 4, 2014 · 8989d06 · 8989d06
1 parent 5795e4f
commit 8989d06
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 58 deletions.
diff --git 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<Mul
                 responses.set(i, new MultiGetItemResponse(null, new MultiGetResponse.Failure(item.index(), item.type(), item.id(), "[" + item.index() + "] missing")));
                 continue;
             }
-            if (item.routing() == null && clusterState.getMetaData().routingRequired(item.index(), item.type())) {
-                responses.set(i, new MultiGetItemResponse(null, new MultiGetResponse.Failure(item.index(), item.type(), item.id(), "routing is required, but hasn't been specified")));
-                continue;
-            }
 
             item.routing(clusterState.metaData().resolveIndexRouting(item.routing(), item.index()));
             item.index(clusterState.metaData().concreteSingleIndex(item.index(), item.indicesOptions()));
+            if (item.routing() == null && clusterState.getMetaData().routingRequired(item.index(), item.type())) {
+                responses.set(i, new MultiGetItemResponse(null, new MultiGetResponse.Failure(item.index(), item.type(), item.id(),
+                        "routing is required for [" + item.index() + "]/[" + item.type() + "]/[" + item.id() + "]")));
+                continue;
+            }
             ShardId shardId = clusterService.operationRouting()
                     .getShards(clusterState, item.index(), item.type(), item.id(), item.routing(), null).shardId();
             MultiGetShardRequest shardRequest = shardRequests.get(shardId);

diff --git a/src/main/java/org/elasticsearch/action/termvector/TransportMultiTermVectorsAction.java b/src/main/java/org/elasticsearch/action/termvector/TransportMultiTermVectorsAction.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;
@@ -71,12 +68,12 @@ protected void doExecute(final MultiTermVectorsRequest request, final ActionList
                         termVectorRequest.type(), termVectorRequest.id(), "[" + termVectorRequest.index() + "] missing")));
                 continue;
             }
+            termVectorRequest.index(clusterState.metaData().concreteSingleIndex(termVectorRequest.index(), termVectorRequest.indicesOptions()));
             if (termVectorRequest.routing() == null && clusterState.getMetaData().routingRequired(termVectorRequest.index(), termVectorRequest.type())) {
-                responses.set(i, new MultiTermVectorsItemResponse(null, new MultiTermVectorsResponse.Failure(termVectorRequest.index(),
-                        termVectorRequest.type(), termVectorRequest.id(), "routing is required, but hasn't been specified")));
+                responses.set(i, new MultiTermVectorsItemResponse(null, new MultiTermVectorsResponse.Failure(termVectorRequest.index(), termVectorRequest.type(), termVectorRequest.id(),
+                        "routing is required for [" + termVectorRequest.index() + "]/[" + termVectorRequest.type() + "]/[" + termVectorRequest.id() + "]")));
                 continue;
             }
-            termVectorRequest.index(clusterState.metaData().concreteSingleIndex(termVectorRequest.index(), termVectorRequest.indicesOptions()));
             ShardId shardId = clusterService
                     .operationRouting()
                     .getShards(clusterState, termVectorRequest.index(), termVectorRequest.type(), termVectorRequest.id(),

diff --git a/src/test/java/org/elasticsearch/mget/SimpleMgetTests.java b/src/test/java/org/elasticsearch/mget/SimpleMgetTests.java
@@ -98,7 +98,7 @@ public void testThatParentPerDocumentIsSupported() throws Exception {
 
         assertThat(mgetResponse.getResponses()[1].isFailed(), is(true));
         assertThat(mgetResponse.getResponses()[1].getResponse(), nullValue());
-        assertThat(mgetResponse.getResponses()[1].getFailure().getMessage(), equalTo("routing is required, but hasn't been specified"));
+        assertThat(mgetResponse.getResponses()[1].getFailure().getMessage(), equalTo("routing is required for [test]/[test]/[1]"));
     }
 
     @SuppressWarnings("unchecked")