diff --git a/CHANGELOG.md b/CHANGELOG.md index 17cdf6a1f..9aa81003d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,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, jdbc-v2]** Reduced noisy and potentially sensitive logging; SQL that fails to parse is no longer logged at `WARN` (it could contain credentials/PII). (https://github.com/ClickHouse/clickhouse-java/issues/2970) - **[client-v2]** Fixed `BigDecimal` values written into a `Dynamic` column being silently truncated when the 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..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,11 +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; @@ -65,4 +80,110 @@ 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 testDefaultServiceFallback() { + Assert.assertNotNull(ClickHouseDnsResolver.getInstance()); + Assert.assertNotNull(ClickHouseRequestManager.getInstance()); + } + + @Test(groups = { "unit" }) + public void testServicesLoadFromNamedModuleProvider() throws Exception { + if (getJavaVersion() < 11) { + throw new SkipException("Requires Java 11 or later"); + } + + String baseDir = System.getProperty("basedir", "."); + 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 { +}