From 8f36c8f34dae7acf593b9dae1f58643baa921c02 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Tue, 5 Mar 2024 12:32:05 +0100
Subject: [PATCH 1/9] Add tests for stub objects

---
 ...marksRegularSyncResponseHandlerTests.swift | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift
index 46c3bba2a..543f5f787 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift
@@ -480,6 +480,45 @@ 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"]),
+            .mobileFavoritesFolder(favorites: ["1"]),
+            .desktopFavoritesFolder(favorites: ["2"])
+        ]
+
+        _ = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context)
+
+        let received2: [Syncable] = [
+            .bookmark(id: "1"),
+            .bookmark(id: "2")
+        ]
+
+        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((unified?.favoritesArray ?? []).count, 2)
+
+            let mobile = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: context)
+            XCTAssertEqual((mobile?.favoritesArray ?? []).count, 1)
+            XCTAssertEqual(mobile?.favoritesArray.first?.uuid, "1")
+
+            let desktop = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.desktop.rawValue, in: context)
+            XCTAssertEqual((desktop?.favoritesArray ?? []).count, 1)
+            XCTAssertEqual(desktop?.favoritesArray.first?.uuid, "2")
+        }
+    }
+
     // MARK: - Handling Decryption Failures
     func testThatDecryptionFailureDoesntAffectBookmarksOrCrash() async throws {
         let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType)

From bba51060149149501fabb5c9919e12b155da12e6 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Tue, 5 Mar 2024 19:29:26 +0100
Subject: [PATCH 2/9] Add Stub implementation for Bookmarks

---
 Sources/Bookmarks/BookmarkEntity.swift        | 15 ++++++++--
 Sources/Bookmarks/BookmarkListViewModel.swift |  5 ++--
 Sources/Bookmarks/BookmarkUtils.swift         | 17 +++++++----
 .../.xccurrentversion                         |  2 +-
 .../BookmarksModel 6.xcdatamodel/contents     | 28 +++++++++++++++++++
 .../FaviconsFetchOperation.swift              |  5 ++--
 .../BookmarkCoreDataImporter.swift            |  5 ++--
 .../Bookmarks/MenuBookmarksViewModel.swift    |  5 ++--
 .../internal/BookmarkEntity+Syncable.swift    |  1 +
 .../internal/BookmarksResponseHandler.swift   | 14 ++++++++++
 .../BookmarkDomainsTests.swift                |  5 ++--
 ...marksRegularSyncResponseHandlerTests.swift | 15 +++++-----
 12 files changed, 90 insertions(+), 27 deletions(-)
 create mode 100644 Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 6.xcdatamodel/contents

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<BookmarkEntity> {
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 089ba7105..0d3f6aea0 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)]
@@ -117,9 +118,10 @@ public struct BookmarkUtils {
 
     public static func fetchAllBookmarksUUIDs(in context: NSManagedObjectContext) -> [String] {
         let request = NSFetchRequest<NSFetchRequestResult>(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 +133,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,7 +147,9 @@ 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)) ?? []
     }
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 @@
 <plist version="1.0">
 <dict>
 	<key>_XCCurrentVersionName</key>
-	<string>BookmarksModel 5.xcdatamodel</string>
+	<string>BookmarksModel 6.xcdatamodel</string>
 </dict>
 </plist>
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 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="22522" systemVersion="22G313" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier="">
+    <entity name="BookmarkEntity" representedClassName="BookmarkEntity" syncable="YES">
+        <attribute name="isFolder" attributeType="Boolean" usesScalarValueType="YES"/>
+        <attribute name="isPendingDeletion" optional="YES" attributeType="Boolean" defaultValueString="NO" usesScalarValueType="YES" versionHashModifier="3"/>
+        <attribute name="isStub" optional="YES" attributeType="Boolean" usesScalarValueType="YES"/>
+        <attribute name="lastChildrenPayloadReceivedFromSync" optional="YES" attributeType="String"/>
+        <attribute name="modifiedAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
+        <attribute name="title" optional="YES" attributeType="String"/>
+        <attribute name="url" optional="YES" attributeType="String" versionHashModifier="3"/>
+        <attribute name="uuid" attributeType="String" versionHashModifier="3"/>
+        <relationship name="children" optional="YES" toMany="YES" deletionRule="Cascade" ordered="YES" destinationEntity="BookmarkEntity" inverseName="parent" inverseEntity="BookmarkEntity"/>
+        <relationship name="favoriteFolders" optional="YES" toMany="YES" deletionRule="Nullify" destinationEntity="BookmarkEntity" inverseName="favorites" inverseEntity="BookmarkEntity" elementID="favoriteFolder"/>
+        <relationship name="favorites" optional="YES" toMany="YES" deletionRule="Nullify" ordered="YES" destinationEntity="BookmarkEntity" inverseName="favoriteFolders" inverseEntity="BookmarkEntity"/>
+        <relationship name="parent" optional="YES" maxCount="1" deletionRule="Nullify" destinationEntity="BookmarkEntity" inverseName="children" inverseEntity="BookmarkEntity"/>
+        <fetchIndex name="byUUID">
+            <fetchIndexElement property="uuid" type="Binary" order="ascending"/>
+            <fetchIndexElement property="isPendingDeletion" type="Binary" order="ascending"/>
+        </fetchIndex>
+        <fetchIndex name="byURL">
+            <fetchIndexElement property="url" type="Binary" order="ascending"/>
+            <fetchIndexElement property="isPendingDeletion" type="Binary" order="ascending"/>
+        </fetchIndex>
+        <fetchIndex name="byIsPendingDeletion">
+            <fetchIndexElement property="isPendingDeletion" type="Binary" order="ascending"/>
+        </fetchIndex>
+    </entity>
+</model>
\ 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 b4f04154d..cf3236b47 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<NSFetchRequestResult>(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/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..cf8a50e96 100644
--- a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift
+++ b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift
@@ -151,6 +151,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
                 }
             }
 
