Skip to content

Consolidate script parsing from object (7.x) #59509

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 14, 2020
Merged

Consolidate script parsing from object (7.x) #59509

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d47a00
Consolidate script parsing from object
javanna Jul 14, 2020
b03246d
iter
javanna Jul 14, 2020
3a8333d
add deprecation warning
javanna Jul 14, 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
72 changes: 1 addition & 71 deletions modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestUpdateByQueryAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,19 @@

package org.elasticsearch.index.reindex;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptType;

import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;

import static java.util.Arrays.asList;
import static java.util.Collections.unmodifiableList;
import static org.elasticsearch.rest.RestRequest.Method.POST;
import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_LANG;

public class RestUpdateByQueryAction extends AbstractBulkByQueryRestHandler<UpdateByQueryRequest, UpdateByQueryAction> {

Expand Down Expand Up @@ -73,76 +67,12 @@ protected UpdateByQueryRequest buildRequest(RestRequest request) throws IOExcept

Map<String, Consumer<Object>> consumers = new HashMap<>();
consumers.put("conflicts", o -> internal.setConflicts((String) o));
consumers.put("script", o -> internal.setScript(parseScript(o)));
consumers.put("script", o -> internal.setScript(Script.parse(o)));
consumers.put("max_docs", s -> setMaxDocsValidateIdentical(internal, ((Number) s).intValue()));

parseInternalRequest(internal, request, consumers);

internal.setPipeline(request.param("pipeline"));
return internal;
}

@SuppressWarnings("unchecked")
private static Script parseScript(Object config) {
assert config != null : "Script should not be null";

if (config instanceof String) {
return new Script((String) config);
} else if (config instanceof Map) {
Map<String,Object> configMap = (Map<String, Object>) config;
String script = null;
ScriptType type = null;
String lang = null;
Map<String, Object> params = Collections.emptyMap();
for (Iterator<Map.Entry<String, Object>> itr = configMap.entrySet().iterator(); itr.hasNext();) {
Map.Entry<String, Object> entry = itr.next();
String parameterName = entry.getKey();
Object parameterValue = entry.getValue();
if (Script.LANG_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof String || parameterValue == null) {
lang = (String) parameterValue;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
} else if (Script.PARAMS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof Map || parameterValue == null) {
params = (Map<String, Object>) parameterValue;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
} else if (ScriptType.INLINE.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof String || parameterValue == null) {
script = (String) parameterValue;
type = ScriptType.INLINE;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
} else if (ScriptType.STORED.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof String || parameterValue == null) {
script = (String) parameterValue;
type = ScriptType.STORED;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
}
}
if (script == null) {
throw new ElasticsearchParseException("expected one of [{}] or [{}] fields, but found none",
ScriptType.INLINE.getParseField().getPreferredName(), ScriptType.STORED.getParseField().getPreferredName());
}
assert type != null : "if script is not null, type should definitely not be null";

if (type == ScriptType.STORED) {
if (lang != null) {
throw new IllegalArgumentException("lang cannot be specified for stored scripts");
}

return new Script(type, null, script, null, params);
} else {
return new Script(type, lang == null ? DEFAULT_SCRIPT_LANG : lang, script, params);
}
} else {
throw new IllegalArgumentException("Script value should be a String or a Map");
}
}
}
77 changes: 77 additions & 0 deletions server/src/main/java/org/elasticsearch/script/Script.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.script;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
Expand Down Expand Up @@ -404,6 +405,82 @@ public static Script parse(XContentParser parser, String defaultLang) throws IOE
return PARSER.apply(parser, null).build(defaultLang);
}

/**
* Parse a {@link Script} from an {@link Object}, that can either be a {@link String} or a {@link Map}.
* @see #parse(XContentParser, String)
* @param config The object to parse the script from.
* @return The parsed {@link Script}.
*/
@SuppressWarnings("unchecked")
public static Script parse(Object config) {
Objects.requireNonNull(config, "Script must not be null");
if (config instanceof String) {
return new Script((String) config);
} else if (config instanceof Map) {
Map<String,Object> configMap = (Map<String, Object>) config;
String script = null;
ScriptType type = null;
String lang = null;
Map<String, Object> params = Collections.emptyMap();
Map<String, String> options = Collections.emptyMap();
for (Map.Entry<String, Object> entry : configMap.entrySet()) {
String parameterName = entry.getKey();
Object parameterValue = entry.getValue();
if (Script.LANG_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof String || parameterValue == null) {
lang = (String) parameterValue;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
} else if (Script.PARAMS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof Map || parameterValue == null) {
params = (Map<String, Object>) parameterValue;
} else {
throw new ElasticsearchParseException("Value must be of type Map: [" + parameterName + "]");
}
} else if (Script.OPTIONS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof Map || parameterValue == null) {
options = (Map<String, String>) parameterValue;
} else {
throw new ElasticsearchParseException("Value must be of type Map: [" + parameterName + "]");
}
} else if (ScriptType.INLINE.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof String || parameterValue == null) {
script = (String) parameterValue;
type = ScriptType.INLINE;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
} else if (ScriptType.STORED.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
if (parameterValue instanceof String || parameterValue == null) {
script = (String) parameterValue;
type = ScriptType.STORED;
} else {
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
}
}
}
if (script == null) {
throw new ElasticsearchParseException("Expected one of [{}] or [{}] fields, but found none",
ScriptType.INLINE.getParseField().getPreferredName(), ScriptType.STORED.getParseField().getPreferredName());
}
assert type != null : "if script is not null, type should definitely not be null";

