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

understandable error when export target directory is not writable (#36535)

parent 6d8cefdc
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 98 additions and 53 deletions
enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
+ 5
− 0
View file @ 0d5bf036
(ns metabase-enterprise.serialization.cmd
(:refer-clojure :exclude [load])
(:require
[clojure.java.io :as io]
[metabase-enterprise.serialization.dump :as dump]
[metabase-enterprise.serialization.load :as load]
[metabase-enterprise.serialization.v2.entity-ids :as v2.entity-ids]
......@@ -206,6 +207,10 @@
(mdb/setup-db!)
(check-premium-token!)
(t2/select User) ;; TODO -- why??? [editor's note: this comment originally from Cam]
(let [f (io/file path)]
(.mkdirs f)
(when-not (.canWrite f)
(throw (ex-info (format "Destination path is not writeable: %s" path) {:filename path}))))
(serdes/with-cache
(-> (cond-> opts
(seq collection-ids) (assoc :targets (v2.extract/make-targets-of-type "Collection" collection-ids)))
......
enterprise/backend/src/metabase_enterprise/serialization/dump.clj
+ 7
− 2
View file @ 0d5bf036
......@@ -62,8 +62,13 @@
Writes (even nested) yaml keys in a deterministic fashion."
[filename obj]
(io/make-parents filename)
(spit filename (yaml/generate-string (serialization-deep-sort obj)
{:dumper-options {:flow-style :block :split-lines false}})))
(try
(spit filename (yaml/generate-string (serialization-deep-sort obj)
{:dumper-options {:flow-style :block :split-lines false}}))
(catch Exception e
(if-not (.canWrite (.getParentFile (io/file filename)))
(throw (ex-info (format "Destination path is not writeable: %s" filename) {:filename filename}))
(throw e)))))
(defn- as-file?
[instance]
......
enterprise/backend/src/metabase_enterprise/serialization/v2/storage.clj
+ 2
− 0
View file @ 0d5bf036
......@@ -6,6 +6,8 @@
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log]))
(set! *warn-on-reflection* true)
(defn- escape-segment
"Given a path segment, which is supposed to be the name of a single file or directory, escape any slashes inside it.
This occurs in practice, for example with a `Field.name` containing a slash like \"Company/organization website\"."
......
enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
+ 13
− 1
View file @ 0d5bf036
......@@ -4,6 +4,7 @@
[clojure.test :refer :all]
[metabase-enterprise.serialization.load :as load]
[metabase-enterprise.serialization.test-util :as ts]
[metabase-enterprise.serialization.v2.extract :as v2.extract]
[metabase.cmd :as cmd]
[metabase.models :refer [Card Dashboard DashboardCard Database User]]
[metabase.public-settings.premium-features-test :as premium-features-test]
......@@ -140,7 +141,7 @@
(deftest premium-features-test
(testing "without a premium token"
(let [dump-dir (ts/random-dump-dir "serdes-")]
(ts/with-random-dump-dir [dump-dir "serdes-"]
(testing "dump should fail"
(is (thrown-with-msg? Exception #"Please upgrade"
(cmd/dump dump-dir "--user" "crowberto@metabase.com"))))
......@@ -151,3 +152,14 @@
(cmd/load dump-dir
"--mode" "update"
"--on-error" "abort"))))))))
(deftest dump-readonly-dir-test
(testing "command exits early when destination is not writable"
(premium-features-test/with-premium-features #{:serialization}
(ts/with-random-dump-dir [dump-dir "serdesv2-"]
(.mkdirs (io/file dump-dir))
(.setWritable (io/file dump-dir) false)
(with-redefs [v2.extract/extract (fn [& _args]
(throw (ex-info "Do not call me!" {})))]
(is (thrown-with-msg? Exception #"Destination path is not writeable: "
(cmd/export dump-dir))))))))
enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
+ 2
− 1
View file @ 0d5bf036
......@@ -12,6 +12,7 @@
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.test :as mt]
[metabase.test.data :as data]
[metabase.util.files :as u.files]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
......@@ -102,7 +103,7 @@
`(~'&do-with-dest-db (fn [] ~@body)))
(defn random-dump-dir [prefix]
(str (System/getProperty "java.io.tmpdir") "/" prefix (mt/random-name)))
(str (u.files/get-path (System/getProperty "java.io.tmpdir") prefix (mt/random-name))))
(defn do-with-random-dump-dir [prefix f]
(let [dump-dir (random-dump-dir (or prefix ""))]
......
enterprise/backend/test/metabase_enterprise/serialization/v2/storage_test.clj
+ 69
− 49
View file @ 0d5bf036
......@@ -156,52 +156,72 @@
(update :base_type keyword))))))))))
(deftest yaml-sorted-test
(mt/with-empty-h2-app-db
(ts/with-temp-dpc [Database db {:name "My Company Data"}
Table t {:name "Customers" :db_id (:id db)}
Field w {:name "Company/organization website" :table_id (:id t)}
FieldValues _ {:field_id (:id w)}
Collection col {:name "Some Collection"}
Card c1 {:name "some card" :collection_id nil}
Card c2 {:name "other card" :collection_id (:id col)}
Dashboard d1 {:name "some dash" :collection_id (:id col)}
DashboardCard _ {:card_id (:id c1) :dashboard_id (:id d1)}
DashboardCard _ {:card_id (:id c2) :dashboard_id (:id d1)}
NativeQuerySnippet _ {:name "root snippet" :collection_id nil}]
(let [export (extract/extract nil)
check-sort (fn [coll order]
(loop [[k :as ks] (keys coll)
idx -1]
(let [new-idx (get order k)]
(if (nil? new-idx)
;; rest are sorted alphabetically
(is (= (not-empty (sort ks))
(not-empty ks)))
(do
;; check every present key is sorted in a monotone increasing order
(is (< idx (get order k)))
(recur (rest ks)
(long new-idx)))))))
descend (fn descend
([coll]
(let [model (-> (:serdes/meta coll) last :model)]
(is model)
(descend coll [(keyword model)])))
([coll path]
(let [order (or (get @@#'dump/serialization-order path)
(get @@#'dump/serialization-order (last path)))]
(testing (str "Path = " path)
(is order)
(check-sort coll order))
(doseq [[k v] coll]
(cond
(map? v) (descend v (conj path k))
(and (sequential? v)
(map? (first v))) (run! #(descend % (conj path k)) v))))))]
(with-redefs [spit (fn [fname yaml-data]
(testing (format "File %s\n" fname)
(let [coll (yaml/parse-string yaml-data)]
(if (str/ends-with? fname "settings.yaml")
(descend coll [:settings])
(descend coll)))))]
(storage/store! export "/non-existent"))))))
(ts/with-random-dump-dir [dump-dir "serdesv2-"]
(mt/with-empty-h2-app-db
(ts/with-temp-dpc [Database db {:name "My Company Data"}
Table t {:name "Customers" :db_id (:id db)}
Field w {:name "Company/organization website" :table_id (:id t)}
FieldValues _ {:field_id (:id w)}
Collection col {:name "Some Collection"}
Card c1 {:name "some card" :collection_id nil}
Card c2 {:name "other card" :collection_id (:id col)}
Dashboard d1 {:name "some dash" :collection_id (:id col)}
DashboardCard _ {:card_id (:id c1) :dashboard_id (:id d1)}
DashboardCard _ {:card_id (:id c2) :dashboard_id (:id d1)}
NativeQuerySnippet _ {:name "root snippet" :collection_id nil}]
(let [export (extract/extract nil)
check-sort (fn [coll order]
(loop [[k :as ks] (keys coll)
idx -1]
(let [new-idx (get order k)]
(if (nil? new-idx)
;; rest are sorted alphabetically
(is (= (not-empty (sort ks))
(not-empty ks)))
(do
;; check every present key is sorted in a monotone increasing order
(is (< idx (get order k)))
(recur (rest ks)
(long new-idx)))))))
descend (fn descend
([coll]
(let [model (-> (:serdes/meta coll) last :model)]
(is model)
(descend coll [(keyword model)])))
([coll path]
(let [order (or (get @@#'dump/serialization-order path)
(get @@#'dump/serialization-order (last path)))]
(testing (str "Path = " path)
(is order)
(check-sort coll order))
(doseq [[k v] coll]
(cond
(map? v) (descend v (conj path k))
(and (sequential? v)
(map? (first v))) (run! #(descend % (conj path k)) v))))))]
(with-redefs [spit (fn [fname yaml-data]
(testing (format "File %s\n" fname)
(let [coll (yaml/parse-string yaml-data)]
(if (str/ends-with? fname "settings.yaml")
(descend coll [:settings])
(descend coll)))))]
(storage/store! export dump-dir)))))))
(deftest store-error-test
(testing "destination not writable"
(ts/with-random-dump-dir [parent-dir "serdesv2-"]
(let [dump-dir (str parent-dir "/test")]
(testing "parent is not writable, cannot create own directory"
(.mkdirs (io/file parent-dir))
(.setWritable (io/file parent-dir) false)
(is (thrown-with-msg? Exception #"Destination path is not writeable: "
(storage/store! [{:serdes/meta [{:model "A" :id "B"}]}]
dump-dir))))
(testing "directory exists but is not writable"
(.setWritable (io/file parent-dir) true)
(.mkdirs (io/file dump-dir))
(io/make-parents dump-dir "inner")
(.setWritable (io/file dump-dir) false)
(is (thrown-with-msg? Exception #"Destination path is not writeable: "
(storage/store! [{:serdes/meta [{:model "A" :id "B"}]}]
dump-dir))))))))
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