If actions are not supported, actions should be disabled for a database (#28607)

* Add comment for database-enable-actions

* Enforce actions not supported implies actions are disabled

* Remove test and update check

* Fix typos

* Revert "Remove test and update check"

This reverts commit 8b1f14f44b.

* Fix test
This commit is contained in:
Cal Herries
2023-02-27 17:14:58 +02:00
committed by GitHub
parent 7e5a8b7afa
commit b2d7687f36
2 changed files with 29 additions and 2 deletions

View File

@@ -156,17 +156,20 @@
[{new-metadata-schedule :metadata_sync_schedule,
new-fieldvalues-schedule :cache_field_values_schedule,
new-engine :engine
new-settings :settings
:as database}]
(let [{is-sample? :is_sample
old-metadata-schedule :metadata_sync_schedule
old-fieldvalues-schedule :cache_field_values_schedule
existing-settings :settings
existing-engine :engine
existing-name :name} (db/select-one [Database
:metadata_sync_schedule
:cache_field_values_schedule
:engine
:name
:is_sample] :id (u/the-id database))
:is_sample
:settings] :id (u/the-id database))
new-engine (some-> new-engine keyword)]
(if (and is-sample?
new-engine
@@ -194,7 +197,15 @@
(schedule-tasks!
(assoc database
:metadata_sync_schedule new-metadata-schedule
:cache_field_values_schedule new-fieldvalues-schedule)))))))))
:cache_field_values_schedule new-fieldvalues-schedule)))))
;; This maintains a constraint that if a driver doesn't support actions, it can never be enabled
;; If we drop support for actions for a driver, we'd need to add a migration to disable actions for all databases
(when (and (:database-enable-actions (or new-settings existing-settings))
(not (driver/database-supports? (or new-engine existing-engine) :actions database)))
(throw (ex-info (trs "The database does not support actions.")
{:status-code 400
:existing-engine existing-engine
:new-engine new-engine})))))))
(defn- pre-insert [{:keys [details], :as database}]
(-> (cond-> database

View File

@@ -94,6 +94,22 @@
"id" 3}
(encode-decode pg-db)))))))
(deftest driver-supports-actions-and-database-enable-actions-test
(mt/test-drivers #{:sqlite}
(testing "Updating database-enable-actions to true should fail if the engine doesn't support actions"
(mt/with-temp Database [database {:engine :sqlite}]
(is (= false (driver/database-supports? :sqlite :actions database)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"The database does not support actions."
(db/update! Database (:id database) :settings {:database-enable-actions true})))))
(testing "Updating the engine when database-enable-actions is true should fail if the engine doesn't support actions"
(mt/with-temp Database [database {:engine :h2 :settings {:database-enable-actions true}}]
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"The database does not support actions."
(db/update! Database (:id database) :engine :sqlite)))))))
(deftest sensitive-data-redacted-test
(let [encode-decode (fn [obj] (decode (encode obj)))
project-id "random-project-id" ; the actual value here doesn't seem to matter