From 8845188509d2954e753ba855e4b5ee203889be9f Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Thu, 10 Aug 2023 18:45:01 -0700
Subject: [PATCH] Test metadata helper function consolidation (#33031)
* Test metadata helper function consolidation
* More code consolidation
---
src/metabase/lib/core.cljc | 3 +-
src/metabase/lib/query.cljc | 19 +--
test/metabase/lib/card_test.cljc | 23 ++--
test/metabase/lib/column_group_test.cljc | 20 ++--
test/metabase/lib/field_test.cljc | 30 +++--
test/metabase/lib/filter_test.cljc | 6 +-
test/metabase/lib/join_test.cljc | 18 +--
test/metabase/lib/metadata/jvm_test.clj | 22 ++--
test/metabase/lib/metadata_test.cljc | 7 +-
test/metabase/lib/native_test.cljc | 17 ++-
test/metabase/lib/query_test.cljc | 14 +--
test/metabase/lib/test_metadata.cljc | 141 ++++-------------------
test/metabase/lib/test_util.cljc | 135 ++++++++++++----------
13 files changed, 174 insertions(+), 281 deletions(-)
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index ffba3a97dd9..9d3af8f64c3 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -224,8 +224,7 @@
[lib.normalize
normalize]
[lib.query
- query
- saved-question-query]
+ query]
[lib.ref
ref]
[lib.remove-replace
diff --git a/src/metabase/lib/query.cljc b/src/metabase/lib/query.cljc
index 366cb337e70..38b5499776a 100644
--- a/src/metabase/lib/query.cljc
+++ b/src/metabase/lib/query.cljc
@@ -10,8 +10,6 @@
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.id :as lib.schema.id]
[metabase.lib.util :as lib.util]
- [metabase.shared.util.i18n :as i18n]
- [metabase.util.log :as log]
[metabase.util.malli :as mu]))
(defmethod lib.normalize/normalize :mbql/query
@@ -62,7 +60,7 @@
(mu/defn ^:private query-from-existing :- ::lib.schema/query
[metadata-providerable :- lib.metadata/MetadataProviderable
query :- lib.util/LegacyOrPMBQLQuery]
- (let [query (lib.util/pipeline query)]
+ (let [query (lib.convert/->pMBQL query)]
(query-with-stages metadata-providerable (:stages query))))
(defmulti ^:private ->query
@@ -96,21 +94,6 @@
x]
(->query metadata-providerable x))
-(mu/defn saved-question-query :- ::lib.schema/query
- "Convenience for creating a query from a Saved Question (i.e., a Card)."
- [metadata-providerable :- lib.metadata/MetadataProviderable
- {mbql-query :dataset-query, metadata :result-metadata, :as saved-question}]
- (assert mbql-query (i18n/tru "Saved Question is missing query"))
- (when-not metadata
- (log/warn (i18n/trs "Saved Question {0} {1} is missing result metadata"
- (:id saved-question)
- (pr-str (:name saved-question-query)))))
- (let [mbql-query (cond-> (assoc (lib.convert/->pMBQL mbql-query)
- :lib/metadata (lib.metadata/->metadata-provider metadata-providerable))
- metadata
- (lib.util/update-query-stage -1 assoc :lib/stage-metadata (lib.util/->stage-metadata metadata)))]
- (query metadata-providerable mbql-query)))
-
(mu/defn query-from-legacy-inner-query :- ::lib.schema/query
"Create a pMBQL query from a legacy inner query."
[metadata-providerable :- lib.metadata/MetadataProviderable
diff --git a/test/metabase/lib/card_test.cljc b/test/metabase/lib/card_test.cljc
index 6132a224d6a..3be4736199b 100644
--- a/test/metabase/lib/card_test.cljc
+++ b/test/metabase/lib/card_test.cljc
@@ -1,5 +1,6 @@
(ns metabase.lib.card-test
(:require
+ [clojure.set :as set]
[clojure.test :refer [deftest is testing]]
[metabase.lib.core :as lib]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
@@ -49,26 +50,22 @@
(lib/display-info query col))))))))
(deftest ^:parallel card-source-query-metadata-test
- (doseq [metadata [{:id 1
- :name "My Card"
- :result-metadata meta/card-results-metadata
- :dataset-query {}}
- ;; in some cases [the FE unit tests are broken] the FE is transforming the metadata like this, not sure why but handle it anyway
+ (doseq [metadata [(:venues lib.tu/mock-cards)
+ ;; in some cases [the FE unit tests are broken] the FE is transforming the metadata like this, not
+ ;; sure why but handle it anyway
;; (#29739)
- {:id 1
- :name "My Card"
- :fields meta/card-results-metadata
- :dataset-query {}}]]
+ (set/rename-keys (:venues lib.tu/mock-cards) {:result-metadata :fields})]]
(testing (str "metadata = \n" (u/pprint-to-str metadata))
(let [query {:lib/type :mbql/query
:lib/metadata (lib.tu/mock-metadata-provider
{:cards [metadata]})
:database (meta/id)
:stages [{:lib/type :mbql.stage/mbql
- :lib/options {:lib/uuid (str (random-uuid))}
- :source-card 1}]}]
- (is (=? (for [col meta/card-results-metadata]
- (assoc col :lib/source :source/card))
+ :source-card (:id metadata)}]}]
+ (is (=? (for [col (get-in lib.tu/mock-cards [:venues :result-metadata])]
+ (-> col
+ (assoc :lib/source :source/card)
+ (dissoc :fk-target-field-id)))
(lib.metadata.calculation/returned-columns query)))))))
(deftest ^:parallel card-results-metadata-merge-metadata-provider-metadata-test
diff --git a/test/metabase/lib/column_group_test.cljc b/test/metabase/lib/column_group_test.cljc
index 9880ab69e76..48f4005841e 100644
--- a/test/metabase/lib/column_group_test.cljc
+++ b/test/metabase/lib/column_group_test.cljc
@@ -275,31 +275,31 @@
(testing "#32509 when building a join against a Card"
(doseq [{:keys [message card metadata-provider]}
[{:message "MBQL Card"
- :card lib.tu/categories-mbql-card
- :metadata-provider lib.tu/metadata-provider-with-categories-mbql-card}
+ :card (:categories lib.tu/mock-cards)
+ :metadata-provider lib.tu/metadata-provider-with-mock-cards}
{:message "Native Card"
- :card lib.tu/categories-native-card
- :metadata-provider lib.tu/metadata-provider-with-categories-native-card}]]
+ :card (lib.tu/mock-cards :categories/native)
+ :metadata-provider lib.tu/metadata-provider-with-mock-cards}]]
(testing message
(let [cols (rhs-columns lib.tu/venues-query card)
groups (lib/group-columns cols)]
(testing `lib/group-columns
(is (=? [{:lib/type :metadata/column-group
- :card-id 1
+ :card-id (:id card)
::lib.column-group/group-type :group-type/join.explicit
- ::lib.column-group/columns [{:name "ID", :lib/card-id 1}
- {:name "NAME", :lib/card-id 1}]}]
+ ::lib.column-group/columns [{:name "ID", :lib/card-id (:id card)}
+ {:name "NAME", :lib/card-id (:id card)}]}]
groups)))
(testing `lib/display-info
(testing "Card is not present in MetadataProvider"
- (is (=? [{:display-name "Question 1"
+ (is (=? [{:display-name (str "Question " (:id card))
:is-from-join true}]
(for [group groups]
(lib/display-info lib.tu/venues-query group)))))
(testing "Card *is* present in MetadataProvider"
(let [query (assoc lib.tu/venues-query :lib/metadata metadata-provider)]
- (is (=? [{:name "Tarot Card"
- :display-name "Tarot Card"
+ (is (=? [{:name "Mock categories card"
+ :display-name "Mock Categories Card"
:is-from-join true}]
(for [group groups]
(lib/display-info query group))))))))))))
diff --git a/test/metabase/lib/field_test.cljc b/test/metabase/lib/field_test.cljc
index 552389bd8ff..353257da75b 100644
--- a/test/metabase/lib/field_test.cljc
+++ b/test/metabase/lib/field_test.cljc
@@ -21,9 +21,9 @@
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
(deftest ^:parallel field-from-results-metadata-test
- (let [field-metadata (lib.metadata/stage-column (lib/saved-question-query
+ (let [field-metadata (lib.metadata/stage-column (lib.tu/query-with-stage-metadata-from-card
meta/metadata-provider
- meta/saved-question)
+ (:venues lib.tu/mock-cards))
"ID")]
(is (=? {:lib/type :metadata/column
:name "ID"}
@@ -151,9 +151,7 @@
;; legacy result metadata will already include the Join name in the `:display-name`, so simulate that. Make
;; sure we're not including it twice.
result-metadata (for [col (lib.metadata.calculation/returned-columns
- (lib/saved-question-query
- meta/metadata-provider
- {:dataset-query card-query}))]
+ (lib/query meta/metadata-provider card-query))]
(cond-> col
(:source-alias col)
(update :display-name (fn [display-name]
@@ -459,18 +457,16 @@
(lib/display-info query))))))))
(deftest ^:parallel joined-field-column-name-test
- (let [card {:dataset-query {:database (meta/id)
- :type :query
- :query {:source-table (meta/id :venues)
- :joins [{:fields :all
- :source-table (meta/id :categories)
- :conditions [[:=
- [:field (meta/id :venues :category-id) nil]
- [:field (meta/id :categories :id) {:join-alias "Cat"}]]]
- :alias "Cat"}]}}}
- query (lib/saved-question-query
- meta/metadata-provider
- card)]
+ (let [legacy-query {:database (meta/id)
+ :type :query
+ :query {:source-table (meta/id :venues)
+ :joins [{:fields :all
+ :source-table (meta/id :categories)
+ :conditions [[:=
+ [:field (meta/id :venues :category-id) nil]
+ [:field (meta/id :categories :id) {:join-alias "Cat"}]]]
+ :alias "Cat"}]}}
+ query (lib/query meta/metadata-provider legacy-query)]
(is (=? [{:lib/desired-column-alias "ID"}
{:lib/desired-column-alias "NAME"}
{:lib/desired-column-alias "CATEGORY_ID"}
diff --git a/test/metabase/lib/filter_test.cljc b/test/metabase/lib/filter_test.cljc
index 1fd28bb8f7f..a22042b57e3 100644
--- a/test/metabase/lib/filter_test.cljc
+++ b/test/metabase/lib/filter_test.cljc
@@ -8,12 +8,14 @@
[metabase.lib.test-util :as lib.tu]
[metabase.lib.types.isa :as lib.types.isa]))
+#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
+
(defn- test-clause [result-filter f & args]
(is (=? result-filter
(apply f args))))
(deftest ^:parallel general-filter-clause-test
- (let [q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
+ (let [q2 (lib.tu/query-with-stage-metadata-from-card meta/metadata-provider (:venues lib.tu/mock-cards))
venues-category-id-metadata (meta/field-metadata :venues :category-id)
venues-name-metadata (meta/field-metadata :venues :name)
venues-latitude-metadata (meta/field-metadata :venues :latitude)
@@ -107,7 +109,7 @@
(deftest ^:parallel filter-test
(let [q1 (lib/query meta/metadata-provider (meta/table-metadata :categories))
- q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
+ q2 (lib.tu/query-with-stage-metadata-from-card meta/metadata-provider (:venues lib.tu/mock-cards))
venues-category-id-metadata (meta/field-metadata :venues :category-id)
original-filter
[:between
diff --git a/test/metabase/lib/join_test.cljc b/test/metabase/lib/join_test.cljc
index 187f0ae22a2..dafa5e4fcf4 100644
--- a/test/metabase/lib/join_test.cljc
+++ b/test/metabase/lib/join_test.cljc
@@ -108,7 +108,7 @@
(meta/id :venues :category-id)]]]}]}]}
(-> (lib/query meta/metadata-provider (meta/table-metadata :categories))
(lib/join (lib/join-clause
- (lib/saved-question-query meta/metadata-provider meta/saved-question)
+ (lib.tu/query-with-stage-metadata-from-card meta/metadata-provider (:venues lib.tu/mock-cards))
[(lib/= (meta/field-metadata :categories :id)
(meta/field-metadata :venues :category-id))]))
(dissoc :lib/metadata)))))
@@ -116,7 +116,7 @@
(deftest ^:parallel join-condition-field-metadata-test
(testing "Should be able to use raw Field metadatas in the join condition"
(let [q1 (lib/query meta/metadata-provider (meta/table-metadata :categories))
- q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
+ q2 (lib.tu/query-with-stage-metadata-from-card meta/metadata-provider (:venues lib.tu/mock-cards))
venues-category-id-metadata (meta/field-metadata :venues :category-id)
categories-id-metadata (lib.metadata/stage-column q2 "ID")]
(let [clause (lib/join-clause q2 [(lib/= categories-id-metadata venues-category-id-metadata)])]
@@ -739,12 +739,12 @@
(testing "RHS columns when building a join against"
(doseq [query [lib.tu/venues-query
lib.tu/query-with-join]
- [card-type card] {"Native" lib.tu/categories-native-card
- "MBQL" lib.tu/categories-mbql-card}]
+ [card-type card] {"Native" (:categories/native lib.tu/mock-cards)
+ "MBQL" (:categories lib.tu/mock-cards)}]
(testing (str "a " card-type " Card")
(let [cols (lib/join-condition-rhs-columns query card nil nil)]
- (is (=? [{:display-name "ID", :lib/source :source/joins, :lib/card-id 1}
- {:display-name "Name", :lib/source :source/joins, :lib/card-id 1}]
+ (is (=? [{:display-name "ID", :lib/source :source/joins, :lib/card-id (:id card)}
+ {:display-name "Name", :lib/source :source/joins, :lib/card-id (:id card)}]
cols))
(is (=? [{:display-name "ID", :is-from-join true}
{:display-name "Name", :is-from-join true}]
@@ -767,7 +767,7 @@
(let [query lib.tu/venues-query]
(doseq [lhs [nil (lib.metadata/field query (meta/id :venues :id))]
joined-thing [(meta/table-metadata :venues)
- meta/saved-question-CardMetadata]]
+ (:venues lib.tu/mock-cards)]]
(testing (str "lhs = " (pr-str lhs)
"\njoined-thing = " (pr-str joined-thing))
(is (=? [{:lib/desired-column-alias "ID"}
@@ -898,7 +898,7 @@
{:lib/type :metadata/column, :name "PRICE"}]
(lib/joinable-columns lib.tu/venues-query -1 table-or-card))
(meta/table-metadata :venues)
- meta/saved-question-CardMetadata))
+ (:venues lib.tu/mock-cards)))
(deftest ^:parallel joinable-columns-join-test
(let [query lib.tu/query-with-join
@@ -960,7 +960,7 @@
2 (lib.tu/add-joins query "J1" "J2")}
[first-join? join? join-or-joinable] (list*
[(zero? num-existing-joins) false (meta/table-metadata :venues)]
- [(zero? num-existing-joins) false meta/saved-question-CardMetadata]
+ [(zero? num-existing-joins) false (:venues lib.tu/mock-cards)]
[(zero? num-existing-joins) false nil]
(when-let [[first-join & more] (not-empty (lib/joins query))]
(cons [true true first-join]
diff --git a/test/metabase/lib/metadata/jvm_test.clj b/test/metabase/lib/metadata/jvm_test.clj
index cb12b22cdd1..df9109f787f 100644
--- a/test/metabase/lib/metadata/jvm_test.clj
+++ b/test/metabase/lib/metadata/jvm_test.clj
@@ -25,18 +25,16 @@
(is (nil? (lib.metadata.protocols/database (lib.metadata.jvm/application-database-metadata-provider Integer/MAX_VALUE))))))
(deftest ^:parallel saved-question-metadata-test
- (let [card {:dataset-query {:database (mt/id)
- :type :query
- :query {:source-table (mt/id :venues)
- :joins [{:fields :all
- :source-table (mt/id :categories)
- :condition [:=
- [:field (mt/id :venues :category_id) nil]
- [:field (mt/id :categories :id) {:join-alias "Cat"}]]
- :alias "Cat"}]}}}
- query (lib/saved-question-query
- (lib.metadata.jvm/application-database-metadata-provider (mt/id))
- card)]
+ (let [query {:database (mt/id)
+ :type :query
+ :query {:source-table (mt/id :venues)
+ :joins [{:fields :all
+ :source-table (mt/id :categories)
+ :condition [:=
+ [:field (mt/id :venues :category_id) nil]
+ [:field (mt/id :categories :id) {:join-alias "Cat"}]]
+ :alias "Cat"}]}}
+ query (lib/query (lib.metadata.jvm/application-database-metadata-provider (mt/id)) query)]
(is (=? [{:lib/desired-column-alias "ID"}
{:lib/desired-column-alias "NAME"}
{:lib/desired-column-alias "CATEGORY_ID"}
diff --git a/test/metabase/lib/metadata_test.cljc b/test/metabase/lib/metadata_test.cljc
index d2325872b08..bb690600b98 100644
--- a/test/metabase/lib/metadata_test.cljc
+++ b/test/metabase/lib/metadata_test.cljc
@@ -8,6 +8,7 @@
[metabase.lib.test-util :as lib.tu]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
+(comment lib/keep-me)
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
(deftest ^:parallel field-metadata-test
@@ -17,7 +18,7 @@
(lib.metadata/field meta/metadata-provider (meta/id :venues :category-id)))))
(deftest ^:parallel stage-metadata-test
- (let [query (lib/saved-question-query meta/metadata-provider meta/saved-question)]
+ (let [query (lib.tu/query-with-stage-metadata-from-card meta/metadata-provider (:venues lib.tu/mock-cards))]
(is (=? {:columns [{:name "ID"}
{:name "NAME"}
{:name "CATEGORY_ID"}
@@ -27,9 +28,9 @@
(lib.metadata/stage query -1)))))
(deftest ^:parallel stage-column-metadata-test
- (let [query (lib/saved-question-query meta/metadata-provider meta/saved-question)]
+ (let [query (lib.tu/query-with-stage-metadata-from-card meta/metadata-provider (:venues lib.tu/mock-cards))]
(are [x] (=? {:lib/type :metadata/column
- :display-name "CATEGORY_ID"
+ :display-name "Category ID"
:name "CATEGORY_ID"
:base-type :type/Integer
:effective-type :type/Integer
diff --git a/test/metabase/lib/native_test.cljc b/test/metabase/lib/native_test.cljc
index 29384c8eff9..f25b785ce9e 100644
--- a/test/metabase/lib/native_test.cljc
+++ b/test/metabase/lib/native_test.cljc
@@ -164,16 +164,29 @@
(is (= clj-tags (-> clj-tags (#'lib.native/TemplateTags->) (#'lib.native/->TemplateTags))))
(is (test.js/= js-tags (-> js-tags (#'lib.native/->TemplateTags) (#'lib.native/TemplateTags->))))))))
+(def ^:private qp-results-metadata
+ "Capture of the `data.results_metadata` that would come back when running `SELECT * FROM VENUES;` with the Query
+ Processor.
+
+ IRL queries actually come back with both `data.cols` and `data.results_metadata.columns`, which are slightly
+ different from one another; the frontend merges these together into one unified metadata map. This is both icky and
+ silly. I'm hoping we can get away with just using one or the other in the future. So let's try to use just the stuff
+ here and see how far we get. If it turns out we need something in `data.cols` that's missing from here, let's just
+ add it to `data.results_metadata.columns` in QP results, and add it here as well, so we can start moving toward a
+ world where we don't have two versions of the metadata in query responses."
+ {:lib/type :metadata/results
+ :columns (get-in lib.tu/mock-cards [:venues :result-metadata])})
+
(deftest ^:parallel native-query-test
(is (=? {:lib/type :mbql/query
:database (meta/id)
:stages [{:lib/type :mbql.stage/native
:lib/options {:lib/uuid string?}
:native "SELECT * FROM VENUES;"}]}
- (lib/native-query meta/metadata-provider "SELECT * FROM VENUES;" meta/qp-results-metadata nil))))
+ (lib/native-query meta/metadata-provider "SELECT * FROM VENUES;" qp-results-metadata nil))))
(deftest ^:parallel native-query-suggested-name-test
- (let [query (lib/native-query meta/metadata-provider "SELECT * FROM VENUES;" meta/qp-results-metadata nil)]
+ (let [query (lib/native-query meta/metadata-provider "SELECT * FROM VENUES;" qp-results-metadata nil)]
(is (= "Native query"
(lib.metadata.calculation/describe-query query)))
(is (nil? (lib.metadata.calculation/suggested-name query)))))
diff --git a/test/metabase/lib/query_test.cljc b/test/metabase/lib/query_test.cljc
index 17a92756e23..7bda0551402 100644
--- a/test/metabase/lib/query_test.cljc
+++ b/test/metabase/lib/query_test.cljc
@@ -14,7 +14,8 @@
(deftest ^:parallel describe-query-test
(let [query (-> lib.tu/venues-query
(lib/aggregate (lib/sum (meta/field-metadata :venues :price))))
- ;; wrong arity: there's a bug in our Kondo config, see https://metaboat.slack.com/archives/C04DN5VRQM6/p1679022185079739?thread_ts=1679022025.317059&cid=C04DN5VRQM6
+ ;; wrong arity: there's a bug in our Kondo config, see
+ ;; https://metaboat.slack.com/archives/C04DN5VRQM6/p1679022185079739?thread_ts=1679022025.317059&cid=C04DN5VRQM6
query (-> #_{:clj-kondo/ignore [:invalid-arity]}
(lib/filter query (lib/= (meta/field-metadata :venues :name) "Toucannery"))
(lib/breakout (meta/field-metadata :venues :category-id))
@@ -30,17 +31,6 @@
(lib.metadata.calculation/describe-query query)
(lib.metadata.calculation/suggested-name query)))))
-(deftest ^:parallel card-source-query-test
- (is (=? {:lib/type :mbql/query
- :database (meta/id)
- :stages [{:lib/type :mbql.stage/native
- :native "SELECT * FROM VENUES;"}]}
- (lib/saved-question-query meta/metadata-provider
- {:dataset-query {:database (meta/id)
- :type :native
- :native {:query "SELECT * FROM VENUES;"}}
- :result-metadata meta/card-results-metadata}))))
-
(deftest ^:parallel notebook-query-test
(is (=? {:lib/type :mbql/query
:database (meta/id)
diff --git a/test/metabase/lib/test_metadata.cljc b/test/metabase/lib/test_metadata.cljc
index d6d7adbe957..90b21882d56 100644
--- a/test/metabase/lib/test_metadata.cljc
+++ b/test/metabase/lib/test_metadata.cljc
@@ -123,7 +123,7 @@
{:arglists '([table-name])}
keyword)
-(defmulti field-metadata-method
+(defmulti ^:private field-metadata-method
"Metadata for fields"
{:arglists '([table-name field-name])}
(fn [table-name field-name]
@@ -186,13 +186,12 @@
:display-name "Name"
:database-position 1
:database-required true
- :fingerprint
- {:global {:distinct-count 75, :nil% 0.0}
- :type {:type/Text {:percent-json 0.0
- :percent-url 0.0
- :percent-email 0.0
- :percent-state 0.0
- :average-length 8.333333333333334}}}
+ :fingerprint {:global {:distinct-count 75, :nil% 0.0}
+ :type {:type/Text {:percent-json 0.0
+ :percent-url 0.0
+ :percent-email 0.0
+ :percent-state 0.0
+ :average-length 8.333333333333334}}}
:base-type :type/Text
:points-of-interest nil
:lib/type :metadata/column})
@@ -2198,6 +2197,22 @@
"[[metabase.lib.metadata.protocols/MetadataProvider]] using the test [[metadata]]."
(meta.graph-provider/->SimpleGraphMetadataProvider metadata))
+(mu/defn tables :- [:set :keyword]
+ "Set of valid table names."
+ []
+ (set (keys (methods table-metadata-method))))
+
+(mu/defn fields :- [:set :keyword]
+ "Set of valid table names for a `:table-name`."
+ [table-name :- :keyword]
+ (assert ((tables) table-name)
+ (str "Invalid table: " table-name))
+ (into #{}
+ (keep (fn [[a-table-name a-field-name]]
+ (when (= a-table-name table-name)
+ a-field-name)))
+ (keys (methods field-metadata-method))))
+
(mu/defn table-metadata :- lib.metadata/TableMetadata
"Get Table metadata for a one of the `test-data` Tables in the test metadata, e.g. `:venues`. This is here so you can
test things that should consume Table metadata."
@@ -2210,113 +2225,3 @@
[table-name :- :keyword
field-name :- :keyword]
(field-metadata-method table-name field-name))
-
-(def card-results-metadata
- "Capture of the `result_metadata` saved with a Card with a `SELECT * FROM VENUES;` query. Actually this is a little
- different because this is pre-massaged into the MLv2 shape (it has `:lib/type` and uses `kebab-case` keys), to make
- it easier to use in tests. It should not make a difference because transformation to the MLv2 shape is idempotent...
- and at some point we'll probably update the backend to store stuff in this shape anyway."
- [{:lib/type :metadata/column
- :display-name "ID"
- :name "ID"
- :base-type :type/BigInteger
- :effective-type :type/BigInteger
- :semantic-type :type/PK
- :fingerprint nil}
- {:lib/type :metadata/column
- :display-name "NAME" ; TODO -- these display names are icky
- :name "NAME"
- :base-type :type/Text
- :effective-type :type/Text
- :semantic-type :type/Name
- :fingerprint {:global {:distinct-count 100, :nil% 0.0}
- :type {:type/Text {:percent-json 0.0
- :percent-url 0.0
- :percent-email 0.0
- :percent-state 0.0
- :average-length 15.63}}}}
- {:lib/type :metadata/column
- :display-name "CATEGORY_ID"
- :name "CATEGORY_ID"
- :base-type :type/Integer
- :effective-type :type/Integer
- :semantic-type :type/FK
- :fingerprint {:global {:distinct-count 28, :nil% 0.0}
- :type {:type/Number
- {:min 2.0
- :q1 6.89564392373896
- :q3 49.240253073352044
- :max 74.0
- :sd 23.058108414099443
- :avg 29.98}}}}
- {:lib/type :metadata/column
- :display-name "LATITUDE"
- :name "LATITUDE"
- :base-type :type/Float
- :effective-type :type/Float
- :semantic-type :type/Latitude
- :fingerprint
- {:global {:distinct-count 94, :nil% 0.0}
- :type {:type/Number {:min 10.0646
- :q1 34.06098873016278
- :q3 37.77185
- :max 40.7794
- :sd 3.4346725397190827
- :avg 35.505891999999996}}}}
- {:lib/type :metadata/column
- :display-name "LONGITUDE"
- :name "LONGITUDE"
- :base-type :type/Float
- :effective-type :type/Float
- :semantic-type :type/Longitude
- :fingerprint {:global {:distinct-count 84, :nil% 0.0}
- :type {:type/Number
- {:min -165.374
- :q1 -122.40857106781186
- :q3 -118.2635
- :max -73.9533
- :sd 14.162810671348238
- :avg -115.99848699999998}}}}
- {:lib/type :metadata/column
- :display-name "PRICE"
- :name "PRICE"
- :base-type :type/Integer
- :effective-type :type/Integer
- :semantic-type nil
- :fingerprint {:global {:distinct-count 4, :nil% 0.0}
- :type {:type/Number
- {:min 1.0
- :q1 1.4591129021415095
- :q3 2.493086095768049
- :max 4.0
- :sd 0.7713951678941896
- :avg 2.03}}}}])
-
-(def qp-results-metadata
- "Capture of the `data.results_metadata` that would come back when running `SELECT * FROM VENUES;` with the Query
- Processor.
-
- IRL queries actually come back with both `data.cols` and `data.results_metadata.columns`, which are slightly
- different from one another; the frontend merges these together into one unified metadata map. This is both icky and
- silly. I'm hoping we can get away with just using one or the other in the future. So let's try to use just the stuff
- here and see how far we get. If it turns out we need something in `data.cols` that's missing from here, let's just
- add it to `data.results_metadata.columns` in QP results, and add it here as well, so we can start moving toward a
- world where we don't have two versions of the metadata in query responses."
- {:lib/type :metadata/results
- :columns card-results-metadata})
-
-(def saved-question
- "An representative Saved Question, with [[results-metadata]], against `VENUES`. For testing queries that use a Saved
- Question as their source. See also [[saved-question-CardMetadata]] below."
- {:dataset-query {:database (id)
- :type :query
- :query {:source-table (id :venues)}}
- :result-metadata card-results-metadata})
-
-(def saved-question-CardMetadata
- "Mock [[metabase.lib.metadata/CardMetadata]] with a query against `VENUES`. See
- also [[metabase.lib.test-util/categories-mbql-card]] and [[metabase.lib.test-util/categories-native-card]]."
- (assoc saved-question
- :lib/type :metadata/card
- :id 1
- :name "Card 1"))
diff --git a/test/metabase/lib/test_util.cljc b/test/metabase/lib/test_util.cljc
index 4c56664e047..22c8c866784 100644
--- a/test/metabase/lib/test_util.cljc
+++ b/test/metabase/lib/test_util.cljc
@@ -2,16 +2,18 @@
"Misc test utils for Metabase lib."
(:require
[clojure.core.protocols]
- [clojure.test :refer [is]]
+ [clojure.test :refer [deftest is]]
[malli.core :as mc]
[medley.core :as m]
+ [metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
- [metabase.lib.metadata.composed-provider
- :as lib.metadata.composed-provider]
+ [metabase.lib.metadata.composed-provider :as lib.metadata.composed-provider]
[metabase.lib.metadata.protocols :as metadata.protocols]
+ [metabase.lib.query :as lib.query]
[metabase.lib.schema :as lib.schema]
[metabase.lib.test-metadata :as meta]
+ [metabase.lib.util :as lib.util]
[metabase.util.malli :as mu]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
@@ -97,8 +99,6 @@
(datafy [_this]
(list `mock-metadata-provider m))))
-
-
(def metadata-provider-with-card
"[[meta/metadata-provider]], but with a Card with ID 1."
(lib.metadata.composed-provider/composed-metadata-provider
@@ -226,69 +226,78 @@
:semantic-type :type/FK}]}
:native "SELECT whatever"}]})
-(def categories-mbql-card
- "Mock MBQL query Card against the `CATEGORIES` Table."
- {:lib/type :metadata/card
- :id 1
- :name "Tarot Card"
- :dataset-query {:database (meta/id)
- :type :query
- :query {:source-table (meta/id :categories)}}
- :result-metadata [(meta/field-metadata :categories :id)
- (meta/field-metadata :categories :name)]})
-
-(def metadata-provider-with-categories-mbql-card
- "A metadata provider with the [[categories-mbql-card]] as Card 1. Composed with the
- normal [[meta/metadata-provider]]."
- (lib.metadata.composed-provider/composed-metadata-provider
- meta/metadata-provider
- (mock-metadata-provider
- {:cards [categories-mbql-card]})))
-
-(def categories-native-card
- "Mock native query Card against the `CATEGORIES` Table."
- {:lib/type :metadata/card
- :id 1
- :name "Tarot Card"
- :dataset-query {:database (meta/id)
- :type :native
- :native {:query "SELECT * FROM CATEGORIES;"}}
- :result-metadata (mapv #(dissoc % :id :table-id)
- [(meta/field-metadata :categories :id)
- (meta/field-metadata :categories :name)])})
-
-(def metadata-provider-with-categories-native-card
- "A metadata provider with the [[categories-native-card]] as Card 1. Composed with the
- normal [[meta/metadata-provider]]."
- (lib.metadata.composed-provider/composed-metadata-provider
- meta/metadata-provider
- (mock-metadata-provider
- {:cards [categories-native-card]})))
-
(def mock-cards
- "Map of mock MBQL query Card against the test tables."
+ "Map of mock MBQL query Card against the test tables. There are three versions of the Card for each table:
+
+ * `:venues`, a Card WITH `:result-metadata`
+ * `:venues/no-metadata`, a Card WITHOUT `:result-metadata`
+ * `:venues/native`, a Card with `:result-metadata` and a NATIVE query."
(into {}
- (for [[idx table] (m/indexed [:categories
- :checkins
- :users
- :venues
- :products
- :orders
- :people
- :reviews])]
- [table {:lib/type :metadata/card
- :id (inc idx)
- :name (str "Mock " (name table) " card")
- :dataset-query {:database (meta/id)
- :type :query
- :query {:source-table (meta/id table)}}
- :result-metadata (for [[[meta-table meta-col] _] (methods meta/field-metadata-method)
- :when (= table meta-table)]
- (dissoc (meta/field-metadata table meta-col) :id :table-id))}])))
+ (comp (mapcat (fn [table]
+ [{:table table, :metadata? true, :native? false, :card-name table}
+ {:table table, :metadata? true, :native? true, :card-name (keyword (name table) "native")}
+ {:table table, :metadata? false, :native? false, :card-name (keyword (name table) "no-metadata")}]))
+ (map-indexed (fn [idx {:keys [table metadata? native? card-name]}]
+ [card-name
+ (merge
+ {:lib/type :metadata/card
+ :id (inc idx)
+ :name (str "Mock " (name table) " card")
+ :dataset-query (if native?
+ {:database (meta/id)
+ :type :native
+ :native {:query (str "SELECT * FROM " (name table))}}
+ {:database (meta/id)
+ :type :query
+ :query {:source-table (meta/id table)}})}
+ (when metadata?
+ {:result-metadata
+ (->> (meta/fields table)
+ (map (partial meta/field-metadata table))
+ (sort-by :id)
+ (mapv #(dissoc % :id :table-id)))}))])))
+ (meta/tables)))
(def metadata-provider-with-mock-cards
"A metadata provider with all of the [[mock-cards]]. Composed with the normal [[meta/metadata-provider]]."
(lib.metadata.composed-provider/composed-metadata-provider
meta/metadata-provider
(mock-metadata-provider
- {:cards (vals mock-cards)})))
+ {:cards (vals mock-cards)})))
+
+(mu/defn query-with-mock-card-as-source-card :- [:and
+ ::lib.schema/query
+ [:map
+ [:stages [:tuple
+ [:map
+ [:source-card integer?]]]]]]
+ "Create a query with one of the [[mock-cards]] as its `:source-card`."
+ [table-name :- (into [:enum] (sort (keys mock-cards)))]
+ (lib/query metadata-provider-with-mock-cards (mock-cards table-name)))
+
+(mu/defn query-with-stage-metadata-from-card :- ::lib.schema/query
+ "Convenience for creating a query that has `:lib/metadata` stage metadata attached to it from a Card. Note that this
+ does not create a query with a `:source-card`.
+
+ This is mostly around for historic reasons; consider using either [[metabase.lib.core/query]]
+ or [[query-with-mock-card-as-source-card]] instead, which are closer to real-life usage."
+ [metadata-providerable :- lib.metadata/MetadataProviderable
+ {mbql-query :dataset-query, metadata :result-metadata} :- [:map
+ [:dataset-query :map]
+ [:result-metadata [:sequential {:min 1} :map]]]]
+ (let [mbql-query (cond-> (assoc (lib.convert/->pMBQL mbql-query)
+ :lib/metadata (lib.metadata/->metadata-provider metadata-providerable))
+ metadata
+ (lib.util/update-query-stage -1 assoc :lib/stage-metadata (lib.util/->stage-metadata metadata)))]
+ (lib.query/query metadata-providerable mbql-query)))
+
+(deftest ^:parallel card-source-query-test
+ (is (=? {:lib/type :mbql/query
+ :database (meta/id)
+ :stages [{:lib/type :mbql.stage/native
+ :native "SELECT * FROM VENUES;"}]}
+ (query-with-stage-metadata-from-card meta/metadata-provider
+ {:dataset-query {:database (meta/id)
+ :type :native
+ :native {:query "SELECT * FROM VENUES;"}}
+ :result-metadata (get-in mock-cards [:venues :result-metadata])}))))
--
GitLab