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

Add support for exposing delegate observers. #89

Merged
merged 13 commits into from
Sep 22, 2017
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ final class AutoDisposingObserverImpl<T> implements AutoDisposingObserver<T> {
this.delegate = delegate;
}

@Override public Observer<? super T> delegateObserver() {
return delegate;
}

@Override public void onSubscribe(final Disposable d) {
if (AutoDisposeEndConsumerHelper.setOnce(lifecycleDisposable,
lifecycle.doOnEvent(new BiConsumer<Object, Throwable>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,10 @@
* A {@link Disposable} {@link Observer} that can automatically dispose itself.
* Interface here for type safety but enforcement is left to the implementation.
*/
public interface AutoDisposingObserver<T> extends Observer<T>, Disposable {}
public interface AutoDisposingObserver<T> extends Observer<T>, Disposable {

/**
* @return {@link Observer} The delegate Observer that is used under the hood for introspection purposes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Did you mean to link the Observer here rather than inline?

I'd expect this to be like @return The delegate {@link Observer} that is used under the hood for introspection purposes.

*/
Observer<? super T> delegateObserver();
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,34 @@
package com.uber.autodispose;

import com.uber.autodispose.test.RecordingObserver;
import com.uber.autodispose.observers.AutoDisposingObserver;
import io.reactivex.Maybe;
import io.reactivex.Observable;
import io.reactivex.ObservableEmitter;
import io.reactivex.ObservableOnSubscribe;
import io.reactivex.Observer;
import io.reactivex.disposables.Disposable;
import io.reactivex.functions.BiFunction;
import io.reactivex.functions.Cancellable;
import io.reactivex.functions.Consumer;
import io.reactivex.functions.Predicate;
import io.reactivex.observers.TestObserver;
import io.reactivex.plugins.RxJavaPlugins;
import io.reactivex.subjects.BehaviorSubject;
import io.reactivex.subjects.MaybeSubject;
import io.reactivex.subjects.PublishSubject;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.After;
import org.junit.Test;

import static com.google.common.truth.Truth.assertThat;

public class AutoDisposeObserverTest {

private final AtomicReference<Observer> atomicObserver = new AtomicReference();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since these are only used in one test, let's make them final local variables in those methods instead

private final AtomicReference<Observer> atomicAutoDisposingObserver = new AtomicReference<>();

private static final RecordingObserver.Logger LOGGER = new RecordingObserver.Logger() {
@Override public void log(String message) {
System.out.println(AutoDisposeObserverTest.class.getSimpleName() + ": " + message);
Expand Down Expand Up @@ -154,7 +162,7 @@ public class AutoDisposeObserverTest {
lifecycle.onNext(1);
source.onNext(2);

assertThat(source.hasObservers()).isTrue();
assertThat(source.hasObservers()).isTrue();Å
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wat.gif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was from you trying to use my keyboard.

assertThat(lifecycle.hasObservers()).isTrue();
assertThat(o.takeNext()).isEqualTo(2);

Expand Down Expand Up @@ -256,6 +264,31 @@ public class AutoDisposeObserverTest {
});
}

@Test public void verifyObserverDelegate() {
try {
RxJavaPlugins.setOnObservableSubscribe(new BiFunction<Observable, Observer, Observer>() {
@Override public Observer apply(Observable source, Observer observer) {
if (atomicObserver.get() == null) {
atomicObserver.set(observer);
} else if (atomicAutoDisposingObserver.get() == null) {
atomicAutoDisposingObserver.set(observer);
RxJavaPlugins.setOnObservableSubscribe(null);
}
return observer;
}
});
Observable.just(1).to(new ObservableScoper<Integer>(Maybe.never())).subscribe();

assertThat(atomicAutoDisposingObserver.get()).isNotNull();
assertThat(atomicAutoDisposingObserver.get()).isInstanceOf(AutoDisposingObserver.class);
assertThat(((AutoDisposingObserver)atomicAutoDisposingObserver.get()).delegateObserver()).isNotNull();
assertThat(((AutoDisposingObserver)atomicAutoDisposingObserver.get()).delegateObserver())
.isEqualTo(atomicObserver.get());
Copy link
Collaborator

@ZacSweers ZacSweers Sep 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a reference check, not equality

} finally {
RxJavaPlugins.reset();
}
}

@Test public void verifyCancellation() throws Exception {
final AtomicInteger i = new AtomicInteger();
//noinspection unchecked because Java
Expand Down