[6R2yEhGU] No procedure with name xxxx registered for this database i…

…nstance (#3800) (#3873) * [6R2yEhGU] No procedure with name xxxx registered for this database instance * [6R2yEhGU] added testCallEventually
neo4j-contrib · Jan 12, 2024 · befe7ff · befe7ff
1 parent ad8d265
commit befe7ff
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 3 deletions.
diff --git 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<ProcedureSignature> currentProceduresToRemove = new HashSet<>(registeredProcedureSignatures);
         Set<UserFunctionSignature> currentUserFunctionsToRemove = new HashSet<>(registeredUserFunctionSignatures);
@@ -237,7 +237,7 @@ private <T> T withSystemDb(Function<Transaction, T> 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
@@ -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<String, Object> expected = Map.of("value", 3L);
         assertEquals(expected, map);
     }
+
+    @Test
+    public void testRestoreProcedureWorksCorrectlyWithoutConflicts() {
+        // create a list of ["proc1", "proc2", "proc3" ....] strings
+        List<String> 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<String> 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"))
+                )
+        );
+    }
 }