From 80687b8a420b8cd3e58294b8e2cdf96566f99f06 Mon Sep 17 00:00:00 2001
From: Alexander Solovyov <alexander@solovyov.net>
Date: Mon, 27 May 2024 11:19:06 +0300
Subject: [PATCH] [serdes] nested fields should be properly serialized (#43066)
---
.../serialization/v2/extract_test.clj | 27 ++++++++++++-------
.../serialization/v2/load_test.clj | 24 ++++++++++++-----
src/metabase/models/field.clj | 4 ++-
3 files changed, 39 insertions(+), 16 deletions(-)
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 461403fd6d8..ada774d8a6c 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
@@ -1658,24 +1658,33 @@
(map serdes/path)
set)))))))))
-(deftest foreign-key-field-test
+(deftest field-references-test
(mt/with-empty-h2-app-db
- (ts/with-temp-dpc [Database {db-id :id} {:name "My Database"}
- Table {no-schema-id :id} {:name "Schemaless Table" :db_id db-id}
- Field {some-field-id :id} {:name "Some Field" :table_id no-schema-id}
- Table {schema-id :id} {:name "Schema'd Table"
+ (ts/with-temp-dpc [Database {db-id :id} {:name "My Database"}
+ Table {no-schema-id :id} {:name "Schemaless Table" :db_id db-id}
+ Field {some-field-id :id} {:name "Some Field" :table_id no-schema-id}
+ Table {schema-id :id} {:name "Schema'd Table"
:db_id db-id
:schema "PUBLIC"}
- Field _ {:name "Other Field" :table_id schema-id}
- Field {fk-id :id} {:name "Foreign Key"
+ Field {other-field-id :id} {:name "Other Field" :table_id schema-id}
+ Field {fk-id :id} {:name "Foreign Key"
:table_id schema-id
- :fk_target_field_id some-field-id}]
+ :fk_target_field_id some-field-id}
+ Field {nested-id :id} {:name "Nested Field"
+ :table_id schema-id
+ :parent_id other-field-id}]
(testing "fields that reference foreign keys are properly exported as Field references"
(is (= ["My Database" nil "Schemaless Table" "Some Field"]
(->> (t2/select-one Field :id fk-id)
(serdes/extract-one "Field" {})
- :fk_target_field_id)))))))
+ :fk_target_field_id))))
+
+ (testing "Fields that reference parents are properly exported as Field references"
+ (is (= ["My Database" "PUBLIC" "Schema'd Table" "Other Field"]
+ (->> (t2/select-one Field :id nested-id)
+ (serdes/extract-one "Field" {})
+ :parent_id)))))))
(deftest escape-report-test
(mt/with-empty-h2-app-db
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 d5afe756b73..800452df524 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
@@ -126,7 +126,8 @@
t1s (atom nil)
t2s (atom nil)
f1s (atom nil)
- f2s (atom nil)]
+ f2s (atom nil)
+ f3s (atom nil)]
(ts/with-dbs [source-db dest-db]
(testing "serializing the two databases"
(ts/with-db source-db
@@ -136,6 +137,7 @@
(reset! t2s (ts/create! Table :name "posts" :db_id (:id @db2s))) ; Deliberately the same name!
(reset! f1s (ts/create! Field :name "Target Field" :table_id (:id @t1s)))
(reset! f2s (ts/create! Field :name "Foreign Key" :table_id (:id @t2s) :fk_target_field_id (:id @f1s)))
+ (reset! f3s (ts/create! Field :name "Nested Field" :table_id (:id @t1s) :parent_id (:id @f1s)))
(reset! serialized (into [] (serdes.extract/extract {})))))
(testing "serialization of databases is based on the :name"
@@ -152,10 +154,16 @@
(testing "foreign key references are serialized as a field path"
(is (= ["db1" nil "posts" "Target Field"]
(->> @serialized
- (filter #(-> % :serdes/meta last :model (= "Field")))
- (filter #(-> % :table_id (= ["db2" nil "posts"])))
- (keep :fk_target_field_id)
- first))))
+ (u/seek #(and (-> % :serdes/meta last :model (= "Field"))
+ (-> % :name (= "Foreign Key"))))
+ :fk_target_field_id))))
+
+ (testing "Parent field references are serialized as a field path"
+ (is (= ["db1" nil "posts" "Target Field"]
+ (->> @serialized
+ (u/seek #(and (-> % :serdes/meta last :model (= "Field"))
+ (-> % :name (= "Nested Field"))))
+ :parent_id))))
(testing "deserialization works properly, keeping the same-named tables apart"
(ts/with-db dest-db
@@ -170,7 +178,11 @@
(is (= #{(:id @db1d) (:id @db2d)}
(t2/select-fn-set :db_id Table :name "posts")))
(is (t2/exists? Table :name "posts" :db_id (:id @db1d)))
- (is (t2/exists? Table :name "posts" :db_id (:id @db2d)))))))))
+ (is (t2/exists? Table :name "posts" :db_id (:id @db2d)))
+ (is (= (t2/select-one-fn :id Field :name (:name @f1s))
+ (t2/select-one-fn :fk_target_field_id Field :name (:name @f2s))))
+ (is (= (t2/select-one-fn :id Field :name (:name @f1s))
+ (t2/select-one-fn :parent_id Field :name (:name @f3s))))))))))
(deftest card-dataset-query-test
;; Card.dataset_query is a JSON-encoded MBQL query, which contain database, table, and field IDs - these need to be
diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj
index b5d845378bf..d970ce80412 100644
--- a/src/metabase/models/field.clj
+++ b/src/metabase/models/field.clj
@@ -398,13 +398,15 @@
(update :dimensions extract-dimensions)
(update :table_id serdes/*export-table-fk*)
(update :fk_target_field_id serdes/*export-field-fk*)
+ (update :parent_id serdes/*export-field-fk*)
(dissoc :fingerprint :last_analyzed :fingerprint_version))))
(defmethod serdes/load-xform "Field"
[field]
(-> (serdes/load-xform-basics field)
(update :table_id serdes/*import-table-fk*)
- (update :fk_target_field_id serdes/*import-field-fk*)))
+ (update :fk_target_field_id serdes/*import-field-fk*)
+ (update :parent_id serdes/*import-field-fk*)))
(defmethod serdes/load-find-local "Field"
[path]
--
GitLab