Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
e468fdf
Introduce per endpoint media types
pgomulka Oct 30, 2020
5cfa11e
Merge branch 'master' into compat/parsed_media_type
pgomulka Oct 30, 2020
66224e7
allow smile and cbor to parse charset param. See ClientYamlTestExecut…
pgomulka Oct 30, 2020
29b27e8
javadocs
pgomulka Oct 30, 2020
4690362
fix javadoc
pgomulka Oct 30, 2020
e7866a9
Allow registration of compatible version-1 handlers
pgomulka Oct 30, 2020
0330d97
add tests
pgomulka Nov 2, 2020
d3cb858
Introduce CompatibleVersion plugin
pgomulka Nov 2, 2020
78d4660
Merge branch 'master' into compat/register_compatible_handlers
pgomulka Nov 2, 2020
bc5de8b
pass version to xcontentbuilder
pgomulka Nov 2, 2020
f958cdd
Merge branch 'master' into compat/register_compatible_handlers
pgomulka Nov 2, 2020
e829b17
Merge branch 'master' into compat/compatible_plugin
pgomulka Nov 2, 2020
f32cf70
spotless
pgomulka Nov 2, 2020
2bda74f
code review follow up
pgomulka Nov 3, 2020
7779083
minor tweaks
pgomulka Nov 3, 2020
dd26408
Merge branch 'master' into compat/introduce_per_endpoint_media_types
pgomulka Nov 3, 2020
94b4a0a
fix test after exception msg rename
pgomulka Nov 3, 2020
4385591
rename to header value
pgomulka Nov 3, 2020
dc61731
remove charset validation
pgomulka Nov 4, 2020
4a3138d
Apply suggestions from code review
pgomulka Nov 5, 2020
0b565e5
javadoc
pgomulka Nov 5, 2020
b35ad2c
Merge branch 'master' into compat/introduce_per_endpoint_media_types
pgomulka Nov 5, 2020
b908bfe
Merge branch 'compat/introduce_per_endpoint_media_types' of github.co…
pgomulka Nov 5, 2020
950ba7b
Merge branch 'compat/introduce_per_endpoint_media_types' into compat/…
pgomulka Nov 5, 2020
310b735
Merge branch 'master' into compat/register_compatible_handlers
pgomulka Nov 5, 2020
3cf408a
Merge branch 'compat/introduce_per_endpoint_media_types' into compat/…
pgomulka Nov 5, 2020
4c880c0
Merge branch 'master' into compat/compatible_plugin
pgomulka Nov 5, 2020
afba70d
javadoc and cleanup
pgomulka Nov 5, 2020
385f5a9
Merge branch 'master' into compat/register_compatible_handlers
pgomulka Nov 6, 2020
836fe0f
tests and javadoc
pgomulka Nov 6, 2020
8f2ea92
javadoc
pgomulka Nov 9, 2020
d70a071
do not set compatible version twice
pgomulka Nov 9, 2020
87d762f
Merge branch 'master' into compat/register_compatible_handlers
pgomulka Nov 9, 2020
deff739
javadocs
pgomulka Nov 9, 2020
ee40697
Merge branch 'compat/register_compatible_handlers' into compat/compat…
pgomulka Nov 9, 2020
a8eb296
Merge branch 'master' into compat/compatible_plugin
pgomulka Nov 16, 2020
63c2987
line
pgomulka Nov 16, 2020
aab691b
testcase for textformat versioned
pgomulka Nov 16, 2020
b2e12ec
spotless
pgomulka Nov 16, 2020
16a1b83
minimum rest compat
pgomulka Nov 17, 2020
b75d415
code review follow up
pgomulka Nov 18, 2020
c3b7b51
fix messages and toString for parsedmediatype
pgomulka Nov 19, 2020
51de734
assertion on message in test
pgomulka Nov 23, 2020
445032c
Merge branch 'master' into compat/compatible_plugin
elasticmachine Nov 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@
* @see MediaTypeRegistry
*/
public class ParsedMediaType {
private final String originalHeaderValue;
private final String type;
private final String subType;
private final Map<String, String> parameters;
// tchar pattern as defined by RFC7230 section 3.2.6
private static final Pattern TCHAR_PATTERN = Pattern.compile("[a-zA-z0-9!#$%&'*+\\-.\\^_`|~]+");

private ParsedMediaType(String type, String subType, Map<String, String> parameters) {
private ParsedMediaType(String originalHeaderValue, String type, String subType, Map<String, String> parameters) {
this.originalHeaderValue = originalHeaderValue;
this.type = type;
this.subType = subType;
this.parameters = Collections.unmodifiableMap(parameters);
Expand Down Expand Up @@ -79,7 +81,7 @@ public static ParsedMediaType parseMediaType(String headerValue) {
throw new IllegalArgumentException("invalid media-type [" + headerValue + "]");
}
if (elements.length == 1) {
return new ParsedMediaType(splitMediaType[0].trim(), splitMediaType[1].trim(), Collections.emptyMap());
return new ParsedMediaType(headerValue, splitMediaType[0].trim(), splitMediaType[1].trim(), Collections.emptyMap());
} else {
Map<String, String> parameters = new HashMap<>();
for (int i = 1; i < elements.length; i++) {
Expand All @@ -96,7 +98,7 @@ public static ParsedMediaType parseMediaType(String headerValue) {
String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT).trim();
parameters.put(parameterName, parameterValue);
}
return new ParsedMediaType(splitMediaType[0].trim().toLowerCase(Locale.ROOT),
return new ParsedMediaType(headerValue, splitMediaType[0].trim().toLowerCase(Locale.ROOT),
splitMediaType[1].trim().toLowerCase(Locale.ROOT), parameters);
}
}
Expand Down Expand Up @@ -144,4 +146,9 @@ private boolean isValidParameter(String paramName, String value, Map<String, Pat
//TODO undefined parameters are allowed until https://github.com/elastic/elasticsearch/issues/63080
return true;
}

@Override
public String toString() {
return originalHeaderValue;
}
}
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public boolean isCompatible(Version version) {
* Returns the minimum version that can be used for compatible REST API
*/
public Version minimumRestCompatibilityVersion() {
return Version.CURRENT.previousMajor();
return this.previousMajor();
}

/**
Expand Down
13 changes: 11 additions & 2 deletions server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.plugins.RepositoryPlugin;
import org.elasticsearch.plugins.RestCompatibilityPlugin;
import org.elasticsearch.plugins.ScriptPlugin;
import org.elasticsearch.plugins.SearchPlugin;
import org.elasticsearch.plugins.SystemIndexPlugin;
Expand Down Expand Up @@ -723,8 +724,16 @@ protected Node(final Environment initialEnvironment,
* package scope for testing
*/
CompatibleVersion getRestCompatibleFunction() {
// TODO PG Until compatible version plugin is implemented, return current version.
return CompatibleVersion.CURRENT_VERSION;
List<RestCompatibilityPlugin> restCompatibilityPlugins = pluginsService.filterPlugins(RestCompatibilityPlugin.class);
final CompatibleVersion compatibleVersion;
if (restCompatibilityPlugins.size() > 1) {
throw new IllegalStateException("Only one RestCompatibilityPlugin is allowed");
} else if (restCompatibilityPlugins.size() == 1) {
compatibleVersion = restCompatibilityPlugins.get(0)::getCompatibleVersion;
} else {
compatibleVersion = CompatibleVersion.CURRENT_VERSION;
}
return compatibleVersion;
}

protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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.plugins;

import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.xcontent.ParsedMediaType;


/**
* An extension point for Compatible API plugin implementation.
*/
public interface RestCompatibilityPlugin {
/**
* Returns a version which was requested on Accept and Content-Type headers
*
* @param acceptHeader - a ParsedMediaType parsed from Accept header
* @param contentTypeHeader - a ParsedMediaType parsed from Content-Type header
* @param hasContent - a flag indicating if a request has content
* @return a requested Compatible API Version
*/
Version getCompatibleVersion(@Nullable ParsedMediaType acceptHeader, @Nullable ParsedMediaType contentTypeHeader, boolean hasContent);
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
/** Rest headers that are copied to internal requests made during a rest request. */
private final Set<RestHeaderDefinition> headersToCopy;
private final UsageService usageService;
private CompatibleVersion compatibleVersion;
private final CompatibleVersion compatibleVersion;

public RestController(Set<RestHeaderDefinition> headersToCopy, UnaryOperator<RestHandler> handlerWrapper,
NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService,
Expand Down
56 changes: 56 additions & 0 deletions server/src/test/java/org/elasticsearch/node/NodeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@

import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.Version;
import org.elasticsearch.bootstrap.BootstrapCheck;
import org.elasticsearch.bootstrap.BootstrapContext;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.breaker.CircuitBreaker;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.transport.BoundTransportAddress;
import org.elasticsearch.common.xcontent.ParsedMediaType;
import org.elasticsearch.env.Environment;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.engine.Engine.Searcher;
Expand All @@ -36,6 +38,8 @@
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.plugins.CircuitBreakerPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.RestCompatibilityPlugin;
import org.elasticsearch.rest.CompatibleVersion;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.InternalTestCluster;
import org.elasticsearch.test.MockHttpTransport;
Expand Down Expand Up @@ -339,4 +343,56 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) {
myCircuitBreaker.set(circuitBreaker);
}
}

public static class TestRestCompatibility1 extends Plugin implements RestCompatibilityPlugin {
@Override
public Version getCompatibleVersion(ParsedMediaType acceptHeader, ParsedMediaType contentTypeHeader, boolean hasContent) {
return Version.CURRENT.previousMajor();
}
}

public static class TestRestCompatibility2 extends Plugin implements RestCompatibilityPlugin {
@Override
public Version getCompatibleVersion(ParsedMediaType acceptHeader, ParsedMediaType contentTypeHeader, boolean hasContent) {
return null;
}
}

public void testLoadingMultipleRestCompatibilityPlugins() throws IOException {
Settings.Builder settings = baseSettings();

// throw an exception when two plugins are registered
List<Class<? extends Plugin>> plugins = basePlugins();
plugins.add(TestRestCompatibility1.class);
plugins.add(TestRestCompatibility2.class);

IllegalStateException e = expectThrows(IllegalStateException.class, () -> new MockNode(settings.build(), plugins));
assertThat(e.getMessage(), equalTo("Only one RestCompatibilityPlugin is allowed"));
}

public void testCorrectUsageOfRestCompatibilityPlugin() throws IOException {
Settings.Builder settings = baseSettings();

// the correct usage expects one plugin
List<Class<? extends Plugin>> plugins = basePlugins();
plugins.add(TestRestCompatibility1.class);

try (Node node = new MockNode(settings.build(), plugins)) {
CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction();
assertThat(restCompatibleFunction.get(null, null, false), equalTo(Version.CURRENT.previousMajor()));
}
}


public void testDefaultingRestCompatibilityPlugin() throws IOException {
Settings.Builder settings = baseSettings();

// default to CompatibleVersion.CURRENT_VERSION when no plugins provided
List<Class<? extends Plugin>> plugins = basePlugins();

try (Node node = new MockNode(settings.build(), plugins)) {
CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction();
assertThat(restCompatibleFunction.get(null, null, false), equalTo(Version.CURRENT));
}
}
}
32 changes: 32 additions & 0 deletions x-pack/plugin/rest-compatibility/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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.
*/

apply plugin: 'elasticsearch.esplugin'

esplugin {
name 'rest-compatibility'
description 'A plugin for Compatible Rest API'
classname 'org.elasticsearch.compat.CompatibleVersionPlugin'
extendedPlugins = ['x-pack-core']
}

dependencies {
compileOnly project(path: xpackModule('core'), configuration: 'default')
testImplementation project(path: xpackModule('core'), configuration: 'testArtifacts')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.compat;

import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.xcontent.MediaType;
import org.elasticsearch.common.xcontent.ParsedMediaType;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.RestCompatibilityPlugin;
import org.elasticsearch.rest.RestStatus;

public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin {

@Override
public Version getCompatibleVersion(
@Nullable ParsedMediaType acceptHeader,
@Nullable ParsedMediaType contentTypeHeader,
boolean hasContent
) {
Byte aVersion = parseVersion(acceptHeader);
byte acceptVersion = aVersion == null ? Version.CURRENT.major : Integer.valueOf(aVersion).byteValue();
Byte cVersion = parseVersion(contentTypeHeader);
byte contentTypeVersion = cVersion == null ? Version.CURRENT.major : Integer.valueOf(cVersion).byteValue();

// accept version must be current or prior
if (acceptVersion > Version.CURRENT.major || acceptVersion < Version.CURRENT.minimumRestCompatibilityVersion().major) {
throw new ElasticsearchStatusException(
"Accept version must be either version {} or {}, but found {}. Accept={}",
RestStatus.BAD_REQUEST,
Version.CURRENT.major,
Version.CURRENT.minimumRestCompatibilityVersion().major,
acceptVersion,
acceptHeader
);
}
if (hasContent) {

// content-type version must be current or prior
if (contentTypeVersion > Version.CURRENT.major
|| contentTypeVersion < Version.CURRENT.minimumRestCompatibilityVersion().major) {
throw new ElasticsearchStatusException(
"Content-Type version must be either version {} or {}, but found {}. Content-Type={}",
RestStatus.BAD_REQUEST,
Version.CURRENT.major,
Version.CURRENT.minimumRestCompatibilityVersion().major,
contentTypeVersion,
contentTypeHeader
);
}
// if both accept and content-type are sent, the version must match
if (contentTypeVersion != acceptVersion) {
throw new ElasticsearchStatusException(
"A compatible version is required on both Content-Type and Accept headers "
+ "if either one has requested a compatible version "
+ "and the compatible versions must match. Accept={}, Content-Type={}",
RestStatus.BAD_REQUEST,
acceptHeader,
contentTypeHeader
);
}
// both headers should be versioned or none
if ((cVersion == null && aVersion != null) || (aVersion == null && cVersion != null)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this ever evaluate true ? wouldn't the above if catch any this case ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the above if if (contentTypeVersion != acceptVersion) { would not catch the scenario when one of the headers is set to compatible-with=8 (current in general) and the other is not set (defaults to current)

   expectThrows(
            ElasticsearchStatusException.class,
            () -> requestWith(acceptHeader(CURRENT_VERSION), contentTypeHeader(null), bodyPresent())
        );
expectThrows(
            ElasticsearchStatusException.class,
            () -> requestWith(acceptHeader(null), contentTypeHeader(CURRENT_VERSION), bodyPresent())
        );

throw new ElasticsearchStatusException(
"A compatible version is required on both Content-Type and Accept headers "
+ "if either one has requested a compatible version. Accept={}, Content-Type={}",
RestStatus.BAD_REQUEST,
acceptHeader,
contentTypeHeader
);
}
if (contentTypeVersion < Version.CURRENT.major) {
return Version.CURRENT.previousMajor();
}
}

if (acceptVersion < Version.CURRENT.major) {
return Version.CURRENT.previousMajor();
}

return Version.CURRENT;
}

// scope for testing
static Byte parseVersion(ParsedMediaType parsedMediaType) {
if (parsedMediaType != null) {
String version = parsedMediaType.getParameters().get(MediaType.COMPATIBLE_WITH_PARAMETER_NAME);
return version != null ? Byte.parseByte(version) : null;
}
return null;
}
}
Loading