Skip to content
This repository was archived by the owner on Jul 5, 2024. It is now read-only.

Commit 1cf013d

Browse files
mattmassicottemattmassicotte
authored
Merge pull request #10 from ChimeHQ/feature/timeout-improvements
Timeout improvements
2 parents eb5d3c1 + 7baf9cc commit 1cf013d

10 files changed

+163
-51
lines changed

‎OperationPlus.xcodeproj/project.pbxproj

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
C949420F232C7C8700BCFC88 /* AsyncBlockProducerOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C949420E232C7C8700BCFC88 /* AsyncBlockProducerOperation.swift */; };
1515
C9494211232DB95000BCFC88 /* AsyncBlockConsumerProducerOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9494210232DB95000BCFC88 /* AsyncBlockConsumerProducerOperation.swift */; };
1616
C9494213232DBEE900BCFC88 /* BlockConsumerProducerOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9494212232DBEE900BCFC88 /* BlockConsumerProducerOperation.swift */; };
17+
C990C94423CF79D600203A47 /* NeverFinishingProducerOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C990C94323CF79D600203A47 /* NeverFinishingProducerOperation.swift */; };
1718
C9B0213B2203CE91006A1D9F /* OperationPlus.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C9B021312203CE91006A1D9F /* OperationPlus.framework */; };
1819
C9B021422203CE91006A1D9F /* OperationPlus.h in Headers */ = {isa = PBXBuildFile; fileRef = C9B021342203CE91006A1D9F /* OperationPlus.h */; settings = {ATTRIBUTES = (Public, ); }; };
1920
C9B0214C2203CFD1006A1D9F /* BaseOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9B0214B2203CFD1006A1D9F /* BaseOperation.swift */; };
@@ -72,6 +73,7 @@
7273
C949420E232C7C8700BCFC88 /* AsyncBlockProducerOperation.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AsyncBlockProducerOperation.swift; sourceTree = "<group>"; };
7374
C9494210232DB95000BCFC88 /* AsyncBlockConsumerProducerOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AsyncBlockConsumerProducerOperation.swift; sourceTree = "<group>"; };
7475
C9494212232DBEE900BCFC88 /* BlockConsumerProducerOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockConsumerProducerOperation.swift; sourceTree = "<group>"; };
76+
C990C94323CF79D600203A47 /* NeverFinishingProducerOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NeverFinishingProducerOperation.swift; sourceTree = "<group>"; };
7577
C9B021312203CE91006A1D9F /* OperationPlus.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OperationPlus.framework; sourceTree = BUILT_PRODUCTS_DIR; };
7678
C9B021342203CE91006A1D9F /* OperationPlus.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = OperationPlus.h; sourceTree = "<group>"; };
7779
C9B021352203CE91006A1D9F /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
@@ -202,6 +204,7 @@
202204
C9B021872203D3A2006A1D9F /* NeverFinishingOperation.swift */,
203205
C9B021852203D3A1006A1D9F /* OperationExpectation.swift */,
204206
C9B021842203D310006A1D9F /* OperationTestingPlus.xcconfig */,
207+
C990C94323CF79D600203A47 /* NeverFinishingProducerOperation.swift */,
205208
);
206209
path = OperationTestingPlus;
207210
sourceTree = "<group>";
@@ -407,6 +410,7 @@
407410
isa = PBXSourcesBuildPhase;
408411
buildActionMask = 2147483647;
409412
files = (
413+
C990C94423CF79D600203A47 /* NeverFinishingProducerOperation.swift in Sources */,
410414
C9B021882203D3A2006A1D9F /* OperationExpectation.swift in Sources */,
411415
C9B0218A2203D3A2006A1D9F /* NeverFinishingOperation.swift in Sources */,
412416
C9B021892203D3A2006A1D9F /* FulfillExpectationOperation.swift in Sources */,
@@ -565,7 +569,6 @@
565569
CODE_SIGN_IDENTITY = "";
566570
CODE_SIGN_STYLE = Automatic;
567571
COMBINE_HIDPI_IMAGES = YES;
568-
CURRENT_PROJECT_VERSION = 7;
569572
DEFINES_MODULE = YES;
570573
DYLIB_COMPATIBILITY_VERSION = 1;
571574
DYLIB_CURRENT_VERSION = 1;
@@ -577,7 +580,6 @@
577580
"@executable_path/../Frameworks",
578581
"@loader_path/Frameworks",
579582
);
580-
MARKETING_VERSION = 1.4.0;
581583
SKIP_INSTALL = YES;
582584
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
583585
SWIFT_VERSION = 5.0;
@@ -592,7 +594,6 @@
592594
CODE_SIGN_IDENTITY = "";
593595
CODE_SIGN_STYLE = Automatic;
594596
COMBINE_HIDPI_IMAGES = YES;
595-
CURRENT_PROJECT_VERSION = 7;
596597
DEFINES_MODULE = YES;
597598
DYLIB_COMPATIBILITY_VERSION = 1;
598599
DYLIB_CURRENT_VERSION = 1;
@@ -604,7 +605,6 @@
604605
"@executable_path/../Frameworks",
605606
"@loader_path/Frameworks",
606607
);
607-
MARKETING_VERSION = 1.4.0;
608608
SKIP_INSTALL = YES;
609609
SWIFT_VERSION = 5.0;
610610
};
@@ -650,7 +650,6 @@
650650
CODE_SIGN_IDENTITY = "";
651651
CODE_SIGN_STYLE = Automatic;
652652
COMBINE_HIDPI_IMAGES = YES;
653-
CURRENT_PROJECT_VERSION = 6;
654653
DEFINES_MODULE = YES;
655654
DYLIB_COMPATIBILITY_VERSION = 1;
656655
DYLIB_CURRENT_VERSION = 1;
@@ -662,7 +661,6 @@
662661
"@executable_path/../Frameworks",
663662
"@loader_path/Frameworks",
664663
);
665-
MARKETING_VERSION = 1.3.0;
666664
SKIP_INSTALL = YES;
667665
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
668666
SWIFT_VERSION = 5.0;
@@ -677,7 +675,6 @@
677675
CODE_SIGN_IDENTITY = "";
678676
CODE_SIGN_STYLE = Automatic;
679677
COMBINE_HIDPI_IMAGES = YES;
680-
CURRENT_PROJECT_VERSION = 6;
681678
DEFINES_MODULE = YES;
682679
DYLIB_COMPATIBILITY_VERSION = 1;
683680
DYLIB_CURRENT_VERSION = 1;
@@ -689,7 +686,6 @@
689686
"@executable_path/../Frameworks",
690687
"@loader_path/Frameworks",
691688
);
692-
MARKETING_VERSION = 1.3.0;
693689
SKIP_INSTALL = YES;
694690
SWIFT_VERSION = 5.0;
695691
};

