From 1c5170716a769bdc9fa25fd0480d58ea753c27ec Mon Sep 17 00:00:00 2001
From: Braden Shepherdson <Braden.Shepherdson@gmail.com>
Date: Wed, 14 Sep 2022 17:02:05 -0400
Subject: [PATCH] Serdes v2: Find YAML files with human-friendly labels
(#25390)
The YAML file names have an optional `:label` portion that becomes the
latter part of the filename. Reconstructed paths (eg. from
`serdes-dependencies`) don't have those labels.
This change makes the YAML ingestion code able to find a file with a
human-readable label even if the request didn't include it.
No ambiguity results because the file names are always based on the
unique serdes `:id`, usually an `entity_id`.
---
.../serialization/v2/ingest/yaml.clj | 5 +++--
.../serialization/v2/utils/yaml.clj | 15 +++++++++++--
.../serialization/v2/ingest/yaml_test.clj | 21 +++++++++++++++++++
3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/ingest/yaml.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/ingest/yaml.clj
index c24c90eca7f..dfba665e4d2 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/v2/ingest/yaml.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/ingest/yaml.clj
@@ -43,8 +43,9 @@
original labels by eg. truncating them to keep the file names from getting too long. The labels aren't used at all on
the loading side, so it's fine to drop them."
[root-dir hierarchy]
- (let [unlabeled (mapv #(dissoc % :label) hierarchy)]
- (-> (u.yaml/hierarchy->file root-dir hierarchy) ; Use the original hierarchy for the filesystem.
+ (let [unlabeled (mapv #(dissoc % :label) hierarchy)
+ file (u.yaml/hierarchy->file root-dir hierarchy)] ; Use the original hierarchy for the filesystem.
+ (-> (when (.exists file) file) ; If the returned file doesn't actually exist, replace it with nil.
yaml/from-file
read-timestamps
(assoc :serdes/meta unlabeled)))) ; But return the hierarchy without labels.
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/utils/yaml.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/utils/yaml.clj
index fc7d1aee336..944d6936d5f 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/v2/utils/yaml.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/utils/yaml.clj
@@ -26,8 +26,19 @@
[model id]))
;; The last part of the hierarchy is used for the basename; this is the only part with the label.
{:keys [id model label]} (last hierarchy)
- leaf-name (leaf-file-name id label)]
- (apply io/file root-dir (concat prefix [model leaf-name]))))
+ leaf-name (leaf-file-name id label)
+ as-given (apply io/file root-dir (concat prefix [model leaf-name]))]
+ (if (.exists ^File as-given)
+ as-given
+ ; If that file name doesn't exist, check the directory to see if there's one that's the requested file plus a
+ ; human-readable portion.
+ (let [dir (apply io/file root-dir (concat prefix [model]))
+ matches (filter #(and (.startsWith ^String % (str id "+"))
+ (.endsWith ^String % ".yaml"))
+ (.list ^File dir))]
+ (if (empty? matches)
+ (io/file dir leaf-name)
+ (io/file dir (first matches)))))))
(defn path-split
"Given a root directory and a file underneath it, return a sequence of path parts to get there.
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/ingest/yaml_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/ingest/yaml_test.clj
index 23d819a8672..88f2981139c 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/ingest/yaml_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/ingest/yaml_test.clj
@@ -43,3 +43,24 @@
(get abs-path)
(assoc :serdes/meta (mapv #(dissoc % :label) abs-path)))
(ingest/ingest-one ingestable abs-path))))))))
+
+(deftest flexible-file-matching-test
+ (ts/with-random-dump-dir [dump-dir "serdesv2-"]
+ (io/make-parents dump-dir "Collection" "fake")
+ (spit (io/file dump-dir "Collection" "entity-id+human-readable-things.yaml")
+ (yaml/generate-string {:some "made up" :data "here"}))
+
+ (let [ingestable (ingest.yaml/ingest-yaml dump-dir)
+ exp {:some "made up"
+ :data "here"
+ :serdes/meta [{:model "Collection" :id "entity-id"}]}]
+ (testing "the returned set of files has the human-readable labels"
+ (is (= #{[{:model "Collection" :id "entity-id" :label "human-readable-things"}]}
+ (into #{} (ingest/ingest-list ingestable)))))
+
+ (testing "fetching the file with the label works"
+ (is (= exp
+ (ingest/ingest-one ingestable [{:model "Collection" :id "entity-id" :label "human-readable-things"}]))))
+ (testing "fetching the file without the label also works"
+ (is (= exp
+ (ingest/ingest-one ingestable [{:model "Collection" :id "entity-id"}])))))))
--
GitLab