From f941a1397f7c6ef4aeffc0ddea6d271cf5d07889 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 28 Sep 2026 13:21:27 -0600 Subject: [PATCH 1/9] fix(ios): let the cache policy refresh plugin and theme assets `EditorService`'s cache policy was documented to cover asset manifests, but `prepareAssetBundle` returned the newest bundle on disk before the policy was ever consulted. `.maxAge` and `.ignore` refreshed API responses and never assets, so the only way to pick up a plugin or theme change was `purge()`, which forces a cold load. The policy now decides when to check the site's manifest again. An unchanged manifest keeps the bundle already on disk and resets its age rather than downloading every asset again: asset URLs carry their version, so the same manifest means the same assets. A changed manifest builds the new bundle beside the old one, which `cleanup()` removes later. `.always`, the default and what WordPress-iOS uses, behaves as before. The check for an existing bundle runs inside the shared build for its directory, so marking a bundle current doesn't race a build of the same bundle replacing it. `fetchManifest()` no longer consults the policy: once the manifest is fetched, a bundle with the same checksum was built from exactly that manifest, so reusing its parsed copy is always right. `downloadAssetBundle(cachePolicy:progress:)` never used `cachePolicy`. It's now a deprecated overload of `downloadAssetBundle(progress:)`. --- docs/code/preloading.md | 28 +++ .../Sources/Model/EditorAssetBundle.swift | 6 +- .../Sources/Model/EditorCachePolicy.swift | 3 +- .../Sources/Services/EditorService.swift | 6 +- .../Sources/Stores/EditorAssetLibrary.swift | 78 +++++-- .../Services/EditorServiceTests.swift | 87 +++++++ .../Stores/EditorAssetLibraryTests.swift | 214 +++++++++--------- 7 files changed, 298 insertions(+), 124 deletions(-) diff --git a/docs/code/preloading.md b/docs/code/preloading.md index 7912f3365..cd014a798 100644 --- a/docs/code/preloading.md +++ b/docs/code/preloading.md @@ -84,6 +84,8 @@ The `EditorURLCache` provides disk-based caching for API responses, keyed by URL | `.maxAge(TimeInterval)` | Use cached responses younger than the specified age | | `.always` | Always use cached responses regardless of age | +The same policy decides when an `EditorService` checks for new plugin and theme assets; see [Refreshing](#refreshing). + Example: **Swift** @@ -245,6 +247,32 @@ try await service.purge() //tbd ``` +### Refreshing + +An `EditorService`'s cache policy covers plugin and theme assets as well as API responses. For assets, it decides when to check the site's asset manifest again: + +| Policy | API responses | Asset bundle | +| ----------------------- | ------------------------------- | ------------------------------------------------------ | +| `.always` (default) | Fetched only when not cached | Manifest checked only when no bundle is on disk | +| `.maxAge(TimeInterval)` | Fetched once older than the age | Manifest checked once the bundle is older than the age | +| `.ignore` | Always fetched | Manifest always checked | + +If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. If it has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. `cleanup()` removes the old one later. + +To refresh a site's editor data — on pull-to-refresh, for instance — prepare a separate service that ignores the cache, and give its dependencies to the next editor: + +**Swift** + +```swift +let dependencies = try await EditorService(configuration: configuration, cachePolicy: .ignore).prepare() +``` + +Nothing is deleted first, so an editor opened during the refresh still loads straight from what's on disk, and a refresh that fails leaves it all in place. An editor given no dependencies prepares its own with `.always`, so it uses whatever the last refresh left. To download assets again even when their manifest hasn't changed, `purge()` instead, at the cost of a cold load for the next editor. + +**Kotlin** + +Not yet: Android's `EditorService` still checks the asset manifest only when no bundle is on disk, whatever its cache policy. + ## Offline Mode When `EditorConfiguration.isOfflineModeEnabled` is `true`, the preloading system returns empty dependencies: diff --git a/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift b/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift index b92e06937..4525e4b6f 100644 --- a/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift +++ b/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift @@ -48,9 +48,11 @@ public struct EditorAssetBundle: Sendable, Equatable, Hashable { /// The manifest that defines which assets belong to this bundle. let manifest: LocalEditorAssetManifest - /// The date this bundle was created by downloading the manifest contents. + /// The date this bundle was created by downloading the manifest contents, or last found to still match the + /// site's manifest. /// - /// Used to determine which bundle is most recent when multiple bundles exist. + /// Used to determine which bundle is most recent when multiple bundles exist, and how old it is for the + /// cache policy. let downloadDate: Date /// The number of assets stored in this bundle. diff --git a/ios/Sources/GutenbergKit/Sources/Model/EditorCachePolicy.swift b/ios/Sources/GutenbergKit/Sources/Model/EditorCachePolicy.swift index 292498cd3..d46d7b78f 100644 --- a/ios/Sources/GutenbergKit/Sources/Model/EditorCachePolicy.swift +++ b/ios/Sources/GutenbergKit/Sources/Model/EditorCachePolicy.swift @@ -4,7 +4,8 @@ import Foundation /// /// `EditorCachePolicy` provides three caching strategies that control when cached /// HTTP responses are considered valid. This is used by `EditorURLCache` to decide -/// whether to return a cached response or require a fresh network request. +/// whether to return a cached response or require a fresh network request, and by +/// `EditorAssetLibrary` to decide when to check a site's asset manifest again. /// /// ## Usage /// diff --git a/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift b/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift index 0aee8e0fa..585b5859c 100644 --- a/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift +++ b/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift @@ -55,7 +55,9 @@ public actor EditorService { /// - cachePolicy: The policy that determines when cached responses are considered valid. /// Use `.ignore` to always fetch fresh data, `.maxAge(_:)` to expire entries after /// a time interval, or `.always` (the default) to use cached data regardless of age. - /// This policy applies to both API response caching and asset manifest caching. + /// This policy applies to both API responses and plugin and theme assets. For assets, it + /// decides when to check the site's asset manifest again; an unchanged manifest keeps the + /// bundle already on disk rather than downloading its assets again. public init( configuration: EditorConfiguration, httpClient: (any EditorHTTPClientProtocol)? = nil, @@ -233,7 +235,7 @@ public actor EditorService { } private func prepareAssetBundle() async throws -> EditorAssetBundle { - if let latestAssetBundle = try await self.assetLibrary.readAssetBundles().first { + if let latestAssetBundle = try await self.assetLibrary.readLatestAssetBundle() { await self.incrementProgress(for: .assetBundle) return latestAssetBundle } diff --git a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift index dae3364dd..17838479b 100644 --- a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift +++ b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift @@ -18,9 +18,10 @@ public actor EditorAssetLibrary { /// - Parameters: /// - configuration: The editor configuration containing site-specific settings. /// - httpClient: The HTTP client used to fetch remote assets. - /// - cachePolicy: The policy that determines when cached asset manifests are considered valid. - /// Use `.ignore` to always fetch fresh manifests, `.maxAge(_:)` to expire entries after - /// a time interval, or `.always` (the default) to use cached manifests regardless of age. + /// - cachePolicy: The policy that determines how long the newest bundle on disk is used + /// before the site's manifest is checked again. Use `.ignore` to check it every time, + /// `.maxAge(_:)` to check it once the bundle is older than a time interval, or `.always` + /// (the default) to check it only when there is no bundle on disk. /// - storageRoot: The root directory where asset bundles will be stored on disk. public init( configuration: EditorConfiguration, @@ -38,7 +39,8 @@ public actor EditorAssetLibrary { /// Retrieve the manifest for a given site configuration. /// - /// Applications should periodically check for a new editor manifest. This can be very expensive, so this method defaults to returning an existing one on-disk. + /// Parsing a manifest is expensive, so when a bundle built from the same manifest is already on disk, this + /// method returns that bundle's copy rather than parsing it again. /// func fetchManifest() async throws -> LocalEditorAssetManifest { guard configuration.shouldUsePlugins else { return .empty } @@ -47,14 +49,13 @@ public actor EditorAssetLibrary { ).0 let remoteManifest = try RemoteEditorAssetManifest(data: data) - guard - let existingManifest = self.existingBundle(forManifestChecksum: remoteManifest.checksum), - self.cachePolicy.allowsResponseWith(date: existingManifest.downloadDate) - else { - return try LocalEditorAssetManifest(remoteManifest: remoteManifest) + // The checksum covers the whole response, so a bundle with the same one was built from + // this exact manifest. + if let existingBundle = self.existingBundle(forManifestChecksum: remoteManifest.checksum) { + return existingBundle.manifest } - return existingManifest.manifest + return try LocalEditorAssetManifest(remoteManifest: remoteManifest) } // MARK: - Bundle Handling @@ -72,19 +73,46 @@ public actor EditorAssetLibrary { .sorted { $0.downloadDate > $1.downloadDate } } + /// The newest bundle on disk, if the cache policy still trusts it. + /// + /// Returns `nil` when there is no bundle, or when the newest one is too old for the policy. Either way, call + /// ``downloadAssetBundle(progress:)`` next to check the site's manifest. + func readLatestAssetBundle() throws -> EditorAssetBundle? { + guard + let latestBundle = try self.readAssetBundles().first, + self.cachePolicy.allowsResponseWith(date: latestBundle.downloadDate) + else { + return nil + } + + return latestBundle + } + /// Fetches the latest manifest from the server and downloads all of its resources, caching them on-disk. /// + /// If a bundle built from the same manifest is already on disk, it's returned instead, without downloading its + /// assets again: they're versioned by URL, so an unchanged manifest means unchanged assets. The bundle then + /// counts as newly downloaded, both for the cache policy and as the newest bundle on disk. To download every + /// asset again regardless, ``purge()`` the library first. + /// /// - Parameter progress: An optional callback that receives progress updates as assets are downloaded. /// - Returns: The downloaded `EditorAssetBundle` containing all cached assets. /// - Throws: An error if the manifest cannot be fetched or assets fail to download. public func downloadAssetBundle( - cachePolicy: EditorCachePolicy = .always, progress: EditorProgressCallback? = nil ) async throws -> EditorAssetBundle { let manifest = try await self.fetchManifest() return try await self.buildBundle(for: manifest, progress: progress) } + @available(*, deprecated, message: "`cachePolicy` has no effect; the library's own cache policy applies. Drop the argument.") + public func downloadAssetBundle( + cachePolicy: EditorCachePolicy, + progress: EditorProgressCallback? = nil + ) async throws -> EditorAssetBundle { + try await self.downloadAssetBundle(progress: progress) + } + /// Checks whether a complete bundle with the given manifest checksum exists on disk. /// /// A bundle is considered complete only if both `manifest.json` and `editor-representation.json` exist. @@ -110,7 +138,8 @@ public actor EditorAssetLibrary { /// Downloads all of the assets for a given manifest and assembles them into a bundle. /// /// Assets are downloaded concurrently and stored in a temporary directory. Once all downloads - /// complete successfully, the bundle is atomically moved to its final location. + /// complete successfully, the bundle is atomically moved to its final location. If a complete + /// bundle for the manifest is already there, it's marked current and returned instead. func buildBundle( for manifest: LocalEditorAssetManifest, progress: EditorProgressCallback? = nil @@ -126,7 +155,30 @@ public actor EditorAssetLibrary { // join a build in flight rather than race a second one into it. let destination = self.bundleRoot(for: manifest.checksum).standardizedFileURL return try await Self.inFlightBuilds.value(for: destination, progress: progress) { report in - try await self.build(manifest, reportingTo: report) + // Checked here rather than before joining, so that a build finishing in between is + // reused, and so that marking a bundle current doesn't race a build in flight + // replacing it. + if let existingBundle = await self.existingBundle(forManifestChecksum: manifest.checksum) { + await report(EditorProgress(completed: 1, total: 1)) + return await self.markCurrent(existingBundle) + } + + return try await self.build(manifest, reportingTo: report) + } + } + + /// Records that the site's manifest still matches `bundle`, by resetting its download date to now. That makes + /// it fresh again for the cache policy, and the newest bundle on disk — which matters when a site goes back to + /// a manifest it had before, whose bundle is older than the one it replaced. + private func markCurrent(_ bundle: EditorAssetBundle) -> EditorAssetBundle { + do { + let current = try EditorAssetBundle(manifest: bundle.manifest, bundleRoot: bundle.bundleRoot) + try current.writeManifest() + return current + } catch { + // The bundle is still complete and correct; it'll just be checked again sooner. + log(.warn, "Failed to mark asset bundle \(bundle.id) current: \(error.localizedDescription)") + return bundle } } diff --git a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift index 0355f4b15..59eaa33e1 100644 --- a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift +++ b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift @@ -138,6 +138,46 @@ struct EditorServiceTests: MakesTestFixtures { _ = try await second.value } + // MARK: - Cache Policy + + @Test("prepare() under .always uses the bundle on disk without checking the manifest") + func prepareUnderAlwaysUsesBundleOnDisk() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + let again = try await site.service(cachePolicy: .always).prepare().assetBundle + + #expect(again.id == bundle.id) + #expect(site.manifestRequestCount == 1) + #expect(site.client.downloadCallCount == 1) + } + + @Test("prepare() under .ignore checks the manifest, and keeps the bundle when it hasn't changed") + func prepareUnderIgnoreKeepsUnchangedBundle() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + let refreshed = try await site.service(cachePolicy: .ignore).prepare().assetBundle + + #expect(refreshed.id == bundle.id) + #expect(site.manifestRequestCount == 2) + #expect(site.client.downloadCallCount == 1) + } + + @Test("prepare() under .ignore picks up a changed manifest, which later editors then load") + func prepareUnderIgnorePicksUpChangedManifest() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + site.manifest = Self.pluginManifest(version: "2") + let refreshed = try await site.service(cachePolicy: .ignore).prepare().assetBundle + let afterwards = try await site.service(cachePolicy: .always).prepare().assetBundle + + #expect(refreshed.id != bundle.id) + #expect(afterwards.id == refreshed.id) + #expect(site.client.downloadCallCount == 2) + } + // MARK: - Test Helpers /// URL-based response handler for EditorService.prepare() tests. @@ -163,6 +203,53 @@ struct EditorServiceTests: MakesTestFixtures { return Data("{}".utf8) } } + + /// A manifest with one plugin script, whose URL carries `version` the way WordPress versions its + /// assets. + private static func pluginManifest(version: String) -> String { + #"{"scripts":"","styles":"","allowed_block_types":[]}"# + } + + /// One site's server and storage, shared by every service a test makes for it — as a host's + /// services for one site share them. + private final class TestSite { + let configuration: EditorConfiguration + let client = EditorAssetLibraryMockHTTPClient() + let storageRoot = URL.randomTemporaryDirectory + let cacheRoot = URL.randomTemporaryDirectory + + /// What the site's `editor-assets` endpoint answers. + var manifest: String { + didSet { serve(manifest) } + } + + var manifestRequestCount: Int { + client.requestedURLs.filter { $0.absoluteString.contains("editor-assets") }.count + } + + init(configuration: EditorConfiguration, manifest: String) { + self.configuration = configuration + self.manifest = manifest + serve(manifest) + } + + func service(cachePolicy: EditorCachePolicy) -> EditorService { + EditorService( + configuration: configuration, + httpClient: client, + cachePolicy: cachePolicy, + storageRoot: storageRoot, + cacheRoot: cacheRoot + ) + } + + private func serve(_ manifest: String) { + client.urlResponseHandler = { url in + url.absoluteString.contains("editor-assets") + ? Data(manifest.utf8) : EditorServiceTests.editorServiceResponseHandler(url) + } + } + } } /// Answers every request from `respond`, but holds each one until the test releases the caller diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 2dfa7640d..77a4a114e 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -87,8 +87,8 @@ struct EditorAssetLibraryTests { #expect(manifest.rawStyles.contains("plugin.css")) } - @Test("fetchManifest with ignore cache policy always fetches new data") - func fetchManifestIgnoreCachePolicyAlwaysFetches() async throws { + @Test("fetchManifest requests the manifest on every call") + func fetchManifestRequestsOnEveryCall() async throws { let manifestJSON = """ { "scripts": "", @@ -100,7 +100,7 @@ struct EditorAssetLibraryTests { let mockClient = EditorAssetLibraryMockHTTPClient() mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + let library = makeLibrary(httpClient: mockClient) _ = try await library.fetchManifest() _ = try await library.fetchManifest() @@ -108,14 +108,15 @@ struct EditorAssetLibraryTests { #expect(mockClient.getCallCount == 2) } - @Test("fetchManifest with always cache policy returns cached manifest when bundle exists on disk") - func fetchManifestAlwaysCachePolicyReturnsCachedManifest() async throws { + @Test("fetchManifest returns the manifest of a matching bundle on disk, even under .ignore") + func fetchManifestReturnsMatchingBundleManifest() async throws { let manifestJSON = uniqueManifestJSON(identifier: "test-cached-manifest-\(UUID().uuidString)") let mockClient = EditorAssetLibraryMockHTTPClient() mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - let library = makeLibrary(httpClient: mockClient, cachePolicy: .always) + // The policy decides whether to check the manifest at all, not what to make of the answer + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) // First, fetch the manifest and create a bundle on disk let originalManifest = try await library.fetchManifest() @@ -133,16 +134,16 @@ struct EditorAssetLibraryTests { #expect(mockClient.getCallCount == 2) } - @Test("fetchManifest with always cache policy falls back to new manifest when no bundle exists") - func fetchManifestAlwaysCachePolicyFallsBackWhenNoBundleExists() async throws { + @Test("fetchManifest parses a new manifest when no bundle matches") + func fetchManifestParsesWhenNoBundleMatches() async throws { let manifestJSON = uniqueManifestJSON(identifier: "test-no-cache-fallback-\(UUID().uuidString)") let mockClient = EditorAssetLibraryMockHTTPClient() mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - let library = makeLibrary(httpClient: mockClient, cachePolicy: .always) + let library = makeLibrary(httpClient: mockClient) - // Fetch with always cache policy when no bundle exists on disk + // Fetch when no bundle exists on disk let manifest = try await library.fetchManifest() // Should still return a valid manifest (created from remote data) @@ -150,9 +151,8 @@ struct EditorAssetLibraryTests { #expect(mockClient.getCallCount == 1) } - @Test( - "fetchManifest with always cache policy avoids expensive LocalEditorAssetManifest creation when cached") - func fetchManifestAlwaysCachePolicyAvoidsExpensiveCreation() async throws { + @Test("fetchManifest avoids expensive LocalEditorAssetManifest creation when a bundle matches") + func fetchManifestAvoidsExpensiveCreationWhenBundleMatches() async throws { // Use a manifest with multiple block types but no scripts/styles to avoid download issues let manifestJSON = """ { @@ -165,7 +165,7 @@ struct EditorAssetLibraryTests { let mockClient = EditorAssetLibraryMockHTTPClient() mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - let library = makeLibrary(httpClient: mockClient, cachePolicy: .always) + let library = makeLibrary(httpClient: mockClient) // First fetch and create the bundle let originalManifest = try await library.fetchManifest() @@ -313,125 +313,127 @@ struct EditorAssetLibraryTests { #expect(retrievedBundle.manifest.allowedBlockTypes == blockTypes) } - // MARK: - CachePolicy Tests - - @Test("EditorCachePolicy.always is default behavior") - func editorCachePolicyAlwaysIsDefault() async throws { - let manifestJSON = """ - { - "scripts": "", - "styles": "", - "allowed_block_types": [] - } - """ - - let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - - let library = makeLibrary(httpClient: mockClient) + // MARK: - Cache Policy Tests - // Call fetchManifest with default cache policy - _ = try await library.fetchManifest() + @Test("readLatestAssetBundle returns nil when there are no bundles") + func readLatestAssetBundleReturnsNilWithoutBundles() async throws { + let library = makeLibrary(cachePolicy: .always) - // The HTTP client should have been called - #expect(mockClient.getCallCount == 1) + #expect(try await library.readLatestAssetBundle() == nil) } - @Test("EditorCachePolicy.maxAge uses cached manifest when within timeout") - func editorCachePolicyMaxAgeUsesCachedWhenWithinTimeout() async throws { - let manifestJSON = uniqueManifestJSON(identifier: "test-maxage-within-\(UUID().uuidString)") + @Test("readLatestAssetBundle returns the newest bundle, however old, under .always") + func readLatestAssetBundleIgnoresAgeUnderAlways() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .always) + let bundle = try #require(try await library.readAssetBundles().first) + try backdate(bundle, by: 365 * 86_400) - let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } + #expect(try await library.readLatestAssetBundle()?.id == bundle.id) + } - // Set maxAge to 1 hour (3600 seconds) - let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(3600)) + @Test("readLatestAssetBundle returns nil under .ignore, even for a new bundle") + func readLatestAssetBundleReturnsNilUnderIgnore() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .ignore) - // First fetch and create the bundle - let originalManifest = try await library.fetchManifest() - _ = try await library.buildBundle(for: originalManifest) + #expect(try await library.readAssetBundles().count == 1) + #expect(try await library.readLatestAssetBundle() == nil) + } - // Second fetch should use cached manifest since we're within the 1 hour timeout - let cachedManifest = try await library.fetchManifest() + @Test("readLatestAssetBundle returns a bundle younger than .maxAge") + func readLatestAssetBundleReturnsBundleWithinMaxAge() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(3600)) + let bundle = try #require(try await library.readAssetBundles().first) + try backdate(bundle, by: 1800) - #expect(cachedManifest.checksum == originalManifest.checksum) - // Should have made 2 HTTP calls but second one used cached bundle - #expect(mockClient.getCallCount == 2) + #expect(try await library.readLatestAssetBundle()?.id == bundle.id) } - @Test("EditorCachePolicy.maxAge fetches new manifest when timeout expired") - func editorCachePolicyMaxAgeFetchesNewWhenExpired() async throws { - let manifestJSON = uniqueManifestJSON(identifier: "test-maxage-expired-\(UUID().uuidString)") + @Test("readLatestAssetBundle returns nil for a bundle older than .maxAge") + func readLatestAssetBundleReturnsNilPastMaxAge() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(3600)) + let bundle = try #require(try await library.readAssetBundles().first) + try backdate(bundle, by: 7200) - let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - - // Set maxAge to 0 seconds (immediately expired) - let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + #expect(try await library.readLatestAssetBundle() == nil) + } - // First fetch and create the bundle - let originalManifest = try await library.fetchManifest() - _ = try await library.buildBundle(for: originalManifest) + @Test("downloadAssetBundle keeps a bundle whose manifest hasn't changed, and marks it current") + func downloadAssetBundleKeepsUnchangedBundle() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .maxAge(3600)) + let bundle = try #require(try await library.readAssetBundles().first) + try backdate(bundle, by: 7200) + #expect(mockClient.downloadCallCount == 1) - // Second fetch should NOT use cached manifest since maxAge(0) means immediately expired - let newManifest = try await library.fetchManifest() + let checked = try await library.downloadAssetBundle() - // The checksums should still match (same data) but the cache was bypassed - #expect(newManifest.checksum == originalManifest.checksum) - #expect(mockClient.getCallCount == 2) + #expect(checked.id == bundle.id) + #expect(mockClient.getCallCount == 2) // The manifest, checked again + #expect(mockClient.downloadCallCount == 1) // Its asset, not downloaded again + #expect(try await library.readAssetBundles().count == 1) + #expect(try await library.readLatestAssetBundle()?.id == bundle.id) } - @Test("EditorCachePolicy.maxAge with short timeout expires after delay") - func editorCachePolicyMaxAgeExpiresAfterDelay() async throws { - let manifestJSON = uniqueManifestJSON(identifier: "test-maxage-delay-\(UUID().uuidString)") - - let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } + @Test("downloadAssetBundle builds a new bundle when the manifest has changed") + func downloadAssetBundleBuildsChangedBundle() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let original = try #require(try await library.readAssetBundles().first) - // Set maxAge to 0.05 seconds (50 milliseconds) - let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0.05)) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + let changed = try await library.downloadAssetBundle() - // First fetch and create the bundle - let originalManifest = try await library.fetchManifest() - _ = try await library.buildBundle(for: originalManifest) + #expect(changed.id != original.id) + #expect(mockClient.downloadCallCount == 2) + #expect(try await library.readAssetBundles().map(\.id) == [changed.id, original.id]) + } - // Wait for the cache to expire - try await Task.sleep(for: .milliseconds(100)) + @Test("downloadAssetBundle makes the bundle for a manifest the site went back to the newest again") + func downloadAssetBundleRestoresReturningBundle() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let original = try #require(try await library.readAssetBundles().first) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + let changed = try await library.downloadAssetBundle() - // Third fetch should bypass cache since it's expired - _ = try await library.fetchManifest() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let restored = try await library.downloadAssetBundle() - // Both fetches should have made HTTP calls since cache expired - #expect(mockClient.getCallCount == 2) + #expect(restored.id == original.id) + #expect(mockClient.downloadCallCount == 2) + #expect(try await library.readAssetBundles().map(\.id) == [original.id, changed.id]) } - @Test("EditorCachePolicy.maxAge uses cache before expiry then fetches after") - func editorCachePolicyMaxAgeTransitionsCorrectly() async throws { - let manifestJSON = uniqueManifestJSON(identifier: "test-maxage-transition-\(UUID().uuidString)") - + /// A library whose storage holds one bundle, built from ``manifestJSON(scriptVersion:)`` with version `1`. + private func makeLibraryWithBundle( + cachePolicy: EditorCachePolicy + ) async throws -> (EditorAssetLibrary, EditorAssetLibraryMockHTTPClient) { let mockClient = EditorAssetLibraryMockHTTPClient() - mockClient.urlResponseHandler = { _ in Data(manifestJSON.utf8) } - - // Set maxAge to 0.1 seconds (100 milliseconds) - let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0.1)) - - // First fetch and create the bundle - let originalManifest = try await library.fetchManifest() - _ = try await library.buildBundle(for: originalManifest) - - // Immediate second fetch should use cache (within 100ms) - let cachedManifest = try await library.fetchManifest() - #expect(cachedManifest.checksum == originalManifest.checksum) - - // Wait for cache to expire - try await Task.sleep(for: .milliseconds(150)) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: cachePolicy) + _ = try await library.downloadAssetBundle() + return (library, mockClient) + } + + /// A manifest with one script, whose URL carries `scriptVersion` the way WordPress versions its assets. + private static func manifestJSON(scriptVersion: String) -> String { + """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + } - // Third fetch should create new manifest since cache expired - let newManifest = try await library.fetchManifest() - #expect(newManifest.checksum == originalManifest.checksum) + private static func responses(forManifest manifestJSON: String) -> (URL) throws -> Data { + { url in url.path.contains("editor-assets") ? Data(manifestJSON.utf8) : Data("mock content".utf8) } + } - // Should have made 3 HTTP calls total - #expect(mockClient.getCallCount == 3) + /// Rewrites `bundle`'s download date on disk to `interval` seconds ago. + private func backdate(_ bundle: EditorAssetBundle, by interval: TimeInterval) throws { + try EditorAssetBundle( + manifest: bundle.manifest, + downloadDate: Date(timeIntervalSinceNow: -interval), + bundleRoot: bundle.bundleRoot + ).writeManifest() } // MARK: - Bundle Fetching Tests with Real Manifest Data From 802793b8787d7e41f89f7aab5e391fa0b95efdf1 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:14:43 -0600 Subject: [PATCH 2/9] fix(ios): stop JSON's description raising an exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `JSON.description` handed the enum itself to `JSONSerialization`, which takes Foundation objects and raises an Objective-C exception for anything else. Swift can't catch one, so describing a `JSON` — or anything holding one, like an `EditorSettings` or `EditorDependencies` — ended the process. Nothing in the library describes one, but a log message, a `po` in the debugger, or a failing test expectation does. It now encodes with `JSONEncoder`, which the type already supports, and writes a number JSON can't represent as a string rather than failing. --- .../GutenbergKit/Sources/Model/JSON.swift | 19 +++++++++- .../GutenbergKitTests/Model/JSONTests.swift | 37 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Model/JSON.swift b/ios/Sources/GutenbergKit/Sources/Model/JSON.swift index 6435fb466..450a7307d 100644 --- a/ios/Sources/GutenbergKit/Sources/Model/JSON.swift +++ b/ios/Sources/GutenbergKit/Sources/Model/JSON.swift @@ -23,9 +23,24 @@ public enum JSON: Sendable, Equatable, Hashable, CustomStringConvertible { self = try JSONDecoder().decode(JSON.self, from: data) } + /// The value as JSON, laid out for reading. public var description: String { - let data = try! JSONSerialization.data(withJSONObject: self, options: [.prettyPrinted, .withoutEscapingSlashes]) - return String(data: data, encoding: .utf8)! + // Not `JSONSerialization`, which takes Foundation objects: handed this enum, it raises an + // exception that Swift can't catch. + let encoder = JSONEncoder() + encoder.outputFormatting = [.prettyPrinted, .sortedKeys, .withoutEscapingSlashes] + // JSON has no way to write these numbers, and a description shouldn't fail over one. + encoder.nonConformingFloatEncodingStrategy = .convertToString( + positiveInfinity: "Infinity", + negativeInfinity: "-Infinity", + nan: "NaN" + ) + + guard let data = try? encoder.encode(self) else { + return "" + } + + return String(decoding: data, as: UTF8.self) } } diff --git a/ios/Tests/GutenbergKitTests/Model/JSONTests.swift b/ios/Tests/GutenbergKitTests/Model/JSONTests.swift index ed99a5fb5..2a40752e3 100644 --- a/ios/Tests/GutenbergKitTests/Model/JSONTests.swift +++ b/ios/Tests/GutenbergKitTests/Model/JSONTests.swift @@ -819,6 +819,43 @@ struct JSONTests { #expect(decodedFalse == originalFalse) } + // MARK: - Description Tests + + @Test("description is the value as JSON") + func descriptionIsJSON() throws { + let json: JSON = ["b": [1, 2.5, "three", true, nil], "a": ["url": "https://example.com/a"]] + + #expect(try JSON(Data(json.description.utf8)) == json) + // Readable: one member to a line, in a stable order, with slashes left alone + #expect(json.description.contains("\n")) + #expect(json.description.contains("https://example.com/a")) + #expect(try #require(json.description.firstRange(of: #""a""#)).lowerBound < #require(json.description.firstRange(of: #""b""#)).lowerBound) + } + + @Test("description handles a value that isn't an object or an array") + func descriptionHandlesFragments() { + #expect(JSON.string("a/b").description == #""a/b""#) + #expect(JSON.number(1.5).description == "1.5") + #expect(JSON.boolean(true).description == "true") + #expect(JSON.null.description == "null") + } + + @Test("description handles a number that JSON can't represent") + func descriptionHandlesNonFiniteNumbers() { + #expect(JSON.number(.infinity).description == #""Infinity""#) + #expect(JSON.number(-.infinity).description == #""-Infinity""#) + #expect(JSON.number(.nan).description == #""NaN""#) + } + + @Test("a value holding JSON can be described, as a failing expectation or a log message would") + func valuesHoldingJSONCanBeDescribed() throws { + let settings = try EditorSettings(data: Data(#"{"styles":[],"alignWide":true}"#.utf8)) + let dependencies = EditorDependencies(editorSettings: settings, assetBundle: .empty, preloadList: nil) + + #expect(String(describing: settings).contains("alignWide")) + #expect(String(reflecting: dependencies).contains("alignWide")) + } + // MARK: - Resource File Validation Tests static let objectResourceFiles = [ From da09de08286fdfec9bb8d1be0e01770e58543076 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:14:43 -0600 Subject: [PATCH 3/9] fix(ios): close the gaps a review found in refreshing asset bundles A refresh could return a bundle that was no longer on disk. The check for an existing bundle and the write marking it current were separated by a progress report, and a `cleanup()` or `purge()` landing in between left `markCurrent` recreating the directory with only `manifest.json`. The editor then opened without plugin or theme assets, and said nothing. The bundle is now looked for again before it's marked, under a lock shared with `cleanup()` and `purge()`, and built again if it's gone. `cleanup()` keeps every bundle handed out since launch. A refresh makes a second bundle on disk routine, and dependencies a host prepared earlier, or an open editor, may still be reading the first. Those go in a cleanup after the next launch. A check now stamps `lastCheckedDate` in the bundle's manifest rather than resetting `downloadDate`, and bundle equality ignores it, so a check leaves a bundle equal to the copy a host already holds. Bundles are ordered, and aged for the cache policy, by when the manifest last matched them: the last check, or the download for a bundle never checked. `readAssetBundles()` also built each bundle's path from the directory listing, which resolves symlinks. On macOS, where the tests run, it returned a bundle under `/private/var` where a build returned the same bundle under `/var`, and the two compared unequal. It now builds the path from the storage root, as a build does. With automatic network fallback, a `.maxAge` or `.ignore` service that can't reach the site returns what's on disk, however old, before it falls back to empty dependencies. The documented refresh recipe hands its result to the next editor, which would otherwise have opened with nothing while everything it needed was on disk. Also: - A check downloads the assets an earlier build of the bundle failed to, rather than marking a bundle with gaps as fresh. - The manifest request for a check asks for a fresh answer, so neither a stored response nor a request in flight can stand in for it. `.always` still shares a request in flight. - `readLatestAssetBundle()` is public, since it's what applies the library's cache policy. - Automatic cleanup takes its daily turn per site. One key covered every site, so whichever was prepared first each day used it. - Progress no longer passes its total. The bundle download reports a running total, which was added in full on every report. Each fix has a test that fails without it. --- docs/code/preloading.md | 30 ++- .../Sources/Model/EditorAssetBundle.swift | 49 +++- .../Sources/Model/EditorConfiguration.swift | 3 +- .../Sources/Services/EditorService.swift | 74 +++++- .../Sources/Stores/EditorAssetLibrary.swift | 210 +++++++++++++----- .../Services/EditorServiceTests.swift | 172 +++++++++++++- .../Stores/EditorAssetLibraryTests.swift | 186 +++++++++++++++- ios/Tests/GutenbergKitTests/TestHelpers.swift | 26 +++ 8 files changed, 653 insertions(+), 97 deletions(-) diff --git a/docs/code/preloading.md b/docs/code/preloading.md index cd014a798..e56a717a8 100644 --- a/docs/code/preloading.md +++ b/docs/code/preloading.md @@ -213,16 +213,20 @@ This filtering is performed by `EditorURLResponse.asPreloadResponse()`. ### Automatic Cleanup -`EditorService` automatically cleans up old asset bundles once per day: +`EditorService` automatically cleans up each site's old asset bundles once per day: **Swift** ```swift -try await onceEvery(.seconds(86_400)) { - try await self.cleanup() -} +try await onceEvery( + .seconds(86_400), + { try await self.cleanup() }, + handle: "asset-bundle-cleanup-\(self.configuration.siteId)" +) ``` +A cleanup keeps the site's latest bundle, and any bundle the app has been handed since it launched — an open editor, or dependencies the host still holds, may be reading it. + **Kotlin** ```kotlin @@ -251,13 +255,15 @@ try await service.purge() An `EditorService`'s cache policy covers plugin and theme assets as well as API responses. For assets, it decides when to check the site's asset manifest again: -| Policy | API responses | Asset bundle | -| ----------------------- | ------------------------------- | ------------------------------------------------------ | -| `.always` (default) | Fetched only when not cached | Manifest checked only when no bundle is on disk | -| `.maxAge(TimeInterval)` | Fetched once older than the age | Manifest checked once the bundle is older than the age | -| `.ignore` | Always fetched | Manifest always checked | +| Policy | API responses | Asset bundle | +| ----------------------- | ------------------------------- | ---------------------------------------------------------- | +| `.always` (default) | Fetched only when not cached | Manifest checked only when no bundle is on disk | +| `.maxAge(TimeInterval)` | Fetched once older than the age | Manifest checked once the last check is older than the age | +| `.ignore` | Always fetched | Manifest always checked | -If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. If it has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. `cleanup()` removes the old one later. +If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. Only an asset that failed to download when the bundle was built is tried again. If the manifest has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. + +The old bundle stays on disk for as long as the app is running, because an open editor — or dependencies the host prepared earlier and still holds — may be reading it. `cleanup()` removes it after the next launch. To refresh a site's editor data — on pull-to-refresh, for instance — prepare a separate service that ignores the cache, and give its dependencies to the next editor: @@ -269,6 +275,8 @@ let dependencies = try await EditorService(configuration: configuration, cachePo Nothing is deleted first, so an editor opened during the refresh still loads straight from what's on disk, and a refresh that fails leaves it all in place. An editor given no dependencies prepares its own with `.always`, so it uses whatever the last refresh left. To download assets again even when their manifest hasn't changed, `purge()` instead, at the cost of a cold load for the next editor. +A refresh that can't reach the site throws. If the configuration's `networkFallbackMode` is `.automatic`, it returns the dependencies already on disk instead — however old they are — so they're still safe to give to the next editor. It returns empty dependencies only when something the editor needs has never been cached. + **Kotlin** Not yet: Android's `EditorService` still checks the asset manifest only when no bundle is on disk, whatever its cache policy. @@ -308,6 +316,8 @@ let config = EditorConfigurationBuilder( When a network error is caught (e.g., `notConnectedToInternet`, `timedOut`, `cannotConnectToHost`), `EditorService.prepare()` returns empty dependencies — the same as offline mode — so the bundled editor loads instead of showing an error. Non-network errors (e.g., decoding failures) still propagate normally. +On iOS, a service whose cache policy is `.maxAge` or `.ignore` first falls back to the dependencies already on disk, however old: they can't be checked against a site that can't be reached, and they're better than none. It returns empty dependencies only if some are missing. + On the JavaScript side, an `OfflineIndicator` component displays a "Working Offline" status bar at the top of the editor when the device loses connectivity. The indicator automatically appears and disappears based on the browser's `online`/`offline` events. | Mode | Use case | Behavior | diff --git a/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift b/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift index 4525e4b6f..3af451c5f 100644 --- a/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift +++ b/ios/Sources/GutenbergKit/Sources/Model/EditorAssetBundle.swift @@ -14,7 +14,7 @@ import SwiftSoup /// /// Assets are accessed via URL lookup - the bundle maintains a mapping from /// original remote URLs to local file paths. -public struct EditorAssetBundle: Sendable, Equatable, Hashable { +public struct EditorAssetBundle: Sendable { /// The EditorRepresentation has the exact same format as `RemoteEditorAssetManifest.RawManifest` – what we're passing to Gutenberg /// looks exactly like what it'd get if it called `/wpcom/v2/editor-assets` directly. @@ -35,6 +35,8 @@ public struct EditorAssetBundle: Sendable, Equatable, Hashable { struct RawAssetBundle: Codable { let manifest: LocalEditorAssetManifest let downloadDate: Date + /// Absent from a bundle stored before this was recorded. + var lastCheckedDate: Date? } /// The bundle's unique identifier, derived from its manifest checksum. @@ -48,13 +50,26 @@ public struct EditorAssetBundle: Sendable, Equatable, Hashable { /// The manifest that defines which assets belong to this bundle. let manifest: LocalEditorAssetManifest - /// The date this bundle was created by downloading the manifest contents, or last found to still match the - /// site's manifest. + /// The date this bundle was created by downloading the manifest contents. /// - /// Used to determine which bundle is most recent when multiple bundles exist, and how old it is for the - /// cache policy. + /// Used to determine which bundle is most recent when multiple bundles exist, until the bundle + /// has a ``lastCheckedDate``. let downloadDate: Date + /// The date the site's manifest was last found to match this bundle, if that's been recorded. + /// + /// It says how recently the bundle was confirmed, not what the bundle is, so two copies of a + /// bundle that differ only in this are equal. + let lastCheckedDate: Date? + + /// When the site's manifest is last known to have matched this bundle: when it was last + /// checked, or else when it was downloaded. + /// + /// Used to determine which bundle is the site's latest, and how old it is for the cache policy. + var lastMatchedDate: Date { + lastCheckedDate ?? downloadDate + } + /// The number of assets stored in this bundle. public var assetCount: Int { manifest.assetUrls.count @@ -65,12 +80,19 @@ public struct EditorAssetBundle: Sendable, Equatable, Hashable { init(raw: RawAssetBundle, bundleRoot: URL) { self.manifest = raw.manifest self.downloadDate = raw.downloadDate + self.lastCheckedDate = raw.lastCheckedDate self.bundleRoot = bundleRoot } - init(manifest: LocalEditorAssetManifest, downloadDate: Date = Date(), bundleRoot: URL) throws { + init( + manifest: LocalEditorAssetManifest, + downloadDate: Date = Date(), + lastCheckedDate: Date? = nil, + bundleRoot: URL + ) throws { self.manifest = manifest self.downloadDate = downloadDate + self.lastCheckedDate = lastCheckedDate self.bundleRoot = bundleRoot } @@ -188,7 +210,8 @@ public struct EditorAssetBundle: Sendable, Equatable, Hashable { func dataRepresentation() throws -> Data { try JSONEncoder().encode(RawAssetBundle( manifest: self.manifest, - downloadDate: self.downloadDate + downloadDate: self.downloadDate, + lastCheckedDate: self.lastCheckedDate )) } @@ -235,3 +258,15 @@ public struct EditorAssetBundle: Sendable, Equatable, Hashable { bundleRoot: URL.temporaryDirectory ) } + +extension EditorAssetBundle: Equatable, Hashable { + public static func == (lhs: EditorAssetBundle, rhs: EditorAssetBundle) -> Bool { + lhs.manifest == rhs.manifest && lhs.downloadDate == rhs.downloadDate && lhs.bundleRoot == rhs.bundleRoot + } + + public func hash(into hasher: inout Hasher) { + hasher.combine(manifest) + hasher.combine(downloadDate) + hasher.combine(bundleRoot) + } +} diff --git a/ios/Sources/GutenbergKit/Sources/Model/EditorConfiguration.swift b/ios/Sources/GutenbergKit/Sources/Model/EditorConfiguration.swift index 69661facd..c03ca74d0 100644 --- a/ios/Sources/GutenbergKit/Sources/Model/EditorConfiguration.swift +++ b/ios/Sources/GutenbergKit/Sources/Model/EditorConfiguration.swift @@ -9,7 +9,8 @@ import Foundation public enum NetworkFallbackMode: Sendable, Hashable { /// Network failures are fatal and propagate as errors (current default behavior). case disabled - /// Automatically fall back to the bundled editor when network requests fail. + /// Automatically fall back when network requests fail: to the dependencies already on disk, + /// even ones the cache policy considers too old, or else to the bundled editor. case automatic } diff --git a/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift b/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift index 585b5859c..342223f0b 100644 --- a/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift +++ b/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift @@ -21,9 +21,17 @@ public actor EditorService { private let restRepository: RESTAPIRepository private let assetLibrary: EditorAssetLibrary + /// What `prepare()` falls back to when the site can't be reached and the network fallback is + /// automatic: this service under the `.always` policy, which uses whatever is on disk however + /// old it is. `nil` when the service wouldn't use it. + private let diskFallback: EditorService? + private var progress: EditorProgress? private var progressCallback: EditorProgressCallback? + /// How much of the asset bundle's weight has been counted toward `progress`. + private var assetBundleProgress = 0 + enum DependencyWeights: CaseIterable { case editorSettings case assetBundle @@ -116,6 +124,19 @@ public actor EditorService { cachePolicy: cachePolicy, storageRoot: storageRoot ?? Paths.storageRoot(for: configuration) ) + + switch (configuration.networkFallbackMode, cachePolicy) { + case (.automatic, .maxAge), (.automatic, .ignore): + self.diskFallback = EditorService( + configuration: configuration, + httpClient: httpClient, + cachePolicy: .always, + storageRoot: storageRoot, + cacheRoot: cacheRoot + ) + case (.automatic, .always), (.disabled, _): + self.diskFallback = nil + } } /// Returns the number of asset bundles currently stored on disk. @@ -128,6 +149,10 @@ public actor EditorService { /// This method fetches editor settings, plugin assets, and preload data concurrently, /// caching results for future use. If offline mode is enabled, returns empty dependencies. /// + /// If the site can't be reached and the configuration's network fallback is automatic, this + /// returns the dependencies on disk instead of throwing — even ones too old for the cache + /// policy, which can't be checked without the site — and empty dependencies if any are missing. + /// /// - Parameter progress: A callback invoked with progress updates during loading. /// - Returns: The complete set of dependencies needed to initialize the editor. /// - Throws: An error if any required resource fails to download. @@ -144,6 +169,7 @@ public actor EditorService { self.progress = EditorProgress(completed: 1, total: 100) self.progressCallback = progress + self.assetBundleProgress = 0 defer { self.progressCallback = nil self.progress = nil @@ -154,6 +180,13 @@ public actor EditorService { return try await fetchDependencies() } catch { guard isNetworkError(error) else { throw error } + + // Nothing on disk can be checked against a site that can't be reached, and what's + // there beats loading with nothing — however old it is. + if let diskFallback { + return try await diskFallback.prepare() + } + return EditorDependencies( editorSettings: .undefined, assetBundle: .empty, @@ -168,6 +201,7 @@ public actor EditorService { /// Clear unused on-disk resources associated with this service's configuration. /// /// Calling this method will preserve the most recent cache entries, ensuring that the editor still loads quickly without continuing to use unnecessary disk space. + /// It also preserves any asset bundle the app has been handed since it launched, which an open editor or dependencies the host is holding may still be using. /// Use this method to regularly clean up unused editor assets. public func cleanup() async throws { try await self.assetLibrary.cleanup() @@ -181,14 +215,32 @@ public actor EditorService { try self.restRepository.purge() } - private func incrementProgress(for weight: DependencyWeights, fraction: Double = 1.0) async { + private func incrementProgress(for weight: DependencyWeights) async { + await self.incrementProgress(by: Int(weight.rawValue)) + } + + /// Counts an asset bundle download's progress toward the total. The download reports how far + /// along it is each time, not how much further than the last time, so only what's new is added. + private func incrementProgress(forAssetBundleDownload download: EditorProgress) async { + let assetBundleProgress = Int(DependencyWeights.assetBundle.rawValue * download.fractionCompleted) + let increase = assetBundleProgress - self.assetBundleProgress + guard increase > 0 else { return } + + self.assetBundleProgress = assetBundleProgress + await self.incrementProgress(by: increase) + } + + private func incrementProgress(by amount: Int) async { // Progress can arrive after the `prepare()` it belongs to has returned and cleared it. A // bundle build shared with another service may already be calling in when this service // gives up on it, and an overlapping `prepare()` on this service is cleared by whichever // finishes first. There is nothing left to report to, so drop it. guard let current = self.progress else { return } + + // Progress starts at 1, and a post adds its weight to the others', so the weights can add + // up to more than the total. let progress = EditorProgress( - completed: current.completed + Int(weight.rawValue * fraction), + completed: min(current.completed + amount, current.total), total: current.total) self.progress = progress await self.progressCallback?(progress) @@ -199,10 +251,12 @@ public actor EditorService { async let assetBundle = try self.prepareAssetBundle() async let preloadList = try preparePreloadList() - // Automatically clean up old asset bundles - try await onceEvery(.seconds(86_400)) { - try await self.cleanup() - } + // Automatically clean up old asset bundles, once a day for each site + try await onceEvery( + .seconds(86_400), + { try await self.cleanup() }, + handle: "asset-bundle-cleanup-\(self.configuration.siteId)" + ) return try await EditorDependencies( editorSettings: settings, @@ -240,9 +294,13 @@ public actor EditorService { return latestAssetBundle } - return try await self.assetLibrary.downloadAssetBundle { progress in - await self.incrementProgress(for: .assetBundle, fraction: progress.fractionCompleted) + let assetBundle = try await self.assetLibrary.downloadAssetBundle { progress in + await self.incrementProgress(forAssetBundleDownload: progress) } + + // A bundle with nothing to download reports no progress + await self.incrementProgress(forAssetBundleDownload: EditorProgress(completed: 1, total: 1)) + return assetBundle } private func preparePreloadList() async throws -> EditorPreloadList { diff --git a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift index 17838479b..47329508e 100644 --- a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift +++ b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift @@ -13,15 +13,25 @@ public actor EditorAssetLibrary { /// library, so this is shared across all of them. static let inFlightBuilds = InFlightTasks() + /// Guards every change to which bundles are on disk and which of them is a site's latest — + /// marking the latest, `cleanup()` and `purge()` — along with `handedOut`. Every service + /// builds its own library, so an actor's isolation doesn't order these between libraries. + private static let storageLock = NSLock() + + /// The directory of every bundle this process has handed to a caller. An editor, or + /// dependencies a host is holding, may still be reading one, so `cleanup()` leaves them be. + nonisolated(unsafe) private static var handedOut: Set = [] + /// Creates a new `EditorAssetLibrary` instance. /// /// - Parameters: /// - configuration: The editor configuration containing site-specific settings. /// - httpClient: The HTTP client used to fetch remote assets. - /// - cachePolicy: The policy that determines how long the newest bundle on disk is used - /// before the site's manifest is checked again. Use `.ignore` to check it every time, - /// `.maxAge(_:)` to check it once the bundle is older than a time interval, or `.always` - /// (the default) to check it only when there is no bundle on disk. + /// - cachePolicy: The policy that determines how long ``readLatestAssetBundle()`` goes on + /// returning the latest bundle on disk before the site's manifest has to be checked again. + /// Use `.ignore` to check it every time, `.maxAge(_:)` to check it once the last check is + /// older than a time interval, or `.always` (the default) to check it only when there is + /// no bundle on disk. /// - storageRoot: The root directory where asset bundles will be stored on disk. public init( configuration: EditorConfiguration, @@ -44,9 +54,17 @@ public actor EditorAssetLibrary { /// func fetchManifest() async throws -> LocalEditorAssetManifest { guard configuration.shouldUsePlugins else { return .empty } - let data = try await httpClient.perform( - URLRequest(method: .GET, url: self.editorAssetsUrl(for: self.configuration)) - ).0 + var request = URLRequest(method: .GET, url: self.editorAssetsUrl(for: self.configuration)) + switch self.cachePolicy { + case .always: + // Only asked when there's no bundle to use, so an answer already on its way will do + break + case .maxAge, .ignore: + // A check is to find out what the site serves now, which neither a stored response + // nor a request already in flight can say. + request.cachePolicy = .reloadIgnoringLocalCacheData + } + let data = try await httpClient.perform(request).0 let remoteManifest = try RemoteEditorAssetManifest(data: data) // The checksum covers the whole response, so a bundle with the same one was built from @@ -60,7 +78,8 @@ public actor EditorAssetLibrary { // MARK: - Bundle Handling - /// The downloaded asset bundles for a given `EditorConfiguration`. Ordered newest to oldest. + /// The downloaded asset bundles for a given `EditorConfiguration`, ordered by when the site's manifest last + /// matched each: the latest first. /// public func readAssetBundles() throws -> [EditorAssetBundle] { try FileManager.default.createDirectory(at: self.storageRoot, withIntermediateDirectories: true) @@ -68,32 +87,41 @@ public actor EditorAssetLibrary { .contentsOfDirectory(at: self.storageRoot, includingPropertiesForKeys: [.isDirectoryKey]) .filter { $0.hasDirectoryPath } // Only include directories .filter { $0.pathExtension != "download" } // Don't include bundles that are being downloaded - .map { $0.appending(path: "manifest.json") } + // Not the listed URL itself: a listing resolves symlinks in the path (`/var` to `/private/var`), which + // would make a bundle read here unequal to the same bundle built or looked up by checksum. + .map { self.bundleManifestPath(for: $0.lastPathComponent) } .compactMap { try? EditorAssetBundle(url: $0) } // Skip invalid/incomplete bundles - .sorted { $0.downloadDate > $1.downloadDate } + // Not by when each was downloaded: a site can go back to a manifest it had before, whose bundle + // was downloaded earlier than the one it replaced. + .sorted { $0.lastMatchedDate > $1.lastMatchedDate } } - /// The newest bundle on disk, if the cache policy still trusts it. + /// The latest bundle on disk, if the cache policy still trusts it. /// - /// Returns `nil` when there is no bundle, or when the newest one is too old for the policy. Either way, call - /// ``downloadAssetBundle(progress:)`` next to check the site's manifest. - func readLatestAssetBundle() throws -> EditorAssetBundle? { + /// Returns `nil` when there is no bundle, or when the site's manifest was last checked too long ago for the + /// policy. Either way, call ``downloadAssetBundle(progress:)`` next to check it. + public func readLatestAssetBundle() throws -> EditorAssetBundle? { guard let latestBundle = try self.readAssetBundles().first, - self.cachePolicy.allowsResponseWith(date: latestBundle.downloadDate) + self.cachePolicy.allowsResponseWith(date: latestBundle.lastMatchedDate) else { return nil } - return latestBundle + return Self.storageLock.withLock { + guard self.hasBundle(forManifestChecksum: latestBundle.id) else { return nil } + Self.handedOut.insert(self.bundleRoot(for: latestBundle).standardizedFileURL) + return latestBundle + } } /// Fetches the latest manifest from the server and downloads all of its resources, caching them on-disk. /// /// If a bundle built from the same manifest is already on disk, it's returned instead, without downloading its - /// assets again: they're versioned by URL, so an unchanged manifest means unchanged assets. The bundle then - /// counts as newly downloaded, both for the cache policy and as the newest bundle on disk. To download every - /// asset again regardless, ``purge()`` the library first. + /// assets again: they're versioned by URL, so an unchanged manifest means unchanged assets. Only an asset that + /// an earlier build failed to download is tried again. Either way the bundle becomes the site's latest, and + /// its age for the cache policy starts over. To download every asset again regardless, ``purge()`` the + /// library first. /// /// - Parameter progress: An optional callback that receives progress updates as assets are downloaded. /// - Returns: The downloaded `EditorAssetBundle` containing all cached assets. @@ -105,7 +133,7 @@ public actor EditorAssetLibrary { return try await self.buildBundle(for: manifest, progress: progress) } - @available(*, deprecated, message: "`cachePolicy` has no effect; the library's own cache policy applies. Drop the argument.") + @available(*, deprecated, message: "`cachePolicy` has no effect: this always checks the site's manifest. Drop the argument.") public func downloadAssetBundle( cachePolicy: EditorCachePolicy, progress: EditorProgressCallback? = nil @@ -139,7 +167,8 @@ public actor EditorAssetLibrary { /// /// Assets are downloaded concurrently and stored in a temporary directory. Once all downloads /// complete successfully, the bundle is atomically moved to its final location. If a complete - /// bundle for the manifest is already there, it's marked current and returned instead. + /// bundle for the manifest is already there, it's returned instead, once any asset it's missing + /// has been tried again. Either way, the bundle is marked as the site's latest. func buildBundle( for manifest: LocalEditorAssetManifest, progress: EditorProgressCallback? = nil @@ -156,29 +185,66 @@ public actor EditorAssetLibrary { let destination = self.bundleRoot(for: manifest.checksum).standardizedFileURL return try await Self.inFlightBuilds.value(for: destination, progress: progress) { report in // Checked here rather than before joining, so that a build finishing in between is - // reused, and so that marking a bundle current doesn't race a build in flight - // replacing it. + // reused, and so that reusing a bundle doesn't race a build in flight replacing it. if let existingBundle = await self.existingBundle(forManifestChecksum: manifest.checksum) { - await report(EditorProgress(completed: 1, total: 1)) - return await self.markCurrent(existingBundle) + try await self.downloadMissingAssets(of: existingBundle, reportingTo: report) + + // A `cleanup()` or `purge()` can delete the bundle after it's found here, in which + // case there is nothing to reuse after all. + if let latestBundle = await self.markLatest(existingBundle) { + return latestBundle + } } - return try await self.build(manifest, reportingTo: report) + let bundle = try await self.build(manifest, reportingTo: report) + return await self.markLatest(bundle) ?? bundle + } + } + + /// Downloads whichever of `bundle`'s assets aren't on disk. A build publishes its bundle without any asset + /// that fails to download, and an unchanged manifest would otherwise never give that asset another try. + private func downloadMissingAssets( + of bundle: EditorAssetBundle, + reportingTo progress: EditorProgressCallback + ) async throws { + let missingAssets = self.downloadableAssets(in: bundle.manifest) + .filter { !FileManager.default.fileExists(at: self.assetPath(for: $0, in: bundle)) } + + guard !missingAssets.isEmpty else { + await progress(EditorProgress(completed: 1, total: 1)) + return } + + try await self.downloadAssets(missingAssets, into: bundle, reportingTo: progress) } - /// Records that the site's manifest still matches `bundle`, by resetting its download date to now. That makes - /// it fresh again for the cache policy, and the newest bundle on disk — which matters when a site goes back to - /// a manifest it had before, whose bundle is older than the one it replaced. - private func markCurrent(_ bundle: EditorAssetBundle) -> EditorAssetBundle { - do { - let current = try EditorAssetBundle(manifest: bundle.manifest, bundleRoot: bundle.bundleRoot) - try current.writeManifest() - return current - } catch { - // The bundle is still complete and correct; it'll just be checked again sooner. - log(.warn, "Failed to mark asset bundle \(bundle.id) current: \(error.localizedDescription)") - return bundle + /// Records that the site's manifest matches `bundle` now, and that the bundle has been handed out. That makes + /// it the site's latest bundle, and starts its age for the cache policy over. Returns `nil` if the bundle is + /// no longer on disk. + private func markLatest(_ bundle: EditorAssetBundle) -> EditorAssetBundle? { + Self.storageLock.withLock { + guard self.hasBundle(forManifestChecksum: bundle.id) else { + return nil + } + + Self.handedOut.insert(self.bundleRoot(for: bundle).standardizedFileURL) + + do { + let latestBundle = try EditorAssetBundle( + manifest: bundle.manifest, + downloadDate: bundle.downloadDate, + lastCheckedDate: Date(), + bundleRoot: bundle.bundleRoot + ) + // Written straight to the file, which is known to be there: `writeManifest()` would + // create the bundle's directory if it weren't. + try latestBundle.dataRepresentation().write(to: self.bundleManifestPath(for: bundle), options: .atomic) + return latestBundle + } catch { + // The bundle is still complete and correct; it'll just be checked again sooner. + log(.warn, "Failed to mark asset bundle \(bundle.id) as the latest: \(error.localizedDescription)") + return bundle + } } } @@ -186,8 +252,6 @@ public actor EditorAssetLibrary { _ manifest: LocalEditorAssetManifest, reportingTo progress: EditorProgressCallback ) async throws -> EditorAssetBundle { - var complete = 0 - let tempDirectory = URL.temporaryDirectory.appending(path: UUID().uuidString) let bundle = try EditorAssetBundle( @@ -198,10 +262,26 @@ public actor EditorAssetLibrary { let editorRepresentation = try manifest.buildEditorRepresentation(for: self.configuration) try bundle.writeManifest(editorRepresentation: editorRepresentation) - await withTaskGroup { group in - let links = (manifest.scripts + manifest.styles).filter { self.isSupportedAsset($0) } + try await self.downloadAssets(self.downloadableAssets(in: manifest), into: bundle, reportingTo: progress) - for asset in links { + return try bundle.copy(to: self.bundleRoot(for: bundle)) + } + + /// The assets in `manifest` that belong in its bundle. + private func downloadableAssets(in manifest: LocalEditorAssetManifest) -> [URL] { + (manifest.scripts + manifest.styles).filter { self.isSupportedAsset($0) } + } + + /// Downloads `assets` into `bundle` concurrently, tolerating any that fail. + private func downloadAssets( + _ assets: [URL], + into bundle: EditorAssetBundle, + reportingTo progress: EditorProgressCallback + ) async throws { + var complete = 0 + + await withTaskGroup { group in + for asset in assets { group.addTask { do { try await self.fetchAsset(url: asset, into: bundle) @@ -215,17 +295,16 @@ public actor EditorAssetLibrary { for await _ in group { complete += 1 - await progress(EditorProgress(completed: complete, total: links.count)) + await progress(EditorProgress(completed: complete, total: assets.count)) } } // The group swallows every per-asset failure, cancellation included, so a - // cancelled build still arrives here with assets missing. Nothing downstream + // cancelled download still arrives here with assets missing. Nothing downstream // checks for them — `readAssetBundles()` reads only the manifest — so publishing - // it would serve the gap on every later launch. + // the bundle, or recording it as the latest, would serve the gap on every later + // launch. try Task.checkCancellation() - - return try bundle.copy(to: self.bundleRoot(for: bundle)) } /// Downloads a single asset and copies it into the temporary bundle directory. @@ -236,7 +315,7 @@ public actor EditorAssetLibrary { try await httpClient.download(URLRequest(method: .GET, url: url)).0 } - let destinationPath = bundle.bundleRoot.appending(path: url.path(percentEncoded: false)) + let destinationPath = self.assetPath(for: url, in: bundle) let destinationParent = destinationPath.deletingLastPathComponent() // Ensure the destination directory exists @@ -247,6 +326,11 @@ public actor EditorAssetLibrary { return destinationPath } + /// Where the asset at `url` is stored in `bundle`. + private func assetPath(for url: URL, in bundle: EditorAssetBundle) -> URL { + bundle.bundleRoot.appending(path: url.path(percentEncoded: false)) + } + /// Checks if the given `url` is eligible to be downloaded into the local bundle /// /// Only HTTP/HTTPS URLs with `.js`, `.css`, or `.js.map` extensions are supported. @@ -283,15 +367,21 @@ public actor EditorAssetLibrary { /// Cleans up outdated library entries for this site. /// - /// This method removes all asset bundles except the most recent one, freeing disk space - /// while ensuring the editor can still load quickly with cached assets. + /// This method removes all asset bundles except the latest one, freeing disk space + /// while ensuring the editor can still load quickly with cached assets. It also keeps any + /// bundle the app has been handed since it launched: an open editor, or dependencies the + /// host is still holding, may be reading it. Those are removed by a cleanup after the next launch. /// /// - Throws: An error if the list of bundles cannot be read, or any bundle cannot be removed. public func cleanup() throws { - let bundles = try self.readAssetBundles().dropFirst() + try Self.storageLock.withLock { + for bundle in try self.readAssetBundles().dropFirst() { + let bundleRoot = self.bundleRoot(for: bundle) - for bundle in bundles { - try FileManager.default.removeItem(at: self.bundleRoot(for: bundle)) + if !Self.handedOut.contains(bundleRoot.standardizedFileURL) { + try FileManager.default.removeItem(at: bundleRoot) + } + } } } @@ -302,12 +392,14 @@ public actor EditorAssetLibrary { /// /// - Throws: An error if the storage directory cannot be removed or recreated. public func purge() throws { - guard FileManager.default.directoryExists(at: self.storageRoot) else { - return - } + try Self.storageLock.withLock { + guard FileManager.default.directoryExists(at: self.storageRoot) else { + return + } - try FileManager.default.removeItem(at: self.storageRoot) - try FileManager.default.createDirectory(at: self.storageRoot, withIntermediateDirectories: true) + try FileManager.default.removeItem(at: self.storageRoot) + try FileManager.default.createDirectory(at: self.storageRoot, withIntermediateDirectories: true) + } } // MARK: - File Path Helpers diff --git a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift index 59eaa33e1..a84e81d65 100644 --- a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift +++ b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift @@ -178,6 +178,137 @@ struct EditorServiceTests: MakesTestFixtures { #expect(site.client.downloadCallCount == 2) } + @Test("prepare() uses the bundle on disk without checking the manifest by default") + func prepareUsesBundleOnDiskByDefault() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + let again = try await EditorService( + configuration: site.configuration, + httpClient: site.client, + storageRoot: site.storageRoot, + cacheRoot: site.cacheRoot + ).prepare().assetBundle + + #expect(again == bundle) + #expect(site.manifestRequestCount == 1) + } + + @Test("prepare() under .maxAge checks the manifest only once the last check is older than the age") + func prepareUnderMaxAgeChecksOnceExpired() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + _ = try await site.service(cachePolicy: .maxAge(3600)).prepare() + #expect(site.manifestRequestCount == 1) + + try site.setLastManifestCheck(of: bundle, to: Date(timeIntervalSinceNow: -7200)) + let checked = try await site.service(cachePolicy: .maxAge(3600)).prepare().assetBundle + #expect(site.manifestRequestCount == 2) + #expect(checked == bundle) + + // The check started the bundle's age over + _ = try await site.service(cachePolicy: .maxAge(3600)).prepare() + #expect(site.manifestRequestCount == 2) + #expect(site.client.downloadCallCount == 1) + } + + @Test("a manifest check that fails fails prepare(), and leaves what's on disk for later editors") + func failedManifestCheckLeavesDiskUntouched() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + site.isOffline = true + await #expect(throws: URLError.self) { + try await site.service(cachePolicy: .ignore).prepare() + } + + #expect(try await site.service(cachePolicy: .always).prepare().assetBundle == bundle) + #expect(try await site.service(cachePolicy: .always).fetchAssetBundleCount() == 1) + } + + @Test("cleanup() keeps a superseded bundle that dependencies prepared earlier still use") + func cleanupKeepsBundleStillInUse() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let held = try await site.service(cachePolicy: .always).prepare() + + site.manifest = Self.pluginManifest(version: "2") + let refreshed = try await site.service(cachePolicy: .ignore).prepare() + try await site.service(cachePolicy: .always).cleanup() + + #expect(refreshed.assetBundle.id != held.assetBundle.id) + #expect((try? held.assetBundle.getEditorRepresentation() as EditorAssetBundle.EditorRepresentation) != nil) + } + + @Test("with automatic fallback, a refresh that can't reach the site returns what's on disk") + func failedRefreshFallsBackToDisk() async throws { + let configuration = makeConfiguration().toBuilder().setNetworkFallbackMode(.automatic).build() + let site = TestSite(configuration: configuration, manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.isOffline = true + let refreshed = try await site.service(cachePolicy: .ignore).prepare() + + #expect(refreshed.assetBundle.assetCount == 1) + #expect(refreshed == prepared) + } + + @Test("with automatic fallback, prepare() returns empty dependencies when offline with nothing on disk") + func offlineWithNothingOnDiskReturnsEmptyDependencies() async throws { + let configuration = makeConfiguration().toBuilder().setNetworkFallbackMode(.automatic).build() + let site = TestSite(configuration: configuration, manifest: Self.pluginManifest(version: "1")) + site.isOffline = true + + let dependencies = try await site.service(cachePolicy: .ignore).prepare() + + #expect(dependencies.editorSettings == .undefined) + #expect(dependencies.assetBundle.assetCount == 0) + #expect(dependencies.preloadList == nil) + } + + // MARK: - Automatic Cleanup + + @Test("a site's old bundles are cleaned up, whichever site was prepared first that day") + func automaticCleanupRunsPerSite() async throws { + let first = TestSite( + configuration: makeConfiguration(siteURL: Self.uniqueSiteURL()), manifest: Self.pluginManifest(version: "1")) + let second = TestSite( + configuration: makeConfiguration(siteURL: Self.uniqueSiteURL()), manifest: Self.pluginManifest(version: "2")) + defer { Self.forgetAutomaticCleanups(of: [first, second]) } + try await plantBundles( + forManifests: [Self.pluginManifest(version: "1"), Self.pluginManifest(version: "2")], + in: second.storageRoot + ) + #expect(try await second.service(cachePolicy: .always).fetchAssetBundleCount() == 2) + + _ = try await first.service(cachePolicy: .always).prepare() + _ = try await second.service(cachePolicy: .always).prepare() + + #expect(try await second.service(cachePolicy: .always).fetchAssetBundleCount() == 1) + } + + // MARK: - Progress Totals + + @Test("prepare() progress never passes its total, and ends on it") + func prepareProgressStaysWithinTotal() async throws { + let scripts = ["a", "b", "c", "d"] + .map { #""# } + .joined() + let site = TestSite( + configuration: makeConfiguration(), + manifest: #"{"scripts":"\#(scripts)","styles":"","allowed_block_types":[]}"# + ) + let tracker = ProgressTracker() + + _ = try await site.service(cachePolicy: .always).prepare { tracker.append($0) } + + let completed = tracker.updates.map(\.completed) + #expect(site.client.downloadCallCount == 4) + #expect(tracker.updates.allSatisfy { $0.completed <= $0.total }) + #expect(tracker.updates.last?.completed == tracker.updates.last?.total) + #expect(completed == completed.sorted()) + } + // MARK: - Test Helpers /// URL-based response handler for EditorService.prepare() tests. @@ -220,7 +351,12 @@ struct EditorServiceTests: MakesTestFixtures { /// What the site's `editor-assets` endpoint answers. var manifest: String { - didSet { serve(manifest) } + didSet { serve() } + } + + /// Whether every request to the site fails as it would with no connection. + var isOffline = false { + didSet { serve() } } var manifestRequestCount: Int { @@ -230,7 +366,7 @@ struct EditorServiceTests: MakesTestFixtures { init(configuration: EditorConfiguration, manifest: String) { self.configuration = configuration self.manifest = manifest - serve(manifest) + serve() } func service(cachePolicy: EditorCachePolicy) -> EditorService { @@ -243,13 +379,39 @@ struct EditorServiceTests: MakesTestFixtures { ) } - private func serve(_ manifest: String) { - client.urlResponseHandler = { url in - url.absoluteString.contains("editor-assets") + /// Makes it look as though the site's manifest was last found to match `bundle` at `date`. + func setLastManifestCheck(of bundle: EditorAssetBundle, to date: Date) throws { + try EditorAssetBundle( + manifest: bundle.manifest, + downloadDate: bundle.downloadDate, + lastCheckedDate: date, + bundleRoot: bundle.bundleRoot + ).writeManifest() + } + + private func serve() { + client.urlResponseHandler = { [manifest, isOffline] url in + guard !isOffline else { throw URLError(.notConnectedToInternet) } + return url.absoluteString.contains("editor-assets") ? Data(manifest.utf8) : EditorServiceTests.editorServiceResponseHandler(url) } } } + + /// A site no other test, and no earlier run, has prepared. + private static func uniqueSiteURL() -> URL { + URL(string: "https://\(UUID().uuidString.lowercased()).example")! + } + + /// Removes the record of when each site's bundles were last cleaned up automatically, which + /// would otherwise outlive the test in the user defaults. + private static func forgetAutomaticCleanups(of sites: [TestSite]) { + let hosts = sites.compactMap { $0.configuration.siteURL.host() } + for key in UserDefaults.standard.dictionaryRepresentation().keys + where key.hasPrefix("once-every-") && hosts.contains(where: key.contains) { + UserDefaults.standard.removeObject(forKey: key) + } + } } /// Answers every request from `respond`, but holds each one until the test releases the caller diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 77a4a114e..5032a5ac4 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -121,13 +121,21 @@ struct EditorAssetLibraryTests { // First, fetch the manifest and create a bundle on disk let originalManifest = try await library.fetchManifest() - _ = try await library.buildBundle(for: originalManifest) + let bundle = try await library.buildBundle(for: originalManifest) + + // Mark the copy on disk, so that it can be told apart from the response parsed again + let manifestPath = await library.bundleManifestPath(for: bundle) + var stored = try #require(try JSONSerialization.jsonObject(with: Data(contentsOf: manifestPath)) as? [String: Any]) + var storedManifest = try #require(stored["manifest"] as? [String: Any]) + storedManifest["allowedBlockTypes"] = ["only/on-disk"] + stored["manifest"] = storedManifest + try JSONSerialization.data(withJSONObject: stored).write(to: manifestPath, options: .atomic) // Now fetch again - should return the on-disk manifest let cachedManifest = try await library.fetchManifest() - // The checksums should match since it's the same manifest data #expect(cachedManifest.checksum == originalManifest.checksum) + #expect(cachedManifest.allowedBlockTypes == ["only/on-disk"]) // Verify we made 2 HTTP calls (one for each fetchManifest) // but the second one used the cached bundle's manifest @@ -401,6 +409,161 @@ struct EditorAssetLibraryTests { #expect(try await library.readAssetBundles().map(\.id) == [original.id, changed.id]) } + @Test("downloadAssetBundle leaves a bundle whose manifest hasn't changed exactly as it was") + func downloadAssetBundleLeavesUnchangedBundleAsItWas() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let bundle = try #require(try await library.readAssetBundles().first) + + let checked = try await library.downloadAssetBundle() + + // A host comparing dependencies sees no change, and the date still says when it was downloaded + #expect(checked == bundle) + #expect(checked.downloadDate == bundle.downloadDate) + #expect(try await library.readAssetBundles() == [bundle]) + } + + @Test("downloadAssetBundle downloads an asset that an earlier build of the bundle failed to") + func downloadAssetBundleRepairsMissingAsset() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + let gapped = try await library.downloadAssetBundle() + #expect(!gapped.hasAssetData(for: Self.scriptURL)) + + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let repaired = try await library.downloadAssetBundle() + + #expect(repaired.id == gapped.id) + #expect(repaired.hasAssetData(for: Self.scriptURL)) + #expect(mockClient.downloadCallCount == 2) + } + + @Test( + "a manifest check asks the site afresh, rather than taking a stored response or a request in flight", + arguments: [EditorCachePolicy.ignore, .maxAge(3600)] + ) + func manifestCheckAsksAfresh(cachePolicy: EditorCachePolicy) async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: cachePolicy) + + _ = try await library.downloadAssetBundle() + + #expect(mockClient.requests.last?.cachePolicy == .reloadIgnoringLocalCacheData) + } + + @Test("under .always, a manifest request can still be shared with one in flight") + func manifestRequestIsSharableUnderAlways() async throws { + let (_, mockClient) = try await makeLibraryWithBundle(cachePolicy: .always) + + #expect(mockClient.requests.last?.cachePolicy == .useProtocolCachePolicy) + } + + @Test("downloadAssetBundle builds a bundle again if another service's cleanup deletes it mid-check") + func downloadAssetBundleRebuildsBundleCleanedUpDuringCheck() async throws { + let storageRoot = URL.randomTemporaryDirectory + let planted = try await plantBundles( + forManifests: [Self.manifestJSON(scriptVersion: "1"), Self.manifestJSON(scriptVersion: "2")], + in: storageRoot + ) + let mockClient = EditorAssetLibraryMockHTTPClient() + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + let other = makeLibrary(storageRoot: storageRoot) + + // The site is back on the older manifest. Another service's cleanup removes that manifest's + // bundle — not yet the latest — after the check has found it on disk. + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let restored = try await library.downloadAssetBundle { _ in try? await other.cleanup() } + + #expect(restored.id == planted[0].id) + #expect((try? restored.getEditorRepresentation() as EditorAssetBundle.EditorRepresentation) != nil) + #expect(restored.hasAssetData(for: Self.scriptURL)) + #expect(try await library.readAssetBundles().first?.id == restored.id) + } + + @Test("downloadAssetBundle builds a bundle again if a purge deletes it mid-check") + func downloadAssetBundleRebuildsBundlePurgedDuringCheck() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let bundle = try #require(try await library.readAssetBundles().first) + + let checked = try await library.downloadAssetBundle { _ in try? await library.purge() } + + #expect(checked.id == bundle.id) + #expect((try? checked.getEditorRepresentation() as EditorAssetBundle.EditorRepresentation) != nil) + #expect(checked.hasAssetData(for: Self.scriptURL)) + #expect(try await library.readAssetBundles().map(\.id) == [bundle.id]) + } + + @Test("a bundle stored before checks were recorded is as old as its download") + func bundleWithoutLastCheckedDateUsesDownloadDate() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(3600)) + let bundle = try #require(try await library.readAssetBundles().first) + let manifestPath = await library.bundleManifestPath(for: bundle) + + // What an earlier version wrote: the manifest and a download date, and nothing else + func store(downloadedAgo interval: TimeInterval) throws { + let stored: [String: Any] = [ + "manifest": try JSONSerialization.jsonObject(with: JSONEncoder().encode(bundle.manifest)), + "downloadDate": Date(timeIntervalSinceNow: -interval).timeIntervalSinceReferenceDate + ] + try JSONSerialization.data(withJSONObject: stored).write(to: manifestPath, options: .atomic) + } + + try store(downloadedAgo: 1800) + #expect(try await library.readLatestAssetBundle()?.id == bundle.id) + + try store(downloadedAgo: 7200) + #expect(try await library.readAssetBundles().first?.lastCheckedDate == nil) + #expect(try await library.readLatestAssetBundle() == nil) + } + + // MARK: - cleanup Tests + + @Test("cleanup removes the bundles an earlier launch left behind, except the newest") + func cleanupRemovesBundlesFromEarlierLaunch() async throws { + let storageRoot = URL.randomTemporaryDirectory + let planted = try await plantBundles( + forManifests: ["1", "2", "3"].map(Self.manifestJSON(scriptVersion:)), + in: storageRoot + ) + let library = makeLibrary(storageRoot: storageRoot) + + try await library.cleanup() + + #expect(try await library.readAssetBundles().map(\.id) == [planted[2].id]) + } + + @Test("cleanup keeps the bundle the site's manifest last matched, though another was downloaded later") + func cleanupKeepsLatestBundleWhateverItsDownloadDate() async throws { + let storageRoot = URL.randomTemporaryDirectory + let planted = try await plantBundles( + forManifests: [Self.manifestJSON(scriptVersion: "1"), Self.manifestJSON(scriptVersion: "2")], + in: storageRoot + ) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + + _ = try await library.downloadAssetBundle() + try await library.cleanup() + + #expect(try await library.readAssetBundles().map(\.id) == [planted[0].id]) + } + + @Test("cleanup keeps a bundle handed out since launch, and purge removes it anyway") + func cleanupKeepsHandedOutBundles() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + _ = try await library.downloadAssetBundle() + + try await library.cleanup() + #expect(try await library.readAssetBundles().count == 2) + + try await library.purge() + #expect(try await library.readAssetBundles().isEmpty) + } + /// A library whose storage holds one bundle, built from ``manifestJSON(scriptVersion:)`` with version `1`. private func makeLibraryWithBundle( cachePolicy: EditorCachePolicy @@ -427,11 +590,15 @@ struct EditorAssetLibraryTests { { url in url.path.contains("editor-assets") ? Data(manifestJSON.utf8) : Data("mock content".utf8) } } - /// Rewrites `bundle`'s download date on disk to `interval` seconds ago. + /// The script in ``manifestJSON(scriptVersion:)`` with version `1`. + private static let scriptURL = URL(string: "https://example.com/plugin.js?ver=1")! + + /// Makes it `interval` seconds since the site's manifest was last found to match `bundle`. private func backdate(_ bundle: EditorAssetBundle, by interval: TimeInterval) throws { try EditorAssetBundle( manifest: bundle.manifest, - downloadDate: Date(timeIntervalSinceNow: -interval), + downloadDate: bundle.downloadDate, + lastCheckedDate: Date(timeIntervalSinceNow: -interval), bundleRoot: bundle.bundleRoot ).writeManifest() } @@ -967,10 +1134,15 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck var downloadedURLs: [URL] = [] private let lock = NSLock() + /// Requests made via `perform(_:)`, in order. + private var _requests: [URLRequest] = [] + var requests: [URLRequest] { + lock.withLock { _requests } + } + /// URLs requested via `perform(_:)`. Use this to verify which endpoints were called. - private var _requestedURLs: [URL] = [] var requestedURLs: [URL] { - lock.withLock { _requestedURLs } + requests.compactMap(\.url) } /// Handler for generating response data based on request URL. @@ -983,7 +1155,7 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck lock.withLock { getCallCount += 1 - _requestedURLs.append(url) + _requests.append(urlRequest) } let responseData = try urlResponseHandler(url) diff --git a/ios/Tests/GutenbergKitTests/TestHelpers.swift b/ios/Tests/GutenbergKitTests/TestHelpers.swift index 618abf431..e60e9ec92 100644 --- a/ios/Tests/GutenbergKitTests/TestHelpers.swift +++ b/ios/Tests/GutenbergKitTests/TestHelpers.swift @@ -26,6 +26,32 @@ func jsonResource(named name: String) throws -> String { String(data: try jsonResource(named: name), encoding: .utf8)! } +/// Puts a bundle for each manifest under `storageRoot` the way an earlier launch would have left +/// them: complete on disk, the last the latest, but not handed out by this process. +@discardableResult +func plantBundles( + forManifests manifests: [String], + in storageRoot: URL, + configuration: EditorConfiguration = EditorAssetLibraryTests.testConfiguration +) async throws -> [EditorAssetBundle] { + let scratchRoot = URL.randomTemporaryDirectory + let client = EditorAssetLibraryMockHTTPClient() + let library = EditorAssetLibrary(configuration: configuration, httpClient: client, storageRoot: scratchRoot) + try FileManager.default.createDirectory(at: storageRoot, withIntermediateDirectories: true) + + var planted: [EditorAssetBundle] = [] + for manifest in manifests { + client.urlResponseHandler = { url in + url.path.contains("editor-assets") ? Data(manifest.utf8) : Data("mock content".utf8) + } + let bundle = try await library.downloadAssetBundle() + let destination = storageRoot.appending(path: bundle.id) + try FileManager.default.moveItem(at: scratchRoot.appending(path: bundle.id), to: destination) + planted.append(try EditorAssetBundle(url: destination.appending(path: "manifest.json"))) + } + return planted +} + protocol MakesTestFixtures { static var testSiteURL: URL { get } static var testApiRoot: URL { get } From 1163f845bba567ab5529087c2bd0e2bab4e9606a Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:54:10 -0600 Subject: [PATCH 4/9] fix(ios): download every asset under .ignore, and only changed ones otherwise `.ignore` checked the manifest every time, but kept the bundle on disk when the manifest hadn't changed. An asset that changed without its URL changing, or one that was stored wrong, could then only be replaced by a `purge()`, and the policy that takes nothing cached as valid still trusted every cached asset. It now downloads every asset, whether or not the manifest has changed. What it downloads goes into a new bundle, in a directory of its own. A bundle on disk is never changed, because an editor may be reading it. So one manifest can now have more than one bundle: a lookup by checksum returns the one the manifest last matched, and the library takes a bundle's location from the bundle rather than from its checksum. A build under `.ignore` doesn't join a build in flight either, since no other build writes its directory. Two things keep that from costing more than it should: - If the manifest hasn't changed and every asset comes back identical to the bundle on disk, that bundle is returned and the new one is discarded. A host comparing dependencies sees no change, and a refresh that changed nothing takes no more disk space. - An asset that fails to download is copied from the latest bundle on disk, if that has it, so a refresh over a poor connection doesn't leave a gap the last bundle didn't have. Under `.always` and `.maxAge`, a changed manifest downloaded every asset again. Its bundle now takes each asset whose URL is the same as in the latest bundle on disk from that bundle, and downloads the rest. Only a URL with a `ver` is carried. One without can change without its URL saying so, and carrying it would leave only `.ignore` to download it again. `.maxAge(0)` is now the policy for checking a site and downloading only what changed, and the tests of a check that keeps its bundle move from `.ignore` to it. A build with nothing to download now reports progress once. --- docs/code/preloading.md | 10 +- .../Sources/Services/EditorService.swift | 3 +- .../Sources/Stores/EditorAssetLibrary.swift | 194 +++++++++++--- .../Services/EditorServiceTests.swift | 16 +- .../Stores/EditorAssetLibraryTests.swift | 241 +++++++++++++++++- 5 files changed, 408 insertions(+), 56 deletions(-) diff --git a/docs/code/preloading.md b/docs/code/preloading.md index e56a717a8..fa09952c8 100644 --- a/docs/code/preloading.md +++ b/docs/code/preloading.md @@ -253,15 +253,17 @@ try await service.purge() ### Refreshing -An `EditorService`'s cache policy covers plugin and theme assets as well as API responses. For assets, it decides when to check the site's asset manifest again: +An `EditorService`'s cache policy covers plugin and theme assets as well as API responses. For assets, it decides when to check the site's asset manifest again, and how much to download when it does: | Policy | API responses | Asset bundle | | ----------------------- | ------------------------------- | ---------------------------------------------------------- | | `.always` (default) | Fetched only when not cached | Manifest checked only when no bundle is on disk | | `.maxAge(TimeInterval)` | Fetched once older than the age | Manifest checked once the last check is older than the age | -| `.ignore` | Always fetched | Manifest always checked | +| `.ignore` | Always fetched | Manifest always checked, and every asset downloaded again | -If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. Only an asset that failed to download when the bundle was built is tried again. If the manifest has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. +Under `.always` and `.maxAge`, a check downloads only what the manifest says has changed. If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. Only an asset that failed to download when the bundle was built is tried again. If the manifest has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. An asset whose versioned URL is the same as in the latest bundle on disk is copied from that bundle. An asset whose URL has no `?ver=` is downloaded again, because its URL can't say whether it changed. + +Under `.ignore`, nothing on disk is taken to be valid, so every asset is downloaded whether or not the manifest has changed. That makes it the way to replace an asset that changed without its URL changing, or one that was stored wrong. The assets go into a new bundle beside any the manifest already has: a bundle on disk is never changed, because an editor may be reading it. An asset that fails to download is taken from the latest bundle on disk, if that has it, rather than left out. If the manifest hasn't changed and its assets all come back the same as the bundle on disk has them, that bundle is returned and the new one is discarded, so a host can tell whether a refresh changed anything, and a refresh that changed nothing takes no more disk space. The old bundle stays on disk for as long as the app is running, because an open editor — or dependencies the host prepared earlier and still holds — may be reading it. `cleanup()` removes it after the next launch. @@ -273,7 +275,7 @@ To refresh a site's editor data — on pull-to-refresh, for instance — prepare let dependencies = try await EditorService(configuration: configuration, cachePolicy: .ignore).prepare() ``` -Nothing is deleted first, so an editor opened during the refresh still loads straight from what's on disk, and a refresh that fails leaves it all in place. An editor given no dependencies prepares its own with `.always`, so it uses whatever the last refresh left. To download assets again even when their manifest hasn't changed, `purge()` instead, at the cost of a cold load for the next editor. +Nothing is deleted first, so an editor opened during the refresh still loads straight from what's on disk, and a refresh that fails leaves it all in place. An editor given no dependencies prepares its own with `.always`, so it uses whatever the last refresh left. A refresh downloads every asset again; to check for changes and download only those, use `.maxAge(0)` instead. A refresh that can't reach the site throws. If the configuration's `networkFallbackMode` is `.automatic`, it returns the dependencies already on disk instead — however old they are — so they're still safe to give to the next editor. It returns empty dependencies only when something the editor needs has never been cached. diff --git a/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift b/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift index 342223f0b..011e4fb11 100644 --- a/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift +++ b/ios/Sources/GutenbergKit/Sources/Services/EditorService.swift @@ -65,7 +65,8 @@ public actor EditorService { /// a time interval, or `.always` (the default) to use cached data regardless of age. /// This policy applies to both API responses and plugin and theme assets. For assets, it /// decides when to check the site's asset manifest again; an unchanged manifest keeps the - /// bundle already on disk rather than downloading its assets again. + /// bundle already on disk rather than downloading its assets again. `.ignore` is the + /// exception: it downloads every asset again whether or not the manifest has changed. public init( configuration: EditorConfiguration, httpClient: (any EditorHTTPClientProtocol)? = nil, diff --git a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift index 47329508e..f6847a6ce 100644 --- a/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift +++ b/ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift @@ -29,9 +29,9 @@ public actor EditorAssetLibrary { /// - httpClient: The HTTP client used to fetch remote assets. /// - cachePolicy: The policy that determines how long ``readLatestAssetBundle()`` goes on /// returning the latest bundle on disk before the site's manifest has to be checked again. - /// Use `.ignore` to check it every time, `.maxAge(_:)` to check it once the last check is - /// older than a time interval, or `.always` (the default) to check it only when there is - /// no bundle on disk. + /// Use `.ignore` to check it every time and download every asset again, `.maxAge(_:)` to + /// check it once the last check is older than a time interval, or `.always` (the default) + /// to check it only when there is no bundle on disk. /// - storageRoot: The root directory where asset bundles will be stored on disk. public init( configuration: EditorConfiguration, @@ -109,19 +109,26 @@ public actor EditorAssetLibrary { } return Self.storageLock.withLock { - guard self.hasBundle(forManifestChecksum: latestBundle.id) else { return nil } - Self.handedOut.insert(self.bundleRoot(for: latestBundle).standardizedFileURL) + guard self.hasBundle(at: latestBundle.bundleRoot) else { return nil } + Self.handedOut.insert(latestBundle.bundleRoot.standardizedFileURL) return latestBundle } } /// Fetches the latest manifest from the server and downloads all of its resources, caching them on-disk. /// - /// If a bundle built from the same manifest is already on disk, it's returned instead, without downloading its - /// assets again: they're versioned by URL, so an unchanged manifest means unchanged assets. Only an asset that - /// an earlier build failed to download is tried again. Either way the bundle becomes the site's latest, and - /// its age for the cache policy starts over. To download every asset again regardless, ``purge()`` the - /// library first. + /// Under `.always` and `.maxAge`, only what the manifest says has changed is downloaded. If a bundle built + /// from the same manifest is already on disk, it's returned instead, without downloading its assets again: + /// they're versioned by URL, so an unchanged manifest means unchanged assets. Only an asset that an earlier + /// build failed to download is tried again. If the manifest has changed, the new bundle takes each asset + /// whose versioned URL (`?ver=`) hasn't changed from the latest bundle on disk, and downloads the rest. + /// + /// Under `.ignore`, every asset is downloaded whether or not the manifest has changed, into a new bundle: a + /// bundle already on disk is never changed, because an editor may be reading it. An asset that fails to + /// download is taken from the latest bundle on disk, if that has it. If the manifest hasn't changed and its + /// assets all come back the same as the bundle on disk has them, that bundle is returned instead. + /// + /// Either way the bundle becomes the site's latest, and its age for the cache policy starts over. /// /// - Parameter progress: An optional callback that receives progress updates as assets are downloaded. /// - Returns: The downloaded `EditorAssetBundle` containing all cached assets. @@ -142,10 +149,14 @@ public actor EditorAssetLibrary { } /// Checks whether a complete bundle with the given manifest checksum exists on disk. + func hasBundle(forManifestChecksum checksum: String) -> Bool { + self.existingBundle(forManifestChecksum: checksum) != nil + } + + /// Checks whether there is a complete bundle at `bundleRoot`. /// /// A bundle is considered complete only if both `manifest.json` and `editor-representation.json` exist. - func hasBundle(forManifestChecksum checksum: String) -> Bool { - let bundleRoot = self.bundleRoot(for: checksum) + private func hasBundle(at bundleRoot: URL) -> Bool { let manifestExists = FileManager.default.fileExists(atPath: bundleRoot.appending(path: "manifest.json").path) let editorRepExists = FileManager.default.fileExists(atPath: bundleRoot.appending(path: "editor-representation.json").path) return manifestExists && editorRepExists @@ -153,22 +164,22 @@ public actor EditorAssetLibrary { /// Retrieves an existing bundle from disk if one exists for the given manifest checksum. /// + /// A manifest can have more than one: each download under `.ignore` that changes a manifest's assets leaves + /// a bundle of its own. This is the one the site's manifest last matched. func existingBundle(forManifestChecksum checksum: String) -> EditorAssetBundle? { - guard self.hasBundle(forManifestChecksum: checksum) else { - return nil - } - - return try? EditorAssetBundle(url: self.bundleManifestPath(for: checksum)) + try? self.readAssetBundles().first { $0.id == checksum } } // MARK: - Individual Asset Handling /// Downloads all of the assets for a given manifest and assembles them into a bundle. /// - /// Assets are downloaded concurrently and stored in a temporary directory. Once all downloads - /// complete successfully, the bundle is atomically moved to its final location. If a complete - /// bundle for the manifest is already there, it's returned instead, once any asset it's missing - /// has been tried again. Either way, the bundle is marked as the site's latest. + /// Unless the cache policy is `.ignore`, assets the site's latest bundle already has are copied + /// from it. The rest are downloaded concurrently, all into a temporary directory. Once all + /// downloads complete successfully, the bundle is atomically moved to its final location. If a + /// complete bundle for the manifest is already there, it's returned instead, once any asset it's + /// missing has been tried again. Under `.ignore`, the bundle is built again regardless, in a + /// directory of its own. Either way, the bundle is marked as the site's latest. func buildBundle( for manifest: LocalEditorAssetManifest, progress: EditorProgressCallback? = nil @@ -180,6 +191,10 @@ public actor EditorAssetLibrary { return .empty } + if case .ignore = self.cachePolicy { + return try await self.buildFreshBundle(for: manifest) { await progress?($0) } + } + // Every build of one manifest writes the same directory, whichever library runs it: // join a build in flight rather than race a second one into it. let destination = self.bundleRoot(for: manifest.checksum).standardizedFileURL @@ -197,18 +212,59 @@ public actor EditorAssetLibrary { } let bundle = try await self.build(manifest, reportingTo: report) + .copy(to: self.bundleRoot(for: manifest.checksum)) return await self.markLatest(bundle) ?? bundle } } - /// Downloads whichever of `bundle`'s assets aren't on disk. A build publishes its bundle without any asset - /// that fails to download, and an unchanged manifest would otherwise never give that asset another try. + /// Builds a bundle for `manifest` with every asset downloaded now, which is what `.ignore` asks for. + /// + /// The bundle goes in a directory of its own rather than over the one the manifest already has: a bundle on + /// disk is never changed, because an editor may be reading it. It has no build to join for the same reason + /// — no other build writes its directory. If the assets all come back the same as the manifest's bundle on + /// disk has them, that bundle is still right, so it's returned instead and the new one is discarded. That + /// keeps a refresh that changed nothing from costing disk space, or looking like a change to a host + /// comparing dependencies. + private func buildFreshBundle( + for manifest: LocalEditorAssetManifest, + reportingTo progress: EditorProgressCallback + ) async throws -> EditorAssetBundle { + let freshBundle = try await self.build(manifest, reportingTo: progress) + + if let existingBundle = self.existingBundle(forManifestChecksum: manifest.checksum), + self.hasSameAssets(freshBundle, as: existingBundle), + let latestBundle = self.markLatest(existingBundle) { + try? FileManager.default.removeItem(at: freshBundle.bundleRoot) + return latestBundle + } + + let directory = "\(manifest.checksum)-\(UUID().uuidString)" + let bundle = try freshBundle.copy(to: self.storageRoot.appending(path: directory)) + return self.markLatest(bundle) ?? bundle + } + + /// Whether two bundles of one manifest hold the same assets: each either missing from both, or identical. + private func hasSameAssets(_ bundle: EditorAssetBundle, as other: EditorAssetBundle) -> Bool { + self.downloadableAssets(in: bundle.manifest).allSatisfy { asset in + let path = self.assetPath(for: asset, in: bundle).path + let otherPath = self.assetPath(for: asset, in: other).path + + guard FileManager.default.fileExists(atPath: path) else { + return !FileManager.default.fileExists(atPath: otherPath) + } + + return FileManager.default.contentsEqual(atPath: path, andPath: otherPath) + } + } + + /// Downloads whichever of `bundle`'s assets aren't on disk. For a bundle being built, that's every asset + /// that wasn't carried forward. For one already published, it's any asset that failed to download when the + /// bundle was built, which an unchanged manifest would otherwise never give another try. private func downloadMissingAssets( of bundle: EditorAssetBundle, reportingTo progress: EditorProgressCallback ) async throws { - let missingAssets = self.downloadableAssets(in: bundle.manifest) - .filter { !FileManager.default.fileExists(at: self.assetPath(for: $0, in: bundle)) } + let missingAssets = self.missingAssets(of: bundle) guard !missingAssets.isEmpty else { await progress(EditorProgress(completed: 1, total: 1)) @@ -218,16 +274,22 @@ public actor EditorAssetLibrary { try await self.downloadAssets(missingAssets, into: bundle, reportingTo: progress) } + /// The assets `bundle` should have that aren't on disk. + private func missingAssets(of bundle: EditorAssetBundle) -> [URL] { + self.downloadableAssets(in: bundle.manifest) + .filter { !FileManager.default.fileExists(at: self.assetPath(for: $0, in: bundle)) } + } + /// Records that the site's manifest matches `bundle` now, and that the bundle has been handed out. That makes /// it the site's latest bundle, and starts its age for the cache policy over. Returns `nil` if the bundle is /// no longer on disk. private func markLatest(_ bundle: EditorAssetBundle) -> EditorAssetBundle? { Self.storageLock.withLock { - guard self.hasBundle(forManifestChecksum: bundle.id) else { + guard self.hasBundle(at: bundle.bundleRoot) else { return nil } - Self.handedOut.insert(self.bundleRoot(for: bundle).standardizedFileURL) + Self.handedOut.insert(bundle.bundleRoot.standardizedFileURL) do { let latestBundle = try EditorAssetBundle( @@ -238,7 +300,8 @@ public actor EditorAssetLibrary { ) // Written straight to the file, which is known to be there: `writeManifest()` would // create the bundle's directory if it weren't. - try latestBundle.dataRepresentation().write(to: self.bundleManifestPath(for: bundle), options: .atomic) + try latestBundle.dataRepresentation() + .write(to: self.bundleManifestPath(relativeTo: bundle.bundleRoot), options: .atomic) return latestBundle } catch { // The bundle is still complete and correct; it'll just be checked again sooner. @@ -248,6 +311,7 @@ public actor EditorAssetLibrary { } } + /// Assembles a bundle for `manifest` in a temporary directory, for the caller to put in its place. private func build( _ manifest: LocalEditorAssetManifest, reportingTo progress: EditorProgressCallback @@ -262,9 +326,51 @@ public actor EditorAssetLibrary { let editorRepresentation = try manifest.buildEditorRepresentation(for: self.configuration) try bundle.writeManifest(editorRepresentation: editorRepresentation) - try await self.downloadAssets(self.downloadableAssets(in: manifest), into: bundle, reportingTo: progress) + switch self.cachePolicy { + case .always, .maxAge: + // Only a versioned URL is taken to mean the same file. One without a version can change + // without its URL saying so, and carrying it forward would leave only `.ignore` to + // download it again. + self.carryForward(self.downloadableAssets(in: manifest).filter { self.isVersioned($0) }, into: bundle) + try await self.downloadMissingAssets(of: bundle, reportingTo: progress) + case .ignore: + // Takes nothing on disk as valid, so every asset is downloaded. One that fails to + // download has no better copy than the one in use until now, which beats a gap. + try await self.downloadMissingAssets(of: bundle, reportingTo: progress) + self.carryForward(self.missingAssets(of: bundle), into: bundle) + } - return try bundle.copy(to: self.bundleRoot(for: bundle)) + return bundle + } + + /// Copies into `bundle` each of `assets` that the site's latest bundle has under the same URL. + private func carryForward(_ assets: [URL], into bundle: EditorAssetBundle) { + guard !assets.isEmpty, let latestBundle = try? self.readAssetBundles().first else { return } + + let latestAssets = Set(self.downloadableAssets(in: latestBundle.manifest)) + + for asset in assets where latestAssets.contains(asset) { + let destination = self.assetPath(for: asset, in: bundle) + + do { + try FileManager.default.createDirectory( + at: destination.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try FileManager.default.copyItem(at: self.assetPath(for: asset, in: latestBundle), to: destination) + } catch { + // The latest bundle never downloaded it, or has been removed since. Leave nothing + // behind, so that it reads as missing. + try? FileManager.default.removeItem(at: destination) + } + } + } + + /// Whether `url` carries a version the way WordPress adds one: a `ver` in its query. + private func isVersioned(_ url: URL) -> Bool { + URLComponents(url: url, resolvingAgainstBaseURL: false)? + .queryItems? + .contains { $0.name == "ver" && $0.value?.isEmpty == false } ?? false } /// The assets in `manifest` that belong in its bundle. @@ -312,7 +418,7 @@ public actor EditorAssetLibrary { @discardableResult private func fetchAsset(url: URL, into bundle: EditorAssetBundle) async throws -> URL { let tempUrl = try await logExecutionTime("Downloading \(url.lastPathComponent)") { - try await httpClient.download(URLRequest(method: .GET, url: url)).0 + try await httpClient.download(self.assetRequest(for: url)).0 } let destinationPath = self.assetPath(for: url, in: bundle) @@ -326,6 +432,15 @@ public actor EditorAssetLibrary { return destinationPath } + /// The request for the asset at `url`. Under `.ignore`, it asks for a stored response not to be used. + private func assetRequest(for url: URL) -> URLRequest { + var request = URLRequest(method: .GET, url: url) + if case .ignore = self.cachePolicy { + request.cachePolicy = .reloadIgnoringLocalCacheData + } + return request + } + /// Where the asset at `url` is stored in `bundle`. private func assetPath(for url: URL, in bundle: EditorAssetBundle) -> URL { bundle.bundleRoot.appending(path: url.path(percentEncoded: false)) @@ -375,12 +490,9 @@ public actor EditorAssetLibrary { /// - Throws: An error if the list of bundles cannot be read, or any bundle cannot be removed. public func cleanup() throws { try Self.storageLock.withLock { - for bundle in try self.readAssetBundles().dropFirst() { - let bundleRoot = self.bundleRoot(for: bundle) - - if !Self.handedOut.contains(bundleRoot.standardizedFileURL) { - try FileManager.default.removeItem(at: bundleRoot) - } + for bundle in try self.readAssetBundles().dropFirst() + where !Self.handedOut.contains(bundle.bundleRoot.standardizedFileURL) { + try FileManager.default.removeItem(at: bundle.bundleRoot) } } } @@ -403,17 +515,19 @@ public actor EditorAssetLibrary { } // MARK: - File Path Helpers + /// Where `bundle` is on disk. Not derived from its checksum: a manifest can have more than one bundle. func bundleRoot(for bundle: EditorAssetBundle) -> URL { - assert(!bundle.id.isEmpty, "Bundle must have a valid ID") - return self.bundleRoot(for: bundle.id) + bundle.bundleRoot } + /// Where a manifest's bundle goes, unless it's built under `.ignore`, which gives each build a directory + /// of its own. func bundleRoot(for checksum: String) -> URL { self.storageRoot.appending(path: checksum) } func bundleManifestPath(for bundle: EditorAssetBundle) -> URL { - bundleManifestPath(relativeTo: self.bundleRoot(for: bundle)) + bundleManifestPath(relativeTo: bundle.bundleRoot) } func bundleManifestPath(relativeTo path: URL) -> URL { diff --git a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift index a84e81d65..c8e8a049e 100644 --- a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift +++ b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift @@ -152,8 +152,8 @@ struct EditorServiceTests: MakesTestFixtures { #expect(site.client.downloadCallCount == 1) } - @Test("prepare() under .ignore checks the manifest, and keeps the bundle when it hasn't changed") - func prepareUnderIgnoreKeepsUnchangedBundle() async throws { + @Test("prepare() under .ignore downloads the bundle's assets again, even when its manifest hasn't changed") + func prepareUnderIgnoreDownloadsUnchangedBundleAgain() async throws { let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle @@ -161,6 +161,18 @@ struct EditorServiceTests: MakesTestFixtures { #expect(refreshed.id == bundle.id) #expect(site.manifestRequestCount == 2) + #expect(site.client.downloadCallCount == 2) + } + + @Test("prepare() under .maxAge checks the manifest once it's due, and keeps the bundle when it hasn't changed") + func prepareUnderMaxAgeKeepsUnchangedBundle() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let bundle = try await site.service(cachePolicy: .always).prepare().assetBundle + + let checked = try await site.service(cachePolicy: .maxAge(0)).prepare().assetBundle + + #expect(checked.id == bundle.id) + #expect(site.manifestRequestCount == 2) #expect(site.client.downloadCallCount == 1) } diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 5032a5ac4..4fb5f8e26 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -396,7 +396,7 @@ struct EditorAssetLibraryTests { @Test("downloadAssetBundle makes the bundle for a manifest the site went back to the newest again") func downloadAssetBundleRestoresReturningBundle() async throws { - let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .maxAge(0)) let original = try #require(try await library.readAssetBundles().first) mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) let changed = try await library.downloadAssetBundle() @@ -409,9 +409,12 @@ struct EditorAssetLibraryTests { #expect(try await library.readAssetBundles().map(\.id) == [original.id, changed.id]) } - @Test("downloadAssetBundle leaves a bundle whose manifest hasn't changed exactly as it was") - func downloadAssetBundleLeavesUnchangedBundleAsItWas() async throws { - let (library, _) = try await makeLibraryWithBundle(cachePolicy: .ignore) + @Test( + "downloadAssetBundle leaves a bundle exactly as it was when neither its manifest nor its assets have changed", + arguments: [EditorCachePolicy.maxAge(0), .ignore] + ) + func downloadAssetBundleLeavesUnchangedBundleAsItWas(cachePolicy: EditorCachePolicy) async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: cachePolicy) let bundle = try #require(try await library.readAssetBundles().first) let checked = try await library.downloadAssetBundle() @@ -422,6 +425,76 @@ struct EditorAssetLibraryTests { #expect(try await library.readAssetBundles() == [bundle]) } + @Test("under .ignore, assets that come back different go into a new bundle, and the one on disk is left as it was") + func downloadAssetBundleBuildsNewBundleForChangedAssetsUnderIgnore() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let bundle = try #require(try await library.readAssetBundles().first) + + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1"), + assetContent: "new content" + ) + let refreshed = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == 2) + #expect(try refreshed.assetData(for: Self.scriptURL) == Data("new content".utf8)) + // An editor may still be reading the bundle it was given + #expect(try bundle.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + // Two bundles for one manifest, which a host comparing dependencies can tell apart + #expect(refreshed.id == bundle.id) + #expect(refreshed != bundle) + #expect(try await library.readAssetBundles().map(\.bundleRoot) == [refreshed.bundleRoot, bundle.bundleRoot]) + #expect(await library.existingBundle(forManifestChecksum: bundle.id)?.bundleRoot == refreshed.bundleRoot) + } + + @Test("under .ignore, a new bundle takes the latest bundle's copy of an asset that fails to download again") + func newBundleUnderIgnoreKeepsAssetThatFailsToDownloadAgain() async throws { + let style = "https://example.com/plugin.css" + let manifest = Self.manifestJSON(scriptVersion: "1", style: style) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + let bundle = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifest.utf8) + } + guard url.path != "/plugin.css" else { throw URLError(.timedOut) } + return Data("new content".utf8) + } + let refreshed = try await library.downloadAssetBundle() + + #expect(refreshed != bundle) + #expect(try refreshed.assetData(for: Self.scriptURL) == Data("new content".utf8)) + #expect(try refreshed.assetData(for: URL(string: style)!) == Data("mock content".utf8)) + } + + @Test("under .ignore, downloadAssetBundle downloads the assets again even when they turn out the same") + func downloadAssetBundleDownloadsIdenticalAssetsAgainUnderIgnore() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + + _ = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == 2) + } + + @Test("under .ignore, an asset that fails to download again keeps the copy on disk") + func downloadAssetBundleKeepsAssetThatFailsToDownloadAgain() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let bundle = try #require(try await library.readAssetBundles().first) + + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let refreshed = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == 2) + #expect(refreshed == bundle) + #expect(try refreshed.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + } + @Test("downloadAssetBundle downloads an asset that an earlier build of the bundle failed to") func downloadAssetBundleRepairsMissingAsset() async throws { let mockClient = EditorAssetLibraryMockHTTPClient() @@ -429,7 +502,7 @@ struct EditorAssetLibraryTests { guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } return Data(Self.manifestJSON(scriptVersion: "1").utf8) } - let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) let gapped = try await library.downloadAssetBundle() #expect(!gapped.hasAssetData(for: Self.scriptURL)) @@ -441,6 +514,107 @@ struct EditorAssetLibraryTests { #expect(mockClient.downloadCallCount == 2) } + @Test( + "a changed manifest's bundle takes the assets whose URL hasn't changed from the latest bundle", + arguments: [EditorCachePolicy.always, .maxAge(60)] + ) + func changedBundleCarriesUnchangedAssetsForward(cachePolicy: EditorCachePolicy) async throws { + let style = "https://example.com/plugin.css?ver=1" + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1", style: style)) + let library = makeLibrary(httpClient: mockClient, cachePolicy: cachePolicy) + _ = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + let changedScript = URL(string: "https://example.com/plugin.js?ver=2")! + #expect(mockClient.downloadedURLs.filter { $0.path == "/plugin.css" }.count == 1) + #expect(mockClient.downloadedURLs.contains(changedScript)) + #expect(try changed.assetData(for: URL(string: style)!) == Data("mock content".utf8)) + #expect(try changed.assetData(for: changedScript) == Data("new content".utf8)) + } + + @Test("under .ignore, a changed manifest's bundle downloads every asset, even one whose URL hasn't changed") + func changedBundleDownloadsEveryAssetUnderIgnore() async throws { + let style = "https://example.com/plugin.css?ver=1" + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1", style: style)) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + _ = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + #expect(mockClient.downloadedURLs.filter { $0.path == "/plugin.css" }.count == 2) + #expect(try changed.assetData(for: URL(string: style)!) == Data("new content".utf8)) + } + + @Test( + "a changed manifest's bundle downloads again an asset whose URL carries no version", + arguments: [ + "https://example.com/plugin.css", + "https://example.com/plugin.css?ver=", + "https://example.com/plugin.css?minify=false", + ] + ) + func changedBundleDownloadsUnversionedAssetsAgain(style: String) async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1", style: style)) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + _ = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + #expect(mockClient.downloadedURLs.filter { $0.path == "/plugin.css" }.count == 2) + #expect(try changed.assetData(for: URL(string: style)!) == Data("new content".utf8)) + } + + @Test("a changed manifest's bundle downloads an unchanged asset that the latest bundle is missing") + func changedBundleDownloadsAssetMissingFromLatestBundle() async throws { + let style = "https://example.com/plugin.css?ver=1" + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(Self.manifestJSON(scriptVersion: "1", style: style).utf8) + } + guard url.path != "/plugin.css" else { throw URLError(.timedOut) } + return Data("mock content".utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + let gapped = try await library.downloadAssetBundle() + #expect(!gapped.hasAssetData(for: URL(string: style)!)) + + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2", style: style)) + let changed = try await library.downloadAssetBundle() + + #expect(changed.hasAssetData(for: URL(string: style)!)) + } + + @Test("a changed manifest's bundle whose assets are all carried forward still reports that it's complete") + func changedBundleReportsProgressWhenNothingIsDownloaded() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .maxAge(60)) + let changedManifest = Self.manifestJSON(scriptVersion: "1").replacing("core/paragraph", with: "core/heading") + mockClient.urlResponseHandler = Self.responses(forManifest: changedManifest) + let progressTracker = ProgressTracker() + + let changed = try await library.downloadAssetBundle { progressTracker.append($0) } + + #expect(mockClient.downloadCallCount == 1) + #expect(changed.hasAssetData(for: Self.scriptURL)) + #expect(progressTracker.count == 1) + } + @Test( "a manifest check asks the site afresh, rather than taking a stored response or a request in flight", arguments: [EditorCachePolicy.ignore, .maxAge(3600)] @@ -453,6 +627,13 @@ struct EditorAssetLibraryTests { #expect(mockClient.requests.last?.cachePolicy == .reloadIgnoringLocalCacheData) } + @Test("under .ignore, an asset is asked for afresh rather than taken from a stored response") + func assetDownloadAsksAfreshUnderIgnore() async throws { + let (_, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + + #expect(mockClient.downloadRequests.map(\.cachePolicy) == [.reloadIgnoringLocalCacheData]) + } + @Test("under .always, a manifest request can still be shared with one in flight") func manifestRequestIsSharableUnderAlways() async throws { let (_, mockClient) = try await makeLibraryWithBundle(cachePolicy: .always) @@ -468,7 +649,7 @@ struct EditorAssetLibraryTests { in: storageRoot ) let mockClient = EditorAssetLibraryMockHTTPClient() - let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0), storageRoot: storageRoot) let other = makeLibrary(storageRoot: storageRoot) // The site is back on the older manifest. Another service's cleanup removes that manifest's @@ -484,7 +665,7 @@ struct EditorAssetLibraryTests { @Test("downloadAssetBundle builds a bundle again if a purge deletes it mid-check") func downloadAssetBundleRebuildsBundlePurgedDuringCheck() async throws { - let (library, _) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(0)) let bundle = try #require(try await library.readAssetBundles().first) let checked = try await library.downloadAssetBundle { _ in try? await library.purge() } @@ -586,8 +767,22 @@ struct EditorAssetLibraryTests { """ } - private static func responses(forManifest manifestJSON: String) -> (URL) throws -> Data { - { url in url.path.contains("editor-assets") ? Data(manifestJSON.utf8) : Data("mock content".utf8) } + /// The manifest from ``manifestJSON(scriptVersion:)``, with the stylesheet at `style` as well. + private static func manifestJSON(scriptVersion: String, style: String) -> String { + """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + } + + private static func responses( + forManifest manifestJSON: String, + assetContent: String = "mock content" + ) -> (URL) throws -> Data { + { url in url.path.contains("editor-assets") ? Data(manifestJSON.utf8) : Data(assetContent.utf8) } } /// The script in ``manifestJSON(scriptVersion:)`` with version `1`. @@ -1104,6 +1299,31 @@ struct EditorAssetLibraryTests { #expect(try await builds[0].value == builds[1].value) #expect(session.requestCount == 1) } + + @Test("under .ignore, a build doesn't join one in flight for the same manifest") + func buildUnderIgnoreDoesNotJoinBuildInFlight() async throws { + let manifest = try LocalEditorAssetManifest( + remoteManifest: RemoteEditorAssetManifest(data: Data(Self.manifestJSON(scriptVersion: "1").utf8)) + ) + + let session = ParkedURLSession() + defer { session.release() } + let storageRoot = URL.randomTemporaryDirectory + let client = EditorHTTPClient(urlSession: session, authHeader: "Bearer test-token") + let inFlight = makeLibrary(httpClient: client, cachePolicy: .always, storageRoot: storageRoot) + let refreshing = makeLibrary(httpClient: client, cachePolicy: .ignore, storageRoot: storageRoot) + let destination = await inFlight.bundleRoot(for: manifest.checksum).standardizedFileURL + + let build = Task { try await inFlight.buildBundle(for: manifest) } + try await waitUntil { EditorAssetLibrary.inFlightBuilds.waiterCount(for: destination) == 1 } + let refresh = Task { try await refreshing.buildBundle(for: manifest) } + try await waitUntil { session.requestCount == 2 } + + #expect(EditorAssetLibrary.inFlightBuilds.waiterCount(for: destination) == 1) + + session.release() // fails both parked downloads, which a build tolerates + _ = try await (build.value, refresh.value) + } } // MARK: - Progress Tracker for Tests @@ -1132,6 +1352,8 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck var getCallCount = 0 var downloadCallCount = 0 var downloadedURLs: [URL] = [] + /// Requests made via `download(_:)`, in order. + var downloadRequests: [URLRequest] = [] private let lock = NSLock() /// Requests made via `perform(_:)`, in order. @@ -1176,6 +1398,7 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck lock.withLock { downloadCallCount += 1 downloadedURLs.append(url) + downloadRequests.append(urlRequest) } let data = try urlResponseHandler(url) From 07b76d36ef95c2da361b34bc86a1666bb4e53b64 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 5 Oct 2026 10:23:16 -0600 Subject: [PATCH 5/9] fix(ios)!: fall back per dependency, and never write into a bundle on disk `prepare()` fell back all or nothing. With automatic network fallback, a network error anywhere replaced every dependency at once and any other error threw, and either way a host couldn't both have what was on disk and know that a fetch had failed. `EditorService.prepareAvailable()` returns both, as an `EditorPreparation`: the dependencies, and a list of what about them isn't as the cache policy asked. Every dependency is resolved the same way: the copy on disk if the policy trusts it, the site otherwise, and when the site can't give it, for whatever reason, the copy on disk however old, or nothing. It doesn't throw for a dependency it couldn't fetch. The post is never stored, so it's the one dependency with no copy to fall back to. `prepare()` is built on it. With the fallback disabled, it throws the error of the first dependency that couldn't be fetched. With it automatic, it returns the dependencies whatever failed, so an HTTP error or a response that can't be read no longer throws there, and a service under `.always` falls back to what's on disk too. A bundle on disk is no longer written to. Storage changes in three ways, each one step under a lock: a finished bundle is moved in, a bundle's manifest is rewritten to record a check, and whole bundles are removed. Assets are only written into a draft in a temporary directory, which is moved into place when it's built and removed when it isn't used. Before, a check that found a bundle missing assets downloaded them into the bundle an editor might be reading, and a build left its temporary directory behind. A bundle that's missing assets is no longer settled. `readLatestAssetBundle()` returns nothing for one under any policy, so each prepare checks the manifest and tries the missing assets again, in a new bundle beside the old one that's kept only if it gains any. Under `.always`, nothing else would ever have asked for them again. Each asset is now one file in the bundle's `assets` directory, named for its host, path and query. It was stored at its URL's path, so two assets that differed only in host or query shared a file, and a link to a site's root read as the bundle's own directory. No name a site sends is used as a path any more, so `hasAssetData(for:)` and `assetDataPath(for:)` no longer trap, and `EditorAssetBundle.isValidAssetPath(for:)` has nothing left to check. A bundle stored the old way reads as missing its assets. Every link a `") + let headers = EditorAssetBundle.AssetHeaders( + contentType: "application/javascript; charset=utf-8", + etag: "\"first\"", + lastModified: "Wed, 30 Sep 2026 21:43:35 GMT" + ) + let bundle = try EditorAssetBundle( + manifest: manifest, + assetHeaders: [EditorAssetBundle.assetKey(for: asset): headers], + bundleRoot: FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + ) + try bundle.writeManifest(editorRepresentation: .empty) + + let loaded = try EditorAssetBundle(url: bundle.bundleRoot.appending(path: "manifest.json")) + + #expect(loaded.headers(for: asset) == headers) + } + + /// The editor asks for an asset under a scheme of its own, and is served it as the site's server would. + @Test( + "contentType(forAssetAt:) gives the type the server sent, for the asset's URL or the editor's request for it", + arguments: [ + "https://example.com/wp-content/app.js?ver=1", + "gbk-cache-https://example.com/wp-content/app.js?ver=1", + ] + ) + func contentTypeIsGivenForAssetOrRequest(url: String) throws { + let manifest = try createManifest( + scripts: "" + ) + let bundle = try EditorAssetBundle( + manifest: manifest, + assetHeaders: [ + "example.com/wp-content/app.js?ver=1": .init(contentType: "application/javascript; charset=utf-8"), + "example.com/wp-content/other.js?ver=1": .init(contentType: "text/plain"), + ], + bundleRoot: FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + ) + + #expect(bundle.contentType(forAssetAt: try #require(URL(string: url))) == "application/javascript; charset=utf-8") + } + + @Test("contentType(forAssetAt:) gives none for an asset whose server sent none, or one the bundle doesn't have") + func contentTypeIsNilWhenNotRecorded() throws { + let manifest = try createManifest(scripts: "") + let bundle = try EditorAssetBundle( + manifest: manifest, + assetHeaders: ["example.com/app.js": .init(etag: "\"first\"")], + bundleRoot: FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + ) + + #expect(bundle.contentType(forAssetAt: try #require(URL(string: "gbk-cache-https://example.com/app.js"))) == nil) + #expect(bundle.contentType(forAssetAt: try #require(URL(string: "gbk-cache-https://example.com/other.js"))) == nil) + // The same path on another host is another asset + #expect(bundle.contentType(forAssetAt: try #require(URL(string: "gbk-cache-https://cdn.example.com/app.js"))) == nil) + } + + @Test("Bundle stored before its assets' headers were recorded has none") + func bundleStoredWithoutAssetHeadersHasNone() throws { + let manifest = try createManifest(scripts: "") + let stored = try JSONEncoder().encode(["manifest": manifest]) + var object = try #require(try JSONSerialization.jsonObject(with: stored) as? [String: Any]) + object["downloadDate"] = 0 + + let bundle = try EditorAssetBundle( + data: try JSONSerialization.data(withJSONObject: object), + bundleRoot: FileManager.default.temporaryDirectory + ) + + #expect(bundle.headers(for: URL(string: "https://example.com/app.js")!) == nil) + } + // MARK: - URL Initialization Tests @Test("Bundle can be initialized from URL") @@ -237,139 +312,146 @@ struct EditorAssetBundleTests { #expect(!bundle.hasAssetData(for: url)) } - @Test("hasAssetData returns true when file exists at expected path") + @Test("hasAssetData returns true when the asset's file exists") func hasAssetDataReturnsTrueWhenFileExists() throws { - // Create temp directory and file let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let assetPath = tempDir.appending(path: "wp-content/plugins/script.js") - try FileManager.default.createDirectory( - at: assetPath.deletingLastPathComponent(), - withIntermediateDirectories: true - ) - try Data("test".utf8).write(to: assetPath) - + defer { try? FileManager.default.removeItem(at: tempDir) } let bundle = makeBundle(bundleRoot: tempDir) let url = URL(string: "https://example.com/wp-content/plugins/script.js")! + try write("test", to: bundle.assetDataPath(for: url)) #expect(bundle.hasAssetData(for: url)) - - // Clean up - try? FileManager.default.removeItem(at: tempDir) - } - - // MARK: - isValidAssetPath Tests - - @Test("isValidAssetPath returns true for valid path within bundle") - func isValidAssetPathReturnsTrueForValidPath() { - let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let bundle = makeBundle(bundleRoot: tempDir) - - let url = URL(string: "https://example.com/wp-content/plugins/script.js")! - #expect(bundle.isValidAssetPath(for: url)) - } - - @Test("isValidAssetPath returns true for nested paths") - func isValidAssetPathReturnsTrueForNestedPaths() { - let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let bundle = makeBundle(bundleRoot: tempDir) - - let url = URL(string: "https://example.com/wp-content/plugins/jetpack/assets/js/script.js")! - #expect(bundle.isValidAssetPath(for: url)) } - @Test("isValidAssetPath returns false for path traversal attempt") - func isValidAssetPathReturnsFalseForPathTraversal() { - let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let bundle = makeBundle(bundleRoot: tempDir) - - let url = URL(string: "https://example.com/../../../etc/passwd")! - #expect(!bundle.isValidAssetPath(for: url)) - } + // MARK: - Asset location Tests - @Test("isValidAssetPath returns false for path escaping via encoded traversal") - func isValidAssetPathReturnsFalseForEncodedTraversal() { + /// The bundle's own directory is there, and it isn't an asset. + @Test("hasAssetData returns false for a link to a site's root") + func hasAssetDataReturnsFalseForRootPathLink() throws { let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } let bundle = makeBundle(bundleRoot: tempDir) - let url = URL(string: "https://example.com/%2e%2e/%2e%2e/etc/passwd")! - #expect(!bundle.isValidAssetPath(for: url)) + #expect(!bundle.hasAssetData(for: try #require(URL(string: "https://s0.wp.com/?custom-css=1&csblog=1")))) } - @Test("isValidAssetPath handles paths with dot segments that stay within bundle") - func isValidAssetPathHandlesDotSegmentsWithinBundle() { + @Test( + "hasAssetData returns false for a link to a directory an asset is under", + arguments: ["https://example.com/wp-content/plugins/", "https://example.com/wp-content"] + ) + func hasAssetDataReturnsFalseForDirectoryOfAnAsset(url: String) throws { let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? FileManager.default.removeItem(at: tempDir) } let bundle = makeBundle(bundleRoot: tempDir) + let asset = try #require(URL(string: "https://example.com/wp-content/plugins/script.js")) + try write("test", to: bundle.assetDataPath(for: asset)) + + #expect(bundle.hasAssetData(for: asset)) + #expect(!bundle.hasAssetData(for: try #require(URL(string: url)))) + } + + @Test( + "assets that differ in their host or their query are stored apart", + arguments: [ + "https://cdn.example.com/wp-content/app.js?ver=1", + "https://example.com/wp-content/app.js?ver=2", + "https://example.com/wp-content/app.js", + ] + ) + func assetsThatDifferInHostOrQueryAreStoredApart(other: String) throws { + let bundle = makeBundle() + let asset = try #require(URL(string: "https://example.com/wp-content/app.js?ver=1")) - let url = URL(string: "https://example.com/wp-content/./plugins/script.js")! - #expect(bundle.isValidAssetPath(for: url)) + #expect(bundle.assetDataPath(for: try #require(URL(string: other))) != bundle.assetDataPath(for: asset)) } - // MARK: - assetDataPath Tests - - @Test("assetDataPath returns correct path based on URL path") - func assetDataPathReturnsCorrectPath() { - let tempDir = FileManager.default.temporaryDirectory.appending(path: "test-bundle") - let bundle = makeBundle(bundleRoot: tempDir) + /// A bundle on disk can be read by someone looking for an asset. + @Test("an asset's file is named for its URL") + func assetFileIsNamedForItsURL() throws { + let bundle = makeBundle() + let asset = try #require(URL(string: "https://example.com/wp-content/plugins/script.js?ver=1.2")) - let url = URL(string: "https://example.com/wp-content/plugins/script.js")! - let result = bundle.assetDataPath(for: url) + let name = bundle.assetDataPath(for: asset).lastPathComponent - #expect(result.path.contains("/wp-content/plugins/script.js")) + #expect(name.hasPrefix("example.com_wp-content_plugins_script.js_ver=1.2.")) + #expect(name.hasSuffix(".js")) } - @Test("assetDataPath allows valid nested paths") - func assetDataPathAllowsValidNestedPaths() { + @Test("an asset's file name fits the file system however long its URL is, and still tells assets apart") + func assetFileNameFitsFileSystem() throws { + let bundle = makeBundle() + let concatenated = String(repeating: "/wp-content/plugins/a-plugin/build/block.js,", count: 40) + let first = try #require(URL(string: "https://s0.wp.com/_static/??\(concatenated)/first.js")) + let second = try #require(URL(string: "https://s0.wp.com/_static/??\(concatenated)/second.js")) + + let firstName = bundle.assetDataPath(for: first).lastPathComponent + let secondName = bundle.assetDataPath(for: second).lastPathComponent + + #expect(firstName.utf8.count <= 255) + #expect(secondName.utf8.count <= 255) + #expect(firstName != secondName) + } + + /// A manifest is whatever the site sends. No name in it is used as a path, so none can put an asset + /// anywhere else, and nothing among the assets is a directory. + @Test( + "every asset is a file directly inside the bundle's assets directory", + arguments: [ + "https://example.com/wp-content/plugins/script.js", + "https://example.com/../../../etc/passwd", + "https://example.com/%2e%2e/%2e%2e/etc/passwd", + "https://example.com/wp-content/../../escaped.js", + "https://../escaped.js", + "https://example.com/", + "https://example.com", + "https://example.com/?custom-css=1", + "https://example.com/a%2Fb/..%2F..%2Fescaped.js", + "https://example.com/wp-content/plugins/", + ] + ) + func everyAssetIsAFileDirectlyInsideAssets(url: String) throws { let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) let bundle = makeBundle(bundleRoot: tempDir) - let url = URL(string: "https://example.com/wp-content/plugins/my-plugin/assets/js/script.js")! - let result = bundle.assetDataPath(for: url) - - #expect(result.standardizedFileURL.path.hasPrefix(tempDir.standardizedFileURL.path)) - } - - @Test("assetDataPath normalizes paths with dot segments") - func assetDataPathNormalizesDotsSegments() { - let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let bundle = makeBundle(bundleRoot: tempDir) + let location = bundle.assetDataPath(for: try #require(URL(string: url))) + + #expect(location.deletingLastPathComponent().standardizedFileURL.path == tempDir.appending(path: "assets").standardizedFileURL.path) + #expect(location.standardizedFileURL.lastPathComponent == location.lastPathComponent) + #expect(![".", ".."].contains(location.lastPathComponent)) + #expect(!location.lastPathComponent.contains("/")) + } + + /// The editor asks for an asset under a scheme of its own, with the URL written the way a web view writes it. + @Test( + "the editor's request for an asset finds the asset", + arguments: [ + ("https://example.com/wp-content/app.js?ver=1", "gbk-cache-https://example.com/wp-content/app.js?ver=1"), + ("https://Example.com/wp-content/app.js", "gbk-cache-https://example.com/wp-content/app.js"), + ("https://example.com/wp-content/plugins/../app.js", "gbk-cache-https://example.com/wp-content/app.js"), + ("https://example.com/my%20plugin/app.js", "gbk-cache-https://example.com/my%20plugin/app.js"), + ("https://example.com/?custom-css=1", "gbk-cache-https://example.com/?custom-css=1"), + ] + ) + func requestForAssetFindsAsset(asset: String, request: String) throws { + let bundle = makeBundle() - // This path has ./ which should be normalized but stay within bundle - let url = URL(string: "https://example.com/wp-content/./plugins/script.js")! - let result = bundle.assetDataPath(for: url) - - #expect(result.standardizedFileURL.path.hasPrefix(tempDir.standardizedFileURL.path)) - #expect(result.path.contains("plugins/script.js")) - } - - #if os(macOS) - @Test("assetDataPath crashes for path traversal attempt") - func assetDataPathCrashesForPathTraversal() async { - await #expect(processExitsWith: .failure) { - let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let bundle = EditorAssetBundle( - raw: EditorAssetBundle.RawAssetBundle(manifest: .empty, downloadDate: Date()), - bundleRoot: tempDir - ) - let url = URL(string: "https://example.com/../../../etc/passwd")! - _ = bundle.assetDataPath(for: url) - } + #expect( + bundle.assetDataPath(for: try #require(URL(string: request))) + == bundle.assetDataPath(for: try #require(URL(string: asset))) + ) } - #endif // MARK: - assetData Tests @Test("assetData returns data for existing file") func assetDataReturnsDataForExistingFile() throws { - // Create temp directory and file let tempDir = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) - let assetPath = tempDir.appending(path: "script.js") let testContent = "console.log('test');" - try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - try Data(testContent.utf8).write(to: assetPath) - let bundle = makeBundle(bundleRoot: tempDir) let requestUrl = URL(string: "https://example.com/script.js")! + try write(testContent, to: bundle.assetDataPath(for: requestUrl)) let data = try bundle.assetData(for: requestUrl) #expect(String(data: data, encoding: .utf8) == testContent) @@ -617,6 +699,11 @@ struct EditorAssetBundleTests { extension EditorAssetBundleTests { + fileprivate func write(_ content: String, to file: URL) throws { + try FileManager.default.createDirectory(at: file.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data(content.utf8).write(to: file) + } + fileprivate func makeBundle( manifest: LocalEditorAssetManifest = .empty, downloadDate: Date? = nil, diff --git a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift index c8e8a049e..692e0037a 100644 --- a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift +++ b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift @@ -278,6 +278,264 @@ struct EditorServiceTests: MakesTestFixtures { #expect(dependencies.preloadList == nil) } + /// The post is the one thing `prepare()` asks the site for every time, so a service for a post + /// can't reach the site whatever its cache policy. + @Test( + "with automatic fallback, prepare() for a post that can't reach the site returns what's on disk, without the post", + arguments: [EditorCachePolicy.always, .maxAge(0), .ignore] + ) + func failedPrepareForPostFallsBackToDisk(cachePolicy: EditorCachePolicy) async throws { + let configuration = makeConfiguration(postID: 123).toBuilder().setNetworkFallbackMode(.automatic).build() + let site = TestSite(configuration: configuration, manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.isOffline = true + let dependencies = try await site.service(cachePolicy: cachePolicy).prepare() + + let preloadList = try #require(dependencies.preloadList) + #expect(dependencies.assetBundle == prepared.assetBundle) + #expect(dependencies.editorSettings == prepared.editorSettings) + #expect(preloadList.postTypeData == prepared.preloadList?.postTypeData) + #expect(preloadList.postData == nil) + // One for each `prepare()`: reading what's on disk asks the site for nothing + #expect(site.postRequestCount == 2) + } + + @Test("with automatic fallback, what's on disk needs no settings or bundle for a configuration that uses neither") + func fallbackToDiskNeedsOnlyWhatConfigurationUses() async throws { + let configuration = makeConfiguration(postID: 123, shouldUsePlugins: false, shouldUseThemeStyles: false) + .toBuilder().setNetworkFallbackMode(.automatic).build() + let site = TestSite(configuration: configuration, manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.isOffline = true + let dependencies = try await site.service(cachePolicy: .always).prepare() + + #expect(dependencies.preloadList?.postTypeData == prepared.preloadList?.postTypeData) + #expect(dependencies.editorSettings == .undefined) + #expect(dependencies.assetBundle == .empty) + } + + @Test("without automatic fallback, prepare() for a post that can't reach the site throws, whatever is on disk") + func failedPrepareForPostThrowsWithoutFallback() async throws { + let site = TestSite(configuration: makeConfiguration(postID: 123), manifest: Self.pluginManifest(version: "1")) + _ = try await site.service(cachePolicy: .always).prepare() + + site.isOffline = true + await #expect(throws: URLError.self) { + try await site.service(cachePolicy: .always).prepare() + } + } + + // MARK: - What a Prepare Came To + + @Test("prepareAvailable() reports no failures when every dependency is fetched") + func prepareAvailableReportsNoFailures() async throws { + let site = TestSite(configuration: makeConfiguration(postID: 123), manifest: Self.pluginManifest(version: "1")) + + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.isComplete) + #expect(preparation.dependencies.assetBundle.assetCount == 1) + #expect(preparation.dependencies.preloadList?.postData != nil) + } + + /// A site can answer and still not have the manifest: the endpoint is removed, or errors. + @Test("a manifest check the site answers with an error gives the bundle on disk, and says the check failed") + func manifestCheckAnsweredWithErrorGivesBundleOnDisk() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.failure = { $0.absoluteString.contains("editor-assets") ? Self.notFound($0) : nil } + let preparation = try await site.service(cachePolicy: .maxAge(0)).prepareAvailable() + + #expect(preparation.dependencies == prepared) + #expect(dependenciesNotFetched(in: preparation) == [.assetBundle: true]) + #expect(preparation.failures.count == 1) + } + + @Test("prepare() gives the bundle on disk when the manifest check errors and the fallback is automatic") + func prepareGivesBundleOnDiskWhenManifestCheckErrors() async throws { + let configuration = makeConfiguration().toBuilder().setNetworkFallbackMode(.automatic).build() + let site = TestSite(configuration: configuration, manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.failure = { $0.absoluteString.contains("editor-assets") ? Self.notFound($0) : nil } + let dependencies = try await site.service(cachePolicy: .maxAge(0)).prepare() + + #expect(dependencies == prepared) + } + + @Test("prepare() throws when the manifest check errors and the fallback is disabled") + func prepareThrowsWhenManifestCheckErrorsWithoutFallback() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + _ = try await site.service(cachePolicy: .always).prepare() + + site.failure = { $0.absoluteString.contains("editor-assets") ? Self.notFound($0) : nil } + await #expect(throws: EditorHTTPClient.ClientError.self) { + try await site.service(cachePolicy: .maxAge(0)).prepare() + } + } + + @Test("each dependency that can't be fetched comes from disk, and each is reported") + func prepareAvailableFallsBackToDiskForEveryDependency() async throws { + let site = TestSite(configuration: makeConfiguration(postID: 123), manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.isOffline = true + let preparation = try await site.service(cachePolicy: .ignore).prepareAvailable() + + #expect(preparation.dependencies.editorSettings == prepared.editorSettings) + #expect(preparation.dependencies.assetBundle == prepared.assetBundle) + #expect(preparation.dependencies.preloadList?.postTypeData == prepared.preloadList?.postTypeData) + #expect(dependenciesNotFetched(in: preparation) == [ + .editorSettings: true, + .assetBundle: true, + .postType: true, + .postTypes: true, + .activeTheme: true, + .settingsOptions: true, + // Never stored, so there's no copy of it to use + .post: false, + ]) + } + + @Test("a dependency with no copy on disk is left out, without emptying the ones that have one") + func prepareAvailableLeavesOutOnlyWhatIsMissing() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + try await EditorAssetLibrary( + configuration: site.configuration, + httpClient: site.client, + storageRoot: site.storageRoot + ).purge() + + site.isOffline = true + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.dependencies.assetBundle == .empty) + #expect(preparation.dependencies.editorSettings == prepared.editorSettings) + #expect(preparation.dependencies.preloadList == prepared.preloadList) + #expect(dependenciesNotFetched(in: preparation) == [.assetBundle: false]) + } + + @Test("with nothing on disk, every dependency is reported as left out") + func prepareAvailableReportsEverythingLeftOut() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.isOffline = true + + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.dependencies.editorSettings == .undefined) + #expect(preparation.dependencies.assetBundle == .empty) + #expect(preparation.dependencies.preloadList == nil) + #expect(dependenciesNotFetched(in: preparation).values.allSatisfy { !$0 }) + #expect(dependenciesNotFetched(in: preparation).count == 6) + } + + @Test("a bundle that's missing an asset is reported, and doesn't stop prepare() even without a fallback") + func bundleMissingAssetIsReported() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.failure = { $0.path == Self.pluginScript.path ? Self.notFound($0) : nil } + + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(assetsMissing(in: preparation) == [Self.pluginScript]) + #expect(dependenciesNotFetched(in: preparation).isEmpty) + #expect(try await site.service(cachePolicy: .always).prepare().assetBundle.id == preparation.dependencies.assetBundle.id) + } + + /// Whatever the cache policy: under `.always`, nothing else would ever ask the site again. + @Test("a bundle that's missing an asset is tried again on every prepare, until it has it") + func bundleMissingAssetIsTriedAgainEveryPrepare() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.failure = { $0.path == Self.pluginScript.path ? Self.notFound($0) : nil } + _ = try await site.service(cachePolicy: .always).prepare() + + let again = try await site.service(cachePolicy: .always).prepareAvailable() + #expect(assetsMissing(in: again) == [Self.pluginScript]) + #expect(site.manifestRequestCount == 2) + #expect(site.client.downloadCallCount == 2) + + site.failure = nil + let repaired = try await site.service(cachePolicy: .always).prepareAvailable() + #expect(repaired.isComplete) + #expect(repaired.dependencies.assetBundle.hasAssetData(for: Self.pluginScript)) + #expect(site.manifestRequestCount == 3) + #expect(site.client.downloadCallCount == 3) + + // And once it has it, there's nothing left to ask for + _ = try await site.service(cachePolicy: .always).prepare() + #expect(site.manifestRequestCount == 3) + #expect(site.client.downloadCallCount == 3) + } + + @Test("a bundle that's missing an asset is still given when the site can't be asked about it") + func bundleMissingAssetIsGivenWhenSiteCannotBeAsked() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.failure = { $0.path == Self.pluginScript.path ? Self.notFound($0) : nil } + let gapped = try await site.service(cachePolicy: .always).prepare().assetBundle + + site.isOffline = true + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.dependencies.assetBundle == gapped) + #expect(dependenciesNotFetched(in: preparation) == [.assetBundle: true]) + #expect(assetsMissing(in: preparation) == [Self.pluginScript]) + } + + /// A copy that can't be read is no copy, and the site can still be asked. + @Test("a dependency whose copy on disk can't be read is fetched, rather than given up on") + func unreadableCopyOnDiskIsFetchedAgain() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + let settingsURL = try #require(site.client.requestedURLs.first { $0.absoluteString.contains("block-editor/v1/settings") }) + try EditorURLCache(siteId: site.configuration.siteId, parentDirectory: site.cacheRoot).store( + EditorURLResponse(data: Data("not editor settings".utf8), responseHeaders: [:]), + for: settingsURL, + httpMethod: .GET + ) + + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.isComplete) + #expect(preparation.dependencies.editorSettings == prepared.editorSettings) + #expect(site.client.requestedURLs.filter { $0 == settingsURL }.count == 2) + } + + @Test("a cancelled prepare throws, rather than reporting everything it was fetching as a failure") + func cancelledPrepareThrows() async throws { + let client = GatedHTTPClient { Self.editorServiceResponseHandler($0) } + let service = EditorService( + configuration: makeConfiguration().toBuilder().setNetworkFallbackMode(.automatic).build(), + httpClient: client, + storageRoot: .randomTemporaryDirectory, + cacheRoot: .randomTemporaryDirectory + ) + + let prepare = Task { + try await GatedHTTPClient.$caller.withValue("prepare") { try await service.prepareAvailable() } + } + try await waitUntil { client.isHolding("prepare") } + prepare.cancel() + + await #expect(throws: CancellationError.self) { try await prepare.value } + } + + @Test("progress ends on its total when dependencies come from disk") + func progressEndsOnTotalWhenFallingBack() async throws { + let site = TestSite(configuration: makeConfiguration(postID: 123), manifest: Self.pluginManifest(version: "1")) + _ = try await site.service(cachePolicy: .always).prepare() + let tracker = ProgressTracker() + + site.isOffline = true + _ = try await site.service(cachePolicy: .ignore).prepareAvailable { tracker.append($0) } + + let last = try #require(tracker.updates.last) + #expect(last.completed == last.total) + #expect(tracker.updates.allSatisfy { $0.completed <= $0.total }) + } + // MARK: - Automatic Cleanup @Test("a site's old bundles are cleaned up, whichever site was prepared first that day") @@ -347,6 +605,32 @@ struct EditorServiceTests: MakesTestFixtures { } } + /// The dependencies `preparation` says weren't fetched, each with whether its copy on disk stands + /// in for it. + private func dependenciesNotFetched(in preparation: EditorPreparation) -> [EditorPreparation.Dependency: Bool] { + var dependencies: [EditorPreparation.Dependency: Bool] = [:] + for case .notFetched(let dependency, _, let usingCopyOnDisk) in preparation.failures { + dependencies[dependency] = usingCopyOnDisk + } + return dependencies + } + + /// The assets `preparation` says its bundle is missing. + private func assetsMissing(in preparation: EditorPreparation) -> [URL] { + var assets: [URL] = [] + for case .assetsMissing(let missing) in preparation.failures { + assets += missing + } + return assets + } + + /// The error a site answers with when it has nothing at a URL. + private static func notFound(_ url: URL) -> any Error { + EditorHTTPClient.ClientError.unknown(response: Data(), statusCode: 404, requestURL: url) + } + + private static let pluginScript = URL(string: "https://example.com/plugin.js?ver=1")! + /// A manifest with one plugin script, whose URL carries `version` the way WordPress versions its /// assets. private static func pluginManifest(version: String) -> String { @@ -371,10 +655,19 @@ struct EditorServiceTests: MakesTestFixtures { didSet { serve() } } + /// The error a request for a URL fails with, for the requests that should fail. + var failure: (@Sendable (URL) -> (any Error)?)? { + didSet { serve() } + } + var manifestRequestCount: Int { client.requestedURLs.filter { $0.absoluteString.contains("editor-assets") }.count } + var postRequestCount: Int { + client.requestedURLs.filter { $0.absoluteString.contains("/wp/v2/posts/") }.count + } + init(configuration: EditorConfiguration, manifest: String) { self.configuration = configuration self.manifest = manifest @@ -402,8 +695,9 @@ struct EditorServiceTests: MakesTestFixtures { } private func serve() { - client.urlResponseHandler = { [manifest, isOffline] url in + client.urlResponseHandler = { [manifest, isOffline, failure] url in guard !isOffline else { throw URLError(.notConnectedToInternet) } + if let error = failure?(url) { throw error } return url.absoluteString.contains("editor-assets") ? Data(manifest.utf8) : EditorServiceTests.editorServiceResponseHandler(url) } diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 4fb5f8e26..1c5773700 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -470,6 +470,51 @@ struct EditorAssetLibraryTests { #expect(try refreshed.assetData(for: URL(string: style)!) == Data("mock content".utf8)) } + @Test("under .ignore, an asset that fails to download comes from the bundle the site went back to") + func assetThatFailsToDownloadComesFromBundleSiteWentBackTo() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let earlier = try await library.downloadAssetBundle() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + _ = try await library.downloadAssetBundle() + + // The site goes back to the earlier manifest, whose script the latest bundle doesn't have + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let refreshed = try await library.downloadAssetBundle() + + #expect(try refreshed.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + #expect(refreshed == earlier) + } + + @Test("under .ignore, assets stored at one path that fail to download keep the copy on disk") + func assetsStoredAtOnePathKeepCopyOnDiskWhenDownloadsFail() async throws { + // The same file on two hosts: a bundle stores an asset by its path alone + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + let bundle = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(manifest.utf8) + } + let refreshed = try await library.downloadAssetBundle() + + #expect(try refreshed.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + #expect(refreshed == bundle) + } + @Test("under .ignore, downloadAssetBundle downloads the assets again even when they turn out the same") func downloadAssetBundleDownloadsIdenticalAssetsAgainUnderIgnore() async throws { let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) @@ -512,6 +557,162 @@ struct EditorAssetLibraryTests { #expect(repaired.id == gapped.id) #expect(repaired.hasAssetData(for: Self.scriptURL)) #expect(mockClient.downloadCallCount == 2) + #expect(try await library.readAssetBundles().first?.bundleRoot == repaired.bundleRoot) + } + + /// A bundle used to keep each asset at its URL's path. Its assets aren't where a bundle looks now, so it + /// reads as one that's missing them, and is repaired the way one is. + @Test("a bundle stored before assets were named for their URLs is downloaded again, once") + func bundleStoredAtAssetPathsIsDownloadedAgain() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .always) + let stored = try await library.downloadAssetBundle() + // As it was stored then + try FileManager.default.moveItem( + at: stored.assetDataPath(for: Self.scriptURL), + to: stored.bundleRoot.appending(path: "plugin.js") + ) + #expect(try await library.readLatestAssetBundle() == nil) + + let repaired = try await library.downloadAssetBundle() + + #expect(repaired.id == stored.id) + #expect(try repaired.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + #expect(mockClient.downloadCallCount == 2) + #expect(try await library.readLatestAssetBundle() == repaired) + } + + /// A bundle's download date is when its build began. Going by that alone, a bundle another service + /// matched while this one was building would still look like the latest. + @Test("a bundle is the site's latest from the moment it's built, whatever was matched while it was building") + func builtBundleIsLatestOnceBuilt() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + let earlier = try await library.downloadAssetBundle() + + // The site's manifest changes, and while its bundle builds the earlier one is matched again + let rematch = OnceOnly { try? self.backdate(earlier, by: 0) } + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(Self.manifestJSON(scriptVersion: "2").utf8) + } + rematch.run() + return Data("new content".utf8) + } + let built = try await library.downloadAssetBundle() + + #expect(built.lastCheckedDate != nil) + #expect(try await library.readAssetBundles().first?.id == built.id) + } + + /// An editor may be reading the bundle that's missing the asset, so the asset doesn't go into it. + @Test("a check that downloads what a bundle is missing leaves that bundle as it was, and builds one beside it") + func repairLeavesBundleOnDiskAsItWas() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + let gapped = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let repaired = try await library.downloadAssetBundle() + + #expect(repaired.bundleRoot != gapped.bundleRoot) + #expect(!gapped.hasAssetData(for: Self.scriptURL)) + #expect((try? gapped.getEditorRepresentation() as EditorAssetBundle.EditorRepresentation) != nil) + } + + @Test("a check whose bundle is still missing an asset afterwards keeps that bundle, and adds no other") + func repairThatGainsNothingKeepsBundle() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0), storageRoot: storageRoot) + let gapped = try await library.downloadAssetBundle() + + let checked = try await library.downloadAssetBundle() + + #expect(checked == gapped) + #expect(mockClient.downloadCallCount == 2) + #expect(try FileManager.default.contentsOfDirectory(atPath: storageRoot.path).count == 1) + } + + /// Nothing is downloaded into the library's storage, so a bundle deleted there stays deleted. + @Test("a bundle deleted while a check downloads what it's missing doesn't come back as loose files") + func deletedBundleIsNotRecreatedByDownload() async throws { + let manifest = Self.manifestJSON(scriptVersion: "1") + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(manifest.utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0), storageRoot: storageRoot) + let gapped = try await library.downloadAssetBundle() + + // The manifest's bundle, in a directory that building the manifest again wouldn't replace + let moved = storageRoot.appending(path: "\(gapped.id)-moved") + try FileManager.default.moveItem(at: gapped.bundleRoot, to: moved) + + // The bundle is deleted while its missing script downloads, the first time that's asked for + let deletion = OnceOnly { try? FileManager.default.removeItem(at: moved) } + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifest.utf8) + } + deletion.run() + return Data("mock content".utf8) + } + let rebuilt = try await library.downloadAssetBundle() + + #expect(rebuilt.hasAssetData(for: Self.scriptURL)) + #expect( + try FileManager.default.contentsOfDirectory(atPath: storageRoot.path) + == [rebuilt.bundleRoot.lastPathComponent] + ) + } + + @Test("an asset that a check downloads for a bundle that was missing it keeps the headers it came with") + func repairedAssetKeepsItsValidator() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let manifest = Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { $0.path == style.path ? ["ETag": "\"first\""] : [:] } + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifest.utf8) + } + guard url.path != style.path else { throw URLError(.timedOut) } + return Data("mock content".utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + let gapped = try await library.downloadAssetBundle() + #expect(!gapped.hasAssetData(for: style)) + + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let repaired = try await library.downloadAssetBundle() + + #expect(repaired.id == gapped.id) + #expect(repaired.headers(for: style)?.etag == "\"first\"") + #expect(try await library.readAssetBundles().first?.headers(for: style)?.etag == "\"first\"") + + // So the next bundle asks only for a newer copy of it + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style.absoluteString), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + let request = try #require(mockClient.downloadRequests.last { $0.url == style }) + #expect(request.value(forHTTPHeaderField: "If-None-Match") == "\"first\"") + #expect(try changed.assetData(for: style) == Data("mock content".utf8)) } @Test( @@ -580,6 +781,149 @@ struct EditorAssetLibraryTests { #expect(try changed.assetData(for: URL(string: style)!) == Data("new content".utf8)) } + @Test( + "a changed manifest's bundle asks only for a newer copy of an asset without a version, and keeps the one on disk when there's none", + arguments: [["ETag": "\"first\""], ["Last-Modified": "Wed, 30 Sep 2026 21:43:35 GMT"]] + ) + func changedBundleKeepsUnversionedAssetServerSaysIsUnchanged(assetHeaders: [String: String]) async throws { + let style = URL(string: "https://example.com/plugin.css")! + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { $0.path == style.path ? assetHeaders : [:] } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + _ = try await library.downloadAssetBundle() + + // Twice, because the second bundle has to keep the asset's headers along with the asset + for scriptVersion in ["2", "3"] { + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: scriptVersion, style: style.absoluteString), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + let request = try #require(mockClient.downloadRequests.last { $0.url == style }) + #expect(request.value(forHTTPHeaderField: "If-None-Match") == assetHeaders["ETag"]) + #expect(request.value(forHTTPHeaderField: "If-Modified-Since") == assetHeaders["Last-Modified"]) + #expect(try changed.assetData(for: style) == Data("mock content".utf8)) + } + } + + @Test("a changed manifest's bundle downloads the newer copy of an asset without a version when the server has one") + func changedBundleDownloadsUnversionedAssetServerSaysHasChanged() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { $0.path == style.path ? ["ETag": "\"first\""] : [:] } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + _ = try await library.downloadAssetBundle() + + mockClient.assetValidators = { $0.path == style.path ? ["ETag": "\"second\""] : [:] } + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style.absoluteString), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + #expect(try changed.assetData(for: style) == Data("new content".utf8)) + #expect(changed.headers(for: style)?.etag == "\"second\"") + } + + /// The editor is served an asset with the type its server gave it, so the type has to stay with the file + /// however the file gets into a bundle: downloaded, copied because its URL hasn't changed, or kept because + /// the server says it hasn't. + @Test("an asset's Content-Type is kept with it, from one bundle to the next") + func assetContentTypeIsKeptWithAsset() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let script = URL(string: "https://example.com/plugin.js?ver=1")! + let secondScript = URL(string: "https://example.com/second.js?ver=1")! + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { url in + switch url.path { + case style.path: ["Content-Type": "text/css; charset=utf-8", "ETag": "\"first\""] + case script.path: ["Content-Type": "application/javascript"] + default: [:] + } + } + func manifest(extraScript: Bool) -> String { + let scripts = [script] + (extraScript ? [secondScript] : []) + let tags = scripts.map { "" }.joined() + return """ + { + "scripts": "\(tags)", + "styles": "", + "allowed_block_types": ["content-type-test"] + } + """ + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + mockClient.urlResponseHandler = Self.responses(forManifest: manifest(extraScript: false)) + let first = try await library.downloadAssetBundle() + + // A changed manifest: the script is copied for its unchanged URL, the stylesheet kept on a 304 + mockClient.urlResponseHandler = Self.responses(forManifest: manifest(extraScript: true)) + let second = try await library.downloadAssetBundle() + + #expect(second.id != first.id) + for bundle in [first, second] { + #expect(bundle.contentType(forAssetAt: style) == "text/css; charset=utf-8") + #expect(bundle.contentType(forAssetAt: script) == "application/javascript") + } + // Its server sent no type, so there is none to serve it with + #expect(second.contentType(forAssetAt: secondScript) == nil) + } + + @Test("under .ignore, an asset is downloaded in full even when the server could say it hasn't changed") + func assetDownloadIsUnconditionalUnderIgnore() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { _ in ["ETag": "\"first\""] } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + _ = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style.absoluteString), + assetContent: "new content" + ) + let refreshed = try await library.downloadAssetBundle() + + #expect(mockClient.downloadRequests.allSatisfy { $0.value(forHTTPHeaderField: "If-None-Match") == nil }) + #expect(try refreshed.assetData(for: style) == Data("new content".utf8)) + } + + @Test("a changed manifest's bundle keeps the copy on disk of an asset without a version that fails to download") + func changedBundleKeepsUnversionedAssetThatFailsToDownload() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { $0.path == style.path ? ["ETag": "\"first\""] : [:] } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + _ = try await library.downloadAssetBundle() + + // The server has a newer copy, which can't be downloaded + mockClient.assetValidators = { $0.path == style.path ? ["ETag": "\"second\""] : [:] } + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(Self.manifestJSON(scriptVersion: "2", style: style.absoluteString).utf8) + } + guard url.path != style.path else { throw URLError(.timedOut) } + return Data("new content".utf8) + } + let changed = try await library.downloadAssetBundle() + + #expect(try changed.assetData(for: style) == Data("mock content".utf8)) + // It's the earlier copy, so it's the earlier copy's headers that go with it + #expect(changed.headers(for: style)?.etag == "\"first\"") + } + @Test("a changed manifest's bundle downloads an unchanged asset that the latest bundle is missing") func changedBundleDownloadsAssetMissingFromLatestBundle() async throws { let style = "https://example.com/plugin.css?ver=1" @@ -913,11 +1257,11 @@ struct EditorAssetLibraryTests { // MARK: - buildBundle Tests - /// Helper to create a unique manifest JSON for each test - private func uniqueManifestJSON(identifier: String) -> String { + /// Helper to create a unique manifest JSON for each test, with the script at `script` if given + private func uniqueManifestJSON(identifier: String, script: String? = nil) -> String { """ { - "scripts": "", + "scripts": "\(script.map { #""# } ?? "")", "styles": "", "allowed_block_types": ["\(identifier)"] } @@ -1158,13 +1502,9 @@ struct EditorAssetLibraryTests { let bundleRoot = await library.bundleRoot(for: bundle) #expect(FileManager.default.fileExists(at: bundleRoot)) - // Verify the nested directory structure was created for the script - let scriptPath = bundleRoot.appending(path: "/wp-content/plugins/jetpack/assets/js/editor.js") - #expect(FileManager.default.fileExists(at: scriptPath)) - - // Verify the nested directory structure was created for the style - let stylePath = bundleRoot.appending(path: "/wp-content/themes/theme/css/blocks/gallery.css") - #expect(FileManager.default.fileExists(at: stylePath)) + // Verify both assets are in the bundle + #expect(bundle.hasAssetData(for: URL(string: "https://example.com/wp-content/plugins/jetpack/assets/js/editor.js")!)) + #expect(bundle.hasAssetData(for: URL(string: "https://example.com/wp-content/themes/theme/css/blocks/gallery.css")!)) } @Test("downloadAssetBundle reports progress") @@ -1227,15 +1567,11 @@ struct EditorAssetLibraryTests { #expect(mockClient.downloadCallCount == 3) // Verify the successful assets were downloaded - let bundleRoot = await library.bundleRoot(for: bundle) - let goodScriptPath = bundleRoot.appending(path: "/good-script.js") - let stylePath = bundleRoot.appending(path: "/style.css") - #expect(FileManager.default.fileExists(at: goodScriptPath)) - #expect(FileManager.default.fileExists(at: stylePath)) + #expect(bundle.hasAssetData(for: URL(string: "https://example.com/good-script.js")!)) + #expect(bundle.hasAssetData(for: URL(string: "https://example.com/style.css")!)) // The failed asset should not exist - let failedScriptPath = bundleRoot.appending(path: "/stats.js") - #expect(!FileManager.default.fileExists(at: failedScriptPath)) + #expect(!bundle.hasAssetData(for: URL(string: "https://blocked.com/stats.js")!)) } @Test("buildBundle publishes nothing when it is cancelled mid-download") @@ -1270,6 +1606,328 @@ struct EditorAssetLibraryTests { #expect(try await library.readAssetBundles().isEmpty) } + @Test("a manifest's links that aren't HTTP are the only ones a bundle doesn't hold") + func linksLeftOutAreTheOnesThatAreNotHTTP() async throws { + let manifestJSON = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + let manifest = try LocalEditorAssetManifest( + remoteManifest: RemoteEditorAssetManifest(data: Data(manifestJSON.utf8)) + ) + + let leftOut = await makeLibrary().linksLeftOut(of: manifest) + + #expect(leftOut.map(\.absoluteString) == ["ftp://example.com/old.js", "data:text/css,a%7Bcolor:red%7D"]) + } + + /// A manifest is whatever the site sends. An asset is stored under a name made from its URL, never at + /// its path, so `..` in one takes it nowhere. + @Test( + "an asset whose path climbs out of the site is stored inside the bundle like any other", + arguments: ["/wp-content/../../", "/wp-content/%2e%2e/%2e%2e/"] + ) + func assetWhosePathClimbsIsStoredInsideBundle(pathOutOfSite: String) async throws { + // Where it would land if its path were followed: beside the directory the bundle is assembled in + let name = "escaped-\(UUID().uuidString).js" + let landing = URL.temporaryDirectory.appending(path: name) + defer { try? FileManager.default.removeItem(at: landing) } + + let climbing = try #require(URL(string: "https://example.com\(pathOutOfSite)\(name)")) + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["\(name)"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient) + + let bundle = try await library.downloadAssetBundle() + + #expect(!FileManager.default.fileExists(at: landing)) + #expect(bundle.hasAssetData(for: Self.scriptURL)) + #expect(bundle.hasAssetData(for: climbing)) + #expect(await library.missingAssets(of: bundle).isEmpty) + + let assets = await library.bundleRoot(for: bundle).appending(path: "assets") + #expect( + bundle.assetDataPath(for: climbing).deletingLastPathComponent().standardizedFileURL.path + == assets.standardizedFileURL.path + ) + #expect(try FileManager.default.contentsOfDirectory(atPath: assets.path).count == 2) + } + + // MARK: - Which links are assets + + /// What makes a link an asset is the tag it's on, not how its URL ends. + @Test("every script and stylesheet link in a manifest is stored, however its URL ends") + func everyScriptAndStylesheetLinkIsStored() async throws { + let scripts = [ + "https://s0.wp.com/_static/??/wp-includes/js/dist/hooks.min.js,/wp-includes/js/dist/i18n.min.js", + "https://www.googletagmanager.com/gtag/js?id=G-TEST", + ] + let styles = [ + "https://fonts-api.wp.com/css2?family=Inter:wght@400", + "https://s0.wp.com/?custom-css=1&csblog=1", + ] + let scriptTags = scripts.map { "" }.joined() + let styleTags = styles.map { "" }.joined() + let manifest = """ + { + "scripts": "\(scriptTags)", + "styles": "\(styleTags)", + "allowed_block_types": ["every-link-is-stored"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + url.path.contains("editor-assets") ? Data(manifest.utf8) : Data("content of \(url.absoluteString)".utf8) + } + mockClient.assetValidators = { url in + ["Content-Type": scripts.contains(url.absoluteString) ? "application/javascript" : "text/css; charset=utf-8"] + } + let library = makeLibrary(httpClient: mockClient) + + let bundle = try await library.downloadAssetBundle() + + for (asset, contentType) in scripts.map({ ($0, "application/javascript") }) + styles.map({ ($0, "text/css; charset=utf-8") }) { + let url = try #require(URL(string: asset)) + #expect(try bundle.assetData(for: url) == Data("content of \(asset)".utf8)) + #expect(bundle.contentType(forAssetAt: url) == contentType) + } + #expect(await library.missingAssets(of: bundle).isEmpty) + } + + /// A site can answer a request for an asset with a web page and still say it went well: one to log in + /// on, or one saying what went wrong. Kept, it would be served to every editor in the asset's place. + @Test( + "a download that's a web page isn't kept, and is missed", + arguments: [ + (isScript: true, contentType: "text/html; charset=UTF-8"), + (isScript: true, contentType: "TEXT/HTML"), + (isScript: false, contentType: "text/html ;charset=utf-8"), + ] + ) + func downloadThatIsAWebPageIsNotKept(isScript: Bool, contentType: String) async throws { + let style = URL(string: "https://example.com/style.css?ver=1")! + let refused = isScript ? Self.scriptURL : style + let kept = isScript ? style : Self.scriptURL + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + mockClient.assetValidators = { $0 == refused ? ["Content-Type": contentType] : [:] } + let library = makeLibrary(httpClient: mockClient) + + let bundle = try await library.downloadAssetBundle() + + #expect(!bundle.hasAssetData(for: refused)) + #expect(bundle.hasAssetData(for: kept)) + #expect(await library.missingAssets(of: bundle) == [refused]) + } + + /// What a web view makes of an asset is for the web view to decide, from the type the asset is served + /// with, as it would if the site had served it. + @Test( + "a download is kept whatever else its type is, or when it has none, and keeps the type it came with", + arguments: [ + (isScript: true, contentType: "application/javascript"), + (isScript: true, contentType: "text/javascript; charset=utf-8"), + (isScript: true, contentType: "text/plain"), + (isScript: true, contentType: "application/octet-stream"), + (isScript: true, contentType: "application/json"), + (isScript: true, contentType: "text/css"), + (isScript: true, contentType: nil), + (isScript: false, contentType: "text/css"), + (isScript: false, contentType: "text/plain"), + (isScript: false, contentType: "application/javascript"), + (isScript: false, contentType: nil), + ] as [(Bool, String?)] + ) + func downloadIsKeptWhateverElseItsTypeIs(isScript: Bool, contentType: String?) async throws { + let style = URL(string: "https://example.com/style.css?ver=1")! + let asset = isScript ? Self.scriptURL : style + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + mockClient.assetValidators = { url in + guard url == asset, let contentType else { return [:] } + return ["Content-Type": contentType] + } + let library = makeLibrary(httpClient: mockClient) + + let bundle = try await library.downloadAssetBundle() + + #expect(bundle.hasAssetData(for: asset)) + #expect(bundle.contentType(forAssetAt: asset) == contentType) + #expect(await library.missingAssets(of: bundle).isEmpty) + } + + @Test("a refresh that's answered with a web page keeps the copy on disk") + func refreshAnsweredWithAWebPageKeepsCopyOnDisk() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + mockClient.assetValidators = { _ in ["Content-Type": "application/javascript"] } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore) + let first = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1"), + assetContent: "Log in" + ) + mockClient.assetValidators = { _ in ["Content-Type": "text/html; charset=UTF-8"] } + let refreshed = try await library.downloadAssetBundle() + + #expect(try refreshed.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + #expect(refreshed.contentType(forAssetAt: Self.scriptURL) == "application/javascript") + #expect(refreshed == first) + } + + /// WordPress tells one version of a file from another by its query, and one host's file from another's + /// by its host. + @Test("assets that share a path are each stored, with their own content") + func assetsSharingAPathAreEachStored() async throws { + let assets = [ + "https://example.com/wp-content/app.js?ver=1", + "https://example.com/wp-content/app.js?ver=2", + "https://cdn.example.com/wp-content/app.js?ver=1", + ] + let scripts = assets.map { "" }.joined() + let manifest = """ + { + "scripts": "\(scripts)", + "styles": "", + "allowed_block_types": ["assets-sharing-a-path"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + url.path.contains("editor-assets") ? Data(manifest.utf8) : Data("content of \(url.absoluteString)".utf8) + } + + let bundle = try await makeLibrary(httpClient: mockClient).downloadAssetBundle() + + for asset in assets { + let url = try #require(URL(string: asset)) + #expect(try bundle.assetData(for: url) == Data("content of \(asset)".utf8)) + } + } + + /// The library writes an asset and the bundle reads it, so the two have to agree on where. + @Test( + "an asset is read from where it was written, whatever characters its path has", + arguments: [ + "https://example.com/wp-content/plugins/my%20plugin/script.js", + "https://example.com/wp-content/plugins/a%23b/script.js", + "https://example.com/wp-content/plugins/a%3Fb/script.js", + "https://example.com/wp-content/plugins/50%25/script.js", + ] + ) + func assetIsReadFromWhereItWasWritten(asset: String) async throws { + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["\(asset)"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + + let bundle = try await makeLibrary(httpClient: mockClient).downloadAssetBundle() + + let url = try #require(URL(string: asset)) + #expect(try bundle.assetData(for: url) == Data("mock content".utf8)) + } + + @Test("a download's file is moved into the bundle, not left where the download put it") + func downloadedFilesAreNotLeftBehind() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { $0.path == style.path ? ["ETag": "\"first\""] : [:] } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(60)) + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + _ = try await library.downloadAssetBundle() + + // A changed manifest, whose stylesheet the server says is unchanged: that answer has a file too + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "2", style: style.absoluteString), + assetContent: "new content" + ) + _ = try await library.downloadAssetBundle() + + #expect(mockClient.downloadedFiles.count == 4) + #expect(mockClient.downloadedFiles.allSatisfy { !FileManager.default.fileExists(at: $0) }) + } + + @Test( + "a build leaves nothing in the temporary directory once its bundle is in place", + arguments: [EditorCachePolicy.always, .ignore] + ) + func buildLeavesNothingInTemporaryDirectory(cachePolicy: EditorCachePolicy) async throws { + let manifestJSON = uniqueManifestJSON( + identifier: "test-temporary-directory-\(UUID().uuidString)", + script: "https://example.com/script.js" + ) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifestJSON) + + let bundle = try await makeLibrary(httpClient: mockClient, cachePolicy: cachePolicy).downloadAssetBundle() + + #expect(bundle.hasAssetData(for: URL(string: "https://example.com/script.js")!)) + #expect(buildsLeftInTemporaryDirectory(ofManifest: bundle.id).isEmpty) + } + + @Test("a build that's cancelled leaves nothing in the temporary directory") + func cancelledBuildLeavesNothingInTemporaryDirectory() async throws { + let manifest = try LocalEditorAssetManifest( + remoteManifest: RemoteEditorAssetManifest( + data: Data( + uniqueManifestJSON( + identifier: "test-cancelled-temporary-directory-\(UUID().uuidString)", + script: "https://example.com/script.js" + ).utf8 + ) + ) + ) + + let session = ParkedURLSession() + defer { session.release() } + let library = makeLibrary(httpClient: EditorHTTPClient(urlSession: session, authHeader: "Bearer test-token")) + let destination = await library.bundleRoot(for: manifest.checksum).standardizedFileURL + + let build = Task { try await library.buildBundle(for: manifest) } + try await session.waitUntilStarted() + let abandoned = try #require(EditorAssetLibrary.inFlightBuilds.task(for: destination)) + #expect(buildsLeftInTemporaryDirectory(ofManifest: manifest.checksum).count == 1) + build.cancel() + + await #expect(throws: CancellationError.self) { try await build.value } + await abandoned.value + #expect(buildsLeftInTemporaryDirectory(ofManifest: manifest.checksum).isEmpty) + } + + /// The bundles for the manifest with `checksum` that are in the temporary directory itself, which is where a + /// build assembles one before putting it in the library's storage. + private func buildsLeftInTemporaryDirectory(ofManifest checksum: String) -> [URL] { + let directories = try? FileManager.default.contentsOfDirectory( + at: .temporaryDirectory, + includingPropertiesForKeys: nil + ) + return (directories ?? []).filter { + (try? EditorAssetBundle(url: $0.appending(path: "manifest.json")))?.id == checksum + } + } + @Test("builds of one bundle share one build, whichever library runs them") func buildsOfOneBundleShareOneBuild() async throws { let manifestJSON = """ @@ -1326,6 +1984,26 @@ struct EditorAssetLibraryTests { } } +// MARK: - Once-Only Action for Tests + +/// Runs its action the first time it's asked to, and never again. +private final class OnceOnly: @unchecked Sendable { + private let lock = NSLock() + private var action: (() -> Void)? + + init(_ action: @escaping () -> Void) { + self.action = action + } + + func run() { + let action = lock.withLock { + defer { self.action = nil } + return self.action + } + action?() + } +} + // MARK: - Progress Tracker for Tests final class ProgressTracker: @unchecked Sendable { @@ -1354,6 +2032,8 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck var downloadedURLs: [URL] = [] /// Requests made via `download(_:)`, in order. var downloadRequests: [URLRequest] = [] + /// The file each `download(_:)` handed back, in order. + var downloadedFiles: [URL] = [] private let lock = NSLock() /// Requests made via `perform(_:)`, in order. @@ -1372,6 +2052,10 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck /// Used by both `perform()` and `download()` methods. var urlResponseHandler: ((URL) throws -> Data) = { _ in Data() } + /// The headers the server sends with the asset at a URL — `ETag`, `Last-Modified`, `Content-Type`, or none. When a + /// download's request sends one back unchanged, the server answers 304 with no body, as a real one would. + var assetValidators: ((URL) -> [String: String]) = { _ in [:] } + func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) { let url = try #require(urlRequest.url) @@ -1401,18 +2085,35 @@ final class EditorAssetLibraryMockHTTPClient: EditorHTTPClientProtocol, @uncheck downloadRequests.append(urlRequest) } - let data = try urlResponseHandler(url) + let assetHeaders = assetValidators(url) + let isUnchanged = Self.asksOnlyForNewerCopy(urlRequest, than: assetHeaders) + + let data = isUnchanged ? Data() : try urlResponseHandler(url) let tempURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) try data.write(to: tempURL) + lock.withLock { downloadedFiles.append(tempURL) } let response = HTTPURLResponse( url: url, - statusCode: 200, + statusCode: isUnchanged ? 304 : 200, httpVersion: "HTTP/1.1", - headerFields: nil + headerFields: assetHeaders )! return (tempURL, response) } + + /// Whether `request` sends back the validator the server would send with the asset now. + private static func asksOnlyForNewerCopy(_ request: URLRequest, than assetHeaders: [String: String]) -> Bool { + if let etag = assetHeaders["ETag"] { + return request.value(forHTTPHeaderField: "If-None-Match") == etag + } + + if let lastModified = assetHeaders["Last-Modified"] { + return request.value(forHTTPHeaderField: "If-Modified-Since") == lastModified + } + + return false + } } From e4dac2713d9df6210649bdaeefc579f85e2d4ffb Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 5 Oct 2026 15:52:18 -0600 Subject: [PATCH 6/9] fix(ios): close the gaps a review found in falling back and in asset bundles A bundle that's missing an asset is never settled, so every prepare goes to the site to try the asset again. When the site couldn't be asked, that counted as the asset bundle not being fetched, and `prepare()` threw with the fallback disabled, though the bundle the cache policy asks for was on disk. One asset that had once failed to download made opening an editor depend on the site answering. A retry that can't be made is now logged and nothing more: the bundle is given as it is, and its missing assets are reported as they stand. A bundle stored before assets were kept in `assets/` read as missing every one of them: all to download again, and none to give an editor until the site could be reached. Its assets are now copied to where a bundle keeps them, in a new bundle beside it, the first time it's read or checked, without asking the site for anything. The copy keeps the old bundle's dates. It records each asset it copied as not refreshed, since such a bundle kept whatever a site answered with, so the next check of the manifest asks for them again. A file that two of the manifest's assets shared in the old layout is taken for neither. An asset that failed to download, with a copy on disk to stand in for it, was neither reported nor tried again: the bundle read as complete. The bundle now records which of its assets are such copies. `prepareAvailable()` reports them as `.assetsNotRefreshed`, a new kind of failure, when it went to the site for the bundle and came back without them, and the next check of the manifest asks for them again, even if their URLs have a version. The bundle is still given from disk without a check while the cache policy trusts it: it has everything an editor loads. A build that comes out the same as the bundle on disk keeps that bundle, and now records in it what the build learned: the headers its assets came with, and which of them failed to download. So a refresh can correct a `Content-Type` that was stored wrong, which it couldn't while the file itself was unchanged. Two bundles that differ in an asset's `Content-Type` are no longer equal, since an editor is served the asset with it. What's recorded for the assets a build kept without asking is what the bundle on disk records at that moment, not what it recorded when the build read it, which another service may have changed since. A bundle is now dated by when its manifest was fetched, which is when the site was found to match it, rather than by when it finished building. A slow build of an older manifest no longer ends up ahead of a bundle for a newer one, and the age a cache policy goes by starts from the fetch. A bundle that replaces its manifest's bundle is dated just after it, a check never moves a bundle's date back, and a library that joins a build dates the bundle by its own, later fetch. A bundle that another service has superseded since it was found is neither marked as the latest nor built over: the manifest's bundle is looked for again. A bundle dated more than a minute ahead of the clock, which a clock that was set back leaves behind, is dated afresh the first time it's read. What tells one asset from another is tightened: - A name that's percent-encoded but isn't UTF-8 decoded to nothing, so URLs like that on one host shared a file. It's kept as it's written. - A character that divides a URL up stays encoded: `a%2Fb.js` and `a/b.js` were one asset. An encoded dot is still a dot, as it is to a web view, and `..` doesn't go back past the root. - A file's extension comes from the same normalised URL as the rest of its name. Also: - A manifest that links one asset twice, or over both `http` and `https`, downloads it once, over `https`. The second download used to fail and take the first one's headers with it. - `EditorHTTPClient.download` removes the file of a response that's an error. No caller is handed it, so it was left in the temporary directory, once for every asset that failed on every prepare. - `EditorAssetBundle.isValidAssetPath(for:)`, which has nothing left to check, is kept as a deprecated method that's always `true` rather than removed. - `existingBundle(forManifestChecksum:)` reads the manifest's own bundles rather than every bundle on disk. `prepare()` still waits for every dependency before it throws with the fallback disabled. Throwing at the first failure was tried: it cancels the others, the asset bundle's download among them, so trying again starts from nothing. Each change in behaviour has a test that fails without it. --- docs/code/preloading.md | 22 +- .../Sources/EditorHTTPClient.swift | 6 + .../Sources/Model/EditorAssetBundle.swift | 175 +++- .../Sources/Model/EditorPreparation.swift | 7 +- .../Sources/Services/EditorService.swift | 70 +- .../Sources/Stores/EditorAssetLibrary.swift | 774 ++++++++++++---- .../EditorHTTPClientTests.swift | 21 + .../Model/EditorAssetBundleTests.swift | 227 +++++ .../Services/EditorServiceTests.swift | 157 +++- .../Stores/EditorAssetLibraryTests.swift | 834 +++++++++++++++++- 10 files changed, 2092 insertions(+), 201 deletions(-) diff --git a/docs/code/preloading.md b/docs/code/preloading.md index af61024a7..c6ee96a80 100644 --- a/docs/code/preloading.md +++ b/docs/code/preloading.md @@ -261,17 +261,26 @@ An `EditorService`'s cache policy covers plugin and theme assets as well as API | `.maxAge(TimeInterval)` | Fetched once older than the age | Manifest checked once the last check is older than the age | | `.ignore` | Always fetched | Manifest always checked, and every asset downloaded again | -Under `.always` and `.maxAge`, a check downloads only what the manifest says has changed. If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. Only an asset that failed to download when the bundle was built is tried again, and it goes into a new bundle beside the old one rather than into it — a bundle on disk is never changed, because an editor may be reading it. A try that gains nothing leaves no second bundle. A bundle that's missing assets is checked again on every prepare, whatever the policy, until it has them. If the manifest has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. An asset whose versioned URL a bundle on disk already has is copied from the latest bundle that has it. An asset whose URL has no `?ver=` is asked for again, because its URL can't say whether it changed. If its server sent an `ETag` or `Last-Modified` with the copy on disk, the request asks only for a newer copy, and the one on disk is kept when the server answers that it has none (a 304). An asset that fails to download is taken from the latest bundle on disk that has it, rather than left out. +Under `.always` and `.maxAge`, a check downloads only what the manifest says has changed. If the manifest hasn't changed, the bundle on disk is kept rather than downloaded again — asset URLs carry their version (`?ver=`), so the same manifest means the same assets — and its age starts over. Only an asset that failed to download when the bundle was built is asked for again; see [When an asset fails to download](#when-an-asset-fails-to-download). If the manifest has changed, the new bundle is built beside the old one, and every service for the site uses it once it's complete. An asset whose versioned URL a bundle on disk already has is copied from the latest bundle that has it. An asset whose URL has no `?ver=` is asked for again, because its URL can't say whether it changed. If its server sent an `ETag` or `Last-Modified` with the copy on disk, the request asks only for a newer copy, and the one on disk is kept when the server answers that it has none (a 304). That trusts the site's versions. WordPress gives an asset registered without a version its own version as `?ver=`, so such a file can change while its URL — and so the manifest — stays the same. Only `.ignore` downloads it again. -Under `.ignore`, nothing on disk is taken to be valid, so every asset is downloaded in full whether or not the manifest has changed. That makes it the way to replace an asset that changed without its URL changing, or one that was stored wrong. The assets go into a new bundle beside any the manifest already has: a bundle on disk is never changed, because an editor may be reading it. An asset that fails to download is taken from the latest bundle on disk that has it, rather than left out. If the manifest hasn't changed and its assets all come back the same as the bundle on disk has them, that bundle is returned and the new one is discarded, so a host can tell whether a refresh changed anything, and a refresh that changed nothing takes no more disk space. +Under `.ignore`, nothing on disk is taken to be valid, so every asset is downloaded in full whether or not the manifest has changed. That makes it the way to replace an asset that changed without its URL changing, or one that was stored wrong. The assets go into a new bundle beside any the manifest already has: a bundle on disk is never changed, because an editor may be reading it. If the manifest hasn't changed and its assets all come back the same as the bundle on disk has them, that bundle is returned and the new one is discarded, so a host can tell whether a refresh changed anything, and a refresh that changed nothing takes no more disk space. The bundle that's kept still records what the refresh learned: the headers its assets came back with, and which of them failed to download. An asset that comes back with a different `Content-Type` is a change — the editor is served it with that — so the bundle returned isn't equal to the one a host held before. The old bundle stays on disk for as long as the app is running, because an open editor — or dependencies the host prepared earlier and still holds — may be reading it. `cleanup()` removes it after the next launch. -A bundle holds every link that a `") + let bundle = try EditorAssetBundle( + manifest: manifest, + assetsNotRefreshed: [EditorAssetBundle.assetKey(for: asset)], + bundleRoot: FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + ) + try bundle.writeManifest(editorRepresentation: .empty) + + let loaded = try EditorAssetBundle(url: bundle.bundleRoot.appending(path: "manifest.json")) + + #expect(loaded.assetsNotRefreshed == [EditorAssetBundle.assetKey(for: asset)]) + // It says how the bundle came to be, not what it holds + #expect( + loaded + == (try EditorAssetBundle( + manifest: manifest, + downloadDate: loaded.downloadDate, + bundleRoot: loaded.bundleRoot + )) + ) } // MARK: - URL Initialization Tests @@ -366,6 +392,117 @@ struct EditorAssetBundleTests { #expect(bundle.assetDataPath(for: try #require(URL(string: other))) != bundle.assetDataPath(for: asset)) } + /// A name that's percent-encoded but isn't UTF-8 — a directory named in Latin-1, say — decodes to + /// nothing at all. Its URL is told apart as it's written instead. + @Test( + "assets whose URLs are encoded as something other than text are stored apart", + arguments: [ + ("https://example.com/caf%E9/a.js", "https://example.com/caf%E9/b.css"), + ("https://example.com/caf%E9/a.js", "https://example.com/th%E9/a.js"), + ("https://example.com/a.js?x=caf%E9", "https://example.com/a.js?x=th%E9"), + ("https://example.com/a.js?x=caf%E9", "https://example.com/a.js"), + ] + ) + func assetsWithEncodingThatIsNotTextAreStoredApart(first: String, second: String) throws { + let bundle = makeBundle() + + #expect( + bundle.assetDataPath(for: try #require(URL(string: first))) + != bundle.assetDataPath(for: try #require(URL(string: second))) + ) + } + + /// A character that divides a URL up means something else when it's encoded: `a%2Fb.js` is one file's + /// name, and `a/b.js` is a file in a directory. + @Test( + "assets whose URLs differ in whether a dividing character is encoded are stored apart", + arguments: [ + ("https://example.com/a%2Fb/app.js", "https://example.com/a/b/app.js"), + ("https://example.com/a%3Fb.js", "https://example.com/a?b.js"), + ("https://example.com/app.js?x=a%26y%3D1", "https://example.com/app.js?x=a&y=1"), + ("https://example.com/app.js?x=a%2Bb", "https://example.com/app.js?x=a+b"), + ("https://example.com/100%25.js", "https://example.com/100%2525.js"), + ] + ) + func assetsThatDifferInEncodedDelimitersAreStoredApart(first: String, second: String) throws { + let bundle = makeBundle() + + #expect( + bundle.assetDataPath(for: try #require(URL(string: first))) + != bundle.assetDataPath(for: try #require(URL(string: second))) + ) + } + + /// However else a URL is written, it's the same asset: a web view may write it another way. + @Test( + "assets whose URLs differ only in how they're written are stored together", + arguments: [ + ("https://example.com/my%20plugin/app.js", "https://example.com/my plugin/app.js"), + ("https://example.com/caf%C3%A9/app.js", "https://example.com/café/app.js"), + ("https://example.com/a%2fb/app.js", "https://example.com/a%2Fb/app.js"), + ("https://example.com/app.js?x=%7B1%7D", "https://example.com/app.js?x={1}"), + ("https://example.com/%61pp.js", "https://example.com/app.js"), + ("https://example.com/a/%2E%2E/app.js", "https://example.com/app.js"), + ("https://example.com/a/%2e/app.js", "https://example.com/a/app.js"), + // What can't be decoded is still the same bytes, whichever case they're written in + ("https://example.com/caf%e9/app.js", "https://example.com/caf%E9/app.js"), + // The whole of a file's name comes of what the URL leads to, its extension included + ("https://example.com/caf%E9/../app.js", "https://example.com/app.js"), + // Nothing is above a site's root, so going back from there goes nowhere + ("https://example.com/a/../../app.js", "https://example.com/app.js"), + ("https://example.com/%2E%2E/%2E%2E/app.js", "https://example.com/app.js"), + ] + ) + func assetsThatDifferOnlyInHowTheyAreWrittenAreStoredTogether(first: String, second: String) throws { + let bundle = makeBundle() + + #expect( + bundle.assetDataPath(for: try #require(URL(string: first))) + == bundle.assetDataPath(for: try #require(URL(string: second))) + ) + } + + // MARK: - Earlier Layout Tests + + @Test("legacyAssetLocation gives the file at an asset's URL path, where a bundle used to keep it") + func legacyAssetLocationFindsFileAtURLPath() throws { + let bundle = makeBundle(bundleRoot: URL.randomTemporaryDirectory.appending(path: "bundle")) + let asset = try #require(URL(string: "https://example.com/wp-content/plugins/a%20plugin/app.js?ver=1")) + let file = bundle.bundleRoot.appending(path: "wp-content/plugins/a plugin/app.js") + try write("content", to: file) + + let location = try #require(bundle.legacyAssetLocation(for: asset)) + + #expect(try Data(contentsOf: location) == Data("content".utf8)) + } + + /// Only a file inside the bundle that isn't one of the bundle's own can be an asset it kept. + @Test( + "legacyAssetLocation gives nothing where a bundle never kept an asset", + arguments: [ + "https://example.com/wp-content/missing.js", + "https://example.com/", + "https://example.com/wp-content/", + "https://example.com/wp-content/../../outside.js", + "https://example.com/%2E%2E/outside.js", + "https://example.com/manifest.json", + "https://example.com/editor-representation.json", + // The same files, on a volume that doesn't tell capitals apart + "https://example.com/Manifest.json", + "https://example.com/EDITOR-REPRESENTATION.JSON", + ] + ) + func legacyAssetLocationIsNilWhereNoAssetWasKept(link: String) throws { + let directory = URL.randomTemporaryDirectory + let bundle = makeBundle(bundleRoot: directory.appending(path: "bundle")) + try write("content", to: bundle.bundleRoot.appending(path: "wp-content/app.js")) + try write("{}", to: bundle.bundleRoot.appending(path: "manifest.json")) + try write("{}", to: bundle.bundleRoot.appending(path: "editor-representation.json")) + try write("outside", to: directory.appending(path: "outside.js")) + + #expect(bundle.legacyAssetLocation(for: try #require(URL(string: link))) == nil) + } + /// A bundle on disk can be read by someone looking for an asset. @Test("an asset's file is named for its URL") func assetFileIsNamedForItsURL() throws { @@ -378,6 +515,14 @@ struct EditorAssetBundleTests { #expect(name.hasSuffix(".js")) } + /// Only a path leads to a file with an extension. A query can have a `/` and a `.` in it too. + @Test("an asset's file takes no extension from its URL's query") + func assetFileTakesNoExtensionFromQuery() throws { + let asset = try #require(URL(string: "https://example.com?load=a/b.js")) + + #expect(!EditorAssetBundle.assetFileName(for: asset).hasSuffix(".js")) + } + @Test("an asset's file name fits the file system however long its URL is, and still tells assets apart") func assetFileNameFitsFileSystem() throws { let bundle = makeBundle() @@ -494,6 +639,88 @@ struct EditorAssetBundleTests { #expect(bundle1 != bundle2) } + /// An editor is served each asset with the headers its bundle holds for it. + @Test("Bundles that hold different headers for their assets are not equal") + func bundlesWithDifferentAssetHeadersNotEqual() throws { + let manifest = try createManifest(scripts: "") + let key = EditorAssetBundle.assetKey(for: URL(string: "https://example.com/app.js")!) + let date = Date() + let root = URL.randomTemporaryDirectory + + let plain = try EditorAssetBundle( + manifest: manifest, + downloadDate: date, + assetHeaders: [key: .init(contentType: "text/plain")], + bundleRoot: root + ) + let script = try EditorAssetBundle( + manifest: manifest, + downloadDate: date, + assetHeaders: [key: .init(contentType: "application/javascript")], + bundleRoot: root + ) + + #expect(plain != script) + #expect(plain.hashValue != script.hashValue) + } + + /// A server can write the same type differently from one answer to the next. + @Test("Bundles that differ only in how their assets' types are written are equal") + func bundlesWithDifferentlyWrittenContentTypesAreEqual() throws { + let manifest = try createManifest(scripts: "") + let key = EditorAssetBundle.assetKey(for: URL(string: "https://example.com/app.js")!) + let date = Date() + let root = URL.randomTemporaryDirectory + + let first = try EditorAssetBundle( + manifest: manifest, + downloadDate: date, + assetHeaders: [key: .init(contentType: "text/css; charset=UTF-8")], + bundleRoot: root + ) + let second = try EditorAssetBundle( + manifest: manifest, + downloadDate: date, + assetHeaders: [key: .init(contentType: "text/css;charset=utf-8")], + bundleRoot: root + ) + + #expect(first == second) + #expect(first.hashValue == second.hashValue) + } + + /// What tells one version of an asset from another is for asking its server, not for an editor. A + /// server can send a new one with the same file. + @Test("Bundles that differ only in how their assets' servers tell versions apart are equal") + func bundlesWithDifferentValidatorsAreEqual() throws { + let manifest = try createManifest(scripts: "") + let key = EditorAssetBundle.assetKey(for: URL(string: "https://example.com/app.js")!) + let date = Date() + let root = URL.randomTemporaryDirectory + + let first = try EditorAssetBundle( + manifest: manifest, + downloadDate: date, + assetHeaders: [key: .init(contentType: "application/javascript", etag: "\"first\"")], + bundleRoot: root + ) + let second = try EditorAssetBundle( + manifest: manifest, + downloadDate: date, + assetHeaders: [ + key: .init( + contentType: "application/javascript", + etag: "\"second\"", + lastModified: "Wed, 30 Sep 2026 21:43:35 GMT" + ) + ], + bundleRoot: root + ) + + #expect(first == second) + #expect(first.hashValue == second.hashValue) + } + @Test("Bundles with different downloadDates are not equal") func bundlesWithDifferentDatesNotEqual() throws { let manifest = try createManifest(blockTypes: ["core/paragraph"]) diff --git a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift index 692e0037a..d1d181d1f 100644 --- a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift +++ b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift @@ -470,7 +470,10 @@ struct EditorServiceTests: MakesTestFixtures { #expect(site.client.downloadCallCount == 3) } - @Test("a bundle that's missing an asset is still given when the site can't be asked about it") + /// The cache policy asks for no more than the bundle on disk. Trying its missing asset again is worth + /// asking the site for, but the bundle doesn't depend on the answer — so no dependency went unfetched, + /// and nothing throws for want of a fallback. + @Test("a bundle that's missing an asset is still given when the site can't be asked about it, even without a fallback") func bundleMissingAssetIsGivenWhenSiteCannotBeAsked() async throws { let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) site.failure = { $0.path == Self.pluginScript.path ? Self.notFound($0) : nil } @@ -480,8 +483,149 @@ struct EditorServiceTests: MakesTestFixtures { let preparation = try await site.service(cachePolicy: .always).prepareAvailable() #expect(preparation.dependencies.assetBundle == gapped) - #expect(dependenciesNotFetched(in: preparation) == [.assetBundle: true]) + #expect(dependenciesNotFetched(in: preparation).isEmpty) #expect(assetsMissing(in: preparation) == [Self.pluginScript]) + #expect(try await site.service(cachePolicy: .always).prepare().assetBundle == gapped) + } + + @Test("a bundle that's missing an asset is still given when the site answers the check with an error") + func bundleMissingAssetIsGivenWhenManifestCheckErrors() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.failure = { $0.path == Self.pluginScript.path ? Self.notFound($0) : nil } + let gapped = try await site.service(cachePolicy: .always).prepare().assetBundle + + // The site no longer has the endpoint that serves its manifest + site.failure = { Self.notFound($0) } + + #expect(try await site.service(cachePolicy: .always).prepare().assetBundle == gapped) + } + + /// The bundle that was trusted when the prepare began isn't the one to give if there's a newer one by + /// the time the site turns out not to answer: another service may have published it in between. + @Test("a bundle that's missing an asset gives way to one published while the site couldn't be asked") + func bundleMissingAssetGivesWayToOnePublishedMeanwhile() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.failure = { $0.path == Self.pluginScript.path ? Self.notFound($0) : nil } + let gapped = try await site.service(cachePolicy: .always).prepare().assetBundle + let publishedRoot = gapped.bundleRoot.deletingLastPathComponent().appending(path: "\(gapped.id)-published") + + // While the manifest is being asked for, and failing, a bundle with the asset is published + site.failure = { _ in + if !FileManager.default.fileExists(atPath: publishedRoot.path) { + try? FileManager.default.copyItem(at: gapped.bundleRoot, to: publishedRoot) + if let published = try? EditorAssetBundle( + manifest: gapped.manifest, + downloadDate: Date(), + lastCheckedDate: Date(), + bundleRoot: publishedRoot + ) { + try? published.writeManifest() + try? FileManager.default.createDirectory( + at: published.assetDataPath(for: Self.pluginScript).deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try? Data("script".utf8).write(to: published.assetDataPath(for: Self.pluginScript)) + } + } + return URLError(.timedOut) + } + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.dependencies.assetBundle.bundleRoot.lastPathComponent == publishedRoot.lastPathComponent) + #expect(preparation.dependencies.assetBundle.hasAssetData(for: Self.pluginScript)) + #expect(preparation.isComplete) + } + + /// The copy on disk stands in for the asset, and the failure isn't hidden behind it. + @Test("an asset that fails to download in a refresh is reported, and asked for again at each check until it downloads") + func assetNotRefreshedIsReportedAndAskedForAgain() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let prepared = try await site.service(cachePolicy: .always).prepare() + + site.failure = { $0.path == Self.pluginScript.path ? URLError(.timedOut) : nil } + let refreshed = try await site.service(cachePolicy: .ignore).prepareAvailable() + + #expect(refreshed.dependencies.assetBundle == prepared.assetBundle) + #expect(assetsNotRefreshed(in: refreshed) == [Self.pluginScript]) + #expect(assetsMissing(in: refreshed).isEmpty) + #expect(dependenciesNotFetched(in: refreshed).isEmpty) + + // The bundle has everything an editor loads, and it's what `.always` asks for: the next editor + // doesn't wait on the site for it, and has nothing to report, because nothing was asked. + let requests = site.client.requests.count + site.client.downloadCallCount + let opened = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(opened.isComplete) + #expect(opened.dependencies.assetBundle == prepared.assetBundle) + #expect(site.client.requests.count + site.client.downloadCallCount == requests) + + // The next check of the manifest asks for the asset again, though its URL has a version + let downloads = site.client.downloadCallCount + let checked = try await site.service(cachePolicy: .maxAge(0)).prepareAvailable() + + #expect(assetsNotRefreshed(in: checked) == [Self.pluginScript]) + #expect(site.client.downloadCallCount == downloads + 1) + + site.failure = nil + let settled = try await site.service(cachePolicy: .maxAge(0)).prepareAvailable() + + #expect(settled.isComplete) + #expect(settled.dependencies.assetBundle == prepared.assetBundle) + #expect(try await site.service(cachePolicy: .always).fetchAssetBundleCount() == 1) + + // And once it has it, a check has nothing left to ask for + let settledDownloads = site.client.downloadCallCount + _ = try await site.service(cachePolicy: .maxAge(0)).prepare() + #expect(site.client.downloadCallCount == settledDownloads) + } + + /// An app update mustn't cost a site its assets while the site can't be reached. + @Test("a bundle stored before assets were named for their URLs gives its assets when the site can't be reached") + func bundleStoredAtAssetPathsGivesItsAssetsOffline() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + let stored = try await site.service(cachePolicy: .always).prepare().assetBundle + // As it was stored then: at its URL's path + try FileManager.default.moveItem( + at: stored.assetDataPath(for: Self.pluginScript), + to: stored.bundleRoot.appending(path: "plugin.js") + ) + + site.isOffline = true + let preparation = try await site.service(cachePolicy: .always).prepareAvailable() + + #expect(preparation.isComplete) + #expect(preparation.dependencies.assetBundle.hasAssetData(for: Self.pluginScript)) + #expect(site.client.downloadCallCount == 1) + + // Nor when the site was to be asked and couldn't be: the bundle wasn't fetched, and that's all + // there is to say about it. None of its assets was asked for, so none failed to download. + let refresh = try await site.service(cachePolicy: .ignore).prepareAvailable() + + #expect(refresh.dependencies.assetBundle.hasAssetData(for: Self.pluginScript)) + #expect(dependenciesNotFetched(in: refresh)[.assetBundle] == true) + #expect(assetsNotRefreshed(in: refresh).isEmpty) + } + + /// Not at the first failure: by the time it throws, whatever could be fetched is stored, so trying + /// again starts from there. + @Test("with no fallback, a dependency that can't be fetched throws once the others have been fetched and stored") + func failureThrowsOnceOtherDependenciesAreStored() async throws { + let site = TestSite(configuration: makeConfiguration(), manifest: Self.pluginManifest(version: "1")) + site.failure = { $0.absoluteString.contains("wp-block-editor/v1/settings") ? Self.notFound($0) : nil } + + await #expect(throws: EditorHTTPClient.ClientError.self) { + try await site.service(cachePolicy: .always).prepare() + } + #expect(try await site.service(cachePolicy: .always).fetchAssetBundleCount() == 1) + + // Trying again asks only for what failed + site.failure = nil + let requests = site.client.requests.count + let prepared = try await site.service(cachePolicy: .always).prepare() + + #expect(prepared.assetBundle.hasAssetData(for: Self.pluginScript)) + #expect(site.client.requests.count == requests + 1) + #expect(site.client.downloadCallCount == 1) } /// A copy that can't be read is no copy, and the site can still be asked. @@ -624,6 +768,15 @@ struct EditorServiceTests: MakesTestFixtures { return assets } + /// The assets `preparation` says its bundle holds an earlier copy of. + private func assetsNotRefreshed(in preparation: EditorPreparation) -> [URL] { + var assets: [URL] = [] + for case .assetsNotRefreshed(let notRefreshed) in preparation.failures { + assets += notRefreshed + } + return assets + } + /// The error a site answers with when it has nothing at a URL. private static func notFound(_ url: URL) -> any Error { EditorHTTPClient.ClientError.unknown(response: Data(), statusCode: 404, requestURL: url) diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 1c5773700..5a4162682 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -540,6 +540,265 @@ struct EditorAssetLibraryTests { #expect(try refreshed.assetData(for: Self.scriptURL) == Data("mock content".utf8)) } + /// The bundle on disk is still right, so it's kept. What the server says about its assets now is still + /// what an editor should be served them with. + @Test("under .ignore, a bundle whose assets come back the same takes the headers they came back with") + func unchangedBundleTakesNewHeadersUnderIgnore() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { _ in ["Content-Type": "text/plain"] } + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + let bundle = try await library.downloadAssetBundle() + + // The server is put right: the same file, served as what it is + mockClient.assetValidators = { _ in ["Content-Type": "application/javascript"] } + let refreshed = try await library.downloadAssetBundle() + + // The same bundle on disk, but not one a host comparing dependencies should take for unchanged + #expect(refreshed.bundleRoot == bundle.bundleRoot) + #expect(refreshed != bundle) + #expect(refreshed.contentType(forAssetAt: Self.scriptURL) == "application/javascript") + #expect( + try await library.readAssetBundles().first?.contentType(forAssetAt: Self.scriptURL) + == "application/javascript" + ) + #expect(try FileManager.default.contentsOfDirectory(atPath: storageRoot.path).count == 1) + } + + /// The bundle on disk is kept, but the failure isn't hidden behind it: the next check of the manifest + /// asks for the asset again. + @Test("under .ignore, a bundle kept because its assets failed to download records that they weren't refreshed") + func bundleKeptUnderIgnoreRecordsAssetsNotRefreshed() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let refreshing = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + let bundle = try await refreshing.downloadAssetBundle() + + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let refreshed = try await refreshing.downloadAssetBundle() + + #expect(refreshed == bundle) + #expect(await refreshing.assetsNotRefreshed(in: refreshed) == [Self.scriptURL]) + #expect(await refreshing.missingAssets(of: refreshed).isEmpty) + + // It has everything an editor loads, so an editor's own library has no reason to ask the site + let library = makeLibrary(httpClient: mockClient, cachePolicy: .always, storageRoot: storageRoot) + #expect(try await library.readLatestAssetBundle() == bundle) + + // A check asks for the asset again, though its URL has a version and a copy of it is on disk + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let downloads = mockClient.downloadCallCount + let settled = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == downloads + 1) + // It's the asset the bundle already had, so it's the same bundle, with nothing left to ask for + #expect(settled == bundle) + #expect(await library.assetsNotRefreshed(in: settled).isEmpty) + #expect(try await library.readLatestAssetBundle() == bundle) + #expect(try FileManager.default.contentsOfDirectory(atPath: storageRoot.path).count == 1) + } + + @Test("an asset that wasn't refreshed is asked for again each time the manifest is checked, until it downloads") + func assetNotRefreshedIsAskedForAgain() async throws { + let style = URL(string: "https://example.com/plugin.css")! + let mockClient = EditorAssetLibraryMockHTTPClient() + let library = makeLibrary(httpClient: mockClient, cachePolicy: .always) + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + ) + _ = try await library.downloadAssetBundle() + + // A changed manifest, whose stylesheet can't be downloaded + let changedManifest = Self.manifestJSON(scriptVersion: "2", style: style.absoluteString) + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(changedManifest.utf8) + } + guard url.path != style.path else { throw URLError(.timedOut) } + return Data("new content".utf8) + } + let changed = try await library.downloadAssetBundle() + let downloads = mockClient.downloadCallCount + + // It still can't: it's asked for and nothing else is, and the bundle is kept as it is + let again = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == downloads + 1) + #expect(mockClient.downloadedURLs.last == style) + #expect(again == changed) + #expect(await library.assetsNotRefreshed(in: again) == [style]) + + // It downloads: the bundle that has it is the site's latest, and there's nothing left to ask for + mockClient.urlResponseHandler = Self.responses(forManifest: changedManifest, assetContent: "new content") + let refreshed = try await library.downloadAssetBundle() + + #expect(try refreshed.assetData(for: style) == Data("new content".utf8)) + #expect(await library.assetsNotRefreshed(in: refreshed).isEmpty) + #expect(try await library.readLatestAssetBundle() == refreshed) + // An editor may still be reading the bundle that held the earlier copy + #expect(try changed.assetData(for: style) == Data("mock content".utf8)) + } + + /// One file to download, however many times it's linked. + @Test("an asset a manifest links more than once is downloaded once, and keeps its headers") + func assetLinkedMoreThanOnceIsDownloadedOnce() async throws { + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.assetValidators = { _ in ["Content-Type": "application/javascript", "ETag": "\"first\""] } + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient) + + let bundle = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == 1) + #expect(bundle.contentType(forAssetAt: Self.scriptURL) == "application/javascript") + #expect(bundle.headers(for: Self.scriptURL)?.etag == "\"first\"") + #expect(await library.missingAssets(of: bundle).isEmpty) + } + + /// Over `http`, a request may not be allowed at all. + @Test("an asset a manifest links over both http and https is asked for over https") + func assetLinkedOverBothSchemesIsAskedForOverHTTPS() async throws { + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient) + + let bundle = try await library.downloadAssetBundle() + + #expect(mockClient.downloadedURLs == [Self.scriptURL]) + #expect(await library.missingAssets(of: bundle).isEmpty) + } + + /// A copy that came over `http` isn't one to take in place of asking over `https`. + @Test("a changed manifest's bundle asks again for an asset the latest bundle has by another scheme") + func changedBundleAsksAgainForAssetLinkedByAnotherScheme() async throws { + func manifest(scheme: String, blockType: String) -> String { + """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["\(blockType)"] + } + """ + } + let mockClient = EditorAssetLibraryMockHTTPClient() + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + mockClient.urlResponseHandler = Self.responses(forManifest: manifest(scheme: "http", blockType: "one")) + _ = try await library.downloadAssetBundle() + + mockClient.urlResponseHandler = Self.responses( + forManifest: manifest(scheme: "https", blockType: "two"), + assetContent: "new content" + ) + let changed = try await library.downloadAssetBundle() + + #expect(mockClient.downloadedURLs.last == Self.scriptURL) + #expect(try changed.assetData(for: Self.scriptURL) == Data("new content".utf8)) + } + + /// Another library can record something in a bundle between this one reading it and marking it: a + /// refresh whose asset failed to download, here. Marking the bundle mustn't write that away. + @Test("a check that keeps a bundle leaves what another library recorded in it meanwhile") + func checkKeepsWhatAnotherLibraryRecordedInBundle() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(0)) + let bundle = try #require(try await library.readAssetBundles().first) + let scriptKey = EditorAssetBundle.assetKey(for: Self.scriptURL) + + // The check has found the bundle on disk, and hasn't marked it yet + let checked = try await library.downloadAssetBundle { _ in + try? EditorAssetBundle( + manifest: bundle.manifest, + downloadDate: bundle.downloadDate, + lastCheckedDate: bundle.lastCheckedDate, + assetsNotRefreshed: [scriptKey], + bundleRoot: bundle.bundleRoot + ).writeManifest() + } + + #expect(checked.assetsNotRefreshed == [scriptKey]) + #expect(try await library.readAssetBundles().first?.assetsNotRefreshed == [scriptKey]) + } + + /// The build kept the stylesheet from the bundle on disk without asking for it, so it knows nothing + /// new about it. What another library recorded about it while the build ran stands, whether the build + /// ends up keeping that bundle or publishing its own. + @Test( + "a build records the assets it kept from the bundle on disk as that bundle records them when it finishes", + arguments: [true, false] + ) + func buildRecordsKeptAssetsAsBundleOnDiskRecordsThem(scriptDownloads: Bool) async throws { + let style = URL(string: "https://example.com/plugin.css")! + let manifest = Self.manifestJSON(scriptVersion: "1", style: style.absoluteString) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifest.utf8) + } + guard url.path == style.path else { throw URLError(.timedOut) } + return Data("mock content".utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + let gapped = try await library.downloadAssetBundle() + #expect(await library.missingAssets(of: gapped) == [Self.scriptURL]) + + // While the check asks for the script again, another library's refresh records that the + // stylesheet failed to download + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(manifest.utf8) + } + try EditorAssetBundle( + manifest: gapped.manifest, + downloadDate: gapped.downloadDate, + lastCheckedDate: gapped.lastCheckedDate, + assetsNotRefreshed: [EditorAssetBundle.assetKey(for: style)], + bundleRoot: gapped.bundleRoot + ).writeManifest() + guard scriptDownloads else { throw URLError(.timedOut) } + return Data("mock content".utf8) + } + let checked = try await library.downloadAssetBundle() + + #expect((checked.bundleRoot == gapped.bundleRoot) == !scriptDownloads) + #expect(await library.assetsNotRefreshed(in: checked) == [style]) + } + + /// Looking at a bundle isn't handing it out. One that's passed over for missing an asset is as free to + /// be cleaned up as it was. + @Test("cleanup removes a bundle that was only passed over for missing an asset") + func cleanupRemovesBundlePassedOverForMissingAsset() async throws { + let storageRoot = URL.randomTemporaryDirectory + let planted = try await plantBundles(forManifests: [Self.manifestJSON(scriptVersion: "1")], in: storageRoot) + try FileManager.default.removeItem(at: planted[0].assetDataPath(for: Self.scriptURL)) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, storageRoot: storageRoot) + + #expect(try await library.readLatestAssetBundle() == nil) + let repaired = try await library.downloadAssetBundle() + try await library.cleanup() + + #expect(try await library.readAssetBundles().map(\.bundleRoot) == [repaired.bundleRoot]) + } + @Test("downloadAssetBundle downloads an asset that an earlier build of the bundle failed to") func downloadAssetBundleRepairsMissingAsset() async throws { let mockClient = EditorAssetLibraryMockHTTPClient() @@ -560,51 +819,513 @@ struct EditorAssetLibraryTests { #expect(try await library.readAssetBundles().first?.bundleRoot == repaired.bundleRoot) } - /// A bundle used to keep each asset at its URL's path. Its assets aren't where a bundle looks now, so it - /// reads as one that's missing them, and is repaired the way one is. - @Test("a bundle stored before assets were named for their URLs is downloaded again, once") - func bundleStoredAtAssetPathsIsDownloadedAgain() async throws { + /// A bundle used to keep each asset at its URL's path, where a bundle doesn't look now. Its assets are + /// copied to where one does, so that an update costs a site neither a download nor its assets while it + /// can't be reached. + @Test("a bundle stored before assets were named for their URLs keeps its assets, without downloading them again") + func bundleStoredAtAssetPathsKeepsItsAssets() async throws { + let storageRoot = URL.randomTemporaryDirectory let mockClient = EditorAssetLibraryMockHTTPClient() mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) - let library = makeLibrary(httpClient: mockClient, cachePolicy: .always) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .always, storageRoot: storageRoot) let stored = try await library.downloadAssetBundle() // As it was stored then + let storedAsset = stored.bundleRoot.appending(path: "plugin.js") + try FileManager.default.moveItem(at: stored.assetDataPath(for: Self.scriptURL), to: storedAsset) + + let latest = try #require(try await library.readLatestAssetBundle()) + + #expect(latest.id == stored.id) + #expect(try latest.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + #expect(mockClient.downloadCallCount == 1) + // The site wasn't asked anything, so the bundle is no newer than it was + #expect(latest.downloadDate == stored.downloadDate) + #expect(latest.lastCheckedDate == stored.lastCheckedDate) + // An editor may be reading the bundle as it was stored + #expect(latest.bundleRoot != stored.bundleRoot) + #expect(FileManager.default.fileExists(at: storedAsset)) + // It's an earlier bundle's copy of the asset, to ask for again when the manifest is next checked + #expect(await library.assetsNotRefreshed(in: latest) == [Self.scriptURL]) + + // And once is enough + #expect(try await library.readLatestAssetBundle() == latest) + #expect(try await library.readLatestAssetBundleOnDisk() == latest) + #expect(try FileManager.default.contentsOfDirectory(atPath: storageRoot.path).count == 2) + } + + /// The copy has to come before the bundle it was made from, wherever that bundle is. Otherwise the next + /// read finds the old one again, and copies it again. + @Test("a bundle stored before assets were named for their URLs is copied once, whatever its directory is called") + func bundleStoredAtAssetPathsIsCopiedOnceWhateverItsDirectory() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .always, storageRoot: storageRoot) + let stored = try await library.downloadAssetBundle() try FileManager.default.moveItem( at: stored.assetDataPath(for: Self.scriptURL), to: stored.bundleRoot.appending(path: "plugin.js") ) - #expect(try await library.readLatestAssetBundle() == nil) + // Not the manifest's own directory, which a refresh's bundle doesn't get when another has it + try FileManager.default.moveItem( + at: stored.bundleRoot, + to: storageRoot.appending(path: "\(stored.id)-refreshed") + ) - let repaired = try await library.downloadAssetBundle() + let latest = try #require(try await library.readLatestAssetBundle()) - #expect(repaired.id == stored.id) - #expect(try repaired.assetData(for: Self.scriptURL) == Data("mock content".utf8)) - #expect(mockClient.downloadCallCount == 2) - #expect(try await library.readLatestAssetBundle() == repaired) + #expect(latest.hasAssetData(for: Self.scriptURL)) + #expect(try await library.readLatestAssetBundle() == latest) + #expect(try await library.readLatestAssetBundleOnDisk() == latest) + #expect(try FileManager.default.contentsOfDirectory(atPath: storageRoot.path).count == 2) + } + + /// A bundle kept one file at a path for every asset with that path, whichever host or query each was + /// asked for by. There's no telling which of them the file holds. + @Test("a file that a bundle stored before assets were named for their URLs kept for two assets is taken for neither") + func fileKeptForTwoAssetsInEarlierLayoutIsTakenForNeither() async throws { + let storageRoot = URL.randomTemporaryDirectory + let cdnScript = URL(string: "https://cdn.example.com/plugin.js?ver=1")! + let style = URL(string: "https://example.com/plugin.css")! + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient, storageRoot: storageRoot) + let stored = try await library.downloadAssetBundle() + // As it was stored then: one file for both scripts, and one for the stylesheet + try FileManager.default.moveItem( + at: stored.assetDataPath(for: Self.scriptURL), + to: stored.bundleRoot.appending(path: "plugin.js") + ) + try FileManager.default.removeItem(at: stored.assetDataPath(for: cdnScript)) + try FileManager.default.moveItem( + at: stored.assetDataPath(for: style), + to: stored.bundleRoot.appending(path: "plugin.css") + ) + + let onDisk = try #require(try await library.readLatestAssetBundleOnDisk()) + + #expect(onDisk.hasAssetData(for: style)) + #expect(await library.missingAssets(of: onDisk) == [Self.scriptURL, cdnScript]) } - /// A bundle's download date is when its build began. Going by that alone, a bundle another service - /// matched while this one was building would still look like the latest. - @Test("a bundle is the site's latest from the moment it's built, whatever was matched while it was building") - func builtBundleIsLatestOnceBuilt() async throws { + /// Such a bundle kept whatever its site answered with, so a check asks for its assets again. They're + /// still what there is to fall back on, wherever the bundle kept them. + @Test( + "a check of a bundle stored before assets were named for their URLs asks for its assets again, and keeps them if that fails", + arguments: [EditorCachePolicy.maxAge(0), .ignore] + ) + func checkOfBundleStoredAtAssetPathsAsksForItsAssetsAgain(cachePolicy: EditorCachePolicy) async throws { + let storageRoot = URL.randomTemporaryDirectory let mockClient = EditorAssetLibraryMockHTTPClient() mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: cachePolicy, storageRoot: storageRoot) + let stored = try await library.downloadAssetBundle() + try FileManager.default.moveItem( + at: stored.assetDataPath(for: Self.scriptURL), + to: stored.bundleRoot.appending(path: "plugin.js") + ) + + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + let checked = try await library.downloadAssetBundle() + + #expect(mockClient.downloadCallCount == 2) + #expect(try checked.assetData(for: Self.scriptURL) == Data("mock content".utf8)) + #expect(await library.assetsNotRefreshed(in: checked) == [Self.scriptURL]) + + // It downloads: there's nothing left to ask for + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let settled = try await library.downloadAssetBundle() + + #expect(settled.bundleRoot == checked.bundleRoot) + #expect(await library.assetsNotRefreshed(in: settled).isEmpty) + } + + /// A copy nobody was given is as free to be cleaned up as the bundle it was made from. + @Test("cleanup removes the copy of a bundle stored before assets were named for their URLs, once it's superseded") + func cleanupRemovesSupersededCopyOfBundleStoredAtAssetPaths() async throws { + let storageRoot = URL.randomTemporaryDirectory + let planted = try await plantBundles(forManifests: [Self.manifestJSON(scriptVersion: "1")], in: storageRoot) + try FileManager.default.moveItem( + at: planted[0].assetDataPath(for: Self.scriptURL), + to: planted[0].bundleRoot.appending(path: "plugin.js") + ) + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1"), + assetContent: "new content" + ) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0), storageRoot: storageRoot) + + let checked = try await library.downloadAssetBundle() + try await library.cleanup() + + #expect(try checked.assetData(for: Self.scriptURL) == Data("new content".utf8)) + #expect(try await library.readAssetBundles().map(\.bundleRoot) == [checked.bundleRoot]) + } + + /// Two links written differently can lead to one file: the earlier layout kept an asset wherever its + /// path led, whatever it went through to get there. + @Test("a file that two paths led to in a bundle stored before assets were named for their URLs is taken for neither") + func fileTwoPathsLedToInEarlierLayoutIsTakenForNeither() async throws { + let storageRoot = URL.randomTemporaryDirectory + let roundabout = URL(string: "https://cdn.example.com/x/%2E%2E/plugin.js?ver=1")! + let manifest = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["core/paragraph"] + } + """ + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + let library = makeLibrary(httpClient: mockClient, storageRoot: storageRoot) + let stored = try await library.downloadAssetBundle() + // As it was stored then: one file, where both paths lead + try FileManager.default.moveItem( + at: stored.assetDataPath(for: Self.scriptURL), + to: stored.bundleRoot.appending(path: "plugin.js") + ) + try FileManager.default.removeItem(at: stored.assetDataPath(for: roundabout)) + + let onDisk = try #require(try await library.readLatestAssetBundleOnDisk()) + + #expect(await library.missingAssets(of: onDisk) == [Self.scriptURL, roundabout]) + } + + /// Not a bundle the tests' own library wrote and then rearranged: one as the last release left it on + /// disk. Its manifest file holds no more than a manifest and a download date, its assets are at their + /// URLs' paths, and it never held a link that didn't end in `.js` or `.css`. + @Test("a bundle as the last release stored it gives the assets it has, and asks only for the one it never held") + func bundleAsLastReleaseStoredItGivesItsAssets() async throws { + let storageRoot = URL.randomTemporaryDirectory + let script = URL(string: "https://example.com/wp-content/plugins/a-plugin/build/index.js?ver=1.2")! + let style = URL(string: "https://example.com/wp-content/plugins/a-plugin/build/style.css?ver=1.2")! + let concatenated = URL(string: "https://s0.example.com/_static/??/a.js,/b.js")! + let manifestJSON = """ + { + "scripts": "", + "styles": "", + "allowed_block_types": ["a-plugin/block"] + } + """ + let manifest = try LocalEditorAssetManifest( + remoteManifest: RemoteEditorAssetManifest(data: Data(manifestJSON.utf8)) + ) + let bundleRoot = storageRoot.appending(path: manifest.checksum) + let downloadDate = Date(timeIntervalSinceReferenceDate: 780_000_000) + try FileManager.default.createDirectory( + at: bundleRoot.appending(path: "wp-content/plugins/a-plugin/build"), + withIntermediateDirectories: true + ) + try JSONSerialization.data(withJSONObject: [ + "manifest": try JSONSerialization.jsonObject(with: JSONEncoder().encode(manifest)), + "downloadDate": downloadDate.timeIntervalSinceReferenceDate, + ]).write(to: bundleRoot.appending(path: "manifest.json")) + try JSONEncoder() + .encode(manifest.buildEditorRepresentation(for: Self.testConfiguration)) + .write(to: bundleRoot.appending(path: "editor-representation.json")) + try Data("script".utf8).write(to: bundleRoot.appending(path: "wp-content/plugins/a-plugin/build/index.js")) + try Data("style".utf8).write(to: bundleRoot.appending(path: "wp-content/plugins/a-plugin/build/style.css")) + + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { _ in throw URLError(.notConnectedToInternet) } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .always, storageRoot: storageRoot) + + // The site can't be reached: what the bundle has is still there to give an editor + let onDisk = try #require(try await library.readLatestAssetBundleOnDisk()) + + #expect(onDisk.id == manifest.checksum) + #expect(onDisk.downloadDate == downloadDate) + #expect(onDisk.lastCheckedDate == nil) + #expect(try onDisk.assetData(for: script) == Data("script".utf8)) + #expect(try onDisk.assetData(for: style) == Data("style".utf8)) + #expect(await library.missingAssets(of: onDisk) == [concatenated]) + #expect(try await library.readLatestAssetBundleOnDisk() == onDisk) + // It's missing an asset, so it isn't one to settle for + #expect(try await library.readLatestAssetBundle() == nil) + + // The site can be reached: everything is asked for, as the bundle records none of it as downloaded + mockClient.urlResponseHandler = Self.responses(forManifest: manifestJSON, assetContent: "new content") + let checked = try await library.downloadAssetBundle() + + #expect(Set(mockClient.downloadedURLs) == [script, style, concatenated]) + #expect(await library.missingAssets(of: checked).isEmpty) + #expect(await library.assetsNotRefreshed(in: checked).isEmpty) + #expect(try await library.readLatestAssetBundle() == checked) + } + + /// A refresh that changes a bundle's assets publishes a bundle of its own, which a check of the same + /// manifest that was already under way knows nothing about. What that check found or built is older, + /// and mustn't go back in front. + @Test("a check that's overtaken by a refresh gives the refresh's bundle, rather than putting its own back in front") + func checkOvertakenByRefreshGivesRefreshedBundle() async throws { + let storageRoot = URL.randomTemporaryDirectory + let manifest = Self.manifestJSON(scriptVersion: "1") + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(manifest.utf8) + } + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0), storageRoot: storageRoot) + let refreshing = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + let gapped = try await library.downloadAssetBundle() + #expect(!gapped.hasAssetData(for: Self.scriptURL)) + + // The check has asked for the script again, and failed again. Before it settles, a refresh + // downloads the script and publishes a bundle that has it. + let refresh = OnceOnlyAsync { + mockClient.urlResponseHandler = Self.responses(forManifest: manifest) + _ = try? await refreshing.downloadAssetBundle() + } + let checked = try await library.downloadAssetBundle { _ in await refresh.run() } + + #expect(checked.bundleRoot != gapped.bundleRoot) + #expect(checked.hasAssetData(for: Self.scriptURL)) + #expect(try await library.readAssetBundles().first?.bundleRoot == checked.bundleRoot) + } + + @Test("a bundle stored before assets were named for their URLs is as old as it was, for the cache policy") + func bundleStoredAtAssetPathsIsNoNewerForBeingCopied() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .maxAge(60)) + let stored = try #require(try await library.readAssetBundles().first) + try backdate(stored, by: 120) + try FileManager.default.moveItem( + at: stored.assetDataPath(for: Self.scriptURL), + to: stored.bundleRoot.appending(path: "plugin.js") + ) + + #expect(try await library.readLatestAssetBundle() == nil) + + // What there is to use when the site can't be asked has its asset all the same + let onDisk = try #require(try await library.readLatestAssetBundleOnDisk()) + #expect(onDisk.hasAssetData(for: Self.scriptURL)) + #expect(mockClient.downloadCallCount == 1) + } + + /// A bundle is dated by when the site was found to have its manifest: when the manifest was fetched, + /// not when the bundle finished building, which can be a good while later. That's what its age for the + /// cache policy counts from, and what decides which bundle is the site's latest. + @Test("a bundle is dated by when its manifest was fetched, whether it's built or kept") + func bundleIsDatedByManifestFetch() async throws { + let assetRequested = RecordedDate() + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = { url in + if url.path.contains("editor-assets") { + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + assetRequested.recordNow() + return Data("mock content".utf8) + } let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) - let earlier = try await library.downloadAssetBundle() - // The site's manifest changes, and while its bundle builds the earlier one is matched again - let rematch = OnceOnly { try? self.backdate(earlier, by: 0) } + let built = try await library.downloadAssetBundle() + + #expect(try #require(built.lastCheckedDate) <= #require(assetRequested.date)) + + // A check that keeps the bundle reports its progress before it marks it + let progressReported = RecordedDate() + let checked = try await library.downloadAssetBundle { _ in progressReported.recordNow() } + + #expect(checked == built) + #expect(try #require(checked.lastCheckedDate) > #require(built.lastCheckedDate)) + #expect(try #require(checked.lastCheckedDate) <= #require(progressReported.date)) + } + + /// The site's manifest changes while its earlier one's bundle is still building, and a refresh builds + /// the new one's first. The slower build is of what the site had before, so it stays behind. + @Test("a bundle whose manifest was fetched before another's stays behind it, however late it's built") + func bundleOfEarlierManifestStaysBehindLaterOne() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0), storageRoot: storageRoot) + let refreshing = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + + let refresh = OnceOnlyAsync { + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + _ = try? await refreshing.downloadAssetBundle() + } + let built = try await library.downloadAssetBundle { _ in await refresh.run() } + + let bundles = try await library.readAssetBundles() + #expect(bundles.count == 2) + #expect(bundles.first?.id != built.id) + #expect(bundles.last?.bundleRoot == built.bundleRoot) + } + + /// While a refresh downloads, another library checks the manifest and finds the bundle on disk still + /// matches it: a more recent look at the site than the refresh's own. The refresh's bundle replaces + /// that one all the same, so it has to come ahead of it. + @Test("a bundle that replaces its manifest's bundle comes ahead of it, though that one was matched while it built") + func bundleThatReplacesAnotherComesAheadOfIt() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let bundle = try #require(try await library.readAssetBundles().first) + + let rematch = OnceOnly { try? self.backdate(bundle, by: 0) } mockClient.urlResponseHandler = { url in if url.path.contains("editor-assets") { - return Data(Self.manifestJSON(scriptVersion: "2").utf8) + return Data(Self.manifestJSON(scriptVersion: "1").utf8) } rematch.run() return Data("new content".utf8) } - let built = try await library.downloadAssetBundle() + let refreshed = try await library.downloadAssetBundle() + + #expect(refreshed.bundleRoot != bundle.bundleRoot) + #expect(try await library.readAssetBundles().first?.bundleRoot == refreshed.bundleRoot) + #expect(await library.existingBundle(forManifestChecksum: bundle.id)?.bundleRoot == refreshed.bundleRoot) + } + + /// The clock was ahead when the bundle on disk was last matched, and has been set back since. What's + /// published now is dated now, and still has to come ahead of it — and stay there. + @Test("a bundle dated in time to come stays behind the bundles published since") + func bundleDatedInTimeToComeStaysBehindBundlesPublishedSince() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .ignore, storageRoot: storageRoot) + let dated = try await library.downloadAssetBundle() + try backdate(dated, by: -86_400) + + // A refresh replaces it, for the same manifest + mockClient.urlResponseHandler = Self.responses( + forManifest: Self.manifestJSON(scriptVersion: "1"), + assetContent: "new content" + ) + let refreshed = try await library.downloadAssetBundle() + #expect(try await library.readAssetBundles().first?.bundleRoot == refreshed.bundleRoot) + // It's been dated afresh, so it won't come back in front when the clock reaches the date it had + #expect(try #require(try await library.readAssetBundles().last?.lastCheckedDate) <= Date()) + + // Another that changes nothing keeps the refreshed bundle, and leaves it in front + let again = try await library.downloadAssetBundle() + #expect(again.bundleRoot == refreshed.bundleRoot) + #expect(try await library.readAssetBundles().first?.bundleRoot == refreshed.bundleRoot) + #expect(try FileManager.default.contentsOfDirectory(atPath: storageRoot.path).count == 2) + + // And so does a bundle for another manifest + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + let changed = try await library.downloadAssetBundle() + #expect(try await library.readAssetBundles().first?.bundleRoot == changed.bundleRoot) + } + + /// All a date still to come tells is that the bundle was matched after the ones with dates that can + /// be believed. It keeps its place ahead of them. + @Test("a bundle dated in time to come stays ahead of the bundles that were there before it") + func bundleDatedInTimeToComeStaysAheadOfEarlierBundles() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let earlier = try await library.downloadAssetBundle() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + let latest = try await library.downloadAssetBundle() + try backdate(latest, by: -86_400) + + let onDisk = try #require(try await library.readLatestAssetBundleOnDisk()) + + #expect(onDisk.id == latest.id) + let bundles = try await library.readAssetBundles() + #expect(bundles.map(\.id) == [latest.id, earlier.id]) + #expect(try #require(bundles.first?.lastCheckedDate) <= Date()) + } + + /// Their dates still say which came later, which is all that's kept of them. + @Test("bundles dated in time to come keep their order when they're dated afresh") + func bundlesDatedInTimeToComeKeepTheirOrder() async throws { + let mockClient = EditorAssetLibraryMockHTTPClient() + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(0)) + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let earlier = try await library.downloadAssetBundle() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "2")) + let later = try await library.downloadAssetBundle() + try backdate(earlier, by: -86_400) + try backdate(later, by: -172_800) + + #expect(try await library.readLatestAssetBundleOnDisk()?.id == later.id) + + let bundles = try await library.readAssetBundles() + #expect(bundles.map(\.id) == [later.id, earlier.id]) + #expect(bundles.allSatisfy { ($0.lastCheckedDate ?? .distantFuture) <= Date() }) + } + + /// A date that's still to come says nothing about how long ago the manifest was checked. + @Test("a bundle dated in time to come isn't trusted for its age, only by a policy that never asks") + func bundleDatedInTimeToComeIsNotTrustedForItsAge() async throws { + let storageRoot = URL.randomTemporaryDirectory + let mockClient = EditorAssetLibraryMockHTTPClient() + mockClient.urlResponseHandler = Self.responses(forManifest: Self.manifestJSON(scriptVersion: "1")) + let library = makeLibrary(httpClient: mockClient, cachePolicy: .maxAge(3600), storageRoot: storageRoot) + let bundle = try await library.downloadAssetBundle() + try backdate(bundle, by: -86_400) + + #expect(try await library.readLatestAssetBundle() == nil) + #expect(try await makeLibrary(cachePolicy: .always, storageRoot: storageRoot).readLatestAssetBundle() == bundle) + } + + /// Another library may have found the site's manifest to match the bundle more recently than this one + /// did, and recorded it first: here, between this check fetching the manifest and marking the bundle. + @Test("a check doesn't make a bundle look less recently matched than it's recorded to be") + func checkDoesNotMoveBundleBackInTime() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(0)) + let bundle = try #require(try await library.readAssetBundles().first) + + let otherMatch = RecordedDate() + + let checked = try await library.downloadAssetBundle { _ in + otherMatch.recordNow() + try? EditorAssetBundle( + manifest: bundle.manifest, + downloadDate: bundle.downloadDate, + lastCheckedDate: otherMatch.date, + bundleRoot: bundle.bundleRoot + ).writeManifest() + } + + #expect(checked.lastCheckedDate == otherMatch.date) + #expect(try await library.readAssetBundles().first?.lastCheckedDate == otherMatch.date) + } + + /// A date that's still to come is one the clock has been set back from. Going by it would leave the + /// bundle unchecked until the clock caught up. + @Test("a check dates a bundle afresh when the date it's recorded with is still to come") + func checkDatesBundleAfreshWhenRecordedDateIsStillToCome() async throws { + let (library, _) = try await makeLibraryWithBundle(cachePolicy: .maxAge(0)) + let bundle = try #require(try await library.readAssetBundles().first) + try backdate(bundle, by: -3600) + + let checked = try await library.downloadAssetBundle() + + #expect(try #require(checked.lastCheckedDate) <= Date()) + } + + /// What a refresh learned is the caller's to be told, whether or not it could be written down. + @Test("a bundle kept after a refresh says which assets failed to download even when that can't be recorded") + func keptBundleSaysWhatFailedWhenItCannotBeRecorded() async throws { + let (library, mockClient) = try await makeLibraryWithBundle(cachePolicy: .ignore) + let bundle = try #require(try await library.readAssetBundles().first) + mockClient.urlResponseHandler = { url in + guard url.path.contains("editor-assets") else { throw URLError(.timedOut) } + return Data(Self.manifestJSON(scriptVersion: "1").utf8) + } + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: bundle.bundleRoot.path) + defer { + try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: bundle.bundleRoot.path) + } - #expect(built.lastCheckedDate != nil) - #expect(try await library.readAssetBundles().first?.id == built.id) + let refreshed = try await library.downloadAssetBundle() + + #expect(refreshed.bundleRoot == bundle.bundleRoot) + #expect(await library.assetsNotRefreshed(in: refreshed) == [Self.scriptURL]) } /// An editor may be reading the bundle that's missing the asset, so the asset doesn't go into it. @@ -922,6 +1643,11 @@ struct EditorAssetLibraryTests { #expect(try changed.assetData(for: style) == Data("mock content".utf8)) // It's the earlier copy, so it's the earlier copy's headers that go with it #expect(changed.headers(for: style)?.etag == "\"first\"") + // The copy stands in for the asset without hiding that the asset failed to download + #expect(await library.assetsNotRefreshed(in: changed) == [style]) + #expect(await library.missingAssets(of: changed).isEmpty) + // It has everything an editor loads, so it's used until the manifest is next checked + #expect(try await library.readLatestAssetBundle() == changed) } @Test("a changed manifest's bundle downloads an unchanged asset that the latest bundle is missing") @@ -1958,6 +2684,41 @@ struct EditorAssetLibraryTests { #expect(session.requestCount == 1) } + /// The build is the library's that started it, and so is the date it gives the bundle. A library that + /// joins it fetched the manifest later, and so has found the site to match more recently. + @Test("a library that joins a build dates the bundle by its own, later fetch of the manifest") + func joinerDatesBundleByItsOwnFetch() async throws { + let manifestJSON = uniqueManifestJSON( + identifier: "joiner-\(UUID().uuidString)", + script: "https://example.com/script.js" + ) + let manifest = try LocalEditorAssetManifest( + remoteManifest: RemoteEditorAssetManifest(data: Data(manifestJSON.utf8)) + ) + + let session = ParkedURLSession() + defer { session.release() } + let storageRoot = URL.randomTemporaryDirectory + let libraries = [ + makeLibrary(httpClient: EditorHTTPClient(urlSession: session, authHeader: "Bearer test-token"), storageRoot: storageRoot), + makeLibrary(httpClient: EditorHTTPClient(urlSession: session, authHeader: "Bearer test-token"), storageRoot: storageRoot), + ] + let destination = await libraries[0].bundleRoot(for: manifest.checksum).standardizedFileURL + let earlierFetch = Date(timeIntervalSinceNow: -120) + let laterFetch = Date(timeIntervalSinceNow: -60) + + let started = Task { try await libraries[0].buildBundle(for: manifest, fetchedAt: earlierFetch) } + try await waitUntil { EditorAssetLibrary.inFlightBuilds.waiterCount(for: destination) == 1 } + let joined = Task { try await libraries[1].buildBundle(for: manifest, fetchedAt: laterFetch) } + try await waitUntil { EditorAssetLibrary.inFlightBuilds.waiterCount(for: destination) == 2 } + + session.release() // fails the parked download, which a build tolerates + + #expect(try await started.value.lastCheckedDate == earlierFetch) + #expect(try await joined.value.lastCheckedDate == laterFetch) + #expect(try await libraries[0].readAssetBundles().first?.lastCheckedDate == laterFetch) + } + @Test("under .ignore, a build doesn't join one in flight for the same manifest") func buildUnderIgnoreDoesNotJoinBuildInFlight() async throws { let manifest = try LocalEditorAssetManifest( @@ -2004,6 +2765,35 @@ private final class OnceOnly: @unchecked Sendable { } } +/// Runs an asynchronous action the first time it's asked to, and never again. +private actor OnceOnlyAsync { + private var action: (@Sendable () async -> Void)? + + init(_ action: @escaping @Sendable () async -> Void) { + self.action = action + } + + func run() async { + let action = self.action + self.action = nil + await action?() + } +} + +/// The moment a test asks it to record, for comparing with a date the library records. +private final class RecordedDate: @unchecked Sendable { + private let lock = NSLock() + private var _date: Date? + + var date: Date? { + lock.withLock { _date } + } + + func recordNow() { + lock.withLock { _date = Date() } + } +} + // MARK: - Progress Tracker for Tests final class ProgressTracker: @unchecked Sendable { From 611768285a9ea4a5b10c805385f1c8b2fbfff988 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:16:53 -0600 Subject: [PATCH 7/9] test(ios): give test waits a minute `waitUntil` and `ParkedURLSession.waitUntilStarted` gave up after ten seconds. On the CI machine a run's first results take half a minute to arrive, so a wait that was about to succeed read as a failure. Both now wait up to `patientTimeout`, one minute. They return as soon as what they wait for happens, so only a wait that is going to fail takes longer. `overlappingPrepareCallsDontTrap` is the wait that ran out. It passes in 0.045s on a Mac. In the iOS Simulator job it took 9.623s before this branch's first commit, and with more tests in the same process it then failed twice in a row at ten seconds. --- .../GutenbergKitTests/Helpers/ParkedURLSession.swift | 2 +- ios/Tests/GutenbergKitTests/TestHelpers.swift | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ios/Tests/GutenbergKitTests/Helpers/ParkedURLSession.swift b/ios/Tests/GutenbergKitTests/Helpers/ParkedURLSession.swift index be31af8be..9a583309f 100644 --- a/ios/Tests/GutenbergKitTests/Helpers/ParkedURLSession.swift +++ b/ios/Tests/GutenbergKitTests/Helpers/ParkedURLSession.swift @@ -51,7 +51,7 @@ final class ParkedURLSession: URLSessionProtocol, @unchecked Sendable { throw URLError(.networkConnectionLost) } - func waitUntilStarted(timeout: Duration = .seconds(10)) async throws { + func waitUntilStarted(timeout: Duration = patientTimeout) async throws { let clock = ContinuousClock() let deadline = clock.now + timeout while clock.now < deadline { diff --git a/ios/Tests/GutenbergKitTests/TestHelpers.swift b/ios/Tests/GutenbergKitTests/TestHelpers.swift index e60e9ec92..d7009db05 100644 --- a/ios/Tests/GutenbergKitTests/TestHelpers.swift +++ b/ios/Tests/GutenbergKitTests/TestHelpers.swift @@ -3,10 +3,17 @@ import Testing @testable import GutenbergKit +/// How long a test waits for something that is supposed to happen before giving up. +/// +/// Generous, because a wait that succeeds returns as soon as it can and only one that is going to +/// fail runs this long. A run's first results take half a minute to arrive on a busy CI machine, +/// which a shorter wait reads as a failure. +let patientTimeout: Duration = .seconds(60) + /// Polls `condition` until it holds, failing the test at the caller's line if it hasn't within /// `timeout`. func waitUntil( - timeout: Duration = .seconds(10), + timeout: Duration = patientTimeout, sourceLocation: SourceLocation = #_sourceLocation, _ condition: () -> Bool ) async throws { From 9ca32adc5815e80a8da672c279d752bb2769c478 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:33:04 -0600 Subject: [PATCH 8/9] test(ios): run the asset library and service suites one test at a time `EditorAssetLibraryTests` and `EditorServiceTests` do their file work on threads of the pool the whole run shares, and under the lock every `EditorAssetLibrary` shares. On the CI machine `swift test` runs both test targets in one process, and with these suites' tests all started together, the suites that should finish at once don't until they have: "HTTPServer Start" took 0.90s on the base branch with 1010 tests in the run, 1.28s with 1094, and 1.52s with 1137. `HTTPServerTimeoutTests` runs in that process against a server whose 500ms read timeout is a task racing the read. Kept from a thread for that long the read loses, the server closes the connection, and the test fails with "Socket is not connected": in two of the three runs with 1137 tests. Both suites are now `.serialized`, so each keeps one test's worth of threads at a time. Other suites still run alongside them. --- .../GutenbergKitTests/Services/EditorServiceTests.swift | 4 +++- .../GutenbergKitTests/Stores/EditorAssetLibraryTests.swift | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift index d1d181d1f..33bb7d15e 100644 --- a/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift +++ b/ios/Tests/GutenbergKitTests/Services/EditorServiceTests.swift @@ -3,7 +3,9 @@ import Testing @testable import GutenbergKit -@Suite +/// One at a time, for the reason `EditorAssetLibraryTests` are: nearly every one of these builds or +/// reads an asset bundle. +@Suite(.serialized) struct EditorServiceTests: MakesTestFixtures { // MARK: - Test Fixtures diff --git a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift index 5a4162682..e87fe4c71 100644 --- a/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift +++ b/ios/Tests/GutenbergKitTests/Stores/EditorAssetLibraryTests.swift @@ -3,6 +3,11 @@ import Testing @testable import GutenbergKit +/// One at a time. A library reads and writes its files on a thread of the pool that every test in the +/// run shares, and holds a lock that every library shares while it changes storage. Run together, +/// these tests keep those threads for long enough that a test timing something else — a server's read +/// timeout, say — sees a delay that isn't the server's. +@Suite(.serialized) struct EditorAssetLibraryTests { // MARK: - Test Fixtures From 5bc8c62c6114923bcea7f0d715b82fc0cd860f89 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:10:11 -0600 Subject: [PATCH 9/9] refactor(ios): stop dating afresh a bundle that's dated ahead of the clock A bundle dated more than a minute ahead of the clock was dated afresh the first time it was read, so that a clock set back since the bundle was dated couldn't leave it ahead of the bundles published after it. That takes a clock that's wrong by more than a minute when a bundle is dated and is then set back, and it cost a second way to rewrite a bundle's manifest, taken on a read. Bundles are now ordered, and aged for the cache policy, by the dates they record and nothing else. If the clock is set back, a bundle built for a changed manifest comes behind the earlier one until the clock passes the earlier one's date, and `.maxAge` takes the earlier one for newer than it is. A bundle that replaces its own manifest's bundle is still dated just after it, so a refresh that finds the same manifest isn't affected. `purge()` clears it. `docs/code/preloading.md` says so. Removes `redateBundlesDatedInTimeToCome(among:)` and the five tests that pinned it. --- docs/code/preloading.md | 2 + .../Sources/Stores/EditorAssetLibrary.swift | 62 +----------- .../Stores/EditorAssetLibraryTests.swift | 99 ------------------- 3 files changed, 4 insertions(+), 159 deletions(-) diff --git a/docs/code/preloading.md b/docs/code/preloading.md index c6ee96a80..4722c5189 100644 --- a/docs/code/preloading.md +++ b/docs/code/preloading.md @@ -269,6 +269,8 @@ Under `.ignore`, nothing on disk is taken to be valid, so every asset is downloa The old bundle stays on disk for as long as the app is running, because an open editor — or dependencies the host prepared earlier and still holds — may be reading it. `cleanup()` removes it after the next launch. +Which bundle is a site's latest goes by the date each records, taken from the device's clock when the site's manifest was last found to match it. If that clock was ahead and has been set back since, a bundle built for a changed manifest in the meantime comes behind the earlier one until the clock passes the earlier one's date. `purge()` clears it. + A bundle holds every link that a `