Browse Source

refactor: Decouple simple queries in db graph

from original *property rules by giving simple queries
their own rules. This allows simple queries to grow in functionality
and complexity without affecting other features. Also fixed
private-property rule wasn't working because deps weren't configured
Gabriel Horner 1 year ago
parent
commit
ce18010b2b

+ 3 - 0
.github/workflows/build.yml

@@ -83,6 +83,9 @@ jobs:
       - 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 query tests against DB version with basic query type
+        run: DB_GRAPH=1 DB_QUERY_TYPE=basic node static/tests.js -r frontend.db.query-dsl-test
+
       - name: Run ClojureScript tests
         run: node static/tests.js -e fix-me
 

+ 4 - 2
deps/db/bb.edn

@@ -31,8 +31,10 @@
            (concat (mapcat val rules/rules)
                    ;; TODO: Update linter to handle false positive on ?str-val for :property
                    (rules/extract-rules (dissoc rules/query-dsl-rules :property))
-                   ;; TODO: Update linter to handle false positive on :task, :priority, :property and :private-property
-                   (rules/extract-rules (dissoc rules/db-query-dsl-rules :task :priority :property :private-property
+                   ;; TODO: Update linter to handle false positive on :task, :priority, :*property* rules
+                   (rules/extract-rules (dissoc rules/db-query-dsl-rules
+                                                :task :priority
+                                                :property :simple-query-property :private-property
                                                 :property-checkbox-default-value
                                                 :property-missing-value
                                                 :has-property-or-default-value)))))}}

+ 51 - 19
deps/db/src/logseq/db/frontend/rules.cljc

@@ -206,13 +206,6 @@
          [?prop-e :db/valueType :db.type/ref]
          (property-default-value ?b ?prop-e :logseq.property/default-value ?val)))]]
 
-    :tags
-    '[(tags ?b ?tags)
-      [?b :block/tags ?t]
-      [?t :block/name ?tag]
-      [(missing? $ ?b :block/link)]
-      [(contains? ?tags ?tag)]]
-
     :object-has-class-property
     '[(object-has-class-property? ?b ?prop)
       [?prop-e :db/ident ?prop]
@@ -228,8 +221,9 @@
             (or [?prop-e :logseq.property/default-value _]
                 [?prop-e :logseq.property/checkbox-default-value _])))]
 
-    :has-property
-    '[(has-property ?b ?prop)
+    ;; Checks if a property exists for simple queries. Supports default values
+    :has-simple-query-property
+    '[(has-simple-query-property ?b ?prop)
       [?prop-e :db/ident ?prop]
       [?prop-e :block/type "property"]
       (has-property-or-default-value? ?b ?prop)
@@ -237,15 +231,46 @@
       [(get ?prop-schema :public? true) ?public]
       [(= true ?public)]]
 
-    ;; Same as has-property except it returns public and private properties like :block/title
-    :has-private-property
-    '[(has-private-property ?b ?prop)
+    ;; Same as has-simple-query-property except it returns public and private properties like :block/title
+    :has-private-simple-query-property
+    '[(has-private-simple-query-property ?b ?prop)
       [?prop-e :db/ident ?prop]
       [?prop-e :block/type "property"]
       (has-property-or-default-value? ?b ?prop)]
 
+    ;; Checks if a property exists for any features that are not simple queries
+    :has-property
+    '[(has-property ?b ?prop)
+      [?b ?prop _]
+      [?prop-e :db/ident ?prop]
+      [?prop-e :block/type "property"]
+      [?prop-e :block/schema ?prop-schema]
+      [(get ?prop-schema :public? true) ?public]
+      [(= true ?public)]]
+
+    ;; Checks if a property has a value for any features that are not simple queries
     :property
     '[(property ?b ?prop ?val)
+      [?prop-e :db/ident ?prop]
+      [?prop-e :block/type "property"]
+      [?prop-e :block/schema ?prop-schema]
+      [(get ?prop-schema :public? true) ?public]
+      [(= true ?public)]
+      [?b ?prop ?pv]
+      (or
+       ;; non-ref value
+       (and
+        [(missing? $ ?prop-e :db/valueType)]
+        [?b ?prop ?val])
+       ;; ref value
+       (and
+        [?prop-e :db/valueType :db.type/ref]
+        (or [?pv :block/title ?val]
+            [?pv :property.value/content ?val])))]
+
+    ;; Checks if a property has a value for simple queries. Supports default values
+    :simple-query-property
+    '[(simple-query-property ?b ?prop ?val)
       [?prop-e :db/ident ?prop]
       [?prop-e :block/type "property"]
       [?prop-e :block/schema ?prop-schema]
@@ -254,13 +279,20 @@
       [(= true ?public)]
       (property-value ?b ?prop-e ?val)]
 
-;; Same as property except it returns public and private properties like :block/title
-    :private-property
-    '[(private-property ?b ?prop ?val)
+    ;; Same as property except it returns public and private properties like :block/title
+    :private-simple-query-property
+    '[(private-simple-query-property ?b ?prop ?val)
       [?prop-e :db/ident ?prop]
       [?prop-e :block/type "property"]
       (property-value ?b ?prop-e ?val)]
 
+    :tags
+    '[(tags ?b ?tags)
+      [?b :block/tags ?t]
+      [?t :block/name ?tag]
+      [(missing? $ ?b :block/link)]
+      [(contains? ?tags ?tag)]]
+
     :task
     '[(task ?b ?statuses)
       ;; and needed to avoid binding error
@@ -281,13 +313,13 @@
    :priority #{:property}
    :property-missing-value #{:object-has-class-property}
    :has-property-or-default-value #{:object-has-class-property}
-   :has-property #{:has-property-or-default-value}
-   :has-private-property #{:has-property-or-default-value}
+   :has-simple-query-property #{:has-property-or-default-value}
+   :has-private-simple-query-property #{:has-property-or-default-value}
    :property-default-value #{:existing-property-value :property-missing-value}
    :property-checkbox-default-value #{:existing-property-value :property-missing-value}
    :property-value #{:property-default-value :property-checkbox-default-value}
-   :property #{:property-value}
-   :has-propeorty #{}})
+   :simple-query-property #{:property-value}
+   :private-simple-query-property #{:property-value}})
 
 (defn- get-full-deps
   [deps rules-deps]

+ 5 - 5
deps/db/test/logseq/db/frontend/rules_test.cljs

@@ -16,14 +16,14 @@
                              :existing-property-value
                              :object-has-class-property}
         property-value-deps (conj default-value-deps :property-value :property-checkbox-default-value)
-        property-deps (conj property-value-deps :property)
-        task-deps (conj property-deps :task)
-        priority-deps (conj property-deps :priority)
-        task-priority-deps (conj property-deps :task :priority)]
+        property-deps (conj property-value-deps :simple-query-property)
+        task-deps #{:property :task}
+        priority-deps #{:property :priority}
+        task-priority-deps  #{:property :task :priority}]
     (are [x y] (= (#'rules/get-full-deps x rules/rules-dependencies) y)
       [:property-default-value] default-value-deps
       [:property-value] property-value-deps
-      [:property] property-deps
+      [:simple-query-property] property-deps
       [:task] task-deps
       [:priority] priority-deps
       [:task :priority] task-priority-deps)))

+ 12 - 6
src/main/frontend/db/query_dsl.cljs

@@ -334,18 +334,24 @@
   (let [k (if db-graph? (->db-keyword-property (nth e 1)) (->file-keyword-property (nth e 1)))
         v (nth e 2)
         v' (if db-graph? (->db-property-value k v) (->file-property-value v))]
-    (if private-property?
-      {:query (list 'private-property '?b k v')
-       :rules [:private-property]}
+    (if db-graph?
+      (if private-property?
+        {:query (list 'private-simple-query-property '?b k v')
+         :rules [:private-simple-query-property]}
+        {:query (list 'simple-query-property '?b k v')
+         :rules [:simple-query-property]})
       {:query (list 'property '?b k v')
        :rules [:property]})))
 
 (defn- build-property-one-arg
   [e {:keys [db-graph? private-property?]}]
   (let [k (if db-graph? (->db-keyword-property (nth e 1)) (->file-keyword-property (nth e 1)))]
-    (if private-property?
-      {:query (list 'has-private-property '?b k)
-       :rules [:has-private-property]}
+    (if db-graph?
+      (if private-property?
+        {:query (list 'has-private-simple-query-property '?b k)
+         :rules [:has-private-simple-query-property]}
+        {:query (list 'has-simple-query-property '?b k)
+         :rules [:has-simple-query-property]})
       {:query (list 'has-property '?b k)
        :rules [:has-property]})))
 

+ 25 - 4
src/test/frontend/db/query_dsl_test.cljs

@@ -19,9 +19,12 @@
 ;; ============
 
 (def dsl-query*
-  "When $EXAMPLE set, prints query result of build query. Useful for
-   documenting examples and debugging"
-  (if (some? js/process.env.EXAMPLE)
+  "Overrides dsl-query/query with ENV variables. When $EXAMPLE is set, prints query
+  result of build query. This is useful for documenting examples and debugging.
+   When $DB_QUERY_TYPE is set, runs query tests against other versions of simple query e.g.
+   more basic property rules"
+  (cond
+    (some? js/process.env.EXAMPLE)
     (fn dsl-query-star [& args]
       (let [old-build-query query-dsl/build-query]
         (with-redefs [query-dsl/build-query
@@ -30,6 +33,24 @@
                           (println "EXAMPLE:" (pr-str (:query res)))
                           res))]
           (apply query-dsl/query args))))
+    (some? js/process.env.DB_QUERY_TYPE)
+    (fn dsl-query-star [& args]
+      (let [old-build-property @#'query-dsl/build-property]
+        (with-redefs [query-dsl/build-property
+                      (fn [& args']
+                        (let [m (apply old-build-property args')
+                              m' (cond
+                                   (= (:rules m) [:simple-query-property])
+                                   {:rules [:property]
+                                    :query (apply list 'property (rest (:query m)))}
+                                   (= (:rules m) [:has-simple-query-property])
+                                   {:rules [:has-property]
+                                    :query (apply list 'has-property (rest (:query m)))}
+                                   :else
+                                   m)]
+                          m'))]
+          (apply query-dsl/query args))))
+    :else
     query-dsl/query))
 
 (defn- ->smart-query
@@ -183,7 +204,7 @@ prop-d:: [[nada]]"}])
            (map :block/title (dsl-query "(property \"zzz name!\")")))
         "filter can handle property name")))
 
-(when js/process.env.DB_GRAPH
+(when (and js/process.env.DB_GRAPH (not js/process.env.DB_QUERY_TYPE))
   (deftest property-default-type-default-value-queries
     (load-test-files-for-db-graph
      {:properties