From 23da94c1aee5c98d927a65726cdf39a46db43b35 Mon Sep 17 00:00:00 2001 From: Alex Sherstnev Date: Sun, 26 Jul 2026 01:36:30 +0200 Subject: [PATCH 1/2] cache image details responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Image details were re-fetched every time a photo was opened. ImageDetailsLoader now keeps loaded details in memory, so reopening a photo (or coming back to one through a description link) is instant. ImageLoader and ImageDetailsLoader are actors: their caches are shared across tasks, and the memory-warning handler in AppGraph has to capture them from a @Sendable closure. AppGraph no longer stores the loaders — they are retained by the remotes and the observer that use them. Closes #102 Co-Authored-By: Claude Opus 5 (1M context) --- Rewind/App/AppGraph.swift | 13 +- Rewind/Network/ImageDetailsLoader.swift | 34 ++++++ Rewind/Network/ImageLoader.swift | 4 +- Rewind/Network/RewindRemotes.swift | 4 +- RewindTests/ImageDetailsLoaderTests.swift | 138 ++++++++++++++++++++++ 5 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 Rewind/Network/ImageDetailsLoader.swift create mode 100644 RewindTests/ImageDetailsLoaderTests.swift diff --git a/Rewind/App/AppGraph.swift b/Rewind/App/AppGraph.swift index bc8f58b..f99fbec 100644 --- a/Rewind/App/AppGraph.swift +++ b/Rewind/App/AppGraph.swift @@ -18,7 +18,6 @@ final class AppGraph { let map: Lazy let urlOpener: UrlOpener - let imageLoader: ImageLoader var orientationLock: Property? @@ -32,7 +31,7 @@ final class AppGraph { urlRequestPerformer: URLSession.shared.data, ) let imageLoader = ImageLoader(requestPerformer: requestPerformer) - self.imageLoader = imageLoader + let imageDetailsLoader = ImageDetailsLoader(requestPerformer: requestPerformer) let annotationStore = AnnotationStore() let storage: KeyValueStorage = UserDefaults.standard let favoritesStorage = FavoritesStorage( @@ -46,6 +45,7 @@ final class AppGraph { let remotes = RewindRemotes( requestPerformer: requestPerformer, imageLoader: imageLoader, + imageDetailsLoader: imageDetailsLoader, ) let settings = makeSettings(storage: storage) @@ -164,14 +164,15 @@ final class AppGraph { }.dispose(in: disposePool) filters.current = mapModel.$state.filters.skipRepeats() - // React to memory warnings by clearing image cache + // React to memory warnings by clearing cached images and image details memoryWarningObserver = NotificationCenter.default.addObserver( forName: UIApplication.didReceiveMemoryWarningNotification, object: nil, queue: .main, - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.imageLoader.clearCache() + ) { [imageLoader, imageDetailsLoader] _ in + Task { + await imageLoader.clearCache() + await imageDetailsLoader.clearCache() } } storeReview.appLaunched() diff --git a/Rewind/Network/ImageDetailsLoader.swift b/Rewind/Network/ImageDetailsLoader.swift new file mode 100644 index 0000000..ceb2d3a --- /dev/null +++ b/Rewind/Network/ImageDetailsLoader.swift @@ -0,0 +1,34 @@ +// +// ImageDetailsLoader.swift +// Rewind +// +// Created by Alexey Sherstnev on 26.07.2026. +// + +import Foundation + +actor ImageDetailsLoader { + private var cache: [Int: Model.ImageDetails] + private let requestPerformer: RequestPerformer + + init(requestPerformer: RequestPerformer) { + cache = [:] + self.requestPerformer = requestPerformer + } + + /// Clears all cached details from memory. + /// Called automatically on memory warnings. + func clearCache() { + cache.removeAll() + } + + func load(cid: Int) async throws -> Model.ImageDetails { + if let cached = cache[cid] { + return cached + } + let networkDetails = try await requestPerformer.perform(request: .imageDetails(cid: cid)) + let details = Model.ImageDetails(networkDetails) + cache[cid] = details + return details + } +} diff --git a/Rewind/Network/ImageLoader.swift b/Rewind/Network/ImageLoader.swift index 78ce089..6b36b33 100644 --- a/Rewind/Network/ImageLoader.swift +++ b/Rewind/Network/ImageLoader.swift @@ -9,7 +9,7 @@ import Foundation import UIKit import VGSL -final class ImageLoader { +actor ImageLoader { private let cache: NSCache private let requestPerformer: RequestPerformer @@ -24,7 +24,7 @@ final class ImageLoader { cache.removeAllObjects() } - func makeImage( + nonisolated func makeImage( path: String, ) -> LoadableUIImage { LoadableUIImage { [weak self] params in diff --git a/Rewind/Network/RewindRemotes.swift b/Rewind/Network/RewindRemotes.swift index 795d58d..3ceece7 100644 --- a/Rewind/Network/RewindRemotes.swift +++ b/Rewind/Network/RewindRemotes.swift @@ -41,6 +41,7 @@ extension RewindRemotes { init( requestPerformer: RequestPerformer, imageLoader: ImageLoader, + imageDetailsLoader: ImageDetailsLoader, ) { annotations = Remote { params in let (nis, ncs) = try await requestPerformer.perform( @@ -63,8 +64,7 @@ extension RewindRemotes { return (images, clusters) }.exponentialBackoff() imageDetails = Remote { cid in - let details = try await requestPerformer.perform(request: .imageDetails(cid: cid)) - return Model.ImageDetails(details) + try await imageDetailsLoader.load(cid: cid) }.exponentialBackoff() streetViewAvailability = Remote { coordinate in try await requestPerformer.perform(request: .streetViewAvailability(coordinate: coordinate)) diff --git a/RewindTests/ImageDetailsLoaderTests.swift b/RewindTests/ImageDetailsLoaderTests.swift new file mode 100644 index 0000000..c57d492 --- /dev/null +++ b/RewindTests/ImageDetailsLoaderTests.swift @@ -0,0 +1,138 @@ +// +// ImageDetailsLoaderTests.swift +// RewindTests +// +// Tests the in-memory cache in ImageDetailsLoader (the details counterpart of ImageLoader's +// image cache): details fetched once are reused for every later load of the same photo, +// distinct photos don't share an entry, clearCache() (fired on memory warnings) forces a +// refetch, and a failed load leaves nothing cached. The stub network echoes the requested cid +// back and stamps every response with its fetch number, so a cached value is distinguishable +// from a refetched one. +// + +import Foundation +@testable import Rewind +import Testing + +struct ImageDetailsLoaderTests { + /// Loading the same photo twice performs a single request; the second load is served from cache. + @Test func repeatedLoadIsServedFromCache() async throws { + let network = NetworkStub() + let loader = makeLoader(network: network) + + let first = try await loader.load(cid: 42) + let second = try await loader.load(cid: 42) + + #expect(await network.requestedCids == [42]) + #expect(first.cid == 42) + #expect(first.title == "fetch #1") + #expect(second.title == "fetch #1") // not refetched, hence not "fetch #2" + } + + /// Every photo gets its own cache entry: two photos are fetched once each and never mixed up. + @Test func detailsAreCachedPerImage() async throws { + let network = NetworkStub() + let loader = makeLoader(network: network) + + let first = try await loader.load(cid: 1) + let second = try await loader.load(cid: 2) + let firstAgain = try await loader.load(cid: 1) + let secondAgain = try await loader.load(cid: 2) + + #expect(await network.requestedCids == [1, 2]) + #expect(firstAgain.cid == 1) + #expect(firstAgain.title == first.title) + #expect(secondAgain.cid == 2) + #expect(secondAgain.title == second.title) + #expect(first.title != second.title) + } + + /// Clearing the cache (what a memory warning does) makes the next load hit the network again. + @Test func clearCacheForcesRefetch() async throws { + let network = NetworkStub() + let loader = makeLoader(network: network) + + _ = try await loader.load(cid: 7) + await loader.clearCache() + let refetched = try await loader.load(cid: 7) + + #expect(await network.requestedCids == [7, 7]) + #expect(refetched.title == "fetch #2") + } + + /// A failed load caches nothing, so a later load retries instead of replaying the failure. + @Test func failedLoadIsNotCached() async throws { + let network = NetworkStub() + await network.failNextRequests(1) + let loader = makeLoader(network: network) + + await #expect(throws: (any Error).self) { + try await loader.load(cid: 3) + } + let details = try await loader.load(cid: 3) + let cached = try await loader.load(cid: 3) + + #expect(await network.requestedCids == [3, 3]) // the failure did not poison the cache + #expect(details.cid == 3) + #expect(cached.title == details.title) + } +} + +private func makeLoader(network: NetworkStub) -> ImageDetailsLoader { + ImageDetailsLoader( + requestPerformer: RequestPerformer( + urlRequestPerformer: { request in try await network.respond(to: request) } + ) + ) +} + +/// Stands in for the PastVu API: records the cid of every request and answers with details for +/// that cid, titled with the number of the fetch that produced them. +private actor NetworkStub { + private(set) var requestedCids: [Int] = [] + private var fetchCount = 0 + private var failuresLeft = 0 + + func failNextRequests(_ count: Int) { + failuresLeft = count + } + + func respond(to request: URLRequest) throws -> (Data, URLResponse) { + let cid = try requestedCid(from: request) + requestedCids.append(cid) + if failuresLeft > 0 { + failuresLeft -= 1 + throw HandlingError("network is down") + } + fetchCount += 1 + return try (detailsJSON(cid: cid, title: "fetch #\(fetchCount)"), URLResponse()) + } +} + +private func requestedCid(from request: URLRequest) throws -> Int { + struct Params: Decodable { + var cid: Int + } + + guard let url = request.url, + let components = URLComponents(url: url, resolvingAgainstBaseURL: false), + let params = components.queryItems?.first(where: { $0.name == "params" })?.value + else { + throw HandlingError("unexpected request: \(request)") + } + return try JSONDecoder().decode(Params.self, from: Data(params.utf8)).cid +} + +private func detailsJSON(cid: Int, title: String) throws -> Data { + let photo: [String: Any] = [ + "cid": cid, + "file": "a/b/c.jpg", + "title": title, + "geo": [44.813047, 20.460579], + "year": 1958, + "year2": 1965, + "ldate": "2022-09-30T16:52:26.687Z", + "user": ["disp": "Николай"], + ] + return try JSONSerialization.data(withJSONObject: ["result": ["photo": photo]]) +} From 7489c1dba9ed183c7167fdd6f99f727f5859be49 Mon Sep 17 00:00:00 2001 From: Alex Sherstnev Date: Sun, 26 Jul 2026 01:46:41 +0200 Subject: [PATCH 2/2] drop the clearCache comment describing external usage Co-Authored-By: Claude Opus 5 (1M context) --- Rewind/Network/ImageDetailsLoader.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Rewind/Network/ImageDetailsLoader.swift b/Rewind/Network/ImageDetailsLoader.swift index ceb2d3a..16cc347 100644 --- a/Rewind/Network/ImageDetailsLoader.swift +++ b/Rewind/Network/ImageDetailsLoader.swift @@ -16,8 +16,6 @@ actor ImageDetailsLoader { self.requestPerformer = requestPerformer } - /// Clears all cached details from memory. - /// Called automatically on memory warnings. func clearCache() { cache.removeAll() }