Change so Neo4jTransactionalContext is kept around for the remainder …

…of a query execution

Before, `getOrBeginNewIfClosed()` would create a new Context if the old one was closed.
Now, it is repopulated with a new transaction instead.

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

@@ -21,6 +21,7 @@ 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
@@ -32,7 +33,6 @@ 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.graphdb.security.AuthorizationViolationException
 import org.neo4j.kernel.api._
 import org.neo4j.kernel.api.security.AccessMode
 import org.neo4j.kernel.impl.api.{KernelStatement, KernelTransactionImplementation, StatementOperationParts}
@@ -74,7 +74,7 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
     // GIVEN
     when(outerTx.failure()).thenThrow(new AssertionError("Shouldn't be called"))
     val tc = new Neo4jTransactionalContext(graph, outerTx, KernelTransaction.Type.`implicit`, AccessMode.Static.FULL,
-      statement, null, locker, null, null, null, null)
+      supply(statement), null, locker, null, null, null)
     val transactionalContext = TransactionalContextWrapperv3_1(tc)
     val context = new TransactionBoundQueryContext(transactionalContext)(indexSearchMonitor)
     // WHEN
@@ -90,7 +90,7 @@ class TransactionBoundQueryContextTest extends CypherFunSuite {
     // GIVEN
     when(outerTx.success()).thenThrow(new AssertionError("Shouldn't be called"))
     val tc = new Neo4jTransactionalContext(graph, outerTx, KernelTransaction.Type.`implicit`, AccessMode.Static.FULL,
-      statement, null, locker, null, null, null, null)
+      supply(statement), null, locker, null, null, null)
     val transactionalContext = TransactionalContextWrapperv3_1(tc)
     val context = new TransactionBoundQueryContext(transactionalContext)(indexSearchMonitor)
     // WHEN
@@ -179,4 +179,8 @@ 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/query/Neo4jTransactionalContext.java

@@ -19,6 +19,8 @@
  */
 package org.neo4j.kernel.impl.query;
 
+import java.util.function.Supplier;
+
 import org.neo4j.graphdb.Lock;
 import org.neo4j.graphdb.PropertyContainer;
 import org.neo4j.kernel.GraphDatabaseQueryService;
@@ -42,9 +44,9 @@ public class Neo4jTransactionalContext implements TransactionalContext
     private final ThreadToStatementContextBridge txBridge;
     private final KernelTransaction.Type transactionType;
     private final AccessMode mode;
+    private final Supplier<Statement> statementSupplier;
     private final DbmsOperations.Factory dbmsOperationsFactory;
     private final Guard guard;
-    private final TransactionalContextFactory factory;
 
     private InternalTransaction transaction;
     private Statement statement;
@@ -58,25 +60,24 @@ public Neo4jTransactionalContext(
             InternalTransaction initialTransaction,
             KernelTransaction.Type transactionType,
             AccessMode transactionMode,
-            Statement initialStatement,
+            Supplier<Statement> statementSupplier,
             ExecutingQuery executingQuery,
             PropertyContainerLocker locker,
             ThreadToStatementContextBridge txBridge,
             DbmsOperations.Factory dbmsOperationsFactory,
-            Guard guard,
-            TransactionalContextFactory factory
+            Guard guard
     ) {
         this.graph = graph;
         this.transaction = initialTransaction;
         this.transactionType = transactionType;
         this.mode = transactionMode;
-        this.statement = initialStatement;
+        this.statementSupplier = statementSupplier;
+        this.statement = statementSupplier.get();
         this.executingQuery = executingQuery;
         this.locker = locker;
         this.txBridge = txBridge;
         this.dbmsOperationsFactory = dbmsOperationsFactory;
         this.guard = guard;
-        this.factory = factory;
     }
 
     @Override
