From ccc3a827b11bd876916e649fe410ac7624e10735 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 12:50:04 +0200 Subject: [PATCH 01/44] Update developer guide to run build in parallel --- doc/developer_guide.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/doc/developer_guide.md b/doc/developer_guide.md index d5af455c..bb292e1c 100644 --- a/doc/developer_guide.md +++ b/doc/developer_guide.md @@ -107,19 +107,19 @@ git clone https://github.com/itsallcode/openfasttrace.git Run unit tests: ```sh -mvn test +mvn -T 1C test ``` Run unit and integration tests and additional checks: ```sh -mvn verify +mvn -T 1C verify ``` Build OFT: ```sh -mvn package -DskipTests +mvn -T 1C package -DskipTests ``` This will build the executable JAR including all modules at `product/target/openfasttrace-$VERSION.jar`. @@ -172,15 +172,13 @@ mvn --update-snapshots versions:display-dependency-updates versions:display-plug Automatically upgrade dependencies: ```bash -mvn --update-snapshots versions:use-latest-releases versions:update-properties +mvn -T 1C --update-snapshots versions:use-latest-releases versions:update-properties ``` ## Run local sonar analysis ```bash -mvn clean org.jacoco:jacoco-maven-plugin:prepare-agent package sonar:sonar \ - -Dsonar.host.url=https://sonarcloud.io \ - -Dsonar.organization=itsallcode \ +mvn -T 1C clean org.jacoco:jacoco-maven-plugin:prepare-agent package sonar:sonar \ -Dsonar.login=[token] ``` From bfcb92345e9703cf66963b97dfcba8e9b936f5bc Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 12:52:09 +0200 Subject: [PATCH 02/44] Update Maven Surefire version to 3.3.0 --- parent/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parent/pom.xml b/parent/pom.xml index e0ed2d8f..85e996d0 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -11,7 +11,7 @@ 4.0.0 17 5.11.0-M2 - 3.2.5 + 3.3.0 UTF-8 UTF-8 ${git.commit.time} From 887a5cab7f4928dc3247c456b4db20abee5ff0a9 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 12:53:12 +0200 Subject: [PATCH 03/44] #413: Implement new plugin service loader --- .../serviceloader/ClassPathServiceLoader.java | 26 +++++ .../core/serviceloader/DelegatingLoader.java | 20 ++++ .../InitializingServiceLoader.java | 32 +++--- .../core/serviceloader/Loader.java | 8 ++ .../serviceloader/PluginLoaderFactory.java | 106 ++++++++++++++++++ .../core/serviceloader/ServiceOrigin.java | 62 ++++++++++ 6 files changed, 241 insertions(+), 13 deletions(-) create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java new file mode 100644 index 00000000..943abfc0 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -0,0 +1,26 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import java.util.ServiceLoader; +import java.util.ServiceLoader.Provider; +import java.util.stream.Stream; + +final class ClassPathServiceLoader implements Loader +{ + private final ServiceLoader serviceLoader; + + private ClassPathServiceLoader(final ServiceLoader serviceLoader) + { + this.serviceLoader = serviceLoader; + } + + static Loader load(final Class serviceType, final ServiceOrigin serviceOrigin) + { + return new ClassPathServiceLoader<>(ServiceLoader.load(serviceType, serviceOrigin.getClassLoader())); + } + + @Override + public Stream load() + { + return this.serviceLoader.stream().map(Provider::get); + } +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java new file mode 100644 index 00000000..ab6e0f16 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java @@ -0,0 +1,20 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import java.util.List; +import java.util.stream.Stream; + +class DelegatingLoader implements Loader +{ + private final List> delegates; + + DelegatingLoader(final List> delegates) + { + this.delegates = delegates; + } + + @Override + public Stream load() + { + return delegates.stream().flatMap(Loader::load); + } +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java index bf31f01e..30af7205 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java @@ -1,5 +1,7 @@ package org.itsallcode.openfasttrace.core.serviceloader; + import java.util.*; +import java.util.stream.Stream; import org.itsallcode.openfasttrace.api.core.serviceloader.Initializable; @@ -12,15 +14,15 @@ * @param * the context type */ -public class InitializingServiceLoader, C> implements Iterable +public final class InitializingServiceLoader, C> implements Iterable, Loader { - private final ServiceLoader serviceLoader; + private final Loader delegate; private final C context; private List services; - private InitializingServiceLoader(final ServiceLoader serviceLoader, final C context) + private InitializingServiceLoader(final Loader delegate, final C context) { - this.serviceLoader = serviceLoader; + this.delegate = delegate; this.context = context; } @@ -41,27 +43,31 @@ private InitializingServiceLoader(final ServiceLoader serviceLoader, final C public static , C> InitializingServiceLoader load( final Class serviceType, final C context) { - return new InitializingServiceLoader<>(ServiceLoader.load(serviceType), context); + final PluginLoaderFactory loaderFactory = PluginLoaderFactory.createDefault(); + return new InitializingServiceLoader<>(loaderFactory.createLoader(serviceType), context); } @Override - public Iterator iterator() + public Stream load() { if (this.services == null) { this.services = loadServices(); } - return this.services.iterator(); + return services.stream(); } private List loadServices() { - final List initializedServices = new ArrayList<>(); - for (final T service : this.serviceLoader) - { + return this.delegate.load().map(service -> { service.init(this.context); - initializedServices.add(service); - } - return initializedServices; + return service; + }).toList(); + } + + @Override + public Iterator iterator() + { + return load().iterator(); } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java new file mode 100644 index 00000000..45d0940a --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java @@ -0,0 +1,8 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import java.util.stream.Stream; + +interface Loader +{ + Stream load(); +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java new file mode 100644 index 00000000..27ef6f96 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java @@ -0,0 +1,106 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.*; + +class PluginLoaderFactory +{ + private final Path pluginsDirectory; + + PluginLoaderFactory(final Path pluginsDirectory) + { + this.pluginsDirectory = pluginsDirectory; + } + + static PluginLoaderFactory createDefault() + { + return new PluginLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins")); + } + + private static Path getHomeDirectory() + { + return Path.of(System.getProperty("user.home")); + } + + Loader createLoader(final Class serviceType) + { + return createLoader(serviceType, findServiceOrigins()); + } + + private Loader createLoader(final Class serviceType, final List origins) + { + final List> loaders = origins.stream().map(origin -> ClassPathServiceLoader.load(serviceType, origin)) + .toList(); + return new DelegatingLoader<>(loaders); + } + + private List findServiceOrigins() + { + final List origins = new ArrayList<>(); + origins.add(ServiceOrigin.forCurrentClassPath()); + origins.addAll(findPluginOrigins()); + return origins; + } + + Collection findPluginOrigins() + { + if (!Files.isDirectory(pluginsDirectory)) + { + return Collections.emptyList(); + } + + try + { + return Files.list(pluginsDirectory) + .map(this::originForPath) + .flatMap(Optional::stream) + .toList(); + } + catch (final IOException exception) + { + throw new UncheckedIOException( + "Failed to list plugin directories in '" + this.pluginsDirectory + "': " + exception.getMessage(), + exception); + } + } + + private Optional originForPath(final Path path) + { + if (isJarFile(path)) + { + return Optional.of(ServiceOrigin.forJars(List.of(path))); + } + if (!Files.isDirectory(path)) + { + return Optional.empty(); + } + final List jars = findJarsInDir(path); + if (jars.isEmpty()) + { + return Optional.empty(); + } + return Optional.of(ServiceOrigin.forJars(jars)); + } + + private static List findJarsInDir(final Path path) + { + try + { + return Files.list(path).filter(PluginLoaderFactory::isJarFile).toList(); + } + catch (final IOException exception) + { + throw new UncheckedIOException( + "Failed to list files in plugin directory '" + path + "': " + exception.getMessage(), + exception); + } + } + + private static boolean isJarFile(final Path path) + { + return path.getFileName().toString().toLowerCase(Locale.ENGLISH).endsWith(".jar"); + } +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java new file mode 100644 index 00000000..b79e8e43 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -0,0 +1,62 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static java.util.stream.Collectors.joining; + +import java.net.*; +import java.nio.file.Path; +import java.util.List; + +final class ServiceOrigin +{ + private final ClassLoader classLoader; + + private ServiceOrigin(final ClassLoader classLoader) + { + this.classLoader = classLoader; + } + + public static ServiceOrigin forCurrentClassPath() + { + return new ServiceOrigin(getBaseClassLoader()); + } + + private static ClassLoader getBaseClassLoader() + { + return Thread.currentThread().getContextClassLoader(); + } + + public static ServiceOrigin forJar(final Path jar) + { + return forJars(List.of(jar)); + } + + public static ServiceOrigin forJars(final List jars) + { + return new ServiceOrigin(createClassLoader(jars)); + } + + private static ClassLoader createClassLoader(final List jars) + { + final String name = "ServiceClassLoader-" + + jars.stream().map(Path::getFileName).map(Path::toString).collect(joining(",")); + final URL[] urls = jars.stream().map(ServiceOrigin::toUrl).toArray(URL[]::new); + return new URLClassLoader(name, urls, getBaseClassLoader()); + } + + private static URL toUrl(final Path path) + { + try + { + return path.toUri().toURL(); + } + catch (final MalformedURLException e) + { + throw new IllegalStateException("Error converting path " + path + " to url", e); + } + } + + public ClassLoader getClassLoader() + { + return classLoader; + } +} From 5e6696c553543af24fd15fd56700d6640af1ecaf Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 13:03:33 +0200 Subject: [PATCH 04/44] Code cleanup for test --- .../TestInitializingServiceLoader.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java index 4925686e..24d64e91 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java @@ -1,11 +1,9 @@ package org.itsallcode.openfasttrace.core.serviceloader; -import static java.util.stream.Collectors.toList; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import java.util.List; -import java.util.stream.StreamSupport; import org.itsallcode.openfasttrace.api.core.serviceloader.Initializable; import org.itsallcode.openfasttrace.api.exporter.ExporterContext; @@ -21,7 +19,9 @@ import org.junit.jupiter.api.Test; /** - * Test for {@link InitializingServiceLoader} + * Test for {@link InitializingServiceLoader} from module {@code core}. This + * test must be located in module {@code product} (which includes all plugin + * modules) so that it can access all plugin services. */ class TestInitializingServiceLoader { @@ -31,8 +31,7 @@ void testNoServicesRegistered() final Object context = new Object(); final InitializingServiceLoader voidServiceLoader = InitializingServiceLoader .load(InitializableServiceStub.class, context); - final List services = StreamSupport - .stream(voidServiceLoader.spliterator(), false).collect(toList()); + final List services = voidServiceLoader.load().toList(); assertThat(services, emptyIterable()); assertThat(voidServiceLoader, emptyIterable()); } @@ -44,10 +43,11 @@ void testImporterFactoriesRegistered() final List services = getRegisteredServices(ImporterFactory.class, context); assertThat(services, hasSize(5)); - assertThat(services, containsInAnyOrder(instanceOf(MarkdownImporterFactory.class), // - instanceOf(RestructuredTextImporterFactory.class), // - instanceOf(SpecobjectImporterFactory.class), // - instanceOf(TagImporterFactory.class), // + assertThat(services, containsInAnyOrder( + instanceOf(MarkdownImporterFactory.class), + instanceOf(RestructuredTextImporterFactory.class), + instanceOf(SpecobjectImporterFactory.class), + instanceOf(TagImporterFactory.class), instanceOf(ZipFileImporterFactory.class))); for (final ImporterFactory importerFactory : services) { @@ -74,7 +74,7 @@ private , C> List getRegisteredServices(final Clas { final InitializingServiceLoader serviceLoader = InitializingServiceLoader.load(type, context); - return StreamSupport.stream(serviceLoader.spliterator(), false).collect(toList()); + return serviceLoader.load().toList(); } class InitializableServiceStub implements Initializable From dccb8df7eb524f306a356a027773691985a9ec44 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 15:46:42 +0200 Subject: [PATCH 05/44] Add unit tests for service loader --- .../serviceloader/ClassPathServiceLoader.java | 4 +- .../serviceloader/PluginLoaderFactory.java | 14 ++- .../core/serviceloader/ServiceOrigin.java | 19 +++- .../ClassPathServiceLoaderTest.java | 63 +++++++++++ .../serviceloader/DelegatingLoaderTest.java | 53 +++++++++ .../serviceloader/PluginLoaderFactoryIT.java | 75 +++++++++++++ .../PluginLoaderFactoryTest.java | 103 ++++++++++++++++++ core/src/test/resources/logging.properties | 4 +- 8 files changed, 324 insertions(+), 11 deletions(-) create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index 943abfc0..9b73c0d2 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -8,12 +8,12 @@ final class ClassPathServiceLoader implements Loader { private final ServiceLoader serviceLoader; - private ClassPathServiceLoader(final ServiceLoader serviceLoader) + ClassPathServiceLoader(final ServiceLoader serviceLoader) { this.serviceLoader = serviceLoader; } - static Loader load(final Class serviceType, final ServiceOrigin serviceOrigin) + static Loader create(final Class serviceType, final ServiceOrigin serviceOrigin) { return new ClassPathServiceLoader<>(ServiceLoader.load(serviceType, serviceOrigin.getClassLoader())); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java index 27ef6f96..e52af978 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java @@ -5,9 +5,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.*; +import java.util.logging.Logger; class PluginLoaderFactory { + private static final Logger LOGGER = Logger.getLogger(PluginLoaderFactory.class.getName()); private final Path pluginsDirectory; PluginLoaderFactory(final Path pluginsDirectory) @@ -32,16 +34,18 @@ Loader createLoader(final Class serviceType) private Loader createLoader(final Class serviceType, final List origins) { - final List> loaders = origins.stream().map(origin -> ClassPathServiceLoader.load(serviceType, origin)) + final List> loaders = origins.stream() + .map(origin -> ClassPathServiceLoader.create(serviceType, origin)) .toList(); return new DelegatingLoader<>(loaders); } - private List findServiceOrigins() + List findServiceOrigins() { final List origins = new ArrayList<>(); origins.add(ServiceOrigin.forCurrentClassPath()); origins.addAll(findPluginOrigins()); + LOGGER.fine(() -> "Found " + origins.size() + " service origins: " + origins + "."); return origins; } @@ -51,10 +55,10 @@ Collection findPluginOrigins() { return Collections.emptyList(); } - try { return Files.list(pluginsDirectory) + .sorted() .map(this::originForPath) .flatMap(Optional::stream) .toList(); @@ -71,17 +75,21 @@ private Optional originForPath(final Path path) { if (isJarFile(path)) { + LOGGER.fine(() -> "Found single plugin jar '" + path + "'."); return Optional.of(ServiceOrigin.forJars(List.of(path))); } if (!Files.isDirectory(path)) { + LOGGER.fine(() -> "Ignore unsupported file '" + path + "'."); return Optional.empty(); } final List jars = findJarsInDir(path); if (jars.isEmpty()) { + LOGGER.fine(() -> "Ignore empty plugin directory '" + path + "'."); return Optional.empty(); } + LOGGER.fine(() -> "Found " + jars.size() + " in '" + path + "': " + jars + "."); return Optional.of(ServiceOrigin.forJars(jars)); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index b79e8e43..c3d5fef0 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -37,10 +37,15 @@ public static ServiceOrigin forJars(final List jars) private static ClassLoader createClassLoader(final List jars) { - final String name = "ServiceClassLoader-" - + jars.stream().map(Path::getFileName).map(Path::toString).collect(joining(",")); - final URL[] urls = jars.stream().map(ServiceOrigin::toUrl).toArray(URL[]::new); - return new URLClassLoader(name, urls, getBaseClassLoader()); + final URL[] urls = jars.stream().map(ServiceOrigin::toUrl) + .toArray(URL[]::new); + return new URLClassLoader(getClassLoaderName(jars), urls, getBaseClassLoader()); + } + + private static String getClassLoaderName(final List jars) + { + return "ServiceClassLoader-" + + jars.stream().map(Path::getFileName).sorted().map(Path::toString).collect(joining(",")); } private static URL toUrl(final Path path) @@ -59,4 +64,10 @@ public ClassLoader getClassLoader() { return classLoader; } + + @Override + public String toString() + { + return "ServiceOrigin [classLoader=" + classLoader + "]"; + } } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java new file mode 100644 index 00000000..9bf6dde5 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java @@ -0,0 +1,63 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; + +import java.util.*; +import java.util.ServiceLoader.Provider; +import java.util.stream.Stream; + +import org.itsallcode.openfasttrace.api.report.ReporterFactory; +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 ClassPathServiceLoaderTest +{ + @Test + void loadingNonAccessibleServiceFails() + { + final ServiceOrigin origin = ServiceOrigin.forCurrentClassPath(); + final ServiceConfigurationError error = assertThrows(ServiceConfigurationError.class, + () -> ClassPathServiceLoader.create(DummyService.class, origin)); + assertThat(error.getMessage(), equalTo( + "org.itsallcode.openfasttrace.core.serviceloader.ClassPathServiceLoaderTest$DummyService: module org.itsallcode.openfasttrace.core does not declare `uses`")); + } + + @Test + void loadingFindsNothing() + { + final List services = ClassPathServiceLoader + .create(ReporterFactory.class, ServiceOrigin.forCurrentClassPath()).load() + .toList(); + assertThat(services, hasSize(0)); + } + + @Test + void loadingReturnsService(@Mock final ServiceLoader serviceLoaderMock, + @Mock final Provider providerMock) + { + + final DummyServiceImpl service = new DummyServiceImpl(); + when(serviceLoaderMock.stream()).thenReturn(Stream.of(providerMock)); + when(providerMock.get()).thenReturn(service); + final List services = new ClassPathServiceLoader(serviceLoaderMock).load().toList(); + assertAll(() -> assertThat(services, hasSize(1)), + () -> assertThat(services.get(0), not(nullValue())), + () -> assertThat(services.get(0), instanceOf(DummyServiceImpl.class)), + () -> assertThat(services.get(0), sameInstance(service))); + } + + static interface DummyService + { + } + + static class DummyServiceImpl implements DummyService + { + } +} diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java new file mode 100644 index 00000000..f528a0e6 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java @@ -0,0 +1,53 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.stream.Stream; + +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 DelegatingLoaderTest +{ + @Test + void noDelegates() + { + assertThat(new DelegatingLoader<>(List.of()).load().toList(), empty()); + } + + @Test + void emptyDelegate(@Mock final Loader loaderMock) + { + when(loaderMock.load()).thenReturn(Stream.empty()); + assertThat(new DelegatingLoader<>(List.of(loaderMock)).load().toList(), empty()); + } + + @Test + void nonEmptyDelegate(@Mock final Loader loaderMock, @Mock final DummyService serviceMock) + { + when(loaderMock.load()).thenReturn(Stream.of(serviceMock)); + assertThat(new DelegatingLoader<>(List.of(loaderMock)).load().toList(), contains(serviceMock)); + } + + @Test + void loadsDelegatesInOrder(@Mock final Loader loaderMock1, + @Mock final Loader loaderMock2, @Mock final DummyService serviceMock1, + @Mock final DummyService serviceMock2) + { + when(loaderMock1.load()).thenReturn(Stream.of(serviceMock1)); + when(loaderMock2.load()).thenReturn(Stream.of(serviceMock2)); + assertThat(new DelegatingLoader<>(List.of(loaderMock1, loaderMock2)).load().toList(), + contains(serviceMock1, serviceMock2)); + } + + static interface DummyService + { + } +} diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java new file mode 100644 index 00000000..d65440b4 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java @@ -0,0 +1,75 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertAll; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; +import java.util.regex.Pattern; + +import org.itsallcode.openfasttrace.api.report.ReporterFactory; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.opentest4j.TestAbortedException; + +class PluginLoaderFactoryIT +{ + @TempDir + Path tempDir; + + @Test + void loadServiceFromWrongJar() throws IOException + { + preparePlugin(Path.of("../reporter/plaintext/target"), + "openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\-javadoc.jar"); + assertThat(loadService(), empty()); + } + + private List loadService() + { + return new PluginLoaderFactory(tempDir).createLoader(ReporterFactory.class).load().toList(); + } + + @Test + void loadServiceFromJar() throws IOException + { + preparePlugin(Path.of("../reporter/plaintext/target"), + "openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\.jar"); + final List services = loadService(); + assertThat(services, hasSize(1)); + final ReporterFactory service = services.get(0); + final ClassLoader pluginClassLoader = service.getClass().getClassLoader(); + assertAll( + () -> assertThat(service.getClass().getName().toString(), + equalTo("org.itsallcode.openfasttrace.report.plaintext.PlaintextReporterFactory")), + () -> assertThat(pluginClassLoader.getName(), + startsWith("ServiceClassLoader-openfasttrace-reporter-plaintext")), + () -> assertThat(pluginClassLoader, not(sameInstance(Thread.currentThread().getContextClassLoader())))); + } + + private void preparePlugin(final Path targetDir, final String filePattern) throws TestAbortedException, IOException + { + final Path jar = findMatchingFile(targetDir, filePattern) + .orElseThrow(() -> new AssertionError( + "Did not file matching '" + filePattern + "' in '" + targetDir + "'")); + preparePlugin(jar); + } + + private Optional findMatchingFile(final Path dir, final String filePattern) throws IOException + { + final Pattern pattern = Pattern.compile(filePattern); + return Files.list(dir).filter(file -> pattern.matcher(file.getFileName().toString()).matches()) + .findFirst(); + } + + private void preparePlugin(final Path pluginJar) throws IOException + { + final Path pluginDir = tempDir.resolve("plugin"); + Files.createDirectories(pluginDir); + Files.copy(pluginJar, pluginDir.resolve(pluginJar.getFileName())); + } +} diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java new file mode 100644 index 00000000..6f1197af --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java @@ -0,0 +1,103 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertAll; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.itsallcode.openfasttrace.api.report.ReporterFactory; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.opentest4j.MultipleFailuresError; + +class PluginLoaderFactoryTest +{ + @TempDir + Path tempDir; + + @Test + void noPluginJar() + { + assertThat(testee().createLoader(ReporterFactory.class).load().toList(), empty()); + } + + private PluginLoaderFactory testee() + { + return new PluginLoaderFactory(tempDir); + } + + @Test + void findServiceMissingParentDir() throws IOException + { + Files.delete(tempDir); + final List origins = testee().findServiceOrigins(); + assertNoPlugins(origins); + } + + private void assertNoPlugins(final List origins) throws MultipleFailuresError + { + assertAll(() -> assertThat(origins, hasSize(1)), + () -> assertThat(origins.get(0).getClassLoader().getName(), equalTo("app"))); + } + + @Test + void findServiceOriginsPluginDir() + { + final List origins = testee().findServiceOrigins(); + assertNoPlugins(origins); + } + + @Test + void findServiceOriginsEmptyPluginDir() throws IOException + { + final Path pluginDir = tempDir.resolve("plugin1"); + Files.createDirectories(pluginDir); + final List origins = testee().findServiceOrigins(); + assertNoPlugins(origins); + } + + @Test + void findServiceOriginsSingleJar() throws IOException + { + final Path pluginDir = tempDir.resolve("plugin1"); + Files.createDirectories(pluginDir); + Files.createFile(pluginDir.resolve("plugin1.jar")); + final List origins = testee().findServiceOrigins(); + assertAll(() -> assertThat(origins, hasSize(2)), + () -> assertThat(origins.get(1).getClassLoader().getName(), equalTo("ServiceClassLoader-plugin1.jar"))); + } + + @Test + void findServiceOriginsMultiJar() throws IOException + { + final Path pluginDir = tempDir.resolve("plugin1"); + Files.createDirectories(pluginDir); + Files.createFile(pluginDir.resolve("plugin1.jar")); + Files.createFile(pluginDir.resolve("plugin2.jar")); + final List origins = testee().findServiceOrigins(); + assertAll(() -> assertThat(origins, hasSize(2)), + () -> assertThat(origins.get(1).getClassLoader().getName(), + equalTo("ServiceClassLoader-plugin1.jar,plugin2.jar"))); + } + + @Test + void findServiceOriginsMultiPlugins() throws IOException + { + final Path pluginDir1 = tempDir.resolve("plugin1"); + final Path pluginDir2 = tempDir.resolve("plugin2"); + Files.createDirectories(pluginDir1); + Files.createDirectories(pluginDir2); + Files.createFile(pluginDir1.resolve("plugin1.jar")); + Files.createFile(pluginDir2.resolve("plugin2.jar")); + final List origins = testee().findServiceOrigins(); + assertAll(() -> assertThat(origins, hasSize(3)), + () -> assertThat(origins.get(1).getClassLoader().getName(), + equalTo("ServiceClassLoader-plugin1.jar")), + () -> assertThat(origins.get(2).getClassLoader().getName(), + equalTo("ServiceClassLoader-plugin2.jar"))); + } +} diff --git a/core/src/test/resources/logging.properties b/core/src/test/resources/logging.properties index b0722c14..20697e38 100644 --- a/core/src/test/resources/logging.properties +++ b/core/src/test/resources/logging.properties @@ -1,10 +1,10 @@ handlers = java.util.logging.ConsoleHandler org.itsallcode.openfasttrace.testutil.log.NoOpLoggingHandler .level = INFO -java.util.logging.ConsoleHandler.level = INFO +java.util.logging.ConsoleHandler.level = ALL java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter java.util.logging.ConsoleHandler.encoding = UTF-8 java.util.logging.SimpleFormatter.format = %1$tF %1$tT [%4$s] %2$s - %5$s %6$s%n org.itsallcode.openfasttrace.testutil.log.NoOpLoggingHandler.level = ALL -org.itsallcode.openfasttrace.level = FINEST +org.itsallcode.openfasttrace.level = INFO From f2cdbdbe05f538be2926bc10d02dc471115f1014 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 16:51:31 +0200 Subject: [PATCH 06/44] Format GH workflows --- .github/workflows/broken_links_checker.yml | 8 +- .github/workflows/build.yml | 156 ++++++++++----------- .github/workflows/codeql-analysis.yml | 36 ++--- .github/workflows/gh-pages.yml | 3 - 4 files changed, 99 insertions(+), 104 deletions(-) diff --git a/.github/workflows/broken_links_checker.yml b/.github/workflows/broken_links_checker.yml index 52cd2351..1a266605 100644 --- a/.github/workflows/broken_links_checker.yml +++ b/.github/workflows/broken_links_checker.yml @@ -2,7 +2,7 @@ name: Broken Links Checker on: push: - branches: [ main ] + branches: [main] pull_request: jobs: @@ -18,6 +18,6 @@ jobs: echo '{ "aliveStatusCodes": [429, 200] }' > ./target/broken_links_checker.json - uses: gaurav-nelson/github-action-markdown-link-check@v1 with: - use-quiet-mode: 'yes' - use-verbose-mode: 'yes' - config-file: ./target/broken_links_checker.json \ No newline at end of file + use-quiet-mode: "yes" + use-verbose-mode: "yes" + config-file: ./target/broken_links_checker.json diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c2e380ba..f62b92eb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,12 +2,11 @@ name: Build on: push: - branches: [ main ] + branches: [main] pull_request: jobs: matrix-build: - permissions: contents: read @@ -34,83 +33,82 @@ jobs: DEFAULT_OS: ubuntu-latest steps: - - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - uses: actions/setup-java@v4 - with: - distribution: 'temurin' - java-version: | - 17 - 21 - cache: 'maven' - - - name: Cache SonarQube packages - if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} - uses: actions/cache@v4 - with: - path: ~/.sonar/cache - key: ${{ runner.os }}-sonar - restore-keys: ${{ runner.os }}-sonar - - - name: Build with Java ${{ matrix.java }} - run: | - mvn --batch-mode -T 1C clean org.jacoco:jacoco-maven-plugin:prepare-agent install \ - -Djava.version=${{ matrix.java }} - - - name: Sonar analysis - if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java && env.SONAR_TOKEN != null }} - run: | - mvn --batch-mode org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \ - -Dsonar.token=$SONAR_TOKEN - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - - - name: Verify reproducible build - # Build fails on Windows with error "Failed to execute goal org.apache.maven.plugins:maven-artifact-plugin:3.5.0:compare (default-cli) on project openfasttrace-reporter-plaintext: Could not copy D:\a\openfasttrace\openfasttrace\reporter\plaintext\target\openfasttrace-reporter-plaintext-3.8.0.buildcompareto D:\a\openfasttrace\openfasttrace\target\openfasttrace-root-0.0.0.buildcompare" - if: ${{ matrix.os != 'windows-latest' }} - run: | - mvn --batch-mode -T 1C clean verify artifact:compare -DskipTests \ - -Djava.version=${{ matrix.java }} - - - name: Archive aggregated reproducible build report - uses: actions/upload-artifact@v4 - if: ${{ matrix.os != 'windows-latest' }} - with: - name: reproducible-build-report-${{ matrix.os }}-java-${{ matrix.java }} - path: | - target/openfasttrace-root-0.0.0.buildcompare - target/openfasttrace-root-0.0.0.buildinfo - if-no-files-found: error - - - name: Archive oft binary - uses: actions/upload-artifact@v4 - if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} - with: - name: openfasttrace-binaries - path: | - product/target/openfasttrace-*.jar - !product/target/openfasttrace-*-javadoc.jar - !product/target/openfasttrace-*-sources.jar - if-no-files-found: error - - - name: Run self-trace - run: ./oft-self-trace.sh - - - name: Upload self-tracing report - uses: actions/upload-artifact@v4 - if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} - with: - name: self-tracing-report - path: target/self-trace-report.html - if-no-files-found: error - - - name: Check shell scripts - if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} - run: .github/workflows/run_shellcheck.sh + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-java@v4 + with: + distribution: "temurin" + java-version: | + 17 + 21 + cache: "maven" + + - name: Cache SonarQube packages + if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} + uses: actions/cache@v4 + with: + path: ~/.sonar/cache + key: ${{ runner.os }}-sonar + restore-keys: ${{ runner.os }}-sonar + + - name: Build with Java ${{ matrix.java }} + run: | + mvn --batch-mode -T 1C clean org.jacoco:jacoco-maven-plugin:prepare-agent install \ + -Djava.version=${{ matrix.java }} + + - name: Sonar analysis + if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java && env.SONAR_TOKEN != null }} + run: | + mvn --batch-mode org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \ + -Dsonar.token=$SONAR_TOKEN + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + + - name: Verify reproducible build + # Build fails on Windows with error "Failed to execute goal org.apache.maven.plugins:maven-artifact-plugin:3.5.0:compare (default-cli) on project openfasttrace-reporter-plaintext: Could not copy D:\a\openfasttrace\openfasttrace\reporter\plaintext\target\openfasttrace-reporter-plaintext-3.8.0.buildcompareto D:\a\openfasttrace\openfasttrace\target\openfasttrace-root-0.0.0.buildcompare" + if: ${{ matrix.os != 'windows-latest' }} + run: | + mvn --batch-mode -T 1C clean verify artifact:compare -DskipTests \ + -Djava.version=${{ matrix.java }} + + - name: Archive aggregated reproducible build report + uses: actions/upload-artifact@v4 + if: ${{ matrix.os != 'windows-latest' }} + with: + name: reproducible-build-report-${{ matrix.os }}-java-${{ matrix.java }} + path: | + target/openfasttrace-root-0.0.0.buildcompare + target/openfasttrace-root-0.0.0.buildinfo + if-no-files-found: error + + - name: Archive oft binary + uses: actions/upload-artifact@v4 + if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} + with: + name: openfasttrace-binaries + path: | + product/target/openfasttrace-*.jar + !product/target/openfasttrace-*-javadoc.jar + !product/target/openfasttrace-*-sources.jar + if-no-files-found: error + + - name: Run self-trace + run: ./oft-self-trace.sh + + - name: Upload self-tracing report + uses: actions/upload-artifact@v4 + if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} + with: + name: self-tracing-report + path: target/self-trace-report.html + if-no-files-found: error + + - name: Check shell scripts + if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java }} + run: .github/workflows/run_shellcheck.sh build: needs: matrix-build diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index aa2db593..7942d9f9 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -2,11 +2,11 @@ name: "CodeQL" on: push: - branches: [ main ] + branches: [main] pull_request: - branches: [ main ] + branches: [main] schedule: - - cron: '0 4 * * 3' + - cron: "0 4 * * 3" jobs: analyze: @@ -21,22 +21,22 @@ jobs: cancel-in-progress: true steps: - - name: Checkout repository - uses: actions/checkout@v4 + - name: Checkout repository + uses: actions/checkout@v4 - - uses: actions/setup-java@v4 - with: - distribution: 'temurin' - java-version: 17 - cache: 'maven' + - uses: actions/setup-java@v4 + with: + distribution: "temurin" + java-version: 17 + cache: "maven" - - name: Initialize CodeQL - uses: github/codeql-action/init@v3 - with: - languages: java + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: java - - name: Autobuild - uses: github/codeql-action/autobuild@v3 + - name: Autobuild + uses: github/codeql-action/autobuild@v3 - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v3 + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/gh-pages.yml b/.github/workflows/gh-pages.yml index b4155054..757c4f48 100644 --- a/.github/workflows/gh-pages.yml +++ b/.github/workflows/gh-pages.yml @@ -4,9 +4,6 @@ on: push: branches: ["main"] workflow_dispatch: - # Temporarily also run on pull requests - pull_request: - branches: ["main"] permissions: contents: read From 4ef6914b67406e32c7fc63052701f66ce28e439d Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 16:52:32 +0200 Subject: [PATCH 07/44] Run build with correct java version --- .github/workflows/build.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f62b92eb..9669c2e7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -53,10 +53,13 @@ jobs: key: ${{ runner.os }}-sonar restore-keys: ${{ runner.os }}-sonar - - name: Build with Java ${{ matrix.java }} - run: | - mvn --batch-mode -T 1C clean org.jacoco:jacoco-maven-plugin:prepare-agent install \ - -Djava.version=${{ matrix.java }} + - name: Build with Java 17 + if: ${{ matrix.java == 17 }} + run: JAVA_HOME=$JAVA_HOME_17_X64 mvn --batch-mode -T 1C clean install -Djava.version=${{ matrix.java }} + + - name: Build with Java 21 + if: ${{ matrix.java == 21 }} + run: JAVA_HOME=JAVA_HOME_21_X64 mvn --batch-mode -T 1C clean install -Djava.version=${{ matrix.java }} - name: Sonar analysis if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java && env.SONAR_TOKEN != null }} From 65bb5745233e230c9afc8f76ffd90acbbd6ef921 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 16:58:15 +0200 Subject: [PATCH 08/44] Install only the required JDK version --- .github/workflows/build.yml | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9669c2e7..1bfc60d4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -38,11 +38,10 @@ jobs: fetch-depth: 0 - uses: actions/setup-java@v4 + name: Set up Java ${{ matrix.java }} with: distribution: "temurin" - java-version: | - 17 - 21 + java-version: ${{ matrix.java }} cache: "maven" - name: Cache SonarQube packages @@ -53,13 +52,10 @@ jobs: key: ${{ runner.os }}-sonar restore-keys: ${{ runner.os }}-sonar - - name: Build with Java 17 - if: ${{ matrix.java == 17 }} - run: JAVA_HOME=$JAVA_HOME_17_X64 mvn --batch-mode -T 1C clean install -Djava.version=${{ matrix.java }} - - - name: Build with Java 21 - if: ${{ matrix.java == 21 }} - run: JAVA_HOME=JAVA_HOME_21_X64 mvn --batch-mode -T 1C clean install -Djava.version=${{ matrix.java }} + - name: Build with Java ${{ matrix.java }} + run: | + mvn --batch-mode -T 1C clean org.jacoco:jacoco-maven-plugin:prepare-agent install \ + -Djava.version=${{ matrix.java }} - name: Sonar analysis if: ${{ env.DEFAULT_OS == matrix.os && env.DEFAULT_JAVA == matrix.java && env.SONAR_TOKEN != null }} From 7d490329240e2d0455fa0a4681cbbf6f2ac30d54 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 18:01:39 +0200 Subject: [PATCH 09/44] #413: Add tests --- .../serviceloader/ChildFirstClassLoader.java | 49 +++++++++++++++++++ .../serviceloader/ClassPathServiceLoader.java | 24 +++++++-- .../serviceloader/PluginLoaderFactory.java | 11 +++-- .../core/serviceloader/ServiceOrigin.java | 16 +++--- .../ClassPathServiceLoaderTest.java | 7 +-- .../PluginLoaderFactoryTest.java | 2 +- .../serviceloader/PluginLoaderFactoryIT.java | 7 ++- product/src/test/resources/logging.properties | 10 ++++ 8 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java rename {core => product}/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java (89%) create mode 100644 product/src/test/resources/logging.properties diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java new file mode 100644 index 00000000..870e0503 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java @@ -0,0 +1,49 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import java.net.URL; +import java.net.URLClassLoader; + +/** + * This class loader will first try to load the class from the given URLs and + * then from the parent class loader, unlike {@link URLClassLoader} which does + * it the other way around. + *

