Kimirock/metabase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move Dimensions to FieldUserSettings in serdes (#83652)

Browse Source
This commit is contained in:
Alexander Polyankin
2026-10-05 22:26:31 -04:00
committed by GitHub
parent 145357fbb1
commit 9050b176d6
65 changed files with 729 additions and 385 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.clj-kondo/config/modules/config.edn
View File

@@ -4202,6 +4202,7 @@
:model/Collection
:model/Dashboard
:model/DataApp
:model/Dimension
:model/Document
:model/Field
:model/FieldUserSettings

82
enterprise/backend/src/metabase_enterprise/remote_sync/db.clj
View File

@@ -141,10 +141,12 @@
:limit limit}))
(mu/defn instance
"The instance of `model` with `id`, or nil."
"The instance of `model` with `id`, or nil; a Table is read through the overlay."
[model :- :keyword
id :- ms/PositiveInt]
(t2/select-one model :id id))
(t2/select-one model :id id (if (= model :model/Table)
{:from [(warehouse-schema-overlay/table-query)]}
{})))
(mu/defn instance-with-columns
"The `columns` of the instance of `model` with `id`, or nil; a Table is read through the overlay."
@@ -292,15 +294,75 @@
[card-ids :- [:sequential ::lib.schema.id/card]]
(t2/select [:model/Card :id :type :display] :id [:in card-ids]))
(mu/defn user-settings-exist-for-table?
"Whether the Table with `table-id`, or any of its Fields, has a user-settings row."
(mu/defn table-user-settings-exist? :- :boolean
"Whether the Table with `table-id` has a TableUserSettings row."
[table-id :- ::lib.schema.id/table]
(or (t2/exists? :model/TableUserSettings :table_id table-id)
(t2/exists? :model/FieldUserSettings
{:from [[(t2/table-name :model/FieldUserSettings) :u]]
:join [(warehouse-schema-overlay/field-query {:alias :f :user-settings? false})
[:= :f.id :u.field_id]]
:where [:= :f.table_id table-id]})))
(t2/exists? :model/TableUserSettings :table_id table-id))
(mu/defn field-user-settings-exist? :- :boolean
"Whether the Field with `field-id` has a FieldUserSettings row."
[field-id :- ::lib.schema.id/field]
(t2/exists? :model/FieldUserSettings :field_id field-id))
(mu/defn dimension-exists-for-field? :- :boolean
"Whether the Field with `field-id` has a Dimension."
[field-id :- ::lib.schema.id/field]
(t2/exists? :model/Dimension :field_id field-id))
(mu/defn published-table-ids :- [:set ::lib.schema.id/table]
"The ids of the Tables published in the Collections with `collection-ids`."
[collection-ids :- [:sequential ::lib.schema.id/collection]]
(set (t2/select-pks-set :model/Table {:from [(warehouse-schema-overlay/table-query {:alias :t})]
:where [:and [:= :t.is_published true] [:in :t.collection_id collection-ids]]})))
(mu/defn table-ids-with-user-settings
"The ids of the Tables among `table-ids` that have a TableUserSettings row."
[table-ids :- [:sequential ::lib.schema.id/table]]
(t2/select-fn-vec :table_id :model/TableUserSettings {:select [:table_id] :where [:in :table_id table-ids]}))
(mu/defn field-ids-with-user-settings
"The ids of the Fields of the Tables with `table-ids` that have a FieldUserSettings row."
[table-ids :- [:sequential ::lib.schema.id/table]]
(t2/select-fn-vec :field_id :model/FieldUserSettings
{:select [:u.field_id]
:from [[(t2/table-name :model/FieldUserSettings) :u]]
:join [(warehouse-schema-overlay/field-query {:alias :f :user-settings? false}) [:= :f.id :u.field_id]]
:where [:in :f.table_id table-ids]}))
(mu/defn field-ids-with-dimensions
"The ids of the Fields of the Tables with `table-ids` that have a Dimension."
[table-ids :- [:sequential ::lib.schema.id/table]]
(t2/select-fn-vec :field_id :model/Dimension
{:select [:d.field_id]
:from [[(t2/table-name :model/Dimension) :d]]
:join [(warehouse-schema-overlay/field-query {:alias :f :user-settings? false}) [:= :f.id :d.field_id]]
:where [:in :f.table_id table-ids]}))
(mu/defn delete-table-user-settings!
"Delete the TableUserSettings of the Tables with `table-ids`, returning the number deleted."
[table-ids :- [:sequential ::lib.schema.id/table]]
(t2/delete! :model/TableUserSettings :table_id [:in table-ids]))
(mu/defn delete-field-user-settings!
"Delete the FieldUserSettings of the Fields with `field-ids`, returning the number deleted."
[field-ids :- [:sequential ::lib.schema.id/field]]
(t2/delete! :model/FieldUserSettings :field_id [:in field-ids]))
(mu/defn delete-dimensions!
"Delete the Dimensions of the Fields with `field-ids`, returning the number deleted."
[field-ids :- [:sequential ::lib.schema.id/field]]
(t2/delete! :model/Dimension :field_id [:in field-ids]))
(mu/defn tables-tracking-details
"The `:id`, `:name`, and `:collection_id` of the Tables with `table-ids`, read through the overlay."
[table-ids :- [:sequential ::lib.schema.id/table]]
(t2/select [:model/Table :id :name :collection_id] :id [:in table-ids] {:from [(warehouse-schema-overlay/table-query)]}))
(mu/defn fields-tracking-details
"The `:id`, name, table id, collection id, and table name of the Fields with `field-ids`."
[field-ids :- [:sequential ::lib.schema.id/field]]
(let [{:keys [select from join]} (tracking-select-parts :model/Field)]
(t2/query {:select (into [:f.id] select) :from from :join join :where [:in :f.id field-ids]})))
(mu/defn snippets
"The `:id`, `:name`, and `:collection_id` of every NativeQuerySnippet."

39
enterprise/backend/src/metabase_enterprise/remote_sync/events.clj
View File

@@ -11,7 +11,8 @@
Tracked model types:
- Card, Dashboard, Document, NativeQuerySnippet, Timeline, Collection
- Table and TableUserSettings, which also carries a Field's own edits (when published in a remote-synced collection)
- Table and TableUserSettings (when published in a remote-synced collection)
- FieldUserSettings and Dimension, keyed by their Field (when its Table is published in a remote-synced collection)
- Segment, Measure (when belonging to a published table in a remote-synced collection)
- Transform, TransformTag, transforms-namespace Collections (when remote-sync-transforms setting is enabled)
- NativeQuerySnippet, snippets-namespace Collections, Glossary (when Library is remote-synced)"
@@ -378,7 +379,7 @@
(log/infof "Collection %s no longer needs syncing, marking as removed" (:id object))
(create-or-update-remote-sync-object-entry! "Collection" (:id object) "removed" hydrate-collection-details)))))
;;; ----------------------------------------- TableUserSettings Tracking -----------------------------------------------
;;; ------------------------------------------- User Settings Tracking -------------------------------------------------
(def ^:private table-spec (get spec/remote-sync-specs :model/Table))
(def ^:private field-spec (get spec/remote-sync-specs :model/Field))
@@ -389,33 +390,39 @@
(let [details (spec/hydrate-model-details table-spec table-id)]
(assoc details :table_id (:id details) :table_name (:name details))))
(defn- sync-table-user-settings!
"Track the Table's TableUserSettings when eligible and any of its user settings exist, else mark it removed."
[table-id eligible?]
(cond
(and eligible? (remote-sync.db/user-settings-exist-for-table? table-id))
(create-or-update-remote-sync-object-entry!
"TableUserSettings" table-id "update" hydrate-table-user-settings-details)
(defn- hydrate-field-details
"The RemoteSyncObject details of the Field with `field-id`."
[field-id]
(spec/hydrate-model-details field-spec field-id))
(and (not eligible?)
(remote-sync.db/rso-exists? "TableUserSettings" table-id))
(create-or-update-remote-sync-object-entry!
"TableUserSettings" table-id "removed" hydrate-table-user-settings-details)))
(defn- sync-user-settings!
"Track `model-type` `id` as updated when `eligible?` and `exists?` holds for it, else mark it removed when tracked."
[model-type id eligible? exists? hydrate-details]
(cond
(and eligible? (exists? id))
(create-or-update-remote-sync-object-entry! model-type id "update" hydrate-details)
(remote-sync.db/rso-exists? model-type id)
(create-or-update-remote-sync-object-entry! model-type id "removed" hydrate-details)))
(events/derive! :event/field-update ::field-update-event)
(events/derive! ::field-update-event :metabase/event)
(methodical/defmethod events/publish-event! ::field-update-event
[_topic {:keys [object]}]
(when-let [table-id (:table_id object)]
(sync-table-user-settings! table-id (spec/check-eligibility field-spec object))))
(let [eligible? (spec/check-eligibility field-spec object)]
(sync-user-settings! "FieldUserSettings" (:id object) eligible?
remote-sync.db/field-user-settings-exist? hydrate-field-details)
(sync-user-settings! "Dimension" (:id object) eligible?
remote-sync.db/dimension-exists-for-field? hydrate-field-details)))
(defn- handle-table-event!
"The generic Table handler plus the Table's TableUserSettings; one handler, since a second primary method on the
same events would displace the generic one."
[topic {:keys [object] :as event}]
(handle-model-event-from-spec table-spec topic event)
(sync-table-user-settings! (:id object) (spec/check-eligibility table-spec object)))
(sync-user-settings! "TableUserSettings" (:id object) (spec/check-eligibility table-spec object)
remote-sync.db/table-user-settings-exist? hydrate-table-user-settings-details))
(let [event-kws (spec/event-keywords table-spec)
parent-kw (:parent event-kws)]

97
enterprise/backend/src/metabase_enterprise/remote_sync/impl.clj
View File

@@ -247,7 +247,7 @@
Returns:
- [{:model_type :model_id :path :content_hash}] (entities that fail to serialize are omitted)"
[rows repo-paths]
(let [storage-opts (source/storage-context)
(let [storage-opts (serdes/storage-base-context)
repo-by-eid (into {} (map (fn [{:keys [model_type entity_id path]}] [[model_type entity_id] path])) repo-paths)
serialize (fn [model-type opts id->eid instance]
(try
@@ -297,6 +297,70 @@
(doseq [chunk (partition-all app-db-batch-size rows)]
(remote-sync.db/insert-rsos! (merge-content-metadata chunk (import-content-metadata chunk repo-paths))))))
(defn- in-batches
"The concatenated results of `f` called on `ids` in batches of [[app-db-batch-size]]."
[f ids]
(into [] (mapcat #(f (vec %))) (partition-all app-db-batch-size ids)))
(defn- remove-unsynced-user-settings!
"Deletes the TableUserSettings, FieldUserSettings, and Dimensions of the Tables published in
`synced-collection-ids` that `seen-paths` did not import, returning those Tables' ids."
[synced-collection-ids ingestable seen-paths]
(when-let [table-ids (some-> (not-empty synced-collection-ids) remote-sync.db/published-table-ids not-empty vec)]
(let [paths (group-by (comp :model last) seen-paths)
parent-id #(:id (serdes/load-find-local (pop %)))
imported #(into #{} (keep parent-id) (get paths %))
legacy-tables (into #{}
(comp (filter #(contains? (serialization/ingest-one ingestable %) :fields))
(keep parent-id))
(get paths "TableUserSettings"))
kept-tables (imported "TableUserSettings")
kept-fields (imported "FieldUserSettings")
kept-dims (into (imported "Dimension") (keep #(:id (serdes/load-find-local %))) (get paths "Field"))]
(doseq [batch (->> (in-batches remote-sync.db/table-ids-with-user-settings table-ids)
(remove kept-tables)
(partition-all app-db-batch-size))]
(remote-sync.db/delete-table-user-settings! (vec batch)))
(doseq [batch (->> (in-batches remote-sync.db/field-ids-with-user-settings (remove legacy-tables table-ids))
(remove kept-fields)
(partition-all app-db-batch-size))]
(remote-sync.db/delete-field-user-settings! (vec batch)))
(doseq [batch (->> (in-batches remote-sync.db/field-ids-with-dimensions table-ids)
(remove kept-dims)
(partition-all app-db-batch-size))]
(remote-sync.db/delete-dimensions! (vec batch)))
table-ids)))
(defn- track-user-settings!
"Inserts a synced RemoteSyncObject for each TableUserSettings, FieldUserSettings, and Dimension of the Tables with
`table-ids` as of `timestamp`."
[table-ids timestamp]
(let [table-rows (for [{:keys [id name collection_id]}
(in-batches remote-sync.db/tables-tracking-details
(in-batches remote-sync.db/table-ids-with-user-settings table-ids))]
{:model_type "TableUserSettings"
:model_id id
:model_name name
:model_collection_id collection_id
:model_table_id id
:model_table_name name
:status "synced"
:status_changed_at timestamp})
field-rows (for [[model-type field-ids] {"FieldUserSettings" (in-batches remote-sync.db/field-ids-with-user-settings table-ids)
"Dimension" (in-batches remote-sync.db/field-ids-with-dimensions table-ids)}
{:keys [id name table_id collection_id table_name]}
(in-batches remote-sync.db/fields-tracking-details field-ids)]
{:model_type model-type
:model_id id
:model_name name
:model_collection_id collection_id
:model_table_id table_id
:model_table_name table_name
:status "synced"
:status_changed_at timestamp})]
(doseq [batch (partition-all app-db-batch-size (concat table-rows field-rows))]
(remote-sync.db/insert-rsos! (vec batch)))))
(defn- branch-changed-since-scheduling?
"Returns true if `pre-task-branch` was captured by the async-* function and the
`remote-sync-branch` setting has since drifted to a different value. Used as a
@@ -352,14 +416,17 @@
;; commit, blocking the heartbeat for the whole reconcile and hashing phase.
(report 0.75 {:force? true})
(t2/with-transaction [_conn]
(remove-unsynced! (spec/all-syncable-collection-ids) imported-data)
(let [synced-collection-ids (spec/all-syncable-collection-ids)
_ (remove-unsynced! synced-collection-ids imported-data)
settings-table-ids (remove-unsynced-user-settings! synced-collection-ids base-ingestable seen-paths)]
;; Replace the RemoteSyncObject table, folding each entity's repo file_path (so later renames/deletes
;; resolve the real file) and serialized-content hash (so a post-pull no-op edit stays synced) into the
;; insert. Chunked so insert/IN params and memory stay bounded.
(remote-sync.db/delete-all-rsos!)
(insert-with-metadata! (spec/sync-all-entities! sync-timestamp imported-data)
(source.ingestable/cached-file-paths base-ingestable))
(when finalize! (finalize!)))
(track-user-settings! settings-table-ids sync-timestamp)
(when finalize! (finalize!))))
(report 0.9 {:force? true})
(when (and (not has-transforms?)
(settings/remote-sync-transforms))
@@ -1007,7 +1074,7 @@
- {:writes [{:id :model_type :model_id :file_path}] :delete-paths [path] :removed-ids [id]}, or
- :remote-sync/incremental-not-possible when any row can't go incrementally"
[snapshot rows]
(let [opts (source/storage-context)
(let [opts (serdes/storage-base-context)
;; create/update rows on entity-id models need an entity (extracted per chunk); everything else
;; (removed/delete, non-entity-id, bad status) is decided with no entity
{cu-rows true other-rows false} (group-by #(boolean (and (#{"create" "update"} (:status %))
@@ -1079,21 +1146,27 @@
(source.p/stage-delete! commit delete-path)))
(defn- exportable-write-rows
"WriteRows for a full export — every exportable id tagged with its RemoteSyncObject id (untracked deps get :id nil)."
"WriteRows for a full export — every exportable id tagged with its RemoteSyncObject id (untracked deps and
Dimensions, whose RemoteSyncObject is keyed by their Field, get :id nil)."
[]
(let [rso-id (u/index-by (juxt :model_type :model_id) :id (remote-sync.db/rso-keys))]
(for [[model ids] (spec/exportable-entities)
id ids]
{:model_type model :model_id id :id (rso-id [model id])})))
{:model_type model :model_id id :id (when-not (= "Dimension" model) (rso-id [model id]))})))
(def ^:private user-settings-model-types
"The model types tracked by the user-settings event handlers rather than a remote-sync spec."
#{"TableUserSettings" "FieldUserSettings" "Dimension"})
(defn- find-departed-entities
"Find RSO rows for entity-id entities that left the synced set (removed/delete status and not in
`targets`), matching the incremental path; path/hybrid and still-exported rows are kept."
"Find RSO rows pending removal or deletion of user settings, and of entity-id entities that are not in
`exported-rows`; path/hybrid and still-exported rows are kept."
[exported-rows]
(let [exported (into #{} (map #(select-keys % [:model_type :model_id])) exported-rows)]
(->> (remote-sync.db/departed-rso-keys)
(filter #(= :entity-id (:identity (spec/spec-for-model-type (:model_type %)))))
(remove #(exported (select-keys % [:model_type :model_id])))
(filter #(or (user-settings-model-types (:model_type %))
(and (= :entity-id (:identity (spec/spec-for-model-type (:model_type %))))
(not (exported (select-keys % [:model_type :model_id]))))))
(map :id))))
(defn- mark-rows-synced!
@@ -1119,7 +1192,7 @@
total (count export-rows)
span (- export-progress-serialize export-progress-plan-done)]
(report export-progress-plan-done {:force? true})
(let [opts (source/storage-context)
(let [opts (serdes/storage-base-context)
[synced version] (commit-staged! snapshot message
(fn [commit]
(source/replace-managed-files! commit snapshot) ; replace the managed files wholesale
@@ -1151,7 +1224,7 @@
total (max 1 (count writes))
span (- export-progress-serialize export-progress-plan-done)]
(report export-progress-plan-done {:force? true})
(let [opts (source/storage-context)
(let [opts (serdes/storage-base-context)
[synced version] (commit-staged! snapshot message
(fn [commit]
(let [synced (stage-writes commit opts writes

11
enterprise/backend/src/metabase_enterprise/remote_sync/source.clj
View File

@@ -68,11 +68,6 @@
(atom nil) (atom []))
(seq root-dependencies) (ingestable/wrap-root-dep-ingestable root-dependencies)))
(defn storage-context
"The serdes storage context for git sync: a Table's user settings file stands in for the Table's own."
[]
(assoc (serdes/storage-base-context) :inline-user-settings true))
(defn entity->path
"The repo-relative path an extracted `entity` serializes to, using storage context `opts`."
[opts entity]
@@ -103,7 +98,7 @@
:resources (vec (resource-specs path entity))})
(defn entity->file-spec
"[[entity->file-spec-at]] the path storage context `opts` (from [[storage-context]]) gives `entity`."
"[[entity->file-spec-at]] the path storage context `opts` (from [[serdes/storage-base-context]]) gives `entity`."
[opts entity]
(entity->file-spec-at (entity->path opts entity) entity))
@@ -127,7 +122,7 @@
({:model_type :model_id}), or nil if it can't be extracted."
[row]
(when-let [entity (first (spec/extract-entities-for-rows [row]))]
(let [fspec (entity->file-spec (storage-context) entity)]
(let [fspec (entity->file-spec (serdes/storage-base-context) entity)]
{:path (:path fspec), :content-hash (file-spec-hash fspec)})))
(defn row->content-hash
@@ -147,7 +142,7 @@
Throws Exception if any entity in the stream is an Exception instance."
[stream task-id & {:keys [total]}]
(let [opts (storage-context)
(let [opts (serdes/storage-base-context)
stream (if (or (nil? task-id) total (counted? stream)) stream (vec stream))
total (or total (when task-id (count stream)))
report (if (and task-id (pos? total))

3
enterprise/backend/src/metabase_enterprise/remote_sync/spec.clj
View File

@@ -1262,8 +1262,7 @@
{:include-field-values false
:include-database-secrets false
:continue-on-error false
:skip-archived true
:inline-user-settings true})
:skip-archived true})
(def ^:private models-traversed-but-not-stored
"Models git sync walks through but never writes."

6
enterprise/backend/src/metabase_enterprise/serialization/v2/models.clj
View File

@@ -4,7 +4,9 @@
(def data-model
"Schema model types"
["Database"
"Dimension"
"Field"
"FieldUserSettings"
"Measure"
"Segment"
"Table"
@@ -17,6 +19,8 @@
["Table"
"TableUserSettings"
"Field"
"FieldUserSettings"
"Dimension"
"Segment"])
(def content
@@ -53,8 +57,6 @@
For example, the models should also have their entity_id fields populated (if they have one)."
["DashboardCard"
"DashboardTab"
"Dimension"
"FieldUserSettings"
"ParameterCard"
"DashboardCardSeries"
"MetabotPrompt"

9
enterprise/backend/src/metabase_enterprise/serialization/v2/storage/util.clj
View File

@@ -25,17 +25,18 @@
(u.str/limit-chars max-label-length))))
(defn- resolve-path
"Given a storage path (vector of `{:label ... :key ... :style ...}` maps), resolves to a vector of strings
with deduplication per folder."
"Given a storage path (vector of `{:label ... :key ... :style ... :suffix ...}` maps), resolves to a vector of
strings with deduplication per folder; a `:suffix` is appended after the label is slugified and truncated."
[unique-name-fns path]
(loop [remaining path
resolved []]
(if (empty? remaining)
resolved
(let [{:keys [label key style] :or {style :name}} (first remaining)
slug (case style
(let [{:keys [label key style suffix] :or {style :name}} (first remaining)
slug (str (case style
:name (slugify-name label)
:slug label)
suffix)
gen (or (get @unique-name-fns resolved)
(let [g (lib/non-truncating-unique-name-generator)]
(swap! unique-name-fns assoc resolved g)

56
enterprise/backend/test/metabase_enterprise/remote_sync/events_test.clj
View File

@@ -1351,9 +1351,9 @@
;;; ----------------------------------------- TableUserSettings RSO Tracking from Field Events --------------------------
(deftest field-update-creates-table-user-settings-rso-when-fus-exists-test
(testing "field-update on an eligible field with a FUS row creates a TableUserSettings RSO for its Table — no
Field or FieldUserSettings RSO"
(deftest field-update-creates-field-user-settings-rso-when-fus-exists-test
(testing "field-update on an eligible field with a FUS row creates a FieldUserSettings RSO for the Field — no
Field or TableUserSettings RSO"
(mt/with-temp [:model/Collection coll {:is_remote_synced true :name "Remote-Sync" :type "library-data"}
:model/Table table {:name "T" :is_published true :collection_id (:id coll)}
:model/Field field {:name "f" :table_id (:id table) :base_type :type/Text}]
@@ -1361,13 +1361,25 @@
(t2/delete! :model/RemoteSyncObject)
(events/publish-event! :event/field-update {:object field :user-id (mt/user->id :rasta)})
(let [entries (t2/select :model/RemoteSyncObject)]
(is (= 1 (count entries)) "exactly one RSO — the Table's TableUserSettings, no Field/FieldUserSettings RSO")
(is (=? {:model_type "TableUserSettings"
:model_id (:id table)
(is (= 1 (count entries)) "exactly one RSO — the Field's FieldUserSettings")
(is (=? {:model_type "FieldUserSettings"
:model_id (:id field)
:status "update"
:model_table_id (:id table)}
(first entries)))))))
(deftest field-update-tracks-field-of-table-published-in-its-user-settings-test
(testing "field-update on a field of a Table published through its TableUserSettings creates a FieldUserSettings RSO"
(mt/with-temp [:model/Collection coll {:is_remote_synced true :name "Remote-Sync" :type "library-data"}
:model/Table table {:name "T"}
:model/Field field {:name "f" :table_id (:id table) :base_type :type/Text}]
(t2/insert! :model/TableUserSettings {:table_id (:id table) :is_published true :collection_id (:id coll)})
(t2/insert! :model/FieldUserSettings {:field_id (:id field) :description "curated"})
(t2/delete! :model/RemoteSyncObject)
(events/publish-event! :event/field-update {:object field :user-id (mt/user->id :rasta)})
(is (=? [{:model_type "FieldUserSettings" :model_id (:id field) :status "update"}]
(t2/select :model/RemoteSyncObject))))))
;;; ------------------------------------- Concurrent Un-Sync Race Tests -------------------------------------
;;;
;;; When a card-update event and a collection disable run concurrently, the handler must not resurrect the
@@ -1471,6 +1483,21 @@
(deref handler 10000 nil)))))
(assert-removal-survived coll-id card))))))
(deftest dimension-change-creates-dimension-rso-test
(testing "adding or removing a Field's Dimension through the API tracks the Field's Dimension"
(mt/with-temporary-setting-values [remote-sync-type :read-write]
(mt/with-temp [:model/Collection coll {:is_remote_synced true :name "Remote-Sync" :type "library-data"}
:model/Table table {:name "T" :is_published true :collection_id (:id coll)}
:model/Field field {:name "f" :table_id (:id table) :base_type :type/Text}]
(t2/delete! :model/RemoteSyncObject)
(mt/user-http-request :crowberto :post 200 (format "field/%d/dimension" (:id field))
{:type "internal" :name "Remapped"})
(is (=? [{:model_type "Dimension" :model_id (:id field) :status "update"}]
(t2/select :model/RemoteSyncObject)))
(mt/user-http-request :crowberto :delete 204 (format "field/%d/dimension" (:id field)))
(is (=? [{:model_type "Dimension" :model_id (:id field) :status "removed"}]
(t2/select :model/RemoteSyncObject)))))))
(deftest field-update-no-rso-when-no-fus-row-test
(testing "field-update on an eligible field with NO FUS row, and no TableUserSettings row either, creates no
RSOs at all"
@@ -1502,24 +1529,21 @@
(events/publish-event! :event/field-update {:object field :user-id (mt/user->id :rasta)})
(is (empty? (t2/select :model/RemoteSyncObject))))))
(deftest field-update-table-user-settings-rso-marks-removed-when-table-leaves-sync-scope-test
(testing "existing TableUserSettings RSO is marked removed when the field's table leaves sync scope — no
Field or FieldUserSettings RSO created"
(deftest field-update-field-user-settings-rso-marks-removed-when-table-leaves-sync-scope-test
(testing "an existing FieldUserSettings RSO is marked removed when the field's table leaves sync scope"
(mt/with-temp [:model/Collection synced {:is_remote_synced true :name "Synced" :type "library-data"}
:model/Collection normal {:name "Normal" :type "library-data"}
:model/Table table {:name "T" :is_published true :collection_id (:id synced)}
:model/Field field {:name "f" :table_id (:id table) :base_type :type/Text}]
(t2/insert! :model/FieldUserSettings {:field_id (:id field) :description "curated"})
(t2/delete! :model/RemoteSyncObject)
(t2/insert! :model/RemoteSyncObject {:model_type "TableUserSettings"
:model_id (:id table)
:model_name "T"
(t2/insert! :model/RemoteSyncObject {:model_type "FieldUserSettings"
:model_id (:id field)
:model_name "f"
:model_table_id (:id table)
:status "synced"
:status_changed_at (t/offset-date-time)})
(t2/update! :model/Table (:id table) {:collection_id (:id normal)})
(events/publish-event! :event/field-update {:object field :user-id (mt/user->id :rasta)})
(let [entries (t2/select :model/RemoteSyncObject)]
(is (= 1 (count entries)) "still only one RSO — no Field/FieldUserSettings RSO added")
(is (= "removed"
(:status (t2/select-one :model/RemoteSyncObject :model_type "TableUserSettings" :model_id (:id table)))))))))
(is (=? [{:model_type "FieldUserSettings" :model_id (:id field) :status "removed"}]
(t2/select :model/RemoteSyncObject))))))

114
enterprise/backend/test/metabase_enterprise/remote_sync/impl_test.clj
View File

@@ -19,6 +19,8 @@
[metabase.settings.core :as setting]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
[metabase.util.yaml :as yaml]
[toucan2.core :as t2]))
(use-fixtures :once (fixtures/initialize :db))
@@ -2279,10 +2281,16 @@ serdes/meta:
;;; --------------------------------- Table/Field user-settings inline round trip ---------------------------------
(defn- import-snapshot!
"Run a forced import of `mock-source`, closing the task afterwards, returning the import result."
[mock-source]
(let [task-id (t2/insert-returning-pk! :model/RemoteSyncTask {:sync_task_type "import" :initiated_by (mt/user->id :rasta)})]
(u/prog1 (impl/import! (source.p/snapshot mock-source) task-id :force? true)
(remote-sync.task/complete-sync-task! task-id))))
(deftest table-and-field-user-settings-round-trip-test
(testing "a full export writes one file per published Table with user edits -- the TableUserSettings entity at
the Table's own path, inlining its edited Fields' settings under :fields -- and a later import
restores it, pruning field edits the file no longer lists"
(testing "a full export writes a file per settings row and Dimension of a published Table, and an import makes the
instance match them, deleting what has no file"
(mt/with-temporary-setting-values [remote-sync-type :read-write]
(mt/with-temp [:model/Database {db-id :id} {:name "test-db"}
:model/Collection {coll-id :id} {:name "RS" :is_remote_synced true :location "/"}
@@ -2292,34 +2300,88 @@ serdes/meta:
:model/Field {f2-id :id} {:name "F2" :table_id table-id :base_type :type/Text}]
(t2/insert! :model/TableUserSettings {:table_id table-id :display_name "Renamed"})
(t2/insert! :model/FieldUserSettings {:field_id f1-id :description "curated"})
(t2/insert! :model/Dimension {:field_id f2-id :name "Remapped F2" :type :internal})
(let [export-task-id (t2/insert-returning-pk! :model/RemoteSyncTask {:sync_task_type "export" :initiated_by (mt/user->id :rasta)})
mock-source (test-helpers/create-mock-source)
export-result (impl/export! (source.p/snapshot mock-source) export-task-id "Test export" :force? true)]
(is (= :success (:status export-result)))
table-files #(into {} (filter (fn [[path _]] (str/includes? path "test_table"))) (get @(:files-atom mock-source) "main"))]
(is (= :success (:status (impl/export! (source.p/snapshot mock-source) export-task-id "Test export" :force? true))))
(remote-sync.task/complete-sync-task! export-task-id)
(let [files (get @(:files-atom mock-source) "main")
table-files (into {} (filter (fn [[p _]] (str/includes? p "test_table"))) files)]
(is (= 1 (count table-files))
(str "expected exactly one Table-related file, got " (keys table-files)))
(let [[path content] (first table-files)]
(is (str/ends-with? path "test_table.yaml")
"the file lives at the Table's own path, not a ___tableusersettings/___fieldusersettings file")
(is (not (str/includes? path "___tableusersettings")))
(is (not (str/includes? path "___fieldusersettings")))
(is (str/includes? content "Renamed"))
(is (str/includes? content "fields:"))
(is (str/includes? content "curated"))))
(testing "each settings row and Dimension is a file of its own, and the Table and Fields are not written"
(is (= #{"test_table___tableusersettings.yaml" "f1___fieldusersettings.yaml" "f2___dimension.yaml"}
(set (map #(last (str/split % #"/")) (keys (table-files)))))))
(testing "an import restores the files' rows and deletes the ones with no file"
(t2/insert! :model/FieldUserSettings {:field_id f2-id :description "stale"})
(t2/update! :model/TableUserSettings :table_id table-id {:display_name "Local Edit"})
(let [import-task-id (t2/insert-returning-pk! :model/RemoteSyncTask {:sync_task_type "import" :initiated_by (mt/user->id :rasta)})
import-result (impl/import! (source.p/snapshot mock-source) import-task-id :force? true)]
(is (= :success (:status import-result)))
(is (= "Renamed" (:display_name (t2/select-one :model/TableUserSettings :table_id table-id)))
"T's user display_name is restored from the import")
(is (= "curated" (:description (t2/select-one :model/FieldUserSettings :field_id f1-id)))
"F1's edited description is present")
(is (not (t2/exists? :model/FieldUserSettings :field_id f2-id))
"F2's stale local row, absent from the imported file's :fields, is gone")))))))
(t2/delete! :model/Dimension :field_id f2-id)
(is (= :success (:status (import-snapshot! mock-source))))
(is (= "Renamed" (t2/select-one-fn :display_name :model/TableUserSettings :table_id table-id)))
(is (= "curated" (t2/select-one-fn :description :model/FieldUserSettings :field_id f1-id)))
(is (not (t2/exists? :model/FieldUserSettings :field_id f2-id)))
(is (= "Remapped F2" (t2/select-one-fn :name :model/Dimension :field_id f2-id))))
(testing "a settings file v64 wrote, carrying its Fields' settings under `fields`, keeps them"
(let [files (table-files)
content (fn [suffix] (yaml/parse-string (val (u/seek #(str/ends-with? (key %) suffix) files))))
legacy (assoc (content "test_table___tableusersettings.yaml")
:fields [(content "f1___fieldusersettings.yaml")])
[path _] (u/seek #(str/ends-with? (key %) "test_table___tableusersettings.yaml") files)]
(swap! (:files-atom mock-source) update "main"
#(-> (into {} (remove (fn [[file _]] (str/includes? file "test_table"))) %)
(assoc (str/replace path "___tableusersettings" "") (yaml/generate-string legacy)))))
(is (= :success (:status (import-snapshot! mock-source))))
(is (= "curated" (t2/select-one-fn :description :model/FieldUserSettings :field_id f1-id)))
(is (not (t2/exists? :model/Dimension :field_id f2-id))))
(testing "a settings file v64 wrote for a Table without Field settings carries an empty `fields`"
(swap! (:files-atom mock-source) update "main"
(fn [files]
(into {} (map (fn [[file content]]
[file (cond-> content
(str/ends-with? file "test_table.yaml")
(-> yaml/parse-string (assoc :fields []) yaml/generate-string))]))
files)))
(is (= :success (:status (import-snapshot! mock-source))))
(is (not (t2/exists? :model/FieldUserSettings :field_id f1-id))))
(testing "a Field file an older git sync wrote keeps the Dimensions it carries"
(swap! (:files-atom mock-source) update "main"
assoc "databases/test-db/tables/test_table/fields/f2.yaml"
(yaml/generate-string {:name "F2"
:table_id ["test-db" nil "Test Table"]
:base_type "type/Text"
:dimensions [{:name "Remapped F2" :type "internal" :entity_id (u/generate-nano-id)}]
:serdes/meta [{:model "Database" :id "test-db"}
{:model "Table" :id "Test Table"}
{:model "Field" :id "F2"}]}))
(is (= :success (:status (import-snapshot! mock-source))))
(is (= "Remapped F2" (t2/select-one-fn :name :model/Dimension :field_id f2-id))))
(testing "deleting the files drops the Table's settings and its Fields'"
(swap! (:files-atom mock-source) update "main"
#(into {} (remove (fn [[file _]] (str/includes? file "test_table"))) %))
(is (= :success (:status (import-snapshot! mock-source))))
(is (not (t2/exists? :model/TableUserSettings :table_id table-id)))
(is (not (t2/exists? :model/FieldUserSettings :field_id f1-id)))))))))
(deftest user-settings-removal-after-import-test
(testing "removing a Dimension an import brought is tracked, and pushing the removal leaves nothing pending"
(mt/with-temporary-setting-values [remote-sync-type :read-write]
(mt/with-temp [:model/Database {db-id :id} {:name "test-db"}
:model/Collection {coll-id :id} {:name "RS" :is_remote_synced true :location "/"}
:model/Table {table-id :id} {:name "Test Table" :db_id db-id
:is_published true :collection_id coll-id}
:model/Field {field-id :id} {:name "F1" :table_id table-id :base_type :type/Text}]
(t2/insert! :model/Dimension {:field_id field-id :name "Remapped" :type :internal})
(let [mock-source (test-helpers/create-mock-source)
export! (fn []
(let [task-id (t2/insert-returning-pk! :model/RemoteSyncTask {:sync_task_type "export" :initiated_by (mt/user->id :rasta)})]
(u/prog1 (impl/export! (source.p/snapshot mock-source) task-id "Test export" :force? true)
(remote-sync.task/complete-sync-task! task-id))))
dimension-rso #(t2/select-one :model/RemoteSyncObject :model_type "Dimension" :model_id field-id)]
(is (= :success (:status (export!))))
(is (= :success (:status (import-snapshot! mock-source))))
(mt/user-http-request :crowberto :delete 204 (format "field/%d/dimension" field-id))
(is (=? {:status "removed"} (dimension-rso)))
(is (= :success (:status (export!))))
(is (nil? (dimension-rso)))
(mt/user-http-request :crowberto :put 200 (format "field/%d" field-id) {:description "edited"})
(is (nil? (dimension-rso))))))))
;; ---------- run-task-body!: the row is always closed, whatever the worker does ----------------

25
enterprise/backend/test/metabase_enterprise/remote_sync/spec_test.clj
View File

@@ -688,8 +688,8 @@
(deftest git-sync-exports-only-user-settings-test
(testing "git sync stores what users changed about a Table and its Fields, never the Table or Fields themselves --
those belong to sync, which runs against each instance's own warehouse -- as one TableUserSettings
entity per Table inlining its Fields' edits, never separate FieldUserSettings entities"
those belong to sync, which runs against each instance's own warehouse -- each settings row and Dimension
as an entity of its own"
(mt/with-premium-features #{:library}
(mt/with-temp [:model/Collection {coll-id :id} {:is_remote_synced true :name "RS" :type "library-data"}
:model/Database {db-id :id} {:name "DB"}
@@ -699,17 +699,16 @@
:model/Field {f2 :id} {:name "F2" :table_id table-id}]
(t2/insert! :model/FieldUserSettings {:field_id f2 :description "curated" :description_set true})
(t2/insert! :model/TableUserSettings {:table_id table-id :display_name "Renamed"})
(let [exportable (spec/exportable-entities)]
(is (contains? (set (get exportable "TableUserSettings")) table-id)
"the Table's own edit, plus its edited Field's, are carried by one TableUserSettings entity")
(is (nil? (get exportable "FieldUserSettings"))
"FieldUserSettings is never exported as its own entity")
(let [dimension-id (t2/insert-returning-pk! :model/Dimension {:field_id f1 :name "F1" :type :internal})
exportable (spec/exportable-entities)]
(is (= [table-id] (get exportable "TableUserSettings")))
(is (= [f2] (get exportable "FieldUserSettings")))
(is (= [dimension-id] (get exportable "Dimension")))
(is (not (contains? (set (get exportable "Table")) table-id)))
(is (empty? (filter #{f1 f2} (get exportable "Field")))))))))
(deftest git-sync-exports-table-user-settings-for-field-only-edit-test
(testing "a Table with no TableUserSettings row of its own, but an edited Field, is still exportable -- the
TableUserSettings entity is synthesized to carry the Field's edit"
(deftest git-sync-exports-no-table-user-settings-for-field-only-edit-test
(testing "a Table with no TableUserSettings row of its own exports only its edited Field's settings"
(mt/with-premium-features #{:library}
(mt/with-temp [:model/Collection {coll-id :id} {:is_remote_synced true :name "RS" :type "library-data"}
:model/Database {db-id :id} {:name "DB"}
@@ -718,10 +717,8 @@
:model/Field {f1 :id} {:name "F1" :table_id table-id}]
(t2/insert! :model/FieldUserSettings {:field_id f1 :description "curated" :description_set true})
(let [exportable (spec/exportable-entities)]
(is (not (t2/exists? :model/TableUserSettings :table_id table-id))
"the Table has no settings row of its own")
(is (contains? (set (get exportable "TableUserSettings")) table-id)
"the Table is still exportable, synthesized from its Field's edit"))))))
(is (nil? (get exportable "TableUserSettings")))
(is (= [f1] (get exportable "FieldUserSettings"))))))))
(deftest ^:parallel exportable-entity-count-test
(testing "exportable-entity-count sums the ids across every model in the targets map"

3
enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
View File

@@ -2,6 +2,7 @@
{:clj-kondo/config '{:linters {:deprecated-var {:exclude {metabase.test.data/mbql-query {:namespaces [metabase-enterprise.serialization.v2.e2e-test]}}}}}}
(:require
[clojure.java.io :as io]
[clojure.string :as str]
[clojure.test :refer :all]
[medley.core :as m]
[metabase-enterprise.serialization.cmd :as cmd]
@@ -246,7 +247,7 @@
table (subdirs (io/file dump-dir "databases" db "tables"))
:let [fields-dir (io/file table "fields")]
:when (.exists fields-dir)]
(count (dir->file-set fields-dir)))))
(count (remove #(str/includes? % "___") (dir->file-set fields-dir))))))
"Fields are scattered, so the directories are harder to count"))
(testing "for cards, dashboards, and timelines"
;; In the new storage format, cards/dashboards/timelines are stored directly

107
enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
View File

@@ -546,46 +546,36 @@
:type "external"
:field_id fk-id
:human_readable_field_id cust-name}]
(testing "dimensions without foreign keys are inlined into their Fields\n"
(let [ser (ts/extract-one "Field" email-id)]
(testing "a dimension without a foreign key is its own entity under its Field\n"
(let [ser (ts/extract-one "Dimension" dim1-eid)]
(is (malli= [:map
[:serdes/meta [:= [{:model "Database", :id "My Database"}
{:model "Table", :id "Schemaless Table"}
{:model "Field", :id "email"}]]]
[:dimensions [:sequential
[:map
{:model "Field", :id "email"}
{:model "Dimension", :id dim1-eid}]]]
[:created_at :string]
[:human_readable_field_id {:optional true} [:maybe [:sequential [:maybe :string]]]]]]]]
[:human_readable_field_id {:optional true} [:maybe [:sequential [:maybe :string]]]]]
ser))
(is (not (contains? ser :id)))
(testing "As of #27062 a Field can only have one Dimension. For historic reasons it comes back as a list"
(is (= [dim1-eid]
(->> ser :dimensions (map :entity_id)))))
(testing "depend only on the Database; the Table is synthesized on import if missing"
(is (not (contains? ser :field_id)) ":field_id is dropped; it's implied by the path")
(testing "depend only on the Database; the Table and Field are synthesized on import if missing"
(is (= #{[{:model "Database" :id "My Database"}]}
(set (serdes/deserialization-dependencies ser)))))))
(testing "foreign key dimensions are inlined into their Fields"
(let [ser (ts/extract-one "Field" fk-id)]
(is (malli= [:map
[:serdes/meta [:= [{:model "Database" :id "My Database"}
(testing "a foreign key dimension is its own entity under its Field"
(let [ser (ts/extract-one "Dimension" (t2/select-one-fn :entity_id :model/Dimension :field_id fk-id))]
(is (=? {:serdes/meta [{:model "Database" :id "My Database"}
{:model "Schema" :id "PUBLIC"}
{:model "Table" :id "Orders"}
{:model "Field" :id "customer_id"}]]]
[:name [:= "customer_id"]]
[:fk_target_field_id [:= ["My Database" "PUBLIC" "Customers" "id"]]]
[:dimensions [:sequential
[:map
[:human_readable_field_id [:maybe [:sequential [:maybe :string]]]]
[:created_at :string]]]]]
{:model "Field" :id "customer_id"}
{:model "Dimension" :id string?}]
:human_readable_field_id ["My Database" "PUBLIC" "Customers" "name"]
:created_at string?}
ser))
(is (not (contains? ser :id)))
(testing "dimensions are properly inlined"
(is (=? [{:human_readable_field_id ["My Database" "PUBLIC" "Customers" "name"]
:created_at string?}]
(:dimensions ser))))
(testing "depend only on the Database; the Table, FK target and human-readable Fields are synthesized on import if missing"
(testing "depend only on the Database; the FK target and human-readable Fields are synthesized on import if missing"
(is (= #{[{:model "Database" :id "My Database"}]}
(set (serdes/deserialization-dependencies ser))))))))))
(set (serdes/deserialization-dependencies ser)))))))
(testing "Fields carry no dimensions"
(is (not (contains? (ts/extract-one "Field" fk-id) :dimensions)))))))
(deftest native-query-snippets-test
(mt/with-empty-h2-app-db!
@@ -1187,10 +1177,10 @@
(testing "extract-metabase behavior"
(let [entities (into [] (extract/extract {}))
models (map (comp :model last :serdes/meta) entities)]
(is (empty? (filter #{"FieldUserSettings"} models))
"written inside its Table's settings, never on its own")
(is (= 1 (count (filter #{"TableUserSettings"} models))))
(is (= 1 (count (:fields (first (filter #(= "TableUserSettings" (-> % :serdes/meta last :model)) entities)))))))))))
(is (= 1 (count (filter #{"FieldUserSettings"} models)))
"written as its own entity, only for the Field that has settings")
(is (empty? (filter #{"TableUserSettings"} models))
"no TableUserSettings for a Table without a settings row"))))))
(deftest table-descendants-user-settings-test
(mt/with-empty-h2-app-db!
@@ -1202,11 +1192,14 @@
(testing "every Field is a descendant, and none has user settings to speak of yet"
(is (= #{["Field" f1-id] ["Field" f2-id] ["Field" f3-id]}
(set (keys (serdes/descendants "Table" table-id {}))))))
(testing "a Field with user settings makes the Table's settings a descendant, since they carry the Field's"
(testing "a Field's settings and Dimension are descendants of their own"
(t2/insert! :model/FieldUserSettings {:field_id f2-id :description "edited"})
(is (= #{["Field" f1-id] ["Field" f2-id] ["Field" f3-id] ["TableUserSettings" table-id]}
(set (keys (serdes/descendants "Table" table-id {})))))
(t2/delete! :model/FieldUserSettings :field_id f2-id))
(let [dimension-id (t2/insert-returning-pk! :model/Dimension {:field_id f3-id :name "F3" :type :internal})]
(is (= #{["Field" f1-id] ["Field" f2-id] ["Field" f3-id]
["FieldUserSettings" f2-id] ["Dimension" dimension-id]}
(set (keys (serdes/descendants "Table" table-id {}))))))
(t2/delete! :model/FieldUserSettings :field_id f2-id)
(t2/delete! :model/Dimension :field_id f3-id))
(testing "a Table with user settings appears as TableUserSettings"
(t2/insert! :model/TableUserSettings {:table_id table-id :display_name "Renamed"})
(is (contains? (set (keys (serdes/descendants "Table" table-id {})))
@@ -1214,40 +1207,24 @@
(testing "Field is a leaf node in the descendants graph"
(is (empty? (serdes/descendants "Field" f1-id {})))))))
(deftest inline-user-settings-extract-test
(deftest table-user-settings-extract-test
(mt/with-empty-h2-app-db!
(ts/with-temp-dpc [:model/Database {db-id :id} {:name "DB"}
:model/Table {table-id :id} {:name "T" :db_id db-id}
:model/Table {other-id :id} {:name "Unedited T" :db_id db-id}
:model/Field {f1-id :id} {:name "F1" :table_id table-id}
:model/Field {_f2-id :id} {:name "F2" :table_id table-id}
:model/FieldUserSettings _ {:field_id f1-id :description "edited"}]
(testing "a Table with only a Field edit synthesizes a TableUserSettings entity with that Field inlined"
(let [entities (into [] (serdes/extract-all "TableUserSettings"
{:inline-user-settings true
:filter-column :table_id
:filter-ids [table-id]}))]
(is (= 1 (count entities)))
(let [entity (first entities)]
(is (=? [{:model "Database" :id "DB"}
{:model "Table" :id "T"}
{:model "TableUserSettings" :id "1"}]
(:serdes/meta entity))
"synthesized for a Table with no settings row of its own")
(is (nil? (:display_name entity)))
(is (= 1 (count (:fields entity))))
(is (=? {:description "edited"
:model/FieldUserSettings _ {:field_id f1-id :description "edited"}
:model/TableUserSettings _ {:table_id table-id :display_name "Renamed"}]
(testing "a Table's settings carry only its own values, not its Fields'"
(is (=? [{:display_name "Renamed"
:serdes/meta [{:model "Database" :id "DB"}
{:model "Table" :id "T"}
{:model "Field" :id "F1"}
{:model "FieldUserSettings" :id "1"}]}
(first (:fields entity)))
"the edited Field is inlined, with its own serdes/meta")
(is (not (contains? (first (:fields entity)) :field_id))))))
(testing "a Table with no edits at all yields nothing"
(is (empty? (into [] (serdes/extract-all "TableUserSettings"
{:inline-user-settings true
:filter-column :table_id
{:model "TableUserSettings" :id "1"}]}]
(into [] (serdes/extract-all "TableUserSettings" {:filter-column :table_id
:filter-ids [table-id]}))))
(is (not (contains? (ts/extract-one "TableUserSettings" table-id) :fields))))
(testing "a Table with no settings row yields nothing"
(is (empty? (into [] (serdes/extract-all "TableUserSettings" {:filter-column :table_id
:filter-ids [other-id]}))))))))
(deftest collection-export-includes-user-settings-test
@@ -1270,8 +1247,8 @@
(set (map :name (by-model "Field")))))
(is (= 1 (count (by-model "TableUserSettings"))))
(is (= #{"F2"}
(set (map #(-> % :serdes/meta (nth 2) :id) (:fields (first (by-model "TableUserSettings"))))))
"only the field the user edited has settings to export, inside the Table's"))))))
(set (map #(-> % :serdes/meta (nth 2) :id) (by-model "FieldUserSettings"))))
"only the field the user edited has settings to export"))))))
(deftest cards-test
(mt/with-empty-h2-app-db!

125
enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
View File

@@ -178,46 +178,109 @@
(is (= "Basic Collection" (:name (first colls))))
(is (= eid1 (:entity_id (first colls)))))))))))
(deftest inline-user-settings-round-trip-test
(testing "git sync's inline-user-settings mode replaces a Table's FieldUserSettings wholesale on import"
(let [serialized (atom nil)]
(deftest user-settings-round-trip-test
(testing "Table settings, Field settings and Dimensions round-trip as entities of their own"
(let [serialized (atom nil)
extract (fn [model-name filter-column ids]
(serdes/extract-all model-name {:filter-column filter-column :filter-ids ids}))]
(ts/with-dbs [source-db dest-db]
(testing "extracting inline from the source"
(ts/with-db source-db
(let [db (ts/create! :model/Database :name "my-db")
table (ts/create! :model/Table :name "customers" :db_id (:id db))
f1 (ts/create! :model/Field :name "age" :table_id (:id table))
f2 (ts/create! :model/Field :name "email" :table_id (:id table))]
age (ts/create! :model/Field :name "age" :table_id (:id table))
email (ts/create! :model/Field :name "email" :table_id (:id table))]
(t2/insert! :model/TableUserSettings {:table_id (:id table) :display_name "Renamed"})
(t2/insert! :model/FieldUserSettings {:field_id (:id f1) :description "edited"})
(t2/insert! :model/FieldUserSettings {:field_id (:id age) :description "edited"})
(ts/create! :model/Dimension :field_id (:id email) :name "Email" :type :internal)
(reset! serialized
(into [(ts/extract-one "Database" (:id db))
(-> [(ts/extract-one "Database" (:id db))
(ts/extract-one "Table" (:id table))
(ts/extract-one "Field" (:id f1))
(ts/extract-one "Field" (:id f2))]
(serdes/extract-all "TableUserSettings"
{:inline-user-settings true
:filter-column :table_id
:filter-ids [(:id table)]}))))))
(testing "the extracted TableUserSettings inlines F1's edit and no others"
(let [tus (first (by-model @serialized "TableUserSettings"))]
(is (= "Renamed" (:display_name tus)))
(is (= 1 (count (:fields tus))))
(is (= "edited" (:description (first (:fields tus)))))))
(testing "loading into a destination where the same Database/Table/Fields exist, and F2 has a stale row"
(ts/extract-one "Field" (:id age))
(ts/extract-one "Field" (:id email))]
(into (extract "TableUserSettings" :table_id [(:id table)]))
(into (extract "FieldUserSettings" :field_id [(:id age)]))
(into (extract "Dimension" :field_id [(:id email)]))))))
(testing "each is extracted on its own, with nothing nested"
(is (=? {:display_name "Renamed"} (first (by-model @serialized "TableUserSettings"))))
(is (not (contains? (first (by-model @serialized "TableUserSettings")) :fields)))
(is (=? [{:description "edited"}] (by-model @serialized "FieldUserSettings")))
(is (=? [{:name "Email"}] (by-model @serialized "Dimension")))
(is (not-any? #(contains? % :dimensions) (by-model @serialized "Field"))))
(ts/with-db dest-db
(let [db (ts/create! :model/Database :name "my-db")
table (ts/create! :model/Table :name "customers" :db_id (:id db))
f1 (ts/create! :model/Field :name "age" :table_id (:id table))
f2 (ts/create! :model/Field :name "email" :table_id (:id table))]
(t2/insert! :model/FieldUserSettings {:field_id (:id f2) :description "stale"})
age (ts/create! :model/Field :name "age" :table_id (:id table))
email (ts/create! :model/Field :name "email" :table_id (:id table))]
(ts/create! :model/Dimension :field_id (:id email) :name "Local" :type :internal)
(serdes.load/load-metabase! (ingestion-in-memory @serialized))
(testing "the Table's settings arrive"
(is (= "Renamed" (t2/select-one-fn :display_name :model/TableUserSettings :table_id (:id table)))))
(testing "F1's settings arrive"
(is (= "edited" (t2/select-one-fn :description :model/FieldUserSettings :field_id (:id f1)))))
(testing "F2's stale row is deleted -- replace semantics"
(is (nil? (t2/select-one :model/FieldUserSettings :field_id (:id f2))))))))))))
(is (= "Renamed" (t2/select-one-fn :display_name :model/TableUserSettings :table_id (:id table))))
(is (= "edited" (t2/select-one-fn :description :model/FieldUserSettings :field_id (:id age))))
(testing "the imported Dimension replaces the Field's local one"
(is (= ["Email"] (t2/select-fn-vec :name :model/Dimension :field_id (:id email)))))))))))
(deftest legacy-field-dimensions-import-test
(testing "a Field file written before Dimensions got files of their own still carries them"
(mt/with-empty-h2-app-db!
(let [db (ts/create! :model/Database :name "my-db")
table (ts/create! :model/Table :name "customers" :db_id (:id db))
email (ts/create! :model/Field :name "email" :table_id (:id table))
dimension (dissoc (ts/extract-one "Dimension" (:entity_id (ts/create! :model/Dimension :field_id (:id email)
:name "Email" :type :internal)))
:serdes/meta)
field (ts/extract-one "Field" (:id email))
load! (fn [entity] (serdes.load/load-metabase! (ingestion-in-memory [entity])))]
(t2/delete! :model/Dimension :field_id (:id email))
(load! (assoc field :dimensions [dimension]))
(is (= "Email" (t2/select-one-fn :name :model/Dimension :field_id (:id email))))
(testing "an empty list deletes them"
(load! (assoc field :dimensions []))
(is (not (t2/exists? :model/Dimension :field_id (:id email)))))))))
(deftest legacy-table-user-settings-fields-import-test
(testing "a settings file written before Field settings got files of their own replaces its Table's Field settings"
(mt/with-empty-h2-app-db!
(let [db (ts/create! :model/Database :name "my-db")
table (ts/create! :model/Table :name "customers" :db_id (:id db))
age (ts/create! :model/Field :name "age" :table_id (:id table))
email (ts/create! :model/Field :name "email" :table_id (:id table))
_ (t2/insert! :model/FieldUserSettings {:field_id (:id age) :description "edited"})
legacy {:serdes/meta [{:model "Database" :id "my-db"}
{:model "Table" :id "customers"}
{:model "TableUserSettings" :id "1"}]
:fields [(ts/extract-one "FieldUserSettings" (:id age))]}]
(t2/delete! :model/FieldUserSettings :field_id (:id age))
(t2/insert! :model/FieldUserSettings {:field_id (:id email) :description "stale"})
(serdes.load/load-metabase! (ingestion-in-memory [legacy]))
(is (= "edited" (t2/select-one-fn :description :model/FieldUserSettings :field_id (:id age))))
(is (not (t2/exists? :model/FieldUserSettings :field_id (:id email))))))))
(deftest field-user-settings-import-clears-unset-values-test
(testing "importing a Field's settings over an existing row clears the user values its file leaves out"
(mt/with-empty-h2-app-db!
(let [db (ts/create! :model/Database :name "my-db")
table (ts/create! :model/Table :name "customers" :db_id (:id db))
email (ts/create! :model/Field :name "email" :table_id (:id table))
_ (t2/insert! :model/FieldUserSettings {:field_id (:id email) :description "edited"})
settings (ts/extract-one "FieldUserSettings" (:id email))]
(t2/update! :model/FieldUserSettings (:id email) {:display_name "Local"})
(serdes.load/load-metabase! (ingestion-in-memory [settings]))
(is (=? {:description "edited" :display_name nil}
(t2/select-one :model/FieldUserSettings :field_id (:id email))))))))
(deftest nested-field-dimension-import-test
(testing "a Dimension of a nested Field in a Table without a schema imports onto that Field"
(mt/with-empty-h2-app-db!
(let [db (ts/create! :model/Database :name "my-db")
table (ts/create! :model/Table :name "orders" :db_id (:id db))
customer (ts/create! :model/Field :name "customer" :table_id (:id table))
tier (ts/create! :model/Field :name "tier" :table_id (:id table) :parent_id (:id customer))
dimension (ts/create! :model/Dimension :field_id (:id tier) :name "Tier" :type :internal)
extracted (ts/extract-one "Dimension" (:entity_id dimension))
tables (t2/count :model/Table)]
(t2/delete! :model/Dimension :field_id (:id tier))
(serdes.load/load-metabase! (ingestion-in-memory [extracted]))
(is (= [(:id tier)] (t2/select-fn-vec :field_id :model/Dimension :entity_id (:entity_id dimension))))
(is (= tables (t2/count :model/Table)))))))
(deftest escape-continue-on-error-roundtrip-test
(testing "archive exported past escape analysis imports under continue-on-error without crashing (#74622)"
@@ -2210,9 +2273,9 @@
(reset! serialized (into [] (serdes.extract/extract {})))
(testing "the labeled Field and its FieldUserSettings both export the label"
(is (= :PII (:data_sensitivity (field-ser @serialized "CONTACT_EMAIL"))))
(is (= :PII (-> (u/seek #(and (in-db? %) (-> % :serdes/meta last :model (= "TableUserSettings")))
(is (= :PII (-> (u/seek #(and (in-db? %) (-> % :serdes/meta last :model (= "FieldUserSettings")))
@serialized)
:fields first :data_sensitivity))))
:data_sensitivity))))
(testing "the unlabeled Field exports no data_sensitivity key"
(is (not (contains? (field-ser @serialized "CONTACT_ROW") :data_sensitivity))))))
(ts/with-db dest-db

48
enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj
View File

@@ -158,25 +158,51 @@
(storage/store! (into [] (extract/extract {})) (storage.files/file-writer dump-dir))
(let [fields-dir (io/file dump-dir "databases" "my_company_data" "tables" "customers" "fields")
read-yaml (fn [file-name] (yaml/from-file (io/file fields-dir file-name)))]
(testing "the label is written as the bare enum string on the Field and inside the Table's settings"
(testing "the label is written as the bare enum string on the Field and its settings"
(is (= "PII" (:data_sensitivity (read-yaml "email.yaml"))))
(is (= "PII" (-> (yaml/from-file (io/file fields-dir ".." "customers___tableusersettings.yaml"))
:fields first :data_sensitivity))))
(is (= "PII" (:data_sensitivity (read-yaml "email___fieldusersettings.yaml")))))
(testing "an unlabeled field's file has no data_sensitivity line"
(is (not (contains? (read-yaml "id.yaml") :data_sensitivity)))))))))
(deftest inline-user-settings-storage-path-test
(deftest user-settings-storage-test
(ts/with-random-dump-dir [dump-dir "serdesv2-"]
(mt/with-empty-h2-app-db!
(ts/with-temp-dpc [:model/Database db {:name "My Company Data"}
:model/Table table {:name "Customers" :db_id (:id db)}
:model/Field email {:name "Email" :table_id (:id table)}
:model/FieldUserSettings _ {:field_id (:id email) :description "edited"}]
(let [[tus] (into [] (serdes/extract-all "TableUserSettings" {:filter-column :table_id :filter-ids [(:id table)]}))]
(is (= 1 (count (:fields tus))))
(testing "git sync writes the entity at the Table's own path"
(is (= "Customers" (-> (serdes/storage-path tus {:inline-user-settings true}) peek :label))))
(testing "the CLI export writes it beside the Table's file"
(is (= "Customers___tableusersettings" (-> (serdes/storage-path tus {}) peek :label))))))))
:model/Field _ {:name "Id" :table_id (:id table)}
:model/TableUserSettings _ {:table_id (:id table) :display_name "Clients"}
:model/FieldUserSettings _ {:field_id (:id email) :description "edited"}
:model/Dimension _ {:field_id (:id email) :name "Email" :type :internal}]
(storage/store! (into [] (extract/extract {})) (storage.files/file-writer dump-dir))
(testing "each settings entity is a file beside its Table's or Field's, only where one exists"
(is (= #{["customers.yaml"]
["customers___tableusersettings.yaml"]
["fields" "email.yaml"]
["fields" "email___fieldusersettings.yaml"]
["fields" "email___dimension.yaml"]
["fields" "id.yaml"]}
(file-set (io/file dump-dir "databases" "my_company_data" "tables" "customers")))))))))
(deftest long-field-name-user-settings-storage-test
(testing "a long Field name keeps the settings, Dimension and values suffixes, so their files don't replace the Field's"
(ts/with-random-dump-dir [dump-dir "serdesv2-"]
(mt/with-empty-h2-app-db!
(let [field-name (apply str (repeat 150 "x"))
slug (apply str (repeat 100 "x"))]
(ts/with-temp-dpc [:model/Database db {:name "My Company Data"}
:model/Table table {:name "Customers" :db_id (:id db)}
:model/Field field {:name field-name :table_id (:id table)}
:model/FieldUserSettings _ {:field_id (:id field) :description "edited"}
:model/Dimension _ {:field_id (:id field) :name "Long" :type :internal}
:model/FieldValues _ {:field_id (:id field)}]
(storage/store! (into [] (extract/extract {:include-field-values true})) (storage.files/file-writer dump-dir))
(is (= #{[(str slug ".yaml")]
[(str slug "___fieldusersettings.yaml")]
[(str slug "___dimension.yaml")]
[(str slug "___fieldvalues.yaml")]}
(file-set (io/file dump-dir "databases" "my_company_data" "tables" "customers" "fields"))))))))))
(deftest entity-counts-report-test
(ts/with-random-dump-dir [dump-dir "serdesv2-"]
(mt/with-empty-h2-app-db!

5
src/metabase/models/interface.clj
View File

@@ -239,6 +239,11 @@
{:in u/qualified-name
:out keyword})
(def transform-boolean
"Transform for booleans; a boolean computed in SQL comes back as a number from MySQL and MariaDB."
{:in identity
:out (fn [v] (if (number? v) (pos? v) v))})
(def transform-trim
"Transform that trims whitespace on input and output. Useful for fixed-width char columns that pad with spaces."
{:in str/trim

11
src/metabase/models/serialization.clj
View File

@@ -772,10 +772,10 @@
[{:label (:name entity) :key (:entity_id entity)}]])))
(defmulti storage-path
"Returns a vector of maps with `:label` and optional `:key` and `:style` for each path segment.
"Returns a vector of maps with `:label` and optional `:key`, `:style` and `:suffix` for each path segment.
`:label` is the human-readable name; `:key` is a deduplication identity (entity_id, name, or nil); `:style` is
`:name` (the default) for a label that is slugified into a file name, or `:slug` for a label that is already a slug
safe to use as a file name as is.
safe to use as a file name as is; `:suffix` is appended to the file name after the label is slugified and truncated.
Dispatches on model name."
{:arglists '([entity ctx])}
(fn [entity _] (ingested-model entity)))
@@ -1015,6 +1015,13 @@
{:model "Table" :id table-name}
{:model "Field" :id field-name}]))
(defn field-path->field-ref
"The `[db-name schema table-name & field-names]` reference of the Field at `field-path`, nested Fields included."
[field-path]
(let [[table-path fields] (split-with #(not= "Field" (:model %)) field-path)
id-of (fn [model] (some #(when (= model (:model %)) (:id %)) table-path))]
(into [(id-of "Database") (id-of "Schema") (id-of "Table")] (map :id) fields)))
;;; ## MBQL Fields
(mr/def ::mbql-node

51
src/metabase/warehouse_schema/db.clj
View File

@@ -185,13 +185,21 @@
"The columns an insert or update of a FieldUserSettings accepts."
[:field_id :created_at :updated_at :semantic_type :description :display_name :visibility_type :fk_target_field_id :has_field_values :effective_type :coercion_strategy :caveats :points_of_interest :nfc_path :json_unfolding :settings :data_sensitivity :custom_position :description_set :semantic_type_set :fk_target_field_id_set])
(mu/defn field-user-settings-exist-for-table?
"Whether any Field of the ::warehouse-schema.schema/table with `table-id` has a FieldUserSettings row."
(mu/defn field-ids-with-user-settings-for-table :- [:set ::lib.schema.id/field]
"The ids of the Fields of the ::warehouse-schema.schema/table with `table-id` that have a FieldUserSettings row."
[table-id :- ::lib.schema.id/table]
(t2/exists? :model/FieldUserSettings
(set (t2/select-fn-set :field_id :model/FieldUserSettings
{:from [[(t2/table-name :model/FieldUserSettings) :u]]
:join [(warehouse-schema-overlay/field-query {:alias :f, :user-settings? false}) [:= :f.id :u.field_id]]
:where [:= :f.table_id table-id]}))
:where [:= :f.table_id table-id]})))
(mu/defn dimension-ids-for-table :- [:set ::lib.schema.id/dimension]
"The ids of the Dimensions of the Fields of the ::warehouse-schema.schema/table with `table-id`."
[table-id :- ::lib.schema.id/table]
(set (t2/select-pks-set :model/Dimension
{:from [[(t2/table-name :model/Dimension) :d]]
:join [(warehouse-schema-overlay/field-query {:alias :f, :user-settings? false}) [:= :f.id :d.field_id]]
:where [:= :f.table_id table-id]})))
(mu/defn field-user-settings-for-tables
"The FieldUserSettings of the Fields of `table-ids`, each with its Field's `:table_id`, in Field name order."
@@ -286,29 +294,6 @@
[table-id :- ::lib.schema.id/table]
(t2/delete! :model/TableUserSettings :table_id table-id))
(mu/defn table-user-settings-with-field-settings
"One TableUserSettings per Table among `table-ids` (all when nil) that has a settings row or a Field with one,
synthesized as `{:table_id id}` when the Table has no row of its own."
[table-ids :- [:maybe [:sequential ::lib.schema.id/table]]]
(let [flag-columns (set (vals warehouse-schema-overlay/table-user-settings-flags))
value-columns (remove (some-fn #{:table_id} flag-columns) table-user-settings-update-keys)]
(t2/select
:model/TableUserSettings
{:select (into [[:t.id :table_id]]
(concat (map #(u/qualified-key :u %) value-columns)
(map (fn [flag] [[:coalesce (u/qualified-key :u flag) false] flag]) flag-columns)))
:from [(warehouse-schema-overlay/table-query {:alias :t, :user-settings? false})]
:left-join [[(t2/table-name :model/TableUserSettings) :u] [:= :u.table_id :t.id]]
:where [:and
(if table-ids [:in :t.id table-ids] true)
[:or
[:not= :u.table_id nil]
[:exists ^:allow-subquery
{:select [1]
:from [[(t2/table-name :model/FieldUserSettings) :fu]]
:join [(warehouse-schema-overlay/field-query {:alias :f, :user-settings? false}) [:= :f.id :fu.field_id]]
:where [:= :f.table_id :t.id]}]]]})))
(mu/defn delete-field-user-settings-for-table!
"Delete the FieldUserSettings of the Fields of the ::warehouse-schema.schema/table with `table-id`, returning the
number deleted."
@@ -319,6 +304,13 @@
:from [[(t2/table-name :model/Field) :f]]
:where [:and [:= :f.id :field_id] [:= :f.table_id table-id]]}]}))
(mu/defn table-user-settings-for-tables
"The TableUserSettings of the Tables with `table-ids`, or every TableUserSettings when nil."
[table-ids :- [:maybe [:sequential ::lib.schema.id/table]]]
(if table-ids
(t2/select :model/TableUserSettings :table_id [:in table-ids])
(t2/select :model/TableUserSettings)))
;;; ---------------------------------------------- FieldValues ----------------------------------------------
(mu/defn field-values-of-type
@@ -372,6 +364,11 @@
field-ids :- [:set ::lib.schema.id/field]]
(t2/select columns :field_id [:in field-ids] :type :full))
(mu/defn dimension-for-field
"The Dimension of the ::warehouse-schema.schema/field with `field-id`, or nil."
[field-id :- ::lib.schema.id/field]
(t2/select-one :model/Dimension :field_id field-id))
(mu/defn dimensions-for-fields
"The Dimensions of the Fields with `field-ids`."
[field-ids :- [:set ::lib.schema.id/field]]

31
src/metabase/warehouse_schema/models/dimension.clj
View File

@@ -5,6 +5,7 @@
(:require
[metabase.models.interface :as mi]
[metabase.models.serialization :as serdes]
[metabase.warehouse-schema.db :as warehouse-schema.db]
[metabase.warehouse-schema.schema]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -24,9 +25,37 @@
(t2/deftransforms :model/Dimension
{:type mi/transform-keyword})
(defmethod serdes/generate-path "Dimension" [_ {:keys [field_id entity_id]}]
(conj (serdes/generate-path "Field" {:id field_id})
{:model "Dimension" :id entity_id}))
(defmethod serdes/load-find-local "Dimension" [path]
(or ((get-method serdes/load-find-local :default) path)
(when-let [field (serdes/load-find-local (pop path))]
(warehouse-schema.db/dimension-for-field (:id field)))))
(defmethod serdes/deserialization-dependencies "Dimension" [dimension]
[[(first (serdes/path dimension))]])
(defmethod serdes/load-update! "Dimension" [model-name ingested local]
((get-method serdes/load-update! :default) model-name (merge {:human_readable_field_id nil} ingested) local))
(defmethod serdes/make-spec "Dimension" [_model-name _opts]
{:copy [:name :type :entity_id]
:skip []
:transform {:created_at (serdes/date)
:human_readable_field_id (serdes/fk :model/Field)
:field_id (serdes/parent-ref)}})
:field_id {::serdes/fk true
:export (constantly ::serdes/skip)
:import-with-context
(fn [current _ field-id]
(or field-id
(serdes/*import-field-fk*
(serdes/field-path->field-ref (pop (serdes/path current))))))}}})
(def ^:private dimension-slug "___dimension")
(defmethod serdes/storage-path "Dimension" [dimension _ctx]
(let [field-path (serdes/storage-path-prefixes (pop (serdes/path dimension)))]
(update field-path (dec (count field-path))
(fn [segment] (assoc segment :suffix dimension-slug)))))

26
src/metabase/warehouse_schema/models/field.clj
View File

@@ -106,12 +106,6 @@
{:in mi/json-in
:out (comp update-semantic-numeric-values mi/json-out-with-keywordization)})
(def ^:private transform-field-boolean
"Boolean column transform; a boolean computed in SQL (e.g. `COALESCE` over `json_unfolding`) comes back as a number
from MySQL."
{:in identity
:out (fn [v] (if (number? v) (pos? v) v))})
(def ^:private field-transforms
{:base_type transform-field-base-type
:effective_type transform-field-effective-type
@@ -123,7 +117,7 @@
:fingerprint transform-json-fingerprints
:settings mi/transform-json
:nfc_path mi/transform-json
:json_unfolding transform-field-boolean})
:json_unfolding mi/transform-boolean})
(t2/deftransforms :model/Field field-transforms)
@@ -461,13 +455,24 @@
[path]
(let [[table-path fields] (split-with #(not= "Field" (:model %)) path)
table (serdes/load-find-local table-path)]
(warehouse-schema.db/field-in-path (:id table) (map :id (reverse fields)))))
(when table
(warehouse-schema.db/field-in-path (:id table) (map :id (reverse fields))))))
(def ^:private legacy-dimensions
"The Dimensions a Field file carried before they got files of their own."
(serdes/nested :model/Dimension :field_id {}))
(defmethod serdes/load-one! "Field" [ingested maybe-local]
(let [field (serdes/default-load-one! ingested maybe-local)]
(when (contains? ingested :dimensions)
((:import-with-context legacy-dimensions) field :dimensions (:dimensions ingested)))
field))
(defmethod serdes/deserialization-dependencies "Field" [field]
(let [db-path (first (serdes/path field))]
#{[db-path]}))
(defmethod serdes/make-spec "Field" [_model-name opts]
(defmethod serdes/make-spec "Field" [_model-name _opts]
{:copy [:active :base_type :caveats :coercion_strategy :data_sensitivity :database_default :database_indexed
:database_is_auto_increment :database_is_generated :database_is_nullable :database_is_pk
:database_partitioned :database_position :database_required :database_type
@@ -478,8 +483,7 @@
:transform {:created_at (serdes/date)
:table_id (serdes/fk :model/Table)
:fk_target_field_id (serdes/fk :model/Field)
:parent_id (serdes/fk :model/Field)
:dimensions (serdes/nested :model/Dimension :field_id (merge {:sort-by (juxt :name :created_at)} opts))}
:parent_id (serdes/fk :model/Field)}
:defaults {:active true
:database_is_auto_increment false
:database_required false

29
src/metabase/warehouse_schema/models/field_user_settings.clj
View File

@@ -110,12 +110,18 @@
(conj (serdes/generate-path "Field" {:id field_id})
{:model "FieldUserSettings" :id "1"}))
(defn- field-path->field-ref [field-values-path]
(let [[db schema table field :as field-ref] (map :id (pop field-values-path))]
(if field
field-ref
;; It's too short, so no schema. Shift them over and add a nil schema.
[db nil schema table])))
(defmethod serdes/deserialization-dependencies "FieldUserSettings" [field-user-settings]
[[(first (serdes/path field-user-settings))]])
(defmethod serdes/load-find-local "FieldUserSettings" [path]
(when-let [field (serdes/load-find-local (pop path))]
(warehouse-schema.db/field-user-settings (:id field))))
(defmethod serdes/load-update! "FieldUserSettings" [model-name ingested local]
((get-method serdes/load-update! :default)
model-name
(merge (zipmap warehouse-schema-overlay/user-settable-field-columns (repeat nil)) ingested)
local))
(defmethod serdes/make-spec "FieldUserSettings" [_model-name _opts]
{:copy [:semantic_type :description :display_name :visibility_type
@@ -130,5 +136,12 @@
:field_id {::serdes/fk true
:export (constantly ::serdes/skip)
:import-with-context (fn [current _ _]
(let [field-ref (field-path->field-ref (serdes/path current))]
(serdes/*import-field-fk* field-ref)))}}})
(serdes/*import-field-fk*
(serdes/field-path->field-ref (pop (serdes/path current)))))}}})
(def ^:private field-user-settings-slug "___fieldusersettings")
(defmethod serdes/storage-path "FieldUserSettings" [field-user-settings _ctx]
(let [field-path (serdes/storage-path-prefixes (pop (serdes/path field-user-settings)))]
(update field-path (dec (count field-path))
(fn [segment] (assoc segment :suffix field-user-settings-slug)))))

2
src/metabase/warehouse_schema/models/field_values.clj
View File

@@ -741,4 +741,4 @@
(let [hierarchy (serdes/path fv)
field-path (serdes/storage-path-prefixes (drop-last hierarchy))]
(update field-path (dec (count field-path))
(fn [segment] (update segment :label str field-values-slug)))))
(fn [segment] (assoc segment :suffix field-values-slug)))))

19
src/metabase/warehouse_schema/models/table.clj
View File

@@ -126,16 +126,10 @@
:silver :final
:gold :final})
(def ^:private transform-table-boolean
"Boolean column transform; a boolean computed in SQL (the merge in `table-query`) comes back as a number from MySQL
and MariaDB, which have no boolean type of their own."
{:in identity
:out (fn [v] (if (number? v) (pos? v) v))})
(t2/deftransforms :model/Table
{:entity_type mi/transform-keyword
:is_published transform-table-boolean
:show_in_getting_started transform-table-boolean
:is_published mi/transform-boolean
:show_in_getting_started mi/transform-boolean
:visibility_type mi/transform-keyword
:data_layer (mi/transform-validator-with-fixes
mi/transform-keyword
@@ -574,14 +568,17 @@
(defmethod serdes/descendants "Table" [_model-name id {:keys [skip-archived]}]
(let [fields (into {} (for [field-id (warehouse-schema.db/field-ids-for-table id)]
[["Field" field-id] {"Table" id}]))
settings (when (or (warehouse-schema.db/table-user-settings-exist? id)
(warehouse-schema.db/field-user-settings-exist-for-table? id))
settings (when (warehouse-schema.db/table-user-settings-exist? id)
{["TableUserSettings" id] {"Table" id}})
field-settings (into {} (for [field-id (warehouse-schema.db/field-ids-with-user-settings-for-table id)]
[["FieldUserSettings" field-id] {"Table" id}]))
dimensions (into {} (for [dimension-id (warehouse-schema.db/dimension-ids-for-table id)]
[["Dimension" dimension-id] {"Table" id}]))
segments (into {} (for [segment-id (warehouse-schema.db/segment-ids-for-table id skip-archived)]
[["Segment" segment-id] {"Table" id}]))
measures (into {} (for [measure-id (warehouse-schema.db/measure-ids-for-table id skip-archived)]
[["Measure" measure-id] {"Table" id}]))]
(merge fields settings segments measures)))
(merge fields settings field-settings dimensions segments measures)))
(defmethod serdes/generate-path "Table" [_ table]
(let [db-name (warehouse-schema.db/database-name (:db_id table))]

38
src/metabase/warehouse_schema/models/table_user_settings.clj
View File

@@ -151,9 +151,8 @@
;;; ------------------------------------------------- Serialization -------------------------------------------------
(defmethod serdes/extract-query "TableUserSettings" [_model-name {:keys [filter-ids] :as opts}]
(serdes/extract-reducible-nested "TableUserSettings" (dissoc opts :filter-column :filter-ids)
(warehouse-schema.db/table-user-settings-with-field-settings filter-ids)))
(defmethod serdes/extract-query "TableUserSettings" [_model-name {:keys [filter-ids]}]
(warehouse-schema.db/table-user-settings-for-tables filter-ids))
(defmethod serdes/entity-id "TableUserSettings" [_ _] nil)
@@ -166,12 +165,6 @@
(cond-> [[db-path]]
(:collection_id tus) (conj [{:model "Collection" :id (:collection_id tus)}]))))
(defmethod serdes/load-one! "TableUserSettings" [ingested maybe-local]
(let [settings (serdes/default-load-one! ingested maybe-local)]
(when (:field_order ingested)
(table/update-field-positions! (warehouse-schema.db/table (:table_id settings))))
settings))
(defmethod serdes/load-find-local "TableUserSettings" [path]
(let [found-table (serdes/load-find-local (pop path))]
(warehouse-schema.db/table-user-settings (:id found-table))))
@@ -183,6 +176,21 @@
;; It's too short, so no schema. Shift them over and add a nil schema.
[db nil schema])))
(def ^:private legacy-fields
"The FieldUserSettings a settings file carried before they got files of their own."
(serdes/nested :model/FieldUserSettings :table_id
{:delete-children! warehouse-schema.db/delete-field-user-settings-for-table!}))
(defmethod serdes/load-one! "TableUserSettings" [ingested maybe-local]
(let [settings (serdes/default-load-one! ingested maybe-local)
table-id (serdes/*import-table-fk* (table-path->table-ref (serdes/path ingested)))]
(when (contains? ingested :fields)
((:import-with-context legacy-fields)
(t2/instance :model/TableUserSettings {:table_id table-id}) :fields (:fields ingested)))
(when (:field_order ingested)
(table/update-field-positions! (warehouse-schema.db/table table-id)))
settings))
(defmethod serdes/make-spec "TableUserSettings" [_model-name _opts]
{:copy [:display_name :description :entity_type :visibility_type :field_order :caveats :points_of_interest
:show_in_getting_started :data_authority :data_source :owner_email :is_published
@@ -201,17 +209,11 @@
:table_id {::serdes/fk true
:export (constantly ::serdes/skip)
:import-with-context (fn [current _ _]
(serdes/*import-table-fk* (table-path->table-ref (serdes/path current))))}
:fields (serdes/nested :model/FieldUserSettings :table_id
{:sort-by :field_name
:delete-children! warehouse-schema.db/delete-field-user-settings-for-table!})}})
(serdes/*import-table-fk* (table-path->table-ref (serdes/path current))))}}})
(def ^:private table-user-settings-slug "___tableusersettings")
(defmethod serdes/storage-path "TableUserSettings" [tus {:keys [inline-user-settings]}]
(defmethod serdes/storage-path "TableUserSettings" [tus _ctx]
(let [table-path (pop (serdes/path tus))]
(if inline-user-settings
(conj (serdes/storage-path-prefixes table-path)
{:label (:id (peek table-path)) :key (:id (peek table-path))})
(conj (serdes/storage-path-prefixes table-path)
{:label (str (:id (peek table-path)) table-user-settings-slug)}))))
{:label (:id (peek table-path)) :suffix table-user-settings-slug})))

4
src/metabase/warehouse_schema_rest/api/field.clj
View File

@@ -267,6 +267,8 @@
:type dimension-type
:name dimension-name
:human_readable_field_id human-readable-field-id})))
(events/publish-event! :event/field-update {:object (warehouse-schema-rest.db/field id)
:user-id api/*current-user-id*})
(warehouse-schema-rest.db/dimension-for-field id))
;; TODO (Cam 2025-11-25) please add a response schema to this API endpoint, it makes it easier for our customers to
@@ -279,6 +281,8 @@
[:id ms/PositiveInt]]]
(api/write-check :model/Field id)
(warehouse-schema-rest.db/delete-dimensions-for-field! id)
(events/publish-event! :event/field-update {:object (warehouse-schema-rest.db/field id)
:user-id api/*current-user-id*})
api/generic-204-no-content)
;; TODO (Cam 2025-11-25) please add a response schema to this API endpoint, it makes it easier for our customers to

13
test/metabase/models/serialization_test.clj
View File

@@ -395,3 +395,16 @@
clojure.lang.ExceptionInfo
#"Invalid input.*:template-tags"
(serdes/import-mbql query-with-unknown-tag-type))))))
(deftest ^:parallel field-path->field-ref-test
(testing "a Field path turns into the reference `*import-field-fk*` takes, with or without a schema"
(is (= ["db" "PUBLIC" "orders" "id"]
(serdes/field-path->field-ref [{:model "Database" :id "db"} {:model "Schema" :id "PUBLIC"}
{:model "Table" :id "orders"} {:model "Field" :id "id"}])))
(is (= ["db" nil "orders" "id"]
(serdes/field-path->field-ref [{:model "Database" :id "db"} {:model "Table" :id "orders"}
{:model "Field" :id "id"}]))))
(testing "a nested Field of a Table without a schema keeps the Table and every parent Field in place"
(is (= ["db" nil "orders" "customer" "tier"]
(serdes/field-path->field-ref [{:model "Database" :id "db"} {:model "Table" :id "orders"}
{:model "Field" :id "customer"} {:model "Field" :id "tier"}])))))

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/created_at.yaml
View File

@@ -10,7 +10,6 @@ database_type: TIMESTAMP WITH TIME ZONE
base_type: type/DateTimeWithLocalTZ
effective_type: type/DateTimeWithLocalTZ
semantic_type: type/CreationTimestamp
dimensions: []
position: 7
database_position: 7
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/discount.yaml
View File

@@ -10,7 +10,6 @@ database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
semantic_type: type/Discount
dimensions: []
position: 6
database_position: 6
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/id.yaml
View File

@@ -11,7 +11,6 @@ base_type: type/BigInteger
effective_type: type/BigInteger
semantic_type: type/PK
database_is_auto_increment: true
dimensions: []
position: 0
database_position: 0
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/product_id.yaml
View File

@@ -15,7 +15,6 @@ fk_target_field_id:
- PUBLIC
- PRODUCTS
- ID
dimensions: []
position: 2
database_position: 2
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/quantity.yaml
View File

@@ -10,7 +10,6 @@ database_type: INTEGER
base_type: type/Integer
effective_type: type/Integer
semantic_type: type/Quantity
dimensions: []
position: 8
database_position: 8
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/subtotal.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
dimensions: []
position: 3
database_position: 3
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/tax.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
dimensions: []
position: 4
database_position: 4
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/total.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
dimensions: []
position: 5
database_position: 5
serdes/meta:

13
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/total___fieldusersettings.yaml Normal file
View File

@@ -0,0 +1,13 @@
created_at: '2025-06-13T12:52:06.383265Z'
description: Total order amount including tax
serdes/meta:
- id: Sample Database
model: Database
- id: PUBLIC
model: Schema
- id: ORDERS
model: Table
- id: TOTAL
model: Field
- id: '1'
model: FieldUserSettings

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/user_id.yaml
View File

@@ -15,7 +15,6 @@ fk_target_field_id:
- PUBLIC
- PEOPLE
- ID
dimensions: []
position: 1
database_position: 1
serdes/meta:

20
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/fields/user_id___dimension.yaml Normal file
View File

@@ -0,0 +1,20 @@
created_at: '2025-06-13T12:52:06.383265Z'
entity_id: aMXK1q9eBB518aoCBQ6oo
human_readable_field_id:
- Sample Database
- PUBLIC
- PEOPLE
- NAME
name: User
type: external
serdes/meta:
- id: Sample Database
model: Database
- id: PUBLIC
model: Schema
- id: ORDERS
model: Table
- id: USER_ID
model: Field
- id: aMXK1q9eBB518aoCBQ6oo
model: Dimension

14
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/orders/orders___tableusersettings.yaml
View File

@@ -1,20 +1,6 @@
created_at: '2025-06-13T12:52:06.383265Z'
description: Every order placed, as the team describes it
display_name: Customer Orders
fields:
- created_at: '2025-06-13T12:52:06.383265Z'
description: Total order amount including tax
serdes/meta:
- id: Sample Database
model: Database
- id: PUBLIC
model: Schema
- id: ORDERS
model: Table
- id: TOTAL
model: Field
- id: '1'
model: FieldUserSettings
serdes/meta:
- id: Sample Database
model: Database

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/address.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
dimensions: []
position: 1
database_position: 1
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/birth_date.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: DATE
base_type: type/Date
effective_type: type/Date
dimensions: []
position: 9
database_position: 9
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/city.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/City
dimensions: []
position: 5
database_position: 5
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/created_at.yaml
View File

@@ -10,7 +10,6 @@ database_type: TIMESTAMP WITH TIME ZONE
base_type: type/DateTimeWithLocalTZ
effective_type: type/DateTimeWithLocalTZ
semantic_type: type/CreationTimestamp
dimensions: []
position: 12
database_position: 12
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/email.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/Email
dimensions: []
position: 2
database_position: 2
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/id.yaml
View File

@@ -11,7 +11,6 @@ base_type: type/BigInteger
effective_type: type/BigInteger
semantic_type: type/PK
database_is_auto_increment: true
dimensions: []
position: 0
database_position: 0
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/latitude.yaml
View File

@@ -10,7 +10,6 @@ database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
semantic_type: type/Latitude
dimensions: []
position: 11
database_position: 11
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/longitude.yaml
View File

@@ -10,7 +10,6 @@ database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
semantic_type: type/Longitude
dimensions: []
position: 6
database_position: 6
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/name.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/Name
dimensions: []
position: 4
database_position: 4
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/password.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
dimensions: []
position: 3
database_position: 3
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/source.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/Source
dimensions: []
position: 8
database_position: 8
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/state.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/State
dimensions: []
position: 7
database_position: 7
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/people/fields/zip.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
dimensions: []
position: 10
database_position: 10
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/category.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/Category
dimensions: []
position: 3
database_position: 3
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/created_at.yaml
View File

@@ -10,7 +10,6 @@ database_type: TIMESTAMP WITH TIME ZONE
base_type: type/DateTimeWithLocalTZ
effective_type: type/DateTimeWithLocalTZ
semantic_type: type/CreationTimestamp
dimensions: []
position: 7
database_position: 7
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/ean.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
dimensions: []
position: 1
database_position: 1
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/id.yaml
View File

@@ -11,7 +11,6 @@ base_type: type/BigInteger
effective_type: type/BigInteger
semantic_type: type/PK
database_is_auto_increment: true
dimensions: []
position: 0
database_position: 0
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/price.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
dimensions: []
position: 5
database_position: 5
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/rating.yaml
View File

@@ -10,7 +10,6 @@ database_type: DOUBLE PRECISION
base_type: type/Float
effective_type: type/Float
semantic_type: type/Score
dimensions: []
position: 6
database_position: 6
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/title.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/Title
dimensions: []
position: 2
database_position: 2
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/products/fields/vendor.yaml
View File

@@ -10,7 +10,6 @@ database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
semantic_type: type/Company
dimensions: []
position: 4
database_position: 4
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/reviews/fields/body.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
dimensions: []
preview_display: false
position: 4
database_position: 4

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/reviews/fields/created_at.yaml
View File

@@ -10,7 +10,6 @@ database_type: TIMESTAMP WITH TIME ZONE
base_type: type/DateTimeWithLocalTZ
effective_type: type/DateTimeWithLocalTZ
semantic_type: type/CreationTimestamp
dimensions: []
position: 5
database_position: 5
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/reviews/fields/id.yaml
View File

@@ -11,7 +11,6 @@ base_type: type/BigInteger
effective_type: type/BigInteger
semantic_type: type/PK
database_is_auto_increment: true
dimensions: []
position: 0
database_position: 0
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/reviews/fields/product_id.yaml
View File

@@ -15,7 +15,6 @@ fk_target_field_id:
- PUBLIC
- PRODUCTS
- ID
dimensions: []
position: 1
database_position: 1
serdes/meta:

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/reviews/fields/rating.yaml
View File

@@ -10,7 +10,6 @@ database_type: INTEGER
base_type: type/Integer
effective_type: type/Integer
semantic_type: type/Score
dimensions: []
position: 3
database_position: 3
has_field_values: auto-list

1
test_resources/serialization_baseline/databases/sample_database/schemas/public/tables/reviews/fields/reviewer.yaml
View File

@@ -9,7 +9,6 @@ table_id:
database_type: CHARACTER VARYING
base_type: type/Text
effective_type: type/Text
dimensions: []
position: 2
database_position: 2
serdes/meta: