Refactor put mapping request validation for reuse

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<PutMappingRequest> 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<DiscoveryNodes> nodesInCluster) {
     protected void configure() {
         bind(ActionFilters.class).toInstance(actionFilters);
         bind(DestructiveOperations.class).toInstance(destructiveOperations);
-        bind(TransportPutMappingAction.RequestValidators.class).toInstance(mappingRequestValidators);
+        bind(new TypeLiteral<RequestValidators<PutMappingRequest>>() {}).toInstance(mappingRequestValidators);
         bind(AutoCreateIndex.class).toInstance(autoCreateIndex);
         bind(TransportLivenessAction.class).asEagerSingleton();
 

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

@@ -0,0 +1,67 @@
+/*
+ * 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;
+import org.elasticsearch.index.Index;
+
+import java.util.Collection;
+import java.util.Optional;
+
+public class RequestValidators<T extends ActionRequest> {
+
+        private final Collection<RequestValidator<T>> validators;
+
+        public RequestValidators(Collection<RequestValidator<T>> validators) {
+            this.validators = validators;
+        }
+
+        public Optional<Exception> validateRequest(final T request, final ClusterState state, final Index[] indices) {
+            Exception exception = null;
+            for (final var validator : validators) {
+                final Optional<Exception> 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<T extends ActionRequest> {
+
+        /**
+         * 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<Exception> validateRequest(T request, ClusterState state, Index[] indices);
+
+    }
+
+}

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,25 +38,30 @@
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
-import java.util.Collection;
+import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Put mapping action.
  */
 public class TransportPutMappingAction extends TransportMasterNodeAction<PutMappingRequest, AcknowledgedResponse> {
 
     private final MetaDataMappingService metaDataMappingService;
-    private final RequestValidators requestValidators;
+    private final RequestValidators<PutMappingRequest> 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<PutMappingRequest> requestValidators) {
         super(PutMappingAction.NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver,
             PutMappingRequest::new);
         this.metaDataMappingService = metaDataMappingService;
-        this.requestValidators = requestValidators;
+        this.requestValidators = Objects.requireNonNull(requestValidators);
     }
 
     @Override
@@ -87,9 +93,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<Exception> maybeValidationException = requestValidators.validateRequest(request, state, concreteIndices);
+            if (maybeValidationException.isPresent()) {
+                listener.onFailure(maybeValidationException.get());
                 return;
             }
             PutMappingClusterStateUpdateRequest updateRequest = new PutMappingClusterStateUpdateRequest()
@@ -118,26 +124,4 @@ public void onFailure(Exception t) {
         }
     }
 
-
-    public static class RequestValidators {
-        private final Collection<MappingRequestValidator> validators;
-
-        public RequestValidators(Collection<MappingRequestValidator> 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;
-        }
-    }
 }

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<MappingRequestValidator> mappingRequestValidators() {
+    default Collection<RequestValidators.RequestValidator<PutMappingRequest>> mappingRequestValidators() {
         return Collections.emptyList();
     }
+
 }

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<PutMappingRequest> EMPTY = (request, state, indices) -> Optional.empty();
+    private final RequestValidators.RequestValidator<PutMappingRequest> FAIL =
+            (request, state, indices) -> Optional.of(new Exception("failure"));
+
+    public void testValidates() {
+        final int numberOfValidations = randomIntBetween(0, 8);
+        final List<RequestValidators.RequestValidator<PutMappingRequest>> validators = new ArrayList<>(numberOfValidations);
+        for (int i = 0; i < numberOfValidations; i++) {
+            validators.add(EMPTY);
+        }
+        final RequestValidators<PutMappingRequest> requestValidators = new RequestValidators<>(validators);
+        assertThat(requestValidators.validateRequest(null, null, null), OptionalMatchers.isEmpty());
+    }
+
+    public void testFailure() {
+        final RequestValidators<PutMappingRequest> validators = new RequestValidators<>(List.of(FAIL));
+        assertThat(validators.validateRequest(null, null, null), OptionalMatchers.isPresent());
+    }
+
+    public void testValidatesAfterFailure() {
+        final RequestValidators<PutMappingRequest> 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<RequestValidators.RequestValidator<PutMappingRequest>> validators = new ArrayList<>(numberOfFailures);
+        for (int i = 0; i < numberOfFailures; i++) {
+            validators.add(FAIL);
+        }
+        final RequestValidators<PutMappingRequest> requestValidators = new RequestValidators<>(validators);
+        final Optional<Exception> 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<RequestValidators.RequestValidator<PutMappingRequest>> 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<PutMappingRequest> requestValidators = new RequestValidators<>(validators);
+        final Optional<Exception> 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));
+        }
+    }
+
+}