Skip to content
This repository has been archived by the owner on May 23, 2020. It is now read-only.

Don't save null-value items #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

almozavr
Copy link

@almozavr almozavr commented Apr 1, 2015

Saving null-value item makes loader's callback trigger onNext with null as argument. So If you just do nothing, but went out of activity, it was killed, you come back and oops, your onNext was triggered just like it had result, but it doesn't.
E.g.

@Override protected void onLoad(Bundle savedInstanceState) {
            RxLoaderManager loaderManager = RxLoaderManager.get(activityHolder.getActivity());
            loginLoader = loaderManager.create((LoginParams params) -> {
                        return sessionManager.login(params);
                    },
                    new RxLoaderObserver<Session>() {
                        @Override public void onNext(Session value) {
                            Flow.get(getView()).replaceTo(new HomeScreen());
                        }
                    }
            ).save();
        }

Saving null-value item makes loader's callback trigger onNext with null as argument. So If you just do nothing, but went out of activity, it was killed, you come back and oops, your onNext was triggered just like it had result, but it doesn't.
E.g.
```java
@OverRide protected void onLoad(Bundle savedInstanceState) {
            RxLoaderManager loaderManager = RxLoaderManager.get(activityHolder.getActivity());
            loginLoader = loaderManager.create((LoginParams params) -> {
                        return sessionManager.login(params);
                    },
                    new RxLoaderObserver<Session>() {
                        @OverRide public void onNext(Session value) {
                            Flow.get(getView()).replaceTo(new HomeScreen());
                        }
                    }
            ).save();
        }
```
@evant
Copy link
Owner

evant commented Apr 9, 2015

Just to understand the issue correctly. onNext() is getting called with a null value when the state is restored when you use save(), even if onNext() is never called? If so, I'm not sure how this pull request actually solves the issue because erroneously restoring the the culprit, not saving.

@almozavr
Copy link
Author

The problem as I see it is outState bundle is not empty but has a key with null value, so the check for null-ness somewhere inside the lib doesn't work (bundle is not empty, it has key). This is a quick fix, so possibly yes, there is a better way.

@almozavr
Copy link
Author

Also, it could be RxLoaderBackendFragmentHelper#107 where any (and null) value may pass

@evant
Copy link
Owner

evant commented Apr 10, 2015

Ah, I think I understand why your fix seems to work then. If you have multiple loaders it will fail though, since it saves all items in the same bundle, causing it to be non-empty even if one of them hasn't saved anything. Ideally, the save/restore methods on the callback should not be called at all if onNext() hasn't fired. Furthermore, it is technically correct to pass a null value to onNext() yourself, and ideally that shouldn't behave any differently. Unfortunately, I don't see a way to detect that case without persisting an extra bit of state on whether on not onNext() has fired or not. I'll try to come up with a better solution for fixing this problem.

@almozavr
Copy link
Author

Basically, I moved to Mortar-Flow approach: ViewObservable.bindView from presenter linked to ReplaySubject – what gives me the very same result but seems like a bit cleaner. So sorry I can't dig into RxLoader and give some help. Anyway thanks!

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

Successfully merging this pull request may close these issues.

2 participants