From b3543e2f3f5b8c46788dd5691d4a2b9e77a011e2 Mon Sep 17 00:00:00 2001 From: karntrehan Date: Mon, 12 Feb 2018 19:54:35 +0530 Subject: [PATCH] [ENH] Tested ViewModel, local data source, improved injections --- .../posts/details/di/DetailsComponent.kt | 13 ++-- .../details/model/DetailsDataContract.kt | 5 +- .../posts/details/model/DetailsRepository.kt | 9 +-- .../details/viewmodel/DetailsViewModel.kt | 15 ++--- .../viewmodel/DetailsViewModelFactory.kt | 5 +- .../karntrehan/posts/list/di/ListComponent.kt | 9 ++- .../posts/list/model/ListDataContract.kt | 5 +- .../posts/list/model/ListRepository.kt | 13 ++-- .../posts/list/viewmodel/ListViewModel.kt | 15 ++--- .../list/viewmodel/ListViewModelFactory.kt | 5 +- .../posts/list/model/ListRepositoryTest.kt | 12 ++-- .../posts/list/viewmodel/ListViewModelTest.kt | 64 +++++++++++++++++++ 12 files changed, 122 insertions(+), 48 deletions(-) create mode 100644 posts/src/test/java/com/karntrehan/posts/list/viewmodel/ListViewModelTest.kt diff --git a/posts/src/main/java/com/karntrehan/posts/details/di/DetailsComponent.kt b/posts/src/main/java/com/karntrehan/posts/details/di/DetailsComponent.kt index 6fefce8..904ac0c 100644 --- a/posts/src/main/java/com/karntrehan/posts/details/di/DetailsComponent.kt +++ b/posts/src/main/java/com/karntrehan/posts/details/di/DetailsComponent.kt @@ -14,6 +14,7 @@ import com.karntrehan.posts.list.di.ListComponent import dagger.Component import dagger.Module import dagger.Provides +import io.reactivex.disposables.CompositeDisposable @DetailsScope @Component(dependencies = [ListComponent::class], modules = [DetailsModule::class]) @@ -34,15 +35,15 @@ class DetailsModule { /*ViewModel*/ @Provides @DetailsScope - fun detailsViewModelFactory(repo: DetailsDataContract.Repository): DetailsViewModelFactory { - return DetailsViewModelFactory(repo) + fun detailsViewModelFactory(repo: DetailsDataContract.Repository,compositeDisposable: CompositeDisposable): DetailsViewModelFactory { + return DetailsViewModelFactory(repo,compositeDisposable) } /*Repository*/ @Provides @DetailsScope - fun detailsRepo(local: DetailsDataContract.Local, remote: DetailsDataContract.Remote, scheduler: Scheduler) - : DetailsDataContract.Repository = DetailsRepository(local, remote, scheduler) + fun detailsRepo(local: DetailsDataContract.Local, remote: DetailsDataContract.Remote, scheduler: Scheduler,compositeDisposable: CompositeDisposable) + : DetailsDataContract.Repository = DetailsRepository(local, remote, scheduler,compositeDisposable) @Provides @DetailsScope @@ -51,4 +52,8 @@ class DetailsModule { @Provides @DetailsScope fun localData(postDb: PostDb, scheduler: Scheduler): DetailsDataContract.Local = DetailsLocalData(postDb, scheduler) + + @Provides + @DetailsScope + fun compositeDisposable(): CompositeDisposable = CompositeDisposable() } \ No newline at end of file diff --git a/posts/src/main/java/com/karntrehan/posts/details/model/DetailsDataContract.kt b/posts/src/main/java/com/karntrehan/posts/details/model/DetailsDataContract.kt index 9315791..4f29553 100644 --- a/posts/src/main/java/com/karntrehan/posts/details/model/DetailsDataContract.kt +++ b/posts/src/main/java/com/karntrehan/posts/details/model/DetailsDataContract.kt @@ -3,14 +3,13 @@ package com.karntrehan.posts.details.model import com.karntrehan.posts.commons.data.local.Comment import com.mpaani.core.networking.Outcome import io.reactivex.Flowable -import io.reactivex.disposables.CompositeDisposable import io.reactivex.subjects.PublishSubject interface DetailsDataContract { interface Repository { val commentsFetchOutcome: PublishSubject>> - fun fetchCommentsFor(postId: Int?, compositeDisposable: CompositeDisposable) - fun refreshComments(postId: Int, compositeDisposable: CompositeDisposable) + fun fetchCommentsFor(postId: Int?) + fun refreshComments(postId: Int) fun saveCommentsForPost(comments: List) fun handleError(error: Throwable) } diff --git a/posts/src/main/java/com/karntrehan/posts/details/model/DetailsRepository.kt b/posts/src/main/java/com/karntrehan/posts/details/model/DetailsRepository.kt index 8a0e785..edd1b2c 100644 --- a/posts/src/main/java/com/karntrehan/posts/details/model/DetailsRepository.kt +++ b/posts/src/main/java/com/karntrehan/posts/details/model/DetailsRepository.kt @@ -10,13 +10,14 @@ import io.reactivex.subjects.PublishSubject class DetailsRepository(private val local: DetailsDataContract.Local, private val remote: DetailsDataContract.Remote, - private val scheduler: Scheduler) : DetailsDataContract.Repository { + private val scheduler: Scheduler, + private val compositeDisposable: CompositeDisposable) : DetailsDataContract.Repository { override val commentsFetchOutcome: PublishSubject>> = PublishSubject.create>>() var remoteFetch = true - override fun fetchCommentsFor(postId: Int?, compositeDisposable: CompositeDisposable) { + override fun fetchCommentsFor(postId: Int?) { if (postId == null) return @@ -26,13 +27,13 @@ class DetailsRepository(private val local: DetailsDataContract.Local, .subscribe({ retailers -> commentsFetchOutcome.success(retailers) if (remoteFetch) - refreshComments(postId, compositeDisposable) + refreshComments(postId) remoteFetch = false }, { error -> handleError(error) }) .addTo(compositeDisposable) } - override fun refreshComments(postId: Int, compositeDisposable: CompositeDisposable) { + override fun refreshComments(postId: Int) { commentsFetchOutcome.loading(true) remote.getCommentsForPost(postId) .performOnBackOutOnMain(scheduler) diff --git a/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModel.kt b/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModel.kt index f758e31..f2a2215 100644 --- a/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModel.kt +++ b/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModel.kt @@ -9,15 +9,19 @@ import com.karntrehan.posts.details.model.DetailsDataContract import com.mpaani.core.networking.Outcome import io.reactivex.disposables.CompositeDisposable -class DetailsViewModel(private val repo: DetailsDataContract.Repository) : ViewModel() { - private val compositeDisposable = CompositeDisposable() +class DetailsViewModel(private val repo: DetailsDataContract.Repository, private val compositeDisposable: CompositeDisposable) : ViewModel() { val commentsOutcome: LiveData>> by lazy { repo.commentsFetchOutcome.toLiveData(compositeDisposable) } fun loadCommentsFor(postId: Int?) { - repo.fetchCommentsFor(postId, compositeDisposable) + repo.fetchCommentsFor(postId) + } + + fun refreshCommentsFor(postId: Int?) { + if (postId != null) + repo.refreshComments(postId) } override fun onCleared() { @@ -26,9 +30,4 @@ class DetailsViewModel(private val repo: DetailsDataContract.Repository) : ViewM compositeDisposable.clear() PostDH.destroyDetailsComponent() } - - fun refreshCommentsFor(postId: Int?) { - if (postId != null) - repo.refreshComments(postId, compositeDisposable) - } } \ No newline at end of file diff --git a/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModelFactory.kt b/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModelFactory.kt index 1938615..3b9393b 100644 --- a/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModelFactory.kt +++ b/posts/src/main/java/com/karntrehan/posts/details/viewmodel/DetailsViewModelFactory.kt @@ -3,11 +3,12 @@ package com.karntrehan.posts.details.viewmodel import android.arch.lifecycle.ViewModel import android.arch.lifecycle.ViewModelProvider import com.karntrehan.posts.details.model.DetailsDataContract +import io.reactivex.disposables.CompositeDisposable @Suppress("UNCHECKED_CAST") -class DetailsViewModelFactory(private val repository: DetailsDataContract.Repository) : +class DetailsViewModelFactory(private val repository: DetailsDataContract.Repository, private val compositeDisposable: CompositeDisposable) : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return DetailsViewModel(repository) as T + return DetailsViewModel(repository,compositeDisposable) as T } } \ No newline at end of file diff --git a/posts/src/main/java/com/karntrehan/posts/list/di/ListComponent.kt b/posts/src/main/java/com/karntrehan/posts/list/di/ListComponent.kt index f78f969..9f4c682 100644 --- a/posts/src/main/java/com/karntrehan/posts/list/di/ListComponent.kt +++ b/posts/src/main/java/com/karntrehan/posts/list/di/ListComponent.kt @@ -18,6 +18,7 @@ import com.squareup.picasso.Picasso import dagger.Component import dagger.Module import dagger.Provides +import io.reactivex.disposables.CompositeDisposable import retrofit2.Retrofit @ListScope @@ -46,12 +47,12 @@ class ListModule { /*ViewModel*/ @Provides @ListScope - fun listViewModelFactory(repository: ListDataContract.Repository): ListViewModelFactory = ListViewModelFactory(repository) + fun listViewModelFactory(repository: ListDataContract.Repository,compositeDisposable: CompositeDisposable): ListViewModelFactory = ListViewModelFactory(repository,compositeDisposable) /*Repository*/ @Provides @ListScope - fun listRepo(local: ListDataContract.Local, remote: ListDataContract.Remote, scheduler: Scheduler): ListDataContract.Repository = ListRepository(local, remote, scheduler) + fun listRepo(local: ListDataContract.Local, remote: ListDataContract.Remote, scheduler: Scheduler, compositeDisposable: CompositeDisposable): ListDataContract.Repository = ListRepository(local, remote, scheduler, compositeDisposable) @Provides @ListScope @@ -61,6 +62,10 @@ class ListModule { @ListScope fun localData(postDb: PostDb, scheduler: Scheduler): ListDataContract.Local = ListLocalData(postDb, scheduler) + @Provides + @ListScope + fun compositeDisposable(): CompositeDisposable = CompositeDisposable() + /*Parent providers to dependents*/ @Provides @ListScope diff --git a/posts/src/main/java/com/karntrehan/posts/list/model/ListDataContract.kt b/posts/src/main/java/com/karntrehan/posts/list/model/ListDataContract.kt index 923b5f1..a8f96eb 100644 --- a/posts/src/main/java/com/karntrehan/posts/list/model/ListDataContract.kt +++ b/posts/src/main/java/com/karntrehan/posts/list/model/ListDataContract.kt @@ -5,14 +5,13 @@ import com.karntrehan.posts.commons.data.local.Post import com.karntrehan.posts.commons.data.local.User import com.mpaani.core.networking.Outcome import io.reactivex.Flowable -import io.reactivex.disposables.CompositeDisposable import io.reactivex.subjects.PublishSubject interface ListDataContract { interface Repository { val postFetchOutcome: PublishSubject>> - fun fetchPosts(compositeDisposable: CompositeDisposable) - fun refreshPosts(compositeDisposable: CompositeDisposable) + fun fetchPosts() + fun refreshPosts() fun saveUsersAndPosts(users: List, posts: List) fun handleError(error: Throwable) } diff --git a/posts/src/main/java/com/karntrehan/posts/list/model/ListRepository.kt b/posts/src/main/java/com/karntrehan/posts/list/model/ListRepository.kt index 1060883..0d766a2 100644 --- a/posts/src/main/java/com/karntrehan/posts/list/model/ListRepository.kt +++ b/posts/src/main/java/com/karntrehan/posts/list/model/ListRepository.kt @@ -11,17 +11,18 @@ import io.reactivex.disposables.CompositeDisposable import io.reactivex.functions.BiFunction import io.reactivex.subjects.PublishSubject + class ListRepository(private val local: ListDataContract.Local, private val remote: ListDataContract.Remote, - private val scheduler: Scheduler) : ListDataContract.Repository { + private val scheduler: Scheduler, + private val compositeDisposable: CompositeDisposable) : ListDataContract.Repository { - override val postFetchOutcome: PublishSubject>> - = PublishSubject.create>>() + override val postFetchOutcome: PublishSubject>> = PublishSubject.create>>() //Need to perform a remoteFetch or not? var remoteFetch = true - override fun fetchPosts(compositeDisposable: CompositeDisposable) { + override fun fetchPosts() { postFetchOutcome.loading(true) //Observe changes to the db local.getPostsWithUsers() @@ -29,13 +30,13 @@ class ListRepository(private val local: ListDataContract.Local, .subscribe({ retailers -> postFetchOutcome.success(retailers) if (remoteFetch) - refreshPosts(compositeDisposable) + refreshPosts() remoteFetch = false }, { error -> handleError(error) }) .addTo(compositeDisposable) } - override fun refreshPosts(compositeDisposable: CompositeDisposable) { + override fun refreshPosts() { postFetchOutcome.loading(true) Flowable.zip( remote.getUsers(), diff --git a/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModel.kt b/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModel.kt index 4cb7731..af62342 100644 --- a/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModel.kt +++ b/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModel.kt @@ -9,22 +9,21 @@ import com.karntrehan.posts.list.model.ListDataContract import com.mpaani.core.networking.Outcome import io.reactivex.disposables.CompositeDisposable -class ListViewModel(private val repo: ListDataContract.Repository) : ViewModel() { - - private val compositeDisposable = CompositeDisposable() +class ListViewModel(private val repo: ListDataContract.Repository, + private val compositeDisposable: CompositeDisposable) : ViewModel() { val postsOutcome: LiveData>> by lazy { //Convert publish subject to livedata repo.postFetchOutcome.toLiveData(compositeDisposable) } - fun refreshPosts() { - repo.refreshPosts(compositeDisposable) - } - fun getPosts() { if (postsOutcome.value == null) - repo.fetchPosts(compositeDisposable) + repo.fetchPosts() + } + + fun refreshPosts() { + repo.refreshPosts() } override fun onCleared() { diff --git a/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModelFactory.kt b/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModelFactory.kt index c92785a..9f87628 100644 --- a/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModelFactory.kt +++ b/posts/src/main/java/com/karntrehan/posts/list/viewmodel/ListViewModelFactory.kt @@ -3,11 +3,12 @@ package com.karntrehan.posts.list.viewmodel import android.arch.lifecycle.ViewModel import android.arch.lifecycle.ViewModelProvider import com.karntrehan.posts.list.model.ListDataContract +import io.reactivex.disposables.CompositeDisposable @Suppress("UNCHECKED_CAST") -class ListViewModelFactory(private val repository: ListDataContract.Repository) : +class ListViewModelFactory(private val repository: ListDataContract.Repository, private val compositeDisposable: CompositeDisposable) : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return ListViewModel(repository) as T + return ListViewModel(repository, compositeDisposable) as T } } \ No newline at end of file diff --git a/posts/src/test/java/com/karntrehan/posts/list/model/ListRepositoryTest.kt b/posts/src/test/java/com/karntrehan/posts/list/model/ListRepositoryTest.kt index d187488..cb358b4 100644 --- a/posts/src/test/java/com/karntrehan/posts/list/model/ListRepositoryTest.kt +++ b/posts/src/test/java/com/karntrehan/posts/list/model/ListRepositoryTest.kt @@ -27,7 +27,7 @@ class ListRepositoryTest { @Before fun init() { - repository = ListRepository(local, remote, TestScheduler()) + repository = ListRepository(local, remote, TestScheduler(), compositeDisposable) whenever(local.getPostsWithUsers()).doReturn(Flowable.just(emptyList())) whenever(remote.getUsers()).doReturn(Flowable.just(emptyList())) whenever(remote.getPosts()).doReturn(Flowable.just(emptyList())) @@ -47,7 +47,7 @@ class ListRepositoryTest { repository.postFetchOutcome.subscribe(obs) obs.assertEmpty() - repository.fetchPosts(compositeDisposable) + repository.fetchPosts() verify(local).getPostsWithUsers() obs.assertValueAt(0, Outcome.loading(true)) @@ -62,7 +62,7 @@ class ListRepositoryTest { @Test fun testFirstFetchPostsTriggersRemote() { repository.remoteFetch = true - repository.fetchPosts(compositeDisposable) + repository.fetchPosts() verify(remote).getPosts() verify(remote).getUsers() } @@ -75,7 +75,7 @@ class ListRepositoryTest { @Test fun testSubsequentFetchPostsNeverTriggersRemote() { repository.remoteFetch = false - repository.fetchPosts(compositeDisposable) + repository.fetchPosts() verify(remote, never()).getPosts() verify(remote, never()).getUsers() } @@ -91,7 +91,7 @@ class ListRepositoryTest { whenever(remote.getUsers()).doReturn(Flowable.just(dummyUsers)) whenever(remote.getPosts()).doReturn(Flowable.just(dummyPosts)) - repository.refreshPosts(compositeDisposable) + repository.refreshPosts() verify(local).saveUsersAndPosts(dummyUsers, dummyPosts) } @@ -107,7 +107,7 @@ class ListRepositoryTest { val obs = TestObserver>>() repository.postFetchOutcome.subscribe(obs) - repository.refreshPosts(compositeDisposable) + repository.refreshPosts() obs.assertValueAt(0, Outcome.loading(true)) obs.assertValueAt(1, Outcome.loading(false)) diff --git a/posts/src/test/java/com/karntrehan/posts/list/viewmodel/ListViewModelTest.kt b/posts/src/test/java/com/karntrehan/posts/list/viewmodel/ListViewModelTest.kt new file mode 100644 index 0000000..3c24a60 --- /dev/null +++ b/posts/src/test/java/com/karntrehan/posts/list/viewmodel/ListViewModelTest.kt @@ -0,0 +1,64 @@ +package com.karntrehan.posts.list.viewmodel + +import android.arch.lifecycle.Observer +import com.karntrehan.posts.commons.DummyData +import com.karntrehan.posts.commons.data.PostWithUser +import com.karntrehan.posts.list.model.ListDataContract +import com.mpaani.core.networking.Outcome +import com.nhaarman.mockito_kotlin.doReturn +import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.whenever +import io.reactivex.disposables.CompositeDisposable +import io.reactivex.subjects.PublishSubject +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.IOException + +@RunWith(RobolectricTestRunner::class) +class ListViewModelTest { + + private lateinit var viewModel: ListViewModel + + private val repo: ListDataContract.Repository = mock() + + private val outcome: Observer>> = mock() + + @Before + fun init() { + viewModel = ListViewModel(repo, CompositeDisposable()) + whenever(repo.postFetchOutcome).doReturn(PublishSubject.create()) + viewModel.postsOutcome.observeForever(outcome) + } + + @Test + fun testGetPostsSuccess() { + viewModel.getPosts() + verify(repo).fetchPosts() + + repo.postFetchOutcome.onNext(Outcome.loading(true)) + verify(outcome).onChanged(Outcome.loading(true)) + + repo.postFetchOutcome.onNext(Outcome.loading(false)) + verify(outcome).onChanged(Outcome.loading(false)) + + val data = listOf(DummyData.PostWithUser(1), DummyData.PostWithUser(2)) + repo.postFetchOutcome.onNext(Outcome.success(data)) + verify(outcome).onChanged(Outcome.success(data)) + } + + @Test + fun testGetPostsError() { + val exception = IOException() + repo.postFetchOutcome.onNext(Outcome.failure(exception)) + verify(outcome).onChanged(Outcome.failure(exception)) + } + + @Test + fun testRefreshPosts() { + viewModel.refreshPosts() + verify(repo).refreshPosts() + } +} \ No newline at end of file