diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index 12266d75f68c2ff5b5d3ea00229061c3faece16e..c7c11a7a08f3431ca2257227f58fd52029774029 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -243,7 +243,6 @@
metabase.api.preview-embed api.preview-embed
metabase.api.public api.public
metabase.api.pulse api.pulse
- metabase.api.query-description api.qd
metabase.api.revision api.revision
metabase.api.search api.search
metabase.api.segment api.segment
diff --git a/bin/kondo.sh b/bin/kondo.sh
index f2269f565f94f3a487495577606670500628461c..0fb1261ae8fe43b3aeb413ad80a11d94e998d227 100755
--- a/bin/kondo.sh
+++ b/bin/kondo.sh
@@ -8,6 +8,9 @@ set -euxo pipefail
clj-kondo --copy-configs --dependencies --lint "$(clojure -A:dev -Spath)" --skip-lint --parallel
+rm -rf .clj-kondo/metosin/malli-types-clj/
+rm -rf .clj-kondo/.cache
+
# Run Kondo against all of our Clojure files in the various directories they might live.
find modules/drivers shared enterprise/backend \
-maxdepth 2 \
diff --git a/frontend/src/metabase-lib/queries/utils/description.js b/frontend/src/metabase-lib/queries/utils/description.js
deleted file mode 100644
index 061f3ba8f8c45f89fc8684c94001f31373c02479..0000000000000000000000000000000000000000
--- a/frontend/src/metabase-lib/queries/utils/description.js
+++ /dev/null
@@ -1,156 +0,0 @@
-import React from "react";
-
-import { t } from "ttag";
-import _ from "underscore";
-import inflection from "inflection";
-
-import { FilterClause, MetricClause } from "./description.styled.tsx";
-
-// -----------------------------------------------------------------------------
-// These functions use the `query_description` field returned by the Segment and
-// Metric APIs. They're meant for cases where you do not have the full database
-// metadata available, and the server side will generate a data structure
-// containing all the applicable data for formatting a user-friendly description
-// of a query.
-//
-// TODO: This is almost 100% duplicated with code in Question.ts, find a way to
-// consolidate them
-// -----------------------------------------------------------------------------
-
-function formatTableDescription({ table }, options = {}) {
- return [inflection.pluralize(table)];
-}
-
-function formatAggregationDescription({ aggregation }, options = {}) {
- if (!aggregation || !aggregation.length) {
- return [];
- }
-
- return conjunctList(
- aggregation.map(agg => {
- switch (agg["type"]) {
- case "aggregation":
- return [agg["arg"]];
- case "metric":
- return [
- options.jsx ? (
- <MetricClause>{agg["arg"]}</MetricClause>
- ) : (
- agg["arg"]
- ),
- ];
- case "rows":
- return [t`Raw data`];
- case "count":
- return [t`Count`];
- case "cum-count":
- return [t`Cumulative count`];
- case "avg":
- return [t`Average of `, agg["arg"]];
- case "median":
- return [t`Median of `, agg["arg"]];
- case "distinct":
- return [t`Distinct values of `, agg["arg"]];
- case "stddev":
- return [t`Standard deviation of `, agg["arg"]];
- case "sum":
- return [t`Sum of `, agg["arg"]];
- case "cum-sum":
- return [t`Cumulative sum of `, agg["arg"]];
- case "max":
- return [t`Maximum of `, agg["arg"]];
- case "min":
- return [t`Minimum of `, agg["arg"]];
- default: {
- console.warn(
- "Unexpected aggregation type in formatAggregationDescription: ",
- agg["type"],
- );
- return null;
- }
- }
- }),
- );
-}
-
-function formatFilterDescription({ filter }, options = {}) {
- if (!filter || !filter.length) {
- return [];
- }
-
- return [
- t`Filtered by `,
- joinList(
- filter.map(f => {
- if (f["segment"] != null) {
- return options.jsx ? (
- <FilterClause>{f["segment"]}</FilterClause>
- ) : (
- f["segment"]
- );
- } else if (f["field"] != null) {
- return f["field"];
- }
- }),
- ", ",
- ),
- ];
-}
-
-export function formatQueryDescription(parts, options = {}) {
- if (!parts) {
- return "";
- }
-
- options = {
- jsx: false,
- sections: ["table", "aggregation", "filter"],
- ...options,
- };
-
- const sectionFns = {
- table: formatTableDescription,
- aggregation: formatAggregationDescription,
- filter: formatFilterDescription,
- };
-
- // these array gymnastics are needed to support JSX formatting
- const sections = (options.sections || [])
- .map(section =>
- _.flatten(sectionFns[section](parts, options)).filter(s => !!s),
- )
- .filter(s => s && s.length > 0);
-
- const description = _.flatten(joinList(sections, ", "));
- if (options.jsx) {
- return <span>{description}</span>;
- } else {
- return description.join("");
- }
-}
-
-function joinList(list, joiner) {
- return _.flatten(
- list.map((l, i) => (i === list.length - 1 ? [l] : [l, joiner])),
- true,
- );
-}
-
-function conjunctList(list, conjunction) {
- switch (list.length) {
- case 0:
- return null;
- case 1:
- return list[0];
- case 2:
- return [list[0], " ", conjunction, " ", list[1]];
- default:
- return [
- list.slice(0, -1).join(", "),
- ", ",
- conjunction,
- " ",
- list[list.length - 1],
- ];
- }
-}
diff --git a/frontend/src/metabase-lib/queries/utils/description.styled.tsx b/frontend/src/metabase-lib/queries/utils/description.styled.tsx
deleted file mode 100644
index e187ba0f0839bc5deb125f9cd48c886138e2b46b..0000000000000000000000000000000000000000
--- a/frontend/src/metabase-lib/queries/utils/description.styled.tsx
+++ /dev/null
@@ -1,12 +0,0 @@
-import styled from "@emotion/styled";
-import { color } from "metabase/lib/colors";
-
-export const MetricClause = styled.span`
- color: ${color("summarize")};
- font-weight: 700;
-`;
-
-export const FilterClause = styled.span`
- color: ${color("filter")};
- font-weight: 700;
-`;
diff --git a/frontend/src/metabase/admin/datamodel/components/MetricItem.jsx b/frontend/src/metabase/admin/datamodel/components/MetricItem.jsx
index 0f420eac79ce0e0d54bafc1a2846908144707ea0..c228f1300af3e5341236c6e7862847deae46d880 100644
--- a/frontend/src/metabase/admin/datamodel/components/MetricItem.jsx
+++ b/frontend/src/metabase/admin/datamodel/components/MetricItem.jsx
@@ -2,7 +2,6 @@ import React, { Component } from "react";
import PropTypes from "prop-types";
import Icon from "metabase/components/Icon";
-import { formatQueryDescription } from "metabase-lib/queries/utils/description";
import ObjectActionSelect from "./ObjectActionSelect";
export default class MetricItem extends Component {
@@ -14,11 +13,6 @@ export default class MetricItem extends Component {
render() {
const { metric, onRetire } = this.props;
- const description = formatQueryDescription(metric.query_description, {
- sections: ["table", "aggregation", "filter"],
- jsx: true,
- });
-
return (
<tr>
<td className="px1 py1 text-wrap">
@@ -27,7 +21,7 @@ export default class MetricItem extends Component {
<span className="text-dark text-bold">{metric.name}</span>
</span>
</td>
- <td className="px1 py1 text-wrap">{description}</td>
+ <td className="px1 py1 text-wrap">{metric.definition_description}</td>
<td className="px1 py1 text-centered">
<ObjectActionSelect
object={metric}
diff --git a/frontend/src/metabase/admin/datamodel/components/SegmentItem.jsx b/frontend/src/metabase/admin/datamodel/components/SegmentItem.jsx
index 7252c06a0c6b3e998dc018b063c3a08473cbfa16..a92ddb35d18709467d304d561cac01953fd0907c 100644
--- a/frontend/src/metabase/admin/datamodel/components/SegmentItem.jsx
+++ b/frontend/src/metabase/admin/datamodel/components/SegmentItem.jsx
@@ -2,7 +2,6 @@ import React, { Component } from "react";
import PropTypes from "prop-types";
import Icon from "metabase/components/Icon";
-import { formatQueryDescription } from "metabase-lib/queries/utils/description";
import ObjectActionSelect from "./ObjectActionSelect";
export default class SegmentItem extends Component {
@@ -14,11 +13,6 @@ export default class SegmentItem extends Component {
render() {
const { segment, onRetire } = this.props;
- const description = formatQueryDescription(segment.query_description, {
- sections: ["filter"],
- jsx: true,
- });
-
return (
<tr className="mt1 mb3">
<td className="px1 py1 text-wrap">
@@ -31,7 +25,7 @@ export default class SegmentItem extends Component {
<span className="text-dark text-bold">{segment.name}</span>
</span>
</td>
- <td className="px1 py1 text-wrap">{description}</td>
+ <td className="px1 py1 text-wrap">{segment.definition_description}</td>
<td className="px1 py1 text-centered">
<ObjectActionSelect
object={segment}
diff --git a/shared/src/metabase/mbql/util.cljc b/shared/src/metabase/mbql/util.cljc
index 329428867c15d1651610e278832f5221cd8fe641..9d6c58c5c50bb4fe5b245fcb8a1c76810bbcc1ef 100644
--- a/shared/src/metabase/mbql/util.cljc
+++ b/shared/src/metabase/mbql/util.cljc
@@ -733,6 +733,18 @@
(when (keyword? k)
(namespace k)))))))
+(defn referenced-field-ids
+ "Find all the `:field` references with integer IDs in `coll`, which can be a full MBQL query, a snippet of MBQL, or a
+ sequence of those things; return a set of Field IDs. Includes Fields referenced indirectly via `:source-field`.
+ Returns `nil` if no IDs are found."
+ [coll]
+ (not-empty
+ (into #{}
+ (comp cat (filter some?))
+ (mbql.match/match coll
+ [:field (id :guard integer?) opts]
+ [id (:source-field opts)]))))
+
#?(:clj
(p/import-vars
[mbql.match
diff --git a/src/metabase/api/metric.clj b/src/metabase/api/metric.clj
index 5d91c15a1880f7d8c63b175671fd1475f476429d..25ac7d25fd26a134a4fb7672ef0443cf986a0633 100644
--- a/src/metabase/api/metric.clj
+++ b/src/metabase/api/metric.clj
@@ -4,7 +4,6 @@
[clojure.data :as data]
[compojure.core :refer [DELETE GET POST PUT]]
[metabase.api.common :as api]
- [metabase.api.query-description :as api.qd]
[metabase.events :as events]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.models :refer [Metric MetricImportantField Table]]
@@ -14,6 +13,7 @@
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log]
+ [metabase.util.malli.schema :as ms]
[metabase.util.schema :as su]
[schema.core :as s]
[toucan.db :as db]
@@ -44,20 +44,11 @@
(-> (api/read-check (t2/select-one Metric :id id))
(hydrate :creator)))
-(defn- add-query-descriptions
- [metrics] {:pre [(coll? metrics)]}
- (when (some? metrics)
- (for [metric metrics]
- (let [table (t2/select-one Table :id (:table_id metric))]
- (assoc metric
- :query_description
- (api.qd/generate-query-description table (:definition metric)))))))
-
-#_{:clj-kondo/ignore [:deprecated-var]}
-(api/defendpoint-schema GET "/:id"
+(api/defendpoint GET "/:id"
"Fetch `Metric` with ID."
[id]
- (first (add-query-descriptions [(hydrated-metric id)])))
+ {id ms/PositiveInt}
+ (hydrated-metric id))
(defn- add-db-ids
"Add `:database_id` fields to `metrics` by looking them up from their `:table_id`."
@@ -67,15 +58,14 @@
(for [metric metrics]
(assoc metric :database_id (table-id->db-id (:table_id metric)))))))
-#_{:clj-kondo/ignore [:deprecated-var]}
-(api/defendpoint-schema GET "/"
+(api/defendpoint GET "/"
"Fetch *all* `Metrics`."
[]
(as-> (t2/select Metric, :archived false, {:order-by [:%lower.name]}) metrics
- (hydrate metrics :creator)
+ (hydrate metrics :creator :definition_description)
(add-db-ids metrics)
(filter mi/can-read? metrics)
- (add-query-descriptions metrics)))
+ metrics))
(defn- write-check-and-update-metric!
"Check whether current user has write permissions, then update Metric with values in `body`. Publishes appropriate
diff --git a/src/metabase/api/query_description.clj b/src/metabase/api/query_description.clj
deleted file mode 100644
index 42c399c8bddb8cf6cf02de7f292f2f460ce5b687..0000000000000000000000000000000000000000
--- a/src/metabase/api/query_description.clj
+++ /dev/null
@@ -1,128 +0,0 @@
-(ns metabase.api.query-description
- "Functions for generating human friendly query descriptions"
- (:require
- [clojure.string :as str]
- [metabase.mbql.predicates :as mbql.preds]
- [metabase.mbql.util :as mbql.u]
- [metabase.models.field :refer [Field]]
- [metabase.models.metric :refer [Metric]]
- [metabase.models.segment :refer [Segment]]
- [metabase.util.i18n :refer [deferred-tru]]
- [metabase.util.log :as log]
- [toucan2.core :as t2]))
-
-(defn- get-table-description
- [metadata _query]
- {:table (:display_name metadata)})
-
-(defn- field-clause->display-name [clause]
- (mbql.u/match-one clause
- [:field (id :guard integer?) _]
- (t2/select-one-fn :display_name Field :id id)
-
- [:field (field-name :guard string?) _]
- field-name))
-
-(defn- get-aggregation-details
- [metadata query]
- (letfn [(field-name [match] (or (when (mbql.preds/Field? match)
- (field-clause->display-name match))
- (flatten (get-aggregation-details metadata match))))]
- (when-let [agg-matches (mbql.u/match query
- [:aggregation-options _ (options :guard :display-name)]
- {:type :aggregation :arg (:display-name options)}
-
- [:aggregation-options ag _]
- #_:clj-kondo/ignore
- (recur ag)
-
- [(operator :guard #{:+ :- :/ :*}) & args]
- (interpose (name operator) (map field-name args))
-
- [:metric (arg :guard integer?)]
- {:type :metric
- :arg (let [metric-name (t2/select-one-fn :name Metric :id arg)]
- (if-not (str/blank? metric-name)
- metric-name
- (deferred-tru "[Unknown Metric]")))}
-
- [:rows] {:type :rows}
- [:count] {:type :count}
- [:cum-count] {:type :cum-count}
- [:avg arg] {:type :avg :arg (field-name arg)}
- [:distinct arg] {:type :distinct :arg (field-name arg)}
- [:stddev arg] {:type :stddev :arg (field-name arg)}
- [:sum arg] {:type :sum :arg (field-name arg)}
- [:cum-sum arg] {:type :cum-sum :arg (field-name arg)}
- [:max arg] {:type :max :arg (field-name arg)}
- [:min arg] {:type :min :arg (field-name arg)})]
- agg-matches)))
-
-(defn- get-aggregation-description
- [metadata query]
- (when-let [details (get-aggregation-details metadata query)]
- {:aggregation details}))
-
-(defn- get-breakout-description
- [_metadata query]
- (when-let [breakouts (seq (:breakout query))]
- {:breakout (map #(t2/select-one-fn :display_name Field :id %) breakouts)}))
-
-(defn- get-filter-clause-description
- [_metadata filt]
- (let [typ (first filt)]
- (condp = typ
- :field {:field (field-clause->display-name filt)}
- :segment {:segment (let [segment (t2/select-one Segment :id (second filt))]
- (if segment
- (:name segment)
- (deferred-tru "[Unknown Segment]")))}
- nil)))
-
-(defn- get-filter-description
- [metadata query]
- (when-let [filters (:filter query)]
- {:filter (map #(get-filter-clause-description metadata %)
- (mbql.u/match filters #{:field :segment} &match))}))
-
-(defn- get-order-by-description
- [_metadata query]
- (when (:order-by query)
- {:order-by (map (fn [[direction field]]
- {:field (field-clause->display-name field)
- :direction direction})
- (mbql.u/match query #{:asc :desc} &match))}))
-
-(defn- get-limit-description
- [_metadata query]
- (when-let [limit (:limit query)]
- {:limit limit}))
-
-(def ^:private query-descriptor-functions
- [get-table-description
- get-aggregation-description
- get-breakout-description
- get-filter-description
- get-order-by-description
- get-limit-description])
-
-(defn generate-query-description
- "Analyze a query and return a data structure with the parts broken down for display
- in the UI.
-
- Ex:
- {
- :table \"Orders\"
- :filters [\"Created At\", \"Product ID\"]
- :order-by [{\"Created At\" :asc}]
- }
-
- This data structure allows the UI to format the strings appropriately (including JSX)"
- [metadata query]
- (try
- (apply merge
- (map (fn [f] (f metadata query))
- query-descriptor-functions))
- (catch Exception e
- (log/warn e "Error generating query description")
- {})))
diff --git a/src/metabase/api/segment.clj b/src/metabase/api/segment.clj
index c5dd3e419210f8752a6bc3cc1d5b32386a3c9666..3cb620928973681694a49d00d0c9e97450d9f27b 100644
--- a/src/metabase/api/segment.clj
+++ b/src/metabase/api/segment.clj
@@ -3,17 +3,16 @@
(:require
[compojure.core :refer [DELETE GET POST PUT]]
[metabase.api.common :as api]
- [metabase.api.query-description :as api.qd]
[metabase.events :as events]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.models.interface :as mi]
[metabase.models.revision :as revision]
[metabase.models.segment :as segment :refer [Segment]]
- [metabase.models.table :as table :refer [Table]]
[metabase.related :as related]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log]
+ [metabase.util.malli.schema :as ms]
[metabase.util.schema :as su]
[schema.core :as s]
[toucan.hydrate :refer [hydrate]]
@@ -43,29 +42,18 @@
(-> (api/read-check (t2/select-one Segment :id id))
(hydrate :creator)))
-(defn- add-query-descriptions
- [segments] {:pre [(coll? segments)]}
- (when (some? segments)
- (for [segment segments]
- (let [table (t2/select-one Table :id (:table_id segment))]
- (assoc segment
- :query_description
- (api.qd/generate-query-description table (:definition segment)))))))
-
-#_{:clj-kondo/ignore [:deprecated-var]}
-(api/defendpoint-schema GET "/:id"
+(api/defendpoint GET "/:id"
"Fetch `Segment` with ID."
[id]
- (first (add-query-descriptions [(hydrated-segment id)])))
+ {id ms/PositiveInt}
+ (hydrated-segment id))
-#_{:clj-kondo/ignore [:deprecated-var]}
-(api/defendpoint-schema GET "/"
+(api/defendpoint GET "/"
"Fetch *all* `Segments`."
[]
(as-> (t2/select Segment, :archived false, {:order-by [[:%lower.name :asc]]}) segments
(filter mi/can-read? segments)
- (hydrate segments :creator)
- (add-query-descriptions segments)))
+ (hydrate segments :creator :definition_description)))
(defn- write-check-and-update-segment!
"Check whether current user has write permissions, then update Segment with values in `body`. Publishes appropriate
@@ -114,7 +102,6 @@
(write-check-and-update-segment! id {:archived true, :revision_message revision_message})
api/generic-204-no-content)
-
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/:id/revisions"
"Fetch `Revisions` for `Segment` with ID."
@@ -122,7 +109,6 @@
(api/read-check Segment id)
(revision/revisions+details Segment id))
-
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema POST "/:id/revert"
"Revert a `Segement` to a prior `Revision`."
diff --git a/src/metabase/lib/aggregation.cljc b/src/metabase/lib/aggregation.cljc
index 3aa20c3eeadbd484ec2d3f328d9766a400117741..aa1426984c2705a771e66dd8d30a399bc3264b20 100644
--- a/src/metabase/lib/aggregation.cljc
+++ b/src/metabase/lib/aggregation.cljc
@@ -24,7 +24,7 @@
:stage-number stage-number})))
(nth aggregations index)))
-(defmethod lib.metadata.calculation/describe-top-level-key :aggregation
+(defmethod lib.metadata.calculation/describe-top-level-key-method :aggregation
[query stage-number _k]
(when-let [aggregations (not-empty (:aggregation (lib.util/query-stage query stage-number)))]
(lib.util/join-strings-with-conjunction
diff --git a/src/metabase/lib/breakout.cljc b/src/metabase/lib/breakout.cljc
index edcdc2801542fecbf62a4adf486846217a7ba10e..2c1ac99b4ef09669b707e1e3d86389330dfc1075 100644
--- a/src/metabase/lib/breakout.cljc
+++ b/src/metabase/lib/breakout.cljc
@@ -9,7 +9,7 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
-(defmethod lib.metadata.calculation/describe-top-level-key :breakout
+(defmethod lib.metadata.calculation/describe-top-level-key-method :breakout
[query stage-number _k]
(when-let [breakouts (not-empty (:breakout (lib.util/query-stage query stage-number)))]
(i18n/tru "Grouped by {0}"
diff --git a/src/metabase/lib/convert.cljc b/src/metabase/lib/convert.cljc
index f77735e4822d352517551e9cfe6eaddcca5f9445..38c83573fb90016cb9ebd2de701831191959e836 100644
--- a/src/metabase/lib/convert.cljc
+++ b/src/metabase/lib/convert.cljc
@@ -68,6 +68,15 @@
(let [[tag opts & args] (->pMBQL aggregation)]
(into [tag (merge opts options)] args)))
+(defn legacy-query-from-inner-query
+ "Convert a legacy 'inner query' to a full legacy 'outer query' so you can pass it to stuff
+ like [[metabase.mbql.normalize/normalize]], and then probably to [[->pMBQL]]."
+ [database-id inner-query]
+ (merge {:database database-id, :type :query}
+ (if (:native inner-query)
+ {:native (set/rename-keys inner-query {:native :query})}
+ {:query inner-query})))
+
(defmulti ->legacy-MBQL
"Coerce something to legacy MBQL (the version of MBQL understood by the query processor and Metabase Lib v1) if it's
not already legacy MBQL."
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index a148d11410eddd4faa69531c8466ba7d86abf778..849e36c9a5058feef115feb8af2e803f5b96e791 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -16,6 +16,7 @@
[metabase.lib.metric :as lib.metric]
[metabase.lib.order-by :as lib.order-by]
[metabase.lib.query :as lib.query]
+ [metabase.lib.segment :as lib.segment]
[metabase.lib.stage :as lib.stage]
[metabase.lib.table :as lib.table]
[metabase.lib.temporal-bucket :as lib.temporal-bucket]
@@ -33,6 +34,7 @@
lib.metric/keep-me
lib.order-by/keep-me
lib.query/keep-me
+ lib.segment/keep-me
lib.stage/keep-me
lib.table/keep-me
lib.temporal-bucket/keep-me)
@@ -131,6 +133,7 @@
[lib.metadata.calculation
column-name
describe-query
+ describe-top-level-key
display-name
suggested-name]
[lib.order-by
diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc
index 24e3c91ef2fd72ca4d2ad5ec00debad7e4a432db..f7849ce55545da2c5b3256984a315d248027e4f3 100644
--- a/src/metabase/lib/field.cljc
+++ b/src/metabase/lib/field.cljc
@@ -105,10 +105,12 @@
(defmethod lib.metadata.calculation/display-name-method :field
[query stage-number [_field {:keys [join-alias temporal-unit], :as _opts} _id-or-name, :as field-clause]]
- (let [field-metadata (cond-> (resolve-field-metadata query stage-number field-clause)
- join-alias (assoc :source_alias join-alias)
- temporal-unit (assoc :unit temporal-unit))]
- (lib.metadata.calculation/display-name query stage-number field-metadata)))
+ (if-let [field-metadata (cond-> (resolve-field-metadata query stage-number field-clause)
+ join-alias (assoc :source_alias join-alias)
+ temporal-unit (assoc :unit temporal-unit))]
+ (lib.metadata.calculation/display-name query stage-number field-metadata)
+ ;; mostly for the benefit of JS, which does not enforce the Malli schemas.
+ (i18n/tru "[Unknown Field]")))
(defmulti ^:private ->field
{:arglists '([query stage-number field])}
diff --git a/src/metabase/lib/filter.cljc b/src/metabase/lib/filter.cljc
index b2ea06bdc3ae8870f1ed326cb3704d0623c91931..430b80b1c46f03785226d4920f11e2d6b130a9ea 100644
--- a/src/metabase/lib/filter.cljc
+++ b/src/metabase/lib/filter.cljc
@@ -16,7 +16,7 @@
(comment metabase.lib.schema/keep-me)
-(defmethod lib.metadata.calculation/describe-top-level-key :filter
+(defmethod lib.metadata.calculation/describe-top-level-key-method :filter
[query stage-number _key]
(when-let [filter-clause (:filter (lib.util/query-stage query stage-number))]
(i18n/tru "Filtered by {0}" (lib.metadata.calculation/display-name query stage-number filter-clause))))
diff --git a/src/metabase/lib/js/metadata.cljs b/src/metabase/lib/js/metadata.cljs
index 734b0c0a309879934e93dd977072ab5968eb7eae..3c9fcad9814cd77b5ab91aadd7fc4c8e7c5de175 100644
--- a/src/metabase/lib/js/metadata.cljs
+++ b/src/metabase/lib/js/metadata.cljs
@@ -87,7 +87,7 @@
(fn [object]
(try
(let [parsed (assoc (obj->clj xform object) :lib/type lib-type-name)]
- (log/infof "Parsed metadata %s %s\n%s" object-type (:id parsed) (u/pprint-to-str parsed))
+ (log/debugf "Parsed metadata %s %s\n%s" object-type (:id parsed) (u/pprint-to-str parsed))
parsed)
(catch js/Error e
(log/errorf e "Error parsing %s %s: %s" object-type (pr-str object) (ex-message e))
@@ -304,7 +304,7 @@
"Use a `metabase-lib/metadata/Metadata` as a [[metabase.lib.metadata.protocols/MetadataProvider]]."
[database-id metadata]
(let [metadata (parse-metadata metadata)]
- (log/info "Created metadata provider for metadata")
+ (log/debug "Created metadata provider for metadata")
(reify lib.metadata.protocols/MetadataProvider
(database [_this] (database metadata database-id))
(table [_this table-id] (table metadata table-id))
diff --git a/src/metabase/lib/limit.cljc b/src/metabase/lib/limit.cljc
index dd6780fa3cd0c4a11dde3119c77cbec12b950e94..29851c337285f73233fdc741479912275fd104be 100644
--- a/src/metabase/lib/limit.cljc
+++ b/src/metabase/lib/limit.cljc
@@ -7,7 +7,7 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
-(defmethod lib.metadata.calculation/describe-top-level-key :limit
+(defmethod lib.metadata.calculation/describe-top-level-key-method :limit
[query stage-number _k]
(when-let [limit (:limit (lib.util/query-stage query stage-number))]
(str limit \space (i18n/trun "row" "rows" limit))))
diff --git a/src/metabase/lib/metadata.cljc b/src/metabase/lib/metadata.cljc
index 958689e89630df5b4b9956623c5b468159f80554..72c3429ef1ec0e34f83bbed6cd5d9c7c56b6c229 100644
--- a/src/metabase/lib/metadata.cljc
+++ b/src/metabase/lib/metadata.cljc
@@ -44,7 +44,8 @@
[:map
[:lib/type [:= :metadata/field]] ; TODO -- should this be changed to `:metadata/column`?
[:id {:optional true} ::lib.schema.id/field]
- [:name ::lib.schema.common/non-blank-string]])
+ [:name ::lib.schema.common/non-blank-string]
+ [:display_name {:optional true} [:maybe ::lib.schema.common/non-blank-string]]])
(def ^:private CardMetadata
[:map
@@ -69,7 +70,8 @@
[:lib/type [:= :metadata/table]]
[:id ::lib.schema.id/table]
[:name ::lib.schema.common/non-blank-string]
- [:schema {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
+ [:display_name {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
+ [:schema {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
;; This is now optional! If the [[MetadataProvider]] provides it, great, but if not we can always make the
;; subsequent request to fetch fields separately.
[:fields {:optional true} [:maybe [:sequential ColumnMetadata]]]
diff --git a/src/metabase/lib/metadata/cached_provider.cljc b/src/metabase/lib/metadata/cached_provider.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..2995280baadc2c8f71dd240589b4aa7f195dece9
--- /dev/null
+++ b/src/metabase/lib/metadata/cached_provider.cljc
@@ -0,0 +1,71 @@
+(ns metabase.lib.metadata.cached-provider
+ (:require
+ [clojure.set :as set]
+ [metabase.lib.metadata.protocols :as lib.metadata.protocols]
+ [metabase.util.log :as log]
+ #?@(:clj ([pretty.core :as pretty]))))
+
+(defn- get-in-cache [cache ks]
+ (when-some [cached-value (get-in @cache ks)]
+ (when-not (= cached-value ::nil)
+ cached-value)))
+
+(defn- store-in-cache! [cache ks value]
+ (let [value (if (some? value) value ::nil)]
+ (swap! cache assoc-in ks value)
+ value))
+
+(defn- get-in-cache-or-fetch [cache ks fetch-thunk]
+ (if-some [cached-value (get-in @cache ks)]
+ (when-not (= cached-value ::nil)
+ cached-value)
+ (store-in-cache! cache ks (fetch-thunk))))
+
+(defn- bulk-metadata [cache uncached-provider metadata-type ids]
+ (when (seq ids)
+ (log/debugf "Getting %s metadata with IDs %s" metadata-type (pr-str (sort ids)))
+ (let [existing-ids (set (keys (get @cache metadata-type)))
+ missing-ids (set/difference (set ids) existing-ids)]
+ (log/debugf "Already fetched %s: %s" metadata-type (pr-str (sort (set/intersection (set ids) existing-ids))))
+ (when (seq missing-ids)
+ (log/debugf "Need to fetch %s: %s" metadata-type (pr-str (sort missing-ids)))
+ ;; TODO -- we should probably store `::nil` markers for things we tried to fetch that didn't exist
+ (doseq [instance (lib.metadata.protocols/bulk-metadata uncached-provider metadata-type missing-ids)]
+ (store-in-cache! cache [metadata-type (:id instance)] instance))))
+ (for [id ids]
+ (get-in-cache cache [metadata-type id]))))
+
+(deftype CachedProxyMetadataProvider [cache metadata-provider]
+ lib.metadata.protocols/MetadataProvider
+ (database [_this] (get-in-cache-or-fetch cache [:metadata/database] #(lib.metadata.protocols/database metadata-provider)))
+ (table [_this table-id] (get-in-cache-or-fetch cache [:metadata/table table-id] #(lib.metadata.protocols/table metadata-provider table-id)))
+ (field [_this field-id] (get-in-cache-or-fetch cache [:metadata/field field-id] #(lib.metadata.protocols/field metadata-provider field-id)))
+ (card [_this card-id] (get-in-cache-or-fetch cache [:metadata/card card-id] #(lib.metadata.protocols/card metadata-provider card-id)))
+ (metric [_this metric-id] (get-in-cache-or-fetch cache [:metadata/metric metric-id] #(lib.metadata.protocols/metric metadata-provider metric-id)))
+ (segment [_this segment-id] (get-in-cache-or-fetch cache [:metadata/segment segment-id] #(lib.metadata.protocols/segment metadata-provider segment-id)))
+ (tables [_this] (get-in-cache-or-fetch cache [::database-tables] #(lib.metadata.protocols/tables metadata-provider)))
+ (fields [_this table-id] (get-in-cache-or-fetch cache [::table-fields table-id] #(lib.metadata.protocols/fields metadata-provider table-id)))
+
+ lib.metadata.protocols/CachedMetadataProvider
+ (cached-database [_this] (get-in-cache cache [:metadata/database]))
+ (cached-metadata [_this metadata-type id] (get-in-cache cache [metadata-type id]))
+ (store-database! [_this database-metadata] (store-in-cache! cache [:metadata/database] (assoc database-metadata :lib/type :metadata/database)))
+ (store-metadata! [_this metadata-type id metadata] (store-in-cache! cache [metadata-type id] (assoc metadata :lib/type metadata-type)))
+
+ ;; these only work if the underlying metadata provider is also a [[BulkMetadataProvider]].
+ lib.metadata.protocols/BulkMetadataProvider
+ (bulk-metadata [_this metadata-type ids]
+ (bulk-metadata cache metadata-provider metadata-type ids))
+
+ #?@(:clj
+ [pretty.core/PrettyPrintable
+ (pretty [_this]
+ (list `->CachedProxyMetadataProvider cache metadata-provider))]))
+
+(defn cached-metadata-provider
+ "Wrap `metadata-provider` with an implementation that automatically caches results.
+
+ If the metadata provider implements [[lib.metadata.protocols/BulkMetadataProvider]],
+ then [[lib.metadata.protocols/bulk-metadata]] will work as expected; it can be done for side-effects as well."
+ ^CachedProxyMetadataProvider [metadata-provider]
+ (->CachedProxyMetadataProvider (atom {}) metadata-provider))
diff --git a/src/metabase/lib/metadata/calculation.cljc b/src/metabase/lib/metadata/calculation.cljc
index fdc6f8205e8c3557696a0bb95b6952e73f3c67ec..70e1d8daaf97022000ae658859c85a1ea5cca007 100644
--- a/src/metabase/lib/metadata/calculation.cljc
+++ b/src/metabase/lib/metadata/calculation.cljc
@@ -24,22 +24,25 @@
(fn [_query _stage-number x]
(lib.dispatch/dispatch-value x)))
-(mu/defn display-name :- ::lib.schema.common/non-blank-string
+(mu/defn ^:export display-name :- ::lib.schema.common/non-blank-string
"Calculate a nice human-friendly display name for something."
- [query :- ::lib.schema/query
- stage-number :- :int
- x]
- (or
- ;; if this is an MBQL clause with `:display-name` in the options map, then use that rather than calculating a name.
- (:display-name (lib.options/options x))
- (try
- (display-name-method query stage-number x)
- ;; if this errors, just catch the error and return something like `Unknown :field`. We shouldn't blow up the
- ;; whole Query Builder if there's a bug
- (catch #?(:clj Throwable :cljs js/Error) e
- (throw (ex-info (i18n/tru "Error calculating display name for {0}: {1}" (pr-str x) (ex-message e))
- {:query query, :x x}
- e))))))
+ ([query x]
+ (display-name query -1 x))
+
+ ([query :- ::lib.schema/query
+ stage-number :- :int
+ x]
+ (or
+ ;; if this is an MBQL clause with `:display-name` in the options map, then use that rather than calculating a name.
+ (:display-name (lib.options/options x))
+ (try
+ (display-name-method query stage-number x)
+ ;; if this errors, just catch the error and return something like `Unknown :field`. We shouldn't blow up the
+ ;; whole Query Builder if there's a bug
+ (catch #?(:clj Throwable :cljs js/Error) e
+ (throw (ex-info (i18n/tru "Error calculating display name for {0}: {1}" (pr-str x) (ex-message e))
+ {:query query, :x x}
+ e)))))))
(mu/defn column-name :- ::lib.schema.common/non-blank-string
"Calculate a database-friendly name to use for an expression."
@@ -63,7 +66,7 @@
;; hopefully this is dev-facing only, so not i18n'ed.
(log/warnf "Don't know how to calculate display name for %s. Add an impl for %s for %s"
(pr-str x)
- `display-name*
+ `display-name-method
(lib.dispatch/dispatch-value x))
(if (and (vector? x)
(keyword? (first x)))
@@ -84,13 +87,27 @@
[query stage-number x]
(slugify (display-name query stage-number x)))
-(defmulti describe-top-level-key
- "'top-level' here means the top level of an individual stage. Describe part of a stage of a query, e.g. the `:filter`
- part or the `:aggregation` part."
+(defmulti describe-top-level-key-method
+ "Implementation for [[describe-top-level-key]]. Describe part of a stage of a query, e.g. the `:filter` part or the
+ `:aggregation` part. Return `nil` if there is nothing to describe."
{:arglists '([query stage-number top-level-key])}
(fn [_query _stage-number top-level-key]
top-level-key))
+(def ^:private TopLevelKey
+ "In the interest of making this easy to use in JS-land we'll accept either strings or keywords."
+ [:enum :aggregation :breakout :filter :limit :order-by :source-table])
+
+(mu/defn describe-top-level-key :- [:maybe ::lib.schema.common/non-blank-string]
+ "'top-level' here means the top level of an individual stage. Generate a human-friendly string describing a specific
+ part of an MBQL stage, or `nil` if that part doesn't exist."
+ ([query top-level-key]
+ (describe-top-level-key query -1 top-level-key))
+ ([query :- ::lib.schema/query
+ stage-number :- :int
+ top-level-key :- TopLevelKey]
+ (describe-top-level-key-method query stage-number (keyword top-level-key))))
+
(defmulti metadata
"Calculate appropriate metadata for something. What this looks like depends on what we're calculating metadata for. If
it's a reference or expression of some sort, this should return a single `:metadata/field` map (i.e., something
@@ -110,7 +127,7 @@
(mu/defn describe-query :- ::lib.schema.common/non-blank-string
"Convenience for calling [[display-name]] on a query to describe the results of its final stage."
[query]
- (display-name query -1 query))
+ (display-name query query))
(mu/defn suggested-name :- [:maybe ::lib.schema.common/non-blank-string]
"Name you might want to use for a query when saving an previously-unsaved query. This is the same
diff --git a/src/metabase/lib/metadata/graph_provider.cljc b/src/metabase/lib/metadata/graph_provider.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..bc62d0b402174702a0467e44223cdedbdd77c740
--- /dev/null
+++ b/src/metabase/lib/metadata/graph_provider.cljc
@@ -0,0 +1,56 @@
+(ns metabase.lib.metadata.graph-provider
+ (:require [medley.core :as m]
+ [metabase.lib.metadata.protocols :as lib.metadata.protocols]))
+
+(defn- graph-database [metadata-graph]
+ (dissoc metadata-graph :tables))
+
+(defn- graph-table [metadata-graph table-id]
+ (some (fn [table-metadata]
+ (when (= (:id table-metadata) table-id)
+ (dissoc table-metadata :fields :metrics :segments)))
+ (:tables metadata-graph)))
+
+(defn- graph-field [metadata-graph field-id]
+ (some (fn [table-metadata]
+ (m/find-first #(= (:id %) field-id)
+ (:fields table-metadata)))
+ (:tables metadata-graph)))
+
+(defn- graph-metric [metadata-graph metric-id]
+ (some (fn [table-metadata]
+ (m/find-first #(= (:id %) metric-id)
+ (:metrics table-metadata)))
+ (:tables metadata-graph)))
+
+(defn- graph-segment [metadata-graph segment-id]
+ (some (fn [table-metadata]
+ (m/find-first #(= (:id %) segment-id)
+ (:segments table-metadata)))
+ (:tables metadata-graph)))
+
+(defn- graph-card [_metadata-graph _card-id]
+ ;; not implemented for the simple graph metadata provider.
+ nil)
+
+(defn- graph-tables [metadata-graph]
+ (for [table-metadata (:tables metadata-graph)]
+ (dissoc table-metadata :fields :metrics :segments)))
+
+(defn- graph-fields [metadata-graph table-id]
+ (some (fn [table-metadata]
+ (when (= (:id table-metadata) table-id)
+ (:fields table-metadata)))
+ (:tables metadata-graph)))
+
+(deftype ^{:doc "A simple implementation of [[MetadataProvider]] that returns data from a complete graph
+ e.g. the response provided by `GET /api/database/:id/metadata`."} SimpleGraphMetadataProvider [metadata-graph]
+ lib.metadata.protocols/MetadataProvider
+ (database [_this] (graph-database metadata-graph))
+ (table [_this table-id] (graph-table metadata-graph table-id))
+ (field [_this field-id] (graph-field metadata-graph field-id))
+ (metric [_this metric-id] (graph-metric metadata-graph metric-id))
+ (segment [_this segment-id] (graph-segment metadata-graph segment-id))
+ (card [_this card-id] (graph-card metadata-graph card-id))
+ (tables [_this] (graph-tables metadata-graph))
+ (fields [_this table-id] (graph-fields metadata-graph table-id)))
diff --git a/src/metabase/lib/metadata/jvm.clj b/src/metabase/lib/metadata/jvm.clj
new file mode 100644
index 0000000000000000000000000000000000000000..0f43e9bd0a4c52dd64498dfd2af1b7566651e974
--- /dev/null
+++ b/src/metabase/lib/metadata/jvm.clj
@@ -0,0 +1,86 @@
+(ns metabase.lib.metadata.jvm
+ "Implementation(s) of [[metabase.lib.metadata.protocols/MetadataProvider]] only for the JVM."
+ (:require
+ [metabase.lib.metadata.cached-provider :as lib.metadata.cached-provider]
+ [metabase.lib.metadata.protocols :as lib.metadata.protocols]
+ [metabase.util.log :as log]
+ [potemkin :as p]
+ [pretty.core :as pretty]
+ [toucan2.core :as t2]))
+
+(defn- metadata-type->model [metadata-type]
+ (case metadata-type
+ :metadata/database :metabase.models.database/Database
+ :metadata/table :metabase.models.table/Table
+ :metadata/field :metabase.models.field/Field
+ :metadata/card :metabase.models.card/Card
+ :metadata/metric :metabase.models.metric/Metric
+ :metadata/segment :metabase.models.segment/Segment))
+
+(defn- fetch-instance [metadata-type id]
+ {:pre [(integer? id)]}
+ (let [model (metadata-type->model metadata-type)]
+ (log/debugf "Fetching %s %d" model id)
+ (when-some [instance (t2/select-one model :id id)]
+ (assoc instance :lib/type metadata-type))))
+
+(defn- bulk-instances [metadata-type ids]
+ (let [model (metadata-type->model metadata-type)]
+ (log/debugf "Fetching instances of %s with ID in %s" model (pr-str (sort ids)))
+ (for [instance (t2/select model :id [:in ids])]
+ (assoc instance :lib/type metadata-type))))
+
+(p/deftype+ UncachedApplicationDatabaseMetadataProvider [database-id]
+ lib.metadata.protocols/MetadataProvider
+ (database [_this]
+ (when-not database-id
+ (throw (ex-info (format "Cannot use %s with %s with a nil Database ID"
+ `lib.metadata.protocols/database
+ `UncachedApplicationDatabaseMetadataProvider)
+ {})))
+ (fetch-instance :metadata.models.database/Database database-id))
+
+ (table [_this table-id] (fetch-instance :metadata/table table-id))
+ (field [_this field-id] (fetch-instance :metadata/field field-id))
+ (card [_this card-id] (fetch-instance :metadata/card card-id))
+ (metric [_this metric-id] (fetch-instance :metadata/metric metric-id))
+ (segment [_this segment-id] (fetch-instance :metadata/segment segment-id))
+
+ (tables [_this]
+ (when-not database-id
+ (throw (ex-info (format "Cannot use %s with %s with a nil Database ID"
+ `lib.metadata.protocols/tables
+ `UncachedApplicationDatabaseMetadataProvider)
+ {})))
+ (log/debugf "Fetching all Tables for Database %d" database-id)
+ (into []
+ (map #(assoc % :lib/type :metadata/table))
+ (t2/reducible-select :metabase.models.table/Table :db_id database-id)))
+
+ (fields [_this table-id]
+ (log/debugf "Fetching all Fields for Table %d" table-id)
+ (into []
+ (map #(assoc % :lib/type :metadata/field))
+ (t2/reducible-select :table_id table-id)))
+
+ lib.metadata.protocols/BulkMetadataProvider
+ (bulk-metadata [_this metadata-type ids]
+ (bulk-instances metadata-type ids))
+
+ pretty/PrettyPrintable
+ (pretty [_this]
+ (list `->UncachedApplicationDatabaseMetadataProvider database-id)))
+
+(defn application-database-metadata-provider
+ "An implementation of [[metabase.lib.metadata.protocols/MetadataProvider]] for the application database.
+
+ The application database metadata provider implements both of the optional
+ protocols, [[metabase.lib.metadata.protocols/CachedMetadataProvider]]
+ and [[metabase.lib.metadata.protocols/BulkMetadataProvider]]. All operations are cached; so you can use the bulk
+ operations to pre-warm the cache if you need to."
+ ([]
+ (application-database-metadata-provider nil))
+
+ ([database-id]
+ (lib.metadata.cached-provider/cached-metadata-provider
+ (->UncachedApplicationDatabaseMetadataProvider database-id))))
diff --git a/src/metabase/lib/metadata/protocols.cljc b/src/metabase/lib/metadata/protocols.cljc
index 40981760ca54a6121cc5c6b85548031b98fbe620..6b8045930e99b17f2e9b4c78bf1ac13734a79639 100644
--- a/src/metabase/lib/metadata/protocols.cljc
+++ b/src/metabase/lib/metadata/protocols.cljc
@@ -1,8 +1,6 @@
(ns metabase.lib.metadata.protocols
(:require
- [medley.core :as m]
- #?@(:clj ([potemkin :as p]
- [pretty.core :as pretty]))))
+ #?@(:clj ([potemkin :as p]))))
(#?(:clj p/defprotocol+ :cljs defprotocol) MetadataProvider
"Protocol for something that we can get information about Tables and Fields from. This can be provided in various ways
@@ -26,26 +24,26 @@
For all of these methods: if no matching object can be found, you should generally return `nil` rather than throwing
an Exception. Let [[metabase.lib.metadata]] worry about throwing exceptions."
- (database [this]
+ (database [metadata-provider]
"Metadata about the Database we're querying. Should match the [[metabase.lib.metadata/DatabaseMetadata]] schema.
This includes important info such as the supported `:features` and the like.")
- (table [this table-id]
+ (table [metadata-provider table-id]
"Return metadata for a specific Table. Metadata should satisfy [[metabase.lib.metadata/TableMetadata]].")
- (field [this field-id]
+ (field [metadata-provider field-id]
"Return metadata for a specific Field. Metadata should satisfy [[metabase.lib.metadata/ColumnMetadata]].")
- (card [this card-id]
+ (card [metadata-provider card-id]
"Return information about a specific Saved Question, aka a Card. This should
match [[metabase.lib.metadata/CardMetadata]. Currently just used for display name purposes if you have a Card as a
source query.")
- (metric [this metric-id]
+ (metric [metadata-provider metric-id]
"Return metadata for a particular capital-M Metric, i.e. something from the `metric` table in the application
database. Metadata should match [[metabase.lib.metadata/MetricMetadata]].")
- (segment [this segment-id]
+ (segment [metadata-provider segment-id]
"Return metadata for a particular captial-S Segment, i.e. something from the `segment` table in the application
database. Metadata should match [[metabase.lib.metadata/SegmentMetadata]]." )
@@ -54,14 +52,14 @@
;; I'm on the fence about maybe putting these in a different protocol. They're part of this protocol for now tho so
;; implement them anyway.
- (tables [this]
+ (tables [metadata-provider]
"Return a sequence of Tables in this Database. Tables should satisfy the [[metabase.lib.metadata/TableMetadata]]
schema. This should also include things that serve as 'virtual' tables, e.g. Saved Questions or Models. But users of
MLv2 should not need to know that! If we add support for Super Models or Quantum Questions in the future, they can
just come back from this method in the same shape as everything else, the Query Builder can display them, and the
internals can be tucked away here in MLv2.")
- (fields [this table-id]
+ (fields [metadata-provider table-id]
"Return a sequence of Fields associated with a Table with the given `table-id`. Fields should satisfy
the [[metabase.lib.metadata/ColumnMetadata]] schema. If no such Table exists, this should error."))
@@ -70,64 +68,30 @@
[x]
(satisfies? MetadataProvider x))
-;;;; Graph provider and impl
-
-(defn- graph-database [metadata-graph]
- (dissoc metadata-graph :tables))
-
-(defn- graph-table [metadata-graph table-id]
- (some (fn [table-metadata]
- (when (= (:id table-metadata) table-id)
- (dissoc table-metadata :fields :metrics :segments)))
- (:tables metadata-graph)))
-
-(defn- graph-field [metadata-graph field-id]
- (some (fn [table-metadata]
- (m/find-first #(= (:id %) field-id)
- (:fields table-metadata)))
- (:tables metadata-graph)))
-
-(defn- graph-metric [metadata-graph metric-id]
- (some (fn [table-metadata]
- (m/find-first #(= (:id %) metric-id)
- (:metrics table-metadata)))
- (:tables metadata-graph)))
-
-(defn- graph-segment [metadata-graph segment-id]
- (some (fn [table-metadata]
- (m/find-first #(= (:id %) segment-id)
- (:segments table-metadata)))
- (:tables metadata-graph)))
-
-(defn- graph-card [_metadata-graph _card-id]
- ;; not implemented for the simple graph metadata provider.
- nil)
-
-(defn- graph-tables [metadata-graph]
- (for [table-metadata (:tables metadata-graph)]
- (dissoc table-metadata :fields :metrics :segments)))
-
-(defn- graph-fields [metadata-graph table-id]
- (some (fn [table-metadata]
- (when (= (:id table-metadata) table-id)
- (:fields table-metadata)))
- (:tables metadata-graph)))
-
-(deftype ^{:doc "A simple implementation of [[MetadataProvider]] that returns data from a complete graph
- e.g. the response provided by `GET /api/database/:id/metadata`."} SimpleGraphMetadataProvider [metadata-graph]
- MetadataProvider
- (database [_this] (graph-database metadata-graph))
- (table [_this table-id] (graph-table metadata-graph table-id))
- (field [_this field-id] (graph-field metadata-graph field-id))
- (metric [_this metric-id] (graph-metric metadata-graph metric-id))
- (segment [_this segment-id] (graph-segment metadata-graph segment-id))
- (card [_this card-id] (graph-card metadata-graph card-id))
- (tables [_this] (graph-tables metadata-graph))
- (fields [_this table-id] (graph-fields metadata-graph table-id))
-
- #?@(:clj
- [pretty/PrettyPrintable
- (pretty [_this]
- ;; don't actually print the whole thing because it's going to make my eyes bleed to see all
- ;; of [[metabase.lib.test-metadata]] every single time a test fails
- `SimpleGraphMetadataProvider)]))
+(#?(:clj p/defprotocol+ :cljs defprotocol) CachedMetadataProvider
+ "Optional. A protocol for a MetadataProvider that some sort of internal cache. This is mostly useful for MetadataProviders that
+ can hit some sort of relatively expensive external service,
+ e.g. [[metabase.lib.metadata.jvm/application-database-metadata-provider]].
+
+ See [[cached-metadata-provider]] below to wrap for a way to wrap an existing MetadataProvider to add caching on top
+ of it."
+ (cached-database [cached-metadata-provider]
+ "Get cached metadata for the query's Database.")
+ (cached-metadata [cached-metadata-provider metadata-type id]
+ "Get cached metadata of a specific type, e.g. `:metadata/table`.")
+ (store-database! [cached-metadata-provider database-metadata]
+ "Store metadata for the query's Database.")
+ (store-metadata! [cached-metadata-provider metadata-type id metadata]
+ "Store metadata of a specific type, e.g. `:metadata/table`."))
+
+(#?(:clj p/defprotocol+ :cljs defprotocol) BulkMetadataProvider
+ "A protocol for a MetadataProvider that can fetch several objects in a single batched operation. This is mostly
+ useful for MetadataProviders e.g. [[metabase.lib.metadata.jvm/application-database-metadata-provider]]."
+ (bulk-metadata [bulk-metadata-provider metadata-type ids]
+ "Fetch lots of metadata of a specific type, e.g. `:metadata/table`, in a single bulk operation."))
+
+(defn store-metadatas!
+ "Convenience. Store several metadata maps at once."
+ [cached-metadata-provider metadata-type metadatas]
+ (doseq [metadata metadatas]
+ (store-metadata! cached-metadata-provider metadata-type (:id metadata) metadata)))
diff --git a/src/metabase/lib/metric.cljc b/src/metabase/lib/metric.cljc
index 13dd7fd59a630637945d0e24e2bd41c0bf21a474..2a7fee07c65466b8d48025be14d3a97634e45d6e 100644
--- a/src/metabase/lib/metric.cljc
+++ b/src/metabase/lib/metric.cljc
@@ -7,11 +7,11 @@
(defmethod lib.metadata.calculation/display-name-method :metadata/metric
[_query _stage-number metric-metadata]
(or ((some-fn :display_name :name) metric-metadata)
- (i18n/tru "Metric")))
+ (i18n/tru "[Unknown Metric]")))
(defmethod lib.metadata.calculation/display-name-method :metric
[query stage-number [_tag _opts metric-id-or-name]]
(or (when (integer? metric-id-or-name)
(when-let [metric-metadata (lib.metadata/metric query metric-id-or-name)]
(lib.metadata.calculation/display-name query stage-number metric-metadata)))
- (i18n/tru "Metric")))
+ (i18n/tru "[Unknown Metric]")))
diff --git a/src/metabase/lib/order_by.cljc b/src/metabase/lib/order_by.cljc
index c4f7814054b067359d6024556afb89cd280d69e9..6d11f6f8425438e028d002c31030e2d8766cbded 100644
--- a/src/metabase/lib/order_by.cljc
+++ b/src/metabase/lib/order_by.cljc
@@ -15,7 +15,7 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
-(defmethod lib.metadata.calculation/describe-top-level-key :order-by
+(defmethod lib.metadata.calculation/describe-top-level-key-method :order-by
[query stage-number _k]
(when-let [order-bys (not-empty (:order-by (lib.util/query-stage query stage-number)))]
(i18n/tru "Sorted by {0}"
diff --git a/src/metabase/lib/query.cljc b/src/metabase/lib/query.cljc
index 81b7852e635660e375d91a5e33b73e905bf65882..f3e0d628b359f8e89e36124c0cf160a9afc04f0d 100644
--- a/src/metabase/lib/query.cljc
+++ b/src/metabase/lib/query.cljc
@@ -1,10 +1,12 @@
(ns metabase.lib.query
(:require
+ [metabase.lib.convert :as lib.convert]
[metabase.lib.dispatch :as lib.dispatch]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema]
+ [metabase.lib.schema.id :as lib.schema.id]
[metabase.lib.util :as lib.util]
[metabase.util.malli :as mu]))
@@ -16,12 +18,25 @@
[query stage-number x]
(lib.metadata.calculation/display-name query stage-number (lib.util/query-stage x stage-number)))
-(defn- query-with-stages [metadata-provider stages]
- {:lib/type :mbql/query
- :lib/metadata metadata-provider
- :database (:id (lib.metadata/database metadata-provider))
- :type :pipeline
- :stages (mapv lib.options/ensure-uuid stages)})
+(defn query-with-stages
+ "Create a query from a sequence of stages."
+ ([metadata-provider stages]
+ (query-with-stages (:id (lib.metadata/database metadata-provider)) metadata-provider stages))
+
+ ([database-id metadata-provider stages]
+ {:lib/type :mbql/query
+ :lib/metadata metadata-provider
+ :database database-id
+ :type :pipeline
+ :stages (mapv lib.options/ensure-uuid stages)}))
+
+(defn query-with-stage
+ "Create a query from a specific stage."
+ ([metadata-provider stage]
+ (query-with-stages metadata-provider [stage]))
+
+ ([database-id metadata-provider stage]
+ (query-with-stages database-id metadata-provider [stage])))
(defn- query-from-existing [metadata-provider query]
(let [query (lib.util/pipeline query)]
@@ -84,3 +99,12 @@
metadata
(lib.util/update-query-stage -1 assoc :lib/stage-metadata metadata))]
(query metadata-provider mbql-query)))
+
+(mu/defn query-from-legacy-inner-query :- ::lib.schema/query
+ "Create a pMBQL query from a legacy inner query."
+ [metadata-provider :- lib.metadata/MetadataProvider
+ database-id :- ::lib.schema.id/database
+ inner-query :- :map]
+ (->> (lib.convert/legacy-query-from-inner-query database-id inner-query)
+ lib.convert/->pMBQL
+ (query metadata-provider)))
diff --git a/src/metabase/lib/segment.cljc b/src/metabase/lib/segment.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..e77cf376ec580d49368fa79e9127018ab9426af4
--- /dev/null
+++ b/src/metabase/lib/segment.cljc
@@ -0,0 +1,17 @@
+(ns metabase.lib.segment
+ (:require
+ [metabase.lib.metadata :as lib.metadata]
+ [metabase.lib.metadata.calculation :as lib.metadata.calculation]
+ [metabase.shared.util.i18n :as i18n]))
+
+(defmethod lib.metadata.calculation/display-name-method :metadata/segment
+ [_query _stage-number segment-metadata]
+ (or ((some-fn :display_name :name) segment-metadata)
+ (i18n/tru "[Unknown Segment]")))
+
+(defmethod lib.metadata.calculation/display-name-method :segment
+ [query stage-number [_tag _opts segment-id-or-name]]
+ (or (when (integer? segment-id-or-name)
+ (when-let [segment-metadata (lib.metadata/segment query segment-id-or-name)]
+ (lib.metadata.calculation/display-name query stage-number segment-metadata)))
+ (i18n/tru "[Unknown Segment]")))
diff --git a/src/metabase/lib/stage.cljc b/src/metabase/lib/stage.cljc
index 24395ec728b6a8ffe3953a2ac7beb772b87ec979..74eafaebfbd74034f65993c01786df3ebb90f5c3 100644
--- a/src/metabase/lib/stage.cljc
+++ b/src/metabase/lib/stage.cljc
@@ -203,8 +203,7 @@
(defmethod lib.metadata.calculation/display-name-method :mbql.stage/mbql
[query stage-number _stage]
- (let [parts display-name-parts
- descriptions (for [k parts]
+ (let [descriptions (for [k display-name-parts]
(lib.metadata.calculation/describe-top-level-key query stage-number k))]
(str/join ", " (remove str/blank? descriptions))))
diff --git a/src/metabase/lib/table.cljc b/src/metabase/lib/table.cljc
index ed0864134a37c6227e2b25fdedaeeb25a53ddf24..5ef144bdf67f55e2d56daf5f30d77fb32cd9b62a 100644
--- a/src/metabase/lib/table.cljc
+++ b/src/metabase/lib/table.cljc
@@ -28,7 +28,7 @@
;; If for some reason the metadata is unavailable. This is better than returning nothing I guess
(i18n/tru "Saved Question {0}" card-id)))))
-(defmethod lib.metadata.calculation/describe-top-level-key :source-table
+(defmethod lib.metadata.calculation/describe-top-level-key-method :source-table
[query stage-number _k]
(let [stage (lib.util/query-stage query stage-number)]
(when-let [source-table-id (:source-table stage)]
diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj
index d38e13d1f5d10610660e7ab652ed64c7c240bb6e..f473694a5ea5fd3b7157f9afee9364092afdc24e 100644
--- a/src/metabase/models/metric.clj
+++ b/src/metabase/models/metric.clj
@@ -5,13 +5,23 @@
(:require
[clojure.set :as set]
[medley.core :as m]
+ [metabase.lib.core :as lib]
+ [metabase.lib.metadata :as lib.metadata]
+ [metabase.lib.metadata.jvm :as lib.metadata.jvm]
+ [metabase.lib.metadata.protocols :as lib.metadata.protocols]
+ [metabase.lib.query :as lib.query]
+ [metabase.lib.schema.common :as lib.schema.common]
+ [metabase.mbql.util :as mbql.u]
[metabase.models.interface :as mi]
[metabase.models.revision :as revision]
[metabase.models.serialization :as serdes]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
+ [metabase.util.malli :as mu]
+ [methodical.core :as methodical]
[toucan.models :as models]
- [toucan2.core :as t2]))
+ [toucan2.core :as t2]
+ [toucan2.tools.hydrate :as t2.hydrate]))
(models/defmodel Metric :metric)
@@ -40,9 +50,38 @@
::mi/entity-id true})
:pre-update pre-update})
-(defmethod serdes/hash-fields Metric
- [_metric]
- [:name (serdes/hydrated-hash :table) :created_at])
+(mu/defn ^:private definition-description :- [:maybe ::lib.schema.common/non-blank-string]
+ "Calculate a nice description of a Metric's definition."
+ [metadata-provider :- lib.metadata/MetadataProvider
+ {:keys [definition], table-id :table_id, :as _metric}]
+ (when (seq definition)
+ (when-let [{database-id :db_id} (lib.metadata.protocols/table metadata-provider table-id)]
+ (let [query (lib.query/query-from-legacy-inner-query metadata-provider database-id definition)]
+ (lib/describe-query query)))))
+
+(defn- warmed-metadata-provider [metrics]
+ (let [metadata-provider (doto (lib.metadata.jvm/application-database-metadata-provider)
+ (lib.metadata.protocols/store-metadatas! :metadata/metric metrics))
+ segment-ids (into #{} (mbql.u/match (map :definition metrics)
+ [:segment (id :guard integer?) & _]
+ id))
+ segments (lib.metadata.protocols/bulk-metadata metadata-provider :metadata/segment segment-ids)
+ field-ids (mbql.u/referenced-field-ids (into []
+ (comp cat (map :definition))
+ [metrics segments]))
+ fields (lib.metadata.protocols/bulk-metadata metadata-provider :metadata/field field-ids)
+ table-ids (into #{}
+ (comp cat (map :table_id))
+ [fields segments metrics])]
+ ;; this is done for side-effects
+ (lib.metadata.protocols/bulk-metadata metadata-provider :metadata/table table-ids)
+ metadata-provider))
+
+(methodical/defmethod t2.hydrate/batched-hydrate [Metric :definition_description]
+ [_model _key metrics]
+ (let [metadata-provider (warmed-metadata-provider metrics)]
+ (for [metric metrics]
+ (assoc metric :definition_description (definition-description metadata-provider metric)))))
;;; --------------------------------------------------- REVISIONS ----------------------------------------------------
@@ -70,6 +109,11 @@
;;; ------------------------------------------------- SERIALIZATION --------------------------------------------------
+
+(defmethod serdes/hash-fields Metric
+ [_metric]
+ [:name (serdes/hydrated-hash :table) :created_at])
+
(defmethod serdes/extract-one "Metric"
[_model-name _opts metric]
(-> (serdes/extract-one-basics "Metric" metric)
diff --git a/src/metabase/models/segment.clj b/src/metabase/models/segment.clj
index 20f5a88f4350c75c3ef5c70e2568073d8cea4e7f..4165cd104c1e384f838abbdfa447a846b41554a1 100644
--- a/src/metabase/models/segment.clj
+++ b/src/metabase/models/segment.clj
@@ -4,13 +4,23 @@
(:require
[clojure.set :as set]
[medley.core :as m]
+ [metabase.lib.core :as lib]
+ [metabase.lib.metadata :as lib.metadata]
+ [metabase.lib.metadata.jvm :as lib.metadata.jvm]
+ [metabase.lib.metadata.protocols :as lib.metadata.protocols]
+ [metabase.lib.query :as lib.query]
+ [metabase.lib.schema.common :as lib.schema.common]
+ [metabase.mbql.util :as mbql.u]
[metabase.models.interface :as mi]
[metabase.models.revision :as revision]
[metabase.models.serialization :as serdes]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
+ [metabase.util.malli :as mu]
+ [methodical.core :as methodical]
[toucan.models :as models]
- [toucan2.core :as t2]))
+ [toucan2.core :as t2]
+ [toucan2.tools.hydrate :as t2.hydrate]))
(models/defmodel Segment :segment)
@@ -40,9 +50,32 @@
:hydration-keys (constantly [:segment])
:pre-update pre-update})
-(defmethod serdes/hash-fields Segment
- [_segment]
- [:name (serdes/hydrated-hash :table) :created_at])
+(mu/defn ^:private definition-description :- [:maybe ::lib.schema.common/non-blank-string]
+ "Calculate a nice description of a Segment's definition."
+ [metadata-provider :- lib.metadata/MetadataProvider
+ {table-id :table_id, :keys [definition], :as _segment}]
+ (when (seq definition)
+ (when-let [{database-id :db_id} (lib.metadata.protocols/table metadata-provider table-id)]
+ (let [query (lib.query/query-from-legacy-inner-query metadata-provider database-id definition)]
+ (lib/describe-top-level-key query :filter)))))
+
+(defn- warmed-metadata-provider [segments]
+ (let [metadata-provider (doto (lib.metadata.jvm/application-database-metadata-provider)
+ (lib.metadata.protocols/store-metadatas! :metadata/segment segments))
+ field-ids (mbql.u/referenced-field-ids (map :definition segments))
+ fields (lib.metadata.protocols/bulk-metadata metadata-provider :metadata/field field-ids)
+ table-ids (into #{}
+ (comp cat (map :table_id))
+ [fields segments])]
+ ;; this is done for side effects
+ (lib.metadata.protocols/bulk-metadata metadata-provider :metadata/table table-ids)
+ metadata-provider))
+
+(methodical/defmethod t2.hydrate/batched-hydrate [Segment :definition_description]
+ [_model _key segments]
+ (let [metadata-provider (warmed-metadata-provider segments)]
+ (for [segment segments]
+ (assoc segment :definition_description (definition-description metadata-provider segment)))))
;;; --------------------------------------------------- Revisions ----------------------------------------------------
@@ -70,6 +103,11 @@
;;; ------------------------------------------------ Serialization ---------------------------------------------------
+
+(defmethod serdes/hash-fields Segment
+ [_segment]
+ [:name (serdes/hydrated-hash :table) :created_at])
+
(defmethod serdes/extract-one "Segment"
[_model-name _opts segment]
(-> (serdes/extract-one-basics "Segment" segment)
diff --git a/test/metabase/api/metric_test.clj b/test/metabase/api/metric_test.clj
index d2aacb45c0059a8a66002c52a50de7632388a1c5..c6159a8cec0e6e355ee0a79eb69e2efd4a8fea20 100644
--- a/test/metabase/api/metric_test.clj
+++ b/test/metabase/api/metric_test.clj
@@ -3,18 +3,15 @@
(:require
[clojure.test :refer :all]
[metabase.http-client :as client]
- [metabase.models.database :refer [Database]]
+ [metabase.models :refer [Database Revision Segment Table]]
[metabase.models.metric :as metric :refer [Metric]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
- [metabase.models.revision :refer [Revision]]
- [metabase.models.table :refer [Table]]
[metabase.server.middleware.util :as mw.util]
[metabase.test :as mt]
- [metabase.test.data :as data]
[metabase.util :as u]
- [toucan.hydrate :refer [hydrate]]
- [toucan2.core :as t2]))
+ [toucan2.core :as t2]
+ [toucan2.tools.with-temp :as t2.with-temp]))
;; ## Helper Fns
@@ -393,18 +390,32 @@
(deftest list-metrics-test
(testing "GET /api/metric/"
- (mt/with-temp* [Metric [metric-1 {:name "Metric A"}]
- Metric [metric-2 {:name "Metric B"}]
- ;; inactive metrics shouldn't show up
- Metric [_ {:archived true}]]
- (is (= (mt/derecordize (hydrate [(assoc metric-1 :database_id (data/id))
- (assoc metric-2 :database_id (data/id))]
- :creator))
- (map #(dissoc % :query_description) (mt/user-http-request
- :rasta :get 200 "metric/"))))))
-
- (is (= []
- (mt/user-http-request :rasta :get 200 "metric/"))))
+ (t2.with-temp/with-temp [Segment {segment-id :id} {:name "Segment"
+ :table_id (mt/id :checkins)
+ :definition (:query (mt/mbql-query checkins
+ {:filter [:= $id 1]}))}
+ Metric {id-1 :id} {:name "Metric A"
+ :table_id (mt/id :users)}
+ Metric {id-2 :id} {:name "Metric B"
+ :definition (:query (mt/mbql-query venues
+ {:aggregation [[:sum $category_id->categories.name]]
+ :filter [:and
+ [:= $price 4]
+ [:segment segment-id]]}))
+ :table_id (mt/id :venues)}
+ ;; inactive metrics shouldn't show up
+ Metric {id-3 :id} {:archived true}]
+ (is (=? [{:name "Metric A"
+ :id id-1
+ :creator {}
+ :definition_description nil}
+ {:name "Metric B"
+ :id id-2
+ :creator {}
+ :definition_description "Venues, Sum of Name, Filtered by Price equals 4 and Segment"}]
+ (filter (fn [{metric-id :id}]
+ (contains? #{id-1 id-2 id-3} metric-id))
+ (mt/user-http-request :rasta :get 200 "metric/")))))))
(deftest metric-related-entities-test
(testing "Test related/recommended entities"
diff --git a/test/metabase/api/query_description_test.clj b/test/metabase/api/query_description_test.clj
deleted file mode 100644
index af4cbcb09b24d44a2badd8367307021d75296564..0000000000000000000000000000000000000000
--- a/test/metabase/api/query_description_test.clj
+++ /dev/null
@@ -1,119 +0,0 @@
-(ns metabase.api.query-description-test
- (:require
- [clojure.test :refer :all]
- [metabase.api.query-description :as api.qd]
- [metabase.models.metric :refer [Metric]]
- [metabase.models.segment :refer [Segment]]
- [metabase.models.table :refer [Table]]
- [metabase.test :as mt]
- [metabase.test.fixtures :as fixtures]
- [metabase.util.i18n :refer [deferred-tru]]
- [toucan.util.test :as tt]
- [toucan2.core :as t2]))
-
-(use-fixtures :once (fixtures/initialize :db))
-
-(deftest metrics-query-test
- (testing "queries"
-
- (testing "without any arguments, just the table"
- (is (= {:table "Venues"}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues))))))
-
- (testing "with limit"
- (is (= {:table "Venues"
- :limit 10}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:limit 10}))))))
-
- (testing "with cumulative sum of price"
- (is (= {:table "Venues"
- :aggregation [{:type :cum-sum
- :arg "Price"}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:aggregation [[:cum-sum $price]]}))))))
- (testing "with equality filter"
- (is (= {:table "Venues"
- :filter [{:field "Price"}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:filter [:= [$price 1234]]}))))))
-
- (testing "with order-by clause"
- (is (= {:table "Venues"
- :order-by [{:field "Price"
- :direction :asc}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:order-by [[:asc $price]]}))))))
-
- (testing "with an aggregation metric"
- (tt/with-temp Metric [metric {:table_id (mt/id :venues)
- :name "Test Metric 1"
- :definition {:aggregation [[:count]]}}]
- (is (= {:table "Venues"
- :aggregation [{:type :metric
- :arg "Test Metric 1"}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:aggregation [[:metric (:id metric)]]}))))))
-
- (is (= {:table "Venues"
- :aggregation [{:type :metric
- :arg (deferred-tru "[Unknown Metric]")}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:aggregation [[:metric -1]]})))))
-
- ;; confirm that it doesn't crash for non-integer metrics
- (is (= {:table "Venues"}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:aggregation [[:metric "not-a-integer"]]}))))))
-
- (testing "with segment filters"
- (tt/with-temp Segment [segment {:name "Test Segment 1"}]
- (is (= {:table "Venues"
- :filter [{:segment "Test Segment 1"}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:filter [[:segment (:id segment)]]}))))))
-
- (is (= {:table "Venues"
- :filter [{:segment (deferred-tru "[Unknown Segment]")}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- (:query (mt/mbql-query venues
- {:filter [[:segment -1]]}))))))
-
- (testing "with named aggregation"
- (is (= {:table "Venues"
- :aggregation [{:type :aggregation
- :arg "Nonsensical named metric"}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- {:aggregation [[:aggregation-options
- [:sum [:*
- [:field (mt/id :venues :latitude) nil]
- [:field (mt/id :venues :longitude) nil]]]
- {:display-name "Nonsensical named metric"}]]}))))
-
- (testing "with unnamed complex aggregation"
- (is (= {:table "Venues"
- :aggregation [{:type :sum
- :arg ["Latitude" "*" "Longitude"]}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- {:aggregation [[:sum [:*
- [:field (mt/id :venues :latitude) nil]
- [:field (mt/id :venues :longitude) nil]]]]}))))
-
- (testing "with unnamed complex aggregation with multiple arguments"
- (is (= {:table "Venues"
- :aggregation [{:type :sum
- :arg ["Latitude" "+" "Longitude" "+" "ID"]}]}
- (api.qd/generate-query-description (t2/select-one Table :id (mt/id :venues))
- {:aggregation [[:sum [:+
- [:field (mt/id :venues :latitude) nil]
- [:field (mt/id :venues :longitude) nil]
- [:field (mt/id :venues :id) nil]]]]}))))))
diff --git a/test/metabase/api/segment_test.clj b/test/metabase/api/segment_test.clj
index 5da2ceee06bd3ffc1b0dce640fadebfe4cfae480..c00f22b7bb116fa167d6502957056f7de6b5aef3 100644
--- a/test/metabase/api/segment_test.clj
+++ b/test/metabase/api/segment_test.clj
@@ -3,17 +3,15 @@
(:require
[clojure.test :refer :all]
[metabase.http-client :as client]
- [metabase.models.database :refer [Database]]
+ [metabase.models :refer [Database Revision Table]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
- [metabase.models.revision :refer [Revision]]
[metabase.models.segment :as segment :refer [Segment]]
- [metabase.models.table :refer [Table]]
[metabase.server.middleware.util :as mw.util]
[metabase.test :as mt]
[metabase.util :as u]
- [toucan.hydrate :refer [hydrate]]
- [toucan2.core :as t2]))
+ [toucan2.core :as t2]
+ [toucan2.tools.with-temp :as t2.with-temp]))
;; ## Helper Fns
@@ -211,21 +209,20 @@
(is (= nil
(mt/user-http-request :crowberto :delete 204 (format "segment/%d" id) :revision_message "carryon")))
(testing "should still be able to fetch the archived segment"
- (is (= {:name "Toucans in the rainforest"
- :description "Lookin' for a blueberry"
- :show_in_getting_started false
- :caveats nil
- :points_of_interest nil
- :creator_id (mt/user->id :rasta)
- :creator (user-details (mt/fetch-user :rasta))
- :created_at true
- :updated_at true
- :entity_id true
- :archived true
- :definition nil}
- (-> (mt/user-http-request :crowberto :get 200 (format "segment/%d" id))
- segment-response
- (dissoc :query_description))))))))
+ (is (=? {:name "Toucans in the rainforest"
+ :description "Lookin' for a blueberry"
+ :show_in_getting_started false
+ :caveats nil
+ :points_of_interest nil
+ :creator_id (mt/user->id :rasta)
+ :creator (user-details (mt/fetch-user :rasta))
+ :created_at true
+ :updated_at true
+ :entity_id true
+ :archived true
+ :definition nil}
+ (-> (mt/user-http-request :crowberto :get 200 (format "segment/%d" id))
+ segment-response)))))))
;; ## GET /api/segment/:id
@@ -408,21 +405,30 @@
(for [revision (mt/user-http-request :crowberto :get 200 (format "segment/%d/revisions" id))]
(dissoc revision :timestamp :id))))))))
-
-;;; GET /api/segement/
-
(deftest list-test
- (testing "GET /api/segement/"
- (mt/with-temp* [Segment [segment-1 {:name "Segment 1"}]
- Segment [segment-2 {:name "Segment 2"}]
- ;; inactive segments shouldn't show up
- Segment [_ {:archived true}]]
- (is (= (mt/derecordize (hydrate [segment-1
- segment-2] :creator))
- (map #(dissoc % :query_description) (mt/user-http-request :rasta :get 200 "segment/")))))
-
- (is (= []
- (mt/user-http-request :rasta :get 200 "segment/")))))
+ (testing "GET /api/segment/"
+ (t2.with-temp/with-temp [Segment {id-1 :id} {:name "Segment 1"
+ :table_id (mt/id :users)}
+ Segment {id-2 :id} {:name "Segment 2"
+ :definition (:query (mt/mbql-query venues
+ {:filter
+ [:and
+ [:= $price 4]
+ [:= $category_id->categories.name "BBQ"]]}))}
+ ;; inactive segments shouldn't show up
+ Segment {id-3 :id} {:archived true}]
+ (is (=? [{:id id-1
+ :name "Segment 1"
+ :creator {}
+ :definition_description nil}
+ {:id id-2
+ :name "Segment 2"
+ :definition {}
+ :creator {}
+ :definition_description "Filtered by Price equals 4 and Name equals \"BBQ\""}]
+ (filter (fn [{segment-id :id}]
+ (contains? #{id-1 id-2 id-3} segment-id))
+ (mt/user-http-request :rasta :get 200 "segment/")))))))
(deftest related-entities-test
(testing "GET /api/segment/:id/related"
diff --git a/test/metabase/lib/breakout_test.cljc b/test/metabase/lib/breakout_test.cljc
index 396cd74120c369c7a7b9272006c543aee49fd81c..1dafabc42384559a66fd55793ffc295e3beb8899 100644
--- a/test/metabase/lib/breakout_test.cljc
+++ b/test/metabase/lib/breakout_test.cljc
@@ -22,4 +22,6 @@
(meta/id :checkins :date)]]}]}
query))
(is (= "Checkins, Count, Grouped by Date (year)"
+ (lib/display-name query query)
+ (lib/describe-query query)
(lib/suggested-name query)))))
diff --git a/test/metabase/lib/metadata/cached_provider_test.cljc b/test/metabase/lib/metadata/cached_provider_test.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..c0b0df2d906aac43f21002d2b4060345e10fe6ff
--- /dev/null
+++ b/test/metabase/lib/metadata/cached_provider_test.cljc
@@ -0,0 +1,54 @@
+(ns metabase.lib.metadata.cached-provider-test
+ (:require
+ [clojure.test :refer [deftest is testing]]
+ [metabase.lib.metadata.cached-provider :as lib.metadata.cached-provider]
+ [metabase.lib.metadata.protocols :as lib.metadata.protocols]))
+
+(deftest ^:parallel caching-test
+ (let [fetch-count (atom 0)
+ provider (lib.metadata.cached-provider/cached-metadata-provider
+ (reify
+ lib.metadata.protocols/MetadataProvider
+ (table [_this id]
+ (swap! fetch-count inc)
+ {:id id})
+
+ lib.metadata.protocols/BulkMetadataProvider
+ (bulk-metadata [_this metadata-type ids]
+ (case metadata-type
+ :metadata/table (for [id ids]
+ (do
+ (swap! fetch-count inc)
+ {:id id}))))))]
+ (testing "Initial fetch"
+ (is (= {:id 1}
+ (lib.metadata.protocols/table provider 1)))
+ (is (= 1
+ @fetch-count)))
+ (testing "Second fetch"
+ (is (= {:id 1}
+ (lib.metadata.protocols/table provider 1)))
+ (is (= 1
+ @fetch-count)))
+ (testing "Second fetch"
+ (is (= {:id 1}
+ (lib.metadata.protocols/table provider 1)))
+ (is (= 1
+ @fetch-count)))
+ (testing "Bulk fetch"
+ (is (= [{:id 1}]
+ (lib.metadata.protocols/bulk-metadata provider :metadata/table #{1})))
+ (is (= 1
+ @fetch-count))
+ (testing "Fetch a new Table, 1 Table already fetched"
+ (is (= [{:id 1}
+ {:id 2}]
+ (lib.metadata.protocols/bulk-metadata provider :metadata/table #{1 2})))
+ (is (= 2
+ @fetch-count)))
+ (testing "Bulk fetch again, should use cached results"
+ (is (= [{:id 1}
+ {:id 2}]
+ (lib.metadata.protocols/bulk-metadata provider :metadata/table #{1 2})))
+ (is (= 2
+ @fetch-count))))))
diff --git a/test/metabase/lib/test_metadata.cljc b/test/metabase/lib/test_metadata.cljc
index 9885e057d3234d33fd20ff56113f7610915dfac9..8b54a78a42e296cf4fa9eb684a0ebee8dc8b9387 100644
--- a/test/metabase/lib/test_metadata.cljc
+++ b/test/metabase/lib/test_metadata.cljc
@@ -7,7 +7,7 @@
will not be reflected here, for example if we add new information to the metadata. We'll have to manually update
these things if that happens and Metabase lib is meant to consume it."
(:require
- [metabase.lib.metadata.protocols :as lib.metadata.protocols]))
+ [metabase.lib.metadata.graph-provider :as lib.metadata.graph-provider]))
(defonce ^:private ^{:doc "Generate a random prefix to add to all of the [[id]]s below, so that they change between
test runs to catch places where people are hardcoding IDs rather than using [[id]]."}
@@ -786,7 +786,7 @@
(def metadata-provider
"[[metabase.lib.metadata.protocols/MetadataProvider]] using the test [[metadata]]."
- (lib.metadata.protocols/->SimpleGraphMetadataProvider metadata))
+ (lib.metadata.graph-provider/->SimpleGraphMetadataProvider metadata))
(def results-metadata
"Capture of the `data.results_metadata` that would come back when running `SELECT * FROM VENUES;` with the Query
diff --git a/test/metabase/models/metric_test.clj b/test/metabase/models/metric_test.clj
index cbca9a4823fc4129b6855a2b34f608a1328678b7..e866190c62c254cf3b8cd3eba2197f94f577790d 100644
--- a/test/metabase/models/metric_test.clj
+++ b/test/metabase/models/metric_test.clj
@@ -1,13 +1,13 @@
(ns metabase.models.metric-test
(:require
[clojure.test :refer :all]
- [metabase.models.database :refer [Database]]
+ [metabase.models :refer [Database Segment Table]]
[metabase.models.metric :as metric :refer [Metric]]
[metabase.models.revision :as revision]
[metabase.models.serialization :as serdes]
- [metabase.models.table :refer [Table]]
[metabase.test :as mt]
- [toucan2.core :as t2])
+ [toucan2.core :as t2]
+ [toucan2.tools.with-temp :as t2.with-temp])
(:import
(java.time LocalDateTime)))
@@ -124,3 +124,23 @@
(is (= "a2318866"
(serdes/raw-hash ["measurement" (serdes/identity-hash table) now])
(serdes/identity-hash metric)))))))
+
+(deftest definition-descriptions-test
+ (testing ":definition_description should hydrate to nil if :definition is missing"
+ (t2.with-temp/with-temp [Metric metric {:name "Metric A"
+ :table_id (mt/id :users)}]
+ (is (= nil
+ (:definition_description (t2/hydrate metric :definition_description))))))
+ (t2.with-temp/with-temp [Segment {segment-id :id} {:name "Checkins with ID = 1"
+ :table_id (mt/id :checkins)
+ :definition (:query (mt/mbql-query checkins
+ {:filter [:= $id 1]}))}
+ Metric metric {:name "Metric B"
+ :table_id (mt/id :venues)
+ :definition (:query (mt/mbql-query venues
+ {:aggregation [[:sum $category_id->categories.name]]
+ :filter [:and
+ [:= $price 4]
+ [:segment segment-id]]}))}]
+ (is (= "Venues, Sum of Name, Filtered by Price equals 4 and Checkins with ID = 1"
+ (:definition_description (t2/hydrate metric :definition_description))))))
diff --git a/test/metabase/models/segment_test.clj b/test/metabase/models/segment_test.clj
index b7e09dcbf0c59e77789c2a4e3dd3d7cc81b65715..02f3a4e3d68ef461712fd907af43feb12f30e1aa 100644
--- a/test/metabase/models/segment_test.clj
+++ b/test/metabase/models/segment_test.clj
@@ -7,7 +7,8 @@
[metabase.models.serialization :as serdes]
[metabase.models.table :refer [Table]]
[metabase.test :as mt]
- [toucan2.core :as t2])
+ [toucan2.core :as t2]
+ [toucan2.tools.with-temp :as t2.with-temp])
(:import
(java.time LocalDateTime)))
@@ -115,3 +116,27 @@
(is (= "be199b7c"
(serdes/raw-hash ["big customers" (serdes/identity-hash table) now])
(serdes/identity-hash segment)))))))
+
+(deftest definition-description-test
+ (testing "Do not hydrate definition description if definition is nil"
+ (t2.with-temp/with-temp [Segment segment {:name "Segment"
+ :table_id (mt/id :users)}]
+ (is (=? {:definition_description nil}
+ (t2/hydrate segment :definition_description)))))
+ (t2.with-temp/with-temp [Segment segment {:name "Expensive BBQ Spots"
+ :definition (:query (mt/mbql-query venues
+ {:filter
+ [:and
+ [:= $price 4]
+ [:= $category_id->categories.name "BBQ"]]}))}]
+ (is (= "Filtered by Price equals 4 and Name equals \"BBQ\""
+ (:definition_description (t2/hydrate segment :definition_description))))
+ (testing "Segments that reference other Segments (inception)"
+ (t2.with-temp/with-temp [Segment segment-2 {:name "Segment 2"
+ :definition (:query (mt/mbql-query categories
+ {:filter
+ [:and
+ [:segment (:id segment)]
+ [:not-null $id]]}))}]
+ (is (= "Filtered by Expensive BBQ Spots and ID is not empty"
+ (:definition_description (t2/hydrate segment-2 :definition_description))))))))