fix(#304): stop deduplicate() from discarding favorites, read state, and podcast progress

MacMagazine/Features/FeedLibrary/Sources/FeedLibrary/Database/FeedDB.swift

@@ -16,7 +16,9 @@ public final class FeedDB {
     public var excerpt: String = ""
     public var fullContent: String = ""
     public var favorite: Bool = false
+    public var favoriteModifiedAt: Date = Date.distantPast
     public var read: Bool = false
+    public var readModifiedAt: Date = Date.distantPast
     public var modifiedAt: Date = Date()
 
     public init(
@@ -31,7 +33,9 @@ public final class FeedDB {
         excerpt: String = "",
         fullContent: String = "",
         favorite: Bool = false,
-        ead: Bool = false,
+        favoriteModifiedAt: Date = Date.distantPast,
+        read: Bool = false,
+        readModifiedAt: Date = Date.distantPast,
         modifiedAt: Date = Date()
     ) {
         self.postId = postId
@@ -45,7 +49,9 @@ public final class FeedDB {
         self.excerpt = excerpt
         self.fullContent = fullContent
         self.favorite = favorite
+        self.favoriteModifiedAt = favoriteModifiedAt
         self.read = read
+        self.readModifiedAt = readModifiedAt
         self.modifiedAt = modifiedAt
     }
 }
@@ -75,6 +81,14 @@ extension FeedDB: ModelFavoritable {
         data.forEach { context.delete($0) }
         try? context.save()
     }
+
+    /// Flips `favorite` and stamps `favoriteModifiedAt`, so `deduplicate()` can tell this
+    /// explicit action apart from a blank sync-created duplicate.
+    public func toggleFavorite() {
+        favorite.toggle()
+        favoriteModifiedAt = Date()
+        modifiedAt = Date()
+    }
 }
 
 extension FeedDB: ModelReadable {
@@ -83,24 +97,53 @@ extension FeedDB: ModelReadable {
         guard let context,
               let data = try? context.fetch(descriptor) else { return }
         for post in data {
-            post.read = true
-            post.modifiedAt = Date()
+            post.markAsRead()
         }
         try? context.save()
     }
+
+    /// Flips `read` and stamps `readModifiedAt`, so `deduplicate()` can tell this explicit
+    /// action apart from a blank sync-created duplicate.
+    public func toggleRead() {
+        read.toggle()
+        readModifiedAt = Date()
+        modifiedAt = Date()
+    }
+
+    /// Sets `read` to `true` and stamps `readModifiedAt`, so `deduplicate()` can tell this
+    /// explicit action apart from a blank sync-created duplicate.
+    public func markAsRead() {
+        read = true
+        readModifiedAt = Date()
+        modifiedAt = Date()
+    }
 }
 
+extension FeedDB: ModelPrioritizable {}
+
 extension FeedDB: ModelDuplicable {
     public static func deduplicate(using context: ModelContext?) {
         let descriptor = FetchDescriptor<FeedDB>(sortBy: [SortDescriptor(\FeedDB.pubDate, order: .reverse)])
         guard let context,
               let data = try? context.fetch(descriptor) else { return }
 
-        let recordsToDelete = Dictionary(grouping: data, by: \.postId)
-            .values
-            .flatMap { $0.sorted { $0.modifiedAt > $1.modifiedAt }.dropFirst() }
+        FeedDB.resolveDuplicates(in: data, postId: \.postId, modifiedAt: \.modifiedAt) { survivor, group in
+            if let latestFavorite = FeedDB.latest(
+                in: group, value: { $0.favorite }, modifiedAt: { $0.favoriteModifiedAt },
+                preferOnTie: { candidate, _ in candidate }
+            ) {
+                survivor.favorite = latestFavorite.value
+                survivor.favoriteModifiedAt = latestFavorite.modifiedAt
+            }
+            if let latestRead = FeedDB.latest(
+                in: group, value: { $0.read }, modifiedAt: { $0.readModifiedAt },
+                preferOnTie: { candidate, _ in candidate }
+            ) {
+                survivor.read = latestRead.value
+                survivor.readModifiedAt = latestRead.modifiedAt
+            }
+        } delete: { context.delete($0) }
 
-        recordsToDelete.forEach { context.delete($0) }
         try? context.save()
     }
 }

MacMagazine/Features/FeedLibrary/Sources/FeedLibrary/Database/PodcastDB.swift

@@ -15,8 +15,10 @@ public final class PodcastDB {
     public var duration: String = ""
     public var podcastFrame: String = ""
     public var favorite: Bool = false
+    public var favoriteModifiedAt: Date = Date.distantPast
     public var playable: Bool = false
     public var current: Double = 0.0
+    public var progressModifiedAt: Date = Date.distantPast
     public var modifiedAt: Date = Date()
 
     public init(
@@ -31,8 +33,10 @@ public final class PodcastDB {
         duration: String = "",
         podcastFrame: String = "",
         favorite: Bool = false,
+        favoriteModifiedAt: Date = Date.distantPast,
         playable: Bool = false,
         current: Double = 0.0,
+        progressModifiedAt: Date = Date.distantPast,
         modifiedAt: Date = Date()
     ) {
         self.postId = postId
@@ -46,8 +50,10 @@ public final class PodcastDB {
         self.duration = duration
         self.podcastFrame = podcastFrame
         self.favorite = favorite
+        self.favoriteModifiedAt = favoriteModifiedAt
         self.playable = playable
         self.current = current
+        self.progressModifiedAt = progressModifiedAt
         self.modifiedAt = modifiedAt
     }
 }
@@ -60,19 +66,51 @@ extension PodcastDB: ModelFavoritable {
         data.forEach { context.delete($0) }
         try? context.save()
     }
+
+    /// Flips `favorite` and stamps `favoriteModifiedAt`, so `deduplicate()` can tell this
+    /// explicit action apart from a blank sync-created duplicate.
+    public func toggleFavorite() {
+        favorite.toggle()
+        favoriteModifiedAt = Date()
+        modifiedAt = Date()
+    }
 }
 
+extension PodcastDB {
+    /// Sets `current` and stamps `progressModifiedAt`, so `deduplicate()` can tell this
+    /// explicit action apart from a blank sync-created duplicate.
+    public func updateProgress(_ current: Double) {
+        self.current = current
+        progressModifiedAt = Date()
+        modifiedAt = Date()
+    }
+}
+
+extension PodcastDB: ModelPrioritizable {}
+
 extension PodcastDB: ModelDuplicable {
     public static func deduplicate(using context: ModelContext?) {
         let descriptor = FetchDescriptor<PodcastDB>()
         guard let context,
               let data = try? context.fetch(descriptor) else { return }
 
-        let recordsToDelete = Dictionary(grouping: data, by: \.pubDate)
-            .values
-            .flatMap { $0.sorted { $0.modifiedAt > $1.modifiedAt }.dropFirst() }
+        PodcastDB.resolveDuplicates(in: data, postId: \.postId, modifiedAt: \.modifiedAt) { survivor, group in
+            if let latestFavorite = PodcastDB.latest(
+                in: group, value: { $0.favorite }, modifiedAt: { $0.favoriteModifiedAt },
+                preferOnTie: { candidate, _ in candidate }
+            ) {
+                survivor.favorite = latestFavorite.value
+                survivor.favoriteModifiedAt = latestFavorite.modifiedAt
+            }
+            if let latestProgress = PodcastDB.latest(
+                in: group, value: { $0.current }, modifiedAt: { $0.progressModifiedAt },
+                preferOnTie: { candidate, current in candidate > current }
+            ) {
+                survivor.current = latestProgress.value
+                survivor.progressModifiedAt = latestProgress.modifiedAt
+            }
+        } delete: { context.delete($0) }
 
-        recordsToDelete.forEach { context.delete($0) }
         try? context.save()
     }
 }

MacMagazine/Features/FeedLibrary/Tests/FeedLibraryTests/FeedDBTests.swift

@@ -4,6 +4,19 @@ import StorageLibrary
 import SwiftData
 import Testing
 
+private struct FieldMergeCase: Sendable {
+    let aFavorite: Bool
+    let aFavoriteModifiedAt: Date
+    let aRead: Bool
+    let aReadModifiedAt: Date
+    let bFavorite: Bool
+    let bFavoriteModifiedAt: Date
+    let bRead: Bool
+    let bReadModifiedAt: Date
+    let expectedFavorite: Bool
+    let expectedRead: Bool
+}
+
 @Suite("FeedDB Tests")
 @MainActor
 struct FeedDBTests {
@@ -51,6 +64,12 @@ struct FeedDBTests {
         #expect(feed.favorite == favorite)
     }
 
+    @Test("FeedDB initializer's read parameter actually sets the read property")
+    func initializationSetsReadFromParameter() {
+        let feed = FeedDB(postId: "1", read: true)
+        #expect(feed.read == true)
+    }
+
     @Test("FeedDB should initialize with default values")
     func initializationWithDefaults() {
         // When
@@ -321,6 +340,22 @@ struct FeedDBTests {
         #expect(remaining.isEmpty)
     }
 
+    @Test("toggleFavorite flips favorite and stamps favoriteModifiedAt without touching read")
+    func toggleFavoriteStampsFavoriteModifiedAt() {
+        let feed = FeedDB(postId: "1")
+        feed.read = true
+        let readModifiedAtBefore = feed.readModifiedAt
+
+        feed.toggleFavorite()
+
+        #expect(feed.favorite == true)
+        #expect(feed.favoriteModifiedAt != Date.distantPast)
+        #expect(feed.readModifiedAt == readModifiedAtBefore)
+
+        feed.toggleFavorite()
+        #expect(feed.favorite == false)
+    }
+
     // MARK: - ModelReadable Tests
 
     @Test("markAllAsRead should mark all unread posts as read")
@@ -383,6 +418,30 @@ struct FeedDBTests {
         #expect(fetched?.modifiedAt != oldDate)
     }
 
+    @Test("markAsRead sets read and stamps readModifiedAt without touching favorite")
+    func markAsReadStampsReadModifiedAt() {
+        let feed = FeedDB(postId: "1", favorite: true)
+        let favoriteModifiedAtBefore = feed.favoriteModifiedAt
+
+        feed.markAsRead()
+
+        #expect(feed.read == true)
+        #expect(feed.readModifiedAt != Date.distantPast)
+        #expect(feed.favoriteModifiedAt == favoriteModifiedAtBefore)
+    }
+
+    @Test("toggleRead flips read and stamps readModifiedAt")
+    func toggleReadStampsReadModifiedAt() {
+        let feed = FeedDB(postId: "1")
+
+        feed.toggleRead()
+        #expect(feed.read == true)
+        #expect(feed.readModifiedAt != Date.distantPast)
+
+        feed.toggleRead()
+        #expect(feed.read == false)
+    }
+
     // MARK: - ModelDuplicable Tests
 
     @Test("deduplicate removes duplicate postIds keeping most recently modified")
@@ -436,6 +495,108 @@ struct FeedDBTests {
         #expect(remaining.contains { $0.title == "Y-new" })
     }
 
+    @Test("deduplicate keeps the most recently modified content even when a different duplicate wins the favorite merge")
+    func deduplicateDecouplesContentSurvivorFromFavoriteMerge() {
+        let storage = Database(models: [FeedDB.self], inMemory: true)
+        let originallyFavorited = FeedDB(postId: "dup-1", title: "Original", favorite: true, favoriteModifiedAt: Date(timeIntervalSince1970: 5000), modifiedAt: Date(timeIntervalSince1970: 1000))
+        let freshSync = FeedDB(postId: "dup-1", title: "Fresh Sync", favorite: false, modifiedAt: Date(timeIntervalSince1970: 2000))
+
+        storage.context.insert(originallyFavorited)
+        storage.context.insert(freshSync)
+        try? storage.context.save()
+
+        FeedDB.deduplicate(using: storage.context)
+
+        let remaining = storage.fetch(FeedDB.self)
+        #expect(remaining.count == 1)
+        #expect(remaining.first?.title == "Fresh Sync")
+        #expect(remaining.first?.favorite == true)
+    }
+
+    @Test("deduplicate respects an explicit cross-device unfavorite over an older favorite")
+    func deduplicateRespectsExplicitUnfavorite() {
+        let storage = Database(models: [FeedDB.self], inMemory: true)
+        let favoritedOnDeviceA = FeedDB(postId: "dup-1", favorite: true, favoriteModifiedAt: Date(timeIntervalSince1970: 1000), modifiedAt: Date(timeIntervalSince1970: 1000))
+        let unfavoritedOnDeviceB = FeedDB(postId: "dup-1", favorite: false, favoriteModifiedAt: Date(timeIntervalSince1970: 2000), modifiedAt: Date(timeIntervalSince1970: 2000))
+
+        storage.context.insert(favoritedOnDeviceA)
+        storage.context.insert(unfavoritedOnDeviceB)
+        try? storage.context.save()
+
+        FeedDB.deduplicate(using: storage.context)
+
+        let remaining = storage.fetch(FeedDB.self)
+        #expect(remaining.count == 1)
+        #expect(remaining.first?.favorite == false)
+    }
+
+    @Test(
+        "deduplicate merges favorite and read using each field's own timestamp",
+        arguments: [
+            FieldMergeCase(
+                aFavorite: true, aFavoriteModifiedAt: Date(timeIntervalSince1970: 1000), aRead: true, aReadModifiedAt: Date(timeIntervalSince1970: 1000),
+                bFavorite: false, bFavoriteModifiedAt: .distantPast, bRead: false, bReadModifiedAt: .distantPast,
+                expectedFavorite: true, expectedRead: true
+            ),
+            FieldMergeCase(
+                aFavorite: true, aFavoriteModifiedAt: Date(timeIntervalSince1970: 1000), aRead: false, aReadModifiedAt: Date(timeIntervalSince1970: 1000),
+                bFavorite: false, bFavoriteModifiedAt: Date(timeIntervalSince1970: 2000), bRead: true, bReadModifiedAt: Date(timeIntervalSince1970: 2000),
+                expectedFavorite: false, expectedRead: true
+            ),
+            FieldMergeCase(
+                aFavorite: true, aFavoriteModifiedAt: Date(timeIntervalSince1970: 2000), aRead: false, aReadModifiedAt: Date(timeIntervalSince1970: 1000),
+                bFavorite: false, bFavoriteModifiedAt: Date(timeIntervalSince1970: 1000), bRead: true, bReadModifiedAt: Date(timeIntervalSince1970: 2000),
+                expectedFavorite: true, expectedRead: true
+            ),
+            FieldMergeCase(
+                aFavorite: false, aFavoriteModifiedAt: .distantPast, aRead: false, aReadModifiedAt: .distantPast,
+                bFavorite: false, bFavoriteModifiedAt: .distantPast, bRead: false, bReadModifiedAt: .distantPast,
+                expectedFavorite: false, expectedRead: false
+            )
+        ]
+    )
+    fileprivate func deduplicateMergesFieldsByOwnTimestamp(_ testCase: FieldMergeCase) {
+        let storage = Database(models: [FeedDB.self], inMemory: true)
+        let recordA = FeedDB(postId: "dup-1", favorite: testCase.aFavorite, favoriteModifiedAt: testCase.aFavoriteModifiedAt, modifiedAt: Date(timeIntervalSince1970: 1000))
+        recordA.read = testCase.aRead
+        recordA.readModifiedAt = testCase.aReadModifiedAt
+        let recordB = FeedDB(postId: "dup-1", favorite: testCase.bFavorite, favoriteModifiedAt: testCase.bFavoriteModifiedAt, modifiedAt: Date(timeIntervalSince1970: 2000))
+        recordB.read = testCase.bRead
+        recordB.readModifiedAt = testCase.bReadModifiedAt
+
+        storage.context.insert(recordA)
+        storage.context.insert(recordB)
+        try? storage.context.save()
+
+        FeedDB.deduplicate(using: storage.context)
+
+        let remaining = storage.fetch(FeedDB.self)
+        #expect(remaining.count == 1)
+        #expect(remaining.first?.favorite == testCase.expectedFavorite)
+        #expect(remaining.first?.read == testCase.expectedRead)
+    }
+
+    @Test("deduplicate breaks an exact favoriteModifiedAt tie by preferring true, regardless of insertion order")
+    func deduplicateBreaksFavoriteTieDeterministically() {
+        let insertionOrders: [(first: Bool, second: Bool)] = [(true, false), (false, true)]
+
+        for order in insertionOrders {
+            let storage = Database(models: [FeedDB.self], inMemory: true)
+            let first = FeedDB(postId: "dup-1", favorite: order.first, modifiedAt: Date(timeIntervalSince1970: 1000))
+            let second = FeedDB(postId: "dup-1", favorite: order.second, modifiedAt: Date(timeIntervalSince1970: 1000))
+
+            storage.context.insert(first)
+            storage.context.insert(second)
+            try? storage.context.save()
+
+            FeedDB.deduplicate(using: storage.context)
+
+            let remaining = storage.fetch(FeedDB.self)
+            #expect(remaining.count == 1)
+            #expect(remaining.first?.favorite == true)
+        }
+    }
+
     @Test("deduplicate is safe on empty database")
     func deduplicateSafeOnEmpty() {
         let storage = Database(models: [FeedDB.self], inMemory: true)