From 074fb9a551b7560ff08c87e37b032b6e350c5afd Mon Sep 17 00:00:00 2001
From: Alexey Zimarev <alex@zimarev.com>
Date: Thu, 7 Apr 2022 15:14:18 +0200
Subject: [PATCH] Ensure that the new aggregate has an id. Proposed fix for #75

---
 src/Core/src/Eventuous/Aggregate.cs           |  2 +-
 src/Core/src/Eventuous/AggregateId.cs         | 10 +++---
 .../AppService/ApplicationService.cs          |  3 ++
 .../src/Eventuous/Exceptions/Exceptions.cs    | 12 ++++---
 .../test/Eventuous.Tests/ForgotToSetId.cs     | 36 +++++++++++++++++++
 5 files changed, 53 insertions(+), 10 deletions(-)
 create mode 100644 src/Core/test/Eventuous.Tests/ForgotToSetId.cs

diff --git a/src/Core/src/Eventuous/Aggregate.cs b/src/Core/src/Eventuous/Aggregate.cs
index abd45f92..8e53f230 100644
--- a/src/Core/src/Eventuous/Aggregate.cs
+++ b/src/Core/src/Eventuous/Aggregate.cs
@@ -114,5 +114,5 @@ public abstract class Aggregate<T, TId> : Aggregate<T>
     where T : AggregateState<T, TId>, new()
     where TId : AggregateId {
     /// <inheritdoc />
-    public override string GetId() => State.Id;
+    public override string GetId() => State.Id ?? throw new Exceptions.InvalidIdException<TId>();
 }
diff --git a/src/Core/src/Eventuous/AggregateId.cs b/src/Core/src/Eventuous/AggregateId.cs
index a8be22e5..5f89fc1f 100644
--- a/src/Core/src/Eventuous/AggregateId.cs
+++ b/src/Core/src/Eventuous/AggregateId.cs
@@ -1,10 +1,9 @@
-namespace Eventuous; 
+namespace Eventuous;
 
 [PublicAPI]
 public abstract record AggregateId {
     protected AggregateId(string value) {
-        if (string.IsNullOrWhiteSpace(value))
-            throw new Exceptions.InvalidIdException(this);
+        if (string.IsNullOrWhiteSpace(value)) throw new Exceptions.InvalidIdException(this);
 
         Value = value;
     }
@@ -13,7 +12,8 @@ protected AggregateId(string value) {
 
     public sealed override string ToString() => Value;
 
-    public static implicit operator string(AggregateId id) => id.ToString();
+    public static implicit operator string(AggregateId? id)
+        => id?.ToString() ?? throw new Exceptions.InvalidIdException(typeof(AggregateId));
 
     public void Deconstruct(out string value) => value = Value;
-}
\ No newline at end of file
+}
diff --git a/src/Core/src/Eventuous/AppService/ApplicationService.cs b/src/Core/src/Eventuous/AppService/ApplicationService.cs
index 069941e4..9b58806b 100644
--- a/src/Core/src/Eventuous/AppService/ApplicationService.cs
+++ b/src/Core/src/Eventuous/AppService/ApplicationService.cs
@@ -196,6 +196,9 @@ public async Task<Result<TState, TId>> Handle(object command, CancellationToken
                 .Handler(aggregate!, command, cancellationToken)
                 .NoContext();
 
+            // Zero in the global position would mean nothing, so the receiver need to check the Changes.Length
+            if (result.Changes.Count == 0) return new OkResult<TState, TId>(result.State, Array.Empty<Change>(), 0);
+            
             var storeResult = await Store.Store(
                     streamName != default ? streamName : GetAggregateStreamName(result),
                     result,
diff --git a/src/Core/src/Eventuous/Exceptions/Exceptions.cs b/src/Core/src/Eventuous/Exceptions/Exceptions.cs
index 5161d93e..a34d7ad1 100644
--- a/src/Core/src/Eventuous/Exceptions/Exceptions.cs
+++ b/src/Core/src/Eventuous/Exceptions/Exceptions.cs
@@ -2,9 +2,13 @@ namespace Eventuous;
 
 public static class Exceptions {
     public class InvalidIdException : Exception {
-        public InvalidIdException(AggregateId id) : base(
-            $"Aggregate id {id.GetType().Name} cannot have an empty value"
-        ) { }
+        public InvalidIdException(AggregateId id) : this(id.GetType()) { }
+
+        public InvalidIdException(Type idType) : base($"Aggregate id {idType.Name} cannot have an empty value") { }
+    }
+
+    public class InvalidIdException<T> : InvalidIdException where T : AggregateId {
+        public InvalidIdException() : base(typeof(T)) { }
     }
 
     public class CommandHandlerNotFound : Exception {
@@ -26,4 +30,4 @@ public CommandHandlerAlreadyRegistered() : base(
             $"Command handler for ${typeof(T).Name} already registered"
         ) { }
     }
-}
\ No newline at end of file
+}
diff --git a/src/Core/test/Eventuous.Tests/ForgotToSetId.cs b/src/Core/test/Eventuous.Tests/ForgotToSetId.cs
new file mode 100644
index 00000000..110bacd0
--- /dev/null
+++ b/src/Core/test/Eventuous.Tests/ForgotToSetId.cs
@@ -0,0 +1,36 @@
+using Eventuous.Tests.Fixtures;
+
+namespace Eventuous.Tests;
+
+public class ForgotToSetId : NaiveFixture {
+    public ForgotToSetId() => Service = new TestService(this.AggregateStore);
+
+    [Fact]
+    public async Task ShouldFailWithNoId() {
+        var cmd     = new DoIt(Auto.Create<string>());
+        var result  = await Service.Handle(cmd, default);
+        result.Success.Should().BeFalse();
+        (result as ErrorResult<TestState, TestId>)!.Exception.Should().BeOfType<Exceptions.InvalidIdException>();
+    }
+
+    TestService Service { get; }
+
+    class TestService : ApplicationService<TestAggregate, TestState, TestId> {
+        public TestService(IAggregateStore store) : base(store)
+            => OnNew<DoIt>((test, cmd) => test.DoIt(new TestId(cmd.Id)));
+    }
+
+    record DoIt(string Id);
+
+    class TestAggregate : Aggregate<TestState, TestId> {
+        public void DoIt(TestId id) => Apply(new TestEvent(id));
+    }
+
+    record TestState : AggregateState<TestState, TestId>;
+
+    record TestId : AggregateId {
+        public TestId(string value) : base(value) { }
+    }
+
+    record TestEvent(string Id);
+}