Define a new persist-models-enabled setting for database. (#32825)

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 ----------------------------------------------------

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")))))))))