Skip to content

Expose a document's mapped version member, and let the enum in-fragments be told the stored name - #592

Merged
jeremydmiller merged 1 commit into
masterfrom
feat/mapped-version-member-seam
Sep 12, 2026
Merged

jeremydmiller merged 1 commit into
masterfrom
feat/mapped-version-member-seam

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #590. Closes #591.

Two additive seams a document store can opt into. Both default to today's behaviour, neither changes
what a migration does to an existing database, and nothing in Weasel, Marten, Polecat or Fisher
changes until a store adopts them.

A storage can expose a document's mapped version / revision member

IDocumentStorage<T> gains two default interface members:

Guid? MappedVersionFor(T document) => null;
long? MappedRevisionFor(T document) => null;

A session needs the expected version for a write's concurrency guard. It can test a document for a
versioned marker interface directly, but a document that instead maps a plain member as its version
carries the same information somewhere the session cannot reach — it holds the storage only as
IDocumentStorage<T>.

In Marten (JasperFx/marten#5372, reported by @JurJean) the consequence was that a document configured
with Metadata(m => m.Version.MapTo(x => x.Etag)) bound DBNull into its
ON CONFLICT … WHERE table.mt_version = ? guard on every cross-session Store(), so no RETURNING row
came back and a write that should have succeeded was reported as a ConcurrencyException. The member
is populated on save and on load, so the mapping looks like it works — only the write guard never
sees it.

Every alternative needed a runtime type test on the storage plus explicit forwarding in each
decorator, and a decorator that forgets to forward fails silently, which is the wrong failure
mode for a concurrency guard.

MappedRevisionFor returns a long whatever the member's own width is, matching
DocumentRevisionBinder, which already accepts an int or a long revision member.

The enum in fragments can be told the name the serializer stored

EnumIsOneOfWhereFragment and EnumIsNotOneOfWhereFragment each gain a constructor overload taking
an optional Func<object, string>? nameForValue.

Under EnumStorage.AsString both rendered every value with ToString() — the member's declared
name, which stops being the stored name the moment somebody renames the member with
[JsonStringEnumMemberName] (System.Text.Json) or [EnumMember] (Newtonsoft). The filter then
compares against a name that is not in the data, matches nothing, and reports it as no rows rather
than as an error.

This is the collection-shaped half of JasperFx/marten#5376. Marten's own EnumAsStringMember covers
== / !=; every in / Contains shape funnels into these two fragments instead — IsOneOf,
IsNotOneOf, both EnumerableContains branches, and MemoryExtensionsContains — and kept the bug.

Passing nothing keeps the previous behaviour byte for byte, which is why the existing three-argument
constructors are unchanged and delegate. A delegate rather than an ISerializer keeps
Weasel.Postgresql.SqlGeneration free of a serialization dependency — and Weasel.Core.ISerializer
could not have served anyway, since it carries no enum rendering.

Worth recording because it is the tempting fix: reflecting over the enum's fields for the rename
attribute is not a substitute. It agrees with the serializer under a JIT and finds no attribute at
all in a trimmed Native AOT binary, so it would quietly fall back to the declared name in exactly the
deployment where the symptom is hardest to diagnose.

Verification

  • Solution-wide dotnet build: 0 errors. The only in-repo implementer of IDocumentStorage<T>
    (NoopDocumentStorage<T> in Weasel.Benchmarks) compiles untouched, which is the default-interface-
    member guarantee doing its job.
  • EnumIsOneOfWhereFragmentTests — 7 new tests, green: both fragments, with and without the renderer,
    AsString and AsInteger, plus the AsInteger path asserting the renderer is never consulted and
    the IsOneOf null entry still taking the is null branch without being rendered.

Not here

The version bump — that belongs to the release commit, following the convention in this repo's
history. Suggested release is 9.32.0 (additive minor); docs/release-9-32.md is written and
registered in the sidebar.

🤖 Generated with Claude Code

https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3

… enum in-fragments be told the stored name

Two additive seams a document store can opt into, both defaulting to today's
behaviour. Neither changes what a migration does to an existing database.

IDocumentStorage<T> gains MappedVersionFor / MappedRevisionFor as default
interface members returning null. A session needs the expected version for a
write's concurrency guard. It can test a document for a versioned marker
interface directly, but a document that instead maps a plain member as its
version carries the same information somewhere the session cannot reach, since
it holds the storage only as IDocumentStorage<T>. In Marten (marten#5372) the
upsert therefore bound DBNull into its WHERE ... mt_version = ? guard and
reported every cross-session write of such a document as a ConcurrencyException
-- a documented feature that could not do the one thing it is named for. The
alternatives all needed a runtime type test on the storage plus explicit
forwarding through each decorator, which fails silently when a decorator is
missed; that is the wrong failure mode for a concurrency guard.

EnumIsOneOfWhereFragment and EnumIsNotOneOfWhereFragment gain a constructor
overload taking an optional Func<object, string>. Under EnumStorage.AsString
both rendered every value with ToString(), which is the member's declared name
-- and that stops being the stored name as soon as the member is renamed with
[JsonStringEnumMemberName] or [EnumMember]. The filter then compares against a
name that is not in the data, matches nothing, and reports it as no rows rather
than as an error. Only the serializer knows the name it wrote, so the caller
supplies it; passing nothing keeps the previous behaviour byte for byte, which
is why the three-argument constructors are unchanged. A delegate rather than an
ISerializer keeps SqlGeneration free of a serialization dependency, and
Weasel.Core.ISerializer carries no enum rendering to reuse anyway.

Reflecting over the enum's fields for the rename attribute is not a substitute,
which is worth recording because it is the tempting fix: it agrees with the
serializer under a JIT and finds no attribute at all in a trimmed Native AOT
binary, so it would quietly fall back to the declared name in exactly the
deployment where the symptom is hardest to diagnose.

The version bump is deliberately not here -- it belongs to the release commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
@jeremydmiller
jeremydmiller merged commit e98197e into master Sep 12, 2026
18 checks passed
@jeremydmiller
jeremydmiller deleted the feat/mapped-version-member-seam branch September 12, 2026 13:46
jeremydmiller added a commit that referenced this pull request Sep 12, 2026
Two additive seams a document store can opt into (#592), closing #590 and #591:
IDocumentStorage<T>.MappedVersionFor / MappedRevisionFor, and an optional
stored-name renderer on EnumIsOneOfWhereFragment / EnumIsNotOneOfWhereFragment.
Both default to the previous behaviour. See docs/release-9-32.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant