Keep calling the Nuke 13 willCache

.scripts/generate-llms.py

@@ -381,7 +381,7 @@ def llms_index():
 
 What you remember about Nuke is likely out of date. In particular:
 
-- A pipeline delegate method with the wrong signature compiles, and is never called. The Nuke 13 `willCache(data:image:for:pipeline:completion:)` is one. Copy the signatures from `llms-api.txt`.
+- A pipeline delegate method with the wrong signature compiles, and is never called. The Nuke 12 `imageTaskDidCancel(_:pipeline:)` is one. Copy the signatures from `llms-api.txt`.
 - `LazyImage` plays GIFs with no setup, and `AnimatedImageView` does in UIKit. Don't decode the frames yourself. A custom `LazyImage` content closure needs `AnimatedImage`.
 - Load into an image view with `ImageLoader.load(_:into:)` from NukeUI, not `loadImage(with:into:)` from NukeExtensions, which is soft-deprecated.
 - To find out why an image doesn't load, launch with `NUKE_DIAGNOSTICS_LOG=1` and read the log.

Documentation/Migrations/Nuke 14 Migration Guide.md

@@ -277,9 +277,11 @@ func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pi
 
 The method runs on `@ImagePipelineActor`, and the pipeline awaits it before storing the data.
 
-> Warning: **The Nuke 13 method still compiles, and the pipeline never calls it.** Every delegate method has a default implementation, so nothing requires yours, and the compiler has no way to tell you that it became dead code. A delegate that used `willCache` to keep private images off the disk stops doing so, silently. Search your code for `completion: @escaping (Data?) -> Void`.
+The Nuke 13 method is soft-deprecated, and it keeps working: the default implementation of the new method calls it, so a delegate that uses it to keep private images off the disk still does. If a delegate implements both, the pipeline calls the new one. Nuke 13 didn't wait for the completion closure, and now the image waits for it when the pipeline stores the original data, so call it right away.
 
-The same goes for every other delegate method whose signature doesn't match, such as the Nuke 12 `imageTaskDidCancel(_:pipeline:)`. Declare the conformance in an extension, not on the class:
+> Warning: **Every other delegate method whose signature doesn't match still compiles, and the pipeline never calls it,** such as the Nuke 12 `imageTaskDidCancel(_:pipeline:)`. Every delegate method has a default implementation, so nothing requires yours, and the compiler has no way to tell you that it became dead code.
+
+Declare the conformance in an extension, not on the class:
 
 ```swift
 final class MyDelegate: Sendable {}
@@ -292,7 +294,7 @@ extension MyDelegate: ImagePipeline.Delegate {
 }
 ```
 
-The compiler only compares a method to the requirements of the protocol the extension declares, and only if the number of parameters is the same. So the warning catches the method above, but not the old `willCache`, which has one more parameter.
+The compiler only compares a method to the requirements of the protocol the extension declares, and only if the number of parameters is the same. So the warning catches the method above, but not one with a different number of parameters.
 
 ## Renamed `TaskQueue.maxConcurrentOperationCount`
 

Documentation/Nuke.docc/Extensions/ImagePipelineDelegate-Extension.md

@@ -19,6 +19,7 @@
 - ``dataCache(for:pipeline:)``
 - ``cacheKey(for:pipeline:)``
 - ``willCache(data:image:for:pipeline:)``
+- ``willCache(data:image:for:pipeline:completion:)``
 
 ### Decompression
 

Sources/Nuke/Pipeline/ImagePipeline+Delegate.swift

@@ -3,6 +3,7 @@
 // Copyright (c) 2015-2026 Alexander Grebenyuk (github.com/kean).
 
 import Foundation
