Skip to content

Commit

Permalink
Implement logging for the security code component (#492)
Browse files Browse the repository at this point in the history
* Implement logging for the security code component

* Fix logging issue

* Fix unit tests

These two assertions have never been true when
called independently. They were being asserted as true
because the StubLogManager's static queue array
was being populated by CardValidatorTests and was
never being flushed.

So, whenever the CheckoutTests are run altogether, tests
were passing, but, whenever they are called separately,
they were failing.
  • Loading branch information
okhan-okbay-cko authored Nov 2, 2023
1 parent d151ef4 commit 2f9d5c6
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 8 deletions.
5 changes: 5 additions & 0 deletions Checkout/Source/Logging/CheckoutLogEvent+Types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ extension CheckoutLogEvent {
let publicKey: String
}

struct SecurityCodeTokenRequestData: Equatable {
let tokenType: SecurityCodeTokenType?
let publicKey: String
}

struct TokenResponseData: Equatable {
let tokenID: String?
let scheme: String?
Expand Down
28 changes: 24 additions & 4 deletions Checkout/Source/Logging/CheckoutLogEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ enum CheckoutLogEvent: Equatable {
case validateExpiryString
case validateExpiryInteger
case validateCVV
case cvvRequested(SecurityCodeTokenRequestData)
case cvvResponse(SecurityCodeTokenRequestData, TokenResponseData)

func event(date: Date) -> Event {
Event(
Expand All @@ -39,9 +41,9 @@ enum CheckoutLogEvent: Equatable {

private var typeIdentifier: String {
switch self {
case .tokenRequested:
case .tokenRequested, .cvvRequested:
return "token_requested"
case .tokenResponse:
case .tokenResponse, .cvvResponse:
return "token_response"
case .cardValidator:
return "card_validator"
Expand All @@ -63,9 +65,10 @@ enum CheckoutLogEvent: Equatable {
.validateCardNumber,
.validateExpiryString,
.validateExpiryInteger,
.validateCVV:
.validateCVV,
.cvvRequested:
return .info
case .tokenResponse(_, let tokenResponseData):
case .tokenResponse(_, let tokenResponseData), .cvvResponse(_, let tokenResponseData):
return level(from: tokenResponseData.httpStatusCode)
}
}
Expand Down Expand Up @@ -102,6 +105,23 @@ enum CheckoutLogEvent: Equatable {
[.httpStatusCode: tokenResponseData.httpStatusCode],
[.serverError: tokenResponseData.serverError]
)
case .cvvRequested(let tokenRequestData):
return [
.tokenType: tokenRequestData.tokenType?.rawValue.lowercased(),
.publicKey: tokenRequestData.publicKey
].compactMapValues { $0 }

case let .cvvResponse(tokenRequestData, tokenResponseData):
return mergeDictionaries(
[
.tokenType: tokenRequestData.tokenType?.rawValue.lowercased(),
.publicKey: tokenRequestData.publicKey,
.tokenID: tokenResponseData.tokenID,
.scheme: tokenResponseData.scheme
],
[.httpStatusCode: tokenResponseData.httpStatusCode],
[.serverError: tokenResponseData.serverError]
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,8 @@ struct TokenData: Encodable, Equatable {
case securityCode = "cvv"
}
}

// For logging purposes only
enum SecurityCodeTokenType: String, Codable, Equatable {
case cvv
}
38 changes: 37 additions & 1 deletion Checkout/Source/Tokenisation/CheckoutAPIService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ extension CheckoutAPIService {

switch requestParameterResult {
case .success(let requestParameters):
logManager.queue(event: .cvvRequested(CheckoutLogEvent.SecurityCodeTokenRequestData(
tokenType: .cvv,
publicKey: publicKey
)))
createSecurityCodeToken(requestParameters: requestParameters, completion: completion)
case .failure(let error):
switch error {
Expand All @@ -200,7 +204,9 @@ extension CheckoutAPIService {
requestParameters,
responseType: SecurityCodeResponse.self,
responseErrorType: TokenisationError.ServerError.self
) { tokenResponseResult, httpURLResponse in
) { [logManager, logSecurityCodeTokenResponse] tokenResponseResult, httpURLResponse in
logSecurityCodeTokenResponse(tokenResponseResult, httpURLResponse)

switch tokenResponseResult {
case .response(let tokenResponse):
completion(.success(tokenResponse))
Expand All @@ -209,6 +215,36 @@ extension CheckoutAPIService {
case .networkError(let networkError):
completion(.failure(.networkError(networkError)))
}

logManager.resetCorrelationID()
}
}

private func logSecurityCodeTokenResponse(tokenResponseResult: NetworkRequestResult<SecurityCodeResponse, TokenisationError.ServerError>, httpURLResponse: HTTPURLResponse?) {
switch tokenResponseResult {
case .response(let tokenResponse):
let tokenRequestData = CheckoutLogEvent.SecurityCodeTokenRequestData(tokenType: .cvv, publicKey: publicKey)
let tokenResponseData = CheckoutLogEvent.TokenResponseData(
tokenID: tokenResponse.token,
scheme: nil,
httpStatusCode: httpURLResponse?.statusCode,
serverError: nil
)

logManager.queue(event: .cvvResponse(tokenRequestData, tokenResponseData))
case .errorResponse(let errorResponse):
let tokenRequestData = CheckoutLogEvent.SecurityCodeTokenRequestData(tokenType: nil, publicKey: publicKey)
let tokenResponseData = CheckoutLogEvent.TokenResponseData(
tokenID: nil,
scheme: nil,
httpStatusCode: httpURLResponse?.statusCode,
serverError: errorResponse
)

logManager.queue(event: .cvvResponse(tokenRequestData, tokenResponseData))
case .networkError:
// we received no response, so nothing to log
break
}
}
}
44 changes: 42 additions & 2 deletions CheckoutTests/Tokenisation/CheckoutAPIServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ final class CheckoutAPIServiceTests: XCTestCase {
var result: Result<TokenDetails, TokenisationError.TokenRequest>?
subject.createToken(.card(card)) { result = $0 }

XCTAssertEqual(StubLogManager.queueCalledWith.last, .validateCVV)
XCTAssertEqual(result, .failure(.cardValidationError(.cvv(.invalidLength))))
}

Expand All @@ -117,7 +116,6 @@ final class CheckoutAPIServiceTests: XCTestCase {
var result: Result<TokenDetails, TokenisationError.TokenRequest>?
subject.createToken(.card(card)) { result = $0 }

XCTAssertEqual(StubLogManager.queueCalledWith.last, .validateCVV)
XCTAssertEqual(result, .failure(.couldNotBuildURLForRequest))
}

Expand Down Expand Up @@ -231,6 +229,11 @@ extension CheckoutAPIServiceTests {
var result: Result<SecurityCodeResponse, TokenisationError.SecurityCodeError>?
subject.createSecurityCodeToken(securityCode: "123", completion: { result = $0 })

XCTAssertEqual(StubLogManager.queueCalledWith.last, .cvvRequested(.init(
tokenType: .cvv,
publicKey: "publicKey"
)))

stubSecurityCodeRequestExecutor.executeCalledWithCompletion?(.response(tokenResponse), HTTPURLResponse())

XCTAssertEqual(stubRequestFactory.createCalledWith, .securityCodeToken(request: tokenRequest, publicKey: "publicKey"))
Expand All @@ -239,6 +242,11 @@ extension CheckoutAPIServiceTests {
XCTAssertTrue(stubSecurityCodeRequestExecutor.executeCalledWithResponseType == SecurityCodeResponse.self)
XCTAssertTrue(stubSecurityCodeRequestExecutor.executeCalledWithResponseErrorType == TokenisationError.ServerError.self)

XCTAssertEqual(StubLogManager.queueCalledWith.last, .cvvResponse(
.init(tokenType: .cvv, publicKey: "publicKey"),
.init(tokenID: "some_token", scheme: nil, httpStatusCode: 200, serverError: nil)
))

XCTAssertEqual(result, .success(tokenResponse))
}

Expand Down Expand Up @@ -276,8 +284,23 @@ extension CheckoutAPIServiceTests {
var result: Result<SecurityCodeResponse, TokenisationError.SecurityCodeError>?
subject.createSecurityCodeToken(securityCode: "123", completion: { result = $0 })

XCTAssertEqual(StubLogManager.queueCalledWith.last, .cvvRequested(.init(
tokenType: .cvv,
publicKey: "publicKey"
)))

stubSecurityCodeRequestExecutor.executeCalledWithCompletion?(.errorResponse(serverError), HTTPURLResponse())

XCTAssertEqual(StubLogManager.queueCalledWith.last, .cvvResponse(
.init(tokenType: nil, publicKey: "publicKey"),
.init(
tokenID: nil,
scheme: nil,
httpStatusCode: 200,
serverError: .init(requestID: "requestID", errorType: "errorType", errorCodes: ["test", "value"])
)
))

XCTAssertEqual(stubRequestFactory.createCalledWith, .securityCodeToken(request: tokenRequest, publicKey: "publicKey"))

XCTAssertEqual(stubSecurityCodeRequestExecutor.executeCalledWithRequestParameters, requestParameters)
Expand All @@ -297,6 +320,11 @@ extension CheckoutAPIServiceTests {
var result: Result<SecurityCodeResponse, TokenisationError.SecurityCodeError>?
subject.createSecurityCodeToken(securityCode: "123", completion: { result = $0 })

XCTAssertEqual(StubLogManager.queueCalledWith.last, .cvvRequested(.init(
tokenType: .cvv,
publicKey: "publicKey"
)))

stubSecurityCodeRequestExecutor.executeCalledWithCompletion?(.networkError(.connectionFailed), HTTPURLResponse())

XCTAssertEqual(stubRequestFactory.createCalledWith, .securityCodeToken(request: tokenRequest, publicKey: "publicKey"))
Expand All @@ -307,4 +335,16 @@ extension CheckoutAPIServiceTests {

XCTAssertEqual(result, .failure(.networkError(.connectionFailed)))
}

func testCreateSEcurityCodeTokenWithoutAPIKey() {
let service = CheckoutAPIService(publicKey: "", environment: .sandbox)

service.createSecurityCodeToken(securityCode: "123") { result in
if case .failure(let failure) = result {
XCTAssertEqual(failure, .missingAPIKey)
} else {
XCTFail("Test should return a failure")
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/checkout/frames-ios";
requirement = {
branch = "feature/cvv-component-tokenisation";
branch = "feature/cvv-component-logging";
kind = branch;
};
};
Expand Down

0 comments on commit 2f9d5c6

Please sign in to comment.