diff --git a/Package.swift b/Package.swift index c3244e754..a8e184702 100644 --- a/Package.swift +++ b/Package.swift @@ -360,6 +360,9 @@ let package = Package( .copy("Resources/Bookmarks_V4.sqlite"), .copy("Resources/Bookmarks_V4.sqlite-shm"), .copy("Resources/Bookmarks_V4.sqlite-wal"), + .copy("Resources/Bookmarks_V5.sqlite"), + .copy("Resources/Bookmarks_V5.sqlite-shm"), + .copy("Resources/Bookmarks_V5.sqlite-wal"), ], plugins: [swiftlintPlugin] ), diff --git a/Sources/Bookmarks/BookmarkEntity.swift b/Sources/Bookmarks/BookmarkEntity.swift index 41dd930b3..8703832cb 100644 --- a/Sources/Bookmarks/BookmarkEntity.swift +++ b/Sources/Bookmarks/BookmarkEntity.swift @@ -63,6 +63,7 @@ public class BookmarkEntity: NSManagedObject { @NSManaged public var title: String? @NSManaged public var url: String? @NSManaged public var uuid: String? + @NSManaged public var isStub: Bool @NSManaged public var children: NSOrderedSet? @NSManaged public fileprivate(set) var lastChildrenPayloadReceivedFromSync: String? @NSManaged public fileprivate(set) var favoriteFolders: NSSet? @@ -127,6 +128,16 @@ public class BookmarkEntity: NSManagedObject { try validate() } + public override func prepareForDeletion() { + super.prepareForDeletion() + + if isFolder { + for child in children?.array as? [BookmarkEntity] ?? [] where child.isStub { + managedObjectContext?.delete(child) + } + } + } + public var urlObject: URL? { guard let url = url else { return nil } return url.isBookmarklet() ? url.toEncodedBookmarklet() : URL(string: url) @@ -138,12 +149,12 @@ public class BookmarkEntity: NSManagedObject { public var childrenArray: [BookmarkEntity] { let children = children?.array as? [BookmarkEntity] ?? [] - return children.filter { $0.isPendingDeletion == false } + return children.filter { $0.isStub == false && $0.isPendingDeletion == false } } public var favoritesArray: [BookmarkEntity] { let children = favorites?.array as? [BookmarkEntity] ?? [] - return children.filter { $0.isPendingDeletion == false } + return children.filter { $0.isStub == false && $0.isPendingDeletion == false } } public var favoriteFoldersSet: Set { diff --git a/Sources/Bookmarks/BookmarkListViewModel.swift b/Sources/Bookmarks/BookmarkListViewModel.swift index d19d34efe..30bb4c93e 100644 --- a/Sources/Bookmarks/BookmarkListViewModel.swift +++ b/Sources/Bookmarks/BookmarkListViewModel.swift @@ -255,9 +255,10 @@ public class BookmarkListViewModel: BookmarkListInteracting, ObservableObject { public var totalBookmarksCount: Int { let countRequest = BookmarkEntity.fetchRequest() countRequest.predicate = NSPredicate( - format: "%K == false && %K == NO", + format: "%K == false && %K == NO && (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.isFolder), - #keyPath(BookmarkEntity.isPendingDeletion) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub) ) return (try? context.count(for: countRequest)) ?? 0 diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift index 3996a7b7c..2a328312b 100644 --- a/Sources/Bookmarks/BookmarkUtils.swift +++ b/Sources/Bookmarks/BookmarkUtils.swift @@ -59,10 +59,11 @@ public struct BookmarkUtils { public static func fetchOrphanedEntities(_ context: NSManagedObjectContext) -> [BookmarkEntity] { let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate( - format: "NOT %K IN %@ AND %K == NO AND %K == nil", + format: "NOT %K IN %@ AND %K == NO AND (%K == NO OR %K == nil) AND %K == nil", #keyPath(BookmarkEntity.uuid), BookmarkEntity.Constants.favoriteFoldersIDs.union([BookmarkEntity.Constants.rootFolderID]), #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.parent) ) request.sortDescriptors = [NSSortDescriptor(key: #keyPath(BookmarkEntity.uuid), ascending: true)] @@ -71,6 +72,17 @@ public struct BookmarkUtils { return (try? context.fetch(request)) ?? [] } + public static func fetchStubbedEntities(_ context: NSManagedObjectContext) -> [BookmarkEntity] { + let request = BookmarkEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K == YES", + #keyPath(BookmarkEntity.isStub) + ) + request.sortDescriptors = [NSSortDescriptor(key: #keyPath(BookmarkEntity.uuid), ascending: true)] + request.returnsObjectsAsFaults = false + + return (try? context.fetch(request)) ?? [] + } + public static func prepareFoldersStructure(in context: NSManagedObjectContext) { if fetchRootFolder(context) == nil { @@ -117,9 +129,10 @@ public struct BookmarkUtils { public static func fetchAllBookmarksUUIDs(in context: NSManagedObjectContext) -> [String] { let request = NSFetchRequest(entityName: "BookmarkEntity") - request.predicate = NSPredicate(format: "%K == NO AND %K == NO", + request.predicate = NSPredicate(format: "%K == NO AND %K == NO AND (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.isFolder), - #keyPath(BookmarkEntity.isPendingDeletion)) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub)) request.resultType = .dictionaryResultType request.propertiesToFetch = [#keyPath(BookmarkEntity.uuid)] @@ -131,10 +144,11 @@ public struct BookmarkUtils { predicate: NSPredicate = NSPredicate(value: true), context: NSManagedObjectContext) -> BookmarkEntity? { let request = BookmarkEntity.fetchRequest() - let urlPredicate = NSPredicate(format: "%K == %@ AND %K == NO", + let urlPredicate = NSPredicate(format: "%K == %@ AND %K == NO AND (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.url), url.absoluteString, - #keyPath(BookmarkEntity.isPendingDeletion)) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub)) request.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [urlPredicate, predicate]) request.returnsObjectsAsFaults = false request.fetchLimit = 1 @@ -144,14 +158,18 @@ public struct BookmarkUtils { public static func fetchBookmarksPendingDeletion(_ context: NSManagedObjectContext) -> [BookmarkEntity] { let request = BookmarkEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K == YES", #keyPath(BookmarkEntity.isPendingDeletion)) + request.predicate = NSPredicate(format: "%K == YES AND (%K == NO OR %K == nil)", + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub)) return (try? context.fetch(request)) ?? [] } public static func fetchModifiedBookmarks(_ context: NSManagedObjectContext) -> [BookmarkEntity] { let request = BookmarkEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K != nil", #keyPath(BookmarkEntity.modifiedAt)) + request.predicate = NSPredicate(format: "%K != nil AND (%K == NO OR %K == nil)", + #keyPath(BookmarkEntity.modifiedAt), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub)) return (try? context.fetch(request)) ?? [] } diff --git a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion index 6801520c4..9950206a9 100644 --- a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion +++ b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/.xccurrentversion @@ -3,6 +3,6 @@ _XCCurrentVersionName - BookmarksModel 5.xcdatamodel + BookmarksModel 6.xcdatamodel diff --git a/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 6.xcdatamodel/contents b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 6.xcdatamodel/contents new file mode 100644 index 000000000..57c23efc5 --- /dev/null +++ b/Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 6.xcdatamodel/contents @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/Sources/Bookmarks/FaviconsFetcher/FaviconsFetchOperation.swift b/Sources/Bookmarks/FaviconsFetcher/FaviconsFetchOperation.swift index 161aed5c8..1ca0d5ed2 100644 --- a/Sources/Bookmarks/FaviconsFetcher/FaviconsFetchOperation.swift +++ b/Sources/Bookmarks/FaviconsFetcher/FaviconsFetchOperation.swift @@ -229,10 +229,11 @@ final class FaviconsFetchOperation: Operation { private func mapBookmarkDomainsToUUIDs(for uuids: any Sequence & CVarArg) -> BookmarkDomains { let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate( - format: "%K IN %@ AND %K == NO AND %K == NO", + format: "%K IN %@ AND %K == NO AND %K == NO AND (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.uuid), uuids, #keyPath(BookmarkEntity.isFolder), - #keyPath(BookmarkEntity.isPendingDeletion) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub) ) request.propertiesToFetch = [#keyPath(BookmarkEntity.uuid), #keyPath(BookmarkEntity.url)] request.relationshipKeyPathsForPrefetching = [#keyPath(BookmarkEntity.favoriteFolders), #keyPath(BookmarkEntity.parent)] diff --git a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift index 77cd73b1c..6c2dd512b 100644 --- a/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift +++ b/Sources/Bookmarks/ImportExport/BookmarkCoreDataImporter.swift @@ -60,9 +60,10 @@ public class BookmarkCoreDataImporter { private func bookmarkURLToID(in context: NSManagedObjectContext) throws -> [String: NSManagedObjectID] { let fetch = NSFetchRequest(entityName: "BookmarkEntity") fetch.predicate = NSPredicate( - format: "%K == false && %K == NO", + format: "%K == false && %K == NO AND (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.isFolder), - #keyPath(BookmarkEntity.isPendingDeletion) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub) ) fetch.resultType = .dictionaryResultType diff --git a/Sources/Bookmarks/MenuBookmarksViewModel.swift b/Sources/Bookmarks/MenuBookmarksViewModel.swift index 0fd251068..7374402c3 100644 --- a/Sources/Bookmarks/MenuBookmarksViewModel.swift +++ b/Sources/Bookmarks/MenuBookmarksViewModel.swift @@ -136,10 +136,11 @@ public class MenuBookmarksViewModel: MenuBookmarksInteracting { } return BookmarkUtils.fetchBookmark(for: url, predicate: NSPredicate( - format: "ANY %K CONTAINS %@ AND %K == NO", + format: "ANY %K CONTAINS %@ AND %K == NO AND (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.favoriteFolders), favoritesFolder, - #keyPath(BookmarkEntity.isPendingDeletion) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub) ), context: context) } diff --git a/Sources/BookmarksTestDBBuilder/BookmarksTestDBBuilder.swift b/Sources/BookmarksTestDBBuilder/BookmarksTestDBBuilder.swift index edca05151..0ecc81305 100644 --- a/Sources/BookmarksTestDBBuilder/BookmarksTestDBBuilder.swift +++ b/Sources/BookmarksTestDBBuilder/BookmarksTestDBBuilder.swift @@ -28,7 +28,7 @@ import Bookmarks struct BookmarksTestDBBuilder { static func main() { - generateDatabase(modelVersion: 3) + generateDatabase(modelVersion: 5) } private static func generateDatabase(modelVersion: Int) { diff --git a/Sources/BookmarksTestsUtils/BookmarkTree.swift b/Sources/BookmarksTestsUtils/BookmarkTree.swift index b1ffe86b3..08ab69bda 100644 --- a/Sources/BookmarksTestsUtils/BookmarkTree.swift +++ b/Sources/BookmarksTestsUtils/BookmarkTree.swift @@ -59,50 +59,59 @@ public struct ModifiedAtConstraint { } public enum BookmarkTreeNode { - 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, lastChildrenArrayReceivedFromSync: [String]?, modifiedAtConstraint: ModifiedAtConstraint?) + case bookmark(id: String, name: String?, url: String?, favoritedOn: [FavoritesFolderID], modifiedAt: Date?, isDeleted: Bool, isStub: Bool, isOrphaned: Bool, modifiedAtConstraint: ModifiedAtConstraint?) + case folder(id: String, name: String?, children: [BookmarkTreeNode], modifiedAt: Date?, isDeleted: Bool, isStub: Bool, isOrphaned: Bool, lastChildrenArrayReceivedFromSync: [String]?, modifiedAtConstraint: ModifiedAtConstraint?) public var id: String { switch self { - case .bookmark(let id, _, _, _, _, _, _, _): + case .bookmark(let id, _, _, _, _, _, _, _, _): return id - case .folder(let id, _, _, _, _, _, _, _): + case .folder(let id, _, _, _, _, _, _, _, _): return id } } public var name: String? { switch self { - case .bookmark(_, let name, _, _, _, _, _, _): + case .bookmark(_, let name, _, _, _, _, _, _, _): return name - case .folder(_, let name, _, _, _, _, _, _): + case .folder(_, let name, _, _, _, _, _, _, _): return name } } public var modifiedAt: Date? { switch self { - case .bookmark(_, _, _, _, let modifiedAt, _, _, _): + case .bookmark(_, _, _, _, let modifiedAt, _, _, _, _): return modifiedAt - case .folder(_, _, _, let modifiedAt, _, _, _, _): + case .folder(_, _, _, let modifiedAt, _, _, _, _, _): return modifiedAt } } public var isDeleted: Bool { switch self { - case .bookmark(_, _, _, _, _, let isDeleted, _, _): + case .bookmark(_, _, _, _, _, let isDeleted, _, _, _): return isDeleted - case .folder(_, _, _, _, let isDeleted, _, _, _): + case .folder(_, _, _, _, let isDeleted, _, _, _, _): return isDeleted } } + public var isStub: Bool { + switch self { + case .bookmark(_, _, _, _, _, _, let isStub, _, _): + return isStub + case .folder(_, _, _, _, _, let isStub, _, _, _): + return isStub + } + } + public var isOrphaned: Bool { switch self { - case .bookmark(_, _, _, _, _, _, let isOrphaned, _): + case .bookmark(_, _, _, _, _, _, _, let isOrphaned, _): return isOrphaned - case .folder(_, _, _, _, _, let isOrphaned, _, _): + case .folder(_, _, _, _, _, _, let isOrphaned, _, _): return isOrphaned } } @@ -111,16 +120,16 @@ public enum BookmarkTreeNode { switch self { case .bookmark: return nil - case .folder(_, _, _, _, _, _, let lastChildrenArrayReceivedFromSync, _): + case .folder(_, _, _, _, _, _, _, let lastChildrenArrayReceivedFromSync, _): return lastChildrenArrayReceivedFromSync } } public var modifiedAtConstraint: ModifiedAtConstraint? { switch self { - case .bookmark(_, _, _, _, _, _, _, let modifiedAtConstraint): + case .bookmark(_, _, _, _, _, _, _, _, let modifiedAtConstraint): return modifiedAtConstraint - case .folder(_, _, _, _, _, _, _, let modifiedAtConstraint): + case .folder(_, _, _, _, _, _, _, _, let modifiedAtConstraint): return modifiedAtConstraint } } @@ -137,22 +146,29 @@ public struct Bookmark: BookmarkTreeNodeConvertible { var favoritedOn: [FavoritesFolderID] var modifiedAt: Date? var isDeleted: Bool + var isStub: Bool var isOrphaned: Bool var modifiedAtConstraint: ModifiedAtConstraint? - 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) { + public init(_ name: String? = nil, id: String? = nil, url: String? = nil, favoritedOn: [FavoritesFolderID] = [], modifiedAt: Date? = nil, isDeleted: Bool = false, isStub: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil) { self.id = id ?? UUID().uuidString - self.name = name ?? id - self.url = (url ?? name) ?? id + if isStub { + self.name = nil + self.url = nil + } else { + self.name = name ?? id + self.url = (url ?? name) ?? id + } self.favoritedOn = favoritedOn self.modifiedAt = modifiedAt self.isDeleted = isDeleted + self.isStub = isStub self.modifiedAtConstraint = modifiedAtConstraint self.isOrphaned = isOrphaned } public func asBookmarkTreeNode() -> BookmarkTreeNode { - .bookmark(id: id, name: name, url: url, favoritedOn: favoritedOn, modifiedAt: modifiedAt, isDeleted: isDeleted, isOrphaned: isOrphaned, modifiedAtConstraint: modifiedAtConstraint) + .bookmark(id: id, name: name, url: url, favoritedOn: favoritedOn, modifiedAt: modifiedAt, isDeleted: isDeleted, isStub: isStub, isOrphaned: isOrphaned, modifiedAtConstraint: modifiedAtConstraint) } } @@ -161,28 +177,30 @@ public struct Folder: BookmarkTreeNodeConvertible { var name: String? var modifiedAt: Date? var isDeleted: Bool + var isStub: Bool var isOrphaned: Bool var modifiedAtConstraint: ModifiedAtConstraint? var lastChildrenArrayReceivedFromSync: [String]? var children: [BookmarkTreeNode] - public init(_ name: String? = nil, id: String? = nil, modifiedAt: Date? = nil, isDeleted: Bool = false, isOrphaned: Bool = false, lastChildrenArrayReceivedFromSync: [String]? = nil, @BookmarkTreeBuilder children: () -> [BookmarkTreeNode] = { [] }) { - self.init(name, id: id, modifiedAt: modifiedAt, isDeleted: isDeleted, isOrphaned: isOrphaned, modifiedAtConstraint: nil, lastChildrenArrayReceivedFromSync: lastChildrenArrayReceivedFromSync, children: children) + public init(_ name: String? = nil, id: String? = nil, modifiedAt: Date? = nil, isDeleted: Bool = false, isStub: Bool = false, isOrphaned: Bool = false, lastChildrenArrayReceivedFromSync: [String]? = nil, @BookmarkTreeBuilder children: () -> [BookmarkTreeNode] = { [] }) { + self.init(name, id: id, modifiedAt: modifiedAt, isDeleted: isDeleted, isStub: isStub, isOrphaned: isOrphaned, modifiedAtConstraint: nil, lastChildrenArrayReceivedFromSync: lastChildrenArrayReceivedFromSync, children: children) } - public init(_ name: String? = nil, id: String? = nil, modifiedAt: Date? = nil, isDeleted: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil, lastChildrenArrayReceivedFromSync: [String]? = nil, @BookmarkTreeBuilder children: () -> [BookmarkTreeNode] = { [] }) { + public init(_ name: String? = nil, id: String? = nil, modifiedAt: Date? = nil, isDeleted: Bool = false, isStub: Bool = false, isOrphaned: Bool = false, modifiedAtConstraint: ModifiedAtConstraint? = nil, lastChildrenArrayReceivedFromSync: [String]? = nil, @BookmarkTreeBuilder children: () -> [BookmarkTreeNode] = { [] }) { self.id = id ?? UUID().uuidString self.name = name ?? id self.modifiedAt = modifiedAt self.isDeleted = isDeleted self.isOrphaned = isOrphaned + self.isStub = isStub self.lastChildrenArrayReceivedFromSync = lastChildrenArrayReceivedFromSync self.modifiedAtConstraint = modifiedAtConstraint self.children = children() } public func asBookmarkTreeNode() -> BookmarkTreeNode { - .folder(id: id, name: name, children: children, modifiedAt: modifiedAt, isDeleted: isDeleted, isOrphaned: isOrphaned, lastChildrenArrayReceivedFromSync: lastChildrenArrayReceivedFromSync, modifiedAtConstraint: modifiedAtConstraint) + .folder(id: id, name: name, children: children, modifiedAt: modifiedAt, isDeleted: isDeleted, isStub: isStub, isOrphaned: isOrphaned, lastChildrenArrayReceivedFromSync: lastChildrenArrayReceivedFromSync, modifiedAtConstraint: modifiedAtConstraint) } } @@ -263,7 +281,7 @@ public extension BookmarkEntity { let node = queue.removeFirst() switch node { - case .bookmark(let id, let name, let url, let favoritedOn, let modifiedAt, let isDeleted, let isOrphaned, let modifiedAtConstraint): + case .bookmark(let id, let name, let url, let favoritedOn, let modifiedAt, let isDeleted, let isStub, let isOrphaned, let modifiedAtConstraint): let bookmarkEntity = BookmarkEntity(context: context) if entity == nil { entity = bookmarkEntity @@ -272,6 +290,7 @@ public extension BookmarkEntity { bookmarkEntity.isFolder = false bookmarkEntity.title = name bookmarkEntity.url = url + bookmarkEntity.isStub = isStub bookmarkEntity.modifiedAt = modifiedAt modifiedAtConstraints[id] = modifiedAtConstraint @@ -287,7 +306,7 @@ public extension BookmarkEntity { if !isOrphaned { bookmarkEntity.parent = parent } - case .folder(let id, let name, let children, let modifiedAt, let isDeleted, let isOrphaned, let lastChildrenArrayReceivedFromSync, let modifiedAtConstraint): + case .folder(let id, let name, let children, let modifiedAt, let isDeleted, let isStub, let isOrphaned, let lastChildrenArrayReceivedFromSync, let modifiedAtConstraint): let bookmarkEntity = BookmarkEntity(context: context) if entity == nil { entity = bookmarkEntity @@ -295,6 +314,7 @@ public extension BookmarkEntity { bookmarkEntity.uuid = id bookmarkEntity.isFolder = true bookmarkEntity.title = name + bookmarkEntity.isStub = isStub bookmarkEntity.modifiedAt = modifiedAt modifiedAtConstraints[id] = modifiedAtConstraint if isDeleted { @@ -361,9 +381,10 @@ public extension XCTestCase { XCTAssertEqual(expectedNode.uuid, thisNode.uuid, "uuid mismatch", file: file, line: line) XCTAssertEqual(expectedNode.title, thisNode.title, "title mismatch for \(thisUUID)", file: file, line: line) XCTAssertEqual(expectedNode.url, thisNode.url, "url mismatch for \(thisUUID)", file: file, line: line) + XCTAssertEqual(expectedNode.isStub, thisNode.isStub, "stub mismatch for \(thisUUID)", file: file, line: line) 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.childrenArray.count, thisNode.childrenArray.count, "children count 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] { @@ -377,9 +398,9 @@ public extension XCTestCase { if withLastChildrenArrayReceivedFromSync { XCTAssertEqual(expectedNode.lastChildrenArrayReceivedFromSync, thisNode.lastChildrenArrayReceivedFromSync, "lastChildrenArrayReceivedFromSync mismatch for \(thisUUID)", file: file, line: line) } - XCTAssertEqual(expectedNode.childrenArray.count, thisNode.childrenArray.count, "children count mismatch for \(thisUUID)", file: file, line: line) - expectedTreeQueue.append(contentsOf: expectedNode.childrenArray) - thisTreeQueue.append(contentsOf: thisNode.childrenArray) + XCTAssertEqual(expectedNode.children?.count, thisNode.children?.count, "children count mismatch for \(thisUUID)", file: file, line: line) + expectedTreeQueue.append(contentsOf: (expectedNode.children?.array as? [BookmarkEntity]) ?? []) + thisTreeQueue.append(contentsOf: (thisNode.children?.array as? [BookmarkEntity]) ?? []) } } } diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarkEntity+Syncable.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarkEntity+Syncable.swift index 1247d0df0..16cf01251 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarkEntity+Syncable.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarkEntity+Syncable.swift @@ -113,6 +113,7 @@ extension BookmarkEntity { cancelDeletion() modifiedAt = nil + isStub = false if let encryptedTitle = syncable.encryptedTitle { title = try decrypt(encryptedTitle) diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift index 638b63ac4..7e665ac8c 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift @@ -127,6 +127,7 @@ final class BookmarksResponseHandler { } try processOrphanedBookmarks() processReceivedFavorites() + cleanupOrphanedStubs() } // MARK: - Private @@ -140,7 +141,8 @@ final class BookmarksResponseHandler { // For non-first sync we rely fully on the server response if !shouldDeduplicateEntities { - favoritesFolder.favoritesArray.forEach { $0.removeFromFavorites(favoritesRoot: favoritesFolder) } + let favorites = favoritesFolder.favorites?.array as? [BookmarkEntity] ?? [] + favorites.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. @@ -151,6 +153,13 @@ final class BookmarksResponseHandler { if let bookmark = entitiesByUUID[uuid] { bookmark.removeFromFavorites(favoritesRoot: favoritesFolder) bookmark.addToFavorites(favoritesRoot: favoritesFolder) + } else { + let newStubEntity = BookmarkEntity.make(withUUID: uuid, + isFolder: false, + in: context) + newStubEntity.isStub = true + newStubEntity.addToFavorites(favoritesRoot: favoritesFolder) + entitiesByUUID[uuid] = newStubEntity } } @@ -158,6 +167,14 @@ final class BookmarksResponseHandler { } } + private func cleanupOrphanedStubs() { + let stubs = BookmarkUtils.fetchStubbedEntities(context) + + for stub in stubs where stub.parent == nil && (stub.favoriteFolders?.count ?? 0) == 0 { + context.delete(stub) + } + } + private func processTopLevelFolder(_ topLevelFolderSyncable: SyncableBookmarkAdapter) throws { guard let topLevelFolderUUID = topLevelFolderSyncable.uuid else { return @@ -171,11 +188,10 @@ final class BookmarksResponseHandler { var queue = queues.removeFirst() let parentUUID = parentUUIDs.removeFirst() let parent = BookmarkEntity.fetchFolder(withUUID: parentUUID, in: context) - assert(parent != nil) // For non-first sync we rely fully on the server response if !shouldDeduplicateEntities { - parent?.childrenArray.forEach { parent?.removeFromChildren($0) } + (parent?.children?.array as? [BookmarkEntity] ?? []).forEach { parent?.removeFromChildren($0) } } while !queue.isEmpty { @@ -195,6 +211,13 @@ final class BookmarksResponseHandler { } else if let existingEntity = entitiesByUUID[syncableUUID] { existingEntity.parent?.removeFromChildren(existingEntity) parent?.addToChildren(existingEntity) + } else { + let newStubEntity = BookmarkEntity.make(withUUID: syncableUUID, + isFolder: false, + in: context) + newStubEntity.isStub = true + parent?.addToChildren(newStubEntity) + entitiesByUUID[syncableUUID] = newStubEntity } } } diff --git a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift index 742521cef..487d69b52 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift @@ -89,10 +89,13 @@ extension Syncable { } if bookmark.isFolder { let children: [String] = { + let allChildren: [BookmarkEntity] if BookmarkEntity.Constants.favoriteFoldersIDs.contains(uuid) { - return bookmark.favoritesArray.compactMap(\.uuid) + allChildren = bookmark.favorites?.array as? [BookmarkEntity] ?? [] + } else { + allChildren = bookmark.children?.array as? [BookmarkEntity] ?? [] } - return bookmark.childrenArray.compactMap(\.uuid) + return allChildren.filter { $0.isPendingDeletion == false }.compactMap(\.uuid) }() let lastReceivedChildren = bookmark.lastChildrenArrayReceivedFromSync ?? [] diff --git a/Tests/BookmarksTests/BookmarkMigrationTests.swift b/Tests/BookmarksTests/BookmarkMigrationTests.swift index 15b88f1b7..8724a0d03 100644 --- a/Tests/BookmarksTests/BookmarkMigrationTests.swift +++ b/Tests/BookmarksTests/BookmarkMigrationTests.swift @@ -91,6 +91,10 @@ class BookmarkMigrationTests: XCTestCase { try commonMigrationTestForDatabase(name: "Bookmarks_V4") } + func testWhenMigratingFromV5ThenRootFoldersContentsArePreservedInOrder() throws { + try commonMigrationTestForDatabase(name: "Bookmarks_V5") + } + func commonMigrationTestForDatabase(name: String) throws { try copyDatabase(name: name, formDirectory: resourceURLDir, toDirectory: location) @@ -111,6 +115,9 @@ class BookmarkMigrationTests: XCTestCase { let mobileFavoritesArray = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: latestContext)?.favoritesArray.compactMap(\.uuid) XCTAssertEqual(legacyFavoritesInOrder, mobileFavoritesArray) + + let uuids = BookmarkUtils.fetchAllBookmarksUUIDs(in: latestContext) + XCTAssert(!uuids.isEmpty) }) // Test whole structure diff --git a/Tests/BookmarksTests/FaviconsFetcher/BookmarkDomainsTests.swift b/Tests/BookmarksTests/FaviconsFetcher/BookmarkDomainsTests.swift index 22b84160c..a57cee815 100644 --- a/Tests/BookmarksTests/FaviconsFetcher/BookmarkDomainsTests.swift +++ b/Tests/BookmarksTests/FaviconsFetcher/BookmarkDomainsTests.swift @@ -164,9 +164,10 @@ private extension BookmarkDomains { static func make(withAllBookmarksIn context: NSManagedObjectContext) -> BookmarkDomains { let request = BookmarkEntity.fetchRequest() request.predicate = NSPredicate( - format: "%K == NO AND %K == NO", + format: "%K == NO AND %K == NO AND (%K == NO OR %K == nil)", #keyPath(BookmarkEntity.isFolder), - #keyPath(BookmarkEntity.isPendingDeletion) + #keyPath(BookmarkEntity.isPendingDeletion), + #keyPath(BookmarkEntity.isStub), #keyPath(BookmarkEntity.isStub) ) request.propertiesToFetch = [#keyPath(BookmarkEntity.url)] request.relationshipKeyPathsForPrefetching = [#keyPath(BookmarkEntity.favoriteFolders), #keyPath(BookmarkEntity.parent)] diff --git a/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite new file mode 100644 index 000000000..1a3bbe609 Binary files /dev/null and b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite differ diff --git a/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-shm b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-shm new file mode 100644 index 000000000..5aa8e0e27 Binary files /dev/null and b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-shm differ diff --git a/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-wal b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-wal new file mode 100644 index 000000000..16f8ec71a Binary files /dev/null and b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-wal differ diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index 2694a5558..06aac8b49 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -815,6 +815,170 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { }) } + // MARK: - Stubs + + func testThatLastChildrenArrayTakesIntoAccountStubs() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree {} + + context.performAndWait { + BookmarkUtils.prepareFoldersStructure(in: context) + bookmarkTree.createEntities(in: context) + try! context.save() + } + + let received: [Syncable] = [ + .rootFolder(children: ["1", "2"]), // Creates a Stub with id 2 + .bookmark(id: "1") + ] + + let rootFolder = try await handleInitialSyncResponse(received: received, clientTimestamp: Date(), serverTimestamp: "1234", in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: ["1", "2"]) { + Bookmark(id: "1") + Bookmark(id: "2", isStub: true) + }) + + // Add new bookmark with id 3 + context.performAndWait { + let root = BookmarkUtils.fetchRootFolder(context)! + let newBookmark = BookmarkEntity.makeBookmark(title: "3", url: "3", parent: root, context: context) + newBookmark.uuid = "3" + try! context.save() + } + + let sent = try await provider.fetchChangedObjects(encryptedUsing: crypter) + + // Only Root and "3" should be sent + XCTAssertEqual(sent.count, 2) + + let sentRootData = sent.first(where: { $0.payload["id"] as? String == rootFolder.uuid }) + XCTAssertNotNil(sentRootData) + let folderChanges = sentRootData?.payload["folder"] as? [String: [String: [String]]] + XCTAssertNotNil(folderChanges) + + // We expect to send create for 3 + XCTAssertEqual(folderChanges?["children"]?["insert"], ["3"]) + + // Ensure there is no removal for 2 + XCTAssertNil(folderChanges?["children"]?["remove"]) + } + + func testThatPatchPreservesOrderWithStubs() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree {} + + context.performAndWait { + BookmarkUtils.prepareFoldersStructure(in: context) + bookmarkTree.createEntities(in: context) + try! context.save() + } + + let received: [Syncable] = [ + .rootFolder(children: ["2", "1", "3"]), // Create Stubs with id 2 and 3 + .favoritesFolder(favorites: ["3", "1", "2"]), + .bookmark(id: "1") + ] + + let rootFolder = try await handleInitialSyncResponse(received: received, clientTimestamp: Date(), serverTimestamp: "1234", in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: ["2", "1", "3"]) { + Bookmark(id: "2", favoritedOn: [.unified], isStub: true) + Bookmark(id: "1", favoritedOn: [.unified]) + Bookmark(id: "3", favoritedOn: [.unified], isStub: true) + }) + + context.performAndWait { + let bookmarks = BookmarkEntity.fetchBookmarks(with: ["1", "2", "3"], in: context) + bookmarks.forEach { $0.modifiedAt = nil } + + let favoriteFolder = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.unified.rawValue, in: context) + favoriteFolder?.modifiedAt = Date() + rootFolder.modifiedAt = Date() + try! context.save() + } + + let patchData = try await provider.fetchChangedObjects(encryptedUsing: crypter) + + let changedObjects = patchData.map(SyncableBookmarkAdapter.init(syncable:)) + + XCTAssertEqual(changedObjects.count, 2) + let changedRoot = changedObjects.first(where: { $0.uuid == BookmarkEntity.Constants.rootFolderID }) + let changedFavRoot = changedObjects.first(where: { BookmarkEntity.Constants.favoriteFoldersIDs.contains($0.uuid!) }) + XCTAssertEqual(changedRoot?.children, ["2", "1", "3"]) + XCTAssertEqual(changedFavRoot?.children, ["3", "1", "2"]) + } + + func testThatRemoteRemovalOfStubReferenceRemovesTheStub() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree {} + + context.performAndWait { + BookmarkUtils.prepareFoldersStructure(in: context) + bookmarkTree.createEntities(in: context) + try! context.save() + } + + let received: [Syncable] = [ + .rootFolder(children: ["1", "2", "3"]), // Create Stubs with id 2 and 3 + .bookmark(id: "1") + ] + + var rootFolder = try await handleInitialSyncResponse(received: received, clientTimestamp: Date(), serverTimestamp: "1234", in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: ["1", "2", "3"]) { + Bookmark(id: "1") + Bookmark(id: "2", isStub: true) + Bookmark(id: "3", isStub: true) + }) + + // Simulate two kinds of "removal": + // - "2" is only removed from children list. + // - "3" is removed from children list and deleted. + let receivedUpdate: [Syncable] = [ + .rootFolder(children: ["1"]), + .bookmark(id: "3", isDeleted: true) + ] + + rootFolder = try await handleSyncResponse(sent: [], received: receivedUpdate, clientTimestamp: Date(), serverTimestamp: "1234", in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: ["1"]) { + Bookmark(id: "1") + }) + } + + func testThatRemoteRemovalOfFolderRemovesTheStub() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree {} + + context.performAndWait { + BookmarkUtils.prepareFoldersStructure(in: context) + bookmarkTree.createEntities(in: context) + try! context.save() + } + + let received: [Syncable] = [ + .rootFolder(children: ["1"]), + .folder(id: "1", children: ["2"]) + ] + + var rootFolder = try await handleInitialSyncResponse(received: received, clientTimestamp: Date(), serverTimestamp: "1234", in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: ["1"]) { + Folder(id: "1", lastChildrenArrayReceivedFromSync: ["2"]) { + Bookmark(id: "2", isStub: true) + } + }) + + let receivedUpdate: [Syncable] = [ + .rootFolder(children: []), + .folder(id: "1", isDeleted: true) + ] + + rootFolder = try await handleSyncResponse(sent: [], received: receivedUpdate, clientTimestamp: Date(), serverTimestamp: "1234", in: context) + assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: []) { + }) + } + // MARK: - Helpers func handleInitialSyncResponse( diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift index 46c3bba2a..99c2a1223 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift @@ -480,6 +480,44 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase }) } + // MARK: - Responses with parts of data structure missing + + func testWhenChildIsMissingAndReceivedLaterThenRelationIsPersisted() async throws { + let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + + let bookmarkTree = BookmarkTree {} + + let received: [Syncable] = [ + .rootFolder(children: ["1", "2"]), + .favoritesFolder(favorites: ["1", "2", "3"]), + .mobileFavoritesFolder(favorites: ["1"]), + .desktopFavoritesFolder(favorites: ["2", "3"]) + ] + + _ = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context) + + let received2: [Syncable] = [ + .bookmark(id: "1"), + .bookmark(id: "2"), + .bookmark(id: "3"), + ] + + let root = try await handleSyncResponse(received: received2, in: context) + + context.performAndWait { + XCTAssertEqual(root.childrenArray.count, 2) + + let unified = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.unified.rawValue, in: context) + XCTAssertEqual(Set((unified?.favoritesArray.map { $0.uuid }) ?? []), Set(["1", "2", "3"])) + + let mobile = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: context) + XCTAssertEqual(Set((mobile?.favoritesArray.map { $0.uuid }) ?? []), Set(["1"])) + + let desktop = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.desktop.rawValue, in: context) + XCTAssertEqual(Set((desktop?.favoritesArray.map { $0.uuid }) ?? []), Set(["2", "3"])) + } + } + // MARK: - Handling Decryption Failures func testThatDecryptionFailureDoesntAffectBookmarksOrCrash() async throws { let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType)