From b22f4c512faf4d919c5c487fa47a9ae493619aee Mon Sep 17 00:00:00 2001 From: Miltiadis Vasilakis Date: Thu, 22 Jun 2023 10:20:37 +0300 Subject: [PATCH] Handle lookup item & attachment creation in IdentifierLookupController --- Zotero.xcodeproj/project.pbxproj | 4 + Zotero/Controllers/Controllers.swift | 8 ++ .../IdentifierLookupController.swift | 118 ++++++++++++++++++ .../Detail/Lookup/LookupCoordinator.swift | 8 +- .../ViewModels/LookupActionHandler.swift | 92 ++++---------- .../Lookup/Views/LookupViewController.swift | 4 +- 6 files changed, 164 insertions(+), 70 deletions(-) create mode 100644 Zotero/Controllers/IdentifierLookupController.swift diff --git a/Zotero.xcodeproj/project.pbxproj b/Zotero.xcodeproj/project.pbxproj index 74530cfc2..d84b56f16 100644 --- a/Zotero.xcodeproj/project.pbxproj +++ b/Zotero.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 618404262A4456A9005AAF22 /* IdentifierLookupController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */; }; B300B33324291C8D00C1FE1E /* RTranslatorMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = B300B33224291C8D00C1FE1E /* RTranslatorMetadata.swift */; }; B300B3352429222B00C1FE1E /* TranslatorMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = B300B3342429222B00C1FE1E /* TranslatorMetadata.swift */; }; B300B3362429234C00C1FE1E /* TranslatorMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = B300B3342429222B00C1FE1E /* TranslatorMetadata.swift */; }; @@ -1176,6 +1177,7 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdentifierLookupController.swift; sourceTree = ""; }; B300B33224291C8D00C1FE1E /* RTranslatorMetadata.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RTranslatorMetadata.swift; sourceTree = ""; }; B300B3342429222B00C1FE1E /* TranslatorMetadata.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TranslatorMetadata.swift; sourceTree = ""; }; B300B3372429254900C1FE1E /* SyncTranslatorsDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncTranslatorsDbRequest.swift; sourceTree = ""; }; @@ -2122,6 +2124,7 @@ B36A988C2428E059005D5790 /* TranslatorsAndStylesController.swift */, B3972688247D403200A8B469 /* UrlDetector.swift */, B324276C25C81F2000567504 /* WebSocketController.swift */, + 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */, ); path = Controllers; sourceTree = ""; @@ -5019,6 +5022,7 @@ B340ECA6290FDC9F00EE920D /* AnnotationToolbarViewController.swift in Sources */, B3401D572567D8F700BB8D6E /* AnnotationViewController.swift in Sources */, B3830CE2255451C200910FE0 /* TagPickerActionHandler.swift in Sources */, + 618404262A4456A9005AAF22 /* IdentifierLookupController.swift in Sources */, B305662223FC051F003304F2 /* StoreVersionSyncAction.swift in Sources */, B3BC25CD247E6BA000AC27B5 /* DateParser.swift in Sources */, B305662523FC051F003304F2 /* RevertLibraryUpdatesSyncAction.swift in Sources */, diff --git a/Zotero/Controllers/Controllers.swift b/Zotero/Controllers/Controllers.swift index 207ffa8ed..471b5bd3a 100644 --- a/Zotero/Controllers/Controllers.swift +++ b/Zotero/Controllers/Controllers.swift @@ -296,6 +296,7 @@ final class UserControllers { let backgroundUploadObserver: BackgroundUploadObserver let fileDownloader: AttachmentDownloader let remoteFileDownloader: RemoteAttachmentDownloader + let identifierLookupController: IdentifierLookupController let webSocketController: WebSocketController let fileCleanupController: AttachmentFileCleanupController let citationController: CitationController @@ -363,6 +364,13 @@ final class UserControllers { self.backgroundUploadObserver = backgroundUploadObserver self.fileDownloader = fileDownloader self.remoteFileDownloader = RemoteAttachmentDownloader(apiClient: controllers.apiClient, fileStorage: controllers.fileStorage) + self.identifierLookupController = IdentifierLookupController( + dbStorage: dbStorage, + fileStorage: controllers.fileStorage, + schemaController: controllers.schemaController, + dateParser: controllers.dateParser, + remoteFileDownloader: remoteFileDownloader + ) self.webSocketController = webSocketController self.fileCleanupController = fileCleanupController self.citationController = CitationController(stylesController: controllers.translatorsAndStylesController, fileStorage: controllers.fileStorage, diff --git a/Zotero/Controllers/IdentifierLookupController.swift b/Zotero/Controllers/IdentifierLookupController.swift new file mode 100644 index 000000000..ad4dd9ff3 --- /dev/null +++ b/Zotero/Controllers/IdentifierLookupController.swift @@ -0,0 +1,118 @@ +// +// IdentifierLookupController.swift +// Zotero +// +// Created by Miltiadis Vasilakis on 22/6/23. +// Copyright © 2023 Corporation for Digital Scholarship. All rights reserved. +// + +import Foundation + +import CocoaLumberjackSwift +import RxSwift + +final class IdentifierLookupController: BackgroundDbProcessingActionHandler { + // MARK: Types + struct Update { + enum Kind: Hashable { + case itemStored + case pendingAttachments + case itemCreationFailed + } + + let identifier: String + let response: ItemResponse + let attachments: [(Attachment, URL)] + let kind: Kind + } + + // MARK: Properties + let observable: PublishSubject + internal let backgroundQueue: DispatchQueue + internal unowned let dbStorage: DbStorage + private unowned let fileStorage: FileStorage + private unowned let schemaController: SchemaController + private unowned let dateParser: DateParser + private unowned let remoteFileDownloader: RemoteAttachmentDownloader + private let disposeBag: DisposeBag + + // MARK: Object Lifecycle + init(dbStorage: DbStorage, fileStorage: FileStorage, schemaController: SchemaController, dateParser: DateParser, remoteFileDownloader: RemoteAttachmentDownloader) { + self.fileStorage = fileStorage + self.dbStorage = dbStorage + self.schemaController = schemaController + self.dateParser = dateParser + self.remoteFileDownloader = remoteFileDownloader + + self.backgroundQueue = DispatchQueue(label: "org.zotero.IdentifierLookupController.backgroundProcessing", qos: .userInitiated) + self.observable = PublishSubject() + self.disposeBag = DisposeBag() + + remoteFileDownloader.observable + .subscribe { [weak self] update in + guard let self else { return } + switch update.kind { + case .ready(let attachment): + self.finish(download: update.download, attachment: attachment) + + case .cancelled, .failed, .progress: + break + } + } + .disposed(by: self.disposeBag) + } + + // MARK: Actions + func process(identifier: String, response: ItemResponse, attachments: [(Attachment, URL)]) { + backgroundQueue.async { [weak self] in + guard let self = self else { return } + do { + try self.storeDataAndDownloadAttachmentIfNecessary(identifier: identifier, response: response, attachments: attachments) + } catch let error { + DDLogError("IdentifierLookupController: can't create item(s) - \(error)") + observable.on(.next(Update(identifier: identifier, response: response, attachments: attachments, kind: .itemCreationFailed))) + } + } + } + + // MARK: Helper Methods + private func storeDataAndDownloadAttachmentIfNecessary(identifier: String, response: ItemResponse, attachments: [(Attachment, URL)]) throws { + let request = CreateTranslatedItemsDbRequest(responses: [response], schemaController: schemaController, dateParser: dateParser) + try dbStorage.perform(request: request, on: backgroundQueue) + observable.on(.next(Update(identifier: identifier, response: response, attachments: attachments, kind: .itemStored))) + + guard Defaults.shared.shareExtensionIncludeAttachment else { return } + + let downloadData = attachments.map({ ($0, $1, response.key) }) + guard !downloadData.isEmpty else { return } + remoteFileDownloader.download(data: downloadData) + observable.on(.next(Update(identifier: identifier, response: response, attachments: attachments, kind: .pendingAttachments))) + } + + private func finish(download: RemoteAttachmentDownloader.Download, attachment: Attachment) { + let localizedType = schemaController.localized(itemType: ItemTypes.attachment) ?? ItemTypes.attachment + + backgroundQueue.async { [weak self] in + guard let self else { return } + + do { + let request = CreateAttachmentDbRequest( + attachment: attachment, + parentKey: download.parentKey, + localizedType: localizedType, + includeAccessDate: attachment.hasUrl, + collections: [], + tags: [] + ) + _ = try self.dbStorage.perform(request: request, on: self.backgroundQueue) + } catch let error { + DDLogError("IdentifierLookupController: can't store attachment after download - \(error)") + + // Storing item failed, remove downloaded file + guard case .file(let filename, let contentType, _, _) = attachment.type else { return } + let file = Files.attachmentFile(in: attachment.libraryId, key: attachment.key, filename: filename, contentType: contentType) + try? self.fileStorage.remove(file) + } + } + } +} diff --git a/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift b/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift index ec2d67c76..31305365a 100644 --- a/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift +++ b/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift @@ -50,8 +50,12 @@ final class LookupCoordinator: NSObject, Coordinator { private func lookupController(multiLookupEnabled: Bool, hasDarkBackground: Bool, userControllers: UserControllers) -> LookupViewController { let collectionKeys = Defaults.shared.selectedCollectionId.key.flatMap({ Set([$0]) }) ?? [] let state = LookupState(multiLookupEnabled: multiLookupEnabled, hasDarkBackground: hasDarkBackground, collectionKeys: collectionKeys, libraryId: Defaults.shared.selectedLibrary) - let handler = LookupActionHandler(dbStorage: userControllers.dbStorage, fileStorage: self.controllers.fileStorage, translatorsController: self.controllers.translatorsAndStylesController, - schemaController: self.controllers.schemaController, dateParser: self.controllers.dateParser, remoteFileDownloader: userControllers.remoteFileDownloader) + let handler = LookupActionHandler( + identifierLookupController: controllers.userControllers!.identifierLookupController, + translatorsController: controllers.translatorsAndStylesController, + schemaController: controllers.schemaController, + dateParser: controllers.dateParser + ) let viewModel = ViewModel(initialState: state, handler: handler) return LookupViewController(viewModel: viewModel, remoteDownloadObserver: userControllers.remoteFileDownloader.observable, remoteFileDownloader: userControllers.remoteFileDownloader, diff --git a/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift b/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift index b869dce0c..d777e9948 100644 --- a/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift +++ b/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift @@ -11,29 +11,23 @@ import Foundation import CocoaLumberjackSwift import RxSwift -final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingActionHandler { +final class LookupActionHandler: ViewModelActionHandler { typealias State = LookupState typealias Action = LookupAction - unowned let dbStorage: DbStorage - let backgroundQueue: DispatchQueue - private unowned let fileStorage: FileStorage + private unowned let identifierLookupController: IdentifierLookupController private unowned let translatorsController: TranslatorsAndStylesController private unowned let schemaController: SchemaController private unowned let dateParser: DateParser - private unowned let remoteFileDownloader: RemoteAttachmentDownloader private let disposeBag: DisposeBag private var lookupWebViewHandler: LookupWebViewHandler? - init(dbStorage: DbStorage, fileStorage: FileStorage, translatorsController: TranslatorsAndStylesController, schemaController: SchemaController, dateParser: DateParser, remoteFileDownloader: RemoteAttachmentDownloader) { - self.backgroundQueue = DispatchQueue(label: "org.zotero.ItemsActionHandler.backgroundProcessing", qos: .userInitiated) - self.fileStorage = fileStorage - self.dbStorage = dbStorage + init(identifierLookupController: IdentifierLookupController, translatorsController: TranslatorsAndStylesController, schemaController: SchemaController, dateParser: DateParser) { + self.identifierLookupController = identifierLookupController self.translatorsController = translatorsController self.schemaController = schemaController self.dateParser = dateParser - self.remoteFileDownloader = remoteFileDownloader self.disposeBag = DisposeBag() } @@ -50,15 +44,23 @@ final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingA }) .disposed(by: self.disposeBag) - self.remoteFileDownloader.observable + identifierLookupController.observable .observe(on: MainScheduler.instance) - .subscribe(with: viewModel, onNext: { [weak self] viewModel, update in + .subscribe(with: viewModel) { [weak self] viewModel, update in + guard let self else { return } switch update.kind { - case .ready(let attachment): - self?.finish(download: update.download, attachment: attachment, in: viewModel) - case .cancelled, .failed, .progress: break + case .itemStored: + break + + case .pendingAttachments: + let translatedData = LookupState.LookupData(identifier: update.identifier, state: .translated(update.parsedData)) + self.update(lookupData: translatedData, in: viewModel) + + case .itemCreationFailed: + let failedData = LookupState.LookupData(identifier: update.identifier, state: .failed) + self.update(lookupData: failedData, in: viewModel) } - }) + } .disposed(by: self.disposeBag) case .lookUp(let identifier): @@ -66,26 +68,6 @@ final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingA } } - private func finish(download: RemoteAttachmentDownloader.Download, attachment: Attachment, in viewModel: ViewModel) { - let localizedType = self.schemaController.localized(itemType: ItemTypes.attachment) ?? ItemTypes.attachment - - self.backgroundQueue.async { [weak self] in - guard let self = self else { return } - - do { - let request = CreateAttachmentDbRequest(attachment: attachment, parentKey: download.parentKey, localizedType: localizedType, includeAccessDate: attachment.hasUrl, collections: [], tags: []) - _ = try self.dbStorage.perform(request: request, on: self.backgroundQueue) - } catch let error { - DDLogError("RemoteAttachmentDownloader: can't store attachment after download - \(error)") - - // Storing item failed, remove downloaded file - guard case .file(let filename, let contentType, _, _) = attachment.type else { return } - let file = Files.attachmentFile(in: attachment.libraryId, key: attachment.key, filename: filename, contentType: contentType) - try? self.fileStorage.remove(file) - } - } - } - private func process(result: Result, in viewModel: ViewModel) { switch result { case .success(let data): @@ -161,27 +143,7 @@ final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingA return } - self.backgroundQueue.async { [weak self, weak viewModel] in - guard let self = self else { return } - - do { - try self.storeDataAndDownloadAttachmentIfNecessary(parsedData) - - inMainThread { [weak self] in - guard let self = self, let viewModel = viewModel else { return } - let translatedData = LookupState.LookupData(identifier: identifier, state: .translated(parsedData)) - self.update(lookupData: translatedData, in: viewModel) - } - } catch let error { - DDLogError("LookupActionHandler: can't create item(s) - \(error)") - - inMainThread { [weak self] in - guard let self = self, let viewModel = viewModel else { return } - let failedData = LookupState.LookupData(identifier: identifier, state: .failed) - self.update(lookupData: failedData, in: viewModel) - } - } - } + identifierLookupController.process(identifier: identifier, response: parsedData.response, attachments: parsedData.attachments) } } @@ -207,16 +169,6 @@ final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingA return result } - private func storeDataAndDownloadAttachmentIfNecessary(_ data: LookupState.TranslatedLookupData) throws { - let request = CreateTranslatedItemsDbRequest(responses: [data.response], schemaController: self.schemaController, dateParser: self.dateParser) - try self.dbStorage.perform(request: request, on: self.backgroundQueue) - - guard Defaults.shared.shareExtensionIncludeAttachment else { return } - - let downloadData = data.attachments.map({ ($0, $1, data.response.key) }) - self.remoteFileDownloader.download(data: downloadData) - } - /// Tries to parse `ItemResponse` from data returned by translation server. It prioritizes items with attachments if there are multiple items. /// - parameter data: Data to parse /// - parameter schemaController: SchemaController which is used for validating item type and field types @@ -247,3 +199,9 @@ final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingA } } } + +extension IdentifierLookupController.Update { + var parsedData: LookupState.TranslatedLookupData { + .init(response: response, attachments: attachments) + } +} diff --git a/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift b/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift index e4bdec218..1a790f892 100644 --- a/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift +++ b/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift @@ -216,7 +216,9 @@ class LookupViewController: UIViewController { } private func closeAfterUpdateIfNeeded() { - let activeDownload = self.dataSource.snapshot().itemIdentifiers.first(where: { row in + let itemIdentifiers = dataSource.snapshot().itemIdentifiers + guard !itemIdentifiers.isEmpty else { return } + let activeDownload = itemIdentifiers.first(where: { row in switch row { case .attachment(_, let update): switch update {