Skip to content

Fix issue where custom JSOn serializer generates Base64 encoded data in state store#504

Merged
artursouza merged 7 commits into
dapr:masterfrom
abogdanov37:feature_fix-statestore-serialization
Mar 11, 2021
Merged

Fix issue where custom JSOn serializer generates Base64 encoded data in state store#504
artursouza merged 7 commits into
dapr:masterfrom
abogdanov37:feature_fix-statestore-serialization

Conversation

@abogdanov37

Copy link
Copy Markdown
Contributor

Description

Refactor ObjectSerializer class to add ability set custom object mapper.
Refactor DaprStateAsyncprovider class to use custom serializers based on ObjectSerializer class as Json serializer.
Add unit tests.

Issue reference

#503

Checklist

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

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Andrey Bogdanov added 3 commits March 9, 2021 16:10
…er. Refactor DaprStateAsyncprovider class to use custom serializers based on ObjectSerializer class as Json serializer. Add unit tests.
@artursouza

Copy link
Copy Markdown
Contributor

Please, use the DaprObjectSerializer interface to bring your own serializer. The default serializer is just for examples and getting started.

Comment thread sdk/src/main/java/io/dapr/client/ObjectSerializer.java Outdated
Comment thread sdk-actors/src/main/java/io/dapr/actors/runtime/DaprStateAsyncProvider.java Outdated
@artursouza

Copy link
Copy Markdown
Contributor

Please, focus on fixing the issue where a custom JSON serializer becomes Base64 encoded. Making DefaultObjectSerializer is a controversial change since it is not supposed to be used outside examples or getting started with Dapr.

@abogdanov37

abogdanov37 commented Mar 10, 2021

Copy link
Copy Markdown
Contributor Author

Please, focus on fixing the issue where a custom JSON serializer becomes Base64 encoded. Making DefaultObjectSerializer is a controversial change since it is not supposed to be used outside examples or getting started with Dapr.

Ok. I will revert changes with ObjectSerializer and focused on the issue. Thanks for your feedback.

@abogdanov37 abogdanov37 requested a review from artursouza March 11, 2021 19:08
@codecov

codecov Bot commented Mar 11, 2021

Copy link
Copy Markdown

Codecov Report

Merging #504 (51b44be) into master (a5051e0) will not change coverage.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #504   +/-   ##
=========================================
  Coverage     80.43%   80.43%           
+ Complexity      975      974    -1     
=========================================
  Files            88       88           
  Lines          3015     3015           
  Branches        331      330    -1     
=========================================
  Hits           2425     2425           
  Misses          424      424           
  Partials        166      166           
Impacted Files Coverage Δ Complexity Δ
...io/dapr/actors/runtime/DaprStateAsyncProvider.java 60.86% <33.33%> (ø) 13.00 <0.00> (-1.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5051e0...51b44be. Read the comment docs.

this.daprClient = daprClient;
this.stateSerializer = stateSerializer;
this.isStateSerializerDefault = stateSerializer.getClass() == DefaultObjectSerializer.class;
this.isStateSerializerJson = JSON_CONTENT_TYPE.equals(stateSerializer.getContentType());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make this check even more extensible by having a deeper understanding of different media types. This change is already an important improvement.

@artursouza artursouza changed the title Add ability to customize json serializer Fix issue where custom JSOn serializer generates Base64 encoded data in state store Mar 11, 2021
@artursouza artursouza merged commit 1eff655 into dapr:master Mar 11, 2021
artursouza added a commit that referenced this pull request Mar 16, 2021
Merge pull request #504 from abogdanov37/feature_fix-statestore-seria…
artursouza added a commit that referenced this pull request Mar 17, 2021
…re-serialization" (#520)

This reverts commit 1eff655, reversing
changes made to a5051e0.
artursouza added a commit that referenced this pull request Mar 24, 2021
…re-serialization" (#519)

This reverts commit 1eff655, reversing
changes made to a5051e0.
fackee pushed a commit to fackee/java-sdk that referenced this pull request Mar 28, 2022
…estore-serialization" (dapr#519)

This reverts commit 1eff655, reversing
changes made to a5051e0.

Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
fackee pushed a commit to fackee/java-sdk that referenced this pull request Mar 28, 2022
…estore-serialization" (dapr#519)

This reverts commit 1eff655, reversing
changes made to a5051e0.

Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants