Skip to content

[9uFWHlVy] Check compatibility on system database instead of default database #207

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 11, 2022
Merged

[9uFWHlVy] Check compatibility on system database instead of default database #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6223d39
[9uFWHlVy] Check compatibility on system database instead of default …
Lojjs Oct 7, 2022
6ab27be
[9uFWHlVy] Add clarifying comment
Lojjs Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions core/src/main/java/apoc/cypher/CypherInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.kernel.api.procedure.GlobalProcedures;
import org.neo4j.kernel.availability.AvailabilityListener;
import org.neo4j.kernel.database.DefaultDatabaseResolver;
import org.neo4j.kernel.internal.GraphDatabaseAPI;
import org.neo4j.logging.Log;

Expand Down Expand Up @@ -62,16 +61,18 @@ public void available() {
awaitApocProceduresRegistered();
}

var defaultDb = dependencyResolver.resolveDependency(DefaultDatabaseResolver.class).defaultDatabase(null);
if (defaultDb.equals(db.databaseName())) {
// This code is running once per database.
// We only want to check compatibility once, so we do it when we are on the system database.
if (isSystemDatabase) {
try {
awaitDbmsComponentsProcedureRegistered();
final List<String> versions = db.executeTransactionally("CALL dbms.components", Collections.emptyMap(),
r -> (List<String>) r.next().get("versions"));
final String apocFullVersion = Version.class.getPackage().getImplementationVersion();
if (isVersionDifferent(versions, apocFullVersion)) {
final String apocVersion = Version.class.getPackage().getImplementationVersion();
if (isVersionDifferent(versions, apocVersion)) {
userLog.warn("The apoc version (%s) and the Neo4j DBMS versions %s are incompatible. \n" +
"See the compatibility matrix in https://neo4j.com/labs/apoc/4.4/installation/ to see the correct version",
apocFullVersion, versions.toString());
"The two first numbers of both versions needs to be the same.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? I thought after 5.0 we wanted to have a compatibility layer that works differently

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me, Nacho and Daniel discussed this a bit at the lunch lecture on GDS compatibility layer last week. It is leaning towards that we will not do a compatibility layer, but just say you need to use e.g. APOC 5.2.0 with Neo4j 5.2.0 or Neo4j 5.2.1 (if there is a security patch)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay :)

apocVersion, versions.toString());
}
} catch (Exception ignored) {
userLog.info("Cannot check APOC version compatibility because of a transient error. Retrying your request at a later time may succeed");
Expand Down Expand Up @@ -130,14 +131,20 @@ private void putIfNotBlank(Map<String,String> map, String key, String value) {
}

private void awaitApocProceduresRegistered() {
while (!areApocProceduresRegistered()) {
while (!areProceduresRegistered("apoc")) {
Util.sleep(100);
}
}

private boolean areApocProceduresRegistered() {
private void awaitDbmsComponentsProcedureRegistered() {
while (!areProceduresRegistered("dbms.components")) {
Util.sleep(100);
}
}

private boolean areProceduresRegistered(String procStart) {
try {
return procs.getAllProcedures().stream().anyMatch(signature -> signature.name().toString().startsWith("apoc"));
return procs.getAllProcedures().stream().anyMatch(signature -> signature.name().toString().startsWith(procStart));
} catch (ConcurrentModificationException e) {
// if a CME happens (possible during procedure scanning)
// we return false and the caller will try again
Expand Down
2 changes: 2 additions & 0 deletions core/src/test/java/apoc/cypher/CypherIsVersionDifferentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public void shouldReturnFalseOnlyWithCompatibleVersion() {
assertFalse(isVersionDifferent(List.of("3.5.12"), "3.5.0.9"));
assertFalse(isVersionDifferent(List.of("3.5.12"), "3.5.1.9"));
assertFalse(isVersionDifferent(List.of("4.4.5"), "4.4.0.4"));
assertFalse(isVersionDifferent(List.of("5.1.1"), "5.1.0"));
assertTrue(isVersionDifferent(List.of("5.1.0"), "5.2.0"));

// we expect that APOC versioning is always consistent to Neo4j versioning
assertTrue(isVersionDifferent(List.of(""), "5_2_0_1"));
Expand Down