Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix parsing of the API response #850

Merged
merged 9 commits into from
Apr 29, 2024
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NQuinn27 appreciate thoughts on naming here.

The idea is that this will always provide an empty success model because success is implicit based on a 200 code, and the actual response contents are ignored.

ImplicitSuccessResponseFactory or AutoSuccessResponseFactory maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ImplicitSuccessResponseFactory is best, but I dont have very strong feelings

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
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