From ab9407085d5ed2879eefcc3c0131048db29c9674 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:35:50 +0200 Subject: [PATCH 01/14] docs(result): document that failure equality ignores error content Result/Result equality treats any two failures as equal regardless of their Error, which was undocumented and surprising for failure-specific branching. --- CHANGELOG.md | 2 ++ .../ResultEqualityTests.cs | 20 +++++++++++++++++++ src/request.result/Result.Equality.cs | 20 ++++++++++++++++--- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f9bdb..5c9e9c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ To have a consistent experience across all packages, some public interfaces have ### Changed +- **request.result:** Document that `Result`/`Result` equality treats any two failures as equal, ignoring error content; branch on the `Error` directly for failure-specific logic (see PLAN.md Inconsistencies#E) + ### Fixed - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) diff --git a/src/request.result.tests/ResultEqualityTests.cs b/src/request.result.tests/ResultEqualityTests.cs index ad54683..99d7094 100644 --- a/src/request.result.tests/ResultEqualityTests.cs +++ b/src/request.result.tests/ResultEqualityTests.cs @@ -190,6 +190,26 @@ internal sealed class ResultEqualityTests await Assert.That(success.GetHashCode()).IsNotEqualTo(failure.GetHashCode()); } + [Test] + public async Task I_can_equal_failure_and_failure_ignoring_error_content() + { + var a = Prelude.Failure("error 1"); + var b = Prelude.Failure("error 2"); + + await Assert.That(a.Equals(b)).IsTrue(); + await Assert.That(a.Error).IsNotEqualTo(b.Error); + } + + [Test] + public async Task I_can_equal_non_generic_failure_and_failure_ignoring_error_content() + { + var a = Prelude.Failure("error 1"); + var b = Prelude.Failure("error 2"); + + await Assert.That(a.Equals(b)).IsTrue(); + await Assert.That(a.Error).IsNotEqualTo(b.Error); + } + [Test] public async Task I_can_equal_non_generic_result_and_get_true_for_success_and_success() { diff --git a/src/request.result/Result.Equality.cs b/src/request.result/Result.Equality.cs index 2a8cbbb..da9372d 100644 --- a/src/request.result/Result.Equality.cs +++ b/src/request.result/Result.Equality.cs @@ -9,7 +9,15 @@ namespace Geekeey.Request.Result; public partial class Result : IEquatable { - /// + /// + /// Checks whether two results are equal. Results are equal if both are success or both are failure. + /// + /// + /// Two failures are considered equal regardless of their content. Equality therefore + /// ignores the error; if you need to branch on a specific failure, compare the values + /// directly instead of relying on equality. + /// + /// The result to check for equality with the current result. [Pure] public bool Equals(Result? other) { @@ -49,7 +57,8 @@ public partial class Result : IEquatable public partial class Result : IEqualityOperators { /// - /// Checks whether two results are equal. Results are equal if they are both success or both failure. + /// Checks whether two results are equal. Results are equal if they are both success or both failure. Two + /// failures are equal regardless of their error content. /// /// The first result to compare. /// The second result to compare. @@ -79,6 +88,11 @@ public partial class Result : IEquatable>, IEquatable /// Checks whether the result is equal to another result. Results are equal if both results are success values and /// the success values are equal, or if both results are failures. /// + /// + /// Two failures are considered equal regardless of their content. Equality therefore + /// ignores the error; if you need to branch on a specific failure, compare the values + /// directly instead of relying on equality. + /// /// The result to check for equality with the current result. [Pure] public bool Equals(Result? other) @@ -194,7 +208,7 @@ public partial class Result : IEqualityOperators, Result, bool>, { /// /// Checks whether two results are equal. Results are equal if both results are success values and the success - /// values are equal, or if both results are failures. + /// values are equal, or if both results are failures. Two failures are equal regardless of their error content. /// /// The first result to compare. /// The second result to compare. From 68194d3cb3c33038a457a1fb36304fcf08914a8b Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:42:07 +0200 Subject: [PATCH 02/14] fix(validation): guard null comparison bounds in comparison builders Passing a null bound to GreaterThan/LessThan/Between etc. called value.CompareTo(null), throwing NullReferenceException for non-null reference-type values. Short-circuit when the bound is null so the rule is satisfied instead of crashing. --- CHANGELOG.md | 1 + .../RuleBuilderExtensionsTests.cs | 65 +++++++++++++++++++ .../_fixtures/ComparableModel.cs | 19 ++++++ .../RuleBuilderExtensions.cs | 12 ++-- 4 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 src/request.validation.tests/_fixtures/ComparableModel.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c9e9c8..65a9a08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` +- **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) ### Removed diff --git a/src/request.validation.tests/RuleBuilderExtensionsTests.cs b/src/request.validation.tests/RuleBuilderExtensionsTests.cs index 5b5271a..c0d2cae 100644 --- a/src/request.validation.tests/RuleBuilderExtensionsTests.cs +++ b/src/request.validation.tests/RuleBuilderExtensionsTests.cs @@ -314,6 +314,71 @@ internal sealed class RuleBuilderExtensionsTests .Throws(); } + [Test] + public async Task I_can_validate_greater_than_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.GreaterThan(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_greater_than_or_equal_to_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.GreaterThanOrEqualTo(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_less_than_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.LessThan(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_less_than_or_equal_to_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.LessThanOrEqualTo(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_between_with_null_bounds_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.Between(null, null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + [Test] public async Task I_can_see_it_throw_for_invalid_between_configuration() { diff --git a/src/request.validation.tests/_fixtures/ComparableModel.cs b/src/request.validation.tests/_fixtures/ComparableModel.cs new file mode 100644 index 0000000..9178228 --- /dev/null +++ b/src/request.validation.tests/_fixtures/ComparableModel.cs @@ -0,0 +1,19 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Validation.Tests; + +internal sealed class ComparableModel +{ + public ComparableValue? Value { get; init; } +} + +internal sealed class ComparableValue : IComparable +{ + public int Number { get; init; } + + public int CompareTo(ComparableValue? other) + { + return Number.CompareTo(other!.Number); + } +} diff --git a/src/request.validation/RuleBuilderExtensions.cs b/src/request.validation/RuleBuilderExtensions.cs index 528b387..7e1a5db 100644 --- a/src/request.validation/RuleBuilderExtensions.cs +++ b/src/request.validation/RuleBuilderExtensions.cs @@ -170,7 +170,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) > 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) > 0, $"Value must be greater than {comparisonValue}."); } @@ -189,7 +189,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) >= 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) >= 0, $"Value must be greater than or equal to {comparisonValue}."); } @@ -208,7 +208,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) < 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) < 0, $"Value must be less than {comparisonValue}."); } @@ -227,7 +227,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) <= 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) <= 0, $"Value must be less than or equal to {comparisonValue}."); } @@ -254,7 +254,9 @@ public static class RuleBuilderExtensions "Maximum value must be greater than or equal to minimum value."); } - return rule.Must(value => IsNull(value) || (value.CompareTo(minValue) >= 0 && value.CompareTo(maxValue) <= 0), + return rule.Must(value => IsNull(value) + || ((minValue is null || value.CompareTo(minValue) >= 0) + && (maxValue is null || value.CompareTo(maxValue) <= 0)), $"Value must be between {minValue} and {maxValue}."); } From 552f88df18d9a4a30a9ff81b02270abe8cdc0aef Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:43:37 +0200 Subject: [PATCH 03/14] fix(validation): return no problems for null reference instance in Rule.Validate Rule.Validate cast a null instance to T and validated it, so a reference T whose accessor dereferences the instance threw NullReferenceException on a direct validator.Validate(context) with a null instance. Guard the null-instance path and return no problems instead. --- CHANGELOG.md | 1 + src/request.validation.tests/ValidatorTests.cs | 11 +++++++++++ src/request.validation/Rule.cs | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65a9a08..491f224 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` - **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) +- **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) ### Removed diff --git a/src/request.validation.tests/ValidatorTests.cs b/src/request.validation.tests/ValidatorTests.cs index 14239f7..6613720 100644 --- a/src/request.validation.tests/ValidatorTests.cs +++ b/src/request.validation.tests/ValidatorTests.cs @@ -45,6 +45,17 @@ internal sealed class ValidatorTests } } + [Test] + public async Task I_can_validate_with_a_null_instance_without_throwing() + { + var validator = new PropertyValidator(person + => person!.Name, rule => rule.Must(value => value is not null, "Name is required.")); + + var result = validator.Validate((Person?)null); + + await Assert.That(result.IsValid).IsTrue(); + } + [Test] public async Task I_can_compose_nested_validators_and_aggregate_property_paths() { diff --git a/src/request.validation/Rule.cs b/src/request.validation/Rule.cs index 0c65cc0..a6a397c 100644 --- a/src/request.validation/Rule.cs +++ b/src/request.validation/Rule.cs @@ -46,7 +46,7 @@ internal abstract record Rule : Rule if (context.Instance is null && default(T) is null) { - return Validate((T)context.Instance!, context); + return []; } var actualType = context.Instance?.GetType().FullName ?? "null"; From f7263c33a835aaa2497a57baf6d542c7528726fb Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:50:06 +0200 Subject: [PATCH 04/14] fix(validation): make Severity meaningful in Validation.IsValid Validation.IsValid previously counted every problem regardless of severity, leaving Severity inert. Now IsValid only fails on Error problems, so Warning and Info no longer invalidate the result, and IsValidFor(Severity) lets callers set the failing-severity threshold. --- CHANGELOG.md | 1 + .../ValidationSeverityTests.cs | 49 +++++++++++++++++++ .../ValidatorTests.cs | 6 ++- src/request.validation/Validation.cs | 22 ++++++++- 4 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 src/request.validation.tests/ValidationSeverityTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 491f224..3de75fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` - **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) - **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) +- **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold (see PLAN.md Inconsistencies#B) ### Removed diff --git a/src/request.validation.tests/ValidationSeverityTests.cs b/src/request.validation.tests/ValidationSeverityTests.cs new file mode 100644 index 0000000..6f24c8d --- /dev/null +++ b/src/request.validation.tests/ValidationSeverityTests.cs @@ -0,0 +1,49 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Validation.Tests; + +internal sealed class ValidationSeverityTests +{ + [Test] + public async Task I_can_have_warning_problems_not_invalidate_by_default() + { + var validator = new PropertyValidator(person + => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.") + .WithSeverity(Severity.Warning)); + + var result = validator.Validate(new Person { Name = "" }); + + await Assert.That(result.Problems).Count().IsEqualTo(1); + await Assert.That(result.IsValid).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Warning)).IsFalse(); + await Assert.That(result.IsValidFor(Severity.Info)).IsFalse(); + } + + [Test] + public async Task I_can_have_info_problems_only_invalidate_at_info_threshold() + { + var validator = new PropertyValidator(person + => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.") + .WithSeverity(Severity.Info)); + + var result = validator.Validate(new Person { Name = "" }); + + await Assert.That(result.Problems).Count().IsEqualTo(1); + await Assert.That(result.IsValid).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Warning)).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Info)).IsFalse(); + } + + [Test] + public async Task I_can_have_error_problems_invalidate_by_default() + { + var validator = new PropertyValidator(person + => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.")); + + var result = validator.Validate(new Person { Name = "" }); + + await Assert.That(result.IsValid).IsFalse(); + await Assert.That(result.IsValidFor(Severity.Error)).IsFalse(); + } +} diff --git a/src/request.validation.tests/ValidatorTests.cs b/src/request.validation.tests/ValidatorTests.cs index 6613720..820edc4 100644 --- a/src/request.validation.tests/ValidatorTests.cs +++ b/src/request.validation.tests/ValidatorTests.cs @@ -14,7 +14,8 @@ internal sealed class ValidatorTests var result = validator.Validate(new Person { Name = "" }); - await Assert.That(result.IsValid).IsFalse(); + await Assert.That(result.IsValid).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Warning)).IsFalse(); await Assert.That(result.Problems).Count().IsEqualTo(1); var problem = result.Problems.Single(); @@ -39,7 +40,8 @@ internal sealed class ValidatorTests using (Assert.Multiple()) { - await Assert.That(invalid.IsValid).IsFalse(); + await Assert.That(invalid.IsValid).IsTrue(); + await Assert.That(invalid.IsValidFor(Severity.Warning)).IsFalse(); await Assert.That(valid.IsValid).IsTrue(); await Assert.That(valid.Problems).IsEmpty(); } diff --git a/src/request.validation/Validation.cs b/src/request.validation/Validation.cs index fadadff..8e9c7c7 100644 --- a/src/request.validation/Validation.cs +++ b/src/request.validation/Validation.cs @@ -23,7 +23,27 @@ public sealed class Validation /// /// Whether the validation was successful. /// - public bool IsValid => Problems.Count is 0; + /// + /// A validation is considered successful unless it contains at least one problem with + /// . Problems of lower severity ( or + /// ) do not invalidate the result. Use + /// to control which severities are treated as failures. + /// + public bool IsValid => IsValidFor(Severity.Error); + + /// + /// Whether the validation was successful, treating problems at or above the given severity as failures. + /// + /// The minimum severity that should be considered a failure. + /// + /// A problem invalidates the validation when its is at least as severe as + /// . For example, IsValidFor(Severity.Warning) fails on errors and + /// warnings but not on informational problems. + /// + public bool IsValidFor(Severity minimum) + { + return !Problems.Any(problem => problem.Severity <= minimum); + } /// /// The problems that were found during validation. From f79e8780f7cc610c8837e9d8646d4a7af4f9aa7d Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 19:18:21 +0200 Subject: [PATCH 05/14] fix(dispatcher): throw on ambiguous handlers instead of silent .First() Multiple handlers for the same request were previously resolved via .First(), with ordering depending on Type.GetInterfaces() order (non-deterministic). Now ScalarRequestInvoker/StreamRequestInvoker throw InvalidOperationException listing the candidate handlers when more than one matches. --- CHANGELOG.md | 1 + .../ScalarDispatcherTests.cs | 33 ++++++++++++++++--- .../StreamDispatcherTests.cs | 31 +++++++++++++++-- .../_fixtures/DuplicateScalarHandler.cs | 24 ++++++++++++++ .../_fixtures/DuplicateStreamHandler.cs | 24 ++++++++++++++ .../ScalarRequestInvoker.cs | 15 ++++++++- .../StreamRequestInvoker.cs | 15 ++++++++- 7 files changed, 134 insertions(+), 9 deletions(-) create mode 100644 src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs create mode 100644 src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3de75fa..b0a11e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) - **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) - **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold (see PLAN.md Inconsistencies#B) +- **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()` (matches MediatR). Also throws when no handler is registered (see PLAN.md Inconsistencies#C) ### Removed diff --git a/src/request.dispatcher.tests/ScalarDispatcherTests.cs b/src/request.dispatcher.tests/ScalarDispatcherTests.cs index 5c10998..8c5ffb1 100644 --- a/src/request.dispatcher.tests/ScalarDispatcherTests.cs +++ b/src/request.dispatcher.tests/ScalarDispatcherTests.cs @@ -116,7 +116,7 @@ internal sealed class ScalarDispatcherTests } [Test] - public async Task I_can_dispatch_a_request_async_with_an_interface_constrained_handler() + public async Task I_can_see_it_throw_on_ambiguous_concrete_and_constrained_scalar_handlers() { var sc = new ServiceCollection(); sc.AddRequestDispatcher(builder => builder @@ -126,13 +126,18 @@ internal sealed class ScalarDispatcherTests var dispatcher = provider.GetRequiredService(); var request = new InterfaceInheritedScalarRequest { Name = "Constrained" }; - var result = await dispatcher.DispatchAsync(request); // Both InterfaceInheritedScalarHandler and InterfaceConstrainedScalarHandler could match. // InterfaceInheritedScalarHandler is a concrete match for InterfaceInheritedScalarRequest. // InterfaceConstrainedScalarHandler is an open generic match. - // Currently Dispatcher.SendAsync checks concrete handlers first. - await Assert.That(result).IsEquivalentTo("Constrained-InterfaceHandled"); + // Ambiguity is no longer resolved silently with .First(); it throws instead. + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedScalarHandler)); + await Assert.That(ex?.Message).Contains("InterfaceConstrainedScalarHandler"); + } } [Test] @@ -234,6 +239,26 @@ internal sealed class ScalarDispatcherTests } } + [Test] + public async Task I_can_see_it_throw_on_ambiguous_scalar_handlers() + { + var sc = new ServiceCollection(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(DuplicateScalarHandlerA)) + .Add(typeof(DuplicateScalarHandlerB))); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + + var request = new DuplicateScalarRequest(); + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(DuplicateScalarHandlerA)); + await Assert.That(ex?.Message).Contains(nameof(DuplicateScalarHandlerB)); + } + } + [Test] public async Task I_can_see_it_throw_if_dispatcher_options_are_modified_after_build() { diff --git a/src/request.dispatcher.tests/StreamDispatcherTests.cs b/src/request.dispatcher.tests/StreamDispatcherTests.cs index c06cb36..5fcc7b2 100644 --- a/src/request.dispatcher.tests/StreamDispatcherTests.cs +++ b/src/request.dispatcher.tests/StreamDispatcherTests.cs @@ -7,6 +7,26 @@ namespace Geekeey.Request.Dispatcher.Tests; public class StreamDispatcherTests { + [Test] + public async Task I_can_see_it_throw_on_ambiguous_stream_handlers() + { + var sc = new ServiceCollection(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(DuplicateStreamHandlerA)) + .Add(typeof(DuplicateStreamHandlerB))); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + + var request = new DuplicateStreamRequest(); + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(DuplicateStreamHandlerA)); + await Assert.That(ex?.Message).Contains(nameof(DuplicateStreamHandlerB)); + } + } + [Test] public async Task I_can_dispatch_a_request_async_with_an_open_generic_handler() { @@ -121,13 +141,18 @@ public class StreamDispatcherTests var dispatcher = provider.GetRequiredService(); var request = new InterfaceInheritedStreamRequest { Name = "Constrained" }; - var results = await dispatcher.DispatchAsync(request).ToListAsync(); // Both InterfaceInheritedStreamHandler and InterfaceConstrainedStreamHandler could match. // InterfaceInheritedStreamHandler is a concrete match for InterfaceInheritedStreamRequest. // InterfaceConstrainedStreamHandler is an open generic match. - // Currently Dispatcher checks concrete handlers first. - await Assert.That(results).IsEquivalentTo(["Constrained-InterfaceHandled-0", "Constrained-InterfaceHandled-1"]); + // Ambiguity is no longer resolved silently with .First(); it throws instead. + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedStreamHandler)); + await Assert.That(ex?.Message).Contains("InterfaceConstrainedStreamHandler"); + } } [Test] diff --git a/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs b/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs new file mode 100644 index 0000000..6a0504e --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs @@ -0,0 +1,24 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public sealed class DuplicateScalarRequest : IScalarRequest +{ +} + +public sealed class DuplicateScalarHandlerA : IScalarRequestHandler +{ + public Task HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken) + { + return Task.FromResult("A"); + } +} + +public sealed class DuplicateScalarHandlerB : IScalarRequestHandler +{ + public Task HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken) + { + return Task.FromResult("B"); + } +} diff --git a/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs b/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs new file mode 100644 index 0000000..d3f0414 --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs @@ -0,0 +1,24 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public sealed class DuplicateStreamRequest : IStreamRequest +{ +} + +public sealed class DuplicateStreamHandlerA : IStreamRequestHandler +{ + public async IAsyncEnumerable HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + yield return "A"; + } +} + +public sealed class DuplicateStreamHandlerB : IStreamRequestHandler +{ + public async IAsyncEnumerable HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + yield return "B"; + } +} diff --git a/src/request.dispatcher/ScalarRequestInvoker.cs b/src/request.dispatcher/ScalarRequestInvoker.cs index 63de03e..2c6d129 100644 --- a/src/request.dispatcher/ScalarRequestInvoker.cs +++ b/src/request.dispatcher/ScalarRequestInvoker.cs @@ -43,7 +43,20 @@ internal sealed class ScalarRequestInvoker : ScalarRequestI Task Head(IScalarRequest r, CancellationToken ct) { - return options.GetRequestHandlers>(serviceProvider).First().HandleAsync((TRequest)r, ct); + var handlers = options.GetRequestHandlers>(serviceProvider).ToArray(); + + if (handlers.Length is 0) + { + throw new InvalidOperationException($"No handler is registered for request '{typeof(TRequest).FullName}'."); + } + + if (handlers.Length > 1) + { + var candidates = string.Join(", ", handlers.Select(handler => handler.GetType().FullName)); + throw new InvalidOperationException($"Multiple handlers are registered for request '{typeof(TRequest).FullName}'. Ambiguous handlers: {candidates}"); + } + + return handlers[0].HandleAsync((TRequest)r, ct); } } } diff --git a/src/request.dispatcher/StreamRequestInvoker.cs b/src/request.dispatcher/StreamRequestInvoker.cs index 882d538..817b5b2 100644 --- a/src/request.dispatcher/StreamRequestInvoker.cs +++ b/src/request.dispatcher/StreamRequestInvoker.cs @@ -47,7 +47,20 @@ internal sealed class StreamRequestInvoker : StreamRequestI IAsyncEnumerable Head(IStreamRequest r, CancellationToken ct) { - return options.GetRequestHandlers>(serviceProvider).First().HandleAsync((TRequest)r, ct); + var handlers = options.GetRequestHandlers>(serviceProvider).ToArray(); + + if (handlers.Length is 0) + { + throw new InvalidOperationException($"No handler is registered for request '{typeof(TRequest).FullName}'."); + } + + if (handlers.Length > 1) + { + var candidates = string.Join(", ", handlers.Select(handler => handler.GetType().FullName)); + throw new InvalidOperationException($"Multiple handlers are registered for request '{typeof(TRequest).FullName}'. Ambiguous handlers: {candidates}"); + } + + return handlers[0].HandleAsync((TRequest)r, ct); } } } From 595ba01ea62ef5bbe0a9ec8923f872d64d7347cc Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 19:22:50 +0200 Subject: [PATCH 06/14] fix(dispatcher): resolve handlers in registration order across closed and open generics Handler/behavior resolution previously forced closed generics before open generics regardless of registration order, which was surprising and fragile. TypeIndex now tracks each registered type's index and orders matching candidates by registration order, so closed/open ordering is deterministic and follows registration. --- CHANGELOG.md | 1 + .../ScalarDispatcherTests.cs | 23 ++++++++++++++ .../RequestDispatcherOptions.cs | 31 +++++++++++++------ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0a11e5..c4018f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) - **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold (see PLAN.md Inconsistencies#B) - **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()` (matches MediatR). Also throws when no handler is registered (see PLAN.md Inconsistencies#C) +- **request.dispatcher:** Resolve handlers/behaviors in registration order across closed and open generics instead of the fixed closed-before-open reflection order (see PLAN.md Inconsistencies#D) ### Removed diff --git a/src/request.dispatcher.tests/ScalarDispatcherTests.cs b/src/request.dispatcher.tests/ScalarDispatcherTests.cs index 8c5ffb1..f274da9 100644 --- a/src/request.dispatcher.tests/ScalarDispatcherTests.cs +++ b/src/request.dispatcher.tests/ScalarDispatcherTests.cs @@ -259,6 +259,29 @@ internal sealed class ScalarDispatcherTests } } + [Test] + public async Task I_can_see_candidates_ordered_by_registration_not_closed_first() + { + var sc = new ServiceCollection(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(InterfaceConstrainedScalarHandler<>)) + .Add(typeof(InterfaceInheritedScalarHandler))); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + + var request = new InterfaceInheritedScalarRequest { Name = "Constrained" }; + + // The open generic handler is registered before the closed handler, so candidates must be + // listed in registration order (open first), not the old fixed closed-before-open order. + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); + + var message = ex?.Message ?? string.Empty; + var openIndex = message.IndexOf("InterfaceConstrainedScalarHandler", StringComparison.Ordinal); + var closedIndex = message.IndexOf("InterfaceInheritedScalarHandler", StringComparison.Ordinal); + + await Assert.That(openIndex >= 0 && closedIndex >= 0 && openIndex < closedIndex).IsTrue(); + } + [Test] public async Task I_can_see_it_throw_if_dispatcher_options_are_modified_after_build() { diff --git a/src/request.dispatcher/RequestDispatcherOptions.cs b/src/request.dispatcher/RequestDispatcherOptions.cs index d18def9..6560f46 100644 --- a/src/request.dispatcher/RequestDispatcherOptions.cs +++ b/src/request.dispatcher/RequestDispatcherOptions.cs @@ -46,11 +46,16 @@ internal sealed class RequestDispatcherOptions protected readonly Dictionary> _closedTypeInfo = []; protected readonly List _openTypeInfo = []; + protected readonly Dictionary _order = []; protected TypeIndex(IEnumerable collection, Func predicate) { + var index = 0; + foreach (var type in collection) { + _order[type] = index++; + if (type.IsGenericTypeDefinition) { if (type.GetInterfaces().Any(predicate)) @@ -102,11 +107,14 @@ internal sealed class RequestDispatcherOptions { protected override IReadOnlyList IsAssignableTo(Type @interface) { - var result = new List(); + var result = new List<(int Order, Type Type)>(); if (_closedTypeInfo.TryGetValue(@interface, out var list)) { - result.AddRange(list); + foreach (var type in list) + { + result.Add((_order[type], type)); + } } var requestType = @interface.GetGenericArguments()[0]; @@ -121,7 +129,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[type], impl)); } } catch (ArgumentException) @@ -137,7 +145,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[type], impl)); } } } @@ -146,7 +154,7 @@ internal sealed class RequestDispatcherOptions } } - return result; + return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); } } @@ -162,11 +170,14 @@ internal sealed class RequestDispatcherOptions { protected override IReadOnlyList IsAssignableTo(Type @interface) { - var result = new List(); + var result = new List<(int Order, Type Type)>(); if (_closedTypeInfo.TryGetValue(@interface, out var list)) { - result.AddRange(list); + foreach (var type in list) + { + result.Add((_order[type], type)); + } } var requestType = @interface.GetGenericArguments()[0]; @@ -180,7 +191,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType, responseType); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[behaviour], impl)); } } catch (ArgumentException) @@ -193,7 +204,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[behaviour], impl)); } } catch (ArgumentException) @@ -201,7 +212,7 @@ internal sealed class RequestDispatcherOptions } } - return result; + return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); } } } From ae2c2679f2f7893a33e8f8e7a92d001e5e0dbada Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 19:30:59 +0200 Subject: [PATCH 07/14] feat(dispatcher): add registration-time pipeline behavior ordering via AddBehavior(order:) Pipeline behavior ordering previously depended on discovery/registration order. Add an AddBehavior(order:) API that captures the order at registration time; the BehaviorTypeIndex now orders matching behaviors by that explicit order (lower runs first), falling back to registration order for ties. Removed the runtime Order property approach. --- CHANGELOG.md | 1 + .../ScalarBehaviourTests.cs | 20 ++++++++ .../StreamBehaviourTests.cs | 20 ++++++++ .../_fixtures/OrderedScalarBehavior.cs | 22 +++++++++ .../_fixtures/OrderedStreamBehavior.cs | 28 +++++++++++ .../RequestDispatcherBuilderExtensions.cs | 21 ++++++++ .../RequestDispatcherOptions.cs | 48 ++++++++++++------- 7 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs create mode 100644 src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index c4018f2..3d071e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold (see PLAN.md Inconsistencies#B) - **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()` (matches MediatR). Also throws when no handler is registered (see PLAN.md Inconsistencies#C) - **request.dispatcher:** Resolve handlers/behaviors in registration order across closed and open generics instead of the fixed closed-before-open reflection order (see PLAN.md Inconsistencies#D) +- **request.dispatcher:** Add `AddBehavior(order:)` registration-time API so pipeline behavior ordering is explicit and deterministic (lower order runs first, registration order as tie-break) instead of relying on reflection/discovery order (see PLAN.md Tips — Explicit pipeline ordering API) ### Removed diff --git a/src/request.dispatcher.tests/ScalarBehaviourTests.cs b/src/request.dispatcher.tests/ScalarBehaviourTests.cs index 9c10e4b..d2a6dd6 100644 --- a/src/request.dispatcher.tests/ScalarBehaviourTests.cs +++ b/src/request.dispatcher.tests/ScalarBehaviourTests.cs @@ -87,6 +87,26 @@ internal sealed class ScalarBehaviourTests await Assert.That(tracker.Executed).IsTrue(); } + [Test] + public async Task I_can_order_behaviours_explicitly_by_order_property() + { + var sc = new ServiceCollection(); + sc.AddSingleton(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(ScalarTestHandler)) + .AddBehavior(2) + .AddBehavior(1)); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + var tracker = provider.GetRequiredService(); + + var request = new ScalarTestRequest { Value = "Hello" }; + await dispatcher.DispatchAsync(request); + + // Registered A (order 2) then B (order 1) via AddBehavior; explicit order overrides discovery order. + await Assert.That(tracker.Log).IsEquivalentTo(["OrderedB", "OrderedA"]); + } + [Test] public async Task I_can_maintain_the_ordering_between_open_and_closed_behaviours() { diff --git a/src/request.dispatcher.tests/StreamBehaviourTests.cs b/src/request.dispatcher.tests/StreamBehaviourTests.cs index c96b530..7e9377f 100644 --- a/src/request.dispatcher.tests/StreamBehaviourTests.cs +++ b/src/request.dispatcher.tests/StreamBehaviourTests.cs @@ -66,6 +66,26 @@ internal sealed class StreamBehaviourTests await Assert.That(tracker.Log[1]).IsEquivalentTo("Behaviour2"); } + [Test] + public async Task I_can_order_behaviours_explicitly_by_order_property() + { + var sc = new ServiceCollection(); + sc.AddSingleton(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(StreamTestHandler)) + .AddBehavior(2) + .AddBehavior(1)); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + var tracker = provider.GetRequiredService(); + + var request = new StreamTestRequest { Value = "Hello" }; + await dispatcher.DispatchAsync(request).ToListAsync(); + + // Registered A (order 2) then B (order 1) via AddBehavior; explicit order overrides discovery order. + await Assert.That(tracker.Log).IsEquivalentTo(["OrderedB", "OrderedA"]); + } + [Test] public async Task I_can_work_with_a_generic_wrapper_request_and_the_open_behaviour() { diff --git a/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs b/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs new file mode 100644 index 0000000..1eab048 --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs @@ -0,0 +1,22 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public class OrderedScalarBehaviorA(ScalarTestTracker tracker) : IScalarRequestBehavior +{ + public async Task HandleAsync(ScalarTestRequest request, ScalarHandlerDelegate next, CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedA"); + return await next(request, cancellationToken); + } +} + +public class OrderedScalarBehaviorB(ScalarTestTracker tracker) : IScalarRequestBehavior +{ + public async Task HandleAsync(ScalarTestRequest request, ScalarHandlerDelegate next, CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedB"); + return await next(request, cancellationToken); + } +} diff --git a/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs b/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs new file mode 100644 index 0000000..0439f21 --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs @@ -0,0 +1,28 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public class OrderedStreamBehaviorA(StreamTestTracker tracker) : IStreamRequestBehavior +{ + public async IAsyncEnumerable HandleAsync(StreamTestRequest request, StreamHandlerDelegate next, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedA"); + await foreach (var item in next(request, cancellationToken)) + { + yield return item; + } + } +} + +public class OrderedStreamBehaviorB(StreamTestTracker tracker) : IStreamRequestBehavior +{ + public async IAsyncEnumerable HandleAsync(StreamTestRequest request, StreamHandlerDelegate next, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedB"); + await foreach (var item in next(request, cancellationToken)) + { + yield return item; + } + } +} diff --git a/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs b/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs index 8d53e3b..c2b6976 100644 --- a/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs +++ b/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs @@ -70,6 +70,27 @@ public static class RequestDispatcherBuilderExtensions return builder; } + /// + /// Adds the specified pipeline behavior to the request dispatcher configuration, with an explicit + /// pipeline used to determine its position in the pipeline. Lower values run + /// first (outermost); behaviors with equal order run in registration order. + /// + /// The behavior type to add. Must implement a request behavior interface. + /// The to configure. + /// The pipeline order for this behavior. Defaults to 0. + /// The instance for further configuration. + public static IRequestDispatcherBuilder AddBehavior(this IRequestDispatcherBuilder builder, int order = 0) + where TBehavior : class + { + ArgumentNullException.ThrowIfNull(builder); + ValidateNoNestedRequestHandlers([typeof(TBehavior)]); + + builder.Services.AddOptions() + .Configure(options => options.Inspect([typeof(TBehavior)], order)); + + return builder; + } + /// /// Adds the specified type to the request dispatcher configuration for inspection. /// diff --git a/src/request.dispatcher/RequestDispatcherOptions.cs b/src/request.dispatcher/RequestDispatcherOptions.cs index 6560f46..f7f8c68 100644 --- a/src/request.dispatcher/RequestDispatcherOptions.cs +++ b/src/request.dispatcher/RequestDispatcherOptions.cs @@ -10,24 +10,27 @@ namespace Geekeey.Request.Dispatcher; internal sealed class RequestDispatcherOptions { - private readonly List _search = []; + private readonly List<(Type Type, int? Order)> _search = []; private readonly Lazy _behaviorsTypeIndex; private readonly Lazy _handlersTypeIndex; public RequestDispatcherOptions() { - _behaviorsTypeIndex = new Lazy(() => new BehaviorTypeIndex(_search.Distinct())); - _handlersTypeIndex = new Lazy(() => new HandlerTypeIndex(_search.Distinct())); + _behaviorsTypeIndex = new Lazy(() => new BehaviorTypeIndex(_search.DistinctBy(x => x.Type))); + _handlersTypeIndex = new Lazy(() => new HandlerTypeIndex(_search.DistinctBy(x => x.Type))); } - public void Inspect(IEnumerable assembly) + public void Inspect(IEnumerable assembly, int? order = null) { if (_behaviorsTypeIndex.IsValueCreated || _handlersTypeIndex.IsValueCreated) { throw new InvalidOperationException("The type index has already been created. Cannot inspect new assemblies."); } - _search.AddRange(assembly); + foreach (var type in assembly) + { + _search.Add((type, order)); + } } public IEnumerable GetRequestBehaviors(IServiceProvider services) @@ -47,15 +50,21 @@ internal sealed class RequestDispatcherOptions protected readonly Dictionary> _closedTypeInfo = []; protected readonly List _openTypeInfo = []; protected readonly Dictionary _order = []; + protected readonly Dictionary _explicitOrder = []; - protected TypeIndex(IEnumerable collection, Func predicate) + protected TypeIndex(IEnumerable<(Type Type, int? Order)> collection, Func predicate) { var index = 0; - foreach (var type in collection) + foreach (var (type, order) in collection) { _order[type] = index++; + if (order is { } explicitOrder) + { + _explicitOrder[type] = explicitOrder; + } + if (type.IsGenericTypeDefinition) { if (type.GetInterfaces().Any(predicate)) @@ -73,6 +82,11 @@ internal sealed class RequestDispatcherOptions } } + protected int EffectiveOrder(Type type) + { + return _explicitOrder.TryGetValue(type, out var order) ? order : _order[type]; + } + public IEnumerable Resolve(IServiceProvider services) { return (IEnumerable)_cache.GetOrAdd(typeof(T), CreateResolverFactory)(services); @@ -102,7 +116,7 @@ internal sealed class RequestDispatcherOptions type.GetGenericTypeDefinition() == typeof(IStreamRequestHandler<,>)); } - private sealed class HandlerTypeIndex(IEnumerable collection) + private sealed class HandlerTypeIndex(IEnumerable<(Type Type, int? Order)> collection) : TypeIndex(collection, IsRequestHandlerType) { protected override IReadOnlyList IsAssignableTo(Type @interface) @@ -113,7 +127,7 @@ internal sealed class RequestDispatcherOptions { foreach (var type in list) { - result.Add((_order[type], type)); + result.Add((EffectiveOrder(type), type)); } } @@ -129,7 +143,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[type], impl)); + result.Add((EffectiveOrder(type), impl)); } } catch (ArgumentException) @@ -145,7 +159,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[type], impl)); + result.Add((EffectiveOrder(type), impl)); } } } @@ -154,7 +168,7 @@ internal sealed class RequestDispatcherOptions } } - return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); + return [.. result.OrderBy(static item => item.Order).Select(static item => item.Type)]; } } @@ -165,7 +179,7 @@ internal sealed class RequestDispatcherOptions type.GetGenericTypeDefinition() == typeof(IStreamRequestBehavior<,>)); } - private sealed class BehaviorTypeIndex(IEnumerable collection) + private sealed class BehaviorTypeIndex(IEnumerable<(Type Type, int? Order)> collection) : TypeIndex(collection, IsRequestBehaviorType) { protected override IReadOnlyList IsAssignableTo(Type @interface) @@ -176,7 +190,7 @@ internal sealed class RequestDispatcherOptions { foreach (var type in list) { - result.Add((_order[type], type)); + result.Add((EffectiveOrder(type), type)); } } @@ -191,7 +205,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType, responseType); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[behaviour], impl)); + result.Add((EffectiveOrder(behaviour), impl)); } } catch (ArgumentException) @@ -204,7 +218,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[behaviour], impl)); + result.Add((EffectiveOrder(behaviour), impl)); } } catch (ArgumentException) @@ -212,7 +226,7 @@ internal sealed class RequestDispatcherOptions } } - return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); + return [.. result.OrderBy(static item => item.Order).Select(static item => item.Type)]; } } } From 5b368602fe47d6edf18f116244c2c039ace2f2ff Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:35:50 +0200 Subject: [PATCH 08/14] docs(result): document that failure equality ignores error content Result/Result equality treats any two failures as equal regardless of their Error, which was undocumented and surprising for failure-specific branching. --- CHANGELOG.md | 2 ++ .../ResultEqualityTests.cs | 20 +++++++++++++++++++ src/request.result/Result.Equality.cs | 20 ++++++++++++++++--- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f9bdb..006fd7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ To have a consistent experience across all packages, some public interfaces have ### Changed +- **request.result:** Document that `Result`/`Result` equality treats any two failures as equal, ignoring error content; branch on the `Error` directly for failure-specific logic + ### Fixed - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) diff --git a/src/request.result.tests/ResultEqualityTests.cs b/src/request.result.tests/ResultEqualityTests.cs index ad54683..99d7094 100644 --- a/src/request.result.tests/ResultEqualityTests.cs +++ b/src/request.result.tests/ResultEqualityTests.cs @@ -190,6 +190,26 @@ internal sealed class ResultEqualityTests await Assert.That(success.GetHashCode()).IsNotEqualTo(failure.GetHashCode()); } + [Test] + public async Task I_can_equal_failure_and_failure_ignoring_error_content() + { + var a = Prelude.Failure("error 1"); + var b = Prelude.Failure("error 2"); + + await Assert.That(a.Equals(b)).IsTrue(); + await Assert.That(a.Error).IsNotEqualTo(b.Error); + } + + [Test] + public async Task I_can_equal_non_generic_failure_and_failure_ignoring_error_content() + { + var a = Prelude.Failure("error 1"); + var b = Prelude.Failure("error 2"); + + await Assert.That(a.Equals(b)).IsTrue(); + await Assert.That(a.Error).IsNotEqualTo(b.Error); + } + [Test] public async Task I_can_equal_non_generic_result_and_get_true_for_success_and_success() { diff --git a/src/request.result/Result.Equality.cs b/src/request.result/Result.Equality.cs index 2a8cbbb..da9372d 100644 --- a/src/request.result/Result.Equality.cs +++ b/src/request.result/Result.Equality.cs @@ -9,7 +9,15 @@ namespace Geekeey.Request.Result; public partial class Result : IEquatable { - /// + /// + /// Checks whether two results are equal. Results are equal if both are success or both are failure. + /// + /// + /// Two failures are considered equal regardless of their content. Equality therefore + /// ignores the error; if you need to branch on a specific failure, compare the values + /// directly instead of relying on equality. + /// + /// The result to check for equality with the current result. [Pure] public bool Equals(Result? other) { @@ -49,7 +57,8 @@ public partial class Result : IEquatable public partial class Result : IEqualityOperators { /// - /// Checks whether two results are equal. Results are equal if they are both success or both failure. + /// Checks whether two results are equal. Results are equal if they are both success or both failure. Two + /// failures are equal regardless of their error content. /// /// The first result to compare. /// The second result to compare. @@ -79,6 +88,11 @@ public partial class Result : IEquatable>, IEquatable /// Checks whether the result is equal to another result. Results are equal if both results are success values and /// the success values are equal, or if both results are failures. /// + /// + /// Two failures are considered equal regardless of their content. Equality therefore + /// ignores the error; if you need to branch on a specific failure, compare the values + /// directly instead of relying on equality. + /// /// The result to check for equality with the current result. [Pure] public bool Equals(Result? other) @@ -194,7 +208,7 @@ public partial class Result : IEqualityOperators, Result, bool>, { /// /// Checks whether two results are equal. Results are equal if both results are success values and the success - /// values are equal, or if both results are failures. + /// values are equal, or if both results are failures. Two failures are equal regardless of their error content. /// /// The first result to compare. /// The second result to compare. From fd6ed647557a314558ae9aa3612db6ec792fdd21 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:42:07 +0200 Subject: [PATCH 09/14] fix(validation): guard null comparison bounds in comparison builders Passing a null bound to GreaterThan/LessThan/Between etc. called value.CompareTo(null), throwing NullReferenceException for non-null reference-type values. Short-circuit when the bound is null so the rule is satisfied instead of crashing. --- CHANGELOG.md | 1 + .../RuleBuilderExtensionsTests.cs | 65 +++++++++++++++++++ .../_fixtures/ComparableModel.cs | 19 ++++++ .../RuleBuilderExtensions.cs | 12 ++-- 4 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 src/request.validation.tests/_fixtures/ComparableModel.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 006fd7b..a7ba39b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` +- **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) ### Removed diff --git a/src/request.validation.tests/RuleBuilderExtensionsTests.cs b/src/request.validation.tests/RuleBuilderExtensionsTests.cs index 5b5271a..c0d2cae 100644 --- a/src/request.validation.tests/RuleBuilderExtensionsTests.cs +++ b/src/request.validation.tests/RuleBuilderExtensionsTests.cs @@ -314,6 +314,71 @@ internal sealed class RuleBuilderExtensionsTests .Throws(); } + [Test] + public async Task I_can_validate_greater_than_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.GreaterThan(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_greater_than_or_equal_to_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.GreaterThanOrEqualTo(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_less_than_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.LessThan(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_less_than_or_equal_to_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.LessThanOrEqualTo(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_between_with_null_bounds_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.Between(null, null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + [Test] public async Task I_can_see_it_throw_for_invalid_between_configuration() { diff --git a/src/request.validation.tests/_fixtures/ComparableModel.cs b/src/request.validation.tests/_fixtures/ComparableModel.cs new file mode 100644 index 0000000..9178228 --- /dev/null +++ b/src/request.validation.tests/_fixtures/ComparableModel.cs @@ -0,0 +1,19 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Validation.Tests; + +internal sealed class ComparableModel +{ + public ComparableValue? Value { get; init; } +} + +internal sealed class ComparableValue : IComparable +{ + public int Number { get; init; } + + public int CompareTo(ComparableValue? other) + { + return Number.CompareTo(other!.Number); + } +} diff --git a/src/request.validation/RuleBuilderExtensions.cs b/src/request.validation/RuleBuilderExtensions.cs index 528b387..7e1a5db 100644 --- a/src/request.validation/RuleBuilderExtensions.cs +++ b/src/request.validation/RuleBuilderExtensions.cs @@ -170,7 +170,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) > 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) > 0, $"Value must be greater than {comparisonValue}."); } @@ -189,7 +189,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) >= 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) >= 0, $"Value must be greater than or equal to {comparisonValue}."); } @@ -208,7 +208,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) < 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) < 0, $"Value must be less than {comparisonValue}."); } @@ -227,7 +227,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) <= 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) <= 0, $"Value must be less than or equal to {comparisonValue}."); } @@ -254,7 +254,9 @@ public static class RuleBuilderExtensions "Maximum value must be greater than or equal to minimum value."); } - return rule.Must(value => IsNull(value) || (value.CompareTo(minValue) >= 0 && value.CompareTo(maxValue) <= 0), + return rule.Must(value => IsNull(value) + || ((minValue is null || value.CompareTo(minValue) >= 0) + && (maxValue is null || value.CompareTo(maxValue) <= 0)), $"Value must be between {minValue} and {maxValue}."); } From 581b0a1cedcdb474c6369a6fc395ac68ece63921 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:43:37 +0200 Subject: [PATCH 10/14] fix(validation): return no problems for null reference instance in Rule.Validate Rule.Validate cast a null instance to T and validated it, so a reference T whose accessor dereferences the instance threw NullReferenceException on a direct validator.Validate(context) with a null instance. Guard the null-instance path and return no problems instead. --- CHANGELOG.md | 1 + src/request.validation.tests/ValidatorTests.cs | 11 +++++++++++ src/request.validation/Rule.cs | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7ba39b..c2c6fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` - **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) +- **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) ### Removed diff --git a/src/request.validation.tests/ValidatorTests.cs b/src/request.validation.tests/ValidatorTests.cs index 14239f7..6613720 100644 --- a/src/request.validation.tests/ValidatorTests.cs +++ b/src/request.validation.tests/ValidatorTests.cs @@ -45,6 +45,17 @@ internal sealed class ValidatorTests } } + [Test] + public async Task I_can_validate_with_a_null_instance_without_throwing() + { + var validator = new PropertyValidator(person + => person!.Name, rule => rule.Must(value => value is not null, "Name is required.")); + + var result = validator.Validate((Person?)null); + + await Assert.That(result.IsValid).IsTrue(); + } + [Test] public async Task I_can_compose_nested_validators_and_aggregate_property_paths() { diff --git a/src/request.validation/Rule.cs b/src/request.validation/Rule.cs index 0c65cc0..a6a397c 100644 --- a/src/request.validation/Rule.cs +++ b/src/request.validation/Rule.cs @@ -46,7 +46,7 @@ internal abstract record Rule : Rule if (context.Instance is null && default(T) is null) { - return Validate((T)context.Instance!, context); + return []; } var actualType = context.Instance?.GetType().FullName ?? "null"; From 15bf9a8e56359345430470853d32aea173a0b0f8 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:50:06 +0200 Subject: [PATCH 11/14] fix(validation): make Severity meaningful in Validation.IsValid Validation.IsValid previously counted every problem regardless of severity, leaving Severity inert. Now IsValid only fails on Error problems, so Warning and Info no longer invalidate the result, and IsValidFor(Severity) lets callers set the failing-severity threshold. --- CHANGELOG.md | 1 + .../ValidationSeverityTests.cs | 49 +++++++++++++++++++ .../ValidatorTests.cs | 6 ++- src/request.validation/Validation.cs | 22 ++++++++- 4 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 src/request.validation.tests/ValidationSeverityTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index c2c6fb0..c41600a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` - **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) - **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) +- **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold ### Removed diff --git a/src/request.validation.tests/ValidationSeverityTests.cs b/src/request.validation.tests/ValidationSeverityTests.cs new file mode 100644 index 0000000..6f24c8d --- /dev/null +++ b/src/request.validation.tests/ValidationSeverityTests.cs @@ -0,0 +1,49 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Validation.Tests; + +internal sealed class ValidationSeverityTests +{ + [Test] + public async Task I_can_have_warning_problems_not_invalidate_by_default() + { + var validator = new PropertyValidator(person + => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.") + .WithSeverity(Severity.Warning)); + + var result = validator.Validate(new Person { Name = "" }); + + await Assert.That(result.Problems).Count().IsEqualTo(1); + await Assert.That(result.IsValid).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Warning)).IsFalse(); + await Assert.That(result.IsValidFor(Severity.Info)).IsFalse(); + } + + [Test] + public async Task I_can_have_info_problems_only_invalidate_at_info_threshold() + { + var validator = new PropertyValidator(person + => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.") + .WithSeverity(Severity.Info)); + + var result = validator.Validate(new Person { Name = "" }); + + await Assert.That(result.Problems).Count().IsEqualTo(1); + await Assert.That(result.IsValid).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Warning)).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Info)).IsFalse(); + } + + [Test] + public async Task I_can_have_error_problems_invalidate_by_default() + { + var validator = new PropertyValidator(person + => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.")); + + var result = validator.Validate(new Person { Name = "" }); + + await Assert.That(result.IsValid).IsFalse(); + await Assert.That(result.IsValidFor(Severity.Error)).IsFalse(); + } +} diff --git a/src/request.validation.tests/ValidatorTests.cs b/src/request.validation.tests/ValidatorTests.cs index 6613720..820edc4 100644 --- a/src/request.validation.tests/ValidatorTests.cs +++ b/src/request.validation.tests/ValidatorTests.cs @@ -14,7 +14,8 @@ internal sealed class ValidatorTests var result = validator.Validate(new Person { Name = "" }); - await Assert.That(result.IsValid).IsFalse(); + await Assert.That(result.IsValid).IsTrue(); + await Assert.That(result.IsValidFor(Severity.Warning)).IsFalse(); await Assert.That(result.Problems).Count().IsEqualTo(1); var problem = result.Problems.Single(); @@ -39,7 +40,8 @@ internal sealed class ValidatorTests using (Assert.Multiple()) { - await Assert.That(invalid.IsValid).IsFalse(); + await Assert.That(invalid.IsValid).IsTrue(); + await Assert.That(invalid.IsValidFor(Severity.Warning)).IsFalse(); await Assert.That(valid.IsValid).IsTrue(); await Assert.That(valid.Problems).IsEmpty(); } diff --git a/src/request.validation/Validation.cs b/src/request.validation/Validation.cs index fadadff..8e9c7c7 100644 --- a/src/request.validation/Validation.cs +++ b/src/request.validation/Validation.cs @@ -23,7 +23,27 @@ public sealed class Validation /// /// Whether the validation was successful. /// - public bool IsValid => Problems.Count is 0; + /// + /// A validation is considered successful unless it contains at least one problem with + /// . Problems of lower severity ( or + /// ) do not invalidate the result. Use + /// to control which severities are treated as failures. + /// + public bool IsValid => IsValidFor(Severity.Error); + + /// + /// Whether the validation was successful, treating problems at or above the given severity as failures. + /// + /// The minimum severity that should be considered a failure. + /// + /// A problem invalidates the validation when its is at least as severe as + /// . For example, IsValidFor(Severity.Warning) fails on errors and + /// warnings but not on informational problems. + /// + public bool IsValidFor(Severity minimum) + { + return !Problems.Any(problem => problem.Severity <= minimum); + } /// /// The problems that were found during validation. From a7d7e9fbaade2f5abaf8c901a8221681df1e45d2 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 19:18:21 +0200 Subject: [PATCH 12/14] fix(dispatcher): throw on ambiguous handlers instead of silent .First() Multiple handlers for the same request were previously resolved via .First(), with ordering depending on Type.GetInterfaces() order (non-deterministic). Now ScalarRequestInvoker/StreamRequestInvoker throw InvalidOperationException listing the candidate handlers when more than one matches. --- CHANGELOG.md | 1 + .../ScalarDispatcherTests.cs | 33 ++++++++++++++++--- .../StreamDispatcherTests.cs | 31 +++++++++++++++-- .../_fixtures/DuplicateScalarHandler.cs | 24 ++++++++++++++ .../_fixtures/DuplicateStreamHandler.cs | 24 ++++++++++++++ .../ScalarRequestInvoker.cs | 15 ++++++++- .../StreamRequestInvoker.cs | 15 ++++++++- 7 files changed, 134 insertions(+), 9 deletions(-) create mode 100644 src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs create mode 100644 src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index c41600a..b05e20b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) - **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) - **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold +- **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()`. Also throws when no handler is registered ### Removed diff --git a/src/request.dispatcher.tests/ScalarDispatcherTests.cs b/src/request.dispatcher.tests/ScalarDispatcherTests.cs index 5c10998..8c5ffb1 100644 --- a/src/request.dispatcher.tests/ScalarDispatcherTests.cs +++ b/src/request.dispatcher.tests/ScalarDispatcherTests.cs @@ -116,7 +116,7 @@ internal sealed class ScalarDispatcherTests } [Test] - public async Task I_can_dispatch_a_request_async_with_an_interface_constrained_handler() + public async Task I_can_see_it_throw_on_ambiguous_concrete_and_constrained_scalar_handlers() { var sc = new ServiceCollection(); sc.AddRequestDispatcher(builder => builder @@ -126,13 +126,18 @@ internal sealed class ScalarDispatcherTests var dispatcher = provider.GetRequiredService(); var request = new InterfaceInheritedScalarRequest { Name = "Constrained" }; - var result = await dispatcher.DispatchAsync(request); // Both InterfaceInheritedScalarHandler and InterfaceConstrainedScalarHandler could match. // InterfaceInheritedScalarHandler is a concrete match for InterfaceInheritedScalarRequest. // InterfaceConstrainedScalarHandler is an open generic match. - // Currently Dispatcher.SendAsync checks concrete handlers first. - await Assert.That(result).IsEquivalentTo("Constrained-InterfaceHandled"); + // Ambiguity is no longer resolved silently with .First(); it throws instead. + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedScalarHandler)); + await Assert.That(ex?.Message).Contains("InterfaceConstrainedScalarHandler"); + } } [Test] @@ -234,6 +239,26 @@ internal sealed class ScalarDispatcherTests } } + [Test] + public async Task I_can_see_it_throw_on_ambiguous_scalar_handlers() + { + var sc = new ServiceCollection(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(DuplicateScalarHandlerA)) + .Add(typeof(DuplicateScalarHandlerB))); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + + var request = new DuplicateScalarRequest(); + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(DuplicateScalarHandlerA)); + await Assert.That(ex?.Message).Contains(nameof(DuplicateScalarHandlerB)); + } + } + [Test] public async Task I_can_see_it_throw_if_dispatcher_options_are_modified_after_build() { diff --git a/src/request.dispatcher.tests/StreamDispatcherTests.cs b/src/request.dispatcher.tests/StreamDispatcherTests.cs index c06cb36..5fcc7b2 100644 --- a/src/request.dispatcher.tests/StreamDispatcherTests.cs +++ b/src/request.dispatcher.tests/StreamDispatcherTests.cs @@ -7,6 +7,26 @@ namespace Geekeey.Request.Dispatcher.Tests; public class StreamDispatcherTests { + [Test] + public async Task I_can_see_it_throw_on_ambiguous_stream_handlers() + { + var sc = new ServiceCollection(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(DuplicateStreamHandlerA)) + .Add(typeof(DuplicateStreamHandlerB))); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + + var request = new DuplicateStreamRequest(); + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(DuplicateStreamHandlerA)); + await Assert.That(ex?.Message).Contains(nameof(DuplicateStreamHandlerB)); + } + } + [Test] public async Task I_can_dispatch_a_request_async_with_an_open_generic_handler() { @@ -121,13 +141,18 @@ public class StreamDispatcherTests var dispatcher = provider.GetRequiredService(); var request = new InterfaceInheritedStreamRequest { Name = "Constrained" }; - var results = await dispatcher.DispatchAsync(request).ToListAsync(); // Both InterfaceInheritedStreamHandler and InterfaceConstrainedStreamHandler could match. // InterfaceInheritedStreamHandler is a concrete match for InterfaceInheritedStreamRequest. // InterfaceConstrainedStreamHandler is an open generic match. - // Currently Dispatcher checks concrete handlers first. - await Assert.That(results).IsEquivalentTo(["Constrained-InterfaceHandled-0", "Constrained-InterfaceHandled-1"]); + // Ambiguity is no longer resolved silently with .First(); it throws instead. + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws(); + + using (Assert.Multiple()) + { + await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedStreamHandler)); + await Assert.That(ex?.Message).Contains("InterfaceConstrainedStreamHandler"); + } } [Test] diff --git a/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs b/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs new file mode 100644 index 0000000..6a0504e --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs @@ -0,0 +1,24 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public sealed class DuplicateScalarRequest : IScalarRequest +{ +} + +public sealed class DuplicateScalarHandlerA : IScalarRequestHandler +{ + public Task HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken) + { + return Task.FromResult("A"); + } +} + +public sealed class DuplicateScalarHandlerB : IScalarRequestHandler +{ + public Task HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken) + { + return Task.FromResult("B"); + } +} diff --git a/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs b/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs new file mode 100644 index 0000000..d3f0414 --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs @@ -0,0 +1,24 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public sealed class DuplicateStreamRequest : IStreamRequest +{ +} + +public sealed class DuplicateStreamHandlerA : IStreamRequestHandler +{ + public async IAsyncEnumerable HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + yield return "A"; + } +} + +public sealed class DuplicateStreamHandlerB : IStreamRequestHandler +{ + public async IAsyncEnumerable HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + yield return "B"; + } +} diff --git a/src/request.dispatcher/ScalarRequestInvoker.cs b/src/request.dispatcher/ScalarRequestInvoker.cs index 63de03e..2c6d129 100644 --- a/src/request.dispatcher/ScalarRequestInvoker.cs +++ b/src/request.dispatcher/ScalarRequestInvoker.cs @@ -43,7 +43,20 @@ internal sealed class ScalarRequestInvoker : ScalarRequestI Task Head(IScalarRequest r, CancellationToken ct) { - return options.GetRequestHandlers>(serviceProvider).First().HandleAsync((TRequest)r, ct); + var handlers = options.GetRequestHandlers>(serviceProvider).ToArray(); + + if (handlers.Length is 0) + { + throw new InvalidOperationException($"No handler is registered for request '{typeof(TRequest).FullName}'."); + } + + if (handlers.Length > 1) + { + var candidates = string.Join(", ", handlers.Select(handler => handler.GetType().FullName)); + throw new InvalidOperationException($"Multiple handlers are registered for request '{typeof(TRequest).FullName}'. Ambiguous handlers: {candidates}"); + } + + return handlers[0].HandleAsync((TRequest)r, ct); } } } diff --git a/src/request.dispatcher/StreamRequestInvoker.cs b/src/request.dispatcher/StreamRequestInvoker.cs index 882d538..817b5b2 100644 --- a/src/request.dispatcher/StreamRequestInvoker.cs +++ b/src/request.dispatcher/StreamRequestInvoker.cs @@ -47,7 +47,20 @@ internal sealed class StreamRequestInvoker : StreamRequestI IAsyncEnumerable Head(IStreamRequest r, CancellationToken ct) { - return options.GetRequestHandlers>(serviceProvider).First().HandleAsync((TRequest)r, ct); + var handlers = options.GetRequestHandlers>(serviceProvider).ToArray(); + + if (handlers.Length is 0) + { + throw new InvalidOperationException($"No handler is registered for request '{typeof(TRequest).FullName}'."); + } + + if (handlers.Length > 1) + { + var candidates = string.Join(", ", handlers.Select(handler => handler.GetType().FullName)); + throw new InvalidOperationException($"Multiple handlers are registered for request '{typeof(TRequest).FullName}'. Ambiguous handlers: {candidates}"); + } + + return handlers[0].HandleAsync((TRequest)r, ct); } } } From 8099898f3d29c00a47a11761fc7ce113fd1fccee Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 19:22:50 +0200 Subject: [PATCH 13/14] fix(dispatcher): resolve handlers in registration order across closed and open generics Handler/behavior resolution previously forced closed generics before open generics regardless of registration order, which was surprising and fragile. TypeIndex now tracks each registered type's index and orders matching candidates by registration order, so closed/open ordering is deterministic and follows registration. --- CHANGELOG.md | 1 + .../ScalarDispatcherTests.cs | 23 ++++++++++++++ .../RequestDispatcherOptions.cs | 31 +++++++++++++------ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b05e20b..0541f06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) - **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold - **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()`. Also throws when no handler is registered +- **request.dispatcher:** Resolve handlers/behaviors in registration order across closed and open generics instead of the fixed closed-before-open reflection order ### Removed diff --git a/src/request.dispatcher.tests/ScalarDispatcherTests.cs b/src/request.dispatcher.tests/ScalarDispatcherTests.cs index 8c5ffb1..f274da9 100644 --- a/src/request.dispatcher.tests/ScalarDispatcherTests.cs +++ b/src/request.dispatcher.tests/ScalarDispatcherTests.cs @@ -259,6 +259,29 @@ internal sealed class ScalarDispatcherTests } } + [Test] + public async Task I_can_see_candidates_ordered_by_registration_not_closed_first() + { + var sc = new ServiceCollection(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(InterfaceConstrainedScalarHandler<>)) + .Add(typeof(InterfaceInheritedScalarHandler))); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + + var request = new InterfaceInheritedScalarRequest { Name = "Constrained" }; + + // The open generic handler is registered before the closed handler, so candidates must be + // listed in registration order (open first), not the old fixed closed-before-open order. + var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); + + var message = ex?.Message ?? string.Empty; + var openIndex = message.IndexOf("InterfaceConstrainedScalarHandler", StringComparison.Ordinal); + var closedIndex = message.IndexOf("InterfaceInheritedScalarHandler", StringComparison.Ordinal); + + await Assert.That(openIndex >= 0 && closedIndex >= 0 && openIndex < closedIndex).IsTrue(); + } + [Test] public async Task I_can_see_it_throw_if_dispatcher_options_are_modified_after_build() { diff --git a/src/request.dispatcher/RequestDispatcherOptions.cs b/src/request.dispatcher/RequestDispatcherOptions.cs index d18def9..6560f46 100644 --- a/src/request.dispatcher/RequestDispatcherOptions.cs +++ b/src/request.dispatcher/RequestDispatcherOptions.cs @@ -46,11 +46,16 @@ internal sealed class RequestDispatcherOptions protected readonly Dictionary> _closedTypeInfo = []; protected readonly List _openTypeInfo = []; + protected readonly Dictionary _order = []; protected TypeIndex(IEnumerable collection, Func predicate) { + var index = 0; + foreach (var type in collection) { + _order[type] = index++; + if (type.IsGenericTypeDefinition) { if (type.GetInterfaces().Any(predicate)) @@ -102,11 +107,14 @@ internal sealed class RequestDispatcherOptions { protected override IReadOnlyList IsAssignableTo(Type @interface) { - var result = new List(); + var result = new List<(int Order, Type Type)>(); if (_closedTypeInfo.TryGetValue(@interface, out var list)) { - result.AddRange(list); + foreach (var type in list) + { + result.Add((_order[type], type)); + } } var requestType = @interface.GetGenericArguments()[0]; @@ -121,7 +129,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[type], impl)); } } catch (ArgumentException) @@ -137,7 +145,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[type], impl)); } } } @@ -146,7 +154,7 @@ internal sealed class RequestDispatcherOptions } } - return result; + return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); } } @@ -162,11 +170,14 @@ internal sealed class RequestDispatcherOptions { protected override IReadOnlyList IsAssignableTo(Type @interface) { - var result = new List(); + var result = new List<(int Order, Type Type)>(); if (_closedTypeInfo.TryGetValue(@interface, out var list)) { - result.AddRange(list); + foreach (var type in list) + { + result.Add((_order[type], type)); + } } var requestType = @interface.GetGenericArguments()[0]; @@ -180,7 +191,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType, responseType); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[behaviour], impl)); } } catch (ArgumentException) @@ -193,7 +204,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add(impl); + result.Add((_order[behaviour], impl)); } } catch (ArgumentException) @@ -201,7 +212,7 @@ internal sealed class RequestDispatcherOptions } } - return result; + return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); } } } From f9de3a99de51c4de520721a176462638a41507f6 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 19:30:59 +0200 Subject: [PATCH 14/14] feat(dispatcher): add registration-time pipeline behavior ordering via AddBehavior(order:) Pipeline behavior ordering previously depended on discovery/registration order. Add an AddBehavior(order:) API that captures the order at registration time; the BehaviorTypeIndex now orders matching behaviors by that explicit order (lower runs first), falling back to registration order for ties. Removed the runtime Order property approach. --- CHANGELOG.md | 1 + .../ScalarBehaviourTests.cs | 20 ++++++++ .../StreamBehaviourTests.cs | 20 ++++++++ .../_fixtures/OrderedScalarBehavior.cs | 22 +++++++++ .../_fixtures/OrderedStreamBehavior.cs | 28 +++++++++++ .../RequestDispatcherBuilderExtensions.cs | 21 ++++++++ .../RequestDispatcherOptions.cs | 48 ++++++++++++------- 7 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs create mode 100644 src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0541f06..a7052a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold - **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()`. Also throws when no handler is registered - **request.dispatcher:** Resolve handlers/behaviors in registration order across closed and open generics instead of the fixed closed-before-open reflection order +- **request.dispatcher:** Add `AddBehavior(order:)` registration-time API so pipeline behavior ordering is explicit and deterministic (lower order runs first, registration order as tie-break) instead of relying on reflection/discovery order ### Removed diff --git a/src/request.dispatcher.tests/ScalarBehaviourTests.cs b/src/request.dispatcher.tests/ScalarBehaviourTests.cs index 9c10e4b..d2a6dd6 100644 --- a/src/request.dispatcher.tests/ScalarBehaviourTests.cs +++ b/src/request.dispatcher.tests/ScalarBehaviourTests.cs @@ -87,6 +87,26 @@ internal sealed class ScalarBehaviourTests await Assert.That(tracker.Executed).IsTrue(); } + [Test] + public async Task I_can_order_behaviours_explicitly_by_order_property() + { + var sc = new ServiceCollection(); + sc.AddSingleton(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(ScalarTestHandler)) + .AddBehavior(2) + .AddBehavior(1)); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + var tracker = provider.GetRequiredService(); + + var request = new ScalarTestRequest { Value = "Hello" }; + await dispatcher.DispatchAsync(request); + + // Registered A (order 2) then B (order 1) via AddBehavior; explicit order overrides discovery order. + await Assert.That(tracker.Log).IsEquivalentTo(["OrderedB", "OrderedA"]); + } + [Test] public async Task I_can_maintain_the_ordering_between_open_and_closed_behaviours() { diff --git a/src/request.dispatcher.tests/StreamBehaviourTests.cs b/src/request.dispatcher.tests/StreamBehaviourTests.cs index c96b530..7e9377f 100644 --- a/src/request.dispatcher.tests/StreamBehaviourTests.cs +++ b/src/request.dispatcher.tests/StreamBehaviourTests.cs @@ -66,6 +66,26 @@ internal sealed class StreamBehaviourTests await Assert.That(tracker.Log[1]).IsEquivalentTo("Behaviour2"); } + [Test] + public async Task I_can_order_behaviours_explicitly_by_order_property() + { + var sc = new ServiceCollection(); + sc.AddSingleton(); + sc.AddRequestDispatcher(builder => builder + .Add(typeof(StreamTestHandler)) + .AddBehavior(2) + .AddBehavior(1)); + var provider = sc.BuildServiceProvider(); + var dispatcher = provider.GetRequiredService(); + var tracker = provider.GetRequiredService(); + + var request = new StreamTestRequest { Value = "Hello" }; + await dispatcher.DispatchAsync(request).ToListAsync(); + + // Registered A (order 2) then B (order 1) via AddBehavior; explicit order overrides discovery order. + await Assert.That(tracker.Log).IsEquivalentTo(["OrderedB", "OrderedA"]); + } + [Test] public async Task I_can_work_with_a_generic_wrapper_request_and_the_open_behaviour() { diff --git a/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs b/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs new file mode 100644 index 0000000..1eab048 --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs @@ -0,0 +1,22 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public class OrderedScalarBehaviorA(ScalarTestTracker tracker) : IScalarRequestBehavior +{ + public async Task HandleAsync(ScalarTestRequest request, ScalarHandlerDelegate next, CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedA"); + return await next(request, cancellationToken); + } +} + +public class OrderedScalarBehaviorB(ScalarTestTracker tracker) : IScalarRequestBehavior +{ + public async Task HandleAsync(ScalarTestRequest request, ScalarHandlerDelegate next, CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedB"); + return await next(request, cancellationToken); + } +} diff --git a/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs b/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs new file mode 100644 index 0000000..0439f21 --- /dev/null +++ b/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs @@ -0,0 +1,28 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Dispatcher.Tests; + +public class OrderedStreamBehaviorA(StreamTestTracker tracker) : IStreamRequestBehavior +{ + public async IAsyncEnumerable HandleAsync(StreamTestRequest request, StreamHandlerDelegate next, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedA"); + await foreach (var item in next(request, cancellationToken)) + { + yield return item; + } + } +} + +public class OrderedStreamBehaviorB(StreamTestTracker tracker) : IStreamRequestBehavior +{ + public async IAsyncEnumerable HandleAsync(StreamTestRequest request, StreamHandlerDelegate next, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) + { + tracker.Log.Add("OrderedB"); + await foreach (var item in next(request, cancellationToken)) + { + yield return item; + } + } +} diff --git a/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs b/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs index 8d53e3b..c2b6976 100644 --- a/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs +++ b/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs @@ -70,6 +70,27 @@ public static class RequestDispatcherBuilderExtensions return builder; } + /// + /// Adds the specified pipeline behavior to the request dispatcher configuration, with an explicit + /// pipeline used to determine its position in the pipeline. Lower values run + /// first (outermost); behaviors with equal order run in registration order. + /// + /// The behavior type to add. Must implement a request behavior interface. + /// The to configure. + /// The pipeline order for this behavior. Defaults to 0. + /// The instance for further configuration. + public static IRequestDispatcherBuilder AddBehavior(this IRequestDispatcherBuilder builder, int order = 0) + where TBehavior : class + { + ArgumentNullException.ThrowIfNull(builder); + ValidateNoNestedRequestHandlers([typeof(TBehavior)]); + + builder.Services.AddOptions() + .Configure(options => options.Inspect([typeof(TBehavior)], order)); + + return builder; + } + /// /// Adds the specified type to the request dispatcher configuration for inspection. /// diff --git a/src/request.dispatcher/RequestDispatcherOptions.cs b/src/request.dispatcher/RequestDispatcherOptions.cs index 6560f46..f7f8c68 100644 --- a/src/request.dispatcher/RequestDispatcherOptions.cs +++ b/src/request.dispatcher/RequestDispatcherOptions.cs @@ -10,24 +10,27 @@ namespace Geekeey.Request.Dispatcher; internal sealed class RequestDispatcherOptions { - private readonly List _search = []; + private readonly List<(Type Type, int? Order)> _search = []; private readonly Lazy _behaviorsTypeIndex; private readonly Lazy _handlersTypeIndex; public RequestDispatcherOptions() { - _behaviorsTypeIndex = new Lazy(() => new BehaviorTypeIndex(_search.Distinct())); - _handlersTypeIndex = new Lazy(() => new HandlerTypeIndex(_search.Distinct())); + _behaviorsTypeIndex = new Lazy(() => new BehaviorTypeIndex(_search.DistinctBy(x => x.Type))); + _handlersTypeIndex = new Lazy(() => new HandlerTypeIndex(_search.DistinctBy(x => x.Type))); } - public void Inspect(IEnumerable assembly) + public void Inspect(IEnumerable assembly, int? order = null) { if (_behaviorsTypeIndex.IsValueCreated || _handlersTypeIndex.IsValueCreated) { throw new InvalidOperationException("The type index has already been created. Cannot inspect new assemblies."); } - _search.AddRange(assembly); + foreach (var type in assembly) + { + _search.Add((type, order)); + } } public IEnumerable GetRequestBehaviors(IServiceProvider services) @@ -47,15 +50,21 @@ internal sealed class RequestDispatcherOptions protected readonly Dictionary> _closedTypeInfo = []; protected readonly List _openTypeInfo = []; protected readonly Dictionary _order = []; + protected readonly Dictionary _explicitOrder = []; - protected TypeIndex(IEnumerable collection, Func predicate) + protected TypeIndex(IEnumerable<(Type Type, int? Order)> collection, Func predicate) { var index = 0; - foreach (var type in collection) + foreach (var (type, order) in collection) { _order[type] = index++; + if (order is { } explicitOrder) + { + _explicitOrder[type] = explicitOrder; + } + if (type.IsGenericTypeDefinition) { if (type.GetInterfaces().Any(predicate)) @@ -73,6 +82,11 @@ internal sealed class RequestDispatcherOptions } } + protected int EffectiveOrder(Type type) + { + return _explicitOrder.TryGetValue(type, out var order) ? order : _order[type]; + } + public IEnumerable Resolve(IServiceProvider services) { return (IEnumerable)_cache.GetOrAdd(typeof(T), CreateResolverFactory)(services); @@ -102,7 +116,7 @@ internal sealed class RequestDispatcherOptions type.GetGenericTypeDefinition() == typeof(IStreamRequestHandler<,>)); } - private sealed class HandlerTypeIndex(IEnumerable collection) + private sealed class HandlerTypeIndex(IEnumerable<(Type Type, int? Order)> collection) : TypeIndex(collection, IsRequestHandlerType) { protected override IReadOnlyList IsAssignableTo(Type @interface) @@ -113,7 +127,7 @@ internal sealed class RequestDispatcherOptions { foreach (var type in list) { - result.Add((_order[type], type)); + result.Add((EffectiveOrder(type), type)); } } @@ -129,7 +143,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[type], impl)); + result.Add((EffectiveOrder(type), impl)); } } catch (ArgumentException) @@ -145,7 +159,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[type], impl)); + result.Add((EffectiveOrder(type), impl)); } } } @@ -154,7 +168,7 @@ internal sealed class RequestDispatcherOptions } } - return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); + return [.. result.OrderBy(static item => item.Order).Select(static item => item.Type)]; } } @@ -165,7 +179,7 @@ internal sealed class RequestDispatcherOptions type.GetGenericTypeDefinition() == typeof(IStreamRequestBehavior<,>)); } - private sealed class BehaviorTypeIndex(IEnumerable collection) + private sealed class BehaviorTypeIndex(IEnumerable<(Type Type, int? Order)> collection) : TypeIndex(collection, IsRequestBehaviorType) { protected override IReadOnlyList IsAssignableTo(Type @interface) @@ -176,7 +190,7 @@ internal sealed class RequestDispatcherOptions { foreach (var type in list) { - result.Add((_order[type], type)); + result.Add((EffectiveOrder(type), type)); } } @@ -191,7 +205,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType, responseType); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[behaviour], impl)); + result.Add((EffectiveOrder(behaviour), impl)); } } catch (ArgumentException) @@ -204,7 +218,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add((_order[behaviour], impl)); + result.Add((EffectiveOrder(behaviour), impl)); } } catch (ArgumentException) @@ -212,7 +226,7 @@ internal sealed class RequestDispatcherOptions } } - return result.OrderBy(x => x.Order).Select(x => x.Type).ToList(); + return [.. result.OrderBy(static item => item.Order).Select(static item => item.Type)]; } } }