Fix: stuck re-index when there're multiple whiteboards have the same UUID (#6890)

* fix: keep parsing even if some whiteboards can't be transacted

* enhance: notice parse errors

* enhance: instrument parse-and-load-error

* chore: add tests for whiteboards parsing and loading

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

@@ -209,6 +209,12 @@
   [file content {:keys [verbose] :or {verbose true}}]
   (let [_ (when verbose (println "Parsing start: " file))
         {:keys [pages blocks]} (gp-util/safe-read-string content)
+        blocks (map
+                 (fn [block]
+                   (-> block
+                       (gp-util/dissoc-in [:block/parent :block/name])
+                       (gp-util/dissoc-in [:block/left :block/name])))
+                 blocks)
         serialized-page (first pages)
         ;; whiteboard edn file should normally have valid :block/original-name, :block/name, :block/uuid
         page-name (-> (or (:block/name serialized-page)

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

@@ -167,3 +167,23 @@
     (catch :default e
       (log/error :parse/read-string-failed e)
       {})))
+
+;; Copied from Medley
+;; https://github.com/weavejester/medley/blob/d1e00337cf6c0843fb6547aadf9ad78d981bfae5/src/medley/core.cljc#L22
+(defn dissoc-in
+  "Dissociate a value in a nested associative structure, identified by a sequence
+  of keys. Any collections left empty by the operation will be dissociated from
+  their containing structures."
+  ([m ks]
+   (if-let [[k & ks] (seq ks)]
+     (if (seq ks)
+       (let [v (dissoc-in (get m k) ks)]
+         (if (empty? v)
+           (dissoc m k)
+           (assoc m k v)))
+       (dissoc m k))
+     m))
+  ([m ks & kss]
+   (if-let [[ks' & kss] (seq kss)]
+     (recur (dissoc-in m ks) ks' kss)
+     (dissoc-in m ks))))

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

@@ -12,14 +12,48 @@
   "Example exported whiteboard page as an edn exportable."
   '{:blocks
     ({:block/content "foo content a",
-      :block/format :markdown},
+      :block/format :markdown
+      :block/parent {:block/uuid #uuid "16c90195-6a03-4b3f-839d-095a496d9acd"}},
      {:block/content "foo content b",
-      :block/format :markdown}),
+      :block/format :markdown
+      :block/parent {:block/uuid #uuid "16c90195-6a03-4b3f-839d-095a496d9acd"}}),
     :pages
     ({:block/format :markdown,
+      :block/name "foo"
       :block/original-name "Foo"
+      :block/uuid #uuid "16c90195-6a03-4b3f-839d-095a496d9acd"
       :block/properties {:title "my whiteboard foo"}})})
 
+(def foo-conflict-edn
+  "Example exported whiteboard page as an edn exportable."
+  '{:blocks
+    ({:block/content "foo content a",
+      :block/format :markdown},
+     {:block/content "foo content b",
+      :block/format :markdown}),
+    :pages
+    ({:block/format :markdown,
+      :block/name "foo conflicted"
+      :block/original-name "Foo conflicted"
+      :block/uuid #uuid "16c90195-6a03-4b3f-839d-095a496d9acd"})})
+
+(def bar-edn
+  "Example exported whiteboard page as an edn exportable."
+  '{:blocks
+    ({:block/content "foo content a",
+      :block/format :markdown
+      :block/parent {:block/uuid #uuid "71515b7d-b5fc-496b-b6bf-c58004a34ee3"
+                     :block/name "foo"}},
+     {:block/content "foo content b",
+      :block/format :markdown
+      :block/parent {:block/uuid #uuid "71515b7d-b5fc-496b-b6bf-c58004a34ee3"
+                     :block/name "foo"}}),
+    :pages
+    ({:block/format :markdown,
+      :block/name "bar"
+      :block/original-name "Bar"
+      :block/uuid #uuid "71515b7d-b5fc-496b-b6bf-c58004a34ee3"})})
+
 (deftest parse-file
   (testing "id properties"
     (let [conn (ldb/start-conn)]
@@ -56,7 +90,7 @@
           (catch :default _)))
       (is (= nil @deleted-page)
           "Page should not be deleted when there is unexpected failure")))
-  
+
   (testing "parsing whiteboard page"
     (let [conn (ldb/start-conn)]
       (graph-parser/parse-file conn "/whiteboards/foo.edn" (pr-str foo-edn) {})
@@ -75,7 +109,27 @@
                 :block/type "whiteboard"
                 :block/file {:file/path "/whiteboards/foo.edn"}}
                parent)
-            "parsed block in the whiteboard page has correct parent page")))))
+            "parsed block in the whiteboard page has correct parent page"))))
+
+  (testing "Loading whiteboard pages that same block/uuid should throw an error."
+    (let [conn (ldb/start-conn)]
+      (graph-parser/parse-file conn "/whiteboards/foo.edn" (pr-str foo-edn) {})
+      (is (thrown-with-msg?
+           js/Error
+           #"Conflicting upserts"
+           (graph-parser/parse-file conn "/whiteboards/foo-conflict.edn" (pr-str foo-conflict-edn) {})))))
+
+  (testing "Loading whiteboard pages should ignore the :block/name property inside :block/parent."
+    (let [conn (ldb/start-conn)]
+      (graph-parser/parse-file conn "/whiteboards/foo.edn" (pr-str foo-edn) {})
+      (graph-parser/parse-file conn "/whiteboards/bar.edn" (pr-str bar-edn) {})
+      (let [pages (d/q '[:find ?name
+                         :in $
+                         :where
+                         [?b :block/name ?name]
+                         [?b :block/type "whiteboard"]]
+                    @conn)]
+        (is (= pages #{["foo"] ["bar"]}))))))
 
 (defn- test-property-order [num-properties]
   (let [conn (ldb/start-conn)

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

@@ -8,8 +8,7 @@
             [frontend.mobile.util :as mobile-util]
             [logseq.graph-parser :as graph-parser]
             [logseq.graph-parser.util :as gp-util]
-            [logseq.graph-parser.config :as gp-config]
-            [lambdaisland.glogi :as log]))
+            [logseq.graph-parser.config :as gp-config]))
 
 (defn- page-exists-in-another-file
   "Conflict of files towards same page"
@@ -39,41 +38,37 @@
   ([repo-url file content]
    (reset-file! repo-url file content {}))
   ([repo-url file content {:keys [verbose] :as options}]
-   (try
-     (let [electron-local-repo? (and (util/electron?)
-                                     (config/local-db? repo-url))
-           file (cond
-                  (and electron-local-repo?
-                       util/win32?
-                       (utils/win32 file))
-                  file
+   (let [electron-local-repo? (and (util/electron?)
+                                   (config/local-db? repo-url))
+         file (cond
+                (and electron-local-repo?
+                     util/win32?
+                     (utils/win32 file))
+                file
 
-                  (and electron-local-repo? (or
-                                             util/win32?
-                                             (not= "/" (first file))))
-                  (str (config/get-repo-dir repo-url) "/" file)
+                (and electron-local-repo? (or
+                                           util/win32?
+                                           (not= "/" (first file))))
+                (str (config/get-repo-dir repo-url) "/" file)
 
-                  (and (mobile-util/native-android?) (not= "/" (first file)))
-                  file
+                (and (mobile-util/native-android?) (not= "/" (first file)))
+                file
 
-                  (and (mobile-util/native-ios?) (not= "/" (first file)))
-                  file
+                (and (mobile-util/native-ios?) (not= "/" (first file)))
+                file
 
-                  :else
-                  file)
-           file (gp-util/path-normalize file)
-           new? (nil? (db/entity [:file/path file]))
-           options (merge (dissoc options :verbose)
-                          {:new? new?
-                           :delete-blocks-fn (partial get-delete-blocks repo-url)
-                           :extract-options (merge
-                                             {:user-config (state/get-config)
-                                              :date-formatter (state/get-date-formatter)
-                                              :page-name-order (state/page-name-order)
-                                              :block-pattern (config/get-block-pattern (gp-util/get-format file))
-                                              :supported-formats (gp-config/supported-formats)}
-                                             (when (some? verbose) {:verbose verbose}))})]
-       (:tx (graph-parser/parse-file (db/get-db repo-url false) file content options)))
-     (catch :default e
-       (prn "Reset file failed " {:file file})
-       (log/error :exception e)))))
+                :else
+                file)
+         file (gp-util/path-normalize file)
+         new? (nil? (db/entity [:file/path file]))
+         options (merge (dissoc options :verbose)
+                        {:new? new?
+                         :delete-blocks-fn (partial get-delete-blocks repo-url)
+                         :extract-options (merge
+                                           {:user-config (state/get-config)
+                                            :date-formatter (state/get-date-formatter)
+                                            :page-name-order (state/page-name-order)
+                                            :block-pattern (config/get-block-pattern (gp-util/get-format file))
+                                            :supported-formats (gp-config/supported-formats)}
+                                           (when (some? verbose) {:verbose verbose}))})]
+     (:tx (graph-parser/parse-file (db/get-db repo-url false) file content options)))))

src/main/frontend/handler/events.cljs

@@ -57,7 +57,8 @@
             [goog.dom :as gdom]
             [logseq.db.schema :as db-schema]
             [promesa.core :as p]
-            [rum.core :as rum]))
+            [rum.core :as rum]
+            [logseq.graph-parser.config :as gp-config]))
 
 ;; TODO: should we move all events here?
 
