From d79599b1299077ef39c4f23514a897115ae407b8 Mon Sep 17 00:00:00 2001 From: Matt Beanland Date: Thu, 23 Apr 2026 22:08:02 +0930 Subject: [PATCH] added bolt version policy, fixed dateTime encoding so version related, improved temporal tests --- .gitignore | 5 +- AGENTS.md | 293 ++++++++++++++++++ .../bolt_protocol/message/run_message.ex | 13 +- lib/bolty/bolt_protocol/message_encoder.ex | 21 +- lib/bolty/client.ex | 5 +- lib/bolty/connection.ex | 7 +- lib/bolty/error.ex | 5 +- lib/bolty/pack_stream.ex | 13 +- lib/bolty/pack_stream/packer.ex | 186 +++++++---- lib/bolty/policy.ex | 34 ++ lib/bolty/policy/resolver.ex | 52 ++++ test/bolty/pack_stream_test.exs | 146 ++++++++- test/bolty/policy/resolver_test.exs | 50 +++ test/bolty_test.exs | 129 ++++++-- 14 files changed, 839 insertions(+), 120 deletions(-) create mode 100644 AGENTS.md create mode 100644 lib/bolty/policy.ex create mode 100644 lib/bolty/policy/resolver.ex create mode 100644 test/bolty/policy/resolver_test.exs diff --git a/.gitignore b/.gitignore index 30cbde4..4bcb4a8 100644 --- a/.gitignore +++ b/.gitignore @@ -128,4 +128,7 @@ logs/ *.py[co] __pycache__ .pytest_cache -*.egg-info \ No newline at end of file +*.egg-info + +# agent related +.agent-notes/ \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..18074d5 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,293 @@ +# AGENTS.md — bolty + +> Status: v1 draft. Written primarily for agents that will **use** or **work on** bolty. Secondary audience: future maintainers (us) who need to remember why it is shaped the way it is. Expected to evolve. + +## 1. What bolty is + +`bolty` is an Elixir driver for [Neo4j](https://neo4j.com/) and other Bolt-speaking graph databases (notably Memgraph). It is a **reluctant fork** of [`boltx`](https://github.com/sagastume/boltx) — kept alive because specific fixes were needed (duration handling, maintenance), not because we wanted a new driver. Treat it as boltx-compatible in spirit; the upstream acknowledgment belongs to Luis Sagastume (`boltx`) and Florin Patrascu (`bolt_sips`). + +- **Protocol**: Bolt 1.0 → 5.4, with version negotiation at handshake time. +- **Server compatibility**: Neo4j 3.0.x → 5.13, Memgraph 2.13 (Bolt 5.0–5.2, advertised as Neo4j/5.2.0). +- **Pooling/transactions/prepared queries** via [`DBConnection`](https://hexdocs.pm/db_connection). +- **Hex package**: `:bolty` (current version in `mix.exs`). + +## 2. When to use bolty — and when not to + +**Use bolty when**: +- You need direct Cypher/Bolt access from Elixir with `DBConnection` pooling. +- You are speaking to Neo4j or a Bolt-compatible engine (Memgraph). +- You want to hand-write Cypher and deal in `Bolty.Types.*` structs. + +**Do not use bolty when**: +- You want Ash-style resources, actions, policies on top of Neo4j — use `ash_neo4j`, which sits on top of bolty. +- You need **streaming** of large result sets (not implemented; see Feature Support). +- You need **cluster routing** (not implemented; see Feature Support). + +If in doubt: agents operating *inside an Ash application* should almost always be going through `ash_neo4j`. bolty is the right tool for driver-level work, tests, benchmarks, and building higher-level abstractions. + +## 3. Quick start + +```elixir +# Start a pool +{:ok, conn} = + Bolty.start_link( + uri: "bolt://localhost:7687", + auth: [username: "neo4j", password: "password"], + pool_size: 10 + ) + +# Query +Bolty.query!(conn, "RETURN 1 AS n") |> Bolty.Response.first() +# => %{"n" => 1} + +# Transaction (commits on normal return, rolls back on raise or explicit rollback) +Bolty.transaction(conn, fn conn -> + Bolty.query!(conn, "CREATE (m:Movie {title: $t}) RETURN m", %{t: "Matrix"}) +end) +``` + +Supervised: + +```elixir +children = [ + {Bolty, Application.get_env(:bolty, Bolt)} +] +Supervisor.start_link(children, strategy: :one_for_one) +``` + +`Bolty.child_spec/1` returns a `DBConnection` pool child spec — name it via the standard `:name` option. + +## 4. Public API — what agents call + +| Function | Purpose | Notes | +| --- | --- | --- | +| `Bolty.start_link(opts)` | Start a pooled connection | Returns `{:ok, pid}`. Delegates to `DBConnection.start_link`. | +| `Bolty.child_spec(opts)` | Supervisor child spec | For embedding in a supervision tree. | +| `Bolty.query(conn, cypher, params \\ %{}, opts \\ [])` | Run one query | Returns `{:ok, %Bolty.Response{}} \| {:error, %Bolty.Error{}}`. | +| `Bolty.query!/4` | Raising variant | Raises `Bolty.Error` on failure. | +| `Bolty.query_many/4`, `query_many!/4` | Run a batch of statements | Returns list of responses. | +| `Bolty.transaction(conn, fun, opts \\ [], extra \\ %{})` | Transaction | `extra` is threaded into the BEGIN message (see §7). | +| `Bolty.rollback(conn, reason)` | Explicit rollback | Delegates to `DBConnection.rollback/2`. | +| `Bolty.Response.first/1` | Grab the first result row | Returns a map `%{field => value}` or `nil`. | + +**`params`** is a map of Cypher parameters. Most Elixir values pass through unchanged; `Bolty.Types.Point` is formatted specially (see §6). If you want to pass `TimeWithTZOffset` / `DateTimeWithTZOffset` into Cypher, call `format_param/1` yourself — only `Point` is auto-formatted at the top level today. + +**`opts`** accepts per-query extras lifted into the Bolt `extra` map: `:bookmarks`, `:mode` (`"r"` / `"w"`), `:db`, `:tx_metadata`. Everything else flows through to `DBConnection`. + +## 5. Connection options + +Canonical option names (what `Bolty.Client.Config.new/1` actually reads): + +| Option | Meaning | Default | +| --- | --- | --- | +| `:uri` | `://[:]` — wins over host/port/scheme | `nil` | +| `:hostname` | Host | `BOLT_HOST` env → `"localhost"` | +| `:port` | Port | `BOLT_TCP_PORT` env → `7687` | +| `:scheme` | One of the schemes below | `"bolt+s"` | +| `:auth` | `[username: ..., password: ...]` | required | +| `:versions` | Bolt versions to negotiate (e.g. `[4.4]` to prevent Bolt 5) | server-driven negotiation | +| `:user_agent` | Client identity string | `"bolty/"` | +| `:notifications_minimum_severity` | Bolt 5.2+ | `nil` | +| `:notifications_disabled_categories` | Bolt 5.2+ | `nil` | +| `:connect_timeout` | ms | `15_000` | +| `:ssl_opts` | `:ssl.tls_client_option()` list | merged with scheme-implied defaults | +| `:socket_options` | `:gen_tcp.connect_option()` list | `[mode: :binary, packet: :raw, active: false]` | +| DBConnection opts (`:name`, `:pool_size`, `:max_overflow`, `:after_connect`, ...) | flow through | | + +**Env-var precedence for auth is a sharp edge**: `BOLT_USER` and `BOLT_PWD` override the values you pass in `:auth`. Unset them explicitly if you don't want that. + +### URI schemes / TLS + +| URI scheme | TLS | ssl_opts merge | +| --- | --- | --- | +| `neo4j`, `bolt` | off | — | +| `neo4j+s`, `bolt+s` | on | `verify: :verify_none` (full cert, but no verification) | +| `neo4j+ssc`, `bolt+ssc` | on | `verify: :verify_peer` (self-signed allowed) | + +Default scheme when nothing is specified is `bolt+s`. + +## 6. Value mapping — Elixir ↔ Bolt/Neo4j + +All in `Bolty.Types`: + +- Graph: `Node`, `Relationship`, `UnboundRelationship`, `Path` (with `Path.graph/1` walking helper). +- Temporal (Bolt v2+): standard Elixir `Time`, `NaiveDateTime`, `Duration`; and `TimeWithTZOffset`, `DateTimeWithTZOffset` when you need integer-offset timezones. DateTime encoding is now policy-driven: the connection resolves a `%Bolty.Policy{datetime: :legacy | :evolved}` at HELLO and the packer emits the matching struct tag (0x46/0x66 legacy on Bolt ≤ 4.x, 0x49/0x69 evolved on Bolt 5.x) with the matching body semantics (legacy = local-wall-clock seconds; evolved = UTC-instant seconds). Unpacker handles both on decode. Resolved in 0.0.10 — issue [#10](https://github.com/diffo-dev/bolty/issues/10). `Duration` round-trip as a native Neo4j duration was broken in 0.0.7 and fixed through 0.0.8 (microseconds) and 0.0.9 (stored-as-string) — issues [#6](https://github.com/diffo-dev/bolty/issues/6) and [#8](https://github.com/diffo-dev/bolty/issues/8). +- Spatial (Bolt v2+): `Point` — 2D/3D, cartesian/WGS-84. Construct via `Point.create(:cartesian | :wgs_84 | , x, y [, z])`. + +`Path` has a quirk worth knowing: the Bolt protocol uses signed byte indices into the relationships list, but a raw `-1` comes through as `255`. `Path.graph/1` patches this explicitly. Flagged in the source as "oh dear"; keep the patch, do not "clean it up" without regression tests. + +## 7. Response shape and iteration + +`Bolty.Response`: + +```elixir +%Bolty.Response{ + results: [%{field => value}, ...], # zipped rows, usually what you want + fields: [String.t()], + records: [[raw_value, ...]], # untransformed column-major rows + plan: nil | map, + notifications: list, + stats: list | map, + profile: nil | any, + type: nil | String.t(), + bookmark: nil | String.t() +} +``` + +- `Bolty.Response.first/1` returns the first row (or `nil` on empty). +- `Enumerable` is implemented over `results` — `Enum.map(response, & &1)`, `Enum.count/1`, `for row <- response, do: ...` all work. Note `Enum.slice/2` on a non-empty response raises due to the custom `slice/1` returning `:error`; stick to `reduce`-backed calls if possible. + +## 8. Transactions + +```elixir +Bolty.transaction(conn, fn conn -> + Bolty.query!(conn, "CREATE (n:Thing) RETURN n") + # raise / rollback / normal return — DBConnection decides commit vs rollback +end, [], %{db: "mydb", mode: "w", tx_metadata: %{caller: "agent-me"}}) +``` + +- 4th arg (`extra_parameters`) is threaded into the Bolt BEGIN message via `extra_parameters` opt. This is the only way to scope `:db`, `:mode`, `:tx_metadata`, `:bookmarks` to the whole transaction (per-query `opts` only apply to a single RUN). +- `Bolty.rollback(conn, reason)` inside the fun aborts and returns `{:error, reason}` from the outer call. +- On syntax/semantic errors the driver proactively sends `RESET` (Bolt ≥ 3.0) or `ACK_FAILURE` (< 3.0) to recover the session. + +## 9. JSON encoding + +`Bolty.ResponseEncoder.encode(data, :json)` turns anything containing `Bolty.Types.*` into a JSON string. Two-step and overridable: + +1. Type → jsonable (protocol: `Bolty.ResponseEncoder.Json`) — implement your own `defimpl` for custom handling. +2. Jsonable → string — choose `Bolty.ResponseEncoder.Json.Jason` (default) or `Bolty.ResponseEncoder.Json.Poison`; both optional deps declared in `mix.exs`. + +## 10. Errors + +`%Bolty.Error{module, code, bolt, packstream}` — a `defexception`. Known code atoms: + +| Bolt error | Atom | +| --- | --- | +| `Neo.ClientError.Security.Unauthorized` | `:unauthorized` | +| `Neo.ClientError.Request.Invalid` | `:request_invalid` | +| `Neo.ClientError.Statement.SemanticError` | `:semantic_error` | +| `Neo.ClientError.Statement.SyntaxError` | `:syntax_error` | + +Everything else becomes `:unknown`, with the raw map still available in `error.bolt`. Expand `@error_map` in `lib/bolty/error.ex` when a new code becomes worth pattern-matching on. + +## 11. Feature support matrix + +| Capability | Status | +| --- | --- | +| Queries (RUN/PULL) | ✅ | +| Transactions (explicit + implicit) | ✅ | +| Pooling (DBConnection) | ✅ | +| Encoding/decoding of graph, temporal, spatial types | ✅ | +| TLS variants (full / self-signed / off) | ✅ | +| Notifications opt-out (Bolt 5.2+) | ✅ | +| Streaming result sets | ❌ | +| Cluster routing (`neo4j://` autodiscovery) | ❌ | +| Vector / vector search (indexes, similarity ops) | ❌ — under investigation, issue [#13](https://github.com/diffo-dev/bolty/issues/13) | + +If an agent needs routing or streaming today, that is not bolty's job — surface the gap to Matt rather than working around it silently. + +## 12. Running tests + +Tests are version-tagged. Defaults run only `:core`; everything else is disabled unless you opt in with env vars and tags. + +- Env vars: `BOLT_VERSIONS` (e.g. `"5.2"`), `BOLT_TCP_PORT` (e.g. `7690`), `BOLT_USER`, `BOLT_PWD`, `BOLT_HOST`. +- Tags: `:core`, `:bolt_version_X_Y` (e.g. `:bolt_version_5_2`), `:bolt_X_x` (e.g. `:bolt_5_x`), `:last_version`. + +Local server matrix via `docker-compose.yml`: + +| Service | Image | Ports (host:container) | Bolt versions | +| --- | --- | --- | --- | +| `neo4j-3.4.0` | `neo4j:3.4.0` | `7688:7687` | 1.0, 2.0 | +| `neo4j-4.4` | `neo4j:4.4.27-community` | `7689:7687` | 3.0, 4.0–4.4 | +| `neo4j-5.26.22` | `neo4j:5.26.22-community` | `7690:7687` | 5.0–5.4 | +| `memgraph-2.13.0` | `memgraph/memgraph:2.13.0` | `7691:7687` | 5.0–5.2 | + +All use credentials `neo4j / boltyPassword`. + +Test runner orchestrates this via `./scripts/test-runner.sh -c "mix test" -b "1.0,5.2" -d "neo4j,memgraph"`. Requires Docker, docker-compose, `jq`; `bats` for the script's own tests. See `scripts/README.md`. + +## 13. Development loop + +- Elixir `~> 1.14`. `.tool-versions` pins the expected runtime. +- `mix format` — `.formatter.exs` configured. +- `mix credo` — `.credo.exs` tuned; keep warnings at 0. +- `mix dialyzer` — PLT adds `:jason`, `:poison`, `:mix`; `.dialyzer_ignore.exs` holds accepted noise. +- `mix docs` — ex_doc; README.md is the main page. +- `mix test --cover` / `mix coveralls` — 70% threshold gate. +- `mix bench` (if present as an alias) — uses `benchee`, outputs via `benchee_html`; benchees live in `benchees/`. + +## 14. Implementation notes (for maintainers) + +Layering, top to bottom: + +``` +Bolty (top-level API; format_param dispatch, transaction wrap) + └── Bolty.Connection (DBConnection behaviour; version-aware init) + └── Bolty.Client (socket I/O, handshake, message send/receive) + └── Bolty.BoltProtocol.* + ├── Message.* (HELLO, LOGON, RUN, BEGIN, ...) + ├── MessageEncoder / MessageDecoder + ├── Versions + └── ServerResponse (statement_result / pull_result records) + └── Bolty.PackStream.* (Markers, Packer, Unpacker) +``` + +Init dispatch in `Bolty.Connection.do_init/3`: + +- Bolt ≤ 2.0 → `INIT`. +- Bolt 3.0–5.0 → `HELLO`. +- Bolt ≥ 5.1 → `HELLO` then `LOGON` (auth split out of HELLO). + +`handle_execute/4` always runs via `DBConnection.prepare_execute` — bolty does **not** use real prepared statements; `DBConnection.Query` is implemented as a no-op passthrough in `lib/bolty/query.ex`. `handle_prepare`, `handle_close`, `handle_declare`, `handle_fetch`, `handle_deallocate` are all trivial; `handle_status` is hardcoded to `:idle`. Revisit if true streaming lands. + +Fork posture: drift from boltx is minimised on purpose. When applying fixes, prefer surgical patches over refactors so upstream back-ports remain feasible. + +Compliance goals: bolty aims for [REUSE](https://reuse.software/) compliance (licence metadata on every file, including deps handling). Currently non-compliant — tracked in issue [#12](https://github.com/diffo-dev/bolty/issues/12). Keep this in mind when adding new source files. + +### Policy-driven packstream + +Version-aware encoding lives in `%Bolty.Policy{}` — an internal struct resolved once at HELLO completion from `(bolt_version, server_version)` via `Bolty.Policy.Resolver`, stashed on both `Bolty.Connection` and `Bolty.Client` state, and threaded through every `pack/2` call as a second argument. Codecs pattern-match on policy fields and never read a version number directly; that is the acceptance criterion for any future dimension. + +Policy is **not** user-facing — it's the driver's own distillation of negotiated facts about how Bolt and the server have evolved. Dimensions today: + +- `:datetime` — `:legacy` on Bolt ≤ 4.x (tags 0x46/0x66, body carries local-wall-clock seconds) or `:evolved` on Bolt ≥ 5.0 (tags 0x49/0x69, body carries UTC-instant seconds). Implemented in 0.0.10 to resolve issue [#10](https://github.com/diffo-dev/bolty/issues/10). + +When vectors (issue [#13](https://github.com/diffo-dev/bolty/issues/13)) land, add a new dimension to `Bolty.Policy`, extend `Bolty.Policy.Resolver` with a pure `put_vectors/3` clause, and dispatch the relevant codec on it — do **not** bypass the boundary. + +Authoritative design (including calibration history and non-goals): [`.agent-notes/policy-design.md`](./.agent-notes/policy-design.md). + +## 15. Sharp edges / known quirks + +- `.iex.exs` still references `Bolty.Router`, `Bolty.ConnectionSupervisor`, `Bolty.Protocol`, and a `Bolty.conn()` helper that **do not exist**. The example snippet uses legacy `url:` / `basic_auth:` options instead of the current `uri:` / `auth:`. Treat it as historical until cleaned up. +- `config/test.exs` also uses the legacy `url:` / `basic_auth:` keys and a `:queue_interval` / `:queue_target` / `:prefix` triple that the current config reader does not consume — harmless but misleading. +- `Bolty.Response`'s struct docstring is in Spanish (inherited from boltx). Rewrite in English when touching the module. +- `@error_map` only covers four Neo bolt errors; everything else collapses to `:unknown`. Extend when you need finer-grained handling. +- `format_param/1` at the top level only rewrites `Point`. Temporal-with-offset structs pass through as-is; call their own `format_param/1` if you need Cypher-ready strings. +- Env-var-overrides-opts precedence for `:auth` (BOLT_USER / BOLT_PWD) — as noted in §5. + +## 16. Issues (GitHub) + +Snapshot from `.agent-notes/issues.json` — re-dump to refresh. + +**Open** + +| # | Title | Label | Summary | +| --- | --- | --- | --- | +| [#12](https://github.com/diffo-dev/bolty/issues/12) | reuse compliance | enhancement | Make bolty (and its deps handling) REUSE-compliant. | +| [#13](https://github.com/diffo-dev/bolty/issues/13) | vector | enhancement | Investigate Neo4j vector / vector-search support. DozerDB caps at Neo4j 5.26.3 so the useful envelope is bounded; may need negotiated Bolt-version behaviour. | + +**Closed (for historical context)** + +| # | Title | Resolved in | Notes | +| --- | --- | --- | --- | +| [#5](https://github.com/diffo-dev/bolty/issues/5) | hex publish | 0.0.7 | Initial fork from `boltx` and Hex publication under the `bolty` name to avoid namespace collisions. | +| [#6](https://github.com/diffo-dev/bolty/issues/6) | duration microseconds | 0.0.8 | Elixir `Duration` was being serialised to ISO8601 string rather than native Neo4j duration struct. | +| [#8](https://github.com/diffo-dev/bolty/issues/8) | duration stored as string | 0.0.9 | Further fix — `Duration` outside of a map/struct wrapper was still being stored as a string. | +| [#10](https://github.com/diffo-dev/bolty/issues/10) | dateTime param illegal | 0.0.10 | `%DateTime{}` was packed with the evolved 0x69 tag unconditionally and broke against Neo4j 5.x when `:versions` was constrained to Bolt 4.x. Fixed by policy-driven packstream: `%Bolty.Policy{datetime: :legacy \| :evolved}` resolved at HELLO, dispatched in the packer. See `.agent-notes/policy-design.md`. | + +## 17. Evolving this document + +- Keep it agent-first: dense, scannable, honest about gaps. +- When bolty gains a capability, update §11 and add a usage snippet in §3 if there is a new ergonomic. +- When a sharp edge is fixed, move it out of §15 rather than deleting silently — commit history is the only other record. +- If the document exceeds ~500 lines, split into topic files and keep this one as an index. diff --git a/lib/bolty/bolt_protocol/message/run_message.ex b/lib/bolty/bolt_protocol/message/run_message.ex index db04448..e751349 100644 --- a/lib/bolty/bolt_protocol/message/run_message.ex +++ b/lib/bolty/bolt_protocol/message/run_message.ex @@ -2,22 +2,25 @@ defmodule Bolty.BoltProtocol.Message.RunMessage do @moduledoc false alias Bolty.BoltProtocol.MessageEncoder + alias Bolty.Policy @signature 0x10 - def encode(bolt_version, query, parameters, extra_parameters) + def encode(bolt_version, query, parameters, extra_parameters, policy \\ %Policy{}) + + def encode(bolt_version, query, parameters, extra_parameters, policy) when is_float(bolt_version) and bolt_version >= 3.0 do message = [query, parameters, get_extra_parameters(extra_parameters)] - MessageEncoder.encode(@signature, message) + MessageEncoder.encode(@signature, message, policy) end - def encode(bolt_version, query, parameters, _extra_parameters) + def encode(bolt_version, query, parameters, _extra_parameters, policy) when is_float(bolt_version) and bolt_version <= 2.0 do message = [query, parameters] - MessageEncoder.encode(@signature, message) + MessageEncoder.encode(@signature, message, policy) end - def encode(_, _, _, _) do + def encode(_, _, _, _, _) do {:error, Bolty.Error.wrap(__MODULE__, %{ code: :unsupported_message_version, diff --git a/lib/bolty/bolt_protocol/message_encoder.ex b/lib/bolty/bolt_protocol/message_encoder.ex index e810a25..38380ef 100644 --- a/lib/bolty/bolt_protocol/message_encoder.ex +++ b/lib/bolty/bolt_protocol/message_encoder.ex @@ -2,6 +2,7 @@ defmodule Bolty.BoltProtocol.MessageEncoder do @moduledoc false alias Bolty.PackStream + alias Bolty.Policy @max_chunk_size 65_535 @end_marker <<0x00, 0x00>> @@ -9,40 +10,40 @@ defmodule Bolty.BoltProtocol.MessageEncoder do @struct8_marker 0xDC @struct16_marker 0xDD - def encode(signature, data) do + def encode(signature, data, policy \\ %Policy{}) do Bolty.Utils.Logger.log_message(:client, :message_type, data) encoded = signature - |> do_encode(data) + |> do_encode(data, policy) |> generate_chunks([]) Bolty.Utils.Logger.log_message(:client, :message_type, encoded, :hex) encoded |> IO.iodata_to_binary() end - defp do_encode(signature, list) when length(list) <= 15 do + defp do_encode(signature, list, policy) when length(list) <= 15 do [ <<@tiny_struct_marker::4, length(list)::4, signature>>, - encode_list_data(list) + encode_list_data(list, policy) ] end - defp do_encode(signature, list) when length(list) <= 255 do - [<<@struct8_marker::8, length(list)::8, signature>>, encode_list_data(list)] + defp do_encode(signature, list, policy) when length(list) <= 255 do + [<<@struct8_marker::8, length(list)::8, signature>>, encode_list_data(list, policy)] end - defp do_encode(signature, list) when length(list) <= 65_535 do + defp do_encode(signature, list, policy) when length(list) <= 65_535 do [ <<@struct16_marker::8, length(list)::16, signature>>, - encode_list_data(list) + encode_list_data(list, policy) ] end - defp encode_list_data(data) do + defp encode_list_data(data, policy) do Enum.map( data, - &PackStream.pack!(&1) + &PackStream.pack!(&1, policy) ) end diff --git a/lib/bolty/client.ex b/lib/bolty/client.ex index c97aea0..f649b52 100644 --- a/lib/bolty/client.ex +++ b/lib/bolty/client.ex @@ -27,7 +27,7 @@ defmodule Bolty.Client do LogoffMessage } - defstruct [:sock, :bolt_version] + defstruct [:sock, :bolt_version, policy: %Bolty.Policy{}] defmodule Config do @moduledoc false @@ -282,7 +282,8 @@ defmodule Bolty.Client do end def send_run(client, query, parameters, extra_parameters) do - payload = RunMessage.encode(client.bolt_version, query, parameters, extra_parameters) + payload = + RunMessage.encode(client.bolt_version, query, parameters, extra_parameters, client.policy) with :ok <- send_packet(client, payload) do recv_packets(client, &RunMessage.prepare_messages/2, :infinity) diff --git a/lib/bolty/connection.ex b/lib/bolty/connection.ex index eea844d..0631b93 100644 --- a/lib/bolty/connection.ex +++ b/lib/bolty/connection.ex @@ -5,6 +5,7 @@ defmodule Bolty.Connection do import Bolty.BoltProtocol.ServerResponse alias Bolty.Client + alias Bolty.Policy alias Bolty.Response defstruct [ @@ -12,7 +13,8 @@ defmodule Bolty.Connection do :server_version, :hints, :patch_bolt, - :connection_id + :connection_id, + :policy ] @impl true @@ -21,8 +23,9 @@ defmodule Bolty.Connection do with {:ok, %Client{} = client} <- Client.connect(config), {:ok, response_server_metadata} <- do_init(client, opts) do + policy = Policy.Resolver.resolve(client.bolt_version, response_server_metadata) state = get_server_metadata_state(response_server_metadata) - {:ok, %__MODULE__{state | client: client}} + {:ok, %__MODULE__{state | client: %{client | policy: policy}, policy: policy}} end end diff --git a/lib/bolty/error.ex b/lib/bolty/error.ex index 2c8f20a..49b2947 100644 --- a/lib/bolty/error.ex +++ b/lib/bolty/error.ex @@ -18,13 +18,14 @@ defmodule Bolty.Error do @spec wrap(module(), atom()) :: t() def wrap(module, code) when is_atom(code), do: %__MODULE__{module: module, code: code} - @spec wrap(module(), binary()) :: t() + @spec wrap(module(), String.t()) :: t() def wrap(module, code) when is_binary(code), do: wrap(module, to_atom(code)) @spec wrap(module(), map()) :: t() def wrap(module, bolt_error) when is_map(bolt_error), do: %__MODULE__{module: module, code: bolt_error.code |> to_atom(), bolt: bolt_error} + @spec wrap(any(), any(), any()) :: t() def wrap(module, code, packstream), do: %__MODULE__{module: module, code: code, packstream: packstream} @@ -47,7 +48,7 @@ defmodule Bolty.Error do @doc """ Gets the corresponding atom based on the error code. """ - @spec to_atom(t()) :: String.t() + @spec to_atom(String.t()) :: atom() def to_atom(error_message) do Map.get(@error_map, error_message, :unknown) end diff --git a/lib/bolty/pack_stream.ex b/lib/bolty/pack_stream.ex index 17b5f30..3e541b8 100644 --- a/lib/bolty/pack_stream.ex +++ b/lib/bolty/pack_stream.ex @@ -3,12 +3,13 @@ defmodule Bolty.PackStream do alias Bolty.PackStream.Packer alias Bolty.PackStream.Unpacker + alias Bolty.Policy - def pack(term, options \\ []) do + def pack(term, policy \\ %Policy{}, options \\ []) do iodata? = Keyword.get(options, :iodata, false) try do - Packer.pack(term) + Packer.pack(term, policy) catch :throw, error -> {:error, error} @@ -24,9 +25,9 @@ defmodule Bolty.PackStream do end end - @spec pack!(term, Keyword.t()) :: iodata | no_return - def pack!(term, options \\ []) do - case pack(term, options) do + @spec pack!(term, Policy.t(), Keyword.t()) :: iodata | no_return + def pack!(term, policy \\ %Policy{}, options \\ []) do + case pack(term, policy, options) do {:ok, result} -> result @@ -35,7 +36,7 @@ defmodule Bolty.PackStream do end end - @spec unpack(binary()) :: list() + @spec unpack(any()) :: {:error, any()} | {:ok, list()} def unpack(iodata) do try do iodata diff --git a/lib/bolty/pack_stream/packer.ex b/lib/bolty/pack_stream/packer.ex index 240a07e..fe5cf74 100644 --- a/lib/bolty/pack_stream/packer.ex +++ b/lib/bolty/pack_stream/packer.ex @@ -3,6 +3,11 @@ defprotocol Bolty.PackStream.Packer do The `Bolty.PackStream.Packer` protocol is responsible for serializing any Elixir data structure according to the PackStream specification. + Every concrete implementation receives the resolved `%Bolty.Policy{}` for the + current connection as a second argument. Types whose wire format is + version-dependent (currently `DateTime` and `DateTimeWithTZOffset`) pattern- + match on it; the rest ignore it. + ## Serializing for structs By default, all structures are serialized with all their fields. However, if it is necessary that only certain fields be considered for serialization, it is necessary to @@ -15,31 +20,31 @@ defprotocol Bolty.PackStream.Packer do """ @fallback_to_any true - def pack(term) + def pack(term, policy) end defimpl Bolty.PackStream.Packer, for: Atom do use Bolty.PackStream.Markers - def pack(nil), do: <<@null_marker>> - def pack(false), do: <<@false_marker>> - def pack(true), do: <<@true_marker>> + def pack(nil, _policy), do: <<@null_marker>> + def pack(false, _policy), do: <<@false_marker>> + def pack(true, _policy), do: <<@true_marker>> - def pack(atom) do + def pack(atom, policy) do atom |> Atom.to_string() - |> @protocol.BitString.pack() + |> @protocol.BitString.pack(policy) end end defimpl Bolty.PackStream.Packer, for: BitString do use Bolty.PackStream.Markers - def pack(binary) when is_binary(binary) do + def pack(binary, _policy) when is_binary(binary) do [marker(binary), binary] end - def pack(bits) do + def pack(bits, _policy) do throw(Bolty.Error.wrap(__MODULE__, :not_encodable, bits: bits)) end @@ -59,11 +64,11 @@ end defimpl Bolty.PackStream.Packer, for: Integer do use Bolty.PackStream.Markers - def pack(integer) when integer in -16..127 do + def pack(integer, _policy) when integer in -16..127 do <> end - def pack(integer) do + def pack(integer, _policy) do case integer do integer when integer in @int8 -> <<@int8_marker, integer>> @@ -83,7 +88,7 @@ end defimpl Bolty.PackStream.Packer, for: Float do use Bolty.PackStream.Markers - def pack(number) do + def pack(number, _policy) do <<@float_marker, number::float>> end end @@ -91,8 +96,8 @@ end defimpl Bolty.PackStream.Packer, for: List do use Bolty.PackStream.Markers - def pack(list) do - [marker(list), list |> Enum.map(&@protocol.pack(&1))] + def pack(list, policy) do + [marker(list), list |> Enum.map(&@protocol.pack(&1, policy))] end defp marker(list) do @@ -111,8 +116,8 @@ end defimpl Bolty.PackStream.Packer, for: Map do use Bolty.PackStream.Markers - def pack(map) do - [marker(map), map |> encode_kv()] + def pack(map, policy) do + [marker(map), map |> encode_kv(policy)] end defp marker(map) do @@ -127,16 +132,16 @@ defimpl Bolty.PackStream.Packer, for: Map do end end - @spec encode_kv(map()) :: binary() - defp encode_kv(map) do - Enum.reduce(map, <<>>, fn data, acc -> [acc, do_reduce_kv(data)] end) + @spec encode_kv(map(), Bolty.Policy.t()) :: binary() + defp encode_kv(map, policy) do + Enum.reduce(map, <<>>, fn data, acc -> [acc, do_reduce_kv(data, policy)] end) end - @spec do_reduce_kv({atom(), any()}) :: [binary()] - defp do_reduce_kv({key, value}) do + @spec do_reduce_kv({atom(), any()}, Bolty.Policy.t()) :: [binary()] + defp do_reduce_kv({key, value}, policy) do [ - @protocol.pack(key), - @protocol.pack(value) + @protocol.pack(key, policy), + @protocol.pack(value, policy) ] end end @@ -144,12 +149,12 @@ end defimpl Bolty.PackStream.Packer, for: Time do use Bolty.PackStream.Markers - def pack(time) do + def pack(time, policy) do local_time = day_time(time) [ <<@tiny_struct_marker::4, @local_time_struct_size::4, @local_time_signature>>, - @protocol.pack(local_time) + @protocol.pack(local_time, policy) ] end @@ -162,36 +167,61 @@ end defimpl Bolty.PackStream.Packer, for: Date do use Bolty.PackStream.Markers - def pack(date) do + def pack(date, policy) do epoch = Date.diff(date, ~D[1970-01-01]) - [<<@tiny_struct_marker::4, @date_struct_size::4, @date_signature>>, @protocol.pack(epoch)] + + [ + <<@tiny_struct_marker::4, @date_struct_size::4, @date_signature>>, + @protocol.pack(epoch, policy) + ] end end defimpl Bolty.PackStream.Packer, for: DateTime do use Bolty.PackStream.Markers - def pack(datetime) do - data = - Enum.map( - decompose_datetime(DateTime.to_naive(datetime)) ++ [datetime.time_zone], - &@protocol.pack(&1) - ) + alias Bolty.Policy + + # Bolt 5 (evolved): body carries UTC-instant seconds since epoch. + # Distinct from the legacy encoding whenever the zone is non-UTC — this is + # exactly the "UTC-aware DateTime" fix that Bolt 5 introduced. + def pack(%DateTime{} = dt, %Policy{datetime: :evolved} = policy) do + body = decompose_utc(dt) ++ [dt.time_zone] + + [ + <<@tiny_struct_marker::4, @datetime_with_zone_id_struct_size::4, + @datetime_with_zone_id_signature>>, + Enum.map(body, &@protocol.pack(&1, policy)) + ] + end + + # Bolt <= 4 (legacy): body carries local-wall-clock seconds (naive diff from + # epoch, ignoring zone offset). Symmetric with the unpacker's legacy path + # which rebuilds via `NaiveDateTime.add` + `datetime_with_micro`. + def pack(%DateTime{} = dt, %Policy{datetime: :legacy} = policy) do + body = decompose_local(dt) ++ [dt.time_zone] [ <<@tiny_struct_marker::4, @legacy_datetime_with_zone_id_struct_size::4, @legacy_datetime_with_zone_id_signature>>, - data + Enum.map(body, &@protocol.pack(&1, policy)) ] end - @spec decompose_datetime(Calendar.naive_datetime()) :: [integer()] - defp decompose_datetime(%NaiveDateTime{} = datetime) do - datetime_micros = NaiveDateTime.diff(datetime, ~N[1970-01-01 00:00:00.000], :microsecond) - - seconds = div(datetime_micros, 1_000_000) - nanoseconds = rem(datetime_micros, 1_000_000) * 1_000 + @spec decompose_utc(DateTime.t()) :: [integer()] + defp decompose_utc(%DateTime{} = dt) do + total_us = DateTime.to_unix(dt, :microsecond) + seconds = Integer.floor_div(total_us, 1_000_000) + nanoseconds = (total_us - seconds * 1_000_000) * 1_000 + [seconds, nanoseconds] + end + @spec decompose_local(DateTime.t()) :: [integer()] + defp decompose_local(%DateTime{} = dt) do + naive = DateTime.to_naive(dt) + total_us = NaiveDateTime.diff(naive, ~N[1970-01-01 00:00:00.000], :microsecond) + seconds = Integer.floor_div(total_us, 1_000_000) + nanoseconds = (total_us - seconds * 1_000_000) * 1_000 [seconds, nanoseconds] end end @@ -199,11 +229,11 @@ end defimpl Bolty.PackStream.Packer, for: NaiveDateTime do use Bolty.PackStream.Markers - def pack(local_datetime) do + def pack(local_datetime, policy) do data = Enum.map( decompose_datetime(local_datetime), - &@protocol.pack(&1) + &@protocol.pack(&1, policy) ) [<<@tiny_struct_marker::4, @local_datetime_struct_size::4, @local_datetime_signature>>, data] @@ -223,13 +253,13 @@ end defimpl Bolty.PackStream.Packer, for: Bolty.Types.TimeWithTZOffset do use Bolty.PackStream.Markers - def pack(%Bolty.Types.TimeWithTZOffset{time: time, timezone_offset: offset}) do + def pack(%Bolty.Types.TimeWithTZOffset{time: time, timezone_offset: offset}, policy) do time_and_offset = [day_time(time), offset] data = Enum.map( time_and_offset, - &@protocol.pack(&1) + &@protocol.pack(&1, policy) ) [<<@tiny_struct_marker::4, @time_with_tz_struct_size::4, @time_with_tz_signature>>, data] @@ -244,27 +274,47 @@ end defimpl Bolty.PackStream.Packer, for: Bolty.Types.DateTimeWithTZOffset do use Bolty.PackStream.Markers - def pack(%Bolty.Types.DateTimeWithTZOffset{naive_datetime: ndt, timezone_offset: tz_offset}) do - data = - Enum.map( - decompose_datetime(ndt) ++ [tz_offset], - &@protocol.pack(&1) - ) + alias Bolty.Policy + + # Bolt 5 (evolved): body carries UTC-instant seconds. Because the struct's + # `naive_datetime` field stores the local wall clock, we subtract the zone + # offset to obtain the UTC seconds the wire expects. The unpacker's evolved + # path re-adds the offset to rebuild the local naive — so this round-trips. + def pack( + %Bolty.Types.DateTimeWithTZOffset{naive_datetime: ndt, timezone_offset: offset}, + %Policy{datetime: :evolved} = policy + ) do + [local_seconds, nanoseconds] = decompose_naive(ndt) + body = [local_seconds - offset, nanoseconds, offset] [ - <<@tiny_struct_marker::4, @legacy_datetime_with_zone_offset_struct_size::4, - @legacy_datetime_with_zone_offset_signature>>, - data + <<@tiny_struct_marker::4, @datetime_with_zone_offset_struct_size::4, + @datetime_with_zone_offset_signature>>, + Enum.map(body, &@protocol.pack(&1, policy)) ] end - @spec decompose_datetime(Calendar.naive_datetime()) :: [integer()] - defp decompose_datetime(%NaiveDateTime{} = datetime) do - datetime_micros = NaiveDateTime.diff(datetime, ~N[1970-01-01 00:00:00.000], :microsecond) + # Bolt <= 4 (legacy): body carries local-wall-clock seconds unchanged. The + # unpacker's legacy path rebuilds the naive directly without offset + # arithmetic, so no adjustment here. + def pack( + %Bolty.Types.DateTimeWithTZOffset{naive_datetime: ndt, timezone_offset: offset}, + %Policy{datetime: :legacy} = policy + ) do + body = decompose_naive(ndt) ++ [offset] - seconds = div(datetime_micros, 1_000_000) - nanoseconds = rem(datetime_micros, 1_000_000) * 1_000 + [ + <<@tiny_struct_marker::4, @legacy_datetime_with_zone_offset_struct_size::4, + @legacy_datetime_with_zone_offset_signature>>, + Enum.map(body, &@protocol.pack(&1, policy)) + ] + end + @spec decompose_naive(Calendar.naive_datetime()) :: [integer()] + defp decompose_naive(%NaiveDateTime{} = datetime) do + total_us = NaiveDateTime.diff(datetime, ~N[1970-01-01 00:00:00.000], :microsecond) + seconds = Integer.floor_div(total_us, 1_000_000) + nanoseconds = (total_us - seconds * 1_000_000) * 1_000 [seconds, nanoseconds] end end @@ -272,11 +322,11 @@ end defimpl Bolty.PackStream.Packer, for: Duration do use Bolty.PackStream.Markers - def pack(duration) do + def pack(duration, policy) do data = Enum.map( compact_duration(duration), - &@protocol.pack(&1) + &@protocol.pack(&1, policy) ) [<<@tiny_struct_marker::4, @duration_struct_size::4, @duration_signature>>, data] @@ -296,21 +346,21 @@ end defimpl Bolty.PackStream.Packer, for: Bolty.Types.Point do use Bolty.PackStream.Markers - def pack(%Bolty.Types.Point{z: nil} = point) do + def pack(%Bolty.Types.Point{z: nil} = point, policy) do data = Enum.map( [point.srid, point.x, point.y], - &@protocol.pack(&1) + &@protocol.pack(&1, policy) ) [<<@tiny_struct_marker::4, @point2d_struct_size::4, @point2d_signature>>, data] end - def pack(%Bolty.Types.Point{} = point) do + def pack(%Bolty.Types.Point{} = point, policy) do data = Enum.map( [point.srid, point.x, point.y, point.z], - &@protocol.pack(&1) + &@protocol.pack(&1, policy) ) [<<@tiny_struct_marker::4, @point3d_struct_size::4, @point3d_signature>>, data] @@ -350,19 +400,19 @@ defimpl Bolty.PackStream.Packer, for: Any do quote do defimpl unquote(@protocol), for: unquote(module) do - def pack(struct) do + def pack(struct, policy) do unquote(extractor) - |> @protocol.Map.pack() + |> @protocol.Map.pack(policy) end end end end - def pack(%{__struct__: _} = struct) do - @protocol.Map.pack(Map.from_struct(struct)) + def pack(%{__struct__: _} = struct, policy) do + @protocol.Map.pack(Map.from_struct(struct), policy) end - def pack(term) do + def pack(term, _policy) do raise Protocol.UndefinedError, protocol: @protocol, value: term end end diff --git a/lib/bolty/policy.ex b/lib/bolty/policy.ex new file mode 100644 index 0000000..1220d29 --- /dev/null +++ b/lib/bolty/policy.ex @@ -0,0 +1,34 @@ +defmodule Bolty.Policy do + @moduledoc """ + Resolved driver behaviour for a single connection. + + Produced once at HELLO completion by `Bolty.Policy.Resolver`, stashed on the + connection state, and threaded into every pack/unpack call. Codecs + pattern-match on policy fields and never read a Bolt or server version + directly. + + Policy is an internal distillation of negotiated facts, not a user-facing + configuration surface. Users influence policy by passing connection options + (e.g. constraining `:versions` at negotiation); the resolver responds + accordingly. + + See `.agent-notes/policy-design.md` for the authoritative design. + """ + + @typedoc """ + DateTime encoding dialect. + + * `:legacy` — emit legacy struct tags (0x46 for DateTime-with-offset, 0x66 + for DateTime-with-zone-id). Required for Bolt 4.x wire, regardless of + the server's own version. + * `:evolved` — emit evolved struct tags (0x49, 0x69). Required for Bolt + 5.x wire. + """ + @type datetime :: :legacy | :evolved + + @type t :: %__MODULE__{ + datetime: datetime() + } + + defstruct datetime: :legacy +end diff --git a/lib/bolty/policy/resolver.ex b/lib/bolty/policy/resolver.ex new file mode 100644 index 0000000..73b61d5 --- /dev/null +++ b/lib/bolty/policy/resolver.ex @@ -0,0 +1,52 @@ +defmodule Bolty.Policy.Resolver do + @moduledoc false + + alias Bolty.Policy + + @doc """ + Resolve a `%Bolty.Policy{}` from the negotiated Bolt version and the HELLO + response metadata. + + Pure function — no I/O, no process calls, no globals. Safe to call in tests + against synthetic inputs. + + `server_metadata` is the raw map returned by HELLO (keys are strings: + `"server"`, `"hints"`, `"patch_bolt"`, etc.). We extract what we need and + thread both `bolt_version` and `server_version` into every per-dimension + decision so that adding a server_version branch later is a clause change, + not a signature change. + """ + @spec resolve(float() | nil, map()) :: Policy.t() + def resolve(bolt_version, server_metadata) when is_map(server_metadata) do + server_version = Map.get(server_metadata, "server") + + %Policy{} + |> put_datetime(bolt_version, server_version) + + # |> put_vectors(bolt_version, server_version) # issue #13 + end + + # Working hypothesis, to be calibrated against the docker-compose matrix: + # bolt_version alone discriminates the three scenarios we care about for + # datetime, and server_version is not actually consulted. + # + # Neo4j 4.x x Bolt 4.x -> :legacy (legacy tags over legacy wire) + # Neo4j 5.x x Bolt 4.x -> :legacy (legacy wire still requires legacy tags) + # Neo4j 5.x x Bolt 5.x -> :evolved (evolved wire, evolved tags) + # + # A 4.x server never negotiates Bolt 5.x, so that combination is unreachable. + # + # Two realistic ways this could be wrong (resolvable via the matrix): + # - Scenario 2 (Neo4j 5.x speaking Bolt 4.x) may not accept the same + # legacy tags that Neo4j 4.x accepts -> add a server_version branch. + # - Memgraph advertises `server: "Neo4j/5.2.0"` but its Bolt 5.x datetime + # handling may diverge -> add a server_version branch on scenario 3. + # + # Both facts are named here so adding such a branch is clause-local. + defp put_datetime(policy, bolt_version, _server_version) + when is_float(bolt_version) and bolt_version >= 5.0 do + %{policy | datetime: :evolved} + end + + defp put_datetime(policy, _bolt_version, _server_version), do: policy +end diff --git a/test/bolty/pack_stream_test.exs b/test/bolty/pack_stream_test.exs index 9e7d556..812fe9e 100644 --- a/test/bolty/pack_stream_test.exs +++ b/test/bolty/pack_stream_test.exs @@ -2,10 +2,14 @@ defmodule Bolty.PackStreamTest do use ExUnit.Case, async: true alias Bolty.PackStream + alias Bolty.Policy alias Bolty.Types.{TimeWithTZOffset, DateTimeWithTZOffset, Point} alias Bolty.TypesHelper alias Bolty.TestDerivationStruct + @legacy %Policy{datetime: :legacy} + @evolved %Policy{datetime: :evolved} + defmodule TestStruct do defstruct foo: "bar" end @@ -150,19 +154,94 @@ defmodule Bolty.PackStreamTest do PackStream.pack!(ttz) end - test "datetime with timezone offset" do + test "datetime with timezone offset — :legacy emits 0x46" do dt = DateTimeWithTZOffset.create(~N[2016-05-24 13:26:08.654321], 7200) assert <<0xB3, 0x46, 0xCA, 0x57, 0x44, 0x56, 0x70, 0xCA, 0x27, 0x0, 0x25, 0x68, 0xC9, 0x1C, - 0x20>> == PackStream.pack!(dt) + 0x20>> == PackStream.pack!(dt, @legacy) + end + + test "datetime with timezone offset — :evolved emits 0x49 with UTC-shifted seconds" do + # Bolt 5's UTC-aware encoding: body carries UTC-instant seconds, so the + # seconds field is local_seconds - offset (1_464_096_368 - 7200 = + # 1_464_089_168 = 0x57443A50). Nanoseconds and offset are unchanged from + # the legacy encoding. The unpacker's evolved path adds the offset back + # to rebuild the local naive — see `unpacker.ex` @datetime_with_zone_offset. + dt = DateTimeWithTZOffset.create(~N[2016-05-24 13:26:08.654321], 7200) + + assert <<0xB3, 0x49, 0xCA, 0x57, 0x44, 0x3A, 0x50, 0xCA, 0x27, 0x0, 0x25, 0x68, 0xC9, 0x1C, + 0x20>> == PackStream.pack!(dt, @evolved) + end + + test "datetime with timezone offset — evolved seconds == legacy seconds - offset" do + # Explicit body-divergence check: the only difference between the two + # encodings is the seconds field (shifted by the zone offset). The fix + # for issue #10 hinges on this distinction. + dt = DateTimeWithTZOffset.create(~N[2016-05-24 13:26:08.654321], 7200) + + <<0xB3, 0x46, _int_marker, legacy_seconds::signed-32, _rest_legacy::binary>> = + :erlang.iolist_to_binary(PackStream.pack!(dt, @legacy)) + + <<0xB3, 0x49, _int_marker2, evolved_seconds::signed-32, _rest_evolved::binary>> = + :erlang.iolist_to_binary(PackStream.pack!(dt, @evolved)) + + assert evolved_seconds == legacy_seconds - 7200 end - test "datetime with timezone id" do + test "datetime with timezone id — :legacy emits 0x66" do dt = TypesHelper.datetime_with_micro(~N[2016-05-24 13:26:08.654321], "Europe/Berlin") assert <<0xB3, 0x66, 0xCA, 0x57, 0x44, 0x56, 0x70, 0xCA, 0x27, 0x0, 0x25, 0x68, 0x8D, 0x45, 0x75, 0x72, 0x6F, 0x70, 0x65, 0x2F, 0x42, 0x65, 0x72, 0x6C, 0x69, 0x6E>> == - PackStream.pack!(dt) + PackStream.pack!(dt, @legacy) + end + + test "datetime with timezone id — :evolved emits 0x69 with UTC-shifted seconds" do + # May 2016 Berlin is CEST (UTC+2). Evolved body carries the UTC instant — + # so the seconds word drops by 7200 relative to legacy (0x57445670 local + # → 0x57443A50 UTC). Nanoseconds and zone-id string are unchanged. + dt = TypesHelper.datetime_with_micro(~N[2016-05-24 13:26:08.654321], "Europe/Berlin") + + assert <<0xB3, 0x69, 0xCA, 0x57, 0x44, 0x3A, 0x50, 0xCA, 0x27, 0x0, 0x25, 0x68, 0x8D, 0x45, + 0x75, 0x72, 0x6F, 0x70, 0x65, 0x2F, 0x42, 0x65, 0x72, 0x6C, 0x69, 0x6E>> == + PackStream.pack!(dt, @evolved) + end + + test "datetime with timezone id — evolved Berlin seconds == legacy seconds - CEST offset" do + # Body-divergence check for the zone-id variant. Confirms the split is + # not just a tag rename — it genuinely reshapes the seconds field by the + # effective UTC offset of the zone at that instant (CEST = 7200s). + dt = TypesHelper.datetime_with_micro(~N[2016-05-24 13:26:08.654321], "Europe/Berlin") + + <<0xB3, 0x66, _int_marker, legacy_seconds::signed-32, _rest_legacy::binary>> = + :erlang.iolist_to_binary(PackStream.pack!(dt, @legacy)) + + <<0xB3, 0x69, _int_marker2, evolved_seconds::signed-32, _rest_evolved::binary>> = + :erlang.iolist_to_binary(PackStream.pack!(dt, @evolved)) + + assert evolved_seconds == legacy_seconds - 7200 + end + + test "datetime with timezone id from elixir DateTime — :legacy emits 0x66" do + datetime = ~U[2025-05-11 07:45:41.429903Z] + + assert <<0xB3, 0x66, 0xCA, 0x68, 0x20, 0x55, 0xA5, 0xCA, 0x19, 0x9F, 0xCC, 0x98, 0x87, 0x45, + 0x74, 0x63, 0x2F, 0x55, 0x54, 0x43>> == + PackStream.pack!(datetime, @legacy) + end + + test "datetime with timezone id from elixir DateTime — :evolved emits 0x69" do + datetime = ~U[2025-05-11 07:45:41.429903Z] + + assert <<0xB3, 0x69, 0xCA, 0x68, 0x20, 0x55, 0xA5, 0xCA, 0x19, 0x9F, 0xCC, 0x98, 0x87, 0x45, + 0x74, 0x63, 0x2F, 0x55, 0x54, 0x43>> == + PackStream.pack!(datetime, @evolved) + end + + test "default policy is :legacy (DateTime falls back to 0x66)" do + datetime = ~U[2025-05-11 07:45:41.429903Z] + <<0xB3, tag, _rest::binary>> = PackStream.pack!(datetime) + assert tag == 0x66 end test "duration with all values" do @@ -503,6 +582,65 @@ defmodule Bolty.PackStreamTest do ) end + test "Datetime with zone id — evolved (0x69) decodes UTC-instant seconds" do + # Symmetric with the evolved packer: the wire carries UTC seconds + # (0x57443A50 = 1_464_089_168) plus the zone id; the unpacker reconstructs + # via DateTime.from_unix + DateTime.shift_zone, so the local wall clock + # lands at 13:26:08 CEST — the same DateTime a legacy 0x66 of the same + # local wall clock would produce. + dt = + Bolty.TypesHelper.datetime_with_micro( + ~N[2016-05-24 13:26:08.654321], + "Europe/Berlin" + ) + + assert [dt] == + PackStream.unpack!( + <<0xB3, 0x69, 0xCA, 0x57, 0x44, 0x3A, 0x50, 0xCA, 0x27, 0x0, 0x25, 0x68, 0x8D, + 0x45, 0x75, 0x72, 0x6F, 0x70, 0x65, 0x2F, 0x42, 0x65, 0x72, 0x6C, 0x69, 0x6E>> + ) + end + + test "Datetime with zone offset — evolved (0x49) decodes UTC-instant seconds" do + # Symmetric with the evolved packer: wire carries UTC seconds + # (0x57443A50) plus offset (7200); the unpacker re-adds the offset to + # rebuild the local naive (2016-05-24 13:26:08.654321). + assert [ + %DateTimeWithTZOffset{ + naive_datetime: ~N[2016-05-24 13:26:08.654321], + timezone_offset: 7200 + } + ] = + PackStream.unpack!( + <<0xB3, 0x49, 0xCA, 0x57, 0x44, 0x3A, 0x50, 0xCA, 0x27, 0x0, 0x25, 0x68, 0xC9, + 0x1C, 0x20>> + ) + end + + test "Datetime zone-id/zone-offset — legacy and evolved round-trip to the same value" do + # End-to-end sanity: pack under each policy, unpack the bytes, and + # assert the values match. Catches any body-semantics mismatch between + # packer and unpacker on either branch. + dt_zone_id = + Bolty.TypesHelper.datetime_with_micro( + ~N[2016-05-24 13:26:08.654321], + "Europe/Berlin" + ) + + legacy_bytes = :erlang.iolist_to_binary(PackStream.pack!(dt_zone_id, @legacy)) + evolved_bytes = :erlang.iolist_to_binary(PackStream.pack!(dt_zone_id, @evolved)) + + assert [dt_zone_id] == PackStream.unpack!(legacy_bytes) + assert [dt_zone_id] == PackStream.unpack!(evolved_bytes) + + dt_offset = DateTimeWithTZOffset.create(~N[2016-05-24 13:26:08.654321], 7200) + legacy_offset_bytes = :erlang.iolist_to_binary(PackStream.pack!(dt_offset, @legacy)) + evolved_offset_bytes = :erlang.iolist_to_binary(PackStream.pack!(dt_offset, @evolved)) + + assert [dt_offset] == PackStream.unpack!(legacy_offset_bytes) + assert [dt_offset] == PackStream.unpack!(evolved_offset_bytes) + end + test "Duration" do assert [ %Duration{ diff --git a/test/bolty/policy/resolver_test.exs b/test/bolty/policy/resolver_test.exs new file mode 100644 index 0000000..0376792 --- /dev/null +++ b/test/bolty/policy/resolver_test.exs @@ -0,0 +1,50 @@ +defmodule Bolty.Policy.ResolverTest do + use ExUnit.Case, async: true + + alias Bolty.Policy + alias Bolty.Policy.Resolver + + describe "resolve/2 datetime dimension" do + @describetag :core + + test "Bolt 3.0 resolves to :legacy" do + assert %Policy{datetime: :legacy} = Resolver.resolve(3.0, %{"server" => "Neo4j/3.5.0"}) + end + + test "Bolt 4.0 resolves to :legacy" do + assert %Policy{datetime: :legacy} = Resolver.resolve(4.0, %{"server" => "Neo4j/4.4.27"}) + end + + test "Bolt 4.4 against a Neo4j 5 server resolves to :legacy (scenario 2)" do + # Neo4j 5 explicitly supports legacy datetime structs over Bolt 4.x for + # backward compatibility. This is the exact shape that broke issue #10 + # in bolty 0.0.9 because the packer emitted 0x69 unconditionally. + assert %Policy{datetime: :legacy} = Resolver.resolve(4.4, %{"server" => "Neo4j/5.26.22"}) + end + + test "Bolt 5.0 resolves to :evolved" do + assert %Policy{datetime: :evolved} = Resolver.resolve(5.0, %{"server" => "Neo4j/5.26.22"}) + end + + test "Bolt 5.4 resolves to :evolved" do + assert %Policy{datetime: :evolved} = Resolver.resolve(5.4, %{"server" => "Neo4j/5.26.22"}) + end + + test "Memgraph masquerading as Neo4j/5.2.0 at Bolt 5.2 resolves to :evolved" do + # If calibration later shows Memgraph needs :legacy at Bolt 5.x, add a + # server_version branch in `put_datetime/3`. + assert %Policy{datetime: :evolved} = Resolver.resolve(5.2, %{"server" => "Neo4j/5.2.0"}) + end + + test "missing server metadata does not crash; still decides from bolt_version" do + assert %Policy{datetime: :legacy} = Resolver.resolve(4.4, %{}) + assert %Policy{datetime: :evolved} = Resolver.resolve(5.0, %{}) + end + + test "nil bolt_version falls through to defaults (:legacy)" do + # Defensive — connect-time call should always pass a negotiated version, + # but the resolver shouldn't crash if something odd gets through. + assert %Policy{datetime: :legacy} = Resolver.resolve(nil, %{"server" => "Neo4j/5.26.22"}) + end + end +end diff --git a/test/bolty_test.exs b/test/bolty_test.exs index 846c394..bc929bb 100644 --- a/test/bolty_test.exs +++ b/test/bolty_test.exs @@ -226,6 +226,58 @@ defmodule BoltyTest do assert to_timeout(n.properties["max_session"]) == to_timeout(Duration.new!(day: 1)) end + @tag :core + test "executing a Cypher query, with date_time parameter", c do + cypher = """ + CREATE(n:User {name: $name, joined: $joined}) RETURN n + """ + + parameters = %{name: "Kote", joined: DateTime.utc_now()} + + n = + Bolty.query!(c.conn, cypher, parameters) + |> Response.first() + |> Map.get("n") + + assert n.labels == ["User"] + assert n.properties["name"] == "Kote" + assert n.properties["joined"] == parameters.joined + end + + @tag :core + test "executing a Cypher query, with non-UTC zoned DateTime parameter (Europe/Berlin)", c do + # Regression for issue #10 — UTC round-trip passes even when body semantics + # are wrong because UTC offset is 0. A zoned DateTime in Europe/Berlin + # forces the evolved packer to emit UTC-instant seconds (local − offset); + # if we accidentally emit local-wall-clock seconds on Bolt 5, the server + # stores an instant shifted by one zone offset and `DateTime.compare/2` + # returns `:gt` or `:lt` instead of `:eq`. + cypher = """ + CREATE(n:User {name: $name, joined: $joined}) RETURN n + """ + + {:ok, berlin_now} = DateTime.now("Europe/Berlin") + parameters = %{name: "Kvothe", joined: berlin_now} + + n = + Bolty.query!(c.conn, cypher, parameters) + |> Response.first() + |> Map.get("n") + + assert n.labels == ["User"] + assert n.properties["name"] == "Kvothe" + # Strict equality: server should preserve the zone id string, and under + # the evolved policy the UTC instant should round-trip exactly. If the + # evolved packer accidentally emits local-wall-clock seconds instead of + # UTC seconds, the returned DateTime will be shifted by one zone offset + # and this assertion fails with a one-hour (or one-CEST-offset) delta. + assert n.properties["joined"] == parameters.joined + # Belt-and-braces check: even if the returned zone is canonicalised, + # the UTC instant must still match. This isolates body-semantics + # regressions from zone-string canonicalisation drift. + assert DateTime.compare(n.properties["joined"], parameters.joined) == :eq + end + @tag :core test "executing a Cypher query, with struct parameters", c do cypher = """ @@ -490,7 +542,7 @@ defmodule BoltyTest do @tag :bolt_3_x @tag :bolt_4_x @tag :bolt_5_x - test "Cypher with plan resul", c do + test "Cypher with plan result", c do assert %Response{plan: plan} = Bolty.query!(c.conn, "EXPLAIN RETURN 1") refute plan == nil assert Regex.match?(~r/[3|4|5]/iu, plan["args"]["planner-version"]) @@ -547,38 +599,75 @@ defmodule BoltyTest do ] end + @tag :core @tag :bolt_2_x @tag :bolt_3_x @tag :bolt_4_x @tag :bolt_5_x - test "transform Duration in cypher-compliant data", c do - query = "RETURN duration($d) AS d" + test "Duration as Cypher input: datetime + duration → datetime", c do + # Exercises DURATION-as-input. Cypher's `+` operator between a datetime + # and a duration requires a proper DURATION on the right — if the packer + # regresses to ISO-8601 string serialisation (the pre-#8 bug), the server + # responds with a type error rather than performing the arithmetic. + # + # Also exercises the policy-driven DateTime packer we fixed for #10: + # $t is a `%DateTime{}` and must round-trip correctly on every negotiated + # Bolt version. + query = "RETURN $t + $d AS result" params = %{ - d: %Duration{ - day: 0, - hour: 0, - minute: 54, - month: 12, - microsecond: {0, 6}, - second: 65, - week: 0, - year: 1 - } + t: ~U[2020-01-01 00:00:00Z], + d: %Duration{year: 1, minute: 30} } + assert {:ok, %Response{results: [%{"result" => result}]}} = + Bolty.query(c.conn, query, params) + + # Neo4j applies month arithmetic first, then seconds — so 2020-01-01 + + # 1 year + 30 min = 2021-01-01 00:30:00 UTC. Compare by instant: the + # server may return microsecond precision {0, 6} where the sigil gives + # {0, 0}, and that's a representation detail we don't want to assert on. + assert DateTime.compare(result, ~U[2021-01-01 00:30:00Z]) == :eq + end + + @tag :core + @tag :bolt_2_x + @tag :bolt_3_x + @tag :bolt_4_x + @tag :bolt_5_x + test "Duration as Cypher output: duration.inSeconds(t1, t2) → duration", c do + # Exercises DURATION-as-output. The server computes the duration from + # two datetimes; bolty decodes it via `TypesHelper.create_duration/4`, + # which splits the raw (months, days, seconds, nanoseconds) tuple into + # year/month/day/hour/minute/second/microsecond buckets. + # + # `duration.inSeconds/2` is preferred over `duration.between/2` because + # it returns a deterministic seconds-only duration — `between` returns a + # compound (months+days+seconds) duration whose canonical form depends on + # server calendar logic and is harder to pin down in an assertion. + query = "RETURN duration.inSeconds($t1, $t2) AS d" + + params = %{ + t1: ~U[2020-01-01 00:00:00Z], + t2: ~U[2020-01-01 01:30:00Z] + } + + # 5400 seconds → create_duration splits into 1h 30m 0s. year/month/week/ + # day are all 0; microsecond tuple is {0, 6} because create_duration + # hardcodes 6-digit precision. expected = %Duration{ - day: 0, - hour: 0, - minute: 55, + year: 0, month: 0, - microsecond: {0, 6}, - second: 5, week: 0, - year: 2 + day: 0, + hour: 1, + minute: 30, + second: 0, + microsecond: {0, 6} } - assert {:ok, %Response{results: [%{"d" => ^expected}]}} = Bolty.query(c.conn, query, params) + assert {:ok, %Response{results: [%{"d" => ^expected}]}} = + Bolty.query(c.conn, query, params) end end