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); } } }