Browse Source

Fix up model tests and remove unused rules

Gabriel Horner 2 years ago
parent
commit
70a476dde3
3 changed files with 108 additions and 131 deletions
  1. 56 55
      deps/db/src/logseq/db/rules.cljc
  2. 3 12
      src/main/frontend/db/model.cljs
  3. 49 64
      src/test/frontend/db/model_test.cljs

+ 56 - 55
deps/db/src/logseq/db/rules.cljc

@@ -2,62 +2,63 @@
   "Datalog rules for use with logseq.db.schema")
 
 (def ^:large-vars/data-var rules
+  "Rules used mainly in frontend.db.model"
   ;; rule "parent" is optimized for parent node -> child node nesting queries
-  '[[(parent ?p ?c)
-     [?c :block/parent ?p]]
-    [(parent ?p ?c)
-     [?c :block/parent ?t]
-     (parent ?p ?t)]
-
-  ;; rule "child" is optimized for child node -> parent node nesting queries
-    [(child ?p ?c)
-     [?c :block/parent ?p]]
-    [(child ?p ?c)
-     [?t :block/parent ?p]
-     (child ?t ?c)]
-
-  ;; rule "namespace" is optimized for child node -> node of upper namespace level nesting queries
-    [(namespace ?p ?c)
-     [?c :block/namespace ?p]]
-    [(namespace ?p ?c)
-     [?t :block/namespace ?p]
-     (namespace ?t ?c)]
-
-    ;; Select rules carefully, as it is critical for performance.
-    ;; The rules have different clause order and resolving directions.
-    ;; Clause order Reference:
-    ;; https://docs.datomic.com/on-prem/query/query-executing.html#clause-order
-    ;; Recursive optimization Reference:
-    ;; https://stackoverflow.com/questions/42457136/recursive-datalog-queries-for-datomic-really-slow
-    ;; Should optimize for query the decendents of a block
-    ;; Quote:
-    ;; My theory is that your rules are not written in a way that Datalog can optimize for this read pattern - probably resulting in a traversal of all the entities. I suggest to rewrite them as follows:
-    ;; [[(ubersymbol ?c ?p)
-    ;;   (?c :ml/parent ?p)]
-    ;;  [(ubersymbol ?c ?p)
-    ;;   ;; we bind a child of the ancestor, instead of a parent of the descendant
-    ;;   (?c1 :ml/parent ?p)
-    ;;   (ubersymbol ?c ?c1)]]
-
-    ;; This way of writing the ruleset is optimized to find the descendants of some node. The way you originally wrote it is optimized to find the anscestors of some node.
-
-    ;; from https://stackoverflow.com/questions/43784258/find-entities-whose-ref-to-many-attribute-contains-all-elements-of-input
-    ;; Quote:
-    ;; You're tackling the general problem of 'dynamic conjunction' in Datomic's Datalog.
-    ;; Write a dynamic Datalog query which uses 2 negations and 1 disjunction or a recursive rule
-    ;; Datalog has no direct way of expressing dynamic conjunction (logical AND / 'for all ...' / set intersection).
-    ;; However, you can achieve it in pure Datalog by combining one disjunction
-    ;; (logical OR / 'exists ...' / set union) and two negations, i.e
-    ;; (For all ?g in ?Gs p(?e,?g)) <=> NOT(Exists ?g in ?Gs, such that NOT(p(?e, ?g)))
-
-    ;; [(matches-all ?e ?a ?vs)
-    ;;  [(first ?vs) ?v0]
-    ;;  [?e ?a ?v0]
-    ;;  (not-join [?e ?vs]
-    ;;            [(identity ?vs) [?v ...]]
-    ;;            (not-join [?e ?v]
-    ;;                      [?e ?a ?v]))]
-    ])
+  {:namespace
+   '[[(namespace ?p ?c)
+      [?c :block/namespace ?p]]
+     [(namespace ?p ?c)
+      [?t :block/namespace ?p]
+      (namespace ?t ?c)]]
+
+   :alias
+   '[[(alias ?e2 ?e1)
+      [?e2 :block/alias ?e1]]
+     [(alias ?e2 ?e1)
+      [?e1 :block/alias ?e2]]
+     [(alias ?e1 ?e3)
+      [?e1 :block/alias ?e2]
+      [?e2 :block/alias ?e3]]
+     [(alias ?e3 ?e1)
+      [?e1 :block/alias ?e2]
+      [?e2 :block/alias ?e3]]]})
+
+;; Rules writing advice
+;; ====================
+;; Select rules carefully, as it is critical for performance.
+;; The rules have different clause order and resolving directions.
+;; Clause order Reference:
+;; https://docs.datomic.com/on-prem/query/query-executing.html#clause-order
+;; Recursive optimization Reference:
+;; https://stackoverflow.com/questions/42457136/recursive-datalog-queries-for-datomic-really-slow
+;; Should optimize for query the decendents of a block
+;; Quote:
+;; My theory is that your rules are not written in a way that Datalog can optimize for this read pattern - probably resulting in a traversal of all the entities. I suggest to rewrite them as follows:
+;; [[(ubersymbol ?c ?p)
+;;   (?c :ml/parent ?p)]
+;;  [(ubersymbol ?c ?p)
+;;   ;; we bind a child of the ancestor, instead of a parent of the descendant
+;;   (?c1 :ml/parent ?p)
+;;   (ubersymbol ?c ?c1)]]
+
+;; This way of writing the ruleset is optimized to find the descendants of some node. The way you originally wrote it is optimized to find the anscestors of some node.
+
+;; from https://stackoverflow.com/questions/43784258/find-entities-whose-ref-to-many-attribute-contains-all-elements-of-input
+;; Quote:
+;; You're tackling the general problem of 'dynamic conjunction' in Datomic's Datalog.
+;; Write a dynamic Datalog query which uses 2 negations and 1 disjunction or a recursive rule
+;; Datalog has no direct way of expressing dynamic conjunction (logical AND / 'for all ...' / set intersection).
+;; However, you can achieve it in pure Datalog by combining one disjunction
+;; (logical OR / 'exists ...' / set union) and two negations, i.e
+;; (For all ?g in ?Gs p(?e,?g)) <=> NOT(Exists ?g in ?Gs, such that NOT(p(?e, ?g)))
+
+;; [(matches-all ?e ?a ?vs)
+;;  [(first ?vs) ?v0]
+;;  [?e ?a ?v0]
+;;  (not-join [?e ?vs]
+;;            [(identity ?vs) [?v ...]]
+;;            (not-join [?e ?v]
+;;                      [?e ?a ?v]))]
 
 (def ^:large-vars/data-var query-dsl-rules
   "Rules used by frontend.db.query-dsl. The symbols ?b and ?p respectively refer

+ 3 - 12
src/main/frontend/db/model.cljs

@@ -16,7 +16,7 @@
             [frontend.util :as util :refer [react]]
             [frontend.util.drawer :as drawer]
             [logseq.db.default :as default-db]
-            [logseq.db.rules :refer [rules]]
+            [logseq.db.rules :as rules]
             [logseq.db.schema :as db-schema]
             [logseq.graph-parser.config :as gp-config]
             [logseq.graph-parser.text :as text]
@@ -300,16 +300,7 @@
             (alias ?page ?e)]
           (conn/get-db repo-url)
           (util/safe-page-name-sanity-lc page)
