fix(outlinekit): Full Local Sync now recurses into nested documents
performFullSync only ever cached each collection's root-level documents — a document's own sub-documents were never cached at all, so nested content stayed unreachable offline even with a full sync (and even after implicitly re-fetching individual documents, since the walk that seeds the cache never visited them). Now recurses depth-first into every document's children via cacheDocumentTree, terminating naturally once a branch runs out of sub-documents. Also caches each collection individually under "collection:<id>" (was only ever cached as part of a paginated list blob), and adds OfflineCacheStore.loadAll(keyPrefix:) plus cachedDocumentsIndex()/cachedCollectionsIndex() on CachingOutlineAPIClient to read everything back as a flat local index — no per-id lookup needed, no network involved. Found and fixed a real infinite-recursion bug in the process: an existing test's stub ignored parentDocumentId and kept returning the same root documents at every recursion depth, hanging swift test indefinitely once the real code started recursing into children. Fixed the stub, added a dedicated regression test for the recursive behavior itself. 78/78 tests passing.
This commit is contained in:
@@ -433,6 +433,15 @@ public actor CachingOutlineAPIClient: OutlineAPIClient {
|
|||||||
/// existing cached-read methods already do the caching as a side effect,
|
/// existing cached-read methods already do the caching as a side effect,
|
||||||
/// this just has to drive the walk and separately cache each document by
|
/// this just has to drive the walk and separately cache each document by
|
||||||
/// id (`listDocuments`'s cache key is the list, not the individual doc).
|
/// id (`listDocuments`'s cache key is the list, not the individual doc).
|
||||||
|
///
|
||||||
|
/// Recurses into every document's children, not just collections' own
|
||||||
|
/// root-level documents — a document with sub-documents used to leave
|
||||||
|
/// them uncached entirely (only reachable if something else happened to
|
||||||
|
/// open them individually first). Also caches each collection under its
|
||||||
|
/// own `"collection:<id>"` key (previously only cached as part of the
|
||||||
|
/// paginated list blob), so both are individually enumerable afterward
|
||||||
|
/// via `OfflineCacheStore.loadAll(keyPrefix:)` — see
|
||||||
|
/// `cachedDocumentsIndex()`/`cachedCollectionsIndex()`.
|
||||||
public func performFullSync() async -> FullSyncSummary {
|
public func performFullSync() async -> FullSyncSummary {
|
||||||
var documentsCount = 0
|
var documentsCount = 0
|
||||||
var errors: [String] = []
|
var errors: [String] = []
|
||||||
@@ -457,26 +466,62 @@ public actor CachingOutlineAPIClient: OutlineAPIClient {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for collection in collections {
|
for collection in collections {
|
||||||
|
await cacheCollection(collection)
|
||||||
|
let result = await cacheDocumentTree(collectionId: collection.id, parentDocumentId: nil, collectionName: collection.name)
|
||||||
|
documentsCount += result.count
|
||||||
|
errors.append(contentsOf: result.errors)
|
||||||
|
}
|
||||||
|
|
||||||
|
return FullSyncSummary(collectionsCount: collections.count, documentsCount: documentsCount, errors: errors, finishedAt: Date())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Caches every document under `parentDocumentId` (`nil` = a
|
||||||
|
/// collection's root level) and recurses into each one's own children,
|
||||||
|
/// depth-first, until a branch runs out of sub-documents. Returns a
|
||||||
|
/// plain `(count, errors)` pair rather than mutating shared state across
|
||||||
|
/// `await` boundaries, since this calls itself recursively.
|
||||||
|
private func cacheDocumentTree(
|
||||||
|
collectionId: String,
|
||||||
|
parentDocumentId: String?,
|
||||||
|
collectionName: String
|
||||||
|
) async -> (count: Int, errors: [String]) {
|
||||||
|
var count = 0
|
||||||
|
var errors: [String] = []
|
||||||
var offset = 0
|
var offset = 0
|
||||||
let limit = 100
|
let limit = 100
|
||||||
while true {
|
while true {
|
||||||
let documents: [OutlineDocument]
|
let documents: [OutlineDocument]
|
||||||
do {
|
do {
|
||||||
documents = try await listDocuments(collectionId: collection.id, parentDocumentId: nil, offset: offset, limit: limit)
|
documents = try await listDocuments(collectionId: collectionId, parentDocumentId: parentDocumentId, offset: offset, limit: limit)
|
||||||
} catch {
|
} catch {
|
||||||
errors.append("\(collection.name): \(errorDescription(error))")
|
errors.append("\(collectionName): \(errorDescription(error))")
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
for document in documents {
|
for document in documents {
|
||||||
await cacheDocument(document)
|
await cacheDocument(document)
|
||||||
|
count += 1
|
||||||
|
let childResult = await cacheDocumentTree(collectionId: collectionId, parentDocumentId: document.id, collectionName: collectionName)
|
||||||
|
count += childResult.count
|
||||||
|
errors.append(contentsOf: childResult.errors)
|
||||||
}
|
}
|
||||||
documentsCount += documents.count
|
|
||||||
guard documents.count == limit else { break }
|
guard documents.count == limit else { break }
|
||||||
offset += limit
|
offset += limit
|
||||||
}
|
}
|
||||||
|
return (count, errors)
|
||||||
}
|
}
|
||||||
|
|
||||||
return FullSyncSummary(collectionsCount: collections.count, documentsCount: documentsCount, errors: errors, finishedAt: Date())
|
/// Every individually cached document from the last Full Local Sync —
|
||||||
|
/// empty if a sync has never run (or found nothing). Purely a local
|
||||||
|
/// SwiftData read, no network involved.
|
||||||
|
public func cachedDocumentsIndex() async -> [OutlineDocument] {
|
||||||
|
let payloads = await cache.loadAll(keyPrefix: "document:")
|
||||||
|
return payloads.compactMap { try? decoder.decode(OutlineDocument.self, from: $0) }
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Every individually cached collection from the last Full Local Sync.
|
||||||
|
public func cachedCollectionsIndex() async -> [OutlineCollection] {
|
||||||
|
let payloads = await cache.loadAll(keyPrefix: "collection:")
|
||||||
|
return payloads.compactMap { try? decoder.decode(OutlineCollection.self, from: $0) }
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Helpers
|
// MARK: - Helpers
|
||||||
|
|||||||
@@ -31,6 +31,15 @@ public actor OfflineCacheStore {
|
|||||||
return try? modelContext.fetch(descriptor).first?.payload
|
return try? modelContext.fetch(descriptor).first?.payload
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Everything cached under a key prefix — e.g. every individually
|
||||||
|
/// cached document (`"document:<id>"`) or collection
|
||||||
|
/// (`"collection:<id>"`) after a Full Local Sync, for building a local
|
||||||
|
/// search index without a per-item exact-key lookup.
|
||||||
|
public func loadAll(keyPrefix: String) -> [Data] {
|
||||||
|
let descriptor = FetchDescriptor<CachedPayload>(predicate: #Predicate { $0.key.starts(with: keyPrefix) })
|
||||||
|
return ((try? modelContext.fetch(descriptor)) ?? []).map(\.payload)
|
||||||
|
}
|
||||||
|
|
||||||
/// Used to drop a temporary `pending-*` document's cache entry once a
|
/// Used to drop a temporary `pending-*` document's cache entry once a
|
||||||
/// queued create syncs and the server hands back the real id — the
|
/// queued create syncs and the server hands back the real id — the
|
||||||
/// placeholder key would otherwise sit around as a dead orphan forever.
|
/// placeholder key would otherwise sit around as a dead orphan forever.
|
||||||
|
|||||||
@@ -421,8 +421,13 @@ final class CachingOutlineAPIClientTests: XCTestCase {
|
|||||||
func testPerformFullSyncCachesEachDocumentIndividually() async throws {
|
func testPerformFullSyncCachesEachDocumentIndividually() async throws {
|
||||||
let stub = StubOutlineAPIClient()
|
let stub = StubOutlineAPIClient()
|
||||||
stub.listCollectionsHandler = { offset, _ in offset == 0 ? [self.makeCollection(id: "col-1")] : [] }
|
stub.listCollectionsHandler = { offset, _ in offset == 0 ? [self.makeCollection(id: "col-1")] : [] }
|
||||||
stub.listDocumentsHandler = { _, _, offset, _ in
|
// Must return empty for any non-nil parentDocumentId (no children) —
|
||||||
offset == 0 ? [self.makeDocument(id: "doc-1"), self.makeDocument(id: "doc-2")] : []
|
// performFullSync now recurses into every document's own children,
|
||||||
|
// so a stub that ignores parentDocumentId and always returns the
|
||||||
|
// same root documents regardless would recurse into itself forever.
|
||||||
|
stub.listDocumentsHandler = { _, parentDocumentId, offset, _ in
|
||||||
|
guard parentDocumentId == nil else { return [] }
|
||||||
|
return offset == 0 ? [self.makeDocument(id: "doc-1"), self.makeDocument(id: "doc-2")] : []
|
||||||
}
|
}
|
||||||
let cache = try makeCache()
|
let cache = try makeCache()
|
||||||
let sut = CachingOutlineAPIClient(live: stub, cache: cache)
|
let sut = CachingOutlineAPIClient(live: stub, cache: cache)
|
||||||
@@ -437,6 +442,34 @@ final class CachingOutlineAPIClientTests: XCTestCase {
|
|||||||
XCTAssertEqual(cachedDoc.id, "doc-2")
|
XCTAssertEqual(cachedDoc.id, "doc-2")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func testPerformFullSyncRecursesIntoNestedDocuments() async throws {
|
||||||
|
let stub = StubOutlineAPIClient()
|
||||||
|
stub.listCollectionsHandler = { offset, _ in offset == 0 ? [self.makeCollection(id: "col-1")] : [] }
|
||||||
|
// doc-1 (root) -> doc-2 (child of doc-1) -> doc-3 (grandchild) —
|
||||||
|
// regression test for the real gap this fixed: only root-level
|
||||||
|
// documents were ever cached before, so a document's own
|
||||||
|
// sub-documents were never reachable offline at all unless
|
||||||
|
// something else happened to open them individually first.
|
||||||
|
stub.listDocumentsHandler = { _, parentDocumentId, offset, _ in
|
||||||
|
guard offset == 0 else { return [] }
|
||||||
|
switch parentDocumentId {
|
||||||
|
case nil: return [self.makeDocument(id: "doc-1")]
|
||||||
|
case "doc-1": return [self.makeDocument(id: "doc-2")]
|
||||||
|
case "doc-2": return [self.makeDocument(id: "doc-3")]
|
||||||
|
default: return []
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let cache = try makeCache()
|
||||||
|
let sut = CachingOutlineAPIClient(live: stub, cache: cache)
|
||||||
|
|
||||||
|
let summary = await sut.performFullSync()
|
||||||
|
|
||||||
|
XCTAssertEqual(summary.documentsCount, 3)
|
||||||
|
stub.documentInfoHandler = { _ in throw StubTransportError() }
|
||||||
|
let cachedGrandchild = try await sut.documentInfo(id: "doc-3")
|
||||||
|
XCTAssertEqual(cachedGrandchild.id, "doc-3")
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: - Offline document creation
|
// MARK: - Offline document creation
|
||||||
|
|
||||||
func testCreateDocumentQueuesAndReturnsUsableDocumentWhenOffline() async throws {
|
func testCreateDocumentQueuesAndReturnsUsableDocumentWhenOffline() async throws {
|
||||||
|
|||||||
Reference in New Issue
Block a user