fix(#569): bind IEvent<T> natural key sources, stop fabricating aggregates, fail loudly - #571
Merged
Merged
Conversation
…gates, fail loudly [NaturalKeySource] discovery silently dropped IEvent<T> handlers and invoked user Apply methods against a fabricated blank aggregate, which threw at append time. Reported downstream as JasperFx/marten#5041. Widen the extraction contract from event data to the event. NaturalKeyEventMapping .Extractor is now Func<IEvent, object?> rather than Func<object, object?>. That is what makes an IEvent<T> handler bindable at all — buildExtractor used to set canCall = false for an IEvent parameter because it had no IEvent to pass, then fall through to a property-matching fallback that cannot match a strong-typed key carried on the event as a string. The result was a mapping that was never registered, so the natural key lookup table was simply never written for that event type: FetchLatest returning null after a rename, live and on rebuild alike. It also makes a key derived from event metadata expressible. BREAKING for the one consumer: Marten's NaturalKeyProjection calls mapping.Extractor(@event.Data) at both call sites and must pass @event instead. Both already hold the real IEvent. Rank the extraction strategies, most trustworthy first: 1. a static [NaturalKeySource] returning the natural key type — a pure function of the event, so nothing is fabricated and no user aggregation code runs; 2. a property of the key's type carried on the event body — and only when it is unambiguous, since "the first property of the right type" answers with the OLD key on an event that carries both; 3. invoking the user's method against a fabricated aggregate. (3) stays last and is now gated on the aggregate being safely constructible. Expression.New bypasses required-member enforcement, so it handed the user's method an aggregate C# itself would never have let them create — the marten#5041 ArgumentNullException, thrown out of the extractor, out of the inline natural key maintenance, and out of the caller's SaveChangesAsync. It also reads the key off what an evolve method returned rather than off the blank aggregate. Make NaturalKeyBuilder reachable. Its constructor was internal and nothing in JasperFx.Events or Marten ever constructed it, so SetBy — exactly the escape hatch a user needs when discovery cannot bind their method — was unreachable dead code. It is public now, with a SetByEvent overload for metadata-derived keys, and NaturalKeyFor() on the aggregation projection to reach it. An explicit registration replaces a discovered mapping for the same event type. Fail loudly at configuration time. Discovery used to `catch { }` and drop a null extractor with no diagnostic, so a user annotated their methods, got no warning of any kind, and found out at runtime. Unbindable methods are recorded as NaturalKeyDefinition.DiscoveryProblems and AssembleAndAssertValidity throws an InvalidProjectionException naming the method, the reason, and the two supported ways to fix it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QYfvCxHCUHo9MriHQocuoD
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #569. Fixes the upstream half of JasperFx/marten#5041.
Important
Breaking for both downstream event stores.
NaturalKeyEventMapping.Extractoris nowFunc<IEvent, object?>instead ofFunc<object, object?>. #569 was written believing Marten was the only consumer; Polecat maintains its ownpc_natural_key_Xlookup table off the sameNaturalKeyDefinition. Every call site already holds the realIEvent, so it is a one-line change in each:NaturalKeyProjection.ApplyAsyncandNaturalKeyProjection.QueueUpsertsForEvents:mapping.Extractor(@event.Data)→mapping.Extractor(@event). Tracked in JasperFx/marten#5052.NaturalKeyProjection.QueueOperationForEvent(shared by the inline append and rebuild paths):mapping.Extractor(e.Data)→mapping.Extractor(e). Tracked in JasperFx/polecat#369.Bug 1 —
IEvent<T>parameters produced no mapping, silentlydiscoverNaturalKeySourceMethodscorrectly unwrappedIEvent<T>to determine the event type, butbuildExtractorcould not build anything for it: the extractor only received the event data, so there was noIEventto hand the method, and the static branch explicitly setcanCall = falsefor anIEventparameter. Control fell to the property-matching fallback, which cannot match a strong-typed key (Code) carried on the event as astring, so it returned null — and a null extractor was skipped with no error, no log, and no configuration-time validation.The fix is the issue's suggested direction 1: widen the contract from the event data to the event. That makes
IEvent<T>— a first class signature everywhere else in aggregation discovery — directly bindable rather than fixed by synthesizing a fake wrapper, and it makes a key derived from event metadata (stream key, timestamp, headers) expressible at all.Bug 2 — extractors invoked user
Applymethods against a fabricated blank aggregateThe extraction strategies are now ranked, most trustworthy first:
[NaturalKeySource]whose return type is the natural key type. A pure function of the event, so nothing is fabricated and no user aggregation code runs:Expression.Newbypasses required-member enforcement, which is how the user's method got an aggregate C# itself would never have let them create, and how a plain event append ended inArgumentNullExceptionout ofSaveChangesAsync. This path also now reads the key off what an evolve method returned rather than off the blank aggregate.The hard constraint stays and is now documented on the contract: the key is derived from the event alone. Marten maintains the lookup table inline at append time, where no prior aggregate exists under an
Asyncsnapshot lifecycle, so a key that depends on prior aggregate state is not expressible here.Bug 3 — no supported escape hatch
NaturalKeyBuilder<TDoc>.SetBy<TEvent>was exactly the explicit registration a user needs when discovery cannot bind their method — and its constructor wasinternal, with nothing in JasperFx.Events or Marten ever constructing it. Unreachable dead code.It is public now, with a
SetByEvent<TEvent>(Func<IEvent<TEvent>, object?>)overload for metadata-derived keys, reachable fromNaturalKeyFor()on the aggregation projection:An explicit registration replaces whatever discovery found for the same event type, and clears the configuration-time error an unbindable method would otherwise raise.
Bug 4 — failures were swallowed
discoverNaturalKeySourceMethodswrapped extractor construction incatch { /* Silently skip */ }and dropped null extractors with no diagnostic. A user annotated their methods, got no warning of any kind, and discovered at runtime that natural key lookups returnednull.Unbindable methods are now recorded as
NaturalKeyDefinition.DiscoveryProblems, andAssembleAndAssertValidity()throws anInvalidProjectionExceptionnaming the method, the reason, and the two supported fixes:The check runs ahead of the source-generated short circuit, since natural key discovery is independent of how the aggregation itself is dispatched.
Tests
NaturalKeySourceDiscoveryTestsgrows from 4 to 12. The four existing cases (instance method, marten#4277 self-aggregating factory, separate projection class, marten#4966 evolve-that-changes-the-key) still pass, updated for theIEventcontract. New coverage, using the repro shapes from the issue:ievent_handlers_produce_a_mapping— Bug 1a_key_extraction_method_needs_no_aggregate_at_all— the dedicated signature binds even for an aggregate that cannot be fabricated, including theIEvent<T>form readingStreamKeywill_not_invoke_a_handler_against_an_aggregate_it_cannot_safely_build— Bug 2, the required-member reproan_unbindable_source_method_fails_at_configuration_time— Bug 4an_explicit_registration_wins_and_clears_the_discovery_problem/..._replaces_a_discovered_mapping_for_the_same_event— Bug 3an_event_carrying_two_candidate_keys_is_not_resolved_by_declaration_ordera_source_method_with_no_event_parameter_is_reportedFull solution builds.
EventTests629 passed,EventStoreTests72 passed, 0 failed.🤖 Generated with Claude Code
https://claude.ai/code/session_01QYfvCxHCUHo9MriHQocuoD