dapr / dotnet-sdk

Dapr SDK for .NET
Apache License 2.0
1.12k stars 340 forks source link

Add overload to deserialize GetBulkStateAsync item values #1173

Closed WhitWaldo closed 9 months ago

WhitWaldo commented 1 year ago

Adds overload to BulkStateItem and GetBulkStateAsync to perform SDK-based deserialization of returned values instead of strictly returning serialized strings.

Signed-off-by: Whit Waldo whit.waldo@innovian.net

Description

Explained in more detail at #1172 , but GetBulkItemAsync returns a BulkStateItem with a Key, ETag and Value wherein each is typed as a string and the user is expected to deserialize the Value. Because the state might have been originally added one by one, relying on Dapr and its serialization options to serialize everything, this can cause downstream issues deserializing values not originally serialized by the developer.

As such, just as GetStateAsync allows a type parameter to deserialize each returned value, I've added the same overload to GetBulkItemAsync to deserialize using the same mechanism as GetStateAsync to deserialize each of the returned items as part of an overloaded and generically-typed BulkItemState.

Issue reference

As it's a simple addition, I didn't think it really merited a lot of discussion as the functionality is already available for single items (and this just adds the same for multiple items).

Please reference the issue this PR will close: #1172

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

I'll work on finding relevant documentation and updating that right now.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (034de3e) 67.28% compared to head (fada655) 67.36%.

Files Patch % Lines
src/Dapr.Client/BulkStateItem.cs 71.42% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1173 +/- ## ========================================== + Coverage 67.28% 67.36% +0.08% ========================================== Files 174 174 Lines 6006 6028 +22 Branches 670 672 +2 ========================================== + Hits 4041 4061 +20 - Misses 1798 1800 +2 Partials 167 167 ``` | [Flag](https://app.codecov.io/gh/dapr/dotnet-sdk/pull/1173/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr) | Coverage Δ | | |---|---|---| | [net6](https://app.codecov.io/gh/dapr/dotnet-sdk/pull/1173/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr) | `67.34% <92.30%> (+0.08%)` | :arrow_up: | | [net7](https://app.codecov.io/gh/dapr/dotnet-sdk/pull/1173/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr) | `67.34% <92.30%> (+0.08%)` | :arrow_up: | | [net8](https://app.codecov.io/gh/dapr/dotnet-sdk/pull/1173/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr) | `67.35% <92.30%> (+0.08%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hhunter-ms commented 10 months ago

@halspang @philliphoff is this ready for merge? just checking since there's a corresponding docs PR waiting in the wings for this :)

philliphoff commented 9 months ago

@hhunter-ms @halspang If Hal is ok with the last set to changes he requested (and the required merge is clean), I say merge it.