From a9f440109b3a679757b8d9656e504a1ceceee3c7 Mon Sep 17 00:00:00 2001 From: Braden Shepherdson <braden@metabase.com> Date: Fri, 21 Oct 2022 15:09:49 -0400 Subject: [PATCH] Serdes v2: Refactor entire serdes flow to use real Toucan, not low level (#25981) Previously, some automatic behavior was causing problems in serdes. (The worst example is generating an `entity_id` on insert while deserializing an entity we don't own - if deserialized again it would be duplicated.) A whole cascade of design choices fell out of this problem: - `insert!` would generate `entity_id`s, so use `simple-insert!` - But `simple-insert!` doesn't convert eg. MBQL `:definition` maps back into JSON strings - We got the maps as Clojure data and not JSON strings because `select` and `simple-select` both run `post-select` and parse the JSON. - So we ended up with a raw query on the select side and `simple-insert!` on the storage side. This change unwinds that whole stack, and instead uses a dynamic var to suppress the few pieces of `pre-insert` and `pre-update` logic that causes problems. The end result is much cleaner, and much more consistent with the rest of Metabase's backend logic. --- dev/src/dev/render_png.clj | 4 +- .../serialization/v2/extract.clj | 51 +- .../serialization/v2/load.clj | 20 +- .../serialization/v2/e2e/yaml_test.clj | 465 +++++++++++------- .../serialization/v2/extract_test.clj | 150 +++--- .../serialization/v2/load_test.clj | 18 +- src/metabase/models/card.clj | 15 +- src/metabase/models/collection.clj | 20 +- src/metabase/models/dashboard.clj | 12 +- src/metabase/models/dashboard_card.clj | 24 +- src/metabase/models/database.clj | 1 + src/metabase/models/dimension.clj | 4 +- src/metabase/models/field_values.clj | 12 +- src/metabase/models/interface.clj | 11 +- src/metabase/models/metric.clj | 9 +- src/metabase/models/native_query_snippet.clj | 18 +- src/metabase/models/pulse.clj | 2 +- src/metabase/models/pulse_card.clj | 5 +- src/metabase/models/pulse_channel.clj | 16 +- src/metabase/models/segment.clj | 4 +- src/metabase/models/serialization/base.clj | 89 +--- src/metabase/models/serialization/hash.clj | 22 +- src/metabase/models/serialization/util.clj | 79 +-- src/metabase/models/timeline_event.clj | 8 + test/metabase/test/generate.clj | 2 +- 25 files changed, 578 insertions(+), 483 deletions(-) diff --git a/dev/src/dev/render_png.clj b/dev/src/dev/render_png.clj index 50cc8a333bb..35a69a04504 100644 --- a/dev/src/dev/render_png.clj +++ b/dev/src/dev/render_png.clj @@ -80,7 +80,7 @@ [img-bytes] (str "data:image/png;base64," (String. (Base64Coder/encode img-bytes)))) -(defn svg-image [kind] +#_(defn svg-image [kind] (let [line|bar-data [["2015-02-01T00:00:00-08:00" 443] ["2015-03-01T00:00:00-08:00" 875] ["2015-04-01T00:00:00-07:00" 483] @@ -100,7 +100,7 @@ (throw (ex-info (str "Invalid chart type: " kind "\n Valid choices are :line, :bar, :donut") {}))))) -(defn preview-html +#_(defn preview-html "Chart type is one of :line, :bar, :donut. Html is a string with a placeholder {{chart}} which will be replaced with the [:img {:src chart-placeholder}] and the resulting html will be opened." [{:keys [chart html-file html-inline]}] diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/extract.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/extract.clj index 9673694568a..27e411a7276 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/v2/extract.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/extract.clj @@ -14,6 +14,23 @@ [toucan.db :as db] [toucan.hydrate :refer [hydrate]])) +(defn collection-set-for-user + "Given an optional user ID, find the transitive set of all Collection IDs which are either: + (a) global (ie. no one's personal collection); + (b) owned by this user (when user ID is non-nil); or + (c) descended from one of the above." + [user-or-nil] + (let [roots (db/select ['Collection :id :location :personal_owner_id] :location "/") + unowned (remove :personal_owner_id roots) + owned (when user-or-nil + (filter #(= user-or-nil (:personal_owner_id %)) roots)) + top-ids (->> (concat owned unowned) + (map :id) + set)] + (->> (concat unowned owned) + (map collection/descendant-ids) + (reduce set/union top-ids)))) + (defn extract-metabase "Extracts the appdb into a reducible stream of serializable maps, with `:serdes/meta` keys. @@ -26,7 +43,9 @@ (log/tracef "Extracting Metabase with options: %s" (pr-str opts)) (let [model-pred (if (:data-model-only opts) #{"Database" "Dimension" "Field" "FieldValues" "Metric" "Segment" "Table"} - (constantly true))] + (constantly true)) + ;; This set of unowned top-level collections is used in several `extract-query` implementations. + opts (assoc opts :collection-set (collection-set-for-user (:user opts)))] (eduction cat (for [model serdes.models/exported-models :when (model-pred model)] (serdes.base/extract-all model opts))))) @@ -60,16 +79,21 @@ cards (reduce set/union (for [coll-id collection-set] (db/select-ids Card :collection_id coll-id))) - ;; Map of {dashboard-id #{DashboardCard}} for dashcards whose cards are outside the transitive collection set. + ;; Map of {dashboard-id #{DashboardCard}} for dashcards whose cards OR parameter-bound cards are outside the + ;; transitive collection set. escaped-dashcards (into {} (for [dash dashboards :let [dcs (db/select DashboardCard :dashboard_id (:id dash)) escapees (->> dcs - (filter :card_id) ; Text cards have a nil card_id - (filter (comp not cards :card_id)) - set)] - :when (seq escapees)] - [(:id dash) escapees])) + (keep :card_id) ; Text cards have a nil card_id + set) + params (->> dcs + (mapcat :parameter_mappings) + (keep :card_id) + set) + combined (set/difference (set/union escapees params) cards)] + :when (seq combined)] + [(:id dash) combined])) ;; {source-card-id target-card-id} the key is in the curated set, the value is not. all-cards (for [id cards] (db/select-one [Card :id :collection_id :dataset_query] :id id)) @@ -86,10 +110,7 @@ :when (not (cards card-id))] [(:id card) card-id]) escaped-questions (into {} (concat bad-source bad-template-tags)) - problem-cards (set/union (set (vals escaped-questions)) - (set (for [[_ dashcards] escaped-dashcards - dc dashcards] - (:card_id dc))))] + problem-cards (reduce set/union (set (vals escaped-questions)) (vals escaped-dashcards))] (cond-> nil (seq escaped-dashcards) (assoc :escaped-dashcards escaped-dashcards) (seq escaped-questions) (assoc :escaped-questions escaped-questions) @@ -110,12 +131,12 @@ (when-not (empty? escaped-dashcards) (println "Dashboard cards outside the collection") (println "======================================") - (doseq [[dash-id dashcards] escaped-dashcards + (doseq [[dash-id card-ids] escaped-dashcards :let [dash-name (db/select-one-field :name Dashboard :id dash-id)]] (printf "Dashboard %d: %s\n" dash-id dash-name) - (doseq [{:keys [card_id col row]} dashcards + (doseq [card_id card-ids :let [card (db/select-one [Card :collection_id :name] :id card_id)]] - (printf " %dx%d \tCard %d: %s\n" col row card_id (:name card)) + (printf " \tCard %d: %s\n" card_id (:name card)) (printf " from collection %s\n" (collection-label (:collection_id card)))))) (when-not (empty? escaped-questions) @@ -155,4 +176,4 @@ (m/map-vals #(set (map second %))))] (eduction cat (for [[model ids] by-model] (eduction (map #(serdes.base/extract-one model opts %)) - (serdes.base/raw-reducible-query model {:where [:in :id ids]}))))))) + (db/select-reducible (symbol model) :id [:in ids]))))))) diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj index 5089ae86b35..8947113c563 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/load.clj @@ -29,7 +29,13 @@ (cond (expanding path) (throw (ex-info (format "Circular dependency on %s" (pr-str path)) {:path path})) (seen path) ctx ; Already been done, just skip it. - :else (let [ingested (serdes.ingest/ingest-one ingestion path) + :else (let [ingested (try + (serdes.ingest/ingest-one ingestion path) + (catch Exception e + (throw (ex-info (format "Failed to read file for %s" (pr-str path)) + {:path path + :deps-chain expanding} + e)))) deps (serdes.base/serdes-dependencies ingested) ctx (-> ctx (update :expanding conj path) @@ -38,9 +44,15 @@ (update :expanding disj path)) ;; Use the abstract path as attached by the ingestion process, not the original one we were passed. rebuilt-path (serdes.base/serdes-path ingested) - local-pk-or-nil (serdes.base/load-find-local rebuilt-path) - _ (serdes.base/load-one! ingested local-pk-or-nil)] - ctx))) + local-pk-or-nil (serdes.base/load-find-local rebuilt-path)] + (try + (serdes.base/load-one! ingested local-pk-or-nil) + ctx + (catch Exception e + (throw (ex-info (format "Failed to load into database for %s" (pr-str path)) + {:path path + :deps-chain expanding} + e))))))) (defn load-metabase "Loads in a database export from an ingestion source, which is any Ingestable instance." diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj index f827448b27e..f290aae64eb 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj @@ -1,19 +1,17 @@ (ns metabase-enterprise.serialization.v2.e2e.yaml-test (:require [clojure.java.io :as io] [clojure.test :refer :all] - [java-time :as t] + [medley.core :as m] [metabase-enterprise.serialization.test-util :as ts] [metabase-enterprise.serialization.v2.extract :as extract] - [metabase-enterprise.serialization.v2.ingest :as ingest] [metabase-enterprise.serialization.v2.ingest.yaml :as ingest.yaml] + [metabase-enterprise.serialization.v2.load :as serdes.load] [metabase-enterprise.serialization.v2.storage.yaml :as storage.yaml] - [metabase-enterprise.serialization.v2.utils.yaml :as u.yaml] [metabase.models.serialization.base :as serdes.base] [metabase.test.generate :as test-gen] - [metabase.util.date-2 :as u.date] [reifyhealth.specmonstah.core :as rs] - [yaml.core :as yaml]) - (:import java.time.ZoneId)) + [toucan.db :as db] + [yaml.core :as yaml])) (defn- dir->file-set [dir] (->> dir @@ -27,244 +25,335 @@ .listFiles (remove #(.isFile %)))) -(defn- strip-labels [path] - (mapv #(dissoc % :label) path)) - -(defn- clean-up-expected [entity] - (cond-> entity - true (dissoc :serdes/meta :updated_at) - (:created_at entity) (update :created_at u.date/format))) - (defn- random-keyword ([prefix n] (random-keyword prefix n 0)) - ([prefix n floor] (keyword (str prefix (+ floor (rand-int n)))))) + ([prefix n floor] (keyword (str (name prefix) (+ floor (rand-int n)))))) + +(defn- random-fks + "Generates a specmonstah query with the :refs populated with the randomized bindings. + `(random-fks {:spec-gen {:foo :bar}} + {:creator_id [:u 10] + :db_id [:db 20 15]})` + this will return a query like: + `{:spec-gen {:foo :bar} + :refs {:creator_id :u6 :db_id 17}}` + + The bindings map has the same keys as `:refs`, but the values are `[base-keyword width]` pairs or + `[base-keyword width floor]` triples. These are passed to [[random-keyword]]." + [base bindings] + (update base :refs merge (m/map-vals #(apply random-keyword %) bindings))) + +(defn- many-random-fks [n base bindings] + (vec (repeatedly n #(vector 1 (random-fks base bindings))))) + +(defn- table->db [{:keys [table_id] :as refs}] + (let [table-number (-> table_id + name + (subs 1) + (Integer/parseInt))] + (assoc refs :database_id (keyword (str "db" (quot table-number 10)))))) + +(defn- clean-entity + "Removes any comparison-confounding fields, like `:created_at`." + [entity] + (dissoc entity :created_at)) (deftest e2e-storage-ingestion-test (ts/with-random-dump-dir [dump-dir "serdesv2-"] - (ts/with-empty-h2-app-db - ;; TODO Generating some nested collections would make these tests more robust. - (test-gen/insert! - {:collection [[100 {:refs {:personal_owner_id ::rs/omit}}] - [10 {:refs {:personal_owner_id ::rs/omit} - :spec-gen {:namespace :snippets}}]] - :database [[10]] - :table (into [] (for [db [:db0 :db1 :db2 :db3 :db4 :db5 :db6 :db7 :db8 :db9]] - [10 {:refs {:db_id db}}])) - :field (into [] (for [n (range 100) - :let [table (keyword (str "t" n))]] - [10 {:refs {:table_id table}}])) - :core-user [[10]] - :card [[100 {:refs (let [db (rand-int 10) - t (rand-int 10)] - {:database_id (keyword (str "db" db)) - :table_id (keyword (str "t" (+ t (* 10 db)))) - :collection_id (random-keyword "coll" 100) - :creator_id (random-keyword "u" 10)})}]] - :dashboard [[100 {:refs {:collection_id (random-keyword "coll" 100) - :creator_id (random-keyword "u" 10)}}]] - :dashboard-card [[300 {:refs {:card_id (random-keyword "c" 100) - :dashboard_id (random-keyword "d" 100)}}]] - :dimension [;; 20 with both IDs set - [20 {:refs {:field_id (random-keyword "field" 1000) - :human_readable_field_id (random-keyword "field" 1000)}}] - ;; 20 with just :field_id - [20 {:refs {:field_id (random-keyword "field" 1000) - :human_readable_field_id ::rs/omit}}]] - :metric [[30 {:refs {:table_id (random-keyword "t" 100) - :creator_id (random-keyword "u" 10)}}]] - :segment [[30 {:refs {:table_id (random-keyword "t" 100) - :creator_id (random-keyword "u" 10)}}]] - :native-query-snippet [[10 {:refs {:creator_id (random-keyword "u" 10) - :collection_id (random-keyword "coll" 10 100)}}]] - :timeline [[10 {:refs {:creator_id (random-keyword "u" 10) - :collection_id (random-keyword "coll" 100)}}]] - :timeline-event [[90 {:refs {:timeline_id (random-keyword "timeline" 10)}}]] - :pulse [[10 {:refs {:collection_id (random-keyword "coll" 100)}}] - [10 {:refs {:collection_id ::rs/omit}}] - [10 {:refs {:collection_id ::rs/omit - :dashboard_id (random-keyword "d" 100)}}]] - :pulse-card [[60 {:refs {:card_id (random-keyword "c" 100) - :pulse_id (random-keyword "pulse" 10)}}] - [60 {:refs {:card_id (random-keyword "c" 100) - :pulse_id (random-keyword "pulse" 10 20) - :dashboard_card_id (random-keyword "dc" 300)}}]] - :pulse-channel [[15 {:refs {:pulse_id (random-keyword "pulse" 10)}}] - [15 {:refs {:pulse_id (random-keyword "pulse" 10 20)}}]] - :pulse-channel-recipient [[40 {:refs {:pulse_channel_id (random-keyword "pulse-channel" 30) - :user_id (random-keyword "u" 10)}}]]}) - (let [extraction (into [] (extract/extract-metabase {})) - entities (reduce (fn [m entity] - (update m (-> entity :serdes/meta last :model) - (fnil conj []) entity)) - {} extraction)] - (is (= 110 (-> entities (get "Collection") count))) + (let [extraction (atom nil) + entities (atom nil)] + (ts/with-source-and-dest-dbs + ;; TODO Generating some nested collections would make these tests more robust. + (ts/with-source-db + (testing "insert" + (test-gen/insert! + {:collection [[100 {:refs {:personal_owner_id ::rs/omit}}] + [10 {:refs {:personal_owner_id ::rs/omit} + :spec-gen {:namespace :snippets}}]] + :database [[10]] + ;; Tables are special - we define table 0-9 under db0, 10-19 under db1, etc. The :card spec below + ;; depends on this relationship. + :table (into [] (for [db [:db0 :db1 :db2 :db3 :db4 :db5 :db6 :db7 :db8 :db9]] + [10 {:refs {:db_id db}}])) + :field (many-random-fks 1000 {} {:table_id [:t 100]}) + :core-user [[100]] + :card (mapv #(update-in % [1 :refs] table->db) + (many-random-fks + 100 + {:spec-gen {:dataset_query {:database 1 + :query {:source-table 3 + :aggregation [[:count]] + :breakout [[:field 16 nil]]} + :type :query}}} + {:table_id [:t 100] + :collection_id [:coll 100] + :creator_id [:u 10]})) + :dashboard (many-random-fks 100 {} {:collection_id [:coll 100] + :creator_id [:u 10]}) + :dashboard-card (many-random-fks 300 {} {:card_id [:c 100] + :dashboard_id [:d 100]}) + :dimension (vec (concat + ;; 20 with both IDs set + (many-random-fks 20 {} + {:field_id [:field 1000] + :human_readable_field_id [:field 1000]}) + ;; 20 with just :field_id + (many-random-fks 20 {:refs {:human_readable_field_id ::rs/omit}} + {:field_id [:field 1000]}))) + :metric (many-random-fks 30 {:spec-gen {:definition {:aggregation [[:count]] + :source-table 9}}} + {:table_id [:t 100] + :creator_id [:u 10]}) + :segment (many-random-fks 30 {:spec-gen {:definition {:filter [:!= [:field 60 nil] 50], + :source-table 4}}} + {:table_id [:t 100] + :creator_id [:u 10]}) + :native-query-snippet (many-random-fks 10 {} {:creator_id [:u 10] + :collection_id [:coll 10 100]}) + :timeline (many-random-fks 10 {} {:creator_id [:u 10] + :collection_id [:coll 100]}) + :timeline-event (many-random-fks 90 {} {:timeline_id [:timeline 10]}) + :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]})})) + + (is (= 100 (count (db/select-field :email 'User)))) + + (testing "extraction" + (reset! extraction (into [] (extract/extract-metabase {}))) + (reset! entities (reduce (fn [m entity] + (update m (-> entity :serdes/meta last :model) + (fnil conj []) entity)) + {} @extraction)) + (is (= 110 (-> @entities (get "Collection") count))))) (testing "storage" - (storage.yaml/store! (seq extraction) dump-dir) + (storage.yaml/store! (seq @extraction) dump-dir) + (testing "for Collections" - (is (= 110 (count (dir->file-set (io/file dump-dir "Collection"))))) - (doseq [{:keys [entity_id slug] :as coll} (get entities "Collection") - :let [filename (#'u.yaml/leaf-file-name entity_id slug)]] - (is (= (clean-up-expected coll) - (yaml/from-file (io/file dump-dir "Collection" filename)))))) + (is (= 110 (count (dir->file-set (io/file dump-dir "Collection")))))) (testing "for Databases" - (is (= 10 (count (dir->file-set (io/file dump-dir "Database"))))) - (doseq [{:keys [name] :as coll} (get entities "Database") - :let [filename (#'u.yaml/leaf-file-name name)]] - (is (= (clean-up-expected coll) - (yaml/from-file (io/file dump-dir "Database" filename)))))) + (is (= 10 (count (dir->file-set (io/file dump-dir "Database")))))) (testing "for Tables" (is (= 100 - (reduce + (for [db (get entities "Database") + (reduce + (for [db (get @entities "Database") :let [tables (dir->file-set (io/file dump-dir "Database" (:name db) "Table"))]] (count tables)))) - "Tables are scattered, so the directories are harder to count") - - (doseq [{:keys [db_id name] :as coll} (get entities "Table")] - (is (= (clean-up-expected coll) - (yaml/from-file (io/file dump-dir "Database" db_id "Table" (str name ".yaml"))))))) + "Tables are scattered, so the directories are harder to count")) (testing "for Fields" (is (= 1000 - (reduce + (for [db (get entities "Database") + (reduce + (for [db (get @entities "Database") table (subdirs (io/file dump-dir "Database" (:name db) "Table"))] (->> (io/file table "Field") dir->file-set count)))) - "Fields are scattered, so the directories are harder to count") - - (doseq [{[db schema table] :table_id name :name :as coll} (get entities "Field")] - (is (nil? schema)) - (is (= (clean-up-expected coll) - (yaml/from-file (io/file dump-dir "Database" db "Table" table "Field" (str name ".yaml"))))))) + "Fields are scattered, so the directories are harder to count")) (testing "for cards" - (is (= 100 (count (dir->file-set (io/file dump-dir "Card"))))) - (doseq [{:keys [entity_id] :as card} (get entities "Card") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected card) - (yaml/from-file (io/file dump-dir "Card" filename)))))) + (is (= 100 (count (dir->file-set (io/file dump-dir "Card")))))) (testing "for dashboards" - (is (= 100 (count (dir->file-set (io/file dump-dir "Dashboard"))))) - (doseq [{:keys [entity_id] :as dash} (get entities "Dashboard") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected dash) - (yaml/from-file (io/file dump-dir "Dashboard" filename)))))) + (is (= 100 (count (dir->file-set (io/file dump-dir "Dashboard")))))) (testing "for dashboard cards" (is (= 300 - (reduce + (for [dash (get entities "Dashboard") + (reduce + (for [dash (get @entities "Dashboard") :let [card-dir (io/file dump-dir "Dashboard" (:entity_id dash) "DashboardCard")]] (if (.exists card-dir) (count (dir->file-set card-dir)) - 0))))) - - (doseq [{:keys [dashboard_id entity_id] - :as dashcard} (get entities "DashboardCard") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected dashcard) - (yaml/from-file (io/file dump-dir "Dashboard" dashboard_id "DashboardCard" filename)))))) + 0)))))) (testing "for dimensions" - (is (= 40 (count (dir->file-set (io/file dump-dir "Dimension"))))) - (doseq [{:keys [entity_id] :as dim} (get entities "Dimension") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected dim) - (yaml/from-file (io/file dump-dir "Dimension" filename)))))) + (is (= 40 (count (dir->file-set (io/file dump-dir "Dimension")))))) (testing "for metrics" - (is (= 30 (count (dir->file-set (io/file dump-dir "Metric"))))) - (doseq [{:keys [entity_id name] :as metric} (get entities "Metric") - :let [filename (#'u.yaml/leaf-file-name entity_id name)]] - (is (= (clean-up-expected metric) - (yaml/from-file (io/file dump-dir "Metric" filename)))))) + (is (= 30 (count (dir->file-set (io/file dump-dir "Metric")))))) (testing "for segments" - (is (= 30 (count (dir->file-set (io/file dump-dir "Segment"))))) - (doseq [{:keys [entity_id name] :as segment} (get entities "Segment") - :let [filename (#'u.yaml/leaf-file-name entity_id name)]] - (is (= (clean-up-expected segment) - (yaml/from-file (io/file dump-dir "Segment" filename)))))) + (is (= 30 (count (dir->file-set (io/file dump-dir "Segment")))))) (testing "for pulses" - (is (= 30 (count (dir->file-set (io/file dump-dir "Pulse"))))) - (doseq [{:keys [entity_id] :as pulse} (get entities "Pulse") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected pulse) - (yaml/from-file (io/file dump-dir "Pulse" filename)))))) + (is (= 30 (count (dir->file-set (io/file dump-dir "Pulse")))))) (testing "for pulse cards" - (is (= 120 (reduce + (for [pulse (get entities "Pulse")] + (is (= 120 (reduce + (for [pulse (get @entities "Pulse")] (->> (io/file dump-dir "Pulse" (:entity_id pulse) "PulseCard") dir->file-set - count))))) - (doseq [{:keys [entity_id pulse_id] :as card} (get entities "PulseCard") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected card) - (yaml/from-file (io/file dump-dir "Pulse" pulse_id "PulseCard" filename)))))) + count)))))) (testing "for pulse channels" - (is (= 30 (reduce + (for [pulse (get entities "Pulse")] + (is (= 30 (reduce + (for [pulse (get @entities "Pulse")] (->> (io/file dump-dir "Pulse" (:entity_id pulse) "PulseChannel") dir->file-set count))))) - (is (= 40 (reduce + (for [{:keys [recipients]} (get entities "PulseChannel")] - (count recipients))))) - (doseq [{:keys [entity_id pulse_id] :as channel} (get entities "PulseChannel") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected channel) - (yaml/from-file (io/file dump-dir "Pulse" pulse_id "PulseChannel" filename)))))) + (is (= 40 (reduce + (for [{:keys [recipients]} (get @entities "PulseChannel")] + (count recipients)))))) (testing "for native query snippets" - (is (= 10 (count (dir->file-set (io/file dump-dir "NativeQuerySnippet"))))) - (doseq [{:keys [entity_id name] :as snippet} (get entities "NativeQuerySnippet") - :let [filename (#'u.yaml/leaf-file-name entity_id name)]] - (is (= (clean-up-expected snippet) - (yaml/from-file (io/file dump-dir "NativeQuerySnippet" filename)))))) + (is (= 10 (count (dir->file-set (io/file dump-dir "NativeQuerySnippet")))))) (testing "for timelines and events" (is (= 10 (count (dir->file-set (io/file dump-dir "Timeline"))))) - (doseq [{:keys [entity_id] :as timeline} (get entities "Timeline") - :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (clean-up-expected timeline) - (yaml/from-file (io/file dump-dir "Timeline" filename))))) - (is (= 90 (reduce + (for [timeline (get entities "Timeline")] + (is (= 90 (reduce + (for [timeline (get @entities "Timeline")] (->> (io/file dump-dir "Timeline" (:entity_id timeline) "TimelineEvent") dir->file-set - count))))) - (doseq [{:keys [name timeline_id timestamp] :as event} (get entities "TimelineEvent") - :let [filename (#'u.yaml/leaf-file-name timestamp name)]] - (is (= (clean-up-expected event) - (yaml/from-file (io/file dump-dir "Timeline" timeline_id "TimelineEvent" filename)))))) + count)))))) (testing "for settings" - (is (= (into {} (for [{:keys [key value]} (get entities "Setting")] - [key value])) - (yaml/from-file (io/file dump-dir "settings.yaml")))))) - - (testing "ingestion" - (let [ingestable (ingest.yaml/ingest-yaml dump-dir)] - (testing "ingest-list is accurate" - (is (= (into #{} (comp cat - (map (fn [entity] - (mapv #(cond-> % - (:label %) (update :label #'u.yaml/truncate-label)) - (serdes.base/serdes-path entity))))) - (vals entities)) - (into #{} (ingest/ingest-list ingestable))))) - - - (testing "each entity matches its in-memory original" - (doseq [entity extraction] - (let [->utc #(t/zoned-date-time % (ZoneId/of "UTC"))] - (is (= (cond-> entity - true (update :serdes/meta strip-labels) - true (dissoc :updated_at) - ;; TIMESTAMP WITH TIME ZONE columns come out of the database as OffsetDateTime, but read back - ;; from YAML as ZonedDateTimes; coerce the expected value to match. - (t/offset-date-time? (:created_at entity)) (update :created_at ->utc)) - (ingest/ingest-one ingestable (serdes.base/serdes-path entity))))))))))))) + (is (.exists (io/file dump-dir "settings.yaml"))))) + + (testing "ingest and load" + (ts/with-dest-db + (testing "doing ingestion" + (is (serdes.load/load-metabase (ingest.yaml/ingest-yaml dump-dir)) + "successful")) + + (testing "for Collections" + (doseq [{:keys [entity_id] :as coll} (get @entities "Collection")] + (is (= (clean-entity coll) + (->> (db/select-one 'Collection :entity_id entity_id) + (serdes.base/extract-one "Collection" {}) + clean-entity))))) + + (testing "for Databases" + (doseq [{:keys [name] :as coll} (get @entities "Database")] + (is (= (clean-entity coll) + (->> (db/select-one 'Database :name name) + (serdes.base/extract-one "Database" {}) + clean-entity))))) + + (testing "for Tables" + (doseq [{:keys [db_id name] :as coll} (get @entities "Table")] + (is (= (clean-entity coll) + (->> (db/select-one-field :id 'Database :name db_id) + (db/select-one 'Table :name name :db_id) + (serdes.base/extract-one "Table" {}) + clean-entity))))) + + (testing "for Fields" + (doseq [{[db schema table] :table_id name :name :as coll} (get @entities "Field")] + (is (nil? schema)) + (is (= (clean-entity coll) + (->> (db/select-one-field :id 'Database :name db) + (db/select-one-field :id 'Table :schema schema :name table :db_id) + (db/select-one 'Field :name name :table_id) + (serdes.base/extract-one "Field" {}) + clean-entity))))) + + (testing "for cards" + (doseq [{:keys [entity_id] :as card} (get @entities "Card")] + (is (= (clean-entity card) + (->> (db/select-one 'Card :entity_id entity_id) + (serdes.base/extract-one "Card" {}) + clean-entity))))) + + (testing "for dashboards" + (doseq [{:keys [entity_id] :as dash} (get @entities "Dashboard")] + (is (= (clean-entity dash) + (->> (db/select-one 'Dashboard :entity_id entity_id) + (serdes.base/extract-one "Dashboard" {}) + clean-entity))))) + + (testing "for dashboard cards" + (doseq [{:keys [entity_id] :as dashcard} (get @entities "DashboardCard")] + (is (= (clean-entity dashcard) + (->> (db/select-one 'DashboardCard :entity_id entity_id) + (serdes.base/extract-one "DashboardCard" {}) + clean-entity))))) + + (testing "for dimensions" + (doseq [{:keys [entity_id] :as dim} (get @entities "Dimension")] + (is (= (clean-entity dim) + (->> (db/select-one 'Dimension :entity_id entity_id) + (serdes.base/extract-one "Dimension" {}) + clean-entity))))) + + (testing "for metrics" + (doseq [{:keys [entity_id] :as metric} (get @entities "Metric")] + (is (= (clean-entity metric) + (->> (db/select-one 'Metric :entity_id entity_id) + (serdes.base/extract-one "Metric" {}) + clean-entity))))) + + (testing "for segments" + (doseq [{:keys [entity_id] :as segment} (get @entities "Segment")] + (is (= (clean-entity segment) + (->> (db/select-one 'Segment :entity_id entity_id) + (serdes.base/extract-one "Segment" {}) + clean-entity))))) + + (testing "for pulses" + (doseq [{:keys [entity_id] :as pulse} (get @entities "Pulse")] + (is (= (clean-entity pulse) + (->> (db/select-one 'Pulse :entity_id entity_id) + (serdes.base/extract-one "Pulse" {}) + clean-entity))))) + + (testing "for pulse cards" + (doseq [{:keys [entity_id] :as card} (get @entities "PulseCard")] + (is (= (clean-entity card) + (->> (db/select-one 'PulseCard :entity_id entity_id) + (serdes.base/extract-one "PulseCard" {}) + clean-entity))))) + + (testing "for pulse channels" + (doseq [{:keys [entity_id] :as channel} (get @entities "PulseChannel")] + ;; The :recipients list is in arbitrary order - turn them into sets for comparison. + (is (= (-> channel + (update :recipients set) + clean-entity) + (let [loaded-channel (->> (db/select-one 'PulseChannel :entity_id entity_id) + (serdes.base/extract-one "PulseChannel" {}))] + (-> loaded-channel + (update :recipients set) + clean-entity)))))) + + (testing "for native query snippets" + (doseq [{:keys [entity_id] :as snippet} (get @entities "NativeQuerySnippet")] + (is (= (clean-entity snippet) + (->> (db/select-one 'NativeQuerySnippet :entity_id entity_id) + (serdes.base/extract-one "NativeQuerySnippet" {}) + clean-entity))))) + + (testing "for timelines and events" + (doseq [{:keys [entity_id] :as timeline} (get @entities "Timeline")] + (is (= (clean-entity timeline) + (->> (db/select-one 'Timeline :entity_id entity_id) + (serdes.base/extract-one "Timeline" {}) + clean-entity)))) + + (doseq [{:keys [timeline_id timestamp] :as event} (get @entities "TimelineEvent")] + (is (= (clean-entity event) + (->> (db/select-one-id 'Timeline :entity_id timeline_id) + (db/select-one 'TimelineEvent :timestamp timestamp :timeline_id) + (serdes.base/extract-one "TimelineEvent" {}) + clean-entity))))) + + (testing "for settings" + (is (= (into {} (for [{:keys [key value]} (get @entities "Setting")] + [key value])) + (yaml/from-file (io/file dump-dir "settings.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 82953f86ded..651079b4726 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj @@ -1,18 +1,15 @@ (ns metabase-enterprise.serialization.v2.extract-test - (:require [cheshire.core :as json] - [clojure.set :as set] + (:require [clojure.set :as set] [clojure.test :refer :all] [metabase-enterprise.serialization.test-util :as ts] [metabase-enterprise.serialization.v2.extract :as extract] [metabase.models :refer [Card Collection Dashboard DashboardCard Database Dimension Field FieldValues Metric NativeQuerySnippet Pulse PulseCard Segment Table Timeline TimelineEvent User]] [metabase.models.serialization.base :as serdes.base] - [schema.core :as s]) + [schema.core :as s] + [toucan.db :as db]) (:import [java.time LocalDateTime OffsetDateTime])) -(defn- select-one [model-name where] - (first (into [] (serdes.base/raw-reducible-query model-name {:where where})))) - (defn- by-model [model-name extraction] (->> extraction (into []) @@ -40,7 +37,7 @@ :personal_owner_id mark-id}]] (testing "a top-level collection is extracted correctly" - (let [ser (serdes.base/extract-one "Collection" {} (select-one "Collection" [:= :id coll-id]))] + (let [ser (serdes.base/extract-one "Collection" {} (db/select-one 'Collection :id coll-id))] (is (schema= {:serdes/meta (s/eq [{:model "Collection" :id coll-eid :label coll-slug}]) :personal_owner_id (s/eq nil) :parent_id (s/eq nil) @@ -50,7 +47,7 @@ (is (not (contains? ser :id))))) (testing "a nested collection is extracted with the right parent_id" - (let [ser (serdes.base/extract-one "Collection" {} (select-one "Collection" [:= :id child-id]))] + (let [ser (serdes.base/extract-one "Collection" {} (db/select-one 'Collection :id child-id))] (is (schema= {:serdes/meta (s/eq [{:model "Collection" :id child-eid :label child-slug}]) :personal_owner_id (s/eq nil) :parent_id (s/eq coll-eid) @@ -60,7 +57,7 @@ (is (not (contains? ser :id))))) (testing "personal collections are extracted with email as key" - (let [ser (serdes.base/extract-one "Collection" {} (select-one "Collection" [:= :id pc-id]))] + (let [ser (serdes.base/extract-one "Collection" {} (db/select-one 'Collection :id pc-id))] (is (schema= {:serdes/meta (s/eq [{:model "Collection" :id pc-eid :label pc-slug}]) :parent_id (s/eq nil) :personal_owner_id (s/eq "mark@direstrai.ts") @@ -111,12 +108,10 @@ :table_id no-schema-id :collection_id coll-id :creator_id mark-id - :dataset_query - (json/generate-string - {:query {:source-table no-schema-id - :filter [:>= [:field field-id nil] 18] - :aggregation [[:count]]} - :database db-id})}] + :dataset_query {:query {:source-table no-schema-id + :filter [:>= [:field field-id nil] 18] + :aggregation [[:count]]} + :database db-id}}] Card [{c2-id :id c2-eid :entity_id} {:name "Second Question" :database_id db-id @@ -163,10 +158,9 @@ :collection_id coll-id :creator_id mark-id :dataset_query - (json/generate-string - {:query {:source-table no-schema-id - :filter [:>= [:field field-id nil] 18]} - :database db-id})}] + {:query {:source-table no-schema-id + :filter [:>= [:field field-id nil] 18]} + :database db-id}}] Card [{c5-id :id c5-eid :entity_id} {:name "Dependent Question" :database_id db-id @@ -174,10 +168,9 @@ :collection_id coll-id :creator_id mark-id :dataset_query - (json/generate-string - {:query {:source-table (str "card__" c4-id) - :aggregation [[:count]]} - :database db-id})}] + {:query {:source-table (str "card__" c4-id) + :aggregation [[:count]]} + :database db-id}}] Dashboard [{dash-id :id dash-eid :entity_id} {:name "Shared Dashboard" @@ -219,13 +212,13 @@ :column_settings {(str "[\"ref\",[\"field\"," field2-id ",null]]") {:column_title "Locus"}}}}]] (testing "table and database are extracted as [db schema table] triples" - (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c1-id]))] + (let [ser (serdes.base/extract-one "Card" {} (db/select-one 'Card :id c1-id))] (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c1-eid}]) :table_id (s/eq ["My Database" nil "Schemaless Table"]) :creator_id (s/eq "mark@direstrai.ts") :collection_id (s/eq coll-eid) :dataset_query (s/eq {:query {:source-table ["My Database" nil "Schemaless Table"] - :filter [">=" [:field ["My Database" nil "Schemaless Table" "Some Field"] nil] 18] + :filter [:>= [:field ["My Database" nil "Schemaless Table" "Some Field"] nil] 18] :aggregation [[:count]]} :database "My Database"}) :created_at LocalDateTime @@ -243,7 +236,7 @@ [{:model "Collection" :id coll-eid}]} (set (serdes.base/serdes-dependencies ser)))))) - (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c2-id]))] + (let [ser (serdes.base/extract-one "Card" {} (db/select-one 'Card :id c2-id))] (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c2-eid}]) :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) :creator_id (s/eq "mark@direstrai.ts") @@ -274,7 +267,7 @@ {:model "Field" :id "Other Field"}]} (set (serdes.base/serdes-dependencies ser)))))) - (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c3-id]))] + (let [ser (serdes.base/extract-one "Card" {} (db/select-one 'Card :id c3-id))] (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c3-eid}]) :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) :creator_id (s/eq "mark@direstrai.ts") @@ -285,19 +278,19 @@ :table.cell_column "sum" :table.columns [{:name "SOME_FIELD" - :fieldRef ["field" ["My Database" nil "Schemaless Table" "Some Field"] nil] + :fieldRef [:field ["My Database" nil "Schemaless Table" "Some Field"] nil] :enabled true} {:name "OTHER_FIELD" - :fieldRef ["field" ["My Database" "PUBLIC" "Schema'd Table" "Other Field"] nil] + :fieldRef [:field ["My Database" "PUBLIC" "Schema'd Table" "Other Field"] nil] :enabled true} {:name "sum" - :fieldRef ["field" "sum" {:base-type "type/Float"}] + :fieldRef [:field "sum" {:base-type :type/Float}] :enabled true} {:name "count" - :fieldRef ["field" "count" {:base-type "type/BigInteger"}] + :fieldRef [:field "count" {:base-type :type/BigInteger}] :enabled true} {:name "Average order total" - :fieldRef ["field" "Average order total" {:base-type "type/Float"}] + :fieldRef [:field "Average order total" {:base-type :type/Float}] :enabled true}] :column_settings {"[\"ref\",[\"field\",[\"My Database\",\"PUBLIC\",\"Schema'd Table\",\"Other Field\"],null]]" {:column_title "Locus"}}}) @@ -321,7 +314,7 @@ {:model "Field" :id "Other Field"}]} (set (serdes.base/serdes-dependencies ser)))))) - (let [ser (serdes.base/extract-one "DashboardCard" {} (select-one "DashboardCard" [:= :id dc1-id]))] + (let [ser (serdes.base/extract-one "DashboardCard" {} (db/select-one 'DashboardCard :id dc1-id))] (is (schema= {:serdes/meta (s/eq [{:model "Dashboard" :id dash-eid} {:model "DashboardCard" :id dc1-eid}]) :dashboard_id (s/eq dash-eid) @@ -347,7 +340,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "Cards can be based on other cards" - (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c5-id]))] + (let [ser (serdes.base/extract-one "Card" {} (db/select-one 'Card :id c5-id))] (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c5-eid}]) :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) :creator_id (s/eq "mark@direstrai.ts") @@ -370,7 +363,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "Dashcard :visualization_settings are included in their deps" - (let [ser (serdes.base/extract-one "DashboardCard" {} (select-one "DashboardCard" [:= :id dc2-id]))] + (let [ser (serdes.base/extract-one "DashboardCard" {} (db/select-one 'DashboardCard :id dc2-id))] (is (schema= {:serdes/meta (s/eq [{:model "Dashboard" :id other-dash} {:model "DashboardCard" :id dc2-eid}]) :dashboard_id (s/eq other-dash) @@ -378,16 +371,16 @@ :table.cell_column "sum" :table.columns [{:name "SOME_FIELD" - :fieldRef ["field" ["My Database" nil "Schemaless Table" "Some Field"] nil] + :fieldRef [:field ["My Database" nil "Schemaless Table" "Some Field"] nil] :enabled true} {:name "sum" - :fieldRef ["field" "sum" {:base-type "type/Float"}] + :fieldRef [:field "sum" {:base-type :type/Float}] :enabled true} {:name "count" - :fieldRef ["field" "count" {:base-type "type/BigInteger"}] + :fieldRef [:field "count" {:base-type :type/BigInteger}] :enabled true} {:name "Average order total" - :fieldRef ["field" "Average order total" {:base-type "type/Float"}] + :fieldRef [:field "Average order total" {:base-type :type/Float}] :enabled true}] :column_settings {"[\"ref\",[\"field\",[\"My Database\",\"PUBLIC\",\"Schema'd Table\",\"Other Field\"],null]]" {:column_title "Locus"}}}) @@ -411,34 +404,50 @@ (testing "collection filtering based on :user option" (testing "only unowned collections are returned with no user" (is (= ["Some Collection"] - (->> (serdes.base/extract-all "Collection" {}) + (->> (serdes.base/extract-all "Collection" {:collection-set #{coll-id}}) (into []) (map :name))))) (testing "unowned collections and the personal one with a user" (is (= #{coll-eid mark-coll-eid} - (by-model "Collection" (serdes.base/extract-all "Collection" {:user mark-id})))) + (->> {:collection-set (extract/collection-set-for-user mark-id)} + (serdes.base/extract-all "Collection") + (by-model "Collection")))) (is (= #{coll-eid dave-coll-eid} - (by-model "Collection" (serdes.base/extract-all "Collection" {:user dave-id})))))) + (->> {:collection-set (extract/collection-set-for-user dave-id)} + (serdes.base/extract-all "Collection") + (by-model "Collection")))))) (testing "dashboards are filtered based on :user" (testing "dashboards in unowned collections are always returned" (is (= #{dash-eid} - (by-model "Dashboard" (serdes.base/extract-all "Dashboard" {})))) + (->> {:collection-set #{coll-id}} + (serdes.base/extract-all "Dashboard") + (by-model "Dashboard")))) (is (= #{dash-eid} - (by-model "Dashboard" (serdes.base/extract-all "Dashboard" {:user mark-id}))))) + (->> {:collection-set (extract/collection-set-for-user mark-id)} + (serdes.base/extract-all "Dashboard") + (by-model "Dashboard"))))) (testing "dashboards in personal collections are returned for the :user" (is (= #{dash-eid other-dash} - (by-model "Dashboard" (serdes.base/extract-all "Dashboard" {:user dave-id})))))) + (->> {:collection-set (extract/collection-set-for-user dave-id)} + (serdes.base/extract-all "Dashboard") + (by-model "Dashboard")))))) (testing "dashboard cards are filtered based on :user" (testing "dashboard cards whose dashboards are in unowned collections are always returned" (is (= #{dc1-eid} - (by-model "DashboardCard" (serdes.base/extract-all "DashboardCard" {})))) + (->> {:collection-set (extract/collection-set-for-user nil)} + (serdes.base/extract-all "DashboardCard") + (by-model "DashboardCard")))) (is (= #{dc1-eid} - (by-model "DashboardCard" (serdes.base/extract-all "DashboardCard" {:user mark-id}))))) + (->> {:collection-set (extract/collection-set-for-user mark-id)} + (serdes.base/extract-all "DashboardCard") + (by-model "DashboardCard"))))) (testing "dashboard cards whose dashboards are in personal collections are returned for the :user" (is (= #{dc1-eid dc2-eid} - (by-model "DashboardCard" (serdes.base/extract-all "DashboardCard" {:user dave-id}))))))))) + (->> {:collection-set (extract/collection-set-for-user dave-id)} + (serdes.base/extract-all "DashboardCard") + (by-model "DashboardCard"))))))))) (deftest dimensions-test (ts/with-empty-h2-app-db @@ -466,7 +475,7 @@ :field_id fk-id :human_readable_field_id target-id}]] (testing "vanilla user-created dimensions" - (let [ser (serdes.base/extract-one "Dimension" {} (select-one "Dimension" [:= :id dim1-id]))] + (let [ser (serdes.base/extract-one "Dimension" {} (db/select-one 'Dimension :id dim1-id))] (is (schema= {:serdes/meta (s/eq [{:model "Dimension" :id dim1-eid}]) :field_id (s/eq ["My Database" nil "Schemaless Table" "email"]) :human_readable_field_id (s/eq nil) @@ -482,7 +491,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "foreign key dimensions" - (let [ser (serdes.base/extract-one "Dimension" {} (select-one "Dimension" [:= :id dim2-id]))] + (let [ser (serdes.base/extract-one "Dimension" {} (db/select-one 'Dimension :id dim2-id))] (is (schema= {:serdes/meta (s/eq [{:model "Dimension" :id dim2-eid}]) :field_id (s/eq ["My Database" "PUBLIC" "Schema'd Table" "foreign_id"]) :human_readable_field_id (s/eq ["My Database" "PUBLIC" "Foreign Table" "real_field"]) @@ -518,7 +527,7 @@ {:source-table no-schema-id :aggregation [[:sum [:field field-id nil]]]}}]] (testing "metrics" - (let [ser (serdes.base/extract-one "Metric" {} (select-one "Metric" [:= :id m1-id]))] + (let [ser (serdes.base/extract-one "Metric" {} (db/select-one 'Metric :id m1-id))] (is (schema= {:serdes/meta (s/eq [{:model "Metric" :id m1-eid :label "My Metric"}]) :table_id (s/eq ["My Database" nil "Schemaless Table"]) :creator_id (s/eq "ann@heart.band") @@ -558,7 +567,7 @@ :creator_id ann-id}]] (testing "native query snippets" (testing "can belong to :snippets collections" - (let [ser (serdes.base/extract-one "NativeQuerySnippet" {} (select-one "NativeQuerySnippet" [:= :id s1-id]))] + (let [ser (serdes.base/extract-one "NativeQuerySnippet" {} (db/select-one 'NativeQuerySnippet :id s1-id))] (is (schema= {:serdes/meta (s/eq [{:model "NativeQuerySnippet" :id s1-eid :label "Snippet 1"}]) @@ -574,7 +583,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "or can be outside collections" - (let [ser (serdes.base/extract-one "NativeQuerySnippet" {} (select-one "NativeQuerySnippet" [:= :id s2-id]))] + (let [ser (serdes.base/extract-one "NativeQuerySnippet" {} (db/select-one 'NativeQuerySnippet :id s2-id))] (is (schema= {:serdes/meta (s/eq [{:model "NativeQuerySnippet" :id s2-eid :label "Snippet 2"}]) @@ -610,7 +619,7 @@ :timeline_id line-id}]] (testing "timelines" (testing "with no events" - (let [ser (serdes.base/extract-one "Timeline" {} (select-one "Timeline" [:= :id empty-id]))] + (let [ser (serdes.base/extract-one "Timeline" {} (db/select-one 'Timeline :id empty-id))] (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id empty-eid}]) :collection_id (s/eq coll-eid) :creator_id (s/eq "ann@heart.band") @@ -624,7 +633,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "with events" - (let [ser (serdes.base/extract-one "Timeline" {} (select-one "Timeline" [:= :id line-id]))] + (let [ser (serdes.base/extract-one "Timeline" {} (db/select-one 'Timeline :id line-id))] (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id line-eid}]) :collection_id (s/eq coll-eid) :creator_id (s/eq "ann@heart.band") @@ -638,7 +647,7 @@ (set (serdes.base/serdes-dependencies ser)))))))) (testing "timeline events" - (let [ser (serdes.base/extract-one "TimelineEvent" {} (select-one "TimelineEvent" [:= :id e1-id])) + (let [ser (serdes.base/extract-one "TimelineEvent" {} (db/select-one 'TimelineEvent :id e1-id)) stamp "2020-04-11T00:00:00Z"] (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id line-eid} {:model "TimelineEvent" @@ -672,13 +681,13 @@ :aggregation [[:count]] :filter [:< [:field field-id nil] 18]}}]] (testing "segment" - (let [ser (serdes.base/extract-one "Segment" {} (select-one "Segment" [:= :id s1-id]))] + (let [ser (serdes.base/extract-one "Segment" {} (db/select-one 'Segment :id s1-id))] (is (schema= {:serdes/meta (s/eq [{:model "Segment" :id s1-eid :label "My Segment"}]) :table_id (s/eq ["My Database" nil "Schemaless Table"]) :creator_id (s/eq "ann@heart.band") :definition (s/eq {:source-table ["My Database" nil "Schemaless Table"] :aggregation [[:count]] - :filter ["<" [:field ["My Database" nil + :filter [:< [:field ["My Database" nil "Schemaless Table" "Some Field"] nil] 18]}) :created_at LocalDateTime @@ -717,13 +726,13 @@ "Diner" "Indian" "Italian" "Japanese" "Mexican" "Middle Eastern" "Pizza" "Seafood" "Steakhouse" "Tea Room" "Winery"]}]] (testing "field values" - (let [ser (serdes.base/extract-one "FieldValues" {} (select-one "FieldValues" [:= :id fv-id]))] + (let [ser (serdes.base/extract-one "FieldValues" {} (db/select-one 'FieldValues :id fv-id))] (is (schema= {:serdes/meta (s/eq [{:model "Database" :id "My Database"} {:model "Table" :id "Schemaless Table"} {:model "Field" :id "Some Field"} {:model "FieldValues" :id "0"}]) ; Always 0. :created_at LocalDateTime - :values (s/eq (json/generate-string values)) + :values (s/eq values) s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -762,7 +771,7 @@ :collection_id coll-id :dashboard_id dash-id}]] (testing "pulse with neither collection nor dashboard" - (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-none-id]))] + (let [ser (serdes.base/extract-one "Pulse" {} (db/select-one 'Pulse :id p-none-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-none-eid}]) :creator_id (s/eq "ann@heart.band") (s/optional-key :dashboard_id) (s/eq nil) @@ -777,7 +786,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "pulse with just collection" - (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-coll-id]))] + (let [ser (serdes.base/extract-one "Pulse" {} (db/select-one 'Pulse :id p-coll-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-coll-eid}]) :creator_id (s/eq "ann@heart.band") (s/optional-key :dashboard_id) (s/eq nil) @@ -792,7 +801,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "pulse with just dashboard" - (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-dash-id]))] + (let [ser (serdes.base/extract-one "Pulse" {} (db/select-one 'Pulse :id p-dash-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-dash-eid}]) :creator_id (s/eq "ann@heart.band") :dashboard_id (s/eq dash-eid) @@ -807,7 +816,7 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "pulse with both collection and dashboard" - (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-both-id]))] + (let [ser (serdes.base/extract-one "Pulse" {} (db/select-one 'Pulse :id p-both-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-both-eid}]) :creator_id (s/eq "ann@heart.band") :dashboard_id (s/eq dash-eid) @@ -827,7 +836,8 @@ (ts/with-temp-dpc [User [{ann-id :id} {:first_name "Ann" :last_name "Wilson" :email "ann@heart.band"}] - Dashboard [{dash-id :id} {:name "A Dashboard"}] + Dashboard [{dash-id :id + dash-eid :entity_id} {:name "A Dashboard"}] Database [{db-id :id} {:name "My Database"}] Table [{table-id :id} {:name "Schemaless Table" :db_id db-id}] Card [{card1-id :id @@ -860,7 +870,7 @@ :position 1 :dashboard_card_id dashcard-id}]] (testing "legacy pulse cards" - (let [ser (serdes.base/extract-one "PulseCard" {} (select-one "PulseCard" [:= :id pc1-pulse-id]))] + (let [ser (serdes.base/extract-one "PulseCard" {} (db/select-one 'PulseCard :id pc1-pulse-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id pulse-eid} {:model "PulseCard" :id pc1-pulse-eid}]) :card_id (s/eq card1-eid) @@ -874,7 +884,7 @@ [{:model "Card" :id card1-eid}]} (set (serdes.base/serdes-dependencies ser)))))) - (let [ser (serdes.base/extract-one "PulseCard" {} (select-one "PulseCard" [:= :id pc2-pulse-id]))] + (let [ser (serdes.base/extract-one "PulseCard" {} (db/select-one 'PulseCard :id pc2-pulse-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id pulse-eid} {:model "PulseCard" :id pc2-pulse-eid}]) :card_id (s/eq card1-eid) @@ -889,11 +899,11 @@ (set (serdes.base/serdes-dependencies ser))))))) (testing "dashboard sub cards" - (let [ser (serdes.base/extract-one "PulseCard" {} (select-one "PulseCard" [:= :id pc1-sub-id]))] + (let [ser (serdes.base/extract-one "PulseCard" {} (db/select-one 'PulseCard :id pc1-sub-id))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id sub-eid} {:model "PulseCard" :id pc1-sub-eid}]) :card_id (s/eq card1-eid) - :dashboard_card_id (s/eq dashcard-eid) + :dashboard_card_id (s/eq [dash-eid dashcard-eid]) s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -901,7 +911,7 @@ (testing "depends on the pulse, card and dashcard" (is (= #{[{:model "Pulse" :id sub-eid}] [{:model "Card" :id card1-eid}] - [{:model "DashboardCard" :id dashcard-eid}]} + [{:model "Dashboard" :id dash-eid} {:model "DashboardCard" :id dashcard-eid}]} (set (serdes.base/serdes-dependencies ser)))))))))) (deftest selective-serialization-basic-test 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 e0fdc1cb9dc..a433e6d257d 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj @@ -303,9 +303,9 @@ (reset! serialized (into [] (serdes.extract/extract-metabase {}))))) (testing "the serialized form is as desired" - (is (= {:type "query" + (is (= {:type :query :query {:source-table ["my-db" nil "customers"] - :filter [">=" [:field ["my-db" nil "customers" "age"] nil] 18] + :filter [:>= [:field ["my-db" nil "customers" "age"] nil] 18] :aggregation [[:count]]} :database "my-db"} (->> (by-model @serialized "Card") @@ -385,7 +385,7 @@ (testing "exported form is properly converted" (is (= {:source-table ["my-db" nil "customers"] :aggregation [[:count]] - :filter ["<" [:field ["my-db" nil "customers" "age"] nil] 18]} + :filter [:< [:field ["my-db" nil "customers" "age"] nil] 18]} (-> @serialized (by-model "Segment") first @@ -598,16 +598,16 @@ :table.cell_column "sum" :table.columns [{:name "SOME_FIELD" - :fieldRef ["field" ["my-db" nil "orders" "subtotal"] nil] + :fieldRef [:field ["my-db" nil "orders" "subtotal"] nil] :enabled true} {:name "sum" - :fieldRef ["field" "sum" {:base-type "type/Float"}] + :fieldRef [:field "sum" {:base-type :type/Float}] :enabled true} {:name "count" - :fieldRef ["field" "count" {:base-type "type/BigInteger"}] + :fieldRef [:field "count" {:base-type :type/BigInteger}] :enabled true} {:name "Average order total" - :fieldRef ["field" "Average order total" {:base-type "type/Float"}] + :fieldRef [:field "Average order total" {:base-type :type/Float}] :enabled true}] :column_settings {"[\"ref\",[\"field\",[\"my-db\",null,\"orders\",\"invoice\"],null]]" {:column_title "Locus"}}}] @@ -678,8 +678,8 @@ (ts/with-source-db (reset! user1s (ts/create! User :first_name "Tom" :last_name "Scholz" :email "tom@bost.on")) (reset! user2s (ts/create! User :first_name "Neil" :last_name "Peart" :email "neil@rush.yyz")) - (reset! metric1s (ts/create! Metric :name "Large Users" :creator_id (:id @user1s))) - (reset! metric2s (ts/create! Metric :name "Support Headaches" :creator_id (:id @user2s))) + (reset! metric1s (ts/create! Metric :name "Large Users" :creator_id (:id @user1s) :definition {:aggregation [[:count]]})) + (reset! metric2s (ts/create! Metric :name "Support Headaches" :creator_id (:id @user2s) :definition {:aggregation [[:count]]})) (reset! serialized (into [] (serdes.extract/extract-metabase {}))))) (testing "exported form is properly converted" diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index b2d03e0f807..ed4d5e0539d 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -331,15 +331,8 @@ [:name (serdes.hash/hydrated-hash :collection)]) ;;; ------------------------------------------------- Serialization -------------------------------------------------- -(defmethod serdes.base/extract-query "Card" [_ {:keys [user]}] - (serdes.base/raw-reducible-query - "Card" - {:select [:card.*] - :from [[:report_card :card]] - :left-join [[:collection :coll] [:= :coll.id :card.collection_id]] - :where (if user - [:or [:= :coll.personal_owner_id user] [:is :coll.personal_owner_id nil]] - [:is :coll.personal_owner_id nil])})) +(defmethod serdes.base/extract-query "Card" [_ {:keys [collection-set]}] + (db/select-reducible Card :collection_id [:in collection-set])) (defmethod serdes.base/extract-one "Card" [_model-name _opts card] @@ -353,7 +346,7 @@ (update :collection_id serdes.util/export-fk 'Collection) (update :creator_id serdes.util/export-user) (update :made_public_by_id serdes.util/export-user) - (update :dataset_query serdes.util/export-json-mbql) + (update :dataset_query serdes.util/export-mbql) (update :parameter_mappings serdes.util/export-parameter-mappings) (update :visualization_settings serdes.util/export-visualization-settings) (dissoc :result_metadata))) ; Not portable, and can be rebuilt on the other side. @@ -367,7 +360,7 @@ (update :creator_id serdes.util/import-user) (update :made_public_by_id serdes.util/import-user) (update :collection_id serdes.util/import-fk 'Collection) - (update :dataset_query serdes.util/import-json-mbql) + (update :dataset_query serdes.util/import-mbql) (update :parameter_mappings serdes.util/import-parameter-mappings) (update :visualization_settings serdes.util/import-visualization-settings))) diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index 304d14fb422..c85e90dd3c0 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -161,7 +161,7 @@ (let [msg (tru "Invalid Collection location: path is invalid.")] (throw (ex-info msg {:status-code 400, :errors {:location msg}})))) ;; if this is a Personal Collection it's only allowed to go in the Root Collection: you can't put it anywhere else! - (when (contains? collection :personal_owner_id) + (when (:personal_owner_id collection) (when-not (= location "/") (let [msg (tru "You cannot move a Personal Collection.")] (throw (ex-info msg {:status-code 400, :errors {:location msg}}))))) @@ -351,7 +351,7 @@ highest-level (e.g. most distant) ancestor." [{:keys [location]}] (when-let [ancestor-ids (seq (location-path->ids location))] - (db/select [Collection :name :id] :id [:in ancestor-ids] {:order-by [:%lower.name]}))) + (db/select [Collection :name :id] :id [:in ancestor-ids] {:order-by [:location]}))) (s/defn effective-ancestors :- [(s/cond-pre RootCollection (mi/InstanceOf Collection))] "Fetch the ancestors of a `collection`, filtering out any ones the current User isn't allowed to see. This is used @@ -907,20 +907,8 @@ :pre-update pre-update :pre-delete pre-delete})) -(defn- collection-query [maybe-user] - (serdes.base/raw-reducible-query - "Collection" - {:where [:and - [:= :archived false] - (if (nil? maybe-user) - [:is :personal_owner_id nil] - [:= :personal_owner_id maybe-user])]})) - -(defmethod serdes.base/extract-query "Collection" [_ {:keys [user]}] - (let [unowned (collection-query nil)] - (if user - (eduction cat [unowned (collection-query user)]) - unowned))) +(defmethod serdes.base/extract-query "Collection" [_ {:keys [collection-set]}] + (db/select-reducible Collection :id [:in collection-set])) (defmethod serdes.base/extract-one "Collection" ;; Transform :location (which uses database IDs) into a portable :parent_id with the parent's entity ID. diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index d6d2536110e..428f9ea10ed 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -409,16 +409,8 @@ ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | SERIALIZATION | ;;; +----------------------------------------------------------------------------------------------------------------+ -(defmethod serdes.base/extract-query "Dashboard" [_ {:keys [user]}] - ;; TODO This join over the subset of collections this user can see is shared by a few things - factor it out? - (serdes.base/raw-reducible-query - "Dashboard" - {:select [:dash.*] - :from [[:report_dashboard :dash]] - :left-join [[:collection :coll] [:= :coll.id :dash.collection_id]] - :where (if user - [:or [:= :coll.personal_owner_id user] [:is :coll.personal_owner_id nil]] - [:is :coll.personal_owner_id nil])})) +(defmethod serdes.base/extract-query "Dashboard" [_ {:keys [collection-set]}] + (db/select-reducible Dashboard :collection_id [:in collection-set])) ;; TODO Maybe nest collections -> dashboards -> dashcards? (defmethod serdes.base/extract-one "Dashboard" diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index 13cae654f70..ee6b7076e6f 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/src/metabase/models/dashboard_card.clj @@ -52,7 +52,7 @@ (defmethod serdes.hash/identity-hash-fields DashboardCard [_dashboard-card] - [(serdes.hash/hydrated-hash :card) + [(serdes.hash/hydrated-hash :card "<none>") ; :card is optional, eg. text cards (comp serdes.hash/identity-hash #(db/select-one 'Dashboard :id %) :dashboard_id) @@ -219,17 +219,9 @@ (events/publish-event! :dashboard-remove-cards {:id id :actor_id user-id :dashcards [dashboard-card]}))) ;;; ----------------------------------------------- SERIALIZATION ---------------------------------------------------- -(defmethod serdes.base/extract-query "DashboardCard" [_ {:keys [user]}] - ;; TODO This join over the subset of collections this user can see is shared by a few things - factor it out? - (serdes.base/raw-reducible-query - "DashboardCard" - {:select [:dc.*] - :from [[:report_dashboardcard :dc]] - :left-join [[:report_dashboard :dash] [:= :dash.id :dc.dashboard_id] - [:collection :coll] [:= :coll.id :dash.collection_id]] - :where (if user - [:or [:= :coll.personal_owner_id user] [:is :coll.personal_owner_id nil]] - [:is :coll.personal_owner_id nil])})) +(defmethod serdes.base/extract-query "DashboardCard" [_ {:keys [collection-set]}] + (let [dashboards (db/select-ids 'Dashboard :collection_id [:in collection-set])] + (db/select-reducible DashboardCard :dashboard_id [:in dashboards]))) (defmethod serdes.base/serdes-dependencies "DashboardCard" [{:keys [card_id dashboard_id parameter_mappings visualization_settings]}] @@ -260,7 +252,11 @@ (update :visualization_settings serdes.util/import-visualization-settings))) (defmethod serdes.base/serdes-descendants "DashboardCard" [_model-name id] - (let [{:keys [card_id dashboard_id]} (db/select-one DashboardCard :id id)] + (let [{:keys [card_id dashboard_id parameter_mappings]} (db/select-one DashboardCard :id id) + cards-in-params (set (for [{:keys [card_id]} parameter_mappings + :when card_id] + ["Card" card_id]))] (cond-> #{["Dashboard" dashboard_id]} ;; card_id is nil for text cards; in that case there's no Card to depend on. - card_id (conj ["Card" card_id])))) + card_id (conj ["Card" card_id]) + (seq cards-in-params) (set/union cards-in-params)))) diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index b2c2b415d5c..c6a0cea1350 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -293,6 +293,7 @@ ;; There's one optional foreign key: creator_id. Resolve it as an email. (cond-> (serdes.base/extract-one-basics "Database" entity) true (update :creator_id serdes.util/export-user) + true (dissoc :features) ; This is a synthetic column that isn't in the real schema. (= :exclude secrets) (dissoc :details))) (defmethod serdes.base/serdes-entity-id "Database" diff --git a/src/metabase/models/dimension.clj b/src/metabase/models/dimension.clj index 8b38ada9140..86b12a308fa 100644 --- a/src/metabase/models/dimension.clj +++ b/src/metabase/models/dimension.clj @@ -24,8 +24,8 @@ (defmethod serdes.hash/identity-hash-fields Dimension [_dimension] - [(serdes.hash/hydrated-hash :field) - (serdes.hash/hydrated-hash :human_readable_field)]) + [(serdes.hash/hydrated-hash :field "<none>") + (serdes.hash/hydrated-hash :human_readable_field "<none>")]) ;;; ------------------------------------------------- Serialization -------------------------------------------------- (defmethod serdes.base/extract-one "Dimension" diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj index 653d3c04964..40890d8a12e 100644 --- a/src/metabase/models/field_values.clj +++ b/src/metabase/models/field_values.clj @@ -440,9 +440,19 @@ ;; It's too short, so no schema. Shift them over and add a nil schema. [db nil schema table])] (-> (serdes.base/load-xform-basics fv) - (assoc :field_id (serdes.util/import-field-fk field-ref))))) + (assoc :field_id (serdes.util/import-field-fk field-ref)) + (update :type keyword)))) (defmethod serdes.base/load-find-local "FieldValues" [path] ;; Delegate to finding the parent Field, then look up its corresponding FieldValues. (let [field-id (serdes.base/load-find-local (pop path))] (db/select-one-id FieldValues :field_id field-id))) + +(defmethod serdes.base/load-update! "FieldValues" [_ ingested local] + ;; It's illegal to change the :type and :hash_key fields, and there's a pre-update check for this. + ;; This drops those keys from the incoming FieldValues iff they match the local one. If they are actually different, + ;; this preserves the new value so the normal error is produced. + (let [ingested (cond-> ingested + (= (:type ingested) (:type local)) (dissoc :type) + (= (:hash_key ingested) (:hash_key local)) (dissoc :hash_key))] + ((get-method serdes.base/load-update! "") "FieldValues" ingested local))) diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index e4948333e43..8c21a54aad1 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -30,6 +30,12 @@ model instance]) +(def ^:dynamic *deserializing?* + "This is dynamically bound to true when deserializing. A few pieces of the Toucan magic are undesirable for + deserialization. Most notably, we don't want to generate an `:entity_id`, as that would lead to duplicated entities + on a future deserialization." + false) + ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | Toucan Extensions | ;;; +----------------------------------------------------------------------------------------------------------------+ @@ -249,7 +255,10 @@ :update add-updated-at-timestamp) (defn- add-entity-id [obj & _] - (if (contains? obj :entity_id) + (if (or (contains? obj :entity_id) + *deserializing?*) + ;; Don't generate a new entity_id if either: (a) there's already one set; or (b) we're deserializing. + ;; Generating them at deserialization time can lead to duplicated entities if they're deserialized again. obj (assoc obj :entity_id (u/generate-nano-id)))) diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj index c60620fa185..0a60937f149 100644 --- a/src/metabase/models/metric.clj +++ b/src/metabase/models/metric.clj @@ -48,7 +48,7 @@ (defmethod serdes.hash/identity-hash-fields Metric [_metric] - [:name (serdes.hash/hydrated-hash :table)]) + [:name (serdes.hash/hydrated-hash :table "<none>")]) ;;; --------------------------------------------------- REVISIONS ---------------------------------------------------- @@ -82,22 +82,19 @@ (let [base (serdes.base/infer-self-path "Metric" metric)] [(assoc base :label (:name metric))])) -(defmethod serdes.base/extract-query "Metric" [_model-name _opts] - (serdes.base/raw-reducible-query "Metric" {:where [:= :archived false]})) - (defmethod serdes.base/extract-one "Metric" [_model-name _opts metric] (-> (serdes.base/extract-one-basics "Metric" metric) (update :table_id serdes.util/export-table-fk) (update :creator_id serdes.util/export-user) - (update :definition serdes.util/export-json-mbql))) + (update :definition serdes.util/export-mbql))) (defmethod serdes.base/load-xform "Metric" [metric] (-> metric serdes.base/load-xform-basics (update :table_id serdes.util/import-table-fk) (update :creator_id serdes.util/import-user) - (update :definition serdes.util/import-json-mbql))) + (update :definition serdes.util/import-mbql))) (defmethod serdes.base/serdes-dependencies "Metric" [{:keys [definition table_id]}] (into [] (set/union #{(serdes.util/table->path table_id)} diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj index 4fc8f85021f..f97cbeab1cb 100644 --- a/src/metabase/models/native_query_snippet.clj +++ b/src/metabase/models/native_query_snippet.clj @@ -43,7 +43,7 @@ (defmethod serdes.hash/identity-hash-fields NativeQuerySnippet [_snippet] - [:name (serdes.hash/hydrated-hash :collection)]) + [:name (serdes.hash/hydrated-hash :collection "<none>")]) (defmethod mi/can-read? NativeQuerySnippet [& args] @@ -75,18 +75,10 @@ ;;; ------------------------------------------------- Serialization -------------------------------------------------- -(defmethod serdes.base/extract-query "NativeQuerySnippet" [_ {:keys [user]}] - ;; TODO This join over the subset of collections this user can see is shared by a few things - factor it out? - (serdes.base/raw-reducible-query - "NativeQuerySnippet" - {:select [:snippet.*] - :from [[:native_query_snippet :snippet]] - :left-join [[:collection :coll] [:= :coll.id :snippet.collection_id]] - :where (if user - ;; :snippet.collection_id is nullable, but this is a left join, so it works out neatly: - ;; if this snippet has no collection, :coll.personal_owner_id is effectively NULL. - [:or [:= :coll.personal_owner_id user] [:is :coll.personal_owner_id nil]] - [:is :coll.personal_owner_id nil])})) +(defmethod serdes.base/extract-query "NativeQuerySnippet" [_ {:keys [collection-set]}] + (eduction cat [(db/select-reducible NativeQuerySnippet :collection_id nil) + (when (seq collection-set) + (db/select-reducible NativeQuerySnippet :collection_id [:in collection-set]))])) (defmethod serdes.base/serdes-generate-path "NativeQuerySnippet" [_ snippet] [(assoc (serdes.base/infer-self-path "NativeQuerySnippet" snippet) diff --git a/src/metabase/models/pulse.clj b/src/metabase/models/pulse.clj index f0b4a0532b4..a6fa2661848 100644 --- a/src/metabase/models/pulse.clj +++ b/src/metabase/models/pulse.clj @@ -137,7 +137,7 @@ (defmethod serdes.hash/identity-hash-fields Pulse [_pulse] - [:name (serdes.hash/hydrated-hash :collection)]) + [:name (serdes.hash/hydrated-hash :collection "<none>")]) (def ^:private ^:dynamic *automatically-archive-when-last-channel-is-deleted* "Should we automatically archive a Pulse when its last `PulseChannel` is deleted? Normally we do, but this is disabled diff --git a/src/metabase/models/pulse_card.clj b/src/metabase/models/pulse_card.clj index 4e9bc4be738..12e86f9bada 100644 --- a/src/metabase/models/pulse_card.clj +++ b/src/metabase/models/pulse_card.clj @@ -67,12 +67,13 @@ (cond-> (serdes.base/load-xform-basics card) true (update :card_id serdes.util/import-fk 'Card) true (update :pulse_id serdes.util/import-fk 'Pulse) + true (dissoc :dashboard_id) (:dashboard_card_id card) (update :dashboard_card_id serdes.util/import-fk 'DashboardCard))) ;; Depends on the Pulse, Card and (optional) dashboard card. (defmethod serdes.base/serdes-dependencies "PulseCard" [{:keys [card_id dashboard_card_id pulse_id]}] (let [base [[{:model "Card" :id card_id}] [{:model "Pulse" :id pulse_id}]]] - (if dashboard_card_id - (conj base [{:model "DashboardCard" :id dashboard_card_id}]) + (if-let [[dash-id dc-id] dashboard_card_id] + (conj base [{:model "Dashboard" :id dash-id} {:model "DashboardCard" :id dc-id}]) base))) diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj index bde7afd8202..52f06863e70 100644 --- a/src/metabase/models/pulse_channel.clj +++ b/src/metabase/models/pulse_channel.clj @@ -376,7 +376,10 @@ (update :pulse_id serdes.util/import-fk 'Pulse))) (defn- import-recipients [channel-id emails] - (let [incoming-users (db/select-ids 'User :email [:in emails]) + (let [incoming-users (set (for [email emails + :let [id (db/select-one-id 'User :email email)]] + (or id + (:id (user/serdes-synthesize-user! {:email email}))))) current-users (set (db/select-field :user_id PulseChannelRecipient :pulse_channel_id channel-id)) combined (set/union incoming-users current-users)] (when-not (empty? combined) @@ -385,14 +388,15 @@ ;; Customized load-insert! and load-update! to handle the embedded recipients field - it's really a separate table. (defmethod serdes.base/load-insert! "PulseChannel" [_ ingested] (let [;; Call through to the default load-insert! - id ((get-method serdes.base/load-insert! "") "PulseChannel" (dissoc ingested :recipients))] - (import-recipients id (:recipients ingested)))) + chan ((get-method serdes.base/load-insert! "") "PulseChannel" (dissoc ingested :recipients))] + (import-recipients (:id chan) (:recipients ingested)) + chan)) (defmethod serdes.base/load-update! "PulseChannel" [_ ingested local] ;; Call through to the default load-update! - ((get-method serdes.base/load-update! "") "PulseChannel" (dissoc ingested :recipients) local) - (import-recipients (:id local) (:recipients ingested)) - (:id local)) + (let [chan ((get-method serdes.base/load-update! "") "PulseChannel" (dissoc ingested :recipients) local)] + (import-recipients (:id local) (:recipients ingested)) + chan)) ;; Depends on the Pulse. (defmethod serdes.base/serdes-dependencies "PulseChannel" [{:keys [pulse_id]}] diff --git a/src/metabase/models/segment.clj b/src/metabase/models/segment.clj index c912c75f360..a5ddcab911d 100644 --- a/src/metabase/models/segment.clj +++ b/src/metabase/models/segment.clj @@ -87,14 +87,14 @@ (-> (serdes.base/extract-one-basics "Segment" segment) (update :table_id serdes.util/export-table-fk) (update :creator_id serdes.util/export-user) - (update :definition serdes.util/export-json-mbql))) + (update :definition serdes.util/export-mbql))) (defmethod serdes.base/load-xform "Segment" [segment] (-> segment serdes.base/load-xform-basics (update :table_id serdes.util/import-table-fk) (update :creator_id serdes.util/import-user) - (update :definition serdes.util/import-json-mbql))) + (update :definition serdes.util/import-mbql))) (defmethod serdes.base/serdes-dependencies "Segment" [{:keys [definition table_id]}] (into [] (set/union #{(serdes.util/table->path table_id)} diff --git a/src/metabase/models/serialization/base.clj b/src/metabase/models/serialization/base.clj index 925eaa92d8e..231271594ae 100644 --- a/src/metabase/models/serialization/base.clj +++ b/src/metabase/models/serialization/base.clj @@ -166,20 +166,11 @@ Keyed on the model name, the first argument. - Returns a reducible stream of maps with `:serdes/meta` keys on them. It should *not* be a stream of Toucan entities, - but vanilla Clojure maps. + Returns a reducible stream of modeled Toucan maps. - In fact, Toucan's high-level niceties (eg. expanding JSON-encoded fields to Clojure data, decrypting, type - conversions, or hydrating some relationship by default) are counterproductive when our goal is a database-level - export. As a specific example, [[db/simple-select]] expands JSON but [[db/simple-insert!]] doesn't put it back. - There's also no `simple-update!`, making a fresh insert diverge from an update. + Defaults to using `(toucan.db/select model)` for the entire table. - Defaults to using the helper `(raw-reducible-query model)` for the entire table, which is equivalent to - `(db/simple-select-reducible model)` but without running post-select handlers. This returns vanilla maps, not - [[db/IModel]] instances. - - You may want to override this to eg. skip archived entities, or otherwise filter what gets serialized. Prefer using - the two-argument form of [[raw-reducible-query]]." + You may want to override this to eg. skip archived entities, or otherwise filter what gets serialized." (fn [model _] model)) (defmulti extract-one @@ -205,27 +196,8 @@ (eduction (map (partial extract-one model opts)) (extract-query model opts))) -(defn- model-name->table - "The model name is not necessarily the table name. This pulls the table name from the Toucan model." - [model-name] - (-> model-name - symbol - db/resolve-model - :table)) - -(defn raw-reducible-query - "Helper for calling Toucan's raw [[db/reducible-query]]. With just the model name, fetches everything. You can filter - with a HoneySQL map like `{:where [:= :archived true]}`. - - Returns a reducible stream of JDBC row maps." - ([model-name] - (raw-reducible-query model-name nil)) - ([model-name honeysql-form] - (db/reducible-query (merge {:select [:*] :from [(model-name->table model-name)]} - honeysql-form)))) - (defmethod extract-query :default [model-name _] - (raw-reducible-query model-name)) + (db/select (symbol model-name))) (defn extract-one-basics "A helper for writing [[extract-one]] implementations. It takes care of the basics: @@ -238,9 +210,9 @@ [model-name entity] (let [model (db/resolve-model (symbol model-name)) pk (models/primary-key model)] - (-> entity - (assoc :serdes/meta (serdes-generate-path model-name entity)) - (dissoc pk :updated_at)))) + (-> (into {} entity) + (assoc :serdes/meta (serdes-generate-path model-name entity)) + (dissoc pk :updated_at)))) (defmethod extract-one :default [model-name _opts entity] (extract-one-basics model-name entity)) @@ -292,7 +264,6 @@ ;;; - `(load-update! ingested local-entity)` if the local entity exists, or ;;; - `(load-insert! ingested)` if the entity is new. ;;; Both of these have the obvious defaults of [[jdbc/update!]] or [[jdbc/insert!]]. - (defn- ingested-model "The dispatch function for several of the load multimethods: dispatching on the model of the incoming entity." [ingested] @@ -341,7 +312,7 @@ (defmulti load-xform "Given the incoming vanilla map as ingested, transform it so it's suitable for sending to the database (in eg. - [[db/simple-insert!]]). + [[db/insert!]]). For example, this should convert any foreign keys back from a portable entity ID or identity hash into a numeric database ID. This is the mirror of [[extract-one]], in spirit. (They're not strictly inverses - [[extract-one]] drops the primary key but this need not put one back, for example.) @@ -372,49 +343,36 @@ Keyed on the model name (the first argument), because the second argument doesn't have its `:serdes/meta` anymore. - Returns the primary key of the updated entity." + Returns the updated entity." {:arglists '([model-name ingested local])} (fn [model _ _] model)) (defmethod load-update! :default [model-name ingested local] (let [model (db/resolve-model (symbol model-name)) pk (models/primary-key model) - id (get local pk) - adjusted (if (-> model models/properties :timestamped?) - (assoc ingested :updated_at (mi/now)) - ingested)] + id (get local pk)] (log/tracef "Upserting %s %d: old %s new %s" model-name id (pr-str local) (pr-str ingested)) - ;; Using the two-argument form of [[db/update!]] that takes the model and a HoneySQL form for the actual update. - ;; It works differently from the more typical `(db/update! 'Model id updates...)` form: this form doesn't run any of - ;; the pre-update magic, it just updates the database directly. - ;; Therefore we manually set the :updated_at time. - (db/update! model {:where [:= pk id] :set adjusted}) - id)) + (db/update! model id ingested) + (db/select-one model pk id))) (defmulti load-insert! "Called by the default [[load-one!]] if there is no corresponding entity already in the appdb. `(load-insert! \"ModelName\" ingested-and-xformed)` - Defaults to a straightforward [[db/simple-insert!]], and you probably don't need to implement this. - Note that [[db/insert!]] should be avoided - we don't want to populate the `:entity_id` field if it wasn't already - set! + Defaults to a straightforward [[db/insert!]], and you probably don't need to implement this. + + Note that any [[db/insert!]] behavior we don't want to run (like generating an `:entity_id`!) should be skipped based + on the [[mi/*deserializing?*]] dynamic var. Keyed on the model name (the first argument), because the second argument doesn't have its `:serdes/meta` anymore. - Returns the primary key of the newly inserted entity." + Returns the newly inserted entity." {:arglists '([model ingested])} (fn [model _] model)) -(defmethod load-insert! :default [model ingested] - (log/tracef "Inserting %s: %s" model (pr-str ingested)) - ; Toucan's simple-insert! actually does the right thing for our purposes: it doesn't call pre-insert or post-insert, - ; and it returns the new primary key. - (let [model (db/resolve-model (symbol model)) - adjusted (if (-> model models/properties :timestamped?) - (let [now (mi/now)] - (assoc ingested :created_at now :updated_at now)) - ingested)] - (db/simple-insert! model adjusted))) +(defmethod load-insert! :default [model-name ingested] + (log/tracef "Inserting %s: %s" model-name (pr-str ingested)) + (db/insert! (symbol model-name) ingested)) (defmulti load-one! "Black box for integrating a deserialized entity into this appdb. @@ -438,9 +396,10 @@ (let [model (ingested-model ingested) pkey (models/primary-key (db/resolve-model (symbol model))) adjusted (load-xform ingested)] - (if (nil? maybe-local-id) - (load-insert! model adjusted) - (load-update! model adjusted (db/select-one (symbol model) pkey maybe-local-id))))) + (binding [mi/*deserializing?* true] + (if (nil? maybe-local-id) + (load-insert! model adjusted) + (load-update! model adjusted (db/select-one (symbol model) pkey maybe-local-id)))))) (defmulti serdes-descendants "Captures the notion that eg. a dashboard \"contains\" its cards. diff --git a/src/metabase/models/serialization/hash.clj b/src/metabase/models/serialization/hash.clj index bd4f3934cc3..29bc8118ebf 100644 --- a/src/metabase/models/serialization/hash.clj +++ b/src/metabase/models/serialization/hash.clj @@ -81,13 +81,15 @@ (defn hydrated-hash "Many entities reference other entities. Using the autoincrementing ID is not portable, so we use the identity hash of the referenced entity. This is a helper for writing [[identity-hash-fields]]." - [hydration-key] - (fn [entity] - (let [hydrated-value (get (hydrate entity hydration-key) hydration-key)] - (when (nil? hydrated-value) - (throw (ex-info (tru "Error calculating hydrated hash: {0} is nil after hydrating {1}" - (pr-str hydration-key) - (name entity)) - {:entity entity - :hydration-key hydration-key}))) - (identity-hash hydrated-value)))) + ([hydration-key] (hydrated-hash hydration-key nil)) + ([hydration-key default] + (fn [entity] + (let [hydrated-value (get (hydrate entity hydration-key) hydration-key)] + (cond + hydrated-value (identity-hash hydrated-value) + default default + :else (throw (ex-info (tru "Error calculating hydrated hash: {0} is nil after hydrating {1}" + (pr-str hydration-key) + (name entity)) + {:entity entity + :hydration-key hydration-key}))))))) diff --git a/src/metabase/models/serialization/util.clj b/src/metabase/models/serialization/util.clj index b2b1ceb82b7..50a1d940adc 100644 --- a/src/metabase/models/serialization/util.clj +++ b/src/metabase/models/serialization/util.clj @@ -20,19 +20,28 @@ (defn export-fk "Given a numeric foreign key and its model (symbol, name or IModel), looks up the entity by ID and gets its entity ID or identity hash. - Unusual parameter order means this can be used as `(update x :some_id export-fk 'SomeModel)`." + Unusual parameter order means this can be used as `(update x :some_id export-fk 'SomeModel)`. + + NOTE: This works for both top-level and nested entities. Top-level entities like `Card` are returned as just a + portable ID string.. Nested entities are returned as a vector of such ID strings." [id model] (when id (let [model-name (name model) model (db/resolve-model (symbol model-name)) entity (db/select-one model (models/primary-key model) id) - {eid :id} (serdes.base/infer-self-path model-name entity)] - eid))) + path (mapv :id (serdes.base/serdes-generate-path model-name entity))] + (if (= (count path) 1) + (first path) + path)))) (defn import-fk - "Given an entity ID or identity hash, and the model it represents (symbol, name or IModel), looks up the corresponding + "Given an identifier, and the model it represents (symbol, name or IModel), looks up the corresponding entity and gets its primary key. + The identifier can be a single entity ID string, a single identity-hash string, or a vector of entity ID and hash + strings. If the ID is compound, then the last ID is the one that corresponds to the model. This allows for the + compound IDs needed for nested entities like `DashboardCard`s to get their [[serdes.base/serdes-dependencies]]. + Throws if the corresponding entity cannot be found. Unusual parameter order means this can be used as `(update x :some_id import-fk 'SomeModel)`." @@ -40,6 +49,9 @@ (when eid (let [model-name (name model) model (db/resolve-model (symbol model-name)) + eid (if (vector? eid) + (last eid) + eid) entity (serdes.base/lookup-by-id model eid)] (if entity (get entity (models/primary-key model)) @@ -137,7 +149,7 @@ {:model "Table" :id table-name} {:model "Field" :id field-name}])) -;; ---------------------------------------------- JSON-encoded MBQL -------------------------------------------------- +;; ---------------------------------------------- MBQL Fields -------------------------------------------------------- (defn- mbql-entity-reference? "Is given form an MBQL entity reference?" [form] @@ -221,14 +233,11 @@ ;(ids->fully-qualified-names {:aggregation [[:sum [:field 277405 nil]]]}) -(defn export-json-mbql - "Given a JSON string with an MBQL expression inside it, convert it to an EDN structure and turn the non-portable - Database, Table and Field IDs inside it into portable references. Returns it as an EDN structure, which is more - human-fiendly in YAML." +(defn export-mbql + "Given an MBQL expression, convert it to an EDN structure and turn the non-portable Database, Table and Field IDs + inside it into portable references." [encoded] - (-> encoded - (json/parse-string true) - ids->fully-qualified-names)) + (ids->fully-qualified-names encoded)) (defn- portable-id? "True if the provided string is either an Entity ID or identity-hash string." @@ -259,7 +268,9 @@ {:database (fully-qualified-name :guard string?)} (-> &match - (assoc :database (db/select-one-id 'Database :name fully-qualified-name)) + (assoc :database (if (= fully-qualified-name "database/__virtual") + mbql.s/saved-questions-virtual-database-id + (db/select-one-id 'Database :name fully-qualified-name))) mbql-fully-qualified-names->ids*) ; Process other keys {:card-id (entity-id :guard portable-id?)} @@ -297,13 +308,10 @@ [entity] (mbql-fully-qualified-names->ids* entity)) -(defn import-json-mbql - "Given an MBQL expression as an EDN structure with portable IDs embedded, convert the IDs back to raw numeric IDs - and then convert the result back into a JSON string." +(defn import-mbql + "Given an MBQL expression as an EDN structure with portable IDs embedded, convert the IDs back to raw numeric IDs." [exported] - (-> exported - mbql-fully-qualified-names->ids - json/generate-string)) + (mbql-fully-qualified-names->ids exported)) (declare ^:private mbql-deps-map) @@ -328,7 +336,9 @@ (defn- mbql-deps-map [entity] (->> (for [[k v] entity] (cond - (and (= k :database) (string? v)) #{[{:model "Database" :id v}]} + (and (= k :database) + (string? v) + (not= v "database/__virtual")) #{[{:model "Database" :id v}]} (and (= k :source-table) (vector? v)) #{(table->path v)} (and (= k :source-table) (portable-id? v)) #{[{:model "Card" :id v}]} (and (= k :source-field) (vector? v)) #{(field->path v)} @@ -349,35 +359,41 @@ :else (mbql-deps-vector [entity]))) (defn export-parameter-mappings - "Given the :parameter_mappings field of a `Card` or `DashboardCard`, as a JSON-encoded list of objects, converts + "Given the :parameter_mappings field of a `Card` or `DashboardCard`, as a vector of maps, converts it to a portable form with the field IDs replaced with `[db schema table field]` references." [mappings] - (->> (json/parse-string mappings true) - (map ids->fully-qualified-names))) + (map ids->fully-qualified-names mappings)) (defn import-parameter-mappings "Given the :parameter_mappings field as exported by serialization convert its field references - (`[db schema table field]`) back into raw IDs, and encode it back into JSON." + (`[db schema table field]`) back into raw IDs." [mappings] (->> mappings (map mbql-fully-qualified-names->ids) - (map #(m/update-existing % :card_id import-fk 'Card)) - json/generate-string)) + (map #(m/update-existing % :card_id import-fk 'Card)))) (defn- export-visualizations [entity] (mbql.u/replace entity ["field-id" (id :guard number?)] ["field-id" (export-field-fk id)] + [:field-id (id :guard number?)] + [:field-id (export-field-fk id)] ["field-id" (id :guard number?) tail] ["field-id" (export-field-fk id) (export-visualizations tail)] + [:field-id (id :guard number?) tail] + [:field-id (export-field-fk id) (export-visualizations tail)] ["field" (id :guard number?)] ["field" (export-field-fk id)] + [:field (id :guard number?)] + [:field (export-field-fk id)] ["field" (id :guard number?) tail] ["field" (export-field-fk id) (export-visualizations tail)] + [:field (id :guard number?) tail] + [:field (export-field-fk id) (export-visualizations tail)] (_ :guard map?) (m/map-vals export-visualizations &match) @@ -393,14 +409,10 @@ (update-keys settings #(-> % json/parse-string export-visualizations json/generate-string)))) (defn export-visualization-settings - "Given a JSON string encoding the visualization settings for a `Card` or `DashboardCard`, transform it to EDN and - convert all field-ids to portable `[db schema table field]` form." + "Given the `:visualization_settings` map, convert all its field-ids to portable `[db schema table field]` form." [settings] (when settings (-> settings - (json/parse-string (fn [k] (if (re-matches #"^[a-zA-Z0-9_\.\-]+$" k) - (keyword k) - k))) export-visualizations (update :column_settings export-column-settings)))) @@ -429,13 +441,12 @@ (defn import-visualization-settings "Given an EDN value as exported by [[export-visualization-settings]], convert its portable `[db schema table field]` - references into Field IDs and serialize back to JSON." + references into Field IDs." [settings] (when settings (-> settings import-visualizations - (update :column_settings import-column-settings) - json/generate-string))) + (update :column_settings import-column-settings)))) (defn visualization-settings-deps "Given the :visualization_settings (possibly nil) for an entity, return any embedded serdes-deps as a set. diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj index c77a50b2d6d..c575718821b 100644 --- a/src/metabase/models/timeline_event.clj +++ b/src/metabase/models/timeline_event.clj @@ -35,6 +35,14 @@ ;;;; hydration +(defn timeline + "Attach the parent `:timeline` to this [[TimelineEvent]]." + {:hydrate :timeline} + [{:keys [timeline_id]}] + (db/select-one 'Timeline :id timeline_id)) + +;(hydrate (db/select-one 'TimelineEvent)) + (defn- fetch-events "Fetch events for timelines in `timeline-ids`. Can include optional `start` and `end` dates in the options map, as well as `all?`. By default, will return only unarchived events, unless `all?` is truthy and will return all events diff --git a/test/metabase/test/generate.clj b/test/metabase/test/generate.clj index 518893dcda0..8092e643c8f 100644 --- a/test/metabase/test/generate.clj +++ b/test/metabase/test/generate.clj @@ -301,7 +301,7 @@ (get-in [:table-fields (:table_id visit-val)]))) ;; Users' emails need to be unique. This enforces it, and appends junk to before the @ if needed. - (= ent-type :core_user) + (= ent-type :core-user) (update :email unique-email) ;; Database names need to be unique. This enforces it, and appends junk to names if needed. -- GitLab