From 064418d754e1e1581addd3ba75382825112ead48 Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Fri, 29 May 2026 21:26:35 +0200 Subject: [PATCH 1/4] #506: Warn clearly when importer discovery fails. --- .../core/importer/ImporterFactoryLoader.java | 8 +++++++- .../core/importer/TestImporterFactoryLoader.java | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) 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..1f06a57a 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()) + { + LOG.warning("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/TestImporterFactoryLoader.java b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/TestImporterFactoryLoader.java index 4b2b6e0b..9173ea3b 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 @@ -1,6 +1,7 @@ package org.itsallcode.openfasttrace.core.importer; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.same; @@ -35,8 +36,6 @@ class TestImporterFactoryLoader private ImporterFactory supportedFactory2; @Mock private ImporterFactory unsupportedFactory; - @Mock - private ImporterContext contextMock; private ImporterFactoryLoader loader; private InputFile file; @@ -59,6 +58,13 @@ void testNoFactoryRegisteredReturnsNoImporter() assertTrue(this.loader.getImporterFactory(this.file).isEmpty()); } + @Test + void testSupportsFileWhenNoFactoryRegisteredReturnsFalse() + { + simulateFactories(); + assertThat(this.loader.supportsFile(this.file), is(false)); + } + @Test void testMatchingFactoryFoundOnlyOneAvailable() { From e0d123c7e1d4d55dcc352bbf29e9aaf0f9afc8de Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Fri, 29 May 2026 21:29:44 +0200 Subject: [PATCH 2/4] #506: Added changelog entry. --- doc/changes/changes_4.5.0.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/changes/changes_4.5.0.md b/doc/changes/changes_4.5.0.md index 7b2894f5..b6511811 100644 --- a/doc/changes/changes_4.5.0.md +++ b/doc/changes/changes_4.5.0.md @@ -10,6 +10,8 @@ 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 log a warning 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 From e56a6b9ecc6816519916e429e432ec5de447c1f7 Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Sat, 30 May 2026 07:59:49 +0200 Subject: [PATCH 3/4] #506: Changed from warning to Exception, since without importer tracing and converting will fail. --- .../core/importer/ImporterFactoryLoader.java | 2 +- .../ImporterFactoryLoaderClassloaderIT.java | 66 +++++++++++++++++++ .../importer/TestImporterFactoryLoader.java | 16 +++-- doc/changes/changes_4.5.0.md | 5 +- 4 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java 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 1f06a57a..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 @@ -91,7 +91,7 @@ private List getMatchingFactories(final InputFile file) final List allFactories = this.serviceLoader.load().toList(); if (allFactories.isEmpty()) { - LOG.warning("No importers discovered. If you are running OpenFastTrace as a library, " + 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() 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..4b1749ac --- /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 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 9173ea3b..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 @@ -1,8 +1,8 @@ package org.itsallcode.openfasttrace.core.importer; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; 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; @@ -11,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; @@ -52,17 +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 testSupportsFileWhenNoFactoryRegisteredReturnsFalse() + void testSupportsFileWhenNoFactoryRegisteredThrowsException() { simulateFactories(); - assertThat(this.loader.supportsFile(this.file), is(false)); + assertThrows(ImporterException.class, () -> this.loader.supportsFile(this.file)); } @Test @@ -88,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 b6511811..9d81f56d 100644 --- a/doc/changes/changes_4.5.0.md +++ b/doc/changes/changes_4.5.0.md @@ -10,8 +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 log a warning 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. +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 From 5549a3a1b4b6f1538806227e782f335db9ca0ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Sat, 30 May 2026 13:05:02 +0200 Subject: [PATCH 4/4] Update core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java --- .../core/importer/ImporterFactoryLoaderClassloaderIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 4b1749ac..03328b02 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/ImporterFactoryLoaderClassloaderIT.java @@ -17,7 +17,7 @@ * Test to reproduce the classloader mismatch problem when discovering * importers. *

- * This scenario will happen if OFT is used a library with a custom classloader. + * 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. *