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

RxLifecycle interop module #82

Closed
ZacSweers opened this issue Sep 10, 2017 · 8 comments
Closed

RxLifecycle interop module #82

ZacSweers opened this issue Sep 10, 2017 · 8 comments

Comments

@ZacSweers
Copy link
Collaborator

Should we add one? The gist would be that it could understand RxLifecycle's LifecycleProvider mechanics, which in turn should also lend support for its various components (RxActivity, etc)

@bangarharshit
Copy link
Contributor

What will be the use case for such a module?

@ZacSweers
Copy link
Collaborator Author

So when I say which in turn should also lend support for its various components (RxActivity, etc) above, I'm referring to things like RxActivity or implement LifecycleProvider such that anyone currently using RxLifecycle could use AutoDispose rather seemlessly without needing to go redo all their plumbing up front

@bangarharshit
Copy link
Contributor

bangarharshit commented Oct 20, 2017

One solution can be to have static util methods to provide the different type of scopers for Lifecycleprovider.

For MaybeScoper just listen to Lifecycleprovider and emit when it emits (just rough sample code).

  public static <T> Function<Observable<?>, ObservableSubscribeProxy<Object>> lifeCycleProvider(final LifecycleProvider<T> lifecycleProvider) {
    Maybe<T> maybe = Maybe.create(new MaybeOnSubscribe<T>() {
      @Override public void subscribe(final MaybeEmitter<T> e) throws Exception {
        lifecycleProvider.lifecycle().subscribe(new Consumer<T>() {
          @Override public void accept(T t) throws Exception {
            e.onSuccess(t);
          }
        }, new Consumer<Throwable>() {
          @Override public void accept(Throwable throwable) throws Exception {
            // It shouldn't be thrown ideally. Throwing so that AutoDisposingObserverImpl dispose in the error callback.
            e.onError(throwable);
          }
        });
      }
    });
    return AutoDispose.with(maybe).forObservable();
  }

For Lifecyclescopeprovider the consumer can provide the CorrespondingEvents (or the defaults from RXLifecycle can be used).

  public static <T>LifecycleScopeProvider<T> lifecycleScopeProvider(LifecycleProvider<T> lifecycleProvider, final Function<T, T> correspondingEvents) {
    final BehaviorSubject<T> behaviorSubject = BehaviorSubject.create();
    lifecycleProvider.lifecycle().subscribe(new Consumer<T>() {
      @Override public void accept(T t) throws Exception {

      }
    });
    return new LifecycleScopeProvider<T>() {
      @Override public Observable<T> lifecycle() {
        return behaviorSubject.hide();
      }

      @Override public Function<T, T> correspondingEvents() {
        return correspondingEvents;
      }

      @Nullable @Override public T peekLifecycle() {
        return behaviorSubject.getValue();
      }
    };
  }

  public static <T>LifecycleScopeProvider<T> activityLifecycleScopeProvider(LifecycleProvider<T> lifecycleProvider) {
    return lifecycleScopeProvider(lifecycleProvider, RxLifecycleAndroid.ACTIVITY_LIFECYCLE);
  }

Not sure if this is the best approach as it still needs some plumbing. Also, since Rx lifecycle has multiple modules so it may require multiple interop modules.

For RxActivity, I am not sure what will be equivalent of bindUntilEvent

@ZacSweers
Copy link
Collaborator Author

ZacSweers commented Oct 21, 2017

The API should just return scope providers for autodispose to use, not act as an alternative to autodispose itself. Also FYI - The API has changed, Scopers are deprecated.

So my thoughts on this are basically:

  • Have an RxLifecycleInterop class with static factories
  • A bindLifecycle method: static ScopeProvider bindLifecycle(LifecycleProvider<E> provider) that calls the bindToLifecycle method under the hood and creates a ScopeProvider based on its returned transformer
  • A bindUntilEvent method: static ScopeProvider bindLifecycle(LifecycleProvider<E> provider, E event) that does the above but to bindUntilEvent().

An implementation could look like this:

static ScopeProvider bindLifecycle(LifecycleProvider<E> provider) {
  return new ScopeProvider() {
    @Override
    Maybe<?> requestScope() {
      return provider.lifecycle()
          .compose(provider.bindToLifecycle())  // Interop layer. This stream will now complete upon lifecycle end
          .ignoreElements() // Listen for just completions
          .toMaybe()  // Back to Maybe<?>
          .defaultIfEmpty(SOME_STATIC_THROWAWAY_OBJECT) // Because RxLifecycle just completes when it's done, we want to coerce that to a faked "onSuccess" here
    }
  }
}

There will be caveats, but that's ok. This will not really be a LifecycleScopeProvider, but rather just a ScopeProvider that defers to RxLifecycle's boundary checks under the hood.

One question could be if we want to coerce RxLifecycle's OutsideLifecycleException to an AutoDispose LifecycleEndedException via onErrorResumeNext. Pros is it keeps the translation consistent (RxLifecycle in, AutoDispose out). Cons is any of the user's existing error handling logic looking for those will not catch this. We'll probably just have to make a call on what to do by default, but we could add an overload that accepts a boolean param to allow the user to specify which one they want to do. Or we could just decide to transform all of them and offer a utility transformer to convert errors back to RxLifecycle errors if they want (.compose(autoDisposeLifecycleErrorsToRxLifecycleErrors()))

@bangarharshit
Copy link
Contributor

bangarharshit commented Oct 21, 2017

Got it. Static utility method for error is much better than either forcing them or breaking their existing chain.

I think the best part of the approach is that we doesn't need separate methods for RX Activity as the caller takes care of provider.bindToLifeCycle()

@ZacSweers
Copy link
Collaborator Author

Cool. Let's make the default to map to AutoDispose's exception, since I think that will be the common case. That way it'll be picked up by the autodispose plugin hook for lifecycle exceptions too

@bangarharshit
Copy link
Contributor

bangarharshit commented Oct 21, 2017

Looking at RxLifecycle source code RXLifeCycle.java and OutsideLifeCycleException.java, it treats LifecycleEndedException as normal terminal event. Unlike AutoDispose, it doesn't throw the error to the user and silently completes the stream.

@ZacSweers
Copy link
Collaborator Author

Oh right, in that case we don't need to add the error switch at all then

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

No branches or pull requests

2 participants