From f4d3ea6cc40b93266896f0871c253ec39baa6cd1 Mon Sep 17 00:00:00 2001 From: Braden Shepherdson <braden@metabase.com> Date: Fri, 7 Oct 2022 09:32:31 -0400 Subject: [PATCH] Serdes v2: Drop the updated_at field from serialization (#25446) The value is reconstructed on the deserialization side based on the time of deserialization. For the git workflow, `updated_at` makes for a lot of diffs of unrelated files. --- .../serialization/v2/e2e/yaml_test.clj | 84 ++------ .../serialization/v2/extract_test.clj | 202 ++++++++---------- .../serialization/v2/load_test.clj | 4 +- src/metabase/models/pulse_channel.clj | 6 +- src/metabase/models/serialization/base.clj | 30 ++- 5 files changed, 144 insertions(+), 182 deletions(-) diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj index 6bb240e5cf5..f827448b27e 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e/yaml_test.clj @@ -30,6 +30,11 @@ (defn- strip-labels [path] (mapv #(dissoc % :label) path)) +(defn- clean-up-expected [entity] + (cond-> entity + true (dissoc :serdes/meta :updated_at) + (:created_at entity) (update :created_at u.date/format))) + (defn- random-keyword ([prefix n] (random-keyword prefix n 0)) ([prefix n floor] (keyword (str prefix (+ floor (rand-int n)))))) @@ -100,17 +105,14 @@ (is (= 110 (count (dir->file-set (io/file dump-dir "Collection"))))) (doseq [{:keys [entity_id slug] :as coll} (get entities "Collection") :let [filename (#'u.yaml/leaf-file-name entity_id slug)]] - (is (= (dissoc coll :serdes/meta) + (is (= (clean-up-expected coll) (yaml/from-file (io/file dump-dir "Collection" filename)))))) (testing "for Databases" (is (= 10 (count (dir->file-set (io/file dump-dir "Database"))))) (doseq [{:keys [name] :as coll} (get entities "Database") :let [filename (#'u.yaml/leaf-file-name name)]] - (is (= (-> coll - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected coll) (yaml/from-file (io/file dump-dir "Database" filename)))))) (testing "for Tables" @@ -121,10 +123,7 @@ "Tables are scattered, so the directories are harder to count") (doseq [{:keys [db_id name] :as coll} (get entities "Table")] - (is (= (-> coll - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected coll) (yaml/from-file (io/file dump-dir "Database" db_id "Table" (str name ".yaml"))))))) (testing "for Fields" @@ -138,30 +137,21 @@ (doseq [{[db schema table] :table_id name :name :as coll} (get entities "Field")] (is (nil? schema)) - (is (= (-> coll - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected coll) (yaml/from-file (io/file dump-dir "Database" db "Table" table "Field" (str name ".yaml"))))))) (testing "for cards" (is (= 100 (count (dir->file-set (io/file dump-dir "Card"))))) (doseq [{:keys [entity_id] :as card} (get entities "Card") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> card - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected card) (yaml/from-file (io/file dump-dir "Card" filename)))))) (testing "for dashboards" (is (= 100 (count (dir->file-set (io/file dump-dir "Dashboard"))))) (doseq [{:keys [entity_id] :as dash} (get entities "Dashboard") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> dash - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected dash) (yaml/from-file (io/file dump-dir "Dashboard" filename)))))) (testing "for dashboard cards" @@ -175,50 +165,35 @@ (doseq [{:keys [dashboard_id entity_id] :as dashcard} (get entities "DashboardCard") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> dashcard - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected dashcard) (yaml/from-file (io/file dump-dir "Dashboard" dashboard_id "DashboardCard" filename)))))) (testing "for dimensions" (is (= 40 (count (dir->file-set (io/file dump-dir "Dimension"))))) (doseq [{:keys [entity_id] :as dim} (get entities "Dimension") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> dim - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected dim) (yaml/from-file (io/file dump-dir "Dimension" filename)))))) (testing "for metrics" (is (= 30 (count (dir->file-set (io/file dump-dir "Metric"))))) (doseq [{:keys [entity_id name] :as metric} (get entities "Metric") :let [filename (#'u.yaml/leaf-file-name entity_id name)]] - (is (= (-> metric - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected metric) (yaml/from-file (io/file dump-dir "Metric" filename)))))) (testing "for segments" (is (= 30 (count (dir->file-set (io/file dump-dir "Segment"))))) (doseq [{:keys [entity_id name] :as segment} (get entities "Segment") :let [filename (#'u.yaml/leaf-file-name entity_id name)]] - (is (= (-> segment - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected segment) (yaml/from-file (io/file dump-dir "Segment" filename)))))) (testing "for pulses" (is (= 30 (count (dir->file-set (io/file dump-dir "Pulse"))))) (doseq [{:keys [entity_id] :as pulse} (get entities "Pulse") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> pulse - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected pulse) (yaml/from-file (io/file dump-dir "Pulse" filename)))))) (testing "for pulse cards" @@ -228,8 +203,7 @@ count))))) (doseq [{:keys [entity_id pulse_id] :as card} (get entities "PulseCard") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> card - (dissoc :serdes/meta)) + (is (= (clean-up-expected card) (yaml/from-file (io/file dump-dir "Pulse" pulse_id "PulseCard" filename)))))) (testing "for pulse channels" @@ -241,30 +215,21 @@ (count recipients))))) (doseq [{:keys [entity_id pulse_id] :as channel} (get entities "PulseChannel") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> channel - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected channel) (yaml/from-file (io/file dump-dir "Pulse" pulse_id "PulseChannel" filename)))))) (testing "for native query snippets" (is (= 10 (count (dir->file-set (io/file dump-dir "NativeQuerySnippet"))))) (doseq [{:keys [entity_id name] :as snippet} (get entities "NativeQuerySnippet") :let [filename (#'u.yaml/leaf-file-name entity_id name)]] - (is (= (-> snippet - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected snippet) (yaml/from-file (io/file dump-dir "NativeQuerySnippet" filename)))))) (testing "for timelines and events" (is (= 10 (count (dir->file-set (io/file dump-dir "Timeline"))))) (doseq [{:keys [entity_id] :as timeline} (get entities "Timeline") :let [filename (#'u.yaml/leaf-file-name entity_id)]] - (is (= (-> timeline - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected timeline) (yaml/from-file (io/file dump-dir "Timeline" filename))))) (is (= 90 (reduce + (for [timeline (get entities "Timeline")] @@ -273,10 +238,7 @@ count))))) (doseq [{:keys [name timeline_id timestamp] :as event} (get entities "TimelineEvent") :let [filename (#'u.yaml/leaf-file-name timestamp name)]] - (is (= (-> event - (dissoc :serdes/meta) - (update :created_at u.date/format) - (update :updated_at u.date/format)) + (is (= (clean-up-expected event) (yaml/from-file (io/file dump-dir "Timeline" timeline_id "TimelineEvent" filename)))))) (testing "for settings" @@ -301,8 +263,8 @@ (let [->utc #(t/zoned-date-time % (ZoneId/of "UTC"))] (is (= (cond-> entity true (update :serdes/meta strip-labels) + true (dissoc :updated_at) ;; TIMESTAMP WITH TIME ZONE columns come out of the database as OffsetDateTime, but read back ;; from YAML as ZonedDateTimes; coerce the expected value to match. - (t/offset-date-time? (:created_at entity)) (update :created_at ->utc) - (t/offset-date-time? (:updated_at entity)) (update :updated_at ->utc)) + (t/offset-date-time? (:created_at entity)) (update :created_at ->utc)) (ingest/ingest-one ingestable (serdes.base/serdes-path entity))))))))))))) diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj index cd743cb4438..4d819af37bb 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj @@ -229,7 +229,6 @@ :aggregation [[:count]]} :database "My Database"}) :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -245,18 +244,17 @@ (set (serdes.base/serdes-dependencies ser)))))) (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c2-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c2-eid}]) - :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) - :creator_id (s/eq "mark@direstrai.ts") - :collection_id (s/eq coll-eid) - :dataset_query (s/eq {}) - :parameter_mappings (s/eq [{:parameter_id "deadbeef" - :card_id c1-eid - :target [:dimension [:field ["My Database" nil "Schemaless Table" "Some Field"] - {:source-field ["My Database" "PUBLIC" "Schema'd Table" "Other Field"]}]]}]) - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c2-eid}]) + :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) + :creator_id (s/eq "mark@direstrai.ts") + :collection_id (s/eq coll-eid) + :dataset_query (s/eq {}) + :parameter_mappings (s/eq [{:parameter_id "deadbeef" + :card_id c1-eid + :target [:dimension [:field ["My Database" nil "Schemaless Table" "Some Field"] + {:source-field ["My Database" "PUBLIC" "Schema'd Table" "Other Field"]}]]}]) + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -302,8 +300,7 @@ :enabled true}] :column_settings {"[\"ref\",[\"field\",[\"My Database\",\"PUBLIC\",\"Schema'd Table\",\"Other Field\"],null]]" {:column_title "Locus"}}}) - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime + :created_at LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -324,14 +321,15 @@ (set (serdes.base/serdes-dependencies ser)))))) (let [ser (serdes.base/extract-one "DashboardCard" {} (select-one "DashboardCard" [:= :id dc1-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Dashboard" :id dash-eid} - {:model "DashboardCard" :id dc1-eid}]) - :dashboard_id (s/eq dash-eid) - :parameter_mappings (s/eq [{:parameter_id "12345678" - :card_id c1-eid - :target [:dimension [:field ["My Database" nil "Schemaless Table" "Some Field"] - {:source-field ["My Database" "PUBLIC" "Schema'd Table" "Other Field"]}]]}]) - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Dashboard" :id dash-eid} + {:model "DashboardCard" :id dc1-eid}]) + :dashboard_id (s/eq dash-eid) + :parameter_mappings (s/eq [{:parameter_id "12345678" + :card_id c1-eid + :target [:dimension [:field ["My Database" nil "Schemaless Table" "Some Field"] + {:source-field ["My Database" "PUBLIC" "Schema'd Table" "Other Field"]}]]}]) + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -349,16 +347,15 @@ (testing "Cards can be based on other cards" (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c5-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c5-eid}]) - :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) - :creator_id (s/eq "mark@direstrai.ts") - :collection_id (s/eq coll-eid) - :dataset_query (s/eq {:query {:source-table c4-eid - :aggregation [[:count]]} - :database "My Database"}) - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c5-eid}]) + :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"]) + :creator_id (s/eq "mark@direstrai.ts") + :collection_id (s/eq coll-eid) + :dataset_query (s/eq {:query {:source-table c4-eid + :aggregation [[:count]]} + :database "My Database"}) + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -393,7 +390,8 @@ :enabled true}] :column_settings {"[\"ref\",[\"field\",[\"My Database\",\"PUBLIC\",\"Schema'd Table\",\"Other Field\"],null]]" {:column_title "Locus"}}}) - s/Keyword s/Any} + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -471,6 +469,7 @@ (is (schema= {:serdes/meta (s/eq [{:model "Dimension" :id dim1-eid}]) :field_id (s/eq ["My Database" nil "Schemaless Table" "email"]) :human_readable_field_id (s/eq nil) + :created_at LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -486,6 +485,7 @@ (is (schema= {:serdes/meta (s/eq [{:model "Dimension" :id dim2-eid}]) :field_id (s/eq ["My Database" "PUBLIC" "Schema'd Table" "foreign_id"]) :human_readable_field_id (s/eq ["My Database" "PUBLIC" "Foreign Table" "real_field"]) + :created_at LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -518,16 +518,15 @@ :aggregation [[:sum [:field field-id nil]]]}}]] (testing "metrics" (let [ser (serdes.base/extract-one "Metric" {} (select-one "Metric" [:= :id m1-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Metric" :id m1-eid :label "My Metric"}]) - :table_id (s/eq ["My Database" nil "Schemaless Table"]) - :creator_id (s/eq "ann@heart.band") - :definition (s/eq {:source-table ["My Database" nil "Schemaless Table"] - :aggregation - [[:sum [:field ["My Database" nil - "Schemaless Table" "Some Field"] nil]]]}) - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Metric" :id m1-eid :label "My Metric"}]) + :table_id (s/eq ["My Database" nil "Schemaless Table"]) + :creator_id (s/eq "ann@heart.band") + :definition (s/eq {:source-table ["My Database" nil "Schemaless Table"] + :aggregation + [[:sum [:field ["My Database" nil + "Schemaless Table" "Some Field"] nil]]]}) + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -559,14 +558,13 @@ (testing "native query snippets" (testing "can belong to :snippets collections" (let [ser (serdes.base/extract-one "NativeQuerySnippet" {} (select-one "NativeQuerySnippet" [:= :id s1-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "NativeQuerySnippet" - :id s1-eid - :label "Snippet 1"}]) - :collection_id (s/eq coll-eid) - :creator_id (s/eq "ann@heart.band") - :created_at OffsetDateTime - (s/optional-key :updated_at) OffsetDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "NativeQuerySnippet" + :id s1-eid + :label "Snippet 1"}]) + :collection_id (s/eq coll-eid) + :creator_id (s/eq "ann@heart.band") + :created_at OffsetDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -582,7 +580,6 @@ (s/optional-key :collection_id) (s/eq nil) :creator_id (s/eq "ann@heart.band") :created_at OffsetDateTime - (s/optional-key :updated_at) OffsetDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -613,12 +610,11 @@ (testing "timelines" (testing "with no events" (let [ser (serdes.base/extract-one "Timeline" {} (select-one "Timeline" [:= :id empty-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id empty-eid}]) - :collection_id (s/eq coll-eid) - :creator_id (s/eq "ann@heart.band") - :created_at OffsetDateTime - (s/optional-key :updated_at) OffsetDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id empty-eid}]) + :collection_id (s/eq coll-eid) + :creator_id (s/eq "ann@heart.band") + :created_at OffsetDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -628,12 +624,11 @@ (testing "with events" (let [ser (serdes.base/extract-one "Timeline" {} (select-one "Timeline" [:= :id line-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id line-eid}]) - :collection_id (s/eq coll-eid) - :creator_id (s/eq "ann@heart.band") - :created_at OffsetDateTime - (s/optional-key :updated_at) OffsetDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id line-eid}]) + :collection_id (s/eq coll-eid) + :creator_id (s/eq "ann@heart.band") + :created_at OffsetDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -644,16 +639,15 @@ (testing "timeline events" (let [ser (serdes.base/extract-one "TimelineEvent" {} (select-one "TimelineEvent" [:= :id e1-id])) stamp "2020-04-11T00:00:00Z"] - (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id line-eid} - {:model "TimelineEvent" - :id stamp - :label "First Event"}]) - :timestamp (s/eq stamp) - :timeline_id (s/eq line-eid) - :creator_id (s/eq "ann@heart.band") - :created_at OffsetDateTime - (s/optional-key :updated_at) OffsetDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Timeline" :id line-eid} + {:model "TimelineEvent" + :id stamp + :label "First Event"}]) + :timestamp (s/eq stamp) + :timeline_id (s/eq line-eid) + :creator_id (s/eq "ann@heart.band") + :created_at OffsetDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -678,17 +672,16 @@ :filter [:< [:field field-id nil] 18]}}]] (testing "segment" (let [ser (serdes.base/extract-one "Segment" {} (select-one "Segment" [:= :id s1-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Segment" :id s1-eid :label "My Segment"}]) - :table_id (s/eq ["My Database" nil "Schemaless Table"]) - :creator_id (s/eq "ann@heart.band") - :created_at LocalDateTime - :definition (s/eq {:source-table ["My Database" nil "Schemaless Table"] - :aggregation [[:count]] - :filter ["<" [:field ["My Database" nil - "Schemaless Table" "Some Field"] - nil] 18]}) - (s/optional-key :updated_at) LocalDateTime - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Segment" :id s1-eid :label "My Segment"}]) + :table_id (s/eq ["My Database" nil "Schemaless Table"]) + :creator_id (s/eq "ann@heart.band") + :definition (s/eq {:source-table ["My Database" nil "Schemaless Table"] + :aggregation [[:count]] + :filter ["<" [:field ["My Database" nil + "Schemaless Table" "Some Field"] + nil] 18]}) + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -724,14 +717,13 @@ "Seafood" "Steakhouse" "Tea Room" "Winery"]}]] (testing "field values" (let [ser (serdes.base/extract-one "FieldValues" {} (select-one "FieldValues" [:= :id fv-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Database" :id "My Database"} - {:model "Table" :id "Schemaless Table"} - {:model "Field" :id "Some Field"} - {:model "FieldValues" :id "0"}]) ; Always 0. - :created_at LocalDateTime - (s/optional-key :updated_at) OffsetDateTime - :values (s/eq (json/generate-string values)) - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Database" :id "My Database"} + {:model "Table" :id "Schemaless Table"} + {:model "Field" :id "Some Field"} + {:model "FieldValues" :id "0"}]) ; Always 0. + :created_at LocalDateTime + :values (s/eq (json/generate-string values)) + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) (is (not (contains? ser :field_id)) @@ -772,10 +764,9 @@ (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-none-id]))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-none-eid}]) :creator_id (s/eq "ann@heart.band") - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime (s/optional-key :dashboard_id) (s/eq nil) (s/optional-key :collection_id) (s/eq nil) + :created_at LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -788,10 +779,9 @@ (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-coll-id]))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-coll-eid}]) :creator_id (s/eq "ann@heart.band") - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime (s/optional-key :dashboard_id) (s/eq nil) :collection_id (s/eq coll-eid) + :created_at LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -804,10 +794,9 @@ (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-dash-id]))] (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-dash-eid}]) :creator_id (s/eq "ann@heart.band") - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime :dashboard_id (s/eq dash-eid) (s/optional-key :collection_id) (s/eq nil) + :created_at LocalDateTime s/Keyword s/Any} ser)) (is (not (contains? ser :id))) @@ -818,13 +807,12 @@ (testing "pulse with both collection and dashboard" (let [ser (serdes.base/extract-one "Pulse" {} (select-one "Pulse" [:= :id p-both-id]))] - (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-both-eid}]) - :creator_id (s/eq "ann@heart.band") - :created_at LocalDateTime - (s/optional-key :updated_at) LocalDateTime - :dashboard_id (s/eq dash-eid) - :collection_id (s/eq coll-eid) - s/Keyword s/Any} + (is (schema= {:serdes/meta (s/eq [{:model "Pulse" :id p-both-eid}]) + :creator_id (s/eq "ann@heart.band") + :dashboard_id (s/eq dash-eid) + :collection_id (s/eq coll-eid) + :created_at LocalDateTime + s/Keyword s/Any} ser)) (is (not (contains? ser :id))) diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj index d2fbfedeaff..e0fdc1cb9dc 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj @@ -819,5 +819,5 @@ (testing "existing FieldValues are properly found and updated" (is (= (:values @fv1s) (:values @fv1d)))) (testing "new FieldValues are properly added" - (is (= (dissoc @fv2s :id :field_id) - (dissoc @fv2d :id :field_id))))))))) + (is (= (dissoc @fv2s :id :field_id :created_at :updated_at) + (dissoc @fv2d :id :field_id :created_at :updated_at))))))))) diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj index d3d436b9042..c27798ba77e 100644 --- a/src/metabase/models/pulse_channel.clj +++ b/src/metabase/models/pulse_channel.clj @@ -383,11 +383,13 @@ ;; Customized load-insert! and load-update! to handle the embedded recipients field - it's really a separate table. (defmethod serdes.base/load-insert! "PulseChannel" [_ ingested] - (let [id (db/simple-insert! PulseChannel (dissoc ingested :recipients))] + (let [;; Call through to the default load-insert! + id ((get-method serdes.base/load-insert! "") "PulseChannel" (dissoc ingested :recipients))] (import-recipients id (:recipients ingested)))) (defmethod serdes.base/load-update! "PulseChannel" [_ ingested local] - (db/update! PulseChannel {:where [:= :id (:id local)] :set (dissoc ingested :recipients)}) + ;; Call through to the default load-update! + ((get-method serdes.base/load-update! "") "PulseChannel" (dissoc ingested :recipients) local) (import-recipients (:id local) (:recipients ingested)) (:id local)) diff --git a/src/metabase/models/serialization/base.clj b/src/metabase/models/serialization/base.clj index 728e81efad2..e06a2381ebf 100644 --- a/src/metabase/models/serialization/base.clj +++ b/src/metabase/models/serialization/base.clj @@ -10,6 +10,7 @@ If the model is not exported, add it to the exclusion lists in the tests. Every model should be explicitly listed as exported or not, and a test enforces this so serialization isn't forgotten for new models." (:require [clojure.tools.logging :as log] + [metabase.models.interface :as mi] [metabase.models.serialization.hash :as serdes.hash] [toucan.db :as db] [toucan.models :as models])) @@ -231,7 +232,7 @@ - Convert to a vanilla Clojure map. - Add `:serdes/meta` by calling [[serdes-generate-path]]. - Drop the primary key. - - Making :created_at and :updated_at into UTC-based LocalDateTimes. + - Drop :updated_at; it's noisy in git and not really used anywhere. Returns the Clojure map." [model-name entity] @@ -239,7 +240,7 @@ pk (models/primary-key model)] (-> entity (assoc :serdes/meta (serdes-generate-path model-name entity)) - (dissoc pk)))) + (dissoc pk :updated_at)))) (defmethod extract-one :default [model-name _opts entity] (extract-one-basics model-name entity)) @@ -376,14 +377,18 @@ (fn [model _ _] model)) (defmethod load-update! :default [model-name ingested local] - (let [model (db/resolve-model (symbol model-name)) - pk (models/primary-key model) - id (get local pk)] + (let [model (db/resolve-model (symbol model-name)) + pk (models/primary-key model) + id (get local pk) + adjusted (if (-> model models/properties :timestamped?) + (assoc ingested :updated_at (mi/now)) + ingested)] (log/tracef "Upserting %s %d: old %s new %s" model-name id (pr-str local) (pr-str ingested)) - ; Using the two-argument form of [[db/update!]] that takes the model and a HoneySQL form for the actual update. - ; It works differently from the more typical `(db/update! 'Model id updates...)` form: this form doesn't run any of - ; the pre-update magic, it just updates the database directly. - (db/update! (symbol model-name) {:where [:= pk id] :set ingested}) + ;; Using the two-argument form of [[db/update!]] that takes the model and a HoneySQL form for the actual update. + ;; It works differently from the more typical `(db/update! 'Model id updates...)` form: this form doesn't run any of + ;; the pre-update magic, it just updates the database directly. + ;; Therefore we manually set the :updated_at time. + (db/update! model {:where [:= pk id] :set adjusted}) id)) (defmulti load-insert! @@ -404,7 +409,12 @@ (log/tracef "Inserting %s: %s" model (pr-str ingested)) ; Toucan's simple-insert! actually does the right thing for our purposes: it doesn't call pre-insert or post-insert, ; and it returns the new primary key. - (db/simple-insert! (symbol model) ingested)) + (let [model (db/resolve-model (symbol model)) + adjusted (if (-> model models/properties :timestamped?) + (let [now (mi/now)] + (assoc ingested :created_at now :updated_at now)) + ingested)] + (db/simple-insert! model adjusted))) (defmulti load-one! "Black box for integrating a deserialized entity into this appdb. -- GitLab