Forráskód Böngészése

Add cancelAllRequests API (#2890)

* Add bulk request cancellation API.

* Resume tasks before cancelling to ensure we get metrics event.

* Make cancellation reliable by resuming before cancel.

* Add queue parameter, remove unnecessary self.
Jon Shier 6 éve
szülő
commit
9482ebfe43
5 módosított fájl, 245 hozzáadás és 19 törlés
  1. 13 1
      Source/Request.swift
  2. 38 0
      Source/Session.swift
  3. 16 0
      Tests/BaseTestCase.swift
  4. 5 1
      Tests/HTTPBin.swift
  5. 173 17
      Tests/SessionTests.swift

+ 13 - 1
Source/Request.swift

@@ -604,6 +604,8 @@ public class Request {
                 return
             }
 
+            // Resume to ensure metrics are gathered.
+            task.resume()
             task.cancel()
             underlyingQueue.async { self.didCancelTask(task) }
         }
@@ -776,7 +778,8 @@ public class Request {
     // MARK: Cleanup
 
     /// Final cleanup step executed when the instance finishes response serialization.
-    open func cleanup() {
+    func cleanup() {
+        delegate?.cleanup(after: self)
         // No-op: override in subclass
     }
 }
@@ -893,6 +896,11 @@ public protocol RequestDelegate: AnyObject {
     /// `URLSessionConfiguration` used to create the underlying `URLSessionTask`s.
     var sessionConfiguration: URLSessionConfiguration { get }
 
+    /// Notifies the delegate the `Request` has reached a point where it needs cleanup.
+    ///
+    /// - Parameter request: The `Request` to cleanup after.
+    func cleanup(after request: Request)
+
     /// Asynchronously ask the delegate whether a `Request` will be retried.
     ///
     /// - Parameters:
@@ -1255,12 +1263,16 @@ public class DownloadRequest: Request {
             }
 
             if let completionHandler = completionHandler {
+                // Resume to ensure metrics are gathered.
+                task.resume()
                 task.cancel { (resumeData) in
                     self.protectedDownloadMutableState.write { $0.resumeData = resumeData }
                     self.underlyingQueue.async { self.didCancelTask(task) }
                     completionHandler(resumeData)
                 }
             } else {
+                // Resume to ensure metrics are gathered.
+                task.resume()
                 task.cancel()
                 self.underlyingQueue.async { self.didCancelTask(task) }
             }

+ 38 - 0
Source/Session.swift

@@ -64,6 +64,8 @@ open class Session {
 
     /// Internal map between `Request`s and any `URLSessionTasks` that may be in flight for them.
     var requestTaskMap = RequestTaskMap()
+    /// Set of currently active `Request`s.
+    var activeRequests: Set<Request> = []
 
     /// Creates a `Session` from a `URLSession` and other parameters.
     ///
@@ -192,6 +194,24 @@ open class Session {
         session.invalidateAndCancel()
     }
 
+    // MARK: - Cancellation
+
+    /// Cancel all active `Request`s, optionally calling a completion handler when complete.
+    ///
+    /// - Note: This is an asynchronous operation and does not block the creation of future `Request`s. Cancelled
+    ///         `Request`s may not cancel immediately due internal work, and may not cancel at all if they are close to
+    ///         completion when cancelled.
+    ///
+    /// - Parameters:
+    ///   - queue:      `DispatchQueue` on which the completion handler is run. `.main` by default.
+    ///   - completion: Closure to be called when all `Request`s have been cancelled.
+    public func cancelAllRequests(completingOnQueue queue: DispatchQueue = .main, completion: (() -> Void)? = nil) {
+        rootQueue.async {
+            self.activeRequests.forEach { $0.cancel() }
+            queue.async { completion?() }
+        }
+    }
+
     // MARK: - DataRequest
 
     struct RequestConvertible: URLRequestConvertible {
@@ -750,6 +770,12 @@ open class Session {
 
     // MARK: Perform
 
+
+    /// Perform `Request`.
+    ///
+    /// - Note: Called during retry.
+    ///
+    /// - Parameter request: The `Request` to perform.
     func perform(_ request: Request) {
         switch request {
         case let r as DataRequest: perform(r)
@@ -763,6 +789,8 @@ open class Session {
         requestQueue.async {
             guard !request.isCancelled else { return }
 
+            self.activeRequests.insert(request)
+
             self.performSetupOperations(for: request, convertible: request.convertible)
         }
     }
@@ -771,6 +799,8 @@ open class Session {
         requestQueue.async {
             guard !request.isCancelled else { return }
 
+            self.activeRequests.insert(request)
+
             do {
                 let uploadable = try request.upload.createUploadable()
                 self.rootQueue.async { request.didCreateUploadable(uploadable) }
@@ -786,6 +816,8 @@ open class Session {
         requestQueue.async {
             guard !request.isCancelled else { return }
 
+            self.activeRequests.insert(request)
+
             switch request.downloadable {
             case let .request(convertible):
                 self.performSetupOperations(for: request, convertible: convertible)
@@ -863,6 +895,8 @@ open class Session {
                 task.suspend()
                 rootQueue.async { request.didSuspendTask(task) }
             case (_, .cancelled):
+                // Resume to ensure metrics are gathered.
+                task.resume()
                 task.cancel()
                 rootQueue.async { request.didCancelTask(task) }
             case (_, .finished):
@@ -904,6 +938,10 @@ extension Session: RequestDelegate {
         return session.configuration
     }
 
+    public func cleanup(after request: Request) {
+        activeRequests.remove(request)
+    }
+
     public func retryResult(for request: Request, dueTo error: Error, completion: @escaping (RetryResult) -> Void) {
         guard let retrier = retrier(for: request) else {
             rootQueue.async { completion(.doNotRetry) }

+ 16 - 0
Tests/BaseTestCase.swift

@@ -66,4 +66,20 @@ class BaseTestCase: XCTestCase {
             evaluation(reason)
         }
     }
+
+    /// Runs assertions on a particular `DispatchQueue`.
+    ///
+    /// - Parameters:
+    ///   - queue: The `DispatchQueue` on which to run the assertions.
+    ///   - assertions: Closure containing assertions to run
+    func assert(on queue: DispatchQueue, assertions: @escaping () -> Void) {
+        let expect = expectation(description: "all assertions are complete")
+
+        queue.async {
+            assertions()
+            expect.fulfill()
+        }
+
+        waitForExpectations(timeout: timeout)
+    }
 }

+ 5 - 1
Tests/HTTPBin.swift

@@ -39,10 +39,14 @@ extension URL {
 extension URLRequest {
     static func makeHTTPBinRequest(path: String = "get",
                                    method: HTTPMethod = .get,
-                                   headers: HTTPHeaders = .init()) -> URLRequest {
+                                   headers: HTTPHeaders = .init(),
+                                   timeout: TimeInterval = 60,
+                                   cachePolicy: URLRequest.CachePolicy = .useProtocolCachePolicy) -> URLRequest {
         var request = URLRequest(url: .makeHTTPBinURL(path: path))
         request.httpMethod = method.rawValue
         request.headers = headers
+        request.timeoutInterval = timeout
+        request.cachePolicy = cachePolicy
 
         return request
     }

+ 173 - 17
Tests/SessionTests.swift

@@ -26,7 +26,7 @@
 import Foundation
 import XCTest
 
-class SessionTestCase: BaseTestCase {
+final class SessionTestCase: BaseTestCase {
 
     // MARK: Helper Types
 
@@ -895,7 +895,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 3)
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     func testThatSessionCallsRequestRetrierThenSessionRetrierWhenRequestEncountersError() {
@@ -929,7 +932,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(requestHandler.retryCount, 4)
         XCTAssertEqual(request.retryCount, 2)
         XCTAssertEqual(response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     func testThatSessionCallsRequestRetrierWhenRequestInitiallyEncountersAdaptError() {
@@ -960,7 +966,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCalledCount, 1)
         XCTAssertEqual(handler.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, true)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     func testThatSessionCallsRequestRetrierWhenDownloadInitiallyEncountersAdaptError() {
@@ -996,7 +1005,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCalledCount, 1)
         XCTAssertEqual(handler.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, true)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     func testThatSessionCallsRequestRetrierWhenUploadInitiallyEncountersAdaptError() {
@@ -1025,7 +1037,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCalledCount, 1)
         XCTAssertEqual(handler.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, true)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     func testThatSessionCallsAdapterWhenRequestIsRetried() {
@@ -1055,7 +1070,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 1)
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, true)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     func testThatSessionReturnsRequestAdaptationErrorWhenRequestIsRetried() {
@@ -1085,7 +1103,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 3)
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         if let error = response?.result.error?.asAFError {
             XCTAssertTrue(error.isRequestAdaptationError)
@@ -1123,7 +1144,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 3)
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         if let error = response?.result.error?.asAFError {
             XCTAssertTrue(error.isRequestAdaptationError)
@@ -1160,7 +1184,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 0)
         XCTAssertEqual(request.retryCount, 0)
         XCTAssertEqual(response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         if let error = response?.result.error?.asAFError {
             XCTAssertTrue(error.isRequestRetryError)
@@ -1199,7 +1226,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 0)
         XCTAssertEqual(request.retryCount, 0)
         XCTAssertEqual(response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         if let error = response?.error?.asAFError {
             XCTAssertTrue(error.isResponseSerializationError)
@@ -1244,7 +1274,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(request.retryCount, 0)
         XCTAssertEqual(json1Response?.result.isSuccess, false)
         XCTAssertEqual(json2Response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         let errors: [AFError] = [json1Response, json2Response].compactMap { $0?.error?.asAFError }
         XCTAssertEqual(errors.count, 2)
@@ -1295,7 +1328,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(json1Response?.result.isSuccess, false)
         XCTAssertEqual(json2Response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         let errors: [AFError] = [json1Response, json2Response].compactMap { $0?.error?.asAFError }
         XCTAssertEqual(errors.count, 2)
@@ -1347,7 +1383,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(json1Response?.result.isSuccess, false)
         XCTAssertEqual(json2Response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         let errors: [AFError] = [json1Response, json2Response].compactMap { $0?.error?.asAFError }
         XCTAssertEqual(errors.count, 2)
@@ -1399,7 +1438,10 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(request.retryCount, 1)
         XCTAssertEqual(json1Response?.result.isSuccess, false)
         XCTAssertEqual(json2Response?.result.isSuccess, false)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
 
         let errors: [AFError] = [json1Response, json2Response].compactMap { $0?.error?.asAFError }
         XCTAssertEqual(errors.count, 2)
@@ -1469,8 +1511,11 @@ class SessionTestCase: BaseTestCase {
         XCTAssertEqual(handler.retryCount, 0)
         XCTAssertEqual(request.retryCount, 0)
         XCTAssertEqual(response?.result.isSuccess, true)
-        XCTAssertTrue(session.requestTaskMap.isEmpty)
         XCTAssertEqual(completionCallCount, 1)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty)
+            XCTAssertTrue(session.activeRequests.isEmpty)
+        }
     }
 
     // MARK: Tests - Request State
@@ -1498,7 +1543,118 @@ class SessionTestCase: BaseTestCase {
 
 // MARK: -
 
-class SessionManagerConfigurationHeadersTestCase: BaseTestCase {
+final class SessionCancellationTestCase: BaseTestCase {
+    func testThatAutomaticallyResumedRequestsCanBeMassCancelled() {
+        // Given
+        let count = 100
+        let session = Session()
+        var responses: [DataResponse<Data?>] = []
+        let completion = expectation(description: "all requests should finish")
+        completion.expectedFulfillmentCount = count
+        let cancellation = expectation(description: "cancel all requests should be called")
+
+        // When
+        for _ in 1...count {
+            let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
+            session.request(request).response { (response) in
+                responses.append(response)
+                completion.fulfill()
+            }
+        }
+        session.cancelAllRequests {
+            cancellation.fulfill()
+        }
+
+        waitForExpectations(timeout: timeout)
+
+        // Then
+        XCTAssertTrue(responses.allSatisfy { $0.error?.asAFError?.isExplicitlyCancelledError == true })
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty, "requestTaskMap should be empty but has \(session.requestTaskMap.count) items")
+            XCTAssertTrue(session.activeRequests.isEmpty, "activeRequests should be empty but has \(session.activeRequests.count) items")
+        }
+    }
+
+    func testThatManuallyResumedRequestsCanBeMassCancelled() {
+        // Given
+        let count = 100
+        let session = Session(startRequestsImmediately: false)
+        let request = URLRequest.makeHTTPBinRequest(path: "delay/1")
+        var responses: [DataResponse<Data?>] = []
+        let completion = expectation(description: "all requests should finish")
+        completion.expectedFulfillmentCount = count
+        let cancellation = expectation(description: "cancel all requests should be called")
+
+        // When
+        for _ in 1...count {
+            session.request(request).response { (response) in
+                responses.append(response)
+                completion.fulfill()
+            }
+        }
+        session.cancelAllRequests {
+            cancellation.fulfill()
+        }
+
+        waitForExpectations(timeout: timeout)
+
+        // Then
+        XCTAssertTrue(responses.allSatisfy { $0.error?.asAFError?.isExplicitlyCancelledError == true })
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty, "requestTaskMap should be empty but has \(session.requestTaskMap.count) items")
+            XCTAssertTrue(session.activeRequests.isEmpty, "activeRequests should be empty but has \(session.activeRequests.count) items")
+        }
+    }
+
+    func testThatRetriedRequestsCanBeMassCancelled() {
+        // Given
+        final class OnceRetrier: RequestInterceptor {
+            private var hasRetried = false
+
+            func retry(_ request: Request, for session: Session, dueTo error: Error, completion: @escaping (RetryResult) -> Void) {
+                completion(hasRetried ? .doNotRetry : .retry)
+                hasRetried = true
+            }
+        }
+        let monitor = ClosureEventMonitor()
+        let session = Session(interceptor: OnceRetrier(), eventMonitors: [monitor])
+        let request = URLRequest.makeHTTPBinRequest(path: "status/401")
+        let completion = expectation(description: "all requests should finish")
+        let cancellation = expectation(description: "cancel all requests should be called")
+        let createTask = expectation(description: "should create task twice")
+        createTask.expectedFulfillmentCount = 2
+        monitor.requestDidCreateTask = { (_, _) in
+            createTask.fulfill()
+        }
+        // Cancel when retry starts.
+        monitor.requestIsRetrying = { _ in
+            session.cancelAllRequests {
+                cancellation.fulfill()
+            }
+        }
+
+        var received: DataResponse<Data?>?
+
+        // When
+        session.request(request).validate().response { (response) in
+            received = response
+            completion.fulfill()
+        }
+
+        waitForExpectations(timeout: timeout)
+
+        // Then
+        XCTAssertTrue(received?.error?.asAFError?.isExplicitlyCancelledError == true)
+        assert(on: session.rootQueue) {
+            XCTAssertTrue(session.requestTaskMap.isEmpty, "requestTaskMap should be empty but has \(session.requestTaskMap.count) items")
+            XCTAssertTrue(session.activeRequests.isEmpty, "activeRequests should be empty but has \(session.activeRequests.count) items")
+        }
+    }
+}
+
+// MARK: -
+
+final class SessionConfigurationHeadersTestCase: BaseTestCase {
     enum ConfigurationType {
         case `default`, ephemeral, background
     }