AxonFramework / AxonFramework

Framework for Evolutionary Message-Driven Microservices on the JVM
https://axoniq.io/
Apache License 2.0
3.3k stars 788 forks source link

AggregateLifecycle.createNew does not return Entity ID in CommandResultMessage #2059

Open jasperfect opened 2 years ago

jasperfect commented 2 years ago

Basic information

Steps to reproduce

// 1. Sending part:
reactorCommandGateway.send<FooId>(
            CreateFooCommand(
                id = FooId()
            )
)

// 2. Command Handler
class Bar(){
    @CommandHandler
    fun handle(cmd: CreateFooCommand) {
        createNew(Foo::class.java) { Foo(cmd) }
    }
}

Expected behaviour

After Foo's creation, return FooId in the CommandResultMessage.

Actual behaviour

An empty String is returned.

smcvb commented 2 years ago

This is a nice addition to the framework, @jasperfect. Note that this isn't part of Axon's implementation at the moment. Hence, marking it as a bug is incorrect; you should've gone for a Feature request.

smcvb commented 2 years ago

By the way, as a workaround right now, you can simply do a return in your @CommandHandler annotated method to return the aggregate identifier of the newly created aggregate.

jasperfect commented 2 years ago

Hey Steve, thanks for the tips, I'll do it now, but still expecting efforts from your side to make the response behavior consistent :-)

smcvb commented 2 years ago

As you can see, the prioritization on this is rather low on our end. Simply because doing it yourself is very straightforward.

However, as this is an open-source project, any contribution to implement this would be very helpful. Hence, if you have time to provide help here, that would be much appreciated. Added, it might increase the chances of this feature partaking in Axon Framework sooner.

jasperfect commented 2 years ago

Sure, I'll try my best to shift from Tester to Dev :-)

abuijze commented 2 years ago

I'd argue that the behavior is consistent the way it currently is. When invoking a command handler, the return value of that handler is used as the result. The only exception to this is when a command hander is the aggregate's constructor. Since a constructor returns the aggregate itself, which we don't want to expose, we return its identifier instead.

When using AggregateLifecycle.createNew in a command handler, it is just like any command handler: you need to return the value from your command handler that you consider the response of that command. What if your command handler would create 2 aggregate instances? Or only create them conditionally?

That said, I do agree that returning the identifier of the created aggregate in AggregateLifecycle.createNew() is useful. But the return value of the command handler (methods) should remain the responsibility of the application.

jasperfect commented 2 years ago

Hello @abuijze, thanks for the clarification, I got you. AggregateLifecycle.createNew() currently return the aggregate just like its name implies, I like it, but we shouldn't play with the returned aggregate directly (Bypassing CommandBus) right? So the returned aggregate is pretty much useless? From this perspective, maybe return an ID is safer? :-) Anyway, below is not ugly either @CommandHandler fun handle(cmd: CreateFooCommand) = createNew(Foo::class.java) { Foo(cmd) }.identifier()

smcvb commented 1 year ago

We do not envision having sufficient time to work on this before our intended release of 4.8.0. Added, we aim to direct our attention to the following major release. Due to this, it is unclear at this stage when the issue will be resolved.