Skip to content

Commit

Permalink
fix: Fix parsing of the API response (#850)
Browse files Browse the repository at this point in the history
* Fix LinkedCardsComponent leaked continuation

* Add parsing of simple response (String)

* Add success response factory + add tests to cover redirect api

* swiftlint fixes

* Use same pattern for api client in linked cards component as in other components

* inline init

* remove custom string decoding

* Update Sources/PrimerSDK/Classes/Services/Network/PrimerAPIClient.swift

---------

Co-authored-by: Boris Nikolic <[email protected]>
Co-authored-by: Jack Newcombe <[email protected]>
Co-authored-by: Jack Newcombe <[email protected]>
  • Loading branch information
4 people authored Apr 29, 2024
1 parent a8c614f commit 6de9db0
Show file tree
Hide file tree
Showing 14 changed files with 116 additions and 222 deletions.
190 changes: 0 additions & 190 deletions Debug App/Primer.io Debug App SPM.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct MerchantMockDataManager {
static var genericPaymentMethod = ClientSessionRequestBody.PaymentMethod(
vaultOnSuccess: false,
options: nil,
descriptor: nil,
descriptor: "Random descriptor",
paymentType: nil
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public class NolPayLinkedCardsComponent {
var mobileNumber: String?
var countryCode: String?
var phoneMetadataService: NolPayPhoneMetadataProviding?
var apiClient: PrimerAPIClientProtocol?

public init() {}

Expand Down Expand Up @@ -96,8 +95,9 @@ public class NolPayLinkedCardsComponent {
phoneVendor: "Apple",
phoneModel: UIDevice.modelIdentifier!)

let client = PrimerAPIClient()
return try await withCheckedThrowingContinuation { continuation in
self.apiClient?.fetchNolSdkSecret(clientToken: clientToken, paymentRequestBody: requestBody) { result in
client.fetchNolSdkSecret(clientToken: clientToken, paymentRequestBody: requestBody) { result in
switch result {
case .success(let appSecret):
continuation.resume(returning: appSecret.sdkSecret)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ extension PrimerHeadlessCollectDataComponent {
let error = PrimerError.invalidValue(key: key,
value: nil,
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString)
diagnosticsId: UUID().uuidString)
ErrorHandler.handle(error: error)
self.errorDelegate?.didReceiveError(error: error)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class DefaultNetworkService: NetworkService, LogReporter {
} catch {
completion(.failure(error))
}

}
} catch {
ErrorHandler.handle(error: error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,26 @@ protocol NetworkResponseFactory: AnyObject {

extension Endpoint {
var responseFactory: NetworkResponseFactory {
switch self {
default:
return JSONNetworkResponseFactory()
if let endpoint = self as? PrimerAPI {
switch endpoint {
case .redirect:
return SuccessResponseFactory()
default:
break
}
}
return JSONNetworkResponseFactory()
}
}

class SuccessResponseFactory: NetworkResponseFactory {
func model<T>(for response: Data, forMetadata metadata: any ResponseMetadata) throws -> T where T: Decodable {
if let response = SuccessResponse() as? T {
return response
}
throw InternalError.failedToDecode(message: "SuccessResponse model must be used with this endpoint",
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString)
}
}

Expand All @@ -25,32 +41,34 @@ class JSONNetworkResponseFactory: NetworkResponseFactory, LogReporter {
let decoder = JSONDecoder()

func model<T>(for response: Data, forMetadata metadata: ResponseMetadata) throws -> T where T: Decodable {

log(data: response, metadata: metadata)

switch metadata.statusCode {
case 200:
do {
return try decoder.decode(T.self, from: response)
} catch {
throw InternalError.failedToDecode(message: "Failed to decode response of type '\(T.self)' from URL: \(metadata.responseUrl ?? "Unknown")",
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString)
throw InternalError.failedToDecode(
message: "Failed to decode response of type '\(T.self)' from URL: \(metadata.responseUrl ?? "Unknown")",
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString
)
}
case 400...599:
let serverError = try? decoder.decode(PrimerServerErrorResponse.self, from: response)
throw InternalError.serverError(status: metadata.statusCode,
response: serverError?.error,
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString)
throw InternalError.serverError(
status: metadata.statusCode,
response: serverError?.error,
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString
)
default:
break
throw InternalError.failedToDecode(
message: "Failed to determine response from URL: \(metadata.responseUrl ?? "Unknown")",
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString
)
}

throw InternalError.failedToDecode(message: "Failed to determine response from URL: \(metadata.responseUrl ?? "Unknown")",
userInfo: .errorUserInfoDictionary(),
diagnosticsId: UUID().uuidString)

}

func log(data: Data, metadata: ResponseMetadata) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ internal class URLSessionStack: NetworkService, LogReporter {
#endif

if endpoint.shouldParseResponseBody == false, httpResponse?.statusCode == 200 {
guard let dummyRes: T = DummySuccess(success: true) as? T
guard let dummyRes: T = DummySuccess() as? T
else {
fatalError()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ enum NetworkEventType {
var endpoint: Endpoint {
switch self {
case .requestStart(_, let endpoint, _),
.requestEnd(_, let endpoint, _),
.networkConnectivity(let endpoint):
.requestEnd(_, let endpoint, _),
.networkConnectivity(let endpoint):
return endpoint
}
}
Expand Down Expand Up @@ -78,7 +78,7 @@ class DefaultNetworkReportingService: NetworkReportingService {
return false
}
guard let baseURL = primerAPI.baseURL, let url = URL(string: baseURL),
!disallowedTrackingPaths.contains(url.path) else {
!disallowedTrackingPaths.contains(url.path) else {
return false
}
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@ import Foundation

typealias DummySuccess = SuccessResponse

internal struct SuccessResponse: Codable {
let success: Bool
internal struct SuccessResponse: Codable, Equatable {
}
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ final class HUC_TokenizationViewModelTests: XCTestCase {
PrimerAPIConfigurationModule.apiConfiguration = apiConfiguration

let mockApiClient = MockPrimerAPIClient()
mockApiClient.validateClientTokenResult = (SuccessResponse(success: true), nil)
mockApiClient.validateClientTokenResult = (SuccessResponse(), nil)
mockApiClient.pollingResults = [
(PollingResponse(status: .pending, id: "0", source: "src"), nil),
(nil, NSError(domain: "dummy-network-error", code: 100)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class PrimerAPIConfigurationModuleTests: XCTestCase {
checkoutModules: nil)

let mockApiClient = MockPrimerAPIClient()
mockApiClient.validateClientTokenResult = (SuccessResponse(success: true), nil)
mockApiClient.validateClientTokenResult = (SuccessResponse(), nil)
mockApiClient.fetchConfigurationResult = (mockPrimerAPIConfiguration, nil)

PrimerAPIConfigurationModule.apiClient = mockApiClient
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,28 @@ final class NetworkResponseFactoryTests: XCTestCase {
XCTAssertEqual(model, responseModel)
}

func testResponseCreation_Empty_Success() throws {
let model = SuccessResponse()
let metadata = ResponseMetadataModel(responseUrl: "a_url", statusCode: 200, headers: nil)

let successResponseFactory = SuccessResponseFactory()
let responseModel: SuccessResponse = try successResponseFactory.model(for: Data(),
forMetadata: metadata)
XCTAssertEqual(model, responseModel)
}

func testResponseCreation_NonJsonToEmpty_Success() throws {
let model = SuccessResponse()
let metadata = ResponseMetadataModel(responseUrl: "a_url", statusCode: 200, headers: nil)

let jsonNetworkResponseFactory = SuccessResponseFactory()
let string = "<html><head></head><body><a>test</a></body></html>"
let responseModel: SuccessResponse = try jsonNetworkResponseFactory.model(for: string.data(using: .utf8)!,
forMetadata: metadata)

XCTAssertEqual(model, responseModel)
}

func testResponseCreation_Empty_Failure() throws {
let jsonNetworkResponseFactory = JSONNetworkResponseFactory()
let metadata = ResponseMetadataModel(responseUrl: "a_url", statusCode: 200, headers: nil)
Expand Down
48 changes: 46 additions & 2 deletions Tests/Unit Tests/Network/Services/DefaultNetworkServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ final class DefaultNetworkServiceTests: XCTestCase {
requestDispatcher = nil
}

func testBasicRequest_success_sync() throws {
func testBasicRequest_jsonDecodingSuccess_sync() throws {

let expectation = self.expectation(description: "Successful response")

Expand Down Expand Up @@ -92,7 +92,7 @@ final class DefaultNetworkServiceTests: XCTestCase {
waitForExpectations(timeout: 2.0)
}

func testBasicRequest_decodingFailure_sync() throws {
func testBasicRequest_jsonDecodingFailure_sync() throws {

let expectation = self.expectation(description: "Fails with decoding error")

Expand Down Expand Up @@ -122,4 +122,48 @@ final class DefaultNetworkServiceTests: XCTestCase {

}

func testRedirectRequest_successWithEmptyResponse_sync() {
let expectation = self.expectation(description: "Fails with decoding error")

let metadata = ResponseMetadataModel(responseUrl: "https://response_url", statusCode: 200, headers: ["X-Test-Key": "X-Test-Value"])
let data = Data()
requestDispatcher.responseModel = DispatcherResponseModel(metadata: metadata, data: data, error: nil)

let endpoint = PrimerAPI.redirect(clientToken: Mocks.decodedJWTToken, url: URL(string: metadata.responseUrl!)!)
let cancellable = defaultNetworkService.request(endpoint) { (result: APIResult<SuccessResponse>) in
switch result {
case .success(_):
expectation.fulfill()
case .failure(let error):
XCTFail(); return
}
}

XCTAssertNil(cancellable)

waitForExpectations(timeout: 2.0)
}

func testRedirectRequest_successWithNonJsonResponse_sync() {
let expectation = self.expectation(description: "Fails with decoding error")

let metadata = ResponseMetadataModel(responseUrl: "https://response_url", statusCode: 200, headers: ["X-Test-Key": "X-Test-Value"])
let data = "<html><head></head><body><a>test</a></body></html>".data(using: .utf8)
requestDispatcher.responseModel = DispatcherResponseModel(metadata: metadata, data: data, error: nil)

let endpoint = PrimerAPI.redirect(clientToken: Mocks.decodedJWTToken, url: URL(string: metadata.responseUrl!)!)
let cancellable = defaultNetworkService.request(endpoint) { (result: APIResult<SuccessResponse>) in
switch result {
case .success(_):
expectation.fulfill()
case .failure(let error):
XCTFail(); return
}
}

XCTAssertNil(cancellable)

waitForExpectations(timeout: 2.0)
}

}
2 changes: 1 addition & 1 deletion Tests/Unit Tests/v2/MockAPIClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ extension MockPrimerAPIClient {

class Samples {

static let mockValidateClientToken: SuccessResponse = SuccessResponse(success: true)
static let mockValidateClientToken: SuccessResponse = SuccessResponse()
static let mockPrimerAPIConfiguration = Response.Body.Configuration(
coreUrl: "https://primer.io/core",
pciUrl: "https://primer.io/pci",
Expand Down

0 comments on commit 6de9db0

Please sign in to comment.