fix: remaining query dsl tests for db graphs

Re-enable CI step for testing query-dsl and db graphs

.github/workflows/build.yml

@@ -80,9 +80,8 @@ jobs:
       - name: Build test asset
         run: clojure -M:test compile test
 
-      # FIXME: Re-enable once db queries are fixed
-      # - name: Run some ClojureScript tests against DB version
-      #   run: DB_GRAPH=1 node static/tests.js -r frontend.db.query-dsl-test
+      - name: Run some ClojureScript tests against DB version
+        run: DB_GRAPH=1 node static/tests.js -r frontend.db.query-dsl-test
 
       - name: Run ClojureScript tests
         run: node static/tests.js -e fix-me

deps/db/src/logseq/db/frontend/rules.cljc

@@ -169,54 +169,15 @@
       [?prop-e :block/type "property"]
       ;; Some deleted properties leave #{} which this rule shouldn't match on
       [(not= #{} ?v)]]
-    ;; TODO: Delete when DB_GRAPH query-dsl tests are passing
-    #_'[(has-page-property ?p ?prop)
-        [?p :block/name]
-        [?p :block/properties ?bp]
-        [(name ?prop) ?prop-name-str]
-        [?prop-b :block/name ?prop-name-str]
-        [?prop-b :block/type "property"]
-        [?prop-b :block/uuid ?prop-uuid]
-        [(get ?bp ?prop-uuid) ?v]
-      ;; Some deleted properties leave #{} which this rule shouldn't match on
-        [(not= #{} ?v)]]
 
     :page-property
-    '[[(page-property ?p ?prop ?val)
-       [?p :block/name]
-       [?p ?prop ?pv]
-       (or [?pv :block/content ?val]
-           [?pv :block/original-name ?val])
-       [?prop-e :db/ident ?prop]
-       [?prop-e :block/type "property"]]
-      ;; TODO: Delete when DB_GRAPH query-dsl tests are passing
-      ;; Clause 1: Match non-ref values
-      #_[(page-property ?p ?key ?val)
-         [?p :block/name]
-         [?p :block/properties ?prop]
-         [(name ?key) ?key-str]
-         [?prop-b :block/name ?key-str]
-         [?prop-b :block/type "property"]
-         [?prop-b :block/uuid ?prop-uuid]
-         [(get ?prop ?prop-uuid) ?v]
-         (or ([= ?v ?val])
-             [(contains? ?v ?val)])]
-
-      ;; Clause 2: Match values joined by refs
-      #_[(page-property ?p ?key ?val)
-         [?p :block/name]
-         [?p :block/properties ?prop]
-         [(name ?key) ?key-str]
-         [?prop-b :block/name ?key-str]
-         [?prop-b :block/type "property"]
-         [?prop-b :block/uuid ?prop-uuid]
-         [(get ?prop ?prop-uuid) ?v]
-         [(str ?val) ?str-val]
-      ;; str-val is for integer pages that aren't strings
-         [?prop-val-b :block/original-name ?str-val]
-         [?prop-val-b :block/uuid ?val-uuid]
-         (or ([= ?v ?val-uuid])
-             [(contains? ?v ?val-uuid)])]]
+    '[(page-property ?p ?prop ?val)
+      [?p :block/name]
+      [?p ?prop ?pv]
+      (or [?pv :block/content ?val]
+          [?pv :block/original-name ?val])
+      [?prop-e :db/ident ?prop]
+      [?prop-e :block/type "property"]]
 
     :has-property
     '[(has-property ?b ?prop)

src/test/frontend/db/query_dsl_test.cljs

@@ -114,7 +114,6 @@ prop-d:: [[nada]]"}])
               (dsl-query "(and (property prop-c \"page c\"))")))
       "Blocks have property value from a set of values")
 
-  ;; TODO: optimize
   (is (= ["b3"]
          (map (comp first str/split-lines :block/content)
               (dsl-query "(and (property prop-c \"page c\") (property prop-c \"page b\"))")))
@@ -147,12 +146,12 @@ prop-d:: [[nada]]"}])
                 (dsl-query "(property prop-d)")))
         "Blocks that have a property"))
 
