Rollback SQLite migration to shared location
ClosedPublic
Actions

Authored by marcin on Jun 1 2022, 8:24 AM.

Details

Reviewers

tomek
atul

Commits

rCOMMa45ef9d83e06: Rollback SQLite migration to shared location

Summary

This differential rollbacks SQLite file migration from app-specific to app-groups location

Test Plan

Comment re-migration code and use getAppGroupSQLiteFilePath instead. Build the app, check that the path is an app group path. Then uncomment re-migration code, use getSQLiteFilePath and build the app. Check that it works and sqlite file path is app-specific and no database migration is executed. It is essential that app is not uninstalled between those two steps.

Diff Detail

Repository

rCOMM Comm

Branch

marcin/eng-1216

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Jun 1 2022, 8:24 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptJun 1 2022, 8:24 AM

Harbormaster completed remote builds in B9575: Diff 13273.Jun 1 2022, 8:31 AM

marcin requested review of this revision.Jun 1 2022, 8:31 AM

tomek requested changes to this revision.Jun 1 2022, 9:49 AM

tomek added inline comments.

native/ios/Comm/AppDelegate.mm
247–249	Shouldn't we do something opposite: move from `sqliteFilePath` to `[appSpecificSQLiteFilePath UTF8String]`?

This revision now requires changes to proceed.Jun 1 2022, 9:49 AM

marcin requested review of this revision.Jun 3 2022, 5:03 AM

marcin added inline comments.

native/ios/Comm/AppDelegate.mm
247–249	This differential reverts changes that introduced `getAppSpecificSQLiteFilePath` method in the first place. Previously we had just one function in `Tools.h` called `getSQLiteFilePath` which returned database path that was app-specific. Changes that this differential introduces bring back former functionality and `[Tools getSQLiteFilePath]` returns app-specific path for SQLite database.

tomek added inline comments.Jun 3 2022, 5:57 AM

native/ios/Comm/AppDelegate.mm
247–249	Yes, but as a part of the solution we were moving the database from one location to another. Shouldn't we move it back to the app-specific location if `sqliteFilePath` exists?

marcin added inline comments.Jun 6 2022, 1:50 AM

native/ios/Comm/AppDelegate.mm
247–249	Are you asking because the code that migrates database was already released to users? If yes, then you are right. But I suggest that instead of `sqliteFilePath` and `appSpecificSQLiteFilePath` we use `sqliteFilePath` for the app specific (default) and appGroupSQLiteFilePath`. I think it is reasonable that the default sqlite path is accessed with the simplest method name. Otherwise we could be more straightforward and use `appSpecificSQLiteFilePath` and `appGroupSQLiteFilePath`. But we definitely should not use `sqliteFilePath` for the path that is not a default one and we cannot even tell (at this moment) when we are going to use it.