This differential implements CryptoModule serialization and atomic write to a file under given path. Its implementation ensures that write is made atomically and in fault-tolerant manner.
Details
Details
Temporarily make this and deserialization method public. In CommCoreModule in initializeCryptoAccount method pass created CryptoModule instance, path returned from PlatformSpecificTools::getNotifications... and some random string (as pickling key) to this method. Then
pass the same pickling key and path returned from PlatformSpecificTools::getNotifications... to NotificationsCryptoModule::deserializeCryptoModule. Using XCode debugger or logging ensure initial CryptoModule matches the one returned from this method.
Note that it is an e2e test that also tests previous differential in more reliable manner.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp | ||
|---|---|---|
| 69 ↗ | (On Diff #22712) | We use this argument to create a temporary file path. We could create temporary file path using random string but what if any of the steps between temporary file creation and temporary file rename fails? We would be left with some garbage that should be deleted. We could use directory tree search POSIX api to get the path of such garbage temporary file but it would add additional complexity to the solution. |
| native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp | ||
|---|---|---|
| 119 ↗ | (On Diff #22880) | If file at path already exists, this is expected to overwrite it, right? Reading the docs says this behavior is stdlib implementation-dependant:
|
| native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp | ||
|---|---|---|
| 119 ↗ | (On Diff #22880) | Can you link the docs you found? The ones I was using are explicit that existing file will be replaced: |
| native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp | ||
|---|---|---|
| 119 ↗ | (On Diff #22880) | I optimistically suppose that most implementations replace the file anyway. If we want to be 100% sure, we could simply test it on iOS and Android. |
| native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp | ||
|---|---|---|
| 119 ↗ | (On Diff #22880) | Running the test plan on a freshly installed app should test it since on a freshly installed app the file (under the path returned by PlatformSpecificTools::getNotificationsCryptoAccountPath) does not exists so rename method is called with non-existing file path as newname argument. I executed this test plan on a fresh install and it worked as expected. |
| native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp | ||
|---|---|---|
| 119 ↗ | (On Diff #22880) | Ok, thanks for explaining |