From 5567de8efcf2af43b488421cdff642c50910abb0 Mon Sep 17 00:00:00 2001 From: kean Date: Wed, 20 Sep 2023 16:16:43 -0400 Subject: [PATCH] Add MediaImageService unit tests --- .../Classes/Services/MediaImageService.swift | 125 ++++++++++-------- .../MediaImageServiceTests.swift | 56 ++++++++ 2 files changed, 124 insertions(+), 57 deletions(-) diff --git a/WordPress/Classes/Services/MediaImageService.swift b/WordPress/Classes/Services/MediaImageService.swift index 38725cff0e85..7a6cbe07c767 100644 --- a/WordPress/Classes/Services/MediaImageService.swift +++ b/WordPress/Classes/Services/MediaImageService.swift @@ -10,9 +10,10 @@ final class MediaImageService: NSObject { private let mediaFileManager: MediaFileManager private let ioQueue = DispatchQueue(label: "org.automattic.MediaImageService") - init(coreDataStack: CoreDataStackSwift = ContextManager.shared) { + init(coreDataStack: CoreDataStackSwift = ContextManager.shared, + mediaFileManager: MediaFileManager = MediaFileManager(directory: .cache)) { self.coreDataStack = coreDataStack - self.mediaFileManager = MediaFileManager(directory: .cache) + self.mediaFileManager = mediaFileManager let configuration = URLSessionConfiguration.default // `MediaImageService` has its own disk cache, so it's important to @@ -30,49 +31,49 @@ final class MediaImageService: NSObject { /// (bitmapped) and are ready to be displayed. @MainActor func thumbnail(for media: Media) async throws -> UIImage { + let size = ThumbnailSize.small guard media.remoteStatus != .stub else { let media = try await fetchStubMedia(for: media) - // This should never happen, but adding it just in case to avoid recusion guard media.remoteStatus != .stub else { assertionFailure("The fetched media still has a .stub status") throw MediaThumbnailExporter.ThumbnailExportError.failedToGenerateThumbnailFileURL } - return try await _thumbnail(for: media) + return try await _thumbnail(for: media, size: size) } - return try await _thumbnail(for: media) + return try await _thumbnail(for: media, size: size) } @MainActor - private func _thumbnail(for media: Media) async throws -> UIImage { - if let image = await cachedThumbnail(for: media) { + private func _thumbnail(for media: Media, size: ThumbnailSize) async throws -> UIImage { + if let image = await cachedThumbnail(for: media, size: size) { return image } - let targetSize = MediaImageService.preferredThumbnailSize - if let image = await localThumbnail(for: media, targetSize: targetSize) { + if let image = await localThumbnail(for: media, size: size) { return image } - return try await remoteThumbnail(for: media, targetSize: targetSize) + return try await remoteThumbnail(for: media, size: size) } // MARK: - Cached Thumbnail /// Returns a local thumbnail for the given media object (if available). @MainActor - private func cachedThumbnail(for media: Media) async -> UIImage? { + private func cachedThumbnail(for media: Media, size: ThumbnailSize) async -> UIImage? { let objectID = media.objectID - return try? await Task.detached(priority: .userInitiated) { - let imageURL = try self.getCachedThumbnailURL(for: objectID) + return try? await Task.detached { + let imageURL = try self.getCachedThumbnailURL(for: objectID, size: size) let data = try Data(contentsOf: imageURL) return try decompressedImage(from: data) }.value } - private func getCachedThumbnailURL(for objectID: NSManagedObjectID) throws -> URL { + private func getCachedThumbnailURL(for objectID: NSManagedObjectID, size: ThumbnailSize) throws -> URL { let objectID = objectID.uriRepresentation().lastPathComponent return try mediaFileManager.makeLocalMediaURL( - withFilename: "\(objectID)-small-thumbnail", - fileExtension: nil // It can be different between local and remove thumbnails + withFilename: "\(objectID)-\(size.rawValue)-thumbnail", + fileExtension: nil, // It can be different between local and remove thumbnails + incremented: false ) } @@ -80,26 +81,29 @@ final class MediaImageService: NSObject { // exceedingly unlikely it'll result in any duplicated work thanks to the // memore caches. @MainActor - private func saveThumbnail(for media: Media, _ closure: @escaping (URL) throws -> Void) { + private func saveThumbnail(for media: Media, size: ThumbnailSize, _ closure: @escaping (URL) throws -> Void) { let objectID = media.objectID ioQueue.async { - guard let targetURL = try? self.getCachedThumbnailURL(for: objectID) else { return } + guard let targetURL = try? self.getCachedThumbnailURL(for: objectID, size: size) else { return } try? closure(targetURL) } } + /// Flushes all pending I/O changes to disk. + /// + /// - warning: For testing purposes only. + func flush() { + ioQueue.sync {} + } + // MARK: - Local Thumbnail - /// Generates a thumbnail from a local asset. - /// - /// - Parameters: - /// - media: The Media object. - /// - targetSize: An ideal size of the thumbnail in pixels. + /// Generates a thumbnail from a local asset and saves it in cache. @MainActor - private func localThumbnail(for media: Media, targetSize: CGSize) async -> UIImage? { + private func localThumbnail(for media: Media, size: ThumbnailSize) async -> UIImage? { let exporter = MediaThumbnailExporter() exporter.mediaDirectoryType = .cache - exporter.options.preferredSize = MediaImageService.targetSize(for: media, targetSize: targetSize) + exporter.options.preferredSize = MediaImageService.getThumbnailSize(for: media, size: size) exporter.options.scale = 1 // In pixels guard let sourceURL = media.absoluteLocalURL, @@ -111,13 +115,13 @@ final class MediaImageService: NSObject { return nil } - let image = try? await Task.detached(priority: .userInitiated) { + let image = try? await Task.detached { let data = try Data(contentsOf: export.url) return try decompressedImage(from: data) }.value // The order is important to ensure `export.url` still exists when creating an image - saveThumbnail(for: media) { targetURL in + saveThumbnail(for: media, size: size) { targetURL in try FileManager.default.moveItem(at: export.url, to: targetURL) } @@ -126,14 +130,10 @@ final class MediaImageService: NSObject { // MARK: - Remote Thumbnail - /// Downloads thumbnail for the given media object and saves it locally. Returns - /// a file URL for the downloaded thumbnail. - /// - /// - Parameters: - /// - media: The Media object. - /// - targetSize: An ideal size of the thumbnail in pixels. + /// Downloads a remote thumbnail and saves it in cache. @MainActor - private func remoteThumbnail(for media: Media, targetSize: CGSize) async throws -> UIImage { + private func remoteThumbnail(for media: Media, size: ThumbnailSize) async throws -> UIImage { + let targetSize = MediaImageService.getThumbnailSize(for: media, size: size) guard let imageURL = remoteThumbnailURL(for: media, targetSize: targetSize) else { throw URLError(.badURL) } @@ -146,11 +146,11 @@ final class MediaImageService: NSObject { } let (data, _) = try await session.data(for: request) - saveThumbnail(for: media) { targetURL in + saveThumbnail(for: media, size: size) { targetURL in try data.write(to: targetURL) } - return try await Task.detached(priority: .userInitiated) { + return try await Task.detached { try decompressedImage(from: data) }.value } @@ -188,35 +188,46 @@ final class MediaImageService: NSObject { // MARK: - Target Size - /// Returns a preferred thumbnail size (in pixels) optimized for the device. + enum ThumbnailSize: String { + case small + } + + /// Returns an optiomal target size in pixels for a thumbnail of the given + /// size for the given media asset. /// - /// - important: It makes sure the app uses the same thumbnails across - /// different screens and presentation modes to avoid fetching and caching - /// more than one version of the same image. - private static let preferredThumbnailSize: CGSize = { - let screenSide = min(UIScreen.main.bounds.width, UIScreen.main.bounds.height) - let itemPerRow = UIDevice.current.userInterfaceIdiom == .pad ? 5 : 4 - let availableWidth = screenSide - MediaViewController.spacing * CGFloat(itemPerRow - 1) - let targetSide = (availableWidth / CGFloat(itemPerRow)).rounded(.down) - let targetSize = CGSize(width: targetSide, height: targetSide) - return targetSize.scaled(by: UIScreen.main.scale) - }() - - // Both Photon (Site Optimizer) and MediaThumbnailExporter doesn't support - // "aspect-fill" resizing mode, which is want we want for the thumbnails. - // - // This method converts the target size to support aspect fill mode. - // - // Example: if media size is 2000x3000 px and targetSize is 200x200 px, the - // returned value will be 200x300 px. - private static func targetSize(for media: Media, targetSize: CGSize) -> CGSize { + /// The size is calculated to fill a collection view cell, assuming the app + /// displays a few cells in a row. The cell size can vary depending on whether the + /// device is in landscape or portrait mode, but the thumbnail size is + /// guaranteed to always be the same across app launches. + /// + /// Example: if media size is 2000x3000 px and targetSize is 200x200 px, the + /// returned value will be 200x300 px. + static func getThumbnailSize(for media: Media, size: ThumbnailSize) -> CGSize { let mediaSize = CGSize( width: CGFloat(media.width?.floatValue ?? 0), height: CGFloat(media.height?.floatValue ?? 0) ) + let targetSize = MediaImageService.getPreferredThumbnailSize(for: size) return MediaImageService.targetSize(forMediaSize: mediaSize, targetSize: targetSize) } + /// Returns a preferred thumbnail size (in pixels) optimized for the device. + /// + /// - important: It makes sure the app uses the same thumbnails across + /// different screens and presentation modes to avoid fetching and caching + /// more than one version of the same image. + private static func getPreferredThumbnailSize(for thumbnail: ThumbnailSize) -> CGSize { + switch thumbnail { + case .small: + let screenSide = min(UIScreen.main.bounds.width, UIScreen.main.bounds.height) + let itemPerRow = UIDevice.current.userInterfaceIdiom == .pad ? 5 : 4 + let availableWidth = screenSide - MediaViewController.spacing * CGFloat(itemPerRow - 1) + let targetSide = (availableWidth / CGFloat(itemPerRow)).rounded(.down) + let targetSize = CGSize(width: targetSide, height: targetSide) + return targetSize.scaled(by: UIScreen.main.scale) + } + } + static func targetSize(forMediaSize mediaSize: CGSize, targetSize originalTargetSize: CGSize) -> CGSize { guard mediaSize.width > 0 && mediaSize.height > 0 else { return originalTargetSize diff --git a/WordPress/WordPressTest/MediaImageServiceTests.swift b/WordPress/WordPressTest/MediaImageServiceTests.swift index 370f61b77be0..1be337d7c34b 100644 --- a/WordPress/WordPressTest/MediaImageServiceTests.swift +++ b/WordPress/WordPressTest/MediaImageServiceTests.swift @@ -2,6 +2,62 @@ import XCTest @testable import WordPress class MediaImageServiceTests: CoreDataTestCase { + var mediaFileManager: MediaFileManager! + var sut: MediaImageService! + + override func setUp() { + super.setUp() + + mediaFileManager = MediaFileManager(directory: .temporary(id: UUID())) + sut = MediaImageService( + coreDataStack: contextManager, + mediaFileManager: mediaFileManager + ) + } + + override func tearDown() { + super.tearDown() + + if let directoryURL = try? mediaFileManager.directoryURL() { + try? FileManager.default.removeItem(at: directoryURL) + } + } + + // MARK: - Thumbnail for Local Resource + + func testThatSmallThumbnailForLocalAssetIsGenerated() async throws { + // GIVEN + let media = Media(context: mainContext) + media.width = 1024 + media.height = 680 + let localURL = try makeLocalURL(forResource: "test-image", withExtension: "jpg") + media.absoluteLocalURL = localURL + try mainContext.obtainPermanentIDs(for: [media]) + + // WHEN + let thumbnail = try await sut.thumbnail(for: media) + + // THEN a small thumbnail is created + XCTAssertEqual(thumbnail.size, MediaImageService.getThumbnailSize(for: media, size: .small)) + + // GIVEN local asset is deleted + try FileManager.default.removeItem(at: localURL) + sut.flush() + + // WHEN + let cachedThumbnail = try await sut.thumbnail(for: media) + + // THEN cached thumbnail is still available + XCTAssertEqual(cachedThumbnail.size, MediaImageService.getThumbnailSize(for: media, size: .small)) + } + + /// `Media` is hardcoded to work with a specific direcoty URL managed by `MediaFileManager` + func makeLocalURL(forResource name: String, withExtension ext: String) throws -> URL { + let sourceURL = try XCTUnwrap(Bundle.test.url(forResource: "test-image", withExtension: "jpg")) + let mediaURL = try MediaFileManager.default.makeLocalMediaURL(withFilename: name, fileExtension: ext) + try FileManager.default.copyItem(at: sourceURL, to: mediaURL) + return mediaURL + } // MARK: - Target Size