slawlor / ractor

Rust actor framework
MIT License
1.3k stars 66 forks source link

Serde blanket implementation feature #229

Closed stalkopat closed 2 months ago

stalkopat commented 2 months ago

When working with types in outside crates it quickly becomes a nuisance to implement various wrapper types just to be able to implement BytesConvertable. This becomes especially annoying once one has to interface with an existing codebase that has the original types deeply ingrained.

This PR adds an optional feature called "blanket_serde" which replaces the current default implementations of BytesConvertable with a blanket implementation utilizing serde and pot.

In the process a handful of bugs were found, where tests and BytesConvertable implementations were inconsistent in their Deserialization / Serialization. For example there were cases of Strings into_bytes() function being used for serialization in combination with ::from_bytes being used for deserialization, this didn't cause any errors since the default implementation similarly just called .into_bytes().

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 96.03960% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 81.76%. Comparing base (8c310e7) to head (dfb962c). Report is 17 commits behind head on main.

Files Patch % Lines
ractor/src/serialization.rs 94.93% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #229 +/- ## ========================================== + Coverage 79.73% 81.76% +2.02% ========================================== Files 50 50 Lines 9790 9695 -95 ========================================== + Hits 7806 7927 +121 + Misses 1984 1768 -216 ```

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

stalkopat commented 2 months ago

Added blanket_serde to the docker integration tests Changed the docker integration tests to actually use BytesConvertable (Previous PingPong actor never serialized anything to a remote actor) Fixed a bug where the pg_group integration test wrongly returned success when the underlying actor paniced / errored