Merge pull request ReactiveCocoa#1628 from ReactiveCocoa/improve-actions

Improve Action class, add CocoaAction wrapper, remove all automatic scheduling

Author: joshaber

ReactiveCocoa/Swift/Action.swift

@@ -8,39 +8,57 @@
 
 import LlamaKit
 
-/// Represents a UI action that will perform some work when executed.
+/// Represents an action that will perform side effects or a transformation when
+/// executed with an input.
+///
+/// Actions must be executed serially. An attempt to execute the same action
+/// multiple times concurrently will fail.
 public final class Action<Input, Output> {
 	private let executeClosure: Input -> ColdSignal<Output>
-	private let scheduler: Scheduler
+	private let valuesSink: SinkOf<Output>
+	private let errorsSink: SinkOf<NSError>
 
-	private let _executing = ObservableProperty(false)
-	private let _enabled = ObservableProperty(false)
-	private let _values: SinkOf<Output>
-	private let _errors: SinkOf<NSError>
+	/// A signal of all values sent upon the signals returned from apply().
+	public let values: HotSignal<Output>
+
+	/// A signal of all errors (except `RACAction.ActionNotEnabled` errors) sent
+	/// upon the signals returned from apply().
+	public let errors: HotSignal<NSError>
 
 	/// Whether the action is currently executing.
 	///
-	/// This will send the current value immediately, then all future values on
-	/// the scheduler given at initialization time.
+	/// This signal will send the current value immediately (if the action is
+	/// alive), and complete when the action has deinitialized.
 	public var executing: ColdSignal<Bool> {
-		return _executing.values
+		return executingProperty.values
 	}
 
 	/// Whether the action is enabled.
 	///
-	/// This will send the current value immediately, then all future values on
-	/// the scheduler given at initialization time.
+	/// This signal will send the current value immediately (if the action is
+	/// alive), and complete when the action has deinitialized.
 	public var enabled: ColdSignal<Bool> {
-		return _enabled.values
+		return userEnabledProperty.values
+			.combineLatestWith(executingProperty.values)
+			.map(enabled)
 	}
 
-	/// A signal of all values generated from future calls to execute(), sent on
-	/// the scheduler given at initialization time.
-	public let values: HotSignal<Output>
-
-	/// A signal of errors generated from future executions, sent on the
-	/// scheduler given at initialization time.
-	public let errors: HotSignal<NSError>
+	// This queue serializes access to the properties below.
+	//
+	// Work which depends on consistent reads and writes of both properties
+	// should be enqueued as a barrier block.
+	//
+	// If only one property is being used, the work can be enqueued as a normal
+	// (non-barrier) block.
+	private let propertyQueue = dispatch_queue_create("org.reactivecocoa.ReactiveCocoa.Action", DISPATCH_QUEUE_SERIAL)
+	private let userEnabledProperty = ObservableProperty(false)
+	private let executingProperty = ObservableProperty(false)
+
+	/// Whether the action should be enabled when `userEnabledProperty` and
+	/// `executingProperty` have the given values.
+	private func enabled(#userEnabled: Bool, executing: Bool) -> Bool {
+		return userEnabled && !executing
+	}
 
 	/// The file in which this action was defined, if known.
 	internal let file: String?
@@ -55,75 +73,174 @@ public final class Action<Input, Output> {
 	/// a ColdSignal for each execution.
 	///
 	/// Before `enabledIf` sends a value, the command will be disabled.
-	public init(enabledIf: HotSignal<Bool>, serializedOnScheduler scheduler: Scheduler, _ execute: Input -> ColdSignal<Output>, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
+	public init(enabledIf: HotSignal<Bool>, _ execute: Input -> ColdSignal<Output>, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
 		self.file = file
 		self.line = line
 		self.function = function
-		self.scheduler = scheduler
 
-		(values, _values) = HotSignal.pipe()
-		(errors, _errors) = HotSignal.pipe()
+		(values, valuesSink) = HotSignal.pipe()
+		(errors, errorsSink) = HotSignal.pipe()
 		executeClosure = execute
 
-		_enabled <~! ColdSignal.single(false)
-			.concat(enabledIf
-				.replay(1)
-				.deliverOn(scheduler))
-			.combineLatestWith(executing)
-			.map { enabled, executing in enabled && !executing }
-	}
-
-	/// Initializes an action that will deliver all events on the main thread.
-	public convenience init(enabledIf: HotSignal<Bool>, _ execute: Input -> ColdSignal<Output>, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
-		self.init(enabledIf: enabledIf, serializedOnScheduler: MainScheduler(), execute, file: file, line: line, function: function)
+		userEnabledProperty <~ enabledIf
 	}
 
 	/// Initializes an action that will always be enabled.
-	public convenience init(serializedOnScheduler: Scheduler, _ execute: Input -> ColdSignal<Output>, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
+	public convenience init(_ execute: Input -> ColdSignal<Output>, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
 		let (enabled, enabledSink) = HotSignal<Bool>.pipe()
-		self.init(enabledIf: enabled, serializedOnScheduler: serializedOnScheduler, execute, file: file, line: line, function: function)
+		self.init(enabledIf: enabled, execute, file: file, line: line, function: function)
 
 		enabledSink.put(true)
 	}
 
-	/// Initializes an action that will always be enabled, and deliver all
-	/// events on the main thread.
-	public convenience init(_ execute: Input -> ColdSignal<Output>, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
-		self.init(serializedOnScheduler: MainScheduler(), execute, file: file, line: line, function: function)
-	}
-
-	/// Creates a signal that will execute the action on the scheduler given at
-	/// initialization time, with the given input, then forward the results.
+	/// Creates a signal that will execute the action with the given input, then
+	/// forward the results.
 	///
 	/// If the action is disabled when the returned signal is started, the
-	/// signal will send an `NSError` corresponding to
-	/// `RACError.ActionNotEnabled`, and no result will be sent along the action
-	/// itself.
-	public func execute(input: Input) -> ColdSignal<Output> {
+	/// returned signal will send an `NSError` corresponding to
+	/// `RACError.ActionNotEnabled`, and nothing will be sent upon `values` or
+	/// `errors`.
+	public func apply(input: Input) -> ColdSignal<Output> {
 		return ColdSignal<Output>.lazy {
-				let isEnabled = self.enabled.first().value()!
-				if (!isEnabled) {
-					return .error(RACError.ActionNotEnabled.error)
+			var startedExecuting = false
+
+			dispatch_barrier_sync(self.propertyQueue) {
+				if self.enabled(userEnabled: self.userEnabledProperty.value, executing: self.executingProperty.value) {
+					self.executingProperty.value = true
+					startedExecuting = true
 				}
+			}
 
-				return self.executeClosure(input)
-					.deliverOn(self.scheduler)
-					.on(started: {
-						self._executing.value = true
-					}, next: { value in
-						self._values.put(value)
-					}, error: { error in
-						self._errors.put(error)
-					}, disposed: {
-						self._executing.value = false
-					})
+			if !startedExecuting {
+				return .error(RACError.ActionNotEnabled.error)
 			}
-			.evaluateOn(scheduler)
+
+			return self.executeClosure(input)
+				.on(next: { value in
+					self.valuesSink.put(value)
+				}, error: { error in
+					self.errorsSink.put(error)
+				}, disposed: {
+					// This doesn't need to be a barrier because properties are
+					// themselves serialized, and this operation therefore won't
+					// conflict with other read-only or write-only operations.
+					//
+					// We still need to use the queue for mutual exclusion with
+					// the enabledness check above.
+					dispatch_async(self.propertyQueue) {
+						self.executingProperty.value = false
+					}
+				})
+		}
 	}
 }
 
 extension Action: DebugPrintable {
 	public var debugDescription: String {
+		let function = self.function ?? ""
+		let file = self.file ?? ""
+		let line = self.line?.description ?? ""
+
 		return "\(function).Action (\(file):\(line))"
 	}
 }
+
+/// Wraps an Action for use by a GUI control (such as `NSControl` or
+/// `UIControl`), with KVO, or with Cocoa Bindings.
+public final class CocoaAction: NSObject {
+	/// Whether the action is enabled.
+	///
+	/// This property will only change on the main thread, and will generate a
+	/// KVO notification for every change.
+	public var enabled: Bool {
+		return _enabled
+	}
+
+	/// Whether the action is executing.
+	///
+	/// This property will only change on the main thread, and will generate a
+	/// KVO notification for every change.
+	public var executing: Bool {
+		return _executing
+	}
+
+	/// The selector that a caller should invoke upon the receiver in order to
+	/// execute the action.
+	public let selector: Selector = "execute:"
+
+	private let _execute: AnyObject? -> ()
+	private var _enabled = false
+	private var _executing = false
+	private let disposable = CompositeDisposable()
+
+	private init(enabled: ColdSignal<Bool>, executing: ColdSignal<Bool>, execute: AnyObject? -> ()) {
+		_execute = execute
+
+		super.init()
+
+		startSignalOnMainThread(enabled) { [weak self] value in
+			self?.willChangeValueForKey("enabled")
+			self?._enabled = value
+			self?.didChangeValueForKey("enabled")
+		}
+
+		startSignalOnMainThread(executing) { [weak self] value in
+			self?.willChangeValueForKey("executing")
+			self?._executing = value
+			self?.didChangeValueForKey("executing")
+		}
+	}
+
+	/// Initializes a Cocoa action that will always invoke the given action with
+	/// an input of ().
+	public convenience init<Output>(_ action: Action<(), Output>) {
+		self.init(enabled: action.enabled, executing: action.executing, execute: { object in
+			action.apply(()).start()
+			return
+		})
+	}
+
+	/// Initializes a Cocoa action that will always invoke the given action with
+	/// an input of nil.
+	public convenience init<Input: NilLiteralConvertible, Output>(_ action: Action<Input, Output>) {
+		self.init(enabled: action.enabled, executing: action.executing, execute: { object in
+			action.apply(nil).start()
+			return
+		})
+	}
+
+	/// Initializes a Cocoa action that will invoke the given action with the
+	/// object given to execute() if it can be downcast successfully, or nil
+	/// otherwise.
+	public convenience init<Input: AnyObject, Output>(_ action: Action<Input?, Output>) {
+		self.init(enabled: action.enabled, executing: action.executing, execute: { object in
+			action.apply(object as? Input).start()
+			return
+		})
+	}
+
+	deinit {
+		self.disposable.dispose()
+	}
+
+	/// Starts the given signal, delivering its uniqued values to the main
+	/// thread and executing the given closure for each one.
+	private func startSignalOnMainThread<T: Equatable>(signal: ColdSignal<T>, next: T -> ()) {
+		let signalDisposable = signal
+			.skipRepeats(identity)
+			.deliverOn(MainScheduler())
+			.start(next: next)
+
+		self.disposable.addDisposable(signalDisposable)
+	}
+
+	/// Attempts to execute the underlying action with the given input, subject
+	/// to the behavior described by the initializer that was used.
+	@IBAction public func execute(input: AnyObject?) {
+		_execute(input)
+	}
+
+	public override class func automaticallyNotifiesObserversForKey(key: String) -> Bool {
+		return false
+	}
+}

ReactiveCocoa/Swift/ObservableProperty.swift

@@ -10,9 +10,12 @@ import Foundation
 import LlamaKit
 
 /// A mutable property of type T that allows observation of its changes.
+///
+/// Instances of this class are thread-safe.
 public final class ObservableProperty<T> {
-	private let queue = dispatch_queue_create("org.reactivecocoa.ReactiveCocoa.ObservableProperty", DISPATCH_QUEUE_SERIAL)
+	private let queue = dispatch_queue_create("org.reactivecocoa.ReactiveCocoa.ObservableProperty", DISPATCH_QUEUE_CONCURRENT)
 	private var sinks = Bag<SinkOf<Event<T>>>()
+	private var _value: T
 
 	/// The file in which this property was defined, if known.
 	internal let file: String?
@@ -28,10 +31,22 @@ public final class ObservableProperty<T> {
 	/// Setting this to a new value will notify all sinks attached to
 	/// `values`.
 	public var value: T {
-		didSet {
+		get {
+			var readValue: T?
+
 			dispatch_sync(queue) {
+				readValue = self._value
+			}
+
+			return readValue!
+		}
+
+		set(value) {
+			dispatch_barrier_sync(queue) {
+				self._value = value
+
 				for sink in self.sinks {
-					sink.put(.Next(Box(self.value)))
+					sink.put(.Next(Box(value)))
 				}
 			}
 		}
@@ -45,14 +60,14 @@ public final class ObservableProperty<T> {
 			if let strongSelf = self {
 				var token: RemovalToken?
 
-				dispatch_sync(strongSelf.queue) {
+				dispatch_barrier_sync(strongSelf.queue) {
 					token = strongSelf.sinks.insert(sink)
-					sink.put(.Next(Box(strongSelf.value)))
+					sink.put(.Next(Box(strongSelf._value)))
 				}
 
 				disposable.addDisposable {
 					if let strongSelf = self {
-						dispatch_async(strongSelf.queue) {
+						dispatch_barrier_async(strongSelf.queue) {
 							strongSelf.sinks.removeValueForToken(token!)
 						}
 					}
@@ -64,14 +79,15 @@ public final class ObservableProperty<T> {
 	}
 
 	public init(_ value: T, file: String = __FILE__, line: Int = __LINE__, function: String = __FUNCTION__) {
-		self.value = value
+		_value = value
+
 		self.file = file
 		self.line = line
 		self.function = function
 	}
 
 	deinit {
-		dispatch_sync(queue) {
+		dispatch_barrier_sync(queue) {
 			for sink in self.sinks {
 				sink.put(.Completed)
 			}
@@ -87,6 +103,10 @@ extension ObservableProperty: SinkType {
 
 extension ObservableProperty: DebugPrintable {
 	public var debugDescription: String {
+		let function = self.function ?? ""
+		let file = self.file ?? ""
+		let line = self.line?.description ?? ""
+
 		return "\(function).ObservableProperty (\(file):\(line))"
 	}
 }