Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

persist presenters - fight against process death #53

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

passsy
Copy link
Contributor

@passsy passsy commented Jan 5, 2017

Early draft on how to persist Presenters

based on #50

Background

Presenters should be able persist their state so they can reuse their state after the process has been killed. Android provides a simple API (#onSaveInstanceState()) where Bundles can be saved and restored when a new Activity instance gets created (#onCreate(saveInstanceState)). This is already possible with a few lines of code and no further library APIs are required.

The onSaveInstanceState solution has some weaknesses:

  • The Presenter lives longer than the Activity and can manipulate its state after onSaveInstanceState() got called. The mutated state never gets saved.
  • Bundle is part of the Android SDK and cannot be used in unit tests (only mocked)

Solution

As a solution the Presenter gets a public method to save its current state persistState(). The state must be returned by TiPresenter#onSavePersistentState(): byte[].
persistState() can be called at any time; when onSaveInstanceState gets called, later or earlier even when no Activity instance is available.
This triggers a TiPresenterSerializer responsible for the persistence which can be exchanged with other implementations.
Unlike Activity#onCreate() the persistanceState will not be a parameter of TiPresenter#onCreate(). This would be a breaking API change and doesn't provide any value compared to a method getPersistentState(): byte[]

The default implementation of TiPresenterSerializer will store the state in a single file per Presenter in <appfolder>/cache/TiPresenterStates/. We cannot detect when a state isn't required anymore, therefore we cannot cleanup those files properly. Some other mechanism must be applied to prevent an always increasing cache folder. I recommend a fixed cache size by deleting the oldest states like OkHttp.

TODOs

  • Create a TiBundle and use it instead of byte[] returned by getPersistentState() for an easy way to save primitives by key and automatically write all into one byte[]. Androids Bundle throws DeadObjectException when created after the Activity is destroyed and cannot be used. May be also a problem for Android State Serializer #51
  • Implement cache cleanup
  • don't prefetch the persistentState at the first launch of a presenter
  • document all the things
  • make sure the default presenter serializer doesn't break unit tests

Open Questions

  • The default presenter serializer implementation requires a Context and cannot be statically initialized like TiConfiguration.DEFAULT right now. Maybe initialize it in a ContentProvider? Or set the default implementation from the hosting Activity/PresenterInstructor when nothing is defined in config?

@vRallev @k0shk0sh please review

vRallev and others added 3 commits December 19, 2016 19:44
…This is useful to recreate presenters after a process has died.
it works but the API doesn't look nice
@vRallev
Copy link
Contributor

vRallev commented Jan 6, 2017

A few comments to this approach

  • Please make this optional with an extra module. It's a rare use case and not all apps need that feature. With reflection you can find out if the default module was added and automatically setup the serializer if it's available. Especially using another implementation makes the default implementation useless.
  • I know that I recommended the TiBundle, but when I started with my implementation I noticed that it isn't required. The TiBundle would add a lot of overhead and is annoying to work with (all the put and get calls...). You can put this in the extra module for the default implementation, but I wouldn't put the class in the main module.
  • Make the serializer as transparent as possible. I don't think the presenter needs TiPresenter#onSavePersistentState(): byte[]. The serializer should handle this. Also I find getPersistentState(): byte[] unnecessary. The presenter shouldn't care about restoring its state, the serializer should do this and it's only required once after the presenter got recreated. A method protected boolean isRestored() inside the presenter could be helpful in some cases, I'll add this to my implementation. But besides this method and persistState() triggering the process nothing should be required in the presenter.
  • I haven't seen the DeadObjectException, yet, and I don't know how it relates to the activity. I don't think that's a problem. For the Bundle you don't need an activity (especially persisting the state in my implementation). Do you have some links?
  • Regarding your open question: Don't use a ContentProvider. This will add more problems IMO, especially if your app uses multiple processes. For the Context reference you could use: https://github.com/vRallev/app-context

@passsy
Copy link
Contributor Author

passsy commented May 5, 2017

Completely outdated after 0.8.0 release. Will keep it open for reference until a better solution was opened

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

Successfully merging this pull request may close these issues.

None yet

2 participants