Fix: Actor reminders should return null if not registered

Added more checks to determine if a reminder was returned or not having discovered that the runtime returns a 200 even if there's no reminder registered. (#1476)

Author: WhitWaldo

src/Dapr.Actors/Runtime/DefaultActorTimerManager.cs

@@ -90,24 +90,26 @@ public override async Task UnregisterTimerAsync(ActorTimerToken timer)
             await this.interactor.UnregisterTimerAsync(timer.ActorType, timer.ActorId.ToString(), timer.Name);
         }
 
-        private async ValueTask<string> SerializeReminderAsync(ActorReminder reminder)
+        private static async ValueTask<string> SerializeReminderAsync(ActorReminder reminder)
         {
             var info = new ReminderInfo(reminder.State, reminder.DueTime, reminder.Period, reminder.Repetitions, 
                 reminder.Ttl);
             return await info.SerializeAsync();
         }
 
-        private async ValueTask<ActorReminder> DeserializeReminderAsync(Stream stream, ActorReminderToken token)
+        private static async ValueTask<ActorReminder> DeserializeReminderAsync(Stream stream, ActorReminderToken token)
         {
             if (stream == null)
             {
                 throw new ArgumentNullException(nameof(stream));
             }
+            
             var info = await ReminderInfo.DeserializeAsync(stream);
-            if(info == null)
+            if (info == null)
             {
                 return null;
             }
+            
             var reminder = new ActorReminder(token.ActorType, token.ActorId, token.Name, info.Data, info.DueTime, 
                 info.Period);
             return reminder;

src/Dapr.Actors/Runtime/ReminderInfo.cs

@@ -11,99 +11,109 @@
 // limitations under the License.
 // ------------------------------------------------------------------------
 
-namespace Dapr.Actors.Runtime
+#nullable enable
+namespace Dapr.Actors.Runtime;
+
+using System;
+using System.IO;
+using System.Text;
+using System.Text.Json;
+using System.Threading.Tasks;
+
+// represents the wire format used by Dapr to store reminder info with the runtime
+internal class ReminderInfo
 {
-    using System;
-    using System.IO;
-    using System.Text;
-    using System.Text.Json;
-    using System.Threading.Tasks;
-
-    // represents the wire format used by Dapr to store reminder info with the runtime
-    internal class ReminderInfo
+    public ReminderInfo(
+        byte[] data,
+        TimeSpan dueTime,
+        TimeSpan period,
+        int? repetitions = null,
+        TimeSpan? ttl = null)
     {
-        public ReminderInfo(
-            byte[] data,
-            TimeSpan dueTime,
-            TimeSpan period,
-            int? repetitions = null,
-            TimeSpan? ttl = null)
+        this.Data = data;
+        this.DueTime = dueTime;
+        this.Period = period;
+        this.Ttl = ttl;
+        this.Repetitions = repetitions;
+    }
+
+    public TimeSpan DueTime { get; private set; }
+
+    public TimeSpan Period { get; private set; }
+
+    public byte[] Data { get; private set; }
+
+    public TimeSpan? Ttl { get; private set; }
+        
+    public int? Repetitions { get; private set; }
+
+    internal static async Task<ReminderInfo?> DeserializeAsync(Stream stream)
+    {
+        var json = await JsonSerializer.DeserializeAsync<JsonElement>(stream);
+        if(json.ValueKind == JsonValueKind.Null)
         {
-            this.Data = data;
-            this.DueTime = dueTime;
-            this.Period = period;
-            this.Ttl = ttl;
-            this.Repetitions = repetitions;
+            return null;
         }
 
-        public TimeSpan DueTime { get; private set; }
+        var setAnyProperties = false; //Used to determine if anything was actually deserialized
+        var dueTime = TimeSpan.Zero;
+        var period = TimeSpan.Zero;
+        var data = Array.Empty<byte>();
+        int? repetition = null;
+        TimeSpan? ttl = null;
+        if (json.TryGetProperty("dueTime", out var dueTimeProperty))
+        {
+            setAnyProperties = true;
+            var dueTimeString = dueTimeProperty.GetString();
+            dueTime = ConverterUtils.ConvertTimeSpanFromDaprFormat(dueTimeString);
+        }
 
-        public TimeSpan Period { get; private set; }
+        if (json.TryGetProperty("period", out var periodProperty))
+        {
+            setAnyProperties = true;
+            var periodString = periodProperty.GetString();
+            (period, repetition) = ConverterUtils.ConvertTimeSpanValueFromISO8601Format(periodString);
+        }
 
-        public byte[] Data { get; private set; }
+        if (json.TryGetProperty("data", out var dataProperty) && dataProperty.ValueKind != JsonValueKind.Null)
+        {
+            setAnyProperties = true;
+            data = dataProperty.GetBytesFromBase64();
+        }
 
-        public TimeSpan? Ttl { get; private set; }
-        
-        public int? Repetitions { get; private set; }
+        if (json.TryGetProperty("ttl", out var ttlProperty))
+        {
+            setAnyProperties = true;
+            var ttlString = ttlProperty.GetString();
+            ttl = ConverterUtils.ConvertTimeSpanFromDaprFormat(ttlString);
+        }
 
-        internal static async Task<ReminderInfo> DeserializeAsync(Stream stream)
+        if (!setAnyProperties)
         {
-            var json = await JsonSerializer.DeserializeAsync<JsonElement>(stream);
-            if(json.ValueKind == JsonValueKind.Null)
-            {
-                return null;
-            }
-
-            var dueTime = default(TimeSpan);
-            var period = default(TimeSpan);
-            var data = default(byte[]);
-            int? repetition = null;
-            TimeSpan? ttl = null;
-            if (json.TryGetProperty("dueTime", out var dueTimeProperty))
-            {
-                var dueTimeString = dueTimeProperty.GetString();
-                dueTime = ConverterUtils.ConvertTimeSpanFromDaprFormat(dueTimeString);
-            }
-
-            if (json.TryGetProperty("period", out var periodProperty))
-            {
-                var periodString = periodProperty.GetString();
-                (period, repetition) = ConverterUtils.ConvertTimeSpanValueFromISO8601Format(periodString);
-            }
-
-            if (json.TryGetProperty("data", out var dataProperty) && dataProperty.ValueKind != JsonValueKind.Null)
-            {
-                data = dataProperty.GetBytesFromBase64();
-            }
-
-            if (json.TryGetProperty("ttl", out var ttlProperty))
-            {
-                var ttlString = ttlProperty.GetString();
-                ttl = ConverterUtils.ConvertTimeSpanFromDaprFormat(ttlString);
-            }
-
-            return new ReminderInfo(data, dueTime, period, repetition, ttl);
+            return null; //No properties were ever deserialized, so return null instead of default values
         }
 
-        internal async ValueTask<string> SerializeAsync()
+        return new ReminderInfo(data, dueTime, period, repetition, ttl);
+    }
+
+    internal async ValueTask<string> SerializeAsync()
+    {
+        using var stream = new MemoryStream();
+        await using Utf8JsonWriter writer = new Utf8JsonWriter(stream);
+
+        writer.WriteStartObject();
+        writer.WriteString("dueTime", ConverterUtils.ConvertTimeSpanValueInDaprFormat(this.DueTime));
+        writer.WriteString("period", ConverterUtils.ConvertTimeSpanValueInISO8601Format(
+            this.Period, this.Repetitions));
+        writer.WriteBase64String("data", this.Data);
+
+        if (Ttl != null)
         {
-            using var stream = new MemoryStream();
-            using Utf8JsonWriter writer = new Utf8JsonWriter(stream);
-
-            writer.WriteStartObject();
-            writer.WriteString("dueTime", ConverterUtils.ConvertTimeSpanValueInDaprFormat(this.DueTime));
-            writer.WriteString("period", ConverterUtils.ConvertTimeSpanValueInISO8601Format(
-                this.Period, this.Repetitions));
-            writer.WriteBase64String("data", this.Data);
-
-            if (Ttl != null)
-            {
-                writer.WriteString("ttl", ConverterUtils.ConvertTimeSpanValueInDaprFormat(Ttl));
-            }
-
-            writer.WriteEndObject();
-            await writer.FlushAsync();
-            return Encoding.UTF8.GetString(stream.ToArray());
+            writer.WriteString("ttl", ConverterUtils.ConvertTimeSpanValueInDaprFormat(Ttl));
         }
+
+        writer.WriteEndObject();
+        await writer.FlushAsync();
+        return Encoding.UTF8.GetString(stream.ToArray());
     }
 }

test/Dapr.Actors.Test/Runtime/DefaultActorTimerManagerTests.cs

@@ -109,6 +109,25 @@ public async Task GetReminderAsync_ReturnsNullWhenUnavailable()
             var reminderResult = await defaultActorTimerManager.GetReminderAsync(new ActorReminderToken(actorType, new ActorId(actorId), reminderName));
             Assert.Null(reminderResult);
         }
+        
+        [Fact]
+        public async Task GetReminderAsync_ReturnsNullWhenDeserialziationFails()
+        {
+            const string actorId = "123";
+            const string actorType = "abc";
+            const string reminderName = "reminderName";
+            var interactor = new Mock<TestDaprInteractor>();
+            var response = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("{}") };
+            
+            interactor
+                .Setup(d => d.GetReminderAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(),
+                    It.IsAny<CancellationToken>()))
+                .ReturnsAsync(response);
+            var defaultActorTimerManager = new DefaultActorTimerManager(interactor.Object);
+
+            var reminderResult = await defaultActorTimerManager.GetReminderAsync(new ActorReminderToken(actorType, new ActorId(actorId), reminderName));
+            Assert.Null(reminderResult);
+        }
 
         [Fact]
         public async Task GetReminderAsync_ReturnsResultWhenAvailable()