+ * This is based on + * https://medium.com/@isuru89/java-a-child-first-class-loader-cbd9c3d0305 + *

+ */ +class ChildFirstClassLoader extends URLClassLoader +{ + ChildFirstClassLoader(final String name, final URL[] urls, final ClassLoader parent) + { + super(name, urls, parent); + } + + @Override + protected Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException + { + // has the class loaded already? + Class loadedClass = findLoadedClass(name); + if (loadedClass == null) + { + try + { + // find the class from given jar urls + loadedClass = findClass(name); + } + catch (final ClassNotFoundException ignore) + { + // Hmmm... class does not exist in the given urls. + // Let's try finding it in our parent classloader. + // this'll throw ClassNotFoundException in failure. + loadedClass = super.loadClass(name, resolve); + } + } + + if (resolve) + { // marked to resolve + resolveClass(loadedClass); + } + return loadedClass; + } +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index 9b73c0d2..ffb5737f 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -2,25 +2,43 @@ import java.util.ServiceLoader; import java.util.ServiceLoader.Provider; +import java.util.logging.Logger; import java.util.stream.Stream; final class ClassPathServiceLoader implements Loader { + private static final Logger LOGGER = Logger.getLogger(ClassPathServiceLoader.class.getName()); + private final ServiceOrigin serviceOrigin; private final ServiceLoader serviceLoader; - ClassPathServiceLoader(final ServiceLoader serviceLoader) + ClassPathServiceLoader(final ServiceOrigin serviceOrigin, final ServiceLoader serviceLoader) { + this.serviceOrigin = serviceOrigin; this.serviceLoader = serviceLoader; } static Loader create(final Class serviceType, final ServiceOrigin serviceOrigin) { - return new ClassPathServiceLoader<>(ServiceLoader.load(serviceType, serviceOrigin.getClassLoader())); + return new ClassPathServiceLoader<>(serviceOrigin, + ServiceLoader.load(serviceType, serviceOrigin.getClassLoader())); } @Override public Stream load() { - return this.serviceLoader.stream().map(Provider::get); + return this.serviceLoader.stream().map(Provider::get) + .filter(this::filterOtherClassLoader); + } + + private boolean filterOtherClassLoader(final T service) + { + final ClassLoader serviceClassLoader = service.getClass().getClassLoader(); + final boolean correctClassLoader = serviceClassLoader == serviceOrigin.getClassLoader(); + if (!correctClassLoader) + { + LOGGER.fine(() -> "Service " + service + " has wrong class loader: " + serviceClassLoader + ", expected " + + serviceOrigin.getClassLoader()); + } + return correctClassLoader; } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java index e52af978..93281ef2 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java @@ -11,15 +11,17 @@ class PluginLoaderFactory { private static final Logger LOGGER = Logger.getLogger(PluginLoaderFactory.class.getName()); private final Path pluginsDirectory; + private final boolean searchCurrentClasspath; - PluginLoaderFactory(final Path pluginsDirectory) + PluginLoaderFactory(final Path pluginsDirectory, final boolean searchCurrentClasspath) { this.pluginsDirectory = pluginsDirectory; + this.searchCurrentClasspath = searchCurrentClasspath; } static PluginLoaderFactory createDefault() { - return new PluginLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins")); + return new PluginLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins"), true); } private static Path getHomeDirectory() @@ -43,7 +45,10 @@ private Loader createLoader(final Class serviceType, final List findServiceOrigins() { final List origins = new ArrayList<>(); - origins.add(ServiceOrigin.forCurrentClassPath()); + if (searchCurrentClasspath) + { + origins.add(ServiceOrigin.forCurrentClassPath()); + } origins.addAll(findPluginOrigins()); LOGGER.fine(() -> "Found " + origins.size() + " service origins: " + origins + "."); return origins; diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index c3d5fef0..0d0d81cc 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -1,23 +1,27 @@ package org.itsallcode.openfasttrace.core.serviceloader; +import static java.util.Collections.emptyList; import static java.util.stream.Collectors.joining; -import java.net.*; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.file.Path; import java.util.List; final class ServiceOrigin { private final ClassLoader classLoader; + private final List jars; - private ServiceOrigin(final ClassLoader classLoader) + private ServiceOrigin(final ClassLoader classLoader, final List jars) { this.classLoader = classLoader; + this.jars = jars; } public static ServiceOrigin forCurrentClassPath() { - return new ServiceOrigin(getBaseClassLoader()); + return new ServiceOrigin(getBaseClassLoader(), emptyList()); } private static ClassLoader getBaseClassLoader() @@ -32,14 +36,14 @@ public static ServiceOrigin forJar(final Path jar) public static ServiceOrigin forJars(final List jars) { - return new ServiceOrigin(createClassLoader(jars)); + return new ServiceOrigin(createClassLoader(jars), jars); } private static ClassLoader createClassLoader(final List jars) { final URL[] urls = jars.stream().map(ServiceOrigin::toUrl) .toArray(URL[]::new); - return new URLClassLoader(getClassLoaderName(jars), urls, getBaseClassLoader()); + return new ChildFirstClassLoader(getClassLoaderName(jars), urls, getBaseClassLoader()); } private static String getClassLoaderName(final List jars) @@ -68,6 +72,6 @@ public ClassLoader getClassLoader() @Override public String toString() { - return "ServiceOrigin [classLoader=" + classLoader + "]"; + return "ServiceOrigin [classLoader=" + classLoader + ", jars=" + jars + "]"; } } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java index 9bf6dde5..9b191d69 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java @@ -40,13 +40,14 @@ void loadingFindsNothing() @Test void loadingReturnsService(@Mock final ServiceLoader serviceLoaderMock, - @Mock final Provider providerMock) + @Mock final Provider providerMock, @Mock final ServiceOrigin originMock) { - final DummyServiceImpl service = new DummyServiceImpl(); when(serviceLoaderMock.stream()).thenReturn(Stream.of(providerMock)); when(providerMock.get()).thenReturn(service); - final List services = new ClassPathServiceLoader(serviceLoaderMock).load().toList(); + final List services = new ClassPathServiceLoader(originMock, serviceLoaderMock) + .load() + .toList(); assertAll(() -> assertThat(services, hasSize(1)), () -> assertThat(services.get(0), not(nullValue())), () -> assertThat(services.get(0), instanceOf(DummyServiceImpl.class)), diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java index 6f1197af..411679cb 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java @@ -27,7 +27,7 @@ void noPluginJar() private PluginLoaderFactory testee() { - return new PluginLoaderFactory(tempDir); + return new PluginLoaderFactory(tempDir, true); } @Test diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java similarity index 89% rename from core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java rename to product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java index d65440b4..84ce05d0 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java @@ -16,6 +16,11 @@ import org.junit.jupiter.api.io.TempDir; import org.opentest4j.TestAbortedException; +/** + * Test for {@link PluginLoaderFactory} from module {@code core}. This test must + * be located in module {@code product} (which includes all plugin modules) so + * that it can access all plugin services. + */ class PluginLoaderFactoryIT { @TempDir @@ -31,7 +36,7 @@ void loadServiceFromWrongJar() throws IOException private List loadService() { - return new PluginLoaderFactory(tempDir).createLoader(ReporterFactory.class).load().toList(); + return new PluginLoaderFactory(tempDir, false).createLoader(ReporterFactory.class).load().toList(); } @Test diff --git a/product/src/test/resources/logging.properties b/product/src/test/resources/logging.properties new file mode 100644 index 00000000..20697e38 --- /dev/null +++ b/product/src/test/resources/logging.properties @@ -0,0 +1,10 @@ +handlers = java.util.logging.ConsoleHandler org.itsallcode.openfasttrace.testutil.log.NoOpLoggingHandler +.level = INFO +java.util.logging.ConsoleHandler.level = ALL +java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter +java.util.logging.ConsoleHandler.encoding = UTF-8 +java.util.logging.SimpleFormatter.format = %1$tF %1$tT [%4$s] %2$s - %5$s %6$s%n + +org.itsallcode.openfasttrace.testutil.log.NoOpLoggingHandler.level = ALL + +org.itsallcode.openfasttrace.level = INFO From 53b111e5ef4dc730909d690413f41f808997f601 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 18:09:13 +0200 Subject: [PATCH 10/44] Refactoring --- .../InitializingServiceLoader.java | 2 +- ...Factory.java => ServiceLoaderFactory.java} | 21 +++++++++++++------ ...est.java => ServiceLoaderFactoryTest.java} | 13 +++++++++--- ...oryIT.java => ServiceLoaderFactoryIT.java} | 10 ++++----- 4 files changed, 31 insertions(+), 15 deletions(-) rename core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/{PluginLoaderFactory.java => ServiceLoaderFactory.java} (81%) rename core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/{PluginLoaderFactoryTest.java => ServiceLoaderFactoryTest.java} (91%) rename product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/{PluginLoaderFactoryIT.java => ServiceLoaderFactoryIT.java} (89%) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java index 30af7205..4ca74849 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java @@ -43,7 +43,7 @@ private InitializingServiceLoader(final Loader delegate, final C context) public static , C> InitializingServiceLoader load( final Class serviceType, final C context) { - final PluginLoaderFactory loaderFactory = PluginLoaderFactory.createDefault(); + final ServiceLoaderFactory loaderFactory = ServiceLoaderFactory.createDefault(); return new InitializingServiceLoader<>(loaderFactory.createLoader(serviceType), context); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java similarity index 81% rename from core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java rename to core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index 93281ef2..dfcf30f9 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -7,21 +7,30 @@ import java.util.*; import java.util.logging.Logger; -class PluginLoaderFactory +class ServiceLoaderFactory { - private static final Logger LOGGER = Logger.getLogger(PluginLoaderFactory.class.getName()); + private static final Logger LOGGER = Logger.getLogger(ServiceLoaderFactory.class.getName()); private final Path pluginsDirectory; private final boolean searchCurrentClasspath; - PluginLoaderFactory(final Path pluginsDirectory, final boolean searchCurrentClasspath) + /** + * Create a new factory for service {@link Loader}. + * + * @param pluginsDirectory + * the directory to search for plugins + * @param searchCurrentClasspath + * whether to search the current classpath for plugins. This is + * useful for testing to avoid loading plugins twice. + */ + ServiceLoaderFactory(final Path pluginsDirectory, final boolean searchCurrentClasspath) { this.pluginsDirectory = pluginsDirectory; this.searchCurrentClasspath = searchCurrentClasspath; } - static PluginLoaderFactory createDefault() + static ServiceLoaderFactory createDefault() { - return new PluginLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins"), true); + return new ServiceLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins"), true); } private static Path getHomeDirectory() @@ -102,7 +111,7 @@ private static List findJarsInDir(final Path path) { try { - return Files.list(path).filter(PluginLoaderFactory::isJarFile).toList(); + return Files.list(path).filter(ServiceLoaderFactory::isJarFile).toList(); } catch (final IOException exception) { diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java similarity index 91% rename from core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java rename to core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 411679cb..1af758c8 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -14,7 +14,7 @@ import org.junit.jupiter.api.io.TempDir; import org.opentest4j.MultipleFailuresError; -class PluginLoaderFactoryTest +class ServiceLoaderFactoryTest { @TempDir Path tempDir; @@ -25,9 +25,16 @@ void noPluginJar() assertThat(testee().createLoader(ReporterFactory.class).load().toList(), empty()); } - private PluginLoaderFactory testee() + private ServiceLoaderFactory testee() { - return new PluginLoaderFactory(tempDir, true); + return new ServiceLoaderFactory(tempDir, true); + } + + @Test + void findServiceSkipCurrentClassLoader() throws IOException + { + Files.delete(tempDir); + assertThat(new ServiceLoaderFactory(tempDir, false).findServiceOrigins(), empty()); } @Test diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java similarity index 89% rename from product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java rename to product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java index 84ce05d0..6108e39f 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/PluginLoaderFactoryIT.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java @@ -17,11 +17,11 @@ import org.opentest4j.TestAbortedException; /** - * Test for {@link PluginLoaderFactory} from module {@code core}. This test must - * be located in module {@code product} (which includes all plugin modules) so - * that it can access all plugin services. + * Test for {@link ServiceLoaderFactory} from module {@code core}. This test + * must be located in module {@code product} (which includes all plugin modules) + * so that it can access all plugin services. */ -class PluginLoaderFactoryIT +class ServiceLoaderFactoryIT { @TempDir Path tempDir; @@ -36,7 +36,7 @@ void loadServiceFromWrongJar() throws IOException private List loadService() { - return new PluginLoaderFactory(tempDir, false).createLoader(ReporterFactory.class).load().toList(); + return new ServiceLoaderFactory(tempDir, false).createLoader(ReporterFactory.class).load().toList(); } @Test From ff68cde363c0146b69bced4e3015a6e5dc6fc226 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 18:14:09 +0200 Subject: [PATCH 11/44] Fix unit test --- .../ClassPathServiceLoaderTest.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java index 9b191d69..365e9983 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java @@ -4,6 +4,7 @@ import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.util.*; @@ -45,15 +46,28 @@ void loadingReturnsService(@Mock final ServiceLoader serviceLoader final DummyServiceImpl service = new DummyServiceImpl(); when(serviceLoaderMock.stream()).thenReturn(Stream.of(providerMock)); when(providerMock.get()).thenReturn(service); + when(originMock.getClassLoader()).thenReturn(DummyServiceImpl.class.getClassLoader()); final List services = new ClassPathServiceLoader(originMock, serviceLoaderMock) - .load() - .toList(); + .load().toList(); assertAll(() -> assertThat(services, hasSize(1)), () -> assertThat(services.get(0), not(nullValue())), () -> assertThat(services.get(0), instanceOf(DummyServiceImpl.class)), () -> assertThat(services.get(0), sameInstance(service))); } + @Test + void loadingIgnoresServicesFromOtherClassLoaders(@Mock final ServiceLoader serviceLoaderMock, + @Mock final Provider providerMock, @Mock final ServiceOrigin originMock) + { + final DummyServiceImpl service = new DummyServiceImpl(); + when(serviceLoaderMock.stream()).thenReturn(Stream.of(providerMock)); + when(providerMock.get()).thenReturn(service); + when(originMock.getClassLoader()).thenReturn(mock(ClassLoader.class)); + final List services = new ClassPathServiceLoader(originMock, serviceLoaderMock) + .load().toList(); + assertThat(services, empty()); + } + static interface DummyService { } From 4319bb8b0de84a560bfa6a5c2b572d560ed88604 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 16 Jun 2024 19:03:30 +0200 Subject: [PATCH 12/44] Close classloader to fix test failure under windows --- .../serviceloader/ClassPathServiceLoader.java | 6 ++++ .../core/serviceloader/DelegatingLoader.java | 6 ++++ .../InitializingServiceLoader.java | 6 ++++ .../core/serviceloader/Loader.java | 5 ++- .../core/serviceloader/ServiceOrigin.java | 20 +++++++++-- .../serviceloader/ServiceLoaderFactoryIT.java | 34 ++++++++++++------- 6 files changed, 61 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index ffb5737f..b002cc48 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -41,4 +41,10 @@ private boolean filterOtherClassLoader(final T service) } return correctClassLoader; } + + @Override + public void close() + { + this.serviceOrigin.close(); + } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java index ab6e0f16..45cf2f9c 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java @@ -17,4 +17,10 @@ public Stream load() { return delegates.stream().flatMap(Loader::load); } + + @Override + public void close() + { + delegates.forEach(Loader::close); + } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java index 4ca74849..510e5f12 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java @@ -70,4 +70,10 @@ public Iterator iterator() { return load().iterator(); } + + @Override + public void close() + { + delegate.close(); + } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java index 45d0940a..9cb9eead 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java @@ -1,8 +1,11 @@ package org.itsallcode.openfasttrace.core.serviceloader; +import java.io.Closeable; import java.util.stream.Stream; -interface Loader +interface Loader extends Closeable { Stream load(); + + void close(); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 0d0d81cc..fd1c4b1f 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -6,9 +6,10 @@ import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; -final class ServiceOrigin +final class ServiceOrigin implements AutoCloseable { private final ClassLoader classLoader; private final List jars; @@ -16,7 +17,7 @@ final class ServiceOrigin private ServiceOrigin(final ClassLoader classLoader, final List jars) { this.classLoader = classLoader; - this.jars = jars; + this.jars = new ArrayList<>(jars); } public static ServiceOrigin forCurrentClassPath() @@ -74,4 +75,19 @@ public String toString() { return "ServiceOrigin [classLoader=" + classLoader + ", jars=" + jars + "]"; } + + public void close() + { + if (classLoader instanceof AutoCloseable) + { + try + { + ((AutoCloseable) classLoader).close(); + } + catch (final Exception e) + { + throw new IllegalStateException("Error closing class loader " + classLoader, e); + } + } + } } diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java index 6108e39f..54c12cb1 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java @@ -31,12 +31,16 @@ void loadServiceFromWrongJar() throws IOException { preparePlugin(Path.of("../reporter/plaintext/target"), "openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\-javadoc.jar"); - assertThat(loadService(), empty()); + try (Loader loader = createLoader()) + { + final List service = loader.load().toList(); + assertThat(service, empty()); + } } - private List loadService() + private Loader createLoader() { - return new ServiceLoaderFactory(tempDir, false).createLoader(ReporterFactory.class).load().toList(); + return new ServiceLoaderFactory(tempDir, false).createLoader(ReporterFactory.class); } @Test @@ -44,16 +48,20 @@ void loadServiceFromJar() throws IOException { preparePlugin(Path.of("../reporter/plaintext/target"), "openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\.jar"); - final List services = loadService(); - assertThat(services, hasSize(1)); - final ReporterFactory service = services.get(0); - final ClassLoader pluginClassLoader = service.getClass().getClassLoader(); - assertAll( - () -> assertThat(service.getClass().getName().toString(), - equalTo("org.itsallcode.openfasttrace.report.plaintext.PlaintextReporterFactory")), - () -> assertThat(pluginClassLoader.getName(), - startsWith("ServiceClassLoader-openfasttrace-reporter-plaintext")), - () -> assertThat(pluginClassLoader, not(sameInstance(Thread.currentThread().getContextClassLoader())))); + try (Loader loader = createLoader()) + { + final List services = loader.load().toList(); + assertThat(services, hasSize(1)); + final ReporterFactory service = services.get(0); + final ClassLoader pluginClassLoader = service.getClass().getClassLoader(); + assertAll( + () -> assertThat(service.getClass().getName().toString(), + equalTo("org.itsallcode.openfasttrace.report.plaintext.PlaintextReporterFactory")), + () -> assertThat(pluginClassLoader.getName(), + startsWith("ServiceClassLoader-openfasttrace-reporter-plaintext")), + () -> assertThat(pluginClassLoader, + not(sameInstance(Thread.currentThread().getContextClassLoader())))); + } } private void preparePlugin(final Path targetDir, final String filePattern) throws TestAbortedException, IOException From 710aa2b9d6c7091eb008e7e009197631901d5f23 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 17:50:12 +0200 Subject: [PATCH 13/44] Code cleanup --- .../openfasttrace/core/ServiceFactory.java | 6 +-- .../core/exporter/ExporterFactoryLoader.java | 15 +++--- .../core/importer/ImporterFactoryLoader.java | 19 +++---- .../core/report/ReporterFactoryLoader.java | 15 +++--- .../serviceloader/ChildFirstClassLoader.java | 51 ++++++++++++------- .../serviceloader/ClassPathServiceLoader.java | 4 ++ .../core/serviceloader/DelegatingLoader.java | 3 ++ .../InitializingServiceLoader.java | 2 +- .../core/serviceloader/Loader.java | 16 +++++- .../serviceloader/ServiceLoaderFactory.java | 32 +++++++++++- .../core/serviceloader/ServiceOrigin.java | 28 ++++++++-- .../importer/TestImporterFactoryLoader.java | 8 +-- .../tag/config/DescribedPathMatcherTest.java | 3 +- .../serviceloader/DelegatingLoaderTest.java | 16 ++++-- .../tag/LongTagImportingLineConsumer.java | 3 +- .../importer/tag/TagImporterFactory.java | 4 +- .../importer/tag/TestTagImporter.java | 3 +- .../openfasttrace/ITestSelfTrace.java | 4 +- .../TestInitializingServiceLoader.java | 7 ++- 19 files changed, 157 insertions(+), 82 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java index 3e1ee95e..5fcd2422 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java @@ -14,6 +14,7 @@ import org.itsallcode.openfasttrace.core.report.ReportService; import org.itsallcode.openfasttrace.core.report.ReporterFactoryLoader; import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; +import org.itsallcode.openfasttrace.core.serviceloader.Loader; class ServiceFactory { @@ -25,15 +26,14 @@ ExporterService createExporterService() ImporterService createImporterService(final ImportSettings settings) { final ImporterContext context = new ImporterContext(settings); - final InitializingServiceLoader serviceLoader = InitializingServiceLoader - .load(ImporterFactory.class, context); + final Loader serviceLoader = InitializingServiceLoader.load(ImporterFactory.class, context); final ImporterService service = new ImporterServiceImpl( new ImporterFactoryLoader(serviceLoader), settings); context.setImporterService(service); return service; } - Linker createLinker(List items) + Linker createLinker(final List items) { return new Linker(items); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java index 111ec8c2..43028ad4 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java @@ -1,13 +1,11 @@ package org.itsallcode.openfasttrace.core.exporter; -import static java.util.stream.Collectors.toList; - import java.nio.file.Path; import java.util.List; -import java.util.stream.StreamSupport; import org.itsallcode.openfasttrace.api.exporter.*; import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; +import org.itsallcode.openfasttrace.core.serviceloader.Loader; /** * This class is responsible for finding the matching {@link ExporterFactory} @@ -15,7 +13,7 @@ */ public class ExporterFactoryLoader { - private final InitializingServiceLoader serviceLoader; + private final Loader serviceLoader; /** * Create a new loader for the given context. @@ -28,8 +26,7 @@ public ExporterFactoryLoader(final ExporterContext context) this(InitializingServiceLoader.load(ExporterFactory.class, context)); } - ExporterFactoryLoader( - final InitializingServiceLoader serviceLoader) + ExporterFactoryLoader(final Loader serviceLoader) { this.serviceLoader = serviceLoader; } @@ -64,9 +61,9 @@ public ExporterFactory getExporterFactory(final String outputFormat) private List getMatchingFactories(final String format) { - return StreamSupport.stream(this.serviceLoader.spliterator(), false) // - .filter(f -> f.supportsFormat(format)) // - .collect(toList()); + return this.serviceLoader.load() + .filter(f -> f.supportsFormat(format)) + .toList(); } /** 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 45adacb3..21c44013 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 @@ -1,16 +1,14 @@ package org.itsallcode.openfasttrace.core.importer; -import static java.util.stream.Collectors.toList; - import java.nio.file.Path; import java.util.List; import java.util.Optional; import java.util.logging.Logger; -import java.util.stream.StreamSupport; -import org.itsallcode.openfasttrace.api.importer.*; +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.core.serviceloader.InitializingServiceLoader; +import org.itsallcode.openfasttrace.core.serviceloader.Loader; /** * This class is responsible for finding the matching {@link ImporterFactory} @@ -20,7 +18,7 @@ public class ImporterFactoryLoader { private static final Logger LOG = Logger.getLogger(ImporterFactoryLoader.class.getName()); - private final InitializingServiceLoader serviceLoader; + private final Loader serviceLoader; /** * Creates a new loader. @@ -28,8 +26,7 @@ public class ImporterFactoryLoader * @param serviceLoader * the loader used for locating importers. */ - public ImporterFactoryLoader( - final InitializingServiceLoader serviceLoader) + public ImporterFactoryLoader(final Loader serviceLoader) { this.serviceLoader = serviceLoader; } @@ -79,8 +76,8 @@ public boolean supportsFile(final InputFile file) private List getMatchingFactories(final InputFile file) { - return StreamSupport.stream(this.serviceLoader.spliterator(), false) // - .filter(f -> f.supportsFile(file)) // - .collect(toList()); + return this.serviceLoader.load() + .filter(f -> f.supportsFile(file)) + .toList(); } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java index 73f57c0c..ce81e8fe 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java @@ -1,13 +1,11 @@ package org.itsallcode.openfasttrace.core.report; -import static java.util.stream.Collectors.toList; - import java.util.List; -import java.util.stream.StreamSupport; import org.itsallcode.openfasttrace.api.exporter.ExporterException; import org.itsallcode.openfasttrace.api.report.*; import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; +import org.itsallcode.openfasttrace.core.serviceloader.Loader; /** * This class is responsible for finding the matching {@link ReporterFactory} @@ -15,7 +13,7 @@ */ public class ReporterFactoryLoader { - private final InitializingServiceLoader serviceLoader; + private final Loader serviceLoader; /** * Create a new {@link ReporterFactoryLoader}. @@ -29,8 +27,7 @@ public ReporterFactoryLoader(final ReporterContext context) this(InitializingServiceLoader.load(ReporterFactory.class, context)); } - private ReporterFactoryLoader( - final InitializingServiceLoader serviceLoader) + private ReporterFactoryLoader(final Loader serviceLoader) { this.serviceLoader = serviceLoader; } @@ -65,9 +62,9 @@ public ReporterFactory getReporterFactory(final String outputFormat) private List getMatchingFactories(final String format) { - return StreamSupport.stream(this.serviceLoader.spliterator(), false) // - .filter(factory -> factory.supportsFormat(format)) // - .collect(toList()); + return this.serviceLoader.load() + .filter(factory -> factory.supportsFormat(format)) + .toList(); } /** diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java index 870e0503..917e2807 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java @@ -8,6 +8,9 @@ * then from the parent class loader, unlike {@link URLClassLoader} which does * it the other way around. *

+ * This allows us to prefer externl plugins over plugins on the classpath + * included with OFT. + *

* This is based on * https://medium.com/@isuru89/java-a-child-first-class-loader-cbd9c3d0305 *

@@ -22,28 +25,38 @@ class ChildFirstClassLoader extends URLClassLoader @Override protected Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException { - // has the class loaded already? - Class loadedClass = findLoadedClass(name); - if (loadedClass == null) - { - try - { - // find the class from given jar urls - loadedClass = findClass(name); - } - catch (final ClassNotFoundException ignore) - { - // Hmmm... class does not exist in the given urls. - // Let's try finding it in our parent classloader. - // this'll throw ClassNotFoundException in failure. - loadedClass = super.loadClass(name, resolve); - } - } - + final Class loadedClass = findClass(name, resolve); if (resolve) - { // marked to resolve + { resolveClass(loadedClass); } return loadedClass; } + + private Class findClass(final String name, final boolean resolve) throws ClassNotFoundException + { + // Has the class loaded already? + final Class loadedClass = findLoadedClass(name); + if (loadedClass != null) + { + return loadedClass; + } + return loadClassInternally(name, resolve); + } + + private Class loadClassInternally(final String name, final boolean resolve) throws ClassNotFoundException + { + try + { + // Find the class from given jar urls + return findClass(name); + } + catch (final ClassNotFoundException ignore) + { + // Class does not exist in the given urls. + // Let's try finding it in our parent classloader. + // This will throw ClassNotFoundException on failure. + return super.loadClass(name, resolve); + } + } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index b002cc48..2063c299 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -5,6 +5,10 @@ import java.util.logging.Logger; import java.util.stream.Stream; +/** + * A service loader that loads services from the classpath using a given + * {@link ServiceOrigin}. + */ final class ClassPathServiceLoader implements Loader { private static final Logger LOGGER = Logger.getLogger(ClassPathServiceLoader.class.getName()); diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java index 45cf2f9c..d1e45cb1 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java @@ -3,6 +3,9 @@ import java.util.List; import java.util.stream.Stream; +/** + * A service loader that delegates to a list of other loaders. + */ class DelegatingLoader implements Loader { private final List> delegates; diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java index 510e5f12..9e31467e 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java @@ -40,7 +40,7 @@ private InitializingServiceLoader(final Loader delegate, final C context) * instances. * @return an {@link InitializingServiceLoader} for type T */ - public static , C> InitializingServiceLoader load( + public static , C> Loader load( final Class serviceType, final C context) { final ServiceLoaderFactory loaderFactory = ServiceLoaderFactory.createDefault(); diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java index 9cb9eead..efdf4147 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/Loader.java @@ -3,9 +3,23 @@ import java.io.Closeable; import java.util.stream.Stream; -interface Loader extends Closeable +/** + * A loader for services, similar to Java's {@link java.util.ServiceLoader}. + * + * @param + * service type + */ +public interface Loader extends Closeable { + /** + * Load services. + * + * @return a stream of services + */ Stream load(); + /** + * Close the loader and potentially the underlying ClassLoader. + */ void close(); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index dfcf30f9..36b89599 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -6,7 +6,11 @@ import java.nio.file.Path; import java.util.*; import java.util.logging.Logger; +import java.util.stream.Stream; +/** + * Factory for creating {@link Loader} instances for services. + */ class ServiceLoaderFactory { private static final Logger LOGGER = Logger.getLogger(ServiceLoaderFactory.class.getName()); @@ -28,6 +32,11 @@ class ServiceLoaderFactory this.searchCurrentClasspath = searchCurrentClasspath; } + /** + * Create a default factory that searches for plugins in the user's home. + * + * @return a default factory + */ static ServiceLoaderFactory createDefault() { return new ServiceLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins"), true); @@ -38,6 +47,15 @@ private static Path getHomeDirectory() return Path.of(System.getProperty("user.home")); } + /** + * Create a new {@link Loader} for the given service type. + * + * @param + * service type + * @param serviceType + * service type + * @return a new {@link Loader} for the given service type + */ Loader createLoader(final Class serviceType) { return createLoader(serviceType, findServiceOrigins()); @@ -51,6 +69,13 @@ private Loader createLoader(final Class serviceType, final List(loaders); } + /** + * Find all service origins. + *

+ * This method is package-private for testing. + * + * @return a list of service origins + */ List findServiceOrigins() { final List origins = new ArrayList<>(); @@ -63,7 +88,7 @@ List findServiceOrigins() return origins; } - Collection findPluginOrigins() + private Collection findPluginOrigins() { if (!Files.isDirectory(pluginsDirectory)) { @@ -111,7 +136,10 @@ private static List findJarsInDir(final Path path) { try { - return Files.list(path).filter(ServiceLoaderFactory::isJarFile).toList(); + try (Stream stream = Files.list(path)) + { + return stream.filter(ServiceLoaderFactory::isJarFile).toList(); + } } catch (final IOException exception) { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index fd1c4b1f..b0a1749c 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -9,6 +9,9 @@ import java.util.ArrayList; import java.util.List; +/** + * Origin of a service, either the current classpath or a list of jar files. + */ final class ServiceOrigin implements AutoCloseable { private final ClassLoader classLoader; @@ -20,7 +23,12 @@ private ServiceOrigin(final ClassLoader classLoader, final List jars) this.jars = new ArrayList<>(jars); } - public static ServiceOrigin forCurrentClassPath() + /** + * Create a service origin for the current classpath. + * + * @return a service origin for the current classpath + */ + static ServiceOrigin forCurrentClassPath() { return new ServiceOrigin(getBaseClassLoader(), emptyList()); } @@ -30,12 +38,26 @@ private static ClassLoader getBaseClassLoader() return Thread.currentThread().getContextClassLoader(); } - public static ServiceOrigin forJar(final Path jar) + /** + * Create a service origin for a single jar file. + * + * @param jar + * path to the jar file + * @return a service origin for the jar file + */ + static ServiceOrigin forJar(final Path jar) { return forJars(List.of(jar)); } - public static ServiceOrigin forJars(final List jars) + /** + * Create a service origin for a list of jar files. + * + * @param jars + * list of paths to jar files + * @return a service origin for the jar files + */ + static ServiceOrigin forJars(final List jars) { return new ServiceOrigin(createClassLoader(jars), jars); } 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 5979e4d5..4b2b6e0b 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,5 @@ package org.itsallcode.openfasttrace.core.importer; -import static java.util.Arrays.asList; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -9,12 +8,13 @@ import static org.mockito.Mockito.when; import java.nio.file.Paths; +import java.util.Arrays; import org.itsallcode.openfasttrace.api.importer.ImporterContext; import org.itsallcode.openfasttrace.api.importer.ImporterFactory; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; -import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; +import org.itsallcode.openfasttrace.core.serviceloader.Loader; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -28,7 +28,7 @@ class TestImporterFactoryLoader { @Mock - private InitializingServiceLoader serviceLoaderMock; + private Loader serviceLoaderMock; @Mock private ImporterFactory supportedFactory1; @Mock @@ -87,6 +87,6 @@ private void assertFactoryFound(final ImporterFactory expectedFactory) private void simulateFactories(final ImporterFactory... factories) { - when(this.serviceLoaderMock.spliterator()).thenReturn(asList(factories).spliterator()); + when(this.serviceLoaderMock.load()).thenReturn(Arrays.stream(factories)); } } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/importer/tag/config/DescribedPathMatcherTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/tag/config/DescribedPathMatcherTest.java index f5548cbf..682dc092 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/importer/tag/config/DescribedPathMatcherTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/importer/tag/config/DescribedPathMatcherTest.java @@ -1,6 +1,5 @@ package org.itsallcode.openfasttrace.core.importer.tag.config; -import static java.util.stream.Collectors.toList; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.notNullValue; @@ -166,7 +165,7 @@ private void createListMatcher(final String... paths) { final List pathList = Arrays.stream(paths) // .map(Paths::get) // - .collect(toList()); + .toList(); this.matcher = DescribedPathMatcher.createPathListMatcher(pathList); } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java index f528a0e6..2af1775e 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java @@ -19,21 +19,21 @@ class DelegatingLoaderTest @Test void noDelegates() { - assertThat(new DelegatingLoader<>(List.of()).load().toList(), empty()); + assertThat(load(List.of()), empty()); } @Test void emptyDelegate(@Mock final Loader loaderMock) { when(loaderMock.load()).thenReturn(Stream.empty()); - assertThat(new DelegatingLoader<>(List.of(loaderMock)).load().toList(), empty()); + assertThat(load(List.of(loaderMock)), empty()); } @Test void nonEmptyDelegate(@Mock final Loader loaderMock, @Mock final DummyService serviceMock) { when(loaderMock.load()).thenReturn(Stream.of(serviceMock)); - assertThat(new DelegatingLoader<>(List.of(loaderMock)).load().toList(), contains(serviceMock)); + assertThat(load(List.of(loaderMock)), contains(serviceMock)); } @Test @@ -43,10 +43,18 @@ void loadsDelegatesInOrder(@Mock final Loader loaderMock1, { when(loaderMock1.load()).thenReturn(Stream.of(serviceMock1)); when(loaderMock2.load()).thenReturn(Stream.of(serviceMock2)); - assertThat(new DelegatingLoader<>(List.of(loaderMock1, loaderMock2)).load().toList(), + assertThat(load(List.of(loaderMock1, loaderMock2)), contains(serviceMock1, serviceMock2)); } + private List load(final List> delegates) + { + try (DelegatingLoader loader = new DelegatingLoader(delegates)) + { + return loader.load().toList(); + } + } + static interface DummyService { } diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java index d526730b..17853ce2 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java +++ b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java @@ -1,7 +1,6 @@ package org.itsallcode.openfasttrace.importer.tag; import static java.util.Collections.emptyList; -import static java.util.stream.Collectors.toList; import java.util.*; import java.util.function.Predicate; @@ -75,7 +74,7 @@ private List parseCommaSeparatedList(final String input) return Arrays.stream(input.split(",")) .map(String::trim) .filter(Predicate.not(String::isEmpty)) - .collect(toList()); + .toList(); } private SpecificationItemId createItemId(final Matcher matcher, final int lineNumber, final int lineMatchCount, diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporterFactory.java b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporterFactory.java index c9ab2e6c..81ed889f 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporterFactory.java +++ b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporterFactory.java @@ -1,7 +1,5 @@ package org.itsallcode.openfasttrace.importer.tag; -import static java.util.stream.Collectors.toList; - import java.util.*; import java.util.stream.Stream; @@ -91,7 +89,7 @@ public Importer createImporter(final InputFile path, final ImportEventListener l { throw new ImporterException("File '" + path + "' cannot be imported because it does not match any supported file patterns: " - + DEFAULT_FILE_REGEX + " and " + getPathConfigs().collect(toList())); + + DEFAULT_FILE_REGEX + " and " + getPathConfigs().toList()); } final Optional config = findConfig(path); return TagImporter.create(config, path, listener); diff --git a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java b/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java index 0454ffc8..f7017f6c 100644 --- a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java +++ b/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java @@ -1,7 +1,6 @@ package org.itsallcode.openfasttrace.importer.tag; import static java.util.Collections.emptyList; -import static java.util.stream.Collectors.toList; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; @@ -229,7 +228,7 @@ private static Arguments parsedItems(final String content, final SpecificationIt final List expectedItems = Arrays.stream(itemBuilders) .map(TestTagImporter::setLocation) .map(SpecificationItem.Builder::build) - .collect(toList()); + .toList(); return Arguments.of(content, expectedItems); } diff --git a/product/src/test/java/org/itsallcode/openfasttrace/ITestSelfTrace.java b/product/src/test/java/org/itsallcode/openfasttrace/ITestSelfTrace.java index f9a2e705..a3d3938d 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/ITestSelfTrace.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/ITestSelfTrace.java @@ -1,7 +1,5 @@ package org.itsallcode.openfasttrace; -import static java.util.stream.Collectors.toList; - import java.io.*; import java.nio.file.*; import java.util.List; @@ -80,7 +78,7 @@ private List findInputDirectories(final Path rootProjectDir) .filter(path -> Files.isDirectory(path)) // .filter(endsWith(Paths.get("src/main")) // .or(endsWith(Paths.get("src/test/java")))) // - .collect(toList()); + .toList(); } catch (final IOException e) { diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java index 24d64e91..59179f03 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java @@ -29,11 +29,11 @@ class TestInitializingServiceLoader void testNoServicesRegistered() { final Object context = new Object(); - final InitializingServiceLoader voidServiceLoader = InitializingServiceLoader + final Loader voidServiceLoader = InitializingServiceLoader .load(InitializableServiceStub.class, context); final List services = voidServiceLoader.load().toList(); assertThat(services, emptyIterable()); - assertThat(voidServiceLoader, emptyIterable()); + assertThat(voidServiceLoader.load().toList(), emptyIterable()); } @Test @@ -72,8 +72,7 @@ void testExporterFactoriesRegistered() private , C> List getRegisteredServices(final Class type, final C context) { - final InitializingServiceLoader serviceLoader = InitializingServiceLoader.load(type, - context); + final Loader serviceLoader = InitializingServiceLoader.load(type, context); return serviceLoader.load().toList(); } From 098188f032ece9bbb9464e3fe045c70c8c26100c Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 17:59:58 +0200 Subject: [PATCH 14/44] Fix sonar findings --- .../core/serviceloader/DelegatingLoader.java | 3 ++- .../core/serviceloader/ServiceLoaderFactory.java | 16 +++++++++------- .../core/serviceloader/ServiceOrigin.java | 9 +++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java index d1e45cb1..b64684b0 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoader.java @@ -1,5 +1,6 @@ package org.itsallcode.openfasttrace.core.serviceloader; +import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; @@ -12,7 +13,7 @@ class DelegatingLoader implements Loader DelegatingLoader(final List> delegates) { - this.delegates = delegates; + this.delegates = new ArrayList<>(delegates); } @Override diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index 36b89599..4b984485 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -61,7 +61,7 @@ Loader createLoader(final Class serviceType) return createLoader(serviceType, findServiceOrigins()); } - private Loader createLoader(final Class serviceType, final List origins) + private static Loader createLoader(final Class serviceType, final List origins) { final List> loaders = origins.stream() .map(origin -> ClassPathServiceLoader.create(serviceType, origin)) @@ -96,11 +96,13 @@ private Collection findPluginOrigins() } try { - return Files.list(pluginsDirectory) - .sorted() - .map(this::originForPath) - .flatMap(Optional::stream) - .toList(); + try (Stream stream = Files.list(pluginsDirectory)) + { + return stream.sorted() + .map(ServiceLoaderFactory::originForPath) + .flatMap(Optional::stream) + .toList(); + } } catch (final IOException exception) { @@ -110,7 +112,7 @@ private Collection findPluginOrigins() } } - private Optional originForPath(final Path path) + private static Optional originForPath(final Path path) { if (isJarFile(path)) { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index b0a1749c..94b7b7c5 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -98,17 +98,18 @@ public String toString() return "ServiceOrigin [classLoader=" + classLoader + ", jars=" + jars + "]"; } + @Override public void close() { - if (classLoader instanceof AutoCloseable) + if (classLoader instanceof final AutoCloseable closable) { try { - ((AutoCloseable) classLoader).close(); + closable.close(); } - catch (final Exception e) + catch (final Exception exception) { - throw new IllegalStateException("Error closing class loader " + classLoader, e); + throw new IllegalStateException("Error closing class loader " + classLoader, exception); } } } From af238008d3d7e7d4f0b05ab72c5ba1a01d7ab717 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 18:15:10 +0200 Subject: [PATCH 15/44] Improve test coverage --- .../serviceloader/ClassPathServiceLoader.java | 5 +-- .../InitializingServiceLoader.java | 11 ++----- .../core/serviceloader/ServiceOrigin.java | 2 +- .../ServiceLoaderFactoryTest.java | 19 +++++++++++ .../core/serviceloader/ServiceOriginTest.java | 32 +++++++++++++++++++ 5 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index 2063c299..f34d7394 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -40,8 +40,9 @@ private boolean filterOtherClassLoader(final T service) final boolean correctClassLoader = serviceClassLoader == serviceOrigin.getClassLoader(); if (!correctClassLoader) { - LOGGER.fine(() -> "Service " + service + " has wrong class loader: " + serviceClassLoader + ", expected " - + serviceOrigin.getClassLoader()); + LOGGER.finest( + () -> "Service " + service + " has unexpected class loader: " + serviceClassLoader + ", expected " + + serviceOrigin.getClassLoader() + ". This service will not be used."); } return correctClassLoader; } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java index 9e31467e..57b8631c 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java @@ -1,6 +1,7 @@ package org.itsallcode.openfasttrace.core.serviceloader; -import java.util.*; +import java.util.List; +import java.util.ServiceLoader; import java.util.stream.Stream; import org.itsallcode.openfasttrace.api.core.serviceloader.Initializable; @@ -14,7 +15,7 @@ * @param * the context type */ -public final class InitializingServiceLoader, C> implements Iterable, Loader +public final class InitializingServiceLoader, C> implements Loader { private final Loader delegate; private final C context; @@ -65,12 +66,6 @@ private List loadServices() }).toList(); } - @Override - public Iterator iterator() - { - return load().iterator(); - } - @Override public void close() { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 94b7b7c5..8b05dfb9 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -17,7 +17,7 @@ final class ServiceOrigin implements AutoCloseable private final ClassLoader classLoader; private final List jars; - private ServiceOrigin(final ClassLoader classLoader, final List jars) + ServiceOrigin(final ClassLoader classLoader, final List jars) { this.classLoader = classLoader; this.jars = new ArrayList<>(jars); diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 1af758c8..5ec72c02 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -67,6 +67,25 @@ void findServiceOriginsEmptyPluginDir() throws IOException assertNoPlugins(origins); } + @Test + void findServiceOriginsIgnoresNonJarFiles() throws IOException + { + final Path pluginDir = tempDir.resolve("plugin1"); + Files.createDirectories(pluginDir); + Files.createFile(pluginDir.resolve("plugin1.txt")); + final List origins = testee().findServiceOrigins(); + assertNoPlugins(origins); + } + + @Test + void findServiceOriginsIgnoresDirectories() throws IOException + { + final Path pluginDir = tempDir.resolve("plugin1"); + Files.createDirectories(pluginDir.resolve("ignored-directory")); + final List origins = testee().findServiceOrigins(); + assertNoPlugins(origins); + } + @Test void findServiceOriginsSingleJar() throws IOException { diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java new file mode 100644 index 00000000..53a9ba8a --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java @@ -0,0 +1,32 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.mockito.Mockito.*; + +import java.util.Collections; + +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 ServiceOriginTest +{ + @Test + void closesClassLoader(@Mock(extraInterfaces = AutoCloseable.class) final ClassLoader classLoaderMock) + throws Exception + { + final ServiceOrigin serviceOrigin = new ServiceOrigin(classLoaderMock, Collections.emptyList()); + serviceOrigin.close(); + verify(((AutoCloseable) classLoaderMock)).close(); + verifyNoMoreInteractions(classLoaderMock); + } + + @Test + void doesNotCloseClassLoader(@Mock final ClassLoader classLoaderMock) + { + final ServiceOrigin serviceOrigin = new ServiceOrigin(classLoaderMock, Collections.emptyList()); + serviceOrigin.close(); + verifyNoInteractions(classLoaderMock); + } +} From dc4ce5876064fcbc72e613c22de522936fbdf78a Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 18:19:02 +0200 Subject: [PATCH 16/44] Update dependencies --- parent/pom.xml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/parent/pom.xml b/parent/pom.xml index 85e996d0..7b4a112e 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -292,7 +292,7 @@ io.github.git-commit-id git-commit-id-maven-plugin - 8.0.2 + 9.0.0 get-the-git-infos @@ -460,7 +460,7 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.4.1 + 3.5.0 enforce-maven @@ -542,7 +542,7 @@ org.apache.maven.plugins maven-clean-plugin - 3.3.2 + 3.4.0 org.apache.maven.plugins @@ -557,12 +557,12 @@ org.apache.maven.plugins maven-site-plugin - 4.0.0-M13 + 4.0.0-M15 org.apache.maven.plugins maven-shade-plugin - 3.5.3 + 3.6.0 org.apache.maven.plugins @@ -577,7 +577,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.6.3 + 3.7.0 org.codehaus.mojo From feda1507f2df138fcf9d6ee6c9d78d2558ee04c4 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 19:25:11 +0200 Subject: [PATCH 17/44] Add unit tests --- .../ChildFirstClassLoaderTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java new file mode 100644 index 00000000..d0e08720 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java @@ -0,0 +1,86 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Enumeration; + +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; + +class ChildFirstClassLoaderTest +{ + @Test + void parentFindsClass() throws ClassNotFoundException + { + final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, + new MockClassLoader(String.class)); + assertThat(testee.loadClass("ClassName"), sameInstance(String.class)); + } + + @Test + void parentDoesNotFindClass() throws ClassNotFoundException + { + final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, + new MockClassLoader(null)); + final ClassNotFoundException exception = assertThrows(ClassNotFoundException.class, + () -> testee.loadClass("ClassName")); + assertThat(exception.getMessage(), equalTo("ClassName")); + } + + @Test + void classFoundInJar() throws ClassNotFoundException, IOException + { + final Class classToFind = Matchers.class; + final URL jarForClass = findJarForClass(classToFind.getName()); + final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] { jarForClass }, + Thread.currentThread().getContextClassLoader()); + final Class foundClass = testee.loadClass(classToFind.getName()); + assertThat(foundClass, not(sameInstance(classToFind))); + assertThat(foundClass.toGenericString(), equalTo(classToFind.toGenericString())); + } + + public static URL findJarForClass(final String className) throws IOException + { + final String classAsResource = className.replace('.', '/') + ".class"; + final Enumeration urls = Thread.currentThread().getContextClassLoader().getResources(classAsResource); + while (urls.hasMoreElements()) + { + final URL url = urls.nextElement(); + if ("jar".equals(url.getProtocol())) + { + final String jarPath = url.getPath().substring(5, + url.getPath().indexOf("!")); + final Path path = Path.of(jarPath); + assertTrue(Files.exists(path)); + return path.toUri().toURL(); + } + } + throw new AssertionError("No jar found containing " + className); + } + + private static class MockClassLoader extends ClassLoader + { + private final Class clazz; + + private MockClassLoader(final Class clazz) + { + this.clazz = clazz; + } + + public Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException + { + if (clazz == null) + { + throw new ClassNotFoundException("mock"); + } + return clazz; + } + } +} From 0cd925e7063d8c97baef0e005a2fdef8b0ff9802 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 19:33:02 +0200 Subject: [PATCH 18/44] Fix test for windows --- .../core/serviceloader/ChildFirstClassLoaderTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java index d0e08720..a88f801a 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java @@ -13,6 +13,7 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.OS; class ChildFirstClassLoaderTest { @@ -55,8 +56,12 @@ public static URL findJarForClass(final String className) throws IOException final URL url = urls.nextElement(); if ("jar".equals(url.getProtocol())) { - final String jarPath = url.getPath().substring(5, - url.getPath().indexOf("!")); + String jarPath = url.getPath().substring(5, url.getPath().indexOf("!")); + if (OS.WINDOWS.isCurrentOs()) + { + // Remove leading slash of "/C:/Users/user/.m2/..." + jarPath = jarPath.substring(1); + } final Path path = Path.of(jarPath); assertTrue(Files.exists(path)); return path.toUri().toURL(); From e1f51b43ee3d0999e3d33c1d734b59af93df5e0f Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 19:33:25 +0200 Subject: [PATCH 19/44] Don't fail fast --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1bfc60d4..dbaf6598 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,7 +11,7 @@ jobs: contents: read strategy: - fail-fast: true + fail-fast: false matrix: java: [17] os: [ubuntu-latest, macos-latest, windows-latest] From 4e8e660b21a0adf0d77270f68a5d88187aa7d721 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 19:34:52 +0200 Subject: [PATCH 20/44] Fix sonar warning --- .../core/serviceloader/ChildFirstClassLoaderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java index a88f801a..7d9d31d5 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java @@ -26,7 +26,7 @@ void parentFindsClass() throws ClassNotFoundException } @Test - void parentDoesNotFindClass() throws ClassNotFoundException + void parentDoesNotFindClass() { final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, new MockClassLoader(null)); From 7dc0b082aed038fb7b5c29b73cff6523de39aa90 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 19:44:32 +0200 Subject: [PATCH 21/44] Add unit tests --- .../InitializingServiceLoader.java | 2 +- .../InitializingServiceLoaderTest.java | 47 +++++++++++++++++++ .../core/serviceloader/ServiceMock.java | 6 +++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoaderTest.java create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java index 57b8631c..805a3b49 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoader.java @@ -21,7 +21,7 @@ public final class InitializingServiceLoader, C> impl private final C context; private List services; - private InitializingServiceLoader(final Loader delegate, final C context) + InitializingServiceLoader(final Loader delegate, final C context) { this.delegate = delegate; this.context = context; diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoaderTest.java new file mode 100644 index 00000000..3310a078 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/InitializingServiceLoaderTest.java @@ -0,0 +1,47 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.stream.Stream; + +import org.itsallcode.openfasttrace.api.core.serviceloader.Initializable; +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 InitializingServiceLoaderTest +{ + @Mock + Loader delegateMock; + @Mock + Object contextMock; + @Mock + ServiceMock serviceInstanceMock; + + @Test + void initializesServices() + { + when(delegateMock.load()).thenReturn(Stream.of(serviceInstanceMock)); + try (Loader loader = new InitializingServiceLoader<>(delegateMock, contextMock)) + { + loader.load(); + } + verify(serviceInstanceMock).init(same(contextMock)); + } + + @Test + void closesDelegate() + { + new InitializingServiceLoader<>(delegateMock, contextMock).close(); + verify(delegateMock).close(); + } + + private interface ServiceMock extends Initializable + { + + } +} diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java new file mode 100644 index 00000000..0ec065c4 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java @@ -0,0 +1,6 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +public @interface ServiceMock +{ + +} From 117ffe86b01d1c48a84ad4386f1f5670f11f37cb Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Fri, 21 Jun 2024 19:53:48 +0200 Subject: [PATCH 22/44] Code cleanup --- .../ChildFirstClassLoaderTest.java | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java index 7d9d31d5..6f471fec 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java @@ -18,21 +18,25 @@ class ChildFirstClassLoaderTest { @Test - void parentFindsClass() throws ClassNotFoundException + void parentFindsClass() throws ClassNotFoundException, IOException { - final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, - new MockClassLoader(String.class)); - assertThat(testee.loadClass("ClassName"), sameInstance(String.class)); + try (final ChildFirstClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, + new MockClassLoader(String.class))) + { + assertThat(testee.loadClass("ClassName"), sameInstance(String.class)); + } } @Test - void parentDoesNotFindClass() + void parentDoesNotFindClass() throws IOException { - final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, - new MockClassLoader(null)); - final ClassNotFoundException exception = assertThrows(ClassNotFoundException.class, - () -> testee.loadClass("ClassName")); - assertThat(exception.getMessage(), equalTo("ClassName")); + try (final ChildFirstClassLoader testee = new ChildFirstClassLoader("name", new URL[] {}, + new MockClassLoader(null))) + { + final ClassNotFoundException exception = assertThrows(ClassNotFoundException.class, + () -> testee.loadClass("ClassName")); + assertThat(exception.getMessage(), equalTo("ClassName")); + } } @Test @@ -40,11 +44,13 @@ void classFoundInJar() throws ClassNotFoundException, IOException { final Class classToFind = Matchers.class; final URL jarForClass = findJarForClass(classToFind.getName()); - final ClassLoader testee = new ChildFirstClassLoader("name", new URL[] { jarForClass }, - Thread.currentThread().getContextClassLoader()); - final Class foundClass = testee.loadClass(classToFind.getName()); - assertThat(foundClass, not(sameInstance(classToFind))); - assertThat(foundClass.toGenericString(), equalTo(classToFind.toGenericString())); + try (final ChildFirstClassLoader testee = new ChildFirstClassLoader("name", new URL[] { jarForClass }, + Thread.currentThread().getContextClassLoader())) + { + final Class foundClass = testee.loadClass(classToFind.getName()); + assertThat(foundClass, not(sameInstance(classToFind))); + assertThat(foundClass.toGenericString(), equalTo(classToFind.toGenericString())); + } } public static URL findJarForClass(final String className) throws IOException @@ -70,6 +76,10 @@ public static URL findJarForClass(final String className) throws IOException throw new AssertionError("No jar found containing " + className); } + /** + * We can't mock the ClassLoader using Mockito because method + * {@link ClassLoader#loadClass(String, boolean)} is protected. + */ private static class MockClassLoader extends ClassLoader { private final Class clazz; @@ -79,7 +89,8 @@ private MockClassLoader(final Class clazz) this.clazz = clazz; } - public Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException + @Override + protected Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException { if (clazz == null) { From e6b3eeae1a91426e9562585aa503f9fbf66e57f7 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 22 Jun 2024 14:01:40 +0200 Subject: [PATCH 23/44] Refactor ServiceOrigin --- .../core/serviceloader/ServiceOrigin.java | 98 +++++++++++++------ .../ChildFirstClassLoaderTest.java | 30 +----- .../core/serviceloader/ClassPathHelper.java | 73 ++++++++++++++ .../core/serviceloader/ServiceMock.java | 2 +- .../core/serviceloader/ServiceOriginTest.java | 54 ++++++++-- 5 files changed, 190 insertions(+), 67 deletions(-) create mode 100644 core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 8b05dfb9..61419c21 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -1,28 +1,16 @@ package org.itsallcode.openfasttrace.core.serviceloader; -import static java.util.Collections.emptyList; import static java.util.stream.Collectors.joining; -import java.net.MalformedURLException; -import java.net.URL; +import java.net.*; import java.nio.file.Path; -import java.util.ArrayList; import java.util.List; /** * Origin of a service, either the current classpath or a list of jar files. */ -final class ServiceOrigin implements AutoCloseable +interface ServiceOrigin extends AutoCloseable { - private final ClassLoader classLoader; - private final List jars; - - ServiceOrigin(final ClassLoader classLoader, final List jars) - { - this.classLoader = classLoader; - this.jars = new ArrayList<>(jars); - } - /** * Create a service origin for the current classpath. * @@ -30,7 +18,7 @@ final class ServiceOrigin implements AutoCloseable */ static ServiceOrigin forCurrentClassPath() { - return new ServiceOrigin(getBaseClassLoader(), emptyList()); + return new CurrentClassPathOrigin(getBaseClassLoader()); } private static ClassLoader getBaseClassLoader() @@ -59,10 +47,10 @@ static ServiceOrigin forJar(final Path jar) */ static ServiceOrigin forJars(final List jars) { - return new ServiceOrigin(createClassLoader(jars), jars); + return new JarFileOrigin(jars, createClassLoader(jars)); } - private static ClassLoader createClassLoader(final List jars) + private static URLClassLoader createClassLoader(final List jars) { final URL[] urls = jars.stream().map(ServiceOrigin::toUrl) .toArray(URL[]::new); @@ -87,30 +75,80 @@ private static URL toUrl(final Path path) } } - public ClassLoader getClassLoader() - { - return classLoader; - } + /** + * Get the class loader for this service origin. + * + * @return the class loader for this service origin + */ + ClassLoader getClassLoader(); - @Override - public String toString() - { - return "ServiceOrigin [classLoader=" + classLoader + ", jars=" + jars + "]"; - } + /** + * Close the underlying ClassLoader if appropriate. + */ + void close(); - @Override - public void close() + static final class JarFileOrigin implements ServiceOrigin { - if (classLoader instanceof final AutoCloseable closable) + private final List jars; + private final URLClassLoader classLoader; + + JarFileOrigin(final List jars, final URLClassLoader classLoader) + { + this.jars = jars; + this.classLoader = classLoader; + } + + @Override + public ClassLoader getClassLoader() + { + return classLoader; + } + + @Override + public void close() { try { - closable.close(); + classLoader.close(); } catch (final Exception exception) { throw new IllegalStateException("Error closing class loader " + classLoader, exception); } } + + @Override + public String toString() + { + return "JarFileOrigin [classLoader=" + classLoader + ", jars=" + jars + "]"; + } + } + + static final class CurrentClassPathOrigin implements ServiceOrigin + { + private final ClassLoader classLoader; + + CurrentClassPathOrigin(final ClassLoader classLoader) + { + this.classLoader = classLoader; + } + + @Override + public ClassLoader getClassLoader() + { + return classLoader; + } + + @Override + public void close() + { + // The current class loader cannot be closed. + } + + @Override + public String toString() + { + return "CurrentClassPathOrigin [classLoader=" + classLoader + "]"; + } } } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java index 6f471fec..d01a1ca4 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java @@ -3,17 +3,12 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.net.URL; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Enumeration; import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.OS; class ChildFirstClassLoaderTest { @@ -43,7 +38,7 @@ void parentDoesNotFindClass() throws IOException void classFoundInJar() throws ClassNotFoundException, IOException { final Class classToFind = Matchers.class; - final URL jarForClass = findJarForClass(classToFind.getName()); + final URL jarForClass = ClassPathHelper.findUrlForClass(classToFind); try (final ChildFirstClassLoader testee = new ChildFirstClassLoader("name", new URL[] { jarForClass }, Thread.currentThread().getContextClassLoader())) { @@ -53,29 +48,6 @@ void classFoundInJar() throws ClassNotFoundException, IOException } } - public static URL findJarForClass(final String className) throws IOException - { - final String classAsResource = className.replace('.', '/') + ".class"; - final Enumeration urls = Thread.currentThread().getContextClassLoader().getResources(classAsResource); - while (urls.hasMoreElements()) - { - final URL url = urls.nextElement(); - if ("jar".equals(url.getProtocol())) - { - String jarPath = url.getPath().substring(5, url.getPath().indexOf("!")); - if (OS.WINDOWS.isCurrentOs()) - { - // Remove leading slash of "/C:/Users/user/.m2/..." - jarPath = jarPath.substring(1); - } - final Path path = Path.of(jarPath); - assertTrue(Files.exists(path)); - return path.toUri().toURL(); - } - } - throw new AssertionError("No jar found containing " + className); - } - /** * We can't mock the ClassLoader using Mockito because method * {@link ClassLoader#loadClass(String, boolean)} is protected. diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java new file mode 100644 index 00000000..1b7c4cc7 --- /dev/null +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java @@ -0,0 +1,73 @@ +package org.itsallcode.openfasttrace.core.serviceloader; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Enumeration; + +import org.junit.jupiter.api.condition.OS; + +final class ClassPathHelper +{ + static URL findUrlForClass(final Class clazz) + { + return findUrlForClass(clazz.getName()); + } + + static URL findUrlForClass(final String className) + { + try + { + return findJarForClass(className).toUri().toURL(); + } + catch (final MalformedURLException exception) + { + throw new IllegalStateException("Error creating URL for class " + className, exception); + } + } + + static Path findJarForClass(final Class clazz) + { + return findJarForClass(clazz.getName()); + } + + static Path findJarForClass(final String className) + { + final String resourceName = className.replace('.', '/') + ".class"; + final Enumeration urls = getResources(resourceName); + while (urls.hasMoreElements()) + { + final URL url = urls.nextElement(); + if ("jar".equals(url.getProtocol())) + { + String jarPath = url.getPath().substring(5, url.getPath().indexOf("!")); + if (OS.WINDOWS.isCurrentOs()) + { + // Remove leading slash of "/C:/Users/user/.m2/..." + jarPath = jarPath.substring(1); + } + final Path path = Path.of(jarPath); + assertTrue(Files.exists(path)); + return path; + } + } + throw new AssertionError("No jar found containing " + className); + } + + private static Enumeration getResources(final String resourceName) + { + try + { + return Thread.currentThread().getContextClassLoader().getResources(resourceName); + } + catch (final IOException exception) + { + throw new UncheckedIOException("Error finding resource " + resourceName, exception); + } + } +} diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java index 0ec065c4..6f511181 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceMock.java @@ -2,5 +2,5 @@ public @interface ServiceMock { - + // Service mock interface } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java index 53a9ba8a..120b4427 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java @@ -1,7 +1,12 @@ package org.itsallcode.openfasttrace.core.serviceloader; -import static org.mockito.Mockito.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import java.net.URLClassLoader; import java.util.Collections; import org.junit.jupiter.api.Test; @@ -13,20 +18,55 @@ class ServiceOriginTest { @Test - void closesClassLoader(@Mock(extraInterfaces = AutoCloseable.class) final ClassLoader classLoaderMock) + void closesUrlClassLoader(@Mock final URLClassLoader classLoaderMock) throws Exception { - final ServiceOrigin serviceOrigin = new ServiceOrigin(classLoaderMock, Collections.emptyList()); + final ServiceOrigin serviceOrigin = new ServiceOrigin.JarFileOrigin(Collections.emptyList(), classLoaderMock); serviceOrigin.close(); verify(((AutoCloseable) classLoaderMock)).close(); verifyNoMoreInteractions(classLoaderMock); } @Test - void doesNotCloseClassLoader(@Mock final ClassLoader classLoaderMock) + void currentClassPathOriginToString() { - final ServiceOrigin serviceOrigin = new ServiceOrigin(classLoaderMock, Collections.emptyList()); - serviceOrigin.close(); - verifyNoInteractions(classLoaderMock); + final ServiceOrigin origin = ServiceOrigin.forCurrentClassPath(); + assertThat(origin, hasToString(startsWith("CurrentClassPathOrigin [classLoader="))); + } + + @Test + void currentClassPathGetClassLoader() + { + final ServiceOrigin origin = ServiceOrigin.forCurrentClassPath(); + assertThat(origin.getClassLoader(), sameInstance(Thread.currentThread().getContextClassLoader())); + } + + @Test + void currentClassPathClose() + { + final ServiceOrigin origin = ServiceOrigin.forCurrentClassPath(); + assertDoesNotThrow(origin::close); + } + + @Test + void jarFileOriginToString() + { + final ServiceOrigin origin = ServiceOrigin.forJar(ClassPathHelper.findJarForClass(Test.class)); + assertThat(origin, + hasToString(allOf(startsWith("JarFileOrigin [classLoader="), containsString("junit-jupiter-api")))); + } + + @Test + void jarFileOriginGetClassLoader() + { + final ServiceOrigin origin = ServiceOrigin.forJar(ClassPathHelper.findJarForClass(Test.class)); + assertThat(origin.getClassLoader(), instanceOf(URLClassLoader.class)); + } + + @Test + void jarFileOriginClose() + { + final ServiceOrigin origin = ServiceOrigin.forJar(ClassPathHelper.findJarForClass(Test.class)); + assertDoesNotThrow(origin::close); } } From cba6d81543bcbabe6504417e8f5f8cf4f8059053 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 22 Jun 2024 14:25:37 +0200 Subject: [PATCH 24/44] Add link to IDE templates to user guide --- doc/user_guide.md | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/doc/user_guide.md b/doc/user_guide.md index 96b5dece..166d6fed 100644 --- a/doc/user_guide.md +++ b/doc/user_guide.md @@ -957,21 +957,21 @@ or run a report. The following example code use OFT as a converter that scans the current working directory recursively (default import setting) and exports the found artifacts with the standard settings to a ReqM2 file. -```JAVA +```java import org.itsallcode.openfasttrace.Oft; import org.itsallcode.openfasttrace.core.SpecificationItem; ``` Select input paths and import specification items from there: -```JAVA +```java final Oft oft = Oft.create(); final List items = oft.importItems(settings); ``` Export the items: -```JAVA +```java oft.exportToPath(items, Paths.get("/output/path/export.oreqm")); ``` @@ -979,7 +979,7 @@ oft.exportToPath(items, Paths.get("/output/path/export.oreqm")); The example below shows how to use OFT as a reporter. -```JAVA +```java import org.itsallcode.openfasttrace.Oft; import org.itsallcode.openfasttrace.core.LinkedSpecificationItem; import org.itsallcode.openfasttrace.core.SpecificationItem; @@ -988,7 +988,7 @@ import org.itsallcode.openfasttrace.core.Trace; The import is similar to the converter case, except this time we add an input path explicitly for the sake of demonstration: -```JAVA +```java final ImportSettings settings = ImportSettings // .builder() // .addInputs("/input/path") // @@ -999,25 +999,25 @@ final List items = oft.importItems(settings); Now link the items together (i.e. make them navigable): -```JAVA +```java final List linkedItems = oft.link(items); ``` Run the tracer on the linked items: -```JAVA +```java final Trace trace = oft.trace(linkedItems); ``` Create a report from the trace: -```JAVA +```java oft.reportToStdOut(trace); ``` You can also use the trace results in your own code: -```JAVA +```java if (trace.hasNoDefects()) { // ... do something @@ -1028,7 +1028,7 @@ if (trace.hasNoDefects()) There are various reporting formats for OFT and one can set it using the ReportSettings object. -```JAVA +```java ReportSettings reportSettings = ReportSettings.builder().outputFormat("html").build(); ``` @@ -1036,13 +1036,13 @@ The `ReportSettings` builder has other functions as well that allow you to set v OFT allows you to report directly to the standard output or to a file -```JAVA -//Reporting to a file +```java +// Reporting to a file oft.reportToPath(trace, reportPath, reportSettings); ``` -```JAVA -//Reporting to stdout +```java +// Reporting to stdout oft.reportToStdOut(trace); ``` @@ -1056,7 +1056,7 @@ Import, export and report each have an overloaded variant that can be configured Each of those classes comes with a builder which is called like this: -```JAVA +```java ReportSettings settings = ReportSettings.builder().newline(Newline.UNIX).build(); ``` @@ -1082,3 +1082,7 @@ The following editors and integrated development environments are well suited fo | [IntelliJ](https://www.jetbrains.com/idea/) | y | y | y | y | | [Vim](https://www.vim.org/) | y | | | | | [Visual Studio Code](https://code.visualstudio.com/) | y | y | y | | + +### Templates for IDEs + +You can create OpenFastTrace artifacts faster with templates for your IDE. See [the list of available IDE Templates](https://github.com/itsallcode/openfasttrace-ide-templates). From 4850feefa03ef1245a6a874fc74298b8fa271a26 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 22 Jun 2024 15:24:49 +0200 Subject: [PATCH 25/44] #413: Add requirements --- doc/spec/design.md | 57 ++++++++++++++++++++++++++++++++- doc/spec/system_requirements.md | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/doc/spec/design.md b/doc/spec/design.md index 81333ade..efefe850 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -92,6 +92,9 @@ Since the specification item IDs inherently look similar, load tests need to sho # Building Block View +## Plugin Loader +The plugin loader discovers and loads available plugins. + ## Importers For each specification artifact type OFT uses an importer. The importer uses the specification artifact as data source and reads specification items from it. @@ -122,6 +125,48 @@ API users select exporters via their name as strings. # Runtime View +## Discovering and Loading Plugins +`dsn~plugins.loading~1` + +OFT loads plugins at startup from these locations: +* Plugins included with OFT from the current ClassPath +* Third party plugins from folder `$HOME/.oft/plugins//*.jar` + +Rationale: +* A plugin may consist of multiple JARs, e.g. the plugin itself and it's dependencies. + +Covers: +* [`req~plugins.loading~1`](system_requirements.md#loading-plugins) + +Needs: impl, utest, itest + +### Plugin Loader Uses Separate ClassLoaders +`dsn~plugins.loading.separate_classloader~1` + +The Plugin loader uses a separate ClassLoader for each plugin. + +Rationale: + +Separating plugins from each other avoids conflicts between potentially duplicate classes in plugin. + +Covers: +* [`req~plugins.loading~1`](system_requirements.md#loading-plugins) + +Needs: impl, utest, itest + +### Loader Supports Plugin Types +`dsn~plugins.loading.plugin_types~1` + +The Plugin loader supports loading factories the following plugin types: +* Importers: `org.itsallcode.openfasttrace.api.importer.ImporterFactory` +* Exporters: `org.itsallcode.openfasttrace.api.exporter.ExporterFactory` +* Reports: `org.itsallcode.openfasttrace.api.report.ReporterFactory` + +Covers: +* [`req~plugins.types~1`](system_requirements.md#supported-plugin-types) + +Needs: impl, utest, itest + ## Import Depending on the source format a variety of [importers](#importers) takes care of reading the input [specification items](#specification-item). Each importer emits events which an [import event listener](#import-event-listener) consumes. @@ -835,7 +880,7 @@ Needs: impl, utest The CLI expects one of the following commands as first unnamed command line parameter: - command = "trace" / "convert" + command = "trace" / "convert" / "help" Covers: @@ -978,6 +1023,16 @@ Covers: Needs: impl, itest, utest +### Listing Plugins +`dsn~cli.plugins.list~1` + +The CLI allows listing available plugins in OFT. + +Covers: +- [req~plugins.list~1](system_requirements.md#listing-available-plugins) + +Needs: impl, utest, itest + # Design Decisions ## How do we Implement the Command Line Interpreter diff --git a/doc/spec/system_requirements.md b/doc/spec/system_requirements.md index 118d10eb..26c1fbd3 100644 --- a/doc/spec/system_requirements.md +++ b/doc/spec/system_requirements.md @@ -175,6 +175,19 @@ Running traces automatically in a scripted environment is the most important use Needs: req +### Third Party Plugins + +`feat~plugins~1` + +Users can extend OFT's features with plugins from third parties. + +Rationale: + +* Some use cases or proprietary file formats might only be relevant for a very small user group. It does not make sense to integrate this into the core OFT product. +* Some importers/exporters require additional dependencies that we don't want to include in the core OFT product. + +Needs: req + ## Functional Requirements ### Anatomy of Specification Items @@ -794,3 +807,47 @@ Covers: * [feat~plain-text-report](#plain-text-report) Needs: dsn + +### Third Party Plugins + +#### Loading Plugins +`req~plugins.loading~1` + +OFT automatically loads plugins from JAR files located in a predefined location at startup. + +Rationale: + +* Plugins must be only installed once in the correct location. +* This requires no additional configuration by the user. + +Covers: +* [feat~plugins~1](#third-party-plugins) +Needs: dsn + +#### Supported Plugin Types +`req~plugins.types~1` + +OFT supports the following plugin types: +* Importers add support for importing requirements from additional file formats. +* Exporters add support for exporting requirements in additional file formats. +* Reports add support for generating reports in additional formats. + +Covers: +* [feat~plugins~1](#third-party-plugins) +Needs: dsn + +#### Listing Available Plugins +`req~plugins.list~1` + +OFT allows users to list all currently available plugins including the following information: +* Plugin type (importer, exporter, reporter) +* Location (included with OFT, external JAR) +* Version number + +Rationale: + +This is helpful for debugging in case OFT does not use a plugin as expected. + +Covers: +* [feat~plugins~1](#third-party-plugins) +Needs: dsn From 84f357d5b1f4cefc50f4a404826e376eeb9ea805 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 22 Jun 2024 15:46:18 +0200 Subject: [PATCH 26/44] #413: Add requirements & coverage --- .../openfasttrace/core/ServiceFactory.java | 6 +---- .../core/exporter/ExporterFactoryLoader.java | 1 + .../core/importer/ImporterFactoryLoader.java | 18 ++++++++++--- .../core/report/ReporterFactoryLoader.java | 1 + .../serviceloader/ServiceLoaderFactory.java | 4 ++- .../core/serviceloader/ServiceOrigin.java | 2 ++ .../ServiceLoaderFactoryTest.java | 1 + .../core/serviceloader/ServiceOriginTest.java | 2 ++ doc/spec/design.md | 6 ++--- .../serviceloader/ServiceLoaderFactoryIT.java | 1 + .../TestInitializingServiceLoader.java | 26 ++++++++++++++++++- 11 files changed, 55 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java index 5fcd2422..4d0e5d3b 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/ServiceFactory.java @@ -13,8 +13,6 @@ import org.itsallcode.openfasttrace.core.importer.ImporterServiceImpl; import org.itsallcode.openfasttrace.core.report.ReportService; import org.itsallcode.openfasttrace.core.report.ReporterFactoryLoader; -import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; -import org.itsallcode.openfasttrace.core.serviceloader.Loader; class ServiceFactory { @@ -26,9 +24,7 @@ ExporterService createExporterService() ImporterService createImporterService(final ImportSettings settings) { final ImporterContext context = new ImporterContext(settings); - final Loader serviceLoader = InitializingServiceLoader.load(ImporterFactory.class, context); - final ImporterService service = new ImporterServiceImpl( - new ImporterFactoryLoader(serviceLoader), settings); + final ImporterService service = new ImporterServiceImpl(new ImporterFactoryLoader(context), settings); context.setImporterService(service); return service; } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java index 43028ad4..c3d774dd 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java @@ -23,6 +23,7 @@ public class ExporterFactoryLoader */ public ExporterFactoryLoader(final ExporterContext context) { + // [impl->dsn~plugins.loading.plugin_types~1] this(InitializingServiceLoader.load(ExporterFactory.class, context)); } 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 21c44013..8dd06119 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 @@ -5,9 +5,9 @@ import java.util.Optional; import java.util.logging.Logger; -import org.itsallcode.openfasttrace.api.importer.ImporterException; -import org.itsallcode.openfasttrace.api.importer.ImporterFactory; +import org.itsallcode.openfasttrace.api.importer.*; import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; import org.itsallcode.openfasttrace.core.serviceloader.Loader; /** @@ -26,11 +26,23 @@ public class ImporterFactoryLoader * @param serviceLoader * the loader used for locating importers. */ - public ImporterFactoryLoader(final Loader serviceLoader) + ImporterFactoryLoader(final Loader serviceLoader) { this.serviceLoader = serviceLoader; } + /** + * Create a new loader for the given context. + * + * @param context + * the context for the new loader. + */ + public ImporterFactoryLoader(final ImporterContext context) + { + // [impl->dsn~plugins.loading.plugin_types~1] + this(InitializingServiceLoader.load(ImporterFactory.class, context)); + } + /** * Finds a matching {@link ImporterFactory} that can handle the given * {@link Path}. If no or more than one {@link ImporterFactory} is found, diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java index ce81e8fe..ad58a2d7 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java @@ -24,6 +24,7 @@ public class ReporterFactoryLoader */ public ReporterFactoryLoader(final ReporterContext context) { + // [impl->dsn~plugins.loading.plugin_types~1] this(InitializingServiceLoader.load(ReporterFactory.class, context)); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index 4b984485..84550438 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -39,7 +39,8 @@ class ServiceLoaderFactory */ static ServiceLoaderFactory createDefault() { - return new ServiceLoaderFactory(getHomeDirectory().resolve(".oft").resolve("plugins"), true); + final Path pluginsDirectory = getHomeDirectory().resolve(".oft").resolve("plugins"); + return new ServiceLoaderFactory(pluginsDirectory, true); } private static Path getHomeDirectory() @@ -76,6 +77,7 @@ private static Loader createLoader(final Class serviceType, final List * * @return a list of service origins */ + // [impl->dsn~plugins.loading~1] List findServiceOrigins() { final List origins = new ArrayList<>(); diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 61419c21..08395e17 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -9,6 +9,7 @@ /** * Origin of a service, either the current classpath or a list of jar files. */ +// [impl->dsn~plugins.loading~1] interface ServiceOrigin extends AutoCloseable { /** @@ -45,6 +46,7 @@ static ServiceOrigin forJar(final Path jar) * list of paths to jar files * @return a service origin for the jar files */ + // [impl->dsn~plugins.loading.separate_classloader~1] static ServiceOrigin forJars(final List jars) { return new JarFileOrigin(jars, createClassLoader(jars)); diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 5ec72c02..5b8ba795 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.io.TempDir; import org.opentest4j.MultipleFailuresError; +// [utest->dsn~plugins.loading~1] class ServiceLoaderFactoryTest { @TempDir diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java index 120b4427..94f16616 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java @@ -14,6 +14,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +// [utest->dsn~plugins.loading~1] @ExtendWith(MockitoExtension.class) class ServiceOriginTest { @@ -34,6 +35,7 @@ void currentClassPathOriginToString() assertThat(origin, hasToString(startsWith("CurrentClassPathOrigin [classLoader="))); } + // [utest->dsn~plugins.loading.separate_classloader~1] @Test void currentClassPathGetClassLoader() { diff --git a/doc/spec/design.md b/doc/spec/design.md index efefe850..c4c60e38 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -152,7 +152,7 @@ Separating plugins from each other avoids conflicts between potentially duplicat Covers: * [`req~plugins.loading~1`](system_requirements.md#loading-plugins) -Needs: impl, utest, itest +Needs: impl, utest ### Loader Supports Plugin Types `dsn~plugins.loading.plugin_types~1` @@ -165,7 +165,7 @@ The Plugin loader supports loading factories the following plugin types: Covers: * [`req~plugins.types~1`](system_requirements.md#supported-plugin-types) -Needs: impl, utest, itest +Needs: impl, itest ## Import @@ -1031,7 +1031,7 @@ The CLI allows listing available plugins in OFT. Covers: - [req~plugins.list~1](system_requirements.md#listing-available-plugins) -Needs: impl, utest, itest +#Needs: impl, utest, itest # Design Decisions diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java index 54c12cb1..94b15a72 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java @@ -21,6 +21,7 @@ * must be located in module {@code product} (which includes all plugin modules) * so that it can access all plugin services. */ +// [itest->dsn~plugins.loading~1] class ServiceLoaderFactoryIT { @TempDir diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java index 59179f03..0d3b1ff0 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java @@ -10,12 +10,17 @@ import org.itsallcode.openfasttrace.api.exporter.ExporterFactory; import org.itsallcode.openfasttrace.api.importer.ImporterContext; import org.itsallcode.openfasttrace.api.importer.ImporterFactory; +import org.itsallcode.openfasttrace.api.report.ReporterContext; +import org.itsallcode.openfasttrace.api.report.ReporterFactory; import org.itsallcode.openfasttrace.exporter.specobject.SpecobjectExporterFactory; import org.itsallcode.openfasttrace.importer.markdown.MarkdownImporterFactory; import org.itsallcode.openfasttrace.importer.restructuredtext.RestructuredTextImporterFactory; import org.itsallcode.openfasttrace.importer.specobject.SpecobjectImporterFactory; import org.itsallcode.openfasttrace.importer.tag.TagImporterFactory; import org.itsallcode.openfasttrace.importer.zip.ZipFileImporterFactory; +import org.itsallcode.openfasttrace.report.aspec.ASpecReporterFactory; +import org.itsallcode.openfasttrace.report.html.HtmlReporterFactory; +import org.itsallcode.openfasttrace.report.plaintext.PlaintextReporterFactory; import org.junit.jupiter.api.Test; /** @@ -36,6 +41,7 @@ void testNoServicesRegistered() assertThat(voidServiceLoader.load().toList(), emptyIterable()); } + // [itest->dsn~plugins.loading.plugin_types~1] @Test void testImporterFactoriesRegistered() { @@ -55,6 +61,7 @@ void testImporterFactoriesRegistered() } } + // [itest->dsn~plugins.loading.plugin_types~1] @Test void testExporterFactoriesRegistered() { @@ -62,13 +69,30 @@ void testExporterFactoriesRegistered() final List services = getRegisteredServices(ExporterFactory.class, context); assertThat(services, hasSize(1)); - assertThat(services, contains(instanceOf(SpecobjectExporterFactory.class))); + assertThat(services, containsInAnyOrder(instanceOf(SpecobjectExporterFactory.class))); for (final ExporterFactory factory : services) { assertThat(factory.getContext(), sameInstance(context)); } } + // [itest->dsn~plugins.loading.plugin_types~1] + @Test + void testReporterFactoriesRegistered() + { + final ReporterContext context = new ReporterContext(null); + final List services = getRegisteredServices(ReporterFactory.class, + context); + assertThat(services, hasSize(3)); + assertThat(services, containsInAnyOrder(instanceOf(PlaintextReporterFactory.class), + instanceOf(ASpecReporterFactory.class), + instanceOf(HtmlReporterFactory.class))); + for (final ReporterFactory factory : services) + { + assertThat(factory.getContext(), sameInstance(context)); + } + } + private , C> List getRegisteredServices(final Class type, final C context) { From 2b6af37787b2b29720ecd199620d7c6f5584717c Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 08:09:21 +0200 Subject: [PATCH 27/44] Code cleanup --- .../api/importer/MultiFileImporter.java | 4 +-- .../core/importer/MultiFileImporterImpl.java | 32 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/api/src/main/java/org/itsallcode/openfasttrace/api/importer/MultiFileImporter.java b/api/src/main/java/org/itsallcode/openfasttrace/api/importer/MultiFileImporter.java index 1935b7d4..c83399d0 100644 --- a/api/src/main/java/org/itsallcode/openfasttrace/api/importer/MultiFileImporter.java +++ b/api/src/main/java/org/itsallcode/openfasttrace/api/importer/MultiFileImporter.java @@ -26,8 +26,8 @@ public interface MultiFileImporter MultiFileImporter importFile(InputFile file); /** - * Import from the path, independently of whether it is represents a - * directory or a file. + * Import from the path, independently of whether it represents a directory + * or a file. * * @param paths * lists of paths to files or directories diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/importer/MultiFileImporterImpl.java b/core/src/main/java/org/itsallcode/openfasttrace/core/importer/MultiFileImporterImpl.java index 468b22aa..6e53a5cc 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/importer/MultiFileImporterImpl.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/importer/MultiFileImporterImpl.java @@ -4,9 +4,7 @@ import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.PathMatcher; +import java.nio.file.*; import java.util.List; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; @@ -85,18 +83,21 @@ public MultiFileImporter importRecursiveDir(final Path dir, final String glob) final int itemCountBefore = this.specItemBuilder.getItemCount(); try (Stream fileStream = Files.walk(dir)) { - fileStream.filter(path -> !path.toFile().isDirectory()) // - .filter(matcher::matches) // + fileStream.filter(path -> !path.toFile().isDirectory()) + .filter(matcher::matches) .map(path -> RealFileInput.forPath(path, DEFAULT_CHARSET)) .filter(this.factoryLoader::supportsFile) - .map(file -> createImporterIfPossible(file, this.specItemBuilder)).forEach(importer -> { - importer.ifPresent(Importer::runImport); + .map(file -> createImporterIfPossible(file, this.specItemBuilder)) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(importer -> { + importer.runImport(); fileCount.incrementAndGet(); }); } - catch (final IOException e) + catch (final IOException exception) { - throw new ImporterException("Error walking directory " + dir, e); + throw new ImporterException("Error walking directory " + dir, exception); } final int itemCountImported = this.specItemBuilder.getItemCount() - itemCountBefore; LOG.fine(() -> "Imported " + fileCount + " files containing " + itemCountImported @@ -112,14 +113,13 @@ public List getImportedItems() private Optional createImporterIfPossible(final InputFile file, final SpecificationListBuilder builder) { - final Optional importerFactory = this.factoryLoader.getImporterFactory(file); - final Optional importer = importerFactory.isPresent() - ? Optional.of(importerFactory.get().createImporter(file, builder)) - : Optional.empty(); + final Optional importer = this.factoryLoader.getImporterFactory(file) + .map(factory -> factory.createImporter(file, builder)); - LOG.fine(() -> (importer.isPresent() ? "Created importer of type '" + importer.getClass().getSimpleName() - : "No import") - + "' for file '" + file + "'"); + LOG.finest( + () -> (importer.isPresent() ? "Created importer of type '" + importer.get().getClass().getSimpleName() + : "No import") + + "' for file '" + file + "'"); return importer; } From b96f67e852e7bcd4d9621b15fb69453d740d595e Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 08:09:40 +0200 Subject: [PATCH 28/44] Fix compile error --- .../openfasttrace/TestAllServicesAvailable.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/product/src/test/java/org/itsallcode/openfasttrace/TestAllServicesAvailable.java b/product/src/test/java/org/itsallcode/openfasttrace/TestAllServicesAvailable.java index 716e8ac3..33368f05 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/TestAllServicesAvailable.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/TestAllServicesAvailable.java @@ -14,10 +14,7 @@ import org.itsallcode.openfasttrace.api.core.Trace; import org.itsallcode.openfasttrace.api.exporter.Exporter; import org.itsallcode.openfasttrace.api.exporter.ExporterContext; -import org.itsallcode.openfasttrace.api.importer.ImportEventListener; -import org.itsallcode.openfasttrace.api.importer.Importer; -import org.itsallcode.openfasttrace.api.importer.ImporterContext; -import org.itsallcode.openfasttrace.api.importer.ImporterFactory; +import org.itsallcode.openfasttrace.api.importer.*; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; import org.itsallcode.openfasttrace.api.report.Reportable; @@ -25,7 +22,6 @@ import org.itsallcode.openfasttrace.core.exporter.ExporterFactoryLoader; import org.itsallcode.openfasttrace.core.importer.ImporterFactoryLoader; import org.itsallcode.openfasttrace.core.report.ReporterFactoryLoader; -import org.itsallcode.openfasttrace.core.serviceloader.InitializingServiceLoader; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; @@ -54,7 +50,7 @@ private static ImporterFactoryLoader createImporterLoader() final ImporterContext contextMock = mock(ImporterContext.class, withSettings().defaultAnswer(Answers.RETURNS_DEEP_STUBS)); - return new ImporterFactoryLoader(InitializingServiceLoader.load(ImporterFactory.class, contextMock)); + return new ImporterFactoryLoader(contextMock); } private static ExporterFactoryLoader createExporterLoader() From bbfd0149a5a343c3ba96742b867ed195c0f22421 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 08:42:59 +0200 Subject: [PATCH 29/44] Configure log level --- core/src/main/java/module-info.java | 3 +- .../openfasttrace/core/cli/CliArguments.java | 62 +++++++++++++++---- .../openfasttrace/core/cli/CliStarter.java | 2 + .../core/cli/logging/LogLevel.java | 21 +++++++ .../core/cli/logging/LoggingConfigurator.java | 56 +++++++++++++++++ .../serviceloader/ServiceLoaderFactory.java | 2 +- core/src/main/resources/usage.txt | 3 + doc/spec/design.md | 8 +-- doc/spec/system_requirements.md | 6 +- doc/user_guide.md | 5 ++ oft-self-trace.sh | 1 + 11 files changed, 148 insertions(+), 21 deletions(-) create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java create mode 100644 core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java diff --git a/core/src/main/java/module-info.java b/core/src/main/java/module-info.java index 9a911842..e2a7686c 100644 --- a/core/src/main/java/module-info.java +++ b/core/src/main/java/module-info.java @@ -10,12 +10,13 @@ exports org.itsallcode.openfasttrace.core; exports org.itsallcode.openfasttrace.core.cli; exports org.itsallcode.openfasttrace.core.cli.commands; + exports org.itsallcode.openfasttrace.core.cli.logging; exports org.itsallcode.openfasttrace.core.report; exports org.itsallcode.openfasttrace.core.exporter; exports org.itsallcode.openfasttrace.core.importer; exports org.itsallcode.openfasttrace.core.serviceloader; - requires java.logging; + requires transitive java.logging; requires transitive org.itsallcode.openfasttrace.api; uses org.itsallcode.openfasttrace.api.exporter.ExporterFactory; diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java index 37974d57..ac4430a6 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java @@ -12,6 +12,7 @@ import org.itsallcode.openfasttrace.api.report.ReportVerbosity; import org.itsallcode.openfasttrace.core.cli.commands.ConvertCommand; import org.itsallcode.openfasttrace.core.cli.commands.TraceCommand; +import org.itsallcode.openfasttrace.core.cli.logging.LogLevel; import org.itsallcode.openfasttrace.core.exporter.ExporterConstants; /** @@ -42,6 +43,9 @@ public class CliArguments // [impl->dsn~reporting.html.details-display~1] private DetailsSectionDisplay detailsSectionDisplay; + // [impl->dsn~cli.plugins.log~1] + private LogLevel logLevel; + /** * Create new {@link CliArguments}. * @@ -300,18 +304,21 @@ public Set getWantedTags() /** * Get the color scheme. *

- * Defaults to {@link ColorScheme#COLOR}. The switch -f overrides this setting, so that the color - * scheme is always {@link ColorScheme#BLACK_AND_WHITE}. + * Defaults to {@link ColorScheme#COLOR}. The switch -f + * overrides this setting, so that the color scheme is always + * {@link ColorScheme#BLACK_AND_WHITE}. *

* * @return the color scheme */ public ColorScheme getColorScheme() { - if (this.getOutputPath() == null) { + if (this.getOutputPath() == null) + { return (this.colorScheme == null) ? ColorScheme.COLOR : this.colorScheme; } - else { + else + { return ColorScheme.BLACK_AND_WHITE; } } @@ -354,8 +361,7 @@ public void setT(final String tags) /** * Check if origin information should be shown in reports. * - * @return {@code true} if origin information should be shown in - * reports. + * @return {@code true} if origin information should be shown in reports. */ public boolean getShowOrigin() { @@ -366,8 +372,7 @@ public boolean getShowOrigin() * Choose whether to show origin information in reports. * * @param showOrigin - * {@code true} if origin information should be shown in - * reports + * {@code true} if origin information should be shown in reports */ public void setShowOrigin(final boolean showOrigin) { @@ -378,8 +383,7 @@ public void setShowOrigin(final boolean showOrigin) * Choose whether to show origin information in reports. * * @param showOrigin - * {@code true} if origin information should be shown in - * reports + * {@code true} if origin information should be shown in reports */ public void setS(final boolean showOrigin) { @@ -389,7 +393,8 @@ public void setS(final boolean showOrigin) /** * Choose the color scheme. * - * @param colorScheme color scheme to use for console output + * @param colorScheme + * color scheme to use for console output */ public void setColorScheme(final ColorScheme colorScheme) { @@ -399,7 +404,8 @@ public void setColorScheme(final ColorScheme colorScheme) /** * Choose the color scheme. * - * @param colorScheme color scheme to use for console output + * @param colorScheme + * color scheme to use for console output */ public void setC(final ColorScheme colorScheme) { @@ -416,4 +422,36 @@ public void setDetailsSectionDisplay(final DetailsSectionDisplay detailsSectionD { this.detailsSectionDisplay = detailsSectionDisplay; } + + /** + * Get the log level console logging. + * + * @return the log level + */ + public Optional getLogLevel() + { + return Optional.ofNullable(this.logLevel); + } + + /** + * Set the log level for console logging. + * + * @param logLevel + * the log level + */ + public void setLogLevel(final LogLevel logLevel) + { + this.logLevel = logLevel; + } + + /** + * Set the log level for console logging. + * + * @param logLevel + * the log level + */ + public void setL(final LogLevel logLevel) + { + setLogLevel(logLevel); + } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliStarter.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliStarter.java index e540acd1..843897b1 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliStarter.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliStarter.java @@ -4,6 +4,7 @@ import org.itsallcode.openfasttrace.api.cli.DirectoryService; import org.itsallcode.openfasttrace.core.cli.commands.*; +import org.itsallcode.openfasttrace.core.cli.logging.LoggingConfigurator; /** * The main entry point class for the command line application. @@ -51,6 +52,7 @@ public static void main(final String[] args, final DirectoryService directorySer final ArgumentValidator validator = new ArgumentValidator(arguments); if (validator.isValid()) { + LoggingConfigurator.create(arguments).configureLogging(); new CliStarter(arguments).run(); } else diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java new file mode 100644 index 00000000..0f2e75e8 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -0,0 +1,21 @@ +package org.itsallcode.openfasttrace.core.cli.logging; + +import java.util.logging.Level; + +public enum LogLevel +{ + OFF(Level.OFF), SEVERE(Level.SEVERE), WARNING(Level.WARNING), INFO(Level.INFO), CONFIG(Level.CONFIG), FINE( + Level.FINE), FINER(Level.FINER), FINEST(Level.FINEST), ALL(Level.ALL); + + private final Level julLevel; + + private LogLevel(final Level julLevel) + { + this.julLevel = julLevel; + } + + Level getJulLogLevel() + { + return julLevel; + } +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java new file mode 100644 index 00000000..030c5518 --- /dev/null +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -0,0 +1,56 @@ +package org.itsallcode.openfasttrace.core.cli.logging; + +import java.io.*; +import java.nio.charset.StandardCharsets; +import java.util.logging.LogManager; +import java.util.logging.Logger; + +import org.itsallcode.openfasttrace.core.cli.CliArguments; + +public class LoggingConfigurator +{ + private static final String CONFIG_TEMPLATE = """ + handlers=java.util.logging.ConsoleHandler + .level=$LOG_LEVEL + java.util.logging.ConsoleHandler.level=$LOG_LEVEL + java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter + java.util.logging.SimpleFormatter.format=%1$tF %1$tT.%1$tL [%4$-7s] %5$s %n + org.itsallcode.openfasttrace.level=$LOG_LEVEL + """; + private final LogLevel logLevel; + + private LoggingConfigurator(final LogLevel logLevel) + { + this.logLevel = logLevel; + } + + public static LoggingConfigurator create(final CliArguments arguments) + { + return new LoggingConfigurator(arguments.getLogLevel().orElse(LogLevel.INFO)); + } + + public void configureLogging() + { + final LogManager logManager = LogManager.getLogManager(); + configureLogManager(logManager, getConfigContent()); + final Logger rootLogger = logManager.getLogger(""); + rootLogger.info(() -> "Logging configured with level " + this.logLevel + "."); + } + + private String getConfigContent() + { + return CONFIG_TEMPLATE.replace("$LOG_LEVEL", this.logLevel.getJulLogLevel().getName()); + } + + private void configureLogManager(final LogManager logManager, final String configContent) + { + try (InputStream config = new ByteArrayInputStream(configContent.getBytes(StandardCharsets.UTF_8))) + { + logManager.readConfiguration(config); + } + catch (SecurityException | IOException exception) + { + throw new IllegalStateException("Failed to configure logging", exception); + } + } +} diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index 84550438..95534650 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -86,7 +86,7 @@ List findServiceOrigins() origins.add(ServiceOrigin.forCurrentClassPath()); } origins.addAll(findPluginOrigins()); - LOGGER.fine(() -> "Found " + origins.size() + " service origins: " + origins + "."); + LOGGER.finest(() -> "Found " + origins.size() + " service origins: " + origins + "."); return origins; } diff --git a/core/src/main/resources/usage.txt b/core/src/main/resources/usage.txt index ae109588..8f29f55f 100644 --- a/core/src/main/resources/usage.txt +++ b/core/src/main/resources/usage.txt @@ -36,6 +36,9 @@ Common options: least one tag contained in the comma-separated list. Add a single underscore as first item in the list to also import items without any tags. + -l, --log-level Log level for console logging. One of + "OFF", "SEVERE", "WARNING", "INFO", "CONFIG", + "FINE", "FINER", "FINEST", "ALL" Returns: 0 on success diff --git a/doc/spec/design.md b/doc/spec/design.md index c4c60e38..2a5da493 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -1024,14 +1024,14 @@ Covers: Needs: impl, itest, utest ### Listing Plugins -`dsn~cli.plugins.list~1` +`dsn~cli.plugins.log~1` -The CLI allows listing available plugins in OFT. +The CLI logs available plugins in OFT at startup. Covers: -- [req~plugins.list~1](system_requirements.md#listing-available-plugins) +- [req~plugins.log~1](system_requirements.md#logging-available-plugins) -#Needs: impl, utest, itest +Needs: impl, utest, itest # Design Decisions diff --git a/doc/spec/system_requirements.md b/doc/spec/system_requirements.md index 26c1fbd3..5ef883d2 100644 --- a/doc/spec/system_requirements.md +++ b/doc/spec/system_requirements.md @@ -836,10 +836,10 @@ Covers: * [feat~plugins~1](#third-party-plugins) Needs: dsn -#### Listing Available Plugins -`req~plugins.list~1` +#### Logging Available Plugins +`req~plugins.log~1` -OFT allows users to list all currently available plugins including the following information: +OFT logs all currently available plugins at startup including the following information: * Plugin type (importer, exporter, reporter) * Location (included with OFT, external JAR) * Version number diff --git a/doc/user_guide.md b/doc/user_guide.md index 166d6fed..4302a20e 100644 --- a/doc/user_guide.md +++ b/doc/user_guide.md @@ -565,6 +565,11 @@ The available color schemes are `color` : Color output. Also enables font style on the console. + + -l, --log-level + +Log level for console logging. One of `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. + ### Build Integration In order to integrate requirement tracing with OFT into your CI build, we recommend using the OFT plugins for Maven and Gradle: diff --git a/oft-self-trace.sh b/oft-self-trace.sh index 86d3c35d..47b612c0 100755 --- a/oft-self-trace.sh +++ b/oft-self-trace.sh @@ -12,6 +12,7 @@ report_file=$base_dir/target/self-trace-report.html mkdir -p "$(dirname "$report_file")" if $oft_script trace \ + --log-level INFO \ --output-file "$report_file" \ --output-format html \ "$base_dir/doc/spec" \ From 733f12bf92194e20345929c29efa409ec762c0d7 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 08:52:28 +0200 Subject: [PATCH 30/44] Add launch config for vscode to run self-trace --- .vscode/launch.json | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 00000000..94799b60 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,34 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "type": "java", + "name": "Run Self-Trace", + "request": "launch", + "mainClass": "org.itsallcode.openfasttrace.core.cli.CliStarter", + "projectName": "openfasttrace", + "args": [ + "trace", + "--log-level", + "INFO", + "${workspaceFolder}/doc/spec", + "${workspaceFolder}/importer/lightweightmarkup/src", + "${workspaceFolder}/importer/markdown/src", + "${workspaceFolder}/importer/restructuredtext/src", + "${workspaceFolder}/importer/specobject/src", + "${workspaceFolder}/importer/zip/src", + "${workspaceFolder}/importer/tag/src", + "${workspaceFolder}/core/src/main", + "${workspaceFolder}/core/src/test/java", + "${workspaceFolder}/reporter/plaintext/src", + "${workspaceFolder}/reporter/html/src", + "${workspaceFolder}/reporter/aspec/src", + "${workspaceFolder}/product/src/test/java", + "${workspaceFolder}/api/src", + "${workspaceFolder}/exporter/specobject/src", + "${workspaceFolder}/exporter/common/src", + "${workspaceFolder}/testutil/src" + ] + } + ] +} \ No newline at end of file From 945993e11d61f4ff4c4a0e79fd4336d00f91a934 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 09:00:26 +0200 Subject: [PATCH 31/44] Add javadoc --- .../core/cli/logging/LogLevel.java | 45 +++++++++++++++++-- .../core/cli/logging/LoggingConfigurator.java | 15 ++++++- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java index 0f2e75e8..53111cdd 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -2,10 +2,47 @@ import java.util.logging.Level; +/** + * Log levels for console logging. + */ public enum LogLevel { - OFF(Level.OFF), SEVERE(Level.SEVERE), WARNING(Level.WARNING), INFO(Level.INFO), CONFIG(Level.CONFIG), FINE( - Level.FINE), FINER(Level.FINER), FINEST(Level.FINEST), ALL(Level.ALL); + /** + * OFF is a special level that can be used to turn off logging. + */ + OFF(Level.OFF), + /** + * SEVERE is a message level indicating a serious failure. + */ + SEVERE(Level.SEVERE), + /** + * WARNING is a message level indicating a potential problem. + */ + WARNING(Level.WARNING), + /** + * INFO is a message level for informational messages. + */ + INFO(Level.INFO), + /** + * CONFIG is a message level for static configuration messages. + */ + CONFIG(Level.CONFIG), + /** + * FINE is a message level providing tracing information. + */ + FINE(Level.FINE), + /** + * FINER indicates a fairly detailed tracing message. + */ + FINER(Level.FINER), + /** + * FINEST indicates a highly detailed tracing message. + */ + FINEST(Level.FINEST), + /** + * ALL indicates that all messages should be logged. + */ + ALL(Level.ALL); private final Level julLevel; @@ -14,8 +51,8 @@ private LogLevel(final Level julLevel) this.julLevel = julLevel; } - Level getJulLogLevel() + String getJulLogLevel() { - return julLevel; + return julLevel.getName(); } } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index 030c5518..8aca919a 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -7,6 +7,9 @@ import org.itsallcode.openfasttrace.core.cli.CliArguments; +/** + * Configures console logging for the application. + */ public class LoggingConfigurator { private static final String CONFIG_TEMPLATE = """ @@ -24,11 +27,21 @@ private LoggingConfigurator(final LogLevel logLevel) this.logLevel = logLevel; } + /** + * Create a new logging configurator. + * + * @param arguments + * the command line arguments. + * @return a new logging configurator. + */ public static LoggingConfigurator create(final CliArguments arguments) { return new LoggingConfigurator(arguments.getLogLevel().orElse(LogLevel.INFO)); } + /** + * Configures logging according to the configured log level. + */ public void configureLogging() { final LogManager logManager = LogManager.getLogManager(); @@ -39,7 +52,7 @@ public void configureLogging() private String getConfigContent() { - return CONFIG_TEMPLATE.replace("$LOG_LEVEL", this.logLevel.getJulLogLevel().getName()); + return CONFIG_TEMPLATE.replace("$LOG_LEVEL", this.logLevel.getJulLogLevel()); } private void configureLogManager(final LogManager logManager, final String configContent) From 2d7795bab61cb3fc99ee499dbeef2ac9733ab0cd Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 11:02:34 +0200 Subject: [PATCH 32/44] Log loaded plugins --- .../core/serviceloader/ClassPathServiceLoader.java | 12 ++++++++++-- .../core/serviceloader/ServiceLoaderFactory.java | 3 +-- .../core/serviceloader/ServiceOrigin.java | 6 +++--- doc/spec/design.md | 2 +- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index f34d7394..c6983217 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -28,10 +28,13 @@ static Loader create(final Class serviceType, final ServiceOrigin serv } @Override + @SuppressWarnings("java:S3864") // Using peek() for logging only. public Stream load() { - return this.serviceLoader.stream().map(Provider::get) - .filter(this::filterOtherClassLoader); + return this.serviceLoader.stream() + .map(Provider::get) + .filter(this::filterOtherClassLoader) + .peek(this::logService); } private boolean filterOtherClassLoader(final T service) @@ -47,6 +50,11 @@ private boolean filterOtherClassLoader(final T service) return correctClassLoader; } + private void logService(final T service) + { + LOGGER.info(() -> "Loaded service from " + serviceOrigin + ": " + service.getClass().getName()); + } + @Override public void close() { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index 95534650..ffbfd41e 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -80,12 +80,11 @@ private static Loader createLoader(final Class serviceType, final List // [impl->dsn~plugins.loading~1] List findServiceOrigins() { - final List origins = new ArrayList<>(); + final List origins = new ArrayList<>(findPluginOrigins()); if (searchCurrentClasspath) { origins.add(ServiceOrigin.forCurrentClassPath()); } - origins.addAll(findPluginOrigins()); LOGGER.finest(() -> "Found " + origins.size() + " service origins: " + origins + "."); return origins; } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 08395e17..12105f66 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -61,7 +61,7 @@ private static URLClassLoader createClassLoader(final List jars) private static String getClassLoaderName(final List jars) { - return "ServiceClassLoader-" + return "JarClassLoader-" + jars.stream().map(Path::getFileName).sorted().map(Path::toString).collect(joining(",")); } @@ -122,7 +122,7 @@ public void close() @Override public String toString() { - return "JarFileOrigin [classLoader=" + classLoader + ", jars=" + jars + "]"; + return "JarFileOrigin [classLoader=" + classLoader.getName() + ", jars=" + jars + "]"; } } @@ -150,7 +150,7 @@ public void close() @Override public String toString() { - return "CurrentClassPathOrigin [classLoader=" + classLoader + "]"; + return "CurrentClassPathOrigin [classLoader=" + classLoader.getName() + "]"; } } } diff --git a/doc/spec/design.md b/doc/spec/design.md index 2a5da493..16b7fcf0 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -1031,7 +1031,7 @@ The CLI logs available plugins in OFT at startup. Covers: - [req~plugins.log~1](system_requirements.md#logging-available-plugins) -Needs: impl, utest, itest +Needs: impl # Design Decisions From c227897fb7a78a8f221237de35c317438c4384e4 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 11:50:44 +0200 Subject: [PATCH 33/44] #413: Add documentation --- .github/ISSUE_TEMPLATE/New_plugin.md | 13 ++++++ README.md | 2 + .../core/cli/logging/LoggingConfigurator.java | 3 +- core/src/main/resources/usage.txt | 3 +- doc/plugin_developer_guide.md | 41 +++++++++++++++++++ doc/plugins.md | 15 +++++++ doc/user_guide.md | 2 +- 7 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/New_plugin.md create mode 100644 doc/plugin_developer_guide.md create mode 100644 doc/plugins.md diff --git a/.github/ISSUE_TEMPLATE/New_plugin.md b/.github/ISSUE_TEMPLATE/New_plugin.md new file mode 100644 index 00000000..22d77260 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/New_plugin.md @@ -0,0 +1,13 @@ +--- +name: Add a new third-party plugin +about: Update OFT documentation to include a new third-party plugin + +--- + +## Plugin Details + +* Name: +* Description: +* Web page: +* Source code repository: +* Plugin type (importer, exporter or reporter): diff --git a/README.md b/README.md index c59f47c1..e1065ec3 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ Sonarcloud status: **User Guides** * [📖 User Guide](doc/user_guide.md) +* [🔌 Extending OpenFastTrace With Plugins](doc/plugins.md) * [💲 Command Line Usage](core/src/main/resources/usage.txt) **News and Discussions** @@ -47,6 +48,7 @@ Sonarcloud status: * [🎟️ Project Board](https://github.com/orgs/itsallcode/projects/3/views/1) * [🦮 Developer Guide](doc/developer_guide.md) +* [🔌 Plugin Developer Guide](doc/plugin_developer_guide.md) * [🎁 Contributing Guide](CONTRIBUTING.md) * [💡 System Requirements](doc/spec/system_requirements.md) * [👜 Design](doc/spec/design.md) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index 8aca919a..f7dd0a2d 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -12,6 +12,7 @@ */ public class LoggingConfigurator { + private static final LogLevel DEFAULT_LOG_LEVEL = LogLevel.WARNING; private static final String CONFIG_TEMPLATE = """ handlers=java.util.logging.ConsoleHandler .level=$LOG_LEVEL @@ -36,7 +37,7 @@ private LoggingConfigurator(final LogLevel logLevel) */ public static LoggingConfigurator create(final CliArguments arguments) { - return new LoggingConfigurator(arguments.getLogLevel().orElse(LogLevel.INFO)); + return new LoggingConfigurator(arguments.getLogLevel().orElse(DEFAULT_LOG_LEVEL)); } /** diff --git a/core/src/main/resources/usage.txt b/core/src/main/resources/usage.txt index 8f29f55f..f1616092 100644 --- a/core/src/main/resources/usage.txt +++ b/core/src/main/resources/usage.txt @@ -38,7 +38,8 @@ Common options: the list to also import items without any tags. -l, --log-level Log level for console logging. One of "OFF", "SEVERE", "WARNING", "INFO", "CONFIG", - "FINE", "FINER", "FINEST", "ALL" + "FINE", "FINER", "FINEST", "ALL". + Defaults to "WARNING". Returns: 0 on success diff --git a/doc/plugin_developer_guide.md b/doc/plugin_developer_guide.md new file mode 100644 index 00000000..5cd6f2f4 --- /dev/null +++ b/doc/plugin_developer_guide.md @@ -0,0 +1,41 @@ +# Plugin Developer Guide + +This guide describes how to develop [plugins](plugins.md) for OpenFastTrace (OFT). + +## Initial Setup + +1. Create a new Java project and add dependency [`org.itsallcode.openfasttrace:openfasttrace-api`](https://search.maven.org/artifact/org.itsallcode.openfasttrace/openfasttrace-api): + ```xml + + org.itsallcode.openfasttrace + openfasttrace-api + [latest version] + + ``` + +2. Create a new class (e.g. `com.example.oft.import.MyImporter`) implementing one of the following interfaces: + * [`org.itsallcode.openfasttrace.api.report.ReporterFactory`](https://github.com/itsallcode/openfasttrace/blob/main/api/src/main/java/org/itsallcode/openfasttrace/api/report/ReporterFactory.java): Generate tracing report + * [`org.itsallcode.openfasttrace.api.importer.ImporterFactory`](https://github.com/itsallcode/openfasttrace/blob/main/api/src/main/java/org/itsallcode/openfasttrace/api/importer/ImporterFactory.java): Import requirements from a new file format + * [`org.itsallcode.openfasttrace.api.exporter.ExporterFactory`](https://github.com/itsallcode/openfasttrace/blob/main/api/src/main/java/org/itsallcode/openfasttrace/api/exporter/ExporterFactory.java): Export requirements in a new file format + +3. Create a file in `src/main/resources/$INTERFACE_FQN`, using the fully qualified class name of the interface as file name. + +4. Add the fully qualified class name of your new plugin class to the new file, e.g. `com.example.oft.import.MyImporter` + +## Runtime Dependencies + +OpenFastTrace does not use any third-party runtime dependencies by design. You can add any dependencies to your plugin if required (see [note about packaging](#adding-third-party-dependencies)). + +**Warning** The plugin must not use any classes other than those included in the API `openfasttrace-api`. All other classes in OFT are internal and may change in incompatible ways even in patch releases. + +## Packaging + +Build your plugin as a normal Java JAR file, e.g. using `maven-jar-plugin`. The JAR must not contain `openfasttrace-api`. + +### Adding Third-Party Dependencies + +If your plugin uses third party dependencies, you have two options: +* Publish and install the plugin and its dependencies as separate JARs. +* Build a fat JAR, e.g. using `maven-shade-plugin` and include the plugin's dependencies. + + **Important:** do not include `openfasttrace-api` in the fat JAR to avoid having duplicate classes on the classpath at runtime. diff --git a/doc/plugins.md b/doc/plugins.md new file mode 100644 index 00000000..054b3bc1 --- /dev/null +++ b/doc/plugins.md @@ -0,0 +1,15 @@ +# Extending OpenFastTrace With Plugins + +Version 4.1.0 adds support for extending OFT with third-party plugins. + +## Installing Plugins + +You install a plugin by copying its JAR files to `$HOME/.oft/plugins//*.jar`. OFT will automatically load plugins from this location. To check which plugins are available, start OFT with command line argument `--log-level INFO`. This will log all available plugins and their location. + +## Available Plugins + +Currently no third-party plugins are available. If you want to add a new plugin, please create a [GitHub issue](https://github.com/itsallcode/openfasttrace/issues/new?assignees=&labels=&projects=&template=New_plugin.md). + +| Plugin Name | Plugin Type | Description | +|-------------|-------------|-------------| +| N/A | N/A | Currently, no third-party plugins are available for OpenFastTrace. | diff --git a/doc/user_guide.md b/doc/user_guide.md index 4302a20e..cb3894bd 100644 --- a/doc/user_guide.md +++ b/doc/user_guide.md @@ -568,7 +568,7 @@ The available color schemes are -l, --log-level -Log level for console logging. One of `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. +Log level for console logging. One of `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. Defaults to `WARNING`. ### Build Integration From 5a00343c340a7178ab98f39ec8c49d268ea266fc Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 12:04:33 +0200 Subject: [PATCH 34/44] Fix tests --- .../core/serviceloader/ServiceLoaderFactoryTest.java | 12 ++++++------ .../core/serviceloader/ServiceLoaderFactoryIT.java | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 5b8ba795..20948d8f 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -95,7 +95,7 @@ void findServiceOriginsSingleJar() throws IOException Files.createFile(pluginDir.resolve("plugin1.jar")); final List origins = testee().findServiceOrigins(); assertAll(() -> assertThat(origins, hasSize(2)), - () -> assertThat(origins.get(1).getClassLoader().getName(), equalTo("ServiceClassLoader-plugin1.jar"))); + () -> assertThat(origins.get(0).getClassLoader().getName(), equalTo("JarClassLoader-plugin1.jar"))); } @Test @@ -107,8 +107,8 @@ void findServiceOriginsMultiJar() throws IOException Files.createFile(pluginDir.resolve("plugin2.jar")); final List origins = testee().findServiceOrigins(); assertAll(() -> assertThat(origins, hasSize(2)), - () -> assertThat(origins.get(1).getClassLoader().getName(), - equalTo("ServiceClassLoader-plugin1.jar,plugin2.jar"))); + () -> assertThat(origins.get(0).getClassLoader().getName(), + equalTo("JarClassLoader-plugin1.jar,plugin2.jar"))); } @Test @@ -122,9 +122,9 @@ void findServiceOriginsMultiPlugins() throws IOException Files.createFile(pluginDir2.resolve("plugin2.jar")); final List origins = testee().findServiceOrigins(); assertAll(() -> assertThat(origins, hasSize(3)), + () -> assertThat(origins.get(0).getClassLoader().getName(), + equalTo("JarClassLoader-plugin1.jar")), () -> assertThat(origins.get(1).getClassLoader().getName(), - equalTo("ServiceClassLoader-plugin1.jar")), - () -> assertThat(origins.get(2).getClassLoader().getName(), - equalTo("ServiceClassLoader-plugin2.jar"))); + equalTo("JarClassLoader-plugin2.jar"))); } } diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java index 94b15a72..d17cb430 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java @@ -59,7 +59,7 @@ void loadServiceFromJar() throws IOException () -> assertThat(service.getClass().getName().toString(), equalTo("org.itsallcode.openfasttrace.report.plaintext.PlaintextReporterFactory")), () -> assertThat(pluginClassLoader.getName(), - startsWith("ServiceClassLoader-openfasttrace-reporter-plaintext")), + startsWith("JarClassLoader-openfasttrace-reporter-plaintext")), () -> assertThat(pluginClassLoader, not(sameInstance(Thread.currentThread().getContextClassLoader())))); } @@ -69,7 +69,8 @@ private void preparePlugin(final Path targetDir, final String filePattern) throw { final Path jar = findMatchingFile(targetDir, filePattern) .orElseThrow(() -> new AssertionError( - "Did not file matching '" + filePattern + "' in '" + targetDir + "'")); + "Did not file matching '" + filePattern + "' in '" + targetDir + + "'. Ensure the module was built with 'mvn package'.")); preparePlugin(jar); } From 2936f5bf8a1360224d42597ad7ba4ff4ce4e7ee5 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 12:13:57 +0200 Subject: [PATCH 35/44] Cleanup --- .../itsallcode/openfasttrace/core/cli/logging/LogLevel.java | 3 +++ .../openfasttrace/core/cli/logging/LoggingConfigurator.java | 1 + 2 files changed, 4 insertions(+) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java index 53111cdd..9951e38e 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -4,6 +4,9 @@ /** * Log levels for console logging. + *

+ * We can't use {@link java.util.logging.Level} directly for configuration + * because it is not an enum. */ public enum LogLevel { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index f7dd0a2d..c2dc83cc 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -10,6 +10,7 @@ /** * Configures console logging for the application. */ +// [impl->dsn~cli.plugins.log~1] public class LoggingConfigurator { private static final LogLevel DEFAULT_LOG_LEVEL = LogLevel.WARNING; From a2f7a7c113d0b1b1c81a82d707fb7b18b9f1b3a9 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 12:18:14 +0200 Subject: [PATCH 36/44] Increment version --- README.md | 8 ++++---- doc/changes/changes.md | 1 + doc/changes/changes_4.1.0.md | 16 ++++++++++++++++ parent/pom.xml | 2 +- 4 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 doc/changes/changes_4.1.0.md diff --git a/README.md b/README.md index e1065ec3..10af627d 100644 --- a/README.md +++ b/README.md @@ -72,10 +72,10 @@ OpenFastTrace at it's core is a Java Archive (short "[JAR](https://docs.oracle.c ### Getting Pre-Built Packages -Pre-Built JAR files (called `openfasttrace-4.0.1.jar`) are available from the following places: +Pre-Built JAR files (called `openfasttrace-4.1.0.jar`) are available from the following places: -* [Maven Central](https://repo1.maven.org/maven2/org/itsallcode/openfasttrace/openfasttrace/4.0.1/openfasttrace-4.0.1.jar) -* [GitHub](https://github.com/itsallcode/openfasttrace/releases/download/4.0.1/openfasttrace-4.0.1.jar) +* [Maven Central](https://repo1.maven.org/maven2/org/itsallcode/openfasttrace/openfasttrace/4.1.0/openfasttrace-4.1.0.jar) +* [GitHub](https://github.com/itsallcode/openfasttrace/releases/download/4.1.0/openfasttrace-4.1.0.jar) Check our [developer guide](doc/developer_guide.md#getting-the-openfasttrace-library) to learn how to use the OFT JAR as dependency in your own code with popular build tools. @@ -99,7 +99,7 @@ If you just want to run OFT: The most basic variant to run OpenFastTrace is directly from the JAR file via the command line: ```bash -java -jar product/target/openfasttrace-4.0.1.jar trace /path/to/directory/being/traced +java -jar product/target/openfasttrace-4.1.0.jar trace /path/to/directory/being/traced ``` If you want to run OFT automatically as part of a continuous build, we recommend using our plugins for [Gradle](https://github.com/itsallcode/openfasttrace-gradle) and [Maven](https://github.com/itsallcode/openfasttrace-maven-plugin). diff --git a/doc/changes/changes.md b/doc/changes/changes.md index b2796eb5..96de5a6a 100644 --- a/doc/changes/changes.md +++ b/doc/changes/changes.md @@ -1,5 +1,6 @@ # Changes +* [4.1.0](changes_4.1.0.md) * [4.0.1](changes_4.0.1.md) * [4.0.0](changes_4.0.0.md) * [3.8.0](changes_3.8.0.md) diff --git a/doc/changes/changes_4.1.0.md b/doc/changes/changes_4.1.0.md new file mode 100644 index 00000000..dfd1096f --- /dev/null +++ b/doc/changes/changes_4.1.0.md @@ -0,0 +1,16 @@ +# OpenFastTrace 4.1.0, released 2024-06-?? + +Code name: Third-party plugins + +## Summary + +This release adds support for loading third-party plugins from external JAR files. See the documentation for details: + +* [Installation](../plugins.md) +* [Plugin developer guide](../plugin_developer_guide.md) + +The release also adds command line option `--log-level` that allows configuring the log level. Possible values are `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. The default log level is `WARNING`. + +## Features + +* #413: Added support for third-party plugins diff --git a/parent/pom.xml b/parent/pom.xml index 9efab9b0..411f8021 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -8,7 +8,7 @@ Free requirement tracking suite https://github.com/itsallcode/openfasttrace - 4.0.1 + 4.1.0 17 5.11.0-M2 3.3.0 From cb64d24949c6126a2cae742a1a93baf9ea89c953 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 23 Jun 2024 12:25:51 +0200 Subject: [PATCH 37/44] Fix sonar findings --- .../openfasttrace/core/cli/logging/LogLevel.java | 2 +- .../core/cli/logging/LoggingConfigurator.java | 4 ++-- .../core/serviceloader/ServiceOrigin.java | 13 ++++++++----- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java index 9951e38e..c78321f0 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -49,7 +49,7 @@ public enum LogLevel private final Level julLevel; - private LogLevel(final Level julLevel) + LogLevel(final Level julLevel) { this.julLevel = julLevel; } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index c2dc83cc..7886d6c2 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -11,7 +11,7 @@ * Configures console logging for the application. */ // [impl->dsn~cli.plugins.log~1] -public class LoggingConfigurator +public final class LoggingConfigurator { private static final LogLevel DEFAULT_LOG_LEVEL = LogLevel.WARNING; private static final String CONFIG_TEMPLATE = """ @@ -57,7 +57,7 @@ private String getConfigContent() return CONFIG_TEMPLATE.replace("$LOG_LEVEL", this.logLevel.getJulLogLevel()); } - private void configureLogManager(final LogManager logManager, final String configContent) + private static void configureLogManager(final LogManager logManager, final String configContent) { try (InputStream config = new ByteArrayInputStream(configContent.getBytes(StandardCharsets.UTF_8))) { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 12105f66..89ac0a7e 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -2,8 +2,11 @@ import static java.util.stream.Collectors.joining; +import java.io.IOException; +import java.io.UncheckedIOException; import java.net.*; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; /** @@ -89,14 +92,14 @@ private static URL toUrl(final Path path) */ void close(); - static final class JarFileOrigin implements ServiceOrigin + final class JarFileOrigin implements ServiceOrigin { private final List jars; private final URLClassLoader classLoader; JarFileOrigin(final List jars, final URLClassLoader classLoader) { - this.jars = jars; + this.jars = new ArrayList<>(jars); this.classLoader = classLoader; } @@ -113,9 +116,9 @@ public void close() { classLoader.close(); } - catch (final Exception exception) + catch (final IOException exception) { - throw new IllegalStateException("Error closing class loader " + classLoader, exception); + throw new UncheckedIOException("Error closing class loader " + classLoader, exception); } } @@ -126,7 +129,7 @@ public String toString() } } - static final class CurrentClassPathOrigin implements ServiceOrigin + final class CurrentClassPathOrigin implements ServiceOrigin { private final ClassLoader classLoader; From 73296dca3a6a0b6d082865bf4a15a34cd4d65c68 Mon Sep 17 00:00:00 2001 From: Christoph Pirkl <4711730+kaklakariada@users.noreply.github.com> Date: Sat, 20 Jul 2024 16:27:48 +0200 Subject: [PATCH 38/44] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sebastian Bär --- .../openfasttrace/core/cli/CliArguments.java | 8 +++---- .../core/cli/logging/LogLevel.java | 2 +- .../core/cli/logging/LoggingConfigurator.java | 4 ++-- .../core/importer/ImporterFactoryLoader.java | 4 ++-- .../serviceloader/ChildFirstClassLoader.java | 2 +- .../serviceloader/ClassPathServiceLoader.java | 4 ++-- .../serviceloader/ServiceLoaderFactory.java | 17 ++++++------- .../core/serviceloader/ServiceOrigin.java | 24 +++++++++---------- core/src/main/resources/usage.txt | 2 +- .../ClassPathServiceLoaderTest.java | 7 ++---- .../ServiceLoaderFactoryTest.java | 2 +- doc/changes/changes_4.1.0.md | 2 +- doc/spec/design.md | 7 +++++- doc/spec/system_requirements.md | 8 +++++++ doc/user_guide.md | 2 +- .../TestInitializingServiceLoader.java | 2 +- 16 files changed, 52 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java index ac4430a6..629ffdfb 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CliArguments.java @@ -424,9 +424,9 @@ public void setDetailsSectionDisplay(final DetailsSectionDisplay detailsSectionD } /** - * Get the log level console logging. + * Get the log level for console logging. * - * @return the log level + * @return log level */ public Optional getLogLevel() { @@ -437,7 +437,7 @@ public Optional getLogLevel() * Set the log level for console logging. * * @param logLevel - * the log level + * log level */ public void setLogLevel(final LogLevel logLevel) { @@ -448,7 +448,7 @@ public void setLogLevel(final LogLevel logLevel) * Set the log level for console logging. * * @param logLevel - * the log level + * log level */ public void setL(final LogLevel logLevel) { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java index c78321f0..13240497 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -13,7 +13,7 @@ public enum LogLevel /** * OFF is a special level that can be used to turn off logging. */ - OFF(Level.OFF), + OFF(Level.NONE), /** * SEVERE is a message level indicating a serious failure. */ diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index 7886d6c2..8fa00919 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -33,7 +33,7 @@ private LoggingConfigurator(final LogLevel logLevel) * Create a new logging configurator. * * @param arguments - * the command line arguments. + * command line arguments. * @return a new logging configurator. */ public static LoggingConfigurator create(final CliArguments arguments) @@ -49,7 +49,7 @@ public void configureLogging() final LogManager logManager = LogManager.getLogManager(); configureLogManager(logManager, getConfigContent()); final Logger rootLogger = logManager.getLogger(""); - rootLogger.info(() -> "Logging configured with level " + this.logLevel + "."); + rootLogger.debug(() -> "Logging configured with level " + this.logLevel + "."); } private String getConfigContent() 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 8dd06119..9fec635d 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 @@ -35,11 +35,11 @@ public class ImporterFactoryLoader * Create a new loader for the given context. * * @param context - * the context for the new loader. + * context for the new loader */ public ImporterFactoryLoader(final ImporterContext context) { - // [impl->dsn~plugins.loading.plugin_types~1] + // [impl->dsn~plugins.loading.plugin-types~1] this(InitializingServiceLoader.load(ImporterFactory.class, context)); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java index 917e2807..19602124 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java @@ -53,7 +53,7 @@ private Class loadClassInternally(final String name, final boolean resolve) t } catch (final ClassNotFoundException ignore) { - // Class does not exist in the given urls. + // Class does not exist in the given URLs. // Let's try finding it in our parent classloader. // This will throw ClassNotFoundException on failure. return super.loadClass(name, resolve); diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index c6983217..3f84e1f5 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -6,7 +6,7 @@ import java.util.stream.Stream; /** - * A service loader that loads services from the classpath using a given + * A service loader that loads services from the class path using a given * {@link ServiceOrigin}. */ final class ClassPathServiceLoader implements Loader @@ -52,7 +52,7 @@ private boolean filterOtherClassLoader(final T service) private void logService(final T service) { - LOGGER.info(() -> "Loaded service from " + serviceOrigin + ": " + service.getClass().getName()); + LOGGER.info(() -> "Loading service '" + service.getClass().getName() + "'from " + serviceOrigin + "."; } @Override diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java index ffbfd41e..559a03d1 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactory.java @@ -21,7 +21,7 @@ class ServiceLoaderFactory * Create a new factory for service {@link Loader}. * * @param pluginsDirectory - * the directory to search for plugins + * directory to search for plugins * @param searchCurrentClasspath * whether to search the current classpath for plugins. This is * useful for testing to avoid loading plugins twice. @@ -117,32 +117,29 @@ private static Optional originForPath(final Path path) { if (isJarFile(path)) { - LOGGER.fine(() -> "Found single plugin jar '" + path + "'."); + LOGGER.fine(() -> "Found single plugin JAR '" + path + "'."); return Optional.of(ServiceOrigin.forJars(List.of(path))); } if (!Files.isDirectory(path)) { - LOGGER.fine(() -> "Ignore unsupported file '" + path + "'."); + LOGGER.fine(() -> "Ignoring plugin search path '" + path + "' because it is not a directory."); return Optional.empty(); } final List jars = findJarsInDir(path); if (jars.isEmpty()) { - LOGGER.fine(() -> "Ignore empty plugin directory '" + path + "'."); + LOGGER.fine(() -> "Ignoring empty plugin directory '" + path + "'."); return Optional.empty(); } - LOGGER.fine(() -> "Found " + jars.size() + " in '" + path + "': " + jars + "."); + LOGGER.fine(() -> "Found " + jars.size() + " JAR files in '" + path + "': " + jars + "."); return Optional.of(ServiceOrigin.forJars(jars)); } private static List findJarsInDir(final Path path) { - try + try (Stream stream = Files.list(path)) { - try (Stream stream = Files.list(path)) - { - return stream.filter(ServiceLoaderFactory::isJarFile).toList(); - } + return stream.filter(ServiceLoaderFactory::isJarFile).toList(); } catch (final IOException exception) { diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java index 89ac0a7e..cbe42107 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOrigin.java @@ -10,15 +10,15 @@ import java.util.List; /** - * Origin of a service, either the current classpath or a list of jar files. + * Origin of a service, either the current class path or a list of JAR files. */ // [impl->dsn~plugins.loading~1] interface ServiceOrigin extends AutoCloseable { /** - * Create a service origin for the current classpath. + * Create a service origin for the current class path. * - * @return a service origin for the current classpath + * @return a service origin for the current class path */ static ServiceOrigin forCurrentClassPath() { @@ -31,11 +31,11 @@ private static ClassLoader getBaseClassLoader() } /** - * Create a service origin for a single jar file. + * Create a service origin for a single JAR file. * * @param jar - * path to the jar file - * @return a service origin for the jar file + * path to the JAR file + * @return a service origin for the JAR file */ static ServiceOrigin forJar(final Path jar) { @@ -43,13 +43,13 @@ static ServiceOrigin forJar(final Path jar) } /** - * Create a service origin for a list of jar files. + * Create a service origin for a list of JAR files. * * @param jars - * list of paths to jar files - * @return a service origin for the jar files + * list of paths to JAR files + * @return a service origin for the JAR files */ - // [impl->dsn~plugins.loading.separate_classloader~1] + // [impl->dsn~plugins.loading.separate-classloader~1] static ServiceOrigin forJars(final List jars) { return new JarFileOrigin(jars, createClassLoader(jars)); @@ -74,9 +74,9 @@ private static URL toUrl(final Path path) { return path.toUri().toURL(); } - catch (final MalformedURLException e) + catch (final MalformedURLException exception) { - throw new IllegalStateException("Error converting path " + path + " to url", e); + throw new IllegalStateException("Error converting path " + path + " to url", exception); } } diff --git a/core/src/main/resources/usage.txt b/core/src/main/resources/usage.txt index f1616092..0d25bfe5 100644 --- a/core/src/main/resources/usage.txt +++ b/core/src/main/resources/usage.txt @@ -37,7 +37,7 @@ Common options: list. Add a single underscore as first item in the list to also import items without any tags. -l, --log-level Log level for console logging. One of - "OFF", "SEVERE", "WARNING", "INFO", "CONFIG", + "NONE", "SEVERE", "WARNING", "INFO", "CONFIG", "FINE", "FINER", "FINEST", "ALL". Defaults to "WARNING". diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java index 365e9983..4f27e2b6 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java @@ -36,7 +36,7 @@ void loadingFindsNothing() final List services = ClassPathServiceLoader .create(ReporterFactory.class, ServiceOrigin.forCurrentClassPath()).load() .toList(); - assertThat(services, hasSize(0)); + assertThat(services, emptyIterable()); } @Test @@ -49,10 +49,7 @@ void loadingReturnsService(@Mock final ServiceLoader serviceLoader when(originMock.getClassLoader()).thenReturn(DummyServiceImpl.class.getClassLoader()); final List services = new ClassPathServiceLoader(originMock, serviceLoaderMock) .load().toList(); - assertAll(() -> assertThat(services, hasSize(1)), - () -> assertThat(services.get(0), not(nullValue())), - () -> assertThat(services.get(0), instanceOf(DummyServiceImpl.class)), - () -> assertThat(services.get(0), sameInstance(service))); + assertThat(services, contains(sameInstance(service))); } @Test diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 20948d8f..3f5653d8 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -26,7 +26,7 @@ void noPluginJar() assertThat(testee().createLoader(ReporterFactory.class).load().toList(), empty()); } - private ServiceLoaderFactory testee() + private ServiceLoaderFactory factory() { return new ServiceLoaderFactory(tempDir, true); } diff --git a/doc/changes/changes_4.1.0.md b/doc/changes/changes_4.1.0.md index dfd1096f..533a3608 100644 --- a/doc/changes/changes_4.1.0.md +++ b/doc/changes/changes_4.1.0.md @@ -9,7 +9,7 @@ This release adds support for loading third-party plugins from external JAR file * [Installation](../plugins.md) * [Plugin developer guide](../plugin_developer_guide.md) -The release also adds command line option `--log-level` that allows configuring the log level. Possible values are `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. The default log level is `WARNING`. +The release also adds command line option `--log-level` that allows configuring the log level. Possible values are `NONE`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. The default log level is `WARNING`. ## Features diff --git a/doc/spec/design.md b/doc/spec/design.md index 16b7fcf0..e5ee23c5 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -133,9 +133,12 @@ OFT loads plugins at startup from these locations: * Third party plugins from folder `$HOME/.oft/plugins//*.jar` Rationale: + * A plugin may consist of multiple JARs, e.g. the plugin itself and it's dependencies. +* A plugin might need extra resources Covers: + * [`req~plugins.loading~1`](system_requirements.md#loading-plugins) Needs: impl, utest, itest @@ -150,6 +153,7 @@ Rationale: Separating plugins from each other avoids conflicts between potentially duplicate classes in plugin. Covers: + * [`req~plugins.loading~1`](system_requirements.md#loading-plugins) Needs: impl, utest @@ -157,7 +161,8 @@ Needs: impl, utest ### Loader Supports Plugin Types `dsn~plugins.loading.plugin_types~1` -The Plugin loader supports loading factories the following plugin types: +The Plugin loader supports loading factories for the following plugin types: + * Importers: `org.itsallcode.openfasttrace.api.importer.ImporterFactory` * Exporters: `org.itsallcode.openfasttrace.api.exporter.ExporterFactory` * Reports: `org.itsallcode.openfasttrace.api.report.ReporterFactory` diff --git a/doc/spec/system_requirements.md b/doc/spec/system_requirements.md index 5ef883d2..afa50894 100644 --- a/doc/spec/system_requirements.md +++ b/doc/spec/system_requirements.md @@ -186,6 +186,8 @@ Rationale: * Some use cases or proprietary file formats might only be relevant for a very small user group. It does not make sense to integrate this into the core OFT product. * Some importers/exporters require additional dependencies that we don't want to include in the core OFT product. +* We want to be able to release OFT and the plugins independently. Especially if the plugins have many dependencies, it is expected that they need frequent security updates. By keeping them separate, we make sure that users don't have to update OFT whenever a plugin needs a security update. + Needs: req ## Functional Requirements @@ -822,18 +824,22 @@ Rationale: Covers: * [feat~plugins~1](#third-party-plugins) + Needs: dsn #### Supported Plugin Types `req~plugins.types~1` OFT supports the following plugin types: + * Importers add support for importing requirements from additional file formats. * Exporters add support for exporting requirements in additional file formats. * Reports add support for generating reports in additional formats. Covers: + * [feat~plugins~1](#third-party-plugins) + Needs: dsn #### Logging Available Plugins @@ -849,5 +855,7 @@ Rationale: This is helpful for debugging in case OFT does not use a plugin as expected. Covers: + * [feat~plugins~1](#third-party-plugins) + Needs: dsn diff --git a/doc/user_guide.md b/doc/user_guide.md index 6b36b6e9..189dc798 100644 --- a/doc/user_guide.md +++ b/doc/user_guide.md @@ -569,7 +569,7 @@ The available color schemes are -l, --log-level -Log level for console logging. One of `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. Defaults to `WARNING`. +Log level for console logging. One of `NONE`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. Defaults to `WARNING`. ### Build Integration diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java index 0d3b1ff0..7b4d4fce 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java @@ -76,7 +76,7 @@ void testExporterFactoriesRegistered() } } - // [itest->dsn~plugins.loading.plugin_types~1] + // [itest->dsn~plugins.loading.plugin-types~1] @Test void testReporterFactoriesRegistered() { From d4915594a0c1374a6aba2662767f673ab06c72f6 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 20 Jul 2024 16:28:15 +0200 Subject: [PATCH 39/44] Fix review findings --- .../openfasttrace/core/serviceloader/ChildFirstClassLoader.java | 2 +- .../core/serviceloader/ChildFirstClassLoaderTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java index 917e2807..3bf9bfe7 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoader.java @@ -8,7 +8,7 @@ * then from the parent class loader, unlike {@link URLClassLoader} which does * it the other way around. *

- * This allows us to prefer externl plugins over plugins on the classpath + * This allows us to prefer external plugins over plugins on the classpath * included with OFT. *

* This is based on diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java index d01a1ca4..9cd24e44 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ChildFirstClassLoaderTest.java @@ -49,7 +49,7 @@ void classFoundInJar() throws ClassNotFoundException, IOException } /** - * We can't mock the ClassLoader using Mockito because method + * We can't stub the ClassLoader using Mockito because method * {@link ClassLoader#loadClass(String, boolean)} is protected. */ private static class MockClassLoader extends ClassLoader From c6b34a1fa1d16590005280f5e3248c834c22d8da Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 20 Jul 2024 16:33:41 +0200 Subject: [PATCH 40/44] Fix compile errors --- .../core/cli/logging/LogLevel.java | 2 +- .../core/cli/logging/LoggingConfigurator.java | 2 +- .../serviceloader/ClassPathServiceLoader.java | 2 +- core/src/main/resources/usage.txt | 2 +- .../ServiceLoaderFactoryTest.java | 18 +++++++++--------- doc/changes/changes_4.1.0.md | 2 +- doc/user_guide.md | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java index 13240497..c78321f0 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -13,7 +13,7 @@ public enum LogLevel /** * OFF is a special level that can be used to turn off logging. */ - OFF(Level.NONE), + OFF(Level.OFF), /** * SEVERE is a message level indicating a serious failure. */ diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index 8fa00919..12a31eab 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -49,7 +49,7 @@ public void configureLogging() final LogManager logManager = LogManager.getLogManager(); configureLogManager(logManager, getConfigContent()); final Logger rootLogger = logManager.getLogger(""); - rootLogger.debug(() -> "Logging configured with level " + this.logLevel + "."); + rootLogger.fine(() -> "Logging configured with level " + this.logLevel + "."); } private String getConfigContent() diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index 3f84e1f5..05d35732 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -52,7 +52,7 @@ private boolean filterOtherClassLoader(final T service) private void logService(final T service) { - LOGGER.info(() -> "Loading service '" + service.getClass().getName() + "'from " + serviceOrigin + "."; + LOGGER.info(() -> "Loading service '" + service.getClass().getName() + "'from " + serviceOrigin + "."); } @Override diff --git a/core/src/main/resources/usage.txt b/core/src/main/resources/usage.txt index 0d25bfe5..f1616092 100644 --- a/core/src/main/resources/usage.txt +++ b/core/src/main/resources/usage.txt @@ -37,7 +37,7 @@ Common options: list. Add a single underscore as first item in the list to also import items without any tags. -l, --log-level Log level for console logging. One of - "NONE", "SEVERE", "WARNING", "INFO", "CONFIG", + "OFF", "SEVERE", "WARNING", "INFO", "CONFIG", "FINE", "FINER", "FINEST", "ALL". Defaults to "WARNING". diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 3f5653d8..f87621a8 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -23,7 +23,7 @@ class ServiceLoaderFactoryTest @Test void noPluginJar() { - assertThat(testee().createLoader(ReporterFactory.class).load().toList(), empty()); + assertThat(factory().createLoader(ReporterFactory.class).load().toList(), empty()); } private ServiceLoaderFactory factory() @@ -42,7 +42,7 @@ void findServiceSkipCurrentClassLoader() throws IOException void findServiceMissingParentDir() throws IOException { Files.delete(tempDir); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertNoPlugins(origins); } @@ -55,7 +55,7 @@ private void assertNoPlugins(final List origins) throws MultipleF @Test void findServiceOriginsPluginDir() { - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertNoPlugins(origins); } @@ -64,7 +64,7 @@ void findServiceOriginsEmptyPluginDir() throws IOException { final Path pluginDir = tempDir.resolve("plugin1"); Files.createDirectories(pluginDir); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertNoPlugins(origins); } @@ -74,7 +74,7 @@ void findServiceOriginsIgnoresNonJarFiles() throws IOException final Path pluginDir = tempDir.resolve("plugin1"); Files.createDirectories(pluginDir); Files.createFile(pluginDir.resolve("plugin1.txt")); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertNoPlugins(origins); } @@ -83,7 +83,7 @@ void findServiceOriginsIgnoresDirectories() throws IOException { final Path pluginDir = tempDir.resolve("plugin1"); Files.createDirectories(pluginDir.resolve("ignored-directory")); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertNoPlugins(origins); } @@ -93,7 +93,7 @@ void findServiceOriginsSingleJar() throws IOException final Path pluginDir = tempDir.resolve("plugin1"); Files.createDirectories(pluginDir); Files.createFile(pluginDir.resolve("plugin1.jar")); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertAll(() -> assertThat(origins, hasSize(2)), () -> assertThat(origins.get(0).getClassLoader().getName(), equalTo("JarClassLoader-plugin1.jar"))); } @@ -105,7 +105,7 @@ void findServiceOriginsMultiJar() throws IOException Files.createDirectories(pluginDir); Files.createFile(pluginDir.resolve("plugin1.jar")); Files.createFile(pluginDir.resolve("plugin2.jar")); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertAll(() -> assertThat(origins, hasSize(2)), () -> assertThat(origins.get(0).getClassLoader().getName(), equalTo("JarClassLoader-plugin1.jar,plugin2.jar"))); @@ -120,7 +120,7 @@ void findServiceOriginsMultiPlugins() throws IOException Files.createDirectories(pluginDir2); Files.createFile(pluginDir1.resolve("plugin1.jar")); Files.createFile(pluginDir2.resolve("plugin2.jar")); - final List origins = testee().findServiceOrigins(); + final List origins = factory().findServiceOrigins(); assertAll(() -> assertThat(origins, hasSize(3)), () -> assertThat(origins.get(0).getClassLoader().getName(), equalTo("JarClassLoader-plugin1.jar")), diff --git a/doc/changes/changes_4.1.0.md b/doc/changes/changes_4.1.0.md index 533a3608..dfd1096f 100644 --- a/doc/changes/changes_4.1.0.md +++ b/doc/changes/changes_4.1.0.md @@ -9,7 +9,7 @@ This release adds support for loading third-party plugins from external JAR file * [Installation](../plugins.md) * [Plugin developer guide](../plugin_developer_guide.md) -The release also adds command line option `--log-level` that allows configuring the log level. Possible values are `NONE`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. The default log level is `WARNING`. +The release also adds command line option `--log-level` that allows configuring the log level. Possible values are `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. The default log level is `WARNING`. ## Features diff --git a/doc/user_guide.md b/doc/user_guide.md index 189dc798..6b36b6e9 100644 --- a/doc/user_guide.md +++ b/doc/user_guide.md @@ -569,7 +569,7 @@ The available color schemes are -l, --log-level -Log level for console logging. One of `NONE`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. Defaults to `WARNING`. +Log level for console logging. One of `OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`. Defaults to `WARNING`. ### Build Integration From 959f796e4771dc9246f3b35616c7492705ae6146 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 20 Jul 2024 16:40:23 +0200 Subject: [PATCH 41/44] Fix requirements tracing --- .../openfasttrace/core/exporter/ExporterFactoryLoader.java | 2 +- .../openfasttrace/core/report/ReporterFactoryLoader.java | 2 +- .../core/serviceloader/ClassPathServiceLoader.java | 2 +- .../openfasttrace/core/serviceloader/ServiceOriginTest.java | 2 +- doc/spec/design.md | 4 ++-- .../core/serviceloader/TestInitializingServiceLoader.java | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java index c3d774dd..d2edb904 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/exporter/ExporterFactoryLoader.java @@ -23,7 +23,7 @@ public class ExporterFactoryLoader */ public ExporterFactoryLoader(final ExporterContext context) { - // [impl->dsn~plugins.loading.plugin_types~1] + // [impl->dsn~plugins.loading.plugin-types~1] this(InitializingServiceLoader.load(ExporterFactory.class, context)); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java index ad58a2d7..ac9d22cc 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/report/ReporterFactoryLoader.java @@ -24,7 +24,7 @@ public class ReporterFactoryLoader */ public ReporterFactoryLoader(final ReporterContext context) { - // [impl->dsn~plugins.loading.plugin_types~1] + // [impl->dsn~plugins.loading.plugin-types~1] this(InitializingServiceLoader.load(ReporterFactory.class, context)); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java index 05d35732..81b0b42c 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoader.java @@ -52,7 +52,7 @@ private boolean filterOtherClassLoader(final T service) private void logService(final T service) { - LOGGER.info(() -> "Loading service '" + service.getClass().getName() + "'from " + serviceOrigin + "."); + LOGGER.fine(() -> "Loading service '" + service.getClass().getName() + "' from " + serviceOrigin + "."); } @Override diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java index 94f16616..a9472123 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceOriginTest.java @@ -35,7 +35,7 @@ void currentClassPathOriginToString() assertThat(origin, hasToString(startsWith("CurrentClassPathOrigin [classLoader="))); } - // [utest->dsn~plugins.loading.separate_classloader~1] + // [utest->dsn~plugins.loading.separate-classloader~1] @Test void currentClassPathGetClassLoader() { diff --git a/doc/spec/design.md b/doc/spec/design.md index e5ee23c5..47a084b6 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -144,7 +144,7 @@ Covers: Needs: impl, utest, itest ### Plugin Loader Uses Separate ClassLoaders -`dsn~plugins.loading.separate_classloader~1` +`dsn~plugins.loading.separate-classloader~1` The Plugin loader uses a separate ClassLoader for each plugin. @@ -159,7 +159,7 @@ Covers: Needs: impl, utest ### Loader Supports Plugin Types -`dsn~plugins.loading.plugin_types~1` +`dsn~plugins.loading.plugin-types~1` The Plugin loader supports loading factories for the following plugin types: diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java index 7b4d4fce..63f89839 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/TestInitializingServiceLoader.java @@ -41,7 +41,7 @@ void testNoServicesRegistered() assertThat(voidServiceLoader.load().toList(), emptyIterable()); } - // [itest->dsn~plugins.loading.plugin_types~1] + // [itest->dsn~plugins.loading.plugin-types~1] @Test void testImporterFactoriesRegistered() { @@ -61,7 +61,7 @@ void testImporterFactoriesRegistered() } } - // [itest->dsn~plugins.loading.plugin_types~1] + // [itest->dsn~plugins.loading.plugin-types~1] @Test void testExporterFactoriesRegistered() { From 0d9ce17ed1439e48d67f38c3beebc585b2cc691f Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 20 Jul 2024 17:08:46 +0200 Subject: [PATCH 42/44] Implement review findings --- .../core/cli/logging/LogLevel.java | 6 +++--- .../core/cli/logging/LoggingConfigurator.java | 2 +- .../core/serviceloader/ClassPathHelper.java | 19 ++++++++++++------- .../ClassPathServiceLoaderTest.java | 1 - .../serviceloader/DelegatingLoaderTest.java | 16 ++++++++-------- .../ServiceLoaderFactoryTest.java | 11 ++++++----- core/src/test/resources/logging.properties | 2 +- doc/spec/system_requirements.md | 1 + .../serviceloader/ServiceLoaderFactoryIT.java | 11 +++++------ 9 files changed, 37 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java index c78321f0..496b1817 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LogLevel.java @@ -5,8 +5,8 @@ /** * Log levels for console logging. *

- * We can't use {@link java.util.logging.Level} directly for configuration - * because it is not an enum. + * We can't use {@code java.util.logging} (JUL) {@link java.util.logging.Level} + * directly for configuration because it is not an enum. */ public enum LogLevel { @@ -54,7 +54,7 @@ public enum LogLevel this.julLevel = julLevel; } - String getJulLogLevel() + String getJavaUtilLoggingLogLevel() { return julLevel.getName(); } diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java index 12a31eab..c9e36d38 100644 --- a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java +++ b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/logging/LoggingConfigurator.java @@ -54,7 +54,7 @@ public void configureLogging() private String getConfigContent() { - return CONFIG_TEMPLATE.replace("$LOG_LEVEL", this.logLevel.getJulLogLevel()); + return CONFIG_TEMPLATE.replace("$LOG_LEVEL", this.logLevel.getJavaUtilLoggingLogLevel()); } private static void configureLogManager(final LogManager logManager, final String configContent) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java index 1b7c4cc7..14fa39cc 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathHelper.java @@ -45,13 +45,7 @@ static Path findJarForClass(final String className) final URL url = urls.nextElement(); if ("jar".equals(url.getProtocol())) { - String jarPath = url.getPath().substring(5, url.getPath().indexOf("!")); - if (OS.WINDOWS.isCurrentOs()) - { - // Remove leading slash of "/C:/Users/user/.m2/..." - jarPath = jarPath.substring(1); - } - final Path path = Path.of(jarPath); + final Path path = Path.of(getJarPath(url)); assertTrue(Files.exists(path)); return path; } @@ -59,6 +53,17 @@ static Path findJarForClass(final String className) throw new AssertionError("No jar found containing " + className); } + private static String getJarPath(final URL url) + { + final String jarPath = url.getPath().substring(5, url.getPath().indexOf("!")); + if (OS.WINDOWS.isCurrentOs()) + { + // Remove leading slash of "/C:/Users/user/.m2/..." + return jarPath.substring(1); + } + return jarPath; + } + private static Enumeration getResources(final String resourceName) { try diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java index 4f27e2b6..8e1ae46e 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ClassPathServiceLoaderTest.java @@ -2,7 +2,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; -import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java index 2af1775e..9a5591c6 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/DelegatingLoaderTest.java @@ -22,6 +22,14 @@ void noDelegates() assertThat(load(List.of()), empty()); } + private List load(final List> delegates) + { + try (DelegatingLoader loader = new DelegatingLoader(delegates)) + { + return loader.load().toList(); + } + } + @Test void emptyDelegate(@Mock final Loader loaderMock) { @@ -47,14 +55,6 @@ void loadsDelegatesInOrder(@Mock final Loader loaderMock1, contains(serviceMock1, serviceMock2)); } - private List load(final List> delegates) - { - try (DelegatingLoader loader = new DelegatingLoader(delegates)) - { - return loader.load().toList(); - } - } - static interface DummyService { } diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index f87621a8..91d87af6 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -34,12 +34,12 @@ private ServiceLoaderFactory factory() @Test void findServiceSkipCurrentClassLoader() throws IOException { - Files.delete(tempDir); - assertThat(new ServiceLoaderFactory(tempDir, false).findServiceOrigins(), empty()); + final Path missingDirectory = tempDir.resolve("missing-dir"); + assertThat(new ServiceLoaderFactory(missingDirectory, false).findServiceOrigins(), empty()); } @Test - void findServiceMissingParentDir() throws IOException + void findServiceMissingParentDirFindsNoPlugins() throws IOException { Files.delete(tempDir); final List origins = factory().findServiceOrigins(); @@ -48,8 +48,9 @@ void findServiceMissingParentDir() throws IOException private void assertNoPlugins(final List origins) throws MultipleFailuresError { - assertAll(() -> assertThat(origins, hasSize(1)), - () -> assertThat(origins.get(0).getClassLoader().getName(), equalTo("app"))); + assertThat(origins, contains( + hasProperty("classLoader", + hasProperty("name", equalTo("app"))))); } @Test diff --git a/core/src/test/resources/logging.properties b/core/src/test/resources/logging.properties index 20697e38..edc57772 100644 --- a/core/src/test/resources/logging.properties +++ b/core/src/test/resources/logging.properties @@ -7,4 +7,4 @@ java.util.logging.SimpleFormatter.format = %1$tF %1$tT [%4$s] %2$s - %5$s %6$s%n org.itsallcode.openfasttrace.testutil.log.NoOpLoggingHandler.level = ALL -org.itsallcode.openfasttrace.level = INFO +org.itsallcode.openfasttrace.level = FINEST diff --git a/doc/spec/system_requirements.md b/doc/spec/system_requirements.md index afa50894..d739ab9f 100644 --- a/doc/spec/system_requirements.md +++ b/doc/spec/system_requirements.md @@ -821,6 +821,7 @@ Rationale: * Plugins must be only installed once in the correct location. * This requires no additional configuration by the user. +* OFT adheres to the standard locations for plugin installation depending on the OS. Covers: * [feat~plugins~1](#third-party-plugins) diff --git a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java index d17cb430..5ab223eb 100644 --- a/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java +++ b/product/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryIT.java @@ -31,7 +31,7 @@ class ServiceLoaderFactoryIT void loadServiceFromWrongJar() throws IOException { preparePlugin(Path.of("../reporter/plaintext/target"), - "openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\-javadoc.jar"); + Pattern.compile("openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\-javadoc.jar")); try (Loader loader = createLoader()) { final List service = loader.load().toList(); @@ -48,7 +48,7 @@ private Loader createLoader() void loadServiceFromJar() throws IOException { preparePlugin(Path.of("../reporter/plaintext/target"), - "openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\.jar"); + Pattern.compile("openfasttrace-reporter-plaintext-\\d\\.\\d\\.\\d\\.jar")); try (Loader loader = createLoader()) { final List services = loader.load().toList(); @@ -65,7 +65,7 @@ void loadServiceFromJar() throws IOException } } - private void preparePlugin(final Path targetDir, final String filePattern) throws TestAbortedException, IOException + private void preparePlugin(final Path targetDir, final Pattern filePattern) throws TestAbortedException, IOException { final Path jar = findMatchingFile(targetDir, filePattern) .orElseThrow(() -> new AssertionError( @@ -74,10 +74,9 @@ private void preparePlugin(final Path targetDir, final String filePattern) throw preparePlugin(jar); } - private Optional findMatchingFile(final Path dir, final String filePattern) throws IOException + private Optional findMatchingFile(final Path dir, final Pattern filePattern) throws IOException { - final Pattern pattern = Pattern.compile(filePattern); - return Files.list(dir).filter(file -> pattern.matcher(file.getFileName().toString()).matches()) + return Files.list(dir).filter(file -> filePattern.matcher(file.getFileName().toString()).matches()) .findFirst(); } From b413699011d93b048fe898f2c160693868ac7fd7 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sat, 20 Jul 2024 17:22:38 +0200 Subject: [PATCH 43/44] Fix sonar warning --- .../core/serviceloader/ServiceLoaderFactoryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java index 91d87af6..2741e34d 100644 --- a/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java +++ b/core/src/test/java/org/itsallcode/openfasttrace/core/serviceloader/ServiceLoaderFactoryTest.java @@ -32,7 +32,7 @@ private ServiceLoaderFactory factory() } @Test - void findServiceSkipCurrentClassLoader() throws IOException + void findServiceSkipCurrentClassLoader() { final Path missingDirectory = tempDir.resolve("missing-dir"); assertThat(new ServiceLoaderFactory(missingDirectory, false).findServiceOrigins(), empty()); From 65167451a68ff88ef8ee67536283216549d0da70 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Sun, 11 Aug 2024 09:27:40 +0200 Subject: [PATCH 44/44] Update release date --- doc/changes/changes_4.1.0.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changes/changes_4.1.0.md b/doc/changes/changes_4.1.0.md index 2a694f42..eee32d73 100644 --- a/doc/changes/changes_4.1.0.md +++ b/doc/changes/changes_4.1.0.md @@ -1,4 +1,4 @@ -# OpenFastTrace 4.1.0, released 2024-06-?? +# OpenFastTrace 4.1.0, released 2024-08-11 Code name: Third-party plugins