Bugfix: AbstractPlugin supports all features

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 See [Release](https://github.com/itsallcode/white-rabbit/releases/tag/v1.7.0) / [Milestone](https://github.com/itsallcode/white-rabbit/milestone/9?closed=1)
 
+### Fixed
+
+* [#191](https://github.com/itsallcode/white-rabbit/pull/191): Starting WR with all plugins enabled failed with exception `IllegalStateException: Found multiple plugins supporting org.itsallcode.whiterabbit.api.features.MonthDataStorage`.
+
 ### Added
 
 * [#158](https://github.com/itsallcode/white-rabbit/issues/158): PMSmart plugin: Support optional configuration  `pmsmart.transfer.comments` to skip transfer of comments.

api/src/main/java/org/itsallcode/whiterabbit/api/AbstractPlugin.java

@@ -35,17 +35,20 @@ public void close()
     @Override
     public boolean supports(Class<? extends PluginFeature> featureType)
     {
-        return featureType.isAssignableFrom(featureType);
+        return this.featureType.isAssignableFrom(featureType);
     }
 
     protected abstract S createInstance();
 
     @Override
     public <T extends PluginFeature> T getFeature(Class<T> featureType)
     {
-        if (featureType.isAssignableFrom(this.featureType))
+        if (this.supports(featureType))
         {
-            return featureType.cast(createInstance());
+            if (this.supports(featureType))
+            {
+                return featureType.cast(createInstance());
+            }
         }
         throw new IllegalArgumentException("Feature " + featureType.getName() + " not supported by plugin " + getId());
     }

api/src/main/java/org/itsallcode/whiterabbit/api/Plugin.java

@@ -1,5 +1,6 @@
 package org.itsallcode.whiterabbit.api;
 
+import org.itsallcode.whiterabbit.api.features.Holidays;
 import org.itsallcode.whiterabbit.api.features.MonthDataStorage;
 import org.itsallcode.whiterabbit.api.features.PluginFeature;
 import org.itsallcode.whiterabbit.api.features.ProjectReportExporter;
@@ -14,6 +15,7 @@
  * <ul>
  * <li>{@link ProjectReportExporter}</li>
  * <li>{@link MonthDataStorage}</li>
+ * <li>{@link Holidays}</li>
  * </ul>
  */
 public interface Plugin

api/src/test/java/org/itsallcode/whiterabbit/api/AbstractPluginTest.java

@@ -0,0 +1,84 @@
+package org.itsallcode.whiterabbit.api;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.itsallcode.whiterabbit.api.features.PluginFeature;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class AbstractPluginTest
+{
+    private static final String PLUGIN_ID = "testing-plugin";
+
+    @Mock
+    SupportedFeature supportedFeatureMock;
+
+    private TestingAbstractPlugin plugin;
+
+    @BeforeEach
+    void setUp()
+    {
+        plugin = new TestingAbstractPlugin();
+    }
+
+    @Test
+    void supportsReturnsTrueForSupportedFeature()
+    {
+        assertThat(plugin.supports(SupportedFeature.class)).isTrue();
+    }
+
+    @Test
+    void supportsReturnsFalseForUnsupportedFeature()
+    {
+        assertThat(plugin.supports(UnsupportedFeature.class)).isFalse();
+    }
+
+    @Test
+    void gettingUnsupportedFeatureThrowsException()
+    {
+        assertThatThrownBy(() -> plugin.getFeature(UnsupportedFeature.class))
+                .isInstanceOf(IllegalArgumentException.class).hasMessage(
+                        "Feature " + UnsupportedFeature.class.getName() + " not supported by plugin " + PLUGIN_ID);
+    }
+
+    @Test
+    void gettingSupportedFeatureWorks()
+    {
+        assertThat(plugin.getFeature(SupportedFeature.class)).isSameAs(supportedFeatureMock);
+    }
+
+    @Test
+    void getPluginId()
+    {
+        assertThat(plugin.getId()).isEqualTo(PLUGIN_ID);
+    }
+
+    private interface SupportedFeature extends PluginFeature
+    {
+
+    }
+
+    private interface UnsupportedFeature extends PluginFeature
+    {
+
+    }
+
+    private class TestingAbstractPlugin extends AbstractPlugin<SupportedFeature>
+    {
+        protected TestingAbstractPlugin()
+        {
+            super(PLUGIN_ID, SupportedFeature.class);
+        }
+
+        @Override
+        protected SupportedFeature createInstance()
+        {
+            return supportedFeatureMock;
+        }
+    }
+}

plugins/csv/src/main/java/org/itsallcode/whiterabbit/plugin/csv/CSVConfig.java

@@ -1,33 +1,37 @@
 package org.itsallcode.whiterabbit.plugin.csv;
 
-import org.itsallcode.whiterabbit.api.PluginConfiguration;
-
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
-public class CSVConfig {
+import org.itsallcode.whiterabbit.api.PluginConfiguration;
+
+class CSVConfig
+{
     private final boolean filterForWeekDays;
     private final Path outPath;
     private final String separator;
 
-    public boolean getFilterForWeekDays() {
+    boolean getFilterForWeekDays()
+    {
         return filterForWeekDays;
     }
 
-    public Path getOutPath() {
+    Path getOutPath()
+    {
         return outPath;
     }
 
-    public String getSeparator() {
+    String getSeparator()
+    {
         return separator;
     }
 
-    public CSVConfig(PluginConfiguration config) {
+    CSVConfig(PluginConfiguration config)
+    {
         final String defaultPath = System.getProperty("user.home");
         this.outPath = Paths.get(config.getOptionalValue("destination").orElse(defaultPath));
         this.separator = config.getOptionalValue("separator").orElse(",");
-        this.filterForWeekDays =
-                config.getOptionalValue("filter_for_weekdays")
-                        .orElse("false").equalsIgnoreCase("true");
+        this.filterForWeekDays = config.getOptionalValue("filter_for_weekdays")
+                .orElse("false").equalsIgnoreCase("true");
     }
 }

plugins/csv/src/main/java/org/itsallcode/whiterabbit/plugin/csv/CSVExporterPlugin.java

@@ -1,12 +1,13 @@
 package org.itsallcode.whiterabbit.plugin.csv;
 
 import org.itsallcode.whiterabbit.api.AbstractPlugin;
+import org.itsallcode.whiterabbit.api.features.ProjectReportExporter;
 
-public class CSVExporterPlugin extends AbstractPlugin<CSVProjectReportExporter>
+public class CSVExporterPlugin extends AbstractPlugin<ProjectReportExporter>
 {
     public CSVExporterPlugin()
     {
-        super("csv", CSVProjectReportExporter.class);
+        super("csv", ProjectReportExporter.class);
     }
 
     @Override

plugins/csv/src/main/java/org/itsallcode/whiterabbit/plugin/csv/CSVProjectReportExporter.java

@@ -17,42 +17,49 @@
 import org.itsallcode.whiterabbit.api.model.ProjectReportActivity;
 import org.itsallcode.whiterabbit.api.model.ProjectReportDay;
 
-class CSVProjectReportExporter implements ProjectReportExporter {
+class CSVProjectReportExporter implements ProjectReportExporter
+{
     private final OutStreamProvider outStreamProvider;
     private final CSVConfig csvConfig;
 
-    CSVProjectReportExporter(CSVConfig csvConfig, OutStreamProvider outStreamProvider) {
+    CSVProjectReportExporter(CSVConfig csvConfig, OutStreamProvider outStreamProvider)
+    {
         this.csvConfig = csvConfig;
         this.outStreamProvider = outStreamProvider;
     }
 
     @Override
-    public void export(ProjectReport report, ProgressMonitor progressMonitor) {
+    public void export(ProjectReport report, ProgressMonitor progressMonitor)
+    {
         progressMonitor.setTaskName("Exporting...");
-        if (progressMonitor.isCanceled()) {
+        if (progressMonitor.isCanceled())
+        {
             return;
         }
         final String separator = csvConfig.getSeparator();
 
-        final List<ProjectReportDay> filteredDays =
-                report.getDays().stream()
-                        .filter(Objects::nonNull)
-                        .filter(day -> !csvConfig.getFilterForWeekDays() || day.getType() == DayType.WORK)
-                        .collect(Collectors.toList());
+        final List<ProjectReportDay> filteredDays = report.getDays().stream()
+                .filter(Objects::nonNull)
+                .filter(day -> !csvConfig.getFilterForWeekDays() || day.getType() == DayType.WORK)
+                .collect(Collectors.toList());
 
-        try (final PrintStream os = new PrintStream(outStreamProvider.getStream(report.getMonth().toString()))) {
+        try (final PrintStream os = new PrintStream(outStreamProvider.getStream(report.getMonth().toString())))
+        {
             os.println(MessageFormat.format("Date{0}Project{0}TimePerProject{0}TimePerDay{0}Comment", separator));
 
-            for (final ProjectReportDay day : filteredDays) {
+            for (final ProjectReportDay day : filteredDays)
+            {
                 final String dayComment = formatEmptyString(day.getComment());
                 final Duration dayWorkingTime = day.getProjects() == null ? Duration.ZERO : getDayWorkingTime(day);
                 final String dayDurationStr = formatDuration(dayWorkingTime);
 
                 os.println(MessageFormat.format("{0}{1}{1}{1}{2}{1}{3}",
                         day.getDate(), separator, dayDurationStr, dayComment));
 
-                if (day.getProjects() != null) {
-                    for (final ProjectReportActivity project : day.getProjects()) {
+                if (day.getProjects() != null)
+                {
+                    for (final ProjectReportActivity project : day.getProjects())
+                    {
                         final String projectWorkingTime = formatDuration(project.getWorkingTime());
                         final String projectLabel = formatEmptyString(project.getProject().getLabel());
                         final String activityComment = formatEmptyString(project.getComment());
@@ -61,22 +68,27 @@ public void export(ProjectReport report, ProgressMonitor progressMonitor) {
                     }
                 }
             }
-        } catch (IOException ex) {
+        }
+        catch (final IOException ex)
+        {
             throw new UncheckedIOException("Error exporting report to CSV:" + ex, ex);
         }
     }
 
-    private Duration getDayWorkingTime(ProjectReportDay day) {
+    private Duration getDayWorkingTime(ProjectReportDay day)
+    {
         return day.getProjects().stream().reduce(Duration.ZERO,
-                (subtotal, element) -> element == null ?
-                        subtotal : element.getWorkingTime().plus(subtotal), Duration::plus);
+                (subtotal, element) -> element == null ? subtotal : element.getWorkingTime().plus(subtotal),
+                Duration::plus);
     }
 
-    private String formatEmptyString(String value) {
+    private String formatEmptyString(String value)
+    {
         return value == null ? "" : value;
     }
 
-    private String formatDuration(Duration duration) {
+    private String formatDuration(Duration duration)
+    {
         final long minutes = TimeUnit.SECONDS.toMinutes(duration.getSeconds());
         final long absMinutes = Math.abs(minutes);
         final String positive = String.format("%02d:%02d", absMinutes / 60, absMinutes % 60);

plugins/csv/src/main/java/org/itsallcode/whiterabbit/plugin/csv/DirectoryStreamProvider.java

@@ -5,17 +5,25 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 
-public class DirectoryStreamProvider implements OutStreamProvider {
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
+class DirectoryStreamProvider implements OutStreamProvider
+{
+    private static final Logger LOG = LogManager.getLogger(DirectoryStreamProvider.class);
     private final Path outPath;
 
-    DirectoryStreamProvider(Path outPath) {
-        this.outPath= outPath;
+    DirectoryStreamProvider(Path outPath)
+    {
+        this.outPath = outPath;
     }
 
     @Override
-    public OutputStream getStream(String name) throws IOException {
+    public OutputStream getStream(String name) throws IOException
+    {
         final String outFile = String.format("%s_working_time.csv", name);
-        return Files.newOutputStream(outPath.resolve(outFile));
+        final Path path = outPath.resolve(outFile);
+        LOG.info("Writing output to {}", path);
+        return Files.newOutputStream(path);
     }
 }

plugins/csv/src/main/java/org/itsallcode/whiterabbit/plugin/csv/OutStreamProvider.java

@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.io.OutputStream;
 
-public interface OutStreamProvider {
+interface OutStreamProvider
+{
     OutputStream getStream(String name) throws IOException;
 }

plugins/csv/src/test/java/org/itsallcode/whiterabbit/plugin/csv/CSVExporterPluginTest.java

@@ -0,0 +1,46 @@
+package org.itsallcode.whiterabbit.plugin.csv;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.itsallcode.whiterabbit.api.PluginConfiguration;
+import org.itsallcode.whiterabbit.api.features.Holidays;
+import org.itsallcode.whiterabbit.api.features.ProjectReportExporter;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class CSVExporterPluginTest
+{
+    @Mock
+    private PluginConfiguration configMock;
+
+    private CSVExporterPlugin plugin;
+
+    @BeforeEach
+    void setUp()
+    {
+        plugin = new CSVExporterPlugin();
+    }
+
+    @Test
+    void pluginSupportsProjectReportExporterFeature()
+    {
+        assertThat(plugin.supports(ProjectReportExporter.class)).isTrue();
+    }
+
+    @Test
+    void pluginDoesNotSupportHolidaysFeature()
+    {
+        assertThat(plugin.supports(Holidays.class)).isFalse();
+    }
+
+    @Test
+    void getFeature()
+    {
+        plugin.init(configMock);
+        assertThat(plugin.getFeature(ProjectReportExporter.class)).isNotNull();
+    }
+}

plugins/holiday-calculator/src/main/java/org/itsallcode/whiterabbit/plugin/holidaycalculator/HolidayCalculatorPlugin.java

@@ -1,12 +1,13 @@
 package org.itsallcode.whiterabbit.plugin.holidaycalculator;
 
 import org.itsallcode.whiterabbit.api.AbstractPlugin;
+import org.itsallcode.whiterabbit.api.features.Holidays;
 
-public class HolidayCalculatorPlugin extends AbstractPlugin<CalculatedHolidays>
+public class HolidayCalculatorPlugin extends AbstractPlugin<Holidays>
 {
     public HolidayCalculatorPlugin()
     {
-        super("holidaycalculator", CalculatedHolidays.class);
+        super("holidaycalculator", Holidays.class);
     }
 
     @Override