diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index a3074adfb8fcee49fc4804f1ac2705090065fb9f..1e2a691f3b32ff958c83f9cc8fa6ed081428f291 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -10,10 +10,10 @@ [metabase.models.permissions-group :as perms-group] [metabase.models.secret :as secret :refer [Secret]] [metabase.models.serialization :as serdes] - [metabase.models.setting :as setting] + [metabase.models.setting :as setting :refer [defsetting]] [metabase.plugins.classloader :as classloader] [metabase.util :as u] - [metabase.util.i18n :refer [trs]] + [metabase.util.i18n :refer [deferred-tru trs]] [metabase.util.log :as log] [methodical.core :as methodical] [toucan2.core :as t2] @@ -247,6 +247,12 @@ [_database] [:name :engine]) +(defsetting persist-models-enabled + (deferred-tru "Whether to enable models persistence for a specific Database.") + :default false + :type :boolean + :visibility :public + :database-local :only) ;;; ---------------------------------------------- Hydration / Util Fns ---------------------------------------------- @@ -300,15 +306,20 @@ (sensitive-fields-for-db db)))))] (update db :settings (fn [settings] (when (map? settings) - (into {} - (filter (fn [[setting-name _v]] - (try - (setting/can-read-setting? setting-name - (setting/current-user-readable-visibilities)) - (catch Throwable _e - true))) - settings)))))) - + (m/filter-keys + (fn [setting-name] + (try + (setting/can-read-setting? setting-name + (setting/current-user-readable-visibilities)) + (catch Throwable e + ;; there is an known issue with exception is ignored when render API response (#32822) + ;; If you see this error, you probably need to define a setting for `setting-name`. + ;; But ideally, we should resovle the above issue, and remove this try/catch + (log/error e (format "Error checking the readability of %s setting. The setting will be hidden in API response." setting-name)) + ;; let's be conservative and hide it by defaults, if you want to see it, + ;; you need to define it :) + false))) + settings))))) json-generator)) ;;; ------------------------------------------------ Serialization ---------------------------------------------------- diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj index df0625ccbf65e0fe804b09532a63e2348ca23bb1..4891e62de6b59873f78f12959c6f89aca80afd70 100644 --- a/test/metabase/api/database_test.clj +++ b/test/metabase/api/database_test.clj @@ -1638,6 +1638,17 @@ (is (= {:database-enable-actions false} (settings))))))))) +(deftest log-an-error-if-contains-undefined-setting-test + (testing "should log an error message if database contains undefined settings" + (t2.with-temp/with-temp [Database {db-id :id} {:settings {:undefined-setting true}}] + (is (= "Error checking the readability of :undefined-setting setting. The setting will be hidden in API response." + (-> (mt/with-log-messages-for-level :error + (testing "does not includes undefined keys by default" + (is (not (contains? (:settings (mt/user-http-request :crowberto :get 200 (str "database/" db-id))) + :undefined-setting))))) + first + last)))))) + (deftest persist-database-test-2 (mt/test-drivers (mt/normal-drivers-with-feature :persist-models) (mt/dataset test-data @@ -1662,7 +1673,10 @@ :card_id (:id card)))) (is (true? (t2/select-one-fn (comp :persist-models-enabled :settings) Database - :id db-id)))) + :id db-id))) + (is (true? (get-in (mt/user-http-request :crowberto :get 200 + (str "database/" db-id)) + [:settings :persist-models-enabled])))) (testing "it's okay to trigger persist even though the database is already persisted" (mt/user-http-request :crowberto :post 204 (str "database/" db-id "/persist")))))))))