Fix broken tests

neo4j-contrib · Jan 22, 2025 · 84a7b58 · 84a7b58
1 parent a45c39b
commit 84a7b58
Show file tree

Hide file tree

Showing 8 changed files with 141 additions and 145 deletions.
diff --git a/extended/src/main/java/apoc/ExtendedApocConfig.java b/extended/src/main/java/apoc/ExtendedApocConfig.java
@@ -1,14 +1,13 @@
 package apoc;
 
-import static apoc.ApocConfig.SUN_JAVA_COMMAND;
+import static org.neo4j.configuration.GraphDatabaseSettings.configuration_directory;
 
 import apoc.util.SimpleRateLimiter;
 
 import java.io.File;
 import java.time.Duration;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.stream.Stream;
 
 import org.apache.commons.configuration2.CombinedConfiguration;
 import org.apache.commons.configuration2.Configuration;
@@ -19,6 +18,7 @@
 import org.apache.commons.configuration2.ex.ConversionException;
 import org.apache.commons.configuration2.io.FileHandler;
 import org.apache.commons.configuration2.tree.OverrideCombiner;
+import org.neo4j.configuration.Config;
 import org.neo4j.kernel.api.procedure.GlobalProcedures;
 import org.neo4j.kernel.lifecycle.LifecycleAdapter;
 import org.neo4j.logging.Log;
@@ -60,6 +60,7 @@ public enum UuidFormatType { hex, base64 }
     private final String defaultConfigPath;
 
     private Configuration config;
+    private Config neo4jConfig;
 
     private static ExtendedApocConfig theInstance;
 
@@ -73,7 +74,12 @@ public enum UuidFormatType { hex, base64 }
 
     public static final String CONFIG_DIR = "config-dir=";
 
