Use optype CREATE for single auto-id index requests (#47353)

Changes auto-id index requests to use optype CREATE, making it compliant with our docs.
This will also make these auto-id index requests compatible with the new "create-doc" index
privilege (which is based on the optype), the default optype is changed to create, just as it is
already documented.

Author: ywelsch

qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java

@@ -23,8 +23,10 @@
 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 void testAutoIdWithOpTypeCreate() throws IOException {
 
         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");

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"
       },

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"
       },

server/src/main/java/org/elasticsearch/action/ActionModule.java

@@ -206,6 +206,7 @@
 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,18 +368,20 @@ public class ActionModule extends AbstractModule {
     private final RestController restController;
     private final RequestValidators<PutMappingRequest> mappingRequestValidators;
     private final RequestValidators<IndicesAliasesRequest> indicesAliasesRequestRequestValidators;
+    private final ClusterService clusterService;
 
     public ActionModule(boolean transportClient, Settings settings, IndexNameExpressionResolver indexNameExpressionResolver,
                         IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter,
                         ThreadPool threadPool, List<ActionPlugin> actionPlugins, NodeClient nodeClient,
-            CircuitBreakerService circuitBreakerService, UsageService usageService) {
+                        CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService) {
         this.transportClient = transportClient;
         this.settings = settings;
         this.indexNameExpressionResolver = indexNameExpressionResolver;
         this.indexScopedSettings = indexScopedSettings;
         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 void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
         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));

server/src/main/java/org/elasticsearch/client/transport/TransportClient.java

@@ -186,7 +186,7 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
             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(),

server/src/main/java/org/elasticsearch/node/Node.java

@@ -453,7 +453,7 @@ protected Node(
 
             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();

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 RestIndexAction(RestController controller) {
         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 @@ void validateOpType(String opType) {
         }
     }
 
+    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;

server/src/test/java/org/elasticsearch/action/ActionModuleTests.java

@@ -109,7 +109,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() {
         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 List<RestHandler> getRestHandlers(Settings settings, RestController restC
             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 List<RestHandler> getRestHandlers(Settings settings, RestController restC
             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, () ->

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<ClusterState> 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 void testCreateWithTypeInPath() {
     }
 
     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 void testCreateOpTypeValidation() {
         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<IndexRequest> argumentCaptor = ArgumentCaptor.forClass(IndexRequest.class);
+        verify(nodeClient).index(argumentCaptor.capture(), any(ActionListener.class));
+        IndexRequest indexRequest = argumentCaptor.getValue();
+        assertEquals(expectedOpType, indexRequest.opType());
+    }
 }

test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java

@@ -39,11 +39,13 @@
  */
 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());
     }