Skip to content

Commit

Permalink
Fix ImagePipelineDelgate not being weak
Browse files Browse the repository at this point in the history
  • Loading branch information
Amol Prabhu committed Aug 8, 2024
1 parent 311016d commit 8e13096
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 30 deletions.
14 changes: 7 additions & 7 deletions Sources/Nuke/Pipeline/ImagePipeline+Cache.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,15 @@ extension ImagePipeline.Cache {

/// Returns image cache (memory cache) key for the given request.
public func makeImageCacheKey(for request: ImageRequest) -> ImageCacheKey {
if let customKey = pipeline.delegate.cacheKey(for: request, pipeline: pipeline) {
if let customKey = pipeline.delegate?.cacheKey(for: request, pipeline: pipeline) {
return ImageCacheKey(key: customKey)
}
return ImageCacheKey(request: request) // Use the default key
}

/// Returns data cache (disk cache) key for the given request.
public func makeDataCacheKey(for request: ImageRequest) -> String {
if let customKey = pipeline.delegate.cacheKey(for: request, pipeline: pipeline) {
if let customKey = pipeline.delegate?.cacheKey(for: request, pipeline: pipeline) {
return customKey
}
return "\(request.preferredImageId)\(request.thumbnail?.identifier ?? "")\(ImageProcessors.Composition(request.processors).identifier)"
Expand All @@ -223,24 +223,24 @@ extension ImagePipeline.Cache {

private func decodeImageData(_ data: Data, for request: ImageRequest) -> ImageContainer? {
let context = ImageDecodingContext(request: request, data: data, cacheType: .disk)
guard let decoder = pipeline.delegate.imageDecoder(for: context, pipeline: pipeline) else {
guard let decoder = pipeline.delegate?.imageDecoder(for: context, pipeline: pipeline) else {
return nil
}
return (try? decoder.decode(context))?.container
}

private func encodeImage(_ image: ImageContainer, for request: ImageRequest) -> Data? {
let context = ImageEncodingContext(request: request, image: image.image, urlResponse: nil)
let encoder = pipeline.delegate.imageEncoder(for: context, pipeline: pipeline)
return encoder.encode(image, context: context)
let encoder = pipeline.delegate?.imageEncoder(for: context, pipeline: pipeline)
return encoder?.encode(image, context: context)
}

private func imageCache(for request: ImageRequest) -> (any ImageCaching)? {
pipeline.delegate.imageCache(for: request, pipeline: pipeline)
pipeline.delegate?.imageCache(for: request, pipeline: pipeline)
}

private func dataCache(for request: ImageRequest) -> (any DataCaching)? {
pipeline.delegate.dataCache(for: request, pipeline: pipeline)
pipeline.delegate?.dataCache(for: request, pipeline: pipeline)
}

// MARK: Options
Expand Down
16 changes: 8 additions & 8 deletions Sources/Nuke/Pipeline/ImagePipeline.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public final class ImagePipeline: @unchecked Sendable {
/// Provides access to the underlying caching subsystems.
public var cache: ImagePipeline.Cache { .init(pipeline: self) }

let delegate: any ImagePipelineDelegate
weak var delegate: (any ImagePipelineDelegate)?

private var tasks = [ImageTask: TaskSubscription]()

Expand Down Expand Up @@ -289,7 +289,7 @@ public final class ImagePipeline: @unchecked Sendable {
let task = ImageTask(taskId: nextTaskId, request: request, isDataTask: isDataTask, pipeline: self, onEvent: onEvent)
// Important to call it before `imageTaskStartCalled`
if !isDataTask {
delegate.imageTaskCreated(task, pipeline: self)
delegate?.imageTaskCreated(task, pipeline: self)
}
task._task = Task {
try await withUnsafeThrowingContinuation { continuation in
Expand Down Expand Up @@ -317,7 +317,7 @@ public final class ImagePipeline: @unchecked Sendable {
tasks[task] = worker.subscribe(priority: task.priority.taskPriority, subscriber: task) { [weak task] in
task?._process($0)
}
delegate.imageTaskDidStart(task, pipeline: self)
delegate?.imageTaskDidStart(task, pipeline: self)
onTaskStarted?(task)
}

Expand Down Expand Up @@ -346,16 +346,16 @@ public final class ImagePipeline: @unchecked Sendable {
}

if !isDataTask {
delegate.imageTask(task, didReceiveEvent: event, pipeline: self)
delegate?.imageTask(task, didReceiveEvent: event, pipeline: self)
switch event {
case .progress(let progress):
delegate.imageTask(task, didUpdateProgress: progress, pipeline: self)
delegate?.imageTask(task, didUpdateProgress: progress, pipeline: self)
case .preview(let response):
delegate.imageTask(task, didReceivePreview: response, pipeline: self)
delegate?.imageTask(task, didReceivePreview: response, pipeline: self)
case .cancelled:
delegate.imageTaskDidCancel(task, pipeline: self)
delegate?.imageTaskDidCancel(task, pipeline: self)
case .finished(let result):
delegate.imageTask(task, didCompleteWithResult: result, pipeline: self)
delegate?.imageTask(task, didCompleteWithResult: result, pipeline: self)
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions Sources/Nuke/Tasks/TaskFetchOriginalData.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ final class TaskFetchOriginalData: AsyncPipelineTask<(Data, URLResponse?)> {

signpost(self, "LoadImageData", .begin, "URL: \(urlRequest.url?.absoluteString ?? ""), resumable data: \(Formatter.bytes(resumableData?.data.count ?? 0))")

let dataLoader = pipeline.delegate.dataLoader(for: request, pipeline: pipeline)
let dataTask = dataLoader.loadData(with: urlRequest, didReceiveData: { [weak self] data, response in
let dataLoader = pipeline.delegate?.dataLoader(for: request, pipeline: pipeline)
let dataTask = dataLoader?.loadData(with: urlRequest, didReceiveData: { [weak self] data, response in
guard let self else { return }
self.pipeline.queue.async {
self.dataTask(didReceiveData: data, response: response)
Expand All @@ -99,7 +99,7 @@ final class TaskFetchOriginalData: AsyncPipelineTask<(Data, URLResponse?)> {
guard let self else { return }

signpost(self, "LoadImageData", .end, "Cancelled")
dataTask.cancel()
dataTask?.cancel()
finish() // Finish the operation!

self.tryToSaveResumableData()
Expand Down Expand Up @@ -170,11 +170,11 @@ final class TaskFetchOriginalData: AsyncPipelineTask<(Data, URLResponse?)> {
extension AsyncPipelineTask where Value == (Data, URLResponse?) {
func storeDataInCacheIfNeeded(_ data: Data) {
let request = makeSanitizedRequest()
guard let dataCache = pipeline.delegate.dataCache(for: request, pipeline: pipeline), shouldStoreDataInDiskCache() else {
guard let dataCache = pipeline.delegate?.dataCache(for: request, pipeline: pipeline), shouldStoreDataInDiskCache() else {
return
}
let key = pipeline.cache.makeDataCacheKey(for: request)
pipeline.delegate.willCache(data: data, image: nil, for: request, pipeline: pipeline) {
pipeline.delegate?.willCache(data: data, image: nil, for: request, pipeline: pipeline) {
guard let data = $0 else { return }
// Important! Storing directly ignoring `ImageRequest.Options`.
dataCache.storeData(data, for: key)
Expand Down
2 changes: 1 addition & 1 deletion Sources/Nuke/Tasks/TaskFetchOriginalImage.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ final class TaskFetchOriginalImage: AsyncPipelineTask<ImageResponse> {
if let decoder {
return decoder
}
let decoder = pipeline.delegate.imageDecoder(for: context, pipeline: pipeline)
let decoder = pipeline.delegate?.imageDecoder(for: context, pipeline: pipeline)
self.decoder = decoder
return decoder
}
Expand Down
22 changes: 13 additions & 9 deletions Sources/Nuke/Tasks/TaskLoadImage.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class TaskLoadImage: AsyncPipelineTask<ImageResponse> {

private func decodeCachedData(_ data: Data) {
let context = ImageDecodingContext(request: request, data: data, cacheType: .disk)
guard let decoder = pipeline.delegate.imageDecoder(for: context, pipeline: pipeline) else {
guard let decoder = pipeline.delegate?.imageDecoder(for: context, pipeline: pipeline) else {
return didFinishDecoding(with: nil)
}
decode(context, decoder: decoder) { [weak self] in
Expand Down Expand Up @@ -82,7 +82,8 @@ final class TaskLoadImage: AsyncPipelineTask<ImageResponse> {
ImagePipeline.Error.processingFailed(processor: processor, context: context, error: error)
}
}
self.pipeline.queue.async {
self.pipeline.queue.async { [weak self] in
guard let self else { return }
self.operation = nil
self.didFinishProcessing(result: result, isCompleted: isCompleted)
}
Expand Down Expand Up @@ -115,9 +116,12 @@ final class TaskLoadImage: AsyncPipelineTask<ImageResponse> {
operation = pipeline.configuration.imageDecompressingQueue.add { [weak self] in
guard let self else { return }
let response = signpost(isCompleted ? "DecompressImage" : "DecompressProgressiveImage") {
self.pipeline.delegate.decompress(response: response, request: self.request, pipeline: self.pipeline)
self.pipeline.delegate?.decompress(response: response, request: self.request, pipeline: self.pipeline)
}
self.pipeline.queue.async {

guard let response else { return }
self.pipeline.queue.async { [weak self] in
guard let self else { return }
self.operation = nil
self.didReceiveDecompressedImage(response, isCompleted: isCompleted)
}
Expand All @@ -128,7 +132,7 @@ final class TaskLoadImage: AsyncPipelineTask<ImageResponse> {
ImageDecompression.isDecompressionNeeded(for: response) &&
!request.options.contains(.skipDecompression) &&
hasDirectSubscribers &&
pipeline.delegate.shouldDecompress(response: response, for: request, pipeline: pipeline)
pipeline.delegate?.shouldDecompress(response: response, for: request, pipeline: pipeline) ?? false
}

private func didReceiveDecompressedImage(_ response: ImageResponse, isCompleted: Bool) {
Expand All @@ -149,19 +153,19 @@ final class TaskLoadImage: AsyncPipelineTask<ImageResponse> {
}

private func storeImageInDataCache(_ response: ImageResponse) {
guard let dataCache = pipeline.delegate.dataCache(for: request, pipeline: pipeline) else {
guard let dataCache = pipeline.delegate?.dataCache(for: request, pipeline: pipeline) else {
return
}
let context = ImageEncodingContext(request: request, image: response.image, urlResponse: response.urlResponse)
let encoder = pipeline.delegate.imageEncoder(for: context, pipeline: pipeline)
let encoder = pipeline.delegate?.imageEncoder(for: context, pipeline: pipeline)
let key = pipeline.cache.makeDataCacheKey(for: request)
pipeline.configuration.imageEncodingQueue.addOperation { [weak pipeline, request] in
guard let pipeline else { return }
let encodedData = signpost("EncodeImage") {
encoder.encode(response.container, context: context)
encoder?.encode(response.container, context: context)
}
guard let data = encodedData, !data.isEmpty else { return }
pipeline.delegate.willCache(data: data, image: response.container, for: request, pipeline: pipeline) {
pipeline.delegate?.willCache(data: data, image: response.container, for: request, pipeline: pipeline) {
guard let data = $0, !data.isEmpty else { return }
// Important! Storing directly ignoring `ImageRequest.Options`.
dataCache.storeData(data, for: key) // This is instant, writes are async
Expand Down

0 comments on commit 8e13096

Please sign in to comment.