diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 8b92b5272978..27d16d45ae30 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,5 +1,6 @@ 22.1 ----- +* [***] [internal] Refactor uploading photos (from the device photo, the Free Photo library, and other sources) to the WordPress Media Library. Affected areas are where you can choose a photo and upload, including the "Media" screen, adding images to a post, updating site icon, etc. [#20322] 22.0 diff --git a/WordPress/Classes/Services/MediaCoordinator.swift b/WordPress/Classes/Services/MediaCoordinator.swift index f443beb2921b..5fdd415fa80a 100644 --- a/WordPress/Classes/Services/MediaCoordinator.swift +++ b/WordPress/Classes/Services/MediaCoordinator.swift @@ -138,10 +138,8 @@ class MediaCoordinator: NSObject { /// - parameter blog: The blog that the asset should be added to. /// - parameter origin: The location in the app where the upload was initiated (optional). /// - @discardableResult - func addMedia(from asset: ExportableAsset, to blog: Blog, analyticsInfo: MediaAnalyticsInfo? = nil) -> Media? { - let coordinator = mediaLibraryProgressCoordinator - return addMedia(from: asset, blog: blog, post: nil, coordinator: coordinator, analyticsInfo: analyticsInfo) + func addMedia(from asset: ExportableAsset, to blog: Blog, analyticsInfo: MediaAnalyticsInfo? = nil) { + addMedia(from: asset, blog: blog, post: nil, coordinator: mediaLibraryProgressCoordinator, analyticsInfo: analyticsInfo) } /// Adds the specified media asset to the specified post. The upload process @@ -153,57 +151,88 @@ class MediaCoordinator: NSObject { /// @discardableResult func addMedia(from asset: ExportableAsset, to post: AbstractPost, analyticsInfo: MediaAnalyticsInfo? = nil) -> Media? { - let coordinator = self.coordinator(for: post) - return addMedia(from: asset, blog: post.blog, post: post, coordinator: coordinator, analyticsInfo: analyticsInfo) + addMedia(from: asset, post: post, coordinator: coordinator(for: post), analyticsInfo: analyticsInfo) } - @discardableResult - private func addMedia(from asset: ExportableAsset, blog: Blog, post: AbstractPost?, coordinator: MediaProgressCoordinator, analyticsInfo: MediaAnalyticsInfo? = nil) -> Media? { + /// Create a `Media` instance from the main context and upload the asset to the Meida Library. + /// + /// - Warning: This function must be called from the main thread. + /// + /// - SeeAlso: `MediaImportService.createMedia(with:blog:post:thumbnailCallback:completion:)` + private func addMedia(from asset: ExportableAsset, post: AbstractPost, coordinator: MediaProgressCoordinator, analyticsInfo: MediaAnalyticsInfo? = nil) -> Media? { coordinator.track(numberOfItems: 1) - let service = mediaServiceFactory.create(mainContext) + let service = MediaImportService(coreDataStack: coreDataStack) let totalProgress = Progress.discreteProgress(totalUnitCount: MediaExportProgressUnits.done) - var creationProgress: Progress? = nil - let mediaOptional = service.createMedia(with: asset, - blog: blog, - post: post, - progress: &creationProgress, - thumbnailCallback: { [weak self] media, url in - self?.thumbnailReady(url: url, for: media) - }, - completion: { [weak self] media, error in - guard let strongSelf = self else { - return - } - if let error = error as NSError? { - if let media = media { - coordinator.attach(error: error as NSError, toMediaID: media.uploadID) - strongSelf.fail(error as NSError, media: media) - } else { - // If there was an error and we don't have a media object we just say to the coordinator that one item was finished - coordinator.finishOneItem() - } - return - } - guard let media = media, !media.isDeleted else { - return - } - - strongSelf.trackUploadOf(media, analyticsInfo: analyticsInfo) - - let uploadProgress = strongSelf.uploadMedia(media) - totalProgress.addChild(uploadProgress, withPendingUnitCount: MediaExportProgressUnits.threeQuartersDone) - }) - guard let media = mediaOptional else { + let result = service.createMedia( + with: asset, + blog: post.blog, + post: post, + thumbnailCallback: { [weak self] media, url in + self?.thumbnailReady(url: url, for: media) + }, + completion: { [weak self] media, error in + self?.handleMediaImportResult(coordinator: coordinator, totalProgress: totalProgress, analyticsInfo: analyticsInfo, media: media, error: error) + } + ) + guard let (media, creationProgress) = result else { return nil } + processing(media) - if let creationProgress = creationProgress { - totalProgress.addChild(creationProgress, withPendingUnitCount: MediaExportProgressUnits.quarterDone) - coordinator.track(progress: totalProgress, of: media, withIdentifier: media.uploadID) - } + + totalProgress.addChild(creationProgress, withPendingUnitCount: MediaExportProgressUnits.quarterDone) + coordinator.track(progress: totalProgress, of: media, withIdentifier: media.uploadID) + return media } + /// Create a `Media` instance and upload the asset to the Meida Library. + /// + /// - SeeAlso: `MediaImportService.createMedia(with:blog:post:receiveUpdate:thumbnailCallback:completion:)` + private func addMedia(from asset: ExportableAsset, blog: Blog, post: AbstractPost?, coordinator: MediaProgressCoordinator, analyticsInfo: MediaAnalyticsInfo? = nil) { + coordinator.track(numberOfItems: 1) + let service = MediaImportService(coreDataStack: coreDataStack) + let totalProgress = Progress.discreteProgress(totalUnitCount: MediaExportProgressUnits.done) + let creationProgress = service.createMedia( + with: asset, + blog: blog, + post: post, + receiveUpdate: { [weak self] media in + self?.processing(media) + coordinator.track(progress: totalProgress, of: media, withIdentifier: media.uploadID) + }, + thumbnailCallback: { [weak self] media, url in + self?.thumbnailReady(url: url, for: media) + }, + completion: { [weak self] media, error in + self?.handleMediaImportResult(coordinator: coordinator, totalProgress: totalProgress, analyticsInfo: analyticsInfo, media: media, error: error) + } + ) + + totalProgress.addChild(creationProgress, withPendingUnitCount: MediaExportProgressUnits.quarterDone) + } + + private func handleMediaImportResult(coordinator: MediaProgressCoordinator, totalProgress: Progress, analyticsInfo: MediaAnalyticsInfo?, media: Media?, error: Error?) -> Void { + if let error = error as NSError? { + if let media = media { + coordinator.attach(error: error as NSError, toMediaID: media.uploadID) + fail(error as NSError, media: media) + } else { + // If there was an error and we don't have a media object we just say to the coordinator that one item was finished + coordinator.finishOneItem() + } + return + } + guard let media = media, !media.isDeleted else { + return + } + + trackUploadOf(media, analyticsInfo: analyticsInfo) + + let uploadProgress = uploadMedia(media) + totalProgress.addChild(uploadProgress, withPendingUnitCount: MediaExportProgressUnits.threeQuartersDone) + } + /// Retry the upload of a media object that previously has failed. /// /// - Parameters: @@ -330,13 +359,14 @@ class MediaCoordinator: NSObject { self.fail(nserror, media: media) } - coreDataStack.performAndSave { context in - let service = self.mediaServiceFactory.create(context) - var progress: Progress? = nil - service.uploadMedia(media, automatedRetry: automatedRetry, progress: &progress, success: success, failure: failure) - if let progress { - resultProgress.addChild(progress, withPendingUnitCount: resultProgress.totalUnitCount) - } + // For some reason, this `MediaService` instance has to be created with the main context, otherwise + // the successfully uploaded media is shown as a "local" assets incorrectly (see the issue comment linked below). + // https://github.com/wordpress-mobile/WordPress-iOS/issues/20298#issuecomment-1465319707 + let service = self.mediaServiceFactory.create(coreDataStack.mainContext) + var progress: Progress? = nil + service.uploadMedia(media, automatedRetry: automatedRetry, progress: &progress, success: success, failure: failure) + if let progress { + resultProgress.addChild(progress, withPendingUnitCount: resultProgress.totalUnitCount) } uploading(media, progress: resultProgress) diff --git a/WordPress/Classes/Services/MediaImportService.swift b/WordPress/Classes/Services/MediaImportService.swift index ac5f40a5ec0b..b5f5a4b789b2 100644 --- a/WordPress/Classes/Services/MediaImportService.swift +++ b/WordPress/Classes/Services/MediaImportService.swift @@ -6,11 +6,11 @@ import CocoaLumberjack /// - Note: Methods with escaping closures will call back via the configured managedObjectContext /// method and its corresponding thread. /// -open class MediaImportService: LocalCoreDataService { +class MediaImportService: NSObject { private static let defaultImportQueue: DispatchQueue = DispatchQueue(label: "org.wordpress.mediaImportService", autoreleaseFrequency: .workItem) - @objc public lazy var importQueue: DispatchQueue = { + @objc lazy var importQueue: DispatchQueue = { return MediaImportService.defaultImportQueue }() @@ -20,55 +20,258 @@ open class MediaImportService: LocalCoreDataService { /// @objc static let preferredImageCompressionQuality = 0.9 - /// Allows the caller to designate supported import file types - @objc var allowableFileExtensions = Set<String>() - static let defaultAllowableFileExtensions = Set<String>(["docx", "ppt", "mp4", "ppsx", "3g2", "mpg", "ogv", "pptx", "xlsx", "jpeg", "xls", "mov", "key", "3gp", "png", "avi", "doc", "pdf", "gif", "odt", "pps", "m4v", "wmv", "jpg"]) /// Completion handler for a created Media object. /// - public typealias MediaCompletion = (Media) -> Void + typealias MediaCompletion = (Media) -> Void /// Error handler. /// - public typealias OnError = (Error) -> Void + typealias OnError = (Error) -> Void + + private let coreDataStack: CoreDataStackSwift + + /// The initialiser for Objective-C code. + /// + /// Using `ContextManager` as the argument becuase `CoreDataStackSwift` is not accessible from Objective-C code. + @objc + convenience init(contextManager: ContextManager) { + self.init(coreDataStack: contextManager) + } + + init(coreDataStack: CoreDataStackSwift) { + self.coreDataStack = coreDataStack + } // MARK: - Instance methods + /// Create a media object using the `ExportableAsset` provided as the source of media. + /// + /// - Note: All blocks arguments are called from the main thread. The `Media` argument in the blocks is bound to + /// the main context. + /// + /// - Warning: This function must be called from the main thread. + /// + /// This functions returns a `Media` instance. To ensure the returned `Media` instance continues to be a valid + /// instance, it can't be bound to a background context which are all temporary context. The only long living + /// context is the main context. And the safe way to create and return an object bound to the main context is + /// doing it from the main thread, which is why this function must be called from the main thread. + /// + /// - Parameters: + /// - exportable: an object that conforms to `ExportableAsset` + /// - blog: the blog object to associate to the media + /// - post: the optional post object to associate to the media + /// - thumbnailCallback: a closure that will be invoked when the thumbnail for the media object is ready + /// - completion: a closure that will be invoked when the media is created, on success it will return a valid `Media` + /// object, on failure it will return a `nil` `Media` and an error object with the details. + /// + /// - Returns: The new `Media` instance and a `Process` instance that tracks the progress of the export process + /// + /// - SeeAlso: `createMedia(with:blog:post:thumbnailCallback:completion:)` + func createMedia( + with exportable: ExportableAsset, + blog: Blog, + post: AbstractPost?, + thumbnailCallback: ((Media, URL) -> Void)?, + completion: @escaping (Media?, Error?) -> Void + ) -> (Media, Progress)? { + assert(Thread.isMainThread, "\(#function) can only be called from the main thread") + + guard let media = try? createMedia(with: exportable, blogObjectID: blog.objectID, postObjectID: post?.objectID, in: coreDataStack.mainContext) else { + return nil + } + + coreDataStack.saveContextAndWait(coreDataStack.mainContext) + + let blogInContext: Blog + do { + blogInContext = try coreDataStack.mainContext.existingObject(with: blog.objectID) as! Blog + } catch { + completion(nil, error) + return nil + } + + let createProgress = self.import(exportable, to: media, blog: blogInContext, thumbnailCallback: thumbnailCallback) { + switch $0 { + case let .success(media): + completion(media, nil) + case let .failure(error): + completion(media, error) + } + } + + return (media, createProgress) + } + + /// Create a media object using the `ExportableAsset` provided as the source of media. + /// + /// Unlike `createMedia(with:blog:post:thumbnailCallback:completion:)`, this function can be called from any thread. + /// + /// - Note: All blocks arguments are called from the main thread. The `Media` argument in the blocks is bound to + /// the main context. + /// + /// - Parameters: + /// - exportable: an object that conforms to `ExportableAsset` + /// - blog: the blog object to associate to the media + /// - post: the optional post object to associate to the media + /// - progress: a NSProgress that tracks the progress of the export process. + /// - receiveUpdate: a closure that will be invoked with the created `Media` instance. + /// - thumbnailCallback: a closure that will be invoked when the thumbnail for the media object is ready + /// - completion: a closure that will be invoked when the media is created, on success it will return a valid Media + /// object, on failure it will return a nil Media and an error object with the details. + @objc + @discardableResult + func createMedia( + with exportable: ExportableAsset, + blog: Blog, + post: AbstractPost?, + receiveUpdate: ((Media) -> Void)?, + thumbnailCallback: ((Media, URL) -> Void)?, + completion: @escaping (Media?, Error?) -> Void + ) -> Progress { + let createProgress = Progress.discreteProgress(totalUnitCount: 1) + coreDataStack.performAndSave({ context in + let media = try self.createMedia(with: exportable, blogObjectID: blog.objectID, postObjectID: post?.objectID, in: context) + try context.obtainPermanentIDs(for: [media]) + return media.objectID + }, completion: { (result: Result<NSManagedObjectID, Error>) in + let transformed = result.flatMap { mediaObjectID in + Result { + ( + try self.coreDataStack.mainContext.existingObject(with: mediaObjectID) as! Media, + try self.coreDataStack.mainContext.existingObject(with: blog.objectID) as! Blog + ) + } + } + switch transformed { + case let .success((media, blog)): + let progress = self.import(exportable, to: media, blog: blog, thumbnailCallback: thumbnailCallback) { + switch $0 { + case let .success(media): + completion(media, nil) + case let .failure(error): + completion(media, error) + } + } + createProgress.addChild(progress, withPendingUnitCount: 1) + receiveUpdate?(media) + case let .failure(error): + completion(nil, error) + } + }, on: .main) + return createProgress + } + + private func createMedia(with exportable: ExportableAsset, blogObjectID: NSManagedObjectID, postObjectID: NSManagedObjectID?, in context: NSManagedObjectContext) throws -> Media { + let blogInContext = try context.existingObject(with: blogObjectID) as! Blog + let postInContext = try postObjectID.flatMap(context.existingObject(with:)) as? AbstractPost + + let media = postInContext.flatMap(Media.makeMedia(post:)) ?? Media.makeMedia(blog: blogInContext) + media.mediaType = exportable.assetMediaType + media.remoteStatus = .processing + return media + } + + private func `import`( + _ exportable: ExportableAsset, + to media: Media, + blog: Blog, + thumbnailCallback: ((Media, URL) -> Void)?, + completion: @escaping (Result<Media, Error>) -> Void + ) -> Progress { + assert(Thread.isMainThread) + assert(media.managedObjectContext == coreDataStack.mainContext) + assert(blog.managedObjectContext == coreDataStack.mainContext) + + var allowedFileTypes = blog.allowedFileTypes as? Set<String> ?? [] + // HEIC isn't supported when uploading an image, so we filter it out (http://git.io/JJAae) + allowedFileTypes.remove("heic") + + let completion: (Result<Media, Error>) -> Void = { result in + self.coreDataStack.performAndSave({ context in + let media = try context.existingObject(with: media.objectID) as! Media + switch result { + case let .success(media): + media.remoteStatus = .local + media.error = nil + case let .failure(error): + media.remoteStatus = .failed + media.error = error + } + }, completion: { result in + let transformed = result.flatMap { + Result { + try self.coreDataStack.mainContext.existingObject(with: media.objectID) as! Media + } + } + + if case let .success(media) = transformed { + // Pre-generate a thumbnail image, see the method notes. + self.exportPlaceholderThumbnail(for: media) { url in + assert(Thread.isMainThread) + guard let url, let media = try? self.coreDataStack.mainContext.existingObject(with: media.objectID) as? Media else { + return + } + thumbnailCallback?(media, url) + } + } + + completion(transformed) + }, on: .main) + } + + return self.import(exportable, to: media, allowableFileExtensions: allowedFileTypes, completion: completion) + } + /// Imports media from a PHAsset to the Media object, asynchronously. /// /// - Parameters: /// - exportable: the exportable resource where data will be read from. /// - media: the media object to where media will be imported to. - /// - onCompletion: Called if the Media was successfully created and the asset's data imported to the absoluteLocalURL. - /// - onError: Called if an error was encountered during creation, error convertible to NSError with a localized description. + /// - onCompletion: Called if the Media was successfully created and the asset's data imported to the + /// `absoluteLocalURL`. This closure is called on the main thread. The closure's `media` argument is also + /// bound to the main context (`CoreDataStack.mainContext`). + /// - onError: Called if an error was encountered during creation, error convertible to `NSError` with a + /// localized description. This closure is called on the main thread. /// /// - Returns: a progress object that report the current state of the import process. /// - @objc(importResource:toMedia:onCompletion:onError:) - func `import`(_ exportable: ExportableAsset, to media: Media, onCompletion: @escaping MediaCompletion, onError: @escaping OnError) -> Progress? { + private func `import`(_ exportable: ExportableAsset, to media: Media, allowableFileExtensions: Set<String>, completion: @escaping (Result<Media, Error>) -> Void) -> Progress { let progress: Progress = Progress.discreteProgress(totalUnitCount: 1) importQueue.async { - guard let exporter = self.makeExporter(for: exportable) else { + guard let exporter = self.makeExporter(for: exportable, allowableFileExtensions: allowableFileExtensions) else { preconditionFailure("An exporter needs to be availale") } - let exportProgress = exporter.export(onCompletion: { export in - self.managedObjectContext.perform { - self.configureMedia(media, withExport: export) - ContextManager.sharedInstance().save(self.managedObjectContext, completion: { - onCompletion(media) + let exportProgress = exporter.export( + onCompletion: { export in + self.coreDataStack.performAndSave({ context in + let mediaInContext = try context.existingObject(with: media.objectID) as! Media + self.configureMedia(mediaInContext, withExport: export) + }, completion: { result in + completion( + result.flatMap { + Result { + try self.coreDataStack.mainContext.existingObject(with: media.objectID) as! Media + } + } + ) }, on: .main) + }, + onError: { error in + MediaImportService.logExportError(error) + // Return the error via the context's queue, and as an NSError to ensure it carries over the right code/message. + DispatchQueue.main.async { + completion(.failure(error)) + } } - }, onError: { mediaExportError in - self.handleExportError(mediaExportError, errorHandler: onError) - } ) progress.addChild(exportProgress, withPendingUnitCount: 1) } return progress } - func makeExporter(for exportable: ExportableAsset) -> MediaExporter? { + private func makeExporter(for exportable: ExportableAsset, allowableFileExtensions: Set<String>) -> MediaExporter? { switch exportable { case let asset as PHAsset: let exporter = MediaAssetExporter(asset: asset) @@ -84,7 +287,7 @@ open class MediaImportService: LocalCoreDataService { let exporter = MediaURLExporter(url: url) exporter.imageOptions = self.exporterImageOptions exporter.videoOptions = self.exporterVideoOptions - exporter.urlOptions = self.exporterURLOptions + exporter.urlOptions = self.exporterURLOptions(allowableFileExtensions: allowableFileExtensions) return exporter case let stockPhotosMedia as StockPhotosMedia: let exporter = MediaExternalExporter(externalAsset: stockPhotosMedia) @@ -97,6 +300,30 @@ open class MediaImportService: LocalCoreDataService { } } + /// Generate a thumbnail image for the `Media` so that consumers of the `absoluteThumbnailLocalURL` property + /// will have an image ready to load, without using the async methods provided via `MediaThumbnailService`. + /// + /// This is primarily used as a placeholder image throughout the code-base, particulary within the editors. + /// + /// Note: Ideally we wouldn't need this at all, but the synchronous usage of `absoluteThumbnailLocalURL` across the code-base + /// to load a thumbnail image is relied on quite heavily. In the future, transitioning to asynchronous thumbnail loading + /// via the new thumbnail service methods is much preferred, but would indeed take a good bit of refactoring away from + /// using `absoluteThumbnailLocalURL`. + func exportPlaceholderThumbnail(for media: Media, completion: ((URL?) -> Void)?) { + let thumbnailService = MediaThumbnailService(coreDataStack: coreDataStack) + thumbnailService.thumbnailURL(forMedia: media, preferredSize: .zero) { url in + self.coreDataStack.performAndSave({ context in + let mediaInContext = try context.existingObject(with: media.objectID) as! Media + // Set the absoluteThumbnailLocalURL with the generated thumbnail's URL. + mediaInContext.absoluteThumbnailLocalURL = url + }, completion: { _ in + completion?(url) + }, on: .main) + } onError: { error in + DDLogError("Error occurred exporting placeholder thumbnail: \(error)") + } + } + // MARK: - Helpers class func logExportError(_ error: MediaExportError) { @@ -120,21 +347,9 @@ open class MediaImportService: LocalCoreDataService { DDLogError("\(errorLogMessage), code: \(nerror.code), error: \(nerror)") } - /// Handle the OnError callback and logging any errors encountered. - /// - fileprivate func handleExportError(_ error: MediaExportError, errorHandler: OnError?) { - MediaImportService.logExportError(error) - // Return the error via the context's queue, and as an NSError to ensure it carries over the right code/message. - if let errorHandler = errorHandler { - self.managedObjectContext.perform { - errorHandler(error) - } - } - } - // MARK: - Media export configurations - fileprivate var exporterImageOptions: MediaImageExporter.Options { + private var exporterImageOptions: MediaImageExporter.Options { var options = MediaImageExporter.Options() options.maximumImageSize = self.exporterMaximumImageSize() options.stripsGeoLocationIfNeeded = MediaSettings().removeLocationSetting @@ -142,14 +357,14 @@ open class MediaImportService: LocalCoreDataService { return options } - fileprivate var exporterVideoOptions: MediaVideoExporter.Options { + private var exporterVideoOptions: MediaVideoExporter.Options { var options = MediaVideoExporter.Options() options.stripsGeoLocationIfNeeded = MediaSettings().removeLocationSetting options.exportPreset = MediaSettings().maxVideoSizeSetting.videoPreset return options } - fileprivate var exporterURLOptions: MediaURLExporter.Options { + private func exporterURLOptions(allowableFileExtensions: Set<String>) -> MediaURLExporter.Options { var options = MediaURLExporter.Options() options.allowableFileExtensions = allowableFileExtensions options.stripsGeoLocationIfNeeded = MediaSettings().removeLocationSetting @@ -161,7 +376,7 @@ open class MediaImportService: LocalCoreDataService { /// - Note: Eventually we'll rewrite MediaSettings.imageSizeForUpload to do this for us, but want to leave /// that class alone while implementing MediaExportService. /// - fileprivate func exporterMaximumImageSize() -> CGFloat? { + private func exporterMaximumImageSize() -> CGFloat? { let maxUploadSize = MediaSettings().imageSizeForUpload if maxUploadSize < Int.max { return CGFloat(maxUploadSize) @@ -171,7 +386,7 @@ open class MediaImportService: LocalCoreDataService { /// Configure Media with a MediaExport. /// - fileprivate func configureMedia(_ media: Media, withExport export: MediaExport) { + private func configureMedia(_ media: Media, withExport export: MediaExport) { media.absoluteLocalURL = export.url media.filename = export.url.lastPathComponent media.mediaType = (export.url as NSURL).assetMediaType diff --git a/WordPress/Classes/Services/MediaService.h b/WordPress/Classes/Services/MediaService.h index 666472aea719..5f8f0f726e0e 100644 --- a/WordPress/Classes/Services/MediaService.h +++ b/WordPress/Classes/Services/MediaService.h @@ -21,28 +21,6 @@ typedef NS_ERROR_ENUM(MediaServiceErrorDomain, MediaServiceError) { @interface MediaService : LocalCoreDataService -/** - This property determines if multiple thumbnail generation will be done in parallel. - By default this value is NO. - */ -@property (nonatomic, assign) BOOL concurrentThumbnailGeneration; -/** - Create a media object using the url provided as the source of media. - - @param exportable an object that implements the exportable interface - @param blog the blog object to associate to the media - @param post the post object to associate to the media - @param progress a NSProgress that tracks the progress of the export process. - @param thumbnailCallback a block that will be invoked when the thumbail for the media object is ready - @param completion a block that will be invoked when the media is created, on success it will return a valid Media object, on failure it will return a nil Media and an error object with the details. - */ -- (nullable Media *)createMediaWith:(nonnull id<ExportableAsset>)exportable - blog:(nonnull Blog *)blog - post:(nullable AbstractPost *)post - progress:(NSProgress * __nullable __autoreleasing * __nullable)progress - thumbnailCallback:(nullable void (^)(Media * __nonnull media, NSURL * __nonnull thumbnailURL))thumbnailCallback - completion:(nullable void (^)(Media * __nullable media, NSError * __nullable error))completion; - /** Get the Media object from the server using the blog and the mediaID as the identifier of the resource diff --git a/WordPress/Classes/Services/MediaService.m b/WordPress/Classes/Services/MediaService.m index 59adc85b5aa1..dddbc477eb48 100644 --- a/WordPress/Classes/Services/MediaService.m +++ b/WordPress/Classes/Services/MediaService.m @@ -16,140 +16,8 @@ NSErrorDomain const MediaServiceErrorDomain = @"MediaServiceErrorDomain"; -@interface MediaService () - -@property (nonatomic, strong) MediaThumbnailService *thumbnailService; - -@end - @implementation MediaService -- (instancetype)initWithManagedObjectContext:(NSManagedObjectContext *)context -{ - self = [super initWithManagedObjectContext:context]; - if (self) { - _concurrentThumbnailGeneration = NO; - } - return self; -} - -#pragma mark - Creating media - -- (Media *)createMediaWith:(id<ExportableAsset>)exportable - blog:(Blog *)blog - post:(AbstractPost *)post - progress:(NSProgress **)progress - thumbnailCallback:(void (^)(Media *media, NSURL *thumbnailURL))thumbnailCallback - completion:(void (^)(Media *media, NSError *error))completion -{ - NSParameterAssert(post == nil || blog == post.blog); - NSParameterAssert(blog.managedObjectContext == self.managedObjectContext); - NSProgress *createProgress = [NSProgress discreteProgressWithTotalUnitCount:1]; - __block Media *media; - __block NSSet<NSString *> *allowedFileTypes = [NSSet set]; - [self.managedObjectContext performBlockAndWait:^{ - if ( blog == nil ) { - if (completion) { - NSError *error = [NSError errorWithDomain: MediaServiceErrorDomain code: MediaServiceErrorUnableToCreateMedia userInfo: nil]; - completion(nil, error); - } - return; - } - - if (blog.allowedFileTypes != nil) { - // HEIC isn't supported when uploading an image, so we filter it out (http://git.io/JJAae) - NSMutableSet *mutableAllowedFileTypes = [blog.allowedFileTypes mutableCopy]; - [mutableAllowedFileTypes removeObject:@"heic"]; - allowedFileTypes = mutableAllowedFileTypes; - } - - if (post != nil) { - media = [Media makeMediaWithPost:post]; - } else { - media = [Media makeMediaWithBlog:blog]; - } - media.mediaType = exportable.assetMediaType; - media.remoteStatus = MediaRemoteStatusProcessing; - - [self.managedObjectContext obtainPermanentIDsForObjects:@[media] error:nil]; - [[ContextManager sharedInstance] saveContextAndWait:self.managedObjectContext]; - }]; - if (media == nil) { - return nil; - } - NSManagedObjectID *mediaObjectID = media.objectID; - [self.managedObjectContext performBlock:^{ - // Setup completion handlers - void(^completionWithMedia)(Media *) = ^(Media *media) { - media.remoteStatus = MediaRemoteStatusLocal; - media.error = nil; - // Pre-generate a thumbnail image, see the method notes. - [self exportPlaceholderThumbnailForMedia:media - completion:^(NSURL *url){ - if (thumbnailCallback) { - thumbnailCallback(media, url); - } - }]; - if (completion) { - completion(media, nil); - } - }; - void(^completionWithError)( NSError *) = ^(NSError *error) { - Media *mediaInContext = (Media *)[self.managedObjectContext existingObjectWithID:mediaObjectID error:nil]; - if (mediaInContext) { - mediaInContext.error = error; - mediaInContext.remoteStatus = MediaRemoteStatusFailed; - [[ContextManager sharedInstance] saveContext:self.managedObjectContext]; - } - if (completion) { - completion(media, error); - } - }; - - // Export based on the type of the exportable. - MediaImportService *importService = [[MediaImportService alloc] initWithManagedObjectContext:self.managedObjectContext]; - importService.allowableFileExtensions = allowedFileTypes; - NSProgress *importProgress = [importService importResource:exportable toMedia:media onCompletion:completionWithMedia onError:completionWithError]; - [createProgress addChild:importProgress withPendingUnitCount:1]; - }]; - if (progress != nil) { - *progress = createProgress; - } - return media; -} - -/** - Generate a thumbnail image for the Media asset so that consumers of the absoluteThumbnailLocalURL property - will have an image ready to load, without using the async methods provided via MediaThumbnailService. - - This is primarily used as a placeholder image throughout the code-base, particulary within the editors. - - Note: Ideally we wouldn't need this at all, but the synchronous usage of absoluteThumbnailLocalURL across the code-base - to load a thumbnail image is relied on quite heavily. In the future, transitioning to asynchronous thumbnail loading - via the new thumbnail service methods is much preferred, but would indeed take a good bit of refactoring away from - using absoluteThumbnailLocalURL. -*/ -- (void)exportPlaceholderThumbnailForMedia:(Media *)media completion:(void (^)(NSURL *thumbnailURL))thumbnailCallback -{ - [self.thumbnailService thumbnailURLForMedia:media - preferredSize:CGSizeZero - onCompletion:^(NSURL *url) { - [self.managedObjectContext performBlock:^{ - if (url) { - // Set the absoluteThumbnailLocalURL with the generated thumbnail's URL. - media.absoluteThumbnailLocalURL = url; - [[ContextManager sharedInstance] saveContext:self.managedObjectContext]; - } - if (thumbnailCallback) { - thumbnailCallback(url); - } - }]; - } - onError:^(NSError *error) { - DDLogError(@"Error occurred exporting placeholder thumbnail: %@", error); - }]; -} - #pragma mark - Uploading media - (BOOL)isValidFileInMedia:(Media *)media error:(NSError **)error { @@ -674,19 +542,6 @@ - (void)getMediaLibraryServerCountForBlog:(Blog *)blog }]; } -#pragma mark - Thumbnails - -- (MediaThumbnailService *)thumbnailService -{ - if (!_thumbnailService) { - _thumbnailService = [[MediaThumbnailService alloc] initWithContextManager:[ContextManager sharedInstance]]; - if (self.concurrentThumbnailGeneration) { - _thumbnailService.exportQueue = dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0); - } - } - return _thumbnailService; -} - #pragma mark - Private helpers - (NSString *)mimeTypeForMediaType:(NSNumber *)mediaType diff --git a/WordPress/Classes/ViewRelated/Blog/Site Picker/SitePickerViewController+SiteIcon.swift b/WordPress/Classes/ViewRelated/Blog/Site Picker/SitePickerViewController+SiteIcon.swift index 2e33865e0d89..9a6556bb30de 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Picker/SitePickerViewController+SiteIcon.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Picker/SitePickerViewController+SiteIcon.swift @@ -142,12 +142,12 @@ extension SitePickerViewController { } func uploadDroppedSiteIcon(_ image: UIImage, completion: @escaping (() -> Void)) { - var creationProgress: Progress? - mediaService.createMedia( + let service = MediaImportService(coreDataStack: ContextManager.shared) + _ = service.createMedia( with: image, blog: blog, post: nil, - progress: &creationProgress, + receiveUpdate: nil, thumbnailCallback: nil, completion: { [weak self] media, error in guard let media = media, error == nil else { diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift index 8082b949011b..2df78633c172 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift @@ -100,15 +100,17 @@ class SiteIconPickerPresenter: NSObject { self.onCompletion?(media, nil) } else { let mediaService = MediaService(managedObjectContext: ContextManager.sharedInstance().mainContext) + let importService = MediaImportService(coreDataStack: ContextManager.sharedInstance()) WPAnalytics.track(.siteSettingsSiteIconCropped) - mediaService.createMedia(with: image, - blog: self.blog, - post: nil, - progress: nil, - thumbnailCallback: nil, - completion: { (media, error) in + importService.createMedia( + with: image, + blog: self.blog, + post: nil, + receiveUpdate: nil, + thumbnailCallback: nil + ) { (media, error) in guard let media = media, error == nil else { WPAnalytics.track(.siteSettingsSiteIconUploadFailed) self.onCompletion?(nil, error) @@ -125,7 +127,7 @@ class SiteIconPickerPresenter: NSObject { WPAnalytics.track(.siteSettingsSiteIconUploadFailed) self.onCompletion?(nil, error) }) - }) + } } } self.mediaPickerViewController.show(after: imageCropViewController) diff --git a/WordPress/Classes/ViewRelated/Media/MediaLibraryPickerDataSource.m b/WordPress/Classes/ViewRelated/Media/MediaLibraryPickerDataSource.m index 60d3df81664e..5f89dea12b9b 100644 --- a/WordPress/Classes/ViewRelated/Media/MediaLibraryPickerDataSource.m +++ b/WordPress/Classes/ViewRelated/Media/MediaLibraryPickerDataSource.m @@ -307,8 +307,8 @@ -(void)addMediaFromAssetIdentifier:(NSString *)assetIdentifier } PHFetchResult *result = [PHAsset fetchAssetsWithLocalIdentifiers:@[assetIdentifier] options:nil]; PHAsset *asset = [result firstObject]; - MediaService *mediaService = [[MediaService alloc] initWithManagedObjectContext:self.blog.managedObjectContext]; - [mediaService createMediaWith:asset blog:self.blog post: self.post progress:nil thumbnailCallback:nil completion:^(Media *media, NSError *error) { + MediaImportService *service = [[MediaImportService alloc] initWithContextManager:[ContextManager sharedInstance]]; + [service createMediaWith:asset blog:self.blog post: self.post receiveUpdate:nil thumbnailCallback:nil completion:^(Media *media, NSError *error) { [self loadDataWithOptions:WPMediaLoadOptionsAssets success:^{ completionBlock(media, error); } failure:^(NSError *error) { @@ -322,13 +322,13 @@ -(void)addMediaFromAssetIdentifier:(NSString *)assetIdentifier -(void)addMediaFromURL:(NSURL *)url completionBlock:(WPMediaAddedBlock)completionBlock { - MediaService *mediaService = [[MediaService alloc] initWithManagedObjectContext:self.blog.managedObjectContext]; - [mediaService createMediaWith:url - blog:self.blog - post:self.post - progress:nil - thumbnailCallback:nil - completion:^(Media *media, NSError *error) { + MediaImportService *service = [[MediaImportService alloc] initWithContextManager:[ContextManager sharedInstance]]; + [service createMediaWith:url + blog:self.blog + post:self.post + receiveUpdate:nil + thumbnailCallback:nil + completion:^(Media *media, NSError *error) { [self loadDataWithOptions:WPMediaLoadOptionsAssets success:^{ completionBlock(media, error); } failure:^(NSError *error) { diff --git a/WordPress/WordPressTest/MediaPicker/MediaLibraryPickerDataSourceTests.swift b/WordPress/WordPressTest/MediaPicker/MediaLibraryPickerDataSourceTests.swift index b9cbf1b4eb54..81e7bb0b7992 100644 --- a/WordPress/WordPressTest/MediaPicker/MediaLibraryPickerDataSourceTests.swift +++ b/WordPress/WordPressTest/MediaPicker/MediaLibraryPickerDataSourceTests.swift @@ -119,9 +119,9 @@ class MediaLibraryPickerDataSourceTests: CoreDataTestCase { return nil } - let mediaService = MediaService(managedObjectContext: mainContext) + let service = MediaImportService(coreDataStack: contextManager) let expect = self.expectation(description: "Media should be create with success") - mediaService.createMedia(with: url as NSURL, blog: blog, post: post, progress: nil, thumbnailCallback: nil, completion: { (media, error) in + _ = service.createMedia(with: url as NSURL, blog: blog, post: post, thumbnailCallback: nil, completion: { (media, error) in expect.fulfill() if let _ = error { XCTFail("Media should be created without error")