Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move WPAccount wordPressComRestAPI definition to Swift #24230

Merged
merged 4 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 63 additions & 2 deletions WordPress/Classes/Models/WPAccount+RestApi.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,59 @@
import Foundation
import WordPressShared
import WordPressKit
import WordPressShared

extension WPAccount {

/// A `WordPressRestComApi` object if the account is a WordPress.com account. Otherwise, `nil`.
///
/// Warning: The getter has various side effects when there is no previous value set, including triggering the signup flow!
///
/// This used to be defined in the Objective-C layer, but we moved it here in a Swift extension in an attempt to decouple the model code from it.
/// This was done in the context of https://github.com/wordpress-mobile/WordPress-iOS/pull/24165 .
@objc var wordPressComRestApi: WordPressComRestApi? {
get {
guard let api = objc_getAssociatedObject(self, &apiAssociatedKey) as? WordPressComRestApi else {
guard authToken.isEmpty else {
let api = WordPressComRestApi.defaultApi(
oAuthToken: authToken,
userAgent: WPUserAgent.defaultUserAgent(),
localeKey: WordPressComRestApi.LocaleKeyDefault
)

api.setInvalidTokenHandler { [weak self] in
guard let self else { return }

self.authToken = nil
WordPressAuthenticationManager.showSigninForWPComFixingAuthToken()

guard self.isDefaultWordPressComAccount else { return }

// At the time of writing, there is an implicit assumption on what the object parameter value means.
// For example, the WordPressAppDelegate.handleDefaultAccountChangedNotification(_:) subscriber inspects the object parameter to decide whether the notification was sent as a result of a login.
// If the object is non-nil, then the method considers the source a login.
//
// The code path in which we are is that of an invalid token, and that's neither a login nor a logout, it's more appropriate to consider it a logout.
// That's because if the token is invalid the app will soon received errors from the API and it's therefore better to force the user to login again.
NotificationCenter.default.post(
name: NSNotification.Name.WPAccountDefaultWordPressComAccountChanged,
object: nil
)
}

return api
}

DispatchQueue.main.async {
WordPressAuthenticationManager.showSigninForWPComFixingAuthToken()
}
return nil
}
return api
}
set(api) {
objc_setAssociatedObject(self, &apiAssociatedKey, api, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
}
}

/// Returns an instance of the WPCOM REST API suitable for v2 endpoints.
/// If the user is not authenticated, this will be anonymous.
///
Expand All @@ -13,4 +64,14 @@ extension WPAccount {

return WordPressComRestApi.defaultApi(oAuthToken: token, userAgent: userAgent, localeKey: localeKey)
}

/// A `WordPressRestComApi` object if a default account exists in the giveng `NSManagedObjectContext` and is a WordPress.com account.
/// Otherwise, it returns `nil`
static func defaultWordPressComAccountRestAPI(in context: NSManagedObjectContext) throws -> WordPressComRestApi? {
let account = try WPAccount.lookupDefaultWordPressComAccount(in: context)
return account?.wordPressComRestApi
}

}

private var apiAssociatedKey: UInt8 = 0
10 changes: 6 additions & 4 deletions WordPress/Classes/Models/WPAccount.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
/// @name API Helpers
///------------------

/**
A WordPressRestComApi object if the account is a WordPress.com account. Otherwise, it returns `nil`
*/
@property (nonatomic, readonly) WordPressComRestApi *wordPressComRestApi;
/// A WordPressRestComApi object if the account is a WordPress.com account. Otherwise, it returns `nil`.
///
/// Important: Do not set this directly!
///
/// It's reserved for Objective-C to Swift interoperability in the context of separating this model from the app target and will be removed at some point.
@property (nonatomic, strong) WordPressComRestApi *wordPressComRestApi;

@end

Expand Down
44 changes: 5 additions & 39 deletions WordPress/Classes/Models/WPAccount.m
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

@interface WPAccount ()

@property (nonatomic, strong, readwrite) WordPressComRestApi *wordPressComRestApi;
@property (nonatomic, strong, readwrite) NSString *cachedToken;

@end
Expand All @@ -23,7 +22,7 @@ @implementation WPAccount
@dynamic userID;
@dynamic avatarURL;
@dynamic settings;
@synthesize wordPressComRestApi = _wordPressComRestApi;
@synthesize wordPressComRestApi;
@synthesize cachedToken;

#pragma mark - NSManagedObject subclass methods
Expand All @@ -35,15 +34,15 @@ - (void)prepareForDeletion
return;
}

