This repository was archived by the owner on Sep 15, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 16
Modified locale handling to inspect request parameters #80
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e179e37
Podfile.lock was updated via pod install
sabaranski ef151d0
Partitioned existing locale handling tests
sabaranski 3c9c204
Modified OHHTTPStubsTestBlock methods to use URLComponents for shallo…
sabaranski 0b38086
Modified existing locale tests to use URLComponents
sabaranski 0e9b279
Introduced failing test to illustrate issue
sabaranski 7ab1087
Improved locale handling by inspecting request parameters
sabaranski 06f6c41
Updated podspec
sabaranski 38ea510
Removed stray comment observed during PR review
sabaranski fafdb59
Apply injected key to query item
sabaranski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,7 @@ open class WordPressComRestApi: NSObject { | |
|
|
||
| fileprivate let backgroundUploads: Bool | ||
|
|
||
| fileprivate static let localeKey = "locale" | ||
| static let localeKey = "locale" | ||
|
|
||
| fileprivate let oAuthToken: String? | ||
| fileprivate let userAgent: String? | ||
|
|
@@ -145,7 +145,7 @@ open class WordPressComRestApi: NSObject { | |
| success: @escaping SuccessResponseBlock, | ||
| failure: @escaping FailureReponseBlock) -> Progress? { | ||
|
|
||
| guard let URLString = buildRequestURLFor(path: urlString) else { | ||
| guard let URLString = buildRequestURLFor(path: urlString, parameters: parameters) else { | ||
| let error = NSError(domain: String(describing: WordPressComRestApiError.self), | ||
| code: WordPressComRestApiError.requestSerializationFailed.rawValue, | ||
| userInfo: [NSLocalizedDescriptionKey: NSLocalizedString("Failed to serialize request to the REST API.", comment: "Error message to show when wrong URL format is used to access the REST API")]) | ||
|
|
@@ -241,7 +241,7 @@ open class WordPressComRestApi: NSObject { | |
| success: @escaping SuccessResponseBlock, | ||
| failure: @escaping FailureReponseBlock) -> Progress? { | ||
|
|
||
| guard let URLString = buildRequestURLFor(path: URLString) else { | ||
| guard let URLString = buildRequestURLFor(path: URLString, parameters: parameters) else { | ||
| let error = NSError(domain: String(describing: WordPressComRestApiError.self), | ||
| code: WordPressComRestApiError.requestSerializationFailed.rawValue, | ||
| userInfo: [NSLocalizedDescriptionKey: NSLocalizedString("Failed to serialize request to the REST API.", comment: "Error message to show when wrong URL format is used to access the REST API")]) | ||
|
|
@@ -299,18 +299,52 @@ open class WordPressComRestApi: NSObject { | |
| return "\(String(describing: oAuthToken)),\(String(describing: userAgent))".hashValue | ||
| } | ||
|
|
||
| fileprivate func buildRequestURLFor(path: String) -> String? { | ||
| let pathWithLocale = appendLocaleIfNeeded(path) | ||
| /// This method assembles a valid request URL for the specified path & parameters. | ||
| /// The framework relies on a field (`appendsPreferredLanguageLocale`) to influence whether or not locale should be | ||
| /// added to the path of requests. This approach did not consider request parameters. | ||
| /// | ||
| /// This method now considers both the path and specified request parameters when performing the substitution. | ||
| /// It only accounts for the locale parameter. AlamoFire encodes other parameters via `SessionManager.request(_:method:parameters:encoding:headers:)` | ||
| /// | ||
| /// - Parameters: | ||
| /// - path: the path for the request, which might include `locale` | ||
| /// - parameters: the request parameters, which could conceivably include `locale` | ||
| /// - localeKey: the locale key to search for (`locale` in v1 endpoints, `_locale` for v2) | ||
| /// - Returns: a request URL if successful, `nil` otherwise. | ||
| /// | ||
| func buildRequestURLFor(path: String, parameters: [String: AnyObject]? = [:], localeKey: String = WordPressComRestApi.localeKey) -> String? { | ||
|
|
||
| let baseURL = URL(string: WordPressComRestApi.apiBaseURLString) | ||
| let requestURLString = URL(string: pathWithLocale, relativeTo: baseURL)?.absoluteString | ||
| return requestURLString | ||
|
|
||
| guard let requestURLString = URL(string: path, relativeTo: baseURL)?.absoluteString, | ||
| let urlComponents = URLComponents(string: requestURLString) else { | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| let urlComponentsWithLocale = applyLocaleIfNeeded(urlComponents: urlComponents, parameters: parameters, localeKey: localeKey) | ||
|
|
||
| return urlComponentsWithLocale?.url?.absoluteString | ||
| } | ||
|
|
||
| fileprivate func appendLocaleIfNeeded(_ path: String) -> String { | ||
| private func applyLocaleIfNeeded(urlComponents: URLComponents, parameters: [String: AnyObject]? = [:], localeKey: String) -> URLComponents? { | ||
| guard appendsPreferredLanguageLocale else { | ||
| return path | ||
| return urlComponents | ||
| } | ||
| return WordPressComRestApi.pathByAppendingPreferredLanguageLocale(path) | ||
|
|
||
| var componentsWithLocale = urlComponents | ||
| var existingQueryItems = componentsWithLocale.queryItems ?? [] | ||
|
|
||
| let existingLocaleQueryItems = existingQueryItems.filter { $0.name == localeKey } | ||
| if let parameters = parameters, parameters[localeKey] == nil, existingLocaleQueryItems.isEmpty { | ||
| let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug | ||
| let localeQueryItem = URLQueryItem(name: localeKey, value: preferredLanguageIdentifier) | ||
|
|
||
| existingQueryItems.append(localeQueryItem) | ||
| } | ||
| componentsWithLocale.queryItems = existingQueryItems | ||
|
|
||
| return componentsWithLocale | ||
| } | ||
|
|
||
| @objc public func temporaryFileURL(withExtension fileExtension: String) -> URL { | ||
|
|
@@ -412,7 +446,6 @@ extension WordPressComRestApi { | |
| let errorWithLocalizedMessage = NSError(domain: nsError.domain, code: nsError.code, userInfo:userInfo) | ||
| return errorWithLocalizedMessage | ||
| } | ||
|
|
||
| } | ||
|
|
||
| extension WordPressComRestApi { | ||
|
|
@@ -421,25 +454,6 @@ extension WordPressComRestApi { | |
| @objc class public func anonymousApi(userAgent: String) -> WordPressComRestApi { | ||
| return WordPressComRestApi(oAuthToken: nil, userAgent: userAgent) | ||
| } | ||
|
|
||
| /// Append the user's preferred device locale as a query param to the URL path. | ||
| /// If the locale already exists the original path is returned. | ||
| /// | ||
| /// - Parameters: | ||
| /// - path: A URL string. Can be an absolute or relative URL string. | ||
| /// | ||
| /// - Returns: The path with the locale appended, or the original path if it already had a locale param. | ||
| /// | ||
| @objc class public func pathByAppendingPreferredLanguageLocale(_ path: String) -> String { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we no longer need to use this method on ObjC in the main app?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I searched for consumers of this in the |
||
| let localeKey = WordPressComRestApi.localeKey | ||
| if path.isEmpty || path.contains("\(localeKey)=") { | ||
| return path | ||
| } | ||
| let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug | ||
| let separator = path.contains("?") ? "&" : "?" | ||
| return "\(path)\(separator)\(localeKey)=\(preferredLanguageIdentifier)" | ||
| } | ||
|
|
||
| } | ||
|
|
||
| @objc extension Progress { | ||
|
|
||
167 changes: 167 additions & 0 deletions
167
WordPressKitTests/WordPressComRestApiTests+Locale.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| import Foundation | ||
| import XCTest | ||
|
|
||
| import WordPressShared | ||
| @testable import WordPressKit | ||
|
|
||
| extension WordPressComRestApiTests { | ||
|
|
||
| func testThatAppendingLocaleWorks() { | ||
| // Given | ||
| let path = "/path/path" | ||
| let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug | ||
|
|
||
| // When | ||
| let localeAppendedPath = WordPressComRestApi().buildRequestURLFor(path: path) | ||
|
|
||
| // Then | ||
| XCTAssertNotNil(localeAppendedPath) | ||
| let actualURL = URL(string: localeAppendedPath!, relativeTo: URL(string: WordPressComRestApi.apiBaseURLString)) | ||
| XCTAssertNotNil(actualURL) | ||
|
|
||
| let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false) | ||
| XCTAssertNotNil(actualURLComponents) | ||
|
|
||
| let expectedPath = path | ||
| let actualPath = actualURLComponents!.path | ||
| XCTAssertEqual(expectedPath, actualPath) | ||
|
|
||
| let actualQueryItems = actualURLComponents!.queryItems | ||
| XCTAssertNotNil(actualQueryItems) | ||
|
|
||
| let expectedQueryItemCount = 1 | ||
| let actualQueryItemCount = actualQueryItems!.count | ||
| XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount) | ||
|
|
||
| let actualQueryItem = actualQueryItems!.first | ||
| XCTAssertNotNil(actualQueryItem!) | ||
|
|
||
| let actualQueryItemKey = actualQueryItem!.name | ||
| let expectedQueryItemKey = WordPressComRestApi.localeKey | ||
| XCTAssertEqual(expectedQueryItemKey, actualQueryItemKey) | ||
|
|
||
| let actualQueryItemValue = actualQueryItem!.value | ||
| XCTAssertNotNil(actualQueryItemValue) | ||
|
|
||
| let expectedQueryItemValue = preferredLanguageIdentifier | ||
| XCTAssertEqual(expectedQueryItemValue, actualQueryItemValue!) | ||
| } | ||
|
|
||
| func testThatAppendingLocaleWorksWithExistingParams() { | ||
| // Given | ||
| let path = "/path/path" | ||
| let params: [String: AnyObject] = [ | ||
| "someKey": "value" as AnyObject | ||
| ] | ||
|
|
||
| // When | ||
| let localeAppendedPath = WordPressComRestApi().buildRequestURLFor(path: path, parameters: params) | ||
|
|
||
| // Then | ||
| XCTAssertNotNil(localeAppendedPath) | ||
| let actualURL = URL(string: localeAppendedPath!, relativeTo: URL(string: WordPressComRestApi.apiBaseURLString)) | ||
| XCTAssertNotNil(actualURL) | ||
|
|
||
| let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false) | ||
| XCTAssertNotNil(actualURLComponents) | ||
|
|
||
| let expectedPath = "/path/path" | ||
| let actualPath = actualURLComponents!.path | ||
| XCTAssertEqual(expectedPath, actualPath) | ||
|
|
||
| let actualQueryItems = actualURLComponents!.queryItems | ||
| XCTAssertNotNil(actualQueryItems) | ||
|
|
||
| let expectedQueryItemCount = 1 | ||
| let actualQueryItemCount = actualQueryItems!.count | ||
| XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount) | ||
|
|
||
| let actualQueryString = actualURLComponents?.query | ||
| XCTAssertNotNil(actualQueryString) | ||
|
|
||
| let queryStringIncludesLocale = actualQueryString!.contains(WordPressComRestApi.localeKey) | ||
| XCTAssertTrue(queryStringIncludesLocale) | ||
| } | ||
|
|
||
| func testThatLocaleIsNotAppendedIfAlreadyIncludedInPath() { | ||
| // Given | ||
| let preferredLanguageIdentifier = "foo" | ||
| let path = "/path/path?locale=\(preferredLanguageIdentifier)" | ||
|
|
||
| // When | ||
| let localeAppendedPath = WordPressComRestApi().buildRequestURLFor(path: path) | ||
|
|
||
| // Then | ||
| XCTAssertNotNil(localeAppendedPath) | ||
| let actualURL = URL(string: localeAppendedPath!, relativeTo: URL(string: WordPressComRestApi.apiBaseURLString)) | ||
| XCTAssertNotNil(actualURL) | ||
|
|
||
| let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false) | ||
| XCTAssertNotNil(actualURLComponents) | ||
|
|
||
| let expectedPath = "/path/path" | ||
| let actualPath = actualURLComponents!.path | ||
| XCTAssertEqual(expectedPath, actualPath) | ||
|
|
||
| let actualQueryItems = actualURLComponents!.queryItems | ||
| XCTAssertNotNil(actualQueryItems) | ||
|
|
||
| let expectedQueryItemCount = 1 | ||
| let actualQueryItemCount = actualQueryItems!.count | ||
| XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount) | ||
|
|
||
| let actualQueryItem = actualQueryItems!.first | ||
| XCTAssertNotNil(actualQueryItem!) | ||
|
|
||
| let actualQueryItemKey = actualQueryItem!.name | ||
| let expectedQueryItemKey = WordPressComRestApi.localeKey | ||
| XCTAssertEqual(expectedQueryItemKey, actualQueryItemKey) | ||
|
|
||
| let actualQueryItemValue = actualQueryItem!.value | ||
| XCTAssertNotNil(actualQueryItemValue) | ||
|
|
||
| let expectedQueryItemValue = preferredLanguageIdentifier | ||
| XCTAssertEqual(expectedQueryItemValue, actualQueryItemValue!) | ||
| } | ||
|
|
||
| func testThatAppendingLocaleIgnoresIfAlreadyIncludedInRequestParameters() { | ||
| // Given | ||
| let inputPath = "/path/path" | ||
| let expectedLocaleValue = "foo" | ||
| let params: [String : AnyObject] = [ | ||
| WordPressComRestApi.localeKey: expectedLocaleValue as AnyObject | ||
| ] | ||
|
|
||
| // When | ||
| let requestURLString = WordPressComRestApi().buildRequestURLFor(path: inputPath, parameters: params) | ||
|
|
||
| // Then | ||
| let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug | ||
| XCTAssertFalse(requestURLString!.contains(preferredLanguageIdentifier)) | ||
| } | ||
|
|
||
| func testThatLocaleIsNotAppendedWhenDisabled() { | ||
| // Given | ||
| let path = "/path/path" | ||
|
|
||
| // When | ||
| let api = WordPressComRestApi() | ||
| api.appendsPreferredLanguageLocale = false | ||
| let localeAppendedPath = api.buildRequestURLFor(path: path) | ||
|
|
||
| // Then | ||
| XCTAssertNotNil(localeAppendedPath) | ||
| let actualURL = URL(string: localeAppendedPath!, relativeTo: URL(string: WordPressComRestApi.apiBaseURLString)) | ||
| XCTAssertNotNil(actualURL) | ||
|
|
||
| let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false) | ||
| XCTAssertNotNil(actualURLComponents) | ||
|
|
||
| let expectedPath = path | ||
| let actualPath = actualURLComponents!.path | ||
| XCTAssertEqual(expectedPath, actualPath) | ||
|
|
||
| let actualQueryItems = actualURLComponents!.queryItems | ||
| XCTAssertNil(actualQueryItems) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.