diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index c7c11a7a08f3431ca2257227f58fd52029774029..f72aa65e406bb8cade9bacba30a29709ae80ca61 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -409,6 +409,7 @@
metabase.test.domain-entities test.de
metabase.test.mock.util mock.util
metabase.test.sync test.sync
+ metabase.test.util.js test.js
metabase.test.util.timezone test.tz
metabase.timeseries-query-processor-test.util tqpt
metabase.transforms.core tf
diff --git a/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js b/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
index 00a4acf71877e6b94573a2528a53f5d642ef5e38..e1095a3b79b1ad40a0c080c1617518b0a4114533 100644
--- a/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
+++ b/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
@@ -110,7 +110,7 @@ const MODEL_NAME = "Test Action Model";
clickHelper("Update Score");
cy.findByRole("dialog").within(() => {
- cy.findByLabelText("New score").type("55");
+ cy.findByLabelText("New Score").type("55");
cy.button(ACTION_NAME).click();
});
diff --git a/e2e/test/scenarios/models/model-actions.cy.spec.js b/e2e/test/scenarios/models/model-actions.cy.spec.js
index 0b0e05720fcb14ce376abfa7da4a16bba5fcf350..1bbebf6e3121f613c0e0276d48140a1c5686a449 100644
--- a/e2e/test/scenarios/models/model-actions.cy.spec.js
+++ b/e2e/test/scenarios/models/model-actions.cy.spec.js
@@ -294,7 +294,7 @@ describe(
fillActionQuery("{{id}}");
cy.findByLabelText("#1-orders-model").should("not.exist");
cy.findByLabelText("101").should("not.exist");
- cy.findByLabelText("Id").should("be.visible");
+ cy.findByLabelText("ID").should("be.visible");
});
},
);
diff --git a/frontend/src/metabase-lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/queries/NativeQuery.ts
index a6f4ff62310f2fee03f2b111721b1a203f8f61ec..56893803ce2bcd1421d41b67a9f509131ab01563 100644
--- a/frontend/src/metabase-lib/queries/NativeQuery.ts
+++ b/frontend/src/metabase-lib/queries/NativeQuery.ts
@@ -4,8 +4,7 @@ import { t } from "ttag";
import { assoc, assocIn, chain, getIn, updateIn } from "icepick";
import _ from "underscore";
import slugg from "slugg";
-import { humanize } from "metabase/lib/formatting";
-import Utils from "metabase/lib/utils";
+import * as ML from "cljs/metabase.lib.js";
import { ParameterValuesConfig } from "metabase-types/api";
import {
Card,
@@ -25,7 +24,6 @@ import AtomicQuery from "metabase-lib/queries/AtomicQuery";
import { getTemplateTagParameter } from "metabase-lib/parameters/utils/template-tags";
import Variable from "metabase-lib/variables/Variable";
import TemplateTagVariable from "metabase-lib/variables/TemplateTagVariable";
-import { createTemplateTag } from "metabase-lib/queries/TemplateTag";
import ValidationError from "metabase-lib/ValidationError";
import { isFieldReference } from "metabase-lib/references";
import Dimension, { FieldDimension, TemplateTagDimension } from "../Dimension";
@@ -47,30 +45,8 @@ export const NATIVE_QUERY_TEMPLATE: NativeDatasetQuery = {
///////////////////////////
// QUERY TEXT TAG UTILS
-const VARIABLE_TAG_REGEX: RegExp = /\{\{\s*([A-Za-z0-9_\.]+)\s*\}\}/g;
-const SNIPPET_TAG_REGEX: RegExp = /\{\{\s*(snippet:\s*[^}]+)\s*\}\}/g;
export const CARD_TAG_REGEX: RegExp =
/\{\{\s*(#([0-9]*)(-[a-z0-9-]*)?)\s*\}\}/g;
-const TAG_REGEXES: RegExp[] = [
- VARIABLE_TAG_REGEX,
- SNIPPET_TAG_REGEX,
- CARD_TAG_REGEX,
-];
-
-// look for variable usage in the query (like '{{varname}}'). we only allow alphanumeric characters for the variable name
-// a variable name can optionally end with :start or :end which is not considered part of the actual variable name
-// expected pattern is like mustache templates, so we are looking for something like {{category}}
-// anything that doesn't match our rule is ignored, so {{&foo!}} would simply be ignored
-// See unit tests for examples
-export function recognizeTemplateTags(queryText: string): string[] {
- const tagNames = TAG_REGEXES.flatMap(r =>
- Array.from(queryText.matchAll(r)),
- ).map(m => m[1]);
- return _.uniq(tagNames);
-}
-
-// matches '#123-foo-bar' and '#123' but not '#123foo'
-const CARD_TAG_NAME_REGEX: RegExp = /^#([0-9]*)(-[a-z0-9-]*)?$/;
function tagRegex(tagName: string): RegExp {
return new RegExp(`{{\\s*${tagName}\\s*}}`, "g");
@@ -87,23 +63,6 @@ function replaceTagName(
return query.setQueryText(queryText);
}
-export function cardIdFromTagName(name: string): number | null {
- const match = name.match(CARD_TAG_NAME_REGEX);
- return parseInt(match?.[1]) || null;
-}
-
-function isCardTagName(tagName: string): boolean {
- return CARD_TAG_NAME_REGEX.test(tagName);
-}
-
-function snippetNameFromTagName(name: string): string {
- return name.slice("snippet:".length).trim();
-}
-
-function isSnippetTagName(name: string): boolean {
- return name.startsWith("snippet:");
-}
-
export function updateCardTemplateTagNames(
query: NativeQuery,
cards: Card[],
@@ -478,77 +437,9 @@ export default class NativeQuery extends AtomicQuery {
* special handling for NATIVE cards to automatically detect parameters ... {{varname}}
*/
private _getUpdatedTemplateTags(queryText: string): TemplateTags {
- if (queryText && this.supportsNativeParameters()) {
- const tags = recognizeTemplateTags(queryText);
- const existingTemplateTags = this.templateTagsMap();
- const existingTags = Object.keys(existingTemplateTags);
-
- // if we ended up with any variables in the query then update the card parameters list accordingly
- if (tags.length > 0 || existingTags.length > 0) {
- const newTags = _.difference(tags, existingTags);
-
- const oldTags = _.difference(existingTags, tags);
-
- const templateTags = { ...existingTemplateTags };
-
- if (oldTags.length === 1 && newTags.length === 1) {
- // renaming
- const newTag = { ...templateTags[oldTags[0]] };
-
- if (newTag["display-name"] === humanize(oldTags[0])) {
- newTag["display-name"] = humanize(newTags[0]);
- }
-
- newTag.name = newTags[0];
-
- if (isCardTagName(newTag.name)) {
- newTag.type = "card";
- newTag["card-id"] = cardIdFromTagName(newTag.name);
- } else if (isSnippetTagName(newTag.name)) {
- newTag.type = "snippet";
- newTag["snippet-name"] = snippetNameFromTagName(newTag.name);
- }
-
- templateTags[newTag.name] = newTag;
- delete templateTags[oldTags[0]];
- } else {
- // remove old vars
- for (const name of oldTags) {
- delete templateTags[name];
- }
-
- // create new vars
- for (const tagName of newTags) {
- templateTags[tagName] = createTemplateTag(tagName);
-
- // parse card ID from tag name for card query template tags
- if (isCardTagName(tagName)) {
- templateTags[tagName] = Object.assign(templateTags[tagName], {
- type: "card",
- "card-id": cardIdFromTagName(tagName),
- });
- } else if (isSnippetTagName(tagName)) {
- // extract snippet name from snippet tag
- templateTags[tagName] = Object.assign(templateTags[tagName], {
- type: "snippet",
- "snippet-name": snippetNameFromTagName(tagName),
- });
- }
- }
- }
-
- // ensure all tags have an id since we need it for parameter values to work
- for (const tag: TemplateTag of Object.values(templateTags)) {
- if (tag.id == null) {
- tag.id = Utils.uuid();
- }
- }
-
- return templateTags;
- }
- }
-
- return {};
+ return queryText && this.supportsNativeParameters()
+ ? ML.template_tags(queryText, this.templateTagsMap())
+ : {};
}
dependentMetadata(): DependentMetadataItem[] {
diff --git a/frontend/src/metabase-lib/queries/TemplateTag.ts b/frontend/src/metabase-lib/queries/TemplateTag.ts
deleted file mode 100644
index eae0aeeecc7de76c7a2cb23f8997f65b46076c3d..0000000000000000000000000000000000000000
--- a/frontend/src/metabase-lib/queries/TemplateTag.ts
+++ /dev/null
@@ -1,12 +0,0 @@
-import Utils from "metabase/lib/utils";
-import { humanize } from "metabase/lib/formatting";
-import { TemplateTag } from "metabase-types/types/Query";
-
-export const createTemplateTag = (tagName: string): TemplateTag => {
- return {
- id: Utils.uuid(),
- name: tagName,
- "display-name": humanize(tagName),
- type: "text",
- };
-};
diff --git a/frontend/src/metabase-lib/queries/TemplateTag.unit.spec.ts b/frontend/src/metabase-lib/queries/TemplateTag.unit.spec.ts
deleted file mode 100644
index c36819b96077804f2969d72475de4a342d44976c..0000000000000000000000000000000000000000
--- a/frontend/src/metabase-lib/queries/TemplateTag.unit.spec.ts
+++ /dev/null
@@ -1,11 +0,0 @@
-import { createTemplateTag } from "metabase-lib/queries/TemplateTag";
-
-describe("createTemplateTag", () => {
- it("should create a proper template tag", () => {
- const tag = createTemplateTag("stars");
- expect(tag.name).toEqual("stars");
- expect(tag.type).toEqual("text");
- expect(typeof tag.id).toEqual("string");
- expect(tag["display-name"]).toEqual("Stars");
- });
-});
diff --git a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
index fab98aab68aa7c77e596de291aee8df981f7eb4d..5897dee4f49ffdb226757223ac13491bd5897f94 100644
--- a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
@@ -7,8 +7,6 @@ import {
} from "__support__/sample_database_fixture";
import NativeQuery, {
- recognizeTemplateTags,
- cardIdFromTagName,
updateCardTemplateTagNames,
} from "metabase-lib/queries/NativeQuery";
@@ -177,7 +175,7 @@ describe("NativeQuery", () => {
);
const tagMaps = newQuery.templateTagsMap();
expect(tagMaps["max_price"].name).toEqual("max_price");
- expect(tagMaps["max_price"]["display-name"]).toEqual("Max price");
+ expect(tagMaps["max_price"]["display-name"]).toEqual("Max Price");
});
});
@@ -241,7 +239,7 @@ describe("NativeQuery", () => {
{ "snippet-name": snippetName, "display-name": displayName, type },
] = q.templateTags();
expect(snippetName).toEqual("foo");
- expect(displayName).toEqual("Snippet: foo ");
+ expect(displayName).toEqual("Snippet: Foo");
expect(type).toEqual("snippet");
});
it("should update query text with new snippet names", () => {
@@ -378,57 +376,4 @@ describe("NativeQuery", () => {
expect(barTag["name"]).toEqual("#1234-bar"); // bar's name is the same
});
});
-
- describe("recognizeTemplateTags", () => {
- it("should handle standard variable names", () => {
- expect(recognizeTemplateTags("SELECT * from {{products}}")).toEqual([
- "products",
- ]);
- });
-
- it("should allow duplicated variables", () => {
- expect(
- recognizeTemplateTags("SELECT {{col}} FROM {{t}} ORDER BY {{col}} "),
- ).toEqual(["col", "t"]);
- });
-
- it("should ignore non-alphanumeric markers", () => {
- expect(recognizeTemplateTags("SELECT * from X -- {{&universe}}")).toEqual(
- [],
- );
- });
-
- it("should handle snippets", () => {
- expect(
- recognizeTemplateTags(
- "SELECT * from {{snippet: A snippet name}} cross join {{ snippet: another-snippet with *&#) }}",
- ),
- ).toEqual([
- "snippet: A snippet name",
- "snippet: another-snippet with *&#) ",
- ]);
- });
-
- it("should handle card references", () => {
- expect(
- recognizeTemplateTags(
- "SELECT * from {{#123}} cross join {{ #456-a-card-name }} cross join {{#not-this}} cross join {{#123or-this}}",
- ),
- ).toEqual(["#123", "#456-a-card-name"]);
- });
- });
-
- describe("cardIdFromTagName", () => {
- it("should get card Ids from a card tag name", () => {
- expect(cardIdFromTagName("#123-foo")).toEqual(123);
- expect(cardIdFromTagName("#123-foo-456")).toEqual(123);
- expect(cardIdFromTagName("#123")).toEqual(123);
- });
-
- it("should return null for invalid card tag names", () => {
- expect(cardIdFromTagName("123-foo")).toEqual(null);
- expect(cardIdFromTagName("#123foo")).toEqual(null);
- expect(cardIdFromTagName("123")).toEqual(null);
- });
- });
});
diff --git a/src/metabase/domain_entities/converters.cljs b/src/metabase/domain_entities/converters.cljs
index 631df8c12b6990b08335f31cbab8939a0043c8a1..14b60ad080bf609544ad0e576e067ce9447ac909 100644
--- a/src/metabase/domain_entities/converters.cljs
+++ b/src/metabase/domain_entities/converters.cljs
@@ -5,27 +5,39 @@
[malli.transform :as mtx]))
(defn- decode-map [schema _]
- (let [drop-nil? (set (for [[map-key props val-schema] (mc/children schema)
- :when (and (:optional props)
- (not (#{'nil? :nil} (mc/type val-schema))))]
- map-key))]
+ (let [by-prop (into {} (for [[map-key props] (mc/children schema)]
+ [(or (get props :js/prop)
+ (csk/->snake_case_string map-key))
+ {:map-key map-key}]))]
{:enter (fn [x]
(cond
- (map? x) x
- (object? x) (into {} (for [prop (js/Object.keys x)
- :let [js-val (unchecked-get x prop)
- map-key (csk/->kebab-case-keyword prop)]
- ;; If the value is nil, and it's both :optional and not a :maybe,
- ;; we just discard the value.
- :when (not (and (nil? js-val)
- (drop-nil? map-key)))]
- [map-key js-val]))))
+ (map? x) x
+ (object? x)
+ (into {} (for [prop (js-keys x)
+ :let [js-val (unchecked-get x prop)
+ map-key (or (get-in by-prop [prop :map-key])
+ (csk/->kebab-case-keyword prop))]]
+ [map-key js-val]))))
:leave (fn [x]
- (if (map? x)
- x
+ (if (object? x)
(throw (ex-info "decode-map leaving with a JS object not a CLJS map"
{:value x
- :schema schema}))))}))
+ :schema (mc/form schema)}))
+ x))}))
+
+(defn- infer-child-decoder [schema _]
+ (let [mapping (into {} (for [c (mc/children schema)]
+ (if (keyword? c)
+ [(name c) c]
+ [c c])))]
+ {:enter #(mapping % %)}))
+
+(defn- infer-child-encoder [schema _]
+ (let [mapping (into {} (for [c (mc/children schema)]
+ (if (keyword? c)
+ [c (name c)]
+ [c c])))]
+ {:enter #(mapping % %)}))
(defn- decode-map-of [keydec x]
(cond
@@ -42,6 +54,14 @@
#js {}
x)))
+(def ^:private identity-transformers
+ (-> ['string? :string
+ 'number? :number
+ 'int? :int
+ 'double? :double
+ 'float? :float]
+ (zipmap (repeat {:enter identity}))))
+
(def js-transformer
"Malli transformer for converting JavaScript data to and from CLJS data.
@@ -72,32 +92,44 @@
they are JS arrays."
(mtx/transformer
{:name :js
- :decoders {:keyword keyword
- 'keyword? keyword
- :qualified-keyword keyword
- :vector {:enter vec}
- :sequential {:enter vec}
- :tuple {:enter vec}
- :map {:compile decode-map}
- :map-of {:compile (fn [schema _]
- (let [[key-schema] (mc/children schema)
- keydec (mc/decoder key-schema js-transformer)]
- {:enter #(decode-map-of keydec %)}))}}
- :encoders {:keyword name
- 'keyword? name
- :qualified-keyword #(str (namespace %) "/" (name %))
- :vector {:leave clj->js}
- :sequential {:leave clj->js}
- :tuple {:leave clj->js}
- :map {:compile
- (fn [schema _]
- (let [js-props (into {} (for [[k props] (mc/children schema)
- :when (:js/prop props)]
- [k (:js/prop props)]))
- keyenc (fn [k] (or (get js-props k)
- (csk/->snake_case_string k)))]
- {:leave #(encode-map % keyenc)}))}
- :map-of {:leave #(encode-map % name)}}}))
+ :decoders
+ (merge identity-transformers
+ {:keyword keyword
+ 'keyword? keyword
+ :qualified-keyword keyword
+ :uuid parse-uuid
+ :vector {:enter #(and % (vec %))}
+ :sequential {:enter #(and % (vec %))}
+ :tuple {:enter #(and % (vec %))}
+ :cat {:enter #(and % (vec %))}
+ :catn {:enter #(and % (vec %))}
+ :enum {:compile infer-child-decoder}
+ := {:compile infer-child-decoder}
+ :map {:compile decode-map}
+ :map-of {:compile (fn [schema _]
+ (let [[key-schema] (mc/children schema)
+ keydec (mc/decoder key-schema js-transformer)]
+ {:enter #(decode-map-of keydec %)}))}})
+ :encoders
+ (merge identity-transformers
+ {:keyword name
+ 'keyword? name
+ :qualified-keyword #(str (namespace %) "/" (name %))
+ :uuid str
+ :vector {:leave clj->js}
+ :sequential {:leave clj->js}
+ :tuple {:leave clj->js}
+ :enum {:compile infer-child-encoder}
+ := {:compile infer-child-encoder}
+ :map {:compile
+ (fn [schema _]
+ (let [js-props (into {} (for [[k props] (mc/children schema)
+ :when (:js/prop props)]
+ [k (:js/prop props)]))
+ keyenc (fn [k] (or (get js-props k)
+ (csk/->snake_case_string k)))]
+ {:leave #(encode-map % keyenc)}))}
+ :map-of {:leave #(encode-map % name)}})}))
(defn incoming
"Returns a function for converting a JS value into CLJS data structures, based on a schema."
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index b1f7100d344efe47c5402986e1c442ff5b3fea80..03f01df2858bc60d684bfed7d48b44b07d192e6a 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -14,6 +14,7 @@
[metabase.lib.limit :as lib.limit]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.metric :as lib.metric]
+ [metabase.lib.native :as lib.native]
[metabase.lib.order-by :as lib.order-by]
[metabase.lib.query :as lib.query]
[metabase.lib.segment :as lib.segment]
@@ -32,6 +33,7 @@
lib.limit/keep-me
lib.metadata.calculation/keep-me
lib.metric/keep-me
+ lib.native/keep-me
lib.order-by/keep-me
lib.query/keep-me
lib.segment/keep-me
@@ -136,6 +138,11 @@
describe-top-level-key
display-name
suggested-name]
+ [lib.native
+ #?@(:cljs [->TemplateTags
+ TemplateTags->])
+ recognize-template-tags
+ template-tags]
[lib.order-by
order-by
order-by-clause
diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs
index 4c1c08312789e0ce73f696931f2f1183698d1953..9014717d41022f6285249b14bf27e690f09ad021 100644
--- a/src/metabase/lib/js.cljs
+++ b/src/metabase/lib/js.cljs
@@ -13,6 +13,38 @@
;;; this is mostly to ensure all the relevant namespaces with multimethods impls get loaded.
(comment lib.core/keep-me)
+;; TODO: This pattern of "re-export some function and slap a `clj->js` at the end" is going to keep appearing.
+;; Generalize the machinery in `metabase.domain-entities.malli` to handle this case, so we get schema-powered automatic
+;; conversion for incoming args and outgoing return values. I'm imagining something like
+;; `(mu/js-export lib.core/recognize-template-tags)` where that function has a Malli schema and it works like
+;; `metabase.shared.util.namespaces/import-fn` plus wrapping it with conversion for all args and the return value.
+(defn ^:export recognize-template-tags
+ "Given the text of a native query, extract a possibly-empty set of template tag strings from it.
+
+ These looks like mustache templates. For variables, we only allow alphanumeric characters, eg. `{{foo}}`.
+ For snippets they start with `snippet:`, eg. `{{ snippet: arbitrary text here }}`.
+ And for card references either `{{ #123 }}` or with the optional human label `{{ #123-card-title-slug }}`.
+
+ Invalid patterns are simply ignored, so something like `{{&foo!}}` is just disregarded."
+ [query-text]
+ (-> query-text
+ lib.core/recognize-template-tags
+ clj->js))
+
+(defn ^:export template-tags
+ "Extract the template tags from a native query's text.
+
+ If the optional map of existing tags previously parsed is given, this will reuse the existing tags where
+ they match up with the new one (in particular, it will preserve the UUIDs).
+
+ See [[recognize-template-tags]] for how the tags are parsed."
+ ([query-text] (template-tags query-text {}))
+ ([query-text existing-tags]
+ (->> existing-tags
+ lib.core/->TemplateTags
+ (lib.core/template-tags query-text)
+ lib.core/TemplateTags->)))
+
(defn ^:export suggestedName
"Return a nice description of a query."
[query]
diff --git a/src/metabase/lib/native.cljc b/src/metabase/lib/native.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..dd7e486ea3c30ab5cbc25cfcfa5738adb45fb620
--- /dev/null
+++ b/src/metabase/lib/native.cljc
@@ -0,0 +1,125 @@
+(ns metabase.lib.native
+ "Functions for working with native queries."
+ (:require
+ [clojure.set :as set]
+ [clojure.string :as str]
+ [medley.core :as m]
+ [metabase.lib.schema.common :as common]
+ [metabase.util.humanization :as u.humanization]
+ [metabase.util.malli :as mu]
+ #?@(:cljs ([metabase.domain-entities.converters :as converters]))))
+
+(def ^:private TemplateTag
+ [:map
+ [:type [:enum :text :snippet :card]]
+ [:id :uuid]
+ [:name ::common/non-blank-string]
+ [:display-name {:js/prop "display-name" :optional true} ::common/non-blank-string]
+ [:snippet-name {:js/prop "snippet-name" :optional true} ::common/non-blank-string]
+ [:card-id {:js/prop "card-id" :optional true} :int]
+ [:dimension {:optional true} :any]
+ [:widget-type {:js/prop "widget-type" :optional true} :string]])
+
+(def ^:private TemplateTags
+ [:map-of :string TemplateTag])
+
+(def ^:private variable-tag-regex
+ #"\{\{\s*([A-Za-z0-9_\.]+)\s*\}\}")
+
+(def ^:private snippet-tag-regex
+ #"\{\{\s*(snippet:\s*[^}]+)\s*\}\}")
+
+(def ^:private card-tag-regex
+ #"\{\{\s*(#([0-9]*)(-[a-z0-9-]*)?)\s*\}\}")
+
+(def ^:private tag-regexes
+ [variable-tag-regex snippet-tag-regex card-tag-regex])
+
+(mu/defn recognize-template-tags :- [:set ::common/non-blank-string]
+ "Given the text of a native query, extract a possibly-empty set of template tag strings from it."
+ [query-text :- ::common/non-blank-string]
+ (into #{}
+ (comp (mapcat #(re-seq % query-text))
+ (map second))
+ tag-regexes))
+
+(defn- tag-name->card-id [tag-name]
+ (when-let [[_ id-str] (re-matches #"^#(\d+)(-[a-z0-9-]*)?$" tag-name)]
+ (parse-long id-str)))
+
+(defn- tag-name->snippet-name [tag-name]
+ (when (str/starts-with? tag-name "snippet:")
+ (str/trim (subs tag-name (count "snippet:")))))
+
+(defn- fresh-tag [tag-name]
+ {:type :text
+ :name tag-name
+ :id (m/random-uuid)})
+
+(defn- finish-tag [{tag-name :name :as tag}]
+ (merge tag
+ (when-let [card-id (tag-name->card-id tag-name)]
+ {:type :card
+ :card-id card-id})
+ (when-let [snippet-name (tag-name->snippet-name tag-name)]
+ {:type :snippet
+ :snippet-name snippet-name})
+ (when-not (:display-name tag)
+ {:display-name (u.humanization/name->human-readable-name :simple tag-name)})))
+
+(defn- rename-template-tag
+ [existing-tags old-name new-name]
+ (let [old-tag (get existing-tags old-name)
+ display-name (if (= (:display-name old-tag)
+ (u.humanization/name->human-readable-name :simple old-name))
+ ;; Replace the display name if it was the default; keep it if customized.
+ (u.humanization/name->human-readable-name :simple new-name)
+ (:display-name old-tag))
+ new-tag (-> old-tag
+ (dissoc :snippet-name :card-id)
+ (assoc :display-name display-name
+ :name new-name))]
+ (-> existing-tags
+ (dissoc old-name)
+ (assoc new-name new-tag))))
+
+(defn- unify-template-tags
+ [query-tag-names existing-tags existing-tag-names]
+ (let [new-tags (set/difference query-tag-names existing-tag-names)
+ old-tags (set/difference existing-tag-names query-tag-names)
+ tags (if (= 1 (count new-tags) (count old-tags))
+ ;; With exactly one change, we treat it as a rename.
+ (rename-template-tag existing-tags (first old-tags) (first new-tags))
+ ;; With more than one change, just drop the old ones and add the new.
+ (merge (m/remove-keys old-tags existing-tags)
+ (m/index-by :name (map fresh-tag new-tags))))]
+ (update-vals tags finish-tag)))
+
+(mu/defn ^:export template-tags :- TemplateTags
+ "Extract the template tags from a native query's text.
+
+ If the optional map of existing tags previously parsed is given, this will reuse the existing tags where
+ they match up with the new one (in particular, it will preserve the UUIDs).
+
+ See [[recognize-template-tags]] for how the tags are parsed."
+ ([query-text :- ::common/non-blank-string]
+ (template-tags query-text nil))
+ ([query-text :- ::common/non-blank-string
+ existing-tags :- [:maybe TemplateTags]]
+ (let [query-tag-names (not-empty (recognize-template-tags query-text))
+ existing-tag-names (not-empty (set (keys existing-tags)))]
+ (if (or query-tag-names existing-tag-names)
+ ;; If there's at least some tags, unify them.
+ (unify-template-tags query-tag-names existing-tags existing-tag-names)
+ ;; Otherwise just an empty map, no tags.
+ {}))))
+
+#?(:cljs
+ (do
+ (def ->TemplateTags
+ "Converter to a map of `TemplateTag`s keyed by their string names."
+ (converters/incoming TemplateTags))
+
+ (def TemplateTags->
+ "Converter from a map of `TemplateTag`s keyed by their string names to vanilla JS."
+ (converters/outgoing TemplateTags))))
diff --git a/test/metabase/domain_entities/converters_test.cljs b/test/metabase/domain_entities/converters_test.cljs
index aa115f27d86c444d1670a668e03a81d409a8218f..af6c890bb36a8c558f03cb794623cc52cad2fcf8 100644
--- a/test/metabase/domain_entities/converters_test.cljs
+++ b/test/metabase/domain_entities/converters_test.cljs
@@ -1,7 +1,8 @@
(ns metabase.domain-entities.converters-test
(:require
- [clojure.test :refer [deftest is testing]]
- [metabase.domain-entities.converters :as converters]))
+ [clojure.test :refer [deftest is testing]]
+ [metabase.domain-entities.converters :as converters]
+ [metabase.test.util.js :as test.js]))
(deftest incoming-basics-test
(testing "simple values are not transformed"
@@ -37,7 +38,6 @@
(is (= kw (-> kw kw-> ->kw)))
(is (= s (-> s ->kw kw->)))))))
-
(def HalfDeclared
[:map
[:declared-camel {:js/prop "declaredCamel"} string?]
@@ -47,29 +47,6 @@
(def ->half-declared
(converters/incoming HalfDeclared))
-(defmulti js= (fn [a b]
- (let [ta (type a)
- tb (type b)]
- (if (= ta tb)
- (.-name ta)
- ::mismatched))))
-
-(defmethod js= :default [a b]
- (= a b))
-
-(defmethod js= ::mismatched [_ _]
- false)
-
-(defmethod js= "Array" [a b]
- (and (= (count a) (count b))
- (empty? (filter false? (map js= a b)))))
-
-(defmethod js= "Object" [a b]
- (and (every? (fn [k] (and (.hasOwnProperty b k) (js= (unchecked-get a k) (unchecked-get b k))))
- (js/Object.keys a))
- (every? (fn [k] (and (.hasOwnProperty a k) (js= (unchecked-get a k) (unchecked-get b k))))
- (js/Object.keys b))))
-
(deftest map-basics-test
(testing "incoming maps"
(testing "become CLJS maps"
@@ -125,13 +102,13 @@
(testing "are converted per the schema by :js/prop; defaulting to snake_case"
(let [adjusted (assoc obj :declared-camel "no")]
(is (not (identical? obj adjusted)))
- (is (js= #js {"declaredCamel" "no"
- "declared_snake" "also"
- "declared-kebab" "finally"
- "undeclared_camel" 7
- "undeclared_snake" 8
- "undeclared_kebab" 9}
- ((converters/outgoing HalfDeclared) adjusted))))))))
+ (is (test.js/= #js {"declaredCamel" "no"
+ "declared_snake" "also"
+ "declared-kebab" "finally"
+ "undeclared_camel" 7
+ "undeclared_snake" 8
+ "undeclared_kebab" 9}
+ ((converters/outgoing HalfDeclared) adjusted))))))))
(def Child
[:map [:inner-value {:js/prop "innerValue"} string?]])
@@ -148,7 +125,7 @@
exp-clj {:parent {:child {:inner-value "asdf"}}}
converted ((converters/incoming Grandparent) input)]
(is (= exp-clj converted))
- (is (js= input ((converters/outgoing Grandparent) converted)))))
+ (is (test.js/= input ((converters/outgoing Grandparent) converted)))))
(testing "nesting kitchen sink"
(let [schema [:map
@@ -208,7 +185,7 @@
(is (map? converted))
(is (= exp converted)))
(testing "round-trips as expected"
- (is (js= input (-> input ->sink sink->)))))))
+ (is (test.js/= input (-> input ->sink sink->)))))))
(deftest idempotency-test
(testing "CLJS maps are not further converted"
@@ -236,3 +213,27 @@
(is (identical? js-obj (-> converted :wrapper :inner)))
(is (identical? js-obj (let [^Object wrapper (.-wrapper returned)]
(.-inner wrapper)))))))
+
+(deftest uuid-test
+ (testing "UUIDs are converted to strings in JS and back to #uuid objects in CLJS"
+ (let [uuid (random-uuid)]
+ (is (= (str uuid)
+ ((converters/outgoing :uuid) uuid)))
+ (is (= uuid
+ ((converters/incoming :uuid) (str uuid))))))
+
+ (testing "UUIDs nested in maps work too"
+ (let [uuid (random-uuid)
+ schema [:map [:id :uuid]]]
+ (is (test.js/= #js {:id (str uuid)}
+ ((converters/outgoing schema) {:id uuid})))
+ (is (= {:id uuid}
+ ((converters/incoming schema) #js {:id (str uuid)})))))
+
+ (testing "UUIDs nested in maps inside a map-of work too"
+ (let [uuid (random-uuid)
+ schema [:map-of :string [:map [:id :uuid]]]]
+ (is (test.js/= #js{"abc" #js {:id (str uuid)}}
+ ((converters/outgoing schema) {"abc" {:id uuid}})))
+ (is (= {"abc" {:id uuid}}
+ ((converters/incoming schema) #js {"abc" #js {:id (str uuid)}}))))))
diff --git a/test/metabase/lib/native_test.cljc b/test/metabase/lib/native_test.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..f9da568fd8a1050dcfa364aa5a84ae579ee504af
--- /dev/null
+++ b/test/metabase/lib/native_test.cljc
@@ -0,0 +1,163 @@
+(ns metabase.lib.native-test
+ (:require
+ [clojure.test :refer [are deftest is testing]]
+ [metabase.lib.native :as lib.native]
+ #?@(:clj ([medley.core :as m]
+ [metabase.util.humanization :as u.humanization])
+ :cljs ([metabase.test.util.js :as test.js]))))
+
+(deftest ^:parallel variable-tag-test
+ (are [exp input] (= exp (lib.native/recognize-template-tags input))
+ #{"foo"} "SELECT * FROM table WHERE {{foo}} AND some_field IS NOT NULL"
+ #{"foo" "bar"} "SELECT * FROM table WHERE {{foo}} AND some_field = {{bar}}"
+ ;; Duplicates are flattened.
+ #{"foo" "bar"} "SELECT * FROM table WHERE {{foo}} AND some_field = {{bar }} OR {{ foo}}"
+ ;; Ignoring non-alphanumeric vars
+ #{} "SELECT * FROM table WHERE {{&foo}}"))
+
+(deftest ^:parallel snippet-tag-test
+ (are [exp input] (= exp (lib.native/recognize-template-tags input))
+ #{"snippet: foo "} "SELECT * FROM table WHERE {{snippet: foo }} AND some_field IS NOT NULL"
+ #{"snippet: foo *#&@"} "SELECT * FROM table WHERE {{snippet: foo *#&@}}"
+ ;; TODO: This logic should trim the whitespace and unify these two snippet names.
+ ;; I think this is a bug in the original code but am aiming to reproduce it exactly for now.
+ #{"snippet: foo" "snippet:foo"} "SELECT * FROM table WHERE {{snippet: foo}} AND {{snippet:foo}}"))
+
+(deftest ^:parallel card-tag-test
+ (are [exp input] (= exp (lib.native/recognize-template-tags input))
+ #{"#123"} "SELECT * FROM table WHERE {{ #123 }} AND some_field IS NOT NULL"
+ ;; TODO: This logic should trim the whitespace and unify these two card tags.
+ ;; I think this is a bug in the original code but am aiming to reproduce it exactly for now.
+ #{"#123" "#123-with-slug"} "SELECT * FROM table WHERE {{ #123 }} AND {{ #123-with-slug }}"
+ #{"#123"} "SELECT * FROM table WHERE {{ #not-this }} AND {{#123}}"
+ #{} "{{ #123foo }}"))
+
+#?(:clj
+ ;; TODO: This is only CLJ-only because =? from Hawk is not available in CLJS currently.
+ ;; I don't think there's any reason why it can't work there too.
+ (deftest ^:parallel template-tags-test
+ (testing "snippet tags"
+ (is (=? {"snippet:foo" {:type :snippet
+ :name "snippet:foo"
+ :snippet-name "foo"
+ :id uuid?}}
+ (lib.native/template-tags "SELECT * FROM table WHERE {{snippet:foo}}")))
+ (is (=? {"snippet:foo" {:type :snippet
+ :name "snippet:foo"
+ :snippet-name "foo"
+ :id uuid?}
+ "snippet: foo" {:type :snippet
+ :name "snippet: foo"
+ :snippet-name "foo"
+ :id uuid?}}
+ ;; TODO: This should probably be considered a bug - whitespace matters for the name.
+ (lib.native/template-tags "SELECT * FROM {{snippet: foo}} WHERE {{snippet:foo}}"))))
+
+ (testing "renaming a variable"
+ (let [old-tag {:type :text
+ :name "foo"
+ :display-name "Foo"
+ :id (m/random-uuid)}]
+ (testing "changes display-name if the original is not customized"
+ (is (=? {"bar" {:type :text
+ :name "bar"
+ :display-name "Bar"
+ :id (:id old-tag)}}
+ (lib.native/template-tags "SELECT * FROM {{bar}}"
+ {"foo" old-tag}))))
+ (testing "keeps display-name if it's customized"
+ (is (=? {"bar" {:type :text
+ :name "bar"
+ :display-name "Custom Name"
+ :id (:id old-tag)}}
+ (lib.native/template-tags "SELECT * FROM {{bar}}"
+ {"foo" (assoc old-tag :display-name "Custom Name")}))))
+
+ (testing "works with other variables present, if they don't change"
+ (let [other {:type :text
+ :name "other"
+ :display-name "Some Var"
+ :id (m/random-uuid)}]
+ (is (=? {"other" other
+ "bar" {:type :text
+ :name "bar"
+ :display-name "Bar"
+ :id (:id old-tag)}}
+ (lib.native/template-tags "SELECT * FROM {{bar}} AND field = {{other}}"
+ {"foo" old-tag
+ "other" other})))))))
+
+ (testing "general case, add and remove"
+ (let [mktag (fn [base]
+ (merge {:type :text
+ :display-name (u.humanization/name->human-readable-name :simple (:name base))
+ :id uuid?}
+ base))
+ v1 (mktag {:name "foo"})
+ v2 (mktag {:name "bar"})
+ v3 (mktag {:name "baz"})
+ s1 (mktag {:name "snippet:first snippet"
+ :snippet-name "first snippet"
+ :type :snippet})
+ s2 (mktag {:name "snippet:another snippet"
+ :snippet-name "another snippet"
+ :type :snippet})
+
+ c1 (mktag {:name "#123-card-1"
+ :type :card
+ :card-id 123})
+ c2 (mktag {:name "#321"
+ :type :card
+ :card-id 321})]
+ (is (=? {"foo" v1
+ "#123-card-1" c1
+ "snippet:first snippet" s1}
+ (lib.native/template-tags
+ "SELECT * FROM {{#123-card-1}} WHERE {{foo}} AND {{ snippet:first snippet}}")))
+ (is (=? {"bar" v2
+ "baz" v3
+ "snippet:another snippet" s2
+ "#321" c2}
+ (lib.native/template-tags
+ "SELECT * FROM {{#321}} WHERE {{baz}} AND {{bar}} AND {{snippet:another snippet}}"
+ {"foo" (assoc v1 :id (random-uuid))
+ "#123-card-1" (assoc c1 :id (random-uuid))
+ "snippet:first snippet" (assoc s1 :id (random-uuid))})))))))
+
+#?(:cljs
+ (deftest converters-test
+ (let [clj-tags {"a" {:id #uuid "c5ad010c-632a-4498-b667-9188fbe965f9"
+ :name "a"
+ :display-name "A"
+ :type :text}
+ "#123-foo" {:id #uuid "7e58e086-5d63-4986-8fe7-87e05dfa4089"
+ :name "#123-foo"
+ :display-name "#123-foo"
+ :type :card
+ :card-id 123}
+ "snippet:b" {:id #uuid "604131d0-a74c-4822-b113-8e9515b1a985"
+ :name "snippet:b"
+ :display-name "Snippet B"
+ :type :snippet
+ :snippet-name "b"}}
+ js-tags #js {"a" #js {"id" "c5ad010c-632a-4498-b667-9188fbe965f9"
+ "name" "a"
+ "display-name" "A"
+ "type" "text"}
+ "#123-foo" #js {"id" "7e58e086-5d63-4986-8fe7-87e05dfa4089"
+ "name" "#123-foo"
+ "display-name" "#123-foo"
+ "type" "card"
+ "card-id" 123}
+ "snippet:b" #js {"id" "604131d0-a74c-4822-b113-8e9515b1a985"
+ "name" "snippet:b"
+ "display-name" "Snippet B"
+ "type" "snippet"
+ "snippet-name" "b"}}]
+ (testing "incoming converter works"
+ (is (= clj-tags (#'lib.native/->TemplateTags js-tags))))
+ (testing "outgoing converter works"
+ (is (test.js/= js-tags (#'lib.native/TemplateTags-> clj-tags))))
+ (testing "round trips work"
+ (is (= clj-tags (-> clj-tags (#'lib.native/TemplateTags->) (#'lib.native/->TemplateTags))))
+ (is (test.js/= js-tags (-> js-tags (#'lib.native/->TemplateTags) (#'lib.native/TemplateTags->))))))))
diff --git a/test/metabase/test/util/js.cljs b/test/metabase/test/util/js.cljs
new file mode 100644
index 0000000000000000000000000000000000000000..ff5b0a8db9e55f62a19a4fc9ee94c1bc6d0672a6
--- /dev/null
+++ b/test/metabase/test/util/js.cljs
@@ -0,0 +1,26 @@
+(ns metabase.test.util.js
+ "Test utils for CLJS."
+ (:refer-clojure :exclude [=]))
+
+(defmulti = (fn [a b]
+ (let [ta (type a)
+ tb (type b)]
+ (if (clojure.core/= ta tb)
+ (.-name ta)
+ ::mismatched))))
+
+(defmethod = :default [a b]
+ (clojure.core/= a b))
+
+(defmethod = ::mismatched [_ _]
+ false)
+
+(defmethod = "Array" [a b]
+ (and (clojure.core/= (count a) (count b))
+ (empty? (filter false? (map = a b)))))
+
+(defmethod = "Object" [a b]
+ (and (every? (fn [k] (and (.hasOwnProperty b k) (= (unchecked-get a k) (unchecked-get b k))))
+ (js/Object.keys a))
+ (every? (fn [k] (and (.hasOwnProperty a k) (= (unchecked-get a k) (unchecked-get b k))))
+ (js/Object.keys b))))