Merge pull request #48 from liscio/cl-dispatchtimeinterval

Use DispatchTimeInterval to offset Date values
ReactiveCocoa · Nov 6, 2016 · 5b49431 · 5b49431
2 parents 7ad4b11 + b395576
commit 5b49431
Show file tree

Hide file tree

Showing 9 changed files with 184 additions and 62 deletions.
diff --git a/Sources/Deprecations+Removals.swift b/Sources/Deprecations+Removals.swift
@@ -315,6 +315,12 @@ extension DateSchedulerProtocol {
 
 	@available(*, unavailable, renamed:"schedule(after:interval:leeway:)")
 	func scheduleAfter(date: Date, repeatingEvery: TimeInterval, withLeeway: TimeInterval, action: () -> Void) -> Disposable? { fatalError() }
+
+	@available(*, unavailable, message:"schedule(after:interval:leeway:action:) now uses DispatchTimeInterval")
+	func schedule(after date: Date, interval: TimeInterval, leeway: TimeInterval, action: @escaping () -> Void) -> Disposable? { fatalError() }
+
+	@available(*, unavailable, message:"schedule(after:interval:action:) now uses DispatchTimeInterval")
+	public func schedule(after date: Date, interval: TimeInterval, action: @escaping () -> Void) -> Disposable? { fatalError() }
 }
 
 extension TestScheduler {
@@ -323,6 +329,12 @@ extension TestScheduler {
 
 	@available(*, unavailable, renamed:"advance(to:)")
 	public func advanceToDate(_ date: Date) { fatalError() }
+
+	@available(*, unavailable, message:"advance(by:) now uses DispatchTimeInterval")
+	public func advance(by interval: TimeInterval) { fatalError() }
+
+	@available(*, unavailable, message:"rewind(by:) now uses DispatchTimeInterval")
+	public func rewind(by interval: TimeInterval) { fatalError() }
 }
 
 extension QueueScheduler {
@@ -342,6 +354,12 @@ extension URLSession {
 
 // Free functions
 
+@available(*, unavailable, message:"timer(interval:on:) now uses DispatchTimeInterval")
+public func timer(interval: TimeInterval, on scheduler: DateSchedulerProtocol) -> SignalProducer<Date, NoError> { fatalError() }
+
+@available(*, unavailable, message:"timer(interval:on:leeway:) now uses DispatchTimeInterval")
+public func timer(interval: TimeInterval, on scheduler: DateSchedulerProtocol, leeway: TimeInterval) -> SignalProducer<Date, NoError> { fatalError() }
+
 @available(*, unavailable, renamed:"Signal.combineLatest")
 public func combineLatest<A, B, Error>(_ a: Signal<A, Error>, _ b: Signal<B, Error>) -> Signal<(A, B), Error> { fatalError() }
 

diff --git a/Sources/FoundationExtensions.swift b/Sources/FoundationExtensions.swift
@@ -7,8 +7,14 @@
 //
 
 import Foundation
+import Dispatch
 import enum Result.NoError
 
+#if os(Linux)
+	import let CDispatch.NSEC_PER_USEC
+	import let CDispatch.NSEC_PER_SEC
+#endif
+
 extension NotificationCenter: ReactiveExtensionsProvider {}
 
 extension Reactive where Base: NotificationCenter {
@@ -73,3 +79,61 @@ extension Reactive where Base: URLSession {
 		}
 	}
 }
+
+extension Date {
+	internal func addingTimeInterval(_ interval: DispatchTimeInterval) -> Date {
+		return addingTimeInterval(interval.timeInterval)
+	}
+}
+
+extension DispatchTimeInterval {
+	internal var timeInterval: TimeInterval {
+		switch self {
+		case let .seconds(s):
+			return TimeInterval(s)
+		case let .milliseconds(ms):
+			return TimeInterval(TimeInterval(ms) / 1000.0)
+		case let .microseconds(us):
+			return TimeInterval( UInt64(us) * NSEC_PER_USEC ) / TimeInterval(NSEC_PER_SEC)
+		case let .nanoseconds(ns):
+			return TimeInterval(ns) / TimeInterval(NSEC_PER_SEC)
+		}
+	}
+
+	// This was added purely so that our test scheduler to "go backwards" in
+	// time. See `TestScheduler.rewind(by interval: DispatchTimeInterval)`.
+	internal static prefix func -(lhs: DispatchTimeInterval) -> DispatchTimeInterval {
+		switch lhs {
+		case let .seconds(s):
+			return .seconds(-s)
+		case let .milliseconds(ms):
+			return .milliseconds(-ms)
+		case let .microseconds(us):
+			return .microseconds(-us)
+		case let .nanoseconds(ns):
+			return .nanoseconds(-ns)
+		}
+	}
+
+	/// Scales a time interval by the given scalar specified in `rhs`.
+	///
+	/// - note: This method is only used internally to "scale down" a time 
+	///			interval. Specifically it's used only to scale intervals to 10% 
+	///			of their original value for the default `leeway` parameter in 
+	///			`SchedulerProtocol.schedule(after:action:)` schedule and similar
+	///			other methods.
+	///
+	///			If seconds is over 200,000, 10% is ~2,000, and hence we end up
+	///			with a value of ~2,000,000,000. Not quite overflowing a signed
+	///			integer on 32-bit platforms, but close.
+	///
+	///			Even still, 200,000 seconds should be a rarely (if ever)
+	///			specified interval for our APIs. And even then, folks should be
+	///			smart and specify their own `leeway` parameter.
+	///
+	/// - returns: Scaled interval in microseconds
+	internal static func *(lhs: DispatchTimeInterval, rhs: Double) -> DispatchTimeInterval {
+		let seconds = lhs.timeInterval * rhs
+		return .microseconds(Int(seconds * 1000 * 1000))
+	}
+}
diff --git a/Sources/Scheduler.swift b/Sources/Scheduler.swift
@@ -54,10 +54,14 @@ public protocol DateSchedulerProtocol: SchedulerProtocol {
 	///   - withLeeway: Some delta for repetition.
 	///   - action: Closure of the action to perform.
 	///
+	///	- note: If you plan to specify an `interval` value greater than 200,000
+	///			seconds, use `schedule(after:interval:leeway:action)` instead
+	///			and specify your own `leeway` value to avoid potential overflow.
+	///
 	/// - returns: Optional `Disposable` that can be used to cancel the work
 	///            before it begins.
 	@discardableResult
-	func schedule(after date: Date, interval: TimeInterval, leeway: TimeInterval, action: @escaping () -> Void) -> Disposable?
+	func schedule(after date: Date, interval: DispatchTimeInterval, leeway: DispatchTimeInterval, action: @escaping () -> Void) -> Disposable?
 }
 
 /// A scheduler that performs all work synchronously.
@@ -263,10 +267,14 @@ public final class QueueScheduler: DateSchedulerProtocol {
 	///   - repeatingEvery: Repetition interval.
 	///   - action: Closure of the action to repeat.
 	///
+	///	- note: If you plan to specify an `interval` value greater than 200,000 
+	///			seconds, use `schedule(after:interval:leeway:action)` instead 
+	///			and specify your own `leeway` value to avoid potential overflow.
+	///
 	/// - returns: Optional disposable that can be used to cancel the work
 	///            before it begins.
 	@discardableResult
-	public func schedule(after date: Date, interval: TimeInterval, action: @escaping () -> Void) -> Disposable? {
+	public func schedule(after date: Date, interval: DispatchTimeInterval, action: @escaping () -> Void) -> Disposable? {
 		// Apple's "Power Efficiency Guide for Mac Apps" recommends a leeway of
 		// at least 10% of the timer interval.
 		return schedule(after: date, interval: interval, leeway: interval * 0.1, action: action)
@@ -284,20 +292,17 @@ public final class QueueScheduler: DateSchedulerProtocol {
 	/// - returns: Optional `Disposable` that can be used to cancel the work
 	///            before it begins.
 	@discardableResult
-	public func schedule(after date: Date, interval: TimeInterval, leeway: TimeInterval, action: @escaping () -> Void) -> Disposable? {
-		precondition(interval >= 0)
-		precondition(leeway >= 0)
-
-		let msecInterval = interval * 1000
-		let msecLeeway = leeway * 1000
+	public func schedule(after date: Date, interval: DispatchTimeInterval, leeway: DispatchTimeInterval, action: @escaping () -> Void) -> Disposable? {
+		precondition(interval.timeInterval >= 0)
+		precondition(leeway.timeInterval >= 0)
 
 		let timer = DispatchSource.makeTimerSource(
 			flags: DispatchSource.TimerFlags(rawValue: UInt(0)),
 			queue: queue
 		)
 		timer.scheduleRepeating(wallDeadline: wallTime(with: date),
-		                        interval: .milliseconds(Int(msecInterval)),
-		                        leeway: .milliseconds(Int(msecLeeway)))
+		                        interval: interval,
+		                        leeway: leeway)
 		timer.setEventHandler(handler: action)
 		timer.resume()
 
@@ -386,7 +391,7 @@ public final class TestScheduler: DateSchedulerProtocol {
 	/// - returns: Optional disposable that can be used to cancel the work
 	///            before it begins.
 	@discardableResult
-	public func schedule(after delay: TimeInterval, action: @escaping () -> Void) -> Disposable? {
+	public func schedule(after delay: DispatchTimeInterval, action: @escaping () -> Void) -> Disposable? {
 		return schedule(after: currentDate.addingTimeInterval(delay), action: action)
 	}
 
@@ -403,10 +408,14 @@ public final class TestScheduler: DateSchedulerProtocol {
 	///   - repeatingEvery: Repetition interval.
 	///   - action: Closure of the action to repeat.
 	///
+	///	- note: If you plan to specify an `interval` value greater than 200,000
+	///			seconds, use `schedule(after:interval:leeway:action)` instead
+	///			and specify your own `leeway` value to avoid potential overflow.
+	///
 	/// - returns: Optional `Disposable` that can be used to cancel the work
 	///            before it begins.
-	private func schedule(after date: Date, interval: TimeInterval, disposable: SerialDisposable, action: @escaping () -> Void) {
-		precondition(interval >= 0)
+	private func schedule(after date: Date, interval: DispatchTimeInterval, disposable: SerialDisposable, action: @escaping () -> Void) {
+		precondition(interval.timeInterval >= 0)
 
 		disposable.innerDisposable = schedule(after: date) { [unowned self] in
 			action()
@@ -426,7 +435,7 @@ public final class TestScheduler: DateSchedulerProtocol {
 	/// - returns: Optional `Disposable` that can be used to cancel the work
 	///            before it begins.
 	@discardableResult
-	public func schedule(after delay: TimeInterval, interval: TimeInterval, leeway: TimeInterval = 0, action: @escaping () -> Void) -> Disposable? {
+	public func schedule(after delay: DispatchTimeInterval, interval: DispatchTimeInterval, leeway: DispatchTimeInterval = .seconds(0), action: @escaping () -> Void) -> Disposable? {
 		return schedule(after: currentDate.addingTimeInterval(delay), interval: interval, leeway: leeway, action: action)
 	}
 
@@ -441,7 +450,7 @@ public final class TestScheduler: DateSchedulerProtocol {
 	///
 	/// - returns: Optional `Disposable` that can be used to cancel the work
 	///	           before it begins.
-	public func schedule(after date: Date, interval: TimeInterval, leeway: TimeInterval = 0, action: @escaping () -> Void) -> Disposable? {
+	public func schedule(after date: Date, interval: DispatchTimeInterval, leeway: DispatchTimeInterval = .seconds(0), action: @escaping () -> Void) -> Disposable? {
 		let disposable = SerialDisposable()
 		schedule(after: date, interval: interval, disposable: disposable, action: action)
 		return disposable
@@ -453,15 +462,15 @@ public final class TestScheduler: DateSchedulerProtocol {
 	/// This is intended to be used as a way to execute actions that have been
 	/// scheduled to run as soon as possible.
 	public func advance() {
-		advance(by: Double.ulpOfOne)
+		advance(by: .nanoseconds(1))
 	}
 
 	/// Advances the virtualized clock by the given interval, dequeuing and
 	/// executing any actions along the way.
 	///
 	/// - parameters:
 	///   - interval: Interval by which the current date will be advanced.
-	public func advance(by interval: TimeInterval) {
+	public func advance(by interval: DispatchTimeInterval) {
 		lock.lock()
 		advance(to: currentDate.addingTimeInterval(interval))
 		lock.unlock()
@@ -504,7 +513,7 @@ public final class TestScheduler: DateSchedulerProtocol {
 	///
 	/// - parameters:
 	///   - interval: Interval by which the current date will be retreated.
-	public func rewind(by interval: TimeInterval) {
+	public func rewind(by interval: DispatchTimeInterval) {
 		lock.lock()
 
 		let newDate = currentDate.addingTimeInterval(-interval)

diff --git a/Sources/SignalProducer.swift b/Sources/SignalProducer.swift
@@ -1919,12 +1919,16 @@ private struct ReplayState<Value, Error: Swift.Error> {
 ///
 /// - precondition: Interval must be non-negative number.
 ///
+///	- note: If you plan to specify an `interval` value greater than 200,000
+///			seconds, use `timer(interval:on:leeway:)` instead
+///			and specify your own `leeway` value to avoid potential overflow.
+///
 /// - parameters:
 ///   - interval: An interval between invocations.
 ///   - scheduler: A scheduler to deliver events on.
 ///
 /// - returns: A producer that sends `NSDate` values every `interval` seconds.
-public func timer(interval: TimeInterval, on scheduler: DateSchedulerProtocol) -> SignalProducer<Date, NoError> {
+public func timer(interval: DispatchTimeInterval, on scheduler: DateSchedulerProtocol) -> SignalProducer<Date, NoError> {
 	// Apple's "Power Efficiency Guide for Mac Apps" recommends a leeway of
 	// at least 10% of the timer interval.
 	return timer(interval: interval, on: scheduler, leeway: interval * 0.1)
@@ -1947,9 +1951,9 @@ public func timer(interval: TimeInterval, on scheduler: DateSchedulerProtocol) -
 ///             recommends a leeway of at least 10% of the timer interval.
 ///
 /// - returns: A producer that sends `NSDate` values every `interval` seconds.
-public func timer(interval: TimeInterval, on scheduler: DateSchedulerProtocol, leeway: TimeInterval) -> SignalProducer<Date, NoError> {
-	precondition(interval >= 0)
-	precondition(leeway >= 0)
+public func timer(interval: DispatchTimeInterval, on scheduler: DateSchedulerProtocol, leeway: DispatchTimeInterval) -> SignalProducer<Date, NoError> {
+	precondition(interval.timeInterval >= 0)
+	precondition(leeway.timeInterval >= 0)
 
 	return SignalProducer { observer, compositeDisposable in
 		compositeDisposable += scheduler.schedule(after: scheduler.currentDate.addingTimeInterval(interval),

diff --git a/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift b/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift
@@ -7,11 +7,11 @@
 //
 
 import Foundation
-
+import Dispatch
 import Result
 import Nimble
 import Quick
-import ReactiveSwift
+@testable import ReactiveSwift
 
 extension Notification.Name {
 	static let racFirst = Notification.Name(rawValue: "rac_notifications_test")
@@ -85,5 +85,32 @@ class FoundationExtensionsSpec: QuickSpec {
 				expect(signal).toNot(beNil())
 			}
 		}
+
+		describe("DispatchTimeInterval") {
+			it("should scale time values as expected") {
+				expect((DispatchTimeInterval.seconds(1) * 0.1).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(100).timeInterval))
+				expect((DispatchTimeInterval.milliseconds(100) * 0.1).timeInterval).to(beCloseTo(DispatchTimeInterval.microseconds(10000).timeInterval))
+
+				expect((DispatchTimeInterval.seconds(5) * 0.5).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(2500).timeInterval))
+				expect((DispatchTimeInterval.seconds(1) * 0.25).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(250).timeInterval))
+			}
+
+			it("should produce the expected TimeInterval values") {
+				expect(DispatchTimeInterval.seconds(1).timeInterval).to(beCloseTo(1.0))
+				expect(DispatchTimeInterval.milliseconds(1).timeInterval).to(beCloseTo(0.001))
+				expect(DispatchTimeInterval.microseconds(1).timeInterval).to(beCloseTo(0.000001, within: 0.0000001))
+				expect(DispatchTimeInterval.nanoseconds(1).timeInterval).to(beCloseTo(0.000000001, within: 0.0000000001))
+
+				expect(DispatchTimeInterval.milliseconds(500).timeInterval).to(beCloseTo(0.5))
+				expect(DispatchTimeInterval.milliseconds(250).timeInterval).to(beCloseTo(0.25))
+			}
+
+			it("should negate as you'd hope") {
+				expect(-DispatchTimeInterval.seconds(1).timeInterval).to(beCloseTo(-1.0))
+				expect(-DispatchTimeInterval.milliseconds(1).timeInterval).to(beCloseTo(-0.001))
+				expect(-DispatchTimeInterval.microseconds(1).timeInterval).to(beCloseTo(-0.000001, within: 0.0000001))
+				expect(-DispatchTimeInterval.nanoseconds(1).timeInterval).to(beCloseTo(-0.000000001, within: 0.0000000001))
+			}
+		}
 	}
 }
diff --git a/Tests/ReactiveSwiftTests/SchedulerSpec.swift b/Tests/ReactiveSwiftTests/SchedulerSpec.swift
@@ -203,7 +203,7 @@ class SchedulerSpec: QuickSpec {
 					var count = 0
 					let timesToRun = 3
 
-					disposable.innerDisposable = scheduler.schedule(after: Date(), interval: 0.01, leeway: 0) {
+					disposable.innerDisposable = scheduler.schedule(after: Date(), interval: .milliseconds(10), leeway: .seconds(0)) {
 						expect(Thread.isMainThread) == false
 
 						count += 1
@@ -259,38 +259,38 @@ class SchedulerSpec: QuickSpec {
 			it("should run actions when advanced past the target date") {
 				var string = ""
 
-				scheduler.schedule(after: 15) { [weak scheduler] in
+				scheduler.schedule(after: .seconds(15)) { [weak scheduler] in
 					string += "bar"
 					expect(Thread.isMainThread) == true
 					expect(scheduler?.currentDate).to(beCloseTo(startDate.addingTimeInterval(15), within: dateComparisonDelta))
 				}
 
-				scheduler.schedule(after: 5) { [weak scheduler] in
+				scheduler.schedule(after: .seconds(5)) { [weak scheduler] in
 					string += "foo"
 					expect(Thread.isMainThread) == true
 					expect(scheduler?.currentDate).to(beCloseTo(startDate.addingTimeInterval(5), within: dateComparisonDelta))
 				}
 
 				expect(string) == ""
 
-				scheduler.advance(by: 10)
+				scheduler.advance(by: .seconds(10))
 				expect(scheduler.currentDate).to(beCloseTo(startDate.addingTimeInterval(10), within: TimeInterval(dateComparisonDelta)))
 				expect(string) == "foo"
 
-				scheduler.advance(by: 10)
+				scheduler.advance(by: .seconds(10))
 				expect(scheduler.currentDate).to(beCloseTo(startDate.addingTimeInterval(20), within: dateComparisonDelta))
 				expect(string) == "foobar"
 			}
 
 			it("should run all remaining actions in order") {
 				var string = ""
 
-				scheduler.schedule(after: 15) {
+				scheduler.schedule(after: .seconds(15)) {
 					string += "bar"
 					expect(Thread.isMainThread) == true
 				}
 
-				scheduler.schedule(after: 5) {
+				scheduler.schedule(after: .seconds(5)) {
 					string += "foo"
 					expect(Thread.isMainThread) == true
 				}