From 3b0e3a6b9c3e610333d2eca43fa1c88e4b6bcd2f Mon Sep 17 00:00:00 2001
From: Alexander Solovyov <alexander@solovyov.net>
Date: Fri, 16 Aug 2024 18:36:31 +0300
Subject: [PATCH] [serdes] extract-nested instead of relying on hydration
(#46912)
---
.../serialization/test_util.clj | 10 +
.../serialization/v2/e2e_test.clj | 233 +++++++++---------
.../serialization/v2/extract_test.clj | 74 +++++-
.../serialization/v2/storage_test.clj | 5 +-
src/metabase/models/action.clj | 4 +-
src/metabase/models/card.clj | 3 -
src/metabase/models/collection.clj | 8 +-
src/metabase/models/dashboard.clj | 4 -
src/metabase/models/dashboard_card.clj | 16 +-
src/metabase/models/field.clj | 7 -
src/metabase/models/serialization.clj | 68 +++--
11 files changed, 251 insertions(+), 181 deletions(-)
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
index f06ba4e7017..b8c181dee69 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
@@ -9,9 +9,11 @@
[metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Database
Field NativeQuerySnippet Pulse PulseCard Segment Table User]]
[metabase.models.collection :as collection]
+ [metabase.models.serialization :as serdes]
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.test :as mt]
[metabase.test.data :as data]
+ [metabase.util :as u]
[metabase.util.files :as u.files]
[toucan2.connection :as t2.conn]
[toucan2.core :as t2]
@@ -526,3 +528,11 @@
;; Don't memoize as IDs change in each `with-world` context
(alter-var-root #'names/path->context (fn [_] #'names/path->context*))
+
+(defn extract-one [model-name where]
+ (let [where (cond
+ (nil? where) true
+ (number? where) [:= :id where]
+ (string? where) [:= :entity_id where]
+ :else where)]
+ (u/rfirst (serdes/extract-all model-name {:where where}))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
index e9a34efdc4a..d597e125ecd 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
@@ -17,7 +17,6 @@
Field
ParameterCard
Table]]
- [metabase.models.action :as action]
[metabase.models.serialization :as serdes]
[metabase.models.setting :as setting]
[metabase.test :as mt]
@@ -113,87 +112,87 @@
(ts/with-db source-db
(testing "insert"
(test-gen/insert!
- {;; Actions are special case where there is a 1:1 relationship between an action and an action subtype (query, implicit, or http)
- ;; We generate 10 actions for each subtype, and 10 of each subtype.
- ;; actions 0-9 are query actions, 10-19 are implicit actions, and 20-29 are http actions.
- :action (apply concat
- (for [type [:query :implicit :http]]
- (many-random-fks 10
- {:spec-gen {:type type}}
- {:model_id [:sm 10]
- :creator_id [:u 10]})))
- :query-action (map-indexed
- (fn [idx x]
- (assoc-in x [1 :refs :action_id] (keyword (str "action" idx))))
- (many-random-fks 10 {} {:database_id [:db 10]}))
- :implicit-action (map-indexed
- (fn [idx x]
- (update-in x [1 :refs]
- (fn [refs]
- (assoc refs :action_id (keyword (str "action" (+ 10 idx)))))))
- (many-random-fks 10 {} {}))
- :http-action (map-indexed
- (fn [idx x]
- (update-in x [1 :refs]
- (fn [refs]
- (assoc refs :action_id (keyword (str "action" (+ 20 idx)))))))
- (many-random-fks 10 {} {}))
- :collection [[100 {:refs {:personal_owner_id ::rs/omit}}]
- [10 {:refs {:personal_owner_id ::rs/omit}
- :spec-gen {:namespace :snippets}}]]
- :database [[10]]
- ;; Tables are special - we define table 0-9 under db0, 10-19 under db1, etc. The :card spec below
- ;; depends on this relationship.
- :table (into [] (for [db [:db0 :db1 :db2 :db3 :db4 :db5 :db6 :db7 :db8 :db9]]
- [10 {:refs {:db_id db}}]))
- :field (many-random-fks 1000 {} {:table_id [:t 100]})
- :core-user [[100]]
- :card (mapv #(update-in % [1 :refs] table->db)
- (many-random-fks
- 100
- {:spec-gen {:dataset_query {:database 1
- :type :native
- :native {:query "SELECT * FROM whatever;"}}
- :type :model}}
- {:table_id [:t 100]
- :collection_id [:coll 100]
- :creator_id [:u 10]}))
- ;; Simple model is primary used for actions.
- ;; We can't use :card for actions because implicit actions require the model's query to contain
- ;; nothing but a source table
- :simple-model (mapv #(update-in % [1 :refs] table->db)
- (many-random-fks
- 10
- {:spec-gen {:dataset_query {:database 1
- :query {:source-table 3}
- :type :query}
- :type :model}}
- {:table_id [:t 10]
- :collection_id [:coll 10]
- :creator_id [:u 10]}))
- :dashboard (concat (many-random-fks 100 {} {:collection_id [:coll 100]
- :creator_id [:u 10]})
- ;; create some root collection dashboards
- (many-random-fks 50 {} {:creator_id [:u 10]}))
- :dashboard-card (many-random-fks 300 {} {:card_id [:c 100]
- :dashboard_id [:d 100]})
- :dimension (vec (concat
- ;; 20 with both IDs set
- (many-random-fks 20 {}
- {:field_id [:field 1000]
- :human_readable_field_id [:field 1000]})
- ;; 20 with just :field_id
- (many-random-fks 20 {:refs {:human_readable_field_id ::rs/omit}}
- {:field_id [:field 1000]})))
- :segment (many-random-fks 30 {:spec-gen {:definition {:filter [:!= [:field 60 nil] 50],
- :source-table 4}}}
- {:table_id [:t 100]
- :creator_id [:u 10]})
- :native-query-snippet (many-random-fks 10 {} {:creator_id [:u 10]
- :collection_id [:coll 10 100]})
- :timeline (many-random-fks 10 {} {:creator_id [:u 10]
- :collection_id [:coll 100]})
- :timeline-event (many-random-fks 90 {} {:timeline_id [:timeline 10]})}))
+ { ;; Actions are special case where there is a 1:1 relationship between an action and an action subtype (query, implicit, or http)
+ ;; We generate 10 actions for each subtype, and 10 of each subtype.
+ ;; actions 0-9 are query actions, 10-19 are implicit actions, and 20-29 are http actions.
+ :action (apply concat
+ (for [type [:query :implicit :http]]
+ (many-random-fks 10
+ {:spec-gen {:type type}}
+ {:model_id [:sm 10]
+ :creator_id [:u 10]})))
+ :query-action (map-indexed
+ (fn [idx x]
+ (assoc-in x [1 :refs :action_id] (keyword (str "action" idx))))
+ (many-random-fks 10 {} {:database_id [:db 10]}))
+ :implicit-action (map-indexed
+ (fn [idx x]
+ (update-in x [1 :refs]
+ (fn [refs]
+ (assoc refs :action_id (keyword (str "action" (+ 10 idx)))))))
+ (many-random-fks 10 {} {}))
+ :http-action (map-indexed
+ (fn [idx x]
+ (update-in x [1 :refs]
+ (fn [refs]
+ (assoc refs :action_id (keyword (str "action" (+ 20 idx)))))))
+ (many-random-fks 10 {} {}))
+ :collection [[100 {:refs {:personal_owner_id ::rs/omit}}]
+ [10 {:refs {:personal_owner_id ::rs/omit}
+ :spec-gen {:namespace :snippets}}]]
+ :database [[10]]
+ ;; Tables are special - we define table 0-9 under db0, 10-19 under db1, etc. The :card spec below
+ ;; depends on this relationship.
+ :table (into [] (for [db [:db0 :db1 :db2 :db3 :db4 :db5 :db6 :db7 :db8 :db9]]
+ [10 {:refs {:db_id db}}]))
+ :field (many-random-fks 1000 {} {:table_id [:t 100]})
+ :core-user [[100]]
+ :card (mapv #(update-in % [1 :refs] table->db)
+ (many-random-fks
+ 100
+ {:spec-gen {:dataset_query {:database 1
+ :type :native
+ :native {:query "SELECT * FROM whatever;"}}
+ :type :model}}
+ {:table_id [:t 100]
+ :collection_id [:coll 100]
+ :creator_id [:u 10]}))
+ ;; Simple model is primary used for actions.
+ ;; We can't use :card for actions because implicit actions require the model's query to contain
+ ;; nothing but a source table
+ :simple-model (mapv #(update-in % [1 :refs] table->db)
+ (many-random-fks
+ 10
+ {:spec-gen {:dataset_query {:database 1
+ :query {:source-table 3}
+ :type :query}
+ :type :model}}
+ {:table_id [:t 10]
+ :collection_id [:coll 10]
+ :creator_id [:u 10]}))
+ :dashboard (concat (many-random-fks 100 {} {:collection_id [:coll 100]
+ :creator_id [:u 10]})
+ ;; create some root collection dashboards
+ (many-random-fks 50 {} {:creator_id [:u 10]}))
+ :dashboard-card (many-random-fks 300 {} {:card_id [:c 100]
+ :dashboard_id [:d 100]})
+ :dimension (vec (concat
+ ;; 20 with both IDs set
+ (many-random-fks 20 {}
+ {:field_id [:field 1000]
+ :human_readable_field_id [:field 1000]})
+ ;; 20 with just :field_id
+ (many-random-fks 20 {:refs {:human_readable_field_id ::rs/omit}}
+ {:field_id [:field 1000]})))
+ :segment (many-random-fks 30 {:spec-gen {:definition {:filter [:!= [:field 60 nil] 50],
+ :source-table 4}}}
+ {:table_id [:t 100]
+ :creator_id [:u 10]})
+ :native-query-snippet (many-random-fks 10 {} {:creator_id [:u 10]
+ :collection_id [:coll 10 100]})
+ :timeline (many-random-fks 10 {} {:creator_id [:u 10]
+ :collection_id [:coll 100]})
+ :timeline-event (many-random-fks 90 {} {:timeline_id [:timeline 10]})}))
(is (= 101 (count (t2/select-fn-set :email 'User)))) ; +1 for the internal user
@@ -291,91 +290,79 @@
(testing "for Actions"
(doseq [{:keys [entity_id] :as coll} (get @entities "Action")]
(is (= (clean-entity coll)
- (->> (t2/select-one 'Action :entity_id entity_id)
- (@#'action/hydrate-subtype)
- (serdes/extract-one "Action" {})
- clean-entity)))))
+ (-> (ts/extract-one "Action" entity_id)
+ clean-entity)))))
(testing "for Collections"
(doseq [{:keys [entity_id] :as coll} (get @entities "Collection")]
(is (= (clean-entity coll)
- (->> (t2/select-one 'Collection :entity_id entity_id)
- (serdes/extract-one "Collection" {})
- clean-entity)))))
+ (-> (ts/extract-one "Collection" entity_id)
+ clean-entity)))))
(testing "for Databases"
(doseq [{:keys [name] :as db} (get @entities "Database")]
(is (= (assoc (clean-entity db) :initial_sync_status "complete")
- (->> (t2/select-one 'Database :name name)
- (serdes/extract-one "Database" {})
- clean-entity)))))
+ (-> (ts/extract-one "Database" [:= :name name])
+ clean-entity)))))
(testing "for Tables"
(doseq [{:keys [db_id name] :as coll} (get @entities "Table")]
(is (= (clean-entity coll)
- (->> (t2/select-one-fn :id 'Database :name db_id)
- (t2/select-one 'Table :name name :db_id)
- (serdes/extract-one "Table" {})
- clean-entity)))))
+ (-> (ts/extract-one "Table" [:and
+ [:= :name name]
+ [:= :db_id (t2/select-one-pk 'Database :name db_id)]])
+ clean-entity)))))
(testing "for Fields"
(doseq [{[db schema table] :table_id name :name :as coll} (get @entities "Field")]
(is (nil? schema))
- (is (= (clean-entity coll)
- (->> (t2/select-one-fn :id 'Database :name db)
- (t2/select-one-fn :id 'Table :schema schema :name table :db_id)
- (t2/select-one 'Field :name name :table_id)
- (serdes/extract-one "Field" {})
- clean-entity)))))
+ (let [db (t2/select-one-pk 'Database :name db)
+ table (t2/select-one-fn :id 'Table :schema schema :name table :db_id db)]
+ (is (= (clean-entity coll)
+ (-> (ts/extract-one "Field" [:and [:= :name name] [:= :table_id table]])
+ clean-entity))))))
(testing "for cards"
(doseq [{:keys [entity_id] :as card} (get @entities "Card")]
(is (= (clean-entity card)
- (->> (t2/select-one 'Card :entity_id entity_id)
- (serdes/extract-one "Card" {})
- clean-entity)))))
+ (-> (ts/extract-one "Card" entity_id)
+ clean-entity)))))
(testing "for dashboards"
(doseq [{:keys [entity_id] :as dash} (get @entities "Dashboard")]
(is (= (clean-entity dash)
- (->> (t2/select-one 'Dashboard :entity_id entity_id)
- (serdes/extract-one "Dashboard" {})
- clean-entity)))))
+ (-> (ts/extract-one "Dashboard" entity_id)
+ clean-entity)))))
(testing "for dashboard cards"
(doseq [{:keys [entity_id] :as dashcard} (get @entities "DashboardCard")]
(is (= (clean-entity dashcard)
- (->> (t2/select-one 'DashboardCard :entity_id entity_id)
- (serdes/extract-one "DashboardCard" {})
- clean-entity)))))
+ (-> (ts/extract-one "DashboardCard" entity_id)
+ clean-entity)))))
(testing "for dimensions"
(doseq [{:keys [entity_id] :as dim} (get @entities "Dimension")]
(is (= (clean-entity dim)
- (->> (t2/select-one 'Dimension :entity_id entity_id)
- (serdes/extract-one "Dimension" {})
- clean-entity)))))
+ (-> (ts/extract-one "Dimension" entity_id)
+ clean-entity)))))
(testing "for segments"
(doseq [{:keys [entity_id] :as segment} (get @entities "Segment")]
(is (= (clean-entity segment)
- (->> (t2/select-one 'Segment :entity_id entity_id)
- (serdes/extract-one "Segment" {})
- clean-entity)))))
+ (-> (ts/extract-one "Segment" entity_id)
+ clean-entity)))))
(testing "for native query snippets"
(doseq [{:keys [entity_id] :as snippet} (get @entities "NativeQuerySnippet")]
(is (= (clean-entity snippet)
- (->> (t2/select-one 'NativeQuerySnippet :entity_id entity_id)
- (serdes/extract-one "NativeQuerySnippet" {})
- clean-entity)))))
+ (-> (ts/extract-one "NativeQuerySnippet" entity_id)
+ clean-entity)))))
(testing "for timelines and events"
(doseq [{:keys [entity_id] :as timeline} (get @entities "Timeline")]
(is (= (clean-entity timeline)
- (->> (t2/select-one 'Timeline :entity_id entity_id)
- (serdes/extract-one "Timeline" {})
- clean-entity)))))
+ (-> (ts/extract-one "Timeline" entity_id)
+ clean-entity)))))
(testing "for settings"
(let [settings (get @entities "Setting")]
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
index 9e9afab15f8..a975cc8a177 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
@@ -455,7 +455,7 @@
(set (serdes/dependencies ser)))))))
(testing "Dashboards include their Dashcards"
- (let [ser (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id other-dash-id]}))]
+ (let [ser (ts/extract-one "Dashboard" other-dash-id)]
(is (=? {:serdes/meta [{:model "Dashboard" :id other-dash :label "dave_s_dash"}]
:entity_id other-dash
:dashcards
@@ -496,7 +496,7 @@
(set (serdes/dependencies ser)))))))
(testing "Dashboards with parameters where the source is a card"
- (let [ser (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id param-dash-id]}))]
+ (let [ser (ts/extract-one "Dashboard" param-dash-id)]
(is (=? {:parameters
[{:id "abc"
:name "CATEGORY"
@@ -515,7 +515,7 @@
(set (serdes/dependencies ser))))))
(testing "Cards with parameters where the source is a card"
- (let [ser (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id param-dash-id]}))]
+ (let [ser (ts/extract-one "Dashboard" param-dash-id)]
(is (=? {:parameters
[{:id "abc"
:name "CATEGORY"
@@ -579,7 +579,7 @@
:model/DashboardCardSeries _ {:card_id c2-id, :dashboardcard_id dc1-id, :position 0}]
(testing "Inlined dashcards include their series' card entity IDs"
(let [ser (t2/with-call-count [q]
- (u/prog1 (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id dash-id]}))
+ (u/prog1 (ts/extract-one "Dashboard" dash-id)
(is (< (q) 13))))]
(is (=? {:entity_id dash-eid
:dashcards [{:entity_id dc1-eid
@@ -626,7 +626,7 @@
:field_id fk-id
:human_readable_field_id cust-name}]
(testing "dimensions without foreign keys are inlined into their Fields\n"
- (let [ser (u/rfirst (serdes/extract-all "Field" {:where [:= :id email-id]}))]
+ (let [ser (ts/extract-one "Field" email-id)]
(is (malli= [:map
[:serdes/meta [:= [{:model "Database", :id "My Database"}
{:model "Table", :id "Schemaless Table"}
@@ -648,7 +648,7 @@
(set (serdes/dependencies ser)))))))
(testing "foreign key dimensions are inlined into their Fields"
- (let [ser (u/rfirst (serdes/extract-all "Field" {:where [:= :id fk-id]}))]
+ (let [ser (ts/extract-one "Field" fk-id)]
(is (malli= [:map
[:serdes/meta [:= [{:model "Database" :id "My Database"}
{:model "Schema" :id "PUBLIC"}
@@ -1707,3 +1707,65 @@
:no-data-model true})]
(is (= #{(:entity_id c)}
(by-model "Collection" ser)))))))
+
+(deftest extract-nested-test
+ (testing "extract-nested working"
+ (mt/with-temp [:model/Dashboard d {:name "Top Dash"}
+ :model/Card c1 {:name "Some Card"}
+ :model/Card c2 {:name "Some Inner Card"}
+ :model/DashboardCard dc1 {:dashboard_id (:id d) :card_id (:id c1)}
+ :model/DashboardCardSeries s {:dashboardcard_id (:id dc1) :card_id (:id c2)}]
+ (let [spec (serdes/make-spec "DashboardCard" nil)]
+ (is (= {(:id dc1) [s]}
+ (#'serdes/transform->nested (-> spec :transform :series) [dc1])))
+ (is (=? (assoc dc1 :series [s])
+ (u/rfirst (serdes/extract-query "DashboardCard" {:where [:= :id (:id dc1)]})))))
+ (let [spec (serdes/make-spec "Dashboard" nil)]
+ (is (= {(:id d) [(assoc dc1 :series [s])]}
+ (#'serdes/transform->nested (-> spec :transform :dashcards) [d])))
+ (is (=? (assoc d
+ :dashcards [(assoc dc1 :series [s])]
+ :tabs nil)
+ (u/rfirst (serdes/extract-query "Dashboard" {:where [:= :id (:id d)]}))))))))
+
+(deftest extract-nested-efficient-test
+ (testing "extract-nested is efficient"
+ (mt/with-temp [:model/Dashboard d1 {:name "Top Dash 1"}
+ :model/Dashboard d2 {:name "Top Dash 2"}
+ :model/Card c1 {:name "Some Card"}
+ :model/Card c2 {:name "Some Inner Card"}
+ :model/Card c3 {:name "Card for Dash 2"}
+ :model/DashboardCard dc1 {:dashboard_id (:id d1) :card_id (:id c1)}
+ :model/DashboardCard dc2 {:dashboard_id (:id d2) :card_id (:id c2)}
+ :model/DashboardCard dc3 {:dashboard_id (:id d2) :card_id (:id c3)}
+ :model/DashboardCardSeries s {:dashboardcard_id (:id dc1) :card_id (:id c2)}]
+ (t2/with-call-count [qc]
+ (is (=? [(assoc d1
+ :dashcards [(assoc dc1 :series [s])]
+ :tabs nil)
+ (assoc d2
+ :dashcards [(assoc dc2 :series nil)
+ (assoc dc3 :series nil)]
+ :tabs nil)]
+ (into [] (serdes/extract-query "Dashboard" {:where [:in :id [(:id d1) (:id d2)]]}))))
+ ;; 1 per dashboard/dashcard/series/tabs
+ (is (= 4 (qc)))))))
+
+(deftest extract-nested-partitioned-test
+ (testing "extract-nested will partition stuff by 100s"
+ (mt/with-empty-h2-app-db
+ (let [d (ts/create! :model/Dashboard {:name "Dash"})
+ c1 (ts/create! :model/Card {:name "Card"})
+ dcs (vec (for [_ (range 7)]
+ (ts/create! :model/DashboardCard {:dashboard_id (:id d)
+ :card_id (:id c1)})))]
+ (t2/with-call-count [qc]
+ (is (=? [(assoc d :dashcards dcs)]
+ (into [] (serdes/extract-query "Dashboard" {:batch-limit 5
+ :where [:= :id (:id d)]}))))
+ ;; query count breakdown:
+ ;; - 1 for dashboard
+ ;; - 1 for tabs, there are none
+ ;; - 1 for dashcards, there are 7
+ ;; - 2 for series (7 dashcards / 5 -> 2 batches)
+ (is (= 5 (qc))))))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj
index b71df33fd23..96757ea7b94 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj
@@ -10,7 +10,6 @@
[metabase-enterprise.serialization.v2.storage :as storage]
[metabase.models :refer [Card Collection Dashboard DashboardCard Database Field FieldValues NativeQuerySnippet
Table]]
- [metabase.models.serialization :as serdes]
[metabase.test :as mt]
[metabase.util.date-2 :as u.date]
[metabase.util.yaml :as yaml]
@@ -145,8 +144,8 @@
"Slashes in directory names get escaped"))
(testing "the Field was properly exported"
- (is (= (-> (into {} (serdes/extract-one "Field" {} (t2/select-one 'Field :id (:id website))))
- (update :created_at u.date/format))
+ (is (= (-> (ts/extract-one "Field" (:id website))
+ (update :created_at u.date/format))
(-> (yaml/from-file (io/file dump-dir
"databases" "My Company Data"
"tables" "Customers"
diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj
index a86c57cb519..38637824117 100644
--- a/src/metabase/models/action.clj
+++ b/src/metabase/models/action.clj
@@ -352,9 +352,9 @@
;;; ------------------------------------------------ Serialization ---------------------------------------------------
-(defmethod serdes/extract-query "Action" [_model _opts]
+(defmethod serdes/extract-query "Action" [_model opts]
(eduction (map hydrate-subtype)
- (t2/reducible-select Action)))
+ (t2/reducible-select Action {:where (or (:where opts) true)})))
(defmethod serdes/hash-fields :model/Action [_action]
[:name (serdes/hydrated-hash :model) :created_at])
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index bc257d3425d..9b8ee57293a 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -813,9 +813,6 @@
;;; ------------------------------------------------- Serialization --------------------------------------------------
-(defmethod serdes/extract-query "Card" [_ opts]
- (serdes/extract-query-collections Card opts))
-
(defn- export-result-metadata [metadata]
(when metadata
(for [m metadata]
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index e6821153129..ca37d0bc030 100644
--- a/src/metabase/models/collection.clj
+++ b/src/metabase/models/collection.clj
@@ -1224,7 +1224,7 @@
[_collection]
[:name :namespace parent-identity-hash :created_at])
-(defmethod serdes/extract-query "Collection" [_model {:keys [collection-set]}]
+(defmethod serdes/extract-query "Collection" [_model {:keys [collection-set where]}]
(let [not-trash-clause [:or
[:= :type nil]
[:not= :type trash-collection-type]]]
@@ -1233,12 +1233,14 @@
{:where
[:and
[:in :id collection-set]
- not-trash-clause]})
+ not-trash-clause
+ (or where true)]})
(t2/reducible-select Collection
{:where
[:and
[:= :personal_owner_id nil]
- not-trash-clause]}))))
+ not-trash-clause
+ (or where true)]}))))
(defmethod serdes/extract-one "Collection"
;; Transform :location (which uses database IDs) into a portable :parent_id with the parent's entity ID.
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 7d5c6e70aba..53c3ffd79b0 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -580,10 +580,6 @@
;;; | SERIALIZATION |
;;; +----------------------------------------------------------------------------------------------------------------+
-(defmethod serdes/extract-query "Dashboard" [_ opts]
- (eduction (map #(t2/hydrate % :tabs [:dashcards :series]))
- (serdes/extract-query-collections Dashboard opts)))
-
(defmethod serdes/make-spec "Dashboard" [_model-name opts]
{:copy [:archived :archived_directly :auto_apply_filters :cache_ttl :caveats :collection_position
:description :embedding_params :enable_embedding :entity_id :initially_published_at :name
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index 509361901e9..665392001cc 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -372,15 +372,7 @@
:import serdes/import-parameter-mappings}
:visualization_settings {:export serdes/export-visualization-settings
:import serdes/import-visualization-settings}
- :series
- (-> (serdes/nested :model/DashboardCardSeries :dashboardcard_id
- (assoc opts
- :sort-by :position
- :key-field :card_id))
- ;; FIXME: this waits to be removed when `extract-nested` (instead of using hydration) is
- ;; implemented; see comment at `make-spec` for `DashboardCardSeries`
- (assoc :export (fn [data]
- (vec (map-indexed (fn [i x]
- {:card_id (serdes/*export-fk* (:id x) :model/Card)
- :position i})
- data)))))}})
+ :series (serdes/nested :model/DashboardCardSeries :dashboardcard_id
+ (assoc opts
+ :sort-by :position
+ :key-field :card_id)) }})
diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj
index 17ff8800eec..cbb61f1ed32 100644
--- a/src/metabase/models/field.clj
+++ b/src/metabase/models/field.clj
@@ -377,13 +377,6 @@
(let [table (serdes/load-find-local (pop path))]
(t2/select-one Field :name (-> path last :id) :table_id (:id table))))
-(defmethod serdes/extract-query "Field" [_model-name opts]
- (let [d (t2/select Dimension)
- dimensions (->> d
- (group-by :field_id))]
- (eduction (map #(assoc % :dimensions (get dimensions (:id %))))
- (t2/reducible-select Field {:where (:where opts true)}))))
-
(defmethod serdes/dependencies "Field" [field]
;; Fields depend on their parent Table, plus any foreign Fields referenced by their Dimensions.
;; Take the path, but drop the Field section to get the parent Table's path instead.
diff --git a/src/metabase/models/serialization.clj b/src/metabase/models/serialization.clj
index 694f04bfca7..061dade1944 100644
--- a/src/metabase/models/serialization.clj
+++ b/src/metabase/models/serialization.clj
@@ -32,6 +32,9 @@
(set! *warn-on-reflection* true)
+;; there was no science behind picking 100 as a number
+(def ^:private extract-nested-batch-limit "max amount of entities to fetch nested entities for" 100)
+
;;; # Serialization Overview
;;;
;;; Serialization (or "serdes") is a system for exporting entities (Dashboards, Cards, Collections, etc.) from one
@@ -330,7 +333,7 @@
(m/assoc-some :serdes/meta (generate-path model-name instance))
(into (for [[k transform] (:transform spec)
:let [res ((:export transform) (get instance k))]
- ;; include only non-nil transform results
+ ;; include only non-nil `transform` results
:when res]
[k res])))))
(catch Exception e
@@ -410,23 +413,53 @@
(eduction (map (partial log-and-extract-one model opts))
(extract-query model opts)))
+(declare extract-query)
+
+(defn- transform->nested [transform opts batch]
+ (let [backward-fk (:backward-fk transform)
+ entities (-> (extract-query (name (:model transform))
+ (assoc opts :where [:in backward-fk (map :id batch)]))
+ t2.realize/realize)]
+ (group-by backward-fk entities)))
+
+(defn- extract-batch-nested [model-name opts batch]
+ (let [spec (make-spec model-name opts)]
+ (reduce-kv (fn [batch k transform]
+ (if-not (::nested transform)
+ batch
+ (mi/instances-with-hydrated-data batch k #(transform->nested transform opts batch) :id)))
+ batch
+ (:transform spec))))
+
+(defn- extract-reducible-nested [model-name opts reducible]
+ (eduction (comp (map t2.realize/realize)
+ (partition-all (or (:batch-limit opts)
+ extract-nested-batch-limit))
+ (map (partial extract-batch-nested model-name opts))
+ cat)
+ reducible))
+
(defn extract-query-collections
"Helper for the common (but not default) [[extract-query]] case of fetching everything that isn't in a personal
collection."
- [model {:keys [collection-set where]}]
- (if collection-set
- ;; If collection-set is defined, select everything in those collections, or with nil :collection_id.
- (t2/reducible-select model {:where [:or
- [:in :collection_id collection-set]
- (when (contains? collection-set nil)
- [:= :collection_id nil])
- (when where
- where)]})
- ;; If collection-set is nil, just select everything.
- (t2/reducible-select model {:where (or where true)})))
-
-(defmethod extract-query :default [model-name {:keys [where]}]
- (t2/reducible-select (symbol model-name) {:where (or where true)}))
+ [model {:keys [collection-set where] :as opts}]
+ (let [spec (make-spec (name model) opts)]
+ (if (or (nil? collection-set)
+ (nil? (-> spec :transform :collection_id)))
+ ;; either no collections specified or our model has no collection
+ (t2/reducible-select model {:where (or where true)})
+ (t2/reducible-select model {:where [:or
+ [:in :collection_id collection-set]
+ (when (contains? collection-set nil)
+ [:= :collection_id nil])
+ (when where
+ where)]}))))
+
+(defmethod extract-query :default [model-name opts]
+ (let [spec (make-spec model-name opts)
+ nested? (some ::nested (vals (:transform spec)))]
+ (cond->> (extract-query-collections (keyword "model" model-name) opts)
+ nested? (extract-reducible-nested model-name (dissoc opts :where)))))
(defn extract-one-basics
"A helper for writing [[extract-one]] implementations. It takes care of the basics:
@@ -1534,15 +1567,14 @@
{::nested true
:model model
:backward-fk backward-fk
+ :opts opts
:export (fn [data]
(assert (every? #(t2/instance-of? model %) data)
(format "Nested data is expected to be a %s, not %s" model (t2/model (first data))))
;; `nil? data` check is for `extract-one` case in tests; make sure to add empty vectors in
;; `extract-query` implementations for nested collections
(try
- (->> (or data (when (nil? data)
- (t2/select model backward-fk (:id *current*))))
- (sort-by sorter)
+ (->> (sort-by sorter data)
(mapv #(extract-one model-name opts %)))
(catch Exception e
(throw (ex-info (format "Error exporting nested %s" model)
--
GitLab