‎OperationPlus/BaseOperation.swift

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ open class BaseOperation : Operation {
1313
case notStarted
1414
case running
1515
case finished
16-
case timedOut
1716
}
1817

1918
private let lock: NSRecursiveLock
2019
private var state: State = .notStarted
2120
public let timeoutInterval: TimeInterval
21+
private var hasTimedOut: Bool = false
2222

2323
public init(timeout: TimeInterval = .greatestFiniteMagnitude) {
2424
self.lock = NSRecursiveLock()
@@ -48,26 +48,50 @@ open class BaseOperation : Operation {
4848
lock.lock()
4949
defer { lock.unlock() }
5050

51-
return state == .finished || state == .timedOut
51+
return state == .finished
5252
}
5353

54+
open var isTimedOut: Bool {
55+
lock.lock()
56+
defer { lock.unlock() }
57+
58+
return hasTimedOut
59+
}
60+
61+
private func markTimedOut() {
62+
lock.lock()
63+
defer { lock.unlock() }
64+
65+
if isFinished {
66+
return
67+
}
68+
69+
hasTimedOut = true
70+
timedOut()
71+
}
72+
73+
/// Called when an operation times out.
74+
///
75+
/// This is a useful hook for subclassers. By default, this will call cancel() and then finish(). If you
76+
/// do override this method, be sure to either call super or explicitly finish the operation.
5477
open func timedOut() {
55-
transition(to: .timedOut)
78+
cancel()
79+
finish()
5680
}
5781

58-
func prepareForMain() {
82+
func beginExecution() {
5983
transition(to: .running)
6084

6185
setupTimeout()
6286
}
6387

6488
override open func start() {
89+
beginExecution()
90+
6591
if checkForCancellation() {
6692
return
6793
}
6894

69-
prepareForMain()
70-
7195
main()
7296
}
7397

@@ -90,13 +114,6 @@ open class BaseOperation : Operation {
90114
}
91115

92116
extension BaseOperation {
93-
public var isTimedOut: Bool {
94-
lock.lock()
95-
defer { lock.unlock() }
96-
97-
return state == .timedOut
98-
}
99-
100117
private var deadlineTime: DispatchTime {
101118
return DispatchTime.now() + .seconds(Int(timeoutInterval))
102119
}
@@ -106,18 +123,31 @@ extension BaseOperation {
106123
return
107124
}
108125

126+
guard !isCancelled else {
127+
return
128+
}
129+
109130
DispatchQueue.global().asyncAfter(deadline: deadlineTime) { [weak self] in
110131
guard let op = self else {
111132
return
112133
}
113134

114-
op.timedOut()
135+
op.markTimedOut()
115136
}
116137
}
117138
}
118139

119140
extension BaseOperation {
120141
public func finish() {
142+
lock.lock()
143+
defer { lock.unlock() }
144+
145+
// If we've timed out, it's still likely that finish
146+
// is going to be called again. We should be ok with that.
147+
if isTimedOut && isFinished {
148+
return
149+
}
150+
121151
transition(to: .finished)
122152
}
123153

@@ -127,12 +157,6 @@ extension BaseOperation {
127157
return true
128158
}
129159

130-
// this is just a guard to protect against
131-
// accidental early finishes
132-
if isFinished {
133-
return true
134-
}
135-
136160
return false
137161
}
138162

@@ -145,17 +169,13 @@ extension BaseOperation {
145169
willChangeValue(forKey: "isExecuting")
146170
state = newState;
147171
didChangeValue(forKey: "isExecuting")
148-
case (.running, .finished), (.running, .timedOut):
172+
case (.running, .finished):
149173
willChangeValue(forKey: "isExecuting")
150174
willChangeValue(forKey: "isFinished")
151175
state = newState;
152176
didChangeValue(forKey: "isFinished")
153177
didChangeValue(forKey: "isExecuting")
154-
case (.finished, .timedOut), (.timedOut, .finished):
155-
break
156-
case (_, .notStarted):
157-
fallthrough
158-
case (.finished, _), (.timedOut, _), (.running, _), (.notStarted, _):
178+
case (_, .notStarted), (.finished, _), (.running, _), (.notStarted, _):
159179
handleError(.stateTransitionInvalid(newState))
160180
}
161181
}

