Skip to content

Commit af2f85b

Browse files
javannajavanna
authored
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.
1 parent e302c66 commit af2f85b

File tree

3 files changed

+191
-71
lines changed

3 files changed

+191
-71
lines changed

‎modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestUpdateByQueryAction.java

Lines changed: 1 addition & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,19 @@
1919

2020
package org.elasticsearch.index.reindex;
2121

22-
import org.elasticsearch.ElasticsearchParseException;
2322
import org.elasticsearch.client.node.NodeClient;
24-
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
2523
import org.elasticsearch.rest.RestRequest;
2624
import org.elasticsearch.script.Script;
27-
import org.elasticsearch.script.ScriptType;
2825

2926
import java.io.IOException;
30-
import java.util.Collections;
3127
import java.util.HashMap;
32-
import java.util.Iterator;
3328
import java.util.List;
3429
import java.util.Map;
3530
import java.util.function.Consumer;
3631

3732
import static java.util.Arrays.asList;
3833
import static java.util.Collections.unmodifiableList;
3934
import static org.elasticsearch.rest.RestRequest.Method.POST;
40-
import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_LANG;
4135

4236
public class RestUpdateByQueryAction extends AbstractBulkByQueryRestHandler<UpdateByQueryRequest, UpdateByQueryAction> {
4337

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

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

7973
parseInternalRequest(internal, request, consumers);
8074

8175
internal.setPipeline(request.param("pipeline"));
8276
return internal;
8377
}
84-
85-
@SuppressWarnings("unchecked")
86-
private static Script parseScript(Object config) {
87-
assert config != null : "Script should not be null";
88-
89-
if (config instanceof String) {
90-
return new Script((String) config);
91-
} else if (config instanceof Map) {
92-
Map<String,Object> configMap = (Map<String, Object>) config;
93-
String script = null;
94-
ScriptType type = null;
95-
String lang = null;
96-
Map<String, Object> params = Collections.emptyMap();
97-
for (Iterator<Map.Entry<String, Object>> itr = configMap.entrySet().iterator(); itr.hasNext();) {
98-
Map.Entry<String, Object> entry = itr.next();
99-
String parameterName = entry.getKey();
100-
Object parameterValue = entry.getValue();
101-
if (Script.LANG_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
102-
if (parameterValue instanceof String || parameterValue == null) {
103-
lang = (String) parameterValue;
104-
} else {
105-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
106-
}
107-
} else if (Script.PARAMS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
108-
if (parameterValue instanceof Map || parameterValue == null) {
109-
params = (Map<String, Object>) parameterValue;
110-
} else {
111-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
112-
}
113-
} else if (ScriptType.INLINE.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
114-
if (parameterValue instanceof String || parameterValue == null) {
115-
script = (String) parameterValue;
116-
type = ScriptType.INLINE;
117-
} else {
118-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
119-
}
120-
} else if (ScriptType.STORED.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
121-
if (parameterValue instanceof String || parameterValue == null) {
122-
script = (String) parameterValue;
123-
type = ScriptType.STORED;
124-
} else {
125-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
126-
}
127-
}
128-
}
129-
if (script == null) {
130-
throw new ElasticsearchParseException("expected one of [{}] or [{}] fields, but found none",
131-
ScriptType.INLINE.getParseField().getPreferredName(), ScriptType.STORED.getParseField().getPreferredName());
132-
}
133-
assert type != null : "if script is not null, type should definitely not be null";
134-
135-
if (type == ScriptType.STORED) {
136-
if (lang != null) {
137-
throw new IllegalArgumentException("lang cannot be specified for stored scripts");
138-
}
139-
140-
return new Script(type, null, script, null, params);
141-
} else {
142-
return new Script(type, lang == null ? DEFAULT_SCRIPT_LANG : lang, script, params);
143-
}
144-
} else {
145-
throw new IllegalArgumentException("Script value should be a String or a Map");
146-
}
147-
}
14878
}

‎server/src/main/java/org/elasticsearch/script/Script.java

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@
1919

2020
package org.elasticsearch.script;
2121

22+
import org.apache.logging.log4j.LogManager;
23+
import org.elasticsearch.ElasticsearchParseException;
2224
import org.elasticsearch.common.ParseField;
2325
import org.elasticsearch.common.Strings;
2426
import org.elasticsearch.common.bytes.BytesArray;
2527
import org.elasticsearch.common.bytes.BytesReference;
2628
import org.elasticsearch.common.io.stream.StreamInput;
2729
import org.elasticsearch.common.io.stream.StreamOutput;
2830
import org.elasticsearch.common.io.stream.Writeable;
31+
import org.elasticsearch.common.logging.DeprecationLogger;
2932
import org.elasticsearch.common.settings.Settings;
3033
import org.elasticsearch.common.xcontent.AbstractObjectParser;
3134
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
@@ -83,6 +86,8 @@
8386
*/
8487
public final class Script implements ToXContentObject, Writeable {
8588

89+
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(Script.class));
90+
8691
/**
8792
* The name of the of the default scripting language.
8893
*/
@@ -404,6 +409,85 @@ public static Script parse(XContentParser parser, String defaultLang) throws IOE
404409
return PARSER.apply(parser, null).build(defaultLang);
405410
}
406411

