diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java index 061f5fa856f..35a5eb64b07 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java @@ -23,8 +23,10 @@ import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.action.document.RestBulkAction; @@ -160,9 +162,10 @@ public class IndexingIT extends AbstractRollingTestCase { switch (CLUSTER_TYPE) { case OLD: - Request createTestIndex = new Request("PUT", "/" + indexName); - createTestIndex.setJsonEntity("{\"settings\": {\"index.number_of_replicas\": 0}}"); - client().performRequest(createTestIndex); + Settings.Builder settings = Settings.builder() + .put(IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1) + .put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0); + createIndex(indexName, settings.build()); break; case MIXED: Request waitForGreen = new Request("GET", "/_cluster/health"); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/create.json b/rest-api-spec/src/main/resources/rest-api-spec/api/create.json index 0a96ebf5ecd..171f3da44d3 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/create.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/create.json @@ -83,8 +83,7 @@ "options":[ "internal", "external", - "external_gte", - "force" + "external_gte" ], "description":"Specific version type" }, diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/index.json b/rest-api-spec/src/main/resources/rest-api-spec/api/index.json index c1f8c95d86d..7ecd7a0e927 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/index.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/index.json @@ -96,8 +96,7 @@ "index", "create" ], - "default":"index", - "description":"Explicit operation type" + "description":"Explicit operation type. Defaults to `index` for requests with an explicit document ID, and to `create`for requests without an explicit document ID" }, "refresh":{ "type":"enum", @@ -125,8 +124,7 @@ "options":[ "internal", "external", - "external_gte", - "force" + "external_gte" ], "description":"Specific version type" }, diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index f4571e0abc9..0aa65578f22 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -206,6 +206,7 @@ import org.elasticsearch.action.update.UpdateAction; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.NamedRegistry; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.TypeLiteral; @@ -367,11 +368,12 @@ public class ActionModule extends AbstractModule { private final RestController restController; private final RequestValidators mappingRequestValidators; private final RequestValidators indicesAliasesRequestRequestValidators; + private final ClusterService clusterService; public ActionModule(boolean transportClient, Settings settings, IndexNameExpressionResolver indexNameExpressionResolver, IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter, ThreadPool threadPool, List actionPlugins, NodeClient nodeClient, - CircuitBreakerService circuitBreakerService, UsageService usageService) { + CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService) { this.transportClient = transportClient; this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; @@ -379,6 +381,7 @@ public class ActionModule extends AbstractModule { this.clusterSettings = clusterSettings; this.settingsFilter = settingsFilter; this.actionPlugins = actionPlugins; + this.clusterService = clusterService; actions = setupActions(actionPlugins); actionFilters = setupActionFilters(actionPlugins); autoCreateIndex = transportClient ? null : new AutoCreateIndex(settings, clusterSettings, indexNameExpressionResolver); @@ -624,7 +627,7 @@ public class ActionModule extends AbstractModule { registerHandler.accept(new RestUpgradeStatusAction(restController)); registerHandler.accept(new RestClearIndicesCacheAction(restController)); - registerHandler.accept(new RestIndexAction(restController)); + registerHandler.accept(new RestIndexAction(restController, clusterService)); registerHandler.accept(new RestGetAction(restController)); registerHandler.accept(new RestGetSourceAction(restController)); registerHandler.accept(new RestMultiGetAction(settings, restController)); diff --git a/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java index 70e5949c7d1..ddc3e6ded60 100644 --- a/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -186,7 +186,7 @@ public abstract class TransportClient extends AbstractClient { modules.add(b -> b.bind(ThreadPool.class).toInstance(threadPool)); ActionModule actionModule = new ActionModule(true, settings, null, settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(), threadPool, - pluginsService.filterPlugins(ActionPlugin.class), null, null, null); + pluginsService.filterPlugins(ActionPlugin.class), null, null, null, null); modules.add(actionModule); CircuitBreakerService circuitBreakerService = Node.createCircuitBreakerService(settingsModule.getSettings(), diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index f30750017fe..06938b91d21 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -453,7 +453,7 @@ public class Node implements Closeable { ActionModule actionModule = new ActionModule(false, settings, clusterModule.getIndexNameExpressionResolver(), settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(), - threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService); + threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, clusterService); modules.add(actionModule); final RestController restController = actionModule.getRestController(); diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index 80794be9827..20dfe10e82b 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -20,9 +20,11 @@ package org.elasticsearch.rest.action.document; import org.apache.logging.log4j.LogManager; +import org.elasticsearch.Version; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.mapper.MapperService; @@ -45,8 +47,13 @@ public class RestIndexAction extends BaseRestHandler { "index requests is deprecated, use the typeless endpoints instead (/{index}/_doc/{id}, /{index}/_doc, " + "or /{index}/_create/{id})."; - public RestIndexAction(RestController controller) { - controller.registerHandler(POST, "/{index}/_doc", this); // auto id creation + private final ClusterService clusterService; + + public RestIndexAction(RestController controller, ClusterService clusterService) { + this.clusterService = clusterService; + + AutoIdHandler autoIdHandler = new AutoIdHandler(); + controller.registerHandler(POST, "/{index}/_doc", autoIdHandler); // auto id creation controller.registerHandler(PUT, "/{index}/_doc/{id}", this); controller.registerHandler(POST, "/{index}/_doc/{id}", this); @@ -55,7 +62,7 @@ public class RestIndexAction extends BaseRestHandler { controller.registerHandler(POST, "/{index}/_create/{id}/", createHandler); // Deprecated typed endpoints. - controller.registerHandler(POST, "/{index}/{type}", this); // auto id creation + controller.registerHandler(POST, "/{index}/{type}", autoIdHandler); // auto id creation controller.registerHandler(PUT, "/{index}/{type}/{id}", this); controller.registerHandler(POST, "/{index}/{type}/{id}", this); controller.registerHandler(PUT, "/{index}/{type}/{id}/_create", createHandler); @@ -90,6 +97,26 @@ public class RestIndexAction extends BaseRestHandler { } } + final class AutoIdHandler extends BaseRestHandler { + protected AutoIdHandler() { + } + + @Override + public String getName() { + return "document_create_action"; + } + + @Override + public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { + assert request.params().get("id") == null : "non-null id: " + request.params().get("id"); + if (request.params().get("op_type") == null && clusterService.state().nodes().getMinNodeVersion().onOrAfter(Version.CURRENT)) { + // default to op_type create + request.params().put("op_type", "create"); + } + return RestIndexAction.this.prepareRequest(request, client); + } + } + @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { IndexRequest indexRequest; diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index cb5ce5ddb6e..7ee6d1f024c 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -109,7 +109,7 @@ public class ActionModuleTests extends ESTestCase { UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(false, settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), null, emptyList(), null, - null, usageService); + null, usageService, null); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail Exception e = expectThrows(IllegalArgumentException.class, () -> @@ -132,7 +132,7 @@ public class ActionModuleTests extends ESTestCase { UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(false, settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, - singletonList(dupsMainAction), null, null, usageService); + singletonList(dupsMainAction), null, null, usageService, null); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET")); } finally { @@ -164,7 +164,7 @@ public class ActionModuleTests extends ESTestCase { UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(false, settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, - singletonList(registersFakeHandler), null, null, usageService); + singletonList(registersFakeHandler), null, null, usageService, null); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail Exception e = expectThrows(IllegalArgumentException.class, () -> diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java index 2fd0ce25805..e7ee91b1807 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java @@ -20,21 +20,41 @@ package org.elasticsearch.rest.action.document; import org.elasticsearch.Version; -import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.DocWriteRequest; +import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.Before; +import org.mockito.ArgumentCaptor; + +import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class RestIndexActionTests extends RestActionTestCase { private RestIndexAction action; + private final AtomicReference clusterStateSupplier = new AtomicReference(); @Before public void setUpAction() { - action = new RestIndexAction(controller()); + ClusterService clusterService = mock(ClusterService.class); + when(clusterService.state()).thenAnswer(invocationOnMock -> clusterStateSupplier.get()); + action = new RestIndexAction(controller(), clusterService); } public void testTypeInPath() { @@ -68,7 +88,6 @@ public class RestIndexActionTests extends RestActionTestCase { } public void testCreateOpTypeValidation() { - Settings settings = settings(Version.CURRENT).build(); RestIndexAction.CreateHandler create = action.new CreateHandler(); String opType = randomFrom("CREATE", null); @@ -78,4 +97,30 @@ public class RestIndexActionTests extends RestActionTestCase { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> create.validateOpType(illegalOpType)); assertThat(e.getMessage(), equalTo("opType must be 'create', found: [" + illegalOpType + "]")); } + + public void testAutoIdDefaultsToOptypeCreate() { + checkAutoIdOpType(Version.CURRENT, DocWriteRequest.OpType.CREATE); + } + + public void testAutoIdDefaultsToOptypeIndexForOlderVersions() { + checkAutoIdOpType(VersionUtils.randomVersionBetween(random(), null, + VersionUtils.getPreviousVersion(Version.CURRENT)), DocWriteRequest.OpType.INDEX); + } + + private void checkAutoIdOpType(Version minClusterVersion, DocWriteRequest.OpType expectedOpType) { + RestRequest autoIdRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(RestRequest.Method.POST) + .withPath("/some_index/_doc") + .withContent(new BytesArray("{}"), XContentType.JSON) + .build(); + clusterStateSupplier.set(ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder() + .add(new DiscoveryNode("test", buildNewFakeTransportAddress(), minClusterVersion)) + .build()).build()); + dispatchRequest(autoIdRequest); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(IndexRequest.class); + verify(nodeClient).index(argumentCaptor.capture(), any(ActionListener.class)); + IndexRequest indexRequest = argumentCaptor.getValue(); + assertEquals(expectedOpType, indexRequest.opType()); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java index f5ab14971b8..a5d932a3d1a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java @@ -39,11 +39,13 @@ import static org.mockito.Mockito.mock; */ public abstract class RestActionTestCase extends ESTestCase { private RestController controller; + protected NodeClient nodeClient; @Before public void setUpController() { + nodeClient = mock(NodeClient.class); controller = new RestController(Collections.emptySet(), null, - mock(NodeClient.class), + nodeClient, new NoneCircuitBreakerService(), new UsageService()); }