From f2981efc5cc9c96c4b5ea6eec037086c6c7436e3 Mon Sep 17 00:00:00 2001
From: Braden Shepherdson <Braden.Shepherdson@gmail.com>
Date: Thu, 15 Sep 2022 11:17:41 -0400
Subject: [PATCH] Serdes v2: Handle missing User refs by synthesizing dummy
users (#25391)
Many entities have `creator_id` and similar fields. `User`s are not
serialized. Foreign keys to users are serialized as email addresses.
During deserialization in a different instance (eg. a local dev instance
importing a dump from a prod instance) many such users may not exist.
This change creates new `User` entities on the fly with empty names,
generated passwords, and the email set.
---
.../serialization/v2/load_test.clj | 58 +++++++++++++++++++
src/metabase/models/card.clj | 4 +-
src/metabase/models/collection.clj | 10 ++--
src/metabase/models/dashboard.clj | 4 +-
src/metabase/models/database.clj | 9 +--
src/metabase/models/metric.clj | 4 +-
src/metabase/models/native_query_snippet.clj | 4 +-
src/metabase/models/pulse.clj | 4 +-
src/metabase/models/segment.clj | 4 +-
src/metabase/models/serialization/util.clj | 28 +++++++--
src/metabase/models/timeline.clj | 4 +-
src/metabase/models/timeline_event.clj | 4 +-
src/metabase/models/user.clj | 6 ++
13 files changed, 114 insertions(+), 29 deletions(-)
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 bf635ac89b9..d2fbfedeaff 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
@@ -660,6 +660,64 @@
:target [:dimension [:field (:id @field1d) {:source-field (:id @field2d)}]]}]
(:parameter_mappings @dashcard1d))))))))))
+(deftest users-test
+ ;; Users are serialized as their email address. If a corresponding user is found during deserialization, its ID is
+ ;; used. However, if no such user exists, a new one is created with mostly blank fields.
+ (testing "existing users are found and used; missing users are created on the fly"
+ (let [serialized (atom nil)
+ metric1s (atom nil)
+ metric2s (atom nil)
+ user1s (atom nil)
+ user2s (atom nil)
+ user1d (atom nil)
+ metric1d (atom nil)
+ metric2d (atom nil)]
+
+ (ts/with-source-and-dest-dbs
+ (testing "serializing the original entities"
+ (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! serialized (into [] (serdes.extract/extract-metabase {})))))
+
+ (testing "exported form is properly converted"
+ (is (= "tom@bost.on"
+ (-> @serialized
+ (by-model "Metric")
+ first
+ :creator_id))))
+
+ (testing "deserializing finds the matching user and synthesizes the missing one"
+ (ts/with-dest-db
+ ;; Create another random user to change the user IDs.
+ (ts/create! User :first_name "Gideon" :last_name "Nav" :email "griddle@ninth.tomb")
+ ;; Likewise, create some other metrics.
+ (ts/create! Metric :name "Other metric A")
+ (ts/create! Metric :name "Other metric B")
+ (ts/create! Metric :name "Other metric C")
+ (reset! user1d (ts/create! User :first_name "Tom" :last_name "Scholz" :email "tom@bost.on"))
+
+ ;; Load the serialized content.
+ (serdes.load/load-metabase (ingestion-in-memory @serialized))
+
+ ;; Fetch the relevant bits
+ (reset! metric1d (db/select-one Metric :name "Large Users"))
+ (reset! metric2d (db/select-one Metric :name "Support Headaches"))
+
+ (testing "the Metrics and Users have different IDs now"
+ (is (not= (:id @metric1s) (:id @metric1d)))
+ (is (not= (:id @metric2s) (:id @metric2d)))
+ (is (not= (:id @user1s) (:id @user1d))))
+
+ (testing "both existing User and the new one are set up properly"
+ (is (= (:id @user1d) (:creator_id @metric1d)))
+ (let [user2d-id (:creator_id @metric2d)
+ user2d (db/select-one User :id user2d-id)]
+ (is (any? user2d))
+ (is (= (:email @user2s) (:email user2d)))))))))))
+
(deftest field-values-test
;; FieldValues are a bit special - they map 1-1 with Fields but are a separate table serialized separately.
;; The main special thing to test here is that the custom load-find-local correctly finds an existing FieldValues.
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 689082fe826..c9c316054f2 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -355,7 +355,7 @@
(update :database_id serdes.util/export-fk-keyed 'Database :name)
(update :table_id serdes.util/export-table-fk)
(update :collection_id serdes.util/export-fk 'Collection)
- (update :creator_id serdes.util/export-fk-keyed 'User :email)
+ (update :creator_id serdes.util/export-user)
(update :dataset_query serdes.util/export-json-mbql)
(update :parameter_mappings serdes.util/export-parameter-mappings)
(update :visualization_settings serdes.util/export-visualization-settings)
@@ -367,7 +367,7 @@
serdes.base/load-xform-basics
(update :database_id serdes.util/import-fk-keyed 'Database :name)
(update :table_id serdes.util/import-table-fk)
- (update :creator_id serdes.util/import-fk-keyed 'User :email)
+ (update :creator_id serdes.util/import-user)
(update :collection_id serdes.util/import-fk 'Collection)
(update :dataset_query serdes.util/import-json-mbql)
(update :parameter_mappings serdes.util/import-parameter-mappings)
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index 52cc250ad6a..81e40e0ba68 100644
--- a/src/metabase/models/collection.clj
+++ b/src/metabase/models/collection.clj
@@ -16,6 +16,7 @@
[metabase.models.permissions :as perms :refer [Permissions]]
[metabase.models.serialization.base :as serdes.base]
[metabase.models.serialization.hash :as serdes.hash]
+ [metabase.models.serialization.util :as serdes.util]
[metabase.public-settings.premium-features :as premium-features]
[metabase.util :as u]
[metabase.util.honeysql-extensions :as hx]
@@ -942,17 +943,16 @@
(assoc :parent_id parent-id :personal_owner_id owner-email)
(assoc-in [:serdes/meta 0 :label] (:slug coll)))))
-(defmethod serdes.base/load-xform "Collection" [{:keys [parent_id personal_owner_id] :as contents}]
+(defmethod serdes.base/load-xform "Collection" [{:keys [parent_id] :as contents}]
(let [loc (if parent_id
(let [{:keys [id location]} (serdes.base/lookup-by-id Collection parent_id)]
(str location id "/"))
- "/")
- user-id (when personal_owner_id
- (db/select-one-field :id 'User :email personal_owner_id))]
+ "/")]
(-> contents
serdes.base/load-xform-basics
(dissoc :parent_id)
- (assoc :location loc :personal_owner_id user-id))))
+ (assoc :location loc)
+ (update :personal_owner_id serdes.util/import-user))))
(defmethod serdes.base/serdes-dependencies "Collection"
[{:keys [parent_id]}]
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 6aed0b6a852..304741ed5b2 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -434,14 +434,14 @@
[_model-name _opts dash]
(-> (serdes.base/extract-one-basics "Dashboard" dash)
(update :collection_id serdes.util/export-fk 'Collection)
- (update :creator_id serdes.util/export-fk-keyed 'User :email)))
+ (update :creator_id serdes.util/export-user)))
(defmethod serdes.base/load-xform "Dashboard"
[dash]
(-> dash
serdes.base/load-xform-basics
(update :collection_id serdes.util/import-fk 'Collection)
- (update :creator_id serdes.util/import-fk-keyed 'User :email)))
+ (update :creator_id serdes.util/import-user)))
(defmethod serdes.base/serdes-dependencies "Dashboard"
[{:keys [collection_id]}]
diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj
index 50ded97170b..cc107c26ff5 100644
--- a/src/metabase/models/database.clj
+++ b/src/metabase/models/database.clj
@@ -12,6 +12,7 @@
[metabase.models.secret :as secret :refer [Secret]]
[metabase.models.serialization.base :as serdes.base]
[metabase.models.serialization.hash :as serdes.hash]
+ [metabase.models.serialization.util :as serdes.util]
[metabase.plugins.classloader :as classloader]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
@@ -287,7 +288,7 @@
;; TODO Support alternative encryption of secret database details.
;; There's one optional foreign key: creator_id. Resolve it as an email.
(cond-> (serdes.base/extract-one-basics "Database" entity)
- (:creator_id entity) (assoc :creator_id (db/select-one-field :email 'User :id (:creator_id entity)))
+ true (update :creator_id serdes.util/export-user)
(= :exclude secrets) (dissoc :details)))
(defmethod serdes.base/serdes-entity-id "Database"
@@ -302,6 +303,6 @@
[[{:keys [id]}]]
(db/select-one-field :id Database :name id))
-(defmethod serdes.base/load-xform "Database" [{:keys [creator_id] :as entity}]
- (cond-> (serdes.base/load-xform-basics entity)
- creator_id (assoc :creator_id (db/select-one-field :id 'User :email creator_id))))
+(defmethod serdes.base/load-xform "Database" [entity]
+ (-> (serdes.base/load-xform-basics entity)
+ (update :creator_id serdes.util/import-user)))
diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj
index e36b8307b40..82f8585594f 100644
--- a/src/metabase/models/metric.clj
+++ b/src/metabase/models/metric.clj
@@ -88,14 +88,14 @@
[_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-fk-keyed 'User :email)
+ (update :creator_id serdes.util/export-user)
(update :definition serdes.util/export-json-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-fk-keyed 'User :email)
+ (update :creator_id serdes.util/import-user)
(update :definition serdes.util/import-json-mbql)))
(defmethod serdes.base/serdes-dependencies "Metric" [{:keys [definition table_id]}]
diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj
index 88cb839a70d..55b9db23908 100644
--- a/src/metabase/models/native_query_snippet.clj
+++ b/src/metabase/models/native_query_snippet.clj
@@ -94,13 +94,13 @@
(defmethod serdes.base/extract-one "NativeQuerySnippet"
[_model-name _opts snippet]
(-> (serdes.base/extract-one-basics "NativeQuerySnippet" snippet)
- (update :creator_id serdes.util/export-fk-keyed 'User :email)
+ (update :creator_id serdes.util/export-user)
(update :collection_id #(when % (serdes.util/export-fk % 'Collection)))))
(defmethod serdes.base/load-xform "NativeQuerySnippet" [snippet]
(-> snippet
serdes.base/load-xform-basics
- (update :creator_id serdes.util/import-fk-keyed 'User :email)
+ (update :creator_id serdes.util/import-user)
(update :collection_id #(when % (serdes.util/import-fk % 'Collection)))))
(defmethod serdes.base/serdes-dependencies "NativeQuerySnippet"
diff --git a/src/metabase/models/pulse.clj b/src/metabase/models/pulse.clj
index 35c1ab93c4a..5d2428fdb53 100644
--- a/src/metabase/models/pulse.clj
+++ b/src/metabase/models/pulse.clj
@@ -567,11 +567,11 @@
(cond-> (serdes.base/extract-one-basics "Pulse" pulse)
(:collection_id pulse) (update :collection_id serdes.util/export-fk 'Collection)
(:dashboard_id pulse) (update :dashboard_id serdes.util/export-fk 'Dashboard)
- true (update :creator_id serdes.util/export-fk-keyed 'User :email)))
+ true (update :creator_id serdes.util/export-user)))
(defmethod serdes.base/load-xform "Pulse" [pulse]
(cond-> (serdes.base/load-xform-basics pulse)
- true (update :creator_id serdes.util/import-fk-keyed 'User :email)
+ true (update :creator_id serdes.util/import-user)
(:collection_id pulse) (update :collection_id serdes.util/import-fk 'Collection)
(:dashboard_id pulse) (update :dashboard_id serdes.util/import-fk 'Dashboard)))
diff --git a/src/metabase/models/segment.clj b/src/metabase/models/segment.clj
index 85c1fc28c59..894fe929845 100644
--- a/src/metabase/models/segment.clj
+++ b/src/metabase/models/segment.clj
@@ -89,14 +89,14 @@
[_model-name _opts segment]
(-> (serdes.base/extract-one-basics "Segment" segment)
(update :table_id serdes.util/export-table-fk)
- (update :creator_id serdes.util/export-fk-keyed 'User :email)
+ (update :creator_id serdes.util/export-user)
(update :definition serdes.util/export-json-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-fk-keyed 'User :email)
+ (update :creator_id serdes.util/import-user)
(update :definition serdes.util/import-json-mbql)))
(defmethod serdes.base/serdes-dependencies "Segment" [{:keys [definition table_id]}]
diff --git a/src/metabase/models/serialization/util.clj b/src/metabase/models/serialization/util.clj
index 6e4cd0affb4..2f9c31e709f 100644
--- a/src/metabase/models/serialization/util.clj
+++ b/src/metabase/models/serialization/util.clj
@@ -47,9 +47,9 @@
(defn export-fk-keyed
"Given a numeric ID, look up a different identifying field for that entity, and return it as a portable ID.
- Eg. `User.email`, `Database.name`.
+ Eg. `Database.name`.
[[import-fk-keyed]] is the inverse.
- Unusual parameter order lets this be called as, for example, `(update x :creator_id export-fk-keyed 'User :email).
+ Unusual parameter order lets this be called as, for example, `(update x :creator_id export-fk-keyed 'Database :name)`.
Note: This assumes the primary key is called `:id`."
[id model field]
@@ -58,12 +58,32 @@
(defn import-fk-keyed
"Given a single, portable, identifying field and the model it refers to, this resolves the entity and returns its
numeric `:id`.
- Eg. `User.email` or `Database.name`.
+ Eg. `Database.name`.
- Unusual parameter order lets this be called as, for example, `(update x :creator_id import-fk-keyed 'User :email)`."
+ Unusual parameter order lets this be called as, for example,
+ `(update x :creator_id import-fk-keyed 'Database :name)`."
[portable model field]
(db/select-one-id model field portable))
+;; -------------------------------------------------- Users ----------------------------------------------------------
+(defn export-user
+ "Exports a user as the email address.
+ This just calls [[export-fk-keyed]], but the counterpart [[import-user]] is more involved. This is a unique function
+ so they form a pair."
+ [id]
+ (when id (export-fk-keyed id 'User :email)))
+
+(defn import-user
+ "Imports a user by their email address.
+ If a user with that email address exists, returns its primary key.
+ If no such user exists, creates a dummy one with the default settings, blank name, and randomized password.
+ Does not send any invite emails."
+ [email]
+ (when email
+ (or (import-fk-keyed email 'User :email)
+ ;; Need to break a circular dependency here.
+ (:id ((resolve 'metabase.models.user/serdes-synthesize-user!) {:email email})))))
+
;; -------------------------------------------------- Tables ---------------------------------------------------------
(defn export-table-fk
"Given a numeric `table_id`, return a portable table reference.
diff --git a/src/metabase/models/timeline.clj b/src/metabase/models/timeline.clj
index c79b5a6e567..280f2d7c811 100644
--- a/src/metabase/models/timeline.clj
+++ b/src/metabase/models/timeline.clj
@@ -66,13 +66,13 @@
[_model-name _opts timeline]
(-> (serdes.base/extract-one-basics "Timeline" timeline)
(update :collection_id serdes.util/export-fk 'Collection)
- (update :creator_id serdes.util/export-fk-keyed 'User :email)))
+ (update :creator_id serdes.util/export-user)))
(defmethod serdes.base/load-xform "Timeline" [timeline]
(-> timeline
serdes.base/load-xform-basics
(update :collection_id serdes.util/import-fk 'Collection)
- (update :creator_id serdes.util/import-fk-keyed 'User :email)))
+ (update :creator_id serdes.util/import-user)))
(defmethod serdes.base/serdes-dependencies "Timeline" [{:keys [collection_id]}]
[[{:model "Collection" :id collection_id}]])
diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj
index fa0aa69597d..7abf5d7aee8 100644
--- a/src/metabase/models/timeline_event.clj
+++ b/src/metabase/models/timeline_event.clj
@@ -110,14 +110,14 @@
[_model-name _opts event]
(-> (serdes.base/extract-one-basics "TimelineEvent" event)
(update :timeline_id serdes.util/export-fk 'Timeline)
- (update :creator_id serdes.util/export-fk-keyed 'User :email)
+ (update :creator_id serdes.util/export-user)
(update :timestamp #(u.date/format (t/offset-date-time %)))))
(defmethod serdes.base/load-xform "TimelineEvent" [event]
(-> event
serdes.base/load-xform-basics
(update :timeline_id serdes.util/import-fk 'Timeline)
- (update :creator_id serdes.util/import-fk-keyed 'User :email)
+ (update :creator_id serdes.util/import-user)
(update :timestamp u.date/parse)))
(defmethod serdes.base/serdes-dependencies "TimelineEvent" [{:keys [timeline_id]}]
diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index f2f82f73a15..0e03c59e35b 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -280,6 +280,12 @@
[new-user :- NewUser]
(db/insert! User (update new-user :password #(or % (str (UUID/randomUUID))))))
+(defn serdes-synthesize-user!
+ "Creates a new user with a default password, when deserializing eg. a `:creator_id` field whose email address doesn't
+ match any existing user."
+ [new-user]
+ (insert-new-user! new-user))
+
(s/defn create-and-invite-user!
"Convenience function for inviting a new `User` and sending out the welcome email."
[new-user :- NewUser, invitor :- Invitor, setup? :- s/Bool]
--
GitLab