From 0b17477b4b31f243a7ad9abaa6d44150edd3e2aa Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 15 Feb 2024 12:30:44 +1300 Subject: [PATCH 01/16] Update WordPressKit --- Podfile | 2 +- Podfile.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Podfile b/Podfile index 9efaa9422828..94d533be57b6 100644 --- a/Podfile +++ b/Podfile @@ -51,7 +51,7 @@ end def wordpress_kit # pod 'WordPressKit', '~> 13.0' - pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: '92c1536724af8bb57ee677996919c2a9e37434e8' + pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: 'dfb2134ca3b91233d44b3551d91844bf2087996f' # pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', branch: '' # pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', tag: '' # pod 'WordPressKit', path: '../WordPressKit-iOS' diff --git a/Podfile.lock b/Podfile.lock index 4ff3f02d6122..bee2985d117f 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -120,7 +120,7 @@ DEPENDENCIES: - SwiftLint (~> 0.50) - WordPress-Editor-iOS (~> 1.19.9) - WordPressAuthenticator (from `https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git`, commit `fa06fca7178b268d382d91861752b3be0729e8a8`) - - WordPressKit (from `https://github.com/wordpress-mobile/WordPressKit-iOS.git`, commit `92c1536724af8bb57ee677996919c2a9e37434e8`) + - WordPressKit (from `https://github.com/wordpress-mobile/WordPressKit-iOS.git`, commit `dfb2134ca3b91233d44b3551d91844bf2087996f`) - WordPressShared (~> 2.3) - WordPressUI (~> 1.15) - ZendeskSupportSDK (= 5.3.0) @@ -178,7 +178,7 @@ EXTERNAL SOURCES: :commit: fa06fca7178b268d382d91861752b3be0729e8a8 :git: https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git WordPressKit: - :commit: 92c1536724af8bb57ee677996919c2a9e37434e8 + :commit: dfb2134ca3b91233d44b3551d91844bf2087996f :git: https://github.com/wordpress-mobile/WordPressKit-iOS.git CHECKOUT OPTIONS: @@ -189,7 +189,7 @@ CHECKOUT OPTIONS: :commit: fa06fca7178b268d382d91861752b3be0729e8a8 :git: https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git WordPressKit: - :commit: 92c1536724af8bb57ee677996919c2a9e37434e8 + :commit: dfb2134ca3b91233d44b3551d91844bf2087996f :git: https://github.com/wordpress-mobile/WordPressKit-iOS.git SPEC CHECKSUMS: @@ -236,6 +236,6 @@ SPEC CHECKSUMS: ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba ZIPFoundation: d170fa8e270b2a32bef9dcdcabff5b8f1a5deced -PODFILE CHECKSUM: d7f320bedde48770f2ca24d6086bc5214172ddec +PODFILE CHECKSUM: e000a2048651eeea961e5c16e130a60d9c7cd1b9 COCOAPODS: 1.14.2 From 7d4a72ef9e02678906009e26d50c4b8ff1af457e Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 15 Feb 2024 12:30:27 +1300 Subject: [PATCH 02/16] Adopt to the breaking changes in WordPressOrgRestApi --- WordPress/Classes/Models/Blog.m | 2 +- .../WordPressOrgRestApi+WordPress.swift | 30 +++++++--- .../Services/BlockEditorSettingsService.swift | 15 ++--- .../BloggingPromptsService.swift | 21 +++---- .../Gutenberg/GutenbergNetworking.swift | 41 +++++++------ .../JetpackNativeConnectionService.swift | 57 +++++++++---------- 6 files changed, 82 insertions(+), 84 deletions(-) diff --git a/WordPress/Classes/Models/Blog.m b/WordPress/Classes/Models/Blog.m index a78d7c196428..c458b73e994e 100644 --- a/WordPress/Classes/Models/Blog.m +++ b/WordPress/Classes/Models/Blog.m @@ -889,7 +889,7 @@ - (WordPressOrgXMLRPCApi *)xmlrpcApi - (WordPressOrgRestApi *)wordPressOrgRestApi { if (_wordPressOrgRestApi == nil) { - _wordPressOrgRestApi = [[WordPressOrgRestApi alloc] initWithBlog:self]; + _wordPressOrgRestApi = self.account == nil ? [[WordPressOrgRestApi alloc] initWithBlog:self] : nil; } return _wordPressOrgRestApi; } diff --git a/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift b/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift index 3fe174b723d2..70e911f78dbe 100644 --- a/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift +++ b/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift @@ -37,15 +37,29 @@ private func apiBase(blog: Blog) -> URL? { } extension WordPressOrgRestApi { - @objc public convenience init?(blog: Blog) { - guard let apiBase = apiBase(blog: blog), - let authenticator = makeAuthenticator(blog: blog) else { + @objc + convenience init?(blog: Blog) { + if let dotComID = blog.dotComID?.uint64Value, + let token = blog.account?.authToken, + token.count > 0 { + self.init(dotComSiteID: dotComID, bearerToken: token, userAgent: WPUserAgent.wordPress()) + } else if let apiBase = apiBase(blog: blog), + let loginURL = try? blog.loginUrl().asURL(), + let adminURL = try? blog.adminUrl(withPath: "").asURL(), + let username = blog.username, + let password = blog.password { + self.init( + selfHostedSiteWPJSONURL: apiBase, + credential: .init( + loginURL: loginURL, + username: username, + password: password, + adminURL: adminURL + ), + userAgent: WPUserAgent.wordPress() + ) + } else { return nil } - self.init( - apiBase: apiBase, - authenticator: authenticator, - userAgent: WPUserAgent.wordPress() - ) } } diff --git a/WordPress/Classes/Services/BlockEditorSettingsService.swift b/WordPress/Classes/Services/BlockEditorSettingsService.swift index 455e3f7d4844..d5a2d962a167 100644 --- a/WordPress/Classes/Services/BlockEditorSettingsService.swift +++ b/WordPress/Classes/Services/BlockEditorSettingsService.swift @@ -22,14 +22,7 @@ class BlockEditorSettingsService { } convenience init?(blog: Blog, coreDataStack: CoreDataStackSwift) { - let remoteAPI: WordPressRestApi - if blog.isAccessibleThroughWPCom(), - blog.dotComID?.intValue != nil, - let restAPI = blog.wordPressComRestApi() { - remoteAPI = restAPI - } else if let orgAPI = blog.wordPressOrgRestApi { - remoteAPI = orgAPI - } else { + guard let remoteAPI = WordPressOrgRestApi(blog: blog) else { // This is should only happen if there is a problem with the blog itsself. return nil } @@ -37,7 +30,7 @@ class BlockEditorSettingsService { self.init(blog: blog, remoteAPI: remoteAPI, coreDataStack: coreDataStack) } - init(blog: Blog, remoteAPI: WordPressRestApi, coreDataStack: CoreDataStackSwift) { + init(blog: Blog, remoteAPI: WordPressOrgRestApi, coreDataStack: CoreDataStackSwift) { assert(blog.objectID.persistentStore != nil, "The blog instance should be saved first") self.blog = blog self.coreDataStack = coreDataStack @@ -65,7 +58,7 @@ class BlockEditorSettingsService { // MARK: Editor `theme_supports` support private extension BlockEditorSettingsService { func fetchTheme(_ completion: @escaping BlockEditorSettingsServiceCompletion) { - remote.fetchTheme(forSiteID: blog.dotComID?.intValue) { [weak self] (response) in + remote.fetchTheme { [weak self] (response) in guard let `self` = self else { return } switch response { case .success(let editorTheme): @@ -127,7 +120,7 @@ private extension BlockEditorSettingsService { // MARK: Editor Global Styles support private extension BlockEditorSettingsService { func fetchBlockEditorSettings(_ completion: @escaping BlockEditorSettingsServiceCompletion) { - remote.fetchBlockEditorSettings(forSiteID: blog.dotComID?.intValue) { [weak self] (response) in + remote.fetchBlockEditorSettings { [weak self] (response) in guard let `self` = self else { return } switch response { case .success(let remoteSettings): diff --git a/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift b/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift index 09f626e14a3b..ecdc03280b63 100644 --- a/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift +++ b/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift @@ -323,20 +323,17 @@ private extension BloggingPromptsService { return params }() - api.GET(path, parameters: requestParameter as [String: AnyObject]) { result, _ in - switch result { - case .success(let responseObject): - do { - let data = try JSONSerialization.data(withJSONObject: responseObject, options: []) - let remotePrompts = try Self.jsonDecoder.decode([BloggingPromptRemoteObject].self, from: data) - completion(.success(remotePrompts)) - } catch { - completion(.failure(error)) - } - case .failure(let error): + api.GET(path, parameters: requestParameter as [String: AnyObject], success: { (responseObject, _) in + do { + let data = try JSONSerialization.data(withJSONObject: responseObject, options: []) + let remotePrompts = try Self.jsonDecoder.decode([BloggingPromptRemoteObject].self, from: data) + completion(.success(remotePrompts)) + } catch { completion(.failure(error)) } - } + }, failure: { (error, _) in + completion(.failure(error)) + }) } /// Loads local prompts based on the given parameters. diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift index 21dcb8140470..e59f1ae38f24 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift @@ -70,37 +70,36 @@ struct GutenbergNetworkRequest { return } + let task: Task, Never> + switch method { case .get: - api.GET(path, parameters: nil) { (result, httpResponse) in - switch result { - case .success(let response): - completion(.success(response)) - case .failure(let error): - if handleEmbedError(path: path, error: error, completion: completion) { - return - } - completion(.failure(error as NSError)) - } + task = Task { + await api.get(path: path) } case .post: - api.POST(path, parameters: data) { (result, httpResponse) in - switch result { - case .success(let response): - completion(.success(response)) - case .failure(let error): - if handleEmbedError(path: path, error: error, completion: completion) { - return - } - completion(.failure(error as NSError)) + task = Task { + await api.post(path: path, parameters: data ?? [:]) + } + } + + Task { @MainActor in + let result = await task.value + switch result { + case .success(let response): + completion(.success(response)) + case .failure(let error): + if handleEmbedError(path: path, error: error, completion: completion) { + return } + completion(.failure(error as NSError)) } } } - private func handleEmbedError(path: String, error: Error, completion: @escaping CompletionHandler) -> Bool { + private func handleEmbedError(path: String, error: WordPressAPIError, completion: @escaping CompletionHandler) -> Bool { if path.starts(with: "/oembed/1.0/") { - if let error = error as? AFError, error.responseCode == 404 { + if case let .unacceptableStatusCode(response: response, body: _) = error, response.statusCode == 404 { completion(.failure(URLError(URLError.Code(rawValue: 404)) as NSError)) return true } diff --git a/WordPress/Classes/ViewRelated/Jetpack/Install/JetpackNativeConnectionService.swift b/WordPress/Classes/ViewRelated/Jetpack/Install/JetpackNativeConnectionService.swift index d78d6bc5a2fd..cbdb16df14be 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Install/JetpackNativeConnectionService.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Install/JetpackNativeConnectionService.swift @@ -40,26 +40,25 @@ final class JetpackNativeConnectionService: NSObject { /// - Parameter completion: Result with either Jetpack connection URL or JetpackNativeConnectionURLError /// func fetchJetpackConnectionURL(completion: @escaping (Result) -> ()) { - api.request(method: .get, path: Path.getConnectionURL, parameters: [:], completion: { result, response in - switch result { - case .success(let data): - if let urlString = data as? String, - let url = URL(string: urlString), - urlString.hasPrefix(Constants.jetpackAccountConnectionURL) { - completion(.success(url)) - } else { - /// If the site didn't implement the site-level connection, the URL would be at the form: https://{site_url}/wp-admin/admin.php?page=jetpack&action=register&_wpnonce={nonce} - /// In this case, we need to take cookies from current response, call the returned URL, - /// and get the connection URL through redirection (See https://github.com/woocommerce/woocommerce-android/issues/7525) + Task { @MainActor in + let result = await self.api.get(path: Path.getConnectionURL, type: String.self) + .mapError { JetpackNativeConnectionURLError.remote($0.localizedDescription) } + .flatMap { urlString in + if let url = URL(string: urlString), + urlString.hasPrefix(Constants.jetpackAccountConnectionURL) { + return .success(url) + } else { + /// If the site didn't implement the site-level connection, the URL would be at the form: https://{site_url}/wp-admin/admin.php?page=jetpack&action=register&_wpnonce={nonce} + /// In this case, we need to take cookies from current response, call the returned URL, + /// and get the connection URL through redirection (See https://github.com/woocommerce/woocommerce-android/issues/7525) - /// When site-level connection is not implemented, JetpackConnectionWebViewController - /// does not use JetpackNativeConnectionService so this case is ignored for now - completion(.failure(.jetpackSiteNotRegistered)) + /// When site-level connection is not implemented, JetpackConnectionWebViewController + /// does not use JetpackNativeConnectionService so this case is ignored for now + return .failure(.jetpackSiteNotRegistered) + } } - case .failure(let error): - completion(.failure(.remote(error.localizedDescription))) - } - }) + completion(result) + } } /// Fetches Jetpack User that contains Jetpack plugin connection information using Jetpack REST API @@ -68,19 +67,15 @@ final class JetpackNativeConnectionService: NSObject { /// - Parameter completion: Result with either JetpackUser or JetpackNativeConnectionDataError /// func fetchJetpackUser(completion: @escaping (Result) -> ()) { - api.request(method: .get, path: Path.getJetpackUserData, parameters: [:], completion: { result, response in - switch result { - case .success(let json): - do { - let data = try JSONSerialization.data(withJSONObject: json) - let jetpackUserData = try JSONDecoder().decode(JetpackUserData.self, from: data) - completion(.success(jetpackUserData.currentUser)) - } catch { - completion(.failure(.parsingError)) + Task { @MainActor in + let result = await self.api.get(path: Path.getJetpackUserData, type: JetpackUserData.self) + .map { $0.currentUser } + .flatMapError { original in + if case let .unparsableResponse(response, _, underlyingError) = original, response?.statusCode == 200, underlyingError is DecodingError { + return .failure(JetpackNativeConnectionDataError.parsingError) + } + return .failure(.remote(original.localizedDescription)) } - case .failure(let error): - completion(.failure(.remote(error.localizedDescription))) - } - }) + } } } From aebe3f9d930211fde341d990ce244dd01cefd50f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 15 Feb 2024 12:44:23 +1300 Subject: [PATCH 03/16] Rename `Blog.wordPressOrgRestApi` to `selfHostedSiteRestApi` --- WordPress/Classes/Models/Blog.h | 2 +- WordPress/Classes/Models/Blog.m | 18 +++++++++--------- WordPress/Classes/Stores/PluginStore.swift | 2 +- .../Gutenberg/GutenbergNetworking.swift | 2 +- .../JetpackConnectionWebViewController.swift | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/WordPress/Classes/Models/Blog.h b/WordPress/Classes/Models/Blog.h index f651d93b67bc..4022651da0a8 100644 --- a/WordPress/Classes/Models/Blog.h +++ b/WordPress/Classes/Models/Blog.h @@ -208,7 +208,7 @@ typedef NS_ENUM(NSInteger, SiteVisibility) { @property (nonatomic, weak, readonly, nullable) NSArray *sortedConnections; @property (nonatomic, readonly, nullable) NSArray *sortedRoles; @property (nonatomic, strong, readonly, nullable) WordPressOrgXMLRPCApi *xmlrpcApi; -@property (nonatomic, strong, readonly, nullable) WordPressOrgRestApi *wordPressOrgRestApi; +@property (nonatomic, strong, readonly, nullable) WordPressOrgRestApi *selfHostedSiteRestApi; @property (nonatomic, weak, readonly, nullable) NSString *version; @property (nonatomic, strong, readonly, nullable) NSString *authToken; @property (nonatomic, strong, readonly, nullable) NSSet *allowedFileTypes; diff --git a/WordPress/Classes/Models/Blog.m b/WordPress/Classes/Models/Blog.m index c458b73e994e..cb31df21949e 100644 --- a/WordPress/Classes/Models/Blog.m +++ b/WordPress/Classes/Models/Blog.m @@ -34,7 +34,7 @@ @interface Blog () @property (nonatomic, strong, readwrite) WordPressOrgXMLRPCApi *xmlrpcApi; -@property (nonatomic, strong, readwrite) WordPressOrgRestApi *wordPressOrgRestApi; +@property (nonatomic, strong, readwrite) WordPressOrgRestApi *selfHostedSiteRestApi; @end @@ -99,7 +99,7 @@ @implementation Blog @synthesize videoPressEnabled; @synthesize isSyncingMedia; @synthesize xmlrpcApi = _xmlrpcApi; -@synthesize wordPressOrgRestApi = _wordPressOrgRestApi; +@synthesize selfHostedSiteRestApi = _selfHostedSiteRestApi; #pragma mark - NSManagedObject subclass methods @@ -113,7 +113,7 @@ - (void)prepareForDeletion } [_xmlrpcApi invalidateAndCancelTasks]; - [_wordPressOrgRestApi invalidateAndCancelTasks]; + [_selfHostedSiteRestApi invalidateAndCancelTasks]; } - (void)didTurnIntoFault @@ -122,7 +122,7 @@ - (void)didTurnIntoFault // Clean up instance variables self.xmlrpcApi = nil; - self.wordPressOrgRestApi = nil; + self.selfHostedSiteRestApi = nil; [[NSNotificationCenter defaultCenter] removeObserver:self]; } @@ -701,7 +701,7 @@ - (BOOL)supportsPluginManagement // Reference: https://make.wordpress.org/core/2020/07/16/new-and-modified-rest-api-endpoints-in-wordpress-5-5/ if(!supports && !self.account){ supports = !self.isHostedAtWPcom - && self.wordPressOrgRestApi + && self.selfHostedSiteRestApi && [self hasRequiredWordPressVersion:@"5.5"]; } @@ -886,12 +886,12 @@ - (WordPressOrgXMLRPCApi *)xmlrpcApi return _xmlrpcApi; } -- (WordPressOrgRestApi *)wordPressOrgRestApi +- (WordPressOrgRestApi *)selfHostedSiteRestApi { - if (_wordPressOrgRestApi == nil) { - _wordPressOrgRestApi = self.account == nil ? [[WordPressOrgRestApi alloc] initWithBlog:self] : nil; + if (_selfHostedSiteRestApi == nil) { + _selfHostedSiteRestApi = self.account == nil ? [[WordPressOrgRestApi alloc] initWithBlog:self] : nil; } - return _wordPressOrgRestApi; + return _selfHostedSiteRestApi; } - (WordPressComRestApi *)wordPressComRestApi diff --git a/WordPress/Classes/Stores/PluginStore.swift b/WordPress/Classes/Stores/PluginStore.swift index ec35bf46750b..d8c909bedd5b 100644 --- a/WordPress/Classes/Stores/PluginStore.swift +++ b/WordPress/Classes/Stores/PluginStore.swift @@ -803,7 +803,7 @@ private extension PluginStore { } private func selfHostedRemoteClient(site: JetpackSiteRef) -> PluginManagementClient? { - guard let remote = BlogService.blog(with: site)?.wordPressOrgRestApi else { + guard let remote = BlogService.blog(with: site)?.selfHostedSiteRestApi else { return nil } diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift index e59f1ae38f24..be537c55681e 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift @@ -65,7 +65,7 @@ struct GutenbergNetworkRequest { } private func performSelfHostedRequest(completion: @escaping CompletionHandler) { - guard let api = blog.wordPressOrgRestApi else { + guard let api = blog.selfHostedSiteRestApi else { completion(.failure(NSError(domain: NSURLErrorDomain, code: NSURLErrorUnknown, userInfo: nil))) return } diff --git a/WordPress/Classes/ViewRelated/Jetpack/Install/Webview/JetpackConnectionWebViewController.swift b/WordPress/Classes/ViewRelated/Jetpack/Install/Webview/JetpackConnectionWebViewController.swift index 63193c821b41..699762a7ea11 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Install/Webview/JetpackConnectionWebViewController.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Install/Webview/JetpackConnectionWebViewController.swift @@ -361,7 +361,7 @@ private extension JetpackConnectionWebViewController { /// private extension JetpackConnectionWebViewController { func startNativeConnectionFlow() { - guard let api = blog.wordPressOrgRestApi else { + guard let api = blog.selfHostedSiteRestApi else { DDLogInfo("WordPressOrgRestAPI not loaded to perform native Jetpack connection") startConnectionFlow() return From bb220fd20e053352c84fbf386d71bc97db6030cd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 15 Feb 2024 12:32:02 +1300 Subject: [PATCH 04/16] Remove unused functions --- .../WordPressOrgRestApi+WordPress.swift | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift b/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift index 70e911f78dbe..959aead589cb 100644 --- a/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift +++ b/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift @@ -1,33 +1,6 @@ import Foundation import WordPressKit -private func makeAuthenticator(blog: Blog) -> Authenticator? { - return blog.account != nil - ? makeTokenAuthenticator(blog: blog) - : makeCookieNonceAuthenticator(blog: blog) -} - -private func makeTokenAuthenticator(blog: Blog) -> Authenticator? { - guard let token = blog.authToken else { - DDLogError("Failed to initialize a .com API client with blog: \(blog)") - return nil - } - return TokenAuthenticator(token: token) -} - -private func makeCookieNonceAuthenticator(blog: Blog) -> Authenticator? { - guard let loginURL = try? blog.loginUrl().asURL(), - let adminURL = try? blog.adminUrl(withPath: "").asURL(), - let username = blog.username, - let password = blog.password, - let version = blog.version as String? else { - DDLogError("Failed to initialize a .org API client with blog: \(blog)") - return nil - } - - return CookieNonceAuthenticator(username: username, password: password, loginURL: loginURL, adminURL: adminURL, version: version) -} - private func apiBase(blog: Blog) -> URL? { guard blog.account == nil else { assertionFailure(".com support has not been implemented yet") From f5afa997efb592633101d7c2f9c2b9ce7956d786 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 16 Feb 2024 22:25:07 +1300 Subject: [PATCH 05/16] Update BlockEditorSettingsService tests Use HTTP stubs instead of fake implementations --- .../BlockEditorSettingsServiceTests.swift | 136 ++++++++++-------- WordPress/WordPressTest/BlogBuilder.swift | 1 + .../MockWordPressComRestApi.swift | 30 ---- 3 files changed, 77 insertions(+), 90 deletions(-) diff --git a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift index 789cbc1864c8..4c0053031fb1 100644 --- a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift +++ b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift @@ -1,5 +1,6 @@ import XCTest import Nimble +import OHHTTPStubs @testable import WordPress class BlockEditorSettingsServiceTests: CoreDataTestCase { @@ -10,18 +11,16 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { private let blockSettingsThemeJSONResponseFilename = "wp-block-editor-v1-settings-success-ThemeJSON" private var service: BlockEditorSettingsService! - var mockRemoteApi: MockWordPressComRestApi! var gssOriginalValue: Bool! private var blog: Blog! override func setUp() { - mockRemoteApi = MockWordPressComRestApi() blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.8") .withAnAccount() .build() contextManager.saveContextAndWait(mainContext) - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockRemoteApi, coreDataStack: contextManager) + service = BlockEditorSettingsService(blog: blog, coreDataStack: contextManager) } // MARK: Editor `theme_supports` support @@ -29,11 +28,14 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.7") .with(isHostedAtWPCom: true) + .withAnAccount() .build() contextManager.saveContextAndWait(mainContext) - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockRemoteApi, coreDataStack: contextManager) let mockedResponse = mockedData(withFilename: twentytwentyoneResponseFilename) + stubThemeRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + + service = BlockEditorSettingsService(blog: blog, coreDataStack: contextManager) let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -46,8 +48,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateThemeResponse() XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) @@ -57,15 +57,18 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.7") .with(isHostedAtWPCom: true) + .withAnAccount() .build() contextManager.saveContextAndWait(mainContext) - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockRemoteApi, coreDataStack: contextManager) + service = BlockEditorSettingsService(blog: blog, coreDataStack: contextManager) setData(withFilename: twentytwentyResponseFilename) let originalChecksum = blog.blockEditorSettings?.checksum ?? "" let mockedResponse = mockedData(withFilename: twentytwentyoneResponseFilename) + stubThemeRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -78,8 +81,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateThemeResponse() XCTAssertNotEqual(self.blog.blockEditorSettings?.checksum, originalChecksum) @@ -89,14 +90,17 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.7") .with(isHostedAtWPCom: true) + .withAnAccount() .build() contextManager.saveContextAndWait(mainContext) - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockRemoteApi, coreDataStack: contextManager) + service = BlockEditorSettingsService(blog: blog, coreDataStack: contextManager) setData(withFilename: twentytwentyoneResponseFilename) let originalChecksum = blog.blockEditorSettings?.checksum ?? "" + let mockedResponse = mockedData(withFilename: twentytwentyoneResponseFilename) + stubThemeRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in @@ -109,7 +113,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) waitForExpectations(timeout: expectationTimeout) validateThemeResponse() @@ -117,17 +120,26 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } private func validateThemeResponse() { - let siteID = blog.dotComID ?? 0 - XCTAssertTrue(self.mockRemoteApi.getMethodCalled) - XCTAssertEqual(self.mockRemoteApi.URLStringPassedIn!, "/wp/v2/sites/\(siteID)/themes") - XCTAssertEqual((self.mockRemoteApi.parametersPassedIn as! [String: String])["status"], "active") XCTAssertGreaterThan(self.blog.blockEditorSettings!.colors!.count, 0) XCTAssertGreaterThan(self.blog.blockEditorSettings!.gradients!.count, 0) } + private func stubThemeRequest(response: HTTPStubsResponse) { + let siteID = blog.dotComID ?? 0 + stub( + condition: + isMethodGET() + && { $0.url?.absoluteString.contains("/wp/v2/sites/\(siteID)/themes") == true } + && { $0.url?.absoluteString.contains("status=active") == true }, + response: { _ in response } + ) + } + // MARK: Editor Global Styles support func testFetchBlockEditorSettingsNotThemeJSON() { let mockedResponse = mockedData(withFilename: blockSettingsNOTThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -140,8 +152,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateBlockEditorSettingsResponse(isGlobalStyles: false) XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) @@ -149,6 +159,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { func testFetchBlockEditorSettingsThemeJSON() { let mockedResponse = mockedData(withFilename: blockSettingsThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -161,8 +173,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateBlockEditorSettingsResponse() XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) @@ -173,6 +183,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { let originalChecksum = blog.blockEditorSettings?.checksum ?? "" let mockedResponse = mockedData(withFilename: blockSettingsThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -185,8 +197,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateBlockEditorSettingsResponse() XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) @@ -198,6 +208,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { let originalChecksum = blog.blockEditorSettings?.checksum ?? "" let mockedResponse = mockedData(withFilename: blockSettingsThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -210,8 +222,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateBlockEditorSettingsResponse() XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) @@ -224,6 +234,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { let originalChecksum = blog.blockEditorSettings?.checksum ?? "" let mockedResponse = mockedData(withFilename: blockSettingsThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { result in switch result { @@ -236,8 +248,6 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) validateBlockEditorSettingsResponse() XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) @@ -245,41 +255,44 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } func testFetchBlockEditorSettings_OrgSite_NoPlugin() { + let first = expectation(description: "The app will call the none-experimental path first but fail because of compatibility reasons") + let second = expectation(description: "The app will then retry the wp/v2/themes path") + let mockedResponse = mockedData(withFilename: blockSettingsNOTThemeJSONResponseFilename) - let mockOrgRemoteApi = MockWordPressOrgRestApi() - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockOrgRemoteApi, coreDataStack: contextManager) + stub(condition: isAbsoluteURLString("https://w.org/wp-json/wp-block-editor/v1/settings?context=mobile")) { _ in + first.fulfill() + return HTTPStubsResponse(error: URLError(.networkConnectionLost)) + } + stub(condition: isAbsoluteURLString("https://w.org/wp-json/wp/v2/themes?status=active")) { _ in + second.fulfill() + return HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil) + } + + let remoteAPI = WordPressOrgRestApi(selfHostedSiteWPJSONURL: URL(string: "https://w.org/wp-json")!, credential: .init(loginURL: URL(string: "https://not-used.org")!, username: "user", password: "pass", adminURL: URL(string: "https://not-used.org")!)) + service = BlockEditorSettingsService(blog: blog, remoteAPI: remoteAPI, coreDataStack: contextManager) let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { _ in waitExpectation.fulfill() } - // The app will call the none-experimental path first but fail because of compatibility reasons - mockOrgRemoteApi.completionPassedIn!(.failure(NSError(domain: "test", code: 404, userInfo: nil)), HTTPURLResponse()) - // The app will then retry the wp/v2/themes path. - mockOrgRemoteApi.completionPassedIn!(.success(mockedResponse), HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) - - XCTAssertTrue(mockOrgRemoteApi.getMethodCalled) - XCTAssertEqual(mockOrgRemoteApi.URLStringPassedIn!, "/wp/v2/themes") - XCTAssertEqual((mockOrgRemoteApi.parametersPassedIn as! [String: String])["status"], "active") + wait(for: [first, second, waitExpectation], timeout: 0.3, enforceOrder: true) } func testFetchBlockEditorSettings_OrgSite() { let mockedResponse = mockedData(withFilename: blockSettingsNOTThemeJSONResponseFilename) - let mockOrgRemoteApi = MockWordPressOrgRestApi() - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockOrgRemoteApi, coreDataStack: contextManager) + stub(condition: isAbsoluteURLString("https://w.org/wp-json/wp-block-editor/v1/settings?context=mobile")) { _ in + HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil) + } + + let remoteAPI = WordPressOrgRestApi(selfHostedSiteWPJSONURL: URL(string: "https://w.org/wp-json")!, credential: .init(loginURL: URL(string: "https://not-used.org")!, username: "user", password: "pass", adminURL: URL(string: "https://not-used.org")!)) + service = BlockEditorSettingsService(blog: blog, remoteAPI: remoteAPI, coreDataStack: contextManager) let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { _ in waitExpectation.fulfill() } - mockOrgRemoteApi.completionPassedIn!(.success(mockedResponse), HTTPURLResponse()) waitForExpectations(timeout: expectationTimeout) - - XCTAssertTrue(mockOrgRemoteApi.getMethodCalled) - XCTAssertEqual(mockOrgRemoteApi.URLStringPassedIn!, "/wp-block-editor/v1/settings") - XCTAssertEqual((mockOrgRemoteApi.parametersPassedIn as! [String: String])["context"], "mobile") } func testFetchBlockEditorSettings_Com5_8Site() { @@ -289,19 +302,17 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { .build() contextManager.saveContextAndWait(mainContext) - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockRemoteApi, coreDataStack: contextManager) + service = BlockEditorSettingsService(blog: blog, coreDataStack: contextManager) let mockedResponse = mockedData(withFilename: blockSettingsNOTThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) + let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { _ in waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) - XCTAssertTrue(self.mockRemoteApi.getMethodCalled) - XCTAssertEqual(self.mockRemoteApi.URLStringPassedIn!, "/wp-block-editor/v1/sites/\(blog.dotComID!.intValue)/settings") - XCTAssertEqual((self.mockRemoteApi.parametersPassedIn as! [String: String])["context"], "mobile") + waitForExpectations(timeout: expectationTimeout) } func testFetchBlockEditorSettings_Com5_9Site() { @@ -311,26 +322,19 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { .build() contextManager.saveContextAndWait(mainContext) - service = BlockEditorSettingsService(blog: blog, remoteAPI: mockRemoteApi, coreDataStack: contextManager) + service = BlockEditorSettingsService(blog: blog, coreDataStack: contextManager) let mockedResponse = mockedData(withFilename: blockSettingsNOTThemeJSONResponseFilename) + stubBlockEditorSettingsRequest(response: HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil)) let waitExpectation = expectation(description: "Theme should be successfully fetched") service.fetchSettings { _ in waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) - waitForExpectations(timeout: expectationTimeout) - XCTAssertTrue(self.mockRemoteApi.getMethodCalled) - XCTAssertEqual(self.mockRemoteApi.URLStringPassedIn!, "/wp-block-editor/v1/sites/\(blog.dotComID!.intValue)/settings") - XCTAssertEqual((self.mockRemoteApi.parametersPassedIn as! [String: String])["context"], "mobile") + waitForExpectations(timeout: expectationTimeout) } private func validateBlockEditorSettingsResponse(isGlobalStyles: Bool = true) { - XCTAssertTrue(self.mockRemoteApi.getMethodCalled) - XCTAssertEqual(self.mockRemoteApi.URLStringPassedIn!, "/wp-block-editor/v1/sites/\(blog.dotComID!.intValue)/settings") - XCTAssertEqual((self.mockRemoteApi.parametersPassedIn as! [String: String])["context"], "mobile") - if isGlobalStyles { XCTAssertNotNil(self.blog.blockEditorSettings?.rawStyles) XCTAssertNotNil(self.blog.blockEditorSettings?.rawFeatures) @@ -355,6 +359,17 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } XCTAssertGreaterThan(gradients.count, 0) } + + private func stubBlockEditorSettingsRequest(response: HTTPStubsResponse) { + let siteID = blog.dotComID ?? 0 + stub( + condition: + isMethodGET() + && { $0.url?.absoluteString.contains("/wp-block-editor/v1/sites/\(siteID)/settings") == true } + && { $0.url?.absoluteString.contains("context=mobile") == true }, + response: { _ in response } + ) + } } extension BlockEditorSettingsServiceTests { @@ -368,10 +383,11 @@ extension BlockEditorSettingsServiceTests { func setData(withFilename filename: String) { let waitExpectation = expectation(description: "Theme should be successfully fetched") let mockedResponse = mockedData(withFilename: filename) + let descriptor = stub(condition: { _ in true }, response: { _ in HTTPStubsResponse(jsonObject: mockedResponse, statusCode: 200, headers: nil) }) service.fetchSettings { _ in waitExpectation.fulfill() } - mockRemoteApi.successBlockPassedIn!(mockedResponse, HTTPURLResponse()) waitForExpectations(timeout: expectationTimeout) + HTTPStubs.removeStub(descriptor) } } diff --git a/WordPress/WordPressTest/BlogBuilder.swift b/WordPress/WordPressTest/BlogBuilder.swift index 9ea54aaee42f..8ae0ab8bd5e9 100644 --- a/WordPress/WordPressTest/BlogBuilder.swift +++ b/WordPress/WordPressTest/BlogBuilder.swift @@ -97,6 +97,7 @@ final class BlogBuilder { let account = NSEntityDescription.insertNewObject(forEntityName: WPAccount.entityName(), into: context) as! WPAccount account.displayName = "displayName" account.username = username + account.authToken = "authtoken" blog.account = account return self diff --git a/WordPress/WordPressTest/MockWordPressComRestApi.swift b/WordPress/WordPressTest/MockWordPressComRestApi.swift index b8cdcdd42259..7f63f6db514e 100644 --- a/WordPress/WordPressTest/MockWordPressComRestApi.swift +++ b/WordPress/WordPressTest/MockWordPressComRestApi.swift @@ -56,33 +56,3 @@ class MockWordPressComRestApi: WordPressComRestApi { return method } } - -class MockWordPressOrgRestApi: WordPressOrgRestApi { - var getMethodCalled = false - var URLStringPassedIn: String? - var parametersPassedIn: AnyObject? - var completionPassedIn: WordPressOrgRestApi.Completion? - - init() { - super.init(apiBase: URL(string: "https://example.com")!) - } - - override func GET(_ path: String, parameters: [String: AnyObject]?, completion: @escaping WordPressOrgRestApi.Completion) -> Progress? { - getMethodCalled = true - URLStringPassedIn = path - parametersPassedIn = parameters as AnyObject? - completionPassedIn = completion - - return Progress() - } - - @objc func methodCalled() -> String { - - var method = "Unknown" - if getMethodCalled { - method = "GET" - } - - return method - } -} From 8b828aee677f4cf51c1e07bce037f2c4d3c508dc Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 16 Feb 2024 22:29:04 +1300 Subject: [PATCH 06/16] Add a release note --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index cb2189dc0db4..6ddc3a6e35e6 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -2,6 +2,7 @@ ----- * [**] Multiple pre-publishing sheet fixes and improvements [#22606] * [*] Gravatar: Adds informative new view about Gravatar to the profile editing page. [#22615] +* [**] [internal] Refactored .org REST API calls. [#22612] 24.2 ----- From 73fef127c43881ed8000b4212e93cdaccdb0745b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 19 Feb 2024 11:40:14 +1300 Subject: [PATCH 07/16] Fix flaky unit tests caused by randomly generated `dotComID` --- WordPress/Classes/Models/Blog.h | 1 + WordPress/WordPressTest/BlogBuilder.swift | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/Models/Blog.h b/WordPress/Classes/Models/Blog.h index 4022651da0a8..42e4530f84d7 100644 --- a/WordPress/Classes/Models/Blog.h +++ b/WordPress/Classes/Models/Blog.h @@ -121,6 +121,7 @@ typedef NS_ENUM(NSInteger, SiteVisibility) { @interface Blog : NSManagedObject @property (nonatomic, strong, readwrite, nullable) NSNumber *blogID __deprecated_msg("Use dotComID instead"); +/// WordPress.com site ID stored as signed 32-bit integer. @property (nonatomic, strong, readwrite, nullable) NSNumber *dotComID; @property (nonatomic, strong, readwrite, nullable) NSString *xmlrpc; @property (nonatomic, strong, readwrite, nullable) NSString *apiKey; diff --git a/WordPress/WordPressTest/BlogBuilder.swift b/WordPress/WordPressTest/BlogBuilder.swift index 8ae0ab8bd5e9..b59bb98910e2 100644 --- a/WordPress/WordPressTest/BlogBuilder.swift +++ b/WordPress/WordPressTest/BlogBuilder.swift @@ -17,7 +17,7 @@ final class BlogBuilder { blog = NSEntityDescription.insertNewObject(forEntityName: Blog.entityName(), into: context) as! Blog // Non-null properties in Core Data - blog.dotComID = NSNumber(value: arc4random_uniform(UInt32.max)) + blog.dotComID = NSNumber(value: arc4random_uniform(999_999)) blog.url = "https://\(blog.dotComID!).example.com" blog.xmlrpc = "https://\(blog.dotComID!).example.com/xmlrpc.php" } From 18d83829905d10901eb3736a53e77e8462f938ae Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 19 Feb 2024 11:45:16 +1300 Subject: [PATCH 08/16] Log error into test failures --- .../BlockEditorSettingsServiceTests.swift | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift index 4c0053031fb1..6d5b4ea21a55 100644 --- a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift +++ b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift @@ -42,8 +42,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertTrue(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -75,8 +75,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertTrue(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -108,8 +108,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertFalse(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -146,8 +146,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertTrue(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -167,8 +167,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertTrue(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -191,8 +191,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertTrue(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -216,8 +216,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertTrue(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } @@ -242,8 +242,8 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { case .success(let settings): XCTAssertFalse(settings.hasChanges) XCTAssertNotNil(settings.blockEditorSettings) - case .failure: - XCTFail() + case let .failure(error): + XCTFail("Unexpected failure: \(error)") } waitExpectation.fulfill() } From 876b975f451de840dd09be34f6f877aea0e2752c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 19 Feb 2024 11:45:56 +1300 Subject: [PATCH 09/16] Throw on nil value --- .../BlockEditorSettingsServiceTests.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift index 6d5b4ea21a55..e5ff809749b1 100644 --- a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift +++ b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift @@ -24,7 +24,7 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } // MARK: Editor `theme_supports` support - func testThemeSupportsNewTheme() { + func testThemeSupportsNewTheme() throws { blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.7") .with(isHostedAtWPCom: true) @@ -49,11 +49,11 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } waitForExpectations(timeout: expectationTimeout) - validateThemeResponse() + try validateThemeResponse() XCTAssertNotNil(self.blog.blockEditorSettings?.checksum) } - func testThemeSupportsThemeChange() { + func testThemeSupportsThemeChange() throws { blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.7") .with(isHostedAtWPCom: true) @@ -82,11 +82,11 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } waitForExpectations(timeout: expectationTimeout) - validateThemeResponse() + try validateThemeResponse() XCTAssertNotEqual(self.blog.blockEditorSettings?.checksum, originalChecksum) } - func testThemeSupportsThemeIsTheSame() { + func testThemeSupportsThemeIsTheSame() throws { blog = BlogBuilder(mainContext) .with(wordPressVersion: "5.7") .with(isHostedAtWPCom: true) @@ -115,13 +115,13 @@ class BlockEditorSettingsServiceTests: CoreDataTestCase { } waitForExpectations(timeout: expectationTimeout) - validateThemeResponse() + try validateThemeResponse() XCTAssertEqual(self.blog.blockEditorSettings?.checksum, originalChecksum) } - private func validateThemeResponse() { - XCTAssertGreaterThan(self.blog.blockEditorSettings!.colors!.count, 0) - XCTAssertGreaterThan(self.blog.blockEditorSettings!.gradients!.count, 0) + private func validateThemeResponse() throws { + try XCTAssertGreaterThan(XCTUnwrap(self.blog.blockEditorSettings?.colors?.count), 0) + try XCTAssertGreaterThan(XCTUnwrap(self.blog.blockEditorSettings?.gradients?.count), 0) } private func stubThemeRequest(response: HTTPStubsResponse) { From a3d03a61165940bb4bc8e49f13fc1c97527af543 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Mon, 19 Feb 2024 11:46:14 +1300 Subject: [PATCH 10/16] Do not use mock in a CommentService unit test --- .../BlockEditorSettingsServiceTests.swift | 2 +- .../CommentService+RepliesTests.swift | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift index e5ff809749b1..870d8cf9de67 100644 --- a/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift +++ b/WordPress/WordPressTest/BlockEditorSettingsServiceTests.swift @@ -387,7 +387,7 @@ extension BlockEditorSettingsServiceTests { service.fetchSettings { _ in waitExpectation.fulfill() } - waitForExpectations(timeout: expectationTimeout) + wait(for: [waitExpectation], timeout: expectationTimeout) HTTPStubs.removeStub(descriptor) } } diff --git a/WordPress/WordPressTest/CommentService+RepliesTests.swift b/WordPress/WordPressTest/CommentService+RepliesTests.swift index c1afb8d7d222..b7ab0b468636 100644 --- a/WordPress/WordPressTest/CommentService+RepliesTests.swift +++ b/WordPress/WordPressTest/CommentService+RepliesTests.swift @@ -86,19 +86,22 @@ final class CommentService_RepliesTests: CoreDataTestCase { } func test_getReplies_addsCommentIdInParameter() { - let (mockService, mockApi) = makeMockService() + var request: URLRequest? + stub(condition: { $0.url?.absoluteString.contains("/wp/v2/sites/\(self.siteID)/comments") == true }) { + request = $0 + return HTTPStubsResponse(error: URLError(.networkConnectionLost)) + } + let parentKey = CommentServiceRemoteREST.RequestKeys.parent.rawValue - mockService.getLatestReplyID(for: commentID, + self.commentService.getLatestReplyID(for: commentID, siteID: siteID, accountService: accountService, success: { _ in }, failure: { _ in }) - var parameters = [String: Any]() - expect(mockApi.parametersPassedIn).toNot(beNil()) - expect { parameters = mockApi.parametersPassedIn! as! [String: Any] }.toNot(throwError()) - expect(parameters[parentKey] as? Int).to(equal(commentID)) + expect(request).toNotEventually(beNil()) + expect(request?.url?.query).to(contain("parent=\(commentID)")) } func test_replyToPost_givenSuccessfulAPICall_insertsNewComment() throws { From 9ab9cf3b9b64a5fc162454eb570ac9dae9059ad3 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Mon, 19 Feb 2024 15:11:26 +1100 Subject: [PATCH 11/16] Use a WordPressKit version that should fix the Reader loading issues See how the UI tests failed in https://buildkite.com/automattic/wordpress-ios/builds/20452#018dbe88-2fc5-4998-ad35-1b455f547b59 The WordPressKit PR is https://github.com/wordpress-mobile/WordPressKit-iOS/pull/727 --- Podfile | 2 +- Podfile.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Podfile b/Podfile index 280eb8a2103f..7582a0f5a967 100644 --- a/Podfile +++ b/Podfile @@ -51,7 +51,7 @@ end def wordpress_kit # pod 'WordPressKit', '~> 13.0' - pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: 'dfb2134ca3b91233d44b3551d91844bf2087996f' + pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: '3df47e113c7647b76c049180e39489e3af7a27dc' # pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', branch: '' # pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', tag: '' # pod 'WordPressKit', path: '../WordPressKit-iOS' diff --git a/Podfile.lock b/Podfile.lock index 027915ebe487..6511715f554b 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -120,7 +120,7 @@ DEPENDENCIES: - SwiftLint (~> 0.50) - WordPress-Editor-iOS (~> 1.19.9) - WordPressAuthenticator (from `https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git`, commit `fa06fca7178b268d382d91861752b3be0729e8a8`) - - WordPressKit (from `https://github.com/wordpress-mobile/WordPressKit-iOS.git`, commit `dfb2134ca3b91233d44b3551d91844bf2087996f`) + - WordPressKit (from `https://github.com/wordpress-mobile/WordPressKit-iOS.git`, commit `3df47e113c7647b76c049180e39489e3af7a27dc`) - WordPressShared (~> 2.3) - WordPressUI (~> 1.15) - ZendeskSupportSDK (= 5.3.0) @@ -178,7 +178,7 @@ EXTERNAL SOURCES: :commit: fa06fca7178b268d382d91861752b3be0729e8a8 :git: https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git WordPressKit: - :commit: dfb2134ca3b91233d44b3551d91844bf2087996f + :commit: 3df47e113c7647b76c049180e39489e3af7a27dc :git: https://github.com/wordpress-mobile/WordPressKit-iOS.git CHECKOUT OPTIONS: @@ -189,7 +189,7 @@ CHECKOUT OPTIONS: :commit: fa06fca7178b268d382d91861752b3be0729e8a8 :git: https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git WordPressKit: - :commit: dfb2134ca3b91233d44b3551d91844bf2087996f + :commit: 3df47e113c7647b76c049180e39489e3af7a27dc :git: https://github.com/wordpress-mobile/WordPressKit-iOS.git SPEC CHECKSUMS: @@ -236,6 +236,6 @@ SPEC CHECKSUMS: ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba ZIPFoundation: d170fa8e270b2a32bef9dcdcabff5b8f1a5deced -PODFILE CHECKSUM: 91ca97cb58a7ec8391611ea499d341d47f633cda +PODFILE CHECKSUM: f732e6e39fc98112f1812c770b6bb5daf5a73672 COCOAPODS: 1.14.2 From 57cd3abdc2215fd9ce6b0266fdee888ed5ea4771 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 21 Feb 2024 09:43:12 +1300 Subject: [PATCH 12/16] Update WordPressKit --- Podfile | 2 +- Podfile.lock | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Podfile b/Podfile index 7582a0f5a967..cdb48d5dab15 100644 --- a/Podfile +++ b/Podfile @@ -51,7 +51,7 @@ end def wordpress_kit # pod 'WordPressKit', '~> 13.0' - pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: '3df47e113c7647b76c049180e39489e3af7a27dc' + pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: '7c01ab3f347d16c083239ce9f2945636e7fd4611' # pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', branch: '' # pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', tag: '' # pod 'WordPressKit', path: '../WordPressKit-iOS' diff --git a/Podfile.lock b/Podfile.lock index 6511715f554b..54127aec862c 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -70,7 +70,7 @@ PODS: - WordPressKit (~> 13.0) - WordPressShared (~> 2.1-beta) - WordPressUI (~> 1.7-beta) - - WordPressKit (13.0.0): + - WordPressKit (13.1.0): - Alamofire (~> 4.8.0) - NSObject-SafeExpectations (~> 0.0.4) - UIDeviceIdentifier (~> 2.0) @@ -120,7 +120,7 @@ DEPENDENCIES: - SwiftLint (~> 0.50) - WordPress-Editor-iOS (~> 1.19.9) - WordPressAuthenticator (from `https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git`, commit `fa06fca7178b268d382d91861752b3be0729e8a8`) - - WordPressKit (from `https://github.com/wordpress-mobile/WordPressKit-iOS.git`, commit `3df47e113c7647b76c049180e39489e3af7a27dc`) + - WordPressKit (from `https://github.com/wordpress-mobile/WordPressKit-iOS.git`, commit `7c01ab3f347d16c083239ce9f2945636e7fd4611`) - WordPressShared (~> 2.3) - WordPressUI (~> 1.15) - ZendeskSupportSDK (= 5.3.0) @@ -178,7 +178,7 @@ EXTERNAL SOURCES: :commit: fa06fca7178b268d382d91861752b3be0729e8a8 :git: https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git WordPressKit: - :commit: 3df47e113c7647b76c049180e39489e3af7a27dc + :commit: 7c01ab3f347d16c083239ce9f2945636e7fd4611 :git: https://github.com/wordpress-mobile/WordPressKit-iOS.git CHECKOUT OPTIONS: @@ -189,7 +189,7 @@ CHECKOUT OPTIONS: :commit: fa06fca7178b268d382d91861752b3be0729e8a8 :git: https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git WordPressKit: - :commit: 3df47e113c7647b76c049180e39489e3af7a27dc + :commit: 7c01ab3f347d16c083239ce9f2945636e7fd4611 :git: https://github.com/wordpress-mobile/WordPressKit-iOS.git SPEC CHECKSUMS: @@ -223,7 +223,7 @@ SPEC CHECKSUMS: WordPress-Aztec-iOS: fbebd569c61baa252b3f5058c0a2a9a6ada686bb WordPress-Editor-iOS: bda9f7f942212589b890329a0cb22547311749ef WordPressAuthenticator: ba69878bfa1368636e92d29fcfb5bd1e0a11a3ef - WordPressKit: 7e5250e9e28fcdca787f8d313a7dc89a8571d774 + WordPressKit: c1ba7b4f531693a0914f676423808fdffd820d81 WordPressShared: cad7777b283d3ce2752f283df587342a581cd49b WordPressUI: a491454affda3b0fb812812e637dc5e8f8f6bd06 wpxmlrpc: 68db063041e85d186db21f674adf08d9c70627fd @@ -236,6 +236,6 @@ SPEC CHECKSUMS: ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba ZIPFoundation: d170fa8e270b2a32bef9dcdcabff5b8f1a5deced -PODFILE CHECKSUM: f732e6e39fc98112f1812c770b6bb5daf5a73672 +PODFILE CHECKSUM: f5237dc3ffdf654de2b02889eaa7f113cab1f358 COCOAPODS: 1.14.2 From e0d37446df0808dbe9fc7ac41f33127fb75080bf Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 21 Feb 2024 09:43:26 +1300 Subject: [PATCH 13/16] Use WP.com API url from AppEnvironment So that WP.com .org REST API calls can be mocked by API-Mocks during UI tests. --- .../Classes/Networking/WordPressOrgRestApi+WordPress.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift b/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift index 959aead589cb..ccba1f67df81 100644 --- a/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift +++ b/WordPress/Classes/Networking/WordPressOrgRestApi+WordPress.swift @@ -15,7 +15,12 @@ extension WordPressOrgRestApi { if let dotComID = blog.dotComID?.uint64Value, let token = blog.account?.authToken, token.count > 0 { - self.init(dotComSiteID: dotComID, bearerToken: token, userAgent: WPUserAgent.wordPress()) + self.init( + dotComSiteID: dotComID, + bearerToken: token, + userAgent: WPUserAgent.wordPress(), + apiURL: AppEnvironment.current.wordPressComApiBase + ) } else if let apiBase = apiBase(blog: blog), let loginURL = try? blog.loginUrl().asURL(), let adminURL = try? blog.adminUrl(withPath: "").asURL(), From e01a8db4fc397f9d2c850ba4551eb97c110d22cd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 21 Feb 2024 10:02:16 +1300 Subject: [PATCH 14/16] Use new `PluginDirectoryServiceRemote` API --- WordPress/Classes/Stores/PluginStore.swift | 30 ++++++++++------------ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/WordPress/Classes/Stores/PluginStore.swift b/WordPress/Classes/Stores/PluginStore.swift index d8c909bedd5b..57547a57a436 100644 --- a/WordPress/Classes/Stores/PluginStore.swift +++ b/WordPress/Classes/Stores/PluginStore.swift @@ -690,18 +690,16 @@ private extension PluginStore { } func fetchPluginDirectoryEntry(slug: String) { - let remote = PluginDirectoryServiceRemote() state.fetchingDirectoryEntry[slug] = true - remote.getPluginInformation( - slug: slug, - completion: { [actionDispatcher] (result) in - switch result { - case .success(let entry): - actionDispatcher.dispatch(PluginAction.receivePluginDirectoryEntry(slug: slug, entry: entry)) - case .failure(let error): - actionDispatcher.dispatch(PluginAction.receivePluginDirectoryEntryFailed(slug: slug, error: error)) - } - }) + Task { @MainActor [actionDispatcher] in + let remote = PluginDirectoryServiceRemote() + do { + let entry = try await remote.getPluginInformation(slug: slug) + actionDispatcher.dispatch(PluginAction.receivePluginDirectoryEntry(slug: slug, entry: entry)) + } catch { + actionDispatcher.dispatch(PluginAction.receivePluginDirectoryEntryFailed(slug: slug, error: error)) + } + } } func receivePluginDirectoryEntry(slug: String, entry: PluginDirectoryEntry) { @@ -747,12 +745,12 @@ private extension PluginStore { func fetchPluginDirectoryFeed(feed: PluginDirectoryFeedType) { state.fetchingDirectoryFeed[feed.slug] = true - let remote = PluginDirectoryServiceRemote() - remote.getPluginFeed(feed) { [actionDispatcher] result in - switch result { - case .success(let response): + Task { @MainActor [actionDispatcher] in + let remote = PluginDirectoryServiceRemote() + do { + let response = try await remote.getPluginFeed(feed) actionDispatcher.dispatch(PluginAction.receivePluginDirectoryFeed(feed: feed, response: response)) - case .failure(let error): + } catch { actionDispatcher.dispatch(PluginAction.receivePluginDirectoryFeedFailed(feed: feed, error: error)) } } From 5407c043c2341a7e86505498834a6324b0ec3c7d Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 21 Feb 2024 10:07:46 +1300 Subject: [PATCH 15/16] Move a release note to the next release --- RELEASE-NOTES.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 6fea3f4464af..926f0ba50fd0 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,6 +1,6 @@ 24.4 ----- - +* [**] [internal] Refactored .org REST API calls. [#22612] 24.3 ----- @@ -8,7 +8,6 @@ * [**] Multiple pre-publishing sheet fixes and improvements [#22606] * [*] Gravatar: Adds informative new view about Gravatar to the profile editing page. [#22615] -* [**] [internal] Refactored .org REST API calls. [#22612] 24.2 ----- From 3de5c2fdd67e59d1b073e267ac6fff4b8dd2016a Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 21 Feb 2024 11:34:41 +1300 Subject: [PATCH 16/16] Add a comment about `GutenbergNetworkRequest.path` --- .../Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift index be537c55681e..58b6bc1f205d 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergNetworking.swift @@ -4,6 +4,11 @@ import WordPressKit struct GutenbergNetworkRequest { typealias CompletionHandler = (Swift.Result) -> Void + // Please note: even though this variable is named 'path', the actually values (passed from React Native) contain + // path and query. + // + // For example, when uploading an image to a post in a self-hosted site, this struct is instantiated with a path + // `/wp/v2/media/?context=edit&_locale=user`. private let path: String private unowned let blog: Blog private let method: HTTPMethod