Browse Source

Bug: Pass queue to `responseStreamDecodable` (#3879)

### Issue Link :link:
Fixes #3878

### Goals :soccer:
This PR ensures the queue for `responseStreamDecodable` is properly
passed to the underlying serializer.

### Testing Details :mag:
Tests added for methods on arbitrary queues.
Jon Shier 1 year ago
parent
commit
6b3296bd3e
4 changed files with 148 additions and 27 deletions
  1. 38 26
      .github/workflows/ci.yml
  2. 1 0
      Source/Core/DataStreamRequest.swift
  3. 1 1
      Tests/CombineTests.swift
  4. 108 0
      Tests/DataStreamTests.swift

+ 38 - 26
.github/workflows/ci.yml

@@ -31,9 +31,14 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - xcode: "Xcode_15.4"
+            runsOn: firebreak
+            name: "macOS 14, Xcode 15.4, Swift 5.10"
+            testPlan: "macOS"
+            outputFilter: xcbeautify --renderer github-actions
           - xcode: "Xcode_15.2"
             runsOn: firebreak
-            name: "macOS 13, Xcode 15.2, Swift 5.9.2"
+            name: "macOS 14, Xcode 15.2, Swift 5.9.2"
             testPlan: "macOS"
             outputFilter: xcbeautify --renderer github-actions
           - xcode: "Xcode_15.1"
@@ -79,6 +84,9 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - xcode: "Xcode_15.4"
+            name: "Catalyst 15.4"
+            runsOn: firebreak
           - xcode: "Xcode_15.2"
             name: "Catalyst 15.2"
             runsOn: firebreak
@@ -107,6 +115,11 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - destination: "OS=17.5,name=iPhone 15 Pro"
+            name: "iOS 17.5"
+            testPlan: "iOS"
+            xcode: "Xcode_15.4"
+            runsOn: firebreak
           - destination: "OS=17.2,name=iPhone 15 Pro"
             name: "iOS 17.2"
             testPlan: "iOS"
@@ -122,11 +135,6 @@ jobs:
             testPlan: "iOS"
             xcode: "Xcode_14.3.1"
             runsOn: macOS-13
-          - destination: "OS=15.5,name=iPhone 13 Pro"
-            name: "iOS 15.5"
-            testPlan: "iOS-NoTS"
-            xcode: "Xcode_14.3.1"
-            runsOn: firebreak
     steps:
       - uses: actions/checkout@v4
       - name: Install Firewalk
@@ -143,6 +151,11 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - destination: "OS=17.5,name=Apple TV"
+            name: "tvOS 17.5"
+            testPlan: "tvOS"
+            xcode: "Xcode_15.4"
+            runsOn: firebreak
           - destination: "OS=17.2,name=Apple TV"
             name: "tvOS 17.2"
             testPlan: "tvOS"
@@ -158,11 +171,6 @@ jobs:
             testPlan: "tvOS"
             xcode: "Xcode_14.3.1"
             runsOn: macOS-13
-          - destination: "OS=15.4,name=Apple TV"
-            name: "tvOS 15.4"
-            testPlan: "tvOS-NoTS"
-            xcode: "Xcode_14.3.1"
-            runsOn: firebreak
     steps:
       - uses: actions/checkout@v4
       - name: Install Firewalk
@@ -173,16 +181,23 @@ jobs:
     name: ${{ matrix.name }}
     runs-on: ${{ matrix.runsOn }}
     env:
-      DEVELOPER_DIR: "/Applications/Xcode_15.2.app/Contents/Developer"
+      DEVELOPER_DIR: "/Applications/${{ matrix.xcode }}.app/Contents/Developer"
     timeout-minutes: 10
     strategy:
       fail-fast: false
       matrix:
         include:
+          - destination: "OS=1.2,name=Apple Vision Pro"
+            name: "visionOS 1.2"
+            testPlan: "visionOS"
+            scheme: "Alamofire visionOS"
+            xcode: "Xcode_15.4"
+            runsOn: firebreak
           - destination: "OS=1.0,name=Apple Vision Pro"
             name: "visionOS 1.0"
             testPlan: "visionOS"
             scheme: "Alamofire visionOS"
+            xcode: "Xcode_15.2"
             runsOn: firebreak
     steps:
       - uses: actions/checkout@v4
@@ -200,6 +215,11 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - destination: "OS=10.5,name=Apple Watch Series 9 (45mm)"
+            name: "watchOS 10.5"
+            testPlan: "watchOS"
+            xcode: "Xcode_15.4"
+            runsOn: firebreak
           - destination: "OS=10.2,name=Apple Watch Series 9 (45mm)"
             name: "watchOS 10.2"
             testPlan: "watchOS"
@@ -215,11 +235,6 @@ jobs:
             testPlan: "watchOS"
             xcode: "Xcode_14.3.1"
             runsOn: macOS-13
-          - destination: "OS=8.5,name=Apple Watch Series 7 (45mm)"
-            name: "watchOS 8.5"
-            testPlan: "watchOS-NoTS"
-            xcode: "Xcode_14.3.1"
-            runsOn: firebreak
     steps:
       - uses: actions/checkout@v4
       - name: Install Firewalk
@@ -236,9 +251,13 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          - xcode: "Xcode_15.4"
+            runsOn: firebreak
+            name: "macOS 14, SPM 5.10 Test"
+            outputFilter: xcbeautify --renderer github-actions
           - xcode: "Xcode_15.2"
             runsOn: firebreak
-            name: "macOS 13, SPM 5.9.2 Test"
+            name: "macOS 14, SPM 5.9.2 Test"
             outputFilter: xcbeautify --renderer github-actions
           - xcode: "Xcode_15.1"
             runsOn: macOS-14
@@ -275,27 +294,20 @@ jobs:
         include:
           - image: swift:5.8-focal
           - image: swift:5.8-jammy
-          - image: swift:5.8-centos7
-          - image: swift:5.8-amazonlinux2
           - image: swift:5.8-rhel-ubi9
           - image: swift:5.9-focal
           - image: swift:5.9-jammy
-          - image: swift:5.9-centos7
-          - image: swift:5.9-amazonlinux2
           - image: swift:5.9-rhel-ubi9
           - image: swift:5.10-focal
           - image: swift:5.10-jammy
-          - image: swift:5.10-centos7
-          - image: swift:5.10-amazonlinux2
           - image: swift:5.10-rhel-ubi9
           - image: swiftlang/swift:nightly-focal
           - image: swiftlang/swift:nightly-jammy
-          - image: swiftlang/swift:nightly-amazonlinux2
     container:
       image: ${{ matrix.image }}
     timeout-minutes: 10
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: ${{ matrix.image }}
         run: swift build --build-tests -c debug
   Android:

+ 1 - 0
Source/Core/DataStreamRequest.swift

@@ -464,6 +464,7 @@ public final class DataStreamRequest: Request {
                                                       preprocessor: DataPreprocessor = PassthroughPreprocessor(),
                                                       stream: @escaping Handler<T, AFError>) -> Self {
         responseStream(using: DecodableStreamSerializer<T>(decoder: decoder, dataPreprocessor: preprocessor),
+                       on: queue,
                        stream: stream)
     }
 }