+import os
 
 extension ImagePipeline {
     /// A delegate that allows you to customize the pipeline dynamically on a per-request basis.
@@ -82,7 +83,9 @@ extension ImagePipeline {
         /// This method is called only if the request parameters and data caching policy
         /// of the pipeline already allow caching.
         ///
-        /// The default implementation returns `data` unchanged.
+        /// The default implementation calls
+        /// ``willCache(data:image:for:pipeline:completion:)``, which leaves `data`
+        /// unchanged unless a delegate written for Nuke 13 implements it.
         ///
         /// - parameters:
         ///   - data: Either the original data or the encoded image in case of storing
@@ -95,6 +98,30 @@ extension ImagePipeline {
         @ImagePipelineActor
         func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline) async -> Data?
 
+        /// The Nuke 13 form of ``willCache(data:image:for:pipeline:)``, whose
+        /// default implementation calls this method. The method above takes
+        /// precedence if a delegate implements both.
+        ///
+        /// It stays a requirement because a method left over from Nuke 13 compiles
+        /// without a diagnostic: if the pipeline stopped calling it, a delegate
+        /// that keeps private images off the disk would silently stop doing so.
+        ///
+        /// The default implementation calls `completion` with `data`.
+        ///
+        /// - warning: Soft-deprecated in Nuke 14. Implement
+        /// ``willCache(data:image:for:pipeline:)`` instead.
+        ///
+        /// - parameters:
+        ///   - data: Either the original data or the encoded image in case of storing
+        ///   a processed or re-encoded image.
+        ///   - image: Non-nil in case storing an encoded image.
+        ///   - request: The request for which image is being stored.
+        ///   - pipeline: The pipeline that is about to store the data.
+        ///   - completion: Call it from any thread with the data to store, or `nil`
+        ///   to prevent caching. The pipeline waits for it before delivering
+        ///   the original data.
+        nonisolated func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline, completion: @escaping (Data?) -> Void)
+
         // MARK: Decompression
 
         /// Returns `true` if the pipeline should decompress the given response.
@@ -157,7 +184,16 @@ extension ImagePipeline.Delegate {
 
     @ImagePipelineActor
     public func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline) async -> Data? {
-        data
+        await withUnsafeContinuation { continuation in
+            let completion = WillCacheCompletion(continuation)
+            willCache(data: data, image: image, for: request, pipeline: pipeline) { @Sendable in
+                completion.resume(returning: $0)
+            }
+        }
+    }
+
+    public func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline, completion: @escaping (Data?) -> Void) {
+        completion(data)
     }
 
     public func shouldDecompress(response: ImageResponse, for request: ImageRequest, pipeline: ImagePipeline) -> Bool {
@@ -214,3 +250,23 @@ extension ImagePipeline {
 }
 
 final class ImagePipelineDefaultDelegate: ImagePipeline.Delegate {}
+
+/// Nuke 13 didn't wait for the completion closure of `willCache`, so a delegate
+/// got away with not calling it, which kept the data off the disk, or with
+/// calling it twice. The pipeline waits for it now, and neither of those can
+/// hang the load or crash it.
+private final class WillCacheCompletion: Sendable {
+    private let continuation: OSAllocatedUnfairLock<UnsafeContinuation<Data?, Never>?>
+
+    init(_ continuation: UnsafeContinuation<Data?, Never>) {
+        self.continuation = OSAllocatedUnfairLock(initialState: continuation)
+    }
+
+    func resume(returning data: Data?) {
+        continuation.withLock { $0.take() }?.resume(returning: data)
+    }
+
+    deinit {
+        resume(returning: nil)
+    }
+}

Tests/NukeTests/ImagePipelineTests/ImagePipelineDelegateCachingTests.swift

@@ -4,6 +4,7 @@
 
 import Testing
 import Foundation
+import os
 @testable import Nuke
 
 /// How the pipeline consults the caching hooks of ``ImagePipeline/Delegate-swift.protocol``
@@ -103,6 +104,151 @@ struct ImagePipelineDelegateCachingTests {
         #expect(dataCache.store == [Test.url.absoluteString + "p1": Data("sealedencoded".utf8)])
     }
 
+    // MARK: willCache With a Completion Closure
+
+    private func makePipeline(delegate: any ImagePipeline.Delegate, _ policy: ImagePipeline.DataCachePolicy = .storeOriginalData, _ configure: (inout ImagePipeline.Configuration) -> Void = { _ in }) -> ImagePipeline {
+        ImagePipeline(delegate: delegate) {
+            $0.dataLoader = dataLoader
+            $0.imageCache = nil
+            $0.dataCache = dataCache
+            $0.dataCachePolicy = policy
+            configure(&$0)
+        }
+    }
+
+    /// The method compiles after an upgrade with no diagnostic, so if the
+    /// pipeline stopped calling it, private images would end up on the disk.
+    @Test func nuke13WillCacheKeepsDataOffTheDisk() async throws {
+        // GIVEN a delegate written for Nuke 13
+        let delegate = Nuke13CachingDelegate { _, completion in completion(nil) }
+        let pipeline = makePipeline(delegate: delegate)
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(delegate.callCount == 1)
+        #expect(dataCache.store.isEmpty)
+    }
+
+    @Test func nuke13WillCacheDeclaredInAnExtensionKeepsDataOffTheDisk() async throws {
+        // GIVEN
+        let delegate = Nuke13ExtensionDelegate()
+        let pipeline = makePipeline(delegate: delegate)
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(delegate.isCalled.withLock { $0 })
+        #expect(dataCache.store.isEmpty)
+    }
+
+    @Test func nuke13WillCacheCanReplaceTheData() async throws {
+        // GIVEN
+        let delegate = Nuke13CachingDelegate { data, completion in completion(Data("sealed".utf8) + data) }
+        let pipeline = makePipeline(delegate: delegate)
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(dataCache.store == [Test.url.absoluteString: Data("sealed".utf8) + Test.data])
+    }
+
+    @Test func nuke13WillCacheCanPreventStoringTheEncodedImage() async throws {
+        // GIVEN
+        let delegate = Nuke13CachingDelegate { _, completion in completion(nil) }
+        let pipeline = makePipeline(delegate: delegate, .automatic) {
+            $0.makeImageEncoder = { _ in MockImageEncoder(result: Data("encoded".utf8)) }
+        }
+        let request = ImageRequest(url: Test.url, processors: [MockImageProcessor(id: "p1")])
+
+        // WHEN
+        _ = try await pipeline.image(for: request)
+        await pipeline.configuration.imageEncodingQueue.waitUntilAllOperationsAreFinished()
+
+        // THEN
+        #expect(delegate.callCount == 1)
+        #expect(dataCache.store.isEmpty)
+    }
+
+    /// The reason to take a closure: encrypting the data on a queue of its
+    /// own, for example. The closure must not assume it's called on the actor.
+    @Test func nuke13WillCacheCanCompleteOnAnotherThread() async throws {
+        // GIVEN
+        let delegate = Nuke13CachingDelegate { data, completion in
+            nonisolated(unsafe) let completion = completion
+            DispatchQueue.global().async { completion(Data("sealed".utf8) + data) }
+        }
+        let pipeline = makePipeline(delegate: delegate)
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(dataCache.store == [Test.url.absoluteString: Data("sealed".utf8) + Test.data])
+    }
+
+    /// Nuke 13 didn't wait for the closure, so leaving it uncalled kept the
+    /// data off the disk, and the image still loaded. The pipeline now waits,
+    /// and must not wait forever.
+    @Test func nuke13WillCacheThatNeverCompletesDoesNotBlockTheLoad() async throws {
+        // GIVEN
+        let delegate = Nuke13CachingDelegate { _, _ in }
+        let pipeline = makePipeline(delegate: delegate)
+
+        // WHEN
+        let image = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(image.size == Test.image.size)
+        #expect(delegate.callCount == 1)
+        #expect(dataCache.store.isEmpty)
+    }
+
+    /// Nuke 13 stored the data on each call, so it survived the mistake.
+    @Test func nuke13WillCacheThatCompletesTwiceUsesTheFirstResult() async throws {
+        // GIVEN
+        let delegate = Nuke13CachingDelegate { data, completion in
+            completion(data)
+            completion(nil)
+        }
+        let pipeline = makePipeline(delegate: delegate)
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(dataCache.store == [Test.url.absoluteString: Test.data])
+    }
+
+    @Test func delegateWithoutWillCacheStoresTheData() async throws {
+        // GIVEN
+        final class Delegate: ImagePipeline.Delegate {}
+        let pipeline = makePipeline(delegate: Delegate())
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(dataCache.store == [Test.url.absoluteString: Test.data])
+    }
+
+    @Test func asyncWillCacheWinsOverTheNuke13Method() async throws {
+        // GIVEN a delegate that adopted the new method and left the old one
+        let delegate = Nuke13CachingDelegate { _, completion in completion(nil) }
+        let migrated = MigratedCachingDelegate(nuke13: delegate)
+        let pipeline = makePipeline(delegate: migrated)
+
+        // WHEN
+        _ = try await pipeline.image(for: Test.request)
+
+        // THEN
+        #expect(delegate.callCount == 0)
+        #expect(dataCache.store == [Test.url.absoluteString: Data("async".utf8)])
+    }
+
     /// Documented: "This method is called only if the request parameters and
     /// data caching policy of the pipeline already allow caching."
     @Test func willCacheIsNotCalledForOriginalDataWhenTheRequestDisablesDiskWrites() async throws {
@@ -284,3 +430,53 @@ struct ImagePipelineDelegateCachingTests {
         #expect(context.image.nk_test_processorIDs == ["p1"])
     }
 }
+
+// MARK: - Helpers
+
+/// A delegate written for Nuke 13, when `willCache` took a completion closure.
+private final class Nuke13CachingDelegate: ImagePipeline.Delegate, Sendable {
+    typealias Handler = @Sendable (Data, @escaping (Data?) -> Void) -> Void
+
+    private let handler: Handler
+    private let _callCount = OSAllocatedUnfairLock(initialState: 0)
+
+    var callCount: Int { _callCount.withLock { $0 } }
+
+    init(handler: @escaping Handler) {
+        self.handler = handler
+    }
+
+    func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline, completion: @escaping (Data?) -> Void) {
+        _callCount.withLock { $0 += 1 }
+        handler(data, completion)
+    }
+}
+
+/// The way the migration guide says to declare a conformance.
+private final class Nuke13ExtensionDelegate: Sendable {
+    let isCalled = OSAllocatedUnfairLock(initialState: false)
+}
+
+extension Nuke13ExtensionDelegate: ImagePipeline.Delegate {
+    func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline, completion: @escaping (Data?) -> Void) {
+        isCalled.withLock { $0 = true }
+        completion(nil)
+    }
+}
+
+/// Implements both forms of `willCache`.
+private final class MigratedCachingDelegate: ImagePipeline.Delegate, Sendable {
+    private let nuke13: Nuke13CachingDelegate
+
+    init(nuke13: Nuke13CachingDelegate) {
+        self.nuke13 = nuke13
+    }
+
+    func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline) async -> Data? {
+        Data("async".utf8)
+    }
+
+    func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pipeline: ImagePipeline, completion: @escaping (Data?) -> Void) {
+        nuke13.willCache(data: data, image: image, for: request, pipeline: pipeline, completion: completion)
+    }
+}

