Repository: focus-shift/jollyday
Pull request #431 (2024-01-16 15:34:09)

Add tests for Cache and HolidayManagerValueHandler
<!--

Thanks for contributing.
Please review the following notes before submitting you pull request.

Please look for other issues or pull requests which already work on this topic. Is somebody already on it? Do you need to synchronize?

# Security Vulnerabilities

🛑 STOP! 🛑 If your contribution fixes a security vulnerability, please do not submit it.
Instead, please write an E-Mail to to one of the maintainer with all the information
to recreate the security vulnerability.

# Describing Your Changes

If, having reviewed the notes above, you're ready to submit your pull request, please
provide a brief description of the proposed changes.

If they:
 🐞 fix a bug, please describe the broken behaviour and how the changes fix it.
    Please label with 'type: bug' and 'status: new'
    
 🎁 make an enhancement, please describe the new functionality and why you believe it's useful.
    Please label with 'type: enhancement' and 'status: new'
 
If your pull request relates to any existing issues,
please reference them by using the issue number prefixed with #.

-->



Patch:
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java
index 41888638..45a36add 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java
@@ -3,8 +3,10 @@ package de.focus_shift.jollyday.core;
 import de.focus_shift.jollyday.core.caching.Cache;
 import de.focus_shift.jollyday.core.caching.HolidayManagerValueHandler;
 import de.focus_shift.jollyday.core.configuration.ConfigurationProviderManager;
+import de.focus_shift.jollyday.core.datasource.ConfigurationServiceManager;
 import de.focus_shift.jollyday.core.parser.functions.CalendarToLocalDate;
 import de.focus_shift.jollyday.core.spi.ConfigurationService;
