From ca39cd97b703a6320dcd6fb105bda5a6dfcbcb63 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 13 Nov 2024 11:00:49 +0100 Subject: [PATCH 1/8] Implement different db users to seperate the tenants from each other --- ...FlywayMultitenantMigrationInitializer.java | 11 +- .../okr/multitenancy/HibernateContext.java | 78 ++++- .../SchemaMultiTenantConnectionProvider.java | 64 ++-- .../multitenancy/TenantConfigProvider.java | 62 +++- .../okr/multitenancy/TenantConfigs.java | 21 ++ .../main/resources/application-dev.properties | 16 +- .../application-integration-test.properties | 12 +- ...ayMultitenantMigrationInitializerTest.java | 82 +++-- .../multitenancy/HibernateContextTest.java | 80 ++++- ...TenantConnectionProviderInternalsTest.java | 279 +++++++++++++++--- .../TenantConfigProviderTestIT.java | 46 ++- .../okr/multitenancy/TenantConfigsTest.java | 70 +++++ .../ch/puzzle/okr/security/JwtHelperTest.java | 4 +- .../TenantJwtIssuerValidatorTest.java | 2 +- .../authorization/UserUpdateHelperTest.java | 4 +- .../clientconfig/ClientConfigServiceTest.java | 2 +- docker/dataset/init.sql | 65 +++- frontend/src/assets/i18n/de.json | 32 ++ 18 files changed, 780 insertions(+), 150 deletions(-) create mode 100644 backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigs.java create mode 100644 backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigsTest.java diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java index d58c195c77..9e7fe04603 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java @@ -2,6 +2,8 @@ import jakarta.persistence.EntityNotFoundException; import org.flywaydb.core.Flyway; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; @@ -10,6 +12,8 @@ public class FlywayMultitenantMigrationInitializer { private final TenantConfigProviderInterface tenantConfigProvider; private final String[] scriptLocations; + private static final Logger logger = LoggerFactory.getLogger(FlywayMultitenantMigrationInitializer.class); + public FlywayMultitenantMigrationInitializer(TenantConfigProviderInterface tenantConfigProvider, final @Value("${spring.flyway.locations}") String[] scriptLocations) { this.tenantConfigProvider = tenantConfigProvider; @@ -20,9 +24,11 @@ public void migrateFlyway() { this.tenantConfigProvider.getTenantConfigs().forEach((tenantConfig) -> { TenantConfigProvider.DataSourceConfig dataSourceConfig = this.tenantConfigProvider .getTenantConfigById(tenantConfig.tenantId()) - .map(TenantConfigProvider.TenantConfig::dataSourceConfig).orElseThrow( + .map(TenantConfigProvider.TenantConfig::dataSourceConfigFlyway).orElseThrow( () -> new EntityNotFoundException("Cannot find tenant for configuring flyway migration")); + logUsedHibernateConfig(dataSourceConfig); + Flyway tenantSchemaFlyway = Flyway.configure() // .dataSource(dataSourceConfig.url(), dataSourceConfig.name(), dataSourceConfig.password()) // .locations(scriptLocations) // @@ -32,6 +38,9 @@ public void migrateFlyway() { tenantSchemaFlyway.migrate(); }); + } + private void logUsedHibernateConfig(TenantConfigProvider.DataSourceConfig dataSourceConfig) { + logger.error("use DbConfig: user={}", dataSourceConfig.name()); } } \ No newline at end of file diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java index 42e068ece0..86b8f0e8d9 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java @@ -1,9 +1,25 @@ package ch.puzzle.okr.multitenancy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.core.env.ConfigurableEnvironment; import java.util.Properties; +/** + * Reads the (not tenant specific) hibernate configuration form the "hibernate.x" properties in the + * applicationX.properties file. It then caches the configuration as DbConfig object. The data from the DbConfig object + * is used by the SchemaMultiTenantConnectionProvider via getHibernateConfig() and getHibernateConfig(tenantId). + * + * 
+ * getHibernateConfig() returns the cached DbConfig as properties.
+ *
+ * + * 
+ * getHibernateConfig(tenantId) patches the DbConfig data with tenant specific data (from
+ * TenantConfigProvider) and returns the patched data as properties
+ *
+ */ public class HibernateContext { public static final String HIBERNATE_CONNECTION_URL = "hibernate.connection.url"; public static final String HIBERNATE_CONNECTION_USERNAME = "hibernate.connection.username"; @@ -14,6 +30,8 @@ public class HibernateContext { public static String SPRING_DATASOURCE_USERNAME = "spring.datasource.username"; public static String SPRING_DATASOURCE_PASSWORD = "spring.datasource.password"; + private static final Logger logger = LoggerFactory.getLogger(HibernateContext.class); + public record DbConfig(String url, String username, String password, String multiTenancy) { public boolean isValid() { @@ -29,8 +47,15 @@ private boolean hasEmptyValues() { } } + // general (not tenant specific) hibernate config private static DbConfig cachedHibernateConfig; + public static void extractAndSetHibernateConfig(ConfigurableEnvironment environment) { + DbConfig dbConfig = extractHibernateConfig(environment); + setHibernateConfig(dbConfig); + logUsedHibernateConfig(dbConfig); + } + public static void setHibernateConfig(DbConfig dbConfig) { if (dbConfig == null || !dbConfig.isValid()) { throw new RuntimeException("Invalid hibernate configuration " + dbConfig); @@ -38,11 +63,6 @@ public static void setHibernateConfig(DbConfig dbConfig) { cachedHibernateConfig = dbConfig; } - public static void extractAndSetHibernateConfig(ConfigurableEnvironment environment) { - DbConfig dbConfig = extractHibernateConfig(environment); - HibernateContext.setHibernateConfig(dbConfig); - } - private static DbConfig extractHibernateConfig(ConfigurableEnvironment environment) { String url = environment.getProperty(HibernateContext.HIBERNATE_CONNECTION_URL); String username = environment.getProperty(HibernateContext.HIBERNATE_CONNECTION_USERNAME); @@ -60,7 +80,9 @@ public static Properties getHibernateConfig() { if (cachedHibernateConfig == null) { throw new RuntimeException("No cached hibernate configuration found"); } - return getConfigAsProperties(cachedHibernateConfig); + var config = getConfigAsProperties(cachedHibernateConfig); + logUsedHibernateConfig(config); + return config; } private static Properties getConfigAsProperties(DbConfig dbConfig) { @@ -74,4 +96,48 @@ private static Properties getConfigAsProperties(DbConfig dbConfig) { properties.put(HibernateContext.SPRING_DATASOURCE_PASSWORD, dbConfig.password()); return properties; } + + public static Properties getHibernateConfig(String tenantIdentifier) { + if (cachedHibernateConfig == null) { + throw new RuntimeException("No cached hibernate configuration found (for tenant " + tenantIdentifier + ")"); + } + var config = getConfigAsPropertiesAndPatch(cachedHibernateConfig, tenantIdentifier); + logUsedHibernateConfig(tenantIdentifier, config); + return config; + } + + private static Properties getConfigAsPropertiesAndPatch(DbConfig dbConfig, String tenantIdentifier) { + Properties properties = getConfigAsProperties(dbConfig); + return patchConfigAppForTenant(properties, tenantIdentifier); + } + + private static Properties patchConfigAppForTenant(Properties properties, String tenantIdentifier) { + TenantConfigProvider.TenantConfig cachedTenantConfig = TenantConfigProvider + .getCachedTenantConfig(tenantIdentifier); + if (cachedTenantConfig == null) { + throw new RuntimeException("No cached tenant configuration found (for tenant " + tenantIdentifier + ")"); + } + + TenantConfigProvider.DataSourceConfig dataSourceConfigApp = cachedTenantConfig.dataSourceConfigApp(); + properties.put(HibernateContext.HIBERNATE_CONNECTION_USERNAME, dataSourceConfigApp.name()); + properties.put(HibernateContext.HIBERNATE_CONNECTION_PASSWORD, dataSourceConfigApp.password()); + properties.put(HibernateContext.SPRING_DATASOURCE_USERNAME, dataSourceConfigApp.name()); + properties.put(HibernateContext.SPRING_DATASOURCE_PASSWORD, dataSourceConfigApp.password()); + return properties; + } + + private static void logUsedHibernateConfig(DbConfig hibernateConfig) { + logger.error("set DbConfig: user={}", hibernateConfig.username()); + } + + private static void logUsedHibernateConfig(Properties hibernateConfig) { + logger.error("use DbConfig: user={}", + hibernateConfig.getProperty(HibernateContext.HIBERNATE_CONNECTION_USERNAME)); // + } + + private static void logUsedHibernateConfig(String tenantId, Properties hibernateConfig) { + logger.error("use DbConfig: tenant={} user={}", tenantId, + hibernateConfig.getProperty(HibernateContext.HIBERNATE_CONNECTION_USERNAME)); + } + } diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java index 05ff90ce53..5c11016066 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java @@ -15,6 +15,35 @@ import static ch.puzzle.okr.multitenancy.TenantContext.DEFAULT_TENANT_ID; +/** + * The central piece of code of multitenancy. + * + * 
+ * getConnection(tenantId) sets in each tenant request the specific db schema for the
+ * tenant. This guarantees that each tenant always works in its own DB schema.
+ *
+ * getConnection(tenantId) -> Connection calls in the abstract super class the
+ * getConnection(tenantId) -> Connection which calls the abstract
+ * selectConnectionProvider(tenantIdentifier) -> ConnectionProvider which is implemented
+ * in SchemaMultiTenantConnectionProvider.
+ *
+ * + * 
+ * Some coding details:
+ *
+ * selectConnectionProvider(tenantId) -> ConnectionProvider returns for a tenant a
+ * ConnectionProvider. It first checks if the ConnectionProvider for the tenant is already
+ * cached (in connectionProviderMap). If the ConnectionProvider is cached, it returns it.
+ * Otherwise it creates a ConnectionProvider for the tenant, cache it and return it.
+ *
+ * To create a ConnectionProvider for the tenant, it tries to load the configuration from
+ * the hibernate properties. For this it uses 2 methods of HibernateContext:
+ * getHibernateConfig() if the tenant is the DEFAULT_TENANT_ID (public) and
+ * getHibernateConfig(tenantId) for all other tenants. With this information its then
+ * possible to create and cache a ConnectionProvider for the tenant. If no matching
+ * hibernate properties are found, then an exception is thrown.
+ *
+ */ public class SchemaMultiTenantConnectionProvider extends AbstractMultiTenantConnectionProvider { private static final Logger logger = LoggerFactory.getLogger(SchemaMultiTenantConnectionProvider.class); @@ -31,7 +60,7 @@ public Connection getConnection(String tenantIdentifier) throws SQLException { return getConnection(tenantIdentifier, connection); } - protected Connection getConnection(String tenantIdentifier, Connection connection) throws SQLException { + Connection getConnection(String tenantIdentifier, Connection connection) throws SQLException { String schema = convertTenantIdToSchemaName(tenantIdentifier); logger.debug("Setting schema to {}", schema); @@ -39,7 +68,7 @@ protected Connection getConnection(String tenantIdentifier, Connection connectio return connection; } - private String convertTenantIdToSchemaName(String tenantIdentifier) { + String convertTenantIdToSchemaName(String tenantIdentifier) { return Objects.equals(tenantIdentifier, DEFAULT_TENANT_ID) ? tenantIdentifier : MessageFormat.format("okr_{0}", tenantIdentifier); } @@ -54,13 +83,13 @@ protected ConnectionProvider selectConnectionProvider(String tenantIdentifier) { return getConnectionProvider(tenantIdentifier); } - protected ConnectionProvider getConnectionProvider(String tenantIdentifier) { + ConnectionProvider getConnectionProvider(String tenantIdentifier) { return Optional.ofNullable(tenantIdentifier) // .map(connectionProviderMap::get) // - .orElseGet(() -> createNewConnectionProvider(tenantIdentifier)); + .orElseGet(() -> createAndCacheNewConnectionProvider(tenantIdentifier)); } - private ConnectionProvider createNewConnectionProvider(String tenantIdentifier) { + private ConnectionProvider createAndCacheNewConnectionProvider(String tenantIdentifier) { return Optional.ofNullable(tenantIdentifier) // .map(this::createConnectionProvider) // .map(connectionProvider -> { @@ -78,10 +107,10 @@ private ConnectionProvider createConnectionProvider(String tenantIdentifier) { .orElse(null); } - protected Properties getHibernatePropertiesForTenantIdentifier(String tenantIdentifier) { - Properties properties = getHibernateProperties(); - if (properties == null || properties.isEmpty()) { - throw new RuntimeException("Cannot load hibernate properties from application.properties)"); + Properties getHibernatePropertiesForTenantIdentifier(String tenantIdentifier) { + Properties properties = getHibernateProperties(tenantIdentifier); + if (properties.isEmpty()) { + throw new RuntimeException("Cannot load hibernate properties from application.properties"); } if (!Objects.equals(tenantIdentifier, DEFAULT_TENANT_ID)) { properties.put(AvailableSettings.DEFAULT_SCHEMA, MessageFormat.format("okr_{0}", tenantIdentifier)); @@ -89,18 +118,14 @@ protected Properties getHibernatePropertiesForTenantIdentifier(String tenantIden return properties; } - private ConnectionProvider initConnectionProvider(Properties hibernateProperties) { + ConnectionProvider initConnectionProvider(Properties hibernateProperties) { Map configProperties = convertPropertiesToMap(hibernateProperties); - DriverManagerConnectionProviderImpl connectionProvider = getDriverManagerConnectionProviderImpl(); + DriverManagerConnectionProviderImpl connectionProvider = new DriverManagerConnectionProviderImpl(); connectionProvider.configure(configProperties); return connectionProvider; } - protected DriverManagerConnectionProviderImpl getDriverManagerConnectionProviderImpl() { - return new DriverManagerConnectionProviderImpl(); - } - - private Map convertPropertiesToMap(Properties properties) { + Map convertPropertiesToMap(Properties properties) { Map configProperties = new HashMap<>(); for (String key : properties.stringPropertyNames()) { String value = properties.getProperty(key); @@ -109,7 +134,10 @@ private Map convertPropertiesToMap(Properties properties) { return configProperties; } - protected Properties getHibernateProperties() { - return HibernateContext.getHibernateConfig(); + private Properties getHibernateProperties(String tenantIdentifier) { + if (tenantIdentifier.equals(DEFAULT_TENANT_ID)) { + return HibernateContext.getHibernateConfig(); + } + return HibernateContext.getHibernateConfig(tenantIdentifier); } } \ No newline at end of file diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java index 97521d9d7a..d1dbe99d6c 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java @@ -1,5 +1,7 @@ package ch.puzzle.okr.multitenancy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.env.Environment; import org.springframework.stereotype.Component; @@ -7,21 +9,54 @@ import java.text.MessageFormat; import java.util.*; +/** + * Reads the configuration of the tenants (as TenantConfig objects) from the applicationX.properties and caches each + * TenantConfig in the TenantConfigs class. + */ @Component public class TenantConfigProvider implements TenantConfigProviderInterface { private static final String EMAIL_DELIMITER = ","; private final Map tenantConfigs = new HashMap<>(); private final Environment env; + private enum DbType { + bootstrap, app, fly + } + + private static final Logger logger = LoggerFactory.getLogger(TenantConfigProvider.class); + public TenantConfigProvider(final @Value("${okr.tenant-ids}") String[] tenantIds, Environment env) { this.env = env; for (String tenantId : tenantIds) { OauthConfig c = readOauthConfig(tenantId); - tenantConfigs.put(tenantId, - createTenantConfig(c.jwkSetUri(), c.frontendClientIssuerUrl(), c.frontendClientId(), tenantId)); + TenantConfig tenantConfig = createTenantConfig(c.jwkSetUri(), c.frontendClientIssuerUrl(), + c.frontendClientId(), tenantId); + + tenantConfigs.put(tenantId, tenantConfig); + cacheTenantConfig(tenantId, tenantConfig); // cache tenantConfig for Hibernate connections } } + private void cacheTenantConfig(String tenantId, TenantConfig tenantConfig) { + TenantConfigs.add(tenantId, tenantConfig); + logCachingTenantConfig(tenantId, tenantConfig); + } + + private void logCachingTenantConfig(String tenantId, TenantConfig tenantConfig) { + logger.error("cache TenantConfig: tenantId={}, users={}", // + tenantId, // + tenantConfig.dataSourceConfigFlyway().name() + " | " + tenantConfig.dataSourceConfigApp().name()); + } + + public static TenantConfigProvider.TenantConfig getCachedTenantConfig(String tenantId) { + return TenantConfigs.get(tenantId); + } + + // for tests + public static void clearTenantConfigsCache() { + TenantConfigs.clear(); + } + private OauthConfig readOauthConfig(String tenantId) { return new OauthConfig( env.getProperty(MessageFormat.format("okr.tenants.{0}.security.oauth2.resourceserver.jwt.jwk-set-uri", @@ -32,8 +67,11 @@ private OauthConfig readOauthConfig(String tenantId) { private TenantConfig createTenantConfig(String jwkSetUriTemplate, String frontendClientIssuerUrl, String frontendClientId, String tenantId) { - return new TenantConfig(tenantId, getOkrChampionEmailsFromTenant(tenantId), jwkSetUriTemplate, - frontendClientIssuerUrl, frontendClientId, this.readDataSourceConfig(tenantId)); + + return new TenantConfig(tenantId, getOkrChampionEmailsFromTenant(tenantId), jwkSetUriTemplate, // + frontendClientIssuerUrl, frontendClientId, // + this.readDataSourceConfigFlyway(tenantId), // + this.readDataSourceConfigApp(tenantId)); } private String[] getOkrChampionEmailsFromTenant(String tenantId) { @@ -45,11 +83,19 @@ public List getTenantConfigs() { return this.tenantConfigs.values().stream().toList(); } - private DataSourceConfig readDataSourceConfig(String tenantId) { + private DataSourceConfig readDataSourceConfigFlyway(String tenantId) { + return readDataSourceConfig(tenantId, DbType.fly); + } + + private DataSourceConfig readDataSourceConfigApp(String tenantId) { + return readDataSourceConfig(tenantId, DbType.app); + } + + private DataSourceConfig readDataSourceConfig(String tenantId, DbType dbType) { return new DataSourceConfig(env.getProperty("okr.datasource.driver-class-name"), env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.url", tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.username", tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.password", tenantId)), + env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.username." + dbType, tenantId)), + env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.password." + dbType, tenantId)), env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.schema", tenantId))); } @@ -62,7 +108,7 @@ public Optional getJwkSetUri(String tenantId) { } public record TenantConfig(String tenantId, String[] okrChampionEmails, String jwkSetUri, String issuerUrl, - String clientId, DataSourceConfig dataSourceConfig) { + String clientId, DataSourceConfig dataSourceConfigFlyway, DataSourceConfig dataSourceConfigApp) { } public record DataSourceConfig(String driverClassName, String url, String name, String password, String schema) { diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigs.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigs.java new file mode 100644 index 0000000000..0a6c22f699 --- /dev/null +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigs.java @@ -0,0 +1,21 @@ +package ch.puzzle.okr.multitenancy; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +public class TenantConfigs { + + private static final Map tenantConfigs = new ConcurrentHashMap<>(); + + public static void add(String tenantId, TenantConfigProvider.TenantConfig tenantConfigProvider) { + tenantConfigs.putIfAbsent(tenantId, tenantConfigProvider); + } + + public static TenantConfigProvider.TenantConfig get(String tenantId) { + return tenantConfigs.get(tenantId); + } + + public static void clear() { + tenantConfigs.clear(); + } +} diff --git a/backend/src/main/resources/application-dev.properties b/backend/src/main/resources/application-dev.properties index b995d8a397..5e4f58bcc8 100644 --- a/backend/src/main/resources/application-dev.properties +++ b/backend/src/main/resources/application-dev.properties @@ -16,16 +16,18 @@ okr.datasource.driver-class-name=org.postgresql.Driver # security connect.src=http://localhost:8544 http://localhost:8545 -# hibernate +# hibernate (for full db access use: user/pwd) hibernate.connection.url=jdbc:postgresql://localhost:5432/okr -hibernate.connection.username=user +hibernate.connection.username=bootstrap_app hibernate.connection.password=pwd hibernate.multiTenancy=SCHEMA # pitc okr.tenants.pitc.datasource.url=jdbc:postgresql://localhost:5432/okr -okr.tenants.pitc.datasource.username=pitc -okr.tenants.pitc.datasource.password=pwd +okr.tenants.pitc.datasource.username.fly=pitc_fly +okr.tenants.pitc.datasource.password.fly=pwd +okr.tenants.pitc.datasource.username.app=pitc_app +okr.tenants.pitc.datasource.password.app=pwd okr.tenants.pitc.datasource.schema=okr_pitc okr.tenants.pitc.user.champion.emails=gl@gl.com okr.tenants.pitc.security.oauth2.resourceserver.jwt.jwk-set-uri=http://localhost:8544/realms/pitc/protocol/openid-connect/certs @@ -34,8 +36,10 @@ okr.tenants.pitc.security.oauth2.frontend.client-id=pitc_okr_staging # acme okr.tenants.acme.datasource.url=jdbc:postgresql://localhost:5432/okr -okr.tenants.acme.datasource.username=acme -okr.tenants.acme.datasource.password=pwd +okr.tenants.acme.datasource.username.fly=acme_fly +okr.tenants.acme.datasource.password.fly=pwd +okr.tenants.acme.datasource.username.app=acme_app +okr.tenants.acme.datasource.password.app=pwd okr.tenants.acme.datasource.schema=okr_acme okr.tenants.acme.user.champion.emails=gl@acme.com okr.tenants.acme.security.oauth2.resourceserver.jwt.jwk-set-uri=http://localhost:8545/realms/acme/protocol/openid-connect/certs diff --git a/backend/src/main/resources/application-integration-test.properties b/backend/src/main/resources/application-integration-test.properties index 44769ffd8a..64843cd91c 100644 --- a/backend/src/main/resources/application-integration-test.properties +++ b/backend/src/main/resources/application-integration-test.properties @@ -27,8 +27,10 @@ hibernate.multiTenancy=SCHEMA # pitc okr.tenants.pitc.datasource.url=jdbc:h2:mem:db;DB_CLOSE_DELAY=-1;INIT=CREATE SCHEMA IF NOT EXISTS okr_pitc -okr.tenants.pitc.datasource.username=user -okr.tenants.pitc.datasource.password=sa +okr.tenants.pitc.datasource.username.fly=user +okr.tenants.pitc.datasource.password.fly=sa +okr.tenants.pitc.datasource.username.app=user +okr.tenants.pitc.datasource.password.app=sa okr.tenants.pitc.datasource.schema=okr_pitc okr.tenants.pitc.user.champion.emails=peggimann@puzzle.ch,wunderland@puzzle.ch okr.tenants.pitc.security.oauth2.resourceserver.jwt.jwk-set-uri=http://localhost:8544/realms/pitc/protocol/openid-connect/certs @@ -37,8 +39,10 @@ okr.tenants.pitc.security.oauth2.frontend.client-id=pitc_okr_staging # acme okr.tenants.acme.datasource.url=jdbc:h2:mem:db;DB_CLOSE_DELAY=-1;INIT=CREATE SCHEMA IF NOT EXISTS okr_acme -okr.tenants.acme.datasource.username=user -okr.tenants.acme.datasource.password=sa +okr.tenants.acme.datasource.username.fly=user +okr.tenants.acme.datasource.password.fly=sa +okr.tenants.acme.datasource.username.app=user +okr.tenants.acme.datasource.password.app=sa okr.tenants.acme.datasource.schema=okr_acme okr.tenants.acme.user.champion.emails=peggimann@puzzle.ch,wunderland@puzzle.ch okr.tenants.acme.security.oauth2.resourceserver.jwt.jwk-set-uri=http://localhost:8544/realms/pitc/protocol/openid-connect/certs diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializerTest.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializerTest.java index 3ab1cb4eb4..53b7ffa724 100644 --- a/backend/src/test/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializerTest.java +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializerTest.java @@ -1,7 +1,9 @@ package ch.puzzle.okr.multitenancy; +import jakarta.persistence.EntityNotFoundException; import org.flywaydb.core.Flyway; import org.flywaydb.core.api.configuration.FluentConfiguration; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; @@ -66,40 +68,45 @@ public String getLog() { } } - private final TenantConfigProviderInterface providerInterfaceMock = new TenantConfigProviderInterface() { - - private final TenantConfigProvider.DataSourceConfig dataSourceConfig = new TenantConfigProvider.DataSourceConfig( - NOT_USED, URL, NAME, PASSWORD, SCHEMA); - - private final TenantConfigProvider.TenantConfig tenantConfig = new TenantConfigProvider.TenantConfig(NOT_USED, - new String[] { NOT_USED }, NOT_USED, NOT_USED, NOT_USED, dataSourceConfig); - - @Override - public List getTenantConfigs() { - return List.of(tenantConfig); - } - - @Override - public Optional getTenantConfigById(String tenantId) { - return Optional.of(tenantConfig); - } - - @Override - public Optional getJwkSetUri(String tenantId) { - return Optional.empty(); - } - }; + private final TenantConfigProvider.TenantConfig tenantConfig = new TenantConfigProvider.TenantConfig(NOT_USED, + new String[] { NOT_USED }, NOT_USED, NOT_USED, NOT_USED, + new TenantConfigProvider.DataSourceConfig(NOT_USED, URL, NAME, PASSWORD, SCHEMA), // flyway user config + null // app user config; not used in this test + ); + + private TenantConfigProviderInterface mockTenantConfigProviderInterface( + List tenantConfigs, TenantConfigProvider.TenantConfig tenantConfigById) { + + return new TenantConfigProviderInterface() { + @Override + public List getTenantConfigs() { + return tenantConfigs; + } + + @Override + public Optional getTenantConfigById(String tenantId) { + return Optional.ofNullable(tenantConfigById); + } + + @Override + public Optional getJwkSetUri(String tenantId) { + return Optional.empty(); + } + }; + } - @DisplayName("Flyway.configure() should return FluentConfiguration which we can assert") + @DisplayName("flywayConfigure() should return FluentConfiguration which we can assert") @Test void flywayConfigureShouldReturnFluentConfigurationWhichWeCanAssert() { try (MockedStatic mockedStatic = Mockito.mockStatic(Flyway.class)) { // arrange - FluentConfigurationSpy fluentConfiguration = new FluentConfigurationSpy(); + var fluentConfiguration = new FluentConfigurationSpy(); mockedStatic.when(Flyway::configure).thenReturn(fluentConfiguration); - FlywayMultitenantMigrationInitializer migrationInitializer = new FlywayMultitenantMigrationInitializer( - providerInterfaceMock, new String[] { SCRIPT_LOCATION }); + // returns for getTenantConfigs() and getTenantConfigById() tenantConfig + var migrationInitializer = new FlywayMultitenantMigrationInitializer( + mockTenantConfigProviderInterface(List.of(tenantConfig), tenantConfig), + new String[] { SCRIPT_LOCATION }); // act migrationInitializer.migrateFlyway(); @@ -111,4 +118,25 @@ void flywayConfigureShouldReturnFluentConfigurationWhichWeCanAssert() { } } + @DisplayName("flywayConfigure() should throw exception when TenantConfigById is not found") + @Test + void flywayConfigureShouldThrowExceptionWhenTenantConfigByIdIsNotFound() { + try (MockedStatic mockedStatic = Mockito.mockStatic(Flyway.class)) { + // arrange + var fluentConfiguration = new FluentConfigurationSpy(); + mockedStatic.when(Flyway::configure).thenReturn(fluentConfiguration); + + // returns for getTenantConfigs() tenantConfig and for getTenantConfigById() empty Optional + var migrationInitializer = new FlywayMultitenantMigrationInitializer( + mockTenantConfigProviderInterface(List.of(tenantConfig), null), // + new String[] { SCRIPT_LOCATION }); + + // act + assert + var entityNotFoundException = Assertions.assertThrows(EntityNotFoundException.class, // + migrationInitializer::migrateFlyway); + + assertEquals("Cannot find tenant for configuring flyway migration", entityNotFoundException.getMessage()); + } + } + } \ No newline at end of file diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java index 7dbc3a5b16..9e64499915 100644 --- a/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java @@ -1,5 +1,6 @@ package ch.puzzle.okr.multitenancy; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -18,11 +19,18 @@ public class HibernateContextTest { + private static final String NOT_USED = "not_used"; + @BeforeEach void setUp() { resetHibernateConfig(); } + @AfterEach + void tearDown() { + resetHibernateConfig(); + } + @DisplayName("setHibernateConfig() should throw exception if db config is null") @Test void setHibernateConfigShouldThrowExceptionIfDbConfigIsNull() { @@ -30,7 +38,7 @@ void setHibernateConfigShouldThrowExceptionIfDbConfigIsNull() { DbConfig dbConfig = null; // act + assert - RuntimeException exception = assertThrows(RuntimeException.class, () -> setHibernateConfig(dbConfig)); + var exception = assertThrows(RuntimeException.class, () -> setHibernateConfig(dbConfig)); assertEquals("Invalid hibernate configuration null", exception.getMessage()); } @@ -41,10 +49,10 @@ void setHibernateConfigShouldThrowExceptionIfDbConfigHasNullOrEmptyValues(String String password, String tenant) { // arrange - DbConfig dbConfig = new DbConfig(url, username, password, tenant); + var dbConfig = new DbConfig(url, username, password, tenant); // act + assert - RuntimeException exception = assertThrows(RuntimeException.class, () -> setHibernateConfig(dbConfig)); + var exception = assertThrows(RuntimeException.class, () -> setHibernateConfig(dbConfig)); assertTrue(exception.getMessage().startsWith("Invalid hibernate configuration")); } @@ -60,13 +68,13 @@ private static Stream invalidDbConfig() { Arguments.of("url", "username", "password", "")); } - @DisplayName("extractAndSetHibernateConfig() should extract hibernate properties from environment and set it") + @DisplayName("extractAndSetHibernateConfig() should extract hibernate properties from environment and cache it") @Test - void extractAndSetHibernateConfigShouldExtractHibernatePropertiesFromEnvironmentAndSetIt() { + void extractAndSetHibernateConfigShouldExtractHibernatePropertiesFromEnvironmentAndCacheIt() { // arrange String url = "url", username = "username", password = "password", multiTenancy = "multiTenancy"; - ConfigurableEnvironment environment = mock(ConfigurableEnvironment.class); + var environment = mock(ConfigurableEnvironment.class); when(environment.getProperty(HIBERNATE_CONNECTION_URL)).thenReturn(url); when(environment.getProperty(HIBERNATE_CONNECTION_USERNAME)).thenReturn(username); when(environment.getProperty(HIBERNATE_CONNECTION_PASSWORD)).thenReturn(password); @@ -74,7 +82,7 @@ void extractAndSetHibernateConfigShouldExtractHibernatePropertiesFromEnvironment // act extractAndSetHibernateConfig(environment); - Properties hibernateProperties = getHibernateConfig(); + var hibernateProperties = getHibernateConfig(); // assert assertNotNull(hibernateProperties); @@ -84,10 +92,10 @@ void extractAndSetHibernateConfigShouldExtractHibernatePropertiesFromEnvironment @DisplayName("getHibernateConfig() should throw exception if setHibernateConfig() is not called before with valid configuration") @Test void getHibernateConfigShouldThrowExceptionIfSetHibernateConfigIsNotCalledBeforeWithValidConfiguration() { - // arrange + // arrange: no DbConfig is set // act + assert - RuntimeException exception = assertThrows(RuntimeException.class, HibernateContext::getHibernateConfig); + var exception = assertThrows(RuntimeException.class, HibernateContext::getHibernateConfig); assertEquals("No cached hibernate configuration found", exception.getMessage()); } @@ -96,17 +104,67 @@ void getHibernateConfigShouldThrowExceptionIfSetHibernateConfigIsNotCalledBefore void getHibernateConfigShouldReturnHibernateConfigAsPropertiesIfDbConfigIsValid() { // arrange String url = "url", username = "username", password = "password", multiTenancy = "multiTenancy"; - DbConfig dbConfig = new DbConfig(url, username, password, multiTenancy); + var dbConfig = new DbConfig(url, username, password, multiTenancy); setHibernateConfig(dbConfig); // act - Properties hibernateProperties = getHibernateConfig(); + var hibernateProperties = getHibernateConfig(); // assert assertNotNull(hibernateProperties); assertProperties(url, username, password, multiTenancy, hibernateProperties); } + @DisplayName("getHibernateConfigForTenantId() should throw exception if setHibernateConfig() is not called before with valid configuration") + @Test + void getHibernateConfigForTenantIdShouldThrowExceptionIfSetHibernateConfigIsNotCalledBeforeWithValidConfiguration() { + // arrange: no DbConfig is set + + // act + assert + var exception = assertThrows(RuntimeException.class, () -> getHibernateConfig("tenantId")); + assertEquals("No cached hibernate configuration found (for tenant tenantId)", exception.getMessage()); + } + + @DisplayName("setHibernateConfigForTenantId() should throw exception when no tenant config is cached") + @Test + void setHibernateConfigForTenantIdShouldThrowExceptionWhenNoTenantConfigIsCached() { + // arrange + String url = "url", username = "username", password = "password", multiTenancy = "multiTenancy"; + var dbConfig = new DbConfig(url, username, password, multiTenancy); + setHibernateConfig(dbConfig); + + var tenantId = "tenantId"; // but no tenant config is cached + + // act + assert + var exception = assertThrows(RuntimeException.class, () -> { + getHibernateConfig(tenantId); + }); + assertEquals("No cached tenant configuration found (for tenant tenantId)", exception.getMessage()); + } + + @DisplayName("getHibernateConfigForTenantId() should return hibernate config with patched tenant username and password as properties if db config is valid") + @Test + void getHibernateConfigForTenantIdShouldReturnHibernateConfigWithPatchedTenantUserNameAndPasswordAsPropertiesIfDbConfigIsValid() { + // arrange + String url = "url", username = "username", password = "password", multiTenancy = "multiTenancy"; + var dbConfig = new DbConfig(url, username, password, multiTenancy); + setHibernateConfig(dbConfig); + + String tenantId = "tenantId", tenantName = "tenantName", tenantPassword = "tenantPassword"; + var tenantConfig = new TenantConfigProvider.TenantConfig(tenantId, new String[] {}, NOT_USED, NOT_USED, + NOT_USED, null, // fly user config not used in test + new TenantConfigProvider.DataSourceConfig(NOT_USED, NOT_USED, tenantName, tenantPassword, NOT_USED)); + + TenantConfigs.add(tenantId, tenantConfig); // cache tenant db config + + // act + var hibernateProperties = getHibernateConfig(tenantId); + + // assert + assertNotNull(hibernateProperties); + assertProperties(url, tenantName, tenantPassword, multiTenancy, hibernateProperties); + } + private void assertProperties(String url, String username, String password, String multiTenancy, Properties properties) { diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderInternalsTest.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderInternalsTest.java index e8bc0dc7b0..5099c2eb17 100644 --- a/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderInternalsTest.java +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderInternalsTest.java @@ -2,87 +2,296 @@ import ch.puzzle.okr.exception.ConnectionProviderException; import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider; -import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; import java.util.Properties; -import static org.mockito.Mockito.mock; +import static ch.puzzle.okr.multitenancy.HibernateContext.*; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; public class SchemaMultiTenantConnectionProviderInternalsTest { - private static final String TENANT_ID = "pitc"; + private static final String TENANT_ID_ACME = "acme"; + private static final String DEFAULT_TENANT_ID = TenantContext.DEFAULT_TENANT_ID; + private static final String NOT_USED = ""; + + private static MockedStatic mockedStatic; + + /** + * A mock for the SchemaMultiTenantConnectionProvider, which is configurable. It can be configured via static + * factory methods: use cachedConnectionProvider(), for a cached ConnectionProvider.Or use + * inPropertiesDefinedConnectionProvider(), for no cached ConnectionProvider, but the data for creating a + * ConnectionProvider is defined in the (hibernate) properties. + */ + private static class ConnectionProviderMock extends SchemaMultiTenantConnectionProvider { + + public static ConnectionProviderMock cachedConnectionProvider( // + String tenantId, ConnectionProvider connectionProvider) { + return new ConnectionProviderMock(tenantId, connectionProvider); + } - private static class ConfigurableConnectionProviderMock extends SchemaMultiTenantConnectionProvider { + private ConnectionProviderMock(String tenantId, ConnectionProvider connectionProvider) { + connectionProviderMap.put(tenantId, connectionProvider); + } + + public static ConnectionProviderMock inPropertiesDefinedConnectionProvider( // + String tenantId, Properties hibernateProperties) { + return new ConnectionProviderMock(tenantId, hibernateProperties); + } + + private ConnectionProviderMock(String tenantId, Properties hibernateProperties) { + // mock dependencies to HibernateContext used in getHibernateProperties() + mockedStatic.when(() -> getHibernateConfig(tenantId)).thenReturn(hibernateProperties); + mockedStatic.when(() -> getHibernateConfig()).thenReturn(hibernateProperties); + } @Override - protected Properties getHibernateProperties() { - return new Properties(); + protected ConnectionProvider initConnectionProvider(Properties hibernateProperties) { + return mock(ConnectionProvider.class); } - public void registerProvider(String tenantIdentifier, ConnectionProvider connectionProvider) { - connectionProviderMap.put(tenantIdentifier, connectionProvider); + public boolean isCached(String tenantIdentifier) { + return connectionProviderMap.containsKey(tenantIdentifier); } } - @DisplayName("getConnectionProvider() return ConnectionProvider if TenantId is registered") + private Properties getConfigAsProperties(String tenantId) { + var properties = new Properties(); + properties.put(HIBERNATE_CONNECTION_URL, concat("url", tenantId)); + properties.put(HIBERNATE_CONNECTION_USERNAME, concat("username", tenantId)); + properties.put(HIBERNATE_CONNECTION_PASSWORD, concat("password", tenantId)); + properties.put(HIBERNATE_MULTITENANCY, concat("multitenancy", tenantId)); + properties.put(SPRING_DATASOURCE_URL, concat("url", tenantId)); + properties.put(SPRING_DATASOURCE_USERNAME, concat("username", tenantId)); + properties.put(SPRING_DATASOURCE_PASSWORD, concat("password", tenantId)); + return properties; + } + + private String concat(String data, String tenantId) { + return data + "_" + tenantId; + } + + @BeforeEach + public void setUp() { + mockedStatic = mockStatic(HibernateContext.class); + } + + @AfterEach + public void tearDown() { + mockedStatic.close(); + } + + @DisplayName("getConnectionProvider() should throw exception when tenantId is null") @Test - void getConnectionProviderReturnConnectionProviderIfTenantIdIsRegistered() { + void getConnectionProviderShouldThrowExceptionWhenTenantIdIsNull() { + // arrange + String tenantId = null; + var mockProvider = ConnectionProviderMock.inPropertiesDefinedConnectionProvider(NOT_USED, new Properties()); + + // act + assert + var exception = assertThrows(ConnectionProviderException.class, + () -> mockProvider.getConnectionProvider(tenantId)); + + // assert + assertEquals("Cannot create new connection provider for tenant: null", exception.getMessage()); + assertGetHibernateConfigNotCalled(); + } + + @DisplayName("getConnectionProvider() should return ConnectionProvider from cache") + @ParameterizedTest + @ValueSource(strings = { TENANT_ID, TENANT_ID_ACME }) + void getConnectionProviderShouldReturnConnectionProviderFromCache(String tenantId) { // arrange - ConfigurableConnectionProviderMock mockProvider = new ConfigurableConnectionProviderMock(); - mockProvider.registerProvider(TENANT_ID, mock(ConnectionProvider.class)); + var mockProvider = ConnectionProviderMock.cachedConnectionProvider(tenantId, mock(ConnectionProvider.class)); + + // pre-conditions + assertCached(mockProvider, tenantId); // act - ConnectionProvider foundConnectionProvider = mockProvider.getConnectionProvider(TENANT_ID); + var foundConnectionProvider = mockProvider.getConnectionProvider(tenantId); // assert - Assertions.assertNotNull(foundConnectionProvider); + assertNotNull(foundConnectionProvider); + assertGetHibernateConfigNotCalled(tenantId); + + // post-condition + assertCached(mockProvider, tenantId); } - @DisplayName("getConnectionProvider() throws Exception when lookup TenantId is null") - @Test - void getConnectionProviderThrowsExceptionWhenLookupTenantIdIsNull() { + @DisplayName("getConnectionProvider() should throw exception when ConnectionProvider is not cached and tenant is not configured in hibernate properties") + @ParameterizedTest + @ValueSource(strings = { TENANT_ID, TENANT_ID_ACME }) + void getConnectionProviderShouldThrowExceptionWhenConnectionProviderIsNotCachedAndTenantIsNotConfiguredInHibernateProperties( + String tenantId) { // arrange - ConfigurableConnectionProviderMock mockProvider = new ConfigurableConnectionProviderMock(); + var mockProvider = ConnectionProviderMock.inPropertiesDefinedConnectionProvider(tenantId, new Properties()); + + // pre-conditions + assertNotCached(mockProvider, tenantId); // act + assert - Assertions.assertThrows(ConnectionProviderException.class, () -> mockProvider.getConnectionProvider(null)); + var exception = assertThrows(RuntimeException.class, () -> mockProvider.getConnectionProvider(tenantId)); + + // assert + assertEquals("Cannot load hibernate properties from application.properties", exception.getMessage()); + assertGetHibernateConfigCalledWithTenantId(tenantId); + + // post-conditions + assertNotCached(mockProvider, tenantId); } - @DisplayName("selectConnectionProvider() return ConnectionProvider if TenantId is registered") + @DisplayName("getConnectionProvider() should cache ConnectionProvider for DEFAULT_TENANT_ID ('public') if ConnectionProvider is not cached but in hibernate properties") @Test - void selectConnectionProviderReturnConnectionProviderIfTenantIdIsRegistered() { + void getConnectionProviderShouldCacheConnectionProviderForDefaultTenantIdIfConnectionProviderIsNotCachedButInHibernateProperties() { // arrange - ConfigurableConnectionProviderMock mockProvider = new ConfigurableConnectionProviderMock(); - mockProvider.registerProvider(TENANT_ID, mock(ConnectionProvider.class)); + Properties properties = getConfigAsProperties(DEFAULT_TENANT_ID); + var mockProvider = ConnectionProviderMock.inPropertiesDefinedConnectionProvider(DEFAULT_TENANT_ID, properties); + + // pre-condition + assertNotCached(mockProvider, DEFAULT_TENANT_ID); // act - ConnectionProvider foundConnectionProvider = mockProvider.selectConnectionProvider(TENANT_ID); + var connectionProvider = mockProvider.getConnectionProvider(DEFAULT_TENANT_ID); // assert - Assertions.assertNotNull(foundConnectionProvider); + assertNotNull(connectionProvider); + assertGetHibernateConfigCalled(DEFAULT_TENANT_ID); + + // post-condition + assertCached(mockProvider, DEFAULT_TENANT_ID); } - @DisplayName("getAnyConnectionProvider() return ConnectionProvider for TenantId public") + @DisplayName("selectConnectionProvider() should cache ConnectionProvider for DEFAULT_TENANT_ID ('public') if ConnectionProvider is not cached but in hibernate properties") @Test - void getAnyConnectionProviderReturnConnectionProviderForTenantIdPublic() { + void selectConnectionProviderShouldCacheConnectionProviderForDefaultTenantIdIfConnectionProviderIsNotCachedButInHibernateProperties() { // arrange - ConfigurableConnectionProviderMock mockProvider = new ConfigurableConnectionProviderMock(); - mockProvider.registerProvider("public", mock(ConnectionProvider.class)); + Properties properties = getConfigAsProperties(DEFAULT_TENANT_ID); + var mockProvider = ConnectionProviderMock.inPropertiesDefinedConnectionProvider(DEFAULT_TENANT_ID, properties); + + // pre-condition + assertNotCached(mockProvider, DEFAULT_TENANT_ID); // act - ConnectionProvider foundConnectionProvider = mockProvider.getAnyConnectionProvider(); + var connectionProvider = mockProvider.selectConnectionProvider(DEFAULT_TENANT_ID); // assert - Assertions.assertNotNull(foundConnectionProvider); + assertNotNull(connectionProvider); + assertGetHibernateConfigCalled(DEFAULT_TENANT_ID); + + // post-condition + assertCached(mockProvider, DEFAULT_TENANT_ID); } - @DisplayName("getConnectionProviderShouldThrowRuntimeExceptionWhenNoPropertiesAreFound") + @DisplayName("getAnyConnectionProvider() should cache ConnectionProvider for DEFAULT_TENANT_ID ('public') if ConnectionProvider is not cached but in hibernate properties") @Test - void getConnectionProviderShouldThrowRuntimeExceptionWhenNoPropertiesAreFound() { - ConfigurableConnectionProviderMock mockProvider = new ConfigurableConnectionProviderMock(); + void getAnyConnectionProviderShouldCacheConnectionProviderForDefaultTenantIdIfConnectionProviderIsNotCachedButInHibernateProperties() { + // arrange + Properties properties = getConfigAsProperties(DEFAULT_TENANT_ID); + var mockProvider = ConnectionProviderMock.inPropertiesDefinedConnectionProvider(DEFAULT_TENANT_ID, properties); + + // pre-condition + assertNotCached(mockProvider, DEFAULT_TENANT_ID); + + // act + var connectionProvider = mockProvider.getAnyConnectionProvider(); - Assertions.assertThrows(RuntimeException.class, () -> mockProvider.getConnectionProvider(TENANT_ID)); + // assert + assertNotNull(connectionProvider); + assertGetHibernateConfigCalled(DEFAULT_TENANT_ID); + + // post-condition + assertCached(mockProvider, DEFAULT_TENANT_ID); + } + + @DisplayName("getConnectionProvider() should cache ConnectionProvider for tenantId if ConnectionProvider is not cached but in hibernate properties") + @ParameterizedTest + @ValueSource(strings = { TENANT_ID, TENANT_ID_ACME }) + void getConnectionProviderShouldCacheConnectionProviderForTenantIdIfConnectionProviderIsNotCachedButInHibernateProperties( + String tenantId) { + // arrange + Properties properties = getConfigAsProperties(tenantId); + var mockProvider = ConnectionProviderMock.inPropertiesDefinedConnectionProvider(tenantId, properties); + + // pre-condition + assertNotCached(mockProvider, tenantId); + + // act + var connectionProvider = mockProvider.getConnectionProvider(tenantId); + + // assert + assertNotNull(connectionProvider); + assertGetHibernateConfigCalledWithTenantId(tenantId); + + // post-condition + assertCached(mockProvider, tenantId); + } + + @DisplayName("convertPropertiesToMap() should convert properties to map") + @Test + void convertPropertiesToMapShouldConvertPropertiesToMap() { + // arrange + var connectionProvider = new SchemaMultiTenantConnectionProvider(); + var properties = getConfigAsProperties(TENANT_ID); + + // act + var map = connectionProvider.convertPropertiesToMap(properties); + + // assert + assertEquals(properties.size(), map.size()); + + assertEquals("url_pitc", map.get(HIBERNATE_CONNECTION_URL)); + assertEquals("username_pitc", map.get(HIBERNATE_CONNECTION_USERNAME)); + assertEquals("password_pitc", map.get(HIBERNATE_CONNECTION_PASSWORD)); + assertEquals("multitenancy_pitc", map.get(HIBERNATE_MULTITENANCY)); + assertEquals("url_pitc", map.get(SPRING_DATASOURCE_URL)); + assertEquals("username_pitc", map.get(SPRING_DATASOURCE_USERNAME)); + assertEquals("password_pitc", map.get(SPRING_DATASOURCE_PASSWORD)); } + + @DisplayName("connectionProvider() should return the schema name for tenantId") + @Test + void connectionProviderShouldReturnTheSchemaNameForTenantId() { + // arrange + var connectionProvider = new SchemaMultiTenantConnectionProvider(); + // act + var schema = connectionProvider.convertTenantIdToSchemaName(TENANT_ID); + // assert + assertEquals("okr_pitc", schema); + } + + private void assertCached(ConnectionProviderMock mockProvider, String tenantId) { + assertTrue(mockProvider.isCached(tenantId)); + } + + private void assertNotCached(ConnectionProviderMock mockProvider, String tenantId) { + assertFalse(mockProvider.isCached(tenantId)); + } + + private void assertGetHibernateConfigCalledWithTenantId(String tenantId) { + mockedStatic.verify(() -> getHibernateConfig(tenantId)); + mockedStatic.verify(() -> getHibernateConfig(), never()); + } + + private void assertGetHibernateConfigCalled(String tenantId) { + mockedStatic.verify(() -> getHibernateConfig()); + mockedStatic.verify(() -> getHibernateConfig(tenantId), never()); + } + + private void assertGetHibernateConfigNotCalled() { + mockedStatic.verify(() -> getHibernateConfig(), never()); + mockedStatic.verify(() -> getHibernateConfig(anyString()), never()); + } + + private void assertGetHibernateConfigNotCalled(String tenantId) { + mockedStatic.verify(() -> getHibernateConfig(), never()); + mockedStatic.verify(() -> getHibernateConfig(tenantId), never()); + } + } diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigProviderTestIT.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigProviderTestIT.java index 940473d020..4aaf694f7f 100644 --- a/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigProviderTestIT.java +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigProviderTestIT.java @@ -1,6 +1,7 @@ package ch.puzzle.okr.multitenancy; import ch.puzzle.okr.test.SpringIntegrationTest; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -24,8 +25,10 @@ public class TenantConfigProviderTestIT { private static final String FRONTEND_CLIENT_ISSUER_URL = "frontendClientIssuerUrl"; private static final String FRONTEND_CLIENT_ID = "frontendClientId"; private static final String DATASOURCE_URL = "datasourceUrl"; - private static final String DATASOURCE_NAME = "datasourceName"; - private static final String DATASOURCE_PASSWORD = "datasourcePassword"; + private static final String DATASOURCE_NAME_FLY = "datasourceNameFly"; + private static final String DATASOURCE_PASSWORD_FLY = "datasourcePasswordFly"; + private static final String DATASOURCE_NAME_APP = "datasourceNameApp"; + private static final String DATASOURCE_PASSWORD_APP = "datasourcePasswordApp"; private static final String DATASOURCE_SCHEMA = "datasourceSchema"; private static final String DRIVER_CLASS_NAME = "driverClassName"; private static final String CHAMPION_EMAILS_1 = "a@pitc.ch"; @@ -37,11 +40,19 @@ public class TenantConfigProviderTestIT { @Mock private Environment env; + private TenantConfigProvider configProvider; + @BeforeEach void setUp() { for (String tenantId : tenantIds) { setupPropertiesForTenantWithId(tenantId); } + configProvider = new TenantConfigProvider(tenantIds, env); + } + + @AfterEach + void tearDown() { + TenantConfigProvider.clearTenantConfigsCache(); } private void setupPropertiesForTenantWithId(String id) { @@ -49,8 +60,10 @@ private void setupPropertiesForTenantWithId(String id) { mockProperty("okr.tenants.{0}.security.oauth2.frontend.issuer-url", FRONTEND_CLIENT_ISSUER_URL, id); mockProperty("okr.tenants.{0}.security.oauth2.frontend.client-id", FRONTEND_CLIENT_ID, id); mockProperty("okr.tenants.{0}.datasource.url", DATASOURCE_URL, id); - mockProperty("okr.tenants.{0}.datasource.username", DATASOURCE_NAME, id); - mockProperty("okr.tenants.{0}.datasource.password", DATASOURCE_PASSWORD, id); + mockProperty("okr.tenants.{0}.datasource.username.fly", DATASOURCE_NAME_FLY, id); + mockProperty("okr.tenants.{0}.datasource.password.fly", DATASOURCE_PASSWORD_FLY, id); + mockProperty("okr.tenants.{0}.datasource.username.app", DATASOURCE_NAME_APP, id); + mockProperty("okr.tenants.{0}.datasource.password.app", DATASOURCE_PASSWORD_APP, id); mockProperty("okr.tenants.{0}.datasource.schema", DATASOURCE_SCHEMA, id); mockProperty("okr.datasource.driver-class-name", DRIVER_CLASS_NAME); @@ -78,10 +91,10 @@ private String prefix(String tenantId) { @DisplayName("getTenantConfigs returns all TenantConfigs as List") @Test public void testGetTenantConfigs() { - TenantConfigProvider configProvider = new TenantConfigProvider(tenantIds, env); List tenantConfigs = configProvider.getTenantConfigs(); for (TenantConfigProvider.TenantConfig config : tenantConfigs) { assertTenantConfigProvider(config); + assertTenantConfigIsCached(config.tenantId()); } } @@ -89,7 +102,6 @@ public void testGetTenantConfigs() { @ParameterizedTest @CsvSource({ "pitc, acme" }) void testGetTenantConfigByIdForExistingTenantId(String tenantId) { - TenantConfigProvider configProvider = new TenantConfigProvider(tenantIds, env); Optional config = configProvider.getTenantConfigById(tenantId); assertTrue(config.isPresent()); assertTenantConfigProvider(config.get()); @@ -99,7 +111,6 @@ void testGetTenantConfigByIdForExistingTenantId(String tenantId) { @ParameterizedTest @CsvSource({ "PITC-London" }) void testGetTenantConfigByIdForNonExistingTenantId(String nonExistingTenantId) { - TenantConfigProvider configProvider = new TenantConfigProvider(tenantIds, env); Optional config = configProvider.getTenantConfigById(nonExistingTenantId); assertTrue(config.isEmpty()); } @@ -108,9 +119,6 @@ void testGetTenantConfigByIdForNonExistingTenantId(String nonExistingTenantId) { @ParameterizedTest @CsvSource({ "pitc", "acme" }) void testGetJwkSetUriForExistingTenantId(String tenantId) { - // arrange - TenantConfigProvider configProvider = new TenantConfigProvider(tenantIds, env); - // act Optional jwkSetUri = configProvider.getJwkSetUri(tenantId); @@ -123,7 +131,6 @@ void testGetJwkSetUriForExistingTenantId(String tenantId) { @ParameterizedTest @CsvSource({ "PITC-London" }) void testGetJwkSetUriForNonExistingTenantId(String nonExistingTenantId) { - TenantConfigProvider configProvider = new TenantConfigProvider(tenantIds, env); Optional jwkSetUri = configProvider.getJwkSetUri(nonExistingTenantId); assertTrue(jwkSetUri.isEmpty()); } @@ -134,13 +141,20 @@ private void assertTenantConfigProvider(TenantConfigProvider.TenantConfig tenant assertEquals(prefix(tenantId) + JWK_SET_URI, tenantConfig.jwkSetUri()); assertEquals(prefix(tenantId) + FRONTEND_CLIENT_ISSUER_URL, tenantConfig.issuerUrl()); assertEquals(prefix(tenantId) + FRONTEND_CLIENT_ID, tenantConfig.clientId()); - assertEquals(prefix(tenantId) + DATASOURCE_URL, tenantConfig.dataSourceConfig().url()); - assertEquals(prefix(tenantId) + DATASOURCE_NAME, tenantConfig.dataSourceConfig().name()); - assertEquals(prefix(tenantId) + DATASOURCE_PASSWORD, tenantConfig.dataSourceConfig().password()); - assertEquals(prefix(tenantId) + DATASOURCE_SCHEMA, tenantConfig.dataSourceConfig().schema()); + assertEquals(prefix(tenantId) + DATASOURCE_URL, tenantConfig.dataSourceConfigFlyway().url()); + assertEquals(prefix(tenantId) + DATASOURCE_NAME_FLY, tenantConfig.dataSourceConfigFlyway().name()); + assertEquals(prefix(tenantId) + DATASOURCE_PASSWORD_FLY, tenantConfig.dataSourceConfigFlyway().password()); + assertEquals(prefix(tenantId) + DATASOURCE_NAME_APP, tenantConfig.dataSourceConfigApp().name()); + assertEquals(prefix(tenantId) + DATASOURCE_PASSWORD_APP, tenantConfig.dataSourceConfigApp().password()); + assertEquals(prefix(tenantId) + DATASOURCE_SCHEMA, tenantConfig.dataSourceConfigApp().schema()); assertArrayEquals(new String[] { CHAMPION_EMAILS_1, CHAMPION_EMAILS_2 }, tenantConfig.okrChampionEmails()); - assertEquals(DRIVER_CLASS_NAME, tenantConfig.dataSourceConfig().driverClassName()); + assertEquals(DRIVER_CLASS_NAME, tenantConfig.dataSourceConfigFlyway().driverClassName()); } + private void assertTenantConfigIsCached(String tenantId) { + var cachedConfig = TenantConfigProvider.getCachedTenantConfig(tenantId); + assertNotNull(cachedConfig); + assertTenantConfigProvider(cachedConfig); + } } diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigsTest.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigsTest.java new file mode 100644 index 0000000000..8eb3119c0e --- /dev/null +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/TenantConfigsTest.java @@ -0,0 +1,70 @@ +package ch.puzzle.okr.multitenancy; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class TenantConfigsTest { + + private static final String PITC = "pitc"; + private static final String ACME = "acme"; + + @BeforeEach + void setUp() { + TenantConfigs.clear(); + } + + @AfterEach + void tearDown() { + TenantConfigs.clear(); + } + + private TenantConfigProvider.TenantConfig createTenantConfig(String tenantId) { + return new TenantConfigProvider.TenantConfig( // + tenantId, null, "jwkSetUri", "issuerUrl", "clientId", // + new TenantConfigProvider.DataSourceConfig("driverClassName", "url", // + "flyway_" + tenantId, "password", "schema_" + tenantId), + new TenantConfigProvider.DataSourceConfig("driverClassName", "url", // + "app_" + tenantId, "password", "schema_" + tenantId)); + } + + @DisplayName("add() should add TenantConfig for tenantId in the map") + @ParameterizedTest + @ValueSource(strings = { PITC, ACME }) + void addShouldAddTenantConfigForTenantIdToMap(String tenantId) { + // arrange + act + TenantConfigs.add(tenantId, createTenantConfig(tenantId)); + + // act + var currentTenantConfig = TenantConfigs.get(tenantId); + + // assert + assertTenantConfig(tenantId, currentTenantConfig); + } + + @DisplayName("add() should not override TenantConfig for existing tenantId in the map") + @Test + void addShouldNotOverrideTenantConfigForExistingTenantIdInMap() { + // arrange + act + TenantConfigs.add(PITC, createTenantConfig(PITC)); + TenantConfigs.add(PITC, createTenantConfig("dummy")); + + // act + var currentTenantConfig = TenantConfigs.get(PITC); + + // assert + assertTenantConfig(PITC, currentTenantConfig); + } + + private void assertTenantConfig(String tenantId, TenantConfigProvider.TenantConfig current) { + assertEquals(tenantId, current.tenantId()); + assertEquals("flyway_" + tenantId, current.dataSourceConfigFlyway().name()); + assertEquals("app_" + tenantId, current.dataSourceConfigApp().name()); + } + +} diff --git a/backend/src/test/java/ch/puzzle/okr/security/JwtHelperTest.java b/backend/src/test/java/ch/puzzle/okr/security/JwtHelperTest.java index 35f0367478..e454e38aaf 100644 --- a/backend/src/test/java/ch/puzzle/okr/security/JwtHelperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/security/JwtHelperTest.java @@ -84,7 +84,7 @@ void getTenantFromTokenReturnsTenantIfTenantFoundInTenantConfigProvider() { when(tenantConfigProviderMock.getTenantConfigById(PITC)).thenReturn(Optional.of( // new TenantConfigProvider.TenantConfig(PITC, // new String[] {}, "jwkSetUri", "issuerUrl", // - "clientId", null) // + "clientId", null, null) // )); JwtHelper jwtHelper = new JwtHelper(tenantConfigProviderMock, null, null, null); @@ -122,7 +122,7 @@ void getTenantFromJWTClaimsSetReturnsTenantIfTenantFoundInTenantConfigProvider() when(tenantConfigProviderWithDataMock.getTenantConfigById(PITC)).thenReturn(Optional.of( // new TenantConfigProvider.TenantConfig(PITC, // new String[] {}, "jwkSetUri", "issuerUrl", // - "clientId", null) // + "clientId", null, null) // )); JwtHelper jwtHelper = new JwtHelper(tenantConfigProviderWithDataMock, null, null, null); diff --git a/backend/src/test/java/ch/puzzle/okr/security/TenantJwtIssuerValidatorTest.java b/backend/src/test/java/ch/puzzle/okr/security/TenantJwtIssuerValidatorTest.java index 7c675c4204..cb94be43c2 100644 --- a/backend/src/test/java/ch/puzzle/okr/security/TenantJwtIssuerValidatorTest.java +++ b/backend/src/test/java/ch/puzzle/okr/security/TenantJwtIssuerValidatorTest.java @@ -51,7 +51,7 @@ void validateReturnOAuth2TokenValidatorResultIfTenantConfigWithIssuerUrlIsFound( when(tenantConfigProviderWithPitcConfig.getTenantConfigById(PITC)).thenReturn(Optional.of( // new TenantConfigProvider.TenantConfig( // PITC, new String[] {}, "jwkSetUri", // - ISSUER_URL, "clientId", null))); + ISSUER_URL, "clientId", null, null))); TenantJwtIssuerValidator tenantJwtIssuerValidator = new TenantJwtIssuerValidator( tenantConfigProviderWithPitcConfig, jwtHelper) { diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java index 958eaac311..32838f4882 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/UserUpdateHelperTest.java @@ -47,7 +47,7 @@ void updateUserAsNoChampion() { // arrange User noChampionUser = User.Builder.builder().withEmail("no@champions.ch").build(); TenantConfigProvider.TenantConfig tenantConfig = new TenantConfigProvider.TenantConfig(null, // - new String[] { "yes@champions.ch" }, null, null, null, null); + new String[] { "yes@champions.ch" }, null, null, null, null, null); // act User updatedUser = helper.setOkrChampionFromProperties(noChampionUser, tenantConfig); @@ -61,7 +61,7 @@ void updateUserAsChampion() { // arrange User championUser = User.Builder.builder().withEmail("yes@champions.ch").build(); TenantConfigProvider.TenantConfig tenantConfig = new TenantConfigProvider.TenantConfig(null, // - new String[] { "yes@champions.ch" }, null, null, null, null); + new String[] { "yes@champions.ch" }, null, null, null, null, null); // act User updatedUser = helper.setOkrChampionFromProperties(championUser, tenantConfig); diff --git a/backend/src/test/java/ch/puzzle/okr/service/clientconfig/ClientConfigServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/clientconfig/ClientConfigServiceTest.java index aced05adae..3e5b180fe5 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/clientconfig/ClientConfigServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/clientconfig/ClientConfigServiceTest.java @@ -102,7 +102,7 @@ private TenantConfigProvider.TenantConfig getTenantConfig(String tenantId) { prefix(tenantId) + "jwkSetUri", // prefix(tenantId) + "issuerUrl", // prefix(tenantId) + "clientId", // - null); + null, null); } private TenantClientCustomization getTenantClientCustomization(String tenantId) { diff --git a/docker/dataset/init.sql b/docker/dataset/init.sql index 9824907b93..cba97a314e 100644 --- a/docker/dataset/init.sql +++ b/docker/dataset/init.sql @@ -1,15 +1,56 @@ --- pitc -create user pitc with encrypted password 'pwd'; +-- flyway user for pitc +-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +create user pitc_fly with encrypted password 'pwd'; create schema if not exists okr_pitc; -grant create on schema okr_pitc to pitc; -grant all privileges on all tables in schema okr_pitc to pitc; -grant all privileges on all sequences in schema okr_pitc to pitc; -grant usage on schema okr_pitc to pitc; +grant create on schema okr_pitc to pitc_fly; -- "create" permissions on schema okr_pitc +grant usage on schema okr_pitc to pitc_fly; -- "usage" permissions on schema okr_pitc --- acme -create user acme with encrypted password 'pwd'; +-- grants the highest possible level of access to all objects (tables, sequences) within the okr_pitc schema to the user account pitc_fly. +-- pitc_fly will have complete access to all objects (tables, sequences) within the okr_pitc schema and can perform any operation on those objects, +-- including: +-- Creating new tables +-- Modifying existing tables (e.g., adding or dropping columns) +-- Dropping tables +-- Executing queries against tables +grant all privileges on all tables in schema okr_pitc to pitc_fly; +grant all privileges on all sequences in schema okr_pitc to pitc_fly; + + +-- flyway user for acme (has access (read, write, truncate etc.) to schema okr_acme) +-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +create user acme_fly with encrypted password 'pwd'; create schema if not exists okr_acme; -grant create on schema okr_acme to acme; -grant all privileges on all tables in schema okr_acme to acme; -grant all privileges on all sequences in schema okr_acme to acme; -grant usage on schema okr_acme to acme; \ No newline at end of file +grant create on schema okr_acme to acme_fly; +grant usage on schema okr_acme to acme_fly; +grant all privileges on all tables in schema okr_acme to acme_fly; +grant all privileges on all sequences in schema okr_acme to acme_fly; + + +-- app user for no specific tenant (has no access to okr_pitc/okr_acme schemas) +-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +create user bootstrap_app with encrypted password 'pwd'; +grant usage on schema public to bootstrap_app; + + +-- pitc application user +-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +create user pitc_app with encrypted password 'pwd'; +grant usage on schema okr_pitc to pitc_app; + +-- grant select/insert/update/delete to user pitc_app in schema okr_pitc +alter default privileges for user pitc_fly in schema okr_pitc + grant select, insert, update, delete on tables to pitc_app; + +alter default privileges for user pitc_fly in schema okr_pitc + grant usage, select on sequences to pitc_app; + + +-- acme application user +-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +create user acme_app with encrypted password 'pwd'; +grant usage on schema okr_acme to acme_app; +alter default privileges for user acme_fly in schema okr_acme + grant select, insert, update, delete on tables to acme_app; + +alter default privileges for user acme_fly in schema okr_acme + grant usage, select on sequences to acme_app; diff --git a/frontend/src/assets/i18n/de.json b/frontend/src/assets/i18n/de.json index be9c8d61a4..fb5e054302 100644 --- a/frontend/src/assets/i18n/de.json +++ b/frontend/src/assets/i18n/de.json @@ -65,6 +65,38 @@ } } }, + "CONFIRMATION": { + "DRAFT_CREATE": { + "TITLE": "Check-in im Draft-Status", + "TEXT": "Dein Objective befindet sich noch im DRAFT Status. Möchtest du das Check-in trotzdem erfassen?" + }, + "RELEASE": { + "TITLE": "Objective veröffentlichen", + "TEXT": "Soll dieses Objective veröffentlicht werden?" + }, + "TO_DRAFT": { + "TITLE": "Objective als Draft speichern", + "TEXT": "Soll dieses Objective als Draft gespeichert werden?" + }, + "DELETE": { + "ACTION":{ + "TITLE": "Löschen bestätigen", + "TEXT": "Möchtest du die Action wirklich löschen?" + }, + "TEAM":{ + "TITLE": "Löschen bestätigen", + "TEXT": "Möchtest du dieses Team wirklich löschen? Zugehörige Objectives werden dadurch in allen Quartalen ebenfalls gelöscht!" + }, + "OBJECTIVE":{ + "TITLE": "Objective löschen", + "TEXT": "Möchtest du dieses Objective wirklich löschen? Zugehörige Key Results werden dadurch ebenfalls gelöscht!" + }, + "KEYRESULT":{ + "TITLE": "Key Result löschen", + "TEXT": "Möchtest du dieses Key Result wirklich löschen? Zugehörige Check-ins werden dadurch ebenfalls gelöscht!" + } + } + }, "ERROR": { "UNAUTHORIZED": "Du bist nicht autorisiert, um das Objekt mit der Id {1} zu öffnen.", "NOT_FOUND": "Das Objekt '{0}' mit der Id {1} konnte nicht gefunden werden.", From d17af9c57f5d2c0cfca7d418d6d8cbf71b70a188 Mon Sep 17 00:00:00 2001 From: MasterEvarior Date: Thu, 21 Nov 2024 08:06:33 +0100 Subject: [PATCH 2/8] Remove duplicated translation key --- frontend/src/assets/i18n/de.json | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/frontend/src/assets/i18n/de.json b/frontend/src/assets/i18n/de.json index fb5e054302..be9c8d61a4 100644 --- a/frontend/src/assets/i18n/de.json +++ b/frontend/src/assets/i18n/de.json @@ -65,38 +65,6 @@ } } }, - "CONFIRMATION": { - "DRAFT_CREATE": { - "TITLE": "Check-in im Draft-Status", - "TEXT": "Dein Objective befindet sich noch im DRAFT Status. Möchtest du das Check-in trotzdem erfassen?" - }, - "RELEASE": { - "TITLE": "Objective veröffentlichen", - "TEXT": "Soll dieses Objective veröffentlicht werden?" - }, - "TO_DRAFT": { - "TITLE": "Objective als Draft speichern", - "TEXT": "Soll dieses Objective als Draft gespeichert werden?" - }, - "DELETE": { - "ACTION":{ - "TITLE": "Löschen bestätigen", - "TEXT": "Möchtest du die Action wirklich löschen?" - }, - "TEAM":{ - "TITLE": "Löschen bestätigen", - "TEXT": "Möchtest du dieses Team wirklich löschen? Zugehörige Objectives werden dadurch in allen Quartalen ebenfalls gelöscht!" - }, - "OBJECTIVE":{ - "TITLE": "Objective löschen", - "TEXT": "Möchtest du dieses Objective wirklich löschen? Zugehörige Key Results werden dadurch ebenfalls gelöscht!" - }, - "KEYRESULT":{ - "TITLE": "Key Result löschen", - "TEXT": "Möchtest du dieses Key Result wirklich löschen? Zugehörige Check-ins werden dadurch ebenfalls gelöscht!" - } - } - }, "ERROR": { "UNAUTHORIZED": "Du bist nicht autorisiert, um das Objekt mit der Id {1} zu öffnen.", "NOT_FOUND": "Das Objekt '{0}' mit der Id {1} konnte nicht gefunden werden.", From 2d68444218e0c15331f5debc33f8c18afdf19fa2 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 25 Nov 2024 12:52:00 +0100 Subject: [PATCH 3/8] renaming enum DbType + convenience method + getProperty() helper methods --- .../multitenancy/TenantConfigProvider.java | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java index d1dbe99d6c..aafd990c8b 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java @@ -9,6 +9,8 @@ import java.text.MessageFormat; import java.util.*; +import static java.text.MessageFormat.format; + /** * Reads the configuration of the tenants (as TenantConfig objects) from the applicationX.properties and caches each * TenantConfig in the TenantConfigs class. @@ -20,7 +22,11 @@ public class TenantConfigProvider implements TenantConfigProviderInterface { private final Environment env; private enum DbType { - bootstrap, app, fly + BOOTSTRAP, APP, FLY; + + public String nameUsedInProperties() { + return this.name().toLowerCase(); + } } private static final Logger logger = LoggerFactory.getLogger(TenantConfigProvider.class); @@ -58,11 +64,10 @@ public static void clearTenantConfigsCache() { } private OauthConfig readOauthConfig(String tenantId) { - return new OauthConfig( - env.getProperty(MessageFormat.format("okr.tenants.{0}.security.oauth2.resourceserver.jwt.jwk-set-uri", - tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.security.oauth2.frontend.issuer-url", tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.security.oauth2.frontend.client-id", tenantId))); + return new OauthConfig( // + getProperty("okr.tenants.{0}.security.oauth2.resourceserver.jwt.jwk-set-uri", tenantId), + getProperty("okr.tenants.{0}.security.oauth2.frontend.issuer-url", tenantId), + getProperty("okr.tenants.{0}.security.oauth2.frontend.client-id", tenantId)); } private TenantConfig createTenantConfig(String jwkSetUriTemplate, String frontendClientIssuerUrl, @@ -75,8 +80,8 @@ private TenantConfig createTenantConfig(String jwkSetUriTemplate, String fronten } private String[] getOkrChampionEmailsFromTenant(String tenantId) { - return Arrays.stream(env.getProperty(MessageFormat.format("okr.tenants.{0}.user.champion.emails", tenantId), "") - .split(EMAIL_DELIMITER)).map(String::trim).toArray(String[]::new); + return Arrays.stream(getProperty("okr.tenants.{0}.user.champion.emails", tenantId, "").split(EMAIL_DELIMITER)) + .map(String::trim).toArray(String[]::new); } public List getTenantConfigs() { @@ -84,19 +89,32 @@ public List getTenantConfigs() { } private DataSourceConfig readDataSourceConfigFlyway(String tenantId) { - return readDataSourceConfig(tenantId, DbType.fly); + return readDataSourceConfig(tenantId, DbType.FLY); } private DataSourceConfig readDataSourceConfigApp(String tenantId) { - return readDataSourceConfig(tenantId, DbType.app); + return readDataSourceConfig(tenantId, DbType.APP); } private DataSourceConfig readDataSourceConfig(String tenantId, DbType dbType) { - return new DataSourceConfig(env.getProperty("okr.datasource.driver-class-name"), - env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.url", tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.username." + dbType, tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.password." + dbType, tenantId)), - env.getProperty(MessageFormat.format("okr.tenants.{0}.datasource.schema", tenantId))); + return new DataSourceConfig( // + getProperty("okr.datasource.driver-class-name"), + getProperty("okr.tenants.{0}.datasource.url", tenantId), + getProperty("okr.tenants.{0}.datasource.username." + dbType.nameUsedInProperties(), tenantId), + getProperty("okr.tenants.{0}.datasource.password." + dbType.nameUsedInProperties(), tenantId), + getProperty("okr.tenants.{0}.datasource.schema", tenantId)); + } + + private String getProperty(String key) { + return env.getProperty(String.format(key)); + } + + private String getProperty(String key, String tenantId) { + return env.getProperty(MessageFormat.format(key, tenantId)); + } + + private String getProperty(String key, String tenantId, String defaultValue) { + return env.getProperty(format(key, tenantId), defaultValue); } public Optional getTenantConfigById(String tenantId) { From 681f81723b200f0f37fe1d8dae8687fa43c4d608 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 25 Nov 2024 13:08:31 +0100 Subject: [PATCH 4/8] remove tearDown() --- .../puzzle/okr/multitenancy/HibernateContextTest.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java index 9e64499915..c972b39cb5 100644 --- a/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java @@ -1,6 +1,5 @@ package ch.puzzle.okr.multitenancy; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -26,11 +25,6 @@ void setUp() { resetHibernateConfig(); } - @AfterEach - void tearDown() { - resetHibernateConfig(); - } - @DisplayName("setHibernateConfig() should throw exception if db config is null") @Test void setHibernateConfigShouldThrowExceptionIfDbConfigIsNull() { @@ -136,9 +130,7 @@ void setHibernateConfigForTenantIdShouldThrowExceptionWhenNoTenantConfigIsCached var tenantId = "tenantId"; // but no tenant config is cached // act + assert - var exception = assertThrows(RuntimeException.class, () -> { - getHibernateConfig(tenantId); - }); + var exception = assertThrows(RuntimeException.class, () -> getHibernateConfig(tenantId)); assertEquals("No cached tenant configuration found (for tenant tenantId)", exception.getMessage()); } From 5c411048041d5dc3f333bef867ca6ffd55f6cf7d Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 25 Nov 2024 13:10:18 +0100 Subject: [PATCH 5/8] change Log level: error -> info --- .../multitenancy/FlywayMultitenantMigrationInitializer.java | 2 +- .../java/ch/puzzle/okr/multitenancy/HibernateContext.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java index 9e7fe04603..3c95301464 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java @@ -41,6 +41,6 @@ public void migrateFlyway() { } private void logUsedHibernateConfig(TenantConfigProvider.DataSourceConfig dataSourceConfig) { - logger.error("use DbConfig: user={}", dataSourceConfig.name()); + logger.info("use DbConfig: user={}", dataSourceConfig.name()); } } \ No newline at end of file diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java index 86b8f0e8d9..19edea8c8f 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java @@ -127,16 +127,16 @@ private static Properties patchConfigAppForTenant(Properties properties, String } private static void logUsedHibernateConfig(DbConfig hibernateConfig) { - logger.error("set DbConfig: user={}", hibernateConfig.username()); + logger.info("set DbConfig: user={}", hibernateConfig.username()); } private static void logUsedHibernateConfig(Properties hibernateConfig) { - logger.error("use DbConfig: user={}", + logger.info("use DbConfig: user={}", hibernateConfig.getProperty(HibernateContext.HIBERNATE_CONNECTION_USERNAME)); // } private static void logUsedHibernateConfig(String tenantId, Properties hibernateConfig) { - logger.error("use DbConfig: tenant={} user={}", tenantId, + logger.info("use DbConfig: tenant={} user={}", tenantId, hibernateConfig.getProperty(HibernateContext.HIBERNATE_CONNECTION_USERNAME)); } From 08bc29cb314371f93e4ed33ad9ca03bc8eb67c1f Mon Sep 17 00:00:00 2001 From: clean-coder Date: Mon, 25 Nov 2024 13:10:56 +0100 Subject: [PATCH 6/8] renaming enum DbType + convenience method + getProperty() helper methods --- .../java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java index aafd990c8b..1614b8d433 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/TenantConfigProvider.java @@ -49,7 +49,7 @@ private void cacheTenantConfig(String tenantId, TenantConfig tenantConfig) { } private void logCachingTenantConfig(String tenantId, TenantConfig tenantConfig) { - logger.error("cache TenantConfig: tenantId={}, users={}", // + logger.info("cache TenantConfig: tenantId={}, users={}", // tenantId, // tenantConfig.dataSourceConfigFlyway().name() + " | " + tenantConfig.dataSourceConfigApp().name()); } From 1d8b104ef944d683abc173edf3d9814b25e47a99 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Tue, 26 Nov 2024 11:15:41 +0100 Subject: [PATCH 7/8] for null values for tenantId throw always a ConnectionProviderException test for tenantId == null --- .../SchemaMultiTenantConnectionProvider.java | 17 +++++-- ...hemaMultiTenantConnectionProviderTest.java | 49 +++++++++++++++---- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java index 5c11016066..c10aeb50b9 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProvider.java @@ -89,7 +89,7 @@ ConnectionProvider getConnectionProvider(String tenantIdentifier) { .orElseGet(() -> createAndCacheNewConnectionProvider(tenantIdentifier)); } - private ConnectionProvider createAndCacheNewConnectionProvider(String tenantIdentifier) { + ConnectionProvider createAndCacheNewConnectionProvider(String tenantIdentifier) { return Optional.ofNullable(tenantIdentifier) // .map(this::createConnectionProvider) // .map(connectionProvider -> { @@ -110,7 +110,7 @@ private ConnectionProvider createConnectionProvider(String tenantIdentifier) { Properties getHibernatePropertiesForTenantIdentifier(String tenantIdentifier) { Properties properties = getHibernateProperties(tenantIdentifier); if (properties.isEmpty()) { - throw new RuntimeException("Cannot load hibernate properties from application.properties"); + throw new ConnectionProviderException("Cannot load hibernate properties from application.properties"); } if (!Objects.equals(tenantIdentifier, DEFAULT_TENANT_ID)) { properties.put(AvailableSettings.DEFAULT_SCHEMA, MessageFormat.format("okr_{0}", tenantIdentifier)); @@ -135,9 +135,16 @@ Map convertPropertiesToMap(Properties properties) { } private Properties getHibernateProperties(String tenantIdentifier) { - if (tenantIdentifier.equals(DEFAULT_TENANT_ID)) { - return HibernateContext.getHibernateConfig(); + if (tenantIdentifier == null) { + throw new ConnectionProviderException("No hibernate configuration found for tenant: " + tenantIdentifier); + } + try { + if (tenantIdentifier.equals(DEFAULT_TENANT_ID)) { + return HibernateContext.getHibernateConfig(); + } + return HibernateContext.getHibernateConfig(tenantIdentifier); + } catch (RuntimeException e) { + throw new ConnectionProviderException(e.getMessage()); } - return HibernateContext.getHibernateConfig(tenantIdentifier); } } \ No newline at end of file diff --git a/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderTest.java b/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderTest.java index e9a97a6d14..66c9b9685f 100644 --- a/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderTest.java +++ b/backend/src/test/java/ch/puzzle/okr/multitenancy/SchemaMultiTenantConnectionProviderTest.java @@ -1,6 +1,8 @@ package ch.puzzle.okr.multitenancy; +import ch.puzzle.okr.exception.ConnectionProviderException; import ch.puzzle.okr.test.SpringIntegrationTest; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.mockito.Mock; @@ -9,30 +11,40 @@ import java.sql.SQLException; import java.sql.Statement; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @SpringIntegrationTest public class SchemaMultiTenantConnectionProviderTest { + @Mock - Connection connection; + private Connection connection; @Mock - Statement statement; + private Statement statement; + + private SchemaMultiTenantConnectionProvider connectionProvider; + + private static final String PITC_TENANT_ID = "pitc"; + private static final String DEFAULT_TENANT_ID = TenantContext.DEFAULT_TENANT_ID; - private static final String TENANT_ID = "pitc"; + @BeforeEach + void setup() { + connectionProvider = new SchemaMultiTenantConnectionProvider(); + } @DisplayName("getConnection() should set schema with okr prefix") @Test void getConnectionShouldSetSchemaWithOkrPrefix() throws SQLException { // arrange when(connection.createStatement()).thenReturn(statement); - SchemaMultiTenantConnectionProvider provider = new SchemaMultiTenantConnectionProvider(); // act - provider.getConnection(TENANT_ID, connection); + connectionProvider.getConnection(PITC_TENANT_ID, connection); // assert - verify(statement).execute("SET SCHEMA 'okr_" + TENANT_ID + "';"); + verify(statement).execute("SET SCHEMA 'okr_" + PITC_TENANT_ID + "';"); } @DisplayName("getConnection() should set schema without okr prefix if tenant id is DEFAULT_TENANT_ID") @@ -40,13 +52,32 @@ void getConnectionShouldSetSchemaWithOkrPrefix() throws SQLException { void getConnectionShouldSetSchemaWithoutOkrPrefixIfTenantIdIsDefaultTenantId() throws SQLException { // arrange when(connection.createStatement()).thenReturn(statement); - SchemaMultiTenantConnectionProvider provider = new SchemaMultiTenantConnectionProvider(); // act - provider.getConnection(TenantContext.DEFAULT_TENANT_ID, connection); + connectionProvider.getConnection(DEFAULT_TENANT_ID, connection); // assert - verify(statement).execute("SET SCHEMA '" + TenantContext.DEFAULT_TENANT_ID + "';"); + verify(statement).execute("SET SCHEMA '" + DEFAULT_TENANT_ID + "';"); + } + + @DisplayName("createAndCacheNewConnectionProvider() should throw exception if tenantId is null") + @Test + void createAndCacheNewConnectionProviderShouldThrowExceptionIfTenantIdIsNull() { + // act + assert + var exception = assertThrows(ConnectionProviderException.class, + () -> connectionProvider.createAndCacheNewConnectionProvider(null)); + + assertEquals("Cannot create new connection provider for tenant: null", exception.getMessage()); + } + + @DisplayName("getHibernatePropertiesForTenantIdentifier() should throw exception if tenantId is null") + @Test + void getHibernatePropertiesForTenantIdentifierShouldThrowExceptionIfTenantIdIsNull() { + // act + assert + var exception = assertThrows(ConnectionProviderException.class, + () -> connectionProvider.getHibernatePropertiesForTenantIdentifier(null)); + + assertEquals("No hibernate configuration found for tenant: null", exception.getMessage()); } } From fe7a055c2df7a93402fd7aa145c1fe8381092f64 Mon Sep 17 00:00:00 2001 From: clean-coder Date: Tue, 26 Nov 2024 11:39:43 +0100 Subject: [PATCH 8/8] variable renaming: dataSourceConfig -> dataSourceConfigFlyway --- .../FlywayMultitenantMigrationInitializer.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java b/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java index 3c95301464..00154abb10 100644 --- a/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java +++ b/backend/src/main/java/ch/puzzle/okr/multitenancy/FlywayMultitenantMigrationInitializer.java @@ -22,18 +22,20 @@ public FlywayMultitenantMigrationInitializer(TenantConfigProviderInterface tenan public void migrateFlyway() { this.tenantConfigProvider.getTenantConfigs().forEach((tenantConfig) -> { - TenantConfigProvider.DataSourceConfig dataSourceConfig = this.tenantConfigProvider + TenantConfigProvider.DataSourceConfig dataSourceConfigFlyway = this.tenantConfigProvider .getTenantConfigById(tenantConfig.tenantId()) - .map(TenantConfigProvider.TenantConfig::dataSourceConfigFlyway).orElseThrow( + .map(TenantConfigProvider.TenantConfig::dataSourceConfigFlyway) // + .orElseThrow( () -> new EntityNotFoundException("Cannot find tenant for configuring flyway migration")); - logUsedHibernateConfig(dataSourceConfig); + logUsedHibernateConfig(dataSourceConfigFlyway); Flyway tenantSchemaFlyway = Flyway.configure() // - .dataSource(dataSourceConfig.url(), dataSourceConfig.name(), dataSourceConfig.password()) // + .dataSource(dataSourceConfigFlyway.url(), dataSourceConfigFlyway.name(), + dataSourceConfigFlyway.password()) // .locations(scriptLocations) // .baselineOnMigrate(Boolean.TRUE) // - .schemas(dataSourceConfig.schema()) // + .schemas(dataSourceConfigFlyway.schema()) // .load(); tenantSchemaFlyway.migrate();