[MLv2] Move template tag extraction into CLJC for use from BE (#29526)

This supports both parsing out the tags and constructing the template tags map, with `card-id` and all.

This is not quite a FE no-op because the humanization of text is slightly different between `metabase.util.humanization` and the JS humanization library.

This improves JS<->CLJS conversion to handle UUIDs and be smarter about enums, both of which were needed to handle converting template tag maps.

Extracted a `js=` test helper to `metabase.test.util.js/=` since many tests need to compare JS objects, which are not JS `==` to each other.

.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

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();
           });
 

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");
     });
   },
 );

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[] {

frontend/src/metabase-lib/queries/TemplateTag.ts

-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",
-  };
-};

frontend/src/metabase-lib/queries/TemplateTag.unit.spec.ts

-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");
-  });
-});

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);
-    });
-  });
 });

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."

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

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]

src/metabase/lib/native.cljc

+(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))))

test/metabase/domain_entities/converters_test.cljs

 (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)}}))))))

test/metabase/lib/native_test.cljc

+(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->))))))))

test/metabase/test/util/js.cljs

+(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))))