Skip to content
Snippets Groups Projects
Unverified Commit 3b0e3a6b authored by Alexander Solovyov's avatar Alexander Solovyov Committed by GitHub
Browse files

[serdes] extract-nested instead of relying on hydration (#46912)

parent d959430a
No related branches found
No related tags found
No related merge requests found
Expand all Hide whitespace changes
Inline Side-by-side
Showing
with 251 additions and 181 deletions
enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
+ 10
− 0
View file @ 3b0e3a6b
......@@ -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}))))
enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+ 68
− 6
View file @ 3b0e3a6b
......@@ -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))))))))
enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj
+ 2
− 3
View file @ 3b0e3a6b
......@@ -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"
......
src/metabase/models/action.clj
+ 2
− 2
View file @ 3b0e3a6b
......@@ -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])
......
src/metabase/models/card.clj
+ 0
− 3
View file @ 3b0e3a6b
......@@ -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]
......
src/metabase/models/collection.clj
+ 5
− 3
View file @ 3b0e3a6b
......@@ -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.
......
src/metabase/models/dashboard.clj
+ 0
− 4
View file @ 3b0e3a6b
......@@ -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
......
src/metabase/models/dashboard_card.clj
+ 4
− 12
View file @ 3b0e3a6b
......@@ -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)) }})
src/metabase/models/field.clj
+ 0
− 7
View file @ 3b0e3a6b
......@@ -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.
......
src/metabase/models/serialization.clj
+ 50
− 18
View file @ 3b0e3a6b
......@@ -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)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment