Skip to content

Commit c45ceef

Browse files
cnooncnoon
authored
Refactored activity counting to more robust request id tracking (#49)
* Refactored activity counting to more robust request id tracking * Removed unnecessary parameter portion of selector signature * Removed unnecessary lock on state didSet implementation
1 parent e40c5fb commit c45ceef

File tree

2 files changed

+84
-51
lines changed

2 files changed

+84
-51
lines changed

‎Source/NetworkActivityIndicatorManager.swift‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//
22
// NetworkActivityIndicatorManager.swift
33
//
4-
// Copyright (c) 2016-2018 Alamofire Software Foundation (http://alamofire.org/)
4+
// Copyright (c) 2016 Alamofire Software Foundation (http://alamofire.org/)
55
//
66
// Permission is hereby granted, free of charge, to any person obtaining a copy
77
// of this software and associated documentation files (the "Software"), to deal
@@ -103,13 +103,13 @@ public class NetworkActivityIndicatorManager {
103103
}
104104
}
105105

106-
private var activityCount: Int = 0
106+
private var requestIDs: Set<String> = []
107107
private var enabled: Bool = true
108108

109109
private var startDelayTimer: Timer?
110110
private var completionDelayTimer: Timer?
111111

112-
private let lock = NSLock()
112+
private let lock = NSRecursiveLock()
113113

114114
// MARK: - Internal - Initialization
115115

@@ -124,31 +124,29 @@ public class NetworkActivityIndicatorManager {
124124
invalidateCompletionDelayTimer()
125125
}
126126

127-
// MARK: - Activity Count
127+
// MARK: - Request Tracking
128128

129-
/// Increments the number of active network requests.
129+
/// Adds the requestID as an active request driving the activity indicator.
130130
///
131-
/// If this number was zero before incrementing, the network activity indicator will start spinning after
132-
/// the `startDelay`.
131+
/// This method results in a no-op if the request is already being tracked.
133132
///
134-
/// Generally, this method should not need to be used directly.
135-
public func incrementActivityCount() {
133+
/// - Parameter requestID: The request identifier.
134+
public func requestDidStart(requestID: String) {
136135
lock.lock() ; defer { lock.unlock() }
137136

138-
activityCount += 1
137+
requestIDs.insert(requestID)
139138
updateActivityIndicatorStateForNetworkActivityChange()
140139
}
141140

142-
/// Decrements the number of active network requests.
141+
/// Removes the requestID from the set of active requests driving the activity indicator.
143142
///
144-
/// If the number of active requests is zero after calling this method, the network activity indicator will stop
145-
/// spinning after the `completionDelay`.
143+
/// This method results in a no-op if the request is not being tracked.
146144
///
147-
/// Generally, this method should not need to be used directly.
148-
public func decrementActivityCount() {
145+
/// - Parameter requestID: The request identifier.
146+
public func requestDidStop(requestID: String) {
149147
lock.lock() ; defer { lock.unlock() }
150148

151-
activityCount -= 1
149+
requestIDs.remove(requestID)
152150
updateActivityIndicatorStateForNetworkActivityChange()
153151
}
154152

@@ -159,14 +157,14 @@ public class NetworkActivityIndicatorManager {
159157

160158
switch activityIndicatorState {
161159
case .notActive:
162-
if activityCount > 0 { activityIndicatorState = .delayingStart }
160+
if !requestIDs.isEmpty { activityIndicatorState = .delayingStart }
163161
case .delayingStart:
164162
// No-op - let the delay timer finish
165163
break
166164
case .active:
167-
if activityCount == 0 { activityIndicatorState = .delayingCompletion }
165+
if requestIDs.isEmpty { activityIndicatorState = .delayingCompletion }
168166
case .delayingCompletion:
169-
if activityCount > 0 { activityIndicatorState = .active }
167+
if !requestIDs.isEmpty { activityIndicatorState = .active }
170168
}
171169
}
172170

@@ -184,14 +182,14 @@ public class NetworkActivityIndicatorManager {
184182

185183
notificationCenter.addObserver(
186184
self,
187-
selector: #selector(NetworkActivityIndicatorManager.networkRequestDidComplete),
185+
selector: #selector(NetworkActivityIndicatorManager.networkRequestDidStop),
188186
name: Request.didSuspend,
189187
object: nil
190188
)
191189

192190
notificationCenter.addObserver(
193191
self,
194-
selector: #selector(NetworkActivityIndicatorManager.networkRequestDidComplete),
192+
selector: #selector(NetworkActivityIndicatorManager.networkRequestDidStop),
195193
name: Request.didFinish,
196194
object: nil
197195
)
@@ -203,12 +201,14 @@ public class NetworkActivityIndicatorManager {
203201

204202
// MARK: - Private - Notifications
205203

206-
@objc private func networkRequestDidStart() {
207-
incrementActivityCount()
204+
@objc private func networkRequestDidStart(notification: Notification) {
205+
guard let request = notification.request else { return }
206+
requestDidStart(requestID: request.id.uuidString)
208207
}
209208

210-
@objc private func networkRequestDidComplete() {
211-
decrementActivityCount()
209+
@objc private func networkRequestDidStop(notification: Notification) {
210+
guard let request = notification.request else { return }
211+
requestDidStop(requestID: request.id.uuidString)
212212
}
213213

214214
// MARK: - Private - Timers
@@ -250,7 +250,7 @@ public class NetworkActivityIndicatorManager {
250250
@objc private func startDelayTimerFired() {
251251
lock.lock() ; defer { lock.unlock() }
252252

253-
if activityCount > 0 {
253+
if !requestIDs.isEmpty {
254254
activityIndicatorState = .active
255255
} else {
256256
activityIndicatorState = .notActive

‎Tests/NetworkActivityIndicatorManagerTests.swift‎

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//
22
// NetworkActivityIndicatorManagerTests.swift
33
//
4-
// Copyright (c) 2016-2018 Alamofire Software Foundation (http://alamofire.org/)
4+
// Copyright (c) 2016 Alamofire Software Foundation (http://alamofire.org/)
55
//
66
// Permission is hereby granted, free of charge, to any person obtaining a copy
77
// of this software and associated documentation files (the "Software"), to deal
@@ -30,9 +30,9 @@ import XCTest
3030
class NetworkActivityIndicatorManagerTestCase: XCTestCase {
3131
let timeout = 10.0
3232

33-
// MARK: - Tests - Manual Activity Count Updates
33+
// MARK: - Tests - Manual Request Tracking
3434

35-
func testThatManagerCanTurnOnAndOffIndicatorWhenManuallyIncrementingAndDecrementingActivityCount() {
35+
func testThatManagerCanTurnOnAndOffIndicatorWhenManuallyControllingRequests() {
3636
// Given
3737
let manager = NetworkActivityIndicatorManager()
3838
manager.startDelay = 0.0
@@ -48,8 +48,8 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
4848
}
4949

5050
// When
51-
manager.incrementActivityCount()
52-
dispatchAfter(0.1) { manager.decrementActivityCount() }
51+
manager.requestDidStart(requestID: "r1")
52+
dispatchAfter(0.1) { manager.requestDidStop(requestID: "r1") }
5353
waitForExpectations(timeout: timeout, handler: nil)
5454

5555
// Then
@@ -62,7 +62,7 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
6262
}
6363
}
6464

65-
func testThatManagerCanTurnOnAndOffIndicatorWhenManuallyControllingActivityCountWithMultipleChanges() {
65+
func testThatManagerCanTurnOnAndOffIndicatorWhenManuallyControllingRequestsWithMultipleChanges() {
6666
// Given
6767
let manager = NetworkActivityIndicatorManager()
6868
manager.startDelay = 0.0
@@ -78,10 +78,10 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
7878
}
7979

8080
// When
81-
manager.incrementActivityCount()
82-
dispatchAfter(0.05) { manager.incrementActivityCount() }
83-
dispatchAfter(0.10) { manager.decrementActivityCount() }
84-
dispatchAfter(0.15) { manager.decrementActivityCount() }
81+
manager.requestDidStart(requestID: "r1")
82+
dispatchAfter(0.05) { manager.requestDidStart(requestID: "r2") }
83+
dispatchAfter(0.10) { manager.requestDidStop(requestID: "r1") }
84+
dispatchAfter(0.15) { manager.requestDidStop(requestID: "r2") }
8585

8686
waitForExpectations(timeout: timeout, handler: nil)
8787

@@ -95,7 +95,7 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
9595
}
9696
}
9797

98-
func testThatManagerAppliesStartDelayWhenManuallyControllingActivityCount() {
98+
func testThatManagerAppliesStartDelayWhenManuallyControllingRequests() {
9999
// Given
100100
let manager = NetworkActivityIndicatorManager()
101101
manager.startDelay = 0.125
@@ -107,8 +107,8 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
107107
}
108108

109109
// When
110-
manager.incrementActivityCount()
111-
dispatchAfter(0.05) { manager.decrementActivityCount() }
110+
manager.requestDidStart(requestID: "r1")
111+
dispatchAfter(0.05) { manager.requestDidStop(requestID: "r1") }
112112

113113
let expectation = self.expectation(description: "visibility should change twice")
114114
dispatchAfter(0.25) { expectation.fulfill() }
@@ -120,7 +120,7 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
120120
XCTAssertTrue(visibilityStates.isEmpty)
121121
}
122122

123-
func testThatManagerAppliesFinishDelayWhenManuallyControllingActivityCount() {
123+
func testThatManagerAppliesFinishDelayWhenManuallyControllingRequests() {
124124
// Given
125125
let manager = NetworkActivityIndicatorManager()
126126
manager.startDelay = 0.0
@@ -136,10 +136,10 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
136136
}
137137

138138
// When
139-
manager.incrementActivityCount()
140-
dispatchAfter(0.1) { manager.decrementActivityCount() }
141-
dispatchAfter(0.2) { manager.incrementActivityCount() }
142-
dispatchAfter(0.3) { manager.decrementActivityCount() }
139+
manager.requestDidStart(requestID: "r1")
140+
dispatchAfter(0.1) { manager.requestDidStop(requestID: "r1") }
141+
dispatchAfter(0.2) { manager.requestDidStart(requestID: "r2") }
142+
dispatchAfter(0.3) { manager.requestDidStop(requestID: "r2") }
143143

144144
waitForExpectations(timeout: timeout, handler: nil)
145145

@@ -167,8 +167,8 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
167167
}
168168

169169
// When
170-
manager.incrementActivityCount()
171-
dispatchAfter(0.1) { manager.decrementActivityCount() }
170+
manager.requestDidStart(requestID: "r1")
171+
dispatchAfter(0.1) { manager.requestDidStop(requestID: "r1") }
172172

173173
let expectation = self.expectation(description: "visibility should change twice")
174174
dispatchAfter(0.2) { expectation.fulfill() }
@@ -180,7 +180,7 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
180180
XCTAssertTrue(visibilityStates.isEmpty)
181181
}
182182

183-
func testThatManagerIgnoresDecrementingActivityCountWhenAlreadyZero() {
183+
func testThatManagerIgnoresDuplicateRequestDidStartCalls() {
184184
// Given
185185
let manager = NetworkActivityIndicatorManager()
186186
manager.startDelay = 0.0
@@ -193,10 +193,43 @@ class NetworkActivityIndicatorManagerTestCase: XCTestCase {
193193
}
194194

195195
// When
196-
manager.incrementActivityCount()
197-
dispatchAfter(0.10) { manager.decrementActivityCount() }
198-
dispatchAfter(0.15) { manager.decrementActivityCount() }
199-
dispatchAfter(0.20) { manager.decrementActivityCount() }
196+
manager.requestDidStart(requestID: "r1")
197+
dispatchAfter(0.10) { manager.requestDidStart(requestID: "r1") }
198+
dispatchAfter(0.15) { manager.requestDidStart(requestID: "r1") }
199+
dispatchAfter(0.20) { manager.requestDidStop(requestID: "r1") }
200+
201+
let expectation = self.expectation(description: "visibility should change twice")
202+
dispatchAfter(0.25) { expectation.fulfill() }
203+
204+
waitForExpectations(timeout: timeout, handler: nil)
205+
206+
// Then
207+
XCTAssertTrue(manager.isEnabled)
208+
XCTAssertEqual(visibilityStates.count, 2)
209+
210+
if visibilityStates.count == 2 {
211+
XCTAssertTrue(visibilityStates[0])
212+
XCTAssertFalse(visibilityStates[1])
213+
}
214+
}
215+
216+
func testThatManagerIgnoresDuplicateRequestDidStopCalls() {
217+
// Given
218+
let manager = NetworkActivityIndicatorManager()
219+
manager.startDelay = 0.0
220+
manager.completionDelay = 0.0
221+
222+
var visibilityStates: [Bool] = []
223+
224+
manager.networkActivityIndicatorVisibilityChanged = { isVisible in
225+
visibilityStates.append(isVisible)
226+
}
227+
228+
// When
229+
manager.requestDidStart(requestID: "r1")
230+
dispatchAfter(0.10) { manager.requestDidStop(requestID: "r1") }
231+
dispatchAfter(0.15) { manager.requestDidStop(requestID: "r1") }
232+
dispatchAfter(0.20) { manager.requestDidStop(requestID: "r1") }
200233

201234
let expectation = self.expectation(description: "visibility should change twice")
202235
dispatchAfter(0.25) { expectation.fulfill() }

0 commit comments

Comments
 (0)