From bda2f7c002c2a3126d4e7d50854319ce665fcaa8 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 3 Sep 2019 10:34:25 -0700 Subject: [PATCH] Fix #4, #70, #71, #86, #87: Introduce data module (#85) * Initial introduction of the data source module. High-level notes: - Data sources are now being referred to as 'DataProviders' - This commit also fixes the build which was broken in a previous PR ('compile' needs to be used for the proto library) - The data source module is referred to as just 'data' - This introduces a few style changes to prohibit wildcard imports - This also normalizes the JDK versions needed across modules, potentially fixing some of the repeated reversions the IDE does for a Java SDK version setting Functional differences: the app now properly shows the correct string upon initial open ("Welcome to Oppia"), and reopening thereafter correctly shows the updated string ("Welcome back to Oppia"). Rotating the device is currently broken without Dagger integration. Overall, this moves all general data provider functionality into the new module, and introduces a PersistentCacheStore to store proto messages on-disk, or load them into memory if they are available. Overall, this commit is introducing a potentially reasonable medium-term solution for data providers and on-disk proto storage. A long-term design should still be determined, but this seems like a good place to start. Tests are currently broken or missing. New TODOs will also be resolved in a later commit. * Introduce one possible integration with Dagger 2. This is loosely based on both https://github.com/tfcporciuncula/dagger-simple-way and https://dagger.dev/android.html, except this approach is meant to keep activities and fragments as thin as possible and push as much UI presentation and service business logic into Dagger-provided objects as possible. This maximizes Dagger use throughout the codebase, and opens the potential for future code generation for these thin adapter fragments & services. This fixes the rotation bug in the app: rotating upon the initial app open retains the 'Welcome to Oppia' label since the same user app history controller is used in both instances of HomeFragment due to it being singleton scoped and not needing to be recreated. No tests were updated with these changes. * Move data singletons and blocking/background dispatchers to Dagger modules & Dagger graph. Update HomeScreenActivityTest to include a new test for the second-app-launch test case. Note that a significant amount of the work going into this commit was around properly setting up tests to switch out their coroutine dispatchers with dispatchers that could be controlled in a test context. Although the affected test suites are passing in this commit, the work isn't quite done yet. Espresso and Robolectric require slightly different mechanisms to block on dispatchers, and with Gradle there isn't an easy way to set up the build graph such that Espresso can do this reliably. A temporary sleep-based workaround wad added for Espresso. Long-term, all tests should use Espresso's idling resource and facilitate proper background thread usage (which also simplifies the Robolectric side--no need to force sequential execution or switch the main thread dispatcher). Otherwise, this commit essentially "Dagger-ifies" the rest of the app. This will become more beneficial over time, especially after the codebase moves to Bazel since we'll be able to more easily swap out modules to change dependency implementations in different contexts. * Switch TODOs with references to issues. * Fix typo. * Introduce tests for InMemoryBlockingCache, DataProviders, and PersistentCacheStore. This fixes a few issues in PersistentCacheStore and the DataProvider->LiveData conversion (mostly to ensure that duplicate data notifications don't actually result in propagations to the UI). This also includes correct equals, hash code, and toString behavior for AsyncResult. The PersistentCacheStore tests aren't perfect, but they do expose some issues with the current implementation and seem to provide a reasonable breadth in core coverage. It may be worthwhile to eventually rewrite this class into something both cleaner and easier to maintain. --- .gitignore | 1 + .idea/codeStyles/Project.xml | 6 + .idea/gradle.xml | 1 + app/build.gradle | 29 +- app/src/main/AndroidManifest.xml | 3 +- .../main/java/org/oppia/app/HomeActivity.kt | 14 - .../main/java/org/oppia/app/HomeFragment.kt | 55 - .../org/oppia/app/UserAppHistoryViewModel.kt | 10 - .../oppia/app/activity/ActivityComponent.kt | 23 + .../org/oppia/app/activity/ActivityModule.kt | 7 + .../org/oppia/app/activity/ActivityScope.kt | 6 + .../activity/InjectableAppCompatActivity.kt | 43 + .../app/application/ApplicationComponent.kt | 22 + .../app/application/ApplicationContext.kt | 6 + .../app/application/ApplicationModule.kt | 26 + .../oppia/app/application/OppiaApplication.kt | 23 + .../oppia/app/fragment/FragmentComponent.kt | 19 + .../org/oppia/app/fragment/FragmentScope.kt | 6 + .../oppia/app/fragment/InjectableFragment.kt | 23 + .../java/org/oppia/app/home/HomeActivity.kt | 16 + .../oppia/app/home/HomeActivityController.kt | 18 + .../java/org/oppia/app/home/HomeFragment.kt | 23 + .../oppia/app/home/HomeFragmentController.kt | 37 + .../oppia/app/home/UserAppHistoryViewModel.kt | 33 + .../app/viewmodel/ViewModelBridgeFactory.kt | 27 + .../oppia/app/viewmodel/ViewModelProvider.kt | 17 + app/src/main/res/layout/home_activity.xml | 2 +- app/src/main/res/layout/home_fragment.xml | 2 +- .../java/org/oppia/app/HomeActivityTest.kt | 33 - .../org/oppia/app/home/HomeActivityTest.kt | 223 ++++ data/build.gradle | 62 ++ data/proguard-rules.pro | 21 + data/src/main/AndroidManifest.xml | 1 + .../data/persistence/PersistentCacheStore.kt | 222 ++++ .../persistence/PersistentCacheStoreTest.kt | 584 +++++++++++ domain/build.gradle | 32 +- .../oppia/domain/UserAppHistoryController.kt | 143 +-- .../domain/UserAppHistoryControllerTest.kt | 134 ++- model/build.gradle | 3 - model/src/main/proto/example.proto | 4 + settings.gradle | 2 +- utility/build.gradle | 37 +- .../org/oppia/util/data/AsyncDataSource.kt | 12 - .../util/data/AsyncDataSubscriptionManager.kt | 68 ++ .../java/org/oppia/util/data/AsyncResult.kt | 62 +- .../java/org/oppia/util/data/DataProvider.kt | 26 + .../java/org/oppia/util/data/DataProviders.kt | 241 +++++ .../oppia/util/data/InMemoryBlockingCache.kt | 142 +++ .../util/threading/BackgroundDispatcher.kt | 6 + .../util/threading/BlockingDispatcher.kt | 6 + .../util/threading/ConcurrentCollections.kt | 36 + .../oppia/util/threading/DispatcherModule.kt | 29 + .../org/oppia/util/data/AsyncResultTest.kt | 260 +++++ .../org/oppia/util/data/DataProvidersTest.kt | 956 ++++++++++++++++++ .../util/data/InMemoryBlockingCacheTest.kt | 798 +++++++++++++++ 55 files changed, 4370 insertions(+), 271 deletions(-) delete mode 100644 app/src/main/java/org/oppia/app/HomeActivity.kt delete mode 100644 app/src/main/java/org/oppia/app/HomeFragment.kt delete mode 100644 app/src/main/java/org/oppia/app/UserAppHistoryViewModel.kt create mode 100644 app/src/main/java/org/oppia/app/activity/ActivityComponent.kt create mode 100644 app/src/main/java/org/oppia/app/activity/ActivityModule.kt create mode 100644 app/src/main/java/org/oppia/app/activity/ActivityScope.kt create mode 100644 app/src/main/java/org/oppia/app/activity/InjectableAppCompatActivity.kt create mode 100644 app/src/main/java/org/oppia/app/application/ApplicationComponent.kt create mode 100644 app/src/main/java/org/oppia/app/application/ApplicationContext.kt create mode 100644 app/src/main/java/org/oppia/app/application/ApplicationModule.kt create mode 100644 app/src/main/java/org/oppia/app/application/OppiaApplication.kt create mode 100644 app/src/main/java/org/oppia/app/fragment/FragmentComponent.kt create mode 100644 app/src/main/java/org/oppia/app/fragment/FragmentScope.kt create mode 100644 app/src/main/java/org/oppia/app/fragment/InjectableFragment.kt create mode 100644 app/src/main/java/org/oppia/app/home/HomeActivity.kt create mode 100644 app/src/main/java/org/oppia/app/home/HomeActivityController.kt create mode 100644 app/src/main/java/org/oppia/app/home/HomeFragment.kt create mode 100644 app/src/main/java/org/oppia/app/home/HomeFragmentController.kt create mode 100644 app/src/main/java/org/oppia/app/home/UserAppHistoryViewModel.kt create mode 100644 app/src/main/java/org/oppia/app/viewmodel/ViewModelBridgeFactory.kt create mode 100644 app/src/main/java/org/oppia/app/viewmodel/ViewModelProvider.kt delete mode 100644 app/src/sharedTest/java/org/oppia/app/HomeActivityTest.kt create mode 100644 app/src/sharedTest/java/org/oppia/app/home/HomeActivityTest.kt create mode 100644 data/build.gradle create mode 100644 data/proguard-rules.pro create mode 100644 data/src/main/AndroidManifest.xml create mode 100644 data/src/main/java/org/oppia/data/persistence/PersistentCacheStore.kt create mode 100644 data/src/test/java/org/oppia/data/persistence/PersistentCacheStoreTest.kt delete mode 100644 utility/src/main/java/org/oppia/util/data/AsyncDataSource.kt create mode 100644 utility/src/main/java/org/oppia/util/data/AsyncDataSubscriptionManager.kt create mode 100644 utility/src/main/java/org/oppia/util/data/DataProvider.kt create mode 100644 utility/src/main/java/org/oppia/util/data/DataProviders.kt create mode 100644 utility/src/main/java/org/oppia/util/data/InMemoryBlockingCache.kt create mode 100644 utility/src/main/java/org/oppia/util/threading/BackgroundDispatcher.kt create mode 100644 utility/src/main/java/org/oppia/util/threading/BlockingDispatcher.kt create mode 100644 utility/src/main/java/org/oppia/util/threading/ConcurrentCollections.kt create mode 100644 utility/src/main/java/org/oppia/util/threading/DispatcherModule.kt create mode 100644 utility/src/test/java/org/oppia/util/data/DataProvidersTest.kt create mode 100644 utility/src/test/java/org/oppia/util/data/InMemoryBlockingCacheTest.kt diff --git a/.gitignore b/.gitignore index 4eb2f7645e2..c26873b6b36 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ /.idea/navEditor.xml /.idea/assetWizardSettings.xml app/build +data/build domain/build model/build utility/build diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index c26bd1cd7a5..ef06a053e64 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -13,7 +13,13 @@ + + + diff --git a/.idea/gradle.xml b/.idea/gradle.xml index 80feec202be..d456ac543b2 100644 --- a/.idea/gradle.xml +++ b/.idea/gradle.xml @@ -9,6 +9,7 @@