-(deftest ^:done block-property-queries
+(deftest block-property-queries
   (testing "block property tests with default config"
     (test-helper/with-config {}
       (block-property-queries-test))))
 
-(deftest ^:done block-property-query-performance
+(deftest block-property-query-performance
   (let [pages (->> (repeat 10 {:tags ["tag1" "tag2"]})
                    (map-indexed (fn [idx {:keys [tags]}]
                                   {:file/path (str "pages/page" idx ".md")
@@ -233,7 +232,7 @@ prop-d:: [[nada]]"}])
              (map :block/name (dsl-query "(page-property interesting false)")))
           "Boolean false")))
 
-(deftest ^:done page-property-queries
+(deftest page-property-queries
   (testing "page property tests with default config"
     (test-helper/with-config {}
       (page-property-queries-test))))
@@ -241,46 +240,45 @@ prop-d:: [[nada]]"}])
 (deftest task-queries
   (load-test-files [{:file/path "pages/page1.md"
                      :file/content "foo:: bar
-- NOW b1
+- DONE b1
 - TODO b2
-- LATER b3
-- LATER [#A] b4
-- LATER [#B] b5"}])
+- DOING b3
+- DOING b4 [[A]]
+- DOING b5 [[B]]"}])
 
   (testing "Lowercase query"
-    (is (= ["NOW b1"]
-           (map :block/content (dsl-query "(task now)"))))
+      (is (= ["DONE b1"]
+             (map testable-content (dsl-query "(task done)"))))
 
-    (is (= ["LATER b3" "LATER [#A] b4" "LATER [#B] b5"]
-           (map :block/content (dsl-query "(task later)")))))
+      (is (= #{"DOING b3" "DOING b4" "DOING b5"}
+             (set (map testable-content (dsl-query "(task doing)"))))))
 
-  (is (= ["LATER b3" "LATER [#A] b4" "LATER [#B] b5"]
-         (map :block/content (dsl-query "(task LATER)")))
+  (is (= #{"DOING b3" "DOING b4" "DOING b5"}
+         (set (map testable-content (dsl-query "(task DOING)"))))
       "Uppercase query")
 
   (testing "Multiple specified tasks results in ORed results"
-    (is (= ["NOW b1" "LATER b3" "LATER [#A] b4" "LATER [#B] b5"]
-           (map :block/content (dsl-query "(task now later)"))))
+    (is (= #{"DONE b1" "DOING b3" "DOING b4" "DOING b5"}
+           (set (map testable-content (dsl-query "(task done doing)")))))
 
-    (is (= ["NOW b1" "LATER b3" "LATER [#A] b4" "LATER [#B] b5"]
-           (map :block/content (dsl-query "(task [now later])")))
+    (is (= #{"DONE b1" "DOING b3" "DOING b4" "DOING b5"}
+           (set (map testable-content (dsl-query "(task [done doing])"))))
         "Multiple arguments specified with vector notation"))
 
-  (is (= ["NOW b1" "LATER [#A] b4"]
-         (map :block/content
-              (dsl-query "(or (todo now) (and (todo later) (priority a)))")))
+  (is (= ["DONE b1" "DOING b4"]
+         (map testable-content
+              (dsl-query "(or (task done) (and (task doing) [[A]]))")))
       "Multiple boolean operators with todo and priority operators")
 
-  (is (= ["LATER [#A] b4" "LATER [#B] b5"]
-         (map :block/content
-              (dsl-query "(and (todo later) (or (priority a) (priority b)))")))))
+  (is (= ["DOING b4" "DOING b5"]
+           (map testable-content
+                (dsl-query "(and (task doing) (or [[A]] [[B]]))")))))
 
 (deftest sample-queries
   (load-test-files [{:file/path "pages/page1.md"
                      :file/content "foo:: bar
 - TODO b1
 - TODO b2"}])
-
   (is (= 1
          (count (dsl-query "(and (task todo) (sample 1))")))
       "Correctly limits block results")
@@ -288,7 +286,7 @@ prop-d:: [[nada]]"}])
          (count (dsl-query "(and (page-property foo) (sample 1))")))
       "Correctly limits page results"))
 
-(deftest ^:done priority-queries
+(deftest priority-queries
   (load-test-files (if js/process.env.DB_GRAPH
                      [{:page {:block/original-name "page1"}
                        :blocks [{:block/content "[#A] b1"
@@ -326,7 +324,7 @@ prop-d:: [[nada]]"}])
                      (dsl-query (if js/process.env.DB_GRAPH "(priority high medium low)" "(priority a b c)")))))
         "Three arg queries and args that have no match"))
 
-(deftest ^:done nested-boolean-queries
+(deftest nested-boolean-queries
   (load-test-files [{:file/path "pages/page1.md"
                      :file/content "foo:: bar
 - DONE b1 [[page 1]] [[page 3]]
@@ -337,9 +335,9 @@ prop-d:: [[nada]]"}])
 - LATER b4Z [[page 2]]
 "}])
 
-  (let [task-filter (if js/process.env.DB_GRAPH "(todo doing todo)" "(todo now later)")]
+  (let [task-filter (if js/process.env.DB_GRAPH "(task doing todo)" "(task now later)")]
     (is (= []
-           (dsl-query "(and (todo done) (not [[page 1]]))")))
+           (dsl-query "(and (task done) (not [[page 1]]))")))
 
     (is (= ["DONE b1"]
            (map testable-content
@@ -356,7 +354,7 @@ prop-d:: [[nada]]"}])
              "DONE b2Z"}
            (set (map testable-content
                      (dsl-query (str "(and "
-                                     (if js/process.env.DB_GRAPH "(todo doing todo done)" "(todo now later done)")
+                                     (if js/process.env.DB_GRAPH "(task doing todo done)" "(task now later done)")
                                      " (or [[page 1]] (not [[page 1]])))"))))))
 
     (is (= (if js/process.env.DB_GRAPH #{"bar" "DONE b1" "DONE b2Z"} #{"foo:: bar" "DONE b1" "DONE b2Z"})
@@ -376,7 +374,7 @@ prop-d:: [[nada]]"}])
   ;        (dsl-query "(or (priority a) (not (priority c)))")))
   )
 
-(deftest ^:done page-tags-and-all-page-tags-queries
+(deftest page-tags-and-all-page-tags-queries
   (load-test-files
    [{:file/path "pages/page1.md"
      :file/content "tags:: [[page-tag-1]], [[page-tag-2]]"}
@@ -431,7 +429,7 @@ prop-d:: [[nada]]"}])
               (dsl-query "(page nope)")))
       "Correctly returns no results"))
 
-(deftest ^:done empty-queries
+(deftest empty-queries
   (testing "nil or blank strings should be ignored"
     (are [x] (nil? (dsl-query x))
       nil
@@ -439,7 +437,7 @@ prop-d:: [[nada]]"}])
       " "
       "\"\"")))
 
-(deftest ^:done page-ref-and-boolean-queries
+(deftest page-ref-and-boolean-queries
   (load-test-files [{:file/path "pages/page1.md"
                      :file/content "foo:: bar
 - b1 [[page 1]] [[tag2]]
@@ -484,7 +482,7 @@ prop-d:: [[nada]]"}])
                 (set)))
         "NOT query")))
 
-(deftest ^:done nested-page-ref-queries
+(deftest nested-page-ref-queries
   (load-test-files (if js/process.env.DB_GRAPH
                      [{:page {:block/original-name "page1"}
                        :blocks [{:block/content "p1 [[Parent page]]"
@@ -503,7 +501,7 @@ prop-d:: [[nada]]"}])
          (map testable-content
               (dsl-query "(and [[Parent page]] (not [[Child page]]))")))))
 
-(deftest ^:done between-queries
+(deftest between-queries
   (load-test-files [{:file/path "journals/2020_12_26.md"
                      :file/content "foo::bar
 - DONE 26-b1
@@ -530,7 +528,7 @@ created-at:: 1608968448116
        ;; 3
       )))
 
-(deftest ^:done custom-query-test
+(deftest custom-query-test
   (load-test-files [{:file/path "pages/page1.md"
                      :file/content "foo:: bar
 - NOW b1
@@ -607,7 +605,7 @@ created-at:: 1608968448116
                (->> (dsl-query "(and (page-property rating) (sort-by rating))")
                     (map #(get-property-value % :rating))))))))
 
-(deftest ^:done simplify-query
+(deftest simplify-query
   (are [x y] (= (query-dsl/simplify-query x) y)
     '(and [[foo]])
     '[[foo]]

src/test/frontend/test/helper.cljs

@@ -71,132 +71,17 @@
                           val)))]))))
        (into {})))
 
-#_(defn- build-block-properties
-  "Parses out properties from a file's content and associates it with the page name
-   or block content"
-  [file-content]
-  (if (string/includes? file-content "\n-")
-    {:block-properties
-     (->> (string/split file-content #"\n-\s*")
-          (mapv (fn [s]
-                  (let [[content & props] (string/split-lines s)]
-                    (cond-> {:block/content content}
-                      ;; If no property chars may accidentally parse child blocks
-                      ;; so don't do property parsing
-                      (and (string/includes? s ":: ") props)
-                      (assoc :build/properties (property-lines->properties props)))))))}
-    {:page-properties
-     (->> file-content
-          string/split-lines
-          property-lines->properties)}))
-
-#_(defn- update-file-for-db-graph
-  "Adds properties by block/page for a file and updates block content"
-  [file]
-  (let [{:keys [block-properties page-properties]}
-        (build-block-properties (:file/content file))]
-    (if page-properties
-      (merge file
-             {:file/block-properties (vec (keep #(when (seq (:properties %)) %)
-                                                [{:name-or-content (file-path->page-name (:file/path file))
-                                                  :properties page-properties
-                                                  :page-properties? true}]))})
-      (merge file
-             {:file/block-properties
-              ;; Filter out empty empty properties to avoid needless downstream processing
-              (cond-> (vec (keep #(when (seq (:properties %)) %) block-properties))
-                                       ;; Optionally add page properties as a page block
-                (re-find #"^\s*[^-]+" (:name-or-content (first block-properties)))
-                (conj {:name-or-content (file-path->page-name (:file/path file))
-                       :properties (->> (:name-or-content (first block-properties))
-                                        string/split-lines
-                                        property-lines->properties)
-                       :page-properties? true}))}
-             ;; Rewrite content to strip it of properties which shouldn't be in content
-             ;; but only if properties are detected
-             (when (some #(seq (:properties %)) block-properties)
-               {:file/content (string/join "\n"
-                                           (map (fn [x] (str "- " (:name-or-content x))) block-properties))})))))
-
-#_(defn- load-test-files-for-db-graph-old
-  [files*]
-  (let [files (mapv update-file-for-db-graph files*)]
-    ;; TODO: Use sqlite instead of file graph to create client db
-    (file-repo-handler/parse-files-and-load-to-db!
-     test-db
-     files
-     {:re-render? false :verbose false :refresh? true})
-    (let [content-uuid-map (into {} (d/q
-                                     '[:find ?content ?uuid
-                                       :where
-                                       [?b :block/content ?content]
-                                       [?b :block/uuid ?uuid]]
-                                     (db/get-db test-db)))
-          page-name-map (into {} (d/q
-                                  '[:find ?name ?uuid
-                                    :where
-                                    [?b :block/name ?name]
-                                    [?b :block/uuid ?uuid]]
-                                  (db/get-db test-db)))
-          property-uuids (->> files
-                              (mapcat #(->> % :file/block-properties (map :properties) (mapcat keys)))
-                              set
-                              ;; Property pages may be created by file graphs automatically,
-                              ;; usually by page properties. Delete this if file graphs are long
-                              ;; used to create datascript db
-                              (map #(vector % (or (page-name-map (name %)) (random-uuid))))
-                              (into {}))
-          ;; from upsert-property!
-          new-properties-tx (mapv (fn [[prop-name uuid]]
-                                    (sqlite-util/block-with-timestamps
-                                     {:block/uuid uuid
-                                      :block/schema {:type :default}
-                                      :block/original-name (name prop-name)
-                                      :block/name (string/lower-case (name prop-name))
-                                      :block/type "property"}))
-                                  property-uuids)
-          page-uuids (->> files
-                          (mapcat #(->> %
-                                        :file/block-properties
-                                        (map :properties)
-                                        (mapcat (fn [m]
-                                                  (->> m vals (filter set?) (apply set/union))))))
-                          set
-                          (map #(vector % (random-uuid)))
-                          (into {}))
-          page-tx (mapv (fn [[page-name uuid]]
-                          (sqlite-util/block-with-timestamps
-                           {:block/name (string/lower-case page-name)
-                            :block/original-name page-name
-                            :block/uuid uuid}))
-                        page-uuids)
-          ;; from add-property!
-          block-properties-tx
-          (mapcat
-           (fn [file]
-             (map
-              (fn [{:keys [name-or-content properties page-properties?]}]
-                (cond-> {:block/uuid (if page-properties?
-                                       (or (page-name-map name-or-content)
-                                           (throw (ex-info "No uuid for page" {:page-name name-or-content})))
-                                       (or (content-uuid-map name-or-content)
-                                           (throw (ex-info "No uuid for content" {:content name-or-content}))))
-                         :block/properties
-                         (->> (dissoc properties :created-at)
-                              (map
-                               (fn [[prop-name val]]
-                                 [(or (property-uuids prop-name)
-                                      (throw (ex-info "No uuid for property" {:name prop-name})))
-                                  (if (set? val)
-                                    (set (map (fn [p] (or (page-uuids p) (throw (ex-info "No uuid for page" {:name p}))))
-                                              val))
-                                    val)]))
-                              (into {}))}
-                  (:created-at properties)
-                  (assoc :block/created-at (:created-at properties))))
-              (:file/block-properties file)))
-           files)]
-      (db/transact! test-db (vec (concat page-tx new-properties-tx block-properties-tx))))))
+(defn- property-lines->attributes
+  "Converts markdown property lines e.g. `foo:: bar\nfoo:: baz` to properties
+  and attributes. All are treated as properties except for tags -> :block/tags
+  and created-at -> :block/created-at"
+  [lines]
+  (let [props (property-lines->properties lines)]
+    (cond-> {:build/properties (dissoc props :created-at :tags)}
+      (:tags props)
+      (assoc :build/tags (mapv keyword (:tags props)))
+      (:created-at props)
+      (assoc :block/created-at (:created-at props)))))
 
 (def file-to-db-statuses
   {"TODO" :logseq.task/status.todo
@@ -210,24 +95,17 @@
    "CANCELED" :logseq.task/status.canceled
    "CANCELLED" :logseq.task/status.canceled})
 
-(defn- property-lines->attributes
-  [lines]
-  (let [props (property-lines->properties lines)]
-    (cond-> {:build/properties (dissoc props :created-at :tags)}
-      (:tags props)
-      (assoc :build/tags (mapv keyword (:tags props)))
-      (:created-at props)
-      (assoc :block/created-at (:created-at props)))))
-
 (defn- parse-content
+  "Given a file's content as markdown, returns blocks and page attributes for the file
+   to be used with sqlite-build/build-blocks-tx"
   [content*]
   (let [blocks** (if (string/includes? content* "\n-")
                    (->> (string/split content* #"\n-\s*")
                         (mapv (fn [s]
                                 (let [[content & props] (string/split-lines s)]
                                   (cond-> {:block/content content}
-                                  ;; If no property chars may accidentally parse child blocks
-                                  ;; so don't do property parsing
+                                    ;; If no property chars may accidentally parse child blocks
+                                    ;; so don't do property parsing
                                     (and (string/includes? s ":: ") props)
                                     (merge (property-lines->attributes props)))))))
                    ;; only has a page pre-block
@@ -247,11 +125,14 @@
                    blocks)
      :page-attributes page-attrs}))
 
-(defn- build-blocks-tx-options [options*]
+(defn- build-blocks-tx-options
+  "Given arguments to load-test-files, parses and converts them to options for
+  sqlite-build/build-blocks-tx. Supports a limited set of markdown including
+  task keywords, page properties and block properties. See query-dsl-test for examples"
+  [options*]
   (let [pages-and-blocks
         (mapv (fn [{:file/keys [path content]}]
                 (let [{:keys [blocks page-attributes]} (parse-content content)
-                      ;; _ (prn :parse-content content blocks)
                       unique-page-attrs
                       (if (string/starts-with? path "journals")
                         {:build/journal
@@ -275,7 +156,6 @@
   [options*]
   (let [;; Builds options from markdown :file/content unless given explicit build-blocks config
         options (if (:page (first options*)) {:pages-and-blocks options*} (build-blocks-tx-options options*))
-        _ (prn :opt options)
         {:keys [init-tx block-props-tx]} (sqlite-build/build-blocks-tx options)]
     (db/transact! test-db init-tx)
     (when (seq block-props-tx)