Repository: broadwaylamb/OpenCombine
Pull request #110 (2019-11-25 13:30:24)

If Multicast receives subscription twice, cancel the second one
This PR also marks a milestone of 900 tests in our test suite, which is awesome, but still not enough!

codecov[bot]: # [Codecov](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=h1) Report
> Merging [#110](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=desc) into [master](https://codecov.io/gh/broadwaylamb/OpenCombine/commit/130125cb66ddb2ab532a5fe3b36af8bbdbbb7a36?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110/graphs/tree.svg?width=650&token=XMi8wfbUeP&height=150&src=pr)](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   97.47%   97.47%   -0.01%     
==========================================
  Files          64       64              
  Lines        3251     3250       -1     
==========================================
- Hits         3169     3168       -1     
  Misses         82       82
```


| [Impacted Files](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../OpenCombine/Publishers/Publishers.Multicast.swift](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110/diff?src=pr&el=tree#diff-U291cmNlcy9PcGVuQ29tYmluZS9QdWJsaXNoZXJzL1B1Ymxpc2hlcnMuTXVsdGljYXN0LnN3aWZ0) | `100% <100%> (ø)` | :arrow_up: |
| [...rces/OpenCombine/Publishers/Result.Publisher.swift](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110/diff?src=pr&el=tree#diff-U291cmNlcy9PcGVuQ29tYmluZS9QdWJsaXNoZXJzL1Jlc3VsdC5QdWJsaXNoZXIuc3dpZnQ=) | `100% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=footer). Last update [130125c...2926110](https://codecov.io/gh/broadwaylamb/OpenCombine/pull/110?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

OpenCombineBot: 
<!--
  0 failure: 
  0 warning: 
  
  1 markdown notices
  DangerID: danger-id-Danger;
-->



LGTM
<p align="right">
  Generated by :no_entry_sign: <a href="https://danger.systems/swift">Danger Swift</a> against 292611090b14578aaa2f495254e1efcfb1545601
</p>


Patch:
diff --git a/Sources/OpenCombine/Publishers/Publishers.Multicast.swift b/Sources/OpenCombine/Publishers/Publishers.Multicast.swift
index 0b14180..84f84e1 100644
--- a/Sources/OpenCombine/Publishers/Publishers.Multicast.swift
+++ b/Sources/OpenCombine/Publishers/Publishers.Multicast.swift
@@ -159,6 +159,7 @@ extension Publishers.Multicast {
             lock.lock()
             guard case let .ready(upstream, downstream) = state else {
                 lock.unlock()
+                subscription.cancel()
                 return
             }
             state = .subscribed(upstream: upstream,


Test patch:
diff --git a/Tests/OpenCombineTests/Helpers/TrackingSubscriber.swift b/Tests/OpenCombineTests/Helpers/TrackingSubscriber.swift
index 2f69f03..1b50361 100644
--- a/Tests/OpenCombineTests/Helpers/TrackingSubscriber.swift
+++ b/Tests/OpenCombineTests/Helpers/TrackingSubscriber.swift
@@ -282,10 +282,10 @@ final class TrackingSubjectBase<Output: Equatable, Failure: Error>
 
     private let _passthrough = PassthroughSubject<Output, Failure>()
     private(set) var history: [Event] = []
-    private let _receiveSubscriber: ((CustomCombineIdentifierConvertible) -> Void)?
+    private let _receiveSubscriber: ((AnySubscriber<Output, Failure>) -> Void)?
     private let _onDeinit: (() -> Void)?
 
-    init(receiveSubscriber: ((CustomCombineIdentifierConvertible) -> Void)? = nil,
+    init(receiveSubscriber: ((AnySubscriber<Output, Failure>) -> Void)? = nil,
          onDeinit: (() -> Void)? = nil) {
         _receiveSubscriber = receiveSubscriber
         _onDeinit = onDeinit
@@ -313,7 +313,7 @@ final class TrackingSubjectBase<Output: Equatable, Failure: Error>
     func receive<Downstream: Subscriber>(subscriber: Downstream)
         where Failure == Downstream.Failure, Output == Downstream.Input
     {
-        _receiveSubscriber?(subscriber)
+        _receiveSubscriber?(AnySubscriber(subscriber))
         history.append(.subscriber)
         _passthrough.subscribe(subscriber)
     }
diff --git a/Tests/OpenCombineTests/PublisherTests/MulticastTests.swift b/Tests/OpenCombineTests/PublisherTests/MulticastTests.swift
index 3049134..efc1ce1 100644
--- a/Tests/OpenCombineTests/PublisherTests/MulticastTests.swift
+++ b/Tests/OpenCombineTests/PublisherTests/MulticastTests.swift
@@ -308,6 +308,19 @@ final class MulticastTests: XCTestCase {
         XCTAssertEqual(counter, 1, "The createSubject closure should be called once")
     }
 
+    func testMulticastReceiveSubscriptionTwice() {
+        let publisher = CustomPublisher(subscription: CustomSubscription())
+        var inner: AnySubscriber<Int, TestingError>?
+        let trackingSubject = TrackingSubject<Int>(receiveSubscriber: { inner = $0 })
+        let multicast = publisher.multicast(subject: trackingSubject)
+        let tracking = TrackingSubscriber()
+        multicast.subscribe(tracking)
+        XCTAssertNotNil(inner)
+        let extraSubscription = CustomSubscription()
+        inner?.receive(subscription: extraSubscription)
+        XCTAssertEqual(extraSubscription.history, [.cancelled])
+    }
+
     func testMulticastReceiveValueBeforeSubscription() {
         testReceiveValueBeforeSubscription(
             value: 0,