Fix flaky metabase.api.notify-test/post-db-id-test (#27465)

It seems the expectation was that the binding *execute-asynchronously* to false
in the test would affect the binding seen by the server. Instead of this a new
parameter is added to the request that makes the server return only when the
asynchronous operation has completed.

src/metabase/api/notify.clj

@@ -11,8 +11,6 @@
    [schema.core :as s]
    [toucan.db :as db]))
 
-(def ^:private ^:dynamic *execute-asynchronously* true)
-
 (api/defendpoint POST "/db/:id"
   "Notification about a potential schema change to one of our `Databases`.
   Caller can optionally specify a `:table_id` or `:table_name` in the body to limit updates to a single
@@ -20,24 +18,21 @@
   regardless if a `:table_id` or `:table_name` is passed.
   This endpoint is secured by an API key that needs to be passed as a `X-METABASE-APIKEY` header which needs to be defined in
   the `MB_API_KEY` [environment variable](https://www.metabase.com/docs/latest/configuring-metabase/environment-variables.html#mb_api_key)"
-  [id :as {{:keys [table_id table_name scan]} :body}]
+  [id :as {{:keys [table_id table_name scan synchronous?]} :body}]
   {table_id   (s/maybe su/IntGreaterThanZero)
    table_name (s/maybe su/NonBlankString)
    scan       (s/maybe (s/enum "full" "schema"))}
   (let [schema?       (when scan (#{"schema" :schema} scan))
         table-sync-fn (if schema? sync-metadata/sync-table-metadata! sync/sync-table!)
-        db-sync-fn    (if schema? sync-metadata/sync-db-metadata! sync/sync-database!)
-        execute!      (fn [thunk]
-                        (if *execute-asynchronously*
-                          (future (thunk))
-                          (thunk)))]
+        db-sync-fn    (if schema? sync-metadata/sync-db-metadata! sync/sync-database!)]
     (api/let-404 [database (db/select-one Database :id id)]
-      (cond
-        table_id   (api/let-404 [table (db/select-one Table :db_id id, :id (int table_id))]
-                     (execute! #(table-sync-fn table)))
-        table_name (api/let-404 [table (db/select-one Table :db_id id, :name table_name)]
-                     (execute! #(table-sync-fn table)))
-        :else      (execute! #(db-sync-fn database)))))
+      (cond-> (cond
+                table_id   (api/let-404 [table (db/select-one Table :db_id id, :id (int table_id))]
+                             (future (table-sync-fn table)))
+                table_name (api/let-404 [table (db/select-one Table :db_id id, :name table_name)]
+                             (future (table-sync-fn table)))
+                :else      (future (db-sync-fn database)))
+        synchronous? deref)))
   {:success true})
 
 

test/metabase/api/notify_test.clj

@@ -2,7 +2,6 @@
   (:require
    [clj-http.client :as http]
    [clojure.test :refer :all]
-   [metabase.api.notify :as api.notify]
    [metabase.http-client :as client]
    [metabase.models.database :as database]
    [metabase.server.middleware.util :as mw.util]
@@ -55,44 +54,44 @@
                       (select-keys (ex-data e) [:status :body])))))))))
 
 (deftest post-db-id-test
-  (binding [api.notify/*execute-asynchronously* false]
-    (mt/test-drivers (mt/normal-drivers)
-      (let [table-name (->> (mt/db) database/tables first :name)
-            post       (fn post-api
-                         ([payload] (post-api payload 200))
-                         ([payload expected-code]
-                          (mt/with-temporary-setting-values [api-key "test-api-key"]
-                            (mt/client :post expected-code (format "notify/db/%d" (u/the-id (mt/db)))
-                                       {:request-options api-headers}
-                                       payload))))]
-        (testing "sync just table when table is provided"
-          (let [long-sync-called? (atom false), short-sync-called? (atom false)]
-            (with-redefs [metabase.sync/sync-table!                        (fn [_table] (reset! long-sync-called? true))
-                          metabase.sync.sync-metadata/sync-table-metadata! (fn [_table] (reset! short-sync-called? true))]
-              (post {:scan :full, :table_name table-name})
-              (is @long-sync-called?)
-              (is (not @short-sync-called?)))))
-        (testing "only a quick sync when quick parameter is provided"
-          (let [long-sync-called? (atom false), short-sync-called? (atom false)]
-            (with-redefs [metabase.sync/sync-table!                        (fn [_table] (reset! long-sync-called? true))
-                          metabase.sync.sync-metadata/sync-table-metadata! (fn [_table] (reset! short-sync-called? true))]
-              (post {:scan :schema, :table_name table-name})
-              (is (not @long-sync-called?))
-              (is @short-sync-called?))))
-        (testing "full db sync by default"
-          (let [full-sync? (atom false)]
-            (with-redefs [metabase.sync/sync-database! (fn [_db] (reset! full-sync? true))]
-              (post {})
-              (is @full-sync?))))
-        (testing "simple sync with params"
-          (let [full-sync?   (atom false)
-                smaller-sync (atom true)]
-            (with-redefs [metabase.sync/sync-database!                  (fn [_db] (reset! full-sync? true))
-                          metabase.sync.sync-metadata/sync-db-metadata! (fn [_db] (reset! smaller-sync true))]
-              (post {:scan :schema})
-              (is (not @full-sync?))
-              (is @smaller-sync))))
-        (testing "errors on unrecognized scan options"
-          (is (= {:errors
-                  {:scan "value may be nil, or if non-nil, value must be one of: `full`, `schema`."}}
-                 (post {:scan :unrecognized} 400))))))))
+  (mt/test-drivers (mt/normal-drivers)
+    (let [table-name (->> (mt/db) database/tables first :name)
+          post       (fn post-api
+                       ([payload] (post-api payload 200))
+                       ([payload expected-code]
+                        (mt/with-temporary-setting-values [api-key "test-api-key"]
+                          (mt/client :post expected-code (format "notify/db/%d" (u/the-id (mt/db)))
+                                     {:request-options api-headers}
+                                     (merge {:synchronous? true}
+                                            payload)))))]
+      (testing "sync just table when table is provided"
+        (let [long-sync-called? (promise), short-sync-called? (promise)]
+          (with-redefs [metabase.sync/sync-table!                        (fn [_table] (deliver long-sync-called? true))
+                        metabase.sync.sync-metadata/sync-table-metadata! (fn [_table] (deliver short-sync-called? true))]
+            (post {:scan :full, :table_name table-name})
+            (is @long-sync-called?)
+            (is (not (realized? short-sync-called?))))))
+      (testing "only a quick sync when quick parameter is provided"
+        (let [long-sync-called? (promise), short-sync-called? (promise)]
+          (with-redefs [metabase.sync/sync-table!                        (fn [_table] (deliver long-sync-called? true))
+                        metabase.sync.sync-metadata/sync-table-metadata! (fn [_table] (deliver short-sync-called? true))]
+            (post {:scan :schema, :table_name table-name})
+            (is (not (realized? long-sync-called?)))
+            (is @short-sync-called?))))
+      (testing "full db sync by default"
+        (let [full-sync? (promise)]
+          (with-redefs [metabase.sync/sync-database! (fn [_db] (deliver full-sync? true))]
+            (post {})
+            (is @full-sync?))))
+      (testing "simple sync with params"
+        (let [full-sync?   (promise)
+              smaller-sync (promise)]
+          (with-redefs [metabase.sync/sync-database!                  (fn [_db] (deliver full-sync? true))
+                        metabase.sync.sync-metadata/sync-db-metadata! (fn [_db] (deliver smaller-sync true))]
+            (post {:scan :schema})
+            (is (not (realized? full-sync?)))
+            (is @smaller-sync))))
+      (testing "errors on unrecognized scan options"
+        (is (= {:errors
+                {:scan "value may be nil, or if non-nil, value must be one of: `full`, `schema`."}}
+               (post {:scan :unrecognized} 400)))))))