Skip to content

Commit 72d7b13

Browse files
pgomulkapgomulka
authored
Merge pull request elastic#21 from jakelandis/jake-xpack-spi-request
Some revisions to the idea of using a function
2 parents 88598dc + 3470081 commit 72d7b13

File tree

13 files changed

+141
-200
lines changed

13 files changed

+141
-200
lines changed

‎libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java

+3-35
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626

2727
import java.util.Locale;
2828
import java.util.Objects;
29-
import java.util.regex.Matcher;
30-
import java.util.regex.Pattern;
3129

3230
/**
3331
* The content type of {@link org.elasticsearch.common.xcontent.XContent}.
@@ -116,19 +114,13 @@ public XContent xContent() {
116114
}
117115
};
118116

119-
private static final Pattern COMPATIBLE_API_HEADER_PATTERN = Pattern.compile(
120-
"(application|text)/(vnd.elasticsearch\\+)?([^;]+)(\\s*;\\s*compatible-with=(\\d+))?",
121-
Pattern.CASE_INSENSITIVE);
122-
123117
/**
124118
* Accepts either a format string, which is equivalent to {@link XContentType#shortName()} or a media type that optionally has
125119
* parameters and attempts to match the value to an {@link XContentType}. The comparisons are done in lower case format and this method
126120
* also supports a wildcard accept for {@code application/*}. This method can be used to parse the {@code Accept} HTTP header or a
127121
* format query string parameter. This method will return {@code null} if no match is found
128122
*/
129-
public static XContentType fromMediaTypeOrFormat(String mediaTypeHeaderValue) {
130-
String mediaType = parseMediaType(mediaTypeHeaderValue);
131-
123+
public static XContentType fromMediaTypeOrFormat(String mediaType) {
132124
if (mediaType == null) {
133125
return null;
134126
}
@@ -138,7 +130,7 @@ public static XContentType fromMediaTypeOrFormat(String mediaTypeHeaderValue) {
138130
}
139131
}
140132
final String lowercaseMediaType = mediaType.toLowerCase(Locale.ROOT);
141-
if (lowercaseMediaType.startsWith("application/*") || lowercaseMediaType.equals("*/*")) {
133+
if (lowercaseMediaType.startsWith("application/*")) {
142134
return JSON;
143135
}
144136

@@ -150,9 +142,7 @@ public static XContentType fromMediaTypeOrFormat(String mediaTypeHeaderValue) {
150142
* The provided media type should not include any parameters. This method is suitable for parsing part of the {@code Content-Type}
151143
* HTTP header. This method will return {@code null} if no match is found
152144
*/
153-
public static XContentType fromMediaType(String mediaTypeHeaderValue) {
154-
String mediaType = parseMediaType(mediaTypeHeaderValue);
155-
145+
public static XContentType fromMediaType(String mediaType) {
156146
final String lowercaseMediaType = Objects.requireNonNull(mediaType, "mediaType cannot be null").toLowerCase(Locale.ROOT);
157147
for (XContentType type : values()) {
158148
if (type.mediaTypeWithoutParameters().equals(lowercaseMediaType)) {
@@ -167,28 +157,6 @@ public static XContentType fromMediaType(String mediaTypeHeaderValue) {
167157
return null;
168158
}
169159

170-
//public scope needed for text formats hack
171-
public static String parseMediaType(String mediaType) {
172-
if (mediaType != null) {
173-
Matcher matcher = COMPATIBLE_API_HEADER_PATTERN.matcher(mediaType);
174-
if (matcher.find()) {
175-
return (matcher.group(1) + "/" + matcher.group(3)).toLowerCase(Locale.ROOT);
176-
}
177-
}
178-
179-
return mediaType;
180-
}
181-
182-
public static String parseVersion(String mediaType){
183-
if(mediaType != null){
184-
Matcher matcher = COMPATIBLE_API_HEADER_PATTERN.matcher(mediaType);
185-
if (matcher.find() && "vnd.elasticsearch+".equalsIgnoreCase(matcher.group(2))) {
186-
187-
return matcher.group(5);
188-
}
189-
}
190-
return null;
191-
}
192160
private static boolean isSameMediaTypeOrFormatAs(String stringType, XContentType type) {
193161
return type.mediaTypeWithoutParameters().equalsIgnoreCase(stringType) ||
194162
stringType.toLowerCase(Locale.ROOT).startsWith(type.mediaTypeWithoutParameters().toLowerCase(Locale.ROOT) + ";") ||

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

+2-14
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,6 @@
258258
import org.elasticsearch.persistent.UpdatePersistentTaskStatusAction;
259259
import org.elasticsearch.plugins.ActionPlugin;
260260
import org.elasticsearch.plugins.ActionPlugin.ActionHandler;
261-
import org.elasticsearch.plugins.RestCompatibilityPlugin;
262261
import org.elasticsearch.rest.RestController;
263262
import org.elasticsearch.rest.RestHandler;
264263
import org.elasticsearch.rest.RestHeaderDefinition;
@@ -428,7 +427,7 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr
428427
IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter,
429428
ThreadPool threadPool, List<ActionPlugin> actionPlugins, NodeClient nodeClient,
430429
CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService,
431-
List<RestCompatibilityPlugin> restCompatPlugins) {
430+
BiFunction<String, String, Version> restCompatibleFunction) {
432431
this.settings = settings;
433432
this.indexNameExpressionResolver = indexNameExpressionResolver;
434433
this.indexScopedSettings = indexScopedSettings;
@@ -460,18 +459,7 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr
460459
indicesAliasesRequestRequestValidators = new RequestValidators<>(
461460
actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).collect(Collectors.toList()));
462461

463-
BiFunction<Map<String, List<String>>, Boolean, Boolean> minimumRestCompatibilityVersion = getMinimumRestCompatibilityVersion(restCompatPlugins);
464-
restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, minimumRestCompatibilityVersion);
465-
}
466-
467-
private BiFunction<Map<String, List<String>>, Boolean, Boolean> getMinimumRestCompatibilityVersion(List<RestCompatibilityPlugin> restCompatPlugins) {
468-
if (restCompatPlugins.size() > 1) {
469-
throw new IllegalStateException("Only one rest compatibility plugin is allowed");
470-
}
471-
return (headers, hasContent) -> restCompatPlugins.stream()
472-
.findFirst()
473-
.orElse((a, b) -> false)
474-
.isRequestingCompatibility(headers, hasContent);
462+
restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, restCompatibleFunction);
475463
}
476464

477465
public Map<String, ActionHandler<?, ?>> getActions() {

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

+18-2
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@
141141
import org.elasticsearch.plugins.Plugin;
142142
import org.elasticsearch.plugins.PluginsService;
143143
import org.elasticsearch.plugins.RepositoryPlugin;
144-
import org.elasticsearch.plugins.RestCompatibilityPlugin;
144+
import org.elasticsearch.plugins.RestCompatibility;
145145
import org.elasticsearch.plugins.ScriptPlugin;
146146
import org.elasticsearch.plugins.SearchPlugin;
147147
import org.elasticsearch.plugins.SystemIndexPlugin;
@@ -190,6 +190,7 @@
190190
import java.util.Set;
191191
import java.util.concurrent.CountDownLatch;
192192
import java.util.concurrent.TimeUnit;
193+
import java.util.function.BiFunction;
193194
import java.util.function.Function;
194195
import java.util.function.UnaryOperator;
195196
import java.util.stream.Collectors;
@@ -513,7 +514,7 @@ protected Node(final Environment initialEnvironment,
513514
ActionModule actionModule = new ActionModule(settings, clusterModule.getIndexNameExpressionResolver(),
514515
settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
515516
threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, clusterService,
516-
pluginsService.filterPlugins(RestCompatibilityPlugin.class));
517+
getRestCompatibleFunction());
517518
modules.add(actionModule);
518519

519520
final RestController restController = actionModule.getRestController();
@@ -682,6 +683,21 @@ protected Node(final Environment initialEnvironment,
682683
}
683684
}
684685

686+
/**
687+
* @return A function that can be used to determine the requested REST compatible version
688+
*/
689+
private BiFunction<String, String, Version> getRestCompatibleFunction(){
690+
List<RestCompatibility> restCompatibilityPlugins = pluginsService.filterPlugins(RestCompatibility.class);
691+
BiFunction<String, String, Version> restCompatibleFunction = (a, b) -> Version.CURRENT;
692+
if (restCompatibilityPlugins.size() > 1) {
693+
throw new IllegalStateException("Only one rest compatibility plugin is allowed");
694+
} else if (restCompatibilityPlugins.size() == 1){
695+
restCompatibleFunction =
696+
(acceptHeader, contentTypeHeader) -> restCompatibilityPlugins.get(0).getCompatibleVersion(acceptHeader, contentTypeHeader);
697+
}
698+
return restCompatibleFunction;
699+
}
700+
685701
protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool,
686702
TransportInterceptor interceptor,
687703
Function<BoundTransportAddress, DiscoveryNode> localNodeFactory,

‎server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java renamed to ‎server/src/main/java/org/elasticsearch/plugins/RestCompatibility.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
package org.elasticsearch.plugins;
2121

2222
import org.elasticsearch.Version;
23+
import org.elasticsearch.common.Nullable;
2324

2425
import java.util.List;
2526
import java.util.Map;
2627

27-
public interface RestCompatibilityPlugin {
28-
boolean isRequestingCompatibility(Map<String, List<String>> headers, boolean hasContent);
28+
@FunctionalInterface
29+
public interface RestCompatibility {
30+
Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader);
2931
}

‎server/src/main/java/org/elasticsearch/rest/RestController.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,15 @@ public class RestController implements HttpServerTransport.Dispatcher {
7777
/** Rest headers that are copied to internal requests made during a rest request. */
7878
private final Set<RestHeaderDefinition> headersToCopy;
7979
private final UsageService usageService;
80-
private BiFunction<Map<String,List<String>>,Boolean,Boolean> isRestCompatibleFunction;
80+
private BiFunction<String, String, Version> restCompatibleFunction;
81+
8182

8283
public RestController(Set<RestHeaderDefinition> headersToCopy, UnaryOperator<RestHandler> handlerWrapper,
8384
NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService,
84-
BiFunction<Map<String, List<String>>, Boolean, Boolean> isRestCompatibleFunction) {
85+
BiFunction<String, String, Version> restCompatibleFunction) {
8586
this.headersToCopy = headersToCopy;
8687
this.usageService = usageService;
87-
this.isRestCompatibleFunction = isRestCompatibleFunction;
88+
this.restCompatibleFunction = restCompatibleFunction;
8889
if (handlerWrapper == null) {
8990
handlerWrapper = h -> h; // passthrough if no wrapper set
9091
}
@@ -300,8 +301,8 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel
300301
final String rawPath = request.rawPath();
301302
final String uri = request.uri();
302303
final RestRequest.Method requestMethod;
303-
//once we have a version then we can find a handler registered for path, method and version
304-
Version version = request.getCompatibleApiVersion(isRestCompatibleFunction);
304+
//TODO: now that we have a version we can implement a REST handler that accepts path, method AND version
305+
//Version version = request.getRequestedCompatibility(restCompatibleFunction);
305306
try {
306307
// Resolves the HTTP method and fails if the method is invalid
307308
requestMethod = request.method();

‎server/src/main/java/org/elasticsearch/rest/RestRequest.java

+9-53
Original file line numberDiff line numberDiff line change
@@ -175,64 +175,19 @@ public static RestRequest requestWithoutParameters(NamedXContentRegistry xConten
175175
requestIdGenerator.incrementAndGet());
176176
}
177177

178-
/**
179-
* An http request can be accompanied with a compatible version indicating with what version a client is using.
180-
* Only a major Versions are supported. Internally we use Versions objects, but only use Version(major,0,0)
181-
* @return a version with what a client is compatible with.
182-
*/
183-
public Version getCompatibleApiVersion(BiFunction<Map<String,List<String>>,Boolean,Boolean> isRequestingCompatibilityFunction) {
184-
if (/*headersValidation &&*/ isRequestingCompatibilityFunction.apply(getHeaders(),hasContent())) {
185-
return Version.fromString(Version.CURRENT.major-1+".0.0");
186-
} else {
187-
return Version.CURRENT;
188-
}
178+
public Version getRequestedCompatibility(BiFunction<String, String, Version> restCompatibleFunction) {
179+
return restCompatibleFunction.apply(getSingleHeader("Accept"), getSingleHeader("Content-Type"));
189180
}
190181

191-
192-
private boolean isRequestingCompatibility() {
193-
/* String acceptHeader = header(CompatibleConstants.COMPATIBLE_ACCEPT_HEADER);
194-
String aVersion = XContentType.parseVersion(acceptHeader);
195-
byte acceptVersion = aVersion == null ? Version.CURRENT.major : Integer.valueOf(aVersion).byteValue();
196-
String contentTypeHeader = header(CompatibleConstants.COMPATIBLE_CONTENT_TYPE_HEADER);
197-
String cVersion = XContentType.parseVersion(contentTypeHeader);
198-
byte contentTypeVersion = cVersion == null ? Version.CURRENT.major : Integer.valueOf(cVersion).byteValue();
199-
200-
if(Version.CURRENT.major < acceptVersion || Version.CURRENT.major - acceptVersion > 1 ){
201-
throw new CompatibleApiHeadersCombinationException(
202-
String.format(Locale.ROOT, "Unsupported version provided. " +
203-
"Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader,
204-
contentTypeHeader, hasContent(), path(), params.toString(), method().toString()));
205-
}
206-
if (hasContent()) {
207-
if(Version.CURRENT.major < contentTypeVersion || Version.CURRENT.major - contentTypeVersion > 1 ){
208-
throw new CompatibleApiHeadersCombinationException(
209-
String.format(Locale.ROOT, "Unsupported version provided. " +
210-
"Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader,
211-
contentTypeHeader, hasContent(), path(), params.toString(), method().toString()));
212-
}
213-
214-
if (contentTypeVersion != acceptVersion) {
215-
throw new CompatibleApiHeadersCombinationException(
216-
String.format(Locale.ROOT, "Content-Type and Accept headers have to match when content is present. " +
217-
"Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader,
218-
contentTypeHeader, hasContent(), path(), params.toString(), method().toString()));
219-
}
220-
// both headers should be versioned or none
221-
if ((cVersion == null && aVersion!=null) || (aVersion ==null && cVersion!=null) ){
222-
throw new CompatibleApiHeadersCombinationException(
223-
String.format(Locale.ROOT, "Versioning is required on both Content-Type and Accept headers. " +
224-
"Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader,
225-
contentTypeHeader, hasContent(), path(), params.toString(), method().toString()));
226-
}
227-
228-
return contentTypeVersion < Version.CURRENT.major;
182+
private final String getSingleHeader(String name) {
183+
//TODO: is this case sensitive ?
184+
List<String> values = headers.get(name);
185+
if (values != null && values.isEmpty() == false) {
186+
return values.get(0);
229187
}
230-
231-
return acceptVersion < Version.CURRENT.major;*/
232-
return true;
188+
return null;
233189
}
234190

235-
236191
public enum Method {
237192
GET, POST, PUT, DELETE, OPTIONS, HEAD, PATCH, TRACE, CONNECT
238193
}
@@ -597,4 +552,5 @@ public static class BadParameterException extends RuntimeException {
597552
}
598553

599554
}
555+
600556
}

0 commit comments

Comments
 (0)