[Ci2sJ23z] apoc.trigger.show doesn't work in a system follower (neo4j…

…-contrib#292)
ncordon · Feb 9, 2023 · 9b808a7 · 9b808a7
1 parent d0a5987
commit 9b808a7
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 10 deletions.
diff --git 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,15 +22,14 @@
 
 
 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.";
     public static final String DB_NOT_FOUND_ERROR = "The user database with name '%s' does not exist";
 
     @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<String> databases = tx.execute("SHOW DATABASES", Collections.emptyMap())
@@ -124,7 +130,7 @@ public Stream<TriggerInfo> 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<TriggerInfo> 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
@@ -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<Session, String> 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<Session, String> 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<Session, String> testTrigger) {
         final List<Neo4jContainerExtension> 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
@@ -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
@@ -220,6 +220,10 @@ public static void testResult(Session session, String call, Map<String,Object> p
         });
     }
 
+    public static void testCallEmpty(Session session, String call, Map<String,Object> params) {
+        testResult(session, call, params, (res) -> assertFalse("Expected no results", res.hasNext()) );
+    }
+
     public static void testCallInReadTransaction(Session session, String call, Map<String,Object> params, Consumer<Map<String, Object>> consumer) {
         testResultInReadTransaction(session, call, params, (res) -> {
             try {