From d5b25cd9c2f346d104f8499fc410f71a3cb9b0aa Mon Sep 17 00:00:00 2001
From: github-automation-metabase
<166700802+github-automation-metabase@users.noreply.github.com>
Date: Wed, 14 Aug 2024 02:58:12 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20backported=20"[serdes]=20convert?=
=?UTF-8?q?=20database,=20card=20and=20dashboard=20with=20friends=20to=20u?=
=?UTF-8?q?sing=20spec"=20(#46720)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Alexander Solovyov <alexander@solovyov.net>
---
dev/src/dev/toucan2_monitor.clj | 154 ++++++++++++++
.../serialization/load_test.clj | 12 --
.../serialization/v2/e2e_test.clj | 33 +--
.../serialization/v2/extract_test.clj | 73 +++----
.../serialization/v2/load_test.clj | 137 +++++++++++-
.../serialization/v2/models_test.clj | 55 ++---
src/metabase/models/card.clj | 39 ++--
src/metabase/models/dashboard.clj | 117 ++---------
src/metabase/models/dashboard_card.clj | 61 +++---
src/metabase/models/dashboard_card_series.clj | 15 ++
src/metabase/models/dashboard_tab.clj | 13 +-
src/metabase/models/database.clj | 37 +---
src/metabase/models/dimension.clj | 17 +-
src/metabase/models/field.clj | 65 ++----
src/metabase/models/field_values.clj | 3 +
src/metabase/models/serialization.clj | 197 +++++++++++++-----
src/metabase/util.cljc | 5 +
test/metabase/test/generate.clj | 3 +-
18 files changed, 631 insertions(+), 405 deletions(-)
create mode 100644 dev/src/dev/toucan2_monitor.clj
diff --git a/dev/src/dev/toucan2_monitor.clj b/dev/src/dev/toucan2_monitor.clj
new file mode 100644
index 00000000000..6db39e6cfea
--- /dev/null
+++ b/dev/src/dev/toucan2_monitor.clj
@@ -0,0 +1,154 @@
+(ns dev.toucan2-monitor
+ "Utilities to track and monitor queries made by toucan2.
+
+ Usage:
+ (start!) ;; start tracking
+ ;; do some query using toucan 2 or via UI
+ (queries) ;; get all queries and its execution time
+ ;; => [[[\"SELECT * FROM report_card\"] 100]]
+ (stop!)
+ (to-csv!)
+ ;; to save all queries to a csv file"
+ (:require
+ [clojure.data.csv :as csv]
+ [clojure.java.io :as io]
+ [clojure.stacktrace :as stacktrace]
+ [clojure.string :as str]
+ [dev.util :as dev.u]
+ [metabase.test.util.log :as tu.log]
+ [metabase.util :as u]
+ [metabase.util.log :as log]
+ [methodical.core :as methodical]
+ [toucan2.pipeline :as t2.pipeline])
+ (:import
+ (java.io File)))
+
+(set! *warn-on-reflection* true)
+
+(def queries*
+ "An atom to store all the queries and its execution time."
+ (atom []))
+
+(defn queries
+ "Get all the queries and its execution time in ms.
+
+ Usage:
+ (queries)
+ ;; => [[[\"SELECT * FROM report_card\"] 100]]"
+ []
+ @queries*)
+
+(defn reset-queries!
+ "Reset all the queries and its execution time."
+ []
+ (reset! queries* []))
+
+(defn summary
+ "Get the total number of queries and total execution time in ms."
+ []
+ (let [qs (queries)]
+ {:total-queries (count qs)
+ :total-execution-time-ms (->> qs (map second) (apply +) int)}))
+
+(defn call-site
+ "Return first callsite inside Metabase that's not `metabase.db`"
+ []
+ (let [trace (->> (with-out-str
+ (stacktrace/print-stack-trace (Exception. "tracker")))
+ str/split-lines)]
+ (some-> (u/seek #(and (re-find #"^\s*metabase\." %)
+ (not (re-find #"^\s*metabase\.db" %))) trace)
+ str/trim)))
+
+(defn- track-query-execution-fn
+ [next-method rf conn query-type model query]
+ (let [start (System/nanoTime)
+ result (next-method rf conn query-type model query)
+ end (System/nanoTime)]
+ (swap! queries* (fnil conj []) [query (/ (- end start) 1e6) (call-site)])
+ result))
+
+(def ^:private log-thread-ref (volatile! nil))
+
+(defn- create-log-thread! []
+ (Thread.
+ (fn []
+ (while (not (Thread/interrupted))
+ (let [{:keys [total-queries total-execution-time-ms]} (summary)]
+ (log/infof "Total queries: %d, Total execution time: %dms" total-queries total-execution-time-ms)
+ (Thread/sleep 1000))))))
+
+(defn- start-log! []
+ (tu.log/set-ns-log-level! *ns* :debug)
+ (when-not (some? @log-thread-ref)
+ (let [new-thread (create-log-thread!)]
+ (vreset! log-thread-ref new-thread)
+ (.start ^Thread new-thread))))
+
+(defn- stop-log! []
+ (when-let [thread @log-thread-ref]
+ (.interrupt ^Thread thread)
+ (vreset! log-thread-ref nil)))
+
+(defn start!
+ "Start tracking queries."
+ []
+ (stop-log!)
+ (start-log!)
+ (methodical/add-aux-method-with-unique-key!
+ #'t2.pipeline/transduce-execute-with-connection
+ :around
+ :default
+ track-query-execution-fn
+ ::monitor))
+
+(defn stop!
+ "Stop tracking queries."
+ []
+ (stop-log!)
+ (methodical/remove-aux-method-with-unique-key!
+ #'t2.pipeline/transduce-execute-with-connection
+ :around
+ :default
+ ::monitor))
+
+(defn to-csv!
+ "Save all the queries and its execution time to a csv file with 3 columns: query, params, execution time."
+ []
+ (let [qs (queries)
+ format-q (fn [[q t]]
+ [(-> q first #_mdb.query/format-sql) (-> q rest vec) t])
+ temp-file (File/createTempFile "queries" ".csv")]
+ (with-open [w (io/writer temp-file)]
+ (csv/write-csv w (cons ["query" "params" "execution-time(ms)"] (map format-q qs))))
+ (dev.u/os-open temp-file)))
+
+(defn do-with-queries
+ "Implementation for [[with-queries]]."
+ [f]
+ (reset-queries!)
+ (start!)
+ (u/prog1 (f queries)
+ (stop!)))
+
+(defmacro with-queries
+ "See Toucan queries executed:
+
+ ```clj
+ (with-queries [queries]
+ (select ...)
+ (println :total (count (queries)))) ;; -> :total 1
+ ```"
+ [[queries-binding] & body]
+ `(do-with-queries (^:once fn* [~queries-binding] ~@body)))
+
+(comment
+ (start!)
+ (queries)
+ (stop!)
+ (reset-queries!)
+ (summary)
+ (to-csv!)
+ (doseq [q (querles)]
+ #_:clj-kondo/ignore
+ (println q)))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
index 63242efa4e3..0e32fddc3e4 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
@@ -427,15 +427,3 @@
(let [parameters [{:values_source_config {:card_id "foo"}}]]
(with-redefs [load/fully-qualified-name->card-id {"foo" 1}]
(is (= [1] (mapv (comp :card_id :values_source_config) (#'load/resolve-dashboard-parameters parameters)))))))
-
-(deftest with-dbs-works-as-expected-test
- (ts/with-dbs [source-db dest-db]
- (ts/with-db source-db
- (mt/with-temp
- [:model/Card _ {:name "MY CARD"}]
- (testing "card is available in the source db"
- (is (some? (t2/select-one :model/Card :name "MY CARD"))))
- (ts/with-db dest-db
- (testing "card should not be available in the dest db"
- ;; FAIL, select is returning a Card
- (is (nil? (t2/select-one :model/Card :name "MY CARD")))))))))
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 63d533f7c0a..5a1bfb243e9 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
@@ -22,6 +22,7 @@
[metabase.models.setting :as setting]
[metabase.test :as mt]
[metabase.test.generate :as test-gen]
+ [metabase.util :as u]
[metabase.util.yaml :as yaml]
[reifyhealth.specmonstah.core :as rs]
[toucan2.core :as t2]
@@ -54,6 +55,9 @@
(filter #(-> % :serdes/meta last :model (= model-name))
entities))
+(defn- by-eid [entities entity-id]
+ (u/seek #(= (:entity_id %) entity-id) entities))
+
(defn- collections [dir]
(for [coll-dir (subdirs dir)
:when (->> ["cards" "dashboards" "timelines"]
@@ -197,30 +201,7 @@
: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]})
- :pulse (vec (concat
- ;; 10 classic pulses, from collections
- (many-random-fks 10 {} {:collection_id [:coll 100]})
- ;; 10 classic pulses, no collection
- (many-random-fks 10 {:refs {:collection_id ::rs/omit}} {})
- ;; 10 dashboard subs
- (many-random-fks 10 {:refs {:collection_id ::rs/omit}}
- {:dashboard_id [:d 100]})))
- :pulse-card (vec (concat
- ;; 60 pulse cards for the classic pulses
- (many-random-fks 60 {} {:card_id [:c 100]
- :pulse_id [:pulse 10]})
- ;; 60 pulse cards connected to dashcards for the dashboard subs
- (many-random-fks 60 {} {:card_id [:c 100]
- :pulse_id [:pulse 10 20]
- :dashboard_card_id [:dc 300]})))
- :pulse-channel (vec (concat
- ;; 15 channels for the classic pulses
- (many-random-fks 15 {} {:pulse_id [:pulse 10]})
- ;; 15 channels for the dashboard subs
- (many-random-fks 15 {} {:pulse_id [:pulse 10 20]})))
- :pulse-channel-recipient (many-random-fks 40 {} {:pulse_channel_id [:pulse-channel 30]
- :user_id [:u 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
@@ -486,7 +467,7 @@
["my-db" nil "CUSTOMERS" (:name field1s)]
nil]},
:values_source_type "card"}]
- (:parameters (first (by-model extraction "Card")))))
+ (:parameters (by-eid extraction (:entity_id card2s)))))
(storage/store! (seq extraction) dump-dir)))
@@ -875,7 +856,7 @@
(let [logs (mt/with-log-messages-for-level ['metabase-enterprise :error]
(let [files (->> (#'ingest/ingest-all (io/file dump-dir))
(map (comp second second))
- (map #(.getName %))
+ (map #(.getName ^File %))
set)]
(testing "Hidden YAML wasn't read even though it's not throwing errors"
(is (not (contains? files ".hidden.yaml"))))))]
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 0d66aab9fe5..72147e76162 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
@@ -31,6 +31,7 @@
[metabase.models.action :as action]
[metabase.models.serialization :as serdes]
[metabase.test :as mt]
+ [metabase.util :as u]
[metabase.util.malli.schema :as ms]
[toucan2.core :as t2])
(:import
@@ -112,20 +113,6 @@
(is (= #{coll-eid child-eid}
(by-model "Collection" (extract/extract {:user-id 218921})))))))))
-(deftest database-test
- (mt/with-empty-h2-app-db
- (ts/with-temp-dpc [Database _ {:name "My Database"}]
- (testing "without :include-database-secrets"
- (let [extracted (extract/extract {})
- dbs (filter #(= "Database" (:model (last (serdes/path %)))) extracted)]
- (is (= 1 (count dbs)))
- (is (not-any? :details dbs))))
- (testing "with :include-database-secrets"
- (let [extracted (extract/extract {:include-database-secrets true})
- dbs (filter #(= "Database" (:model (last (serdes/path %)))) extracted)]
- (is (= 1 (count dbs)))
- (is (every? :details dbs)))))))
-
#_{:clj-kondo/ignore [:metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests]}
(deftest dashboard-and-cards-test
(mt/with-empty-h2-app-db
@@ -469,7 +456,7 @@
(set (serdes/dependencies ser)))))))
(testing "Dashboards include their Dashcards"
- (let [ser (serdes/extract-one "Dashboard" {} (t2/select-one Dashboard :id other-dash-id))]
+ (let [ser (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id other-dash-id]}))]
(is (=? {:serdes/meta [{:model "Dashboard" :id other-dash :label "dave_s_dash"}]
:entity_id other-dash
:dashcards
@@ -510,7 +497,7 @@
(set (serdes/dependencies ser)))))))
(testing "Dashboards with parameters where the source is a card"
- (let [ser (serdes/extract-one "Dashboard" {} (t2/select-one Dashboard :id param-dash-id))]
+ (let [ser (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id param-dash-id]}))]
(is (=? {:parameters
[{:id "abc"
:name "CATEGORY"
@@ -529,7 +516,7 @@
(set (serdes/dependencies ser))))))
(testing "Cards with parameters where the source is a card"
- (let [ser (serdes/extract-one "Dashboard" {} (t2/select-one Dashboard :id param-dash-id))]
+ (let [ser (u/rfirst (serdes/extract-all "Dashboard" {:where [:= :id param-dash-id]}))]
(is (=? {:parameters
[{:id "abc"
:name "CATEGORY"
@@ -582,28 +569,32 @@
(deftest dashboard-card-series-test
(mt/with-empty-h2-app-db
(ts/with-temp-dpc
- [:model/Collection {coll-id :id, coll-eid :entity_id} {:name "Some Collection"}
- :model/Card {c1-id :id, c1-eid :entity_id} {:name "Some Question", :collection_id coll-id}
- :model/Card {c2-id :id, c2-eid :entity_id} {:name "Series Question A", :collection_id coll-id}
- :model/Card {c3-id :id, c3-eid :entity_id} {:name "Series Question B", :collection_id coll-id}
- :model/Dashboard {dash-id :id, dash-eid :entity_id} {:name "Shared Dashboard", :collection_id coll-id}
- :model/DashboardCard {dc1-id :id, dc1-eid :entity_id} {:card_id c1-id, :dashboard_id dash-id}
- :model/DashboardCard {dc2-eid :entity_id} {:card_id c1-id, :dashboard_id dash-id}
- :model/DashboardCardSeries _ {:card_id c3-id, :dashboardcard_id dc1-id, :position 1}
- :model/DashboardCardSeries _ {:card_id c2-id, :dashboardcard_id dc1-id, :position 0}]
- (testing "Inlined dashcards include their series' card entity IDs"
- (let [ser (serdes/extract-one "Dashboard" {} (t2/select-one Dashboard :id dash-id))]
- (is (=? {:entity_id dash-eid
- :dashcards [{:entity_id dc1-eid, :series (mt/exactly=? [{:card_id c2-eid} {:card_id c3-eid}])}
- {:entity_id dc2-eid, :series []}]}
- ser))
+ [:model/Collection {coll-id :id, coll-eid :entity_id} {:name "Some Collection"}
+ :model/Card {c1-id :id, c1-eid :entity_id} {:name "Some Question", :collection_id coll-id}
+ :model/Card {c2-id :id, c2-eid :entity_id} {:name "Series Question A", :collection_id coll-id}
+ :model/Card {c3-id :id, c3-eid :entity_id} {:name "Series Question B", :collection_id coll-id}
+ :model/Dashboard {dash-id :id, dash-eid :entity_id} {:name "Shared Dashboard", :collection_id coll-id}
+ :model/DashboardCard {dc1-id :id, dc1-eid :entity_id} {:card_id c1-id, :dashboard_id dash-id}
+ :model/DashboardCard {dc2-eid :entity_id} {:card_id c1-id, :dashboard_id dash-id}
+ :model/DashboardCardSeries _ {:card_id c3-id, :dashboardcard_id dc1-id, :position 1}
+ :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]}))
+ (is (< (q) 13))))]
+ (is (=? {:entity_id dash-eid
+ :dashcards [{:entity_id dc1-eid
+ :series (mt/exactly=? [{:card_id c2-eid :position 0}
+ {:card_id c3-eid :position 1}])}
+ {:entity_id dc2-eid, :series []}]}
+ ser))
- (testing "and depend on all referenced cards, including cards from dashboard cards' series"
- (is (= #{[{:model "Card" :id c1-eid}]
- [{:model "Card" :id c2-eid}]
- [{:model "Card" :id c3-eid}]
- [{:model "Collection" :id coll-eid}]}
- (set (serdes/dependencies ser))))))))))
+ (testing "and depend on all referenced cards, including cards from dashboard cards' series"
+ (is (= #{[{:model "Card" :id c1-eid}]
+ [{:model "Card" :id c2-eid}]
+ [{:model "Card" :id c3-eid}]
+ [{:model "Collection" :id coll-eid}]}
+ (set (serdes/dependencies ser))))))))))
(deftest dimensions-test
(mt/with-empty-h2-app-db
@@ -635,8 +626,8 @@
:type "external"
:field_id fk-id
:human_readable_field_id cust-name}]
- (testing "dimensions without foreign keys are inlined into their Fields"
- (let [ser (serdes/extract-one "Field" {} (t2/select-one Field :id email-id))]
+ (testing "dimensions without foreign keys are inlined into their Fields\n"
+ (let [ser (u/rfirst (serdes/extract-all "Field" {:where [:= :id email-id]}))]
(is (malli= [:map
[:serdes/meta [:= [{:model "Database", :id "My Database"}
{:model "Table", :id "Schemaless Table"}
@@ -658,7 +649,7 @@
(set (serdes/dependencies ser)))))))
(testing "foreign key dimensions are inlined into their Fields"
- (let [ser (serdes/extract-one "Field" {} (t2/select-one Field :id fk-id))]
+ (let [ser (u/rfirst (serdes/extract-all "Field" {:where [:= :id fk-id]}))]
(is (malli= [:map
[:serdes/meta [:= [{:model "Database" :id "My Database"}
{:model "Schema" :id "PUBLIC"}
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 f2fd281ff14..52e299274f0 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
@@ -1,5 +1,6 @@
(ns ^:mb/once metabase-enterprise.serialization.v2.load-test
(:require
+ [clojure.string :as str]
[clojure.test :refer :all]
[java-time.api :as t]
[metabase-enterprise.serialization.test-util :as ts]
@@ -1093,7 +1094,7 @@
(reset! tab1s (ts/create! :model/DashboardTab :name "Tab 1" :dashboard_id (:id @dash1s)))
(reset! dashcard1s (ts/create! DashboardCard :dashboard_id (:id @dash1s) :dashboard_tab_id (:id tab1s)))
- (reset! serialized (into [] (serdes.extract/extract {})))))
+ (reset! serialized (into [] (serdes.extract/extract {:no-settings true})))))
(testing "New dashcard will be removed on load"
(ts/with-db dest-db
@@ -1139,7 +1140,7 @@
series1s (ts/create! :model/DashboardCardSeries :dashboardcard_id (:id dashcard1s) :card_id (:id series-card1s) :position 0)
series2s (ts/create! :model/DashboardCardSeries :dashboardcard_id (:id dashcard1s) :card_id (:id series-card2s) :position 1)
series3s (ts/create! :model/DashboardCardSeries :dashboardcard_id (:id dashcard1s) :card_id (:id series-card3s) :position 2)
- extract1 (into [] (serdes.extract/extract {}))]
+ extract1 (into [] (serdes.extract/extract {:no-settings true}))]
(ts/with-db dest-db
(serdes.load/load-metabase! (ingestion-in-memory extract1))
(ts/with-db source-db
@@ -1147,7 +1148,7 @@
(t2/delete! :model/DashboardCardSeries (:id series1s))
(t2/update! :model/DashboardCardSeries (:id series3s) {:position 0})
(t2/update! :model/DashboardCardSeries (:id series2s) {:position 1})
- (let [extract2 (into [] (serdes.extract/extract {}))]
+ (let [extract2 (into [] (serdes.extract/extract {:no-settings true :no-data-model true}))]
(ts/with-db dest-db
(let [series-card2d (t2/select-one :model/Card :entity_id (:entity_id series-card2s))
series-card3d (t2/select-one :model/Card :entity_id (:entity_id series-card3s))
@@ -1174,6 +1175,25 @@
(->> (t2/select :model/DashboardCardSeries :dashboardcard_id (:dashboardcard_id series-to-be-deleted))
(sort-by :position))))))))))))))))
+(deftest dashcard-series-multi-test
+ (ts/with-dbs [source-db dest-db]
+ (testing "Dashcard series works correctly with one card in multiple series"
+ (ts/with-db source-db
+ (mt/with-temp [:model/Dashboard dash {:name "Dashboard"}
+ :model/Card c1 {:name "Card 1"}
+ :model/Card c2 {:name "Card 2"}
+ :model/Card sc {:name "Series Card"}
+ :model/DashboardCard dc1 {:card_id (:id c1) :dashboard_id (:id dash)}
+ :model/DashboardCard dc2 {:card_id (:id c2) :dashboard_id (:id dash)}
+ :model/DashboardCardSeries _s1 {:dashboardcard_id (:id dc1) :card_id (:id sc) :position 0}
+ :model/DashboardCardSeries _s2 {:dashboardcard_id (:id dc2) :card_id (:id sc) :position 0}]
+ (let [extract (into [] (serdes.extract/extract {:no-settings true}))]
+ (ts/with-db dest-db
+ (serdes.load/load-metabase! (ingestion-in-memory extract))
+ (testing "Both series get imported even though they point at the same card"
+ (is (= 2
+ (t2/count :model/DashboardCardSeries)))))))))))
+
(deftest extraneous-keys-test
(let [serialized (atom nil)
eid (u/generate-nano-id)]
@@ -1255,12 +1275,11 @@
: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})))
+ 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)))
+ (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))))
@@ -1307,3 +1326,107 @@
(let [report (serdes.load/load-metabase! (ingestion-in-memory changed) {:continue-on-error true})]
(is (= 1 (count (:errors report))))
(is (= 3 (count (:seen report)))))))))))))))
+
+(deftest with-dbs-works-as-expected-test
+ (ts/with-dbs [source-db dest-db]
+ (ts/with-db source-db
+ (mt/with-temp
+ [:model/Card _ {:name "MY CARD"}]
+ (testing "card is available in the source db"
+ (is (some? (t2/select-one :model/Card :name "MY CARD"))))
+ (ts/with-db dest-db
+ (testing "card should not be available in the dest db"
+ (is (nil? (t2/select-one :model/Card :name "MY CARD")))))))))
+
+(deftest database-test
+ (ts/with-dbs [source-db dest-db]
+ (ts/with-db source-db
+ (mt/with-temp [Database _ {:name "My Database"
+ :details {:some "secret"}}]
+ (testing "without :include-database-secrets"
+ (let [extracted (vec (serdes.extract/extract {:no-settings true}))
+ dbs (filterv #(= "Database" (:model (last (serdes/path %)))) extracted)]
+ (is (= 1 (count dbs)))
+ (is (not-any? :details dbs))
+ (ts/with-db dest-db
+ (testing "loading still works even if there are no details"
+ (serdes.load/load-metabase! (ingestion-in-memory extracted))
+ (is (= {}
+ (t2/select-one-fn :details Database)))
+ (testing "If we did not export details - it won't override existing data"
+ (t2/update! Database {:details {:other "secret"}})
+ (serdes.load/load-metabase! (ingestion-in-memory extracted))
+ (is (= {:other "secret"}
+ (t2/select-one-fn :details Database)))))))))
+
+ (mt/with-temp [Database _ {:name "My Database"
+ :details {:some "secret"}}]
+ (testing "with :include-database-secrets"
+ (let [extracted (vec (serdes.extract/extract {:no-settings true :include-database-secrets true}))
+ dbs (filterv #(= "Database" (:model (last (serdes/path %)))) extracted)]
+ (is (= 1 (count dbs)))
+ (is (every? :details dbs))
+ (ts/with-db dest-db
+ (testing "Details are imported if provided"
+ (serdes.load/load-metabase! (ingestion-in-memory extracted))
+ (is (= (:details (first dbs))
+ (t2/select-one-fn :details Database)))))))))))
+
+(deftest unique-dimensions-test
+ (ts/with-dbs [source-db dest-db]
+ (ts/with-db source-db
+ (mt/with-temp [:model/Dimension d1 {:name "Some Dimension"
+ :field_id (mt/id :venues :price)
+ :type "internal"}]
+ (let [ser (vec (serdes.extract/extract {:no-settings true}))]
+ (ts/with-db dest-db
+ (mt/with-temp [:model/Dimension d2 {:name "Absolutely Other Dimension"
+ :field_id (mt/id :venues :price)
+ :type "internal"}]
+ (serdes.load/load-metabase! (ingestion-in-memory ser))
+ (is (= (:entity_id d1)
+ (t2/select-one-fn :entity_id :model/Dimension :field_id (mt/id :venues :price))))
+ (is (= nil
+ (t2/select-one :model/Dimension :entity_id (:entity_id d2)))))))))))
+
+(deftest nested-identity-hashes-test ;; tests serdes/nested behavior for identity hashes
+ (let [ids (atom {})]
+ (ts/with-dbs [source-db dest-db]
+ (ts/with-db source-db
+ (mt/with-temp [:model/Collection coll {:name "Coll"}
+ :model/Dashboard dash {:name "Dash"}
+ :model/Card c1 {:name "Card 1"}
+ :model/DashboardCard dc1 {:dashboard_id (:id dash)
+ :card_id (:id c1)}]
+ (testing "Store deserialized data ids in preparation for test"
+ (let [ser1 (vec (serdes.extract/extract {:no-settings true}))]
+ (ts/with-db dest-db
+ (serdes.load/load-metabase! (ingestion-in-memory ser1))
+ (reset! ids
+ (vec
+ (for [[_name e] {:coll coll :dash dash :c1 c1 :dc1 dc1}]
+ [(t2/model e) (:id (t2/select-one (t2/model e) :entity_id (:entity_id e)))]))))))
+
+ (testing "Convert everything to using identity hashes"
+ (t2/update! :model/Collection :id (:id coll) {:entity_id (serdes/identity-hash coll)})
+ (t2/update! :model/Dashboard :id (:id dash) {:entity_id (serdes/identity-hash dash)})
+ (t2/update! :model/Card :id (:id c1) {:entity_id (serdes/identity-hash c1)})
+ (t2/update! :model/DashboardCard :id (:id dc1) {:entity_id (serdes/identity-hash dc1)}))
+
+ (is (= 8 (count (serdes/entity-id "Card"
+ (t2/select-one [:model/Card :entity_id] :id (:id c1))))))
+
+ (testing "Identity hashes end up in target db in place of entity ids"
+ (let [ser2 (vec (serdes.extract/extract {:no-settings true :no-data-model true}))]
+ (testing "\nWe exported identity hashes"
+ (doseq [e ser2
+ :when (:entity_id e)]
+ (is (= 8 (count (-> e :entity_id str/trim))))))
+ (ts/with-db dest-db
+ (serdes.load/load-metabase! (ingestion-in-memory ser2))
+ (testing "\nAll entities (including nested dashcards) were updated"
+ (doseq [[model id] @ids
+ :let [e (t2/select-one model :id id)]]
+ (testing (format "%s has identity hash in the db" model)
+ (is (= (serdes/identity-hash e)
+ (serdes/entity-id (name model) e))))))))))))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj
index 7565e42d021..4baf4f9c33f 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj
@@ -1,5 +1,6 @@
(ns metabase-enterprise.serialization.v2.models-test
(:require
+ [clojure.set :as set]
[clojure.test :refer :all]
[metabase-enterprise.serialization.v2.backfill-ids :as serdes.backfill]
[metabase-enterprise.serialization.v2.entity-ids :as v2.entity-ids]
@@ -44,34 +45,36 @@
(deftest serialization-complete-spec-test
(mt/with-empty-h2-app-db
;; When serialization spec is defined, it describes every column
- (doseq [m serdes.models/exported-models
- :let [spec (serdes/make-spec m)]
- :when spec]
+ (doseq [m (-> (methods serdes/make-spec)
+ (dissoc :default)
+ keys)
+ :let [spec (serdes/make-spec m nil)]]
(let [t (t2/table-name (keyword "model" m))
fields (u.conn/app-db-column-types (mdb/app-db) t)
- spec' (merge (zipmap (:copy spec) (repeat :copy))
- (zipmap (:skip spec) (repeat :skip))
- (:transform spec))]
- (testing (format "%s should declare every column in serialization spec" m)
- (is (= (->> (keys fields)
- (map u/lower-case-en)
- set)
- (->> (keys spec')
- (map name)
- set))))
+ spec' (-> (merge (zipmap (:copy spec) (repeat :copy))
+ (zipmap (:skip spec) (repeat :skip))
+ (zipmap [:id :updated_at] (repeat :skip)) ; always skipped
+ (:transform spec))
+ ;; `nil`s are mostly fields which differ on `opts`
+ (dissoc nil))]
+ (testing (format "%s has no duplicates in serialization spec\n" m)
+ (are [x y] (empty? (set/intersection (set x) (set y)))
+ (:copy spec) (:skip spec)
+ (:copy spec) (keys (:transform spec))
+ (:skip spec) (keys (:transform spec))))
+ (testing (format "%s should declare every column in serialization spec\n" m)
+ (is (set/subset?
+ (->> (keys fields)
+ (map u/lower-case-en)
+ set)
+ (->> (keys spec')
+ (map name)
+ set))))
(testing "Foreign keys should be declared as such\n"
(doseq [[fk _] (filter #(:fk (second %)) fields)
- :let [fk (u/lower-case-en fk)
- action (get spec' (keyword fk))]]
+ :let [fk (u/lower-case-en fk)
+ transform (get spec' (keyword fk))]
+ :when (not= transform :skip)]
(testing (format "%s.%s is foreign key which is handled correctly" m fk)
- ;; FIXME: serialization can guess where FK points by itself, but `collection_id` and `database_id` are
- ;; specifying that themselves right now
- (when-not (#{"collection_id" "database_id"} fk)
- (is (#{:skip
- serdes/*export-fk*
- serdes/*export-fk-keyed*
- serdes/*export-table-fk*
- serdes/*export-user*}
- (if (vector? action)
- (first action) ;; tuple of [ser des]
- action)))))))))))
+ ;; uses `(serdes/fk ...)` function
+ (is (::serdes/fk transform)))))))))
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 1ce16ae1769..cafea4d0a17 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -1023,38 +1023,27 @@ saved later when it is ready."
(when (:id m) #{(serdes/field->path (:id m))})])))))
(defmethod serdes/make-spec "Card"
- [_model-name]
- {:copy [:archived :collection_position :collection_preview :created_at :description :display
+ [_model-name _opts]
+ {:copy [:archived :archived_directly :collection_position :collection_preview :created_at :description :display
:embedding_params :enable_embedding :entity_id :metabase_version :public_uuid :query_type :type :name]
- :skip [ ;; always instance-specific
- :id :updated_at
- ;; cache invalidation is instance-specific
+ :skip [;; cache invalidation is instance-specific
:cache_invalidated_at
;; those are instance-specific analytic columns
:view_count :last_used_at :initially_published_at
;; this column is not used anymore
:cache_ttl]
:transform
- {:database_id [#(serdes/*export-fk-keyed* % 'Database :name)
- #(serdes/*import-fk-keyed* % 'Database :name)]
- :table_id [serdes/*export-table-fk*
- serdes/*import-table-fk*]
- :collection_id [#(serdes/*export-fk* % 'Collection)
- #(serdes/*import-fk* % 'Collection)]
- :creator_id [serdes/*export-user*
- serdes/*import-user*]
- :made_public_by_id [serdes/*export-user*
- serdes/*import-user*]
- :dataset_query [serdes/export-mbql
- serdes/import-mbql]
- :parameters [serdes/export-parameters
- serdes/import-parameters]
- :parameter_mappings [serdes/export-parameter-mappings
- serdes/import-parameter-mappings]
- :visualization_settings [serdes/export-visualization-settings
- serdes/import-visualization-settings]
- :result_metadata [export-result-metadata
- import-result-metadata]}})
+ {:database_id (serdes/fk :model/Database :name)
+ :table_id (serdes/fk :model/Table)
+ :source_card_id (serdes/fk :model/Card)
+ :collection_id (serdes/fk :model/Collection)
+ :creator_id (serdes/fk :model/User)
+ :made_public_by_id (serdes/fk :model/User)
+ :dataset_query {:export serdes/export-mbql :import serdes/import-mbql}
+ :parameters {:export serdes/export-parameters :import serdes/import-parameters}
+ :parameter_mappings {:export serdes/export-parameter-mappings :import serdes/import-parameter-mappings}
+ :visualization_settings {:export serdes/export-visualization-settings :import serdes/import-visualization-settings}
+ :result_metadata {:export export-result-metadata :import import-result-metadata}}})
(defmethod serdes/dependencies "Card"
[{:keys [collection_id database_id dataset_query parameters parameter_mappings
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 37039182aa3..f33bb73075d 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -578,107 +578,34 @@
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | SERIALIZATION |
;;; +----------------------------------------------------------------------------------------------------------------+
+
(defmethod serdes/extract-query "Dashboard" [_ opts]
- (eduction (map #(t2/hydrate % [:dashcards :series]))
+ (eduction (map #(t2/hydrate % :tabs [:dashcards :series]))
(serdes/extract-query-collections Dashboard opts)))
-(defn export-dashboard-card-series
- "Given the hydrated `:series` of a DashboardCard, as a vector of maps, converts it to a portable form with
- the card IDs replaced with their entity IDs."
- [cards]
- (mapv (fn [card]
- {:card_id (serdes/*export-fk* (:id card) :model/Card)})
- cards))
-
-(defn- extract-dashcard
- [dashcard]
- (-> (into (sorted-map) dashcard)
- (dissoc :id :collection_authority_level :dashboard_id :updated_at)
- (update :card_id serdes/*export-fk* 'Card)
- (update :action_id serdes/*export-fk* 'Action)
- (update :dashboard_tab_id serdes/*export-fk* :model/DashboardTab)
- (update :series export-dashboard-card-series)
- (update :parameter_mappings serdes/export-parameter-mappings)
- (update :visualization_settings serdes/export-visualization-settings)))
-
-(defn- extract-dashtab
- [dashtab]
- (dissoc dashtab :id :dashboard_id :updated_at))
-
-(defmethod serdes/extract-one "Dashboard"
- [_model-name _opts dash]
- (let [dash (cond-> dash
- (nil? (:dashcards dash))
- (t2/hydrate [:dashcards :series])
- (nil? (:tabs dash))
- (t2/hydrate :tabs))]
- (-> (serdes/extract-one-basics "Dashboard" dash)
- (update :dashcards #(mapv extract-dashcard %))
- (update :tabs #(mapv extract-dashtab %))
- (update :parameters serdes/export-parameters)
- (update :collection_id serdes/*export-fk* Collection)
- (update :creator_id serdes/*export-user*)
- (update :made_public_by_id serdes/*export-user*)
- (dissoc :view_count))))
-
-(defmethod serdes/load-xform "Dashboard"
- [dash]
- (-> dash
- serdes/load-xform-basics
- ;; Deliberately not doing anything to :dashcards - they get handled by load-one! below.
- (update :collection_id serdes/*import-fk* Collection)
- (update :parameters serdes/import-parameters)
- (update :creator_id serdes/*import-user*)
- (update :made_public_by_id serdes/*import-user*)))
-
-(defn- dashcard-for [dashcard dashboard]
- (assoc dashcard
- :dashboard_id (:entity_id dashboard)
- :serdes/meta (remove nil?
- [{:model "Dashboard" :id (:entity_id dashboard)}
- (when-let [dashtab-eeid (last (:dashboard_tab_id dashcard))]
- {:model "DashboardTab" :id dashtab-eeid})
- {:model "DashboardCard" :id (:entity_id dashcard)}])))
-
-(defn- dashtab-for [tab dashboard]
- (assoc tab
- :dashboard_id (:entity_id dashboard)
- :serdes/meta [{:model "Dashboard" :id (:entity_id dashboard)}
- {:model "DashboardTab" :id (:entity_id tab)}]))
-
-(defn- drop-excessive-nested!
- "Remove nested entities which are not present in incoming serialization load"
- [hydration-key ingested local]
- (let [local-nested (get (t2/hydrate local hydration-key) hydration-key)
- ingested-nested (get ingested hydration-key)
- to-remove (set/difference (set (map :entity_id local-nested))
- (set (map :entity_id ingested-nested)))
- model (t2/model (first local-nested))]
- (when (seq to-remove)
- (t2/delete! model :entity_id [:in to-remove]))))
-
-;; Call the default load-one! for the Dashboard, then for each DashboardCard.
-(defmethod serdes/load-one! "Dashboard" [ingested maybe-local]
- (let [dashboard ((get-method serdes/load-one! :default) (dissoc ingested :dashcards :tabs) maybe-local)]
-
- (drop-excessive-nested! :tabs ingested dashboard)
- (doseq [tab (:tabs ingested)]
- (serdes/load-one! (dashtab-for tab dashboard)
- (t2/select-one :model/DashboardTab :entity_id (:entity_id tab))))
-
- (drop-excessive-nested! :dashcards ingested dashboard)
- (doseq [dashcard (:dashcards ingested)]
- (serdes/load-one! (dashcard-for dashcard dashboard)
- (t2/select-one :model/DashboardCard :entity_id (:entity_id dashcard))))))
+(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
+ :points_of_interest :position :public_uuid :show_in_getting_started :width]
+ :skip [;; those stats are inherently local state
+ :view_count :last_viewed_at]
+ :transform {:created_at (serdes/date)
+ :collection_id (serdes/fk :model/Collection)
+ :creator_id (serdes/fk :model/User)
+ :made_public_by_id (serdes/fk :model/User)
+ :parameters {:export serdes/export-parameters :import serdes/import-parameters}
+ :tabs (serdes/nested :model/DashboardTab :dashboard_id opts)
+ :dashcards (serdes/nested :model/DashboardCard :dashboard_id opts)}})
(defn- serdes-deps-dashcard
[{:keys [action_id card_id parameter_mappings visualization_settings series]}]
- (->> (mapcat serdes/mbql-deps parameter_mappings)
- (concat (serdes/visualization-settings-deps visualization_settings))
- (concat (when card_id #{[{:model "Card" :id card_id}]}))
- (concat (when action_id #{[{:model "Action" :id action_id}]}))
- (concat (for [s series] [{:model "Card" :id (:card_id s)}]))
- set))
+ (set
+ (concat
+ (mapcat serdes/mbql-deps parameter_mappings)
+ (serdes/visualization-settings-deps visualization_settings)
+ (when card_id #{[{:model "Card" :id card_id}]})
+ (when action_id #{[{:model "Action" :id action_id}]})
+ (for [s series] [{:model "Card" :id (:card_id s)}]))))
(defmethod serdes/dependencies "Dashboard"
[{:keys [collection_id dashcards parameters]}]
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index 354eab1fa67..509361901e9 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -9,7 +9,6 @@
[metabase.models.pulse-card :refer [PulseCard]]
[metabase.models.serialization :as serdes]
[metabase.util :as u]
- [metabase.util.date-2 :as u.date]
[metabase.util.honey-sql-2 :as h2x]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
@@ -353,9 +352,7 @@
(compare row-1 row-2)))
;;; ----------------------------------------------- SERIALIZATION ----------------------------------------------------
-;; DashboardCards are not serialized as their own, separate entities. They are inlined onto their parent Dashboards.
-;; If the parent dashboard has tabs, the dashcards are inlined under each DashboardTab, which are inlined on the Dashboard.
-;; However, we can reuse some of the serdes machinery (especially load-one!) by implementing a few serdes methods.
+
(defmethod serdes/generate-path "DashboardCard" [_ dashcard]
(remove nil?
[(serdes/infer-self-path "Dashboard" (t2/select-one 'Dashboard :id (:dashboard_id dashcard)))
@@ -363,35 +360,27 @@
(serdes/infer-self-path "DashboardTab" (t2/select-one :model/DashboardTab :id (:dashboard_tab_id dashcard))))
(serdes/infer-self-path "DashboardCard" dashcard)]))
-(defmethod serdes/load-xform "DashboardCard"
- [dashcard]
- (-> dashcard
- ;; Deliberately not doing anything to :series, they get handled by load-one! below
- (dissoc :serdes/meta)
- (update :card_id serdes/*import-fk* :model/Card)
- (update :action_id serdes/*import-fk* :model/Action)
- (update :dashboard_id serdes/*import-fk* :model/Dashboard)
- (update :dashboard_tab_id serdes/*import-fk* :model/DashboardTab)
- (update :created_at #(if (string? %) (u.date/parse %) %))
- (update :parameter_mappings serdes/import-parameter-mappings)
- (update :visualization_settings serdes/import-visualization-settings)))
-
-(defn- dashboard-card-series-xform
- [ingested]
- (-> ingested
- (update :card_id serdes/*import-fk* :model/Card)
- (update :dashboardcard_id serdes/*import-fk* :model/DashboardCard)))
-
-(defmethod serdes/load-one! "DashboardCard"
- [ingested maybe-local]
- (let [dashcard ((get-method serdes/load-one! :default) (dissoc ingested :series) maybe-local)]
- ;; drop all existing series for this card and recreate them
- ;; TODO: this is unnecessary, but it is simple to implement
- (t2/delete! :model/DashboardCardSeries :dashboardcard_id (:id dashcard))
- (doseq [[idx single-series] (map-indexed vector (:series ingested))] ;; a single series has a :card_id only
- ;; instead of load-one! we use load-insert! here because :serdes/meta isn't necessary because no other
- ;; entities depend on DashboardCardSeries
- (serdes/load-insert! "DashboardCardSeries" (-> single-series
- (assoc :dashboardcard_id (:entity_id dashcard)
- :position idx)
- dashboard-card-series-xform)))))
+(defmethod serdes/make-spec "DashboardCard" [_model-name opts]
+ {:copy [:col :entity_id :row :size_x :size_y]
+ :skip []
+ :transform {:created_at (serdes/date)
+ :dashboard_id (serdes/parent-ref)
+ :card_id (serdes/fk :model/Card)
+ :action_id (serdes/fk :model/Action)
+ :dashboard_tab_id (serdes/fk :model/DashboardTab)
+ :parameter_mappings {:export serdes/export-parameter-mappings
+ :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)))))}})
diff --git a/src/metabase/models/dashboard_card_series.clj b/src/metabase/models/dashboard_card_series.clj
index 8d818e6a989..129c4d0aecb 100644
--- a/src/metabase/models/dashboard_card_series.clj
+++ b/src/metabase/models/dashboard_card_series.clj
@@ -1,5 +1,6 @@
(ns metabase.models.dashboard-card-series
(:require
+ [metabase.models.serialization :as serdes]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -12,3 +13,17 @@
(doto :model/DashboardCardSeries
(derive :metabase/model))
+
+;; Serialization
+
+(defmethod serdes/generate-path "DashboardCardSeries" [_ _] nil)
+
+;; TODO: this is not used atm as `DashboardCard` has custom :export/:import defined; see comment there
+;; to be implemented.
+(defmethod serdes/make-spec "DashboardCardSeries" [_model-name _opts]
+ ;; We did not have `position` in serialization before, it was inferred from the order, but we're trying to keep
+ ;; code more generic right now - so it's carried over as data rather than implied.
+ {:copy [:position]
+ :skip []
+ :transform {:dashboardcard_id (serdes/parent-ref)
+ :card_id (serdes/fk :model/Card)}})
diff --git a/src/metabase/models/dashboard_tab.clj b/src/metabase/models/dashboard_tab.clj
index 8b60e6602fc..58818d13e5f 100644
--- a/src/metabase/models/dashboard_tab.clj
+++ b/src/metabase/models/dashboard_tab.clj
@@ -5,7 +5,6 @@
[metabase.models.interface :as mi]
[metabase.models.serialization :as serdes]
[metabase.util :as u]
- [metabase.util.date-2 :as u.date]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[methodical.core :as methodical]
@@ -59,17 +58,15 @@
:position
:created_at])
-;; DashboardTabs are not serialized as their own, separate entities. They are inlined onto their parent Dashboards.
(defmethod serdes/generate-path "DashboardTab" [_ dashcard]
[(serdes/infer-self-path "Dashboard" (t2/select-one :model/Dashboard :id (:dashboard_id dashcard)))
(serdes/infer-self-path "DashboardTab" dashcard)])
-(defmethod serdes/load-xform "DashboardTab"
- [dashtab]
- (-> dashtab
- (dissoc :serdes/meta)
- (update :dashboard_id serdes/*import-fk* :model/Dashboard)
- (update :created_at #(if (string? %) (u.date/parse %) %))))
+(defmethod serdes/make-spec "DashboardTab" [_model-name _opts]
+ {:copy [:entity_id :name :position]
+ :skip []
+ :transform {:created_at (serdes/date)
+ :dashboard_id (serdes/parent-ref)}})
;;; -------------------------------------------------- CRUD fns ------------------------------------------------------
diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj
index aa2c55b55bd..c92286326f7 100644
--- a/src/metabase/models/database.clj
+++ b/src/metabase/models/database.clj
@@ -432,12 +432,17 @@
;;; ------------------------------------------------ Serialization ----------------------------------------------------
-(defmethod serdes/extract-one "Database"
- [_model-name {:keys [include-database-secrets]} entity]
- (-> (serdes/extract-one-basics "Database" entity)
- (update :creator_id serdes/*export-user*)
- (dissoc :features) ; This is a synthetic column that isn't in the real schema.
- (cond-> (not include-database-secrets) (dissoc :details))))
+(defmethod serdes/make-spec "Database"
+ [_model-name {:keys [include-database-secrets]}]
+ {:copy [:auto_run_queries :cache_field_values_schedule :cache_ttl :caveats :created_at :dbms_version
+ :description :engine :is_audit :is_full_sync :is_on_demand :is_sample :metadata_sync_schedule :name
+ :points_of_interest :refingerprint :settings :timezone :uploads_enabled :uploads_schema_name
+ :uploads_table_prefix]
+ :skip []
+ :transform {;; details should be imported if available regardless of options
+ :details {:export #(when include-database-secrets %) :import identity}
+ :creator_id (serdes/fk :model/User)
+ :initial_sync_status {:export identity :import (constantly "complete")}}})
(defmethod serdes/entity-id "Database"
[_ {:keys [name]}]
@@ -451,26 +456,6 @@
[[{:keys [id]}]]
(t2/select-one Database :name id))
-(defmethod serdes/load-xform "Database"
- [database]
- (-> database
- serdes/load-xform-basics
- (update :creator_id serdes/*import-user*)
- (assoc :initial_sync_status "complete")))
-
-(defmethod serdes/load-insert! "Database" [_ ingested]
- (let [m (get-method serdes/load-insert! :default)]
- (m "Database"
- (if (:details ingested)
- ingested
- (assoc ingested :details {})))))
-
-(defmethod serdes/load-update! "Database" [_ ingested local]
- (let [m (get-method serdes/load-update! :default)]
- (m "Database"
- (update ingested :details #(or % (:details local) {}))
- local)))
-
(defmethod serdes/storage-path "Database" [{:keys [name]} _]
;; ["databases" "db_name" "db_name"] directory for the database with same-named file inside.
["databases" name name])
diff --git a/src/metabase/models/dimension.clj b/src/metabase/models/dimension.clj
index 81c4bb8afbc..1f6e8a8f9f7 100644
--- a/src/metabase/models/dimension.clj
+++ b/src/metabase/models/dimension.clj
@@ -5,7 +5,6 @@
(:require
[metabase.models.interface :as mi]
[metabase.models.serialization :as serdes]
- [metabase.util.date-2 :as u.date]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -36,12 +35,10 @@
:created_at])
;;; ------------------------------------------------- Serialization --------------------------------------------------
-;; Dimensions are inlined onto their parent Fields.
-;; We can reuse the [[serdes/load-one!]] logic by implementing [[serdes/load-xform]] though.
-(defmethod serdes/load-xform "Dimension"
- [dim]
- (-> dim
- serdes/load-xform-basics
- ;; No need to handle :field_id, it was just added as the raw ID by the caller; see Field's load-one!
- (update :human_readable_field_id serdes/*import-field-fk*)
- (update :created_at u.date/parse)))
+
+(defmethod serdes/make-spec "Dimension" [_model-name _opts]
+ {:copy [:name :type :entity_id]
+ :skip []
+ :transform {:created_at (serdes/date)
+ :human_readable_field_id (serdes/fk :model/Field)
+ :field_id (serdes/parent-ref)}})
diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj
index a866f6d8890..17ff8800eec 100644
--- a/src/metabase/models/field.clj
+++ b/src/metabase/models/field.clj
@@ -372,12 +372,17 @@
(defmethod serdes/entity-id "Field" [_ {:keys [name]}]
name)
-(defmethod serdes/extract-query "Field" [_model-name _opts]
- (let [d (t2/select Dimension)
+(defmethod serdes/load-find-local "Field"
+ [path]
+ (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))))
+ (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.
@@ -393,49 +398,21 @@
fks (set/union #{fks})
true (disj this))))
-(defn- extract-dimensions [dimensions]
- (->> (for [dim dimensions]
- (-> (into (sorted-map) dim)
- (dissoc :field_id :updated_at) ; :field_id is implied by the nesting under that field.
- (update :human_readable_field_id serdes/*export-field-fk*)))
- (sort-by :created_at)))
-
-(defmethod serdes/extract-one "Field"
- [_model-name _opts field]
- (let [field (if (contains? field :dimensions)
- field
- (assoc field :dimensions (t2/select Dimension :field_id (:id field))))]
- (-> (serdes/extract-one-basics "Field" field)
- (update :dimensions extract-dimensions)
- (update :table_id serdes/*export-table-fk*)
- (update :fk_target_field_id serdes/*export-field-fk*)
- (update :parent_id serdes/*export-field-fk*)
- (dissoc :fingerprint :last_analyzed :fingerprint_version))))
-
-(defmethod serdes/load-xform "Field"
- [field]
- (-> (serdes/load-xform-basics field)
- (update :table_id serdes/*import-table-fk*)
- (update :fk_target_field_id serdes/*import-field-fk*)
- (update :parent_id serdes/*import-field-fk*)))
-
-(defmethod serdes/load-find-local "Field"
- [path]
- (let [table (serdes/load-find-local (pop path))]
- (t2/select-one Field :name (-> path last :id) :table_id (:id table))))
-
-(defmethod serdes/load-one! "Field" [ingested maybe-local]
- (let [field ((get-method serdes/load-one! :default) (dissoc ingested :dimensions) maybe-local)]
- (doseq [dim (:dimensions ingested)]
- (let [local (t2/select-one Dimension :entity_id (:entity_id dim))
- dim (assoc dim
- :field_id (:id field)
- :serdes/meta [{:model "Dimension" :id (:entity_id dim)}])]
- (serdes/load-one! dim local)))))
+(defmethod serdes/make-spec "Field" [_model-name opts]
+ {:copy [:active :base_type :caveats :coercion_strategy :custom_position :database_indexed
+ :database_is_auto_increment :database_partitioned :database_position :database_required :database_type
+ :description :display_name :effective_type :has_field_values :is_defective_duplicate :json_unfolding
+ :name :nfc_path :points_of_interest :position :preview_display :semantic_type :settings
+ :unique_field_helper :visibility_type]
+ :skip [:fingerprint :fingerprint_version :last_analyzed]
+ :transform {:created_at (serdes/date)
+ :table_id (serdes/fk :model/Table)
+ :fk_target_field_id (serdes/fk :model/Field)
+ :parent_id (serdes/fk :model/Field)
+ :dimensions (serdes/nested :model/Dimension :field_id opts)}})
(defmethod serdes/storage-path "Field" [field _]
- (-> field
- serdes/path
+ (-> (serdes/path field)
drop-last
serdes/storage-table-path-prefix
(concat ["fields" (:name field)])))
diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj
index c2db5a5bca9..652d8360059 100644
--- a/src/metabase/models/field_values.clj
+++ b/src/metabase/models/field_values.clj
@@ -559,6 +559,9 @@
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Serialization |
;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defmethod serdes/entity-id "FieldValues" [_ _] nil)
+
(defmethod serdes/generate-path "FieldValues" [_ {:keys [field_id]}]
(let [field (t2/select-one 'Field :id field_id)]
(conj (serdes/generate-path "Field" field)
diff --git a/src/metabase/models/serialization.clj b/src/metabase/models/serialization.clj
index 74dfeed3495..3485e1dd5eb 100644
--- a/src/metabase/models/serialization.clj
+++ b/src/metabase/models/serialization.clj
@@ -24,9 +24,11 @@
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.util :as u]
[metabase.util.connection :as u.conn]
+ [metabase.util.date-2 :as u.date]
[metabase.util.log :as log]
[toucan2.core :as t2]
- [toucan2.model :as t2.model]))
+ [toucan2.model :as t2.model]
+ [toucan2.realize :as t2.realize]))
(set! *warn-on-reflection* true)
@@ -69,6 +71,8 @@
;;; - For entities that existed before the column was added, have a portable way to rebuild them (see below on
;;; hashing).
+(def ^:private ^:dynamic *current* "Instance/map being exported/imported currently" nil)
+
(defmulti entity-id
"Given the model name and an entity, returns its entity ID (which might be nil).
@@ -79,8 +83,8 @@
{:arglists '([model-name instance])}
(fn [model-name _instance] model-name))
-(defmethod entity-id :default [_ {:keys [entity_id]}]
- (str/trim entity_id))
+(defmethod entity-id :default [_ instance]
+ (some-> instance :entity_id str/trim))
(defn eid->id
"Given model name and its entity id, returns it database-local id.
@@ -193,7 +197,9 @@
`(generate-path \"ModelName\" entity)`
The path is a vector of maps, root first and this entity itself last. Each map looks like:
- `{:model \"ModelName\" :id \"entity ID, identity hash, or custom ID\" :label \"optional human label\"}`"
+ `{:model \"ModelName\" :id \"entity ID, identity hash, or custom ID\" :label \"optional human label\"}`
+
+ Nested models with no entity_id need to return nil for generate-path."
{:arglists '([model-name instance])}
(fn [model-name _instance] model-name))
@@ -214,13 +220,12 @@
For example, a Card's or Dashboard's `:name` field."
[model-name entity slug-key]
(let [self (infer-self-path model-name entity)
- label (get entity slug-key)]
- [(if label
- (assoc self :label (u/slugify label {:unicode? true}))
- self)]))
+ label (slug-key entity)]
+ [(-> self
+ (m/assoc-some :label (some-> label (u/slugify {:unicode? true}))))]))
(defmethod generate-path :default [model-name entity]
- ;; This default works for most models, but needs overriding for nested ones.
+ ;; This default works for most models, but needs overriding for those that don't rely on entity_id.
(maybe-labeled model-name entity :name))
;;; # Export Process
@@ -311,16 +316,27 @@
`:transform` is a map from field name to a `{:ser (fn [v] ...) :des (fn [v] ...)}` map with functions to
serialize/deserialize data.
- For behavior, see `extract-by-spec` and `load-by-spec`."
- (fn [model-name] model-name))
+ For behavior, see `extract-by-spec` and `xform-by-spec`."
+ (fn [model-name _opts] model-name))
-(defmethod make-spec :default [_] nil)
+(defmethod make-spec :default [_ _] nil)
-(defn- extract-by-spec [model-name _opts instance]
- (when-let [spec (make-spec model-name)]
- (into (select-keys instance (:copy spec))
- (for [[k [ser _des]] (:transform spec)]
- [k (ser (get instance k))]))))
+(defn- extract-by-spec [model-name opts instance]
+ (try
+ (binding [*current* instance]
+ (when-let [spec (make-spec model-name opts)]
+ (-> (select-keys instance (:copy spec))
+ ;; won't assoc if `generate-path` returned `nil`
+ (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
+ :when res]
+ [k res])))))
+ (catch Exception e
+ (throw (ex-info (format "Error extracting %s %s" model-name (:id instance))
+ (assoc (ex-data e) :model model-name :id (:id instance))
+ e)))))
(defmulti extract-all
"Entry point for extracting all entities of a particular model:
@@ -372,7 +388,7 @@
(defn log-and-extract-one
"Extracts a single entity; will replace `extract-one` as public interface once `extract-one` overrides are gone."
[model opts instance]
- (log/infof "Extracting %s %s" model (:id instance))
+ (log/infof "Extracting %s %s %s" model (:id instance) (entity-id model instance))
(try
(extract-one model opts instance)
(catch Exception e
@@ -396,18 +412,20 @@
(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]}]
+ [model {:keys [collection-set where]}]
(if collection-set
;; If collection-set is defined, select everything in those collections, or with nil :collection_id.
- (let [in-colls (t2/reducible-select model :collection_id [:in collection-set])]
- (if (contains? collection-set nil)
- (eduction cat [in-colls (t2/reducible-select model :collection_id nil)])
- in-colls))
+ (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)))
+ (t2/reducible-select model {:where (or where true)})))
-(defmethod extract-query :default [model-name _]
- (t2/reducible-select (symbol model-name)))
+(defmethod extract-query :default [model-name {:keys [where]}]
+ (t2/reducible-select (symbol model-name) {:where (or where true)}))
(defn extract-one-basics
"A helper for writing [[extract-one]] implementations. It takes care of the basics:
@@ -427,8 +445,7 @@
(defmethod extract-one :default [model-name opts entity]
;; `extract-by-spec` is called here since most of tests use `extract-one` right now
- (or (some-> (extract-by-spec model-name opts entity)
- (assoc :serdes/meta (generate-path model-name entity)))
+ (or (extract-by-spec model-name opts entity)
(extract-one-basics model-name entity)))
(defmulti descendants
@@ -680,24 +697,36 @@
(fn [ingested _]
(ingested-model ingested)))
-(defn- load-by-spec [ingested]
- (let [model-name (ingested-model ingested)
- spec (make-spec model-name)]
+(defn- xform-by-spec [model-name ingested]
+ (let [spec (make-spec model-name nil)]
(when spec
- (into (select-keys ingested (:copy spec))
- (for [[k [_ser des]] (:transform spec)]
- [k (des (get ingested k))])))))
+ (-> (select-keys ingested (:copy spec))
+ (into (for [[k transform] (:transform spec)
+ :when (not (::nested transform))
+ :let [res ((:import transform) (get ingested k))]
+ ;; do not try to insert nil values if transformer returns nothing
+ :when res]
+ [k res]))))))
+
+(defn- spec-nested! [model-name ingested instance]
+ (binding [*current* instance]
+ (let [spec (make-spec model-name nil)]
+ (doseq [[k transform] (:transform spec)
+ :when (::nested transform)]
+ ((:import transform) (get ingested k))))))
(defn default-load-one!
"Default implementation of `load-one!`"
[ingested maybe-local]
- (let [model (ingested-model ingested)
- adjusted (or (load-by-spec ingested)
- (load-xform ingested))]
- (binding [mi/*deserializing?* true]
- (if (nil? maybe-local)
- (load-insert! model adjusted)
- (load-update! model adjusted maybe-local)))))
+ (let [model-name (ingested-model ingested)
+ adjusted (or (xform-by-spec model-name ingested)
+ (load-xform ingested))
+ instance (binding [mi/*deserializing?* true]
+ (if (nil? maybe-local)
+ (load-insert! model-name adjusted)
+ (load-update! model-name adjusted maybe-local)))]
+ (spec-nested! model-name ingested instance)
+ instance))
(defmethod load-one! :default [ingested maybe-local]
(default-load-one! ingested maybe-local))
@@ -719,6 +748,7 @@
[model id-hash]
(->> (t2/reducible-select model)
(into [] (comp (filter #(= id-hash (identity-hash %)))
+ (map t2.realize/realize)
(take 1)))
first))
@@ -800,7 +830,6 @@
[id model]
(when id
(let [model-name (name model)
- model (t2.model/resolve-model (symbol model-name))
entity (t2/select-one model (first (t2/primary-keys model)) id)
path (when entity
(mapv :id (generate-path model-name entity)))]
@@ -824,12 +853,10 @@
Unusual parameter order means this can be used as `(update x :some_id import-fk 'SomeModel)`."
[eid model]
(when eid
- (let [model-name (name model)
- model (t2.model/resolve-model (symbol model-name))
- eid (if (vector? eid)
- (last eid)
- eid)
- entity (lookup-by-id model eid)]
+ (let [eid (if (vector? eid)
+ (last eid)
+ eid)
+ entity (lookup-by-id model eid)]
(if entity
(get entity (first (t2/primary-keys model)))
(throw (ex-info "Could not find foreign key target - bad serdes dependencies or other serialization error"
@@ -1486,6 +1513,80 @@
(set/union (viz-click-behavior-descendants viz)
(viz-column-settings-descendants viz)))
+;;; Common transformers
+
+(defn fk "Export Foreign Key" [model & [field-name]]
+ (cond
+ ;; this `::fk` is used in tests to determine that foreign keys are handled
+ (= model :model/User) {::fk true :export *export-user* :import *import-user*}
+ (= model :model/Table) {::fk true :export *export-table-fk* :import *import-table-fk*}
+ (= model :model/Field) {::fk true :export *export-field-fk* :import *import-field-fk*}
+ field-name {::fk true
+ :export #(*export-fk-keyed* % model field-name)
+ :import #(*import-fk-keyed* % model field-name)}
+ :else {::fk true :export #(*export-fk* % model) :import #(*import-fk* % model)}))
+
+(defn nested "Nested entities" [model backward-fk opts]
+ (let [model-name (name model)
+ sorter (:sort-by opts :created_at)
+ key-field (:key-field opts :entity_id)]
+ {::nested true
+ :model model
+ :backward-fk backward-fk
+ :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)
+ (mapv #(extract-one model-name opts %)))
+ (catch Exception e
+ (throw (ex-info (format "Error exporting nested %s" model)
+ {:model model
+ :parent-id (:id *current*)}
+ e)))))
+ :import (fn [lst]
+ (let [parent-id (:id *current*)
+ first-eid (some->> (first lst)
+ (entity-id model-name))
+ enrich (fn [ingested]
+ (-> ingested
+ (assoc backward-fk parent-id)
+ (update :serdes/meta #(or % [{:model model-name :id (get ingested key-field)}]))))]
+ (cond
+ (nil? first-eid) ; no entity id, just drop existing stuff
+ (do (t2/delete! model backward-fk parent-id)
+ (doseq [ingested lst]
+ (load-one! (enrich ingested) nil)))
+
+ (entity-id? first-eid) ; proper entity id, match by them
+ (do (t2/delete! model backward-fk parent-id :entity_id [:not-in (map :entity_id lst)])
+ (doseq [ingested lst
+ :let [ingested (enrich ingested)
+ local (lookup-by-id model (entity-id model-name ingested))]]
+ (load-one! ingested local)))
+
+ :else ; identity hash
+ (let [incoming (set (map #(entity-id model-name %) lst))
+ local (->> (t2/reducible-select model backward-fk parent-id)
+ (into [] (map t2.realize/realize))
+ (m/index-by identity-hash))
+ to-delete (into [] (comp (filter #(contains? incoming (key %)))
+ (map #(:id (val %))))
+ local)]
+ (t2/delete! model :id [:in (map :id to-delete)])
+ (doseq [ingested lst]
+ (load-one! (enrich ingested) (get local (entity-id model-name ingested))))))))}))
+
+(defn parent-ref "Transformer for parent id for nested entities" []
+ {::fk true :export (constantly nil) :import identity})
+
+(defn date "Transformer to parse the dates" []
+ {:export identity :import #(if (string? %) (u.date/parse %) %)})
+
;;; ## Memoizing appdb lookups
(defmacro with-cache
diff --git a/src/metabase/util.cljc b/src/metabase/util.cljc
index d3e5d8bd832..8faa19c70e6 100644
--- a/src/metabase/util.cljc
+++ b/src/metabase/util.cljc
@@ -1017,3 +1017,8 @@
(let [buf (js/Uint8Array. max-length-bytes)
result (.encodeInto (js/TextEncoder.) s buf)] ;; JS obj {read: chars_converted, write: bytes_written}
(subs s 0 (.-read result)))))
+
+(defn rfirst
+ "Return first item from Reducible"
+ [reducible]
+ (reduce (fn [_ fst] (reduced fst)) nil reducible))
diff --git a/test/metabase/test/generate.clj b/test/metabase/test/generate.clj
index 7389361bfc9..e51a717d2f0 100644
--- a/test/metabase/test/generate.clj
+++ b/test/metabase/test/generate.clj
@@ -170,7 +170,8 @@
(s/def ::pulse-card (s/keys :req-un [::id ::position]))
(s/def ::channel_type ::not-empty-string)
-(s/def ::schedule_type ::not-empty-string)
+(s/def ::schedule_type (s/with-gen (s/and string? #(contains? #{"hourly" "weekly" "monthly"} %))
+ #(gen/elements ["hourly" "weekly" "monthly"])))
(s/def ::pulse-channel (s/keys :req-un [::id ::channel_type ::details ::schedule_type]))
(s/def ::pulse-channel-recipient (s/keys :req-un [::id]))
--
GitLab