From 68bc5cf1f5421de41df12bcb72462e5efd3b3558 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 7 Jun 2019 15:20:20 -0400 Subject: [PATCH 1/4] Refactor put mapping request validation for reuse This commit refactors put mapping request validation for reuse. The concrete case that we are after here is the ability to apply effectively the same framework to indices aliases requests. This commit refactors the put mapping request validation framework to allow for that. --- .../elasticsearch/action/ActionModule.java | 11 ++- .../action/RequestValidators.java | 48 ++++++++++ .../mapping/put/MappingRequestValidator.java | 40 -------- .../put/TransportPutMappingAction.java | 35 ++----- .../elasticsearch/plugins/ActionPlugin.java | 11 ++- .../action/RequestValidatorsTests.java | 94 +++++++++++++++++++ ...sportPutMappingRequestValidatorsTests.java | 88 ----------------- .../put/ValidateMappingRequestPluginIT.java | 9 +- .../snapshots/SnapshotResiliencyTests.java | 4 +- .../test/hamcrest/OptionalMatchers.java | 84 +++++++++++++++++ .../java/org/elasticsearch/xpack/ccr/Ccr.java | 5 +- .../xpack/ccr/action/CcrRequests.java | 39 ++++---- .../core/LocalStateCompositeXPackPlugin.java | 4 +- 13 files changed, 281 insertions(+), 191 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/action/RequestValidators.java delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/MappingRequestValidator.java create mode 100644 server/src/test/java/org/elasticsearch/action/RequestValidatorsTests.java delete mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingRequestValidatorsTests.java create mode 100644 test/framework/src/main/java/org/elasticsearch/test/hamcrest/OptionalMatchers.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 13d7f9c0794b2..bd1249bc3e132 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -118,6 +118,7 @@ import org.elasticsearch.action.admin.indices.mapping.get.TransportGetFieldMappingsIndexAction; import org.elasticsearch.action.admin.indices.mapping.get.TransportGetMappingsAction; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction; import org.elasticsearch.action.admin.indices.open.OpenIndexAction; import org.elasticsearch.action.admin.indices.open.TransportOpenIndexAction; @@ -204,6 +205,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.NamedRegistry; import org.elasticsearch.common.inject.AbstractModule; +import org.elasticsearch.common.inject.TypeLiteral; import org.elasticsearch.common.inject.multibindings.MapBinder; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.IndexScopedSettings; @@ -358,7 +360,7 @@ public class ActionModule extends AbstractModule { private final AutoCreateIndex autoCreateIndex; private final DestructiveOperations destructiveOperations; private final RestController restController; - private final TransportPutMappingAction.RequestValidators mappingRequestValidators; + private final RequestValidators mappingRequestValidators; public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpressionResolver, IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter, @@ -389,9 +391,8 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr restWrapper = newRestWrapper; } } - mappingRequestValidators = new TransportPutMappingAction.RequestValidators( - actionPlugins.stream().flatMap(p -> p.mappingRequestValidators().stream()).collect(Collectors.toList()) - ); + mappingRequestValidators = new RequestValidators<>( + actionPlugins.stream().flatMap(p -> p.mappingRequestValidators().stream()).collect(Collectors.toList())); restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService); } @@ -684,7 +685,7 @@ public void initRestHandlers(Supplier nodesInCluster) { protected void configure() { bind(ActionFilters.class).toInstance(actionFilters); bind(DestructiveOperations.class).toInstance(destructiveOperations); - bind(TransportPutMappingAction.RequestValidators.class).toInstance(mappingRequestValidators); + bind(new TypeLiteral>() {}).toInstance(mappingRequestValidators); bind(AutoCreateIndex.class).toInstance(autoCreateIndex); bind(TransportLivenessAction.class).asEagerSingleton(); diff --git a/server/src/main/java/org/elasticsearch/action/RequestValidators.java b/server/src/main/java/org/elasticsearch/action/RequestValidators.java new file mode 100644 index 0000000000000..168fafd98f2a2 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/RequestValidators.java @@ -0,0 +1,48 @@ +package org.elasticsearch.action; + +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.index.Index; + +import java.util.Collection; +import java.util.Optional; + +public class RequestValidators { + + private final Collection> validators; + + public RequestValidators(Collection> validators) { + this.validators = validators; + } + + public Optional validateRequest(final T request, final ClusterState state, final Index[] indices) { + Exception exception = null; + for (final var validator : validators) { + final Optional maybeException = validator.validateRequest(request, state, indices); + if (maybeException.isEmpty()) continue; + if (exception == null) { + exception = maybeException.get(); + } else { + exception.addSuppressed(maybeException.get()); + } + } + return Optional.ofNullable(exception); + } + + /** + * A validator that validates an request associated with indices before executing it. + */ + public interface RequestValidator { + + /** + * Validates a given request with its associated concrete indices and the current state. + * + * @param request the request to validate + * @param state the current cluster state + * @param indices the concrete indices that associated with the given request + * @return an optional exception indicates a reason that the given request should be aborted, otherwise empty + */ + Optional validateRequest(T request, ClusterState state, Index[] indices); + + } + +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/MappingRequestValidator.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/MappingRequestValidator.java deleted file mode 100644 index 8d6608c575874..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/MappingRequestValidator.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.action.admin.indices.mapping.put; - -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.index.Index; - -/** - * A validator that validates a {@link PutMappingRequest} before executing it. - * @see TransportPutMappingAction.RequestValidators - */ -public interface MappingRequestValidator { - - /** - * Validates a given put mapping request with its associated concrete indices and the current state. - * - * @param request the request to validate - * @param state the current cluster state - * @param indices the concrete indices that associated with the given put mapping request - * @return a non-null exception indicates a reason that the given request should be aborted; otherwise returns null. - */ - Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices); -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java index cc18e852941f3..29c28b85e1417 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java @@ -21,6 +21,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.RequestValidators; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.TransportMasterNodeAction; @@ -37,7 +38,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.Collection; +import java.util.Optional; /** * Put mapping action. @@ -45,13 +46,13 @@ public class TransportPutMappingAction extends TransportMasterNodeAction { private final MetaDataMappingService metaDataMappingService; - private final RequestValidators requestValidators; + private final RequestValidators requestValidators; @Inject public TransportPutMappingAction(TransportService transportService, ClusterService clusterService, ThreadPool threadPool, MetaDataMappingService metaDataMappingService, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, - RequestValidators requestValidators) { + RequestValidators requestValidators) { super(PutMappingAction.NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver, PutMappingRequest::new); this.metaDataMappingService = metaDataMappingService; @@ -87,9 +88,9 @@ protected void masterOperation(final PutMappingRequest request, final ClusterSta final Index[] concreteIndices = request.getConcreteIndex() == null ? indexNameExpressionResolver.concreteIndices(state, request) : new Index[] {request.getConcreteIndex()}; - final Exception validationException = requestValidators.validateRequest(request, state, concreteIndices); - if (validationException != null) { - listener.onFailure(validationException); + final Optional maybeValidationException = requestValidators.validateRequest(request, state, concreteIndices); + if (maybeValidationException.isPresent()) { + listener.onFailure(maybeValidationException.get()); return; } PutMappingClusterStateUpdateRequest updateRequest = new PutMappingClusterStateUpdateRequest() @@ -118,26 +119,4 @@ public void onFailure(Exception t) { } } - - public static class RequestValidators { - private final Collection validators; - - public RequestValidators(Collection validators) { - this.validators = validators; - } - - Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices) { - Exception firstException = null; - for (MappingRequestValidator validator : validators) { - final Exception e = validator.validateRequest(request, state, indices); - if (e == null) continue; - if (firstException == null) { - firstException = e; - } else { - firstException.addSuppressed(e); - } - } - return firstException; - } - } } diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index adc2fa8f0b282..44400549657a9 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -22,8 +22,8 @@ import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.admin.indices.mapping.put.MappingRequestValidator; -import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction; +import org.elasticsearch.action.RequestValidators; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.TransportActions; @@ -183,10 +183,11 @@ public int hashCode() { } /** - * Returns a collection of validators that are used by {@link TransportPutMappingAction.RequestValidators} to - * validate a {@link org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest} before the executing it. + * Returns a collection of validators that are used by {@link RequestValidators} to validate a + * {@link org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest} before the executing it. */ - default Collection mappingRequestValidators() { + default Collection> mappingRequestValidators() { return Collections.emptyList(); } + } diff --git a/server/src/test/java/org/elasticsearch/action/RequestValidatorsTests.java b/server/src/test/java/org/elasticsearch/action/RequestValidatorsTests.java new file mode 100644 index 0000000000000..f339a0b3e072e --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/RequestValidatorsTests.java @@ -0,0 +1,94 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action; + +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; +import org.elasticsearch.common.Randomness; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.hamcrest.OptionalMatchers; +import org.hamcrest.Matchers; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +public class RequestValidatorsTests extends ESTestCase { + + private final RequestValidators.RequestValidator EMPTY = (request, state, indices) -> Optional.empty(); + private final RequestValidators.RequestValidator FAIL = + (request, state, indices) -> Optional.of(new Exception("failure")); + + public void testValidates() { + final int numberOfValidations = randomIntBetween(0, 8); + final List> validators = new ArrayList<>(numberOfValidations); + for (int i = 0; i < numberOfValidations; i++) { + validators.add(EMPTY); + } + final RequestValidators requestValidators = new RequestValidators<>(validators); + assertThat(requestValidators.validateRequest(null, null, null), OptionalMatchers.isEmpty()); + } + + public void testFailure() { + final RequestValidators validators = new RequestValidators<>(List.of(FAIL)); + assertThat(validators.validateRequest(null, null, null), OptionalMatchers.isPresent()); + } + + public void testValidatesAfterFailure() { + final RequestValidators validators = new RequestValidators<>(List.of(FAIL, EMPTY)); + assertThat(validators.validateRequest(null, null, null), OptionalMatchers.isPresent()); + } + + public void testMultipleFailures() { + final int numberOfFailures = randomIntBetween(2, 8); + final List> validators = new ArrayList<>(numberOfFailures); + for (int i = 0; i < numberOfFailures; i++) { + validators.add(FAIL); + } + final RequestValidators requestValidators = new RequestValidators<>(validators); + final Optional e = requestValidators.validateRequest(null, null, null); + assertThat(e, OptionalMatchers.isPresent()); + // noinspection OptionalGetWithoutIsPresent + assertThat(e.get().getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1)); + } + + public void testRandom() { + final int numberOfValidations = randomIntBetween(0, 8); + final int numberOfFailures = randomIntBetween(0, 8); + final List> validators = + new ArrayList<>(numberOfValidations + numberOfFailures); + for (int i = 0; i < numberOfValidations; i++) { + validators.add(EMPTY); + } + for (int i = 0; i < numberOfFailures; i++) { + validators.add(FAIL); + } + Randomness.shuffle(validators); + final RequestValidators requestValidators = new RequestValidators<>(validators); + final Optional e = requestValidators.validateRequest(null, null, null); + if (numberOfFailures == 0) { + assertThat(e, OptionalMatchers.isEmpty()); + } else { + assertThat(e, OptionalMatchers.isPresent()); + // noinspection OptionalGetWithoutIsPresent + assertThat(e.get().getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1)); + } + } + +} diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingRequestValidatorsTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingRequestValidatorsTests.java deleted file mode 100644 index 1a6aeb04207c4..0000000000000 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingRequestValidatorsTests.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.action.admin.indices.mapping.put; - -import org.elasticsearch.common.Randomness; -import org.elasticsearch.test.ESTestCase; -import org.hamcrest.Matchers; - -import java.util.ArrayList; -import java.util.List; - -public class TransportPutMappingRequestValidatorsTests extends ESTestCase { - - private final MappingRequestValidator EMPTY = (request, state, indices) -> null; - private final MappingRequestValidator FAIL = (request, state, indices) -> new Exception("failure"); - - public void testValidates() { - final int numberOfValidations = randomIntBetween(0, 8); - final List validators = new ArrayList<>(numberOfValidations); - for (int i = 0; i < numberOfValidations; i++) { - validators.add(EMPTY); - } - final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators); - assertNull(requestValidators.validateRequest(null, null, null)); - } - - public void testFailure() { - final TransportPutMappingAction.RequestValidators validators = new TransportPutMappingAction.RequestValidators(List.of(FAIL)); - assertNotNull(validators.validateRequest(null, null, null)); - } - - public void testValidatesAfterFailure() { - final TransportPutMappingAction.RequestValidators validators = - new TransportPutMappingAction.RequestValidators(List.of(FAIL, EMPTY)); - assertNotNull(validators.validateRequest(null, null, null)); - } - - public void testMultipleFailures() { - final int numberOfFailures = randomIntBetween(2, 8); - final List validators = new ArrayList<>(numberOfFailures); - for (int i = 0; i < numberOfFailures; i++) { - validators.add(FAIL); - } - final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators); - final Exception e = requestValidators.validateRequest(null, null, null); - assertNotNull(e); - assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1)); - } - - public void testRandom() { - final int numberOfValidations = randomIntBetween(0, 8); - final int numberOfFailures = randomIntBetween(0, 8); - final List validators = new ArrayList<>(numberOfValidations + numberOfFailures); - for (int i = 0; i < numberOfValidations; i++) { - validators.add(EMPTY); - } - for (int i = 0; i < numberOfFailures; i++) { - validators.add(FAIL); - } - Randomness.shuffle(validators); - final TransportPutMappingAction.RequestValidators requestValidators = new TransportPutMappingAction.RequestValidators(validators); - final Exception e = requestValidators.validateRequest(null, null, null); - if (numberOfFailures == 0) { - assertNull(e); - } else { - assertNotNull(e); - } - assertThat(e.getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1)); - } - -} diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/ValidateMappingRequestPluginIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/ValidateMappingRequestPluginIT.java index b25c9ecb5fc91..48398742e9dd6 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/ValidateMappingRequestPluginIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/ValidateMappingRequestPluginIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.mapping.put; +import org.elasticsearch.action.RequestValidators; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.index.Index; import org.elasticsearch.plugins.ActionPlugin; @@ -29,6 +30,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.Optional; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.containsString; @@ -38,14 +40,15 @@ public class ValidateMappingRequestPluginIT extends ESSingleNodeTestCase { static final Map> allowedOrigins = ConcurrentCollections.newConcurrentMap(); public static class TestPlugin extends Plugin implements ActionPlugin { @Override - public Collection mappingRequestValidators() { + public Collection> mappingRequestValidators() { return Collections.singletonList((request, state, indices) -> { for (Index index : indices) { if (allowedOrigins.getOrDefault(index.getName(), Collections.emptySet()).contains(request.origin()) == false) { - return new IllegalStateException("not allowed: index[" + index.getName() + "] origin[" + request.origin() + "]"); + return Optional.of( + new IllegalStateException("not allowed: index[" + index.getName() + "] origin[" + request.origin() + "]")); } } - return null; + return Optional.empty(); }); } } diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java index 285234999564e..4b9443294d189 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java @@ -24,6 +24,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.RequestValidators; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.put.TransportPutRepositoryAction; import org.elasticsearch.action.admin.cluster.reroute.ClusterRerouteAction; @@ -47,6 +48,7 @@ import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; import org.elasticsearch.action.admin.indices.delete.TransportDeleteIndexAction; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction; import org.elasticsearch.action.admin.indices.shards.IndicesShardStoresAction; import org.elasticsearch.action.admin.indices.shards.TransportIndicesShardStoresAction; @@ -1154,7 +1156,7 @@ clusterService, indicesService, threadPool, shardStateAction, mappingUpdatedActi ); actions.put(PutMappingAction.INSTANCE, new TransportPutMappingAction(transportService, clusterService, threadPool, metaDataMappingService, - actionFilters, indexNameExpressionResolver, new TransportPutMappingAction.RequestValidators(Collections.emptyList()))); + actionFilters, indexNameExpressionResolver, new RequestValidators<>(Collections.emptyList()))); final ResponseCollectorService responseCollectorService = new ResponseCollectorService(clusterService); final SearchTransportService searchTransportService = new SearchTransportService(transportService, SearchExecutionStatsCollector.makeWrapper(responseCollectorService)); diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/OptionalMatchers.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/OptionalMatchers.java new file mode 100644 index 0000000000000..6481dcd352548 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/OptionalMatchers.java @@ -0,0 +1,84 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.test.hamcrest; + +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; + +import java.util.Optional; + +public class OptionalMatchers { + + private static class IsEmptyMatcher extends TypeSafeMatcher> { + + @Override + protected boolean matchesSafely(final Optional item) { + // noinspection OptionalAssignedToNull + return item != null && item.isEmpty(); + } + + @Override + public void describeTo(final Description description) { + description.appendText("expected empty optional"); + } + + @Override + protected void describeMismatchSafely(final Optional item, final Description mismatchDescription) { + if (item == null) { + mismatchDescription.appendText("was null"); + } else { + mismatchDescription.appendText("was ").appendText(item.toString()); + } + } + + } + + public static IsEmptyMatcher isEmpty() { + return new IsEmptyMatcher(); + } + + private static class IsPresentMatcher extends TypeSafeMatcher> { + + @Override + protected boolean matchesSafely(final Optional item) { + return item != null && item.isPresent(); + } + + @Override + public void describeTo(final Description description) { + description.appendText("expected non-empty optional"); + } + + @Override + protected void describeMismatchSafely(final Optional item, final Description mismatchDescription) { + if (item == null) { + mismatchDescription.appendText("was null"); + } else { + mismatchDescription.appendText("was empty"); + } + } + + } + + public static IsPresentMatcher isPresent() { + return new IsPresentMatcher(); + } + +} diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java index c74e39f017a3c..89893637e65b8 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/Ccr.java @@ -9,7 +9,8 @@ import org.apache.lucene.util.SetOnce; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.admin.indices.mapping.put.MappingRequestValidator; +import org.elasticsearch.action.RequestValidators; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; @@ -344,7 +345,7 @@ public Collection createGuiceModules() { protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); } @Override - public Collection mappingRequestValidators() { + public Collection> mappingRequestValidators() { return Collections.singletonList(CcrRequests.CCR_PUT_MAPPING_REQUEST_VALIDATOR); } diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java index 02ee7d1f138c9..a50e3c301bdc3 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java @@ -7,9 +7,9 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.RequestValidators; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; -import org.elasticsearch.action.admin.indices.mapping.put.MappingRequestValidator; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -85,21 +86,23 @@ public static void getIndexMetadata(Client client, Index index, long mappingVers )); } - public static final MappingRequestValidator CCR_PUT_MAPPING_REQUEST_VALIDATOR = (request, state, indices) -> { - if (request.origin() == null) { - return null; // a put-mapping-request on old versions does not have origin. - } - final List followingIndices = Arrays.stream(indices) - .filter(index -> { - final IndexMetaData indexMetaData = state.metaData().index(index); - return indexMetaData != null && CcrSettings.CCR_FOLLOWING_INDEX_SETTING.get(indexMetaData.getSettings()); - }).collect(Collectors.toList()); - if (followingIndices.isEmpty() == false && "ccr".equals(request.origin()) == false) { - final String errorMessage = "can't put mapping to the following indices " - + "[" + followingIndices.stream().map(Index::getName).collect(Collectors.joining(", ")) + "]; " - + "the mapping of the following indices are self-replicated from its leader indices"; - return new ElasticsearchStatusException(errorMessage, RestStatus.FORBIDDEN); - } - return null; - }; + public static final RequestValidators.RequestValidator CCR_PUT_MAPPING_REQUEST_VALIDATOR = + (request, state, indices) -> { + if (request.origin() == null) { + return Optional.empty(); // a put-mapping-request on old versions does not have origin. + } + final List followingIndices = Arrays.stream(indices) + .filter(index -> { + final IndexMetaData indexMetaData = state.metaData().index(index); + return indexMetaData != null && CcrSettings.CCR_FOLLOWING_INDEX_SETTING.get(indexMetaData.getSettings()); + }).collect(Collectors.toList()); + if (followingIndices.isEmpty() == false && "ccr".equals(request.origin()) == false) { + final String errorMessage = "can't put mapping to the following indices " + + "[" + followingIndices.stream().map(Index::getName).collect(Collectors.joining(", ")) + "]; " + + "the mapping of the following indices are self-replicated from its leader indices"; + return Optional.of(new ElasticsearchStatusException(errorMessage, RestStatus.FORBIDDEN)); + } + return Optional.empty(); + }; + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java index 3f8b279e5016c..ef478e59bb0a6 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java @@ -7,7 +7,9 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.RequestValidators; import org.elasticsearch.action.admin.indices.mapping.put.MappingRequestValidator; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.bootstrap.BootstrapCheck; import org.elasticsearch.client.Client; @@ -431,7 +433,7 @@ public Optional getEngineFactory(IndexSettings indexSettings) { } @Override - public Collection mappingRequestValidators() { + public Collection> mappingRequestValidators() { return filterPlugins(ActionPlugin.class).stream().flatMap(p -> p.mappingRequestValidators().stream()) .collect(Collectors.toList()); } From 63b5752388496dcdc8376f99d46abc4b356b3588 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 7 Jun 2019 16:17:39 -0400 Subject: [PATCH 2/4] Fix compilation --- .../mapping/put/TransportPutMappingAction.java | 15 ++++++++++----- .../core/LocalStateCompositeXPackPlugin.java | 7 ++++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java index 29c28b85e1417..a271a6c819928 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java @@ -38,6 +38,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import java.util.Objects; import java.util.Optional; /** @@ -49,14 +50,18 @@ public class TransportPutMappingAction extends TransportMasterNodeAction requestValidators; @Inject - public TransportPutMappingAction(TransportService transportService, ClusterService clusterService, - ThreadPool threadPool, MetaDataMappingService metaDataMappingService, - ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, - RequestValidators requestValidators) { + public TransportPutMappingAction( + final TransportService transportService, + final ClusterService clusterService, + final ThreadPool threadPool, + final MetaDataMappingService metaDataMappingService, + final ActionFilters actionFilters, + final IndexNameExpressionResolver indexNameExpressionResolver, + final RequestValidators requestValidators) { super(PutMappingAction.NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver, PutMappingRequest::new); this.metaDataMappingService = metaDataMappingService; - this.requestValidators = requestValidators; + this.requestValidators = Objects.requireNonNull(requestValidators); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java index ef478e59bb0a6..9caf304a4f5b3 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java @@ -8,7 +8,6 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.RequestValidators; -import org.elasticsearch.action.admin.indices.mapping.put.MappingRequestValidator; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.bootstrap.BootstrapCheck; @@ -434,8 +433,10 @@ public Optional getEngineFactory(IndexSettings indexSettings) { @Override public Collection> mappingRequestValidators() { - return filterPlugins(ActionPlugin.class).stream().flatMap(p -> p.mappingRequestValidators().stream()) - .collect(Collectors.toList()); + return filterPlugins(ActionPlugin.class) + .stream() + .flatMap(p -> p.mappingRequestValidators().stream()) + .collect(Collectors.toList()); } private List filterPlugins(Class type) { From bf62e09a3337464c9b51a1da51603d224af79a6e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 7 Jun 2019 16:29:26 -0400 Subject: [PATCH 3/4] Fix imports --- .../org/elasticsearch/snapshots/SnapshotResiliencyTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java index 4b9443294d189..f94e76f91d4f5 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java @@ -48,7 +48,6 @@ import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; import org.elasticsearch.action.admin.indices.delete.TransportDeleteIndexAction; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction; import org.elasticsearch.action.admin.indices.shards.IndicesShardStoresAction; import org.elasticsearch.action.admin.indices.shards.TransportIndicesShardStoresAction; From 1f00a66450cd88a76a088be5a8f770cf7201414b Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 7 Jun 2019 16:40:57 -0400 Subject: [PATCH 4/4] Add license header --- .../action/RequestValidators.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/action/RequestValidators.java b/server/src/main/java/org/elasticsearch/action/RequestValidators.java index 168fafd98f2a2..95bfcca51d218 100644 --- a/server/src/main/java/org/elasticsearch/action/RequestValidators.java +++ b/server/src/main/java/org/elasticsearch/action/RequestValidators.java @@ -1,3 +1,22 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.elasticsearch.action; import org.elasticsearch.cluster.ClusterState;