TODO
Address open issues on GitHub
See https://github.com/johannessen/neo4j-driver-perl/issues.
Functionality and API
Implement spatial and temporal types.
Add timers to Neo4j::Driver::ResultSummary (see
Neo4j::Bolt
).croak()
error objects (e. g. Exception::Class) instead of strings. It seems there are about four types that would need to be distinguished: illegal usage errors, internal driver errors, Network errors, and Neo4j server errors. See also #7.
Experimental features
"Parameter syntax conversion" in Neo4j::Driver: make stable and move filter implementation to
Transaction
in preparation of Bolt v3 support"Calling in list context" in Neo4j::Driver::Session: make illegal
"Concurrent transactions" in Neo4j::Driver::Session: make illegal for both nested explicit and nested autocommit transactions (for consistency with Bolt), but provide a driver config option (e. g.
nested_transactions => 1
) that lifts this restriction for HTTP"Calling in list context" in Neo4j::Driver::Transaction: make illegal
"Execute multiple statements at once" in Neo4j::Driver::Transaction: This feature should no longer be exposed to the client. It complicates the API significantly and is not that big of an optimisation anyway, because results are typically fetched before the next statement is run.
"Return results in graph format" in Neo4j::Driver::Transaction, "graph" in Neo4j::Driver::Record: make driver config option
Calling list methods in scalar context (in Node, in Path, in Result, and in ResultSummary) should probably generally behave the same way using a list in scalar context would: It should return the number of items in the list.
Jolt: The
jolt
config option will be deprecated once Jolt implementation is fully complete. Those who use a modern Neo4j version and still want to get plain old JSON responses can easily write their own net module overriding theAccept
header.
Tests, code quality, documentation
Test roundtrip of special numeric values (very large integers, -0.0, ±Inf, ±NaN).
Improve test coverage:
Many yet uncovered code paths are obviously fine, but difficult or impossible to cover. In some of these cases, it may be possible to refactor the code, such as by banking on autovivification (i.e. don't defend against undefined
$a
in expressions like$a->{b}
; see "Using References" in perlref).The
deep_bless
subs contain a lot of assertions and other checks that are not normally necessary. Since this logic seems to work fine, it should be simplified. (This may also make it easier to unroll the recursion later.)The current policy of not documenting deprecated methods is informed by the principle to "design interfaces that are: consistent; easy to use correctly; hard to use incorrectly". Perhaps simply listing the deprecated method names with short note like "deprecated in 0.13" would be an acceptable addition that also fulfils the pod coverage requirements.
Documenting each attribute in Neo4j::Driver::SummaryCounters as individual methods might be a quick way to bring up the pod coverage stats a bit.
Neo4j::Test should auto-detect the Neo4j server version and set the
cypher_filter
config option accordingly.Write new unit tests for all modules.
Optimise the simulator for
$hash = 0
. Use of<<
causes statements to end with\n
, which the simulator could filter out. The internals test "transaction: REST 404 error handling" should run a distinct statement.Verify that "get" in Neo4j::Driver::Type::Node and "get" in Neo4j::Driver::Type::Relationship really do return undef (as documented), even when called in list context.
Check that after a server error, the next statement will succeed (there might be issues with perlbolt; see majensen/libneo4j-client#8).
List possible
croak
output in "DIAGNOSTICS" in Neo4j::Driver, allowing for indexing by search engines."notifications" in Neo4j::Driver::ResultSummary: Clarify docs that this method is to be called in list context.
Other ideas for specific modules
Neo4j::Driver
Make the URL a config option, so that it can be queried (and changed).
Make the auth data a config option, so that it can be queried. As alternative ways to set the auth data,
basic_auth()
should continue to be supported as an alias and the user info should be parsed from the URL if given (however, URLs without user info should not change the auth data stored in the driver). A possible implementation would be to create a new AuthToken module that would offer suitable methods, but this seems like overkill. While sending/accepting userinfo to/from others is explicitly forbidden now by RFC7230:2.7.1, it's still allowed to use it in cases such as ours.Allow passing config options directly to the constructor, e. g. in place of the URL (
Neo4j::Driver->new({ url=>"bolt:", timeout=>30 })
).Change the default URI scheme from HTTP to auto-detect, i. e. try Bolt first, then HTTP in case of failure. This could be explicitly specified as e. g.
//localhost
.The
neo4j
scheme could perhaps be mapped onto Bolt or onto the default URL scheme, just so thatneo4j://...
URLs will kind of work.Consider writing a concrete example that re-creates LOMS logic by re-blessing the structural types into custom types from the business logic using
$cypher_types->{init}
. (For example, check for Neo4j nodes that are labelled:Person
and re-bless those asLocal::Person
or whatever.)
Neo4j::Driver::Session
Once a session is created, the driver object becomes immutable. It should therefore be possible to store the ServerInfo in the driver object once it is obtained. If the default database is added as well, the Discovery API doesn't need to be used again for a new session. This change would keep down network utilisation in scenarios where many sessions are created (such as running the driver's test suite).
Consider whether to offer transaction functions. If available, these should consist of subrefs passed to methods called
write_transaction
andread_transaction
. These access modes are only an optimisation for Enterprise features. We don't target those at present, butread_transaction
could then eventually be routed to a high-performance read-only server once clusters are supported. It would make sense to offer both methods right away even though initially they'd work exactly the same.
Neo4j::Driver::Transaction
Consider supporting re-using
Record
objects for query parameters inrun
. The Java and C# drivers do this.Run statements lazily: Just like with the official drivers, statements passed to
run
should be gathered until their results are actually accessed. Then, and only then, all statements gathered so far should be sent to the server using a single request. Challenges of this approach include that notifications are not associated with a single statement, so there must be an option to disable this behaviour; indeed, disabled should probably be the default when stats are requested. Additionally, there are some bugs with multiple statements (see testsnon-arrayref individual statement
andinclude empty statement
). Since stats are now requested by default, this item might mean investing time in developing an optimisation feature that is almost never used. Since the server is often run on localhost anyway where latency is very close to zero, this item should not have high priority.
Neo4j::Driver::Record
Consider whether to implement methods to query the list of fields for this record (
keys
,has
,size
) and/or a mapping function for all fields (map
/forEach
). Given that this data should easily be available through the Result object, these seem slightly superfluous though.Implement
graph
; see https://github.com/neo4j/neo4j/blob/3.5/community/server/src/main/java/org/neo4j/server/rest/transactional/GraphExtractionWriter.java, https://github.com/neo4j/neo4j-python-driver/wiki/1.6-changelog#160a1.Add
field()
as alias forget()
, enabling clients to avoid the possibly confusing$record->get->get
pattern. The official drivers only offerget()
, andfield()
might be too similar tofields()
in the official Java driver, so this alias should perhaps be experimental.
Neo4j::Driver::Result
Perhaps
fetch()
should always buffer two records instead of just one. With the current implementation, the bolt connection might remain attached longer than desirable in cases where the client knows in advance how many records there will be and callsfetch()
exactly that number of times. (In theory, such a change might even slightly improve performance if the driver uses Perl threads to fill the buffer in the background.)Consider unrolling
deep_bless
recursion. Based on initial profiling, this may save up to about 5% CPU time (for a specific HTTP test query cached in RAM, performance went from about 2700/s to 2850/s when skipping the call todeep_bless
entirely). However, when accessing the database, the bottleneck is typically I/O (querying Neo4j itself instead of the RAM-cached response let the performance for the very same query drop down to 650/s when executed over HTTP). So this optimisation may not be worth it (OTOH, Bolt performance was something like 7000/s, so optimisingdeep_bless
may be more useful there).
Neo4j::Driver::ResultColumns
The entire package can probably be removed now.
Neo4j::Driver::ResultSummary
Profile the server-side performance penalty of requesting stats for various kinds of queries. If the penalty turns out to be high, stats should perhaps have to be requested explicitly by clients (rather than being obtained by default, as with 0.13 and higher). However, using Bolt always provides stats, and different APIs for HTTP and Bolt seem like a bad idea.
Neo4j::Driver::SummaryCounters
use Class::Accessor::Fast 0.34;
It seems Neo4j 4 added new counters for system updates.
Neo4j::Driver::Net::Bolt
Rollback behaviour on errors needs further study. Neo4j Status Codes says that all errors have a rollback effect, but in at least some cases, the effect seems to be merely to mark the tx as failed and uncommittable, which isn't quite the same thing. This may or may not vary across error types, Neo4j versions, or Bolt versions. OTOH, some errors are internal client errors that shouldn't rollback the tx (majensen/libneo4j-client#7). Not sure if these occur in practice, but we should probably be able to handle them correctly anyway.
Neo4j::Driver::Net::HTTP
If a 201 is received without a
Location
header, it is currently simply ignored by_parse_tx_status()
. (The simulator requires this.) According to RFC 7231, such a response means the location hasn't changed, i. e. the resource has been created at the default transaction endpoint. That should never happen; in fact, it should only ever happen for a PUT request, but we don't use those here. So ignoring this is probably the right choice. But it may still be useful to revisit this logic later on.
Neo4j::Driver::Type::*
The
eq
andne
operators should be overloaded to allow for ID comparison on nodes and relationships.Consider whether to use
use overload '%{}'
to allow direct access to e. g. properties using the pre-0.13 hashref syntax. See https://metacpan.org/pod/overload#Two-face-References for an example. perltie might perhaps also work. Note that overload might be a memory hog; see "BUGS" in Types::Serialiser.Try to refactor Neo4j::Driver::Type::Path's internal representation to allow either elements or nodes+rels. Have one autogenerate from the other, then cache the results. May not actually have advantages for deep_bless though.
Add
property()
as alias forget()
in Neo4j::Driver::Type::Node and Neo4j::Driver::Type::Relationship, enabling clients to avoid the possibly confusing$record->get->get
pattern.