Speed up audit DB tests (#35522)

Co-authored-by: bryan <bryan.maass@gmail.com>

enterprise/backend/src/metabase_enterprise/audit_db.clj

@@ -9,6 +9,7 @@
    [metabase.db.env :as mdb.env]
    [metabase.models.database :refer [Database]]
    [metabase.models.permissions :as perms]
+   [metabase.models.setting :refer [defsetting]]
    [metabase.plugins :as plugins]
    [metabase.public-settings.premium-features :refer [defenterprise]]
    [metabase.sync.util :as sync-util]
@@ -110,7 +111,7 @@
                              :name             "Internal Metabase Database"
                              :description      "Internal Audit DB used to power metabase analytics."
                              :engine           engine
-                             :is_full_sync true
+                             :is_full_sync     true
                              :is_on_demand     false
                              :creator_id       nil
                              :auto_run_queries true})))))
@@ -151,26 +152,6 @@
                   {:name [:upper :name]}))
     (log/infof "Adjusted Audit DB to match host engine: %s" (name mdb.env/db-type))))
 
-(defn ensure-db-installed!
-  "Called on app startup to ensure the existance of the audit db in enterprise apps.
-
-  The return values indicate what action was taken."
-  []
-  (let [audit-db (t2/select-one Database :is_audit true)]
-    (cond
-      (nil? audit-db)
-      (u/prog1 ::installed
-        (log/info "Installing Audit DB...")
-        (install-database! mdb.env/db-type))
-
-      (not= mdb.env/db-type (:engine audit-db))
-      (u/prog1 ::updated
-        (log/infof "App DB change detected. Changing Audit DB source to match: %s." (name mdb.env/db-type))
-        (adjust-audit-db-to-host! audit-db))
-
-      :else
-      ::no-op)))
-
 (def analytics-dir-resource
   "A resource dir containing analytics content created by Metabase to load into the app instance on startup."
   (io/resource "instance_analytics"))
@@ -196,28 +177,56 @@
                     {:replace-existing true})
       (log/info "Copying complete."))))
 
+(defsetting load-analytics-content
+  "Whether or not we should load Metabase analytics content on startup. Defaults to true, but can be disabled via environment variable."
+  :type       :boolean
+  :default    true
+  :visibility :internal
+  :setter     :none
+  :audit      :never)
+
+(defn- maybe-load-analytics-content!
+  [audit-db]
+  (when (and analytics-dir-resource (load-analytics-content))
+    (ee.internal-user/ensure-internal-user-exists!)
+    (adjust-audit-db-to-source! audit-db)
+    (log/info "Loading Analytics Content...")
+    (ia-content->plugins)
+    (log/info (str "Loading Analytics Content from: plugins/instance_analytics"))
+    ;; The EE token might not have :serialization enabled, but audit features should still be able to use it.
+    (let [report (log/with-no-logs
+                   (serialization.cmd/v2-load-internal! "plugins/instance_analytics"
+                                                       {}
+                                                       :token-check? false))]
+      (if (not-empty (:errors report))
+        (log/info (str "Error Loading Analytics Content: " (pr-str report)))
+        (log/info (str "Loading Analytics Content Complete (" (count (:seen report)) ") entities loaded."))))
+    (when-let [audit-db (t2/select-one :model/Database :is_audit true)]
+      (adjust-audit-db-to-host! audit-db))))
+
+(defn- maybe-install-audit-db
+  []
+  (let [audit-db (t2/select-one :model/Database :is_audit true)]
+    (cond
+      (nil? audit-db)
+      (u/prog1 ::installed
+       (log/info "Installing Audit DB...")
+       (install-database! mdb.env/db-type))
+
+      (not= mdb.env/db-type (:engine audit-db))
+      (u/prog1 ::updated
+       (log/infof "App DB change detected. Changing Audit DB source to match: %s." (name mdb.env/db-type))
+       (adjust-audit-db-to-host! audit-db))
+
+      :else
+      ::no-op)))
+
 (defenterprise ensure-audit-db-installed!
-  "EE implementation of `ensure-db-installed!`. Also forces an immediate sync on audit-db."
+  "EE implementation of `ensure-db-installed!`. Installs audit db if it does not already exist, and loads audit
+  content if it is available."
   :feature :none
   []
-  (u/prog1 (ensure-db-installed!)
-    (let [audit-db (t2/select-one :model/Database :is_audit true)]
-      (when analytics-dir-resource
-        ;; prevent sync while loading
-        ((sync-util/with-duplicate-ops-prevented :sync-database audit-db
-           (fn []
-             (ee.internal-user/ensure-internal-user-exists!)
-             (adjust-audit-db-to-source! audit-db)
-             (log/info "Loading Analytics Content...")
-             (ia-content->plugins)
-             (log/info (str "Loading Analytics Content from: plugins/instance_analytics"))
-             ;; The EE token might not have :serialization enabled, but audit features should still be able to use it.
-             (let [report (log/with-no-logs
-                            (serialization.cmd/v2-load-internal! "plugins/instance_analytics"
-                                                                {}
-                                                                :token-check? false))]
-               (if (not-empty (:errors report))
-                 (log/info (str "Error Loading Analytics Content: " (pr-str report)))
-                 (log/info (str "Loading Analytics Content Complete (" (count (:seen report)) ") entities loaded."))))
-             (when-let [audit-db (t2/select-one :model/Database :is_audit true)]
-               (adjust-audit-db-to-host! audit-db)))))))))
+  (u/prog1 (maybe-install-audit-db)
+   (let [audit-db (t2/select-one :model/Database :is_audit true)]
+       ;; prevent sync while loading
+     ((sync-util/with-duplicate-ops-prevented :sync-database audit-db (partial maybe-load-analytics-content! audit-db))))))

