Skip to content
Snippets Groups Projects
Unverified Commit cb423767 authored by metamben's avatar metamben Committed by GitHub
Browse files

Handle snippets when determining if a query is fully parametrized (#24069)

parent da0ad3b1
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/collection.clj
+ 30
− 12
View file @ cb423767
......@@ -16,6 +16,7 @@
[metabase.db :as mdb]
[metabase.driver.common.parameters :as params]
[metabase.driver.common.parameters.parse :as params.parse]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.models.card :refer [Card]]
[metabase.models.collection :as collection :refer [Collection]]
[metabase.models.collection.graph :as graph]
......@@ -301,13 +302,22 @@
(not (zero? v))
v))
(defn- fully-parametrized?
"Decide if the query in the card represented by the result row `row` is fully parametrized.
(declare fully-parametrized-text?)
The rules to consider a query fully parametrized is as follows:
(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).
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.
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
......@@ -317,22 +327,30 @@
and queries that are technically executable without parameters can be unacceptably slow
without the necessary constraints. (Marking parameters in optional blocks as required doesn't
seem to be useful any way, but if the user said it is required, we honor this flag.)"
[row]
(let [native-query (-> row :dataset_query (json/parse-string keyword) :native)]
[text template-tags]
(let [obligatory-params (into #{}
(comp (filter params/Param?)
(map :k))
(params.parse/parse text))]
(and (every? #(or (= (:type %) :dimension)
(:default %)
(fully-parametrized-snippet? %))
(map template-tags obligatory-params))
(every? #(or (not (:required %))
(:default %))
(vals template-tags)))))
(defn- fully-parametrized-query? [row]
(let [native-query (-> row :dataset_query json/parse-string mbql.normalize/normalize :native)]
(if-let [template-tags (:template-tags native-query)]
(let [obligatory-params (into #{}
(comp (filter params/Param?)
(map (comp keyword :k)))
(-> native-query :query params.parse/parse))]
(and (every? #(or (:default %) (= (:type %) "dimension")) (map template-tags obligatory-params))
(every? #(or (:default %) (not (:required %))) (vals template-tags))))
(fully-parametrized-text? (:query native-query) template-tags)
true)))
(defn- post-process-card-row [row]
(-> row
(dissoc :authority_level :icon :personal_owner_id :dataset_query)
(update :collection_preview bit->boolean)
(assoc :fully_parametrized (fully-parametrized? row))))
(assoc :fully_parametrized (fully-parametrized-query? row))))
(defmethod post-process-collection-children :card
[_ rows]
......
test/metabase/api/collection_test.clj
+ 112
− 72
View file @ cb423767
......@@ -1065,72 +1065,33 @@
:param1 {:required false}
:param2 {:required false}}
:query "select {{param0}}, {{param1}} [[ , {{param2}} ]]"}}}]
(is (= (let [collection (collection/user->personal-collection (mt/user->id :crowberto))]
[{:name "Business Card"
:description nil
:collection_position nil
:collection_preview true
:display "table"
:moderated_status nil
:entity_id (:entity_id card)
:model "card"
:fully_parametrized false}
{:name "Crowberto Corv's Personal Collection"
:description nil
:model "collection"
:authority_level nil
:entity_id (:entity_id collection)
:can_write true}])
(for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))]
(dissoc item :id))))))
(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 "is false even if a required field-filter parameter has no default"
(mt/with-temp Card [card {:name "Business Card"
:dataset_query {:native {:template-tags {:param0 {:default 0}
:param1 {:type "dimension", :required true}}
:query "select {{param0}}, {{param1}}"}}}]
(is (= (let [collection (collection/user->personal-collection (mt/user->id :crowberto))]
[{:name "Business Card"
:description nil
:collection_position nil
:collection_preview true
:display "table"
:moderated_status nil
:entity_id (:entity_id card)
:model "card"
:fully_parametrized false}
{:name "Crowberto Corv's Personal Collection"
:description nil
:model "collection"
:authority_level nil
:entity_id (:entity_id collection)
:can_write true}])
(for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))]
(dissoc item :id))))))
(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 "is false even if an optional required parameter has no default"
(mt/with-temp Card [card {:name "Business Card"
:dataset_query {:native {:template-tags {:param0 {:default 0}
:param1 {:required true}}
:query "select {{param0}}, [[ , {{param1}} ]]"}}}]
(is (= (let [collection (collection/user->personal-collection (mt/user->id :crowberto))]
[{:name "Business Card"
:description nil
:collection_position nil
:collection_preview true
:display "table"
:moderated_status nil
:entity_id (:entity_id card)
:model "card"
:fully_parametrized false}
{:name "Crowberto Corv's Personal Collection"
:description nil
:model "collection"
:authority_level nil
:entity_id (:entity_id collection)
:can_write true}])
(for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))]
(dissoc item :id))))))
(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 "is true if all obligatory parameters have defaults"
(mt/with-temp Card [card {:name "Business Card"
......@@ -1139,24 +1100,103 @@
:param2 {}
:param3 {:type "dimension"}}
:query "select {{param0}}, {{param1}} [[ , {{param2}} ]] from t {{param3}}"}}}]
(is (= (let [collection (collection/user->personal-collection (mt/user->id :crowberto))]
[{:name "Business Card"
:description nil
:collection_position nil
:collection_preview true
:display "table"
:moderated_status nil
:entity_id (:entity_id card)
:model "card"
:fully_parametrized true}
{:name "Crowberto Corv's Personal Collection"
:description nil
:model "collection"
:authority_level nil
:entity_id (:entity_id collection)
:can_write true}])
(for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))]
(dissoc item :id)))))))))
(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 snippet without parameters is true"
(mt/with-temp* [NativeQuerySnippet [snippet {:content "table"
: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 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 ------------------------------------
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment