Serialization: support dashboard cards being in a different collection (#15842)

Make the load multimethod return a function, which serves as a "reload function", to be called after the first entire pass is complete (via cmd)

Updating various places to combine these functions together and invoke at the top level from cmd.clj when load finishes

Updating dashboard load implementation to check for presence of cards referred to by its dashcards, and if missing, reload those dashboards(s) on a second pass

Splitting out a separate load-dashboards fn that contains the bulk of the body from the previous multimethod implementation (which now delegates to it) to support easier reload functionality

enterprise/backend/src/metabase_enterprise/serialization/cmd.clj

@@ -47,13 +47,18 @@
                        context)]
     (try
       (do
-        (load/load (str path "/users") context)
-        (load/load (str path "/databases") context)
-        (load/load (str path "/collections") context)
-        (load/load-settings path context)
-        (load/load-dependencies path context))
+        (let [all-res    [(load/load (str path "/users") context)
+                          (load/load (str path "/databases") context)
+                          (load/load (str path "/collections") context)
+                          (load/load-settings path context)
+                          (load/load-dependencies path context)]
+              reload-fns (filter fn? all-res)]
+          (if-not (empty? reload-fns)
+            (do (log/info (trs "Finished first pass of load; now performing second pass"))
+                (doseq [reload-fn reload-fns]
+                  (reload-fn))))))
       (catch Throwable e
-        (log/error (trs "Error loading dump: {0}" (.getMessage e)))))))
+        (log/error e (trs "Error loading dump: {0}" (.getMessage e)))))))
 
 (defn- select-entities-in-collections
   ([model collections]

enterprise/backend/src/metabase_enterprise/serialization/load.clj

@@ -185,33 +185,69 @@
         (update :card_id fully-qualified-name->card-id)
         (update :target mbql-fully-qualified-names->ids))))
 
-(defmethod load "dashboards"
-  [path context]
-  (let [dashboards         (slurp-dir path)
-        dashboard-ids      (maybe-upsert-many! context Dashboard
-                             (for [dashboard dashboards]
-                               (-> dashboard
-                                   (dissoc :dashboard_cards)
-                                   (assoc :collection_id (:collection context)
-                                          :creator_id    @default-user))))
-        dashboard-cards    (map :dashboard_cards dashboards)
-        dashboard-card-ids (maybe-upsert-many! context DashboardCard
-                             (for [[dashboard-cards dashboard-id] (map vector dashboard-cards dashboard-ids)
-                                   dashboard-card dashboard-cards
-                                   :when dashboard-id]
-                               (-> dashboard-card
-                                   (dissoc :series)
-                                   (update :card_id fully-qualified-name->card-id)
-                                   (assoc :dashboard_id dashboard-id)
-                                   (update :parameter_mappings update-parameter-mappings))))]
+(defn load-dashboards
+  "Loads `dashboards` (which is a sequence of maps parsed from a YAML dump of dashboards) in a given `context`."
+  {:added "0.40.0"}
+  [context dashboards]
+  (let [dashboard-ids   (maybe-upsert-many! context Dashboard
+                          (for [dashboard dashboards]
+                            (-> dashboard
+                                (dissoc :dashboard_cards)
+                                (assoc :collection_id (:collection context)
+                                       :creator_id    @default-user))))
+        dashboard-cards (map :dashboard_cards dashboards)
+        ;; a function that prepares a dash card for insertion, while also validating to ensure the underlying
+        ;; card_id could be resolved from the fully qualified name
+        prepare-card-fn (fn [idx dashboard-id card]
+                          (let [final-c (-> (assoc card :fully-qualified-card-name (:card_id card))
+                                            (update :card_id fully-qualified-name->card-id)
+                                            (assoc :dashboard_id dashboard-id)
+                                            (update :parameter_mappings update-parameter-mappings))]
+                            (if (and (nil? (:card_id final-c))
+                                     (some? (:fully-qualified-card-name final-c)))
+                              ;; the lookup of some fully qualified card name failed
+                              ;; we will need to revisit this dashboard in the next pass
+                              {:revisit idx}
+                              ;; card was looked up OK, we can process it
+                              {:process final-c})))
+        filtered-cards  (reduce-kv
+                         (fn [acc idx [cards dash-id]]
+                           (if dash-id
+                             (let [{:keys [process revisit]} (apply
+                                                              merge-with
+                                                              conj
+                                                              {:process [] :revisit []}
+                                                              (map (partial prepare-card-fn idx dash-id) cards))]
+                               (-> acc
+                                   (update :process #(concat % process))
+                                   (update :revisit #(concat % revisit))))
+                             acc))
+                         {:process [] :revisit []}
+                         (mapv vector dashboard-cards dashboard-ids))
+        revisit-indexes (:revisit filtered-cards)
+        proceed-cards   (->> (:process filtered-cards)
+                             (map #(dissoc % :fully-qualified-card-name)))
+        dashcard-ids    (maybe-upsert-many! context DashboardCard (map #(dissoc % :series) proceed-cards))]
     (maybe-upsert-many! context DashboardCardSeries
-      (for [[series dashboard-card-id] (map vector (mapcat (partial map :series) dashboard-cards)
-                                                   dashboard-card-ids)
+      (for [[series dashboard-card-id] (map vector (mapcat (partial map :series) proceed-cards)
+                                            dashcard-ids)
             dashboard-card-series series
             :when (and dashboard-card-series dashboard-card-id)]
         (-> dashboard-card-series
             (assoc :dashboardcard_id dashboard-card-id)
-            (update :card_id fully-qualified-name->card-id))))))
+            (update :card_id fully-qualified-name->card-id))))
+    (let [revisit-dashboards (map (partial nth dashboards) revisit-indexes)]
+      (if-not (empty? revisit-dashboards)
+        (fn []
+          (log/infof
+           "Retrying dashboards for collection %s: %s"
+           (or (:collection context) "root")
+           (str/join ", " (map :name revisit-dashboards)))
+          (load-dashboards context revisit-dashboards))))))
+
+(defmethod load "dashboards"
+  [path context]
+  (load-dashboards context (slurp-dir path)))
 
 (defmethod load "pulses"
   [path context]
@@ -314,17 +350,22 @@
 
 (defmethod load "collections"
   [path context]
-  (doseq [path (list-dirs path)]
-    (let [context (assoc context
-                    :collection (->> (slurp-dir path)
-                                     (map (fn [collection]
-                                            (assoc collection :location (derive-location context))))
-                                     (maybe-upsert-many! context Collection)
-                                     first))]
-      (load (str path "/collections") context)
-      (load (str path "/cards") context)
-      (load (str path "/pulses") context)
-      (load (str path "/dashboards") context))))
+  (let [res-fns (for [path (list-dirs path)]
+                  (let [context (assoc context
+                                  :collection (->> (slurp-dir path)
+                                                   (map (fn [collection]
+                                                          (assoc collection :location (derive-location context))))
+                                                   (maybe-upsert-many! context Collection)
+                                                   first))]
+                    (filter fn? [(load (str path "/collections") context)
+                                 (load (str path "/cards") context)
+                                 (load (str path "/pulses") context)
+                                 (load (str path "/dashboards") context)])))
+        all-fns (apply concat res-fns)]
+    (if-not (empty? all-fns)
+      (fn []
+        (doseq [reload-fn all-fns]
+          (reload-fn))))))
 
 (defn load-settings
   "Load a dump of settings."

enterprise/backend/src/metabase_enterprise/serialization/names.clj

@@ -232,6 +232,7 @@
                          (str/join ", " (map name (keys (:value (ex-data e)))))
                          fully-qualified-name)
                     {:fully-qualified-name fully-qualified-name
+                     :resolve-name-failed? true
                      :context              context})))))))
 
 (defn name-for-logging

enterprise/backend/src/metabase_enterprise/serialization/specs.clj

+(ns metabase-enterprise.serialization.specs
+  "Shared specs for serialization related code."
+  (:require [clojure.spec.alpha :as s]))
+
+;; a structure that represents items that can be loaded
+;; (s/def ::load-items (s/cat ::context map? ::model some? ::entity-dir string? ::entity-names (s/+ string?)))
+
+;; a structure that represents some items having failed to load; items are fully qualified entity names
+(s/def ::failed-name-resolution (s/map-of string? (s/+ string?)))

enterprise/backend/test/metabase_enterprise/serialization/load_test.clj

@@ -59,7 +59,8 @@
                          [Card          (Card card-id-nested)]
                          [Card          (Card card-id-nested-query)]
                          [Card          (Card card-id-native-query)]
-                         [DashboardCard (DashboardCard dashcard-id)]])]
+                         [DashboardCard (DashboardCard dashcard-id)]
+                         [DashboardCard (DashboardCard dashcard-with-click-actions)]])]
       (with-world-cleanup
         (load dump-dir {:on-error :abort :mode :skip})
         (doseq [[model entity] fingerprint]

enterprise/backend/test/metabase_enterprise/serialization/test_util.clj

@@ -106,7 +106,13 @@
                                              :position 0}]
                    DashboardCardSeries [~'_ {:dashboardcard_id ~'dashcard-id
                                              :card_id ~'card-id-nested
-                                             :position 1}]]
+                                             :position 1}]
+                   DashboardCard       [{~'dashcard-with-click-actions :id}
+                                        {:dashboard_id ~'dashboard-id
+                                         :card_id      ~'card-id-root}]
+                   DashboardCardSeries [~'_ {:dashboardcard_id ~'dashcard-with-click-actions
+                                             :card_id          ~'card-id-root
+                                             :position         2}]]
      ~@body))
 
 ;; Don't memoize as IDs change in each `with-world` context