[GitHub] [jena] kinow commented on a diff in pull request #1805: GH-1804: Configurable fuseki modules

["kinow (via GitHub)" <gi...@apache.org>]

kinow commented on code in PR #1805:
URL: https://github.com/apache/jena/pull/1805#discussion_r1141028009


##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/sys/FusekiModules.java:
##########
@@ -18,67 +18,56 @@
 
 package org.apache.jena.fuseki.main.sys;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
 
-import org.apache.jena.base.module.Subsystem;
-
-/** Registry of modules */
+/**
+ * List of {@linkplain FusekiModule Fuseki modules}.
+ * This is the immutable unit of a modules for a server.
+ * <p>
+ * @see FusekiModulesLoaded
+ */
 public class FusekiModules {
 
-    private static Object lock = new Object();
+    /** A Fuseki module with no members. */
+    public static FusekiModules empty = FusekiModules.create();

Review Comment:
   Make it `final` maybe? Doesn't seem to be changed in the class methods.



##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java:
##########
@@ -1037,6 +1051,26 @@ public Builder addFilter(String pathSpec, Filter filter) {
             return this;
         }
 
+        /**
+         * Set the {@link FusekiModule Fuseki Module} for a server.
+         * If no modules are added to a builder, then the system-wide default set (found by loading FusekiModule
+         * via Java's {@link ServiceLoader} mechanism, is used.

Review Comment:
   missing closing `)`?



##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/sys/FusekiModules.java:
##########
@@ -18,67 +18,56 @@
 
 package org.apache.jena.fuseki.main.sys;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
 
-import org.apache.jena.base.module.Subsystem;
-
-/** Registry of modules */
+/**
+ * List of {@linkplain FusekiModule Fuseki modules}.
+ * This is the immutable unit of a modules for a server.
+ * <p>
+ * @see FusekiModulesLoaded
+ */
 public class FusekiModules {
 
-    private static Object lock = new Object();
+    /** A Fuseki module with no members. */
+    public static FusekiModules empty = FusekiModules.create();
 
-    // Record of what is loaded.
-    private static List<FusekiModule> registry = null;
-
-    private static Subsystem<FusekiModule> subsystem = null;
+    /** Create a collection of Fuseki modules */
+    public static FusekiModules create(FusekiModule ... modules) {
+        return new FusekiModules(modules);
+    }
 
-    public static void load() {
-        if ( registry == null )
-            reload();
+    /** Create a collection of Fuseki modules */
+    public static FusekiModules create(List<FusekiModule> modules) {
+        return new FusekiModules(modules);
     }
 
-    public static void reload() {
-        registry = new ArrayList<>();
-        subsystem = new Subsystem<FusekiModule>(FusekiModule.class);
-        subsystem.initialize();
-        synchronized(lock) {
-            subsystem.forEach(registry::add);
-        }
+    private final List<FusekiModule> modules;
+
+    private FusekiModules(FusekiModule ... modules) {
+        this.modules = List.of(modules);

Review Comment:
   NPE here if `modules` is `null` (coming from the public `#create` method).



##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/sys/FusekiModules.java:
##########
@@ -18,67 +18,56 @@
 
 package org.apache.jena.fuseki.main.sys;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
 
-import org.apache.jena.base.module.Subsystem;
-
-/** Registry of modules */
+/**
+ * List of {@linkplain FusekiModule Fuseki modules}.
+ * This is the immutable unit of a modules for a server.

Review Comment:
   s/of a modules/of modules?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org