MLv2 filter schema (#28912)

* Make nil equality comparable
* Add Malli schema for filter expressions

src/metabase/lib/schema/expression.cljc

@@ -49,9 +49,10 @@
 
 ;;; Any type of expression that can appear in an `:=` or `!=`. I guess this is currently everything?
 (mr/def ::equality-comparable
-  [:or
-   ::boolean
-   ::string
-   ::number
-   ::temporal
-   ::ref/ref])
+  [:maybe
+   [:or
+    ::boolean
+    ::string
+    ::number
+    ::temporal
+    ::ref/ref]])

src/metabase/lib/schema/filter.cljc

@@ -3,15 +3,116 @@
   a boolean expression."
   (:require
    [metabase.lib.schema.common :as common]
+   [metabase.lib.schema.ref :as ref]
    [metabase.util.malli.registry :as mr]))
 
-(mr/def ::=
+(def ^:private eq-comparable
+  ;; avoid circular refs between these namespaces.
+  [:schema [:ref :metabase.lib.schema.expression/equality-comparable]])
+
+(def ^:private orderable
+  ;; avoid circular refs between these namespaces.
+  [:schema [:ref :metabase.lib.schema.expression/orderable]])
+
+(defn- defclause
+  [sch & args]
+  {:pre [(qualified-keyword? sch)]}
+  (mr/def sch
+    [:catn
+     [:clause [:= (keyword (name sch))]]
+     [:options ::common/options]
+     (into [:args] args)]))
+
+(doseq [op [::and ::or]]
+  (defclause op [:repeat {:min 2} [:schema [:ref ::filter]]]))
+
+(defclause ::not
+  [:schema [:ref ::filter]])
+
+(doseq [op [::= ::!=]]
+  (defclause op
+    [:repeat {:min 2} eq-comparable]))
+
+(doseq [op [::< ::<= ::> ::>=]]
+  (defclause op
+    [:cat orderable orderable]))
+
+(defclause ::between
+  [:catn [:field orderable] [:min orderable] [:max orderable]])
+
+;; sugar: a pair of `:between` clauses
+(defclause ::inside
+  [:catn
+   [:lat-field orderable]
+   [:lon-field orderable]
+   [:lat-max   orderable]
+   [:lon-min   orderable]
+   [:lat-min   orderable]
+   [:lon-max   orderable]])
+
+;; [:= ... nil], [:!= ... nil], [:or [:= ... nil] [:= ... ""]], [:and [:!= ... nil] [:!= ... ""]]
+(doseq [op [::is-null ::not-null ::is-empty ::not-empty]]
+  (defclause op
+    ::ref/field))
+
+(def ^:private string-filter-options
+  [:map [:case-sensitive {:optional true} :boolean]]) ; default true
+
+(def ^:private string
+  [:schema [:ref :metabase.lib.schema.expression/string]])
+
+;; [:does-not-contain ...] = [:not [:contains ...]]
+(doseq [op [::starts-with ::ends-with ::contains ::does-not-contain]]
+  (mr/def op
+    [:catn
+     [:clause [:= (keyword (name op))]]
+     [:options [:merge ::common/options string-filter-options]]
+     [:args [:catn [:whole string] [:part string]]]]))
+
+(def ^:private time-interval-options
+  [:map [:include-current {:optional true} :boolean]]) ; default false
+
+(def ^:private relative-datetime-unit
+  [:enum :default :minute :hour :day :week :month :quarter :year])
+
+;; SUGAR: rewritten as a filter clause with a relative-datetime value
+(mr/def ::time-interval
   [:catn
-   [:clause [:= :=]]
-   [:options ::common/options]
-   ;; avoid circular refs between these namespaces.
-   [:args [:+ {:min 2} :metabase.lib.schema.expression/equality-comparable]]])
+   [:clause [:= :time-interval]]
+   [:options [:merge ::common/options time-interval-options]]
+   [:args [:catn
+           [:field ::ref/field]
+           [:n [:or :int [:enum :current :last :next]]]
+           [:unit relative-datetime-unit]]]])
+
+(defclause ::segment
+  [:catn [:segment-id [:or ::common/int-greater-than-zero ::common/non-blank-string]]])
+
+(defclause ::case
+  [:+ [:catn
+       [:pred [:schema [:ref ::filter]]]
+       [:expr [:schema [:ref :metabase.lib.schema.expression/boolean]]]]])
 
+;; Boolean literals are not included here because they are not very portable
+;; across different databases. In places where they should also be allowed
+;; the :metabase.lib.schema.expression/boolean schema can be used.
 (mr/def ::filter
   [:or
-   ::=])
+   [:ref ::ref/field]
+   ;; primitive clauses
+   ::and
+   ::or
+   ::not
+   ::= ::!=
+   ::< ::<=
+   ::> ::>=
+   ::between
+   ::starts-with ::ends-with ::contains
+   ::case
+   ;; sugar clauses
+   ::inside
+   ::is-null ::not-null
+   ::is-empty ::not-empty
+   ::does-not-contain
+   ::time-interval
+   ::segment])

src/metabase/util/malli/registry.cljc

@@ -2,11 +2,12 @@
   (:refer-clojure :exclude [def])
   (:require
    [malli.core :as mc]
-   [malli.registry :as mr])
+   [malli.registry :as mr]
+   [malli.util :as mut])
   #?(:cljs (:require-macros [metabase.util.malli.registry])))
 
 (defonce ^:private registry*
-  (atom (mc/default-schemas)))
+  (atom (merge (mc/default-schemas) (mut/schemas))))
 
 (defonce ^:private registry (mr/mutable-registry registry*))
 

test/metabase/lib/schema/filter_test.cljc

+(ns metabase.lib.schema.filter-test
+  (:require
+   [clojure.test :refer [deftest is testing]]
+   [clojure.walk :as walk]
+   [malli.core :as mc]
+   malli.registry
+   ;; expression and filter recursively depend on each other
+   metabase.lib.schema.expression
+   [metabase.lib.schema.filter :as filter]
+   [metabase.util.malli.registry :as mr]))
+
+(defn- ensure-uuids [filter-expr]
+  (walk/postwalk
+   (fn [f]
+     (if (and (vector? f) (not (map-entry? f)))
+       (let [[op & args] f]
+         (cond
+           (and (map? (first args)) (not (:lib/uuid (first args))))
+           (assoc-in f [1 :lib/uuid] (str (random-uuid)))
+
+           (not-any? #(and (map? %) (:lib/uuid %)) args)
+           (into [op {:lib/uuid (str (random-uuid))}] args)
+
+           :else f))
+       f))
+   filter-expr))
+
+(defn- known-filter-ops
+  "Return all registered filter operators."
+  []
+  (into #{}
+        (comp (filter #(and (qualified-keyword? %)
+                            (not= % ::filter/filter)
+                            (= (namespace %) (namespace ::filter/filter))))
+              (map (comp keyword name)))
+        (keys (malli.registry/schemas @#'mr/registry))))
+
+(defn- filter-ops
+  "Return the set of filter operators in `filter-expr`."
+  [filter-expr]
+  (loop [stack [filter-expr] ops #{}]
+    (if (seq stack)
+      (let [top (peek stack)
+            others (pop stack)]
+        (if (and (vector? top)
+                 (keyword? (first top))
+                 (not (#{:field} (first top))))
+          (recur (into others (rest top)) (conj ops (first top)))
+          (recur others ops)))
+      ops)))
+
+(deftest ^:parallel filter-test
+  (testing "valid filters"
+    (let [field [:field 1 {:lib/uuid (str (random-uuid))}]
+          boolean-field [:field 2 {:lib/uuid (str (random-uuid))
+                                   :base-type :type/Boolean}]
+          filter-expr
+          [:and
+           boolean-field
+           [:not [:!= "a" nil]]
+           [:or
+            [:inside 2.0 13.4 34 0 1.0 55]
+            [:between 3 -3 42]
+            [:= true false [:< 13 42] [:<= 33.0 2] [:> 13 42] [:>= 33.0 2]]]
+           [:is-empty field]
+           [:is-null field]
+           [:not-empty field]
+           [:not-null field]
+           [:starts-with "abc" "a"]
+           [:starts-with {:case-sensitive false} "abc" "a"]
+           [:ends-with "abc" "a"]
+           [:ends-with {:case-sensitive true} "abc" "a"]
+           [:contains "abc" "a"]
+           [:contains {:case-sensitive false} "abc" "a"]
+           [:does-not-contain "abc" "a"]
+           [:does-not-contain {:case-sensitive false} "abc" "a"]
+           [:time-interval field :last :hour]
+           [:time-interval field 4 :hour]
+           [:time-interval {:include-current true} field :next :default]
+           [:segment 1]
+           [:segment "segment-id"]
+           [:case [:= 1 1] true [:not-null field] [:< 0 1]]]]
+      (is (= (known-filter-ops) (filter-ops filter-expr)))
+      (is (true? (mc/validate ::filter/filter (ensure-uuids filter-expr))))))
+
+  (testing "invalid filter"
+    (is (false? (mc/validate
+                 ::filter/filter
+                 (ensure-uuids [:xor 13 [:field 1 {:lib/uuid (str (random-uuid))}]]))))))