Refactor AccountService to use CoreDataStack

[crazytonyli]

Issue

Currently we have a lot of service types that inherit from LocalCoreDataService, which uses aNSManagedObjectContext. The idea is the service class uses the provided context object to perform all the Core Data operations, including read and write. This approach introduces two potential issues:

1. The mainContext—which should be used for reading only—may be used to write changes to database, since the service class simply uses the provided context object without checking.
2. In some implementation of the service type (i.e. this createOrUpdateAccount function), there is no guarantee the context object is used in the correct thread—the caller needs to make sure of it which isn't ideal.

Proposal

To resolve these potential issues, this PR refactor the service types in two ways, which will be applied across all other service types in future PRs.

1. Replacing LocalCoreDataService with CoreDataService, which takes a CoreDataStack instance.
2. The service implementation needs to guarantee that context objects are used in their correct thread by writing coreDataStack.performAndSave and read using coreDataStack.mainContext.performBlock. By doing this, the service type should be thread-safe and can be called from any thread—one less thing for the callers to worry about.

There is also an "implicit pattern", which is hard to enforce syntactically.

For functions in a service type that take a NSManagedObject argument, they should not assume what context the object belongs to (re-querying it using context.existingObject(with:) if needed, typically when updating them). Hence, it's preferred to take NSManagedObjectID as arugments, instead of NSManagedObject.

In a similar way, functions in a service type should not return NSManagedObject instances (unless the function is absolutely sure it's called from a main thread and returns objects of the mainContext), they should return NSManagedObjectID instead.

Note
Since this is quite a large PR, I've carefully split the changes into individually reviewable commit. Most of the commits consist of changes in one single AccountService function refactor and their callers. So, I'd recommend reviewing this PR commit by commit.

Test instructions

The changed files should indicate the affected areas, it's worth doing a bit regression testing on these areas. But I think a quick smoke testing on the app would be good too.

Regression Notes

1. Potential unintended areas of impact
   None.

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   None.

3. What automated tests I added (or what prevented me from doing so)
   None.

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[peril-wordpress-mobile]

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

[wpmobilebot]

You can test the changes in Jetpack from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19893-3d5078d on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[wpmobilebot]

You can test the changes in WordPress from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19893-3d5078d on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[crazytonyli]

LoginTests.testWPcomLoginLogout failed on CI. I can't reproduce this failure on the app, but I did find out the cause by running the UI test. I'll write down the cause for now and figure out the fix later.

The UI test is quite simple, log in using a WP.com account and then log out, the test expects the app to end up at login prologue screen (the one with "Log in or sign up with WP.com" button). But the actual behavior is, after the "Log out" alert button is tapped, the app presents the "Log in" modal. See the recording below:

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-01-12.at.14.50.02.mp4

Here are some key events that happened in the UI test to cause the failure:

1. The "Log out" alert button is tapped, AccountHelper calls AccountService.removeDefaultWordPressComAccount.
2. The current account from a background context, which is a change part of this PR. Originally, the account object is deleted from the mainContext.
3. The Core Data framework automatically merges the above changes into the mainContext. BlogDetailsViewController.handleDataModelChange since it's registered to respond to the mainContext changes notification.
4. Apparently, there are more than just the account deletion change, whole bunch of other objects are also changed in the mainContext, including the Blog instance. BlogDetailViewController notices the Blog object has changed, then start to reload it's content.
5. Within this reloading, WPAccount.wordPressComRestApi is eventually called. But there is no token in the account (Core Data probably set the authToken property to nil during the merging changes step mentioned above), which causes the "Log In" screen to be presented.

[mokagio]

Bumped the milestone from 21.7 to 21.8 because I'm about to start the 21.7 code freeze and this has merge conflicts.

Happy to help with cherry-picking into 21.7 if necessary later on.

[jkmassel]

The feedback I left has been well-addressed and I couldn't find anything else to suggest!

[jkmassel]

I guess some day we'll probably want a throws-ing variant of performAndSave 😅

[crazytonyli]

A while ago, I tried to get the current one to rethrow, but I don't think Swift compiler can handle an "escaped" rethrow block. We probably will have to have a performAndSaveThrowing variant, instead of sharing the reusing the same function "performAndSave" name.