Skip to content

Commit 6c0dc20

Browse files
committed
fix: preserve existing BatchContext fields in ComponentPreFetcher
Previously, ComponentPreFetcher created a new BatchContext with only the components field, discarding existing versions, granularCache, and nodes. This caused issues when pre-fetching components inside batch mode where these fields were already populated. Now the prefetcher merges the new components with all existing context fields, ensuring batch orchestration state is preserved.
1 parent ec485a6 commit 6c0dc20

5 files changed

Lines changed: 238 additions & 7 deletions

File tree

Sources/ExFig/Batch/FileVersionPreFetcher.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// swiftlint:disable file_length
22
import FigmaAPI
33
import Foundation
4+
import Logging
45

56
/// Configuration for pre-fetch operation.
67
struct PreFetchConfiguration {
@@ -49,6 +50,8 @@ struct FileVersionPreFetcher: Sendable {
4950
let client: Client
5051
let ui: TerminalUI
5152

53+
private let logger = Logger(label: "com.alexey1312.exfig.file-version-prefetcher")
54+
5255
// MARK: - Static Factory
5356

5457
/// Pre-fetches file versions for all unique file IDs if cache is enabled.
@@ -316,14 +319,13 @@ struct FileVersionPreFetcher: Sendable {
316319
private func fetchAllMetadata(fileIds: [String]) async throws -> PreFetchedFileVersions {
317320
try await withThrowingTaskGroup(of: (String, FileMetadata?).self) { group in
318321
for fileId in fileIds {
319-
group.addTask { [client] in
322+
group.addTask { [client, logger] in
320323
do {
321324
let endpoint = FileMetadataEndpoint(fileId: fileId)
322325
let metadata = try await client.request(endpoint)
323326
return (fileId, metadata)
324327
} catch {
325-
// Individual file fetch failed, return nil
326-
// Will be handled as partial failure
328+
logger.warning("Pre-fetch metadata failed for file \(fileId): \(error.localizedDescription)")
327329
return (fileId, nil)
328330
}
329331
}
@@ -425,13 +427,13 @@ struct FileVersionPreFetcher: Sendable {
425427
private func fetchAllComponents(fileIds: [String]) async throws -> PreFetchedComponents {
426428
try await withThrowingTaskGroup(of: (String, [Component]?).self) { group in
427429
for fileId in fileIds {
428-
group.addTask { [client] in
430+
group.addTask { [client, logger] in
429431
do {
430432
let endpoint = ComponentsEndpoint(fileId: fileId)
431433
let components = try await client.request(endpoint)
432434
return (fileId, components)
433435
} catch {
434-
// Individual file fetch failed, return nil
436+
logger.warning("Pre-fetch components failed for file \(fileId): \(error.localizedDescription)")
435437
return (fileId, nil)
436438
}
437439
}

Sources/ExFig/Cache/ImageTrackingManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct FileVersionInfo: Sendable {
2828
///
2929
/// ## Batch Mode
3030
///
31-
/// When `batchMode` is true (detected via `BatchContextStorage.context?.granularCache`),
31+
/// When `batchMode` is true (detected via `BatchContextStorage.context?.isBatchMode`),
3232
/// the manager uses shared cache instead of loading from disk, and defers
3333
/// cache updates to the batch orchestrator.
3434
final class ImageTrackingManager: @unchecked Sendable {

Sources/ExFig/Shared/ComponentPreFetcher.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,13 @@ enum ComponentPreFetcher {
3030
}
3131

3232
let preFetched = PreFetchedComponents(components: componentsMap)
33-
let localContext = BatchContext(components: preFetched)
33+
let existingContext = BatchContextStorage.context
34+
let localContext = BatchContext(
35+
versions: existingContext?.versions,
36+
components: preFetched,
37+
granularCache: existingContext?.granularCache,
38+
nodes: existingContext?.nodes
39+
)
3440

3541
return try await BatchContextStorage.$context.withValue(localContext) {
3642
try await process()

Tests/ExFigTests/Batch/SharedGranularCacheTests.swift

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
@testable import ExFig
2+
@testable import FigmaAPI
23
import XCTest
34

45
final class SharedGranularCacheTests: XCTestCase {
@@ -233,4 +234,40 @@ final class BatchContextStorageTests: XCTestCase {
233234
XCTAssertFalse(BatchContextStorage.context?.isBatchMode ?? true)
234235
}
235236
}
237+
238+
func testBatchContextWithAllFieldsPopulated() {
239+
// Create test data for all four fields
240+
let versions = PreFetchedFileVersions(versions: [:])
241+
let components = PreFetchedComponents(components: [:])
242+
let nodes = PreFetchedNodes(nodes: [:])
243+
244+
var cache = ImageTrackingCache()
245+
cache.updateFileVersion(fileId: "fileA", version: "v1")
246+
let cachePath = tempDirectory.appendingPathComponent("test-cache.json")
247+
let granularCache = SharedGranularCache(cache: cache, cachePath: cachePath)
248+
249+
// Create context with all fields
250+
let context = BatchContext(
251+
versions: versions,
252+
components: components,
253+
granularCache: granularCache,
254+
nodes: nodes
255+
)
256+
257+
BatchContextStorage.$context.withValue(context) {
258+
let ctx = BatchContextStorage.context
259+
260+
// Verify all fields are accessible
261+
XCTAssertNotNil(ctx?.versions)
262+
XCTAssertNotNil(ctx?.components)
263+
XCTAssertNotNil(ctx?.granularCache)
264+
XCTAssertNotNil(ctx?.nodes)
265+
266+
// Verify isBatchMode is true
267+
XCTAssertTrue(ctx?.isBatchMode ?? false)
268+
269+
// Verify hasGranularCache is true
270+
XCTAssertTrue(ctx?.hasGranularCache ?? false)
271+
}
272+
}
236273
}
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
@testable import ExFig
2+
@testable import FigmaAPI
3+
import XCTest
4+
5+
final class ComponentPreFetcherTests: XCTestCase {
6+
var tempDirectory: URL!
7+
8+
override func setUp() {
9+
super.setUp()
10+
tempDirectory = FileManager.default.temporaryDirectory
11+
.appendingPathComponent(UUID().uuidString)
12+
try? FileManager.default.createDirectory(at: tempDirectory, withIntermediateDirectories: true)
13+
}
14+
15+
override func tearDown() {
16+
try? FileManager.default.removeItem(at: tempDirectory)
17+
super.tearDown()
18+
}
19+
20+
// MARK: - Context Preservation Tests
21+
22+
func testPreFetchOutsideBatchModeCreatesLocalContext() async throws {
23+
// Given: No existing batch context
24+
XCTAssertNil(BatchContextStorage.context)
25+
26+
let client = MockClient()
27+
let params = Params.make(lightFileId: "file123")
28+
29+
// Mock components response
30+
let mockComponents = [
31+
Component.make(nodeId: "1:1", name: "icon_test", frameName: "Icons"),
32+
]
33+
client.setResponse(mockComponents, for: ComponentsEndpoint.self)
34+
35+
// When: Pre-fetching components outside batch mode
36+
var capturedContext: BatchContext?
37+
_ = try await ComponentPreFetcher.withPreFetchedComponentsIfNeeded(
38+
client: client,
39+
params: params
40+
) {
41+
capturedContext = BatchContextStorage.context
42+
return "result"
43+
}
44+
45+
// Then: Local context was created with components only
46+
XCTAssertNotNil(capturedContext)
47+
XCTAssertNotNil(capturedContext?.components)
48+
XCTAssertNil(capturedContext?.versions)
49+
XCTAssertNil(capturedContext?.granularCache)
50+
XCTAssertNil(capturedContext?.nodes)
51+
52+
// And: Context is no longer available after closure
53+
XCTAssertNil(BatchContextStorage.context)
54+
}
55+
56+
func testPreFetchPreservesExistingVersionsInBatchMode() async throws {
57+
// Given: Existing batch context with versions
58+
let existingVersions = PreFetchedFileVersions(versions: ["fileA": makeMetadata(version: "v1")])
59+
let existingContext = BatchContext(versions: existingVersions)
60+
61+
let client = MockClient()
62+
let params = Params.make(lightFileId: "file123")
63+
64+
// Mock components response
65+
let mockComponents = [
66+
Component.make(nodeId: "1:1", name: "icon_test", frameName: "Icons"),
67+
]
68+
client.setResponse(mockComponents, for: ComponentsEndpoint.self)
69+
70+
// When: Pre-fetching components inside batch mode with existing versions
71+
var capturedContext: BatchContext?
72+
await BatchContextStorage.$context.withValue(existingContext) {
73+
do {
74+
_ = try await ComponentPreFetcher.withPreFetchedComponentsIfNeeded(
75+
client: client,
76+
params: params
77+
) {
78+
capturedContext = BatchContextStorage.context
79+
return "result"
80+
}
81+
} catch {
82+
XCTFail("Unexpected error: \(error)")
83+
}
84+
}
85+
86+
// Then: Both components and versions are available
87+
XCTAssertNotNil(capturedContext?.components)
88+
XCTAssertNotNil(capturedContext?.versions)
89+
XCTAssertEqual(capturedContext?.versions?.metadata(for: "fileA")?.version, "v1")
90+
}
91+
92+
func testPreFetchPreservesAllExistingContextFields() async throws {
93+
// Given: Existing batch context with all fields populated
94+
let existingVersions = PreFetchedFileVersions(versions: ["fileA": makeMetadata(version: "v1")])
95+
let existingNodes = PreFetchedNodes(nodes: ["fileA": [:]])
96+
97+
var cache = ImageTrackingCache()
98+
cache.updateFileVersion(fileId: "fileA", version: "v1")
99+
let cachePath = tempDirectory.appendingPathComponent("test-cache.json")
100+
let existingGranularCache = SharedGranularCache(cache: cache, cachePath: cachePath)
101+
102+
let existingContext = BatchContext(
103+
versions: existingVersions,
104+
components: nil, // No components yet
105+
granularCache: existingGranularCache,
106+
nodes: existingNodes
107+
)
108+
109+
let client = MockClient()
110+
let params = Params.make(lightFileId: "file123")
111+
112+
// Mock components response
113+
let mockComponents = [
114+
Component.make(nodeId: "1:1", name: "icon_test", frameName: "Icons"),
115+
]
116+
client.setResponse(mockComponents, for: ComponentsEndpoint.self)
117+
118+
// When: Pre-fetching components inside batch mode with all context fields
119+
var capturedContext: BatchContext?
120+
await BatchContextStorage.$context.withValue(existingContext) {
121+
do {
122+
_ = try await ComponentPreFetcher.withPreFetchedComponentsIfNeeded(
123+
client: client,
124+
params: params
125+
) {
126+
capturedContext = BatchContextStorage.context
127+
return "result"
128+
}
129+
} catch {
130+
XCTFail("Unexpected error: \(error)")
131+
}
132+
}
133+
134+
// Then: All context fields are preserved
135+
XCTAssertNotNil(capturedContext?.versions, "versions should be preserved")
136+
XCTAssertNotNil(capturedContext?.components, "components should be added")
137+
XCTAssertNotNil(capturedContext?.granularCache, "granularCache should be preserved")
138+
XCTAssertNotNil(capturedContext?.nodes, "nodes should be preserved")
139+
}
140+
141+
func testPreFetchSkipsWhenComponentsAlreadyAvailable() async throws {
142+
// Given: Existing batch context already has components
143+
let existingComponents = PreFetchedComponents(components: ["fileA": [
144+
Component.make(nodeId: "1:1", name: "existing_icon", frameName: "Icons"),
145+
]])
146+
let existingContext = BatchContext(components: existingComponents)
147+
148+
let client = MockClient()
149+
let params = Params.make(lightFileId: "file123")
150+
151+
// When: Pre-fetching components when they're already available
152+
var capturedContext: BatchContext?
153+
await BatchContextStorage.$context.withValue(existingContext) {
154+
do {
155+
_ = try await ComponentPreFetcher.withPreFetchedComponentsIfNeeded(
156+
client: client,
157+
params: params
158+
) {
159+
capturedContext = BatchContextStorage.context
160+
return "result"
161+
}
162+
} catch {
163+
XCTFail("Unexpected error: \(error)")
164+
}
165+
}
166+
167+
// Then: Original components are preserved (no new fetch)
168+
XCTAssertEqual(client.requestCount, 0, "No API calls should be made")
169+
XCTAssertNotNil(capturedContext?.components)
170+
XCTAssertTrue(capturedContext?.components?.hasComponents(for: "fileA") ?? false)
171+
}
172+
173+
// MARK: - Helpers
174+
175+
private func makeMetadata(version: String) -> FileMetadata {
176+
let json = """
177+
{
178+
"version": "\(version)",
179+
"name": "Test File",
180+
"lastModified": "2024-01-01T00:00:00Z"
181+
}
182+
"""
183+
// swiftlint:disable:next force_try
184+
return try! JSONDecoder().decode(FileMetadata.self, from: Data(json.utf8))
185+
}
186+
}

0 commit comments

Comments
 (0)