-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue where custom JSON serializer generates Base64 encoded data in state store #539
base: master
Are you sure you want to change the base?
Changes from all commits
536ba0d
4f87869
301587d
199673a
5155e49
0886ff2
db1f822
d73e06b
8c08b82
7153b41
0e71cf5
278c7ff
bea49e8
c735f66
9dc08d4
2d2e162
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
|
||
import java.io.IOException; | ||
import java.nio.charset.Charset; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.ArrayList; | ||
|
||
/** | ||
|
@@ -68,6 +69,19 @@ <T> Mono<T> load(String actorType, ActorId actorId, String stateName, TypeRef<T> | |
return Mono.empty(); | ||
} | ||
|
||
if (!this.isStateSerializerDefault) { | ||
if (s.length == 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this needed only in the deserialize path? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In another way, we'll get an exception as I remember correcly |
||
return Mono.empty(); | ||
} | ||
s = OBJECT_MAPPER.readValue(s, byte[].class); | ||
if (type.getType().equals(String.class) && s[0] != '"') { | ||
byte[] out = new byte[s.length + 2]; | ||
out[0] = '"'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not cover the scenarios where the string contains a quote already since it would not be escaped. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quotes will be added in the serialization process. Give me please an example I'll add it in tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I add two test cases for quoted strings. As I saw in debugging, additional quotes had been escaped with the serializer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am still concerned about this logic. This should not be needed. I will need more time to try this PR on my desktop first. |
||
System.arraycopy(s, 0, out, 1, s.length); | ||
out[out.length - 1] = '"'; | ||
s = out; | ||
} | ||
} | ||
T response = this.stateSerializer.deserialize(s, type); | ||
if (this.isStateSerializerDefault && (response instanceof byte[])) { | ||
if (s.length == 0) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static final variables should be capitalized: ERROR_MSG