‎OperationPlus/OperationPlus.xcconfig

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
PRODUCT_NAME = OperationPlus
1515
PRODUCT_BUNDLE_IDENTIFIER = com.chimehq.OperationPlus
1616
PRODUCT_MODULE_NAME = OperationPlus
17-
CURRENT_PROJECT_VERSION = 7
17+
CURRENT_PROJECT_VERSION = 8
18+
MARKETING_VERSION = 1.5.0
1819

1920
INFOPLIST_FILE = OperationPlus/Info.plist
2021

‎OperationPlus/ProducerOperation.swift

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,16 @@ import Foundation
1515
public typealias ResultOperation<T> = ProducerOperation<T>
1616

1717
open class ProducerOperation<T> : BaseOperation {
18+
public enum ResultCompletionBlockBehavior {
19+
case onCompletionOnly
20+
case onTimeOut(T)
21+
}
22+
1823
public typealias ResultBlock = (T) -> Void
1924

2025
public var resultCompletionBlock: ResultBlock?
2126
public var value: T?
27+
public var resultCompletionBlockBehavior = ResultCompletionBlockBehavior.onCompletionOnly
2228

2329
/// Block to return a cached value
2430
///
@@ -39,27 +45,42 @@ open class ProducerOperation<T> : BaseOperation {
3945
self.value = v
4046
writeCacheBlock?(v)
4147

42-
let invokeBlock = !(isCancelled || isTimedOut)
43-
44-
if invokeBlock {
48+
switch (resultCompletionBlockBehavior, isCancelled, isTimedOut) {
49+
case (_, false, false):
4550
resultCompletionBlock?(v)
51+
case (_, _, true):
52+
return // do not call finish in this case
53+
default:
54+
break
4655
}
4756

4857
finish()
4958
}
5059

5160
override open func start() {
61+
beginExecution()
62+
5263
if checkForCancellation() {
5364
return
5465
}
5566

56-
prepareForMain()
57-
5867
if let v = readCacheBlock?() {
5968
self.finish(with: v)
6069
return
6170
}
6271

6372
main()
6473
}
74+
75+
override open func timedOut() {
76+
super.timedOut()
77+
78+
switch (resultCompletionBlockBehavior, isCancelled, isTimedOut) {
79+
case (.onTimeOut(let v), _, true):
80+
self.value = v
81+
resultCompletionBlock?(v)
82+
default:
83+
break
84+
}
85+
}
6586
}

‎OperationPlusTests/AsyncBlockOperationTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class AsyncBlockOperationTests: XCTestCase {
3434
let expectation = OperationExpectation(operation: op)
3535
expectation.isInverted = true
3636

37-
wait(for: [expectation], timeout: 1.0)
37+
wait(for: [expectation], timeout: 0.1)
3838

3939
XCTAssertTrue(op.isReady)
4040
XCTAssertFalse(op.isFinished)

0 commit comments

Comments
 (0)