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 documentsCount = 0
|
||||||
var errors: [String] = []
|
var errors: [String] = []
|
||||||
var collections: [OutlineCollection] = []
|
var collections: [OutlineCollection] = []
|
||||||
|
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 {
|
do {
|
||||||
collections = try await listCollections(offset: 0, limit: 250)
|
page = try await listCollections(offset: collectionsOffset, limit: collectionsLimit)
|
||||||
} catch {
|
} catch {
|
||||||
errors.append(errorDescription(error))
|
errors.append(errorDescription(error))
|
||||||
|
break
|
||||||
|
}
|
||||||
|
collections.append(contentsOf: page)
|
||||||
|
guard page.count == collectionsLimit else { break }
|
||||||
|
collectionsOffset += collectionsLimit
|
||||||
}
|
}
|
||||||
|
|
||||||
for collection in collections {
|
for collection in collections {
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ private final class StubOutlineAPIClient: OutlineAPIClient, @unchecked Sendable
|
|||||||
var updateDocumentHandler: (@Sendable (UpdateDocumentRequest) async throws -> OutlineDocument)?
|
var updateDocumentHandler: (@Sendable (UpdateDocumentRequest) async throws -> OutlineDocument)?
|
||||||
var createPinHandler: (@Sendable (CreatePinRequest) async throws -> OutlinePin)?
|
var createPinHandler: (@Sendable (CreatePinRequest) async throws -> OutlinePin)?
|
||||||
var deletePinHandler: (@Sendable (String) async throws -> Void)?
|
var deletePinHandler: (@Sendable (String) async throws -> Void)?
|
||||||
|
var listDocumentsHandler: (@Sendable (String?, String?, Int, Int) async throws -> [OutlineDocument])?
|
||||||
|
|
||||||
func authInfo() async throws -> OutlineAuthInfo { throw NotStubbed() }
|
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] {
|
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() }
|
func documentsList(_ request: DocumentsListRequest) async throws -> [OutlineDocument] { throw NotStubbed() }
|
||||||
@@ -339,4 +341,48 @@ final class CachingOutlineAPIClientTests: XCTestCase {
|
|||||||
let clearedSummary = await sut.cacheStorageSummary()
|
let clearedSummary = await sut.cacheStorageSummary()
|
||||||
XCTAssertEqual(clearedSummary.itemCount, 0)
|
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 {
|
struct ContentView_macOS: View {
|
||||||
@Environment(SessionStore.self) private var session
|
@Environment(SessionStore.self) private var session
|
||||||
@Environment(AppNavigation.self) private var navigation
|
@Environment(AppNavigation.self) private var navigation
|
||||||
|
@AppStorage(CachingOutlineAPIClient.offlineModeDefaultsKey) private var isOfflineModeEnabled = false
|
||||||
/// The landing state — no collection selected yet is what Home actually
|
/// The landing state — no collection selected yet is what Home actually
|
||||||
/// means, so this starts `true` rather than auto-selecting the first
|
/// means, so this starts `true` rather than auto-selecting the first
|
||||||
/// collection the way this used to work.
|
/// collection the way this used to work.
|
||||||
@@ -50,8 +51,8 @@ struct ContentView_macOS: View {
|
|||||||
VStack(spacing: 0) {
|
VStack(spacing: 0) {
|
||||||
SidebarSearchField(text: $globalSearchQuery)
|
SidebarSearchField(text: $globalSearchQuery)
|
||||||
Divider()
|
Divider()
|
||||||
if !session.networkMonitor.isOnline {
|
if isOfflineModeEnabled || !session.networkMonitor.isOnline {
|
||||||
OfflineBanner()
|
OfflineBanner(isManual: isOfflineModeEnabled)
|
||||||
Divider()
|
Divider()
|
||||||
}
|
}
|
||||||
sidebar
|
sidebar
|
||||||
|
|||||||
@@ -1,15 +1,21 @@
|
|||||||
#if os(macOS)
|
#if os(macOS)
|
||||||
import SwiftUI
|
import SwiftUI
|
||||||
|
|
||||||
/// Shown at the top of the sidebar whenever `NetworkMonitor` reports no
|
/// Shown at the top of the sidebar whenever the app is offline — either for
|
||||||
/// connection — cached collections/documents keep browsing working, but the
|
/// real (`NetworkMonitor` reports no connection) or because the user turned
|
||||||
/// user should know they might be looking at stale data.
|
/// 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 {
|
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 {
|
var body: some View {
|
||||||
HStack(spacing: 8) {
|
HStack(spacing: 8) {
|
||||||
Image(systemName: "wifi.slash")
|
Image(systemName: "wifi.slash")
|
||||||
.foregroundStyle(.orange)
|
.foregroundStyle(.orange)
|
||||||
Text("Offline — showing cached content")
|
Text(isManual ? "Offline Mode — showing cached content" : "Offline — showing cached content")
|
||||||
.font(.callout)
|
.font(.callout)
|
||||||
Spacer(minLength: 0)
|
Spacer(minLength: 0)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -14,10 +14,17 @@ final class NetworkMonitor {
|
|||||||
private let queue = DispatchQueue(label: "com.outpost.network-monitor")
|
private let queue = DispatchQueue(label: "com.outpost.network-monitor")
|
||||||
|
|
||||||
init() {
|
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
|
let online = path.status == .satisfied
|
||||||
Task { @MainActor [weak self] in
|
Task { @MainActor in
|
||||||
self?.isOnline = online
|
self.isOnline = online
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
monitor.start(queue: queue)
|
monitor.start(queue: queue)
|
||||||
|
|||||||
Reference in New Issue
Block a user