@@ -195,6 +202,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/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/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift
index 543f5f787..99c2a1223 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksRegularSyncResponseHandlerTests.swift
@@ -489,16 +489,17 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase
 
         let received: [Syncable] = [
             .rootFolder(children: ["1", "2"]),
-            .favoritesFolder(favorites: ["1, 2"]),
+            .favoritesFolder(favorites: ["1", "2", "3"]),
             .mobileFavoritesFolder(favorites: ["1"]),
-            .desktopFavoritesFolder(favorites: ["2"])
+            .desktopFavoritesFolder(favorites: ["2", "3"])
         ]
 
         _ = try await createEntitiesAndHandleSyncResponse(with: bookmarkTree, received: received, in: context)
 
         let received2: [Syncable] = [
             .bookmark(id: "1"),
-            .bookmark(id: "2")
+            .bookmark(id: "2"),
+            .bookmark(id: "3"),
         ]
 
         let root = try await handleSyncResponse(received: received2, in: context)
@@ -507,15 +508,13 @@ final class BookmarksRegularSyncResponseHandlerTests: BookmarksProviderTestsBase
             XCTAssertEqual(root.childrenArray.count, 2)
 
             let unified = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.unified.rawValue, in: context)
-            XCTAssertEqual((unified?.favoritesArray ?? []).count, 2)
+            XCTAssertEqual(Set((unified?.favoritesArray.map { $0.uuid }) ?? []), Set(["1", "2", "3"]))
 
             let mobile = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: context)
-            XCTAssertEqual((mobile?.favoritesArray ?? []).count, 1)
-            XCTAssertEqual(mobile?.favoritesArray.first?.uuid, "1")
+            XCTAssertEqual(Set((mobile?.favoritesArray.map { $0.uuid }) ?? []), Set(["1"]))
 
             let desktop = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.desktop.rawValue, in: context)
-            XCTAssertEqual((desktop?.favoritesArray ?? []).count, 1)
-            XCTAssertEqual(desktop?.favoritesArray.first?.uuid, "2")
+            XCTAssertEqual(Set((desktop?.favoritesArray.map { $0.uuid }) ?? []), Set(["2", "3"]))
         }
     }
 

From b011e3d9e4769c51f52ef5d35293bbded41d9a36 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Tue, 12 Mar 2024 17:42:33 +0100
Subject: [PATCH 3/9] Add tests for migration

---
 .../BookmarksTestDBBuilder.swift                |   2 +-
 .../BookmarksTests/BookmarkMigrationTests.swift |   7 +++++++
 .../Resources/Bookmarks_V5.sqlite               | Bin 0 -> 57344 bytes
 .../Resources/Bookmarks_V5.sqlite-shm           | Bin 0 -> 32768 bytes
 .../Resources/Bookmarks_V5.sqlite-wal           | Bin 0 -> 41232 bytes
 5 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite
 create mode 100644 Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-shm
 create mode 100644 Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-wal

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/Tests/BookmarksTests/BookmarkMigrationTests.swift b/Tests/BookmarksTests/BookmarkMigrationTests.swift
index 15b88f1b7..550cf740c 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 uudis = BookmarkUtils.fetchAllBookmarksUUIDs(in: latestContext)
+            XCTAssert(!uudis.isEmpty)
         })
 
         // Test whole structure
