Serialization issues with Transaction upsert operation

[mukundansundar]

In what area(s)?

/area runtime

What version of Dapr?

0.10.0

Expected Behavior

When a state is saved via the the executeTransaction method via GRPC, Multi method implementation for redis state store, and the state is again read in via the getState API, there is a deserialization issue.

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `io.dapr.it.state.MyData` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('{"propertyA":"data in property A","propertyB":"data in property B"}')
 at [Source: (byte[])""{\"propertyA\":\"data in property A\",\"propertyB\":\"data in property B\"}""; line: 1, column: 1]

Found during implementation of the transaction API for Java SDK.

Actual Behavior

State saved via transactional API or the normal save state API should be read correctly via the Get State API

Steps to Reproduce the Problem

1. Install Dapr 0.10.0
2. Create a GRPC server implementing the Execute Transaction call
3. Save State via the transaction API GRPC
4. Get State and deserialize

Potential issue

In the runtime, the req.Value parameter is converted to string and then the Multi interface is called.
Specifically in redis component, the String value is marshaled again.
When the value is an object, this causes additional escape characters to be added and finally the object is only deserialized once into a string which is where the String Args constructor exception comes from.

Example of escape characters introduced:

data: "\"{\\\"propertyA\\\":\\\"data in property A\\\",\\\"propertyB\\\":\\\"data in property B\\\"}\""

instead of normal serialization into

data: "{\"propertyA\":\"data in property A\",\"propertyB\":\"data in property B\"}"

Potential fix:

1. Do not convert byte array into string in runtime, let the components themselves handle either byte array or string cases in a component specific manner. line
2. Handle redis component Upsert transaction serialization in the same way serialization is handled in the saveState method.
   code to fix save state flow
3. Test for regression scenarios for actors as well.

[mukundansundar]

With Java SDK
Using the saveState API saves it as

127.0.0.1:6379> hgetall "HttpStateClientIT_||myKey"
1) "data"
2) "\"my state\""
3) "version"
4) "1"

Using GRPC Execute Transaction. Saves it as a string and also we can see the extra escaped quotes.

127.0.0.1:6379> get "GRPCStateClientIT_||myTKey"
"\"{\\\"propertyA\\\":\\\"data in property A\\\",\\\"propertyB\\\":\\\"data in property B\\\"}\""
127.0.0.1:6379>

[youngbupark]

cc/ @yaron2 @orizohar @mchmarny @msfussell

[yaron2]

My opinion is to go with fix .1 in the runtime.

[mukundansundar]

@yaron2 All three steps of the fix are necessary.

[yaron2]

@yaron2 All three steps of the fix are necessary.

Ok, just making sure we're not doing a Redis only fix here. We need to make sure that the runtime does the right thing.

[vinayada1]

Here is the behavior with .net sdk:-
With SaveState API:-
127.0.0.1:6379> hgetall 'gRPC_Client||mykey'

1. data
2. {"size":"small","color":"yellow"}
3. version
4. 1

With TransactionState Upsert operation:-
127.0.0.1:6379> get "gRPC_Client||mystate"
"{"Size":"small","Color":"yellow"}"

With Transaction APIs, get after a save state results in a deserialization exception:-
== APP == Unhandled exception. System.Text.Json.JsonException: The JSON value could not be converted to DaprClient.Program+Widget. Path: $ | LineNumber: 0 | BytePositionInLine: 43.

== APP == at System.Text.Json.ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType)

== APP == at System.Text.Json.JsonPropertyInfoNotNullable`4.OnRead(ReadStack& state, Utf8JsonReader& reader)

== APP == at System.Text.Json.JsonPropertyInfo.Read(JsonTokenType tokenType, ReadStack& state, Utf8JsonReader& reader)

== APP == at System.Text.Json.JsonSerializer.ReadCore(JsonSerializerOptions options, Utf8JsonReader& reader, ReadStack& readStack)

== APP == at System.Text.Json.JsonSerializer.ReadCore(Type returnType, JsonSerializerOptions options, Utf8JsonReader& reader)

== APP == at System.Text.Json.JsonSerializer.Deserialize(String json, Type returnType, JsonSerializerOptions options)

== APP == at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)

== APP == at Dapr.Client.DaprClientGrpc.GetStateAsync[TValue](String storeName, String key, Nullable1 consistencyMode, Dictionary2 metadata, CancellationToken cancellationToken) in /Users/vinayada/dapr/dotnet-sdk/src/Dapr.Client/DaprClientGrpc.cs:line 326

== APP == at DaprClient.Program.GetStateAfterTransactionAsync(DaprClient client) in /Users/vinayada/dapr/dotnet-sdk/samples/Client/DaprClient/Program.cs:line 133

== APP == at DaprClient.Program.Main(String[] args) in /Users/vinayada/dapr/dotnet-sdk/samples/Client/DaprClient/Program.cs:line 55

== APP == at DaprClient.Program.

(String[] args)

[vinayada1]

Also, note that the data is being stored differently to the state store by Java and .net sdks. Therefore, set by one and get by other would fail too.

[mchmarny]

the go-sdk has an explicit upsert test to validate this in a unit test but it looks like when used with a state component backed by Redis service the serialization issue is there:

== APP == expected: { "message": "hello" }
== APP == got     : "\"{ \\\"message\\\": \\\"hello\\\" }\""

[mchmarny]

I was able to replicate in PostgreSQL as well... that's after fighting Mongo replicaset for 20 min (required for transaction support). Looks like this is Dapr core level issue.

== APP == expected: { "message": "hello" }
== APP == got     : "\"{ \\\"message\\\": \\\"hello\\\" }\""

[youngbupark]

HTTP transaction api (including actor transactional api) deserializes the entire payload to extract operation, value, and key, so value is non-string or non-byte array value when user pass non-string/non-byte array. then component serializes this non-string and non-bytes value.

But grpc apis requires no deseralization to extract value. When Dapr passes this value to the existing redis component, redis component will serialize this to base64 string (which is json primitive type conversion). if we convert this to string, then serializer will add double-quotes. e.g. []byte("hello") -> ""hello""

I agree upon the potential fix that @mukundansundar. component needs to take care about deserialized value (for http), raw value (for grpc) accordingly. we need to find the way to minimize the impact on this change.

[mchmarny]

My bad, fat-fingered trackpad click :(