diff --git a/extended/src/main/java/apoc/custom/CypherProceduresHandler.java b/extended/src/main/java/apoc/custom/CypherProceduresHandler.java index ddc417170a..128b2d1850 100644 --- a/extended/src/main/java/apoc/custom/CypherProceduresHandler.java +++ b/extended/src/main/java/apoc/custom/CypherProceduresHandler.java @@ -206,7 +206,7 @@ private UserFunctionDescriptor userFunctionDescriptor(Node node) { ), statement, forceSingle, mapResult); } - public void restoreProceduresAndFunctions() { + public synchronized void restoreProceduresAndFunctions() { lastUpdate = System.currentTimeMillis(); Set currentProceduresToRemove = new HashSet<>(registeredProcedureSignatures); Set currentUserFunctionsToRemove = new HashSet<>(registeredUserFunctionSignatures); @@ -237,7 +237,7 @@ private T withSystemDb(Function action) { } } - public void storeFunction(UserFunctionSignature signature, String statement, boolean forceSingle, boolean mapResult) { + public synchronized void storeFunction(UserFunctionSignature signature, String statement, boolean forceSingle, boolean mapResult) { withSystemDb(tx -> { Node node = Util.mergeNode(tx, ExtendedSystemLabels.ApocCypherProcedures, ExtendedSystemLabels.Function, Pair.of(SystemPropertyKeys.database.name(), api.databaseName()), @@ -259,7 +259,7 @@ public void storeFunction(UserFunctionSignature signature, String statement, boo }); } - public void storeProcedure(ProcedureSignature signature, String statement) { + public synchronized void storeProcedure(ProcedureSignature signature, String statement) { withSystemDb(tx -> { Node node = Util.mergeNode(tx, ExtendedSystemLabels.ApocCypherProcedures, ExtendedSystemLabels.Procedure, Pair.of(SystemPropertyKeys.database.name(), api.databaseName()), diff --git a/extended/src/test/java/apoc/custom/CypherProceduresStorageTest.java b/extended/src/test/java/apoc/custom/CypherProceduresStorageTest.java index c0076c0aed..72c72ebcc1 100644 --- a/extended/src/test/java/apoc/custom/CypherProceduresStorageTest.java +++ b/extended/src/test/java/apoc/custom/CypherProceduresStorageTest.java @@ -13,6 +13,7 @@ import org.neo4j.dbms.api.DatabaseManagementService; import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.Node; +import org.neo4j.graphdb.Result; import org.neo4j.test.TestDatabaseManagementServiceBuilder; import java.io.File; @@ -21,6 +22,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import static apoc.custom.CypherProceduresHandler.CUSTOM_PROCEDURES_REFRESH; import static apoc.util.DbmsTestUtil.startDbWithApocConfigs; @@ -38,6 +41,7 @@ public class CypherProceduresStorageTest { private final static String QUERY_CREATE = "RETURN $input1 + $input2 as answer"; private final static String QUERY_OVERWRITE = "RETURN $input1 + $input2 + 123 as answer"; + private final int refreshTime = 50; private int greaterThanRefreshTime; @Rule @@ -685,4 +689,95 @@ private static void assertIsMap(Map map) { Map expected = Map.of("value", 3L); assertEquals(expected, map); } + + @Test + public void testRestoreProcedureWorksCorrectlyWithoutConflicts() { + // create a list of ["proc1", "proc2", "proc3" ....] strings + List listProcNames = IntStream.range(0, 200) + .mapToObj(i -> "proc" + i) + .collect(Collectors.toList()); + + // for each element, declare a procedure with that name, + // then call the custom procedure and finally overwrite it + listProcNames.forEach(name -> { + String declareProc = String.format("CALL apoc.custom.declareProcedure('%s() :: (answer::INT)', $query)", name); + + db.executeTransactionally(declareProc, + Map.of("query", "RETURN 42 AS answer"), + Result::resultAsString + ); + + TestUtil.testCall(db, + String.format("call custom.%s", name), + (row) -> assertEquals(42L, row.get("answer")) + ); + + // overwriting + db.executeTransactionally(declareProc, + Map.of("query", "RETURN 1 AS answer"), + Result::resultAsString + ); + }); + + // check that the previous overwrite works correctly + listProcNames.forEach(name -> TestUtil.testCallEventually(db, + String.format("call custom.%s", name), + (row) -> assertEquals(1L, row.get("answer")), + 10) + ); + + // check that everything works correctly after a db restart + restartDb(); + listProcNames.forEach(name -> TestUtil.testCall(db, + String.format("call custom.%s", name), + (row) -> assertEquals(1L, row.get("answer")) + ) + ); + } + + @Test + public void testRestoreFunctionWorksCorrectlyWithoutConflicts() { + // create a list of ["fun1", "fun2", "fun3" ....] strings + List listFunNames = IntStream.range(0, 200) + .mapToObj(i -> "fun" + i) + .collect(Collectors.toList()); + final String funQuery = "return custom.%s() as row"; + + // for each element, declare a function with that name, + // then call the custom function and finally overwrite it + listFunNames.forEach(name -> { + final String declareFunction = String.format("CALL apoc.custom.declareFunction('%s() :: INT', $query)", name); + + db.executeTransactionally(declareFunction, + Map.of("query", "RETURN 42 as answer"), + Result::resultAsString + ); + + TestUtil.testCall(db, + String.format(funQuery, name), + (row) -> assertEquals(42L, row.get("row")) + ); + + // overwriting + db.executeTransactionally(declareFunction, + Map.of("query", "RETURN 1 as answer"), + Result::resultAsString + ); + }); + + // check that the previous overwrite works correctly + listFunNames.forEach(name -> TestUtil.testCallEventually(db, + String.format(funQuery, name), + (row) -> assertEquals(1L, row.get("row")), + 10) + ); + + // check that everything works correctly after a db restart + restartDb(); + listFunNames.forEach(name -> TestUtil.testCall(db, + String.format(funQuery, name), + (row) -> assertEquals(1L, row.get("row")) + ) + ); + } }