From 775d7183bb40e685183d5458daf4911825317f9e Mon Sep 17 00:00:00 2001
From: John Swanson <john.swanson@metabase.com>
Date: Mon, 8 Jul 2024 13:55:39 -0700
Subject: [PATCH] Nicer error messages for linting migrations (#43667)
* Nicer error messages for linting migrations
In general, just throw exceptions instead of using clojure spec here.
You'll only get one error per run, but that seems fine.
I did keep some specs around, but run them slightly differently. Rather
than validating the whole collection of changeSets at once with `s/+`,
just `doseq` through the changeSets and validate each one separately.
That way, the value that's presented as erroneous is much smaller (a
single changeSet) and it's easier to see what went wrong.
* Update bin/lint-migrations-file/test/lint_migrations_file_test.clj
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
* Update bin/lint-migrations-file/src/lint_migrations_file.clj
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
* Require preConditions for certain liquibase change types to encourage idempotence (#44578)
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
* remove `!`s from non-side-effecty fns
* add preConditions to a few v51 migrations
---------
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
---
.../src/change_set/strict.clj | 27 +-
.../src/lint_migrations_file.clj | 136 ++++++----
.../test/lint_migrations_file_test.clj | 241 ++++++++++++------
.../migrations/001_update_migrations.yaml | 4 +
4 files changed, 279 insertions(+), 129 deletions(-)
diff --git a/bin/lint-migrations-file/src/change_set/strict.clj b/bin/lint-migrations-file/src/change_set/strict.clj
index 045b4142087..7c60f133cd1 100644
--- a/bin/lint-migrations-file/src/change_set/strict.clj
+++ b/bin/lint-migrations-file/src/change_set/strict.clj
@@ -35,7 +35,7 @@
(s/keys :opt-un [::dbms]))
(s/def ::preConditions
- (s/coll-of ::preCondition))
+ (s/nilable (s/coll-of ::preCondition)))
(s/def ::dbms
(s/keys :req-un [::type]))
@@ -122,6 +122,30 @@
(some change-types-supporting-rollback (mapcat keys changes))
(contains? change-set :rollback)))
+(def ^:private change-types-requiring-preconditions
+ #{:createTable
+ :dropTable
+ :addColumn
+ :dropColumn
+ :createIndex
+ :dropIndex
+ :addForeignKeyConstraint
+ :dropForeignKeyConstraint})
+
+(defn- precondition-present-when-required?
+ "Ensures that certain changeSet types include a preCondition. The intent is for the preCondition to ensure that the changeSet is
+ idempotent by checking whether the table/column/index/etc does not exist before trying to create it. (Or inversely, that it does
+ exist before trying to drop it.)
+
+ We don't currently assert on the structure of the preCondition to provide flexibility if there are cases where idempotence is not
+ desired."
+ [{:keys [id changes] :as change-set}]
+ (or
+ (int? id)
+ (< (major-version id) 51)
+ (not-any? change-types-requiring-preconditions (mapcat keys changes))
+ (contains? change-set :preConditions)))
+
(defn- disallow-delete-cascade-with-add-column
"Returns false if addColumn changeSet uses deleteCascade. See Metabase issue #14321"
[{:keys [changes]}]
@@ -133,6 +157,7 @@
(s/def ::change-set
(s/and
rollback-present-when-required?
+ precondition-present-when-required?
disallow-delete-cascade-with-add-column
(s/keys :req-un [::id ::author ::changes ::comment]
:opt-un [::preConditions])))
diff --git a/bin/lint-migrations-file/src/lint_migrations_file.clj b/bin/lint-migrations-file/src/lint_migrations_file.clj
index 2a5e914270b..5b2166cc4da 100644
--- a/bin/lint-migrations-file/src/lint_migrations_file.clj
+++ b/bin/lint-migrations-file/src/lint_migrations_file.clj
@@ -4,20 +4,32 @@
[clj-yaml.core :as yaml]
[clojure.java.io :as io]
[clojure.pprint :as pprint]
+ [clojure.set :as set]
[clojure.spec.alpha :as s]
- [clojure.string :as str]))
+ [clojure.string :as str])
+ (:import
+ [clojure.lang ExceptionInfo]))
(set! *warn-on-reflection* true)
(comment change-set.strict/keep-me)
+(defmacro validation-error
+ "An exception with `::validation-error` in the data, for easy id later."
+ [msg & [data]]
+ `(ex-info ~msg (merge ~data {::validation-error true})))
+
+(defn- validation-error? [e]
+ (::validation-error (ex-data e)))
+
;; just print ordered maps like normal maps.
(defmethod print-method flatland.ordered.map.OrderedMap
[m writer]
(print-method (into {} m) writer))
-(s/def ::migrations
- (s/keys :req-un [::databaseChangeLog]))
+(defn- require-database-change-log! [migrations]
+ (when-not (contains? migrations :databaseChangeLog)
+ (throw (validation-error "Missing `databaseChangeLog` key."))))
(defn- change-set-ids
"Returns all the change set ids given a change-log."
@@ -26,14 +38,26 @@
:when id]
id))
-(defn- distinct-change-set-ids? [change-log]
- (let [ids (change-set-ids change-log)]
- ;; can't apply distinct? with so many IDs
- (= (count ids) (count (set ids)))))
-
-(defn- change-set-ids-in-order? [change-log]
- (let [ids (change-set-ids change-log)]
- (= ids (sort-by identity compare ids))))
+(defn- require-distinct-change-set-ids [change-log]
+ (let [ids (change-set-ids change-log)
+ duplicates (->> ids
+ (group-by identity)
+ (keep (fn [[k v]]
+ (when (< 1 (count v))
+ k))))]
+ (when (seq duplicates)
+ (throw (validation-error "Change set IDs are not distinct." {:duplicates duplicates})))))
+
+(defn- require-change-set-ids-in-order [change-log]
+ (let [ids (change-set-ids change-log)
+ out-of-order-ids (->> ids
+ (partition 2 1)
+ (filter (fn [[id1 id2]]
+ (pos? (compare id1 id2)))))]
+
+ (when (seq out-of-order-ids)
+ (throw (validation-error "Change set IDs are not in order"
+ {:out-of-order-ids out-of-order-ids})))))
(defn- check-change-use-types?
"Return `true` if change use any type in `types`."
@@ -59,40 +83,45 @@
[target-types change-set]
(some #(check-change-use-types? target-types %) (get-in change-set [:changeSet :changes])))
-(defn- assert-no-types-in-change-log
+(defn- require-no-types-in-change-log!
"Returns true if none of the changes in the change-log contain usage of any type specified in `target-types`.
`id-filter-fn` is a function that takes an ID and return true if the changeset should be checked."
([target-types change-log]
- (assert-no-types-in-change-log target-types change-log (constantly true)))
+ (require-no-types-in-change-log! target-types change-log (constantly true)))
([target-types change-log id-filter-fn]
{:pre [(set? target-types)]}
- (->> change-log
- (filter (fn [change-set]
- (let [id (get-in change-set [:changeSet :id])]
- (and (string? id)
- (id-filter-fn id)))))
- (some #(check-change-set-use-types? target-types %))
- not)))
-
-(defn no-bare-blob-or-text-types?
- "Ensures that no \"text\" or \"blob\" type columns are added in changesets with id later than 320 (i.e. version
- 0.42.0). From that point on, \"${text.type}\" should be used instead, so that MySQL can handle it correctly (by using
- `LONGTEXT`). And similarly, from an earlier point, \"${blob.type}\" should be used instead of \"blob\"."
+ (when-let [using-types? (->> change-log
+ (filter (fn [change-set]
+ (let [id (get-in change-set [:changeSet :id])]
+ (and (string? id)
+ (id-filter-fn id)))))
+ (filter #(check-change-set-use-types? target-types %))
+ (map #(get-in % [:changeSet :id]))
+ seq)]
+ (throw (validation-error
+ (format "Migration(s) [%s] uses invalid types (in %s)"
+ (str/join "," (map #(str "'" % "'") using-types?))
+ (str/join "," (map #(str "'" % "'") target-types)))
+ {:invalid-ids using-types?
+ :target-types target-types})))))
+
+(defn require-no-bare-blob-or-text-types
+ "Ensures that no \"text\" or \"blob\" type columns are added in any changesets."
[change-log]
- (assert-no-types-in-change-log #{"blob" "text"} change-log))
+ (require-no-types-in-change-log! #{"blob" "text"} change-log))
-(defn no-bare-boolean-types?
+(defn require-no-bare-boolean-types
"Ensures that no \"boolean\" type columns are added in changesets with id later than v49.00-032. From that point on,
\"${boolean.type}\" should be used instead, so that we can consistently use `BIT(1)` for Boolean columns on MySQL."
[change-log]
- (assert-no-types-in-change-log #{"boolean"} change-log #(pos? (compare % "v49.00-032"))))
+ (require-no-types-in-change-log! #{"boolean"} change-log #(pos? (compare % "v49.00-032"))))
-(defn no-datetime-type?
+(defn require-no-datetime-type
"Ensures that no \"datetime\" or \"timestamp without time zone\".
From that point on, \"${timestamp_type}\" should be used instead, so that all of our time related columsn are tz-aware."
[change-log]
- (assert-no-types-in-change-log
+ (require-no-types-in-change-log!
#{"datetime" "timestamp" "timestamp without time zone"}
change-log
#(pos? (compare % "v49.00-000"))))
@@ -100,22 +129,30 @@
(s/def ::changeSet
(s/spec :change-set.strict/change-set))
-(s/def ::databaseChangeLog
- (s/and distinct-change-set-ids?
- change-set-ids-in-order?
- no-bare-blob-or-text-types?
- no-bare-boolean-types?
- no-datetime-type?
- (s/+ (s/alt :property (s/keys :req-un [::property])
- :objectQuotingStrategy (s/keys :req-un [::objectQuotingStrategy])
- :changeSet (s/keys :req-un [::changeSet])))))
+(defn- only-key [m]
+ (let [keys (keys m)]
+ (when-not (= 1 (count keys))
+ (throw (validation-error "Expected exactly one key." {:keys keys})))
+ (first keys)))
+
+(defn- validate-database-change-log [change-log]
+ (require-distinct-change-set-ids change-log)
+ (require-change-set-ids-in-order change-log)
+ (require-no-bare-blob-or-text-types change-log)
+ (require-no-bare-boolean-types change-log)
+ (require-no-datetime-type change-log)
+ (let [{:keys [changeSet]
+ :as all} (group-by only-key change-log)]
+ (when-not (set/subset? (set (keys all)) #{:property :objectQuotingStrategy :changeSet})
+ (throw (validation-error "Expected exactly one of :property, :objectQuotingStrategy, or :changeSet."
+ {:keys (keys all)})))
+ (doseq [{:keys [changeSet]} changeSet
+ :when (not (s/valid? ::changeSet changeSet))]
+ (throw (validation-error "Invalid change set." (s/explain-data ::changeSet changeSet))))))
(defn- validate-migrations [migrations]
- (when (= (s/conform ::migrations migrations) ::s/invalid)
- (let [data (s/explain-data ::migrations migrations)]
- (throw (ex-info (str "Validation failed:\n" (with-out-str (pprint/pprint (mapv #(dissoc % :val)
- (::s/problems data)))))
- (or (dissoc data ::s/value) {})))))
+ (require-database-change-log! migrations)
+ (validate-database-change-log (:databaseChangeLog migrations))
:ok)
(def ^:private filename
@@ -144,6 +181,17 @@
(validate-all)
(println "Ok.")
(System/exit 0)
+ (catch ExceptionInfo e
+ (if (validation-error? e)
+ (do
+ (println)
+ (printf "Error:\t%s\n" (.getMessage e))
+ (printf "Details:\n\n %s" (with-out-str (pprint/pprint (dissoc (ex-data e) ::validation-error))))
+ (println))
+ (do
+ (pprint/pprint (Throwable->map e))
+ (println (.getMessage e))))
+ (System/exit 1))
(catch Throwable e
(pprint/pprint (Throwable->map e))
(println (.getMessage e))
diff --git a/bin/lint-migrations-file/test/lint_migrations_file_test.clj b/bin/lint-migrations-file/test/lint_migrations_file_test.clj
index 079c6b8392a..b3a4da842ab 100644
--- a/bin/lint-migrations-file/test/lint_migrations_file_test.clj
+++ b/bin/lint-migrations-file/test/lint_migrations_file_test.clj
@@ -38,30 +38,52 @@
(try (#'lint-migrations-file/validate-migrations {:databaseChangeLog changes})
(catch Exception e (ex-data e))))
+(defmacro is-thrown-with-error-info? [msg info & body]
+ `(let [exception# (try (do ~@body)
+ nil
+ (catch clojure.lang.ExceptionInfo e# e#)
+ (catch Throwable t#
+ (throw (ex-info "An unexpected exception type was thrown"
+ {:expected clojure.lang.ExceptionInfo
+ :actual t#}))))]
+ (is (instance? clojure.lang.ExceptionInfo exception#)
+ "Expected clojure.lang.ExceptionInfo but caught different type.")
+ (is (not (nil? exception#))
+ "No exception was thrown.")
+ (is (::lint-migrations-file/validation-error (ex-data exception#))
+ "The exception was not a validation error.")
+ (let [ex-msg# (.getMessage exception#)
+ ex-data# (dissoc (ex-data exception#) ::lint-migrations-file/validation-error)]
+ (is (= ~msg ex-msg#)
+ "Error message does not match expected.")
+ (is (= ~info ex-data#)
+ "Error info does not match expected."))))
+
(deftest require-unique-ids-test
(testing "Make sure all migration IDs are unique"
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"distinct-change-set-ids"
- (validate
- (mock-change-set :id "v49.2024-01-01T10:30:00")
- (mock-change-set :id "v49.2024-01-01T10:30:00"))))))
+ (is-thrown-with-error-info?
+ "Change set IDs are not distinct."
+ {:duplicates ["v49.2024-01-01T10:30:00"]}
+ (validate
+ (mock-change-set :id "v49.2024-01-01T10:30:00")
+ (mock-change-set :id "v49.2024-01-01T10:30:00")))))
(deftest require-migrations-in-order-test
(testing "Migrations must be in order"
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"change-set-ids-in-order"
- (validate
- (mock-change-set :id "v45.00-002")
- (mock-change-set :id "v45.00-001"))))
+ (is-thrown-with-error-info?
+ "Change set IDs are not in order"
+ {:out-of-order-ids [["v45.00-002" "v45.00-001"]]}
+ (validate
+ (mock-change-set :id "v45.00-002")
+ (mock-change-set :id "v45.00-001")))
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"change-set-ids-in-order"
- (validate
- (mock-change-set :id "v49.2023-12-14T08:54:54")
- (mock-change-set :id "v49.2023-12-14T08:54:53"))))))
+ (is-thrown-with-error-info?
+ "Change set IDs are not in order"
+ {:out-of-order-ids [["v49.2023-12-14T08:54:54"
+ "v49.2023-12-14T08:54:53"]]}
+ (validate
+ (mock-change-set :id "v49.2023-12-14T08:54:54")
+ (mock-change-set :id "v49.2023-12-14T08:54:53")))))
(deftest only-one-column-per-add-column-test
(testing "we should only allow one column per addColumn change"
@@ -73,7 +95,7 @@
:changes [(mock-add-column-changes)]))))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #"Extra input"
+ #"Invalid change set\."
(validate
(mock-change-set
:id id
@@ -84,7 +106,7 @@
(testing "only allow one change per change set"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #"Extra input"
+ #"Invalid change set\."
(validate
(mock-change-set :changes [(mock-add-column-changes) (mock-add-column-changes)]))))))
@@ -92,7 +114,7 @@
(testing "require a comment for a change set"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #"Validation failed:"
+ #"Invalid change set\."
(validate (update (mock-change-set) :changeSet dissoc :comment))))
(is (= :ok
(validate (mock-change-set :id "v49.2024-01-01T10:30:00", :comment "Added x.45.0"))))))
@@ -111,14 +133,14 @@
(testing (format "Change set =\n%s" (pr-str change-set))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #"onDelete is only for addForeignKeyConstraints"
+ #"Invalid change set\."
(validate change-set)))))))
(deftest require-remarks-for-create-table-test
(testing "require remarks for newly created tables"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":remarks"
+ #"Invalid change set\."
(validate
(mock-change-set
:id 200
@@ -137,7 +159,7 @@
(testing "should fail if *any* change is missing dbms"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":dbms"
+ #"Invalid change set\."
(validate
(mock-change-set
:changes
@@ -147,7 +169,7 @@
(testing "should fail if a DBMS is repeated"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":changes"
+ #"Invalid change set\."
(validate
(mock-change-set
:changes
@@ -166,11 +188,11 @@
(testing "invalid date components should throw an error"
(are [msg id]
- (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
- (validate-id "v49.2024-30-01T10:30:00")
- msg)
+ (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Invalid change set\."
+ (validate-id "v49.2024-30-01T10:30:00")
+ msg)
"invalid month" "v49.2024-13-01T10:30:00"
"invalid day" "v49.2024-01-32T10:30:00"
"invalid hour" "v49.2024-01-01T25:30:00"
@@ -180,42 +202,43 @@
(deftest prevent-text-types-test
(testing "should allow \"${text.type}\" columns from being added"
(is (= :ok
- (validate
- (mock-change-set
+ (validate
+ (mock-change-set
:id "v49.2024-01-01T10:30:00"
:changes [(mock-add-column-changes :columns [(mock-column :type "${text.type}")])]))))
(doseq [problem-type ["blob" "text"]]
(testing (format "should prevent \"%s\" columns from being added after ID 320" problem-type)
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"(?s)^.*no-bare-blob-or-text-types\\?.*$"
- (validate
- (mock-change-set
- :id "v49.2024-01-01T10:30:00"
- :changes [(mock-add-column-changes :columns [(mock-column :type problem-type)])]))))))))
+ (is-thrown-with-error-info?
+ "Migration(s) ['v49.2024-01-01T10:30:00'] uses invalid types (in 'blob','text')"
+ {:invalid-ids ["v49.2024-01-01T10:30:00"]
+ :target-types #{"blob" "text"}}
+ (validate
+ (mock-change-set
+ :id "v49.2024-01-01T10:30:00"
+ :changes [(mock-add-column-changes :columns [(mock-column :type problem-type)])])))))))
(deftest prevent-bare-boolean-type-test
(testing "should allow adding \"${boolean.type}\" columns"
(is (= :ok
- (validate
- (mock-change-set
+ (validate
+ (mock-change-set
:id "v49.00-033"
:changes [(mock-add-column-changes :columns [(mock-column :type "${boolean.type}")])]))))
- (testing (format "should prevent \"boolean\" columns from being added after ID v49.00-033")
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"(?s)^.*no-bare-boolean-types\\?.*$"
- (validate
- (mock-change-set
- :id "v49.00-033"
- :changes [(mock-add-column-changes :columns [(mock-column :type "boolean")])])))))))
+ (testing "should prevent \"boolean\" columns from being added after ID v49.00-033"
+ (is-thrown-with-error-info?
+ "Migration(s) ['v49.00-033'] uses invalid types (in 'boolean')"
+ {:invalid-ids ["v49.00-033"]
+ :target-types #{"boolean"}}
+ (validate (mock-change-set
+ :id "v49.00-033"
+ :changes [(mock-add-column-changes :columns [(mock-column :type "boolean")])]))))))
(deftest require-rollback-test
(testing "change types with no automatic rollback support"
(testing "missing rollback key fails"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #"rollback-present-when-required"
+ #"Invalid change set\."
(validate (update (mock-change-set :id "v49.2024-01-01T10:30:00" :changes [{:sql {:sql "select 1"}}])
:changeSet dissoc :rollback)))))
(testing "nil rollback is allowed"
@@ -229,11 +252,40 @@
(testing "change types with automatic rollback support are allowed"
(is (= :ok (validate (mock-change-set :id "v49.2024-01-01T10:30:00" :changes [(mock-add-column-changes)]))))))
+(deftest require-precondition-test
+ (testing "certain change types require preConditions"
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Invalid change set\."
+ (validate (mock-change-set
+ :id "v51.2024-01-01T10:30:00"
+ :changes [(mock-create-table-changes)])))))
+
+ (testing "other change types are exempt"
+ (is (= :ok
+ (validate
+ (mock-change-set
+ :changes
+ [{:sql {:dbms "h2", :sql "1"}}])))))
+
+ (testing "nil preConditions is allowed"
+ (is (= :ok
+ (validate (mock-change-set
+ :id "v51.2024-01-01T10:30:00"
+ :changes [(mock-create-table-changes)]
+ :preConditions nil)))))
+
+ (testing "changeSets prior to v51 are exempt"
+ (is (= :ok
+ (validate (mock-change-set
+ :id "v50.2024-01-01T10:30:00"
+ :changes [(mock-create-table-changes)]))))))
+
(deftest disallow-deletecascade-in-addcolumn-test
(testing "addColumn with deleteCascade fails"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #"disallow-delete-cascade"
+ #"Invalid change set."
(validate (mock-change-set :id "v49.2024-01-01T10:30:00"
:changes [(mock-add-column-changes
:columns [(mock-column :constraints {:deleteCascade true})])]))))))
@@ -267,45 +319,66 @@
(deftest forbidden-new-types-test
(testing "should throw if changes contains text type"
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
+ (is (is-thrown-with-error-info?
+ "Migration(s) ['v45.12-345'] uses invalid types (in 'blob','text')"
+ {:invalid-ids ["v45.12-345"]
+ :target-types #{"blob" "text"}}
(validate (mock-change-set :id "v45.12-345"
:changes [{:modifyDataType {:newDataType "text"}}]
:rollback nil))))
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
- (validate (mock-change-set :id "v45.12-345"
- :changes [{:createTable {:columns [{:column {:type "text"}}]}}]
- :rollback nil)))))
+ (is-thrown-with-error-info?
+ "Migration(s) ['v45.12-345'] uses invalid types (in 'blob','text')"
+ {:invalid-ids ["v45.12-345"]
+ :target-types #{"blob" "text"}}
+ (validate (mock-change-set :id "v45.12-345"
+ :changes [{:createTable {:tableName "my_table"
+ :remarks "meow"
+ :columns [{:column {:name "foo"
+ :remarks "none"
+ :type "text"}}]}}]
+ :rollback nil))))
(testing "should throw if changes contains boolean type"
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
- (validate (mock-change-set :id "v49.00-033"
- :changes [{:modifyDataType {:newDataType "boolean"}}]
- :rollback nil))))
+ (is-thrown-with-error-info?
+ "Migration(s) ['v49.00-033'] uses invalid types (in 'boolean')"
+ {:invalid-ids ["v49.00-033"]
+ :target-types #{"boolean"}}
+ (validate (mock-change-set :id "v49.00-033"
+ :changes [{:modifyDataType {:newDataType "boolean"}}]
+ :rollback nil)))
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
- (validate (mock-change-set :id "v45.12-345"
- :changes [{:createTable {:columns [{:column {:type "boolean"}}]}}]
- :rollback nil)))))
+ (is-thrown-with-error-info?
+ "Migration(s) ['v49.00-033'] uses invalid types (in 'boolean')"
+ {:invalid-ids ["v49.00-033"]
+ :target-types #{"boolean"}}
+ (validate (mock-change-set :id "v49.00-033"
+ :changes [{:createTable {:tableName "my_table"
+ :remarks "meow"
+ :columns [{:column {:name "foo"
+ :remarks "none"
+ :type "boolean"}}]}}])))
+ (testing "does not throw for older migrations"
+ (is (validate (mock-change-set :id "v45.00-033"
+ :changes [{:createTable {:tableName "my_table"
+ :remarks "meow"
+ :columns [{:column {:name "foo"
+ :remarks "none"
+ :type "boolean"}}]}}])))))
(testing "should throw if changes contains datetime type"
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
- (validate (mock-change-set :id "v49.00-033"
- :changes [{:modifyDataType {:newDataType "datetime"}}]
- :rollback nil))))
+ (is-thrown-with-error-info?
+ "Migration(s) ['v49.00-033'] uses invalid types (in 'timestamp','timestamp without time zone','datetime')"
+ {:invalid-ids ["v49.00-033"]
+ :target-types #{"timestamp" "timestamp without time zone" "datetime"}}
+ (validate (mock-change-set :id "v49.00-033"
+ :changes [{:modifyDataType {:newDataType "datetime"}}]
+ :rollback nil)))
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Validation failed"
- (validate (mock-change-set :id "v45.12-345"
- :changes [{:createTable {:columns [{:column {:type "timestamp with time zone"}}]}}]
- :rollback nil))))))
+ (testing "(but not if it's an older migration)"
+ (is (validate (mock-change-set :id "v45.12-345"
+ :changes [{:createTable {:tableName "my_table"
+ :remarks "meow"
+ :columns [{:column {:name "foo"
+ :remarks "none"
+ :type "timestamp with time zone"}}]}}]
+ :rollback nil))))))
diff --git a/resources/migrations/001_update_migrations.yaml b/resources/migrations/001_update_migrations.yaml
index 828294b6cbf..0fe9355a23b 100644
--- a/resources/migrations/001_update_migrations.yaml
+++ b/resources/migrations/001_update_migrations.yaml
@@ -8181,6 +8181,7 @@ databaseChangeLog:
name: dataset_query_metrics_v2_migration_backup
remarks: The copy of dataset_query before the metrics v2 migration
type: ${text.type}
+ preConditions:
- changeSet:
id: v51.2024-05-13T16:00:00
@@ -8205,6 +8206,7 @@ databaseChangeLog:
defaultValue: "view"
constraints:
nullable: false
+ preConditions:
- changeSet:
id: v51.2024-06-07T12:37:36
@@ -8234,6 +8236,7 @@ databaseChangeLog:
name: session_id
- column:
name: device_id
+ preConditions:
- changeSet:
id: v51.2024-06-12T18:53:03
@@ -8252,6 +8255,7 @@ databaseChangeLog:
- dropIndex:
tableName: login_history
indexName: idx_user_id_device_id
+ preConditions:
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
--
GitLab