honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Yannick Welsch 2019-10-02 11:04:19 +02:00
parent 0024695dd8
commit 99d2fe295d
10 changed files with 100 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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");

3
rest-api-spec/src/main/resources/rest-api-spec/api/create.json
View File

@ -83,8 +83,7 @@
"options":[
"internal",
"external",
"external_gte",
"force"
"external_gte"
],
"description":"Specific version type"
},

6
rest-api-spec/src/main/resources/rest-api-spec/api/index.json
View File

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

7
server/src/main/java/org/elasticsearch/action/ActionModule.java
View File

@ -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<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;
@ -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));

2
server/src/main/java/org/elasticsearch/client/transport/TransportClient.java
View File

@ -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(),

2
server/src/main/java/org/elasticsearch/node/Node.java
View File

@ -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();

33
server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java
View File

@ -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;

6
server/src/test/java/org/elasticsearch/action/ActionModuleTests.java
View File

@ -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, () ->

51
server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java
View File

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

4
test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java
View File

@ -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());
}