Skip to content

Commit

Permalink
Deprecate types in search and multi search templates. (#35669)
Browse files Browse the repository at this point in the history
This PR adds deprecation warnings to the relevant `Rest*Action` classes, plus tests in `Rest*ActionTests`. No updates to REST tests, the Java HLRC, or documentation were necessary, since they didn't make use of types.
  • Loading branch information
jtibshirani committed Nov 27, 2018
1 parent 7e772e3 commit 25c416b
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

package org.elasticsearch.script.mustache;

import org.apache.logging.log4j.LogManager;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
Expand All @@ -36,7 +38,8 @@
import static org.elasticsearch.rest.RestRequest.Method.POST;

public class RestMultiSearchTemplateAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
LogManager.getLogger(RestMultiSearchAction.class));
private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TYPED_KEYS_PARAM);

private final boolean allowExplicitIndex;
Expand Down Expand Up @@ -75,6 +78,9 @@ public static MultiSearchTemplateRequest parseRequest(RestRequest restRequest, b

RestMultiSearchAction.parseMultiLineRequest(restRequest, multiRequest.indicesOptions(), allowExplicitIndex,
(searchRequest, bytes) -> {
if (searchRequest.types().length > 0) {
deprecationLogger.deprecated(RestMultiSearchAction.TYPES_DEPRECATION_MESSAGE);
}
SearchTemplateRequest searchTemplateRequest = SearchTemplateRequest.fromXContent(bytes);
if (searchTemplateRequest.getScript() != null) {
searchTemplateRequest.setRequest(searchRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.search.RestMultiSearchAction;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.search.Scroll;
import org.elasticsearch.search.builder.SearchSourceBuilder;
Expand Down Expand Up @@ -77,6 +78,8 @@ public void testParseRequest() throws Exception {
assertEquals(1, request.requests().get(0).getScriptParams().size());
assertEquals(1, request.requests().get(1).getScriptParams().size());
assertEquals(1, request.requests().get(2).getScriptParams().size());

assertWarnings(RestMultiSearchAction.TYPES_DEPRECATION_MESSAGE);
}

public void testParseWithCarriageReturn() throws Exception {
Expand Down Expand Up @@ -105,10 +108,10 @@ public void testMaxConcurrentSearchRequests() {
expectThrows(IllegalArgumentException.class, () ->
request.maxConcurrentSearchRequests(randomIntBetween(Integer.MIN_VALUE, 0)));
}

public void testMultiSearchTemplateToJson() throws Exception {
final int numSearchRequests = randomIntBetween(1, 10);
MultiSearchTemplateRequest multiSearchTemplateRequest = new MultiSearchTemplateRequest();
MultiSearchTemplateRequest multiSearchTemplateRequest = new MultiSearchTemplateRequest();
for (int i = 0; i < numSearchRequests; i++) {
// Create a random request.
String[] indices = {"test"};
Expand All @@ -118,25 +121,25 @@ public void testMultiSearchTemplateToJson() throws Exception {
// batched reduce size is currently not set-able on a per-request basis as it is a query string parameter only
searchRequest.setBatchedReduceSize(SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE);
SearchTemplateRequest searchTemplateRequest = new SearchTemplateRequest(searchRequest);

searchTemplateRequest.setScript("{\"query\": { \"match\" : { \"{{field}}\" : \"{{value}}\" }}}");
searchTemplateRequest.setScriptType(ScriptType.INLINE);
searchTemplateRequest.setProfile(randomBoolean());

Map<String, Object> scriptParams = new HashMap<>();
scriptParams.put("field", "name");
scriptParams.put("value", randomAlphaOfLengthBetween(2, 5));
searchTemplateRequest.setScriptParams(scriptParams);
multiSearchTemplateRequest.add(searchTemplateRequest);

multiSearchTemplateRequest.add(searchTemplateRequest);
}

//Serialize the request
String serialized = toJsonString(multiSearchTemplateRequest);

//Deserialize the request
RestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry())
.withContent(new BytesArray(serialized), XContentType.JSON).build();
.withContent(new BytesArray(serialized), XContentType.JSON).build();
MultiSearchTemplateRequest deser = RestMultiSearchTemplateAction.parseRequest(restRequest, true);

// For object equality purposes need to set the search requests' source to non-null
Expand All @@ -145,17 +148,17 @@ public void testMultiSearchTemplateToJson() throws Exception {
if (sr.source() == null) {
sr.source(new SearchSourceBuilder());
}
}
}
// Compare the deserialized request object with the original request object
assertEquals(multiSearchTemplateRequest, deser);

// Finally, serialize the deserialized request to compare JSON equivalence (in case Object.equals() fails to reveal a discrepancy)
assertEquals(serialized, toJsonString(deser));
}

protected String toJsonString(MultiSearchTemplateRequest multiSearchTemplateRequest) throws IOException {
byte[] bytes = MultiSearchTemplateRequest.writeMultiLineFormat(multiSearchTemplateRequest, XContentType.JSON.xContent());
return new String(bytes, StandardCharsets.UTF_8);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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.script.mustache;

import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.search.RestMultiSearchAction;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestChannel;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.usage.UsageService;

import java.nio.charset.StandardCharsets;
import java.util.Collections;

import static org.mockito.Mockito.mock;

public class RestMultiSearchTemplateActionTests extends ESTestCase {
private RestController controller;

public void setUp() throws Exception {
super.setUp();
controller = new RestController(Collections.emptySet(), null,
mock(NodeClient.class),
new NoneCircuitBreakerService(),
new UsageService());
new RestMultiSearchTemplateAction(Settings.EMPTY, controller);
}

public void testTypeInPath() {
String content = "{ \"index\": \"some_index\" } \n" +
"{\"source\": {\"query\" : {\"match_all\" :{}}}} \n";
BytesArray bytesContent = new BytesArray(content.getBytes(StandardCharsets.UTF_8));

RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.GET)
.withPath("/some_index/some_type/_msearch/template")
.withContent(bytesContent, XContentType.JSON)
.build();

performRequest(request);
assertWarnings(RestMultiSearchAction.TYPES_DEPRECATION_MESSAGE);
}

public void testTypeInBody() {
String content = "{ \"index\": \"some_index\", \"type\": \"some_type\" } \n" +
"{\"source\": {\"query\" : {\"match_all\" :{}}}} \n";
BytesArray bytesContent = new BytesArray(content.getBytes(StandardCharsets.UTF_8));

RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withPath("/some_index/_msearch/template")
.withContent(bytesContent, XContentType.JSON)
.build();

performRequest(request);
assertWarnings(RestMultiSearchAction.TYPES_DEPRECATION_MESSAGE);
}

private void performRequest(RestRequest request) {
RestChannel channel = new FakeRestChannel(request, false, 1);
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
controller.dispatchRequest(request, channel, threadContext);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* 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.script.mustache;

import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.search.RestSearchAction;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestChannel;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.usage.UsageService;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static org.mockito.Mockito.mock;

public class RestSearchTemplateActionTests extends ESTestCase {
private RestController controller;

public void setUp() throws Exception {
super.setUp();
controller = new RestController(Collections.emptySet(), null,
mock(NodeClient.class),
new NoneCircuitBreakerService(),
new UsageService());
new RestSearchTemplateAction(Settings.EMPTY, controller);
}

public void testTypeInPath() {
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.GET)
.withPath("/some_index/some_type/_search/template")
.build();

performRequest(request);
assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE);
}

public void testTypeParameter() {
Map<String, String> params = new HashMap<>();
params.put("type", "some_type");

RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.GET)
.withPath("/some_index/_search/template")
.withParams(params)
.build();

performRequest(request);
assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE);
}

private void performRequest(RestRequest request) {
RestChannel channel = new FakeRestChannel(request, false, 1);
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
controller.dispatchRequest(request, channel, threadContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@
import static org.elasticsearch.rest.RestRequest.Method.POST;

public class RestMultiSearchAction extends BaseRestHandler {
private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TYPED_KEYS_PARAM);

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
LogManager.getLogger(RestMultiSearchAction.class));
static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" +
public static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" +
" Specifying types in multi search requests is deprecated.";

private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TYPED_KEYS_PARAM);

private final boolean allowExplicitIndex;

public RestMultiSearchAction(Settings settings, RestController controller) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class RestSearchAction extends BaseRestHandler {
private static final Set<String> RESPONSE_PARAMS = Collections.singleton(TYPED_KEYS_PARAM);

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestSearchAction.class));
static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" +
public static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" +
" Specifying types in search requests is deprecated.";

public RestSearchAction(Settings settings, RestController controller) {
Expand Down

0 comments on commit 25c416b

Please sign in to comment.