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.
This commit is contained in:
parent
15bf9a8e56
commit
a7d7e9fbaa
7 changed files with 134 additions and 9 deletions
|
|
@ -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<IRequestDispatcher>();
|
||||
|
||||
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<InvalidOperationException>();
|
||||
|
||||
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<IRequestDispatcher>();
|
||||
|
||||
var request = new DuplicateScalarRequest();
|
||||
var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws<InvalidOperationException>();
|
||||
|
||||
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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<IRequestDispatcher>();
|
||||
|
||||
var request = new DuplicateStreamRequest();
|
||||
var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws<InvalidOperationException>();
|
||||
|
||||
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<IRequestDispatcher>();
|
||||
|
||||
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<InvalidOperationException>();
|
||||
|
||||
using (Assert.Multiple())
|
||||
{
|
||||
await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedStreamHandler));
|
||||
await Assert.That(ex?.Message).Contains("InterfaceConstrainedStreamHandler");
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
|
|
|
|||
|
|
@ -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<string>
|
||||
{
|
||||
}
|
||||
|
||||
public sealed class DuplicateScalarHandlerA : IScalarRequestHandler<DuplicateScalarRequest, string>
|
||||
{
|
||||
public Task<string> HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken)
|
||||
{
|
||||
return Task.FromResult("A");
|
||||
}
|
||||
}
|
||||
|
||||
public sealed class DuplicateScalarHandlerB : IScalarRequestHandler<DuplicateScalarRequest, string>
|
||||
{
|
||||
public Task<string> HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken)
|
||||
{
|
||||
return Task.FromResult("B");
|
||||
}
|
||||
}
|
||||
|
|
@ -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<string>
|
||||
{
|
||||
}
|
||||
|
||||
public sealed class DuplicateStreamHandlerA : IStreamRequestHandler<DuplicateStreamRequest, string>
|
||||
{
|
||||
public async IAsyncEnumerable<string> HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken)
|
||||
{
|
||||
yield return "A";
|
||||
}
|
||||
}
|
||||
|
||||
public sealed class DuplicateStreamHandlerB : IStreamRequestHandler<DuplicateStreamRequest, string>
|
||||
{
|
||||
public async IAsyncEnumerable<string> HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken)
|
||||
{
|
||||
yield return "B";
|
||||
}
|
||||
}
|
||||
|
|
@ -43,7 +43,20 @@ internal sealed class ScalarRequestInvoker<TRequest, TResponse> : ScalarRequestI
|
|||
|
||||
Task<TResponse> Head(IScalarRequest<TResponse> r, CancellationToken ct)
|
||||
{
|
||||
return options.GetRequestHandlers<IScalarRequestHandler<TRequest, TResponse>>(serviceProvider).First().HandleAsync((TRequest)r, ct);
|
||||
var handlers = options.GetRequestHandlers<IScalarRequestHandler<TRequest, TResponse>>(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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -47,7 +47,20 @@ internal sealed class StreamRequestInvoker<TRequest, TResponse> : StreamRequestI
|
|||
|
||||
IAsyncEnumerable<TResponse> Head(IStreamRequest<TResponse> r, CancellationToken ct)
|
||||
{
|
||||
return options.GetRequestHandlers<IStreamRequestHandler<TRequest, TResponse>>(serviceProvider).First().HandleAsync((TRequest)r, ct);
|
||||
var handlers = options.GetRequestHandlers<IStreamRequestHandler<TRequest, TResponse>>(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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue