From 54d9d7789293ad51151d79540eea91d27ff118e0 Mon Sep 17 00:00:00 2001 From: Cam Saul <1455846+camsaul@users.noreply.github.com> Date: Tue, 28 Jan 2025 13:32:48 -0800 Subject: [PATCH] Remove defendpoint 1 (#52785) --- .clj-kondo/config.edn | 1 - .clj-kondo/src/hooks/metabase/api/common.clj | 32 -- .../test/hooks/metabase/api/common_test.clj | 35 -- .dir-locals.el | 4 - .lsp/config.edn | 6 +- dev/src/dev/rewrite_defendpoint_1.clj | 511 ------------------ src/metabase/api/common.clj | 155 +----- src/metabase/api/common/internal.clj | 234 +------- src/metabase/api/common/openapi.clj | 123 +---- src/metabase/models/setting.clj | 7 +- src/metabase/query_processor/streaming.clj | 2 +- test/metabase/api/common/internal_test.clj | 366 +------------ test/metabase/api/common/openapi_test.clj | 277 ++++------ 13 files changed, 129 insertions(+), 1624 deletions(-) delete mode 100644 .clj-kondo/src/hooks/metabase/api/common.clj delete mode 100644 .clj-kondo/test/hooks/metabase/api/common_test.clj delete mode 100644 dev/src/dev/rewrite_defendpoint_1.clj diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 0004929e306..74bf4255c92 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -887,7 +887,6 @@ metabase.api.card-test/with-temp-native-card! hooks.common/with-two-bindings metabase.api.card-test/with-temp-native-card-with-params! hooks.common/with-two-bindings metabase.api.collection-test/with-french-user-and-personal-collection! hooks.common/with-two-top-level-bindings - metabase.api.common/defendpoint hooks.metabase.api.common/defendpoint metabase.api.common.internal-test/with-jetty-server hooks.common/with-one-binding metabase.api.dashboard-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value metabase.api.dashboard-test/with-simple-dashboard-with-tabs hooks.common/with-one-binding diff --git a/.clj-kondo/src/hooks/metabase/api/common.clj b/.clj-kondo/src/hooks/metabase/api/common.clj deleted file mode 100644 index 187df91c6ec..00000000000 --- a/.clj-kondo/src/hooks/metabase/api/common.clj +++ /dev/null @@ -1,32 +0,0 @@ -(ns hooks.metabase.api.common - (:require - [clj-kondo.hooks-api :as api] - [clojure.string :as str] - [hooks.common])) - -(defn route-fn-name - "route fn hook" - [method route] - (let [route (if (vector? route) (first route) route)] - (-> (str (name method) route) - (str/replace #"/" "_") - symbol))) - -(defn defendpoint - [arg] - (letfn [(update-defendpoint [node] - (let [[_defendpoint method route & body] (:children node)] - (-> (api/list-node - (list - (api/token-node 'do) - (-> (api/token-node (symbol "compojure.core" (str (api/sexpr method)))) - (with-meta (meta method))) - (api/list-node - (list* - (api/token-node 'clojure.core/defn) - (-> (api/token-node (route-fn-name (api/sexpr method) (api/sexpr route))) - (with-meta (meta route))) - body)))) - (with-meta (meta node)) - hooks.common/add-lsp-ignore-unused-public-var-metadata)))] - (update arg :node update-defendpoint))) diff --git a/.clj-kondo/test/hooks/metabase/api/common_test.clj b/.clj-kondo/test/hooks/metabase/api/common_test.clj deleted file mode 100644 index 0402b5c1356..00000000000 --- a/.clj-kondo/test/hooks/metabase/api/common_test.clj +++ /dev/null @@ -1,35 +0,0 @@ -(ns hooks.metabase.api.common-test - (:require - [clj-kondo.hooks-api :as api] - [clojure.test :refer :all] - [hooks.metabase.api.common])) - -(deftest ^:parallel defendpoint-test - (is (= '(do - compojure.core/POST - (clojure.core/defn - POST_:id_copy - "Copy a `Card`, with the new name 'Copy of _name_'" - [id] - {id [:maybe ms/PositiveInt]} - (let [orig-card (api/read-check Card id) - new-name (str (trs "Copy of ") (:name orig-card)) - new-card (assoc orig-card :name new-name)] - (-> (card/create-card! new-card @api/*current-user*) - hydrate-card-details - (assoc :last-edit-info (last-edit/edit-information-for-user @api/*current-user*)))))) - (-> {:node (-> '(api/defendpoint POST "/:id/copy" - "Copy a `Card`, with the new name 'Copy of _name_'" - [id] - {id [:maybe ms/PositiveInt]} - (let [orig-card (api/read-check Card id) - new-name (str (trs "Copy of ") (:name orig-card)) - new-card (assoc orig-card :name new-name)] - (-> (card/create-card! new-card @api/*current-user*) - hydrate-card-details - (assoc :last-edit-info (last-edit/edit-information-for-user @api/*current-user*))))) - pr-str - api/parse-string)} - hooks.metabase.api.common/defendpoint - :node - api/sexpr)))) diff --git a/.dir-locals.el b/.dir-locals.el index 25926c26f4c..aad57d9e25e 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -20,11 +20,7 @@ (clojure-mode ;; Specify which arg is the docstring for certain macros ;; (Add more as needed) - (eval . (put 'defendpoint-schema 'clojure-doc-string-elt 3)) - (eval . (put 'defendpoint 'clojure-doc-string-elt 3)) (eval . (put 'define-premium-feature 'clojure-doc-string-elt 2)) - (eval . (put 'api/defendpoint-schema 'clojure-doc-string-elt 3)) - (eval . (put 'api/defendpoint 'clojure-doc-string-elt 3)) (eval . (put 'api.macros/defendpoint 'clojure-doc-string-elt 3)) (eval . (put 'defsetting 'clojure-doc-string-elt 2)) (eval . (put 'setting/defsetting 'clojure-doc-string-elt 2)) diff --git a/.lsp/config.edn b/.lsp/config.edn index 0e767070d94..e4fc0388be4 100644 --- a/.lsp/config.edn +++ b/.lsp/config.edn @@ -4,8 +4,4 @@ :classpath-cmd ["clojure" "-A:dev:ee:ee-dev:drivers:drivers-dev:build:build/all" "-Spath"]}] - :clean {:ns-inner-blocks-indentation :keep} - :linters {:clojure-lsp/unused-public-var {:level :warning - :exclude-when-defined-by - #{metabase.api.common/defendpoint-schema - metabase.api.common/defendpoint}}}} + :clean {:ns-inner-blocks-indentation :keep}} diff --git a/dev/src/dev/rewrite_defendpoint_1.clj b/dev/src/dev/rewrite_defendpoint_1.clj deleted file mode 100644 index ac273728411..00000000000 --- a/dev/src/dev/rewrite_defendpoint_1.clj +++ /dev/null @@ -1,511 +0,0 @@ -(ns dev.rewrite-defendpoint-1 - (:require - [clojure.string :as str] - [flatland.ordered.map :as ordered-map] - [metabase.util.malli :as mu] - [metabase.util.malli.registry :as mr] - [rewrite-clj.node :as n] - [rewrite-clj.parser :as r.parser] - [rewrite-clj.zip :as z])) - -(set! *warn-on-reflection* true) - -(mr/def ::zloc vector?) - -(mr/def ::defendpoint-loc - [:and - ::zloc - [:fn - {:error/message "zipper location pointing to list node"} - #(= (z/tag %) :list)]]) - -(mr/def ::node - [:fn - {:error/message "Valid rewrite-clj node"} - (fn node? [x] - (and - (n/node? x) - (or (not (n/inner? x)) - (every? node? (n/children x)))))]) - -(mr/def ::schema-map - "Map of raw symbol => schema node." - [:maybe - [:map-of symbol? ::node]]) - -(mr/def ::parsed - [:map - [:route ::node] - [:method ::node] - [:fn-args ::node] - [:schema-map ::schema-map]]) - -(mu/defn- parse-defendpoint-1-args :- ::parsed - [defendpoint :- ::defendpoint-loc] - (let [symb (z/down defendpoint) - method (z/right symb) - route (z/right method) - args (z/right route) - ;; skip over docstring - fn-args (if (string? (z/sexpr args)) - (z/right args) - args) - args (z/right fn-args) - schema-map (when (and (= (z/tag args) :map) - ;; there has to be more stuff after this - (z/right args)) - (into (ordered-map/ordered-map) - (comp (take-while some?) - (partition-all 2) - (map (fn [[k v]] - [(z/sexpr k) (z/node v)]))) - (iterate z/right (z/down args))))] - {:route (z/node route) - :method (z/node method) - :fn-args (z/node fn-args) - :schema-map schema-map})) - -(mu/defn- route-query-args-symbols :- [:sequential symbol?] - [{:keys [fn-args], :as _parsed} :- ::parsed] - (into [] - (comp (take-while some?) - (take-while #(not= (z/sexpr %) :as)) - (map z/sexpr)) - (iterate z/right (z/down (z/of-node fn-args))))) - -(defn- max-key-length [m] - (transduce - (comp (map str) - (map count)) - max - Long/MIN_VALUE - (keys m))) - -(defn- schema-should-be-optional? [node] - (case (n/tag node) - :token ('#{:any ms/MaybeBooleanValue} (n/sexpr node)) - :vector (= (n/sexpr (first (n/children node))) :maybe) - :list (and (= (n/sexpr (first (n/children node))) 'mu/with) - (schema-should-be-optional? (second (n/children node)))) - false)) - -(mu/defn- schema-map->malli :- [:maybe ::node] - [schema-map :- ::schema-map] - (when (seq schema-map) - (n/vector-node - (into [(n/keyword-node :map)] - (mapcat (let [max-key-length (max-key-length schema-map)] - (fn [[k schema]] - [(n/newline-node "\n") - (n/whitespace-node " ") - (n/vector-node - (concat - [(n/keyword-node (keyword k)) - ;; pad the space between key and schema so the map is nicely aligned. - (n/whitespace-node (str/join (repeat (- (+ max-key-length 2) - (count (str (keyword k)))) - \space)))] - ;; add `:optional` or `:default` values to the map as needed - (when (schema-should-be-optional? schema) - [(n/map-node - ;; apparently the old behavior for booleans coerced `nil`, to `false`, so let's replicate - ;; that. - (if (= (n/sexpr schema) [:maybe 'ms/BooleanValue]) - [(n/keyword-node :default) (n/whitespace-node " ") (n/token-node 'false)] - [(n/keyword-node :optional) (n/whitespace-node " ") (n/token-node 'true)])) - (n/whitespace-node " ")]) - [schema]))]))) - schema-map)))) - -(defn- select-keys* - "Normal Clojure select-keys does not preserve the original map type e.g. ordered-map." - [m ks] - (when m - (into (empty m) - (filter (let [key-set (set ks)] - (fn [[k v]] - (when (key-set k) - [k v])))) - m))) - -(mu/defn- keys-map :- ::node - [args :- [:sequential symbol?]] - (n/map-node [(n/keyword-node :keys) - (n/whitespace-node " ") - (n/vector-node (into [] - (comp (map n/token-node) - (interpose (n/whitespace-node " "))) - args))])) - -(mu/defn- unused-binding? [node :- ::node] - (and (= (n/tag node) :token) - (str/starts-with? (n/sexpr node) "_"))) - -(mu/defn- schema-specifier :- [:maybe [:sequential ::node]] - [schema :- [:maybe ::node]] - (when schema - [(n/whitespace-node " ") - (n/keyword-node :-) - (n/whitespace-node " ") - schema])) - -;;; -;;; route params -;;; - -(declare query-args-binding - body-binding) - -(mu/defn- route-args-symbols :- [:sequential symbol?] - [{:keys [route], :as _parsed}] - (let [route (if (= (n/tag route) :vector) - (-> route z/of-node z/down z/node) - route) - route (n/sexpr route)] - (assert (string? route) - (format "Expected route string, got %s" (pr-str route))) - (map (comp symbol second) (re-seq #":([^:/]+)" route)))) - -(mu/defn- route-args-schema :- [:maybe ::node] - [{:keys [schema-map], :as parsed} :- ::parsed] - (some-> schema-map - (select-keys* (route-args-symbols parsed)) - ;; route args cannot be optional so fix incorrect schemas if we see them. - (update-vals (fn [schema] - (if (and (= (n/tag schema) :vector) - (= (first (n/sexpr schema)) :maybe)) - (z/node (-> (z/of-node schema) z/next z/next)) - schema))) - schema-map->malli)) - -(mu/defn- route-args-binding :- ::node - [parsed] - (if-let [args (not-empty (route-args-symbols parsed))] - (keys-map args) - (n/token-node '_route-params))) - -(mu/defn- new-route-param-arg-nodes :- [:maybe [:sequential ::node]] - [parsed] - (when (or (not (unused-binding? (route-args-binding parsed))) - (not (unused-binding? (query-args-binding parsed))) - (not (unused-binding? (body-binding parsed)))) - (list* - (route-args-binding parsed) - (schema-specifier (route-args-schema parsed))))) - -;;; -;;; query params -;;; - -(mu/defn- query-args-symbols :- [:sequential symbol?] - [parsed] - (remove (set (route-args-symbols parsed)) - (route-query-args-symbols parsed))) - -(mu/defn- query-args-binding :- ::node - [parsed] - (if-let [args (not-empty (query-args-symbols parsed))] - (keys-map args) - (n/token-node '_query-params))) - -(mu/defn- query-args-schema :- [:maybe ::node] - [{:keys [schema-map], :as parsed} :- ::parsed] - (some-> schema-map - (select-keys* (query-args-symbols parsed)) - schema-map->malli)) - -(mu/defn- new-query-param-arg-nodes :- [:maybe [:sequential ::node]] - [parsed :- ::parsed] - (when (or (not (unused-binding? (query-args-binding parsed))) - (not (unused-binding? (body-binding parsed)))) - (list* - (n/newline-node "\n") - (n/whitespace-node " ") - (query-args-binding parsed) - (schema-specifier (query-args-schema parsed))))) - -;;; -;;; body -;;; - -(mu/defn- body-binding :- ::node - [{:keys [fn-args], :as _parsed} :- ::parsed] - (or (when-let [as (z/find (z/down (z/of-node fn-args)) - #(= (z/sexpr %) :as))] - (let [request-map (z/right as)] - (assert (= #{:body} - (set (vals (z/sexpr request-map)))) - (format "Don't know how to rewrite request map with non-:body keys: %s" (pr-str request-map))) - (when-let [body-key (z/find (z/down request-map) - #(= (z/sexpr %) :body))] - (z/node (z/left body-key))))) - (n/token-node '_body))) - -(mu/defn- body-schema :- [:maybe ::node] - [{:keys [schema-map], :as parsed} :- ::parsed] - (when schema-map - (if (= (n/tag (body-binding parsed)) :token) - ;; body is a plain symbol, do not build a the usual map schema. - (get schema-map (n/sexpr (body-binding parsed))) - ;; otherwise body is a destructured map - (let [body-keys (->> schema-map - keys - (remove (set (route-args-symbols parsed))) - (remove (set (query-args-symbols parsed))))] - (schema-map->malli (select-keys* schema-map body-keys)))))) - -(mu/defn- new-body-arg-nodes :- [:maybe [:sequential ::node]] - [parsed :- ::parsed] - (when-not (unused-binding? (body-binding parsed)) - (list* - (n/newline-node "\n") - (n/whitespace-node " ") - (body-binding parsed) - (schema-specifier (body-schema parsed))))) - -;;; -;;; rewriting defendpoint symbol -;;; - -(mu/defn- rewrite-defendpoint-symbol :- ::zloc - [_parsed :- ::parsed - defendpoint :- ::defendpoint-loc] - (z/replace (z/down defendpoint) (n/token-node 'api.macros/defendpoint))) - -;;; -;;; rewrite method -;;; - -(mu/defn- rewrite-method :- ::zloc - [_parsed defendpoint :- ::defendpoint-loc] - (let [method (-> defendpoint z/down z/right)] - (z/replace method (n/keyword-node (keyword (str/lower-case (name (z/sexpr method)))))))) - -;;; -;;; rewrite argslist -;;; - -(mu/defn- find-route :- [:and - ::zloc - [:fn - {:error/message "zipper location pointing to route node (string or vector)"} - #(or (#{:vector :string} (z/tag %)) - ;; for whatever reason sometimes string nodes are `:token` nodes? - (string? (z/sexpr %)))]] - [defendpoint :- ::defendpoint-loc] - (-> defendpoint - z/down ; symb - z/right ; method - z/right)) ; route - -(mu/defn- find-argslist :- [:and - ::zloc - [:fn - {:error/message "zipper location pointing to vector node"} - #(= (z/tag %) :vector)]] - [defendpoint :- ::defendpoint-loc] - (let [route (find-route defendpoint)] - ;; first vector after route - (z/find-next route #(= (z/tag %) :vector)))) - -(mu/defn- new-args-node :- ::node - [parsed :- ::parsed] - (n/vector-node - (into [] - (mapcat (fn [f] - (f parsed))) - [new-route-param-arg-nodes - new-query-param-arg-nodes - new-body-arg-nodes]))) - -(mu/defn- rewrite-args :- ::zloc - [parsed :- ::parsed - defendpoint :- ::defendpoint-loc] - (z/replace (find-argslist defendpoint) (new-args-node parsed))) - -;;; -;;; Add metadata map if needed -;;; - -(mu/defn- add-metadata-map :- ::zloc - [parsed :- ::parsed - defendpoint :- ::defendpoint-loc] - (if (= (n/tag (:method parsed)) :meta) - (let [metadata (first (n/children (:method parsed))) - node (if (= (n/tag metadata) :token) - ;; TODO -- what about multiple tokens? Does this work? - (n/map-node [metadata (n/whitespace-node " ") (n/token-node true)]) - metadata)] - (as-> defendpoint defendpoint - (z/subedit-node defendpoint (fn [defendpoint] (z/insert-left (find-argslist defendpoint) node))) - (z/subedit-node defendpoint (fn [defendpoint] (z/insert-left (find-argslist defendpoint) (n/newline-node "\n")))))) - defendpoint)) - -;;; -;;; rewrite body -;;; - -(mu/defn- find-body :- ::zloc - [defendpoint :- ::defendpoint-loc] - (z/right (find-argslist defendpoint))) - -(mu/defn- rewrite-body :- ::zloc - [_parsed :- ::parsed - defendpoint :- ::defendpoint-loc] - (let [body (find-body defendpoint)] - ;; if the first form is a map but there are more forms after, it's the schema map and we can remove it. - (if (and (= (z/tag body) :map) - (z/right body)) - (z/remove body) - body))) - -;;; -;;; rewrite entire form -;;; - -(mu/defn- rewrite-defendpoint* :- ::zloc - [defendpoint :- ::defendpoint-loc] - (let [parsed (parse-defendpoint-1-args defendpoint)] - (reduce - (fn [defendpoint f] - (z/subedit-node defendpoint (partial f parsed))) - defendpoint - [rewrite-defendpoint-symbol - rewrite-method - add-metadata-map - rewrite-args - rewrite-body]))) - -(mu/defn- defendpoint-node? [node :- ::node] - (and - (= (n/tag node) :list) - (let [first-child (first (n/children node))] - (and - (n/symbol-node? first-child) - (= (n/sexpr first-child) 'api/defendpoint))))) - -(mu/defn- kondo-ignore-node? [node :- ::node] - (and - (= (n/tag node) :uneval) - (= (n/sexpr (first (n/children node))) - {:clj-kondo/ignore [:deprecated-var]}))) - -(mu/defn- remove-preceding-kondo-ignore-node :- [:maybe ::zloc] - [defendpoint :- ::defendpoint-loc] - (when-let [previous (z/left defendpoint)] - (when (some-> previous z/node kondo-ignore-node?) - (-> (z/subedit-node - (z/up defendpoint) ; move up to top-level node so the kondo ignore form is subeditable - (fn [_] - (z/remove previous))) - ; move back to first child - z/down)))) - -(mu/defn- rewrite-defendpoint :- ::defendpoint-loc - [defendpoint :- ::defendpoint-loc] - (try - (-> defendpoint - rewrite-defendpoint* - remove-preceding-kondo-ignore-node) - (catch Throwable e - (println "Error rewriting defendpoint:" e) - defendpoint))) - -(mu/defn- rewrite-all-defendpoints :- ::zloc - [zloc :- ::zloc] - (if-let [defendpoint (z/find-next zloc #(defendpoint-node? (z/node %)))] - (let [zloc' (rewrite-defendpoint defendpoint)] - (recur zloc')) - zloc)) - -(mu/defn- ns-node? - [node :- ::node] - (when (= (n/tag node) :list) - (let [[first-child] (n/children node)] - (and (= (n/tag first-child) :token) - (= (n/sexpr first-child) 'ns))))) - -(mu/defn- find-ns-node :- ::zloc - [zloc :- ::zloc] - (z/find-depth-first zloc #(ns-node? (z/node %)))) - -(mu/defn- require-node? - [node :- ::node] - (when (= (n/tag node) :list) - (let [[first-child] (n/children node)] - (and (= (n/tag first-child) :token) - (= (n/sexpr first-child) :require))))) - -(mu/defn- find-require :- ::zloc - [zloc :- ::zloc] - (let [ns-node-loc (find-ns-node zloc)] - (z/find-depth-first ns-node-loc #(require-node? (z/node %))))) - -(mu/defn- has-api-macros-require? - [require-loc :- ::zloc] - (z/find-next - (z/down require-loc) - (fn [zloc] - (and (= (z/tag zloc) :vector) - (let [[first-child] (n/children (z/node zloc))] - (and (= (n/tag first-child) :token) - (= (n/sexpr first-child) 'metabase.api.macros))))))) - -(mu/defn- add-api-macros-require :- ::zloc - [require-loc :- ::zloc] - ;; keep iterating thru the requires until we find one that should appear after the `metabase.api.macros` - (letfn [(required-namespace-symb [zloc] - (when (= (z/tag zloc) :vector) - (let [[first-child] (n/children (z/node zloc))] - (when (= (n/tag first-child) :token) - (n/sexpr first-child))))) - (before-or-after [zloc] - (or (when-let [symb (required-namespace-symb zloc)] - (when (= (sort [symb 'metabase.api.macros]) - [symb 'metabase.api.macros]) - :before)) - :after))] - (let [preceding-zloc (loop [zloc (z/find-depth-first require-loc required-namespace-symb)] - (case (before-or-after zloc) - :before (if (z/rightmost? zloc) - zloc - (recur (z/right zloc))) - :after (z/left zloc)))] - (-> preceding-zloc - (z/insert-right (n/vector-node - [(n/token-node 'metabase.api.macros) - (n/whitespace-node " ") - (n/keyword-node :as) - (n/whitespace-node " ") - (n/token-node 'api.macros)])) - (z/insert-right (n/whitespace-node " ")) - (z/insert-right (n/newline-node "\n")))))) - -(mu/defn- add-macros-namespace-require :- ::zloc - [top-level-forms :- ::zloc] - (z/subedit-node - top-level-forms - (fn [top-level-forms] - (let [require-loc (find-require top-level-forms)] - (if (has-api-macros-require? require-loc) - require-loc - (add-api-macros-require require-loc)))))) - -(mu/defn- rewrite-namespace :- ::zloc - [top-level-forms :- ::zloc] - (-> top-level-forms - add-macros-namespace-require - rewrite-all-defendpoints)) - -(defn rewrite-file! [{:keys [^String filename write?], :or {write? true}}] - {:pre [(string? filename)]} - (println "Rewriting" filename) - (let [node (r.parser/parse-file-all filename) - top-level-forms (-> (z/of-node node) - rewrite-namespace)] - (letfn [(print-root [w] - (z/print-root top-level-forms w))] - (if write? - (with-open [w (java.io.FileWriter. filename)] - (print-root w)) - (print-root *out*))))) diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj index 2cbf984247b..12a6603031e 100644 --- a/src/metabase/api/common.clj +++ b/src/metabase/api/common.clj @@ -79,30 +79,17 @@ (ns metabase.api.common "Dynamic variables and utility functions/macros for writing API functions." (:require - [clojure.set :as set] - [clojure.spec.alpha :as s] - [clojure.string :as str] [clojure.tools.macro :as macro] [compojure.core :as compojure] - [medley.core :as m] - [metabase.api.common.internal - :refer [add-route-param-schema - auto-coerce - route-dox - route-fn-name - validate-params - wrap-response-if-needed]] [metabase.api.common.openapi :as openapi] [metabase.config :as config] [metabase.events :as events] [metabase.models.interface :as mi] [metabase.util :as u] [metabase.util.i18n :as i18n :refer [deferred-tru tru]] - [metabase.util.log :as log] [metabase.util.malli :as mu] [metabase.util.malli.schema :as ms] [potemkin :as p] - [ring.middleware.multipart-params :as mp] [toucan2.core :as t2])) (declare check-403 check-404) @@ -302,118 +289,6 @@ "A 'No Content' response for `DELETE` endpoints to return." {:status 204, :body nil}) -;;; --------------------------------------- DEFENDPOINT AND RELATED FUNCTIONS ---------------------------------------- - -(s/def ::defendpoint-args - (s/cat - :method symbol? - :route (some-fn string? sequential?) - :docstr (s/? string?) - :args vector? - :arg->schema (s/? (s/map-of symbol? any?)) ;; any? is either a plumatic or malli schema - :body (s/* any?))) - -(defn- parse-defendpoint-args [args] - (let [parsed (s/conform ::defendpoint-args args)] - (when (= parsed ::s/invalid) - (throw (ex-info (str "Invalid defendpoint args: " (s/explain-str ::defendpoint-args args)) - (s/explain-data ::defendpoint-args args)))) - (let [{:keys [method route docstr args arg->schema body]} parsed - fn-name (route-fn-name method route) - route (add-route-param-schema arg->schema route) - ;; eval the vals in arg->schema to make sure the actual schemas are resolved so we can document - ;; their API error messages - route-doc (route-dox method route docstr args - (m/map-vals #_{:clj-kondo/ignore [:discouraged-var]} eval arg->schema) - body)] - ;; Don't i18n this, it's dev-facing only - (when-not docstr - (log/warn (u/format-color 'red "Warning: endpoint %s/%s does not have a docstring. Go add one." - (ns-name *ns*) fn-name))) - (assoc parsed :fn-name fn-name, :route route, :route-doc route-doc, :docstr docstr)))) - -(defn validate-param-values - "Log a warning if the request body contains any parameters not included in `expected-params` (which is presumably - populated by the defendpoint schema)" - [{method :request-method uri :uri body :body} expected-params] - (when (and (not config/is-prod?) - (map? body)) - (let [extraneous-params (set/difference (set (keys body)) - (set expected-params))] - (when (seq extraneous-params) - (log/warnf "Unexpected parameters at %s: %s\nPlease add them to the schema or remove them from the API client" - [method uri] (vec extraneous-params)))))) - -(defn method-symbol->keyword - "Convert Compojure-style HTTP method symbols (PUT, POST, etc.) to the keywords used internally by - Compojure (:put, :post, ...)" - [method-symbol] - (-> method-symbol - name - u/lower-case-en - keyword)) - -(defmacro defendpoint* - "Impl macro for [[defendpoint]]; don't use this directly." - [{:keys [method route fn-name route-doc docstr args body arg->schema]}] - {:pre [(or (string? route) (vector? route))]} - (let [method-kw (method-symbol->keyword method) - allowed-params (mapv keyword (keys arg->schema)) - prep-route #'compojure/prepare-route - multipart? (get (meta method) :multipart false) - handler-wrapper (if multipart? mp/wrap-multipart-params identity) - schema (into [:map] (for [[k v] arg->schema] - [(keyword k) v])) - quoted-args (list 'quote args)] - `(def ~(vary-meta fn-name - merge - {:doc route-doc - :orig-doc docstr - :method method-kw - :path route - :schema schema - :args quoted-args - :is-endpoint? true} - (meta method)) - ;; The next form is a copy of `compojure/compile-route`, with the sole addition of the call to - ;; `validate-param-values`. This is because to validate the request body we need to intercept the request - ;; before the destructuring takes place. I.e., we need to validate the value of `(:body request#)`, and that's - ;; not available if we called `compile-route` ourselves. - (compojure/make-route - ~method-kw - ~(prep-route route) - (~handler-wrapper - (fn [request#] - (validate-param-values request# (quote ~allowed-params)) - (compojure/let-request [~args request#] - ~@body))))))) - -(defmacro ^{:deprecated "0.53.0"} defendpoint - "DEPRECATED! Use [[metabase.api.macros/defendpoint]] instead going forward. - - Define an API function. - This automatically does several things: - - - converts `route` from a simple form like `\"/:id\"` to a regex-typed one like `[\"/:id\" :id #\"[0-9]+\"]` based - on its malli schema - - - sequentially applies specified annotation functions on args to validate them. - - - automatically calls `wrap-response-if-needed` on the result of `body` - - - tags function's metadata in a way that subsequent calls to `define-routes` (see below) will automatically include - the function in the generated `defroutes` form. - - - Generates a super-sophisticated Markdown-formatted docstring" - {:arglists '([method route docstr? args schemas-map? & body])} - [& defendpoint-args] - (let [{:keys [args body arg->schema], :as defendpoint-args} (parse-defendpoint-args defendpoint-args)] - `(defendpoint* ~(assoc defendpoint-args - :body `((auto-coerce ~args ~arg->schema - ~@(validate-params arg->schema) - (wrap-response-if-needed - (do ~@body)))))))) - (defn- pass-thru-handler ([_request] nil) ([_request respond _raise] (respond nil))) @@ -436,24 +311,6 @@ ;; it's not defined just use the [[pass-thru-handler]] above instead. (or (handler-fn) pass-thru-handler)))) -(defn- namespace->defendpoint-1-route-vars - "Return a sequence of all API endpoint functions defined by `defendpoint` in a namespace." - [nmspace] - (for [[_symb varr] (ns-publics nmspace) - :when (:is-endpoint? (meta varr))] - varr)) - -(defn- api-routes-docstring [nmspace route-fns middleware] - (str - (format "Ring routes for %s:\n%s" - (-> (ns-name nmspace) - (str/replace #"^metabase\." "") - (str/replace #"\." "/")) - (u/pprint-to-str route-fns)) - (when (seq middleware) - (str "\nMiddleware applied to all endpoints in this namespace:\n" - (u/pprint-to-str middleware))))) - (defmacro define-routes "Create a `(defroutes routes ...)` form that automatically includes all functions created with `defendpoint` in the current namespace. Optionally specify middleware that will apply to all of the endpoints in the current namespace. @@ -461,14 +318,10 @@ (api/define-routes api/+check-superuser) ; all API endpoints in this namespace will require superuser access" {:style/indent 0} [& middleware] - (let [api-route-fns (vec (namespace->defendpoint-1-route-vars *ns*)) - routes `(-> (compojure/routes - (defendpoint-2-handler '~(ns-name *ns*)) - ~@api-route-fns) - (with-meta {:routes ~api-route-fns, :api/defendpoint-2-namespace '~(ns-name *ns*)})) + (let [routes `(-> (defendpoint-2-handler '~(ns-name *ns*)) + (with-meta {:api/defendpoint-2-namespace '~(ns-name *ns*)})) docstring (str "Routes for " *ns*)] `(def ~(vary-meta 'routes assoc - :doc (api-routes-docstring *ns* api-route-fns middleware) ;; not really super clear whether the expectation is someone should be looking at the metadata on ;; the varr ;; @@ -478,8 +331,8 @@ ;; ;; (meta metabase.api.timeline/routes) ;; - ;; ... the [[metabase.api.common.openapi]] code seems to do a little of both, so just support both for right now. - :routes api-route-fns + ;; ... the [[metabase.api.common.openapi]] code seems to do a little of both, so just support both + ;; for right now. :api/defendpoint-2-namespace (list 'quote (ns-name *ns*))) ~docstring ~(if (seq middleware) diff --git a/src/metabase/api/common/internal.clj b/src/metabase/api/common/internal.clj index 782bc846dd6..8a88a0aeaf6 100644 --- a/src/metabase/api/common/internal.clj +++ b/src/metabase/api/common/internal.clj @@ -1,155 +1,18 @@ (ns metabase.api.common.internal - "Internal functions used by [[metabase.api.common]]. - These are primarily used as the internal implementation of legacy [[metabase.api.common/defendpoint]]. Most of this - stuff can be removed once we remove the legacy implementation." + "Internal functions used by [[metabase.api.common]]." (:require - [clojure.string :as str] [clojure.walk :as walk] [malli.core :as mc] - [malli.error :as me] [malli.transform :as mtx] - [metabase.config :as config] - [metabase.server.streaming-response :as streaming-response] + [metabase.server.streaming-response] [metabase.util :as u] - [metabase.util.i18n :refer [tru]] - [metabase.util.log :as log] - [metabase.util.malli :as mu] - [metabase.util.malli.describe :as umd] - [metabase.util.malli.registry :as mr] - [metabase.util.malli.schema :as ms] [potemkin.types :as p.types]) (:import (metabase.server.streaming_response StreamingResponse))) (set! *warn-on-reflection* true) -(comment streaming-response/keep-me) - -(defn- handle-nonstandard-namespaces - "HACK to make sure some enterprise endpoints are consistent with the code. - The right way to fix this is to move them -- see #22687" - [name] - (let [replacements - [;; Standard API replacement - ["^metabase\\.api\\." "/api/"] - ;; /api/ee/sandbox/table -> /api/table, this is an override route for /api/table if sandbox is available - ["^metabase-enterprise\\.sandbox\\.api\\.table" "/api/table"] - ;; /api/ee/sandbox -> /api/mt - ["^metabase-enterprise\\.sandbox\\.api\\." "/api/mt/"] - ;; /api/ee/content-verification -> /api/moderation-review - ["^metabase-enterprise\\.content-verification\\.api\\." "/api/moderation-review/"] - ;; /api/ee/sso/sso/ -> /auth/sso - ["^metabase-enterprise\\.sso\\.api\\." "/auth/"] - ["^metabase-enterprise\\.serialization\\.api" "/api/ee/serialization"] - ["^metabase-enterprise\\.advanced-config\\.api\\.logs" "/api/ee/logs"] - ["^metabase-enterprise\\.query-reference-validation\\.api" "/api/ee/query-reference-validation"] - ["^metabase-enterprise\\.upload-management\\.api" "/api/ee/upload-management"] - ;; this should be the only replace for enterprise once we resolved #22687 - ["^metabase-enterprise\\.([^\\.]+)\\.api\\." "/api/ee/$1/"]]] - (reduce (fn [result [pattern replacement]] - (str/replace result (re-pattern pattern) replacement)) - name - replacements))) - -(defn- endpoint-name - "Generate a string like `GET /api/meta/db/:id` for a defendpoint route." - ([method route] - (endpoint-name *ns* method route)) - - ([endpoint-namespace method route] - (format "%s %s%s" - (name method) - (-> (.getName (the-ns endpoint-namespace)) - handle-nonstandard-namespaces) - (if (vector? route) - (first route) - route)))) - -(defn- args-form-flatten - "A version of `flatten` that will actually flatten a form such as: - - [id :as {{:keys [dataset_query description display name visualization_settings]} :body}]" - [form] - (cond - (map? form) (args-form-flatten (mapcat (fn [[k v]] - [(args-form-flatten k) (args-form-flatten v)]) - form)) - (sequential? form) (mapcat args-form-flatten form) - :else [form])) - -(defn- args-form-symbols - "Return a map of arg -> nil for args taken from the arguments vector. This map is merged with the ones found in the - schema validation map to build a complete map of args used by the endpoint." - [form] - (into {} (for [arg (args-form-flatten form) - :when (and (symbol? arg) - (not= arg 'body))] - {arg nil}))) - -(defn- dox-for-schema - "Generate the docstring for `schema` for use in auto-generated API documentation." - [schema route-str] - (try - ;; we can ignore the warning printed by umd/describe when schema is `nil`. - (binding [*out* (new java.io.StringWriter)] - (umd/describe schema)) - (catch Exception e - (when (and schema config/is-dev?) ; schema is nil for any var without a schema. That's ok! - (log/warn - e - (u/format-color 'red "Invalid Malli Schema: %s defined at %s" - (u/pprint-to-str schema) - (u/add-period route-str)))) - ""))) - -(defn- param-name - "Return the appropriate name for this `param-symb` based on its `schema`. Usually this is just the name of the - `param-symb`, but if the schema used a call to `su/api-param` we;ll use that name instead." - [param-symb schema] - (or (when (record? schema) - (:api-param-name schema)) - (name param-symb))) - -(defn- format-route-schema-dox - "Generate the `params` section of the documentation for a `defendpoint`-defined function by using the - `param-symb->schema` map passed in after the argslist." - [param-symb->schema route-str] - (let [params (for [[param-symb schema] param-symb->schema - :let [p-name (param-name param-symb schema) - p-desc (dox-for-schema schema route-str)] - :when (not (or - (str/blank? p-name) - (= "_" p-name)))] - (format "- **`%s`** %s" - p-name - (u/add-period p-desc)))] - (when (seq params) - (str "\n\n### PARAMS:\n\n" - (str/join "\n\n" params))))) - -(defn- format-route-dox - "Return a markdown-formatted string to be used as documentation for a `defendpoint` function." - [route-str docstr param->schema] - (str (format "## `%s`" route-str) - (when (seq docstr) - (str "\n\n" (u/add-period docstr))) - (format-route-schema-dox param->schema route-str))) - -(defn- contains-superuser-check? - "Does the `body` of this `defendpoint` form contain a call to `check-superuser`?" - [body] - (let [body (set body)] - (or (contains? body '(check-superuser)) - (contains? body '(api/check-superuser))))) - -(defn route-dox - "Prints a markdown route doc for defendpoint" - [method route docstr args param->schema body] - (format-route-dox (endpoint-name method route) - (str (u/add-period docstr) (when (contains-superuser-check? body) - "\n\nYou must be a superuser to do this.")) - (merge (args-form-symbols args) - param->schema))) +(comment metabase.server.streaming-response/keep-me) (defn route-arg-keywords "Return a sequence of keywords for URL args in string `route`. @@ -188,43 +51,6 @@ 'uuid? u/uuid-regex nil)])) -(def ^:private no-regex-schemas #{(mc/type ms/NonBlankString) - (mc/type (mc/schema [:maybe ms/PositiveInt])) - (mc/type [:enum "a" "b"]) - :fn - :string - :or}) - -(defn add-route-param-schema - "Expand a `route` string like \"/:id\" into a Compojure route form with regexes to match parameters based on - malli schemas given in the `arg->schema` map. - - (add-route-param-schema '{id :int} \"/:id/card\") -> [\"/:id/card\" :id #\"[0-9]+\"] - (add-route-param-schema {} \"/:id/card\") -> \"/:id/card\"" - [arg->schema route] - (if (vector? route) - route - (let [[wildcard & wildcards] - (->> (for [[k schema] arg->schema - :when (re-find (re-pattern (str ":" k)) route) - :let [[schema-type re] (->matching-regex schema)]] - (if re - [route (keyword k) re] - (when (and config/is-dev? (not (contains? no-regex-schemas schema-type))) - (let [overview (str "Warning: missing route-param regex for schema: " - route " " [k schema]) - fix (str "Either add `" (pr-str schema-type) "` to " - "metabase.api.common.internal/->matching-regex or " - "metabase.api.common.internal/no-regex-schemas.")] - (log/warn (u/colorize :red overview)) - (log/warn (u/colorize :green fix)))))) - (remove nil?))] - (cond - ;; multiple hits -> tack them onto the original route shape. - wildcards (vec (reduce into wildcard (mapv #(drop 1 %) wildcards))) - wildcard wildcard - :else route)))) - (def defendpoint-transformer "Transformer used on values coming over the API via defendpoint." (mtx/transformer @@ -232,60 +58,6 @@ (mtx/json-transformer) (mtx/default-value-transformer))) -(defn- extract-symbols [in] - (let [*symbols (atom [])] - (walk/postwalk - (fn [x] (when (symbol? x) (swap! *symbols conj x)) x) - in) - @*symbols)) - -(defn- mauto-let-form [arg->schema arg-symbol] - (when arg->schema - (when-let [schema (arg->schema arg-symbol)] - `[~arg-symbol (mc/decode ~schema ~arg-symbol defendpoint-transformer)]))) - -(defmacro auto-coerce - "Create a `let` form that tries to coerce the value bound to any symbol in `args` that are present in - `arg->schema` using [[defendpoint-transformer]]." - {:style/indent 1} - [args arg->schema & body] - (let [let-forms (->> args - extract-symbols - (mapcat #(mauto-let-form arg->schema %)) - (remove nil?))] - `(let [~@let-forms] ~@body))) - -(defn validate-param - "Validate a parameter against its respective malli schema, or throw an Exception." - [field-name value schema] - (when-not (mr/validate schema value) - (throw (ex-info (tru "Invalid m field: {0}" field-name) - {:status-code 400 - :errors {(keyword field-name) (umd/describe schema)} - :specific-errors {(keyword field-name) - (-> schema - (mr/explain value) - me/with-spell-checking - (me/humanize {:wrap mu/humanize-include-value}))}})))) - -(defn validate-params - "Generate a series of `validate-param` calls for each param and malli schema pair in PARAM->SCHEMA." - [param->schema] - (for [[param schema] param->schema] - `(validate-param '~param ~param ~schema))) - -(defn route-fn-name - "Generate a symbol suitable for use as the name of an API endpoint fn. Name is just `method` + `route` with slashes - replaced by underscores. - - (route-fn-name GET \"/:id\") ;-> GET_:id" - [method route] - ;; if we were passed a vector like [":id" :id #"[0-9+]"] only use first part - (let [route (if (vector? route) (first route) route)] - (-> (str (name method) route) - (^String .replace "/" "_") - symbol))) - (p.types/defprotocol+ EndpointResponse "Protocol for transformations that should be done to the value returned by a `defendpoint` form before it Compojure/Ring see it." diff --git a/src/metabase/api/common/openapi.clj b/src/metabase/api/common/openapi.clj index 60abdc8e3ba..437b34a5cd5 100644 --- a/src/metabase/api/common/openapi.clj +++ b/src/metabase/api/common/openapi.clj @@ -1,17 +1,16 @@ (ns metabase.api.common.openapi "Generate OpenAPI schema for defendpoint routes" (:require - [clojure.set :as set] [clojure.string :as str] + [malli.core :as mc] [malli.json-schema :as mjs] [medley.core :as m] [metabase.api.macros :as api.macros] [metabase.config :as config] [metabase.util :as u] [metabase.util.malli :as mu] - [metabase.util.malli.schema :as ms]) - (:import - (clojure.lang PersistentVector))) + [metabase.util.malli.registry :as mr] + [metabase.util.malli.schema :as ms])) (set! *warn-on-reflection* true) @@ -87,19 +86,6 @@ ;;; OpenAPI generation -(defn- first-if-vec [x] - (if (vector? x) (first x) x)) - -(defn- collect-defendpoint-1-routes [walk {:keys [prefix tag]} handler-var] - (let [tag (or (not-empty tag) (some-> (not-empty prefix) (subs 1))) - path (str prefix (-> handler-var meta :path first-if-vec))] - (cons {:path (path->openapi path) - :tag tag - :route handler-var} - (when-let [routes (-> handler-var meta :routes not-empty)] - (mapcat (partial walk {:prefix path :tag tag}) - routes))))) - (defn- collect-defendpoint-2-routes [{:keys [prefix tag]} ns-symb] (for [route (vals (-> ns-symb the-ns meta :api/endpoints))] {:path (path->openapi (str prefix (get-in route [:form :route :path]))) @@ -119,49 +105,13 @@ "Collect routes with schemas with their full paths." [root] (letfn [(walk [context handler] - (concat - (when-let [ns-symb (:api/defendpoint-2-namespace (meta handler))] - (collect-defendpoint-2-routes context ns-symb)) - (collect-defendpoint-1-routes walk context handler)))] + (when-let [ns-symb (:api/defendpoint-2-namespace (meta handler))] + (collect-defendpoint-2-routes context ns-symb)))] (->> (walk {:prefix ""} root) (filter #(or (-> % :route meta :schema) ; defendpoint 1 handler (:defendpoint-2-definition %)))))) -(defn- compojure-query-params - "This function is not trying to parse whole compojure syntax, just get the names of query parameters out" - [args] - (loop [args args - params []] - (if (empty? args) - (map keyword params) - (let [[x y] args] - (cond - ;; we don't care about rest binding - (= '& x) (recur (nnext args) params) - ;; and about coercion too - (= :<< x) (recur (nnext args) params) - (= :as x) (recur (nnext args) - (into params (when (map? y) - (let [qp (:query-params (set/map-invert y))] - (when (map? qp) - ;; {c :count :keys [a b}} ; => [count a b] - (flatten (vals qp))))))) - (symbol? x) (recur (next args) - (conj params x))))))) - -(defn- compojure-renames - "Find out everything that's renamed in Compojure routes" - [args] - (let [idx (inc (.indexOf ^PersistentVector args :as)) - req-bindings (get args idx)] - (when (and (pos? idx) - (map? req-bindings)) - (let [renames (->> (keys req-bindings) ; {{c :count} :query-params} ; => [{c :count}] - (filter map?) ; no stuff like {:keys [a]} - (apply merge))] - (update-keys renames keyword))))) - (defn- schema->params* [schema in-fn renames] (let [{:keys [properties required]} (mjs-collect-definitions schema) @@ -178,44 +128,14 @@ :schema (dissoc schema :optional :description)} (:description schema) (assoc :description (str (:description schema))))))) -(defn- schema->params - "https://spec.openapis.org/oas/latest.html#parameter-object" - [full-path args schema] - (let [in-path? (set (map (comp keyword second) (re-seq #"\{([^}]+)\}" full-path))) - in-query? (set (compojure-query-params args)) - in-fn (fn [k] - (cond - (in-path? k) :path - (in-query? k) :query)) - renames (some-> args compojure-renames)] - (schema->params* schema in-fn renames))) - -(mu/defn- defendpoint->path-item - "Generate OpenAPI desc for a single legacy `defendpoint` handler - - https://spec.openapis.org/oas/latest.html#path-item-object" - [tag - full-path :- string? - handler-var :- (ms/InstanceOfClass clojure.lang.Var)] - (let [{:keys [method] :as data} (meta handler-var) - params (schema->params full-path (:args data) (:schema data)) - non-body-param? (set (map :name params)) - body-params (when (not= method :get) - (remove #(non-body-param? (first %)) (rest (:schema data)))) - body-schema (when (seq body-params) - (fix-json-schema - (mjs-collect-definitions (into [:map] body-params)))) - ctype (if (:multipart (meta handler-var)) - "multipart/form-data" - "application/json")] - ;; summary is the string in the sidebar of Scalar - {method (cond-> {:summary (str (u/upper-case-en (name method)) " " full-path) - :description (some-> (or (:orig-doc data) - (:doc data)) - str) - :parameters params} - tag (assoc :tags [tag]) - body-schema (assoc :requestBody {:content {ctype {:schema body-schema}}}))})) +(mu/defn- multipart-schema [form :- ::api.macros/parsed-args] + (when-let [request-schema (get-in form [:params :request :schema])] + (let [schema (-> request-schema mr/resolve-schema mc/schema)] + (when (= (mc/type schema) :map) + (some (fn [[k _opts schema]] + (when (= k :multipart-params) + schema)) + (mc/children schema)))))) (mu/defn- defendpoint-2->path-item "Generate OpenAPI desc for `defendpoint` 2.0 ([[metabase.api.macros/defendpoint]]) handler. @@ -233,13 +153,14 @@ (for [param route-params] (assoc param :in :path)) query-params) - body-schema (some-> (get-in form [:params :body :schema]) - mjs-collect-definitions - fix-json-schema) - ;; multipart is not yet implemented for defendpoint 2 but this is a placeholder until it is I guess ctype (if (get-in form [:metadata :multipart]) "multipart/form-data" - "application/json")] + "application/json") + body-schema (some-> (if (= ctype "multipart/form-data") + (multipart-schema form) + (get-in form [:params :body :schema])) + mjs-collect-definitions + fix-json-schema)] ;; summary is the string in the sidebar of Scalar {method (cond-> {:summary (str (u/upper-case-en (name method)) " " full-path) :description (some-> (:docstr form) str) @@ -257,11 +178,9 @@ (let [paths (reduce (fn [acc [k v]] (merge-with into acc {k v})) {} - (for [{:keys [path tag route defendpoint-2-definition]} (collect-routes root)] + (for [{:keys [path tag defendpoint-2-definition]} (collect-routes root)] (try - [path (if defendpoint-2-definition - (defendpoint-2->path-item tag path defendpoint-2-definition) - (defendpoint->path-item tag path route))] + [path (defendpoint-2->path-item tag path defendpoint-2-definition)] (catch Exception e (throw (ex-info (str "Exception at " path) {} e))))))] {:openapi "3.1.0" diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index d41153d7f0d..281a4d1c5b0 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -1422,9 +1422,8 @@ `options` are passed to [[user-facing-value]]. - This is currently used by `GET /api/setting` ([[metabase.api.setting/GET_]]; admin-only; powers the Admin Settings - page) so all admin-visible Settings should be included. We *do not* want to return env var values, since admins - are not allowed to modify them. + This is currently used by `GET /api/setting` (admin-only; powers the Admin Settings page) so all admin-visible + Settings should be included. We *do not* want to return env var values, since admins are not allowed to modify them. For settings managers who are not admins, only the subset of settings with the :settings-manager visibility level are returned." @@ -1470,7 +1469,7 @@ Settings marked `:sensitive?` (e.g. passwords) are excluded. - This is currently used by `GET /api/session/properties` ([[metabase.api.session/GET_properties]]) and + This is currently used by `GET /api/session/properties` and in [[metabase.server.routes.index/load-entrypoint-template]]. These are used as read-only sources of Settings for the frontend client. For that reason, these Settings *should* include values that come back from environment variables, *unless* they are marked `:sensitive?`." diff --git a/src/metabase/query_processor/streaming.clj b/src/metabase/query_processor/streaming.clj index ea37b55c5aa..fa462c4387f 100644 --- a/src/metabase/query_processor/streaming.clj +++ b/src/metabase/query_processor/streaming.clj @@ -200,7 +200,7 @@ Typical example: - (api/defendpoint-schema GET \"/whatever\" [] + (api.macros/defendpoint :get \"/whatever\" [] (qp.streaming/streaming-response [rff :json] (qp/process-query (qp/userland-query-with-default-constraints query) rff))) diff --git a/test/metabase/api/common/internal_test.clj b/test/metabase/api/common/internal_test.clj index 0811e9940b5..84c053d34f2 100644 --- a/test/metabase/api/common/internal_test.clj +++ b/test/metabase/api/common/internal_test.clj @@ -1,338 +1,10 @@ (ns metabase.api.common.internal-test (:require - [clj-http.client :as http] - [clojure.string :as str] [clojure.test :refer :all] - [compojure.core :refer [POST]] - [malli.util :as mut] - [metabase.api.common :as api] - [metabase.api.common.internal :as internal] - [metabase.config :as config] - [metabase.logger :as logger] - [metabase.server.middleware.exceptions :as mw.exceptions] - [metabase.test :as mt] - [metabase.util :as u] - [metabase.util.json :as json] - [metabase.util.malli.registry :as mr] - [metabase.util.malli.schema :as ms] - [ring.adapter.jetty :as jetty] - [ring.middleware.params :refer [wrap-params]]) - (:import - (org.eclipse.jetty.server Server))) + [metabase.api.common.internal :as internal])) (set! *warn-on-reflection* true) -(mr/def ::card-type - [:enum :question :model :metric]) - -(deftest ^:parallel dox-for-schema-test - (testing "We should resolve schemas in the Malli registry when generating documentation (#46799)\n" - (are [schema] (= "nullable enum of :question, :model, :metric" - (#'internal/dox-for-schema schema nil)) - [:maybe [:enum :question :model :metric]] - [:maybe ::card-type] - [:maybe [:schema ::card-type]] - [:maybe [:schema [:schema ::card-type]]]))) - -(def ^:private TestAddress - [:map - {:title "Address"} - [:id :string] - [:tags [:set :keyword]] - [:address - [:map - [:street :string] - [:city :string] - [:zip :int] - [:lonlat [:tuple :double :double]]]]]) - -(def ^:private ClosedTestAddress - (mut/closed-schema TestAddress)) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/post/any" [:as {body :body :as _request}] - {:status 200 :body body}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/post/id-int" - [:as {{:keys [id] :as body} :body :as _request}] - {id :int} - {:status 200 :body body}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/post/test-address" - [:as {address :body :as _request}] - {address TestAddress} - {:status 200 :body address}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/post/closed-test-address" - [:as {address :body :as _request}] - {address ClosedTestAddress} - {:status 200 :body address}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/test-localized-error" - [:as {address :body :as _request}] - {address ms/NonBlankString} - {:status 200 :body address}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/auto-coerce-pos-square/:x" - [x] - ;; if not for this annotation, x would continue to be a string: - {x ms/PositiveInt} - (* x x)) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/auto-coerce-string-repeater" - [:as {body :body :as _req}] - {body [:map - [:str :string] - [:n pos-int?] - [:join [:maybe :string]]]} - (let [{:keys [str n join]} body] - (str/join (or join "") (repeat n str)))) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/auto-coerce-destructure" - [:as {{:keys [set-of-kw]} :body :as _req}] - {set-of-kw [:set :keyword]} - {:pr-strd (pr-str set-of-kw) - :api-returns set-of-kw}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/closed-map-spellcheck" - [:as {body :body :as _req}] - {body [:map {:closed true} - [:user-state :string] - [:po-box [:string {:min 1 :max 10}]] - [:archipelago :string]]} - (let [{:keys [state po-box archipelago]} body] - (str/join " | " [state po-box archipelago]))) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint GET "/with-query-params/" - [:as {params :params}] - {params [:and - ;; decodes the keyword _keys_ - [:map-of :keyword any?] - ;; ^ if you don't care about the shape you can stop there - [:map - [:bool-key {:optional true} :boolean] - [:string-key {:optional true} :string] - [:enum-key [:enum :a :b :c]] - [:kw-key :keyword]]]} - (pr-str params)) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/accept-thing/:a" [a] {a [:re #"a.*"]} "hit route for a.") -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/accept-thing/:b" [b] {b [:re #"b.*"]} "hit route for b.") -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/accept-thing/:c" [c] {c [:re #"c.*"]} "hit route for c.") -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/accept-thing/:d" [d] {d [:re #"d.*"]} "hit route for d.") -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/accept-thing/:e" [e] {e [:re #"e.*"]} "hit route for e.") - -(api/define-routes) - -(defn- json-mw [handler] - (fn [req] - (-> req - (update :body #(-> % slurp json/decode+kw)) - handler - (update :body json/encode)))) - -(defn exception-mw [handler] - (fn [req] (try - (handler req) - (catch Exception e (mw.exceptions/api-exception-response e))))) - -(defn- do-with-jetty-server [f] - (let [^Server server (jetty/run-jetty (json-mw - (exception-mw - (wrap-params #'routes))) {:port 0 :join? false}) - port (.. server getURI getPort) - get (fn [route] - (http/get (str "http://localhost:" port route) - {:throw-exceptions false - :accept :json - :as :json - :coerce :always})) - post (fn [route body] - (http/post (str "http://localhost:" port route) - {:throw-exceptions false - :accept :json - :as :json - :coerce :always - :body (json/encode body)}))] - (try - (testing (format "With temp Jetty server on port %d" port) - (f {:get get, :post! post})) - (finally - (.stop server))))) - -(defmacro ^:private with-jetty-server [bindings & body] - `(do-with-jetty-server (fn ~bindings ~@body))) - -(deftest defendpoint-query-params-test - (with-jetty-server [{:keys [get]}] - (is (= "{:bool-key true, :string-key \"abc\", :enum-key :a, :kw-key :abc}" - (:body (get "/with-query-params/?bool-key=true&string-key=abc&enum-key=a&kw-key=abc")))))) - -(deftest defendpoint-validation-test - (with-jetty-server [{:keys [post!]}] - (testing "validation" - (is (= {:a 1 :b 2} (:body (post! "/post/any" {:a 1 :b 2})))) - - (is (= {:id 1} (:body (post! "/post/id-int" {:id 1})))) - - ;; this is coercable now! - (is (= {:id "1"} (:body (post! "/post/id-int" {:id "1"})))) - - (mt/with-log-level [metabase.api.common :warn] - (is (= {:id "myid" - :tags ["abc"] - :address {:street "abc" :city "sdasd" :zip 2999 :lonlat [0.0 0.0]}} - (:body (post! "/post/test-address" - {:id "myid" - :tags ["abc"] - :address {:street "abc" - :city "sdasd" - :zip 2999 - :lonlat [0.0 0.0]}})))) - (is (some (fn [{message :msg, :as entry}] - (when (str/includes? (str message) - (str "Unexpected parameters at [:post \"/post/test-address\"]: [:tags :id]\n" - "Please add them to the schema or remove them from the API client")) - entry)) - (logger/messages)))) - - (is (= {:errors - {:address - "map (titled: ‘Address’) where {:id -> , :tags -> , :address -> , :city -> , :zip -> , :lonlat -> }>}"}, - :specific-errors - {:address - {:id ["missing required key, received: nil"], - :tags ["missing required key, received: nil"], - :address ["missing required key, received: nil"]}}} - (:body (post! "/post/test-address" {:x "1"})))) - - (is (= {:errors - {:address - "map (titled: ‘Address’) where {:id -> , :tags -> , :address -> , :city -> , :zip -> , :lonlat -> }>}"}, - :specific-errors - {:address - {:id ["should be a string, received: 1288"], - :tags ["invalid type, received: \"a,b,c\""], - :address {:street ["missing required key, received: nil"]}}}} - (:body (post! "/post/test-address" {:id 1288 - :tags "a,b,c" - :address {:streeqt "abc" - :city "sdasd" - :zip "12342" - :lonlat [0.0 0.0]}})))) - - (is (= {:errors - {:address - "map (titled: ‘Address’) where {:id -> , :tags -> , :address -> , :city -> , :zip -> , :lonlat -> } with no other keys>} with no other keys"}, - :specific-errors - {:address - {:address ["missing required key, received: nil"], - :a ["disallowed key, received: 1"], - :b ["disallowed key, received: 2"]}}} - (:body (post! "/post/closed-test-address" {:id "1" :tags [] :a 1 :b 2}))))))) - -(deftest defendpoint-validation-localiazed-errors-test - (with-jetty-server [{:keys [post!]}] - (testing "validation" - (testing "malli schema message are localized" - (mt/with-mock-i18n-bundles! {"es" {:messages - {"value must be a non-blank string." - "el valor debe ser una cadena que no esté en blanco."}}} - (mt/with-temporary-setting-values [site-locale "es"] - (is (= {:errors {:address "el valor debe ser una cadena que no esté en blanco."}, - ;; TODO remove .'s from ms schemas - ;; TODO translate received (?) - :specific-errors - {:address ["should be a string, received: {:address \"\"}" "non-blank string, received: {:address \"\"}"]}} - (:body (post! "/test-localized-error" {:address ""})))))))))) - -(deftest defendpoint-auto-coercion-test - (with-jetty-server [{:keys [post!]}] - (testing "auto-coercion" - - (is (= 16 (:body (post! "/auto-coerce-pos-square/4" {})))) - - ;; Does not match route, since we expected a regex matching an int: - (is (= nil (:body (post! "/auto-coerce-pos-square/-4" {})))) - - (is (= nil - ;; Does not match route, since we expected a regex matching an int: - (:body (post! "/auto-coerce-pos-square/not-an-int" {})))) - - (is (= "chirp! chirp!" - (:body (post! "/auto-coerce-string-repeater" {:n 2 :str "chirp!" :join " "})))) - - (is (= {:errors {:body "map where {:str -> , :n -> , :join -> }"} - :specific-errors {:body {:n ["should be a positive int, received: -3"]}}} - (:body (post! "/auto-coerce-string-repeater" - {:n -3 :str "chirp!" :join " "})))) - - (is (= {:errors {:body "map where {:str -> , :n -> , :join -> }"} - :specific-errors {:body {:str ["should be a string, received: 123"]}}} - (:body (post! "/auto-coerce-string-repeater" - {:n 3 :str 123 :join " "})))) - - (is (= {:errors {:body "map where {:str -> , :n -> , :join -> }"} - :specific-errors {:body {:str ["missing required key, received: nil"], - :n ["missing required key, received: nil"], - :join ["missing required key, received: nil"]}}} - (:body (post! "/auto-coerce-string-repeater" {})))) - - (is (= {;; in the defendpoint body, it is coerced properly: - :pr-strd "#{:c :d/e :b :a}", - ;; but it gets turned back into json in the request, of course. - :api-returns ["c" "d/e" "b" "a"]} - (:body (post! "/auto-coerce-destructure" {:set-of-kw ["a" "b" "c" "d/e"]})))) - - (is (= {:errors {:set-of-kw "set of keyword"} - :specific-errors {:set-of-kw ["invalid type, received: \"This wont work\""]}} - (:body (post! "/auto-coerce-destructure" {:set-of-kw "This wont work"})))) - - (is (= {:errors {:set-of-kw "set of keyword"} - :specific-errors {:set-of-kw ["invalid type, received: nil"]}} - (:body (post! "/auto-coerce-destructure" {})))) - - (is (= {:errors - {:body "map where {:user-state -> , :po-box -> , :archipelago -> } with no other keys"}, - :specific-errors {:body {:ser-state ["should be spelled :user-state, received: \"my state\""], - :o-box ["should be spelled :po-box, received: \"my po-box\""], - :rchipelago ["should be spelled :archipelago, received: \"my archipelago\""]}}} - (:body (post! "/closed-map-spellcheck" {:ser-state "my state" - :o-box "my po-box" - :rchipelago "my archipelago"}))))))) - -(deftest defendpoint-arbitrary-routes-test - (with-jetty-server [{:keys [post!]}] - (testing "routes need to not be arbitrarily chosen" - (is (= "hit route for a." (:body (post! "/accept-thing/a123" {})))) - (is (= "hit route for b." (:body (post! "/accept-thing/b123" {})))) - (is (= "hit route for c." (:body (post! "/accept-thing/c123" {})))) - (is (= "hit route for d." (:body (post! "/accept-thing/d123" {})))) - (is (= "hit route for e." (:body (post! "/accept-thing/e123" {})))) - (is (= nil (:body (post! "/accept-thing/f123" {}))))))) - -(deftest ^:parallel route-fn-name-test - (are [method route expected] (= expected - (internal/route-fn-name method route)) - 'GET "/" 'GET_ - 'GET "/:id/fks" 'GET_:id_fks - ;; check that internal/route-fn-name can handle routes with regex conditions - 'GET ["/:id" :id #"[0-9]+"] 'GET_:id)) - (deftest ^:parallel route-arg-keywords-test (are [route expected] (= expected (internal/route-arg-keywords route)) @@ -341,39 +13,3 @@ "/:id/card" [:id] "/:id/etc/:org" [:id :org] "/:card-id" [:card-id])) - -(deftest ^:parallel add-route-param-schema-test - (are [route expected] (= expected - (let [result (internal/add-route-param-schema - {'id ms/PositiveInt - 'card-id ms/PositiveInt - 'crazy-id ms/PositiveInt - 'uuid ms/UUIDString} - route)] - (cond (string? result) result - (coll? result) (mapv - (fn [x] (if (= (type x) java.util.regex.Pattern) - (str "#" x) - x)) - result)))) - "/" "/" - "/:id" ["/:id" :id "#[0-9]+"] - "/:id/card" ["/:id/card" :id "#[0-9]+"] - "/:card-id" ["/:card-id" :card-id "#[0-9]+"] - "/:fish" "/:fish" - "/:id/tables/:card-id" ["/:id/tables/:card-id" :id "#[0-9]+" :card-id "#[0-9]+"] - ;; don't try to typify route that's already typified - ["/:id/:crazy-id" :crazy-id "#[0-9]+"] ["/:id/:crazy-id" :crazy-id "#[0-9]+"] - ;; Check :uuid args - "/:uuid/toucans" ["/:uuid/toucans" :uuid (str \# u/uuid-regex)] - "/:id/:card-id" ["/:id/:card-id" :id "#[0-9]+" :card-id "#[0-9]+"] - "/:unlisted/:card-id" ["/:unlisted/:card-id" :card-id "#[0-9]+"])) - -(deftest ^:parallel enterprise-endpoint-name-test - (when config/ee-available? - (testing "Make sure the route name for enterprise API endpoints is somewhat correct" - (require 'metabase-enterprise.advanced-permissions.api.application) - (is (= "GET /api/ee/advanced-permissions/application/graph" - (#'internal/endpoint-name (the-ns 'metabase-enterprise.advanced-permissions.api.application) - 'GET - "/graph")))))) diff --git a/test/metabase/api/common/openapi_test.clj b/test/metabase/api/common/openapi_test.clj index dc64e158bbd..f5f4d61c333 100644 --- a/test/metabase/api/common/openapi_test.clj +++ b/test/metabase/api/common/openapi_test.clj @@ -2,7 +2,6 @@ (:require [clojure.string :as str] [clojure.test :refer :all] - [compojure.core :refer [GET POST]] [malli.json-schema :as mjs] [metabase.api.common :as api] [metabase.api.common.openapi :as openapi] @@ -51,87 +50,15 @@ (mjs/transform [:sequential [:map [:params {:optional true} [:maybe [:sequential :string]]]]])))))) -;;; inner helpers - -(deftest ^:parallel parse-compojure-test - (are [res args] (= res (#'openapi/compojure-query-params args)) - [:id :value] '[id value] - [:id :value] '[id :as {{:strs [value]} :query-params}] - [:id] '[id :as {raw-params :params}] - [:count] '[:as {{c :count} :query-params}]) - (are [res args] (= res (#'openapi/compojure-renames args)) - {:c :count} '[:as {{c :count} :query-params}])) - ;;; definitions -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint GET "/:id" - "docstring" - [id] - {id ms/PositiveInt} - {:id (str id)}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/:id" - "docstring" - [id value] - {id ms/PositiveInt - value ms/NonBlankString} - {:id (str id) - :value value}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint ^:multipart POST "/:id/upload" - "docstring" - [id :as {{:strs [file]} :multipart-params}] - {id ms/PositiveInt - file (mu/with ms/File - {:description "File to upload"})} - {:id id :data file}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint POST "/export" - "docstring" - [:as {{:strs [collection settings data-model]} :query-params}] - {collection [:maybe (ms/QueryVectorOf ms/PositiveInt)] - settings [:maybe ms/BooleanValue] - data-model ms/MaybeBooleanValue} - {:collections collection :settings settings}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint GET "/rename" - "this one renames query parameter trying to trick us" - [:as {{c :count} :query-params}] - {c ms/PositiveInt} - {:count c}) - -#_{:clj-kondo/ignore [:deprecated-var]} -(api/defendpoint PUT "/complex/:id" - "More complex body schema" - [id :as {data :body}] - {id ms/PositiveInt - data [:map - [:name {:optional true} [:maybe ms/NonBlankString]] - [:dashcards (ms/maps-with-unique-key - [:sequential [:map - [:id int?] - [:params {:optional true} [:maybe [:sequential [:map - [:param_id ms/NonBlankString] - [:target :any]]]]]]] - :id)]]} - {:id id :data data}) - -;;; -;;; defendpoint 2.0 versions of the routes -;;; - -(api.macros/defendpoint :get "/d2/:id" +(api.macros/defendpoint :get "/:id" "docstring" [{:keys [id]} :- [:map [:id ms/PositiveInt]]] {:id (str id)}) -(api.macros/defendpoint :post "/d2/:id" +(api.macros/defendpoint :post "/:id" "docstring" [{:keys [id]} :- [:map [:id ms/PositiveInt]] @@ -140,7 +67,7 @@ {:id (str id) :value value}) -(api.macros/defendpoint :post "/d2/export" +(api.macros/defendpoint :post "/export" "docstring" [_route-params {:keys [collection settings]} :- [:map @@ -149,14 +76,14 @@ [:data-model ms/MaybeBooleanValue]]] {:collections collection :settings settings}) -(api.macros/defendpoint :get "/d2/rename" +(api.macros/defendpoint :get "/rename" "this one renames query parameter trying to trick us (actually doesn't really trick us much anymore with defendpoint 2)" [_route-params {c :count} :- [:map [:count {:optional true} ms/PositiveInt]]] {:count c}) -(api.macros/defendpoint :put "/d2/complex/:id" +(api.macros/defendpoint :put "/complex/:id" "More complex body schema" [{:keys [id]} :- [:map [:id ms/PositiveInt]] @@ -173,6 +100,19 @@ :id)]]]]] {:id id :data data}) +(api.macros/defendpoint :post "/:id/upload" + "docstring" + {:multipart true} + [{:keys [id]} :- [:map + [:id ms/PositiveInt]] + _query-params + _body + {{:strs [file]} :multipart-params, :as _request} :- [:map + [:multipart-params + [:map + [:file (mu/with ms/File {:description "File to upload"})]]]]] + {:id id :data file}) + (defn- find-route [method route] (api.macros/find-route 'metabase.api.common.openapi-test method route)) @@ -194,11 +134,6 @@ (deftest ^:parallel collect-routes-test (testing "can collect routes in simple case" (is (=? [{:path "/complex/{id}"} - {:path "/d2/complex/{id}"} - {:path "/d2/export"} - {:path "/d2/rename"} - {:path "/d2/{id}"} - {:path "/d2/{id}"} {:path "/export"} {:path "/rename"} {:path "/{id}"} @@ -218,36 +153,28 @@ (some? (->> route meta :routes (some meta))))))))) (deftest ^:parallel defendpoint->openapi-test - (doseq [defendpoint-version [1 2]] - (testing (format "Defendpoint version = %d" defendpoint-version) - (is (=? {:get - {:parameters [{:in :path - :name :id - :required true - :description some? - :schema {:type "integer" - :minimum 1}}]}} - (case defendpoint-version - 1 (#'openapi/defendpoint->path-item nil "/{id}" #'GET_:id) - 2 (#'openapi/defendpoint-2->path-item nil "/{id}" (:form (find-route :get "/d2/:id"))))))))) + (is (=? {:get + {:parameters [{:in :path + :name :id + :required true + :description some? + :schema {:type "integer" + :minimum 1}}]}} + (#'openapi/defendpoint-2->path-item nil "/{id}" (:form (find-route :get "/:id")))))) (deftest ^:parallel defendpoint->openapi-test-2 - (doseq [defendpoint-version [1 2]] - (testing (format "Defendpoint version = %d" defendpoint-version) - (is (=? {:post - {:parameters [{:in :path - :name :id - :required true - :description some? - :schema {:type "integer" - :minimum 1}} - {:in :query - :name :value - :required true - :schema {:$ref "#/components/schemas/metabase.lib.schema.common~1non-blank-string"}}]}} - (case defendpoint-version - 1 (#'openapi/defendpoint->path-item nil "/{id}" #'POST_:id) - 2 (#'openapi/defendpoint-2->path-item nil "/{id}" (:form (find-route :post "/d2/:id"))))))))) + (is (=? {:post + {:parameters [{:in :path + :name :id + :required true + :description some? + :schema {:type "integer" + :minimum 1}} + {:in :query + :name :value + :required true + :schema {:$ref "#/components/schemas/metabase.lib.schema.common~1non-blank-string"}}]}} + (#'openapi/defendpoint-2->path-item nil "/{id}" (:form (find-route :post "/:id")))))) (deftest ^:parallel multipart-defendpoint->openapi-test ;; multipart defendpoint 2 isn't implemented yet, so this only tests defendpoint 1 (for now) @@ -263,85 +190,71 @@ {:type "object" :properties {:file {}} :required [:file]}}}}}} - (#'openapi/defendpoint->path-item nil "/{id}" #'POST_:id_upload)))) + (#'openapi/defendpoint-2->path-item nil "/{id}" (:form (find-route :post "/:id/upload")))))) (deftest ^:parallel defendpoint->openapi-test-3 - (doseq [defendpoint-version [1 2]] - (testing (format "Defendpoint version = %d" defendpoint-version) - (is (=? {:post - {:parameters [{:in :query - :name :collection - :required false - :schema {:type "array" - :items - {:type "integer" - :minimum 1}}} - {:in :query - :name :settings - :required false - :schema {:type "boolean"}} - {:in :query - :name :data-model - :required false - :schema {:type "boolean"}}]}} - (case defendpoint-version - 1 (#'openapi/defendpoint->path-item nil "/export" #'POST_export) - 2 (#'openapi/defendpoint-2->path-item nil "/export" (:form (find-route :post "/d2/export"))))))))) + (is (=? {:post + {:parameters [{:in :query + :name :collection + :required false + :schema {:type "array" + :items + {:type "integer" + :minimum 1}}} + {:in :query + :name :settings + :required false + :schema {:type "boolean"}} + {:in :query + :name :data-model + :required false + :schema {:type "boolean"}}]}} + (#'openapi/defendpoint-2->path-item nil "/export" (:form (find-route :post "/export")))))) (deftest ^:parallel defendpoint->openapi-renamed-parameters-test - (doseq [defendpoint-version [1 2]] - (testing (format "Defendpoint version = %d" defendpoint-version) - (is (=? {:get {:parameters [{:in :query - :name :count - :required false - :schema {:type "integer" :minimum 1}}]}} - (case defendpoint-version - 1 (#'openapi/defendpoint->path-item nil "/rename" #'GET_rename) - 2 (#'openapi/defendpoint-2->path-item nil "/rename" (:form (find-route :get "/d2/rename"))))))))) + (is (=? {:get {:parameters [{:in :query + :name :count + :required false + :schema {:type "integer" :minimum 1}}]}} + (#'openapi/defendpoint-2->path-item nil "/rename" (:form (find-route :get "/rename")))))) (deftest ^:parallel defendpoint->openapi-complex-test - (doseq [defendpoint-version [1 2]] - (testing (format "Defendpoint version = %d" defendpoint-version) - (is (=? {:put - {:summary "PUT /complex/{id}" - :parameters - [{:in :path - :name :id - :required true - :schema {:type "integer", :minimum 1} - :description string?}] - :requestBody - {:content - {"application/json" - {:schema - {:type "object" - :required [:data] - :properties - {:data - {:type "object" - :required [:dashcards] - :properties - {:name {:description string? - :$ref "#/components/schemas/metabase.lib.schema.common~1non-blank-string"} - :dashcards {:type "array" - :description string? - :items {:type "object" - :required [:id] - :properties {:id {:type "integer"} - :params {:type "array" - :items {:type "object" - :required [:param_id :target] - :properties {:param_id {} - :target {}}}}}}}}}}}}}}}} - (case defendpoint-version - 1 (#'openapi/defendpoint->path-item nil "/complex/{id}" #'PUT_complex_:id) - 2 (#'openapi/defendpoint-2->path-item nil "/complex/{id}" (:form (find-route :put "/d2/complex/:id"))))))))) + (is (=? {:put + {:summary "PUT /complex/{id}" + :parameters + [{:in :path + :name :id + :required true + :schema {:type "integer", :minimum 1} + :description string?}] + :requestBody + {:content + {"application/json" + {:schema + {:type "object" + :required [:data] + :properties + {:data + {:type "object" + :required [:dashcards] + :properties + {:name {:description string? + :$ref "#/components/schemas/metabase.lib.schema.common~1non-blank-string"} + :dashcards {:type "array" + :description string? + :items {:type "object" + :required [:id] + :properties {:id {:type "integer"} + :params {:type "array" + :items {:type "object" + :required [:param_id :target] + :properties {:param_id {} + :target {}}}}}}}}}}}}}}}} + (#'openapi/defendpoint-2->path-item nil "/complex/{id}" (:form (find-route :put "/complex/:id")))))) (deftest ^:parallel openapi-object-test - (is (=? {:paths {"/{id}" {:get {} - :post {}} - "/{id}/upload" {:post {}} - "/d2/{id}" {:get {} + (is (=? {:paths {"/{id}/upload" {:post {}} + "/{id}" {:get {} :post {}}} :components {:schemas {"metabase.lib.schema.common/non-blank-string" {:type "string", :minLength 1}}}} (openapi/openapi-object #'routes))))