Skip to content

Commit

Permalink
Fix #4, #70, #71, #86, #87: Introduce data module (#85)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
BenHenning authored and rt4914 committed Sep 3, 2019
1 parent 03424b2 commit bda2f7c
Show file tree
Hide file tree
Showing 55 changed files with 4,370 additions and 271 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
/.idea/navEditor.xml
/.idea/assetWizardSettings.xml
app/build
data/build
domain/build
model/build
utility/build
Expand Down
6 changes: 6 additions & 0 deletions .idea/codeStyles/Project.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .idea/gradle.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 27 additions & 2 deletions app/build.gradle
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
apply plugin: 'com.android.application'
apply plugin: 'kotlin-android'
apply plugin: 'kotlin-android-extensions'
apply plugin: 'kotlin-kapt'

android {
compileSdkVersion 29
compileSdkVersion 28
buildToolsVersion "29.0.1"
defaultConfig {
applicationId "org.oppia.app"
minSdkVersion 16
minSdkVersion 19
targetSdkVersion 28
versionCode 1
versionName "1.0"
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
// https://developer.android.com/training/testing/junit-runner#ato-gradle
testInstrumentationRunnerArguments clearPackageData: 'true'
}
compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}
kotlinOptions {
jvmTarget = JavaVersion.VERSION_1_8
}
buildTypes {
release {
Expand All @@ -23,6 +33,8 @@ android {
enabled = true
}
testOptions {
// https://developer.android.com/training/testing/junit-runner#ato-gradle
execution 'ANDROIDX_TEST_ORCHESTRATOR'
unitTests {
includeAndroidResources = true
}
Expand All @@ -49,6 +61,7 @@ dependencies {
'androidx.constraintlayout:constraintlayout:1.1.3',
'androidx.core:core-ktx:1.0.2',
'androidx.lifecycle:lifecycle-runtime-ktx:2.2.0-alpha03',
'com.google.dagger:dagger:2.24',
"org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version",
)
testImplementation(
Expand All @@ -65,6 +78,18 @@ dependencies {
'androidx.test:runner:1.2.0',
'com.google.truth:truth:0.43',
)
androidTestUtil(
'androidx.test:orchestrator:1.2.0',
)
kapt(
'com.google.dagger:dagger-compiler:2.24'
)
kaptTest(
'com.google.dagger:dagger-compiler:2.24'
)
kaptAndroidTest(
'com.google.dagger:dagger-compiler:2.24'
)
implementation project(":model")
implementation project(":domain")
implementation project(":utility")
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
package="org.oppia.app">

<application
android:name=".application.OppiaApplication"
android:allowBackup="true"
android:icon="@mipmap/ic_launcher"
android:label="@string/app_name"
android:roundIcon="@mipmap/ic_launcher_round"
android:supportsRtl="true"
android:theme="@style/OppiaTheme">
<activity android:name="org.oppia.app.HomeActivity">
<activity android:name="org.oppia.app.home.HomeActivity">
<intent-filter>
<action android:name="android.intent.action.MAIN"/>
<category android:name="android.intent.category.LAUNCHER"/>
Expand Down
14 changes: 0 additions & 14 deletions app/src/main/java/org/oppia/app/HomeActivity.kt

This file was deleted.

55 changes: 0 additions & 55 deletions app/src/main/java/org/oppia/app/HomeFragment.kt

This file was deleted.

10 changes: 0 additions & 10 deletions app/src/main/java/org/oppia/app/UserAppHistoryViewModel.kt

This file was deleted.

23 changes: 23 additions & 0 deletions app/src/main/java/org/oppia/app/activity/ActivityComponent.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.oppia.app.activity

import androidx.appcompat.app.AppCompatActivity
import dagger.BindsInstance
import dagger.Subcomponent
import org.oppia.app.fragment.FragmentComponent
import org.oppia.app.home.HomeActivity
import javax.inject.Provider

/** Root subcomponent for all activities. */
@Subcomponent(modules = [ActivityModule::class])
@ActivityScope
interface ActivityComponent {
@Subcomponent.Builder
interface Builder {
@BindsInstance fun setActivity(appCompatActivity: AppCompatActivity): Builder
fun build(): ActivityComponent
}

fun getFragmentComponentBuilderProvider(): Provider<FragmentComponent.Builder>

fun inject(homeActivity: HomeActivity)
}
7 changes: 7 additions & 0 deletions app/src/main/java/org/oppia/app/activity/ActivityModule.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.oppia.app.activity

import dagger.Module
import org.oppia.app.fragment.FragmentComponent

/** Root activity module. */
@Module(subcomponents = [FragmentComponent::class]) class ActivityModule
6 changes: 6 additions & 0 deletions app/src/main/java/org/oppia/app/activity/ActivityScope.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.oppia.app.activity

import javax.inject.Scope

/** A custom scope corresponding to dependencies that should be recreated for each activity. */
@Scope annotation class ActivityScope
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.oppia.app.activity

import android.os.Bundle
import android.os.PersistableBundle
import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.Fragment
import org.oppia.app.application.OppiaApplication
import org.oppia.app.fragment.FragmentComponent

/**
* An [AppCompatActivity] that facilitates field injection to child activities and constituent fragments that extend
* [org.oppia.app.fragment.InjectableFragment].
*/
abstract class InjectableAppCompatActivity: AppCompatActivity() {
/**
* The [ActivityComponent] corresponding to this activity. This cannot be used before [onCreate] is called, and can be
* used to inject lateinit fields in child activities during activity creation (which is recommended to be done in an
* override of [onCreate]).
*/
lateinit var activityComponent: ActivityComponent

override fun onCreate(savedInstanceState: Bundle?) {
// Note that the activity component must be initialized before onCreate() since it's possible for onCreate() to
// synchronously attach fragments (e.g. during a configuration change), which requires the activity component for
// createFragmentComponent(). This means downstream dependencies should not perform any major operations to the
// injected activity since it's not yet fully created.
initializeActivityComponent()
super.onCreate(savedInstanceState)
}

override fun onCreate(savedInstanceState: Bundle?, persistentState: PersistableBundle?) {
super.onCreate(savedInstanceState, persistentState)
initializeActivityComponent()
}

private fun initializeActivityComponent() {
activityComponent = (application as OppiaApplication).createActivityComponent(this)
}

fun createFragmentComponent(fragment: Fragment): FragmentComponent {
return activityComponent.getFragmentComponentBuilderProvider().get().setFragment(fragment).build()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.oppia.app.application

import android.app.Application
import dagger.BindsInstance
import dagger.Component
import org.oppia.app.activity.ActivityComponent
import org.oppia.util.threading.DispatcherModule
import javax.inject.Provider
import javax.inject.Singleton

/** Root Dagger component for the application. All application-scoped modules should be included in this component. */
@Singleton
@Component(modules = [ApplicationModule::class, DispatcherModule::class])
interface ApplicationComponent {
@Component.Builder
interface Builder {
@BindsInstance fun setApplication(application: Application): Builder
fun build(): ApplicationComponent
}

fun getActivityComponentBuilderProvider(): Provider<ActivityComponent.Builder>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.oppia.app.application

import javax.inject.Qualifier

/** Qualifier for injecting the application context. */
@Qualifier annotation class ApplicationContext
26 changes: 26 additions & 0 deletions app/src/main/java/org/oppia/app/application/ApplicationModule.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.oppia.app.application

import android.app.Application
import android.content.Context
import dagger.Module
import dagger.Provides
import org.oppia.app.activity.ActivityComponent
import javax.inject.Singleton

/** Provides core infrastructure needed to support all other dependencies in the app. */
@Module(subcomponents = [ActivityComponent::class])
class ApplicationModule {
@Provides
@Singleton
@ApplicationContext
fun provideApplicationContext(application: Application): Context {
return application
}

// TODO(#59): Remove this provider once all modules have access to the @ApplicationContext qualifier.
@Provides
@Singleton
fun provideContext(@ApplicationContext context: Context): Context {
return context
}
}
23 changes: 23 additions & 0 deletions app/src/main/java/org/oppia/app/application/OppiaApplication.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.oppia.app.application

import android.app.Application
import androidx.appcompat.app.AppCompatActivity
import org.oppia.app.activity.ActivityComponent

/** The root [Application] of the Oppia app. */
class OppiaApplication: Application() {
/** The root [ApplicationComponent]. */
private val component: ApplicationComponent by lazy {
DaggerApplicationComponent.builder()
.setApplication(this)
.build()
}

/**
* Returns a new [ActivityComponent] for the specified activity. This should only be used by
* [org.oppia.app.activity.InjectableAppCompatActivity].
*/
fun createActivityComponent(activity: AppCompatActivity): ActivityComponent {
return component.getActivityComponentBuilderProvider().get().setActivity(activity).build()
}
}
19 changes: 19 additions & 0 deletions app/src/main/java/org/oppia/app/fragment/FragmentComponent.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.oppia.app.fragment

import androidx.fragment.app.Fragment
import dagger.BindsInstance
import dagger.Subcomponent
import org.oppia.app.home.HomeFragment

/** Root subcomponent for all fragments. */
@Subcomponent
@FragmentScope
interface FragmentComponent {
@Subcomponent.Builder
interface Builder {
@BindsInstance fun setFragment(fragment: Fragment): Builder
fun build(): FragmentComponent
}

fun inject(homeFragment: HomeFragment)
}
6 changes: 6 additions & 0 deletions app/src/main/java/org/oppia/app/fragment/FragmentScope.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.oppia.app.fragment

import javax.inject.Scope

/** A custom scope corresponding to dependencies that should be recreated for each fragment. */
@Scope annotation class FragmentScope
Loading

0 comments on commit bda2f7c

Please sign in to comment.