diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java index 1714717d0e6d3..fc17506555edb 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java @@ -48,7 +48,7 @@ public void testEscapeFields() throws Exception { processor = new DotExpanderProcessor("_tag", null, null, "foo.bar"); processor.execute(document); assertThat(document.getSource().size(), equalTo(1)); - assertThat(document.getMetadataMap().size(), equalTo(1)); // the default version + assertThat(document.getMetadata().size(), equalTo(1)); // the default version assertThat(document.getFieldValue("foo.bar", List.class).size(), equalTo(2)); assertThat(document.getFieldValue("foo.bar.0", String.class), equalTo("baz2")); assertThat(document.getFieldValue("foo.bar.1", String.class), equalTo("baz1")); @@ -60,7 +60,7 @@ public void testEscapeFields() throws Exception { processor = new DotExpanderProcessor("_tag", null, null, "foo.bar"); processor.execute(document); assertThat(document.getSource().size(), equalTo(1)); - assertThat(document.getMetadataMap().size(), equalTo(1)); // the default version + assertThat(document.getMetadata().size(), equalTo(1)); // the default version assertThat(document.getFieldValue("foo.bar", List.class).size(), equalTo(2)); assertThat(document.getFieldValue("foo.bar.0", Integer.class), equalTo(1)); assertThat(document.getFieldValue("foo.bar.1", String.class), equalTo("2")); diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java index 4cab0b999c248..32566e82baf80 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java @@ -140,11 +140,11 @@ public void testRenameAtomicOperationSetFails() throws Exception { Map metadata = new HashMap<>(); metadata.put("list", Collections.singletonList("item")); - IngestDocument ingestDocument = TestIngestDocument.ofMetadataWithValidator(metadata, Map.of("new_field", (k, v) -> { + IngestDocument ingestDocument = TestIngestDocument.ofMetadataWithValidator(metadata, Map.of("new_field", (o, k, v) -> { if (v != null) { throw new UnsupportedOperationException(); } - }, "list", (k, v) -> {})); + }, "list", (o, k, v) -> {})); Processor processor = createRenameProcessor("list", "new_field", false); try { processor.execute(ingestDocument); @@ -160,7 +160,7 @@ public void testRenameAtomicOperationRemoveFails() throws Exception { Map metadata = new HashMap<>(); metadata.put("list", Collections.singletonList("item")); - IngestDocument ingestDocument = TestIngestDocument.ofMetadataWithValidator(metadata, Map.of("list", (k, v) -> { + IngestDocument ingestDocument = TestIngestDocument.ofMetadataWithValidator(metadata, Map.of("list", (o, k, v) -> { if (v == null) { throw new UnsupportedOperationException(); } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/ingest/IngestClientIT.java b/server/src/internalClusterTest/java/org/elasticsearch/ingest/IngestClientIT.java index 2e69cdff0fe80..9b05421d479d2 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/ingest/IngestClientIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/ingest/IngestClientIT.java @@ -115,7 +115,7 @@ public void testSimulate() throws Exception { source.put("processed", true); IngestDocument ingestDocument = new IngestDocument("index", "id", Versions.MATCH_ANY, null, null, source); assertThat(simulateDocumentBaseResult.getIngestDocument().getSource(), equalTo(ingestDocument.getSource())); - assertThat(simulateDocumentBaseResult.getIngestDocument().getMetadataMap(), equalTo(ingestDocument.getMetadataMap())); + assertThat(simulateDocumentBaseResult.getIngestDocument().getMetadata().getMap(), equalTo(ingestDocument.getMetadata().getMap())); assertThat(simulateDocumentBaseResult.getFailure(), nullValue()); // cleanup diff --git a/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java b/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java index e9e2882763e33..3a7e3c11fa141 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java @@ -107,10 +107,11 @@ IngestDocument getIngestDocument() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(DOC_FIELD); - Map metadataMap = ingestDocument.getMetadataMap(); - for (Map.Entry metadata : metadataMap.entrySet()) { - if (metadata.getValue() != null) { - builder.field(metadata.getKey(), metadata.getValue().toString()); + org.elasticsearch.script.Metadata metadata = ingestDocument.getMetadata(); + for (String key : metadata.keySet()) { + Object value = metadata.get(key); + if (value != null) { + builder.field(key, value.toString()); } } if (builder.getRestApiVersion() == RestApiVersion.V_7) { diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index e6e8f428efb3a..a77d5d57b3170 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -68,7 +68,7 @@ public IngestDocument(String index, String id, long version, String routing, Ver source ); this.ingestMetadata = new HashMap<>(); - this.ingestMetadata.put(TIMESTAMP, sourceAndMetadata.getTimestamp()); + this.ingestMetadata.put(TIMESTAMP, sourceAndMetadata.getMetadata().getTimestamp()); } /** @@ -76,12 +76,7 @@ public IngestDocument(String index, String id, long version, String routing, Ver */ public IngestDocument(IngestDocument other) { this( - new IngestSourceAndMetadata( - deepCopyMap(other.sourceAndMetadata.getSource()), - deepCopyMap(other.sourceAndMetadata.getMetadata()), - other.getIngestSourceAndMetadata().timestamp, - other.getIngestSourceAndMetadata().validators - ), + new IngestSourceAndMetadata(deepCopyMap(other.sourceAndMetadata.getSource()), other.sourceAndMetadata.getMetadata().clone()), deepCopyMap(other.ingestMetadata) ); } @@ -93,14 +88,12 @@ public IngestDocument(Map sourceAndMetadata, Map Tuple, Map> sm = IngestSourceAndMetadata.splitSourceAndMetadata(sourceAndMetadata); this.sourceAndMetadata = new IngestSourceAndMetadata( sm.v1(), - sm.v2(), - IngestSourceAndMetadata.getTimestamp(ingestMetadata), - IngestSourceAndMetadata.VALIDATORS + new org.elasticsearch.script.Metadata(sm.v2(), IngestSourceAndMetadata.getTimestamp(ingestMetadata)) ); this.ingestMetadata = new HashMap<>(ingestMetadata); this.ingestMetadata.computeIfPresent(TIMESTAMP, (k, v) -> { if (v instanceof String) { - return this.sourceAndMetadata.getTimestamp(); + return this.sourceAndMetadata.getMetadata().getTimestamp(); } return v; }); @@ -737,18 +730,11 @@ public IngestSourceAndMetadata getIngestSourceAndMetadata() { return sourceAndMetadata; } - /** - * Get all Metadata values in a Map - */ - public Map getMetadataMap() { - return sourceAndMetadata.getMetadata(); - } - /** * Get the strongly typed metadata */ public org.elasticsearch.script.Metadata getMetadata() { - return sourceAndMetadata; + return sourceAndMetadata.getMetadata(); } /** diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 67f0abae7b23d..152c9a80f7d62 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -897,27 +897,27 @@ private void innerExecute( itemDroppedHandler.accept(slot); handler.accept(null); } else { - IngestSourceAndMetadata sourceAndMetadata = ingestDocument.getIngestSourceAndMetadata(); + org.elasticsearch.script.Metadata metadata = ingestDocument.getMetadata(); // it's fine to set all metadata fields all the time, as ingest document holds their starting values // before ingestion, which might also get modified during ingestion. - indexRequest.index(sourceAndMetadata.getIndex()); - indexRequest.id(sourceAndMetadata.getId()); - indexRequest.routing(sourceAndMetadata.getRouting()); - indexRequest.version(sourceAndMetadata.getVersion()); - if (sourceAndMetadata.getVersionType() != null) { - indexRequest.versionType(VersionType.fromString(sourceAndMetadata.getVersionType())); + indexRequest.index(metadata.getIndex()); + indexRequest.id(metadata.getId()); + indexRequest.routing(metadata.getRouting()); + indexRequest.version(metadata.getVersion()); + if (metadata.getVersionType() != null) { + indexRequest.versionType(VersionType.fromString(metadata.getVersionType())); } Number number; - if ((number = sourceAndMetadata.getIfSeqNo()) != null) { + if ((number = metadata.getIfSeqNo()) != null) { indexRequest.setIfSeqNo(number.longValue()); } - if ((number = sourceAndMetadata.getIfPrimaryTerm()) != null) { + if ((number = metadata.getIfPrimaryTerm()) != null) { indexRequest.setIfPrimaryTerm(number.longValue()); } try { boolean ensureNoSelfReferences = ingestDocument.doNoSelfReferencesCheck(); - indexRequest.source(sourceAndMetadata.getSource(), indexRequest.getContentType(), ensureNoSelfReferences); + indexRequest.source(ingestDocument.getSource(), indexRequest.getContentType(), ensureNoSelfReferences); } catch (IllegalArgumentException ex) { // An IllegalArgumentException can be thrown when an ingest // processor creates a source map that is self-referencing. @@ -933,7 +933,7 @@ private void innerExecute( return; } Map map; - if ((map = sourceAndMetadata.getDynamicTemplates()) != null) { + if ((map = metadata.getDynamicTemplates()) != null) { Map mergedDynamicTemplates = new HashMap<>(indexRequest.getDynamicTemplates()); mergedDynamicTemplates.putAll(map); indexRequest.setDynamicTemplates(mergedDynamicTemplates); diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestSourceAndMetadata.java b/server/src/main/java/org/elasticsearch/ingest/IngestSourceAndMetadata.java index 68d779be37812..16a79d6c7f074 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestSourceAndMetadata.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestSourceAndMetadata.java @@ -9,6 +9,7 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.util.Maps; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.VersionType; import org.elasticsearch.script.Metadata; @@ -17,7 +18,6 @@ import java.util.AbstractCollection; import java.util.AbstractMap; import java.util.AbstractSet; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -25,7 +25,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.BiConsumer; import java.util.stream.Collectors; /** @@ -41,37 +40,10 @@ * * The map is expected to be used by processors, server code should the typed getter and setters where possible. */ -class IngestSourceAndMetadata extends AbstractMap implements Metadata { - protected final ZonedDateTime timestamp; - - /** - * map of key to validating function. Should throw {@link IllegalArgumentException} on invalid value - */ - static final Map> VALIDATORS = Map.of( - IngestDocument.Metadata.INDEX.getFieldName(), - IngestSourceAndMetadata::stringValidator, - IngestDocument.Metadata.ID.getFieldName(), - IngestSourceAndMetadata::stringValidator, - IngestDocument.Metadata.ROUTING.getFieldName(), - IngestSourceAndMetadata::stringValidator, - IngestDocument.Metadata.VERSION.getFieldName(), - IngestSourceAndMetadata::versionValidator, - IngestDocument.Metadata.VERSION_TYPE.getFieldName(), - IngestSourceAndMetadata::versionTypeValidator, - IngestDocument.Metadata.DYNAMIC_TEMPLATES.getFieldName(), - IngestSourceAndMetadata::mapValidator, - IngestDocument.Metadata.IF_SEQ_NO.getFieldName(), - IngestSourceAndMetadata::longValidator, - IngestDocument.Metadata.IF_PRIMARY_TERM.getFieldName(), - IngestSourceAndMetadata::longValidator, - IngestDocument.Metadata.TYPE.getFieldName(), - IngestSourceAndMetadata::stringValidator - ); +class IngestSourceAndMetadata extends AbstractMap { protected final Map source; - protected final Map metadata; - protected final Map> validators; - private EntrySet entrySet; // cache to avoid recreation + protected final Metadata metadata; /** * Create an IngestSourceAndMetadata with the given metadata, source and default validators @@ -85,46 +57,26 @@ class IngestSourceAndMetadata extends AbstractMap implements Met ZonedDateTime timestamp, Map source ) { - this(new HashMap<>(source), metadataMap(index, id, version, routing, versionType), timestamp, VALIDATORS); + this(new HashMap<>(source), new Metadata(index, id, version, routing, versionType, timestamp)); } /** - * Create IngestSourceAndMetadata with custom validators. + * Create IngestSourceAndMetadata from a source and metadata * * @param source the source document map * @param metadata the metadata map - * @param timestamp the time of ingestion - * @param validators validators to run on metadata map, if a key is in this map, the value is stored in metadata. - * if null, use the default validators from {@link #VALIDATORS} */ - IngestSourceAndMetadata( - Map source, - Map metadata, - ZonedDateTime timestamp, - Map> validators - ) { + IngestSourceAndMetadata(Map source, Metadata metadata) { this.source = source != null ? source : new HashMap<>(); - this.metadata = metadata != null ? metadata : new HashMap<>(); - this.timestamp = timestamp; - this.validators = validators != null ? validators : VALIDATORS; - validateMetadata(); - } - - /** - * Create the backing metadata map with the standard contents assuming default validators. - */ - protected static Map metadataMap(String index, String id, long version, String routing, VersionType versionType) { - Map metadata = Maps.newHashMapWithExpectedSize(IngestDocument.Metadata.values().length); - metadata.put(IngestDocument.Metadata.INDEX.getFieldName(), index); - metadata.put(IngestDocument.Metadata.ID.getFieldName(), id); - metadata.put(IngestDocument.Metadata.VERSION.getFieldName(), version); - if (routing != null) { - metadata.put(IngestDocument.Metadata.ROUTING.getFieldName(), routing); - } - if (versionType != null) { - metadata.put(IngestDocument.Metadata.VERSION_TYPE.getFieldName(), VersionType.toString(versionType)); + this.metadata = metadata; + Set badKeys = Sets.intersection(this.metadata.keySet(), this.source.keySet()); + if (badKeys.size() > 0) { + throw new IllegalArgumentException( + "unexpected metadata [" + + badKeys.stream().sorted().map(k -> k + ":" + this.source.get(k)).collect(Collectors.joining(", ")) + + "] in source" + ); } - return metadata; } /** @@ -132,11 +84,12 @@ protected static Map metadataMap(String index, String id, long v */ public static Tuple, Map> splitSourceAndMetadata(Map sourceAndMetadata) { if (sourceAndMetadata instanceof IngestSourceAndMetadata ingestSourceAndMetadata) { - return new Tuple<>(new HashMap<>(ingestSourceAndMetadata.source), new HashMap<>(ingestSourceAndMetadata.metadata)); + return new Tuple<>(new HashMap<>(ingestSourceAndMetadata.source), new HashMap<>(ingestSourceAndMetadata.metadata.getMap())); } Map metadata = Maps.newHashMapWithExpectedSize(IngestDocument.Metadata.values().length); Map source = new HashMap<>(sourceAndMetadata); - for (String metadataName : VALIDATORS.keySet()) { + for (IngestDocument.Metadata ingestDocumentMetadata : IngestDocument.Metadata.values()) { + String metadataName = ingestDocumentMetadata.getFieldName(); if (sourceAndMetadata.containsKey(metadataName)) { metadata.put(metadataName, source.remove(metadataName)); } @@ -171,107 +124,17 @@ public Map getSource() { /** * get the metadata map, if externally modified then the guarantees of this class are not enforced */ - public Map getMetadata() { + public Metadata getMetadata() { return metadata; } - // These are available to scripts - public String getIndex() { - return getString(IngestDocument.Metadata.INDEX.getFieldName()); - } - - public void setIndex(String index) { - put(IngestDocument.Metadata.INDEX.getFieldName(), index); - } - - public String getId() { - return getString(IngestDocument.Metadata.ID.getFieldName()); - } - - public void setId(String id) { - put(IngestDocument.Metadata.ID.getFieldName(), id); - } - - public String getRouting() { - return getString(IngestDocument.Metadata.ROUTING.getFieldName()); - } - - public void setRouting(String routing) { - put(IngestDocument.Metadata.ROUTING.getFieldName(), routing); - } - - public String getVersionType() { - return getString(IngestDocument.Metadata.VERSION_TYPE.getFieldName()); - } - - public void setVersionType(String versionType) { - put(IngestDocument.Metadata.VERSION_TYPE.getFieldName(), versionType); - } - - public long getVersion() { - Number version = getNumber(IngestDocument.Metadata.VERSION.getFieldName()); - assert version != null : IngestDocument.Metadata.VERSION.getFieldName() + " validation allowed null version"; - return version.longValue(); - } - - public void setVersion(long version) { - put(IngestDocument.Metadata.VERSION.getFieldName(), version); - } - - // timestamp isn't backed by the map - public ZonedDateTime getTimestamp() { - return timestamp; - } - - // These are not available to scripts - public Number getIfSeqNo() { - return getNumber(IngestDocument.Metadata.IF_SEQ_NO.getFieldName()); - } - - public Number getIfPrimaryTerm() { - return getNumber(IngestDocument.Metadata.IF_PRIMARY_TERM.getFieldName()); - } - - @SuppressWarnings("unchecked") - public Map getDynamicTemplates() { - return (Map) metadata.get(IngestDocument.Metadata.DYNAMIC_TEMPLATES.getFieldName()); - } - - /** - * Check that all metadata map contains only valid metadata and no extraneous keys and source map contains no metadata - */ - protected void validateMetadata() { - int numMetadata = 0; - for (Map.Entry> entry : validators.entrySet()) { - String key = entry.getKey(); - if (metadata.containsKey(key)) { - numMetadata++; - } - entry.getValue().accept(key, metadata.get(key)); - if (source.containsKey(key)) { - throw new IllegalArgumentException("Unexpected metadata key [" + key + "] in source with value [" + source.get(key) + "]"); - } - } - if (numMetadata < metadata.size()) { - Set keys = new HashSet<>(metadata.keySet()); - keys.removeAll(validators.keySet()); - throw new IllegalArgumentException( - "Unexpected metadata keys [" - + keys.stream().sorted().map(k -> k + ":" + metadata.get(k)).collect(Collectors.joining(", ")) - + "]" - ); - } - } - /** * Returns an entrySet that respects the validators of the map. */ @Override public Set> entrySet() { - if (entrySet == null) { - entrySet = new EntrySet(source.entrySet(), metadata.entrySet()); - } - return entrySet; + // Make a copy of the Metadata.keySet() to avoid a ConcurrentModificationException when removing a value from the iterator + return new EntrySet(source.entrySet(), new HashSet<>(metadata.keySet())); } /** @@ -280,9 +143,7 @@ public Set> entrySet() { */ @Override public Object put(String key, Object value) { - BiConsumer validator = validators.get(key); - if (validator != null) { - validator.accept(key, value); + if (metadata.isAvailable(key)) { return metadata.put(key, value); } return source.put(key, value); @@ -295,11 +156,9 @@ public Object put(String key, Object value) { @Override public Object remove(Object key) { // uses map directly to avoid AbstractMaps linear time implementation using entrySet() - if (key instanceof String strKey) { - BiConsumer validator = validators.get(key); - if (validator != null) { - validator.accept(strKey, null); - return metadata.remove(key); + if (key instanceof String str) { + if (metadata.isAvailable(str)) { + return metadata.remove(str); } } return source.remove(key); @@ -312,12 +171,9 @@ public Object remove(Object key) { @Override public void clear() { // AbstractMap uses entrySet().clear(), it should be quicker to run through the validators, then call the wrapped maps clear - validators.forEach((k, v) -> { - if (metadata.containsKey(k)) { - v.accept(k, null); - } - }); - metadata.clear(); + for (String key : metadata.keySet()) { + metadata.remove(key); + } source.clear(); } @@ -336,42 +192,23 @@ public boolean containsValue(Object value) { @Override public boolean containsKey(Object key) { // uses map directly to avoid AbstractMaps linear time implementation using entrySet() - return metadata.containsKey(key) || source.containsKey(key); + if (key instanceof String str) { + return metadata.containsKey(str) || source.containsKey(key); + } + return source.containsKey(key); } @Override public Object get(Object key) { // uses map directly to avoid AbstractMaps linear time implementation using entrySet() - if (validators.get(key) != null) { - return metadata.get(key); + if (key instanceof String str) { + if (metadata.isAvailable(str)) { + return metadata.get(str); + } } return source.get(key); } - /** - * Get the String version of the value associated with {@code key}, or null - */ - public String getString(Object key) { - return Objects.toString(get(key), null); - } - - /** - * Get the {@link Number} associated with key, or null - * @throws IllegalArgumentException if the value is not a {@link Number} - */ - public Number getNumber(Object key) { - Object value = get(key); - if (value == null) { - return null; - } - if (value instanceof Number number) { - return number; - } - throw new IllegalArgumentException( - "unexpected type for [" + key + "] with value [" + value + "], expected Number, got [" + value.getClass().getName() + "]" - ); - } - /** * Set of entries of the wrapped map that calls the appropriate validator before changing an entries value or removing an entry. * @@ -382,32 +219,31 @@ public Number getNumber(Object key) { */ class EntrySet extends AbstractSet> { Set> sourceSet; - Set> metadataSet; + Set metadataKeys; - EntrySet(Set> sourceSet, Set> metadataSet) { + EntrySet(Set> sourceSet, Set metadataKeys) { this.sourceSet = sourceSet; - this.metadataSet = metadataSet; + this.metadataKeys = metadataKeys; } @Override public Iterator> iterator() { - return new EntrySetIterator(sourceSet.iterator(), metadataSet.iterator()); + return new EntrySetIterator(sourceSet.iterator(), metadataKeys.iterator()); } @Override public int size() { - return sourceSet.size() + metadataSet.size(); + return sourceSet.size() + metadataKeys.size(); } @Override public boolean remove(Object o) { - if (metadataSet.contains(o)) { - if (o instanceof Map.Entry entry) { - if (entry.getKey()instanceof String key) { - BiConsumer validator = validators.get(key); - if (validator != null) { - validator.accept(key, null); - return metadataSet.remove(o); + if (o instanceof Map.Entry entry) { + if (entry.getKey()instanceof String key) { + if (metadata.containsKey(key)) { + if (Objects.equals(entry.getValue(), metadata.get(key))) { + metadata.remove(key); + return true; } } } @@ -424,25 +260,25 @@ public boolean remove(Object o) { */ class EntrySetIterator implements Iterator> { final Iterator> sourceIter; - final Iterator> metadataIter; + final Iterator metadataKeyIter; boolean sourceCur = true; - Entry cur; + Map.Entry cur; - EntrySetIterator(Iterator> sourceIter, Iterator> metadataIter) { + EntrySetIterator(Iterator> sourceIter, Iterator metadataKeyIter) { this.sourceIter = sourceIter; - this.metadataIter = metadataIter; + this.metadataKeyIter = metadataKeyIter; } @Override public boolean hasNext() { - return sourceIter.hasNext() || metadataIter.hasNext(); + return sourceIter.hasNext() || metadataKeyIter.hasNext(); } @Override public Map.Entry next() { sourceCur = sourceIter.hasNext(); - return cur = new Entry(sourceCur ? sourceIter.next() : metadataIter.next(), sourceCur); + return cur = sourceCur ? sourceIter.next() : new Entry(metadataKeyIter.next()); } /** @@ -458,145 +294,48 @@ public void remove() { if (sourceCur) { sourceIter.remove(); } else { - BiConsumer validator = validators.get(cur.getKey()); - if (validator != null) { - validator.accept(cur.getKey(), null); - } - metadataIter.remove(); + metadata.remove(cur.getKey()); } } } /** - * Wrapped Map.Entry that calls the key's validator on {@link #setValue(Object)} + * Map.Entry that stores metadata key and calls into {@link #metadata} for {@link #setValue} */ class Entry implements Map.Entry { - final Map.Entry entry; - final boolean isSource; + final String key; - Entry(Map.Entry entry, boolean isSource) { - this.entry = entry; - this.isSource = isSource; + Entry(String key) { + this.key = key; } @Override public String getKey() { - return entry.getKey(); + return key; } @Override public Object getValue() { - return entry.getValue(); + return metadata.get(key); } - /** - * Associate the value with the Entry's key in the linked Map. If the Entry's key has a validator, it is applied before association - * @throws IllegalArgumentException if value does not pass validation for the Entry's key - */ @Override public Object setValue(Object value) { - if (isSource == false) { - BiConsumer validator = validators.get(entry.getKey()); - if (validator != null) { - validator.accept(entry.getKey(), value); - } - } - return entry.setValue(value); - } - } - - /** - * Allow a String or null - */ - protected static void stringValidator(String key, Object value) { - if (value == null || value instanceof String) { - return; - } - throw new IllegalArgumentException( - key + " must be null or a String but was [" + value + "] with type [" + value.getClass().getName() + "]" - ); - } - - /** - * Allow Numbers that can be represented as longs without loss of precision - */ - protected static void longValidator(String key, Object value) { - if (value == null) { - return; // Allow null version for now - } - if (value instanceof Number number) { - long version = number.longValue(); - // did we round? - if (number.doubleValue() == version) { - return; - } - } - throw new IllegalArgumentException( - key + " may only be set to an int or a long but was [" + value + "] with type [" + value.getClass().getName() + "]" - ); - } - - /** - * Version must be non-null and representable as a long without loss of precision - */ - protected static void versionValidator(String key, Object value) { - if (value == null) { - throw new IllegalArgumentException(key + " cannot be null"); - } - longValidator(key, value); - } - - /** - * Allow lower case Strings that map to VersionType values, or null - */ - protected static void versionTypeValidator(String key, Object value) { - if (value == null) { - return; - } - if (value instanceof String versionType) { - try { - VersionType.fromString(versionType); - return; - } catch (IllegalArgumentException ignored) {} - } - throw new IllegalArgumentException( - key - + " must be a null or one of [" - + Arrays.stream(VersionType.values()).map(vt -> VersionType.toString(vt)).collect(Collectors.joining(", ")) - + "] but was [" - + value - + "] with type [" - + value.getClass().getName() - + "]" - ); - } - - /** - * Allow maps - */ - protected static void mapValidator(String key, Object value) { - if (value == null || value instanceof Map) { - return; + return metadata.put(key, value); } - throw new IllegalArgumentException( - key + " must be a null or a Map but was [" + value + "] with type [" + value.getClass().getName() + "]" - ); } @Override public boolean equals(Object o) { if (this == o) return true; - if ((o instanceof IngestSourceAndMetadata) == false) return false; + if (o == null || getClass() != o.getClass()) return false; if (super.equals(o) == false) return false; IngestSourceAndMetadata that = (IngestSourceAndMetadata) o; - return Objects.equals(timestamp, that.timestamp) - && source.equals(that.source) - && metadata.equals(that.metadata) - && validators.equals(that.validators); + return Objects.equals(source, that.source) && Objects.equals(metadata, that.metadata); } @Override public int hashCode() { - return Objects.hash(timestamp, source, metadata, validators); + return Objects.hash(source, metadata); } } diff --git a/server/src/main/java/org/elasticsearch/script/Metadata.java b/server/src/main/java/org/elasticsearch/script/Metadata.java index e598a211c109a..8118e4f5f0cb7 100644 --- a/server/src/main/java/org/elasticsearch/script/Metadata.java +++ b/server/src/main/java/org/elasticsearch/script/Metadata.java @@ -8,53 +8,411 @@ package org.elasticsearch.script; +import org.elasticsearch.common.util.Maps; +import org.elasticsearch.index.VersionType; +import org.elasticsearch.ingest.IngestDocument; + import java.time.ZonedDateTime; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; /** - * Ingest and update metadata available to write scripts + * Ingest and update metadata available to write scripts. + * + * Provides a map-like interface for backwards compatibility with the ctx map. + * - {@link #put(String, Object)} + * - {@link #get(String)} + * - {@link #remove(String)} + * - {@link #containsKey(String)} + * - {@link #containsValue(Object)} + * - {@link #keySet()} for iteration + * - {@link #size()} + * - {@link #isAvailable(String)} for determining if a key is a metadata key + * + * Provides getters and setters for script usage. + * + * Validates all updates whether originating in map-like interface or setters. */ -public interface Metadata { +public class Metadata { + protected static final String INDEX = "_index"; + protected static final String ID = "_id"; + protected static final String ROUTING = "_routing"; + protected static final String VERSION_TYPE = "_version_type"; + protected static final String VERSION = "_version"; + protected static final String TYPE = "_type"; // type is deprecated so it's supported in the map but not available as a getter + protected static final String IF_SEQ_NO = "_if_seq_no"; + protected static final String IF_PRIMARY_TERM = "_if_primary_term"; + protected static final String DYNAMIC_TEMPLATES = "_dynamic_templates"; + + protected static final Map VALIDATORS = Map.of( + INDEX, + Metadata::stringValidator, + ID, + Metadata::stringValidator, + ROUTING, + Metadata::stringValidator, + VERSION_TYPE, + Metadata::versionTypeValidator, + VERSION, + Metadata::notNullLongValidator, + TYPE, + Metadata::stringValidator, + IF_SEQ_NO, + Metadata::longValidator, + IF_PRIMARY_TERM, + Metadata::longValidator, + DYNAMIC_TEMPLATES, + Metadata::mapValidator + ); + + protected final Map map; + protected final Map validators; + + // timestamp is new to ingest metadata, so it doesn't need to be backed by the map for back compat + protected final ZonedDateTime timestamp; + + public Metadata(String index, String id, long version, String routing, VersionType versionType, ZonedDateTime timestamp) { + this(metadataMap(index, id, version, routing, versionType), timestamp, VALIDATORS); + } + + public Metadata(Map map, ZonedDateTime timestamp) { + this(map, timestamp, VALIDATORS); + } + + Metadata(Map map, ZonedDateTime timestamp, Map validators) { + this.map = map; + this.timestamp = timestamp; + this.validators = validators; + validateMetadata(); + } + /** - * The destination index + * Create the backing metadata map with the standard contents assuming default validators. */ - String getIndex(); + protected static Map metadataMap(String index, String id, long version, String routing, VersionType versionType) { + Map metadata = Maps.newHashMapWithExpectedSize(IngestDocument.Metadata.values().length); + metadata.put(IngestDocument.Metadata.INDEX.getFieldName(), index); + metadata.put(IngestDocument.Metadata.ID.getFieldName(), id); + metadata.put(IngestDocument.Metadata.VERSION.getFieldName(), version); + if (routing != null) { + metadata.put(IngestDocument.Metadata.ROUTING.getFieldName(), routing); + } + if (versionType != null) { + metadata.put(IngestDocument.Metadata.VERSION_TYPE.getFieldName(), VersionType.toString(versionType)); + } + return metadata; + } + + /** + * Check that all metadata map contains only valid metadata and no extraneous keys and source map contains no metadata + */ + protected void validateMetadata() { + int numMetadata = 0; + for (Map.Entry entry : validators.entrySet()) { + String key = entry.getKey(); + if (map.containsKey(key)) { + numMetadata++; + } + entry.getValue().accept(MapOperation.INIT, key, map.get(key)); + } + if (numMetadata < map.size()) { + Set keys = new HashSet<>(map.keySet()); + keys.removeAll(validators.keySet()); + throw new IllegalArgumentException( + "Unexpected metadata keys [" + keys.stream().sorted().map(k -> k + ":" + map.get(k)).collect(Collectors.joining(", ")) + "]" + ); + } + } + + // These are available to scripts + public String getIndex() { + return getString(INDEX); + } + + public void setIndex(String index) { + put(INDEX, index); + } + + public String getId() { + return getString(ID); + } + + public void setId(String id) { + put(ID, id); + } + + public String getRouting() { + return getString(ROUTING); + } - void setIndex(String index); + public void setRouting(String routing) { + put(ROUTING, routing); + } + + public String getVersionType() { + return getString(VERSION_TYPE); + } + + public void setVersionType(String versionType) { + put(VERSION_TYPE, versionType); + } + + public long getVersion() { + return getNumber(VERSION).longValue(); + } + + public void setVersion(long version) { + put(VERSION, version); + } + + public ZonedDateTime getTimestamp() { + return timestamp; + } + + // These are not available to scripts + public Number getIfSeqNo() { + return getNumber(IF_SEQ_NO); + } + + public Number getIfPrimaryTerm() { + return getNumber(IF_PRIMARY_TERM); + } + + @SuppressWarnings("unchecked") + public Map getDynamicTemplates() { + return (Map) get(DYNAMIC_TEMPLATES); + } /** - * The document id + * Get the String version of the value associated with {@code key}, or null */ - String getId(); + protected String getString(String key) { + return Objects.toString(get(key), null); + } - void setId(String id); + /** + * Get the {@link Number} associated with key, or null + * @throws IllegalArgumentException if the value is not a {@link Number} + */ + protected Number getNumber(String key) { + Object value = get(key); + if (value == null) { + return null; + } + if (value instanceof Number number) { + return number; + } + throw new IllegalStateException( + "unexpected type for [" + key + "] with value [" + value + "], expected Number, got [" + value.getClass().getName() + "]" + ); + } /** - * The document routing string + * Is this key a Metadata key? A {@link #remove}d key would return false for {@link #containsKey(String)} but true for + * this call. */ - String getRouting(); + public boolean isAvailable(String key) { + return validators.containsKey(key); + } - void setRouting(String routing); + /** + * Create the mapping from key to value. + * @throws IllegalArgumentException if {@link #isAvailable(String)} is false or the key cannot be updated to the value. + */ + public Object put(String key, Object value) { + Validator v = validators.getOrDefault(key, this::badKey); + v.accept(MapOperation.UPDATE, key, value); + return map.put(key, value); + } /** - * The version of the document + * Does the metadata contain the key? */ - long getVersion(); + public boolean containsKey(String key) { + return map.containsKey(key); + } - void setVersion(long version); + /** + * Does the metadata contain the value. + */ + public boolean containsValue(Object value) { + return map.containsValue(value); + } /** - * The version type of the document, {@link org.elasticsearch.index.VersionType} as a lower-case string. + * Get the value associated with {@param key} */ - String getVersionType(); + public Object get(String key) { + return map.get(key); + } /** - * Set the version type of the document. - * @param versionType {@link org.elasticsearch.index.VersionType} as a lower-case string + * Remove the mapping associated with {@param key} + * @throws IllegalArgumentException if {@link #isAvailable(String)} is false or the key cannot be removed. */ - void setVersionType(String versionType); + public Object remove(String key) { + Validator v = validators.getOrDefault(key, this::badKey); + v.accept(MapOperation.REMOVE, key, null); + return map.remove(key); + } /** - * Timestamp of this ingestion or update + * Return the list of keys with mappings */ - ZonedDateTime getTimestamp(); + public Set keySet() { + return Collections.unmodifiableSet(map.keySet()); + } + + /** + * The number of metadata keys currently mapped. + */ + public int size() { + return map.size(); + } + + @Override + public Metadata clone() { + return new Metadata(new HashMap<>(map), timestamp, new HashMap<>(validators)); + } + + /** + * Get the backing map, if modified then the guarantees of this class may not hold + */ + public Map getMap() { + return map; + } + + /** + * Allow a String or null. + * @throws IllegalArgumentException if {@param value} is neither a {@link String} nor null + */ + protected static void stringValidator(MapOperation op, String key, Object value) { + if (op == MapOperation.REMOVE || value == null || value instanceof String) { + return; + } + throw new IllegalArgumentException( + key + " must be null or a String but was [" + value + "] with type [" + value.getClass().getName() + "]" + ); + } + + /** + * Allow Numbers that can be represented as longs without loss of precision or null + * @throws IllegalArgumentException if the value cannot be represented as a long + */ + protected static void longValidator(MapOperation op, String key, Object value) { + if (op == MapOperation.REMOVE || value == null) { + return; + } + if (value instanceof Number number) { + long version = number.longValue(); + // did we round? + if (number.doubleValue() == version) { + return; + } + } + throw new IllegalArgumentException( + key + " may only be set to an int or a long but was [" + value + "] with type [" + value.getClass().getName() + "]" + ); + } + + /** + * Same as {@link #longValidator(MapOperation, String, Object)} but {@param value} cannot be null. + * @throws IllegalArgumentException if value is null or cannot be represented as a long. + */ + protected static void notNullLongValidator(MapOperation op, String key, Object value) { + if (op == MapOperation.REMOVE || value == null) { + throw new IllegalArgumentException(key + " cannot be removed or set to null"); + } + longValidator(op, key, value); + } + + /** + * Allow maps. + * @throws IllegalArgumentException if {@param value} is not a {@link Map} + */ + protected static void mapValidator(MapOperation op, String key, Object value) { + if (op == MapOperation.REMOVE || value == null || value instanceof Map) { + return; + } + throw new IllegalArgumentException( + key + " must be a null or a Map but was [" + value + "] with type [" + value.getClass().getName() + "]" + ); + } + + /** + * Allow lower case Strings that map to VersionType values, or null. + * @throws IllegalArgumentException if {@param value} cannot be converted via {@link VersionType#fromString(String)} + */ + protected static void versionTypeValidator(MapOperation op, String key, Object value) { + if (op == MapOperation.REMOVE || value == null) { + return; + } + if (value instanceof String versionType) { + try { + VersionType.fromString(versionType); + return; + } catch (IllegalArgumentException ignored) {} + } + throw new IllegalArgumentException( + key + + " must be a null or one of [" + + Arrays.stream(VersionType.values()).map(vt -> VersionType.toString(vt)).collect(Collectors.joining(", ")) + + "] but was [" + + value + + "] with type [" + + value.getClass().getName() + + "]" + ); + } + + private void badKey(MapOperation op, String key, Object value) { + throw new IllegalArgumentException( + "unexpected metadata key [" + + key + + "], expected one of [" + + validators.keySet().stream().sorted().collect(Collectors.joining(", ")) + + "]" + ); + } + + /** + * The operation being performed on the value in the map. + * INIT: Initial value - the metadata value as passed into this class + * UPDATE: the metadata is being set to a different value + * REMOVE: the metadata mapping is being removed + */ + public enum MapOperation { + INIT, + UPDATE, + REMOVE + } + + /** + * A "TriConsumer" that tests if the {@link MapOperation}, the metadata key and value are valid. + * + * throws IllegalArgumentException if the given triple is invalid + */ + @FunctionalInterface + public interface Validator { + void accept(MapOperation op, String key, Object value); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Metadata metadata = (Metadata) o; + return Objects.equals(map, metadata.map) && Objects.equals(timestamp, metadata.timestamp); + } + + @Override + public int hashCode() { + return Objects.hash(map, timestamp); + } } diff --git a/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java b/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java index 958c59f67a838..f609114832a70 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java @@ -378,7 +378,7 @@ public boolean isAsync() { for (int id = 0; id < numDocs; id++) { SimulateDocumentBaseResult result = (SimulateDocumentBaseResult) response.getResults().get(id); assertThat( - result.getIngestDocument().getMetadataMap().get(IngestDocument.Metadata.ID.getFieldName()), + result.getIngestDocument().getMetadata().get(IngestDocument.Metadata.ID.getFieldName()), equalTo(Integer.toString(id)) ); assertThat(result.getIngestDocument().getSourceAndMetadata().get("processed"), is(true)); diff --git a/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java b/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java index e906a2d619cb0..bdc14e32c2a6f 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java @@ -100,9 +100,8 @@ public void testParseUsingPipelineStore() throws Exception { Iterator> expectedDocsIterator = expectedDocs.iterator(); for (IngestDocument ingestDocument : actualRequest.documents()) { Map expectedDocument = expectedDocsIterator.next(); - Map metadataMap = ingestDocument.getMetadataMap(); - assertThat(metadataMap.get(INDEX.getFieldName()), equalTo(expectedDocument.get(INDEX.getFieldName()))); - assertThat(metadataMap.get(ID.getFieldName()), equalTo(expectedDocument.get(ID.getFieldName()))); + assertThat(ingestDocument.getMetadata().get(INDEX.getFieldName()), equalTo(expectedDocument.get(INDEX.getFieldName()))); + assertThat(ingestDocument.getMetadata().get(ID.getFieldName()), equalTo(expectedDocument.get(ID.getFieldName()))); assertThat(ingestDocument.getSource(), equalTo(expectedDocument.get(Fields.SOURCE))); } @@ -196,14 +195,14 @@ public void testParseWithProvidedPipeline() throws Exception { Iterator> expectedDocsIterator = expectedDocs.iterator(); for (IngestDocument ingestDocument : actualRequest.documents()) { Map expectedDocument = expectedDocsIterator.next(); - Map metadataMap = ingestDocument.getMetadataMap(); - assertThat(metadataMap.get(INDEX.getFieldName()), equalTo(expectedDocument.get(INDEX.getFieldName()))); - assertThat(metadataMap.get(ID.getFieldName()), equalTo(expectedDocument.get(ID.getFieldName()))); - assertThat(metadataMap.get(ROUTING.getFieldName()), equalTo(expectedDocument.get(ROUTING.getFieldName()))); - assertThat(metadataMap.get(VERSION.getFieldName()), equalTo(expectedDocument.get(VERSION.getFieldName()))); - assertThat(metadataMap.get(VERSION_TYPE.getFieldName()), equalTo(expectedDocument.get(VERSION_TYPE.getFieldName()))); - assertThat(metadataMap.get(IF_SEQ_NO.getFieldName()), equalTo(expectedDocument.get(IF_SEQ_NO.getFieldName()))); - assertThat(metadataMap.get(IF_PRIMARY_TERM.getFieldName()), equalTo(expectedDocument.get(IF_PRIMARY_TERM.getFieldName()))); + org.elasticsearch.script.Metadata metadata = ingestDocument.getMetadata(); + assertThat(metadata.get(INDEX.getFieldName()), equalTo(expectedDocument.get(INDEX.getFieldName()))); + assertThat(metadata.get(ID.getFieldName()), equalTo(expectedDocument.get(ID.getFieldName()))); + assertThat(metadata.get(ROUTING.getFieldName()), equalTo(expectedDocument.get(ROUTING.getFieldName()))); + assertThat(metadata.get(VERSION.getFieldName()), equalTo(expectedDocument.get(VERSION.getFieldName()))); + assertThat(metadata.get(VERSION_TYPE.getFieldName()), equalTo(expectedDocument.get(VERSION_TYPE.getFieldName()))); + assertThat(metadata.get(IF_SEQ_NO.getFieldName()), equalTo(expectedDocument.get(IF_SEQ_NO.getFieldName()))); + assertThat(metadata.get(IF_PRIMARY_TERM.getFieldName()), equalTo(expectedDocument.get(IF_PRIMARY_TERM.getFieldName()))); assertThat(ingestDocument.getSource(), equalTo(expectedDocument.get(Fields.SOURCE))); } @@ -349,12 +348,12 @@ public void testIngestPipelineWithDocumentsWithType() throws Exception { Iterator> expectedDocsIterator = expectedDocs.iterator(); for (IngestDocument ingestDocument : actualRequest.documents()) { Map expectedDocument = expectedDocsIterator.next(); - Map metadataMap = ingestDocument.getMetadataMap(); - assertThat(metadataMap.get(INDEX.getFieldName()), equalTo(expectedDocument.get(INDEX.getFieldName()))); - assertThat(metadataMap.get(ID.getFieldName()), equalTo(expectedDocument.get(ID.getFieldName()))); - assertThat(metadataMap.get(ROUTING.getFieldName()), equalTo(expectedDocument.get(ROUTING.getFieldName()))); - assertThat(metadataMap.get(VERSION.getFieldName()), equalTo(expectedDocument.get(VERSION.getFieldName()))); - assertThat(metadataMap.get(VERSION_TYPE.getFieldName()), equalTo(expectedDocument.get(VERSION_TYPE.getFieldName()))); + org.elasticsearch.script.Metadata metadata = ingestDocument.getMetadata(); + assertThat(metadata.get(INDEX.getFieldName()), equalTo(expectedDocument.get(INDEX.getFieldName()))); + assertThat(metadata.get(ID.getFieldName()), equalTo(expectedDocument.get(ID.getFieldName()))); + assertThat(metadata.get(ROUTING.getFieldName()), equalTo(expectedDocument.get(ROUTING.getFieldName()))); + assertThat(metadata.get(VERSION.getFieldName()), equalTo(expectedDocument.get(VERSION.getFieldName()))); + assertThat(metadata.get(VERSION_TYPE.getFieldName()), equalTo(expectedDocument.get(VERSION_TYPE.getFieldName()))); assertThat(ingestDocument.getSource(), equalTo(expectedDocument.get(Fields.SOURCE))); } assertThat(actualRequest.pipeline().getId(), equalTo(SIMULATED_PIPELINE_ID)); diff --git a/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java b/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java index 03007d1e84712..6d70802bab021 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java @@ -148,19 +148,18 @@ public void testToXContent() throws IOException { Map toXContentSource = (Map) toXContentDoc.get("_source"); Map toXContentIngestMetadata = (Map) toXContentDoc.get("_ingest"); - Map metadataMap = ingestDocument.getMetadataMap(); - for (Map.Entry metadata : metadataMap.entrySet()) { - String fieldName = metadata.getKey(); - if (metadata.getValue() == null) { + for (String fieldName : ingestDocument.getMetadata().keySet()) { + Object value = ingestDocument.getMetadata().get(fieldName); + if (value == null) { assertThat(toXContentDoc.containsKey(fieldName), is(false)); } else { - assertThat(toXContentDoc.get(fieldName), equalTo(metadata.getValue().toString())); + assertThat(toXContentDoc.get(fieldName), equalTo(value.toString())); } } - Map sourceAndMetadata = Maps.newMapWithExpectedSize(toXContentSource.size() + metadataMap.size()); + Map sourceAndMetadata = Maps.newMapWithExpectedSize(toXContentSource.size() + ingestDocument.getMetadata().size()); sourceAndMetadata.putAll(toXContentSource); - sourceAndMetadata.putAll(metadataMap); + ingestDocument.getMetadata().keySet().forEach(k -> sourceAndMetadata.put(k, ingestDocument.getMetadata().get(k))); IngestDocument serializedIngestDocument = new IngestDocument(sourceAndMetadata, toXContentIngestMetadata); // TODO(stu): is this test correct? Comparing against ingestDocument fails due to incorrectly failed byte array comparisons assertThat(serializedIngestDocument, equalTo(serializedIngestDocument)); diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index 7fcb2b07a1f81..39d93a0691856 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -2230,7 +2230,7 @@ private class IngestDocumentMatcher implements ArgumentMatcher { public boolean matches(IngestDocument other) { // ingest metadata and IngestSourceAndMetadata will not be the same (timestamp differs every time) return Objects.equals(ingestDocument.getSource(), other.getSource()) - && Objects.equals(ingestDocument.getMetadataMap(), other.getMetadataMap()); + && Objects.equals(ingestDocument.getMetadata().getMap(), other.getMetadata().getMap()); } } diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestSourceAndMetadataTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestSourceAndMetadataTests.java index f0085abda82ee..34a05e9ef2e03 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestSourceAndMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestSourceAndMetadataTests.java @@ -9,6 +9,8 @@ package org.elasticsearch.ingest; import org.elasticsearch.index.VersionType; +import org.elasticsearch.script.Metadata; +import org.elasticsearch.script.TestMetadata; import org.elasticsearch.test.ESTestCase; import java.util.HashMap; @@ -17,15 +19,13 @@ import java.util.Map; import java.util.Set; -import static org.elasticsearch.ingest.TestIngestDocument.replaceValidator; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasEntry; -import static org.hamcrest.Matchers.notNullValue; public class IngestSourceAndMetadataTests extends ESTestCase { IngestSourceAndMetadata map; + Metadata md; public void testSettersAndGetters() { Map metadata = new HashMap<>(); @@ -37,65 +37,32 @@ public void testSettersAndGetters() { metadata.put("_if_primary_term", 10000); metadata.put("_version_type", "internal"); metadata.put("_dynamic_templates", Map.of("foo", "bar")); - map = new IngestSourceAndMetadata(new HashMap<>(), metadata, null, null); - assertEquals("myIndex", map.getIndex()); - map.setIndex("myIndex2"); - assertEquals("myIndex2", map.getIndex()); + map = new IngestSourceAndMetadata(new HashMap<>(), new Metadata(metadata, null)); + md = map.getMetadata(); + assertEquals("myIndex", md.getIndex()); + md.setIndex("myIndex2"); + assertEquals("myIndex2", md.getIndex()); - assertEquals("myId", map.getId()); - map.setId("myId2"); - assertEquals("myId2", map.getId()); + assertEquals("myId", md.getId()); + md.setId("myId2"); + assertEquals("myId2", md.getId()); - assertEquals("myRouting", map.getRouting()); - map.setRouting("myRouting2"); - assertEquals("myRouting2", map.getRouting()); + assertEquals("myRouting", md.getRouting()); + md.setRouting("myRouting2"); + assertEquals("myRouting2", md.getRouting()); - assertEquals(20, map.getVersion()); - map.setVersion(10); - assertEquals(10, map.getVersion()); + assertEquals(20, md.getVersion()); + md.setVersion(10); + assertEquals(10, md.getVersion()); - assertEquals("internal", map.getVersionType()); - map.setVersionType("external_gte"); - assertEquals("external_gte", map.getVersionType()); + assertEquals("internal", md.getVersionType()); + md.setVersionType("external_gte"); + assertEquals("external_gte", md.getVersionType()); - assertEquals(Map.of("foo", "bar"), map.getDynamicTemplates()); + assertEquals(Map.of("foo", "bar"), md.getDynamicTemplates()); - assertEquals(500, map.getIfSeqNo()); - assertEquals(10000, map.getIfPrimaryTerm()); - } - - public void testGetString() { - Map metadata = new HashMap<>(); - metadata.put("_routing", "myRouting"); - Map source = new HashMap<>(); - source.put("str", "myStr"); - source.put("toStr", new Object() { - @Override - public String toString() { - return "myToString()"; - } - }); - source.put("missing", null); - map = new IngestSourceAndMetadata(source, metadata, null, replaceValidator("_version", IngestSourceAndMetadata::longValidator)); - assertNull(map.getString("missing")); - assertNull(map.getString("no key")); - assertEquals("myToString()", map.getString("toStr")); - assertEquals("myStr", map.getString("str")); - assertEquals("myRouting", map.getString("_routing")); - } - - public void testGetNumber() { - Map metadata = new HashMap<>(); - metadata.put("_version", Long.MAX_VALUE); - Map source = new HashMap<>(); - source.put("number", "NaN"); - source.put("missing", null); - map = new IngestSourceAndMetadata(source, metadata, null, null); - assertEquals(Long.MAX_VALUE, map.getNumber("_version")); - IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> map.getNumber("number")); - assertEquals("unexpected type for [number] with value [NaN], expected Number, got [java.lang.String]", err.getMessage()); - assertNull(map.getNumber("missing")); - assertNull(map.getNumber("no key")); + assertEquals(500, md.getIfSeqNo()); + assertEquals(10000, md.getIfPrimaryTerm()); } public void testInvalidMetadata() { @@ -103,7 +70,7 @@ public void testInvalidMetadata() { metadata.put("_version", Double.MAX_VALUE); IllegalArgumentException err = expectThrows( IllegalArgumentException.class, - () -> new IngestSourceAndMetadata(new HashMap<>(), metadata, null, null) + () -> new IngestSourceAndMetadata(new HashMap<>(), new Metadata(metadata, null)) ); assertThat(err.getMessage(), containsString("_version may only be set to an int or a long but was [")); assertThat(err.getMessage(), containsString("] with type [java.lang.Double]")); @@ -114,9 +81,9 @@ public void testSourceInMetadata() { source.put("_version", 25); IllegalArgumentException err = expectThrows( IllegalArgumentException.class, - () -> new IngestSourceAndMetadata(source, source, null, null) + () -> new IngestSourceAndMetadata(source, new Metadata(source, null)) ); - assertEquals("Unexpected metadata key [_version] in source with value [25]", err.getMessage()); + assertEquals("unexpected metadata [_version:25] in source", err.getMessage()); } public void testExtraMetadata() { @@ -126,7 +93,7 @@ public void testExtraMetadata() { metadata.put("routing", "myRouting"); IllegalArgumentException err = expectThrows( IllegalArgumentException.class, - () -> new IngestSourceAndMetadata(new HashMap<>(), metadata, null, null) + () -> new IngestSourceAndMetadata(new HashMap<>(), new Metadata(metadata, null)) ); assertEquals("Unexpected metadata keys [routing:myRouting, version:567]", err.getMessage()); } @@ -135,7 +102,7 @@ public void testPutSource() { Map metadata = new HashMap<>(); metadata.put("_version", 123); Map source = new HashMap<>(); - map = new IngestSourceAndMetadata(source, metadata, null, null); + map = new IngestSourceAndMetadata(source, new Metadata(metadata, null)); } public void testRemove() { @@ -143,11 +110,11 @@ public void testRemove() { String canRemove = "canRemove"; Map metadata = new HashMap<>(); metadata.put(cannotRemove, "value"); - map = new IngestSourceAndMetadata(new HashMap<>(), metadata, null, Map.of(cannotRemove, (k, v) -> { + map = new IngestSourceAndMetadata(new HashMap<>(), new TestMetadata(metadata, Map.of(cannotRemove, (o, k, v) -> { if (v == null) { throw new IllegalArgumentException(k + " cannot be null or removed"); } - }, canRemove, (k, v) -> {})); + }, canRemove, (o, k, v) -> {}))); String msg = "cannotRemove cannot be null or removed"; IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> map.remove(cannotRemove)); assertEquals(msg, err.getMessage()); @@ -208,7 +175,8 @@ public void testEntryAndIterator() { source.put("foo", "bar"); source.put("baz", "qux"); source.put("noz", "zon"); - map = new IngestSourceAndMetadata(source, metadata, null, replaceValidator("_version", IngestSourceAndMetadata::longValidator)); + map = new IngestSourceAndMetadata(source, TestMetadata.withNullableVersion(metadata)); + md = map.getMetadata(); for (Map.Entry entry : map.entrySet()) { if ("foo".equals(entry.getKey())) { @@ -218,7 +186,7 @@ public void testEntryAndIterator() { } } assertEquals("changed", map.get("foo")); - assertEquals("external_gte", map.getVersionType()); + assertEquals("external_gte", md.getVersionType()); assertEquals(5, map.entrySet().size()); assertEquals(5, map.size()); @@ -235,7 +203,7 @@ public void testEntryAndIterator() { } } - assertNull(map.getVersionType()); + assertNull(md.getVersionType()); assertFalse(map.containsKey("baz")); assertTrue(map.containsKey("_version")); assertTrue(map.containsKey("foo")); @@ -247,7 +215,7 @@ public void testEntryAndIterator() { } public void testContainsValue() { - map = new IngestSourceAndMetadata(Map.of("myField", "fieldValue"), Map.of("_version", 5678), null, null); + map = new IngestSourceAndMetadata(Map.of("myField", "fieldValue"), new Metadata(Map.of("_version", 5678), null)); assertTrue(map.containsValue(5678)); assertFalse(map.containsValue(5679)); assertTrue(map.containsValue("fieldValue")); @@ -256,39 +224,40 @@ public void testContainsValue() { public void testValidators() { map = new IngestSourceAndMetadata("myIndex", "myId", 1234, "myRouting", VersionType.EXTERNAL, null, new HashMap<>()); + md = map.getMetadata(); IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> map.put("_index", 555)); assertEquals("_index must be null or a String but was [555] with type [java.lang.Integer]", err.getMessage()); - assertEquals("myIndex", map.getIndex()); + assertEquals("myIndex", md.getIndex()); err = expectThrows(IllegalArgumentException.class, () -> map.put("_id", 555)); assertEquals("_id must be null or a String but was [555] with type [java.lang.Integer]", err.getMessage()); - assertEquals("myId", map.getId()); + assertEquals("myId", md.getId()); map.put("_id", "myId2"); - assertEquals("myId2", map.getId()); + assertEquals("myId2", md.getId()); err = expectThrows(IllegalArgumentException.class, () -> map.put("_routing", 555)); assertEquals("_routing must be null or a String but was [555] with type [java.lang.Integer]", err.getMessage()); - assertEquals("myRouting", map.getRouting()); + assertEquals("myRouting", md.getRouting()); map.put("_routing", "myRouting2"); - assertEquals("myRouting2", map.getRouting()); + assertEquals("myRouting2", md.getRouting()); err = expectThrows(IllegalArgumentException.class, () -> map.put("_version", "five-five-five")); assertEquals( "_version may only be set to an int or a long but was [five-five-five] with type [java.lang.String]", err.getMessage() ); - assertEquals(1234, map.getVersion()); + assertEquals(1234, md.getVersion()); map.put("_version", 555); - assertEquals(555, map.getVersion()); + assertEquals(555, md.getVersion()); err = expectThrows(IllegalArgumentException.class, () -> map.put("_version_type", "vt")); assertEquals( "_version_type must be a null or one of [internal, external, external_gte] but was [vt] with type [java.lang.String]", err.getMessage() ); - assertEquals("external", map.getVersionType()); + assertEquals("external", md.getVersionType()); map.put("_version_type", "internal"); - assertEquals("internal", map.getVersionType()); + assertEquals("internal", md.getVersionType()); err = expectThrows(IllegalArgumentException.class, () -> map.put("_version_type", VersionType.EXTERNAL.toString())); assertEquals( "_version_type must be a null or one of [internal, external, external_gte] but was [EXTERNAL] with type [java.lang.String]", @@ -300,8 +269,8 @@ public void testValidators() { + " [org.elasticsearch.index.VersionType$2]", err.getMessage() ); - assertEquals("internal", map.getVersionType()); - err = expectThrows(IllegalArgumentException.class, () -> map.setVersionType(VersionType.EXTERNAL.toString())); + assertEquals("internal", md.getVersionType()); + err = expectThrows(IllegalArgumentException.class, () -> md.setVersionType(VersionType.EXTERNAL.toString())); assertEquals( "_version_type must be a null or one of [internal, external, external_gte] but was [EXTERNAL] with type [java.lang.String]", err.getMessage() @@ -311,33 +280,27 @@ public void testValidators() { assertEquals("_dynamic_templates must be a null or a Map but was [5] with type [java.lang.String]", err.getMessage()); Map dt = Map.of("a", "b"); map.put("_dynamic_templates", dt); - assertThat(dt, equalTo(map.getDynamicTemplates())); - } - - public void testDefaultValidatorForAllMetadata() { - for (IngestDocument.Metadata m : IngestDocument.Metadata.values()) { - assertThat(IngestSourceAndMetadata.VALIDATORS, hasEntry(equalTo(m.getFieldName()), notNullValue())); - } - assertEquals(IngestDocument.Metadata.values().length, IngestSourceAndMetadata.VALIDATORS.size()); + assertThat(dt, equalTo(md.getDynamicTemplates())); } public void testHandlesAllVersionTypes() { - Map md = new HashMap<>(); - md.put("_version", 1234); - map = new IngestSourceAndMetadata(new HashMap<>(), md, null, null); - assertNull(map.getVersionType()); + Map mdRawMap = new HashMap<>(); + mdRawMap.put("_version", 1234); + map = new IngestSourceAndMetadata(new HashMap<>(), new Metadata(mdRawMap, null)); + md = map.getMetadata(); + assertNull(md.getVersionType()); for (VersionType vt : VersionType.values()) { - map.setVersionType(VersionType.toString(vt)); + md.setVersionType(VersionType.toString(vt)); assertEquals(VersionType.toString(vt), map.get("_version_type")); } for (VersionType vt : VersionType.values()) { map.put("_version_type", VersionType.toString(vt)); - assertEquals(vt.toString().toLowerCase(Locale.ROOT), map.getVersionType()); + assertEquals(vt.toString().toLowerCase(Locale.ROOT), md.getVersionType()); } - map.setVersionType(null); - assertNull(map.getVersionType()); + md.setVersionType(null); + assertNull(md.getVersionType()); } private static class TestEntry implements Map.Entry { diff --git a/server/src/test/java/org/elasticsearch/script/MetadataTests.java b/server/src/test/java/org/elasticsearch/script/MetadataTests.java new file mode 100644 index 0000000000000..5469bd91ec9f4 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/script/MetadataTests.java @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.script; + +import org.elasticsearch.ingest.IngestDocument; +import org.elasticsearch.test.ESTestCase; + +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.notNullValue; + +public class MetadataTests extends ESTestCase { + Metadata md; + + public void testDefaultValidatorForAllMetadata() { + for (IngestDocument.Metadata m : IngestDocument.Metadata.values()) { + assertThat(Metadata.VALIDATORS, hasEntry(equalTo(m.getFieldName()), notNullValue())); + } + assertEquals(IngestDocument.Metadata.values().length, Metadata.VALIDATORS.size()); + } + + public void testGetString() { + Map metadata = new HashMap<>(); + metadata.put("a", "A"); + metadata.put("b", new Object() { + @Override + public String toString() { + return "myToString()"; + } + }); + metadata.put("c", null); + metadata.put("d", 1234); + md = new TestMetadata(metadata, allowAllValidators("a", "b", "c", "d")); + assertNull(md.getString("c")); + assertNull(md.getString("no key")); + assertEquals("myToString()", md.getString("b")); + assertEquals("A", md.getString("a")); + assertEquals("1234", md.getString("d")); + } + + public void testGetNumber() { + Map metadata = new HashMap<>(); + metadata.put("a", Long.MAX_VALUE); + metadata.put("b", Double.MAX_VALUE); + metadata.put("c", "NaN"); + metadata.put("d", null); + md = new TestMetadata(metadata, allowAllValidators("a", "b", "c", "d")); + assertEquals(Long.MAX_VALUE, md.getNumber("a")); + assertEquals(Double.MAX_VALUE, md.getNumber("b")); + IllegalStateException err = expectThrows(IllegalStateException.class, () -> md.getNumber("c")); + assertEquals("unexpected type for [c] with value [NaN], expected Number, got [java.lang.String]", err.getMessage()); + assertNull(md.getNumber("d")); + assertNull(md.getNumber("no key")); + } + + private static Map allowAllValidators(String... keys) { + Map validators = new HashMap<>(); + for (String key : keys) { + validators.put(key, (o, k, v) -> {}); + } + return validators; + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java b/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java index b6b6949a07290..ffd9ca324f63b 100644 --- a/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java +++ b/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java @@ -11,11 +11,12 @@ import org.elasticsearch.common.lucene.uid.Versions; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.VersionType; +import org.elasticsearch.script.Metadata; +import org.elasticsearch.script.TestMetadata; import org.elasticsearch.test.ESTestCase; import java.util.HashMap; import java.util.Map; -import java.util.function.BiConsumer; /** * Construct ingest documents for testing purposes @@ -36,10 +37,8 @@ public static IngestDocument withNullableVersion(Map sourceAndMe * _versions. Normally null _version is not allowed, but many tests don't care about that invariant. */ public static IngestDocument ofIngestWithNullableVersion(Map sourceAndMetadata, Map ingestMetadata) { - Map> validators = replaceValidator(VERSION, IngestSourceAndMetadata::longValidator); Tuple, Map> sm = IngestSourceAndMetadata.splitSourceAndMetadata(sourceAndMetadata); - IngestSourceAndMetadata withNullableVersion = new IngestSourceAndMetadata(sm.v1(), sm.v2(), null, validators); - return new IngestDocument(withNullableVersion, ingestMetadata); + return new IngestDocument(new IngestSourceAndMetadata(sm.v1(), TestMetadata.withNullableVersion(sm.v2())), ingestMetadata); } /** @@ -53,22 +52,12 @@ public static IngestDocument withDefaultVersion(Map sourceAndMet return new IngestDocument(sourceAndMetadata, new HashMap<>()); } - /** - * Return the default validator map with a single validator replaced, if that validator was already present in the default validators - * map - */ - protected static Map> replaceValidator(String key, BiConsumer validator) { - Map> validators = new HashMap<>(IngestSourceAndMetadata.VALIDATORS); - validators.computeIfPresent(key, (k, v) -> validator); - return validators; - } - /** * Create an IngestDocument with a metadata map and validators. The metadata map is passed by reference, not copied, so callers * can observe changes to the map directly. */ - public static IngestDocument ofMetadataWithValidator(Map metadata, Map> validators) { - return new IngestDocument(new IngestSourceAndMetadata(new HashMap<>(), metadata, null, validators), new HashMap<>()); + public static IngestDocument ofMetadataWithValidator(Map metadata, Map validators) { + return new IngestDocument(new IngestSourceAndMetadata(new HashMap<>(), new TestMetadata(metadata, validators)), new HashMap<>()); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/script/TestMetadata.java b/test/framework/src/main/java/org/elasticsearch/script/TestMetadata.java new file mode 100644 index 0000000000000..87a45ea857549 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/script/TestMetadata.java @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.script; + +import java.util.HashMap; +import java.util.Map; + +/** + * An implementation of {@link Metadata} with customizable {@link org.elasticsearch.script.Metadata.Validator}s for use in testing. + */ +public class TestMetadata extends Metadata { + public TestMetadata(Map map, Map validators) { + super(map, null, validators); + } + + public static TestMetadata withNullableVersion(Map map) { + Map updatedValidators = new HashMap<>(VALIDATORS); + updatedValidators.replace(VERSION, Metadata::longValidator); + return new TestMetadata(map, updatedValidators); + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/inference/results/InferenceResultsTestCase.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/inference/results/InferenceResultsTestCase.java index 1663c63eeeae3..58e58bd9eb1f0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/inference/results/InferenceResultsTestCase.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/inference/results/InferenceResultsTestCase.java @@ -55,14 +55,15 @@ public void testWriteToDocAndSerialize() throws IOException { InferenceResults.writeResult(inferenceResult, document, parentField, modelId); try (XContentBuilder builder = XContentFactory.jsonBuilder()) { builder.startObject(); - Map metadataMap = document.getMetadataMap(); - for (Map.Entry metadata : metadataMap.entrySet()) { - if (metadata.getValue() != null) { - builder.field(metadata.getKey(), metadata.getValue().toString()); + org.elasticsearch.script.Metadata metadata = document.getMetadata(); + for (String key : metadata.keySet()) { + Object value = metadata.get(key); + if (value != null) { + builder.field(key, value.toString()); } } Map source = IngestDocument.deepCopyMap(document.getSourceAndMetadata()); - metadataMap.keySet().forEach(mD -> source.remove(mD)); + metadata.keySet().forEach(source::remove); builder.field("_source", source); builder.field("_ingest", document.getIngestMetadata()); builder.endObject();