Repository: commanded/commanded (elixir)

Issue:
Support :ok tagged tuple events from aggregate
Allow an aggregate to return a tagged success tuple - `{:ok, events}` - containing the resultant events from a command function. 



Pull request description:
Support `:ok` tagged tuple events from aggregate
Add support to allow the following values to be returned by a command handler function, such as aggregate `execute/2` functions.


| Aggregate return value | Events produced |
|-------------|------------------|
| `:ok`, `nil` , `[]` | none |
| `event` , `{:ok, event}` | one event |
| `[event1, event2, ..]` , `{:ok, [event1, event2, ..]}` | list of events |
| `{:error, reason`} | an error occurred |


Closes #52.

Patch:
diff --git a/lib/commanded/aggregates/aggregate.ex b/lib/commanded/aggregates/aggregate.ex
index 05dee02..f9ecd5e 100644
--- a/lib/commanded/aggregates/aggregate.ex
+++ b/lib/commanded/aggregates/aggregate.ex
@@ -407,11 +407,7 @@ defmodule Commanded.Aggregates.Aggregate do
       retry_attempts: retry_attempts
     } = context
 
-    %Aggregate{
-      aggregate_module: aggregate_module,
-      aggregate_version: expected_version,
-      aggregate_state: aggregate_state
-    } = state
+    %Aggregate{aggregate_version: expected_version, aggregate_state: aggregate_state} = state
 
     Logger.debug(fn -> describe(state) <> " executing command: #{inspect(command)}" end)
 
@@ -420,7 +416,7 @@ defmodule Commanded.Aggregates.Aggregate do
         {:error, _error} = reply ->
           {reply, state}
 
-        none when none in [nil, []] ->
+        none when none in [:ok, nil, []] ->
           {{:ok, expected_version, []}, state}
 
         %Commanded.Aggregate.Multi{} = multi ->
@@ -432,11 +428,11 @@ defmodule Commanded.Aggregates.Aggregate do
               persist_events(pending_events, aggregate_state, context, state)
           end
 
-        events ->
-          pending_events = List.wrap(events)
-          aggregate_state = apply_events(aggregate_module, aggregate_state, pending_events)
+        {:ok, pending_events} ->
+          record_events(pending_events, context, state)
 
-          persist_events(pending_events, aggregate_state, context, state)
+        pending_events ->
+          record_events(pending_events, context, state)
       end
 
     case reply do
@@ -454,6 +450,15 @@ defmodule Commanded.Aggregates.Aggregate do
     end
   end
 
+  defp record_events(pending_events, context, %Aggregate{} = state) do
+    %Aggregate{aggregate_module: aggregate_module, aggregate_state: aggregate_state} = state
+
+    pending_events = List.wrap(pending_events)
+    aggregate_state = apply_events(aggregate_module, aggregate_state, pending_events)
+
+    persist_events(pending_events, aggregate_state, context, state)
+  end
+
   defp apply_events(aggregate_module, aggregate_state, events) do
     Enum.reduce(events, aggregate_state, &aggregate_module.apply(&2, &1))
   end


Test patch:
diff --git a/test/aggregates/execute_command_test.exs b/test/aggregates/execute_command_test.exs
index 99c31ed..28d19cb 100644
--- a/test/aggregates/execute_command_test.exs
+++ b/test/aggregates/execute_command_test.exs
@@ -5,13 +5,13 @@ defmodule Commanded.Aggregates.ExecuteCommandTest do
   alias Commanded.ExampleDomain.{BankAccount, OpenAccountHandler}
   alias Commanded.ExampleDomain.BankAccount.Commands.OpenAccount
   alias Commanded.ExampleDomain.BankAccount.Events.BankAccountOpened
-  alias Commanded.Helpers.{ProcessHelper,Wait}
+  alias Commanded.Helpers.{ProcessHelper, Wait}
   alias Commanded.Registration
 
   test "execute command against an aggregate" do
