From fd2cf5d0c662ad3811444d2c8ef1c2cc4bff4f27 Mon Sep 17 00:00:00 2001 From: 014-code <2402143478@qq.com> Date: Mon, 20 Jul 2026 21:36:20 +0800 Subject: [PATCH 1/2] fix(clickhouse-client): load services from client module Load ClickHouseRequestManager and ClickHouseDnsResolver providers from the com.clickhouse.client module instead of delegating through ClickHouseUtils in com.clickhouse.data. This keeps JPMS ServiceLoader uses checks aligned with the module that owns the service types. Add the missing ClickHouseRequestManager uses directives to the Java 9 and Java 11 module descriptors and cover the fallback/module descriptor contract in ClickHouseClientTest. Fixes #2669 --- CHANGELOG.md | 5 +++++ .../client/ClickHouseDnsResolver.java | 16 +++++++++++++--- .../client/ClickHouseRequestManager.java | 16 +++++++++++++--- .../src/main/java11/module-info.java | 1 + .../src/main/java9/module-info.java | 1 + .../client/ClickHouseClientTest.java | 18 ++++++++++++++++++ 6 files changed, 51 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab70ea343..16c430e2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ### Bug Fixes +- **[clickhouse-client]** Fixed JPMS/module-path service loading for `ClickHouseRequestManager` by loading client + services from the `com.clickhouse.client` module, which declares the required `uses` directives. This avoids + `ServiceConfigurationError` failures from `com.clickhouse.data` when applications run on the module path. + (https://github.com/ClickHouse/clickhouse-java/issues/2669) + - **[client-v2]** Fixed binary varint decoding for length and count fields so overflowing or overlong values fail with an `IOException` instead of being decoded into corrupted or negative `int` values. (https://github.com/ClickHouse/clickhouse-java/issues/2902) - **[client-v2]** Fixed container query parameters being sent unquoted, so `Client.query(sql, params, settings)` binding diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseDnsResolver.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseDnsResolver.java index d3b949842..8acb16542 100644 --- a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseDnsResolver.java +++ b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseDnsResolver.java @@ -1,10 +1,10 @@ package com.clickhouse.client; import java.net.InetSocketAddress; +import java.util.ServiceLoader; import com.clickhouse.client.config.ClickHouseDefaults; import com.clickhouse.client.naming.SrvResolver; -import com.clickhouse.data.ClickHouseUtils; import com.clickhouse.logging.Logger; import com.clickhouse.logging.LoggerFactory; @@ -17,8 +17,18 @@ public class ClickHouseDnsResolver { private static final Logger log = LoggerFactory.getLogger(ClickHouseDnsResolver.class); - private static final ClickHouseDnsResolver instance = ClickHouseUtils.getService(ClickHouseDnsResolver.class, - new ClickHouseDnsResolver()); + private static final ClickHouseDnsResolver instance = loadResolver(); + + private static ClickHouseDnsResolver loadResolver() { + for (ClickHouseDnsResolver resolver : ServiceLoader.load(ClickHouseDnsResolver.class, + ClickHouseDnsResolver.class.getClassLoader())) { + if (resolver != null) { + return resolver; + } + } + + return new ClickHouseDnsResolver(); + } protected static ClickHouseDnsResolver newInstance() { ClickHouseDnsResolver resolver = null; diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequestManager.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequestManager.java index d0e0e69f4..f9bf9113b 100644 --- a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequestManager.java +++ b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequestManager.java @@ -1,9 +1,9 @@ package com.clickhouse.client; +import java.util.ServiceLoader; import java.util.UUID; import com.clickhouse.data.ClickHouseChecker; -import com.clickhouse.data.ClickHouseUtils; /** * Request manager is responsible for generating query and session ID, as well @@ -17,13 +17,23 @@ public class ClickHouseRequestManager { * Inner class for static initialization. */ static final class InstanceHolder { - private static final ClickHouseRequestManager instance = ClickHouseUtils - .getService(ClickHouseRequestManager.class, ClickHouseRequestManager::new); + private static final ClickHouseRequestManager instance = loadManager(); private InstanceHolder() { } } + private static ClickHouseRequestManager loadManager() { + for (ClickHouseRequestManager manager : ServiceLoader.load(ClickHouseRequestManager.class, + ClickHouseRequestManager.class.getClassLoader())) { + if (manager != null) { + return manager; + } + } + + return new ClickHouseRequestManager(); + } + /** * Gets instance of request manager. * diff --git a/clickhouse-client/src/main/java11/module-info.java b/clickhouse-client/src/main/java11/module-info.java index a503f3d6a..d4a095dd4 100644 --- a/clickhouse-client/src/main/java11/module-info.java +++ b/clickhouse-client/src/main/java11/module-info.java @@ -11,5 +11,6 @@ uses com.clickhouse.client.ClickHouseClient; uses com.clickhouse.client.ClickHouseDnsResolver; + uses com.clickhouse.client.ClickHouseRequestManager; uses com.clickhouse.client.ClickHouseSslContextProvider; } diff --git a/clickhouse-client/src/main/java9/module-info.java b/clickhouse-client/src/main/java9/module-info.java index a503f3d6a..d4a095dd4 100644 --- a/clickhouse-client/src/main/java9/module-info.java +++ b/clickhouse-client/src/main/java9/module-info.java @@ -11,5 +11,6 @@ uses com.clickhouse.client.ClickHouseClient; uses com.clickhouse.client.ClickHouseDnsResolver; + uses com.clickhouse.client.ClickHouseRequestManager; uses com.clickhouse.client.ClickHouseSslContextProvider; } diff --git a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java index eb6d2add8..a43c8cefd 100644 --- a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java +++ b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java @@ -5,6 +5,9 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.concurrent.ExecutionException; import com.clickhouse.client.ClickHouseRequest.Mutation; @@ -65,4 +68,19 @@ public void testMutation() throws ExecutionException, InterruptedException { Assert.assertNull(req.sql); Assert.assertNull(req.table("my_table").format(ClickHouseFormat.RowBinary).execute().get()); } + + @Test(groups = { "unit" }) + public void testClientModuleDeclaresLoadedServices() throws IOException { + Assert.assertNotNull(ClickHouseDnsResolver.getInstance()); + Assert.assertNotNull(ClickHouseRequestManager.getInstance()); + assertModuleInfoDeclaresUses("src/main/java9/module-info.java"); + assertModuleInfoDeclaresUses("src/main/java11/module-info.java"); + } + + private static void assertModuleInfoDeclaresUses(String moduleInfo) throws IOException { + String baseDir = System.getProperty("basedir", "."); + String content = new String(Files.readAllBytes(Paths.get(baseDir, moduleInfo)), StandardCharsets.UTF_8); + Assert.assertTrue(content.contains("uses com.clickhouse.client.ClickHouseDnsResolver;"), moduleInfo); + Assert.assertTrue(content.contains("uses com.clickhouse.client.ClickHouseRequestManager;"), moduleInfo); + } } From 004b70dd0a539cad83e28c9d2665d08ed9bfa02a Mon Sep 17 00:00:00 2001 From: 014-code <2402143478@qq.com> Date: Sat, 1 Aug 2026 19:17:35 +0800 Subject: [PATCH 2/2] test(clickhouse-client): cover JPMS service loading --- .../client/ClickHouseClientTest.java | 117 ++++++++++++++++-- .../jpms-service-provider/module-info.java | 6 + .../test/provider/Main.java | 18 +++ .../test/provider/TestDnsResolver.java | 6 + .../test/provider/TestRequestManager.java | 6 + 5 files changed, 146 insertions(+), 7 deletions(-) create mode 100644 clickhouse-client/src/test/resources/jpms-service-provider/module-info.java create mode 100644 clickhouse-client/src/test/resources/jpms-service-provider/test/provider/Main.java create mode 100644 clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestDnsResolver.java create mode 100644 clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestRequestManager.java diff --git a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java index a43c8cefd..38d0550c4 100644 --- a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java +++ b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseClientTest.java @@ -1,14 +1,26 @@ package com.clickhouse.client; import org.testng.Assert; +import org.testng.SkipException; import org.testng.annotations.Test; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.ExecutionException; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; import com.clickhouse.client.ClickHouseRequest.Mutation; import com.clickhouse.data.ClickHouseFormat; @@ -70,17 +82,108 @@ public void testMutation() throws ExecutionException, InterruptedException { } @Test(groups = { "unit" }) - public void testClientModuleDeclaresLoadedServices() throws IOException { + public void testDefaultServiceFallback() { Assert.assertNotNull(ClickHouseDnsResolver.getInstance()); Assert.assertNotNull(ClickHouseRequestManager.getInstance()); - assertModuleInfoDeclaresUses("src/main/java9/module-info.java"); - assertModuleInfoDeclaresUses("src/main/java11/module-info.java"); } - private static void assertModuleInfoDeclaresUses(String moduleInfo) throws IOException { + @Test(groups = { "unit" }) + public void testServicesLoadFromNamedModuleProvider() throws Exception { + if (getJavaVersion() < 11) { + throw new SkipException("Requires Java 11 or later"); + } + String baseDir = System.getProperty("basedir", "."); - String content = new String(Files.readAllBytes(Paths.get(baseDir, moduleInfo)), StandardCharsets.UTF_8); - Assert.assertTrue(content.contains("uses com.clickhouse.client.ClickHouseDnsResolver;"), moduleInfo); - Assert.assertTrue(content.contains("uses com.clickhouse.client.ClickHouseRequestManager;"), moduleInfo); + Path clientClasses = Paths.get(baseDir, "target", "classes"); + Path dataClasses = Paths.get(baseDir, "..", "clickhouse-data", "target", "classes").normalize(); + if (!Files.isRegularFile(clientClasses.resolve("META-INF/versions/11/module-info.class")) + || !Files.isRegularFile(dataClasses.resolve("META-INF/versions/11/module-info.class"))) { + throw new SkipException("Multi-release module descriptors were not compiled"); + } + + Path tempDir = Files.createTempDirectory("clickhouse-client-module-path-"); + try { + Path clientJar = tempDir.resolve("clickhouse-client.jar"); + Path dataJar = tempDir.resolve("clickhouse-data.jar"); + Path providerClasses = tempDir.resolve("provider-classes"); + createModuleJar(clientClasses, clientJar); + createModuleJar(dataClasses, dataJar); + + Path fixtures = Paths.get(baseDir, "src", "test", "resources", "jpms-service-provider"); + runProcess(Arrays.asList(javaTool("javac"), "--release", "11", "--module-path", + clientJar + File.pathSeparator + dataJar, "-d", providerClasses.toString(), + fixtures.resolve("module-info.java").toString(), fixtures.resolve("test/provider/TestRequestManager.java").toString(), + fixtures.resolve("test/provider/TestDnsResolver.java").toString(), + fixtures.resolve("test/provider/Main.java").toString())); + + runProcess(Arrays.asList(javaTool("java"), "--module-path", + clientJar + File.pathSeparator + dataJar + File.pathSeparator + providerClasses, "-m", + "test.clickhouse.client.provider/test.provider.Main")); + } finally { + deleteRecursively(tempDir); + } + } + + private static int getJavaVersion() { + String version = System.getProperty("java.specification.version"); + return Integer.parseInt(version.startsWith("1.") ? version.substring(2) : version); + } + + private static String javaTool(String name) { + String extension = System.getProperty("os.name").startsWith("Windows") ? ".exe" : ""; + return new File(new File(System.getProperty("java.home"), "bin"), name + extension).getAbsolutePath(); + } + + private static void runProcess(List command) throws IOException, InterruptedException { + Process process = new ProcessBuilder(command).redirectErrorStream(true).start(); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + for (int read; (read = process.getInputStream().read(buffer)) != -1;) { + output.write(buffer, 0, read); + } + + int exitCode = process.waitFor(); + Assert.assertEquals(exitCode, 0, "Command failed: " + command + "\n" + + new String(output.toByteArray(), StandardCharsets.UTF_8)); + } + + private static void createModuleJar(final Path classesDir, Path moduleJar) throws IOException { + Manifest manifest = new Manifest(); + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + manifest.getMainAttributes().putValue("Multi-Release", "true"); + + try (JarOutputStream output = new JarOutputStream(Files.newOutputStream(moduleJar), manifest)) { + Files.walkFileTree(classesDir, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attributes) throws IOException { + String entryName = classesDir.relativize(file).toString().replace(File.separatorChar, '/'); + if (!"META-INF/MANIFEST.MF".equalsIgnoreCase(entryName)) { + output.putNextEntry(new JarEntry(entryName)); + Files.copy(file, output); + output.closeEntry(); + } + return FileVisitResult.CONTINUE; + } + }); + } + } + + private static void deleteRecursively(Path path) throws IOException { + Files.walkFileTree(path, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attributes) throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path directory, IOException exception) throws IOException { + if (exception != null) { + throw exception; + } + Files.delete(directory); + return FileVisitResult.CONTINUE; + } + }); } } diff --git a/clickhouse-client/src/test/resources/jpms-service-provider/module-info.java b/clickhouse-client/src/test/resources/jpms-service-provider/module-info.java new file mode 100644 index 000000000..1dab7109a --- /dev/null +++ b/clickhouse-client/src/test/resources/jpms-service-provider/module-info.java @@ -0,0 +1,6 @@ +module test.clickhouse.client.provider { + requires com.clickhouse.client; + + provides com.clickhouse.client.ClickHouseDnsResolver with test.provider.TestDnsResolver; + provides com.clickhouse.client.ClickHouseRequestManager with test.provider.TestRequestManager; +} diff --git a/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/Main.java b/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/Main.java new file mode 100644 index 000000000..1d01a9060 --- /dev/null +++ b/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/Main.java @@ -0,0 +1,18 @@ +package test.provider; + +import com.clickhouse.client.ClickHouseDnsResolver; +import com.clickhouse.client.ClickHouseRequestManager; + +public final class Main { + private Main() { + } + + public static void main(String[] args) { + if (ClickHouseDnsResolver.getInstance().getClass() != TestDnsResolver.class) { + throw new AssertionError("ClickHouseDnsResolver provider was not loaded"); + } + if (ClickHouseRequestManager.getInstance().getClass() != TestRequestManager.class) { + throw new AssertionError("ClickHouseRequestManager provider was not loaded"); + } + } +} diff --git a/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestDnsResolver.java b/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestDnsResolver.java new file mode 100644 index 000000000..a9100550e --- /dev/null +++ b/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestDnsResolver.java @@ -0,0 +1,6 @@ +package test.provider; + +import com.clickhouse.client.ClickHouseDnsResolver; + +public class TestDnsResolver extends ClickHouseDnsResolver { +} diff --git a/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestRequestManager.java b/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestRequestManager.java new file mode 100644 index 000000000..9a2b95061 --- /dev/null +++ b/clickhouse-client/src/test/resources/jpms-service-provider/test/provider/TestRequestManager.java @@ -0,0 +1,6 @@ +package test.provider; + +import com.clickhouse.client.ClickHouseRequestManager; + +public class TestRequestManager extends ClickHouseRequestManager { +}