From 02e29664a39e7b9622bb42b39e8d255eefcaa71f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 25 Oct 2023 11:40:48 +0200 Subject: [PATCH] More comprehensive registration/release of config in some tests If someone calls ConfigProvider.getConfig() out of the blue in a test that doesn't use any Quarkus*Test extension, this will indirectly call QuarkusConfigFactory and leak config in two ways: 1. In QuarkusConfigFactory#config 2. In SmallRyeConfigProviderResolver, registering config for the TCCL, which in such a case is most likely the system CL. Thus, a well-behaved test should call QuarkusConfigFactory.setConfig(null) to clean up all that, no just SmallRyeConfigProviderResolver.releaseConfig(). Similarly, tests that register configuration explicitly can just call QuarkusConfigFactory.setConfig(config) at the beginning and QuarkusConfigFactory.setConfig(null) at the end, which will properly simulate how a real Quarkus application behaves, and should cover all edge cases involving multiple classloaders, properly cleaning up everything at the end of the test. --- .../yaml/runtime/ApplicationYamlTest.java | 16 +++------------- .../restclient/config/RestClientConfigTest.java | 17 ++++++++--------- .../restclient/runtime/RestClientBaseTest.java | 12 ------------ .../RestClientCDIDelegateBuilderTest.java | 12 ------------ .../ConfigMappingStartupValidatorTest.java | 12 +----------- .../io/quarkus/test/common/LauncherUtil.java | 9 --------- .../test/common/TestResourceManager.java | 5 ++--- 7 files changed, 14 insertions(+), 69 deletions(-) diff --git a/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java b/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java index ec9621c9d49d85..29aa7fb2ff1024 100644 --- a/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java +++ b/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java @@ -5,7 +5,6 @@ import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -21,19 +20,12 @@ */ public class ApplicationYamlTest { - static volatile SmallRyeConfig config; - @BeforeAll public static void doBefore() { SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder(); builder.addDefaultSources().addDiscoveredConverters().addDiscoveredSources(); - QuarkusConfigFactory.setConfig(config = builder.build()); - Config conf = ConfigProvider.getConfig(); - if (conf != config) { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(conf); - ConfigProvider.getConfig(); - } + SmallRyeConfig config = builder.build(); + QuarkusConfigFactory.setConfig(config); System.out.println(System.getProperty("user.dir")); } @@ -47,8 +39,6 @@ public void testBasicApplicationYaml() { @AfterAll public static void doAfter() { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(config); - config = null; + QuarkusConfigFactory.setConfig(null); } } diff --git a/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java b/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java index 8ce5ca469dd374..51997b5a9c47f8 100644 --- a/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java +++ b/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java @@ -8,29 +8,28 @@ import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.spi.ConfigBuilder; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.rest.client.ext.QueryParamStyle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import io.quarkus.runtime.configuration.QuarkusConfigFactory; import io.smallrye.config.PropertiesConfigSource; +import io.smallrye.config.SmallRyeConfigBuilder; +import io.smallrye.config.SmallRyeConfigProviderResolver; public class RestClientConfigTest { - private static ClassLoader classLoader; - private static ConfigProviderResolver configProviderResolver; + private static SmallRyeConfigProviderResolver configProviderResolver; @BeforeAll public static void initConfig() { - classLoader = Thread.currentThread().getContextClassLoader(); - configProviderResolver = ConfigProviderResolver.instance(); + configProviderResolver = (SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver.instance(); } @AfterEach public void releaseConfig() { - configProviderResolver.releaseConfig(configProviderResolver.getConfig()); + QuarkusConfigFactory.setConfig(null); } @Test @@ -76,10 +75,10 @@ private void verifyConfig(RestClientConfig config) { } private static void setupMPConfig() throws IOException { - ConfigBuilder configBuilder = configProviderResolver.getBuilder(); + SmallRyeConfigBuilder configBuilder = configProviderResolver.getBuilder(); URL propertyFile = RestClientConfigTest.class.getClassLoader().getResource("application.properties"); assertThat(propertyFile).isNotNull(); configBuilder.withSources(new PropertiesConfigSource(propertyFile)); - configProviderResolver.registerConfig(configBuilder.build(), classLoader); + QuarkusConfigFactory.setConfig(configBuilder.build()); } } diff --git a/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java b/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java index 858fcb8bd346f4..52f2b42a2aa3f2 100644 --- a/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java +++ b/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java @@ -21,13 +21,10 @@ import jakarta.ws.rs.client.ClientResponseContext; import jakarta.ws.rs.client.ClientResponseFilter; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.rest.client.RestClientBuilder; import org.eclipse.microprofile.rest.client.ext.QueryParamStyle; import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -42,7 +39,6 @@ public class RestClientBaseTest { private static Path truststorePath; private static Path keystorePath; - private static Config createdConfig; @BeforeAll public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { @@ -65,14 +61,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat } } - @AfterEach - public void afterEach() { - if (createdConfig != null) { - ConfigProviderResolver.instance().releaseConfig(createdConfig); - createdConfig = null; - } - } - @AfterAll public static void afterAll() { if (truststorePath != null) { diff --git a/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java b/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java index 36556d81add905..f028d6841b9dfa 100644 --- a/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java +++ b/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java @@ -16,12 +16,9 @@ import jakarta.ws.rs.client.ClientResponseContext; import jakarta.ws.rs.client.ClientResponseFilter; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.rest.client.ext.QueryParamStyle; import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; import org.jboss.resteasy.reactive.client.api.QuarkusRestClientProperties; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -42,7 +39,6 @@ public class RestClientCDIDelegateBuilderTest { static File tempDir; private static File truststoreFile; private static File keystoreFile; - private static Config createdConfig; @BeforeAll public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { @@ -60,14 +56,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat keystore.store(new FileOutputStream(keystoreFile), KEYSTORE_PASSWORD.toCharArray()); } - @AfterEach - public void afterEach() { - if (createdConfig != null) { - ConfigProviderResolver.instance().releaseConfig(createdConfig); - createdConfig = null; - } - } - @Test public void testClientSpecificConfigs() { // given diff --git a/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java b/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java index 3abcdfc7058e22..16652a5a9c2131 100644 --- a/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java +++ b/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java @@ -11,9 +11,6 @@ import jakarta.inject.Inject; import jakarta.validation.constraints.Pattern; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -88,19 +85,12 @@ public void validateMapping(Class mappingClass, String prefix, Object mapping } }); QuarkusConfigFactory.setConfig(smallryeConfig = builder.build()); - Config conf = ConfigProvider.getConfig(); - if (conf != smallryeConfig) { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(conf); - ConfigProvider.getConfig(); - } suppressedConfigValidatorExceptions = new HashMap<>(); } @AfterAll public static void doAfter() { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(smallryeConfig); + QuarkusConfigFactory.setConfig(null); smallryeConfig = null; suppressedConfigValidatorExceptions = null; } diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java b/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java index 3742af25af83f5..544514617d4881 100644 --- a/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java +++ b/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java @@ -22,7 +22,6 @@ import java.util.regex.Pattern; import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import io.quarkus.runtime.LaunchMode; import io.quarkus.runtime.configuration.ConfigUtils; @@ -41,14 +40,6 @@ private LauncherUtil() { public static Config installAndGetSomeConfig() { final SmallRyeConfig config = ConfigUtils.configBuilder(false, LaunchMode.NORMAL).build(); QuarkusConfigFactory.setConfig(config); - final ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - try { - final Config installed = cpr.getConfig(); - if (installed != config) { - cpr.releaseConfig(installed); - } - } catch (IllegalStateException ignored) { - } return config; } diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java index 9e088371f3fdee..3326508b5ef37c 100644 --- a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java +++ b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java @@ -31,7 +31,7 @@ import org.jboss.jandex.DotName; import org.jboss.jandex.IndexView; -import io.smallrye.config.SmallRyeConfigProviderResolver; +import io.quarkus.runtime.configuration.QuarkusConfigFactory; public class TestResourceManager implements Closeable { @@ -180,8 +180,7 @@ public void close() { } } try { - ((SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver.instance()) - .releaseConfig(Thread.currentThread().getContextClassLoader()); + QuarkusConfigFactory.setConfig(null); } catch (Throwable ignored) { } configProperties.clear();