Fix page-properties bug and dsl-query bug

parser test caught page properties bug and dsl-query tests once written
correctly exposed another bug. Also remove unit test as they are
replaced by integration style tests in graph-parser-test

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

@@ -157,17 +157,6 @@
              distinct)
     []))
 
-;; Regex first checks page-refs
-;; to handle multi-word
-;; page-refs e.g. #[[foo bar]]
-(def ^:private page-ref-or-tag-re
-  (re-pattern (str "#?" (page-ref/->page-ref-re-str "(.*?)") "|#(\\S+)")))
-
-(defn- extract-page-refs-and-tags [string]
-  (let [refs (map #(or (second %) (get % 2))
-                  (re-seq page-ref-or-tag-re string))]
-    (or (seq refs) string)))
-
 (defn- get-page-ref-names-from-properties
   [format properties user-config]
   (let [page-refs (->>
@@ -186,7 +175,7 @@
                                  (not (gp-mldoc/link? format v)))
                             (let [v (string/trim v)
                                   result (if (:property-values-allow-links-and-text? user-config)
-                                           (extract-page-refs-and-tags v)
+                                           (text/extract-page-refs-and-tags v)
                                            (text/split-page-refs-without-brackets v {:un-brackets? false}))]
                               (if (coll? result)
                                 (map text/page-ref-un-brackets! result)
@@ -233,8 +222,6 @@
                                                  (and (= (keyword k) :file-path)
                                                       (string/starts-with? v "file:"))
                                                  v
-                                                 (:property-values-allow-links-and-text? user-config)
-                                                 (extract-page-refs-and-tags v)
                                                  :else
                                                  (text/parse-property format k v user-config)))
                                            k (keyword k)

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

@@ -200,9 +200,8 @@
 (defonce non-parsing-properties
   (atom #{"background-color" "background_color"}))
 
-(defn parse-property-value
-  "Property agnostic value parsing used to save properties to db and to
-  construct simple queries from user input"
+(defn parse-non-string-property-value
+  "Return parsed non-string property value or nil if none is found"
   [v]
   (cond
     (= v "true")
@@ -212,10 +211,18 @@
     false
 
     (gp-util/safe-re-find #"^\d+$" v)
-    (parse-long v)
+    (parse-long v)))
 
-    :else
-    (split-page-refs-without-brackets v)))
+(def ^:private page-ref-or-tag-re
+  (re-pattern (str "#?" (page-ref/->page-ref-re-str "(.*?)") "|#(\\S+)")))
+
+(defn extract-page-refs-and-tags
+  "Returns set of page-refs and tags in given string or returns string if none
+  are found"
+  [string]
+  (let [refs (map #(or (second %) (get % 2))
+                  (re-seq page-ref-or-tag-re string))]
+    (if (seq refs) (set refs) string)))
 
 (defn parse-property
   "Property value parsing that takes into account built-in properties, format
@@ -244,8 +251,9 @@
        (contains? gp-property/editable-linkable-built-in-properties (keyword k))
        (split-page-refs-without-brackets v)
 
-       (:property-values-allow-links-and-text? config-state)
-       v
-
        :else
-       (parse-property-value v)))))
+       (if-some [res (parse-non-string-property-value v)]
+         res
+         (if (:property-values-allow-links-and-text? config-state)
+           (extract-page-refs-and-tags v)
+           (split-page-refs-without-brackets v)))))))

deps/graph-parser/test/logseq/graph_parser/block_test.cljs

@@ -69,23 +69,4 @@
                                          ;; tags is linkable and background-color is not
                                          [["tags" "foo, bar"] ["background-color" "#008000"]]
                                          {:property-pages/enabled? true})))