@@ -689,6 +690,62 @@
   (p/let [_ (file-handler/alter-file repo path content {:from-disk? true})]
     (ui-handler/re-render-root!)))
 
+(rum/defcs file-id-conflict-item <
+  (rum/local false ::resolved?)
+  [state repo file data]
+  (let [resolved? (::resolved? state)
+        id (last (:assertion data))]
+    [:li {:key file}
+     [:div
+      [:a {:on-click #(js/window.apis.openPath file)} file]
+      (if @resolved?
+        [:div.flex.flex-row.items-center
+         (ui/icon "circle-check" {:style {:font-size 20}})
+         [:div.ml-1 "Resolved"]]
+        [:div
+         [:p
+          (str "It seems that another whiteboard file already has the ID \"" id
+               "\". You can fix it by changing the ID in this file with another UUID.")]
+         [:p
+          "Or, let me"
+          (ui/button "Fix"
+            :on-click (fn []
+                        (let [dir (config/get-repo-dir repo)]
+                          (p/let [content (fs/read-file dir file)]
+                            (let [new-content (string/replace content (str id) (str (random-uuid)))]
+                              (p/let [_ (fs/write-file! repo
+                                                        dir
+                                                        file
+                                                        new-content
+                                                        {})]
+                                (reset! resolved? true))))))
+            :class "inline mx-1")
+          "it."]])]]))
+
+(defmethod handle :file/parse-and-load-error [[_ repo parse-errors]]
+  (state/pub-event! [:notification/show
+                     {:content
+                      [:div
+                       [:h2.title "Oops, those files are failed to imported to your graph:"]
+                       [:ol.my-2
+                        (for [[file error] parse-errors]
+                          (let [data (ex-data error)]
+                            (cond
+                             (and (gp-config/whiteboard? file)
+                                  (= :transact/upsert (:error data))
+                                  (uuid? (last (:assertion data))))
+                             (rum/with-key (file-id-conflict-item repo file data) file)
+
+                             :else
+                             (do
+                               (state/pub-event! [:instrument {:type :file/parse-and-load-error
+                                                               :payload error}])
+                               [:li.my-1 {:key file}
+                                [:a {:on-click #(js/window.apis.openPath file)} file]
+                                [:p (.-message error)]]))))]
+                       [:p "Don't forget to re-index your graph when all the conflicts are resolved."]]
+                      :status :error}]))
+
 (defn run!
   []
   (let [chan (state/get-events-chan)]

src/main/frontend/handler/file.cljs

@@ -116,7 +116,7 @@
                           [:db/retract page-id :block/tags]]
                          opts)))
                    (file-common-handler/reset-file! repo path content (merge opts
-                                                         (when (some? verbose) {:verbose verbose}))))
+                                                                             (when (some? verbose) {:verbose verbose}))))
                  (db/set-file-content! repo path content opts))]
     (util/p-handle (write-file!)
                    (fn [_]

src/main/frontend/handler/repo.cljs

@@ -28,6 +28,7 @@
             [frontend.db.persist :as db-persist]
             [logseq.graph-parser.util :as gp-util]
             [logseq.graph-parser :as graph-parser]
+            [logseq.graph-parser.config :as gp-config]
             [electron.ipc :as ipc]
             [cljs-bean.core :as bean]
             [clojure.core.async :as async]
@@ -191,12 +192,17 @@
                                              :from-disk? true
                                              :skip-db-transact? skip-db-transact?}
                                             (when (some? verbose) {:verbose verbose}))))
