From 0bab25fab568de7d13e6eed01937719b12971b36 Mon Sep 17 00:00:00 2001
From: Christian Noon <christian.noon@gmail.com>
Date: Tue, 9 Jul 2019 09:39:41 -0700
Subject: [PATCH] Appending response serializer now resumes request if finished
 to handle races (#2862)

---
 Source/AFError.swift               |  4 ---
 Source/Request.swift               |  2 +-
 Tests/AFError+AlamofireTests.swift | 10 --------
 Tests/RequestTests.swift           | 41 +++++++++++++++++++++++++++---
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/Source/AFError.swift b/Source/AFError.swift
index 5959c18..47a41a3 100644
--- a/Source/AFError.swift
+++ b/Source/AFError.swift
@@ -111,8 +111,6 @@ public enum AFError: Error {
         case decodingFailed(error: Error)
         /// Generic serialization failed for an empty response that wasn't type `Empty` but instead the associated type.
         case invalidEmptyResponse(type: String)
-        /// A response serializer was added to the request after the request was already finished.
-        case responseSerializerAddedAfterRequestFinished
     }
 
     /// Underlying reason a server trust evaluation error occurred.
@@ -541,8 +539,6 @@ extension AFError.ResponseSerializationFailureReason {
             return "Empty response could not be serialized to type: \(type). Use Empty as the expected type for such responses."
         case .decodingFailed(let error):
             return "Response could not be decoded because of error:\n\(error.localizedDescription)"
-        case .responseSerializerAddedAfterRequestFinished:
-            return "Response serializer was added to the request after it had already finished."
         }
     }
 }
diff --git a/Source/Request.swift b/Source/Request.swift
index 01f2b36..3b8a13b 100644
--- a/Source/Request.swift
+++ b/Source/Request.swift
@@ -442,7 +442,7 @@ public class Request {
             mutableState.responseSerializers.append(closure)
 
             if mutableState.state == .finished {
-                mutableState.error = AFError.responseSerializationFailed(reason: .responseSerializerAddedAfterRequestFinished)
+                mutableState.state = .resumed
             }
 
             if mutableState.responseSerializerProcessingFinished {
diff --git a/Tests/AFError+AlamofireTests.swift b/Tests/AFError+AlamofireTests.swift
index 53132f2..d5041b4 100644
--- a/Tests/AFError+AlamofireTests.swift
+++ b/Tests/AFError+AlamofireTests.swift
@@ -142,11 +142,6 @@ extension AFError {
         return false
     }
 
-    var isResponseSerializerAddedAfterRequestFinished: Bool {
-        if case let .responseSerializationFailed(reason) = self, reason.isResponseSerializerAddedAfterRequestFinished { return true }
-        return false
-    }
-
     // ResponseValidationFailureReason
 
     var isDataFileNil: Bool {
@@ -295,11 +290,6 @@ extension AFError.ResponseSerializationFailureReason {
         if case .invalidEmptyResponse = self { return true }
         return false
     }
-
-    var isResponseSerializerAddedAfterRequestFinished: Bool {
-        if case .responseSerializerAddedAfterRequestFinished = self { return true }
-        return false
-    }
 }
 
 // MARK: -
diff --git a/Tests/RequestTests.swift b/Tests/RequestTests.swift
index cf3fd4a..377751a 100644
--- a/Tests/RequestTests.swift
+++ b/Tests/RequestTests.swift
@@ -652,15 +652,16 @@ class RequestResponseTestCase: BaseTestCase {
         XCTAssertEqual(response2?.error?.asAFError?.isExplicitlyCancelledError, true)
     }
 
-    func testThatAppendingResponseSerializerToCompletedRequestCallsCompletion() {
+    func testThatAppendingResponseSerializerToCompletedRequestInsideCompletionResumesRequest() {
         // Given
         let session = Session()
 
         var response1: DataResponse<Any>?
         var response2: DataResponse<Any>?
+        var response3: DataResponse<Any>?
 
         let expect = expectation(description: "both response serializer completions should be called")
-        expect.expectedFulfillmentCount = 2
+        expect.expectedFulfillmentCount = 3
 
         // When
         let request = session.request(URLRequest.makeHTTPBinRequest())
@@ -672,6 +673,11 @@ class RequestResponseTestCase: BaseTestCase {
             request.responseJSON { resp in
                 response2 = resp
                 expect.fulfill()
+
+                request.responseJSON { resp in
+                    response3 = resp
+                    expect.fulfill()
+                }
             }
         }
 
@@ -679,7 +685,36 @@ class RequestResponseTestCase: BaseTestCase {
 
         // Then
         XCTAssertNotNil(response1?.value)
-        XCTAssertEqual(response2?.error?.asAFError?.isResponseSerializerAddedAfterRequestFinished, true)
+        XCTAssertNotNil(response2?.value)
+        XCTAssertNotNil(response3?.value)
+    }
+
+    func testThatAppendingResponseSerializerToCompletedRequestOutsideCompletionResumesRequest() {
+        // Given
+        let session = Session()
+        let request = session.request(URLRequest.makeHTTPBinRequest())
+
+        var response1: DataResponse<Any>?
+        var response2: DataResponse<Any>?
+        var response3: DataResponse<Any>?
+
+        // When
+        let expect1 = expectation(description: "response serializer 1 completion should be called")
+        request.responseJSON { response1 = $0; expect1.fulfill() }
+        waitForExpectations(timeout: timeout, handler: nil)
+
+        let expect2 = expectation(description: "response serializer 2 completion should be called")
+        request.responseJSON { response2 = $0; expect2.fulfill() }
+        waitForExpectations(timeout: timeout, handler: nil)
+
+        let expect3 = expectation(description: "response serializer 3 completion should be called")
+        request.responseJSON { response3 = $0; expect3.fulfill() }
+        waitForExpectations(timeout: timeout, handler: nil)
+
+        // Then
+        XCTAssertNotNil(response1?.value)
+        XCTAssertNotNil(response2?.value)
+        XCTAssertNotNil(response3?.value)
     }
 }
 
-- 
GitLab