enterprise/backend/test/metabase_enterprise/audit_app/api/collection_test.clj

 (ns metabase-enterprise.audit-app.api.collection-test
   (:require
    [clojure.test :refer :all]
-   [metabase-enterprise.audit-db :as audit-db]
    [metabase-enterprise.audit-db-test :as audit-db-test]
    [metabase.models :refer [Collection]]
-   [metabase.public-settings.premium-features-test :as premium-features-test]
+   [metabase.public-settings.premium-features-test
+    :as premium-features-test]
    [metabase.test :as mt]
    [toucan2.tools.with-temp :as t2.with-temp]))
 
 (deftest list-collections-instance-analytics-test
   (premium-features-test/with-premium-features #{:audit-app}
     (audit-db-test/with-audit-db-restoration
-      (audit-db/ensure-audit-db-installed!)
       (t2.with-temp/with-temp [Collection _ {:name "Zippy"}]
         (testing "Instance Analytics Collection should be the last collection."
           (testing "GET /api/collection"
@@ -26,7 +25,6 @@
                         :type))))))))
   (premium-features-test/with-premium-features #{}
     (audit-db-test/with-audit-db-restoration
-      (audit-db/ensure-audit-db-installed!)
       (t2.with-temp/with-temp [Collection _ {:name "Zippy"}]
         (testing "Instance Analytics Collection should not show up when audit-app isn't enabled."
           (testing "GET /api/collection"

enterprise/backend/test/metabase_enterprise/audit_app/api/database_test.clj

 (ns metabase-enterprise.audit-app.api.database-test
   (:require
    [clojure.test :refer :all]
-   [metabase-enterprise.audit-db :as audit-db]
    [metabase-enterprise.audit-db-test :as audit-db-test]
    [metabase.models.permissions :as perms]
-   [metabase.public-settings.premium-features-test :as premium-features-test]
+   [metabase.public-settings.premium-features-test
+    :as premium-features-test]
    [metabase.test :as mt]))
 
 (deftest audit-db-unmodifiable-test
   (premium-features-test/with-premium-features #{:audit-app}
     (audit-db-test/with-audit-db-restoration
-      (audit-db/ensure-audit-db-installed!)
       (testing "Neither admin nor regular users can modify the audit database"
         (doseq [[verb path] [[:post "database/%d/unpersist"]
                              [:put "database/%d"]

enterprise/backend/test/metabase_enterprise/audit_app/permissions_test.clj

@@ -32,56 +32,54 @@
 
 (deftest audit-db-basic-query-test
   (mt/test-drivers #{:postgres :h2 :mysql}
-   (audit-db-test/with-audit-db-restoration
-     (audit-db/ensure-audit-db-installed!)
-     (premium-features-test/with-premium-features #{:audit-app}
-       (mt/with-test-user :crowberto
-         (testing "A query using a saved audit model as the source table runs succesfully"
-           (let [audit-card (t2/select-one :model/Card :database_id perms/audit-db-id :dataset true)]
-             (is (partial=
-                  {:status :completed}
-                  (qp/process-query
-                   {:database perms/audit-db-id
-                    :type     :query
-                    :query    {:source-table (str "card__" (u/the-id audit-card))}})))))
+    (audit-db-test/with-audit-db-restoration
+      (premium-features-test/with-premium-features #{:audit-app}
+        (mt/with-test-user :crowberto
+          (testing "A query using a saved audit model as the source table runs succesfully"
+            (let [audit-card (t2/select-one :model/Card :database_id perms/audit-db-id :dataset true)]
+              (is (partial=
+                   {:status :completed}
+                   (qp/process-query
+                    {:database perms/audit-db-id
+                     :type     :query
+                     :query    {:source-table (str "card__" (u/the-id audit-card))}})))))
 
-         (testing "A non-native query can be run on views in the audit DB"
-           (let [audit-view (t2/select-one :model/Table :db_id perms/audit-db-id)]
-             (is (partial=
-                  {:status :completed}
-                  (qp/process-query
-                   {:database perms/audit-db-id
-                    :type     :query
-                    :query    {:source-table (u/the-id audit-view)}}))))))))))
+          (testing "A non-native query can be run on views in the audit DB"
+            (let [audit-view (t2/select-one :model/Table :db_id perms/audit-db-id)]
+              (is (partial=
+                   {:status :completed}
+                   (qp/process-query
+                    {:database perms/audit-db-id
+                     :type     :query
+                     :query    {:source-table (u/the-id audit-view)}}))))))))))
 
 (deftest audit-db-disallowed-queries-test
   (mt/test-drivers #{:postgres :h2 :mysql}
-   (audit-db-test/with-audit-db-restoration
-     (audit-db/ensure-audit-db-installed!)
-     (premium-features-test/with-premium-features #{:audit-app}
-       (mt/with-test-user :crowberto
-         (testing "Native queries are not allowed to be run on audit DB views, even by admins"
-           (is (thrown-with-msg?
-                clojure.lang.ExceptionInfo
-                #"Native queries are not allowed on the audit database"
-                (qp/process-query
-                 {:database perms/audit-db-id
-                  :type     :native
-                  :native   {:query "SELECT * FROM v_audit_log;"}}))))
+    (audit-db-test/with-audit-db-restoration
+      (premium-features-test/with-premium-features #{:audit-app}
+        (mt/with-test-user :crowberto
+          (testing "Native queries are not allowed to be run on audit DB views, even by admins"
+            (is (thrown-with-msg?
+                 clojure.lang.ExceptionInfo
+                 #"Native queries are not allowed on the audit database"
+                 (qp/process-query
+                  {:database perms/audit-db-id
+                   :type     :native
+                   :native   {:query "SELECT * FROM v_audit_log;"}}))))
 
-         (testing "Non-native queries are not allowed to run on tables in the audit DB that are not views"
-           ;; Nothing should be synced directly from the audit DB, just loaded via serialization, so only the views
-           ;; should have metadata present in the app DB in the first place. But in case this changes, we want to
-           ;; explicitly block other tables from being queried.
-           (t2.with-temp/with-temp [:model/Table table {:db_id perms/audit-db-id}
-                                    :model/Field _     {:table_id (u/the-id table)}]
-             (is (thrown-with-msg?
-                  clojure.lang.ExceptionInfo
-                  #"Audit queries are only allowed on audit views"
-                  (qp/process-query
-                   {:database perms/audit-db-id
-                    :type     :query
-                    :query   {:source-table (u/the-id table)}}))))))))))
+          (testing "Non-native queries are not allowed to run on tables in the audit DB that are not views"
+            ;; Nothing should be synced directly from the audit DB, just loaded via serialization, so only the views
+            ;; should have metadata present in the app DB in the first place. But in case this changes, we want to
+            ;; explicitly block other tables from being queried.
+            (t2.with-temp/with-temp [:model/Table table {:db_id perms/audit-db-id}
+                                     :model/Field _     {:table_id (u/the-id table)}]
+              (is (thrown-with-msg?
+                   clojure.lang.ExceptionInfo
+                   #"Audit queries are only allowed on audit views"
+                   (qp/process-query
+                    {:database perms/audit-db-id
+                     :type     :query
+                     :query   {:source-table (u/the-id table)}}))))))))))
 
 (deftest permissions-instance-analytics-audit-v2-test
   (premium-features-test/with-premium-features #{:audit-app}

enterprise/backend/test/metabase_enterprise/audit_db_test.clj

@@ -2,7 +2,7 @@
   (:require
    [babashka.fs :as fs]
    [clojure.java.io :as io]
-   [clojure.test :refer [deftest is]]
+   [clojure.test :refer [deftest is testing]]
    [metabase-enterprise.audit-db :as audit-db]
    [metabase.core :as mbc]
    [metabase.models.database :refer [Database]]
@@ -12,59 +12,54 @@
    [toucan2.core :as t2]))
 
 (defmacro with-audit-db-restoration [& body]
-  `(let [original-audit-db# (t2/select-one Database :is_audit true)]
-     (try
-       (t2/delete! Database :is_audit true)
-       ~@body
-       (finally
-         (t2/delete! Database :is_audit true)
-         (when original-audit-db#
-           (#'mbc/ensure-audit-db-installed!))))))
+  "Calls `ensure-audit-db-installed!` before and after `body` to ensure that the audit DB is installed and then
+  restored if necessary. Also disables audit content loading if it is already loaded."
+  `(let [audit-collection-exists?# (t2/exists? :model/Collection :type "instance-analytics")]
+     (mt/with-temp-env-var-value [mb-load-analytics-content (not audit-collection-exists?#)]
+       (mbc/ensure-audit-db-installed!)
+       (try
+         ~@body
+         (finally
+           (mbc/ensure-audit-db-installed!))))))
 
-(deftest audit-db-is-installed-then-left-alone
+(deftest audit-db-installation-test
   (mt/test-drivers #{:postgres :h2 :mysql}
-    (with-audit-db-restoration
-      (t2/delete! Database :is_audit true)
-      (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-db-installed!)))
-      (is (= :metabase-enterprise.audit-db/no-op (audit-db/ensure-db-installed!)))
-
-      (t2/update! Database :is_audit true {:engine "datomic"})
-      (is (= :metabase-enterprise.audit-db/updated (audit-db/ensure-db-installed!)))
-      (is (= :metabase-enterprise.audit-db/no-op (audit-db/ensure-db-installed!))))))
-
-(deftest audit-db-content-is-not-installed-when-not-found
-  (mt/test-drivers #{:postgres :h2 :mysql}
-    (with-audit-db-restoration
+    (testing "Audit DB content is not installed when it is not found"
+      (t2/delete! :model/Database :is_audit true)
       (with-redefs [audit-db/analytics-dir-resource nil]
         (is (= nil audit-db/analytics-dir-resource))
-        (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-audit-db-installed!)))
+        (is (= ::audit-db/installed (audit-db/ensure-audit-db-installed!)))
         (is (= perms/audit-db-id (t2/select-one-fn :id 'Database {:where [:= :is_audit true]}))
             "Audit DB is installed.")
         (is (= 0 (t2/count :model/Card {:where [:= :database_id perms/audit-db-id]}))
-            "No cards created for Audit DB.")))))
+            "No cards created for Audit DB."))
+      (t2/delete! :model/Database :is_audit true))
 
-(deftest audit-db-content-is-installed-when-found
-  (mt/test-drivers #{:postgres :h2 :mysql}
-    (with-audit-db-restoration
-      (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-audit-db-installed!)))
+    (testing "Audit DB content is installed when it is found"
+      (is (= ::audit-db/installed (audit-db/ensure-audit-db-installed!)))
       (is (= perms/audit-db-id (t2/select-one-fn :id 'Database {:where [:= :is_audit true]}))
           "Audit DB is installed.")
       (is (some? (io/resource "instance_analytics")))
       (is (not= 0 (t2/count :model/Card {:where [:= :database_id perms/audit-db-id]}))
-          "Cards should be created for Audit DB when the content is there."))))
+          "Cards should be created for Audit DB when the content is there."))
 
-(deftest audit-db-does-not-have-scheduled-syncs
-  (mt/test-drivers #{:postgres :h2 :mysql}
-    (with-audit-db-restoration
-      (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-audit-db-installed!)))
+    (testing "Audit DB does not have scheduled syncs"
       (let [db-has-sync-job-trigger? (fn [db-id]
                                        (contains?
                                         (set (map #(-> % :data (get "db-id"))
                                                   (task/job-info "metabase.task.sync-and-analyze.job")))
                                         db-id))]
-        (is (not (db-has-sync-job-trigger? perms/audit-db-id)))))))
+        (is (not (db-has-sync-job-trigger? perms/audit-db-id)))))
+
+    (testing "Audit DB doesn't get re-installed unless the engine changes"
+      (with-redefs [audit-db/load-analytics-content (constantly nil)]
+        (is (= ::audit-db/no-op (audit-db/ensure-audit-db-installed!)))
+        (t2/update! Database :is_audit true {:engine "datomic"})
+        (is (= ::audit-db/updated (audit-db/ensure-audit-db-installed!)))
+        (is (= ::audit-db/no-op (audit-db/ensure-audit-db-installed!)))
+        (t2/update! Database :is_audit true {:engine "h2"})))))
 
-(deftest audit-db-instance-analytics-content-is-coppied-properly
+(deftest audit-db-instance-analytics-content-is-copied-properly
   (fs/delete-tree "plugins/instance_analytics")
   (is (not (contains? (set (map str (fs/list-dir "plugins")))
                       "plugins/instance_analytics")))