Stores: Remove StoreCipher::{en,de}crypt_value_[base64_]typed
#3638
Conversation
reuse `IndexeddbSerializer::maybe_encrypt_value` instead of re-inventing it.
Instead of un-base64-ing and calling `decrypt_value_typed` (which deserializes the result`), call `decrypt_value_base64_data` (which un-base64s before decrypting but does not deserialize the result), then deserialize. This makes it more symmetrical with `encrypt_value_base64_typed`, and helps me get rid of `decrypt_value_typed` (which is barely used.)
looks like they got C&Ped from `encrypt_value_typed`.
Each of these are quite simple, are only used in two places, and their existence melts my brain.
... to use `en/decrypt_value_base64_data` instead of `en/decrypt_value_base64_typed`. We have to have the de/serialization code for the unencrypted case anyway, so using the higher-level method isn't helping us much.
Outside of tests, these things are totally unused.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3638 +/- ##
==========================================
+ Coverage 84.19% 84.22% +0.03%
==========================================
Files 256 256
Lines 26594 26585 -9
==========================================
+ Hits 22391 22392 +1
+ Misses 4203 4193 -10 ☔ View full report in Codecov by Sentry. |
| /// assert_eq!(value, decrypted); | ||
| /// # anyhow::Ok(()) }; | ||
| /// ``` | ||
| pub fn decrypt_value_typed<T: DeserializeOwned>( |
There was a problem hiding this comment.
Removing this removes the ability to use the StoreCipher with more efficient serialization formats, which at least one user used in the past: #688.
Also, we wanted to remove the other, the JSON based, methods: #718.
While I understand the baroqueness argument and the poor naming of the methods I don't think I'd want to remove this functionality.
There was a problem hiding this comment.
Removing this removes the ability to use the
StoreCipherwith more efficient serialization formats, which at least one user used in the past: #688.
Well, you can use encrypt_value_data instead?
Also, we wanted to remove the other, the JSON based, methods: #718.
While I understand the baroqueness argument and the poor naming of the methods I don't think I'd want to remove this functionality.
Well, I'd quite like to get rid of encrypt_value and decrypt_value as well; I just didn't get that far.
Per my note on #718, I'm slightly confused about what it's asking for. In particular I note that #3086 deprecated encrypt_value_typed. Has your thinking changed here?
There was a problem hiding this comment.
Well, you can use encrypt_value_data instead?
Ah indeed, we still have that one. Then I don't have any objections.
StoreCipher's interface has grown baroque over the years. These four functions aren't giving a whole lot of value: they are only used in a couple of places in the codebase, and are pretty simple.This PR is a mission to clean that up.
Review commit-by-commit.