Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent state with MVI - no easy way to pass the saved State to presenter after activity died #27

Open
agta1991 opened this issue Jul 17, 2017 · 7 comments

Comments

@agta1991
Copy link

Hi!

I'm currently trying to implement save and restore instance state with Conductor and MVI architecture, like described in Mosby MVI Part 6 - Persistent State. All went well until I faced what happens if the activity dies (example onLowMemory).

With conductor I'm able to reconstruct the backstack, but I can't prepare the presenter with the initial state, because bindIntents run in postCreate cycle, and the Controller only calls restoreInstanceState after postCreate finished. I dived into the code of the Controller class in Conductor and find a hacky workaround for it, but sadly as I said its a hack....

If you pass the last ViewState in the controller's saveInstanceState function to the Controllers original args, it will be persisted if the activity dies. So after the new Activity is created, when the Conductor lib restores the backstack, the Controller will be created with the last ViewState in its' args. With this implementation I can push the last State down to the presenter when createPresenter is called, because it's present in the Controller's constructor.

Do you have a better solution for this problem?

Thanks,
Agta

@sockeqwe
Copy link
Owner

Hi,
I have to dig deeper into the lifecycles of conductor. Maybe there are better suited lifecycle callbacks.
However, from what I have heard about Conductor 3.0 there is a chance that onCreate(Bundle) will be introduced along with another callback (I can remember the name, but something like onAttachedToActivity()) that can be used for dependency injection. Probably this will fix this kind of issue too.

Unfortunately, I'm a little bit busy right now but I hope to take a look at this in first week of August.

@sevar83
Copy link

sevar83 commented Jul 27, 2017

Hi, I'm also affected by this issue. @agta1991, could you post a snippet of your workaround please?

@larten
Copy link

larten commented Jul 27, 2017

agta1991 is AFK, but here is the current solution:

abstract class BaseController<V : ViewContract<VS>, VS : ViewState, P : MviPresenter<V, VS>>(bundle: Bundle = Bundle()) :
        MviController<V, P>(bundle), ViewContract<VS> {

    protected val KEY_VIEW_STATE = "MviController.viewState"

    private var lastViewState: VS? = null

    abstract fun inflateView(inflater: LayoutInflater, container: ViewGroup): View

    abstract fun createPresenter(initialViewState: VS): P

    abstract fun getInitialViewState(): VS

    @CallSuper
    override fun render(viewState: VS) {
        this.lastViewState = viewState
    }

    open fun onPostCreateView(view: View) {}

    override fun createPresenter(): P {
        if (args.containsKey(KEY_VIEW_STATE)) {
            return createPresenter(args.getParcelable<Parcelable>(KEY_VIEW_STATE) as VS)
        }
        return createPresenter(getInitialViewState())
    }

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup): View {
        val view = inflateView(inflater, container)
        onPostCreateView(view)
        return view
    }

    override fun onSaveInstanceState(outState: Bundle) {
        if (lastViewState != null) {
            args.putParcelable(KEY_VIEW_STATE, lastViewState as Parcelable)
        }
    }
}

@sockeqwe
Copy link
Owner

You can always workaround such issues with a

public Observable<MyState> restorePersitentStateIntent()

And then read the retained state from Bundle and emit it through that intent to your business logic. The only thing you have to watch out os that the restore intent should be the first intent of all your intents that emits something

@sevar83
Copy link

sevar83 commented Jul 27, 2017

Thank you guys. @larten, do you have injection in the sub-classes and do you make any use of the restoration callbacks (onRestoreViewState, onRestoreInstanceState) ?

p.s. Beware of the Parcelable VS. My older implementation used Parcelable ViewStates and it's quite easy to hit the memory limit which is only ~500K. Esp. if you have growing lists of items. Better store the minimal amount of information enough to restore the state.

@larten
Copy link

larten commented Sep 25, 2017

Currenly I left this solution, because, as you wrote, it's easy to eat the full memory :)

@sevar83
Copy link

sevar83 commented Sep 25, 2017

Currenly I left this solution, because, as you wrote, it's easy to eat the full memory :)

@larten Lol, and I came back to the Parcelable ViewState but I'm now very cautious. It's easy to avoid OOM if you keep the danger always in mind. I've made the growing lists of paged items transient fields and I'm safe now. But this requires other mechanisms to repopulate the state on demand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants