Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solve no suitable method found for register #3666

Merged
merged 12 commits into from
Aug 24, 2023
2 changes: 1 addition & 1 deletion .github/workflows/CI.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'

- name: Download neo4j dev docker container
Expand Down
1 change: 1 addition & 0 deletions extended/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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


Expand Down
36 changes: 23 additions & 13 deletions extended/src/main/java/apoc/custom/CypherProceduresHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);

}

Expand Down Expand Up @@ -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);

Expand All @@ -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;
Expand Down Expand Up @@ -406,7 +416,7 @@ public AnyValue apply(org.neo4j.kernel.api.procedure.Context ctx, AnyValue[] inp
}

}
}, true);
});
if (isStatementNull) {
registeredUserFunctionSignatures.remove(signature);
} else {
Expand Down
2 changes: 2 additions & 0 deletions extended/src/main/java/apoc/export/parquet/ImportParquet.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package apoc.export.parquet;

import apoc.Extended;
import apoc.Pools;
import apoc.export.util.BatchTransaction;
import apoc.export.util.ProgressReporter;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package apoc.export.parquet;

import apoc.util.Util;
import org.apache.parquet.hadoop.ParquetFileWriter;

import java.util.Collections;
import java.util.Map;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package apoc.export.parquet;

import apoc.ApocConfig;
import apoc.load.LoadParquet;
import apoc.util.JsonUtil;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configuration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import org.apache.parquet.example.data.GroupFactory;
import org.apache.parquet.example.data.simple.NanoTime;
import org.apache.parquet.example.data.simple.SimpleGroupFactory;
import org.apache.parquet.io.api.Binary;
import org.apache.parquet.schema.GroupType;
import org.apache.parquet.schema.LogicalTypeAnnotation;
import org.apache.parquet.schema.MessageType;
Expand Down
3 changes: 1 addition & 2 deletions extended/src/main/java/apoc/load/LoadHtmlBrowser.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.time.Duration;
import java.util.Map;

import static java.util.Optional.ofNullable;
Expand Down Expand Up @@ -133,7 +132,7 @@ private static InputStream getInputStreamWithBrowser(String url, Map<String, Str

final long wait = config.getWait();
if (wait > 0) {
Wait<WebDriver> driverWait = new WebDriverWait(driver, Duration.ofSeconds(wait));
Wait<WebDriver> driverWait = new WebDriverWait(driver, wait);
try {
driverWait.until(webDriver -> query.values().stream()
.noneMatch(selector -> webDriver.findElements(By.cssSelector(selector)).isEmpty()));
Expand Down
6 changes: 2 additions & 4 deletions extended/src/main/java/apoc/load/LoadParquet.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
package apoc.load;

import apoc.Extended;
import apoc.export.parquet.ApocParquetReader;
import apoc.export.parquet.ParquetConfig;
import apoc.result.MapResult;
import apoc.util.Util;
import org.apache.parquet.io.DelegatingSeekableInputStream;
import org.apache.parquet.io.InputFile;
import org.apache.parquet.io.SeekableInputStream;
import org.neo4j.logging.Log;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Description;
import org.neo4j.procedure.Name;
import org.neo4j.procedure.Procedure;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.Map;
import java.util.Spliterator;
Expand All @@ -24,6 +21,7 @@

import static apoc.export.parquet.ParquetReadUtil.getReader;

@Extended
public class LoadParquet {

@Context public Log log;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
10 changes: 10 additions & 0 deletions extended/src/main/resources/extended.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion extended/src/test/java/apoc/StartupExtendedTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading