Merge pull request #2999 from omnivore-app/fix/ios-pdf-downloads

iOS: fix for PDF downloads, always use the download signed URL, not the original URL
2026-03-11 08:54:26 +00:00 · 2023-10-24 13:22:51 +08:00 · 2023-10-24 13:22:51 +08:00 · 87fbeb54e9
commit 87fbeb54e9
parent 0814feb974 f1b7ac9b21
18 changed files with 118 additions and 43 deletions
--- a/android/Omnivore/app/build.gradle
+++ b/android/Omnivore/app/build.gradle
@ -17,8 +17,8 @@ android {
        applicationId "app.omnivore.omnivore"
        minSdk 26
        targetSdk 33
-        versionCode 118
-        versionName "0.0.118"
+        versionCode 122
+        versionName "0.0.122"

        testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
        vectorDrawables {
--- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SavedItemQuery.kt
+++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SavedItemQuery.kt
@ -1,9 +1,15 @@
 package app.omnivore.omnivore.networking

+import android.util.Log
 import app.omnivore.omnivore.graphql.generated.GetArticleQuery
+import app.omnivore.omnivore.graphql.generated.type.ContentReader
 import app.omnivore.omnivore.persistence.entities.SavedItem
 import app.omnivore.omnivore.persistence.entities.SavedItemLabel
 import app.omnivore.omnivore.persistence.entities.Highlight
+import java.io.File
+import java.net.URL
+import java.nio.file.Files
+import java.nio.file.StandardCopyOption

 data class SavedItemQueryResponse(
  val item: SavedItem?,
@ -60,7 +66,18 @@ suspend fun Networker.savedItem(slug: String): SavedItemQueryResponse {
      )
    }

-    // TODO: handle errors
+    var localPDFPath: String? = null
+    if (article.articleFields.contentReader == ContentReader.PDF) {
+      // download the PDF and save it locally
+      // article.articleFields.url
+
+      val localFile = File.createTempFile("pdf-" + article.articleFields.id, ".pdf", )
+      val url = URL(article.articleFields.url)
+      Log.d("pdf", "creating local file: $localFile")
+
+      url.openStream().use { Files.copy(it, localFile.toPath(), StandardCopyOption.REPLACE_EXISTING) }
+      localPDFPath = localFile.toPath().toString()
+    }

    val savedItem = SavedItem(
      savedItemId = article.articleFields.id,
@ -82,7 +99,8 @@ suspend fun Networker.savedItem(slug: String): SavedItemQueryResponse {
      isArchived = article.articleFields.isArchived,
      contentReader = article.articleFields.contentReader.rawValue,
      content = article.articleFields.content,
-      wordsCount = article.articleFields.wordsCount
+      wordsCount = article.articleFields.wordsCount,
+      localPDFPath = localPDFPath
    )

    return SavedItemQueryResponse(item = savedItem, highlights, labels = savedItemLabels, state = article.articleFields.state?.rawValue ?: "")
--- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/AppDatabase.kt
+++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/AppDatabase.kt
@ -13,7 +13,7 @@ import app.omnivore.omnivore.persistence.entities.*
    SavedItemAndSavedItemLabelCrossRef::class,
    SavedItemAndHighlightCrossRef::class
  ],
-  version = 11
+  version = 12
 )
 abstract class AppDatabase : RoomDatabase() {
  abstract fun viewerDao(): ViewerDao
--- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/entities/SavedItem.kt
+++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/entities/SavedItem.kt
@ -40,7 +40,8 @@ data class SavedItem(
  @ColumnInfo(typeAffinity = ColumnInfo.BLOB) val pdfData: ByteArray? = null,
  var serverSyncStatus: Int = 0,
  val tempPDFURL: String? = null,
-  val wordsCount: Int? = null
+  val wordsCount: Int? = null,
+  val localPDFPath: String? = null

 // hasMany highlights
 // hasMany labels
--- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/PDFReaderViewModel.kt
+++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/PDFReaderViewModel.kt
@ -7,7 +7,7 @@ import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import app.omnivore.omnivore.DatastoreRepository
-import app.omnivore.omnivore.EventTracker
+import app.omnivore.omnivore.dataService.DataService
 import app.omnivore.omnivore.graphql.generated.type.CreateHighlightInput
 import app.omnivore.omnivore.graphql.generated.type.MergeHighlightInput
 import app.omnivore.omnivore.graphql.generated.type.UpdateHighlightInput
@ -21,11 +21,16 @@ import com.pspdfkit.document.download.DownloadJob
 import com.pspdfkit.document.download.DownloadRequest
 import com.pspdfkit.document.download.Progress
 import dagger.hilt.android.lifecycle.HiltViewModel
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import org.json.JSONObject
 import java.io.File
 import java.lang.Double.max
 import java.lang.Double.min
+import java.lang.Exception
+import java.net.URLEncoder
+import java.nio.file.FileSystem
 import java.util.*
 import javax.inject.Inject

@ -38,8 +43,8 @@ data class PDFReaderParams(
@HiltViewModel
 class PDFReaderViewModel @Inject constructor(
  private val datastoreRepo: DatastoreRepository,
-  private val networker: Networker,
-  private val eventTracker: EventTracker,
+  private val dataService: DataService,
+  private val networker: Networker
 ): ViewModel() {
  var annotationUnderNoteEdit: Annotation? = null
  val pdfReaderParamsLiveData = MutableLiveData<PDFReaderParams?>(null)
@ -47,21 +52,51 @@ class PDFReaderViewModel @Inject constructor(

  fun loadItem(slug: String, context: Context) {
    viewModelScope.launch {
+      loadItemFromDB(slug)
+      loadItemFromNetwork(slug, context)
+    }
+  }
+
+  private suspend fun loadItemFromDB(slug: String) {
+    withContext(Dispatchers.IO) {
+      val persistedItem = dataService.db.savedItemDao().getSavedItemWithLabelsAndHighlights(slug)
+      persistedItem?.let { persistedItem ->
+        persistedItem?.savedItem?.localPDF?.let { localPDF ->
+          val localFile = File(localPDF)
+
+          if (localFile.exists()) {
+            val articleContent = ArticleContent(
+              title = persistedItem.savedItem.title,
+              htmlContent = "",
+              highlights = persistedItem.highlights,
+              contentStatus = "SUCCEEDED",
+              objectID = "",
+              labelsJSONString = Gson().toJson(persistedItem.labels)
+            )
+
+            pdfReaderParamsLiveData.postValue(
+              PDFReaderParams(
+                persistedItem.savedItem,
+                articleContent,
+                Uri.fromFile(localFile)
+              )
+            )
+          }
+        }
+      }
+    }
+  }
+
+  private suspend fun loadItemFromNetwork(slug: String, context: Context) {
+    withContext(Dispatchers.IO) {
      val articleQueryResult = networker.savedItem(slug)
-
-      val article = articleQueryResult.item ?: return@launch
-
+      val article = articleQueryResult.item ?: return@withContext
      val request = DownloadRequest.Builder(context)
        .uri(article.pageURLString)
        .build()

      val job = DownloadJob.startDownload(request)
-
      job.setProgressListener(object : DownloadJob.ProgressListenerAdapter() {
-        override fun onProgress(progress: Progress) {
-//          progressBar.setProgress((100f * progress.bytesReceived / progress.totalBytes).toInt())
-        }
-
        override fun onComplete(output: File) {
          val articleContent = ArticleContent(
            title = article.title,
@ -72,22 +107,18 @@ class PDFReaderViewModel @Inject constructor(
            labelsJSONString = Gson().toJson(articleQueryResult.labels)
          )

-          val pdfReaderParams = PDFReaderParams(article, articleContent, Uri.fromFile(output))
-
-          eventTracker.track("link_read",
-            com.posthog.android.Properties()
-              .putValue("linkID", pdfReaderParams.item.savedItemId)
-              .putValue("slug", pdfReaderParams.item.slug)
-              .putValue("originalArticleURL", pdfReaderParams.item.pageURLString)
-              .putValue("loaded_from", "network")
-          )
-
          currentReadingProgress = article.readingProgress
-          pdfReaderParamsLiveData.postValue(pdfReaderParams)
+          pdfReaderParamsLiveData.postValue(
+            PDFReaderParams(
+              article,
+              articleContent,
+              Uri.fromFile(output)
+            )
+          )
        }

        override fun onError(exception: Throwable) {
-//          handleDownloadError(exception)
+//      handleDownloadError(exception)
        }
      })
    }
--- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/WebReaderContent.kt
+++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/WebReaderContent.kt
@ -91,7 +91,7 @@ data class WebReaderContent(
                  url: `${item.pageURLString}`,
                  title: `${articleContent.title.replace("`", "\\`")}`,
                  content: document.getElementById('_omnivore-htmlContent').innerHTML,
-                  originalArticleUrl: "${item.pageURLString}",
+                  originalArticleUrl: "${item.publisherURLString}",
                  contentReader: "WEB",
                  readingProgressPercent: ${item.readingProgress},
                  readingProgressAnchorIndex: ${item.readingProgressAnchor},
--- a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift
+++ b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift
@ -96,7 +96,18 @@ final class PDFViewerViewModel: ObservableObject {
        }
      }

-      return try await dataService.loadPDFData(slug: pdfItem.slug, pageURLString: pdfItem.originalArticleURL)
+      if let result = try? await dataService.loadPDFData(slug: pdfItem.slug, downloadURL: pdfItem.downloadURL) {
+        return result
+      }
+
+      // Downloading failed, try to get the article again, and then download
+      if let content = try? await dataService.loadArticleContentWithRetries(itemID: pdfItem.itemID, username: "me") {
+        // refetched the content, now try one more time then throw
+        if let result = try await dataService.loadPDFData(slug: pdfItem.slug, downloadURL: content.downloadURL) {
+          return result
+        }
+      }
+      return nil
    } catch {
      print("error downloading PDF", error)
      return nil
--- a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift
+++ b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift
@ -46,7 +46,7 @@ import Views
  private func trackReadEvent() {
    guard let itemID = item?.unwrappedID ?? pdfItem?.itemID else { return }
    guard let slug = item?.unwrappedSlug ?? pdfItem?.slug else { return }
-    guard let originalArticleURL = item?.unwrappedPageURLString ?? pdfItem?.originalArticleURL else { return }
+    guard let originalArticleURL = item?.unwrappedPageURLString ?? pdfItem?.downloadURL else { return }

    EventTracker.track(
      .linkRead(
--- a/apple/OmnivoreKit/Sources/Models/CoreData/CoreDataModel.xcdatamodeld/CoreDataModel.xcdatamodel/contents
+++ b/apple/OmnivoreKit/Sources/Models/CoreData/CoreDataModel.xcdatamodeld/CoreDataModel.xcdatamodel/contents
@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="21513" systemVersion="21G531" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier="">
+<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="22225" systemVersion="22G74" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier="">
    <entity name="Highlight" representedClassName="Highlight" syncable="YES" codeGenerationType="class">
        <attribute name="annotation" optional="YES" attributeType="String"/>
        <attribute name="color" optional="YES" attributeType="String"/>
@ -32,6 +32,7 @@
        <attribute name="createdAt" attributeType="Date" usesScalarValueType="NO"/>
        <attribute name="createdId" optional="YES" attributeType="String"/>
        <attribute name="descriptionText" optional="YES" attributeType="String"/>
+        <attribute name="downloadURL" optional="YES" attributeType="String"/>
        <attribute name="htmlContent" optional="YES" attributeType="String"/>
        <attribute name="id" attributeType="String"/>
        <attribute name="imageURLString" optional="YES" attributeType="String"/>
--- a/apple/OmnivoreKit/Sources/Models/DataModels/ArticleContent.swift
+++ b/apple/OmnivoreKit/Sources/Models/DataModels/ArticleContent.swift
@ -16,19 +16,22 @@ public struct ArticleContent {
  public let highlightsJSONString: String
  public let contentStatus: ArticleContentStatus
  public let objectID: NSManagedObjectID?
+  public let downloadURL: String

  public init(
    title: String,
    htmlContent: String,
    highlightsJSONString: String,
    contentStatus: ArticleContentStatus,
-    objectID: NSManagedObjectID?
+    objectID: NSManagedObjectID?,
+    downloadURL: String
  ) {
    self.title = title
    self.htmlContent = htmlContent
    self.highlightsJSONString = highlightsJSONString
    self.contentStatus = contentStatus
    self.objectID = objectID
+    self.downloadURL = downloadURL
  }
 }

--- a/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift
+++ b/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift
@ -56,12 +56,14 @@ public struct JSONArticle: Decodable {
  public let isArchived: Bool
  public let language: String?
  public let wordsCount: Int?
+  public let downloadURL: String
 }

 public extension LinkedItem {
  var unwrappedID: String { id ?? "" }
  var unwrappedSlug: String { slug ?? "" }
  var unwrappedTitle: String { title ?? "" }
+  var unwrappedDownloadURLString: String { downloadURL ?? "" }
  var unwrappedPageURLString: String { pageURLString ?? "" }
  var unwrappedSavedAt: Date { savedAt ?? Date() }
  var unwrappedCreatedAt: Date { createdAt ?? Date() }
--- a/apple/OmnivoreKit/Sources/Models/DataModels/PDFItem.swift
+++ b/apple/OmnivoreKit/Sources/Models/DataModels/PDFItem.swift
@ -14,7 +14,7 @@ public struct PDFItem {
  public let readingProgressAnchor: Int
  public let isArchived: Bool
  public let isRead: Bool
-  public let originalArticleURL: String
+  public let downloadURL: String
  public let highlights: [Highlight]

  public static func make(item: LinkedItem) -> PDFItem? {
@ -32,7 +32,7 @@ public struct PDFItem {
      readingProgressAnchor: Int(item.readingProgressAnchor),
      isArchived: item.isArchived,
      isRead: item.isRead,
-      originalArticleURL: item.unwrappedPageURLString,
+      downloadURL: item.unwrappedPageURLString,
      highlights: item.highlights.asArray(of: Highlight.self)
    )
  }
--- a/apple/OmnivoreKit/Sources/Services/DataService/ContentLoading.swift
+++ b/apple/OmnivoreKit/Sources/Services/DataService/ContentLoading.swift
@ -63,7 +63,8 @@ extension DataService {
      htmlContent: fetchResult.htmlContent,
      highlightsJSONString: fetchResult.highlights.asJSONString,
      contentStatus: fetchResult.item.isPDF ? .succeeded : fetchResult.item.state,
-      objectID: objectID
+      objectID: objectID,
+      downloadURL: fetchResult.item.downloadURL
    )
  }

@ -91,7 +92,8 @@ extension DataService {
          .filter { $0.serverSyncStatus != ServerSyncStatus.needsDeletion.rawValue }
          .map { InternalHighlight.make(from: $0) }.asJSONString,
        contentStatus: .succeeded,
-        objectID: linkedItem.objectID
+        objectID: linkedItem.objectID,
+        downloadURL: linkedItem.downloadURL ?? ""
      )
    }
  }
@ -166,7 +168,7 @@ extension DataService {
    }

    if articleProps.item.isPDF, needsPDFDownload {
-      _ = try await loadPDFData(slug: articleProps.item.slug, pageURLString: articleProps.item.pageURLString)
+      _ = try await loadPDFData(slug: articleProps.item.slug, downloadURL: articleProps.item.downloadURL)
    }

    try await backgroundContext.perform { [weak self] in
--- a/apple/OmnivoreKit/Sources/Services/DataService/Public/PDFLoading.swift
+++ b/apple/OmnivoreKit/Sources/Services/DataService/Public/PDFLoading.swift
@ -4,8 +4,8 @@ import Models
 import Utils

 public extension DataService {
-  func loadPDFData(slug: String, pageURLString: String) async throws -> URL? {
-    guard let url = URL(string: pageURLString) else {
+  func loadPDFData(slug: String, downloadURL: String) async throws -> URL? {
+    guard let url = URL(string: downloadURL) else {
      throw BasicError.message(messageText: "No PDF URL found")
    }

--- a/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift
+++ b/apple/OmnivoreKit/Sources/Services/DataService/Queries/ArticleContentQuery.swift
@ -45,6 +45,7 @@ extension DataService {
          originalHtml: nil,
          language: try $0.language(),
          wordsCount: try $0.wordsCount(),
+          downloadURL: try $0.url(),
          recommendations: try $0.recommendations(selection: recommendationSelection.list.nullable) ?? [],
          labels: try $0.labels(selection: feedItemLabelSelection.list.nullable) ?? []
        ),
--- a/apple/OmnivoreKit/Sources/Services/DataService/Queries/LinkedItemNetworkQuery.swift
+++ b/apple/OmnivoreKit/Sources/Services/DataService/Queries/LinkedItemNetworkQuery.swift
@ -277,6 +277,7 @@ private let libraryArticleSelection = Selection.Article {
    originalHtml: nil,
    language: try $0.language(),
    wordsCount: try $0.wordsCount(),
+    downloadURL: try $0.url(),
    recommendations: try $0.recommendations(selection: recommendationSelection.list.nullable) ?? [],
    labels: try $0.labels(selection: feedItemLabelSelection.list.nullable) ?? []
  )
@ -316,6 +317,7 @@ private let searchItemSelection = Selection.SearchItem {
    originalHtml: nil,
    language: try $0.language(),
    wordsCount: try $0.wordsCount(),
+    downloadURL: try $0.url(),
    recommendations: try $0.recommendations(selection: recommendationSelection.list.nullable) ?? [],
    labels: try $0.labels(selection: feedItemLabelSelection.list.nullable) ?? []
  )
--- a/apple/OmnivoreKit/Sources/Services/InternalModels/InternalLinkedItem.swift
+++ b/apple/OmnivoreKit/Sources/Services/InternalModels/InternalLinkedItem.swift
@ -27,6 +27,7 @@ struct InternalLinkedItem {
  let originalHtml: String?
  let language: String?
  let wordsCount: Int?
+  let downloadURL: String
  let recommendations: [InternalRecommendation]
  var labels: [InternalLinkedItemLabel]

@ -65,6 +66,7 @@ struct InternalLinkedItem {
    linkedItem.originalHtml = originalHtml
    linkedItem.language = language
    linkedItem.wordsCount = Int64(wordsCount ?? 0)
+    linkedItem.downloadURL = downloadURL

    // Remove existing labels in case a label had been deleted
    if let existingLabels = linkedItem.labels {
@ -146,6 +148,7 @@ extension JSONArticle {
      originalHtml: nil,
      language: language,
      wordsCount: wordsCount,
+      downloadURL: downloadURL,
      recommendations: [],
      labels: []
    )
--- a/package.json
+++ b/package.json
@ -2,7 +2,7 @@
  "name": "omnivore-app",
  "version": "1.0.0",
  "description": "",
-  "private": false,
+  "private": true,
  "workspaces": [
    "packages/*"
  ],
@ -40,4 +40,4 @@
    "yarn": "1.22.19"
  },
  "dependencies": {}
-}
+}