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

Fix compatibility with Neo4j server and Neo4j::Driver #38

Merged
merged 8 commits into from
Dec 11, 2024
Merged

Conversation

johannessen
Copy link
Collaborator

Hi Mark @majensen,

I’ve been working on an update for Neo4j::Driver that is just about ready for release now. (Neo4j-Driver-1.02-TRIAL) The new version removes some old cruft, cutting lines of code by over 10 %. It also improves performance. One of these improvements is that values in the query result are now passed through directly from Neo4j::Bolt to the user. That change breaks use of the Bolt protocol with REST::Neo4p. (HTTP still works fine.)

This PR fixes that issue. It also contains several other fixes, including for problems in the REST::Neo4p tests related to very old and new Neo4j versions. However, the known problem with indices in Neo4j 5 still exists. #34 (comment) This PR simply skips the associated tests on unsupported Neo4j versions.

This PR should make REST::Neo4p work reasonably well on all Neo4j versions in the range 1.9 through 5.26, and possibly beyond.

I understand you don’t plan on spending much time with REST::Neo4p going forward. But as long as it involves relatively little effort (such as merging these PRs and making a new release ☺️), I think there is value in keeping it working.

Neo4j::Driver 1.02 no longer reblesses values received via Neo4j::Bolt. Like before, the returned values are still Neo4j::Types implementations, so their interface doesn't actually change. But the package name of such values changes from Neo4j::Driver::Type::* to Neo4j::Bolt::*, which trips up some checks in REST::Neo4p that expect to see "Driver" in the package name.

Original discussion: majensen/perlbolt#37
It's necessary to call a method on the result within the try block. Any method will do; has_next() is one of the cheapest ones to call.

Resolves #28.
ResultProcessor.pm doesn't actually use warnings, so `no warnings` is currently unnecessary, but I'm still adding it in this change. While doing so, I also fix some typos that had gone unnoticed because ResultProcessor.pm didn't use strict, either.

Closes #34.
Neo4j 5 removed a number of deprecated procedures used by Neo4p to implement indices. As a result, indices don't work with Neo4p 0.4003 on Neo4j 5.

Simply trying to replace the deprecated calls in DriverActions.pm with the new syntax seemed to work in Neo4j 4, but not Neo4 5. The issue is possibly related to the way Neo4p uses properties on the index. There might be changes that I missed in Neo4j 5 in addition to the new syntax.

But since Neo4j 5 currently isn't officially supported by Neo4p, it might not matter much. This change for now simply skips the failing tests.

Note that Neo4j < 5.17 doesn't allow query parameters in the new index syntax.
Back when Neo4p was updated for Neo4j 4, some tests were rewritten to use the $param syntax, which broke those tests on old Neo4j versions. This change reverts the syntax in affected tests back to {param} and adds the Neo4j::Driver option cypher_params v2 to keep things working on Neo4j 4+.

The cypher_params option replaced the experimental cypher_filter option in 2021.

https://metacpan.org/release/AJNN/Neo4j-Driver-0.52/view/lib/Neo4j/Driver/Deprecations.pod#Parameter-syntax-conversion-using-cypher_filter
Back when Neo4p was updated for Neo4j 4, queries that begin with MATCH were added to some tests. My understanding is that such queries are unsupported on Neo4j 1. This change simply skips the affected tests on Neo4j 1.
REST::Neo4p 0.4003 can't parse the metadata for entities returned from Neo4j < 3.5 via the transaction endpoint (unless the Neo4j::Driver agent is used). This change prevents that bug from breaking the test suite.

To reproduce the bug (will trigger a "Can't call method on unblessed reference" error):

REST::Neo4p->begin_work;
(my $q = REST::Neo4p::Query->new('MATCH (n) RETURN n LIMIT 1'))->execute;
print $q->fetch->[0]->id;

Issues with the tx response format aren't new. The changelog for version 0.2220 included a fix for it, but the response format was updated in later Neo4j versions, forcing further changes. I believe the current iteration of the bug may have been introduced in 2077e4b.
@johannessen johannessen requested a review from majensen December 11, 2024 13:24
@majensen majensen merged commit c2a6381 into master Dec 11, 2024
22 of 25 checks passed
@majensen
Copy link
Owner

This is great @johannessen - more power to you!

@johannessen johannessen deleted the compat-neo4j branch December 12, 2024 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecation of legacy numeric IDs by Neo4j 5 Driver agent may discard statements before they have executed
2 participants