is there an existing task tracking implementing backup restore for corrupt database recovery, for both native and web? I want to add a comment referencing this code in SQLiteDataHandler, to make sure it's updated as part of that task.
Are we going to backup this table? If so then we should add it to NativeSQLiteConnectionManager so that logs are capture for this table. If not then we should update createMainCompaction method so that removeDeviceSpecificDataSQL script removes data from this table after backup is created.
Just to make sure: sessionRecoveryInProgress isn't persisted, right?
I think that the practice of keeping commented-out types has a big advantage of reducing confusion when searching older revisions while it doesn't have any significant disadvantages.
However, if there is no specialRole field encountered, we will assume we're dealing with a Server/"legacy" type and fallback to sketchy string search.
What is the scenario when this can happen?
We should reintroduce the util (probably as a simple function) if we would need to decide if we can edit / delete a role in some other places.
I don't understand the reason behind this diff and the task from the summary doesn't have any description. Why can't we simply check RoleInfo and its specialRole field?
Add detail to code comment about removing boundSetNewSession call once usingCommServicesAccessToken is true
Sat, Mar 2
Fri, Mar 1
Just going to accept this to move it off my queue, but in our 1:1 @atul mentioned he still wanted to do some additional testing before landing
Looks good to me. Might want to adjust the diff title and test plan but that's really it.
@will can you give this one more pass? it's changed quite a bit
make the allow origin list more configurable
Refactor to match changes in parent differential
- Refactor to match new iOS logic.
- Address Bartek and Tomek comments.
Update iOS code to fix reentrancy issues with the NSE
Fix reentrancy issues with MMKV in the NSE.
This differential is incorrect. The reason is that in case user logs out the main app will update encryption key in secure store, but the NSE will the old encryption key in a static variable. The NSE is not guaranteed to be restarted for each notif so this is a concern. To mitigate this we need to fetch encryption key each time we call initialize provided we are running in the NSE. Unfortunately it is not enough. MMKV is implemented in such a way that there is a global dictionary mapping from mmkv id to MMKV* instance. When we call mmkvWithID the library first checks if an instance is in the dictionary and returns it in case it is. Each MMKV* instance keeps cryptKey as an instance variable. Therefore even if we fetch new encryption key in running NSE process we will still get MMKV* instance with outdated encryption key. To fully solve the issue we should rotate both encryption key and mmkv id. I will update the diff shortly.