+ 1 - 1
Tests/CombineTests.swift

@@ -1423,7 +1423,7 @@ final class DownloadRequestCombineTests: CombineTestCase {
 
 @available(macOS 10.15, iOS 13, watchOS 6, tvOS 13, *)
 class CombineTestCase: BaseTestCase {
-    private lazy var storage: Set<AnyCancellable> = { Set<AnyCancellable>() }()
+    private lazy var storage: Set<AnyCancellable> = .init()
 
     override func tearDown() {
         storage.removeAll()

+ 108 - 0
Tests/DataStreamTests.swift

@@ -70,6 +70,46 @@ final class DataStreamTests: BaseTestCase {
         XCTAssertTrue(completeOnMain)
     }
 
+    func testThatDataCanBeStreamedOnArbitraryQueue() {
+        // Given
+        let expectedSize = 5
+        var accumulatedData = Data()
+        var initialResponse: HTTPURLResponse?
+        var response: HTTPURLResponse?
+        let streamQueue = DispatchQueue(label: "com.alamofire.tests.ArbitraryQueue")
+        let didReceiveResponse = expectation(description: "stream should receive response once")
+        let didReceive = expectation(description: "stream should receive once")
+        let didComplete = expectation(description: "stream should complete")
+
+        // When
+        AF.streamRequest(.bytes(expectedSize))
+            .onHTTPResponse { response in
+                initialResponse = response
+                didReceiveResponse.fulfill()
+            }
+            .responseStream(on: streamQueue) { stream in
+                dispatchPrecondition(condition: .onQueue(streamQueue))
+                switch stream.event {
+                case let .stream(result):
+                    switch result {
+                    case let .success(data):
+                        accumulatedData.append(data)
+                    }
+                    didReceive.fulfill()
+                case let .complete(completion):
+                    response = completion.response
+                    didComplete.fulfill()
+                }
+            }
+
+        wait(for: [didReceiveResponse, didReceive, didComplete], timeout: timeout, enforceOrder: true)
+
+        // Then
+        XCTAssertEqual(response?.statusCode, 200)
+        XCTAssertEqual(initialResponse, response)
+        XCTAssertEqual(accumulatedData.count, expectedSize)
+    }
+
     func testThatDataCanBeStreamedByByte() throws {
         guard #available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) else {
             throw XCTSkip("Older OSes don't return individual bytes.")
@@ -660,6 +700,38 @@ final class DataStreamSerializationTests: BaseTestCase {
         XCTAssertEqual(response?.statusCode, 200)
     }
 
+    func testThatDataStreamsCanBeAStringOnAnArbitraryQueue() {
+        // Given
+        var responseString: String?
+        var response: HTTPURLResponse?
+        let streamQueue = DispatchQueue(label: "com.alamofire.tests.ArbitraryQueue")
+        let didStream = expectation(description: "did stream")
+        let didComplete = expectation(description: "stream complete")
+
+        // When
+        AF.streamRequest(.stream(1))
+            .responseStreamString(on: streamQueue) { stream in
+                dispatchPrecondition(condition: .onQueue(streamQueue))
+                switch stream.event {
+                case let .stream(result):
+                    switch result {
+                    case let .success(string):
+                        responseString = string
+                    }
+                    didStream.fulfill()
+                case let .complete(completion):
+                    response = completion.response
+                    didComplete.fulfill()
+                }
+            }
+
+        wait(for: [didStream, didComplete], timeout: timeout, enforceOrder: true)
+
+        // Then
+        XCTAssertNotNil(responseString)
+        XCTAssertEqual(response?.statusCode, 200)
+    }
+
     func testThatDataStreamsCanBeDecoded() {
         // Given
         var response: TestResponse?
@@ -700,6 +772,42 @@ final class DataStreamSerializationTests: BaseTestCase {
         XCTAssertNil(decodingError)
     }
 
+    func testThatDataStreamsCanBeDecodedOnAnArbitraryQueue() {
+        // Given
+        var response: TestResponse?
+        var httpResponse: HTTPURLResponse?
+        var decodingError: AFError?
+        let streamQueue = DispatchQueue(label: "com.alamofire.tests.ArbitraryQueue")
+        let didReceive = expectation(description: "stream did receive")
+        let didComplete = expectation(description: "stream complete")
+
+        // When
+        AF.streamRequest(.stream(1))
+            .responseStreamDecodable(of: TestResponse.self, on: streamQueue) { stream in
+                dispatchPrecondition(condition: .onQueue(streamQueue))
+                switch stream.event {
+                case let .stream(result):
+                    switch result {
+                    case let .success(value):
+                        response = value
+                    case let .failure(error):
+                        decodingError = error
+                    }
+                    didReceive.fulfill()
+                case let .complete(completion):
+                    httpResponse = completion.response
+                    didComplete.fulfill()
+                }
+            }
+
+        wait(for: [didReceive, didComplete], timeout: timeout, enforceOrder: true)
+
+        // Then
+        XCTAssertNotNil(response)
+        XCTAssertEqual(httpResponse?.statusCode, 200)
+        XCTAssertNil(decodingError)
+    }
+
     func testThatDataStreamSerializerCanBeUsedDirectly() {
         // Given
         var response: HTTPURLResponse?