From 9b808a787d9267834e5ec081bcf0b7c9267b4d85 Mon Sep 17 00:00:00 2001 From: Giuseppe Villani Date: Thu, 9 Feb 2023 09:53:52 +0100 Subject: [PATCH] [Ci2sJ23z] apoc.trigger.show doesn't work in a system follower (#292) --- .../apoc/trigger/TriggerNewProcedures.java | 14 +++++++--- .../trigger/TriggerClusterRoutingTest.java | 28 +++++++++++++++---- .../trigger/TriggerNewProceduresTest.java | 11 ++++++++ .../java/apoc/util/TestContainerUtil.java | 4 +++ 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/apoc/trigger/TriggerNewProcedures.java b/core/src/main/java/apoc/trigger/TriggerNewProcedures.java index dadcecfb4a..8d46aea23e 100644 --- a/core/src/main/java/apoc/trigger/TriggerNewProcedures.java +++ b/core/src/main/java/apoc/trigger/TriggerNewProcedures.java @@ -4,7 +4,6 @@ import org.neo4j.graphdb.Transaction; import org.neo4j.kernel.api.procedure.SystemProcedure; import org.neo4j.kernel.internal.GraphDatabaseAPI; -import org.neo4j.logging.Log; import org.neo4j.procedure.Admin; import org.neo4j.procedure.Context; import org.neo4j.procedure.Description; @@ -23,6 +22,7 @@ public class TriggerNewProcedures { + public static final String NON_SYS_DB_ERROR = "The procedure should be executed against a system database."; public static final String TRIGGER_NOT_ROUTED_ERROR = "No write operations are allowed directly on this database. " + "Writes must pass through the leader. The role of this server is: FOLLOWER"; public static final String TRIGGER_BAD_TARGET_ERROR = "Triggers can only be installed on user databases."; @@ -30,8 +30,6 @@ public class TriggerNewProcedures { @Context public GraphDatabaseAPI db; - @Context public Log log; - @Context public Transaction tx; private void checkInSystemWriter() { @@ -41,6 +39,14 @@ private void checkInSystemWriter() { throw new RuntimeException(TRIGGER_NOT_ROUTED_ERROR); } } + + private void checkInSystem() { + TriggerHandlerNewProcedures.checkEnabled(); + + if (!db.databaseName().equals(SYSTEM_DATABASE_NAME)) { + throw new RuntimeException(NON_SYS_DB_ERROR); + } + } private void checkTargetDatabase(String databaseName) { final Set databases = tx.execute("SHOW DATABASES", Collections.emptyMap()) @@ -124,7 +130,7 @@ public Stream start(@Name("databaseName") String databaseName, @Nam @Procedure(name = "apoc.trigger.show", mode = Mode.READ) @Description("Lists all eventually installed triggers for a database.") public Stream show(@Name("databaseName") String databaseName) { - checkInSystemWriter(); + checkInSystem(); return TriggerHandlerNewProcedures.getTriggerNodesList(databaseName, tx); } diff --git a/core/src/test/java/apoc/trigger/TriggerClusterRoutingTest.java b/core/src/test/java/apoc/trigger/TriggerClusterRoutingTest.java index fa89116f82..08de5d8110 100644 --- a/core/src/test/java/apoc/trigger/TriggerClusterRoutingTest.java +++ b/core/src/test/java/apoc/trigger/TriggerClusterRoutingTest.java @@ -14,10 +14,12 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.function.BiConsumer; import static apoc.trigger.Trigger.SYS_DB_NON_WRITER_ERROR; import static apoc.trigger.TriggerNewProcedures.TRIGGER_NOT_ROUTED_ERROR; import static apoc.util.TestContainerUtil.testCall; +import static apoc.util.TestContainerUtil.testCallEmpty; import static org.junit.Assert.assertEquals; import static org.neo4j.configuration.GraphDatabaseSettings.DEFAULT_DATABASE_NAME; import static org.neo4j.configuration.GraphDatabaseSettings.SYSTEM_DATABASE_NAME; @@ -71,22 +73,36 @@ public void testTriggerDropAllowedOnlyInSysLeaderMember() { triggerInSysLeaderMemberCommon(query, TRIGGER_NOT_ROUTED_ERROR, SYSTEM_DATABASE_NAME); } + @Test + public void testTriggerShowAllowedOnlyInSysLeaderMember() { + final String query = "CALL apoc.trigger.show('neo4j')"; + final BiConsumer testTrigger = (session, name) -> testCallEmpty(session, query, Collections.emptyMap()); + triggerInSysLeaderMemberCommon(query, TRIGGER_NOT_ROUTED_ERROR, SYSTEM_DATABASE_NAME, true, testTrigger); + } + private static void triggerInSysLeaderMemberCommon(String query, String triggerNotRoutedError, String dbName) { + final BiConsumer testTrigger = (session, name) -> testCall(session, query, + Map.of("name", name), + row -> assertEquals(name, row.get("name"))); + triggerInSysLeaderMemberCommon(query, triggerNotRoutedError, dbName, false, testTrigger); + } + + private static void triggerInSysLeaderMemberCommon(String query, String triggerNotRoutedError, String dbName, boolean nonWriteOperation, BiConsumer testTrigger) { final List members = cluster.getClusterMembers(); assertEquals(4, members.size()); for (Neo4jContainerExtension container: members) { - // we skip READ_REPLICA members - final Driver driver = getDriverIfNotReplica(container); + // we skip READ_REPLICA members with write operations + final Driver driver = nonWriteOperation + ? container.getDriver() + : getDriverIfNotReplica(container); if (driver == null) { continue; } Session session = driver.session(SessionConfig.forDatabase(dbName)); final String address = container.getEnvMap().get("NEO4J_dbms_connector_bolt_advertised__address"); - if (dbIsWriter(session, SYSTEM_DATABASE_NAME, address)) { + if (nonWriteOperation || dbIsWriter(session, SYSTEM_DATABASE_NAME, address)) { final String name = UUID.randomUUID().toString(); - testCall( session, query, - Map.of("name", name), - row -> assertEquals(name, row.get("name")) ); + testTrigger.accept(session, name); } else { try { testCall(session, query, diff --git a/core/src/test/java/apoc/trigger/TriggerNewProceduresTest.java b/core/src/test/java/apoc/trigger/TriggerNewProceduresTest.java index e15f1b42ad..4c33d60b35 100644 --- a/core/src/test/java/apoc/trigger/TriggerNewProceduresTest.java +++ b/core/src/test/java/apoc/trigger/TriggerNewProceduresTest.java @@ -31,6 +31,7 @@ import static apoc.ApocConfig.SUN_JAVA_COMMAND; import static apoc.trigger.TriggerNewProcedures.DB_NOT_FOUND_ERROR; import static apoc.trigger.TriggerNewProcedures.TRIGGER_BAD_TARGET_ERROR; +import static apoc.trigger.TriggerNewProcedures.NON_SYS_DB_ERROR; import static apoc.trigger.TriggerNewProcedures.TRIGGER_NOT_ROUTED_ERROR; import static apoc.trigger.TriggerTestUtil.TIMEOUT; import static apoc.trigger.TriggerTestUtil.TRIGGER_DEFAULT_REFRESH; @@ -634,6 +635,16 @@ public void testInstallTriggerInUserDb() { assertTrue(e.getMessage().contains(TRIGGER_NOT_ROUTED_ERROR)); } } + + @Test + public void testShowTriggerInUserDb() { + try { + testCall(db, "CALL apoc.trigger.show('neo4j')", + r -> fail("Should fail because of user db execution")); + } catch (QueryExecutionException e) { + assertTrue(e.getMessage().contains(NON_SYS_DB_ERROR)); + } + } @Test public void testInstallTriggerInNotExistentDb() { diff --git a/test-utils/src/main/java/apoc/util/TestContainerUtil.java b/test-utils/src/main/java/apoc/util/TestContainerUtil.java index 1345f299ea..0e7107ac68 100644 --- a/test-utils/src/main/java/apoc/util/TestContainerUtil.java +++ b/test-utils/src/main/java/apoc/util/TestContainerUtil.java @@ -220,6 +220,10 @@ public static void testResult(Session session, String call, Map p }); } + public static void testCallEmpty(Session session, String call, Map params) { + testResult(session, call, params, (res) -> assertFalse("Expected no results", res.hasNext()) ); + } + public static void testCallInReadTransaction(Session session, String call, Map params, Consumer> consumer) { testResultInReadTransaction(session, call, params, (res) -> { try {