From 5047084dada68b119a9390c72fc0b6a259a7cd1b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Jan 2024 09:10:49 +1300 Subject: [PATCH 01/12] Add a test case for the existing URL query encoding behaviour --- .../WordPressComRestApiTests.swift | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/WordPressKitTests/WordPressComRestApiTests.swift b/WordPressKitTests/WordPressComRestApiTests.swift index 1beea5d5..8148f1b7 100644 --- a/WordPressKitTests/WordPressComRestApiTests.swift +++ b/WordPressKitTests/WordPressComRestApiTests.swift @@ -60,6 +60,66 @@ class WordPressComRestApiTests: XCTestCase { } } + @available(iOS 16.0, *) + func testQuery() { + var requestURL: URL? + stub(condition: isRestAPIRequest()) { + requestURL = $0.url + return HTTPStubsResponse(error: URLError(URLError.Code.networkConnectionLost)) + } + + let expect = self.expectation(description: "One callback should be invoked") + let api = WordPressComRestApi(oAuthToken: "fakeToken") + api.GET( + wordPressMediaRoutePath, + parameters: [ + "number": 1, + "true": true, + "false": false, + "string": "true", + "dict": ["foo": true, "bar": "string"], + "nested-dict": [ + "outter1": [ + "inner1": "value1", + "inner2": "value2" + ], + "outter2": [ + "inner1": "value1", + "inner2": "value2" + ] + ], + "array": ["true", 1, false] + ] as [String: AnyObject], + success: { _, _ in expect.fulfill() }, + failure: { (_, _) in expect.fulfill() } + ) + wait(for: [expect], timeout: 0.1) + + let query = requestURL? + .query(percentEncoded: false)? + .split(separator: "&") + .reduce(into: Set()) { $0.insert(String($1)) } + let expected = [ + "number=1", + "true=1", + "false=0", + "string=true", + "dict[foo]=1", + "dict[bar]=string", + "nested-dict[outter1][inner1]=value1", + "nested-dict[outter1][inner2]=value2", + "nested-dict[outter2][inner1]=value1", + "nested-dict[outter2][inner2]=value2", + "array[]=true", + "array[]=1", + "array[]=0", + ] + + for item in expected { + XCTAssertTrue(query?.contains(item) == true, "Missing query item: \(item)") + } + } + func testSuccessfullCall() { stub(condition: isRestAPIRequest()) { _ in let stubPath = OHPathForFile("WordPressComRestApiMedia.json", type(of: self)) From 706079dde3f8817bd0ca453fb36e4521e5750ca4 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Jan 2024 09:17:20 +1300 Subject: [PATCH 02/12] Support encoding 'Any' query in HTTPRequestBuilder The implementation is the same as the existing query encoding behaviour. --- WordPressKit/HTTPRequestBuilder.swift | 23 ++++++++ .../Utilities/HTTPRequestBuilderTests.swift | 54 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index f5b30a23..059a1730 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -56,6 +56,29 @@ final class HTTPRequestBuilder { append(query: [URLQueryItem(name: name, value: value)], override: override) } + func query(_ dict: [String: Any]) -> Self { + dict.reduce(self) { + $0.query(name: $1.key, value: $1.value) + } + } + + func query(name: String, value: Any) -> Self { + switch value { + case let array as [Any]: + return array.reduce(self) { + $0.query(name: "\(name)[]", value: $1) + } + case let object as [String: Any]: + return object.reduce(self) { + $0.query(name: "\(name)[\($1.key)]", value: $1.value) + } + case let value as Bool: + return query(name: name, value: value ? "1" : "0", override: false) + default: + return query(name: name, value: "\(value)", override: false) + } + } + func append(query: [URLQueryItem], override: Bool = false) -> Self { var allQuery = urlComponents.queryItems ?? [] diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index a4798677..e10849f4 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -140,6 +140,60 @@ class HTTPRequestBuilderTests: XCTestCase { ) } + @available(iOS 16.0, *) + func testSetQueryWithDictionary() throws { + // The test data and assertions hre should be kept the same as the ones in `WordPressComRestApiTests.testQuery()`. + + let query = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) + .query( + [ + "number": 1, + "nsnumbe-true": NSNumber(value: true), + "true": true, + "false": false, + "string": "true", + "dict": ["foo": true, "bar": "string"], + "nested-dict": [ + "outter1": [ + "inner1": "value1", + "inner2": "value2" + ], + "outter2": [ + "inner1": "value1", + "inner2": "value2" + ] + ], + "array": ["true", 1, false] + ] + ) + .build() + .url? + .query(percentEncoded: false)? + .split(separator: "&") + .reduce(into: Set()) { $0.insert(String($1)) } + + let expected = [ + "number=1", + "nsnumbe-true=1", + "true=1", + "false=0", + "string=true", + "dict[foo]=1", + "dict[bar]=string", + "nested-dict[outter1][inner1]=value1", + "nested-dict[outter1][inner2]=value2", + "nested-dict[outter2][inner1]=value1", + "nested-dict[outter2][inner2]=value2", + "array[]=true", + "array[]=1", + "array[]=0", + ] + + for item in expected { + XCTAssertTrue(query?.contains(item) == true, "Missing query item: \(item)") + } + } + func testJSONBody() throws { var request = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) .method(.post) From 092c43a77fa720369c61f03b61ba6359e6027bc3 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Jan 2024 12:28:17 +1300 Subject: [PATCH 03/12] Support setting default query items in HTTPRequestBuilder --- WordPressKit/HTTPRequestBuilder.swift | 21 +++++++++++++++++-- .../Utilities/HTTPRequestBuilderTests.swift | 9 ++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index 059a1730..58c85fc9 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -16,6 +16,7 @@ final class HTTPRequestBuilder { private var urlComponents: URLComponents private var method: Method = .get private var headers: [String: String] = [:] + private var defaultQuery: [URLQueryItem] = [] private var bodyBuilder: ((inout URLRequest) throws -> Void)? init(url: URL) { @@ -52,6 +53,11 @@ final class HTTPRequestBuilder { return self } + func query(defaults: [URLQueryItem]) -> Self { + defaultQuery = defaults + return self + } + func query(name: String, value: String?, override: Bool = false) -> Self { append(query: [URLQueryItem(name: name, value: value)], override: override) } @@ -97,7 +103,8 @@ final class HTTPRequestBuilder { func body(form: [String: String]) -> Self { headers["Content-Type"] = "application/x-www-form-urlencoded; charset=utf-8" bodyBuilder = { req in - let content = form.map { + let content = form + .map { "\(HTTPRequestBuilder.urlEncode($0))=\(HTTPRequestBuilder.urlEncode($1))" } .joined(separator: "&") @@ -135,7 +142,17 @@ final class HTTPRequestBuilder { } func build() throws -> URLRequest { - guard let url = urlComponents.url else { + var allQuery = urlComponents.queryItems ?? [] + if !defaultQuery.isEmpty { + let allQueryKeys = allQuery.reduce(into: Set()) { $0.insert($1.name) } + let toBeAdded = defaultQuery.filter { !allQueryKeys.contains($0.name) } + allQuery.append(contentsOf: toBeAdded) + } + + var components = urlComponents + components.queryItems = allQuery.isEmpty ? nil : allQuery + + guard let url = components.url else { throw URLError(.badURL) } diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index e10849f4..f288ff3a 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -194,6 +194,15 @@ class HTTPRequestBuilderTests: XCTestCase { } } + func testDefaultQuery() throws { + let builder = HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) + .query(defaults: [URLQueryItem(name: "locale", value: "en")]) + + try XCTAssertEqual(builder.build().url?.query, "locale=en") + try XCTAssertEqual(builder.query(name: "locale", value: "zh").build().url?.query, "locale=zh") + try XCTAssertEqual(builder.query(name: "foo", value: "bar").build().url?.query, "locale=zh&foo=bar") + } + func testJSONBody() throws { var request = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) .method(.post) From cb8338cf9a447af2c2ec835d96b1f5151f78c068 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Jan 2024 16:32:49 +1300 Subject: [PATCH 04/12] Make sure adding query doesn't modify the original url --- WordPressKit/HTTPRequestBuilder.swift | 111 +++++++++++++++++--------- 1 file changed, 72 insertions(+), 39 deletions(-) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index 58c85fc9..164c34c4 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -1,5 +1,9 @@ import Foundation +/// A builder type that adding HTTP request data onto an URL. +/// +/// Calling this class's url related functions (the ones that changes path, query, etc) does not modify the +/// original URL string. The URL will be perserved in the final result that's returned by the `build` function. final class HTTPRequestBuilder { enum Method: String { case get = "GET" @@ -17,6 +21,7 @@ final class HTTPRequestBuilder { private var method: Method = .get private var headers: [String: String] = [:] private var defaultQuery: [URLQueryItem] = [] + private var appendedQuery: [URLQueryItem] = [] private var bodyBuilder: ((inout URLRequest) throws -> Void)? init(url: URL) { @@ -62,52 +67,25 @@ final class HTTPRequestBuilder { append(query: [URLQueryItem(name: name, value: value)], override: override) } - func query(_ dict: [String: Any]) -> Self { - dict.reduce(self) { - $0.query(name: $1.key, value: $1.value) - } - } - - func query(name: String, value: Any) -> Self { - switch value { - case let array as [Any]: - return array.reduce(self) { - $0.query(name: "\(name)[]", value: $1) - } - case let object as [String: Any]: - return object.reduce(self) { - $0.query(name: "\(name)[\($1.key)]", value: $1.value) - } - case let value as Bool: - return query(name: name, value: value ? "1" : "0", override: false) - default: - return query(name: name, value: "\(value)", override: false) - } + func query(_ parameters: [String: Any]) -> Self { + append(query: parameters.flatten(), override: false) } func append(query: [URLQueryItem], override: Bool = false) -> Self { - var allQuery = urlComponents.queryItems ?? [] - if override { let newKeys = Set(query.map { $0.name }) - allQuery.removeAll(where: { newKeys.contains($0.name) }) + appendedQuery.removeAll(where: { newKeys.contains($0.name) }) } - allQuery.append(contentsOf: query) - - urlComponents.queryItems = allQuery + appendedQuery.append(contentsOf: query) return self } - func body(form: [String: String]) -> Self { + func body(form: [String: Any]) -> Self { headers["Content-Type"] = "application/x-www-form-urlencoded; charset=utf-8" bodyBuilder = { req in - let content = form - .map { - "\(HTTPRequestBuilder.urlEncode($0))=\(HTTPRequestBuilder.urlEncode($1))" - } - .joined(separator: "&") + let content = form.flatten().percentEncoded req.httpBody = content.data(using: .utf8) } return self @@ -142,15 +120,27 @@ final class HTTPRequestBuilder { } func build() throws -> URLRequest { - var allQuery = urlComponents.queryItems ?? [] + var components = urlComponents + + // Add default query items if they don't exist in `appendedQuery`. + var newQuery = appendedQuery if !defaultQuery.isEmpty { - let allQueryKeys = allQuery.reduce(into: Set()) { $0.insert($1.name) } - let toBeAdded = defaultQuery.filter { !allQueryKeys.contains($0.name) } - allQuery.append(contentsOf: toBeAdded) + let toBeAdded = defaultQuery.filter { item in + !newQuery.contains(where: { $0.name == item.name}) + } + newQuery.append(contentsOf: toBeAdded) } - var components = urlComponents - components.queryItems = allQuery.isEmpty ? nil : allQuery + // Bypass `URLComponents`'s URL query encoding, use our own implementation instead. + if !newQuery.isEmpty { + var percentEncodedQuery = components.percentEncodedQuery ?? "" + if !percentEncodedQuery.isEmpty && !percentEncodedQuery.hasSuffix("&") { + percentEncodedQuery += "&" + } + percentEncodedQuery += newQuery.percentEncoded + + components.percentEncodedQuery = percentEncodedQuery + } guard let url = components.url else { throw URLError(.badURL) @@ -193,3 +183,46 @@ private extension HTTPRequestBuilder { return text.addingPercentEncoding(withAllowedCharacters: allowed) ?? text } } + +private extension Dictionary where Key == String, Value == Any { + + static func urlEncode(into result: inout [URLQueryItem], name: String, value: Any) { + switch value { + case let array as [Any]: + for value in array { + urlEncode(into: &result, name: "\(name)[]", value: value) + } + case let object as [String: Any]: + for (key, value) in object { + urlEncode(into: &result, name: "\(name)[\(key)]", value: value) + } + case let value as Bool: + urlEncode(into: &result, name: name, value: value ? "1" : "0") + default: + result.append(URLQueryItem(name: name, value: "\(value)")) + } + } + + func flatten() -> [URLQueryItem] { + reduce(into: []) { result, entry in + Self.urlEncode(into: &result, name: entry.key, value: entry.value) + } + } + +} + +extension Array where Element == URLQueryItem { + + var percentEncoded: String { + map { + let name = HTTPRequestBuilder.urlEncode($0.name) + guard let value = $0.value else { + return name + } + + return "\(name)=\(HTTPRequestBuilder.urlEncode(value))" + } + .joined(separator: "&") + } + +} From 40aaf3e1fc7f1e97d164256b14582f62e3fa73d0 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 08:56:49 +1300 Subject: [PATCH 05/12] Refactor appending path --- WordPressKit/HTTPRequestBuilder.swift | 47 ++++++++++++++----- .../Utilities/HTTPRequestBuilderTests.swift | 16 +++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index 164c34c4..fe855b77 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -19,6 +19,7 @@ final class HTTPRequestBuilder { private var urlComponents: URLComponents private var method: Method = .get + private var appendedPath: String = "" private var headers: [String: String] = [:] private var defaultQuery: [URLQueryItem] = [] private var appendedQuery: [URLQueryItem] = [] @@ -39,16 +40,7 @@ final class HTTPRequestBuilder { func append(path: String) -> Self { assert(!path.contains("?") && !path.contains("#"), "Path should not have query or fragment: \(path)") - var relPath = path - if relPath.hasPrefix("/") { - _ = relPath.removeFirst() - } - - if urlComponents.path.hasSuffix("/") { - urlComponents.path = urlComponents.path.appending(relPath) - } else { - urlComponents.path = urlComponents.path.appending("/").appending(relPath) - } + appendedPath = Self.join(appendedPath, path) return self } @@ -122,6 +114,12 @@ final class HTTPRequestBuilder { func build() throws -> URLRequest { var components = urlComponents + var newPath = Self.join(components.path, appendedPath) + if !newPath.isEmpty, !newPath.hasPrefix("/") { + newPath = "/\(newPath)" + } + components.path = newPath + // Add default query items if they don't exist in `appendedQuery`. var newQuery = appendedQuery if !defaultQuery.isEmpty { @@ -176,12 +174,39 @@ extension HTTPRequestBuilder { } } -private extension HTTPRequestBuilder { +extension HTTPRequestBuilder { static func urlEncode(_ text: String) -> String { let specialCharacters = ":#[]@!$&'()*+,;=" let allowed = CharacterSet.urlQueryAllowed.subtracting(.init(charactersIn: specialCharacters)) return text.addingPercentEncoding(withAllowedCharacters: allowed) ?? text } + + /// Join a list of strings using a separator only if neighbour items aren't already separated with the given separator. + static func join(_ aList: String..., separator: String = "/") -> String { + guard !aList.isEmpty else { return "" } + + var list = aList + let start = list.removeFirst() + return list.reduce(into: start) { result, path in + guard !path.isEmpty else { return } + + guard !result.isEmpty else { + result = path + return + } + + switch (result.hasSuffix(separator), path.hasPrefix(separator)) { + case (true, true): + var prefixRemoved = path + prefixRemoved.removePrefix(separator) + result.append(prefixRemoved) + case (true, false), (false, true): + result.append(path) + case (false, false): + result.append("\(separator)\(path)") + } + } + } } private extension Dictionary where Key == String, Value == Any { diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index f288ff3a..f3bd07a4 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -288,6 +288,22 @@ class HTTPRequestBuilderTests: XCTestCase { XCTAssertEqual(form, decodedForm) } + func testJoin() throws { + XCTAssertEqual(HTTPRequestBuilder.join("foo", "bar"), "foo/bar") + XCTAssertEqual(HTTPRequestBuilder.join("foo/", "bar"), "foo/bar") + XCTAssertEqual(HTTPRequestBuilder.join("foo", "/bar"), "foo/bar") + XCTAssertEqual(HTTPRequestBuilder.join("foo/", "/bar"), "foo/bar") + XCTAssertEqual(HTTPRequestBuilder.join("foo=1", "bar=2", separator: "&"), "foo=1&bar=2") + XCTAssertEqual(HTTPRequestBuilder.join("foo=1/", "bar=2", separator: "&"), "foo=1/&bar=2") + XCTAssertEqual(HTTPRequestBuilder.join("foo=1/", "&bar=2", separator: "&"), "foo=1/&bar=2") + + XCTAssertEqual(HTTPRequestBuilder.join("", "foo"), "foo") + XCTAssertEqual(HTTPRequestBuilder.join("foo", ""), "foo") + XCTAssertEqual(HTTPRequestBuilder.join("foo", "/"), "foo/") + XCTAssertEqual(HTTPRequestBuilder.join("/", "/foo"), "/foo") + XCTAssertEqual(HTTPRequestBuilder.join("", "/foo"), "/foo") + } + } private extension URLRequest { From 31171815aaaad4a60571ef5bfc515342bfacbf9f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 09:14:00 +1300 Subject: [PATCH 06/12] Use the same join path function in joining query strings --- WordPressKit/HTTPRequestBuilder.swift | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index fe855b77..77441933 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -131,13 +131,7 @@ final class HTTPRequestBuilder { // Bypass `URLComponents`'s URL query encoding, use our own implementation instead. if !newQuery.isEmpty { - var percentEncodedQuery = components.percentEncodedQuery ?? "" - if !percentEncodedQuery.isEmpty && !percentEncodedQuery.hasSuffix("&") { - percentEncodedQuery += "&" - } - percentEncodedQuery += newQuery.percentEncoded - - components.percentEncodedQuery = percentEncodedQuery + components.percentEncodedQuery = Self.join(components.percentEncodedQuery ?? "", newQuery.percentEncoded, separator: "&") } guard let url = components.url else { From 77e7f70a71a8a3121285fad5e9e0cd7c2dc53cd8 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 09:00:45 +1300 Subject: [PATCH 07/12] Change the original url variable name --- WordPressKit/HTTPRequestBuilder.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index 77441933..4853d59f 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -17,7 +17,7 @@ final class HTTPRequestBuilder { } } - private var urlComponents: URLComponents + private let original: URLComponents private var method: Method = .get private var appendedPath: String = "" private var headers: [String: String] = [:] @@ -29,7 +29,7 @@ final class HTTPRequestBuilder { assert(url.scheme == "http" || url.scheme == "https") assert(url.host != nil) - urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: true)! + original = URLComponents(url: url, resolvingAgainstBaseURL: true)! } func method(_ method: Method) -> Self { @@ -112,7 +112,7 @@ final class HTTPRequestBuilder { } func build() throws -> URLRequest { - var components = urlComponents + var components = original var newPath = Self.join(components.path, appendedPath) if !newPath.isEmpty, !newPath.hasPrefix("/") { From 825986b34e2770e49fd55fe7c229c3fd13e02ef8 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 09:40:46 +1300 Subject: [PATCH 08/12] Extract a test data into a shared constant --- .../Utilities/HTTPRequestBuilderTests.swift | 83 ++++++++++--------- .../WordPressComRestApiTests.swift | 40 ++------- 2 files changed, 49 insertions(+), 74 deletions(-) diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index f3bd07a4..d7426f21 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -5,6 +5,43 @@ import XCTest class HTTPRequestBuilderTests: XCTestCase { + static let nestedParameters: [String: Any] = + [ + "number": 1, + "nsnumbe-true": NSNumber(value: true), + "true": true, + "false": false, + "string": "true", + "dict": ["foo": true, "bar": "string"], + "nested-dict": [ + "outter1": [ + "inner1": "value1", + "inner2": "value2" + ], + "outter2": [ + "inner1": "value1", + "inner2": "value2" + ] + ], + "array": ["true", 1, false] + ] + static let nestedParametersEncoded = [ + "number=1", + "nsnumbe-true=1", + "true=1", + "false=0", + "string=true", + "dict[foo]=1", + "dict[bar]=string", + "nested-dict[outter1][inner1]=value1", + "nested-dict[outter1][inner2]=value2", + "nested-dict[outter2][inner1]=value1", + "nested-dict[outter2][inner2]=value2", + "array[]=true", + "array[]=1", + "array[]=0", + ] + func testURL() throws { try XCTAssertEqual(HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!).build().url?.absoluteString, "https://wordpress.org") try XCTAssertEqual(HTTPRequestBuilder(url: URL(string: "https://wordpress.com")!).build().url?.absoluteString, "https://wordpress.com") @@ -145,52 +182,18 @@ class HTTPRequestBuilderTests: XCTestCase { // The test data and assertions hre should be kept the same as the ones in `WordPressComRestApiTests.testQuery()`. let query = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) - .query( - [ - "number": 1, - "nsnumbe-true": NSNumber(value: true), - "true": true, - "false": false, - "string": "true", - "dict": ["foo": true, "bar": "string"], - "nested-dict": [ - "outter1": [ - "inner1": "value1", - "inner2": "value2" - ], - "outter2": [ - "inner1": "value1", - "inner2": "value2" - ] - ], - "array": ["true", 1, false] - ] - ) + .query(HTTPRequestBuilderTests.nestedParameters) .build() .url? .query(percentEncoded: false)? .split(separator: "&") .reduce(into: Set()) { $0.insert(String($1)) } + ?? [] - let expected = [ - "number=1", - "nsnumbe-true=1", - "true=1", - "false=0", - "string=true", - "dict[foo]=1", - "dict[bar]=string", - "nested-dict[outter1][inner1]=value1", - "nested-dict[outter1][inner2]=value2", - "nested-dict[outter2][inner1]=value1", - "nested-dict[outter2][inner2]=value2", - "array[]=true", - "array[]=1", - "array[]=0", - ] - - for item in expected { - XCTAssertTrue(query?.contains(item) == true, "Missing query item: \(item)") + XCTAssertEqual(query.count, HTTPRequestBuilderTests.nestedParametersEncoded.count) + + for item in HTTPRequestBuilderTests.nestedParametersEncoded { + XCTAssertTrue(query.contains(item), "Missing query item: \(item)") } } diff --git a/WordPressKitTests/WordPressComRestApiTests.swift b/WordPressKitTests/WordPressComRestApiTests.swift index 8148f1b7..c428e8dc 100644 --- a/WordPressKitTests/WordPressComRestApiTests.swift +++ b/WordPressKitTests/WordPressComRestApiTests.swift @@ -72,24 +72,7 @@ class WordPressComRestApiTests: XCTestCase { let api = WordPressComRestApi(oAuthToken: "fakeToken") api.GET( wordPressMediaRoutePath, - parameters: [ - "number": 1, - "true": true, - "false": false, - "string": "true", - "dict": ["foo": true, "bar": "string"], - "nested-dict": [ - "outter1": [ - "inner1": "value1", - "inner2": "value2" - ], - "outter2": [ - "inner1": "value1", - "inner2": "value2" - ] - ], - "array": ["true", 1, false] - ] as [String: AnyObject], + parameters: HTTPRequestBuilderTests.nestedParameters as [String: AnyObject], success: { _, _ in expect.fulfill() }, failure: { (_, _) in expect.fulfill() } ) @@ -99,24 +82,13 @@ class WordPressComRestApiTests: XCTestCase { .query(percentEncoded: false)? .split(separator: "&") .reduce(into: Set()) { $0.insert(String($1)) } - let expected = [ - "number=1", - "true=1", - "false=0", - "string=true", - "dict[foo]=1", - "dict[bar]=string", - "nested-dict[outter1][inner1]=value1", - "nested-dict[outter1][inner2]=value2", - "nested-dict[outter2][inner1]=value1", - "nested-dict[outter2][inner2]=value2", - "array[]=true", - "array[]=1", - "array[]=0", - ] + ?? [] + let expected = HTTPRequestBuilderTests.nestedParametersEncoded + ["locale=en"] + + XCTAssertEqual(query.count, expected.count) for item in expected { - XCTAssertTrue(query?.contains(item) == true, "Missing query item: \(item)") + XCTAssertTrue(query.contains(item), "Missing query item: \(item)") } } From 768f880502f5f0d0c31477ce7a1bcccba6f43318 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 09:54:08 +1300 Subject: [PATCH 09/12] Add an unit test to make sure the original url is not changed --- .../Utilities/HTTPRequestBuilderTests.swift | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index d7426f21..187c9160 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -307,6 +307,25 @@ class HTTPRequestBuilderTests: XCTestCase { XCTAssertEqual(HTTPRequestBuilder.join("", "/foo"), "/foo") } + func testPreserveOriginalURL() throws { + try XCTAssertEqual( + HTTPRequestBuilder(url: URL(string: "https://wordpress.org/api?locale=en")!) + .query(name: "locale", value: "zh") + .build() + .url? + .query, + "locale=en&locale=zh" + ) + try XCTAssertEqual( + HTTPRequestBuilder(url: URL(string: "https://wordpress.org/api?locale=en")!) + .query(name: "foo", value: "bar") + .build() + .url? + .query, + "locale=en&foo=bar" + ) + } + } private extension URLRequest { From 205979591b4aa1d020c4c23bced073ac1ff0d456 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 11:11:56 +1300 Subject: [PATCH 10/12] Remove trailing whitespaces --- WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index 187c9160..fa826565 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -191,7 +191,7 @@ class HTTPRequestBuilderTests: XCTestCase { ?? [] XCTAssertEqual(query.count, HTTPRequestBuilderTests.nestedParametersEncoded.count) - + for item in HTTPRequestBuilderTests.nestedParametersEncoded { XCTAssertTrue(query.contains(item), "Missing query item: \(item)") } From b94e4404845000f4f7a5e90864f16d77592e7799 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Jan 2024 12:41:56 +1300 Subject: [PATCH 11/12] Fix typos --- .../Utilities/HTTPRequestBuilderTests.swift | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index fa826565..327e5a61 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -14,11 +14,11 @@ class HTTPRequestBuilderTests: XCTestCase { "string": "true", "dict": ["foo": true, "bar": "string"], "nested-dict": [ - "outter1": [ + "outer1": [ "inner1": "value1", "inner2": "value2" ], - "outter2": [ + "outer2": [ "inner1": "value1", "inner2": "value2" ] @@ -33,10 +33,10 @@ class HTTPRequestBuilderTests: XCTestCase { "string=true", "dict[foo]=1", "dict[bar]=string", - "nested-dict[outter1][inner1]=value1", - "nested-dict[outter1][inner2]=value2", - "nested-dict[outter2][inner1]=value1", - "nested-dict[outter2][inner2]=value2", + "nested-dict[outer1][inner1]=value1", + "nested-dict[outer1][inner2]=value2", + "nested-dict[outer2][inner1]=value1", + "nested-dict[outer2][inner2]=value2", "array[]=true", "array[]=1", "array[]=0", @@ -179,8 +179,6 @@ class HTTPRequestBuilderTests: XCTestCase { @available(iOS 16.0, *) func testSetQueryWithDictionary() throws { - // The test data and assertions hre should be kept the same as the ones in `WordPressComRestApiTests.testQuery()`. - let query = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!) .query(HTTPRequestBuilderTests.nestedParameters) .build() From 59bd717519e4ce71a4091193bd2015ea59454607 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 24 Jan 2024 11:53:15 +1300 Subject: [PATCH 12/12] Fix typos --- WordPressKit/HTTPRequestBuilder.swift | 2 +- WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/WordPressKit/HTTPRequestBuilder.swift b/WordPressKit/HTTPRequestBuilder.swift index 4853d59f..0a6c3582 100644 --- a/WordPressKit/HTTPRequestBuilder.swift +++ b/WordPressKit/HTTPRequestBuilder.swift @@ -1,6 +1,6 @@ import Foundation -/// A builder type that adding HTTP request data onto an URL. +/// A builder type that appends HTTP request data to a URL. /// /// Calling this class's url related functions (the ones that changes path, query, etc) does not modify the /// original URL string. The URL will be perserved in the final result that's returned by the `build` function. diff --git a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift index 327e5a61..7dc698f5 100644 --- a/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift +++ b/WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift @@ -8,7 +8,7 @@ class HTTPRequestBuilderTests: XCTestCase { static let nestedParameters: [String: Any] = [ "number": 1, - "nsnumbe-true": NSNumber(value: true), + "nsnumber-true": NSNumber(value: true), "true": true, "false": false, "string": "true", @@ -27,7 +27,7 @@ class HTTPRequestBuilderTests: XCTestCase { ] static let nestedParametersEncoded = [ "number=1", - "nsnumbe-true=1", + "nsnumber-true=1", "true=1", "false=0", "string=true",