Static embed dashboard and question endpoints (#47270)

* entity id translation + tests

* add api level test

* simplify definition of eid-table->model + add test

* update tests to take keywords

* improve comment

* generate the eid-table->model map

* delete now-obsolete test

* make it work in oss

* put the resulting response into a key, so we can add more information later

* formatting

* use model names without the model/ prefix as keys

* Creates list of `api/model->db-model`

- update keys for util/entity_id request
- update shape of util/entity_id response
- add test for not-found eids

* formatting

* Respond to code review feedback

* entity-ids work on /embed/{card,dashboard}* routes

* make entity id translation work for all tokens

* Merge branch 'static-embed-dashboard-and-question-endpoints' of github.com:metabase/metabase into static-embed-dashboard-and-question-endpoints

* massage schema. add tons of tests

* decode more eids

* test alignment

* don't require ns inside of itself :think:

* make the test functions to create tokens better

* bring back `dashboard-url`

---------

Co-authored-by: Oisin Coveney <oisin@metabase.com>

src/metabase/api/embed.clj

@@ -29,11 +29,39 @@
    [metabase.query-processor.pivot :as qp.pivot]
    [metabase.util :as u]
    [metabase.util.embed :as embed]
+   [metabase.util.malli :as mu]
    [metabase.util.malli.schema :as ms]
    [toucan2.core :as t2]))
 
 (set! *warn-on-reflection* true)
 
