-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Compatible version of typed Index And Get API #53228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a4f80ca
c99b818
c9c0e55
258542e
548fbd9
0dbea8a
4cf6bc1
3ac22b1
38721fe
f2db19f
b83b4ce
cf1d340
5c4a02d
0ef7e18
835ce56
f440231
84f1dde
d106d1b
cf61bbd
f00f438
ae1799f
a0fce89
7e55744
00fe62d
a6f0b9a
4eff534
2d4161c
6830f97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
esplugin { | ||
description 'Adds a compatiblity layer for the prior major versions REST API' | ||
classname 'org.elasticsearch.rest.compat.RestCompatPlugin' | ||
} | ||
|
||
integTest.enabled = false | ||
test.enabled = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/* | ||
* 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.rest.compat; | ||
|
||
import org.elasticsearch.Version; | ||
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; | ||
import org.elasticsearch.cluster.node.DiscoveryNodes; | ||
import org.elasticsearch.common.settings.ClusterSettings; | ||
import org.elasticsearch.common.settings.IndexScopedSettings; | ||
import org.elasticsearch.common.settings.Settings; | ||
import org.elasticsearch.common.settings.SettingsFilter; | ||
import org.elasticsearch.plugins.ActionPlugin; | ||
import org.elasticsearch.plugins.Plugin; | ||
import org.elasticsearch.rest.RestController; | ||
import org.elasticsearch.rest.RestHandler; | ||
import org.elasticsearch.rest.compat.version7.RestGetActionV7; | ||
import org.elasticsearch.rest.compat.version7.RestIndexActionV7; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.function.Supplier; | ||
|
||
public class RestCompatPlugin extends Plugin implements ActionPlugin { | ||
|
||
@Override | ||
public List<RestHandler> getRestHandlers( | ||
Settings settings, | ||
RestController restController, | ||
ClusterSettings clusterSettings, | ||
IndexScopedSettings indexScopedSettings, | ||
SettingsFilter settingsFilter, | ||
IndexNameExpressionResolver indexNameExpressionResolver, | ||
Supplier<DiscoveryNodes> nodesInCluster | ||
) { | ||
if (Version.CURRENT.major == 8) { | ||
return List.of( | ||
new RestGetActionV7(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't we never have more than one major versions worth of compat in a single version? If that is the case, why do we need to version the class names? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a narrow window where 2 versions can be present. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am +1 on using the version in the class for this reason, and for the obviousness they provide. While (arguably) ugly, these classes are special in that they are not intended to be maintained, rather they are intended to be deleted after a some point in time. Having the version in the name makes is more obvious and helps to prevent accidental modifications (like from a simple class search) and makes it really obvious if these things exist for too long. |
||
new RestIndexActionV7.CompatibleRestIndexAction(), | ||
new RestIndexActionV7.CompatibleCreateHandler(), | ||
new RestIndexActionV7.CompatibleAutoIdHandler(nodesInCluster) | ||
); | ||
} | ||
return Collections.emptyList(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/* | ||
* 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.rest.compat.version7; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.elasticsearch.Version; | ||
import org.elasticsearch.client.node.NodeClient; | ||
import org.elasticsearch.common.logging.DeprecationLogger; | ||
import org.elasticsearch.rest.RestRequest; | ||
import org.elasticsearch.rest.action.document.RestGetAction; | ||
|
||
import java.io.IOException; | ||
import java.util.List; | ||
|
||
import static org.elasticsearch.rest.RestRequest.Method.GET; | ||
import static org.elasticsearch.rest.RestRequest.Method.HEAD; | ||
|
||
public class RestGetActionV7 extends RestGetAction { | ||
|
||
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestGetAction.class)); | ||
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in " | ||
+ "document get requests is deprecated, use the /{index}/_doc/{id} endpoint instead."; | ||
|
||
@Override | ||
public List<Route> routes() { | ||
assert Version.CURRENT.major == 8 : "REST API compatibility for version 7 is only supported on version 8"; | ||
|
||
return List.of(new Route(GET, "/{index}/{type}/{id}"), new Route(HEAD, "/{index}/{type}/{id}")); | ||
} | ||
|
||
@Override | ||
public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { | ||
deprecationLogger.deprecatedAndMaybeLog("get_with_types", TYPES_DEPRECATION_MESSAGE); | ||
request.param("type"); | ||
return super.prepareRequest(request, client); | ||
} | ||
|
||
@Override | ||
public boolean compatibilityRequired() { | ||
return true; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
/* | ||
* 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.rest.compat.version7; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.elasticsearch.Version; | ||
import org.elasticsearch.client.node.NodeClient; | ||
import org.elasticsearch.cluster.node.DiscoveryNodes; | ||
import org.elasticsearch.common.logging.DeprecationLogger; | ||
import org.elasticsearch.rest.RestRequest; | ||
import org.elasticsearch.rest.action.document.RestIndexAction; | ||
|
||
import java.io.IOException; | ||
import java.util.List; | ||
import java.util.function.Supplier; | ||
|
||
import static java.util.Arrays.asList; | ||
import static java.util.Collections.singletonList; | ||
import static java.util.Collections.unmodifiableList; | ||
import static org.elasticsearch.rest.RestRequest.Method.POST; | ||
import static org.elasticsearch.rest.RestRequest.Method.PUT; | ||
|
||
public class RestIndexActionV7 { | ||
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in document " | ||
+ "index requests is deprecated, use the typeless endpoints instead (/{index}/_doc/{id}, /{index}/_doc, " | ||
+ "or /{index}/_create/{id})."; | ||
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestIndexAction.class)); | ||
|
||
private static void logDeprecationMessage() { | ||
deprecationLogger.deprecatedAndMaybeLog("index_with_types", TYPES_DEPRECATION_MESSAGE); | ||
} | ||
|
||
public static class CompatibleRestIndexAction extends RestIndexAction { | ||
@Override | ||
public List<Route> routes() { | ||
assert Version.CURRENT.major == 8 : "REST API compatilbity for version 7 is only supported on version 8"; | ||
|
||
return List.of(new Route(POST, "/{index}/{type}/{id}"), new Route(PUT, "/{index}/{type}/{id}")); | ||
} | ||
|
||
@Override | ||
public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { | ||
logDeprecationMessage(); | ||
request.param("type"); | ||
return super.prepareRequest(request, client); | ||
} | ||
|
||
@Override | ||
public boolean compatibilityRequired() { | ||
return true; | ||
} | ||
} | ||
|
||
public static class CompatibleCreateHandler extends RestIndexAction.CreateHandler { | ||
@Override | ||
public List<Route> routes() { | ||
return unmodifiableList( | ||
asList(new Route(POST, "/{index}/{type}/{id}/_create"), new Route(PUT, "/{index}/{type}/{id}/_create")) | ||
); | ||
} | ||
|
||
@Override | ||
public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { | ||
logDeprecationMessage(); | ||
request.param("type"); | ||
return super.prepareRequest(request, client); | ||
} | ||
|
||
@Override | ||
public boolean compatibilityRequired() { | ||
return true; | ||
} | ||
} | ||
|
||
public static final class CompatibleAutoIdHandler extends RestIndexAction.AutoIdHandler { | ||
|
||
public CompatibleAutoIdHandler(Supplier<DiscoveryNodes> nodesInCluster) { | ||
super(nodesInCluster); | ||
} | ||
|
||
@Override | ||
public List<Route> routes() { | ||
return singletonList(new Route(POST, "/{index}/{type}")); | ||
} | ||
|
||
@Override | ||
public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { | ||
logDeprecationMessage(); | ||
request.param("type"); | ||
return super.prepareRequest(request, client); | ||
} | ||
|
||
@Override | ||
public boolean compatibilityRequired() { | ||
return true; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see tests in this PR; why are they disabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they should be enabled. fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually there are no integTests, but there are unit tests so integTests.enabled = false tests.enabled=true