Browse Source

fix: move db-pipeline and datascript-report back to outliner dep

graph-parser is primarily a file graph dep that is maintained separately
from db graphs. We want to move as many namespaces out of it as
possible. Also moved differing count assertions to
their respective tests
Gabriel Horner 1 year ago
parent
commit
3681ac354b

+ 1 - 3
deps/graph-parser/src/logseq/graph_parser/cli.cljs

@@ -7,8 +7,7 @@
             [logseq.common.config :as common-config]
             [logseq.graph-parser :as graph-parser]
             [logseq.common.util :as common-util]
-            [logseq.graph-parser.db :as gp-db]
-            [logseq.graph-parser.db-pipeline :as db-pipeline]))
+            [logseq.graph-parser.db :as gp-db]))
 
 (defn- slurp
   "Return file contents like clojure.core/slurp"
@@ -76,7 +75,6 @@
    (let [config (read-config dir)
          files (or (:files options) (build-graph-files dir config))
          conn (or (:conn options) (gp-db/start-conn))
-         _ (db-pipeline/add-listener conn)
          _ (when-not (:files options) (println "Parsing" (count files) "files..."))
          asts (parse-files conn files (merge options {:config config}))]
      {:conn conn

+ 0 - 103
deps/graph-parser/src/logseq/graph_parser/db_pipeline.cljs

@@ -1,103 +0,0 @@
-(ns logseq.graph-parser.db-pipeline
-  "This ns provides a datascript listener for DB graphs to add additional changes
-   that the frontend also adds per transact.
-   Known limitations:
-   * Deleted blocks don't update effected :block/tx-id"
-  (:require [datascript.core :as d]
-            [clojure.set :as set]
-            [logseq.db :as ldb]
-            [logseq.graph-parser.datascript-report :as ds-report]))
-
-(defn filter-deleted-blocks
-  [datoms]
-  (keep
-   (fn [d]
-     (when (and (= :block/uuid (:a d)) (false? (:added d)))
-       (:v d)))
-   datoms))
-
-;; TODO: it'll be great if we can calculate the :block/path-refs before any
-;; outliner transaction, this way we can group together the real outliner tx
-;; and the new path-refs changes, which makes both undo/redo and
-;; react-query/refresh! easier.
-
-;; TODO: also need to consider whiteboard transactions
-
-;; Steps:
-;; 1. For each changed block, new-refs = its page + :block/refs + parents :block/refs
-;; 2. Its children' block/path-refs might need to be updated too.
-(defn- compute-block-path-refs
-  [{:keys [db-before db-after]} blocks*]
-  (let [blocks (remove :block/name blocks*)
-        *computed-ids (atom #{})]
-    (mapcat (fn [block]
-              (when (and (not (@*computed-ids (:block/uuid block))) ; not computed yet
-                         (not (:block/name block)))
-                (let [parents (ldb/get-block-parents db-after (:block/uuid block) {})
-                      parents-refs (->> (mapcat :block/path-refs parents)
-                                        (map :db/id))
-                      old-refs (if db-before
-                                 (set (map :db/id (:block/path-refs (d/entity db-before (:db/id block)))))
-                                 #{})
-                      new-refs (set (concat
-                                     (some-> (:db/id (:block/page block)) vector)
-                                     (map :db/id (:block/refs block))
-                                     parents-refs))
-                      refs-changed? (not= old-refs new-refs)
-                      children (ldb/get-block-children-ids db-after (:block/uuid block))
-                            ;; Builds map of children ids to their parent id and :block/refs ids
-                      children-maps (into {}
-                                          (map (fn [id]
-                                                 (let [entity (d/entity db-after [:block/uuid id])]
-                                                   [(:db/id entity)
-                                                    {:parent-id (get-in entity [:block/parent :db/id])
-                                                     :block-ref-ids (map :db/id (:block/refs entity))}]))
-                                               children))
-                      children-refs (map (fn [[id {:keys [block-ref-ids] :as child-map}]]
-                                           {:db/id id
-                                                  ;; Recalculate :block/path-refs as db contains stale data for this attribute
-                                            :block/path-refs
-                                            (set/union
-                                                   ;; Refs from top-level parent
-                                             new-refs
-                                                   ;; Refs from current block
-                                             block-ref-ids
-                                                   ;; Refs from parents in between top-level
-                                                   ;; parent and current block
-                                             (loop [parent-refs #{}
-                                                    parent-id (:parent-id child-map)]
-                                               (if-let [parent (children-maps parent-id)]
-                                                 (recur (into parent-refs (:block-ref-ids parent))
-                                                        (:parent-id parent))
-                                                       ;; exits when top-level parent is reached
-                                                 parent-refs)))})
-                                         children-maps)]
-                  (swap! *computed-ids set/union (set (cons (:block/uuid block) children)))
-                  (concat
-                   (when (and (seq new-refs) refs-changed?)
-                     [{:db/id (:db/id block)
-                       :block/path-refs new-refs}])
-                   children-refs))))
-            blocks)))
-
-(defn compute-block-path-refs-tx
-  [tx-report blocks]
-  (let [refs-tx (compute-block-path-refs tx-report blocks)
-        truncate-refs-tx (map (fn [m] [:db/retract (:db/id m) :block/path-refs]) refs-tx)]
-    (concat truncate-refs-tx refs-tx)))
-
-(defn- invoke-hooks
-  "Modified copy of frontend.modules.outliner.pipeline/invoke-hooks that doesn't
-  handle :block/tx-id"
-  [conn tx-report]
-  (when (not (get-in tx-report [:tx-meta :pipeline-replace?]))
-    (let [{:keys [blocks]} (ds-report/get-blocks-and-pages tx-report)
-          block-path-refs-tx (distinct (compute-block-path-refs-tx tx-report blocks))]
-      (when (seq block-path-refs-tx)
-        (d/transact! conn block-path-refs-tx {:pipeline-replace? true})))))
-
-(defn add-listener
-  "Adds a listener to the datascript connection to add additional changes from outliner.pipeline"
-  [conn]
-  (d/listen! conn :pipeline-updates (fn pipeline-updates [tx-report]
-                                      (invoke-hooks conn tx-report))))

+ 0 - 7
deps/graph-parser/src/logseq/graph_parser/test/docs_graph_helper.cljs

@@ -162,13 +162,6 @@
   ;; only increase over time as the docs graph rarely has deletions
   (testing "Counts"
     (is (= 303 (count files)) "Correct file count")
-    (is (= 63926 (count (d/datoms db :eavt))) "Correct datoms count")
-
-    (is (= 5946
-           (ffirst
-            (d/q '[:find (count ?b)
-                   :where [?b :block/path-refs ?bp] [?bp :block/name]] db)))
-        "Correct referenced blocks count")
     (is (= 23
            (ffirst
             (d/q '[:find (count ?b)

+ 5 - 3
deps/graph-parser/test/logseq/graph_parser/cli_test.cljs

@@ -30,6 +30,9 @@
 
     (docs-graph-helper/docs-graph-assertions @conn graph-dir files)
 
+    (testing "Additional counts"
+      (is (= 48225 (count (d/datoms @conn :eavt))) "Correct datoms count"))
+
     (testing "Asts"
       (is (seq asts) "Asts returned are non-zero")
       (is (= files (map :file asts))
@@ -158,7 +161,7 @@
                        datoms->entity-maps
                        (map #(assoc (or (not-empty (select-keys % [:block/content :block/name]))
                                         %)
-                                    :attributes (disj (set (keys %)) :block/file :block/format :block/path-refs)))
+                                    :attributes (disj (set (keys %)) :block/file :block/format)))
                        set)
         db-ents (->> (d/datoms graph-db :eavt)
                      datoms->entity-maps
@@ -167,8 +170,7 @@
                                   :attributes (cond-> (disj (set (keys %))
                                                             ;; Don't compare :block/format as db graphs
                                                             ;; are purposely different
-                                                            :block/format
-                                                            :block/path-refs)
+                                                            :block/format)
                                                 (seq (:block/content %))
                                                 (set/difference #{:block/created-at :block/updated-at}))))
                      set)]

+ 1 - 1
deps/outliner/.carve/ignore

@@ -1,5 +1,5 @@
 ;; API fn
-logseq.outliner.cli.pipeline/add-listener
+logseq.outliner.db-pipeline/add-listener
 ;; private
 logseq.outliner.core/*transaction-opts*
 ;; API fn

+ 1 - 1
deps/outliner/script/transact.cljs

@@ -1,6 +1,6 @@
 (ns transact
   "This script generically runs transactions against the queried blocks"
-  (:require [logseq.graph-parser.db-pipeline :as db-pipeline]
+  (:require [logseq.outliner.db-pipeline :as db-pipeline]
             [logseq.db.sqlite.db :as sqlite-db]
             [logseq.db.frontend.rules :as rules]
             [datascript.core :as d]

+ 1 - 1
deps/graph-parser/src/logseq/graph_parser/datascript_report.cljs → deps/outliner/src/logseq/outliner/datascript_report.cljs

@@ -1,4 +1,4 @@
-(ns logseq.graph-parser.datascript-report
+(ns logseq.outliner.datascript-report
   "Datascript fns related to getting data from a connection listener's tx-report"
   (:require [clojure.set :as set]
             [datascript.core :as d]))

+ 25 - 0
deps/outliner/src/logseq/outliner/db_pipeline.cljs

@@ -0,0 +1,25 @@
+(ns ^:node-only logseq.outliner.db-pipeline
+  "This ns provides a datascript listener for DB graphs to add additional changes
+   that the frontend also adds per transact.
+   Missing features from frontend.worker.pipeline including:
+   * Deleted blocks don't update effected :block/tx-id
+   * Delete empty property parent"
+  (:require [datascript.core :as d]
+            [logseq.outliner.pipeline :as outliner-pipeline]
+            [logseq.outliner.datascript-report :as ds-report]))
+
+(defn- invoke-hooks
+  "Modified copy of frontend.worker.pipeline/invoke-hooks that doesn't
+  handle :block/tx-id"
+  [conn tx-report]
+  (when (not (get-in tx-report [:tx-meta :pipeline-replace?]))
+    (let [{:keys [blocks]} (ds-report/get-blocks-and-pages tx-report)
+          block-path-refs-tx (distinct (outliner-pipeline/compute-block-path-refs-tx tx-report blocks))]
+      (when (seq block-path-refs-tx)
+        (d/transact! conn block-path-refs-tx {:pipeline-replace? true})))))
+
+(defn add-listener
+  "Adds a listener to the datascript connection to add additional changes from outliner.pipeline"
+  [conn]
+  (d/listen! conn :pipeline-updates (fn pipeline-updates [tx-report]
+                                      (invoke-hooks conn tx-report))))

+ 3 - 2
deps/outliner/src/logseq/outliner/pipeline.cljs

@@ -1,5 +1,5 @@
 (ns logseq.outliner.pipeline
-  "Core fns for use with frontend.modules.outliner.pipeline"
+  "Core fns for use with frontend worker and node"
   (:require [datascript.core :as d]
             [clojure.set :as set]
             [logseq.db :as ldb]))
@@ -77,7 +77,8 @@
             blocks)))
 
 (defn compute-block-path-refs-tx
+  "Main fn for computing path-refs"
   [tx-report blocks]
   (let [refs-tx (compute-block-path-refs tx-report blocks)
         truncate-refs-tx (map (fn [m] [:db/retract (:db/id m) :block/path-refs]) refs-tx)]
-    (concat truncate-refs-tx refs-tx)))
+    (concat truncate-refs-tx refs-tx)))

+ 1 - 1
scripts/README.md

@@ -65,4 +65,4 @@ Created graph schema!
 #### Update graph scripts
 
 For database graphs, it is recommended to use
-`logseq.graph-parser.db-pipeline/add-listener!` when updating graphs.  TODO
+`logseq.outliner.db-pipeline/add-listener!` when updating graphs.  TODO

+ 1 - 1
scripts/src/logseq/tasks/db_graph/create_graph.cljs

@@ -7,7 +7,7 @@
             [logseq.db.sqlite.util :as sqlite-util]
             [logseq.db.sqlite.create-graph :as sqlite-create-graph]
             [logseq.db.frontend.property.build :as db-property-build]
-            [logseq.graph-parser.db-pipeline :as db-pipeline]
+            [logseq.outliner.db-pipeline :as db-pipeline]
             [logseq.common.util :as common-util]
             [clojure.string :as string]
             [clojure.set :as set]

+ 2 - 2
src/main/frontend/worker/pipeline.cljs

@@ -8,8 +8,8 @@
             [logseq.db :as ldb]
             [logseq.db.frontend.validate :as db-validate]
             [logseq.db.sqlite.util :as sqlite-util]
-            [logseq.graph-parser.datascript-report :as ds-report]
-            [logseq.graph-parser.db-pipeline :as outliner-pipeline]
+            [logseq.outliner.datascript-report :as ds-report]
+            [logseq.outliner.pipeline :as outliner-pipeline]
             [logseq.db.frontend.property :as db-property]
             [logseq.outliner.core :as outliner-core]))
 

+ 10 - 1
src/test/frontend/handler/repo_test.cljs

@@ -7,6 +7,7 @@
             [logseq.common.util.block-ref :as block-ref]
             [frontend.db.model :as model]
             [frontend.db.conn :as conn]
+            [datascript.core :as d]
             [clojure.edn :as edn]
             ["path" :as node-path]
             ["fs" :as fs]))
@@ -22,7 +23,15 @@
             (file-repo-handler/parse-files-and-load-to-db! test-helper/test-db files {:re-render? false :verbose false}))
         db (conn/get-db test-helper/test-db)]
 
-    (docs-graph-helper/docs-graph-assertions db graph-dir (map :file/path files))))
+    (docs-graph-helper/docs-graph-assertions db graph-dir (map :file/path files))
+    (testing "Additional Counts"
+      (is (= 63926 (count (d/datoms db :eavt))) "Correct datoms count")
+
+      (is (= 5946
+             (ffirst
+              (d/q '[:find (count ?b)
+                     :where [?b :block/path-refs ?bp] [?bp :block/name]] db)))
+          "Correct referenced blocks count"))))
 
 (deftest parse-files-and-load-to-db-with-block-refs-on-reload
   (testing "Refs to blocks on a page are retained if that page is reloaded"