+(def ^:private ResourceId [:or ms/PositiveInt ms/NanoIdString])
+(def ^:private Token [:map
+                      [:resource [:map
+                                  [:question  {:optional true} ResourceId]
+                                  [:dashboard {:optional true} ResourceId]]]
+                      [:params :any]])
+
+(defn- conditional-update-in
+  "If there's a value at `path`, apply `f`, otherwise return `m`."
+  [m path f]
+  (if-let [value (get-in m path)]
+    (assoc-in m path (f value))
+    m))
+
+(mu/defn translate-token-ids :- Token
+  "Translate `entity_id` keys to `card_id` and `dashboard_id` respectively."
+  [unsigned :- Token]
+  (-> unsigned
+      (conditional-update-in [:resource :question]  #(api.embed.common/->id :model/Card %))
+      (conditional-update-in [:resource :dashboard] #(api.embed.common/->id :model/Dashboard %))))
+
+(defn unsign-and-translate-ids
+  "Unsign a JWT and translate `entity_id` keys to `card_id` and `dashboard_id` respectively. If they are already
+   sequential ids, they are left as is."
+  [message]
+  (translate-token-ids (embed/unsign message)))
+
 ;;; ------------------------------------------- /api/embed/card endpoints --------------------------------------------
 
 (api/defendpoint GET "/card/:token"
@@ -43,7 +71,7 @@
 
      {:resource {:question <card-id>}}"
   [token]
-  (let [unsigned (embed/unsign token)]
+  (let [unsigned (unsign-and-translate-ids token)]
     (api.embed.common/check-embedding-enabled-for-card (embed/get-in-unsigned-token-or-throw unsigned [:resource :question]))
     (u/prog1 (api.embed.common/card-for-unsigned-token unsigned, :constraints [:enable_embedding true])
       (events/publish-event! :event/card-read {:object-id (:id <>), :user-id api/*current-user-id*, :context :question}))))
@@ -75,7 +103,7 @@
      {:resource {:question <card-id>}
       :params   <parameters>}"
   [token & query-params]
-  (run-query-for-unsigned-token-async (embed/unsign token) :api (api.embed.common/parse-query-params query-params)))
+  (run-query-for-unsigned-token-async (unsign-and-translate-ids token) :api (api.embed.common/parse-query-params query-params)))
 
 (api/defendpoint GET ["/card/:token/query/:export-format", :export-format api.dataset/export-format-regex]
   "Like `GET /api/embed/card/query`, but returns the results as a file in the specified format."
@@ -83,7 +111,7 @@
   {export-format (into [:enum] api.dataset/export-formats)
    format_rows   [:maybe :boolean]}
   (run-query-for-unsigned-token-async
-   (embed/unsign token)
+   (unsign-and-translate-ids token)
    export-format
    (api.embed.common/parse-query-params (dissoc (m/map-keys keyword query-params) :format_rows))
    :constraints nil
@@ -100,7 +128,7 @@
 
      {:resource {:dashboard <dashboard-id>}}"
   [token]
-  (let [unsigned (embed/unsign token)]
+  (let [unsigned (unsign-and-translate-ids token)]
     (api.embed.common/check-embedding-enabled-for-dashboard (embed/get-in-unsigned-token-or-throw unsigned [:resource :dashboard]))
     (u/prog1 (api.embed.common/dashboard-for-unsigned-token unsigned, :constraints [:enable_embedding true])
       (events/publish-event! :event/dashboard-read {:object-id (:id <>), :user-id api/*current-user-id*}))))
@@ -109,7 +137,7 @@
   "Fetch the results of running a Card belonging to a Dashboard using a JSON Web Token signed with the
    `embedding-secret-key`.
 
-   Token should have the following format:
+   [[Token]] should have the following format:
 
      {:resource {:dashboard <dashboard-id>}
       :params   <parameters>}
@@ -121,7 +149,7 @@
    & {:keys [constraints qp middleware]
       :or   {constraints (qp.constraints/default-query-constraints)
              qp          qp.card/process-query-for-card-default-qp}}]
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         dashboard-id   (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])]
     (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id)
     (api.embed.common/process-query-for-dashcard
@@ -156,7 +184,7 @@
   "Fetch FieldValues for a Field that is referenced by an embedded Card."
   [token field-id]
   {field-id ms/PositiveInt}
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         card-id        (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])]
     (api.embed.common/check-embedding-enabled-for-card card-id)
     (api.public/card-and-field-id->values card-id field-id)))
@@ -165,7 +193,7 @@
   "Fetch FieldValues for a Field that is used as a param in an embedded Dashboard."
   [token field-id]
   {field-id ms/PositiveInt}
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         dashboard-id   (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])]
     (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id)
     (api.public/dashboard-and-field-id->values dashboard-id field-id)))
@@ -179,7 +207,7 @@
    search-field-id ms/PositiveInt
    value           ms/NonBlankString
    limit           [:maybe ms/PositiveInt]}
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         card-id        (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])]
     (api.embed.common/check-embedding-enabled-for-card card-id)
     (api.public/search-card-fields card-id field-id search-field-id value (when limit (Integer/parseInt limit)))))
@@ -191,7 +219,7 @@
    search-field-id ms/PositiveInt
    value           ms/NonBlankString
    limit           [:maybe ms/PositiveInt]}
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         dashboard-id   (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])]
     (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id)
     (api.public/search-dashboard-fields dashboard-id field-id search-field-id value (when limit
@@ -206,7 +234,7 @@
   {field-id    ms/PositiveInt
    remapped-id ms/PositiveInt
    value       ms/NonBlankString}
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         card-id        (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])]
     (api.embed.common/check-embedding-enabled-for-card card-id)
     (api.public/card-field-remapped-values card-id field-id remapped-id value)))
@@ -218,7 +246,7 @@
   {field-id    ms/PositiveInt
    remapped-id ms/PositiveInt
    value       ms/NonBlankString}
-  (let [unsigned-token (embed/unsign token)
+  (let [unsigned-token (unsign-and-translate-ids token)
         dashboard-id   (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])]
     (api.embed.common/check-embedding-enabled-for-dashboard dashboard-id)
     (api.public/dashboard-field-remapped-values dashboard-id field-id remapped-id value)))
@@ -265,7 +293,7 @@
 (api/defendpoint GET "/card/:token/params/:param-key/values"
   "Embedded version of api.card filter values endpoint."
   [token param-key]
-  (let [unsigned (embed/unsign token)
+  (let [unsigned (unsign-and-translate-ids token)
         card-id  (embed/get-in-unsigned-token-or-throw unsigned [:resource :question])
         card     (t2/select-one Card :id card-id)]
     (api.embed.common/check-embedding-enabled-for-card card-id)
@@ -276,7 +304,7 @@
 (api/defendpoint GET "/card/:token/params/:param-key/search/:prefix"
   "Embedded version of chain filter search endpoint."
   [token param-key prefix]
-  (let [unsigned (embed/unsign token)
+  (let [unsigned (unsign-and-translate-ids token)
         card-id  (embed/get-in-unsigned-token-or-throw unsigned [:resource :question])
         card     (t2/select-one Card :id card-id)]
     (api.embed.common/check-embedding-enabled-for-card card-id)
@@ -293,7 +321,7 @@
      {:resource {:question <card-id>}
       :params   <parameters>}"
   [token & query-params]
-  (run-query-for-unsigned-token-async (embed/unsign token)
+  (run-query-for-unsigned-token-async (unsign-and-translate-ids token)
                                       :api (api.embed.common/parse-query-params query-params)
                                       :qp qp.pivot/run-pivot-query))
 

src/metabase/api/embed/common.clj

@@ -274,6 +274,99 @@
       (assoc (select-keys param [:type :target :slug])
              :value value))))
 
+;;; -------------------------------------- Entity ID transformation functions ------------------------------------------
+
+(def ^:private api-models
+  "The models that we will service for entity-id transformations."
+  (->> (descendants :metabase/model)
+       (filter #(= (namespace %) "model"))
+       (filter (fn has-entity-id?
+                 [model] (or ;; toucan1 models
+                          (isa? model :metabase.models.interface/entity-id)
+                          ;; toucan2 models
+                          (isa? model :hook/entity-id))))
+       (map keyword)
+       set))
+
+(def ^:private api-name->model
+  "Map of model names used on the API to their corresponding model."
+  (->> api/model->db-model
+       (map (fn [[k v]] [(keyword k) (:db-model v)]))
+       (filter (fn [[_ v]] (contains? api-models v)))
+       (into {})))
+
+(defn- ->model
+  "Takes a model keyword or an api-name and returns the corresponding model keyword."
+  [model-or-api-name]
+  (if (contains? api-models model-or-api-name)
+    model-or-api-name
+    (api-name->model model-or-api-name)))
+
+(def ^:private eid-api-models
+  "Sorted vec of api models that have an entity_id column"
+  (vec (sort (keys api-name->model))))
+
+(def ^:private ApiModel (into [:enum] eid-api-models))
+
+(def ^:private EntityId
+  "A Malli schema for an entity id, this is a little looser because it needs to be fast."
+  [:and {:description "entity_id"}
+   :string
+   [:fn {:error/fn (fn [{:keys [value]} _]
+                     (str "\"" value "\" should be 21 characters long, but it is " (count value)))}
+    (fn eid-length-good? [eid] (= 21 (count eid)))]])
+
+(def ^:private ModelToEntityIds
+  "A Malli schema for a map of model names to a sequence of entity ids."
+  (mc/schema [:map-of ApiModel [:sequential :string]]))
+
+(mu/defn- entity-ids->id-for-model
+  "Given a model and a sequence of entity ids on that model, return a pairs of entity-id, id."
+  [api-name eids]
+  (let [model (->model api-name) ;; This lookup is safe because we've already validated the api-names
+        eid->id (into {} (t2/select-fn->fn :entity_id :id [model :id :entity_id] :entity_id [:in eids]))]
+    (mapv (fn entity-id-info [entity-id]
+            [entity-id (if-let [id (get eid->id entity-id)]
+                         {:id id :type api-name :status "ok"}
+                         ;; handle errors
+                         (if (mc/validate EntityId entity-id)
+                           {:type api-name
+                            :status "not-found"}
+                           {:type api-name
+                            :status "invalid-format"
+                            :reason (me/humanize (mc/explain EntityId entity-id))}))])
+          eids)))
+
+(defn model->entity-ids->ids
+  "Given a map of model names to a sequence of entity-ids for each, return a map from entity-id -> id."
+  [model-key->entity-ids]
+  (when-not (mc/validate ModelToEntityIds model-key->entity-ids)
+    (throw (ex-info "Invalid format." {:explanation (me/humanize
+                                                     (me/with-spell-checking
+                                                       (mc/explain ModelToEntityIds model-key->entity-ids)))
+                                       :allowed-models (sort (keys api-name->model))
+                                       :status-code 400})))
+  (into {}
+        (mapcat
+         (fn [[model eids]] (entity-ids->id-for-model model eids))
+         model-key->entity-ids)))
+
+(mu/defn ->id :- :int
+  "Translates a single entity_id -> id. This reuses the batched version: [[model->entity-ids->ids]].
+   Please use that if you have to do man lookups at once."
+  [pre-model id :- [:or :int :string]]
+  (if (string? id)
+    (let [model (->model pre-model)
+          [[_ {:keys [status] :as info}]] (entity-ids->id-for-model model [id])]
+      (if-not (= "ok" status)
+        (throw (ex-info "problem looking up id from entity_id"
+                        {:pre-model pre-model
+                         :model model
+                         :id id
+                         :status status}))
+        (:id info)))
+    id))
+
 ;;; ---------------------------- Card Fns used by both /api/embed and /api/preview_embed -----------------------------
 
 (defn card-for-unsigned-token
@@ -281,7 +374,8 @@
   `public-card` function that fetches the Card."
   [unsigned-token & {:keys [embedding-params constraints]}]
   {:pre [((some-fn empty? sequential?) constraints) (even? (count constraints))]}
-  (let [card-id      (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])
+  (let [pre-card-id  (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :question])
+        card-id      (->id :model/Card pre-card-id)
         token-params (embed/get-in-unsigned-token-or-throw unsigned-token [:params])]
     (-> (apply api.public/public-card :id card-id, constraints)
         api.public/combine-parameters-and-template-tags
@@ -350,7 +444,8 @@
   the `public-dashboard` function that fetches the Dashboard."
   [unsigned-token & {:keys [embedding-params constraints]}]
   {:pre [((some-fn empty? sequential?) constraints) (even? (count constraints))]}
-  (let [dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])
+  (let [pre-dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])
+        dashboard-id (->id :model/Dashboard pre-dashboard-id)
         embedding-params (or embedding-params
                              (t2/select-one-fn :embedding_params :model/Dashboard, :id dashboard-id))
         token-params (embed/get-in-unsigned-token-or-throw unsigned-token [:params])]
@@ -443,7 +538,8 @@
   [token searched-param-id prefix id-query-params
    & {:keys [preview] :or {preview false}}]
   (let [unsigned-token                                 (embed/unsign token)
-        dashboard-id                                   (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])
+        pre-dashboard-id                               (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])
+        dashboard-id                                   (->id :model/Dashboard pre-dashboard-id)
         _                                              (when-not preview (check-embedding-enabled-for-dashboard dashboard-id))
         slug-token-params                              (embed/get-in-unsigned-token-or-throw unsigned-token [:params])
         {parameters                 :parameters
@@ -490,67 +586,3 @@
                          e)]
           (log/errorf e "Chain filter error\n%s" (u/pprint-to-str (u/all-ex-data e)))
           (throw e))))))
-
-;;; -------------------------------------- Entity ID transformation functions ------------------------------------------
-
-(def ^:private api-models
-  "The models that we will service for entity-id transformations."
-  (->> (descendants :metabase/model)
-       (filter #(= (namespace %) "model"))
-       (filter (fn has-entity-id?
-                 [model] (or ;; toucan1 models
-                          (isa? model :metabase.models.interface/entity-id)
-                          ;; toucan2 models
-                          (isa? model :hook/entity-id))))
-       (map keyword)
-       set))
-
-(def ^:private api-name->model
-  "Map of model names used on the API to their corresponding model."
-  (->> api/model->db-model
-       (map (fn [[k v]] [(keyword k) (:db-model v)]))
-       (filter (fn [[_ v]] (contains? api-models v)))
-       (into {})))
-
-(def ^:private eid-api-models
-  "Sorted vec of api models that have an entity_id column"
-  (vec (sort (keys api-name->model))))
-
-(def ^:private ApiModel (into [:enum] eid-api-models))
-
-(def ^:private EntityId
-  "A Malli schema for an entity id, this is a little looser because it needs to be fast."
-  [:and {:description "entity_id"}
-   :string
-   [:fn {:error/fn (fn [{:keys [value]} _]
-                     (str "\"" value "\" should be 21 characters long, but it is " (count value)))}
-    (fn eid-length-good? [eid] (= 21 (count eid)))]])
-
-(def ^:private ModelToEntityIds
-  "A Malli schema for a map of model names to a sequence of entity ids."
-  (mc/schema [:map-of ApiModel [:sequential EntityId]]))
-
-(mu/defn- entity-ids->id-for-model
-  "Given a model and a sequence of entity ids on that model, return a pairs of entity-id, id."
-  [api-name eids]
-  (let [model (api-name->model api-name) ;; This lookup is safe because we've already validated the api-names
-        eid->id (into {} (t2/select-fn->fn :entity_id :id [model :id :entity_id] :entity_id [:in eids]))]
-    (mapv (fn [entity-id]
-            [entity-id (if-let [id (get eid->id entity-id)]
-                         {:id id :type api-name}
-                         {:type api-name :status "not-found"})])
-          eids)))
-
-(defn model->entity-ids->ids
-  "Given a map of model names to a sequence of entity-ids for each, return a map from entity-id -> id."
-  [model-key->entity-ids]
-  (when-not (mc/validate ModelToEntityIds model-key->entity-ids)
-    (throw (ex-info "Invalid format." {:explanation (me/humanize
-                                                     (me/with-spell-checking
-                                                       (mc/explain ModelToEntityIds model-key->entity-ids)))
-                                       :allowed-models (sort (keys api-name->model))
-                                       :status-code 400})))
-  (into {}
-        (mapcat
-         (fn [[model eids]] (entity-ids->id-for-model model eids))
-         model-key->entity-ids)))

test/metabase/api/util_test.clj

@@ -84,7 +84,7 @@
 
 (deftest entity-id-translation-test
   (mt/with-temp [:model/Card {card-id :id card-eid :entity_id} {}]
-    (is (= {card-eid {:id card-id :type "card"}}
+    (is (= {card-eid {:id card-id :type "card" :status "ok"}}
            (-> (mt/user-http-request :crowberto :post 200
                                      "util/entity_id"
                                      {:entity_ids {"card" [card-eid]}})