Skip to content

GH-4439: natural keys resolve through the store the chain is routed to - #4442

Merged
jeremydmiller merged 1 commit into
mainfrom
gh-4439-natural-key-ancillary-store
Sep 14, 2026
Merged

jeremydmiller merged 1 commit into
mainfrom
gh-4439-natural-key-ancillary-store

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Fixes #4439.

The bug

An aggregate identified by [NaturalKey] and registered only on an ancillary store failed codegen with Unable to determine an aggregate id for the parameter 'order'. The identical handler against the default store worked, which is what makes this a store-routing bug rather than a natural-key one.

Natural-key detection was the last step of the aggregate handler workflow still asking the default store by name:

if (container.GetInstance<IDocumentStore>().Options is not StoreOptions storeOptions) return null;
return storeOptions.Projections.FindNaturalKeyDefinition(aggregateType)?.OuterType;

FindNaturalKeyDefinition searches one store's registered projections, so for an aggregate that lives only on an ancillary store it returned null, the natural-key branch was silently skipped, and the workflow blamed the handler signature. Everything downstream was already store-correct — LoadAggregateFrame writes the fetch against whatever session the chain has.

The fix

IEventSourcingFrameProvider.TryDetermineNaturalKeyType now takes the IChain, and all three stores move together — Marten, Polecat and Fisher resolve the chain's store before falling back to the default one.

Reading chain.AncillaryStoreType directly is not enough, and this is the part worth reviewing:

  • On a handler chain, parameter attributes run before chain attributes (HandlerChain.applyCustomizations: "THIS has to go before the baseline attributes"). The property is non-null only because the Phase-A eager policies pre-assign it at HandlerGraph.Compile.
  • On an HTTP chain it is still null. HttpChain.MapToRoute triggers parameter matching from the [WolverinePost] attribute in the constructor, long before applyAttributesAndConfigureMethods applies [MartenStore] — and the eager policies are IHandlerPolicy, so they never see an endpoint.

So this adds IAncillaryStoreAttribute (implemented by [Storage], [MartenStore], [PolecatStore], [FisherStore] — all four already had an identical StoreType property and no common type) plus chain.DetermineAncillaryStoreType(), which prefers the assigned property and otherwise reads the attribute off the handler. That answers for both chain types without reordering HTTP chain construction, which the ordering comments there warn against.

Two further defects found, both Fisher

  1. Fisher's natural-key workflow had never worked, on any store. TryDetermineNaturalKeyType was a stub returning null on the claim that Fisher has no natural-key concept — untrue since fisher#40 — which left the IsNaturalKey branch that Fisher's LoadAggregateFrame has always carried unreachable. There were no Fisher natural-key tests anywhere. Core's single generated spelling turns out to be correct for Fisher as-is: its FetchForWriting<T, TId> routes to FetchForWritingByNaturalKey when the aggregate declares a key. Definitions are read through public API (Projections.All + IAggregateProjection.NaturalKeyDefinition), because Fisher's own NaturalKeyFor is internal where Marten and Polecat expose a public FindNaturalKeyDefinition.
  2. FisherStoreEagerPolicy was dead code — nothing ever registered it, where Marten registers its twin at MartenIntegration.cs:101 and Polecat at PolecatIntegration.cs:127. [FisherStore] handlers therefore reached codegen with a null AncillaryStoreType, which also means Message from external system is stored in main store instead of ancillary store (ignoring [MartenStore] attribute at the message handler) #2944 interop messages landed in the main store's inbox. It hid because every existing Fisher ancillary test uses [Storage], which core's always-registered StorageAttributeEagerPolicy serves.

Tests

Red-baselined first in every case — the fix was reverted and each suite confirmed failing with the reported exception before being made green.

Suite Coverage
MartenTests/AncillaryStores ancillary natural key, [WriteAggregate] + [WriteModel], asserts the main store never saw the stream
PolecatTests/AncillaryStores same, Polecat
FisherTests ancillary and main-store natural key — the first Fisher natural-key coverage in Wolverine
Wolverine.Http.Tests/Marten end-to-end HTTP: seeded stream, real POST, asserts the document actually changed