[_wordPressComRestApi invalidateAndCancelTasks];
_wordPressComRestApi = nil;
[self.wordPressComRestApi invalidateAndCancelTasks];
self.wordPressComRestApi = nil;
self.authToken = nil;
}

- (void)didTurnIntoFault
{
[super didTurnIntoFault];
_wordPressComRestApi = nil;
self.wordPressComRestApi = nil;
self.cachedToken = nil;
}

Expand Down Expand Up @@ -115,7 +114,7 @@ - (void)setAuthToken:(NSString *)authToken
}

// Make sure to release any RestAPI alloc'ed, since it might have an invalid token
_wordPressComRestApi = nil;
self.wordPressComRestApi = nil;
}

- (BOOL)hasAtomicSite {
Expand Down Expand Up @@ -160,37 +159,4 @@ + (NSString *)authKeychainServiceName
return [AppConstants authKeychainServiceName];
}

#pragma mark - API Helpers

- (WordPressComRestApi *)wordPressComRestApi
{
if (!_wordPressComRestApi) {
if (self.authToken.length > 0) {
__weak __typeof(self) weakSelf = self;
_wordPressComRestApi = [WordPressComRestApi defaultApiWithOAuthToken:self.authToken
userAgent:[WPUserAgent wordPressUserAgent]
localeKey:[WordPressComRestApi LocaleKeyDefault]];
[_wordPressComRestApi setInvalidTokenHandler:^{
[weakSelf setAuthToken:nil];
[WordPressAuthenticationManager showSigninForWPComFixingAuthToken];
if (weakSelf.isDefaultWordPressComAccount) {
// At the time of writing, there is an implicit assumption on what the object parameter value means.
// For example, the WordPressAppDelegate.handleDefaultAccountChangedNotification(_:) subscriber inspects the object parameter to decide whether the notification was sent as a result of a login.
// If the object is non-nil, then the method considers the source a login.
//
// The code path in which we are is that of an invalid token, and that's neither a login nor a logout, it's more appropriate to consider it a logout.
// That's because if the token is invalid the app will soon received errors from the API and it's therefore better to force the user to login again.
[[NSNotificationCenter defaultCenter] postNotificationName:WPAccountDefaultWordPressComAccountChangedNotification object:nil];
}
}];
} else {
dispatch_async(dispatch_get_main_queue(), ^{
[WordPressAuthenticationManager showSigninForWPComFixingAuthToken];
});
}
}
return _wordPressComRestApi;

}

