Remove DbmsOperations from KernelStatement

- QueryTransactionalContext is extended with dbms operations - DbmsOperations are made independent of the kernel transaction as a separate dependency. - A NonTransactionalDbmsOperations implementation is provided by DataSourceModule. - The context for dbms procedure calls are set up with only AuthSubject (as AccessMode), and no kernel transaction. (This context could instead be extended in the future with a new type of dbms transaction when needed.)
neo4j · Mar 10, 2016 · 4a78781 · 4a78781
1 parent 6f6ddb4
commit 4a78781
Show file tree

Hide file tree

Showing 19 changed files with 129 additions and 116 deletions.
diff --git a/...0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/DelegatingQueryContext.scala b/...0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/DelegatingQueryContext.scala
@@ -230,8 +230,12 @@ class DelegatingQueryTransactionalContext(val inner: QueryTransactionalContext)
 
   override type ReadOps = inner.ReadOps
 
+  override type DbmsOps = inner.DbmsOps
+
   override def readOperations: ReadOps = inner.readOperations
 
+  override def dbmsOperations: DbmsOps = inner.dbmsOperations
+
   override def commitAndRestartTx() { inner.commitAndRestartTx() }
 
   override def isTopLevelTx: Boolean = inner.isTopLevelTx

diff --git a/...ompiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/QueryContext.scala b/...ompiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/QueryContext.scala
@@ -196,9 +196,12 @@ trait Operations[T <: PropertyContainer] {
 trait QueryTransactionalContext {
 
   type ReadOps
+  type DbmsOps
 
   def readOperations: ReadOps
 
+  def dbmsOperations: DbmsOps
+
   def isTopLevelTx: Boolean
 
   def close(success: Boolean)

diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala
@@ -138,7 +138,7 @@ class ExecutionEngine(val queryService: GraphDatabaseQueryService, logProvider:
         val tc = externalTransactionalContext.provideContext()
 
         // Temporarily change access mode during query planning
-        val revertable = tc.restrict(AccessMode.Static.READ)
+        val revertable = tc.restrictCurrentTransaction(AccessMode.Static.READ)
 
         val ((plan: ExecutionPlan, extractedParameters), touched) = try {
           // fetch plan cache

diff --git a/...her/cypher/src/main/scala/org/neo4j/cypher/internal/spi/TransactionalContextWrapper.scala b/...her/cypher/src/main/scala/org/neo4j/cypher/internal/spi/TransactionalContextWrapper.scala
@@ -25,13 +25,15 @@ import org.neo4j.kernel.GraphDatabaseQueryService
 import org.neo4j.kernel.api.KernelTransaction.Revertable
 import org.neo4j.kernel.api.security.AccessMode
 import org.neo4j.kernel.api.txstate.TxStateHolder
-import org.neo4j.kernel.api.{ReadOperations, Statement}
+import org.neo4j.kernel.api.{DbmsOperations, ReadOperations, Statement}
 import org.neo4j.kernel.impl.query.TransactionalContext
 
 case class TransactionalContextWrapper(tc: TransactionalContext) extends QueryTransactionalContext {
 
   override type ReadOps = ReadOperations
 
+  override type DbmsOps = DbmsOperations
+
   def provideContext(): TransactionalContextWrapper = TransactionalContextWrapper(tc.provideContext())
 
   def isOpen: Boolean = tc.isOpen
@@ -49,11 +51,15 @@ case class TransactionalContextWrapper(tc: TransactionalContext) extends QueryTr
 
   override def readOperations: ReadOperations = tc.readOperations()
 
+  override def dbmsOperations: DbmsOperations = tc.dbmsOperations()
+
   override def commitAndRestartTx() { tc.commitAndRestartTx() }
 
   override def isTopLevelTx: Boolean = tc.isTopLevelTx
 
   override def close(success: Boolean) { tc.close(success) }
 
-  def restrict(accessMode: AccessMode): Revertable = tc.restrict(accessMode)
+  def restrictCurrentTransaction(accessMode: AccessMode): Revertable = tc.restrictCurrentTransaction(accessMode)
+
+  def accessMode: AccessMode = tc.accessMode
 }
diff --git a/...pher/src/main/scala/org/neo4j/cypher/internal/spi/v3_0/TransactionBoundQueryContext.scala b/...pher/src/main/scala/org/neo4j/cypher/internal/spi/v3_0/TransactionBoundQueryContext.scala
@@ -554,7 +554,9 @@ final class TransactionBoundQueryContext(val transactionalContext: Transactional
     callProcedure(name, args, transactionalContext.statement.dataWriteOperations().procedureCallWrite )
 
   override def callDbmsProcedure(name: ProcedureName, args: Seq[Any]) =
-    callProcedure(name, args, transactionalContext.statement.dbmsOperations().procedureCallDbms )
+    callProcedure(name, args,
+                  (kn: proc.ProcedureSignature.ProcedureName, input: Array[AnyRef]) =>
+                    transactionalContext.dbmsOperations.procedureCallDbms(kn, input, transactionalContext.accessMode))
 
   private def callProcedure(name: ProcedureName, args: Seq[Any],
                             call: (proc.ProcedureSignature.ProcedureName, Array[AnyRef]) => RawIterator[Array[AnyRef], ProcedureException]) = {

diff --git a/...unity/cypher/cypher/src/test/scala/org/neo4j/cypher/internal/compiler/v3_0/LazyTest.scala b/...unity/cypher/cypher/src/test/scala/org/neo4j/cypher/internal/compiler/v3_0/LazyTest.scala
@@ -245,8 +245,8 @@ class LazyTest extends ExecutionEngineFunSuite {
 
     val tx = fakeGraph.beginTransaction(KernelTransaction.Type.`implicit`, AccessMode.Static.FULL)
     val context = new Neo4jTransactionalContext(service, tx, fakeStatement, new PropertyContainerLocker)
-    val revertableRestrict = mock[Revertable]
-    when(context.restrict(anyObject())).thenReturn(revertableRestrict)
+    val revertable = mock[Revertable]
+    when(context.restrictCurrentTransaction(anyObject())).thenReturn(revertable)
     val session = QueryEngineProvider.embeddedSession(context)
 
     //When:

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/DbmsOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/api/DbmsOperations.java
@@ -19,41 +19,10 @@
  */
 package org.neo4j.kernel.api;
 
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Set;
-import java.util.function.Function;
-
 import org.neo4j.collection.RawIterator;
-import org.neo4j.collection.primitive.PrimitiveIntIterator;
-import org.neo4j.collection.primitive.PrimitiveLongIterator;
-import org.neo4j.cursor.Cursor;
-import org.neo4j.graphdb.Direction;
-import org.neo4j.kernel.api.constraints.NodePropertyConstraint;
-import org.neo4j.kernel.api.constraints.PropertyConstraint;
-import org.neo4j.kernel.api.constraints.RelationshipPropertyConstraint;
-import org.neo4j.kernel.api.exceptions.EntityNotFoundException;
-import org.neo4j.kernel.api.exceptions.LabelNotFoundKernelException;
 import org.neo4j.kernel.api.exceptions.ProcedureException;
-import org.neo4j.kernel.api.exceptions.PropertyKeyIdNotFoundKernelException;
-import org.neo4j.kernel.api.exceptions.RelationshipTypeIdNotFoundKernelException;
-import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
-import org.neo4j.kernel.api.exceptions.legacyindex.LegacyIndexNotFoundKernelException;
-import org.neo4j.kernel.api.exceptions.schema.DuplicateIndexSchemaRuleException;
-import org.neo4j.kernel.api.exceptions.schema.IndexBrokenKernelException;
-import org.neo4j.kernel.api.exceptions.schema.SchemaRuleNotFoundException;
-import org.neo4j.kernel.api.index.IndexDescriptor;
-import org.neo4j.kernel.api.index.InternalIndexState;
-import org.neo4j.kernel.api.proc.CallableProcedure;
 import org.neo4j.kernel.api.proc.ProcedureSignature;
-import org.neo4j.kernel.impl.api.RelationshipVisitor;
-import org.neo4j.kernel.impl.api.store.RelationshipIterator;
-import org.neo4j.register.Register.DoubleLongRegister;
-import org.neo4j.storageengine.api.NodeItem;
-import org.neo4j.storageengine.api.RelationshipItem;
-import org.neo4j.storageengine.api.Token;
-import org.neo4j.storageengine.api.lock.ResourceType;
-import org.neo4j.storageengine.api.schema.PopulationProgress;
+import org.neo4j.kernel.api.security.AccessMode;
 
 /**
  * Defines all types of system-oriented operations - i.e. those which do not read from or write to the graph - that can be done from the {@link KernelAPI}.
@@ -66,9 +35,6 @@ public interface DbmsOperations
     //=================================================
 
     /** Invoke a DBMS procedure by name */
-    RawIterator<Object[], ProcedureException> procedureCallDbms( ProcedureSignature.ProcedureName name, Object[] input ) throws ProcedureException;
-
-    //=================================================
-    //== AUTH OPERATIONS ==
-    //=================================================
+    RawIterator<Object[],ProcedureException> procedureCallDbms( ProcedureSignature.ProcedureName name, Object[] input,
+            AccessMode accessMode ) throws ProcedureException;
 }
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/Statement.java b/community/kernel/src/main/java/org/neo4j/kernel/api/Statement.java
@@ -58,9 +58,4 @@ public interface Statement extends Resource
      * and it's of a different type, e.g {@link #dataWriteOperations()}.
      */
     SchemaWriteOperations schemaWriteOperations() throws InvalidTransactionTypeKernelException;
-
-    /**
-     * @return interface exposing all DBMS operations.
-     */
-    DbmsOperations dbmsOperations();
 }
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/proc/CallableProcedure.java b/community/kernel/src/main/java/org/neo4j/kernel/api/proc/CallableProcedure.java
@@ -26,6 +26,7 @@
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.exceptions.Status;
+import org.neo4j.kernel.api.security.AccessMode;
 
 public interface CallableProcedure
 {
@@ -41,6 +42,7 @@ public interface CallableProcedure
     interface Context
     {
         Key<KernelTransaction> KERNEL_TRANSACTION = Key.key( "KernelTransaction", KernelTransaction.class );
+        Key<AccessMode> ACCESS_MODE = Key.key( "AccessMode", AccessMode.class );
 
         <T> T get( Key<T> key ) throws ProcedureException;
     }

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/builtinprocs/AlterUserPasswordProcedure.java b/community/kernel/src/main/java/org/neo4j/kernel/builtinprocs/AlterUserPasswordProcedure.java
@@ -54,7 +54,7 @@ public AlterUserPasswordProcedure( ProcedureName name )
     @Override
     public RawIterator<Object[],ProcedureException> apply( Context ctx, Object[] input ) throws ProcedureException
     {
-        AccessMode accessMode = ctx.get( Context.KERNEL_TRANSACTION ).mode();
+        AccessMode accessMode = ctx.get( Context.ACCESS_MODE );
         if ( !(accessMode instanceof AuthSubject) )
         {
             throw new AuthorizationViolationException( "Invalid attempt to change the password" );
@@ -69,7 +69,7 @@ public RawIterator<Object[],ProcedureException> apply( Context ctx, Object[] inp
         catch ( IOException e )
         {
             throw new ProcedureException( Status.Security.Forbidden, e,
-                    "The password was not able to be changed for the provided username");
+                    "Failed to change the password for the provided username" );
         }
     }
 }
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java
@@ -22,7 +22,6 @@
 import org.neo4j.graphdb.NotInTransactionException;
 import org.neo4j.graphdb.TransactionTerminatedException;
 import org.neo4j.graphdb.security.AuthorizationViolationException;
-import org.neo4j.kernel.api.DbmsOperations;
 import org.neo4j.kernel.api.DataWriteOperations;
 import org.neo4j.kernel.api.ReadOperations;
 import org.neo4j.kernel.api.SchemaWriteOperations;
@@ -117,12 +116,6 @@ public SchemaWriteOperations schemaWriteOperations()
         return facade;
     }
 
-    @Override
-    public DbmsOperations dbmsOperations()
-    {
-        return facade;
-    }
-
     @Override
     public TransactionState txState()
     {

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/NonTransactionalDbmsOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/NonTransactionalDbmsOperations.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.kernel.impl.api;
+
+import org.neo4j.collection.RawIterator;
+import org.neo4j.kernel.api.DbmsOperations;
+import org.neo4j.kernel.api.exceptions.ProcedureException;
+import org.neo4j.kernel.api.proc.CallableProcedure;
+import org.neo4j.kernel.api.proc.ProcedureSignature;
+import org.neo4j.kernel.api.security.AccessMode;
+import org.neo4j.kernel.impl.proc.Procedures;
+
+public class NonTransactionalDbmsOperations implements DbmsOperations
+{
+
+    private final Procedures procedures;
+
+    public NonTransactionalDbmsOperations( Procedures procedures )
+    {
+        this.procedures = procedures;
+    }
+
+    @Override
+    public RawIterator<Object[],ProcedureException> procedureCallDbms( ProcedureSignature.ProcedureName name,
+            Object[] input, AccessMode accessMode ) throws ProcedureException
+    {
+        CallableProcedure.BasicContext ctx = new CallableProcedure.BasicContext();
+        ctx.put( CallableProcedure.Context.ACCESS_MODE, accessMode );
+        return procedures.call( ctx, name, input );
+    }
+
+}
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java
@@ -31,7 +31,6 @@
 import org.neo4j.cursor.Cursor;
 import org.neo4j.graphdb.Direction;
 import org.neo4j.kernel.api.security.AccessMode;
-import org.neo4j.kernel.api.DbmsOperations;
 import org.neo4j.kernel.api.DataWriteOperations;
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.kernel.api.LegacyIndexHits;
@@ -91,7 +90,7 @@
 import org.neo4j.storageengine.api.lock.ResourceType;
 import org.neo4j.storageengine.api.schema.PopulationProgress;
 
-public class OperationsFacade implements ReadOperations, DataWriteOperations, SchemaWriteOperations, DbmsOperations
+public class OperationsFacade implements ReadOperations, DataWriteOperations, SchemaWriteOperations
 {
     private final KernelTransaction tx;
     private final KernelStatement statement;
@@ -1099,21 +1098,6 @@ public void uniqueIndexDrop( IndexDescriptor descriptor ) throws DropIndexFailur
 
     // </SchemaWrite>
 
-    // <DBMS>
-
-    @Override
-    public RawIterator<Object[],ProcedureException> procedureCallDbms( ProcedureName name,
-            Object[] input ) throws ProcedureException
-    {
-        statement.assertOpen();
-
-        CallableProcedure.BasicContext ctx = new CallableProcedure.BasicContext();
-        ctx.put( CallableProcedure.Context.KERNEL_TRANSACTION, tx );
-        return procedures.call( ctx, name, input );
-    }
-
-    // </DBMS>
-
     // <Locking>
     @Override
     public void acquireExclusive( ResourceType type, long id )

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/DataSourceModule.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/DataSourceModule.java
@@ -36,13 +36,15 @@
 import org.neo4j.kernel.AvailabilityGuard;
 import org.neo4j.kernel.DatabaseAvailability;
 import org.neo4j.kernel.NeoStoreDataSource;
+import org.neo4j.kernel.api.DbmsOperations;
 import org.neo4j.kernel.api.KernelAPI;
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.kernel.api.Statement;
 import org.neo4j.kernel.api.legacyindex.AutoIndexing;
 import org.neo4j.kernel.builtinprocs.BuiltInProcedures;
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.kernel.guard.Guard;
+import org.neo4j.kernel.impl.api.NonTransactionalDbmsOperations;
 import org.neo4j.kernel.impl.api.NonTransactionalTokenNameLookup;
 import org.neo4j.kernel.impl.api.SchemaWriteGuard;
 import org.neo4j.kernel.impl.api.index.IndexingService;
@@ -165,6 +167,9 @@ public DataSourceModule( final GraphDatabaseFacadeFactory.Dependencies dependenc
         this.queryExecutor = queryExecutor::get;
         Procedures procedures = setupProcedures( platformModule, editionModule.coreAPIAvailabilityGuard );
 
+        DbmsOperations dbmsOperations = new NonTransactionalDbmsOperations( procedures );
+        deps.satisfyDependency( dbmsOperations );
+
         NonTransactionalTokenNameLookup tokenNameLookup = new NonTransactionalTokenNameLookup(
                 editionModule.labelTokenHolder,
                 editionModule.relationshipTypeTokenHolder,

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java
@@ -22,6 +22,7 @@
 import org.neo4j.graphdb.Lock;
 import org.neo4j.graphdb.PropertyContainer;
 import org.neo4j.kernel.GraphDatabaseQueryService;
+import org.neo4j.kernel.api.DbmsOperations;
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.kernel.api.ReadOperations;
 import org.neo4j.kernel.api.Statement;
@@ -38,6 +39,7 @@ public class Neo4jTransactionalContext implements TransactionalContext
     private final ThreadToStatementContextBridge txBridge;
     private final KernelTransaction.Type transactionType;
     private final AccessMode mode;
+    private final DbmsOperations dbmsOperations;
 
     private InternalTransaction transaction;
     private Statement statement;
@@ -55,6 +57,7 @@ public Neo4jTransactionalContext( GraphDatabaseQueryService graph, InternalTrans
         this.statement = initialStatement;
         this.locker = locker;
         this.txBridge = graph.getDependencyResolver().resolveDependency( ThreadToStatementContextBridge.class );
+        this.dbmsOperations = graph.getDependencyResolver().resolveDependency( DbmsOperations.class );
     }
 
     @Override
@@ -63,6 +66,12 @@ public ReadOperations readOperations()
         return statement.readOperations();
     }
 
+    @Override
+    public DbmsOperations dbmsOperations()
+    {
+        return dbmsOperations;
+    }
+
     @Override
     public boolean isTopLevelTx()
     {
@@ -163,8 +172,14 @@ public Lock acquireWriteLock( PropertyContainer p )
     }
 
     @Override
-    public KernelTransaction.Revertable restrict( AccessMode accessMode )
+    public KernelTransaction.Revertable restrictCurrentTransaction( AccessMode accessMode )
     {
         return transaction.restrict( accessMode );
     }
+
+    @Override
+    public AccessMode accessMode()
+    {
+        return mode;
+    }
 }