[native] Create a function that processes all the ops
ClosedPublic
Actions

Authored by tomek on May 7 2024, 7:55 AM.

Details

Reviewers

kamil
marcin
varun

Commits

rCOMM41680772eef6: [native] Create a function that processes all the ops

Summary

Create a single vector where all the ops are stored and process them in a single transaction.
This function isn't ideal, but I have discovered a lot of ways of how it can't be implemented. I checked a lot of ideas from Internet, but I haven't found anything that compiles (mainly due to a fact that we're storing a vector of unique pointers - but avoiding the pointers also requires a lot of work).

https://linear.app/comm/issue/ENG-6004/wrap-all-store-operations-from-one-action-in-transaction

Depends on D11902

Test Plan

Modified redux-utils and used this function to process the drafts operations. Then created a draft, closed and reopened the app and checked if the modified draft was present.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

tomek created this revision.May 7 2024, 7:55 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMay 7 2024, 7:55 AM

tomek added inline comments.May 7 2024, 7:56 AM

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
18 ↗	(On Diff #39892)	This includes the other `std::move` https://en.cppreference.com/w/cpp/algorithm/move

Harbormaster completed remote builds in B28734: Diff 39892.May 7 2024, 8:15 AM

tomek requested review of this revision.May 7 2024, 8:15 AM

tomek added a child revision: D11922: [native] Use processDBStoreOperations function everywhere.May 7 2024, 8:31 AM

marcin accepted this revision.May 8 2024, 3:15 AM

marcin added inline comments.

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
436 ↗	(On Diff #39892)	Perhaps we could create a hashmap in `CommCoreModule` constructor mapping from key string to a pointer to a store instance and just iterate here?

This revision is now accepted and ready to land.May 8 2024, 3:15 AM

tomek added inline comments.May 8 2024, 3:19 AM

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
436 ↗	(On Diff #39892)	Considered this, but each store has a different type - these are all template classes. I haven't found a way of making it compile.

marcin added inline comments.May 8 2024, 3:21 AM

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
436 ↗	(On Diff #39892)	Perhaps this could work: https://github.com/CommE2E/comm/blob/master/native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/MessageOperationsUtilities/MessageSpecs.h#L51-L104

tomek added inline comments.May 9 2024, 4:28 AM

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
436 ↗	(On Diff #39892)	The problem is that all the stores have different parents - each of them inherits from a template class with different type parameters, which means that there's no common parent class. Also, it would be more important if we would need to iterate in more than one place.