+import de.focus_shift.jollyday.core.support.LazyServiceLoaderCache;
 import de.focus_shift.jollyday.core.util.CalendarUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -32,14 +34,24 @@ public abstract class HolidayManager {
    * to getInstance will return a newly instantiated and initialized manager.
    */
   private static boolean cachingEnabled = true;
+
   /**
    * Cache for manager instances on a per country basis.
    */
   private static final Cache<HolidayManager> HOLIDAY_MANAGER_CACHE = new Cache<>();
+
   /**
    * Manager for configuration providers. Delivers the jollyday configuration.
    */
   private static final ConfigurationProviderManager CONFIGURATION_MANAGER_PROVIDER = new ConfigurationProviderManager();
+
+  /**
+   * Manager for providing configuration data sources which return the holiday
+   * data.
+   */
+  private static final ConfigurationServiceManager configurationServiceManager =
+    new ConfigurationServiceManager(new LazyServiceLoaderCache<>(ConfigurationService.class));
+
   /**
    * the holiday cache
    */
@@ -100,7 +112,7 @@ public abstract class HolidayManager {
     LOG.debug("Creating HolidayManager for calendar '{}'. Caching enabled: {}", parameter, isManagerCachingEnabled());
     CONFIGURATION_MANAGER_PROVIDER.mergeConfigurationProperties(parameter);
     final String managerImplClassName = readManagerImplClassName(parameter);
-    final HolidayManagerValueHandler holidayManagerValueHandler = new HolidayManagerValueHandler(parameter, managerImplClassName);
+    final HolidayManagerValueHandler holidayManagerValueHandler = new HolidayManagerValueHandler(parameter, managerImplClassName, configurationServiceManager);
     if (isManagerCachingEnabled()) {
       return HOLIDAY_MANAGER_CACHE.get(holidayManagerValueHandler);
     } else {
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/Cache.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/Cache.java
index 5aae2a0c..dc85e7cc 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/Cache.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/Cache.java
@@ -9,22 +9,17 @@ import java.util.concurrent.ConcurrentHashMap;
  * @param <V> the type of cached values
  */
 public class Cache<V> {
-  /**
-   * Map for caching
-   */
+
   private final Map<String, V> cachingMap = new ConcurrentHashMap<>();
 
   /**
    * Returns the value defined by the {@link ValueHandler}
    *
    * @param valueHandler which creates the key and the value if necessary
-   * @return the eventually cached value
+   * @return the eventually cached value, otherwise the newly created via {@link ValueHandler#createValue}
    */
   public V get(ValueHandler<V> valueHandler) {
-    final String key = valueHandler.getKey();
-    // Try to first get the value which is most likely cached to avoid creating a lambda.
-    final V value = cachingMap.get(key);
-    return value != null ? value : cachingMap.computeIfAbsent(key, k -> valueHandler.createValue());
+    return cachingMap.computeIfAbsent(valueHandler.getKey(), key -> valueHandler.createValue());
   }
 
   /**
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java
index 1a1dcade..94eedcb9 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java
@@ -4,7 +4,6 @@ import de.focus_shift.jollyday.core.HolidayManager;
 import de.focus_shift.jollyday.core.ManagerParameter;
 import de.focus_shift.jollyday.core.datasource.ConfigurationServiceManager;
 import de.focus_shift.jollyday.core.spi.ConfigurationService;
-import de.focus_shift.jollyday.core.support.LazyServiceLoaderCache;
 import de.focus_shift.jollyday.core.util.ClassLoadingUtil;
 
 /**
@@ -14,22 +13,17 @@ public class HolidayManagerValueHandler implements Cache.ValueHandler<HolidayMan
 
   private final ManagerParameter parameter;
   private final String managerImplClassName;
-
-  /**
-   * Manager for providing configuration data sources which return the holiday
-   * data.
-   */
-  private final ConfigurationServiceManager configurationServiceManager =
-    new ConfigurationServiceManager(new LazyServiceLoaderCache<>(ConfigurationService.class));
+  private final ConfigurationServiceManager configurationServiceManager;
 
   /**
    * Utility to load classes.
    */
   private final ClassLoadingUtil classLoadingUtil = new ClassLoadingUtil();
 
-  public HolidayManagerValueHandler(final ManagerParameter parameter, final String managerImplClassName) {
+  public HolidayManagerValueHandler(final ManagerParameter parameter, final String managerImplClassName, final ConfigurationServiceManager configurationServiceManager) {
     this.parameter = parameter;
     this.managerImplClassName = managerImplClassName;
+    this.configurationServiceManager = configurationServiceManager;
   }
 
   @Override
@@ -40,8 +34,10 @@ public class HolidayManagerValueHandler implements Cache.ValueHandler<HolidayMan
   @Override
   public HolidayManager createValue() {
     final HolidayManager manager = instantiateManagerImpl(managerImplClassName);
+
     final ConfigurationService configurationService = configurationServiceManager.getConfigurationService();
     manager.setConfigurationService(configurationService);
+
     manager.init(parameter);
     return manager;
   }


Test patch:
diff --git a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/CacheTest.java b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/CacheTest.java
new file mode 100644
index 00000000..28f90636
--- /dev/null
+++ b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/CacheTest.java
@@ -0,0 +1,77 @@
+package de.focus_shift.jollyday.core.caching;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class CacheTest {
+
+  @Test
+  void ensureToWriteIntoCacheAndGetSameValueWithSameKey() {
+    final int[] callCounter = new int[]{0};
+
+    final Cache.ValueHandler<String> valueHandler = new Cache.ValueHandler<>() {
+      @Override
+      public String getKey() {
+        return "key";
+      }
+
+      @Override
+      public String createValue() {
+        callCounter[0]++;
+        return "value";
+      }
+    };
+
+    final Cache<String> stringCache = new Cache<>();
+    assertThat(stringCache.get(valueHandler)).isEqualTo("value");
+    assertThat(stringCache.get(valueHandler)).isEqualTo("value");
+    assertThat(callCounter[0]).isEqualTo(1);
+  }
+
+  @Test
+  void ensureToWriteIntoCacheWithDifferentKeys() {
+    final int[] callCounter = new int[]{0, 0};
+
+    final Cache.ValueHandler<String> valueHandler = new Cache.ValueHandler<>() {
+      @Override
+      public String getKey() {
+        return String.valueOf(callCounter[1]++);
+      }
+
+      @Override
+      public String createValue() {
+        return "value" + callCounter[0]++;
+      }
+    };
+
+    final Cache<String> stringCache = new Cache<>();
+    assertThat(stringCache.get(valueHandler)).isEqualTo("value0");
+    assertThat(stringCache.get(valueHandler)).isEqualTo("value1");
+    assertThat(callCounter[0]).isEqualTo(2);
+  }
+
+  @Test
+  void ensureToClearTheCacheSoThatTheSecondCallWasNotCachedAndMustBeCreatedAgain() {
+    final int[] callCounter = new int[]{0};
+
+    final Cache.ValueHandler<String> valueHandler = new Cache.ValueHandler<>() {
+      @Override
+      public String getKey() {
+        return "key";
+      }
+
+      @Override
+      public String createValue() {
+        callCounter[0]++;
+        return "value";
+      }
+    };
+
+    final Cache<String> stringCache = new Cache<>();
+    assertThat(stringCache.get(valueHandler)).isEqualTo("value");
+    stringCache.clear();
+    assertThat(stringCache.get(valueHandler)).isEqualTo("value");
+    assertThat(callCounter[0]).isEqualTo(2);
+  }
+}
diff --git a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandlerTest.java b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandlerTest.java
new file mode 100644
index 00000000..f79ef045
--- /dev/null
+++ b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandlerTest.java
@@ -0,0 +1,73 @@
+package de.focus_shift.jollyday.core.caching;
+
+import de.focus_shift.jollyday.core.HolidayManager;
+import de.focus_shift.jollyday.core.ManagerParameter;
+import de.focus_shift.jollyday.core.datasource.ConfigurationServiceManager;
+import de.focus_shift.jollyday.core.impl.DefaultHolidayManager;
+import de.focus_shift.jollyday.core.impl.JapaneseHolidayManager;
+import de.focus_shift.jollyday.core.spi.Configuration;
+import de.focus_shift.jollyday.core.spi.ConfigurationService;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.stream.Stream;
+
+import static de.focus_shift.jollyday.core.ManagerParameters.create;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class HolidayManagerValueHandlerTest {
+
+  @Test
+  void ensureToGetCorrectKey() {
+    final String managerImplClassName = "de.focus_shift.jollyday.core.impl.DefaultHolidayManager";
+    final ConfigurationServiceManager configurationServiceManager = mock(ConfigurationServiceManager.class);
+    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), managerImplClassName, configurationServiceManager);
+
+    final String key = sut.getKey();
+    assertThat(key).isEqualTo("de");
+  }
+
+  private static Stream<Arguments> provideHolidayManagers() {
+    return Stream.of(
+      Arguments.of("de.focus_shift.jollyday.core.impl.DefaultHolidayManager", DefaultHolidayManager.class),
+      Arguments.of("de.focus_shift.jollyday.core.impl.JapaneseHolidayManager", JapaneseHolidayManager.class)
+    );
+  }
+
+  @ParameterizedTest
+  @MethodSource("provideHolidayManagers")
+  void ensureToCreateCorrectHolidayManagerAndReturnIt(final String managerImplClass, final Class<HolidayManager> clazz) {
+
+    final ConfigurationServiceManager configurationServiceManager = mock(ConfigurationServiceManager.class);
+    final ConfigurationService configurationService = mock(ConfigurationService.class);
+    final Configuration configuration = mock(Configuration.class);
+
+    when(configurationServiceManager.getConfigurationService()).thenReturn(configurationService);
+    when(configurationService.getConfiguration(any(ManagerParameter.class))).thenReturn(configuration);
+
+    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), managerImplClass, configurationServiceManager);
+
+    final HolidayManager holidayManager = sut.createValue();
+    assertThat(holidayManager).isInstanceOf(clazz);
+  }
+
+  @Test
+  void ensureToThrowIllegalStateExceptionIfHolidayManagerIsWrong() {
+
+    final String wrongManagerImplClassName = "de.focus_shift.jollyday.core.impl.WrongHolidayManager";
+    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), wrongManagerImplClassName, mock(ConfigurationServiceManager.class));
+
+    assertThatThrownBy(sut::createValue)
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessageContaining("Cannot create manager class de.focus_shift.jollyday.core.impl.WrongHolidayManager");
+  }
+}