airbnb / RxGroups

Easily group RxJava Observables together and tie them to your Android Activity lifecycle
Apache License 2.0
693 stars 44 forks source link

Make mulitiple observer support more clear #39

Closed BenSchwab closed 7 years ago

BenSchwab commented 7 years ago

As I prepare AirRequest for a 1.0 port, I realized that the new dual tag system partially supported having an observable with multiple subscribers.

i.e:

class Main {
  Observable source = Observable.just("helloWorld);
  AutoResubscribringObserver one;
  AutoResubscribingObserver two;

  public void init() {
   source.transform(one, "source').subscribe(one);
   source.transform(two, "source").subscribe(two);
  }
}

now works because we will store one -> "source" and two -> "source". Generally, this seems like a useful change.

This updates some of the docs to make this behavior more clear.

Also, a cosmetic change. In the convenience case of

source.transform(one).subscribe(one);

will store one -> Main__one instead of one -> null.

I discovered this because AirRequest supports the behavior of cancelling a request (observable) with just the (observable) tag. This becomes more complicated because this tag may now be associated with multiple observers. So I added cancelAndRemoveAllWithTag to clearly communicate this.

Now, I don't necessarily think we will use this behavior internally -- and don't anticipate using it when I migrate to 1.0 internally. Our observables tags are currently unique within a group, and will remain that way. However, I can see how it would be useful if an observable was multicasted.

ConnectableObservable connectbleSource = source.publish();
connectableSource.map(-> some map).transform(one, "connectbleSource").subscribe(one);
connectableSource.filter().transform(two, "connectableSource").subscribe(two, "connectbleSource");
connectableSource.publish()

// later
group.cancelAndRemoveAllWithTag("connectedSource")

would unsubscribe everything attached to connectableSource in one shot.

Alternatively, of we choose not to support this, we would have to check that each obseravbleTag is globally unique within the group, and cancelAndRemove the duplicate.

TODO:

BenSchwab commented 7 years ago

Let's hold off on the cancelling via observableTag for now, and wait to see if an actual use case necessitates it. Originally added because we use a similar pattern in pre 1.0, but I agree moving forward code should generally send both the observer, and if appropriate, tag.