Refactor dependency injection for Neo4jTransactionalContext{Facory} i…

…nto separate Dependencies class

community/cypher/cypher/src/test/scala/org/neo4j/cypher/internal/spi/v3_1/TransactionBoundQueryContextTest.scala

@@ -21,7 +21,6 @@ package org.neo4j.cypher.internal.spi.v3_1
 
 import java.net.URL
 import java.util.Collections
-import java.util.function.Supplier
 
 import org.mockito.Mockito._
 import org.neo4j.cypher.internal.compiler.v3_1.helpers.DynamicIterable
@@ -33,13 +32,16 @@ import org.neo4j.cypher.javacompat.internal.GraphDatabaseCypherService
 import org.neo4j.graphdb._
 import org.neo4j.graphdb.config.Setting
 import org.neo4j.graphdb.factory.GraphDatabaseSettings
+import org.neo4j.kernel.GraphDatabaseQueryService
 import org.neo4j.kernel.api._
+import org.neo4j.kernel.api.security.AnonymousContext
 import org.neo4j.kernel.api.security.SecurityContext.AUTH_DISABLED
-import org.neo4j.kernel.api.security.{AnonymousContext, SecurityContext}
 import org.neo4j.kernel.impl.api.{KernelStatement, KernelTransactionImplementation, StatementOperationParts}
+import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge
 import org.neo4j.kernel.impl.coreapi.{InternalTransaction, PropertyContainerLocker}
 import org.neo4j.kernel.impl.factory.CanWrite
 import org.neo4j.kernel.impl.proc.Procedures
+import org.neo4j.kernel.impl.query.Neo4jTransactionalContext.Dependencies
 import org.neo4j.kernel.impl.query.{Neo4jTransactionalContext, Neo4jTransactionalContextFactory, QuerySource}
 import org.neo4j.storageengine.api.StorageStatement
 import org.neo4j.test.TestGraphDatabaseFactory
@@ -71,18 +73,30 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
     graph.getGraphDatabaseService.shutdown()
   }
 
+  def dependencies(graph: GraphDatabaseCypherService, statement: Statement, locker: PropertyContainerLocker): Dependencies =
+    new Dependencies {
+      override def txBridge(): ThreadToStatementContextBridge = ???
+      override def locker(): PropertyContainerLocker = locker
+      override def currentStatement(): Statement = currentStatement
+      override def queryService(): GraphDatabaseQueryService = graph
+      override def check(statement: KernelStatement): Unit = ???
+    }
+
   test("should mark transaction successful if successful") {
     // GIVEN
     when(outerTx.failure()).thenThrow(new AssertionError("Shouldn't be called"))
-    val tc = new Neo4jTransactionalContext(
-      graph, outerTx, KernelTransaction.Type.`implicit`, AUTH_DISABLED, supply(statement), null, null, locker
-    )
+    val deps = dependencies(graph, statement, locker)
+    when(outerTx.transactionType()).thenReturn(KernelTransaction.Type.`implicit`)
+    when(outerTx.securityContext()).thenReturn(AUTH_DISABLED)
+    val tc = new Neo4jTransactionalContext(deps, outerTx, statement, null)
     val transactionalContext = TransactionalContextWrapperv3_1(tc)
     val context = new TransactionBoundQueryContext(transactionalContext)(indexSearchMonitor)
     // WHEN
     context.transactionalContext.close(success = true)
 
     // THEN
+    verify(outerTx).transactionType()
+    verify(outerTx).securityContext()
     verify(outerTx).success()
     verify(outerTx).close()
     verifyNoMoreInteractions(outerTx)
@@ -91,15 +105,18 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
   test("should mark transaction failed if not successful") {
     // GIVEN
     when(outerTx.success()).thenThrow(new AssertionError("Shouldn't be called"))
-    val tc = new Neo4jTransactionalContext(
-      graph, outerTx, KernelTransaction.Type.`implicit`, AUTH_DISABLED, supply(statement), null, null, locker
-    )
+    when(outerTx.transactionType()).thenReturn(KernelTransaction.Type.`implicit`)
+    when(outerTx.securityContext()).thenReturn(AUTH_DISABLED)
+    val deps = dependencies(graph, statement, locker)
+    val tc = new Neo4jTransactionalContext(deps, outerTx, statement, null)
     val transactionalContext = TransactionalContextWrapperv3_1(tc)
     val context = new TransactionBoundQueryContext(transactionalContext)(indexSearchMonitor)
     // WHEN
     context.transactionalContext.close(success = false)
 
     // THEN
+    verify(outerTx).transactionType()
+    verify(outerTx).securityContext()
     verify(outerTx).failure()
     verify(outerTx).close()
     verifyNoMoreInteractions(outerTx)
@@ -182,8 +199,4 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
       tx.close()
     }
   }
-
-  private def supply[T](f: => T): Supplier[T] = new Supplier[T] {
-    override def get(): T = f
-  }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/factory/DataSourceModule.java

@@ -119,6 +119,8 @@ public class DataSourceModule
 
     public final AutoIndexing autoIndexing;
 
+    public final Guard guard;
+
     public DataSourceModule( final PlatformModule platformModule, EditionModule editionModule,
             Supplier<QueryExecutionEngine> queryExecutionEngineSupplier )
     {
@@ -159,7 +161,7 @@ public DataSourceModule( final PlatformModule platformModule, EditionModule edit
         SchemaWriteGuard schemaWriteGuard = deps.satisfyDependency( editionModule.schemaWriteGuard );
 
         Clock clock = getClock();
-        Guard guard = createGuard( deps, clock, logging );
+        guard = createGuard( deps, clock, logging );
 
         kernelEventHandlers = new KernelEventHandlers( logging.getInternalLog( KernelEventHandlers.class ) );
 

community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java

@@ -24,6 +24,7 @@
 import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
 
 import org.neo4j.collection.primitive.PrimitiveLongCollections;
 import org.neo4j.collection.primitive.PrimitiveLongIterator;
@@ -69,6 +70,7 @@
 import org.neo4j.kernel.api.security.SecurityContext;
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.kernel.guard.Guard;
+import org.neo4j.kernel.impl.api.KernelStatement;
 import org.neo4j.kernel.impl.api.TokenAccess;
 import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing;
 import org.neo4j.kernel.impl.api.operations.KeyReadOperations;
@@ -88,6 +90,7 @@
 import org.neo4j.kernel.impl.coreapi.StandardRelationshipActions;
 import org.neo4j.kernel.impl.coreapi.TopLevelTransaction;
 import org.neo4j.kernel.impl.coreapi.schema.SchemaImpl;
+import org.neo4j.kernel.impl.query.Neo4jTransactionalContext;
 import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory;
 import org.neo4j.kernel.impl.query.QueryEngineProvider;
 import org.neo4j.kernel.impl.query.TransactionalContext;
@@ -100,6 +103,7 @@
 
 import static java.lang.String.format;
 import static org.neo4j.collection.primitive.PrimitiveLongCollections.map;
+import static org.neo4j.function.Suppliers.*;
 import static org.neo4j.helpers.collection.Iterators.emptyIterator;
 import static org.neo4j.kernel.api.security.SecurityContext.AUTH_DISABLED;
 import static org.neo4j.kernel.impl.api.operations.KeyReadOperations.NO_SUCH_LABEL;
@@ -197,11 +201,29 @@ public GraphDatabaseFacade()
 
     /**
      * Create a new Core API facade, backed by the given SPI.
+     *
+     * Any required dependencies are resolved using the resolver obtained from the SPI.
      */
-    public void init( SPI spi, ThreadToStatementContextBridge txBridge, Config config )
+    public final void init( SPI spi, Config config )
+    {
+        DependencyResolver resolver = spi.resolver();
+        init(
+            spi,
+            resolver.resolveDependency( Guard.class ),
+            resolver.resolveDependency( ThreadToStatementContextBridge.class ),
+            config
+        );
+    }
+
+    /**
+     * Create a new Core API facade, backed by the given SPI and using pre-resolved dependencies
+     */
+    public void init( SPI spi, Guard guard, ThreadToStatementContextBridge txBridge, Config config )
     {
         this.spi = spi;
         this.defaultTransactionTimeout = config.get( GraphDatabaseSettings.transaction_timeout );
+
+        // TODO: Initialize these fields lazily, esp for Procedures this makes sense
         this.schema = new SchemaImpl( spi::currentStatement );
 
         this.relActions = new StandardRelationshipActions( spi::currentStatement, spi::currentTransaction,
@@ -220,9 +242,41 @@ public void init( SPI spi, ThreadToStatementContextBridge txBridge, Config confi
                 spi.autoIndexing().relationships() );
         this.indexManager = new IndexManagerImpl( spi::currentStatement, idxProvider, nodeAutoIndexer, relAutoIndexer );
 
-        this.contextFactory = new Neo4jTransactionalContextFactory(
-            spi::queryService, spi::currentStatement, txBridge, locker
-        );
+        this.contextFactory = new Neo4jTransactionalContextFactory( new Neo4jTransactionalContext.Dependencies()
+        {
+            // We cache this since existing SPIs implement this via dependency resolution at runtime
+            private final Supplier<GraphDatabaseQueryService> queryService = lazySingleton( spi::queryService );
+
+            @Override
+            public GraphDatabaseQueryService queryService()
+            {
+                return queryService.get();
+            }
+
+            @Override
+            public Statement currentStatement()
+            {
+                return spi.currentStatement();
+            }
+
+            @Override
+            public void check( KernelStatement statement )
+            {
+                guard.check( statement );
+            }
+
+            @Override
+            public ThreadToStatementContextBridge txBridge()
+            {
+                return txBridge;
+            }
+
+            @Override
+            public PropertyContainerLocker locker()
+            {
+                return locker;
+            }
+        } );
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacadeFactory.java

@@ -149,6 +149,7 @@ public GraphDatabaseFacade initFacade( File storeDir, Map<String,String> params,
         ClassicCoreSPI spi = new ClassicCoreSPI( platform, dataSource, msgLog, coreAPIAvailabilityGuard );
         graphDatabaseFacade.init(
             spi,
+            dataSource.guard,
             dataSource.threadToTransactionBridge,
             platform.config
         );

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ProcedureGDSFactory.java

@@ -33,6 +33,7 @@
 import org.neo4j.kernel.api.legacyindex.AutoIndexing;
 import org.neo4j.kernel.api.proc.Context;
 import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.guard.Guard;
 import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
 import org.neo4j.kernel.impl.coreapi.CoreAPIAvailabilityGuard;
 import org.neo4j.kernel.impl.factory.GraphDatabaseFacade;
@@ -48,6 +49,7 @@ public class ProcedureGDSFactory implements ThrowingFunction<Context,GraphDataba
     private final Supplier<QueryExecutionEngine> queryExecutor;
     private final CoreAPIAvailabilityGuard availability;
     private final ThrowingFunction<URL, URL, URLAccessValidationError> urlValidator;
+    private final Guard guard;
     private final ThreadToStatementContextBridge txBridge;
 
     public ProcedureGDSFactory( Config config,
@@ -65,6 +67,7 @@ public ProcedureGDSFactory( Config config,
         this.queryExecutor = queryExecutor;
         this.availability = availability;
         this.urlValidator = url -> urlAccessRule.validate( config, url );
+        this.guard = resolver.resolveDependency( Guard.class );
         this.txBridge = resolver.resolveDependency( ThreadToStatementContextBridge.class );
     }
 
@@ -86,6 +89,7 @@ public GraphDatabaseService apply( Context context ) throws ProcedureException
                 availability,
                 urlValidator
             ),
+            guard,
             txBridge,
             config
         );

community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java

@@ -27,49 +27,36 @@
 import org.neo4j.kernel.api.dbms.DbmsOperations;
 import org.neo4j.kernel.api.security.SecurityContext;
 import org.neo4j.kernel.api.txstate.TxStateHolder;
-import org.neo4j.kernel.guard.Guard;
 import org.neo4j.kernel.impl.api.KernelStatement;
 import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
 import org.neo4j.kernel.impl.coreapi.InternalTransaction;
 import org.neo4j.kernel.impl.coreapi.PropertyContainerLocker;
 
-import java.util.function.Supplier;
-
 public class Neo4jTransactionalContext implements TransactionalContext
 {
     private final GraphDatabaseQueryService graph;
-    private final ThreadToStatementContextBridge txBridge;
     private final KernelTransaction.Type transactionType;
     private final SecurityContext securityContext;
-    private final Supplier<Statement> statementSupplier;
     private final ExecutingQuery executingQuery;
-    private final PropertyContainerLocker locker;
-    private final Guard guard;
+    private final Dependencies dependencies;
 
     private InternalTransaction transaction;
     private Statement statement;
     private boolean isOpen = true;
 
     public Neo4jTransactionalContext(
-            GraphDatabaseQueryService graph,
+            Dependencies dependencies,
             InternalTransaction initialTransaction,
-            KernelTransaction.Type transactionType,
-            SecurityContext securityContext,
-            Supplier<Statement> statementSupplier,
-            ExecutingQuery executingQuery,
-            ThreadToStatementContextBridge txBridge,
-            PropertyContainerLocker locker
+            Statement initialStatement,
+            ExecutingQuery executingQuery
     ) {
-        this.graph = graph;
+        this.graph = dependencies.queryService();
+        this.dependencies = dependencies;
         this.transaction = initialTransaction;
-        this.transactionType = transactionType;
-        this.securityContext = securityContext;
-        this.statementSupplier = statementSupplier;
-        this.statement = statementSupplier.get();
+        this.transactionType = initialTransaction.transactionType();
+        this.securityContext = initialTransaction.securityContext();
+        this.statement = initialStatement;
         this.executingQuery = executingQuery;
-        this.guard = graph.getGuard();
-        this.txBridge = txBridge;
-        this.locker = locker;
     }
 
     @Override
@@ -99,7 +86,7 @@ public boolean isTopLevelTx()
     @Override
     public void check()
     {
-        guard.check( (KernelStatement) statement );
+        dependencies.check( (KernelStatement) statement );
     }
 
     public void close( boolean success )
@@ -151,6 +138,8 @@ public void terminate()
     @Override
     public void commitAndRestartTx()
     {
+        ThreadToStatementContextBridge txBridge = dependencies.txBridge();
+
        /*
         * This method is use by the Cypher runtime to cater for PERIODIC COMMIT, which allows a single query to
         * periodically, after x number of rows, to commit a transaction and spawn a new one.
@@ -206,7 +195,7 @@ public void cleanForReuse()
         // to either a schema data or a schema statement, so that the locks are "handed over".
         statement.queryRegistration().unregisterExecutingQuery( executingQuery );
         statement.close();
-        statement = txBridge.get();
+        statement = dependencies.txBridge().get();
         statement.queryRegistration().registerExecutingQuery( executingQuery );
     }
 
@@ -216,7 +205,7 @@ public TransactionalContext getOrBeginNewIfClosed()
         if ( !isOpen )
         {
             transaction = graph.beginTransaction( transactionType, securityContext );
-            statement = statementSupplier.get();
+            statement = dependencies.currentStatement();
             statement.queryRegistration().registerExecutingQuery( executingQuery );
             isOpen = true;
         }
@@ -250,7 +239,7 @@ public TxStateHolder stateView()
     @Override
     public Lock acquireWriteLock( PropertyContainer p )
     {
-        return locker.exclusiveLock( statement, p );
+        return dependencies.locker().exclusiveLock( statement, p );
     }
 
     @Override
@@ -264,4 +253,13 @@ public SecurityContext securityContext()
     {
         return securityContext;
     }
+
+    public interface Dependencies
+    {
+        GraphDatabaseQueryService queryService();
+        Statement currentStatement();
+        void check( KernelStatement statement );
+        ThreadToStatementContextBridge txBridge();
+        PropertyContainerLocker locker();
+    }
 }