Use ProcedureGDSFactory to prevent procedures from invoking blocked/u…

…nsupported operations, like db.shutdown

o Also fixes bug where draining an internal iterator in a ProcedureExecutionResult forgot to call success()

community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/executionplan/procs/ProcedureExecutionResult.scala

@@ -58,8 +58,11 @@ class ProcedureExecutionResult[E <: Exception](context: QueryContext,
     signature.mode.call(context, signature, args.map(javaValues.asDeepJavaResultValue))
 
   override protected def createInner = new util.Iterator[util.Map[String, Any]]() {
-    override def next(): util.Map[String, Any] = resultAsMap(executionResults.next())
-    override def hasNext: Boolean = executionResults.hasNext
+    override def next(): util.Map[String, Any] =
+      try { resultAsMap( executionResults.next( ) ) }
+      catch { case e: NoSuchElementException => success(); throw e }
+
+    override def hasNext: Boolean = if (executionResults.hasNext) true else { success(); false }
   }
 
   override def accept[EX <: Exception](visitor: InternalResultVisitor[EX]) = {

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

@@ -56,7 +56,9 @@
 import org.neo4j.kernel.impl.core.StartupStatisticsProvider;
 import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
 import org.neo4j.kernel.impl.core.TokenNotFoundException;
+import org.neo4j.kernel.impl.coreapi.CoreAPIAvailabilityGuard;
 import org.neo4j.kernel.impl.logging.LogService;
+import org.neo4j.kernel.impl.proc.ProcedureGDSFactory;
 import org.neo4j.kernel.impl.proc.Procedures;
 import org.neo4j.kernel.impl.proc.TypeMappers.SimpleConverter;
 import org.neo4j.kernel.impl.query.QueryEngineProvider;
@@ -154,7 +156,11 @@ public DataSourceModule( final GraphDatabaseFacadeFactory.Dependencies dependenc
                 logging.getInternalLog( DatabaseHealth.class ) ) );
 
         autoIndexing = new InternalAutoIndexing( platformModule.config, editionModule.propertyKeyTokenHolder );
-        Procedures procedures = setupProcedures( platformModule );
+
+
+        AtomicReference<QueryExecutionEngine> queryExecutor = new AtomicReference<>( QueryEngineProvider.noEngine() );
+        this.queryExecutor = queryExecutor::get;
+        Procedures procedures = setupProcedures( platformModule, editionModule.coreAPIAvailabilityGuard );
 
         neoStoreDataSource = deps.satisfyDependency( new NeoStoreDataSource( storeDir, config,
                 editionModule.idGeneratorFactory, logging, platformModule.jobScheduler,
@@ -182,9 +188,6 @@ public DataSourceModule( final GraphDatabaseFacadeFactory.Dependencies dependenc
         // Kernel event handlers should be the very last, i.e. very first to receive shutdown events
         life.add( kernelEventHandlers );
 
-        final AtomicReference<QueryExecutionEngine> queryExecutor = new AtomicReference<>( QueryEngineProvider
-                .noEngine() );
-
         dataSourceManager.addListener( new DataSourceManager.Listener()
         {
             private QueryExecutionEngine engine;
@@ -212,7 +215,6 @@ public void unregistered( NeoStoreDataSource dataSource )
 
         this.storeId = neoStoreDataSource::getStoreId;
         this.kernelAPI = neoStoreDataSource::getKernel;
-        this.queryExecutor = queryExecutor::get;
     }
 
     protected RelationshipProxy.RelationshipActions createRelationshipActions(
@@ -308,7 +310,7 @@ public Relationship newRelationshipProxy( long id, long startNodeId, int typeId,
         };
     }
 
-    private Procedures setupProcedures( PlatformModule platform )
+    private Procedures setupProcedures( PlatformModule platform, CoreAPIAvailabilityGuard coreAPIAvailabilityGuard )
     {
         File pluginDir = platform.config.get( GraphDatabaseSettings.plugin_dir );
         Log internalLog = platform.logging.getInternalLog( Procedures.class );
@@ -321,14 +323,14 @@ private Procedures setupProcedures( PlatformModule platform )
         procedures.registerType( Relationship.class, new SimpleConverter( NTRelationship, Relationship.class ) );
         procedures.registerType( Path.class, new SimpleConverter( NTPath, Path.class ) );
 
-        // Register injectables
-        procedures.registerComponent( GraphDatabaseService.class,
-                (ctx) -> platform.dependencies.resolveDependency( GraphDatabaseService.class ) );
-
+        // Register injected public API components
         Log proceduresLog = platform.logging.getUserLog( Procedures.class  );
         procedures.registerComponent( Log.class, (ctx) -> proceduresLog );
 
-        // Not public API, but useful to have available in procedures to access the kernel etc.
+        // Register injected private API components: useful to have available in procedures to access the kernel etc.
+        ProcedureGDSFactory gdsFactory = new ProcedureGDSFactory( platform.config, platform.storeDir, platform.dependencies, storeId, this.queryExecutor, coreAPIAvailabilityGuard, platform.urlAccessRule );
+        procedures.registerComponent( GraphDatabaseService.class, gdsFactory::apply );
+
         procedures.registerComponent( DependencyResolver.class, (ctx) -> platform.dependencies );
 
         return procedures;

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

@@ -21,33 +21,34 @@
 
 import java.util.HashMap;
 import java.util.Map;
-import java.util.function.Function;
 
+import org.neo4j.function.ThrowingFunction;
+import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.proc.CallableProcedure;
 
 /**
  * Tracks components that can be injected into user-defined procedures.
  */
 public class ComponentRegistry
 {
-    private final Map<Class<?>, Function<CallableProcedure.Context, ?>> suppliers;
+    private final Map<Class<?>, ThrowingFunction<CallableProcedure.Context, ?,ProcedureException>> suppliers;
 
     public ComponentRegistry()
     {
         this( new HashMap<>() );
     }
 
-    public ComponentRegistry( Map<Class<?>,Function<CallableProcedure.Context,?>> suppliers )
+    public ComponentRegistry( Map<Class<?>,ThrowingFunction<CallableProcedure.Context, ?,ProcedureException>> suppliers )
     {
         this.suppliers = suppliers;
     }
 
-    public Function<CallableProcedure.Context,?> supplierFor( Class<?> type )
+    public ThrowingFunction<CallableProcedure.Context, ?,ProcedureException> supplierFor( Class<?> type )
     {
         return suppliers.get( type );
     }
 
-    public <T> void register( Class<T> cls, Function<CallableProcedure.Context,T> supplier )
+    public <T> void register( Class<T> cls, ThrowingFunction<CallableProcedure.Context, ?,ProcedureException> supplier )
     {
         suppliers.put( cls, supplier );
     }

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

@@ -25,8 +25,8 @@
 import java.lang.reflect.Modifier;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.function.Function;
 
+import org.neo4j.function.ThrowingFunction;
 import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.kernel.api.proc.CallableProcedure;
@@ -51,9 +51,9 @@ public static class FieldSetter
     {
         private final Field field;
         private final MethodHandle setter;
-        private final Function<CallableProcedure.Context,?> supplier;
+        private final ThrowingFunction<CallableProcedure.Context, ?, ProcedureException> supplier;
 
-        public FieldSetter( Field field, MethodHandle setter, Function<CallableProcedure.Context,?> supplier )
+        public FieldSetter( Field field, MethodHandle setter, ThrowingFunction<CallableProcedure.Context, ?,ProcedureException> supplier )
         {
             this.field = field;
             this.setter = setter;
@@ -115,7 +115,7 @@ private FieldSetter createInjector( Class<?> cls, Field field ) throws Procedure
     {
         try
         {
-            Function<CallableProcedure.Context,?> supplier = components.supplierFor( field.getType() );
+            ThrowingFunction<CallableProcedure.Context, ?,ProcedureException> supplier = components.supplierFor( field.getType() );
             if( supplier == null )
             {
                 throw new ProcedureException( Status.Procedure.FailedRegistration,

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

@@ -21,9 +21,9 @@
 
 import java.io.File;
 import java.util.Set;
-import java.util.function.Function;
 
 import org.neo4j.collection.RawIterator;
+import org.neo4j.function.ThrowingFunction;
 import org.neo4j.kernel.api.exceptions.KernelException;
 import org.neo4j.kernel.api.exceptions.ProcedureException;
 import org.neo4j.kernel.api.proc.CallableProcedure;
@@ -95,7 +95,7 @@ public void registerType( Class<?> javaClass, TypeMappers.NeoValueConverter toNe
      * @param cls the type of component to be registered (this is what users 'ask' for in their field declaration)
      * @param supplier a function that supplies the actual component, given the context of a procedure invocation
      */
-    public <T> void registerComponent( Class<T> cls, Function<CallableProcedure.Context, T> supplier )
+    public <T> void registerComponent( Class<T> cls, ThrowingFunction<CallableProcedure.Context, ?,ProcedureException> supplier )
     {
         components.register( cls, supplier );
     }

integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java

@@ -19,36 +19,45 @@
  */
 package org.neo4j.procedure;
 
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Stream;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
-import org.neo4j.graphdb.*;
+
+import org.neo4j.graphdb.GraphDatabaseService;
+import org.neo4j.graphdb.Label;
+import org.neo4j.graphdb.Node;
+import org.neo4j.graphdb.QueryExecutionException;
+import org.neo4j.graphdb.Result;
+import org.neo4j.graphdb.Transaction;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.kernel.impl.proc.JarBuilder;
 import org.neo4j.kernel.impl.proc.Procedures;
 import org.neo4j.logging.AssertableLogProvider;
 import org.neo4j.logging.Log;
 import org.neo4j.test.TestGraphDatabaseFactory;
 
-import java.io.IOException;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.stream.Stream;
-
 import static java.lang.System.lineSeparator;
 import static java.util.Spliterator.IMMUTABLE;
 import static java.util.Spliterator.ORDERED;
 import static java.util.Spliterators.spliteratorUnknownSize;
 import static java.util.stream.StreamSupport.stream;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.IsEqual.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+
 import static org.neo4j.graphdb.Label.label;
+import static org.neo4j.helpers.collection.IteratorUtil.single;
 import static org.neo4j.helpers.collection.MapUtil.map;
 import static org.neo4j.logging.AssertableLogProvider.inLog;
 
@@ -260,7 +269,7 @@ public void shouldLogLikeThereIsNoTomorrow() throws Throwable
     }
 
     @Test
-    public void readOnlyProcedureCannotWrite() throws Throwable
+    public void shouldDenyReadOnlyProcedureToPerformWrites() throws Throwable
     {
         // Expect
         exception.expect( QueryExecutionException.class );
@@ -274,13 +283,12 @@ public void readOnlyProcedureCannotWrite() throws Throwable
     }
 
     @Test
-    public void procedureMarkedForWritingCanWrite() throws Throwable
+    public void shouldAllowWriteProcedureToPerformWrites() throws Throwable
     {
         // When
         try ( Transaction tx = db.beginTx() )
         {
-            // TODO: #hasNext should not be needed here, result of writes should be eagerized
-            db.execute( "CALL org.neo4j.procedure.writingProcedure()" ).hasNext();
+            db.execute( "CALL org.neo4j.procedure.writingProcedure()" );
             tx.success();
         }
 
@@ -307,13 +315,12 @@ public void shouldNotBeAbleToCallWriteProcedureThroughReadProcedure() throws Thr
     }
 
     @Test
-    public void procedureMarkedForWritingCanCallOtherWritingProcedure() throws Throwable
+    public void shouldBeAbleToCallWriteProcedureThroughWriteProcedure() throws Throwable
     {
         // When
         try ( Transaction tx = db.beginTx() )
         {
-            // TODO: #hasNext should not be needed here, result of writes should be eagerized
-            db.execute( "CALL org.neo4j.procedure.writeProcedureCallingWriteProcedure()" ).hasNext();
+            db.execute( "CALL org.neo4j.procedure.writeProcedureCallingWriteProcedure()" );
             tx.success();
         }
 
@@ -405,6 +412,42 @@ public void shouldBeAbleToCallDelegatingVoidProcedure() throws Throwable
         }
     }
 
+    @Test
+    public void shouldBeAbleToPerformWritesOnNodesReturnedFromReadOnlyProcedure() throws Throwable
+    {
+        // When
+        try (Transaction tx = db.beginTx())
+        {
+            long nodeId = db.createNode().getId();
+            Node node = single( db.execute( "CALL org.neo4j.procedure.node", map( "id", nodeId ) ).columnAs( "node" ) );
+            node.setProperty( "name", "Stefan" );
+            tx.success();
+        };
+    }
+
+    @Test
+    public void shouldFailToShutdown()
+    {
+        // Expect
+        exception.expect( QueryExecutionException.class );
+        exception.expectMessage( "Failed to invoke procedure `org.neo4j.procedure.shutdown`: `java.lang.UnsupportedOperationException` was thrown when invoking the procedure." );
+
+        try ( Transaction ignore = db.beginTx() )
+        {
+            db.execute( "CALL org.neo4j.procedure.shutdown()" );
+        }
+    }
+
+    @Test
+    public void shouldBeAbleToWriteAfterCallingReadOnlyProcedure()
+    {
+        try ( Transaction ignore = db.beginTx() )
+        {
+            db.execute( "CALL org.neo4j.procedure.simpleArgument(12)" );
+            db.createNode();
+        }
+    }
+
     @Before
     public void setUp() throws IOException
     {
@@ -638,6 +681,11 @@ public void sideEffect( @Name( "value" ) String value )
             db.createNode( Label.label( value ) );
         }
 
+        @Procedure
+        public void shutdown() {
+            db.shutdown();
+        }
+
         @Procedure
         @PerformsWrites
         public void delegatingSideEffect( @Name( "value" ) String value )