Skip to content

Commit aefa20f

Browse files
nik9000astefan
authored andcommitted
Fix concurrent serialization test for StoredScript (elastic#82278)
This fixes the concurrent toXContent tests for `StoredScript`. It turns out that `StoredScript`'s tests are the only tests to encode a very long string and `SMILE`'s xcontent serialization has unpredictable behavior for long strings. It'll always produce a valid encoding, but sometimes long strings will be output with the marker for utf-8 instead of ascii. That causes the bytes comparison that the concurrent serialization tests. So we never test with SMILE here. That's ok. The test is mostly about concurrency, not xcontent type. Also! The error message was quite difficult to debug here. So I improved it while I was there. Closes elastic#82257
1 parent 212bbd0 commit aefa20f

File tree

2 files changed

+26
-9
lines changed

2 files changed

+26
-9
lines changed

server/src/main/java/org/elasticsearch/script/StoredScriptSource.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import org.elasticsearch.common.io.stream.StreamInput;
1818
import org.elasticsearch.common.io.stream.StreamOutput;
1919
import org.elasticsearch.common.io.stream.Writeable;
20-
import org.elasticsearch.common.logging.DeprecationLogger;
2120
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
2221
import org.elasticsearch.xcontent.NamedXContentRegistry;
2322
import org.elasticsearch.xcontent.ObjectParser;
@@ -43,12 +42,6 @@
4342
* saved in the {@link ClusterState}.
4443
*/
4544
public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> implements Writeable, ToXContentObject {
46-
47-
/**
48-
* Standard deprecation logger for used to deprecate allowance of empty templates.
49-
*/
50-
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(StoredScriptSource.class);
51-
5245
/**
5346
* Standard {@link ParseField} for outer level of stored script source.
5447
*/

test/framework/src/main/java/org/elasticsearch/test/AbstractSerializingTestCase.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.elasticsearch.xcontent.XContentBuilder;
1717
import org.elasticsearch.xcontent.XContentParser;
1818
import org.elasticsearch.xcontent.XContentType;
19+
import org.elasticsearch.xcontent.cbor.CborXContent;
20+
import org.elasticsearch.xcontent.smile.SmileXContent;
1921

2022
import java.io.IOException;
2123
import java.time.Instant;
@@ -53,7 +55,16 @@ public final void testFromXContent() throws IOException {
5355
* concurrently.
5456
*/
5557
public final void testConcurrentToXContent() throws IOException, InterruptedException, ExecutionException {
56-
XContentType xContentType = randomFrom(XContentType.values());
58+
XContentType xContentType = randomValueOtherThanMany(
59+
/*
60+
* SMILE will sometimes use the unicode marker for ascii strings
61+
* if the it's internal buffer doeesn't have enough space for the
62+
* whole string. That's fine for SMILE readers, but we're comparing
63+
* bytes here so we can't use it.
64+
*/
65+
type -> type.xContent() == SmileXContent.smileXContent,
66+
() -> randomFrom(XContentType.values())
67+
);
5768
T testInstance = createXContextTestInstance(xContentType);
5869
ToXContent.Params params = new ToXContent.DelegatingMapParams(
5970
singletonMap(RestSearchAction.TYPED_KEYS_PARAM, "true"),
@@ -71,7 +82,20 @@ public final void testConcurrentToXContent() throws IOException, InterruptedExce
7182
concurrentTest(() -> {
7283
try {
7384
for (int r = 0; r < rounds; r++) {
74-
assertEquals(firstTimeBytes, toXContent(testInstance, xContentType, params, humanReadable).toBytesRef());
85+
BytesRef thisRoundBytes = toXContent(testInstance, xContentType, params, humanReadable).toBytesRef();
86+
if (firstTimeBytes.bytesEquals(thisRoundBytes)) {
87+
continue;
88+
}
89+
StringBuilder error = new StringBuilder("Failed to round trip over ");
90+
if (humanReadable) {
91+
error.append("human readable ");
92+
}
93+
error.append(xContentType);
94+
error.append("\nCanonical is:\n").append(Strings.toString(testInstance, true, true));
95+
boolean showBytes = xContentType.xContent() == CborXContent.cborXContent;
96+
error.append("\nWanted : ").append(showBytes ? firstTimeBytes : firstTimeBytes.utf8ToString());
97+
error.append("\nBut got: ").append(showBytes ? thisRoundBytes : thisRoundBytes.utf8ToString());
98+
fail(error.toString());
7599
}
76100
} catch (IOException e) {
77101
throw new AssertionError(e);

0 commit comments

Comments
 (0)