From 74e5da49c78af0e7e3a145330afd2a570dd0d735 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Thu, 30 Oct 2025 11:08:10 -0400 Subject: [PATCH] fix: moving h2 logic out of Database so that it can be resolved (#43750) closes: #43687 Signed-off-by: Steve Hawkins --- .../keycloak/config/database/Database.java | 44 +-------------- .../mappers/DatabasePropertyMappers.java | 56 +++++++++++++++++-- .../DatasourcesConfigurationTest.java | 9 ++- 3 files changed, 61 insertions(+), 48 deletions(-) diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/database/Database.java b/quarkus/config-api/src/main/java/org/keycloak/config/database/Database.java index 51a5e251695..a6b052d0863 100644 --- a/quarkus/config-api/src/main/java/org/keycloak/config/database/Database.java +++ b/quarkus/config-api/src/main/java/org/keycloak/config/database/Database.java @@ -106,7 +106,7 @@ public final class Database { public String apply(String namedProperty, String alias) { if ("dev-file".equalsIgnoreCase(alias)) { var separator = escapeReplacements(File.separator); - return amendH2(new StringBuilder() + return new StringBuilder() .append("jdbc:h2:file:") .append("${kc.db-url-path:${kc.home.dir:%s}}".formatted(escapeReplacements(System.getProperty("user.home")))) .append(separator) @@ -115,10 +115,9 @@ public final class Database { .append(getFolder(namedProperty)) .append(separator) .append(getDbName(namedProperty)) - .append(getProperty(DatabaseOptions.DB_URL_PROPERTIES, namedProperty)) - .toString()); + .toString(); } - return amendH2("jdbc:h2:mem:%s%s".formatted(getDbName(namedProperty), getProperty(DatabaseOptions.DB_URL_PROPERTIES, namedProperty))); + return "jdbc:h2:mem:%s".formatted(getDbName(namedProperty)); } private String getFolder(String namedProperty) { @@ -139,43 +138,6 @@ public final class Database { return snippet; } - /** - * Starting with H2 version 2.x, marking "VALUE" as a non-keyword is necessary as some columns are named "VALUE" in the Keycloak schema. - *

- * Alternatives considered and rejected: - *

- * Downsides of this solution: Release notes needed to point out that any H2 JDBC URL parameter with NON_KEYWORDS needs to add the keyword VALUE manually. - * @return JDBC URL with NON_KEYWORDS=VALUE appended if the URL doesn't contain NON_KEYWORDS= yet - */ - private String addH2NonKeywords(String jdbcUrl) { - if (!jdbcUrl.contains("NON_KEYWORDS=")) { - jdbcUrl = jdbcUrl + ";NON_KEYWORDS=VALUE"; - } - return jdbcUrl; - } - - /** - * Required so that the H2 db instance is closed only when the Agroal connection pool is closed during - * Keycloak shutdown. We cannot rely on the default H2 ShutdownHook as this can result in the DB being - * closed before dependent resources, e.g. JDBC_PING2, are shutdown gracefully. This solution also - * requires the Agroal min-pool connection size to be at least 1. - */ - private String addH2CloseOnExit(String jdbcUrl) { - if (!jdbcUrl.contains("DB_CLOSE_ON_EXIT=")) { - jdbcUrl = jdbcUrl + ";DB_CLOSE_ON_EXIT=FALSE"; - } - if (!jdbcUrl.contains("DB_CLOSE_DELAY=")) { - jdbcUrl = jdbcUrl + ";DB_CLOSE_DELAY=0"; - } - return jdbcUrl; - } - - private String amendH2(String jdbcUrl) { - return addH2CloseOnExit(addH2NonKeywords(jdbcUrl)); - } }, "liquibase.database.core.H2Database", "dev-mem", "dev-file" diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java index 97f8aa8f3c6..44ae3630053 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java @@ -132,17 +132,12 @@ final class DatabasePropertyMappers implements PropertyMapperGrouping { String dbDriver = Configuration.getConfigValue(DatabaseOptions.DB_DRIVER).getValue(); String dbUrl = Configuration.getConfigValue(DatabaseOptions.DB_URL).getValue(); - String dbUrlProperties = Configuration.getConfigValue(DatabaseOptions.DB_URL_PROPERTIES).getValue(); if (!Objects.equals(Database.getDriver(db, true).orElse(null), dbDriver) && !Objects.equals(Database.getDriver(db, false).orElse(null), dbDriver)) { // Custom JDBC-Driver, for example, AWS JDBC Wrapper. return null; } - if (dbUrlProperties != null && dbUrl != null && dbUrl.contains("${kc.db-url-properties:}") && dbUrlProperties.contains("targetServerType")) { - // targetServerType already set to same or different value in db-url-properties, ignore - return null; - } if (dbUrl != null && dbUrl.contains("targetServerType")) { // targetServerType already set to same or different value in db-url, ignore return null; @@ -151,8 +146,57 @@ final class DatabasePropertyMappers implements PropertyMapperGrouping { return "primary"; } + /** + * Starting with H2 version 2.x, marking "VALUE" as a non-keyword is necessary as some columns are named "VALUE" in the Keycloak schema. + *

+ * Alternatives considered and rejected: + *

+ * Downsides of this solution: Release notes needed to point out that any H2 JDBC URL parameter with NON_KEYWORDS needs to add the keyword VALUE manually. + * @return JDBC URL with NON_KEYWORDS=VALUE appended if the URL doesn't contain NON_KEYWORDS= yet + */ + private static String addH2NonKeywords(String jdbcUrl) { + if (!jdbcUrl.contains("NON_KEYWORDS=")) { + jdbcUrl = jdbcUrl + ";NON_KEYWORDS=VALUE"; + } + return jdbcUrl; + } + + /** + * Required so that the H2 db instance is closed only when the Agroal connection pool is closed during + * Keycloak shutdown. We cannot rely on the default H2 ShutdownHook as this can result in the DB being + * closed before dependent resources, e.g. JDBC_PING2, are shutdown gracefully. This solution also + * requires the Agroal min-pool connection size to be at least 1. + */ + private static String addH2CloseOnExit(String jdbcUrl) { + if (!jdbcUrl.contains("DB_CLOSE_ON_EXIT=")) { + jdbcUrl = jdbcUrl + ";DB_CLOSE_ON_EXIT=FALSE"; + } + if (!jdbcUrl.contains("DB_CLOSE_DELAY=")) { + jdbcUrl = jdbcUrl + ";DB_CLOSE_DELAY=0"; + } + return jdbcUrl; + } + + private static String amendH2(String jdbcUrl) { + return addH2CloseOnExit(addH2NonKeywords(jdbcUrl)); + } + private static String getDatabaseUrl(String name, String value, ConfigSourceInterceptorContext c) { - return Database.getDefaultUrl(name, value).orElse(null); + String url = Database.getDefaultUrl(name, value).orElse(null); + if (isDevModeDatabase(value)) { + String key = Optional.ofNullable(name).map( + n -> DatabaseOptions.Datasources.getNamedKey(DatabaseOptions.DB_URL_PROPERTIES, n).orElseThrow()) + .orElse(DatabaseOptions.DB_URL_PROPERTIES.getKey()); + String urlProps = Configuration.getKcConfigValue(key).getValue(); + if (urlProps != null) { + url += urlProps; + } + url = amendH2(url); + } + return url; } private static String getXaOrNonXaDriver(String name, String value, ConfigSourceInterceptorContext context) { diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java index ef37609e274..ce7e03e7f01 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java @@ -101,7 +101,6 @@ public class DatasourcesConfigurationTest extends AbstractConfigurationTest { assertConfig("db-dialect-store", H2Dialect.class.getName()); // XA datasource is the default assertExternalConfig("quarkus.datasource.\"store\".jdbc.driver", JdbcDataSource.class.getName()); - assertExternalConfig("quarkus.datasource.\"store\".jdbc.url", "jdbc:h2:file:" + Environment.getHomeDir() + "/data/h2-store/keycloakdb-store;NON_KEYWORDS=VALUE;DB_CLOSE_ON_EXIT=FALSE;DB_CLOSE_DELAY=0"); onAfter(); ConfigArgsConfigSource.setCliArgs("--db-kind-store=dev-mem"); @@ -185,6 +184,14 @@ public class DatasourcesConfigurationTest extends AbstractConfigurationTest { "quarkus.datasource.\"asdf\".db-kind", "postgresql" )); onAfter(); + + ConfigArgsConfigSource.setCliArgs("--db-kind-asdf=dev-file", "--db-url-properties-asdf=;DB_CLOSE_ON_EXIT=true"); + initConfig(); + assertExternalConfig(Map.of( + "quarkus.datasource.\"asdf\".jdbc.url", "jdbc:h2:file:" + Environment.getHomeDir() + "/data/h2-asdf/keycloakdb-asdf;DB_CLOSE_ON_EXIT=true;NON_KEYWORDS=VALUE;DB_CLOSE_DELAY=0", + "quarkus.datasource.\"asdf\".db-kind", "h2" + )); + onAfter(); }