Skip to content

fix(mongodb,couchbase): ledger queries must name the fields the writer wrote - #122

Merged
bfarmer67 merged 1 commit into
mainfrom
devs/bfarmer/ledger-field-name-contract
Aug 26, 2026
Merged

bfarmer67 merged 1 commit into
mainfrom
devs/bfarmer/ledger-field-name-contract

Conversation

@bfarmer67

Copy link
Copy Markdown
Contributor

Stacked on #121. Base is devs/bfarmer/opensearch-ledger-mget-index-inference because both fixes are instances of the same invariant and ADR-0029 lands there. GitHub will retarget this to main when #121 merges. Review the second commit only.

MongoDB — live bug, silent, shipping since 3.0.0

IntersectWithSquashedAsync built its filter half typed and half literal:

Filter.Eq( x => x.Kind, Squash )   // typed  → renders through the class map → "Kind"
Filter.In( "replaces", ... )       // literal → renders verbatim             → "replaces"

The driver's default element name is the member name, so the writer stores Replaces. Rendered, the filter is

{ "Kind": 1, "replaces": { "$in": [900] } }

against a document of

{ "_id": "...", "RunOn": {...}, "Checksum": "...", "Kind": 1, "Replaces": [900, 901] }

It can never match. IntersectWithSquashedAsync returned an empty set for every input — so a squash was never recognized as covering its replaced versions, and those migrations re-ran.

It stayed invisible because empty is also the correct answer whenever nothing is squashed. There is no error, no log line, no failed run — just squashes that quietly do nothing.

Both terms are now typed, so both route through the same serialization path as the writer. AnyIn is the array-valued form of In (Replaces is a long[]; the predicate is "contains any of"). No wire change — the query is corrected to match bytes that were always being written.

Why not pin the element names

[BsonElement] or a registered BsonClassMap would repair the library's self-consistency, and would also orphan any deployment whose consumer registered a global naming convention. A camelCase convention pack applied with t => true is a common MongoDB setup line; those deployments have a self-consistent camelCase ledger today, and a pinned PascalCase map would stop reading it. Routing everything through one path is correct under any configuration, including none, and changes nothing for anyone.

Couchbase — same class, latent rather than live

N1QL has no typed field reference, so IntersectWithSquashedAsync must name ledger fields as text (m.kind, m.replaces) — while documents serialized through ClusterOptions.Serializer, which the consumer owns. The stock serializer happens to emit camelCase, so it works today by luck. A consumer setting a System.Text.Json serializer, or a Newtonsoft one without the camelCase resolver, writes Kind/Replaces and the squash query silently matches nothing — same failure, same consequence, one config line away.

Ledger KV reads and writes now use a library-owned DefaultSerializer in its default configuration.

Behavior note worth a second look during review. This is byte-for-byte identical for consumers on the stock serializer (the default). Consumers who set a custom ClusterOptions.Serializer will see new ledger rows written in the canonical camelCase shape:

  • ExistsAsync / IntersectWithAppliedAsync are key-based — unaffected.
  • ReadAsync on a row written under a custom shape may return null RunOn/Checksum, which can cost one extra cron evaluation.
  • Squash reconciliation, which was broken for exactly this configuration, starts working.

That population is already in a broken state, so I judged this net-positive — but it is a behavior change to a shipping provider and it is isolated to its own hunk if you'd rather defer it.

Tests

MongoDBLedgerWireTests / CouchbaseLedgerWireTests render the real queries the record stores issue and compare them against the real serializer output. No mock, no container, milliseconds.

Query construction moved to internal statics (BuildSquashFilter, BuildAppliedFilter, BuildSquashQuery) so the tests read the same query the store issues rather than a copy that can drift. Built inline, the invariant was untestable — which is how this shipped.

The MongoDB assertions are convention-independent on purpose. They assert the field a query asks for is the field the writer wrote, not that a field has a particular casing. They still pass for a consumer with a camelCase convention. Pinning a literal casing would have written a test that agrees with the bug.

MongoDBRecordStoreIntegrationTests covers squash and applied reconciliation against a real MongoDB. IntersectWithSquashedAsync had no coverage at any tier.

Verified red → green:

before after
MongoDBLedgerWireTests (4) 2 fail — filter references [Kind, replaces], but {"replaces"} do(es) not match 4 pass
MongoDBRecordStoreIntegrationTests (3, real MongoDB) 1 fail — AreEquivalent failed. Expected:<2>. Actual:<0> 3 pass
CouchbaseLedgerWireTests (4) — 4 pass
full unit suite 433 pass 436 pass

The Expected:<2>. Actual:<0> against a real MongoDB is the whole bug in one line.

Note

Adds InternalsVisibleTo for Hyperbee.Migrations.Tests and Hyperbee.Migrations.Integration.Tests to the MongoDB provider — it only had Squash.Tests, unlike every other provider.

🤖 Generated with Claude Code

…r wrote

MongoDB -- IntersectWithSquashedAsync built its filter half typed and half
literal:

    Filter.Eq( x => x.Kind, Squash )   -> renders through the class map -> "Kind"
    Filter.In( "replaces", ... )       -> raw string, renders verbatim  -> "replaces"

The driver's default element name is the member name, so the writer stores
`Replaces`. The rendered filter { "Kind": 1, "replaces": { "$in": [...] } }
could never match. The method returned an empty set for every input, so a
squash was never recognized as covering its replaced versions and those
migrations re-ran. It stayed silent because an empty set is also the correct
answer whenever nothing is squashed.

Both terms are now typed, so both route through the same serialization path
as the writer. No wire change -- the query is corrected to match bytes that
were always being written.

Deliberately NOT fixed by pinning element names with [BsonElement] or a
registered class map. Pinning repairs the library's self-consistency at the
cost of orphaning any deployment whose consumer registered a global naming
convention: their ledger is self-consistent under that convention today, and
a pinned map would stop reading it. Routing everything through one path is
correct under any configuration, including none.

Couchbase -- the same class of coupling, latent rather than live. N1QL has no
typed field reference, so IntersectWithSquashedAsync names ledger fields as
text (m.kind, m.replaces) while documents serialized through the
consumer-owned ClusterOptions.Serializer. A custom serializer writes
Kind/Replaces and the squash query silently matches nothing. Ledger KV
operations now use a library-owned DefaultSerializer in its default
configuration -- byte-identical for anyone on the stock serializer.

Query construction moved to internal statics so the wire tests can assert the
field names a query references against the field names the writer emits. Built
inline, that invariant was untestable, which is how this shipped.

The MongoDB wire assertions are convention-INDEPENDENT on purpose: they assert
that the field a query asks for is the field the writer wrote, not that a field
has a particular casing. Pinning a literal casing would have made the tests
agree with the bug.

Verified against a real MongoDB: the squash reconciliation integration test
fails Expected:<2> Actual:<0> before the fix.

Per ADR-0029.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/ledger-field-name-contract branch from c0b6a9c to 5a99951 Compare August 26, 2026 21:52
@bfarmer67
bfarmer67 changed the base branch from devs/bfarmer/opensearch-ledger-mget-index-inference to main August 26, 2026 21:52
@bfarmer67
bfarmer67 merged commit 0933164 into main Aug 26, 2026
2 checks passed
@bfarmer67
bfarmer67 deleted the devs/bfarmer/ledger-field-name-contract branch August 29, 2026 03:40
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.

1 participant