From cacdf54064f8ecf28d5bd443c91fcf4413f1ee2b Mon Sep 17 00:00:00 2001 From: Daniel Leaver Date: Fri, 16 Dec 2022 10:05:43 +0100 Subject: [PATCH] [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" --- .../apoc/trigger/TriggerNewProcedures.java | 11 ++++++++-- .../trigger/TriggerNewProceduresTest.java | 22 ++++++++++--------- .../test/java/apoc/trigger/TriggerTest.java | 19 ++++++++++++---- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/apoc/trigger/TriggerNewProcedures.java b/core/src/main/java/apoc/trigger/TriggerNewProcedures.java index b6f3dc0dc..dc80a4625 100644 --- 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 e) { String name = e.getKey(); if (e.getValue() instanceof Map) { @@ -85,9 +92,9 @@ public TriggerInfo toTriggerInfo(Map.Entry 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 install(@Name("databaseName") String databaseName, @Name("name") String name, @Name("kernelTransaction") String statement, @Name(value = "selector") Map selector, @Name(value = "config", defaultValue = "{}") Map config) { + public Stream install(@Name("databaseName") String databaseName, @Name("name") String name, @Name("statement") String statement, @Name(value = "selector") Map selector, @Name(value = "config", defaultValue = "{}") Map config) { checkInSystemWriter(); - + checkTargetDatabase(databaseName); Map params = (Map)config.getOrDefault("params", Collections.emptyMap()); Map 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 index f7a20a518..56b8cee3c 100644 --- 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 index 37a6df4d9..645437044 100644 --- 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])";