fix: duplicated block ids in multiple files (#8668)

fix: duplicate block ids in multiple files
Co-authored-by: Gabriel Horner <[email protected]>

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

@@ -609,6 +609,12 @@
                                                (str (gp-property/colons-org "id") " " (:uuid block)))))]
                            (string/replace-first c replace-str ""))))))
 
+(defn block-exists-in-another-page?
+  [db block-uuid current-page-name]
+  (when (and db current-page-name)
+    (when-let [block-page-name (:block/name (:block/page (d/entity db [:block/uuid block-uuid])))]
+      (not= current-page-name block-page-name))))
+
 (defn extract-blocks
   "Extract headings from mldoc ast.
   Args:
@@ -617,13 +623,14 @@
     `with-id?`: If `with-id?` equals to true, all the referenced pages will have new db ids.
     `format`: content's format, it could be either :markdown or :org-mode.
     `options`: Options supported are :user-config, :block-pattern :supported-formats,
-               :extract-macros, :date-formatter and :db"
-  [blocks content with-id? format {:keys [user-config] :as options}]
+               :extract-macros, :extracted-block-ids, :date-formatter, :page-name and :db"
+  [blocks content with-id? format {:keys [user-config db page-name extracted-block-ids] :as options}]
   {:pre [(seq blocks) (string? content) (boolean? with-id?) (contains? #{:markdown :org} format)]}
   (let [encoded-content (utf8/encode content)
+        *block-ids (or extracted-block-ids (atom #{}))
+        ;; TODO: nbb doesn't support `Atom`
         [blocks body pre-block-properties]
         (loop [headings []
-               block-ids #{}
                blocks (reverse blocks)
                timestamps {}
                properties {}
@@ -639,22 +646,25 @@
                 (paragraph-timestamp-block? block)
                 (let [timestamps (extract-timestamps block)
                       timestamps' (merge timestamps timestamps)]
-                  (recur headings block-ids (rest blocks) timestamps' properties body))
+                  (recur headings (rest blocks) timestamps' properties body))
 
                 (gp-property/properties-ast? block)
                 (let [properties (extract-properties (second block) (assoc user-config :format format))]
-                  (recur headings block-ids (rest blocks) timestamps properties body))
+                  (recur headings (rest blocks) timestamps properties body))
 
                 (heading-block? block)
                 (let [block' (construct-block block properties timestamps body encoded-content format pos-meta with-id? options)
                       block'' (assoc block' :macros (extract-macros-from-ast (cons block body)))
-                      [block-ids block] (if (block-ids (:uuid block''))
-                                          [block-ids (fix-duplicate-id block'')]
-                                          [(conj block-ids (:uuid block'')) block''])]
-                  (recur (conj headings block) block-ids (rest blocks) {} {} []))
+                      block-uuid (:uuid block'')
+                      fixed-block (if (or (@*block-ids block-uuid)
+                                          (block-exists-in-another-page? db block-uuid page-name))
+                                    (fix-duplicate-id block'')
+                                    block'')]
+                  (swap! *block-ids conj (:uuid fixed-block))
+                  (recur (conj headings fixed-block) (rest blocks) {} {} []))
 
                 :else
-                (recur headings block-ids (rest blocks) timestamps properties (conj body block))))
+                (recur headings (rest blocks) timestamps properties (conj body block))))
             [(-> (reverse headings)
                  sanity-blocks-data)
              body

deps/graph-parser/src/logseq/graph_parser/extract.cljc

@@ -133,9 +133,7 @@
   (try
     (let [page (get-page-name file ast uri-encoded? filename-format)
           [page page-name _journal-day] (gp-block/convert-page-if-journal page date-formatter)
-          options' (-> options
-                       (assoc :page-name page-name
-                              :original-page-name page))
+          options' (assoc options :page-name page-name)
           blocks (->> (gp-block/extract-blocks ast content false format options')
                       (gp-block/with-parent-and-left {:block/name page-name})
                       (vec))

deps/graph-parser/test/logseq/graph_parser_test.cljs

@@ -376,3 +376,49 @@
                   (map (comp :block/name first))
                   (remove built-in-pages)
                   set))))))
+
+(deftest duplicated-ids
+  (testing "duplicated block ids in same file"
+    (let [conn (ldb/start-conn)
+          extract-block-ids (atom #{})
+          parse-opts {:extract-options {:extract-block-ids extract-block-ids}}
+          block-id #uuid "63f199bc-c737-459f-983d-84acfcda14fe"]
+      (graph-parser/parse-file conn
+                               "foo.md"
+                               "- foo
+id:: 63f199bc-c737-459f-983d-84acfcda14fe
+- bar
+id:: 63f199bc-c737-459f-983d-84acfcda14fe
+"
+                               parse-opts)
+      (let [blocks (:block/_parent (d/entity @conn [:block/name "foo"]))]
+        (is (= 2 (count blocks)))
+        (is (= 1 (count (filter #(= (:block/uuid %) block-id) blocks)))))))
+
+  (testing "duplicated block ids in multiple files"
+    (let [conn (ldb/start-conn)
+          extract-block-ids (atom #{})
+          parse-opts {:extract-options {:extract-block-ids extract-block-ids}}
+          block-id #uuid "63f199bc-c737-459f-983d-84acfcda14fe"]
+      (graph-parser/parse-file conn
+                               "foo.md"
+                               "- foo
+id:: 63f199bc-c737-459f-983d-84acfcda14fe
+bar
+- test"
+                               parse-opts)
+      (graph-parser/parse-file conn
+                               "bar.md"
+                               "- bar
+id:: 63f199bc-c737-459f-983d-84acfcda14fe
+bar
+- test
+"
+                               parse-opts)
+      (is (= "foo"
+             (-> (d/entity @conn [:block/uuid block-id])
+                 :block/page
+                 :block/name)))
+      (let [bar-block (first (:block/_parent (d/entity @conn [:block/name "bar"])))]
+        (is (some? (:block/uuid bar-block)))
+        (is (not= (:block/uuid bar-block) block-id))))))

src/main/frontend/format/block.cljs

@@ -17,7 +17,7 @@
 (defn extract-blocks
   "Wrapper around logseq.graph-parser.block/extract-blocks that adds in system state
 and handles unexpected failure."
-  [blocks content format {:keys [with-id?]
+  [blocks content format {:keys [with-id? page-name]
                           :or {with-id? true}}]
   (try
     (gp-block/extract-blocks blocks content with-id? format
@@ -25,7 +25,8 @@ and handles unexpected failure."
                               :block-pattern (config/get-block-pattern format)
                               :supported-formats (gp-config/supported-formats)
                               :db (db/get-db (state/get-current-repo))
-                              :date-formatter (state/get-date-formatter)})
+                              :date-formatter (state/get-date-formatter)
+                              :page-name page-name})
     (catch :default e
       (log/error :exception e)
       (state/pub-event! [:capture-error {:error e

src/main/frontend/handler/common/file.cljs

@@ -84,6 +84,7 @@
                                             :block-pattern (config/get-block-pattern (gp-util/get-format file))
                                             :supported-formats (gp-config/supported-formats)
                                             :uri-encoded? (boolean (mobile-util/native-platform?))
-                                            :filename-format (state/get-filename-format repo-url)}
+                                            :filename-format (state/get-filename-format repo-url)
+                                            :extracted-block-ids (:extracted-block-ids options)}
                                            (when (some? verbose) {:verbose verbose}))})]
      (:tx (graph-parser/parse-file (db/get-db repo-url false) file content options)))))

src/main/frontend/handler/editor.cljs

@@ -2007,7 +2007,7 @@
 
 (defn- block-tree->blocks
   "keep-uuid? - maintain the existing :uuid in tree vec"
-  [tree-vec format keep-uuid?]
+  [tree-vec format keep-uuid? page-name]
   (->> (outliner-core/tree-vec-flatten tree-vec)
        (map (fn [block]
               (let [content (:content block)
@@ -2015,7 +2015,7 @@
                     content* (str (if (= :markdown format) "- " "* ")
                                   (property/insert-properties format content props))
                     ast (mldoc/->edn content* (gp-mldoc/default-config format))
-                    blocks (block/extract-blocks ast content* format {})
+                    blocks (block/extract-blocks ast content* format {:page-name page-name})
                     fst-block (first blocks)
                     fst-block (if (and keep-uuid? (uuid? (:uuid block)))
                                 (assoc fst-block :block/uuid (:uuid block))
@@ -2027,8 +2027,9 @@
   "`tree-vec`: a vector of blocks.
    A block element: {:content :properties :children [block-1, block-2, ...]}"
   [tree-vec format {:keys [target-block keep-uuid?] :as opts}]
-  (let [blocks (block-tree->blocks tree-vec format keep-uuid?)
-        page-id (:db/id (:block/page target-block))
+  (let [page-id (:db/id (:block/page target-block))
+        page-name (some-> page-id (db/entity) :block/name)
+        blocks (block-tree->blocks tree-vec format keep-uuid? page-name)
         blocks (gp-block/with-parent-and-left page-id blocks)
         block-refs (->> (mapcat :block/refs blocks)
                         (set)

src/main/frontend/handler/external.cljs

@@ -87,10 +87,10 @@
           [headers parsed-blocks] (mldoc/opml->edn config data)
           ;; add empty pos metadata
           parsed-blocks (map (fn [b] [b {}]) parsed-blocks)
+          page-name (:title headers)
           parsed-blocks (->>
-                         (block/extract-blocks parsed-blocks "" :markdown {})
-                         (mapv editor/wrap-parse-block))
-          page-name (:title headers)]
+                         (block/extract-blocks parsed-blocks "" :markdown {:page-name page-name})
+                         (mapv editor/wrap-parse-block))]
       (when (not (db/page-exists? page-name))
         (page-handler/create! page-name {:redirect? false}))
       (let [page-block (db/entity [:block/name (util/page-name-sanity-lc page-name)])

src/main/frontend/handler/file.cljs

@@ -122,7 +122,7 @@
 ;; TODO: Remove this function in favor of `alter-files`
 (defn alter-file
   [repo path content {:keys [reset? re-render-root? from-disk? skip-compare? new-graph? verbose
-                             skip-db-transact?]
+                             skip-db-transact? extracted-block-ids]
                       :or {reset? true
                            re-render-root? false
                            from-disk? false
@@ -133,7 +133,8 @@
     (when-not (and config-file? (not config-valid?)) ; non-config file or valid config
       (let [opts {:new-graph? new-graph?
                   :from-disk? from-disk?
-                  :skip-db-transact? skip-db-transact?}
+                  :skip-db-transact? skip-db-transact?
+                  :extracted-block-ids extracted-block-ids}
             result (if reset?
                      (do
                        (when-not skip-db-transact?

src/main/frontend/handler/paste.cljs

@@ -26,7 +26,9 @@
   (when-let [editing-block (state/get-edit-block)]
     (let [page-id (:db/id (:block/page editing-block))
           blocks (block/extract-blocks
-                  (mldoc/->edn text (gp-mldoc/default-config format)) text format {})
+                  (mldoc/->edn text (gp-mldoc/default-config format))
+                  text format
+                  {:page-name (:block/name (db/entity page-id))})
           blocks' (gp-block/with-parent-and-left page-id blocks)]
       (editor-handler/paste-blocks blocks' {}))))
 

src/main/frontend/handler/repo.cljs

@@ -133,7 +133,7 @@
 (defonce *file-tx (atom nil))
 
 (defn- parse-and-load-file!
-  [repo-url file {:keys [new-graph? verbose skip-db-transact?]
+  [repo-url file {:keys [new-graph? verbose skip-db-transact? extracted-block-ids]
                   :or {skip-db-transact? true}}]
   (try
     (reset! *file-tx
@@ -143,7 +143,8 @@
                                      (merge {:new-graph? new-graph?
                                              :re-render-root? false
                                              :from-disk? true
-                                             :skip-db-transact? skip-db-transact?}
+                                             :skip-db-transact? skip-db-transact?
+                                             :extracted-block-ids extracted-block-ids}
                                             (when (some? verbose) {:verbose verbose}))))
     (state/set-parsing-state! (fn [m]
                                 (update m :finished inc)))
@@ -182,7 +183,8 @@
         total (count supported-files)
         large-graph? (> total 1000)
         *page-names (atom #{})
-        *page-name->path (atom {})]
+        *page-name->path (atom {})
+        *extracted-block-ids (atom #{})]
     (when (seq delete-data) (db/transact! repo-url delete-data {:delete-files? true}))
     (state/set-current-repo! repo-url)
     (state/set-parsing-state! {:total (count supported-files)})
@@ -210,7 +212,8 @@
 
             (when yield-for-ui? (async/<! (async/timeout 1)))
 
-            (let [opts' (select-keys opts [:new-graph? :verbose])
+            (let [opts' (-> (select-keys opts [:new-graph? :verbose])
+                            (assoc :extracted-block-ids *extracted-block-ids))
                   ;; whiteboards might have conflicting block IDs so that db transaction could be failed
                   opts' (if whiteboard?
                           (assoc opts' :skip-db-transact? false)