From deec88f8698eb04255c414d097fd99331aef96f3 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:54:23 -0500 Subject: [PATCH 1/4] Update EditorMediaUtility to use ImageDownloader directly (without AuthenticatedImageDownload redirect) --- .../Gutenberg/EditorMediaUtility.swift | 29 ++++++++++--------- .../Gutenberg/GutenbergImageLoader.swift | 18 ------------ .../Utils/GutenbergMediaEditorImage.swift | 2 +- 3 files changed, 17 insertions(+), 32 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift b/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift index c740486437b5..c6dd9ff97a38 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift @@ -137,17 +137,18 @@ class EditorMediaUtility { callbackQueue.async { failure(error) } - return EmptyImageDownloaderTask() - case let .success((requestURL, mediaHost)): - let imageDownload = AuthenticatedImageDownload( - url: requestURL, - mediaHost: mediaHost, - callbackQueue: callbackQueue, - onSuccess: success, - onFailure: failure - ) - imageDownload.start() - return imageDownload + return MeediaUtilityTask { /* do nothing */ } + case let .success((imageURL, host)): + let task = Task { @MainActor in + do { + let image = try await ImageDownloader.shared.image(from: imageURL, host: host) + success(image) + } catch { + failure(error) + + } + } + return MeediaUtilityTask { task.cancel() } } } @@ -251,8 +252,10 @@ class EditorMediaUtility { } } -private class EmptyImageDownloaderTask: ImageDownloaderTask { +private struct MeediaUtilityTask: ImageDownloaderTask { + let closure: @Sendable () -> Void + func cancel() { - // Do nothing + closure() } } diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergImageLoader.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergImageLoader.swift index a0ce0202c7c6..2a1b46e5f9c6 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergImageLoader.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergImageLoader.swift @@ -19,12 +19,6 @@ class GutenbergImageLoader: NSObject, RCTImageURLLoader { } func loadImage(for imageURL: URL, size: CGSize, scale: CGFloat, resizeMode: RCTResizeMode, progressHandler: RCTImageLoaderProgressBlock, partialLoadHandler: RCTImageLoaderPartialLoadBlock, completionHandler: @escaping RCTImageLoaderCompletionBlock) -> RCTImageLoaderCancellationBlock? { - let cacheKey = getCacheKey(for: imageURL, size: size) - - if let image = AnimatedImageCache.shared.cachedStaticImage(url: cacheKey) { - completionHandler(nil, image) - return {} - } var finalSize = size var finalScale = scale @@ -36,7 +30,6 @@ class GutenbergImageLoader: NSObject, RCTImageURLLoader { } let task = mediaUtility.downloadImage(from: imageURL, size: finalSize, scale: finalScale, post: post, success: { (image) in - AnimatedImageCache.shared.cacheStaticImage(url: cacheKey, image: image) completionHandler(nil, image) }, onFailure: { (error) in completionHandler(error, nil) @@ -56,17 +49,6 @@ class GutenbergImageLoader: NSObject, RCTImageURLLoader { return nil } - private func getCacheKey(for url: URL, size: CGSize) -> URL? { - guard size != CGSize.zero else { - return url - } - var components = URLComponents(url: url, resolvingAgainstBaseURL: true) - let queryItems = components?.queryItems - let newQueryItems = (queryItems ?? []) + [URLQueryItem(name: "cachekey", value: "\(size)")] - components?.queryItems = newQueryItems - return components?.url - } - static func moduleName() -> String! { return String(describing: self) } diff --git a/WordPress/Classes/ViewRelated/Gutenberg/Utils/GutenbergMediaEditorImage.swift b/WordPress/Classes/ViewRelated/Gutenberg/Utils/GutenbergMediaEditorImage.swift index 9f4b34ae79f3..a3882df36f10 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/Utils/GutenbergMediaEditorImage.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/Utils/GutenbergMediaEditorImage.swift @@ -32,7 +32,7 @@ class GutenbergMediaEditorImage: AsyncImage { init(url: URL, post: AbstractPost) { originalURL = url self.post = post - thumb = AnimatedImageCache.shared.cachedStaticImage(url: originalURL) + thumb = ImageDownloader.shared.cachedImage(for: originalURL) } /** From e86a623da9499f4eff6bcdd5d1bb516048c544c9 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:57:51 -0500 Subject: [PATCH 2/4] Remove AuthenticatedImageDownload --- .../Gutenberg/EditorMediaUtility.swift | 60 ++----------------- 1 file changed, 5 insertions(+), 55 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift b/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift index c6dd9ff97a38..225583627599 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift @@ -4,60 +4,6 @@ import Gridicons import WordPressShared import WordPressMedia -final class AuthenticatedImageDownload: AsyncOperation, @unchecked Sendable { - enum DownloadError: Error { - case blogNotFound - } - - let url: URL - let mediaHost: MediaHost - private let callbackQueue: DispatchQueue - private let onSuccess: (UIImage) -> () - private let onFailure: (Error) -> () - - init(url: URL, mediaHost: MediaHost, callbackQueue: DispatchQueue, onSuccess: @escaping (UIImage) -> (), onFailure: @escaping (Error) -> ()) { - self.url = url - self.mediaHost = mediaHost - self.callbackQueue = callbackQueue - self.onSuccess = onSuccess - self.onFailure = onFailure - } - - override func main() { - let mediaRequestAuthenticator = MediaRequestAuthenticator() - mediaRequestAuthenticator.authenticatedRequest( - for: url, - from: mediaHost, - onComplete: { request in - ImageDownloader.shared.downloadImage(for: request) { (image, error) in - self.state = .isFinished - - self.callbackQueue.async { - guard let image else { - DDLogError("Unable to download image for attachment with url = \(String(describing: request.url)). Details: \(String(describing: error?.localizedDescription))") - if let error { - self.onFailure(error) - } else { - self.onFailure(NSError(domain: NSURLErrorDomain, code: NSURLErrorUnknown, userInfo: nil)) - } - - return - } - - self.onSuccess(image) - } - } - }, - onFailure: { error in - self.state = .isFinished - self.callbackQueue.async { - self.onFailure(error) - } - } - ) - } -} - class EditorMediaUtility { private static let InternalInconsistencyError = NSError(domain: NSExceptionName.internalInconsistencyException.rawValue, code: 0) @@ -65,6 +11,10 @@ class EditorMediaUtility { static let placeholderDocumentLink = URL(string: "documentUploading://")! } + enum DownloadError: Error { + case blogNotFound + } + func placeholderImage(for attachment: NSTextAttachment, size: CGSize, tintColor: UIColor?) -> UIImage { var icon: UIImage switch attachment { @@ -161,7 +111,7 @@ class EditorMediaUtility { ) throws -> (URL, MediaHost) { // This function is added to debug the issue linked below. let safeExistingObject: (NSManagedObjectID) throws -> NSManagedObject = { objectID in - var object: Result = .failure(AuthenticatedImageDownload.DownloadError.blogNotFound) + var object: Result = .failure(DownloadError.blogNotFound) do { // Catch an Objective-C `NSInvalidArgumentException` exception from `existingObject(with:)`. // See https://github.com/wordpress-mobile/WordPress-iOS/issues/20630 From 934e7eebc096545cf38b5e2e4d81b3b699b95995 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 13:11:34 -0500 Subject: [PATCH 3/4] Update MediaExternalExporter to use ImageDownloader for downloading GIF data --- .../Utility/Media/MediaExternalExporter.swift | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/WordPress/Classes/Utility/Media/MediaExternalExporter.swift b/WordPress/Classes/Utility/Media/MediaExternalExporter.swift index 75dfc1bbbe69..07f9e5e681b4 100644 --- a/WordPress/Classes/Utility/Media/MediaExternalExporter.swift +++ b/WordPress/Classes/Utility/Media/MediaExternalExporter.swift @@ -49,21 +49,16 @@ class MediaExternalExporter: MediaExporter { /// Downloads an external GIF file, or uses one from the AnimatedImageCache. /// private func downloadGif(from url: URL, onCompletion: @escaping OnMediaExport, onError: @escaping OnExportError) -> Progress { - let request = URLRequest(url: url) - let task = AnimatedImageCache.shared.animatedImage(request, placeholderImage: nil, - success: { (data, _) in - self.gifDataDownloaded(data: data, - fromURL: url, - error: nil, - onCompletion: onCompletion, - onError: onError) - }, failure: { error in - if let error { - onError(self.exporterErrorWith(error: error)) + Task { + do { + let options = ImageRequestOptions(isMemoryCacheEnabled: false) + let data = try await ImageDownloader.shared.data(for: ImageRequest(url: url, options: options)) + self.gifDataDownloaded(data: data, fromURL: url, error: nil, onCompletion: onCompletion, onError: onError) + } catch { + onError(ExportError.downloadError(error as NSError)) } - }) - - return task?.progress ?? Progress.discreteCompletedProgress() + } + return Progress.discreteCompletedProgress() } /// Saves downloaded GIF data to the filesystem and exports it. From 0136de2172bbb7a5d4fa5affe12fe808ae77b255 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 13:12:04 -0500 Subject: [PATCH 4/4] Remove AnimatedImageCache --- .../Utility/Media/MediaExternalExporter.swift | 2 - .../Media/MemoryCache+Extensions.swift | 2 - .../Media/AnimatedImageCache.swift | 99 ------------------- 3 files changed, 103 deletions(-) delete mode 100644 WordPress/Classes/ViewRelated/Media/AnimatedImageCache.swift diff --git a/WordPress/Classes/Utility/Media/MediaExternalExporter.swift b/WordPress/Classes/Utility/Media/MediaExternalExporter.swift index 07f9e5e681b4..25809dfccba3 100644 --- a/WordPress/Classes/Utility/Media/MediaExternalExporter.swift +++ b/WordPress/Classes/Utility/Media/MediaExternalExporter.swift @@ -46,8 +46,6 @@ class MediaExternalExporter: MediaExporter { return Progress.discreteCompletedProgress() } - /// Downloads an external GIF file, or uses one from the AnimatedImageCache. - /// private func downloadGif(from url: URL, onCompletion: @escaping OnMediaExport, onError: @escaping OnExportError) -> Progress { Task { do { diff --git a/WordPress/Classes/Utility/Media/MemoryCache+Extensions.swift b/WordPress/Classes/Utility/Media/MemoryCache+Extensions.swift index 48e208b35a4e..1cbada8b8a06 100644 --- a/WordPress/Classes/Utility/Media/MemoryCache+Extensions.swift +++ b/WordPress/Classes/Utility/Media/MemoryCache+Extensions.swift @@ -13,8 +13,6 @@ extension MemoryCache { UIImageView.af.sharedImageDownloader = AlamofireImage.ImageDownloader( imageCache: AlamofireImageCacheAdapter(cache: .shared) ) - - // WordPress.AnimatedImageCache uses WordPress.MemoryCache directly } } diff --git a/WordPress/Classes/ViewRelated/Media/AnimatedImageCache.swift b/WordPress/Classes/ViewRelated/Media/AnimatedImageCache.swift deleted file mode 100644 index 47d0f2b782f2..000000000000 --- a/WordPress/Classes/ViewRelated/Media/AnimatedImageCache.swift +++ /dev/null @@ -1,99 +0,0 @@ -import UIKit -import WordPressMedia - -/// AnimatedImageCache is an image + animated gif data cache used in -/// CachedAnimatedImageView. It should be accessed via the `shared` singleton. -/// -final class AnimatedImageCache { - - // MARK: Singleton - - static let shared: AnimatedImageCache = AnimatedImageCache() - - private init() {} - - // MARK: Private fields - - fileprivate lazy var session: URLSession = { - let sessionConfiguration = URLSessionConfiguration.default - let session = URLSession(configuration: sessionConfiguration) - return session - }() - - // MARK: Instance methods - - func cacheData(data: Data, url: URL?) { - guard let url else { return } - let key = url.absoluteString + Constants.keyDataSuffix - MemoryCache.shared.setData(data, forKey: key) - } - - func cachedData(url: URL?) -> Data? { - guard let url else { return nil } - let key = url.absoluteString + Constants.keyDataSuffix - return MemoryCache.shared.geData(forKey: key) - } - - func cacheStaticImage(url: URL?, image: UIImage?) { - guard let url, let image else { return } - let key = url.absoluteString + Constants.keyStaticImageSuffix - MemoryCache.shared.setImage(image, forKey: key) - } - - func cachedStaticImage(url: URL?) -> UIImage? { - guard let url else { return nil } - let key = url.absoluteString + Constants.keyStaticImageSuffix - return MemoryCache.shared.getImage(forKey: key) - } - - func animatedImage(_ urlRequest: URLRequest, - placeholderImage: UIImage?, - success: ((Data, UIImage?) -> Void)?, - failure: ((NSError?) -> Void)? ) -> URLSessionTask? { - - if let cachedImageData = cachedData(url: urlRequest.url) { - success?(cachedImageData, cachedStaticImage(url: urlRequest.url)) - return nil - } - - let task = session.dataTask(with: urlRequest, completionHandler: { [weak self] (data, response, error) in - //check if view is still here - guard let self else { - return - } - // check if there is an error - if let error { - let nsError = error as NSError - // task.cancel() triggers an error that we don't want to send to the error handler. - if nsError.code != NSURLErrorCancelled { - failure?(nsError) - } - return - } - // check if data is here and is animated gif - guard let data else { - failure?(nil) - return - } - - let staticImage = UIImage(data: data) - if let key = urlRequest.url { - self.cacheData(data: data, url: key) - self.cacheStaticImage(url: key, image: staticImage) - } - success?(data, staticImage) - }) - - task.resume() - return task - } -} - -// MARK: - Constants - -private extension AnimatedImageCache { - struct Constants { - static let keyDataSuffix = "_data" - static let keyStaticImageSuffix = "_static_image" - } -}