diff --git a/src/EventTests/Projections/NaturalKeySourceDiscoveryTests.cs b/src/EventTests/Projections/NaturalKeySourceDiscoveryTests.cs index 312683a..1cf9d37 100644 --- a/src/EventTests/Projections/NaturalKeySourceDiscoveryTests.cs +++ b/src/EventTests/Projections/NaturalKeySourceDiscoveryTests.cs @@ -1,6 +1,7 @@ using System.Linq; using JasperFx.Events; using JasperFx.Events.Aggregation; +using JasperFx.Events.Projections; using Shouldly; namespace EventTests.Projections; @@ -19,7 +20,7 @@ public void discovers_natural_key_source_on_instance_method_of_aggregate() .SingleOrDefault(m => m.EventType == typeof(NkCreatedEvent)); mapping.ShouldNotBeNull(); - var extracted = mapping!.Extractor(new NkCreatedEvent("abc")); + var extracted = mapping!.Extractor(new Event(new NkCreatedEvent("abc"))); extracted.ShouldBe(new NkAggregateKey("abc")); } @@ -38,7 +39,7 @@ public void discovers_natural_key_source_on_static_factory_of_self_aggregating_a .SingleOrDefault(m => m.EventType == typeof(NkCreatedEvent)); mapping.ShouldNotBeNull(); - var extracted = mapping!.Extractor(new NkCreatedEvent("self-agg")); + var extracted = mapping!.Extractor(new Event(new NkCreatedEvent("self-agg"))); extracted.ShouldBe(new NkSelfAggregate(default, new NkAggregateKey("self-agg")).Key); } @@ -54,7 +55,9 @@ public void discovers_natural_key_source_on_static_method_of_separate_projection .SingleOrDefault(m => m.EventType == typeof(NkSeparateProjectionCreatedEvent)); mapping.ShouldNotBeNull(); - var extracted = mapping!.Extractor(new NkSeparateProjectionCreatedEvent(new NkAggregateKey("separate"))); + var extracted = mapping!.Extractor( + new Event( + new NkSeparateProjectionCreatedEvent(new NkAggregateKey("separate")))); extracted.ShouldBe(new NkAggregateKey("separate")); } @@ -76,13 +79,160 @@ public void discovers_natural_key_source_on_static_evolve_method_that_changes_th var created = projection.NaturalKeyDefinition!.EventMappings .SingleOrDefault(m => m.EventType == typeof(NkCreatedEvent)); created.ShouldNotBeNull(); - created!.Extractor(new NkCreatedEvent("first")).ShouldBe(new NkAggregateKey("first")); + created!.Extractor(new Event(new NkCreatedEvent("first"))) + .ShouldBe(new NkAggregateKey("first")); // ...and so does the two-arg evolve method that changes the key. var changed = projection.NaturalKeyDefinition.EventMappings .SingleOrDefault(m => m.EventType == typeof(NkKeyChangedEvent)); changed.ShouldNotBeNull(); - changed!.Extractor(new NkKeyChangedEvent("second")).ShouldBe(new NkAggregateKey("second")); + changed!.Extractor(new Event(new NkKeyChangedEvent("second"))) + .ShouldBe(new NkAggregateKey("second")); + } + + // ───────────────── jasperfx#569 / marten#5041 ───────────────── + + [Fact] + public void ievent_handlers_produce_a_mapping() + { + // Bug 1. IEvent is a first class signature everywhere else in aggregation discovery, but + // buildExtractor explicitly gave up on it: the extractor only received the event DATA, so + // there was no IEvent to hand the method. Control fell through to the property-matching + // fallback, which cannot match a strong-typed key carried on the event as a string, and the + // method was dropped with no error, no log, and no configuration-time validation. The + // downstream symptom was a natural key lookup table that was simply never written for that + // event type — FetchLatest returning null after a rename, live and on rebuild alike. + var projection = new NkIEventProjection(); + + projection.NaturalKeyDefinition.ShouldNotBeNull(); + projection.NaturalKeyDefinition!.DiscoveryProblems.ShouldBeEmpty(); + + var mapping = projection.NaturalKeyDefinition.EventMappings + .SingleOrDefault(m => m.EventType == typeof(NkCodeChangedViaEvent)); + mapping.ShouldNotBeNull(); + + mapping!.Extractor(new Event(new NkCodeChangedViaEvent("B"))) + .ShouldBe(new NkCode("B")); + } + + [Fact] + public void a_key_extraction_method_needs_no_aggregate_at_all() + { + // The dedicated signature: a static [NaturalKeySource] returning the natural key type is a + // pure function of the event, so it binds even for an aggregate that CANNOT be fabricated + // (this one declares a required member). That is the supported answer to Bug 2 — nothing + // constructs a blank aggregate and nothing runs the user's aggregation code. + var projection = new NkKeyFactoryProjection(); + + projection.NaturalKeyDefinition.ShouldNotBeNull(); + projection.NaturalKeyDefinition!.DiscoveryProblems.ShouldBeEmpty(); + + projection.NaturalKeyDefinition.EventMappings + .Single(m => m.EventType == typeof(NkRegistered)) + .Extractor(new Event(new NkRegistered("A"))) + .ShouldBe(new NkCode("A")); + + // ...and the same signature taking IEvent, so a key derived from event metadata rather + // than the body alone is expressible now that the extractor receives the whole event. + projection.NaturalKeyDefinition.EventMappings + .Single(m => m.EventType == typeof(NkCodeChangedViaEvent)) + .Extractor(new Event(new NkCodeChangedViaEvent("B")) { StreamKey = "s-1" }) + .ShouldBe(new NkCode("s-1/B")); + } + + [Fact] + public void will_not_invoke_a_handler_against_an_aggregate_it_cannot_safely_build() + { + // Bug 2. The fabricated-aggregate path emitted Expression.New(docType) and invoked the + // user's Apply against it — and Expression.New bypasses C# required-member enforcement, so + // History was null on an aggregate the user could never have constructed that way. The + // ArgumentNullException propagated out of the extractor, out of the inline natural key + // maintenance, and aborted a plain SaveChangesAsync. It is now refused up front. + var projection = new NkRequiredMembersProjection(); + + projection.NaturalKeyDefinition.ShouldNotBeNull(); + projection.NaturalKeyDefinition!.EventMappings.ShouldBeEmpty(); + + var problem = projection.NaturalKeyDefinition.DiscoveryProblems.ShouldHaveSingleItem(); + problem.Method.Name.ShouldBe(nameof(NkRequiredMembersAggregate.Apply)); + problem.EventType.ShouldBe(typeof(NkCodeChangedInline)); + problem.Reason.ShouldContain("required members"); + } + + [Fact] + public void an_unbindable_source_method_fails_at_configuration_time() + { + // Bug 3/4. Discovery used to `catch { }` and move on, so the user annotated their methods, + // got no warning of any kind, and found out at runtime. Name the method and the reason + // while the projection is being registered instead. + var ex = Should.Throw( + () => new NkRequiredMembersProjection().AssembleAndAssertValidity()); + + ex.Message.ShouldContain(nameof(NkRequiredMembersAggregate.Apply)); + ex.Message.ShouldContain("required members"); + ex.Message.ShouldContain("NaturalKeyFor"); + } + + [Fact] + public void an_explicit_registration_wins_and_clears_the_discovery_problem() + { + // Bug 3. NaturalKeyBuilder.SetBy was exactly the escape hatch a user needed when discovery + // could not bind their method — and its constructor was internal, with nothing in + // JasperFx.Events or Marten ever constructing it. It is reachable now, and an explicit + // registration both overrides discovery and satisfies validation. + var projection = new NkExplicitlyConfiguredProjection(); + + projection.NaturalKeyDefinition.ShouldNotBeNull(); + projection.NaturalKeyDefinition!.DiscoveryProblems.ShouldBeEmpty(); + + projection.NaturalKeyDefinition.EventMappings + .Single(m => m.EventType == typeof(NkCodeChangedInline)) + .Extractor(new Event(new NkCodeChangedInline("C"))) + .ShouldBe(new NkCode("C")); + + Should.NotThrow(() => projection.AssembleAndAssertValidity()); + } + + [Fact] + public void an_explicit_registration_replaces_a_discovered_mapping_for_the_same_event() + { + var projection = new NkEvolvingKeyProjection(); + projection.NaturalKeyFor(x => x.SetBy(e => new NkAggregateKey("overridden:" + e.Key))); + + projection.NaturalKeyDefinition!.EventMappings + .Count(m => m.EventType == typeof(NkCreatedEvent)).ShouldBe(1); + + projection.NaturalKeyDefinition.EventMappings + .Single(m => m.EventType == typeof(NkCreatedEvent)) + .Extractor(new Event(new NkCreatedEvent("first"))) + .ShouldBe(new NkAggregateKey("overridden:first")); + } + + [Fact] + public void an_event_carrying_two_candidate_keys_is_not_resolved_by_declaration_order() + { + // Reading a matching property off the event body is preferred over invoking user code, but + // "the first property of the key's type" is a guess. When an event carries both the old and + // the new key, defer to the method that actually knows which is which. + var projection = new NkSwapProjection(); + + projection.NaturalKeyDefinition.ShouldNotBeNull(); + projection.NaturalKeyDefinition!.DiscoveryProblems.ShouldBeEmpty(); + + projection.NaturalKeyDefinition.EventMappings + .Single(m => m.EventType == typeof(NkCodeSwapped)) + .Extractor(new Event(new NkCodeSwapped(new NkCode("old"), new NkCode("new")))) + .ShouldBe(new NkCode("new")); + } + + [Fact] + public void a_source_method_with_no_event_parameter_is_reported() + { + var projection = new NkNoEventParameterProjection(); + + var problem = projection.NaturalKeyDefinition!.DiscoveryProblems.ShouldHaveSingleItem(); + problem.Method.Name.ShouldBe(nameof(NkNoEventParameterAggregate.Whoops)); + problem.Reason.ShouldContain("no event parameter"); } } @@ -161,3 +311,146 @@ public static NkEvolvingKeyAggregate Apply(NkKeyChangedEvent e, NkEvolvingKeyAgg public class NkEvolvingKeyProjection : SingleStreamProjection { } + +// ───────── jasperfx#569 fixtures — the marten#5041 repro shapes ───────── + +public sealed record NkCode(string Value); + +public record NkRegistered(string Code); + +public record NkCodeChangedViaEvent(string NewCode); + +public record NkCodeChangedInline(string NewCode); + +public sealed record NkIEventAggregate +{ + public Guid Id { get; set; } + + [NaturalKey] + public NkCode Code { get; set; } = default!; + + [NaturalKeySource] + public static NkIEventAggregate Create(NkRegistered e) => new() { Code = new NkCode(e.Code) }; + + [NaturalKeySource] + public static NkIEventAggregate Apply(IEvent e, NkIEventAggregate current) + => current with { Code = new NkCode(e.Data.NewCode) }; +} + +public class NkIEventProjection : SingleStreamProjection +{ +} + +// An aggregate that CANNOT be fabricated — it declares a required member, so Expression.New would +// hand the user's method an aggregate C# itself would never have let them create. +public sealed record NkKeyFactoryAggregate +{ + public Guid Id { get; set; } + + [NaturalKey] + public NkCode Code { get; set; } = default!; + + public required IEnumerable History { get; set; } + + [NaturalKeySource] + public static NkCode KeyFor(NkRegistered e) => new(e.Code); + + [NaturalKeySource] + public static NkCode KeyFor(IEvent e) => new($"{e.StreamKey}/{e.Data.NewCode}"); +} + +public class NkKeyFactoryProjection : SingleStreamProjection +{ +} + +// The reported repro: an instance Apply on a required-member aggregate whose body touches state +// other than the key. Fabricating a blank aggregate here threw ArgumentNullException out of a +// plain event append. +public sealed record NkRequiredMembersAggregate +{ + public Guid Id { get; set; } + + [NaturalKey] + public NkCode Code { get; set; } = default!; + + public required IEnumerable History { get; set; } + + [NaturalKeySource] + public void Apply(NkCodeChangedInline e) + { + Code = new NkCode(e.NewCode); + History = History.Append(Code); + } +} + +public class NkRequiredMembersProjection : SingleStreamProjection +{ +} + +// Same unbindable shape, but the projection registers the mapping explicitly. The [NaturalKeySource] +// method is deliberately not named Apply/Create so that the aggregation side of validation is +// unambiguous and this test is only about the natural key. +public sealed record NkExplicitlyConfiguredAggregate +{ + public Guid Id { get; set; } + + [NaturalKey] + public NkCode Code { get; set; } = default!; + + public required IEnumerable History { get; set; } + + [NaturalKeySource] + public void RenameTo(NkCodeChangedInline e) + { + Code = new NkCode(e.NewCode); + History = History.Append(Code); + } +} + +public class NkExplicitlyConfiguredProjection : SingleStreamProjection +{ + public NkExplicitlyConfiguredProjection() + { + NaturalKeyFor(x => x.SetBy(e => new NkCode(e.NewCode))); + } + + public override NkExplicitlyConfiguredAggregate? Evolve(NkExplicitlyConfiguredAggregate? snapshot, Guid id, + IEvent e) => snapshot; +} + +// An event carrying both the old and the new key — "first property of the right type" would answer +// with the old one. +public record NkCodeSwapped(NkCode Previous, NkCode Next); + +public sealed record NkSwapAggregate +{ + public Guid Id { get; set; } + + [NaturalKey] + public NkCode Code { get; set; } = default!; + + [NaturalKeySource] + public static NkSwapAggregate Apply(NkCodeSwapped e, NkSwapAggregate current) + => current with { Code = e.Next }; +} + +public class NkSwapProjection : SingleStreamProjection +{ +} + +public sealed record NkNoEventParameterAggregate +{ + public Guid Id { get; set; } + + [NaturalKey] + public NkCode Code { get; set; } = default!; + + [NaturalKeySource] + public void Whoops() + { + } +} + +public class NkNoEventParameterProjection : SingleStreamProjection +{ +} diff --git a/src/JasperFx.Events/Aggregation/JasperFxAggregationProjectionBase.cs b/src/JasperFx.Events/Aggregation/JasperFxAggregationProjectionBase.cs index 65e94ab..c8165ed 100644 --- a/src/JasperFx.Events/Aggregation/JasperFxAggregationProjectionBase.cs +++ b/src/JasperFx.Events/Aggregation/JasperFxAggregationProjectionBase.cs @@ -1,6 +1,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.CompilerServices; using JasperFx.Core; using JasperFx.Core.Reflection; using JasperFx.Events.Daemon; @@ -72,6 +73,49 @@ protected JasperFxAggregationProjectionBase(AggregationScope scope) public NaturalKeyDefinition? NaturalKeyDefinition { get; } + /// + /// Explicitly register how the natural key of is derived from events, + /// bypassing (or correcting) [NaturalKeySource] attribute discovery: + /// + /// NaturalKeyFor(x => x + /// .SetBy<ProductRegistered>(e => new ProductCode(e.Code)) + /// .SetByEvent<ProductCodeChanged>(e => new ProductCode(e.Data.NewCode))); + /// + /// An explicit registration replaces whatever discovery found for the same event type, and clears + /// the configuration-time error a method that could not be bound would otherwise raise. The key + /// has to be a function of the event alone — the lookup table is maintained inline as events are + /// appended, where no prior aggregate exists. See jasperfx#569. + /// + public void NaturalKeyFor(Action> configure) + { + if (NaturalKeyDefinition == null) + { + throw new InvalidProjectionException( + $"{typeof(TDoc).FullNameInCode()} has no property marked with [NaturalKey], so there is no natural key to configure."); + } + + configure(new NaturalKeyBuilder(NaturalKeyDefinition)); + } + + /// + /// jasperfx#569: a [NaturalKeySource] method that discovery could not turn into a key extraction + /// used to be swallowed whole — no mapping, no log, no error — and the user found out when the + /// natural key lookup silently returned null for events of that type. Fail at configuration time + /// instead, naming the method and the reason. + /// + private void assertNaturalKeyValidity() + { + if (NaturalKeyDefinition == null || NaturalKeyDefinition.DiscoveryProblems.Count == 0) return; + + var problems = NaturalKeyDefinition.DiscoveryProblems + .Select(x => " * " + x) + .Join(System.Environment.NewLine); + + throw new InvalidProjectionException( + $"Unable to derive the natural key '{typeof(TDoc).FullNameInCode()}.{NaturalKeyDefinition.Member.Name}' from every [NaturalKeySource] method on {GetType().FullNameInCode()}:{System.Environment.NewLine}{problems}{System.Environment.NewLine}" + + $"Either give the method a signature that derives the key from the event alone — a static method returning {NaturalKeyDefinition.OuterType.FullNameInCode()} that takes the event or IEvent — or register the mapping explicitly with NaturalKeyFor(x => x.SetBy(...))."); + } + public Type ImplementationType => GetType(); public SubscriptionType Type { get; } public ShardName[] ShardNames() => [ShardName.Compose(Name, version: Version)]; @@ -378,6 +422,10 @@ private IEnumerable collectGeneratedEvolverAttributes public override void AssembleAndAssertValidity() { + // Ahead of everything else, and outside the source-generated short circuit below: natural key + // discovery is independent of how the aggregation itself is dispatched. + assertNaturalKeyValidity(); + // If a source-generated evolver was found (either for Apply/Create or Evolve/EvolveAsync), // skip conventional method validation — the evolver handles everything if (_generatedEvolverEventTypes != null) @@ -673,6 +721,9 @@ public virtual Task EnrichEventsAsync(SliceGroup group, TQuerySession return definition; } + private static readonly PropertyInfo _eventDataProperty = + typeof(IEvent).GetProperty(nameof(IEvent.Data))!; + private static void discoverNaturalKeySourceMethods( NaturalKeyDefinition definition, PropertyInfo naturalKeyProp, @@ -686,156 +737,344 @@ private static void discoverNaturalKeySourceMethods( foreach (var method in methods) { var parameters = method.GetParameters(); - if (parameters.Length == 0) continue; - - // Determine the event type from the first parameter. - // It can be the raw event type or IEvent. - var firstParamType = parameters[0].ParameterType; - Type eventType; - if (firstParamType.IsGenericType && - firstParamType.GetGenericTypeDefinition() == typeof(IEvent<>)) - { - eventType = firstParamType.GetGenericArguments()[0]; - } - else if (typeof(IEvent).IsAssignableFrom(firstParamType)) - { - eventType = firstParamType; - } - else + + var eventType = determineEventType(parameters, docType); + if (eventType == null) { - eventType = firstParamType; + definition.RecordProblem(method, method.DeclaringType ?? searchType, + "no event parameter could be identified — a [NaturalKeySource] method has to accept the event, either as the event type itself or as IEvent"); + continue; } // Skip if we already have a mapping for this event type - if (definition.EventMappings.Any(m => m.EventType == eventType)) - continue; + if (definition.HasMappingFor(eventType)) continue; try { - var extractor = buildExtractor(method, naturalKeyProp, docType, parameters); + var extractor = buildExtractor(method, naturalKeyProp, docType, parameters, eventType, + out var reason); if (extractor != null) { - definition.EventMappings.Add(new NaturalKeyEventMapping(eventType, extractor)); + definition.AddOrReplaceMapping(eventType, extractor); } + else + { + definition.RecordProblem(method, eventType, reason!); + } + } + catch (Exception e) + { + // jasperfx#569: this used to be a bare `catch { }`, so a method that could not be bound + // produced no mapping, no log, and no configuration-time error — the natural key lookup + // table was simply never written for that event type, and the user found out when + // FetchLatest returned null. Keep the failure, and make validation report it. + definition.RecordProblem(method, eventType, + $"building a key extraction failed with {e.GetType().Name}: {e.Message}"); } - catch + } + } + + /// + /// Which event does this [NaturalKeySource] method handle? The event can arrive as the event type + /// itself or as IEvent<T>, and it is not necessarily the first parameter — an evolve method + /// may take the prior aggregate first. + /// + private static Type? determineEventType(ParameterInfo[] parameters, Type docType) + { + foreach (var parameter in parameters) + { + var parameterType = parameter.ParameterType; + + if (parameterType.IsGenericType && parameterType.GetGenericTypeDefinition() == typeof(IEvent<>)) { - // Silently skip methods we can't build extractors for + return parameterType.GetGenericArguments()[0]; } + + // The prior aggregate, and a bare IEvent, both say nothing about which event this handles. + if (parameterType == docType || parameterType == typeof(IEvent)) continue; + + return parameterType; } + + return null; } - private static Func? buildExtractor( + /// + /// Compile the "derive this event's natural key value" function for one [NaturalKeySource] method. + /// The extractor receives the whole (jasperfx#569), which is what makes an + /// IEvent<T> handler bindable at all. Preference order, most trustworthy first: + /// 1. a static method that IS a key extraction — it returns the natural key type and is a pure + /// function of the event, so nothing has to be fabricated; + /// 2. a property of the natural key type carried directly on the event body; + /// 3. invoking the user's aggregation method against a fabricated blank aggregate. + /// (3) is the legacy path and stays last for a reason: it runs user code against an aggregate that + /// was never built by any constructor the user wrote, so a handler body that touches any state other + /// than the key throws — out of the extractor, out of the inline natural key maintenance, and out of + /// the caller's SaveChangesAsync. It is now gated on the aggregate being safely constructible. Note + /// that whatever path is used, the key is derived from the event alone: Marten maintains the lookup + /// table inline at append time, where no prior aggregate exists under an Async snapshot lifecycle, so + /// a key that depends on prior aggregate state is not expressible here. + /// + private static Func? buildExtractor( MethodInfo method, PropertyInfo naturalKeyProp, Type docType, - ParameterInfo[] parameters) - { - var eventParam = Expression.Parameter(typeof(object), "e"); - var firstParamType = parameters[0].ParameterType; - - // For instance methods on the aggregate (the original working pattern): - // Create a new TDoc, call the method, read the natural key property - if (!method.IsStatic && method.DeclaringType == docType) - { - var eventType = firstParamType; - var docParam = Expression.Variable(docType, "doc"); - - var body = Expression.Block( - [docParam], - Expression.Assign(docParam, Expression.New(docType)), - Expression.Call(docParam, method, Expression.Convert(eventParam, eventType)), - Expression.Convert(Expression.Property(docParam, naturalKeyProp), typeof(object)) - ); - - return Expression.Lambda>(body, eventParam).Compile(); - } - - // For static methods on the aggregate itself that return a TDoc, call the method - // and read the natural key property off the returned aggregate. This covers BOTH: - // * the self-aggregating create factory (JasperFx/marten#4277): - // public static TDoc Create(TEvent e) => new TDoc(...); - // * an evolve/update method that CHANGES the natural key (JasperFx/marten#4966): - // public static TDoc Apply(TEvent e, TDoc current) => current with { Key = ... }; - // Build one argument per parameter: the event parameter receives the raw event data - // (converted); a TDoc parameter (the prior aggregate in an evolve method) receives a - // fresh default aggregate, mirroring the instance-method branch above. Only the event - // data reaches the extractor (NaturalKeyProjection passes @event.Data), so a parameter - // that needs IEvent metadata — or a doc type without a public parameterless ctor — - // can't be satisfied here; in that case fall through to the property-matching fallback. - if (method.IsStatic && method.DeclaringType == docType && method.ReturnType == docType) - { - var callArgs = new Expression[parameters.Length]; - var eventArgBound = false; - var canCall = true; - - for (var i = 0; i < parameters.Length; i++) + ParameterInfo[] parameters, + Type eventType, + out string? reason) + { + reason = null; + + var keyType = naturalKeyProp.PropertyType; + var eventParam = Expression.Parameter(typeof(IEvent), "e"); + var blockers = new List(); + + // 1. The dedicated key extraction signature: static, returns the natural key type, and every + // parameter comes off the event. Nothing is fabricated and no user aggregation code runs. + // [NaturalKeySource] public static Code KeyFor(CodeChanged e) => new Code(e.NewCode); + // [NaturalKeySource] public static Code KeyFor(IEvent e) => ...; + if (method.IsStatic && method.ReturnType == keyType) + { + if (tryBindArguments(eventParam, parameters, eventType, docType, null, out var keyArgs, + out var blocker)) { - var paramType = parameters[i].ParameterType; - var isIEvent = paramType.IsGenericType - && paramType.GetGenericTypeDefinition() == typeof(IEvent<>); + return compile(Expression.Convert(Expression.Call(method, keyArgs!), typeof(object)), + eventParam); + } - if (paramType == docType && docType.GetConstructor(System.Type.EmptyTypes) != null) - { - callArgs[i] = Expression.New(docType); - } - else if (!eventArgBound && paramType != docType && !isIEvent) - { - callArgs[i] = Expression.Convert(eventParam, paramType); - eventArgBound = true; - } - else + blockers.Add(blocker!); + } + + // 2. The natural key value carried directly on the event body. + var fromEventBody = tryReadKeyOffTheEvent(eventParam, eventType, naturalKeyProp, out var matchBlocker); + if (fromEventBody != null) + { + return compile(fromEventBody, eventParam); + } + + if (matchBlocker != null) blockers.Add(matchBlocker); + + // 3. Last resort: run the user's aggregation method against a fabricated aggregate. + var fabricated = tryInvokeAgainstFabricatedAggregate(method, naturalKeyProp, docType, parameters, + eventType, eventParam, out var fabricationBlocker); + if (fabricated != null) + { + return compile(fabricated, eventParam); + } + + if (fabricationBlocker != null) blockers.Add(fabricationBlocker); + + reason = blockers.Any() + ? blockers.Join("; ") + : $"no way to derive a {keyType.FullNameInCode()} from a {eventType.FullNameInCode()} could be determined from this signature"; + + return null; + } + + /// + /// Bind one argument per parameter out of the the extractor is handed. + /// is non-null only on the fabricated-aggregate path; a + /// TDoc parameter is otherwise unbindable, which is exactly what keeps path (1) honest. + /// + private static bool tryBindArguments( + ParameterExpression eventParam, + ParameterInfo[] parameters, + Type eventType, + Type docType, + Expression? fabricatedAggregate, + out Expression[]? arguments, + out string? blocker) + { + var args = new Expression[parameters.Length]; + + for (var i = 0; i < parameters.Length; i++) + { + var parameterType = parameters[i].ParameterType; + + if (parameterType.IsGenericType && parameterType.GetGenericTypeDefinition() == typeof(IEvent<>)) + { + args[i] = Expression.Convert(eventParam, parameterType); + } + else if (parameterType == typeof(IEvent)) + { + args[i] = eventParam; + } + else if (parameterType == docType) + { + if (fabricatedAggregate == null) { - canCall = false; - break; + arguments = null; + blocker = + $"parameter '{parameters[i].Name}' is the prior {docType.FullNameInCode()}, which is not available when the natural key is derived from the event alone"; + return false; } - } - if (canCall && eventArgBound) + args[i] = fabricatedAggregate; + } + else if (parameterType.IsAssignableFrom(eventType)) { - var body = Expression.Convert( - Expression.Property( - Expression.Call(method, callArgs), - naturalKeyProp), - typeof(object)); - - return Expression.Lambda>(body, eventParam).Compile(); + args[i] = Expression.Convert(Expression.Property(eventParam, _eventDataProperty), parameterType); + } + else + { + arguments = null; + blocker = + $"parameter '{parameters[i].Name}' of type {parameterType.FullNameInCode()} cannot be supplied from the event"; + return false; } } - // For static methods on the projection class, we can't safely call them - // (they may need IEvent with StreamKey, etc.). Instead, find a matching - // property on the event data type and read it directly. - Type eventDataType; - if (firstParamType.IsGenericType && firstParamType.GetGenericTypeDefinition() == typeof(IEvent<>)) + arguments = args; + blocker = null; + return true; + } + + /// + /// Read the natural key straight off the event body when the event carries a property of the key's + /// type. A single candidate is unambiguous; several are only usable when one of them shares the name + /// of the natural key property, because silently taking the first declared one is how an event that + /// carries both an old and a new key gets the wrong answer. + /// + private static Expression? tryReadKeyOffTheEvent( + ParameterExpression eventParam, + Type eventType, + PropertyInfo naturalKeyProp, + out string? blocker) + { + blocker = null; + + var candidates = eventType.GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.PropertyType == naturalKeyProp.PropertyType && p.GetMethod != null) + .ToArray(); + + if (candidates.Length == 0) return null; + + var chosen = candidates.Length == 1 + ? candidates[0] + : candidates.FirstOrDefault(p => + string.Equals(p.Name, naturalKeyProp.Name, StringComparison.OrdinalIgnoreCase)); + + if (chosen == null) + { + blocker = + $"{eventType.FullNameInCode()} carries more than one {naturalKeyProp.PropertyType.FullNameInCode()} property ({candidates.Select(x => x.Name).Join(", ")}) and none of them is named '{naturalKeyProp.Name}', so which one is the natural key is ambiguous"; + return null; + } + + return Expression.Convert( + Expression.Property( + Expression.Convert(Expression.Property(eventParam, _eventDataProperty), eventType), + chosen), + typeof(object)); + } + + /// + /// The legacy path: build a blank TDoc, run the user's own Create/Apply method against it, and read + /// the natural key back off the result. Covers the self-aggregating create factory (marten#4277) and + /// the static evolve method that changes the key (marten#4966). Gated on TDoc being safely + /// constructible, since Expression.New bypasses required-member enforcement and hands the + /// user's method an aggregate that no constructor of theirs ever produced (marten#5041). + /// + private static Expression? tryInvokeAgainstFabricatedAggregate( + MethodInfo method, + PropertyInfo naturalKeyProp, + Type docType, + ParameterInfo[] parameters, + Type eventType, + ParameterExpression eventParam, + out string? blocker) + { + var needsAggregate = !method.IsStatic || parameters.Any(x => x.ParameterType == docType); + + if (needsAggregate && !canSafelyFabricate(docType, out blocker)) + { + return null; + } + + if (!method.IsStatic && method.DeclaringType != docType) + { + blocker = + $"it is an instance method on {method.DeclaringType?.FullNameInCode()}, which discovery has no instance of — make it static, or register the mapping explicitly with NaturalKeyFor()"; + return null; + } + + var docVariable = Expression.Variable(docType, "doc"); + + if (!tryBindArguments(eventParam, parameters, eventType, docType, docVariable, out var args, + out blocker)) + { + return null; + } + + var call = method.IsStatic + ? Expression.Call(method, args!) + : Expression.Call(docVariable, method, args!); + + var statements = new List(); + if (needsAggregate) { - eventDataType = firstParamType.GetGenericArguments()[0]; + statements.Add(Expression.Assign(docVariable, Expression.New(docType))); } - else if (firstParamType == docType && parameters.Length >= 2) + + Expression keyExpression; + if (method.ReturnType == naturalKeyProp.PropertyType) { - var secondType = parameters[1].ParameterType; - eventDataType = secondType.IsGenericType && secondType.GetGenericTypeDefinition() == typeof(IEvent<>) - ? secondType.GetGenericArguments()[0] - : secondType; + // e.g. an instance method on the aggregate that simply returns the key + keyExpression = call; + } + else if (method.ReturnType == docType) + { + // The evolve/create shape — read the key off what the method produced, not off the blank. + keyExpression = Expression.Property(call, naturalKeyProp); + } + else if (!method.IsStatic) + { + // The classic mutating instance Apply — call it, then read the key it set on the aggregate. + statements.Add(call); + keyExpression = Expression.Property(docVariable, naturalKeyProp); } else { - eventDataType = firstParamType; + blocker = + $"it is static and returns {method.ReturnType.FullNameInCode()}, which is neither the aggregate nor the natural key type {naturalKeyProp.PropertyType.FullNameInCode()}, so there is nothing to read the key from"; + return null; } - // Search for a property on the event data that matches the natural key by type - var eventKeyProp = eventDataType - .GetProperties(BindingFlags.Public | BindingFlags.Instance) - .FirstOrDefault(p => p.PropertyType == naturalKeyProp.PropertyType); + statements.Add(Expression.Convert(keyExpression, typeof(object))); - if (eventKeyProp == null) return null; + blocker = null; + return Expression.Block([docVariable], statements); + } - var body2 = Expression.Convert( - Expression.Property( - Expression.Convert(eventParam, eventDataType), - eventKeyProp), - typeof(object)); + private static bool canSafelyFabricate(Type docType, out string? blocker) + { + var constructor = docType.GetConstructor(System.Type.EmptyTypes); + if (constructor == null) + { + blocker = + $"deriving the key means calling this method against a blank {docType.FullNameInCode()}, which has no public parameterless constructor"; + return false; + } + + // Expression.New happily ignores `required`, so the user's method would run against an aggregate + // with null members that C# itself would never have let them create. That is the marten#5041 + // ArgumentNullException, thrown out of a plain event append. + var hasRequiredMembers = + docType.GetCustomAttributes(inherit: true).Any(x => x is RequiredMemberAttribute); + var constructorSetsThem = + constructor.GetCustomAttributes(inherit: true).Any(x => x is SetsRequiredMembersAttribute); - return Expression.Lambda>(body2, eventParam).Compile(); + if (hasRequiredMembers && !constructorSetsThem) + { + blocker = + $"deriving the key means calling this method against a blank {docType.FullNameInCode()}, but it declares required members that a parameterless constructor cannot satisfy"; + return false; + } + + blocker = null; + return true; } + + private static Func compile(Expression body, ParameterExpression eventParam) + => Expression.Lambda>(body, eventParam).Compile(); } \ No newline at end of file diff --git a/src/JasperFx.Events/NaturalKeyBuilder.cs b/src/JasperFx.Events/NaturalKeyBuilder.cs index 1cac577..198a22f 100644 --- a/src/JasperFx.Events/NaturalKeyBuilder.cs +++ b/src/JasperFx.Events/NaturalKeyBuilder.cs @@ -3,27 +3,46 @@ namespace JasperFx.Events; /// -/// Fluent builder for configuring natural key event mappings on a projection. +/// Fluent builder for configuring natural key event mappings on a projection. Reach it from +/// NaturalKeyFor() on a single or multi stream projection. This is the supported way to bypass +/// [NaturalKeySource] attribute discovery when the key cannot be derived from the event by +/// convention — or when you would simply rather be explicit. See jasperfx#569. /// public class NaturalKeyBuilder { private readonly NaturalKeyDefinition _definition; - internal NaturalKeyBuilder(NaturalKeyDefinition definition) + public NaturalKeyBuilder(NaturalKeyDefinition definition) { - _definition = definition; + _definition = definition ?? throw new ArgumentNullException(nameof(definition)); } /// - /// Register an event type that sets or changes the natural key value. + /// Register an event type that sets or changes the natural key value. Replaces any mapping already + /// registered for the same event type, so an explicit registration always wins over attribute + /// discovery. /// - /// Lambda to extract the natural key value from the event. + /// Lambda to extract the natural key value from the event body. /// The event type that carries the natural key value. public NaturalKeyBuilder SetBy(Func extractor) { - _definition.EventMappings.Add(new NaturalKeyEventMapping( - typeof(TEvent), - e => extractor((TEvent)e))); + if (extractor == null) throw new ArgumentNullException(nameof(extractor)); + + _definition.AddOrReplaceMapping(typeof(TEvent), e => extractor((TEvent)e.Data)); + return this; + } + + /// + /// for a key that also depends on event + /// metadata — stream key, timestamp, headers — rather than the event body alone. + /// + /// Lambda to extract the natural key value from the event. + /// The event type that carries the natural key value. + public NaturalKeyBuilder SetByEvent(Func, object?> extractor) where TEvent : notnull + { + if (extractor == null) throw new ArgumentNullException(nameof(extractor)); + + _definition.AddOrReplaceMapping(typeof(TEvent), e => extractor((IEvent)e)); return this; } } diff --git a/src/JasperFx.Events/NaturalKeyDefinition.cs b/src/JasperFx.Events/NaturalKeyDefinition.cs index 57fb5fe..85084dc 100644 --- a/src/JasperFx.Events/NaturalKeyDefinition.cs +++ b/src/JasperFx.Events/NaturalKeyDefinition.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Reflection; using JasperFx.Core.Reflection; @@ -11,14 +12,43 @@ namespace JasperFx.Events; /// public class NaturalKeyEventMapping { - public NaturalKeyEventMapping(Type eventType, Func extractor) + public NaturalKeyEventMapping(Type eventType, Func extractor) { EventType = eventType; Extractor = extractor; } public Type EventType { get; } - public Func Extractor { get; } + + /// + /// Derives the natural key value carried by a single event. Receives the whole + /// rather than just (jasperfx#569) so that an IEvent<T> handler + /// — a first class signature everywhere else in aggregation discovery — is directly bindable, and so + /// that a key derived from event metadata (stream key, timestamp, headers) is expressible at all. + /// + public Func Extractor { get; } +} + +/// +/// A [NaturalKeySource] method that discovery could not turn into a usable extractor, along with +/// the reason. Surfaced instead of being swallowed so that projection validation can fail loudly at +/// configuration time naming the method — see jasperfx#569. +/// +public class NaturalKeySourceProblem +{ + public NaturalKeySourceProblem(MethodInfo method, Type eventType, string reason) + { + Method = method; + EventType = eventType; + Reason = reason; + } + + public MethodInfo Method { get; } + public Type EventType { get; } + public string Reason { get; } + + public override string ToString() + => $"{Method.DeclaringType?.FullNameInCode()}.{Method.Name}() for event {EventType.FullNameInCode()}: {Reason}"; } /// @@ -31,6 +61,8 @@ public NaturalKeyEventMapping(Type eventType, Func extractor) Justification = "Class-level: reflective Type results assigned to DAM-annotated targets during natural-key discovery. Source types preserved at registration.")] public class NaturalKeyDefinition { + private readonly List _problems = new(); + public NaturalKeyDefinition(Type aggregateType, MemberInfo member) { AggregateType = aggregateType; @@ -83,6 +115,41 @@ public NaturalKeyDefinition(Type aggregateType, MemberInfo member) /// public List EventMappings { get; } = new(); + /// + /// [NaturalKeySource] methods that discovery could not bind, and why. Empty when every + /// annotated method produced a mapping. Projection validation turns any leftovers into an + /// InvalidProjectionException at configuration time rather than a natural key lookup table + /// that is silently never written. See jasperfx#569. + /// + public IReadOnlyList DiscoveryProblems => _problems; + + /// + /// Is there already a key extraction registered for this event type? + /// + public bool HasMappingFor(Type eventType) => EventMappings.Any(x => x.EventType == eventType); + + /// + /// Register (or replace) the key extraction for an event type. Replacing is what lets an explicit + /// registration override — and clear the recorded problem of — + /// an attribute-discovered method that discovery could not bind. + /// + public void AddOrReplaceMapping(Type eventType, Func extractor) + { + EventMappings.RemoveAll(x => x.EventType == eventType); + EventMappings.Add(new NaturalKeyEventMapping(eventType, extractor)); + _problems.RemoveAll(x => x.EventType == eventType); + } + + /// + /// Record a [NaturalKeySource] method that could not be bound to an extractor. + /// + public void RecordProblem(MethodInfo method, Type eventType, string reason) + { + if (HasMappingFor(eventType)) return; + + _problems.Add(new NaturalKeySourceProblem(method, eventType, reason)); + } + /// /// Unwrap a natural key value to its inner primitive representation. ///