@@ -197,15 +198,14 @@ public void cleanForReuse()
     @Override
     public TransactionalContext getOrBeginNewIfClosed()
     {
-        if ( isOpen )
-        {
-            return this;
-        }
-        else
+        if ( !isOpen )
         {
-            InternalTransaction transaction = graph.beginTransaction( transactionType, mode );
-            return factory.newContext( this.executingQuery, transaction );
+            transaction = graph.beginTransaction( transactionType, mode );
+            statement = statementSupplier.get();
+            statement.queryRegistration().registerExecutingQuery( executingQuery );
+            isOpen = true;
         }
+        return this;
     }
 
     @Override

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

@@ -115,34 +115,12 @@ public Neo4jTransactionalContext newContext(
                 tx,
                 tx.transactionType(),
                 tx.mode(),
-                statement,
+                statementSupplier,
                 executingQuery,
                 locker,
                 txBridge,
                 dbmsOpsFactory,
-                guard,
-                this
+                guard
         );
     }
-
-    @Override
-    public Neo4jTransactionalContext newContext( ExecutingQuery query, InternalTransaction transaction )
-    {
-        Statement statement = statementSupplier.get();
-        GraphDatabaseQueryService queryService = queryServiceSupplier.get();
-        statement.queryRegistration().registerExecutingQuery( query );
-
-        return new Neo4jTransactionalContext(
-                queryService,
-                transaction,
-                transaction.transactionType(),
-                transaction.mode(),
-                statement,
-                query,
-                locker,
-                txBridge,
-                dbmsOpsFactory,
-                guard,
-                this );
-    }
 }

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

@@ -21,7 +21,6 @@
 
 import java.util.Map;
 
-import org.neo4j.kernel.api.ExecutingQuery;
 import org.neo4j.kernel.impl.coreapi.InternalTransaction;
 
 public interface TransactionalContextFactory
@@ -31,6 +30,4 @@ TransactionalContext newContext( QuerySource descriptor,
                   String queryText,
                   Map<String,Object> queryParameters
     );
-
-    TransactionalContext newContext( ExecutingQuery query, InternalTransaction tx );
 }

community/kernel/src/test/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContextTest.java

@@ -49,13 +49,10 @@
 public class Neo4jTransactionalContextTest
 {
     private GraphDatabaseQueryService databaseQueryService;
-    private DependencyResolver dependencyResolver;
-    private ThreadToStatementContextBridge statementContextBridge;
     private Guard guard;
     private KernelStatement statement;
     private PropertyContainerLocker propertyContainerLocker;
     private TopLevelTransaction transaction;
-    private QueryRegistryOperations queryRegistryOperations;
 
     @Before
     public void setUp()
@@ -71,12 +68,11 @@ public void checkKernelStatementOnCheck() throws Exception
         ExecutingQuery executingQuery = null;
         DbmsOperations.Factory dbmsOperationsFactory = null;
         ThreadToStatementContextBridge txBridge = null;
-        Neo4jTransactionalContextFactory factory = null;
 
         Neo4jTransactionalContext transactionalContext =
                 new Neo4jTransactionalContext(
-                        databaseQueryService, transaction, transactionType, transactionMode, statement, executingQuery,
-                        propertyContainerLocker, txBridge, dbmsOperationsFactory, guard, factory );
+                        databaseQueryService, transaction, transactionType, transactionMode, () -> statement,
+                        executingQuery, propertyContainerLocker, txBridge, dbmsOperationsFactory, guard );
 
         transactionalContext.check();
 
@@ -98,7 +94,6 @@ public void neverStopsExecutingQueryDuringCommitAndRestartTx()
         ThreadToStatementContextBridge txBridge = mock( ThreadToStatementContextBridge.class );
         Guard guard = mock( Guard.class );
         DbmsOperations.Factory dbmsOperationsFactory = null;
-        Neo4jTransactionalContextFactory factory = mock( Neo4jTransactionalContextFactory.class );
 
         KernelTransaction secondKTX = mock( KernelTransaction.class );
         InternalTransaction secondTransaction = mock( InternalTransaction.class );
@@ -108,14 +103,15 @@ public void neverStopsExecutingQueryDuringCommitAndRestartTx()
         when( executingQuery.queryText() ).thenReturn( "X" );
         when( executingQuery.queryParameters() ).thenReturn( Collections.emptyMap() );
         when( statement.queryRegistration() ).thenReturn( initialQueryRegistry );