+    (state/set-parsing-state! (fn [m]
+                                (update m :finished inc)))
+    @*file-tx
     (catch :default e
+      (println "Parse and load file failed: " (str (:file/path file)))
+      (js/console.error e)
       (state/set-parsing-state! (fn [m]
-                                  (update m :failed-parsing-files conj [(:file/path file) e])))))
-  (state/set-parsing-state! (fn [m]
-                              (update m :finished inc)))
-  @*file-tx)
+                                  (update m :failed-parsing-files conj [(:file/path file) e])))
+      (state/set-parsing-state! (fn [m]
+                                  (update m :finished inc)))
+      nil)))
 
 (defn- after-parse
   [repo-url files file-paths db-encrypted? re-render? re-render-opts opts graph-added-chan]
@@ -209,6 +215,9 @@
   (when re-render?
     (ui-handler/re-render-root! re-render-opts))
   (state/pub-event! [:graph/added repo-url opts])
+  (let [parse-errors (get-in @state/state [:graph/parsing-state repo-url :failed-parsing-files])]
+    (when (seq parse-errors)
+      (state/pub-event! [:file/parse-and-load-error repo-url parse-errors])))
   (state/reset-parsing-state!)
   (state/set-loading-files! repo-url false)
   (async/offer! graph-added-chan true))
