From 97c4495c4f1bd38a6ce04218de64b8fe8ed069cb Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Fri, 7 Oct 2022 11:38:00 +0200 Subject: [PATCH] Updating H2 database to 2.x Closes #12607 Co-authored-by: Stian Thorgersen --- model/jpa/pom.xml | 2 +- .../DefaultJpaConnectionProviderFactory.java | 38 +++++++++++++++++-- .../events/jpa/JpaEventStoreProvider.java | 31 +++++++++++---- pom.xml | 6 +-- .../keycloak/config/database/Database.java | 24 ++++++++++-- .../configuration/test/ConfigurationTest.java | 8 ++-- testsuite/integration-arquillian/pom.xml | 2 +- testsuite/model/pom.xml | 2 +- 8 files changed, 88 insertions(+), 25 deletions(-) diff --git a/model/jpa/pom.xml b/model/jpa/pom.xml index dd35a78442..fc7386ce34 100755 --- a/model/jpa/pom.xml +++ b/model/jpa/pom.xml @@ -34,7 +34,7 @@ keycloak sa - jdbc:h2:mem:test;MVCC=TRUE;DB_CLOSE_DELAY=-1 + jdbc:h2:mem:test;DB_CLOSE_DELAY=-1 com.h2database h2 ${h2.version} diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java index 1f63166b67..ea612bb913 100755 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java @@ -175,8 +175,13 @@ public class DefaultJpaConnectionProviderFactory implements JpaConnectionProvide properties.put(AvailableSettings.JPA_NON_JTA_DATASOURCE, dataSource); } } else { - properties.put(AvailableSettings.JPA_JDBC_URL, config.get("url")); - properties.put(AvailableSettings.JPA_JDBC_DRIVER, config.get("driver")); + String url = config.get("url"); + String driver = config.get("driver"); + if (driver.equals("org.h2.Driver")) { + url = addH2NonKeywords(url); + } + properties.put(AvailableSettings.JPA_JDBC_URL, url); + properties.put(AvailableSettings.JPA_JDBC_DRIVER, driver); String user = config.get("user"); if (user != null) { @@ -319,6 +324,7 @@ public class DefaultJpaConnectionProviderFactory implements JpaConnectionProvide return sql2012Dialect; } } + // For Oracle19c, we may need to set dialect explicitly to workaround https://hibernate.atlassian.net/browse/HHH-13184 if (dbProductName.equals("Oracle") && connection.getMetaData().getDatabaseMajorVersion() > 12) { logger.debugf("Manually specify dialect for Oracle to org.hibernate.dialect.Oracle12cDialect"); @@ -413,8 +419,13 @@ public class DefaultJpaConnectionProviderFactory implements JpaConnectionProvide DataSource dataSource = (DataSource) new InitialContext().lookup(dataSourceLookup); return dataSource.getConnection(); } else { - Class.forName(config.get("driver")); - return DriverManager.getConnection(StringPropertyReplacer.replaceProperties(config.get("url"), System.getProperties()), config.get("user"), config.get("password")); + String url = config.get("url"); + String driver = config.get("driver"); + if (driver.equals("org.h2.Driver")) { + url = addH2NonKeywords(url); + } + Class.forName(driver); + return DriverManager.getConnection(StringPropertyReplacer.replaceProperties(url, System.getProperties()), config.get("user"), config.get("password")); } } catch (Exception e) { throw new RuntimeException("Failed to connect to database", e); @@ -448,4 +459,23 @@ public class DefaultJpaConnectionProviderFactory implements JpaConnectionProvide private void migrateModel(KeycloakSession session) { KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), MigrationModelManager::migrate); } + + /** + * 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; + } + } diff --git a/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java b/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java index 441f937410..2ffe4e9fec 100755 --- a/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java +++ b/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java @@ -276,17 +276,32 @@ public class JpaEventStoreProvider implements EventStoreProvider { CriteriaBuilder cb = em.getCriteriaBuilder(); CriteriaQuery cr = cb.createQuery(RealmAttributeEntity.class); Root root = cr.from(RealmAttributeEntity.class); - cr.select(root).where(cb.and(cb.equal(root.get("name"),RealmAttributes.ADMIN_EVENTS_EXPIRATION),cb.greaterThan(root.get("value").as(Long.class),Long.valueOf(0)))); - Map> realms = em.createQuery(cr).getResultStream().collect(Collectors.groupingBy(attribute -> Long.valueOf(attribute.getValue()))); + // unable to cast the CLOB to a BIGINT in the select for H2 2.x, therefore comparing strings only in the DB, and filtering again in the next statement + cr.select(root).where(cb.and(cb.equal(root.get("name"),RealmAttributes.ADMIN_EVENTS_EXPIRATION),cb.notEqual(root.get("value"), "0"))); + Map> realms = em.createQuery(cr).getResultStream() + // filtering again on the attribute as paring the CLOB to BIGINT didn't work in H2 2.x + .filter(attribute -> { + try { + return Long.parseLong(attribute.getValue()) > 0; + } catch (NumberFormatException ex) { + logger.warnf("Unable to parse value '%s' for attribute '%s' in realm '%s' (expecting it to be decimal numeric)", + attribute.getValue(), + RealmAttributes.ADMIN_EVENTS_EXPIRATION, + attribute.getRealm().getId(), + ex); + return false; + } + }) + .collect(Collectors.groupingBy(attribute -> Long.valueOf(attribute.getValue()))); long current = Time.currentTimeMillis(); - realms.entrySet().forEach(entry -> { - List realmIds = entry.getValue().stream().map(RealmAttributeEntity::getRealm).map(RealmEntity::getId).collect(Collectors.toList()); + realms.forEach((key, value) -> { + List realmIds = value.stream().map(RealmAttributeEntity::getRealm).map(RealmEntity::getId).collect(Collectors.toList()); int currentNumDeleted = em.createQuery("delete from AdminEventEntity where realmId in :realmIds and time < :eventTime") - .setParameter("realmIds", realmIds) - .setParameter("eventTime", current - (Long.valueOf(entry.getKey()) * 1000)) - .executeUpdate(); - logger.tracef("Deleted %d admin events for the expiration %d", currentNumDeleted, entry.getKey()); + .setParameter("realmIds", realmIds) + .setParameter("eventTime", current - (key * 1000)) + .executeUpdate(); + logger.tracef("Deleted %d admin events for the expiration %d", currentNumDeleted, key); }); } } diff --git a/pom.xml b/pom.xml index d44509cf9d..232de97a65 100644 --- a/pom.xml +++ b/pom.xml @@ -88,10 +88,10 @@ 3.3.10 3.3.10 2.1.3 - 1.4.197 + 2.1.214 2.2.3 - 5.3.24.Final - 5.3.24.Final + 5.6.10.Final + 5.6.10.Final 13.0.10.Final 4.4.1.Final 1.3.2 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 cc8d429813..5f23772c37 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 @@ -107,11 +107,29 @@ public final class Database { @Override public String apply(String alias) { if ("dev-file".equalsIgnoreCase(alias)) { - return "jdbc:h2:file:${kc.home.dir:${kc.db-url-path:" + System.getProperty("user.home") + "}}" + File.separator + "${kc.data.dir:data}" + return addH2NonKeywords("jdbc:h2:file:${kc.home.dir:${kc.db-url-path:" + System.getProperty("user.home") + "}}" + File.separator + "${kc.data.dir:data}" + File.separator + "h2" + File.separator - + "keycloakdb${kc.db-url-properties:;;AUTO_SERVER=TRUE}"; + + "keycloakdb${kc.db-url-properties:;;AUTO_SERVER=TRUE}"); } - return "jdbc:h2:mem:keycloakdb${kc.db-url-properties:}"; + return addH2NonKeywords("jdbc:h2:mem:keycloakdb${kc.db-url-properties:}"); + } + + /** + * 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: + *

    + *
  • customizing H2 Database dialect -> wouldn't work for existing Liquibase scripts.
    • + *
  • adding quotes to @Column(name="VALUE") annotations -> would require testing for all DBs, wouldn't work for existing Liquibase scripts.
    • + *
