From dd8e3cdc04173a619c49a8670f5daa0934d5c3f4 Mon Sep 17 00:00:00 2001 From: Stelios Petrakis Date: Wed, 28 Jul 2021 10:15:17 +0200 Subject: [PATCH] Update CDSHandler logic to v2 * Updates `CDSHandler` logic so that it can work with the version 2 of the CDS documentation. * Updates the response data structures that the new CDS version produces. * Updates the push logic with the new job oriented approach. * Updates unit test logic. * Adds unit tests for the new job oriented approach on push. --- Sources/Transifex/CDSHandler.swift | 214 +++++++++++++++++++--- Tests/TransifexTests/TransifexTests.swift | 178 +++++++++++------- 2 files changed, 298 insertions(+), 94 deletions(-) diff --git a/Sources/Transifex/CDSHandler.swift b/Sources/Transifex/CDSHandler.swift index c636b9c..0cee97b 100644 --- a/Sources/Transifex/CDSHandler.swift +++ b/Sources/Transifex/CDSHandler.swift @@ -13,9 +13,6 @@ public typealias TXPullCompletionHandler = (TXTranslations, [Error]) -> Void /// Handles the logic of a pull HTTP request to CDS for a certain locale code class CDSPullRequest { - - private static let MAX_RETRIES = 20 - let code : String let request : URLRequest let session : URLSession @@ -92,7 +89,7 @@ class CDSPullRequest { case CDSHandler.HTTP_STATUS_CODE_ACCEPTED: Logger.info("Received 202 response while fetching locale: \(self.code)") - if self.retryCount < CDSPullRequest.MAX_RETRIES { + if self.retryCount < CDSHandler.MAX_RETRIES { self.retryCount += 1 self.perform(with: completionHandler) } @@ -112,11 +109,14 @@ class CDSPullRequest { /// Handles communication with the Content Delivery Service. class CDSHandler { + /// Max retries for both the pull and the push / job status requests + fileprivate static let MAX_RETRIES = 20 private static let CDS_HOST = "https://cds.svc.transifex.net" private static let CONTENT_ENDPOINT = "content" private static let INVALIDATE_ENDPOINT = "invalidate" + private static let FILTER_TAGS_PARAM = "filter[tags]" fileprivate static let HTTP_STATUS_CODE_OK = 200 @@ -134,28 +134,61 @@ class CDSHandler { /// Private structure that's used to parse the data received by the invalidate endpoint private struct InvalidationResponseData: Decodable { - var status: String - var token: String - var count: Int + struct Data: Decodable { + var status: String + var token: String + var count: Int + } + var data: Data } /// Private structure that's used to parse the server response when pushing source strings private struct PushResponseData: Decodable { + struct Links: Decodable { + var job: String + } + struct Data: Decodable { + var id: String + var links: Links + } + var data: Data + } + + /// Private structure that's used to parse the server response when fetching the job status. + /// + /// The errors field is available only in the 'completed' and 'failed' statuses and the details field is + /// available only in the 'completed' status. + private struct JobStatusResponseData: Decodable { + struct Data: Decodable { + var status: JobStatus + var errors: [JobError]? + var details: JobDetails? + } + var data: Data + } + + private struct JobDetails: Decodable { var created: Int var updated: Int var skipped: Int var deleted: Int var failed: Int - var errors: [PushResponseError] } - private struct PushResponseError: Decodable { + private struct JobError: Decodable { var status: String var code: String var title: String var detail: String var source: [String: String] } + + private enum JobStatus: String, Decodable { + case pending + case processing + case completed + case failed + } /// A list of locale codes for the configured languages in the application let localeCodes: [String] @@ -327,13 +360,13 @@ class CDSHandler { let response = try decoder.decode(InvalidationResponseData.self, from: data) - if response.status != "success" { + if response.data.status != "success" { Logger.error("Unsuccessful invalidation request") completionHandler(false) return } - Logger.verbose("Invalidated \(response.count) translations from CDS for all locales in the project") + Logger.verbose("Invalidated \(response.data.count) translations from CDS for all locales in the project") completionHandler(true) } catch { @@ -387,21 +420,14 @@ class CDSHandler { return } - let statusCode = httpResponse.statusCode - - if statusCode == CDSHandler.HTTP_STATUS_CODE_OK { - completionHandler(true) - return - } - - Logger.error("HTTP Status error while pushing strings: \(statusCode)") - - if statusCode == CDSHandler.HTTP_STATUS_CODE_FORBIDDEN { + if httpResponse.statusCode != CDSHandler.HTTP_STATUS_CODE_ACCEPTED { + Logger.error("HTTP Status error while pushing strings: \(httpResponse.statusCode)") completionHandler(false) return } guard let data = data else { + Logger.error("Error: No data received while pushing strings") completionHandler(false) return } @@ -417,8 +443,46 @@ class CDSHandler { Logger.error("Error while decoding CDS push response: \(error)") } - if let response = response { - for error in response.errors { + guard let finalResponse = response else { + completionHandler(false) + return + } + + self.pollJobStatus(jobURL: finalResponse.data.links.job, + retryCount: 0, + completionHandler: completionHandler) + + }.resume() + } + + /// Polls the job status for CDSHandler.MAX_RETRIES times, or until it receives a failure or a + /// successful job status. + /// + /// Warning: Do not call this method from the main thread as it sleeps for 1 second before performing + /// the actual network request. + /// + /// - Parameters: + /// - jobURL: The relative job url (e.g. /jobs/content/123) + /// - retryCount: The current retry number + /// - completionHandler: The completion handler that informs the caller whether the job was + /// successful or not. + private func pollJobStatus(jobURL: String, + retryCount: Int, + completionHandler: @escaping (Bool) -> Void) { + // Delay the job status request by 1 second, so that the server can + // have enough time to process the job. + Thread.sleep(forTimeInterval: 1.0) + + fetchJobStatus(jobURL: jobURL) { + jobStatus, jobErrors, jobDetails in + guard let finalJobStatus = jobStatus else { + Logger.error("Error: Fetch job status request failed") + completionHandler(false) + return + } + + if let errors = jobErrors { + for error in errors { Logger.error(""" \(error.title) (\(error.status) - \(error.code)): \(error.detail) @@ -427,8 +491,105 @@ Source: """) } } + + if let details = jobDetails { + Logger.verbose(""" +created: \(details.created) +updated: \(details.updated) +skipped: \(details.skipped) +deleted: \(details.deleted) +failed: \(details.failed) +""") + } + + switch finalJobStatus { + case .pending: + fallthrough + case .processing: + if retryCount < CDSHandler.MAX_RETRIES { + self.pollJobStatus(jobURL: jobURL, + retryCount: retryCount + 1, + completionHandler: completionHandler) + } + else { + Logger.error("Error: Max retries \(CDSHandler.MAX_RETRIES) reached") + completionHandler(false) + } + case .failed: + completionHandler(false) + case .completed: + completionHandler(true) + } + } + } + + /// Peforms a single job status request to the CDS for a given job id and returns the response + /// asynchronously + /// + /// - Parameters: + /// - jobURL: The relative job url (e.g. /jobs/content/123) + /// - completionHandler: A completion handler that contains the parsed response. The + /// response consists of the job status (which is nil in case of a failure), an optional array of errors + /// in case job failed or succeeded with errros and an optional structure of the job details in case job + /// was successful. + private func fetchJobStatus(jobURL: String, + completionHandler: @escaping (JobStatus?, + [JobError]?, + JobDetails?) -> Void) { + guard let cdsHostURL = URL(string: cdsHost) else { + Logger.error("Error: Invalid CDS host URL: \(cdsHost)") + completionHandler(nil, nil, nil) + return + } + + Logger.verbose("Fetching job status for job: \(jobURL)...") + + let baseURL = cdsHostURL + .appendingPathComponent(jobURL) + var request = URLRequest(url: baseURL) + request.httpMethod = "GET" + request.allHTTPHeaderFields = getHeaders(withSecret: true) - completionHandler(false) + session.dataTask(with: request) { (data, response, error) in + guard error == nil else { + Logger.error("Error retrieving job status: \(error!)") + completionHandler(nil, nil, nil) + return + } + + guard let httpResponse = response as? HTTPURLResponse else { + Logger.error("Error retrieving job status: Not a valid HTTP response") + completionHandler(nil, nil, nil) + return + } + + if httpResponse.statusCode != CDSHandler.HTTP_STATUS_CODE_OK { + Logger.error("HTTP Status error while retrieving job status: \(httpResponse.statusCode)") + completionHandler(nil, nil, nil) + return + } + + guard let finalData = data else { + Logger.error("Error: No data received while retrieving job status") + completionHandler(nil, nil, nil) + return + } + + let decoder = JSONDecoder() + var responseData: JobStatusResponseData? = nil + + do { + responseData = try decoder.decode(JobStatusResponseData.self, + from: finalData) + } + catch { + Logger.error("Error while decoding CDS job status response: \(error)") + } + + completionHandler(responseData?.data.status, + responseData?.data.errors, + responseData?.data.details) + }.resume() } @@ -471,8 +632,9 @@ Source: etag: String? = nil) -> [String: String] { var headers = [ "Accept-Encoding": "gzip", - "Content-Type": "application/json", - "X-NATIVE-SDK": "mobile/ios/\(TXNative.version)" + "Content-Type": "application/json; charset=utf-8", + "X-NATIVE-SDK": "mobile/ios/\(TXNative.version)", + "Accept-version": "v2" ] if withSecret == true, let secret = secret { diff --git a/Tests/TransifexTests/TransifexTests.swift b/Tests/TransifexTests/TransifexTests.swift index 9ddaee5..53823bd 100644 --- a/Tests/TransifexTests/TransifexTests.swift +++ b/Tests/TransifexTests/TransifexTests.swift @@ -16,10 +16,10 @@ class URLSessionDataTaskMock: URLSessionDataTask { } struct MockResponse { + var url : URL var data : Data? var statusCode : Int? var error : Error? - var url : URL? } class URLSessionMock: URLSession { @@ -34,61 +34,27 @@ class URLSessionMock: URLSession { with request: URLRequest, completionHandler: @escaping CompletionHandler ) -> URLSessionDataTask { - // For POST requests, return 200 if the data included in the - // mock response is equal to the data of the HTTP body of the - // request. - if request.httpMethod == "POST", - let mockResponse = mockResponses?[mockResponseIndex] { - mockResponseIndex += 1 - - return URLSessionDataTaskMock { - if mockResponse.data == request.httpBody { - let response = HTTPURLResponse(url: request.url!, - statusCode: 200, - httpVersion: nil, - headerFields: nil) - completionHandler(nil, response, nil) - } - else { - let response = HTTPURLResponse(url: request.url!, - statusCode: 403, - httpVersion: nil, - headerFields: nil) - completionHandler(nil, response, nil) - } - } - } - else if let mockResponse = mockResponses?[mockResponseIndex] { - mockResponseIndex += 1 - - let requestURL = request.url! - let data = mockResponse.data - let error = mockResponse.error - let statusCode = mockResponse.statusCode - let url = mockResponse.url - - var finalStatusCode = 200 - - if let statusCode = statusCode { - finalStatusCode = statusCode - } - else if let url = url { - finalStatusCode = (url == requestURL ? 200 : 403) - } - - return URLSessionDataTaskMock { - let response = HTTPURLResponse(url: requestURL, - statusCode: finalStatusCode, - httpVersion: nil, - headerFields: nil) - completionHandler(data, response, error) - } - } - else { + guard + let requestURL = request.url, + let mockResponse = mockResponses?[mockResponseIndex], + mockResponse.url == requestURL else { return URLSessionDataTaskMock { completionHandler(nil, nil, nil) } } + + mockResponseIndex += 1 + + let data = mockResponse.data + let error = mockResponse.error + + return URLSessionDataTaskMock { + let response = HTTPURLResponse(url: requestURL, + statusCode: mockResponse.statusCode ?? 200, + httpVersion: nil, + headerFields: nil) + completionHandler(data, response, error) + } } } @@ -230,11 +196,12 @@ final class TransifexTests: XCTestCase { func testFetchTranslationsWithTags() { let expectation = self.expectation(description: "Waiting for translations to be fetched") var translationErrors : [Error]? = nil + let mockResponseData = "{\"data\":{\"testkey1\":{\"string\":\"test string 1\"},\"testkey2\":{\"string\":\"test string 2\"}}}".data(using: .utf8) + let expectedURL = URL(string: "https://cds.svc.transifex.net/content/en?filter%5Btags%5D=ios")! - let expectedURL = URL(string: "https://cds.svc.transifex.net/content/en?filter%5Btags%5D=ios") - let mockResponse = MockResponse(data: mockResponseData, - url: expectedURL) + let mockResponse = MockResponse(url: expectedURL, + data: mockResponseData) let urlSession = URLSessionMock() urlSession.mockResponses = [mockResponse] @@ -260,10 +227,11 @@ final class TransifexTests: XCTestCase { var translationsResult : TXTranslations? = nil let mockResponseData = "{\"data\":{\"testkey1\":{\"string\":\"test string 1\"},\"testkey2\":{\"string\":\"test string 2\"}}}".data(using: .utf8) + let expectedURL = URL(string: "https://cds.svc.transifex.net/content/en")! - let mockResponse = MockResponse(data: mockResponseData, - statusCode: 200, - error: nil) + let mockResponse = MockResponse(url: expectedURL, + data: mockResponseData, + statusCode: 200) let urlSession = URLSessionMock() urlSession.mockResponses = [mockResponse] @@ -293,13 +261,13 @@ final class TransifexTests: XCTestCase { var translationsResult : TXTranslations? = nil let mockResponseData = "{\"data\":{\"testkey1\":{\"string\":\"test string 1\"},\"testkey2\":{\"string\":\"test string 2\"}}}".data(using: .utf8) + let expectedURL = URL(string: "https://cds.svc.transifex.net/content/en")! - let mockResponseNotReady = MockResponse(data: nil, - statusCode: 202, - error: nil) - let mockResponseReady = MockResponse(data: mockResponseData, - statusCode: 200, - error: nil) + let mockResponseNotReady = MockResponse(url: expectedURL, + statusCode: 202) + let mockResponseReady = MockResponse(url: expectedURL, + data: mockResponseData, + statusCode: 200) let urlSession = URLSessionMock() urlSession.mockResponses = [ @@ -337,17 +305,90 @@ final class TransifexTests: XCTestCase { characterLimit: 0) ] - let expectedDataString = "{\"meta\":{\"purge\":false},\"data\":{\"testkey\":{\"string\":\"sourceString\",\"meta\":{\"character_limit\":0,\"occurrences\":[]}}}}" + let expectedDataString = "{\"data\":{\"id\":\"123\",\"links\":{\"job\":\"/jobs/content/456\"}}}" + let expectedURL = URL(string: "https://cds.svc.transifex.net/content")! + + let mockResponse = MockResponse(url: expectedURL, + data: expectedDataString.data(using: .utf8), + statusCode: 202) + + let expectedJobDataString = "{\"data\":{\"details\":{\"created\":1,\"updated\":0,\"skipped\":0,\"deleted\":0,\"failed\":0},\"errors\":[],\"status\":\"completed\"}}" + let expectedJobURL = URL(string: "https://cds.svc.transifex.net/jobs/content/456")! + + let mockJobResponse = MockResponse(url: expectedJobURL, + data: expectedJobDataString.data(using: .utf8), + statusCode: 200) + + let urlSession = URLSessionMock() + urlSession.mockResponses = [ mockResponse, mockJobResponse ] + let cdsHandler = CDSHandler(localeCodes: [ "en" ], + token: "test_token", + session: urlSession) + + var pushResult = false + cdsHandler.pushTranslations(translations) { (result) in + pushResult = result + expectation.fulfill() + } + + waitForExpectations(timeout: 1.0) { (error) in + XCTAssertTrue(pushResult) + } + } + + func testPushTranslationsNotReady() { + let expectation = self.expectation(description: "Waiting for translations to be pushed") + let translations = [ + TXSourceString(key: "testkey", + sourceString: "sourceString", + occurrences: [], + characterLimit: 0) + ] + + let expectedDataString = "{\"data\":{\"id\":\"123\",\"links\":{\"job\":\"/jobs/content/456\"}}}" + let expectedURL = URL(string: "https://cds.svc.transifex.net/content")! - let mockResponse = MockResponse(data: expectedDataString.data(using: .utf8)) + let mockResponse = MockResponse(url: expectedURL, + data: expectedDataString.data(using: .utf8), + statusCode: 202) + + let expectedJobURL = URL(string: "https://cds.svc.transifex.net/jobs/content/456")! + + let expectedPendingJobDataString = "{\"data\":{\"status\":\"pending\"}}" + let mockPendingJobResponse = MockResponse(url: expectedJobURL, + data: expectedPendingJobDataString.data(using: .utf8), + statusCode: 200) + + let expectedProcessingJobDataString = "{\"data\":{\"status\":\"processing\"}}" + let mockProcessingJobResponse = MockResponse(url: expectedJobURL, + data: expectedProcessingJobDataString.data(using: .utf8), + statusCode: 200) + + let expectedCompletedJobDataString = "{\"data\":{\"details\":{\"created\":1,\"updated\":0,\"skipped\":0,\"deleted\":0,\"failed\":0},\"errors\":[],\"status\":\"completed\"}}" + let mockCompletedJobResponse = MockResponse(url: expectedJobURL, + data: expectedCompletedJobDataString.data(using: .utf8), + statusCode: 200) let urlSession = URLSessionMock() - urlSession.mockResponses = [ mockResponse ] + urlSession.mockResponses = [ + mockResponse, + mockPendingJobResponse, + mockPendingJobResponse, + mockPendingJobResponse, + mockPendingJobResponse, + mockPendingJobResponse, + mockProcessingJobResponse, + mockProcessingJobResponse, + mockProcessingJobResponse, + mockProcessingJobResponse, + mockProcessingJobResponse, + mockCompletedJobResponse + ] let cdsHandler = CDSHandler(localeCodes: [ "en" ], token: "test_token", session: urlSession) - var pushResult : Bool = false + var pushResult = false cdsHandler.pushTranslations(translations) { (result) in pushResult = result expectation.fulfill() @@ -612,6 +653,7 @@ final class TransifexTests: XCTestCase { ("testFetchTranslations", testFetchTranslations), ("testFetchTranslationsNotReady", testFetchTranslationsNotReady), ("testPushTranslations", testPushTranslations), + ("testPushTranslationsNotReady", testPushTranslationsNotReady), ("testReplaceAllPolicy", testReplaceAllPolicy), ("testUpdateUsingTranslatePolicy", testUpdateUsingTranslatePolicy), ("testReadOnlyCache", testReadOnlyCache),