diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj
index 834ec9571d1bfafae1c4a6b213b0f3c2e6b8a90d..0a8ef093752191d27b46346a9bcc9b0a7836c752 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj
@@ -11,6 +11,13 @@
(declare load-one!)
+(defn- without-references
+ "Remove references to other entities from a given one. Used to break circular dependencies when loading."
+ [entity]
+ (if (:dashcards entity)
+ (dissoc entity :dashcards)
+ (throw (ex-info "No known references found when breaking circular dependency!" {:entity entity}))))
+
(defn- load-deps!
"Given a list of `deps` (hierarchies), [[load-one]] them all.
If [[load-one]] throws because it can't find that entity in the filesystem, check if it's already loaded in
@@ -22,29 +29,41 @@
(try
(load-one! ctx dep)
(catch Exception e
- (if (and (= (:error (ex-data e)) ::not-found)
- (serdes/load-find-local dep))
+ (cond
;; It was missing but we found it locally, so just return the context.
+ (and (= (:error (ex-data e)) ::not-found)
+ (serdes/load-find-local dep))
ctx
- ;; Different error, or couldn't find it locally, so rethrow.
+
+ ;; It's a circular dep, strip off probable cause and retry. This will store an incomplete version
+ ;; of an entity, but this is not a problem - a full version is waiting to be stored up the stack.
+ (= (:error (ex-data e)) ::circular)
+ (do
+ (log/debug "Detected circular dependency" (serdes/log-path-str dep))
+ (load-one! (update ctx :expanding disj dep) dep without-references))
+
+ :else
(throw e)))))]
(reduce loader ctx deps))))
(defn- load-one!
- "Loads a single entity, specified by its `:serdes/meta` abstract path, into the appdb, doing some bookkeeping to avoid
- cycles.
+ "Loads a single entity, specified by its `:serdes/meta` abstract path, into the appdb, doing some bookkeeping to
+ avoid cycles.
- If the incoming entity has any dependencies, they are recursively processed first (postorder) so that any foreign key
- references in this entity can be resolved properly.
+ If the incoming entity has any dependencies, they are recursively processed first (postorder) so that any foreign
+ key references in this entity can be resolved properly.
This is mostly bookkeeping for the overall deserialization process - the actual load of any given entity is done by
[[metabase.models.serialization/load-one!]] and its various overridable parts, which see.
- Circular dependencies are not allowed, and are detected and thrown as an error."
- [{:keys [expanding ingestion seen] :as ctx} path]
+ Error is thrown on a circular dependency, should be handled and retried at the caller. `modfn` is an optional
+ parameter to modify entity data after reading and before other processing (before loading dependencies, finding
+ local version, and storing in the db)."
+ [{:keys [expanding ingestion seen] :as ctx} path & [modfn]]
(log/infof "Loading %s" (serdes/log-path-str path))
(cond
- (expanding path) (throw (ex-info (format "Circular dependency on %s" (pr-str path)) {:path path}))
+ (expanding path) (throw (ex-info (format "Circular dependency on %s" (pr-str path)) {:path path
+ :error ::circular}))
(seen path) ctx ; Already been done, just skip it.
:else (let [ingested (try
(serdes.ingest/ingest-one ingestion path)
@@ -54,7 +73,10 @@
:deps-chain expanding
:error ::not-found}
e))))
+ ingested (cond-> ingested
+ modfn modfn)
deps (serdes/dependencies ingested)
+ _ (log/debug "Loading dependencies" deps)
ctx (-> ctx
(update :expanding conj path)
(load-deps! deps)
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
index 88dfc7aa7847c0418e3705021c327e36b5d76834..bf49cbe26d8c3f8ab4fae66ac3187f9ec37a1598 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
@@ -1223,3 +1223,36 @@
(serdes.load/load-metabase! (ingestion-in-memory @serialized))))
(is (= (str "qwe_" (:name coll))
(t2/select-one-fn :name Collection :id (:id card))))))))))
+
+(deftest circular-links-test
+ (mt/with-empty-h2-app-db
+ (let [coll (ts/create! Collection :name "coll")
+ card (ts/create! Card :name "card" :collection_id (:id coll))
+ dash1 (ts/create! Dashboard :name "dash1" :collection_id (:id coll))
+ dash2 (ts/create! Dashboard :name "dash2" :collection_id (:id coll))
+ dash3 (ts/create! Dashboard :name "dash3" :collection_id (:id coll))
+ dc1 (ts/create! DashboardCard :dashboard_id (:id dash1) :card_id (:id card)
+ :visualization_settings {:click_behavior {:type "link"
+ :linkType "dashboard"
+ :targetId (:id dash2)}})
+ dc2 (ts/create! DashboardCard :dashboard_id (:id dash2) :card_id (:id card)
+ :visualization_settings {:click_behavior {:type "link"
+ :linkType "dashboard"
+ :targetId (:id dash3)}})
+ dc3 (ts/create! DashboardCard :dashboard_id (:id dash2) :card_id (:id card)
+ :visualization_settings {:click_behavior {:type "link"
+ :linkType "dashboard"
+ :targetId (:id dash1)}})
+ ser (atom nil)]
+ (reset! ser (into [] (serdes.extract/extract {:no-settings true
+ :no-data-model true})))
+ (t2/delete! DashboardCard :id [:in (map :id [dc1 dc2 dc3])])
+ (testing "Circular dependencies are loaded correctly"
+ (is (serdes.load/load-metabase! (ingestion-in-memory @ser)))
+ (let [select-target #(-> % :visualization_settings :click_behavior :targetId)]
+ (is (= (:id dash2)
+ (t2/select-one-fn select-target DashboardCard :entity_id (:entity_id dc1))))
+ (is (= (:id dash3)
+ (t2/select-one-fn select-target DashboardCard :entity_id (:entity_id dc2))))
+ (is (= (:id dash1)
+ (t2/select-one-fn select-target DashboardCard :entity_id (:entity_id dc3)))))))))