Hector
Hector

Reputation: 5356

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"

Upvotes: 7

Views: 6367

Answers (2)

EpicPandaForce
EpicPandaForce

Reputation: 81549

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.

Upvotes: 6

CommonsWare
CommonsWare

Reputation: 1006944

following a cal to onCleared() my viewModelScoped cororoutine Launch stops executing

That's a feature, not a bug.

Once the ViewModel is cleared, you should not be doing anything in that ViewModel or whatever its LifecycleOwner was. All of that is now defunct and should no longer be used.

however is this the only way to achieve my desired result?

The correct solution is to get rid of the code from the ViewModel. If you are expecting some background work to go past the lifetime of an activity or fragment, then that code does not belong in the activity/fragment or its associated viewmodels. It belongs in something that has a matching lifetime to the work that you are trying to do.

Upvotes: 12

Related Questions