Comments (16)
Your test passes now, should we consider the issue resolved?
from eventuous.
0.15.0-beta.4.19 is on MyGet
from eventuous.
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.
from eventuous.
Hm, it should not be mapping object
, it must have a proper type. I will try to reproduce it.
from eventuous.
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.
from eventuous.
Maybe we can start by adding a test with this scenario, so the issue would be clearly illustrated.
from eventuous.
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.
Testing
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']
Possible solution 1
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
Possible solution 2 (?)
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?
from eventuous.
IMHO there should be a better solution instead of creating *Untyped
methods for every generic method with TCommand...
from eventuous.
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.
from eventuous.
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.
from eventuous.
Ok, it should work with the latest alpha package
from eventuous.
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.
from eventuous.
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...
from eventuous.
Sorry, I think the publish job has failed because the tests crashed in the pipeline. Checking now.
from eventuous.
0.15.0-beta.4.19 is on MyGet
You mean 0.15.0-beta.6?
from eventuous.
I released beta.6 right after. It's a tagged beta and it went to NuGet. Other preview packages are only on MyGet.
from eventuous.
Related Issues (20)
- Ability to return a custom return type for a command API reponse HOT 5
- SQL Server __schema__.Streams.StreamName is too big to be indexed
- Unable to create isolated Aggregates with State and Id HOT 5
- SQL Server Subscription skips an event on non-initial startup
- Provide StreamContext in UpdateBuilder<TBuilder>.Configure() for MondoDb HOT 1
- With the SQL Server event store, event handlers are sometimes called twice for each event HOT 4
- Health check stays "healthy" even if Event Handler throws an exception HOT 5
- Transient error drops the subscription. (Connection reset by peer) HOT 6
- Add subscription drop/resubscribe tests HOT 1
- Add subscription measure for RDBMS subs HOT 1
- Error at schema initialization (PostgresException: '42704: type public.stream_message[] does not exist) HOT 1
- Eventuous should always return `Result<TState>` HOT 2
- Does it make sense to keep `Aggregate` without state? HOT 9
- Preserve original exception stacktrace in command service HOT 1
- Enable custom configuration of NpgsqlDataSourceBuilder when using AddEventuousPostgres HOT 1
- DuplicateTypeException in CommandService HOT 11
- "Can not start an Activity that was already started" in BaseProducer
- Memory leak (CancellationTokenSource) in AsyncHandlingFilter HOT 1
- Use source generator for type mapping HOT 1
- Use source generator for mapping HTTP API to command handlers HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from eventuous.