refactor: Move null conditions in Hibernate and Hikari context so no method solely exists for tests #1587

Author: Miguel7373

backend/src/main/java/ch/puzzle/okr/multitenancy/HibernateContext.java

@@ -32,14 +32,14 @@ private boolean hasEmptyValues() {
     private static DbConfig cachedHibernateConfig;
 
     public static void setHibernateConfig(DbConfig dbConfig) {
-        if (dbConfig == null || !dbConfig.isValid()) {
-            throw new HibernateContextException("Invalid hibernate configuration " + dbConfig);
-        }
         cachedHibernateConfig = dbConfig;
     }
 
     public static void extractAndSetHibernateConfig(ConfigurableEnvironment environment) {
         DbConfig dbConfig = extractHibernateConfig(environment);
+        if (!dbConfig.isValid()) {
+            throw new HibernateContextException("Invalid hikari configuration " + dbConfig);
+        }
         HibernateContext.setHibernateConfig(dbConfig);
     }
 
@@ -51,13 +51,9 @@ private static DbConfig extractHibernateConfig(ConfigurableEnvironment environme
         return new DbConfig(url, username, password, multiTenancy);
     }
 
-    // for testing
-    public static void resetHibernateConfig() {
-        cachedHibernateConfig = null;
-    }
-
     public static Properties getHibernateConfig() {
-        if (cachedHibernateConfig == null) {
+        // Assert non null due to prevent null pointer in method getConfigAsProperties()
+        if (cachedHibernateConfig == null || !cachedHibernateConfig.isValid()) {
             throw new HibernateContextException("No cached hibernate configuration found");
         }
         return getConfigAsProperties(cachedHibernateConfig);

backend/src/main/java/ch/puzzle/okr/multitenancy/HikariContext.java

@@ -25,14 +25,14 @@ private boolean hasEmptyValues() {
     private static DbConfig cachedHikariConfig;
 
     public static void setHikariConfig(DbConfig dbConfig) {
-        if (dbConfig == null || !dbConfig.isValid()) {
-            throw new HikariContextException("Invalid hikari configuration " + dbConfig);
-        }
         cachedHikariConfig = dbConfig;
     }
 
     public static void extractAndSetHikariConfig(ConfigurableEnvironment environment) {
         DbConfig dbConfig = extractHikariConfig(environment);
+        if (!dbConfig.isValid()) {
+            throw new HikariContextException("Invalid hikari configuration " + dbConfig);
+        }
         HikariContext.setHikariConfig(dbConfig);
     }
 
@@ -41,13 +41,9 @@ private static DbConfig extractHikariConfig(ConfigurableEnvironment environment)
         return new DbConfig(maximumPoolSize);
     }
 
-    // for testing
-    public static void resetHikariConfig() {
-        cachedHikariConfig = null;
-    }
-
     public static Properties getHikariConfig() {
-        if (cachedHikariConfig == null) {
+        // Assert non null due to prevent null pointer in method getConfigAsProperties()
+        if (cachedHikariConfig == null || !cachedHikariConfig.isValid()) {
             throw new HikariContextException("No cached hikari configuration found");
         }
         return getConfigAsProperties(cachedHikariConfig);
@@ -58,4 +54,4 @@ private static Properties getConfigAsProperties(DbConfig dbConfig) {
         properties.put(HikariContext.HIKARI_MAXIMUM_POOL_SIZE, dbConfig.maximumPoolSize());
         return properties;
     }
-}
+}

backend/src/test/java/ch/puzzle/okr/multitenancy/HibernateContextTest.java

@@ -7,59 +7,16 @@
 
 import ch.puzzle.okr.exception.HibernateContextException;
 import java.util.Properties;
-import java.util.stream.Stream;
 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.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.core.env.ConfigurableEnvironment;
 
 class HibernateContextTest {
 
     @BeforeEach
     void setUp() {
-        resetHibernateConfig();
-    }
-
-    @DisplayName("setHibernateConfig() should throw exception if db config is null")
-    @Test
-    void setHibernateConfigShouldThrowExceptionIfDbConfigIsNull() {
-        // arrange
-        DbConfig dbConfig = null;
-
-        // act + assert
-        HibernateContextException exception = assertThrows(HibernateContextException.class,
-                                                           () -> setHibernateConfig(dbConfig));
-        assertEquals("Invalid hibernate configuration null", exception.getMessage());
-    }
-
-    @ParameterizedTest(name = "setHibernateConfig() should throw exception if db config has null or empty values")
-    @MethodSource("invalidDbConfig")
-    void setHibernateConfigShouldThrowExceptionIfDbConfigHasNullOrEmptyValues(String url, String username,
-                                                                              String password, String tenant) {
-
-        // arrange
-        DbConfig dbConfig = new DbConfig(url, username, password, tenant);
-
-        // act + assert
-        HibernateContextException exception = assertThrows(HibernateContextException.class,
-                                                           () -> setHibernateConfig(dbConfig));
-        assertTrue(exception.getMessage().startsWith("Invalid hibernate configuration"));
-    }
-
-    private static Stream<Arguments> invalidDbConfig() {
-        return Stream
-                .of( //
-                    Arguments.of(null, "username", "password", "multiTenancy"), //
-                    Arguments.of("", "username", "password", "multiTenancy"), //
-                    Arguments.of("url", null, "password", "multiTenancy"), //
-                    Arguments.of("url", "", "password", "multiTenancy"), //
-                    Arguments.of("url", "username", null, "multiTenancy"), //
-                    Arguments.of("url", "username", "", "multiTenancy"), //
-                    Arguments.of("url", "username", "password", null), //
-                    Arguments.of("url", "username", "password", ""));
+        HibernateContext.setHibernateConfig(null);
     }
 
     @DisplayName("extractAndSetHibernateConfig() should extract hibernate properties from environment and set it")
@@ -86,8 +43,6 @@ void extractAndSetHibernateConfigShouldExtractHibernatePropertiesFromEnvironment
     @DisplayName("getHibernateConfig() should throw exception if setHibernateConfig() is not called before with valid configuration")
     @Test
     void getHibernateConfigShouldThrowExceptionIfSetHibernateConfigIsNotCalledBeforeWithValidConfiguration() {
-        // arrange
-
         // act + assert
         HibernateContextException exception = assertThrows(HibernateContextException.class,
                                                            HibernateContext::getHibernateConfig);

backend/src/test/java/ch/puzzle/okr/multitenancy/HikariContextTest.java

@@ -2,54 +2,19 @@
 
 import static ch.puzzle.okr.multitenancy.HikariContext.*;
 import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 import ch.puzzle.okr.exception.HikariContextException;
 import java.util.Properties;
-import java.util.stream.Stream;
 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.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.core.env.ConfigurableEnvironment;
 
 public class HikariContextTest {
     @BeforeEach
     void setUp() {
-        resetHikariConfig();
-    }
-
-    @DisplayName("setHikariConfig() should throw exception if db config is null")
-    @Test
-    void setHikariConfigShouldThrowExceptionIfDbConfigIsNull() {
-        // arrange
-        HikariContext.DbConfig dbConfig = null;
-
-        // act + assert
-        HikariContextException exception = assertThrows(HikariContextException.class, () -> setHikariConfig(dbConfig));
-        assertEquals("Invalid hikari configuration null", exception.getMessage());
-    }
-
-    private static Stream<Arguments> invalidDbConfig() {
-        return Stream
-                .of( //
-                    Arguments.of(null, ""), //
-                    Arguments.of("")); //
-    }
-
-    @ParameterizedTest(name = "setHikariConfig() should throw exception if db config has null or empty values")
-    @MethodSource("invalidDbConfig")
-    void setHikariConfigShouldThrowExceptionIfDbConfigHasNullOrEmptyValues(String maximumPoolSize) {
-
-        // arrange
-        DbConfig dbConfig = new DbConfig(maximumPoolSize);
-
-        // act + assert
-        HikariContextException exception = assertThrows(HikariContextException.class, () -> setHikariConfig(dbConfig));
-        assertTrue(exception.getMessage().startsWith("Invalid hikari configuration"));
+        HikariContext.setHikariConfig(null);
     }
 
     @DisplayName("extractAndSetHikariConfig() should extract hikari properties from environment and set it")
@@ -67,14 +32,12 @@ void extractAndSetHikariConfigShouldExtractHikariPropertiesFromEnvironmentAndSet
 
         // assert
         assertNotNull(hikariProperties);
-        assertProperties(maximumPoolSize, hikariProperties);
+        assertProperty(maximumPoolSize, hikariProperties);
     }
 
     @DisplayName("getHikariConfig() should throw exception if setHikariConfig() is not called before with valid configuration")
     @Test
     void getHikariConfigShouldThrowExceptionIfSetHikariConfigIsNotCalledBeforeWithValidConfiguration() {
-        // arrange
-
         // act + assert
         HikariContextException exception = assertThrows(HikariContextException.class, HikariContext::getHikariConfig);
         assertEquals("No cached hikari configuration found", exception.getMessage());
@@ -93,12 +56,11 @@ void getHikariConfigShouldReturnHikariConfigAsPropertiesIfDbConfigIsValid() {
 
         // assert
         assertNotNull(hikariProperties);
-        assertProperties(maximumPoolSize, hikariProperties);
+        assertProperty(maximumPoolSize, hikariProperties);
     }
 
-    private void assertProperties(String maximumPoolSize, Properties properties) {
-
-        assertEquals(maximumPoolSize, properties.get(HIKARI_MAXIMUM_POOL_SIZE));
+    private void assertProperty(String maximumPoolSize, Properties property) {
+        assertEquals(maximumPoolSize, property.get(HIKARI_MAXIMUM_POOL_SIZE));
     }
 
 }