From 4976869af48e0755c0a25be1a2eefa6aa3636502 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Tue, 15 Feb 2022 16:54:58 -0800
Subject: [PATCH] Replace `copy-site-url-setting-and-remove-trailing-slashes`
with a Liquibase migration (#20402)
* Replace metabase.db.migrations/copy-site-url-setting-and-remove-trailing-slashes with a Liquibase migration
* Tweak comment
* `setting`, not `settings`
* Clean namespace
* `key` has to be quoted for MySQL
* Add tests for migrations v0.43.00-008 and v0.43.00-009
* fix new migration for MySQL 5.7
---
resources/migrations/000_migrations.yaml | 54 +++++++++++++++++++++
src/metabase/db/data_migrations.clj | 9 ----
src/metabase/models/setting.clj | 3 +-
test/metabase/db/schema_migrations_test.clj | 22 ++++++++-
4 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 5f4b329c107..d2188d07da8 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -9907,6 +9907,60 @@ databaseChangeLog:
- sql:
sql: UPDATE metabase_database SET engine = 'bigquery-cloud-sdk' WHERE engine = 'bigquery'
+ #
+ # The following migration replaces metabase.db.migrations/copy-site-url-setting-and-remove-trailing-slashes, added 0.23.0
+ #
+ # Copy the value of the old setting `-site-url` to the new `site-url` if applicable. (`site-url` used to be stored
+ # internally as `-site-url`; this was confusing, see #4188 for details.) Make sure `site-url` has no trailing slashes
+ # originally fixed in #4123.
+ - changeSet:
+ id: v43.00-008
+ author: camsaul
+ comment: >-
+ Added 0.43.0. Migrate legacy '-site-url' Setting to 'site-url'. Trim trailing slashes.
+ preConditions:
+ - onFail: MARK_RAN
+ - or:
+ - and:
+ - dbms:
+ type: postgresql,h2
+ - sqlCheck:
+ expectedResult: 0
+ sql: SELECT count(*) FROM setting WHERE key = 'site-url';
+ - and:
+ - dbms:
+ type: mysql,mariadb
+ - sqlCheck:
+ expectedResult: 0
+ sql: SELECT count(*) FROM setting WHERE `key` = 'site-url';
+ changes:
+ - sql:
+ dbms: h2,postgresql
+ sql: >-
+ INSERT INTO setting (key, value)
+ SELECT
+ 'site-url' AS key,
+ regexp_replace(value, '/$', '') AS value
+ FROM setting
+ WHERE key = '-site-url';
+ - sql:
+ dbms: mysql,mariadb
+ # MySQL 5.7 doesn't support regexp_replace :(
+ # 'key' has to be quoted in MySQL
+ sql: >-
+ INSERT INTO setting (`key`, value)
+ SELECT
+ 'site-url' AS `key`,
+ CASE
+ WHEN value LIKE '%/'
+ THEN substring(value, 1, length(value) - 1)
+ ELSE
+ value
+ END
+ AS value
+ FROM setting
+ WHERE `key` = '-site-url';
+
#
# The following migration replaces metabase.db.migrations/ensure-protocol-specified-in-site-url, added in 0.25.1
#
diff --git a/src/metabase/db/data_migrations.clj b/src/metabase/db/data_migrations.clj
index 7dddb0db1e9..fca749bc024 100644
--- a/src/metabase/db/data_migrations.clj
+++ b/src/metabase/db/data_migrations.clj
@@ -20,9 +20,7 @@
[metabase.models.permissions-group :as perm-group :refer [PermissionsGroup]]
[metabase.models.permissions-group-membership :as perm-membership :refer [PermissionsGroupMembership]]
[metabase.models.pulse :refer [Pulse]]
- [metabase.models.setting :as setting :refer [Setting]]
[metabase.models.user :refer [User]]
- [metabase.public-settings :as public-settings]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
[toucan.db :as db]
@@ -114,13 +112,6 @@
:object (perms/data-perms-path database-id)
:group_id group-id)))))
-;; Copy the value of the old setting `-site-url` to the new `site-url` if applicable. (`site-url` used to be stored
-;; internally as `-site-url`; this was confusing, see #4188 for details) This has the side effect of making sure the
-;; `site-url` has no trailing slashes (as part of the magic setter fn; this was fixed as part of #4123)
-(defmigration ^{:author "camsaul", :added "0.23.0"} copy-site-url-setting-and-remove-trailing-slashes
- (when-let [site-url (db/select-one-field :value Setting :key "-site-url")]
- (public-settings/site-url site-url)))
-
;; Before 0.30.0, we were storing the LDAP user's password in the `core_user` table (though it wasn't used). This
;; migration clears those passwords and replaces them with a UUID. This is similar to a new account setup, or how we
;; disable passwords for Google authenticated users
diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj
index 3a93c2d0f57..411d4eb7818 100644
--- a/src/metabase/models/setting.clj
+++ b/src/metabase/models/setting.clj
@@ -101,7 +101,8 @@
(def ^:private retired-setting-names
"A set of setting names which existed in previous versions of Metabase, but are no longer used. New settings may not use
these names to avoid unintended side-effects if an application database still stores values for these settings."
- #{"enable-advanced-humanization"
+ #{"-site-url"
+ "enable-advanced-humanization"
"metabot-enabled"})
(models/defmodel Setting
diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj
index 5d9c4b255fc..3aa2523d487 100644
--- a/test/metabase/db/schema_migrations_test.clj
+++ b/test/metabase/db/schema_migrations_test.clj
@@ -13,7 +13,7 @@
[clojure.test :refer :all]
[metabase.db.schema-migrations-test.impl :as impl]
[metabase.driver :as driver]
- [metabase.models :refer [Database Field Table]]
+ [metabase.models :refer [Database Field Setting Table]]
[metabase.models.interface :as mi]
[metabase.models.user :refer [User]]
[metabase.test :as mt]
@@ -303,3 +303,23 @@
:details {:service-account-json "{\"fake_key\": 14}"}}]
(migrate!)
(is (= :bigquery-cloud-sdk (db/select-one-field :engine Database :id (u/the-id db))))))))
+
+(deftest migrate-legacy-site-url-setting-test
+ (testing "Migration v043.00-008: migrate legacy `-site-url` Setting to `site-url`; remove trailing slashes (#4123, #4188, #20402)"
+ (impl/test-migrations ["v43.00-008"] [migrate!]
+ (db/execute! {:insert-into Setting
+ :values [{:key "-site-url"
+ :value "http://localhost:3000/"}]})
+ (migrate!)
+ (is (= [{:key "site-url", :value "http://localhost:3000"}]
+ (db/query {:select [:*], :from [Setting], :where [:= :key "site-url"]}))))))
+
+(deftest site-url-ensure-protocol-test
+ (testing "Migration v043.00-009: ensure `site-url` Setting starts with a protocol (#20403)"
+ (impl/test-migrations ["v43.00-009"] [migrate!]
+ (db/execute! {:insert-into Setting
+ :values [{:key "site-url"
+ :value "localhost:3000"}]})
+ (migrate!)
+ (is (= [{:key "site-url", :value "http://localhost:3000"}]
+ (db/query {:select [:*], :from [Setting], :where [:= :key "site-url"]}))))))
--
GitLab