Migrate SharedPreferences to DataStore #2934
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update methods to handle flows - Initialize flows in the datastore to prevent duplication and have single source of truth
ellykits
reviewed
Dec 20, 2023
Comment on lines
45
to
46
| fileName = USER_INFO_DATASTORE_JSON, | ||
| serializer = UserInfoDataStoreSerializer, |
There was a problem hiding this comment.
Looks like extra leading spaces were applied. Did you run spotlessApply?
There was a problem hiding this comment.
No, PR is not ready yet. Would you advise to do spotless on every commit?
There was a problem hiding this comment.
Okay. Running spotlessApply after every commit would be preferred. You can run it once when the PR is ready.
There was a problem hiding this comment.
Enable ktfmt to format the code directly in the IDE. Follow the setup here in the Developer setup docs
- Add new keys - Migrate WritePractitionerDetails
kelvin-ngure
force-pushed
the
2930-migrate-sharedpreferences-to-datastore
branch
from
3ed612f
to
4c84ee1
Compare
…moval of sharedpreferences - Update quest/ConfigurationRegistry, LoginViewModel, and AppSetting View model to utilize preferences datastore and ditch sharedpreferences - Refactor key names and variables in writePractitionerDetailsToShredPref() for readability - Refactor test assertion that cmpared ResourceType enum values to strings - Update Rules Factory with preferences datastore - Update all associated tests
…test - Update RulesFactory enum names and flows
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2934 +/- ##
=========================================
+ Coverage 64.5% 66.1% +1.6%
- Complexity 1075 1127 +52
=========================================
Files 218 232 +14
Lines 9635 10862 +1227
Branches 1897 1980 +83
=========================================
+ Hits 6218 7190 +972
- Misses 2234 2426 +192
- Partials 1183 1246 +63
Flags with carried forward coverage won't be shown. Click here to find out more.
|
- Update Preferences DataStore read and write to have separate Generics for Key and return type. Useful e.g for LOCATION_IDS key is String, return type is List<String>
- Change the flow returning read()s to observe() and create a blocking read() function
- Update RulesFactory and RulesFactoryTest functions. Remove verify{} since no functions called. Flow values still validatable via every{}. read() to be tested on datastore tests
- Replace sharedpreferences usage in ConfigService
- Replace sharedpreferences usage in network module
- Replace usage in RegisterRepository and add tests
- Update tests for changed classes
Add a default value argument to datastore read() to allow setting default value e.g Locale.ENGLISH for language - Update AppSettingViewModel to datastore
- Replace observe()
- WIP to be replaced with readOnce()
- More integration
# Conflicts: # android/engine/build.gradle.kts # android/engine/src/main/java/org/smartregister/fhircore/engine/datastore/PreferenceDataStore.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/configuration/ConfigurationRegistryTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/data/local/register/RegisterRepositoryTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/sync/SyncBroadcasterTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/task/FhirCompleteCarePlanWorkerTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/task/FhirResourceExpireWorkerTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/task/FhirTaskStatusUpdateWorkerTest.kt # android/quest/src/androidTest/java/org/smartregister/fhircore/quest/integration/Faker.kt # android/quest/src/androidTest/java/org/smartregister/fhircore/quest/integration/ui/report/measure/components/MeasureReportResultScreenTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/CqlContentTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/app/ConfigurationRegistryTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/appsetting/AppSettingViewModelTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/login/LoginViewModelTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/pin/PinViewModelTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireViewModelTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/register/RegisterFragmentTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/register/RegisterViewModelTest.kt
- Add UninstallModules due to duplicate bindings error
# Conflicts: # android/engine/src/test/java/org/smartregister/fhircore/engine/data/local/DefaultRepositoryTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/data/local/register/RegisterRepositoryTest.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/task/FhirResourceExpireWorkerTest.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainViewModel.kt
…m practitioner Datastore and use comma delimiter
# Conflicts: # android/engine/src/main/java/org/smartregister/fhircore/engine/configuration/ConfigurationRegistry.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/configuration/ConfigurationRegistryTest.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/ui/appsetting/AppSettingViewModel.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/ui/register/RegisterViewModel.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/app/ConfigurationRegistryTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/appsetting/AppSettingViewModelTest.kt
- Serializable wrapper for Location Hierarchies - Remore PractitionerDetails write and use datasore state to check for authentication - Refactor user insights screen usage of lists stored in datastore
# Conflicts: # android/engine/src/main/java/org/smartregister/fhircore/engine/di/NetworkModule.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/app/fakes/Faker.kt # android/engine/src/test/java/org/smartregister/fhircore/engine/configuration/ConfigurationRegistryTest.kt # android/quest/src/androidTest/java/org/smartregister/fhircore/quest/integration/Faker.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/data/DataMigration.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainViewModel.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/ui/register/RegisterFragment.kt # android/quest/src/main/java/org/smartregister/fhircore/quest/ui/register/RegisterViewModel.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/CqlContentTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/app/fakes/Faker.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/main/AppMainActivityTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireViewModelTest.kt # android/quest/src/test/java/org/smartregister/fhircore/quest/ui/report/measure/MeasureReportViewModelTest.kt
…com:opensrp/fhircore into 2930-migrate-sharedpreferences-to-datastore
11 tasks
|
Closing this PR in favor of #3205 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
IMPORTANT: Where possible all PRs must be linked to a Github issue
Fixes #2930
Pull request highlights
Moved Provision of the datastore to DataStore module. In test, Hilt injection overrides the dispatcher yet instantiation uses the correct testdispatcher. As a result, we don't use the DataStore module in test but rather ask the Faker to build a preferencesDataStore instance
Made a lot of the flows ready for observation from within the PreferencesDataStore.kt file. It therefore acts as a repository as well. These flows use the read() (now renamed to observe()) function from the previous PR.
The read() function has been renamed to observe(). It is arguably a more apt description of what the flow does
A new function called readOnce() has been introduced to allow us to capture the value of a flow when we do not want to subscribe to it’s emissions using the observe() function. Currently most, of the code in the engine needs this readOnce() behavior. Nevertheless, the observe() function will prove useful e.g in the feature to allow multiple users to log in on the same device or when consumed by StateFlows or LiveData in the UI.
This function is also useful for testing. It allows us to keep the tests unchanged in most of the places since it is a blocking read just like the sharedPreferences read (replacing reads with a flow would render all
verify{}blocks useless since no function is called). Any need for structured concurrency can be addressed in the future for individual data stored. At this time, the focus is on introducing datastore into the codebase without refactoring too much to prevent adding bugs through what is likely to be a fairly large PRAdded a reified read() (and write()) that used gson for encoding similar to the one in the SharedPreferences. This is useful because
i. It is useful for storing List structures and not having casts all over the code
ii. It may be useful for temporarily storing structured data in the preferences datastore as we await decisions on what is to be stored in protodatastore
location and locations have been renamed to locationIds and locationNames for readability. The same refactors have been applied to careTeam and careTeams. Similarly named keys have been created for them in the PreferencesDatastore
The observe() function Generic signature has changed from T to T,M this allows the key and the output to have different types. This was necessary e.g when trying to observe a key value pair whose key is a string but whose reified datatype is a List such as in the case of CARE_TEAM_IDS
Moved keys to once place. Using ResourceType.name ended up being restrictive. For example, we store, location ids and location names in SharedPreferences. ResourceType only has Location. This caused location ids to be stored under ResourceType.name (where ResourceType = Location) and location names be stored under SharedPreferenceKeys.location. As a result, any data being stored has a specific key name made for it within PreferencesDatastore.kt for code readability and flexibility of use.
Will need discussion on whether my replacement of sharedPreferences in BaseMultiLanguageActivity is correct
Engineer Checklist
strings.xmlfile./gradlew spotlessApplyand./gradlew spotlessCheckto check my code follows the project's style guideCode Reviewer Checklist
strings.xmlfile