From 9748defff4859c4b2064c23a6ebf1a5925b8cbd4 Mon Sep 17 00:00:00 2001 From: Lucas Nelaupe <7482647+lucas34@users.noreply.github.com> Date: Mon, 22 Jan 2018 09:23:43 +0800 Subject: [PATCH] Fix an issue where a periodic job will not be re-scheduled (#63) --- Sources/SwiftQueue/Job.swift | 4 +- Sources/SwiftQueue/JobInfo.swift | 12 +++--- Sources/SwiftQueue/SwiftQueueJob.swift | 26 +++++++---- Sources/SwiftQueue/Utils.swift | 33 +++++++++++++- Tests/SwiftQueueTests/ConstraintTests.swift | 43 +++++++++++++++++++ .../SwiftQueueBuilderTests.swift | 8 ++-- Tests/SwiftQueueTests/TestUtils.swift | 12 ++++++ 7 files changed, 116 insertions(+), 22 deletions(-) diff --git a/Sources/SwiftQueue/Job.swift b/Sources/SwiftQueue/Job.swift index 706a28a9..52a45887 100644 --- a/Sources/SwiftQueue/Job.swift +++ b/Sources/SwiftQueue/Job.swift @@ -56,7 +56,7 @@ public final class JobBuilder { public func periodic(limit: Limit = .unlimited, interval: TimeInterval = 0) -> Self { assert(interval >= 0) - info.maxRun = limit.intValue + info.maxRun = limit info.interval = interval return self } @@ -82,7 +82,7 @@ public final class JobBuilder { /// Limit number of retry. Overall for the lifecycle of the SwiftQueueManager. /// For a periodic job, the retry count will not be reset at each period. public func retry(limit: Limit) -> Self { - info.retries = limit.intValue + info.retries = limit return self } diff --git a/Sources/SwiftQueue/JobInfo.swift b/Sources/SwiftQueue/JobInfo.swift index 81b432a3..eeed87b8 100644 --- a/Sources/SwiftQueue/JobInfo.swift +++ b/Sources/SwiftQueue/JobInfo.swift @@ -27,9 +27,9 @@ struct JobInfo { var createTime: Date = Date() var interval: TimeInterval = -1.0 - var maxRun: Int = 0 + var maxRun: Limit = .limited(0) - var retries: Int = 0 + var retries: Limit = .limited(0) var runCount: Int = 0 var currentRepetition: Int = 0 // Do not serialize @@ -65,9 +65,9 @@ struct JobInfo { dictionary.assign(&self.createTime, key: "createTime", dateFormatter.date) dictionary.assign(&self.interval, key: "interval") - dictionary.assign(&self.maxRun, key: "maxRun") + dictionary.assign(&self.maxRun, key: "maxRun", Limit.fromIntValue) - dictionary.assign(&self.retries, key: "retries") + dictionary.assign(&self.retries, key: "retries", Limit.fromIntValue) dictionary.assign(&self.runCount, key: "runCount") } @@ -86,8 +86,8 @@ struct JobInfo { dict["params"] = self.params dict["createTime"] = dateFormatter.string(from: self.createTime) dict["runCount"] = self.runCount - dict["maxRun"] = self.maxRun - dict["retries"] = self.retries + dict["maxRun"] = self.maxRun.intValue + dict["retries"] = self.retries.intValue dict["interval"] = self.interval return dict } diff --git a/Sources/SwiftQueue/SwiftQueueJob.swift b/Sources/SwiftQueue/SwiftQueueJob.swift index 982f3448..18ba8bde 100644 --- a/Sources/SwiftQueue/SwiftQueueJob.swift +++ b/Sources/SwiftQueue/SwiftQueueJob.swift @@ -121,10 +121,16 @@ internal final class SwiftQueueJob: Operation, JobResult { private func completionFail(error: Swift.Error) { lastError = error - if info.retries > 0 { + switch info.retries { + case .limited(let value): + if value > 0 { + retryJob(retry: handler.onRetry(error: error)) + } else { + onTerminate() + } + break + case .unlimited: retryJob(retry: handler.onRetry(error: error)) - } else { - onTerminate() } } @@ -135,21 +141,21 @@ internal final class SwiftQueueJob: Operation, JobResult { case .retry(let after): guard after > 0 else { // Retry immediately - info.retries -= 1 + info.retries.decreaseValue(by: 1) self.run() return } // Retry after time in parameter runInBackgroundAfter(after) { [weak self] in - self?.info.retries -= 1 + self?.info.retries.decreaseValue(by: 1) self?.run() } case .exponential(let initial): info.currentRepetition += 1 let delay = info.currentRepetition == 1 ? initial : initial * pow(2, Double(info.currentRepetition - 1)) runInBackgroundAfter(delay) { [weak self] in - self?.info.retries -= 1 + self?.info.retries.decreaseValue(by: 1) self?.run() } } @@ -159,10 +165,12 @@ internal final class SwiftQueueJob: Operation, JobResult { lastError = nil info.currentRepetition = 0 - guard info.runCount + 1 < info.maxRun else { + if case .limited(let limit) = info.maxRun { // Reached run limit - onTerminate() - return + guard info.runCount + 1 < limit else { + onTerminate() + return + } } guard info.interval > 0 else { diff --git a/Sources/SwiftQueue/Utils.swift b/Sources/SwiftQueue/Utils.swift index 44182b69..0b85f09c 100644 --- a/Sources/SwiftQueue/Utils.swift +++ b/Sources/SwiftQueue/Utils.swift @@ -33,7 +33,15 @@ func assertNotEmptyString(_ string: @autoclosure () -> String, file: StaticStrin internal extension Limit { - internal var intValue: Int { + static func fromIntValue(value: Int) -> Limit { + if value < 0 { + return Limit.unlimited + } else { + return Limit.limited(value) + } + } + + var intValue: Int { switch self { case .unlimited: return -1 @@ -43,5 +51,28 @@ internal extension Limit { } } + + mutating func decreaseValue(by: Int) { + if case .limited(let limit) = self { + let value = limit - by + assert(value >= 0) + self = Limit.limited(value) + } + } } + + + extension Limit: Equatable { + + public static func ==(lhs: Limit, rhs: Limit) -> Bool { + switch (lhs, rhs) { + case let (.limited(a), .limited(b)): + return a == b + case (.unlimited, .unlimited): + return true + default: + return false + } + } +} \ No newline at end of file diff --git a/Tests/SwiftQueueTests/ConstraintTests.swift b/Tests/SwiftQueueTests/ConstraintTests.swift index bf431f01..420a114f 100644 --- a/Tests/SwiftQueueTests/ConstraintTests.swift +++ b/Tests/SwiftQueueTests/ConstraintTests.swift @@ -27,6 +27,27 @@ class ConstraintTests: XCTestCase { XCTAssertEqual(job.onCancelCalled, 0) } + func testPeriodicJobUnlimited() { + let job = TestJob() + let type = UUID().uuidString + + let creator = TestCreator([type: job]) + + let manager = SwiftQueueManager(creators: [creator]) + JobBuilder(type: type) + .periodic(limit: .unlimited) + .schedule(manager: manager) + + // Should run at least 100 times + job.awaitRun(value: 1000) + + // Semaphore is async so the value is un-predicable + XCTAssertTrue(job.onRunJobCalled > 50) + XCTAssertEqual(job.onCompleteCalled, 0) + XCTAssertEqual(job.onRetryCalled, 0) + XCTAssertEqual(job.onCancelCalled, 0) + } + func testRetryFailJobWithRetryConstraint() { let job = TestJob() let type = UUID().uuidString @@ -71,6 +92,28 @@ class ConstraintTests: XCTestCase { XCTAssertEqual(job.onCancelCalled, 1) } + func testRetryUnlimitedShouldRetryManyTimes() { + let job = TestJob() + let type = UUID().uuidString + + let creator = TestCreator([type: job]) + + job.result = JobError() + job.retryConstraint = .retry(delay: 0) + + let manager = SwiftQueueManager(creators: [creator]) + JobBuilder(type: type) + .retry(limit: .unlimited) + .schedule(manager: manager) + + job.awaitRun(value: 10000) + + XCTAssertTrue(job.onRunJobCalled > 50) + XCTAssertEqual(job.onCompleteCalled, 0) + XCTAssertTrue(job.onRetryCalled > 50) + XCTAssertEqual(job.onCancelCalled, 0) + } + func testRetryFailJobWithCancelConstraint() { let job = TestJob() let type = UUID().uuidString diff --git a/Tests/SwiftQueueTests/SwiftQueueBuilderTests.swift b/Tests/SwiftQueueTests/SwiftQueueBuilderTests.swift index c1893198..f0336284 100644 --- a/Tests/SwiftQueueTests/SwiftQueueBuilderTests.swift +++ b/Tests/SwiftQueueTests/SwiftQueueBuilderTests.swift @@ -64,7 +64,7 @@ class SwiftQueueBuilderTests: XCTestCase { let interval: Double = 12341 let jobInfo = toJobInfo(type: type, JobBuilder(type: type).periodic(limit: .unlimited, interval: interval)) - XCTAssertEqual(jobInfo?.maxRun, -1) + XCTAssertEqual(jobInfo?.maxRun, Limit.unlimited) XCTAssertEqual(jobInfo?.interval, interval) } @@ -74,7 +74,7 @@ class SwiftQueueBuilderTests: XCTestCase { let interval: Double = 12342 let jobInfo = toJobInfo(type: type, JobBuilder(type: type).periodic(limit: .limited(limited), interval: interval)) - XCTAssertEqual(jobInfo?.maxRun, limited) + XCTAssertEqual(jobInfo?.maxRun, Limit.limited(limited)) XCTAssertEqual(jobInfo?.interval, interval) } @@ -106,7 +106,7 @@ class SwiftQueueBuilderTests: XCTestCase { let type = UUID().uuidString let jobInfo = toJobInfo(type: type, JobBuilder(type: type).retry(limit: .unlimited)) - XCTAssertEqual(jobInfo?.retries, -1) + XCTAssertEqual(jobInfo?.retries, Limit.unlimited) } public func testBuilderRetryLimited() { @@ -114,7 +114,7 @@ class SwiftQueueBuilderTests: XCTestCase { let limited = 123 let jobInfo = toJobInfo(type: type, JobBuilder(type: type).retry(limit: .limited(limited))) - XCTAssertEqual(jobInfo?.retries, limited) + XCTAssertEqual(jobInfo?.retries, Limit.limited(limited)) } public func testBuilderAddTag() { diff --git a/Tests/SwiftQueueTests/TestUtils.swift b/Tests/SwiftQueueTests/TestUtils.swift index 62b540c3..89bdc6b2 100644 --- a/Tests/SwiftQueueTests/TestUtils.swift +++ b/Tests/SwiftQueueTests/TestUtils.swift @@ -25,12 +25,18 @@ class TestJob: Job { public let completionTimeout: TimeInterval + var runSemaphoreValue = 0 + let runSemaphore = DispatchSemaphore(value: 0) + init(_ completionTimeout: TimeInterval = 0) { self.completionTimeout = completionTimeout } func onRun(callback: JobResult) { onRunJobCalled += 1 + if runSemaphoreValue == onRunJobCalled { + runSemaphore.signal() + } runInBackgroundAfter(completionTimeout) { if let error = self.result { callback.done(.fail(error)) @@ -63,6 +69,12 @@ class TestJob: Job { let delta = DispatchTime.now() + Double(Int64(seconds) * Int64(NSEC_PER_SEC)) / Double(NSEC_PER_SEC) _ = semaphore.wait(timeout: delta) } + + func awaitRun(value: Int, _ seconds: TimeInterval = TimeInterval(5)) { + let delta = DispatchTime.now() + Double(Int64(seconds) * Int64(NSEC_PER_SEC)) / Double(NSEC_PER_SEC) + runSemaphoreValue = value + _ = runSemaphore.wait(timeout: delta) + } } class TestCreator: JobCreator {