airbnb / mavericks

Mavericks: Android on Autopilot
https://airbnb.io/mavericks/
Apache License 2.0
5.85k stars 499 forks source link

How to handle the result of Async when the ViewModel is destroyed #613

Open Kilnn opened 2 years ago

Kilnn commented 2 years ago

A simple usage scenario, I have a page to modify the password. User enters old and new password and submits request, like this:

class ModifyPwdViewModel {

    fun modifyPwd(oldPassword: String, newPassword: String) {
        suspend {
            modifyPwdUseCase.get().invoke(ModifyPwdUseCase.Params(oldPassword, newPassword))
        }.execute {
            copy(async = it)
        }
    }
}

And observer the result in fragment, like this:

class ModifyPwdFragment : Fragment, MavericksView {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        viewModel.onAsync(
            SingleAsyncState::async,
            uniqueOnly(),
            onFail = {
                toast.showFailed(it)
            },
            onSuccess = {
                findNavController().popBackStack()
            }
        )
    }
}

Normally, it works fine. But when I'm requesting, exit the APP to the background. If after a while, the Activity is destroyed and rebuilt, when I enter again, I will lose the result of the last request.

In this example, after exiting to the background, the ViewModel continued to work and successfully changed the password. When I go back again, it still stays on this fragment, which makes the user think that the last password change was unsuccessful.

So , how can I handle this scenario? Or, is there an easy way to persist this Async Success state and let me know that the last request was successful.

gpeal commented 2 years ago

You can use @PersistState for this. @PersistState doesn't work for Async values, however because saving and restoring Loading could lead to infinite hangs if the app decides to not restart a request because the state is Loading even though it was merely from a state restoration.

You could add a new value @PersistState complete: Boolean = false and change your execute to copy(async = it, success = it is Success and then pop the back stack when success becomes true.

https://airbnb.io/mavericks/#/persist-state

Kilnn commented 2 years ago

Something weird is happening, @PersistState doesn't work properly. It seems that the data submitted by setState { copy(complete = true) } cannot be saved after the ViewModel exits the background.

I've tested it many times and don't know why.

fun modifyPwd(oldPassword: String, newPassword: String) {
    suspend {
        modifyPwdUseCase.get().invoke(ModifyPwdUseCase.Params(oldPassword, newPassword))
    }.execute {
        if (it is Success) {
            copy(async = it, complete = true)
        } else {
            copy(async = it)
        }
    }
}
gpeal commented 2 years ago

You can simplify this to

fun modifyPwd(oldPassword: String, newPassword: String) {
    suspend {
        modifyPwdUseCase.get().invoke(ModifyPwdUseCase.Params(oldPassword, newPassword))
    }.execute { copy(async = it, complete = is is Success) }
}

And make sure to actually annotate your state property with @PersistState. If this doesn't work, there may be something specific to your application code.

Kilnn commented 2 years ago

I'm sure the PersistState is added.

data class SingleAsyncState(
    val async: Async<Unit> = Uninitialized,
    @PersistState val complete: Boolean = false
) : MavericksState

There seems to be nothing special about my code, I added some logs to monitor it

class ModifyPwdViewModel @AssistedInject constructor(
    @Assisted initState: SingleAsyncState
) : MavericksViewModel<SingleAsyncState>(initState) {

    init {
        viewModelScope.launch {
            //collect state to let me know that modifyPwd is done
            stateFlow.collect {
                Log.e("Kilnn", "collect state:" + it)
            }
        }
    }

    override fun onCleared() {
        super.onCleared()
        //print a log let me know that ViewModel destroyed
        Log.e("Kilnn", "ModifyPwdViewModel onCleared")
    }

    fun modifyPwd(oldPassword: String, newPassword: String) {
        suspend {
            delay(5000)
        }.execute {
            copy(async = it, complete = it is Success)
        }
    }
}

Maybe it has something to do with my testing process:

  1. In my app fragment , click a button and execute modifyPwd.
  2. Exit to the background and wait for modifyPwd completed. I can see the log Kilnn: collect state:SingleAsyncState(async=Success(value=kotlin.Unit), complete=true)
  3. Open System Settings->Developer options,turn on Don't keep activities. Than, the ViewModel destroyed, I can see the log Kilnn: ModifyPwdViewModel onCleared
  4. Reopen my app, the Activity/Fragment recreate, and the complete print false in fragment invalidate method
gpeal commented 2 years ago

@Kilnn It's pretty hard to help without a complete repro app. Could you make one?

Kilnn commented 2 years ago

@gpeal I have create a Sample here. It has only one page to reproduce the problem flow by my test step:

  1. In my app fragment , click a button and execute modifyPwd.
  2. Exit to the background and wait for modifyPwd completed. I can see the log Kilnn: collect state:SingleAsyncState(async=Success(value=kotlin.Unit), complete=true)
  3. Open System Settings->Developer options,turn on Don't keep activities. Than, the ViewModel destroyed, I can see the log Kilnn: ModifyPwdViewModel onCleared
  4. Reopen my app, the Activity/Fragment recreate, and the complete print false in fragment invalidate method
elihart commented 2 years ago

My guess is that something in the hilt setup is getting in the way of the normal state restoration from bundle. if you set some breakpoints in that flow you should be able to see what is going on and why it may not be working

Kilnn commented 2 years ago

@elihart I tried as you suggested. The result does match the reason I guessed: The data submitted by setState cannot be saved after the ViewModel exits the background. Because the MavericksViewModelProvider save Bundle data with the following code:

viewModelContext.savedStateRegistry.registerSavedStateProvider(key) {
                viewModel.viewModel.getSavedStateBundle(
                    restoredContext.args,
                    stateRestorer?.viewModelClass ?: viewModelClass,
                    stateRestorer?.stateClass ?: stateClass
                )
            }

The SavedStateRegistry#registerSavedStateProvider only trigger with Activity/Fragment lifecycle change. So it can only save the data when Fragment.onPause. Submitted data does not trigger a save after staying in the background for a few seconds.

Perhaps there could be a public API that would allow us to manually save when we see fit, such as when PersistState data changes.

elihart commented 2 years ago

It's part of the activity/fragment contract that you are not allowed to save any additional state after onPause. Afaik android does not expose any API to do so after onSaveInstanceState.

if you have work being done in your viewmodel asynchronously that started while resumed and finished after paused so that it was not saved you may be forced to do something like runBlocking { vm.awaitState() } in your fragment's onPause callback