if (type == ScriptType.STORED) {
if (lang != null) {
throw new IllegalArgumentException("[" + Script.LANG_PARSE_FIELD.getPreferredName() +
"] cannot be specified for stored scripts");
}

return new Script(type, null, script, null, params);
} else {
return new Script(type, lang == null ? DEFAULT_SCRIPT_LANG : lang, script, options, params);
}
} else {
throw new IllegalArgumentException("Script value should be a String or a Map");
}
}

private final ScriptType type;
private final String lang;
private final String idOrCode;
Expand Down
106 changes: 106 additions & 0 deletions server/src/test/java/org/elasticsearch/script/ScriptTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@

package org.elasticsearch.script;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.InputStreamStreamInput;
import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.ESTestCase;
Expand All @@ -34,6 +36,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -96,4 +99,107 @@ public void testParse() throws IOException {
}
}
}

public void testParseFromObjectShortSyntax() {
Script script = Script.parse("doc['my_field']");
assertEquals(Script.DEFAULT_SCRIPT_LANG, script.getLang());
assertEquals("doc['my_field']", script.getIdOrCode());
assertEquals(0, script.getParams().size());
assertEquals(0, script.getOptions().size());
assertEquals(ScriptType.INLINE, script.getType());
}

public void testParseFromObject() {
Map<String, Object> map = new HashMap<>();
map.put("source", "doc['my_field']");
Map<String, Object> params = new HashMap<>();
int numParams = randomIntBetween(0, 3);
for (int i = 0; i < numParams; i++) {
params.put("param" + i, i);
}
map.put("params", params);
Map<String, String> options = new HashMap<>();
int numOptions = randomIntBetween(0, 3);
for (int i = 0; i < numOptions; i++) {
options.put("option" + i, Integer.toString(i));
}
map.put("options", options);
String lang = Script.DEFAULT_SCRIPT_LANG;;
if (randomBoolean()) {
map.put("lang", lang);
} else if(randomBoolean()) {
lang = "expression";
map.put("lang", lang);
}

Script script = Script.parse(map);
assertEquals(lang, script.getLang());
assertEquals("doc['my_field']", script.getIdOrCode());
assertEquals(ScriptType.INLINE, script.getType());
assertEquals(params, script.getParams());
assertEquals(options, script.getOptions());
}

public void testParseFromObjectFromScript() {
Map<String, Object> params = new HashMap<>();
int numParams = randomIntBetween(0, 3);
for (int i = 0; i < numParams; i++) {
params.put("param" + i, i);
}
Map<String, String> options = new HashMap<>();
int numOptions = randomIntBetween(0, 3);
for (int i = 0; i < numOptions; i++) {
options.put("option" + i, Integer.toString(i));
}
Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "doc['field']", options, params);
Map<String, Object> scriptObject = XContentHelper.convertToMap(XContentType.JSON.xContent(), Strings.toString(script), false);
Script parsedScript = Script.parse(scriptObject);
assertEquals(script, parsedScript);
}

public void testParseFromObjectWrongFormat() {
{
NullPointerException exc = expectThrows(
NullPointerException.class,
() -> Script.parse((Object)null)
);
assertEquals("Script must not be null", exc.getMessage());
}
{
IllegalArgumentException exc = expectThrows(
IllegalArgumentException.class,
() -> Script.parse(3)
);
assertEquals("Script value should be a String or a Map", exc.getMessage());
}
{
ElasticsearchParseException exc = expectThrows(
ElasticsearchParseException.class,
() -> Script.parse(Collections.emptyMap())
);
assertEquals("Expected one of [source] or [id] fields, but found none", exc.getMessage());
}
}

public void testParseFromObjectWrongOptionsFormat() {
Map<String, Object> map = new HashMap<>();
map.put("source", "doc['my_field']");
map.put("options", 3);
ElasticsearchParseException exc = expectThrows(
ElasticsearchParseException.class,
() -> Script.parse(map)
);
assertEquals("Value must be of type Map: [options]", exc.getMessage());
}

public void testParseFromObjectWrongParamsFormat() {
Map<String, Object> map = new HashMap<>();
map.put("source", "doc['my_field']");
map.put("params", 3);
ElasticsearchParseException exc = expectThrows(
ElasticsearchParseException.class,
() -> Script.parse(map)
);
assertEquals("Value must be of type Map: [params]", exc.getMessage());
}
}