dotnet-sdk
dotnet-sdk copied to clipboard
dapr
Proposal: Add TryGetStateAsync method for non-actor state management operations
Describe the proposal
When accessing a state store from a non-actor, there exists a GetStateAsync<T> method. Unfortunately, if the key doesn't exist, this returns a default(T), which, depending on the type of T doesn't necessarily tell you much about whether the state actually exists or not.
When accessing state from an Actor, there exists, a TryGetStateAsync<T> which addresses this very problem since it returns a ConditionalValue<T>, indicating whether the key exists or not.
I would like to propose that TryGetStateAsync<T> be added to the non-actor state store API. It should return a ConditionalValue<T> like that of the actor API. Most importantly, it fills a gap in the API right now in that there appears to be no definite way to confirm that a given key otherwise exists or not.
I'd be happy to contribute a PR, if this is an acceptable addition.
Proposed breaking change
Today, ConditionalValue exists only in Dapr.Actors. To avoid having two identical types, this should be refactored out to a more common project like Dapr.Client.
Should be it - this looks to just be an issue in how the .NET API works and I don't see that it requires any changes to the sidecar itself.
While I'm personally not fond of ConditionalValue<T> in the first place--I'd rather rely on overloads of GetStateAsync<T>() that use Nullable<T> for non-reference types and nullable contexts for reference types--we are where we are. It seems reasonable to me to have a TryGetStateAsync<T>() method on DaprClient that matches what currently exists on IActorStateManager.
Since Dapr.Actors references Dapr.Client, it should be ok to move ConditionalValue<T> to the latter assembly.
I do wonder if/how that might impact the bulk get state operations (e.g. in light of #1173). Do they also need a means of indicating that the state was not found? (Off the top of my head, I don't recall whether bulk get state operations omit entries for keys that do not exist, and the API docs don't explicitly state one way or the other.)
This whole proposal evolved from reading through the SDK code to answer the question "What happens if the key doesn't exist in the state?". The docs don't mention it, but after completing this work, I'd be happy to clarify this either way in the docs as well.
I wasn't a huge fan of ConditionalValue<T> in the first place either and favored returning a null instead of the value, but I think that ship has sailed as it wouldn't make sense to handle state operations in actors vs non-actors differently, I think.
Another method that could be addressed here: GetStateAndETagAsync does the same thing as GetStateAsync right now (returns a default(T) if not found), so I'd propose adding a TryGetStateAndETagAsync and just wrap the result in a ConditionalValue<T>.
With regards to #1173 , I'm going to have to dig through what's happening in the sidecar as I just look at look at the .NET SDK and it's just enumerating the returned items, so any exclusions for missing keys happen external to the SDK. That said, I'd be happy to take a stab at making changes there as well (if necessary) to better support a Try variant of the GetBulkStateAsync methods as well.