Skip to content

Commit

Permalink
Retain block references on file reload
Browse files Browse the repository at this point in the history
Commit fixes a bug where block references (i.e. `:block/refs`
association in the db) are lost when the page containing the referenced
block is re-loaded due to an update of its underlying file.

Description of Bug:

The bug occurs because when a file is re-loaded to the DB from disk, all existing blocks
belonging to the file are deleted via `retractEntity`, and then blocks
from the parsed file are added. If the file had only had small changes,
the new block set will be very similar to the previous one, although
with different db/ids.

However, while new blocks with "id:: " properties will assume the UUID value
of the previous version of the block, any references to that block via
UUID will *not* be restored in the DB; they are deleted with the
retractEntity command. This results in an inconsistent DB state, where references that should
exist do not.

Description of Fix:

The 'delete-blocks-fn' passed to the graph_parser has been modified as
such:

- It now accepts a list of block uuids to *retain*; graph parser will
  pass the blocks parsed from the new file content.
- For any blocks which match a UUID in the retain list, instead of
  deleting via retractEntity, the individual attributes are deleted via
  retractAttribute (the `retract-attributes` from schema.cljs is used
  for this purpose).
  • Loading branch information
mrtracy committed Nov 16, 2022
1 parent 037ec33 commit be7e37e
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
2 changes: 1 addition & 1 deletion deps/graph-parser/src/logseq/graph_parser.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
(extract/extract-whiteboard-edn file content extract-options')

:else nil)
delete-blocks (delete-blocks-fn (first pages) file)
block-ids (map (fn [block] {:block/uuid (:block/uuid block)}) blocks)
delete-blocks (delete-blocks-fn (first pages) file block-ids)
block-refs-ids (->> (mapcat :block/refs blocks)
(filter (fn [ref] (and (vector? ref)
(= :block/uuid (first ref)))))
Expand Down
38 changes: 30 additions & 8 deletions src/main/frontend/handler/common/file.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
[frontend.db :as db]
["/frontend/utils" :as utils]
[frontend.mobile.util :as mobile-util]
[logseq.db.schema :as db-schema]
[logseq.graph-parser :as graph-parser]
[logseq.graph-parser.util :as gp-util]
[logseq.graph-parser.config :as gp-config]
Expand All @@ -19,20 +20,41 @@
(when (not= file current-file)
current-file))))

(defn- get-delete-blocks [repo-url first-page file]
(let [delete-blocks (->
(concat
(db/delete-file-blocks! repo-url file)
(when first-page (db/delete-page-blocks repo-url (:block/name first-page))))
(distinct))]
(defn- get-clear-blocks-tx
[blocks retain-uuids]
(let [tx-for-block (fn [block] (let [{uuid :block/uuid eid :db/id} block
should-retain? (and uuid (contains? retain-uuids uuid))]
(cond
should-retain?
(map (fn [attr] [:db.fn/retractAttribute eid attr]) db-schema/retract-attributes)
:else
[[:db.fn/retractEntity eid]])))]
(mapcat tx-for-block (distinct blocks)))
)

(defn- get-clear-block-tx
"Returns the transactional operations to clear blocks belonging to the given
page and file.
Blocks are by default fully deleted via retractEntity. However, a collection
of block UUIDs to retain can be passed, and any blocks with matching uuids
will instead have their attributes cleared individually via
'retractAttribute'. This will preserve block references to the retained
UUIDs."
[repo-url first-page file retain-uuid-blocks]
(let [pages-to-clear (filter some? [(db/get-file-page file) file])
blocks (mapcat (fn [page] (db/get-page-blocks-no-cache repo-url page {:pull-keys [:db/id :block/uuid]})) pages-to-clear)
retain-uuids (if (seq retain-uuid-blocks) (set (filter some? (map :block/uuid retain-uuid-blocks))) [])
tx (get-clear-blocks-tx blocks retain-uuids)]
(when-let [current-file (page-exists-in-another-file repo-url first-page file)]
(when (not= file current-file)
(let [error (str "Page already exists with another file: " current-file ", current file: " file)]
(state/pub-event! [:notification/show
{:content error
:status :error
:clear? false}]))))
delete-blocks))
tx
))

(defn reset-file!
"Main fn for updating a db with the results of a parsed file"
Expand Down Expand Up @@ -62,7 +84,7 @@
new? (nil? (db/entity [:file/path file]))
options (merge (dissoc options :verbose)
{:new? new?
:delete-blocks-fn (partial get-delete-blocks repo-url)
:delete-blocks-fn (partial get-clear-block-tx repo-url)
:extract-options (merge
{:user-config (state/get-config)
:date-formatter (state/get-date-formatter)
Expand Down
21 changes: 19 additions & 2 deletions src/test/frontend/db/model_test.cljs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
(ns frontend.db.model-test
(:require [cljs.test :refer [use-fixtures deftest is]]
(:require [cljs.test :refer [use-fixtures deftest testing is]]
[frontend.db.model :as model]
[frontend.test.helper :as test-helper :refer [load-test-files]]))
[frontend.test.helper :as test-helper :refer [load-test-files]]
[logseq.graph-parser.util.block-ref :as block-ref]
))

(use-fixtures :each {:before test-helper/start-test-db!
:after test-helper/destroy-test-db!})
Expand Down Expand Up @@ -136,6 +138,21 @@
(#'model/get-unnecessary-namespaces-name '("one/two/tree" "one" "one/two" "non nested tag" "non nested link")))
"Must be one/two one"))

(deftest refs-to-page-maintained-on-reload
(testing
"Refs to blocks on a page are retained if that page is reload."
(let [ test-uuid "16c90195-6a03-4b3f-839d-095a496d9acd"
target-page-content (str "first line\n- target block\n id:: " test-uuid)
referring-page-content (str "first line\n- " (block-ref/->block-ref test-uuid))]
(load-test-files [{:file/path "pages/target.md"
:file/content target-page-content}
{:file/path "pages/referrer.md"
:file/content referring-page-content}])
(is (= (model/get-all-referenced-blocks-uuid) [(parse-uuid test-uuid)]))
(load-test-files [{:file/path "pages/target.md"
:file/content target-page-content}])
(is (= (model/get-all-referenced-blocks-uuid) [(parse-uuid test-uuid)]))
)))



Expand Down

0 comments on commit be7e37e

Please sign in to comment.