Merge pull request #2277 from Cratis:fix/minor-fixes

Fix/minor-fixes

Author: einari

Documentation/projections/model-bound/children.md

@@ -14,25 +14,30 @@ public record Order(
     [Key]
     Guid OrderId,
 
-    [ChildrenFrom<LineItemAdded>(
-        key: nameof(LineItemAdded.ItemId),
-        identifiedBy: nameof(LineItem.Id))]
+    [ChildrenFrom<LineItemAdded>(key: nameof(LineItemAdded.ItemId))]
     IEnumerable<LineItem> Items);
 
 public record LineItem(
-    [Key] Guid Id,
+    [Key] Guid Id,  // Chronicle automatically discovers this as the key
     string ProductName,
     int Quantity,
     decimal Price);
 ```
 
+In this example, the `[Key]` attribute on the `LineItem.Id` property is automatically discovered by Chronicle, so you don't need to specify `identifiedBy` explicitly in the `ChildrenFrom` attribute.
+
 ### Parameters
 
 - **key** (optional): Property on the event that identifies the child. Defaults to `EventSourceId`
-- **identifiedBy** (optional): Property on the child model that identifies it. Defaults to `Id`
+- **identifiedBy** (optional): Property on the child model that identifies it. If not specified, Chronicle will:
+  1. Look for a property with the `[Key]` attribute
+  2. Look for a property named `Id` (case-insensitive)
+  3. Fall back to `EventSourceId` if neither is found
 - **parentKey** (optional): Property that identifies the parent. Defaults to `EventSourceId`
 - **autoMap** (optional): Whether to automatically map matching properties from the event to the child model. Defaults to `true`
 
+> **Note**: With automatic key discovery, you typically don't need to specify `identifiedBy` explicitly. Just mark your child model's key property with `[Key]` attribute, or name it `Id`, and Chronicle will automatically discover it.
+
 ### Auto-Mapping
 
 By default, `ChildrenFrom` automatically maps properties from the event to the child model when property names match. This behavior is similar to the `FromEvent` attribute:

Source/Clients/ApplicationModel/ReadModels/ReadModelServiceCollectionExtensions.cs

@@ -23,15 +23,22 @@ public static class ReadModelServiceCollectionExtensions
     /// <returns>The service collection for continuation.</returns>
     public static IServiceCollection AddReadModels(this IServiceCollection services, IClientArtifactsProvider clientArtifactsProvider)
     {
-        var readModelTypes = clientArtifactsProvider.Projections
+        var readModelTypesFromProjections = clientArtifactsProvider.Projections
             .Select(projectionType =>
             {
                 var projectionInterface = projectionType.GetInterfaces()
                     .FirstOrDefault(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IProjectionFor<>));
                 return projectionInterface?.GetGenericArguments()[0];
             })
             .Where(type => type?.IsClass == true && !type.IsAbstract)
-            .Cast<Type>()
+            .Cast<Type>();
+
+        var modelBoundReadModels = clientArtifactsProvider.ModelBoundProjections
+            .Where(type => type.IsClass && !type.IsAbstract);
+
+        var readModelTypes = readModelTypesFromProjections
+            .Concat(modelBoundReadModels)
+            .Distinct()
             .ToArray();
 
         foreach (var readModelType in readModelTypes)

Source/Clients/DotNET.Specs/Projections/ModelBound/for_ModelBoundProjectionBuilder/when_building_model_with_children_from_and_child_has_id_by_convention.cs

@@ -0,0 +1,42 @@
+// Copyright (c) Cratis. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#pragma warning disable SA1649 // File name should match first type name
+#pragma warning disable SA1402 // File may only contain a single type
+
+using Cratis.Chronicle.Contracts.Projections;
+using Cratis.Chronicle.Events;
+using Cratis.Chronicle.Keys;
+
+namespace Cratis.Chronicle.Projections.ModelBound.for_ModelBoundProjectionBuilder;
+
+public class when_building_model_with_children_from_and_child_has_id_by_convention : given.a_model_bound_projection_builder
+{
+    ProjectionDefinition _result;
+
+    void Establish()
+    {
+        event_types = new EventTypesForSpecifications([typeof(ItemAddedById)]);
+        builder = new ModelBoundProjectionBuilder(naming_policy, event_types);
+    }
+
+    void Because() => _result = builder.Build(typeof(OrderWithChildHavingIdByConvention));
+
+    [Fact] void should_return_definition() => _result.ShouldNotBeNull();
+    [Fact] void should_have_children_definition() => _result.Children.Count.ShouldEqual(1);
+
+    [Fact] void should_use_id_property_as_identified_by()
+    {
+        var childrenDef = _result.Children[nameof(OrderWithChildHavingIdByConvention.Items)];
+        childrenDef.IdentifiedBy.ShouldEqual(nameof(ItemWithIdByConvention.Id));
+    }
+}
+
+[EventType]
+public record ItemAddedById(Guid ItemId, string Name);
+
+public record ItemWithIdByConvention(Guid Id, string Name);
+
+public record OrderWithChildHavingIdByConvention(
+    [Key] Guid Id,
+    [ChildrenFrom<ItemAddedById>(key: nameof(ItemAddedById.ItemId))] IEnumerable<ItemWithIdByConvention> Items);

Source/Clients/DotNET.Specs/Projections/ModelBound/for_ModelBoundProjectionBuilder/when_building_model_with_children_from_and_child_has_key_attribute.cs

@@ -0,0 +1,42 @@
+// Copyright (c) Cratis. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#pragma warning disable SA1649 // File name should match first type name
+#pragma warning disable SA1402 // File may only contain a single type
+
+using Cratis.Chronicle.Contracts.Projections;
+using Cratis.Chronicle.Events;
+using Cratis.Chronicle.Keys;
+
+namespace Cratis.Chronicle.Projections.ModelBound.for_ModelBoundProjectionBuilder;
+
+public class when_building_model_with_children_from_and_child_has_key_attribute : given.a_model_bound_projection_builder
+{
+    ProjectionDefinition _result;
+
+    void Establish()
+    {
+        event_types = new EventTypesForSpecifications([typeof(ItemAddedWithKey)]);
+        builder = new ModelBoundProjectionBuilder(naming_policy, event_types);
+    }
+
+    void Because() => _result = builder.Build(typeof(OrderWithChildHavingKeyAttribute));
+
+    [Fact] void should_return_definition() => _result.ShouldNotBeNull();
+    [Fact] void should_have_children_definition() => _result.Children.Count.ShouldEqual(1);
+
+    [Fact] void should_use_key_property_as_identified_by()
+    {
+        var childrenDef = _result.Children[nameof(OrderWithChildHavingKeyAttribute.Items)];
+        childrenDef.IdentifiedBy.ShouldEqual(nameof(ItemWithKeyAttribute.ItemId));
+    }
+}
+
+[EventType]
+public record ItemAddedWithKey(Guid ItemId, string Name);
+
+public record ItemWithKeyAttribute([Key] Guid ItemId, string Name);
+
+public record OrderWithChildHavingKeyAttribute(
+    [Key] Guid Id,
+    [ChildrenFrom<ItemAddedWithKey>(key: nameof(ItemAddedWithKey.ItemId))] IEnumerable<ItemWithKeyAttribute> Items);

Source/Clients/DotNET.Specs/Projections/ModelBound/for_ModelBoundProjectionBuilder/when_building_model_with_children_from_and_no_key_property_defaults_to_event_source_id.cs

@@ -0,0 +1,42 @@
+// Copyright (c) Cratis. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#pragma warning disable SA1649 // File name should match first type name
+#pragma warning disable SA1402 // File may only contain a single type
+
+using Cratis.Chronicle.Contracts.Projections;
+using Cratis.Chronicle.Events;
+using Cratis.Chronicle.Keys;
+
+namespace Cratis.Chronicle.Projections.ModelBound.for_ModelBoundProjectionBuilder;
+
+public class when_building_model_with_children_from_and_no_key_property_defaults_to_event_source_id : given.a_model_bound_projection_builder
+{
+    ProjectionDefinition _result;
+
+    void Establish()
+    {
+        event_types = new EventTypesForSpecifications([typeof(ItemAddedWithoutKey)]);
+        builder = new ModelBoundProjectionBuilder(naming_policy, event_types);
+    }
+
+    void Because() => _result = builder.Build(typeof(OrderWithChildHavingNoKey));
+
+    [Fact] void should_return_definition() => _result.ShouldNotBeNull();
+    [Fact] void should_have_children_definition() => _result.Children.Count.ShouldEqual(1);
+
+    [Fact] void should_default_to_event_source_id()
+    {
+        var childrenDef = _result.Children[nameof(OrderWithChildHavingNoKey.Items)];
+        childrenDef.IdentifiedBy.ShouldEqual(WellKnownExpressions.EventSourceId);
+    }
+}
+
+[EventType]
+public record ItemAddedWithoutKey(Guid ItemId, string Name);
+
+public record ItemWithoutKey(string Name, string Description);
+
+public record OrderWithChildHavingNoKey(
+    [Key] Guid Id,
+    [ChildrenFrom<ItemAddedWithoutKey>(key: nameof(ItemAddedWithoutKey.ItemId))] IEnumerable<ItemWithoutKey> Items);

Source/Clients/DotNET/Events/EventSourceId.cs

@@ -14,6 +14,14 @@ public record EventSourceId(string Value) : ConceptAs<string>(Value)
     /// </summary>
     public static readonly EventSourceId Unspecified = new(string.Empty);
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="EventSourceId"/> class.
+    /// </summary>
+    /// <param name="value">The <see cref="Guid"/> value.</param>
+    public EventSourceId(Guid value) : this(value.ToString())
+    {
+    }
+
     /// <summary>
     /// Check whether or not the <see cref="EventSourceId"/> is specified.
     /// </summary>

Source/Clients/DotNET/Projections/ModelBound/ChildrenDefinitionExtensions.cs

@@ -3,6 +3,7 @@
 
 using System.Reflection;
 using Cratis.Chronicle.Contracts.Projections;
+using Cratis.Chronicle.Keys;
 using Cratis.Chronicle.Properties;
 using Cratis.Serialization;
 using EventType = Cratis.Chronicle.Contracts.Events.EventType;
@@ -45,10 +46,16 @@ internal static void ProcessChildrenFromAttribute(
         var autoMapProperty = attr.GetType().GetProperty(nameof(ChildrenFromAttribute<object>.AutoMap));
 
         var key = keyProperty?.GetValue(attr) as string ?? WellKnownExpressions.EventSourceId;
-        var identifiedBy = identifiedByProperty?.GetValue(attr) as string ?? WellKnownExpressions.Id;
         var parentKey = parentKeyProperty?.GetValue(attr) as string ?? WellKnownExpressions.EventSourceId;
         var autoMap = autoMapProperty?.GetValue(attr) as bool? ?? true;
 
+        var childType = GetChildType(memberType);
+        var identifiedBy = identifiedByProperty?.GetValue(attr) as string;
+        if (string.IsNullOrEmpty(identifiedBy))
+        {
+            identifiedBy = DiscoverKeyPropertyName(childType);
+        }
+
         if (!definition.Children.TryGetValue(propertyName, out var childrenDef))
         {
             childrenDef = new ChildrenDefinition
@@ -72,7 +79,6 @@ internal static void ProcessChildrenFromAttribute(
             Properties = new Dictionary<string, string>()
         };
 
-        var childType = GetChildType(memberType);
         if (childType is not null)
         {
             // If autoMap is enabled, map matching properties from event to child model
@@ -105,4 +111,40 @@ internal static void ProcessChildrenFromAttribute(
 
         return enumerableInterface?.GetGenericArguments()[0];
     }
+
+    static string DiscoverKeyPropertyName(Type? childType)
+    {
+        if (childType is null)
+        {
+            return WellKnownExpressions.EventSourceId;
+        }
+
+        var properties = childType.GetProperties(BindingFlags.Public | BindingFlags.Instance);
+
+        var keyProperty = properties.FirstOrDefault(p => p.GetCustomAttribute<KeyAttribute>(true) is not null);
+        if (keyProperty is not null)
+        {
+            return keyProperty.Name;
+        }
+
+        // Check constructor parameters for record types
+        var constructor = childType.GetConstructors().FirstOrDefault();
+        if (constructor is not null)
+        {
+            var keyParameter = constructor.GetParameters().FirstOrDefault(p => p.GetCustomAttribute<KeyAttribute>(true) is not null);
+            if (keyParameter is not null)
+            {
+                // Convert parameter name to property name (capitalize first letter)
+                return char.ToUpperInvariant(keyParameter.Name![0]) + keyParameter.Name.Substring(1);
+            }
+        }
+
+        var idProperty = properties.FirstOrDefault(p => p.Name.Equals("Id", StringComparison.OrdinalIgnoreCase));
+        if (idProperty is not null)
+        {
+            return idProperty.Name;
+        }
+
+        return WellKnownExpressions.EventSourceId;
+    }
 }

Source/Clients/DotNET/Projections/ModelBound/ChildrenFromAttribute.cs

@@ -11,7 +11,7 @@ namespace Cratis.Chronicle.Projections.ModelBound;
 /// Initializes a new instance of <see cref="ChildrenFromAttribute{TEvent}"/>.
 /// </remarks>
 /// <param name="key">Optional property name on the event that identifies the child. Defaults to WellKnownExpressions.EventSourceId.</param>
-/// <param name="identifiedBy">Optional property name on the child model that identifies it. Defaults to WellKnownExpressions.Id.</param>
+/// <param name="identifiedBy">Optional property name on the child model that identifies it. If not specified, will look for [Key] attribute, then an Id property by convention, finally defaulting to WellKnownExpressions.EventSourceId.</param>
 /// <param name="parentKey">Optional property name that identifies the parent. Defaults to WellKnownExpressions.EventSourceId.</param>
 /// <param name="autoMap">Whether to automatically map matching properties from the event to the child model. Defaults to true.</param>
 [AttributeUsage(AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = true)]
@@ -31,7 +31,7 @@ public sealed class ChildrenFromAttribute<TEvent>(
     public string ParentKey { get; } = parentKey ?? WellKnownExpressions.EventSourceId;
 
     /// <inheritdoc/>
-    public string IdentifiedBy { get; } = identifiedBy ?? WellKnownExpressions.Id;
+    public string? IdentifiedBy { get; } = identifiedBy;
 
     /// <inheritdoc/>
     public bool AutoMap { get; } = autoMap;