Skip to content
Snippets Groups Projects
Unverified Commit 62fdd015 authored by Alexander Solovyov's avatar Alexander Solovyov Committed by GitHub
Browse files

[serdes] move Action to using spec (#46973)

parent f07084aa
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
+ 1
− 1
View file @ 62fdd015
......@@ -372,7 +372,7 @@
[key value]))
(yaml/from-file (io/file dump-dir "settings.yaml")))))))))))))
;; This is a seperate test instead of a `testing` block inside `e2e-storage-ingestion-test`
;; This is a separate test instead of a `testing` block inside `e2e-storage-ingestion-test`
;; because it's quite tricky to set up the generative test to generate parameters with source is card
(deftest card-and-dashboard-has-parameter-with-source-is-card-test
(testing "Dashboard and Card that has parameter with source is a card must be deserialized correctly"
......
enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+ 42
− 42
View file @ 62fdd015
......@@ -29,9 +29,7 @@
[metabase.models.serialization :as serdes]
[metabase.test :as mt]
[metabase.util :as u]
[toucan2.core :as t2])
(:import
(java.time OffsetDateTime)))
[toucan2.core :as t2]))
(defn- by-model [model-name extraction]
(->> extraction
......@@ -582,7 +580,7 @@
:dashcards [{:entity_id dc1-eid
:series (mt/exactly=? [{:card_id c2-eid :position 0}
{:card_id c3-eid :position 1}])}
{:entity_id dc2-eid, :series []}]}
{:entity_id dc2-eid}]}
ser))
(testing "and depend on all referenced cards, including cards from dashboard cards' series"
......@@ -845,9 +843,9 @@
(deftest implicit-action-test
(mt/with-empty-h2-app-db
(ts/with-temp-dpc [User {ann-id :id} {:first_name "Ann"
:last_name "Wilson"
:email "ann@heart.band"}
(ts/with-temp-dpc [User {ann-id :id} {:first_name "Ann"
:last_name "Wilson"
:email "ann@heart.band"}
Database {db-id :id :as db} {:name "My Database"}]
(mt/with-db db
(mt/with-actions [{card-id-1 :id
......@@ -860,20 +858,20 @@
:creator_id ann-id}
{:keys [action-id]}
{:name "My Action"
:type :implicit
:kind "row/update"
:creator_id ann-id
:model_id card-id-1}]
{:name "My Action"
:type :implicit
:kind "row/update"
:creator_id ann-id
:model_id card-id-1}]
(let [action (action/select-action :id action-id)]
(testing "implicit action"
(let [ser (serdes/extract-one "Action" {} action)]
(let [ser (ts/extract-one "Action" action-id)]
(is (=? {:serdes/meta [{:model "Action" :id (:entity_id action) :label "my_action"}]
:creator_id "ann@heart.band"
:type "implicit"
:kind "row/update"
:created_at OffsetDateTime
:model_id card-eid-1}
:type :implicit
:created_at string?
:model_id card-eid-1
:implicit [{:kind "row/update"}]}
ser))
(is (not (contains? ser :id)))
......@@ -883,9 +881,9 @@
(deftest http-action-test
(mt/with-empty-h2-app-db
(ts/with-temp-dpc [User {ann-id :id} {:first_name "Ann"
:last_name "Wilson"
:email "ann@heart.band"}
(ts/with-temp-dpc [User {ann-id :id} {:first_name "Ann"
:last_name "Wilson"
:email "ann@heart.band"}
Database {db-id :id :as db} {:name "My Database"}]
(mt/with-db db
(mt/with-actions [{card-id-1 :id
......@@ -898,20 +896,20 @@
:creator_id ann-id}
{:keys [action-id]}
{:name "My Action"
:type :http
:template {}
:creator_id ann-id
:model_id card-id-1}]
{:name "My Action"
:type :http
:template {}
:creator_id ann-id
:model_id card-id-1}]
(let [action (action/select-action :id action-id)]
(testing "action"
(let [ser (serdes/extract-one "Action" {} action)]
(let [ser (ts/extract-one "Action" action-id)]
(is (=? {:serdes/meta [{:model "Action" :id (:entity_id action) :label "my_action"}]
:creator_id "ann@heart.band"
:type "http"
:created_at OffsetDateTime
:template {}
:model_id card-eid-1}
:type :http
:created_at string?
:model_id card-eid-1
:http [{:template {}}]}
ser))
(is (not (contains? ser :id)))
......@@ -921,9 +919,9 @@
(deftest query-action-test
(mt/with-empty-h2-app-db
(ts/with-temp-dpc [User {ann-id :id} {:first_name "Ann"
:last_name "Wilson"
:email "ann@heart.band"}
(ts/with-temp-dpc [User {ann-id :id} {:first_name "Ann"
:last_name "Wilson"
:email "ann@heart.band"}
Database {db-id :id :as db} {:name "My Database"}]
(mt/with-db db
(mt/with-actions [{card-id-1 :id
......@@ -944,15 +942,17 @@
:model_id card-id-1}]
(let [action (action/select-action :id action-id)]
(testing "action"
(let [ser (serdes/extract-one "Action" {} action)]
(is (=? {:serdes/meta [{:model "Action"
:id (:entity_id action)
:label "my_action"}]
:type "query"
:creator_id "ann@heart.band"
:created_at OffsetDateTime
:dataset_query {:type "native", :native {:native "select 1"}, :database db-id}
:model_id card-eid-1}
(let [ser (ts/extract-one "Action" action-id)]
(is (=? {:serdes/meta [{:model "Action"
:id (:entity_id action)
:label "my_action"}]
:type :query
:creator_id "ann@heart.band"
:created_at string?
:query [{:dataset_query {:database "My Database"
:type "native"
:native {:native "select 1"}}}]
:model_id card-eid-1}
ser))
(is (not (contains? ser :id)))
......
enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
+ 7
− 6
View file @ 62fdd015
......@@ -8,7 +8,7 @@
[metabase-enterprise.serialization.v2.ingest :as serdes.ingest]
[metabase-enterprise.serialization.v2.load :as serdes.load]
[metabase.models
:refer [Action Card Collection Dashboard DashboardCard Database Field
:refer [Card Collection Dashboard DashboardCard Database Field
FieldValues NativeQuerySnippet Segment Table Timeline
TimelineEvent User]]
[metabase.models.action :as action]
......@@ -962,13 +962,13 @@
:type :model
:database_id (:id db)
:dataset_query {:database (:id db)
:native {:type :native
:native {:query "wow"}}})
:type :native
:native {:query "select 1"}})
_action-id (action/insert! {:entity_id eid
:name "the action"
:model_id (:id card)
:type :query
:dataset_query "wow"
:dataset_query (mt/mbql-query users {:limit 1})
:database_id (:id db)})]
(reset! serialized (into [] (serdes.extract/extract {})))
(let [action-serialized (first (filter (fn [{[{:keys [model id]}] :serdes/meta}]
......@@ -976,12 +976,13 @@
@serialized))]
(is (some? action-serialized))
(testing ":type should be a string"
(is (string? (:type action-serialized))))))))
(is (keyword? (:type action-serialized))))))))
(testing "loading succeeds"
(ts/with-db dest-db
(serdes.load/load-metabase! (ingestion-in-memory @serialized))
(let [action (t2/select-one Action :entity_id eid)]
(let [action (action/select-action :entity_id eid)]
(is (some? action))
(is (some? (:dataset_query action)))
(testing ":type should be a keyword again"
(is (keyword? (:type action))))))))))
......
src/metabase/models/action.clj
+ 37
− 42
View file @ 62fdd015
......@@ -8,7 +8,6 @@
[metabase.models.serialization :as serdes]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
[metabase.util.log :as log]
[methodical.core :as methodical]
[toucan2.core :as t2]))
......@@ -153,12 +152,6 @@
(t2/insert! new-model (assoc type-row :action_id id)))
(t2/update! existing-model id type-row)))))
(defn- hydrate-subtype [action]
(let [subtype (type->model (:type action))]
(-> action
(merge (t2/select-one subtype :action_id (:id action)))
(dissoc :action_id))))
(defn- normalize-query-actions [actions]
(when (seq actions)
(let [query-actions (t2/select QueryAction :action_id [:in (map :id actions)])
......@@ -352,46 +345,48 @@
;;; ------------------------------------------------ Serialization ---------------------------------------------------
(defmethod serdes/extract-query "Action" [_model opts]
(eduction (map hydrate-subtype)
(t2/reducible-select Action {:where (or (:where opts) true)})))
(defmethod serdes/hash-fields :model/Action [_action]
[:name (serdes/hydrated-hash :model) :created_at])
(defmethod serdes/extract-one "Action" [_model-name _opts action]
(-> (serdes/extract-one-basics "Action" action)
(update :creator_id serdes/*export-user*)
(update :model_id serdes/*export-fk* 'Card)
(update :type name)
(cond-> (= (:type action) :query)
(update :database_id serdes/*export-fk-keyed* 'Database :name))))
(defmethod serdes/load-xform "Action" [action]
(-> action
serdes/load-xform-basics
(update :creator_id serdes/*import-user*)
(update :model_id serdes/*import-fk* 'Card)
(update :type keyword)
(cond-> (= (:type action) "query")
(update :database_id serdes/*import-fk-keyed* 'Database :name))))
(defmethod serdes/ingested-model-columns "Action" [_ingested]
(into #{} (conj action-columns :database_id :dataset_query :kind :template :response_handle :error_handle :type)))
(defmethod serdes/load-update! "Action" [_model-name ingested local]
(log/tracef "Upserting Action %d: old %s new %s" (:id local) (pr-str local) (pr-str ingested))
(update! (assoc ingested :id (:id local)) local)
(select-action :id (:id local)))
(defmethod serdes/load-insert! "Action" [_model-name ingested]
(log/tracef "Inserting Action: %s" (pr-str ingested))
(insert! ingested))
(defmethod serdes/generate-path "QueryAction" [_ _] nil)
(defmethod serdes/make-spec "QueryAction" [_model-name _opts]
{:copy []
:transform {:action_id (serdes/parent-ref)
:database_id (serdes/fk :model/Database :name)
:dataset_query {:export serdes/export-mbql :import serdes/import-mbql}}})
(defmethod serdes/generate-path "HTTPAction" [_ _] nil)
(defmethod serdes/make-spec "HTTPAction" [_model-name _opts]
{:copy [:error_handle :response_handle :template]
:transform {:action_id (serdes/parent-ref)}})
(defmethod serdes/generate-path "ImplicitAction" [_ _] nil)
(defmethod serdes/make-spec "ImplicitAction" [_model-name _opts]
{:copy [:kind]
:transform {:action_id (serdes/parent-ref)}})
(defmethod serdes/make-spec "Action" [_model-name opts]
{:copy [:archived :description :entity_id :name :public_uuid :type]
:skip []
:transform {:created_at (serdes/date)
:creator_id (serdes/fk :model/User)
:made_public_by_id (serdes/fk :model/User)
:model_id (serdes/fk :model/Card)
:query (serdes/nested :model/QueryAction :action_id opts)
:http (serdes/nested :model/HTTPAction :action_id opts)
:implicit (serdes/nested :model/ImplicitAction :action_id opts)
:parameters {:export serdes/export-parameters :import serdes/import-parameters}
:parameter_mappings {:export serdes/export-parameter-mappings
:import serdes/import-parameter-mappings}
:visualization_settings {:export serdes/export-visualization-settings
:import serdes/import-visualization-settings}}})
(defmethod serdes/dependencies "Action" [action]
(concat [[{:model "Card" :id (:model_id action)}]]
(when (= (:type action) "query")
[[{:model "Database" :id (:database_id action)}]])))
(concat
;; other stuff is implicitly referenced through a Card
[[{:model "Card" :id (:model_id action)}]]
(when (= (:type action) :query)
[[{:model "Database" :id (-> action :query first :database_id)}]])))
(defmethod serdes/storage-path "Action" [action _ctx]
(let [{:keys [id label]} (-> action serdes/path last)]
......
src/metabase/models/serialization.clj
+ 3
− 2
View file @ 62fdd015
......@@ -1575,8 +1575,9 @@
;; `nil? data` check is for `extract-one` case in tests; make sure to add empty vectors in
;; `extract-query` implementations for nested collections
(try
(->> (sort-by sorter data)
(mapv #(extract-one model-name opts %)))
(when (seq data)
(->> (sort-by sorter data)
(mapv #(extract-one model-name opts %))))
(catch Exception e
(throw (ex-info (format "Error exporting nested %s" model)
{:model model
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment