diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoader.java index 9fec635d..a22589e4 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoader.java @@ -88,7 +88,13 @@ public boolean supportsFile(final InputFile file) private List getMatchingFactories(final InputFile file) { - return this.serviceLoader.load() + final List allFactories = this.serviceLoader.load().toList(); + if (allFactories.isEmpty()) + { + throw new ImporterException("No importers discovered. If you are running OpenFastTrace as a library, " + + "ensure that the thread context classloader has access to the importer implementations."); + } + return allFactories.stream() .filter(f -> f.supportsFile(file)) .toList(); } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java new file mode 100644 index 00000000..03328b02 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java @@ -0,0 +1,66 @@ +package org.itsallcode.openfasttrace.core.importer; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.nio.file.Paths; + +import org.itsallcode.openfasttrace.api.importer.ImporterContext; +import org.itsallcode.openfasttrace.api.importer.ImporterException; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +/** + * Test to reproduce the classloader mismatch problem when discovering + * importers. + *

+ * This scenario will happen if OFT is used as a library with a custom classloader. + * What we want in that case is a very clear exception message because without + * an importer, neither tracing nor converting have any chance to work. + *

+ */ +class ImporterFactoryLoaderClassloaderIT +{ + /** + * This test demonstrates that if the Thread Context ClassLoader (TCCL) + * does not have access to the importer factories, {@link ImporterFactoryLoader} + * will fail to find any importers and should throw an exception. + */ + @Test + final void testGetImporterFactoryThrowsExceptionWhenTcclIsLimited() + { + final ClassLoader originalTccl = Thread.currentThread().getContextClassLoader(); + final ClassLoader limitedClassLoader = new NoOpClassLoader(); + + try + { + Thread.currentThread().setContextClassLoader(limitedClassLoader); + final ImporterContext context = Mockito.mock(ImporterContext.class); + final ImporterFactoryLoader loaderWithLimitedTccl = new ImporterFactoryLoader(context); + final InputFile file = RealFileInput.forPath(Paths.get("test.md")); + final ImporterException exception = assertThrows(ImporterException.class, () -> + loaderWithLimitedTccl.getImporterFactory(file) + ); + assertThat(exception.getMessage().contains("No importers discovered"), is(true)); + } + finally + { + Thread.currentThread().setContextClassLoader(originalTccl); + } + } + + /** + * A class loader that finds nothing. + */ + private static class NoOpClassLoader extends ClassLoader + { + @Override + public Class loadClass(final String name) throws ClassNotFoundException + { + throw new ClassNotFoundException(name); + } + } +} diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/importer/TestImporterFactoryLoader.java b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/TestImporterFactoryLoader.java index 4b2b6e0b..ac4fb741 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/importer/TestImporterFactoryLoader.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/TestImporterFactoryLoader.java @@ -2,6 +2,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.lenient; @@ -10,7 +11,7 @@ import java.nio.file.Paths; import java.util.Arrays; -import org.itsallcode.openfasttrace.api.importer.ImporterContext; +import org.itsallcode.openfasttrace.api.importer.ImporterException; import org.itsallcode.openfasttrace.api.importer.ImporterFactory; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; @@ -35,8 +36,6 @@ class TestImporterFactoryLoader private ImporterFactory supportedFactory2; @Mock private ImporterFactory unsupportedFactory; - @Mock - private ImporterContext contextMock; private ImporterFactoryLoader loader; private InputFile file; @@ -53,10 +52,17 @@ void beforeEach() } @Test - void testNoFactoryRegisteredReturnsNoImporter() + void testNoFactoryRegisteredThrowsException() { simulateFactories(); - assertTrue(this.loader.getImporterFactory(this.file).isEmpty()); + assertThrows(ImporterException.class, () -> this.loader.getImporterFactory(this.file)); + } + + @Test + void testSupportsFileWhenNoFactoryRegisteredThrowsException() + { + simulateFactories(); + assertThrows(ImporterException.class, () -> this.loader.supportsFile(this.file)); } @Test @@ -82,7 +88,9 @@ void testMultipleMatchingFactoriesFoundReturnNoImporter() private void assertFactoryFound(final ImporterFactory expectedFactory) { - assertThat(this.loader.getImporterFactory(this.file).get(), sameInstance(expectedFactory)); + assertThat(this.loader.getImporterFactory(this.file).orElseThrow(() -> + new AssertionError("Unable to find ImporterFactory.") + ), sameInstance(expectedFactory)); } private void simulateFactories(final ImporterFactory... factories) diff --git a/doc/changes/changes_4.5.0.md b/doc/changes/changes_4.5.0.md index 7b2894f5..9d81f56d 100644 --- a/doc/changes/changes_4.5.0.md +++ b/doc/changes/changes_4.5.0.md @@ -10,6 +10,9 @@ We also refactored the tests around the CLI starter to improve readability and m We added FXML to the list of supported file formats for the tag importer. +When no importer factory is found for a given file format, the importer factory loader will now throw an exception that explains the likely cause. This is helpful in case OFT is used as a library with a custom classloader and thus does not have the right context. + ## Features -* #503: Added `-h` / `--help` to the command line. \ No newline at end of file +* #503: Added `-h` / `--help` to the command line. +* #506: Exception when no importer was found. \ No newline at end of file