Skip to content
Snippets Groups Projects
Unverified Commit 80279d88 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Fix `has-user-setup` Setting not being cached correctly (#26841)

* Fix `has-user-setup` Setting not being cached correctly

* Fix Kondo error
parent 44b14d41
No related branches found
No related tags found
No related merge requests found
(ns metabase.setup (ns metabase.setup
(:require [environ.core :as env] (:require
[metabase.models.setting :as setting :refer [defsetting Setting]] [environ.core :as env]
[metabase.models.user :refer [User]] [metabase.db.connection :as mdb.connection]
[toucan.db :as db]) [metabase.models.setting :as setting :refer [defsetting Setting]]
(:import java.util.UUID)) [metabase.models.user :refer [User]]
[toucan.db :as db])
(:import
(java.util UUID)))
(defsetting setup-token (defsetting setup-token
"A token used to signify that an instance has permissions to create the initial User. This is created upon the first "A token used to signify that an instance has permissions to create the initial User. This is created upon the first
...@@ -39,8 +42,13 @@ ...@@ -39,8 +42,13 @@
;; Once a User is created it's impossible for this to ever become falsey -- deleting the last User is disallowed. ;; Once a User is created it's impossible for this to ever become falsey -- deleting the last User is disallowed.
;; After this returns true once the result is cached and it will continue to return true forever without any ;; After this returns true once the result is cached and it will continue to return true forever without any
;; additional DB hits. ;; additional DB hits.
:getter (fn [] ;;
(let [user-exists? (atom false)] ;; This is keyed by the unique identifier for the application database, to support resetting it in tests or swapping
(or @user-exists? ;; it out in the REPL
(reset! user-exists? (db/exists? User))))) :getter (let [app-db-id->user-exists? (atom {})]
(fn []
(or (get @app-db-id->user-exists? (mdb.connection/unique-identifier))
(let [exists? (db/exists? User)]
(swap! app-db-id->user-exists? assoc (mdb.connection/unique-identifier) exists?)
exists?))))
:doc false) :doc false)
(ns metabase.setup-test
(:require
[clojure.test :refer :all]
[metabase.db :as mdb]
[metabase.db.schema-migrations-test.impl
:as schema-migrations-test.impl]
[metabase.setup :as setup]
[metabase.test :as mt]
[toucan.db :as db]))
(deftest has-user-setup-cached-test
(testing "The has-user-setup getter should cache truthy results since it can never become falsey"
;; make sure some test users are created.
(mt/initialize-if-needed! :test-users)
(db/with-call-counting [call-count]
;; call has-user-setup several times.
(dotimes [_ 5]
(is (= true
(setup/has-user-setup))))
;; `has-user-setup` should have done at most one application database call, as opposed to one call per call to
;; the getter
(is (contains? #{0 1} (call-count)))))
(testing "Return falsey for an empty instance. Values should be cached for current app DB to support swapping in tests/REPL"
;; create a new completely empty database.
(schema-migrations-test.impl/with-temp-empty-app-db [_conn :h2]
;; make sure the DB is setup (e.g., run all the Liquibase migrations)
(mdb/setup-db!)
(db/with-call-counting [call-count]
(dotimes [_ 5]
(is (= false
(setup/has-user-setup))))
(testing "Should continue doing new DB calls as long as there is no User"
(is (= 5
(call-count)))))))
(testing "Switch back to the 'normal' app DB; value should still be cached for it"
(db/with-call-counting [call-count]
(is (= true
(setup/has-user-setup)))
(is (zero? (call-count))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment