From cb829b318fc777c2c9c8caa27d21abe09516f8ef Mon Sep 17 00:00:00 2001 From: tellet-q Date: Fri, 20 Dec 2024 13:26:12 +0100 Subject: [PATCH 1/4] Add version check on client init --- build.gradle | 1 + .../io/qdrant/client/QdrantGrpcClient.java | 77 ++++++++++++--- .../client/VersionsCompatibilityChecker.java | 96 +++++++++++++++++++ .../VersionsCompatibilityCheckerTest.java | 75 +++++++++++++++ 4 files changed, 238 insertions(+), 11 deletions(-) create mode 100644 src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java create mode 100644 src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java diff --git a/build.gradle b/build.gradle index 46ba1f7..9bf2a01 100644 --- a/build.gradle +++ b/build.gradle @@ -100,6 +100,7 @@ dependencies { testImplementation "io.grpc:grpc-testing:${grpcVersion}" testImplementation "org.junit.jupiter:junit-jupiter-api:${jUnitVersion}" + testImplementation "org.junit.jupiter:junit-jupiter-params:${jUnitVersion}" testImplementation "org.mockito:mockito-core:3.4.0" testImplementation "org.slf4j:slf4j-nop:${slf4jVersion}" testImplementation "org.testcontainers:qdrant:${testcontainersVersion}" diff --git a/src/main/java/io/qdrant/client/QdrantGrpcClient.java b/src/main/java/io/qdrant/client/QdrantGrpcClient.java index 17c3aef..1e7169f 100644 --- a/src/main/java/io/qdrant/client/QdrantGrpcClient.java +++ b/src/main/java/io/qdrant/client/QdrantGrpcClient.java @@ -4,13 +4,10 @@ import io.grpc.Deadline; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; -import io.qdrant.client.grpc.CollectionsGrpc; +import io.qdrant.client.grpc.*; import io.qdrant.client.grpc.CollectionsGrpc.CollectionsFutureStub; -import io.qdrant.client.grpc.PointsGrpc; import io.qdrant.client.grpc.PointsGrpc.PointsFutureStub; -import io.qdrant.client.grpc.QdrantGrpc; import io.qdrant.client.grpc.QdrantGrpc.QdrantFutureStub; -import io.qdrant.client.grpc.SnapshotsGrpc; import io.qdrant.client.grpc.SnapshotsGrpc.SnapshotsFutureStub; import java.time.Duration; import java.util.concurrent.TimeUnit; @@ -45,7 +42,7 @@ public class QdrantGrpcClient implements AutoCloseable { * @return a new instance of {@link Builder} */ public static Builder newBuilder(ManagedChannel channel) { - return new Builder(channel, false); + return new Builder(channel, false, true); } /** @@ -56,7 +53,21 @@ public static Builder newBuilder(ManagedChannel channel) { * @return a new instance of {@link Builder} */ public static Builder newBuilder(ManagedChannel channel, boolean shutdownChannelOnClose) { - return new Builder(channel, shutdownChannelOnClose); + return new Builder(channel, shutdownChannelOnClose, true); + } + + /** + * Creates a new builder to build a client. + * + * @param channel The channel for communication. + * @param shutdownChannelOnClose Whether the channel is shutdown on client close. + * @param checkCompatibility Whether to check compatibility between client's and server's + * versions. + * @return a new instance of {@link Builder} + */ + public static Builder newBuilder( + ManagedChannel channel, boolean shutdownChannelOnClose, boolean checkCompatibility) { + return new Builder(channel, shutdownChannelOnClose, checkCompatibility); } /** @@ -66,7 +77,7 @@ public static Builder newBuilder(ManagedChannel channel, boolean shutdownChannel * @return a new instance of {@link Builder} */ public static Builder newBuilder(String host) { - return new Builder(host, 6334, true); + return new Builder(host, 6334, true, true); } /** @@ -77,7 +88,7 @@ public static Builder newBuilder(String host) { * @return a new instance of {@link Builder} */ public static Builder newBuilder(String host, int port) { - return new Builder(host, port, true); + return new Builder(host, port, true, true); } /** @@ -90,7 +101,23 @@ public static Builder newBuilder(String host, int port) { * @return a new instance of {@link Builder} */ public static Builder newBuilder(String host, int port, boolean useTransportLayerSecurity) { - return new Builder(host, port, useTransportLayerSecurity); + return new Builder(host, port, useTransportLayerSecurity, true); + } + + /** + * Creates a new builder to build a client. + * + * @param host The host to connect to. + * @param port The port to connect to. + * @param useTransportLayerSecurity Whether the client uses Transport Layer Security (TLS) to + * secure communications. Running without TLS should only be used for testing purposes. + * @param checkCompatibility Whether to check compatibility between client's and server's + * versions. + * @return a new instance of {@link Builder} + */ + public static Builder newBuilder( + String host, int port, boolean useTransportLayerSecurity, boolean checkCompatibility) { + return new Builder(host, port, useTransportLayerSecurity, checkCompatibility); } /** @@ -168,17 +195,24 @@ public static class Builder { @Nullable private CallCredentials callCredentials; @Nullable private Duration timeout; - Builder(ManagedChannel channel, boolean shutdownChannelOnClose) { + Builder(ManagedChannel channel, boolean shutdownChannelOnClose, boolean checkCompatibility) { this.channel = channel; this.shutdownChannelOnClose = shutdownChannelOnClose; + String clientVersion = Builder.class.getPackage().getImplementationVersion(); + if (checkCompatibility) { + checkVersionsCompatibility(clientVersion); + } } - Builder(String host, int port, boolean useTransportLayerSecurity) { + Builder(String host, int port, boolean useTransportLayerSecurity, boolean checkCompatibility) { String clientVersion = Builder.class.getPackage().getImplementationVersion(); String javaVersion = System.getProperty("java.version"); String userAgent = "java-client/" + clientVersion + " java/" + javaVersion; this.channel = createChannel(host, port, useTransportLayerSecurity, userAgent); this.shutdownChannelOnClose = true; + if (checkCompatibility) { + checkVersionsCompatibility(clientVersion); + } } /** @@ -238,5 +272,26 @@ private static ManagedChannel createChannel( return channelBuilder.build(); } + + private void checkVersionsCompatibility(String clientVersion) { + try { + String serverVersion = + QdrantGrpc.newBlockingStub(this.channel) + .healthCheck(QdrantOuterClass.HealthCheckRequest.getDefaultInstance()) + .getVersion(); + if (!VersionsCompatibilityChecker.isCompatible(clientVersion, serverVersion)) { + System.out.println( + "Qdrant client version " + + clientVersion + + " is incompatible with server version " + + serverVersion + + ". Major versions should match and minor version difference must not exceed 1. " + + "Set check_version=False to skip version check."); + } + } catch (Exception e) { + System.out.println( + "Failed to obtain server version. Unable to check client-server compatibility. Set checkCompatibility=False to skip version check."); + } + } } } diff --git a/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java b/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java new file mode 100644 index 0000000..e70c3e7 --- /dev/null +++ b/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java @@ -0,0 +1,96 @@ +package io.qdrant.client; + +import java.util.ArrayList; +import java.util.Arrays; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class Version { + private final int major; + private final int minor; + private final String rest; + + public Version(int major, int minor, String rest) { + this.major = major; + this.minor = minor; + this.rest = rest; + } + + public int getMajor() { + return major; + } + + public int getMinor() { + return minor; + } + + public String getRest() { + return rest; + } +} + +/** Utility class to check compatibility between server's and client's versions. */ +public class VersionsCompatibilityChecker { + private static final Logger logger = LoggerFactory.getLogger(VersionsCompatibilityChecker.class); + + /** Default constructor. */ + public VersionsCompatibilityChecker() {} + + private static Version parseVersion(String version) throws IllegalArgumentException { + if (version.isEmpty()) { + throw new IllegalArgumentException("Version is None"); + } + + try { + String[] parts = version.split("\\."); + int major = parts.length > 0 ? Integer.parseInt(parts[0]) : 0; + int minor = parts.length > 1 ? Integer.parseInt(parts[1]) : 0; + String rest = + parts.length > 2 + ? String.join(".", new ArrayList<>(Arrays.asList(parts).subList(2, parts.length))) + : ""; + + return new Version(major, minor, rest); + } catch (Exception e) { + throw new IllegalArgumentException( + "Unable to parse version, expected format: x.y.z, found: " + version, e); + } + } + + /** + * Compares server's and client's versions. + * + * @param clientVersion The client's version. + * @param serverVersion The server's version. + * @return True if the versions are compatible, false otherwise. + */ + public static boolean isCompatible(String clientVersion, String serverVersion) { + if (clientVersion.isEmpty()) { + logger.warn("Unable to compare with client version {}", clientVersion); + return false; + } + + if (serverVersion.isEmpty()) { + logger.warn("Unable to compare with server version {}", serverVersion); + return false; + } + + if (clientVersion.equals(serverVersion)) { + return true; + } + + try { + Version parsedServerVersion = parseVersion(serverVersion); + Version parsedClientVersion = parseVersion(clientVersion); + + int majorDiff = Math.abs(parsedServerVersion.getMajor() - parsedClientVersion.getMajor()); + if (majorDiff >= 1) { + return false; + } + return Math.abs(parsedServerVersion.getMinor() - parsedClientVersion.getMinor()) <= 1; + } catch (IllegalArgumentException e) { + logger.warn("Unable to compare versions: {}", e.getMessage()); + return false; + } + } +} diff --git a/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java b/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java new file mode 100644 index 0000000..d21bca1 --- /dev/null +++ b/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java @@ -0,0 +1,75 @@ +package io.qdrant.client; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +public class VersionsCompatibilityCheckerTest { + private static Stream validVersionProvider() { + return Stream.of( + new Object[] {"1.2.3", 1, 2, "3"}, + new Object[] {"1.2.3-alpha", 1, 2, "3-alpha"}, + new Object[] {"1.2", 1, 2, ""}, + new Object[] {"1", 1, 0, ""}, + new Object[] {"1.", 1, 0, ""}); + } + + @ParameterizedTest + @MethodSource("validVersionProvider") + public void testParseVersion_validVersion( + String versionStr, int expectedMajor, int expectedMinor, String expectedRest) + throws Exception { + Method method = + VersionsCompatibilityChecker.class.getDeclaredMethod("parseVersion", String.class); + method.setAccessible(true); + Version version = (Version) method.invoke(null, versionStr); + assertEquals(expectedMajor, version.getMajor()); + assertEquals(expectedMinor, version.getMinor()); + assertEquals(expectedRest, version.getRest()); + } + + private static Stream invalidVersionProvider() { + return Stream.of("v1.12.0", "", ".1", ".1.", "1.null.1", "null.0.1", null); + } + + @ParameterizedTest + @MethodSource("invalidVersionProvider") + public void testParseVersion_invalidVersion(String versionStr) throws Exception { + Method method = + VersionsCompatibilityChecker.class.getDeclaredMethod("parseVersion", String.class); + method.setAccessible(true); + assertThrows( + InvocationTargetException.class, + () -> method.invoke(null, versionStr)); + } + + private static Stream versionCompatibilityProvider() { + return Stream.of( + new Object[] {"1.9.3.dev0", "2.8.1.dev12-something", false}, + new Object[] {"1.9", "2.8", false}, + new Object[] {"1", "2", false}, + new Object[] {"1.9.0", "2.9.0", false}, + new Object[] {"1.1.0", "1.2.9", true}, + new Object[] {"1.2.7", "1.1.8.dev0", true}, + new Object[] {"1.2.1", "1.2.29", true}, + new Object[] {"1.2.0", "1.2.0", true}, + new Object[] {"1.2.0", "1.4.0", false}, + new Object[] {"1.4.0", "1.2.0", false}, + new Object[] {"1.9.0", "3.7.0", false}, + new Object[] {"3.0.0", "1.0.0", false}, + new Object[] {"", "1.0.0", false}, + new Object[] {"1.0.0", "", false}, + new Object[] {"", "", false}); + } + + @ParameterizedTest + @MethodSource("versionCompatibilityProvider") + public void testIsCompatible(String clientVersion, String serverVersion, boolean expected) { + assertEquals(expected, VersionsCompatibilityChecker.isCompatible(clientVersion, serverVersion)); + } +} From 2318daa3d1b395fbf1980407afbbe2c8bf2cdb11 Mon Sep 17 00:00:00 2001 From: tellet-q Date: Fri, 20 Dec 2024 14:34:01 +0100 Subject: [PATCH 2/4] Use logger.warn --- src/main/java/io/qdrant/client/QdrantGrpcClient.java | 7 ++++--- .../io/qdrant/client/VersionsCompatibilityCheckerTest.java | 4 +--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/qdrant/client/QdrantGrpcClient.java b/src/main/java/io/qdrant/client/QdrantGrpcClient.java index 1e7169f..e6af15f 100644 --- a/src/main/java/io/qdrant/client/QdrantGrpcClient.java +++ b/src/main/java/io/qdrant/client/QdrantGrpcClient.java @@ -280,16 +280,17 @@ private void checkVersionsCompatibility(String clientVersion) { .healthCheck(QdrantOuterClass.HealthCheckRequest.getDefaultInstance()) .getVersion(); if (!VersionsCompatibilityChecker.isCompatible(clientVersion, serverVersion)) { - System.out.println( + String logMessage = "Qdrant client version " + clientVersion + " is incompatible with server version " + serverVersion + ". Major versions should match and minor version difference must not exceed 1. " - + "Set check_version=False to skip version check."); + + "Set check_version=False to skip version check."; + logger.warn(logMessage); } } catch (Exception e) { - System.out.println( + logger.warn( "Failed to obtain server version. Unable to check client-server compatibility. Set checkCompatibility=False to skip version check."); } } diff --git a/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java b/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java index d21bca1..3d7773c 100644 --- a/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java +++ b/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java @@ -43,9 +43,7 @@ public void testParseVersion_invalidVersion(String versionStr) throws Exception Method method = VersionsCompatibilityChecker.class.getDeclaredMethod("parseVersion", String.class); method.setAccessible(true); - assertThrows( - InvocationTargetException.class, - () -> method.invoke(null, versionStr)); + assertThrows(InvocationTargetException.class, () -> method.invoke(null, versionStr)); } private static Stream versionCompatibilityProvider() { From e56c800bd2d8a5f5ece747d953f12b5041475f0f Mon Sep 17 00:00:00 2001 From: tellet-q Date: Fri, 20 Dec 2024 14:35:22 +0100 Subject: [PATCH 3/4] Fix texts --- src/main/java/io/qdrant/client/QdrantGrpcClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/qdrant/client/QdrantGrpcClient.java b/src/main/java/io/qdrant/client/QdrantGrpcClient.java index e6af15f..e0ecf75 100644 --- a/src/main/java/io/qdrant/client/QdrantGrpcClient.java +++ b/src/main/java/io/qdrant/client/QdrantGrpcClient.java @@ -286,12 +286,12 @@ private void checkVersionsCompatibility(String clientVersion) { + " is incompatible with server version " + serverVersion + ". Major versions should match and minor version difference must not exceed 1. " - + "Set check_version=False to skip version check."; + + "Set checkCompatibility=false to skip version check."; logger.warn(logMessage); } } catch (Exception e) { logger.warn( - "Failed to obtain server version. Unable to check client-server compatibility. Set checkCompatibility=False to skip version check."); + "Failed to obtain server version. Unable to check client-server compatibility. Set checkCompatibility=false to skip version check."); } } } From c70a00ede2bd4259e61b13a4d017e5aea554e51b Mon Sep 17 00:00:00 2001 From: tellet-q Date: Thu, 2 Jan 2025 09:13:24 +0100 Subject: [PATCH 4/4] Address review --- .../client/VersionsCompatibilityChecker.java | 46 ++++--------------- .../VersionsCompatibilityCheckerTest.java | 14 +++--- 2 files changed, 15 insertions(+), 45 deletions(-) diff --git a/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java b/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java index e70c3e7..7ae733b 100644 --- a/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java +++ b/src/main/java/io/qdrant/client/VersionsCompatibilityChecker.java @@ -1,19 +1,15 @@ package io.qdrant.client; -import java.util.ArrayList; -import java.util.Arrays; import org.slf4j.Logger; import org.slf4j.LoggerFactory; class Version { private final int major; private final int minor; - private final String rest; - public Version(int major, int minor, String rest) { + public Version(int major, int minor) { this.major = major; this.minor = minor; - this.rest = rest; } public int getMajor() { @@ -23,10 +19,6 @@ public int getMajor() { public int getMinor() { return minor; } - - public String getRest() { - return rest; - } } /** Utility class to check compatibility between server's and client's versions. */ @@ -45,15 +37,11 @@ private static Version parseVersion(String version) throws IllegalArgumentExcept String[] parts = version.split("\\."); int major = parts.length > 0 ? Integer.parseInt(parts[0]) : 0; int minor = parts.length > 1 ? Integer.parseInt(parts[1]) : 0; - String rest = - parts.length > 2 - ? String.join(".", new ArrayList<>(Arrays.asList(parts).subList(2, parts.length))) - : ""; - return new Version(major, minor, rest); + return new Version(major, minor); } catch (Exception e) { throw new IllegalArgumentException( - "Unable to parse version, expected format: x.y.z, found: " + version, e); + "Unable to parse version, expected format: x.y[.z], found: " + version, e); } } @@ -65,31 +53,15 @@ private static Version parseVersion(String version) throws IllegalArgumentExcept * @return True if the versions are compatible, false otherwise. */ public static boolean isCompatible(String clientVersion, String serverVersion) { - if (clientVersion.isEmpty()) { - logger.warn("Unable to compare with client version {}", clientVersion); - return false; - } - - if (serverVersion.isEmpty()) { - logger.warn("Unable to compare with server version {}", serverVersion); - return false; - } - - if (clientVersion.equals(serverVersion)) { - return true; - } - try { - Version parsedServerVersion = parseVersion(serverVersion); - Version parsedClientVersion = parseVersion(clientVersion); + Version client = parseVersion(clientVersion); + Version server = parseVersion(serverVersion); + + if (client.getMajor() != server.getMajor()) return false; + return Math.abs(client.getMinor() - server.getMinor()) <= 1; - int majorDiff = Math.abs(parsedServerVersion.getMajor() - parsedClientVersion.getMajor()); - if (majorDiff >= 1) { - return false; - } - return Math.abs(parsedServerVersion.getMinor() - parsedClientVersion.getMinor()) <= 1; } catch (IllegalArgumentException e) { - logger.warn("Unable to compare versions: {}", e.getMessage()); + logger.warn("Version comparison failed: {}", e.getMessage()); return false; } } diff --git a/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java b/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java index 3d7773c..46cc4a5 100644 --- a/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java +++ b/src/test/java/io/qdrant/client/VersionsCompatibilityCheckerTest.java @@ -12,17 +12,16 @@ public class VersionsCompatibilityCheckerTest { private static Stream validVersionProvider() { return Stream.of( - new Object[] {"1.2.3", 1, 2, "3"}, - new Object[] {"1.2.3-alpha", 1, 2, "3-alpha"}, - new Object[] {"1.2", 1, 2, ""}, - new Object[] {"1", 1, 0, ""}, - new Object[] {"1.", 1, 0, ""}); + new Object[] {"1.2.3", 1, 2}, + new Object[] {"1.2.3-alpha", 1, 2}, + new Object[] {"1.2", 1, 2}, + new Object[] {"1", 1, 0}, + new Object[] {"1.", 1, 0}); } @ParameterizedTest @MethodSource("validVersionProvider") - public void testParseVersion_validVersion( - String versionStr, int expectedMajor, int expectedMinor, String expectedRest) + public void testParseVersion_validVersion(String versionStr, int expectedMajor, int expectedMinor) throws Exception { Method method = VersionsCompatibilityChecker.class.getDeclaredMethod("parseVersion", String.class); @@ -30,7 +29,6 @@ public void testParseVersion_validVersion( Version version = (Version) method.invoke(null, versionStr); assertEquals(expectedMajor, version.getMajor()); assertEquals(expectedMinor, version.getMinor()); - assertEquals(expectedRest, version.getRest()); } private static Stream invalidVersionProvider() {