Use Serialization spec for Collection (#47240)

src/metabase/models/collection.clj

@@ -1369,37 +1369,6 @@
                              not-trash-clause
                              (or where true)]}))))
 
-(defmethod serdes/extract-one "Collection"
-  ;; Transform :location (which uses database IDs) into a portable :parent_id with the parent's entity ID.
-  ;; Also transform :personal_owner_id from a database ID to the email string, if it's defined.
-  ;; Use the :slug as the human-readable label.
-  [_model-name _opts coll]
-  (let [fetch-collection    (fn [id]
-                              (t2/select-one Collection :id id))
-        {:keys [parent_id]} (some-> coll :id fetch-collection (t2/hydrate :parent_id))
-        parent              (some-> parent_id fetch-collection)
-        parent-id           (when parent
-                              (or (:entity_id parent) (serdes/identity-hash parent)))
-        owner-email         (when (:personal_owner_id coll)
-                              (t2/select-one-fn :email 'User :id (:personal_owner_id coll)))]
-    (-> (serdes/extract-one-basics "Collection" coll)
-        (dissoc :location)
-        (assoc :parent_id parent-id
-               :personal_owner_id owner-email)
-        (assoc-in [:serdes/meta 0 :label] (:slug coll)))))
-
-(defmethod serdes/load-xform "Collection" [{:keys [parent_id] :as contents}]
-  (let [loc (fn [col-id]
-              (if col-id
-                (let [{:keys [id location]} (serdes/lookup-by-id Collection col-id)]
-                  (str location id "/"))
-                "/"))]
-    (-> contents
-        (dissoc :parent_id)
-        (assoc :location (loc parent_id))
-        (update :personal_owner_id serdes/*import-user*)
-        serdes/load-xform-basics)))
-
 (defmethod serdes/dependencies "Collection"
   [{:keys [parent_id]}]
   (when parent_id
@@ -1431,6 +1400,35 @@
   (let [parental (get collections (:entity_id coll))]
     (concat ["collections"] parental [(last parental)])))
 
+(defn- parent-id->location-path [parent-id]
+  (if-not parent-id
+    "/"
+    ;; It would be great to use a cache rather than a database call to fetch the parent.
+    (let [{:keys [id location]} (t2/select-one Collection parent-id)]
+      (str location id "/"))))
+
+(defmethod serdes/make-spec "Collection" [_model-name _opts]
+  {:copy [:archive_operation_id
+          :archived
+          :archived_directly
+          :authority_level
+          :description
+          :entity_id
+          :is_sample
+          :name
+          :namespace
+          :slug
+          :type]
+   :skip []
+   :transform {:created_at        (serdes/date)
+               ;; We only dump the parent id, and recalculate the location from that on load.
+               :location          (serdes/as :parent_id
+                                             (serdes/compose
+                                              (serdes/fk :model/Collection)
+                                              {:export location-path->parent-id
+                                               :import parent-id->location-path}))
+               :personal_owner_id (serdes/fk :model/User)}})
+
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                           Perms Checking Helper Fns                                            |
 ;;; +----------------------------------------------------------------------------------------------------------------+

src/metabase/models/serialization.clj

@@ -331,7 +331,8 @@
             ;; won't assoc if `generate-path` returned `nil`
             (m/assoc-some :serdes/meta (generate-path model-name instance))
             (into (for [[k transform] (:transform spec)
-                        :let  [res ((:export transform) (get instance k))]
+                        :let  [export-k (:as transform k)
+                               res ((:export transform) (get instance k))]
                         :when (not= res ::skip)]
                     (do
                       (when-not (contains? instance k)
@@ -340,7 +341,7 @@
                                          :key      k
                                          :instance instance})))
 
-                      [k res]))))))
+                      [export-k res]))))))
     (catch Exception e
       (throw (ex-info (format "Error extracting %s %s" model-name (:id instance))
                       (assoc (ex-data e) :model model-name :id (:id instance))
@@ -744,10 +745,11 @@
         (-> (select-keys ingested (:copy spec))
             (into (for [[k transform] (:transform spec)
                         :when         (not (::nested transform))
-                        :let          [res ((:import transform) (get ingested k))]
+                        :let          [import-k (:as transform k)
+                                       res ((:import transform) (get ingested import-k))]
                         :when         (and (not= res ::skip)
                                            (or (some? res)
-                                               (contains? ingested k)))]
+                                               (contains? ingested import-k)))]
                     [k res])))))))
 
 (defn- spec-nested! [model-name ingested instance]
@@ -1639,6 +1641,17 @@
   (constantly
    {:export name :import keyword}))
 
+(defn as
+  "Serialize this field under the given key instead, typically because it has been logically transformed."
+  [k xform]
+  (assoc xform :as k))
+
+(defn compose
+  "Compose two transformations."
+  [inner-xform outer-xform]
+  {:export (comp (:export inner-xform) (:export outer-xform))
+   :import (comp (:import outer-xform) (:import inner-xform))})
+
 ;;; ## Memoizing appdb lookups
 
 (defmacro with-cache