Skip to content

Commit dded863

Browse files
jasontedorjasontedor
authored
Refactor put mapping request validation for reuse (#43005)
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.
1 parent 16e6f5d commit dded863

File tree

13 files changed

+312
-198
lines changed

13 files changed

+312
-198
lines changed

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

+6-5
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
import org.elasticsearch.action.admin.indices.mapping.get.TransportGetFieldMappingsIndexAction;
119119
import org.elasticsearch.action.admin.indices.mapping.get.TransportGetMappingsAction;
120120
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction;
121+
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
121122
import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction;
122123
import org.elasticsearch.action.admin.indices.open.OpenIndexAction;
123124
import org.elasticsearch.action.admin.indices.open.TransportOpenIndexAction;
@@ -204,6 +205,7 @@
204205
import org.elasticsearch.cluster.node.DiscoveryNodes;
205206
import org.elasticsearch.common.NamedRegistry;
206207
import org.elasticsearch.common.inject.AbstractModule;
208+
import org.elasticsearch.common.inject.TypeLiteral;
207209
import org.elasticsearch.common.inject.multibindings.MapBinder;
208210
import org.elasticsearch.common.settings.ClusterSettings;
209211
import org.elasticsearch.common.settings.IndexScopedSettings;
@@ -358,7 +360,7 @@ public class ActionModule extends AbstractModule {
358360
private final AutoCreateIndex autoCreateIndex;
359361
private final DestructiveOperations destructiveOperations;
360362
private final RestController restController;
361-
private final TransportPutMappingAction.RequestValidators mappingRequestValidators;
363+
private final RequestValidators<PutMappingRequest> mappingRequestValidators;
362364

363365
public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpressionResolver,
364366
IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter,
@@ -389,9 +391,8 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr
389391
restWrapper = newRestWrapper;
390392
}
391393
}
392-
mappingRequestValidators = new TransportPutMappingAction.RequestValidators(
393-
actionPlugins.stream().flatMap(p -> p.mappingRequestValidators().stream()).collect(Collectors.toList())
394-
);
394+
mappingRequestValidators = new RequestValidators<>(
395+
actionPlugins.stream().flatMap(p -> p.mappingRequestValidators().stream()).collect(Collectors.toList()));
395396

