fix(offline): pagination-limit bug in full sync, missing manual-mode badge, capture error
- performFullSync was requesting listCollections with limit: 250 —
Outline caps pagination at 100 and rejects anything over that
outright, which was the actual "Synced with 1 error" (now visible
in the UI as of the last fix: "Pagination limit is too large (max
100)"). Paginate collections in increments of 100 the same way
documents already were, continuing until a short page signals the
end — the protocol doesn't expose a total count to ask for up front,
so this is the only way to know when to stop. 2 new regression tests.
- OfflineBanner only ever reflected NetworkMonitor (a real dropped
connection) — turning on the manual "Offline Mode" toggle did
nothing to it, since that's a separate AppStorage flag the banner
never read. ContentView_macOS's sidebar now shows the banner for
either condition, with distinct copy for "you turned this on" vs
"the network's actually down".
- NetworkMonitor: the previous fix (weak self only on the inner Task)
traded one Swift 6 error for another ("'weak' ownership of capture
'self' differs from implicitly-captured strong reference in outer
scope") since the inner closure's capture forced the outer one to
implicitly capture self too. Standard fix: weak capture on the outer
closure, guard-let into a strong local immediately, let the inner
Task closure capture that plain local instead.
This commit is contained in:
@@ -364,10 +364,23 @@ public actor CachingOutlineAPIClient: OutlineAPIClient {
|
||||
var documentsCount = 0
|
||||
var errors: [String] = []
|
||||
var collections: [OutlineCollection] = []
|
||||
do {
|
||||
collections = try await listCollections(offset: 0, limit: 250)
|
||||
} catch {
|
||||
errors.append(errorDescription(error))
|
||||
var collectionsOffset = 0
|
||||
let collectionsLimit = 100
|
||||
// Outline rejects any `limit` over 100 outright — page in increments
|
||||
// of that instead of guessing a total up front (the protocol doesn't
|
||||
// expose `pagination`'s total count, only the page itself); a page
|
||||
// shorter than the limit is what signals "that was the last one".
|
||||
while true {
|
||||
let page: [OutlineCollection]
|
||||
do {
|
||||
page = try await listCollections(offset: collectionsOffset, limit: collectionsLimit)
|
||||
} catch {
|
||||
errors.append(errorDescription(error))
|
||||
break
|
||||
}
|
||||
collections.append(contentsOf: page)
|
||||
guard page.count == collectionsLimit else { break }
|
||||
collectionsOffset += collectionsLimit
|
||||
}
|
||||
|
||||
for collection in collections {
|
||||
|
||||
@@ -12,6 +12,7 @@ private final class StubOutlineAPIClient: OutlineAPIClient, @unchecked Sendable
|
||||
var updateDocumentHandler: (@Sendable (UpdateDocumentRequest) async throws -> OutlineDocument)?
|
||||
var createPinHandler: (@Sendable (CreatePinRequest) async throws -> OutlinePin)?
|
||||
var deletePinHandler: (@Sendable (String) async throws -> Void)?
|
||||
var listDocumentsHandler: (@Sendable (String?, String?, Int, Int) async throws -> [OutlineDocument])?
|
||||
|
||||
func authInfo() async throws -> OutlineAuthInfo { throw NotStubbed() }
|
||||
|
||||
@@ -21,7 +22,8 @@ private final class StubOutlineAPIClient: OutlineAPIClient, @unchecked Sendable
|
||||
}
|
||||
|
||||
func listDocuments(collectionId: String?, parentDocumentId: String?, offset: Int, limit: Int) async throws -> [OutlineDocument] {
|
||||
throw NotStubbed()
|
||||
guard let handler = listDocumentsHandler else { throw NotStubbed() }
|
||||
return try await handler(collectionId, parentDocumentId, offset, limit)
|
||||
}
|
||||
|
||||
func documentsList(_ request: DocumentsListRequest) async throws -> [OutlineDocument] { throw NotStubbed() }
|
||||
@@ -339,4 +341,48 @@ final class CachingOutlineAPIClientTests: XCTestCase {
|
||||
let clearedSummary = await sut.cacheStorageSummary()
|
||||
XCTAssertEqual(clearedSummary.itemCount, 0)
|
||||
}
|
||||
|
||||
// MARK: - Full sync
|
||||
|
||||
func testPerformFullSyncNeverRequestsMoreThan100CollectionsPerPage() async throws {
|
||||
let stub = StubOutlineAPIClient()
|
||||
var requestedLimits: [Int] = []
|
||||
// 105 collections across two pages (100 + 5) — regression test for a
|
||||
// real bug: this used to ask for `limit: 250` in one shot, which
|
||||
// Outline's server rejects outright ("Pagination limit is too large
|
||||
// (max 100)"), turning the whole sync into a single silent failure.
|
||||
stub.listCollectionsHandler = { offset, limit in
|
||||
requestedLimits.append(limit)
|
||||
let remaining = max(0, 105 - offset)
|
||||
let count = min(limit, remaining)
|
||||
return (0..<count).map { self.makeCollection(id: "col-\(offset + $0)") }
|
||||
}
|
||||
stub.listDocumentsHandler = { _, _, _, _ in [] }
|
||||
let sut = CachingOutlineAPIClient(live: stub, cache: try makeCache())
|
||||
|
||||
let summary = await sut.performFullSync()
|
||||
|
||||
XCTAssertEqual(summary.collectionsCount, 105)
|
||||
XCTAssertTrue(summary.errors.isEmpty)
|
||||
XCTAssertTrue(requestedLimits.allSatisfy { $0 <= 100 }, "requested limits: \(requestedLimits)")
|
||||
}
|
||||
|
||||
func testPerformFullSyncCachesEachDocumentIndividually() async throws {
|
||||
let stub = StubOutlineAPIClient()
|
||||
stub.listCollectionsHandler = { offset, _ in offset == 0 ? [self.makeCollection(id: "col-1")] : [] }
|
||||
stub.listDocumentsHandler = { _, _, offset, _ in
|
||||
offset == 0 ? [self.makeDocument(id: "doc-1"), self.makeDocument(id: "doc-2")] : []
|
||||
}
|
||||
let cache = try makeCache()
|
||||
let sut = CachingOutlineAPIClient(live: stub, cache: cache)
|
||||
|
||||
let summary = await sut.performFullSync()
|
||||
XCTAssertEqual(summary.documentsCount, 2)
|
||||
|
||||
// A plain documentInfo read (no live call available) should now hit
|
||||
// the cache full sync populated, not just the list-shaped cache key.
|
||||
stub.documentInfoHandler = { _ in throw StubTransportError() }
|
||||
let cachedDoc = try await sut.documentInfo(id: "doc-2")
|
||||
XCTAssertEqual(cachedDoc.id, "doc-2")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import OutlineKit
|
||||
struct ContentView_macOS: View {
|
||||
@Environment(SessionStore.self) private var session
|
||||
@Environment(AppNavigation.self) private var navigation
|
||||
@AppStorage(CachingOutlineAPIClient.offlineModeDefaultsKey) private var isOfflineModeEnabled = false
|
||||
/// The landing state — no collection selected yet is what Home actually
|
||||
/// means, so this starts `true` rather than auto-selecting the first
|
||||
/// collection the way this used to work.
|
||||
@@ -50,8 +51,8 @@ struct ContentView_macOS: View {
|
||||
VStack(spacing: 0) {
|
||||
SidebarSearchField(text: $globalSearchQuery)
|
||||
Divider()
|
||||
if !session.networkMonitor.isOnline {
|
||||
OfflineBanner()
|
||||
if isOfflineModeEnabled || !session.networkMonitor.isOnline {
|
||||
OfflineBanner(isManual: isOfflineModeEnabled)
|
||||
Divider()
|
||||
}
|
||||
sidebar
|
||||
|
||||
@@ -1,15 +1,21 @@
|
||||
#if os(macOS)
|
||||
import SwiftUI
|
||||
|
||||
/// Shown at the top of the sidebar whenever `NetworkMonitor` reports no
|
||||
/// connection — cached collections/documents keep browsing working, but the
|
||||
/// user should know they might be looking at stale data.
|
||||
/// Shown at the top of the sidebar whenever the app is offline — either for
|
||||
/// real (`NetworkMonitor` reports no connection) or because the user turned
|
||||
/// on the manual "Offline Mode" toggle. Cached collections/documents keep
|
||||
/// browsing working either way, but the user should know they might be
|
||||
/// looking at stale data.
|
||||
struct OfflineBanner: View {
|
||||
/// Whether this is the user's own "Offline Mode" toggle rather than a
|
||||
/// real dropped connection — same banner slot, different explanation.
|
||||
var isManual: Bool = false
|
||||
|
||||
var body: some View {
|
||||
HStack(spacing: 8) {
|
||||
Image(systemName: "wifi.slash")
|
||||
.foregroundStyle(.orange)
|
||||
Text("Offline — showing cached content")
|
||||
Text(isManual ? "Offline Mode — showing cached content" : "Offline — showing cached content")
|
||||
.font(.callout)
|
||||
Spacer(minLength: 0)
|
||||
}
|
||||
|
||||
@@ -14,10 +14,17 @@ final class NetworkMonitor {
|
||||
private let queue = DispatchQueue(label: "com.outpost.network-monitor")
|
||||
|
||||
init() {
|
||||
monitor.pathUpdateHandler = { path in
|
||||
// Weak on the outer closure (it's held by `monitor` for the object's
|
||||
// whole lifetime, so a strong capture here would be a retain cycle),
|
||||
// then unwrapped once into a plain strong local — the inner `Task`
|
||||
// closure capturing that local `self` is what a nested closure needs
|
||||
// to be consistent about capture semantics with its enclosing one.
|
||||
monitor.pathUpdateHandler = { [weak self] path in
|
||||
guard let self else { return }
|
||||
|
||||
let online = path.status == .satisfied
|
||||
Task { @MainActor [weak self] in
|
||||
self?.isOnline = online
|
||||
Task { @MainActor in
|
||||
self.isOnline = online
|
||||
}
|
||||
}
|
||||
monitor.start(queue: queue)
|
||||
|
||||
Reference in New Issue
Block a user