[coZSswV2] Fixes triggers (#3369)

* [coZSswV2] Fix typo in triggers error message * [coZSswV2] Assert new triggers can't add TransactionEventListeners to "system" * [coZSswV2] Assert old triggers can't add TransactionEventListeners to "system"
neo4j · Dec 16, 2022 · 0284596 · 0284596
1 parent cbb1cac
commit 0284596
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 16 deletions.
diff --git a/core/src/main/java/apoc/trigger/TriggerNewProcedures.java b/core/src/main/java/apoc/trigger/TriggerNewProcedures.java
@@ -22,6 +22,7 @@
 public class TriggerNewProcedures {
     // public for testing purpose
     public static final String TRIGGER_NOT_ROUTED_ERROR = "The procedure should be routed and executed against a writer system database";
+    public static final String TRIGGER_BAD_TARGET_ERROR = "Triggers can only be installed on user databases.";
 
     public static class TriggerInfo {
         public String name;
@@ -68,6 +69,12 @@ private void checkInSystemWriter() {
         }
     }
 
+    private void checkTargetDatabase(String databaseName) {
+        if (databaseName.equals(SYSTEM_DATABASE_NAME)) {
+            throw new RuntimeException(TRIGGER_BAD_TARGET_ERROR);
+        }
+    }
+
     public TriggerInfo toTriggerInfo(Map.Entry<String, Object> e) {
         String name = e.getKey();
         if (e.getValue() instanceof Map) {
@@ -85,9 +92,9 @@ public TriggerInfo toTriggerInfo(Map.Entry<String, Object> e) {
     @SystemProcedure
     @Procedure(mode = Mode.WRITE)
     @Description("CALL apoc.trigger.install(databaseName, name, statement, selector, config) | eventually adds a trigger for a given database which is invoked when a successful transaction occurs.")
-    public Stream<TriggerInfo> install(@Name("databaseName") String databaseName, @Name("name") String name, @Name("kernelTransaction") String statement, @Name(value = "selector")  Map<String,Object> selector, @Name(value = "config", defaultValue = "{}") Map<String,Object> config) {
+    public Stream<TriggerInfo> install(@Name("databaseName") String databaseName, @Name("name") String name, @Name("statement") String statement, @Name(value = "selector")  Map<String,Object> selector, @Name(value = "config", defaultValue = "{}") Map<String,Object> config) {
         checkInSystemWriter();
-
+        checkTargetDatabase(databaseName);
         Map<String,Object> params = (Map)config.getOrDefault("params", Collections.emptyMap());
         Map<String, Object> removed = TriggerHandlerNewProcedures.install(databaseName, name, statement, selector, params);
         if (removed.containsKey(SystemPropertyKeys.statement.name())) {

diff --git a/core/src/test/java/apoc/trigger/TriggerNewProceduresTest.java b/core/src/test/java/apoc/trigger/TriggerNewProceduresTest.java
@@ -29,20 +29,13 @@
 import java.util.concurrent.TimeUnit;
 
 import static apoc.ApocConfig.SUN_JAVA_COMMAND;
+import static apoc.trigger.TriggerNewProcedures.TRIGGER_BAD_TARGET_ERROR;
 import static apoc.trigger.TriggerNewProcedures.TRIGGER_NOT_ROUTED_ERROR;
 import static apoc.trigger.TriggerTestUtil.TIMEOUT;
 import static apoc.trigger.TriggerTestUtil.TRIGGER_DEFAULT_REFRESH;
 import static apoc.trigger.TriggerTestUtil.awaitTriggerDiscovered;
-import static apoc.util.TestUtil.testCall;
-import static apoc.util.TestUtil.testCallCount;
-import static apoc.util.TestUtil.testCallCountEventually;
-import static apoc.util.TestUtil.testCallEventually;
-import static apoc.util.TestUtil.testResult;
-import static apoc.util.TestUtil.waitDbsAvailable;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static apoc.util.TestUtil.*;
+import static org.junit.Assert.*;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.neo4j.configuration.GraphDatabaseSettings.procedure_unrestricted;
 import static org.neo4j.internal.helpers.collection.MapUtil.map;
@@ -548,6 +541,15 @@ public void testInstallTriggerInUserDb() {
         }
     }
 
+    @Test
+    public void testInstallTriggerInSystemDb() {
+        try {
+            testCall(sysDb, "CALL apoc.trigger.install('system', 'name', 'SHOW DATABASES', {})", r -> fail(""));
+        } catch (RuntimeException e) {
+            assertTrue(e.getMessage().contains(TRIGGER_BAD_TARGET_ERROR));
+        }
+    }
+
     @Test
     public void testEventualConsistency() {
         long count = 0L;

diff --git a/core/src/test/java/apoc/trigger/TriggerTest.java b/core/src/test/java/apoc/trigger/TriggerTest.java
@@ -23,10 +23,9 @@
 import static apoc.ApocConfig.APOC_TRIGGER_ENABLED;
 import static apoc.ApocConfig.apocConfig;
 import static apoc.util.MapUtil.map;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
+import static apoc.util.TestUtil.testCall;
+import static org.junit.Assert.*;
+import static org.junit.jupiter.api.Assertions.fail;
 import static org.neo4j.configuration.GraphDatabaseSettings.procedure_unrestricted;
 
 /**
@@ -48,6 +47,18 @@ public void setUp() {
         apocConfig().setProperty(APOC_TRIGGER_ENABLED, true);
     }
 
+    @Test
+    public void testInstallTriggerInSystemDb() {
+        // Can't add triggers for system because apoc.trigger.add is does not have the @System annotation
+        try {
+            final var system = db.getManagementService().database("system");
+            testCall(system, "CALL apoc.trigger.add('name', 'SHOW DATABASES', {})", r -> fail(""));
+        } catch (RuntimeException e) {
+            assertTrue(e.getMessage().contains("Not a recognised system command or procedure"));
+        }
+    }
+
+
     @Test
     public void testListTriggers() {
         String query = "MATCH (c:Counter) SET c.count = c.count + size([f IN $deletedNodes WHERE id(f) > 0])";