diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml index 0651fed52066b2eae35b8207517aab84c5fa9044..f6388b9337ddd480b0b59ec1fe8280a5e020be23 100644 --- a/resources/migrations/000_migrations.yaml +++ b/resources/migrations/000_migrations.yaml @@ -11979,6 +11979,15 @@ databaseChangeLog: unique: true tableName: pulse_channel + - changeSet: + id: v44.00-044 + author: qnkhuat + comment: Added 0.44.0 - drop native_query_snippet.template_tags added in v44.00-039 + changes: + - dropColumn: + tableName: native_query_snippet + columnName: template_tags + # >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<< ######################################################################################################################## diff --git a/shared/src/metabase/mbql/normalize.cljc b/shared/src/metabase/mbql/normalize.cljc index 156e73bfbe168766becf647d55c67539cb1e155b..bbfe5b26dbf80d75e52fd64c459b4473d3985050 100644 --- a/shared/src/metabase/mbql/normalize.cljc +++ b/shared/src/metabase/mbql/normalize.cljc @@ -244,7 +244,7 @@ (not (:widget-type tag-def))) (assoc :widget-type :category)))) -(defn normalize-template-tags +(defn- normalize-template-tags "Normalize native-query template tags. Like `expressions` we want to preserve the original name rather than normalize it." [template-tags] diff --git a/shared/src/metabase/mbql/schema.cljc b/shared/src/metabase/mbql/schema.cljc index 49ff90643276c78d00daea1198eabe7826d13e60..5b8c00d5c584c6bc9f9e15f0b81b31bca5156947 100644 --- a/shared/src/metabase/mbql/schema.cljc +++ b/shared/src/metabase/mbql/schema.cljc @@ -913,13 +913,9 @@ ;; ;; Field filters and raw values usually have their value specified by `:parameters` (see [[Parameters]] below). -(def template-tag-types - "List of valid template tag types." - [:snippet :card :dimension :number :text :date]) - (def TemplateTagType "Schema for valid values of template tag `:type`." - (apply s/enum template-tag-types)) + (s/enum :snippet :card :dimension :number :text :date)) (def ^:private TemplateTag:Common "Things required by all template tag types." diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj index 385be93e47451d271c424f68b79b02b9c7cc1092..3877501ebf4b26764741e5a19811d7d65bb9f570 100644 --- a/src/metabase/api/collection.clj +++ b/src/metabase/api/collection.clj @@ -302,22 +302,14 @@ (not (zero? v)) v)) -(declare fully-parametrized-text?) - -(defn- fully-parametrized-snippet? [tag] - (and (= (:type tag) :snippet) - (let [{:keys [content template_tags]} (NativeQuerySnippet (:snippet-id tag))] - (fully-parametrized-text? content template_tags)))) - (defn- fully-parametrized-text? "Decide if `text`, usually (a part of) a query, is fully parametrized given the parameter types - described by `template-tags` (usually the template tags of a native query or a snippet). + described by `template-tags` (usually the template tags of a native query). The rules to consider a piece of text fully parametrized is as follows: - 1. All parameters not in an optional block are field-filters or have a default value. + 1. All parameters not in an optional block are field-filters or snippets or have a default value. 2. All required parameters have a default value. - 3. Rules 1 and 2 recursively apply to all snippets not in an optional block. The first rule is absolutely necessary, as queries violating it cannot be executed without externally supplied parameter values. The second rule is more controversial, as field-filters @@ -332,9 +324,8 @@ (comp (filter params/Param?) (map :k)) (params.parse/parse text))] - (and (every? #(or (= (:type %) :dimension) - (:default %) - (fully-parametrized-snippet? %)) + (and (every? #(or (#{:dimension :snippet} (:type %)) + (:default %)) (map template-tags obligatory-params)) (every? #(or (not (:required %)) (:default %)) diff --git a/src/metabase/api/native_query_snippet.clj b/src/metabase/api/native_query_snippet.clj index 1807df01c92a50259cb176782528e0159bec67c8..5e7084851914af2c6a9a00a000ec4d312b7c45ee 100644 --- a/src/metabase/api/native_query_snippet.clj +++ b/src/metabase/api/native_query_snippet.clj @@ -38,19 +38,17 @@ (api/defendpoint POST "/" "Create a new `NativeQuerySnippet`." - [:as {{:keys [content description name collection_id template_tags]} :body}] + [:as {{:keys [content description name collection_id]} :body}] {content s/Str description (s/maybe s/Str) name native-query-snippet/NativeQuerySnippetName - collection_id (s/maybe su/IntGreaterThanZero) - template_tags (s/maybe su/TemplateTags)} + collection_id (s/maybe su/IntGreaterThanZero)} (check-snippet-name-is-unique name) (let [snippet {:content content :creator_id api/*current-user-id* :description description :name name - :collection_id collection_id - :template_tags (or template_tags {})}] + :collection_id collection_id}] (api/create-check NativeQuerySnippet snippet) (api/check-500 (db/insert! NativeQuerySnippet snippet)))) @@ -61,7 +59,7 @@ (let [snippet (NativeQuerySnippet id) body-fields (u/select-keys-when body :present #{:description :collection_id} - :non-nil #{:archived :content :name :template_tags}) + :non-nil #{:archived :content :name}) [changes] (data/diff body-fields snippet)] (when (seq changes) (api/update-check snippet changes) @@ -72,13 +70,12 @@ (api/defendpoint PUT "/:id" "Update an existing `NativeQuerySnippet`." - [id :as {{:keys [archived content description name collection_id template_tags] :as body} :body}] + [id :as {{:keys [archived content description name collection_id] :as body} :body}] {archived (s/maybe s/Bool) content (s/maybe s/Str) description (s/maybe s/Str) name (s/maybe native-query-snippet/NativeQuerySnippetName) - collection_id (s/maybe su/IntGreaterThanZero) - template_tags (s/maybe su/TemplateTags)} + collection_id (s/maybe su/IntGreaterThanZero)} (check-perms-and-update-snippet! id body)) (api/define-routes) diff --git a/src/metabase/driver/common/parameters.clj b/src/metabase/driver/common/parameters.clj index fa81bc5350fd2425cb213482f1077890cc5c94d2..5adc70c62627ec2588646be793f9597610b3840d 100644 --- a/src/metabase/driver/common/parameters.clj +++ b/src/metabase/driver/common/parameters.clj @@ -32,7 +32,7 @@ ;; ;; `query` is the native query as stored in the Card ;; -;; `params` are positional parameters for a parameterized native query e.g. the JDBC parameters corresponding to +;; `parameters` are positional parameters for a parameterized native query e.g. the JDBC parameters corresponding to ;; `?` placeholders (p.types/defrecord+ ReferencedCardQuery [card-id query params] pretty/PrettyPrintable @@ -44,34 +44,13 @@ [x] (instance? ReferencedCardQuery x)) -;; `ParsedQuerySnippet` is a parsed representation of the content in `NativeQuerySnippet`. -;; It is to be used as an intermediate state when subsituting parameter in a Snippet. -;; -;; `snippet-id` is the integer ID of the row in the application DB from where the snippet content is loaded. -;; -;; `parsed-query` is an array we got from parsing the raw query of the snippet -;; -;; `param->value` is a map with with template tags parsed from the raw query as keys -(p.types/defrecord+ ParsedQuerySnippet [snippet-id parsed-query param->value] - pretty/PrettyPrintable - (pretty [this] - (list (pretty/qualify-symbol-for-*ns* `map->ParsedQuerySnippet) (into {} this)))) - -(defn ParsedQuerySnippet? - "Is `x` an instance of the `ParsedQuerySnippet` record type?" - [x] - (instance? ParsedQuerySnippet x)) - ;; A `ReferencedQuerySnippet` expands to the partial query snippet stored in the `NativeQuerySnippet` table in the ;; application DB. ;; ;; `snippet-id` is the integer ID of the row in the application DB from where the snippet content is loaded. ;; ;; `content` is the raw query snippet which will be replaced, verbatim, for this template tag. -;; -;; `params` are positional parameters for a parameterized native query e.g. the JDBC parameters corresponding to -;; `?` placeholders -(p.types/defrecord+ ReferencedQuerySnippet [snippet-id content params] +(p.types/defrecord+ ReferencedQuerySnippet [snippet-id content] pretty/PrettyPrintable (pretty [this] (list (pretty/qualify-symbol-for-*ns* `map->ReferencedQuerySnippet) (into {} this)))) diff --git a/src/metabase/driver/common/parameters/values.clj b/src/metabase/driver/common/parameters/values.clj index dbdf5aa8bc70b3b830f1ec7740b6c321287bf646..fc5d696c65e66b54914ba8da947bb12e05bfc0dc 100644 --- a/src/metabase/driver/common/parameters/values.clj +++ b/src/metabase/driver/common/parameters/values.clj @@ -11,7 +11,6 @@ (:require [clojure.string :as str] [clojure.tools.logging :as log] [metabase.driver.common.parameters :as params] - [metabase.driver.common.parameters.parse :as params.parse] [metabase.mbql.schema :as mbql.s] [metabase.models.card :refer [Card]] [metabase.models.native-query-snippet :refer [NativeQuerySnippet]] @@ -26,8 +25,7 @@ (:import clojure.lang.ExceptionInfo java.text.NumberFormat java.util.UUID - [metabase.driver.common.parameters CommaSeparatedNumbers FieldFilter - MultipleValues ParsedQuerySnippet ReferencedCardQuery])) + [metabase.driver.common.parameters CommaSeparatedNumbers FieldFilter MultipleValues ReferencedCardQuery ReferencedQuerySnippet])) (defmulti ^:private parse-tag "Parse a tag by its `:type`, returning an appropriate record type such as @@ -160,10 +158,8 @@ :type qp.error-type/invalid-parameter} e)))))) -(declare query->params-map) - -(s/defmethod parse-tag :snippet :- ParsedQuerySnippet - [{:keys [snippet-name snippet-id], :as tag} :- mbql.s/TemplateTag, params] +(s/defmethod parse-tag :snippet :- ReferencedQuerySnippet + [{:keys [snippet-name snippet-id], :as tag} :- mbql.s/TemplateTag, _] (let [snippet-id (or snippet-id (throw (ex-info (tru "Unable to resolve Snippet: missing `:snippet-id`") {:tag tag, :type qp.error-type/invalid-parameter}))) @@ -173,12 +169,10 @@ :snippet-name snippet-name :tag tag :type qp.error-type/invalid-parameter})))] - (params/map->ParsedQuerySnippet - {:snippet-id (:id snippet) - ;; parse the content of snippet so that we could substitute template-tags inside snippet - :parsed-query (params.parse/parse (:content snippet)) - :param->value (query->params-map {:template-tags (:template_tags snippet) - :parameters params})}))) + (params/map->ReferencedQuerySnippet + {:snippet-id (:id snippet) + :content (:content snippet)}))) + ;;; Non-FieldFilter Params (e.g. WHERE x = {{x}}) diff --git a/src/metabase/driver/sql.clj b/src/metabase/driver/sql.clj index 484c78e4b99a8eb97a72a8bda97b225c427c80ec..19ae36656706638dd5d0eb3096291f2cb3e2a991 100644 --- a/src/metabase/driver/sql.clj +++ b/src/metabase/driver/sql.clj @@ -41,8 +41,9 @@ (s/defmethod driver/substitute-native-parameters :sql [_ {:keys [query] :as inner-query} :- {:query su/NonBlankString, s/Keyword s/Any}] - (let [[query params] (sql.params.substitute/substitute (params.parse/parse query) - (params.values/query->params-map inner-query))] + (let [[query params] (-> query + params.parse/parse + (sql.params.substitute/substitute (params.values/query->params-map inner-query)))] (assoc inner-query :query query :params params))) diff --git a/src/metabase/driver/sql/parameters/substitute.clj b/src/metabase/driver/sql/parameters/substitute.clj index 1852c8b1c19550cf0ac66c7ed68d76f19c45d95c..0e244858150fb3ebe39afb44cb8862e30784dad9 100644 --- a/src/metabase/driver/sql/parameters/substitute.clj +++ b/src/metabase/driver/sql/parameters/substitute.clj @@ -8,8 +8,6 @@ [metabase.util :as u] [metabase.util.i18n :refer [tru]])) -(declare substitute) - (defn- substitute-field-filter [[sql args missing] in-optional? k {:keys [_field value], :as v}] (if (and (= params/no-value value) in-optional?) ;; no-value field filters inside optional clauses are ignored, and eventually emitted entirely @@ -24,15 +22,9 @@ (sql.params.substitution/->replacement-snippet-info driver/*driver* v)] [(str sql replacement-snippet) (concat args prepared-statement-args) missing])) -(defn- substitute-native-query-snippet [[sql args missing] {:keys [parsed-query param->value id] :as _v}] - (let [;; substitute template-tags inside snippet - [content snippet-args] (substitute parsed-query param->value) - snippet (params/map->ReferencedQuerySnippet {:content content - :params snippet-args - :snippet-id id}) - {:keys [replacement-snippet prepared-statement-args]} - (sql.params.substitution/->replacement-snippet-info driver/*driver* snippet)] - [(str sql replacement-snippet) (concat args prepared-statement-args) missing])) +(defn- substitute-native-query-snippet [[sql args missing] v] + (let [{:keys [replacement-snippet]} (sql.params.substitution/->replacement-snippet-info driver/*driver* v)] + [(str sql replacement-snippet) args missing])) (defn- substitute-param [param->value [sql args missing] in-optional? {:keys [k]}] (if-not (contains? param->value k) @@ -45,7 +37,7 @@ (params/ReferencedCardQuery? v) (substitute-card-query [sql args missing] v) - (params/ParsedQuerySnippet? v) + (params/ReferencedQuerySnippet? v) (substitute-native-query-snippet [sql args missing] v) (= params/no-value v) diff --git a/src/metabase/driver/sql/parameters/substitution.clj b/src/metabase/driver/sql/parameters/substitution.clj index 6219bd3cb51f87a7aa03004fb6452ff7e7b96b84..98b3f321b6495828cd64468ebd465154f6e952bf 100644 --- a/src/metabase/driver/sql/parameters/substitution.clj +++ b/src/metabase/driver/sql/parameters/substitution.clj @@ -312,6 +312,6 @@ ;;; ---------------------------------- Native Query Snippet replacement snippet info --------------------------------- (defmethod ->replacement-snippet-info [:sql ReferencedQuerySnippet] - [_ {:keys [content params]}] - {:prepared-statement-args (not-empty params) + [_ {:keys [content]}] + {:prepared-statement-args nil :replacement-snippet content}) diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index 824046d6191a62ceb3493b0bf1cc58f2a63e2fe3..027e6060c2a14be6f5f3299337aed0727b86e846 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -96,10 +96,6 @@ :in (comp json-in normalize-parameters-list) :out (comp (catch-normalization-exceptions normalize-parameters-list) json-out-with-keywordization)) -(models/add-type! :template-tags - :in (comp json-in mbql.normalize/normalize-template-tags) - :out (comp (catch-normalization-exceptions mbql.normalize/normalize-template-tags) json-out-with-keywordization)) - (def ^:private MetricSegmentDefinition {(s/optional-key :filter) (s/maybe mbql.s/Filter) (s/optional-key :aggregation) (s/maybe [mbql.s/Aggregation]) diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj index 904d4a3da9ba03395abd6f87c3a0aba349b1ff2b..80d1c745a3adcdade425b48dc22d790eb2281f4c 100644 --- a/src/metabase/models/native_query_snippet.clj +++ b/src/metabase/models/native_query_snippet.clj @@ -21,8 +21,7 @@ #{:snippets}) (defn- pre-insert [snippet] - (u/prog1 (merge {:template_tags {}} - snippet) + (u/prog1 snippet (collection/check-collection-namespace NativeQuerySnippet (:collection_id snippet)))) (defn- pre-update [{:keys [creator_id id], :as updates}] @@ -39,7 +38,6 @@ models/IModelDefaults {:properties (constantly {:timestamped? true :entity_id true}) - :types (constantly {:template_tags :template-tags}) :pre-insert pre-insert :pre-update pre-update}) diff --git a/src/metabase/util/schema.clj b/src/metabase/util/schema.clj index 7dabeb2947bc466d0754855e4e0de73c27b3c2ab..8fdfa00502c818881c598469bfd0650e6467f632 100644 --- a/src/metabase/util/schema.clj +++ b/src/metabase/util/schema.clj @@ -5,7 +5,6 @@ [clojure.string :as str] [clojure.walk :as walk] [medley.core :as m] - [metabase.mbql.schema :as mbql.s] [metabase.types :as types] [metabase.util :as u] [metabase.util.date-2 :as u.date] @@ -353,27 +352,6 @@ s/Keyword s/Any} (deferred-tru "parameter_mapping must be a map with :parameter_id and :target keys"))) -(def ^:private TemplateTagSchema - ;; For the full schema that we use in MBQL, check [[metabase.mbql.schema/TemplateTagMap]] - {(s/either NonBlankString s/Keyword) {:id NonBlankString - :name NonBlankString - :type (apply s/enum (map name mbql.s/template-tag-types)) - (s/optional-key :display-name) s/Str - (s/optional-key :default) s/Any - s/Keyword s/Any}}) - -(def TemplateTags - "Schema for a valid Template tags." - (with-api-error-message - (s/constrained - TemplateTagSchema - (fn [m] - (every? (fn [[tag-name tag-definition]] - (= (name tag-name) (:name tag-definition))) - m)) - "keys in template tag map must match the :name of their values") - (deferred-tru "template tags must be a map with key of name->TemplateTag."))) - (def EmbeddingParams "Schema for a valid map of embedding params." (with-api-error-message (s/maybe {s/Keyword (s/enum "disabled" "enabled" "locked")}) diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj index bfdd808a85f92bc03a621239e7405ef74f507d86..276e97072823b3c27c4f7d0ca6ed452753200b0f 100644 --- a/test/metabase/api/collection_test.clj +++ b/test/metabase/api/collection_test.clj @@ -1122,80 +1122,6 @@ :entity_id (:entity_id card) :model "card" :fully_parametrized true}] - (:data (mt/user-http-request :crowberto :get 200 "collection/root/items")))))) - - (testing "using a not fully parametrized snippet without parameters is false" - (mt/with-temp* [NativeQuerySnippet [snippet {:content "table where x = {{param}}" - :template_tags {"param" {:required false}} - :creator_id (mt/user->id :crowberto) - :name "snippet"}] - Card [card {:name "Business Card" - :dataset_query {:native {:template-tags {:param0 {:required false - :default 0} - :snippet {:name "snippet" - :type :snippet - :snippet-name "snippet" - :snippet-id (:id snippet)}} - :query "select {{param0}} from {{snippet}}"}}}]] - (is (partial= [{:name "Business Card" - :entity_id (:entity_id card) - :model "card" - :fully_parametrized false}] - (:data (mt/user-http-request :crowberto :get 200 "collection/root/items")))))) - - (testing "using a nested snippet without parameters is true" - (mt/with-temp* [NativeQuerySnippet [snippet {:content "table" - :creator_id (mt/user->id :crowberto) - :name "snippet"}] - NativeQuerySnippet [parent-snippet {:content "{{snippet}}" - :creator_id (mt/user->id :crowberto) - :name "parent-snippet" - :template_tags - {"snippet" {:name "snippet" - :type :snippet - :snippet-name "snippet" - :snippet-id (:id snippet)}}}] - Card [card {:name "Business Card" - :dataset_query {:native {:template-tags {:param0 {:required false - :default 0} - :parent-snippet - {:name "parent-snippet" - :type :snippet - :snippet-name "parent-snippet" - :snippet-id (:id parent-snippet)}} - :query "select {{param0}} from {{parent-snippet}}"}}}]] - (is (partial= [{:name "Business Card" - :entity_id (:entity_id card) - :model "card" - :fully_parametrized true}] - (:data (mt/user-http-request :crowberto :get 200 "collection/root/items")))))) - - (testing "using a not fully parametrized nested snippet without parameters is false" - (mt/with-temp* [NativeQuerySnippet [snippet {:content "table where x = {{param}}" - :template_tags {"param" {:required false}} - :creator_id (mt/user->id :crowberto) - :name "snippet"}] - NativeQuerySnippet [parent-snippet {:content "{{snippet}}" - :creator_id (mt/user->id :crowberto) - :name "parent-snippet" - :template_tags - {"snippet" {:name "snippet" - :type :snippet - :snippet-name "snippet" - :snippet-id (:id snippet)}}}] - Card [card {:name "Business Card" - :dataset_query {:native {:template-tags {:param0 {:required false - :default 0} - :parent-snippet - {:name "parent-snippet" - :type :snippet - :snippet-name "parent-snippet" - :snippet-id (:id parent-snippet)}} - :query "select {{param0}} from {{parent-snippet}}"}}}]] - (is (partial= [{:name "Business Card" - :entity_id (:entity_id card) - :model "card" - :fully_parametrized false}] (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))))))))) ;;; ----------------------------------- Effective Children, Ancestors, & Location ------------------------------------ diff --git a/test/metabase/api/native_query_snippet_test.clj b/test/metabase/api/native_query_snippet_test.clj index 45ddb3a8fa9c656729694d90ca8a98b89a4e6c0d..5bcc3c4881f638852e51ecf2a799e278c0037afc 100644 --- a/test/metabase/api/native_query_snippet_test.clj +++ b/test/metabase/api/native_query_snippet_test.clj @@ -201,58 +201,3 @@ (tt/with-temp NativeQuerySnippet [{snippet-id :id}] (is (= {:errors {:collection_id "Collection does not exist."}} ((mt/user->client :rasta) :put 404 (snippet-url snippet-id) {:collection_id Integer/MAX_VALUE})))))))) - -(deftest snippet-with-template-tags-test - (mt/with-model-cleanup [NativeQuerySnippet] - (testing "create" - (let [{snippet-id :id} (mt/user-http-request :crowberto :post 200 (snippet-url) - {:name "A snippet with template-tags" - :content "from products where {{category}}" - :template_tags {"category" {:default nil - :dimension ["field" 1 nil] - :id "random-id-2" - :display-name "Category" - :name "category" - :type "dimension" - :widget-type "string/="}}})] - (testing "get should return template_tags" - (is (= {:category - {:default nil - :dimension ["field" 1 nil] - :id "random-id-2" - :display-name "Category" - :name "category" - :type "dimension" - :widget-type "string/="}} - (:template_tags (mt/user-http-request :crowberto :get 200 (snippet-url snippet-id)))))) - - (testing "update should not allow arbitrary template tag type" - (mt/user-http-request :crowberto :put 400 (snippet-url snippet-id) - {:template_tags {"category" {:default nil - :dimension ["field" 1 nil] - :id "random-id-2" - :display-name "Category" - :name "category" - :type "not-a-type" - :widget-type "string/="}}})) - (testing "update should not allow mismatch template name" - (mt/user-http-request :crowberto :put 400 (snippet-url snippet-id) - {:template_tags {"category" {:default nil - :dimension ["field" 1 nil] - :id "random-id-2" - :display-name "Category" - :name "not-a-template-tag" - :type "dimension" - :widget-type "string/="}}})) - (testing "update type sucessfully" - (is (= {:category {:default nil, - :display-name "Category", - :id "random-id-2" - :name "category" - :type "text"}} - (:template_tags (mt/user-http-request :crowberto :put 200 (snippet-url snippet-id) - {:template_tags {"category" {:default nil - :id "random-id-2" - :display-name "Category" - :name "category" - :type "text"}}}))))))))) diff --git a/test/metabase/driver/common/parameters/values_test.clj b/test/metabase/driver/common/parameters/values_test.clj index 3d65b3127f3612427c9d475f03d32ada97d89638..a7bfb3957c4fc4138eb44c4ffe78c22f97fa7109 100644 --- a/test/metabase/driver/common/parameters/values_test.clj +++ b/test/metabase/driver/common/parameters/values_test.clj @@ -2,7 +2,6 @@ (:require [clojure.test :refer :all] [metabase.driver :as driver] [metabase.driver.common.parameters :as params] - [metabase.driver.common.parameters.parse :as params.parse] [metabase.driver.common.parameters.values :as params.values] [metabase.models :refer [Card Collection NativeQuerySnippet]] [metabase.models.permissions :as perms] @@ -19,7 +18,6 @@ metabase.driver.common.parameters.ReferencedCardQuery)) (def ^:private test-uuid (str (UUID/randomUUID))) -(def ^:private ^{:arglists '([field-name])} param (var-get #'params.parse/param)) (deftest variable-value-test (mt/with-everything-store @@ -394,76 +392,35 @@ (query->params-map query))))))) (deftest snippet-test - (letfn [(query-with-snippet [parameters & {:as snippet-properties}] + (letfn [(query-with-snippet [& {:as snippet-properties}] (assoc (mt/native-query "SELECT * FROM {{expensive-venues}}") :template-tags {"expensive-venues" (merge {:type :snippet :name "expensive-venues" :display-name "Expensive Venues" :snippet-name "expensive-venues"} - snippet-properties)} - :parameters parameters))] + snippet-properties)}))] (testing "`:snippet-id` should be required" (is (thrown? clojure.lang.ExceptionInfo - (query->params-map (query-with-snippet []))))) + (query->params-map (query-with-snippet))))) (testing "If no such Snippet exists, it should throw an Exception" (is (thrown? clojure.lang.ExceptionInfo - (query->params-map (query-with-snippet [] :snippet-id Integer/MAX_VALUE))))) + (query->params-map (query-with-snippet :snippet-id Integer/MAX_VALUE))))) (testing "Snippet parsing should work correctly for a valid Snippet" - (testing "snippet without param" - (mt/with-temp NativeQuerySnippet [{snippet-id :id} {:name "expensive-venues" - :content "venues WHERE price = 4"}] - (let [expected {"expensive-venues" (params/map->ParsedQuerySnippet {:snippet-id snippet-id - :parsed-query ["venues WHERE price = 4"] - :param->value {}})}] + (mt/with-temp NativeQuerySnippet [{snippet-id :id} {:name "expensive-venues" + :content "venues WHERE price = 4"}] + (let [expected {"expensive-venues" (params/map->ReferencedQuerySnippet {:snippet-id snippet-id + :content "venues WHERE price = 4"})}] + (is (= expected + (query->params-map (query-with-snippet :snippet-id snippet-id)))) + + (testing "`:snippet-name` property in query shouldn't have to match `:name` of Snippet in DB" (is (= expected - (query->params-map (query-with-snippet [] :snippet-id snippet-id)))) - - (testing "`:snippet-name` property in query shouldn't have to match `:name` of Snippet in DB" - (is (= expected - (query->params-map (query-with-snippet [] :snippet-id snippet-id, :snippet-name "Old Name")))))))) - - (testing "snippet with params" - (mt/with-temp NativeQuerySnippet [{snippet-id :id} - {:name "expensive-venues" - :content "venues WHERE name = {{name}} and {{price}}" - :template_tags {"price" {:dimension ["field" (mt/id :venues :price) nil] - :display-name "Price" - :id "random-id-1" - :name "price" - :type "dimension" - :widget-type "number/="} - "name" {:display-name "Name" - :id "random-id-2" - :name "name" - :type "text"}}}] - (let [params [{:id "ramdom-id-1", - :target [:variable [:template-tag "name"]], - :type :text, - :value "The Apple Pan"} - {:id "random-id-2", - :target [:dimension [:template-tag "price"]], - :type :string/=, - :value [3]}] - result (query->params-map (query-with-snippet params :snippet-id snippet-id)) - parsed-snippet (get result "expensive-venues")] - (is (= snippet-id (:snippet-id parsed-snippet))) - (is (= ["venues WHERE name = " (param "name") " and " (param "price")] - (:parsed-query parsed-snippet))) - (testing "FieldFilter parameter should have a field and value keys" - (is (= (mt/id :venues :price) - (get-in parsed-snippet [:param->value "price" :field :id]))) - (is (= {:id "random-id-2" - :type :string/= - :value [3]} - (get-in parsed-snippet [:param->value "price" :value])))) - (testing "variable parameter should have value is the value of parameter" - (is (= "The Apple Pan" - (get-in parsed-snippet [:param->value "name"])))))))))) + (query->params-map (query-with-snippet :snippet-id snippet-id, :snippet-name "Old Name")))))))))) (deftest invalid-param-test (testing "Should throw an Exception if we try to pass with a `:type` we don't understand" diff --git a/test/metabase/driver/sql/parameters/substitute_test.clj b/test/metabase/driver/sql/parameters/substitute_test.clj index f813c438f456490936e574fe568a2b12379cb2e5..9dae81c3e529f8b73fa39d5276b8d2c77995bf70 100644 --- a/test/metabase/driver/sql/parameters/substitute_test.clj +++ b/test/metabase/driver/sql/parameters/substitute_test.clj @@ -181,33 +181,11 @@ ;;; --------------------------------------------- Native Query Snippets ---------------------------------------------- (deftest ^:parallel substitute-native-query-snippets-test - (testing "Native query snippet substitution without param" + (testing "Native query snippet substitution" (let [query ["SELECT * FROM test_scores WHERE " (param "snippet:symbol_is_A")]] - (is (= ["SELECT * FROM test_scores WHERE symbol = 'A'" []] - (substitute query {"snippet:symbol_is_A" (params/map->ParsedQuerySnippet {:snippet-id 123 - :parsed-query ["symbol = 'A'"] - :param->value {}})}))))) - - (testing "Native query snippet substitution with param" - (testing "snippet contains one raw param" - (let [query ["SELECT * FROM test_scores WHERE " (param "snippet:symbol_is_A")]] - (is (= ["SELECT * FROM test_scores WHERE symbol = ?" ["A"]] - (substitute query {"snippet:symbol_is_A" (params/map->ParsedQuerySnippet {:snippet-id 123 - :parsed-query ["symbol = " (param "symbol")] - :param->value {"symbol" "A"}})}))))) - - (testing "snippet contains contains multiple params" - (let [query ["SELECT * FROM test_scores WHERE " (param "snippet:symbol_is_A")] - snippet-param->value {"symbol" "A" - "name" (params/map->FieldFilter - {:field (Field (mt/id :categories :name)) - :value {:type :string/= - :value ["American"]}})} - parsed-query-snippet {:snippet-id 123 - :parsed-query ["symbol = " (param "symbol") " and " (param "name")] - :param->value snippet-param->value}] - (is (= ["SELECT * FROM test_scores WHERE symbol = ? and \"PUBLIC\".\"CATEGORIES\".\"NAME\" = ?" ["A" "American"]] - (substitute query {"snippet:symbol_is_A" (params/map->ParsedQuerySnippet parsed-query-snippet)}))))))) + (is (= ["SELECT * FROM test_scores WHERE symbol = 'A'" nil] + (substitute query {"snippet:symbol_is_A" (params/->ReferencedQuerySnippet 123 "symbol = 'A'")})))))) + ;;; ------------------------------------------ simple substitution — {{x}} ------------------------------------------ diff --git a/test/metabase/models/native_query_snippet_test.clj b/test/metabase/models/native_query_snippet_test.clj index aa4cf5587ed4696606ddc376fb11666633f2d20c..067a3832339bd84fabc7fbc3b7dd4a10bb7ff8dc 100644 --- a/test/metabase/models/native_query_snippet_test.clj +++ b/test/metabase/models/native_query_snippet_test.clj @@ -58,32 +58,6 @@ #"A NativeQuerySnippet can only go in Collections in the :snippets namespace" (db/update! NativeQuerySnippet snippet-id :collection_id dest-collection-id))))))) -(deftest normalize-template_tags-test - (testing ":template_tags should get normalized when coming out of the DB" - (mt/with-temp NativeQuerySnippet [{snippet-id :id} {:template_tags {"text" {:display-name "Text-1" - :id "random-id-1" - :name "text-1" - :type "text"} - "field-filter" {:default nil - :dimension ["field" 1 nil] - :id "random-id-2" - :display-name "Field Filter" - :name "field-filter" - :type "dimension" - :widget-type "string/="}}}] - (is (= {"field-filter" {:default nil, - :dimension [:field 1 nil], - :display-name "Field Filter", - :id "random-id-2", - :name "field-filter", - :type :dimension, - :widget-type :string/=}, - "text" {:display-name "Text-1", - :id "random-id-1", - :name "text", - :type :text}} - (db/select-one-field :template_tags NativeQuerySnippet :id snippet-id)))))) - (deftest identity-hash-test (testing "Native query snippet hashes are composed of the name and the collection's hash" (mt/with-temp* [Collection [coll {:name "field-db" :namespace :snippets :location "/"}] diff --git a/test/metabase/query_processor/middleware/parameters_test.clj b/test/metabase/query_processor/middleware/parameters_test.clj index e2a10894f0c5e3743f33e5f691726fbd4d0d1087..c48c6048f1aa3863f1e65489303919841479f744 100644 --- a/test/metabase/query_processor/middleware/parameters_test.clj +++ b/test/metabase/query_processor/middleware/parameters_test.clj @@ -273,7 +273,7 @@ "Filter: expensive venues" (:id where-snippet)})})}]] (testing "multiple snippets are correctly expanded in parent query" (is (= (mt/native-query - {:query "SELECT name, price FROM venues WHERE price > 2", :params []}) + {:query "SELECT name, price FROM venues WHERE price > 2", :params nil}) (substitute-params (:dataset_query card))))) (testing "multiple snippets are expanded from saved sub-query" @@ -284,89 +284,6 @@ {:query (str "SELECT * FROM {{#" (:id card) "}} AS x") :template-tags (card-template-tags [(:id card)])}))))))) -(deftest snippet-with-parameters-test - (mt/with-everything-store - (mt/with-temp* [NativeQuerySnippet [where-snippet {:content "name = {{name}} and {{price}} " - :template_tags {"price" {:dimension ["field" (mt/id :venues :price) nil] - :display-name "Price" - :id "random-id-1" - :name "price" - :type "dimension" - :widget-type "number/="} - "name" {:display-name "Name" - :id "random-id-2" - :name "name" - :type "text"}} - :creator_id (mt/user->id :rasta) - :description "Meant for use in WHERE clause" - :name "Filter: expensive venues"}] - Card [card {:dataset_query - (mt/native-query - {:query (str "SELECT name, price " - "FROM venues " - "WHERE {{ Filter: expensive venues }}") - :template-tags (snippet-template-tags - {"Filter: expensive venues" (:id where-snippet)})})}]] - (testing "parameters in snippet should be expanded correctly" - (is (= (mt/native-query - {:query "SELECT name, price FROM venues WHERE name = ? and \"PUBLIC\".\"VENUES\".\"PRICE\" IN (2)" - :params ["The Apple Pan"]}) - (-> (:dataset_query card) - (assoc :parameters [{:id "random-id-1" - :type :category - :target [:dimension [:template-tag "price"]] - :value [2]} - {:id "random-id-2" - :target [:variable [:template-tag "name"]] - :type :text - :value "The Apple Pan"}]) - substitute-params - (dissoc :user-parameters)))))))) - -(deftest nested-snippet-with-parameter-test - ;; we currently don't use this in FE, but this test is here to demonstrate that we can - (mt/with-everything-store - (mt/with-temp* [NativeQuerySnippet [child-snippet {:content "price > {{price}}" - :template_tags {"price" {:display-name "Price" - :id "random-id-2" - :name "price" - :type "number"}} - :creator_id (mt/user->id :rasta) - :description "Meant for use in WHERE clause" - :name "Filter: expensive venues"}] - NativeQuerySnippet [nested-snipet {:content "name = {{name}} and {{Filter: Expensive Venues}}" - :template_tags (merge - {"name" {:display-name "Name" - :id "random-id-2" - :name "name" - :type "text"}} - (snippet-template-tags {"Filter: Expensive Venues" (:id child-snippet)})) - :creator_id (mt/user->id :rasta) - :description "Meant for use in WHERE clause" - :name "Filter: nested snippet"}] - Card [card {:dataset_query - (mt/native-query - {:query (str "SELECT name, price " - "FROM venues " - "WHERE {{ Filter: nested snippet }}") - :template-tags (snippet-template-tags - {"Filter: nested snippet" (:id nested-snipet)})})}]] - (testing "substitute nested snippet in snippet should be expanded correctly" - (is (= (mt/native-query - {:query "SELECT name, price FROM venues WHERE name = ? and price > 2" - :params ["The Apple Pan"]}) - (-> (:dataset_query card) - (assoc :parameters [{:id "random-id-1" - :type :number - :target [:variable [:template-tag "price"]] - :value 2} - {:id "random-id-2" - :target [:variable [:template-tag "name"]] - :type :text - :value "The Apple Pan"}]) - substitute-params - (dissoc :user-parameters)))))))) - (deftest include-card-parameters-test (testing "Expanding a Card reference should include its parameters (#12236)" (mt/dataset sample-dataset