From 8f5b89cf26bb4e0d26eca254419741278a290ec8 Mon Sep 17 00:00:00 2001 From: Jeff Verkoeyen Date: Wed, 26 Apr 2017 16:47:11 -0400 Subject: [PATCH 1/6] [breaking] Remove automatic unsubscription from the Subscription object. Summary: This means that the disconnect method of observables will no longer be automatically invoked when the Subscription object is released. This has several advantages: - Subscriptions no longer need to be explicitly maintained if unsubscription is unimportant to the subscriber. - The subscribe method's return value can now be marked ignorable, simplifying any call sites that don't care about unsubscribing. The documentation for IndefiniteObservable has been updated to reflect the new behavior. Notably, we'd previously written that Subscriptions maintained a reference to the IndefiniteObservable chain; this isn't/wasn't technically true. Subscriptions only hold references to the chain of disconnect methods. It is now possible to write expressions like so: observable.subscribe { value in print(value) } With no explicit subscription storage required. Reviewers: O2 Material Motion, O4 Material Apple platform reviewers, #material_motion, markwei Reviewed By: O2 Material Motion, O4 Material Apple platform reviewers, #material_motion, markwei Tags: #material_motion Differential Revision: http://codereview.cc/D3116 --- Podfile.lock | 2 +- src/IndefiniteObservable.swift | 16 +++++------ tests/unit/MemoryLeakTests.swift | 44 +++++++++++++++++++++-------- tests/unit/ObservableTests.swift | 48 +++++++++++++------------------- tests/unit/SimpleOperators.swift | 1 + 5 files changed, 61 insertions(+), 50 deletions(-) diff --git a/Podfile.lock b/Podfile.lock index e76cbb9..09e323a 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -19,4 +19,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: 3e4cdba95901e07a289159f0c5a8b830ecb1a5c8 -COCOAPODS: 1.1.1 +COCOAPODS: 1.2.0 diff --git a/src/IndefiniteObservable.swift b/src/IndefiniteObservable.swift index 04e159d..af45fd3 100644 --- a/src/IndefiniteObservable.swift +++ b/src/IndefiniteObservable.swift @@ -57,14 +57,18 @@ open class IndefiniteObservable { /** Subscribes to the IndefiniteObservable. - The returned subscription will hold a strong reference to the IndefiniteObservable chain. The - reference can be released by calling unsubscribe on the returned subscription. The Subscription - is type-erased, making it possible to keep a collection of Subscription objects for as long as - you need the associated streams alive. + The returned subscription will hold a strong reference to the disconnect chain. The reference can + be released by calling unsubscribe on the returned subscription. The Subscription is type-erased, + making it possible to keep a collection of Subscription objects for as long as you need the + associated streams alive. + + If you do not keep the returned subscription then the disconnect calls will never be invoked for + this observer. - Parameter next: A block that will be executed when new values are sent from upstream. - Returns: A subscription. */ + @discardableResult public final func subscribe(observer: O) -> Subscription { return Subscription(connect(observer)) } @@ -82,10 +86,6 @@ public typealias Disconnect = () -> Void /** A Subscription is returned by IndefiniteObservable.subscribe. */ public final class Subscription { - deinit { - unsubscribe() - } - public init(_ disconnect: @escaping () -> Void) { self.disconnect = disconnect } diff --git a/tests/unit/MemoryLeakTests.swift b/tests/unit/MemoryLeakTests.swift index fdc0291..59bec13 100644 --- a/tests/unit/MemoryLeakTests.swift +++ b/tests/unit/MemoryLeakTests.swift @@ -63,7 +63,7 @@ class MemoryLeakTests: XCTestCase { weak var weakObservable = observable autoreleasepool { - let _ = observable!.subscribe { + observable!.subscribe { let _ = $0 } // Remove our only strong reference. @@ -83,7 +83,7 @@ class MemoryLeakTests: XCTestCase { weak var weakObservable = observable autoreleasepool { - let _ = observable!.map { value in + observable!.map { value in return value * value }.subscribe { let _ = $0 @@ -97,7 +97,7 @@ class MemoryLeakTests: XCTestCase { XCTAssertNil(weakObservable) } - func testUnsubscribedObservableWithOperatorIsDeallocated() { + func testObservableWithOperatorIsDeallocated() { weak var weakObservable: ValueObservable? autoreleasepool { let observable: ValueObservable? = ValueObservable { observer in @@ -106,13 +106,11 @@ class MemoryLeakTests: XCTestCase { } weakObservable = observable - let subscription = observable!.map { value in + observable!.map { value in return value * value - }.subscribe { - let _ = $0 - } - // Remove our only strong reference. - subscription.unsubscribe() + }.subscribe { + let _ = $0 + } } // If this fails it means there's a retain cycle. Place a breakpoint here and use the Debug @@ -122,7 +120,6 @@ class MemoryLeakTests: XCTestCase { func testSubscriptionDoesNotKeepObservableInMemory() { weak var weakObservable: ValueObservable? - var subscription: Subscription? autoreleasepool { let value = 10 @@ -132,11 +129,34 @@ class MemoryLeakTests: XCTestCase { } weakObservable = observable - subscription = observable.subscribe { _ in } + observable.subscribe { _ in } } XCTAssertNil(weakObservable) + } + + func testSubscriptionDoesNotKeepObserverInMemory() { + weak var weakObserver: ValueObserver? + + var didDisconnect = false + var didSetObserver = false + + autoreleasepool { + let value = 10 + let observable = ValueObservable { observer in + weakObserver = observer + didSetObserver = true + observer.next(value) + return { + didDisconnect = true + } + } + + observable.subscribe { _ in } + } - subscription?.unsubscribe() + XCTAssertNil(weakObserver) + XCTAssertTrue(didSetObserver) + XCTAssertFalse(didDisconnect) } } diff --git a/tests/unit/ObservableTests.swift b/tests/unit/ObservableTests.swift index a976076..88e2856 100644 --- a/tests/unit/ObservableTests.swift +++ b/tests/unit/ObservableTests.swift @@ -29,7 +29,7 @@ class ObservableTests: XCTestCase { } let wasReceived = expectation(description: "Value was received") - let _ = observable.subscribe { + observable.subscribe { if $0 == value { wasReceived.fulfill() } @@ -38,7 +38,7 @@ class ObservableTests: XCTestCase { waitForExpectations(timeout: 0) } - func testUnsubscribesOnDeallocation() { + func testDoesNotUnsubscribeOnDeallocation() { var didUnsubscribe = false autoreleasepool { @@ -48,10 +48,10 @@ class ObservableTests: XCTestCase { } } - let _ = observable.subscribe { _ in } + observable.subscribe { _ in } } - XCTAssertTrue(didUnsubscribe) + XCTAssertFalse(didUnsubscribe) } func testUnsubscribesOnUnsubscribe() { @@ -78,14 +78,14 @@ class ObservableTests: XCTestCase { } let wasReceived = expectation(description: "Value was received") - let _ = observable.subscribe { + observable.subscribe { if $0 == value { wasReceived.fulfill() } } let wasReceived2 = expectation(description: "Value was received") - let _ = observable.subscribe { + observable.subscribe { if $0 == value { wasReceived2.fulfill() } @@ -103,23 +103,20 @@ class ObservableTests: XCTestCase { } let wasReceived = expectation(description: "Value was received") - let subscription1 = observable.subscribe { + observable.subscribe { if $0 == value { wasReceived.fulfill() } } let wasReceived2 = expectation(description: "Value was received") - let subscription2 = observable.subscribe { + observable.subscribe { if $0 == value { wasReceived2.fulfill() } } waitForExpectations(timeout: 0) - - subscription1.unsubscribe() - subscription2.unsubscribe() } func testMappingValues() { @@ -130,7 +127,7 @@ class ObservableTests: XCTestCase { } let wasReceived = expectation(description: "Value was received") - let _ = observable.map { $0 * $0 }.subscribe { + observable.map { $0 * $0 }.subscribe { if $0 == value * value { wasReceived.fulfill() } @@ -147,7 +144,7 @@ class ObservableTests: XCTestCase { } let wasReceived = expectation(description: "Value was received") - let _ = observable.map { $0.y }.subscribe { + observable.map { $0.y }.subscribe { if $0 == value.y { wasReceived.fulfill() } @@ -165,7 +162,7 @@ class ObservableTests: XCTestCase { } var filteredValues: [CGPoint] = [] - let _ = observable.filter { (state, _) in state == true }.map { $0.1 }.subscribe { + observable.filter { (state, _) in state == true }.map { $0.1 }.subscribe { filteredValues.append($0) } @@ -202,11 +199,11 @@ class ObservableTests: XCTestCase { } var valuesObserved: [Int] = [] - let subscription1 = observable.subscribe { + observable.subscribe { valuesObserved.append($0) } - let subscription2 = observable.subscribe { + observable.subscribe { valuesObserved.append($0 * 2) } @@ -215,9 +212,6 @@ class ObservableTests: XCTestCase { generator.emit(2) XCTAssertEqual(valuesObserved, [5, 10, 10, 20, 2, 4]) - - subscription1.unsubscribe() - subscription2.unsubscribe() } func testGeneratedValuesAreNotReceivedAfterUnsubscription() { @@ -231,22 +225,20 @@ class ObservableTests: XCTestCase { } var valuesObserved: [Int] = [] - let subscription1 = observable.subscribe { + observable.subscribe { valuesObserved.append($0) } - let subscription2 = observable.subscribe { + let subscription = observable.subscribe { valuesObserved.append($0 * 2) } generator.emit(5) generator.emit(10) - subscription2.unsubscribe() + subscription.unsubscribe() generator.emit(2) XCTAssertEqual(valuesObserved, [5, 10, 10, 20, 2]) - - subscription1.unsubscribe() } func testGeneratedValuesAreNotReceivedAfterUnsubscriptionOrder2() { @@ -263,22 +255,20 @@ class ObservableTests: XCTestCase { weakObservable = observable var valuesObserved: [Int] = [] - let subscription1 = observable.subscribe { + let subscription = observable.subscribe { valuesObserved.append($0) } - let subscription2 = observable.map { $0 * 2 }.subscribe { + observable.map { $0 * 2 }.subscribe { valuesObserved.append($0) } generator.emit(5) generator.emit(10) - subscription1.unsubscribe() + subscription.unsubscribe() generator.emit(2) XCTAssertEqual(valuesObserved, [5, 10, 10, 20, 4]) - - subscription2.unsubscribe() } // If this fails it means there's a retain cycle. Place a breakpoint here and use the Debug diff --git a/tests/unit/SimpleOperators.swift b/tests/unit/SimpleOperators.swift index 040860c..43f0160 100644 --- a/tests/unit/SimpleOperators.swift +++ b/tests/unit/SimpleOperators.swift @@ -30,6 +30,7 @@ public final class ValueObserver: Observer { } public class ValueObservable: IndefiniteObservable> { + @discardableResult public final func subscribe(_ next: @escaping (T) -> Void) -> Subscription { return super.subscribe(observer: ValueObserver(next)) } From fb125329170d8fb537f53c16110f4210e89ebf86 Mon Sep 17 00:00:00 2001 From: Jeff Verkoeyen Date: Wed, 26 Apr 2017 17:44:37 -0400 Subject: [PATCH 2/6] Automatic changelog preparation for release. --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b71bdf..6dddd75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# #develop# + + TODO: Enumerate changes. + + # 3.1.0 ## New features From a492076d65aa9540737ac845fb1380c46cf53a9b Mon Sep 17 00:00:00 2001 From: Jeff Verkoeyen Date: Wed, 26 Apr 2017 17:47:14 -0400 Subject: [PATCH 3/6] Update README with new documentation for subscriptions. --- README.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 180db14..a5ab652 100644 --- a/README.md +++ b/README.md @@ -112,20 +112,22 @@ let observable = ValueObservable<<#ValueType#>> { observer in ## How to subscribe to a stream -Streams are kept in memory by their subscriptions. - ```swift -let subscription = observable.subscribe { value in +observable.subscribe { value in print(value) } ``` ## How to unsubscribe from a stream -Unsubscribe from a stream to allow the stream to be released. The stream can be deallocated once all -of its subscriptions have unsubscribed. +Unsubscribing will invoke the observable's disconnect method. To unsubscribe, you must retain a +reference to the subscription instance returned by subscribe. ```swift +let subscription = observable.subscribe { value in + print(value) +} + subscription.unsubscribe() ``` From c5a9612ba51f52d90173a1a17c436c42353fea25 Mon Sep 17 00:00:00 2001 From: Jeff Verkoeyen Date: Wed, 26 Apr 2017 17:56:06 -0400 Subject: [PATCH 4/6] Update release notes. --- CHANGELOG.md | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dddd75..d088ae2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,55 @@ -# #develop# +# 4.0.0 - TODO: Enumerate changes. +This major release improves the general usage of the observable pattern by removing the +auto-unsubscription behavior from the Subscription type on deallocation. +```swift +// Pre-4.0 usage required that you held on to the Subscription instance in order to continue +// receiving values +let subscription = observable.subscribe { value in + print(value) +} + +// 4.0 allows you to subscribe to streams without holding on to the Subscription instance. +observable.subscribe { value in + print(value) +} +``` + +## Breaking changes + +• Subscriptions no longer unsubscribe automatically upon deallocation. + +This means that you no longer need to hold on to a Subscription in order to continue receiving +values from a subscribed stream. + +If you were previously depending on this behavior then you must now ensure that you explicitly +unsubscribe from subscriptions. + +## Source changes + +* [[breaking] Remove automatic unsubscription from the Subscription object.](https://github.com/material-motion/indefinite-observable-swift/commit/8f5b89cf26bb4e0d26eca254419741278a290ec8) (Jeff Verkoeyen) + +## API changes + +Auto-generated by running: + + apidiff origin/stable release-candidate swift IndefiniteObservable.xcworkspace IndefiniteObservable + +## Subscription + +## IndefiniteObservable + +*modified* method: `subscribe(observer:)` in `IndefiniteObservable` + +| Type of change: | key.attributes | +|---|---| +| From: | `( { "key.attribute" = "source.decl.attribute.final"; } )` | +| To: | `( { "key.attribute" = "source.decl.attribute.final"; }, { "key.attribute" = "source.decl.attribute.discardableResult"; } )` | + +## Non-source changes + +* [Update README with new documentation for subscriptions.](https://github.com/material-motion/indefinite-observable-swift/commit/a492076d65aa9540737ac845fb1380c46cf53a9b) (Jeff Verkoeyen) # 3.1.0 From f8da2f8b6de91bfbbb53d4707be010f3c2d3ec9b Mon Sep 17 00:00:00 2001 From: Jeff Verkoeyen Date: Wed, 26 Apr 2017 17:56:21 -0400 Subject: [PATCH 5/6] Bump the version to 4.0.0. --- IndefiniteObservable.podspec | 2 +- Podfile.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/IndefiniteObservable.podspec b/IndefiniteObservable.podspec index a18e15c..e85ef57 100644 --- a/IndefiniteObservable.podspec +++ b/IndefiniteObservable.podspec @@ -1,7 +1,7 @@ Pod::Spec.new do |s| s.name = "IndefiniteObservable" s.summary = "IndefiniteObservable is a minimal implementation of Observable with no concept of completion or failure." - s.version = "3.1.0" + s.version = "4.0.0" s.authors = "The Material Motion Authors" s.license = "Apache 2.0" s.homepage = "https://github.com/material-motion/indefinite-observable-swift" diff --git a/Podfile.lock b/Podfile.lock index 09e323a..6eac55d 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -1,8 +1,8 @@ PODS: - CatalogByConvention (2.0.0) - - IndefiniteObservable/examples (3.1.0): + - IndefiniteObservable/examples (4.0.0): - IndefiniteObservable/lib - - IndefiniteObservable/lib (3.1.0) + - IndefiniteObservable/lib (4.0.0) DEPENDENCIES: - CatalogByConvention @@ -15,7 +15,7 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: CatalogByConvention: be55c2263132e4f9f59299ac8a528ee8715b3275 - IndefiniteObservable: 8dd88933eb00b0186cdcc665692beba53da34f44 + IndefiniteObservable: 0e948393173ea7dc68a9f439530559aa84d96d4c PODFILE CHECKSUM: 3e4cdba95901e07a289159f0c5a8b830ecb1a5c8 From 5373c0542ee270602fb0d41b9f77d94476fcb239 Mon Sep 17 00:00:00 2001 From: Jeff Verkoeyen Date: Wed, 26 Apr 2017 17:58:02 -0400 Subject: [PATCH 6/6] Add swift version file for CocoaPods. --- .swift-version | 1 + 1 file changed, 1 insertion(+) create mode 100644 .swift-version diff --git a/.swift-version b/.swift-version new file mode 100644 index 0000000..9f55b2c --- /dev/null +++ b/.swift-version @@ -0,0 +1 @@ +3.0