From 58c18c3fd2b5a208cea116e7129277c47789e4a8 Mon Sep 17 00:00:00 2001 From: Whit Waldo Date: Thu, 24 Sep 2026 06:36:13 -0500 Subject: [PATCH 1/2] Setting a value for a key always sets it as an update (effectively an upsert) in the actor state. This means that when a delete happens, the delete is always sent to the runtime even if the key has never existed there (e.g. added to state and then removed before the actor turn completes). In all other respects, caching operates the same but without the check to see if the key already exists in the runtime when setting the value. Signed-off-by: Whit Waldo --- src/Dapr.Actors/Runtime/ActorStateManager.cs | 18 ++-- .../Dapr.Actors.Test/ActorStateManagerTest.cs | 83 +++++++++++++++++++ .../Dapr.Actors.Test/DaprStateProviderTest.cs | 32 +++++++ 3 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/Dapr.Actors/Runtime/ActorStateManager.cs b/src/Dapr.Actors/Runtime/ActorStateManager.cs index 032918150..6ab877b41 100644 --- a/src/Dapr.Actors/Runtime/ActorStateManager.cs +++ b/src/Dapr.Actors/Runtime/ActorStateManager.cs @@ -205,14 +205,13 @@ public async Task SetStateAsync(string stateName, T value, CancellationToken stateMetadata.ChangeKind = StateChangeKind.Update; } } - else if (await this.actor.Host.StateProvider.ContainsStateAsync(this.actorTypeName, this.actor.Id.ToString(), stateName, cancellationToken)) - { - stateChangeTracker.Add(stateName, StateMetadata.Create(value, StateChangeKind.Update)); - } else { - stateChangeTracker[stateName] = StateMetadata.Create(value, StateChangeKind.Add); + // Add and Update are both upserts, while Update ensures a later remove stages a delete. + stateChangeTracker[stateName] = StateMetadata.Create(value, StateChangeKind.Update); } + + await Task.CompletedTask; } public async Task SetStateAsync(string stateName, T value, TimeSpan ttl, CancellationToken cancellationToken) @@ -235,14 +234,13 @@ public async Task SetStateAsync(string stateName, T value, TimeSpan ttl, Canc stateMetadata.ChangeKind = StateChangeKind.Update; } } - else if (await this.actor.Host.StateProvider.ContainsStateAsync(this.actorTypeName, this.actor.Id.ToString(), stateName, cancellationToken)) - { - stateChangeTracker.Add(stateName, StateMetadata.Create(value, StateChangeKind.Update, ttl: ttl)); - } else { - stateChangeTracker[stateName] = StateMetadata.Create(value, StateChangeKind.Add, ttl: ttl); + // Add and Update are both upserts, while Update ensures a later remove stages a delete. + stateChangeTracker[stateName] = StateMetadata.Create(value, StateChangeKind.Update, ttl: ttl); } + + await Task.CompletedTask; } public async Task RemoveStateAsync(string stateName, CancellationToken cancellationToken) diff --git a/test/Dapr.Actors.Test/ActorStateManagerTest.cs b/test/Dapr.Actors.Test/ActorStateManagerTest.cs index 60e376c3d..a9e03d176 100644 --- a/test/Dapr.Actors.Test/ActorStateManagerTest.cs +++ b/test/Dapr.Actors.Test/ActorStateManagerTest.cs @@ -740,6 +740,89 @@ public async Task SetStateAsync_WithTTL_UpdatesTTLOnCachedNoneEntry() Assert.Contains("ttlInSeconds", capturedData); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task SetStateAsync_UnknownKeyDoesNotReadFromStateStore(bool useTtl) + { + var interactor = new Mock(); + var host = ActorHost.CreateForTest(); + host.StateProvider = new DaprStateProvider(interactor.Object, new JsonSerializerOptions()); + var mngr = new ActorStateManager(new TestActor(host)); + var token = CancellationToken.None; + string capturedData = null; + + interactor + .Setup(d => d.SaveStateTransactionallyAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((_, _, data, _) => capturedData = data) + .Returns(Task.CompletedTask); + + if (useTtl) + { + await mngr.SetStateAsync("k1", "value", TimeSpan.FromMinutes(5), token); + } + else + { + await mngr.SetStateAsync("k1", "value", token); + } + + Assert.Equal("value", await mngr.GetStateAsync("k1", token)); + Assert.True(await mngr.ContainsStateAsync("k1", token)); + Assert.False(await mngr.TryAddStateAsync("k1", "replacement", token)); + + await mngr.SaveStateAsync(token); + + Assert.Contains("\"operation\":\"upsert\"", capturedData); + Assert.Contains("\"value\":\"value\"", capturedData); + Assert.Equal("value", await mngr.GetStateAsync("k1", token)); + Assert.True(await mngr.ContainsStateAsync("k1", token)); + + await mngr.SaveStateAsync(token); + + interactor.Verify( + d => d.SaveStateTransactionallyAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + interactor.Verify( + d => d.GetStateAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task SetThenRemoveUnknownKeyStagesDeleteWithoutReadingStateStore(bool useTtl) + { + var interactor = new Mock(); + var host = ActorHost.CreateForTest(); + host.StateProvider = new DaprStateProvider(interactor.Object, new JsonSerializerOptions()); + var mngr = new ActorStateManager(new TestActor(host)); + var token = CancellationToken.None; + string capturedData = null; + + interactor + .Setup(d => d.SaveStateTransactionallyAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((_, _, data, _) => capturedData = data) + .Returns(Task.CompletedTask); + + if (useTtl) + { + await mngr.SetStateAsync("k1", "value", TimeSpan.FromMinutes(5), token); + } + else + { + await mngr.SetStateAsync("k1", "value", token); + } + + Assert.True(await mngr.TryRemoveStateAsync("k1", token)); + await mngr.SaveStateAsync(token); + + Assert.Contains("\"operation\":\"delete\"", capturedData); + Assert.DoesNotContain("\"operation\":\"upsert\"", capturedData); + interactor.Verify( + d => d.GetStateAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + // ----- SetStateContext (reentrancy) ----- [Fact] diff --git a/test/Dapr.Actors.Test/DaprStateProviderTest.cs b/test/Dapr.Actors.Test/DaprStateProviderTest.cs index 370f284c4..1ddba72db 100644 --- a/test/Dapr.Actors.Test/DaprStateProviderTest.cs +++ b/test/Dapr.Actors.Test/DaprStateProviderTest.cs @@ -181,6 +181,38 @@ public async Task SaveStateAsync_Update_EmitsUpsertOperation() capturedContent); } + [Fact] + public async Task SaveStateAsync_AddAndUpdateEmitEquivalentUpserts() + { + var interactor = new Mock(); + var provider = new DaprStateProvider(interactor.Object, new JsonSerializerOptions()); + var token = CancellationToken.None; + var capturedContents = new List(); + + interactor + .Setup(d => d.SaveStateTransactionallyAsync( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((_, _, data, _) => capturedContents.Add(data)) + .Returns(Task.CompletedTask); + + await provider.SaveStateAsync( + "actorType", + "actorId", + new[] { new ActorStateChange("key1", typeof(string), "value", StateChangeKind.Add, null) }, + token); + await provider.SaveStateAsync( + "actorType", + "actorId", + new[] { new ActorStateChange("key1", typeof(string), "value", StateChangeKind.Update, null) }, + token); + + Assert.Equal(2, capturedContents.Count); + Assert.Equal(capturedContents[0], capturedContents[1]); + Assert.Equal( + "[{\"operation\":\"upsert\",\"request\":{\"key\":\"key1\",\"value\":\"value\"}}]", + capturedContents[0]); + } + [Fact] public async Task TryLoadStateAsync_ReturnsFalseWhenTTLExpireTimeIsExactlyNow() { From 50f9ed5cb28b7160961b97043777f89470cb4402 Mon Sep 17 00:00:00 2001 From: Whit Waldo Date: Thu, 24 Sep 2026 09:03:39 -0500 Subject: [PATCH 2/2] Corrected miscount since upsert doesn't do an existence check first Signed-off-by: Whit Waldo --- test/Dapr.Actors.Test/ActorStateManagerTest.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/Dapr.Actors.Test/ActorStateManagerTest.cs b/test/Dapr.Actors.Test/ActorStateManagerTest.cs index fddacd647..2c8a471a7 100644 --- a/test/Dapr.Actors.Test/ActorStateManagerTest.cs +++ b/test/Dapr.Actors.Test/ActorStateManagerTest.cs @@ -180,10 +180,8 @@ public async Task ReentrantSaveRefreshesDefaultTrackerWithoutRoundTrip() Assert.Equal("value1", await mngr.GetStateAsync("key1", token)); // A reentrancy-scoped call writes the same key through its own (fresh, empty) - // tracker. SetStateAsync's own ContainsStateAsync existence check against the - // runtime, to decide Add vs Update, is call #2 - unrelated to defaultTracker and - // unavoidable, since the reentrant tracker starts empty and has no local record of - // the key yet. + // tracker. SetStateAsync stages an upsert without checking the runtime, so this + // does not add another GetStateAsync call. await mngr.SetStateContext("ctx1"); await mngr.SetStateAsync("key1", "value2", token); await mngr.SaveStateAsync(token); @@ -191,11 +189,11 @@ public async Task ReentrantSaveRefreshesDefaultTrackerWithoutRoundTrip() // The default tracker must reflect the new value without a further call to the // runtime - it already has the value the save above just confirmed was persisted. - // Total call count stays at 2; a naive reload-on-evict fix would make this 3. + // Total call count stays at 1; a naive reload-on-evict fix would make this 2. Assert.Equal("value2", await mngr.GetStateAsync("key1", token)); interactor.Verify( d => d.GetStateAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), - Times.Exactly(2)); + Times.Once); } [Fact]