From 15f930e6fcf5dacb33880289f580a25d2b34a656 Mon Sep 17 00:00:00 2001 From: kean Date: Mon, 1 Apr 2024 13:36:38 -0400 Subject: [PATCH] Add new sync engine for draft posts --- .../Classes/Models/AbstractPost+HashHelpers.h | 5 + .../Classes/Models/AbstractPost+Local.swift | 2 + ...actPost+MarkAsFailedAndDraftIfNeeded.swift | 1 + WordPress/Classes/Models/AbstractPost.h | 14 +- WordPress/Classes/Models/AbstractPost.m | 35 +- WordPress/Classes/Models/AbstractPost.swift | 69 +++ WordPress/Classes/Models/Post.swift | 66 +-- .../Classes/Services/PostCoordinator.swift | 338 +++++++++++++- WordPress/Classes/Services/PostHelper.h | 1 + WordPress/Classes/Services/PostHelper.m | 8 + .../Classes/Services/PostRepository.swift | 95 +++- .../Classes/System/WordPressAppDelegate.swift | 18 +- .../AztecPostViewController.swift | 24 +- .../EditHomepageViewController.swift | 2 +- .../GutenbergViewController+MoreActions.swift | 26 +- .../Gutenberg/GutenbergViewController.swift | 2 +- .../Post/AbstractPostListViewController.swift | 1 - .../Post/EditPostViewController.swift | 9 +- .../Post/PostCardStatusViewModel.swift | 32 +- .../ViewRelated/Post/PostEditor+Publish.swift | 301 +++++++++++-- .../ViewRelated/Post/PostEditorState.swift | 4 + .../Post/PostListEditorPresenter.swift | 17 +- .../ViewRelated/Post/PostListHeaderView.swift | 7 +- .../Post/PostListViewController.swift | 2 +- .../PostSettingsViewController+Swift.swift | 18 +- .../Post/PostSyncStateViewModel.swift | 61 ++- WordPress/WordPress.xcodeproj/project.pbxproj | 4 + .../WordPressTest/AbstractPostTest.swift | 38 ++ .../MediaImageServiceTests.swift | 6 +- .../PostCompactCellGhostableTests.swift | 24 - .../WordPressTest/PostCompactCellTests.swift | 31 -- .../PostCoordinatorSyncTests.swift | 421 ++++++++++++++++++ .../PostRepositorySaveTests.swift | 321 ++++++++++++- .../PostSyncStateViewModelTests.swift | 8 +- .../Views/PostCardStatusViewModelTests.swift | 4 +- 35 files changed, 1712 insertions(+), 303 deletions(-) create mode 100644 WordPress/WordPressTest/PostCoordinatorSyncTests.swift diff --git a/WordPress/Classes/Models/AbstractPost+HashHelpers.h b/WordPress/Classes/Models/AbstractPost+HashHelpers.h index 7a4f1b24a42b..153fb52b8ff0 100644 --- a/WordPress/Classes/Models/AbstractPost+HashHelpers.h +++ b/WordPress/Classes/Models/AbstractPost+HashHelpers.h @@ -7,14 +7,19 @@ NS_ASSUME_NONNULL_BEGIN // This value is used in Offline Posting — to calculate whether the post that the user _wanted_ to publish, // hasn't changed in the meantime and still is the same post. // It works by calculating a SHA256 hash for a subset of properties of a Post and then combining them together (including the hashes returned by the `additionalContentHashes` method, to let subclasses provide additional sources of truthfulness). +/// - note: deprecated (kahu-offline-mode) - (NSString *)calculateConfirmedChangesContentHash; // This is an extension point for the subclasses to add additional sources of truthfulness. +/// - note: deprecated (kahu-offline-mode) - (NSArray *)additionalContentHashes; +/// - note: deprecated (kahu-offline-mode) - (NSData *)hashForString:(NSString *)string; +/// - note: deprecated (kahu-offline-mode) - (NSData *)hashForNSInteger:(NSInteger)integer; +/// - note: deprecated (kahu-offline-mode) - (NSData *)hashForDouble:(double)dbl; @end diff --git a/WordPress/Classes/Models/AbstractPost+Local.swift b/WordPress/Classes/Models/AbstractPost+Local.swift index 5736b7d512cf..789c15c0c3f0 100644 --- a/WordPress/Classes/Models/AbstractPost+Local.swift +++ b/WordPress/Classes/Models/AbstractPost+Local.swift @@ -2,10 +2,12 @@ import Foundation extension AbstractPost { /// Returns true if the post is a draft and has never been uploaded to the server. + /// - note: deprecated (kahu-offline-mode) var isLocalDraft: Bool { return self.isDraft() && !self.hasRemote() } + /// - warning: deprecated (kahu-offline-mode) var isLocalRevision: Bool { return self.originalIsDraft() && self.isRevision() && self.remoteStatus == .local } diff --git a/WordPress/Classes/Models/AbstractPost+MarkAsFailedAndDraftIfNeeded.swift b/WordPress/Classes/Models/AbstractPost+MarkAsFailedAndDraftIfNeeded.swift index fced068bf6e9..bf33636de3b0 100644 --- a/WordPress/Classes/Models/AbstractPost+MarkAsFailedAndDraftIfNeeded.swift +++ b/WordPress/Classes/Models/AbstractPost+MarkAsFailedAndDraftIfNeeded.swift @@ -21,6 +21,7 @@ /// eventually be made private. /// - SeeAlso: PostCoordinator.resume /// + /// - note: deprecated (kahu-offline-mode) func markAsFailedAndDraftIfNeeded() { guard self.remoteStatus != .failed else { return diff --git a/WordPress/Classes/Models/AbstractPost.h b/WordPress/Classes/Models/AbstractPost.h index 571a7c62181a..1aa0002f7cbf 100644 --- a/WordPress/Classes/Models/AbstractPost.h +++ b/WordPress/Classes/Models/AbstractPost.h @@ -54,12 +54,19 @@ typedef NS_ENUM(NSUInteger, AbstractPostRemoteStatus) { @property (nonatomic, copy, nullable) NSDate *autosaveModifiedDate; @property (nonatomic, copy, nullable) NSNumber *autosaveIdentifier; +@property (nonatomic, strong, nullable) NSString *confirmedChangesHash; +@property (nonatomic, strong, nullable) NSDate *confirmedChangesTimestamp; + // Revision management - (AbstractPost *)createRevision; +/// A new version of `createRevision` that allows you to create revisions based +/// on other revisions. +/// +/// - warning: Work-in-progress (kahu-offline-mode) +- (AbstractPost *)_createRevision; - (void)deleteRevision; - (void)applyRevision; - (AbstractPost *)updatePostFrom:(AbstractPost *)revision; -- (void)updateRevision; - (BOOL)isRevision; - (BOOL)isOriginal; @@ -73,7 +80,7 @@ typedef NS_ENUM(NSUInteger, AbstractPostRemoteStatus) { - (BOOL)hasCategories; - (BOOL)hasTags; -/// True if either the post failed to upload, or the post has media that failed to upload. +/// - note: deprecated (kahu-offline-mode) @property (nonatomic, assign, readonly) BOOL isFailed; @property (nonatomic, assign, readonly) BOOL hasFailedMedia; @@ -86,7 +93,9 @@ typedef NS_ENUM(NSUInteger, AbstractPostRemoteStatus) { - (BOOL)hasRevision; #pragma mark - Conveniece Methods +/// - note: deprecated (kahu-offline-mode) - (void)publishImmediately; +/// - note: deprecated (kahu-offline-mode) - (BOOL)shouldPublishImmediately; - (NSString *)authorNameForDisplay; - (NSString *)blavatarForDisplay; @@ -176,6 +185,7 @@ typedef NS_ENUM(NSUInteger, AbstractPostRemoteStatus) { * * @returns YES if there ever was an attempt to upload this post, NO otherwise. */ +/// - warning: deprecated (kahu-offline-mode) - (BOOL)hasNeverAttemptedToUpload; /** diff --git a/WordPress/Classes/Models/AbstractPost.m b/WordPress/Classes/Models/AbstractPost.m index a17a917c4ab2..002564ea1d6e 100644 --- a/WordPress/Classes/Models/AbstractPost.m +++ b/WordPress/Classes/Models/AbstractPost.m @@ -5,21 +5,6 @@ #import "BasePost.h" @import WordPressKit; -@interface AbstractPost () - -/** - The following pair of properties is used to confirm that the post we'll be trying to automatically retry uploading, - hasn't changed since user has tapped on "confirm", and that we're not suddenly trying to auto-upload a post that the user - might have already forgotten about. - - The public-facing counterparts of those is the `shouldAttemptAutoUpload` property. - */ - -@property (nonatomic, strong, nullable) NSString *confirmedChangesHash; -@property (nonatomic, strong, nullable) NSDate *confirmedChangesTimestamp; - -@end - @implementation AbstractPost @dynamic blog; @@ -151,8 +136,13 @@ - (AbstractPost *)createRevision return self.revision; } + return [self _createRevision]; +} + +- (AbstractPost *)_createRevision { AbstractPost *post = [NSEntityDescription insertNewObjectForEntityForName:NSStringFromClass(self.class) inManagedObjectContext:self.managedObjectContext]; [post cloneFrom:self]; + post.isSyncNeeded = NO; [post setValue:self forKey:@"original"]; [post setValue:nil forKey:@"revision"]; post.isFeaturedImageChanged = self.isFeaturedImageChanged; @@ -193,14 +183,6 @@ - (AbstractPost *)updatePostFrom:(AbstractPost *)revision return self; } -- (void)updateRevision -{ - if ([self isRevision]) { - [self cloneFrom:self.original]; - self.isFeaturedImageChanged = self.original.isFeaturedImageChanged; - } -} - - (BOOL)isRevision { return (![self isOriginal]); @@ -213,11 +195,6 @@ - (BOOL)isOriginal - (AbstractPost *)latest { - // Even though we currently only support 1 revision per-post, we have plans to support multiple - // revisions in the future. That's the reason why we call `[[self revision] latest]` below. - // - // - Diego Rey Mendez, May 19, 2016 - // return [self hasRevision] ? [[self revision] latest] : self; } @@ -239,7 +216,6 @@ - (AbstractPost *)original return original; } - #pragma mark - Helpers - (BOOL)dateCreatedIsNilOrEqualToDateModified @@ -496,6 +472,7 @@ - (BOOL)isUploading #pragma mark - Post +/// - note: Deprecated (kahu-offline-mode) - (BOOL)canSave { NSString* titleWithoutSpaces = [self.postTitle stringByReplacingOccurrencesOfString:@" " withString:@""]; diff --git a/WordPress/Classes/Models/AbstractPost.swift b/WordPress/Classes/Models/AbstractPost.swift index cbd8e950d8d6..7e02b0b87846 100644 --- a/WordPress/Classes/Models/AbstractPost.swift +++ b/WordPress/Classes/Models/AbstractPost.swift @@ -6,8 +6,16 @@ extension AbstractPost { original?.original() ?? self } + /// Returns `true` if the post was never uploaded to the remote and has + /// not revisions that were marked for syncing. + var isNewDraft: Bool { + assert(isOriginal(), "Must be called on the original") + return !hasRemote() && getLatestRevisionNeedingSync() == nil + } + // MARK: - Status + /// - note: deprecated (kahu-offline-mode) @objc var statusTitle: String? { guard let status = self.status else { @@ -17,6 +25,7 @@ extension AbstractPost { return AbstractPost.title(for: status) } + /// - note: deprecated (kahu-offline-mode) @objc var remoteStatus: AbstractPostRemoteStatus { get { @@ -150,4 +159,64 @@ extension AbstractPost { func hasPermanentFailedMedia() -> Bool { return media.first(where: { !$0.willAttemptToUploadLater() }) != nil } + + /// Returns the changes made in the current revision compared to the + /// previous revision or the original post if there is only one revision. + var changes: RemotePostUpdateParameters { + guard let original else { + return RemotePostUpdateParameters() // Empty + } + return RemotePostUpdateParameters.changes(from: original, to: self) + } + + /// Returns all revisions of the post including the original one. + var allRevisions: [AbstractPost] { + var revisions: [AbstractPost] = [self] + var current = self + while let next = current.revision { + revisions.append(next) + current = next + } + return revisions + + } + + // TODO: Replace with a new flag + /// - note: Work-in-progress (kahu-offline-mode) + @objc var isSyncNeeded: Bool { + get { confirmedChangesHash == AbstractPost.syncNeededKey } + set { confirmedChangesHash = newValue ? AbstractPost.syncNeededKey : "" } + } + + static let syncNeededKey = "sync-needed" + + /// Returns the latest saved revisions that needs to be synced with the server. + /// Returns `nil` if there are no such revisions. + func getLatestRevisionNeedingSync() -> AbstractPost? { + assert(original == nil, "Must be called on an original revision") + let revision = allRevisions.last(where: \.isSyncNeeded) + guard revision != self else { + return nil + } + return revision + } + + /// Deletes all of the synced revisions until and including the `latest` + /// one passed as a parameter. + func deleteSyncedRevisions(until latest: AbstractPost) { + assert(original == nil, "Must be called on an original revision") + let tail = latest.revision + + var current = self + while current !== latest, let next = current.revision { + current.deleteRevision() + current = next + } + + if let tail { + willChangeValue(forKey: "revision") + setPrimitiveValue(tail, forKey: "revision") + didChangeValue(forKey: "revision") + } + } } diff --git a/WordPress/Classes/Models/Post.swift b/WordPress/Classes/Models/Post.swift index db9ad239ba6f..127982aeb18d 100644 --- a/WordPress/Classes/Models/Post.swift +++ b/WordPress/Classes/Models/Post.swift @@ -55,41 +55,12 @@ class Post: AbstractPost { } } - // MARK: - Properties - - fileprivate var storedContentPreviewForDisplay = "" - // MARK: - NSManagedObject override class func entityName() -> String { return "Post" } - override func awakeFromFetch() { - super.awakeFromFetch() - buildContentPreview() - } - - override func willSave() { - super.willSave() - - if isDeleted { - return - } - - storedContentPreviewForDisplay = "" - } - - // MARK: - Content Preview - - fileprivate func buildContentPreview() { - if let excerpt = mt_excerpt, excerpt.count > 0 { - storedContentPreviewForDisplay = excerpt.makePlainText() - } else if let content = content { - storedContentPreviewForDisplay = content.summarized() - } - } - // MARK: - Format @objc func postFormatText() -> String? { @@ -301,11 +272,23 @@ class Post: AbstractPost { // MARK: - BasePost override func contentPreviewForDisplay() -> String { - if storedContentPreviewForDisplay.count == 0 { - buildContentPreview() + if let excerpt = mt_excerpt, excerpt.count > 0 { + if let preview = PostPreviewCache.shared.excerpt[excerpt] { + return preview + } + let preview = excerpt.makePlainText() + PostPreviewCache.shared.excerpt[excerpt] = preview + return preview + } else if let content = content { + if let preview = PostPreviewCache.shared.content[content] { + return preview + } + let preview = content.summarized() + PostPreviewCache.shared.content[content] = preview + return preview + } else { + return "" } - - return storedContentPreviewForDisplay } override func hasLocalChanges() -> Bool { @@ -387,3 +370,20 @@ class Post: AbstractPost { hash(for: isStickyPost ? 1 : 0)] } } + +private final class PostPreviewCache { + static let shared = PostPreviewCache() + + let excerpt = Cache() + let content = Cache() +} + +private final class Cache { + private let lock = NSLock() + private var dictionary: [Key: Value] = [:] + + subscript(key: Key) -> Value? { + get { lock.withLock { dictionary[key] } } + set { lock.withLock { dictionary[key] = newValue } } + } +} diff --git a/WordPress/Classes/Services/PostCoordinator.swift b/WordPress/Classes/Services/PostCoordinator.swift index d3b63cb708af..12b79c6e81b4 100644 --- a/WordPress/Classes/Services/PostCoordinator.swift +++ b/WordPress/Classes/Services/PostCoordinator.swift @@ -2,6 +2,8 @@ import Aztec import Foundation import WordPressKit import WordPressFlux +import CocoaLumberjack +import Combine protocol PostCoordinatorDelegate: AnyObject { func postCoordinator(_ postCoordinator: PostCoordinator, promptForPasswordForBlog blog: Blog) @@ -16,6 +18,9 @@ class PostCoordinator: NSObject { @objc static let shared = PostCoordinator() + /// Events about the sync status changes. + let syncEvents = PassthroughSubject() + private let coreDataStack: CoreDataStackSwift private var mainContext: NSManagedObjectContext { @@ -26,8 +31,8 @@ class PostCoordinator: NSObject { private let queue = DispatchQueue(label: "org.wordpress.postcoordinator") + private var workers: [NSManagedObjectID: SyncWorker] = [:] private var pendingPostIDs: Set = [] - private var pendingDeletionPostIDs: Set = [] private var observerUUIDs: [AbstractPost: UUID] = [:] @@ -39,6 +44,9 @@ class PostCoordinator: NSObject { private let actionDispatcherFacade: ActionDispatcherFacade private let isSyncPublishingEnabled: Bool + /// The initial sync retry delay. By default, 5 seconds. + var syncRetryDelay: TimeInterval = 5 + // MARK: - Initializers init(mainService: PostService? = nil, @@ -57,11 +65,19 @@ class PostCoordinator: NSObject { self.actionDispatcherFacade = actionDispatcherFacade self.isSyncPublishingEnabled = isSyncPublishingEnabled + + super.init() + + if isSyncPublishingEnabled { + NotificationCenter.default.addObserver(self, selector: #selector(didUpdateReachability), name: .reachabilityChanged, object: nil) + } } /// Upload or update a post in the server. /// /// - Parameter forceDraftIfCreating Please see `PostService.uploadPost:forceDraftIfCreating`. + /// + /// - note: deprecated (kahu-offline-mode) func save(_ postToSave: AbstractPost, automatedRetry: Bool = false, forceDraftIfCreating: Bool = false, @@ -160,7 +176,7 @@ class PostCoordinator: NSObject { /// /// - warning: Work-in-progress (kahu-offline-mode) @discardableResult @MainActor - func _update(_ post: AbstractPost, changes: RemotePostUpdateParameters? = nil) async throws -> AbstractPost { + func _save(_ post: AbstractPost, changes: RemotePostUpdateParameters? = nil) async throws -> AbstractPost { let post = post.original ?? post do { let isExistingPost = post.hasRemote() @@ -174,6 +190,20 @@ class PostCoordinator: NSObject { } } + /// Patches the post but keeps the local revision if any. + /// + /// - warning: Work-in-progress (kahu-offline-mode) + @MainActor + func _update(_ post: AbstractPost, changes: RemotePostUpdateParameters) async throws { + let post = post.original ?? post + do { + try await PostRepository(coreDataStack: coreDataStack)._update(post, changes: changes) + } catch { + handleError(error, for: post) + throw error + } + } + private func handleError(_ error: Error, for post: AbstractPost) { guard let topViewController = UIApplication.shared.mainWindow?.topmostPresentedViewController else { return @@ -234,6 +264,270 @@ class PostCoordinator: NSObject { } } + // MARK: - Sync + + /// Returns `true` if post has any revisions that need to be synced. + static func isSyncNeeded(for post: AbstractPost) -> Bool { + post.original().getLatestRevisionNeedingSync() != nil + } + + /// Sets a flag to sync the given revision and schedules the next sync. + /// + /// - warning: Should only be used for draft posts. + /// + /// - warning: Work-in-progress (kahu-offline-mode) + func setNeedsSync(for revision: AbstractPost) { + assert(revision.isRevision(), "Must be used only on revisions") + assert(revision.original().status == .draft, "Must be used only with draft posts") + + if !revision.isSyncNeeded { + revision.isSyncNeeded = true + ContextManager.shared.saveContextAndWait(coreDataStack.mainContext) + } + startSync(for: revision.original()) + } + + /// Schedules sync for all the posts with revisions that need syncing. + /// + /// - note: It should typically only be called once during the app launch. + func scheduleSync() { + let request = NSFetchRequest(entityName: NSStringFromClass(AbstractPost.self)) + request.predicate = NSPredicate(format: "confirmedChangesHash == %@", AbstractPost.syncNeededKey) + do { + let revisions = try coreDataStack.mainContext.fetch(request) + let originals = Set(revisions.map { $0.original() }) + for post in originals { + startSync(for: post) + } + } catch { + DDLogError("failed to scheduled sync: \(error)") + } + } + + /// A manages sync for the given post. Every post has its own worker. + private final class SyncWorker { + let post: AbstractPost + var operation: SyncOperation? // The sync operation that's currently running + var error: Error? // The previous sync error + + var nextRetryDelay: TimeInterval { + retryDelay = min(30, retryDelay * 1.5) + return retryDelay + } + var retryDelay: TimeInterval + weak var retryTimer: Timer? + + deinit { + self.log("deinit") + } + + init(post: AbstractPost, retryDelay: TimeInterval) { + self.post = post + self.retryDelay = retryDelay + self.log("created for \"\(post.postTitle ?? "–")\"") + } + + func log(_ string: String) { + DDLogInfo("sync-worker(\(post.objectID.shortDescription)) \(string)") + } + } + + /// An operation for syncing post to the given local revision. + final class SyncOperation { + let id: Int + let post: AbstractPost + let revision: AbstractPost + var state: State = .uploadingMedia // The first step is always media + var isCancelled = false + + enum State { + case uploadingMedia + case syncing + case finished(Result) + } + + private static var nextId = 1 + + init(post: AbstractPost, revision: AbstractPost) { + self.post = post + self.revision = revision + self.id = SyncOperation.nextId + SyncOperation.nextId += 1 + self.log("created for \"\(post.postTitle ?? "–")\"") + } + + func log(_ string: String) { + DDLogInfo("sync-operation(\(id)) (\(post.objectID.shortDescription)→\(revision.objectID.shortDescription))) \(string)") + } + } + + enum SyncEvent { + /// A sync worker started a sync operation. + case started(operation: SyncOperation) + /// A sync worker finished a sync operation. + /// + /// - warning: By the time operation completes, the revision will be deleted. + case finished(operation: SyncOperation, result: Result) + } + + private func startSync(for post: AbstractPost) { + guard let revision = post.getLatestRevisionNeedingSync() else { + return DDLogInfo("sync: \(post.objectID.shortDescription) is already up to date") + } + startSync(for: post, revision: revision) + } + + private func startSync(for post: AbstractPost, revision: AbstractPost) { + assert(Thread.isMainThread) + assert(post.isOriginal()) + assert(!post.objectID.isTemporaryID) + + let worker = getWorker(for: post) + if let operation = worker.operation { + guard operation.revision != revision else { + return worker.log("already syncing to the latest revision") + } + tryCancelSyncOperation(operation, latest: revision) + guard operation.isCancelled else { + return worker.log("waiting until the current operation finishes") + } + } else { + worker.retryTimer?.invalidate() + } + + let operation = SyncOperation(post: post, revision: revision) + worker.operation = operation + startSyncOperation(operation) + syncEvents.send(.started(operation: operation)) + } + + private func getWorker(for post: AbstractPost) -> SyncWorker { + let worker = workers[post.objectID] ?? SyncWorker(post: post, retryDelay: syncRetryDelay) + workers[post.objectID] = worker + return worker + } + + /// Try to cancel the current sync operation, which is not always possible. + private func tryCancelSyncOperation(_ operation: SyncOperation, latest: AbstractPost) { + // If there is a current sync operation running and it doesn't target + // the latest revision, then try to cancel it. + switch operation.state { + case .uploadingMedia: + // Cancel the upload for media, but keep the tasks scheduled + // for the media still present in the revision. + let deleted = Set(operation.revision.media).subtracting(Set(latest.media)) + for media in deleted { + mediaCoordinator.cancelUpload(of: media) + } + if !deleted.isEmpty { + operation.log("cancel upload for deleted media: \(deleted.map(\.filename))") + } + operation.log("cancel media upload") + operation.isCancelled = true + syncOperation(operation, didFinishWithResult: .failure(CancellationError())) // Finish immediatelly + case .syncing: + break // There is no way to safely cancel an in-flight request + case .finished: + break // This should never happen + } + } + + private func startSyncOperation(_ operation: SyncOperation) { + Task { @MainActor in + do { + operation.log("upload remaining media") + try await uploadRemainingResources(for: operation.revision) + guard !operation.isCancelled else { return } + operation.log("sync post contents and settings") + operation.state = .syncing + try await PostRepository(coreDataStack: coreDataStack) + .sync(operation.post, revision: operation.revision) + syncOperation(operation, didFinishWithResult: .success(())) + } catch { + syncOperation(operation, didFinishWithResult: .failure(error)) + } + } + } + + private func syncOperation(_ operation: SyncOperation, didFinishWithResult result: Result) { + operation.log("finished with result: \(result)") + operation.state = .finished(result) + defer { syncEvents.send(.finished(operation: operation, result: result)) } + + guard !operation.isCancelled else { return } + + let worker = getWorker(for: operation.post) + worker.operation = nil + + switch result { + case .success: + worker.retryDelay = syncRetryDelay + worker.error = nil + postDidUpdateNotification(for: operation.post) // TODO: Use syncEvents + + if let revision = operation.post.getLatestRevisionNeedingSync() { + operation.log("more revisions need syncing: \(revision.objectID.shortDescription)") + startSync(for: operation.post, revision: revision) + } else { + workers[operation.post.objectID] = nil + } + case .failure(let error): + worker.error = error + postDidUpdateNotification(for: operation.post) + + if let error = error as? PostRepository.PostSaveError, case .deleted = error { + operation.log("post was permanently deleted") + handlePermanentlyDeleted(operation.post) + workers[operation.post.objectID] = nil + } else { + let delay = worker.nextRetryDelay + worker.retryTimer = Timer.scheduledTimer(withTimeInterval: delay, repeats: false) { [weak self, weak worker] _ in + guard let self, let worker else { return } + self.didRetryTimerFire(for: worker) + } + worker.log("scheduled retry with delay: \(delay)s.") + } + } + } + + private func didRetryTimerFire(for worker: SyncWorker) { + worker.log("retry timer fired") + startSync(for: worker.post) + } + + @objc private func didUpdateReachability(_ notification: Foundation.Notification) { + guard let reachable = notification.userInfo?[Foundation.Notification.reachabilityKey], + (reachable as? Bool) == true else { + return + } + for worker in workers.values { + if let error = worker.error, + let urlError = (error as NSError).underlyingErrors.first as? URLError, + urlError.code == .notConnectedToInternet { + worker.log("connection is reachable – retrying now") + startSync(for: worker.post) + } + } + } + + func syncError(for post: AbstractPost) -> Error? { + assert(post.isOriginal()) + return workers[post.objectID]?.error + } + + private func postDidUpdateNotification(for post: AbstractPost) { + NotificationCenter.default.post(name: .postCoordinatorDidUpdate, object: self, userInfo: [NSUpdatedObjectsKey: Set([post])]) + } + + // MARK: - Upload Resources + + @MainActor + private func uploadRemainingResources(for post: AbstractPost) async throws { + _ = try await withUnsafeThrowingContinuation { continuation in + self.prepareToSave(post, then: continuation.resume(with:)) + } + } + /// If media is still uploading it keeps track of the ongoing media operations and updates the post content when they finish. /// Then, it calls the completion block with the post ready to be saved/uploaded. /// @@ -254,7 +548,14 @@ class PostCoordinator: NSObject { change(post: post, status: .pushing) - if mediaCoordinator.isUploadingMedia(for: post) || post.hasFailedMedia { + let hasPendingMedia: Bool + if isSyncPublishingEnabled { + hasPendingMedia = post.media.contains { $0.remoteStatus != .sync } + } else { + hasPendingMedia = mediaCoordinator.isUploadingMedia(for: post) || post.hasFailedMedia + } + + if hasPendingMedia { change(post: post, status: .pushingMedia) // Only observe if we're not already guard !isObserving(post: post) else { @@ -349,7 +650,9 @@ class PostCoordinator: NSObject { /// This method checks the status of all post objects and updates them to the correct status if needed. /// The main cause of wrong status is the app being killed while uploads of posts are happening. /// + /// - note: deprecated (kahu-offline-mode) @objc func refreshPostStatus() { + guard !isSyncPublishingEnabled else { return } Post.refreshStatus(with: coreDataStack) } @@ -418,10 +721,17 @@ class PostCoordinator: NSObject { case .ended: let successHandler = { self.updateReferences(to: media, in: post) - // Let's check if media uploading is still going, if all finished with success then we can upload the post - if !self.mediaCoordinator.isUploadingMedia(for: post) && !post.hasFailedMedia { - self.removeObserver(for: post) - completion(.success(post)) + if self.isSyncPublishingEnabled { + if post.media.allSatisfy({ $0.remoteStatus == .sync }) { + self.removeObserver(for: post) + completion(.success(post)) + } + } else { + // Let's check if media uploading is still going, if all finished with success then we can upload the post + if !self.mediaCoordinator.isUploadingMedia(for: post) && !post.hasFailedMedia { + self.removeObserver(for: post) + completion(.success(post)) + } } } switch media.mediaType { @@ -699,7 +1009,7 @@ private struct Constants { } extension Foundation.Notification.Name { - /// Contains a set of updated objects under the `NSUpdatedObjectsKey` key + /// Contains a set of updated objects under the `NSUpdatedObjectsKey` key. static let postCoordinatorDidUpdate = Foundation.Notification.Name("org.automattic.postCoordinatorDidUpdate") } @@ -707,6 +1017,9 @@ extension Foundation.Notification.Name { extension PostCoordinator: Uploader { func resume() { + guard isSyncPublishingEnabled else { + return + } failedPostsFetcher.postsAndRetryActions { [weak self] postsAndActions in guard let self = self else { return @@ -764,6 +1077,15 @@ extension PostCoordinator { } } +private extension NSManagedObjectID { + var shortDescription: String { + let description = "\(self)" + guard let index = description.lastIndex(of: "/") else { return description } + return String(description.suffix(from: index)) + .trimmingCharacters(in: CharacterSet(charactersIn: "/>")) + } +} + private enum Strings { static let movePostToTrash = NSLocalizedString("postsList.movePostToTrash.message", value: "Post moved to trash", comment: "A short message explaining that a post was moved to the trash bin.") static let deletePost = NSLocalizedString("postsList.deletePost.message", value: "Post deleted permanently", comment: "A short message explaining that a post was deleted permanently.") diff --git a/WordPress/Classes/Services/PostHelper.h b/WordPress/Classes/Services/PostHelper.h index f6d5b107fb69..e4f0fd82030f 100644 --- a/WordPress/Classes/Services/PostHelper.h +++ b/WordPress/Classes/Services/PostHelper.h @@ -9,6 +9,7 @@ NS_ASSUME_NONNULL_BEGIN @interface PostHelper: NSObject + (void)updatePost:(AbstractPost *)post withRemotePost:(RemotePost *)remotePost inContext:(NSManagedObjectContext *)managedObjectContext; ++ (void)updatePost:(AbstractPost *)post withRemotePost:(RemotePost *)remotePost inContext:(NSManagedObjectContext *)managedObjectContext overwrite:(BOOL)overwrite; /** Creates a RemotePost from an AbstractPost to be used for API calls. diff --git a/WordPress/Classes/Services/PostHelper.m b/WordPress/Classes/Services/PostHelper.m index ed932609ea3e..01e48c12e37f 100644 --- a/WordPress/Classes/Services/PostHelper.m +++ b/WordPress/Classes/Services/PostHelper.m @@ -8,6 +8,14 @@ @implementation PostHelper + (void)updatePost:(AbstractPost *)post withRemotePost:(RemotePost *)remotePost inContext:(NSManagedObjectContext *)managedObjectContext { + [self updatePost:post withRemotePost:remotePost inContext:managedObjectContext overwrite:NO]; +} + ++ (void)updatePost:(AbstractPost *)post withRemotePost:(RemotePost *)remotePost inContext:(NSManagedObjectContext *)managedObjectContext overwrite:(BOOL)overwrite { + if ([RemoteFeature enabled:RemoteFeatureFlagSyncPublishing] && (post.revision != nil && !overwrite)) { + return; + } + NSNumber *previousPostID = post.postID; post.postID = remotePost.postID; // Used to populate author information for self-hosted sites. diff --git a/WordPress/Classes/Services/PostRepository.swift b/WordPress/Classes/Services/PostRepository.swift index a6f9e3ee64d9..41d822eb6f82 100644 --- a/WordPress/Classes/Services/PostRepository.swift +++ b/WordPress/Classes/Services/PostRepository.swift @@ -90,8 +90,7 @@ final class PostRepository { /// require special handling or an error from the underlying system. /// /// - parameters: - /// - post: The post to be synced. It doesn't matter whether you pass - /// the original version of the post or the revision. + /// - post: The post to be synced (original revision) . /// - changes: A set of (optional) changes to be applied on top of the post /// or its latest revision. /// - overwrite: Set to `true` to overwrite the values on the server and @@ -102,34 +101,91 @@ final class PostRepository { /// TODO: update to support XML-RPC (error code, etc) @MainActor func _save(_ post: AbstractPost, changes: RemotePostUpdateParameters? = nil, overwrite: Bool = false) async throws { - let original = post.original ?? post + try await _sync(post, revision: post.latest(), changes: changes, overwrite: overwrite) + } + + /// Syncs revisions that have unsaved changes (see `isSyncNeeded`). + /// + /// - note: This method is designed to be used with drafts. + /// + /// - warning: Work-in-progress (kahu-offline-mode) + @MainActor + func sync(_ post: AbstractPost, revision: AbstractPost? = nil) async throws { + assert(post.original == nil, "Must be called on an original post") + guard let revision = revision ?? post.getLatestRevisionNeedingSync() else { + return assertionFailure("Requires a revision") + } + try await _sync(post, revision: revision) + } - let uploadedPost: RemotePost + /// - parameter revision: The revision to upload (doesn't have to + /// be the latest revision). + @MainActor + private func _sync( + _ post: AbstractPost, + revision: AbstractPost, + changes: RemotePostUpdateParameters? = nil, + overwrite: Bool = false + ) async throws { + let post = post.original() // Defensive code + + let remotePost: RemotePost var isCreated = false - if let postID = original.postID, postID.intValue > 0 { - uploadedPost = try await _patch(post, postID: postID, changes: changes, overwrite: overwrite) + if let postID = post.postID, postID.intValue > 0 { + let changes = RemotePostUpdateParameters.changes(from: post, to: revision, with: changes) + if !changes.isEmpty { + remotePost = try await _patch(post, postID: postID, changes: changes, overwrite: overwrite) + } else { + remotePost = try await getRemoteService(for: post.blog).post(withID: postID) + } } else { isCreated = true - uploadedPost = try await _create(post, changes: changes) + remotePost = try await _create(revision, changes: changes) } let context = coreDataStack.mainContext - original.deleteRevision() - PostHelper.update(original, with: uploadedPost, in: context) + if revision.revision == nil { + // No more revisions were created during the upload, so it's safe + // to fully replace the local version with the remote data + PostHelper.update(post, with: remotePost, in: context, overwrite: true) + } else { + // We have to keep the local changes to make sure the delta can + // still be computed accurately (a smarter algo could merge the changes) + post.clone(from: revision) + post.postID = remotePost.postID // important! + } + + post.deleteSyncedRevisions(until: revision) + if isCreated { PostService(managedObjectContext: context) - .updateMediaFor(post: original, success: {}, failure: { _ in }) + .updateMediaFor(post: post, success: {}, failure: { _ in }) + } + ContextManager.shared.saveContextAndWait(context) + } + + /// Patches the post. + /// + /// - note: This method can be used for quick edits for published posts where + /// revisions are used only for content. + @MainActor + func _update(_ post: AbstractPost, changes: RemotePostUpdateParameters) async throws { + let original = post.original ?? post + guard let postID = original.postID, postID.intValue > 0 else { + assertionFailure("Trying to patch a non-existent post") + return } - /// - warning: Work-in-progress (kahu-offline-mode) - // TODO: Remove when it's no longer needed - original.remoteStatus = AbstractPostRemoteStatus.sync + let uploadedPost = try await _patch(post, postID: postID, changes: changes, overwrite: true) + + let context = coreDataStack.mainContext + PostHelper.update(original, with: uploadedPost, in: context, overwrite: true) ContextManager.shared.saveContextAndWait(context) } @MainActor private func _create(_ post: AbstractPost, changes: RemotePostUpdateParameters?) async throws -> RemotePost { let service = try getRemoteService(for: post.blog) - var parameters = RemotePostCreateParameters(post: post.latest()) + var parameters = RemotePostCreateParameters(post: post) if let changes { parameters.apply(changes) } @@ -137,13 +193,10 @@ final class PostRepository { } @MainActor - private func _patch(_ post: AbstractPost, postID: NSNumber, changes: RemotePostUpdateParameters?, overwrite: Bool) async throws -> RemotePost { + private func _patch(_ post: AbstractPost, postID: NSNumber, changes: RemotePostUpdateParameters, overwrite: Bool) async throws -> RemotePost { let service = try getRemoteService(for: post.blog) - let original = post.original ?? post - let latest = post.latest() - - // Create a diff between local revision and the target revision. - var changes = RemotePostUpdateParameters.changes(from: original, to: latest, with: changes) + let original = post.original() + var changes = changes // Make sure the app never overwrites the content without the user approval. if !overwrite, let date = original.dateModified, changes.content != nil { @@ -286,7 +339,7 @@ final class PostRepository { try? await coreDataStack.performAndSave { context in let post = try context.existingObject(with: postID) if let updatedRemotePost, updatedRemotePost.status != PostStatusDeleted { - PostHelper.update(post, with: updatedRemotePost, in: context) + PostHelper.update(post, with: updatedRemotePost, in: context, overwrite: true) post.latest().statusAfterSync = post.statusAfterSync post.latest().status = post.status } else { diff --git a/WordPress/Classes/System/WordPressAppDelegate.swift b/WordPress/Classes/System/WordPressAppDelegate.swift index a1923cc5a3cd..15d88d928897 100644 --- a/WordPress/Classes/System/WordPressAppDelegate.swift +++ b/WordPress/Classes/System/WordPressAppDelegate.swift @@ -328,6 +328,10 @@ class WordPressAppDelegate: UIResponder, UIApplicationDelegate { self?.uploadsManager.resume() } + DispatchQueue.main.asyncAfter(deadline: .now() + 3) { + PostCoordinator.shared.scheduleSync() + } + setupWordPressExtensions() shortcutCreator.createShortcutsIf3DTouchAvailable(AccountHelper.isLoggedIn) @@ -427,14 +431,16 @@ extension WordPressAppDelegate { return } - let wifi = reachability.isReachableViaWiFi() ? "Y" : "N" - let wwan = reachability.isReachableViaWWAN() ? "Y" : "N" + DispatchQueue.main.async { + let wifi = reachability.isReachableViaWiFi() ? "Y" : "N" + let wwan = reachability.isReachableViaWWAN() ? "Y" : "N" - DDLogInfo("Reachability - Internet - WiFi: \(wifi) WWAN: \(wwan)") - let newValue = reachability.isReachable() - self?.connectionAvailable = newValue + DDLogInfo("Reachability - Internet - WiFi: \(wifi) WWAN: \(wwan)") + let newValue = reachability.isReachable() + self?.connectionAvailable = newValue - NotificationCenter.default.post(name: .reachabilityChanged, object: self, userInfo: [Foundation.Notification.reachabilityKey: newValue]) + NotificationCenter.default.post(name: .reachabilityChanged, object: self, userInfo: [Foundation.Notification.reachabilityKey: newValue]) + } } internetReachability?.reachableBlock = reachabilityBlock diff --git a/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift b/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift index e67423b8d9c8..01991c4da2d0 100644 --- a/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift +++ b/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift @@ -1064,31 +1064,11 @@ extension AztecPostViewController: AztecNavigationControllerDelegate { // extension AztecPostViewController { @IBAction func publishButtonTapped(sender: UIButton) { - handlePublishButtonTap() + handlePrimaryActionButtonTap() } @IBAction func secondaryPublishButtonTapped() { - guard let action = self.postEditorStateContext.secondaryPublishButtonAction else { - // If the user tapped on the secondary publish action button, it means we should have a secondary publish action. - let error = NSError(domain: errorDomain, code: ErrorCode.expectedSecondaryAction.rawValue, userInfo: nil) - WordPressAppDelegate.crashLogging?.logError(error) - return - } - - let secondaryStat = self.postEditorStateContext.secondaryPublishActionAnalyticsStat - - let publishPostClosure = { [unowned self] in - self.publishPost( - action: action, - dismissWhenDone: action.dismissesEditor, - analyticsStat: secondaryStat) - } - - if presentedViewController != nil { - dismiss(animated: true, completion: publishPostClosure) - } else { - publishPostClosure() - } + handleSecondaryActionButtonTap() } @IBAction func closeWasPressed() { diff --git a/WordPress/Classes/ViewRelated/Gutenberg/EditHomepageViewController.swift b/WordPress/Classes/ViewRelated/Gutenberg/EditHomepageViewController.swift index e4eaf1a33bb7..49408d43389f 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/EditHomepageViewController.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/EditHomepageViewController.swift @@ -27,7 +27,7 @@ class EditHomepageViewController: GutenbergViewController { // If there are changes, offer to save them, otherwise continue will dismiss the editor with no changes. override func continueFromHomepageEditing() { if editorHasChanges { - handlePublishButtonTap() + handlePrimaryActionButtonTap() } else { cancelEditing() } diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController+MoreActions.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController+MoreActions.swift index 14e694ac4c9d..c9f61cbbbfac 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController+MoreActions.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController+MoreActions.swift @@ -27,7 +27,7 @@ extension GutenbergViewController { let buttonTitle = postEditorStateContext.secondaryPublishButtonText { alert.addDefaultActionWithTitle(buttonTitle) { _ in - self.secondaryPublishButtonTapped() + self.handleSecondaryActionButtonTap() ActionDispatcher.dispatch(NoticeAction.unlock) } } @@ -86,30 +86,6 @@ extension GutenbergViewController { present(alert, animated: true) } - - func secondaryPublishButtonTapped() { - guard let action = self.postEditorStateContext.secondaryPublishButtonAction else { - // If the user tapped on the secondary publish action button, it means we should have a secondary publish action. - let error = NSError(domain: errorDomain, code: ErrorCode.expectedSecondaryAction.rawValue, userInfo: nil) - WordPressAppDelegate.crashLogging?.logError(error) - return - } - - let secondaryStat = self.postEditorStateContext.secondaryPublishActionAnalyticsStat - - let publishPostClosure = { [unowned self] in - self.publishPost( - action: action, - dismissWhenDone: action.dismissesEditor, - analyticsStat: secondaryStat) - } - - if presentedViewController != nil { - dismiss(animated: true, completion: publishPostClosure) - } else { - publishPostClosure() - } - } } // MARK: - Constants diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift index a11f1394677c..4e862b511ebe 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift @@ -845,7 +845,7 @@ extension GutenbergViewController: GutenbergBridgeDelegate { switch reason { case .publish: if editorHasContent(title: title, content: html) { - handlePublishButtonTap() + handlePrimaryActionButtonTap() } else { showAlertForEmptyPostPublish() } diff --git a/WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift b/WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift index a20bcfe68e9e..b80c1f1e467f 100644 --- a/WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift +++ b/WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift @@ -646,7 +646,6 @@ class AbstractPostListViewController: UIViewController, @objc func moveToDraft(_ post: AbstractPost) { WPAnalytics.track(.postListDraftAction, withProperties: propertiesForAnalytics()) - PostCoordinator.shared.moveToDraft(post) } diff --git a/WordPress/Classes/ViewRelated/Post/EditPostViewController.swift b/WordPress/Classes/ViewRelated/Post/EditPostViewController.swift index eadbc57844bb..43d2039f76a1 100644 --- a/WordPress/Classes/ViewRelated/Post/EditPostViewController.swift +++ b/WordPress/Classes/ViewRelated/Post/EditPostViewController.swift @@ -27,6 +27,8 @@ class EditPostViewController: UIViewController { @objc var onClose: ((_ changesSaved: Bool) -> ())? @objc var afterDismiss: (() -> Void)? + private var originalPostID: NSManagedObjectID? + override var modalPresentationStyle: UIModalPresentationStyle { didSet(newValue) { // make sure this view is transparent with the previous VC visible @@ -69,6 +71,7 @@ class EditPostViewController: UIViewController { /// - Note: it's preferable to use one of the convenience initializers fileprivate init(post: Post?, blog: Blog, loadAutosaveRevision: Bool = false, prompt: BloggingPrompt? = nil) { self.post = post + self.originalPostID = (post?.original ?? post)?.objectID self.loadAutosaveRevision = loadAutosaveRevision if let post = post { if !post.originalIsDraft() { @@ -128,9 +131,9 @@ class EditPostViewController: UIViewController { @objc private func didChangeObjects(_ notification: Foundation.Notification) { guard let userInfo = notification.userInfo else { return } - let deletedPosts = ((userInfo[NSDeletedObjectsKey] as? Set) ?? []) - if let post = self.post?.original ?? self.post, deletedPosts.contains(post) { - closeEditor(animated: true) + let deletedObjects = ((userInfo[NSDeletedObjectsKey] as? Set) ?? []) + if deletedObjects.contains(where: { $0.objectID == originalPostID }) { + closeEditor() } } diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index cec54bd54200..5c86ab4a32d9 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -13,6 +13,7 @@ class PostCardStatusViewModel: NSObject, AbstractPostMenuViewModel { private let isInternetReachable: Bool private let isJetpackFeaturesEnabled: Bool private let isBlazeFlagEnabled: Bool + private let isSyncPublishingEnabled: Bool var progressBlock: ((Float) -> Void)? = nil { didSet { @@ -33,15 +34,25 @@ class PostCardStatusViewModel: NSObject, AbstractPostMenuViewModel { init(post: Post, isInternetReachable: Bool = ReachabilityUtils.isInternetReachable(), isJetpackFeaturesEnabled: Bool = JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled(), - isBlazeFlagEnabled: Bool = BlazeHelper.isBlazeFlagEnabled()) { + isBlazeFlagEnabled: Bool = BlazeHelper.isBlazeFlagEnabled(), + isSyncPublishingEnabled: Bool = RemoteFeatureFlag.syncPublishing.enabled()) { self.post = post self.isInternetReachable = isInternetReachable self.isJetpackFeaturesEnabled = isJetpackFeaturesEnabled self.isBlazeFlagEnabled = isBlazeFlagEnabled + self.isSyncPublishingEnabled = isSyncPublishingEnabled super.init() } var status: String? { + guard isSyncPublishingEnabled else { + return _status + } + return nil + } + + /// - note: Deprecated (kahu-offline-mode) + private var _status: String? { // TODO Move these string constants to the StatusMessages enum if MediaCoordinator.shared.isUploadingMedia(for: post) { return NSLocalizedString("Uploading media...", comment: "Message displayed on a post's card while the post is uploading media") @@ -117,6 +128,9 @@ class PostCardStatusViewModel: NSObject, AbstractPostMenuViewModel { } var shouldHideProgressView: Bool { + if isSyncPublishingEnabled { + return true + } return !(MediaCoordinator.shared.isUploadingMedia(for: post) || post.remoteStatus == .pushing) } @@ -164,14 +178,16 @@ class PostCardStatusViewModel: NSObject, AbstractPostMenuViewModel { buttons.append(.share) } - if autoUploadInteractor.canRetryUpload(of: post) || - autoUploadInteractor.autoUploadAttemptState(of: post) == .reachedLimit || - post.isFailed && isInternetReachable { - buttons.append(.retry) - } + if !isSyncPublishingEnabled { + if autoUploadInteractor.canRetryUpload(of: post) || + autoUploadInteractor.autoUploadAttemptState(of: post) == .reachedLimit || + post.isFailed && isInternetReachable { + buttons.append(.retry) + } - if canCancelAutoUpload && !isInternetReachable { - buttons.append(.cancelAutoUpload) + if canCancelAutoUpload && !isInternetReachable { + buttons.append(.cancelAutoUpload) + } } if canPublish { diff --git a/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift b/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift index 2de9a73b972c..1306639fa7f7 100644 --- a/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift +++ b/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift @@ -83,15 +83,119 @@ extension PublishingEditor { } } - func handlePublishButtonTap() { + func handlePrimaryActionButtonTap() { let action = self.postEditorStateContext.action - publishPost( - action: action, - dismissWhenDone: action.dismissesEditor, - analyticsStat: self.postEditorStateContext.publishActionAnalyticsStat) + // - note: work-in-progress (kahu-offline-mode) + // TODO: enable for pages once we ready + guard RemoteFeatureFlag.syncPublishing.enabled() && post is Post else { + publishPost( + action: action, + dismissWhenDone: action.dismissesEditor, + analyticsStat: self.postEditorStateContext.publishActionAnalyticsStat) + return + } + + performEditorAction(action, analyticsStat: postEditorStateContext.publishActionAnalyticsStat) + } + + func handleSecondaryActionButtonTap() { + guard let action = self.postEditorStateContext.secondaryPublishButtonAction else { + // If the user tapped on the secondary publish action button, it means we should have a secondary publish action. + let error = NSError(domain: EditorError.errorDomain, code: EditorError.expectedSecondaryAction.rawValue, userInfo: nil) + WordPressAppDelegate.crashLogging?.logError(error) + return + } + + let secondaryStat = self.postEditorStateContext.secondaryPublishActionAnalyticsStat + + let publishPostClosure = { [unowned self] in + guard RemoteFeatureFlag.syncPublishing.enabled() else { + publishPost(action: action, dismissWhenDone: action.dismissesEditor, analyticsStat: secondaryStat) + return + } + performEditorAction(action, analyticsStat: secondaryStat) + } + + if presentedViewController != nil { + dismiss(animated: true, completion: publishPostClosure) + } else { + publishPostClosure() + } + } + + func performEditorAction(_ action: PostEditorAction, analyticsStat: WPAnalyticsStat?) { + if action == .publish { + WPAnalytics.track(.editorPostPublishTap) + } + + mapUIContentToPostAndSave(immediate: true) + + switch action { + case .schedule, .publish: + showPrepublishingSheet(for: action, analyticsStat: analyticsStat) + case .saveAsDraft: + performSaveDraftAction() + case .update: + if post.original().status == .draft { + performSaveDraftAction() + } else { + performUpdatePostAction() + } + case .submitForReview: + // TODO: Show Prepublishing VC + fatalError("Not implemented (kahu-offline-mode)") + case .save, .continueFromHomepageEditing: + assertionFailure("No longer used and supported") + assertionFailure("No longer used and supported") + } + } + + private func showPrepublishingSheet(for action: PostEditorAction, analyticsStat: WPAnalyticsStat?) { + displayPublishConfirmationAlert(for: action) { [weak self] result in + guard let self else { return } + switch result { + case .confirmed: + assertionFailure("Not used when .syncPublishing is enabled") + break + case .published: + self.emitPostSaveEvent() + if let analyticsStat { + self.trackPostSave(stat: analyticsStat) + } + self.editorSession.end(outcome: action.analyticsEndOutcome) + + let presentBloggingReminders = JetpackNotificationMigrationService.shared.shouldPresentNotifications() + self.dismissOrPopView(presentBloggingReminders: presentBloggingReminders) + case .cancelled: + self.publishingDismissed() + WPAnalytics.track(.editorPostPublishDismissed) + } + } + } + + private func performSaveDraftAction() { + PostCoordinator.shared.setNeedsSync(for: post) + dismissOrPopView() + } + + private func performUpdatePostAction() { + SVProgressHUD.setDefaultMaskType(.clear) + SVProgressHUD.show() + postEditorStateContext.updated(isBeingPublished: true) + + Task { @MainActor in + do { + try await PostCoordinator.shared._save(post) + dismissOrPopView() + } catch { + postEditorStateContext.updated(isBeingPublished: false) + } + await SVProgressHUD.dismiss() + } } + /// - note: Deprecated (kahu-offline-mode) func publishPost( action: PostEditorAction, dismissWhenDone: Bool, @@ -303,6 +407,28 @@ extension PublishingEditor { ActionDispatcher.dispatch(NoticeAction.clearWithTag(uploadFailureNoticeTag)) stopEditing() + guard RemoteFeatureFlag.syncPublishing.enabled() else { + return _cancelEditing() + } + + guard !post.changes.isEmpty else { + return discardAndDismiss() + } + + if post.original().status == .draft { + showCloseDraftConfirmationAlert() + } else { + showClosePublishedPostConfirmationAlert() + } + } + + private func discardAndDismiss() { + editorSession.end(outcome: .cancel) + discardUnsavedChangesAndUpdateGUI() + } + + /// - note: Deprecated (kahu-offline-mode) + func _cancelEditing() { /// If a post is marked to be auto uploaded and can be saved, it means that the changes /// had been already confirmed by the user. In this case, we just close the editor. /// Otherwise, we'll show an Action Sheet with options. @@ -322,6 +448,7 @@ extension PublishingEditor { } } + /// - note: Deprecated (kahu-offline-mode) private func resumeSaving() { post.shouldAttemptAutoUpload = false let action: PostEditorAction = post.status == .draft ? .update : .publish @@ -334,9 +461,39 @@ extension PublishingEditor { dismissOrPopView(didSave: !postDeleted) } - // Returns true when the post is deleted @discardableResult func discardChanges() -> Bool { + guard RemoteFeatureFlag.syncPublishing.enabled() else { + return _discardChanges() + } + + guard let context = post.managedObjectContext else { + assertionFailure() + return true + } + + WPAppAnalytics.track(.editorDiscardedChanges, withProperties: [WPAppAnalyticsKeyEditorSource: analyticsEditorSource], with: post) + + // TODO: this is incorrect because there might still be media in the previous revision + cancelUploadOfAllMedia(for: post) + + guard let original = post.original else { + assertionFailure("Editor works with revisions") + return true + } + // Original can be either an unsynced revision or an original post at this post + original.deleteRevision() + if original.isNewDraft { + context.delete(original) + } + ContextManager.shared.saveContextAndWait(context) + return true + } + + // Returns true when the post is deleted + @discardableResult + func _discardChanges() -> Bool { + var postDeleted = false guard let managedObjectContext = post.managedObjectContext, let originalPost = post.original else { return postDeleted @@ -381,6 +538,36 @@ extension PublishingEditor { return postDeleted } + private func showCloseDraftConfirmationAlert() { + let alert = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet) + alert.view.accessibilityIdentifier = "post-has-changes-alert" + alert.addCancelActionWithTitle(Strings.closeConfirmationAlertCancel) + let discardTitle = post.original().isNewDraft ? Strings.closeConfirmationAlertDelete : Strings.closeConfirmationAlertDiscardChanges + alert.addDestructiveActionWithTitle(discardTitle) { _ in + self.discardAndDismiss() + } + alert.addActionWithTitle(Strings.closeConfirmationAlertSaveDraft, style: .default) { _ in + self.performSaveDraftAction() + } + alert.popoverPresentationController?.barButtonItem = alertBarButtonItem + present(alert, animated: true, completion: nil) + } + + private func showClosePublishedPostConfirmationAlert() { + let alert = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet) + alert.view.accessibilityIdentifier = "post-has-changes-alert" + alert.addCancelActionWithTitle(Strings.closeConfirmationAlertCancel) + alert.addDestructiveActionWithTitle(Strings.closeConfirmationAlertDiscardChanges) { _ in + self.discardAndDismiss() + } + alert.addActionWithTitle(Strings.closeConfirmationAlertSaveDraftChanges, style: .default) { _ in + fatalError("TODO: save changes locally (kahu-offline-mode") + } + alert.popoverPresentationController?.barButtonItem = alertBarButtonItem + present(alert, animated: true, completion: nil) + } + + /// - note: Deprecated (kahu-offline-mode) func showPostHasChangesAlert() { let title = NSLocalizedString("You have unsaved changes.", comment: "Title of message with options that shown when there are unsaved changes and the author is trying to move away from the post.") let cancelTitle = NSLocalizedString("Keep Editing", comment: "Button shown if there are unsaved changes and the author is trying to move away from the post.") @@ -466,50 +653,32 @@ extension PublishingEditor { mapUIContentToPostAndSave(immediate: true) - if RemoteFeatureFlag.syncPublishing.enabled() { - Task { @MainActor in - do { - self.post = try await PostCoordinator.shared._update(post) - if dismissWhenDone { - self.dismissOrPopView() - } else { - self.createRevisionOfPost() - } - } catch { - // Do nothing - } - self.postEditorStateContext.updated(isBeingPublished: false) - await SVProgressHUD.dismiss() - } - } else { - consolidateChangesIfPostIsNew() + consolidateChangesIfPostIsNew() - PostCoordinator.shared.save(post, - defaultFailureNotice: uploadFailureNotice(action: action)) { [weak self] result in - guard let self = self else { - return - } - self.postEditorStateContext.updated(isBeingPublished: false) - SVProgressHUD.dismiss() + PostCoordinator.shared.save(post, defaultFailureNotice: uploadFailureNotice(action: action)) { [weak self] result in + guard let self = self else { + return + } + self.postEditorStateContext.updated(isBeingPublished: false) + SVProgressHUD.dismiss() - let generator = UINotificationFeedbackGenerator() - generator.prepare() + let generator = UINotificationFeedbackGenerator() + generator.prepare() - switch result { - case .success(let uploadedPost): - self.post = uploadedPost + switch result { + case .success(let uploadedPost): + self.post = uploadedPost - generator.notificationOccurred(.success) - case .failure(let error): - DDLogError("Error publishing post: \(error.localizedDescription)") - generator.notificationOccurred(.error) - } + generator.notificationOccurred(.success) + case .failure(let error): + DDLogError("Error publishing post: \(error.localizedDescription)") + generator.notificationOccurred(.error) + } - if dismissWhenDone { - self.dismissOrPopView() - } else { - self.createRevisionOfPost() - } + if dismissWhenDone { + self.dismissOrPopView() + } else { + self.createRevisionOfPost() } } } @@ -587,8 +756,34 @@ extension PublishingEditor { debouncer.call(immediate: immediate) } - // TODO: Rip this out and put it into the PostService func createRevisionOfPost(loadAutosaveRevision: Bool = false) { + guard RemoteFeatureFlag.syncPublishing.enabled() else { + return _createRevisionOfPost(loadAutosaveRevision: loadAutosaveRevision) + } + guard let managedObjectContext = post.managedObjectContext else { + return assertionFailure() + } + + assert(post.latest() == post, "Must be opened with the latest verison of the post") + + if !(post.isRevision() && !post.isSyncNeeded) { + DDLogDebug("Creating new revision") + post = post._createRevision() + } + + if loadAutosaveRevision { + DDLogDebug("Loading autosave") + post.postTitle = post.autosaveTitle + post.mt_excerpt = post.autosaveExcerpt + post.content = post.autosaveContent + } + + ContextManager.sharedInstance().save(managedObjectContext) + } + + // TODO: Rip this out and put it into the PostService + // - note: Deprecated (kahu-offline-mode) + func _createRevisionOfPost(loadAutosaveRevision: Bool = false) { if post.isLocalRevision, post.original?.postTitle == nil, post.original?.content == nil { // Editing a locally made revision has bit of weirdness in how autosave and @@ -648,10 +843,24 @@ extension PublishingEditor { } } +private enum EditorError: Int { + case expectedSecondaryAction = 1 + + static let errorDomain = "PostEditor.errorDomain" +} + struct PostEditorDebouncerConstants { static let autoSavingDelay = Double(0.5) } +private enum Strings { + static let closeConfirmationAlertCancel = NSLocalizedString("postEditor.closeConfirmationAlert.keepEditing", value: "Keep Editing", comment: "Button to keep the changes in an alert confirming discaring changes") + static let closeConfirmationAlertDelete = NSLocalizedString("postEditor.closeConfirmationAlert.discardDraft", value: "Discard Draft", comment: "Button in an alert confirming discaring a new draft") + static let closeConfirmationAlertDiscardChanges = NSLocalizedString("postEditor.closeConfirmationAlert.discardChanges", value: "Discard Changes", comment: "Button in an alert confirming discaring changes") + static let closeConfirmationAlertSaveDraft = NSLocalizedString("postEditor.closeConfirmationAlert.saveDraft", value: "Save Draft", comment: "Button in an alert confirming saving a new draft") + static let closeConfirmationAlertSaveDraftChanges = NSLocalizedString("postEditor.closeConfirmationAlert.saveDraftChanges", value: "Save Draft Changes", comment: "Button in an alert confirming saving draft changes to an existing published post") +} + private struct MediaUploadingAlert { static let title = NSLocalizedString("Uploading media", comment: "Title for alert when trying to save/exit a post before media upload process is complete.") static let message = NSLocalizedString("You are currently uploading media. Please wait until this completes.", comment: "This is a notification the user receives if they are trying to save a post (or exit) before the media upload process is complete.") diff --git a/WordPress/Classes/ViewRelated/Post/PostEditorState.swift b/WordPress/Classes/ViewRelated/Post/PostEditorState.swift index 75d819cbf248..79a41c4e34d5 100644 --- a/WordPress/Classes/ViewRelated/Post/PostEditorState.swift +++ b/WordPress/Classes/ViewRelated/Post/PostEditorState.swift @@ -6,6 +6,7 @@ import WordPressShared /// None of the associated values should be (nor can be) accessed directly by the UI, only through the `PostEditorStateContext` instance. /// public enum PostEditorAction { + /// - note: Deprecated (kahu-offline-mode) case save case saveAsDraft case schedule @@ -14,6 +15,7 @@ public enum PostEditorAction { case submitForReview case continueFromHomepageEditing + /// - note: Deprecated (kahu-offline-mode) var dismissesEditor: Bool { switch self { case .publish, .schedule, .submitForReview: @@ -23,6 +25,7 @@ public enum PostEditorAction { } } + /// - note: Deprecated (kahu-offline-mode) var isAsync: Bool { switch self { case .publish, .schedule, .submitForReview: @@ -71,6 +74,7 @@ public enum PostEditorAction { } } + /// - note: Deprecated (kahu-offline-mode) var publishingActionLabel: String { switch self { case .publish: diff --git a/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift b/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift index 34be1b55784e..e2218441b74d 100644 --- a/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift +++ b/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift @@ -15,11 +15,18 @@ protocol EditorAnalyticsProperties: AnyObject { struct PostListEditorPresenter { static func handle(post: Post, in postListViewController: EditorPresenterViewController, entryPoint: PostEditorEntryPoint = .unknown) { - - // Return early if a post is still uploading when the editor's requested. - guard !PostCoordinator.shared.isUploading(post: post) else { - presentAlertForPostBeingUploaded() - return + if RemoteFeatureFlag.syncPublishing.enabled() { + // Return early if a post is still uploading when the editor's requested. + guard !PostCoordinator.shared.isUpdating(post) else { + presentAlertForPostBeingUploaded() + return + } + } else { + // Return early if a post is still uploading when the editor's requested. + guard !PostCoordinator.shared.isUploading(post: post) else { + presentAlertForPostBeingUploaded() + return + } } // Autosaves are ignored for posts with local changes. diff --git a/WordPress/Classes/ViewRelated/Post/PostListHeaderView.swift b/WordPress/Classes/ViewRelated/Post/PostListHeaderView.swift index 0f567aa248a2..eeaf372f0df6 100644 --- a/WordPress/Classes/ViewRelated/Post/PostListHeaderView.swift +++ b/WordPress/Classes/ViewRelated/Post/PostListHeaderView.swift @@ -79,6 +79,8 @@ final class PostListHeaderView: UIView { stackView = UIStackView(arrangedSubviews: [textLabel, ellipsisButton]) } + indicator.transform = CGAffineTransform(scaleX: 0.8, y: 0.8) + stackView.spacing = 12 addSubview(stackView) stackView.translatesAutoresizingMaskIntoConstraints = false @@ -90,9 +92,10 @@ final class PostListHeaderView: UIView { return } NSLayoutConstraint.activate([ - icon.widthAnchor.constraint(equalToConstant: 24), - icon.heightAnchor.constraint(equalToConstant: 24) + icon.widthAnchor.constraint(equalToConstant: 22), + icon.heightAnchor.constraint(equalToConstant: 22) ]) + icon.contentMode = .scaleAspectFit } private func setupEllipsisButton() { diff --git a/WordPress/Classes/ViewRelated/Post/PostListViewController.swift b/WordPress/Classes/ViewRelated/Post/PostListViewController.swift index 46af2b57cfec..69bd23e0ff20 100644 --- a/WordPress/Classes/ViewRelated/Post/PostListViewController.swift +++ b/WordPress/Classes/ViewRelated/Post/PostListViewController.swift @@ -114,7 +114,7 @@ final class PostListViewController: AbstractPostListViewController, UIViewContro } let updatedIndexPaths = (tableView.indexPathsForVisibleRows ?? []).filter { let post = fetchResultsController.object(at: $0) - return updatedObjects.contains(post) + return updatedObjects.contains(post) || updatedObjects.contains(post.original()) } if !updatedIndexPaths.isEmpty { tableView.beginUpdates() diff --git a/WordPress/Classes/ViewRelated/Post/PostSettingsViewController+Swift.swift b/WordPress/Classes/ViewRelated/Post/PostSettingsViewController+Swift.swift index 539928db10d0..da88623906a3 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSettingsViewController+Swift.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSettingsViewController+Swift.swift @@ -26,11 +26,18 @@ extension PostSettingsViewController { apost.objectWillChange.sink { [weak self] in self?.didUpdateSettings() }.store(in: &cancellables) - objc_setAssociatedObject(self, &PostSettingsViewController.cancellablesKey, cancellables, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) if RemoteFeatureFlag.syncPublishing.enabled() { - NotificationCenter.default.addObserver(self, selector: #selector(didChangeObjects), name: NSManagedObjectContext.didChangeObjectsNotification, object: apost.managedObjectContext) + let originalPostID = (apost.original ?? apost).objectID + + NotificationCenter.default + .publisher(for: NSManagedObjectContext.didChangeObjectsNotification, object: apost.managedObjectContext) + .sink { [weak self] notification in + self?.didChangeObjects(notification, originalPostID: originalPostID) + }.store(in: &cancellables) } + + objc_setAssociatedObject(self, &PostSettingsViewController.cancellablesKey, cancellables, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) } private func didUpdateSettings() { @@ -83,12 +90,11 @@ extension PostSettingsViewController { } } - @objc private func didChangeObjects(_ notification: Foundation.Notification) { + private func didChangeObjects(_ notification: Foundation.Notification, originalPostID: NSManagedObjectID) { guard let userInfo = notification.userInfo else { return } - let deletedPosts = ((userInfo[NSDeletedObjectsKey] as? Set) ?? []) - let original = self.apost.original ?? self.apost - if deletedPosts.contains(original) { + let deletedObjects = ((userInfo[NSDeletedObjectsKey] as? Set) ?? []) + if deletedObjects.contains(where: { $0.objectID == originalPostID }) { presentingViewController?.dismiss(animated: true) } } diff --git a/WordPress/Classes/ViewRelated/Post/PostSyncStateViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostSyncStateViewModel.swift index 49a1b6401aed..82bd95623c0a 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSyncStateViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSyncStateViewModel.swift @@ -3,17 +3,53 @@ import Foundation final class PostSyncStateViewModel { enum State { case idle - case syncing + // Has unsynced changes + case unsynced + case uploading case offlineChanges case failed } private let post: Post private let isInternetReachable: Bool + private let isSyncPublishingEnabled: Bool + + init(post: Post, + isInternetReachable: Bool = ReachabilityUtils.isInternetReachable(), + isSyncPublishingEnabled: Bool = RemoteFeatureFlag.syncPublishing.enabled()) { + self.post = post + self.isInternetReachable = isInternetReachable + self.isSyncPublishingEnabled = isSyncPublishingEnabled + } var state: State { + guard isSyncPublishingEnabled else { + return _state + } + + if let error = PostCoordinator.shared.syncError(for: post.original()) { + if let saveError = error as? PostRepository.PostSaveError, + case .conflict = saveError { + return .failed // Terminal error + } + if let urlError = (error as NSError).underlyingErrors.first as? URLError, + urlError.code == .notConnectedToInternet { + return .offlineChanges // A better indicator on what's going on + } + } + if PostCoordinator.isSyncNeeded(for: post) { + return .unsynced + } + if PostCoordinator.shared.isDeleting(post) || PostCoordinator.shared.isUpdating(post) { + return .uploading + } + return .idle + } + + /// - note: Deprecated (kahu-offline-mode) + private var _state: State { if post.remoteStatus == .pushing || PostCoordinator.shared.isDeleting(post) || PostCoordinator.shared.isUpdating(post) { - return .syncing + return .uploading } if post.isFailed { return isInternetReachable ? .failed : .offlineChanges @@ -22,7 +58,7 @@ final class PostSyncStateViewModel { } var isEditable: Bool { - state == .idle || state == .offlineChanges || state == .failed + state != .uploading } var isShowingEllipsis: Bool { @@ -30,7 +66,7 @@ final class PostSyncStateViewModel { } var isShowingIndicator: Bool { - state == .syncing + state == .uploading || state == .unsynced } var iconInfo: (image: UIImage?, color: UIColor)? { @@ -39,33 +75,24 @@ final class PostSyncStateViewModel { return (UIImage(systemName: "wifi.slash"), UIColor.listIcon) case .failed: return (UIImage.gridicon(.notice), UIColor.error) - case .idle, .syncing: + case .idle, .uploading, .unsynced: return nil } } var statusMessage: String? { - guard RemoteFeatureFlag.syncPublishing.enabled() else { + guard isSyncPublishingEnabled else { return nil } switch state { case .offlineChanges: return Strings.offlineChanges - case .failed, .idle, .syncing: + case .failed, .idle, .uploading, .unsynced: return nil } } - - init(post: Post, isInternetReachable: Bool = ReachabilityUtils.isInternetReachable()) { - self.post = post - self.isInternetReachable = isInternetReachable - } } private enum Strings { - static let offlineChanges = NSLocalizedString( - "postList.offlineChanges", - value: "Offline changes", - comment: "Label for a post in the post list. Indicates that the post has offline changes." - ) + static let offlineChanges = NSLocalizedString("postList.offlineChanges", value: "Offline changes", comment: "Label for a post in the post list. Indicates that the post has offline changes.") } diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index cadcce50255d..651cfe39939d 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -525,6 +525,7 @@ 0CA10FA52ADB286300CE75AC /* StringRankedSearchTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA10FA42ADB286300CE75AC /* StringRankedSearchTests.swift */; }; 0CA10FA82ADB7C5200CE75AC /* PostSearchService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA10FA62ADB76ED00CE75AC /* PostSearchService.swift */; }; 0CA10FA92ADB7C5300CE75AC /* PostSearchService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA10FA62ADB76ED00CE75AC /* PostSearchService.swift */; }; + 0CA15B4E2BB2128800518D6E /* PostCoordinatorSyncTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA15B4D2BB2128800518D6E /* PostCoordinatorSyncTests.swift */; }; 0CA1C8C12A940EE300F691EE /* AvatarMenuController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA1C8C02A940EE300F691EE /* AvatarMenuController.swift */; }; 0CA1C8C22A940EE300F691EE /* AvatarMenuController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CA1C8C02A940EE300F691EE /* AvatarMenuController.swift */; }; 0CAE8EF22A9E9E8D0073EEB9 /* SiteMediaCollectionCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CAE8EF12A9E9E8D0073EEB9 /* SiteMediaCollectionCell.swift */; }; @@ -6281,6 +6282,7 @@ 0CA10F722ADB014C00CE75AC /* StringRankedSearch.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StringRankedSearch.swift; sourceTree = ""; }; 0CA10FA42ADB286300CE75AC /* StringRankedSearchTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StringRankedSearchTests.swift; sourceTree = ""; }; 0CA10FA62ADB76ED00CE75AC /* PostSearchService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PostSearchService.swift; sourceTree = ""; }; + 0CA15B4D2BB2128800518D6E /* PostCoordinatorSyncTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PostCoordinatorSyncTests.swift; sourceTree = ""; }; 0CA1C8C02A940EE300F691EE /* AvatarMenuController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AvatarMenuController.swift; sourceTree = ""; }; 0CAE8EF12A9E9E8D0073EEB9 /* SiteMediaCollectionCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SiteMediaCollectionCell.swift; sourceTree = ""; }; 0CAE8EF52A9E9EE30073EEB9 /* SiteMediaCollectionCellViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SiteMediaCollectionCellViewModel.swift; sourceTree = ""; }; @@ -15764,6 +15766,7 @@ 8BC6020C2390412000EFE3D0 /* NullBlogPropertySanitizerTests.swift */, 08A2AD7A1CCED8E500E84454 /* PostCategoryServiceTests.m */, 8BC12F71231FEBA1004DDA72 /* PostCoordinatorTests.swift */, + 0CA15B4D2BB2128800518D6E /* PostCoordinatorSyncTests.swift */, 8B821F3B240020E2006B697E /* PostServiceUploadingListTests.swift */, 8BC12F7623201B86004DDA72 /* PostService+MarkAsFailedAndDraftIfNeededTests.swift */, FEFA6AC52A86824A004EE5E6 /* PostHelperJetpackSocialTests.swift */, @@ -23791,6 +23794,7 @@ E18549DB230FBFEF003C620E /* BlogServiceDeduplicationTests.swift in Sources */, 0148CC292859127F00CF5D96 /* StatsWidgetsStoreTests.swift in Sources */, 7320C8BD2190C9FC0082FED5 /* UITextView+SummaryTests.swift in Sources */, + 0CA15B4E2BB2128800518D6E /* PostCoordinatorSyncTests.swift in Sources */, 24A2948325D602710000A51E /* BlogTimeZoneTests.m in Sources */, 80B016CF27FEBDC900D15566 /* DashboardCardTests.swift in Sources */, 7E53AB0420FE6681005796FE /* ActivityContentRouterTests.swift in Sources */, diff --git a/WordPress/WordPressTest/AbstractPostTest.swift b/WordPress/WordPressTest/AbstractPostTest.swift index c4b77622d588..6b4b3cfaeb81 100644 --- a/WordPress/WordPressTest/AbstractPostTest.swift +++ b/WordPress/WordPressTest/AbstractPostTest.swift @@ -36,4 +36,42 @@ class AbstractPostTest: CoreDataTestCase { XCTAssertEqual(post.featuredImageURLForDisplay()?.absoluteString, "https://wp.me/awesome.png") } + func testGetLatestRevisionNeedingSync() { + // GIVEN a post with no revisions + let post = PostBuilder(mainContext).build() + + // THEN + XCTAssertNil(post.getLatestRevisionNeedingSync()) + + // GIVEN a post with a revision that doesn't need sync + let revision1 = post._createRevision() + + // THEN + XCTAssertNil(post.getLatestRevisionNeedingSync()) + + // GIVEN a post with a revision that needs sync + let revision2 = revision1._createRevision() + revision2.isSyncNeeded = true + + // THEN + XCTAssertEqual(post.getLatestRevisionNeedingSync(), revision2) + } + + func testDeleteSyncedRevisions() { + // GIVEN a post with three revisions + let post = PostBuilder(mainContext).build() + let revision1 = post._createRevision() + let revision2 = revision1._createRevision() + let revision3 = revision2._createRevision() + + // WHEN + post.deleteSyncedRevisions(until: revision2) + + // THEN + XCTAssertEqual(post.revision, revision3) + XCTAssertEqual(revision3.original, post) + XCTAssertTrue(revision1.isDeleted) + XCTAssertTrue(revision2.isDeleted) + XCTAssertFalse(revision3.isDeleted) + } } diff --git a/WordPress/WordPressTest/MediaImageServiceTests.swift b/WordPress/WordPressTest/MediaImageServiceTests.swift index 7be4512c973d..c8f9c7819cf1 100644 --- a/WordPress/WordPressTest/MediaImageServiceTests.swift +++ b/WordPress/WordPressTest/MediaImageServiceTests.swift @@ -242,8 +242,12 @@ class MediaImageServiceTests: CoreDataTestCase { return blog } - /// `Media` is hardcoded to work with a specific direcoty URL managed by `MediaFileManager` func makeLocalURL(forResource name: String, fileExtension: String) throws -> URL { + try MediaImageServiceTests.makeLocalURL(forResource: name, fileExtension: fileExtension) + } + + /// `Media` is hardcoded to work with a specific directory URL managed by `MediaFileManager` + static func makeLocalURL(forResource name: String, fileExtension: String) throws -> URL { let sourceURL = try XCTUnwrap(Bundle.test.url(forResource: name, withExtension: fileExtension)) let mediaURL = try MediaFileManager.default.makeLocalMediaURL(withFilename: name, fileExtension: fileExtension) try FileManager.default.copyItem(at: sourceURL, to: mediaURL) diff --git a/WordPress/WordPressTest/PostCompactCellGhostableTests.swift b/WordPress/WordPressTest/PostCompactCellGhostableTests.swift index dc8333748f8e..48e8dc80d177 100644 --- a/WordPress/WordPressTest/PostCompactCellGhostableTests.swift +++ b/WordPress/WordPressTest/PostCompactCellGhostableTests.swift @@ -37,14 +37,6 @@ class PostCompactCellGhostableTests: CoreDataTestCase { XCTAssertTrue(postCell.isUserInteractionEnabled) } - func testShowBadgesLabelAfterConfigure() { - let post = PostBuilder(mainContext).build() - - postCell.configure(with: post) - - XCTAssertFalse(postCell.badgesLabel.isHidden) - } - func testHideGhostAfterConfigure() { let post = PostBuilder(mainContext).build() @@ -54,22 +46,6 @@ class PostCompactCellGhostableTests: CoreDataTestCase { XCTAssertFalse(postCell.contentStackView.isHidden) } - func testMenuButtonOpacityAfterConfigure() { - let post = PostBuilder(mainContext).with(remoteStatus: .sync).build() - - postCell.configure(with: post) - - XCTAssertEqual(postCell.menuButton.layer.opacity, 1) - } - - func testMenuButtonOpacityAfterConfigureWithPushingStatus() { - let post = PostBuilder(mainContext).with(remoteStatus: .pushing).build() - - postCell.configure(with: post) - - XCTAssertEqual(postCell.menuButton.layer.opacity, 0.3) - } - private func postCellFromNib() -> PostCompactCell { let bundle = Bundle(for: PostCompactCell.self) guard let postCell = bundle.loadNibNamed("PostCompactCell", owner: nil)?.first as? PostCompactCell else { diff --git a/WordPress/WordPressTest/PostCompactCellTests.swift b/WordPress/WordPressTest/PostCompactCellTests.swift index 25a6db0c5961..95e75c54dc4f 100644 --- a/WordPress/WordPressTest/PostCompactCellTests.swift +++ b/WordPress/WordPressTest/PostCompactCellTests.swift @@ -53,15 +53,6 @@ class PostCompactCellTests: CoreDataTestCase { XCTAssertEqual(postCell.badgesLabel.text, "Sticky") } - func testHideBadgesWhenEmpty() { - let post = PostBuilder(mainContext).build() - - postCell.configure(with: post) - - XCTAssertEqual(postCell.badgesLabel.text, "Uploading post...") - XCTAssertFalse(postCell.badgesLabel.isHidden) - } - func testShowBadgesWhenNotEmpty() { let post = PostBuilder(mainContext) .with(remoteStatus: .sync) @@ -73,16 +64,6 @@ class PostCompactCellTests: CoreDataTestCase { XCTAssertTrue(postCell.badgesLabel.isHidden) } - func testShowProgressView() { - let post = PostBuilder(mainContext) - .with(remoteStatus: .pushing) - .published().build() - - postCell.configure(with: post) - - XCTAssertFalse(postCell.progressView.isHidden) - } - func testHideProgressView() { let post = PostBuilder(mainContext) .with(remoteStatus: .sync) @@ -93,18 +74,6 @@ class PostCompactCellTests: CoreDataTestCase { XCTAssertTrue(postCell.progressView.isHidden) } - func testShowsWarningMessageForFailedPublishedPosts() { - // Given - let post = PostBuilder(mainContext).published().with(remoteStatus: .failed).confirmedAutoUpload().build() - - // When - postCell.configure(with: post) - - // Then - XCTAssertEqual(postCell.badgesLabel.text, i18n("We'll publish the post when your device is back online.")) - XCTAssertEqual(postCell.badgesLabel.textColor, UIColor.warning) - } - private func postCellFromNib() -> PostCompactCell { let bundle = Bundle(for: PostCompactCell.self) guard let postCell = bundle.loadNibNamed("PostCompactCell", owner: nil)?.first as? PostCompactCell else { diff --git a/WordPress/WordPressTest/PostCoordinatorSyncTests.swift b/WordPress/WordPressTest/PostCoordinatorSyncTests.swift new file mode 100644 index 000000000000..89662b281838 --- /dev/null +++ b/WordPress/WordPressTest/PostCoordinatorSyncTests.swift @@ -0,0 +1,421 @@ +import Combine +import XCTest +import OHHTTPStubs + +@testable import WordPress + +@MainActor +class PostCoordinatorSyncTests: CoreDataTestCase { + private var blog: Blog! + private var mediaCoordinator: MediaCoordinator! + private var coordinator: PostCoordinator! + private var cancellables: [AnyCancellable] = [] + + override func setUpWithError() throws { + try super.setUpWithError() + + blog = BlogBuilder(mainContext) + .with(dotComID: 80511) + .withAnAccount() + .build() + try mainContext.save() + + mediaCoordinator = MediaCoordinator(coreDataStack: contextManager) + coordinator = PostCoordinator(mediaCoordinator: mediaCoordinator, coreDataStack: contextManager, isSyncPublishingEnabled: true) + } + + override func tearDown() { + super.tearDown() + + HTTPStubs.removeAllStubs() + } + + /// Scenario: save a single revision, it successfully syncs. + func testSyncSingleSimpleRevision() async throws { + // GIVEN a draft post that needs sync + let post = PostBuilder(mainContext, blog: blog).build() + post.status = .draft + post.authorID = 29043 + + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.content = "content-a" + + // GIVEN + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { request in + try! HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 201) + } + + // WHEN + coordinator.setNeedsSync(for: revision1) + try await coordinator.waitForSync(post, to: revision1) + + // THEN post got synced + XCTAssertEqual(post.postID, 974) + XCTAssertNil(post.revision) + } + + /// Scenario: first request fails, second one succeedes. + func testSyncSingleSimpleRevisionAfterRetry() async throws { + // GIVEN a draft post that needs sync + let post = PostBuilder(mainContext, blog: blog).build() + post.status = .draft + post.authorID = 29043 + + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.content = "content-a" + + // GIVEN + var requestCount = 0 + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { _ in + requestCount += 1 + switch requestCount { + case 1: + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + case 2: + return try! HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 201) + default: + XCTFail("Unexpected number of requests") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + } + + // GIVEN + coordinator.syncRetryDelay = 0.01 + + // WHEN + coordinator.setNeedsSync(for: revision1) + try await coordinator.waitForSync(post, to: revision1, ignoreErrors: true) + + // THEN post got synced after the retry + XCTAssertEqual(post.postID, 974) + XCTAssertNil(post.revision) + } + + /// Scenario: user saves changes to the post while sync is in progress. + func testSyncRevisionAddedDuringSync() async throws { + // GIVEN a draft post that needs sync + let post = PostBuilder(mainContext, blog: blog).build() + post.status = .draft + post.authorID = 29043 + + let revision1 = post._createRevision() + revision1.postTitle = "title-a" + revision1.content = "content-a" + + // GIVEN + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { _ in + XCTAssertFalse(Thread.isMainThread) + DispatchQueue.main.sync { + let revision2 = revision1._createRevision() + revision2.postTitle = "title-b" + self.coordinator.setNeedsSync(for: revision2) + } + + let response = try! HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 201) + response.responseTime = 0.01 + return response + } + + var isPartialRequestSent = false + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in + XCTAssertEqual(request.getBodyParameters()?["title"] as? String, "title-b") + isPartialRequestSent = true + + var post = WordPressComPost.mock + post.title = "title-b" + post.content = "content-a" + return try! HTTPStubsResponse(value: post, statusCode: 202) + } + + // WHEN + coordinator.setNeedsSync(for: revision1) + + try await coordinator.waitForSync(post) { operation in + operation.revision != revision1 // Must be revision2 + } + + // THEN post got synced after the retry + XCTAssertTrue(isPartialRequestSent) + XCTAssertEqual(post.postID, 974) + XCTAssertEqual(post.postTitle, "title-b") + XCTAssertEqual(post.content, "content-a") + XCTAssertNil(post.revision) + } + + /// Scenario: user created and saved a new draft post with one image block + /// (without waiting for upload to finish). + func testSyncNewDraftWithImageBlock() async throws { + // GIVEN a draft post + let post = PostBuilder(mainContext, blog: blog).build() + post.status = .draft + post.authorID = 29043 + post.postTitle = "title-a" + + // GIVEN a post with a image block with media that needs upload + let media = MediaBuilder(mainContext).build() + media.remoteStatus = .failed + media.blog = post.blog + media.mediaType = .image + media.filename = "test-image.jpg" + media.absoluteLocalURL = try MediaImageServiceTests.makeLocalURL(forResource: "test-image", fileExtension: "jpg") + + // important otherwise MediaService will use temporary objectID and fail + try mainContext.save() + + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.media = [media] + let uploadID = media.gutenbergUploadID + revision1.content = "\n
\n" + + // GIVEN + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { request in + let content = request.getBodyParameters()?["content"] as? String + XCTAssertNotNil(content) + + var mock = WordPressComPost.mock + mock.content = content + return try! HTTPStubsResponse(value: mock, statusCode: 201) + } + + stub(condition: isPath("/rest/v1.1/sites/80511/media/new")) { request in + HTTPStubsResponse(data: mediaResponse.data(using: .utf8)!, statusCode: 202, headers: [:]) + } + + // WHEN + coordinator.setNeedsSync(for: revision1) + try await coordinator.waitForSync(post, to: revision1) + + // THEN image block was updated + XCTAssertEqual(post.content, "\n
\n") + + // THEN revisions were uploaded + XCTAssertFalse(post.hasRevision()) + } + + /// Scenario: sync fails with a "not connected to internet" error and the + /// app quickly re-establishes the connection. + func testSyncFastRetryOnReachabilityChange() async throws { + // GIVEN a draft post that needs sync + let post = PostBuilder(mainContext, blog: blog).build() + post.status = .draft + post.authorID = 29043 + + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.content = "content-a" + + try mainContext.save() + + // GIVEN a first request that fails with a `.notConnectedToInternet` + // error and the second one that succedes + var requestCount = 0 + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { request in + requestCount += 1 + switch requestCount { + case 1: + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + case 2: + return try! HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 201) + default: + XCTFail("Unexpected number of requests: \(requestCount)") + return HTTPStubsResponse(error: URLError(.unknown)) + } + } + + // GIVEN a long default retry + coordinator.syncRetryDelay = 10 + + // GIVEN the app quickly restoring connectivity after the first failure + coordinator.syncEvents.sink { + if case .finished(_, let result) = $0, case .failure = result { + NotificationCenter.default.post(name: .reachabilityChanged, object: nil, userInfo: [Foundation.Notification.reachabilityKey: true]) + } + }.store(in: &cancellables) + + coordinator.setNeedsSync(for: revision1) + try await coordinator.waitForSync(post, to: revision1, ignoreErrors: true) + + // THEN post got synced + XCTAssertEqual(post.postID, 974) + XCTAssertNil(post.revision) + } + + /// Scenario: create and save a revision with a failing image upload. Re-open + /// the editor, delete the image, and try to sync. + func testSyncRevisionAfterDeletingFailingImageUpload() async throws { + // GIVEN a draft post + let post = PostBuilder(mainContext, blog: blog).build() + post.status = .draft + post.authorID = 29043 + post.postTitle = "title-a" + + // GIVEN a post with a image block with media that needs upload + let media = MediaBuilder(mainContext).build() + media.remoteStatus = .failed + media.blog = post.blog + media.mediaType = .image + media.filename = "test-image.jpg" + media.absoluteLocalURL = try MediaImageServiceTests.makeLocalURL(forResource: "test-image", fileExtension: "jpg") + + // important otherwise MediaService will use temporary objectID and fail + try mainContext.save() + + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.media = [media] + let uploadID = media.gutenbergUploadID + revision1.content = "\n
\n" + + // GIVEN + let expectation = self.expectation(description: "started-media-request") + stub(condition: isPath("/rest/v1.1/sites/80511/media/new")) { _ in + let response = HTTPStubsResponse(error: URLError(.unknown)) + expectation.fulfill() + response.responseTime = 10 + return response + } + + // WHEN the app sends the request to upload the image + coordinator.setNeedsSync(for: revision1) + await fulfillment(of: [expectation], timeout: 2) + + let revision2 = post._createRevision() + revision2.media = [] + revision2.content = "empty" + + // GIVEN + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { _ in + try! HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 201) + } + + coordinator.setNeedsSync(for: revision2) + try await coordinator.waitForSync(post, to: revision2) + + // THEN post got synced without waiting for the image to be uploaded + XCTAssertEqual(post.postID, 974) + XCTAssertNil(post.revision) + } + + /// Scenario: syncing changes to an existing draft that was permanently deleted. + func testSyncPermanentlyDeletedPost() async throws { + // GIVEN a draft post that needs sync + let post = PostBuilder(mainContext, blog: blog).build() + post.postID = 974 + post.status = .draft + post.authorID = 29043 + post.postTitle = "title-b" + post.content = "content-a" + + let revision1 = post._createRevision() + revision1.content = "content-b" + + // GIVEN a server where the post was deleted + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { _ in + return try! HTTPStubsResponse(value: [ + "error": "unknown_post", + "message": "Unknown post" + ], statusCode: 404) + } + + // WHEN + coordinator.setNeedsSync(for: revision1) + do { + try await coordinator.waitForSync(post, to: revision1) + XCTFail("Expected sync to fail") + } catch { + guard let error = error as? PostRepository.PostSaveError, + case .deleted = error else { + return XCTFail("Unexpected error") + } + } + + // THEN post got deleted from the database + XCTAssertNil(post.managedObjectContext) + } +} + +private let mediaResponse = """ +{ + "media": [ + { + "ID": 1236, + "URL": "https://example.files.wordpress.com/2024/03/img_0005-1-1.jpg", + "guid": "http://example.files.wordpress.com/2024/03/img_0005-1-1.jpg", + "date": "2024-03-25T18:50:12-04:00", + "post_ID": 0, + "author_ID": 34129043, + "file": "img_0005-1-1.jpg", + "mime_type": "image/jpeg", + "extension": "jpg", + "title": "img_0005-1", + "caption": "", + "description": "", + "alt": "", + "icon": "https://s1.wp.com/wp-includes/images/media/default.png", + "size": "701.54 KB", + "height": 1335, + "width": 2000 + } + ] +} +""" + +private extension URLRequest { + func getBodyParameters() -> [String: Any]? { + guard let data = httpBodyStream?.read(), + let object = try? JSONSerialization.jsonObject(with: data), + let parameters = object as? [String: Any] else { + return nil + } + return parameters + } +} + +private extension PostCoordinator { + func waitForSync(_ post: AbstractPost, to revision: AbstractPost, ignoreErrors: Bool = false, timeout: TimeInterval = 5) async throws { + var olderRevisionIDs = Set(post.allRevisions.filter(\.isSyncNeeded).map(\.objectID)) + olderRevisionIDs.remove(revision.objectID) + return try await waitForSync(post, ignoreErrors: ignoreErrors, timeout: timeout) { operation in + guard !olderRevisionIDs.contains(operation.revision.objectID) else { + return false // Skip operation for older revisions + } + return true + } + } + + /// Taps into the coordinator events and waits until the post syncs to the + /// given revision. + /// + /// - warning: If more revisions are added during after the call is made, + /// it'll still finish. + /// + /// - parameter timeout: The default value is 10 seconds. + /// - parameter handler: Return `true` if the revision matches the one you expected. + func waitForSync(_ post: AbstractPost, ignoreErrors: Bool = false, timeout: TimeInterval = 5, handler: @escaping (PostCoordinator.SyncOperation) -> Bool) async throws { + let result = await syncEvents + .compactMap { event -> Result? in + guard case .finished(let operation, let result) = event else { + return nil + } + if ignoreErrors, case .failure = result { + return nil // Ignore intermitent errors + } + guard handler(operation) else { + return nil + } + return result + } + .first() + .timeout(.seconds(timeout), scheduler: DispatchQueue.main) + .values + .first { _ in true } + + guard let result else { + throw URLError(.unknown) // Should never happen + } + try result.get() + } +} diff --git a/WordPress/WordPressTest/PostRepositorySaveTests.swift b/WordPress/WordPressTest/PostRepositorySaveTests.swift index 1694396f6eed..695fe5a9adbd 100644 --- a/WordPress/WordPressTest/PostRepositorySaveTests.swift +++ b/WordPress/WordPressTest/PostRepositorySaveTests.swift @@ -317,7 +317,7 @@ class PostRepositorySaveTests: CoreDataTestCase { XCTAssertNotNil(post.revision, "Revision is missing") - // GIVEN a server where the post + // GIVEN stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in // THEN the app sends a partial update try assertRequestBody(request, expected: """ @@ -939,6 +939,319 @@ class PostRepositorySaveTests: CoreDataTestCase { // THEN the post is still in the database until the user taps OK and confirms XCTAssertNotNil(post.managedObjectContext) } + + // MARK: - Sync (Drafts) + + /// Scenarios: the app syncs drafts post while the editor is opened. + func testSyncExistingDraftPostWithUnsavedChangesDuringEditing() async throws { + // GIVEN a draft post (synced) + let post = makePost { + $0.status = .draft + $0.postID = 974 + $0.authorID = 29043 + $0.dateCreated = Date(timeIntervalSince1970: 1709852440) + $0.dateModified = Date(timeIntervalSince1970: 1709852440) + $0.postTitle = "title-a" + $0.content = "content-a" + } + + // GIVEN a post that has changes that need to be synced (across two local revision) + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.isSyncNeeded = true + + let revision2 = revision1._createRevision() + revision2.postTitle = "title-c" + revision2.isSyncNeeded = true + + // GIVEN a revision created by an editor + let revision3 = revision2._createRevision() + revision3.postTitle = "title-d" + + // GIVEN a server where the post was deleted + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in + // THEN the app sends a partial update + try assertRequestBody(request, expected: """ + { + "title" : "title-c" + } + """) + + var post = WordPressComPost.mock + post.title = "title-c" + post.sticky = true + return try HTTPStubsResponse(value: post, statusCode: 202) + } + + // WHEN saving the post + try await repository.sync(post) + + // THEN it uploads the latest revision and applies the revision1 to the post + XCTAssertEqual(post.postTitle, "title-c") + XCTAssertTrue(post.revision == revision3) + + // THEN but doesn't apply the remote changes to make sure the app could + // still correctly track local changes (revision0 vs revision2) + XCTAssertFalse(post.isStickyPost) + + // GIVEN + HTTPStubs.removeAllStubs() + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in + // THEN the app sends a partial update + try assertRequestBody(request, expected: """ + { + "title" : "title-d" + } + """) + + var post = WordPressComPost.mock + post.title = "title-d" + post.sticky = true + return try HTTPStubsResponse(value: post, statusCode: 202) + } + + // WHEN + revision3.isSyncNeeded = true + try await repository.sync(post) + + // THEN it uploads the latest revision + XCTAssertEqual(post.postTitle, "title-d") + XCTAssertNil(post.revision) + + // THEN and can now safely apply the changes from the remote + XCTAssertTrue(post.isStickyPost) + } + + /// Scenarios: the app syncs drafts post while the editor is open. The changes + /// include changes to the `content` that might lead to a conflict. + func testSyncExistingDraftPostWithContentChangesDuringEditing() async throws { + // GIVEN a draft post (synced) + let dateModified = Date(timeIntervalSince1970: 1709852440) + let post = makePost { + $0.status = .draft + $0.postID = 974 + $0.authorID = 29043 + $0.dateCreated = dateModified + $0.dateModified = dateModified + $0.postTitle = "title-a" + $0.content = "content-a" + } + + // GIVEN a post that has changes that need to be synced (across two local revision) + let revision1 = post._createRevision() + revision1.content = "content-b" + revision1.isSyncNeeded = true + + let revision2 = revision1._createRevision() + revision2.content = "content-c" + + // GIVEN a server where the post was deleted + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in + // THEN the app sends a partial update + try assertRequestBody(request, expected: """ + { + "content" : "content-b", + "if_not_modified_since" : "2024-03-07T23:00:40+0000" + } + """) + + var post = WordPressComPost.mock + post.content = "content-b" + post.modified = dateModified.addingTimeInterval(5) + return try HTTPStubsResponse(value: post, statusCode: 202) + } + + // WHEN saving the post + try await repository.sync(post) + + // THEN it uploads the latest revision and applies the revision1 to the post + XCTAssertEqual(post.content, "content-b") + XCTAssertTrue(post.revision == revision2) + + // GIVEN the app hits 409 but it's a false positive, so we are good + HTTPStubs.removeAllStubs() + + var requestCount = 0 + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in + // THEN the app sends a partial update but still has the old + // original revision of the post + requestCount += 1 + + if requestCount == 1 { + // THEN the first request contains an `if_not_modified_since` parameter + try assertRequestBody(request, expected: """ + { + "content" : "content-c", + "if_not_modified_since" : "2024-03-07T23:00:40+0000" + } + """) + return HTTPStubsResponse(jsonObject: [ + "error": "old-revision", + "message": "There is a revision of this post that is more recent." + ], statusCode: 409, headers: nil) + } + if requestCount == 2 { + // THEN the second request contains only the delta + try assertRequestBody(request, expected: """ + { + "content" : "content-c" + } + """) + var post = WordPressComPost.mock + post.content = "content-c" + post.modified = dateModified.addingTimeInterval(10) + return try HTTPStubsResponse(value: post, statusCode: 200) + } + + throw URLError(.unknown) + } + + stub(condition: isPath("/rest/v1.1/sites/80511/posts/974")) { request in + var post = WordPressComPost.mock + post.content = "content-b" + post.modified = dateModified.addingTimeInterval(5) + return try HTTPStubsResponse(value: post, statusCode: 200) + } + + // WHEN syncing remaining changes + revision2.isSyncNeeded = true + try await repository.sync(post) + + // THEN it uploads the latest revision and ignores 409 because it's + // false positive – the app made changes based on the latest content + XCTAssertEqual(post.content, "content-c") + XCTAssertEqual(post.dateModified, dateModified.addingTimeInterval(10)) + XCTAssertNil(post.revision) + } + + /// Scenario: created and saved a new draft. + func testSyncNewDraftPost() async throws { + // GIVEN a draft post (new) + let post = makePost { + $0.status = .draft + $0.authorID = 29043 + } + + let revision = post.createRevision() + revision.postTitle = "title-a" + revision.content = "content-a" + revision.isSyncNeeded = true + + // GIVEN a server accepting the new post + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { request in + // THEN the app sends only the required parameters + try assertRequestBody(request, expected: """ + { + "author" : 29043, + "content" : "content-a", + "status" : "draft", + "title" : "title-a", + "type" : "post" + } + """) + return try HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 201) + } + + // WHEN + try await repository.sync(post) + + // THEN the post was created + XCTAssertEqual(post.postID, 974) + XCTAssertEqual(post.status, .draft) + } + + /// Scenario: made an offline change and then reverted it. + func testSyncRevertedChangesAreSkipped() async throws { + // GIVEN a draft post (synced) + let post = makePost { + $0.status = .draft + $0.postID = 974 + $0.authorID = 29043 + $0.dateCreated = Date(timeIntervalSince1970: 1709852440) + $0.dateModified = Date(timeIntervalSince1970: 1709852440) + $0.postTitle = "title-a" + $0.content = "content-a" + } + + // GIVEN a change that was reverted in a more recent revision + let revision1 = post._createRevision() + revision1.postTitle = "title-b" + revision1.isSyncNeeded = true + + let revision2 = revision1._createRevision() + revision2.postTitle = "title-a" + revision2.isSyncNeeded = true + + // GIVEN a server with the an updated content (but not title) + stub(condition: isPath("/rest/v1.2/sites/80511/posts/974")) { request in + XCTFail("No POST requests should be made") + return try HTTPStubsResponse(value: WordPressComPost.mock, statusCode: 202) + } + + stub(condition: isPath("/rest/v1.1/sites/80511/posts/974")) { request in + var post = WordPressComPost.mock + post.title = "title-a" + post.content = "content-b" + return try HTTPStubsResponse(value: post, statusCode: 200) + } + + // WHEN saving the post + try await repository.sync(post) + + // THEN is gets the latest revision from the server but sends no changes + XCTAssertEqual(post.postTitle, "title-a") + XCTAssertEqual(post.content, "content-b") + XCTAssertNil(post.revision) + } + + /// Scenario: saved a new draft and opened an editor before it syncs. + func testSyncCreatedPostWithLocalRevision() async throws { + // GIVEN a new draft post (not synced) + let post = makePost { + $0.status = .draft + $0.authorID = 29043 + } + + // GIVEN a saved revision + let revision1 = post._createRevision() + revision1.postTitle = "title-a" + revision1.content = "content-a" + revision1.isSyncNeeded = true + + // GIVEN a local revision + let revision2 = revision1._createRevision() + revision2.postTitle = "title-b" + + // GIVEN a server accepting the new post + stub(condition: isPath("/rest/v1.2/sites/80511/posts/new")) { request in + // THEN the app sends only the required parameters + try assertRequestBody(request, expected: """ + { + "author" : 29043, + "content" : "content-a", + "status" : "draft", + "title" : "title-a", + "type" : "post" + } + """) + var post = WordPressComPost.mock + post.excerpt = "hello" + return try HTTPStubsResponse(value: post, statusCode: 201) + } + + // WHEN saving the post + try await repository.sync(post) + + // THEN `postID` is saved to make sure new changes are saved using partial updates + XCTAssertEqual(post.postID, 974) + + // THEN local revision is presereved + XCTAssertNotNil(post.revision) + XCTAssertEqual(post.revision?.postTitle, "title-b") + + // THEN the rest of the post content is retained because we have a local revision + XCTAssertNil(post.mt_excerpt) + } } // MARK: - Helpers @@ -1009,7 +1322,7 @@ private enum PostRepositorySaveTestsError: Error { case unexpectedRequestBody(_ lhs: Any, _ rhs: Any) } -private extension HTTPStubsResponse { +extension HTTPStubsResponse { convenience init(value: T, statusCode: Int) throws { let data = try encoder.encode(value) self.init(data: data, statusCode: Int32(statusCode), headers: nil) @@ -1018,7 +1331,7 @@ private extension HTTPStubsResponse { // MARK: - Server -private struct WordPressComPost: Hashable, Codable { +struct WordPressComPost: Hashable, Codable { var id: Int var siteID: Int var date: Date @@ -1063,7 +1376,7 @@ private struct WordPressComPost: Hashable, Codable { }() } -private struct WordPressComAuthor: Hashable, Codable { +struct WordPressComAuthor: Hashable, Codable { var id: Int var login: String? var email: Bool? diff --git a/WordPress/WordPressTest/PostSyncStateViewModelTests.swift b/WordPress/WordPressTest/PostSyncStateViewModelTests.swift index a34c52dd3702..1eb684b49964 100644 --- a/WordPress/WordPressTest/PostSyncStateViewModelTests.swift +++ b/WordPress/WordPressTest/PostSyncStateViewModelTests.swift @@ -24,10 +24,10 @@ final class PostSyncStateViewModelTests: CoreDataTestCase { let post = PostBuilder(mainContext) .with(remoteStatus: .pushing) .build() - let viewModel = PostSyncStateViewModel(post: post, isInternetReachable: true) + let viewModel = PostSyncStateViewModel(post: post, isInternetReachable: true, isSyncPublishingEnabled: false) // When & Then - expect(viewModel.state).to(equal(.syncing)) + expect(viewModel.state).to(equal(.uploading)) expect(viewModel.isEditable).to(beFalse()) expect(viewModel.isShowingEllipsis).to(beFalse()) expect(viewModel.isShowingIndicator).to(beTrue()) @@ -39,7 +39,7 @@ final class PostSyncStateViewModelTests: CoreDataTestCase { let post = PostBuilder(mainContext) .with(remoteStatus: .failed) .build() - let viewModel = PostSyncStateViewModel(post: post, isInternetReachable: false) + let viewModel = PostSyncStateViewModel(post: post, isInternetReachable: false, isSyncPublishingEnabled: false) // When & Then expect(viewModel.state).to(equal(.offlineChanges)) @@ -54,7 +54,7 @@ final class PostSyncStateViewModelTests: CoreDataTestCase { let post = PostBuilder(mainContext) .with(remoteStatus: .failed) .build() - let viewModel = PostSyncStateViewModel(post: post, isInternetReachable: true) + let viewModel = PostSyncStateViewModel(post: post, isInternetReachable: true, isSyncPublishingEnabled: false) // When & Then expect(viewModel.state).to(equal(.failed)) diff --git a/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift b/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift index 109d4c3b09d2..0b80f92c5c5c 100644 --- a/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift +++ b/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift @@ -131,7 +131,7 @@ class PostCardStatusViewModelTests: CoreDataTestCase { func testReturnFailedMessageIfPostFailedAndThereIsConnectivity() { let post = PostBuilder(mainContext).revision().with(remoteStatus: .failed).confirmedAutoUpload().build() - let viewModel = PostCardStatusViewModel(post: post, isInternetReachable: true) + let viewModel = PostCardStatusViewModel(post: post, isInternetReachable: true, isSyncPublishingEnabled: false) expect(viewModel.status).to(equal(i18n("Upload failed"))) expect(viewModel.statusColor).to(equal(.error)) @@ -142,7 +142,7 @@ class PostCardStatusViewModelTests: CoreDataTestCase { func testReturnWillUploadLaterMessageIfPostFailedAndThereIsConnectivity() { let post = PostBuilder(mainContext).revision().with(remoteStatus: .failed).confirmedAutoUpload().build() - let viewModel = PostCardStatusViewModel(post: post, isInternetReachable: false) + let viewModel = PostCardStatusViewModel(post: post, isInternetReachable: false, isSyncPublishingEnabled: false) expect(viewModel.status).to(equal(i18n("We'll publish the post when your device is back online."))) expect(viewModel.statusColor).to(equal(.warning))