fix: import Zotero pdf blocks (#12260)

fix: support zotero files when importing pdf annotations

deps/db/src/logseq/db/frontend/property.cljs

@@ -492,6 +492,11 @@
                                                    :hide? false
                                                    :public? true}
                                           :queryable? true}
+     :logseq.property.asset/external-file-name {:title "External file name"
+                                                :schema {:type :string
+                                                         :hide? true
+                                                         :public? false}
+                                                :queryable? false}
      :logseq.property.asset/size {:title "File Size"
                                   :schema {:type :raw-number
                                            :hide? true

deps/db/src/logseq/db/frontend/schema.cljs

@@ -37,7 +37,7 @@
          (map (juxt :major :minor)
               [(parse-schema-version x) (parse-schema-version y)])))
 
-(def version (parse-schema-version "65.15"))
+(def version (parse-schema-version "65.16"))
 
 (defn major-version
   "Return a number.

deps/graph-parser/src/logseq/graph_parser/exporter.cljs

@@ -889,10 +889,22 @@
          (apply str)
          string/trim)))
 
+;; {:url ["Complex" {:protocol "zotero", :link "select/library/items/6VCW9QFJ"}], :label [["Plain" "Dechow and Struppa - 2015 - Intertwingled.pdf"]], :full_text "[Dechow and Struppa - 2015 - Intertwingled.pdf](zotero://select/library/items/6VCW9QFJ)", :metadata ""}
+(defn- get-zotero-local-pdf-path
+  [config m]
+  (let [link (:link (second (:url m)))
+        label (second (first (:label m)))
+        id (last (string/split link #"/"))]
+    (when (and link id label)
+      (when-let [zotero-data-dir (get-in config [:zotero/settings-v2 "default" :zotero-data-directory])]
+        {:link (str "zotero://" link)
+         :path (node-path/join zotero-data-dir "storage" id label)
+         :base label}))))
+
 (defn- walk-ast-blocks
   "Walks each ast block in order to its full depth. Saves multiple ast types for
   use in build-block-tx. This walk is only done once for perf reasons"
-  [ast-blocks]
+  [config ast-blocks]
   (let [results (atom {:simple-queries []
                        :asset-links []
                        :embeds []})]
@@ -901,10 +913,15 @@
        (cond
          (and (vector? x)
               (= "Link" (first x))
-              (let [path (second (:url (second x)))]
-                (when (string? path)
-                  (or (common-config/local-relative-asset? path)
-                      (string/ends-with? path ".pdf")))))
+              (let [path-or-map (second (:url (second x)))]
+                (cond
+                  (string? path-or-map)
+                  (or (common-config/local-relative-asset? path-or-map)
+                      (string/ends-with? path-or-map ".pdf"))
+                  (and (map? path-or-map) (= "zotero" (:protocol path-or-map)) (string? (:link path-or-map)))
+                  (:link (get-zotero-local-pdf-path config (second x)))
+                  :else
+                  nil)))
          (swap! results update :asset-links conj x)
          (and (vector? x)
               (= "Macro" (first x))
@@ -1139,7 +1156,8 @@
           :logseq.property.asset/checksum (:checksum asset-data)
           :logseq.property.asset/size (:size asset-data)}
          (when-let [external-url (:external-url asset-data)]
-           {:logseq.property.asset/external-url external-url})))
+           {:logseq.property.asset/external-url external-url
+            :logseq.property.asset/external-file-name (:external-file-name asset-data)})))
 
 (defn- get-asset-block-id
   [assets path]
@@ -1190,53 +1208,58 @@
 
 (defn- <handle-assets-in-block
   "If a block contains assets, creates them as #Asset nodes in the Asset page and references them in the block."
-  [block {:keys [asset-links]} {:keys [assets ignored-assets pdf-annotation-pages]} {:keys [notify-user <get-file-stat] :as opts}]
+  [config block {:keys [asset-links]} {:keys [assets ignored-assets pdf-annotation-pages]} {:keys [notify-user <get-file-stat] :as opts}]
   (if (seq asset-links)
     (p/let [asset-maps* (p/all (map
                                 (fn [asset-link]
-                                  (p/let [path (-> asset-link second :url second)
+                                  (p/let [path* (-> asset-link second :url second)
+                                          {:keys [path link base]} (if (map? path*)
+                                                                     (get-zotero-local-pdf-path config (second asset-link))
+                                                                     {:path path*})
                                           asset-name (-> path asset-path->name)
-                                          asset-data* (when asset-name (get @assets asset-name))
-                                          _ (when (and asset-name
+                                          asset-link-or-name (or link (-> path asset-path->name))
+                                          asset-data* (when asset-link-or-name (get @assets asset-link-or-name))
+                                          _ (when (and asset-link-or-name
                                                        (not asset-data*)
                                                        (string/ends-with? path ".pdf")
                                                        (fn? <get-file-stat)) ; external pdf
                                               (->
                                                (p/let [^js stat (<get-file-stat path)]
-                                                 (swap! assets assoc asset-name
+                                                 (swap! assets assoc asset-link-or-name
                                                         {:asset-id (d/squuid)
                                                          :type "pdf"
                                                          ;; avoid using the real checksum since it could be the same with in-graph asset
                                                          :checksum "0000000000000000000000000000000000000000000000000000000000000000"
                                                          :size (.-size stat)
-                                                         :external-url path}))
+                                                         :external-url (or link path)
+                                                         :external-file-name base}))
                                                (p/catch (fn [error]
                                                           (js/console.error error)))))
-                                          asset-data (when asset-name (get @assets asset-name))]
+                                          asset-data (when asset-link-or-name (get @assets asset-link-or-name))]
                                     (if asset-data
                                       (cond
-                                        (not (get-asset-block-id assets asset-name))
-                                        (notify-user {:msg (str "Skipped creating asset " (pr-str asset-name) " because it has no asset id")
+                                        (not (get-asset-block-id assets asset-link-or-name))
+                                        (notify-user {:msg (str "Skipped creating asset " (pr-str asset-link-or-name) " because it has no asset id")
                                                       :level :error})
 
                                         ;; If asset tx is already built, no need to do it again
                                         (:asset-created? asset-data)
-                                        {:asset-name-uuid [asset-name (:asset-id asset-data)]}
+                                        {:asset-name-uuid [asset-link-or-name (:asset-id asset-data)]}
 
                                         :else
                                         (let [new-asset (merge (build-new-asset asset-data)
                                                                {:block/title (db-asset/asset-name->title (node-path/basename asset-name))
-                                                                :block/uuid (get-asset-block-id assets asset-name)}
+                                                                :block/uuid (get-asset-block-id assets asset-link-or-name)}
                                                                (when-let [metadata (not-empty (common-util/safe-read-map-string (:metadata (second asset-link))))]
                                                                  {:logseq.property.asset/resize-metadata metadata}))
-                                              pdf-annotations-tx (when (= "pdf" (path/file-ext asset-name))
-                                                                   (build-pdf-annotations-tx asset-name assets new-asset pdf-annotation-pages opts))
+                                              pdf-annotations-tx (when (= "pdf" (path/file-ext asset-link-or-name))
+                                                                   (build-pdf-annotations-tx asset-link-or-name assets new-asset pdf-annotation-pages opts))
                                               asset-tx (concat [new-asset] pdf-annotations-tx)]
                                           ;; (prn :asset-added! (node-path/basename asset-name))
                                           ;; (cljs.pprint/pprint asset-link)
                                           ;; (prn :debug :asset-tx asset-tx)
-                                          (swap! assets assoc-in [asset-name :asset-created?] true)
-                                          {:asset-name-uuid [asset-name (:block/uuid new-asset)]
+                                          (swap! assets assoc-in [asset-link-or-name :asset-created?] true)
+                                          {:asset-name-uuid [asset-link-or-name (:block/uuid new-asset)]
                                            :asset-tx asset-tx}))
                                       (do
                                         (swap! ignored-assets conj
@@ -1290,18 +1313,18 @@
     block))
 
 (defn- <build-block-tx
-  [db block* pre-blocks {:keys [page-names-to-uuids] :as per-file-state} {:keys [import-state journal-created-ats] :as options}]
+  [db config block* pre-blocks {:keys [page-names-to-uuids] :as per-file-state} {:keys [import-state journal-created-ats] :as options}]
   ;; (prn ::block-in block*)
-  (p/let [walked-ast-blocks (walk-ast-blocks (:block.temp/ast-blocks block*))
+  (p/let [walked-ast-blocks (walk-ast-blocks config (:block.temp/ast-blocks block*))
         ;; needs to come before update-block-refs to detect new property schemas
           {:keys [block properties-tx]}
           (handle-block-properties block* db page-names-to-uuids (:block/refs block*) walked-ast-blocks options)
           {block-after-built-in-props :block deadline-properties-tx :properties-tx}
           (update-block-deadline-and-scheduled block page-names-to-uuids options)
           {block-after-assets :block :keys [asset-blocks-tx]}
-          (<handle-assets-in-block block-after-built-in-props walked-ast-blocks import-state (select-keys options [:log-fn :notify-user :<get-file-stat]))
-
+          (<handle-assets-in-block config block-after-built-in-props walked-ast-blocks import-state (select-keys options [:log-fn :notify-user :<get-file-stat]))
           ;; :block/page should be [:block/page NAME]
+
           journal-page-created-at (some-> (:block/page block*) second journal-created-ats)
           prepared-block (cond-> block-after-assets
                            journal-page-created-at
@@ -1787,6 +1810,16 @@
   (when-let [nodes (seq (filter :block/name txs))]
     (swap! (:all-existing-page-uuids import-state) merge (into {} (map (juxt :block/uuid identity) nodes)))))
 
+(defn- <build-blocks-tx
+  [conn config blocks pre-blocks per-file-state tx-options]
+  (p/loop [tx-data []
+           blocks (remove :block/pre-block? blocks)]
+    (if-let [block (first blocks)]
+      (p/let [block-tx-data (<build-block-tx @conn config block pre-blocks per-file-state
+                                             tx-options)]
+        (p/recur (concat tx-data block-tx-data) (rest blocks)))
+      tx-data)))
+
 (defn <add-file-to-db-graph
   "Parse file and save parsed data to the given db graph. Options available:
 
@@ -1798,10 +1831,10 @@
 * :macros - map of macros for use with macro expansion
 * :notify-user - Displays warnings to user without failing the import. Fn receives a map with :msg
 * :log-fn - Logs messages for development. Defaults to prn"
-  [conn file content {:keys [notify-user log-fn]
-                      :or {notify-user #(println "[WARNING]" (:msg %))
-                           log-fn prn}
-                      :as *options}]
+  [conn config file content {:keys [notify-user log-fn]
+                             :or {notify-user #(println "[WARNING]" (:msg %))
+                                  log-fn prn}
+                             :as *options}]
   (p/let [options (assoc *options :notify-user notify-user :log-fn log-fn :file file)
           {:keys [pages blocks]} (extract-pages-and-blocks @conn file content options)
           tx-options (merge (build-tx-options options)
@@ -1810,19 +1843,15 @@
           ;; Build page and block txs
           {:keys [pages-tx page-properties-tx per-file-state existing-pages]} (build-pages-tx conn pages blocks tx-options)
           whiteboard-pages (->> pages-tx
-                              ;; support old and new whiteboards
+                                ;; support old and new whiteboards
                                 (filter ldb/whiteboard?)
                                 (map (fn [page-block]
                                        (-> page-block
                                            (assoc :logseq.property/ls-type :whiteboard-page)))))
           pre-blocks (->> blocks (keep #(when (:block/pre-block? %) (:block/uuid %))) set)
-          blocks-tx (p/loop [tx-data []
-                             blocks (remove :block/pre-block? blocks)]
-                      (if-let [block (first blocks)]
-                        (p/let [block-tx-data (<build-block-tx @conn block pre-blocks per-file-state
-                                                               (assoc tx-options :whiteboard? (some? (seq whiteboard-pages))))]
-                          (p/recur (concat tx-data block-tx-data) (rest blocks)))
-                        tx-data))
+
+          blocks-tx (let [tx-options' (assoc tx-options :whiteboard? (some? (seq whiteboard-pages)))]
+                      (<build-blocks-tx conn config blocks pre-blocks per-file-state tx-options'))
           {:keys [property-pages-tx property-page-properties-tx] pages-tx' :pages-tx}
           (split-pages-and-properties-tx pages-tx old-properties existing-pages (:import-state options) @(:upstream-properties tx-options))
           ;; _ (when (seq property-pages-tx) (cljs.pprint/pprint {:property-pages-tx property-pages-tx}))
@@ -1869,18 +1898,18 @@
 ;; =======================
 
 (defn- export-doc-file
-  [{:keys [path idx] :as file} conn <read-file
+  [{:keys [path idx] :as file} conn config <read-file
    {:keys [notify-user set-ui-state <export-file]
     :or {set-ui-state (constantly nil)
-         <export-file (fn <export-file [conn m opts]
-                        (<add-file-to-db-graph conn (:file/path m) (:file/content m) opts))}
+         <export-file (fn <export-file [conn config m opts]
+                        (<add-file-to-db-graph conn config (:file/path m) (:file/content m) opts))}
     :as options}]
   ;; (prn :export-doc-file path idx)
   (-> (p/let [_ (set-ui-state [:graph/importing-state :current-idx] (inc idx))
               _ (set-ui-state [:graph/importing-state :current-page] path)
               content (<read-file file)
               m {:file/path path :file/content content}]
-        (<export-file conn m (dissoc options :set-ui-state :<export-file))
+        (<export-file conn config m (dissoc options :set-ui-state :<export-file))
         ;; returning val results in smoother ui updates
         m)
       (p/catch (fn [error]
@@ -1891,9 +1920,9 @@
 (defn export-doc-files
   "Exports all user created files i.e. under journals/ and pages/.
    Recommended to use build-doc-options and pass that as options"
-  [conn *doc-files <read-file {:keys [notify-user set-ui-state]
-                               :or {set-ui-state (constantly nil) notify-user prn}
-                               :as options}]
+  [conn config *doc-files <read-file {:keys [notify-user set-ui-state]
+                                      :or {set-ui-state (constantly nil) notify-user prn}
+                                      :as options}]
   (set-ui-state [:graph/importing-state :total] (count *doc-files))
   (let [doc-files (mapv #(assoc %1 :idx %2)
                         ;; Sort files to ensure reproducible import behavior
@@ -1902,10 +1931,10 @@
                                    [(not (string/starts-with? (node-path/basename path) "hls__")) path])
                                  *doc-files)
                         (range 0 (count *doc-files)))]
-    (-> (p/loop [_file-map (export-doc-file (get doc-files 0) conn <read-file options)
+    (-> (p/loop [_file-map (export-doc-file (get doc-files 0) conn config <read-file options)
                  i 0]
           (when-not (>= i (dec (count doc-files)))
-            (p/recur (export-doc-file (get doc-files (inc i)) conn <read-file options)
+            (p/recur (export-doc-file (get doc-files (inc i)) conn config <read-file options)
                      (inc i))))
         (p/catch (fn [e]
                    (notify-user {:msg (str "Import has unexpected error:\n" (.-message e))
@@ -2145,7 +2174,7 @@
                                    <read-and-copy-asset
                                    (merge (select-keys options [:notify-user :set-ui-state :rpath-key])
                                           {:assets (get-in doc-options [:import-state :assets])}))
-        (export-doc-files conn doc-files <read-file doc-options)
+        (export-doc-files conn config doc-files <read-file doc-options)
         (export-favorites-from-config-edn conn repo-or-conn config {})
         (export-class-properties conn repo-or-conn)
         (move-top-parent-pages-to-library conn repo-or-conn)

deps/graph-parser/test/logseq/graph_parser/exporter_test.cljs

@@ -141,7 +141,7 @@
                                                                                       (dissoc options :user-config :verbose))}
                                                                 (select-keys options [:verbose])))
               files' (mapv #(hash-map :path %) files)
-              _ (gp-exporter/export-doc-files conn files' <read-file doc-options)]
+              _ (gp-exporter/export-doc-files conn {} files' <read-file doc-options)]
         {:import-state (:import-state doc-options)})
       (p/finally (fn [_]
                    (reset! gp-block/*export-to-db-graph? false)))))

src/main/frontend/components/block.cljs

@@ -280,15 +280,16 @@
         asset-height (:logseq.property.asset/height asset-block)]
     (hooks/use-effect!
      (fn []
-       (when-not (or asset-width asset-height)
-         (measure-image!
-          src
-          (fn [width height]
-            (when (nil? (:logseq.property.asset/width asset-block))
-              (property-handler/set-block-properties! (state/get-current-repo)
-                                                      (:block/uuid asset-block)
-                                                      {:logseq.property.asset/width width
-                                                       :logseq.property.asset/height height})))))
+       (when (:block/uuid asset-block)
+         (when-not (or asset-width asset-height)
+           (measure-image!
+            src
+            (fn [width height]
+              (when (nil? (:logseq.property.asset/width asset-block))
+                (property-handler/set-block-properties! (state/get-current-repo)
+                                                        (:block/uuid asset-block)
+                                                        {:logseq.property.asset/width width
+                                                         :logseq.property.asset/height height}))))))
        (fn []))
      [])
     (let [*el-ref (rum/use-ref nil)
@@ -444,7 +445,11 @@
 
 (defn- open-pdf-file
   [e block href]
-  (let [href (or (:logseq.property.asset/external-url block) href)]
+  (let [href (if-let [url (:logseq.property.asset/external-url block)]
+               (if (string/starts-with? url "zotero://")
+                 (zotero/zotero-full-path (last (string/split url #"/")) (:logseq.property.asset/external-file-name block))
+                 url)
+               href)]
     (when-let [s (or href (some-> (.-target e) (.-dataset) (.-href)))]
       (let [load$ (fn []
                     (p/let [href (or href

src/main/frontend/components/imports.cljs

@@ -412,9 +412,9 @@
                    :<read-and-copy-asset #(read-and-copy-asset repo (config/get-repo-dir repo) %1 %2 %3)
                    ;; doc file options
                    ;; Write to frontend first as writing to worker first is poor ux with slow streaming changes
-                   :<export-file (fn <export-file [conn m opts]
+                   :<export-file (fn <export-file [conn config m opts]
                                    (p/let [tx-reports
-                                           (gp-exporter/<add-file-to-db-graph conn (:file/path m) (:file/content m) opts)]
+                                           (gp-exporter/<add-file-to-db-graph conn config (:file/path m) (:file/content m) opts)]
                                      (doseq [tx-report tx-reports]
                                        (db-browser/transact! repo (:tx-data tx-report) (:tx-meta tx-report)))))}
           {:keys [files import-state]} (gp-exporter/export-file-graph repo db-conn config-file *files options)]

src/main/frontend/components/property/value.cljs

@@ -44,8 +44,9 @@
 ;; TODO: support :string editing
 (defonce string-value-on-click
   {:logseq.property.asset/external-url
-   (fn [block]
-     (state/pub-event! [:asset/dialog-edit-external-url block]))})
+   (fn [block property]
+     (when-not (string/starts-with? (get block (:db/ident property)) "zotero://")
+       (state/pub-event! [:asset/dialog-edit-external-url block])))})
 
 (defn- entity-map?
   [m]
@@ -1251,7 +1252,7 @@
                         (:db/ident property))
            [:div.w-full {:on-click (fn []
                                      (let [f (get string-value-on-click (:db/ident property))]
-                                       (f block)))}
+                                       (f block property)))}
             content]
            content)))]))
 

src/main/frontend/extensions/zotero.cljs

@@ -480,18 +480,21 @@
      :target "_blank"
      :href full-path)))
 
+(defn zotero-full-path
+  [item-key filename]
+  (str "file://"
+       (util/node-path.join
+        (setting/setting :zotero-data-directory)
+        "storage"
+        item-key
+        filename)))
+
 (rum/defc zotero-imported-file
   [item-key filename]
   (if (string/blank? (setting/setting :zotero-data-directory))
     [:p.warning "This is a zotero imported file, setting Zotero data directory would allow you to open the file in Logseq"]
     (let [filename (read-string filename)
-          full-path
-          (str "file://"
-               (util/node-path.join
-                (setting/setting :zotero-data-directory)
-                "storage"
-                item-key
-                filename))]
+          full-path (zotero-full-path item-key filename)]
       (open-button full-path))))
 
 (rum/defc zotero-linked-file

src/main/frontend/worker/db/migrate.cljs

@@ -191,7 +191,8 @@
    ["65.14" {:properties [:logseq.property.asset/external-src]}]
    ["65.15" (rename-properties {:logseq.property.asset/external-src
                                 :logseq.property.asset/external-url}
-                               {})]])
+                               {})]
+   ["65.16" {:properties [:logseq.property.asset/external-file-name]}]])
 
 (let [[major minor] (last (sort (map (comp (juxt :major :minor) db-schema/parse-schema-version first)
                                      schema-version->updates)))]

src/test/frontend/worker/rtc/gen_client_op_test.cljs

@@ -167,10 +167,10 @@
                         :block/tags :block/title :db/cardinality}]
     #_{:clj-kondo/ignore [:unresolved-symbol :invalid-arity]}
     (is (->> (me/find (subject/generate-rtc-ops-from-property-entities [ent])
-               ([:move _ {:block-uuid ?block-uuid}]
-                [:update-page _ {:block-uuid ?block-uuid}]
-                [:update _ {:block-uuid ?block-uuid :av-coll ([!av-coll-attrs . _ ...] ...)}])
-               !av-coll-attrs)
+                      ([:move _ {:block-uuid ?block-uuid}]
+                       [:update-page _ {:block-uuid ?block-uuid}]
+                       [:update _ {:block-uuid ?block-uuid :av-coll ([!av-coll-attrs . _ ...] ...)}])
+                      !av-coll-attrs)
              set
              (set/difference av-coll-attrs)
              empty?))))
@@ -183,9 +183,9 @@
                         :block/tags :block/title}]
     #_{:clj-kondo/ignore [:unresolved-symbol :invalid-arity]}
     (is (->> (me/find (subject/generate-rtc-ops-from-class-entities [ent])
-               ([:update-page _ {:block-uuid ?block-uuid}]
-                [:update _ {:block-uuid ?block-uuid :av-coll ([!av-coll-attrs . _ ...] ...)}])
-               !av-coll-attrs)
+                      ([:update-page _ {:block-uuid ?block-uuid}]
+                       [:update _ {:block-uuid ?block-uuid :av-coll ([!av-coll-attrs . _ ...] ...)}])
+                      !av-coll-attrs)
              set
              (set/difference av-coll-attrs)
              empty?))))