Kimirock/metabase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove defendpoint 1 (#52785)

Browse Source
This commit is contained in:
Cam Saul
2025-01-28 13:32:48 -08:00
committed by GitHub
parent 8a2b0538f1
commit 54d9d77892
13 changed files with 129 additions and 1624 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.clj-kondo/config.edn
View File

@@ -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

32
.clj-kondo/src/hooks/metabase/api/common.clj
View File

@@ -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)))

35
.clj-kondo/test/hooks/metabase/api/common_test.clj
View File

@@ -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))))

4
.dir-locals.el
View File

@@ -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))

6
.lsp/config.edn
View File

@@ -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}}

511
dev/src/dev/rewrite_defendpoint_1.clj
View File

@@ -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 <forms> 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*)))))

155
src/metabase/api/common.clj
View File

@@ -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)

234
src/metabase/api/common/internal.clj
View File

@@ -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."

121
src/metabase/api/common/openapi.clj
View File

@@ -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)))]
(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"

7
src/metabase/models/setting.clj
View File

@@ -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?`."

2
src/metabase/query_processor/streaming.clj
View File

@@ -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)))

366
test/metabase/api/common/internal_test.clj
View File

@@ -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 -> <string>, :tags -> <set of keyword>, :address -> <map where {:street -> <string>, :city -> <string>, :zip -> <integer>, :lonlat -> <vector with exactly 2 items of type: double, double>}>}"},
: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 -> <string>, :tags -> <set of keyword>, :address -> <map where {:street -> <string>, :city -> <string>, :zip -> <integer>, :lonlat -> <vector with exactly 2 items of type: double, double>}>}"},
: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 -> <string>, :tags -> <set of keyword>, :address -> <map where {:street -> <string>, :city -> <string>, :zip -> <integer>, :lonlat -> <vector with exactly 2 items of type: double, double>} 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 -> <string>, :n -> <integer greater than 0>, :join -> <nullable string>}"}
: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 -> <string>, :n -> <integer greater than 0>, :join -> <nullable string>}"}
: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 -> <string>, :n -> <integer greater than 0>, :join -> <nullable string>}"}
: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 -> <string>, :po-box -> <string with length between 1 and 10 inclusive>, :archipelago -> <string>} 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"))))))

139
test/metabase/api/common/openapi_test.clj
View File

@@ -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,8 +153,6 @@
(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
@@ -227,13 +160,9 @@
: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")))))))))
(#'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
@@ -245,9 +174,7 @@
: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")))))))))
(#'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,11 +190,9 @@
{: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
@@ -284,24 +209,16 @@
: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")))))))))
(#'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")))))))))
(#'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
@@ -333,15 +250,11 @@
: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")))))))))
(#'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))))