diff --git a/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite
new file mode 100644
index 0000000000000000000000000000000000000000..1a3bbe6095efbfc416950c06ccf62acbae4f7094
GIT binary patch
literal 57344
zcmeI%c|4Tc{{Zlrh_R2tDA6clvJX*88D^L<V`iAK%QP5_WyUgP30Wg#Nol*$YD-92
zDs7fZd!!;I(k`Su;yY8imu~mlef@rad413G8s<6AbC%C}pYxo@d7;^n*gO`}hZ`Ei
z<RJ|qaL9}q5OX9F0)Ytef28?8KMo20iSR^1a2+~v{^NThIps?TqRUX0l0FQPgl&XK
zACsz*4wcfA+$Ql-V!3#h*ds9-><<41IDh~U00KY&2mpcqK>@O)2tr*Q%H%OU16i&N
zTM8COvcy>8ag)!bE$wj_Iu1$4SdehY$#;;Nt_({GnT{jVkrpJ11!}#7C_+^g8jtx=
zA&!p0V(1utp@}o8pDUZVE~v^8XHO$g$VdXeMr)isl4?(|#n`(bi8vQ+e!C711T4}K
zV{eJa*lVJfEI|n_Qb`2bMC0+|;-Z4y#5+y2K2e@uoS*M|UHo&&pCb6{jvxj>SN@@W
zGR7A7qkTcsGzTg_$cfB?3$_^NA4ycawwNfwK?@qsVspG%5v~jiE;k^E85)4&@YuXa
zhG!%pjLPD8vpK$4Rv?SV=5kyuC=?>U!$f`z1iFjFM4SXN7U#^b{r43h`F1~5!tWA+
zM#Yh_1hO@MY;kmfuO)><5)7yXfh^#~&@55$D3~Z>89&_h{}ir+J?W>q#eWwdzsRpc
z65RNE5PZUa7pu0YC}KH3)(!s{D}Sc^6s*{9gB8rKUjlxccmI!YHAO@bcKmRezYdrE
zn|T2H?~(tl_^&(h?_?1(MG-{)6GWT{)xZklNU<l-aWsZ2V^ZSVoSb|`^rs*v<(-_3
z@qIE2m?u>yc<%lxAb67gOj%J#SpKSjiiZe`BCPp{O5Y=!`lv8msTh0yQ}ez0pGIC}
zYH0th<ge+z*NuudoAGUVNidsOUj7V{HYs45@I-SzR?EL~AvJ%|vmXn_lwQ*&U$vqT
z|MHl0XVU681#p6FYK15y%0x&+Ft(6yfMFql{Ou=$xt7QMmYzDi29rB9lmA|X3Eqtf
zHc(N7mKHRL_#I%X<WK5CQyHe_{U$*r{^;e;3;m=uSkMa!_0J`lpCr(}ckoByZ<l^v
z;r?2bzYpG|%nX6&->&lCd;}{3j{mMg(h0UW8XaRx<=0HY(CC(U0tw5XT~v$<iGtyu
zS>gzeIINXD#g^tmww&}v;IG>J7v#hnieFl=%uPy7dVL{L-!||6$6x=0f2e>i0|6ia
z1b_e#00KY&2mk>f00e*l5SW$%Qi7=A(=v1rG!Os+KmZ5;0U!VbfB+Bx0zd!=00AKI
zF9pE<|6f9ZY(M}A00AHX1b_e#00KY&2mk>f00gG7fVd!b@c;j5Jc1x}AOHk_01yBI
zKmZ5;0U!VbfB+Bx0>2jk`~Tk?fgC^p2mk>f00e*l5C8%|00;m9AOHlWy8zh#Pxlc7
z@dE)M00e*l5C8%|00;m9AOHk_01)`S0NDTk-U#FX0zd!=00AHX1b_e#00KY&2mk>f
zFx>^f{(riUAc!9b00AHX1b_e#00KY&2mk>f00e-*?*+jA|Mx~92M_=PKmZ5;0U!Vb
zfB+Bx0zd!=0D<W)0QUdWeFQ=LKmZ5;0U!VbfB+Bx0zd!=00AHX1b#0d#m@qjc?IEr
zzySn+01yBIKmZ5;0U!VbfB+Bx0zd!=On-sd(3ubv3}(UQ1_Uuf18^K3n-}?Q|1T{M
z;eWsZ1b_e#00KY&2mk>f00e*l5C8)IYXXUW{H>xxins}S84jawjMcGPVu02$(l@cx
z!7MSh)G<UG8yXl}SXmh9<2-`{*<n0={kzZ^Lc%jeL}B6*l2UM423&<qqw%<*EL*NO
zE6|Y@8ph^w2;MCIuG5FjPq|Iprbh;|81PvPG7Ynq$8}_Stz8>LVy$BZGT`%nQ3IYC
z=En-dvVB=$JO+HuKjvV-mB}=!pw%!Qi{r(z3S|bd!nvUV6Ol0B$bT$5k!7D4)sbdL
zV)IybXa?MQnUjl;2@}oY*btc{qi`Yy<LeS05{bv)><rMZOdB6gumvfE;~8um8O01D
z8IVX3>$Jj&kz8g#upJZY>R{kNcg7l^BWR9+VH|y5Y=Dm**UrrBmb8q=)IB)`j!YVg
z7LHj4-|Q3g`DWTj&0}-I5>FTI7(2T>duPn?uP!KA;m2?x2n>RNEQMepE|B$*?T}hX
zC!`DV0MZ9}3dKONP#Sa{H1hk@odKPg!n3AMW$5~E`+wo95dH@oKmZ5;foUs{X8;u{
z^V{qs#-q2-85^uO;cjTWVgXCXBbl=;;$YZ2NpP`jc$MsoK7%Vnv^U!DDk=M@u2X7U
z-PXC7BMI;(sjQ&)_h{>HFHTRJm%0CD8veDX*=<+(_sEMiHRCbdxVTMA!k+eB`#Sh_
zY<zrdaO~qshLK!`hPZg95EK!7E>TGsj$vO~9njo+9v|USZt|2X)mzcog;nd!R(f-!
zW^_}8Y4+w*qyr4rH482F%rshVX1s7{koGJBby&hnws<dQL*3lrHQMe%vzg+g<QG&S
zsmd958l)<vw*)U773L)D?op_d*X$CJIp<X=lb^z9JUnDPFR{C^;L(jCnNM=p&zmG^
zaC%icykT=}hVd(RVheLSyme6;5|=#fiFvZQWeXHsY!@6%EK@hx;?@#XwW$jiYgLm|
zkh`Z(wsrM(r2$xSW}>vpb5W6l7*lU^A1~Im`rrjSV6Gu;ic}YxkNW75_B*?zwepLr
zI}fOG6I{?*0WX!;T^3Tf&Xw-1_~O3dl2R58G1Ig_cLmfXbvdMpiIhw@uP<Dlu<w1q
zY1JmzIkk!<@(=xt6gh0{BW#s%t55RwLVOYWbbcrueJMnEj>s|?dY+ZJu0>K=FmZo%
z%{7|~C4TVBXN4Aqv}!6Q-g;J4dQ8cRW+K1KW<gHPI&Rsu>x(~2XC%JKmYF?wk8|V0
zb%)PK9`7Y;1*CZyaI2Iw<c#Qdv}<zyP|sac#g@EapR&*->FC2Z^yf=9DWY4q{81+=
z-s7tIsfSQ#P|Q8d&M>y!EoP{argl~MaijJ-%3`DR!kMFmcURp)$Iyz%#<m(gY<(XS
zIlE->`R62eh#gyFoOwOoQ#1YqRs8K<$?}xX$VNZs)F(&cY@$mWif(Bw4k-1^DVFz2
zwz~7O*RgCx*Sw=zZaq@#-OA!}V{vM0lKXp3WE`?lE7P78cF$<@7-Fq{w-UGcM9nPH
z!sUnl7!E$?>g3=2_2?R-_+wO4cB&jZ%gVF?df&g<>7dbe??+l%0sf}Zjy8!O^d#GC
zJ~!%@>S)hxT}OzZ7d_TI@4tQD(C`cQX!|!UUv+PHu4u=YX*c&iHEizh#4Ra0cxCS`
zBeed5%%r=BDx1Vug?%P@U5SNHsBz5Ib5F%p&q}D|%{Ph{JA6)J<&Y&r+i#<S-yxOG
zthq*;3)>`j5k}gyP2+q96Wec!sQRVoW0B$kNRbfy&3GP68Cy}<$ktURq_p1RX`kNc
z|K3jTio%G^vG5=@1rqYXK1gU++-r3?b2qP3mSKCB+NG2wm33xjuGy=q7`{mNRApv}
za)y&j52VV$KUq63jh%50z36FSWodm2wMnVCW>n!rxw(sWt4FTnNPwy5zP3sgV{?tJ
zOc!pFZRMPSIGy6+<vqG}N@+ueay6bFhxbYp>>4_zY~{4u!MMnI7^~mUQrXjL>|Si(
zwXNCHAv{Uj&HM?o=GhT{E$<sywIT=VC|5nCNUEt+6-MBd7A1f0(A6f0_N{&=i`j1Y
ztFt8UE8ITNK_ms{y^$Z7pDcaBUJ<Qu(`#?=SSqUO_WXYJ&+qG!orhOf&TeCDW`{F-
z5A$39pj&I1&Xg~Ml)`gZjmeLMO02C|itJIR%nvFl8YC~|?6-%yq5^ZxhSXLx?;>>5
z0$=L(-S-;3xGhM^Kd(8cA*?JACB3nA3~lpi@k`E0_vlrbaFMQ!*^>7fWM*wy06m&=
zGZxL+G*$#(W7eM{1iK|BD%%II-)LUp#p9l1y1OSyT~6<Q{is?@ldR^6VOTem`dVLX
z*7Yu^M+8;36t_3|clE!9srT<J%t?N*$*lcCeNRM~+8s`_{bfywNVnd5iEL@*ybEhD
zE2=-b(9AVUShIZH<#*Ec0SDZ9VJ@398^s(Ctn81Gf1U!1kRcbshB|~l>7S8hmm~Ik
zYVAOn%u-RTJD}GyuPMK~L+DwBG1)Xa10Sn&krI}%geP2n!}6fyXu(nM;SpFcTvZy0
z*U){q#Wi2Lpb-&#M|xpgx{#u<qHkLrlebmbdeOBe&Be<*V;uX9bYC9#S}OcZHh9&A
zst(5sQSPou=G_ZOh=%;;h?A9e#<Q<KTtt2=gX~olM}JYoz4Z><>0;&nC1hUpy%W{?
zne!4vDhEZ)7JP|ak+GfE6nP`FP~KI{S8wLVh8dR%J4!q#B3m%t`uT{*$_CmRPHT^a
zonIN<(ysQ=4xQ1L-Qz91$6SIfy9Y-kXXkj!&Dji9+~IF}LL}`{M4m43j-8H2X=#1L
zK$PW&HS#rSd*{phzUolfg{bO$14~^v$6P|cz9sXDSj5BJ8Kybn8t_<&oh0#=YHxA0
z>1N5qa^lD8e9b^FcjgOKE6F2E2f{O9reY^VO))iX>V}s{ZHk=|o>@;Gcq`n8j{D{%
znxx2mRJJxjG0kIkuD@wxlXC}crFB~2#m<K3B_Gvay}V3R&QoaE!Kq56-z?#EZX<op
zDVg`Yfn?~nZ!|_h*-hct`=NWODo*!QDlD_wc3w1n=Y0B7$a9u-OcTZT=1wQmK-|q9
zV?Se^m(4vxTO0RZnP+v2d3D`VCFSc(={NNE>Q^dOT-yWTtU2jyoxZ-UIdyBIOG&AH
zs9RgJR#wdY2bP+r22H6-c#g>nwc0j&)2zIuUb{miNAQNBd25;d9OFA_ULDsJ6VGht
zb@-kQtMhQ|8gUL;Z@5gWHh1K;anu{1=ob%{gy==at$6N!>B}EpwVfW4<T9BY<x?$<
z$N$hR4()oq;MA+ibk|Lss&w`17d~WLkJOE<%RPN{ZG~}kyH|z!<Du#S)B0$=^XX4R
z>lfsw4B}S>sw8`>mRvwoMYWpGeZvaWu^*v@u8PcVPSRtZYB|<7a~K_&+^-?pVmGX^
z3`fqDY4Pn-+J5EGNwj-M`MkT%ccbrmI-Lqge-(N6O1aZ&QTG)0XG=5R21j;Dmgl7h
zE7cw8A`h?!AmOR&_bk7v+>;r~4Y^nseqwgl{JPvO)PUs2nc;aOTSj(`$P}EH>9u~7
zJZf<!fpJx@C*YH5Eb43q<9bD(?ZYb-kGXx^UTzQSZ2n-M$?)#aI-{#c%|;!^V#f$$
zE@SFrka3X>8LOH%3=9maJTK?zh1Av^7})*be%FnY8=l3s#k79S896&ccIkPY)CBF~
zghdg;mUsj@S)94E(%l)gqhaxR4dEo8eGV}3Ad6FQ`7I5`J<J}HhWq>7<Zv!@-0FUf
zxt=F?pnNt(dgj*IyxRo5HOsAO@4ZDEn<^FM!&CKAt(!hJS2PW(WxH>2o25puSF;_)
zZ!|KWyGWi8c97LNmtAz6_%iQ<O%{QIrw}N_8~C$ZJ+fEkUB?&T3v3Dq3_QbzL40=b
zZENi?Y#BvGJuBy8%jeMe6$IU_MVF2}Ng_RYA6isz_fV}{wHwu~)4h<pZE)LQZf(oR
zlf>bG;a9`V&#BS$SmMgUIynznIf9*>?HkRnTaDB$=M?s7Hg0og)uRbJ%eO9OU5;kO
zSK&+Oqc+$RmXY|5eJl3&JATkyzs+|m19by+LyNn3KB~JUt+Yg?o!Ztexi;05repWj
zAOE2EVab5QhQnuy&y=0nKj0)&YJL0hOTzoDQLbO9UtP_L@}15)j31igdNQws#TGgq
z;(Eq$uWhS%HL6oGD8I(s?Vy9d!>&WsewZV+66{*G(((J%ww*pfDIZd6-0mNE-L7)i
zqt&BLI>7q)flkKptm9eLhMnEjdTaL+qju#V@_m)~PBkijrQ`gfXRl^P<*XzY9N7N+
zP*n23UAOmc?`*&5#uUU-$IZug$6AeJ$LZrmvE=dEln6OWrmxz)4Z7Rk7OW82{x#BP
GOy)nIcT}?g

literal 0
HcmV?d00001

diff --git a/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-shm b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-shm
new file mode 100644
index 0000000000000000000000000000000000000000..5aa8e0e2778fecf05bc67045df621fb2c8c679cf
GIT binary patch
literal 32768
zcmeI*zfA&h7zW@I1o;<Zd5JTy00|RNQ%KDM?!XdKxPb%ESh|4;H5H{B==TvrY%Sq?
z-sH_)E_c7;8Q{J4Fl{oO=uOi+jN*D9oo|op*X7IS`{RDG{@BenZ>#6g*VC;1=lgNg
zDS7VqpZq>PUnX(ajowCk(SGzUI*1OV`TaO51PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!Cs{3*_RF5U44TTU$b)ra-RD34xjdxvwY$Y6|4Cs1T?rkej+fpr$~sNeh9R0=c6u
x1ZoQ89|VLzO@a8ghY%n@fB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5Fl{n0zXI_CvgA(

literal 0
HcmV?d00001

diff --git a/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-wal b/Tests/BookmarksTests/Resources/Bookmarks_V5.sqlite-wal
new file mode 100644
index 0000000000000000000000000000000000000000..16f8ec71ae13eba9e740673b52606e15874b905f
GIT binary patch
literal 41232
zcmeI*ONbmr7{KwWepa`e`F6}6HjXR`xeOvY(=&*I2OmMAA`50k5X^(D5Q7eEHbPDT
z59-yUqN@nv<LCnnOjLrpAZT1s_d!e`$sxuBA-*t)Vpa8a!d!#|f)n`H?sQjob@x>N
z`ZOP3Ro}KZc~;$gbX=)X<;us6r3bcuvG0x>KABipyQRNkca7Ljt=hl7|Kj7*ljj~?
zV|Qvfsq8-m0R#|0009ILKmY**5I_I{1g?64F&(R>(RcTHJD=J*vvb4KvpZ(@<o32g
zcWYH#V0!BHch{Y{WtWQ!<h8z%d3nCpm*W!EmgPaYQQnd#-SHFz5I_I{1Q0*~0R#|0
z009L4Z-GX*MrD~XI!Kj>m~nO2=if{|{QlyhZnxV^Tw`WjC!4DI#MWneGdpIdcWs{O
z^=1dR8u8$DOm&?XTCG;o3|wd1>d)%(I)hu+r{$rpKXp})E?lQmw2D?EAN-b))o=OX
z_IA5H5nODn*XvQ$xRexa_cxIZwvSdP&lkQl^u=~ZF}B*ibZe*6xh`^zS?Q$2R3|UK
zR}@w!azs^J;KRw?uikUMt6W?lmV-+EkkfKZmTi@S00IagfB*srAb<b@2q1s}0>dp}
z;z;Wt$;Xw6d@UGLCiJyzRHZ?rvmhB!M*Cf%kDA2pma4eGfp?~w%j-^Ua)|`7tSC7n
zN98X$X{!_j5I_I{1Q0*~0R#|0009ILxM~Da`_o@1d0ei8s9a}ZVq*h33G7N|i5o+Z
zxLc~?0yq6VGj-tYji+5)AeVV1U&)gEBtOb8^1B^Va8|yRWA1ng0tg_000IagfB*sr
zAb<b@2n>xt9BOUz0z#z|UyWVWxN4O89l5Uuewjs~&U~YBV+c}r7gb!~pAC;Z_S&8!
zACWIGG|!ffAb<b@2q1s}0tg_000Iaga2WzVKOj%ad;yyt;J0mtKoVs(kD$yK2wkp#
zA52ii1sZQGY~1~USt;`c?EC_M+W7?z$~im3z;||@fx~uO!HLW045uJ~00IagfB*sr
zAb<b@2q1vKl^2LYqix!N(TQ8wtO28qTcmE0yG7s@Sr{6dHDIS1uwBYrx2m|n=4&_g
z@89;ySQ!_XQ?gGE+4%)rOyFnvRp#VVdGpHO2H$`H0tg_000IagfB*srAb<b@cIba3
zMt8%tk$u77;5!r&-_(kI)8K0nOd8dVE?TKSq@9<W?Vwy2S!^$wb$nyt;sP!Na3L;m
z@A|Lz-uKXRBV}A*zp^t5d?t&sAYa-!29C=q`AzooWDr0A0R#|0009ILKmY**5I|rE
z1XjZaab#aGPz4J7hJ7u8-Syvr0e^^#3tVGAwJKj=`-zwD{Qjk>ImQtT!SkgH2q1s}
a0tg_000IagfB*srT$+Fz8*oYE2>t~V><mHx

literal 0
HcmV?d00001


From 968c429c464688c641d100eab584aa95b039e371 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Tue, 12 Mar 2024 19:37:07 +0100
Subject: [PATCH 4/9] Update file refs

---
 Package.swift | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Package.swift b/Package.swift
index 8e145be80..ae8945705 100644
--- a/Package.swift
+++ b/Package.swift
@@ -365,6 +365,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]
         ),

From 8cf8bdaa01cb151e74c28b45a37e806fc52e778c Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Thu, 14 Mar 2024 10:44:05 +0100
Subject: [PATCH 5/9] Update tests, make sure stubs are processed correctly for
 patch request

---
 Sources/Bookmarks/BookmarkUtils.swift         |  4 +-
 .../BookmarksTestsUtils/BookmarkTree.swift    | 79 ++++++++++++-------
 .../internal/SyncableBookmarkAdapter.swift    |  6 +-
 .../Bookmarks/BookmarksProviderTests.swift    | 49 ++++++++++++
 4 files changed, 107 insertions(+), 31 deletions(-)

diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift
index feae7ec77..01dd8af03 100644
--- a/Sources/Bookmarks/BookmarkUtils.swift
+++ b/Sources/Bookmarks/BookmarkUtils.swift
@@ -156,7 +156,9 @@ public struct BookmarkUtils {
 
     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/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/SyncableBookmarkAdapter.swift b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
index 742521cef..0af0d7672 100644
--- a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
+++ b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
@@ -92,7 +92,11 @@ extension Syncable {
                     if BookmarkEntity.Constants.favoriteFoldersIDs.contains(uuid) {
                         return bookmark.favoritesArray.compactMap(\.uuid)
                     }
-                    return bookmark.childrenArray.compactMap(\.uuid)
+                    let validChildrenIds = bookmark.childrenArray.compactMap(\.uuid)
+
+                    // Take stubs into account - we don't want to remove them.
+                    let stubIds = (bookmark.children?.array as? [BookmarkEntity] ?? []).filter({ $0.isStub }).compactMap(\.uuid)
+                    return validChildrenIds + stubIds
                 }()
 
                 let lastReceivedChildren = bookmark.lastChildrenArrayReceivedFromSync ?? []
diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
index 2694a5558..eaad008cb 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
@@ -815,6 +815,55 @@ 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"])
+    }
+
     // MARK: - Helpers
 
     func handleInitialSyncResponse(

From 792a7395581e0385d6b6d051fee37f702e4beee9 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Thu, 14 Mar 2024 14:31:49 +0100
Subject: [PATCH 6/9] More tests and tweaks

---
 Sources/Bookmarks/BookmarkUtils.swift         | 11 +++
 .../internal/BookmarksResponseHandler.swift   | 15 +++-
 .../Bookmarks/BookmarksProviderTests.swift    | 81 ++++++++++++++++++-
 3 files changed, 100 insertions(+), 7 deletions(-)

diff --git a/Sources/Bookmarks/BookmarkUtils.swift b/Sources/Bookmarks/BookmarkUtils.swift
index 01dd8af03..2a328312b 100644
--- a/Sources/Bookmarks/BookmarkUtils.swift
+++ b/Sources/Bookmarks/BookmarkUtils.swift
@@ -72,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 {
diff --git a/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift b/Sources/SyncDataProviders/Bookmarks/internal/BookmarksResponseHandler.swift
index cf8a50e96..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.
@@ -165,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
@@ -178,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 {
diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
index eaad008cb..3a18f5d8a 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
@@ -834,10 +834,10 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
         ]
 
         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)
-//        })
+        assertEquivalent(withTimestamps: false, rootFolder, BookmarkTree(lastChildrenArrayReceivedFromSync: ["1", "2"]) {
+            Bookmark(id: "1")
+            Bookmark(id: "2", isStub: true)
+        })
 
         // Add new bookmark with id 3
         context.performAndWait {
@@ -864,6 +864,79 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
         XCTAssertNil(folderChanges?["children"]?["remove"])
     }
 
+    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"]), // Creates a Stub with id 2
+            .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":
+        // - Removal only from children list.
+        // - Removal of entity and from children list.
+        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)
+            }
+        })
+
+        // Simulate two kinds of "removal":
+        // - Removal only from children list.
+        // - Removal of entity and from children list.
+        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(

From 66afa99269fad4ffb1bc08444996c398066f55f3 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Fri, 15 Mar 2024 12:49:00 +0100
Subject: [PATCH 7/9] Address PR feedback

---
 .../internal/SyncableBookmarkAdapter.swift    | 11 ++--
 .../BookmarkMigrationTests.swift              |  4 +-
 .../Bookmarks/BookmarksProviderTests.swift    | 53 +++++++++++++++++--
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
index 0af0d7672..487d69b52 100644
--- a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
+++ b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
@@ -89,14 +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] ?? []
                     }
-                    let validChildrenIds = bookmark.childrenArray.compactMap(\.uuid)
-
-                    // Take stubs into account - we don't want to remove them.
-                    let stubIds = (bookmark.children?.array as? [BookmarkEntity] ?? []).filter({ $0.isStub }).compactMap(\.uuid)
-                    return validChildrenIds + stubIds
+                    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 550cf740c..8724a0d03 100644
--- a/Tests/BookmarksTests/BookmarkMigrationTests.swift
+++ b/Tests/BookmarksTests/BookmarkMigrationTests.swift
@@ -116,8 +116,8 @@ class BookmarkMigrationTests: XCTestCase {
             let mobileFavoritesArray = BookmarkUtils.fetchFavoritesFolder(withUUID: FavoritesFolderID.mobile.rawValue, in: latestContext)?.favoritesArray.compactMap(\.uuid)
             XCTAssertEqual(legacyFavoritesInOrder, mobileFavoritesArray)
 
-            let uudis = BookmarkUtils.fetchAllBookmarksUUIDs(in: latestContext)
-            XCTAssert(!uudis.isEmpty)
+            let uuids = BookmarkUtils.fetchAllBookmarksUUIDs(in: latestContext)
+            XCTAssert(!uuids.isEmpty)
         })
 
         // Test whole structure
diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
index 3a18f5d8a..6f42e3383 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
@@ -864,6 +864,52 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
         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"]), // Creates a 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)
+
+        print("%@", patchData)
+        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)
 
@@ -888,8 +934,8 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
         })
 
         // Simulate two kinds of "removal":
-        // - Removal only from children list.
-        // - Removal of entity and from children list.
+        // - "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)
@@ -924,9 +970,6 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
             }
         })
 
