diff --git a/src/metabase/driver/common/parameters/operators.clj b/src/metabase/driver/common/parameters/operators.clj index 7ba42202953cfe924047c7e98bfba69ca454de79..9cba0e1172cd8689dd962057dab3b552b0556147 100644 --- a/src/metabase/driver/common/parameters/operators.clj +++ b/src/metabase/driver/common/parameters/operators.clj @@ -19,6 +19,14 @@ [param-type] (get-in lib.schema.parameter/types [param-type :operator])) +(defn- operator-options-fn + [param-type] + (get-in lib.schema.parameter/types [param-type :options-fn] + ;; Default is to conj on the end if options are provided. + (fn [clause options] + (cond-> clause + options (conj options))))) + (defn operator? "Returns whether param-type is an \"operator\" type." [param-type] @@ -60,19 +68,12 @@ `:type qp.error-type/invalid-parameter` if arity is incorrect." [{param-type :type [a b :as param-value] :value [_ field :as _target] :target options :options :as _param}] (verify-type-and-arity field param-type param-value) - (let [field' (mbql.u/wrap-field-id-if-needed field)] + (let [field' (mbql.u/wrap-field-id-if-needed field) + opts-fn (operator-options-fn param-type)] (case (operator-arity param-type) - :binary - (cond-> [(keyword (name param-type)) field' a b] - (boolean options) (conj options)) - - :unary - (cond-> [(keyword (name param-type)) field' a] - (boolean options) (conj options)) - - :variadic - (cond-> (into [(keyword (name param-type)) field'] param-value) - (boolean options) (conj options)) + :binary (opts-fn [(keyword (name param-type)) field' a b] options) + :unary (opts-fn [(keyword (name param-type)) field' a] options) + :variadic (opts-fn (into [(keyword (name param-type)) field'] param-value) options) (throw (ex-info (format "Unrecognized operator: %s" param-type) {:param-type param-type diff --git a/src/metabase/lib/schema/parameter.cljc b/src/metabase/lib/schema/parameter.cljc index fdfc1534dc1c9baf224693020c325cecf65fcfec..4897f310aca7a1be64338c9387ab81459559e064 100644 --- a/src/metabase/lib/schema/parameter.cljc +++ b/src/metabase/lib/schema/parameter.cljc @@ -25,6 +25,15 @@ [metabase.lib.schema.common :as lib.schema.common] [metabase.util.malli.registry :as mr])) +(defn- variadic-opts-first + "Some clauses, like `:contains`, have optional `options` last in their binary form, and required options first in + variadic form." + [[tag & args :as clause] options] + (if (<= (count args) 2) + (cond-> clause + options (conj options)) + (into [tag (or options {})] args))) + (def types "Map of parameter-type -> info. Info is a map with the following keys: @@ -49,7 +58,12 @@ with a widget type `:date`. Why? It's a potential security risk if someone creates a Card with an \"exact-match\" Field filter like `:date` or `:text` and you pass in a parameter like `string/!=` `NOTHING_WILL_MATCH_THIS`. Non-exact-match parameters can be abused to enumerate *all* the rows in a table when the parameter was supposed to - lock the results down to a single row or set of rows." + lock the results down to a single row or set of rows. + + ### `:options-fn` + + Optional, specifies a function `(f clause-without-options options-map-or-nil) => clause-with-options` to be used for + attaching the options. The default is to `conj` non-nil options on the end." {;; the basic raw-value types. These can be used with [[TemplateTag:RawValue]] template tags as well as ;; [[TemplateTag:FieldFilter]] template tags. :number {:type :numeric, :allowed-for #{:number :number/= :id :category :location/zip_code}} @@ -120,10 +134,10 @@ :string/= {:type :string, :operator :variadic, :allowed-for #{:string/= :text :id :category :location/city :location/state :location/zip_code :location/country}} - :string/contains {:type :string, :operator :unary, :allowed-for #{:string/contains}} - :string/does-not-contain {:type :string, :operator :unary, :allowed-for #{:string/does-not-contain}} - :string/ends-with {:type :string, :operator :unary, :allowed-for #{:string/ends-with}} - :string/starts-with {:type :string, :operator :unary, :allowed-for #{:string/starts-with}}}) + :string/contains {:type :string, :operator :variadic, :options-fn variadic-opts-first, :allowed-for #{:string/contains}} + :string/does-not-contain {:type :string, :operator :variadic, :options-fn variadic-opts-first, :allowed-for #{:string/does-not-contain}} + :string/ends-with {:type :string, :operator :variadic, :options-fn variadic-opts-first, :allowed-for #{:string/ends-with}} + :string/starts-with {:type :string, :operator :variadic, :options-fn variadic-opts-first, :allowed-for #{:string/starts-with}}}) (mr/def ::type (into [:enum {:error/message "valid parameter type" diff --git a/test/metabase/driver/common/parameters/operators_test.clj b/test/metabase/driver/common/parameters/operators_test.clj index 8c79ebb8fa16668ce67cf2996f3556d5c1ae22fe..890366bce7425063cd6213ca936520a186edfb21 100644 --- a/test/metabase/driver/common/parameters/operators_test.clj +++ b/test/metabase/driver/common/parameters/operators_test.clj @@ -43,7 +43,34 @@ 26 {:source-field 5}]] :value ["foo"]}) - [:starts-with [:field 26 {:source-field 5}] "foo"])))) + [:starts-with [:field 26 {:source-field 5}] "foo"])) + (testing "with options" + (is (= (params.ops/to-clause {:type :string/starts-with + :target [:dimension + [:field + 26 + {:source-field 5}]] + :value ["foo"] + :options {:case-insensitive false}}) + [:starts-with [:field 26 {:source-field 5}] "foo" {:case-insensitive false}]))) + (testing "with multiple arguments" + (is (= (params.ops/to-clause {:type :string/starts-with + :target [:dimension + [:field + 26 + {:source-field 5}]] + :value ["foo" "bar"]}) + [:starts-with {} [:field 26 {:source-field 5}] "foo" "bar"])) + + (testing "with options" + (is (= (params.ops/to-clause {:type :string/starts-with + :target [:dimension + [:field + 26 + {:source-field 5}]] + :value ["foo" "bar"] + :options {:case-insensitive false}}) + [:starts-with {:case-insensitive false} [:field 26 {:source-field 5}] "foo" "bar"])))))) (deftest ^:parallel to-clause-test-5 (testing "string operations" @@ -68,7 +95,7 @@ (is (not result) "Did not throw")) (catch Exception e (ex-data e))))] - (doseq [[op values] [[:string/starts-with ["a" "b"]] + (doseq [[op values] [[:number/>= [2 4]] [:number/between [1]] [:number/between [1 2 3]]]] (is (=? {:param-type op