+ * 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; } }, asList("liquibase.database.core.H2Database"), diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java index f49ff376f8..4d7158d244 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java @@ -252,12 +252,12 @@ public class ConfigurationTest { System.setProperty(CLI_ARGS, "--db=dev-file"); SmallRyeConfig config = createConfig(); assertEquals(QuarkusH2Dialect.class.getName(), config.getConfigValue("quarkus.hibernate-orm.dialect").getValue()); - assertEquals("jdbc:h2:file:" + System.getProperty("user.home") + File.separator + "data" + File.separator + "h2" + File.separator + "keycloakdb;;AUTO_SERVER=TRUE", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); + assertEquals("jdbc:h2:file:" + System.getProperty("user.home") + File.separator + "data" + File.separator + "h2" + File.separator + "keycloakdb;;AUTO_SERVER=TRUE;NON_KEYWORDS=VALUE", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); System.setProperty(CLI_ARGS, "--db=dev-mem"); config = createConfig(); assertEquals(QuarkusH2Dialect.class.getName(), config.getConfigValue("quarkus.hibernate-orm.dialect").getValue()); - assertEquals("jdbc:h2:mem:keycloakdb", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); + assertEquals("jdbc:h2:mem:keycloakdb;NON_KEYWORDS=VALUE", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); assertEquals("h2", config.getConfigValue("quarkus.datasource.db-kind").getValue()); System.setProperty(CLI_ARGS, "--db=dev-mem" + ARG_SEPARATOR + "--db-username=other"); @@ -320,13 +320,13 @@ public class ConfigurationTest { System.setProperty(CLI_ARGS, "--db=dev-file"); SmallRyeConfig config = createConfig(); assertEquals(QuarkusH2Dialect.class.getName(), config.getConfigValue("quarkus.hibernate-orm.dialect").getValue()); - assertEquals("jdbc:h2:file:test-dir" + File.separator + "data" + File.separator + "h2" + File.separator + "keycloakdb;;test=test;test1=test1", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); + assertEquals("jdbc:h2:file:test-dir" + File.separator + "data" + File.separator + "h2" + File.separator + "keycloakdb;;test=test;test1=test1;NON_KEYWORDS=VALUE", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); assertEquals("xa", config.getConfigValue("quarkus.datasource.jdbc.transactions").getValue()); System.setProperty(CLI_ARGS, ""); config = createConfig(); assertEquals(QuarkusH2Dialect.class.getName(), config.getConfigValue("quarkus.hibernate-orm.dialect").getValue()); - assertEquals("jdbc:h2:file:test-dir" + File.separator + "data" + File.separator + "h2" + File.separator + "keycloakdb;;test=test;test1=test1", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); + assertEquals("jdbc:h2:file:test-dir" + File.separator + "data" + File.separator + "h2" + File.separator + "keycloakdb;;test=test;test1=test1;NON_KEYWORDS=VALUE", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); System.setProperty("kc.db-url-properties", "?test=test&test1=test1"); System.setProperty(CLI_ARGS, "--db=mariadb"); diff --git a/testsuite/integration-arquillian/pom.xml b/testsuite/integration-arquillian/pom.xml index 475d84da41..723fc69c27 100644 --- a/testsuite/integration-arquillian/pom.xml +++ b/testsuite/integration-arquillian/pom.xml @@ -436,7 +436,7 @@ keycloak sa - jdbc:h2:mem:test;MVCC=TRUE;DB_CLOSE_DELAY=-1 + jdbc:h2:mem:test;DB_CLOSE_DELAY=-1 diff --git a/testsuite/model/pom.xml b/testsuite/model/pom.xml index 10436be15a..a4a0fa4472 100644 --- a/testsuite/model/pom.xml +++ b/testsuite/model/pom.xml @@ -19,7 +19,7 @@ keycloak sa - jdbc:h2:mem:test;MVCC=TRUE;DB_CLOSE_DELAY=-1 + jdbc:h2:mem:test;DB_CLOSE_DELAY=-1 com.h2database h2 ${h2.version}