-          '[[(alias ?e2 ?e1)
-             [?e2 :block/alias ?e1]]
-            [(alias ?e2 ?e1)
-             [?e1 :block/alias ?e2]]
-            [(alias ?e1 ?e3)
-             [?e1 :block/alias ?e2]
-             [?e2 :block/alias ?e3]]
-            [(alias ?e3 ?e1)
-             [?e1 :block/alias ?e2]
-             [?e2 :block/alias ?e3]]])
+          (:alias rules/rules))
      db-utils/seq-flatten
      (set)
      (set/union #{page-id}))))
@@ -1585,7 +1576,7 @@
        [?p :block/name ?namespace]
        (namespace ?p ?c)]
      (conn/get-db repo)
-     rules
+     (:namespace rules/rules)
      namespace)))
 
 (defn- tree [flat-col root]

+ 49 - 64
src/test/frontend/db/model_test.cljs

@@ -1,11 +1,25 @@
 (ns frontend.db.model-test
-  (:require [cljs.test :refer [use-fixtures deftest is]]
+  (:require [cljs.test :refer [use-fixtures deftest is are]]
             [frontend.db.model :as model]
             [frontend.test.helper :as test-helper :refer [load-test-files]]))
 
 (use-fixtures :each {:before test-helper/start-test-db!
                      :after test-helper/destroy-test-db!})
 
+(deftest get-namespace-pages
+  (load-test-files [{:file/path "pages/a.b.c.md"
+                     :file/content "foo"}
+                    {:file/path "pages/b.c.md"
+                     :file/content "bar"}
+                    {:file/path "pages/b.d.md"
+                     :file/content "baz"}])
+
+  (is (= ["a/b" "a/b/c"]
+         (map :block/name (model/get-namespace-pages test-helper/test-db "a"))))
+
+  (is (= ["b/c" "b/d"]
+         (map :block/name (model/get-namespace-pages test-helper/test-db "b")))))
+
 (deftest get-page-namespace-routes
   (load-test-files [{:file/path "pages/a.b.c.md"
                      :file/content "foo"}
@@ -18,69 +32,40 @@
          (map :block/name (model/get-page-namespace-routes test-helper/test-db "b/c")))
       "Empty if page exists"))
 
-;; (deftest test-page-alias-with-multiple-alias
-;;   []
-;;   (p/let [files [{:file/path "a.md"
-;;                   :file/content "---\ntitle: a\nalias: b, c\n---"}
-;;                  {:file/path "b.md"
-;;                   :file/content "---\ntitle: b\nalias: a, d\n---"}
-;;                  {:file/path "e.md"
-;;                   :file/content "---\ntitle: e\n---\n## ref to [[b]]"}]
-;;           _ (-> (repo-handler/parse-files-and-load-to-db! test-db files {:re-render? false})
-;;                 (p/catch (fn [] "ignore indexedDB error")))
-;;           a-aliases (model/page-alias-set test-db "a")
-;;           b-aliases (model/page-alias-set test-db "b")
-;;           alias-names (model/get-page-alias-names test-db "a")
-;;           b-ref-blocks (model/get-page-referenced-blocks test-db "b")
-;;           a-ref-blocks (model/get-page-referenced-blocks test-db "a")]
-;;     (are [x y] (= x y)
-;;       4 (count a-aliases)
-;;       4 (count b-aliases)
-;;       1 (count b-ref-blocks)
-;;       1 (count a-ref-blocks)
-;;       (set ["b" "c" "d"]) (set alias-names))))
-
-;; (deftest test-page-alias-set
-;;   []
-;;   (p/let [files [{:file/path "a.md"
-;;                   :file/content "---\ntitle: a\nalias: [[b]]\n---"}
-;;                  {:file/path "b.md"
-;;                   :file/content "---\ntitle: b\nalias: [[c]]\n---"}
-;;                  {:file/path "d.md"
-;;                   :file/content "---\ntitle: d\n---\n## ref to [[b]]"}]
-;;           _ (-> (repo-handler/parse-files-and-load-to-db! test-db files {:re-render? false})
-;;                 (p/catch (fn [] "ignore indexedDB error")))
-;;           a-aliases (model/page-alias-set test-db "a")
-;;           b-aliases (model/page-alias-set test-db "b")
-;;           alias-names (model/get-page-alias-names test-db "a")
-;;           b-ref-blocks (model/get-page-referenced-blocks test-db "b")
-;;           a-ref-blocks (model/get-page-referenced-blocks test-db "a")]
-;;     (are [x y] (= x y)
-;;       3 (count a-aliases)
-;;       1 (count b-ref-blocks)
-;;       1 (count a-ref-blocks)
-;;       (set ["b" "c"]) (set alias-names))))
-
-;; (deftest test-page-alias-without-brackets
-;;   []
-;;   (p/let [files [{:file/path "a.md"
-;;                   :file/content "---\ntitle: a\nalias: b\n---"}
-;;                  {:file/path "b.md"
-;;                   :file/content "---\ntitle: b\nalias: c\n---"}
-;;                  {:file/path "d.md"
-;;                   :file/content "---\ntitle: d\n---\n## ref to [[b]]"}]
-;;           _ (-> (repo-handler/parse-files-and-load-to-db! test-db files {:re-render? false})
-;;                 (p/catch (fn [] "ignore indexedDB error")))
-;;           a-aliases (model/page-alias-set test-db "a")
-;;           b-aliases (model/page-alias-set test-db "b")
-;;           alias-names (model/get-page-alias-names test-db "a")
-;;           b-ref-blocks (model/get-page-referenced-blocks test-db "b")
-;;           a-ref-blocks (model/get-page-referenced-blocks test-db "a")]
-;;     (are [x y] (= x y)
-;;       3 (count a-aliases)
-;;       1 (count b-ref-blocks)
-;;       1 (count a-ref-blocks)
-;;       (set ["b" "c"]) (set alias-names))))
+(deftest test-page-alias-with-multiple-alias
+  (load-test-files [{:file/path "aa.md"
+                     :file/content "alias:: ab, ac"}
+                    {:file/path "ab.md"
+                     :file/content "alias:: aa, ad"}
+                    {:file/path "ae.md"
+                     :file/content "## ref to [[ab]]"}])
+  (let [a-aliases (model/page-alias-set test-helper/test-db "aa")
+        b-aliases (model/page-alias-set test-helper/test-db "ab")
+        alias-names (model/get-page-alias-names test-helper/test-db "aa")
+        b-ref-blocks (model/get-page-referenced-blocks "ab")
+        a-ref-blocks (model/get-page-referenced-blocks "aa")]
+
+    (are [x y] (= x y)
+         4 (count a-aliases)
+         4 (count b-aliases)
+         4 (count b-ref-blocks)
+         4 (count a-ref-blocks)
+         #{"ab" "ac" "ad"} (set alias-names))))
+
+(deftest test-page-alias-set
+  (load-test-files [{:file/path "aa.md"
+                     :file/content "alias:: ab"}
+                    {:file/path "ab.md"
+                     :file/content "alias:: ac"}
+                    {:file/path "ad.md"
+                     :file/content "## ref to [[ab]]"}])
+  (let [a-aliases (model/page-alias-set test-helper/test-db "aa")
+        alias-names (model/get-page-alias-names test-helper/test-db "aa")
+        a-ref-blocks (model/get-page-referenced-blocks "aa")]
+    (are [x y] (= x y)
+         3 (count a-aliases)
+         3 (count a-ref-blocks)
+         #{"ab" "ac"} (set alias-names))))
 
 (deftest get-pages-that-mentioned-page-with-show-journal
   (load-test-files [{:file/path "journals/2020_08_15.md"