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

doOnUnsubscribe behaviour does not align with documentation #3877

Closed
rossdanderson opened this issue Apr 22, 2016 · 7 comments
Closed

doOnUnsubscribe behaviour does not align with documentation #3877

rossdanderson opened this issue Apr 22, 2016 · 7 comments

Comments

@rossdanderson
Copy link

rossdanderson commented Apr 22, 2016

Hi,

According to the documentation here, when my source observable is reference counted I should only be notified of the final subscriber's unsubscription, however, unless I am misunderstanding the term source - or misusing it, it appears to be called on every subscriber's unsubscription.

Below is a simple test which demonstrates what I mean - onUnsubscribeAfterRefCount will fail, while onUnsubscribeBeforeRefCount will pass

package streams;

import org.junit.Test;
import rx.Observable;
import rx.Subscription;
import rx.observers.TestSubscriber;
import rx.subjects.PublishSubject;

import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.Assert.assertEquals;

public class UnsubscribeTest {

    @Test
    public void onUnsubscribeAfterRefCount() {
        AtomicInteger count = new AtomicInteger();

        Observable<String> publishSubject = PublishSubject.<String>create().publish().refCount().doOnUnsubscribe(count::getAndIncrement);

        TestSubscriber<String> testSubscriber1 = TestSubscriber.create();
        Subscription subscription1 = publishSubject.subscribe(testSubscriber1);

        TestSubscriber<String> testSubscriber2 = TestSubscriber.create();
        Subscription subscription2 = publishSubject.subscribe(testSubscriber2);

        subscription1.unsubscribe();
        subscription2.unsubscribe();

        assertEquals(1, count.get());
    }

    @Test
    public void onUnsubscribeBeforeRefCount() {
        AtomicInteger count = new AtomicInteger();

        Observable<String> publishSubject = PublishSubject.<String>create().doOnUnsubscribe(count::getAndIncrement).publish().refCount();

        TestSubscriber<String> testSubscriber1 = TestSubscriber.create();
        Subscription subscription1 = publishSubject.subscribe(testSubscriber1);

        TestSubscriber<String> testSubscriber2 = TestSubscriber.create();
        Subscription subscription2 = publishSubject.subscribe(testSubscriber2);

        subscription1.unsubscribe();
        subscription2.unsubscribe();

        assertEquals(1, count.get());
    }
}
@akarnokd
Copy link
Member

You get two calls because the two subscribers create two separate chains up until the publish() stage and two unsubscribe calls are issued. In the second (synchronous) case, publish().refCount() makes sure the unsubscription happens only once and the preceding doOnUnsubscribe only gets called once.

@rossdanderson
Copy link
Author

Yes I follow what is happening but this does not seem to align with the documentation where it says the source observable is modified to call doOnUnsubscribe, and that if the source observable is ref counted then it should only happen once.
The source observable is the one on the left I presume? In which case is this an invalid statement?

@akarnokd
Copy link
Member

Ah, yes, that statement is invalid. Would you like to submit a PR and fix that?

@rossdanderson
Copy link
Author

Certainly, is the intention to fix the documentation to match the actual behavour?
Cheers

@akarnokd
Copy link
Member

Fix the documentation.

@akarnokd
Copy link
Member

akarnokd commented May 5, 2016

See my proposed updates to the javadoc in #3907

@akarnokd
Copy link
Member

Closing via #3907.

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