diff --git a/dev/src/dev/toucan2_monitor.clj b/dev/src/dev/toucan2_monitor.clj new file mode 100644 index 0000000000000000000000000000000000000000..6db39e6cfea6ad835818ef05ec25e079426abe00 --- /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 63242efa4e3e2f88a065bbbdfe4de83f2caee733..0e32fddc3e4c2b7f16ab35bfb96a280c59e351c6 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 63d533f7c0aafa16031444d021c7b1c17e2cedbe..5a1bfb243e93532ac4ca5dbaa33552b1bc0c00c4 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 0d66aab9fe5e6a81a8cf4ba6a4005a35e3e9dcdc..72147e76162c5d8ba4103191a41f31e7bb1b49ea 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 f2fd281ff1430030974c971cca7d17b5d34c5b43..52e299274f08792e1026d95a4255b9c0af8f04d5 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 7565e42d021206459dad4bf83e80d7d89e82e546..4baf4f9c33f9fac435cbe6d36afa86edc68276c5 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 1ce16ae17692e665e9635dca18880d50b7762d91..cafea4d0a17a13dca87ad34c9b529da11fcea56f 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 37039182aa360ae03fbd6b54c9a0c9ec2b2dee60..f33bb73075d008330ec18c7b67b7733e3fb5e4b4 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 354eab1fa67790737a077663a8a92a7ae30d663a..509361901e9343ec5604b3164ca44935aa3f78bf 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 8d818e6a989963a90910c12f29102a2e1d6a3688..129c4d0aecbecfe79419b1bce8e21a0c5b1be733 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 8b60e6602fc73924f6a0f2e45d18a133290f2168..58818d13e5fb9f7541d8864736b80795994df1fc 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 aa2c55b55bdd3b195ac78c2a48b12ff9dec93f61..c92286326f72dbcc12e6ac30c6c364986cbe4d1e 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 81c4bb8afbcf323ec582b3d66c62de49353cda47..1f6e8a8f9f7f53a636fb3f83fcce3b7640772f2f 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 a866f6d8890b7ea4eb47818fa9e3dd14286eb099..17ff8800eec45fd3b658f401ca60166882b84e39 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 c2db5a5bca9d79a1b2353bf532c9b9b9f91bef9a..652d8360059a86df2e6fa9c47d38edd39d3a7261 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 74dfeed34956278f56ea033d91cdecea65b8b154..3485e1dd5ebfff42da6eb20afe4347951a8d7c53 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 d3e5d8bd8320955aca539409e948d9af077ed5d5..8faa19c70e6158b3718a6d9cd4fd40a1a25507aa 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 7389361bfc9a1571cecd9f3003eca7fdfd1a77ff..e51a717d2f04ae14bcc3309ae70d921a8da6eab5 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]))