From 666acf86a406c7a264ce3325aa9a80caeb14b20c Mon Sep 17 00:00:00 2001
From: Calvin Cestari <calvin.cestari@gmail.com>
Date: Thu, 7 Mar 2024 16:31:50 -0800
Subject: [PATCH 1/2] Expand multipart subscription test suite

---
 ...ipartResponseSubscriptionParserTests.swift | 212 +++++++++++++++++-
 .../MultipartResponseSubscriptionParser.swift |  43 ++--
 2 files changed, 226 insertions(+), 29 deletions(-)

diff --git a/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift b/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
index c960bf5f1..bccc18ec5 100644
--- a/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
+++ b/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
@@ -10,7 +10,7 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
 
   // MARK: - Error tests
 
-  func test__error__givenChunk_withIncorrectContentType_shouldReturnError() throws {
+  func test__error__givenIncorrectContentType_shouldReturnError() throws {
     let subject = InterceptorTester(interceptor: MultipartResponseParsingInterceptor())
 
     let expectation = expectation(description: "Received callback")
@@ -46,7 +46,7 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
     wait(for: [expectation], timeout: defaultTimeout)
   }
 
-  func test__error__givenChunk_withTransportError_shouldReturnError() throws {
+  func test__error__givenTransportError_shouldReturnError() throws {
     let subject = InterceptorTester(interceptor: MultipartResponseParsingInterceptor())
 
     let expectation = expectation(description: "Received callback")
@@ -84,7 +84,7 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
     wait(for: [expectation], timeout: defaultTimeout)
   }
 
-  func test__error__givenUnrecognizableChunk_shouldReturnError() throws {
+  func test__error__givenTransportErrorWithNullPayload_shouldReturnError() throws {
     let subject = InterceptorTester(interceptor: MultipartResponseParsingInterceptor())
 
     let expectation = expectation(description: "Received callback")
@@ -97,7 +97,14 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
           --graphql
           content-type: application/json
 
-          not_a_valid_json_object
+          {
+            "payload": null,
+            "errors" : [
+              {
+                "message" : "forced test failure!"
+              }
+            ]
+          }
           --graphql
           """.crlfFormattedData()
       )
@@ -108,7 +115,51 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
 
       expect(result).to(beFailure { error in
         expect(error).to(
-          matchError(MultipartResponseSubscriptionParser.ParsingError.cannotParseChunkData)
+          matchError(MultipartResponseSubscriptionParser.ParsingError.irrecoverableError(message: "forced test failure!"))
+        )
+      })
+    }
+
+    wait(for: [expectation], timeout: defaultTimeout)
+  }
+
+  func test__error__givenTransportErrorWithValidPayload_errorShouldTakePrecendenceAndReturnError() throws {
+    let subject = InterceptorTester(interceptor: MultipartResponseParsingInterceptor())
+
+    let expectation = expectation(description: "Received callback")
+
+    subject.intercept(
+      request: .mock(operation: MockSubscription.mock()),
+      response: .mock(
+        headerFields: ["Content-Type": "multipart/mixed;boundary=graphql;subscriptionSpec=1.0"],
+        data: """
+          --graphql
+          content-type: application/json
+
+          {
+            "payload": {
+              "data": {
+                "__typename": "Time",
+                "ticker": 1
+              }
+            },
+            "errors" : [
+              {
+                "message" : "forced test failure!"
+              }
+            ]
+          }
+          --graphql
+          """.crlfFormattedData()
+      )
+    ) { result in
+      defer {
+        expectation.fulfill()
+      }
+
+      expect(result).to(beFailure { error in
+        expect(error).to(
+          matchError(MultipartResponseSubscriptionParser.ParsingError.irrecoverableError(message: "forced test failure!"))
         )
       })
     }
@@ -116,7 +167,7 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
     wait(for: [expectation], timeout: defaultTimeout)
   }
 
-  func test__error__givenChunk_withMissingPayload_shouldReturnError() throws {
+  func test__error__givenTransportErrorIncludingUnknownKeys_shouldReturnErrorWithMessageOnly() throws {
     let subject = InterceptorTester(interceptor: MultipartResponseParsingInterceptor())
 
     let expectation = expectation(description: "Received callback")
@@ -130,7 +181,15 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
           content-type: application/json
 
           {
-            "key": "value"
+            "errors" : [
+              {
+                "message" : "forced test failure!",
+                "path": [
+                  "hello"
+                ],
+                "foo": "bar"
+              }
+            ]
           }
           --graphql
           """.crlfFormattedData()
@@ -142,7 +201,39 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
 
       expect(result).to(beFailure { error in
         expect(error).to(
-          matchError(MultipartResponseSubscriptionParser.ParsingError.cannotParsePayloadData)
+          matchError(MultipartResponseSubscriptionParser.ParsingError.irrecoverableError(message: "forced test failure!"))
+        )
+      })
+    }
+
+    wait(for: [expectation], timeout: defaultTimeout)
+  }
+
+  func test__error__givenMalformedJSON_shouldReturnError() throws {
+    let subject = InterceptorTester(interceptor: MultipartResponseParsingInterceptor())
+
+    let expectation = expectation(description: "Received callback")
+
+    subject.intercept(
+      request: .mock(operation: MockSubscription.mock()),
+      response: .mock(
+        headerFields: ["Content-Type": "multipart/mixed;boundary=graphql;subscriptionSpec=1.0"],
+        data: """
+          --graphql
+          content-type: application/json
+
+          not_a_valid_json_object
+          --graphql
+          """.crlfFormattedData()
+      )
+    ) { result in
+      defer {
+        expectation.fulfill()
+      }
+
+      expect(result).to(beFailure { error in
+        expect(error).to(
+          matchError(MultipartResponseSubscriptionParser.ParsingError.cannotParseChunkData)
         )
       })
     }
@@ -203,7 +294,7 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
     wait(for: [expectation], timeout: defaultTimeout)
   }
 
-  func test__parsing__givenPayloadNull_shouldIgnore() throws {
+  func test__parsing__givenNullPayload_shouldIgnore() throws {
     let network = buildNetworkTransport(responseData: """
       --graphql
       content-type: application/json
@@ -215,7 +306,29 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
       """.crlfFormattedData()
     )
 
-    let expectation = expectation(description: "Payload (null) ignored")
+    let expectation = expectation(description: "Null payload ignored")
+    expectation.isInverted = true
+
+    _ = network.send(operation: MockSubscription<Time>()) { result in
+      expectation.fulfill()
+    }
+
+    wait(for: [expectation], timeout: defaultTimeout)
+  }
+
+  func test__parsing__givenNullErrors_shouldIgnore() throws {
+    let network = buildNetworkTransport(responseData: """
+      --graphql
+      content-type: application/json
+
+      {
+        "errors": null
+      }
+      --graphql
+      """.crlfFormattedData()
+    )
+
+    let expectation = expectation(description: "Null errors ignored")
     expectation.isInverted = true
 
     _ = network.send(operation: MockSubscription<Time>()) { result in
@@ -318,7 +431,48 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
     wait(for: [expectation], timeout: defaultTimeout)
   }
 
-  func test__parsing__givenChunkWithGraphQLError_shouldReturnSuccessWithGraphQLError() throws {
+  func test__parsing__givenPayloadAndUnknownKeys_shouldReturnSuccessAndIgnoreUnknownKeys() throws {
+    let network = buildNetworkTransport(responseData: """
+      --graphql
+      content-type: application/json
+
+      {
+        "foo": "bar",
+        "payload": {
+          "data": {
+            "__typename": "Time",
+            "ticker": 1
+          }
+        }
+      }
+      --graphql
+      """.crlfFormattedData()
+    )
+
+    let expectedData = try Time(data: [
+      "__typename": "Time",
+      "ticker": 1
+    ], variables: nil)
+
+    let expectation = expectation(description: "Multipart data received")
+
+    _ = network.send(operation: MockSubscription<Time>()) { result in
+      defer {
+        expectation.fulfill()
+      }
+
+      switch (result) {
+      case let .success(data):
+        expect(data.data).to(equal(expectedData))
+      case let .failure(error):
+        fail("Unexpected failure result - \(error)")
+      }
+    }
+
+    wait(for: [expectation], timeout: defaultTimeout)
+  }
+
+  func test__parsing__givenPayloadWithDataAndGraphQLError_shouldReturnSuccessWithDataAndGraphQLError() throws {
     let network = buildNetworkTransport(responseData: """
       --graphql
       content-type: application/json
@@ -364,6 +518,42 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
     wait(for: [expectation], timeout: defaultTimeout)
   }
 
+  func test__parsing__givenPayloadWithNullDataAndGraphQLError_shouldReturnSuccessWithOnlyGraphQLError() throws {
+    let network = buildNetworkTransport(responseData: """
+      --graphql
+      content-type: application/json
+
+      {
+        "payload": {
+          "data": null,
+          "errors": [
+            {
+              "message": "test error"
+            }
+          ]
+        }
+      }
+      --graphql
+      """.crlfFormattedData()
+    )
+
+    let expectation = expectation(description: "Multipart data received")
+
+    _ = network.send(operation: MockSubscription<Time>()) { result in
+      switch (result) {
+      case let .success(data):
+        expect(data.errors).to(equal([GraphQLError("test error")]))
+
+        expectation.fulfill()
+
+      case let .failure(error):
+        fail("Unexpected failure result - \(error)")
+      }
+    }
+
+    wait(for: [expectation], timeout: defaultTimeout)
+  }
+
   func test__parsing__givenEndOfStream_shouldReturnSuccess() throws {
     let network = buildNetworkTransport(responseData: """
       --graphql
diff --git a/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift b/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift
index 09eb13122..8302441b9 100644
--- a/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift
+++ b/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift
@@ -9,6 +9,7 @@ struct MultipartResponseSubscriptionParser: MultipartResponseSpecificationParser
     case cannotParseChunkData
     case irrecoverableError(message: String?)
     case cannotParsePayloadData
+    case cannotParseErrorData
 
     public var errorDescription: String? {
       switch self {
@@ -21,6 +22,8 @@ struct MultipartResponseSubscriptionParser: MultipartResponseSpecificationParser
         return "An irrecoverable error occured: \(message ?? "unknown")."
       case .cannotParsePayloadData:
         return "The payload data could not be parsed."
+      case .cannotParseErrorData:
+        return "The error data could not be parsed."
       }
     }
   }
@@ -77,29 +80,33 @@ struct MultipartResponseSubscriptionParser: MultipartResponseSpecificationParser
         }
 
       case let .json(object):
-        if let errors = object.errors {
-          let message = errors.first?["message"] as? String
+        if let errors = object.errors, !(errors is NSNull) {
+          guard
+            let errors = errors as? [JSONObject],
+            let message = errors.first?["message"] as? String
+          else {
+            return .failure(ParsingError.cannotParseErrorData)
+          }
 
           return .failure(ParsingError.irrecoverableError(message: message))
         }
 
-        guard let payload = object.payload else {
-          return .failure(ParsingError.cannotParsePayloadData)
-        }
-
-        if payload is NSNull {
-          // `payload` can be null such as in the case of a transport error
-          break
-        }
+        if let payload = object.payload, !(payload is NSNull) {
+          guard
+            let payload = payload as? JSONObject,
+            let data: Data = try? JSONSerializationFormat.serialize(value: payload)
+          else {
+            return .failure(ParsingError.cannotParsePayloadData)
+          }
 
-        guard
-          let payload = payload as? JSONObject,
-          let data: Data = try? JSONSerializationFormat.serialize(value: payload)
-        else {
-          return .failure(ParsingError.cannotParsePayloadData)
+          return .success(data)
         }
 
-        return .success(data)
+        // 'errors' is optional because valid payloads don't have transport errors.
+        // `errors` can be null because it's taken to be the same as optional.
+        // `payload` is optional because the heartbeat message does not contain a payload field.
+        // `payload` can be null such as in the case of a transport error or future use (TBD).
+        return .success(nil)
 
       case .unknown:
         return .failure(ParsingError.cannotParseChunkData)
@@ -111,8 +118,8 @@ struct MultipartResponseSubscriptionParser: MultipartResponseSpecificationParser
 }
 
 fileprivate extension JSONObject {
-  var errors: [JSONObject]? {
-    self["errors"] as? [JSONObject]
+  var errors: JSONValue? {
+    self["errors"]
   }
 
   var payload: JSONValue? {

From 15f0a48cef37b554a3d980b009229068b831f88f Mon Sep 17 00:00:00 2001
From: Calvin Cestari <calvin.cestari@gmail.com>
Date: Tue, 12 Mar 2024 15:22:51 -0700
Subject: [PATCH 2/2] Refactor tests to not require timeout to succeed

---
 ...ipartResponseSubscriptionParserTests.swift | 68 +++++++++++++++++--
 .../MultipartResponseSubscriptionParser.swift |  2 +-
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift b/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
index bccc18ec5..b63964f8b 100644
--- a/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
+++ b/Tests/ApolloTests/Interceptors/MultipartResponseSubscriptionParserTests.swift
@@ -279,16 +279,42 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
       --graphql
       content-type: application/json
 
+      {
+        "payload": {
+          "data": {
+            "__typename": "Time",
+            "ticker": 1
+          }
+        }
+      }
+      --graphql
+      content-type: application/json
+
       {}
       --graphql
+      content-type: application/json
+
+      {
+        "payload": {
+          "data": {
+            "__typename": "Time",
+            "ticker": 2
+          }
+        }
+      }
+      --graphql
       """.crlfFormattedData()
     )
 
     let expectation = expectation(description: "Heartbeat ignored")
-    expectation.isInverted = true
+    expectation.expectedFulfillmentCount = 2
 
     _ = network.send(operation: MockSubscription<Time>()) { result in
-      expectation.fulfill()
+      defer {
+        expectation.fulfill()
+      }
+
+      expect(result).to(beSuccess())
     }
 
     wait(for: [expectation], timeout: defaultTimeout)
@@ -297,20 +323,34 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
   func test__parsing__givenNullPayload_shouldIgnore() throws {
     let network = buildNetworkTransport(responseData: """
       --graphql
-      content-type: application/json
+      Content-Type: application/json
 
       {
         "payload": null
       }
       --graphql
+      Content-Type: application/json
+
+      {
+        "payload": {
+          "data": {
+            "__typename": "Time",
+            "ticker": 1
+          }
+        }
+      }
+      --graphql
       """.crlfFormattedData()
     )
 
     let expectation = expectation(description: "Null payload ignored")
-    expectation.isInverted = true
 
     _ = network.send(operation: MockSubscription<Time>()) { result in
-      expectation.fulfill()
+      defer {
+        expectation.fulfill()
+      }
+
+      expect(result).to(beSuccess())
     }
 
     wait(for: [expectation], timeout: defaultTimeout)
@@ -325,14 +365,28 @@ final class MultipartResponseSubscriptionParserTests: XCTestCase {
         "errors": null
       }
       --graphql
+      content-type: application/json
+
+      {
+        "payload": {
+          "data": {
+            "__typename": "Time",
+            "ticker": 1
+          }
+        }
+      }
+      --graphql
       """.crlfFormattedData()
     )
 
     let expectation = expectation(description: "Null errors ignored")
-    expectation.isInverted = true
 
     _ = network.send(operation: MockSubscription<Time>()) { result in
-      expectation.fulfill()
+      defer {
+        expectation.fulfill()
+      }
+
+      expect(result).to(beSuccess())
     }
 
     wait(for: [expectation], timeout: defaultTimeout)
diff --git a/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift b/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift
index 8302441b9..45d8e7d9b 100644
--- a/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift
+++ b/apollo-ios/Sources/Apollo/MultipartResponseSubscriptionParser.swift
@@ -46,7 +46,7 @@ struct MultipartResponseSubscriptionParser: MultipartResponseSpecificationParser
         return .heartbeat
       }
 
-      if dataLine.starts(with: contentTypeHeader.description) {
+      if dataLine.lowercased().starts(with: contentTypeHeader.description) {
         let contentType = (dataLine
           .components(separatedBy: ":").last ?? dataLine
         ).trimmingCharacters(in: .whitespaces)