Search code examples
androidmvpmosby

Mosby, Conductor and Rx not playing well


I just started out a new Sample Project to test Conductor with Mosby. The problem is, after I navigate away from the FirstScreen or rotate the device, after Rx onNext callback, the isViewAttached() is returning always false, and I can't understand why. Some snippets to clarify:

public abstract class MvpLceRxPresenter<V extends MvpLceView<M>, M> extends MvpBasePresenter<V>
        implements MvpPresenter<V> {

    protected Subscriber<M> subscriber;

    protected void unsubscribe() {
        if (subscriber != null && !subscriber.isUnsubscribed()) {
            subscriber.unsubscribe();
        }

        subscriber = null;
    }

    public void subscribe(@NonNull UseCase useCase, @Nullable Bundle bundle, final boolean pullToRefresh) {
        if (isViewAttached()) {
            getView().showLoading(pullToRefresh);
        }

        useCase.unsubscribe();

        subscriber = new Subscriber<M>() {
            private boolean ptr = pullToRefresh;

            @Override
            public void onCompleted() {
                MvpLceRxPresenter.this.onCompleted();
            }

            @Override
            public void onError(Throwable e) {
                MvpLceRxPresenter.this.onError(e, ptr);
            }

            @Override
            public void onNext(M m) {
                MvpLceRxPresenter.this.onNext(m);
            }
        };

        if (useCase instanceof DynamicUseCase) {
            DynamicUseCase dynamicUseCase = (DynamicUseCase) useCase;
            dynamicUseCase.execute(subscriber, bundle);
        } else {
            useCase.execute(subscriber);
        }
    }

    public void subscribe(Observable<M> observable, final boolean pullToRefresh) {
        if (isViewAttached()) {
            getView().showLoading(pullToRefresh);
        }

        unsubscribe();

        subscriber = new Subscriber<M>() {
            private boolean ptr = pullToRefresh;

            @Override
            public void onCompleted() {
                MvpLceRxPresenter.this.onCompleted();
            }

            @Override
            public void onError(Throwable e) {
                MvpLceRxPresenter.this.onError(e, ptr);
            }

            @Override
            public void onNext(M m) {
                MvpLceRxPresenter.this.onNext(m);
            }
        };

        observable.subscribeOn(Schedulers.io())
                .observeOn(AndroidSchedulers.mainThread())
                .subscribe(subscriber);

    }

    protected void onCompleted() {
        if (isViewAttached())
            getView().showContent();
        unsubscribe();
    }

    protected void onError(Throwable e, boolean pullToRefresh) {
        if (isViewAttached())
            getView().showError(e, pullToRefresh);
        unsubscribe();
    }

    protected void onNext(M data) {
        if (isViewAttached())
            getView().setData(data);
    }

    @Override
    public void detachView(boolean retainInstance) {
        super.detachView(retainInstance);
        if (!retainInstance)
            unsubscribe();
    }

My presenter:

@DaggerScope(FirstScreenComponent.class)
public class FirstScreenPresenter extends MvpLceRxPresenter<FirstView, List<Contact>> {

    private final GetContacts getContacts;

    @Inject
    public FirstScreenPresenter(GetContacts getContacts) {
        this.getContacts = getContacts;
    }

    public void fetchContacts(boolean mustHaveNumber) {
        Bundle bundle = new Bundle(1);
        bundle.putBoolean("mustHaveNumber", mustHaveNumber);

        subscribe(this.getContacts, bundle, false);
    }
}

And some part of the FirstScreen:

@Override
    public void onPrepareOptionsMenu(Menu menu) {
        super.onPrepareOptionsMenu(menu);
        swapNumberOnlyText(menu.findItem(R.id.number_only));
    }

    private void swapNumberOnlyText(MenuItem item) {
        if(mustHaveNumber)
            item.setTitle("Todos contatos");
        else
            item.setTitle("Somente com número");
    }

    @Override
    public boolean onOptionsItemSelected(MenuItem item) {

        if(item.getItemId() == R.id.number_only){
            mustHaveNumber = !mustHaveNumber;
            this.presenter.fetchContacts(mustHaveNumber);
            swapNumberOnlyText(item);
            return true;
        }

        return super.onOptionsItemSelected(item);
    }
@Override
    public List<Contact> getData() {
        return adapter.getContacts();
    }

    @Override
    public void setData(List<Contact> data) {
        adapter.setContacts(data);
    }

    @Override
    protected String getErrorMessage(Throwable e, boolean pullToRefresh) {
        return null;
    }

    @NonNull
    @Override
    public LceViewState<List<Contact>, FirstView> createViewState() {
        return new RetainingLceViewState<>();
    }

    @NonNull
    @Override
    public FirstScreenPresenter createPresenter() {
        return presenter;
    }

    @Override
    public void loadData(boolean pullToRefresh) {
        this.presenter.fetchContacts(mustHaveNumber);
    }

What is bugging me is that when first opened, I can click on the menu item and it works like a charm, but if I navigate away or rotate, the problem described starts.

Any help would be greatly appreciated !


Solution

  • This is not a Mosby-Conductor problem. this issue has been caused by dagger and the way you inject the presenter.

    You are creating and injecting a new Presenter instance on every screen orientation change in your onViewBound() method. That means every screen orientation a new presenter will be created.

    Moreover, since you are declaring your own field for presenter you are not using MvpLceViewStateController field for Presenter. See: https://github.com/sockeqwe/mosby-conductor/blob/master/mvp/src/main/java/com/hannesdorfmann/mosby/mvp/conductor/MvpController.java#L19

    Mosby internally works as follows:

    if (getPresenter() == null) {
         setPresenter(  createPresenter() ); 
    }
    

    In MvpController (the base class of MvpViewStateLceController you are extending from getPresenter() and setPresenter() are using MvpController.presenter field. https://github.com/sockeqwe/mosby-conductor/blob/master/mvp/src/main/java/com/hannesdorfmann/mosby/mvp/conductor/MvpController.java#L19

    There are two solutions:

    1. Don't declare your own presenter field but rather do something like this (then it will use MvpController.presenter field):

      @Override public FirstScreenPresenter createPresenter() { return DaggerFirstScreenComponent.builder() .activityComponent(((RootActivity) getActivity()).getActivityComponent()) .firstScreenModule(new FirstScreenModule()) .build().presenter(); }

    This ensures that a presenter instance will be created only once since createPresenter() will only be called once.

    1. If you want to use your own field presenter then you also have to override setPresenter() and getPresenter() in your controller that extends from MvpLce

      public class FirstScreen extends BaseController, FirstView, FirstScreenPresenter> implements FirstView {

      private static final String MUST_HAVE_NUMBER = "must_have_number";
      @Inject
      protected FirstScreenPresenter presenter;
      

      @Override public FirstScreenPresenter getPresenter(){ return presenter; }

      @Override
      public void setPresenter(FirstScreenPresenter p){
         this.presenter = p;
      }
      

    because of the way Mosby internal works it calls getPresenter(), createPresenter() and setPresenter(). But still, you have to ensure not to create and inject a new Presenter in your onViewBound() method.

    Btw. You can also run into this issue if you are using Activity / Fragments with Mosby instead of Conductor