@end
3 changes: 1 addition & 2 deletions WordPress/Classes/Services/PostServiceRemoteFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ import WordPressShared
/// - Returns: an instance of WordPressComRestApi
private func apiForRESTRequest(using context: NSManagedObjectContext) -> WordPressComRestApi? {

guard let account = try? WPAccount.lookupDefaultWordPressComAccount(in: context),
let api = account.wordPressComRestApi else {
guard let api = try? WPAccount.defaultWordPressComAccountRestAPI(in: context) else {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Services/PushAuthenticationService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class PushAuthenticationService {

var api: WordPressComRestApi? = nil

if let unwrappedRestApi = (try? WPAccount.lookupDefaultWordPressComAccount(in: context))?.wordPressComRestApi {
if let unwrappedRestApi = (try? WPAccount.defaultWordPressComAccountRestAPI(in: context)) {
if unwrappedRestApi.hasCredentials() {
api = unwrappedRestApi
}
Expand Down
3 changes: 1 addition & 2 deletions WordPress/Classes/Services/ReaderSiteSearchService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ typealias ReaderSiteSearchFailureBlock = (_ error: Error?) -> Void

private func apiRequest() -> WordPressComRestApi {
let api = coreDataStack.performQuery {
let defaultAccount = try? WPAccount.lookupDefaultWordPressComAccount(in: $0)
return defaultAccount?.wordPressComRestApi
try? WPAccount.defaultWordPressComAccountRestAPI(in: $0)
}

if let api, api.hasCredentials() {
Expand Down
3 changes: 1 addition & 2 deletions WordPress/Classes/Services/ReaderSiteService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import Foundation
/// - failure: Closure called when the request fails.
func flagSite(withID id: NSNumber, asBlocked blocked: Bool, success: (() -> Void)? = nil, failure: ((Error?) -> Void)? = nil) {
let queryResult: (NSNumber, WordPressComRestApi)? = self.coreDataStack.performQuery({
guard
let defaultAccount = try? WPAccount.lookupDefaultWordPressComAccount(in: $0),
guard let defaultAccount = try? WPAccount.lookupDefaultWordPressComAccount(in: $0),
let api = defaultAccount.wordPressComRestApi,
api.hasCredentials()
else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extension ReaderTopicService {

private func apiRequest() -> WordPressComRestApi {
let api = coreDataStack.performQuery { context in
try? WPAccount.lookupDefaultWordPressComAccount(in: context)?.wordPressComRestApi
try? WPAccount.defaultWordPressComAccountRestAPI(in: context)
}
if let api, api.hasCredentials() {
return api
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Services/SiteSuggestionService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class SiteSuggestionService {
// add this blog to currently being requested list
blogsCurrentlyBeingRequested.append(blogId)

defaultAccount()?.wordPressComRestApi.GET(suggestPath, parameters: params, success: { [weak self] responseObject, httpResponse in
defaultAccount()?.wordPressComRestApi?.GET(suggestPath, parameters: params, success: { [weak self] responseObject, httpResponse in
guard let `self` = self else { return }

let context = ContextManager.shared.mainContext
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Services/SuggestionService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class SuggestionService {
// add this blog to currently being requested list
blogsCurrentlyBeingRequested.append(blogId)

defaultAccount()?.wordPressComRestApi.GET(suggestPath, parameters: params, success: { [weak self] responseObject, httpResponse in
defaultAccount()?.wordPressComRestApi?.GET(suggestPath, parameters: params, success: { [weak self] responseObject, httpResponse in
guard let `self` = self else { return }
guard let payload = responseObject as? [String: Any] else { return }
guard let restSuggestions = payload["suggestions"] as? [[String: Any]] else { return }
Expand Down
5 changes: 0 additions & 5 deletions WordPress/WordPressTest/MediaServiceUpdateTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@
@import WordPressKit;
@import OCMock;

// Redefine `WPAccount` to make `wordPressComRestApi` writable.
@interface WPAccount ()
@property (nonatomic, readwrite) WordPressComRestApi *wordPressComRestApi;
@end

// Re-implement `MediaService` to mock the remote service `MediaServiceRemote`.
@interface MediaServiceForStubbing : MediaService
@property (nonatomic, strong) MediaServiceRemoteREST *remoteForStubbing;
Expand Down
4 changes: 0 additions & 4 deletions WordPress/WordPressTest/MenusServiceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@

@import OCMock;

@interface WPAccount ()
@property (nonatomic, readwrite) WordPressComRestApi *wordPressComRestApi;
@end

@interface MenusServiceTests : XCTestCase
@property (nonatomic, strong) id<CoreDataStack> manager;
@end
Expand Down
4 changes: 0 additions & 4 deletions WordPress/WordPressTest/PostCategoryServiceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@
@import WordPressKit;
@import OCMock;

@interface WPAccount ()
@property (nonatomic, readwrite) WordPressComRestApi *wordPressComRestApi;
@end

@interface PostCategoryServiceForStubbing : PostCategoryService

@property (nonatomic, strong) TaxonomyServiceRemoteREST *remoteForStubbing;
Expand Down
4 changes: 0 additions & 4 deletions WordPress/WordPressTest/PostTagServiceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ @interface PostTagServiceForStubbing : PostTagService

@end

@interface WPAccount ()
@property (nonatomic, readwrite) WordPressComRestApi *wordPressComRestApi;
@end

@implementation PostTagServiceForStubbing

- (id <TaxonomyServiceRemote>)remoteForBlog:(Blog *)blog
Expand Down
4 changes: 0 additions & 4 deletions WordPress/WordPressTest/ThemeServiceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@

#pragma mark - Support classes

@interface WPAccount ()
@property (nonatomic, readwrite) WordPressComRestApi *wordPressComRestApi;
@end

#pragma mark - Tests

@interface ThemeServiceTests : XCTestCase
Expand Down
9 changes: 9 additions & 0 deletions WordPress/WordPressTest/WPAccount+LookupTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,15 @@ class WPAccountLookupTests: CoreDataTestCase {
try XCTAssertEqual(WPAccount.lookupNumberOfAccounts(in: contextManager.mainContext), 3)
}

// MARK: - Default account WordPress.com REST API

func testNoAPIWhenNoDefaultAccount() throws {
makeAccount()
try XCTAssertNil(WPAccount.defaultWordPressComAccountRestAPI(in: contextManager.mainContext))
}

// MARK: -

@discardableResult
func makeAccount(_ additionalSetup: (AccountBuilder) -> (AccountBuilder) = { $0 }) -> WPAccount {
additionalSetup(AccountBuilder(contextManager.mainContext)).build()
Expand Down