-    public ExtendedApocConfig(LogService log, GlobalProcedures globalProceduresRegistry, String defaultConfigPath) {
+    public ExtendedApocConfig(
+            Config neo4jConfig,
+            LogService log, 
+            GlobalProcedures globalProceduresRegistry, 
+            String defaultConfigPath) {
+        this.neo4jConfig = neo4jConfig;
         this.log = log.getInternalLog(ApocConfig.class);
         this.defaultConfigPath = defaultConfigPath;
         theInstance = this;
@@ -100,26 +106,7 @@ public boolean isInitialized() {
     }
 
     protected String determineNeo4jConfFolder() {
-        String command = System.getProperty(SUN_JAVA_COMMAND);
-        if (command == null) {
-            log.warn(
-                    "system property %s is not set, assuming %s as conf dir. This might cause `apoc.conf` not getting loaded.",
-                    SUN_JAVA_COMMAND, defaultConfigPath);
-            return defaultConfigPath;
-        } else {
-            final String neo4jConfFolder = Stream.of(command.split("--"))
-                    .map(String::trim)
-                    .filter(s -> s.startsWith(CONFIG_DIR))
-                    .map(s -> s.substring(CONFIG_DIR.length()))
-                    .findFirst()
-                    .orElse(defaultConfigPath);
-            if (defaultConfigPath.equals(neo4jConfFolder)) {
-                log.info("cannot determine conf folder from sys property %s, assuming %s", command, defaultConfigPath);
-            } else {
-                log.info("from system properties: NEO4J_CONF=%s", neo4jConfFolder);
-            }
-            return neo4jConfFolder;
-        }
+        return neo4jConfig.get(configuration_directory).toString();
     }
 
     /**

diff --git a/extended/src/main/java/apoc/ExtendedApocConfigExtensionFactory.java b/extended/src/main/java/apoc/ExtendedApocConfigExtensionFactory.java
@@ -32,7 +32,11 @@ public Lifecycle newInstance( ExtensionContext context, Dependencies dependencie
                 .get(neo4j_home)
                 .resolve(Config.DEFAULT_CONFIG_DIR_NAME)
                 .toString();
-        return new ExtendedApocConfig(dependencies.log(), dependencies.globalProceduresRegistry(), defaultConfigPath);
+        return new ExtendedApocConfig(
+                dependencies.config(),
+                dependencies.log(), 
+                dependencies.globalProceduresRegistry(), 
+                defaultConfigPath);
 
     }
 }
diff --git a/extended/src/test/java/apoc/ExtendedApocConfigTest.java b/extended/src/test/java/apoc/ExtendedApocConfigTest.java
@@ -1,12 +1,19 @@
 package apoc;
 
-import static apoc.ApocConfig.SUN_JAVA_COMMAND;
 import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.io.File;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.util.Collections;
+
 import org.junit.Before;
 import org.junit.Test;
+import org.neo4j.configuration.Config;
+import org.neo4j.configuration.GraphDatabaseSettings;
 import org.neo4j.logging.AssertableLogProvider;
 import org.neo4j.logging.InternalLogProvider;
 import org.neo4j.logging.internal.SimpleLogService;
@@ -15,40 +22,37 @@
 public class ExtendedApocConfigTest {
 
     private ExtendedApocConfig cut;
+    private File apocConfigFile;
 
     @Before
-    public void setup() {
+    public void setup() throws URISyntaxException {
+        apocConfigFile =
+                new File(getClass().getClassLoader().getResource("apoc.conf").toURI());
+
+        Config neo4jConfig = mock(Config.class);
+        when(neo4jConfig.getDeclaredSettings()).thenReturn(Collections.emptyMap());
+        when(neo4jConfig.get(any())).thenReturn(null);
+        when(neo4jConfig.get(GraphDatabaseSettings.allow_file_urls)).thenReturn(false);
+        when(neo4jConfig.get(GraphDatabaseSettings.configuration_directory))
+                .thenReturn(Path.of(apocConfigFile.getParent()));
+
         InternalLogProvider logProvider = new AssertableLogProvider();
         GlobalProceduresRegistry registry = mock(GlobalProceduresRegistry.class);
-        cut = new ExtendedApocConfig(new SimpleLogService(logProvider), registry, "C:/neo4j/neo4j-enterprise-5.x.0/conf");
+        cut = new ExtendedApocConfig(
+                neo4jConfig,
+                new SimpleLogService(logProvider), 
+                registry, 
+                "C:/neo4j/neo4j-enterprise-5.x.0/conf");
     }
 
     @Test
     public void testDetermineNeo4jConfFolderDefault() {
-        System.setProperty(SUN_JAVA_COMMAND, "");
-        assertEquals("C:/neo4j/neo4j-enterprise-5.x.0/conf", cut.determineNeo4jConfFolder());
-    }
-
-    @Test
-    public void testDetermineNeo4jConfFolder() {
-        System.setProperty(SUN_JAVA_COMMAND, "com.neo4j.server.enterprise.CommercialEntryPoint --home-dir=/home/stefan/neo4j-enterprise-4.0.0-alpha09mr02 --config-dir=/home/stefan/neo4j-enterprise-4.0.0-alpha09mr02/conf");
-
-        assertEquals("/home/stefan/neo4j-enterprise-4.0.0-alpha09mr02/conf", cut.determineNeo4jConfFolder());
+        assertEquals(apocConfigFile.getParent(), cut.determineNeo4jConfFolder());
     }
-
     @Test
-    public void testApocConfFileBeingLoaded() throws Exception {
-        String confDir = new File(getClass().getClassLoader().getResource("apoc.conf").toURI()).getParent();
-        System.setProperty(SUN_JAVA_COMMAND, "com.neo4j.server.enterprise.CommercialEntryPoint --home-dir=/home/stefan/neo4j-enterprise-4.0.0-alpha09mr02 --config-dir=" + confDir);
+    public void testApocConfFileBeingLoaded() {
         cut.init();
 
         assertEquals("bar", cut.getConfig().getString("foo"));
     }
-
-    @Test
-    public void testDetermineNeo4jConfFolderWithWhitespaces() {
-        System.setProperty(SUN_JAVA_COMMAND, "com.neo4j.server.enterprise.CommercialEntryPoint --config-dir=/home/stefan/neo4j enterprise-4.0.0-alpha09mr02/conf --home-dir=/home/stefan/neo4j enterprise-4.0.0-alpha09mr02");
-
-        assertEquals("/home/stefan/neo4j enterprise-4.0.0-alpha09mr02/conf", cut.determineNeo4jConfFolder());
-    }
 }
diff --git a/extended/src/test/java/apoc/load/LoadDirectoryTest.java b/extended/src/test/java/apoc/load/LoadDirectoryTest.java
@@ -1,6 +1,6 @@
 package apoc.load;
 
-import apoc.util.DbmsUtil;
+import apoc.util.DbmsTestUtil;
 import apoc.util.TestUtil;
 import apoc.util.collection.Iterators;
 import junit.framework.TestCase;
@@ -20,7 +20,6 @@
 import org.neo4j.graphdb.Label;
 import org.neo4j.graphdb.QueryExecutionException;
 import org.neo4j.graphdb.Transaction;
-import org.neo4j.test.TestDatabaseManagementServiceBuilder;
 
 import java.io.File;
 import java.io.FileWriter;
@@ -51,6 +50,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.neo4j.configuration.GraphDatabaseSettings.load_csv_file_url_root;
 import static org.neo4j.test.assertion.Assert.assertEventually;
 
 public class LoadDirectoryTest {
@@ -87,12 +87,11 @@ public static void setUp() throws Exception {
         importFolder = new File(temporaryFolder.getRoot() + File.separator + IMPORT_DIR);
         importPath = encodePath(FILE_PROTOCOL + importFolder.getPath());
 
-        DbmsUtil.setApocConfigs(temporaryFolder.getRoot(),
+        databaseManagementService = DbmsTestUtil.getDbBuilderWithApocConfigs(temporaryFolder,
                 Map.of(APOC_CONFIG_JOBS_POOL_NUM_THREADS, "10",
-                        APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM,  "true"));
+                        APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM,  "true"))
+                .setConfig(load_csv_file_url_root, importFolder.toPath()).build();
 
-        databaseManagementService = new TestDatabaseManagementServiceBuilder(importFolder.toPath())
-                .build();
         db = databaseManagementService.database(GraphDatabaseSettings.DEFAULT_DATABASE_NAME);
 
         TestUtil.registerProcedure(db, PROCS_TO_REGISTER);
@@ -585,8 +584,7 @@ public void testWithFileProtocolAndRecursiveFalse() {
                     assertTrue(rows.contains(rootTempFolder + File.separator + "Foo.csv"));
                     assertTrue(rows.contains(rootTempFolder + File.separator + "Bar.csv"));
                     assertTrue(rows.contains(rootTempFolder + File.separator + "Baz.xls"));
-                    assertTrue(rows.contains(rootTempFolder + File.separator + "apoc.conf"));
-                    assertEquals(4, rows.size());
+                    assertEquals(3, rows.size());
                 }
         );
     }
@@ -669,14 +667,12 @@ public void testLoadDirectoryConcatenatedWithLoadCsv() throws URISyntaxException
         );
     }
 
-    private void restartDb() {
+    private void restartDb() throws IOException {
         databaseManagementService.shutdown();
-
-        DbmsUtil.setApocConfigs(temporaryFolder.getRoot(),
-                Map.of(APOC_CONFIG_JOBS_POOL_NUM_THREADS, "40"));
-
-        databaseManagementService = new TestDatabaseManagementServiceBuilder(importFolder.toPath())
-                .build();
+
+        databaseManagementService = DbmsTestUtil.getDbBuilderWithApocConfigs(temporaryFolder,
+                        Map.of(APOC_CONFIG_JOBS_POOL_NUM_THREADS, "40"))
+                .setConfig(load_csv_file_url_root, importFolder.toPath()).build();
 
         db = databaseManagementService.database(GraphDatabaseSettings.DEFAULT_DATABASE_NAME);
         assertTrue(db.isAvailable(1000));

diff --git a/extended/src/test/java/apoc/ttl/TTLTest.java b/extended/src/test/java/apoc/ttl/TTLTest.java
@@ -5,8 +5,8 @@
 import apoc.util.TestUtil;
 import apoc.util.collection.Iterators;
 
-import java.io.IOException;
 import java.util.Map;
+
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -15,40 +15,47 @@
 import java.util.concurrent.TimeUnit;
 
 import org.junit.rules.TemporaryFolder;
+import org.neo4j.configuration.GraphDatabaseSettings;
+import org.neo4j.dbms.api.DatabaseManagementService;
+import org.neo4j.graphdb.GraphDatabaseService;
 import org.neo4j.graphdb.Label;
 import org.neo4j.graphdb.Transaction;
-import org.neo4j.test.rule.DbmsRule;
-import org.neo4j.test.rule.ImpermanentDbmsRule;
 
 import static apoc.ExtendedApocConfig.APOC_TTL_ENABLED;
 import static apoc.ExtendedApocConfig.APOC_TTL_SCHEDULE;
+import static apoc.util.TestUtil.waitDbsAvailable;
 import static org.junit.Assert.assertTrue;
 
 public class TTLTest {
-    @ClassRule
-    public static TemporaryFolder temporaryFolder = new TemporaryFolder();
 
     @ClassRule
-    public static DbmsRule db = new ImpermanentDbmsRule();
+    public static TemporaryFolder storeDir = new TemporaryFolder();
+
+    private static GraphDatabaseService db;
+    private static DatabaseManagementService databaseManagementService;
 
     @BeforeClass
-    public static void beforeClass() throws IOException {
-        DbmsTestUtil.startDbWithApocConfigs(temporaryFolder,
+    public static void beforeClass() throws Exception {
+        databaseManagementService = DbmsTestUtil.startDbWithApocConfigs(storeDir,
                 Map.of(APOC_TTL_ENABLED, "true",
                         APOC_TTL_SCHEDULE, "3")
         );
+
+        db = databaseManagementService.database(GraphDatabaseSettings.DEFAULT_DATABASE_NAME);
+        waitDbsAvailable(db);
+        TestUtil.registerProcedure(db, TTL.class, Periodic.class);
     }
 
     @AfterClass
-    public static void tearDown() {
-        db.shutdown();
+    public static void afterClass() {
+        databaseManagementService.shutdown();
     }
 
+
     @Test
-    public void testExpireManyNodes() throws Exception {
+    public void testExpireManyNodes() {
         int fooCount = 200;
         int barCount = 300;
-        restartAndRegister(db);
         db.executeTransactionally("UNWIND range(1," + fooCount + ") as range CREATE (:Baz)-[:REL_TEST]->(n:Foo:TTL {id: range, ttl: timestamp() + 100});");
         db.executeTransactionally("UNWIND range(1," + barCount + ") as range CREATE (n:Bar:TTL {id: range, ttl: timestamp() + 100});");
         assertTrue(isNodeCountConsistent(fooCount, barCount));
@@ -57,8 +64,7 @@ public void testExpireManyNodes() throws Exception {
 
     // test extracted from apoc.date
     @Test
-    public void testExpire() throws Exception {
-        restartAndRegister(db);
+    public void testExpire() {
         db.executeTransactionally("CREATE (n:Foo:TTL) SET n.ttl = timestamp() + 100");
         db.executeTransactionally("CREATE (n:Bar) WITH n CALL apoc.ttl.expireIn(n,500,'ms') RETURN count(*)");
         assertTrue(isNodeCountConsistent(1,1));
@@ -74,9 +80,4 @@ private static boolean isNodeCountConsistent(int foo, int bar) {
             return isNotCountConsistent;
         }
     }
-
-    private static void restartAndRegister(DbmsRule db) throws Exception {
-        db.restartDatabase(Map.of());
-        TestUtil.registerProcedure(db, TTL.class, Periodic.class);
-    }
 }
diff --git a/extended/src/test/java/apoc/util/DbmsTestUtil.java b/extended/src/test/java/apoc/util/DbmsTestUtil.java
@@ -11,13 +11,15 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import static apoc.ApocConfig.SUN_JAVA_COMMAND;
 import static org.neo4j.configuration.GraphDatabaseSettings.procedure_unrestricted;
 
 public class DbmsTestUtil {
 
-    public static DatabaseManagementService startDbWithApocConfigs(TemporaryFolder storeDir, Map<String, Object> configMap) throws IOException {
-        final File configFile = storeDir.newFile("apoc.conf");
+    public static DatabaseManagementService startDbWithApocConfigs(
+            TemporaryFolder storeDir, 
+            Map<String, Object> configMap) throws IOException {
+        storeDir.newFolder("conf"); // add the conf folder
+        final File configFile = storeDir.newFile("conf/apoc.conf");
         try (FileWriter writer = new FileWriter(configFile)) {
             // `key=value` lines in apoc.conf file
             String confString = configMap.entrySet()
@@ -27,10 +29,32 @@ public static DatabaseManagementService startDbWithApocConfigs(TemporaryFolder s
 
             writer.write(confString);
         }
-        System.setProperty(SUN_JAVA_COMMAND, "config-dir=" + storeDir.getRoot().getAbsolutePath());
 
         return new TestDatabaseManagementServiceBuilder(storeDir.getRoot().toPath())
                 .setConfig(procedure_unrestricted, List.of("apoc*"))
                 .build();
     }
+
+    public static TestDatabaseManagementServiceBuilder getDbBuilderWithApocConfigs(
+            TemporaryFolder storeDir, 
+            Map<String, Object> configMap) throws IOException {
+        try {
+            storeDir.newFolder("conf"); // add the conf folder
+        } catch (IOException e) {
+            // ignore - already exists
+        }
+        final File configFile = new File(storeDir.getRoot(),"conf/apoc.conf");
+        try (FileWriter writer = new FileWriter(configFile)) {
+            // `key=value` lines in apoc.conf file
+            String confString = configMap.entrySet()
+                    .stream()
+                    .map(e -> e.getKey() + "=" + e.getValue())
+                    .collect(Collectors.joining("\n"));
+
+            writer.write(confString);
+        }
+
+        return new TestDatabaseManagementServiceBuilder(storeDir.getRoot().toPath())
+                .setConfig(procedure_unrestricted, List.of("apoc*"));
+    }
 }