396397
restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService);
397398
}
@@ -684,7 +685,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
684685
protected void configure() {
685686
bind(ActionFilters.class).toInstance(actionFilters);
686687
bind(DestructiveOperations.class).toInstance(destructiveOperations);
687-
bind(TransportPutMappingAction.RequestValidators.class).toInstance(mappingRequestValidators);
688+
bind(new TypeLiteral<RequestValidators<PutMappingRequest>>() {}).toInstance(mappingRequestValidators);
688689
bind(AutoCreateIndex.class).toInstance(autoCreateIndex);
689690
bind(TransportLivenessAction.class).asEagerSingleton();
690691

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

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.action;
21+
22+
import org.elasticsearch.cluster.ClusterState;
23+
import org.elasticsearch.index.Index;
24+
25+
import java.util.Collection;
26+
import java.util.Optional;
27+
28+
public class RequestValidators<T extends ActionRequest> {
29+
30+
private final Collection<RequestValidator<T>> validators;
31+
32+
public RequestValidators(Collection<RequestValidator<T>> validators) {
33+
this.validators = validators;
34+
}
35+
36+
public Optional<Exception> validateRequest(final T request, final ClusterState state, final Index[] indices) {
37+
Exception exception = null;
38+
for (final var validator : validators) {
39+
final Optional<Exception> maybeException = validator.validateRequest(request, state, indices);
40+
if (maybeException.isEmpty()) continue;
41+
if (exception == null) {
42+
exception = maybeException.get();
43+
} else {
44+
exception.addSuppressed(maybeException.get());
45+
}
46+
}
47+
return Optional.ofNullable(exception);
48+
}
49+
50+
/**
51+
* A validator that validates an request associated with indices before executing it.
52+
*/
53+
public interface RequestValidator<T extends ActionRequest> {
54+
55+
/**
56+
* Validates a given request with its associated concrete indices and the current state.
57+
*
58+
* @param request the request to validate
59+
* @param state the current cluster state
60+
* @param indices the concrete indices that associated with the given request
61+
* @return an optional exception indicates a reason that the given request should be aborted, otherwise empty
62+
*/
63+
Optional<Exception> validateRequest(T request, ClusterState state, Index[] indices);
64+
65+
}
66+
67+
}

‎server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/MappingRequestValidator.java

-40
This file was deleted.

‎server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java

+16-32
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.apache.logging.log4j.message.ParameterizedMessage;
2323
import org.elasticsearch.action.ActionListener;
24+
import org.elasticsearch.action.RequestValidators;
2425
import org.elasticsearch.action.support.ActionFilters;
2526
import org.elasticsearch.action.support.master.AcknowledgedResponse;
2627
import org.elasticsearch.action.support.master.TransportMasterNodeAction;
@@ -37,25 +38,30 @@
3738
import org.elasticsearch.threadpool.ThreadPool;
3839
import org.elasticsearch.transport.TransportService;
3940

40-
import java.util.Collection;
41+
import java.util.Objects;
42+
import java.util.Optional;
4143

4244
/**
4345
* Put mapping action.
4446
*/
4547
public class TransportPutMappingAction extends TransportMasterNodeAction<PutMappingRequest, AcknowledgedResponse> {
4648

4749
private final MetaDataMappingService metaDataMappingService;
48-
private final RequestValidators requestValidators;
50+
private final RequestValidators<PutMappingRequest> requestValidators;
4951

5052
@Inject
51-
public TransportPutMappingAction(TransportService transportService, ClusterService clusterService,
52-
ThreadPool threadPool, MetaDataMappingService metaDataMappingService,
53-
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver,
54-
RequestValidators requestValidators) {
53+
public TransportPutMappingAction(
54+
final TransportService transportService,
55+
final ClusterService clusterService,
56+
final ThreadPool threadPool,
57+
final MetaDataMappingService metaDataMappingService,
58+
final ActionFilters actionFilters,
59+
final IndexNameExpressionResolver indexNameExpressionResolver,
60+
final RequestValidators<PutMappingRequest> requestValidators) {
5561
super(PutMappingAction.NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver,
5662
PutMappingRequest::new);
5763
this.metaDataMappingService = metaDataMappingService;
58-
this.requestValidators = requestValidators;
64+
this.requestValidators = Objects.requireNonNull(requestValidators);
5965
}
6066

6167
@Override
@@ -87,9 +93,9 @@ protected void masterOperation(final PutMappingRequest request, final ClusterSta
8793
final Index[] concreteIndices = request.getConcreteIndex() == null ?
8894
indexNameExpressionResolver.concreteIndices(state, request)
8995
: new Index[] {request.getConcreteIndex()};
90-
final Exception validationException = requestValidators.validateRequest(request, state, concreteIndices);
91-
if (validationException != null) {
92-
listener.onFailure(validationException);
96+
final Optional<Exception> maybeValidationException = requestValidators.validateRequest(request, state, concreteIndices);
97+
if (maybeValidationException.isPresent()) {
98+
listener.onFailure(maybeValidationException.get());
9399
return;
94100
}
95101
PutMappingClusterStateUpdateRequest updateRequest = new PutMappingClusterStateUpdateRequest()
@@ -118,26 +124,4 @@ public void onFailure(Exception t) {
118124
}
119125
}
120126

121-
122-
public static class RequestValidators {
123-
private final Collection<MappingRequestValidator> validators;
124-
125-
public RequestValidators(Collection<MappingRequestValidator> validators) {
126-
this.validators = validators;
127-
}
128-
129-
Exception validateRequest(PutMappingRequest request, ClusterState state, Index[] indices) {
130-
Exception firstException = null;
131-
for (MappingRequestValidator validator : validators) {
132-
final Exception e = validator.validateRequest(request, state, indices);
133-
if (e == null) continue;
134-
if (firstException == null) {
135-
firstException = e;
136-
} else {
137-
firstException.addSuppressed(e);
138-
}
139-
}
140-
return firstException;
141-
}
142-
}
143127
}

‎server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
import org.elasticsearch.action.Action;
2323
import org.elasticsearch.action.ActionRequest;
2424
import org.elasticsearch.action.ActionResponse;
25-
import org.elasticsearch.action.admin.indices.mapping.put.MappingRequestValidator;
26-
import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction;
25+
import org.elasticsearch.action.RequestValidators;
26+
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
2727
import org.elasticsearch.action.support.ActionFilter;
2828
import org.elasticsearch.action.support.TransportAction;
2929
import org.elasticsearch.action.support.TransportActions;
@@ -183,10 +183,11 @@ public int hashCode() {
183183
}
184184

185185
/**
186-
* Returns a collection of validators that are used by {@link TransportPutMappingAction.RequestValidators} to
187-
* validate a {@link org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest} before the executing it.
186+
* Returns a collection of validators that are used by {@link RequestValidators} to validate a
187+
* {@link org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest} before the executing it.
188188
*/
189-
default Collection<MappingRequestValidator> mappingRequestValidators() {
189+
default Collection<RequestValidators.RequestValidator<PutMappingRequest>> mappingRequestValidators() {
190190
return Collections.emptyList();
191191
}
192+
192193
}

‎server/src/test/java/org/elasticsearch/action/RequestValidatorsTests.java

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.action;
21+
22+
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
23+
import org.elasticsearch.common.Randomness;
24+
import org.elasticsearch.test.ESTestCase;
25+
import org.elasticsearch.test.hamcrest.OptionalMatchers;
26+
import org.hamcrest.Matchers;
27+
28+
import java.util.ArrayList;
29+
import java.util.List;
30+
import java.util.Optional;
31+
32+
public class RequestValidatorsTests extends ESTestCase {
33+
34+
private final RequestValidators.RequestValidator<PutMappingRequest> EMPTY = (request, state, indices) -> Optional.empty();
35+
private final RequestValidators.RequestValidator<PutMappingRequest> FAIL =
36+
(request, state, indices) -> Optional.of(new Exception("failure"));
37+
38+
public void testValidates() {
39+
final int numberOfValidations = randomIntBetween(0, 8);
40+
final List<RequestValidators.RequestValidator<PutMappingRequest>> validators = new ArrayList<>(numberOfValidations);
41+
for (int i = 0; i < numberOfValidations; i++) {
42+
validators.add(EMPTY);
43+
}
44+
final RequestValidators<PutMappingRequest> requestValidators = new RequestValidators<>(validators);
45+
assertThat(requestValidators.validateRequest(null, null, null), OptionalMatchers.isEmpty());
46+
}
47+
48+
public void testFailure() {
49+
final RequestValidators<PutMappingRequest> validators = new RequestValidators<>(List.of(FAIL));
50+
assertThat(validators.validateRequest(null, null, null), OptionalMatchers.isPresent());
51+
}
52+
53+
public void testValidatesAfterFailure() {
54+
final RequestValidators<PutMappingRequest> validators = new RequestValidators<>(List.of(FAIL, EMPTY));
55+
assertThat(validators.validateRequest(null, null, null), OptionalMatchers.isPresent());
56+
}
57+
58+
public void testMultipleFailures() {
59+
final int numberOfFailures = randomIntBetween(2, 8);
60+
final List<RequestValidators.RequestValidator<PutMappingRequest>> validators = new ArrayList<>(numberOfFailures);
61+
for (int i = 0; i < numberOfFailures; i++) {
62+
validators.add(FAIL);
63+
}
64+
final RequestValidators<PutMappingRequest> requestValidators = new RequestValidators<>(validators);
65+
final Optional<Exception> e = requestValidators.validateRequest(null, null, null);
66+
assertThat(e, OptionalMatchers.isPresent());
67+
// noinspection OptionalGetWithoutIsPresent
68+
assertThat(e.get().getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
69+
}
70+
71+
public void testRandom() {
72+
final int numberOfValidations = randomIntBetween(0, 8);
73+
final int numberOfFailures = randomIntBetween(0, 8);
74+
final List<RequestValidators.RequestValidator<PutMappingRequest>> validators =
75+
new ArrayList<>(numberOfValidations + numberOfFailures);
76+
for (int i = 0; i < numberOfValidations; i++) {
77+
validators.add(EMPTY);
78+
}
79+
for (int i = 0; i < numberOfFailures; i++) {
80+
validators.add(FAIL);
81+
}
82+
Randomness.shuffle(validators);
83+
final RequestValidators<PutMappingRequest> requestValidators = new RequestValidators<>(validators);
84+
final Optional<Exception> e = requestValidators.validateRequest(null, null, null);
85+
if (numberOfFailures == 0) {
86+
assertThat(e, OptionalMatchers.isEmpty());
87+
} else {
88+
assertThat(e, OptionalMatchers.isPresent());
89+
// noinspection OptionalGetWithoutIsPresent
90+
assertThat(e.get().getSuppressed(), Matchers.arrayWithSize(numberOfFailures - 1));
91+
}
92+
}
93+
94+
}

0 commit comments

Comments
 (0)