# PODNAME: Neo4j::Driver::TODO
# ABSTRACT: Information on planned improvements to Neo4j::Driver

=encoding utf8

=head1 TODO

=head2 Address open issues on GitHub

See L<https://github.com/johannessen/neo4j-driver-perl/issues>.

=head2 Functionality and API

=over

=item * Implement spatial and temporal types.

=item * Add timers to L<Neo4j::Driver::ResultSummary> (see C<Neo4j::Bolt>).

=item * C<croak()> error objects (e. g. L<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 L<#7|https://github.com/johannessen/neo4j-driver-perl/issues/7>.

=back

=head2 Experimental features

=over

=item * L<Neo4j::Driver/"Parameter syntax conversion">: make stable

=item * L<Neo4j::Driver/"Custom networking modules">: make stable, but
not until at least part of the C<result_handlers> API has been specified

=item * L<Neo4j::Driver::Result/"Control result stream attachment">:
make C<detach()> stable (it's only needed I<very> rarely, but when it is,
there may be no good alternative); deprecate C<attached()>

=item * L<Neo4j::Driver::Result/"Look ahead in the result stream">:
make C<peek()> stable once C<fetch()> reliably buffers two records

=item * L<Neo4j::Driver::Session/"Concurrent transactions">: make illegal
for both nested explicit and nested autocommit transactions (for
consistency with Bolt), but provide a driver config option (e. g.
C<< nested_transactions => 1 >>) that lifts this restriction for HTTP

=item * L<Neo4j::Driver::Result/"Disable obtaining query statistics">:
Deprecate in favour of modifying the JSON in a custom net module.
Clarify that this is already now an internal API in the deprecation
explanation, so that it can be dropped from the module pod.

=item * L<Neo4j::Driver::Transaction/"Return results in graph format">:
Deprecate in favour of modifying the JSON in a custom net module. Clarify that
this and L<Neo4j::Driver::Record/"graph"> are already now internal APIs in the
deprecation explanation, so that both can be dropped from the module pod.

=item * Calling list methods in scalar context
(L<in Node|Neo4j::Driver::Type::Node/"Calling in scalar context">,
L<in Path|Neo4j::Driver::Type::Path/"Calling in scalar context">,
L<in Result|Neo4j::Driver::Result/"Calling in scalar context">, and
L<in ResultSummary|Neo4j::Driver::ResultSummary/"Calling in scalar context">):
make stable

=item * Jolt: The C<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 the C<Accept> header.

=back

=head2 Tests, code quality, documentation

=over

=item * Test roundtrip of special numeric values
(very large integers, -0.0, ±Inf, ±NaN).

=item * Improve test coverage:

=over

=item * 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
C<$a> in expressions like C<< $a->{b} >>; see L<perlref/"Using References">).

=item * The C<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.)

=item * Documenting each attribute in L<Neo4j::Driver::SummaryCounters> as individual
methods might be a quick way to bring up the B<pod> coverage stats a bit.

=back

=item * Write new unit tests for all modules.

=item * Optimise the simulator for C<$hash = 0>.
Use of C<<< << >>> causes statements to end with C<\n>, which the simulator
could filter out. 
The internals test "transaction: REST 404 error handling" should run a distinct
statement.

=item * Verify that L<Neo4j::Driver::Type::Node/"get"> and
L<Neo4j::Driver::Type::Relationship/"get"> really do return undef
(as documented), even when called in list context.

=item * Check that after a server error, the next statement will succeed
(there I<might> be issues with perlbolt; see
L<majensen/libneo4j-client#8|https://github.com/majensen/libneo4j-client/commit/3bec0ff2306f4d282d36e1a036f828e882dc7426>).

=item * List possible C<croak> output in L<Neo4j::Driver/"DIAGNOSTICS">,
allowing for indexing by search engines.

=item * We need complete working code examples, such as that Neo4j movies app.
Some of the deprecated functionality could also be implemented as a custom
module named S<e. g.> Neo4j::Driver::Net::HTTP::Extra. Such a module might
be directly useful to some users, but most importantly, it would serve to
demonstrate some of the net_module functionality and how that can be used.

=back

=head1 Other ideas for specific modules

=head2 L<Neo4j::Driver>

=over

=item * The C<neo4j> scheme should work for a community edition server out
of the box. For starters, C<neo4j> should check if L<Neo4j::Bolt> is loaded
into C<%INC> and if the specified server actually supports Bolt. If so, it
should behave identically to C<bolt>, otherwise if should behave identically
to either C<http> or C<https>, depending on the encryption config.
This behaviour is different than that of the official drivers; see also
L<https://community.neo4j.com/t/different-between-neo4j-and-bolt/18498>.
It should be sufficient to document that the behaviour is subject to
change (clusters are enterprise-only and therefore currently out of scope,
and the conditions under which Bolt is chosen may also change in future).

=item * Change the default URI scheme from C<http> to C<neo4j>. Because
a C<use Neo4j::Bolt;> statement has so far never been necessary for the
driver and the driver's behaviour won't change it it is not present, and
also because HTTP-only features have so far been experimental features
only, this should not be a breaking change.

=back

=head2 L<Neo4j::Driver::Session>

=over

=item * 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).

