Merge pull request #362 from ReactiveCocoa/property-bugfix

Fixed rare occurences of `interrupted` emission in `Property`.

Author: mdiep

Sources/Property.swift

@@ -518,7 +518,10 @@ public final class Property<Value>: PropertyProtocol {
 	///   - values: A producer that will start immediately and send values to
 	///             the property.
 	public convenience init(initial: Value, then values: SignalProducer<Value, NoError>) {
-		self.init(unsafeProducer: values.prefix(value: initial))
+		self.init(unsafeProducer: SignalProducer { observer, disposables in
+			observer.send(value: initial)
+			disposables += values.start(Observer(mappingInterruptedToCompleted: observer))
+		})
 	}
 
 	/// Initialize a composed property that first takes on `initial`, then each
@@ -528,7 +531,7 @@ public final class Property<Value>: PropertyProtocol {
 	///   - initialValue: Starting value for the property.
 	///   - values: A signal that will send values to the property.
 	public convenience init(initial: Value, then values: Signal<Value, NoError>) {
-		self.init(unsafeProducer: SignalProducer(values).prefix(value: initial))
+		self.init(initial: initial, then: SignalProducer(values))
 	}
 
 	/// Initialize a composed property by applying the unary `SignalProducer`
@@ -627,15 +630,10 @@ public final class MutableProperty<Value>: ComposableMutablePropertyProtocol {
 	/// followed by all changes over time, then complete when the property has
 	/// deinitialized.
 	public var producer: SignalProducer<Value, NoError> {
-		return SignalProducer { [atomic, weak self] producerObserver, producerDisposable in
+		return SignalProducer { [atomic, signal] producerObserver, producerDisposable in
 			atomic.withValue { value in
-				if let strongSelf = self {
-					producerObserver.send(value: value)
-					producerDisposable += strongSelf.signal.observe(producerObserver)
-				} else {
-					producerObserver.send(value: value)
-					producerObserver.sendCompleted()
-				}
+				producerObserver.send(value: value)
+				producerDisposable += signal.observe(Observer(mappingInterruptedToCompleted: producerObserver))
 			}
 		}
 	}
@@ -696,3 +694,16 @@ public final class MutableProperty<Value>: ComposableMutablePropertyProtocol {
 		observer.sendCompleted()
 	}
 }
+
+private extension Observer {
+	convenience init(mappingInterruptedToCompleted observer: Observer<Value, Error>) {
+		self.init { event in
+			switch event {
+			case .value, .completed, .failed:
+				observer.action(event)
+			case .interrupted:
+				observer.sendCompleted()
+			}
+		}
+	}
+}

Tests/ReactiveSwiftTests/PropertySpec.swift

@@ -332,6 +332,7 @@ class PropertySpec: QuickSpec {
 
 					var sentValue: String?
 					var signalCompleted = false
+					var hasUnexpectedEventsEmitted = false
 
 					constantProperty.producer.start { event in
 						switch event {
@@ -340,12 +341,13 @@ class PropertySpec: QuickSpec {
 						case .completed:
 							signalCompleted = true
 						case .failed, .interrupted:
-							break
+							hasUnexpectedEventsEmitted = true
 						}
 					}
 
 					expect(sentValue) == initialPropertyValue
 					expect(signalCompleted) == true
+					expect(hasUnexpectedEventsEmitted) == false
 				}
 			}
 
@@ -359,6 +361,7 @@ class PropertySpec: QuickSpec {
 						var signalSentValue: String?
 						var producerCompleted = false
 						var signalInterrupted = false
+						var hasUnexpectedEventsEmitted = false
 
 						property.producer.start { event in
 							switch event {
@@ -367,25 +370,24 @@ class PropertySpec: QuickSpec {
 							case .completed:
 								producerCompleted = true
 							case .failed, .interrupted:
-								break
+								hasUnexpectedEventsEmitted = true
 							}
 						}
 
 						property.signal.observe { event in
 							switch event {
-							case let .value(value):
-								signalSentValue = value
 							case .interrupted:
 								signalInterrupted = true
-							case .failed, .completed:
-								break
+							case .value, .failed, .completed:
+								hasUnexpectedEventsEmitted = true
 							}
 						}
 
 						expect(sentValue) == initialPropertyValue
 						expect(signalSentValue).to(beNil())
 						expect(producerCompleted) == true
 						expect(signalInterrupted) == true
+						expect(hasUnexpectedEventsEmitted) == false
 					}
 
 					it("should retain the wrapped property") {
@@ -414,6 +416,7 @@ class PropertySpec: QuickSpec {
 						var signalSentValue: String?
 						var producerCompleted = false
 						var signalInterrupted = false
+						var hasUnexpectedEventsEmitted = false
 
 						property.producer.start { event in
 							switch event {
@@ -422,25 +425,24 @@ class PropertySpec: QuickSpec {
 							case .completed:
 								producerCompleted = true
 							case .failed, .interrupted:
-								break
+								hasUnexpectedEventsEmitted = true
 							}
 						}
 
 						property.signal.observe { event in
 							switch event {
-							case let .value(value):
-								signalSentValue = value
 							case .interrupted:
 								signalInterrupted = true
-							case .failed, .completed:
-								break
+							case .value, .failed, .completed:
+								hasUnexpectedEventsEmitted = true
 							}
 						}
 
 						expect(sentValue) == initialPropertyValue
 						expect(signalSentValue).to(beNil())
 						expect(producerCompleted) == true
 						expect(signalInterrupted) == true
+						expect(hasUnexpectedEventsEmitted) == false
 					}
 
 					it("should not retain the wrapped property, and remain accessible after its the property being reflected has deinitialized.") {
@@ -592,26 +594,43 @@ class PropertySpec: QuickSpec {
 				describe("from a value and SignalProducer") {
 					it("should initially take on the supplied value") {
 						let property = Property(initial: initialPropertyValue,
-																		then: SignalProducer.never)
+						                        then: SignalProducer.never)
 
 						expect(property.value) == initialPropertyValue
 					}
 
 					it("should take on each value sent on the producer") {
 						let property = Property(initial: initialPropertyValue,
-																		then: SignalProducer(value: subsequentPropertyValue))
+						                        then: SignalProducer(value: subsequentPropertyValue))
 
 						expect(property.value) == subsequentPropertyValue
 					}
 
+					it("should complete its producer and signal even if the upstream interrupts") {
+						let (signal, observer) = Signal<String, NoError>.pipe()
+
+						let property = Property(initial: initialPropertyValue, then: SignalProducer(signal))
+
+						var isProducerCompleted = false
+						var isSignalCompleted = false
+
+						property.producer.startWithCompleted { isProducerCompleted = true }
+						property.signal.observeCompleted { isSignalCompleted = true }
+						expect(isProducerCompleted) == false
+						expect(isSignalCompleted) == false
+
+						observer.sendInterrupted()
+						expect(isProducerCompleted) == true
+						expect(isSignalCompleted) == true
+					}
+
 					it("should return a producer and a signal that respect the lifetime of its ultimate source") {
 						var signalCompleted = false
 						var producerCompleted = false
 						var signalInterrupted = false
 
 						let (signal, observer) = Signal<Int, NoError>.pipe()
-						var property: Property<Int>? = Property(initial: 1,
-						                                        then: SignalProducer(signal))
+						var property: Property<Int>? = Property(initial: 1, then: SignalProducer(signal))
 						let propertySignal = property!.signal
 
 						propertySignal.observeCompleted { signalCompleted = true }
@@ -641,8 +660,7 @@ class PropertySpec: QuickSpec {
 					it("should initially take on the supplied value, then values sent on the signal") {
 						let (signal, observer) = Signal<String, NoError>.pipe()
 
-						let property = Property(initial: initialPropertyValue,
-																		then: signal)
+						let property = Property(initial: initialPropertyValue, then: signal)
 
 						expect(property.value) == initialPropertyValue
 
@@ -651,15 +669,31 @@ class PropertySpec: QuickSpec {
 						expect(property.value) == subsequentPropertyValue
 					}
 
+					it("should complete its producer and signal even if the upstream interrupts") {
+						let (signal, observer) = Signal<String, NoError>.pipe()
+
+						let property = Property(initial: initialPropertyValue, then: signal)
+
+						var isProducerCompleted = false
+						var isSignalCompleted = false
+
+						property.producer.startWithCompleted { isProducerCompleted = true }
+						property.signal.observeCompleted { isSignalCompleted = true }
+						expect(isProducerCompleted) == false
+						expect(isSignalCompleted) == false
+
+						observer.sendInterrupted()
+						expect(isProducerCompleted) == true
+						expect(isSignalCompleted) == true
+					}
 
 					it("should return a producer and a signal that respect the lifetime of its ultimate source") {
 						var signalCompleted = false
 						var producerCompleted = false
 						var signalInterrupted = false
 
 						let (signal, observer) = Signal<Int, NoError>.pipe()
-						var property: Property<Int>? = Property(initial: 1,
-																											 then: signal)
+						var property: Property<Int>? = Property(initial: 1, then: signal)
 						let propertySignal = property!.signal
 
 						propertySignal.observeCompleted { signalCompleted = true }
@@ -1487,63 +1521,63 @@ class PropertySpec: QuickSpec {
 									break
 								}
 							}
-							
+
 							expect(receivedValues) == [ "0" ]
-							
+
 							outerProperty!.value = secondProperty!
 							secondProperty!.value = 11
 							outerProperty!.value = thirdProperty!
 							thirdProperty!.value = 21
-							
+
 							expect(receivedValues) == [ "0", "10", "11", "20", "21" ]
 							expect(errored) == false
 							expect(completed) == false
-							
+
 							secondProperty!.value = 12
 							secondProperty = nil
 							thirdProperty!.value = 22
 							thirdProperty = nil
-							
+
 							expect(receivedValues) == [ "0", "10", "11", "20", "21", "22" ]
 							expect(errored) == false
 							expect(completed) == false
-							
+
 							outerProperty = nil
 							expect(errored) == false
 							expect(completed) == true
 						}
 					}
 				}
-			}			
-			
+			}
+
 			describe("negated attribute") {
 				it("should return the negate of a value in a Boolean property") {
 					let property = MutableProperty(true)
 					expect(property.negate().value).to(beFalse())
 				}
 			}
-			
+
 			describe("and attribute") {
 				it("should emit true when both properties contains the same value") {
 					let property1 = MutableProperty(true)
 					let property2 = Property(MutableProperty(true))
 					expect(property1.and(property2).value).to(beTrue())
 				}
-				
+
 				it("should emit false when both properties contains opposite values") {
 					let property1 = MutableProperty(true)
 					let property2 = Property(MutableProperty(false))
 					expect(property1.and(property2).value).to(beFalse())
 				}
 			}
-			
+
 			describe("or attribute") {
 				it("should emit true when at least one of the properties contains true") {
 					let property1 = MutableProperty(true)
 					let property2 = Property(MutableProperty(false))
 					expect(property1.or(property2).value).to(beTrue())
 				}
-				
+
 				it("should emit false when both properties contains false") {
 					let property1 = MutableProperty(false)
 					let property2 = Property(MutableProperty(false))
@@ -1579,7 +1613,7 @@ class PropertySpec: QuickSpec {
 					observer.send(value: subsequentPropertyValue)
 					expect(mutableProperty.value) == initialPropertyValue
 				}
-				
+
 				it("should tear down the binding when the property deallocates") {
 					var signal: Signal<String, NoError>? = {
 						let (signal, _) = Signal<String, NoError>.pipe()