Skip to content

Commit

Permalink
Use deep equality when comparing IngestDocument equality
Browse files Browse the repository at this point in the history
This commit changes the `IngestDocument` class equality check to use "deep" equality. Previously we would end up doing the equivalent of `Objects.equals(thisMap, thatMap)` for the document. If a script or processor ended up writing any primitive arrays however, then those would fail the equality check because instance equality was used instead of check the actual values.
  • Loading branch information
dakrone committed Jun 28, 2023
1 parent f9b22a9 commit bd30fb4
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ public boolean equals(Object obj) {
}

IngestDocument other = (IngestDocument) obj;
return Objects.equals(ctxMap, other.ctxMap) && Objects.equals(ingestMetadata, other.ingestMetadata);
return Objects.equals(ctxMap, other.ctxMap) && Maps.deepEquals(ingestMetadata, other.ingestMetadata);
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/script/CtxMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.script;

import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.set.Sets;

import java.util.AbstractCollection;
Expand Down Expand Up @@ -336,9 +337,9 @@ public Object setValue(Object value) {
public boolean equals(Object o) {
if (this == o) return true;
if ((o instanceof CtxMap) == false) return false;
if (super.equals(o) == false) return false;
CtxMap<?> ctxMap = (CtxMap<?>) o;
return source.equals(ctxMap.source) && metadata.equals(ctxMap.metadata);
if (Maps.deepEquals(this, ctxMap) == false) return false;
return Maps.deepEquals(source, ctxMap.source) && metadata.equals(ctxMap.metadata);
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion server/src/main/java/org/elasticsearch/script/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.script;

import org.elasticsearch.common.util.Maps;

import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
Expand Down Expand Up @@ -279,7 +281,7 @@ public boolean equals(Object o) {
if (this == o) return true;
if ((o instanceof Metadata) == false) return false;
Metadata metadata = (Metadata) o;
return map.equals(metadata.map);
return Maps.deepEquals(map, metadata.map);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1158,4 +1158,16 @@ public void testIndexHistory() {
assertFalse(ingestDocument.updateIndexHistory(index1));
assertThat(ingestDocument.getIndexHistory(), Matchers.contains(index1, index2));
}

public void testEqualsAndHashCodeWithArray() {
// Test that equality still works when the ingest document uses primitive arrays,
// since normal .equals() methods would not work for Maps containing these arrays.
byte[] numbers = new byte[] { 0, 1, 2 };
ingestDocument.setFieldValue("somearray", numbers);
IngestDocument copy = new IngestDocument(ingestDocument);
byte[] copiedNumbers = copy.getFieldValue("somearray", byte[].class);
assertArrayEquals(numbers, copiedNumbers);
assertNotEquals(numbers, copiedNumbers);
assertThat(copy, equalTo(ingestDocument));
}
}

0 comments on commit bd30fb4

Please sign in to comment.