From df433e7db9ae1af522cdd7c37d9a5e0eb0e5ede2 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 8 Sep 2023 10:29:20 +0200 Subject: [PATCH 01/56] Add to-many relationship for favorites folders --- .../Bookmarks/BookmarkEditorViewModel.swift | 9 +- Sources/Bookmarks/BookmarkEntity.swift | 87 ++++++++++++++++--- Sources/Bookmarks/BookmarkListViewModel.swift | 14 ++- Sources/Bookmarks/BookmarkUtils.swift | 10 +++ .../.xccurrentversion | 2 +- .../BookmarksModel 4.xcdatamodel/contents | 26 ++++++ Sources/Bookmarks/FavoriteListViewModel.swift | 30 +++++-- .../BookmarkCoreDataImporter.swift | 4 +- .../Bookmarks/MenuBookmarksViewModel.swift | 34 ++++++-- .../BookmarkListViewModelTests.swift | 7 +- .../FavoriteListViewModelTests.swift | 1 + 11 files changed, 188 insertions(+), 36 deletions(-) create mode 100644 Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index b7d8cb1ea..018e63730 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -31,6 +31,7 @@ public class BookmarkEditorViewModel: ObservableObject { } let context: NSManagedObjectContext + let favoritesConfiguration: FavoritesConfiguration @Published public var bookmark: BookmarkEntity @Published public var locations = [Location]() @@ -59,11 +60,13 @@ public class BookmarkEditorViewModel: ObservableObject { public init(editingEntityID: NSManagedObjectID, bookmarksDatabase: CoreDataDatabase, + favoritesConfiguration: FavoritesConfiguration, errorEvents: EventMapping?) { externalUpdates = subject.eraseToAnyPublisher() self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) + self.favoritesConfiguration = favoritesConfiguration guard let entity = context.object(with: editingEntityID) as? BookmarkEntity else { // For sync, this is valid scenario in case of a timing issue @@ -84,11 +87,13 @@ public class BookmarkEditorViewModel: ObservableObject { public init(creatingFolderWithParentID parentFolderID: NSManagedObjectID?, bookmarksDatabase: CoreDataDatabase, + favoritesConfiguration: FavoritesConfiguration, errorEvents: EventMapping?) { externalUpdates = subject.eraseToAnyPublisher() self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) + self.favoritesConfiguration = favoritesConfiguration let parent: BookmarkEntity? if let parentFolderID = parentFolderID { @@ -173,12 +178,12 @@ public class BookmarkEditorViewModel: ObservableObject { } public func removeFromFavorites() { - assert(bookmark.isFavorite) + assert(bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform)) bookmark.removeFromFavorites() } public func addToFavorites() { - assert(!bookmark.isFavorite) + assert(!bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform)) bookmark.addToFavorites(favoritesRoot: favoritesFolder) } diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index f73d03592..09c126a0c 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -21,12 +21,51 @@ import Foundation import CoreData +public enum FavoritesConfiguration { + case displayNative(FavoritesPlatform) + case displayAll(native: FavoritesPlatform) + + var displayedPlatform: FavoritesPlatform { + switch self { + case .displayNative(let platform): + return platform + case .displayAll: + return .all + } + } + + var nativePlatform: FavoritesPlatform { + switch self { + case .displayNative(let native), .displayAll(let native): + return native + } + } +} + +public enum FavoritesPlatform: String { + case mobile = "mobile_favorites_root" + case desktop = "desktop_favorites_root" + case all = "favorites_root" +} + @objc(BookmarkEntity) public class BookmarkEntity: NSManagedObject { public enum Constants { public static let rootFolderID = "bookmarks_root" public static let favoritesFolderID = "favorites_root" + public static let mobileFavoritesFolderID = "mobile_favorites_root" + public static let desktopFavoritesFolderID = "desktop_favorites_root" + + static let favoriteFoldersIDs: Set = [ + desktopFavoritesFolderID, + mobileFavoritesFolderID, + favoritesFolderID + ] + } + + static func isValidFavoritesFolderID(_ value: String) -> Bool { + return Constants.favoriteFoldersIDs.contains(value) } public enum Error: Swift.Error { @@ -49,7 +88,7 @@ public class BookmarkEntity: NSManagedObject { @NSManaged public var url: String? @NSManaged public var uuid: String? @NSManaged public var children: NSOrderedSet? - @NSManaged fileprivate(set) public var favoriteFolder: BookmarkEntity? + @NSManaged public fileprivate(set) var favoriteFolders: NSSet? @NSManaged public fileprivate(set) var favorites: NSOrderedSet? @NSManaged public var parent: BookmarkEntity? @@ -58,8 +97,12 @@ public class BookmarkEntity: NSManagedObject { /// In-memory flag. When set to `false`, disables adjusting `modifiedAt` on `willSave()`. It's reset to `true` on `didSave()`. public var shouldManageModifiedAt: Bool = true - public var isFavorite: Bool { - favoriteFolder != nil + public func isFavorite(on platform: FavoritesPlatform) -> Bool { + favoriteFoldersSet.contains { $0.uuid == platform.rawValue } + } + + public var favoritedOn: [FavoritesPlatform] { + favoriteFoldersSet.compactMap(\.uuid).compactMap(FavoritesPlatform.init) } public convenience init(context moc: NSManagedObjectContext) { @@ -123,6 +166,10 @@ public class BookmarkEntity: NSManagedObject { return children.filter { $0.isPendingDeletion == false } } + public var favoriteFoldersSet: Set { + return favoriteFolders.flatMap(Set.init) ?? [] + } + public static func makeFolder(title: String, parent: BookmarkEntity, insertAtBeginning: Bool = false, @@ -170,7 +217,10 @@ public class BookmarkEntity: NSManagedObject { } public func removeFromFavorites() { - favoriteFolder = nil + guard let favoriteFolders else { + return + } + removeFromFavoriteFolders(favoriteFolders) } public func markPendingDeletion() { @@ -200,20 +250,12 @@ extension BookmarkEntity { func validate() throws { try validateThatFoldersDoNotHaveURLs() try validateThatFolderHierarchyHasNoCycles() - try validateFavoritesStatus() try validateFavoritesFolder() } - func validateFavoritesStatus() throws { - let isInFavoriteCollection = favoriteFolder != nil - if isFavorite != isInFavoriteCollection { - throw Error.invalidFavoritesStatus - } - } - func validateFavoritesFolder() throws { - if let favoritesFolderID = favoriteFolder?.uuid, - favoritesFolderID != Constants.favoritesFolderID { + let uuids = Set(favoriteFoldersSet.compactMap(\.uuid)) + guard uuids.isSubset(of: Constants.favoriteFoldersIDs) else { throw Error.invalidFavoritesFolder } } @@ -296,6 +338,23 @@ extension BookmarkEntity { } +// MARK: Generated accessors for favoriteFolders +extension BookmarkEntity { + + @objc(addFavoriteFoldersObject:) + @NSManaged private func addToFavoriteFolders(_ value: BookmarkEntity) + + @objc(removeFavoriteFoldersObject:) + @NSManaged private func removeFromFavoriteFolders(_ value: BookmarkEntity) + + @objc(addFavoriteFolders:) + @NSManaged private func addToFavoriteFolders(_ values: NSSet) + + @objc(removeFavoriteFolders:) + @NSManaged private func removeFromFavoriteFolders(_ values: NSSet) + +} + extension BookmarkEntity: Identifiable { } diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 858d0c419..e97393282 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -27,6 +27,7 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public let currentFolder: BookmarkEntity? let context: NSManagedObjectContext + let favoritesConfiguration: FavoritesConfiguration public var bookmarks = [BookmarkEntity]() @@ -39,12 +40,14 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { private let errorEvents: EventMapping? public init(bookmarksDatabase: CoreDataDatabase, + favoritesConfiguration: FavoritesConfiguration, parentID: NSManagedObjectID?, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) + self.favoritesConfiguration = favoritesConfiguration if let parentID = parentID { if let bookmark = (try? context.existingObject(with: parentID)) as? BookmarkEntity { @@ -109,10 +112,15 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { } public func toggleFavorite(_ bookmark: BookmarkEntity) { - if bookmark.isFavorite { + if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { bookmark.removeFromFavorites() - } else if let folder = BookmarkUtils.fetchFavoritesFolder(context) { - bookmark.addToFavorites(favoritesRoot: folder) + } else { + let favoriteFoldersUUIDs: Set = [favoritesConfiguration.displayedPlatform.rawValue, favoritesConfiguration.nativePlatform.rawValue] + for uuid in favoriteFoldersUUIDs { + if let folder = BookmarkUtils.fetchFavoritesFolder(withUUID: uuid, in: context) { + bookmark.addToFavorites(favoritesRoot: folder) + } + } } save() } diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 4d47b2bc2..3f257b702 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -30,6 +30,16 @@ public struct BookmarkUtils { return try? context.fetch(request).first } + public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { + assert(BookmarkEntity.isValidFavoritesFolderID(uuid)) + let request = BookmarkEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K == %@", #keyPath(BookmarkEntity.uuid), uuid) + request.returnsObjectsAsFaults = false + request.fetchLimit = 1 + + return try? context.fetch(request).first + } + public static func fetchFavoritesFolder(_ context: NSManagedObjectContext) -> BookmarkEntity? { let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate(format: "%K == %@", #keyPath(BookmarkEntity.uuid), BookmarkEntity.Constants.favoritesFolderID) diff --git a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion index f6574b696..9ebe855d9 100644 --- a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion +++ b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion @@ -3,6 +3,6 @@ _XCCurrentVersionName - BookmarksModel 3.xcdatamodel + BookmarksModel 4.xcdatamodel diff --git a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents new file mode 100644 index 000000000..1781bec32 --- /dev/null +++ b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 291135443..9c299ceab 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -25,6 +25,7 @@ import Common public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject { let context: NSManagedObjectContext + let favoritesConfiguration: FavoritesConfiguration public var favorites = [BookmarkEntity]() @@ -36,11 +37,25 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject private let errorEvents: EventMapping? + private var _favoritesFolder: BookmarkEntity? + private var favoriteFolder: BookmarkEntity? { + if _favoritesFolder == nil { + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesConfiguration.displayedPlatform.rawValue, in: context) + + if _favoritesFolder == nil { + errorEvents?.fire(.fetchingRootItemFailed(.favorites)) + } + } + return _favoritesFolder + } + public init(bookmarksDatabase: CoreDataDatabase, + favoritesConfiguration: FavoritesConfiguration, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() self.errorEvents = errorEvents + self.favoritesConfiguration = favoritesConfiguration self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) refresh() @@ -78,13 +93,13 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject } private func refresh() { - guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) else { + guard let favoriteFolder else { errorEvents?.fire(.fetchingRootItemFailed(.favorites)) favorites = [] return } - readFavorites(with: favoritesFolder) + readFavorites(with: favoriteFolder) } public func favorite(at index: Int) -> BookmarkEntity? { @@ -97,8 +112,8 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject } public func removeFavorite(_ favorite: BookmarkEntity) { - guard let favoriteFolder = favorite.favoriteFolder else { - errorEvents?.fire(.missingParent(.favorite)) + guard let favoriteFolder else { + errorEvents?.fire(.fetchingRootItemFailed(.favorites)) return } @@ -112,8 +127,8 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject public func moveFavorite(_ favorite: BookmarkEntity, fromIndex: Int, toIndex: Int) { - guard let favoriteFolder = favorite.favoriteFolder else { - errorEvents?.fire(.missingParent(.favorite)) + guard let favoriteFolder else { + errorEvents?.fire(.fetchingRootItemFailed(.favorites)) return } @@ -160,7 +175,6 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject } private func readFavorites(with favoritesFolder: BookmarkEntity) { - favorites = (favoritesFolder.favorites?.array as? [BookmarkEntity] ?? []) - .filter { !$0.isPendingDeletion } + favorites = favoritesFolder.favoritesArray } } diff --git a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift index 74ba5eccd..f373363a7 100644 --- a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift +++ b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift @@ -23,9 +23,11 @@ import Persistence public class BookmarkCoreDataImporter { let context: NSManagedObjectContext + let favoritesConfiguration: FavoritesConfiguration - public init(database: CoreDataDatabase) { + public init(database: CoreDataDatabase, favoritesConfiguration: FavoritesConfiguration) { self.context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + self.favoritesConfiguration = favoritesConfiguration } public func importBookmarks(_ bookmarks: [BookmarkOrFolder]) async throws { diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index a36ad8621..0310e98b9 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -24,7 +24,8 @@ import Persistence public class MenuBookmarksViewModel: MenuBookmarksInteracting { let context: NSManagedObjectContext - + let favoritesConfiguration: FavoritesConfiguration + private var _rootFolder: BookmarkEntity? private var rootFolder: BookmarkEntity? { if _rootFolder == nil { @@ -40,7 +41,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { private var _favoritesFolder: BookmarkEntity? private var favoritesFolder: BookmarkEntity? { if _favoritesFolder == nil { - _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesConfiguration.displayedPlatform.rawValue, in: context) if _favoritesFolder == nil { errorEvents?.fire(.fetchingRootItemFailed(.menu)) @@ -49,12 +50,26 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { return _favoritesFolder } + private var _nativeFavoritesFolder: BookmarkEntity? + private var nativeFavoritesFolder: BookmarkEntity? { + if _nativeFavoritesFolder == nil { + _nativeFavoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesConfiguration.nativePlatform.rawValue, in: context) + + if _nativeFavoritesFolder == nil { + errorEvents?.fire(.fetchingRootItemFailed(.menu)) + } + } + return _nativeFavoritesFolder + } + private var observer: NSObjectProtocol? private let errorEvents: EventMapping? public init(bookmarksDatabase: CoreDataDatabase, + favoritesConfiguration: FavoritesConfiguration, errorEvents: EventMapping?) { + self.favoritesConfiguration = favoritesConfiguration self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) registerForChanges() @@ -93,6 +108,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { public func createOrToggleFavorite(title: String, url: URL) { guard let favoritesFolder = favoritesFolder, + let nativeFavoritesFolder = nativeFavoritesFolder, let rootFolder = rootFolder else { return } @@ -100,10 +116,11 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { let queriedBookmark = favorite(for: url) ?? bookmark(for: url) if let bookmark = queriedBookmark { - if bookmark.isFavorite { + if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { bookmark.removeFromFavorites() } else { bookmark.addToFavorites(favoritesRoot: favoritesFolder) + bookmark.addToFavorites(favoritesRoot: nativeFavoritesFolder) } } else { let favorite = BookmarkEntity.makeBookmark(title: title, @@ -111,6 +128,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { parent: rootFolder, context: context) favorite.addToFavorites(favoritesRoot: favoritesFolder) + favorite.addToFavorites(favoritesRoot: nativeFavoritesFolder) } save() @@ -128,10 +146,14 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { } public func favorite(for url: URL) -> BookmarkEntity? { - BookmarkUtils.fetchBookmark(for: url, + guard let favoritesFolder else { + return nil + } + return BookmarkUtils.fetchBookmark(for: url, predicate: NSPredicate( - format: "%K != nil AND %K == NO", - #keyPath(BookmarkEntity.favoriteFolder), + format: "ANY %K CONTAINS %@ AND %K == NO", + #keyPath(BookmarkEntity.favoriteFolders), + favoritesFolder, #keyPath(BookmarkEntity.isPendingDeletion) ), context: context) diff --git a/Tests/BookmarksTests/BookmarkListViewModelTests.swift b/Tests/BookmarksTests/BookmarkListViewModelTests.swift index 0776e2add..e0fb3b09d 100644 --- a/Tests/BookmarksTests/BookmarkListViewModelTests.swift +++ b/Tests/BookmarksTests/BookmarkListViewModelTests.swift @@ -65,7 +65,12 @@ final class BookmarkListViewModelTests: XCTestCase { try! context.save() } - bookmarkListViewModel = BookmarkListViewModel(bookmarksDatabase: bookmarksDatabase, parentID: nil, errorEvents: eventMapping) + bookmarkListViewModel = BookmarkListViewModel( + bookmarksDatabase: bookmarksDatabase, + favoritesConfiguration: .displayNative(.mobile), + parentID: nil, + errorEvents: eventMapping + ) } override func tearDown() { diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index 49a4ccae3..d28146fcf 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -53,6 +53,7 @@ final class FavoriteListViewModelTests: XCTestCase { } favoriteListViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, + favoritesConfiguration: .displayNative(.mobile), errorEvents: eventMapping) } From 214da43908546af369467fc5a48868e35f9dfb5f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 8 Sep 2023 17:17:23 +0200 Subject: [PATCH 02/56] Update addToFavorites API usage --- .../Bookmarks/BookmarkEditorViewModel.swift | 8 +++- Sources/Bookmarks/BookmarkEntity.swift | 18 +++++-- Sources/Bookmarks/BookmarkListViewModel.swift | 8 +--- Sources/Bookmarks/BookmarkUtils.swift | 23 ++++----- .../BookmarkCoreDataImporter.swift | 16 ++++--- .../Bookmarks/MenuBookmarksViewModel.swift | 6 +-- .../BookmarksTestsUtils/BookmarkTree.swift | 6 ++- .../internal/BookmarksResponseHandler.swift | 48 ++++++++++--------- .../BookmarksTests/BookmarkEntityTests.swift | 4 +- .../FavoriteListViewModelTests.swift | 3 +- ...marksInitialSyncResponseHandlerTests.swift | 3 +- .../Bookmarks/BookmarksProviderTests.swift | 3 +- 12 files changed, 81 insertions(+), 65 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index 018e63730..2f51c7b19 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -36,7 +36,10 @@ public class BookmarkEditorViewModel: ObservableObject { @Published public var bookmark: BookmarkEntity @Published public var locations = [Location]() - lazy var favoritesFolder: BookmarkEntity! = BookmarkUtils.fetchFavoritesFolder(context) + lazy var favoritesFolder: BookmarkEntity! = BookmarkUtils.fetchFavoritesFolder( + withUUID: favoritesConfiguration.displayedPlatform.rawValue, + in: context + ) private var observer: NSObjectProtocol? private let subject = PassthroughSubject() @@ -184,7 +187,8 @@ public class BookmarkEditorViewModel: ObservableObject { public func addToFavorites() { assert(!bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform)) - bookmark.addToFavorites(favoritesRoot: favoritesFolder) + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesConfiguration, in: context) + bookmark.addToFavorites(folders: folders) } public func setParentWithID(_ parentID: NSManagedObjectID) { diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 09c126a0c..a7599ee8e 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -40,6 +40,10 @@ public enum FavoritesConfiguration { return native } } + + var folderUUIDs: Set { + return [displayedPlatform.rawValue, nativePlatform.rawValue] + } } public enum FavoritesPlatform: String { @@ -57,14 +61,14 @@ public class BookmarkEntity: NSManagedObject { public static let mobileFavoritesFolderID = "mobile_favorites_root" public static let desktopFavoritesFolderID = "desktop_favorites_root" - static let favoriteFoldersIDs: Set = [ + public static let favoriteFoldersIDs: Set = [ desktopFavoritesFolderID, mobileFavoritesFolderID, favoritesFolderID ] } - static func isValidFavoritesFolderID(_ value: String) -> Bool { + public static func isValidFavoritesFolderID(_ value: String) -> Bool { return Constants.favoriteFoldersIDs.contains(value) } @@ -124,7 +128,7 @@ public class BookmarkEntity: NSManagedObject { guard !changedKeys.isEmpty, !changedKeys.contains(NSStringFromSelector(#selector(getter: modifiedAt))) else { return } - if isInserted && (uuid == Constants.rootFolderID || uuid == Constants.favoritesFolderID) { + if isInserted && (uuid == Constants.rootFolderID || uuid.flatMap(Constants.favoriteFoldersIDs.contains) == true) { return } modifiedAt = Date() @@ -215,7 +219,13 @@ public class BookmarkEntity: NSManagedObject { root.addToFavorites(self) } } - + + public func addToFavorites(folders: [BookmarkEntity]) { + for root in folders { + root.addToFavorites(self) + } + } + public func removeFromFavorites() { guard let favoriteFolders else { return diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index e97393282..20daca329 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -115,12 +115,8 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { bookmark.removeFromFavorites() } else { - let favoriteFoldersUUIDs: Set = [favoritesConfiguration.displayedPlatform.rawValue, favoritesConfiguration.nativePlatform.rawValue] - for uuid in favoriteFoldersUUIDs { - if let folder = BookmarkUtils.fetchFavoritesFolder(withUUID: uuid, in: context) { - bookmark.addToFavorites(favoritesRoot: folder) - } - } + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesConfiguration, in: context) + bookmark.addToFavorites(folders: folders) } save() } diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 3f257b702..0ddadb7dd 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -30,6 +30,10 @@ public struct BookmarkUtils { return try? context.fetch(request).first } + public static func fetchFavoritesFolders(for configuration: FavoritesConfiguration, in context: NSManagedObjectContext) -> [BookmarkEntity] { + configuration.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } + } + public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { assert(BookmarkEntity.isValidFavoritesFolderID(uuid)) let request = BookmarkEntity.fetchRequest() @@ -40,21 +44,12 @@ public struct BookmarkUtils { return try? context.fetch(request).first } - public static func fetchFavoritesFolder(_ context: NSManagedObjectContext) -> BookmarkEntity? { - let request = BookmarkEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K == %@", #keyPath(BookmarkEntity.uuid), BookmarkEntity.Constants.favoritesFolderID) - request.returnsObjectsAsFaults = false - request.fetchLimit = 1 - - return try? context.fetch(request).first - } - public static func fetchOrphanedEntities(_ context: NSManagedObjectContext) -> [BookmarkEntity] { let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate( format: "NOT %K IN %@ AND %K == NO AND %K == nil", #keyPath(BookmarkEntity.uuid), - [BookmarkEntity.Constants.rootFolderID, BookmarkEntity.Constants.favoritesFolderID], + BookmarkEntity.Constants.favoriteFoldersIDs.union([BookmarkEntity.Constants.rootFolderID]), #keyPath(BookmarkEntity.isPendingDeletion), #keyPath(BookmarkEntity.parent) ) @@ -77,9 +72,11 @@ public struct BookmarkUtils { if fetchRootFolder(context) == nil { insertRootFolder(uuid: BookmarkEntity.Constants.rootFolderID, into: context) } - - if fetchFavoritesFolder(context) == nil { - insertRootFolder(uuid: BookmarkEntity.Constants.favoritesFolderID, into: context) + + for uuid in BookmarkEntity.Constants.favoriteFoldersIDs { + if fetchFavoritesFolder(withUUID: uuid, in: context) == nil { + insertRootFolder(uuid: uuid, into: context) + } } } diff --git a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift index f373363a7..279832d9b 100644 --- a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift +++ b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift @@ -36,8 +36,12 @@ public class BookmarkCoreDataImporter { context.performAndWait { () -> Void in do { + let favoritesFolderUUIDs = favoritesConfiguration.folderUUIDs + let favoritesFolders = favoritesFolderUUIDs.compactMap { BookmarkUtils.fetchFavoritesFolder(withUUID: $0, in: context) } + guard let topLevelBookmarksFolder = BookmarkUtils.fetchRootFolder(context), - let topLevelFavoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) else { + favoritesFolders.count == favoritesFolderUUIDs.count + else { throw BookmarksCoreDataError.fetchingExistingItemFailed } @@ -45,7 +49,7 @@ public class BookmarkCoreDataImporter { try recursivelyCreateEntities(from: bookmarks, parent: topLevelBookmarksFolder, - favoritesRoot: topLevelFavoritesFolder, + favoritesFolders: favoritesFolders, bookmarkURLToIDMap: &bookmarkURLToIDMap) try context.save() continuation.resume() @@ -87,7 +91,7 @@ public class BookmarkCoreDataImporter { private func recursivelyCreateEntities(from bookmarks: [BookmarkOrFolder], parent: BookmarkEntity, - favoritesRoot: BookmarkEntity, + favoritesFolders: [BookmarkEntity], bookmarkURLToIDMap: inout [String: NSManagedObjectID]) throws { for bookmarkOrFolder in bookmarks { if bookmarkOrFolder.isInvalidBookmark { @@ -102,20 +106,20 @@ public class BookmarkCoreDataImporter { if let children = bookmarkOrFolder.children { try recursivelyCreateEntities(from: children, parent: folder, - favoritesRoot: favoritesRoot, + favoritesFolders: favoritesFolders, bookmarkURLToIDMap: &bookmarkURLToIDMap) } case .favorite: if let url = bookmarkOrFolder.url { if let objectID = bookmarkURLToIDMap[url.absoluteString], let bookmark = try? context.existingObject(with: objectID) as? BookmarkEntity { - bookmark.addToFavorites(favoritesRoot: favoritesRoot) + bookmark.addToFavorites(folders: favoritesFolders) } else { let newFavorite = BookmarkEntity.makeBookmark(title: bookmarkOrFolder.name, url: url.absoluteString, parent: parent, context: context) - newFavorite.addToFavorites(favoritesRoot: favoritesRoot) + newFavorite.addToFavorites(folders: favoritesFolders) bookmarkURLToIDMap[url.absoluteString] = newFavorite.objectID } } diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 0310e98b9..7fc7a622a 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -119,16 +119,14 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { bookmark.removeFromFavorites() } else { - bookmark.addToFavorites(favoritesRoot: favoritesFolder) - bookmark.addToFavorites(favoritesRoot: nativeFavoritesFolder) + bookmark.addToFavorites(folders: [favoritesFolder, nativeFavoritesFolder]) } } else { let favorite = BookmarkEntity.makeBookmark(title: title, url: url.absoluteString, parent: rootFolder, context: context) - favorite.addToFavorites(favoritesRoot: favoritesFolder) - favorite.addToFavorites(favoritesRoot: nativeFavoritesFolder) + favorite.addToFavorites(folders: [favoritesFolder, nativeFavoritesFolder]) } save() diff --git a/Sources/BookmarksTestsUtils/BookmarkTree.swift b/Sources/BookmarksTestsUtils/BookmarkTree.swift index 50d3526d4..f8b803d39 100644 --- a/Sources/BookmarksTestsUtils/BookmarkTree.swift +++ b/Sources/BookmarksTestsUtils/BookmarkTree.swift @@ -203,7 +203,8 @@ public struct BookmarkTree { public func createEntitiesForCheckingModifiedAt(in context: NSManagedObjectContext) -> (BookmarkEntity, [BookmarkEntity], [String: ModifiedAtConstraint]) { let rootFolder = BookmarkUtils.fetchRootFolder(context)! rootFolder.modifiedAt = modifiedAt - let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context)! + // todo + let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context)! var orphans = [BookmarkEntity]() var modifiedAtConstraints = [String: ModifiedAtConstraint]() if let modifiedAtConstraint { @@ -341,7 +342,8 @@ public extension XCTestCase { XCTAssertEqual(expectedNode.isFolder, thisNode.isFolder, "isFolder mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.isPendingDeletion, thisNode.isPendingDeletion, "isPendingDeletion mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.children?.count, thisNode.children?.count, "children count mismatch for \(thisUUID)", file: file, line: line) - XCTAssertEqual(expectedNode.isFavorite, thisNode.isFavorite, "isFavorite mismatch for \(thisUUID)", file: file, line: line) + // todo + XCTAssertEqual(expectedNode.isFavorite(on: .all), thisNode.isFavorite(on: .all), "isFavorite mismatch for \(thisUUID)", file: file, line: line) if withTimestamps { if let modifiedAtConstraint = modifiedAtConstraints[thisUUID] { modifiedAtConstraint.check(thisNode.modifiedAt) diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index 51dab4d10..c8bfc30e2 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -33,7 +33,7 @@ final class BookmarksResponseHandler { let topLevelFoldersSyncables: [SyncableBookmarkAdapter] let bookmarkSyncablesWithoutParent: [SyncableBookmarkAdapter] - let favoritesUUIDs: [String] + let favoritesUUIDsByFolderUUID: [String: [String]] var entitiesByUUID: [String: BookmarkEntity] = [:] var idsOfItemsThatRetainModifiedAt = Set() @@ -54,7 +54,7 @@ final class BookmarksResponseHandler { var allUUIDs: Set = [] var childrenToParents: [String: String] = [:] var parentFoldersToChildren: [String: [String]] = [:] - var favoritesUUIDs: [String] = [] + var favoritesUUIDsByFolderUUID: [String: [String]] = [:] self.received.forEach { syncable in guard let uuid = syncable.uuid else { @@ -67,8 +67,8 @@ final class BookmarksResponseHandler { allUUIDs.formUnion(syncable.children) } - if uuid == BookmarkEntity.Constants.favoritesFolderID { - favoritesUUIDs = syncable.children + if BookmarkEntity.isValidFavoritesFolderID(uuid) { + favoritesUUIDsByFolderUUID[uuid] = syncable.children } else { if syncable.isFolder { parentFoldersToChildren[uuid] = syncable.children @@ -81,13 +81,13 @@ final class BookmarksResponseHandler { self.allReceivedIDs = allUUIDs self.receivedByUUID = syncablesByUUID - self.favoritesUUIDs = favoritesUUIDs + self.favoritesUUIDsByFolderUUID = favoritesUUIDsByFolderUUID let foldersWithoutParent = Set(parentFoldersToChildren.keys).subtracting(childrenToParents.keys) topLevelFoldersSyncables = foldersWithoutParent.compactMap { syncablesByUUID[$0] } bookmarkSyncablesWithoutParent = allUUIDs.subtracting(childrenToParents.keys) - .subtracting(foldersWithoutParent + [BookmarkEntity.Constants.favoritesFolderID]) + .subtracting(foldersWithoutParent.union(BookmarkEntity.Constants.favoriteFoldersIDs)) .compactMap { syncablesByUUID[$0] } BookmarkEntity.fetchBookmarks(with: allReceivedIDs, in: context) @@ -110,25 +110,27 @@ final class BookmarksResponseHandler { try processOrphanedBookmarks() // populate favorites - if !favoritesUUIDs.isEmpty { - guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) else { - // Error - unable to process favorites - return - } + for (favoritesFolderUUID, favoritesUUIDs) in favoritesUUIDsByFolderUUID { + if !favoritesUUIDs.isEmpty { + guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesFolderUUID, in: 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 favorires locally, we'll need to sync favorites folder back later. - // Let's keep its modifiedAt. - idsOfItemsThatRetainModifiedAt.insert(BookmarkEntity.Constants.favoritesFolderID) - } + // 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(favoritesFolderUUID) + } - favoritesUUIDs.forEach { uuid in - if let bookmark = entitiesByUUID[uuid] { - bookmark.removeFromFavorites() - bookmark.addToFavorites(favoritesRoot: favoritesFolder) + favoritesUUIDs.forEach { uuid in + if let bookmark = entitiesByUUID[uuid] { + bookmark.removeFromFavorites() + bookmark.addToFavorites(favoritesRoot: favoritesFolder) + } } } } diff --git a/Tests/BookmarksTests/BookmarkEntityTests.swift b/Tests/BookmarksTests/BookmarkEntityTests.swift index bc08ea5de..7583c758a 100644 --- a/Tests/BookmarksTests/BookmarkEntityTests.swift +++ b/Tests/BookmarksTests/BookmarkEntityTests.swift @@ -56,10 +56,10 @@ final class BookmarkEntityTests: XCTestCase { try! context.save() let rootFolder = BookmarkUtils.fetchRootFolder(context)! - let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context)! + let favoritesFolders = BookmarkEntity.Constants.favoriteFoldersIDs.map { BookmarkUtils.fetchFavoritesFolder(withUUID: $0, in: context)! } XCTAssertNil(rootFolder.modifiedAt) - XCTAssertNil(favoritesFolder.modifiedAt) + XCTAssertTrue(favoritesFolders.allSatisfy { $0.modifiedAt == nil }) } } diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index d28146fcf..2bfcf5ba6 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -82,7 +82,8 @@ final class FavoriteListViewModelTests: XCTestCase { try! context.save() - let rootFavoriteFolder = BookmarkUtils.fetchFavoritesFolder(context)! + let favoriteFolderUUID = favoriteListViewModel.favoritesConfiguration.displayedPlatform.rawValue + let rootFavoriteFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoriteFolderUUID, in: context)! XCTAssertEqual(rootFavoriteFolder.favoritesArray.map(\.title), ["2", "4"]) let bookmark = BookmarkEntity.fetchBookmark(withUUID: "2", context: context)! diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift index 0f223b06e..527362d91 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift @@ -367,7 +367,8 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase var favoritesFolder: BookmarkEntity! context.performAndWait { - favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context) + // todo + favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context) } XCTAssertNotNil(favoritesFolder.modifiedAt) } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index deb15d360..cbcf0248c 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -74,7 +74,8 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { Bookmark("Bookmark 6", id: "6", modifiedAtConstraint: .notNil()) }) - let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(context)! + // todo + let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context)! XCTAssertNotNil(favoritesFolder.modifiedAt) } } From 7d9e0ed031d68a39e6364d73736053bc9d3495e9 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 8 Sep 2023 18:04:51 +0200 Subject: [PATCH 03/56] Update BookmarkTree and fix unit tests to support form factor specific favorites --- Sources/Bookmarks/BookmarkEntity.swift | 9 ++++- .../BookmarksTestsUtils/BookmarkTree.swift | 34 ++++++++++--------- .../FavoriteListViewModelTests.swift | 8 ++--- ...marksInitialSyncResponseHandlerTests.swift | 30 ++++++++-------- .../Bookmarks/BookmarksProviderTests.swift | 10 +++--- ...marksRegularSyncResponseHandlerTests.swift | 16 ++++----- .../helpers/SyncableBookmarksExtension.swift | 8 +++++ 7 files changed, 66 insertions(+), 49 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index a7599ee8e..303fb73e3 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -46,7 +46,7 @@ public enum FavoritesConfiguration { } } -public enum FavoritesPlatform: String { +public enum FavoritesPlatform: String, CaseIterable { case mobile = "mobile_favorites_root" case desktop = "desktop_favorites_root" case all = "favorites_root" @@ -66,6 +66,13 @@ public class BookmarkEntity: NSManagedObject { mobileFavoritesFolderID, favoritesFolderID ] + + public static let allReservedFoldersIDs: Set = [ + rootFolderID, + desktopFavoritesFolderID, + mobileFavoritesFolderID, + favoritesFolderID + ] } public static func isValidFavoritesFolderID(_ value: String) -> Bool { diff --git a/Sources/BookmarksTestsUtils/BookmarkTree.swift b/Sources/BookmarksTestsUtils/BookmarkTree.swift index f8b803d39..1b12098fb 100644 --- a/Sources/BookmarksTestsUtils/BookmarkTree.swift +++ b/Sources/BookmarksTestsUtils/BookmarkTree.swift @@ -60,7 +60,7 @@ public struct ModifiedAtConstraint { } public enum BookmarkTreeNode { - case bookmark(id: String, name: String?, url: String?, isFavorite: Bool, modifiedAt: Date?, isDeleted: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) + case bookmark(id: String, name: String?, url: String?, favoritedOn: [FavoritesPlatform], modifiedAt: Date?, isDeleted: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) case folder(id: String, name: String?, children: [BookmarkTreeNode], modifiedAt: Date?, isDeleted: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) public var id: String { @@ -126,17 +126,17 @@ public struct Bookmark: BookmarkTreeNodeConvertible { var id: String var name: String? var url: String? - var isFavorite: Bool + var favoritedOn: [FavoritesPlatform] var modifiedAt: Date? var isDeleted: Bool var isOrphaned: Bool var modifiedAtConstraint: ModifiedAtConstraint? - public init(_ name: String? = nil, id: String? = nil, url: String? = nil, isFavorite: Bool = false, modifiedAt: Date? = nil, isDeleted: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil) { + public init(_ name: String? = nil, id: String? = nil, url: String? = nil, favoritedOn: [FavoritesPlatform] = [], modifiedAt: Date? = nil, isDeleted: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil) { self.id = id ?? UUID().uuidString self.name = name ?? id self.url = (url ?? name) ?? id - self.isFavorite = isFavorite + self.favoritedOn = favoritedOn self.modifiedAt = modifiedAt self.isDeleted = isDeleted self.modifiedAtConstraint = modifiedAtConstraint @@ -144,7 +144,7 @@ public struct Bookmark: BookmarkTreeNodeConvertible { } public func asBookmarkTreeNode() -> BookmarkTreeNode { - .bookmark(id: id, name: name, url: url, isFavorite: isFavorite, modifiedAt: modifiedAt, isDeleted: isDeleted, isOrphaned: isOrphaned, modifiedAtConstraint: modifiedAtConstraint) + .bookmark(id: id, name: name, url: url, favoritedOn: favoritedOn, modifiedAt: modifiedAt, isDeleted: isDeleted, isOrphaned: isOrphaned, modifiedAtConstraint: modifiedAtConstraint) } } @@ -203,15 +203,14 @@ public struct BookmarkTree { public func createEntitiesForCheckingModifiedAt(in context: NSManagedObjectContext) -> (BookmarkEntity, [BookmarkEntity], [String: ModifiedAtConstraint]) { let rootFolder = BookmarkUtils.fetchRootFolder(context)! rootFolder.modifiedAt = modifiedAt - // todo - let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context)! + let favoritesFolders = FavoritesPlatform.allCases.map { BookmarkUtils.fetchFavoritesFolder(withUUID: $0.rawValue, in: context)! } var orphans = [BookmarkEntity]() var modifiedAtConstraints = [String: ModifiedAtConstraint]() if let modifiedAtConstraint { modifiedAtConstraints[BookmarkEntity.Constants.rootFolderID] = modifiedAtConstraint } for bookmarkTreeNode in bookmarkTreeNodes { - let (entity, checks) = BookmarkEntity.makeWithModifiedAtConstraints(with: bookmarkTreeNode, rootFolder: rootFolder, favoritesFolder: favoritesFolder, in: context) + let (entity, checks) = BookmarkEntity.makeWithModifiedAtConstraints(with: bookmarkTreeNode, rootFolder: rootFolder, favoritesFolders: favoritesFolders, in: context) if bookmarkTreeNode.isOrphaned { orphans.append(entity) } @@ -231,12 +230,12 @@ public struct BookmarkTree { public extension BookmarkEntity { @discardableResult - static func make(with treeNode: BookmarkTreeNode, rootFolder: BookmarkEntity, favoritesFolder: BookmarkEntity, in context: NSManagedObjectContext) -> BookmarkEntity { - makeWithModifiedAtConstraints(with: treeNode, rootFolder: rootFolder, favoritesFolder: favoritesFolder, in: context).0 + static func make(with treeNode: BookmarkTreeNode, rootFolder: BookmarkEntity, favoritesFolders: [BookmarkEntity], in context: NSManagedObjectContext) -> BookmarkEntity { + makeWithModifiedAtConstraints(with: treeNode, rootFolder: rootFolder, favoritesFolders: favoritesFolders, in: context).0 } @discardableResult - static func makeWithModifiedAtConstraints(with treeNode: BookmarkTreeNode, rootFolder: BookmarkEntity, favoritesFolder: BookmarkEntity, in context: NSManagedObjectContext) -> (BookmarkEntity, [String: ModifiedAtConstraint]) { + static func makeWithModifiedAtConstraints(with treeNode: BookmarkTreeNode, rootFolder: BookmarkEntity, favoritesFolders: [BookmarkEntity], in context: NSManagedObjectContext) -> (BookmarkEntity, [String: ModifiedAtConstraint]) { var entity: BookmarkEntity! var queues: [[BookmarkTreeNode]] = [[treeNode]] @@ -251,7 +250,7 @@ public extension BookmarkEntity { let node = queue.removeFirst() switch node { - case .bookmark(let id, let name, let url, let isFavorite, let modifiedAt, let isDeleted, let isOrphaned, let modifiedAtConstraint): + case .bookmark(let id, let name, let url, let favoritedOn, let modifiedAt, let isDeleted, let isOrphaned, let modifiedAtConstraint): let bookmarkEntity = BookmarkEntity(context: context) if entity == nil { entity = bookmarkEntity @@ -262,9 +261,13 @@ public extension BookmarkEntity { bookmarkEntity.url = url bookmarkEntity.modifiedAt = modifiedAt modifiedAtConstraints[id] = modifiedAtConstraint - if isFavorite { - bookmarkEntity.addToFavorites(favoritesRoot: favoritesFolder) + + for platform in favoritedOn { + if let favoritesFolder = favoritesFolders.first(where: { $0.uuid == platform.rawValue }) { + bookmarkEntity.addToFavorites(favoritesRoot: favoritesFolder) + } } + if isDeleted { bookmarkEntity.markPendingDeletion() } @@ -342,8 +345,7 @@ public extension XCTestCase { XCTAssertEqual(expectedNode.isFolder, thisNode.isFolder, "isFolder mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.isPendingDeletion, thisNode.isPendingDeletion, "isPendingDeletion mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.children?.count, thisNode.children?.count, "children count mismatch for \(thisUUID)", file: file, line: line) - // todo - XCTAssertEqual(expectedNode.isFavorite(on: .all), thisNode.isFavorite(on: .all), "isFavorite mismatch for \(thisUUID)", file: file, line: line) + XCTAssertEqual(expectedNode.favoritedOn, thisNode.favoritedOn, "favoritedOn mismatch for \(thisUUID)", file: file, line: line) if withTimestamps { if let modifiedAtConstraint = modifiedAtConstraints[thisUUID] { modifiedAtConstraint.check(thisNode.modifiedAt) diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index 2bfcf5ba6..109eeb263 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -71,10 +71,10 @@ final class FavoriteListViewModelTests: XCTestCase { let context = favoriteListViewModel.context let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true, isDeleted: true) - Bookmark(id: "2", isFavorite: true) - Bookmark(id: "3", isFavorite: true, isDeleted: true) - Bookmark(id: "4", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.mobile, .all], isDeleted: true) + Bookmark(id: "2", favoritedOn: [.mobile, .all]) + Bookmark(id: "3", favoritedOn: [.mobile, .all], isDeleted: true) + Bookmark(id: "4", favoritedOn: [.mobile, .all]) } context.performAndWait { diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift index 527362d91..0130b337c 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift @@ -137,21 +137,22 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "2", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.mobile]) + Bookmark(id: "2", favoritedOn: [.mobile]) } let received: [Syncable] = [ .rootFolder(children: ["1", "2", "3"]), - .favoritesFolder(favorites: ["1", "2", "3"]), + .mobileFavoritesFolder(favorites: ["1", "2"]), + .desktopFavoritesFolder(favorites: ["3"]), .bookmark(id: "3") ] let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "2", isFavorite: true) - Bookmark(id: "3", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.mobile]) + Bookmark(id: "2", favoritedOn: [.mobile]) + Bookmark(id: "3", favoritedOn: [.desktop]) }) } @@ -159,8 +160,8 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "4", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "4", favoritedOn: [.all]) } let received: [Syncable] = [ @@ -171,9 +172,9 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "4", isFavorite: true) - Bookmark(id: "3", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "4", favoritedOn: [.all]) + Bookmark(id: "3", favoritedOn: [.all]) }) } @@ -350,7 +351,7 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) } let received: [Syncable] = [ @@ -361,13 +362,12 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "2", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "2", favoritedOn: [.all]) }) var favoritesFolder: BookmarkEntity! context.performAndWait { - // todo favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context) } XCTAssertNotNil(favoritesFolder.modifiedAt) diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index cbcf0248c..df4b98b2e 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -41,7 +41,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark("Bookmark 1", id: "1", isFavorite: true) + Bookmark("Bookmark 1", id: "1", favoritedOn: [.mobile]) Bookmark("Bookmark 2", id: "2") Folder("Folder", id: "3") { Bookmark("Bookmark 4", id: "4") @@ -65,7 +65,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let rootFolder = BookmarkUtils.fetchRootFolder(context)! assertEquivalent(rootFolder, BookmarkTree(modifiedAtConstraint: .notNil()) { - Bookmark("Bookmark 1", id: "1", isFavorite: true, modifiedAtConstraint: .notNil()) + Bookmark("Bookmark 1", id: "1", favoritedOn: [.mobile], modifiedAtConstraint: .notNil()) Bookmark("Bookmark 2", id: "2", modifiedAtConstraint: .notNil()) Folder("Folder", id: "3", modifiedAtConstraint: .notNil()) { Bookmark("Bookmark 4", id: "4", modifiedAtConstraint: .notNil()) @@ -84,7 +84,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.mobile]) } context.performAndWait { @@ -98,7 +98,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { XCTAssertEqual( Set(changedObjects.compactMap(\.uuid)), - Set([BookmarkEntity.Constants.favoritesFolderID, BookmarkEntity.Constants.rootFolderID, "1"]) + BookmarkEntity.Constants.allReservedFoldersIDs.union(["1"]) ) } @@ -106,7 +106,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.mobile]) Folder(id: "2") { Bookmark(id: "3") Bookmark(id: "4") diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index 424873249..c6835ba58 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -115,8 +115,8 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "2", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "2", favoritedOn: [.all]) } let received: [Syncable] = [ @@ -127,9 +127,9 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", isFavorite: true) - Bookmark(id: "2", isFavorite: true) - Bookmark(id: "3", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "2", favoritedOn: [.all]) + Bookmark(id: "3", favoritedOn: [.all]) }) } @@ -137,7 +137,7 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) Folder(id: "2") { Bookmark(id: "3") } @@ -151,9 +151,9 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", isFavorite: true) + Bookmark(id: "1", favoritedOn: [.all]) Folder(id: "2") { - Bookmark(id: "3", isFavorite: true) + Bookmark(id: "3", favoritedOn: [.all]) } }) } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift b/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift index 2e81571f5..5d0f3f931 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift @@ -29,6 +29,14 @@ extension Syncable { .folder(id: BookmarkEntity.Constants.favoritesFolderID, children: favorites) } + static func mobileFavoritesFolder(favorites: [String]) -> Syncable { + .folder(id: BookmarkEntity.Constants.mobileFavoritesFolderID, children: favorites) + } + + static func desktopFavoritesFolder(favorites: [String]) -> Syncable { + .folder(id: BookmarkEntity.Constants.desktopFavoritesFolderID, children: favorites) + } + static func bookmark(_ title: String? = nil, id: String, url: String? = nil, lastModified: String? = nil, isDeleted: Bool = false) -> Syncable { var json: [String: Any] = [ "id": id, From 4f4404e0d271157033f729fc841b9c25fd46878e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sat, 9 Sep 2023 00:39:37 +0200 Subject: [PATCH 04/56] Make FavoritesConfiguration conform to Equatable and make its api public --- Sources/Bookmarks/BookmarkEntity.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 303fb73e3..3c0fcbc0b 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -21,11 +21,11 @@ import Foundation import CoreData -public enum FavoritesConfiguration { +public enum FavoritesConfiguration: Equatable { case displayNative(FavoritesPlatform) case displayAll(native: FavoritesPlatform) - var displayedPlatform: FavoritesPlatform { + public var displayedPlatform: FavoritesPlatform { switch self { case .displayNative(let platform): return platform @@ -34,14 +34,14 @@ public enum FavoritesConfiguration { } } - var nativePlatform: FavoritesPlatform { + public var nativePlatform: FavoritesPlatform { switch self { case .displayNative(let native), .displayAll(let native): return native } } - var folderUUIDs: Set { + public var folderUUIDs: Set { return [displayedPlatform.rawValue, nativePlatform.rawValue] } } From 1c45a402700bfae345a5ba3bb7ece58ee54a5d0f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sat, 9 Sep 2023 01:10:12 +0200 Subject: [PATCH 05/56] Fix favorites syncable representation --- .../Bookmarks/internal/SyncableBookmarkAdapter.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift index 5619ed41e..40a1df9ff 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift @@ -79,7 +79,7 @@ extension Syncable { payload["title"] = try encrypt(title) } if bookmark.isFolder { - if bookmark.uuid == BookmarkEntity.Constants.favoritesFolderID { + if BookmarkEntity.Constants.favoriteFoldersIDs.contains(uuid) { payload["folder"] = [ "children": bookmark.favoritesArray.map(\.uuid) ] From 0e835574b5781ad23e928ec7c8aca98aff76a67d Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sat, 9 Sep 2023 10:27:00 +0200 Subject: [PATCH 06/56] Fix FavoritesConfiguration.folderUUIDs --- Sources/Bookmarks/BookmarkEntity.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 3c0fcbc0b..ad0f0ed8d 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -42,7 +42,7 @@ public enum FavoritesConfiguration: Equatable { } public var folderUUIDs: Set { - return [displayedPlatform.rawValue, nativePlatform.rawValue] + return [nativePlatform.rawValue, FavoritesPlatform.all.rawValue] } } From 0e464108f943e29974df48455e677c12b99c0cbd Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sat, 9 Sep 2023 13:25:36 +0200 Subject: [PATCH 07/56] Fix favorites folders in MenuBookmarksViewModel --- Sources/Bookmarks/MenuBookmarksViewModel.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 7fc7a622a..f737ac5d9 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -119,7 +119,8 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { bookmark.removeFromFavorites() } else { - bookmark.addToFavorites(folders: [favoritesFolder, nativeFavoritesFolder]) + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesConfiguration, in: context) + bookmark.addToFavorites(folders: folders) } } else { let favorite = BookmarkEntity.makeBookmark(title: title, From 4ea34550264806cfa8d8529feb88a86281a6cf54 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sat, 9 Sep 2023 13:34:55 +0200 Subject: [PATCH 08/56] Fix populating favorites in Sync response --- Sources/Bookmarks/BookmarkEntity.swift | 4 ++++ .../Bookmarks/internal/BookmarksResponseHandler.swift | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index ad0f0ed8d..f60b44c4f 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -240,6 +240,10 @@ public class BookmarkEntity: NSManagedObject { removeFromFavoriteFolders(favoriteFolders) } + public func removeFromFavorites(favoritesRoot: BookmarkEntity) { + removeFromFavoriteFolders(favoritesRoot) + } + public func markPendingDeletion() { var queue: [BookmarkEntity] = [self] diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index c8bfc30e2..97aa80c2f 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -128,7 +128,7 @@ final class BookmarksResponseHandler { favoritesUUIDs.forEach { uuid in if let bookmark = entitiesByUUID[uuid] { - bookmark.removeFromFavorites() + bookmark.removeFromFavorites(favoritesRoot: favoritesFolder) bookmark.addToFavorites(favoritesRoot: favoritesFolder) } } From 2400000162dc2ac8b44c0561f64cb954639f9fc9 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sat, 9 Sep 2023 17:16:02 +0200 Subject: [PATCH 09/56] Rename FavoritesConfiguration to FavoritesDisplayMode --- .../Bookmarks/BookmarkEditorViewModel.swift | 18 +++++++++--------- Sources/Bookmarks/BookmarkEntity.swift | 2 +- Sources/Bookmarks/BookmarkListViewModel.swift | 10 +++++----- Sources/Bookmarks/BookmarkUtils.swift | 2 +- Sources/Bookmarks/FavoriteListViewModel.swift | 8 ++++---- .../BookmarkCoreDataImporter.swift | 8 ++++---- Sources/Bookmarks/MenuBookmarksViewModel.swift | 14 +++++++------- .../BookmarkListViewModelTests.swift | 2 +- .../FavoriteListViewModelTests.swift | 4 ++-- 9 files changed, 34 insertions(+), 34 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index 2f51c7b19..e61e09503 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -31,13 +31,13 @@ public class BookmarkEditorViewModel: ObservableObject { } let context: NSManagedObjectContext - let favoritesConfiguration: FavoritesConfiguration + let favoritesDisplayMode: FavoritesDisplayMode @Published public var bookmark: BookmarkEntity @Published public var locations = [Location]() lazy var favoritesFolder: BookmarkEntity! = BookmarkUtils.fetchFavoritesFolder( - withUUID: favoritesConfiguration.displayedPlatform.rawValue, + withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context ) @@ -63,13 +63,13 @@ public class BookmarkEditorViewModel: ObservableObject { public init(editingEntityID: NSManagedObjectID, bookmarksDatabase: CoreDataDatabase, - favoritesConfiguration: FavoritesConfiguration, + favoritesDisplayMode: FavoritesDisplayMode, errorEvents: EventMapping?) { externalUpdates = subject.eraseToAnyPublisher() self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) - self.favoritesConfiguration = favoritesConfiguration + self.favoritesDisplayMode = favoritesDisplayMode guard let entity = context.object(with: editingEntityID) as? BookmarkEntity else { // For sync, this is valid scenario in case of a timing issue @@ -90,13 +90,13 @@ public class BookmarkEditorViewModel: ObservableObject { public init(creatingFolderWithParentID parentFolderID: NSManagedObjectID?, bookmarksDatabase: CoreDataDatabase, - favoritesConfiguration: FavoritesConfiguration, + favoritesDisplayMode: FavoritesDisplayMode, errorEvents: EventMapping?) { externalUpdates = subject.eraseToAnyPublisher() self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) - self.favoritesConfiguration = favoritesConfiguration + self.favoritesDisplayMode = favoritesDisplayMode let parent: BookmarkEntity? if let parentFolderID = parentFolderID { @@ -181,13 +181,13 @@ public class BookmarkEditorViewModel: ObservableObject { } public func removeFromFavorites() { - assert(bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform)) + assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) bookmark.removeFromFavorites() } public func addToFavorites() { - assert(!bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform)) - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesConfiguration, in: context) + assert(!bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) } diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index f60b44c4f..a23dd5f4e 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -21,7 +21,7 @@ import Foundation import CoreData -public enum FavoritesConfiguration: Equatable { +public enum FavoritesDisplayMode: Equatable { case displayNative(FavoritesPlatform) case displayAll(native: FavoritesPlatform) diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 20daca329..829dd12b9 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -27,7 +27,7 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public let currentFolder: BookmarkEntity? let context: NSManagedObjectContext - let favoritesConfiguration: FavoritesConfiguration + let favoritesDisplayMode: FavoritesDisplayMode public var bookmarks = [BookmarkEntity]() @@ -40,14 +40,14 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { private let errorEvents: EventMapping? public init(bookmarksDatabase: CoreDataDatabase, - favoritesConfiguration: FavoritesConfiguration, + favoritesDisplayMode: FavoritesDisplayMode, parentID: NSManagedObjectID?, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) - self.favoritesConfiguration = favoritesConfiguration + self.favoritesDisplayMode = favoritesDisplayMode if let parentID = parentID { if let bookmark = (try? context.existingObject(with: parentID)) as? BookmarkEntity { @@ -112,10 +112,10 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { } public func toggleFavorite(_ bookmark: BookmarkEntity) { - if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { + if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { bookmark.removeFromFavorites() } else { - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesConfiguration, in: context) + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) } save() diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 0ddadb7dd..f1f8472ea 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -30,7 +30,7 @@ public struct BookmarkUtils { return try? context.fetch(request).first } - public static func fetchFavoritesFolders(for configuration: FavoritesConfiguration, in context: NSManagedObjectContext) -> [BookmarkEntity] { + public static func fetchFavoritesFolders(for configuration: FavoritesDisplayMode, in context: NSManagedObjectContext) -> [BookmarkEntity] { configuration.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } } diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 9c299ceab..83e583b3c 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -25,7 +25,7 @@ import Common public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject { let context: NSManagedObjectContext - let favoritesConfiguration: FavoritesConfiguration + let favoritesDisplayMode: FavoritesDisplayMode public var favorites = [BookmarkEntity]() @@ -40,7 +40,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject private var _favoritesFolder: BookmarkEntity? private var favoriteFolder: BookmarkEntity? { if _favoritesFolder == nil { - _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesConfiguration.displayedPlatform.rawValue, in: context) + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) if _favoritesFolder == nil { errorEvents?.fire(.fetchingRootItemFailed(.favorites)) @@ -50,12 +50,12 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject } public init(bookmarksDatabase: CoreDataDatabase, - favoritesConfiguration: FavoritesConfiguration, + favoritesDisplayMode: FavoritesDisplayMode, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() self.errorEvents = errorEvents - self.favoritesConfiguration = favoritesConfiguration + self.favoritesDisplayMode = favoritesDisplayMode self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) refresh() diff --git a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift index 279832d9b..03c78f357 100644 --- a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift +++ b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift @@ -23,11 +23,11 @@ import Persistence public class BookmarkCoreDataImporter { let context: NSManagedObjectContext - let favoritesConfiguration: FavoritesConfiguration + let favoritesDisplayMode: FavoritesDisplayMode - public init(database: CoreDataDatabase, favoritesConfiguration: FavoritesConfiguration) { + public init(database: CoreDataDatabase, favoritesDisplayMode: FavoritesDisplayMode) { self.context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) - self.favoritesConfiguration = favoritesConfiguration + self.favoritesDisplayMode = favoritesDisplayMode } public func importBookmarks(_ bookmarks: [BookmarkOrFolder]) async throws { @@ -36,7 +36,7 @@ public class BookmarkCoreDataImporter { context.performAndWait { () -> Void in do { - let favoritesFolderUUIDs = favoritesConfiguration.folderUUIDs + let favoritesFolderUUIDs = favoritesDisplayMode.folderUUIDs let favoritesFolders = favoritesFolderUUIDs.compactMap { BookmarkUtils.fetchFavoritesFolder(withUUID: $0, in: context) } guard let topLevelBookmarksFolder = BookmarkUtils.fetchRootFolder(context), diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index f737ac5d9..53d1320e9 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -24,7 +24,7 @@ import Persistence public class MenuBookmarksViewModel: MenuBookmarksInteracting { let context: NSManagedObjectContext - let favoritesConfiguration: FavoritesConfiguration + let favoritesDisplayMode: FavoritesDisplayMode private var _rootFolder: BookmarkEntity? private var rootFolder: BookmarkEntity? { @@ -41,7 +41,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { private var _favoritesFolder: BookmarkEntity? private var favoritesFolder: BookmarkEntity? { if _favoritesFolder == nil { - _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesConfiguration.displayedPlatform.rawValue, in: context) + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) if _favoritesFolder == nil { errorEvents?.fire(.fetchingRootItemFailed(.menu)) @@ -53,7 +53,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { private var _nativeFavoritesFolder: BookmarkEntity? private var nativeFavoritesFolder: BookmarkEntity? { if _nativeFavoritesFolder == nil { - _nativeFavoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesConfiguration.nativePlatform.rawValue, in: context) + _nativeFavoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.nativePlatform.rawValue, in: context) if _nativeFavoritesFolder == nil { errorEvents?.fire(.fetchingRootItemFailed(.menu)) @@ -67,9 +67,9 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { private let errorEvents: EventMapping? public init(bookmarksDatabase: CoreDataDatabase, - favoritesConfiguration: FavoritesConfiguration, + favoritesDisplayMode: FavoritesDisplayMode, errorEvents: EventMapping?) { - self.favoritesConfiguration = favoritesConfiguration + self.favoritesDisplayMode = favoritesDisplayMode self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) registerForChanges() @@ -116,10 +116,10 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { let queriedBookmark = favorite(for: url) ?? bookmark(for: url) if let bookmark = queriedBookmark { - if bookmark.isFavorite(on: favoritesConfiguration.displayedPlatform) { + if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { bookmark.removeFromFavorites() } else { - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesConfiguration, in: context) + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) } } else { diff --git a/Tests/BookmarksTests/BookmarkListViewModelTests.swift b/Tests/BookmarksTests/BookmarkListViewModelTests.swift index e0fb3b09d..d2ad748a7 100644 --- a/Tests/BookmarksTests/BookmarkListViewModelTests.swift +++ b/Tests/BookmarksTests/BookmarkListViewModelTests.swift @@ -67,7 +67,7 @@ final class BookmarkListViewModelTests: XCTestCase { bookmarkListViewModel = BookmarkListViewModel( bookmarksDatabase: bookmarksDatabase, - favoritesConfiguration: .displayNative(.mobile), + favoritesDisplayMode: .displayNative(.mobile), parentID: nil, errorEvents: eventMapping ) diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index 109eeb263..f720f70fd 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -53,7 +53,7 @@ final class FavoriteListViewModelTests: XCTestCase { } favoriteListViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, - favoritesConfiguration: .displayNative(.mobile), + favoritesDisplayMode: .displayNative(.mobile), errorEvents: eventMapping) } @@ -82,7 +82,7 @@ final class FavoriteListViewModelTests: XCTestCase { try! context.save() - let favoriteFolderUUID = favoriteListViewModel.favoritesConfiguration.displayedPlatform.rawValue + let favoriteFolderUUID = favoriteListViewModel.favoritesDisplayMode.displayedPlatform.rawValue let rootFavoriteFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoriteFolderUUID, in: context)! XCTAssertEqual(rootFavoriteFolder.favoritesArray.map(\.title), ["2", "4"]) From d873593da20c8677ee13749aff579dd1e3f92866 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 10 Sep 2023 14:32:29 +0200 Subject: [PATCH 10/56] Fix adding to favorites on iOS --- .../BookmarkCoreDataImporter.swift | 7 ++----- .../Bookmarks/MenuBookmarksViewModel.swift | 19 +++---------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift index 03c78f357..4a9202039 100644 --- a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift +++ b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift @@ -36,12 +36,9 @@ public class BookmarkCoreDataImporter { context.performAndWait { () -> Void in do { - let favoritesFolderUUIDs = favoritesDisplayMode.folderUUIDs - let favoritesFolders = favoritesFolderUUIDs.compactMap { BookmarkUtils.fetchFavoritesFolder(withUUID: $0, in: context) } + let favoritesFolders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) - guard let topLevelBookmarksFolder = BookmarkUtils.fetchRootFolder(context), - favoritesFolders.count == favoritesFolderUUIDs.count - else { + guard let topLevelBookmarksFolder = BookmarkUtils.fetchRootFolder(context) else { throw BookmarksCoreDataError.fetchingExistingItemFailed } diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 53d1320e9..cd759bd00 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -50,18 +50,6 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { return _favoritesFolder } - private var _nativeFavoritesFolder: BookmarkEntity? - private var nativeFavoritesFolder: BookmarkEntity? { - if _nativeFavoritesFolder == nil { - _nativeFavoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.nativePlatform.rawValue, in: context) - - if _nativeFavoritesFolder == nil { - errorEvents?.fire(.fetchingRootItemFailed(.menu)) - } - } - return _nativeFavoritesFolder - } - private var observer: NSObjectProtocol? private let errorEvents: EventMapping? @@ -107,9 +95,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { } public func createOrToggleFavorite(title: String, url: URL) { - guard let favoritesFolder = favoritesFolder, - let nativeFavoritesFolder = nativeFavoritesFolder, - let rootFolder = rootFolder else { + guard let rootFolder = rootFolder else { return } @@ -127,7 +113,8 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { url: url.absoluteString, parent: rootFolder, context: context) - favorite.addToFavorites(folders: [favoritesFolder, nativeFavoritesFolder]) + let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) + favorite.addToFavorites(folders: folders) } save() From f33013c9fcaee4e81f72dab14c8e0b190ad3e6b5 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 10 Sep 2023 21:09:58 +0200 Subject: [PATCH 11/56] Fix handling favorites in sync response handler --- .../Bookmarks/internal/BookmarksResponseHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index 97aa80c2f..3daabc899 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -119,7 +119,7 @@ final class BookmarksResponseHandler { // For non-first sync we rely fully on the server response if !shouldDeduplicateEntities { - favoritesFolder.favoritesArray.forEach { $0.removeFromFavorites() } + favoritesFolder.favoritesArray.forEach { $0.removeFromFavorites(favoritesRoot: favoritesFolder) } } 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. From bad28507a031c0c2e3d67384b07b6ae553a72717 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 10 Sep 2023 21:38:39 +0200 Subject: [PATCH 12/56] Make favorites display mode public and modifiable in FavoriteListViewModel --- Sources/Bookmarks/FavoriteListViewModel.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 83e583b3c..19f6d580d 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -25,9 +25,13 @@ import Common public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject { let context: NSManagedObjectContext - let favoritesDisplayMode: FavoritesDisplayMode public var favorites = [BookmarkEntity]() + public var favoritesDisplayMode: FavoritesDisplayMode { + didSet { + reloadData() + } + } private var observer: NSObjectProtocol? private let subject = PassthroughSubject() From bb08514bd53fc898dcf025a342b2cccfbcd029a8 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 10 Sep 2023 21:46:47 +0200 Subject: [PATCH 13/56] Make favorites display mode public and modifiable in MenuBookmarksViewModel --- Sources/Bookmarks/MenuBookmarksViewModel.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index cd759bd00..770a66108 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -24,7 +24,11 @@ import Persistence public class MenuBookmarksViewModel: MenuBookmarksInteracting { let context: NSManagedObjectContext - let favoritesDisplayMode: FavoritesDisplayMode + public var favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) { + didSet { + _favoritesFolder = nil + } + } private var _rootFolder: BookmarkEntity? private var rootFolder: BookmarkEntity? { From 547da01fcd744aa7e5c772924596356bd8644132 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 10 Sep 2023 21:48:54 +0200 Subject: [PATCH 14/56] Remove favoritesDisplayMode from initializers in FavoritesListViewModel and MenuBookmarksViewModel --- Sources/Bookmarks/FavoriteListViewModel.swift | 10 ++++------ Sources/Bookmarks/MenuBookmarksViewModel.swift | 5 +---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 19f6d580d..848001892 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -27,8 +27,9 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject let context: NSManagedObjectContext public var favorites = [BookmarkEntity]() - public var favoritesDisplayMode: FavoritesDisplayMode { + public var favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) { didSet { + _favoritesFolder = nil reloadData() } } @@ -53,14 +54,11 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject return _favoritesFolder } - public init(bookmarksDatabase: CoreDataDatabase, - favoritesDisplayMode: FavoritesDisplayMode, - errorEvents: EventMapping?) { + public init(bookmarksDatabase: CoreDataDatabase, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() self.errorEvents = errorEvents - self.favoritesDisplayMode = favoritesDisplayMode - + self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) refresh() registerForChanges() diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 770a66108..b2079d5e6 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -58,10 +58,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { private let errorEvents: EventMapping? - public init(bookmarksDatabase: CoreDataDatabase, - favoritesDisplayMode: FavoritesDisplayMode, - errorEvents: EventMapping?) { - self.favoritesDisplayMode = favoritesDisplayMode + public init(bookmarksDatabase: CoreDataDatabase, errorEvents: EventMapping?) { self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) registerForChanges() From 161088d1ca730187a0390b38b2283011b3c16249 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 10 Sep 2023 21:54:58 +0200 Subject: [PATCH 15/56] Add favoritesDisplayMode to FavoritesListInteracting and MenuBookmarksInteracting --- Sources/Bookmarks/BookmarksModel.swift | 8 ++++++-- Tests/BookmarksTests/FavoriteListViewModelTests.swift | 4 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Sources/Bookmarks/BookmarksModel.swift b/Sources/Bookmarks/BookmarksModel.swift index 9d8561b40..3a653fc5b 100644 --- a/Sources/Bookmarks/BookmarksModel.swift +++ b/Sources/Bookmarks/BookmarksModel.swift @@ -53,9 +53,11 @@ public protocol BookmarkListInteracting: BookmarkStoring { } public protocol FavoritesListInteracting: BookmarkStoring { - + + var favoritesDisplayMode: FavoritesDisplayMode { get set } + var favorites: [BookmarkEntity] { get } - + func favorite(at index: Int) -> BookmarkEntity? func removeFavorite(_ favorite: BookmarkEntity) @@ -67,6 +69,8 @@ public protocol FavoritesListInteracting: BookmarkStoring { public protocol MenuBookmarksInteracting { + var favoritesDisplayMode: FavoritesDisplayMode { get set } + func createOrToggleFavorite(title: String, url: URL) func createBookmark(title: String, url: URL) diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index f720f70fd..a4ac93061 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -52,9 +52,7 @@ final class FavoriteListViewModelTests: XCTestCase { try! context.save() } - favoriteListViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, - favoritesDisplayMode: .displayNative(.mobile), - errorEvents: eventMapping) + favoriteListViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, errorEvents: eventMapping) } override func tearDown() { From bb12fb71965975e43c2114bbda1cdbe3fa098b33 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 11 Sep 2023 14:33:09 +0200 Subject: [PATCH 16/56] Set renaming identifier for favoriteFolders --- .../BookmarksModel 4.xcdatamodel/contents | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents index 1781bec32..d6ce247b3 100644 --- a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents +++ b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents @@ -8,7 +8,7 @@ - + From b4d5db30b3e0b0039aa4652d52f84188e5bdfb52 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 11 Sep 2023 14:36:13 +0200 Subject: [PATCH 17/56] Remove renamingIdentifier from BookmarkEntity --- .../BookmarksModel 4.xcdatamodel/contents | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents index d6ce247b3..539012d01 100644 --- a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents +++ b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 4.xcdatamodel/contents @@ -1,6 +1,6 @@ - + From 21029fc4dcd1e7e43668937902cc7bb44a2f0818 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 11 Sep 2023 14:57:47 +0200 Subject: [PATCH 18/56] Add a helper function to migrate to form factor specific favorites --- Sources/Bookmarks/BookmarkUtils.swift | 55 ++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index f1f8472ea..0e952d041 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -60,15 +60,7 @@ public struct BookmarkUtils { } public static func prepareFoldersStructure(in context: NSManagedObjectContext) { - - func insertRootFolder(uuid: String, into context: NSManagedObjectContext) { - let folder = BookmarkEntity(entity: BookmarkEntity.entity(in: context), - insertInto: context) - folder.uuid = uuid - folder.title = uuid - folder.isFolder = true - } - + if fetchRootFolder(context) == nil { insertRootFolder(uuid: BookmarkEntity.Constants.rootFolderID, into: context) } @@ -79,6 +71,38 @@ public struct BookmarkUtils { } } } + + public static func migrateToFormFactorSpecificFavorites(byCopyingExistingTo platform: FavoritesPlatform, in context: NSManagedObjectContext) { + assert(platform != .all, "You must specify either desktop or mobile platform") + + guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context) else { + return + } + + if BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.desktopFavoritesFolderID, in: context) == nil { + let desktopFavoritesFolder = insertRootFolder(uuid: BookmarkEntity.Constants.desktopFavoritesFolderID, into: context) + + if platform == .desktop { + favoritesFolder.favoritesArray.forEach { bookmark in + bookmark.addToFavorites(folders: [desktopFavoritesFolder]) + } + } else { + desktopFavoritesFolder.shouldManageModifiedAt = false + } + } + + if BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.mobileFavoritesFolderID, in: context) == nil { + let mobileFavoritesFolder = insertRootFolder(uuid: BookmarkEntity.Constants.mobileFavoritesFolderID, into: context) + + if platform == .mobile { + favoritesFolder.favoritesArray.forEach { bookmark in + bookmark.addToFavorites(folders: [mobileFavoritesFolder]) + } + } else { + mobileFavoritesFolder.shouldManageModifiedAt = false + } + } + } public static func fetchBookmark(for url: URL, predicate: NSPredicate = NSPredicate(value: true), @@ -108,4 +132,17 @@ public struct BookmarkUtils { return (try? context.fetch(request)) ?? [] } + + // MARK: Private + + @discardableResult + private static func insertRootFolder(uuid: String, into context: NSManagedObjectContext) -> BookmarkEntity { + let folder = BookmarkEntity(entity: BookmarkEntity.entity(in: context), + insertInto: context) + folder.uuid = uuid + folder.title = uuid + folder.isFolder = true + + return folder + } } From e7db70771b075263c4e43f031c7336b8afc13f4b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 14 Sep 2023 22:47:11 +0200 Subject: [PATCH 19/56] Add UserDefaultsSyncHandler for syncing settings stored in UserDefaults --- .../Settings/SettingsProvider.swift | 21 ++++--- .../SettingsSyncHandling.swift | 4 +- .../UserDefaultsSyncHandler.swift | 62 +++++++++++++++++++ .../helpers/SettingsProviderTestsBase.swift | 1 + 4 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift diff --git a/Sources/SyncDataProviders/Settings/SettingsProvider.swift b/Sources/SyncDataProviders/Settings/SettingsProvider.swift index c5baf4456..7e7ca7c46 100644 --- a/Sources/SyncDataProviders/Settings/SettingsProvider.swift +++ b/Sources/SyncDataProviders/Settings/SettingsProvider.swift @@ -44,9 +44,9 @@ public struct SettingsSyncMetadataSaveError: Error { public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate { public struct Setting: Hashable { - let key: String + public let key: String - init(key: String) { + public init(key: String) { self.key = key } } @@ -55,22 +55,29 @@ public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate metadataDatabase: CoreDataDatabase, metadataStore: SyncMetadataStore, emailManager: EmailManagerSyncSupporting, + userDefaultsHandlers: [UserDefaultsSyncHandler], syncDidUpdateData: @escaping () -> Void ) { let emailProtectionSyncHandler = EmailProtectionSyncHandler(emailManager: emailManager) + let userDefaultsHandlersBySetting = userDefaultsHandlers.reduce(into: [Setting: any SettingsSyncHandling]()) { partialResult, handler in + partialResult[handler.setting] = handler + } + + let settingsHandlers = userDefaultsHandlersBySetting + .merging([.emailProtectionGeneration: emailProtectionSyncHandler], uniquingKeysWith: { $1 }) self.init( metadataDatabase: metadataDatabase, metadataStore: metadataStore, - settingsHandlers: [ - .emailProtectionGeneration: emailProtectionSyncHandler - ], + settingsHandlers: settingsHandlers, syncDidUpdateData: syncDidUpdateData ) register(errorPublisher: errorSubject.eraseToAnyPublisher()) - emailProtectionSyncHandler.delegate = self + settingsHandlers.values.forEach { handler in + handler.delegate = self + } } init( @@ -297,7 +304,7 @@ public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate return idsOfItemsToClearModifiedAt } - func syncHandlerDidUpdateSettingValue(_ handler: SettingsSyncHandling) { + public func syncHandlerDidUpdateSettingValue(_ handler: SettingsSyncHandling) { updateMetadataTimestamp(for: handler.setting) } diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift index fb0174a16..a76674360 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift @@ -32,7 +32,7 @@ import Persistence * * fine-tune Sync behavior for the setting, * * track changes to setting's value and notify delegate accordingly. */ -protocol SettingsSyncHandling { +public protocol SettingsSyncHandling: AnyObject { /** * Returns setting identifier that this handler supports. * @@ -68,7 +68,7 @@ protocol SettingsSyncHandling { * It's implemented by SettingsProvider which owns Settings Sync Handlers * and sets itself as their delegate. */ -protocol SettingsSyncHandlingDelegate: AnyObject { +public protocol SettingsSyncHandlingDelegate: AnyObject { /** * This function must be called whenever setting's value changes for a given Setting Sync Handler. diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift new file mode 100644 index 000000000..5cf80d3f6 --- /dev/null +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift @@ -0,0 +1,62 @@ +// +// UserDefaultsSyncHandler.swift +// DuckDuckGo +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import BrowserServicesKit +import Combine +import DDGSync +import Persistence + +public class UserDefaultsSyncHandler: SettingsSyncHandling { + + public let setting: SettingsProvider.Setting + public weak var delegate: SettingsSyncHandlingDelegate? + + public func getValue() throws -> String? { + userDefaults.string(forKey: userDefaultsKey) + } + + public func setValue(_ value: String?) throws { + userDefaults.set(value, forKey: userDefaultsKey) + } + + public init( + setting: SettingsProvider.Setting, + userDefaults: UserDefaults, + userDefaultsKey: String, + didChangePublisher: AnyPublisher + ) { + self.setting = setting + self.userDefaults = userDefaults + self.userDefaultsKey = userDefaultsKey + + didChangeCancellable = didChangePublisher + .sink { [weak self] in + guard let self else { + return + } + assert(self.delegate != nil, "delegate has not been set for \(type(of: self))") + self.delegate?.syncHandlerDidUpdateSettingValue(self) + } + } + + private let userDefaultsKey: String + private let userDefaults: UserDefaults + private var didChangeCancellable: AnyCancellable? +} diff --git a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift index 218394170..c3835d7ca 100644 --- a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift +++ b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift @@ -135,6 +135,7 @@ internal class SettingsProviderTestsBase: XCTestCase { metadataDatabase: metadataDatabase, metadataStore: LocalSyncMetadataStore(database: metadataDatabase), emailManager: emailManager, + userDefaultsHandlers: [], syncDidUpdateData: {} ) } From 46437811467bc5bd93ab885055a218c15ddbbbbc Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 15 Sep 2023 14:20:37 +0200 Subject: [PATCH 20/56] Add SettingSyncHandler class as a base class for all handlers for syncable settings --- .../Settings/SettingsProvider.swift | 14 +++--- .../EmailProtectionSyncHandler.swift | 26 +++++----- ...Handler.swift => SettingSyncHandler.swift} | 47 +++++++++---------- ...ndling.swift => SettingSyncHandling.swift} | 14 +++--- .../internal/SettingsResponseHandler.swift | 4 +- 5 files changed, 49 insertions(+), 56 deletions(-) rename Sources/SyncDataProviders/Settings/SettingsSyncHandlers/{UserDefaultsSyncHandler.swift => SettingSyncHandler.swift} (50%) rename Sources/SyncDataProviders/Settings/SettingsSyncHandlers/{SettingsSyncHandling.swift => SettingSyncHandling.swift} (84%) diff --git a/Sources/SyncDataProviders/Settings/SettingsProvider.swift b/Sources/SyncDataProviders/Settings/SettingsProvider.swift index 7e7ca7c46..2ab8316e1 100644 --- a/Sources/SyncDataProviders/Settings/SettingsProvider.swift +++ b/Sources/SyncDataProviders/Settings/SettingsProvider.swift @@ -27,7 +27,7 @@ import Persistence /** * Error that may occur while updating timestamp when a setting changes. * - * This error should be published via `SettingsSyncHandling.errorPublisher` + * This error should be published via `SettingSyncHandling.errorPublisher` * whenever settings metadata database fails to save changes after updating * timestamp for a given setting. * @@ -41,7 +41,7 @@ public struct SettingsSyncMetadataSaveError: Error { } } -public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate { +public final class SettingsProvider: DataProvider, SettingSyncHandlingDelegate { public struct Setting: Hashable { public let key: String @@ -55,11 +55,11 @@ public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate metadataDatabase: CoreDataDatabase, metadataStore: SyncMetadataStore, emailManager: EmailManagerSyncSupporting, - userDefaultsHandlers: [UserDefaultsSyncHandler], + userDefaultsHandlers: [SettingSyncHandler], syncDidUpdateData: @escaping () -> Void ) { let emailProtectionSyncHandler = EmailProtectionSyncHandler(emailManager: emailManager) - let userDefaultsHandlersBySetting = userDefaultsHandlers.reduce(into: [Setting: any SettingsSyncHandling]()) { partialResult, handler in + let userDefaultsHandlersBySetting = userDefaultsHandlers.reduce(into: [Setting: any SettingSyncHandling]()) { partialResult, handler in partialResult[handler.setting] = handler } @@ -83,7 +83,7 @@ public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate init( metadataDatabase: CoreDataDatabase, metadataStore: SyncMetadataStore, - settingsHandlers: [Setting: any SettingsSyncHandling], + settingsHandlers: [Setting: any SettingSyncHandling], syncDidUpdateData: @escaping () -> Void ) { self.metadataDatabase = metadataDatabase @@ -304,7 +304,7 @@ public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate return idsOfItemsToClearModifiedAt } - public func syncHandlerDidUpdateSettingValue(_ handler: SettingsSyncHandling) { + func syncHandlerDidUpdateSettingValue(_ handler: SettingSyncHandling) { updateMetadataTimestamp(for: handler.setting) } @@ -341,7 +341,7 @@ public final class SettingsProvider: DataProvider, SettingsSyncHandlingDelegate } private let metadataDatabase: CoreDataDatabase - private let settingsHandlers: [Setting: any SettingsSyncHandling] + private let settingsHandlers: [Setting: any SettingSyncHandling] private let errorSubject = PassthroughSubject() enum Const { diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift index c983be43a..1402ea3ff 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift @@ -37,17 +37,18 @@ extension SettingsProvider.Setting { static let emailProtectionGeneration = SettingsProvider.Setting(key: "email_protection_generation") } -class EmailProtectionSyncHandler: SettingsSyncHandling { +class EmailProtectionSyncHandler: SettingSyncHandler { struct Payload: Codable { let username: String let personalAccessToken: String } - let setting: SettingsProvider.Setting = .emailProtectionGeneration - weak var delegate: SettingsSyncHandlingDelegate? + override var setting: SettingsProvider.Setting { + .emailProtectionGeneration + } - func getValue() throws -> String? { + override func getValue() throws -> String? { guard let user = try emailManager.getUsername() else { return nil } @@ -58,7 +59,7 @@ class EmailProtectionSyncHandler: SettingsSyncHandling { return String(bytes: data, encoding: .utf8) } - func setValue(_ value: String?) throws { + override func setValue(_ value: String?) throws { guard let value, let valueData = value.data(using: .utf8) else { try emailManager.signOut() return @@ -68,19 +69,14 @@ class EmailProtectionSyncHandler: SettingsSyncHandling { try emailManager.signIn(username: payload.username, token: payload.personalAccessToken) } + override var valueDidChangePublisher: AnyPublisher { + emailManager.userDidToggleEmailProtectionPublisher + } + init(emailManager: EmailManagerSyncSupporting) { self.emailManager = emailManager - - emailProtectionStatusDidChangeCancellable = self.emailManager.userDidToggleEmailProtectionPublisher - .sink { [weak self] in - guard let self else { - return - } - assert(self.delegate != nil, "delegate has not been set for \(type(of: self))") - self.delegate?.syncHandlerDidUpdateSettingValue(self) - } + super.init() } private let emailManager: EmailManagerSyncSupporting - private var emailProtectionStatusDidChangeCancellable: AnyCancellable? } diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingSyncHandler.swift similarity index 50% rename from Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift rename to Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingSyncHandler.swift index 5cf80d3f6..643699ec9 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/UserDefaultsSyncHandler.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingSyncHandler.swift @@ -1,5 +1,5 @@ // -// UserDefaultsSyncHandler.swift +// SettingSyncHandler.swift // DuckDuckGo // // Copyright © 2023 DuckDuckGo. All rights reserved. @@ -17,36 +17,32 @@ // limitations under the License. // -import Foundation -import BrowserServicesKit import Combine -import DDGSync -import Persistence +import Foundation -public class UserDefaultsSyncHandler: SettingsSyncHandling { +open class SettingSyncHandler: SettingSyncHandling { - public let setting: SettingsProvider.Setting - public weak var delegate: SettingsSyncHandlingDelegate? + open var setting: SettingsProvider.Setting { + assertionFailure("implementation missing for \(#function)") + return .init(key: "") + } + + open var valueDidChangePublisher: AnyPublisher { + assertionFailure("implementation missing for \(#function)") + return Empty().eraseToAnyPublisher() + } - public func getValue() throws -> String? { - userDefaults.string(forKey: userDefaultsKey) + open func getValue() throws -> String? { + assertionFailure("implementation missing for \(#function)") + return nil } - public func setValue(_ value: String?) throws { - userDefaults.set(value, forKey: userDefaultsKey) + open func setValue(_ value: String?) throws { + assertionFailure("implementation missing for \(#function)") } - public init( - setting: SettingsProvider.Setting, - userDefaults: UserDefaults, - userDefaultsKey: String, - didChangePublisher: AnyPublisher - ) { - self.setting = setting - self.userDefaults = userDefaults - self.userDefaultsKey = userDefaultsKey - - didChangeCancellable = didChangePublisher + public init() { + valueDidChangeCancellable = valueDidChangePublisher .sink { [weak self] in guard let self else { return @@ -56,7 +52,6 @@ public class UserDefaultsSyncHandler: SettingsSyncHandling { } } - private let userDefaultsKey: String - private let userDefaults: UserDefaults - private var didChangeCancellable: AnyCancellable? + weak var delegate: SettingSyncHandlingDelegate? + private var valueDidChangeCancellable: AnyCancellable? } diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingSyncHandling.swift similarity index 84% rename from Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift rename to Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingSyncHandling.swift index a76674360..5f4dbc8b0 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingsSyncHandling.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/SettingSyncHandling.swift @@ -1,5 +1,5 @@ // -// SettingsSyncHandling.swift +// SettingSyncHandling.swift // DuckDuckGo // // Copyright © 2023 DuckDuckGo. All rights reserved. @@ -32,7 +32,9 @@ import Persistence * * fine-tune Sync behavior for the setting, * * track changes to setting's value and notify delegate accordingly. */ -public protocol SettingsSyncHandling: AnyObject { +protocol SettingSyncHandling: AnyObject { + + var valueDidChangePublisher: AnyPublisher { get } /** * Returns setting identifier that this handler supports. * @@ -59,19 +61,19 @@ public protocol SettingsSyncHandling: AnyObject { * * The delegate must be set, otherwise an assertion failure is called. */ - var delegate: SettingsSyncHandlingDelegate? { get set } + var delegate: SettingSyncHandlingDelegate? { get set } } /** - * Protocol defining delegate interface for Settings Sync Handler. + * Protocol defining delegate interface for Setting Sync Handler. * * It's implemented by SettingsProvider which owns Settings Sync Handlers * and sets itself as their delegate. */ -public protocol SettingsSyncHandlingDelegate: AnyObject { +protocol SettingSyncHandlingDelegate: AnyObject { /** * This function must be called whenever setting's value changes for a given Setting Sync Handler. */ - func syncHandlerDidUpdateSettingValue(_ handler: SettingsSyncHandling) + func syncHandlerDidUpdateSettingValue(_ handler: SettingSyncHandling) } diff --git a/Sources/SyncDataProviders/Settings/internal/SettingsResponseHandler.swift b/Sources/SyncDataProviders/Settings/internal/SettingsResponseHandler.swift index 9fe3a4cae..7576a690b 100644 --- a/Sources/SyncDataProviders/Settings/internal/SettingsResponseHandler.swift +++ b/Sources/SyncDataProviders/Settings/internal/SettingsResponseHandler.swift @@ -38,7 +38,7 @@ final class SettingsResponseHandler { init( received: [Syncable], clientTimestamp: Date? = nil, - settingsHandlers: [SettingsProvider.Setting: any SettingsSyncHandling], + settingsHandlers: [SettingsProvider.Setting: any SettingSyncHandling], context: NSManagedObjectContext, crypter: Crypting, deduplicateEntities: Bool @@ -119,5 +119,5 @@ final class SettingsResponseHandler { } } - private let settingsHandlers: [SettingsProvider.Setting: any SettingsSyncHandling] + private let settingsHandlers: [SettingsProvider.Setting: any SettingSyncHandling] } From 08b14198d7d00e3dc665aa97ac4612457185b6d9 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 15 Sep 2023 15:09:41 +0200 Subject: [PATCH 21/56] Add FavoritesDisplayMode.isDisplayAll --- Sources/Bookmarks/BookmarkEntity.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index a23dd5f4e..bacc8f320 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -25,6 +25,15 @@ public enum FavoritesDisplayMode: Equatable { case displayNative(FavoritesPlatform) case displayAll(native: FavoritesPlatform) + public var isDisplayAll: Bool { + switch self { + case .displayNative: + return false + case .displayAll: + return true + } + } + public var displayedPlatform: FavoritesPlatform { switch self { case .displayNative(let platform): From b5ed45b409b28688f0e493f9148831689038633e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 15 Sep 2023 17:15:15 +0200 Subject: [PATCH 22/56] Add BookmarkUtils.fetchFavoritesFoldersForUnfavoriting --- Sources/Bookmarks/BookmarkEditorViewModel.swift | 3 ++- Sources/Bookmarks/BookmarkEntity.swift | 7 +++---- Sources/Bookmarks/BookmarkListViewModel.swift | 3 ++- Sources/Bookmarks/BookmarkUtils.swift | 17 +++++++++++++++++ Sources/Bookmarks/FavoriteListViewModel.swift | 3 ++- Sources/Bookmarks/MenuBookmarksViewModel.swift | 3 ++- 6 files changed, 28 insertions(+), 8 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index e61e09503..c58ecd400 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -182,7 +182,8 @@ public class BookmarkEditorViewModel: ObservableObject { public func removeFromFavorites() { assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) - bookmark.removeFromFavorites() + let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(bookmark, for: favoritesDisplayMode, in: context) + bookmark.removeFromFavorites(folders: folders) } public func addToFavorites() { diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index bacc8f320..f6278da28 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -242,11 +242,10 @@ public class BookmarkEntity: NSManagedObject { } } - public func removeFromFavorites() { - guard let favoriteFolders else { - return + public func removeFromFavorites(folders: [BookmarkEntity]) { + for root in folders { + root.removeFromFavorites(self) } - removeFromFavoriteFolders(favoriteFolders) } public func removeFromFavorites(favoritesRoot: BookmarkEntity) { diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 829dd12b9..97d7e88d3 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -113,7 +113,8 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public func toggleFavorite(_ bookmark: BookmarkEntity) { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - bookmark.removeFromFavorites() + let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(bookmark, for: favoritesDisplayMode, in: context) + bookmark.removeFromFavorites(folders: folders) } else { let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 0e952d041..684b78ce3 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -34,6 +34,23 @@ public struct BookmarkUtils { configuration.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } } + public static func fetchFavoritesFoldersForUnfavoriting(_ bookmark: BookmarkEntity, for configuration: FavoritesDisplayMode, in context: NSManagedObjectContext) -> [BookmarkEntity] { + // if displayAll - always remove from all + // if displayNative: + // - if favorited on non-native: only remove from native + // - else remove from native and all + let uuids: Set = { + if configuration.isDisplayAll { + return Set(FavoritesPlatform.allCases.map(\.rawValue)) + } + if Set(bookmark.favoriteFoldersSet.compactMap(\.uuid)) == configuration.folderUUIDs { + return configuration.folderUUIDs + } + return Set(arrayLiteral: configuration.nativePlatform.rawValue) + }() + return uuids.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } + } + public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { assert(BookmarkEntity.isValidFavoritesFolderID(uuid)) let request = BookmarkEntity.fetchRequest() diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 848001892..f696432cc 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -119,7 +119,8 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject return } - favorite.removeFromFavorites() + let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(favorite, for: favoritesDisplayMode, in: context) + favorite.removeFromFavorites(folders: folders) save() diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index b2079d5e6..37d8cadf8 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -104,7 +104,8 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { if let bookmark = queriedBookmark { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - bookmark.removeFromFavorites() + let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(bookmark, for: favoritesDisplayMode, in: context) + bookmark.removeFromFavorites(folders: folders) } else { let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) From d27a0ff065005654cacc11dbb85a646047f8af12 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 15 Sep 2023 17:26:48 +0200 Subject: [PATCH 23/56] Move unfavoriting folders logic to BookmarkEntity --- Sources/Bookmarks/BookmarkEditorViewModel.swift | 3 +-- Sources/Bookmarks/BookmarkEntity.swift | 14 ++++++++++++++ Sources/Bookmarks/BookmarkListViewModel.swift | 3 +-- Sources/Bookmarks/BookmarkUtils.swift | 17 ----------------- Sources/Bookmarks/FavoriteListViewModel.swift | 3 +-- Sources/Bookmarks/MenuBookmarksViewModel.swift | 3 +-- 6 files changed, 18 insertions(+), 25 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index c58ecd400..9c3939cdd 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -182,8 +182,7 @@ public class BookmarkEditorViewModel: ObservableObject { public func removeFromFavorites() { assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) - let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(bookmark, for: favoritesDisplayMode, in: context) - bookmark.removeFromFavorites(folders: folders) + bookmark.removeFromFavorites(for: favoritesDisplayMode) } public func addToFavorites() { diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index f6278da28..c9c864c64 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -248,6 +248,20 @@ public class BookmarkEntity: NSManagedObject { } } + public func removeFromFavorites(for displayMode: FavoritesDisplayMode) { + // if displayAll - always remove from all + // if displayNative: + // - if favorited on non-native: only remove from native + // - else remove from native and all + if displayMode.isDisplayAll || Set(favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs { + for root in favoriteFoldersSet { + root.removeFromFavorites(self) + } + } else { + favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue })?.removeFromFavorites(self) + } + } + public func removeFromFavorites(favoritesRoot: BookmarkEntity) { removeFromFavoriteFolders(favoritesRoot) } diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 97d7e88d3..9b3fa8b15 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -113,8 +113,7 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public func toggleFavorite(_ bookmark: BookmarkEntity) { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(bookmark, for: favoritesDisplayMode, in: context) - bookmark.removeFromFavorites(folders: folders) + bookmark.removeFromFavorites(for: favoritesDisplayMode) } else { let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 684b78ce3..0e952d041 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -34,23 +34,6 @@ public struct BookmarkUtils { configuration.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } } - public static func fetchFavoritesFoldersForUnfavoriting(_ bookmark: BookmarkEntity, for configuration: FavoritesDisplayMode, in context: NSManagedObjectContext) -> [BookmarkEntity] { - // if displayAll - always remove from all - // if displayNative: - // - if favorited on non-native: only remove from native - // - else remove from native and all - let uuids: Set = { - if configuration.isDisplayAll { - return Set(FavoritesPlatform.allCases.map(\.rawValue)) - } - if Set(bookmark.favoriteFoldersSet.compactMap(\.uuid)) == configuration.folderUUIDs { - return configuration.folderUUIDs - } - return Set(arrayLiteral: configuration.nativePlatform.rawValue) - }() - return uuids.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } - } - public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { assert(BookmarkEntity.isValidFavoritesFolderID(uuid)) let request = BookmarkEntity.fetchRequest() diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index f696432cc..d083ef9d3 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -119,8 +119,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject return } - let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(favorite, for: favoritesDisplayMode, in: context) - favorite.removeFromFavorites(folders: folders) + favorite.removeFromFavorites(for: favoritesDisplayMode) save() diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 37d8cadf8..784bb97c8 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -104,8 +104,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { if let bookmark = queriedBookmark { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - let folders = BookmarkUtils.fetchFavoritesFoldersForUnfavoriting(bookmark, for: favoritesDisplayMode, in: context) - bookmark.removeFromFavorites(folders: folders) + bookmark.removeFromFavorites(for: favoritesDisplayMode) } else { let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) From 06f52b408a23611d993d2162645651b903f587ae Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Sep 2023 15:01:21 +0200 Subject: [PATCH 24/56] Add BookmarkUtils.favoritesFoldersForUnfavoriting and keep BookmarkEntity unaware of display mode --- Sources/Bookmarks/BookmarkEditorViewModel.swift | 3 ++- Sources/Bookmarks/BookmarkEntity.swift | 14 -------------- Sources/Bookmarks/BookmarkListViewModel.swift | 3 ++- Sources/Bookmarks/BookmarkUtils.swift | 17 +++++++++++++++-- Sources/Bookmarks/FavoriteListViewModel.swift | 3 ++- Sources/Bookmarks/MenuBookmarksViewModel.swift | 3 ++- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index 9c3939cdd..595aeb8ca 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -182,7 +182,8 @@ public class BookmarkEditorViewModel: ObservableObject { public func removeFromFavorites() { assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) - bookmark.removeFromFavorites(for: favoritesDisplayMode) + let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: bookmark, with: favoritesDisplayMode) + bookmark.removeFromFavorites(folders: folders) } public func addToFavorites() { diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index c9c864c64..f6278da28 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -248,20 +248,6 @@ public class BookmarkEntity: NSManagedObject { } } - public func removeFromFavorites(for displayMode: FavoritesDisplayMode) { - // if displayAll - always remove from all - // if displayNative: - // - if favorited on non-native: only remove from native - // - else remove from native and all - if displayMode.isDisplayAll || Set(favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs { - for root in favoriteFoldersSet { - root.removeFromFavorites(self) - } - } else { - favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue })?.removeFromFavorites(self) - } - } - public func removeFromFavorites(favoritesRoot: BookmarkEntity) { removeFromFavoriteFolders(favoritesRoot) } diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 9b3fa8b15..58fa7111c 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -113,7 +113,8 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public func toggleFavorite(_ bookmark: BookmarkEntity) { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - bookmark.removeFromFavorites(for: favoritesDisplayMode) + let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: bookmark, with: favoritesDisplayMode) + bookmark.removeFromFavorites(folders: folders) } else { let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 0e952d041..f8966b415 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -30,8 +30,21 @@ public struct BookmarkUtils { return try? context.fetch(request).first } - public static func fetchFavoritesFolders(for configuration: FavoritesDisplayMode, in context: NSManagedObjectContext) -> [BookmarkEntity] { - configuration.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } + public static func fetchFavoritesFolders(for displayMode: FavoritesDisplayMode, in context: NSManagedObjectContext) -> [BookmarkEntity] { + displayMode.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } + } + + public static func favoritesFoldersForUnfavoriting(of bookmark: BookmarkEntity, with displayMode: FavoritesDisplayMode) -> [BookmarkEntity] { + // if displayAll - always remove from all + // if displayNative: + // - if favorited on non-native: only remove from native + // - else remove from native and all + let isFavoritedOnlyOnNativeFormFactor = Set(bookmark.favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs + + if displayMode.isDisplayAll || isFavoritedOnlyOnNativeFormFactor { + return Array(bookmark.favoriteFoldersSet) + } + return bookmark.favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }).flatMap(Array.init) ?? [] } public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index d083ef9d3..bd6a39cf3 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -119,7 +119,8 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject return } - favorite.removeFromFavorites(for: favoritesDisplayMode) + let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: favorite, with: favoritesDisplayMode) + favorite.removeFromFavorites(folders: folders) save() diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 784bb97c8..56fdb88aa 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -104,7 +104,8 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { if let bookmark = queriedBookmark { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - bookmark.removeFromFavorites(for: favoritesDisplayMode) + let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: bookmark, with: favoritesDisplayMode) + bookmark.removeFromFavorites(folders: folders) } else { let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) bookmark.addToFavorites(folders: folders) From 20ac7bda05aaa453c6b1800c26a51e3e4abc88fa Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Sep 2023 17:46:38 +0200 Subject: [PATCH 25/56] Rename FavoritesPlatform with FavoritesFolderID and remove favorites folder IDs from Constants --- Sources/Bookmarks/BookmarkEntity.swift | 39 +++------ Sources/Bookmarks/BookmarkUtils.swift | 12 +-- .../BookmarksTestsUtils/BookmarkTree.swift | 8 +- .../internal/ReceivedBookmarksIndex.swift | 84 ------------------- ...marksInitialSyncResponseHandlerTests.swift | 2 +- .../Bookmarks/BookmarksProviderTests.swift | 7 +- .../helpers/SyncableBookmarksExtension.swift | 6 +- .../SyncDataProvidersTests/CryptingMock.swift | 6 +- 8 files changed, 33 insertions(+), 131 deletions(-) delete mode 100644 Sources/SyncDataProviders/Bookmarks/internal/ReceivedBookmarksIndex.swift diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index f6278da28..9a062b106 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -22,8 +22,8 @@ import Foundation import CoreData public enum FavoritesDisplayMode: Equatable { - case displayNative(FavoritesPlatform) - case displayAll(native: FavoritesPlatform) + case displayNative(FavoritesFolderID) + case displayAll(native: FavoritesFolderID) public var isDisplayAll: Bool { switch self { @@ -34,7 +34,7 @@ public enum FavoritesDisplayMode: Equatable { } } - public var displayedPlatform: FavoritesPlatform { + public var displayedPlatform: FavoritesFolderID { switch self { case .displayNative(let platform): return platform @@ -43,7 +43,7 @@ public enum FavoritesDisplayMode: Equatable { } } - public var nativePlatform: FavoritesPlatform { + public var nativePlatform: FavoritesFolderID { switch self { case .displayNative(let native), .displayAll(let native): return native @@ -51,11 +51,11 @@ public enum FavoritesDisplayMode: Equatable { } public var folderUUIDs: Set { - return [nativePlatform.rawValue, FavoritesPlatform.all.rawValue] + return [nativePlatform.rawValue, FavoritesFolderID.all.rawValue] } } -public enum FavoritesPlatform: String, CaseIterable { +public enum FavoritesFolderID: String, CaseIterable { case mobile = "mobile_favorites_root" case desktop = "desktop_favorites_root" case all = "favorites_root" @@ -66,26 +66,11 @@ public class BookmarkEntity: NSManagedObject { public enum Constants { public static let rootFolderID = "bookmarks_root" - public static let favoritesFolderID = "favorites_root" - public static let mobileFavoritesFolderID = "mobile_favorites_root" - public static let desktopFavoritesFolderID = "desktop_favorites_root" - - public static let favoriteFoldersIDs: Set = [ - desktopFavoritesFolderID, - mobileFavoritesFolderID, - favoritesFolderID - ] - - public static let allReservedFoldersIDs: Set = [ - rootFolderID, - desktopFavoritesFolderID, - mobileFavoritesFolderID, - favoritesFolderID - ] + public static let favoriteFoldersIDs: Set = Set(FavoritesFolderID.allCases.map(\.rawValue)) } public static func isValidFavoritesFolderID(_ value: String) -> Bool { - return Constants.favoriteFoldersIDs.contains(value) + FavoritesFolderID.allCases.contains { $0.rawValue == value } } public enum Error: Swift.Error { @@ -117,12 +102,12 @@ public class BookmarkEntity: NSManagedObject { /// In-memory flag. When set to `false`, disables adjusting `modifiedAt` on `willSave()`. It's reset to `true` on `didSave()`. public var shouldManageModifiedAt: Bool = true - public func isFavorite(on platform: FavoritesPlatform) -> Bool { + public func isFavorite(on platform: FavoritesFolderID) -> Bool { favoriteFoldersSet.contains { $0.uuid == platform.rawValue } } - public var favoritedOn: [FavoritesPlatform] { - favoriteFoldersSet.compactMap(\.uuid).compactMap(FavoritesPlatform.init) + public var favoritedOn: [FavoritesFolderID] { + favoriteFoldersSet.compactMap(\.uuid).compactMap(FavoritesFolderID.init) } public convenience init(context moc: NSManagedObjectContext) { @@ -144,7 +129,7 @@ public class BookmarkEntity: NSManagedObject { guard !changedKeys.isEmpty, !changedKeys.contains(NSStringFromSelector(#selector(getter: modifiedAt))) else { return } - if isInserted && (uuid == Constants.rootFolderID || uuid.flatMap(Constants.favoriteFoldersIDs.contains) == true) { + if isInserted, let uuid, uuid == Constants.rootFolderID || Self.isValidFavoritesFolderID(uuid) { return } modifiedAt = Date() diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index f8966b415..e9209f41b 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -85,15 +85,15 @@ public struct BookmarkUtils { } } - public static func migrateToFormFactorSpecificFavorites(byCopyingExistingTo platform: FavoritesPlatform, in context: NSManagedObjectContext) { + public static func migrateToFormFactorSpecificFavorites(byCopyingExistingTo platform: FavoritesFolderID, in context: NSManagedObjectContext) { assert(platform != .all, "You must specify either desktop or mobile platform") - guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context) else { + guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.all.rawValue, in: context) else { return } - if BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.desktopFavoritesFolderID, in: context) == nil { - let desktopFavoritesFolder = insertRootFolder(uuid: BookmarkEntity.Constants.desktopFavoritesFolderID, into: context) + if BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.desktop.rawValue, in: context) == nil { + let desktopFavoritesFolder = insertRootFolder(uuid: FavoritesFolderID.desktop.rawValue, into: context) if platform == .desktop { favoritesFolder.favoritesArray.forEach { bookmark in @@ -104,8 +104,8 @@ public struct BookmarkUtils { } } - if BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.mobileFavoritesFolderID, in: context) == nil { - let mobileFavoritesFolder = insertRootFolder(uuid: BookmarkEntity.Constants.mobileFavoritesFolderID, into: context) + if BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: context) == nil { + let mobileFavoritesFolder = insertRootFolder(uuid: FavoritesFolderID.mobile.rawValue, into: context) if platform == .mobile { favoritesFolder.favoritesArray.forEach { bookmark in diff --git a/Sources/BookmarksTestsUtils/BookmarkTree.swift b/Sources/BookmarksTestsUtils/BookmarkTree.swift index 1b12098fb..ad7023fc1 100644 --- a/Sources/BookmarksTestsUtils/BookmarkTree.swift +++ b/Sources/BookmarksTestsUtils/BookmarkTree.swift @@ -60,7 +60,7 @@ public struct ModifiedAtConstraint { } public enum BookmarkTreeNode { - case bookmark(id: String, name: String?, url: String?, favoritedOn: [FavoritesPlatform], modifiedAt: Date?, isDeleted: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) + case bookmark(id: String, name: String?, url: String?, favoritedOn: [FavoritesFolderID], modifiedAt: Date?, isDeleted: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) case folder(id: String, name: String?, children: [BookmarkTreeNode], modifiedAt: Date?, isDeleted: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) public var id: String { @@ -126,13 +126,13 @@ public struct Bookmark: BookmarkTreeNodeConvertible { var id: String var name: String? var url: String? - var favoritedOn: [FavoritesPlatform] + var favoritedOn: [FavoritesFolderID] var modifiedAt: Date? var isDeleted: Bool var isOrphaned: Bool var modifiedAtConstraint: ModifiedAtConstraint? - public init(_ name: String? = nil, id: String? = nil, url: String? = nil, favoritedOn: [FavoritesPlatform] = [], modifiedAt: Date? = nil, isDeleted: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil) { + public init(_ name: String? = nil, id: String? = nil, url: String? = nil, favoritedOn: [FavoritesFolderID] = [], modifiedAt: Date? = nil, isDeleted: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil) { self.id = id ?? UUID().uuidString self.name = name ?? id self.url = (url ?? name) ?? id @@ -203,7 +203,7 @@ public struct BookmarkTree { public func createEntitiesForCheckingModifiedAt(in context: NSManagedObjectContext) -> (BookmarkEntity, [BookmarkEntity], [String: ModifiedAtConstraint]) { let rootFolder = BookmarkUtils.fetchRootFolder(context)! rootFolder.modifiedAt = modifiedAt - let favoritesFolders = FavoritesPlatform.allCases.map { BookmarkUtils.fetchFavoritesFolder(withUUID: $0.rawValue, in: context)! } + let favoritesFolders = FavoritesFolderID.allCases.map { BookmarkUtils.fetchFavoritesFolder(withUUID: $0.rawValue, in: context)! } var orphans = [BookmarkEntity]() var modifiedAtConstraints = [String: ModifiedAtConstraint]() if let modifiedAtConstraint { diff --git a/Sources/SyncDataProviders/Bookmarks/internal/ReceivedBookmarksIndex.swift b/Sources/SyncDataProviders/Bookmarks/internal/ReceivedBookmarksIndex.swift deleted file mode 100644 index 727b05bca..000000000 --- a/Sources/SyncDataProviders/Bookmarks/internal/ReceivedBookmarksIndex.swift +++ /dev/null @@ -1,84 +0,0 @@ -// -// ReceivedBookmarksIndex.swift -// DuckDuckGo -// -// Copyright © 2023 DuckDuckGo. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Bookmarks -import CoreData -import DDGSync -import Foundation - -struct ReceivedBookmarksIndex { - let receivedByUUID: [String: SyncableBookmarkAdapter] - let allReceivedIDs: Set - - let topLevelFoldersSyncables: [SyncableBookmarkAdapter] - let bookmarkSyncablesWithoutParent: [SyncableBookmarkAdapter] - let favoritesUUIDs: [String] - - var entitiesByUUID: [String: BookmarkEntity] = [:] - - init(received: [SyncableBookmarkAdapter], in context: NSManagedObjectContext) { - var syncablesByUUID: [String: SyncableBookmarkAdapter] = [:] - var allUUIDs: Set = [] - var childrenToParents: [String: String] = [:] - var parentFoldersToChildren: [String: [String]] = [:] - var favoritesUUIDs: [String] = [] - - received.forEach { syncable in - guard let uuid = syncable.uuid else { - return - } - syncablesByUUID[uuid] = syncable - - allUUIDs.insert(uuid) - if syncable.isFolder { - allUUIDs.formUnion(syncable.children) - } - - if uuid == BookmarkEntity.Constants.favoritesFolderID { - favoritesUUIDs = syncable.children - } else { - if syncable.isFolder { - parentFoldersToChildren[uuid] = syncable.children - } - syncable.children.forEach { child in - childrenToParents[child] = uuid - } - } - } - - self.allReceivedIDs = allUUIDs - self.receivedByUUID = syncablesByUUID - self.favoritesUUIDs = favoritesUUIDs - - let foldersWithoutParent = Set(parentFoldersToChildren.keys).subtracting(childrenToParents.keys) - topLevelFoldersSyncables = foldersWithoutParent.compactMap { syncablesByUUID[$0] } - - bookmarkSyncablesWithoutParent = allUUIDs.subtracting(childrenToParents.keys) - .subtracting(foldersWithoutParent + [BookmarkEntity.Constants.favoritesFolderID]) - .compactMap { syncablesByUUID[$0] } - - BookmarkEntity.fetchBookmarks(with: allReceivedIDs, in: context) - .forEach { bookmark in - guard let uuid = bookmark.uuid else { - return - } - entitiesByUUID[uuid] = bookmark - } - } -} diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift index 0130b337c..bf4513adb 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift @@ -368,7 +368,7 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase var favoritesFolder: BookmarkEntity! context.performAndWait { - favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context) + favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.all.rawValue, in: context) } XCTAssertNotNil(favoritesFolder.modifiedAt) } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index df4b98b2e..3b9c292a0 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -74,9 +74,8 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { Bookmark("Bookmark 6", id: "6", modifiedAtConstraint: .notNil()) }) - // todo - let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: BookmarkEntity.Constants.favoritesFolderID, in: context)! - XCTAssertNotNil(favoritesFolder.modifiedAt) + let favoritesFolders = BookmarkUtils.fetchFavoritesFolders(for: .displayAll(native: .mobile), in: context) + XCTAssertTrue(favoritesFolders.allSatisfy { $0.modifiedAt != nil }) } } @@ -98,7 +97,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { XCTAssertEqual( Set(changedObjects.compactMap(\.uuid)), - BookmarkEntity.Constants.allReservedFoldersIDs.union(["1"]) + BookmarkEntity.Constants.favoriteFoldersIDs.union(["1", BookmarkEntity.Constants.rootFolderID]) ) } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift b/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift index 5d0f3f931..035744b64 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift @@ -26,15 +26,15 @@ extension Syncable { } static func favoritesFolder(favorites: [String]) -> Syncable { - .folder(id: BookmarkEntity.Constants.favoritesFolderID, children: favorites) + .folder(id: FavoritesFolderID.all.rawValue, children: favorites) } static func mobileFavoritesFolder(favorites: [String]) -> Syncable { - .folder(id: BookmarkEntity.Constants.mobileFavoritesFolderID, children: favorites) + .folder(id: FavoritesFolderID.mobile.rawValue, children: favorites) } static func desktopFavoritesFolder(favorites: [String]) -> Syncable { - .folder(id: BookmarkEntity.Constants.desktopFavoritesFolderID, children: favorites) + .folder(id: FavoritesFolderID.desktop.rawValue, children: favorites) } static func bookmark(_ title: String? = nil, id: String, url: String? = nil, lastModified: String? = nil, isDeleted: Bool = false) -> Syncable { diff --git a/Tests/SyncDataProvidersTests/CryptingMock.swift b/Tests/SyncDataProvidersTests/CryptingMock.swift index 846b95263..32810c95e 100644 --- a/Tests/SyncDataProvidersTests/CryptingMock.swift +++ b/Tests/SyncDataProvidersTests/CryptingMock.swift @@ -23,14 +23,16 @@ import Foundation struct CryptingMock: Crypting { + static let reservedFolderIDs = Set(FavoritesFolderID.allCases.map(\.rawValue)).union([BookmarkEntity.Constants.rootFolderID]) + var _encryptAndBase64Encode: (String) throws -> String = { value in - if [BookmarkEntity.Constants.favoritesFolderID, BookmarkEntity.Constants.rootFolderID].contains(value) { + if Self.reservedFolderIDs.contains(value) { return value } return "encrypted_\(value)" } var _base64DecodeAndDecrypt: (String) throws -> String = { value in - if [BookmarkEntity.Constants.favoritesFolderID, BookmarkEntity.Constants.rootFolderID].contains(value) { + if Self.reservedFolderIDs.contains(value) { return value } return value.dropping(prefix: "encrypted_") From 8f4f67ff271b272871d73e115311ca33145cce26 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Sep 2023 21:35:09 +0200 Subject: [PATCH 26/56] Add display mode related API to BookmarkEntity extension --- .../Bookmarks/BookmarkEditorViewModel.swift | 6 +- Sources/Bookmarks/BookmarkEntity.swift | 36 +-------- Sources/Bookmarks/BookmarkListViewModel.swift | 6 +- Sources/Bookmarks/BookmarkUtils.swift | 12 +++ Sources/Bookmarks/FavoriteListViewModel.swift | 3 +- Sources/Bookmarks/FavoritesDisplayMode.swift | 80 +++++++++++++++++++ .../Bookmarks/MenuBookmarksViewModel.swift | 6 +- 7 files changed, 100 insertions(+), 49 deletions(-) create mode 100644 Sources/Bookmarks/FavoritesDisplayMode.swift diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index 595aeb8ca..1f1583fc2 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -182,14 +182,12 @@ public class BookmarkEditorViewModel: ObservableObject { public func removeFromFavorites() { assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) - let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: bookmark, with: favoritesDisplayMode) - bookmark.removeFromFavorites(folders: folders) + bookmark.removeFromFavorites(with: favoritesDisplayMode) } public func addToFavorites() { assert(!bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) - bookmark.addToFavorites(folders: folders) + bookmark.addToFavorites(with: favoritesDisplayMode, in: context) } public func setParentWithID(_ parentID: NSManagedObjectID) { diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 9a062b106..67767f484 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -21,40 +21,6 @@ import Foundation import CoreData -public enum FavoritesDisplayMode: Equatable { - case displayNative(FavoritesFolderID) - case displayAll(native: FavoritesFolderID) - - public var isDisplayAll: Bool { - switch self { - case .displayNative: - return false - case .displayAll: - return true - } - } - - public var displayedPlatform: FavoritesFolderID { - switch self { - case .displayNative(let platform): - return platform - case .displayAll: - return .all - } - } - - public var nativePlatform: FavoritesFolderID { - switch self { - case .displayNative(let native), .displayAll(let native): - return native - } - } - - public var folderUUIDs: Set { - return [nativePlatform.rawValue, FavoritesFolderID.all.rawValue] - } -} - public enum FavoritesFolderID: String, CaseIterable { case mobile = "mobile_favorites_root" case desktop = "desktop_favorites_root" @@ -234,7 +200,7 @@ public class BookmarkEntity: NSManagedObject { } public func removeFromFavorites(favoritesRoot: BookmarkEntity) { - removeFromFavoriteFolders(favoritesRoot) + favoritesRoot.removeFromFavorites(self) } public func markPendingDeletion() { diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 58fa7111c..2d33c0367 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -113,11 +113,9 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public func toggleFavorite(_ bookmark: BookmarkEntity) { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: bookmark, with: favoritesDisplayMode) - bookmark.removeFromFavorites(folders: folders) + bookmark.removeFromFavorites(with: favoritesDisplayMode) } else { - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) - bookmark.addToFavorites(folders: folders) + bookmark.addToFavorites(with: favoritesDisplayMode, in: context) } save() } diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index e9209f41b..fda1d075c 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -49,6 +49,7 @@ public struct BookmarkUtils { public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { assert(BookmarkEntity.isValidFavoritesFolderID(uuid)) + let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate(format: "%K == %@", #keyPath(BookmarkEntity.uuid), uuid) request.returnsObjectsAsFaults = false @@ -57,6 +58,17 @@ public struct BookmarkUtils { return try? context.fetch(request).first } + public static func fetchFavoritesFolders(withUUIDs uuids: Set, in context: NSManagedObjectContext) -> [BookmarkEntity] { + assert(uuids.allSatisfy { BookmarkEntity.isValidFavoritesFolderID($0) }) + + let request = BookmarkEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K in %@", #keyPath(BookmarkEntity.uuid), uuids) + request.returnsObjectsAsFaults = false + request.fetchLimit = uuids.count + + return (try? context.fetch(request)) ?? [] + } + public static func fetchOrphanedEntities(_ context: NSManagedObjectContext) -> [BookmarkEntity] { let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate( diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index bd6a39cf3..36d5006df 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -119,8 +119,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject return } - let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: favorite, with: favoritesDisplayMode) - favorite.removeFromFavorites(folders: folders) + favorite.removeFromFavorites(with: favoritesDisplayMode) save() diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift new file mode 100644 index 000000000..44088b578 --- /dev/null +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -0,0 +1,80 @@ +// +// FavoritesDisplayMode.swift +// DuckDuckGo +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import CoreData +import Foundation + +public enum FavoritesDisplayMode: Equatable { + case displayNative(FavoritesFolderID) + case displayAll(native: FavoritesFolderID) + + public var isDisplayAll: Bool { + switch self { + case .displayNative: + return false + case .displayAll: + return true + } + } + + public var displayedPlatform: FavoritesFolderID { + switch self { + case .displayNative(let platform): + return platform + case .displayAll: + return .all + } + } + + public var nativePlatform: FavoritesFolderID { + switch self { + case .displayNative(let native), .displayAll(let native): + return native + } + } + + public var folderUUIDs: Set { + return [nativePlatform.rawValue, FavoritesFolderID.all.rawValue] + } +} + +extension BookmarkEntity { + + public func addToFavorites(with displayMode: FavoritesDisplayMode, in context: NSManagedObjectContext) { + let folders = BookmarkUtils.fetchFavoritesFolders(withUUIDs: displayMode.folderUUIDs, in: context) + addToFavorites(folders: folders) + } + + public func removeFromFavorites(with displayMode: FavoritesDisplayMode) { + let affectedFolders: [BookmarkEntity] = { + // if displayAll - always remove from all + // if displayNative: + // - if favorited on non-native: only remove from native + // - else remove from native and all + let isFavoritedOnlyOnNativeFormFactor = Set(favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs + if displayMode.isDisplayAll || isFavoritedOnlyOnNativeFormFactor { + return Array(favoriteFoldersSet) + } + return favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }).flatMap(Array.init) ?? [] + }() + + removeFromFavorites(folders: affectedFolders) + } + +} diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 56fdb88aa..d47794f4e 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -104,11 +104,9 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { if let bookmark = queriedBookmark { if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { - let folders = BookmarkUtils.favoritesFoldersForUnfavoriting(of: bookmark, with: favoritesDisplayMode) - bookmark.removeFromFavorites(folders: folders) + bookmark.removeFromFavorites(with: favoritesDisplayMode) } else { - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) - bookmark.addToFavorites(folders: folders) + bookmark.addToFavorites(with: favoritesDisplayMode, in: context) } } else { let favorite = BookmarkEntity.makeBookmark(title: title, From cb8ef49f6734d1562fa97ff5d56be5aa403f834c Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Sep 2023 21:43:15 +0200 Subject: [PATCH 27/56] Remove unneeded code --- Sources/Bookmarks/BookmarkUtils.swift | 15 +-------------- Sources/Bookmarks/MenuBookmarksViewModel.swift | 3 +-- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index fda1d075c..1e3a7b7c4 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -31,20 +31,7 @@ public struct BookmarkUtils { } public static func fetchFavoritesFolders(for displayMode: FavoritesDisplayMode, in context: NSManagedObjectContext) -> [BookmarkEntity] { - displayMode.folderUUIDs.compactMap { fetchFavoritesFolder(withUUID: $0, in: context) } - } - - public static func favoritesFoldersForUnfavoriting(of bookmark: BookmarkEntity, with displayMode: FavoritesDisplayMode) -> [BookmarkEntity] { - // if displayAll - always remove from all - // if displayNative: - // - if favorited on non-native: only remove from native - // - else remove from native and all - let isFavoritedOnlyOnNativeFormFactor = Set(bookmark.favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs - - if displayMode.isDisplayAll || isFavoritedOnlyOnNativeFormFactor { - return Array(bookmark.favoriteFoldersSet) - } - return bookmark.favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }).flatMap(Array.init) ?? [] + fetchFavoritesFolders(withUUIDs: displayMode.folderUUIDs, in: context) } public static func fetchFavoritesFolder(withUUID uuid: String, in context: NSManagedObjectContext) -> BookmarkEntity? { diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index d47794f4e..4b0714361 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -113,8 +113,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { url: url.absoluteString, parent: rootFolder, context: context) - let folders = BookmarkUtils.fetchFavoritesFolders(for: favoritesDisplayMode, in: context) - favorite.addToFavorites(folders: folders) + favorite.addToFavorites(with: favoritesDisplayMode, in: context) } save() From 6bd32a34ec6ba4d2f3d15280790a588bd471188a Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 18 Sep 2023 22:51:42 +0200 Subject: [PATCH 28/56] Rename FavoritesFolderID all to unified --- Sources/Bookmarks/BookmarkEntity.swift | 2 +- Sources/Bookmarks/BookmarkUtils.swift | 4 ++-- Sources/Bookmarks/FavoritesDisplayMode.swift | 4 ++-- .../FavoriteListViewModelTests.swift | 8 ++++---- ...kmarksInitialSyncResponseHandlerTests.swift | 18 +++++++++--------- ...kmarksRegularSyncResponseHandlerTests.swift | 16 ++++++++-------- .../helpers/SyncableBookmarksExtension.swift | 2 +- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 67767f484..6b14645ee 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -24,7 +24,7 @@ import CoreData public enum FavoritesFolderID: String, CaseIterable { case mobile = "mobile_favorites_root" case desktop = "desktop_favorites_root" - case all = "favorites_root" + case unified = "favorites_root" } @objc(BookmarkEntity) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 1e3a7b7c4..70cd43ba3 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -85,9 +85,9 @@ public struct BookmarkUtils { } public static func migrateToFormFactorSpecificFavorites(byCopyingExistingTo platform: FavoritesFolderID, in context: NSManagedObjectContext) { - assert(platform != .all, "You must specify either desktop or mobile platform") + assert(platform != .unified, "You must specify either desktop or mobile platform") - guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.all.rawValue, in: context) else { + guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.unified.rawValue, in: context) else { return } diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift index 44088b578..343a87afa 100644 --- a/Sources/Bookmarks/FavoritesDisplayMode.swift +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -38,7 +38,7 @@ public enum FavoritesDisplayMode: Equatable { case .displayNative(let platform): return platform case .displayAll: - return .all + return .unified } } @@ -50,7 +50,7 @@ public enum FavoritesDisplayMode: Equatable { } public var folderUUIDs: Set { - return [nativePlatform.rawValue, FavoritesFolderID.all.rawValue] + return [nativePlatform.rawValue, FavoritesFolderID.unified.rawValue] } } diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index a4ac93061..f6411066f 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -69,10 +69,10 @@ final class FavoriteListViewModelTests: XCTestCase { let context = favoriteListViewModel.context let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.mobile, .all], isDeleted: true) - Bookmark(id: "2", favoritedOn: [.mobile, .all]) - Bookmark(id: "3", favoritedOn: [.mobile, .all], isDeleted: true) - Bookmark(id: "4", favoritedOn: [.mobile, .all]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified], isDeleted: true) + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified], isDeleted: true) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) } context.performAndWait { diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift index bf4513adb..60097ebac 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift @@ -160,8 +160,8 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) - Bookmark(id: "4", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "4", favoritedOn: [.unified]) } let received: [Syncable] = [ @@ -172,9 +172,9 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) - Bookmark(id: "4", favoritedOn: [.all]) - Bookmark(id: "3", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "4", favoritedOn: [.unified]) + Bookmark(id: "3", favoritedOn: [.unified]) }) } @@ -351,7 +351,7 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) } let received: [Syncable] = [ @@ -362,13 +362,13 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) - Bookmark(id: "2", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "2", favoritedOn: [.unified]) }) var favoritesFolder: BookmarkEntity! context.performAndWait { - favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.all.rawValue, in: context) + favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.unified.rawValue, in: context) } XCTAssertNotNil(favoritesFolder.modifiedAt) } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index c6835ba58..15fd590b1 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -115,8 +115,8 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) - Bookmark(id: "2", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "2", favoritedOn: [.unified]) } let received: [Syncable] = [ @@ -127,9 +127,9 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) - Bookmark(id: "2", favoritedOn: [.all]) - Bookmark(id: "3", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "2", favoritedOn: [.unified]) + Bookmark(id: "3", favoritedOn: [.unified]) }) } @@ -137,7 +137,7 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) Folder(id: "2") { Bookmark(id: "3") } @@ -151,9 +151,9 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.all]) + Bookmark(id: "1", favoritedOn: [.unified]) Folder(id: "2") { - Bookmark(id: "3", favoritedOn: [.all]) + Bookmark(id: "3", favoritedOn: [.unified]) } }) } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift b/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift index 035744b64..bf30700f4 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/helpers/SyncableBookmarksExtension.swift @@ -26,7 +26,7 @@ extension Syncable { } static func favoritesFolder(favorites: [String]) -> Syncable { - .folder(id: FavoritesFolderID.all.rawValue, children: favorites) + .folder(id: FavoritesFolderID.unified.rawValue, children: favorites) } static func mobileFavoritesFolder(favorites: [String]) -> Syncable { From ee5b87d2ed613648e9ef5d5d1b199fd5c5f8b7af Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 08:47:21 +0200 Subject: [PATCH 29/56] Remove FavoriteListViewModel.favoritesFolder --- Sources/Bookmarks/BookmarkListViewModel.swift | 2 +- Sources/Bookmarks/FavoriteListViewModel.swift | 15 +-------------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 2d33c0367..62fdc5bbf 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -40,8 +40,8 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { private let errorEvents: EventMapping? public init(bookmarksDatabase: CoreDataDatabase, - favoritesDisplayMode: FavoritesDisplayMode, parentID: NSManagedObjectID?, + favoritesDisplayMode: FavoritesDisplayMode, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 36d5006df..2e521e01b 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -29,7 +29,6 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject public var favorites = [BookmarkEntity]() public var favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) { didSet { - _favoritesFolder = nil reloadData() } } @@ -42,18 +41,6 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject private let errorEvents: EventMapping? - private var _favoritesFolder: BookmarkEntity? - private var favoriteFolder: BookmarkEntity? { - if _favoritesFolder == nil { - _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) - - if _favoritesFolder == nil { - errorEvents?.fire(.fetchingRootItemFailed(.favorites)) - } - } - return _favoritesFolder - } - public init(bookmarksDatabase: CoreDataDatabase, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() @@ -95,7 +82,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject } private func refresh() { - guard let favoriteFolder else { + guard let favoriteFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) else { errorEvents?.fire(.fetchingRootItemFailed(.favorites)) favorites = [] return From 1c38686c9257862318c3d913d6c903c04b7f763e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 08:53:58 +0200 Subject: [PATCH 30/56] Restore favoriteFolder in FavoriteListViewModel --- Sources/Bookmarks/FavoriteListViewModel.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 2e521e01b..36d5006df 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -29,6 +29,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject public var favorites = [BookmarkEntity]() public var favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) { didSet { + _favoritesFolder = nil reloadData() } } @@ -41,6 +42,18 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject private let errorEvents: EventMapping? + private var _favoritesFolder: BookmarkEntity? + private var favoriteFolder: BookmarkEntity? { + if _favoritesFolder == nil { + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) + + if _favoritesFolder == nil { + errorEvents?.fire(.fetchingRootItemFailed(.favorites)) + } + } + return _favoritesFolder + } + public init(bookmarksDatabase: CoreDataDatabase, errorEvents: EventMapping?) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() @@ -82,7 +95,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject } private func refresh() { - guard let favoriteFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) else { + guard let favoriteFolder else { errorEvents?.fire(.fetchingRootItemFailed(.favorites)) favorites = [] return From 669537f2bbabe8a3862c10e9d9ff0b022af7d96f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 14:54:10 +0200 Subject: [PATCH 31/56] Update SettingsProviderTests with generic settings test cases --- .../BookmarkListViewModelTests.swift | 2 +- .../Settings/SettingsProviderTests.swift | 181 ++++++++++++++++-- .../helpers/SettingsProviderTestsBase.swift | 9 +- .../helpers/SyncableSettingsExtension.swift | 11 +- .../helpers/TestSettingSyncHandler.swift | 61 ++++++ 5 files changed, 247 insertions(+), 17 deletions(-) create mode 100644 Tests/SyncDataProvidersTests/Settings/helpers/TestSettingSyncHandler.swift diff --git a/Tests/BookmarksTests/BookmarkListViewModelTests.swift b/Tests/BookmarksTests/BookmarkListViewModelTests.swift index d2ad748a7..bb3fd46b0 100644 --- a/Tests/BookmarksTests/BookmarkListViewModelTests.swift +++ b/Tests/BookmarksTests/BookmarkListViewModelTests.swift @@ -67,8 +67,8 @@ final class BookmarkListViewModelTests: XCTestCase { bookmarkListViewModel = BookmarkListViewModel( bookmarksDatabase: bookmarksDatabase, - favoritesDisplayMode: .displayNative(.mobile), parentID: nil, + favoritesDisplayMode: .displayNative(.mobile), errorEvents: eventMapping ) } diff --git a/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift b/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift index a553dc79e..902726e38 100644 --- a/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift @@ -37,7 +37,7 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertEqual(provider.lastSyncTimestamp, "12345") } - func testThatPrepareForFirstSyncClearsLastSyncTimestampAndSetsModifiedAtForEmailSettings() throws { + func testThatPrepareForFirstSyncClearsLastSyncTimestampAndSetsModifiedAtForAllSettings() throws { try emailManager.signIn(username: "dax", token: "secret-token") @@ -50,7 +50,7 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertNil(provider.lastSyncTimestamp) settingsMetadata = fetchAllSettingsMetadata(in: context) - XCTAssertEqual(settingsMetadata.count, 1) + XCTAssertEqual(settingsMetadata.count, 2) XCTAssertTrue(settingsMetadata.allSatisfy { $0.lastModified != nil }) } @@ -67,6 +67,23 @@ final class SettingsProviderTests: SettingsProviderTestsBase { ) } + func testThatFetchChangedObjectsReturnsTestSettingWithNonNilModifiedAt() async throws { + + testSettingSyncHandler.syncedValue = "1" + + let changedObjects = try await provider.fetchChangedObjects(encryptedUsing: crypter).map(SyncableSettingAdapter.init) + + XCTAssertEqual( + Set(changedObjects.compactMap(\.uuid)), + Set([SettingsProvider.Setting.testSetting.key]) + ) + } + + func testThatFetchChangedObjectsReturnsEmptyArrayWhenNothingHasChanged() async throws { + let changedObjects = try await provider.fetchChangedObjects(encryptedUsing: crypter).map(SyncableSettingAdapter.init) + XCTAssertTrue(changedObjects.isEmpty) + } + func testWhenEmailProtectionIsDisabledThenFetchChangedObjectsContainsDeletedSyncable() async throws { let otherEmailManager = EmailManager(storage: MockEmailManagerStorage()) @@ -82,6 +99,21 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertEqual(syncable.uuid, SettingsProvider.Setting.emailProtectionGeneration.key) } + func testWhenTestSettingIsClearedThenFetchChangedObjectsContainsDeletedSyncable() async throws { + + testSettingSyncHandler.syncedValue = "1" + testSettingSyncHandler.syncedValue = nil + + let changedObjects = try await provider.fetchChangedObjects(encryptedUsing: crypter).map(SyncableSettingAdapter.init) + + XCTAssertEqual(changedObjects.count, 1) + + let syncable = try XCTUnwrap(changedObjects.first) + + XCTAssertTrue(syncable.isDeleted) + XCTAssertEqual(syncable.uuid, SettingsProvider.Setting.testSetting.key) + } + func testThatSigninInToEmailProtectionStateUpdatesSyncMetadataTimestamp() async throws { try provider.prepareForFirstSync() @@ -89,9 +121,9 @@ final class SettingsProviderTests: SettingsProviderTestsBase { let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let initialSettingsMetadata = fetchAllSettingsMetadata(in: context) - let initialEmailMetadata = try XCTUnwrap(initialSettingsMetadata.first) + let initialEmailMetadata = try XCTUnwrap(initialSettingsMetadata.first(where: { $0.key == SettingsProvider.Setting.emailProtectionGeneration.key })) let initialTimestamp = initialEmailMetadata.lastModified - XCTAssertEqual(initialSettingsMetadata.count, 1) + XCTAssertEqual(initialSettingsMetadata.count, 2) XCTAssertNotNil(initialTimestamp) try await Task.sleep(nanoseconds: 1000) @@ -102,9 +134,9 @@ final class SettingsProviderTests: SettingsProviderTestsBase { context.refreshAllObjects() let settingsMetadata = fetchAllSettingsMetadata(in: context) - let emailMetadata = try XCTUnwrap(settingsMetadata.first) + let emailMetadata = try XCTUnwrap(settingsMetadata.first(where: { $0.key == SettingsProvider.Setting.emailProtectionGeneration.key })) let timestamp = emailMetadata.lastModified - XCTAssertEqual(settingsMetadata.count, 1) + XCTAssertEqual(settingsMetadata.count, 2) XCTAssertNotNil(timestamp) XCTAssertTrue(timestamp! > initialTimestamp!) @@ -119,9 +151,9 @@ final class SettingsProviderTests: SettingsProviderTestsBase { let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let initialSettingsMetadata = fetchAllSettingsMetadata(in: context) - let initialEmailMetadata = try XCTUnwrap(initialSettingsMetadata.first) + let initialEmailMetadata = try XCTUnwrap(initialSettingsMetadata.first(where: { $0.key == SettingsProvider.Setting.emailProtectionGeneration.key })) let initialTimestamp = initialEmailMetadata.lastModified - XCTAssertEqual(initialSettingsMetadata.count, 1) + XCTAssertEqual(initialSettingsMetadata.count, 2) XCTAssertNotNil(initialTimestamp) try await Task.sleep(nanoseconds: 1000) @@ -132,9 +164,36 @@ final class SettingsProviderTests: SettingsProviderTestsBase { context.refreshAllObjects() let settingsMetadata = fetchAllSettingsMetadata(in: context) - let emailMetadata = try XCTUnwrap(settingsMetadata.first) + let emailMetadata = try XCTUnwrap(settingsMetadata.first(where: { $0.key == SettingsProvider.Setting.emailProtectionGeneration.key })) let timestamp = emailMetadata.lastModified - XCTAssertEqual(settingsMetadata.count, 1) + XCTAssertEqual(settingsMetadata.count, 2) + XCTAssertNotNil(timestamp) + + XCTAssertTrue(timestamp! > initialTimestamp!) + } + + func testThatUpdatingSettingValueUpdatesSyncMetadataTimestamp() async throws { + + try provider.prepareForFirstSync() + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let initialSettingsMetadata = fetchAllSettingsMetadata(in: context) + let initialTestSettingMetadata = try XCTUnwrap(initialSettingsMetadata.first(where: { $0.key == SettingsProvider.Setting.testSetting.key })) + let initialTimestamp = initialTestSettingMetadata.lastModified + XCTAssertEqual(initialSettingsMetadata.count, 2) + XCTAssertNotNil(initialTimestamp) + + try await Task.sleep(nanoseconds: 1000) + + testSettingSyncHandler.syncedValue = "1" + + context.refreshAllObjects() + + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first(where: { $0.key == SettingsProvider.Setting.testSetting.key })) + let timestamp = testSettingMetadata.lastModified + XCTAssertEqual(settingsMetadata.count, 2) XCTAssertNotNil(timestamp) XCTAssertTrue(timestamp! > initialTimestamp!) @@ -169,11 +228,11 @@ final class SettingsProviderTests: SettingsProviderTestsBase { let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let settingsMetadata = fetchAllSettingsMetadata(in: context) - let emailMetadata = try XCTUnwrap(settingsMetadata.first) + let emailMetadata = try XCTUnwrap(settingsMetadata.first(where: { $0.key == SettingsProvider.Setting.emailProtectionGeneration.key })) XCTAssertNil(emailMetadata.lastModified) } - func testThatInitialSyncClearsLastModifiedForDeduplicatedCredential() async throws { + func testThatInitialSyncClearsLastModifiedForDeduplicatedEmailProtectionSetting() async throws { let date = Date() @@ -192,6 +251,24 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertNil(emailMetadata.lastModified) } + func testThatInitialSyncClearsLastModifiedForDeduplicatedSetting() async throws { + + let date = Date() + + testSettingSyncHandler.syncedValue = "1" + + let received: [Syncable] = [ + .testSetting("1") + ] + + try await provider.handleInitialSyncResponse(received: received, clientTimestamp: date.addingTimeInterval(1), serverTimestamp: "1234", crypter: crypter) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + } + func testWhenThereIsMergeConflictDuringInitialSyncThenSyncResponseHandlingIsRetried() async throws { let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) @@ -250,6 +327,24 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertEqual(emailManagerStorage.mockToken, "secret-token2") } + func testWhenSettingDeleteIsSentAndUpdateIsReceivedThenSettingIsNotDeleted() async throws { + testSettingSyncHandler.syncedValue = "local" + testSettingSyncHandler.syncedValue = nil + + let received: [Syncable] = [ + .testSetting("remote") + ] + + let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) + try await provider.handleSyncResponse(sent: sent, received: received, clientTimestamp: Date(), serverTimestamp: "1234", crypter: crypter) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "remote") + } + func testWhenEmailProtectionWasSentAndThenDisabledLocallyAndAnUpdateIsReceivedThenEmailProtectionIsDisabled() async throws { let emailManager = EmailManager(storage: emailManagerStorage) @@ -274,6 +369,28 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertNil(emailManagerStorage.mockToken) } + func testWhenSettingWasSentAndThenDeletedLocallyAndAnUpdateIsReceivedThenSettingIsDeleted() async throws { + + testSettingSyncHandler.syncedValue = "local" + + let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) + + testSettingSyncHandler.syncedValue = nil + + let received: [Syncable] = [ + .testSetting("remote") + ] + + try await provider.handleSyncResponse(sent: sent, received: received, clientTimestamp: Date().advanced(by: -1), serverTimestamp: "1234", crypter: crypter) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + XCTAssertEqual(settingsMetadata.count, 1) + XCTAssertEqual(settingsMetadata.first?.key, SettingsProvider.Setting.testSetting.key) + XCTAssertNotNil(settingsMetadata.first?.lastModified) + XCTAssertNil(testSettingSyncHandler.syncedValue) + } + func testWhenEmailProtectionWasEnabledLocallyAfterStartingSyncThenRemoteChangesAreDropped() async throws { let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) @@ -296,6 +413,26 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertEqual(emailManagerStorage.mockToken, "secret-token") } + func testWhenSettingWasUpdatedLocallyAfterStartingSyncThenRemoteChangesAreDropped() async throws { + + let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) + + testSettingSyncHandler.syncedValue = "local" + + let received: [Syncable] = [ + .testSetting("remote") + ] + + try await provider.handleSyncResponse(sent: sent, received: received, clientTimestamp: Date().advanced(by: -1), serverTimestamp: "1234", crypter: crypter) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + XCTAssertEqual(settingsMetadata.count, 1) + XCTAssertEqual(settingsMetadata.first?.key, SettingsProvider.Setting.testSetting.key) + XCTAssertNotNil(settingsMetadata.first?.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "local") + } + func testWhenEmailProtectionWasEnabledLocallyAfterStartingSyncThenRemoteDisableIsDropped() async throws { let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) @@ -318,6 +455,26 @@ final class SettingsProviderTests: SettingsProviderTestsBase { XCTAssertEqual(emailManagerStorage.mockToken, "secret-token") } + func testWhenSettingWasUpdatedLocallyAfterStartingSyncThenRemoteDeleteIsDropped() async throws { + + let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) + + testSettingSyncHandler.syncedValue = "local" + + let received: [Syncable] = [ + .testSettingDeleted() + ] + + try await provider.handleSyncResponse(sent: sent, received: received, clientTimestamp: Date().advanced(by: -1), serverTimestamp: "1234", crypter: crypter) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + XCTAssertEqual(settingsMetadata.count, 1) + XCTAssertEqual(settingsMetadata.first?.key, SettingsProvider.Setting.testSetting.key) + XCTAssertNotNil(settingsMetadata.first?.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "local") + } + func testWhenThereIsMergeConflictDuringRegularSyncThenSyncResponseHandlingIsRetried() async throws { let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) diff --git a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift index c3835d7ca..59f5f4375 100644 --- a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift +++ b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift @@ -111,6 +111,7 @@ internal class SettingsProviderTestsBase: XCTestCase { var metadataDatabaseLocation: URL! var crypter = CryptingMock() var provider: SettingsProvider! + var testSettingSyncHandler: TestSettingSyncHandler! func setUpSyncMetadataDatabase() { metadataDatabaseLocation = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) @@ -127,15 +128,17 @@ internal class SettingsProviderTestsBase: XCTestCase { override func setUpWithError() throws { super.setUp() - setUpSyncMetadataDatabase() - emailManagerStorage = MockEmailManagerStorage() emailManager = EmailManager(storage: emailManagerStorage) + testSettingSyncHandler = .init() + + setUpSyncMetadataDatabase() + provider = SettingsProvider( metadataDatabase: metadataDatabase, metadataStore: LocalSyncMetadataStore(database: metadataDatabase), emailManager: emailManager, - userDefaultsHandlers: [], + userDefaultsHandlers: [testSettingSyncHandler], syncDidUpdateData: {} ) } diff --git a/Tests/SyncDataProvidersTests/Settings/helpers/SyncableSettingsExtension.swift b/Tests/SyncDataProvidersTests/Settings/helpers/SyncableSettingsExtension.swift index 0948e32f9..57efdae4b 100644 --- a/Tests/SyncDataProvidersTests/Settings/helpers/SyncableSettingsExtension.swift +++ b/Tests/SyncDataProvidersTests/Settings/helpers/SyncableSettingsExtension.swift @@ -17,6 +17,7 @@ // limitations under the License. // +import Bookmarks import BrowserServicesKit import DDGSync import Foundation @@ -44,6 +45,14 @@ extension Syncable { } static func emailProtectionDeleted() -> Syncable { - return Self.settings(SettingsProvider.Setting.emailProtectionGeneration, value: nil, isDeleted: true) + Self.settings(SettingsProvider.Setting.emailProtectionGeneration, value: nil, isDeleted: true) + } + + static func testSetting(_ value: String, lastModified: String? = nil, isDeleted: Bool = false) -> Syncable { + Self.settings(SettingsProvider.Setting.testSetting, value: "encrypted_\(value)", lastModified: lastModified, isDeleted: isDeleted) + } + + static func testSettingDeleted() -> Syncable { + Self.settings(SettingsProvider.Setting.testSetting, value: nil, isDeleted: true) } } diff --git a/Tests/SyncDataProvidersTests/Settings/helpers/TestSettingSyncHandler.swift b/Tests/SyncDataProvidersTests/Settings/helpers/TestSettingSyncHandler.swift new file mode 100644 index 000000000..f7448e249 --- /dev/null +++ b/Tests/SyncDataProvidersTests/Settings/helpers/TestSettingSyncHandler.swift @@ -0,0 +1,61 @@ +// +// TestSettingHandler.swift +// DuckDuckGo +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Bookmarks +import Combine +import Foundation +import SyncDataProviders + +extension SettingsProvider.Setting { + static let testSetting = SettingsProvider.Setting(key: "test_setting") +} + +final class TestSettingSyncHandler: SettingSyncHandler { + + override var setting: SettingsProvider.Setting { + .testSetting + } + + override func getValue() throws -> String? { + syncedValue + } + + override func setValue(_ value: String?) throws { + DispatchQueue.main.async { + self.notifyValueDidChange = false + self.syncedValue = value + } + } + + override var valueDidChangePublisher: AnyPublisher { + $syncedValue.dropFirst().map({ _ in }) + .filter { [weak self] in + self?.notifyValueDidChange == true + } + .eraseToAnyPublisher() + } + + @Published var syncedValue: String? { + didSet { + notifyValueDidChange = true + } + } + + private var notifyValueDidChange: Bool = true +} From 8cddf22d67e2b96f6b7f9a633300568934a678ae Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 15:05:30 +0200 Subject: [PATCH 32/56] Update SettingsInitialSyncResponseHandlerTests with generic settings test cases --- ...tingsInitialSyncResponseHandlerTests.swift | 66 ++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/Tests/SyncDataProvidersTests/Settings/SettingsInitialSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Settings/SettingsInitialSyncResponseHandlerTests.swift index 0561df720..7ef3b9007 100644 --- a/Tests/SyncDataProvidersTests/Settings/SettingsInitialSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Settings/SettingsInitialSyncResponseHandlerTests.swift @@ -63,7 +63,7 @@ final class SettingsInitialSyncResponseHandlerTests: SettingsProviderTestsBase { XCTAssertNil(emailManagerStorage.mockToken) } - func testThatEmailProtectionIsEnabledLocallyAndRemotelyThenRemoteStateIsApplied() async throws { + func testWhenEmailProtectionIsEnabledLocallyAndRemotelyThenRemoteStateIsApplied() async throws { let emailManager = EmailManager(storage: emailManagerStorage) try emailManager.signIn(username: "dax-local", token: "secret-token-local") @@ -98,4 +98,68 @@ final class SettingsInitialSyncResponseHandlerTests: SettingsProviderTestsBase { XCTAssertEqual(emailManagerStorage.mockUsername, "dax") XCTAssertEqual(emailManagerStorage.mockToken, "secret-token") } + + func testThatSettingStateIsAppliedLocally() async throws { + testSettingSyncHandler.syncedValue = nil + + let received: [Syncable] = [ + .testSetting("remote") + ] + + try await handleInitialSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "remote") + } + + func testThatDeletedSettingIsIgnoredWhenLocallyIsNil() async throws { + testSettingSyncHandler.syncedValue = nil + + let received: [Syncable] = [ + .testSettingDeleted() + ] + + try await handleInitialSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertNil(testSettingSyncHandler.syncedValue) + } + + func testWhenSettingIsNotNilLocallyAndRemotelyThenRemoteStateIsApplied() async throws { + testSettingSyncHandler.syncedValue = "local" + + let received: [Syncable] = [ + .testSetting("remote") + ] + + try await handleInitialSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "remote") + } + + func testThatSettingValueIsDeduplicated() async throws { + testSettingSyncHandler.syncedValue = "local" + + let received: [Syncable] = [ + .testSetting("local") + ] + + try await handleInitialSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "local") + } } From 2de2631b37ca27ad035fb10ecb20d0f8dea81e7e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 15:14:34 +0200 Subject: [PATCH 33/56] Update SettingsRegularSyncResponseHandlerTests with generic settings test cases --- ...tingsRegularSyncResponseHandlerTests.swift | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/Tests/SyncDataProvidersTests/Settings/SettingsRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Settings/SettingsRegularSyncResponseHandlerTests.swift index 4db01e946..8d6219617 100644 --- a/Tests/SyncDataProvidersTests/Settings/SettingsRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Settings/SettingsRegularSyncResponseHandlerTests.swift @@ -59,7 +59,7 @@ final class SettingsRegularSyncResponseHandlerTests: SettingsProviderTestsBase { XCTAssertNil(emailManagerStorage.mockToken) } - func testThatEmailProtectionIsEnabledLocallyAndRemotelyThenRemoteStateIsApplied() async throws { + func testWhenEmailProtectionIsEnabledLocallyAndRemotelyThenRemoteStateIsApplied() async throws { let emailManager = EmailManager(storage: emailManagerStorage) try emailManager.signIn(username: "dax-local", token: "secret-token-local") @@ -76,4 +76,49 @@ final class SettingsRegularSyncResponseHandlerTests: SettingsProviderTestsBase { XCTAssertEqual(emailManagerStorage.mockUsername, "dax-remote") XCTAssertEqual(emailManagerStorage.mockToken, "secret-token-remote") } + + func testThatSettingStateIsApplied() async throws { + let received: [Syncable] = [ + .testSetting("remote") + ] + + try await handleSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + XCTAssertTrue(settingsMetadata.isEmpty) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "remote") + } + + func testThatSettingDeletedStateIsApplied() async throws { + testSettingSyncHandler.syncedValue = "local" + + let received: [Syncable] = [ + .testSettingDeleted() + ] + + try await handleSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertNil(testSettingSyncHandler.syncedValue) + } + + func testWhenSettingIsSetLocallyAndRemotelyThenRemoteStateIsApplied() async throws { + testSettingSyncHandler.syncedValue = "local" + + let received: [Syncable] = [ + .testSetting("remote") + ] + + try await handleSyncResponse(received: received) + + let context = metadataDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + let settingsMetadata = fetchAllSettingsMetadata(in: context) + let testSettingMetadata = try XCTUnwrap(settingsMetadata.first) + XCTAssertNil(testSettingMetadata.lastModified) + XCTAssertEqual(testSettingSyncHandler.syncedValue, "remote") + } } From e15e04f56b2f06e6cba07e25ab90e17defea692b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 18:24:24 +0200 Subject: [PATCH 34/56] Update Bookmarks Sync Data Provider tests --- .../BookmarksTestsUtils/BookmarkTree.swift | 2 +- ...marksInitialSyncResponseHandlerTests.swift | 29 ++++++++++--------- .../Bookmarks/BookmarksProviderTests.swift | 8 ++--- ...marksRegularSyncResponseHandlerTests.swift | 18 +++++++----- 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/Sources/BookmarksTestsUtils/BookmarkTree.swift b/Sources/BookmarksTestsUtils/BookmarkTree.swift index ad7023fc1..d96fe2483 100644 --- a/Sources/BookmarksTestsUtils/BookmarkTree.swift +++ b/Sources/BookmarksTestsUtils/BookmarkTree.swift @@ -345,7 +345,7 @@ public extension XCTestCase { XCTAssertEqual(expectedNode.isFolder, thisNode.isFolder, "isFolder mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.isPendingDeletion, thisNode.isPendingDeletion, "isPendingDeletion mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.children?.count, thisNode.children?.count, "children count mismatch for \(thisUUID)", file: file, line: line) - XCTAssertEqual(expectedNode.favoritedOn, thisNode.favoritedOn, "favoritedOn mismatch for \(thisUUID)", file: file, line: line) + XCTAssertEqual(Set(expectedNode.favoritedOn), Set(thisNode.favoritedOn), "favoritedOn mismatch for \(thisUUID)", file: file, line: line) if withTimestamps { if let modifiedAtConstraint = modifiedAtConstraints[thisUUID] { modifiedAtConstraint.check(thisNode.modifiedAt) diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift index 60097ebac..47e7e3b60 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksInitialSyncResponseHandlerTests.swift @@ -137,12 +137,13 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.mobile]) - Bookmark(id: "2", favoritedOn: [.mobile]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) } let received: [Syncable] = [ .rootFolder(children: ["1", "2", "3"]), + .favoritesFolder(favorites: ["1", "2", "3"]), .mobileFavoritesFolder(favorites: ["1", "2"]), .desktopFavoritesFolder(favorites: ["3"]), .bookmark(id: "3") @@ -150,9 +151,9 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.mobile]) - Bookmark(id: "2", favoritedOn: [.mobile]) - Bookmark(id: "3", favoritedOn: [.desktop]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.desktop, .unified]) }) } @@ -160,21 +161,22 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) - Bookmark(id: "4", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) } let received: [Syncable] = [ .rootFolder(children: ["3"]), .favoritesFolder(favorites: ["3"]), + .mobileFavoritesFolder(favorites: ["3"]), .bookmark(id: "3") ] let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) - Bookmark(id: "4", favoritedOn: [.unified]) - Bookmark(id: "3", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) }) } @@ -351,19 +353,20 @@ final class BookmarksInitialSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) } let received: [Syncable] = [ .rootFolder(children: ["2"]), .favoritesFolder(favorites: ["2"]), + .mobileFavoritesFolder(favorites: ["2"]), .bookmark(id: "2") ] let rootFolder = try await createEntitiesAndHandleInitialSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) - Bookmark(id: "2", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) }) var favoritesFolder: BookmarkEntity! diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index 3b9c292a0..cde836100 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -41,7 +41,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark("Bookmark 1", id: "1", favoritedOn: [.mobile]) + Bookmark("Bookmark 1", id: "1", favoritedOn: [.mobile, .unified]) Bookmark("Bookmark 2", id: "2") Folder("Folder", id: "3") { Bookmark("Bookmark 4", id: "4") @@ -65,7 +65,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let rootFolder = BookmarkUtils.fetchRootFolder(context)! assertEquivalent(rootFolder, BookmarkTree(modifiedAtConstraint: .notNil()) { - Bookmark("Bookmark 1", id: "1", favoritedOn: [.mobile], modifiedAtConstraint: .notNil()) + Bookmark("Bookmark 1", id: "1", favoritedOn: [.mobile, .unified], modifiedAtConstraint: .notNil()) Bookmark("Bookmark 2", id: "2", modifiedAtConstraint: .notNil()) Folder("Folder", id: "3", modifiedAtConstraint: .notNil()) { Bookmark("Bookmark 4", id: "4", modifiedAtConstraint: .notNil()) @@ -83,7 +83,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.mobile]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) } context.performAndWait { @@ -105,7 +105,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.mobile]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) Folder(id: "2") { Bookmark(id: "3") Bookmark(id: "4") diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index 15fd590b1..a513be441 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -115,21 +115,22 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) - Bookmark(id: "2", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) } let received: [Syncable] = [ .rootFolder(children: ["1", "2", "3"]), .favoritesFolder(favorites: ["1", "2", "3"]), + .mobileFavoritesFolder(favorites: ["1", "2", "3"]), .bookmark(id: "3") ] let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) - Bookmark(id: "2", favoritedOn: [.unified]) - Bookmark(id: "3", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) }) } @@ -137,7 +138,7 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let bookmarkTree = BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) Folder(id: "2") { Bookmark(id: "3") } @@ -145,15 +146,16 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase let received: [Syncable] = [ .favoritesFolder(favorites: ["1", "3"]), + .mobileFavoritesFolder(favorites: ["1", "3"]), .folder(id: "2", children: ["3"]), .bookmark(id: "3") ] let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { - Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) Folder(id: "2") { - Bookmark(id: "3", favoritedOn: [.unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) } }) } From 36987ed8e265aa0d5deaa1d7e4ceecb15cf4a8ce Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 19 Sep 2023 18:31:22 +0200 Subject: [PATCH 35/56] Add tests to verify that invalid form factor favorites are stored 1:1 by Sync --- ...marksRegularSyncResponseHandlerTests.swift | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index a513be441..a99c32df6 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -474,6 +474,66 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase }) } + // MARK: - Invalid Favorites Form Factors + + func testWhenMobileOnlyFavoriteIsReceivedThenItIsSaved() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2") + } + + let received: [Syncable] = [ + .mobileFavoritesFolder(favorites: ["1"]) + ] + + let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile]) + Bookmark(id: "2") + }) + } + + func testWhenUnifiedOnlyFavoriteIsReceivedThenItIsSaved() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2") + } + + let received: [Syncable] = [ + .favoritesFolder(favorites: ["1"]) + ] + + let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "2") + }) + } + + func testWhenNonUnifiedFavoriteIsReceivedThenItIsSaved() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2") + } + + let received: [Syncable] = [ + .mobileFavoritesFolder(favorites: ["1"]), + .desktopFavoritesFolder(favorites: ["1", "2"]) + ] + + let rootFolder = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop]) + Bookmark(id: "2", favoritedOn: [.desktop]) + }) + } + // MARK: - Helpers func createEntitiesAndHandleSyncResponse( From 8804d654bc10082373239256913e6832bdbf9cfe Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Sep 2023 11:56:57 +0200 Subject: [PATCH 36/56] Fix removing a favorite --- Sources/Bookmarks/FavoritesDisplayMode.swift | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift index 343a87afa..035a81c35 100644 --- a/Sources/Bookmarks/FavoritesDisplayMode.swift +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -58,6 +58,7 @@ extension BookmarkEntity { public func addToFavorites(with displayMode: FavoritesDisplayMode, in context: NSManagedObjectContext) { let folders = BookmarkUtils.fetchFavoritesFolders(withUUIDs: displayMode.folderUUIDs, in: context) + print("Adding to \(folders.compactMap(\.uuid).sorted().joined(separator: ", "))") addToFavorites(folders: folders) } @@ -71,10 +72,18 @@ extension BookmarkEntity { if displayMode.isDisplayAll || isFavoritedOnlyOnNativeFormFactor { return Array(favoriteFoldersSet) } - return favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }).flatMap(Array.init) ?? [] + if let nativeFolder = favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }) { + return [nativeFolder] + } + return [] }() - removeFromFavorites(folders: affectedFolders) + assert(!affectedFolders.isEmpty) + + if !affectedFolders.isEmpty { + print("Removing from \(affectedFolders.compactMap(\.uuid).sorted().joined(separator: ", "))") + removeFromFavorites(folders: affectedFolders) + } } } From 3769cd912b9de35bb1347c6367dc1e7c61257cf5 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Sep 2023 14:43:42 +0200 Subject: [PATCH 37/56] Add a helper function that cleans up favorites after disabling sync --- Sources/Bookmarks/BookmarkUtils.swift | 47 +++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 70cd43ba3..e5df08d54 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -84,8 +84,8 @@ public struct BookmarkUtils { } } - public static func migrateToFormFactorSpecificFavorites(byCopyingExistingTo platform: FavoritesFolderID, in context: NSManagedObjectContext) { - assert(platform != .unified, "You must specify either desktop or mobile platform") + public static func migrateToFormFactorSpecificFavorites(byCopyingExistingTo folderID: FavoritesFolderID, in context: NSManagedObjectContext) { + assert(folderID != .unified, "You must specify either desktop or mobile folder") guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.unified.rawValue, in: context) else { return @@ -94,9 +94,9 @@ public struct BookmarkUtils { if BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.desktop.rawValue, in: context) == nil { let desktopFavoritesFolder = insertRootFolder(uuid: FavoritesFolderID.desktop.rawValue, into: context) - if platform == .desktop { + if folderID == .desktop { favoritesFolder.favoritesArray.forEach { bookmark in - bookmark.addToFavorites(folders: [desktopFavoritesFolder]) + bookmark.addToFavorites(favoritesRoot: desktopFavoritesFolder) } } else { desktopFavoritesFolder.shouldManageModifiedAt = false @@ -106,16 +106,49 @@ public struct BookmarkUtils { if BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: context) == nil { let mobileFavoritesFolder = insertRootFolder(uuid: FavoritesFolderID.mobile.rawValue, into: context) - if platform == .mobile { + if folderID == .mobile { favoritesFolder.favoritesArray.forEach { bookmark in - bookmark.addToFavorites(folders: [mobileFavoritesFolder]) + bookmark.addToFavorites(favoritesRoot: mobileFavoritesFolder) } } else { mobileFavoritesFolder.shouldManageModifiedAt = false } } } - + + public static func copyFavorites( + from sourceFolderID: FavoritesFolderID, + to targetFolderID: FavoritesFolderID, + removingNonNativeFavoritesFrom nonNativeFolderID: FavoritesFolderID, + in context: NSManagedObjectContext + ) { + assert(nonNativeFolderID != .unified, "You must specify either desktop or mobile folder") + assert(Set([sourceFolderID, targetFolderID, nonNativeFolderID]).count == 3, "You must pass 3 different folder IDs to this function") + assert([sourceFolderID, targetFolderID].contains(FavoritesFolderID.unified), "You must copy to or from a unified folder") + + let allFavoritesFolders = BookmarkUtils.fetchFavoritesFolders(withUUIDs: Set(FavoritesFolderID.allCases.map(\.rawValue)), in: context) + assert(allFavoritesFolders.count == FavoritesFolderID.allCases.count, "Favorites folders missing") + + guard let sourceFavoritesFolder = allFavoritesFolders.first(where: { $0.uuid == sourceFolderID.rawValue }), + let targetFavoritesFolder = allFavoritesFolders.first(where: { $0.uuid == targetFolderID.rawValue }), + let nonNativeFormFactorFavoritesFolder = allFavoritesFolders.first(where: { $0.uuid == nonNativeFolderID.rawValue }) + else { + return + } + + nonNativeFormFactorFavoritesFolder.favoritesArray.forEach { bookmark in + bookmark.removeFromFavorites(favoritesRoot: nonNativeFormFactorFavoritesFolder) + } + + targetFavoritesFolder.favoritesArray.forEach { bookmark in + bookmark.removeFromFavorites(favoritesRoot: targetFavoritesFolder) + } + + sourceFavoritesFolder.favoritesArray.forEach { bookmark in + bookmark.addToFavorites(favoritesRoot: targetFavoritesFolder) + } + } + public static func fetchBookmark(for url: URL, predicate: NSPredicate = NSPredicate(value: true), context: NSManagedObjectContext) -> BookmarkEntity? { From 151c635bd757586dc755af720699ab0061181fc3 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Sep 2023 17:58:28 +0200 Subject: [PATCH 38/56] Update favorites display mode definition in view models --- Sources/Bookmarks/BookmarkListViewModel.swift | 6 +++++- Sources/Bookmarks/BookmarksModel.swift | 8 +++++--- Sources/Bookmarks/FavoriteListViewModel.swift | 9 +++++++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index 62fdc5bbf..c04191314 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -27,7 +27,11 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public let currentFolder: BookmarkEntity? let context: NSManagedObjectContext - let favoritesDisplayMode: FavoritesDisplayMode + public var favoritesDisplayMode: FavoritesDisplayMode { + didSet { + reloadData() + } + } public var bookmarks = [BookmarkEntity]() diff --git a/Sources/Bookmarks/BookmarksModel.swift b/Sources/Bookmarks/BookmarksModel.swift index 3a653fc5b..7e3c22f4f 100644 --- a/Sources/Bookmarks/BookmarksModel.swift +++ b/Sources/Bookmarks/BookmarksModel.swift @@ -28,8 +28,10 @@ public protocol BookmarkStoring { func reloadData() } -public protocol BookmarkListInteracting: BookmarkStoring { - +public protocol BookmarkListInteracting: BookmarkStoring, AnyObject { + + var favoritesDisplayMode: FavoritesDisplayMode { get set } + var currentFolder: BookmarkEntity? { get } var bookmarks: [BookmarkEntity] { get } var totalBookmarksCount: Int { get } @@ -52,7 +54,7 @@ public protocol BookmarkListInteracting: BookmarkStoring { } -public protocol FavoritesListInteracting: BookmarkStoring { +public protocol FavoritesListInteracting: BookmarkStoring, AnyObject { var favoritesDisplayMode: FavoritesDisplayMode { get set } diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index 36d5006df..a9b3393c4 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -27,7 +27,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject let context: NSManagedObjectContext public var favorites = [BookmarkEntity]() - public var favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) { + public var favoritesDisplayMode: FavoritesDisplayMode { didSet { _favoritesFolder = nil reloadData() @@ -54,9 +54,14 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject return _favoritesFolder } - public init(bookmarksDatabase: CoreDataDatabase, errorEvents: EventMapping?) { + public init( + bookmarksDatabase: CoreDataDatabase, + errorEvents: EventMapping?, + favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) + ) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() + self.favoritesDisplayMode = favoritesDisplayMode self.errorEvents = errorEvents self.context = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) From ade3c2a4bc3473096be7a434586562762d0daa77 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Sep 2023 23:20:19 +0200 Subject: [PATCH 39/56] Add unit tests for form factor specific favorites to BookmarkListViewModelTests --- Sources/Bookmarks/FavoriteListViewModel.swift | 2 +- .../BookmarkListViewModelTests.swift | 225 ++++++++++++++++++ .../FavoriteListViewModelTests.swift | 6 +- 3 files changed, 231 insertions(+), 2 deletions(-) diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index a9b3393c4..d979676c9 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -57,7 +57,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject public init( bookmarksDatabase: CoreDataDatabase, errorEvents: EventMapping?, - favoritesDisplayMode: FavoritesDisplayMode = .displayNative(.mobile) + favoritesDisplayMode: FavoritesDisplayMode ) { self.externalUpdates = self.subject.eraseToAnyPublisher() self.localUpdates = self.localSubject.eraseToAnyPublisher() diff --git a/Tests/BookmarksTests/BookmarkListViewModelTests.swift b/Tests/BookmarksTests/BookmarkListViewModelTests.swift index bb3fd46b0..065b89e50 100644 --- a/Tests/BookmarksTests/BookmarkListViewModelTests.swift +++ b/Tests/BookmarksTests/BookmarkListViewModelTests.swift @@ -313,6 +313,231 @@ final class BookmarkListViewModelTests: XCTestCase { XCTAssertEqual(firedEvents, [.orphanedBookmarksPresent]) } } + + func testDisplayNativeMode_WhenBookmarkIsFavoritedThenItIsAddedToNativeAndUnifiedFolders() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + }) + } + } + + func testDisplayNativeMode_WhenNonNativeFavoriteIsFavoritedThenItIsAddedToNativeFolder() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + }) + } + } + + func testDisplayNativeMode_WhenNonNativeBrokenFavoriteIsFavoritedThenItIsAddedToNativeAndUnifiedFolder() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + }) + } + } + + func testDisplayNativeMode_WhenFavoriteIsUnfavoritedThenItIsRemovedFromNativeAndUnifiedFolder() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + }) + } + } + + func testDisplayNativeMode_WhenAllFormFactorsFavoriteIsUnfavoritedThenItIsOnlyRemovedFromNativeFolder() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop, .unified]) + }) + } + } + + func testDisplayAllMode_WhenBookmarkIsFavoritedThenItIsAddedToNativeAndUnifiedFolders() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + }) + } + } + + func testDisplayAllMode_WhenNonNativeFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + }) + } + } + + func testDisplayAllMode_WhenNonNativeBrokenFavoriteIsFavoritedThenItIsAddedToNativeAndUnifiedFolder() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + }) + } + } + + func testDisplayAllMode_WhenAllFormFactorsFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { + + bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + let context = bookmarkListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + bookmarkListViewModel.reloadData() + bookmarkListViewModel.toggleFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + }) + } + } } extension BookmarkEntity { diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index f6411066f..266ce6245 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -52,7 +52,11 @@ final class FavoriteListViewModelTests: XCTestCase { try! context.save() } - favoriteListViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, errorEvents: eventMapping) + favoriteListViewModel = FavoritesListViewModel( + bookmarksDatabase: bookmarksDatabase, + errorEvents: eventMapping, + favoritesDisplayMode: .displayNative(.mobile) + ) } override func tearDown() { From 2b5f202293fa373bfd2f3319f076f422081f041f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Sep 2023 23:24:36 +0200 Subject: [PATCH 40/56] Add unit tests for form factor specific favorites to FavoriteListViewModelTests --- .../FavoriteListViewModelTests.swift | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index 266ce6245..5655d61bc 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -98,4 +98,103 @@ final class FavoriteListViewModelTests: XCTestCase { } } + func testDisplayNativeMode_WhenFavoriteIsUnfavoritedThenItIsRemovedFromNativeAndUnifiedFolder() async throws { + + favoriteListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = favoriteListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + favoriteListViewModel.reloadData() + favoriteListViewModel.removeFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + }) + } + } + + func testDisplayNativeMode_WhenAllFormFactorsFavoriteIsUnfavoritedThenItIsOnlyRemovedFromNativeFolder() async throws { + + favoriteListViewModel.favoritesDisplayMode = .displayNative(.mobile) + let context = favoriteListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + favoriteListViewModel.reloadData() + favoriteListViewModel.removeFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop, .unified]) + }) + } + } + + func testDisplayAllMode_WhenNonNativeFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { + + favoriteListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + let context = favoriteListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + favoriteListViewModel.reloadData() + favoriteListViewModel.removeFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + }) + } + } + + func testDisplayAllMode_WhenAllFormFactorsFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { + + favoriteListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + let context = favoriteListViewModel.context + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + try! context.save() + + let bookmark = BookmarkEntity.fetchBookmark(withUUID: "1", context: context)! + + favoriteListViewModel.reloadData() + favoriteListViewModel.removeFavorite(bookmark) + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + }) + } + } } From 6d3a8e5b98c2e55363243efb1f8a8fa26212ccae Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Sep 2023 23:59:50 +0200 Subject: [PATCH 41/56] Add unit tests in BookmarkUtils related to form factor specific favorites --- Sources/Bookmarks/BookmarkUtils.swift | 4 +- .../BookmarksTestsUtils/BookmarkTree.swift | 2 +- Tests/BookmarksTests/BookmarkUtilsTests.swift | 166 ++++++++++++++++++ 3 files changed, 169 insertions(+), 3 deletions(-) create mode 100644 Tests/BookmarksTests/BookmarkUtilsTests.swift diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index e5df08d54..f92e40e50 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -178,10 +178,10 @@ public struct BookmarkUtils { return (try? context.fetch(request)) ?? [] } - // MARK: Private + // MARK: Internal @discardableResult - private static func insertRootFolder(uuid: String, into context: NSManagedObjectContext) -> BookmarkEntity { + static func insertRootFolder(uuid: String, into context: NSManagedObjectContext) -> BookmarkEntity { let folder = BookmarkEntity(entity: BookmarkEntity.entity(in: context), insertInto: context) folder.uuid = uuid diff --git a/Sources/BookmarksTestsUtils/BookmarkTree.swift b/Sources/BookmarksTestsUtils/BookmarkTree.swift index d96fe2483..e50347c4d 100644 --- a/Sources/BookmarksTestsUtils/BookmarkTree.swift +++ b/Sources/BookmarksTestsUtils/BookmarkTree.swift @@ -203,7 +203,7 @@ public struct BookmarkTree { public func createEntitiesForCheckingModifiedAt(in context: NSManagedObjectContext) -> (BookmarkEntity, [BookmarkEntity], [String: ModifiedAtConstraint]) { let rootFolder = BookmarkUtils.fetchRootFolder(context)! rootFolder.modifiedAt = modifiedAt - let favoritesFolders = FavoritesFolderID.allCases.map { BookmarkUtils.fetchFavoritesFolder(withUUID: $0.rawValue, in: context)! } + let favoritesFolders = BookmarkUtils.fetchFavoritesFolders(withUUIDs: Set(FavoritesFolderID.allCases.map(\.rawValue)), in: context) var orphans = [BookmarkEntity]() var modifiedAtConstraints = [String: ModifiedAtConstraint]() if let modifiedAtConstraint { diff --git a/Tests/BookmarksTests/BookmarkUtilsTests.swift b/Tests/BookmarksTests/BookmarkUtilsTests.swift new file mode 100644 index 000000000..72c4d5ba2 --- /dev/null +++ b/Tests/BookmarksTests/BookmarkUtilsTests.swift @@ -0,0 +1,166 @@ +// +// BookmarkUtilsTests.swift +// DuckDuckGo +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import BookmarksTestsUtils +import XCTest +import Persistence +@testable import Bookmarks + +final class BookmarkUtilsTests: XCTestCase { + var bookmarksDatabase: CoreDataDatabase! + var location: URL! + + override func setUp() { + super.setUp() + + location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + + let bundle = Bookmarks.bundle + guard let model = CoreDataDatabase.loadModel(from: bundle, named: "BookmarksModel") else { + XCTFail("Failed to load model") + return + } + bookmarksDatabase = CoreDataDatabase(name: className, containerLocation: location, model: model) + bookmarksDatabase.loadStore() + } + + override func tearDown() { + super.tearDown() + + try? bookmarksDatabase.tearDown(deleteStores: true) + bookmarksDatabase = nil + try? FileManager.default.removeItem(at: location) + } + + func testThatMigrationToFormFactorSpecificFavoritesAddsFavoritesToNativeFolder() async throws { + + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + BookmarkUtils.insertRootFolder(uuid: BookmarkEntity.Constants.rootFolderID, into: context) + BookmarkUtils.insertRootFolder(uuid: FavoritesFolderID.unified.rawValue, into: context) + try! context.save() + } + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2", favoritedOn: [.unified]) + Bookmark(id: "3", favoritedOn: [.unified]) + Bookmark(id: "4", favoritedOn: [.unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + + try! context.save() + + BookmarkUtils.migrateToFormFactorSpecificFavorites(byCopyingExistingTo: .mobile, in: context) + + try! context.save() + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) + }) + } + } + + func testCopyFavoritesWhenDisablingSyncInDisplayNativeMode() async throws { + + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + BookmarkUtils.prepareFoldersStructure(in: context) + try! context.save() + } + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) + Bookmark(id: "5", favoritedOn: [.desktop, .unified]) + Bookmark(id: "6", favoritedOn: [.desktop, .unified]) + Bookmark(id: "7", favoritedOn: [.desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + + try! context.save() + + BookmarkUtils.copyFavorites(from: .mobile, to: .unified, removingNonNativeFavoritesFrom: .desktop, in: context) + + try! context.save() + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) + Bookmark(id: "5") + Bookmark(id: "6") + Bookmark(id: "7") + }) + } + } + + func testCopyFavoritesWhenDisablingSyncInDisplayAllMode() async throws { + + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + BookmarkUtils.prepareFoldersStructure(in: context) + try! context.save() + } + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) + Bookmark(id: "5", favoritedOn: [.desktop, .unified]) + Bookmark(id: "6", favoritedOn: [.desktop, .unified]) + Bookmark(id: "7", favoritedOn: [.desktop, .unified]) + } + + context.performAndWait { + bookmarkTree.createEntities(in: context) + + try! context.save() + + BookmarkUtils.copyFavorites(from: .unified, to: .mobile, removingNonNativeFavoritesFrom: .desktop, in: context) + + try! context.save() + + let rootFolder = BookmarkUtils.fetchRootFolder(context)! + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree { + Bookmark(id: "1") + Bookmark(id: "2", favoritedOn: [.mobile, .unified]) + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) + Bookmark(id: "4", favoritedOn: [.mobile, .unified]) + Bookmark(id: "5", favoritedOn: [.mobile, .unified]) + Bookmark(id: "6", favoritedOn: [.mobile, .unified]) + Bookmark(id: "7", favoritedOn: [.mobile, .unified]) + }) + } + } +} From b9a242c1e342d3ec29f9eade7a790cd8b1cb45c0 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 11:08:54 +0200 Subject: [PATCH 42/56] Add documentation to FavoritesDisplayMode --- Sources/Bookmarks/BookmarkEntity.swift | 6 +++ Sources/Bookmarks/FavoritesDisplayMode.swift | 52 +++++++++++++++++--- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 6b14645ee..26b16a11a 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -21,9 +21,15 @@ import Foundation import CoreData +/** + * This enum defines available favorites folders with their UUIDs as raw value. + */ public enum FavoritesFolderID: String, CaseIterable { + /// Mobile form factor favorites folder case mobile = "mobile_favorites_root" + /// Desktop form factor favorites folder case desktop = "desktop_favorites_root" + /// Unified (mobile + desktop) favorites folder case unified = "favorites_root" } diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift index 035a81c35..59d637f68 100644 --- a/Sources/Bookmarks/FavoritesDisplayMode.swift +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -20,10 +20,41 @@ import CoreData import Foundation +/** + * This enum defines which set of favorites should be displayed to the user. + * + * Users only ever see one set of favorites at a time, and as long as Sync + * is not enabled, it's the one corresponding to the local device (native) + * form factor, i.e. `mobile` on iOS and iPadOS and `desktop` on macOS. + * + * When Sync is enabled, users get to choose between displaying their native + * form factor folder, or a unified folder that contains favorites from + * both mobile and desktop combined. + */ public enum FavoritesDisplayMode: Equatable { + /** + * Display native form factor favorites. + * + * This case takes a parameter that specifies the native form factor. + * It's up to the client app to define its native form factor. + * + * Using a parameter gives the flexibility of overriding the form factor + * on a given client in the future (e.g. treat `desktop` as native form + * factor on the iPad). + */ case displayNative(FavoritesFolderID) + + /** + * Display unified favorites (mobile + desktop combined) + * + * This case takes a parameter that specifies the native form factor. + * It's required because all favorites that are added to or deleted from + * the unified folder need also to be added to or deleted from their + * respective native form factor folder. + */ case displayAll(native: FavoritesFolderID) + /// Returns true if the current mode is to display unified folder. public var isDisplayAll: Bool { switch self { case .displayNative: @@ -33,6 +64,7 @@ public enum FavoritesDisplayMode: Equatable { } } + /// Returns the UUID of a folder that is displayed for a given display mode. public var displayedPlatform: FavoritesFolderID { switch self { case .displayNative(let platform): @@ -42,6 +74,7 @@ public enum FavoritesDisplayMode: Equatable { } } + /// Returns the UUID of a native favorites folder for a given display mode. public var nativePlatform: FavoritesFolderID { switch self { case .displayNative(let native), .displayAll(let native): @@ -49,25 +82,31 @@ public enum FavoritesDisplayMode: Equatable { } } + /// Returns UUIDs of folders that all favorites must be added to in the current display mode. public var folderUUIDs: Set { - return [nativePlatform.rawValue, FavoritesFolderID.unified.rawValue] + [nativePlatform.rawValue, FavoritesFolderID.unified.rawValue] } } extension BookmarkEntity { + /** + * Adds sender to favorites according to `displayMode` passed as argument. + */ public func addToFavorites(with displayMode: FavoritesDisplayMode, in context: NSManagedObjectContext) { let folders = BookmarkUtils.fetchFavoritesFolders(withUUIDs: displayMode.folderUUIDs, in: context) - print("Adding to \(folders.compactMap(\.uuid).sorted().joined(separator: ", "))") addToFavorites(folders: folders) } + /** + * Removes sender from favorites according to `displayMode` passed as argument. + * + * When current mode is to display unified favorites - a favorite is removed from all folders. + * When current mode is to display native form factor - it's removed from the native form factor + * folder, and if it's not favorites on non-native form factor then also removed from unified folder. + */ public func removeFromFavorites(with displayMode: FavoritesDisplayMode) { let affectedFolders: [BookmarkEntity] = { - // if displayAll - always remove from all - // if displayNative: - // - if favorited on non-native: only remove from native - // - else remove from native and all let isFavoritedOnlyOnNativeFormFactor = Set(favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs if displayMode.isDisplayAll || isFavoritedOnlyOnNativeFormFactor { return Array(favoriteFoldersSet) @@ -81,7 +120,6 @@ extension BookmarkEntity { assert(!affectedFolders.isEmpty) if !affectedFolders.isEmpty { - print("Removing from \(affectedFolders.compactMap(\.uuid).sorted().joined(separator: ", "))") removeFromFavorites(folders: affectedFolders) } } From 9fb6381eced089c73f1b79b44ad50c5a69aa3fa5 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 11:09:48 +0200 Subject: [PATCH 43/56] Rename displayAll as displayUnified --- Sources/Bookmarks/FavoritesDisplayMode.swift | 12 ++++++------ .../BookmarksTests/BookmarkListViewModelTests.swift | 8 ++++---- .../BookmarksTests/FavoriteListViewModelTests.swift | 4 ++-- .../Bookmarks/BookmarksProviderTests.swift | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift index 59d637f68..8a68f4a78 100644 --- a/Sources/Bookmarks/FavoritesDisplayMode.swift +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -52,14 +52,14 @@ public enum FavoritesDisplayMode: Equatable { * the unified folder need also to be added to or deleted from their * respective native form factor folder. */ - case displayAll(native: FavoritesFolderID) + case displayUnified(native: FavoritesFolderID) /// Returns true if the current mode is to display unified folder. - public var isDisplayAll: Bool { + public var isDisplayUnified: Bool { switch self { case .displayNative: return false - case .displayAll: + case .displayUnified: return true } } @@ -69,7 +69,7 @@ public enum FavoritesDisplayMode: Equatable { switch self { case .displayNative(let platform): return platform - case .displayAll: + case .displayUnified: return .unified } } @@ -77,7 +77,7 @@ public enum FavoritesDisplayMode: Equatable { /// Returns the UUID of a native favorites folder for a given display mode. public var nativePlatform: FavoritesFolderID { switch self { - case .displayNative(let native), .displayAll(let native): + case .displayNative(let native), .displayUnified(let native): return native } } @@ -108,7 +108,7 @@ extension BookmarkEntity { public func removeFromFavorites(with displayMode: FavoritesDisplayMode) { let affectedFolders: [BookmarkEntity] = { let isFavoritedOnlyOnNativeFormFactor = Set(favoriteFoldersSet.compactMap(\.uuid)) == displayMode.folderUUIDs - if displayMode.isDisplayAll || isFavoritedOnlyOnNativeFormFactor { + if displayMode.isDisplayUnified || isFavoritedOnlyOnNativeFormFactor { return Array(favoriteFoldersSet) } if let nativeFolder = favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }) { diff --git a/Tests/BookmarksTests/BookmarkListViewModelTests.swift b/Tests/BookmarksTests/BookmarkListViewModelTests.swift index 065b89e50..19d6ffeb8 100644 --- a/Tests/BookmarksTests/BookmarkListViewModelTests.swift +++ b/Tests/BookmarksTests/BookmarkListViewModelTests.swift @@ -441,7 +441,7 @@ final class BookmarkListViewModelTests: XCTestCase { func testDisplayAllMode_WhenBookmarkIsFavoritedThenItIsAddedToNativeAndUnifiedFolders() async throws { - bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + bookmarkListViewModel.favoritesDisplayMode = .displayUnified(native: .mobile) let context = bookmarkListViewModel.context let bookmarkTree = BookmarkTree { @@ -466,7 +466,7 @@ final class BookmarkListViewModelTests: XCTestCase { func testDisplayAllMode_WhenNonNativeFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { - bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + bookmarkListViewModel.favoritesDisplayMode = .displayUnified(native: .mobile) let context = bookmarkListViewModel.context let bookmarkTree = BookmarkTree { @@ -491,7 +491,7 @@ final class BookmarkListViewModelTests: XCTestCase { func testDisplayAllMode_WhenNonNativeBrokenFavoriteIsFavoritedThenItIsAddedToNativeAndUnifiedFolder() async throws { - bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + bookmarkListViewModel.favoritesDisplayMode = .displayUnified(native: .mobile) let context = bookmarkListViewModel.context let bookmarkTree = BookmarkTree { @@ -516,7 +516,7 @@ final class BookmarkListViewModelTests: XCTestCase { func testDisplayAllMode_WhenAllFormFactorsFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { - bookmarkListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + bookmarkListViewModel.favoritesDisplayMode = .displayUnified(native: .mobile) let context = bookmarkListViewModel.context let bookmarkTree = BookmarkTree { diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index 5655d61bc..33730fa24 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -150,7 +150,7 @@ final class FavoriteListViewModelTests: XCTestCase { func testDisplayAllMode_WhenNonNativeFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { - favoriteListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + favoriteListViewModel.favoritesDisplayMode = .displayUnified(native: .mobile) let context = favoriteListViewModel.context let bookmarkTree = BookmarkTree { @@ -175,7 +175,7 @@ final class FavoriteListViewModelTests: XCTestCase { func testDisplayAllMode_WhenAllFormFactorsFavoriteIsUnfavoritedThenItIsRemovedFromAllFolders() async throws { - favoriteListViewModel.favoritesDisplayMode = .displayAll(native: .mobile) + favoriteListViewModel.favoritesDisplayMode = .displayUnified(native: .mobile) let context = favoriteListViewModel.context let bookmarkTree = BookmarkTree { diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index cde836100..9cac99338 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -74,7 +74,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { Bookmark("Bookmark 6", id: "6", modifiedAtConstraint: .notNil()) }) - let favoritesFolders = BookmarkUtils.fetchFavoritesFolders(for: .displayAll(native: .mobile), in: context) + let favoritesFolders = BookmarkUtils.fetchFavoritesFolders(for: .displayUnified(native: .mobile), in: context) XCTAssertTrue(favoritesFolders.allSatisfy { $0.modifiedAt != nil }) } } From 5b00317aae702d67458b44867cbe8e67cf0de9f7 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 11:39:08 +0200 Subject: [PATCH 44/56] Rename native/displayedPlatform -> native/displayedFolder --- Sources/Bookmarks/BookmarkEditorViewModel.swift | 6 +++--- Sources/Bookmarks/BookmarkListViewModel.swift | 2 +- Sources/Bookmarks/FavoriteListViewModel.swift | 2 +- Sources/Bookmarks/FavoritesDisplayMode.swift | 10 +++++----- Sources/Bookmarks/MenuBookmarksViewModel.swift | 4 ++-- Tests/BookmarksTests/FavoriteListViewModelTests.swift | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index 1f1583fc2..1890aede6 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -37,7 +37,7 @@ public class BookmarkEditorViewModel: ObservableObject { @Published public var locations = [Location]() lazy var favoritesFolder: BookmarkEntity! = BookmarkUtils.fetchFavoritesFolder( - withUUID: favoritesDisplayMode.displayedPlatform.rawValue, + withUUID: favoritesDisplayMode.displayedFolder.rawValue, in: context ) @@ -181,12 +181,12 @@ public class BookmarkEditorViewModel: ObservableObject { } public func removeFromFavorites() { - assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) + assert(bookmark.isFavorite(on: favoritesDisplayMode.displayedFolder)) bookmark.removeFromFavorites(with: favoritesDisplayMode) } public func addToFavorites() { - assert(!bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform)) + assert(!bookmark.isFavorite(on: favoritesDisplayMode.displayedFolder)) bookmark.addToFavorites(with: favoritesDisplayMode, in: context) } diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index c04191314..a2b3a5c67 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -116,7 +116,7 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { } public func toggleFavorite(_ bookmark: BookmarkEntity) { - if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { + if bookmark.isFavorite(on: favoritesDisplayMode.displayedFolder) { bookmark.removeFromFavorites(with: favoritesDisplayMode) } else { bookmark.addToFavorites(with: favoritesDisplayMode, in: context) diff --git a/Sources/Bookmarks/FavoriteListViewModel.swift b/Sources/Bookmarks/FavoriteListViewModel.swift index d979676c9..f5dfbe33b 100644 --- a/Sources/Bookmarks/FavoriteListViewModel.swift +++ b/Sources/Bookmarks/FavoriteListViewModel.swift @@ -45,7 +45,7 @@ public class FavoritesListViewModel: FavoritesListInteracting, ObservableObject private var _favoritesFolder: BookmarkEntity? private var favoriteFolder: BookmarkEntity? { if _favoritesFolder == nil { - _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedFolder.rawValue, in: context) if _favoritesFolder == nil { errorEvents?.fire(.fetchingRootItemFailed(.favorites)) diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift index 8a68f4a78..8fa1f4f97 100644 --- a/Sources/Bookmarks/FavoritesDisplayMode.swift +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -65,7 +65,7 @@ public enum FavoritesDisplayMode: Equatable { } /// Returns the UUID of a folder that is displayed for a given display mode. - public var displayedPlatform: FavoritesFolderID { + public var displayedFolder: FavoritesFolderID { switch self { case .displayNative(let platform): return platform @@ -75,7 +75,7 @@ public enum FavoritesDisplayMode: Equatable { } /// Returns the UUID of a native favorites folder for a given display mode. - public var nativePlatform: FavoritesFolderID { + public var nativeFolder: FavoritesFolderID { switch self { case .displayNative(let native), .displayUnified(let native): return native @@ -83,8 +83,8 @@ public enum FavoritesDisplayMode: Equatable { } /// Returns UUIDs of folders that all favorites must be added to in the current display mode. - public var folderUUIDs: Set { - [nativePlatform.rawValue, FavoritesFolderID.unified.rawValue] + var folderUUIDs: Set { + [nativeFolder.rawValue, FavoritesFolderID.unified.rawValue] } } @@ -111,7 +111,7 @@ extension BookmarkEntity { if displayMode.isDisplayUnified || isFavoritedOnlyOnNativeFormFactor { return Array(favoriteFoldersSet) } - if let nativeFolder = favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativePlatform.rawValue }) { + if let nativeFolder = favoriteFoldersSet.first(where: { $0.uuid == displayMode.nativeFolder.rawValue }) { return [nativeFolder] } return [] diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 4b0714361..edb25fcad 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -45,7 +45,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { private var _favoritesFolder: BookmarkEntity? private var favoritesFolder: BookmarkEntity? { if _favoritesFolder == nil { - _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedPlatform.rawValue, in: context) + _favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesDisplayMode.displayedFolder.rawValue, in: context) if _favoritesFolder == nil { errorEvents?.fire(.fetchingRootItemFailed(.menu)) @@ -103,7 +103,7 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { let queriedBookmark = favorite(for: url) ?? bookmark(for: url) if let bookmark = queriedBookmark { - if bookmark.isFavorite(on: favoritesDisplayMode.displayedPlatform) { + if bookmark.isFavorite(on: favoritesDisplayMode.displayedFolder) { bookmark.removeFromFavorites(with: favoritesDisplayMode) } else { bookmark.addToFavorites(with: favoritesDisplayMode, in: context) diff --git a/Tests/BookmarksTests/FavoriteListViewModelTests.swift b/Tests/BookmarksTests/FavoriteListViewModelTests.swift index 33730fa24..f01cb2e21 100644 --- a/Tests/BookmarksTests/FavoriteListViewModelTests.swift +++ b/Tests/BookmarksTests/FavoriteListViewModelTests.swift @@ -84,7 +84,7 @@ final class FavoriteListViewModelTests: XCTestCase { try! context.save() - let favoriteFolderUUID = favoriteListViewModel.favoritesDisplayMode.displayedPlatform.rawValue + let favoriteFolderUUID = favoriteListViewModel.favoritesDisplayMode.displayedFolder.rawValue let rootFavoriteFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoriteFolderUUID, in: context)! XCTAssertEqual(rootFavoriteFolder.favoritesArray.map(\.title), ["2", "4"]) From 6fba8ab7201e0844f0f12cef5924433ff709756c Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 11:40:39 +0200 Subject: [PATCH 45/56] Rename SettingsProvider initializers --- .../Settings/SettingsProvider.swift | 12 ++++++------ .../Settings/helpers/SettingsProviderTestsBase.swift | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/SyncDataProviders/Settings/SettingsProvider.swift b/Sources/SyncDataProviders/Settings/SettingsProvider.swift index 2ab8316e1..e39b8ff14 100644 --- a/Sources/SyncDataProviders/Settings/SettingsProvider.swift +++ b/Sources/SyncDataProviders/Settings/SettingsProvider.swift @@ -55,21 +55,21 @@ public final class SettingsProvider: DataProvider, SettingSyncHandlingDelegate { metadataDatabase: CoreDataDatabase, metadataStore: SyncMetadataStore, emailManager: EmailManagerSyncSupporting, - userDefaultsHandlers: [SettingSyncHandler], + settingsHandlers: [SettingSyncHandler], syncDidUpdateData: @escaping () -> Void ) { let emailProtectionSyncHandler = EmailProtectionSyncHandler(emailManager: emailManager) - let userDefaultsHandlersBySetting = userDefaultsHandlers.reduce(into: [Setting: any SettingSyncHandling]()) { partialResult, handler in + let settingsHandlersBySetting = settingsHandlers.reduce(into: [Setting: any SettingSyncHandling]()) { partialResult, handler in partialResult[handler.setting] = handler } - let settingsHandlers = userDefaultsHandlersBySetting + let settingsHandlers = settingsHandlersBySetting .merging([.emailProtectionGeneration: emailProtectionSyncHandler], uniquingKeysWith: { $1 }) self.init( metadataDatabase: metadataDatabase, metadataStore: metadataStore, - settingsHandlers: settingsHandlers, + settingsHandlersBySetting: settingsHandlers, syncDidUpdateData: syncDidUpdateData ) @@ -83,11 +83,11 @@ public final class SettingsProvider: DataProvider, SettingSyncHandlingDelegate { init( metadataDatabase: CoreDataDatabase, metadataStore: SyncMetadataStore, - settingsHandlers: [Setting: any SettingSyncHandling], + settingsHandlersBySetting: [Setting: any SettingSyncHandling], syncDidUpdateData: @escaping () -> Void ) { self.metadataDatabase = metadataDatabase - self.settingsHandlers = settingsHandlers + self.settingsHandlers = settingsHandlersBySetting super.init(feature: .init(name: "settings"), metadataStore: metadataStore, syncDidUpdateData: syncDidUpdateData) } diff --git a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift index 59f5f4375..d15b86da2 100644 --- a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift +++ b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift @@ -138,7 +138,7 @@ internal class SettingsProviderTestsBase: XCTestCase { metadataDatabase: metadataDatabase, metadataStore: LocalSyncMetadataStore(database: metadataDatabase), emailManager: emailManager, - userDefaultsHandlers: [testSettingSyncHandler], + settingsHandlers: [testSettingSyncHandler], syncDidUpdateData: {} ) } From eb24eebf1249dcde9d620c6e0fb27f2bf57495cc Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 13:49:19 +0200 Subject: [PATCH 46/56] Add prepareLegacyFoldersStructure to BookmarkUtils and change copyFavorites function signature --- Sources/Bookmarks/BookmarkUtils.swift | 24 +++++++++++++++++++- Sources/Bookmarks/FavoritesDisplayMode.swift | 11 +++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index f92e40e50..10acbe3f3 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -119,7 +119,7 @@ public struct BookmarkUtils { public static func copyFavorites( from sourceFolderID: FavoritesFolderID, to targetFolderID: FavoritesFolderID, - removingNonNativeFavoritesFrom nonNativeFolderID: FavoritesFolderID, + clearingNonNativeFavoritesFolder nonNativeFolderID: FavoritesFolderID, in context: NSManagedObjectContext ) { assert(nonNativeFolderID != .unified, "You must specify either desktop or mobile folder") @@ -191,3 +191,25 @@ public struct BookmarkUtils { return folder } } + +// MARK: - Legacy Migration Support + +extension BookmarkUtils { + + public static func prepareLegacyFoldersStructure(in context: NSManagedObjectContext) { + + if fetchRootFolder(context) == nil { + insertRootFolder(uuid: BookmarkEntity.Constants.rootFolderID, into: context) + } + + if fetchLegacyFavoritesFolder(context) == nil { + insertRootFolder(uuid: legacyFavoritesFolderID, into: context) + } + } + + static func fetchLegacyFavoritesFolder(_ context: NSManagedObjectContext) -> BookmarkEntity? { + fetchFavoritesFolder(withUUID: legacyFavoritesFolderID, in: context) + } + + static let legacyFavoritesFolderID = FavoritesFolderID.unified.rawValue +} diff --git a/Sources/Bookmarks/FavoritesDisplayMode.swift b/Sources/Bookmarks/FavoritesDisplayMode.swift index 8fa1f4f97..8c1058857 100644 --- a/Sources/Bookmarks/FavoritesDisplayMode.swift +++ b/Sources/Bookmarks/FavoritesDisplayMode.swift @@ -88,6 +88,17 @@ public enum FavoritesDisplayMode: Equatable { } } +extension FavoritesDisplayMode: CustomStringConvertible { + public var description: String { + switch self { + case .displayNative: + return "display_native" + case .displayUnified: + return "display_all" + } + } +} + extension BookmarkEntity { /** From 3ac191b60c3c43fe730fce3bd54d41db636a7518 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 13:50:26 +0200 Subject: [PATCH 47/56] Fix tests compilation --- Tests/BookmarksTests/BookmarkUtilsTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/BookmarksTests/BookmarkUtilsTests.swift b/Tests/BookmarksTests/BookmarkUtilsTests.swift index 72c4d5ba2..75266145b 100644 --- a/Tests/BookmarksTests/BookmarkUtilsTests.swift +++ b/Tests/BookmarksTests/BookmarkUtilsTests.swift @@ -107,7 +107,7 @@ final class BookmarkUtilsTests: XCTestCase { try! context.save() - BookmarkUtils.copyFavorites(from: .mobile, to: .unified, removingNonNativeFavoritesFrom: .desktop, in: context) + BookmarkUtils.copyFavorites(from: .mobile, to: .unified, clearingNonNativeFavoritesFolder: .desktop, in: context) try! context.save() @@ -147,7 +147,7 @@ final class BookmarkUtilsTests: XCTestCase { try! context.save() - BookmarkUtils.copyFavorites(from: .unified, to: .mobile, removingNonNativeFavoritesFrom: .desktop, in: context) + BookmarkUtils.copyFavorites(from: .unified, to: .mobile, clearingNonNativeFavoritesFolder: .desktop, in: context) try! context.save() From 9860874a87cffe0e220c9450ef64d63c36bce369 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 14:59:39 +0200 Subject: [PATCH 48/56] Add FavoritesDisplayModeSyncHandlerBase --- .../FavoritesDisplayModeSyncHandlerBase.swift | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 Sources/SyncDataProviders/Settings/SettingsSyncHandlers/FavoritesDisplayModeSyncHandlerBase.swift diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/FavoritesDisplayModeSyncHandlerBase.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/FavoritesDisplayModeSyncHandlerBase.swift new file mode 100644 index 000000000..0414e1da3 --- /dev/null +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/FavoritesDisplayModeSyncHandlerBase.swift @@ -0,0 +1,32 @@ +// +// FavoritesDisplayModeSyncHandlerBase.swift +// DuckDuckGo +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Bookmarks +import Foundation + +extension SettingsProvider.Setting { + static let favoritesDisplayMode = SettingsProvider.Setting(key: "favorites_display_mode") +} + +open class FavoritesDisplayModeSyncHandlerBase: SettingSyncHandler { + + open override var setting: SettingsProvider.Setting { + .favoritesDisplayMode + } +} From d3679044874ca8d367a879543f7f475a1813ee0b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 15:03:06 +0200 Subject: [PATCH 49/56] Make EmailProtectionSyncHandler public and remove EmailManager from SettingsProvider's parameters --- .../Settings/SettingsProvider.swift | 3 --- .../EmailProtectionSyncHandler.swift | 12 ++++++------ .../Settings/helpers/SettingsProviderTestsBase.swift | 4 ++-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/Sources/SyncDataProviders/Settings/SettingsProvider.swift b/Sources/SyncDataProviders/Settings/SettingsProvider.swift index e39b8ff14..2d48fa078 100644 --- a/Sources/SyncDataProviders/Settings/SettingsProvider.swift +++ b/Sources/SyncDataProviders/Settings/SettingsProvider.swift @@ -54,17 +54,14 @@ public final class SettingsProvider: DataProvider, SettingSyncHandlingDelegate { public convenience init( metadataDatabase: CoreDataDatabase, metadataStore: SyncMetadataStore, - emailManager: EmailManagerSyncSupporting, settingsHandlers: [SettingSyncHandler], syncDidUpdateData: @escaping () -> Void ) { - let emailProtectionSyncHandler = EmailProtectionSyncHandler(emailManager: emailManager) let settingsHandlersBySetting = settingsHandlers.reduce(into: [Setting: any SettingSyncHandling]()) { partialResult, handler in partialResult[handler.setting] = handler } let settingsHandlers = settingsHandlersBySetting - .merging([.emailProtectionGeneration: emailProtectionSyncHandler], uniquingKeysWith: { $1 }) self.init( metadataDatabase: metadataDatabase, diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift index 1402ea3ff..8f4792d78 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift @@ -37,18 +37,18 @@ extension SettingsProvider.Setting { static let emailProtectionGeneration = SettingsProvider.Setting(key: "email_protection_generation") } -class EmailProtectionSyncHandler: SettingSyncHandler { +public final class EmailProtectionSyncHandler: SettingSyncHandler { struct Payload: Codable { let username: String let personalAccessToken: String } - override var setting: SettingsProvider.Setting { + public override var setting: SettingsProvider.Setting { .emailProtectionGeneration } - override func getValue() throws -> String? { + public override func getValue() throws -> String? { guard let user = try emailManager.getUsername() else { return nil } @@ -59,7 +59,7 @@ class EmailProtectionSyncHandler: SettingSyncHandler { return String(bytes: data, encoding: .utf8) } - override func setValue(_ value: String?) throws { + public override func setValue(_ value: String?) throws { guard let value, let valueData = value.data(using: .utf8) else { try emailManager.signOut() return @@ -69,11 +69,11 @@ class EmailProtectionSyncHandler: SettingSyncHandler { try emailManager.signIn(username: payload.username, token: payload.personalAccessToken) } - override var valueDidChangePublisher: AnyPublisher { + public override var valueDidChangePublisher: AnyPublisher { emailManager.userDidToggleEmailProtectionPublisher } - init(emailManager: EmailManagerSyncSupporting) { + public init(emailManager: EmailManagerSyncSupporting) { self.emailManager = emailManager super.init() } diff --git a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift index d15b86da2..081e6e494 100644 --- a/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift +++ b/Tests/SyncDataProvidersTests/Settings/helpers/SettingsProviderTestsBase.swift @@ -130,6 +130,7 @@ internal class SettingsProviderTestsBase: XCTestCase { emailManagerStorage = MockEmailManagerStorage() emailManager = EmailManager(storage: emailManagerStorage) + let emailProtectionSyncHandler = EmailProtectionSyncHandler(emailManager: emailManager) testSettingSyncHandler = .init() setUpSyncMetadataDatabase() @@ -137,8 +138,7 @@ internal class SettingsProviderTestsBase: XCTestCase { provider = SettingsProvider( metadataDatabase: metadataDatabase, metadataStore: LocalSyncMetadataStore(database: metadataDatabase), - emailManager: emailManager, - settingsHandlers: [testSettingSyncHandler], + settingsHandlers: [emailProtectionSyncHandler, testSettingSyncHandler], syncDidUpdateData: {} ) } From 54d524b14ef3a35691acbf0e049c4dc541bb0a19 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 15:06:57 +0200 Subject: [PATCH 50/56] Make emailManager public in EmailProtectionSyncHandler --- .../SettingsSyncHandlers/EmailProtectionSyncHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift index 8f4792d78..9656bcbd2 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift @@ -78,5 +78,5 @@ public final class EmailProtectionSyncHandler: SettingSyncHandler { super.init() } - private let emailManager: EmailManagerSyncSupporting + public let emailManager: EmailManagerSyncSupporting } From 4df09909fed75c90e7c246c622b7a40478871857 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 15:07:33 +0200 Subject: [PATCH 51/56] Revert "Make emailManager public in EmailProtectionSyncHandler" This reverts commit 05557686df39c0c9d9c74660492fa55e4ba6acf6. --- .../SettingsSyncHandlers/EmailProtectionSyncHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift index 9656bcbd2..8f4792d78 100644 --- a/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift +++ b/Sources/SyncDataProviders/Settings/SettingsSyncHandlers/EmailProtectionSyncHandler.swift @@ -78,5 +78,5 @@ public final class EmailProtectionSyncHandler: SettingSyncHandler { super.init() } - public let emailManager: EmailManagerSyncSupporting + private let emailManager: EmailManagerSyncSupporting } From 5e7521f92feac3c7b0a29fd370f8cad0e6b432bb Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 15:41:50 +0200 Subject: [PATCH 52/56] Make BookmarkEditorViewModel.favoritesDisplayMode public --- Sources/Bookmarks/BookmarkEditorViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarkEditorViewModel.swift b/Sources/Bookmarks/BookmarkEditorViewModel.swift index 1890aede6..c680380b3 100644 --- a/Sources/Bookmarks/BookmarkEditorViewModel.swift +++ b/Sources/Bookmarks/BookmarkEditorViewModel.swift @@ -31,7 +31,7 @@ public class BookmarkEditorViewModel: ObservableObject { } let context: NSManagedObjectContext - let favoritesDisplayMode: FavoritesDisplayMode + public let favoritesDisplayMode: FavoritesDisplayMode @Published public var bookmark: BookmarkEntity @Published public var locations = [Location]() From ed952741659d99999ec046d969f6a058c5b4fd45 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 16:23:41 +0200 Subject: [PATCH 53/56] Fix BookmarkListInteracting.createBookmark for FFS favorites --- Sources/Bookmarks/BookmarkListViewModel.swift | 6 +++--- Sources/Bookmarks/BookmarksModel.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index a2b3a5c67..db011373f 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -82,13 +82,13 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { } } - public func createBookmark(title: String, url: String, folder: BookmarkEntity, folderIndex: Int, favoritesFolder: BookmarkEntity?, favoritesIndex: Int?) { + public func createBookmark(title: String, url: String, folder: BookmarkEntity, folderIndex: Int, favoritesFoldersAndIndexes: [BookmarkEntity: Int]) { let bookmark = BookmarkEntity.makeBookmark(title: title, url: url, parent: folder, context: context) if let addedIndex = folder.childrenArray.firstIndex(of: bookmark) { moveBookmark(bookmark, fromIndex: addedIndex, toIndex: folderIndex) } - if let favoritesFolder, let favoritesIndex { - bookmark.addToFavorites(insertAt: favoritesIndex, favoritesRoot: favoritesFolder) + for (favoritesFolder, index) in favoritesFoldersAndIndexes { + bookmark.addToFavorites(insertAt: index, favoritesRoot: favoritesFolder) } save() } diff --git a/Sources/Bookmarks/BookmarksModel.swift b/Sources/Bookmarks/BookmarksModel.swift index 7e3c22f4f..85084c283 100644 --- a/Sources/Bookmarks/BookmarksModel.swift +++ b/Sources/Bookmarks/BookmarksModel.swift @@ -50,7 +50,7 @@ public protocol BookmarkListInteracting: BookmarkStoring, AnyObject { func countBookmarksForDomain(_ domain: String) -> Int - func createBookmark(title: String, url: String, folder: BookmarkEntity, folderIndex: Int, favoritesFolder: BookmarkEntity?, favoritesIndex: Int?) + func createBookmark(title: String, url: String, folder: BookmarkEntity, folderIndex: Int, favoritesFoldersAndIndexes: [BookmarkEntity: Int]) } From 0325036b54c869068e8c654363776c0924d5f6d6 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 21 Sep 2023 16:44:05 +0200 Subject: [PATCH 54/56] Make BookmarkUtils.fetchLegacyFavoritesFolder public --- Sources/Bookmarks/BookmarkUtils.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 10acbe3f3..e40af5ea8 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -207,7 +207,7 @@ extension BookmarkUtils { } } - static func fetchLegacyFavoritesFolder(_ context: NSManagedObjectContext) -> BookmarkEntity? { + public static func fetchLegacyFavoritesFolder(_ context: NSManagedObjectContext) -> BookmarkEntity? { fetchFavoritesFolder(withUUID: legacyFavoritesFolderID, in: context) } From 95a3c212d619d18f72e3fc384572b5f7f2deac4d Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 11 Oct 2023 17:21:12 +0200 Subject: [PATCH 55/56] Make Swiftlint happy --- Sources/Bookmarks/BookmarkListViewModel.swift | 8 ++++- Sources/Bookmarks/BookmarkUtils.swift | 6 ++-- .../internal/BookmarksResponseHandler.swift | 36 +++++++++---------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index db011373f..a2bb9a3d2 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -82,7 +82,13 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { } } - public func createBookmark(title: String, url: String, folder: BookmarkEntity, folderIndex: Int, favoritesFoldersAndIndexes: [BookmarkEntity: Int]) { + public func createBookmark( + title: String, + url: String, + folder: BookmarkEntity, + folderIndex: Int, + favoritesFoldersAndIndexes: [BookmarkEntity: Int] + ) { let bookmark = BookmarkEntity.makeBookmark(title: title, url: url, parent: folder, context: context) if let addedIndex = folder.childrenArray.firstIndex(of: bookmark) { moveBookmark(bookmark, fromIndex: addedIndex, toIndex: folderIndex) diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index e40af5ea8..236b0f2c5 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -77,10 +77,8 @@ public struct BookmarkUtils { insertRootFolder(uuid: BookmarkEntity.Constants.rootFolderID, into: context) } - for uuid in BookmarkEntity.Constants.favoriteFoldersIDs { - if fetchFavoritesFolder(withUUID: uuid, in: context) == nil { - insertRootFolder(uuid: uuid, into: context) - } + for uuid in BookmarkEntity.Constants.favoriteFoldersIDs where fetchFavoritesFolder(withUUID: uuid, in: context) == nil { + insertRootFolder(uuid: uuid, into: context) } } diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index 3daabc899..803b7e06b 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -110,27 +110,25 @@ final class BookmarksResponseHandler { try processOrphanedBookmarks() // populate favorites - for (favoritesFolderUUID, favoritesUUIDs) in favoritesUUIDsByFolderUUID { - if !favoritesUUIDs.isEmpty { - guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesFolderUUID, in: context) else { - // Error - unable to process favorites - return - } + for (favoritesFolderUUID, favoritesUUIDs) in favoritesUUIDsByFolderUUID where !favoritesUUIDs.isEmpty { + guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesFolderUUID, in: 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(favoritesRoot: favoritesFolder) } - } 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(favoritesFolderUUID) - } + // For non-first sync we rely fully on the server response + if !shouldDeduplicateEntities { + favoritesFolder.favoritesArray.forEach { $0.removeFromFavorites(favoritesRoot: favoritesFolder) } + } 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(favoritesFolderUUID) + } - favoritesUUIDs.forEach { uuid in - if let bookmark = entitiesByUUID[uuid] { - bookmark.removeFromFavorites(favoritesRoot: favoritesFolder) - bookmark.addToFavorites(favoritesRoot: favoritesFolder) - } + favoritesUUIDs.forEach { uuid in + if let bookmark = entitiesByUUID[uuid] { + bookmark.removeFromFavorites(favoritesRoot: favoritesFolder) + bookmark.addToFavorites(favoritesRoot: favoritesFolder) } } } From 90681899446e0653b8aeebd7801a3e3f7efa8766 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 31 Oct 2023 14:38:13 +0100 Subject: [PATCH 56/56] Fix syncing empty favorites folders --- .../internal/BookmarksResponseHandler.swift | 2 +- ...marksRegularSyncResponseHandlerTests.swift | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index 803b7e06b..cb5764a18 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -110,7 +110,7 @@ final class BookmarksResponseHandler { try processOrphanedBookmarks() // populate favorites - for (favoritesFolderUUID, favoritesUUIDs) in favoritesUUIDsByFolderUUID where !favoritesUUIDs.isEmpty { + for (favoritesFolderUUID, favoritesUUIDs) in favoritesUUIDsByFolderUUID { guard let favoritesFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: favoritesFolderUUID, in: context) else { // Error - unable to process favorites return diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index a99c32df6..11394c6e0 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -160,6 +160,30 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase }) } + func testWhenPayloadContainsEmptyFavoritesFolderThenAllFavoritesAreRemoved() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree { + Bookmark(id: "1", favoritedOn: [.mobile, .unified]) + Folder(id: "2") { + Bookmark(id: "3", favoritedOn: [.mobile, .unified]) + } + } + + let received: [Syncable] = [ + .favoritesFolder(favorites: []), + .mobileFavoritesFolder(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)