-    account_number = UUID.uuid4
+    account_number = UUID.uuid4()
 
-    {:ok, ^account_number} = Commanded.Aggregates.Supervisor.open_aggregate(BankAccount, account_number)
+    {:ok, ^account_number} = open_aggregate(BankAccount, account_number)
 
     command = %OpenAccount{account_number: account_number, initial_balance: 1_000}
     context = %ExecutionContext{command: command, handler: BankAccount, function: :open_account}
@@ -23,16 +23,21 @@ defmodule Commanded.Aggregates.ExecuteCommandTest do
     ProcessHelper.shutdown_aggregate(BankAccount, account_number)
 
     # reload aggregate to fetch persisted events from event store and rebuild state by applying saved events
-    {:ok, ^account_number} = Commanded.Aggregates.Supervisor.open_aggregate(BankAccount, account_number)
+    {:ok, ^account_number} = open_aggregate(BankAccount, account_number)
 
     assert Aggregate.aggregate_version(BankAccount, account_number) == 1
-    assert Aggregate.aggregate_state(BankAccount, account_number) == %BankAccount{account_number: account_number, balance: 1_000, state: :active}
+
+    assert Aggregate.aggregate_state(BankAccount, account_number) == %BankAccount{
+             account_number: account_number,
+             balance: 1_000,
+             state: :active
+           }
   end
 
   test "execute command via a command handler" do
-    account_number = UUID.uuid4
+    account_number = UUID.uuid4()
 
-    {:ok, ^account_number} = Commanded.Aggregates.Supervisor.open_aggregate(BankAccount, account_number)
+    {:ok, ^account_number} = open_aggregate(BankAccount, account_number)
 
     command = %OpenAccount{account_number: account_number, initial_balance: 1_000}
     context = %ExecutionContext{command: command, handler: OpenAccountHandler, function: :handle}
@@ -44,16 +49,21 @@ defmodule Commanded.Aggregates.ExecuteCommandTest do
     ProcessHelper.shutdown_aggregate(BankAccount, account_number)
 
     # reload aggregate to fetch persisted events from event store and rebuild state by applying saved events
-    {:ok, ^account_number} = Commanded.Aggregates.Supervisor.open_aggregate(BankAccount, account_number)
+    {:ok, ^account_number} = open_aggregate(BankAccount, account_number)
 
     assert Aggregate.aggregate_version(BankAccount, account_number) == 1
-    assert Aggregate.aggregate_state(BankAccount, account_number) == %BankAccount{account_number: account_number, balance: 1_000, state: :active}
+
+    assert Aggregate.aggregate_state(BankAccount, account_number) == %BankAccount{
+             account_number: account_number,
+             balance: 1_000,
+             state: :active
+           }
   end
 
   test "aggregate raising an exception should not persist pending events or state" do
-    account_number = UUID.uuid4
+    account_number = UUID.uuid4()
 
-    {:ok, ^account_number} = Commanded.Aggregates.Supervisor.open_aggregate(BankAccount, account_number)
+    {:ok, ^account_number} = open_aggregate(BankAccount, account_number)
 
     command = %OpenAccount{account_number: account_number, initial_balance: 1_000}
     context = %ExecutionContext{command: command, handler: OpenAccountHandler, function: :handle}
@@ -64,16 +74,84 @@ defmodule Commanded.Aggregates.ExecuteCommandTest do
 
     assert_aggregate_exit(BankAccount, account_number, fn ->
       command = %OpenAccount{account_number: account_number, initial_balance: 1}
-      context = %ExecutionContext{command: command, handler: OpenAccountHandler, function: :handle}
+
+      context = %ExecutionContext{
+        command: command,
+        handler: OpenAccountHandler,
+        function: :handle
+      }
 
       Aggregate.execute(BankAccount, account_number, context)
     end)
 
