diff --git a/Zotero.xcodeproj/project.pbxproj b/Zotero.xcodeproj/project.pbxproj index 1e71d00bb..ea797b6e0 100644 --- a/Zotero.xcodeproj/project.pbxproj +++ b/Zotero.xcodeproj/project.pbxproj @@ -22,6 +22,8 @@ 6144B5E12A4AE95E00914B3C /* WebDavControllerSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3202C6B271048FF00485BE4 /* WebDavControllerSpec.swift */; }; 614D65822A79C9AC007CF449 /* Localizable.stringsdict in Resources */ = {isa = PBXBuildFile; fileRef = 614D65802A79C9AC007CF449 /* Localizable.stringsdict */; }; 614D65832A79C9B0007CF449 /* Localizable.stringsdict in Resources */ = {isa = PBXBuildFile; fileRef = 614D65802A79C9AC007CF449 /* Localizable.stringsdict */; }; + 614D65872A8030C9007CF449 /* OrderedCollections in Frameworks */ = {isa = PBXBuildFile; productRef = 614D65862A8030C9007CF449 /* OrderedCollections */; }; + 618404262A4456A9005AAF22 /* IdentifierLookupController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */; }; 61ABA7512A6137D1002A4219 /* ShareableImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61ABA7502A6137D1002A4219 /* ShareableImage.swift */; }; 61BD13952A5831EF008A0704 /* TextKit1TextView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61BD13942A5831EF008A0704 /* TextKit1TextView.swift */; }; 61C817F22A49B5D30085B1E6 /* CollectionResponseSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B1EDEE250242E700D8BC1E /* CollectionResponseSpec.swift */; }; @@ -1200,6 +1202,7 @@ 611486502A9CD511002EEBEF /* ci_post_xcodebuild.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = ci_post_xcodebuild.sh; sourceTree = ""; }; 614D65812A79C9AC007CF449 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = en; path = en.lproj/Localizable.stringsdict; sourceTree = ""; }; 614D65842A7BCC22007CF449 /* ci_post_clone.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = ci_post_clone.sh; sourceTree = ""; }; + 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdentifierLookupController.swift; sourceTree = ""; }; 61ABA7502A6137D1002A4219 /* ShareableImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareableImage.swift; sourceTree = ""; }; 61BD13942A5831EF008A0704 /* TextKit1TextView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextKit1TextView.swift; sourceTree = ""; }; 61FA14CD2B05081D00E7D423 /* TextConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextConverter.swift; sourceTree = ""; }; @@ -2034,6 +2037,7 @@ B356A39E2524A703003F1943 /* CocoaLumberjackSwift in Frameworks */, B356A363252490B3003F1943 /* Alamofire in Frameworks */, B356A369252490DB003F1943 /* KeychainSwift in Frameworks */, + 614D65872A8030C9007CF449 /* OrderedCollections in Frameworks */, B356A37B2524A63D003F1943 /* SwiftyGif in Frameworks */, B356A3A02524A703003F1943 /* CocoaLumberjack in Frameworks */, B35C9C082642A0BC0004C47E /* RealmSwift in Frameworks */, @@ -2165,6 +2169,7 @@ B36A988C2428E059005D5790 /* TranslatorsAndStylesController.swift */, B3972688247D403200A8B469 /* UrlDetector.swift */, B324276C25C81F2000567504 /* WebSocketController.swift */, + 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */, ); path = Controllers; sourceTree = ""; @@ -4051,6 +4056,7 @@ B35C9C072642A0BC0004C47E /* RealmSwift */, B3445F4526EF5EE9007D4009 /* CrashReporter */, B3D84BEF27919FDE005DDD7C /* Starscream */, + 614D65862A8030C9007CF449 /* OrderedCollections */, ); productName = Zotero; productReference = B30D59552206F60400884C4A /* Zotero.app */; @@ -4192,6 +4198,7 @@ B35C9C042642A0BC0004C47E /* XCRemoteSwiftPackageReference "realm-cocoa" */, B3445F4426EF5EE9007D4009 /* XCRemoteSwiftPackageReference "plcrashreporter" */, B3D84BEE27919FDE005DDD7C /* XCRemoteSwiftPackageReference "Starscream" */, + 614D65852A8030C9007CF449 /* XCRemoteSwiftPackageReference "swift-collections" */, ); productRefGroup = B30D59562206F60400884C4A /* Products */; projectDirPath = ""; @@ -5091,6 +5098,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 */, @@ -5999,6 +6007,14 @@ /* End XCConfigurationList section */ /* Begin XCRemoteSwiftPackageReference section */ + 614D65852A8030C9007CF449 /* XCRemoteSwiftPackageReference "swift-collections" */ = { + isa = XCRemoteSwiftPackageReference; + repositoryURL = "https://github.com/apple/swift-collections.git"; + requirement = { + kind = exactVersion; + version = 1.0.4; + }; + }; B3445F4426EF5EE9007D4009 /* XCRemoteSwiftPackageReference "plcrashreporter" */ = { isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/microsoft/plcrashreporter"; @@ -6114,6 +6130,11 @@ /* End XCRemoteSwiftPackageReference section */ /* Begin XCSwiftPackageProductDependency section */ + 614D65862A8030C9007CF449 /* OrderedCollections */ = { + isa = XCSwiftPackageProductDependency; + package = 614D65852A8030C9007CF449 /* XCRemoteSwiftPackageReference "swift-collections" */; + productName = OrderedCollections; + }; B3445F4526EF5EE9007D4009 /* CrashReporter */ = { isa = XCSwiftPackageProductDependency; package = B3445F4426EF5EE9007D4009 /* XCRemoteSwiftPackageReference "plcrashreporter" */; diff --git a/Zotero.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Zotero.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 695d9da63..3322c3878 100644 --- a/Zotero.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Zotero.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -135,6 +135,15 @@ "version" : "4.0.6" } }, + { + "identity" : "swift-collections", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-collections.git", + "state" : { + "revision" : "937e904258d22af6e447a0b72c0bc67583ef64a2", + "version" : "1.0.4" + } + }, { "identity" : "swift-log", "kind" : "remoteSourceControl", diff --git a/Zotero/Assets/en.lproj/Localizable.strings b/Zotero/Assets/en.lproj/Localizable.strings index fc3c75fb9..dd14c1e34 100644 --- a/Zotero/Assets/en.lproj/Localizable.strings +++ b/Zotero/Assets/en.lproj/Localizable.strings @@ -53,6 +53,7 @@ "size" = "Size"; "remove" = "Remove"; "keep" = "Keep"; +"cancel_all" = "Cancel All"; "beta_wipe_title" = "Resync Required"; "beta_wipe_message" = "Due to a beta update, your data must be redownloaded from zotero.org."; @@ -134,6 +135,7 @@ "items.filters.title" = "Filters"; "items.filters.downloads" = "Downloaded Files"; "items.filters.tags" = "Tags"; +"items.toolbar_saved" = "Saved %d / %d"; "items.toolbar_downloaded" = "Downloaded %d / %d"; "items.generating_bib" = "Generating Bibliography"; "items.creator_summary.and" = "%@ and %@"; @@ -470,7 +472,8 @@ "errors.citation.invalid_types" = "Invalid item types selected."; "errors.citation.missing_style" = "No citation style selected. Go to Settings → Quick Copy and select a new style."; "errors.citation.open_settings" = "Open Settings"; -"errors.lookup" = "Zotero could not find any identifiers in your input. Please verify your input and try again."; +"errors.lookup.no_identifiers_and_no_lookup_data" = "Zotero could not find any identifiers in your input. Please verify your input and try again."; +"errors.lookup.no_identifiers_with_lookup_data" = "Zotero could not find any new identifiers in your input, or they are already being added. Please verify your input and try again."; "errors.pdf.merge_too_big_title" = "Unable to merge annotations"; "errors.pdf.merge_too_big" = "The combined annotation would be too large."; "errors.pdf.cant_update_annotation" = "Can't update annotation."; diff --git a/Zotero/Controllers/Attachment Downloader/AttachmentDownloadOperation.swift b/Zotero/Controllers/Attachment Downloader/AttachmentDownloadOperation.swift index 1ce1ec673..9c6e78ffd 100644 --- a/Zotero/Controllers/Attachment Downloader/AttachmentDownloadOperation.swift +++ b/Zotero/Controllers/Attachment Downloader/AttachmentDownloadOperation.swift @@ -37,7 +37,16 @@ class AttachmentDownloadOperation: AsynchronousOperation { var finishedDownload: ((Result<(), Swift.Error>) -> Void)? - init(file: File, download: AttachmentDownloader.Download, progress: Progress, userId: Int, apiClient: ApiClient, webDavController: WebDavController, fileStorage: FileStorage, queue: DispatchQueue) { + init( + file: File, + download: AttachmentDownloader.Download, + progress: Progress, + userId: Int, + apiClient: ApiClient, + webDavController: WebDavController, + fileStorage: FileStorage, + queue: DispatchQueue + ) { self.file = file self.download = download self.progress = progress @@ -231,7 +240,9 @@ class AttachmentDownloadOperation: AsynchronousOperation { self.zipProgress = nil // Try removing downloaded file. try? self.fileStorage.remove(Files.attachmentDirectory(in: self.download.libraryId, key: self.download.key)) - case .done: break + + case .done: + break } self.finishedDownload?(.failure(Error.cancelled)) diff --git a/Zotero/Controllers/Attachment Downloader/AttachmentDownloader.swift b/Zotero/Controllers/Attachment Downloader/AttachmentDownloader.swift index b45a76fe0..b2eec81e6 100644 --- a/Zotero/Controllers/Attachment Downloader/AttachmentDownloader.swift +++ b/Zotero/Controllers/Attachment Downloader/AttachmentDownloader.swift @@ -123,11 +123,13 @@ final class AttachmentDownloader { case .file(let filename, let contentType, let location, let linkType): switch linkType { - case .linkedFile, .embeddedImage: break + case .linkedFile, .embeddedImage: + break case .importedFile, .importedUrl: switch location { - case .local: break + case .local: + break case .remote, .remoteMissing, .localAndChangedRemotely: DDLogInfo("AttachmentDownloader: batch download remote\(location == .remoteMissing ? "ly missing" : "") file \(attachment.key)") @@ -269,8 +271,16 @@ final class AttachmentDownloader { let observer = progress.observe(\.fractionCompleted) { [weak self] progress, _ in self?.observable.on(.next(Update(download: download, parentKey: parentKey, kind: .progress(CGFloat(progress.fractionCompleted))))) } - let operation = AttachmentDownloadOperation(file: file, download: download, progress: progress, userId: self.userId, apiClient: self.apiClient, webDavController: self.webDavController, - fileStorage: self.fileStorage, queue: self.processingQueue) + let operation = AttachmentDownloadOperation( + file: file, + download: download, + progress: progress, + userId: userId, + apiClient: apiClient, + webDavController: webDavController, + fileStorage: fileStorage, + queue: processingQueue + ) operation.finishedDownload = { [weak self] result in guard let self = self else { return } diff --git a/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloadOperation.swift b/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloadOperation.swift index 02c28ce06..0f3c71e59 100644 --- a/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloadOperation.swift +++ b/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloadOperation.swift @@ -24,6 +24,7 @@ class RemoteAttachmentDownloadOperation: AsynchronousOperation { private let url: URL private let file: File + let progress: Progress? private let queue: DispatchQueue private unowned let apiClient: ApiClient private unowned let fileStorage: FileStorage @@ -31,14 +32,14 @@ class RemoteAttachmentDownloadOperation: AsynchronousOperation { private var request: DownloadRequest? private var state: State? private var disposeBag: DisposeBag? - private(set) var progress: Progress? var progressHandler: ((Progress) -> Void)? var finishedDownload: ((Result<(), Swift.Error>) -> Void)? - init(url: URL, file: File, apiClient: ApiClient, fileStorage: FileStorage, queue: DispatchQueue) { + init(url: URL, file: File, progress: Progress, apiClient: ApiClient, fileStorage: FileStorage, queue: DispatchQueue) { self.url = url self.file = file + self.progress = progress self.apiClient = apiClient self.fileStorage = fileStorage self.queue = queue @@ -66,7 +67,7 @@ class RemoteAttachmentDownloadOperation: AsynchronousOperation { self.apiClient.download(request: request, queue: self.queue) .subscribe(onNext: { [weak self] request in // Store download request so that it can be cancelled - self?.progress = request.downloadProgress + self?.progress?.addChild(request.downloadProgress, withPendingUnitCount: 100) self?.progressHandler?(request.downloadProgress) self?.request = request // Start request @@ -137,7 +138,9 @@ class RemoteAttachmentDownloadOperation: AsynchronousOperation { self.request?.cancel() self.disposeBag = nil self.request = nil - case .done: break + + case .done: + break } self.finishedDownload?(.failure(Error.cancelled)) diff --git a/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloader.swift b/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloader.swift index 08fdb2752..3c05205fc 100644 --- a/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloader.swift +++ b/Zotero/Controllers/Attachment Downloader/RemoteAttachmentDownloader.swift @@ -30,7 +30,8 @@ final class RemoteAttachmentDownloader { let kind: Kind } - private let queue: DispatchQueue + private let accessQueue: DispatchQueue + private let processingQueue: DispatchQueue private let operationQueue: OperationQueue private let disposeBag: DisposeBag private unowned let apiClient: ApiClient @@ -41,18 +42,36 @@ final class RemoteAttachmentDownloader { private var operations: [Download: RemoteAttachmentDownloadOperation] private var errors: [Download: Swift.Error] private var progressObservers: [Download: NSKeyValueObservation] + private var batchProgress: Progress? + private var totalBatchCount: Int = 0 + + var batchData: (Progress?, Int, Int) { + var progress: Progress? + var totalBatchCount = 0 + var remainingBatchCount = 0 + + self.accessQueue.sync { [weak self] in + guard let self else { return } + progress = self.batchProgress + remainingBatchCount = self.operations.count + totalBatchCount = self.totalBatchCount + } + + return (progress, remainingBatchCount, totalBatchCount) + } init(apiClient: ApiClient, fileStorage: FileStorage) { - let queue = DispatchQueue(label: "org.zotero.RemoteAttachmentDownloader.ProcessingQueue", qos: .userInteractive, attributes: .concurrent) + let processingQueue = DispatchQueue(label: "org.zotero.RemoteAttachmentDownloader.ProcessingQueue", qos: .userInteractive, attributes: .concurrent) let operationQueue = OperationQueue() operationQueue.maxConcurrentOperationCount = 2 operationQueue.qualityOfService = .userInitiated - operationQueue.underlyingQueue = queue + operationQueue.underlyingQueue = processingQueue self.apiClient = apiClient self.fileStorage = fileStorage self.operationQueue = operationQueue - self.queue = queue + self.accessQueue = DispatchQueue(label: "org.zotero.RemoteAttachmentDownloader.AccessQueue", qos: .userInteractive, attributes: .concurrent) + self.processingQueue = processingQueue self.observable = PublishSubject() self.operations = [:] self.progressObservers = [:] @@ -65,7 +84,7 @@ final class RemoteAttachmentDownloader { var progress: CGFloat? var error: Swift.Error? - self.queue.sync { [weak self] in + self.accessQueue.sync { [weak self] in if let operation = self?.operations[download] { if let _progress = operation.progress { progress = CGFloat(_progress.fractionCompleted) @@ -80,77 +99,105 @@ final class RemoteAttachmentDownloader { } func download(data: [(Attachment, URL, String)]) { - self.queue.async(flags: .barrier) { [weak self] in - guard let self = self else { return } + self.accessQueue.async(flags: .barrier) { [weak self] in + guard let self else { return } DDLogInfo("RemoteAttachmentDownloader: enqueue \(data.count) attachments") - let operations = data.compactMap({ self.createDownload(url: $0.1, attachment: $0.0, parentKey: $0.2) }) - self.operationQueue.addOperations(operations, waitUntilFinished: false) + let downloadAndOperations = data.compactMap({ createDownload(url: $0.1, attachment: $0.0, parentKey: $0.2) }) + let downloads = downloadAndOperations.map({ $0.0 }) + let operations = downloadAndOperations.map({ $0.1 }) + downloads.forEach { + // Send first update to immediately reflect new state + self.observable.on(.next(Update(download: $0, kind: .progress(0)))) + } + operationQueue.addOperations(operations, waitUntilFinished: false) } - } - - private func createDownload(url: URL, attachment: Attachment, parentKey: String) -> RemoteAttachmentDownloadOperation? { - let download = Download(key: attachment.key, parentKey: parentKey, libraryId: attachment.libraryId) + + func createDownload(url: URL, attachment: Attachment, parentKey: String) -> (Download, RemoteAttachmentDownloadOperation)? { + let download = Download(key: attachment.key, parentKey: parentKey, libraryId: attachment.libraryId) - guard self.operations[download] == nil, let file = self.file(for: attachment) else { return nil } + guard operations[download] == nil, let file = file(for: attachment) else { return nil } - let operation = RemoteAttachmentDownloadOperation(url: url, file: file, apiClient: self.apiClient, fileStorage: self.fileStorage, queue: self.queue) - operation.finishedDownload = { [weak self] result in - self?.queue.async(flags: .barrier) { - guard let self = self else { return } - self.finish(download: download, file: file, attachment: attachment, parentKey: parentKey, result: result) + let progress = Progress(totalUnitCount: 100) + let operation = RemoteAttachmentDownloadOperation(url: url, file: file, progress: progress, apiClient: apiClient, fileStorage: fileStorage, queue: processingQueue) + operation.finishedDownload = { [weak self] result in + self?.accessQueue.async(flags: .barrier) { + finish(download: download, file: file, attachment: attachment, parentKey: parentKey, result: result) + } } - } - operation.progressHandler = { [weak self] progress in - self?.queue.async(flags: .barrier) { - guard let self = self else { return } - self.observe(progress: progress, attachment: attachment, download: download) + operation.progressHandler = { [weak self] progress in + self?.accessQueue.async(flags: .barrier) { + observe(progress: progress, attachment: attachment, download: download) + } } - } - self.operations[download] = operation + operations[download] = operation + totalBatchCount += 1 + + if let batchProgress { + batchProgress.addChild(progress, withPendingUnitCount: 100) + batchProgress.totalUnitCount += 100 + } else { + let batchProgress = Progress(totalUnitCount: 100) + batchProgress.addChild(progress, withPendingUnitCount: 100) + self.batchProgress = batchProgress + } - return operation - } + return (download, operation) + + func file(for attachment: Attachment) -> File? { + switch attachment.type { + case .file(let filename, let contentType, _, _): + return Files.attachmentFile(in: attachment.libraryId, key: attachment.key, filename: filename, contentType: contentType) - private func observe(progress: Progress, attachment: Attachment, download: Download) { - let observer = progress.observe(\.fractionCompleted) { [weak self] progress, _ in - self?.observable.on(.next(Update(download: download, kind: .progress(CGFloat(progress.fractionCompleted))))) + case .url: + return nil + } + } + + func finish(download: Download, file: File, attachment: Attachment, parentKey: String, result: Result<(), Swift.Error>) { + operations[download] = nil + progressObservers[download] = nil + resetBatchDataIfNeeded() + + switch result { + case .success: + DDLogInfo("RemoteAttachmentDownloader: finished downloading \(download.key)") + observable.on(.next(Update(download: download, kind: .ready(attachment)))) + errors[download] = nil + + case .failure(let error): + DDLogError("RemoteAttachmentDownloader: failed to download attachment \(download.key), \(download.libraryId) - \(error)") + + let isCancelError = (error as? RemoteAttachmentDownloadOperation.Error) == .cancelled + errors[download] = isCancelError ? nil : error + + if isCancelError { + observable.on(.next(Update(download: download, kind: .cancelled))) + } else { + observable.on(.next(Update(download: download, kind: .failed))) + } + } + } + + func observe(progress: Progress, attachment: Attachment, download: Download) { + let observer = progress.observe(\.fractionCompleted) { [weak self] progress, _ in + self?.observable.on(.next(Update(download: download, kind: .progress(CGFloat(progress.fractionCompleted))))) + } + progressObservers[download] = observer + } } - self.progressObservers[download] = observer } - - private func file(for attachment: Attachment) -> File? { - switch attachment.type { - case .file(let filename, let contentType, _, _): - return Files.attachmentFile(in: attachment.libraryId, key: attachment.key, filename: filename, contentType: contentType) - - case .url: - return nil - } + + private func resetBatchDataIfNeeded() { + guard operations.isEmpty else { return } + totalBatchCount = 0 + batchProgress = nil } - private func finish(download: Download, file: File, attachment: Attachment, parentKey: String, result: Result<(), Swift.Error>) { - self.operations[download] = nil - - switch result { - case .success: - DDLogInfo("RemoteAttachmentDownloader: finished downloading \(download.key)") - self.observable.on(.next(Update(download: download, kind: .ready(attachment)))) - self.errors[download] = nil - - case .failure(let error): - DDLogError("RemoteAttachmentDownloader: failed to download attachment \(download.key), \(download.libraryId) - \(error)") - - let isCancelError = (error as? AttachmentDownloadOperation.Error) == .cancelled - self.errors[download] = isCancelError ? nil : error - - if isCancelError { - self.observable.on(.next(Update(download: download, kind: .cancelled))) - } else { - self.observable.on(.next(Update(download: download, kind: .failed))) - } - } + func stop() { + DDLogInfo("RemoteAttachmentDownloader: stop") + self.operationQueue.cancelAllOperations() } } diff --git a/Zotero/Controllers/Controllers.swift b/Zotero/Controllers/Controllers.swift index 13f919cc1..4e21d4c08 100644 --- a/Zotero/Controllers/Controllers.swift +++ b/Zotero/Controllers/Controllers.swift @@ -264,6 +264,10 @@ final class Controllers { controllers?.disableSync(apiKey: self.apiKey) // Cancel all downloads controllers?.fileDownloader.stop() + // Cancel all identifier lookups + controllers?.identifierLookupController.cancelAllLookups() + // Cancel all remote downloads + controllers?.remoteFileDownloader.stop() // Cancel all background uploads controllers?.backgroundUploadObserver.cancelAllUploads() // Clear user controllers @@ -296,6 +300,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 @@ -362,6 +367,14 @@ 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, + translatorsController: controllers.translatorsAndStylesController, + 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..73f2c778d --- /dev/null +++ b/Zotero/Controllers/IdentifierLookupController.swift @@ -0,0 +1,589 @@ +// +// IdentifierLookupController.swift +// Zotero +// +// Created by Miltiadis Vasilakis on 22/6/23. +// Copyright © 2023 Corporation for Digital Scholarship. All rights reserved. +// + +import Foundation +import WebKit +import OrderedCollections + +import CocoaLumberjackSwift +import RxSwift + +protocol IdentifierLookupWebViewProvider: AnyObject { + func addWebView() -> WKWebView +} + +protocol IdentifierLookupPresenter: AnyObject { + func isPresenting() -> Bool +} + +final class IdentifierLookupController { + // MARK: Types + struct Update { + enum Kind { + case lookupError(error: Swift.Error) + case identifiersDetected(identifiers: [String]) + case lookupInProgress(identifier: String) + case lookupFailed(identifier: String) + case parseFailed(identifier: String) + case itemCreationFailed(identifier: String, response: ItemResponse, attachments: [(Attachment, URL)]) + case itemStored(identifier: String, response: ItemResponse, attachments: [(Attachment, URL)]) + case pendingAttachments(identifier: String, response: ItemResponse, attachments: [(Attachment, URL)]) + case finishedAllLookups + } + + let kind: Kind + let lookupData: [LookupData] + } + + struct LookupData { + enum State: CustomStringConvertible { + case enqueued + case inProgress + case failed + case translated(TranslatedLookupData) + + struct TranslatedLookupData { + let response: ItemResponse + let attachments: [(Attachment, URL)] + let libraryId: LibraryIdentifier + let collectionKeys: Set + } + + var description: String { + switch self { + case .enqueued: + return "enqueued" + + case .inProgress: + return "inProgress" + + case .failed: + return "failed" + + case .translated: + return "translated" + } + } + + var canTransition: Bool { + // [translated, failed] are final states + switch self { + case .enqueued, .inProgress: + return true + + case .translated, .failed: + return false + } + } + + static func isTransitionValid(from: Self, to: Self) -> Bool { + // enqueued is initial state + // enqueued -> [inProgress, failed] + // inProgress -> [translated, failed] + switch (from, to) { + case (.enqueued, .inProgress), (.enqueued, .failed), (.inProgress, .translated), (.inProgress, .failed): + return true + + default: + return false + } + } + } + + let identifier: String + let state: State + } + + // MARK: Properties + let observable: PublishSubject + private let dispatchSpecificKey: DispatchSpecificKey + private let accessQueueLabel: String + private let accessQueue: DispatchQueue + private let backgroundQueue: DispatchQueue + private let backgroundScheduler: SerialDispatchQueueScheduler + private unowned let dbStorage: DbStorage + private unowned let fileStorage: FileStorage + 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 lookupData: OrderedDictionary = [:] + private var lookupSavedCount = 0 + private var lookupFailedCount = 0 + private var lookupTotalCount: Int { + lookupData.count + } + private var lookupRemainingCount: Int { + lookupTotalCount - lookupSavedCount - lookupFailedCount + } + var batchData: (savedCount: Int, failedCount: Int, totalCount: Int) { + var savedCount = 0 + var failedCount = 0 + var totalCount = 0 + + accessQueue.sync { [weak self] in + guard let self else { return } + savedCount = lookupSavedCount + failedCount = lookupFailedCount + totalCount = lookupTotalCount + } + + return (savedCount, failedCount, totalCount) + } + + internal weak var webViewProvider: IdentifierLookupWebViewProvider? + internal weak var presenter: IdentifierLookupPresenter? { + didSet { + guard presenter == nil, oldValue != nil else { return } + cleanupLookupIfNeeded(force: false) { [weak self] cleaned in + guard let self, cleaned else { return } + observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + } + } + private var lookupWebViewHandlersByLookupSettings: [LookupWebViewHandler.LookupSettings: LookupWebViewHandler] = [:] + + // MARK: Object Lifecycle + init( + dbStorage: DbStorage, + fileStorage: FileStorage, + translatorsController: TranslatorsAndStylesController, + schemaController: SchemaController, + dateParser: DateParser, + remoteFileDownloader: RemoteAttachmentDownloader + ) { + self.fileStorage = fileStorage + self.dbStorage = dbStorage + self.translatorsController = translatorsController + self.schemaController = schemaController + self.dateParser = dateParser + self.remoteFileDownloader = remoteFileDownloader + + self.dispatchSpecificKey = DispatchSpecificKey() + self.accessQueueLabel = "org.zotero.IdentifierLookupController.accessQueue" + self.accessQueue = DispatchQueue(label: accessQueueLabel, qos: .userInteractive, attributes: .concurrent) + accessQueue.setSpecific(key: dispatchSpecificKey, value: accessQueueLabel) + self.backgroundQueue = DispatchQueue(label: "org.zotero.IdentifierLookupController.backgroundProcessing", qos: .userInitiated) + self.backgroundScheduler = SerialDispatchQueueScheduler(queue: backgroundQueue, internalSerialQueueName: "org.zotero.IdentifierLookupController.backgroundScheduler") + self.observable = PublishSubject() + self.disposeBag = DisposeBag() + + setupObservers() + } + + // MARK: Actions + func initialize(libraryId: LibraryIdentifier, collectionKeys: Set, completion: @escaping ([LookupData]?) -> Void) { + accessQueue.async(flags: .barrier) { [weak self] in + var lookupData: [LookupData]? + defer { + completion(lookupData) + } + guard let self else { return } + let lookupSettings = LookupWebViewHandler.LookupSettings(libraryIdentifier: libraryId, collectionKeys: collectionKeys) + if lookupWebViewHandlersByLookupSettings[lookupSettings] != nil { + lookupData = Array(self.lookupData.values) + return + } + var lookupWebViewHandler: LookupWebViewHandler? + inMainThread(sync: true) { + if let webView = self.webViewProvider?.addWebView() { + lookupWebViewHandler = LookupWebViewHandler(lookupSettings: lookupSettings, webView: webView, translatorsController: self.translatorsController) + } + } + guard let lookupWebViewHandler else { + DDLogError("IdentifierLookupController: can't create LookupWebViewHandler instance") + return + } + lookupWebViewHandlersByLookupSettings[lookupSettings] = lookupWebViewHandler + setupObserver(for: lookupWebViewHandler) + lookupData = Array(self.lookupData.values) + } + } + + func lookUp(libraryId: LibraryIdentifier, collectionKeys: Set, identifier: String) { + let lookupSettings = LookupWebViewHandler.LookupSettings(libraryIdentifier: libraryId, collectionKeys: collectionKeys) + guard let lookupWebViewHandler = lookupWebViewHandlersByLookupSettings[lookupSettings] else { + DDLogError("IdentifierLookupController: can't find lookup web view handler for settings - \(lookupSettings)") + return + } + lookupWebViewHandler.lookUp(identifier: identifier) + } + + func cancelAllLookups() { + accessQueue.async(flags: .barrier) { [weak self] in + guard let self else { return } + DDLogInfo("IdentifierLookupController: cancel all lookups") + let keys = lookupWebViewHandlersByLookupSettings.keys + for key in keys { + guard let webView = lookupWebViewHandlersByLookupSettings.removeValue(forKey: key)?.webViewHandler.webView else { continue } + inMainThread { + webView.removeFromSuperview() + } + } + remoteFileDownloader.stop() + let lookupData = self.lookupData + cleanupLookupIfNeeded(force: true) { [weak self] _ in + self?.observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + let storedItemResponses: [(ItemResponse, LibraryIdentifier)] = lookupData.values.compactMap { + switch $0.state { + case .translated(let translatedLookupData): + return (translatedLookupData.response, translatedLookupData.libraryId) + + default: + return nil + } + } + backgroundQueue.async { [weak self] in + guard let self else { return } + do { + let requests = storedItemResponses.map({ MarkItemsAsTrashedDbRequest(keys: [$0.0.key], libraryId: $0.1, trashed: true) }) + try dbStorage.perform(writeRequests: requests, on: backgroundQueue) + } catch let error { + DDLogError("IdentifierLookupController: can't trash item(s) - \(error)") + } + } + } + } + + // MARK: Setups + private func setupObservers() { + remoteFileDownloader.observable + .observe(on: backgroundScheduler) + .subscribe { [weak self] update in + var cleanupLookupIfNeeded = false + switch update.kind { + case .ready(let attachment): + finish(download: update.download, attachment: attachment) + cleanupLookupIfNeeded = true + + case .cancelled, .failed: + cleanupLookupIfNeeded = true + + case .progress: + break + } + guard cleanupLookupIfNeeded else { return } + self?.cleanupLookupIfNeeded(force: false) { [weak self] _ in + guard let self else { return } + observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + } + .disposed(by: self.disposeBag) + + func finish(download: RemoteAttachmentDownloader.Download, attachment: Attachment) { + let localizedType = schemaController.localized(itemType: ItemTypes.attachment) ?? ItemTypes.attachment + 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) + } + } + } + + private func setupObserver(for lookupWebViewHandler: LookupWebViewHandler) { + lookupWebViewHandler.observable + .subscribe { result in + process(result: result) + } + .disposed(by: self.disposeBag) + + func process(result: Result) { + switch result { + case .success(let data): + process(data: data) + + case .failure(let error): + DDLogError("IdentifierLookupController: lookup failed - \(error)") + cleanupLookupIfNeeded(force: false) { [weak self] _ in + guard let self else { return } + observable.on(.next(Update(kind: .lookupError(error: error), lookupData: Array(lookupData.values)))) + } + } + + func process(data: LookupWebViewHandler.LookupData) { + switch data { + case .identifiers(let identifiers): + let enqueuedIdentifiers = identifiers.map({ identifier(from: $0) }) + enqueueLookup(for: enqueuedIdentifiers) { [weak self] validIdentifiers in + guard let self else { return } + if validIdentifiers.isEmpty { + cleanupLookupIfNeeded(force: false) { [weak self] _ in + guard let self else { return } + observable.on(.next(Update(kind: .identifiersDetected(identifiers: []), lookupData: Array(lookupData.values)))) + } + } + observable.on(.next(Update(kind: .identifiersDetected(identifiers: validIdentifiers), lookupData: Array(lookupData.values)))) + } + + case .item(let data): + guard let lookupId = data["identifier"] as? [String: String] else { + DDLogWarn("IdentifierLookupController: lookup item data don't contain identifier") + return + } + let identifier = identifier(from: lookupId) + var currentState: LookupData.State? + accessQueue.sync { [weak self] in + guard let self, let currentLookupData = lookupData[identifier] else { return } + currentState = currentLookupData.state + } + guard currentState?.canTransition == true else { + DDLogWarn("IdentifierLookupController: \(identifier) lookup item can't transition from state: \(String(describing: currentState))") + return + } + + if data.keys.count == 1 { + changeLookup(for: identifier, to: .inProgress) { [weak self] didChange in + guard let self, didChange else { return } + observable.on(.next(Update(kind: .lookupInProgress(identifier: identifier), lookupData: Array(lookupData.values)))) + // Since at least one identifier lookup is in progress, there is no need to cleanup if needed. + } + return + } + + if let error = data["error"] { + DDLogError("IdentifierLookupController: \(identifier) lookup failed - \(error)") + changeLookup(for: identifier, to: .failed) { [weak self] didChange in + guard let self, didChange else { return } + observable.on(.next(Update(kind: .lookupFailed(identifier: identifier), lookupData: Array(lookupData.values)))) + cleanupLookupIfNeeded(force: false) { [weak self] cleaned in + guard let self, cleaned else { return } + observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + } + return + } + + let libraryId = lookupWebViewHandler.lookupSettings.libraryIdentifier + let collectionKeys = lookupWebViewHandler.lookupSettings.collectionKeys + guard let itemData = data["data"] as? [[String: Any]], + let item = itemData.first, + let (response, attachments) = parse(item, libraryId: libraryId, collectionKeys: collectionKeys, schemaController: schemaController, dateParser: dateParser) + else { + changeLookup(for: identifier, to: .failed) { [weak self] didChange in + guard let self, didChange else { return } + observable.on(.next(Update(kind: .parseFailed(identifier: identifier), lookupData: Array(lookupData.values)))) + cleanupLookupIfNeeded(force: false) { [weak self] cleaned in + guard let self, cleaned else { return } + observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + } + return + } + + process(identifier: identifier, response: response, attachments: attachments, libraryId: libraryId, collectionKeys: collectionKeys) + } + + func identifier(from data: [String: String]) -> String { + var result = "" + for (key, value) in data { + result += key + ":" + value + } + return result + } + + /// Tries to parse `ItemResponse` from data returned by translation server. It prioritizes items with attachments if there are multiple items. + /// - parameter itemData: Data to parse + /// - parameter schemaController: SchemaController which is used for validating item type and field types + /// - returns: `ItemResponse` of parsed item and optional attachment dictionary with title and url. + func parse( + _ itemData: [String: Any], + libraryId: LibraryIdentifier, + collectionKeys: Set, + schemaController: SchemaController, + dateParser: DateParser + ) -> (ItemResponse, [(Attachment, URL)])? { + do { + let item = try ItemResponse(translatorResponse: itemData, schemaController: schemaController).copy(libraryId: libraryId, collectionKeys: collectionKeys, tags: []) + + let attachments = ((itemData["attachments"] as? [[String: Any]]) ?? []).compactMap { data -> (Attachment, URL)? in + // We can't process snapshots yet, so ignore all text/html attachments + guard let mimeType = data["mimeType"] as? String, mimeType != "text/html", let ext = mimeType.extensionFromMimeType, + let urlString = data["url"] as? String, let url = URL(string: urlString) + else { return nil } + + let key = KeyGenerator.newKey + let filename = FilenameFormatter.filename(from: item, defaultTitle: "Full Text", ext: ext, dateParser: dateParser) + let attachment = Attachment( + type: .file(filename: filename, contentType: mimeType, location: .local, linkType: .importedFile), + title: filename, + key: key, + libraryId: libraryId + ) + + return (attachment, url) + } + + return (item, attachments) + } catch let error { + DDLogError("IdentifierLookupController: can't parse data - \(error)") + return nil + } + } + + func process(identifier: String, response: ItemResponse, attachments: [(Attachment, URL)], libraryId: LibraryIdentifier, collectionKeys: Set) { + backgroundQueue.async { [weak self] in + guard let self else { return } + do { + try storeDataAndDownloadAttachmentIfNecessary(identifier: identifier, response: response, attachments: attachments) + } catch let error { + DDLogError("IdentifierLookupController: can't create item(s) - \(error)") + changeLookup(for: identifier, to: .failed) { [weak self] didChange in + guard let self, didChange else { return } + observable.on(.next(Update(kind: .itemCreationFailed(identifier: identifier, response: response, attachments: attachments), lookupData: Array(lookupData.values)))) + cleanupLookupIfNeeded(force: false) { [weak self] cleaned in + guard let self, cleaned else { return } + observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + } + } + } + + 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) + changeLookup( + for: identifier, + to: .translated(.init(response: response, attachments: attachments, libraryId: libraryId, collectionKeys: collectionKeys)) + ) { [weak self] didChange in + guard let self, didChange else { return } + observable.on(.next(Update(kind: .itemStored(identifier: identifier, response: response, attachments: attachments), lookupData: Array(lookupData.values)))) + + if Defaults.shared.shareExtensionIncludeAttachment, !attachments.isEmpty { + let downloadData = attachments.map({ ($0, $1, response.key) }) + remoteFileDownloader.download(data: downloadData) + observable.on(.next(Update(kind: .pendingAttachments(identifier: identifier, response: response, attachments: attachments), lookupData: Array(lookupData.values)))) + } + + cleanupLookupIfNeeded(force: false) { [weak self] cleaned in + guard let self, cleaned else { return } + observable.on(.next(Update(kind: .finishedAllLookups, lookupData: []))) + } + } + } + } + } + } + } + + // MARK: Lookup Data + private func cleanupLookupIfNeeded(force: Bool, completion: @escaping (Bool) -> Void) { + if DispatchQueue.getSpecific(key: dispatchSpecificKey) == accessQueueLabel { + cleanupLookup(force: force, completion: completion) + } else { + accessQueue.async(flags: .barrier) { + cleanupLookup(force: force, completion: completion) + } + } + + func cleanupLookup(force: Bool, completion: @escaping (Bool) -> Void) { + if force { + // If forced, cleanup and return + cleanup(completion: completion) + return + } + guard lookupRemainingCount == 0, remoteFileDownloader.batchData.2 == 0 else { + // If there are remaining lookups, or downloading attachments, then just return + completion(false) + return + } + guard let presenter else { + // If no presenter is assigned, then cleanup and return + cleanup(completion: completion) + return + } + // Presenter is assigned + DispatchQueue.main.async { [weak self] in + // Checking if it is presenting in main queue. + // Doing so asynchronously, to not cause a deadlock if cleanupLookupIfNeeded is already called from the main thread. + guard !presenter.isPresenting(), let self else { + completion(false) + return + } + // It is not presenting, then cleanup + self.accessQueue.async(flags: .barrier) { + cleanup(completion: completion) + } + } + + func cleanup(completion: @escaping (Bool) -> Void) { + lookupData = [:] + lookupSavedCount = 0 + lookupFailedCount = 0 + DDLogInfo("IdentifierLookupController: cleaned up lookup data") + let keys = lookupWebViewHandlersByLookupSettings.keys + for key in keys { + guard let webView = lookupWebViewHandlersByLookupSettings.removeValue(forKey: key)?.webViewHandler.webView else { continue } + inMainThread { + webView.removeFromSuperview() + } + } + completion(true) + } + } + } + + private func enqueueLookup(for identifiers: [String], completion: @escaping ([String]) -> Void) { + accessQueue.async(flags: .barrier) { [weak self] in + guard let self else { return } + var newUniqueIdentifiers: [String] = [] + var index = 0 + for identifier in identifiers { + guard lookupData[identifier] == nil else { continue } + newUniqueIdentifiers.append(identifier) + lookupData.updateValue(.init(identifier: identifier, state: .enqueued), forKey: identifier, insertingAt: index) + index += 1 + } + completion(newUniqueIdentifiers) + } + } + + private func changeLookup(for identifier: String, to state: LookupData.State, completion: @escaping (Bool) -> Void) { + accessQueue.async(flags: .barrier) { [weak self] in + guard let self else { return } + var didChange = false + defer { + completion(didChange) + } + guard let currentLookupData = lookupData[identifier] else { return } + let currentState = currentLookupData.state + let isTransitionValid = LookupData.State.isTransitionValid(from: currentState, to: state) + guard isTransitionValid else { + DDLogWarn("IdentifierLookupController: \(identifier) lookup item won't transition from state: \(String(describing: currentState)) to state: \(String(describing: state))") + return + } + lookupData[identifier] = .init(identifier: identifier, state: state) + didChange = true + switch state { + case .failed: + lookupFailedCount += 1 + + case .translated: + lookupSavedCount += 1 + + default: + break + } + } + } +} diff --git a/Zotero/Controllers/Web View Handling/LookupWebViewHandler.swift b/Zotero/Controllers/Web View Handling/LookupWebViewHandler.swift index e7d5cc237..a32f5bee5 100644 --- a/Zotero/Controllers/Web View Handling/LookupWebViewHandler.swift +++ b/Zotero/Controllers/Web View Handling/LookupWebViewHandler.swift @@ -14,6 +14,11 @@ import RxCocoa import RxSwift final class LookupWebViewHandler { + struct LookupSettings: Hashable { + let libraryIdentifier: LibraryIdentifier + let collectionKeys: Set + } + /// Handlers for communication with JS in `webView` enum JSHandlers: String, CaseIterable { /// Handler used for reporting new items. @@ -46,14 +51,16 @@ final class LookupWebViewHandler { case failed(Swift.Error) } - private let webViewHandler: WebViewHandler + let lookupSettings: LookupSettings + let webViewHandler: WebViewHandler private let translatorsController: TranslatorsAndStylesController private let disposeBag: DisposeBag let observable: PublishSubject> private var isLoading: BehaviorRelay - init(webView: WKWebView, translatorsController: TranslatorsAndStylesController) { + init(lookupSettings: LookupSettings, webView: WKWebView, translatorsController: TranslatorsAndStylesController) { + self.lookupSettings = lookupSettings self.translatorsController = translatorsController self.webViewHandler = WebViewHandler(webView: webView, javascriptHandlers: JSHandlers.allCases.map({ $0.rawValue })) self.observable = PublishSubject() @@ -76,6 +83,11 @@ final class LookupWebViewHandler { }) .disposed(by: self.disposeBag) } + + convenience init(libraryIdentifier: LibraryIdentifier, collectionKeys: Set, webView: WKWebView, translatorsController: TranslatorsAndStylesController) { + let lookupSettings = LookupSettings(libraryIdentifier: libraryIdentifier, collectionKeys: collectionKeys) + self.init(lookupSettings: lookupSettings, webView: webView, translatorsController: translatorsController) + } func lookUp(identifier: String) { switch self.isLoading.value { diff --git a/Zotero/Controllers/Web View Handling/WebViewHandler.swift b/Zotero/Controllers/Web View Handling/WebViewHandler.swift index 6233b5591..90ade4887 100644 --- a/Zotero/Controllers/Web View Handling/WebViewHandler.swift +++ b/Zotero/Controllers/Web View Handling/WebViewHandler.swift @@ -20,7 +20,7 @@ final class WebViewHandler: NSObject { private let session: URLSession - private weak var webView: WKWebView? + private(set) weak var webView: WKWebView? private var webDidLoad: ((SingleEvent<()>) -> Void)? var receivedMessageHandler: ((String, Any) -> Void)? // Cookies, User-Agent and Referrer from original website are stored and added to requests in `sendRequest(with:)`. diff --git a/Zotero/Extensions/Localizable.swift b/Zotero/Extensions/Localizable.swift index 6636aced9..c5ddf9898 100644 --- a/Zotero/Extensions/Localizable.swift +++ b/Zotero/Extensions/Localizable.swift @@ -22,6 +22,8 @@ internal enum L10n { internal static let betaWipeTitle = L10n.tr("Localizable", "beta_wipe_title", fallback: "Resync Required") /// Cancel internal static let cancel = L10n.tr("Localizable", "cancel", fallback: "Cancel") + /// Cancel All + internal static let cancelAll = L10n.tr("Localizable", "cancel_all", fallback: "Cancel All") /// Clear internal static let clear = L10n.tr("Localizable", "clear", fallback: "Clear") /// Localizable.strings @@ -367,8 +369,6 @@ internal enum L10n { internal static let db = L10n.tr("Localizable", "errors.db", fallback: "Could not connect to database. The device storage might be full.") /// Error creating database. Please try logging in again. internal static let dbFailure = L10n.tr("Localizable", "errors.db_failure", fallback: "Error creating database. Please try logging in again.") - /// Zotero could not find any identifiers in your input. Please verify your input and try again. - internal static let lookup = L10n.tr("Localizable", "errors.lookup", fallback: "Zotero could not find any identifiers in your input. Please verify your input and try again.") /// Could not parse some data. Other data will continue to sync. internal static let parsing = L10n.tr("Localizable", "errors.parsing", fallback: "Could not parse some data. Other data will continue to sync.") /// Some data in My Library could not be downloaded. It may have been saved with a newer version of Zotero. @@ -495,6 +495,12 @@ internal enum L10n { /// Invalid username internal static let invalidUsername = L10n.tr("Localizable", "errors.login.invalid_username", fallback: "Invalid username") } + internal enum Lookup { + /// Zotero could not find any identifiers in your input. Please verify your input and try again. + internal static let noIdentifiersAndNoLookupData = L10n.tr("Localizable", "errors.lookup.no_identifiers_and_no_lookup_data", fallback: "Zotero could not find any identifiers in your input. Please verify your input and try again.") + /// Zotero could not find any new identifiers in your input, or they are already being added. Please verify your input and try again. + internal static let noIdentifiersWithLookupData = L10n.tr("Localizable", "errors.lookup.no_identifiers_with_lookup_data", fallback: "Zotero could not find any new identifiers in your input, or they are already being added. Please verify your input and try again.") + } internal enum Pdf { /// Can't add annotations. internal static let cantAddAnnotations = L10n.tr("Localizable", "errors.pdf.cant_add_annotations", fallback: "Can't add annotations.") @@ -795,6 +801,10 @@ internal enum L10n { internal static func toolbarFilter(_ p1: Int) -> String { return L10n.tr("Localizable", "items.toolbar_filter", p1, fallback: "Plural format key: \"%#@toolbar_filter@\"") } + /// Saved %d / %d + internal static func toolbarSaved(_ p1: Int, _ p2: Int) -> String { + return L10n.tr("Localizable", "items.toolbar_saved", p1, p2, fallback: "Saved %d / %d") + } internal enum Action { /// Add to Collection internal static let addToCollection = L10n.tr("Localizable", "items.action.add_to_collection", fallback: "Add to Collection") diff --git a/Zotero/Scenes/Detail/DetailCoordinator.swift b/Zotero/Scenes/Detail/DetailCoordinator.swift index 69ee1f4d9..b7c975a1d 100644 --- a/Zotero/Scenes/Detail/DetailCoordinator.swift +++ b/Zotero/Scenes/Detail/DetailCoordinator.swift @@ -38,6 +38,7 @@ protocol DetailItemsCoordinatorDelegate: AnyObject { func showMissingStyleError() func showAttachment(key: String, parentKey: String?, libraryId: LibraryIdentifier) func show(error: ItemsError) + func showLookup() } protocol DetailItemDetailCoordinatorDelegate: AnyObject { @@ -121,6 +122,8 @@ final class DetailCoordinator: Coordinator { library: self.library, dbStorage: userControllers.dbStorage, fileDownloader: userControllers.fileDownloader, + remoteFileDownloader: userControllers.remoteFileDownloader, + identifierLookupController: userControllers.identifierLookupController, syncScheduler: userControllers.syncScheduler, citationController: userControllers.citationController, fileCleanupController: userControllers.fileCleanupController, @@ -135,6 +138,8 @@ final class DetailCoordinator: Coordinator { library: Library, dbStorage: DbStorage, fileDownloader: AttachmentDownloader, + remoteFileDownloader: RemoteAttachmentDownloader, + identifierLookupController: IdentifierLookupController, syncScheduler: SynchronizationScheduler, citationController: CitationController, fileCleanupController: AttachmentFileCleanupController, @@ -144,7 +149,20 @@ final class DetailCoordinator: Coordinator { itemsTagFilterDelegate?.clearSelection() let searchTerm = self.searchItemKeys?.joined(separator: " ") - let state = ItemsState(collection: collection, library: library, sortType: .default, searchTerm: searchTerm, filters: [], error: nil) + let downloadBatchData = ItemsState.DownloadBatchData(batchData: fileDownloader.batchData) + let remoteDownloadBatchData = ItemsState.DownloadBatchData(batchData: remoteFileDownloader.batchData) + let identifierLookupBatchData = ItemsState.IdentifierLookupBatchData(batchData: identifierLookupController.batchData) + let state = ItemsState( + collection: collection, + library: library, + sortType: .default, + searchTerm: searchTerm, + filters: [], + downloadBatchData: downloadBatchData, + remoteDownloadBatchData: remoteDownloadBatchData, + identifierLookupBatchData: identifierLookupBatchData, + error: nil + ) let handler = ItemsActionHandler( dbStorage: dbStorage, fileStorage: self.controllers.fileStorage, @@ -319,7 +337,7 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate { controller.popoverPresentationController?.barButtonItem = button controller.addAction(UIAlertAction(title: L10n.Items.lookup, style: .default, handler: { [weak self] _ in - self?.showLookup(startWith: .manual) + self?.showLookup(startWith: .manual(restoreLookupState: false)) })) controller.addAction(UIAlertAction(title: L10n.Items.barcode, style: .default, handler: { [weak self] _ in @@ -642,6 +660,10 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate { self.navigationController?.present(navigationController, animated: true, completion: nil) } + + func showLookup() { + showLookup(startWith: .manual(restoreLookupState: true)) + } } extension DetailCoordinator: DetailItemActionSheetCoordinatorDelegate { diff --git a/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift b/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift index 5efdf65b6..17379126f 100644 --- a/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift +++ b/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift @@ -41,6 +41,8 @@ enum ItemsAction { case cacheItemAccessory(item: RItem) case updateAttachments(AttachmentFileDeletedNotification) case updateDownload(update: AttachmentDownloader.Update, batchData: ItemsState.DownloadBatchData?) + case updateIdentifierLookup(update: IdentifierLookupController.Update, batchData: ItemsState.IdentifierLookupBatchData) + case updateRemoteDownload(update: RemoteAttachmentDownloader.Update, batchData: ItemsState.DownloadBatchData?) case openAttachment(attachment: Attachment, parentKey: String?) case attachmentOpened(String) case updateKeys(items: Results, deletions: [Int], insertions: [Int], modifications: [Int]) diff --git a/Zotero/Scenes/Detail/Items/Models/ItemsState.swift b/Zotero/Scenes/Detail/Items/Models/ItemsState.swift index e1d08956f..fb68be036 100644 --- a/Zotero/Scenes/Detail/Items/Models/ItemsState.swift +++ b/Zotero/Scenes/Detail/Items/Models/ItemsState.swift @@ -31,12 +31,56 @@ struct ItemsState: ViewModelState { let downloaded: Int let total: Int - init?(progress: Progress, remaining: Int, total: Int) { - guard total > 1 else { return nil } + init(fraction: Double, downloaded: Int, total: Int) { + self.fraction = fraction + self.downloaded = downloaded + self.total = total + } + + init?(progress: Progress?, remaining: Int, total: Int) { + guard let progress, total > 0 else { return nil } self.fraction = progress.fractionCompleted self.downloaded = total - remaining self.total = total } + + init?(batchData: (progress: Progress?, remainingCount: Int, totalCount: Int)) { + self.init(progress: batchData.progress, remaining: batchData.remainingCount, total: batchData.totalCount) + } + + static func + (lhs: Self, rhs: Self) -> Self { + let fraction = (lhs.fraction + rhs.fraction) / 2.0 + let downloaded = lhs.downloaded + rhs.downloaded + let total = lhs.total + rhs.total + return .init(fraction: fraction, downloaded: downloaded, total: total) + } + + static func combineDownloadBatchData(_ array: [Self?]) -> Self? { + let validArray = array.compactMap { $0 } + guard let firstData = validArray.first else { return nil } + guard validArray.count > 1 else { return firstData } + return validArray[1..) { + self.update(viewModel: viewModel) { state in + if state.identifierLookupBatchData != batchData { + state.identifierLookupBatchData = batchData + state.changes = .batchData + } + } + } + + private func process(remoteDownloadUpdate update: RemoteAttachmentDownloader.Update, batchData: ItemsState.DownloadBatchData?, in viewModel: ViewModel) { + self.update(viewModel: viewModel) { state in + if state.remoteDownloadBatchData != batchData { + state.remoteDownloadBatchData = batchData + state.changes = .batchData + } + } + } private func cacheItemAccessory(for item: RItem, in viewModel: ViewModel) { // Create cached accessory only if there is nothing in cache yet. diff --git a/Zotero/Scenes/Detail/Items/ViewModels/ItemsToolbarController.swift b/Zotero/Scenes/Detail/Items/ViewModels/ItemsToolbarController.swift index 23b7684cb..6a9ecd7cd 100644 --- a/Zotero/Scenes/Detail/Items/ViewModels/ItemsToolbarController.swift +++ b/Zotero/Scenes/Detail/Items/ViewModels/ItemsToolbarController.swift @@ -13,14 +13,20 @@ import RxSwift protocol ItemsToolbarControllerDelegate: UITraitEnvironment { func process(action: ItemAction.Kind, button: UIBarButtonItem) + func showLookup() } final class ItemsToolbarController { - private static let barButtonItemEmptyTag = 1 - private static let barButtonItemSingleTag = 2 - private static let barButtonItemFilterTag = 3 - private static let barButtonItemTitleTag = 4 - private static let finishVisibilityTime: RxTimeInterval = .seconds(2) + enum ToolbarItem: Int { + case empty = 1 + case single + case filter + case title + + var tag: Int { + rawValue + } + } private unowned let viewController: UIViewController private let editingActions: [ItemAction] @@ -31,7 +37,7 @@ final class ItemsToolbarController { init(viewController: UIViewController, initialState: ItemsState, delegate: ItemsToolbarControllerDelegate) { self.viewController = viewController self.delegate = delegate - self.editingActions = ItemsToolbarController.editingActions(for: initialState) + self.editingActions = Self.editingActions(for: initialState) self.disposeBag = DisposeBag() self.createToolbarItems(for: initialState) @@ -65,7 +71,13 @@ final class ItemsToolbarController { } else { let filters = self.sizeClassSpecificFilters(from: state.filters) self.viewController.toolbarItems = self.createNormalToolbarItems(for: filters) - self.updateNormalToolbarItems(for: filters, downloadBatchData: state.downloadBatchData, results: state.results) + self.updateNormalToolbarItems( + for: filters, + downloadBatchData: state.downloadBatchData, + remoteDownloadBatchData: state.remoteDownloadBatchData, + identifierLookupBatchData: state.identifierLookupBatchData, + results: state.results + ) } } @@ -73,7 +85,13 @@ final class ItemsToolbarController { if state.isEditing { self.updateEditingToolbarItems(for: state.selectedItems, results: state.results) } else { - self.updateNormalToolbarItems(for: self.sizeClassSpecificFilters(from: state.filters), downloadBatchData: state.downloadBatchData, results: state.results) + self.updateNormalToolbarItems( + for: self.sizeClassSpecificFilters(from: state.filters), + downloadBatchData: state.downloadBatchData, + remoteDownloadBatchData: state.remoteDownloadBatchData, + identifierLookupBatchData: state.identifierLookupBatchData, + results: state.results + ) } } @@ -99,24 +117,32 @@ final class ItemsToolbarController { private func updateEditingToolbarItems(for selectedItems: Set, results: Results?) { self.viewController.toolbarItems?.forEach({ item in - switch item.tag { - case ItemsToolbarController.barButtonItemEmptyTag: + switch ToolbarItem(rawValue: item.tag) { + case .empty: item.isEnabled = !selectedItems.isEmpty - case ItemsToolbarController.barButtonItemSingleTag: + case .single: item.isEnabled = selectedItems.count == 1 - default: break + + default: + break } }) } - private func updateNormalToolbarItems(for filters: [ItemsFilter], downloadBatchData: ItemsState.DownloadBatchData?, results: Results?) { - if let item = self.viewController.toolbarItems?.first(where: { $0.tag == ItemsToolbarController.barButtonItemFilterTag }) { + private func updateNormalToolbarItems( + for filters: [ItemsFilter], + downloadBatchData: ItemsState.DownloadBatchData?, + remoteDownloadBatchData: ItemsState.DownloadBatchData?, + identifierLookupBatchData: ItemsState.IdentifierLookupBatchData, + results: Results? + ) { + if let item = self.viewController.toolbarItems?.first(where: { $0.tag == ToolbarItem.filter.tag }) { let filterImageName = filters.isEmpty ? "line.horizontal.3.decrease.circle" : "line.horizontal.3.decrease.circle.fill" item.image = UIImage(systemName: filterImageName) } - if let item = self.viewController.toolbarItems?.first(where: { $0.tag == ItemsToolbarController.barButtonItemTitleTag }), + if let item = self.viewController.toolbarItems?.first(where: { $0.tag == ToolbarItem.title.tag }), let stackView = item.customView as? UIStackView { if let filterLabel = stackView.subviews.first as? UILabel { let itemCount = results?.count ?? 0 @@ -129,10 +155,33 @@ final class ItemsToolbarController { } if let progressView = stackView.subviews.last as? ItemsToolbarDownloadProgressView { - progressView.isHidden = !filters.isEmpty || downloadBatchData == nil + var isUserInteractionEnabled = false + let attributedText = NSMutableAttributedString() + var progress: Float? + let remoteDownloading = remoteDownloadBatchData != nil + let defaultAttributes: [NSAttributedString.Key: Any] = [.foregroundColor: UIColor.label, .font: UIFont.preferredFont(forTextStyle: .footnote)] + if identifierLookupBatchData != .zero, !identifierLookupBatchData.isFinished || remoteDownloading { + // Show "Saved x / y" only if lookup hasn't finished, or there are also ongoing remote downloads + isUserInteractionEnabled = true + let identifierLookupText = L10n.Items.toolbarSaved(identifierLookupBatchData.saved, identifierLookupBatchData.total) + let identifierLookupAttributes: [NSAttributedString.Key: Any] = [.foregroundColor: Asset.Colors.zoteroBlueWithDarkMode.color, .font: UIFont.preferredFont(forTextStyle: .footnote)] + attributedText.append(.init(string: identifierLookupText, attributes: identifierLookupAttributes)) + } + if let combinedDownloadBatchData = ItemsState.DownloadBatchData.combineDownloadBatchData([downloadBatchData, remoteDownloadBatchData]) { + if attributedText.length > 0 { + attributedText.append(.init(string: " / ", attributes: defaultAttributes)) + } + let downloadText = L10n.Items.toolbarDownloaded(combinedDownloadBatchData.downloaded, combinedDownloadBatchData.total) + attributedText.append(.init(string: downloadText, attributes: defaultAttributes)) + progress = Float(combinedDownloadBatchData.fraction) + } + progressView.isUserInteractionEnabled = isUserInteractionEnabled - if let data = downloadBatchData { - progressView.set(downloaded: data.downloaded, total: data.total, progress: Float(data.fraction)) + if !filters.isEmpty || (attributedText.length == 0) { + progressView.isHidden = true + } else { + progressView.set(attributedText: attributedText, progress: progress) + progressView.isHidden = false progressView.sizeToFit() } } @@ -148,7 +197,7 @@ final class ItemsToolbarController { let filterImageName = filters.isEmpty ? "line.horizontal.3.decrease.circle" : "line.horizontal.3.decrease.circle.fill" let filterButton = UIBarButtonItem(image: UIImage(systemName: filterImageName), style: .plain, target: nil, action: nil) - filterButton.tag = ItemsToolbarController.barButtonItemFilterTag + filterButton.tag = ToolbarItem.filter.tag filterButton.accessibilityLabel = L10n.Accessibility.Items.filterItems filterButton.rx.tap.subscribe(onNext: { [weak self] _ in self?.delegate?.process(action: .filter, button: filterButton) @@ -164,7 +213,7 @@ final class ItemsToolbarController { .disposed(by: self.disposeBag) let titleButton = UIBarButtonItem(customView: self.createTitleView()) - titleButton.tag = ItemsToolbarController.barButtonItemTitleTag + titleButton.tag = ToolbarItem.title.tag return [fixedSpacer, filterButton, flexibleSpacer, titleButton, flexibleSpacer, sortButton, fixedSpacer] } @@ -175,7 +224,7 @@ final class ItemsToolbarController { let item = UIBarButtonItem(image: action.image, style: .plain, target: nil, action: nil) switch action.type { case .addToCollection, .trash, .delete, .removeFromCollection, .restore: - item.tag = ItemsToolbarController.barButtonItemEmptyTag + item.tag = ToolbarItem.empty.tag case .sort, .filter, .createParent, .copyCitation, .copyBibliography, .share, .removeDownload, .download, .duplicate: break } switch action.type { @@ -219,6 +268,15 @@ final class ItemsToolbarController { // Batch download view let progressView = ItemsToolbarDownloadProgressView() + let tap = UITapGestureRecognizer() + tap.rx + .event + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] _ in + self?.delegate?.showLookup() + }) + .disposed(by: self.disposeBag) + progressView.addGestureRecognizer(tap) progressView.isHidden = true let stackView = UIStackView(arrangedSubviews: [filterLabel, progressView]) diff --git a/Zotero/Scenes/Detail/Items/Views/ItemsToolbarDownloadProgressView.swift b/Zotero/Scenes/Detail/Items/Views/ItemsToolbarDownloadProgressView.swift index a89485c2a..b6af0e479 100644 --- a/Zotero/Scenes/Detail/Items/Views/ItemsToolbarDownloadProgressView.swift +++ b/Zotero/Scenes/Detail/Items/Views/ItemsToolbarDownloadProgressView.swift @@ -22,9 +22,27 @@ final class ItemsToolbarDownloadProgressView: UIView { self.createViews() } + func set(text: String, progress: Float?) { + label.text = text + set(progress: progress) + } + + func set(attributedText: NSAttributedString, progress: Float?) { + label.attributedText = attributedText + set(progress: progress) + } + + private func set(progress: Float?) { + if let progress { + progressView.progress = progress + progressView.isHidden = false + } else { + progressView.isHidden = true + } + } + func set(downloaded: Int, total: Int, progress: Float) { - self.label.text = L10n.Items.toolbarDownloaded(downloaded, total) - self.progressView.progress = progress + set(text: L10n.Items.toolbarDownloaded(downloaded, total), progress: progress) } private func createViews() { diff --git a/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift b/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift index 854b7a203..ff26c7e7b 100644 --- a/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift +++ b/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift @@ -492,34 +492,52 @@ final class ItemsViewController: UIViewController { }) .disposed(by: self.disposeBag) - guard let downloader = self.controllers.userControllers?.fileDownloader else { return } - - downloader.observable + let downloader = controllers.userControllers?.fileDownloader + downloader?.observable .observe(on: MainScheduler.instance) .subscribe(with: self, onNext: { [weak downloader] `self`, update in - if let downloader = downloader { - let (progress, remainingCount, totalCount) = downloader.batchData - let batchData = progress.flatMap({ ItemsState.DownloadBatchData(progress: $0, remaining: remainingCount, total: totalCount) }) + if let downloader { + let batchData = ItemsState.DownloadBatchData(batchData: downloader.batchData) self.viewModel.process(action: .updateDownload(update: update, batchData: batchData)) } - + if case .progress = update.kind { return } - + guard self.viewModel.state.attachmentToOpen == update.key else { return } - + self.viewModel.process(action: .attachmentOpened(update.key)) - + switch update.kind { case .ready: self.coordinatorDelegate?.showAttachment(key: update.key, parentKey: update.parentKey, libraryId: update.libraryId) - + case .failed(let error): self.coordinatorDelegate?.showAttachmentError(error) - + default: break } }) .disposed(by: self.disposeBag) + + let identifierLookupController = self.controllers.userControllers?.identifierLookupController + identifierLookupController?.observable + .observe(on: MainScheduler.instance) + .subscribe(with: self, onNext: { [weak identifierLookupController] `self`, update in + guard let identifierLookupController else { return } + let batchData = ItemsState.IdentifierLookupBatchData(batchData: identifierLookupController.batchData) + self.viewModel.process(action: .updateIdentifierLookup(update: update, batchData: batchData)) + }) + .disposed(by: self.disposeBag) + + let remoteDownloader = self.controllers.userControllers?.remoteFileDownloader + remoteDownloader?.observable + .observe(on: MainScheduler.instance) + .subscribe(with: self, onNext: { [weak remoteDownloader] `self`, update in + guard let remoteDownloader else { return } + let batchData = ItemsState.DownloadBatchData(batchData: remoteDownloader.batchData) + self.viewModel.process(action: .updateRemoteDownload(update: update, batchData: batchData)) + }) + .disposed(by: self.disposeBag) } private func setupTitle() { @@ -696,6 +714,10 @@ extension ItemsViewController: ItemsToolbarControllerDelegate { func process(action: ItemAction.Kind, button: UIBarButtonItem) { self.process(action: action, for: self.viewModel.state.selectedItems, button: button, completionAction: nil) } + + func showLookup() { + coordinatorDelegate?.showLookup() + } } extension ItemsViewController: TagFilterDelegate { diff --git a/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift b/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift index 1032c1a41..cf2df755b 100644 --- a/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift +++ b/Zotero/Scenes/Detail/Lookup/LookupCoordinator.swift @@ -13,11 +13,11 @@ import RxSwift enum LookupStartingView { case scanner - case manual + case manual(restoreLookupState: Bool) } protocol LookupCoordinatorDelegate: AnyObject { - func lookupController(multiLookupEnabled: Bool, hasDarkBackground: Bool) -> LookupViewController? + func lookupController(restoreLookupState: Bool, hasDarkBackground: Bool) -> LookupViewController? } final class LookupCoordinator: NSObject, Coordinator { @@ -39,6 +39,7 @@ final class LookupCoordinator: NSObject, Coordinator { super.init() navigationController.dismissHandler = { + self.controllers.userControllers?.identifierLookupController.presenter = nil self.parentCoordinator?.childDidFinish(self) } } @@ -46,9 +47,9 @@ final class LookupCoordinator: NSObject, Coordinator { func start(animated: Bool) { let controller: UIViewController switch self.startingView { - case .manual: - DDLogInfo("LookupCoordinator: show manual lookup") - controller = self.manualController + case .manual(let restoreLookupState): + DDLogInfo("LookupCoordinator: show manual lookup \(restoreLookupState ? " with restored lookup state" : "")") + controller = self.manualController(restoreLookupState: restoreLookupState) case .scanner: DDLogInfo("LookupCoordinator: show scanner lookup") @@ -57,17 +58,10 @@ final class LookupCoordinator: NSObject, Coordinator { self.navigationController?.setViewControllers([controller], animated: animated) } - private func lookupController(multiLookupEnabled: Bool, hasDarkBackground: Bool, userControllers: UserControllers) -> LookupViewController { + private func lookupController(restoreLookupState: 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 state = LookupState(restoreLookupState: restoreLookupState, hasDarkBackground: hasDarkBackground, collectionKeys: collectionKeys, libraryId: Defaults.shared.selectedLibrary) + let handler = LookupActionHandler(identifierLookupController: userControllers.identifierLookupController) let viewModel = ViewModel(initialState: state, handler: handler) return LookupViewController( @@ -83,21 +77,23 @@ final class LookupCoordinator: NSObject, Coordinator { let handler = ScannerActionHandler() let controller = ScannerViewController(viewModel: ViewModel(initialState: state, handler: handler)) controller.coordinatorDelegate = self + controllers.userControllers?.identifierLookupController.presenter = controller return controller } - private var manualController: UIViewController { - let state = ManualLookupState() + private func manualController(restoreLookupState: Bool) -> UIViewController { + let state = ManualLookupState(restoreLookupState: restoreLookupState) let handler = ManualLookupActionHandler() let controller = ManualLookupViewController(viewModel: ViewModel(initialState: state, handler: handler)) controller.coordinatorDelegate = self + controllers.userControllers?.identifierLookupController.presenter = controller return controller } } extension LookupCoordinator: LookupCoordinatorDelegate { - func lookupController(multiLookupEnabled: Bool, hasDarkBackground: Bool) -> LookupViewController? { + func lookupController(restoreLookupState: Bool, hasDarkBackground: Bool) -> LookupViewController? { guard let userControllers = self.controllers.userControllers else { return nil } - return self.lookupController(multiLookupEnabled: multiLookupEnabled, hasDarkBackground: hasDarkBackground, userControllers: userControllers) + return self.lookupController(restoreLookupState: restoreLookupState, hasDarkBackground: hasDarkBackground, userControllers: userControllers) } } diff --git a/Zotero/Scenes/Detail/Lookup/Models/LookupAction.swift b/Zotero/Scenes/Detail/Lookup/Models/LookupAction.swift index c280824c4..88ff4d649 100644 --- a/Zotero/Scenes/Detail/Lookup/Models/LookupAction.swift +++ b/Zotero/Scenes/Detail/Lookup/Models/LookupAction.swift @@ -10,6 +10,7 @@ import Foundation import WebKit enum LookupAction { - case initialize(WKWebView) + case initialize case lookUp(String) + case cancelAllLookups } diff --git a/Zotero/Scenes/Detail/Lookup/Models/LookupState.swift b/Zotero/Scenes/Detail/Lookup/Models/LookupState.swift index 40f65c45c..54fef2a72 100644 --- a/Zotero/Scenes/Detail/Lookup/Models/LookupState.swift +++ b/Zotero/Scenes/Detail/Lookup/Models/LookupState.swift @@ -9,46 +9,43 @@ import Foundation struct LookupState: ViewModelState { - struct LookupData { - enum State { - case enqueued - case inProgress - case failed - case translated(TranslatedLookupData) - } - - let identifier: String - let state: State - } - - struct TranslatedLookupData { - let response: ItemResponse - let attachments: [(Attachment, URL)] - } + typealias LookupData = IdentifierLookupController.LookupData + typealias TranslatedLookupData = LookupData.State.TranslatedLookupData enum State { case failed(Swift.Error) + case waitingInput case loadingIdentifiers case lookup([LookupData]) } - enum Error: Swift.Error { - case noIdentifiersDetected + enum Error: Swift.Error, LocalizedError { + case noIdentifiersDetectedAndNoLookupData + case noIdentifiersDetectedWithLookupData + + var errorDescription: String? { + switch self { + case .noIdentifiersDetectedAndNoLookupData: + return L10n.Errors.Lookup.noIdentifiersAndNoLookupData + + case .noIdentifiersDetectedWithLookupData: + return L10n.Errors.Lookup.noIdentifiersWithLookupData + } + } } let collectionKeys: Set let libraryId: LibraryIdentifier - // If enabled, when `lookup(identifier:)` is called, previous identifiers won't be removed. - let multiLookupEnabled: Bool + let restoreLookupState: Bool let hasDarkBackground: Bool var lookupState: State - init(multiLookupEnabled: Bool, hasDarkBackground: Bool, collectionKeys: Set, libraryId: LibraryIdentifier) { - self.multiLookupEnabled = multiLookupEnabled + init(restoreLookupState: Bool, hasDarkBackground: Bool, collectionKeys: Set, libraryId: LibraryIdentifier) { + self.restoreLookupState = restoreLookupState self.collectionKeys = collectionKeys self.libraryId = libraryId - self.lookupState = .loadingIdentifiers + self.lookupState = .waitingInput self.hasDarkBackground = hasDarkBackground } diff --git a/Zotero/Scenes/Detail/Lookup/Models/ManualLookupState.swift b/Zotero/Scenes/Detail/Lookup/Models/ManualLookupState.swift index 2e7001cec..4d37ca9e5 100644 --- a/Zotero/Scenes/Detail/Lookup/Models/ManualLookupState.swift +++ b/Zotero/Scenes/Detail/Lookup/Models/ManualLookupState.swift @@ -10,8 +10,10 @@ import Foundation struct ManualLookupState: ViewModelState { var scannedText: String? + let restoreLookupState: Bool - init() { + init(restoreLookupState: Bool) { + self.restoreLookupState = restoreLookupState } mutating func cleanup() { diff --git a/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift b/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift index b869dce0c..ccee37d2b 100644 --- a/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift +++ b/Zotero/Scenes/Detail/Lookup/ViewModels/LookupActionHandler.swift @@ -11,90 +11,92 @@ 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 translatorsController: TranslatorsAndStylesController - private unowned let schemaController: SchemaController - private unowned let dateParser: DateParser - private unowned let remoteFileDownloader: RemoteAttachmentDownloader + private unowned let identifierLookupController: IdentifierLookupController 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 - self.translatorsController = translatorsController - self.schemaController = schemaController - self.dateParser = dateParser - self.remoteFileDownloader = remoteFileDownloader + init(identifierLookupController: IdentifierLookupController) { + self.identifierLookupController = identifierLookupController self.disposeBag = DisposeBag() } func process(action: LookupAction, in viewModel: ViewModel) { switch action { - case .initialize(let webView): - let handler = LookupWebViewHandler(webView: webView, translatorsController: self.translatorsController) - self.lookupWebViewHandler = handler - - handler.observable - .observe(on: MainScheduler.instance) - .subscribe(with: viewModel, onNext: { [weak self] viewModel, result in - self?.process(result: result, in: viewModel) - }) - .disposed(by: self.disposeBag) - - self.remoteFileDownloader.observable - .observe(on: MainScheduler.instance) - .subscribe(with: viewModel, onNext: { [weak self] viewModel, update in - switch update.kind { - case .ready(let attachment): - self?.finish(download: update.download, attachment: attachment, in: viewModel) - case .cancelled, .failed, .progress: break - } - }) - .disposed(by: self.disposeBag) + case .initialize: + let collectionKeys = viewModel.state.collectionKeys + let libraryId = viewModel.state.libraryId + initialize(with: collectionKeys, in: libraryId) case .lookUp(let identifier): self.lookUp(identifier: identifier, in: viewModel) - } - } - - 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) + + case .cancelAllLookups: + identifierLookupController.cancelAllLookups() + self.update(viewModel: viewModel) { state in + state.lookupState = .waitingInput } } - } - - private func process(result: Result, in viewModel: ViewModel) { - switch result { - case .success(let data): - self.process(data: data, in: viewModel) - - case .failure(let error): - DDLogError("LookupActionHandler: lookup failed - \(error)") - self.update(viewModel: viewModel) { state in - state.lookupState = .failed(error) + + func initialize(with collectionKeys: Set, in libraryId: LibraryIdentifier) { + identifierLookupController.initialize(libraryId: libraryId, collectionKeys: collectionKeys) { [weak self] lookupData in + guard let self, let lookupData else { + DDLogError("LookupActionHandler: can't create observer") + return + } + if viewModel.state.restoreLookupState, !lookupData.isEmpty { + DDLogInfo("LookupActionHandler: restoring lookup state") + self.update(viewModel: viewModel) { state in + state.lookupState = .lookup(lookupData) + } + } + self.identifierLookupController.observable + .observe(on: MainScheduler.instance) + .subscribe(with: viewModel) { [weak self] viewModel, update in + guard let self else { return } + switch viewModel.state.lookupState { + case .failed, .waitingInput: + // Ignore identifier lookup controller updates if waiting for input + return + + default: + break + } + switch update.kind { + case .lookupError(let error): + self.update(viewModel: viewModel) { state in + state.lookupState = .failed(error) + } + + case .identifiersDetected(let identifiers): + if identifiers.isEmpty { + if update.lookupData.isEmpty { + self.update(viewModel: viewModel) { state in + state.lookupState = .failed(LookupState.Error.noIdentifiersDetectedAndNoLookupData) + } + } else { + self.update(viewModel: viewModel) { state in + state.lookupState = .failed(LookupState.Error.noIdentifiersDetectedWithLookupData) + } + } + return + } + self.update(viewModel: viewModel) { state in + state.lookupState = .lookup(update.lookupData) + } + + case .lookupInProgress, .lookupFailed, .parseFailed, .itemCreationFailed, .itemStored, .pendingAttachments: + self.update(viewModel: viewModel) { state in + state.lookupState = .lookup(update.lookupData) + } + + case .finishedAllLookups: + break + } + } + .disposed(by: self.disposeBag) } } } @@ -106,144 +108,18 @@ final class LookupActionHandler: ViewModelActionHandler, BackgroundDbProcessingA guard !newIdentifier.isEmpty else { return } - if !viewModel.state.multiLookupEnabled { - self.update(viewModel: viewModel) { state in - state.lookupState = .loadingIdentifiers - } - } - - self.lookupWebViewHandler?.lookUp(identifier: newIdentifier) - } - - private func process(data: LookupWebViewHandler.LookupData, in viewModel: ViewModel) { - switch data { - case .identifiers(let identifiers): - guard !identifiers.isEmpty else { - self.update(viewModel: viewModel) { state in - state.lookupState = .failed(LookupState.Error.noIdentifiersDetected) - } - return - } - - var lookupData = identifiers.map({ LookupState.LookupData(identifier: self.identifier(from: $0), state: .enqueued) }) - - self.update(viewModel: viewModel) { state in - if !state.multiLookupEnabled { - state.lookupState = .lookup(lookupData) - } else { - switch state.lookupState { - case .lookup(let data): - lookupData.append(contentsOf: data) - default: break - } - - state.lookupState = .lookup(lookupData) - } - } - - case .item(let data): - guard let lookupId = data["identifier"] as? [String: String] else { return } - let identifier = self.identifier(from: lookupId) - - if data.keys.count == 1 { - self.update(lookupData: LookupState.LookupData(identifier: identifier, state: .inProgress), in: viewModel) - return - } - - if let error = data["error"] { - DDLogError("LookupActionHandler: \(identifier) lookup failed - \(error)") - self.update(lookupData: LookupState.LookupData(identifier: identifier, state: .failed), in: viewModel) - return - } - - guard let itemData = data["data"] as? [[String: Any]], let item = itemData.first, let parsedData = self.parse(item, viewModel: viewModel, schemaController: self.schemaController) else { - self.update(lookupData: LookupState.LookupData(identifier: identifier, state: .failed), in: viewModel) - 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) - } - } - } - } - } - - private func update(lookupData: LookupState.LookupData, in viewModel: ViewModel) { switch viewModel.state.lookupState { - case .lookup(let oldData): - var newData = oldData - guard let index = oldData.firstIndex(where: { $0.identifier == lookupData.identifier }) else { return } - newData[index] = lookupData + case .waitingInput, .failed: self.update(viewModel: viewModel) { state in - state.lookupState = .lookup(newData) + state.lookupState = .loadingIdentifiers } - - default: break + + case .loadingIdentifiers, .lookup: + break } - } - - private func identifier(from data: [String: String]) -> String { - var result = "" - for (key, value) in data { - result += key + ":" + value - } - 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 - /// - returns: `ItemResponse` of parsed item and optional attachment dictionary with title and url. - private func parse(_ itemData: [String: Any], viewModel: ViewModel, schemaController: SchemaController) -> LookupState.TranslatedLookupData? { + let collectionKeys = viewModel.state.collectionKeys let libraryId = viewModel.state.libraryId - - do { - let item = try ItemResponse(translatorResponse: itemData, schemaController: self.schemaController).copy(libraryId: libraryId, collectionKeys: collectionKeys, tags: []) - - let attachments = ((itemData["attachments"] as? [[String: Any]]) ?? []).compactMap { data -> (Attachment, URL)? in - // We can't process snapshots yet, so ignore all text/html attachments - guard let mimeType = data["mimeType"] as? String, mimeType != "text/html", let ext = mimeType.extensionFromMimeType, - let urlString = data["url"] as? String, let url = URL(string: urlString) else { return nil } - - let key = KeyGenerator.newKey - let filename = FilenameFormatter.filename(from: item, defaultTitle: "Full Text", ext: ext, dateParser: self.dateParser) - let attachment = Attachment(type: .file(filename: filename, contentType: mimeType, location: .local, linkType: .importedFile), title: filename, key: key, libraryId: libraryId) - - return (attachment, url) - } - - return LookupState.TranslatedLookupData(response: item, attachments: attachments) - } catch let error { - DDLogError("LookupActionHandler: can't parse data - \(error)") - return nil - } + identifierLookupController.lookUp(libraryId: libraryId, collectionKeys: collectionKeys, identifier: newIdentifier) } } diff --git a/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift b/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift index e4bdec218..0701021fc 100644 --- a/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift +++ b/Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift @@ -7,7 +7,6 @@ // import UIKit -import WebKit import CocoaLumberjackSwift import RxSwift @@ -45,7 +44,6 @@ class LookupViewController: UIViewController { @IBOutlet private weak var tableViewHeight: NSLayoutConstraint! @IBOutlet private weak var errorLabel: UILabel! - weak var webView: WKWebView? private var dataSource: UITableViewDiffableDataSource! private var contentSizeObserver: NSKeyValueObservation? var dataReloaded: (() -> Void)? @@ -57,7 +55,12 @@ class LookupViewController: UIViewController { private unowned let schemaController: SchemaController private let disposeBag: DisposeBag - init(viewModel: ViewModel, remoteDownloadObserver: PublishSubject, remoteFileDownloader: RemoteAttachmentDownloader, schemaController: SchemaController) { + init( + viewModel: ViewModel, + remoteDownloadObserver: PublishSubject, + remoteFileDownloader: RemoteAttachmentDownloader, + schemaController: SchemaController + ) { self.viewModel = viewModel self.remoteFileDownloader = remoteFileDownloader self.schemaController = schemaController @@ -85,9 +88,7 @@ class LookupViewController: UIViewController { }) .disposed(by: self.disposeBag) - if let webView = self.webView { - self.viewModel.process(action: .initialize(webView)) - } + self.viewModel.process(action: .initialize) } deinit { @@ -98,13 +99,19 @@ class LookupViewController: UIViewController { private func update(state: LookupState) { switch state.lookupState { - case .failed: + case .failed(let error): self.tableView.isHidden = true self.activityIndicator.stopAnimating() self.activityIndicator.isHidden = true - self.errorLabel.text = L10n.Errors.lookup + self.errorLabel.text = error.localizedDescription self.errorLabel.isHidden = false + case .waitingInput: + self.tableView.isHidden = true + self.errorLabel.isHidden = true + self.activityIndicator.stopAnimating() + self.activityIndicator.isHidden = true + case .loadingIdentifiers: self.tableView.isHidden = true self.errorLabel.isHidden = true @@ -174,7 +181,10 @@ class LookupViewController: UIViewController { self.tableView.isHidden = false self.dataSource.apply(snapshot, animatingDifferences: false) - var isFirstCall = true + guard self.contentSizeObserver == nil else { + completion() + return + } // For some reason, the observer subscription has to be here, doesn't work if it's in `viewDidLoad`. self.contentSizeObserver = self.tableView.observe(\.contentSize, options: [.new]) { [weak self] _, change in guard let self = self, let value = change.newValue, value.height != self.tableViewHeight.constant else { return } @@ -183,14 +193,9 @@ class LookupViewController: UIViewController { if value.height >= self.tableView.frame.height, !self.tableView.isScrollEnabled { self.tableView.isScrollEnabled = true - self.contentSizeObserver = nil } - if isFirstCall { - completion() - } else { - isFirstCall = false - } + completion() } } @@ -216,7 +221,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 hasActiveDownload = itemIdentifiers.contains { row in switch row { case .attachment(_, let update): switch update { @@ -233,9 +240,8 @@ class LookupViewController: UIViewController { case .item: return false } - }) - - if activeDownload == nil { + } + if !hasActiveDownload { self.activeLookupsFinished?() } } diff --git a/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.swift b/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.swift index f6ae80329..2ae0ddbb3 100644 --- a/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.swift +++ b/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.swift @@ -7,13 +7,11 @@ // import UIKit -import WebKit import CocoaLumberjackSwift import RxSwift class ManualLookupViewController: UIViewController { - @IBOutlet private weak var webView: WKWebView! @IBOutlet private weak var container: UIStackView! @IBOutlet private weak var roundedContainer: UIView! @IBOutlet private weak var titleLabel: UILabel! @@ -98,18 +96,37 @@ class ManualLookupViewController: UIViewController { } private func update(state: LookupState) { - self.lookupController?.view.isHidden = false - switch state.lookupState { case .failed: + // Similar to initial state for user input, but with error message displayed. + self.lookupController?.view.isHidden = false + + self.titleLabel.isHidden = false + self.inputContainer.isHidden = false + + self.textView.isUserInteractionEnabled = true + self.scanButton.isEnabled = true + self.textView.becomeFirstResponder() + + self.setupCancelDoneBarButtons() + + case .waitingInput: + // Initial state for user input, when no lookup state has been restored. + self.lookupController?.view.isHidden = true + self.topConstraint.constant = 15 + self.titleLabel.isHidden = false self.inputContainer.isHidden = false self.textView.isUserInteractionEnabled = true self.scanButton.isEnabled = true self.textView.becomeFirstResponder() + + self.setupCancelDoneBarButtons() case .loadingIdentifiers, .lookup: + self.lookupController?.view.isHidden = false + self.titleLabel.isHidden = true self.inputContainer.isHidden = true @@ -119,34 +136,29 @@ class ManualLookupViewController: UIViewController { if self.textView.isFirstResponder { self.textView.resignFirstResponder() } - } - - switch state.lookupState { - case .failed: - self.setupCancelDoneBarButtons() - - case .loadingIdentifiers: - self.setupCloseBarButton(title: L10n.cancel) - - case .lookup(let data): - let didTranslateAll = !data.contains(where: { data in - switch data.state { - case .enqueued, .inProgress: return true - case .failed, .translated: return false - } - }) - self.setupCloseBarButton(title: didTranslateAll ? L10n.close : L10n.cancel) + + self.setupCloseCancelAllBarButtons() } } - private func setupCloseBarButton(title: String) { - self.navigationItem.rightBarButtonItem = nil + private func setupCloseCancelAllBarButtons() { + navigationItem.rightBarButtonItem = nil - let cancelItem = UIBarButtonItem(title: title, style: .plain, target: nil, action: nil) - cancelItem.rx.tap.subscribe(onNext: { [weak self] in + let fixedSpacer = UIBarButtonItem(barButtonSystemItem: .fixedSpace, target: nil, action: nil) + fixedSpacer.width = 16 + + let closeItem = UIBarButtonItem(title: L10n.close, style: .plain, target: nil, action: nil) + closeItem.rx.tap.subscribe(onNext: { [weak self] in self?.close() }).disposed(by: self.disposeBag) - self.navigationItem.leftBarButtonItem = cancelItem + + let cancelAllItem = UIBarButtonItem(title: L10n.cancelAll, style: .plain, target: nil, action: nil) + cancelAllItem.rx.tap.subscribe(onNext: { [weak self] in + self?.lookupController?.viewModel.process(action: .cancelAllLookups) + self?.close() + }).disposed(by: self.disposeBag) + + navigationItem.leftBarButtonItems = [closeItem, fixedSpacer, cancelAllItem] } private func setupCancelDoneBarButtons() { @@ -160,7 +172,7 @@ class ManualLookupViewController: UIViewController { cancelItem.rx.tap.subscribe(onNext: { [weak self] in self?.close() }).disposed(by: self.disposeBag) - self.navigationItem.leftBarButtonItem = cancelItem + self.navigationItem.leftBarButtonItems = [cancelItem] } private func updatePreferredContentSize() { @@ -172,7 +184,7 @@ class ManualLookupViewController: UIViewController { private func updateKeyboardSize(_ data: KeyboardData) { guard UIDevice.current.userInterfaceIdiom == .phone else { return } - self.additionalSafeAreaInsets = UIEdgeInsets(top: 0, left: 0, bottom: data.endFrame.height, right: 0) + self.additionalSafeAreaInsets = UIEdgeInsets(top: 0, left: 0, bottom: data.visibleHeight, right: 0) } // MARK: - Setups @@ -208,8 +220,8 @@ class ManualLookupViewController: UIViewController { } private func setupLookupController() { - guard let controller = self.coordinatorDelegate?.lookupController(multiLookupEnabled: false, hasDarkBackground: false) else { return } - controller.webView = self.webView + let restoreLookupState = self.viewModel.state.restoreLookupState + guard let controller = self.coordinatorDelegate?.lookupController(restoreLookupState: restoreLookupState, hasDarkBackground: false) else { return } controller.view.isHidden = true self.lookupController = controller @@ -257,6 +269,12 @@ class ManualLookupViewController: UIViewController { } } +extension ManualLookupViewController: IdentifierLookupPresenter { + func isPresenting() -> Bool { + lookupController?.view.isHidden == false + } +} + private final class LiveTextResponder: UIResponder, UIKeyInput { private weak var viewModel: ViewModel? diff --git a/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.xib b/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.xib index 6457bf396..6226e28fc 100644 --- a/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.xib +++ b/Zotero/Scenes/Detail/Lookup/Views/ManualLookupViewController.xib @@ -22,7 +22,6 @@ - @@ -30,14 +29,6 @@ - @@ -86,11 +77,7 @@ - - - - diff --git a/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.swift b/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.swift index fbf1ac7d4..85a8e5012 100644 --- a/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.swift +++ b/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.swift @@ -8,7 +8,6 @@ import AVFoundation import UIKit -import WebKit import CocoaLumberjackSwift import RxSwift @@ -18,7 +17,6 @@ final class ScannerViewController: UIViewController { private let sessionQueue: DispatchQueue private let disposeBag: DisposeBag - @IBOutlet private weak var webView: WKWebView! @IBOutlet private weak var barcodeContainer: UIView! @IBOutlet private weak var barcodeStackContainer: UIStackView! @IBOutlet private weak var barcodeTitleLabel: UILabel! @@ -115,8 +113,7 @@ final class ScannerViewController: UIViewController { // MARK: - Setups private func setupLookupController() { - guard let controller = self.coordinatorDelegate?.lookupController(multiLookupEnabled: true, hasDarkBackground: true) else { return } - controller.webView = self.webView + guard let controller = self.coordinatorDelegate?.lookupController(restoreLookupState: true, hasDarkBackground: true) else { return } controller.view.backgroundColor = .clear controller.view.isHidden = true self.lookupController = controller @@ -198,3 +195,9 @@ extension ScannerViewController: AVCaptureMetadataOutputObjectsDelegate { self.lookupController?.view.isHidden = false } } + +extension ScannerViewController: IdentifierLookupPresenter { + func isPresenting() -> Bool { + lookupController?.view.isHidden == false + } +} diff --git a/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.xib b/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.xib index 9c272dbd6..682a7ba4a 100644 --- a/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.xib +++ b/Zotero/Scenes/Detail/Lookup/Views/ScannerViewController.xib @@ -1,9 +1,9 @@ - + - + @@ -15,7 +15,6 @@ - @@ -23,22 +22,14 @@ - - + - +