Skip to content

Commit c3ac852

Browse files
trxcllntbenlesh
authored andcommitted
fix(multicast): Fixes multicast with selector to create a new source connection per subscriber. (#1774)
Multicast with a selector function should create a new ConnectableObservable for each subscriber to the MulticastObservable. This ensures each subscriber creates a new connection to the source Observable, and don't share subscription side-effects.
1 parent c7e2366 commit c3ac852

File tree

5 files changed

+50
-10
lines changed

5 files changed

+50
-10
lines changed

spec/operators/multicast-spec.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@ describe('Observable.prototype.multicast', () => {
4949
connectable.connect();
5050
});
5151

52-
it('should accept selectors to factory functions', () => {
52+
it('should accept a multicast selector and connect to a hot source for each subscriber', () => {
5353
const source = hot('-1-2-3----4-|');
54-
const sourceSubs = ['^ !'];
54+
const sourceSubs = ['^ !',
55+
' ^ !',
56+
' ^ !'];
5557
const multicasted = source.multicast(() => new Subject(),
5658
x => x.zip(x, (a, b) => (parseInt(a) + parseInt(b)).toString()));
5759
const subscriber1 = hot('a| ').mergeMapTo(multicasted);
@@ -67,6 +69,26 @@ describe('Observable.prototype.multicast', () => {
6769
expectSubscriptions(source.subscriptions).toBe(sourceSubs);
6870
});
6971

72+
it('should accept a multicast selector and connect to a cold source for each subscriber', () => {
73+
const source = cold('-1-2-3----4-|');
74+
const sourceSubs = ['^ !',
75+
' ^ !',
76+
' ^ !'];
77+
const multicasted = source.multicast(() => new Subject(),
78+
x => x.zip(x, (a, b) => (parseInt(a) + parseInt(b)).toString()));
79+
const expected1 = '-2-4-6----8-|';
80+
const expected2 = ' -2-4-6----8-|';
81+
const expected3 = ' -2-4-6----8-|';
82+
const subscriber1 = hot('a| ').mergeMapTo(multicasted);
83+
const subscriber2 = hot(' b| ').mergeMapTo(multicasted);
84+
const subscriber3 = hot(' c| ').mergeMapTo(multicasted);
85+
86+
expectObservable(subscriber1).toBe(expected1);
87+
expectObservable(subscriber2).toBe(expected2);
88+
expectObservable(subscriber3).toBe(expected3);
89+
expectSubscriptions(source.subscriptions).toBe(sourceSubs);
90+
});
91+
7092
it('should do nothing if connect is not called, despite subscriptions', () => {
7193
const source = cold('--1-2---3-4--5-|');
7294
const sourceSubs = [];

spec/operators/publish-spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ describe('Observable.prototype.publish', () => {
5454

5555
it('should accept selectors', () => {
5656
const source = hot('-1-2-3----4-|');
57-
const sourceSubs = ['^ !'];
57+
const sourceSubs = ['^ !',
58+
' ^ !',
59+
' ^ !'];
5860
const published = source.publish(x => x.zip(x, (a, b) => (parseInt(a) + parseInt(b)).toString()));
5961
const subscriber1 = hot('a| ').mergeMapTo(published);
6062
const expected1 = '-2-4-6----8-|';
@@ -321,4 +323,4 @@ describe('Observable.prototype.publish', () => {
321323
expect(subscriptions).to.equal(1);
322324
done();
323325
});
324-
});
326+
});

spec/support/debug.opts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--require source-map-support/register
2+
--require spec/support/mocha-setup-node.js
3+
--require spec-js/helpers/test-helper.js
4+
--require spec-js/helpers/ajax-helper.js
5+
--ui spec-js/helpers/testScheduler-ui.js
6+
7+
--reporter dot
8+
--bail
9+
--full-trace
10+
--check-leaks
11+
--globals WebSocket,FormData
12+
13+
--recursive
14+
--timeout 100000

src/observable/MulticastObservable.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
1+
import {Subject} from '../Subject';
12
import {Observable} from '../Observable';
23
import {Subscriber} from '../Subscriber';
34
import {Subscription} from '../Subscription';
45
import {ConnectableObservable} from '../observable/ConnectableObservable';
56

67
export class MulticastObservable<T> extends Observable<T> {
78
constructor(protected source: Observable<T>,
8-
private connectable: ConnectableObservable<T>,
9+
private subjectFactory: () => Subject<T>,
910
private selector: (source: Observable<T>) => Observable<T>) {
1011
super();
1112
}
1213

1314
protected _subscribe(subscriber: Subscriber<T>): Subscription {
14-
const {selector, connectable} = this;
15-
15+
const { selector, source } = this;
16+
const connectable = new ConnectableObservable(source, this.subjectFactory);
1617
const subscription = selector(connectable).subscribe(subscriber);
1718
subscription.add(connectable.connect());
1819
return subscription;
1920
}
20-
}
21+
}

src/operator/multicast.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ export function multicast<T>(subjectOrSubjectFactory: Subject<T> | (() => Subjec
3333
};
3434
}
3535

36-
const connectable = new ConnectableObservable(this, subjectFactory);
37-
return selector ? new MulticastObservable(this, connectable, selector) : connectable;
36+
return !selector ?
37+
new ConnectableObservable(this, subjectFactory) :
38+
new MulticastObservable(this, subjectFactory, selector);
3839
}
3940

4041
export type factoryOrValue<T> = T | (() => T);

0 commit comments

Comments
 (0)