From b260e972faf56c8b8e26c12b8e9b9c5a6af4487d Mon Sep 17 00:00:00 2001 From: bwaresiak Date: Fri, 15 Mar 2024 13:10:19 +0100 Subject: [PATCH 1/4] Stub objects for Bookmarks sync (#713) Required: Task/Issue URL: https://app.asana.com/0/0/1206831058661531/f iOS PR: duckduckgo/iOS#2593 macOS PR: duckduckgo/macos-browser#2418 What kind of version bump will this require?: Major Description: Provide implementation for stub objects. --- Package.swift | 3 + Sources/Bookmarks/BookmarkEntity.swift | 15 +- Sources/Bookmarks/BookmarkListViewModel.swift | 5 +- Sources/Bookmarks/BookmarkUtils.swift | 32 +++- .../.xccurrentversion | 2 +- .../BookmarksModel 6.xcdatamodel/contents | 28 +++ .../FaviconsFetchOperation.swift | 5 +- .../BookmarkCoreDataImporter.swift | 5 +- .../Bookmarks/MenuBookmarksViewModel.swift | 5 +- .../BookmarksTestDBBuilder.swift | 2 +- .../BookmarksTestsUtils/BookmarkTree.swift | 79 +++++---- .../internal/BookmarkEntity+Syncable.swift | 1 + .../internal/BookmarksResponseHandler.swift | 29 +++- .../internal/SyncableBookmarkAdapter.swift | 7 +- .../BookmarkMigrationTests.swift | 7 + .../BookmarkDomainsTests.swift | 5 +- .../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 .../Bookmarks/BookmarksProviderTests.swift | 164 ++++++++++++++++++ ...marksRegularSyncResponseHandlerTests.swift | 38 ++++ 21 files changed, 377 insertions(+), 55 deletions(-) create mode 100644 Sources/Bookmarks/BookmarksModel.xcdatamodeld/BookmarksModel 6.xcdatamodel/contents 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/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 0000000000000000000000000000000000000000..1a3bbe6095efbfc416950c06ccf62acbae4f7094 GIT binary patch literal 57344 zcmeI%c|4Tc{{Zlrh_R2tDA6clvJX*88D^L<<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{)xZklNUyc<%lxAb67gOj%J#SpKSjiiZe`BCPp{O5Y=!`lv8msTh0yQ}ez0pGIC} zYH0th&lCd;}{3j{mMg(h0UW8XaRx<=0HY(CC(U0tw5XT~v$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!=0Df00e*l5C8)IYXXUW{H>xxins}S84jawjMcGPVu02$(l@cx z!7MSh)GLVy$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$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_ zYTGsj$vO~9njo+9v|USZt|2X)mzcog;nd!R(f-! zW^_}8Y4+w*qyr4rH482F%rshVX1s7{koGJBby&hnwsvpb5W6l7*lU^A1~Im`rrjSV6Gu;ic}YxkNW75_B*?zwepLr zI}fOG6I{?*0WX!;T^3Tf&Xw-1_~O3dl2R58G1Ig_cLmfXbvdMpiIhw@uPs|?dY+ZJu0>K=FmZo% z%{7|~C4TVBXN4Aqv}!6Q-g;J4dQ8cRW+K1KWx(~2XC%JKmYF?wk8|V0 zb%)PK9`7Y;1*CZyaI2IwFC2Z^yf=9DWY4q{81+= z-s7tIsfSQ#P|Q8d&M>y!EoP{argl~MaijJ-%3`DR!kMFmcURp)$Iyz%#`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#`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+>4_zY~{4u!MMnI7^~mUQrXjL>|Si( zwXNCHAv{Uj&HM?o=GhT{E$>5 z0$=L(-S-;3xGhM^Kd(8cA*?JACB3nA3~lpi@k`E0_vlrbaFMQ!*^>7fWM*wy06m&= zGZxL+G*$#(W7eM{1iK|BD%%II-)LUp#p9l1y1OSyT~67S8hmm~Ik zYVAOn%u-RTJD}GyuPMK~L+DwBG1)Xa10Sn&krI}%geP2n!}6fyXu(nM;SpFcTvZy0 z*U){q#Wi2Lpb-&#M|xpgx{#u<>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#mQ^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}EpwVfW4lfsw4B}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 zJZf8LOH%3=9maJTK?zh1Av^7})*be%FnY8=l3s#k79S896&ccIkPY)CBF~ zghdg;mUsj@S)94E(%l)gqhaxR4dEo8eGV}3Ad6FQ`7I5`J72o25puSF;_) zZ!|KWyGWi8c97LNmtAz6_%iQbG;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;#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@nvN z`ZOP3Ro}KZc~;$gbX=)X<;us6r3bcuvG0x>KABipyQRNkca7Ljt=hl7|Kj7*ljj~? zV|Qvfsq8-m0R#|0009ILKmY**5I_I{1g?64F&(R>(RcTHJD=J*vvb4KvpZ(@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*srAbdp} 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 zAbxt9BOUz0z#z|UyWVWxN4O89l5Uuewjs~&U~YBV+c}r7gb!~pAC;Z_S&8! zACWIGG|!ffAbImQtT!SkgH2q1s} a0tg_000IagfB*srT$+Fz8*oYE2>t~V> Date: Fri, 15 Mar 2024 13:25:53 +0100 Subject: [PATCH 2/4] Revert "Remove Autoconsent subfeature (#697)" (#730) This reverts commit 2181cdcd27be961f10d988fbb202431a6ec6d56d. --- .../PrivacyConfig/Features/PrivacyFeature.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift index c58ebf4e2..15a5e4332 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift @@ -104,3 +104,11 @@ public enum PrivacyDashboardSubfeature: String, PrivacySubfeature { case toggleReports } + +public enum AutoconsentSubfeature: String, PrivacySubfeature { + public var parent: PrivacyFeature { + .autoconsent + } + + case onByDefault +} From ac2127a26f75b2aa293f6036bcdd2bc241d09819 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Fri, 15 Mar 2024 20:55:19 +0100 Subject: [PATCH 3/4] Reset Cache on Subscription Purchase & Restore (#719) * Reset Cache on Subscription Purchase & Restore * Skip sending sign out notification if account is cleared as part of failed purchase flow * URL comparison * Fix URL * Update subscription product ids * Add TF product ids * Stub objects for Bookmarks sync (#713) --- Sources/Subscription/AccountManager.swift | 6 +++-- .../Flows/AppStore/AppStorePurchaseFlow.swift | 6 ++++- .../Flows/AppStore/AppStoreRestoreFlow.swift | 4 ++++ .../Flows/Stripe/StripePurchaseFlow.swift | 7 ++++++ Sources/Subscription/PurchaseManager.swift | 6 +++-- Sources/Subscription/URL+Subscription.swift | 24 ++++++++++++++++++- Sources/Subscription/UserDefaultsCache.swift | 5 ++++ 7 files changed, 52 insertions(+), 6 deletions(-) diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index 5cd74cfa3..85e631051 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -172,7 +172,7 @@ public class AccountManager: AccountManaging { NotificationCenter.default.post(name: .accountDidSignIn, object: self, userInfo: nil) } - public func signOut() { + public func signOut(skipNotification: Bool = false) { os_log(.info, log: .subscription, "[AccountManager] signOut") do { @@ -188,7 +188,9 @@ public class AccountManager: AccountManaging { } } - NotificationCenter.default.post(name: .accountDidSignOut, object: self, userInfo: nil) + if !skipNotification { + NotificationCenter.default.post(name: .accountDidSignOut, object: self, userInfo: nil) + } } public func migrateAccessTokenToNewStore() throws { diff --git a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift index 2b3bb65fa..95a202194 100644 --- a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift @@ -104,7 +104,7 @@ public final class AppStorePurchaseFlow { return .success(transactionJWS) case .failure(let error): os_log(.error, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription error: %{public}s", String(reflecting: error)) - AccountManager(subscriptionAppGroup: subscriptionAppGroup).signOut() + AccountManager(subscriptionAppGroup: subscriptionAppGroup).signOut(skipNotification: true) switch error { case .purchaseCancelledByUser: return .failure(.cancelledByUser) @@ -117,6 +117,10 @@ public final class AppStorePurchaseFlow { // swiftlint:enable cyclomatic_complexity @discardableResult public static func completeSubscriptionPurchase(with transactionJWS: TransactionJWS, subscriptionAppGroup: String) async -> Result { + + // Clear subscription Cache + SubscriptionService.signOut() + os_log(.info, log: .subscription, "[AppStorePurchaseFlow] completeSubscriptionPurchase") guard let accessToken = AccountManager(subscriptionAppGroup: subscriptionAppGroup).accessToken else { return .failure(.missingEntitlements) } diff --git a/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift b/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift index eec52b7b6..b9006c845 100644 --- a/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift @@ -35,6 +35,10 @@ public final class AppStoreRestoreFlow { } public static func restoreAccountFromPastPurchase(subscriptionAppGroup: String) async -> Result { + + // Clear subscription Cache + SubscriptionService.signOut() + os_log(.info, log: .subscription, "[AppStoreRestoreFlow] restoreAccountFromPastPurchase") guard let lastTransactionJWSRepresentation = await PurchaseManager.mostRecentTransaction() else { diff --git a/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift b/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift index 1ddea84cf..0f0a852c7 100644 --- a/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift +++ b/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift @@ -64,6 +64,9 @@ public final class StripePurchaseFlow { public static func prepareSubscriptionPurchase(emailAccessToken: String?, subscriptionAppGroup: String) async -> Result { os_log(.info, log: .subscription, "[StripePurchaseFlow] prepareSubscriptionPurchase") + // Clear subscription Cache + SubscriptionService.signOut() + var authToken: String = "" switch await AuthService.createAccount(emailAccessToken: emailAccessToken) { @@ -79,6 +82,10 @@ public final class StripePurchaseFlow { } public static func completeSubscriptionPurchase(subscriptionAppGroup: String) async { + + // Clear subscription Cache + SubscriptionService.signOut() + os_log(.info, log: .subscription, "[StripePurchaseFlow] completeSubscriptionPurchase") let accountManager = AccountManager(subscriptionAppGroup: subscriptionAppGroup) diff --git a/Sources/Subscription/PurchaseManager.swift b/Sources/Subscription/PurchaseManager.swift index 9468597b7..a6dfbe962 100644 --- a/Sources/Subscription/PurchaseManager.swift +++ b/Sources/Subscription/PurchaseManager.swift @@ -42,8 +42,10 @@ public enum PurchaseManagerError: Error { public final class PurchaseManager: ObservableObject { static let productIdentifiers = ["ios.subscription.1month", "ios.subscription.1year", - "subscription.1week", "subscription.1month", "subscription.1year", - "review.subscription.1week", "review.subscription.1month", "review.subscription.1year"] + "subscription.1month", "subscription.1year", + "review.subscription.1month", "review.subscription.1year", + "tf.sandbox.subscription.1month", "tf.sandbox.subscription.1year", + "ddg.privacy.pro.monthly.renews.us", "ddg.privacy.pro.yearly.renews.us"] public static let shared = PurchaseManager() diff --git a/Sources/Subscription/URL+Subscription.swift b/Sources/Subscription/URL+Subscription.swift index 0bc7fddd4..5e774de5f 100644 --- a/Sources/Subscription/URL+Subscription.swift +++ b/Sources/Subscription/URL+Subscription.swift @@ -34,7 +34,7 @@ public extension URL { } static var subscriptionFAQ: URL { - URL(string: "https://duckduckgo.com/about")! + URL(string: "https://duckduckgo.com/duckduckgo-help-pages/privacy-pro/")! } // MARK: - Subscription Email @@ -50,6 +50,10 @@ public extension URL { subscriptionBaseURL.appendingPathComponent("manage") } + static var subscriptionActivateSuccess: URL { + subscriptionBaseURL.appendingPathComponent("activate/success") + } + // MARK: - App Store app manage subscription URL static var manageSubscriptionsInAppStoreAppURL: URL { @@ -66,4 +70,22 @@ public extension URL { URL(string: "https://duckduckgo.com/identity-theft-restoration?environment=staging")! } } + + func forComparison() -> URL { + guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { + return self + } + + if let queryItems = components.queryItems, !queryItems.isEmpty { + components.queryItems = queryItems.filter { !["environment"].contains($0.name) } + + if components.queryItems?.isEmpty ?? true { + components.queryItems = nil + } + } else { + components.queryItems = nil + } + return components.url ?? self + } + } diff --git a/Sources/Subscription/UserDefaultsCache.swift b/Sources/Subscription/UserDefaultsCache.swift index a2daf720c..9d7b5de49 100644 --- a/Sources/Subscription/UserDefaultsCache.swift +++ b/Sources/Subscription/UserDefaultsCache.swift @@ -17,6 +17,7 @@ // import Foundation +import Common public struct UserDefaultsCacheSettings { @@ -66,6 +67,7 @@ public class UserDefaultsCache { do { let data = try encoder.encode(cacheObject) userDefaults?.set(data, forKey: key.rawValue) + os_log(.debug, log: .subscription, "Cache Set: \(cacheObject)") } catch { assertionFailure("Failed to encode CacheObject: \(error)") } @@ -77,8 +79,10 @@ public class UserDefaultsCache { do { let cacheObject = try decoder.decode(CacheObject.self, from: data) if cacheObject.expires > Date() { + os_log(.debug, log: .subscription, "Cache Hit: \(ObjectType.self)") return cacheObject.object } else { + os_log(.debug, log: .subscription, "Cache Miss: \(ObjectType.self)") reset() // Clear expired data return nil } @@ -88,6 +92,7 @@ public class UserDefaultsCache { } public func reset() { + os_log(.debug, log: .subscription, "Cache Clean: \(ObjectType.self)") userDefaults?.removeObject(forKey: key.rawValue) } } From eced8f93c945ff2fa4ff92bdd619514d4eff7131 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Sun, 17 Mar 2024 23:38:31 +0100 Subject: [PATCH 4/4] Improves VPN pixel info and adds tests (#732) Task/Issue URL: https://app.asana.com/0/0/1206857816167860/f iOS PR: https://github.com/duckduckgo/iOS/pull/2604 macOS PR: https://github.com/duckduckgo/macos-browser/pull/2434 What kind of version bump will this require?: Patch ## Description Adds `CustomNSError` support to `NetworkProtectionError`. --- .../Diagnostics/NetworkProtectionError.swift | 109 +++++++++++++++++- 1 file changed, 108 insertions(+), 1 deletion(-) diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift index 221720a99..6bfdb55a3 100644 --- a/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift +++ b/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift @@ -22,7 +22,7 @@ protocol NetworkProtectionErrorConvertible { var networkProtectionError: NetworkProtectionError { get } } -public enum NetworkProtectionError: LocalizedError { +public enum NetworkProtectionError: LocalizedError, CustomNSError { // Tunnel configuration errors case noServerRegistrationInfo case couldNotSelectClosestServer @@ -77,6 +77,113 @@ public enum NetworkProtectionError: LocalizedError { // Unhandled error case unhandledError(function: String, line: Int, error: Error) + public static let errorDomain = "com.duckduckgo.NetworkProtectionError.domain" + + public var errorCode: Int { + switch self { + // 0+ - Tunnel configuration errors + case .noServerRegistrationInfo: return 0 + case .couldNotSelectClosestServer: return 1 + case .couldNotGetPeerPublicKey: return 2 + case .couldNotGetPeerHostName: return 3 + case .couldNotGetInterfaceAddressRange: return 4 + // 100+ - Client errors + case .failedToFetchServerList: return 100 + case .failedToParseServerListResponse: return 101 + case .failedToFetchLocationList: return 102 + case .failedToParseLocationListResponse: return 103 + case .failedToEncodeRegisterKeyRequest: return 104 + case .failedToFetchRegisteredServers: return 105 + case .failedToParseRegisteredServersResponse: return 106 + case .failedToEncodeRedeemRequest: return 107 + case .invalidInviteCode: return 108 + case .failedToRedeemInviteCode: return 109 + case .failedToRetrieveAuthToken: return 110 + case .failedToParseRedeemResponse: return 111 + case .invalidAuthToken: return 112 + case .serverListInconsistency: return 113 + // 200+ - Server list store errors + case .failedToEncodeServerList: return 200 + case .failedToDecodeServerList: return 201 + case .failedToWriteServerList: return 202 + case .noServerListFound: return 203 + case .couldNotCreateServerListDirectory: return 204 + case .failedToReadServerList: return 205 + // 300+ - Keychain errors + case .failedToCastKeychainValueToData: return 300 + case .keychainReadError: return 301 + case .keychainWriteError: return 302 + case .keychainUpdateError: return 303 + case .keychainDeleteError: return 304 + // 400+ - Wireguard errors + case .wireGuardCannotLocateTunnelFileDescriptor: return 400 + case .wireGuardInvalidState: return 401 + case .wireGuardDnsResolution: return 402 + case .wireGuardSetNetworkSettings: return 403 + case .startWireGuardBackend: return 404 + // 500+ Auth errors + case .noAuthTokenFound: return 500 + // 600+ Subscription errors + case .vpnAccessRevoked: return 600 + // 700+ Unhandled errors + case .unhandledError: return 700 + } + } + + public var errorUserInfo: [String: Any] { + switch self { + case .noServerRegistrationInfo, + .couldNotSelectClosestServer, + .couldNotGetPeerPublicKey, + .couldNotGetPeerHostName, + .couldNotGetInterfaceAddressRange, + .failedToEncodeRegisterKeyRequest, + .failedToEncodeRedeemRequest, + .invalidInviteCode, + .failedToRetrieveAuthToken, + .invalidAuthToken, + .serverListInconsistency, + .noServerListFound, + .failedToCastKeychainValueToData, + .keychainReadError, + .keychainWriteError, + .keychainUpdateError, + .keychainDeleteError, + .wireGuardCannotLocateTunnelFileDescriptor, + .wireGuardInvalidState, + .wireGuardDnsResolution, + .startWireGuardBackend, + .noAuthTokenFound, + .vpnAccessRevoked: + return [:] + case .failedToFetchServerList(let error), + .failedToFetchLocationList(let error), + .failedToFetchRegisteredServers(let error), + .failedToRedeemInviteCode(let error): + guard let error else { + return [:] + } + + return [ + NSUnderlyingErrorKey: error + ] + case .failedToParseServerListResponse(let error), + .failedToParseLocationListResponse(let error), + .failedToParseRegisteredServersResponse(let error), + .failedToParseRedeemResponse(let error), + .failedToEncodeServerList(let error), + .failedToDecodeServerList(let error), + .failedToWriteServerList(let error), + .couldNotCreateServerListDirectory(let error), + .failedToReadServerList(let error), + .wireGuardSetNetworkSettings(let error), + .unhandledError(_, _, let error): + return [ + NSUnderlyingErrorKey: error + ] + } + } + public var errorDescription: String? { // This is probably not the most elegant error to show to a user but // it's a great way to get detailed reports for those cases we haven't