Closed iappwebdev closed 11 months ago
Hm, it should not be mapping object
, it must have a proper type. I will try to reproduce it.
Hm, it should not be mapping
object
, it must have a proper type. I will try to reproduce it.
I could reproduce it and can give the fix to this, I'll create a pull request.
The main issue is still there, registered and recognised handlers are not found on execution.
Maybe we can start by adding a test with this scenario, so the issue would be clearly illustrated.
Maybe we can start by adding a test with this scenario, so the issue would be clearly illustrated.
I just did this today and found the problem. In file HttpCommandMapping.cs
: when using app.MapDiscoveredCommands<TAggregate>()
, the handlers are registered by using a generic made version of Map:
void MapAssemblyCommands(Assembly assembly) {
var decoratedTypes = assembly.DefinedTypes.Where(
x => x.IsClass && x.CustomAttributes.Any(a => a.AttributeType == attributeType)
);
var method = typeof(RouteBuilderExtensions).GetMethod(nameof(Map), BindingFlags.Static | BindingFlags.NonPublic)!;
foreach (var type in decoratedTypes) {
var attr = type.GetAttribute<HttpCommandAttribute>()!;
if (attr.AggregateType != null && attr.AggregateType != typeof(TAggregate))
throw new InvalidOperationException(
$"Command aggregate is {attr.AggregateType.Name} but expected to be {typeof(TAggregate).Name}"
);
var genericMethod = method.MakeGenericMethod(typeof(TAggregate), type, type);
genericMethod.Invoke(null, new object?[] { builder, attr.Route, null, attr.PolicyName });
}
}
When calling app.MapDiscoveredCommands()
, handlers are registered by a non generic version
void LocalMap(Type aggregateType, Type type, string? route, string? policyName) {
var appServiceBase = typeof(ICommandService<>);
var appServiceType = appServiceBase.MakeGenericType(aggregateType);
var routeBuilder = builder
.MapPost(
GetRoute(type, route),
async Task<IResult> (HttpContext context) => {
var cmd = await context.Request.ReadFromJsonAsync(type, context.RequestAborted);
if (cmd == null) throw new InvalidOperationException("Failed to deserialize the command");
if (context.RequestServices.GetRequiredService(appServiceType) is not ICommandService
service) throw new InvalidOperationException("Unable to resolve the application service");
var result = await service.Handle(cmd, context.RequestAborted);
return result.AsResult();
}
)
Here, deeper at await service.Handle(cmd, context.RequestAborted);
the generic CommandType information gets lost.
I introduced a new (duplicated) command for BookRoom with a wrapper class annotated with AggregateCommands
in BookService
:
[AggregateCommands<Booking>]
static class NestedCommands {
[HttpCommand(Route = "nested-book")]
internal record NestedBookRoom(string BookingId, string RoomId, LocalDate CheckIn, LocalDate CheckOut, float Price, string? GuestId);
}
Then I extended HttpCommandTests
to test MapDiscoveredCommands()
[Fact]
public void RegisterAggregatesCommands() {
var builder = WebApplication.CreateBuilder();
using var app = builder.Build();
var b = app.MapDiscoveredCommands(typeof(NestedCommands).Assembly);
b.DataSources.First().Endpoints[0].DisplayName.Should().Be("HTTP: POST nested-book");
}
The test passed so registering commands using MapDiscoveredCommands()
works correctly.
Next I introducded a test for executing the registered command:
[Fact]
public async Task MapDiscoveredCommand() {
using var fixture = new ServerFixture(
output,
_ => { },
app => app.MapDiscoveredCommands(typeof(NestedCommands).Assembly)
);
using var client = fixture.GetClient();
var cmd = fixture.GetNestedBookRoom();
var response = await client.PostJsonAsync("/nested-book", cmd);
response.Should().Be(HttpStatusCode.OK);
}
The test failed with eventuous.application - EventId: [1], EventName: [CommandHandlerNotFound], Message: [Handler not found for command: 'NestedBookRoom']
I took a close look on how handlers are registered and how they are found. In CommandService
, handlers are registered by AddHandlerUntyped()
void BuildHandlers() {
lock (_handlersLock) {
foreach (var commandType in _builders.Keys) {
var builder = _builders[commandType];
var handler = builder.Build();
_handlers.AddHandlerUntyped(commandType, handler);
}
_initialized = true;
}
}
Command are then searched by using
public async Task<Result<TState>> Handle<TCommand>(TCommand command, CancellationToken cancellationToken) where TCommand : class {
if (!_initialized) BuildHandlers();
if (!_handlers.TryGet<TCommand>(out var registeredHandler)) {
Log.CommandHandlerNotFound<TCommand>();
...
As we saw before, the type information gets lost and TCommand
is of type object
when using app.MapDiscoveredCommands()
. So I extended CommandHandlersMap
by an additional method TryGetUntyped()
:
static readonly MethodInfo TryGetInternalMethod =
typeof(HandlersMap<TAggregate, TId>).GetMethod(nameof(TryGetInternal), BindingFlags.NonPublic | BindingFlags.Instance)!;
...
internal bool TryGetUntyped(Type command, [NotNullWhen(true)] out RegisteredHandler<TAggregate, TId>? handler) {
var parameters = new object?[] { null };
bool tryGet = (bool)TryGetInternalMethod.MakeGenericMethod(command).Invoke(this, parameters)!;
handler = tryGet ? (RegisteredHandler<TAggregate, TId>?)parameters[0] : null;
return tryGet;
Now the handler was found correctly.
As I looked further in the method I realized that there are further places to adapt:
Log.CommandHandlerNotFound<TCommand>();
...
Log.CommandHandled<TCommand>();
...
Log.ErrorHandlingCommand<TCommand>(e);
...
return new ErrorResult<TState>($"Error handling command {typeof(TCommand).Name}", e);
I introduced the corresponding *Untyped
methods in Eventuous.Diagnostics.ApplicationEventSource
.
[NonEvent]
public void CommandHandlerNotFound<TCommand>() => CommandHandlerNotFound(typeof(TCommand).Name);
[NonEvent]
public void CommandHandlerNotFoundUntyped(Type commandType) => CommandHandlerNotFound(commandType.Name);
[NonEvent]
public void ErrorHandlingCommand<TCommand>(Exception e) => ErrorHandlingCommand(typeof(TCommand).Name, e.ToString());
[NonEvent]
public void ErrorHandlingCommandUntyped(Type commandType, Exception e) => ErrorHandlingCommand(commandType.Name, e.ToString());
[NonEvent]
public void CommandHandled<TCommand>() {
if (IsEnabled(EventLevel.Verbose, EventKeywords.All)) CommandHandled(typeof(TCommand).Name);
}
[NonEvent]
public void CommandHandledUntyped(Type commandType) {
if (IsEnabled(EventLevel.Verbose, EventKeywords.All)) CommandHandled(commandType.Name);
}
If all this are acceptable changes, the FunctionalCommandService should be adapted as well. I can create a pull request if you approve my thoughts
I did not succeed to change the LocalMap
method such that the command type information gets passed to the Handle
method. Maybe you have an idea?
IMHO there should be a better solution instead of creating *Untyped
methods for every generic method with TCommand...
Basically, the issue has nothing to do with multiple aggregates, or any sort of edge case. Discovered commands mapping got broken when I changed the Command Service to use the generic type map instead of a dictionary. As commands discovery didn't have any tests, I overlooked the consequences as Handle<T>
must have the specific T
for the type map to work.
Right now, I fixed it by creating a generic call and caching it for consecutive invocations. Ideally, of course, it should be solved by creating a code generator, which would also avoid calling MapDiscoveredCommands(Assembly)
as it would just create code to map discovered commands.
IMHO there should be a better solution instead of creating *Untyped methods for every generic method with TCommand...
The code doesn't create untyped methods, the AddHandlerUntyped
itself has no generic argument, that's why it has such a name.
Your test passes now, should we consider the issue resolved?
Ok, it should work with the latest alpha package
Thank you for checking my thought and for finding the correct cause of the problem. I will check my app with the latest alpha package and let you know if it worked.
Ok, it should work with the latest alpha package
I'm a bit confused, does latest alpha package means 0.14.1-alpha.0.43
? I have currently installed 0.15.0-beta.5...
Sorry, I think the publish job has failed because the tests crashed in the pipeline. Checking now.
0.15.0-beta.4.19 is on MyGet
I am closing it as the tests you added are passing. Feel free to reopen it, or open a new one if something is still not working.
0.15.0-beta.4.19 is on MyGet
You mean 0.15.0-beta.6?
I released beta.6 right after. It's a tagged beta and it went to NuGet. Other preview packages are only on MyGet.
Describe the bug In a ASP NET Web Application,
app.MapDiscoveredCommands()
is not working as expected. All correctly registered commands are found but all are failing on execution with errorTo Reproduce See attached solution TestEventuous.zip with the following structure:
The solution contains Api, Application and Domain projects with commands for Car (
StartEngine
) and Train (OpenDoors
) and the corresponding CommandServices.Running the application displays swagger with two endpoint
car/start-engine
andtrain/open-doors
. Executing those endpoints yields the error described.When changing
app.MapDiscoveredCommands(typeof(ApplicationModule).Assembly);
toapp.MapDiscoveredCommands<Car>(typeof(ApplicationModule).Assembly);
then car command is working as expected (but not train, but that is wanted, as described in the docs)Expected behavior All discovered commands should reach their corresponding handler.
Screenshots