From 52925f72e93d67cd0fa5f78ca5119ce1bd98f0a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 19 Jul 2022 12:13:44 +0200 Subject: [PATCH] Consistently consider quoted AND non-quoted versions of quarkus.datasource configuration properties We were already taking both versions into account in DevServicesDatasourceConfigurationHandlerBuildItem, but not in every other place. --- .../DevServicesAdditionalConfigBuildItem.java | 28 +++++++- .../steps/DevServicesConfigBuildStep.java | 2 +- .../runtime/configuration/ConfigUtils.java | 44 ++++++++++++ .../common/runtime/DataSourceUtil.java | 12 ++++ ...tasourceConfigurationHandlerBuildItem.java | 69 ++++++------------- .../DevServicesDatasourceProcessor.java | 52 +++++++++----- .../orm/deployment/HibernateOrmProcessor.java | 12 ++-- 7 files changed, 141 insertions(+), 78 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesAdditionalConfigBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesAdditionalConfigBuildItem.java index 9f6df8de83d7e..bdc80c289f2f9 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesAdditionalConfigBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesAdditionalConfigBuildItem.java @@ -1,5 +1,8 @@ package io.quarkus.deployment.builditem; +import java.util.Collection; +import java.util.List; + import io.quarkus.builder.item.MultiBuildItem; /** @@ -12,21 +15,40 @@ */ public final class DevServicesAdditionalConfigBuildItem extends MultiBuildItem { - private final String triggeringKey; + private final Collection triggeringKeys; private final String key; private final String value; private final Runnable callbackWhenEnabled; + /** + * @deprecated Call + * {@link DevServicesAdditionalConfigBuildItem#DevServicesAdditionalConfigBuildItem(Collection, String, String, Runnable)} + * instead. + */ + @Deprecated public DevServicesAdditionalConfigBuildItem(String triggeringKey, String key, String value, Runnable callbackWhenEnabled) { - this.triggeringKey = triggeringKey; + this(List.of(triggeringKey), key, value, callbackWhenEnabled); + } + + public DevServicesAdditionalConfigBuildItem(Collection triggeringKeys, + String key, String value, Runnable callbackWhenEnabled) { + this.triggeringKeys = triggeringKeys; this.key = key; this.value = value; this.callbackWhenEnabled = callbackWhenEnabled; } + /** + * @deprecated Call {@link #getTriggeringKeys()} instead. + */ + @Deprecated public String getTriggeringKey() { - return triggeringKey; + return getTriggeringKeys().iterator().next(); + } + + public Collection getTriggeringKeys() { + return triggeringKeys; } public String getKey() { diff --git a/core/deployment/src/main/java/io/quarkus/deployment/steps/DevServicesConfigBuildStep.java b/core/deployment/src/main/java/io/quarkus/deployment/steps/DevServicesConfigBuildStep.java index 1bcd34ce307ca..3efba4098108f 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/steps/DevServicesConfigBuildStep.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/steps/DevServicesConfigBuildStep.java @@ -68,7 +68,7 @@ public void run() { // On contrary to dev services config, "additional" config build items are // produced on each restart, so we don't want to remember them from one restart to the next. for (DevServicesAdditionalConfigBuildItem item : devServicesAdditionalConfigBuildItems) { - if (newProperties.containsKey(item.getTriggeringKey())) { + if (item.getTriggeringKeys().stream().anyMatch(newProperties::containsKey)) { var callback = item.getCallbackWhenEnabled(); if (callback != null) { callback.run(); // This generally involves logging diff --git a/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigUtils.java b/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigUtils.java index c76dcb5702be0..7cf026e96b83e 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigUtils.java +++ b/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigUtils.java @@ -17,6 +17,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.OptionalInt; import java.util.Properties; import java.util.Set; @@ -25,6 +26,7 @@ import java.util.UUID; import java.util.function.IntFunction; +import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; import org.eclipse.microprofile.config.spi.ConfigSource; import org.eclipse.microprofile.config.spi.ConfigSourceProvider; @@ -270,6 +272,48 @@ public static boolean isPropertyPresent(String propertyName) { return ConfigProvider.getConfig().unwrap(SmallRyeConfig.class).isPropertyPresent(propertyName); } + /** + * Checks if any of the given properties is present in the current Configuration. + *

+ * Because the sources may not expose the property directly in {@link ConfigSource#getPropertyNames()}, we cannot + * reliably determine if the property is present in the properties list. The property needs to be retrieved to make + * sure it exists. Also, if the value is an expression, we want to ignore expansion, because this is not relevant + * for the check and the expansion value may not be available at this point. + *

+ * It may be interesting to expose such API in SmallRyeConfig directly. + * + * @param propertyNames The configuration property names + * @return true if the property is present or false otherwise. + */ + public static boolean isAnyPropertyPresent(Collection propertyNames) { + for (String propertyName : propertyNames) { + if (isPropertyPresent(propertyName)) { + return true; + } + } + return false; + } + + /** + * Get the value of the first given property present in the current Configuration, + * or {@link Optional#empty()} if none of the properties is present. + * + * @param The property type + * @param propertyNames The configuration property names + * @param propertyType The type that the resolved property value should be converted to + * @return true if the property is present or false otherwise. + */ + public static Optional getFirstOptionalValue(List propertyNames, Class propertyType) { + Config config = ConfigProvider.getConfig(); + for (String propertyName : propertyNames) { + Optional value = config.getOptionalValue(propertyName, propertyType); + if (value.isPresent()) { + return value; + } + } + return Optional.empty(); + } + /** * We override the EnvConfigSource, because we don't want the nothing back from getPropertiesNames at build time. * The mapping is one way and there is no way to map them back. diff --git a/extensions/datasource/common/src/main/java/io/quarkus/datasource/common/runtime/DataSourceUtil.java b/extensions/datasource/common/src/main/java/io/quarkus/datasource/common/runtime/DataSourceUtil.java index c79d86a8b36bd..7fdf31b27cf9e 100644 --- a/extensions/datasource/common/src/main/java/io/quarkus/datasource/common/runtime/DataSourceUtil.java +++ b/extensions/datasource/common/src/main/java/io/quarkus/datasource/common/runtime/DataSourceUtil.java @@ -1,6 +1,7 @@ package io.quarkus.datasource.common.runtime; import java.util.Collection; +import java.util.List; public final class DataSourceUtil { @@ -14,6 +15,17 @@ public static boolean hasDefault(Collection dataSourceNames) { return dataSourceNames.contains(DEFAULT_DATASOURCE_NAME); } + public static List dataSourcePropertyKeys(String datasourceName, String radical) { + if (datasourceName == null || DataSourceUtil.isDefault(datasourceName)) { + return List.of("quarkus.datasource." + radical); + } else { + // Two possible syntaxes: with or without quotes + return List.of( + "quarkus.datasource.\"" + datasourceName + "\"." + radical, + "quarkus.datasource." + datasourceName + "." + radical); + } + } + private DataSourceUtil() { } diff --git a/extensions/datasource/deployment-spi/src/main/java/io/quarkus/datasource/deployment/spi/DevServicesDatasourceConfigurationHandlerBuildItem.java b/extensions/datasource/deployment-spi/src/main/java/io/quarkus/datasource/deployment/spi/DevServicesDatasourceConfigurationHandlerBuildItem.java index bdbd71fd0c34a..d6574abd842b7 100644 --- a/extensions/datasource/deployment-spi/src/main/java/io/quarkus/datasource/deployment/spi/DevServicesDatasourceConfigurationHandlerBuildItem.java +++ b/extensions/datasource/deployment-spi/src/main/java/io/quarkus/datasource/deployment/spi/DevServicesDatasourceConfigurationHandlerBuildItem.java @@ -1,11 +1,14 @@ package io.quarkus.datasource.deployment.spi; -import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.function.BiFunction; +import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import io.quarkus.builder.item.MultiBuildItem; +import io.quarkus.datasource.common.runtime.DataSourceUtil; import io.quarkus.runtime.configuration.ConfigUtils; /** @@ -56,33 +59,24 @@ public static DevServicesDatasourceConfigurationHandlerBuildItem jdbc(String dbK public Map apply(String dsName, DevServicesDatasourceProvider.RunningDevServicesDatasource runningDevDb) { String jdbcUrl = runningDevDb.getJdbcUrl(); - if (dsName == null) { - return Collections.singletonMap("quarkus.datasource.jdbc.url", jdbcUrl); - } else { - // we use quoted and unquoted versions because depending on whether a user configured other JDBC properties - // one of the URLs may be ignored - // see https://github.com/quarkusio/quarkus/issues/21387 - return Map.of( - datasourceURLPropName(dsName), jdbcUrl, - datasourceURLPropName("\"" + dsName + "\""), jdbcUrl); - } + // we use datasourceURLPropNames to generate quoted and unquoted versions of the property key, + // because depending on whether a user configured other JDBC properties + // one of the URLs may be ignored + // see https://github.com/quarkusio/quarkus/issues/21387 + return datasourceURLPropNames(dsName).stream() + .collect(Collectors.toMap(Function.identity(), ignored -> jdbcUrl)); } }, new Predicate() { @Override public boolean test(String dsName) { - if (dsName == null) { - return ConfigUtils.isPropertyPresent("quarkus.datasource.jdbc.url"); - } else { - return ConfigUtils.isPropertyPresent(datasourceURLPropName(dsName)) || - ConfigUtils.isPropertyPresent(datasourceURLPropName("\"" + dsName + "\"")); - } + return ConfigUtils.isAnyPropertyPresent(datasourceURLPropNames(dsName)); } }); } - private static String datasourceURLPropName(String dsName) { - return String.format("quarkus.datasource.%s.jdbc.url", dsName); + private static List datasourceURLPropNames(String dsName) { + return DataSourceUtil.dataSourcePropertyKeys(dsName, "jdbc.url"); } public static DevServicesDatasourceConfigurationHandlerBuildItem reactive(String dbKind) { @@ -92,41 +86,22 @@ public static DevServicesDatasourceConfigurationHandlerBuildItem reactive(String public Map apply(String dsName, DevServicesDatasourceProvider.RunningDevServicesDatasource runningDevDb) { String reactiveUrl = runningDevDb.getReactiveUrl(); - if (dsName == null) { - return Collections.singletonMap("quarkus.datasource.reactive.url", reactiveUrl); - } else { - // we use quoted and unquoted versions because depending on whether a user configured other JDBC properties - // one of the URLs may be ignored - // see https://github.com/quarkusio/quarkus/issues/21387 - return Map.of( - datasourceReactiveURLPropName(dsName, false), reactiveUrl, - datasourceReactiveURLPropName(dsName, true), reactiveUrl); - } + // we use datasourceURLPropNames to generate quoted and unquoted versions of the property key, + // because depending on whether a user configured other reactive properties + // one of the URLs may be ignored + // see https://github.com/quarkusio/quarkus/issues/21387 + return datasourceReactiveURLPropNames(dsName).stream() + .collect(Collectors.toMap(Function.identity(), ignored -> reactiveUrl)); } }, new Predicate() { @Override public boolean test(String dsName) { - if (dsName == null) { - return ConfigUtils.isPropertyPresent("quarkus.datasource.reactive.url"); - } else { - return ConfigUtils.isPropertyPresent(datasourceReactiveURLPropName(dsName, false)) || - ConfigUtils.isPropertyPresent(datasourceReactiveURLPropName(dsName, true)); - } + return ConfigUtils.isAnyPropertyPresent(datasourceReactiveURLPropNames(dsName)); } }); } - private static String datasourceReactiveURLPropName(String dsName, boolean quotedName) { - StringBuilder key = new StringBuilder("quarkus.datasource"); - key.append('.'); - if (quotedName) { - key.append('"'); - } - key.append(dsName); - if (quotedName) { - key.append('"'); - } - key.append(".reactive.url"); - return key.toString(); + private static List datasourceReactiveURLPropNames(String dsName) { + return DataSourceUtil.dataSourcePropertyKeys(dsName, "reactive.url"); } } diff --git a/extensions/datasource/deployment/src/main/java/io/quarkus/datasource/deployment/devservices/DevServicesDatasourceProcessor.java b/extensions/datasource/deployment/src/main/java/io/quarkus/datasource/deployment/devservices/DevServicesDatasourceProcessor.java index e567ae5854edb..2296040de1355 100644 --- a/extensions/datasource/deployment/src/main/java/io/quarkus/datasource/deployment/devservices/DevServicesDatasourceProcessor.java +++ b/extensions/datasource/deployment/src/main/java/io/quarkus/datasource/deployment/devservices/DevServicesDatasourceProcessor.java @@ -13,6 +13,7 @@ import org.eclipse.microprofile.config.ConfigProvider; import org.jboss.logging.Logger; +import io.quarkus.datasource.common.runtime.DataSourceUtil; import io.quarkus.datasource.deployment.spi.DefaultDataSourceDbKindBuildItem; import io.quarkus.datasource.deployment.spi.DevServicesDatasourceConfigurationHandlerBuildItem; import io.quarkus.datasource.deployment.spi.DevServicesDatasourceContainerConfig; @@ -35,6 +36,7 @@ import io.quarkus.deployment.logging.LoggingSetupBuildItem; import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem; import io.quarkus.runtime.LaunchMode; +import io.quarkus.runtime.configuration.ConfigUtils; public class DevServicesDatasourceProcessor { @@ -257,11 +259,6 @@ private RunningDevService startDevDb(String dbName, consoleInstalledBuildItem, loggingSetupBuildItem); try { - String prefix = "quarkus.datasource."; - if (dbName != null) { - prefix = prefix + dbName + "."; - } - DevServicesDatasourceContainerConfig containerConfig = new DevServicesDatasourceContainerConfig( dataSourceBuildTimeConfig.devservices.imageName, dataSourceBuildTimeConfig.devservices.containerProperties, @@ -273,26 +270,34 @@ private RunningDevService startDevDb(String dbName, dataSourceBuildTimeConfig.devservices.password); DevServicesDatasourceProvider.RunningDevServicesDatasource datasource = devDbProvider - .startDatabase(ConfigProvider.getConfig().getOptionalValue(prefix + "username", String.class), - ConfigProvider.getConfig().getOptionalValue(prefix + "password", String.class), + .startDatabase( + ConfigUtils.getFirstOptionalValue(DataSourceUtil.dataSourcePropertyKeys(dbName, "username"), + String.class), + ConfigUtils.getFirstOptionalValue(DataSourceUtil.dataSourcePropertyKeys(dbName, "password"), + String.class), Optional.ofNullable(dbName), containerConfig, launchMode, globalDevServicesConfig.timeout); - propertiesMap.put(prefix + "db-kind", dataSourceBuildTimeConfig.dbKind.orElse(null)); - String devServicesPrefix = prefix + "devservices."; + for (String key : DataSourceUtil.dataSourcePropertyKeys(dbName, "db-kind")) { + propertiesMap.put(key, dataSourceBuildTimeConfig.dbKind.orElse(null)); + } + String devServicesPrefix = "devservices."; if (dataSourceBuildTimeConfig.devservices.command.isPresent()) { - propertiesMap.put(devServicesPrefix + "command", dataSourceBuildTimeConfig.devservices.command.get()); + setDataSourceProperties(propertiesMap, dbName, devServicesPrefix + "command", + dataSourceBuildTimeConfig.devservices.command.get()); } if (dataSourceBuildTimeConfig.devservices.imageName.isPresent()) { - propertiesMap.put(devServicesPrefix + "image-name", dataSourceBuildTimeConfig.devservices.imageName.get()); + setDataSourceProperties(propertiesMap, dbName, devServicesPrefix + "image-name", + dataSourceBuildTimeConfig.devservices.imageName.get()); } if (dataSourceBuildTimeConfig.devservices.port.isPresent()) { - propertiesMap.put(devServicesPrefix + "port", + setDataSourceProperties(propertiesMap, dbName, devServicesPrefix + "port", Integer.toString(dataSourceBuildTimeConfig.devservices.port.getAsInt())); } if (!dataSourceBuildTimeConfig.devservices.properties.isEmpty()) { for (var e : dataSourceBuildTimeConfig.devservices.properties.entrySet()) { - propertiesMap.put(devServicesPrefix + "properties." + e.getKey(), e.getValue()); + setDataSourceProperties(propertiesMap, dbName, devServicesPrefix + "properties." + e.getKey(), + e.getValue()); } } @@ -301,21 +306,23 @@ private RunningDevService startDevDb(String dbName, devDebProperties.putAll(devDbConfigurationHandlerBuildItem.getConfigProviderFunction() .apply(dbName, datasource)); } - devDebProperties.put(prefix + "db-kind", defaultDbKind.get()); + setDataSourceProperties(devDebProperties, dbName, "db-kind", defaultDbKind.get()); if (datasource.getUsername() != null) { - devDebProperties.put(prefix + "username", datasource.getUsername()); + setDataSourceProperties(devDebProperties, dbName, "username", datasource.getUsername()); } if (datasource.getPassword() != null) { - devDebProperties.put(prefix + "password", datasource.getPassword()); + setDataSourceProperties(devDebProperties, dbName, "password", datasource.getPassword()); } compressor.close(); log.info("Dev Services for " + prettyName + " (" + defaultDbKind.get() + ") started."); - String devservices = prefix + "devservices."; + List devservicesPrefixes = DataSourceUtil.dataSourcePropertyKeys(dbName, "devservices."); for (var name : ConfigProvider.getConfig().getPropertyNames()) { - if (name.startsWith(devservices)) { - devDebProperties.put(name, ConfigProvider.getConfig().getValue(name, String.class)); + for (String prefix : devservicesPrefixes) { + if (name.startsWith(prefix)) { + devDebProperties.put(name, ConfigProvider.getConfig().getValue(name, String.class)); + } } } return new RunningDevService(defaultDbKind.get(), datasource.getId(), datasource.getCloseTask(), devDebProperties); @@ -325,6 +332,13 @@ private RunningDevService startDevDb(String dbName, } } + private void setDataSourceProperties(Map propertiesMap, String dbName, String propertyKeyRadical, + String value) { + for (String key : DataSourceUtil.dataSourcePropertyKeys(dbName, propertyKeyRadical)) { + propertiesMap.put(key, value); + } + } + private DevServicesDatasourceResultBuildItem.DbResult toDbResult(RunningDevService devService) { if (devService == null) { return null; diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java index 832e28ab7950c..b7d286deec9e3 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java @@ -273,22 +273,18 @@ void devServicesAutoGenerateByDefault(List s .collect(HashSet::new, Collection::addAll, Collection::addAll); for (Entry entry : config.getAllPersistenceUnitConfigsAsMap().entrySet()) { - String propertyKeyIndicatingDataSourceConfigured; Optional dataSourceName = entry.getValue().datasource; - if (dataSourceName.isEmpty()) { - propertyKeyIndicatingDataSourceConfigured = "quarkus.datasource.username"; - } else { - propertyKeyIndicatingDataSourceConfigured = "quarkus.datasource." + dataSourceName.get() + ".username"; - } + List propertyKeysIndicatingDataSourceConfigured = DataSourceUtil + .dataSourcePropertyKeys(dataSourceName.orElse(null), "username"); if (!managedSources.contains(dataSourceName.orElse(DataSourceUtil.DEFAULT_DATASOURCE_NAME))) { String databaseGenerationPropertyKey = HibernateOrmRuntimeConfig.puPropertyKey(entry.getKey(), "database.generation"); - if (!ConfigUtils.isPropertyPresent(propertyKeyIndicatingDataSourceConfigured) + if (!ConfigUtils.isAnyPropertyPresent(propertyKeysIndicatingDataSourceConfigured) && !ConfigUtils.isPropertyPresent(databaseGenerationPropertyKey)) { String forcedValue = "drop-and-create"; devServicesAdditionalConfigProducer - .produce(new DevServicesAdditionalConfigBuildItem(propertyKeyIndicatingDataSourceConfigured, + .produce(new DevServicesAdditionalConfigBuildItem(propertyKeysIndicatingDataSourceConfigured, databaseGenerationPropertyKey, forcedValue, () -> LOG.infof("Setting %s=%s to initialize Dev Services managed database", databaseGenerationPropertyKey, forcedValue)));