diff --git a/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/ast/ResolvedFunctionInvocation.scala b/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/ast/ResolvedFunctionInvocation.scala index a0bdf3468db2..ee819c5f0850 100644 --- a/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/ast/ResolvedFunctionInvocation.scala +++ b/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/ast/ResolvedFunctionInvocation.scala @@ -49,7 +49,7 @@ case class ResolvedFunctionInvocation(qualifiedName: QualifiedName, fcnSignature: Option[UserDefinedFunctionSignature], callArguments: IndexedSeq[Expression]) (val position: InputPosition) - extends Expression with UserDefined { + extends Expression { def coerceArguments: ResolvedFunctionInvocation = fcnSignature match { case Some(signature) => @@ -82,12 +82,4 @@ case class ResolvedFunctionInvocation(qualifiedName: QualifiedName, position)) } } - - override def containsNoUpdates = fcnSignature match { - case None => true - case Some(signature) => signature.accessMode match { - case _: ProcedureReadOnlyAccess => true - case _ => false - } - } } diff --git a/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/spi/ProcedureSignature.scala b/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/spi/ProcedureSignature.scala index c42d35d8d815..f88c02f569fe 100644 --- a/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/spi/ProcedureSignature.scala +++ b/community/cypher/cypher-compiler-3.1/src/main/scala/org/neo4j/cypher/internal/compiler/v3_1/spi/ProcedureSignature.scala @@ -37,7 +37,7 @@ case class UserDefinedFunctionSignature(name: QualifiedName, inputSignature: IndexedSeq[CypherType], outputType: CypherType, deprecationInfo: Option[String], - accessMode: ProcedureAccessMode) + accessMode: ProcedureReadOnlyAccess) object QualifiedName { def apply(unresolved: UnresolvedCall): QualifiedName = diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/TransactionBoundPlanContext.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/TransactionBoundPlanContext.scala index 9e1955e2919d..277548996c16 100644 --- a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/TransactionBoundPlanContext.scala +++ b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/TransactionBoundPlanContext.scala @@ -152,9 +152,8 @@ class TransactionBoundPlanContext(tc: TransactionalContextWrapperv3_1) .toIndexedSeq val output = asCypherType(ks.outputType()) val deprecationInfo = asOption(ks.deprecated()) - val mode = asCypherProcMode(ks.mode(), ks.allowed()) - Some(UserDefinedFunctionSignature(name, input, output, deprecationInfo, mode)) + Some(UserDefinedFunctionSignature(name, input, output, deprecationInfo, ProcedureReadOnlyAccess(ks.allowed()))) } else None } diff --git a/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Clause.scala b/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Clause.scala index d699c9428c32..b74f65809473 100644 --- a/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Clause.scala +++ b/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Clause.scala @@ -292,13 +292,10 @@ case class Unwind(expression: Expression, variable: Variable)(val position: Inpu } } -trait UserDefined { - def containsNoUpdates: Boolean -} - -abstract class CallClause extends Clause with UserDefined { +abstract class CallClause extends Clause { override def name = "CALL" def returnColumns: List[String] + def containsNoUpdates: Boolean } case class UnresolvedCall(procedureNamespace: Namespace, diff --git a/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Query.scala b/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Query.scala index 2d43b74d4af4..f96b9d8757d3 100644 --- a/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Query.scala +++ b/community/cypher/frontend-3.1/src/main/scala/org/neo4j/cypher/internal/frontend/v3_1/ast/Query.scala @@ -19,7 +19,6 @@ */ package org.neo4j.cypher.internal.frontend.v3_1.ast -import org.neo4j.cypher.internal.frontend.v3_1.ast.functions.UnresolvedFunction import org.neo4j.cypher.internal.frontend.v3_1.{InputPosition, SemanticChecking, SemanticError, _} case class Query(periodicCommitHint: Option[PeriodicCommitHint], part: QueryPart)(val position: InputPosition) @@ -43,17 +42,13 @@ sealed trait QueryPart extends ASTNode with ASTPhrase with SemanticCheckable { case class SingleQuery(clauses: Seq[Clause])(val position: InputPosition) extends QueryPart { assert(clauses.nonEmpty) - override def containsUpdates = { + override def containsUpdates = clauses.exists { - case call: UserDefined => !call.containsNoUpdates + case call: CallClause => !call.containsNoUpdates case _: UpdateClause => true - case other => other.exists { - //If the function is unresolved it might contain updates, we will know for sure after rewrite - case call: FunctionInvocation if call.function == UnresolvedFunction => true - case call: UserDefined => !call.containsNoUpdates - } + case _ => false } - } + override def returnColumns = clauses.last.returnColumns override def semanticCheck = diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/proc/FunctionSignature.java b/community/kernel/src/main/java/org/neo4j/kernel/api/proc/FunctionSignature.java index e9180e359ba5..aa0ab1acbdd8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/proc/FunctionSignature.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/proc/FunctionSignature.java @@ -38,7 +38,6 @@ public class FunctionSignature private final QualifiedName name; private final List inputSignature; private final Neo4jTypes.AnyType type; - private final Mode mode; private final String[] allowed; private final Optional deprecated; private final Optional description; @@ -46,7 +45,6 @@ public class FunctionSignature public FunctionSignature( QualifiedName name, List inputSignature, Neo4jTypes.AnyType type, - Mode mode, Optional deprecated, String[] allowed, Optional description ) @@ -54,7 +52,6 @@ public FunctionSignature( QualifiedName name, this.name = name; this.inputSignature = unmodifiableList( inputSignature ); this.type = type; - this.mode = mode; this.deprecated = deprecated; this.description = description; this.allowed = allowed; @@ -65,11 +62,6 @@ public QualifiedName name() return name; } - public Mode mode() - { - return mode; - } - public Optional deprecated() { return deprecated; @@ -125,7 +117,6 @@ public static class Builder private final QualifiedName name; private final List inputSignature = new LinkedList<>(); private Neo4jTypes.AnyType outputType; - private Mode mode = Mode.READ_ONLY; private String[] allowed = new String[0]; private Optional deprecated = Optional.empty(); private Optional description = Optional.empty(); @@ -135,12 +126,6 @@ public Builder( String[] namespace, String name ) this.name = new QualifiedName( namespace, name ); } - public Builder mode( Mode mode ) - { - this.mode = mode; - return this; - } - public Builder description(String description) { this.description = Optional.of( description ); @@ -179,7 +164,7 @@ public FunctionSignature build() { throw new IllegalStateException( "output type must be set" ); } - return new FunctionSignature(name, inputSignature, outputType, mode, deprecated, allowed, description ); + return new FunctionSignature(name, inputSignature, outputType, deprecated, allowed, description ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java index 3a804ca12a15..ae977a44673d 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java @@ -203,14 +203,12 @@ private ReflectiveFunction compileFunction( Class procDefinition, MethodHandl Optional description = description( method ); Function function = method.getAnnotation( Function.class ); - Mode mode = mode( function.mode() ); Optional deprecated = deprecated( method, function::deprecatedBy, "Use of @Function(deprecatedBy) without @Deprecated in " + procName ); FunctionSignature signature = - new FunctionSignature( procName, inputSignature, valueConverter.type(), - mode, deprecated, function.allowed(), description ); + new FunctionSignature( procName, inputSignature, valueConverter.type(), deprecated, function.allowed(), description ); return new ReflectiveFunction( signature, constructor, procedureMethod, valueConverter, setters ); } diff --git a/community/kernel/src/main/java/org/neo4j/procedure/Function.java b/community/kernel/src/main/java/org/neo4j/procedure/Function.java index 8115f8baf282..a595817edb43 100644 --- a/community/kernel/src/main/java/org/neo4j/procedure/Function.java +++ b/community/kernel/src/main/java/org/neo4j/procedure/Function.java @@ -121,15 +121,6 @@ */ String name() default ""; - /** - * A function is associated with one of the following modes - * READ allows only reading the graph (default mode) - * WRITE allows reading and writing the graph - * SCHEMA allows reading the graphs and performing schema operations - * DBMS allows managing the database (i.e. change password) - */ - Mode mode() default Mode.DEFAULT; - /** * When deprecating a function it is useful to indicate a possible * replacement procedure that clients might show in warnings @@ -145,5 +136,4 @@ * @return an array of role names whose users are allowed to execute this function. */ String[] allowed() default {}; - } diff --git a/integrationtests/src/test/java/org/neo4j/procedure/FunctionIT.java b/integrationtests/src/test/java/org/neo4j/procedure/FunctionIT.java index 49c4c09aa223..c97222ca0f5d 100644 --- a/integrationtests/src/test/java/org/neo4j/procedure/FunctionIT.java +++ b/integrationtests/src/test/java/org/neo4j/procedure/FunctionIT.java @@ -76,7 +76,6 @@ import static org.neo4j.helpers.collection.Iterables.asList; import static org.neo4j.helpers.collection.MapUtil.map; import static org.neo4j.logging.AssertableLogProvider.inLog; -import static org.neo4j.procedure.Mode.SCHEMA; import static org.neo4j.procedure.Mode.WRITE; public class FunctionIT @@ -316,38 +315,6 @@ public void shouldDenyReadOnlyFunctionToPerformWrites() throws Throwable } } - @Test - public void shouldAllowWriteFunctionToPerformWrites() throws Throwable - { - // When - try ( Transaction tx = db.beginTx() ) - { - db.execute( "RETURN org.neo4j.procedure.writingFunction()" ).next(); - tx.success(); - } - - // Then - try ( Transaction tx = db.beginTx() ) - { - assertEquals( 1, db.getAllNodes().stream().count() ); - tx.success(); - } - } - - @Test - public void shouldNotBeAbleToCallWriteFunctionThroughReadFunction() throws Throwable - { - // Expect - exception.expect( QueryExecutionException.class ); - exception.expectMessage( "Write operations are not allowed" ); - - // When - try ( Transaction ignore = db.beginTx() ) - { - db.execute( "RETURN org.neo4j.procedure.readOnlyCallingWriteFunction()" ).next(); - } - } - @Test public void shouldNotBeAbleToCallWriteProcedureThroughReadFunction() throws Throwable { @@ -362,74 +329,6 @@ public void shouldNotBeAbleToCallWriteProcedureThroughReadFunction() throws Thro } } - @Test - public void shouldNotBeAbleToCallReadFunctionThroughWriteFunctionInWriteOnlyTransaction() throws Throwable - { - // Expect - exception.expect( QueryExecutionException.class ); - exception.expectMessage( "Read operations are not allowed" ); - - GraphDatabaseAPI gdapi = (GraphDatabaseAPI) db; - - // When - try ( Transaction tx = gdapi.beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.WRITE_ONLY ) ) - { - db.execute( "RETURN org.neo4j.procedure.writeFunctionCallingReadFunction()" ).next(); - } - } - - @Test - public void shouldBeAbleToCallWriteFunctionThroughWriteFunction() throws Throwable - { - // When - try ( Transaction tx = db.beginTx() ) - { - db.execute( "RETURN org.neo4j.procedure.writeFunctionCallingWriteFunction()" ).next(); - tx.success(); - } - - // Then - try ( Transaction tx = db.beginTx() ) - { - assertEquals( 1, db.getAllNodes().stream().count() ); - tx.success(); - } - } - - @Test - public void shouldBeAbleToCallWriteProcedureThroughWriteFunction() throws Throwable - { - // When - try ( Transaction tx = db.beginTx() ) - { - db.execute( "RETURN org.neo4j.procedure.writeFunctionCallingWriteProcedure()" ).next(); - tx.success(); - } - - // Then - try ( Transaction tx = db.beginTx() ) - { - assertEquals( 1, db.getAllNodes().stream().count() ); - tx.success(); - } - } - - @Test - public void shouldNotBeAbleToCallSchemaFunctionThroughWriteFunctionInWriteTransaction() throws Throwable - { - // Expect - exception.expect( QueryExecutionException.class ); - exception.expectMessage( "Schema operations are not allowed" ); - - GraphDatabaseAPI gdapi = (GraphDatabaseAPI) db; - - // When - try ( Transaction tx = gdapi.beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.WRITE ) ) - { - db.execute( "RETURN org.neo4j.procedure.writeFunctionCallingSchemaFunction()" ).next(); - } - } - @Test public void shouldDenyReadOnlyFunctionToPerformSchema() throws Throwable { @@ -445,79 +344,6 @@ public void shouldDenyReadOnlyFunctionToPerformSchema() throws Throwable } } - @Test - public void shouldDenyReadWriteFunctionToPerformSchema() throws Throwable - { - // Expect - exception.expect( QueryExecutionException.class ); - exception.expectMessage( "Cannot perform schema updates in a transaction that has performed data updates" ); - - // Give - try ( Transaction ignore = db.beginTx() ) - { - // When - db.execute( "RETURN org.neo4j.procedure.readWriteTryingToWriteSchema()" ).next(); - } - } - - @Test - public void shouldAllowSchemaFunctionToPerformSchema() throws Throwable - { - // Give - try ( Transaction tx = db.beginTx() ) - { - // When - db.execute( "RETURN org.neo4j.procedure.schemaFunction()" ).next(); - tx.success(); - } - - // Then - try ( Transaction tx = db.beginTx() ) - { - assertTrue( db.schema().getConstraints().iterator().hasNext() ); - tx.success(); - } - } - - @Test - public void shouldAllowSchemaCallReadOnly() throws Throwable - { - // Given - long nodeId; - try ( Transaction tx = db.beginTx() ) - { - nodeId = db.createNode().getId(); - tx.success(); - } - - try ( Transaction ignore = db.beginTx() ) - { - // When - Result res = db.execute( "RETURN org.neo4j.procedure.schemaCallReadFunction({id}) AS node", - map( "id", nodeId ) ); - - // Then - Node node = (Node) res.next().get( "node" ); - assertThat( node.getId(), equalTo( nodeId ) ); - assertFalse( res.hasNext() ); - } - } - - @Test - public void shouldDenySchemaFunctionToPerformWrite() throws Throwable - { - // Expect - exception.expect( QueryExecutionException.class ); - exception.expectMessage( "Cannot perform data updates in a transaction that has performed schema updates" ); - - // Give - try ( Transaction ignore = db.beginTx() ) - { - // When - db.execute( "RETURN org.neo4j.procedure.schemaTryingToWrite()" ).next(); - } - } - @Test public void shouldCoerceLongToDoubleAtRuntimeWhenCallingFunction() throws Throwable { @@ -676,16 +502,16 @@ public void shouldBeAbleToUseFunctionCallWithPeriodicCommit() throws IOException //WHEN Result result = db.execute( "USING PERIODIC COMMIT 1 " + "LOAD CSV FROM '" + url + "' AS line " + - "WITH org.neo4j.procedure.createNode(line[0]) AS n " + + "CREATE (n {prop: org.neo4j.procedure.simpleArgument(toInt(line[0]))}) " + "RETURN n.prop" ); // THEN - for ( int i = 1; i <= 100; i++ ) + for ( long i = 1; i <= 100L; i++ ) { - assertThat( result.next().get( "n.prop" ), equalTo( Integer.toString( i ) ) ); + assertThat( result.next().get( "n.prop" ), equalTo( i ) ); } //Make sure all the lines has been properly commited to the database. - String[] dbContents = db.execute( "MATCH (n) return n.prop" ).stream().map( m -> (String) m.get( "n.prop" ) ) + String[] dbContents = db.execute( "MATCH (n) return n.prop" ).stream().map( m -> Long.toString( (long) m.get( "n.prop" ) )) .toArray( String[]::new ); assertThat( dbContents, equalTo( lines ) ); } @@ -707,22 +533,6 @@ public void shouldFailIfUsingPeriodicCommitWithReadOnlyQuery() throws IOExceptio "RETURN val" ); } - @Test - public void shouldBeAbleToUseCallYieldWithLoadCsvAndSet() throws IOException - { - // GIVEN - String url = createCsvFile( "foo" ); - - //WHEN - Result result = db.execute( - "LOAD CSV FROM '" + url + "' AS line " + - "WITH org.neo4j.procedure.createNode(line[0]) AS n " + - "SET n.p = 42 " + - "RETURN n.p" ); - // THEN - assertThat( result.next().get( "n.p" ), equalTo( 42L ) ); - } - @Test public void shouldCallFunctionReturningPaths() throws Throwable { @@ -763,19 +573,6 @@ private String createCsvFile( String... lines ) throws IOException return file.toURI().toURL().toString(); } - @Test - public void shouldReturnNodeListTypedAsNodeList() - { - // When - Result res = db.execute( - "WITH org.neo4j.procedure.nodeList() AS nodes RETURN extract( x IN nodes | id(x) ) as ids" ); - - // Then - assertTrue( res.hasNext() ); - assertThat( ((List) res.next().get( "ids" )).size(), equalTo( 2 ) ); - assertFalse( res.hasNext() ); - } - @Test public void shouldHandleAggregationFunctionInFunctionCall() { @@ -826,40 +623,6 @@ public void shouldNotAllowReadFunctionInNoneTransaction() throws Throwable } } - @Test - public void shouldNotAllowWriteFunctionInReadOnlyTransaction() throws Throwable - { - // Expect - exception.expect( AuthorizationViolationException.class ); - exception.expectMessage( "Write operations are not allowed" ); - - GraphDatabaseAPI gdapi = (GraphDatabaseAPI) db; - - // When - try ( Transaction tx = gdapi.beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.READ ) ) - { - db.execute( "RETURN org.neo4j.procedure.writingFunction()" ).next(); - tx.success(); - } - } - - @Test - public void shouldNotAllowSchemaWriteFunctionInWriteTransaction() throws Throwable - { - // Expect - exception.expect( AuthorizationViolationException.class ); - exception.expectMessage( "Schema operations are not allowed" ); - - GraphDatabaseAPI gdapi = (GraphDatabaseAPI) db; - - // When - try ( Transaction tx = gdapi.beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.WRITE ) ) - { - db.execute( "RETURN org.neo4j.procedure.schemaFunction()" ).next(); - tx.success(); - } - } - @Before public void setUp() throws IOException { @@ -956,16 +719,6 @@ public double squareDouble( double value ) return value * value; } - @Function( mode = WRITE ) - public List nodeList() - { - List nodesList = new ArrayList<>(); - nodesList.add( db.createNode() ); - nodesList.add( db.createNode() ); - - return nodesList; - } - @Function public double avgNumberList( List list ) { @@ -1023,20 +776,6 @@ public Node readOnlyTryingToWrite() return db.createNode(); } - @Function( mode = WRITE ) - public Node writingFunction() - { - return db.createNode(); - } - - @Function( mode = WRITE ) - public Node createNode(String value ) - { - Node node = db.createNode(); - node.setProperty( "prop", value ); - return node; - } - @Function public Node readOnlyCallingWriteFunction() { @@ -1056,32 +795,6 @@ public void writingProcedure() db.createNode(); } - @Function( mode = WRITE ) - public Node writeFunctionCallingWriteFunction() - { - return (Node) db.execute( "RETURN org.neo4j.procedure.writingFunction() AS node" ).next().get("node"); - } - - @Function( mode = WRITE ) - public long writeFunctionCallingWriteProcedure() - { - db.execute( "CALL org.neo4j.procedure.writingProcedure()" ); - return 1337L; - } - - @Function( mode = WRITE ) - public long writeFunctionCallingReadFunction() - { - return (long) db.execute( "RETURN org.neo4j.procedure.integrationTestMe() AS result" ).next() - .get( "result" ); - } - - @Function( mode = WRITE ) - public String writeFunctionCallingSchemaFunction() - { - return (String) db.execute( "RETURN org.neo4j.procedure.schemaFunction() AS result" ).next().get("result"); - } - @Function public String shutdown() { @@ -1089,7 +802,7 @@ public String shutdown() return "oh no!"; } - @Function( mode = WRITE ) + @Function public String unsupportedFunction() { jobs.submit( () -> { @@ -1117,7 +830,7 @@ public Path nodePaths( Node node ) } @Description( "This is a description" ) - @Function( mode = WRITE ) + @Function public Node nodeWithDescription( Node node ) { return node; @@ -1129,33 +842,6 @@ public String readOnlyTryingToWriteSchema() db.execute( "CREATE CONSTRAINT ON (book:Book) ASSERT book.isbn IS UNIQUE" ); return "done"; } - - @Function( mode = WRITE ) - public String readWriteTryingToWriteSchema() - { - db.execute( "CREATE CONSTRAINT ON (book:Book) ASSERT book.isbn IS UNIQUE" ); - return "done"; - } - - @Function( mode = SCHEMA ) - public String schemaFunction() - { - db.execute( "CREATE CONSTRAINT ON (book:Book) ASSERT book.isbn IS UNIQUE" ); - return "done"; - } - - @Function( mode = SCHEMA ) - public Node schemaCallReadFunction( long id ) - { - return (Node) db.execute( "RETURN org.neo4j.procedure.node(" + id + ") AS node" ).next().get( "node" ); - } - - @Function( mode = SCHEMA ) - public Node schemaTryingToWrite() - { - db.execute( "CREATE CONSTRAINT ON (book:Book) ASSERT book.isbn IS UNIQUE" ); - return db.createNode(); - } } private static final ScheduledExecutorService jobs = Executors.newScheduledThreadPool( 5 );