honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

The update by query action parses a script from an object (map or string). We will need to do the same for runtime fields as they are parsed as part of mappings (#59391).

This commit moves the existing parsing of a script from an object from RestUpdateByQueryAction to the Script class. It also adds tests and adjusts some error messages that are incorrect. Also, options were not parsed before and they are now. And unsupported fields trigger now a deprecation warning.
This commit is contained in:
Luca Cavanna 2020-07-14 17:08:29 +02:00 committed by GitHub
parent e302c66847
commit af2f85be15
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 191 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestUpdateByQueryAction.java
View File

@ -19,17 +19,12 @@
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;
@ -37,7 +32,6 @@ 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> {
@ -73,7 +67,7 @@ public class RestUpdateByQueryAction extends AbstractBulkByQueryRestHandler<Upda
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);
@ -81,68 +75,4 @@ public class RestUpdateByQueryAction extends AbstractBulkByQueryRestHandler<Upda
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");
}
}
}

84
server/src/main/java/org/elasticsearch/script/Script.java
View File

@ -19,6 +19,8 @@
package org.elasticsearch.script;
import org.apache.logging.log4j.LogManager;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
@ -26,6 +28,7 @@ import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.AbstractObjectParser;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
@ -83,6 +86,8 @@ import java.util.function.BiConsumer;
*/
public final class Script implements ToXContentObject, Writeable {
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(Script.class));
/**
* The name of the of the default scripting language.
*/
@ -404,6 +409,85 @@ public final class Script implements ToXContentObject, Writeable {
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 + "]");
}
} else {
deprecationLogger.deprecatedAndMaybeLog("script_unsupported_fields", "script section does not support ["
+ 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;

106
server/src/test/java/org/elasticsearch/script/ScriptTests.java
View File

@ -19,6 +19,7 @@
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;
@ -26,6 +27,7 @@ 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;
@ -34,6 +36,7 @@ import java.io.ByteArrayInputStream;
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;
@ -96,4 +99,107 @@ public class ScriptTests extends ESTestCase {
}
}
}
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());
}
}