diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 03f1e57a8315c65a0946240a23aa51a996bc9bfe..70f12165a522ec225e4f147209b71e0e46bd33d9 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -15,7 +15,6 @@ (clojure.core.logic/fresh) (clojure.core.logic/matcha) (clojure.core.logic/run) - (metabase.test/with-temporary-setting-values) (clojure.test/is [query= sql= partial=]) (toucan.util.test/with-temp*) (metabase.test/with-temp*) @@ -33,7 +32,7 @@ :non-arg-vec-return-type-hint {:level :warning} :missing-body-in-when {:level :warning} :missing-docstring {:level :warning} - ; TODO (braden): This is useful, but it doesn't grok eg. enterprise/backend/src + ;; TODO (braden): This is useful, but it doesn't grok eg. enterprise/backend/src :namespace-name-mismatch {:level :off} :use {:level :warning} :deprecated-var {:exclude {metabase.query-processor.util/normalize-token @@ -361,6 +360,7 @@ metabase.db.liquibase/with-liquibase clojure.core/let metabase.models.setting.multi-setting/define-multi-setting clojure.core/def metabase.integrations.ldap/with-ldap-connection clojure.core/fn + metabase.test/are+ clojure.test/are metabase.test/defdataset clojure.core/def metabase.test/with-temp-file clojure.core/let metabase.test/with-open-channels clojure.core/let @@ -382,27 +382,35 @@ honeysql.helpers/defhelper clj-kondo.lint-as/def-catch-all clojure.core.logic/defne clj-kondo.lint-as/def-catch-all monger.operators/defoperator clojure.core/def} - :hooks {:analyze-call {metabase.test.data/dataset hooks.metabase.test.data/dataset - metabase.test/dataset hooks.metabase.test.data/dataset - metabase.test.data/$ids hooks.metabase.test.data/$ids - metabase.test/$ids hooks.metabase.test.data/$ids - metabase.test.data/mbql-query hooks.metabase.test.data/$ids - metabase.test/mbql-query hooks.metabase.test.data/$ids - metabase.test/data-query hooks.metabase.test.data/$ids - metabase.test/query hooks.metabase.test.data/$ids - metabase.test/run-mbql-query hooks.metabase.test.data/$ids - metabase.test.data/run-mbql-query hooks.metabase.test.data/$ids - metabase.api.common/defendpoint hooks.metabase.api.common/defendpoint - metabase.api.common/defendpoint-async hooks.metabase.api.common/defendpoint + :hooks + {:analyze-call + {metabase.api.common/defendpoint hooks.metabase.api.common/defendpoint + metabase.api.common/defendpoint-async hooks.metabase.api.common/defendpoint + metabase.models.setting/defsetting hooks.metabase.models.setting/defsetting + metabase.models.setting.multi-setting/define-multi-setting hooks.metabase.models.setting/defsetting + metabase.test/$ids hooks.metabase.test.data/$ids + metabase.test/data-query hooks.metabase.test.data/$ids + metabase.test/dataset hooks.metabase.test.data/dataset + metabase.test/mbql-query hooks.metabase.test.data/$ids + metabase.test/query hooks.metabase.test.data/$ids + metabase.test/run-mbql-query hooks.metabase.test.data/$ids + metabase.test/with-temp hooks.toucan.util.test/with-temp + metabase.test/with-temporary-setting-values hooks.metabase.test.util/with-temporary-setting-values + metabase.test.data/$ids hooks.metabase.test.data/$ids + metabase.test.data/dataset hooks.metabase.test.data/dataset + metabase.test.data/mbql-query hooks.metabase.test.data/$ids + metabase.test.data/run-mbql-query hooks.metabase.test.data/$ids + metabase.test.util/with-temporary-setting-values hooks.metabase.test.util/with-temporary-setting-values} - metabase.test/with-temp hooks.toucan.util.test/with-temp} - :macroexpand {metabase.query-processor.streaming/streaming-response - metabase.query-processor.streaming/streaming-response - metabase.api.common/define-routes macros.metabase.api.common/define-routes + :macroexpand + {metabase.query-processor.streaming/streaming-response macros.metabase.query-processor.streaming/streaming-response + metabase.api.common/define-routes macros.metabase.api.common/define-routes - toucan.models/defmodel toucan.models/defmodel - clojurewerkz.quartzite.jobs/build macros.quartz/build-job - clojurewerkz.quartzite.triggers/build macros.quartz/build-trigger - clojurewerkz.quartzite.schedule.cron/schedule macros.quartz/schedule - clojurewerkz.quartzite.schedule.simple/schedule macros.quartz/simple-schedule}} - :config-in-comment {:linters {:unresolved-symbol {:level :off}}}} + toucan.models/defmodel macros.toucan.models/defmodel + clojurewerkz.quartzite.jobs/build macros.quartz/build-job + clojurewerkz.quartzite.triggers/build macros.quartz/build-trigger + clojurewerkz.quartzite.schedule.cron/schedule macros.quartz/schedule + clojurewerkz.quartzite.schedule.simple/schedule macros.quartz/simple-schedule}} + + :config-in-comment + {:linters {:unresolved-symbol {:level :off}}}} diff --git a/.clj-kondo/hooks/metabase/models/setting.clj b/.clj-kondo/hooks/metabase/models/setting.clj new file mode 100644 index 0000000000000000000000000000000000000000..4bcc64c558763b3f0406498fafd49c616f0575e7 --- /dev/null +++ b/.clj-kondo/hooks/metabase/models/setting.clj @@ -0,0 +1,46 @@ +(ns hooks.metabase.models.setting + (:require [clj-kondo.hooks-api :as hooks])) + +(defn defsetting + "Rewrite a [[metabase.models.defsetting]] form like + + (defsetting my-setting \"Description\" :type :boolean) + + as + + (let [_ \"Description\" + _ :type + _ :boolean] + (defn my-setting \"Docstring.\" []) + (defn my-setting! \"Docstring.\" [_value-or-nil])) + + for linting purposes." + [{:keys [node]}] + (let [[setting-name & args] (rest (:children node)) + ;; (defn my-setting [] ...) + getter-node (-> (list + (hooks/token-node 'defn) + setting-name + (hooks/string-node "Docstring.") + (hooks/vector-node [])) + hooks/list-node + (with-meta (meta setting-name))) + ;; (defn my-setting! [_x] ...) + setter-node (-> (list + (hooks/token-node 'defn) + (with-meta + (hooks/token-node (symbol (str (:string-value setting-name) \!))) + (meta setting-name)) + (hooks/string-node "Docstring.") + (hooks/vector-node [(hooks/token-node '_value-or-nil)])) + hooks/list-node + (with-meta (meta setting-name)))] + {:node (-> (list + (hooks/token-node 'let) + ;; include description and the options map so they can get validated as well. + (hooks/vector-node (vec (interleave (repeat (hooks/token-node '_)) + args))) + getter-node + setter-node) + hooks/list-node + (with-meta (meta node)))})) diff --git a/.clj-kondo/hooks/metabase/test/util.clj b/.clj-kondo/hooks/metabase/test/util.clj new file mode 100644 index 0000000000000000000000000000000000000000..38c95b20802b04dc6a45ff9dbb7f6911d9737180 --- /dev/null +++ b/.clj-kondo/hooks/metabase/test/util.clj @@ -0,0 +1,30 @@ +(ns hooks.metabase.test.util + (:require [clj-kondo.hooks-api :as hooks])) + +(defn with-temporary-setting-values + "Rewrite a form like + + (with-temporary-setting-values [x 1, y 2] + ...) + + as one like + + (let [_ 1, _ 2] + ...) + + for analysis purposes." + [{:keys [node]}] + (let [[bindings & body] (rest (:children node)) + bindings (if (hooks/vector-node? bindings) + (hooks/vector-node (into [] + (mapcat (fn [[_token-name v]] + [(hooks/token-node '_) v])) + (partition 2 (:children bindings)))) + bindings)] + {:node (-> (list* + (hooks/token-node 'let) + bindings + body) + (with-meta (meta body)) + hooks/list-node + (with-meta (meta body)))})) diff --git a/.clj-kondo/macros/metabase/query_processor/streaming.clj b/.clj-kondo/macros/metabase/query_processor/streaming.clj index af76e51f84adc63160d1d262ef6af80b4ef37c69..ac2c4d8a0c0ebe1bdebd4220dc050087a5801536 100644 --- a/.clj-kondo/macros/metabase/query_processor/streaming.clj +++ b/.clj-kondo/macros/metabase/query_processor/streaming.clj @@ -1,4 +1,4 @@ -(ns metabase.query-processor.streaming) +(ns macros.metabase.query-processor.streaming) (defmacro streaming-response [[x y z] & body] `(clojure.core/let [~x ~y] diff --git a/.clj-kondo/macros/toucan/models.clj b/.clj-kondo/macros/toucan/models.clj index a68f1d87ea627dd94a1f4e526885e4742c1bfece..ce5921bfcc373febbad91833f39bc7692ecbed58 100644 --- a/.clj-kondo/macros/toucan/models.clj +++ b/.clj-kondo/macros/toucan/models.clj @@ -1,8 +1,6 @@ -(ns toucan.models) +(ns macros.toucan.models) -(defmacro defmodel [model-name & ks] - `(do (clojure.core/defrecord ~model-name ~(vec (map (comp symbol name) ks))) - (clojure.core/defrecord - ~(symbol (str model-name "Instance")) - ~(vec (map (comp symbol name) ks))) - ~(symbol (str model-name "Instance")))) +(defmacro defmodel [model-name & _args] + `(do + (def ~model-name "Docstring." nil) + (defrecord ~(symbol (str model-name "Instance")) []))) diff --git a/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/setting_test.clj b/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/setting_test.clj index 1caedf514e525689fa671bfa2cb58f31acd6eef6..2f5cbbd18d5097bd2d2bd5277ec0756c75232bf6 100644 --- a/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/setting_test.clj +++ b/enterprise/backend/test/metabase_enterprise/advanced_permissions/api/setting_test.clj @@ -7,7 +7,7 @@ [metabase.integrations.slack :as slack] [metabase.models :refer [Card Dashboard]] [metabase.models.permissions :as perms] - [metabase.models.setting-test :refer [test-setting-1 test-setting-2]] + [metabase.models.setting-test :as models.setting-test] [metabase.public-settings.premium-features-test :as premium-features-test] [metabase.test :as mt] [metabase.test.fixtures :as fixtures] @@ -18,9 +18,8 @@ (deftest setting-api-test (testing "/api/setting" - (mt/with-user-in-groups - [group {:name "New Group"} - user [group]] + (mt/with-user-in-groups [group {:name "New Group"} + user [group]] (letfn [(get-setting [user status] (testing (format "get setting with %s user" (mt/user-descriptor user)) (mt/user-http-request user :get status "setting"))) @@ -33,8 +32,8 @@ (testing (format "update multiple settings setting with %s user" (mt/user-descriptor user)) (mt/user-http-request user :put status "setting" {:test-setting-1 "ABC", :test-setting-2 "DEF"})))] ;; we focus on permissions in these tests, so set default value to make it easier to test - (test-setting-1 "ABC") - (test-setting-2 "DEF") + (models.setting-test/test-setting-1! "ABC") + (models.setting-test/test-setting-2! "DEF") (testing "if `advanced-permissions` is disabled, require admins" (premium-features-test/with-premium-features #{} diff --git a/enterprise/backend/test/metabase_enterprise/public_settings_test.clj b/enterprise/backend/test/metabase_enterprise/public_settings_test.clj index 00e0cca06b806bd271f6ab97819d3aa9d2147973..09c9c9ad19f05eaea6431b6d3f2f980cb866d058 100644 --- a/enterprise/backend/test/metabase_enterprise/public_settings_test.clj +++ b/enterprise/backend/test/metabase_enterprise/public_settings_test.clj @@ -10,7 +10,7 @@ (tu/with-temporary-setting-values [jwt-enabled true jwt-identity-provider-uri "example.com" jwt-shared-secret "0123456789012345678901234567890123456789012345678901234567890123" - enable-password-login true] - (public-settings/enable-password-login false) + enable-password-login true] + (public-settings/enable-password-login! false) (is (= false (public-settings/enable-password-login))))) diff --git a/src/metabase/analytics/snowplow.clj b/src/metabase/analytics/snowplow.clj index 71c7cc9c9332b9a9730d49b7b248e6b601d0a20b..06ac051dfc5cfa10d5b2f0e2b045a80e71391060 100644 --- a/src/metabase/analytics/snowplow.clj +++ b/src/metabase/analytics/snowplow.clj @@ -125,8 +125,8 @@ [] (new SelfDescribingJson (str "iglu:com.metabase/instance/jsonschema/" (schema->version ::instance)) - {"id" (analytics-uuid), - "version" {"tag" (:tag (public-settings/version))}, + {"id" (analytics-uuid) + "version" {"tag" (:tag (public-settings/version))} "token_features" (m/map-keys name (public-settings/token-features)) "created_at" (u.date/format (instance-creation))})) diff --git a/src/metabase/api/persist.clj b/src/metabase/api/persist.clj index 5ab583db3d8279f4e33d7680192a5ca6e81b102b..8a8b9842591b64bb2ea0c476eb3777ad6e001a9b 100644 --- a/src/metabase/api/persist.clj +++ b/src/metabase/api/persist.clj @@ -97,7 +97,7 @@ [:as {{:keys [hours], :as _body} :body}] {hours HoursInterval} (validation/check-has-application-permission :setting) - (public-settings/persisted-model-refresh-interval-hours hours) + (public-settings/persisted-model-refresh-interval-hours! hours) (task.persist-refresh/reschedule-refresh!) api/generic-204-no-content) @@ -106,7 +106,7 @@ [] (validation/check-has-application-permission :setting) (log/info (tru "Enabling model persistence")) - (public-settings/persisted-models-enabled true) + (public-settings/persisted-models-enabled! true) (task.persist-refresh/enable-persisting!) api/generic-204-no-content) @@ -130,11 +130,11 @@ [] (validation/check-has-application-permission :setting) (when (public-settings/persisted-models-enabled) - (try (public-settings/persisted-models-enabled false) + (try (public-settings/persisted-models-enabled! false) (disable-persisting) (catch Exception e ;; re-enable so can continue to attempt to clean up - (public-settings/persisted-models-enabled true) + (public-settings/persisted-models-enabled! true) (throw e)))) api/generic-204-no-content) diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj index 627053c1f21991aeed43c4615b74d2f0740a6dec..8d152ff748c4044f27bd4eb2aa2d063711af24b8 100644 --- a/src/metabase/api/setup.clj +++ b/src/metabase/api/setup.clj @@ -96,14 +96,14 @@ (defn- setup-set-settings! [_request {:keys [email site-name site-locale allow-tracking?]}] ;; set a couple preferences - (public-settings/site-name site-name) - (public-settings/admin-email email) + (public-settings/site-name! site-name) + (public-settings/admin-email! email) (when site-locale - (public-settings/site-locale site-locale)) + (public-settings/site-locale! site-locale)) ;; default to `true` if allow_tracking isn't specified. The setting will set itself correctly whether a boolean or ;; boolean string is specified - (public-settings/anon-tracking-enabled (or (nil? allow-tracking?) - allow-tracking?))) + (public-settings/anon-tracking-enabled! (or (nil? allow-tracking?) + allow-tracking?))) (api/defendpoint POST "/" "Special endpoint for creating the first user during setup. This endpoint both creates the user AND logs them in and diff --git a/src/metabase/api/slack.clj b/src/metabase/api/slack.clj index e6eab558f81cb6e99503d1625141ee37f50fc68f..ef8016469a636dddfdf5247994fd0a352d4b70a8 100644 --- a/src/metabase/api/slack.clj +++ b/src/metabase/api/slack.clj @@ -24,23 +24,23 @@ (when (and slack-app-token (not config/is-test?) (not (slack/valid-token? slack-app-token))) - (slack/slack-cached-channels-and-usernames []) + (slack/slack-cached-channels-and-usernames! []) (throw (ex-info (tru "Invalid Slack token.") {:errors {:slack-app-token (tru "invalid token")}}))) - (slack/slack-app-token slack-app-token) + (slack/slack-app-token! slack-app-token) (if slack-app-token - (do (slack/slack-token-valid? true) + (do (slack/slack-token-valid?! true) ;; Clear the deprecated `slack-token` when setting a new `slack-app-token` - (slack/slack-token nil) + (slack/slack-token! nil) ;; refresh user/conversation cache when token is newly valid (slack/refresh-channels-and-usernames-when-needed!)) ;; clear user/conversation cache when token is newly empty - (slack/slack-cached-channels-and-usernames [])) + (slack/slack-cached-channels-and-usernames! [])) (let [processed-files-channel (slack/process-files-channel-name slack-files-channel)] (when (and processed-files-channel (not (slack/channel-exists? processed-files-channel))) (throw (ex-info (tru "Slack channel not found.") {:errors {:slack-files-channel (tru "channel not found")}}))) - (slack/slack-files-channel processed-files-channel)) + (slack/slack-files-channel! processed-files-channel)) {:ok true} (catch clojure.lang.ExceptionInfo info {:status 400, :body (ex-data info)}))) diff --git a/src/metabase/core.clj b/src/metabase/core.clj index de8fdb7608c76a9383e8061e3166d95336dc3ec4..83078db5ad76f7c02e9fb7eed048a0fa806891f6 100644 --- a/src/metabase/core.clj +++ b/src/metabase/core.clj @@ -136,7 +136,7 @@ [] (let [start-time (t/zoned-date-time)] (init!*) - (public-settings/startup-time-millis + (public-settings/startup-time-millis! (.toMillis (t/duration start-time (t/zoned-date-time)))))) ;;; -------------------------------------------------- Normal Start -------------------------------------------------- diff --git a/src/metabase/driver/sql_jdbc/execute.clj b/src/metabase/driver/sql_jdbc/execute.clj index 9ae33b69dfd99168b603deb5f705bb758ff51875..774e7ebb2d3f5257cc2de2fc9fbfee36ebf0ca59 100644 --- a/src/metabase/driver/sql_jdbc/execute.clj +++ b/src/metabase/driver/sql_jdbc/execute.clj @@ -273,7 +273,7 @@ (set-parameter driver stmt (inc i) param)) params))) -(defsetting ^:private sql-jdbc-fetch-size +(defsetting sql-jdbc-fetch-size "Fetch size for result sets. We want to ensure that the jdbc ResultSet objects are not realizing the entire results in memory." :default 500 diff --git a/src/metabase/integrations/ldap.clj b/src/metabase/integrations/ldap.clj index 2f1e149d0c0c69758361e6da68c69d77179c1384..793b9a3585e92484a96233b3ca0967542866a368 100644 --- a/src/metabase/integrations/ldap.clj +++ b/src/metabase/integrations/ldap.clj @@ -130,7 +130,7 @@ (ldap/connect (settings->ldap-options))) (defn do-with-ldap-connection - "Impl for `with-ldap-connection` macro." + "Impl for [[with-ldap-connection]] macro." [f] (with-open [conn (get-connection)] (f conn))) diff --git a/src/metabase/integrations/slack.clj b/src/metabase/integrations/slack.clj index fe68ab13ad691335886d04e0cb18441973b669c3..e53f1b16ceeee155151b1025b5722a732a4ad060 100644 --- a/src/metabase/integrations/slack.clj +++ b/src/metabase/integrations/slack.clj @@ -90,7 +90,7 @@ ;; We should send an email if `slack-token-valid?` is `true` or `nil` (i.e. a pre-existing bot integration is ;; being used) (when (slack-token-valid?) (messages/send-slack-token-error-emails!)) - (slack-token-valid? false)) + (slack-token-valid?! false)) (if invalid-token? (log/warn (u/pprint-to-str 'red (trs "🔒 Your Slack authorization token is invalid or has been revoked. Please update your integration in Admin Settings -> Slack."))) (log/warn (u/pprint-to-str 'red error))) @@ -227,8 +227,8 @@ (log/info "Refreshing slack channels and usernames.") (let [users (future (vec (users-list))) conversations (future (vec (conversations-list)))] - (slack-cached-channels-and-usernames (concat @conversations @users)) - (slack-channels-and-usernames-last-updated (t/zoned-date-time))))) + (slack-cached-channels-and-usernames! (concat @conversations @users)) + (slack-channels-and-usernames-last-updated! (t/zoned-date-time))))) (defn refresh-channels-and-usernames-when-needed! "Refreshes users and conversations in slack-cache on a per-instance lock." diff --git a/src/metabase/models/humanization.clj b/src/metabase/models/humanization.clj index 4e600abc743cdd6597d10ed4d84b2035c920e3a2..7f62b0cbba173a2d925fe12e6b0389bd49e30f42 100644 --- a/src/metabase/models/humanization.clj +++ b/src/metabase/models/humanization.clj @@ -23,7 +23,7 @@ strategy defined by the Setting `humanization-strategy`. With two args, you may specify a custom strategy (intended mainly for the internal implementation): - (humanization-strategy :simple) + (humanization-strategy! :simple) (name->human-readable-name \"cool_toucans\") ;-> \"Cool Toucans\" ;; this is the same as: (name->human-readable-name (humanization-strategy) \"cool_toucans\") ;-> \"Cool Toucans\" @@ -87,7 +87,6 @@ (doseq [model ['Table 'Field]] (re-humanize-names! old-strategy model))) - (defn- set-humanization-strategy! [new-value] (let [new-strategy (keyword (or new-value :simple))] ;; check to make sure `new-strategy` is a valid strategy, or throw an Exception it is it not. diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index ea90e1344cf14a809f121785892e5490f99cef76..4ba9397dead4c9c2dec04b198f244c4a32f834da 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -21,10 +21,10 @@ ; OR the default value, if any (in that order) (setting/set! :mandrill-api-key \"NEW_KEY\") - (mandrill-api-key \"NEW_KEY\") + (mandrill-api-key! \"NEW_KEY\") (setting/set! :mandrill-api-key nil) - (mandrill-api-key nil) + (mandrill-api-key! nil) You can define additional Settings types adding implementations of [[default-tag-for-type]], [[get-value-of-type]], and [[set-value-of-type!]]. @@ -761,48 +761,48 @@ ;;; | defsetting macro | ;;; +----------------------------------------------------------------------------------------------------------------+ -(defn metadata-for-setting-fn - "Create metadata for the function automatically generated by [[defsetting]]." - [{:keys [default description tag deprecated], setting-type :type, :as setting}] - {:arglists (list - (with-meta [] {:tag tag}) - (with-meta '[new-value] {:tag tag})) - - ;; indentation below is intentional to make it clearer what shape the generated documentation is going to take. - ;; Turn on auto-complete-mode in Emacs and see for yourself! +(defn- setting-fn-docstring [{:keys [default description], setting-type :type, :as setting}] + ;; indentation below is intentional to make it clearer what shape the generated documentation is going to take. + (str + description \newline + \newline + (format "`%s` is a `%s` Setting. You can get its value by calling:\n" (setting-name setting) setting-type) + \newline + (format " (%s)\n" (setting-name setting)) + \newline + "and set its value by calling:\n" + \newline + (format " (%s! <new-value>)\n" (setting-name setting)) + \newline + (format "You can also set its value with the env var `%s`.\n" (env-var-name setting)) + \newline + "Clear its value by calling:\n" + \newline + (format " (%s! nil)\n" (setting-name setting)) + \newline + (format "Its default value is `%s`." (pr-str default)))) + +(defn setting-fn-metadata + "Impl for [[defsetting]]. Create metadata for [[setting-fn]]." + [getter-or-setter {:keys [tag deprecated], :as setting}] + {:arglists (case getter-or-setter + :getter (list (with-meta [] {:tag tag})) + :setter (list (with-meta '[new-value] {:tag tag}))) :deprecated deprecated - :doc (str/join - "\n" - [ description - "" - (format "`%s` is a %s Setting. You can get its value by calling:" (setting-name setting) (name setting-type)) - "" - (format " (%s)" (setting-name setting)) - "" - "and set its value by calling:" - "" - (format " (%s <new-value>)" (setting-name setting)) - "" - (format "You can also set its value with the env var `%s`." (env-var-name setting)) - "" - "Clear its value by calling:" - "" - (format " (%s nil)" (setting-name setting)) - "" - (format "Its default value is `%s`." (pr-str default))])}) + :doc (setting-fn-docstring setting)}) (defn setting-fn - "Create the automatically defined getter/setter function for settings defined by [[defsetting]]." - [setting] - (fn - ([] - (get setting)) - - ([new-value] - ;; need to qualify this or otherwise the reader gets this confused with the set! used for things like - ;; (set! *warn-on-reflection* true) - ;; :refer-clojure :exclude doesn't seem to work in this case - (metabase.models.setting/set! setting new-value)))) + "Impl for [[defsetting]]. Create the automatically defined `getter-or-setter` function for Settings defined + by [[defsetting]]." + [getter-or-setter setting] + (case getter-or-setter + :getter (fn setting-getter* [] + (get setting)) + :setter (fn setting-setter* [new-value] + ;; need to qualify this or otherwise the reader gets this confused with the set! used for things like + ;; (set! *warn-on-reflection* true) + ;; :refer-clojure :exclude doesn't seem to work in this case + (metabase.models.setting/set! setting new-value)))) ;; The next few functions are for validating the Setting description (i.e., docstring) at macroexpansion time. They ;; check that the docstring is a valid deferred i18n form (e.g. [[metabase.util.i18n/deferred-tru]]) so the Setting @@ -835,7 +835,7 @@ (and (seq exprs-without-strs) (every? valid-trs-or-tru? exprs-without-strs))))) -(defn- validate-description +(defn- validate-description-form "Check that `description-form` is a i18n form (e.g. [[metabase.util.i18n/deferred-tru]]), or a [[str]] form consisting of one or more deferred i18n forms. Returns `description-form` as-is." [description-form] @@ -855,9 +855,9 @@ Conveniently can be used as a getter/setter as well (defsetting mandrill-api-key (trs \"API key for Mandrill.\")) - (mandrill-api-key) ; get the value - (mandrill-api-key new-value) ; update the value - (mandrill-api-key nil) ; delete the value + (mandrill-api-key) ; get the value + (mandrill-api-key! new-value) ; update the value + (mandrill-api-key! nil) ; delete the value A setting can be set from the Admin Panel or via the corresponding env var, eg. `MB_MANDRILL_API_KEY` for the example above. @@ -918,18 +918,33 @@ `new` and calls it with the old and new settings values. By default, the :on-change will be missing, and nothing will happen, in [[call-on-change]] below." {:style/indent 1} - [setting-symb description & {:as options}] - {:pre [(symbol? setting-symb)]} - `(let [desc# ~(if (or (= (:visibility options) :internal) - (= (:setter options) :none)) - description - (validate-description description)) - setting# (register-setting! (assoc ~options - :name ~(keyword setting-symb) - :description desc# - :namespace (ns-name *ns*)))] - (-> (def ~setting-symb (setting-fn setting#)) - (alter-meta! merge (metadata-for-setting-fn setting#))))) + [setting-symbol description & {:as options}] + {:pre [(symbol? setting-symbol) + (not (namespace setting-symbol)) + ;; don't put exclamation points in your Setting names. We don't want functions like `exciting!` for the getter + ;; and `exciting!!` for the setter. + (not (str/includes? (name setting-symbol) "!"))]} + (let [description (if (or (= (:visibility options) :internal) + (= (:setter options) :none)) + description + (validate-description-form description)) + definition-form (assoc options + :name (keyword setting-symbol) + :description description + :namespace (list 'quote (ns-name *ns*))) + ;; create symbols for the getter and setter functions e.g. `my-setting` and `my-setting!` respectively. + ;; preserve metadata from the `setting-symbol` passed to `defsetting`. + setting-getter-fn-symbol setting-symbol + setting-setter-fn-symbol (-> (symbol (str (name setting-symbol) \!)) + (with-meta (meta setting-symbol))) + ;; create a symbol for the Setting definition from [[register-setting!]] + setting-definition-symbol (gensym "setting-")] + `(let [~setting-definition-symbol (register-setting! ~definition-form)] + (-> (def ~setting-getter-fn-symbol (setting-fn :getter ~setting-definition-symbol)) + (alter-meta! merge (setting-fn-metadata :getter ~setting-definition-symbol))) + ~(when-not (= (:setter options) :none) + `(-> (def ~setting-setter-fn-symbol (setting-fn :setter ~setting-definition-symbol)) + (alter-meta! merge (setting-fn-metadata :setter ~setting-definition-symbol))))))) ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj index 41c6db511e93f5eccb55ef747bcb8c3e890e0216..009fe679a8231494bdf7a7532049caae637016ef 100644 --- a/src/metabase/public_settings.clj +++ b/src/metabase/public_settings.clj @@ -113,7 +113,7 @@ (throw (ex-info (tru "Invalid site URL: {0}" (pr-str s)) {:url (pr-str s)}))) s)) -(declare redirect-all-requests-to-https) +(declare redirect-all-requests-to-https!) ;; This value is *guaranteed* to never have a trailing slash :D ;; It will also prepend `http://` to the URL if there's no protocol when it comes in @@ -132,7 +132,7 @@ https? (some-> new-value (str/starts-with? "https:"))] ;; if the site URL isn't HTTPS then disable force HTTPS redirects if set (when-not https? - (redirect-all-requests-to-https false)) + (redirect-all-requests-to-https! false)) (setting/set-value-of-type! :string :site-url new-value)))) (defsetting site-locale diff --git a/src/metabase/server/middleware/misc.clj b/src/metabase/server/middleware/misc.clj index 1ff8f10f30bb777e55a7b36bd1078d8a4ab652e6..eca3e20bb1c73da3595b939b8814624fd3195df4 100644 --- a/src/metabase/server/middleware/misc.clj +++ b/src/metabase/server/middleware/misc.clj @@ -47,7 +47,7 @@ (when-let [site-url (or origin x-forwarded-host host)] (log/info (trs "Setting Metabase site URL to {0}" site-url)) (try - (public-settings/site-url site-url) + (public-settings/site-url! site-url) (catch Throwable e (log/warn e (trs "Failed to set site-url"))))))) diff --git a/src/metabase/task/follow_up_emails.clj b/src/metabase/task/follow_up_emails.clj index 05c3308aed292faf03451b464c7bba4e1675cdcf..8f9d0af1c48772fe748bb734a3eef8572d11face 100644 --- a/src/metabase/task/follow_up_emails.clj +++ b/src/metabase/task/follow_up_emails.clj @@ -45,7 +45,7 @@ (catch Throwable e (log/error "Problem sending follow-up email:" e)) (finally - (follow-up-email-sent true)))))) + (follow-up-email-sent! true)))))) (defn- instance-creation-timestamp "The date this Metabase instance was created. We use the `:date_joined` of the first `User` to determine this." @@ -124,7 +124,7 @@ (catch Throwable e (log/error e (trs "Problem sending abandonment email"))) (finally - (abandonment-email-sent true)))))) + (abandonment-email-sent! true)))))) (jobs/defjob diff --git a/src/metabase/task/upgrade_checks.clj b/src/metabase/task/upgrade_checks.clj index 890456dafdec4606d172448cc477e59a563bb7cf..1695a1e6d8840cc5140a5e99ad756845d5c12846 100644 --- a/src/metabase/task/upgrade_checks.clj +++ b/src/metabase/task/upgrade_checks.clj @@ -25,12 +25,12 @@ (log/debug (trs "Checking for new Metabase version info.")) (try ;; TODO: add in additional request params if anonymous tracking is enabled - (public-settings/version-info-last-checked (t/zoned-date-time)) + (public-settings/version-info-last-checked! (t/zoned-date-time)) (when-let [version-info (get-version-info)] - (public-settings/version-info version-info)) + (public-settings/version-info! version-info)) (catch Throwable e (log/error e (trs "Error fetching version info; setting version-info value to nil")) - (public-settings/version-info nil))))) + (public-settings/version-info! nil))))) (def ^:private job-key "metabase.task.upgrade-checks.job") (def ^:private trigger-key "metabase.task.upgrade-checks.trigger") diff --git a/src/metabase/util.clj b/src/metabase/util.clj index 34da2e29856e7944a46e9a7b2d4ef0662f7c7678..ed3f59bf9db18bb0a689664c499a5038744e00b8 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -188,27 +188,6 @@ (.isReachable host-addr host-up-timeout)) (catch Throwable _ false))) -(defn ^:deprecated rpartial - "Like `partial`, but applies additional args *before* `bound-args`. - Inspired by [`-rpartial` from dash.el](https://github.com/magnars/dash.el#-rpartial-fn-rest-args) - - ((partial - 5) 8) -> (- 5 8) -> -3 - ((rpartial - 5) 8) -> (- 8 5) -> 3 - - DEPRECATED: just use `#()` function literals instead. No need to be needlessly confusing." - [f & bound-args] - (fn [& args] - (apply f (concat args bound-args)))) - -(defmacro pdoseq - "(Almost) just like `doseq` but runs in parallel. Doesn't support advanced binding forms like `:let` or `:when` and - only supports a single binding </3" - {:style/indent 1} - [[binding collection] & body] - `(dorun (pmap (fn [~binding] - ~@body) - ~collection))) - (defmacro prog1 "Execute `first-form`, then any other expressions in `body`, presumably for side-effects; return the result of `first-form`. diff --git a/src/metabase/util/embed.clj b/src/metabase/util/embed.clj index a9a94811a76dab6080c38c6ef9460d49f7605cb8..ac1243497e385dfd7e82ff045f58b17fd8bbc176 100644 --- a/src/metabase/util/embed.clj +++ b/src/metabase/util/embed.clj @@ -14,7 +14,7 @@ ;;; --------------------------------------------- PUBLIC LINKS UTIL FNS ---------------------------------------------- (defn- oembed-url - "Return an oEmbed URL for the RELATIVE-PATH. + "Return an oEmbed URL for the `relative-path`. (oembed-url \"/x\") -> \"http://localhost:3000/api/public/oembed?url=x&format=json\"" ^String [^String relative-url] @@ -52,7 +52,7 @@ ;;; ----------------------------------------------- EMBEDDING UTIL FNS ----------------------------------------------- -(setting/defsetting ^:private embedding-secret-key +(setting/defsetting embedding-secret-key (deferred-tru "Secret key used to sign JSON Web Tokens for requests to `/api/embed` endpoints.") :visibility :admin :setter (fn [new-value] @@ -62,7 +62,7 @@ (setting/set-value-of-type! :string :embedding-secret-key new-value))) (defn- jwt-header - "Parse a JWT MESSAGE and return the header portion." + "Parse a JWT `message` and return the header portion." [^String message] (let [[header] (str/split message #"\.")] (json/parse-string (codecs/bytes->str (codec/base64-decode header)) keyword))) diff --git a/test/metabase/api/geojson_test.clj b/test/metabase/api/geojson_test.clj index 54d289071537d30765716f28a26394c97a715285..648d5681c9fa62e912a1f942e0b48ec25077d0ae 100644 --- a/test/metabase/api/geojson_test.clj +++ b/test/metabase/api/geojson_test.clj @@ -98,7 +98,7 @@ (is (= built-in (api.geojson/custom-geojson)))) (testing "Try to change one of the built-in entries..." - (api.geojson/custom-geojson (assoc-in built-in [:us_states :name] "USA")) + (api.geojson/custom-geojson! (assoc-in built-in [:us_states :name] "USA")) (testing "Value should not have actually changed" (is (= built-in (api.geojson/custom-geojson))))))))) diff --git a/test/metabase/api/pulse_test.clj b/test/metabase/api/pulse_test.clj index f9d694454e442fcdb6a72179b5a8ddcb21a36bc7..733a7d081846b44a7e6833eb0574ad629cfdb1c4 100644 --- a/test/metabase/api/pulse_test.clj +++ b/test/metabase/api/pulse_test.clj @@ -974,9 +974,9 @@ (with-redefs [slack/conversations-list (constantly ["#foo" "#two" "#general"]) slack/users-list (constantly ["@bar" "@baz"])] ;; set the cache to these values - (slack/slack-cached-channels-and-usernames (concat (slack/conversations-list) (slack/users-list))) + (slack/slack-cached-channels-and-usernames! (concat (slack/conversations-list) (slack/users-list))) ;; don't let the cache refresh itself (it will refetch if it is too old) - (slack/slack-channels-and-usernames-last-updated (t/zoned-date-time)) + (slack/slack-channels-and-usernames-last-updated! (t/zoned-date-time)) (is (= [{:name "channel", :type "select", :displayName "Post to", :options ["#foo" "#two" "#general" "@bar" "@baz"], :required true}] (-> (mt/user-http-request :rasta :get 200 "pulse/form_input") (get-in [:channels :slack :fields]))))))) diff --git a/test/metabase/api/setting_test.clj b/test/metabase/api/setting_test.clj index b365b0fa6c0e26530b50b2b39aeef319cb11cb9c..1cc67632a197dd80bcc3b24fb61596f7d29dab68 100644 --- a/test/metabase/api/setting_test.clj +++ b/test/metabase/api/setting_test.clj @@ -1,14 +1,7 @@ (ns metabase.api.setting-test (:require [clojure.test :refer :all] [metabase.models.setting :as setting :refer [defsetting]] - [metabase.models.setting-test - :refer - [test-sensitive-setting - test-setting-1 - test-setting-2 - test-setting-3 - test-user-local-allowed-setting - test-user-local-only-setting]] + [metabase.models.setting-test :as models.setting-test] [metabase.public-settings.premium-features-test :as premium-features-test] [metabase.test :as mt] [metabase.test.fixtures :as fixtures] @@ -40,7 +33,6 @@ :when (re-find #"^test-setting-\d$" (name (:key setting)))] setting)) - (defn- fetch-setting "Fetch a single setting." ([setting-name status] @@ -52,9 +44,9 @@ (deftest fetch-setting-test (testing "GET /api/setting" (testing "Check that we can fetch all Settings, except `:visiblity :internal` ones" - (test-setting-1 nil) - (test-setting-2 "FANCY") - (test-setting-3 "oh hai") ; internal setting that should not be returned + (models.setting-test/test-setting-1! nil) + (models.setting-test/test-setting-2! "FANCY") + (models.setting-test/test-setting-3! "oh hai") ; internal setting that should not be returned (is (= [{:key "test-setting-1" :value nil :is_env_setting false @@ -75,7 +67,7 @@ (testing "GET /api/setting/:key" (testing "Test that we can fetch a single setting" - (test-setting-2 "OK!") + (models.setting-test/test-setting-2! "OK!") (is (= "OK!" (fetch-setting :test-setting-2 200)))) @@ -84,17 +76,15 @@ (fetch-setting :rasta :test-setting-2 403)))) (testing "non-string values work over the api (#20735)" ;; n.b. the api will return nil if a setting is its default value. - (test-api-setting-double 3.14) + (test-api-setting-double! 3.14) (is (= 3.14 (fetch-setting :test-api-setting-double 200))) - (test-api-setting-boolean true) + (test-api-setting-boolean! true) (is (= true (fetch-setting :test-api-setting-boolean 200))) - (test-api-setting-integer 42) + (test-api-setting-integer! 42) (is (= 42 (fetch-setting :test-api-setting-integer 200)))))) -(test-api-setting-integer) - (deftest fetch-calculated-settings-test (testing "GET /api/setting" (testing "Should return the correct `:value` for Settings with no underlying DB/env var value" @@ -113,7 +103,7 @@ (deftest fetch-internal-settings-test (testing "Test that we can't fetch internal settings" - (test-setting-3 "NOPE!") + (models.setting-test/test-setting-3! "NOPE!") (is (= "Setting :test-setting-3 is internal" (:message (fetch-setting :test-setting-3 500)))))) @@ -121,7 +111,7 @@ (testing "PUT /api/setting/:key" (mt/user-http-request :crowberto :put 204 "setting/test-setting-1" {:value "NICE!"}) (is (= "NICE!" - (test-setting-1)) + (models.setting-test/test-setting-1)) "Updated setting should be visible from setting getter") (is (= "NICE!" @@ -139,12 +129,12 @@ (deftest fetch-sensitive-setting-test (testing "Sensitive settings should always come back obfuscated" (testing "GET /api/setting/:name" - (test-sensitive-setting "ABCDEF") + (models.setting-test/test-sensitive-setting! "ABCDEF") (is (= "**********EF" (fetch-setting :test-sensitive-setting 200)))) (testing "GET /api/setting" - (test-sensitive-setting "GHIJKLM") + (models.setting-test/test-sensitive-setting! "GHIJKLM") (is (= {:key "test-sensitive-setting" :value "**********LM" :is_env_setting false @@ -161,34 +151,34 @@ "should *not* obfuscate sensitive Setting values -- that should be done by the API") (mt/user-http-request :crowberto :put 204 "setting/test-sensitive-setting" {:value "123456"}) (is (= "123456" - (test-sensitive-setting)))) + (models.setting-test/test-sensitive-setting)))) (testing "Attempts to set the Setting to an obfuscated value should be ignored" (testing "PUT /api/setting/:name" - (test-sensitive-setting "123456") + (models.setting-test/test-sensitive-setting! "123456") (is (= nil (mt/user-http-request :crowberto :put 204 "setting/test-sensitive-setting" {:value "**********56"}))) (is (= "123456" - (test-sensitive-setting)))) + (models.setting-test/test-sensitive-setting)))) (testing "PUT /api/setting" - (test-sensitive-setting "123456") + (models.setting-test/test-sensitive-setting! "123456") (is (= nil (mt/user-http-request :crowberto :put 204 "setting" {:test-sensitive-setting "**********56"}))) (is (= "123456" - (test-sensitive-setting)))))) + (models.setting-test/test-sensitive-setting)))))) -;; there are additional tests for this functionality in `metabase.model.setting-test/set-many!-test`, since this API -;; endpoint is just a thin wrapper around that function +;; there are additional tests for this functionality in [[metabase.models.models.setting-test/set-many!-test]], since +;; this API endpoint is just a thin wrapper around that function (deftest update-multiple-settings-test (testing "PUT /api/setting/" (testing "admin should be able to update multiple settings at once" (is (= nil (mt/user-http-request :crowberto :put 204 "setting" {:test-setting-1 "ABC", :test-setting-2 "DEF"}))) (is (= "ABC" - (test-setting-1))) + (models.setting-test/test-setting-1))) (is (= "DEF" - (test-setting-2)))) + (models.setting-test/test-setting-2)))) (testing "non-admin should not be able to update multiple settings at once if any of them are not user-local" (is (= "You don't have permissions to do that." @@ -203,19 +193,19 @@ (defn- set-initial-user-local-values [] (mt/with-current-user (mt/user->id :crowberto) - (test-user-local-only-setting "ABC") - (test-user-local-allowed-setting "ABC")) + (models.setting-test/test-user-local-only-setting! "ABC") + (models.setting-test/test-user-local-allowed-setting! "ABC")) (mt/with-current-user (mt/user->id :rasta) - (test-user-local-only-setting "DEF") - (test-user-local-allowed-setting "DEF"))) + (models.setting-test/test-user-local-only-setting! "DEF") + (models.setting-test/test-user-local-allowed-setting! "DEF"))) (defn- clear-user-local-values [] (mt/with-current-user (mt/user->id :crowberto) - (test-user-local-only-setting nil) - (test-user-local-allowed-setting nil)) + (models.setting-test/test-user-local-only-setting! nil) + (models.setting-test/test-user-local-allowed-setting! nil)) (mt/with-current-user (mt/user->id :rasta) - (test-user-local-only-setting nil) - (test-user-local-allowed-setting nil))) + (models.setting-test/test-user-local-only-setting! nil) + (models.setting-test/test-user-local-allowed-setting! nil))) (deftest user-local-settings-test (testing "GET /api/setting/" @@ -288,9 +278,9 @@ (testing "if a non-admin tries to set multiple settings and any aren't user-local, none are updated" (set-initial-user-local-values) - (test-setting-1 "ABC") + (models.setting-test/test-setting-1! "ABC") (mt/user-http-request :rasta :put 403 "setting" {:test-user-local-only-setting "MNO" :test-setting-1 "PQR"}) (is (= "DEF" (mt/with-current-user (mt/user->id :rasta) - (test-user-local-only-setting)))) - (is (= "ABC" (test-setting-1)))))) + (models.setting-test/test-user-local-only-setting)))) + (is (= "ABC" (models.setting-test/test-setting-1)))))) diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj index f35184c6f84cd81e981b164890b2e265052b0c09..23203a92df257bd8ec8b6da8a75428d01ebfaf2e 100644 --- a/test/metabase/api/setup_test.clj +++ b/test/metabase/api/setup_test.clj @@ -289,28 +289,26 @@ (deftest create-superuser-only-once-test (testing "POST /api/setup" (testing "Check that we cannot create a new superuser via setup-token when a user exists" - (let [token (setup/create-token!) - body {:token token - :prefs {:site_locale "es_MX" - :site_name (mt/random-name)} - :user {:first_name (mt/random-name) - :last_name (mt/random-name) - :email (mt/random-email) - :password "p@ssword1"}}] - (with-redefs [setup/has-user-setup (let [value (atom false)] - (fn - ([] @value) - ([t-or-f] (reset! value t-or-f))))] - (mt/discard-setting-changes - [site-name site-locale anon-tracking-enabled admin-email] - (client/client :post 200 "setup" body)) - + (let [token (setup/create-token!) + body {:token token + :prefs {:site_locale "es_MX" + :site_name (mt/random-name)} + :user {:first_name (mt/random-name) + :last_name (mt/random-name) + :email (mt/random-email) + :password "p@ssword1"}} + has-user-setup (atom false)] + (with-redefs [setup/has-user-setup (fn [] @has-user-setup)] + (is (not (setup/has-user-setup))) + (mt/discard-setting-changes [site-name site-locale anon-tracking-enabled admin-email] + (is (schema= {:id client/UUIDString} + (client/client :post 200 "setup" body)))) ;; In the non-test context, this is 'set' iff there is one or more users, and doesn't have to be toggled - (setup/has-user-setup true) - - (mt/discard-setting-changes - [site-name site-locale anon-tracking-enabled admin-email] - (client/client :post 403 "setup" (assoc-in body [:user :email] (mt/random-email))))))))) + (reset! has-user-setup true) + (is (setup/has-user-setup)) + (mt/discard-setting-changes [site-name site-locale anon-tracking-enabled admin-email] + (is (= "The /api/setup route can only be used to create the first user, however a user currently exists." + (client/client :post 403 "setup" (assoc-in body [:user :email] (mt/random-email))))))))))) (deftest transaction-test (testing "POST /api/setup/" diff --git a/test/metabase/automagic_dashboards/core_test.clj b/test/metabase/automagic_dashboards/core_test.clj index 090930c9daab6a4eee765dc02a8d37f81c1bdbd6..6a7077be194cf6efa62c3db71fafad0864f654f4 100644 --- a/test/metabase/automagic_dashboards/core_test.clj +++ b/test/metabase/automagic_dashboards/core_test.clj @@ -484,8 +484,7 @@ ;;; ------------------- Datetime humanization (for chart and dashboard titles) ------------------- (deftest temporal-humanization-test - (let [tz "UTC" - dt #t "1990-09-09T12:30" + (let [dt #t "1990-09-09T12:30" t-str "1990-09-09T12:30:00"] (doseq [[unit expected] {:minute (tru "at {0}" (t/format "h:mm a, MMMM d, YYYY" dt)) :hour (tru "at {0}" (t/format "h a, MMMM d, YYYY" dt)) diff --git a/test/metabase/integrations/google_test.clj b/test/metabase/integrations/google_test.clj index 7afd9f8d06c368803d9436bafb5e79f58e0d19c8..0d47f48673209c5d4ba75527b66855af783f3006 100644 --- a/test/metabase/integrations/google_test.clj +++ b/test/metabase/integrations/google_test.clj @@ -16,14 +16,14 @@ (is (thrown-with-msg? clojure.lang.ExceptionInfo #"Invalid Google Sign-In Client ID: must end with \".apps.googleusercontent.com\"" - (google/google-auth-client-id "invalid-client-id")))) + (google/google-auth-client-id! "invalid-client-id")))) (testing "Trailing whitespace in client ID is stripped upon save" - (google/google-auth-client-id "test-client-id.apps.googleusercontent.com ") + (google/google-auth-client-id! "test-client-id.apps.googleusercontent.com ") (is (= "test-client-id.apps.googleusercontent.com" (google/google-auth-client-id)))) (testing "Saving an empty string will clear the client ID setting" - (google/google-auth-client-id "") + (google/google-auth-client-id! "") (is (= nil (google/google-auth-client-id)))))) @@ -43,7 +43,7 @@ (with-redefs [premium-features/enable-sso? (constantly false)] (is (thrown? clojure.lang.ExceptionInfo - (google.i/google-auth-auto-create-accounts-domain "metabase.com, example.com")))))) + (google.i/google-auth-auto-create-accounts-domain! "metabase.com, example.com")))))) (deftest google-auth-create-new-user!-test (with-redefs [premium-features/enable-sso? (constantly false)] diff --git a/test/metabase/models/humanization_test.clj b/test/metabase/models/humanization_test.clj index 72adb05ebdc929ea2fc0680dca31008d046a3992..f66bb840e82c7b60cf5a857eca39cba136de6255 100644 --- a/test/metabase/models/humanization_test.clj +++ b/test/metabase/models/humanization_test.clj @@ -120,11 +120,11 @@ (is (= (:initial expected) (display-name)))) (testing "switch to :simple" - (humanization/humanization-strategy "simple") + (humanization/humanization-strategy! "simple") (is (= (:simple expected) (display-name)))) (testing "switch to :none" - (humanization/humanization-strategy "none") + (humanization/humanization-strategy! "none") (is (= (:none expected) (display-name)))))))))) @@ -135,6 +135,6 @@ (tt/with-temp Table [{table-id :id} {:name "toucansare_cool", :display_name "My Favorite Table"}] (doseq [new-strategy ["simple" "none"]] (testing (format "switch from %s -> %s" initial-strategy new-strategy) - (humanization/humanization-strategy new-strategy) + (humanization/humanization-strategy! new-strategy) (is (= "My Favorite Table" (db/select-one-field :display_name Table, :id table-id)))))))))) diff --git a/test/metabase/models/setting/cache_test.clj b/test/metabase/models/setting/cache_test.clj index a3c2538d623ddf4db73d757fd120a8885d338900..36aaa50be6c6b7fb1e85334499df6990e8abcd55 100644 --- a/test/metabase/models/setting/cache_test.clj +++ b/test/metabase/models/setting/cache_test.clj @@ -46,7 +46,7 @@ (deftest update-settings-last-updated-test (testing "When I update a Setting, does it set/update `settings-last-updated`?" (setting-test/clear-settings-last-updated-value-in-db!) - (setting-test/toucan-name "Bird Can") + (setting-test/toucan-name! "Bird Can") (is (string? (setting-test/settings-last-updated-value-in-db))) (testing "...and is the value updated in the cache as well?" @@ -57,7 +57,7 @@ ;; MySQL only has the resolution of one second on the timestamps here so we should wait that long to make sure ;; the second-value actually ends up being greater than the first (Thread/sleep (if (= (mdb/db-type) :mysql) 1200 50)) - (setting-test/toucan-name "Bird Can") + (setting-test/toucan-name! "Bird Can") (let [second-value (setting-test/settings-last-updated-value-in-db)] ;; first & second values should be different, and first value should be "less than" the second value (is (not= first-value second-value)) @@ -70,13 +70,13 @@ (testing "But if I set a setting, it should cause the cache to be populated, and be up-to-date" (clear-cache!) - (setting-test/toucan-name "Reggae Toucan") + (setting-test/toucan-name! "Reggae Toucan") (is (= false (#'setting.cache/cache-out-of-date?)))) (testing "If another instance updates a Setting, `cache-out-of-date?` should return `true` based on DB comparisons..." (clear-cache!) - (setting-test/toucan-name "Reggae Toucan") + (setting-test/toucan-name! "Reggae Toucan") (simulate-another-instance-updating-setting! :toucan-name "Bird Can") (is (= true (#'setting.cache/cache-out-of-date?))))) @@ -86,7 +86,7 @@ "updated right away even if another instance updates a value...") (reset-last-update-check!) (clear-cache!) - (setting-test/toucan-name "Sam") + (setting-test/toucan-name! "Sam") ;; should restore cache, and put in {"setting-test/toucan-name" "Sam"} (is (= "Sam" (setting-test/toucan-name))) @@ -101,7 +101,7 @@ ;; be invalidated, so we will manually flush the memoization cache to simulate it happening) (deftest sync-test-1 (clear-cache!) - (setting-test/toucan-name "Reggae Toucan") + (setting-test/toucan-name! "Reggae Toucan") (simulate-another-instance-updating-setting! :toucan-name "Bird Can") (is (= "Bird Can" (db/select-one-field :value Setting :key "toucan-name"))) @@ -126,18 +126,18 @@ (let [external-cache (constantly (atom nil))] (clear-cache!) (reset-last-update-check!) - (setting-test/test-setting-1 "Starfish") + (setting-test/test-setting-1! "Starfish") ;; 1. User writes (with-redefs [setting.cache/cache* external-cache] - (setting-test/toucan-name "Batman Toucan")) - (setting-test/test-setting-1 "Batman") + (setting-test/toucan-name! "Batman Toucan")) + (setting-test/test-setting-1! "Batman") (is (= "Batman Toucan" (setting-test/toucan-name))))) (deftest sync-test-3 (mt/discard-setting-changes [site-locale] (clear-cache!) - (public-settings/site-locale "en") + (public-settings/site-locale! "en") (simulate-another-instance-updating-setting! :site-locale "fr") (reset-last-update-check!) (is (= "fr" diff --git a/test/metabase/models/setting/multi_setting_test.clj b/test/metabase/models/setting/multi_setting_test.clj index ba0f6c9e1a4589d4b6239731a631bcfc52f329aa..721908f3379e1428287cb8e0719a501a797cc52e 100644 --- a/test/metabase/models/setting/multi_setting_test.clj +++ b/test/metabase/models/setting/multi_setting_test.clj @@ -21,6 +21,12 @@ :getter (partial setting/get-value-of-type :string :multi-setting-test-bird-name) :setter (partial setting/set-value-of-type! :string :multi-setting-test-bird-name)) +(deftest preserve-metadata-test + (testing "define-multi-setting should preserve metadata on the setting symbol in the getter/setter functions" + (doseq [varr [#'multi-setting-test-bird-name #'multi-setting-test-bird-name!]] + (testing (format "\nvar = %s" (pr-str varr)) + (is (:private (meta varr))))))) + (deftest multi-setting-test (testing :green-friend (is (= "Green Friend" @@ -28,11 +34,11 @@ (is (thrown-with-msg? UnsupportedOperationException #"You cannot set :multi-setting-test-bird-name; it is a read-only setting" - (multi-setting-test-bird-name "Parroty")))) + (multi-setting-test-bird-name! "Parroty")))) (testing :yellow-friend (binding [*parakeet* :yellow-friend] (is (= "Yellow Friend" - (multi-setting-test-bird-name "Yellow Friend"))) + (multi-setting-test-bird-name! "Yellow Friend"))) (is (= "Yellow Friend" (multi-setting-test-bird-name)))))) @@ -58,7 +64,10 @@ (binding [*parakeet* parakeet] (is (= "Parroty" (multi-setting-read-only))) - (is (thrown-with-msg? - UnsupportedOperationException - #"You cannot set multi-setting-read-only; it is a read-only setting" - (multi-setting-read-only "Parroty")))))))) + (testing "No setter function should have been defined" + (is (not (resolve 'multi-setting-read-only!)))) + (testing "Should not be able to set the Setting with `setting/set!`" + (is (thrown-with-msg? + UnsupportedOperationException + #"You cannot set multi-setting-read-only; it is a read-only setting" + (setting/set! :multi-setting-read-only "Parroty"))))))))) diff --git a/test/metabase/models/setting_test.clj b/test/metabase/models/setting_test.clj index 1c6cbc17c9802632096f5566b7998128ec3cd81d..6600902ac6f8faf916326ad8501eb49eaa06ec9a 100644 --- a/test/metabase/models/setting_test.clj +++ b/test/metabase/models/setting_test.clj @@ -42,7 +42,7 @@ :visibility :internal :type :csv) -(defsetting test-csv-setting-with-default +(defsetting ^:private test-csv-setting-with-default "Test setting - this only shows up in dev (6)" :visibility :internal :type :csv @@ -60,8 +60,7 @@ "Test setting - this only shows up in dev (8)" :type :boolean :setter :none - :getter (fn [] - true)) + :getter (constantly true)) ;; ## HELPER FUNCTIONS @@ -85,24 +84,32 @@ (is (= expected-tag (:tag (meta arglist))))))))) +(deftest preserve-metadata-test + (testing "defsetting should preserve metadata on the setting symbol in the getter/setter functions" + (doseq [varr [#'test-csv-setting-with-default #'test-csv-setting-with-default!]] + (testing (format "\nvar = %s" (pr-str varr)) + (is (:private (meta varr))))))) + (deftest string-tag-test (testing "String vars defined by `defsetting` should have correct `:tag` metadata\n" - (test-assert-setting-has-tag #'test-setting-1 'java.lang.String))) + (doseq [varr [#'test-setting-1 #'test-setting-1!]] + (testing (format "\nVar = %s" (pr-str varr)) + (test-assert-setting-has-tag varr 'java.lang.String))))) (deftest defsetting-getter-fn-test (testing "Test defsetting getter fn. Should return the value from env var MB_TEST_ENV_SETTING" - (test-env-setting nil) + (test-env-setting! nil) (is (= "ABCDEFG" (test-env-setting)))) (testing "Test getting a default value -- if you clear the value of a Setting it should revert to returning the default value" - (test-setting-2 nil) + (test-setting-2! nil) (is (= "[Default Value]" (test-setting-2))))) (deftest user-facing-value-test (testing "`user-facing-value` should return `nil` for a Setting that is using the default value" - (test-setting-2 nil) + (test-setting-2! nil) (is (= nil (setting/user-facing-value :test-setting-2)))) (testing "`user-facing-value` should work correctly for calculated Settings (no underlying value)" @@ -111,8 +118,14 @@ (is (= true (setting/user-facing-value :test-setting-calculated-getter))))) +(deftest do-not-define-setter-function-for-setter-none-test + (testing "Settings with `:setter` `:none` should not have a setter function defined" + (testing "Sanity check: getter should be defined" + (is (some? (resolve `test-setting-calculated-getter)))) + (is (not (resolve `test-setting-calculated-getter!))))) + (deftest defsetting-setter-fn-test - (test-setting-2 "FANCY NEW VALUE <3") + (test-setting-2! "FANCY NEW VALUE <3") (is (= "FANCY NEW VALUE <3" (test-setting-2))) (is (= "FANCY NEW VALUE <3" @@ -159,12 +172,12 @@ (deftest delete-test (testing "delete" (testing "w/o default value" - (test-setting-1 "COOL") + (test-setting-1! "COOL") (is (= "COOL" (test-setting-1))) (is (= true (setting-exists-in-db? :test-setting-1))) - (test-setting-1 nil) + (test-setting-1! nil) (is (= nil (test-setting-1))) (is (= nil @@ -173,12 +186,12 @@ (setting-exists-in-db? :test-setting-1)))) (testing "w/ default value" - (test-setting-2 "COOL") + (test-setting-2! "COOL") (is (= "COOL" (test-setting-2))) (is (= true (setting-exists-in-db? :test-setting-2))) - (test-setting-2 nil) + (test-setting-2! nil) (is (= "[Default Value]" (test-setting-2)) "default value should get returned if none is set") @@ -237,8 +250,8 @@ (deftest admin-writable-settings-test (testing `setting/admin-writable-settings - (test-setting-1 nil) - (test-setting-2 "TOUCANS") + (test-setting-1! nil) + (test-setting-2! "TOUCANS") (is (= {:key :test-setting-2 :value "TOUCANS" :description "Test setting - this only shows up in dev (2)" @@ -251,8 +264,8 @@ (setting/admin-writable-settings)))) (testing "with a custom getter" - (test-setting-1 nil) - (test-setting-2 "TOUCANS") + (test-setting-1! nil) + (test-setting-2! "TOUCANS") (is (= {:key :test-setting-2 :value 7 :description "Test setting - this only shows up in dev (2)" @@ -266,8 +279,8 @@ ;; TODO -- probably don't need both this test and the "TOUCANS" test above, we should combine them (testing "test settings" - (test-setting-1 nil) - (test-setting-2 "S2") + (test-setting-1! nil) + (test-setting-2! "S2") (is (= [{:key :test-setting-1 :value nil :is_env_setting false @@ -330,7 +343,7 @@ (is (thrown-with-msg? Exception #"Invalid value for string: must be either \"true\" or \"false\" \(case-insensitive\)" - (test-boolean-setting "X")))) + (test-boolean-setting! "X")))) (testing "user-facing info should just return `nil` instead of failing entirely" (is (= {:value nil @@ -342,13 +355,13 @@ (deftest set-boolean-setting-test (testing "should be able to set value with a string..." (is (= "false" - (test-boolean-setting "FALSE"))) + (test-boolean-setting! "FALSE"))) (is (= false (test-boolean-setting))) (testing "... or a boolean" (is (= "false" - (test-boolean-setting false))) + (test-boolean-setting! false))) (is (= false (test-boolean-setting)))))) @@ -357,7 +370,7 @@ (deftest set-json-setting-test (is (= "{\"a\":100,\"b\":200}" - (test-json-setting {:a 100, :b 200}))) + (test-json-setting! {:a 100, :b 200}))) (is (= {:a 100, :b 200} (test-json-setting)))) @@ -378,7 +391,7 @@ (fetch-csv-setting-value "A,B,\"C1,C2\",ddd"))))) (defn- set-and-fetch-csv-setting-value! [v] - (test-csv-setting v) + (test-csv-setting! v) {:db-value (db/select-one-field :value setting/Setting :key "test-csv-setting") :parsed-value (test-csv-setting)}) @@ -404,13 +417,13 @@ (set-and-fetch-csv-setting-value! nil)))) (testing "default values for CSV settings should work" - (test-csv-setting-with-default nil) + (test-csv-setting-with-default! nil) (is (= ["A" "B" "C"] (test-csv-setting-with-default))))) (deftest csv-setting-user-facing-value-test (testing "`user-facing-value` should be `nil` for CSV Settings with default values" - (test-csv-setting-with-default nil) + (test-csv-setting-with-default! nil) (is (= nil (setting/user-facing-value :test-csv-setting-with-default))))) @@ -426,7 +439,7 @@ (deftest encrypted-settings-test (testing "If encryption is *enabled*, make sure Settings get saved as encrypted!" (encryption-test/with-secret-key "ABCDEFGH12345678" - (toucan-name "Sad Can") + (toucan-name! "Sad Can") (is (u/base64-string? (actual-value-in-db :toucan-name))) (testing "make sure it can be decrypted as well..." @@ -435,16 +448,15 @@ (testing "But if encryption is not enabled, of course Settings shouldn't get saved as encrypted." (encryption-test/with-secret-key nil - (toucan-name "Sad Can") + (toucan-name! "Sad Can") (is (= "Sad Can" - (mt/suppress-output - (actual-value-in-db :toucan-name)))))))) + (actual-value-in-db :toucan-name))))))) (deftest previously-encrypted-settings-test (testing "Make sure settings that were encrypted don't cause `user-facing-info` to blow up if encyrption key changed" (mt/discard-setting-changes [test-json-setting] (encryption-test/with-secret-key "0B9cD6++AME+A7/oR7Y2xvPRHX3cHA2z7w+LbObd/9Y=" - (test-json-setting {:abc 123}) + (test-json-setting! {:abc 123}) (is (not= "{\"abc\":123}" (actual-value-in-db :test-json-setting)))) (testing (str "If fetching the Setting fails (e.g. because key changed) `user-facing-info` should return `nil` " @@ -470,7 +482,7 @@ (test-assert-setting-has-tag #'test-timestamp-setting 'java.time.temporal.Temporal) (testing "make sure we can set & fetch the value and that it gets serialized/deserialized correctly" - (test-timestamp-setting #t "2018-07-11T09:32:00.000Z") + (test-timestamp-setting! #t "2018-07-11T09:32:00.000Z") (is (= #t "2018-07-11T09:32:00.000Z" (test-timestamp-setting))))) @@ -495,12 +507,12 @@ (deftest uncached-settings-test (encryption-test/with-secret-key nil (testing "make sure uncached setting still saves to the DB" - (uncached-setting "ABCDEF") + (uncached-setting! "ABCDEF") (is (= "ABCDEF" (actual-value-in-db "uncached-setting")))) (testing "make sure that fetching the Setting always fetches the latest value from the DB" - (uncached-setting "ABCDEF") + (uncached-setting! "ABCDEF") (db/update-where! Setting {:key "uncached-setting"} :value "123456") (is (= "123456" @@ -508,7 +520,7 @@ (testing "make sure that updating the setting doesn't update the last-updated timestamp in the cache $$" (clear-settings-last-updated-value-in-db!) - (uncached-setting "abcdef") + (uncached-setting! "abcdef") (is (= nil (settings-last-updated-value-in-db)))))) @@ -521,13 +533,13 @@ (deftest sensitive-settings-test (testing "`user-facing-value` should obfuscate sensitive settings" - (test-sensitive-setting "ABC123") + (test-sensitive-setting! "ABC123") (is (= "**********23" (setting/user-facing-value "test-sensitive-setting")))) (testing "Attempting to set a sensitive setting to an obfuscated value should be ignored -- it was probably done accidentally" - (test-sensitive-setting "123456") - (test-sensitive-setting "**********56") + (test-sensitive-setting! "123456") + (test-sensitive-setting! "**********56") (is (= "123456" (test-sensitive-setting))))) @@ -543,7 +555,7 @@ ;; now set a value for the `toucan-name` setting the wrong way (db/insert! setting/Setting {:key "toucan-name", :value "Reggae"}) ;; ok, now try to set the Setting the correct way - (toucan-name "Banana Beak") + (toucan-name! "Banana Beak") ;; ok, make sure the setting was set (is (= "Banana Beak" (toucan-name))))) @@ -680,16 +692,19 @@ :type :integer) ; `:never` should be the default (deftest database-local-settings-test - (doseq [[database-local-type {:keys [setting-name setting-fn returns]}] - {:only {:setting-name :test-database-local-only-setting - :setting-fn test-database-local-only-setting - :returns [:database-local]} - :allowed {:setting-name :test-database-local-allowed-setting - :setting-fn test-database-local-allowed-setting - :returns [:database-local :site-wide]} - :never {:setting-name :test-database-local-never-setting - :setting-fn test-database-local-never-setting - :returns [:site-wide]}}] + (doseq [[database-local-type {:keys [setting-name setting-getter-fn setting-setter-fn returns]}] + {:only {:setting-name :test-database-local-only-setting + :setting-getter-fn test-database-local-only-setting + :setting-setter-fn test-database-local-only-setting! + :returns [:database-local]} + :allowed {:setting-name :test-database-local-allowed-setting + :setting-getter-fn test-database-local-allowed-setting + :setting-setter-fn test-database-local-allowed-setting! + :returns [:database-local :site-wide]} + :never {:setting-name :test-database-local-never-setting + :setting-getter-fn test-database-local-never-setting + :setting-setter-fn test-database-local-never-setting! + :returns [:site-wide]}}] (testing (format "A Setting with :database-local = %s" database-local-type) (doseq [site-wide-value [1 nil] database-local-value [2 nil] @@ -715,7 +730,7 @@ ;; clear out Setting if it was already set for some reason (except for `:only` where this is explicitly ;; disallowed) (when-not (= database-local-type :only) - (setting-fn nil)) + (setting-setter-fn nil)) ;; now set the Site-wide value (testing (format "\nSite-wide value = %s\nDatabase-local value = %s" (pr-str site-wide-value) (pr-str database-local-value)) @@ -732,7 +747,7 @@ returns)] (testing (format "\nShould return %s value %s" (pr-str expected-value-type) (pr-str expected-value)) (is (= expected-value - (setting-fn))))))))))))) + (setting-getter-fn))))))))))))) (defsetting ^:private test-boolean-database-local-setting "test Setting" @@ -743,11 +758,11 @@ (deftest boolean-database-local-settings-test (testing "Boolean Database-local Settings\n" (testing "Site-wide value is `true`" - (test-boolean-database-local-setting true) + (test-boolean-database-local-setting! true) (is (= true (test-boolean-database-local-setting)))) (testing "Site-wide value is `false`" - (test-boolean-database-local-setting false) + (test-boolean-database-local-setting! false) (is (= false (test-boolean-database-local-setting))) (testing "Database-local value is `true`" @@ -770,7 +785,7 @@ (is (thrown-with-msg? clojure.lang.ExceptionInfo #"Site-wide values are not allowed for Setting :test-database-local-only-setting" - (test-database-local-only-setting 2)))) + (test-database-local-only-setting! 2)))) (testing "Default values should be allowed for Database-local-only Settings" (is (= "DEFAULT" @@ -807,11 +822,11 @@ (deftest integer-setting-test (testing "Should be able to set integer setting with a string" - (test-integer-setting "100") + (test-integer-setting! "100") (is (= 100 (test-integer-setting))) (testing "should be able to set to a negative number (thanks Howon for spotting this)" - (test-integer-setting "-2") + (test-integer-setting! "-2") (is (= -2 (test-integer-setting)))))) @@ -841,32 +856,32 @@ (deftest user-local-settings-test (testing "Reading and writing a user-local-only setting in the context of a user uses the user-local value" (mt/with-current-user (mt/user->id :rasta) - (test-user-local-only-setting "ABC") + (test-user-local-only-setting! "ABC") (is (= "ABC" (test-user-local-only-setting)))) (mt/with-current-user (mt/user->id :crowberto) - (test-user-local-only-setting "DEF") + (test-user-local-only-setting! "DEF") (is (= "DEF" (test-user-local-only-setting)))) (mt/with-current-user (mt/user->id :rasta) (is (= "ABC" (test-user-local-only-setting))))) (testing "A user-local-only setting cannot have a site-wide value" - (is (thrown-with-msg? Throwable #"Site-wide values are not allowed" (test-user-local-only-setting "ABC")))) + (is (thrown-with-msg? Throwable #"Site-wide values are not allowed" (test-user-local-only-setting! "ABC")))) (testing "Reading and writing a user-local-allowed setting in the context of a user uses the user-local value" ;; TODO: mt/with-temporary-setting-values only affects site-wide value, we should figure out whether it should also ;; affect user-local settings. (mt/with-temporary-setting-values [test-user-local-allowed-setting nil] (mt/with-current-user (mt/user->id :rasta) - (test-user-local-allowed-setting "ABC") + (test-user-local-allowed-setting! "ABC") (is (= "ABC" (test-user-local-allowed-setting)))) (mt/with-current-user (mt/user->id :crowberto) - (test-user-local-allowed-setting "DEF") + (test-user-local-allowed-setting! "DEF") (is (= "DEF" (test-user-local-allowed-setting)))) (mt/with-current-user (mt/user->id :rasta) (is (= "ABC" (test-user-local-allowed-setting)))) ;; Calling the setter when not in the context of a user should set the site-wide value (is (nil? (test-user-local-allowed-setting))) - (test-user-local-allowed-setting "GHI") + (test-user-local-allowed-setting! "GHI") (mt/with-current-user (mt/user->id :crowberto) (is (= "DEF" (test-user-local-allowed-setting)))) (mt/with-current-user (mt/user->id :rasta) @@ -874,10 +889,10 @@ (testing "Reading and writing a user-local-never setting in the context of a user uses the site-wide value" (mt/with-current-user (mt/user->id :rasta) - (test-user-local-never-setting "ABC") + (test-user-local-never-setting! "ABC") (is (= "ABC" (test-user-local-never-setting)))) (mt/with-current-user (mt/user->id :crowberto) - (test-user-local-never-setting "DEF") + (test-user-local-never-setting! "DEF") (is (= "DEF" (test-user-local-never-setting)))) (mt/with-current-user (mt/user->id :rasta) (is (= "DEF" (test-user-local-never-setting)))) diff --git a/test/metabase/public_settings_test.clj b/test/metabase/public_settings_test.clj index 50360b05f516a25d5e40e651d48212e9040dd728..6a6ff2120f8a28d467665638f163444a19274783 100644 --- a/test/metabase/public_settings_test.clj +++ b/test/metabase/public_settings_test.clj @@ -14,27 +14,27 @@ (deftest site-url-settings (testing "double-check that setting the `site-url` setting will automatically strip off trailing slashes" (mt/discard-setting-changes [site-url] - (public-settings/site-url "http://localhost:3000/") + (public-settings/site-url! "http://localhost:3000/") (is (= "http://localhost:3000" (public-settings/site-url)))))) (deftest site-url-settings-prepend-http (testing "double-check that setting the `site-url` setting will prepend `http://` if no protocol was specified" (mt/discard-setting-changes [site-url] - (public-settings/site-url "localhost:3000") + (public-settings/site-url! "localhost:3000") (is (= "http://localhost:3000" (public-settings/site-url)))))) (deftest site-url-settings-with-no-trailing-slash (mt/discard-setting-changes [site-url] - (public-settings/site-url "http://localhost:3000") + (public-settings/site-url! "http://localhost:3000") (is (= "http://localhost:3000" (public-settings/site-url))))) (deftest site-url-settings-https (testing "if https:// was specified it should keep it") (mt/discard-setting-changes [site-url] - (public-settings/site-url "https://localhost:3000") + (public-settings/site-url! "https://localhost:3000") (is (= "https://localhost:3000" (public-settings/site-url))))) @@ -43,11 +43,11 @@ (mt/discard-setting-changes [site-url] (is (thrown? clojure.lang.ExceptionInfo - (public-settings/site-url "http://https://www.camsaul.com")))))) + (public-settings/site-url! "http://https://www.camsaul.com")))))) (deftest site-url-settings-set-valid-domain-name (mt/discard-setting-changes [site-url] - (is (some? (public-settings/site-url "https://www.camsaul.x"))))) + (is (some? (public-settings/site-url! "https://www.camsaul.x"))))) (deftest site-url-settings-nil-getter-when-invalid (testing "if `site-url` in the database is invalid, the getter for `site-url` should return `nil` (#9849)" @@ -84,7 +84,7 @@ redirect-all-requests-to-https true] (is (= true (public-settings/redirect-all-requests-to-https))) - (public-settings/site-url "http://example.com") + (public-settings/site-url! "http://example.com") (is (= false (public-settings/redirect-all-requests-to-https))))) @@ -93,7 +93,7 @@ redirect-all-requests-to-https true] (is (= true (public-settings/redirect-all-requests-to-https))) - (public-settings/site-url "https://different.example.com") + (public-settings/site-url! "https://different.example.com") (is (= true (public-settings/redirect-all-requests-to-https)))))) @@ -118,14 +118,14 @@ "API (#9143)") (mt/discard-setting-changes [query-caching-max-kb] (is (= "1000" - (public-settings/query-caching-max-kb "1000"))))) + (public-settings/query-caching-max-kb! "1000"))))) (testing "query-caching-max-kb should throw an error if you try to put in a huge value" (mt/discard-setting-changes [query-caching-max-kb] (is (thrown-with-msg? IllegalArgumentException #"Values greater than 204,800 \(200\.0 MB\) are not allowed" - (public-settings/query-caching-max-kb (* 1024 1024))))))) + (public-settings/query-caching-max-kb! (* 1024 1024))))))) (deftest site-locale-test (testing "site-locale Setting" @@ -136,7 +136,7 @@ (is (thrown-with-msg? clojure.lang.ExceptionInfo #"Invalid locale \"\"" - (public-settings/site-locale ""))) + (public-settings/site-locale! ""))) (is (= "en_US" (public-settings/site-locale))))) @@ -145,20 +145,20 @@ (is (thrown-with-msg? clojure.lang.ExceptionInfo #"Invalid locale \"en_EN\"" - (public-settings/site-locale "en_EN"))) + (public-settings/site-locale! "en_EN"))) (is (= "en_US" (public-settings/site-locale))))))) (testing "should normalize input" (mt/discard-setting-changes [site-locale] - (public-settings/site-locale "en-us") + (public-settings/site-locale! "en-us") (is (= "en_US" (public-settings/site-locale))))) (testing "should be able to unset site locale" (mt/discard-setting-changes [site-locale] - (public-settings/site-locale "es") - (public-settings/site-locale nil) + (public-settings/site-locale! "es") + (public-settings/site-locale! nil) (is (= "en" (public-settings/site-locale)) "should default to English"))))) @@ -170,7 +170,7 @@ (testing "\n`site-url` *is* HTTPS" (mt/with-temporary-setting-values [site-url "https://example.com" redirect-all-requests-to-https false] - (public-settings/redirect-all-requests-to-https v) + (public-settings/redirect-all-requests-to-https! v) (is (= true (public-settings/redirect-all-requests-to-https))))) @@ -179,7 +179,7 @@ redirect-all-requests-to-https false] (is (thrown? AssertionError - (public-settings/redirect-all-requests-to-https v))) + (public-settings/redirect-all-requests-to-https! v))) (is (= false (public-settings/redirect-all-requests-to-https))))))))) diff --git a/test/metabase/query_processor/card_test.clj b/test/metabase/query_processor/card_test.clj index 93347eed7374ad468091b88931a21d03e211e66f..0b7ccf186581dc80b966ee81472266aaafe9c9b8 100644 --- a/test/metabase/query_processor/card_test.clj +++ b/test/metabase/query_processor/card_test.clj @@ -24,7 +24,7 @@ (deftest query-cache-ttl-hierarchy-test (mt/discard-setting-changes [enable-query-caching] - (public-settings/enable-query-caching true) + (public-settings/enable-query-caching! true) (testing "query-magic-ttl converts to seconds correctly" (mt/with-temporary-setting-values [query-caching-ttl-ratio 2] ;; fake average execution time (in millis) diff --git a/test/metabase/test.clj b/test/metabase/test.clj index abef350bcfdba245fcc65728a63d281733398c21..d161be3c0df7c38f7777109359bebbf6782439dc 100644 --- a/test/metabase/test.clj +++ b/test/metabase/test.clj @@ -369,7 +369,7 @@ expr))))) (defmacro are+ - "Like `clojure.test/are` but includes a message for easier test failure debugging. (Also this is somewhat more + "Like [[clojure.test/are]] but includes a message for easier test failure debugging. (Also this is somewhat more efficient since it generates far less code  it uses `doseq` rather than repeating the entire test each time.)" {:style/indent 2} [argv expr & args] diff --git a/test/metabase/test/data/impl.clj b/test/metabase/test/data/impl.clj index 2c73cede06ac8854c270d36b43e832db5bd09c1e..91795eeca4a55f9a05082bed268985b4ee677c5c 100644 --- a/test/metabase/test/data/impl.clj +++ b/test/metabase/test/data/impl.clj @@ -131,7 +131,7 @@ (throw e))))) (catch Throwable e (let [message (format "Failed to create %s '%s' test database: %s" driver database-name (ex-message e))] - (log/error e message) + (log/fatal e message) (if config/is-test? (System/exit -1) (do diff --git a/test/metabase/util_test.clj b/test/metabase/util_test.clj index 20095517c9a5304e4d9d437e3f16e34de676939a..6566e79581835ad218efdd47395f6fb8081cd9b6 100644 --- a/test/metabase/util_test.clj +++ b/test/metabase/util_test.clj @@ -74,8 +74,8 @@ "http:/" false)) (deftest ^:parallel state?-test - (mt/are+ [s expected] (= expected - (u/state? s)) + (mt/are+ [x expected] (= expected + (u/state? x)) "louisiana" true "north carolina" true "WASHINGTON" true @@ -103,12 +103,6 @@ (is (thrown? ClassCastException (u/qualified-name false))))) -(deftest ^:parallel rpartial-test - (is (= 3 - ((u/rpartial - 5) 8))) - (is (= -7 - ((u/rpartial - 5 10) 8)))) - (deftest ^:parallel key-by-test (is (= {1 {:id 1, :name "Rasta"} 2 {:id 2, :name "Lucky"}} @@ -235,7 +229,7 @@ (deftest ^:parallel one-or-many-test (mt/are+ [input expected] (= expected - (u/one-or-many input)) + (u/one-or-many input)) nil nil [nil] [nil] 42 [42] @@ -322,7 +316,7 @@ (let [limit 5 rf (u/sorted-take limit compare)] (reduce (fn [q x] - (let [q' (rf q x)] + (let [_q' (rf q x)] ;; a bit internal but this is really what we're after: bounded size while we look for the biggest ;; elements (is (<= (count q) limit))