From 5dd5afbf78aba5d69e05afc7a1c382acfcae6378 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Wed, 3 Jan 2024 16:30:56 +0100 Subject: [PATCH 01/14] Add unit test checking Get Remote Feature Flag endpont query params existance --- WordPressKit/FeatureFlagRemote.swift | 19 ++++++++ .../Utilities/FeatureFlagRemoteTests.swift | 43 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index d8bce6c6..70ee62d4 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -54,4 +54,23 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { callback(.failure(error)) }) } + + struct GetRemoteFeatureFlagsEndpointParams { + let deviceId: String + let platform: String + let buildNumber: String + let marketingVersion: String + let identifier: String + } +} + +extension FeatureFlagRemote.GetRemoteFeatureFlagsEndpointParams: Decodable { + + enum CodingKeys: String, CodingKey { + case deviceId = "device_id" + case platform = "platform" + case buildNumber = "build_number" + case marketingVersion = "marketing_version" + case identifier = "identifier" + } } diff --git a/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift b/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift index 8e6b1e08..5ec33ebc 100644 --- a/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift +++ b/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift @@ -1,10 +1,40 @@ import XCTest +import OHHTTPStubs @testable import WordPressKit class FeatureFlagRemoteTests: RemoteTestCase, RESTTestable { private let endpoint = "/wpcom/v2/mobile/feature-flags" + func testThatRequestContainsQueryParams() throws { + let expectation = expectation(description: "Get Remote Feature Flags Endpoint should contain query params") + + let response = try makeResponse() + let expectedQueryParams = [ + "identifier": "com.apple.dt.xctest.tool", + "platform": "ios", + "build_number": "22516", + "marketing_version": "15.1", + "device_id": "Test" + ] + + stub { req -> Bool in + let containsQueryParams = containsQueryParams(expectedQueryParams)(req) + let matchesPath = isPath(self.endpoint)(req) + let matchesURL = containsQueryParams && matchesPath + XCTAssertTrue(matchesURL) + return matchesURL + } response: { request in + return response + } + + FeatureFlagRemote(wordPressComRestApi: getRestApi()).getRemoteFeatureFlags(forDeviceId: "Test") { _ in + expectation.fulfill() + } + + wait(for: [expectation], timeout: 1) + } + func testThatResponsesAreHandledCorrectly() throws { let flags = [ FeatureFlag(title: UUID().uuidString, value: true), @@ -78,4 +108,17 @@ class FeatureFlagRemoteTests: RemoteTestCase, RESTTestable { encoder.outputFormatting = [.sortedKeys, .prettyPrinted] return try encoder.encode(object) } + + private func makeResponse() throws -> HTTPStubsResponse { + return try XCTUnwrap({ + let flags = [ + FeatureFlag(title: UUID().uuidString, value: true), + FeatureFlag(title: UUID().uuidString, value: false) + ] + guard let data = try? JSONEncoder().encode(flags.dictionaryValue) else { + return nil + } + return HTTPStubsResponse(data: data, statusCode: 200, headers: [:]) + }()) + } } From d212c99a1544118b4977a6ddb4109c96755a74b5 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Wed, 3 Jan 2024 17:00:50 +0100 Subject: [PATCH 02/14] Add a new getRemoteFeatureFlags variant that accepts an object of type RemoteFeatureFlagsEndpointParams Add a new getRemoteFeatureFlags variant that accepts an object of type RemoteFeatureFlagsEndpointParams --- WordPressKit/FeatureFlagRemote.swift | 31 ++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index 70ee62d4..b9a7c239 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -9,17 +9,22 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } open func getRemoteFeatureFlags(forDeviceId deviceId: String, callback: @escaping FeatureFlagResponseCallback) { + self.getRemoteFeatureFlags(params: .init(deviceId: deviceId), callback: callback) + } + open func getRemoteFeatureFlags(params: RemoteFeatureFlagsEndpointParams, callback: @escaping FeatureFlagResponseCallback) { let endpoint = "mobile/feature-flags" let path = self.path(forEndpoint: endpoint, withVersion: ._2_0) + var parameters: [String: AnyObject]? - let parameters: [String: AnyObject] = [ - "device_id": deviceId as NSString, - "platform": "ios" as NSString, - "build_number": NSString(string: Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "Unknown"), - "marketing_version": NSString(string: Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown"), - "identifier": NSString(string: Bundle.main.bundleIdentifier ?? "Unknown") - ] + do { + let encoder = JSONEncoder() + let data = try encoder.encode(params) + parameters = try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] + } catch let error { + callback(.failure(error)) + return + } wordPressComRestApi.GET(path, parameters: parameters, @@ -55,7 +60,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { }) } - struct GetRemoteFeatureFlagsEndpointParams { + public struct RemoteFeatureFlagsEndpointParams { let deviceId: String let platform: String let buildNumber: String @@ -64,7 +69,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } } -extension FeatureFlagRemote.GetRemoteFeatureFlagsEndpointParams: Decodable { +extension FeatureFlagRemote.RemoteFeatureFlagsEndpointParams: Encodable { enum CodingKeys: String, CodingKey { case deviceId = "device_id" @@ -73,4 +78,12 @@ extension FeatureFlagRemote.GetRemoteFeatureFlagsEndpointParams: Decodable { case marketingVersion = "marketing_version" case identifier = "identifier" } + + init(deviceId: String, bundle: Bundle = .main) { + self.deviceId = deviceId + self.platform = "ios" + self.buildNumber = Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "Unknown" + self.marketingVersion = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown" + self.identifier = Bundle.main.bundleIdentifier ?? "Unknown" + } } From 4da0768053210fe3586d7b7bb8f31e3b92f1978e Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Wed, 3 Jan 2024 18:36:37 +0100 Subject: [PATCH 03/14] Add unit test checking Dashboard Cards endpoint and query params --- .../DashboardServiceRemoteTests.swift | 15 ++++++++++- WordPressKitTests/RemoteTestCase.swift | 25 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/WordPressKitTests/DashboardServiceRemoteTests.swift b/WordPressKitTests/DashboardServiceRemoteTests.swift index 3424af32..cfd22440 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -1,4 +1,5 @@ import XCTest +import OHHTTPStubs @testable import WordPressKit @@ -14,7 +15,19 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { // func testRequestCardsParam() { let expect = expectation(description: "Get cards successfully") - stubRemoteResponse("wpcom/v2/sites/165243437/dashboard/cards-data/?cards=posts,todays_stats", filename: "dashboard-200-with-drafts-and-scheduled-posts.json", contentType: .ApplicationJSON) + + let expectedQueryParams = [ + "cards": "posts,todays_stats", + "locale": "en" + ] + + stubRemoteResponse({ req in + let containsQueryParams = containsQueryParams(expectedQueryParams)(req) + let matchesPath = isPath("/wpcom/v2/sites/165243437/dashboard/cards-data")(req) + let matchesURL = containsQueryParams && matchesPath + XCTAssertTrue(matchesURL) + return matchesURL + }, filename: "dashboard-200-with-drafts-and-scheduled-posts.json", contentType: .ApplicationJSON) dashboardServiceRemote.fetch(cards: ["posts", "todays_stats"], forBlogID: 165243437) { _ in expect.fulfill() diff --git a/WordPressKitTests/RemoteTestCase.swift b/WordPressKitTests/RemoteTestCase.swift index d3ab060b..c6ae96f6 100644 --- a/WordPressKitTests/RemoteTestCase.swift +++ b/WordPressKitTests/RemoteTestCase.swift @@ -40,6 +40,31 @@ class RemoteTestCase: XCTestCase { // extension RemoteTestCase { + /// Helper function that creates a stub which uses a file for the response body. + /// + /// - Parameters: + /// - condition: The endpoint matcher block that determines if the request will be stubbed + /// - filename: The name of the file to use for the response + /// - contentType: The Content-Type returned in the response header + /// - status: The status code to use for the response. Defaults to 200. + /// + func stubRemoteResponse( + _ condition: @escaping (URLRequest) -> Bool, + filename: String, + contentType: ResponseContentType, + status: Int32 = 200 + ) { + stub(condition: condition) { _ in + let stubPath = OHPathForFile(filename, type(of: self)) + var headers: [NSObject: AnyObject]? + + if contentType != .NoContentType { + headers = ["Content-Type" as NSObject: contentType.rawValue as AnyObject] + } + return OHHTTPStubs.fixture(filePath: stubPath!, status: status, headers: headers) + } + } + /// Helper function that creates a stub which uses a file for the response body. /// /// - Parameters: From 6e6254aacef7c2bb965052dd5d19aadaea158c09 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Wed, 3 Jan 2024 18:39:05 +0100 Subject: [PATCH 04/14] Refactor DashboardServiceRemote fetch method --- WordPressKit/DashboardServiceRemote.swift | 23 +++++-------------- .../DashboardServiceRemoteTests.swift | 5 ++++ 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/WordPressKit/DashboardServiceRemote.swift b/WordPressKit/DashboardServiceRemote.swift index f002f3e7..9944710d 100644 --- a/WordPressKit/DashboardServiceRemote.swift +++ b/WordPressKit/DashboardServiceRemote.swift @@ -2,12 +2,14 @@ import Foundation open class DashboardServiceRemote: ServiceRemoteWordPressComREST { open func fetch(cards: [String], forBlogID blogID: Int, success: @escaping (NSDictionary) -> Void, failure: @escaping (Error) -> Void) { - guard let requestUrl = endpoint(for: cards, blogID: blogID) else { - return - } + let requestUrl = self.path(forEndpoint: "sites/\(blogID)/dashboard/cards-data/", withVersion: ._2_0) + + let params: [String: AnyObject] = [ + "cards": cards.joined(separator: ",") as NSString + ] wordPressComRestApi.GET(requestUrl, - parameters: nil, + parameters: params, success: { response, _ in guard let cards = response as? NSDictionary else { failure(ResponseError.decodingFailure) @@ -21,19 +23,6 @@ open class DashboardServiceRemote: ServiceRemoteWordPressComREST { }) } - private func endpoint(for cards: [String], blogID: Int) -> String? { - var path = URLComponents(string: "sites/\(blogID)/dashboard/cards-data/") - - let cardsEncoded = cards.joined(separator: ",") - path?.queryItems = [URLQueryItem(name: "cards", value: cardsEncoded)] - - guard let endpoint = path?.string else { - return nil - } - - return self.path(forEndpoint: endpoint, withVersion: ._2_0) - } - enum ResponseError: Error { case decodingFailure } diff --git a/WordPressKitTests/DashboardServiceRemoteTests.swift b/WordPressKitTests/DashboardServiceRemoteTests.swift index cfd22440..ca41e6b6 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -17,6 +17,11 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { let expect = expectation(description: "Get cards successfully") let expectedQueryParams = [ +// "identifier": "com.apple.dt.xctest.tool", +// "platform": "ios", +// "build_number": "22516", +// "marketing_version": "15.1", +// "device_id": "Test", "cards": "posts,todays_stats", "locale": "en" ] From 1f7ab134604ca1d1a6ea84d62a9e0e63b0e13a60 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Wed, 3 Jan 2024 18:51:55 +0100 Subject: [PATCH 05/14] Update DashboardServiceRemote fetch method to accept a deviceId param --- WordPressKit/DashboardServiceRemote.swift | 32 +++++++++++-- WordPressKit/FeatureFlagRemote.swift | 6 +-- .../DashboardServiceRemoteTests.swift | 47 ++++++++++++++----- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/WordPressKit/DashboardServiceRemote.swift b/WordPressKit/DashboardServiceRemote.swift index 9944710d..8ca76660 100644 --- a/WordPressKit/DashboardServiceRemote.swift +++ b/WordPressKit/DashboardServiceRemote.swift @@ -1,12 +1,21 @@ import Foundation open class DashboardServiceRemote: ServiceRemoteWordPressComREST { - open func fetch(cards: [String], forBlogID blogID: Int, success: @escaping (NSDictionary) -> Void, failure: @escaping (Error) -> Void) { + open func fetch( + cards: [String], + forBlogID blogID: Int, + deviceId: String, + success: @escaping (NSDictionary) -> Void, + failure: @escaping (Error) -> Void + ) { let requestUrl = self.path(forEndpoint: "sites/\(blogID)/dashboard/cards-data/", withVersion: ._2_0) + var params: [String: AnyObject]? - let params: [String: AnyObject] = [ - "cards": cards.joined(separator: ",") as NSString - ] + do { + params = try self.makeQueryParams(cards: cards, deviceId: deviceId) + } catch { + failure(error) + } wordPressComRestApi.GET(requestUrl, parameters: params, @@ -23,6 +32,21 @@ open class DashboardServiceRemote: ServiceRemoteWordPressComREST { }) } + private func makeQueryParams(cards: [String], deviceId: String) throws -> [String: AnyObject] { + let cardsParams: [String: AnyObject] = [ + "cards": cards.joined(separator: ",") as NSString + ] + let featureFlagParams = try { + let params = FeatureFlagRemote.RemoteFeatureFlagsEndpointParams(deviceId: deviceId) + let encoder = JSONEncoder() + let data = try encoder.encode(params) + return try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] + }() + return cardsParams.merging(featureFlagParams ?? [:]) { first, second in + return first + } + } + enum ResponseError: Error { case decodingFailure } diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index b9a7c239..f2e0509c 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -82,8 +82,8 @@ extension FeatureFlagRemote.RemoteFeatureFlagsEndpointParams: Encodable { init(deviceId: String, bundle: Bundle = .main) { self.deviceId = deviceId self.platform = "ios" - self.buildNumber = Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "Unknown" - self.marketingVersion = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown" - self.identifier = Bundle.main.bundleIdentifier ?? "Unknown" + self.buildNumber = bundle.infoDictionary?["CFBundleVersion"] as? String ?? "Unknown" + self.marketingVersion = bundle.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown" + self.identifier = bundle.bundleIdentifier ?? "Unknown" } } diff --git a/WordPressKitTests/DashboardServiceRemoteTests.swift b/WordPressKitTests/DashboardServiceRemoteTests.swift index ca41e6b6..25240dbf 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -17,11 +17,11 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { let expect = expectation(description: "Get cards successfully") let expectedQueryParams = [ -// "identifier": "com.apple.dt.xctest.tool", -// "platform": "ios", -// "build_number": "22516", -// "marketing_version": "15.1", -// "device_id": "Test", + "identifier": "com.apple.dt.xctest.tool", + "platform": "ios", + "build_number": "22516", + "marketing_version": "15.1", + "device_id": "Test", "cards": "posts,todays_stats", "locale": "en" ] @@ -34,7 +34,11 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { return matchesURL }, filename: "dashboard-200-with-drafts-and-scheduled-posts.json", contentType: .ApplicationJSON) - dashboardServiceRemote.fetch(cards: ["posts", "todays_stats"], forBlogID: 165243437) { _ in + dashboardServiceRemote.fetch( + cards: ["posts", "todays_stats"], + forBlogID: 165243437, + deviceId: "Test" + ) { _ in expect.fulfill() } failure: { _ in } @@ -45,9 +49,18 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { // func testRequestCards() { let expect = expectation(description: "Get cards successfully") - stubRemoteResponse("wpcom/v2/sites/165243437/dashboard/cards-data/?cards=posts,todays_stats", filename: "dashboard-200-with-drafts-and-scheduled-posts.json", contentType: .ApplicationJSON) - dashboardServiceRemote.fetch(cards: ["posts", "todays_stats"], forBlogID: 165243437) { cards in + stubRemoteResponse( + isPath("/wpcom/v2/sites/165243437/dashboard/cards-data"), + filename: "dashboard-200-with-drafts-and-scheduled-posts.json", + contentType: .ApplicationJSON + ) + + dashboardServiceRemote.fetch( + cards: ["posts", "todays_stats"], + forBlogID: 165243437, + deviceId: "Test" + ) { cards in XCTAssertTrue((cards["posts"] as! NSDictionary)["has_published"] as! Bool) XCTAssertEqual((cards["todays_stats"] as! NSDictionary)["views"] as! Int, 0) expect.fulfill() @@ -62,7 +75,11 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { let expect = expectation(description: "Get cards successfully") stubRemoteResponse("wpcom/v2/sites/165243437/dashboard/cards-data/?cards=posts,todays_stats", filename: "dashboard-200-with-drafts-and-scheduled-posts.json", contentType: .ApplicationJSON, status: 503) - dashboardServiceRemote.fetch(cards: ["posts", "todays_stats"], forBlogID: 165243437) { _ in + dashboardServiceRemote.fetch( + cards: ["posts", "todays_stats"], + forBlogID: 165243437, + deviceId: "Test" + ) { _ in XCTFail("This call should not suceed") } failure: { error in expect.fulfill() @@ -77,7 +94,11 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { let expect = expectation(description: "Get cards successfully") stubRemoteResponse("wpcom/v2/sites/165243437/dashboard/cards-data/?cards=invalid_card", filename: "dashboard-400-invalid-card.json", contentType: .ApplicationJSON, status: 400) - dashboardServiceRemote.fetch(cards: ["invalid_card"], forBlogID: 165243437) { _ in + dashboardServiceRemote.fetch( + cards: ["invalid_card"], + forBlogID: 165243437, + deviceId: "Test" + ) { _ in XCTFail("This call should not suceed") } failure: { error in expect.fulfill() @@ -92,7 +113,11 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { let expect = expectation(description: "Get cards successfully") stubRemoteResponse("wpcom/v2/sites/165243437/dashboard/cards-data/?cards=posts,todays_stats", data: "foo".data(using: .utf8)!, contentType: .ApplicationJSON) - dashboardServiceRemote.fetch(cards: ["posts", "todays_stats"], forBlogID: 165243437) { _ in + dashboardServiceRemote.fetch( + cards: ["posts", "todays_stats"], + forBlogID: 165243437, + deviceId: "Test" + ) { _ in XCTFail("This call should not suceed") } failure: { error in expect.fulfill() From fa7d74f605f611ba03c1f076b493df3b65b09f7e Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Thu, 4 Jan 2024 00:25:15 +0100 Subject: [PATCH 06/14] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4527f75e..e2de19e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ _None._ ### Breaking Changes -_None._ +- Add `deviceId` param to `DashboardServiceRemote.fetch` method. [#674] ### New Features From accfc22fada868a8b44247acaf15c9edb403ca43 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 02:20:39 +0100 Subject: [PATCH 07/14] Update testRequestCardsParam --- .../DashboardServiceRemoteTests.swift | 36 ++++++++++--------- WordPressKitTests/RemoteTestCase.swift | 30 ++++++++++++++++ .../Utilities/FeatureFlagRemoteTests.swift | 14 ++++---- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/WordPressKitTests/DashboardServiceRemoteTests.swift b/WordPressKitTests/DashboardServiceRemoteTests.swift index 25240dbf..86d62c61 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -14,24 +14,25 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { // Requests the correct set of cards // func testRequestCardsParam() { - let expect = expectation(description: "Get cards successfully") - - let expectedQueryParams = [ - "identifier": "com.apple.dt.xctest.tool", - "platform": "ios", - "build_number": "22516", - "marketing_version": "15.1", - "device_id": "Test", - "cards": "posts,todays_stats", - "locale": "en" + let expect = expectation(description: "Dashboard endpoint should contain query params") + let expectedPath = "/wpcom/v2/sites/165243437/dashboard/cards-data" + let expectedQueryParams: Set = [ + "identifier", + "platform", + "build_number", + "marketing_version", + "device_id", + "cards", + "locale" ] stubRemoteResponse({ req in - let containsQueryParams = containsQueryParams(expectedQueryParams)(req) - let matchesPath = isPath("/wpcom/v2/sites/165243437/dashboard/cards-data")(req) - let matchesURL = containsQueryParams && matchesPath - XCTAssertTrue(matchesURL) - return matchesURL + let url = req.url?.absoluteString ?? "" + let containsQueryParams = self.queryParams(expectedQueryParams, containedInRequest: req) + let matchesPath = isPath(expectedPath)(req) + XCTAssertTrue(matchesPath, "The URL '\(url)' doesn't match the expected path.") + XCTAssertTrue(containsQueryParams, "The URL '\(url)' doesn't contain the expected query params.") + return containsQueryParams && matchesPath }, filename: "dashboard-200-with-drafts-and-scheduled-posts.json", contentType: .ApplicationJSON) dashboardServiceRemote.fetch( @@ -40,7 +41,10 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { deviceId: "Test" ) { _ in expect.fulfill() - } failure: { _ in } + } failure: { error in + XCTFail("Dashboard cards request failed: \(error.localizedDescription)") + expect.fulfill() + } waitForExpectations(timeout: timeout, handler: nil) } diff --git a/WordPressKitTests/RemoteTestCase.swift b/WordPressKitTests/RemoteTestCase.swift index c6ae96f6..52b63d20 100644 --- a/WordPressKitTests/RemoteTestCase.swift +++ b/WordPressKitTests/RemoteTestCase.swift @@ -185,4 +185,34 @@ extension RemoteTestCase { print("Unable to clear cache: \(error)") } } + + /// Checks if the specified set of query parameter names are all present in a given `URLRequest`. + /// This method verifies the presence of query parameter names in the request's URL without evaluating their values. + /// + /// - Parameters: + /// - queryParams: A set of query parameter names to check for in the request. + /// - request: The `URLRequest` to inspect for the presence of query parameter names. + /// - Returns: A Boolean value indicating whether all specified query parameter names are present in the request's URL. + func queryParams(_ queryParams: Set, containedInRequest request: URLRequest) -> Bool { + guard let url = request.url else { + return false + } + return queryParamsContained(queryParams, containedInURL: url) + } + + /// Checks if the specified set of query parameter names are all present in a given `URL`. + /// This method verifies the presence of query parameter names in the URL's query string without evaluating their values. + /// + /// - Parameters: + /// - queryParams: A set of query parameter names to check for in the URL. + /// - url: The `URL` to inspect for the presence of query parameter names. + /// - Returns: A Boolean value indicating whether all specified query parameter names are present in the URL's query string. + func queryParamsContained(_ queryParams: Set, containedInURL url: URL) -> Bool { + guard let components = URLComponents(url: url, resolvingAgainstBaseURL: true), + let queryItems = components.queryItems?.map({ $0.name }) + else { + return false + } + return queryParams.intersection(queryItems) == queryParams + } } diff --git a/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift b/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift index 5ec33ebc..c84197e7 100644 --- a/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift +++ b/WordPressKitTests/Utilities/FeatureFlagRemoteTests.swift @@ -10,16 +10,16 @@ class FeatureFlagRemoteTests: RemoteTestCase, RESTTestable { let expectation = expectation(description: "Get Remote Feature Flags Endpoint should contain query params") let response = try makeResponse() - let expectedQueryParams = [ - "identifier": "com.apple.dt.xctest.tool", - "platform": "ios", - "build_number": "22516", - "marketing_version": "15.1", - "device_id": "Test" + let expectedQueryParams: Set = [ + "identifier", + "platform", + "build_number", + "marketing_version", + "device_id" ] stub { req -> Bool in - let containsQueryParams = containsQueryParams(expectedQueryParams)(req) + let containsQueryParams = self.queryParams(expectedQueryParams, containedInRequest: req) let matchesPath = isPath(self.endpoint)(req) let matchesURL = containsQueryParams && matchesPath XCTAssertTrue(matchesURL) From 8fcb113f3a68591cb5cd29e9a980c3a5fcaebeaa Mon Sep 17 00:00:00 2001 From: Salim Date: Fri, 5 Jan 2024 16:23:54 +0100 Subject: [PATCH 08/14] Update WordPressKit/FeatureFlagRemote.swift Co-authored-by: hassaanelgarem --- WordPressKit/FeatureFlagRemote.swift | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index f2e0509c..87f0148f 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -9,10 +9,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } open func getRemoteFeatureFlags(forDeviceId deviceId: String, callback: @escaping FeatureFlagResponseCallback) { - self.getRemoteFeatureFlags(params: .init(deviceId: deviceId), callback: callback) - } - - open func getRemoteFeatureFlags(params: RemoteFeatureFlagsEndpointParams, callback: @escaping FeatureFlagResponseCallback) { + let params = RemoteFeatureFlagsEndpointParams(deviceId: deviceId) let endpoint = "mobile/feature-flags" let path = self.path(forEndpoint: endpoint, withVersion: ._2_0) var parameters: [String: AnyObject]? From 90bbe5e71ebe3873eff3353cc11715f4662ea44d Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 16:32:23 +0100 Subject: [PATCH 09/14] Move feature flag params encoding to helper method --- WordPressKit/FeatureFlagRemote.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index f2e0509c..e4e37af8 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -86,4 +86,10 @@ extension FeatureFlagRemote.RemoteFeatureFlagsEndpointParams: Encodable { self.marketingVersion = bundle.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown" self.identifier = bundle.bundleIdentifier ?? "Unknown" } + + func dictionaryRepresentation() throws -> [String: AnyObject]? { + let encoder = JSONEncoder() + let data = try encoder.encode(self) + return try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] + } } From d13dbe8d21ff3010efdca9e15d2bc6bd611f3f5e Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 16:36:38 +0100 Subject: [PATCH 10/14] Rename featue flag endpoint params type --- WordPressKit/DashboardServiceRemote.swift | 2 +- WordPressKit/FeatureFlagRemote.swift | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/WordPressKit/DashboardServiceRemote.swift b/WordPressKit/DashboardServiceRemote.swift index 8ca76660..84fa0b6d 100644 --- a/WordPressKit/DashboardServiceRemote.swift +++ b/WordPressKit/DashboardServiceRemote.swift @@ -37,7 +37,7 @@ open class DashboardServiceRemote: ServiceRemoteWordPressComREST { "cards": cards.joined(separator: ",") as NSString ] let featureFlagParams = try { - let params = FeatureFlagRemote.RemoteFeatureFlagsEndpointParams(deviceId: deviceId) + let params = FeatureFlagRemote.FetchAllEndpointParams(deviceId: deviceId) let encoder = JSONEncoder() let data = try encoder.encode(params) return try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index ff3c8111..4180f782 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -9,7 +9,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } open func getRemoteFeatureFlags(forDeviceId deviceId: String, callback: @escaping FeatureFlagResponseCallback) { - let params = RemoteFeatureFlagsEndpointParams(deviceId: deviceId) + let params = FetchAllEndpointParams(deviceId: deviceId) let endpoint = "mobile/feature-flags" let path = self.path(forEndpoint: endpoint, withVersion: ._2_0) var dictionary: [String: AnyObject]? @@ -55,7 +55,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { }) } - public struct RemoteFeatureFlagsEndpointParams { + public struct FetchAllEndpointParams { let deviceId: String let platform: String let buildNumber: String @@ -64,7 +64,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } } -extension FeatureFlagRemote.RemoteFeatureFlagsEndpointParams: Encodable { +extension FeatureFlagRemote.FetchAllEndpointParams: Encodable { enum CodingKeys: String, CodingKey { case deviceId = "device_id" From ca469acbdef04d14063021075af36a7628b77a37 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 16:40:27 +0100 Subject: [PATCH 11/14] Use dictionaryRepresentation method in more locations --- WordPressKit/DashboardServiceRemote.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/WordPressKit/DashboardServiceRemote.swift b/WordPressKit/DashboardServiceRemote.swift index 84fa0b6d..86fda16f 100644 --- a/WordPressKit/DashboardServiceRemote.swift +++ b/WordPressKit/DashboardServiceRemote.swift @@ -36,12 +36,7 @@ open class DashboardServiceRemote: ServiceRemoteWordPressComREST { let cardsParams: [String: AnyObject] = [ "cards": cards.joined(separator: ",") as NSString ] - let featureFlagParams = try { - let params = FeatureFlagRemote.FetchAllEndpointParams(deviceId: deviceId) - let encoder = JSONEncoder() - let data = try encoder.encode(params) - return try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] - }() + let featureFlagParams = try FeatureFlagRemote.FetchAllEndpointParams(deviceId: deviceId).dictionaryRepresentation() return cardsParams.merging(featureFlagParams ?? [:]) { first, second in return first } From 28cc006f19a92ddb7e0b307926cedd1c9449bca4 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 17:46:54 +0100 Subject: [PATCH 12/14] Move Feature Flag Params to global scope --- WordPressKit.xcodeproj/project.pbxproj | 4 +++ WordPressKit/DashboardServiceRemote.swift | 2 +- WordPressKit/FeatureFlagRemote.swift | 35 +------------------ WordPressKit/SessionDetails.swift | 33 +++++++++++++++++ .../DashboardServiceRemoteTests.swift | 2 +- 5 files changed, 40 insertions(+), 36 deletions(-) create mode 100644 WordPressKit/SessionDetails.swift diff --git a/WordPressKit.xcodeproj/project.pbxproj b/WordPressKit.xcodeproj/project.pbxproj index 26028053..8571142e 100644 --- a/WordPressKit.xcodeproj/project.pbxproj +++ b/WordPressKit.xcodeproj/project.pbxproj @@ -612,6 +612,7 @@ F3FF8A25279C960F00E5C90F /* site-email-followers-get-auth-failure.json in Resources */ = {isa = PBXBuildFile; fileRef = F3FF8A24279C960F00E5C90F /* site-email-followers-get-auth-failure.json */; }; F3FF8A27279C967200E5C90F /* site-email-followers-get-failure.json in Resources */ = {isa = PBXBuildFile; fileRef = F3FF8A26279C967200E5C90F /* site-email-followers-get-failure.json */; }; F3FF8A29279C991B00E5C90F /* site-email-followers-get-success-more-pages.json in Resources */ = {isa = PBXBuildFile; fileRef = F3FF8A28279C991B00E5C90F /* site-email-followers-get-success-more-pages.json */; }; + F41D98EA2B48602B004EC050 /* SessionDetails.swift in Sources */ = {isa = PBXBuildFile; fileRef = F41D98E92B48602B004EC050 /* SessionDetails.swift */; }; F4B0F4732ACAF498003ABC61 /* DomainsServiceRemote+AllDomains.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4B0F4722ACAF498003ABC61 /* DomainsServiceRemote+AllDomains.swift */; }; F4B0F47C2ACB4B74003ABC61 /* get-all-domains-response.json in Resources */ = {isa = PBXBuildFile; fileRef = F4B0F47B2ACB4B74003ABC61 /* get-all-domains-response.json */; }; F4B0F4802ACB4EA9003ABC61 /* AllDomainsResultDomainTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4B0F47F2ACB4EA9003ABC61 /* AllDomainsResultDomainTests.swift */; }; @@ -1320,6 +1321,7 @@ F3FF8A24279C960F00E5C90F /* site-email-followers-get-auth-failure.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "site-email-followers-get-auth-failure.json"; sourceTree = ""; }; F3FF8A26279C967200E5C90F /* site-email-followers-get-failure.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "site-email-followers-get-failure.json"; sourceTree = ""; }; F3FF8A28279C991B00E5C90F /* site-email-followers-get-success-more-pages.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "site-email-followers-get-success-more-pages.json"; sourceTree = ""; }; + F41D98E92B48602B004EC050 /* SessionDetails.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionDetails.swift; sourceTree = ""; }; F4B0F4722ACAF498003ABC61 /* DomainsServiceRemote+AllDomains.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DomainsServiceRemote+AllDomains.swift"; sourceTree = ""; }; F4B0F47B2ACB4B74003ABC61 /* get-all-domains-response.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "get-all-domains-response.json"; sourceTree = ""; }; F4B0F47F2ACB4EA9003ABC61 /* AllDomainsResultDomainTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AllDomainsResultDomainTests.swift; sourceTree = ""; }; @@ -2099,6 +2101,7 @@ FEF7419C28085D89002C4203 /* RemoteBloggingPrompt.swift */, FE20A6A3282A96C00025E975 /* RemoteBloggingPromptsSettings.swift */, 1DAC3D2529AF4F250068FE13 /* RemoteVideoPressVideo.swift */, + F41D98E92B48602B004EC050 /* SessionDetails.swift */, ); name = Models; sourceTree = ""; @@ -3381,6 +3384,7 @@ 82FFBF501F45EFD100F4573F /* RemoteBlogJetpackSettings.swift in Sources */, 74650F741F0EA1E200188EDB /* RemoteGravatarProfile.swift in Sources */, 40E7FEB4221063480032834E /* StatsTodayInsight.swift in Sources */, + F41D98EA2B48602B004EC050 /* SessionDetails.swift in Sources */, 436D563C2118E18D00CEAA33 /* WPState.swift in Sources */, 439A44DA2107C93000795ED7 /* RemotePlan_ApiVersion1_3.swift in Sources */, 93BD27811EE73944002BB00B /* WordPressOrgXMLRPCApi.swift in Sources */, diff --git a/WordPressKit/DashboardServiceRemote.swift b/WordPressKit/DashboardServiceRemote.swift index 86fda16f..1ef61006 100644 --- a/WordPressKit/DashboardServiceRemote.swift +++ b/WordPressKit/DashboardServiceRemote.swift @@ -36,7 +36,7 @@ open class DashboardServiceRemote: ServiceRemoteWordPressComREST { let cardsParams: [String: AnyObject] = [ "cards": cards.joined(separator: ",") as NSString ] - let featureFlagParams = try FeatureFlagRemote.FetchAllEndpointParams(deviceId: deviceId).dictionaryRepresentation() + let featureFlagParams = try SessionDetails(deviceId: deviceId).dictionaryRepresentation() return cardsParams.merging(featureFlagParams ?? [:]) { first, second in return first } diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index 4180f782..3a7309b2 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -9,7 +9,7 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } open func getRemoteFeatureFlags(forDeviceId deviceId: String, callback: @escaping FeatureFlagResponseCallback) { - let params = FetchAllEndpointParams(deviceId: deviceId) + let params = SessionDetails(deviceId: deviceId) let endpoint = "mobile/feature-flags" let path = self.path(forEndpoint: endpoint, withVersion: ._2_0) var dictionary: [String: AnyObject]? @@ -54,37 +54,4 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { callback(.failure(error)) }) } - - public struct FetchAllEndpointParams { - let deviceId: String - let platform: String - let buildNumber: String - let marketingVersion: String - let identifier: String - } -} - -extension FeatureFlagRemote.FetchAllEndpointParams: Encodable { - - enum CodingKeys: String, CodingKey { - case deviceId = "device_id" - case platform = "platform" - case buildNumber = "build_number" - case marketingVersion = "marketing_version" - case identifier = "identifier" - } - - init(deviceId: String, bundle: Bundle = .main) { - self.deviceId = deviceId - self.platform = "ios" - self.buildNumber = bundle.infoDictionary?["CFBundleVersion"] as? String ?? "Unknown" - self.marketingVersion = bundle.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown" - self.identifier = bundle.bundleIdentifier ?? "Unknown" - } - - func dictionaryRepresentation() throws -> [String: AnyObject]? { - let encoder = JSONEncoder() - let data = try encoder.encode(self) - return try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] - } } diff --git a/WordPressKit/SessionDetails.swift b/WordPressKit/SessionDetails.swift new file mode 100644 index 00000000..976d4de1 --- /dev/null +++ b/WordPressKit/SessionDetails.swift @@ -0,0 +1,33 @@ +public struct SessionDetails { + + let deviceId: String + let platform: String + let buildNumber: String + let marketingVersion: String + let identifier: String +} + +extension SessionDetails: Encodable { + + enum CodingKeys: String, CodingKey { + case deviceId = "device_id" + case platform = "platform" + case buildNumber = "build_number" + case marketingVersion = "marketing_version" + case identifier = "identifier" + } + + init(deviceId: String, bundle: Bundle = .main) { + self.deviceId = deviceId + self.platform = "ios" + self.buildNumber = bundle.infoDictionary?["CFBundleVersion"] as? String ?? "Unknown" + self.marketingVersion = bundle.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown" + self.identifier = bundle.bundleIdentifier ?? "Unknown" + } + + func dictionaryRepresentation() throws -> [String: AnyObject]? { + let encoder = JSONEncoder() + let data = try encoder.encode(self) + return try JSONSerialization.jsonObject(with: data) as? [String: AnyObject] + } +} diff --git a/WordPressKitTests/DashboardServiceRemoteTests.swift b/WordPressKitTests/DashboardServiceRemoteTests.swift index 86d62c61..a0b13dcd 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -23,7 +23,7 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { "marketing_version", "device_id", "cards", - "locale" + "locale", ] stubRemoteResponse({ req in From 88918817bb32c0321034a9bcdd47bcd92da2e2c3 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 17:47:50 +0100 Subject: [PATCH 13/14] Fix lint warning --- WordPressKit/SessionDetails.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/WordPressKit/SessionDetails.swift b/WordPressKit/SessionDetails.swift index 976d4de1..46f4df45 100644 --- a/WordPressKit/SessionDetails.swift +++ b/WordPressKit/SessionDetails.swift @@ -1,5 +1,4 @@ public struct SessionDetails { - let deviceId: String let platform: String let buildNumber: String From 432a3369081591db6ea13eb06397586ae803eeb6 Mon Sep 17 00:00:00 2001 From: Salim Braksa Date: Fri, 5 Jan 2024 17:56:17 +0100 Subject: [PATCH 14/14] Fix minor typo --- WordPressKitTests/DashboardServiceRemoteTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressKitTests/DashboardServiceRemoteTests.swift b/WordPressKitTests/DashboardServiceRemoteTests.swift index a0b13dcd..86d62c61 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -23,7 +23,7 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { "marketing_version", "device_id", "cards", - "locale", + "locale" ] stubRemoteResponse({ req in