-        "Only editable linkable built-in properties have page-refs in property values"))
-
-  (testing ":property-values-allow-links-and-text? option"
-    (are [x y]
-         (= y (select-keys
-               (gp-block/extract-properties :markdown
-                                            x
-                                            {:property-values-allow-links-and-text? true})
-               [:page-refs :properties]))
-         [["foo" "[[bar]] test #baz #[[multi word bar]]"]]
-         {:page-refs ["bar" "baz" "multi word bar" "foo"]
-          :properties {:foo #{"bar" "baz" "multi word bar"}}}
-
-         [["desc" "This is a multiple sentence description. It has one [[link]]"]]
-         {:page-refs ["link" "desc"]
-          :properties {:desc #{"link"}}}
-
-         [["foo" "[[bar]]"]]
-         {:page-refs ["bar" "foo"]
-          :properties {:foo #{"bar"}}})))
+        "Only editable linkable built-in properties have page-refs in property values")))

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

@@ -159,9 +159,11 @@
            (map :block/properties blocks))
         "pre-block/page and block have expected properties")
 
+    ;; has expected refs
     (are [db-props refs]
          (= (->> (vals db-props)
-                 (mapcat identity)
+                 ;; ignore string values
+                 (mapcat #(if (coll? %) % []))
                  (concat (map name (keys db-props)))
                  set)
             (set refs))
@@ -173,13 +175,15 @@
 (deftest property-relationships
   (let [properties {:single-link "[[bar]]"
                     :multi-link "[[Logseq]] is the fastest #triples #[[text editor]]"
-                    :desc "This is a multiple sentence description. It has one [[link]]"}]
+                    :desc "This is a multiple sentence description. It has one [[link]]"
+                    :comma-prop "one, two,three"}]
     (testing "With default config"
       (property-relationships-test
        properties
        {:single-link #{"bar"}
         :multi-link #{"Logseq" "is the fastest" "triples" "text editor"}
-        :desc #{"This is a multiple sentence description. It has one" "link"}}
+        :desc #{"This is a multiple sentence description. It has one" "link"}
+        :comma-prop #{"one" "two" "three"}}
        {}))
 
     (testing "With :property-values-allow-links-and-text config"
@@ -187,5 +191,6 @@
        properties
        {:single-link #{"bar"}
         :multi-link #{"Logseq" "triples" "text editor"}
-        :desc #{"link"}}
+        :desc #{"link"}
+        :comma-prop "one, two,three"}
        {:property-values-allow-links-and-text? true}))))

src/main/frontend/db/query_dsl.cljs

@@ -234,12 +234,20 @@
     (= 4 (count e))
     (build-between-three-arg e)))
 
+
+(defn parse-property-value
+  "Parses non-string property values or any page-ref like values"
+  [v]
+  (if-some [res (text/parse-non-string-property-value v)]
+    res
+    (text/split-page-refs-without-brackets v)))
+
 (defn- build-property-two-arg
   [e]
   (let [k (string/replace (name (nth e 1)) "_" "-")
         v (nth e 2)
         v (if-not (nil? v)
-            (text/parse-property-value (str v))
+            (parse-property-value (str v))
             v)
         v (if (coll? v) (first v) v)]
     {:query (list 'property '?b (keyword k) v)
@@ -284,7 +292,7 @@
   (let [[k v] (rest e)
         k (string/replace (name k) "_" "-")]
     (if (some? v)
-      (let [v' (text/parse-property-value (str v))
+      (let [v' (parse-property-value (str v))
             val (if (coll? v') (first v') v')]
         {:query (list 'page-property '?p (keyword k) val)
          :rules [:page-property]})

src/test/frontend/db/query_dsl_test.cljs

@@ -47,6 +47,21 @@
 
 (defn- block-property-queries-test
   []
+  (load-test-files [{:file/path "journals/2022_02_28.md"
+                     :file/content "a:: b
+- b1
+prop-a:: val-a
+prop-num:: 2000
+- b2
+prop-a:: val-a
+prop-b:: val-b
+- b3
+prop-c:: [[page a]], [[page b]], [[page c]]
+prop-linked-num:: [[3000]]
+prop-d:: [[no-space-link]]
+- b4
+prop-d:: nada"}])
+
   (testing "Blocks have given property value"
     (is (= #{"b1" "b2"}
            (set (map (comp first str/split-lines :block/content)
@@ -99,30 +114,27 @@
       "Blocks that have a property"))
 
 (deftest block-property-queries
-  (load-test-files [{:file/path "journals/2022_02_28.md"
-                     :file/content "a:: b
-- b1
-prop-a:: val-a
-prop-num:: 2000
-- b2
-prop-a:: val-a
-prop-b:: val-b
-- b3
-prop-c:: [[page a]], [[page b]], [[page c]]
-prop-linked-num:: [[3000]]
-prop-d:: [[no-space-link]]
-- b4
-prop-d:: nada"}])
-
   (testing "block property tests with default config"
     (test-helper/with-config {}
       (block-property-queries-test)))
+
+  (test-helper/start-test-db!) ;; reset db
+
   (testing "block property tests with property-values-allow-links-and-text? config"
     (test-helper/with-config {:property-values-allow-links-and-text? true}
       (block-property-queries-test))))
 
 (defn- page-property-queries-test
   []
+  (load-test-files [{:file/path "pages/page1.md"
+                     :file/content "parent:: [[child page 1]], [[child-no-space]]\ninteresting:: true"}
+                    {:file/path "pages/page2.md"
+                     :file/content "foo:: #bar\ninteresting:: false"}
+                    {:file/path "pages/page3.md"
+                     :file/content "parent:: [[child page 1]], [[child page 2]]\nfoo:: bar\ninteresting:: false"}
+                    {:file/path "pages/page4.md"
+                     :file/content "parent:: [[child page 2]]\nfoo:: baz"}])
+
   (is (= ["page1" "page3" "page4"]
          (map :block/name (dsl-query "(page-property parent)")))
       "Pages have given property")
@@ -173,18 +185,12 @@ prop-d:: nada"}])
         "Boolean false")))
 
 (deftest page-property-queries
-  (load-test-files [{:file/path "pages/page1.md"
-                     :file/content "parent:: [[child page 1]], [[child-no-space]]\ninteresting:: true"}
-                    {:file/path "pages/page2.md"
-                     :file/content "foo:: #bar\ninteresting:: false"}
-                    {:file/path "pages/page3.md"
-                     :file/content "parent:: [[child page 1]], [[child page 2]]\nfoo:: bar\ninteresting:: false"}
-                    {:file/path "pages/page4.md"
-                     :file/content "parent:: [[child page 2]]\nfoo:: baz"}])
-
   (testing "page property tests with default config"
     (test-helper/with-config {}
       (page-property-queries-test)))
+
+  (test-helper/start-test-db!) ;; reset db
+
   (testing "page property tests with property-values-allow-links-and-text? config"
     (test-helper/with-config {:property-values-allow-links-and-text? true}
       (page-property-queries-test))))