@@ -240,17 +249,24 @@
       (async/go-loop [tx []]
         (if-let [item (async/<! chan)]
           (let [[idx file] item
+                whiteboard? (gp-config/whiteboard? (:file/path file))
                 yield-for-ui? (or (not large-graph?)
                                   (zero? (rem idx 10))
-                                  (<= (- total idx) 10))]
+                                  (<= (- total idx) 10)
+                                  whiteboard?)]
             (state/set-parsing-state! (fn [m]
                                         (assoc m :current-parsing-file (:file/path file))))
 
             (when yield-for-ui? (async/<! (async/timeout 1)))
 
-            (let [result (parse-and-load-file! repo-url file (select-keys opts [:new-graph? :verbose]))
-                  tx' (concat tx result)
-                  tx' (if (zero? (rem (inc idx) 100))
+            (let [opts' (select-keys opts [:new-graph? :verbose])
+                  ;; whiteboards might have conflicting block IDs so that db transaction could be failed
+                  opts' (if whiteboard?
+                          (assoc opts' :skip-db-transact? false)
+                          opts')
+                  result (parse-and-load-file! repo-url file opts')
+                  tx' (if whiteboard? tx (concat tx result))
+                  tx' (if (or whiteboard? (zero? (rem (inc idx) 100)))
                         (do (db/transact! repo-url tx' {:from-disk? true})
                             [])
                         tx')]
@@ -432,6 +448,7 @@
 (defn re-index!
   [nfs-rebuild-index! ok-handler]
   (when-let [repo (state/get-current-repo)]
+    (state/reset-parsing-state!)
     (let [dir (config/get-repo-dir repo)]
       (when-not (state/unlinked-dir? dir)
        (route-handler/redirect-to-home!)

src/main/frontend/modules/file/core.cljs

@@ -139,7 +139,7 @@
         file-path (-> (db-utils/entity file-db-id) :file/path)]
     (if (and (string? file-path) (not-empty file-path))
       (let [new-content (if (= "whiteboard" (:block/type page-block))
-                          (pr-str {:blocks (map remove-transit-ids tree)
+                          (pr-str {:blocks tree
                                    :pages (list (remove-transit-ids page-block))})
                           (tree->file-content tree {:init-level init-level}))
             files [[file-path new-content]]

src/main/frontend/modules/outliner/file.cljs

@@ -21,21 +21,22 @@
     :block/uuid
     :block/content
     :block/format
-    {:block/page      [:block/name :block/uuid]}
-    {:block/left      [:block/name :block/uuid]}
-    {:block/parent    [:block/name :block/uuid]}])
+    {:block/page      [:block/uuid]}
+    {:block/left      [:block/uuid]}
+    {:block/parent    [:block/uuid]}])
 
 (defn- cleanup-whiteboard-block
   [block]
   (if (get-in block [:block/properties :ls-type] false)
     (dissoc block
+            :db/id
             :block/uuid ;; shape block uuid is read from properties
             :block/content
             :block/format
             :block/left
             :block/page
             :block/parent) ;; these are auto-generated for whiteboard shapes
-    (dissoc block :block/page)))
+    (dissoc block :db/id :block/page)))
 
 
 (defn do-write-file!
@@ -46,7 +47,7 @@
         blocks-count (model/get-page-blocks-count repo page-db-id)]
     (if (or (and (> blocks-count 500)
                  (not (state/input-idle? repo {:diff 3000}))) ;; long page
-            ;; when this whiteboard page is just being updated 
+            ;; when this whiteboard page is just being updated
             (and whiteboard? (not (state/whiteboard-page-idle? repo page-block))))
       (async/put! (state/get-file-write-chan) [repo page-db-id])
       (let [pull-keys (if whiteboard? whiteboard-blocks-pull-keys-with-persisted-ids '[*])

src/main/frontend/ui.cljs

@@ -203,35 +203,32 @@
 (rum/defc notification-content
   [state content status uid]
   (when (and content status)
-    (let [[color-class svg]
+    (let [svg
           (case status
             :success
-            ["text-gray-900 dark:text-gray-300 "
-             [:svg.h-6.w-6.text-green-400
-              {:stroke "currentColor", :viewBox "0 0 24 24", :fill "none"}
-              [:path
-               {:d               "M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"
-                :stroke-width    "2"
-                :stroke-linejoin "round"
-                :stroke-linecap  "round"}]]]
+            [:svg.h-6.w-6.text-green-400
+             {:stroke "currentColor", :viewBox "0 0 24 24", :fill "none"}
+             [:path
+              {:d               "M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"
+               :stroke-width    "2"
+               :stroke-linejoin "round"
+               :stroke-linecap  "round"}]]
             :warning
-            ["text-gray-900 dark:text-gray-300 "
-             [:svg.h-6.w-6.text-yellow-500
-              {:stroke "currentColor", :viewBox "0 0 24 24", :fill "none"}
-              [:path
-               {:d               "M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z"
-                :stroke-width    "2"
-                :stroke-linejoin "round"
-                :stroke-linecap  "round"}]]]
-
-            ["text-red-500"
-             [:svg.h-6.w-6.text-red-500
-              {:view-box "0 0 20 20", :fill "currentColor"}
-              [:path
-               {:clip-rule "evenodd"
-                :d
-                "M18 10a8 8 0 11-16 0 8 8 0 0116 0zm-7 4a1 1 0 11-2 0 1 1 0 012 0zm-1-9a1 1 0 00-1 1v4a1 1 0 102 0V6a1 1 0 00-1-1z"
-                :fill-rule "evenodd"}]]])]
+            [:svg.h-6.w-6.text-yellow-500
+             {:stroke "currentColor", :viewBox "0 0 24 24", :fill "none"}
+             [:path
+              {:d               "M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z"
+               :stroke-width    "2"
+               :stroke-linejoin "round"
+               :stroke-linecap  "round"}]]
+
+            [:svg.h-6.w-6.text-red-500
+             {:view-box "0 0 20 20", :fill "currentColor"}
+             [:path
+              {:clip-rule "evenodd"
+               :d
+               "M18 10a8 8 0 11-16 0 8 8 0 0116 0zm-7 4a1 1 0 11-2 0 1 1 0 012 0zm-1-9a1 1 0 00-1 1v4a1 1 0 102 0V6a1 1 0 00-1-1z"
+               :fill-rule "evenodd"}]])]
       [:div.ui__notifications-content
        {:style
         (when (or (= state "exiting")
@@ -251,8 +248,7 @@
            [:div.flex-shrink-0
             svg]
            [:div.ml-3.w-0.flex-1
-            [:div.text-sm.leading-5.font-medium.whitespace-pre-line {:style {:margin 0}
-                                                                     :class color-class}
+            [:div.text-sm.leading-5.font-medium.whitespace-pre-line {:style {:margin 0}}
              content]]
            [:div.ml-4.flex-shrink-0.flex
             [:button.inline-flex.text-gray-400.focus:outline-none.focus:text-gray-500.transition.ease-in-out.duration-150.notification-close-button