diff --git a/server/src/main/java/org/elasticsearch/common/util/LocaleUtils.java b/server/src/main/java/org/elasticsearch/common/util/LocaleUtils.java index bdbf843ad2b5..9a718fb27470 100644 --- a/server/src/main/java/org/elasticsearch/common/util/LocaleUtils.java +++ b/server/src/main/java/org/elasticsearch/common/util/LocaleUtils.java @@ -14,15 +14,15 @@ import java.util.Locale; import java.util.MissingResourceException; /** - * Utilities for for dealing with {@link Locale} objects + * Utilities for dealing with {@link Locale} objects */ public class LocaleUtils { /** * Parse the given locale as {@code language}, {@code language-country} or * {@code language-country-variant}. - * Either underscores or hyphens may be used as separators, but consistently, ie. - * you may not use an hyphen to separate the language from the country and an + * Either underscores or hyphens may be used as separators, but consistently, i.e. + * you may not use a hyphen to separate the language from the country and an * underscore to separate the country from the variant. * @throws IllegalArgumentException if there are too many parts in the locale string * @throws IllegalArgumentException if the language or country is not recognized diff --git a/server/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java b/server/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java index ce59a6503735..51eace66b99a 100644 --- a/server/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java +++ b/server/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java @@ -543,18 +543,38 @@ public final class ConfigurationUtils { Script script = new Script(ScriptType.INLINE, DEFAULT_TEMPLATE_LANG, propertyValue, Map.of()); return scriptService.compile(script, TemplateScript.CONTEXT); } else { - return (params) -> new TemplateScript(params) { - @Override - public String execute() { - return propertyValue; - } - }; + return new ConstantTemplateScriptFactory(propertyValue); } } catch (Exception e) { throw ConfigurationUtils.newConfigurationException(processorType, processorTag, propertyName, e); } } + /** + * A 'template script' that ignores the model to which it is applied and just always returns a constant String. + *

+ * Having a separate named class for this allows for some hot code paths to pre-apply the 'template script' statically, + * rather than bothering to invoke it per-document. Note that this is probably only useful if something expensive + * is being done with the result of calling the script, and the code can then avoid doing that thing per-document. + */ + public static class ConstantTemplateScriptFactory implements TemplateScript.Factory { + final TemplateScript script; + + private ConstantTemplateScriptFactory(String value) { + this.script = new TemplateScript(Map.of()) { + @Override + public String execute() { + return value; + } + }; + } + + @Override + public TemplateScript newInstance(Map params) { + return script; + } + } + private static void addMetadataToException( ElasticsearchException exception, String processorType, diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 3dfeb21dd6d9..047ebe954abd 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -83,7 +83,7 @@ public final class IngestDocument { * of the pipeline was that the _index value did not change and so only 'foo' would appear * in the index history. */ - private Set indexHistory = new LinkedHashSet<>(); + private final Set indexHistory = new LinkedHashSet<>(); private boolean doNoSelfReferencesCheck = false; private boolean reroute = false; @@ -238,9 +238,7 @@ public final class IngestDocument { } else if (object instanceof String string) { return Base64.getDecoder().decode(string); } else { - throw new IllegalArgumentException( - "Content field [" + path + "] of unknown type [" + object.getClass().getName() + "], must be string or byte array" - ); + throw new IllegalArgumentException(Errors.notStringOrByteArray(path, object)); } } @@ -268,51 +266,42 @@ public final class IngestDocument { String pathElement = fieldPath.pathElements[i]; if (context == null) { return false; - } - if (context instanceof Map map) { + } else if (context instanceof Map map) { context = map.get(pathElement); } else if (context instanceof List list) { + int index; try { - int index = Integer.parseInt(pathElement); - if (index < 0 || index >= list.size()) { - if (failOutOfRange) { - throw new IllegalArgumentException( - "[" - + index - + "] is out of bounds for array with length [" - + list.size() - + "] as part of path [" - + path - + "]" - ); - } else { - return false; - } - } - context = list.get(index); + index = Integer.parseInt(pathElement); } catch (NumberFormatException e) { return false; } - + if (index < 0 || index >= list.size()) { + if (failOutOfRange) { + throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); + } else { + return false; + } + } else { + context = list.get(index); + } } else { return false; } } String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1]; - if (context instanceof Map map) { + if (context == null) { + return false; + } else if (context instanceof Map map) { return map.containsKey(leafKey); - } - if (context instanceof List list) { + } else if (context instanceof List list) { try { int index = Integer.parseInt(leafKey); if (index >= 0 && index < list.size()) { return true; } else { if (failOutOfRange) { - throw new IllegalArgumentException( - "[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + path + "]" - ); + throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); } else { return false; } @@ -320,8 +309,9 @@ public final class IngestDocument { } catch (NumberFormatException e) { return false; } + } else { + return false; } - return false; } /** @@ -342,79 +332,58 @@ public final class IngestDocument { } String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1]; - if (context instanceof Map map) { + if (context == null) { + throw new IllegalArgumentException(Errors.cannotRemove(path, leafKey, null)); + } else if (context instanceof Map map) { if (map.containsKey(leafKey)) { map.remove(leafKey); - return; + } else { + throw new IllegalArgumentException(Errors.notPresent(path, leafKey)); } - throw new IllegalArgumentException("field [" + leafKey + "] not present as part of path [" + path + "]"); - } - if (context instanceof List list) { + } else if (context instanceof List list) { int index; try { index = Integer.parseInt(leafKey); } catch (NumberFormatException e) { - throw new IllegalArgumentException( - "[" + leafKey + "] is not an integer, cannot be used as an index as part of path [" + path + "]", - e - ); + throw new IllegalArgumentException(Errors.notInteger(path, leafKey), e); } if (index < 0 || index >= list.size()) { - throw new IllegalArgumentException( - "[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + path + "]" - ); + throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); + } else { + list.remove(index); } - list.remove(index); - return; + } else { + throw new IllegalArgumentException(Errors.cannotRemove(path, leafKey, context)); } - - if (context == null) { - throw new IllegalArgumentException("cannot remove [" + leafKey + "] from null as part of path [" + path + "]"); - } - throw new IllegalArgumentException( - "cannot remove [" + leafKey + "] from object of type [" + context.getClass().getName() + "] as part of path [" + path + "]" - ); } private static ResolveResult resolve(String pathElement, String fullPath, Object context) { if (context == null) { - return ResolveResult.error("cannot resolve [" + pathElement + "] from null as part of path [" + fullPath + "]"); - } - if (context instanceof Map) { + return ResolveResult.error(Errors.cannotResolve(fullPath, pathElement, null)); + } else if (context instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) context; Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get if (object == NOT_FOUND) { - return ResolveResult.error("field [" + pathElement + "] not present as part of path [" + fullPath + "]"); + return ResolveResult.error(Errors.notPresent(fullPath, pathElement)); } else { return ResolveResult.success(object); } - } - if (context instanceof List list) { + } else if (context instanceof List list) { int index; try { index = Integer.parseInt(pathElement); } catch (NumberFormatException e) { - return ResolveResult.error( - "[" + pathElement + "] is not an integer, cannot be used as an index as part of path [" + fullPath + "]" - ); + return ResolveResult.error(Errors.notInteger(fullPath, pathElement)); } if (index < 0 || index >= list.size()) { - return ResolveResult.error( - "[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + fullPath + "]" - ); + return ResolveResult.error(Errors.outOfBounds(fullPath, index, list.size())); + } else { + return ResolveResult.success(list.get(index)); } - return ResolveResult.success(list.get(index)); + } else { + return ResolveResult.error(Errors.cannotResolve(fullPath, pathElement, context)); } - return ResolveResult.error( - "cannot resolve [" - + pathElement - + "] from object of type [" - + context.getClass().getName() - + "] as part of path [" - + fullPath - + "]" - ); } /** @@ -515,7 +484,6 @@ public final class IngestDocument { return; } } - setFieldValue(path, value); } @@ -540,7 +508,6 @@ public final class IngestDocument { } } } - setFieldValue(path, value); } @@ -550,9 +517,8 @@ public final class IngestDocument { for (int i = 0; i < fieldPath.pathElements.length - 1; i++) { String pathElement = fieldPath.pathElements[i]; if (context == null) { - throw new IllegalArgumentException("cannot resolve [" + pathElement + "] from null as part of path [" + path + "]"); - } - if (context instanceof Map) { + throw new IllegalArgumentException(Errors.cannotResolve(path, pathElement, null)); + } else if (context instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) context; Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get @@ -568,35 +534,22 @@ public final class IngestDocument { try { index = Integer.parseInt(pathElement); } catch (NumberFormatException e) { - throw new IllegalArgumentException( - "[" + pathElement + "] is not an integer, cannot be used as an index as part of path [" + path + "]", - e - ); + throw new IllegalArgumentException(Errors.notInteger(path, pathElement), e); } if (index < 0 || index >= list.size()) { - throw new IllegalArgumentException( - "[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + path + "]" - ); + throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); + } else { + context = list.get(index); } - context = list.get(index); } else { - throw new IllegalArgumentException( - "cannot resolve [" - + pathElement - + "] from object of type [" - + context.getClass().getName() - + "] as part of path [" - + path - + "]" - ); + throw new IllegalArgumentException(Errors.cannotResolve(path, pathElement, context)); } } String leafKey = fieldPath.pathElements[fieldPath.pathElements.length - 1]; if (context == null) { - throw new IllegalArgumentException("cannot set [" + leafKey + "] with null parent as part of path [" + path + "]"); - } - if (context instanceof Map) { + throw new IllegalArgumentException(Errors.cannotSet(path, leafKey, null)); + } else if (context instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) context; if (append) { @@ -614,42 +567,30 @@ public final class IngestDocument { return; } map.put(leafKey, value); - } else if (context instanceof List) { + } else if (context instanceof List) { @SuppressWarnings("unchecked") List list = (List) context; int index; try { index = Integer.parseInt(leafKey); } catch (NumberFormatException e) { - throw new IllegalArgumentException( - "[" + leafKey + "] is not an integer, cannot be used as an index as part of path [" + path + "]", - e - ); + throw new IllegalArgumentException(Errors.notInteger(path, leafKey), e); } if (index < 0 || index >= list.size()) { - throw new IllegalArgumentException( - "[" + index + "] is out of bounds for array with length [" + list.size() + "] as part of path [" + path + "]" - ); - } - if (append) { - Object object = list.get(index); - Object newList = appendValues(object, value, allowDuplicates); - if (newList != object) { - list.set(index, newList); + throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); + } else { + if (append) { + Object object = list.get(index); + Object newList = appendValues(object, value, allowDuplicates); + if (newList != object) { + list.set(index, newList); + } + return; } - return; + list.set(index, value); } - list.set(index, value); } else { - throw new IllegalArgumentException( - "cannot set [" - + leafKey - + "] with parent object of type [" - + context.getClass().getName() - + "] as part of path [" - + path - + "]" - ); + throw new IllegalArgumentException(Errors.cannotSet(path, leafKey, context)); } } @@ -706,9 +647,7 @@ public final class IngestDocument { if (clazz.isInstance(object)) { return clazz.cast(object); } - throw new IllegalArgumentException( - "field [" + path + "] of type [" + object.getClass().getName() + "] cannot be cast to [" + clazz.getName() + "]" - ); + throw new IllegalArgumentException(Errors.cannotCast(path, object, clazz)); } /** @@ -824,15 +763,12 @@ public final class IngestDocument { @SuppressWarnings("unchecked") private static Set getAllFields(Map input, String prefix) { Set allFields = Sets.newHashSet(); - input.forEach((k, v) -> { allFields.add(prefix + k); - if (v instanceof Map mapValue) { allFields.addAll(getAllFields((Map) mapValue, prefix + k + ".")); } }); - return allFields; } @@ -1055,24 +991,13 @@ public final class IngestDocument { } } - private static class ResolveResult { - boolean wasSuccessful; - String errorMessage; - Object resolvedObject; - + private record ResolveResult(boolean wasSuccessful, Object resolvedObject, String errorMessage) { static ResolveResult success(Object resolvedObject) { - ResolveResult result = new ResolveResult(); - result.wasSuccessful = true; - result.resolvedObject = resolvedObject; - return result; + return new ResolveResult(true, resolvedObject, null); } static ResolveResult error(String errorMessage) { - ResolveResult result = new ResolveResult(); - result.wasSuccessful = false; - result.errorMessage = errorMessage; - return result; - + return new ResolveResult(false, null, errorMessage); } } @@ -1153,4 +1078,57 @@ public final class IngestDocument { throw new UnsupportedOperationException(); } } + + private static final class Errors { + private Errors() { + // utility class + } + + private static String cannotCast(String path, Object value, Class clazz) { + return "field [" + path + "] of type [" + value.getClass().getName() + "] cannot be cast to [" + clazz.getName() + "]"; + } + + private static String cannotRemove(String path, String key, Object value) { + if (value == null) { + return "cannot remove [" + key + "] from null as part of path [" + path + "]"; + } else { + final String type = value.getClass().getName(); + return "cannot remove [" + key + "] from object of type [" + type + "] as part of path [" + path + "]"; + } + } + + private static String cannotResolve(String path, String key, Object value) { + if (value == null) { + return "cannot resolve [" + key + "] from null as part of path [" + path + "]"; + } else { + final String type = value.getClass().getName(); + return "cannot resolve [" + key + "] from object of type [" + type + "] as part of path [" + path + "]"; + } + } + + private static String cannotSet(String path, String key, Object value) { + if (value == null) { + return "cannot set [" + key + "] with null parent as part of path [" + path + "]"; + } else { + final String type = value.getClass().getName(); + return "cannot set [" + key + "] with parent object of type [" + type + "] as part of path [" + path + "]"; + } + } + + private static String outOfBounds(String path, int index, int length) { + return "[" + index + "] is out of bounds for array with length [" + length + "] as part of path [" + path + "]"; + } + + private static String notInteger(String path, String key) { + return "[" + key + "] is not an integer, cannot be used as an index as part of path [" + path + "]"; + } + + private static String notPresent(String path, String key) { + return "field [" + key + "] not present as part of path [" + path + "]"; + } + + private static String notStringOrByteArray(String path, Object value) { + return "Content field [" + path + "] of unknown type [" + value.getClass().getName() + "], must be string or byte array"; + } + } } diff --git a/server/src/test/java/org/elasticsearch/ingest/ConfigurationUtilsTests.java b/server/src/test/java/org/elasticsearch/ingest/ConfigurationUtilsTests.java index 3e282f133abe..4e4fcad2cb2b 100644 --- a/server/src/test/java/org/elasticsearch/ingest/ConfigurationUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/ConfigurationUtilsTests.java @@ -290,6 +290,7 @@ public class ConfigurationUtilsTests extends ESTestCase { propertyValue, scriptService ); + assertThat(result, instanceOf(ConfigurationUtils.ConstantTemplateScriptFactory.class)); assertThat(result.newInstance(null).execute(), equalTo(propertyValue)); verify(scriptService, times(0)).compile(any(), any()); }