Repository: focus-shift/jollyday (java)

Issue:
Configuration to Specify XML Configuration Service Implementation When Multiple Are on the Classpath
### More information

Currently, if the dependencies `de.focus-shift:jollyday-jackson` and `de.focus-shift:jollyday-jaxb` are present on the classpath, Jollyday will fail to work and throw an `IllegalStateException` at runtime.

This behaviour is problematic and should be avoided. To address this, we aim to introduce a configurable option, `configuration.service.impl`, which can be overridden in the `jollyday.properties` file. The specified `ConfigurationService` will then be used as the default.
By default, this should be set to `de.focus_shift.jollyday.jackson.JacksonConfigurationService`.


Pull request:
Add 'configuration.service.imp' to define which 'ConfigurationService…
…s' implementation should be used if multiple on classpath...

... the default if both are on the classpath is the one from jackson

closes #694 

<!--

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/README.md b/README.md
index 7eb6e4c8..5c63611d 100644
--- a/README.md
+++ b/README.md
@@ -157,6 +157,21 @@ If you already use one of these libraries in your project than just use the spec
   The configuration property name starts with `parser.impl` and finishes with the XML class name.
   The value is the parser implementation class name which implements the `HolidayParser` interface. 
 
+
+  **Configuration Service implementation**  
+  A configuration service implementation is used to define which xml unmarshalling implementation should be used
+
+  ```properties
+  configuration.service.impl = de.focus_shift.jollyday.jackson.JacksonConfigurationService
+  ```
+
+  The configuration property `configuration.service.impl` contains the class name as string.
+  The value is the configuration service implementation class name which implements the `ConfigurationService` interface.
+
+  Values are:
+  * `de.focus_shift.jollyday.jackson.JacksonConfigurationService` (default)
+  * `de.focus_shift.jollyday.jaxb.JaxbConfigurationService`
+
 </details>
 
 ### Examples
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 73b7ffb7..58f8ced7 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
@@ -114,7 +114,8 @@ public abstract class HolidayManager {
     CONFIGURATION_MANAGER_PROVIDER.mergeConfigurationProperties(parameter);
 
     final String managerImplClassName = readManagerImplClassName(parameter);
-    final HolidayManagerValueHandler holidayManagerValueHandler = new HolidayManagerValueHandler(parameter, managerImplClassName, configurationServiceManager);
+    final String configurationServiceImplClassName = readConfigurationServiceImplClassName(parameter);
+    final HolidayManagerValueHandler holidayManagerValueHandler = new HolidayManagerValueHandler(parameter, managerImplClassName, configurationServiceImplClassName, configurationServiceManager);
     if (isManagerCachingEnabled()) {
       return HOLIDAY_MANAGER_CACHE.get(holidayManagerValueHandler);
     } else {
@@ -136,6 +137,20 @@ public abstract class HolidayManager {
     return className;
   }
 
+  /**
+   * Reads the managers implementation class from the properties config file.
+   *
+   * @param parameter the parameter to read the manager implementation class from
+   * @return the manager implementation class name
+   */
+  private static String readConfigurationServiceImplClassName(final ManagerParameter parameter) {
+    final String className = parameter.getConfigurationServiceImplClassName();
+    if (className == null) {
+      throw new IllegalStateException("Missing configuration '" + ManagerParameter.CONFIGURATION_SERVICE_IMPL + "'. Cannot create configuration service.");
+    }
+    return className;
+  }
+
   /**
    * If true, instantiated managers will be cached. If false every call to
    * getInstance will create new manager. True by default.
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManagerValueHandler.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManagerValueHandler.java
index f12237ab..07fcea30 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManagerValueHandler.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManagerValueHandler.java
@@ -12,11 +12,16 @@ class HolidayManagerValueHandler implements Cache.ValueHandler<HolidayManager> {
 
   private final ManagerParameter parameter;
   private final String managerImplClassName;
+  private final String configurationServiceImplClassName;
   private final ConfigurationServiceManager configurationServiceManager;
 
-  HolidayManagerValueHandler(final ManagerParameter parameter, final String managerImplClassName, final ConfigurationServiceManager configurationServiceManager) {
+  HolidayManagerValueHandler(final ManagerParameter parameter,
+                             final String managerImplClassName,
+                             final String configurationServiceImplClassName,
+                             final ConfigurationServiceManager configurationServiceManager) {
     this.parameter = parameter;
     this.managerImplClassName = managerImplClassName;
+    this.configurationServiceImplClassName = configurationServiceImplClassName;
     this.configurationServiceManager = configurationServiceManager;
   }
 
@@ -29,7 +34,7 @@ class HolidayManagerValueHandler implements Cache.ValueHandler<HolidayManager> {
   public HolidayManager createValue() {
     final HolidayManager manager = instantiateManagerImpl(managerImplClassName);
 
-    final ConfigurationService configurationService = configurationServiceManager.getConfigurationService();
+    final ConfigurationService configurationService = configurationServiceManager.getConfigurationService(configurationServiceImplClassName);
     manager.setConfigurationService(configurationService);
 
     manager.init(parameter);
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/ManagerParameter.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/ManagerParameter.java
index a5dc3274..43de1a37 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/ManagerParameter.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/ManagerParameter.java
@@ -7,6 +7,8 @@ public interface ManagerParameter {
 
   String MANAGER_IMPL_CLASS_PREFIX = "manager.impl";
 
+  String CONFIGURATION_SERVICE_IMPL = "configuration.service.impl";
+
   void mergeProperties(final Properties properties);
 
   String getProperty(final String key);
@@ -15,6 +17,8 @@ public interface ManagerParameter {
 
   String getManagerImplClassName();
 
+  String getConfigurationServiceImplClassName();
+
   String createCacheKey();
 
   String getDisplayName();
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManager.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManager.java
index 9b233e68..60af2e09 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManager.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManager.java
@@ -18,21 +18,25 @@ public class ConfigurationServiceManager {
     this.configurationServiceCache = configurationServiceCache;
   }
 
-  public ConfigurationService getConfigurationService() {
-    return instantiateDataSource();
+  public ConfigurationService getConfigurationService(final String configurationServiceImplClassName) {
+    return instantiateDataSource(configurationServiceImplClassName);
   }
 
-  private ConfigurationService instantiateDataSource() {
+  private ConfigurationService instantiateDataSource(final String configurationServiceImplClassName) {
 
     final List<ConfigurationService> services = configurationServiceCache.getServices();
 
-    if (services.size() > 1) {
-      throw new IllegalStateException("Cannot instantiate datasource instance because there are two or more implementations available " + services);
+    if (services.size() == 1) {
+      return services.get(0);
     }
-    if (services.isEmpty()) {
-      throw new IllegalStateException("Cannot instantiate datasource instance because there is no implementations");
+
+    if (services.size() > 1) {
+      return services.stream()
+        .filter(configurationService -> configurationServiceImplClassName.equals(configurationService.getClass().getName()))
+        .findFirst()
+        .orElseThrow(() -> new IllegalStateException("Cannot instantiate datasource instance because there are two or more implementations available " + configurationServiceImplClassName));
     }
 
-    return services.get(0);
+    throw new IllegalStateException("Cannot instantiate datasource instance because there is no implementations");
   }
 }
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/parameter/BaseManagerParameter.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/parameter/BaseManagerParameter.java
index 0b187320..32c7c89b 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/parameter/BaseManagerParameter.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/parameter/BaseManagerParameter.java
@@ -38,4 +38,9 @@ public abstract class BaseManagerParameter implements ManagerParameter {
   public String getManagerImplClassName() {
     return getProperty(MANAGER_IMPL_CLASS_PREFIX);
   }
+
+  @Override
+  public String getConfigurationServiceImplClassName() {
+    return getProperty(CONFIGURATION_SERVICE_IMPL);
+  }
 }
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/spi/ConfigurationService.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/spi/ConfigurationService.java
index bc3b6028..cb43f482 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/spi/ConfigurationService.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/spi/ConfigurationService.java
@@ -6,5 +6,6 @@ import de.focus_shift.jollyday.core.ManagerParameter;
  * Service to provide the serialised configuration from XML files.
  */
 public interface ConfigurationService {
+
   Configuration getConfiguration(ManagerParameter parameter);
 }
diff --git a/jollyday-core/src/main/resources/jollyday.properties b/jollyday-core/src/main/resources/jollyday.properties
index 86623dd2..01a9758e 100644
--- a/jollyday-core/src/main/resources/jollyday.properties
+++ b/jollyday-core/src/main/resources/jollyday.properties
@@ -16,3 +16,5 @@ parser.impl.de.focus_shift.jollyday.core.spi.FixedWeekdayBetweenFixed    = de.fo
 parser.impl.de.focus_shift.jollyday.core.spi.FixedWeekdayRelativeToFixed = de.focus_shift.jollyday.core.parser.impl.FixedWeekdayRelativeToFixedParser
 parser.impl.de.focus_shift.jollyday.core.spi.EthiopianOrthodoxHoliday    = de.focus_shift.jollyday.core.parser.impl.EthiopianOrthodoxHolidayParser
 parser.impl.de.focus_shift.jollyday.core.spi.RelativeToEasterSunday      = de.focus_shift.jollyday.core.parser.impl.RelativeToEasterSundayParser
+
+configuration.service.impl                                               = de.focus_shift.jollyday.jackson.JacksonConfigurationService


Test patch:
diff --git a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/HolidayManagerValueHandlerTest.java b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/HolidayManagerValueHandlerTest.java
index 0030650c..787959c7 100644
--- a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/HolidayManagerValueHandlerTest.java
+++ b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/HolidayManagerValueHandlerTest.java
@@ -27,8 +27,9 @@ class HolidayManagerValueHandlerTest {
   @Test
   void ensureToGetCorrectKey() {
     final String managerImplClassName = "de.focus_shift.jollyday.core.impl.DefaultHolidayManager";
+    final String configurationServiceImplClassName = "de.focus_shift.jollyday.jackson.JacksonConfigurationService";
     final ConfigurationServiceManager configurationServiceManager = mock(ConfigurationServiceManager.class);
-    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), managerImplClassName, configurationServiceManager);
+    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), managerImplClassName, configurationServiceImplClassName, configurationServiceManager);
 
     final String key = sut.getKey();
     assertThat(key).isEqualTo("de");
@@ -49,10 +50,10 @@ class HolidayManagerValueHandlerTest {
     final ConfigurationService configurationService = mock(ConfigurationService.class);
     final Configuration configuration = mock(Configuration.class);
 
-    when(configurationServiceManager.getConfigurationService()).thenReturn(configurationService);
+    when(configurationServiceManager.getConfigurationService(any(String.class))).thenReturn(configurationService);
     when(configurationService.getConfiguration(any(ManagerParameter.class))).thenReturn(configuration);
 
-    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), managerImplClass, configurationServiceManager);
+    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), managerImplClass, "configurationServiceImplClassName", configurationServiceManager);
 
     final HolidayManager holidayManager = sut.createValue();
     assertThat(holidayManager).isInstanceOf(clazz);
@@ -62,7 +63,7 @@ class HolidayManagerValueHandlerTest {
   void ensureToThrowIllegalStateExceptionIfHolidayManagerIsWrong() {
 
     final String wrongManagerImplClassName = "de.focus_shift.jollyday.core.impl.WrongHolidayManager";
-    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), wrongManagerImplClassName, mock(ConfigurationServiceManager.class));
+    final HolidayManagerValueHandler sut = new HolidayManagerValueHandler(create("de"), wrongManagerImplClassName, "configurationServiceImplClassName", mock(ConfigurationServiceManager.class));
 
     assertThatThrownBy(sut::createValue)
       .isInstanceOf(IllegalStateException.class)
diff --git a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManagerTest.java b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManagerTest.java
index 46d96224..9ae48602 100644
--- a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManagerTest.java
+++ b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/datasource/ConfigurationServiceManagerTest.java
@@ -36,7 +36,7 @@ class ConfigurationServiceManagerTest {
 
     when(configurationServiceCache.getServices()).thenReturn(List.of());
 
-    final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> sut.getConfigurationService());
+    final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> sut.getConfigurationService("configurationServiceImplClassName"));
     assertThat(exception.getMessage()).contains("Cannot instantiate datasource instance because there is no implementations");
   }
 
@@ -44,19 +44,29 @@ class ConfigurationServiceManagerTest {
   void ensuresToProvideConfigurationServiceIfExactlyOneIsAvailable() {
     when(configurationServiceCache.getServices()).thenReturn(List.of(new MockConfigurationService()));
 
-    final ConfigurationService configurationService = sut.getConfigurationService();
+    final ConfigurationService configurationService = sut.getConfigurationService("configurationServiceImplClassName");
     assertThat(configurationService).isInstanceOf(MockConfigurationService.class);
   }
 
   @Test
-  void ensuresToThrowExceptionIfMultipleImplementationsAreAvailable() {
+  void ensuresToThrowExceptionIfMultipleImplementationsAreAvailableButNoneOfThatAreConfigured() {
 
     when(configurationServiceCache.getServices()).thenReturn(List.of(new MockConfigurationService(), new MockConfigurationService()));
 
-    final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> sut.getConfigurationService());
+    final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> sut.getConfigurationService("configurationServiceImplClassName"));
     assertThat(exception.getMessage()).contains("Cannot instantiate datasource instance because there are two or more implementations available");
   }
 
+  @Test
+  void ensuresToUseTheConfiguredConfigurationServiceIfMultipleAreOnTheClasspath() {
+
+    final MockConfigurationServiceOther mockConfigurationServiceOther = new MockConfigurationServiceOther();
+    when(configurationServiceCache.getServices()).thenReturn(List.of(new MockConfigurationService(), mockConfigurationServiceOther));
+
+    final ConfigurationService configurationService = sut.getConfigurationService(mockConfigurationServiceOther.getClass().getName());
+    assertThat(configurationService).isInstanceOf(MockConfigurationServiceOther.class);
+  }
+
   private static class MockConfigurationService implements ConfigurationService {
 
     @Override
@@ -84,4 +94,32 @@ class ConfigurationServiceManagerTest {
       };
     }
   }
+
+  private static class MockConfigurationServiceOther implements ConfigurationService {
+
+    @Override
+    public Configuration getConfiguration(ManagerParameter parameter) {
+      return new Configuration() {
+        @Override
+        public Holidays holidays() {
+          return null;
+        }
+
+        @Override
+        public Stream<Configuration> subConfigurations() {
+          return null;
+        }
+
+        @Override
+        public String hierarchy() {
+          return null;
+        }
+
+        @Override
+        public String description() {
+          return null;
+        }
+      };
+    }
+  }
 }