make sure all top-level API paths are present in OpenAPI doc (#48718)

src/metabase/api/common.clj

@@ -470,18 +470,20 @@
 
     (api/+check-superuser routes)"
   [handler]
-  (fn
-    ([request]
-     (check-superuser)
-     (handler request))
-    ([request respond raise]
-     (if-let [e (try
-                  (check-superuser)
-                  nil
-                  (catch Throwable e
-                    e))]
-       (raise e)
-       (handler request respond raise)))))
+  (with-meta
+   (fn
+     ([request]
+      (check-superuser)
+      (handler request))
+     ([request respond raise]
+      (if-let [e (try
+                   (check-superuser)
+                   nil
+                   (catch Throwable e
+                     e))]
+        (raise e)
+        (handler request respond raise))))
+   (meta handler)))
 
 ;;; ---------------------------------------- PERMISSIONS CHECKING HELPER FNS -----------------------------------------
 

src/metabase/api/common/openapi.clj

@@ -205,4 +205,9 @@
 (comment
   ;; See what is the result of generation, could be helpful debugging what's wrong with display in rapidoc
   ;; `resolve` is to appease clj-kondo which will complain for #'
-  (defendpoint->path-item nil "/path" (resolve 'metabase-enterprise.serialization.api/POST_export)))
+  (defendpoint->path-item nil "/path" (resolve 'metabase-enterprise.serialization.api/POST_export))
+  (openapi-object (resolve 'metabase.api.pulse/routes))
+  (->> (openapi-object (resolve 'metabase.api.routes/routes))
+       :paths
+       (map #(second (str/split (key %) #"/")))
+       set))

src/metabase/api/search.clj

@@ -30,13 +30,15 @@
                            :expires   (cookie-expiry)}))))
 
 (defn- +engine-cookie [handler]
-  (fn [request respond raise]
-    (if-let [new-engine (get-in request [:params :search_engine])]
-      (handler request (set-engine-cookie! respond new-engine) raise)
-      (handler (->> (get-in request [:cookies engine-cookie-name :value])
-                    (assoc-in request [:params :search_engine]))
-               respond
-               raise))))
+  (with-meta
+   (fn [request respond raise]
+     (if-let [new-engine (get-in request [:params :search_engine])]
+       (handler request (set-engine-cookie! respond new-engine) raise)
+       (handler (->> (get-in request [:cookies engine-cookie-name :value])
+                     (assoc-in request [:params :search_engine]))
+                respond
+                raise)))
+   (meta handler)))
 
 (api/defendpoint POST "/force-reindex"
   "If fulltext search is enabled, this will trigger a synchronous reindexing operation."

src/metabase/api/session.clj

@@ -338,12 +338,14 @@
                              :errors      {:account disabled-account-snippet}}))))))))
 
 (defn- +log-all-request-failures [handler]
-  (fn [request respond raise]
-    (try
-      (handler request respond raise)
-      (catch Throwable e
-        (log/error e "Authentication endpoint error")
-        (throw e)))))
+  (with-meta
+   (fn [request respond raise]
+     (try
+       (handler request respond raise)
+       (catch Throwable e
+         (log/error e "Authentication endpoint error")
+         (throw e))))
+   (meta handler)))
 
 ;;; ----------------------------------------------------- Unsubscribe non-users from pulses -----------------------------------------------
 

src/metabase/pulse/preview.clj

@@ -160,11 +160,13 @@
   a style tag with an attribute 'nonce=%NONCE%'. Specifcally, this was designed to be used with the
   endpoint `api/pulse/preview_dashboard/:id`."
   [only-this-uri handler]
-  (fn [request respond raise]
-    (let [{:keys [uri]} request]
-      (handler
-       request
-       (if (str/starts-with? uri only-this-uri)
-         (comp respond (partial add-style-nonce request))
-         respond)
-       raise))))
+  (with-meta
+   (fn [request respond raise]
+     (let [{:keys [uri]} request]
+       (handler
+        request
+        (if (str/starts-with? uri only-this-uri)
+          (comp respond (partial add-style-nonce request))
+          respond)
+        raise)))
+   (meta handler)))

