Skip to content

Commit 4a12eba

Browse files
jdconradjdconrad
authored
Remove all uses of TransportVersion.onOrAfter (elastic#139462)
This change switches all uses of TransportVersion.onOrAfter to use TransportVersion.supports instead. The one exception is doing the greater-than-or-equal checks by hand for TransportHandshaker. TransportVersion.onOrAfter is changed to throw UnsupportedOperationException.
1 parent fb676a0 commit 4a12eba

File tree

31 files changed

+103
-132
lines changed

31 files changed

+103
-132
lines changed

‎server/src/main/java/org/elasticsearch/TransportVersion.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@
6767
*/
6868
public record TransportVersion(String name, int id, TransportVersion nextPatchVersion) implements VersionId<TransportVersion> {
6969

70+
@Deprecated(forRemoval = true)
71+
public boolean onOrAfter(TransportVersion version) {
72+
throw new UnsupportedOperationException("use TransportVersion.supports(...) instead");
73+
}
74+
75+
@Deprecated(forRemoval = true)
76+
public boolean between(TransportVersion lowerInclusive, TransportVersion upperExclusive) {
77+
throw new UnsupportedOperationException("use TransportVersion.supports(...) && TransportVersion.supports(...) == false instead");
78+
}
79+
7080
/**
7181
* Constructs an unnamed transport version.
7282
*/
@@ -276,7 +286,7 @@ public static TransportVersion max(TransportVersion version1, TransportVersion v
276286
* Returns {@code true} if the specified version is compatible with this running version of Elasticsearch.
277287
*/
278288
public static boolean isCompatible(TransportVersion version) {
279-
return version.onOrAfter(VersionsHolder.MINIMUM_COMPATIBLE);
289+
return version.id >= VersionsHolder.MINIMUM_COMPATIBLE.id;
280290
}
281291

282292
/**
@@ -349,8 +359,8 @@ public static TransportVersion fromString(String str) {
349359
/**
350360
* Returns {@code true} if this version is a patch version at or after {@code version}.
351361
*/
352-
private boolean isPatchFrom(TransportVersion version) {
353-
return onOrAfter(version) && id < version.id + 100 - (version.id % 100);
362+
public boolean isPatchFrom(TransportVersion version) {
363+
return id >= version.id && id < version.id + 100 - (version.id % 100);
354364
}
355365

356366
/**
@@ -391,7 +401,7 @@ private boolean isPatchFrom(TransportVersion version) {
391401
* }
392402
*/
393403
public boolean supports(TransportVersion version) {
394-
if (onOrAfter(version)) {
404+
if (id >= version.id) {
395405
return true;
396406
}
397407
TransportVersion nextPatchVersion = version.nextPatchVersion;

‎server/src/main/java/org/elasticsearch/cluster/NamedDiff.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public interface NamedDiff<T extends Diffable<T>> extends Diff<T>, NamedWriteabl
3333
* @return {@code true} if the instance should be serialized, {@code false} otherwise
3434
*/
3535
default boolean supportsVersion(TransportVersion version) {
36-
return version.onOrAfter(getMinimalSupportedVersion());
36+
return version.supports(getMinimalSupportedVersion());
3737
}
3838

3939
}

‎server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -920,10 +920,10 @@ public Object readGenericValue() throws IOException {
920920
case 6 -> readByteArray();
921921
case 7 -> readCollection(StreamInput::readGenericValue, ArrayList::new, Collections.emptyList());
922922
case 8 -> readArray();
923-
case 9 -> getTransportVersion().onOrAfter(V_8_7_0)
923+
case 9 -> getTransportVersion().supports(V_8_7_0)
924924
? readOrderedMap(StreamInput::readGenericValue, StreamInput::readGenericValue)
925925
: readOrderedMap(StreamInput::readString, StreamInput::readGenericValue);
926-
case 10 -> getTransportVersion().onOrAfter(V_8_7_0)
926+
case 10 -> getTransportVersion().supports(V_8_7_0)
927927
? readMap(StreamInput::readGenericValue, StreamInput::readGenericValue)
928928
: readMap(StreamInput::readGenericValue);
929929
case 11 -> readByte();

‎server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ public void writeMapWithConsistentOrder(@Nullable Map<String, ? extends Object>
614614
Iterator<? extends Map.Entry<String, ?>> iterator = map.entrySet().stream().sorted(Map.Entry.comparingByKey()).iterator();
615615
while (iterator.hasNext()) {
616616
Map.Entry<String, ?> next = iterator.next();
617-
if (this.getTransportVersion().onOrAfter(V_8_7_0)) {
617+
if (this.getTransportVersion().supports(V_8_7_0)) {
618618
this.writeGenericValue(next.getKey());
619619
} else {
620620
this.writeString(next.getKey());
@@ -752,7 +752,7 @@ public final void writeOptionalInstant(@Nullable Instant instant) throws IOExcep
752752
} else {
753753
o.writeByte((byte) 10);
754754
}
755-
if (o.getTransportVersion().onOrAfter(V_8_7_0)) {
755+
if (o.getTransportVersion().supports(V_8_7_0)) {
756756
final Map<?, ?> map = (Map<?, ?>) v;
757757
o.writeMap(map, StreamOutput::writeGenericValue, StreamOutput::writeGenericValue);
758758
} else {

‎server/src/main/java/org/elasticsearch/common/io/stream/VersionedNamedWriteable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public interface VersionedNamedWriteable extends NamedWriteable {
4141
* @return {@code true} if the instance should be serialized, {@code false} otherwise
4242
*/
4343
default boolean supportsVersion(TransportVersion version) {
44-
return version.onOrAfter(getMinimalSupportedVersion());
44+
return version.supports(getMinimalSupportedVersion());
4545
}
4646

4747
/**

‎server/src/main/java/org/elasticsearch/transport/OutboundHandler.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,8 @@ public boolean rstOnClose() {
462462
}
463463

464464
private boolean assertValidTransportVersion(TransportVersion transportVersion) {
465-
assert this.version.before(TransportVersion.minimumCompatible()) // running an incompatible-version test
466-
|| this.version.onOrAfter(transportVersion) : this.version + " vs " + transportVersion;
465+
assert this.version.id() < TransportVersion.minimumCompatible().id() // running an incompatible-version test
466+
|| this.version.id() >= transportVersion.id() : this.version + " vs " + transportVersion;
467467
return true;
468468
}
469469

‎server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ private static TransportVersion ensureCompatibleVersion(
223223
Object channel
224224
) {
225225
if (TransportVersion.isCompatible(remoteTransportVersion)) {
226-
if (remoteTransportVersion.onOrAfter(localTransportVersion)) {
226+
if (remoteTransportVersion.id() >= localTransportVersion.id()) {
227227
// Remote is semantically newer than us (i.e. has a greater transport protocol version), so we propose using our current
228228
// transport protocol version. If we're initiating the connection then that's the version we'll use; if the other end is
229229
// initiating the connection then it's up to the other end to decide whether to use this version (if it knows it) or
@@ -359,7 +359,7 @@ static final class HandshakeRequest extends AbstractTransportRequest {
359359

360360
try (StreamInput messageStreamInput = streamInput.readSlicedBytesReference().streamInput()) {
361361
this.transportVersion = TransportVersion.readVersion(messageStreamInput);
362-
if (streamInput.getTransportVersion().onOrAfter(V9_HANDSHAKE_VERSION)) {
362+
if (streamInput.getTransportVersion().supports(V9_HANDSHAKE_VERSION)) {
363363
this.releaseVersion = messageStreamInput.readString();
364364
} else {
365365
this.releaseVersion = this.transportVersion.toReleaseVersion();
@@ -374,7 +374,7 @@ public void writeTo(StreamOutput streamOutput) throws IOException {
374374
assert transportVersion != null;
375375
try (BytesStreamOutput messageStreamOutput = new BytesStreamOutput(1024)) {
376376
TransportVersion.writeVersion(transportVersion, messageStreamOutput);
377-
if (streamOutput.getTransportVersion().onOrAfter(V9_HANDSHAKE_VERSION)) {
377+
if (streamOutput.getTransportVersion().supports(V9_HANDSHAKE_VERSION)) {
378378
messageStreamOutput.writeString(releaseVersion);
379379
} // else we just send the transport version and rely on a best-effort mapping to release versions
380380
BytesReference reference = messageStreamOutput.bytes();
@@ -406,7 +406,7 @@ static final class HandshakeResponse extends TransportResponse {
406406

407407
HandshakeResponse(StreamInput in) throws IOException {
408408
transportVersion = TransportVersion.readVersion(in);
409-
if (in.getTransportVersion().onOrAfter(V9_HANDSHAKE_VERSION)) {
409+
if (in.getTransportVersion().supports(V9_HANDSHAKE_VERSION)) {
410410
releaseVersion = in.readString();
411411
} else {
412412
releaseVersion = transportVersion.toReleaseVersion();
@@ -416,7 +416,7 @@ static final class HandshakeResponse extends TransportResponse {
416416
@Override
417417
public void writeTo(StreamOutput out) throws IOException {
418418
TransportVersion.writeVersion(transportVersion, out);
419-
if (out.getTransportVersion().onOrAfter(V9_HANDSHAKE_VERSION)) {
419+
if (out.getTransportVersion().supports(V9_HANDSHAKE_VERSION)) {
420420
out.writeString(releaseVersion);
421421
} // else we just send the transport version and rely on a best-effort mapping to release versions
422422
}

‎server/src/test/java/org/elasticsearch/TransportVersionTests.java‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,45 +23,11 @@
2323
import java.util.regex.Pattern;
2424

2525
import static org.hamcrest.Matchers.endsWith;
26-
import static org.hamcrest.Matchers.greaterThan;
2726
import static org.hamcrest.Matchers.is;
28-
import static org.hamcrest.Matchers.lessThan;
2927
import static org.hamcrest.Matchers.sameInstance;
3028

3129
public class TransportVersionTests extends ESTestCase {
3230

33-
public void testVersionComparison() {
34-
TransportVersion older = TransportVersionUtils.randomVersionBetween(
35-
random(),
36-
TransportVersion.minimumCompatible(),
37-
TransportVersionUtils.getPreviousVersion(TransportVersion.current())
38-
);
39-
TransportVersion newer = TransportVersionUtils.randomVersionBetween(
40-
random(),
41-
TransportVersionUtils.getNextVersion(older),
42-
TransportVersion.current()
43-
);
44-
assertThat(older.before(newer), is(true));
45-
assertThat(older.before(older), is(false));
46-
assertThat(newer.before(older), is(false));
47-
48-
assertThat(older.onOrBefore(newer), is(true));
49-
assertThat(older.onOrBefore(older), is(true));
50-
assertThat(newer.onOrBefore(older), is(false));
51-
52-
assertThat(older.after(newer), is(false));
53-
assertThat(older.after(older), is(false));
54-
assertThat(newer.after(older), is(true));
55-
56-
assertThat(older.onOrAfter(newer), is(false));
57-
assertThat(older.onOrAfter(older), is(true));
58-
assertThat(newer.onOrAfter(older), is(true));
59-
60-
assertThat(older, is(lessThan(newer)));
61-
assertThat(older.compareTo(older), is(0));
62-
assertThat(newer, is(greaterThan(older)));
63-
}
64-
6531
public static class CorrectFakeVersion {
6632
public static final TransportVersion V_0_00_01 = new TransportVersion(199);
6733
public static final TransportVersion V_0_000_002 = new TransportVersion(2);
@@ -167,7 +133,7 @@ public void testCURRENTIsLatest() {
167133

168134
public void testPatchVersionsStillAvailable() {
169135
for (TransportVersion tv : TransportVersion.getAllVersions()) {
170-
if (tv.onOrAfter(TransportVersion.fromId(8_84_10_00)) && (tv.id() % 100) > 90) {
136+
if (tv.id() >= 8_84_10_00 && (tv.id() % 100) > 90) {
171137
fail(
172138
"Transport version "
173139
+ tv

‎server/src/test/java/org/elasticsearch/search/vectors/AbstractKnnVectorQueryBuilderTestCase.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.elasticsearch.index.query.TermQueryBuilder;
3535
import org.elasticsearch.test.AbstractBuilderTestCase;
3636
import org.elasticsearch.test.AbstractQueryTestCase;
37-
import org.elasticsearch.test.TransportVersionUtils;
3837
import org.elasticsearch.xcontent.XContentBuilder;
3938
import org.elasticsearch.xcontent.XContentFactory;
4039
import org.junit.Before;
@@ -415,19 +414,15 @@ public void testMustRewrite() throws IOException {
415414
public void testBWCVersionSerialization_GivenAutoPrefiltering() throws IOException {
416415
for (int i = 0; i < NUMBER_OF_TESTQUERIES; i++) {
417416

418-
TransportVersion version = TransportVersionUtils.randomVersionBetween(
419-
random(),
420-
TransportVersion.minimumCompatible(),
421-
TransportVersionUtils.getPreviousVersion(KnnVectorQueryBuilder.AUTO_PREFILTERING)
422-
);
423-
rescoreVectorAllowZero = version.onOrAfter(RescoreVectorBuilder.RESCORE_VECTOR_ALLOW_ZERO);
417+
TransportVersion version = TransportVersion.fromId(KnnVectorQueryBuilder.AUTO_PREFILTERING.id() - 1000);
418+
rescoreVectorAllowZero = version.supports(RescoreVectorBuilder.RESCORE_VECTOR_ALLOW_ZERO);
424419
KnnVectorQueryBuilder query = doCreateTestQueryBuilder().setAutoPrefilteringEnabled(true);
425420
KnnVectorQueryBuilder queryNoAutoPrefiltering = new KnnVectorQueryBuilder(
426421
query.getFieldName(),
427422
query.queryVector(),
428423
query.k(),
429424
query.numCands(),
430-
version.onOrAfter(KnnVectorQueryBuilder.VISIT_PERCENTAGE) ? query.visitPercentage() : null,
425+
version.supports(KnnVectorQueryBuilder.VISIT_PERCENTAGE) ? query.visitPercentage() : null,
431426
query.rescoreVectorBuilder(),
432427
query.getVectorSimilarity()
433428
).queryName(query.queryName()).boost(query.boost()).addFilterQueries(query.filterQueries()).setAutoPrefilteringEnabled(false);

‎server/src/test/java/org/elasticsearch/transport/TransportHandshakerTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void testIncompatibleHandshakeRequest() throws Exception {
112112
StreamInput input = bytesStreamOutput.bytes().streamInput();
113113
input.setTransportVersion(HANDSHAKE_REQUEST_VERSION);
114114

115-
if (handshakeRequest.transportVersion.onOrAfter(TransportVersion.minimumCompatible())) {
115+
if (handshakeRequest.transportVersion.id() >= TransportVersion.minimumCompatible().id()) {
116116

117117
final PlainActionFuture<TransportResponse> responseFuture = new PlainActionFuture<>();
118118
final TestTransportChannel channel = new TestTransportChannel(responseFuture);
@@ -203,7 +203,7 @@ public void testHandshakeResponseFromOlderNodeWithPatchedProtocol() throws Excep
203203
final var randomIncompatibleTransportVersion = getRandomIncompatibleTransportVersion();
204204
final var handshakeResponse = new TransportHandshaker.HandshakeResponse(randomIncompatibleTransportVersion, randomIdentifier());
205205

206-
if (randomIncompatibleTransportVersion.onOrAfter(TransportVersion.minimumCompatible())) {
206+
if (randomIncompatibleTransportVersion.id() >= (TransportVersion.minimumCompatible().id())) {
207207
// we fall back to the best known version
208208
MockLog.assertThatLogger(
209209
() -> handler.handleResponse(handshakeResponse),

0 commit comments

Comments
 (0)