-    {:ok, ^account_number} = Commanded.Aggregates.Supervisor.open_aggregate(BankAccount, account_number)
+    {:ok, ^account_number} = open_aggregate(BankAccount, account_number)
+
     assert state_before == Aggregate.aggregate_state(BankAccount, account_number)
   end
 
-  def assert_aggregate_exit(aggregate_module, aggregate_uuid, fun) do
+  describe "command dispatch return" do
+    alias Commanded.Aggregates.ReturnValue.Command
+    alias Commanded.Aggregates.ReturnValue.Event
+
+    test "should allow `:ok` return value" do
+      assert_no_events(fn %Command{} -> :ok end)
+    end
+
+    test "should allow `nil` return value" do
+      assert_no_events(fn %Command{} -> nil end)
+    end
+
+    test "should allow `[]` return value" do
+      assert_no_events(fn %Command{} -> [] end)
+    end
+
+    test "should allow single events return value" do
+      assert_event_result(fn %Command{id: id} -> %Event{id: id} end)
+    end
+
+    test "should allow event list return value" do
+      assert_event_result(fn %Command{id: id} -> [%Event{id: id}] end)
+    end
+
+    test "should allow `{:ok, event}` tagged tuple return value" do
+      assert_event_result(fn %Command{id: id} -> {:ok, %Event{id: id}} end)
+    end
+
+    test "should allow `{:ok, event}` return value" do
+      assert_event_result(fn %Command{id: id} -> {:ok, [%Event{id: id}]} end)
+    end
+  end
+
+  defp assert_no_events(command_fun) do
+    id = UUID.uuid4()
+
+    assert {:ok, 0, []} = execute_aggregate_command(id, command_fun)
+  end
+
+  defp assert_event_result(command_fun) do
+    alias Commanded.Aggregates.ReturnValue.Event
+
+    id = UUID.uuid4()
+
+    assert {:ok, 1, [%Event{id: ^id}]} = execute_aggregate_command(id, command_fun)
+  end
+
+  defp execute_aggregate_command(id, command_fun) do
+    alias Commanded.Aggregates.ReturnValue.Command
+    alias Commanded.Aggregates.ReturnValue.ExampleAggregate
+
+    {:ok, ^id} = open_aggregate(ExampleAggregate, id)
+
+    context = %ExecutionContext{
+      command: %Command{id: id, fun: command_fun},
+      handler: ExampleAggregate,
+      function: :execute
+    }
+
+    Aggregate.execute(ExampleAggregate, id, context)
+  end
+
+  defp assert_aggregate_exit(aggregate_module, aggregate_uuid, fun) do
     pid = spawn(fun)
 
     # wait for spawned function to terminate
@@ -85,4 +163,8 @@ defmodule Commanded.Aggregates.ExecuteCommandTest do
       assert Registration.whereis_name({aggregate_module, aggregate_uuid}) == :undefined
     end)
   end
+
+  defp open_aggregate(module, id) do
+    Commanded.Aggregates.Supervisor.open_aggregate(module, id)
+  end
 end
diff --git a/test/aggregates/support/return_value_aggregate.ex b/test/aggregates/support/return_value_aggregate.ex
new file mode 100644
index 0000000..c732339
--- /dev/null
+++ b/test/aggregates/support/return_value_aggregate.ex
@@ -0,0 +1,22 @@
+defmodule Commanded.Aggregates.ReturnValue do
+  defmodule Command do
+    defstruct [:id, :fun]
+  end
+
+  defmodule Event do
+    @derive Jason.Encoder
+    defstruct [:id]
+  end
+
+  defmodule ExampleAggregate do
+    defstruct [:id]
+
+    def execute(%ExampleAggregate{}, %Command{fun: fun} = command) when is_function(fun, 1) do
+      %Command{fun: fun} = command
+
+      Kernel.apply(fun, [command])
+    end
+
+    def apply(%ExampleAggregate{} = aggregate, _event), do: aggregate
+  end
+end