src/metabase/server/middleware/exceptions.clj

@@ -18,30 +18,34 @@
   "Catch any exceptions other than 404 thrown in the request handler body and rethrow a generic 400 exception instead.
   This minimizes information available to bad actors when exceptions occur on public endpoints."
   [handler]
-  (fn [request respond _raise]
-    (let [raise (fn [e]
-                  (log/warn e "Exception in API call")
-                  (if (= 404 (:status-code (ex-data e)))
-                    (respond {:status 404, :body (deferred-tru "Not found.")})
-                    (respond {:status 400, :body (deferred-tru "An error occurred.")})))]
-      (try
-        (handler request respond raise)
-        (catch Throwable e
-          (raise e))))))
+  (with-meta
+   (fn [request respond _raise]
+     (let [raise (fn [e]
+                   (log/warn e "Exception in API call")
+                   (if (= 404 (:status-code (ex-data e)))
+                     (respond {:status 404, :body (deferred-tru "Not found.")})
+                     (respond {:status 400, :body (deferred-tru "An error occurred.")})))]
+       (try
+         (handler request respond raise)
+         (catch Throwable e
+           (raise e)))))
+   (meta handler)))
 
 (defn message-only-exceptions
   "Catch any exceptions thrown in the request handler body and rethrow a 400 exception that only has the message from
   the original instead (i.e., don't rethrow the original stacktrace). This reduces the information available to bad
   actors but still provides some information that will prove useful in debugging errors."
   [handler]
-  (fn [request respond _raise]
-    (let [raise (fn [^Throwable e]
-                  (respond {:status 400, :body (ex-message e)}))]
-      (try
-        (handler request respond raise)
-        (catch Throwable e
-          (log/error e "Exception in API call")
-          (raise e))))))
+  (with-meta
+   (fn [request respond _raise]
+     (let [raise (fn [^Throwable e]
+                   (respond {:status 400, :body (ex-message e)}))]
+       (try
+         (handler request respond raise)
+         (catch Throwable e
+           (log/error e "Exception in API call")
+           (raise e)))))
+   (meta handler)))
 
 (defmulti api-exception-response
   "Convert an uncaught exception from an API endpoint into an appropriate format to be returned by the REST API (e.g. a

test/metabase/api/common/openapi_test.clj

 (ns metabase.api.common.openapi-test
   (:require
+   [clojure.string :as str]
    [clojure.test :refer :all]
    [compojure.core :refer [GET POST]]
    [malli.json-schema :as mjs]
@@ -71,12 +72,21 @@
          (#'openapi/path->openapi "/:model/:yyyy-mm"))))
 
 (deftest ^:parallel collect-routes-test
-  (is (=? [{:path "/export"}
-           {:path "/rename"}
-           {:path "/{id}"}
-           {:path "/{id}"}
-           {:path "/{id}/upload"}]
-          (sort-by :path (#'openapi/collect-routes #'routes)))))
+  (testing "can collect routes in simple case"
+    (is (=? [{:path "/export"}
+             {:path "/rename"}
+             {:path "/{id}"}
+             {:path "/{id}"}
+             {:path "/{id}/upload"}]
+            (sort-by :path (#'openapi/collect-routes #'routes)))))
+  (testing "every top-level route only uses middlewares which correctly pass metadata\n"
+    (doseq [route (-> #'routes/routes meta :routes)
+            :when (meta route)
+            :let  [m (meta route)]]
+      (testing (cond-> (:path m)
+                 (:doc m) (str " - " (some-> (:doc m) str/split-lines first)))
+        (is (or (:method m)
+                (some? (->> route meta :routes (some meta)))))))))
 
 (deftest ^:parallel defendpoint->openapi-test
   (is (=? {:get