context7.json

@@ -23,7 +23,7 @@
     "Check the Nuke version in Package.resolved first: these docs describe Nuke 14, and the latest stable release is Nuke 13.",
     "Use LazyImage from NukeUI in SwiftUI, and ImageLoader.load(_:into:) from NukeUI in UIKit and AppKit. Don't use NukeExtensions in new code.",
     "LazyImage plays animated images with no setup. A custom content closure needs AnimatedImage(container:), and UIKit needs AnimatedImageView. Don't decode the frames yourself.",
-    "A pipeline delegate method with the wrong signature compiles and is never called. Declare the conformance in an extension, and check the signatures. In Nuke 14, willCache is async and takes no completion closure.",
+    "A pipeline delegate method with the wrong signature compiles and is never called. Declare the conformance in an extension, and check the signatures. In Nuke 14, implement the async willCache: the form with a completion closure is soft-deprecated.",
     "Keep one ImagePipeline for the app, set on ImagePipeline.shared at launch. Never create one per view.",
     "To find out why an image doesn't load, launch with NUKE_DIAGNOSTICS_LOG=1 and read the log."
   ],

skills/nuke-migrate/SKILL.md

@@ -137,9 +137,9 @@ Go through the list from step 2.
 
 ### Delegate methods that are never called
 