=item * Add
L<transaction functions|https://neo4j.com/docs/driver-manual/4.1/session-api/simple/#driver-simple-transaction-fn>.
These should probably consist of subrefs passed to methods called
C<write_transaction> and C<read_transaction>. These access modes are only an
optimisation for Enterprise features. We don't target those at present, but
C<read_transaction> could then eventually be routed to a high-performance
read-only server (for Bolt connections) once clusters are supported.

=back

=head2 L<Neo4j::Driver::Transaction>

=over

=item * Consider supporting re-using C<Record> objects for query parameters in
C<run>. The Java and C# drivers do this.

=item * Run statements lazily: Just like with the official drivers, statements
passed to C<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 tests C<non-arrayref individual statement> and
C<include 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.

=back

=head2 L<Neo4j::Driver::Record>

=over

=item * Consider whether to implement methods to query the list of fields for
this record (C<keys>, C<has>, C<size>) and/or a mapping function for all fields
(C<map>/C<forEach>). Given that this data should easily be available through
the Result object, these seem slightly superfluous though.

=item * Implement C<graph>; see
L<https://github.com/neo4j/neo4j/blob/3.5/community/server/src/main/java/org/neo4j/server/rest/transactional/GraphExtractionWriter.java>,
L<https://github.com/neo4j/neo4j-python-driver/wiki/1.6-changelog#160a1>.

=item * Add C<field()> as alias for C<get()>, enabling clients to avoid the
possibly confusing C<< $record->get->get >> pattern. The official drivers
only offer C<get()>, and C<field()> might be too similar to C<fields()> in
the official Java driver, so this alias should perhaps be experimental.

=back

=head2 L<Neo4j::Driver::Result>

=over

=item * Improve documentation, esp. wrt. side-effects of e. g. C<has_next()>

=item * Perhaps C<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 calls C<fetch()> 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.)

=item * Consider unrolling C<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 to
C<deep_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 optimising C<deep_bless> may be more useful there).

=back

=head2 L<Neo4j::Driver::ResultColumns>

=over

=item * The entire package can probably be removed now.

=back

=head2 L<Neo4j::Driver::ResultSummary>

=over

=item * 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.

=back

=head2 L<Neo4j::Driver::SummaryCounters>

=over

=item * C<use Class::Accessor::Fast 0.34;>

=item * It seems Neo4j 4 added new counters for system updates.

=back

=head2 L<Neo4j::Driver::Net>

=over

=item * The lingo for the different components is not very clear. Options
to be considered:

=over

=item Should "net helper" be renamed to "net controller"?

=item Should "net module" be renamed to "net agent"?

Problem is, "agent" is already an overloaded term.

=item Should "net module" be renamed to "net plugin"?

Currently we include a net module with the distribution, so that's not really
a plugin. However, we could simply describe that as the "default plugin". Also,
perhaps the LWP module should be externalised anyway, to ease future expansion.

=back

=back

=head2 L<Neo4j::Driver::Net::Bolt>

=over

=item * Rollback behaviour on errors needs further study.
L<Neo4j Status Codes|https://neo4j.com/docs/status-codes/4.2/>
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
(L<majensen/libneo4j-client#7|https://github.com/majensen/libneo4j-client/issues/7#issuecomment-752914015>).
Not sure if these occur in practice, but we should probably
be able to handle them correctly anyway.
(See also L<neo4j#12651|https://github.com/neo4j/neo4j/issues/12651>.)

=back

=head2 L<Neo4j::Driver::Net::HTTP>

=over

=item * If a 201 is received without a C<Location> header, it is currently
simply ignored by C<_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.

=back

=head2 Neo4j::Driver::Type::*

=over

=item * The C<eq> and C<ne> operators should be overloaded to allow
for ID comparison on nodes and relationships.

=item * Consider whether to use C<use overload '%{}'> to allow direct
access to e. g. properties using the pre-0.13 hashref syntax. See
L<https://metacpan.org/pod/overload#Two-face-References> for an example.
L<perltie> might perhaps also work.
Note that L<overload> might be a memory hog; see L<Types::Serialiser/"BUGS">.

=item * Try to refactor L<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.

=item * Add C<property()> as alias for C<get()> in L<Neo4j::Driver::Type::Node>
and L<Neo4j::Driver::Type::Relationship>, enabling clients to avoid the
possibly confusing C<< $record->get->get >> pattern.
Another way to avoid this pattern would be C<< $record->get->properties->{} >>.
Aside from being longer, this is also S<50 %> slower because of the defensive
copy that needs to be created. We should probably cache that one at least.
In the long run, the internal representation of these types will change;
the C<properties()> hash ref will then actually be the fastest way to get a
single property (about 8% faster than C<get()>).

=back