The HTTP endpoints live in a new Wolverine.Http.Tests.AncillaryStore assembly referenced only by Wolverine.Http.Tests. Wolverine.Http discovers endpoints eagerly, so an endpoint whose parameter matching needs a store the host has not registered throws from HttpGraph.DiscoverEndpoints and kills host construction — for every host that scans its assembly. Putting these endpoints in WolverineWebApi (pinned as the application assembly by AppFixture, a process-wide static) took 129 unrelated tests down; putting them in Wolverine.Http.Tests itself, where 37 suites build hosts with IncludeAssembly(GetType().Assembly), took down 144. A filtered run showed neither. CIHttp picks the new project up through the existing ProjectReference — it builds Wolverine.Http.Tests.csproj and its whole reference graph.

Verified on this branch: Marten 8/8, Polecat 5/5 (twice, since its failure mode is rerun-only), Fisher 4/4, full Wolverine.Http.Tests 0 failed / 1048 passed against a pristine-main baseline of 0 failed / 1046 passed, and dotnet build wolverine.slnx -c Release -f net9.0 clean at 0 warnings / 0 errors.

Review notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

An aggregate identified by [NaturalKey] and registered only on an ancillary store
failed codegen with "Unable to determine an aggregate id". Natural-key detection
was the last step of the aggregate handler workflow still asking the DEFAULT store
by name, and FindNaturalKeyDefinition searches one store's registered projections,
so the key was never found and the natural-key branch was silently skipped. The
same handler on the default store worked, which is what makes this a store-routing
bug rather than a natural-key one.

IEventSourcingFrameProvider.TryDetermineNaturalKeyType now takes the IChain, and
all three stores resolve the chain's store before falling back to the default one.

Reading chain.AncillaryStoreType directly is not enough. On a handler chain it is
populated only by the Phase-A eager policies, and on an HTTP chain it is still
null: HttpChain.MapToRoute runs parameter matching from the [WolverinePost]
attribute in the constructor, before [MartenStore] is applied, and the eager
policies are IHandlerPolicy so they never see an endpoint. The new
IAncillaryStoreAttribute marker -- implemented by [Storage], [MartenStore],
[PolecatStore] and [FisherStore] -- plus chain.DetermineAncillaryStoreType()
answers correctly for both chain types without reordering HTTP chain construction.

Two further defects surfaced and are fixed here, both Fisher:

* Fisher's natural-key aggregate workflow had never worked, on any store. Its
  TryDetermineNaturalKeyType was a stub returning null on the claim that Fisher has
  no natural-key concept -- untrue since fisher#40 -- which left the IsNaturalKey
  branch its LoadAggregateFrame has always carried unreachable. There were no
  Fisher natural-key tests anywhere, so nothing exercised it.
* FisherStoreEagerPolicy was dead code: nothing ever registered it, where Marten
  and Polecat register their twins. [FisherStore] handlers therefore reached
  codegen with a null AncillaryStoreType, which also routed GH-2944 interop
  messages to the MAIN store's inbox. It hid because every existing Fisher
  ancillary test used [Storage], which core's always-registered
  StorageAttributeEagerPolicy serves.

Tests cover the ancillary case on Marten, Polecat and Fisher, the Fisher
main-store case (the first Fisher natural-key coverage in Wolverine), and an
end-to-end HTTP endpoint. The HTTP endpoints live in a new
Wolverine.Http.Tests.AncillaryStore assembly referenced only by
Wolverine.Http.Tests: Wolverine.Http discovers endpoints eagerly, so an endpoint
whose parameter matching needs a store the host has not registered throws from
HttpGraph.DiscoverEndpoints and kills host construction for every host that scans
its assembly. Putting these endpoints in WolverineWebApi or in Wolverine.Http.Tests
itself took roughly 130 unrelated tests down.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
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.

Natural key on an aggregate in an ancillary Marten store: [WriteModel] / [WriteAggregate] cannot determine the aggregate id

1 participant