412+
/**
413+
* Parse a {@link Script} from an {@link Object}, that can either be a {@link String} or a {@link Map}.
414+
* @see #parse(XContentParser, String)
415+
* @param config The object to parse the script from.
416+
* @return The parsed {@link Script}.
417+
*/
418+
@SuppressWarnings("unchecked")
419+
public static Script parse(Object config) {
420+
Objects.requireNonNull(config, "Script must not be null");
421+
if (config instanceof String) {
422+
return new Script((String) config);
423+
} else if (config instanceof Map) {
424+
Map<String,Object> configMap = (Map<String, Object>) config;
425+
String script = null;
426+
ScriptType type = null;
427+
String lang = null;
428+
Map<String, Object> params = Collections.emptyMap();
429+
Map<String, String> options = Collections.emptyMap();
430+
for (Map.Entry<String, Object> entry : configMap.entrySet()) {
431+
String parameterName = entry.getKey();
432+
Object parameterValue = entry.getValue();
433+
if (Script.LANG_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
434+
if (parameterValue instanceof String || parameterValue == null) {
435+
lang = (String) parameterValue;
436+
} else {
437+
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
438+
}
439+
} else if (Script.PARAMS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
440+
if (parameterValue instanceof Map || parameterValue == null) {
441+
params = (Map<String, Object>) parameterValue;
442+
} else {
443+
throw new ElasticsearchParseException("Value must be of type Map: [" + parameterName + "]");
444+
}
445+
} else if (Script.OPTIONS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
446+
if (parameterValue instanceof Map || parameterValue == null) {
447+
options = (Map<String, String>) parameterValue;
448+
} else {
449+
throw new ElasticsearchParseException("Value must be of type Map: [" + parameterName + "]");
450+
}
451+
} else if (ScriptType.INLINE.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
452+
if (parameterValue instanceof String || parameterValue == null) {
453+
script = (String) parameterValue;
454+
type = ScriptType.INLINE;
455+
} else {
456+
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
457+
}
458+
} else if (ScriptType.STORED.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
459+
if (parameterValue instanceof String || parameterValue == null) {
460+
script = (String) parameterValue;
461+
type = ScriptType.STORED;
462+
} else {
463+
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
464+
}
465+
} else {
466+
deprecationLogger.deprecatedAndMaybeLog("script_unsupported_fields", "script section does not support ["
467+
+ parameterName + "]");
468+
}
469+
}
470+
if (script == null) {
471+
throw new ElasticsearchParseException("Expected one of [{}] or [{}] fields, but found none",
472+
ScriptType.INLINE.getParseField().getPreferredName(), ScriptType.STORED.getParseField().getPreferredName());
473+
}
474+
assert type != null : "if script is not null, type should definitely not be null";
475+
476+
if (type == ScriptType.STORED) {
477+
if (lang != null) {
478+
throw new IllegalArgumentException("[" + Script.LANG_PARSE_FIELD.getPreferredName() +
479+
"] cannot be specified for stored scripts");
480+
}
481+
482+
return new Script(type, null, script, null, params);
483+
} else {
484+
return new Script(type, lang == null ? DEFAULT_SCRIPT_LANG : lang, script, options, params);
485+
}
486+
} else {
487+
throw new IllegalArgumentException("Script value should be a String or a Map");
488+
}
489+
}
490+
407491
private final ScriptType type;
408492
private final String lang;
409493
private final String idOrCode;

‎server/src/test/java/org/elasticsearch/script/ScriptTests.java

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919

2020
package org.elasticsearch.script;
2121

22+
import org.elasticsearch.ElasticsearchParseException;
2223
import org.elasticsearch.common.Strings;
2324
import org.elasticsearch.common.io.stream.InputStreamStreamInput;
2425
import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
2526
import org.elasticsearch.common.settings.Settings;
2627
import org.elasticsearch.common.xcontent.ToXContent;
2728
import org.elasticsearch.common.xcontent.XContentBuilder;
2829
import org.elasticsearch.common.xcontent.XContentFactory;
30+
import org.elasticsearch.common.xcontent.XContentHelper;
2931
import org.elasticsearch.common.xcontent.XContentParser;
3032
import org.elasticsearch.common.xcontent.XContentType;
3133
import org.elasticsearch.test.ESTestCase;
@@ -34,6 +36,7 @@
3436
import java.io.ByteArrayOutputStream;
3537
import java.io.IOException;
3638
import java.util.Collections;
39+
import java.util.HashMap;
3740
import java.util.Map;
3841

3942
import static org.hamcrest.Matchers.equalTo;
@@ -96,4 +99,107 @@ public void testParse() throws IOException {
9699
}
97100
}
98101
}
102+
103+
public void testParseFromObjectShortSyntax() {
104+
Script script = Script.parse("doc['my_field']");
105+
assertEquals(Script.DEFAULT_SCRIPT_LANG, script.getLang());
106+
assertEquals("doc['my_field']", script.getIdOrCode());
107+
assertEquals(0, script.getParams().size());
108+
assertEquals(0, script.getOptions().size());
109+
assertEquals(ScriptType.INLINE, script.getType());
110+
}
111+
112+
public void testParseFromObject() {
113+
Map<String, Object> map = new HashMap<>();
114+
map.put("source", "doc['my_field']");
115+
Map<String, Object> params = new HashMap<>();
116+
int numParams = randomIntBetween(0, 3);
117+
for (int i = 0; i < numParams; i++) {
118+
params.put("param" + i, i);
119+
}
120+
map.put("params", params);
121+
Map<String, String> options = new HashMap<>();
122+
int numOptions = randomIntBetween(0, 3);
123+
for (int i = 0; i < numOptions; i++) {
124+
options.put("option" + i, Integer.toString(i));
125+
}
126+
map.put("options", options);
127+
String lang = Script.DEFAULT_SCRIPT_LANG;;
128+
if (randomBoolean()) {
129+
map.put("lang", lang);
130+
} else if(randomBoolean()) {
131+
lang = "expression";
132+
map.put("lang", lang);
133+
}
134+
135+
Script script = Script.parse(map);
136+
assertEquals(lang, script.getLang());
137+
assertEquals("doc['my_field']", script.getIdOrCode());
138+
assertEquals(ScriptType.INLINE, script.getType());
139+
assertEquals(params, script.getParams());
140+
assertEquals(options, script.getOptions());
141+
}
142+
143+
public void testParseFromObjectFromScript() {
144+
Map<String, Object> params = new HashMap<>();
145+
int numParams = randomIntBetween(0, 3);
146+
for (int i = 0; i < numParams; i++) {
147+
params.put("param" + i, i);
148+
}
149+
Map<String, String> options = new HashMap<>();
150+
int numOptions = randomIntBetween(0, 3);
151+
for (int i = 0; i < numOptions; i++) {
152+
options.put("option" + i, Integer.toString(i));
153+
}
154+
Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "doc['field']", options, params);
155+
Map<String, Object> scriptObject = XContentHelper.convertToMap(XContentType.JSON.xContent(), Strings.toString(script), false);
156+
Script parsedScript = Script.parse(scriptObject);
157+
assertEquals(script, parsedScript);
158+
}
159+
160+
public void testParseFromObjectWrongFormat() {
161+
{
162+
NullPointerException exc = expectThrows(
163+
NullPointerException.class,
164+
() -> Script.parse((Object)null)
165+
);
166+
assertEquals("Script must not be null", exc.getMessage());
167+
}
168+
{
169+
IllegalArgumentException exc = expectThrows(
170+
IllegalArgumentException.class,
171+
() -> Script.parse(3)
172+
);
173+
assertEquals("Script value should be a String or a Map", exc.getMessage());
174+
}
175+
{
176+
ElasticsearchParseException exc = expectThrows(
177+
ElasticsearchParseException.class,
178+
() -> Script.parse(Collections.emptyMap())
179+
);
180+
assertEquals("Expected one of [source] or [id] fields, but found none", exc.getMessage());
181+
}
182+
}
183+
184+
public void testParseFromObjectWrongOptionsFormat() {
185+
Map<String, Object> map = new HashMap<>();
186+
map.put("source", "doc['my_field']");
187+
map.put("options", 3);
188+
ElasticsearchParseException exc = expectThrows(
189+
ElasticsearchParseException.class,
190+
() -> Script.parse(map)
191+
);
192+
assertEquals("Value must be of type Map: [options]", exc.getMessage());
193+
}
194+
195+
public void testParseFromObjectWrongParamsFormat() {
196+
Map<String, Object> map = new HashMap<>();
197+
map.put("source", "doc['my_field']");
198+
map.put("params", 3);
199+
ElasticsearchParseException exc = expectThrows(
200+
ElasticsearchParseException.class,
201+
() -> Script.parse(map)
202+
);
203+
assertEquals("Value must be of type Map: [params]", exc.getMessage());
204+
}
99205
}

0 commit comments

Comments
 (0)