-        when( databaseQueryService.beginTransaction( transactionType, transactionMode ) ).thenReturn( secondTransaction );
+        when( databaseQueryService.beginTransaction( transactionType, transactionMode ) )
+                .thenReturn( secondTransaction );
         when( txBridge.getKernelTransactionBoundToThisThread( true ) ).thenReturn( initialKTX, secondKTX );
         when( txBridge.get() ).thenReturn( secondStatement );
         when( secondStatement.queryRegistration() ).thenReturn( secondQueryRegistry );
 
         Neo4jTransactionalContext context = new Neo4jTransactionalContext(
-                databaseQueryService, initialTransaction, transactionType, transactionMode, statement, executingQuery,
-                locker, txBridge, dbmsOperationsFactory, guard, factory );
+                databaseQueryService, initialTransaction, transactionType, transactionMode, () -> statement,
+                executingQuery, locker, txBridge, dbmsOperationsFactory, guard );
 
         // When
         context.commitAndRestartTx();
@@ -150,17 +146,18 @@ public void neverStopsExecutingQueryDuringCommitAndRestartTx()
     private void setUpMocks()
     {
         databaseQueryService = mock( GraphDatabaseQueryService.class );
-        dependencyResolver = mock( DependencyResolver.class );
-        statementContextBridge = mock( ThreadToStatementContextBridge.class );
+        DependencyResolver dependencyResolver = mock( DependencyResolver.class );
+        ThreadToStatementContextBridge statementContextBridge = mock( ThreadToStatementContextBridge.class );
         guard = mock( Guard.class );
         statement = mock( KernelStatement.class );
         propertyContainerLocker = mock( PropertyContainerLocker.class );
         transaction = new TopLevelTransaction( mock( KernelTransaction.class ), () -> statement );
-        queryRegistryOperations = mock( QueryRegistryOperations.class );
+        QueryRegistryOperations queryRegistryOperations = mock( QueryRegistryOperations.class );
 
         when( statement.queryRegistration() ).thenReturn( queryRegistryOperations );
         when( databaseQueryService.getDependencyResolver() ).thenReturn( dependencyResolver );
-        when( dependencyResolver.resolveDependency( ThreadToStatementContextBridge.class ) ).thenReturn( statementContextBridge );
+        when( dependencyResolver.resolveDependency( ThreadToStatementContextBridge.class ) )
+                .thenReturn( statementContextBridge );
         when( dependencyResolver.resolveDependency( Guard.class ) ).thenReturn( guard );
     }
 
@@ -180,7 +177,6 @@ public void rollsBackNewlyCreatedTransactionIfTerminationDetectedOnCloseDuringPe
         PropertyContainerLocker locker = null;
         ThreadToStatementContextBridge txBridge = mock( ThreadToStatementContextBridge.class );
         DbmsOperations.Factory dbmsOperationsFactory = null;
-        Neo4jTransactionalContextFactory factory = mock( Neo4jTransactionalContextFactory.class );
 
         KernelTransaction secondKTX = mock( KernelTransaction.class );
         InternalTransaction secondTransaction = mock( InternalTransaction.class );
@@ -197,17 +193,16 @@ public void rollsBackNewlyCreatedTransactionIfTerminationDetectedOnCloseDuringPe
         when( secondStatement.queryRegistration() ).thenReturn( secondQueryRegistry );
 
         Neo4jTransactionalContext context = new Neo4jTransactionalContext(
-                graph, initialTransaction, transactionType, transactionMode, initialStatement, executingQuery,
-                locker, txBridge, dbmsOperationsFactory, guard, factory
-        );
+                graph, initialTransaction, transactionType, transactionMode, () -> initialStatement, executingQuery,
+                locker, txBridge, dbmsOperationsFactory, guard );
 
         // When
         try
         {
             context.commitAndRestartTx();
             throw new AssertionError( "Expected RuntimeException to be thrown" );
         }
-        catch (RuntimeException e)
+        catch ( RuntimeException e )
         {
             // Then
             Object[] mocks =