From 51c7da781060749dd59c784b7167248fb34e8408 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Tue, 17 Feb 2026 11:44:27 +0000 Subject: [PATCH 1/7] Add tests for PaginatedHitSource Adds tests for the subclasses of PaginatedHitSource, including NasicHit, Response, SearchFailure unit tests and Search Failure wire serialisation tests --- .../index/reindex/PaginatedHitSource.java | 32 ++++ .../paginatedhitsource/BasicHitTests.java | 127 +++++++++++++ .../paginatedhitsource/ResponseTests.java | 167 ++++++++++++++++++ .../SearchFailureTests.java | 140 +++++++++++++++ .../SearchFailureWireSerialisationTests.java | 120 +++++++++++++ 5 files changed, 586 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java create mode 100644 server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java create mode 100644 server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java create mode 100644 server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java diff --git a/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java b/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java index 5e795f7bad5ba..45fd7890293a0 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java @@ -34,6 +34,7 @@ import java.io.IOException; import java.util.List; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -481,5 +482,36 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public String toString() { return Strings.toString(this); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SearchFailure that = (SearchFailure) o; + if (!Objects.equals(index, that.index)) return false; + if (!Objects.equals(shardId, that.shardId)) return false; + if (!Objects.equals(nodeId, that.nodeId)) return false; + if (status != that.status) return false; + // Compare Throwable meaningfully, not by identity + if (reason == null && that.reason == null) { + return true; + } + if (reason == null || that.reason == null) { + return false; + } + return reason.getClass().equals(that.reason.getClass()) + && Objects.equals(reason.getMessage(), that.reason.getMessage()); + } + @Override + public int hashCode() { + return Objects.hash( + index, + shardId, + nodeId, + status, + reason.getClass(), + reason.getMessage() + ); + } } } diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java new file mode 100644 index 0000000000000..4e7d6c3a4d85f --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java @@ -0,0 +1,127 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.reindex.paginatedhitsource; + +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.index.reindex.PaginatedHitSource; +import org.elasticsearch.index.reindex.PaginatedHitSource.BasicHit; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.XContentType; + + +public class BasicHitTests extends ESTestCase { + + /** + * Verifies that index, id, and version provided at construction are returned unchanged by their respective getters. + * Verifies that optional fields are null or zero by default before any setters are invoked. + */ + public void testConstructor() { + String index = randomAlphaOfLengthBetween(3, 10); + String id = randomAlphaOfLengthBetween(3, 10); + long version = randomNonNegativeLong(); + PaginatedHitSource.BasicHit hit = new BasicHit(index, id, version); + + assertEquals(index, hit.getIndex()); + assertEquals(id, hit.getId()); + assertEquals(version, hit.getVersion()); + + assertNull(hit.getSource()); + assertNull(hit.getXContentType()); + assertNull(hit.getRouting()); + assertEquals(0L, hit.getSeqNo()); + assertEquals(0L, hit.getPrimaryTerm()); + } + + /** + * Verifies that setSource correctly sets both the source bytes and the associated XContentType. + * Verifies that setSource returns the same instance, allowing fluent-style method chaining. + */ + public void testSetSource() { + BasicHit hit = new BasicHit( + randomAlphaOfLengthBetween(3, 10), + randomAlphaOfLengthBetween(3, 10), + randomNonNegativeLong() + ); + BytesReference source = new BytesArray(randomAlphaOfLengthBetween(5, 50)); + XContentType xContentType = randomFrom(XContentType.values()); + + BasicHit returned = hit.setSource(source, xContentType); + assertSame(source, hit.getSource()); + assertEquals(xContentType, hit.getXContentType()); + assertSame(hit, returned); + } + + /** + * Verifies that routing can be set and retrieved correctly. + * Verifies that setRouting returns the same instance, allowing fluent-style chaining. + */ + public void testSetRouting() { + BasicHit hit = new BasicHit( + randomAlphaOfLengthBetween(3, 10), + randomAlphaOfLengthBetween(3, 10), + randomNonNegativeLong() + ); + String routing = randomAlphaOfLengthBetween(3, 20); + BasicHit returned = hit.setRouting(routing); + assertEquals(routing, hit.getRouting()); + assertSame(hit, returned); + } + + /** + * Verifies that sequence number can be set and retrieved correctly. + */ + public void testSetSeqNo() { + BasicHit hit = new BasicHit( + randomAlphaOfLengthBetween(3, 10), + randomAlphaOfLengthBetween(3, 10), + randomNonNegativeLong() + ); + long seqNo = randomNonNegativeLong(); + hit.setSeqNo(seqNo); + assertEquals(seqNo, hit.getSeqNo()); + } + + /** + * Verifies that primary term can be set and retrieved correctly. + */ + public void testSetPrimaryTerm() { + BasicHit hit = new BasicHit( + randomAlphaOfLengthBetween(3, 10), + randomAlphaOfLengthBetween(3, 10), + randomNonNegativeLong() + ); + long primaryTerm = randomNonNegativeLong(); + hit.setPrimaryTerm(primaryTerm); + assertEquals(primaryTerm, hit.getPrimaryTerm()); + } + + /** + * Verifies that setting all optional fields does not affect the required constructor-provided fields. + */ + public void testOptionalSettersDoNotAffectRequiredFields() { + String index = randomAlphaOfLengthBetween(3, 10); + String id = randomAlphaOfLengthBetween(3, 10); + long version = randomNonNegativeLong(); + BasicHit hit = new BasicHit(index, id, version); + + hit.setRouting(randomAlphaOfLengthBetween(3, 20)); + hit.setSeqNo(randomNonNegativeLong()); + hit.setPrimaryTerm(randomNonNegativeLong()); + hit.setSource( + new BytesArray(randomAlphaOfLengthBetween(5, 50)), + randomFrom(XContentType.values()) + ); + + assertEquals(index, hit.getIndex()); + assertEquals(id, hit.getId()); + assertEquals(version, hit.getVersion()); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java new file mode 100644 index 0000000000000..d4baef4bd9857 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java @@ -0,0 +1,167 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.reindex.paginatedhitsource; + +import org.elasticsearch.index.reindex.PaginatedHitSource; +import org.elasticsearch.index.reindex.PaginatedHitSource.BasicHit; +import org.elasticsearch.index.reindex.PaginatedHitSource.Hit; +import org.elasticsearch.index.reindex.PaginatedHitSource.Response; +import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; +import org.elasticsearch.test.ESTestCase; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + + +public class ResponseTests extends ESTestCase { + + /** + * Verifies that all values provided to the constructor are returned unchanged by their respective getters. + */ + public void testConstructor() { + boolean timedOut = randomBoolean(); + List failures = randomFailures(); + long totalHits = randomNonNegativeLong(); + List hits = randomHits(); + String scrollId = randomAlphaOfLengthBetween(3, 20); + Response response = new Response(timedOut, failures, totalHits, hits, scrollId); + + assertEquals(timedOut, response.isTimedOut()); + assertSame(failures, response.getFailures()); + assertEquals(totalHits, response.getTotalHits()); + assertSame(hits, response.getHits()); + assertEquals(scrollId, response.getScrollId()); + } + + /** + * Verifies that the response correctly reports when it has timed out. + */ + public void testIsTimedOut() { + boolean timedOut = randomBoolean(); + Response response = new Response( + timedOut, + Collections.emptyList(), + randomNonNegativeLong(), + Collections.emptyList(), + randomAlphaOfLengthBetween(3, 20) + ); + assertEquals(timedOut, response.isTimedOut()); + } + + /** + * Verifies that a (possibly empty) failures list is returned unchanged, preserving all failure entries + */ + public void testGetFailures() { + List failures = randomBoolean() ? Collections.emptyList() : randomFailures(); + Response response = new Response( + randomBoolean(), + failures, + randomNonNegativeLong(), + Collections.emptyList(), + randomAlphaOfLengthBetween(3, 20) + ); + assertSame(failures, response.getFailures()); + assertEquals(failures.size(), response.getFailures().size()); + } + + /** + * Verifies that totalHits returned by the getter matches the value provided at construction time. + */ + public void testGetTotalHits() { + long totalHits = randomNonNegativeLong(); + Response response = new Response( + randomBoolean(), + Collections.emptyList(), + totalHits, + Collections.emptyList(), + randomAlphaOfLengthBetween(3, 20) + ); + assertEquals(totalHits, response.getTotalHits()); + } + + /** + * Verifies that a (possible empty) hits list is returned unchanged and preserves all hit entries. + */ + public void testEmptyHitsList() { + List hits = randomBoolean() ? Collections.emptyList() : randomHits(); + Response response = new Response( + randomBoolean(), + Collections.emptyList(), + randomNonNegativeLong(), + hits, + randomAlphaOfLengthBetween(3, 20) + ); + assertSame(hits, response.getHits()); + assertEquals(hits.size(), response.getHits().size()); + } + + /** + * Verifies that the scroll id returned by the getter matches the value provided at construction time. + */ + public void testGetScrollId() { + String scrollId = randomAlphaOfLengthBetween(5, 30); + Response response = new Response( + randomBoolean(), + Collections.emptyList(), + randomNonNegativeLong(), + Collections.emptyList(), + scrollId + ); + assertEquals(scrollId, response.getScrollId()); + } + + /** + * Verifies that providing null values for optional collections is preserved and returned as-is by the getters. + */ + public void testNullCollectionsArePreserved() { + List failures = null; + List hits = null; + Response response = new Response( + randomBoolean(), + failures, + randomNonNegativeLong(), + hits, + randomAlphaOfLengthBetween(3, 20) + ); + assertNull(response.getFailures()); + assertNull(response.getHits()); + } + + private static List randomFailures() { + int size = randomIntBetween(1, 5); + List failures = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + failures.add( + new SearchFailure( + new IllegalStateException(randomAlphaOfLengthBetween(5, 20)), + randomBoolean() ? randomAlphaOfLengthBetween(3, 10) : null, + randomBoolean() ? randomIntBetween(0, 10) : null, + randomBoolean() ? randomAlphaOfLengthBetween(3, 10) : null + ) + ); + } + return failures; + } + + private static List randomHits() { + int size = randomIntBetween(1, 5); + List hits = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + hits.add( + new BasicHit( + randomAlphaOfLengthBetween(3, 10), + randomAlphaOfLengthBetween(3, 10), + randomNonNegativeLong() + ) + ); + } + return hits; + } +} diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java new file mode 100644 index 0000000000000..83da58def4b8e --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java @@ -0,0 +1,140 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.reindex.paginatedhitsource; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.ESTestCase; +import java.util.Map; +import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; +import org.elasticsearch.xcontent.XContentType; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; + +public class SearchFailureTests extends ESTestCase { + + public void testConstructorWithReasonOnly() { + Throwable reason = randomException(); + SearchFailure failure = new SearchFailure(reason); + assertSame(reason, failure.getReason()); + assertNull(failure.getIndex()); + assertNull(failure.getShardId()); + assertNull(failure.getNodeId()); + assertEquals(ExceptionsHelper.status(reason), failure.getStatus()); + } + + public void testConstructorWithAllFields() { + Throwable reason = randomException(); + String index = randomAlphaOfLengthBetween(3, 10); + Integer shardId = randomIntBetween(0, 100); + String nodeId = randomAlphaOfLengthBetween(3, 10); + SearchFailure failure = new SearchFailure(reason, index, shardId, nodeId); + assertSame(reason, failure.getReason()); + assertEquals(index, failure.getIndex()); + assertEquals(shardId, failure.getShardId()); + assertEquals(nodeId, failure.getNodeId()); + assertEquals(ExceptionsHelper.status(reason), failure.getStatus()); + } + + public void testEqualsAndHashCode() { + // Hardcode the exception to match the rest status + Throwable reason = new IllegalStateException("Exception"); + String index = randomAlphaOfLengthBetween(3, 10); + Integer shardId = randomIntBetween(0, 100); + String nodeId = randomAlphaOfLengthBetween(3, 10); + SearchFailure failure1 = new SearchFailure(reason, index, shardId, nodeId); + SearchFailure failure2 = new SearchFailure( + reason, + index, + shardId, + nodeId, + RestStatus.INTERNAL_SERVER_ERROR + ); + assertEquals(failure1, failure2); + assertEquals(failure1.hashCode(), failure2.hashCode()); + } + + public void testEqualsIsFalseForDifferentFields() { + Throwable reason = randomException(); + String index = randomAlphaOfLengthBetween(3, 10); + Integer shardId = randomIntBetween(0, 100); + String nodeId = randomAlphaOfLengthBetween(3, 10); + SearchFailure base = new SearchFailure(reason, index, shardId, nodeId); + + assertNotEquals(base, new SearchFailure(reason, randomValueOtherThan(index, () -> randomAlphaOfLengthBetween(3, 10)), shardId, nodeId)); + assertNotEquals(base, new SearchFailure(reason, index, randomValueOtherThan(shardId, () -> randomIntBetween(0, 100)), nodeId)); + assertNotEquals(base, new SearchFailure(reason, index, shardId, randomValueOtherThan(nodeId, () -> randomAlphaOfLengthBetween(3, 10)))); + } + + public void testEqualsIsFalseForDifferentExceptionTypeOrMessage() { + SearchFailure f1 = new SearchFailure(new IllegalStateException("exception_1")); + SearchFailure f2 = new SearchFailure(new IllegalArgumentException("exception_1")); + SearchFailure f3 = new SearchFailure(new IllegalStateException("exception_2")); + assertNotEquals(f1, f2); + assertNotEquals(f1, f3); + } + + public void testToXContentIncludesExpectedFields() { + String message = randomAlphaOfLengthBetween(1, 20); + Throwable reason = randomException(message); + String index = randomAlphaOfLengthBetween(3, 10); + Integer shardId = randomIntBetween(0, 10); + String nodeId = randomAlphaOfLengthBetween(3, 10); + SearchFailure failure = new SearchFailure(reason, index, shardId, nodeId); + String json = Strings.toString(failure); + Map map = XContentHelper.convertToMap( + XContentType.JSON.xContent(), + json, + false + ); + assertThat(map.get(SearchFailure.INDEX_FIELD), equalTo(index)); + assertThat(map.get(SearchFailure.SHARD_FIELD), equalTo(shardId)); + assertThat(map.get(SearchFailure.NODE_FIELD), equalTo(nodeId)); + assertThat(map.get(SearchFailure.STATUS_FIELD), equalTo(failure.getStatus().getStatus())); + assertThat(map, hasKey(SearchFailure.REASON_FIELD)); + @SuppressWarnings("unchecked") + Map reasonMap = (Map) map.get(SearchFailure.REASON_FIELD); + assertThat(reasonMap.get("type"), notNullValue()); + assertThat(reasonMap.get("reason"), equalTo(message)); + } + + public void testToXContentOmitsNullOptionalFields() { + SearchFailure failure = new SearchFailure(randomException()); + String json = Strings.toString(failure); + Map map = XContentHelper.convertToMap( + XContentType.JSON.xContent(), + json, + false + ); + assertThat(map, not(hasKey(SearchFailure.INDEX_FIELD))); + assertThat(map, not(hasKey(SearchFailure.SHARD_FIELD))); + assertThat(map, not(hasKey(SearchFailure.NODE_FIELD))); + assertThat(map, hasKey(SearchFailure.STATUS_FIELD)); + assertThat(map, hasKey(SearchFailure.REASON_FIELD)); + } + + public static Throwable randomException() { + return randomException(randomAlphaOfLengthBetween(1, 20)); + } + + public static Throwable randomException(String message) { + return randomFrom( + new IllegalArgumentException(message), + new IllegalStateException(message), + new ElasticsearchException(message) + ); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java new file mode 100644 index 0000000000000..313bce012e7f7 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java @@ -0,0 +1,120 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.reindex.paginatedhitsource; + +import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.test.AbstractWireSerializingTestCase; +import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; + +import static org.elasticsearch.index.reindex.paginatedhitsource.SearchFailureTests.randomException; + +public class SearchFailureWireSerialisationTests extends AbstractWireSerializingTestCase { + + @Override + protected SearchFailure createTestInstance() { + Throwable reason = randomException(); + String index = randomBoolean() ? randomAlphaOfLengthBetween(1, 10) : null; + Integer shardId = randomBoolean() ? randomIntBetween(0, 100) : null; + String nodeId = randomBoolean() ? randomAlphaOfLengthBetween(1, 10) : null; + return new SearchFailure(reason, index, shardId, nodeId); + } + + @Override + protected Writeable.Reader instanceReader() { + return SearchFailure::new; + } + + @Override + protected SearchFailure mutateInstance(SearchFailure instance) { + return mutateSearchFailure(instance); + } + + /** + * {@link SearchFailure} contains a {@code Throwable reason}. + * While the XContent output looks identical, the exception instances are not semantically identical after deserialization. + * Therefore, we must provide our own equality override to verify that the exceptions are meaningfully identical, even if the instance + * equality check fails. + */ + @Override + protected void assertEqualInstances(SearchFailure expected, SearchFailure actual) { + assertEquals(expected.getIndex(), actual.getIndex()); + assertEquals(expected.getShardId(), actual.getShardId()); + assertEquals(expected.getNodeId(), actual.getNodeId()); + assertEquals(expected.getStatus(), actual.getStatus()); + + // Compare exception meaningfully, not by identity + assertEquals( + expected.getReason().getClass(), + actual.getReason().getClass() + ); + assertEquals( + expected.getReason().getMessage(), + actual.getReason().getMessage() + ); + } + + public static SearchFailure mutateSearchFailure(SearchFailure instance) { + int fieldToMutate = randomIntBetween(0, 3); + switch (fieldToMutate) { + case 0 -> { + Throwable newReason; + do { + newReason = randomException(); + } while (newReason.getClass().equals(instance.getReason().getClass()) + && String.valueOf(newReason.getMessage()).equals(String.valueOf(instance.getReason().getMessage()))); + return new SearchFailure( + newReason, + instance.getIndex(), + instance.getShardId(), + instance.getNodeId(), + ExceptionsHelper.status(newReason) + ); + } + case 1 -> { + String newIndex = instance.getIndex() == null + ? randomAlphaOfLengthBetween(1, 10) + : randomValueOtherThan(instance.getIndex(), () -> randomAlphaOfLengthBetween(1, 10)); + return new SearchFailure( + instance.getReason(), + newIndex, + instance.getShardId(), + instance.getNodeId(), + instance.getStatus() + ); + } + case 2 -> { + Integer newShardId = instance.getShardId() == null + ? randomIntBetween(0, 100) + : randomValueOtherThan(instance.getShardId(), () -> randomIntBetween(0, 100)); + return new SearchFailure( + instance.getReason(), + instance.getIndex(), + newShardId, + instance.getNodeId(), + instance.getStatus() + ); + } + case 3 -> { + String newNodeId = instance.getNodeId() == null + ? randomAlphaOfLengthBetween(1, 10) + : randomValueOtherThan(instance.getNodeId(), () -> randomAlphaOfLengthBetween(1, 10)); + return new SearchFailure( + instance.getReason(), + instance.getIndex(), + instance.getShardId(), + newNodeId, + instance.getStatus() + ); + } + default -> throw new AssertionError("Unknown field index [" + fieldToMutate + "]"); + } + } +} From ea85c301705540a4d34f4c8c83a32db931ca086c Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Tue, 17 Feb 2026 11:47:19 +0000 Subject: [PATCH 2/7] Spotless apply --- .../index/reindex/PaginatedHitSource.java | 13 ++---- .../paginatedhitsource/BasicHitTests.java | 30 +++----------- .../paginatedhitsource/ResponseTests.java | 18 ++------ .../SearchFailureTests.java | 41 +++++++------------ .../SearchFailureWireSerialisationTests.java | 36 +++------------- 5 files changed, 32 insertions(+), 106 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java b/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java index 45fd7890293a0..2667f5be47f56 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java @@ -499,19 +499,12 @@ public boolean equals(Object o) { if (reason == null || that.reason == null) { return false; } - return reason.getClass().equals(that.reason.getClass()) - && Objects.equals(reason.getMessage(), that.reason.getMessage()); + return reason.getClass().equals(that.reason.getClass()) && Objects.equals(reason.getMessage(), that.reason.getMessage()); } + @Override public int hashCode() { - return Objects.hash( - index, - shardId, - nodeId, - status, - reason.getClass(), - reason.getMessage() - ); + return Objects.hash(index, shardId, nodeId, status, reason.getClass(), reason.getMessage()); } } } diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java index 4e7d6c3a4d85f..3f613a4a76bc3 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java @@ -16,7 +16,6 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentType; - public class BasicHitTests extends ESTestCase { /** @@ -45,11 +44,7 @@ public void testConstructor() { * Verifies that setSource returns the same instance, allowing fluent-style method chaining. */ public void testSetSource() { - BasicHit hit = new BasicHit( - randomAlphaOfLengthBetween(3, 10), - randomAlphaOfLengthBetween(3, 10), - randomNonNegativeLong() - ); + BasicHit hit = new BasicHit(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomNonNegativeLong()); BytesReference source = new BytesArray(randomAlphaOfLengthBetween(5, 50)); XContentType xContentType = randomFrom(XContentType.values()); @@ -64,11 +59,7 @@ public void testSetSource() { * Verifies that setRouting returns the same instance, allowing fluent-style chaining. */ public void testSetRouting() { - BasicHit hit = new BasicHit( - randomAlphaOfLengthBetween(3, 10), - randomAlphaOfLengthBetween(3, 10), - randomNonNegativeLong() - ); + BasicHit hit = new BasicHit(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomNonNegativeLong()); String routing = randomAlphaOfLengthBetween(3, 20); BasicHit returned = hit.setRouting(routing); assertEquals(routing, hit.getRouting()); @@ -79,11 +70,7 @@ public void testSetRouting() { * Verifies that sequence number can be set and retrieved correctly. */ public void testSetSeqNo() { - BasicHit hit = new BasicHit( - randomAlphaOfLengthBetween(3, 10), - randomAlphaOfLengthBetween(3, 10), - randomNonNegativeLong() - ); + BasicHit hit = new BasicHit(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomNonNegativeLong()); long seqNo = randomNonNegativeLong(); hit.setSeqNo(seqNo); assertEquals(seqNo, hit.getSeqNo()); @@ -93,11 +80,7 @@ public void testSetSeqNo() { * Verifies that primary term can be set and retrieved correctly. */ public void testSetPrimaryTerm() { - BasicHit hit = new BasicHit( - randomAlphaOfLengthBetween(3, 10), - randomAlphaOfLengthBetween(3, 10), - randomNonNegativeLong() - ); + BasicHit hit = new BasicHit(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomNonNegativeLong()); long primaryTerm = randomNonNegativeLong(); hit.setPrimaryTerm(primaryTerm); assertEquals(primaryTerm, hit.getPrimaryTerm()); @@ -115,10 +98,7 @@ public void testOptionalSettersDoNotAffectRequiredFields() { hit.setRouting(randomAlphaOfLengthBetween(3, 20)); hit.setSeqNo(randomNonNegativeLong()); hit.setPrimaryTerm(randomNonNegativeLong()); - hit.setSource( - new BytesArray(randomAlphaOfLengthBetween(5, 50)), - randomFrom(XContentType.values()) - ); + hit.setSource(new BytesArray(randomAlphaOfLengthBetween(5, 50)), randomFrom(XContentType.values())); assertEquals(index, hit.getIndex()); assertEquals(id, hit.getId()); diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java index d4baef4bd9857..48171aa6aecf4 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java @@ -15,11 +15,11 @@ import org.elasticsearch.index.reindex.PaginatedHitSource.Response; import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; import org.elasticsearch.test.ESTestCase; + import java.util.ArrayList; import java.util.Collections; import java.util.List; - public class ResponseTests extends ESTestCase { /** @@ -123,13 +123,7 @@ public void testGetScrollId() { public void testNullCollectionsArePreserved() { List failures = null; List hits = null; - Response response = new Response( - randomBoolean(), - failures, - randomNonNegativeLong(), - hits, - randomAlphaOfLengthBetween(3, 20) - ); + Response response = new Response(randomBoolean(), failures, randomNonNegativeLong(), hits, randomAlphaOfLengthBetween(3, 20)); assertNull(response.getFailures()); assertNull(response.getHits()); } @@ -154,13 +148,7 @@ private static List randomHits() { int size = randomIntBetween(1, 5); List hits = new ArrayList<>(size); for (int i = 0; i < size; i++) { - hits.add( - new BasicHit( - randomAlphaOfLengthBetween(3, 10), - randomAlphaOfLengthBetween(3, 10), - randomNonNegativeLong() - ) - ); + hits.add(new BasicHit(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomNonNegativeLong())); } return hits; } diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java index 83da58def4b8e..3b6c373efe27b 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java @@ -13,12 +13,13 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; -import java.util.Map; -import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; import org.elasticsearch.xcontent.XContentType; +import java.util.Map; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.not; @@ -56,13 +57,7 @@ public void testEqualsAndHashCode() { Integer shardId = randomIntBetween(0, 100); String nodeId = randomAlphaOfLengthBetween(3, 10); SearchFailure failure1 = new SearchFailure(reason, index, shardId, nodeId); - SearchFailure failure2 = new SearchFailure( - reason, - index, - shardId, - nodeId, - RestStatus.INTERNAL_SERVER_ERROR - ); + SearchFailure failure2 = new SearchFailure(reason, index, shardId, nodeId, RestStatus.INTERNAL_SERVER_ERROR); assertEquals(failure1, failure2); assertEquals(failure1.hashCode(), failure2.hashCode()); } @@ -74,9 +69,15 @@ public void testEqualsIsFalseForDifferentFields() { String nodeId = randomAlphaOfLengthBetween(3, 10); SearchFailure base = new SearchFailure(reason, index, shardId, nodeId); - assertNotEquals(base, new SearchFailure(reason, randomValueOtherThan(index, () -> randomAlphaOfLengthBetween(3, 10)), shardId, nodeId)); + assertNotEquals( + base, + new SearchFailure(reason, randomValueOtherThan(index, () -> randomAlphaOfLengthBetween(3, 10)), shardId, nodeId) + ); assertNotEquals(base, new SearchFailure(reason, index, randomValueOtherThan(shardId, () -> randomIntBetween(0, 100)), nodeId)); - assertNotEquals(base, new SearchFailure(reason, index, shardId, randomValueOtherThan(nodeId, () -> randomAlphaOfLengthBetween(3, 10)))); + assertNotEquals( + base, + new SearchFailure(reason, index, shardId, randomValueOtherThan(nodeId, () -> randomAlphaOfLengthBetween(3, 10))) + ); } public void testEqualsIsFalseForDifferentExceptionTypeOrMessage() { @@ -95,11 +96,7 @@ public void testToXContentIncludesExpectedFields() { String nodeId = randomAlphaOfLengthBetween(3, 10); SearchFailure failure = new SearchFailure(reason, index, shardId, nodeId); String json = Strings.toString(failure); - Map map = XContentHelper.convertToMap( - XContentType.JSON.xContent(), - json, - false - ); + Map map = XContentHelper.convertToMap(XContentType.JSON.xContent(), json, false); assertThat(map.get(SearchFailure.INDEX_FIELD), equalTo(index)); assertThat(map.get(SearchFailure.SHARD_FIELD), equalTo(shardId)); assertThat(map.get(SearchFailure.NODE_FIELD), equalTo(nodeId)); @@ -114,11 +111,7 @@ public void testToXContentIncludesExpectedFields() { public void testToXContentOmitsNullOptionalFields() { SearchFailure failure = new SearchFailure(randomException()); String json = Strings.toString(failure); - Map map = XContentHelper.convertToMap( - XContentType.JSON.xContent(), - json, - false - ); + Map map = XContentHelper.convertToMap(XContentType.JSON.xContent(), json, false); assertThat(map, not(hasKey(SearchFailure.INDEX_FIELD))); assertThat(map, not(hasKey(SearchFailure.SHARD_FIELD))); assertThat(map, not(hasKey(SearchFailure.NODE_FIELD))); @@ -131,10 +124,6 @@ public static Throwable randomException() { } public static Throwable randomException(String message) { - return randomFrom( - new IllegalArgumentException(message), - new IllegalStateException(message), - new ElasticsearchException(message) - ); + return randomFrom(new IllegalArgumentException(message), new IllegalStateException(message), new ElasticsearchException(message)); } } diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java index 313bce012e7f7..6e1f212e34f2f 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java @@ -11,8 +11,8 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.test.AbstractWireSerializingTestCase; import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; +import org.elasticsearch.test.AbstractWireSerializingTestCase; import static org.elasticsearch.index.reindex.paginatedhitsource.SearchFailureTests.randomException; @@ -51,14 +51,8 @@ protected void assertEqualInstances(SearchFailure expected, SearchFailure actual assertEquals(expected.getStatus(), actual.getStatus()); // Compare exception meaningfully, not by identity - assertEquals( - expected.getReason().getClass(), - actual.getReason().getClass() - ); - assertEquals( - expected.getReason().getMessage(), - actual.getReason().getMessage() - ); + assertEquals(expected.getReason().getClass(), actual.getReason().getClass()); + assertEquals(expected.getReason().getMessage(), actual.getReason().getMessage()); } public static SearchFailure mutateSearchFailure(SearchFailure instance) { @@ -82,37 +76,19 @@ public static SearchFailure mutateSearchFailure(SearchFailure instance) { String newIndex = instance.getIndex() == null ? randomAlphaOfLengthBetween(1, 10) : randomValueOtherThan(instance.getIndex(), () -> randomAlphaOfLengthBetween(1, 10)); - return new SearchFailure( - instance.getReason(), - newIndex, - instance.getShardId(), - instance.getNodeId(), - instance.getStatus() - ); + return new SearchFailure(instance.getReason(), newIndex, instance.getShardId(), instance.getNodeId(), instance.getStatus()); } case 2 -> { Integer newShardId = instance.getShardId() == null ? randomIntBetween(0, 100) : randomValueOtherThan(instance.getShardId(), () -> randomIntBetween(0, 100)); - return new SearchFailure( - instance.getReason(), - instance.getIndex(), - newShardId, - instance.getNodeId(), - instance.getStatus() - ); + return new SearchFailure(instance.getReason(), instance.getIndex(), newShardId, instance.getNodeId(), instance.getStatus()); } case 3 -> { String newNodeId = instance.getNodeId() == null ? randomAlphaOfLengthBetween(1, 10) : randomValueOtherThan(instance.getNodeId(), () -> randomAlphaOfLengthBetween(1, 10)); - return new SearchFailure( - instance.getReason(), - instance.getIndex(), - instance.getShardId(), - newNodeId, - instance.getStatus() - ); + return new SearchFailure(instance.getReason(), instance.getIndex(), instance.getShardId(), newNodeId, instance.getStatus()); } default -> throw new AssertionError("Unknown field index [" + fieldToMutate + "]"); } From e3d1b268a96ad3bda7c71940ffffbc88bbf81fc5 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 18 Feb 2026 15:23:48 +0000 Subject: [PATCH 3/7] Spotless apply --- .../index/reindex/PaginatedHitSource.java | 25 ---- .../SearchFailureWireSerialisationTests.java | 127 ++++++++++++++---- 2 files changed, 98 insertions(+), 54 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java b/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java index 2667f5be47f56..5e795f7bad5ba 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/PaginatedHitSource.java @@ -34,7 +34,6 @@ import java.io.IOException; import java.util.List; -import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -482,29 +481,5 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public String toString() { return Strings.toString(this); } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - SearchFailure that = (SearchFailure) o; - if (!Objects.equals(index, that.index)) return false; - if (!Objects.equals(shardId, that.shardId)) return false; - if (!Objects.equals(nodeId, that.nodeId)) return false; - if (status != that.status) return false; - // Compare Throwable meaningfully, not by identity - if (reason == null && that.reason == null) { - return true; - } - if (reason == null || that.reason == null) { - return false; - } - return reason.getClass().equals(that.reason.getClass()) && Objects.equals(reason.getMessage(), that.reason.getMessage()); - } - - @Override - public int hashCode() { - return Objects.hash(index, shardId, nodeId, status, reason.getClass(), reason.getMessage()); - } } } diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java index 6e1f212e34f2f..f02c79bbe239f 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java @@ -10,61 +10,130 @@ package org.elasticsearch.index.reindex.paginatedhitsource; import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; import org.elasticsearch.test.AbstractWireSerializingTestCase; -import static org.elasticsearch.index.reindex.paginatedhitsource.SearchFailureTests.randomException; +import java.io.IOException; +import java.util.Objects; -public class SearchFailureWireSerialisationTests extends AbstractWireSerializingTestCase { +import static org.elasticsearch.index.reindex.paginatedhitsource.SearchFailureTests.randomException; +public class SearchFailureWireSerialisationTests extends AbstractWireSerializingTestCase< + SearchFailureWireSerialisationTests.SearchFailureWrapper> { @Override - protected SearchFailure createTestInstance() { + protected SearchFailureWrapper createTestInstance() { Throwable reason = randomException(); String index = randomBoolean() ? randomAlphaOfLengthBetween(1, 10) : null; Integer shardId = randomBoolean() ? randomIntBetween(0, 100) : null; String nodeId = randomBoolean() ? randomAlphaOfLengthBetween(1, 10) : null; - return new SearchFailure(reason, index, shardId, nodeId); + return new SearchFailureWrapper(new SearchFailure(reason, index, shardId, nodeId)); } @Override - protected Writeable.Reader instanceReader() { - return SearchFailure::new; + protected Writeable.Reader instanceReader() { + return SearchFailureWrapper::new; } @Override - protected SearchFailure mutateInstance(SearchFailure instance) { - return mutateSearchFailure(instance); + protected SearchFailureWrapper mutateInstance(SearchFailureWrapper instance) { + return new SearchFailureWrapper(mutateSearchFailure(instance.failure())); } /** - * {@link SearchFailure} contains a {@code Throwable reason}. - * While the XContent output looks identical, the exception instances are not semantically identical after deserialization. - * Therefore, we must provide our own equality override to verify that the exceptions are meaningfully identical, even if the instance - * equality check fails. + * Custom semantic equality assertion for wire-serialization testing. + *

+ * {@link SearchFailure} contains a {@link Throwable} whose equality is based on + * object identity. This method performs a field-by-field comparison of the + * meaningful state, including exception class and message, to verify that wire + * serialization preserves observable failure semantics without requiring + * production equality semantics on {@link SearchFailure}. */ @Override - protected void assertEqualInstances(SearchFailure expected, SearchFailure actual) { - assertEquals(expected.getIndex(), actual.getIndex()); - assertEquals(expected.getShardId(), actual.getShardId()); - assertEquals(expected.getNodeId(), actual.getNodeId()); - assertEquals(expected.getStatus(), actual.getStatus()); - - // Compare exception meaningfully, not by identity - assertEquals(expected.getReason().getClass(), actual.getReason().getClass()); - assertEquals(expected.getReason().getMessage(), actual.getReason().getMessage()); + protected void assertEqualInstances(SearchFailureWrapper expected, SearchFailureWrapper actual) { + SearchFailure e = expected.failure(); + SearchFailure a = actual.failure(); + assertEquals(e.getIndex(), a.getIndex()); + assertEquals(e.getShardId(), a.getShardId()); + assertEquals(e.getNodeId(), a.getNodeId()); + assertEquals(e.getStatus(), a.getStatus()); + // Compare exception semantically, not by identity + assertEquals(e.getReason().getClass(), a.getReason().getClass()); + assertEquals(e.getReason().getMessage(), a.getReason().getMessage()); } - public static SearchFailure mutateSearchFailure(SearchFailure instance) { + /** + * Wrapper around {@link SearchFailure} used exclusively for wire-serialization tests. + *

+ * {@link AbstractWireSerializingTestCase} requires instances to be comparable via + * {@code equals}/{@code hashCode()}, but {@link SearchFailure} does not define + * suitable semantic equality due to its embedded {@link Throwable}. + *

+ * This wrapper provides stable, test-only equality semantics without leaking + * test concerns into production code. + */ + static final class SearchFailureWrapper implements Writeable { + private final SearchFailure failure; + + SearchFailureWrapper(SearchFailure failure) { + this.failure = failure; + } + + SearchFailureWrapper(StreamInput in) throws IOException { + this.failure = new SearchFailure(in); + } + + SearchFailure failure() { + return failure; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + failure.writeTo(out); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SearchFailureWrapper that = (SearchFailureWrapper) o; + return failuresEqual(failure, that.failure); + } + + @Override + public int hashCode() { + return Objects.hash( + failure.getIndex(), + failure.getShardId(), + failure.getNodeId(), + failure.getStatus(), + failure.getReason().getClass(), + failure.getReason().getMessage() + ); + } + + private static boolean failuresEqual(SearchFailure a, SearchFailure b) { + return Objects.equals(a.getIndex(), b.getIndex()) + && Objects.equals(a.getShardId(), b.getShardId()) + && Objects.equals(a.getNodeId(), b.getNodeId()) + && a.getStatus() == b.getStatus() + && a.getReason().getClass().equals(b.getReason().getClass()) + && Objects.equals(a.getReason().getMessage(), b.getReason().getMessage()); + } + } + + static SearchFailure mutateSearchFailure(SearchFailure instance) { int fieldToMutate = randomIntBetween(0, 3); - switch (fieldToMutate) { + return switch (fieldToMutate) { case 0 -> { Throwable newReason; do { newReason = randomException(); } while (newReason.getClass().equals(instance.getReason().getClass()) - && String.valueOf(newReason.getMessage()).equals(String.valueOf(instance.getReason().getMessage()))); - return new SearchFailure( + && Objects.equals(newReason.getMessage(), instance.getReason().getMessage())); + yield new SearchFailure( newReason, instance.getIndex(), instance.getShardId(), @@ -76,21 +145,21 @@ public static SearchFailure mutateSearchFailure(SearchFailure instance) { String newIndex = instance.getIndex() == null ? randomAlphaOfLengthBetween(1, 10) : randomValueOtherThan(instance.getIndex(), () -> randomAlphaOfLengthBetween(1, 10)); - return new SearchFailure(instance.getReason(), newIndex, instance.getShardId(), instance.getNodeId(), instance.getStatus()); + yield new SearchFailure(instance.getReason(), newIndex, instance.getShardId(), instance.getNodeId(), instance.getStatus()); } case 2 -> { Integer newShardId = instance.getShardId() == null ? randomIntBetween(0, 100) : randomValueOtherThan(instance.getShardId(), () -> randomIntBetween(0, 100)); - return new SearchFailure(instance.getReason(), instance.getIndex(), newShardId, instance.getNodeId(), instance.getStatus()); + yield new SearchFailure(instance.getReason(), instance.getIndex(), newShardId, instance.getNodeId(), instance.getStatus()); } case 3 -> { String newNodeId = instance.getNodeId() == null ? randomAlphaOfLengthBetween(1, 10) : randomValueOtherThan(instance.getNodeId(), () -> randomAlphaOfLengthBetween(1, 10)); - return new SearchFailure(instance.getReason(), instance.getIndex(), instance.getShardId(), newNodeId, instance.getStatus()); + yield new SearchFailure(instance.getReason(), instance.getIndex(), instance.getShardId(), newNodeId, instance.getStatus()); } default -> throw new AssertionError("Unknown field index [" + fieldToMutate + "]"); - } + }; } } From ecde50fb9644341b9c6ca2dafddd7f118f7ca637 Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Wed, 18 Feb 2026 16:58:50 +0000 Subject: [PATCH 4/7] Remove redundant equals tests --- .../SearchFailureTests.java | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java index 3b6c373efe27b..bc9d0e8c1c873 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java @@ -50,44 +50,6 @@ public void testConstructorWithAllFields() { assertEquals(ExceptionsHelper.status(reason), failure.getStatus()); } - public void testEqualsAndHashCode() { - // Hardcode the exception to match the rest status - Throwable reason = new IllegalStateException("Exception"); - String index = randomAlphaOfLengthBetween(3, 10); - Integer shardId = randomIntBetween(0, 100); - String nodeId = randomAlphaOfLengthBetween(3, 10); - SearchFailure failure1 = new SearchFailure(reason, index, shardId, nodeId); - SearchFailure failure2 = new SearchFailure(reason, index, shardId, nodeId, RestStatus.INTERNAL_SERVER_ERROR); - assertEquals(failure1, failure2); - assertEquals(failure1.hashCode(), failure2.hashCode()); - } - - public void testEqualsIsFalseForDifferentFields() { - Throwable reason = randomException(); - String index = randomAlphaOfLengthBetween(3, 10); - Integer shardId = randomIntBetween(0, 100); - String nodeId = randomAlphaOfLengthBetween(3, 10); - SearchFailure base = new SearchFailure(reason, index, shardId, nodeId); - - assertNotEquals( - base, - new SearchFailure(reason, randomValueOtherThan(index, () -> randomAlphaOfLengthBetween(3, 10)), shardId, nodeId) - ); - assertNotEquals(base, new SearchFailure(reason, index, randomValueOtherThan(shardId, () -> randomIntBetween(0, 100)), nodeId)); - assertNotEquals( - base, - new SearchFailure(reason, index, shardId, randomValueOtherThan(nodeId, () -> randomAlphaOfLengthBetween(3, 10))) - ); - } - - public void testEqualsIsFalseForDifferentExceptionTypeOrMessage() { - SearchFailure f1 = new SearchFailure(new IllegalStateException("exception_1")); - SearchFailure f2 = new SearchFailure(new IllegalArgumentException("exception_1")); - SearchFailure f3 = new SearchFailure(new IllegalStateException("exception_2")); - assertNotEquals(f1, f2); - assertNotEquals(f1, f3); - } - public void testToXContentIncludesExpectedFields() { String message = randomAlphaOfLengthBetween(1, 20); Throwable reason = randomException(message); From 9c7cf135e0e57249ba27fd0e04a8dbf8100bdfdc Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 18 Feb 2026 17:07:05 +0000 Subject: [PATCH 5/7] [CI] Auto commit changes from spotless --- .../index/reindex/paginatedhitsource/SearchFailureTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java index bc9d0e8c1c873..8300993fe571a 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.reindex.PaginatedHitSource.SearchFailure; -import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentType; From 15fb4d94b46b20314690d6b6f509b3130b990fdf Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 5 Mar 2026 15:48:56 +0000 Subject: [PATCH 6/7] Address Comments --- .../paginatedhitsource/ResponseTests.java | 81 +------------------ .../SearchFailureWireSerialisationTests.java | 22 ----- 2 files changed, 2 insertions(+), 101 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java index 48171aa6aecf4..ab297e4377e6c 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java @@ -27,9 +27,9 @@ public class ResponseTests extends ESTestCase { */ public void testConstructor() { boolean timedOut = randomBoolean(); - List failures = randomFailures(); + List failures = randomBoolean() ? Collections.emptyList() : randomFailures(); long totalHits = randomNonNegativeLong(); - List hits = randomHits(); + List hits = randomBoolean() ? Collections.emptyList() : randomHits(); String scrollId = randomAlphaOfLengthBetween(3, 20); Response response = new Response(timedOut, failures, totalHits, hits, scrollId); @@ -40,83 +40,6 @@ public void testConstructor() { assertEquals(scrollId, response.getScrollId()); } - /** - * Verifies that the response correctly reports when it has timed out. - */ - public void testIsTimedOut() { - boolean timedOut = randomBoolean(); - Response response = new Response( - timedOut, - Collections.emptyList(), - randomNonNegativeLong(), - Collections.emptyList(), - randomAlphaOfLengthBetween(3, 20) - ); - assertEquals(timedOut, response.isTimedOut()); - } - - /** - * Verifies that a (possibly empty) failures list is returned unchanged, preserving all failure entries - */ - public void testGetFailures() { - List failures = randomBoolean() ? Collections.emptyList() : randomFailures(); - Response response = new Response( - randomBoolean(), - failures, - randomNonNegativeLong(), - Collections.emptyList(), - randomAlphaOfLengthBetween(3, 20) - ); - assertSame(failures, response.getFailures()); - assertEquals(failures.size(), response.getFailures().size()); - } - - /** - * Verifies that totalHits returned by the getter matches the value provided at construction time. - */ - public void testGetTotalHits() { - long totalHits = randomNonNegativeLong(); - Response response = new Response( - randomBoolean(), - Collections.emptyList(), - totalHits, - Collections.emptyList(), - randomAlphaOfLengthBetween(3, 20) - ); - assertEquals(totalHits, response.getTotalHits()); - } - - /** - * Verifies that a (possible empty) hits list is returned unchanged and preserves all hit entries. - */ - public void testEmptyHitsList() { - List hits = randomBoolean() ? Collections.emptyList() : randomHits(); - Response response = new Response( - randomBoolean(), - Collections.emptyList(), - randomNonNegativeLong(), - hits, - randomAlphaOfLengthBetween(3, 20) - ); - assertSame(hits, response.getHits()); - assertEquals(hits.size(), response.getHits().size()); - } - - /** - * Verifies that the scroll id returned by the getter matches the value provided at construction time. - */ - public void testGetScrollId() { - String scrollId = randomAlphaOfLengthBetween(5, 30); - Response response = new Response( - randomBoolean(), - Collections.emptyList(), - randomNonNegativeLong(), - Collections.emptyList(), - scrollId - ); - assertEquals(scrollId, response.getScrollId()); - } - /** * Verifies that providing null values for optional collections is preserved and returned as-is by the getters. */ diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java index f02c79bbe239f..126b1e970bc42 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java @@ -42,28 +42,6 @@ protected SearchFailureWrapper mutateInstance(SearchFailureWrapper instance) { return new SearchFailureWrapper(mutateSearchFailure(instance.failure())); } - /** - * Custom semantic equality assertion for wire-serialization testing. - *

- * {@link SearchFailure} contains a {@link Throwable} whose equality is based on - * object identity. This method performs a field-by-field comparison of the - * meaningful state, including exception class and message, to verify that wire - * serialization preserves observable failure semantics without requiring - * production equality semantics on {@link SearchFailure}. - */ - @Override - protected void assertEqualInstances(SearchFailureWrapper expected, SearchFailureWrapper actual) { - SearchFailure e = expected.failure(); - SearchFailure a = actual.failure(); - assertEquals(e.getIndex(), a.getIndex()); - assertEquals(e.getShardId(), a.getShardId()); - assertEquals(e.getNodeId(), a.getNodeId()); - assertEquals(e.getStatus(), a.getStatus()); - // Compare exception semantically, not by identity - assertEquals(e.getReason().getClass(), a.getReason().getClass()); - assertEquals(e.getReason().getMessage(), a.getReason().getMessage()); - } - /** * Wrapper around {@link SearchFailure} used exclusively for wire-serialization tests. *

From bcc50a2527b0cfac654543d95ddd88b950e0471c Mon Sep 17 00:00:00 2001 From: Joshua Adams Date: Thu, 12 Mar 2026 11:21:30 +0000 Subject: [PATCH 7/7] Move out of individual folder --- .../index/reindex/{paginatedhitsource => }/BasicHitTests.java | 3 +-- .../index/reindex/{paginatedhitsource => }/ResponseTests.java | 3 +-- .../reindex/{paginatedhitsource => }/SearchFailureTests.java | 2 +- .../SearchFailureWireSerialisationTests.java | 4 ++-- 4 files changed, 5 insertions(+), 7 deletions(-) rename server/src/test/java/org/elasticsearch/index/reindex/{paginatedhitsource => }/BasicHitTests.java (97%) rename server/src/test/java/org/elasticsearch/index/reindex/{paginatedhitsource => }/ResponseTests.java (96%) rename server/src/test/java/org/elasticsearch/index/reindex/{paginatedhitsource => }/SearchFailureTests.java (98%) rename server/src/test/java/org/elasticsearch/index/reindex/{paginatedhitsource => }/SearchFailureWireSerialisationTests.java (97%) diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java b/server/src/test/java/org/elasticsearch/index/reindex/BasicHitTests.java similarity index 97% rename from server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java rename to server/src/test/java/org/elasticsearch/index/reindex/BasicHitTests.java index 3f613a4a76bc3..24cf3b8ecd2b4 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/BasicHitTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/BasicHitTests.java @@ -7,11 +7,10 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -package org.elasticsearch.index.reindex.paginatedhitsource; +package org.elasticsearch.index.reindex; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.index.reindex.PaginatedHitSource; import org.elasticsearch.index.reindex.PaginatedHitSource.BasicHit; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentType; diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java b/server/src/test/java/org/elasticsearch/index/reindex/ResponseTests.java similarity index 96% rename from server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java rename to server/src/test/java/org/elasticsearch/index/reindex/ResponseTests.java index ab297e4377e6c..8a80dd6c6cc76 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/ResponseTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/ResponseTests.java @@ -7,9 +7,8 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -package org.elasticsearch.index.reindex.paginatedhitsource; +package org.elasticsearch.index.reindex; -import org.elasticsearch.index.reindex.PaginatedHitSource; import org.elasticsearch.index.reindex.PaginatedHitSource.BasicHit; import org.elasticsearch.index.reindex.PaginatedHitSource.Hit; import org.elasticsearch.index.reindex.PaginatedHitSource.Response; diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java b/server/src/test/java/org/elasticsearch/index/reindex/SearchFailureTests.java similarity index 98% rename from server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java rename to server/src/test/java/org/elasticsearch/index/reindex/SearchFailureTests.java index 8300993fe571a..a65e0adc1d9a4 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/SearchFailureTests.java @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -package org.elasticsearch.index.reindex.paginatedhitsource; +package org.elasticsearch.index.reindex; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; diff --git a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java b/server/src/test/java/org/elasticsearch/index/reindex/SearchFailureWireSerialisationTests.java similarity index 97% rename from server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java rename to server/src/test/java/org/elasticsearch/index/reindex/SearchFailureWireSerialisationTests.java index 126b1e970bc42..6c27e7f44e725 100644 --- a/server/src/test/java/org/elasticsearch/index/reindex/paginatedhitsource/SearchFailureWireSerialisationTests.java +++ b/server/src/test/java/org/elasticsearch/index/reindex/SearchFailureWireSerialisationTests.java @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -package org.elasticsearch.index.reindex.paginatedhitsource; +package org.elasticsearch.index.reindex; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.io.stream.StreamInput; @@ -19,7 +19,7 @@ import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.index.reindex.paginatedhitsource.SearchFailureTests.randomException; +import static org.elasticsearch.index.reindex.SearchFailureTests.randomException; public class SearchFailureWireSerialisationTests extends AbstractWireSerializingTestCase< SearchFailureWireSerialisationTests.SearchFailureWrapper> {