Search code examples
androiddagger-2kotlin-coroutinesandroid-viewmodel

Why is ViewModelScoped coroutine unusable after ViewModel onCleared() method called


I am sharing an ActivityScoped viewModel between multiple Fragments in my current Android application.

The viewModel employs Coroutine Scope viewModelScope.launch{}

My issue is the .launch{} only works until the owning ViewModel onCleared() method is called.

Is this how ViewModel scoped coroutines are supposed to work?

Is there an approach I can use to "Reset" the viewModelScope so that .launch{} works following the onCleared() method being called?

heres my code::

Fragment

RxSearchView.queryTextChangeEvents(search)
        .doOnSubscribe {
            compositeDisposable.add(it)
        }
        .throttleLast(300, TimeUnit.MILLISECONDS)
        .debounce(300, TimeUnit.MILLISECONDS)
        .map { event -> event.queryText().toString() }
        .observeOn(AndroidSchedulers.mainThread())
        .subscribe { charactersResponse ->
            launch {
                viewModel.search(charactersResponse.trim())
            }
        }

. . .

override fun onDetach() {
    super.onDetach()
    viewModel.cancelSearch()
    compositeDisposable.clear()
}

ViewModel

suspend fun search(searchString: String) {
    cancelSearch()

    if (TextUtils.isEmpty(searchString)) {
        return
    }

    job = viewModelScope.launch {
        repository.search(searchString)
    }
}

fun cancelSearch() {
    job?.cancelChildren()
}

. . .

override fun onCleared() {
    super.onCleared()
    repository.onCleared()
 }

What am I doing wrong?

UPDATE

If I amend my launch code to this

job = GlobalScope.launch {
    repository.search(searchString)
}

It solves my issue, however is this the only way to achieve my desired result?

I was under the impression GlobalScope was "Bad"


Solution

  • repository.onCleared()
    

    This method should not belong to the Repository.

    In fact, the Repository should not be stateful.

    If you check Google's samples, the Repository creates a LiveData that contains a Resource, and the reason why this is relevant is because the actual data loading and caching mechanic is inside this resource, triggered by LiveData.onActive (in this sample, MediatorLiveData.addSource, but technically that's semantically the same thing).

        .subscribe { charactersResponse ->
            launch {
                viewModel.search(charactersResponse.trim())
    

    The Fragment shouldn't be launching coroutines. It should say something like

    .subscribe {
        viewModel.updateSearchText(charactersResponse.trim())
    }
    

    and also

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
    
        viewModel = ViewModelProviders.of(this).get(MyViewModel::class.java, factory)
        viewModel.searchResults.observe(viewLifecycleOwner, Observer { results ->
            searchAdapter.submitList(results)
        })
    }
    

    Then ViewModel would

    class MyViewModel(
        private val repository: MyRepository
    ): ViewModel() {
        private val searchText = MutableLiveData<String>()
    
        fun updateSearchText(searchText: String) {
            this.searchText.value = searchText
        }
    
        val searchResults: LiveData<List<MyData>> = Transformations.switchMap(searchText) {
            repository.search(searchText)
        }
    }
    

    And that's all there should be in the ViewModel, so then the question of "who owns the coroutine scope"? That depends on when the task should be cancelled.

    If "no longer observing" should cancel the task, then it should be LiveData.onInactive() to cancel the task.

    If "no longer observing but not cleared" should retain the task, then ViewModel's onCleared should indeed govern a SupervisorJob inside the ViewModel that would be cancelled in onCleared(), and the search should be launched within that scope, which is probably only possible if you pass over the CoroutineScope to the search method.

    suspend fun search(scope: CoroutineScope, searchText: String): LiveData<List<T>> =
        scope.launch {
            withContext(Dispatchers.IO) { // or network or something
                val results = networkApi.fetchResults(searchText)
                withContext(Dispatchers.MAIN) {
                    MutableLiveData<List<MyData>>().apply { // WARNING: this should probably be replaced with switchMap over the searchText
                        this.value = results
                    }
                }
            }
        }
    

    Would this work? Not sure, I don't actually use coroutines, but I think it should. This example however doesn't handle the equivalent of switchMap-ing inside the LiveData, nor with coroutines.