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 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 f002f3e7..1ef61006 100644 --- a/WordPressKit/DashboardServiceRemote.swift +++ b/WordPressKit/DashboardServiceRemote.swift @@ -1,13 +1,24 @@ 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 + 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]? + + do { + params = try self.makeQueryParams(cards: cards, deviceId: deviceId) + } catch { + failure(error) } wordPressComRestApi.GET(requestUrl, - parameters: nil, + parameters: params, success: { response, _ in guard let cards = response as? NSDictionary else { failure(ResponseError.decodingFailure) @@ -21,17 +32,14 @@ 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 + private func makeQueryParams(cards: [String], deviceId: String) throws -> [String: AnyObject] { + let cardsParams: [String: AnyObject] = [ + "cards": cards.joined(separator: ",") as NSString + ] + let featureFlagParams = try SessionDetails(deviceId: deviceId).dictionaryRepresentation() + return cardsParams.merging(featureFlagParams ?? [:]) { first, second in + return first } - - return self.path(forEndpoint: endpoint, withVersion: ._2_0) } enum ResponseError: Error { diff --git a/WordPressKit/FeatureFlagRemote.swift b/WordPressKit/FeatureFlagRemote.swift index d8bce6c6..3a7309b2 100644 --- a/WordPressKit/FeatureFlagRemote.swift +++ b/WordPressKit/FeatureFlagRemote.swift @@ -9,20 +9,20 @@ open class FeatureFlagRemote: ServiceRemoteWordPressComREST { } open func getRemoteFeatureFlags(forDeviceId deviceId: String, callback: @escaping FeatureFlagResponseCallback) { - + let params = SessionDetails(deviceId: deviceId) let endpoint = "mobile/feature-flags" let path = self.path(forEndpoint: endpoint, withVersion: ._2_0) + var dictionary: [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 { + dictionary = try params.dictionaryRepresentation() + } catch let error { + callback(.failure(error)) + return + } wordPressComRestApi.GET(path, - parameters: parameters, + parameters: dictionary, success: { response, _ in if let featureFlagList = response as? NSDictionary { diff --git a/WordPressKit/SessionDetails.swift b/WordPressKit/SessionDetails.swift new file mode 100644 index 00000000..46f4df45 --- /dev/null +++ b/WordPressKit/SessionDetails.swift @@ -0,0 +1,32 @@ +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 3424af32..86d62c61 100644 --- a/WordPressKitTests/DashboardServiceRemoteTests.swift +++ b/WordPressKitTests/DashboardServiceRemoteTests.swift @@ -1,4 +1,5 @@ import XCTest +import OHHTTPStubs @testable import WordPressKit @@ -13,12 +14,37 @@ class DashboardServiceRemoteTests: RemoteTestCase, RESTTestable { // Requests the correct set of cards // 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) - - dashboardServiceRemote.fetch(cards: ["posts", "todays_stats"], forBlogID: 165243437) { _ in + 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 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( + cards: ["posts", "todays_stats"], + forBlogID: 165243437, + deviceId: "Test" + ) { _ in expect.fulfill() - } failure: { _ in } + } failure: { error in + XCTFail("Dashboard cards request failed: \(error.localizedDescription)") + expect.fulfill() + } waitForExpectations(timeout: timeout, handler: nil) } @@ -27,9 +53,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() @@ -44,7 +79,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() @@ -59,7 +98,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() @@ -74,7 +117,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() diff --git a/WordPressKitTests/RemoteTestCase.swift b/WordPressKitTests/RemoteTestCase.swift index d3ab060b..52b63d20 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: @@ -160,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 8e6b1e08..c84197e7 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: Set = [ + "identifier", + "platform", + "build_number", + "marketing_version", + "device_id" + ] + + stub { req -> Bool in + let containsQueryParams = self.queryParams(expectedQueryParams, containedInRequest: 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: [:]) + }()) + } }