Provide a generic interface to serialize and deserialize presenters. #50
Conversation
…This is useful to recreate presenters after a process has died.
|
I know that you haven't decided what to do with #39, yet. But this would be a good compromise in my opinion. If we want to use your library in our app, then we need a solution for this issue. And I'd hate to fork it. |
| @@ -52,7 +52,7 @@ public TiPresenter recover(final String id) { | |||
| } | |||
|
|
|||
| public String safe(@NonNull final TiPresenter presenter) { | |||
| final String id = generateId(presenter); | |||
| final String id = presenter.getId(); | |||
| TiLog.v(TAG, "safe presenter with id " + id + " " + presenter); | |||
There was a problem hiding this comment.
Why is it better to have the presenter generate the id?
There was a problem hiding this comment.
Because it's the presenter's ID. Otherwise it would be an ID for a presenter used by the PresenterSavior. I'm using the ID at other places.
| * a rare use case the default value is {@code null} and presenters aren't restored. | ||
| * @param serializer An implementation which can serialize and deserialize any presenter. | ||
| */ | ||
| public Builder setPresenterSerializer(TiPresenterSerializer serializer) { |
There was a problem hiding this comment.
Having the serializer as part of the configuration is interesting, I'll have to think about it
There was a problem hiding this comment.
That was the ideal place for me. It's pretty handy with the global config and optional for tests.
…resenter # Conflicts: # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/TiConfiguration.java # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/internal/PresenterSavior.java # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/internal/TiActivityDelegate.java # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/internal/TiFragmentDelegate.java
…resenter # Conflicts: # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/internal/PresenterSavior.java # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/internal/TiActivityDelegate.java # thirtyinch/src/main/java/net/grandcentrix/thirtyinch/internal/TiFragmentDelegate.java
| * a {@link TiPresenter} should be restored, after a process has died, but the UI component is | ||
| * being recreated. | ||
| */ | ||
| public interface TiPresenterSerializer { |
There was a problem hiding this comment.
@passsy After reviewing all the changes in the 0.8.0 version I think this interface can be removed and the new TiPresenterSavior could be used instead. After that it would be possible for me to release the module in #51 separately and I could finally stop working on the fork.
The only changes that would be necessary to make this work is to add the TiPresenterSavior as argument in the TiConfiguration. This way I get a chance to replace the default savior.
The 2nd change that's necessary is to modify the recover method in the savior interface TiPresenter recover(String presenterId, @NonNull Object host);. Here a third parameter would be necessary that you've already implemented: TiPresenterProvider. In most cases that would be the TiFragment or TiActivity. That would give me the option to construct a new presenter in case it can be recovered after the process died.
Would that be possible?
There was a problem hiding this comment.
I think both changes are safe, because nothing of that was exposed to the public API, yet. Do you want me to create a different PR?
There was a problem hiding this comment.
Dang, I just realized that it's not necessary to make any changes in the config class, because in order to access the config, you need to create a presenter first.
There's no way to replace the savior in the delegate from TiFragment or TiActivity at the moment. I think it's better to add a new provider interface for the savior and the default implementation delegate to the PresenterSavior. Although a global config would be nicer.
There was a problem hiding this comment.
I'll look into presenter serialisation for the next release and we'll find a way to provide you an API.
The PresenterSavior is the new heart of this library. I don't think the savior will be replaceable with a third party impl because small mistakes could lead to major leaks. Instead I'm thinking about APIs for the savior to control the presenter store. Haven't thought this through completely but you get the direction.
|
I still don't get why a new API is required. I implemented saving presenter state with see master...feature/save_state This implementation could be moved into a What is missing? |
|
This would be tied to the activity lifecycle again. That's not what you want in case a longer operation is going on. Then there wouldn't be a good hook to restore the presenter. |
This is useful to recreate presenters after a process has died.