From 7b81ad253d895fcf6ff131c2709b81646f6d342a Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 8 Jun 2022 16:08:32 -0700 Subject: [PATCH] Store PDF paths as filenames instead of full URLs This fixes issues where the full URL of our directory path changes, also it moves files into documents instead of caches, and ensures PDFs are downloaded before opening. --- .../Sources/App/PDFSupport/PDFViewer.swift | 19 +++++--- .../App/PDFSupport/PDFViewerViewModel.swift | 19 +++++++- .../App/Views/LinkItemDetailView.swift | 2 +- .../WebReader/WebReaderLoadingContainer.swift | 2 +- .../CoreDataModel.xcdatamodel/contents | 2 +- .../Sources/Models/DataModels/FeedItem.swift | 7 +-- .../Sources/Models/DataModels/PDFItem.swift | 12 ++++- .../Services/DataService/DataService.swift | 5 +-- .../DataService/Mutations/SavePDF.swift | 15 ------- .../Services/DataService/OfflineSync.swift | 6 +-- .../Queries/ArticleContentQuery.swift | 44 +++++++++++-------- .../OmnivoreKit/Sources/Utils/PDFUtils.swift | 24 ++++++++++ 12 files changed, 98 insertions(+), 59 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewer.swift b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewer.swift index e4a80b185..c1ef3e84a 100644 --- a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewer.swift +++ b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewer.swift @@ -21,15 +21,14 @@ import Utils let url: URL } - let pdfURL: URL let viewModel: PDFViewerViewModel @StateObject var pdfStateObject = PDFStateObject() @State var readerView: Bool = false @State private var shareLink: ShareLink? + @State private var errorMessage: String? - init(remoteURL: URL, viewModel: PDFViewerViewModel) { - self.pdfURL = viewModel.pdfItem.localPdfURL ?? remoteURL + init(viewModel: PDFViewerViewModel) { self.viewModel = viewModel } @@ -135,12 +134,20 @@ import Utils .sheet(item: $shareLink) { ShareSheet(activityItems: [$0.url]) } + } else if let errorMessage = errorMessage { + Text(errorMessage) } else { ProgressView() .task { - let document = HighlightedDocument(url: pdfURL, viewModel: viewModel) - pdfStateObject.document = document - pdfStateObject.coordinator = PDFViewCoordinator(document: document, viewModel: viewModel) + // NOTE: the issue here is the PDF is downloaded, but saved to a URL we don't know about + // because it is changed. + if let pdfURL = await viewModel.downloadPDF(dataService: dataService) { + let document = HighlightedDocument(url: pdfURL, viewModel: viewModel) + pdfStateObject.document = document + pdfStateObject.coordinator = PDFViewCoordinator(document: document, viewModel: viewModel) + } else { + errorMessage = "Unable to download PDF." + } } } } diff --git a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift index 978063551..fc9a2f4b1 100644 --- a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift @@ -9,7 +9,6 @@ public final class PDFViewerViewModel: ObservableObject { @Published public var readerView: Bool = false public let pdfItem: PDFItem - private var storedURL: URL? var subscriptions = Set() @@ -81,4 +80,22 @@ public final class PDFViewerViewModel: ObservableObject { return components?.url } + + public var itemDownloaded: Bool { + if let localPdfURL = pdfItem.localPdfURL, FileManager.default.fileExists(atPath: localPdfURL.path) { + return true + } + return false + } + + public func downloadPDF(dataService: DataService) async -> URL? { + do { + if let localURL = try await dataService.fetchPDFData(slug: pdfItem.slug, pageURLString: pdfItem.originalArticleURL) { + return localURL + } + } catch { + print("error downloading PDF", error) + } + return nil + } } diff --git a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift index dea6b1963..79eb3a002 100644 --- a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift @@ -300,7 +300,7 @@ struct LinkItemDetailView: View { @ViewBuilder private var fixedNavBarReader: some View { if let pdfItem = viewModel.pdfItem, let pdfURL = pdfItem.pdfURL { #if os(iOS) - PDFViewer(remoteURL: pdfURL, viewModel: PDFViewerViewModel(pdfItem: pdfItem)) + PDFViewer(viewModel: PDFViewerViewModel(pdfItem: pdfItem)) .navigationBarTitleDisplayMode(.inline) #elseif os(macOS) PDFWrapperView(pdfURL: pdfURL) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderLoadingContainer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderLoadingContainer.swift index 49aea7391..adb28284f 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderLoadingContainer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderLoadingContainer.swift @@ -143,7 +143,7 @@ import Utils public var body: some View { if let item = viewModel.item, item.isReadyToRead { if let pdfItem = PDFItem.make(item: item), let urlStr = item.pageURLString, let remoteUrl = URL(string: urlStr) { - PDFViewer(remoteURL: remoteUrl, viewModel: PDFViewerViewModel(pdfItem: pdfItem)) + PDFViewer(viewModel: PDFViewerViewModel(pdfItem: pdfItem)) .navigationBarHidden(true) .navigationViewStyle(.stack) .accentColor(.appGrayTextContrast) diff --git a/apple/OmnivoreKit/Sources/Models/CoreData/CoreDataModel.xcdatamodeld/CoreDataModel.xcdatamodel/contents b/apple/OmnivoreKit/Sources/Models/CoreData/CoreDataModel.xcdatamodeld/CoreDataModel.xcdatamodel/contents index c677d228a..fec76121b 100644 --- a/apple/OmnivoreKit/Sources/Models/CoreData/CoreDataModel.xcdatamodeld/CoreDataModel.xcdatamodel/contents +++ b/apple/OmnivoreKit/Sources/Models/CoreData/CoreDataModel.xcdatamodeld/CoreDataModel.xcdatamodel/contents @@ -30,7 +30,7 @@ - + diff --git a/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift b/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift index 5b9cab98a..4147401d9 100644 --- a/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift +++ b/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift @@ -1,5 +1,6 @@ import CoreData import Foundation +import Utils public struct HomeFeedData { // TODO: rename this public let items: [NSManagedObjectID] @@ -46,11 +47,7 @@ public extension LinkedItem { var isReadyToRead: Bool { if isPDF { // If its a PDF we verify the local file is available - if let localPdfURL = localPdfURL, let url = URL(string: localPdfURL), FileManager.default.fileExists(atPath: url.path) { - return true - } else { - return false - } + return PDFUtils.exists(filename: localPDF) } // Check the state and whether we have HTML return state == "SUCCEEDED" diff --git a/apple/OmnivoreKit/Sources/Models/DataModels/PDFItem.swift b/apple/OmnivoreKit/Sources/Models/DataModels/PDFItem.swift index 3380e48f1..475f8dfb9 100644 --- a/apple/OmnivoreKit/Sources/Models/DataModels/PDFItem.swift +++ b/apple/OmnivoreKit/Sources/Models/DataModels/PDFItem.swift @@ -1,11 +1,12 @@ import CoreData import Foundation +import Utils public struct PDFItem { public let objectID: NSManagedObjectID public let itemID: String public let pdfURL: URL? - public let localPdfURL: URL? + public let localPDF: String? public let title: String public let slug: String public let readingProgress: Double @@ -22,7 +23,7 @@ public struct PDFItem { objectID: item.objectID, itemID: item.unwrappedID, pdfURL: URL(string: item.unwrappedPageURLString), - localPdfURL: item.localPdfURL.flatMap { URL(string: $0) }, + localPDF: item.localPDF, title: item.unwrappedID, slug: item.unwrappedSlug, readingProgress: item.readingProgress, @@ -33,4 +34,11 @@ public struct PDFItem { highlights: item.highlights.asArray(of: Highlight.self) ) } + + public var localPdfURL: URL? { + if let localPDF = localPDF { + return PDFUtils.localPdfURL(filename: localPDF) + } + return nil + } } diff --git a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift index b161c23c4..0d3ef1d42 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift @@ -148,11 +148,8 @@ public final class DataService: ObservableObject { switch pageScrape.contentType { case let .pdf(localUrl): linkedItem.contentReader = "PDF" - linkedItem.localPdfURL = localUrl.absoluteString linkedItem.title = PDFUtils.titleFromPdfFile(pageScrape.url) -// let thumbnailUrl = PDFUtils.thumbnailUrl(localUrl: localUrl) -// linkedItem.imageURLString = await PDFUtils.createThumbnailFor(inputUrl: localUrl, at: thumbnailUrl) - + linkedItem.localPDF = try PDFUtils.moveToLocal(url: localUrl) case let .html(html: html, title: title, iconURL: iconURL): linkedItem.contentReader = "WEB" linkedItem.originalHtml = html diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/SavePDF.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/SavePDF.swift index 43cd10ef6..dd62e5639 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/SavePDF.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/SavePDF.swift @@ -95,21 +95,6 @@ public extension DataService { } } - func uploadFileInBackground(id: String, localPdfURL: String?, url: URL, usingSession session: URLSession) -> URLSessionTask? { - if let localPdfURL = localPdfURL, let localUrl = URL(string: localPdfURL) { - var request = URLRequest(url: url) - request.httpMethod = "PUT" - request.setValue("application/pdf", forHTTPHeaderField: "content-type") - request.setValue(id, forHTTPHeaderField: "clientRequestId") - - let task = session.uploadTask(with: request, fromFile: localUrl) - return task - } else { - // TODO: How should we handle this scenario? - return nil - } - } - func saveFilePublisher(requestId: String, uploadFileId: String, url: String) async throws -> String? { enum MutationResult { case saved(requestId: String, url: String) diff --git a/apple/OmnivoreKit/Sources/Services/DataService/OfflineSync.swift b/apple/OmnivoreKit/Sources/Services/DataService/OfflineSync.swift index faa00c72a..840622f67 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/OfflineSync.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/OfflineSync.swift @@ -1,6 +1,7 @@ import CoreData import Foundation import Models +import Utils public extension DataService { internal func syncOfflineItemsWithServerIfNeeded() async throws { @@ -55,7 +56,6 @@ public extension DataService { let uploadRequest = try await uploadFileRequest(id: id, url: url) if let urlString = uploadRequest.urlString, let uploadUrl = URL(string: urlString) { try await uploadFile(id: id, localPdfURL: localPdfURL, url: uploadUrl) - // try await services.dataService.saveFilePublisher(requestId: requestId, uploadFileId: uploadFileID, url: url) } else { throw SaveArticleError.badData } @@ -112,10 +112,8 @@ public extension DataService { switch item.contentReader { case "PDF": let id = item.unwrappedID - let localPdfURL = item.localPdfURL let url = item.unwrappedPageURLString - - if let pdfUrlStr = localPdfURL, let localPdfURL = URL(string: pdfUrlStr) { + if let localPDF = item.localPDF, let localPdfURL = PDFUtils.localPdfURL(filename: localPDF) { Task { try await createPageFromPdf(id: id, localPdfURL: localPdfURL, url: url) } diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift b/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift index 81f992514..8c3b98c57 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift @@ -2,14 +2,15 @@ import CoreData import Foundation import Models import SwiftGraphQL +import Utils -extension DataService { - struct PendingLink { +public extension DataService { + internal struct PendingLink { let itemID: String let retryCount: Int } - public func prefetchPages(itemIDs: [String], username: String) async { + func prefetchPages(itemIDs: [String], username: String) async { // TODO: make this concurrent // TODO: make a non-pending page option for BG tasks for itemID in itemIDs { @@ -17,7 +18,7 @@ extension DataService { } } - func prefetchPage(pendingLink: PendingLink, username: String) async { + internal func prefetchPage(pendingLink: PendingLink, username: String) async { let content = try? await articleContent(username: username, itemID: pendingLink.itemID, useCache: false) if content?.contentStatus == .processing, pendingLink.retryCount < 7 { @@ -40,7 +41,7 @@ extension DataService { } } - public func fetchArticleContent( + func fetchArticleContent( itemID: String, username: String? = nil, requestCount: Int = 1 @@ -69,7 +70,7 @@ extension DataService { } // swiftlint:disable:next function_body_length - public func articleContent( + func articleContent( username: String, itemID: String, useCache: Bool @@ -193,7 +194,7 @@ extension DataService { return articleContent } - func persistArticleContent(item: InternalLinkedItem, htmlContent: String, highlights: [InternalHighlight]) async throws { + internal func persistArticleContent(item: InternalLinkedItem, htmlContent: String, highlights: [InternalHighlight]) async throws { try await backgroundContext.perform { [weak self] in guard let self = self else { return } let fetchRequest: NSFetchRequest = LinkedItem.fetchRequest() @@ -243,8 +244,10 @@ extension DataService { } } - func fetchPDFData(slug: String, pageURLString: String) async throws { - guard let url = URL(string: pageURLString) else { return } + func fetchPDFData(slug: String, pageURLString: String) async throws -> URL? { + guard let url = URL(string: pageURLString) else { + throw BasicError.message(messageText: "No PDF URL found") + } let result: (Data, URLResponse)? = try? await URLSession.shared.data(from: url) guard let httpResponse = result?.1 as? HTTPURLResponse, 200 ..< 300 ~= httpResponse.statusCode else { throw BasicError.message(messageText: "pdfFetch failed. no response or bad status code.") @@ -253,6 +256,11 @@ extension DataService { throw BasicError.message(messageText: "pdfFetch failed. no data received.") } + var localPdfURL: URL? + let tempPath = FileManager.default + .urls(for: .cachesDirectory, in: .userDomainMask)[0] + .appendingPathComponent(UUID().uuidString + ".pdf") + try await backgroundContext.perform { [weak self] in let fetchRequest: NSFetchRequest = LinkedItem.fetchRequest() fetchRequest.predicate = NSPredicate(format: "%K == %@", #keyPath(LinkedItem.slug), slug) @@ -263,15 +271,11 @@ extension DataService { throw BasicError.message(messageText: errorMessage) } - let subPath = UUID().uuidString + ".pdf" // linkedItem.title.isEmpty ? UUID().uuidString : linkedItem.title - - let path = FileManager.default - .urls(for: .cachesDirectory, in: .userDomainMask)[0] - .appendingPathComponent(subPath) - do { - try data.write(to: path) - linkedItem.localPdfURL = path.absoluteString + try data.write(to: tempPath) + let localPDF = try PDFUtils.moveToLocal(url: tempPath) + localPdfURL = PDFUtils.localPdfURL(filename: localPDF) + linkedItem.localPDF = localPDF try self?.backgroundContext.save() } catch { self?.backgroundContext.rollback() @@ -279,9 +283,11 @@ extension DataService { throw BasicError.message(messageText: errorMessage) } } + + return localPdfURL } - func cachedArticleContent(itemID: String) async -> ArticleContent? { + internal func cachedArticleContent(itemID: String) async -> ArticleContent? { let linkedItemFetchRequest: NSFetchRequest = LinkedItem.fetchRequest() linkedItemFetchRequest.predicate = NSPredicate( format: "id == %@", itemID @@ -307,7 +313,7 @@ extension DataService { } } - public func syncUnsyncedArticleContent(itemID: String) async { + func syncUnsyncedArticleContent(itemID: String) async { let linkedItemFetchRequest: NSFetchRequest = LinkedItem.fetchRequest() linkedItemFetchRequest.predicate = NSPredicate( format: "id == %@", itemID diff --git a/apple/OmnivoreKit/Sources/Utils/PDFUtils.swift b/apple/OmnivoreKit/Sources/Utils/PDFUtils.swift index 87f483918..5989b57e4 100644 --- a/apple/OmnivoreKit/Sources/Utils/PDFUtils.swift +++ b/apple/OmnivoreKit/Sources/Utils/PDFUtils.swift @@ -11,6 +11,30 @@ import QuickLookThumbnailing import UIKit public enum PDFUtils { + public static func moveToLocal(url: URL) throws -> String { + let subPath = UUID().uuidString + ".pdf" + let dest = FileManager.default + .urls(for: .documentDirectory, in: .userDomainMask)[0] + .appendingPathComponent(subPath) + + try FileManager.default.moveItem(at: url, to: dest) + return subPath + } + + public static func localPdfURL(filename: String) -> URL? { + let url = FileManager.default + .urls(for: .documentDirectory, in: .userDomainMask)[0] + .appendingPathComponent(filename) + return url + } + + public static func exists(filename: String?) -> Bool { + if let filename = filename, let localPdfURL = localPdfURL(filename: filename) { + return FileManager.default.fileExists(atPath: localPdfURL.absoluteString) + } + return false + } + public static func titleFromPdfFile(_ urlStr: String) -> String { let url = URL(string: urlStr) if let url = url {