-
Notifications
You must be signed in to change notification settings - Fork 0
[WIP] Fix inconsistent projection name resolution in event handling #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
db670ee
7bf32b7
a951c18
f998e4b
6d1fed9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -224,7 +224,7 @@ | |||||||||
| { | ||||||||||
|
|
||||||||||
| // Check interfaces for explicit implementation | ||||||||||
| foreach (var iface in type.GetInterfaces()) | ||||||||||
|
Check warning on line 227 in src/BbQ.Events/Engine/DefaultProjectionEngine.cs
|
||||||||||
| { | ||||||||||
| var map = type.GetInterfaceMap(iface); | ||||||||||
| for (int i = 0; i < map.InterfaceMethods.Length; i++) | ||||||||||
|
|
@@ -252,7 +252,7 @@ | |||||||||
| { | ||||||||||
|
|
||||||||||
| // Check interfaces for explicit implementation | ||||||||||
| foreach (var iface in type.GetInterfaces()) | ||||||||||
|
Check warning on line 255 in src/BbQ.Events/Engine/DefaultProjectionEngine.cs
|
||||||||||
| { | ||||||||||
| var map = type.GetInterfaceMap(iface); | ||||||||||
| for (int i = 0; i < map.InterfaceMethods.Length; i++) | ||||||||||
|
|
@@ -377,16 +377,21 @@ | |||||||||
| // Fall back to reading from attribute | ||||||||||
| var attribute = concreteType.GetCustomAttribute<ProjectionAttribute>(); | ||||||||||
|
|
||||||||||
| return new ProjectionOptions | ||||||||||
| var options = new ProjectionOptions | ||||||||||
| { | ||||||||||
| ProjectionName = concreteType.Name, | ||||||||||
| MaxDegreeOfParallelism = attribute?.MaxDegreeOfParallelism ?? 1, | ||||||||||
| CheckpointBatchSize = attribute?.CheckpointBatchSize ?? 100, | ||||||||||
| StartupMode = attribute?.StartupMode ?? ProjectionStartupMode.Resume, | ||||||||||
| ChannelCapacity = attribute?.ChannelCapacity ?? 1000, | ||||||||||
| BackpressureStrategy = attribute?.BackpressureStrategy ?? BackpressureStrategy.Block, | ||||||||||
| // ErrorHandling is already initialized by property initializer to default values | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| // Use ProjectionNameResolver for consistent name resolution | ||||||||||
| // Since ProjectionName is not explicitly set in options, the resolver will use the type name | ||||||||||
| options.ProjectionName = ProjectionNameResolver.Resolve(concreteType, options); | ||||||||||
|
Comment on lines
+391
to
+392
|
||||||||||
| // Since ProjectionName is not explicitly set in options, the resolver will use the type name | |
| options.ProjectionName = ProjectionNameResolver.Resolve(concreteType, options); | |
| // Since ProjectionName is not explicitly set in options, explicitly use the type name fallback | |
| options.ProjectionName = ProjectionNameResolver.Resolve(concreteType, null); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| namespace BbQ.Events.Engine; | ||
|
|
||
| /// <summary> | ||
| /// Provides consistent projection name resolution across all projection components. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This resolver ensures that projection names are resolved using the same logic everywhere: | ||
| /// - DefaultProjectionEngine (runtime processing) | ||
| /// - DefaultReplayService (replay operations) | ||
| /// - DefaultProjectionRebuilder (rebuild operations) | ||
| /// | ||
| /// Resolution logic: | ||
| /// 1. If ProjectionOptions.ProjectionName is set and not empty → use it | ||
| /// 2. Otherwise → use the projection type name | ||
| /// | ||
| /// This consistency prevents mismatched projection identifiers between runtime processing | ||
| /// and replay/rebuild operations. | ||
| /// </remarks> | ||
| public static class ProjectionNameResolver | ||
| { | ||
| /// <summary> | ||
| /// Resolves the projection name for a given projection type and options. | ||
| /// </summary> | ||
| /// <param name="projectionType">The concrete type of the projection handler.</param> | ||
| /// <param name="options">Optional projection options that may contain an explicit projection name.</param> | ||
| /// <returns>The resolved projection name.</returns> | ||
| public static string Resolve(Type projectionType, ProjectionOptions? options) | ||
| { | ||
| if (projectionType == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(projectionType)); | ||
| } | ||
|
|
||
| // If ProjectionOptions.ProjectionName is explicitly set, use it | ||
| if (!string.IsNullOrWhiteSpace(options?.ProjectionName)) | ||
| { | ||
| return options.ProjectionName; | ||
| } | ||
|
|
||
| // Otherwise, fall back to the projection type name | ||
| return projectionType.Name; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| using BbQ.Events.Engine; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace BbQ.Cqrs.Tests; | ||
|
|
||
| /// <summary> | ||
| /// Tests for ProjectionNameResolver to ensure consistent projection name resolution. | ||
| /// </summary> | ||
| [TestFixture] | ||
| public class ProjectionNameResolverTests | ||
| { | ||
| // Test projection class | ||
| private class TestProjection { } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithNullProjectionType_ThrowsArgumentNullException() | ||
| { | ||
| // Act & Assert | ||
| Assert.Throws<ArgumentNullException>(() => | ||
| ProjectionNameResolver.Resolve(null!, null)); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithNullOptions_ReturnsTypeName() | ||
| { | ||
| // Arrange | ||
| var projectionType = typeof(TestProjection); | ||
|
|
||
| // Act | ||
| var result = ProjectionNameResolver.Resolve(projectionType, null); | ||
|
|
||
| // Assert | ||
| Assert.That(result, Is.EqualTo("TestProjection")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithOptionsButNullProjectionName_ReturnsTypeName() | ||
| { | ||
| // Arrange | ||
| var projectionType = typeof(TestProjection); | ||
| var options = new ProjectionOptions | ||
| { | ||
| ProjectionName = null! | ||
| }; | ||
|
|
||
| // Act | ||
| var result = ProjectionNameResolver.Resolve(projectionType, options); | ||
|
|
||
| // Assert | ||
| Assert.That(result, Is.EqualTo("TestProjection")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithOptionsAndEmptyProjectionName_ReturnsTypeName() | ||
| { | ||
| // Arrange | ||
| var projectionType = typeof(TestProjection); | ||
| var options = new ProjectionOptions | ||
| { | ||
| ProjectionName = string.Empty | ||
| }; | ||
|
|
||
| // Act | ||
| var result = ProjectionNameResolver.Resolve(projectionType, options); | ||
|
|
||
| // Assert | ||
| Assert.That(result, Is.EqualTo("TestProjection")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithOptionsAndWhitespaceProjectionName_ReturnsTypeName() | ||
| { | ||
| // Arrange | ||
| var projectionType = typeof(TestProjection); | ||
| var options = new ProjectionOptions | ||
| { | ||
| ProjectionName = " " | ||
| }; | ||
|
|
||
| // Act | ||
| var result = ProjectionNameResolver.Resolve(projectionType, options); | ||
|
|
||
| // Assert | ||
| Assert.That(result, Is.EqualTo("TestProjection")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithOptionsAndCustomProjectionName_ReturnsCustomName() | ||
| { | ||
| // Arrange | ||
| var projectionType = typeof(TestProjection); | ||
| var customName = "CustomProjectionName"; | ||
| var options = new ProjectionOptions | ||
| { | ||
| ProjectionName = customName | ||
| }; | ||
|
|
||
| // Act | ||
| var result = ProjectionNameResolver.Resolve(projectionType, options); | ||
|
|
||
| // Assert | ||
| Assert.That(result, Is.EqualTo(customName)); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_WithDifferentProjectionTypes_ReturnsCorrectTypeNames() | ||
| { | ||
| // Arrange & Act & Assert | ||
| Assert.That( | ||
| ProjectionNameResolver.Resolve(typeof(TestProjection), null), | ||
| Is.EqualTo("TestProjection")); | ||
|
|
||
| Assert.That( | ||
| ProjectionNameResolver.Resolve(typeof(ProjectionNameResolverTests), null), | ||
| Is.EqualTo("ProjectionNameResolverTests")); | ||
|
|
||
| Assert.That( | ||
| ProjectionNameResolver.Resolve(typeof(string), null), | ||
| Is.EqualTo("String")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Resolve_ConsistencyBetweenCalls_ReturnsSameResult() | ||
| { | ||
| // Arrange | ||
| var projectionType = typeof(TestProjection); | ||
| var options = new ProjectionOptions | ||
| { | ||
| ProjectionName = "ConsistentName" | ||
| }; | ||
|
|
||
| // Act | ||
| var result1 = ProjectionNameResolver.Resolve(projectionType, options); | ||
| var result2 = ProjectionNameResolver.Resolve(projectionType, options); | ||
|
|
||
| // Assert | ||
| Assert.That(result1, Is.EqualTo(result2)); | ||
| Assert.That(result1, Is.EqualTo("ConsistentName")); | ||
| } | ||
| } | ||
|
Comment on lines
+1
to
+140
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "Since ProjectionName is not explicitly set in options, the resolver will use the type name" is slightly misleading. The ProjectionName property has a default value of string.Empty (from ProjectionOptions.cs line 55), not null. The resolver checks string.IsNullOrWhiteSpace which correctly treats empty string as "not set", but the comment could be clearer.
Consider revising to: "Since ProjectionName is empty in the newly created options, the resolver will fall back to the type name"