From 5ba6b0ed2274ff575dd3ab530f39b2fecef5fd5d Mon Sep 17 00:00:00 2001 From: Stelios Petrakis Date: Fri, 7 Jul 2023 11:27:33 +0200 Subject: [PATCH 1/2] Improve CDSHandler push logic The `pushTranslations()` method now returns a secondary array of `Error` objects in the completion handler that represents the generated warnings during the processing of the source strings that are non-fatal but can be of interest to the developer. This change introduces a new enum (`TXCDSWarning`) that hosts those warnings, like duplicate source string and empty key detection. The `pushTranslations()` logic has been improved by implementing the following changes: * `serializeTranslations()` is now a private static method as it does not need to look into the state of the `CDSHandler` instance. * That method now returns a tuple that includes a `Result` object as the first member, revealing whether the serialization was successful, thus offering the `Data` object, or a failure, thus offering a `Error` object for further propagation and an array of `TXCDSWarning` objects for any generated warnings during the processing of the source strings. * The `pushTranslations()` implementation has been split so that it's easier to be read. The new method that performs the asynchronous network operation is now separate (`pushData()`) and propagates the generated warnings to the `pollJobStatus()` method. * Any error logging has been removed in favor of the list of `Error` objects in the completion handler. The logging is now a responsibility of the consumer. --- Sources/Transifex/CDSHandler.swift | 152 ++++++++++++---------- Sources/Transifex/Core.swift | 10 +- Tests/TransifexTests/TransifexTests.swift | 4 +- 3 files changed, 90 insertions(+), 76 deletions(-) diff --git a/Sources/Transifex/CDSHandler.swift b/Sources/Transifex/CDSHandler.swift index f4b3ae3..62b685b 100644 --- a/Sources/Transifex/CDSHandler.swift +++ b/Sources/Transifex/CDSHandler.swift @@ -18,7 +18,7 @@ public enum TXCDSError: Error { /// No locale codes were provided to the fetch operation case noLocaleCodes /// Translation strings to be pushed failed to be serialized - case failedSerialization + case failedSerialization(error: Error) /// The CDS request failed with a specific underlying error case requestFailed(error : Error) /// The HTTP response received by CDS was invalid @@ -33,11 +33,22 @@ public enum TXCDSError: Error { case noData /// The job status request failed case failedJobRequest + /// There is no generated data to be sent to CDS + case noDataToBeSent /// A specific job error was returned by CDS case jobError(status: String, code: String, title: String, detail: String, source: [String : String]) } +/// All possible warnings that may be produced when pushing strings to CDS. +public enum TXCDSWarning: Error { + /// A duplicate source string pair has been detected. + case duplicateSourceString(sourceString: String, + duplicate: String) + /// A source string with an empty key has been detected. + case emptyKey(SourceString: String) +} + /// Handles the logic of a pull HTTP request to CDS for a certain locale code class CDSPullRequest { let code : String @@ -401,24 +412,39 @@ class CDSHandler { /// - completionHandler: a callback function to call when the operation is complete public func pushTranslations(_ translations: [TXSourceString], purge: Bool = false, - completionHandler: @escaping (Bool, [Error]) -> Void) { - guard let cdsHostURL = URL(string: configuration.cdsHost) else { - Logger.error("Error: Invalid CDS host URL: \(configuration.cdsHost)") - completionHandler(false, - [TXCDSError.invalidCDSURL]) - return + completionHandler: @escaping (Bool, [TXCDSError], [TXCDSWarning]) -> Void) { + let serializedResult = Self.serializeTranslations(translations, + purge: purge) + switch serializedResult.0 { + case .success(let jsonData): + guard jsonData.count > 0 else { + completionHandler(false, [.noDataToBeSent], serializedResult.1) + return + } + Logger.verbose("Pushing translations to CDS: \(translations)...") + pushData(jsonData, + warnings: serializedResult.1, + completionHandler: completionHandler) + case .failure(let error): + completionHandler(false, [.failedSerialization(error: error)], serializedResult.1) } - - guard let jsonData = serializeTranslations(translations, - purge: purge) else { - Logger.error("Error while serializing translations") - completionHandler(false, - [TXCDSError.failedSerialization]) + } + + /// Pushes the generated JSON data containing the source strings and propagates any generated + /// warnings to the final completion handler. + /// + /// - Parameters: + /// - jsonData: The generated JSON data + /// - warnings: Any generated CDS warnings that have been generated + /// - completionHandler: Callback function to be called when the push operation completes. + private func pushData(_ jsonData: Data, + warnings: [TXCDSWarning], + completionHandler: @escaping (Bool, [TXCDSError], [TXCDSWarning]) -> Void) { + guard let cdsHostURL = URL(string: configuration.cdsHost) else { + completionHandler(false, [.invalidCDSURL], warnings) return } - Logger.verbose("Pushing translations to CDS: \(translations)...") - let baseURL = cdsHostURL.appendingPathComponent(CDSHandler.CONTENT_ENDPOINT) var request = URLRequest(url: baseURL) request.httpBody = jsonData @@ -426,31 +452,23 @@ class CDSHandler { request.allHTTPHeaderFields = getHeaders(withSecret: true) session.dataTask(with: request) { (data, response, error) in - guard error == nil else { - Logger.error("Error pushing strings: \(error!)") - completionHandler(false, - [TXCDSError.requestFailed(error: error!)]) + if let error = error { + completionHandler(false, [.requestFailed(error: error)], warnings) return } guard let httpResponse = response as? HTTPURLResponse else { - Logger.error("Error pushing strings: Not a valid HTTP response") - completionHandler(false, - [TXCDSError.invalidHTTPResponse]) + completionHandler(false, [.invalidHTTPResponse], warnings) return } if httpResponse.statusCode != CDSHandler.HTTP_STATUS_CODE_ACCEPTED { - Logger.error("HTTP Status error while pushing strings: \(httpResponse.statusCode)") - completionHandler(false, - [TXCDSError.serverError(statusCode: httpResponse.statusCode)]) + completionHandler(false, [.serverError(statusCode: httpResponse.statusCode)], warnings) return } guard let data = data else { - Logger.error("Error: No data received while pushing strings") - completionHandler(false, - [TXCDSError.noData]) + completionHandler(false, [.noData], warnings) return } @@ -461,17 +479,15 @@ class CDSHandler { response = try decoder.decode(PushResponseData.self, from: data) } - catch { - Logger.error("Error while decoding CDS push response: \(error)") - } + catch { } guard let finalResponse = response else { - completionHandler(false, - [TXCDSError.nonParsableResponse]) + completionHandler(false, [.nonParsableResponse], warnings) return } self.pollJobStatus(jobURL: finalResponse.data.links.job, + warnings: warnings, retryCount: 0, completionHandler: completionHandler) @@ -490,8 +506,9 @@ class CDSHandler { /// - completionHandler: The completion handler that informs the caller whether the job was /// successful or not. private func pollJobStatus(jobURL: String, + warnings: [TXCDSWarning], retryCount: Int, - completionHandler: @escaping (Bool, [Error]) -> Void) { + completionHandler: @escaping (Bool, [TXCDSError], [TXCDSWarning]) -> 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) @@ -499,27 +516,19 @@ class CDSHandler { fetchJobStatus(jobURL: jobURL) { jobStatus, jobErrors, jobDetails in guard let finalJobStatus = jobStatus else { - Logger.error("Error: Fetch job status request failed") - completionHandler(false, - [TXCDSError.failedJobRequest]) + completionHandler(false, [.failedJobRequest], warnings) return } - var finalErrors: [Error] = [] + var finalErrors: [TXCDSError] = [] - if let errors = jobErrors { + if let errors = jobErrors, errors.count > 0 { for error in errors { - Logger.error(""" -\(error.title) (\(error.status) - \(error.code)): -\(error.detail) -Source: -\(error.source) -""") - finalErrors.append(TXCDSError.jobError(status: error.status, - code: error.code, - title: error.title, - detail: error.detail, - source: error.source)) + finalErrors.append(.jobError(status: error.status, + code: error.code, + title: error.title, + detail: error.detail, + source: error.source)) } } @@ -539,16 +548,17 @@ failed: \(details.failed) case .processing: if retryCount < CDSHandler.MAX_RETRIES { self.pollJobStatus(jobURL: jobURL, + warnings: warnings, retryCount: retryCount + 1, completionHandler: completionHandler) } else { - completionHandler(false, [TXCDSError.maxRetriesReached]) + completionHandler(false, [.maxRetriesReached], warnings) } case .failed: - completionHandler(false, finalErrors) + completionHandler(false, finalErrors, warnings) case .completed: - completionHandler(true, finalErrors) + completionHandler(true, finalErrors, warnings) } } } @@ -627,28 +637,30 @@ failed: \(details.failed) /// /// - Parameter translations: a list of `TXSourceString` objects /// - Parameter purge: Whether the resulting data will replace the entire resource content or not - /// - Returns: a Data object ready to be used in the CDS request - private func serializeTranslations(_ translations: [TXSourceString], - purge: Bool = false) -> Data? { + /// - Returns: A tuple containing the Result object that either contains the Data object ready to be + /// used in the CDS request or an error and the list of warnings generated during processing. + private static func serializeTranslations(_ translations: [TXSourceString], + purge: Bool = false) -> (Result, [TXCDSWarning]) { var sourceStrings: [String:SourceString] = [:] - + var warnings: [TXCDSWarning] = [] + for translation in translations { - sourceStrings[translation.key] = translation.sourceStringRepresentation() + let key = translation.key + let sourceString = translation.sourceStringRepresentation() + if let duplicateSourceString = sourceStrings[key] { + warnings.append(.duplicateSourceString(sourceString: sourceString.debugDescription, + duplicate: duplicateSourceString.debugDescription)) + } + if key.trimmingCharacters(in: .whitespacesAndNewlines).count == 0 { + warnings.append(.emptyKey(SourceString: sourceString.debugDescription)) + } + sourceStrings[key] = translation.sourceStringRepresentation() } - + let data = PushData(data: sourceStrings, meta: PushData.Meta(purge: purge)) - - var jsonData: Data? - - do { - jsonData = try JSONEncoder().encode(data) - } - catch { - Logger.error("Error encoding source strings: \(error)") - } - - return jsonData + + return (Result { try JSONEncoder().encode(data) }, warnings) } /// Return the headers to use when making requests. diff --git a/Sources/Transifex/Core.swift b/Sources/Transifex/Core.swift index 6e4f6f5..37cd907 100644 --- a/Sources/Transifex/Core.swift +++ b/Sources/Transifex/Core.swift @@ -181,10 +181,11 @@ class NativeCore : TranslationProvider { /// - purge: Whether to replace the entire resource content (true) or not (false). Defaults to false. /// - completionHandler: A callback to be called when the push operation is complete with a /// boolean argument that informs the caller that the operation was successful (true) or not (false) and - /// an array that may or may not contain any errors produced during the push operation. + /// an array that may or may not contain any errors produced during the push operation and an array of + /// non-blocking errors (warnings) that may have been generated during the push procedure. func pushTranslations(_ translations: [TXSourceString], purge: Bool = false, - completionHandler: @escaping (Bool, [Error]) -> Void) { + completionHandler: @escaping (Bool, [Error], [Error]) -> Void) { cdsHandler.pushTranslations(translations, purge: purge, completionHandler: completionHandler) @@ -538,11 +539,12 @@ token: \(token) /// - purge: Whether to replace the entire resource content (true) or not (false). Defaults to false. /// - completionHandler: A callback to be called when the push operation is complete with a /// boolean argument that informs the caller that the operation was successful (true) or not (false) and - /// an array that may or may not contain any errors produced during the push operation. + /// an array that may or may not contain any errors produced during the push operation and an array of + /// non-blocking errors (warnings) that may have been generated during the push procedure. @objc public static func pushTranslations(_ translations: [TXSourceString], purge: Bool = false, - completionHandler: @escaping (Bool, [Error]) -> Void) { + completionHandler: @escaping (Bool, [Error], [Error]) -> Void) { tx?.pushTranslations(translations, purge: purge, completionHandler: completionHandler) diff --git a/Tests/TransifexTests/TransifexTests.swift b/Tests/TransifexTests/TransifexTests.swift index cdee2ba..769d139 100644 --- a/Tests/TransifexTests/TransifexTests.swift +++ b/Tests/TransifexTests/TransifexTests.swift @@ -537,7 +537,7 @@ final class TransifexTests: XCTestCase { session: urlSession) var pushResult = false - cdsHandler.pushTranslations(translations) { (result, errors) in + cdsHandler.pushTranslations(translations) { (result, errors, warnings) in pushResult = result expectation.fulfill() } @@ -601,7 +601,7 @@ final class TransifexTests: XCTestCase { session: urlSession) var pushResult = false - cdsHandler.pushTranslations(translations) { (result, errors) in + cdsHandler.pushTranslations(translations) { (result, errors, warnings) in pushResult = result expectation.fulfill() } From 9f7b6e583dafa68447765cb80c59fa93f580d9c7 Mon Sep 17 00:00:00 2001 From: Stelios Petrakis Date: Fri, 7 Jul 2023 11:27:50 +0200 Subject: [PATCH 2/2] Introduce t() method A new public static method has been introduced in the `TXNative` class, allowing developers to programmatically trigger the translation mechanism of Transifex SDK in cases where this is not supported. For example, in SwiftUI, where the swizzling mechanism of Transifex SDK does currently work, developers can replace views like this: ``` Text("test string") ``` with this: ``` Text(TXNative.t("test string")) ``` The method uses the default locale that Transifex SDK has been configured with. Note: If you want to change the current locale used by the SwiftUI preview to test different locales, click on the 'Product' menu item and then visit 'Scheme' > 'Edit Scheme...' > 'Run' > 'Options' > 'App Language' and pick the language you want to test. --- Sources/Transifex/Core.swift | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Sources/Transifex/Core.swift b/Sources/Transifex/Core.swift index 37cd907..d9a6964 100644 --- a/Sources/Transifex/Core.swift +++ b/Sources/Transifex/Core.swift @@ -504,6 +504,23 @@ token: \(token) ) } + /// Helper method used when translation is not possible (e.g. in SwiftUI views). + /// + /// This method applies the translation using the currently selected locale. For pluralization use the + /// `localizedString(format:arguments:)` method. + /// + /// Make sure that this method is called after the SDK has been initialized, otherwise + /// "" string will be shown instead. + /// + /// - Parameter sourceString: The source string to be translated + /// - Returns: The translated string + public static func t(_ sourceString: String) -> String { + return tx?.translate(sourceString: sourceString, + localeCode: nil, + params: [:], + context: nil) ?? "" + } + /// Used by the Swift localizedString(format:arguments:) methods found in the /// TXExtensions.swift file. public static func localizedString(format: String,