-Every method of `ImagePipeline.Delegate` has a default implementation, so a method with an old signature compiles, and the pipeline calls the default instead. Nothing warns about it. The worst case is `willCache`: a delegate that kept private images off the disk stops doing so.
+Every method of `ImagePipeline.Delegate` has a default implementation, so a method with an old signature compiles, and the pipeline calls the default instead. Nothing warns about it. The one exception is the Nuke 13 `willCache` with its completion closure, which the pipeline still calls.
 
-Compare **every** method in **every** conformance with this list. Any method whose name matches and whose signature differs is dead:
+Compare **every** method in **every** conformance with this list. Any other method whose name matches and whose signature differs is dead:
 
 ```swift
 extension PipelineDelegate: ImagePipeline.Delegate {
@@ -160,7 +160,9 @@ extension PipelineDelegate: ImagePipeline.Delegate {
 }
 ```
 
-Those are the only requirements, and the bodies above are not the defaults to copy: leave out what you don't need. Move the conformance into an extension, as above, which is the only place the compiler warns about a method that nearly matches. It doesn't catch a different number of parameters, which is why the old `willCache` needs a person to look at it:
+The bodies above are not the defaults to copy: leave out what you don't need. Move the conformance into an extension, as above, which is the only place the compiler warns about a method that nearly matches. It doesn't catch a different number of parameters.
+
+The default implementation of the new `willCache` calls the Nuke 13 one, so a delegate that keeps private images off the disk still does. It's soft-deprecated, and if a delegate has both, the pipeline calls the new one, so move the logic over:
 
 ```swift
 // Nuke 13
@@ -174,7 +176,9 @@ func willCache(data: Data, image: ImageContainer?, for request: ImageRequest, pi
 }
 ```
 
-If the method does more than decide, such as encrypting the data, keep that logic and return the result. These older methods are dead too:
+If the method does more than decide, such as encrypting the data, keep that logic and return the result. Until it moves, note that Nuke 13 didn't wait for the completion closure, and now the image waits for it when the pipeline stores the original data.
+
+These older methods are dead:
 
 | Never called | Use, in `imageTask(_:didReceiveEvent:pipeline:)` |
 |---|---|