From 0e6017999af4717ecf4deec7fac7e873becbfbf8 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 14:13:39 -0700 Subject: [PATCH] fix(feed): drop deleted posts from the feed cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FeedView.deleteMessage` removed the row from its private working copy only, so `AppDataStore.feedMessages` and the persisted feed cache kept the deleted post and served it back on the next launch that read the cache. `removeFeedMessage(id:)` mirrors `updateFeedMessage(_:)`: mutate the page, persist the cache, no-op on an unknown id (and no revision bump). It removes every copy of the id rather than one index, because `insertFeedMessage` does not deduplicate. No cascade to replies: the feed page is reply-free by construction — the server filters `parentId: null` on `GET /api/messages`, and `ComposeView` skips `insertFeedMessage` when replying — so a deleted parent has no replies in this cache to remove. The backend's `onDelete: Cascade` settles the server side. The view's local removal stays and is load-bearing: `FeedMerge` keeps rows the store does not hold, so the store call alone cannot evict a row the view already holds. Search results now drop the row too, so a delete from the search list disappears immediately. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017bss5MgZa7Jvj2m9zdaUd1 --- InterlinedList/Services/AppDataStore.swift | 14 ++ InterlinedList/Views/FeedView.swift | 5 + .../ServiceTests/AppDataStoreTests.swift | 214 +++++++++++++++++- 3 files changed, 231 insertions(+), 2 deletions(-) diff --git a/InterlinedList/Services/AppDataStore.swift b/InterlinedList/Services/AppDataStore.swift index 22578ba..9baab16 100644 --- a/InterlinedList/Services/AppDataStore.swift +++ b/InterlinedList/Services/AppDataStore.swift @@ -501,6 +501,20 @@ final class AppDataStore: ObservableObject { saveFeedCache() } + /// Drops a deleted row from the feed page and its cache. Unknown id: no-op — + /// the page is a window on the timeline, so deleting a message that has already + /// scrolled out of it is not an error. + /// + /// Removes the id and nothing else: the feed page is reply-free by construction + /// (`GET /api/messages` filters `parentId: null`), so a deleted parent has no + /// replies here to cascade to. `removeAll` rather than a single index because + /// `insertFeedMessage` does not deduplicate. + func removeFeedMessage(id: String) { + guard feedMessages.contains(where: { $0.id == id }) else { return } + feedMessages.removeAll { $0.id == id } + saveFeedCache() + } + func removeList(id: String) { userLists.removeAll { $0.id == id }; saveListsCache() } /// Idempotent upsert by id — safe to call after `createDocumentOffline` diff --git a/InterlinedList/Views/FeedView.swift b/InterlinedList/Views/FeedView.swift index 128aadc..f2f1c46 100644 --- a/InterlinedList/Views/FeedView.swift +++ b/InterlinedList/Views/FeedView.swift @@ -536,7 +536,12 @@ struct FeedView: View { do { try await APIClient.shared.deleteMessage(id: message.id) messageToDelete = nil + // The local removals are load-bearing, not just faster: `FeedMerge` keeps + // rows the store's page does not hold (that is how pagination survives), + // so the store call below cannot evict a row this view already holds. messages.removeAll { $0.id == message.id } + searchResults.removeAll { $0.id == message.id } + store.removeFeedMessage(id: message.id) } catch APIError.status(401) { authState.handleUnauthorized() messageToDelete = nil diff --git a/InterlinedListTests/ServiceTests/AppDataStoreTests.swift b/InterlinedListTests/ServiceTests/AppDataStoreTests.swift index 305be6c..211ba74 100644 --- a/InterlinedListTests/ServiceTests/AppDataStoreTests.swift +++ b/InterlinedListTests/ServiceTests/AppDataStoreTests.swift @@ -6,11 +6,23 @@ import XCTest final class AppDataStoreTests: XCTestCase { var sut: AppDataStore! + /// Ids handed out by `makeCachedUserId()`, cleared from the on-disk cache in + /// `tearDown` so a run leaves no feed JSON behind in the simulator. + private var seededUserIds: [String] = [] + override func setUp() { super.setUp() sut = AppDataStore() } + override func tearDown() async throws { + let uids = seededUserIds + seededUserIds = [] + let cache = DataCache() + for uid in uids { await cache.clearAll(prefix: uid) } + try await super.tearDown() + } + // MARK: - insertFeedMessage func test_insertFeedMessage_insertsAtHead() { @@ -112,6 +124,163 @@ final class AppDataStoreTests: XCTestCase { XCTAssertEqual(sut.feedMessages.first?.content, "kept") } + // MARK: - removeFeedMessage + + func test_removeFeedMessage_dropsTheRow() { + sut.insertFeedMessage(makeMessage(id: "a")) + sut.removeFeedMessage(id: "a") + XCTAssertTrue(sut.feedMessages.isEmpty) + } + + func test_removeFeedMessage_keepsTheOtherRowsInOrder() { + sut.insertFeedMessage(makeMessage(id: "a")) + sut.insertFeedMessage(makeMessage(id: "b")) + sut.insertFeedMessage(makeMessage(id: "c")) + sut.removeFeedMessage(id: "b") + XCTAssertEqual(sut.feedMessages.map(\.id), ["c", "a"]) + } + + func test_removeFeedMessage_unknownId_isANoOp() { + sut.insertFeedMessage(makeMessage(id: "a")) + sut.removeFeedMessage(id: "ghost") + XCTAssertEqual(sut.feedMessages.map(\.id), ["a"]) + } + + func test_removeFeedMessage_unknownId_doesNotMoveTheRevision() { + sut.insertFeedMessage(makeMessage(id: "a")) + let before = sut.feedRevision + sut.removeFeedMessage(id: "ghost") + XCTAssertEqual(sut.feedRevision, before) + } + + func test_removeFeedMessage_movesTheRevision() { + sut.insertFeedMessage(makeMessage(id: "a")) + let before = sut.feedRevision + sut.removeFeedMessage(id: "a") + XCTAssertGreaterThan(sut.feedRevision, before) + } + + /// `insertFeedMessage` deliberately does not deduplicate (see + /// `test_insertFeedMessage_doesNotDuplicateExistingMessage`), so removing a single + /// index would leave a copy of the deleted post behind. + func test_removeFeedMessage_removesEveryCopyOfADuplicatedId() { + sut.insertFeedMessage(makeMessage(id: "dup")) + sut.insertFeedMessage(makeMessage(id: "dup")) + sut.removeFeedMessage(id: "dup") + XCTAssertTrue(sut.feedMessages.isEmpty) + } + + // MARK: - removeFeedMessage: the persisted cache + + func test_removeFeedMessage_dropsTheRowFromThePersistedCache() async { + let uid = makeCachedUserId() + sut.onUserIdAvailable(uid) + sut.insertFeedMessage(makeMessage(id: "a")) + sut.insertFeedMessage(makeMessage(id: "b")) + let seeded = await cachedFeedIds(forUserId: uid, awaiting: ["b", "a"]) + XCTAssertEqual(seeded, ["b", "a"], "cache was not seeded; the assertion below would be vacuous") + + sut.removeFeedMessage(id: "a") + + let after = await cachedFeedIds(forUserId: uid, awaiting: ["b"]) + XCTAssertEqual(after, ["b"]) + } + + /// The reported bug, end to end: a deleted post must not come back on the next + /// launch that reads the cache. The relaunched store hydrates from disk only — + /// `onUserIdAvailable` touches no network. + func test_removedMessage_staysDeletedAcrossARelaunch() async { + let uid = makeCachedUserId() + sut.onUserIdAvailable(uid) + sut.insertFeedMessage(makeMessage(id: "keep")) + sut.insertFeedMessage(makeMessage(id: "gone")) + let seeded = await cachedFeedIds(forUserId: uid, awaiting: ["gone", "keep"]) + XCTAssertEqual(seeded, ["gone", "keep"], "cache was not seeded") + + sut.removeFeedMessage(id: "gone") + _ = await cachedFeedIds(forUserId: uid, awaiting: ["keep"]) + + let relaunched = AppDataStore() + relaunched.onUserIdAvailable(uid) + let ids = await feedIds(of: relaunched, awaiting: ["keep"]) + XCTAssertEqual(ids, ["keep"]) + } + + // MARK: - removeFeedMessage: replies + + /// Deliberate: no cascade. The feed page cannot hold a reply — `GET /api/messages` + /// filters `parentId: null`, and `ComposeView` skips `insertFeedMessage` when it is + /// replying — so a parent's replies are never in this cache to remove. The backend + /// cascades (`onDelete: Cascade`), which settles the server side. This pins the + /// decision: only the named id goes. + func test_removeFeedMessage_removesOnlyTheNamedId_leavingAReplyRow() { + sut.insertFeedMessage(makeMessage(id: "parent")) + sut.insertFeedMessage(makeMessage(id: "reply", parentId: "parent")) + sut.removeFeedMessage(id: "parent") + XCTAssertEqual(sut.feedMessages.map(\.id), ["reply"]) + } + + /// An orphaned reply is an ordinary row: no view branches on `parentId`, so a + /// dangling parent id renders as a standalone post rather than breaking. + func test_removeFeedMessage_orphanedReply_keepsItsFieldsIntact() { + sut.insertFeedMessage(makeMessage(id: "parent")) + sut.insertFeedMessage(makeMessage(id: "reply", content: "a reply", parentId: "parent")) + sut.removeFeedMessage(id: "parent") + let orphan = sut.feedMessages.first + XCTAssertEqual(orphan?.id, "reply") + XCTAssertEqual(orphan?.content, "a reply") + XCTAssertEqual(orphan?.parentId, "parent") + } + + /// The orphan has to survive the cache round-trip too: `[Message]` decodes all or + /// nothing, so a row that failed to decode would take the whole feed cache with it. + func test_removeFeedMessage_orphanedReply_survivesTheCacheRoundTrip() async { + let uid = makeCachedUserId() + sut.onUserIdAvailable(uid) + sut.insertFeedMessage(makeMessage(id: "parent")) + sut.insertFeedMessage(makeMessage(id: "reply", parentId: "parent")) + let seeded = await cachedFeedIds(forUserId: uid, awaiting: ["reply", "parent"]) + XCTAssertEqual(seeded, ["reply", "parent"], "cache was not seeded") + + sut.removeFeedMessage(id: "parent") + _ = await cachedFeedIds(forUserId: uid, awaiting: ["reply"]) + + let relaunched = AppDataStore() + relaunched.onUserIdAvailable(uid) + let ids = await feedIds(of: relaunched, awaiting: ["reply"]) + XCTAssertEqual(ids, ["reply"]) + XCTAssertEqual(relaunched.feedMessages.first?.parentId, "parent") + } + + // MARK: - removeFeedMessage: the view's working copy + + /// The optimistic half. `FeedView.deleteMessage` drops the row from its own copy + /// and then tells the store; a merge of the two afterwards must leave it gone. + func test_storeRowRemoval_withTheLocalRemoval_leavesTheRowGone() { + sut.insertFeedMessage(makeMessage(id: "a")) + sut.insertFeedMessage(makeMessage(id: "b")) + var viewCopy = sut.feedMessages + + sut.removeFeedMessage(id: "a") + viewCopy.removeAll { $0.id == "a" } + + let merged = FeedMerge.merge(existing: viewCopy, incoming: sut.feedMessages) + XCTAssertEqual(merged.messages.map(\.id), ["b"]) + } + + /// Why that local removal is load-bearing rather than merely faster: `FeedMerge` + /// keeps rows the store does not hold, which is how paginated pages survive a + /// merge. The store call alone cannot evict a row the view already holds. + func test_storeRowRemoval_withoutTheLocalRemoval_keepsTheRowOnScreen() { + sut.insertFeedMessage(makeMessage(id: "a")) + let viewCopy = sut.feedMessages + + sut.removeFeedMessage(id: "a") + + let merged = FeedMerge.merge(existing: viewCopy, incoming: sut.feedMessages) + XCTAssertEqual(merged.messages.map(\.id), ["a"]) + } + // MARK: - feedRevision /// The whole point of the revision: `Message` compares by id, so neither the @@ -370,14 +539,55 @@ final class AppDataStoreTests: XCTestCase { // MARK: - Helpers - private func makeMessage(id: String, content: String = "test") -> Message { + private func makeMessage(id: String, content: String = "test", parentId: String? = nil) -> Message { Message(id: id, content: content, publiclyVisible: true, userId: "u1", createdAt: "2026-01-01T00:00:00Z", updatedAt: nil, user: nil, imageUrls: nil, videoUrls: nil, - linkMetadata: nil, parentId: nil, scheduledAt: nil, + linkMetadata: nil, parentId: parentId, scheduledAt: nil, tags: nil, digCount: 0, dugByMe: false, crossPostUrls: nil) } + /// A user id no other test (or earlier run) has cached under, so the feed-cache + /// assertions read only what the test itself wrote. + private func makeCachedUserId() -> String { + let uid = "test-user-\(UUID().uuidString)" + seededUserIds.append(uid) + return uid + } + + /// `saveFeedCache` writes through a detached `Task` into an actor, so the file is + /// not on disk when the mutating call returns. Polls for the expected ids instead + /// of sleeping a fixed interval, and returns whatever it last read so a failure + /// reports the actual cached state. + private func cachedFeedIds(forUserId uid: String, + awaiting expected: [String], + timeout: TimeInterval = 3.0) async -> [String]? { + let cache = DataCache() + let deadline = Date().addingTimeInterval(timeout) + var last: [String]? + repeat { + let loaded: [Message]? = await cache.load(key: "\(uid)_feed") + last = loaded?.map(\.id) + if last == expected { return last } + try? await Task.sleep(nanoseconds: 20_000_000) + } while Date() < deadline + return last + } + + /// `onUserIdAvailable` hydrates from the cache in a detached `Task` — same polling + /// reason as `cachedFeedIds`. + private func feedIds(of store: AppDataStore, + awaiting expected: [String], + timeout: TimeInterval = 3.0) async -> [String] { + let deadline = Date().addingTimeInterval(timeout) + repeat { + let ids = store.feedMessages.map(\.id) + if ids == expected { return ids } + try? await Task.sleep(nanoseconds: 20_000_000) + } while Date() < deadline + return store.feedMessages.map(\.id) + } + private func makeDocument(id: String) -> Document { Document(id: id, title: "Doc \(id)", content: nil, folderId: nil, isPublic: false,