From 5c255622f853ed64501e4282247830335584b568 Mon Sep 17 00:00:00 2001 From: Nolan Caudill Date: Thu, 26 Feb 2026 16:18:28 -0800 Subject: [PATCH] Fix FreshRSS Reader API item discovery window based on published-date ot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reader API sync for the FreshRSS/Miniflux path used lastArticleFetchStartTime to compute the `ot` parameter for `/stream/items/ids`. On these servers, `ot` is applied to entry published date (not read/unread state-change time). This can drop entries from discovery when: 1. an entry is published before desktop’s last fetch timestamp, 2. the entry is read on phone before desktop ingests it, 3. desktop sync then queries with `ot=lastArticleFetchStartTime`. Result: the item may never be fetched locally and can appear to “disappear” until manually toggled unread server-side. Repro: - FreshRSS/Miniflux Reader API account. - Read an unread article on phone first. - Open/sync desktop NetNewsWire. - Article is missing unless marked unread again on the server. Fix: - For ReaderAPI variant `.freshRSS`, stop using lastArticleFetchStartTime for account-level item ID discovery. - Use a stable rolling baseline window (now - 3 months) for `ot` instead. - Keep existing last-fetch behavior for non-FreshRSS Reader API variants. Tests: - Add ReaderAPICaller tests asserting: - FreshRSS uses baseline window for `ot`. - Generic Reader API continues using lastArticleFetchStartTime. --- .../Account/ReaderAPI/ReaderAPICaller.swift | 23 +++- .../ReaderAPI/ReaderAPICallerTests.swift | 120 ++++++++++++++++++ 2 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 Modules/Account/Tests/AccountTests/ReaderAPI/ReaderAPICallerTests.swift diff --git a/Modules/Account/Sources/Account/ReaderAPI/ReaderAPICaller.swift b/Modules/Account/Sources/Account/ReaderAPI/ReaderAPICaller.swift index 3203cfc17c..7ff1d3edf0 100644 --- a/Modules/Account/Sources/Account/ReaderAPI/ReaderAPICaller.swift +++ b/Modules/Account/Sources/Account/ReaderAPI/ReaderAPICaller.swift @@ -447,13 +447,7 @@ enum CreateReaderAPISubscriptionResult { switch type { case .allForAccount: - let since: Date = { - if let lastArticleFetch = self.accountMetadata?.lastArticleFetchStartTime { - return lastArticleFetch - } else { - return Calendar.current.date(byAdding: .month, value: -3, to: Date()) ?? Date() - } - }() + let since = accountItemFetchSinceDate() let sinceTimeInterval = since.timeIntervalSince1970 queryItems.append(URLQueryItem(name: "ot", value: String(Int(sinceTimeInterval)))) @@ -576,6 +570,21 @@ enum CreateReaderAPISubscriptionResult { // MARK: Private private extension ReaderAPICaller { + func accountItemFetchSinceDate() -> Date { + let baselineWindowStart = Calendar.current.date(byAdding: .month, value: -3, to: Date()) ?? Date() + + // For FreshRSS-compatible servers (including Miniflux), using only "last successful fetch" + // can miss entries that were published earlier but had state changes (read/unread) later. + if variant == .freshRSS { + return baselineWindowStart + } + + if let lastArticleFetch = self.accountMetadata?.lastArticleFetchStartTime { + return lastArticleFetch + } + + return baselineWindowStart + } func encodeForURLPath(_ pathComponent: String?) -> String? { guard let pathComponent = pathComponent else { return nil } diff --git a/Modules/Account/Tests/AccountTests/ReaderAPI/ReaderAPICallerTests.swift b/Modules/Account/Tests/AccountTests/ReaderAPI/ReaderAPICallerTests.swift new file mode 100644 index 0000000000..56f7ed2adb --- /dev/null +++ b/Modules/Account/Tests/AccountTests/ReaderAPI/ReaderAPICallerTests.swift @@ -0,0 +1,120 @@ +// +// ReaderAPICallerTests.swift +// AccountTests +// + +import Foundation +import os +import RSWeb +import XCTest + +@testable import Account + +@MainActor final class ReaderAPICallerTests: XCTestCase { + + func testAllForAccountFreshRSSUsesBaselineWindowInsteadOfLastFetch() async throws { + let transport = ReaderAPISpyTransport() + let caller = ReaderAPICaller(transport: transport, logger: Logger(subsystem: "AccountTests", category: "ReaderAPI")) + + let accountMetadata = AccountMetadata() + accountMetadata.endpointURL = URL(string: "https://example.com")! + let lastFetch = Date().addingTimeInterval(-60 * 60 * 24) // 1 day ago + accountMetadata.lastArticleFetchStartTime = lastFetch + + caller.variant = .freshRSS + caller.accountMetadata = accountMetadata + + _ = try await caller.retrieveItemIDs(type: .allForAccount) + + guard let callURL = transport.lastRequestedURL else { + return XCTFail("Expected retrieveItemIDs to issue a request.") + } + guard let ot = otFromURL(callURL) else { + return XCTFail("Expected an ot query parameter in request URL: \(callURL.absoluteString)") + } + + let expectedBaseline = Calendar.current.date(byAdding: .month, value: -3, to: Date())?.timeIntervalSince1970 ?? Date().timeIntervalSince1970 + XCTAssertLessThan(abs(ot - expectedBaseline), 120, "FreshRSS should use the rolling baseline window for ot.") + + XCTAssertGreaterThan(abs(ot - lastFetch.timeIntervalSince1970), 60 * 60 * 24 * 7, "FreshRSS should not use the recent last fetch timestamp for ot.") + } + + func testAllForAccountGenericUsesLastFetchTimestamp() async throws { + let transport = ReaderAPISpyTransport() + let caller = ReaderAPICaller(transport: transport, logger: Logger(subsystem: "AccountTests", category: "ReaderAPI")) + + let accountMetadata = AccountMetadata() + accountMetadata.endpointURL = URL(string: "https://example.com")! + let lastFetch = Date().addingTimeInterval(-60 * 60 * 24) + accountMetadata.lastArticleFetchStartTime = lastFetch + + caller.variant = .generic + caller.accountMetadata = accountMetadata + + _ = try await caller.retrieveItemIDs(type: .allForAccount) + + guard let callURL = transport.lastRequestedURL else { + return XCTFail("Expected retrieveItemIDs to issue a request.") + } + guard let ot = otFromURL(callURL) else { + return XCTFail("Expected an ot query parameter in request URL: \(callURL.absoluteString)") + } + + XCTAssertLessThan(abs(ot - lastFetch.timeIntervalSince1970), 5, "Generic Reader API should continue to use lastArticleFetchStartTime for ot.") + } + + private func otFromURL(_ url: URL) -> TimeInterval? { + guard + let components = URLComponents(url: url, resolvingAgainstBaseURL: false), + let otValue = components.queryItems?.first(where: { $0.name == "ot" })?.value, + let ot = TimeInterval(otValue) + else { + return nil + } + return ot + } +} + +private final class ReaderAPISpyTransport: Transport, @unchecked Sendable { + nonisolated(unsafe) private(set) var lastRequestedURL: URL? + + func cancelAll() { } + + @discardableResult + func send(request: URLRequest) async throws -> (HTTPURLResponse, Data?) { + guard let url = request.url else { + throw TransportError.noURL + } + lastRequestedURL = url + let response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: "HTTP/1.1", headerFields: nil)! + let payload = Data(#"{"itemRefs":[{"id":"1"}]}"#.utf8) + return (response, payload) + } + + func send(request: URLRequest, completion: @escaping @Sendable (Result<(HTTPURLResponse, Data?), Error>) -> Void) { + Task { + do { + let result = try await send(request: request) + completion(.success(result)) + } catch { + completion(.failure(error)) + } + } + } + + func send(request: URLRequest, method: String) async throws { + fatalError("Unimplemented for test.") + } + + func send(request: URLRequest, method: String, completion: @escaping @Sendable (Result) -> Void) { + fatalError("Unimplemented for test.") + } + + func send(request: URLRequest, method: String, payload: Data) async throws -> (HTTPURLResponse, Data?) { + fatalError("Unimplemented for test.") + } + + func send(request: URLRequest, method: String, payload: Data, completion: @escaping @Sendable (Result<(HTTPURLResponse, Data?), Error>) -> Void) { + fatalError("Unimplemented for test.") + } +}