-        // Simulate two kinds of "removal":
-        // - Removal only from children list.
-        // - Removal of entity and from children list.
         let receivedUpdate: [Syncable] = [
             .rootFolder(children: []),
             .folder(id: "1", isDeleted: true)

From da79f8613fd256588599fd5e1305655203e51b2b Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Fri, 15 Mar 2024 12:57:50 +0100
Subject: [PATCH 8/9] Tweak comments

---
 .../Bookmarks/BookmarksProviderTests.swift                    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
index 6f42e3383..87b6ae71c 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
@@ -876,7 +876,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
         }
 
         let received: [Syncable] = [
-            .rootFolder(children: ["2", "1", "3"]), // Creates a Stubs with id 2 and 3
+            .rootFolder(children: ["2", "1", "3"]), // Create Stubs with id 2 and 3
             .favoritesFolder(favorites: ["3", "1", "2"]),
             .bookmark(id: "1")
         ]
@@ -922,7 +922,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
         }
 
         let received: [Syncable] = [
-            .rootFolder(children: ["1", "2", "3"]), // Creates a Stub with id 2
+            .rootFolder(children: ["1", "2", "3"]), // Create Stubs with id 2 and 3
             .bookmark(id: "1")
         ]
 

From e26399a03f0a0763c4eae2acdfe364d04e151162 Mon Sep 17 00:00:00 2001
From: Bartek Waresiak <bartek@duckduckgo.com>
Date: Fri, 15 Mar 2024 13:00:08 +0100
Subject: [PATCH 9/9] Remove print

---
 .../Bookmarks/BookmarksProviderTests.swift                       | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
index 87b6ae71c..06aac8b49 100644
--- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
+++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
@@ -900,7 +900,6 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {
 
         let patchData = try await provider.fetchChangedObjects(encryptedUsing: crypter)
 
-        print("%@", patchData)
         let changedObjects = patchData.map(SyncableBookmarkAdapter.init(syncable:))
 
         XCTAssertEqual(changedObjects.count, 2)