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

Ensure fixed serialization order of InnerHitBuilder (#22820) 

Usually the order in which we serialize sets and maps of things doesn't matter,
but since InnerHitBuilder is part of SearchSourceBuilder, which is in turn used
as a cache key in its bytes serialization, we need to ensure the order of all
these fields when writing them to an output stream.

This adds tests and makes sure we iterate over the scriptField set and the
childInnerHits map in a fixed order.

Closes #22808
This commit is contained in:
Christoph Büscher 2017-01-30 19:28:55 +01:00 committed by GitHub
parent b28ef3063e
commit 4e613139dc
2 changed files with 36 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java
View File

@ -48,6 +48,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@ -266,8 +267,10 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl
out.writeBoolean(hasScriptFields);
if (hasScriptFields) {
out.writeVInt(scriptFields.size());
for (ScriptField scriptField : scriptFields) {
scriptField.writeTo(out);
Iterator<ScriptField> iterator = scriptFields.stream()
.sorted((a, b) -> a.fieldName().compareTo(b.fieldName())).iterator();
while (iterator.hasNext()) {
iterator.next().writeTo(out);
}
}
out.writeOptionalWriteable(fetchSourceContext);
@ -285,7 +288,10 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl
out.writeBoolean(hasChildInnerHits);
if (hasChildInnerHits) {
out.writeVInt(childInnerHits.size());
for (Map.Entry<String, InnerHitBuilder> entry : childInnerHits.entrySet()) {
Iterator<Map.Entry<String, InnerHitBuilder>> iterator = childInnerHits.entrySet().stream()
.sorted((a, b) -> a.getKey().compareTo(b.getKey())).iterator();
while (iterator.hasNext()) {
Map.Entry<String, InnerHitBuilder> entry = iterator.next();
out.writeString(entry.getKey());
entry.getValue().writeTo(out);
}

28
core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.index.query;
import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@ -36,6 +37,7 @@ import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
import org.elasticsearch.search.fetch.subphase.InnerHitsContext;
import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilderTests;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.internal.ShardSearchLocalRequest;
import org.elasticsearch.search.sort.SortBuilder;
import org.elasticsearch.search.sort.SortBuilders;
import org.elasticsearch.search.sort.SortOrder;
@ -95,6 +97,30 @@ public class InnerHitBuilderTests extends ESTestCase {
}
}
/**
* Test that if we serialize and deserialize an object, further
* serialization leads to identical bytes representation.
*
* This is necessary to ensure because we use the serialized BytesReference
* of this builder as part of the cacheKey in
* {@link ShardSearchLocalRequest} (via
* {@link SearchSourceBuilder#collapse(org.elasticsearch.search.collapse.CollapseBuilder)})
*/
public void testSerializationOrder() throws Exception {
for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
InnerHitBuilder original = randomInnerHits();
InnerHitBuilder deserialized = serializedCopy(original);
assertEquals(deserialized, original);
assertEquals(deserialized.hashCode(), original.hashCode());
assertNotSame(deserialized, original);
BytesStreamOutput out1 = new BytesStreamOutput();
BytesStreamOutput out2 = new BytesStreamOutput();
original.writeTo(out1);
deserialized.writeTo(out2);
assertEquals(out1.bytes(), out2.bytes());
}
}
public void testFromAndToXContent() throws Exception {
for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
InnerHitBuilder innerHit = randomInnerHits(true, false);
@ -114,7 +140,7 @@ public class InnerHitBuilderTests extends ESTestCase {
}
}
public void testEqualsAndHashcode() throws IOException {
public void testEqualsAndHashcode() {
for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
checkEqualsAndHashCode(randomInnerHits(), InnerHitBuilderTests::serializedCopy, InnerHitBuilderTests::mutate);
}