Merge pull request #8916 from lassewesth/evermoarrsd

leverage the RequestScopedDependencies parameter object make the builder not have defaults, because trying to reconcile that across all usages might just be convenient, but is also an error-prone form of coupling. Best to set a good example now.

algo/src/main/java/org/neo4j/gds/algorithms/RequestScopedDependencies.java

@@ -29,8 +29,8 @@
 import org.neo4j.gds.termination.TerminationFlag;
 
 /**
- * This is a handy class for transporting similar dependencies through layers.
- * And especially useful when that list grows or shrinks - less sites to edit innit
+ * This is a handy class for transporting similarly scoped dependencies through layers.
+ * And especially useful when that list grows or shrinks - less sites to edit innit.
  */
 public final class RequestScopedDependencies {
     private final DatabaseId databaseId;
@@ -103,14 +103,19 @@ public UserLogRegistryFactory getUserLogRegistryFactory() {
         return userLogRegistryFactory;
     }
 
+    /**
+     * A handy builder where you can include as many or as few components as you are interested in.
+     * We deliberately do not have defaults,
+     * because trying to reconcile convenience across all usages is an error-prone form of coupling.
+     */
     public static class RequestScopedDependenciesBuilder {
-        private DatabaseId databaseId = DatabaseId.DEFAULT;
+        private DatabaseId databaseId;
         private NodePropertyExporterBuilder nodePropertyExporterBuilder;
         private RelationshipExporterBuilder relationshipExporterBuilder;
         private RelationshipStreamExporterBuilder relationshipStreamExporterBuilder;
-        private TerminationFlag terminationFlag = TerminationFlag.DEFAULT;
+        private TerminationFlag terminationFlag;
         private TaskRegistryFactory taskRegistryFactory;
-        private User user = User.DEFAULT;
+        private User user;
         private UserLogRegistryFactory userLogRegistryFactory;
 
         public RequestScopedDependenciesBuilder with(DatabaseId databaseId) {

algo/src/main/java/org/neo4j/gds/algorithms/runner/AlgorithmRunner.java

@@ -32,8 +32,6 @@
 import org.neo4j.gds.core.GraphDimensions;
 import org.neo4j.gds.core.loading.GraphStoreCatalogService;
 import org.neo4j.gds.core.utils.ProgressTimer;
-import org.neo4j.gds.core.utils.progress.TaskRegistryFactory;
-import org.neo4j.gds.core.utils.warnings.UserLogRegistryFactory;
 import org.neo4j.gds.logging.Log;
 import org.neo4j.gds.metrics.algorithms.AlgorithmMetricsService;
 
@@ -46,8 +44,6 @@ public final class AlgorithmRunner {
     private final Log log;
     private final GraphStoreCatalogService graphStoreCatalogService;
     private final AlgorithmMemoryValidationService memoryUsageValidator;
-    private final TaskRegistryFactory taskRegistryFactory;
-    private final UserLogRegistryFactory userLogRegistryFactory;
     private final AlgorithmMetricsService algorithmMetricsService;
     private final RequestScopedDependencies requestScopedDependencies;
 
@@ -56,15 +52,11 @@ public AlgorithmRunner(
         GraphStoreCatalogService graphStoreCatalogService,
         AlgorithmMetricsService algorithmMetricsService,
         AlgorithmMemoryValidationService memoryUsageValidator,
-        RequestScopedDependencies requestScopedDependencies,
-        TaskRegistryFactory taskRegistryFactory,
-        UserLogRegistryFactory userLogRegistryFactory
+        RequestScopedDependencies requestScopedDependencies
     ) {
         this.log = log;
         this.graphStoreCatalogService = graphStoreCatalogService;
         this.memoryUsageValidator = memoryUsageValidator;
-        this.taskRegistryFactory = taskRegistryFactory;
-        this.userLogRegistryFactory = userLogRegistryFactory;
         this.algorithmMetricsService = algorithmMetricsService;
         this.requestScopedDependencies = requestScopedDependencies;
     }
@@ -125,8 +117,8 @@ public <A extends Algorithm<R>, R, C extends AlgoBaseConfig> AlgorithmComputatio
             graph,
             config,
             (org.neo4j.logging.Log) log.getNeo4jLog(),
-            taskRegistryFactory,
-            userLogRegistryFactory
+            requestScopedDependencies.getTaskRegistryFactory(),
+            requestScopedDependencies.getUserLogRegistryFactory()
         );
 
         // this really belongs in the factory build thing
@@ -140,7 +132,7 @@ public <A extends Algorithm<R>, R, C extends AlgoBaseConfig> AlgorithmComputatio
 
     <R> R runAlgorithm(Algorithm<R> algorithm, String algorithmName) {
         var algorithmMetric = algorithmMetricsService.create(algorithmName);
-        try(algorithmMetric) {
+        try (algorithmMetric) {
             algorithmMetric.start();
             return algorithm.compute();
         } catch (Exception e) {

algo/src/main/java/org/neo4j/gds/algorithms/similarity/WriteRelationshipService.java

@@ -19,35 +19,24 @@
  */
 package org.neo4j.gds.algorithms.similarity;
 
+import org.neo4j.gds.algorithms.RequestScopedDependencies;
 import org.neo4j.gds.api.Graph;
 import org.neo4j.gds.api.GraphStore;
 import org.neo4j.gds.api.IdMap;
 import org.neo4j.gds.api.RelationshipWithPropertyConsumer;
 import org.neo4j.gds.api.ResultStore;
 import org.neo4j.gds.config.ArrowConnectionInfo;
-import org.neo4j.gds.core.utils.progress.TaskRegistryFactory;
-import org.neo4j.gds.core.write.RelationshipExporterBuilder;
 import org.neo4j.gds.logging.Log;
-import org.neo4j.gds.termination.TerminationFlag;
 
 import java.util.Optional;
 
 public class WriteRelationshipService {
     private final Log log;
-    private final RelationshipExporterBuilder relationshipExporterBuilder;
-    private final TaskRegistryFactory taskRegistryFactory;
-    private final TerminationFlag terminationFlag;
+    private final RequestScopedDependencies requestScopedDependencies;
 
-    public WriteRelationshipService(
-        Log log,
-        RelationshipExporterBuilder relationshipExporterBuilder,
-        TaskRegistryFactory taskRegistryFactory,
-        TerminationFlag terminationFlag
-    ) {
+    public WriteRelationshipService(Log log, RequestScopedDependencies requestScopedDependencies) {
         this.log = log;
-        this.relationshipExporterBuilder = relationshipExporterBuilder;
-        this.taskRegistryFactory = taskRegistryFactory;
-        this.terminationFlag = terminationFlag;
+        this.requestScopedDependencies = requestScopedDependencies;
     }
 
     public WriteRelationshipResult write(
@@ -65,14 +54,14 @@ public WriteRelationshipResult write(
         return Neo4jDatabaseRelationshipWriter.writeRelationship(
             writeRelationshipType,
             writeProperty,
-            taskRegistryFactory,
-            relationshipExporterBuilder,
+            requestScopedDependencies.getTaskRegistryFactory(),
+            requestScopedDependencies.getRelationshipExporterBuilder(),
             graph,
             graphStore,
             rootIdMap,
             log,
             taskName,
-            terminationFlag,
+            requestScopedDependencies.getTerminationFlag(),
             concurrency,
             arrowConnectionInfo,
             resultStore,

algo/src/main/java/org/neo4j/gds/algorithms/writeservices/WriteNodePropertyService.java

@@ -19,35 +19,23 @@
  */
 package org.neo4j.gds.algorithms.writeservices;
 
+import org.neo4j.gds.algorithms.RequestScopedDependencies;
 import org.neo4j.gds.api.Graph;
 import org.neo4j.gds.api.GraphStore;
 import org.neo4j.gds.api.ResultStore;
 import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
 import org.neo4j.gds.config.ArrowConnectionInfo;
-import org.neo4j.gds.core.utils.progress.TaskRegistryFactory;
-import org.neo4j.gds.core.write.NodePropertyExporterBuilder;
 import org.neo4j.gds.logging.Log;
-import org.neo4j.gds.termination.TerminationFlag;
 
 import java.util.Optional;
 
 public class WriteNodePropertyService {
-
     private final Log log;
-    private  final NodePropertyExporterBuilder nodePropertyExporterBuilder;
-    private final TaskRegistryFactory taskRegistryFactory;
-    private final TerminationFlag terminationFlag;
+    private final RequestScopedDependencies requestScopedDependencies;
 
-    public WriteNodePropertyService(
-        Log log,
-        NodePropertyExporterBuilder nodePropertyExporterBuilder,
-        TaskRegistryFactory taskRegistryFactory,
-        TerminationFlag terminationFlag
-    ) {
-        this.nodePropertyExporterBuilder=nodePropertyExporterBuilder;
+    public WriteNodePropertyService(Log log, RequestScopedDependencies requestScopedDependencies) {
         this.log = log;
-        this.taskRegistryFactory = taskRegistryFactory;
-        this.terminationFlag = terminationFlag;
+        this.requestScopedDependencies = requestScopedDependencies;
     }
 
     public WriteNodePropertyResult write(
@@ -61,8 +49,8 @@ public WriteNodePropertyResult write(
         Optional<ResultStore> resultStore
     ) {
         return Neo4jDatabaseNodePropertyWriter.writeNodeProperty(
-            nodePropertyExporterBuilder,
-            taskRegistryFactory,
+            requestScopedDependencies.getNodePropertyExporterBuilder(),
+            requestScopedDependencies.getTaskRegistryFactory(),
             graph,
             graphStore,
             nodePropertyValues,
@@ -71,7 +59,7 @@ public WriteNodePropertyResult write(
             procedureName,
             arrowConnectionInfo,
             resultStore,
-            terminationFlag,
+            requestScopedDependencies.getTerminationFlag(),
             log
         );
     }

algo/src/test/java/org/neo4j/gds/algorithms/centrality/CentralityAlgorithmsWriteBusinessFacadeTest.java

@@ -22,6 +22,7 @@
 import org.assertj.core.data.Offset;
 import org.junit.jupiter.api.Test;
 import org.neo4j.gds.algorithms.AlgorithmComputationResult;
+import org.neo4j.gds.algorithms.RequestScopedDependencies;
 import org.neo4j.gds.algorithms.centrality.specificfields.DefaultCentralitySpecificFields;
 import org.neo4j.gds.algorithms.writeservices.WriteNodePropertyResult;
 import org.neo4j.gds.algorithms.writeservices.WriteNodePropertyService;
@@ -298,7 +299,7 @@ private static final class WriteNodePropertyServiceStub extends WriteNodePropert
         private final long writeMilliseconds;
 
         WriteNodePropertyServiceStub(long nodePropertiesWritten, long writeMilliseconds) {
-            super(null, null, null, null);
+            super(null, RequestScopedDependencies.builder().build());
             this.nodePropertiesWritten = nodePropertiesWritten;
             this.writeMilliseconds = writeMilliseconds;
         }

algo/src/test/java/org/neo4j/gds/algorithms/community/CommunityAlgorithmsStreamBusinessFacadeTest.java

@@ -39,6 +39,7 @@
 import org.neo4j.gds.logging.Log;
 import org.neo4j.gds.metrics.PassthroughExecutionMetricRegistrar;
 import org.neo4j.gds.metrics.algorithms.AlgorithmMetricsService;
+import org.neo4j.gds.termination.TerminationFlag;
 import org.neo4j.gds.wcc.WccBaseConfig;
 import org.neo4j.gds.wcc.WccParameters;
 
@@ -107,9 +108,11 @@ void wcc() {
                         graphStoreCatalogServiceMock,
                         new AlgorithmMetricsService(new PassthroughExecutionMetricRegistrar()),
                         mock(AlgorithmMemoryValidationService.class),
-                        RequestScopedDependencies.builder().build(),
-                        TaskRegistryFactory.empty(),
-                        EmptyUserLogRegistryFactory.INSTANCE
+                        RequestScopedDependencies.builder()
+                            .with(TaskRegistryFactory.empty())
+                            .with(TerminationFlag.DEFAULT)
+                            .with(EmptyUserLogRegistryFactory.INSTANCE)
+                            .build()
                     )
                 )
             );
@@ -146,9 +149,10 @@ void wccOnEmptyGraph() {
                         graphStoreCatalogServiceMock,
                         new AlgorithmMetricsService(new PassthroughExecutionMetricRegistrar()),
                         null,
-                        RequestScopedDependencies.builder().build(),
-                        mock(TaskRegistryFactory.class),
-                        mock(UserLogRegistryFactory.class)
+                        RequestScopedDependencies.builder()
+                            .with(mock(TaskRegistryFactory.class))
+                            .with(mock(UserLogRegistryFactory.class))
+                            .build()
                     )
                 )
             );

algo/src/test/java/org/neo4j/gds/algorithms/runner/AlgorithmRunnerTest.java

@@ -49,8 +49,6 @@ void shouldRegisterAlgorithmMetricCountForSuccess() {
             null,
             algorithmMetricsServiceMock,
             null,
-            null,
-            null,
             null
         );
 
@@ -85,8 +83,6 @@ void shouldRegisterAlgorithmMetricCountForFailure() {
             null,
             algorithmMetricsServiceMock,
             null,
-            null,
-            null,
             null
         );
 

algo/src/test/java/org/neo4j/gds/algorithms/similarity/NodeSimilarityAlgorithmsFacadeTest.java

@@ -39,6 +39,7 @@
 import org.neo4j.gds.metrics.algorithms.AlgorithmMetricsService;
 import org.neo4j.gds.similarity.SimilarityResult;
 import org.neo4j.gds.similarity.nodesim.NodeSimilarityStreamConfigImpl;
+import org.neo4j.gds.termination.TerminationFlag;
 
 import java.util.Collection;
 import java.util.HashSet;
@@ -122,9 +123,11 @@ void shouldCompute() {
                 graphStoreCatalogServiceMock,
                 new AlgorithmMetricsService(new PassthroughExecutionMetricRegistrar()),
                 mock(AlgorithmMemoryValidationService.class),
-                RequestScopedDependencies.builder().build(),
-                TaskRegistryFactory.empty(),
-                EmptyUserLogRegistryFactory.INSTANCE
+                RequestScopedDependencies.builder()
+                    .with(TaskRegistryFactory.empty())
+                    .with(TerminationFlag.DEFAULT)
+                    .with(EmptyUserLogRegistryFactory.INSTANCE)
+                    .build()
             )
         );
         var nodeSimilarity = similarityAlgorithmsFacade.nodeSimilarity(

algo/src/test/java/org/neo4j/gds/algorithms/someotherpackage/RequestScopedDependenciesTest.java

@@ -28,15 +28,6 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 class RequestScopedDependenciesTest {
-    @Test
-    void shouldBeSafeAndConvenient() {
-        var rsd = RequestScopedDependencies.builder().build();
-
-        assertThat(rsd.getDatabaseId()).isEqualTo(DatabaseId.DEFAULT);
-        assertThat(rsd.getUser()).isEqualTo(User.DEFAULT);
-        assertThat(rsd.getTerminationFlag()).isEqualTo(TerminationFlag.DEFAULT);
-    }
-
     @Test
     void shouldBuildBespokeProducts() {
         var rsd = RequestScopedDependencies.builder()

applications/algorithms/path-finding/src/main/java/org/neo4j/gds/applications/algorithms/pathfinding/BellmanFordWriteStep.java

@@ -19,21 +19,19 @@
  */
 package org.neo4j.gds.applications.algorithms.pathfinding;
 
+import org.neo4j.gds.algorithms.RequestScopedDependencies;
 import org.neo4j.gds.api.DatabaseId;
 import org.neo4j.gds.api.Graph;
 import org.neo4j.gds.api.GraphStore;
 import org.neo4j.gds.api.IdMap;
 import org.neo4j.gds.api.ImmutableExportedRelationship;
 import org.neo4j.gds.api.nodeproperties.ValueType;
-import org.neo4j.gds.core.utils.progress.TaskRegistryFactory;
 import org.neo4j.gds.core.utils.progress.tasks.TaskProgressTracker;
 import org.neo4j.gds.core.write.RelationshipStreamExporter;
-import org.neo4j.gds.core.write.RelationshipStreamExporterBuilder;
 import org.neo4j.gds.logging.Log;
 import org.neo4j.gds.paths.PathResult;
 import org.neo4j.gds.paths.bellmanford.BellmanFordResult;
 import org.neo4j.gds.paths.bellmanford.BellmanFordWriteConfig;
-import org.neo4j.gds.termination.TerminationFlag;
 import org.neo4j.values.storable.Value;
 import org.neo4j.values.storable.Values;
 
@@ -45,22 +43,16 @@
 
 class BellmanFordWriteStep implements MutateOrWriteStep<BellmanFordResult> {
     private final Log log;
-    private final RelationshipStreamExporterBuilder relationshipStreamExporterBuilder;
-    private final TaskRegistryFactory taskRegistryFactory;
-    private final TerminationFlag terminationFlag;
+    private final RequestScopedDependencies requestScopedDependencies;
     private final BellmanFordWriteConfig configuration;
 
     BellmanFordWriteStep(
         Log log,
-        RelationshipStreamExporterBuilder relationshipStreamExporterBuilder,
-        TaskRegistryFactory taskRegistryFactory,
-        TerminationFlag terminationFlag,
+        RequestScopedDependencies requestScopedDependencies,
         BellmanFordWriteConfig configuration
     ) {
         this.log = log;
-        this.relationshipStreamExporterBuilder = relationshipStreamExporterBuilder;
-        this.taskRegistryFactory = taskRegistryFactory;
-        this.terminationFlag = terminationFlag;
+        this.requestScopedDependencies = requestScopedDependencies;
         this.configuration = configuration;
     }
 
@@ -93,13 +85,13 @@ public void execute(
             RelationshipStreamExporter.baseTask("Write shortest Paths"),
             (org.neo4j.logging.Log) log.getNeo4jLog(),
             1,
-            taskRegistryFactory
+            requestScopedDependencies.getTaskRegistryFactory()
         );
 
-        var exporter = relationshipStreamExporterBuilder
+        var exporter = requestScopedDependencies.getRelationshipStreamExporterBuilder()
             .withIdMappingOperator(graph::toOriginalNodeId)
             .withRelationships(relationshipStream)
-            .withTerminationFlag(terminationFlag)
+            .withTerminationFlag(requestScopedDependencies.getTerminationFlag())
             .withProgressTracker(progressTracker)
             .withArrowConnectionInfo(
                 configuration.arrowConnectionInfo(),