Replace SharedPreferences with typed data stores - #702
Conversation
| import app.grapheneos.camera.data.core.store.legacyCommonPreferences | ||
| import app.grapheneos.camera.data.core.store.removeLegacyCommonKeys | ||
|
|
||
| // The stores migrate the shared file in arbitrary order, so each removes only its own keys. |
There was a problem hiding this comment.
By migrating to data store (stored in /data/user/<userid>/app.grapheneos.camera/datastore/), the SharedPreferences exclusion line in https://github.com/m4pl/Camera/blob/prefs-settings-repo/app/src/main/res/xml/full_backup_content.xml no longer applies. We need to confirm if we should continue excluding preferences
| fun updateLastCapturedItem(item: CapturedItem) { | ||
| commonPref.edit { | ||
| saveLastCapturedItem(item, this) | ||
| } | ||
|
|
||
| if (mActivity is SecureMainActivity) { | ||
| // previous call updated ephemeral SharedPreferences that won't be accessible by the | ||
| // "regular" MainActivity | ||
| mActivity.applicationContext.getSharedPreferences( | ||
| COMMON_SHARED_PREFS_NAME, | ||
| Context.MODE_PRIVATE | ||
| ).edit { | ||
| saveLastCapturedItem(item, this) | ||
| } | ||
| } | ||
| runBlocking { capturedItemRepository.saveLastCapturedItem(item) } | ||
|
|
||
| lastCapturedItem = item | ||
| } |
There was a problem hiding this comment.
updateLastCapturedItem can now throw java.io.IOException from capturedItemRepository.saveLastCapturedItem, e.g. full disk errors
| settings = runBlocking { | ||
| settingsRepository.update { it.copy(gridType = value) } | ||
| } |
There was a problem hiding this comment.
Feels like this pattern could maybe block main thread, same for videoQuality and flashMode
There was a problem hiding this comment.
It does block. Will be fixed right after Decompose CamConfig, when the screen moves to a ViewModel.
sdsantos
left a comment
sdsantos
left a comment
There was a problem hiding this comment.
All appears to be working fine, and the migration tests I did went smoothly.
| if (mActivity !is SecureActivity) { | ||
| CapturedItems.init(mActivity, this) | ||
| fetchLastCapturedItemFromSharedPrefs() | ||
| preferencesScope.launch(Dispatchers.Main.immediate) { |
There was a problem hiding this comment.
Why do you need to specify again the Dispatchers.Main.immediate context in launch if it's already set in the CoroutineScope constructor?
5c40e83 to
6899184
Compare
6899184 to
686a415
Compare
8f3a12a to
e07f57c
Compare
686a415 to
2e4bc5d
Compare
e07f57c to
779bc8b
Compare
2e4bc5d to
ee50578
Compare
SharedPreferences kept every setting, every SAF grant and the last captured item in untyped files, read and written on whichever thread asked. Replace it with three DataStores serialized as JSON - one for what the owner configured, one for the SAF trees, one for what the app has captured - each opened once in a SingletonComponent module and reached only through its repository. A DataMigration per family carries the legacy keys over on first use. An absent field means "unset", so a value equal to today's default is never written and a later change to that default still reaches existing installs. Installs migrated from SharedPreferences are the exception for the per-mode settings, whose defaults the old code wrote out explicitly. Lockscreen isolation now falls out of the wiring rather than a branch inside a store: an ActivityComponent provider hands a SecureActivity an in-memory snapshot of the owner's settings and SAF grants, while captured-media state stays durable so a photo taken from the lockscreen survives the session. Writes now block until the store has committed, where SharedPreferences.apply() returned before the write landed.
ee50578 to
9365390
Compare
inthewaves
left a comment
inthewaves
left a comment
There was a problem hiding this comment.
Looks good, I don't think there's not much else we can do if IOExceptions occur for those calls
5 participants
Closes #686