Redesign module system with better dependencies

build.gradle.kts

@@ -1,6 +1,7 @@
 plugins {
     `java-library`
     `maven-publish`
+    jacoco
 }
 
 group = "dev.emortal.api"
@@ -15,6 +16,9 @@ dependencies {
     api("org.jetbrains:annotations:24.0.1")
 
     implementation("org.jgrapht:jgrapht-core:1.5.2")
+
+    testImplementation("org.junit.jupiter:junit-jupiter-api:5.10.0")
+    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.10.0")
 }
 
 java {
@@ -26,6 +30,15 @@ java {
     withJavadocJar()
 }
 
+tasks {
+    test {
+        useJUnitPlatform()
+    }
+    jacocoTestReport {
+        dependsOn(test)
+    }
+}
+
 publishing {
     repositories {
         maven {

src/main/java/dev/emortal/api/modules/Module.java

@@ -12,7 +12,15 @@ protected Module(@NotNull ModuleEnvironment environment) {
         this.environment = environment;
     }
 
-    protected <T extends Module> @Nullable T getModule(@NotNull Class<T> type) {
+    protected <T extends Module> @NotNull T getModule(@NotNull Class<T> type) {
+        T module = this.environment.moduleProvider().getModule(type);
+        if (module == null) {
+            throw new IllegalStateException("Required module was not available! Module should not have loaded if it was not.");
+        }
+        return module;
+    }
+
+    protected <T extends Module> @Nullable T getOptionalModule(@NotNull Class<T> type) {
         return this.environment.moduleProvider().getModule(type);
     }
 

src/main/java/dev/emortal/api/modules/ModuleManager.java

@@ -2,59 +2,68 @@
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Collectors;
 
+import dev.emortal.api.modules.annotation.Dependency;
+import dev.emortal.api.modules.annotation.ModuleData;
 import dev.emortal.api.modules.env.BasicModuleEnvironment;
 import dev.emortal.api.modules.env.ModuleEnvironment;
+import dev.emortal.api.modules.extension.ModuleCandidate;
+import dev.emortal.api.modules.extension.ModuleCandidateResolver;
+import dev.emortal.api.modules.extension.ModuleEnvironmentProvider;
+import dev.emortal.api.modules.extension.ModuleSorter;
+import dev.emortal.api.modules.internal.DefaultModuleCandidateResolver;
+import dev.emortal.api.modules.internal.DefaultModuleSorter;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.jgrapht.Graph;
-import org.jgrapht.graph.DefaultEdge;
-import org.jgrapht.graph.DirectedAcyclicGraph;
-import org.jgrapht.traverse.TopologicalOrderIterator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public final class ModuleManager implements ModuleProvider {
     private static final Logger LOGGER = LoggerFactory.getLogger(ModuleManager.class);
 
-    private final @NotNull ModuleEnvironment.Provider moduleEnvironmentProvider;
+    public static @NotNull Builder builder() {
+        return new Builder();
+    }
 
-    private final Map<Class<? extends Module>, Module> modules = new ConcurrentHashMap<>();
+    private final @NotNull ModuleCandidateResolver candidateResolver;
+    private final @NotNull ModuleSorter sorter;
+    private final @NotNull ModuleEnvironmentProvider environmentProvider;
 
-    public ModuleManager(@NotNull List<LoadableModule> modules, @NotNull ModuleEnvironment.Provider moduleEnvironmentProvider) {
-        this.moduleEnvironmentProvider = moduleEnvironmentProvider;
-        this.loadModules(modules);
-    }
+    private final Map<Class<? extends Module>, Module> modules = new ConcurrentHashMap<>();
 
-    public ModuleManager(@NotNull List<LoadableModule> modules) {
-        this(modules, BasicModuleEnvironment::new);
+    public ModuleManager(@NotNull ModuleCandidateResolver candidateResolver, @NotNull ModuleSorter sorter,
+                          @NotNull ModuleEnvironmentProvider environmentProvider) {
+        this.candidateResolver = candidateResolver;
+        this.sorter = sorter;
+        this.environmentProvider = environmentProvider;
     }
 
-    private void loadModules(@NotNull List<LoadableModule> modules) {
+    public void loadModules(@NotNull Collection<LoadableModule> modules) {
         if (modules.isEmpty()) {
             LOGGER.warn("No modules provided to ModuleManager to be loaded");
             return;
         }
 
-        List<LoadableModule> sortedModules = this.sortModules(modules);
+        List<ModuleCandidate> loadedModules = this.candidateResolver.resolveCandidates(modules);
+        List<ModuleCandidate> sortedModules = this.sorter.sortModules(loadedModules);
+        Set<String> loadedModuleNames = new HashSet<>();
 
-        for (LoadableModule loadable : sortedModules) {
-            ModuleData data = loadable.clazz().getDeclaredAnnotation(ModuleData.class);
-            if (data == null) {
-                LOGGER.error("Module class {} does not have a ModuleData annotation! Skipping...", loadable.clazz().getSimpleName());
-                continue;
-            }
+        for (ModuleCandidate candidate : sortedModules) {
+            if (!this.checkDependencies(candidate, loadedModuleNames)) continue;
+
+            ModuleData data = candidate.data();
+            ModuleEnvironment environment = this.environmentProvider.create(candidate.data(), this);
 
-            ModuleEnvironment environment = this.moduleEnvironmentProvider.create(data, this);
             Module module;
             try {
-                module = loadable.creator().create(environment);
+                module = candidate.creator().create(environment);
             } catch (Exception exception) {
                 LOGGER.error("Failed to create module {}", data.name(), exception);
                 continue;
@@ -68,15 +77,27 @@ private void loadModules(@NotNull List<LoadableModule> modules) {
                 LOGGER.error("Failed to load module {}", data.name(), exception);
                 continue;
             }
+
             Duration loadDuration = Duration.between(loadStart, Instant.now());
+            if (!loadResult) continue; // Failed to load
 
-            if (loadResult) {
-                this.modules.put(loadable.clazz(), module);
-                LOGGER.info("Loaded module {} in {}ms (required: {})", data.name(), loadDuration.toMillis(), data.required());
-            }
+            loadedModuleNames.add(data.name());
+            this.modules.put(candidate.clazz(), module);
+            LOGGER.info("Loaded module {} in {}ms", data.name(), loadDuration.toMillis());
         }
     }
 
+    private boolean checkDependencies(@NotNull ModuleCandidate candidate, @NotNull Set<String> loadedModuleNames) {
+        for (Dependency dependency : candidate.data().dependencies()) {
+            if (!dependency.required()) continue; // Only fail load for required dependencies
+            if (loadedModuleNames.contains(dependency.name())) continue; // Dependency is loaded
+
+            LOGGER.error("Failed to load module {} due to missing dependency {}", candidate.data().name(), dependency.name());
+            return false;
+        }
+        return true;
+    }
+
     @Override
     public <T extends Module> @Nullable T getModule(@NotNull Class<T> type) {
         return type.cast(this.modules.get(type));
@@ -86,8 +107,8 @@ public void onReady() {
         for (Module module : this.modules.values()) {
             Instant readyStart = Instant.now();
             module.onReady();
-            Duration readyDuration = Duration.between(readyStart, Instant.now());
 
+            Duration readyDuration = Duration.between(readyStart, Instant.now());
             LOGGER.info("Fired onReady for module {} in {}ms", module.getClass().getSimpleName(), readyDuration.toMillis());
         }
     }
@@ -96,40 +117,51 @@ public void onUnload() {
         for (Module module : this.modules.values()) {
             Instant unloadStart = Instant.now();
             module.onUnload();
-            Duration unloadDuration = Duration.between(unloadStart, Instant.now());
 
+            Duration unloadDuration = Duration.between(unloadStart, Instant.now());
             LOGGER.info("Unloaded module {} in {}ms", module.getClass().getSimpleName(), unloadDuration.toMillis());
         }
     }
 
-    private @NotNull List<LoadableModule> sortModules(@NotNull Collection<LoadableModule> modules) {
-        Graph<LoadableModule, DefaultEdge> graph = new DirectedAcyclicGraph<>(DefaultEdge.class);
-
-        for (LoadableModule module : modules) {
-            graph.addVertex(module);
-
-            ModuleData data = module.clazz().getDeclaredAnnotation(ModuleData.class);
-            for (Class<? extends Module> dependency : data.softDependencies()) {
-                // find the LoadableModule for the dependency's Class
-                LoadableModule dependencyModule = modules.stream()
-                        .filter(targetModule -> targetModule.clazz().equals(dependency))
-                        .findFirst()
-                        .orElse(null);
-
-                if (dependencyModule == null) {
-                    LOGGER.error("Module {} requires module {} to be loaded first.", module.clazz().getSimpleName(), dependency.getSimpleName());
-                    continue;
-                }
-                graph.addVertex(dependencyModule);
-                graph.addEdge(dependencyModule, module);
-            }
+    public static final class Builder {
+
+        private @Nullable ModuleCandidateResolver candidateResolver;
+        private @Nullable ModuleSorter sorter;
+        private @Nullable ModuleEnvironmentProvider environmentProvider;
+
+        private final Map<Class<? extends Module>, LoadableModule> modules = new HashMap<>();
+
+        private Builder() {
         }
 
-        TopologicalOrderIterator<LoadableModule, DefaultEdge> sortedIterator = new TopologicalOrderIterator<>(graph);
-        List<LoadableModule> sorted = new ArrayList<>();
-        sortedIterator.forEachRemaining(sorted::add);
+        public @NotNull Builder candidateResolver(@NotNull ModuleCandidateResolver candidateResolver) {
+            this.candidateResolver = candidateResolver;
+            return this;
+        }
 
-        LOGGER.info("Loading modules: [{}]", sorted.stream().map(module -> module.clazz().getSimpleName()).collect(Collectors.joining(", ")));
-        return sorted;
+        public @NotNull Builder sorter(@NotNull ModuleSorter sorter) {
+            this.sorter = sorter;
+            return this;
+        }
+
+        public @NotNull Builder environmentProvider(@NotNull ModuleEnvironmentProvider environmentProvider) {
+            this.environmentProvider = environmentProvider;
+            return this;
+        }
+
+        public @NotNull Builder module(@NotNull Class<? extends Module> type, @NotNull LoadableModule.Creator creator) {
+            this.modules.put(type, new LoadableModule(type, creator));
+            return this;
+        }
+
+        public @NotNull ModuleManager build() {
+            if (this.candidateResolver == null) this.candidateResolver = new DefaultModuleCandidateResolver();
+            if (this.sorter == null) this.sorter = new DefaultModuleSorter();
+            if (this.environmentProvider == null) this.environmentProvider = BasicModuleEnvironment::new;
+
+            ModuleManager manager = new ModuleManager(this.candidateResolver, this.sorter, this.environmentProvider);
+            manager.loadModules(this.modules.values());
+            return manager;
+        }
     }
 }

src/main/java/dev/emortal/api/modules/annotation/Dependency.java

@@ -0,0 +1,10 @@
+package dev.emortal.api.modules.annotation;
+
+import org.jetbrains.annotations.NotNull;
+
+public @interface Dependency {
+
+    @NotNull String name();
+
+    boolean required() default true;
+}

src/main/java/dev/emortal/api/modules/ModuleData.java → src/main/java/dev/emortal/api/modules/annotation/ModuleData.java

@@ -1,4 +1,4 @@
-package dev.emortal.api.modules;
+package dev.emortal.api.modules.annotation;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -9,7 +9,5 @@
 
     @NotNull String name();
 
-    boolean required();
-
-    @NotNull Class<? extends Module>@NotNull[] softDependencies() default {};
+    @NotNull Dependency[] dependencies() default {};
 }

src/main/java/dev/emortal/api/modules/env/BasicModuleEnvironment.java

@@ -1,6 +1,6 @@
 package dev.emortal.api.modules.env;
 
-import dev.emortal.api.modules.ModuleData;
+import dev.emortal.api.modules.annotation.ModuleData;
 import dev.emortal.api.modules.ModuleProvider;
 import org.jetbrains.annotations.NotNull;
 

src/main/java/dev/emortal/api/modules/env/ModuleEnvironment.java

@@ -1,6 +1,6 @@
 package dev.emortal.api.modules.env;
 
-import dev.emortal.api.modules.ModuleData;
+import dev.emortal.api.modules.annotation.ModuleData;
 import dev.emortal.api.modules.ModuleProvider;
 import org.jetbrains.annotations.NotNull;
 
@@ -18,10 +18,4 @@ public interface ModuleEnvironment {
      * Provides access to existing modules to facilitate module dependencies.
      */
     @NotNull ModuleProvider moduleProvider();
-
-    @FunctionalInterface
-    interface Provider {
-
-        @NotNull ModuleEnvironment create(@NotNull ModuleData data, @NotNull ModuleProvider moduleProvider);
-    }
 }

src/main/java/dev/emortal/api/modules/extension/ModuleCandidate.java

@@ -0,0 +1,9 @@
+package dev.emortal.api.modules.extension;
+
+import dev.emortal.api.modules.LoadableModule;
+import dev.emortal.api.modules.Module;
+import dev.emortal.api.modules.annotation.ModuleData;
+import org.jetbrains.annotations.NotNull;
+
+public record ModuleCandidate(@NotNull Class<? extends Module> clazz, @NotNull LoadableModule.Creator creator, @NotNull ModuleData data) {
+}

src/main/java/dev/emortal/api/modules/extension/ModuleCandidateResolver.java

@@ -0,0 +1,12 @@
+package dev.emortal.api.modules.extension;
+
+import dev.emortal.api.modules.LoadableModule;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collection;
+import java.util.List;
+
+public interface ModuleCandidateResolver {
+
+    @NotNull List<ModuleCandidate> resolveCandidates(@NotNull Collection<LoadableModule> modules);
+}

src/main/java/dev/emortal/api/modules/extension/ModuleEnvironmentProvider.java

@@ -0,0 +1,11 @@
+package dev.emortal.api.modules.extension;
+
+import dev.emortal.api.modules.ModuleProvider;
+import dev.emortal.api.modules.annotation.ModuleData;
+import dev.emortal.api.modules.env.ModuleEnvironment;
+import org.jetbrains.annotations.NotNull;
+
+public interface ModuleEnvironmentProvider {
+
+    @NotNull ModuleEnvironment create(@NotNull ModuleData data, @NotNull ModuleProvider provider);
+}

src/main/java/dev/emortal/api/modules/extension/ModuleSorter.java

@@ -0,0 +1,11 @@
+package dev.emortal.api.modules.extension;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collection;
+import java.util.List;
+
+public interface ModuleSorter {
+
+    @NotNull List<ModuleCandidate> sortModules(@NotNull Collection<ModuleCandidate> modules);
+}

src/main/java/dev/emortal/api/modules/internal/DefaultModuleCandidateResolver.java

@@ -0,0 +1,34 @@
+package dev.emortal.api.modules.internal;
+
+import dev.emortal.api.modules.LoadableModule;
+import dev.emortal.api.modules.annotation.ModuleData;
+import dev.emortal.api.modules.extension.ModuleCandidate;
+import dev.emortal.api.modules.extension.ModuleCandidateResolver;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+public final class DefaultModuleCandidateResolver implements ModuleCandidateResolver {
+    private static final Logger LOGGER = LoggerFactory.getLogger(DefaultModuleCandidateResolver.class);
+
+    @Override
+    public @NotNull List<ModuleCandidate> resolveCandidates(@NotNull Collection<LoadableModule> modules) {
+        List<ModuleCandidate> result = new ArrayList<>();
+
+        for (LoadableModule module : modules) {
+            ModuleData data = module.clazz().getDeclaredAnnotation(ModuleData.class);
+            if (data == null) {
+                LOGGER.error("ModuleData annotation not found on module class {}", module.clazz().getSimpleName());
+                continue;
+            }
+
+            result.add(new ModuleCandidate(module.clazz(), module.creator(), data));
+        }
+
+        return result;
+    }
+}