From 0ac6d8e2153bec4ddd4e983915da6db09fcbed05 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 7 Nov 2023 13:29:47 +0100 Subject: [PATCH] Fix syncing empty favorites folders (#546) Task/Issue URL: https://app.asana.com/0/414235014887631/1205843304285892/f Description: Update bookmarks sync response handler to actually process favorites also when an empty folder is received from the server. --- .../internal/BookmarksResponseHandler.swift | 51 ++++++++++--------- ...marksRegularSyncResponseHandlerTests.swift | 48 +++++++++++++++++ 2 files changed, 76 insertions(+), 23 deletions(-) diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index 451178aec..5c69cb8c8 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -36,7 +36,7 @@ final class BookmarksResponseHandler { let topLevelFoldersSyncables: [SyncableBookmarkAdapter] let bookmarkSyncablesWithoutParent: [SyncableBookmarkAdapter] - let favoritesUUIDs: [String] + let favoritesUUIDs: [String]? var entitiesByUUID: [String: BookmarkEntity] = [:] var idsOfItemsThatRetainModifiedAt = Set() @@ -57,7 +57,7 @@ final class BookmarksResponseHandler { var allUUIDs: Set = [] var childrenToParents: [String: String] = [:] var parentFoldersToChildren: [String: [String]] = [:] - var favoritesUUIDs: [String] = [] + var favoritesUUIDs: [String]? self.received.forEach { syncable in guard let uuid = syncable.uuid else { @@ -113,33 +113,38 @@ final class BookmarksResponseHandler { } try processOrphanedBookmarks() - // populate favorites - if !favoritesUUIDs.isEmpty { - guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) else { - // Error - unable to process favorites - return - } + processReceivedFavorites() + } - // For non-first sync we rely fully on the server response - if !shouldDeduplicateEntities { - favoritesFolder.favoritesArray.forEach { $0.removeFromFavorites() } - } else if !favoritesFolder.favoritesArray.isEmpty { - // If we're deduplicating and there are favorires locally, we'll need to sync favorites folder back later. - // Let's keep its modifiedAt. - idsOfItemsThatRetainModifiedAt.insert(BookmarkEntity.Constants.favoritesFolderID) - } + // MARK: - Private - favoritesUUIDs.forEach { uuid in - if let bookmark = entitiesByUUID[uuid] { - bookmark.removeFromFavorites() - bookmark.addToFavorites(favoritesRoot: favoritesFolder) - } + private func processReceivedFavorites() { + guard let favoritesUUIDs else { + return + } + + guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) else { + // Error - unable to process favorites + return + } + + // For non-first sync we rely fully on the server response + if !shouldDeduplicateEntities { + favoritesFolder.favoritesArray.forEach { $0.removeFromFavorites() } + } else if !favoritesFolder.favoritesArray.isEmpty { + // If we're deduplicating and there are favorites locally, we'll need to sync favorites folder back later. + // Let's keep its modifiedAt. + idsOfItemsThatRetainModifiedAt.insert(BookmarkEntity.Constants.favoritesFolderID) + } + + favoritesUUIDs.forEach { uuid in + if let bookmark = entitiesByUUID[uuid] { + bookmark.removeFromFavorites() + bookmark.addToFavorites(favoritesRoot: favoritesFolder) } } } - // MARK: - Private - private func processTopLevelFolder(_ topLevelFolderSyncable: SyncableBookmarkAdapter) throws { guard let topLevelFolderUUID = topLevelFolderSyncable.uuid else { return diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index 424873249..211cb3c3d 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -158,6 +158,54 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase }) } + func testWhenPayloadDoesNotContainFavoritesFolderThenFavoritesAreNotAffected() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", isFavorite: true) + Folder(id: "2") { + Bookmark(id: "3", isFavorite: true) + } + } + + let received: [Syncable] = [ + .rootFolder(children: ["1", "2", "4"]), + .bookmark(id: "4") + ] + + let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", isFavorite: true) + Folder(id: "2") { + Bookmark(id: "3", isFavorite: true) + } + Bookmark(id: "4") + }) + } + + func testWhenPayloadContainsEmptyFavoritesFolderThenAllFavoritesAreRemoved() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", isFavorite: true) + Folder(id: "2") { + Bookmark(id: "3", isFavorite: true) + } + } + + let received: [Syncable] = [ + .favoritesFolder(favorites: []) + ] + + let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + Folder(id: "2") { + Bookmark(id: "3") + } + }) + } + func testThatSinglePayloadCanCreateReorderAndOrphanBookmarks() async throws { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType)