Fix various 5.11 build errors (#3722)

* Fix various 5.11 build errors

* small comment changes

* excluded some hadoop deps to avoid conflict with neo4j 5.11 lib deps

* Fix failing path traversal CI tests

* Add explicit mockito test dependency

* Generalized html with js error - it depends on the driver version

* added parquet procs to extended.txt

* wip - include only StartupExtendedTest

* restored build.gradle

* Added Extended annotation

* Added Extended annotation

* Added Extended annotation

* Updated com.diffplug.spotless version

* restore spotless

* Fix custom proc with homonyms

.github/workflows/CI.yaml

@@ -23,7 +23,7 @@ jobs:
       - name: Set up JDK 17
         uses: actions/setup-java@v2
         with:
-          java-version: '17'
+          java-version: '17.0.8'
           distribution: 'temurin'
 
       - uses: actions/cache@v2

extended/build.gradle

@@ -137,6 +137,7 @@ dependencies {
     testImplementation group: 'com.sun.mail', name: 'javax.mail', version: '1.6.0'
     testImplementation group: 'org.postgresql', name: 'postgresql', version: '42.1.4'
     testImplementation group: 'org.zapodot', name: 'embedded-ldap-junit', version: '0.9.0'
+    testImplementation group: 'org.mockito', name: 'mockito-core', version: '5.4.0'
     testImplementation group: 'org.apache.parquet', name: 'parquet-hadoop', version: '1.13.1', withoutServers
 
 

extended/src/main/java/apoc/custom/CypherProceduresHandler.java

@@ -8,7 +8,6 @@
 import apoc.util.Util;
 import org.apache.commons.lang3.tuple.Pair;
 import org.neo4j.collection.RawIterator;
-import org.neo4j.function.ThrowingFunction;
 import org.neo4j.graphdb.Entity;
 import org.neo4j.graphdb.GraphDatabaseService;
 import org.neo4j.graphdb.Node;
@@ -27,7 +26,6 @@
 import org.neo4j.kernel.api.ResourceMonitor;
 import org.neo4j.kernel.api.procedure.CallableProcedure;
 import org.neo4j.kernel.api.procedure.CallableUserFunction;
-import org.neo4j.kernel.api.procedure.Context;
 import org.neo4j.kernel.api.procedure.GlobalProcedures;
 import org.neo4j.kernel.availability.AvailabilityListener;
 import org.neo4j.kernel.impl.util.ValueUtils;
@@ -36,6 +34,7 @@
 import org.neo4j.logging.Log;
 import org.neo4j.procedure.Mode;
 import org.neo4j.procedure.Name;
+import org.neo4j.procedure.impl.ProcedureHolderUtils;
 import org.neo4j.scheduler.Group;
 import org.neo4j.scheduler.JobHandle;
 import org.neo4j.scheduler.JobScheduler;
@@ -99,7 +98,6 @@ public class CypherProceduresHandler extends LifecycleAdapter implements Availab
     private final GlobalProcedures globalProceduresRegistry;
     private final JobScheduler jobScheduler;
     private long lastUpdate;
-    private final ThrowingFunction<Context, Transaction, ProcedureException> transactionComponentFunction;
     private final Set<ProcedureSignature> registeredProcedureSignatures = Collections.synchronizedSet(new HashSet<>());
     private final Set<UserFunctionSignature> registeredUserFunctionSignatures = Collections.synchronizedSet(new HashSet<>());
     private static Group REFRESH_GROUP = Group.STORAGE_MAINTENANCE;
@@ -112,7 +110,6 @@ public CypherProceduresHandler(GraphDatabaseAPI db, JobScheduler jobScheduler, A
         this.jobScheduler = jobScheduler;
         this.systemDb = apocConfig.getSystemDb();
         this.globalProceduresRegistry = globalProceduresRegistry;
-        transactionComponentFunction = globalProceduresRegistry.lookupComponentProvider(Transaction.class, true);
 
     }
 
@@ -329,18 +326,25 @@ private long getLastUpdate() {
      * @return
      */
     public boolean registerProcedure(ProcedureSignature signature, String statement) {
+        QualifiedName name = signature.name();
         try {
+            boolean exists = globalProceduresRegistry.getCurrentView().getAllProcedures().stream()
+                    .anyMatch(s -> s.name().equals(name));
+            if (exists) {
+                ProcedureHolderUtils.unregisterProcedure(name, globalProceduresRegistry);
+            }
+
             final boolean isStatementNull = statement == null;
             globalProceduresRegistry.register(new CallableProcedure.BasicProcedure(signature) {
                 @Override
                 public RawIterator<AnyValue[], ProcedureException> apply(org.neo4j.kernel.api.procedure.Context ctx, AnyValue[] input, ResourceMonitor resourceMonitor) throws ProcedureException {
                     if (isStatementNull) {
                         final String error = String.format("There is no procedure with the name `%s` registered for this database instance. " +
-                                "Please ensure you've spelled the procedure name correctly and that the procedure is properly deployed.", signature.name());
+                                "Please ensure you've spelled the procedure name correctly and that the procedure is properly deployed.", name);
                         throw new QueryExecutionException(error, null, "Neo.ClientError.Statement.SyntaxError");
                     } else {
                         Map<String, Object> params = params(input, signature.inputSignature(), ctx.valueMapper());
-                        Transaction tx = transactionComponentFunction.apply(ctx);
+                        Transaction tx = ctx.transaction();
                         Result result = tx.execute(statement, params);
                         resourceMonitor.registerCloseableResource(result);
 
@@ -352,33 +356,39 @@ public RawIterator<AnyValue[], ProcedureException> apply(org.neo4j.kernel.api.pr
                         return Iterators.asRawIterator(stream);
                     }
                 }
-            }, true);
+            });
             if (isStatementNull) {
                 registeredProcedureSignatures.remove(signature);
             } else {
                 registeredProcedureSignatures.add(signature);
             }
             return true;
         } catch (Exception e) {
-            log.error("Could not register procedure: " + signature.name() + " with " + statement + "\n accepting" + signature.inputSignature() + " resulting in " + signature.outputSignature() + " mode " + signature.mode(), e);
+            log.error("Could not register procedure: " + name + " with " + statement + "\n accepting" + signature.inputSignature() + " resulting in " + signature.outputSignature() + " mode " + signature.mode(), e);
             return false;
         }
     }
 
-    public boolean registerFunction(UserFunctionSignature signature, String statement, boolean forceSingle) {
+    public boolean registerFunction(UserFunctionSignature signature, String statement, boolean forceSingle/*, boolean override*/) {
         try {
+            QualifiedName name = signature.name();
+            boolean exists = globalProceduresRegistry.getCurrentView().getAllNonAggregatingFunctions()
+                    .anyMatch(s -> s.name().equals(name));
+            if (exists) {
+                ProcedureHolderUtils.unregisterFunction(name, globalProceduresRegistry);
+            }
+
             final boolean isStatementNull = statement == null;
             globalProceduresRegistry.register(new CallableUserFunction.BasicUserFunction(signature) {
                 @Override
                 public AnyValue apply(org.neo4j.kernel.api.procedure.Context ctx, AnyValue[] input) throws ProcedureException {
                     if (isStatementNull) {
-                        final String error = String.format("Unknown function '%s'", signature.name());
+                        final String error = String.format("Unknown function '%s'", name);
                         throw new QueryExecutionException(error, null, "Neo.ClientError.Statement.SyntaxError");
                     } else {
                         Map<String, Object> params = params(input, signature.inputSignature(), ctx.valueMapper());
                         AnyType outType = signature.outputType();
-
-                        Transaction tx = transactionComponentFunction.apply(ctx);
+                        Transaction tx = ctx.transaction();
                         try (Result result = tx.execute(statement, params)) {
 //                resourceTracker.registerCloseableResource(result); // TODO
                             if (!result.hasNext()) return null;
@@ -406,7 +416,7 @@ public AnyValue apply(org.neo4j.kernel.api.procedure.Context ctx, AnyValue[] inp
                     }
 
                 }
-            }, true);
+            });
             if (isStatementNull) {
                 registeredUserFunctionSignatures.remove(signature);
             } else {

extended/src/main/java/apoc/export/parquet/ImportParquet.java

@@ -1,5 +1,6 @@
 package apoc.export.parquet;
 
+import apoc.Extended;
 import apoc.Pools;
 import apoc.export.util.BatchTransaction;
 import apoc.export.util.ProgressReporter;
@@ -33,6 +34,7 @@
 import static apoc.export.parquet.ParquetUtil.FIELD_TARGET_ID;
 import static apoc.export.parquet.ParquetUtil.FIELD_TYPE;
 
+@Extended
 public class ImportParquet {
 
     @Context

extended/src/main/java/apoc/load/LoadParquet.java

@@ -1,5 +1,6 @@
 package apoc.load;
 
+import apoc.Extended;
 import apoc.export.parquet.ApocParquetReader;
 import apoc.export.parquet.ParquetConfig;
 import apoc.result.MapResult;
@@ -20,6 +21,7 @@
 
 import static apoc.export.parquet.ParquetReadUtil.getReader;
 
+@Extended
 public class LoadParquet {
 
     @Context public Log log;

extended/src/main/java/org/neo4j/procedure/impl/ProcedureHolderUtils.java

@@ -0,0 +1,56 @@
+package org.neo4j.procedure.impl;
+
+import org.neo4j.internal.kernel.api.procs.QualifiedName;
+import org.neo4j.kernel.api.procedure.GlobalProcedures;
+
+import java.lang.reflect.Field;
+
+public class ProcedureHolderUtils {
+
+    public static void unregisterProcedure(QualifiedName name, GlobalProcedures registry) {
+        String kind = "procedures";
+        unregisterCommon(name, registry, kind);
+    }
+
+    public static void unregisterFunction(QualifiedName name, GlobalProcedures registry) {
+        String kind = "functions";
+        unregisterCommon(name, registry, kind);
+    }
+
+    private static void unregisterCommon(QualifiedName name, GlobalProcedures registry, String kind) {
+        try {
+            GlobalProceduresRegistry globalProcRegistry = getGlobalProcRegistry(registry);
+
+            // get the field `ProcedureRegistry registry` from the GlobalProceduresRegistry instance
+            Field registryField = GlobalProceduresRegistry.class.getDeclaredField("registry");
+            registryField.setAccessible(true);
+            ProcedureRegistry procedureRegistry = (ProcedureRegistry) registryField.get(globalProcRegistry);
+
+            // get `ProcedureHolder <kind>` (i.e `ProcedureHolder procedures` or `ProcedureHolder functions`) field from the ProcedureRegistry instance
+            Field procHolderField = ProcedureRegistry.class.getDeclaredField(kind);
+            procHolderField.setAccessible(true);
+            ProcedureHolder procedureHolder = (ProcedureHolder) procHolderField.get(procedureRegistry);
+
+            // unregister `name` from ProcedureHolder found
+            procedureHolder.unregister(name);
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private static GlobalProceduresRegistry getGlobalProcRegistry(GlobalProcedures registry) {
+        try {
+            // with embedded test database, the instance is of type LazyProcedures,
+            // so we get the field `globalProcedures` from the `LazyProcedures registry` instance
+            Field globalProceduresField = Class.forName("org.neo4j.procedure.LazyProcedures").getDeclaredField("globalProcedures");
+            globalProceduresField.setAccessible(true);
+
+            return (GlobalProceduresRegistry) globalProceduresField.get(registry);
+
+        } catch (Exception e) {
+            // with a real instance, the above code produces, due to LazyProcedures, produce a `NoClassDefFoundError` or an `IllegalArgumentException`
+            // because `registry` is directly of type GlobalProceduresRegistry, so we cast it
+            return (GlobalProceduresRegistry) registry;
+        }
+    }
+}

extended/src/main/resources/extended.txt

@@ -40,6 +40,14 @@ apoc.es.postRaw
 apoc.es.put
 apoc.es.query
 apoc.es.stats
+apoc.export.parquet.all
+apoc.export.parquet.all.stream
+apoc.export.parquet.data
+apoc.export.parquet.data.stream
+apoc.export.parquet.graph
+apoc.export.parquet.graph.stream
+apoc.export.parquet.query
+apoc.export.parquet.query.stream
 apoc.export.xls.all
 apoc.export.xls.data
 apoc.export.xls.graph
@@ -52,6 +60,7 @@ apoc.generate.ws
 apoc.gephi.add
 apoc.get.nodes
 apoc.get.rels
+apoc.import.parquet
 apoc.load.csv
 apoc.load.csvParams
 apoc.load.directory
@@ -65,6 +74,7 @@ apoc.load.htmlPlainText
 apoc.load.jdbc
 apoc.load.jdbcUpdate
 apoc.load.ldap
+apoc.load.parquet
 apoc.load.xls
 apoc.log.debug
 apoc.log.error

extended/src/test/java/apoc/StartupExtendedTest.java

@@ -43,7 +43,7 @@ public class StartupExtendedTest {
     }
 
     @Test
-    public void checkCoreAndFullWithExtraDependenciesJars() {
+    public void checkCoreAndExtendedWithExtraDependenciesJars() {
         // we check that with apoc-extended, apoc-core jar and all extra-dependencies jars every procedure/function is detected
         startContainerSessionWithExtraDeps((version) -> createDB(version, List.of(CORE, EXTENDED), true),
                 session -> {

extended/src/test/java/apoc/custom/CypherProceduresStorageTest.java

@@ -238,6 +238,54 @@ public void testIssue1744() throws Exception {
         testCallIssue1744();
     }
 
+    @Test
+    public void testMultipleOverrideWithFunctionAndProcedures() throws Exception {
+        db.executeTransactionally("CALL apoc.custom.declareProcedure('override() :: (result::LONG)','RETURN 42 as result')");
+
+        // function homonym to procedure
+        db.executeTransactionally("CALL apoc.custom.declareFunction('override() :: LONG','RETURN 10 as answer')");
+
+        // get fun/proc created
+        TestUtil.testCall(db, "RETURN custom.override() as result", r -> {
+            assertEquals(10L, r.get("result"));
+        });
+        TestUtil.testCall(db, "CALL custom.override()", r -> {
+            assertEquals(42L, r.get("result"));
+        });
+
+        // overrides functions and procedures homonym to the previous ones
+        db.executeTransactionally("CALL apoc.custom.declareFunction('override(input::INT) :: INT', 'RETURN $input + 2 AS result')");
+        db.executeTransactionally("CALL apoc.custom.declareFunction('override(input::INT) :: INT', 'RETURN $input AS result')");
+
+        db.executeTransactionally("CALL apoc.custom.declareProcedure('override(input::INT) :: (result::INT)', 'RETURN $input AS result')");
+        db.executeTransactionally("CALL apoc.custom.declareProcedure('override(input::INT) :: (result::INT)', 'RETURN $input + 2 AS result')");
+
+        // get fun/proc updated
+        TestUtil.testCallEventually(db, "RETURN custom.override(3) as result", r -> {
+            assertEquals(3L, r.get("result"));
+        }, 10L);
+        TestUtil.testCallEventually(db, "CALL custom.override(2)", r -> {
+            assertEquals(4L, r.get("result"));
+        }, 10L);
+        restartDb();
+
+        final String logFileContent = Files.readString(new File(FileUtils.getLogDirectory(), "debug.log").toPath());
+        assertFalse(logFileContent.contains("Could not register function: custom.vantagepoint_within_area"));
+        assertFalse(logFileContent.contains("Could not register procedure: custom.vantagepoint_within_area"));
+
+        // override after restart
+        db.executeTransactionally("CALL apoc.custom.declareFunction('override(input::INT) :: INT', 'RETURN $input + 1 AS result')");
+        db.executeTransactionally("CALL apoc.custom.declareProcedure('override(input::INT) :: (result::INT)', 'RETURN $input + 2 AS result')");
+
+        // get fun/proc updated
+        TestUtil.testCallEventually(db, "RETURN custom.override(3) as result", r -> {
+            assertEquals(4L, r.get("result"));
+        }, 10L);
+        TestUtil.testCallEventually(db, "CALL custom.override(2)", r -> {
+            assertEquals(4L, r.get("result"));
+        }, 10L);
+    }
+
     @Test
     public void functionSignatureShouldNotChangeBeforeAndAfterRestart() {
         functionsCreation();

extended/src/test/java/apoc/export/ExportExtendedSecurityTest.java

@@ -184,13 +184,11 @@ public TestPathTraversalAccess(String exportMethod, String exportMethodType, Str
             this.apocProcedure = "apoc.export." + exportMethod + "." + exportMethodType + "(" + exportMethodArguments + ")";
         }
 
-        private static final String case1 = "'file:///%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2fapoc/test.txt'";
-        private static final String case2 = "'file:///%2e%2e%2f%2ftest.txt'";
-        private static final String case3 = "'../test.txt'";
-        private static final String case4 = "'tests/../../test.txt'";
-        private static final String case5 = "'tests/..//..//test.txt'";
+        private static final String case1 = "'../test.txt'";
+        private static final String case2 = "'tests/../../test.txt'";
+        private static final String case3 = "'tests/..//..//test.txt'";
 
-        private static final List<String> cases = Arrays.asList(case1, case2, case3, case4, case5);
+        private static final List<String> cases = Arrays.asList(case1, case2, case3);
 
         private static final Map<String, List<String>> METHOD_ARGUMENTS = Map.of(
                 "query",  cases.stream().map(
@@ -302,8 +300,10 @@ public TestPathTraversalIsNormalisedWithinDirectory(String exportMethod, String
         private static final String case6 = "'file:///tests//..//test.txt'";
         private static final String case7 = "'test.txt'";
         private static final String case8 = "'file:///..//..//..//..//test.txt'";
+        private static final String case9 = "'file:///%2e%2e%2f%2ftest.txt'";
+        public static final String case10 = "'file:///%2e%2e%2f%2ftest.txt'";
 
-        private static final List<String> cases = Arrays.asList(case1, case2, case3, case4, case5, case6, case7, case8);
+        private static final List<String> cases = Arrays.asList(case1, case2, case3, case4, case5, case6, case7, case8, case9, case10);
 
         private static final Map<String, List<String>> METHOD_ARGUMENTS = Map.of(
                 "query", cases.stream().map(

extended/src/test/java/apoc/load/LoadHtmlTest.java

@@ -91,13 +91,13 @@ public void testParseGeneratedJs() {
 
     @Test
     public void testParseGeneratedJsWrongConfigs() {
-        assertWrongConfig(INVALID_CONFIG_ERR + "No enum constant io.github.bonigarcia.wdm.config.OperatingSystem.dunno",
+        assertWrongConfig(INVALID_CONFIG_ERR,
                 map("browser", CHROME, "operatingSystem", "dunno"));
 
-        assertWrongConfig(INVALID_CONFIG_ERR + "No enum constant io.github.bonigarcia.wdm.config.Architecture.dunno",
+        assertWrongConfig(INVALID_CONFIG_ERR,
                 map("browser", FIREFOX, "architecture", "dunno"));
 
-        assertWrongConfig("Error HTTP 404 executing https://raw.githubusercontent.com/bonigarcia/webdrivermanager/master/docs/mirror/geckodriver",
+        assertWrongConfig("Error HTTP 404 executing",
                 map("browser", FIREFOX,
                         "gitHubToken", "12345",
                         "forceDownload", true));
@@ -282,7 +282,6 @@ public void testQueryMetadataWithGetElementsByClassAndHtmlString() {
                     assertEquals(map("attributes", map("class", "firstClass"), "text", "My first paragraph.", "tagName", "p"), firstClass.get(0));
                     final List secondClass = value.get("secondClass");
                     assertEquals(map("attributes", map("class", "secondClass"), "text", "My second paragraph.", "tagName", "p"), secondClass.get(0));
-                    System.out.println("LoadHtmlTest.testQueryMetadataWithGetElementsByClass");
                 });
     }
 

extended/src/test/resources/long-create.cypher

@@ -3,12 +3,3 @@ CREATE (n:NodeOne {id:id});
 
 UNWIND RANGE(0,99999) as id
 CREATE (n:NodeTwo {id:id});
-
-UNWIND RANGE(0,99999) as id
-CREATE (n:NodeThree {id:id});
-
-UNWIND RANGE(0,99999) as id
-CREATE (n:NodeFour {id:id});
-
-UNWIND RANGE(0,99999) as id
-CREATE (n:NodeFive {id:id});

extra-dependencies/hadoop/build.gradle

@@ -30,6 +30,9 @@ shadowJar {
 def commonExclusions = {
     exclude group: 'io.netty'
     exclude group: 'com.sun.jersey'
+    exclude group: 'org.eclipse.jetty'
+    exclude group: 'org.eclipse.jetty.aggregate'
+    exclude group: 'org.slf4j', module: 'slf4j-api'
 }
 
 dependencies {