diff --git a/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/executionplan/procs/ProcedureExecutionResult.scala b/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/executionplan/procs/ProcedureExecutionResult.scala index 7bf46c101e297..e3ec15598929b 100644 --- a/community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/executionplan/procs/ProcedureExecutionResult.scala +++ b/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]) = { 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 index d8a6b4575a7d9..8d3e07665b918 100644 --- 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 @@ -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 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 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; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ComponentRegistry.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ComponentRegistry.java index 05291d3859445..ec1b2a94c1125 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ComponentRegistry.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ComponentRegistry.java @@ -21,8 +21,9 @@ 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; /** @@ -30,24 +31,24 @@ */ public class ComponentRegistry { - private final Map, Function> suppliers; + private final Map, ThrowingFunction> suppliers; public ComponentRegistry() { this( new HashMap<>() ); } - public ComponentRegistry( Map,Function> suppliers ) + public ComponentRegistry( Map,ThrowingFunction> suppliers ) { this.suppliers = suppliers; } - public Function supplierFor( Class type ) + public ThrowingFunction supplierFor( Class type ) { return suppliers.get( type ); } - public void register( Class cls, Function supplier ) + public void register( Class cls, ThrowingFunction supplier ) { suppliers.put( cls, supplier ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/FieldInjections.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/FieldInjections.java index 8f6e49daf9b9b..948ec38f35297 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/FieldInjections.java +++ b/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 supplier; + private final ThrowingFunction supplier; - public FieldSetter( Field field, MethodHandle setter, Function supplier ) + public FieldSetter( Field field, MethodHandle setter, ThrowingFunction supplier ) { this.field = field; this.setter = setter; @@ -115,7 +115,7 @@ private FieldSetter createInjector( Class cls, Field field ) throws Procedure { try { - Function supplier = components.supplierFor( field.getType() ); + ThrowingFunction supplier = components.supplierFor( field.getType() ); if( supplier == null ) { throw new ProcedureException( Status.Procedure.FailedRegistration, diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/Procedures.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/Procedures.java index 5c9dc29997776..bad3ab18b6021 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/Procedures.java +++ b/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 void registerComponent( Class cls, Function supplier ) + public void registerComponent( Class cls, ThrowingFunction supplier ) { components.register( cls, supplier ); } diff --git a/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java b/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java index 830dd17a5a589..2029a81d23650 100644 --- a/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java +++ b/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java @@ -19,13 +19,25 @@ */ 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; @@ -33,22 +45,19 @@ 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 )