Skip to content

Add Single.doOnSuccess() #3417

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

Merged
merged 1 commit into from
Oct 9, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/main/java/rx/Single.java
Original file line number Diff line number Diff line change
Expand Up @@ -1864,4 +1864,38 @@ public void onNext(T t) {

return lift(new OperatorDoOnEach<T>(observer));
}

/**
* Modifies the source {@link Single} so that it invokes an action when it calls {@code onSuccess}.
* <p>
* <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/doOnNext.png" alt="">
* <dl>
* <dt><b>Scheduler:</b></dt>
* <dd>{@code doOnSuccess} does not operate by default on a particular {@link Scheduler}.</dd>
* </dl>
*
* @param onSuccess
* the action to invoke when the source {@link Single} calls {@code onSuccess}
* @return the source {@link Single} with the side-effecting behavior applied
* @see <a href="http://reactivex.io/documentation/operators/do.html">ReactiveX operators documentation: Do</a>
*/
@Experimental
public final Single<T> doOnSuccess(final Action1<? super T> onSuccess) {
Copy link
Member

Choose a reason for hiding this comment

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

This should start out as experimental unless the RxJava contributors want to fast-track this.

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 didn't add @Experimental here and in the other PRs because whole class Single is marked with @Experimental. Do you think we should mark these basic operators too?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, this is a bit of a conflict because #3401 gets merged first, this would also become beta without any votes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, no problem. I'll mark it as @experimental.

чт, 8 Окт 2015, 12:44, David Karnok notifications@github.com:

In src/main/java/rx/Single.java
#3417 (comment):

@@ -1789,4 +1790,36 @@ public void onNext(T t) {
return zip(this, other, zipFunction);
}

  • /**
  • \* Modifies the source {@link Single} so that it invokes an action when it calls {@code onSuccess}.
    
  • \* <p>
    
  • \* <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/doOnNext.png" alt="">
    
  • \* <dl>
    
  • \*  <dt><b>Scheduler:</b></dt>
    
  • \*  <dd>{@code doOnSuccess} does not operate by default on a particular {@link Scheduler}.</dd>
    
  • \* </dl>
    
  • *
    
  • \* @param onSuccess
    
  • \*            the action to invoke when the source {@link Single} calls {@code onSuccess}
    
  • \* @return the source {@link Single} with the side-effecting behavior applied
    
  • \* @see <a href="http://reactivex.io/documentation/operators/do.html">ReactiveX operators documentation: Do</a>
    
  • */
    
  • public final Single doOnSuccess(final Action1<? super T> onSuccess) {

Yep, this is a bit of a conflict because #3401
#3401 gets merged first, this
would also become beta without any votes.


Reply to this email directly or view it on GitHub
https://github.com/ReactiveX/RxJava/pull/3417/files#r41494676.

@artem_zin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added @Experimental

Observer<T> observer = new Observer<T>() {
@Override
public void onCompleted() {
}

@Override
public void onError(Throwable e) {
}

@Override
public void onNext(T t) {
onSuccess.call(t);
}
};

return lift(new OperatorDoOnEach<T>(observer));
}
}
78 changes: 78 additions & 0 deletions src/test/java/rx/SingleTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -570,4 +571,81 @@ public void shouldPassErrorFromCallable() throws Exception {

verify(callable).call();
}

@Test
public void doOnSuccessShouldInvokeAction() {
Action1<String> action = mock(Action1.class);

TestSubscriber<String> testSubscriber = new TestSubscriber<String>();

Single
.just("value")
.doOnSuccess(action)
.subscribe(testSubscriber);

testSubscriber.assertValue("value");
testSubscriber.assertNoErrors();

verify(action).call(eq("value"));
}

@Test
public void doOnSuccessShouldPassErrorFromActionToSubscriber() {
Action1<String> action = mock(Action1.class);

Throwable error = new IllegalStateException();
doThrow(error).when(action).call(eq("value"));

TestSubscriber<String> testSubscriber = new TestSubscriber<String>();

Single
.just("value")
.doOnSuccess(action)
.subscribe(testSubscriber);

testSubscriber.assertNoValues();
testSubscriber.assertError(error);

verify(action).call(eq("value"));
}

@Test
public void doOnSuccessShouldNotCallActionIfSingleThrowsError() {
Action1<Object> action = mock(Action1.class);

Throwable error = new IllegalStateException();

TestSubscriber<Object> testSubscriber = new TestSubscriber<Object>();

Single
.error(error)
.doOnSuccess(action)
.subscribe(testSubscriber);

testSubscriber.assertNoValues();
testSubscriber.assertError(error);

verifyZeroInteractions(action);
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you also test what happens if the action callback throws?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See doOnSuccessShouldNotSwallowExceptionThrownByAction()


@Test
public void doOnSuccessShouldNotSwallowExceptionThrownByAction() {
Action1<String> action = mock(Action1.class);

Throwable exceptionFromAction = new IllegalStateException();

doThrow(exceptionFromAction).when(action).call(eq("value"));

TestSubscriber<String> testSubscriber = new TestSubscriber<String>();

Single
.just("value")
.doOnSuccess(action)
.subscribe(testSubscriber);

testSubscriber.assertNoValues();
testSubscriber.assertError(exceptionFromAction);

verify(action).call(eq("value"));
}
}