Skip to content

Commit 990b075

Browse files
frenchy64frenchy64
committed
fix double expansion/eval in meta, add assertions
1 parent 8bd449e commit 990b075

4 files changed

Lines changed: 2256 additions & 76 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
See also: [compojure-api 1.1.x changelog](./CHANGELOG-1.1.x.md)
22

3+
## NEXT
4+
* Throw an error on malformed `:{body,query,headers}`, in particular is anything other than 2 elements was provided
5+
* Disable check with `-Dcompojure.api.meta.allow-bad-{body,query,headers}=true`
6+
* 50% reduction in the number of times `:{return,body,query,responses,headers,coercion,{body,form,header,query,path}-params}` schemas/arguments are evaluated/expanded
7+
* saves 1 evaluation per schema for static contexts
8+
* saves 1 evaluation per schema, per request, for dynamic contexts
9+
* Fix: Merge `:{form,multipart}-params` `:info :public :parameters :formData` field at runtime
10+
311
## 2.0.0-alpha32 (2024-03-20)
412

513
* Fix empty spec response coercion. [#413](https://github.com/metosin/compojure-api/issues/413)

‎project.clj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
[lein-ring "0.12.5"]
3434
[funcool/codeina "0.5.0"]]
3535
:dependencies [[org.clojure/clojure "1.10.1"]
36+
[org.clojure/core.unify "0.6.0"]
3637
[org.clojure/core.async "0.6.532"]
3738
[javax.servlet/javax.servlet-api "4.0.1"]
3839
[peridot "0.5.2"]

‎src/compojure/api/meta.clj‎

Lines changed: 102 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
(ns compojure.api.meta
22
(:require [clojure.edn :as edn]
3+
[clojure.walk :as walk]
34
[compojure.api.common :refer [extract-parameters]]
45
[compojure.api.middleware :as mw]
56
[compojure.api.routes :as routes]
@@ -295,10 +296,12 @@
295296
" (ok {:name \"Kirsi\"))")))
296297

297298
(defmethod restructure-param :return [_ schema acc]
298-
(let [response (convert-return schema)]
299+
(let [response (convert-return schema)
300+
g (gensym 'response)]
299301
(-> acc
300-
(update-in [:info :public :responses] (fnil conj []) response)
301-
(update-in [:responses] (fnil conj []) response))))
302+
(update-in [:outer-lets] into [g response])
303+
(update-in [:info :public :responses] (fnil conj []) g)
304+
(update-in [:responses] (fnil conj []) g))))
302305

303306
;;
304307
;; responses
@@ -321,9 +324,11 @@
321324
" (bad-request \"kosh\"))")))
322325

323326
(defmethod restructure-param :responses [_ responses acc]
324-
(-> acc
325-
(update-in [:info :public :responses] (fnil conj []) responses)
326-
(update-in [:responses] (fnil conj []) responses)))
327+
(let [g (gensym 'responses)]
328+
(-> acc
329+
(update :outer-lets into [g responses])
330+
(update-in [:info :public :responses] (fnil conj []) g)
331+
(update-in [:responses] (fnil conj []) g))))
327332

328333
;;
329334
;; body
@@ -338,10 +343,16 @@
338343
" :body [body User]"
339344
" (ok body))")))
340345

341-
(defmethod restructure-param :body [_ [value schema] acc]
342-
(-> acc
343-
(update-in [:lets] into [value (src-coerce! schema :body-params :body false)])
344-
(assoc-in [:info :public :parameters :body] schema)))
346+
(defmethod restructure-param :body [_ [value schema :as bv] acc]
347+
(when-not (= "true" (System/getProperty "compojure.api.meta.allow-bad-body"))
348+
(assert (= 2 (count bv))
349+
(str ":body should be [sym schema], provided: " bv
350+
"\nDisable this check with -Dcompojure.api.meta.allow-bad-body=true")))
351+
(let [g (gensym 'body-schema)]
352+
(-> acc
353+
(update :outer-lets into [g schema])
354+
(update-in [:lets] into [value (src-coerce! g :body-params :body false)])
355+
(assoc-in [:info :public :parameters :body] g))))
345356

346357
;;
347358
;; query
@@ -356,10 +367,16 @@
356367
" :query [params {:q s/Str, :max s/Int}]"
357368
" (ok params))")))
358369

359-
(defmethod restructure-param :query [_ [value schema] acc]
360-
(-> acc
361-
(update-in [:lets] into [value (src-coerce! schema :query-params :string)])
362-
(assoc-in [:info :public :parameters :query] schema)))
370+
(defmethod restructure-param :query [_ [value schema :as bv] acc]
371+
(when-not (= "true" (System/getProperty "compojure.api.meta.allow-bad-query"))
372+
(assert (= 2 (count bv))
373+
(str ":query should be [sym schema], provided: " bv
374+
"\nDisable this check with -Dcompojure.api.meta.allow-bad-query=true")))
375+
(let [g (gensym 'query-schema)]
376+
(-> acc
377+
(update :outer-lets into [g schema])
378+
(update-in [:lets] into [value (src-coerce! g :query-params :string)])
379+
(assoc-in [:info :public :parameters :query] g))))
363380

364381
;;
365382
;; headers
@@ -374,10 +391,16 @@
374391
" :headers [headers HeaderSchema]"
375392
" (ok headers))")))
376393

377-
(defmethod restructure-param :headers [_ [value schema] acc]
378-
(-> acc
379-
(update-in [:lets] into [value (src-coerce! schema :headers :string)])
380-
(assoc-in [:info :public :parameters :header] schema)))
394+
(defmethod restructure-param :headers [_ [value schema :as bv] acc]
395+
(when-not (= "true" (System/getProperty "compojure.api.meta.allow-bad-headers"))
396+
(assert (= 2 (count bv))
397+
(str ":headers should be [sym schema], provided: " bv
398+
"\nDisable this check with -Dcompojure.api.meta.allow-bad-headers=true")))
399+
(let [g (gensym 'headers-schema)]
400+
(-> acc
401+
(update :outer-lets into [g schema])
402+
(update-in [:lets] into [value (src-coerce! g :headers :string)])
403+
(assoc-in [:info :public :parameters :header] g))))
381404

382405
;;
383406
;; body-params
@@ -393,10 +416,12 @@
393416
" (ok {:total (+ x y)}))")))
394417

395418
(defmethod restructure-param :body-params [_ body-params acc]
396-
(let [schema (strict (fnk-schema body-params))]
419+
(let [schema (strict (fnk-schema body-params))
420+
g (gensym 'body-params-schema)]
397421
(-> acc
398-
(update-in [:letks] into [body-params (src-coerce! schema :body-params :body)])
399-
(assoc-in [:info :public :parameters :body] schema))))
422+
(update :outer-lets into [g schema])
423+
(update-in [:letks] into [body-params (src-coerce! g :body-params :body)])
424+
(assoc-in [:info :public :parameters :body] g))))
400425

401426
;;
402427
;; form-params
@@ -413,10 +438,12 @@
413438
" (ok {:total (+ x y)}))")))
414439

415440
(defmethod restructure-param :form-params [_ form-params acc]
416-
(let [schema (strict (fnk-schema form-params))]
441+
(let [schema (strict (fnk-schema form-params))
442+
g (gensym 'form-params-schema)]
417443
(-> acc
418-
(update-in [:letks] into [form-params (src-coerce! schema :form-params :string)])
419-
(update-in [:info :public :parameters :formData] st/merge schema)
444+
(update :outer-lets into [g schema])
445+
(update-in [:letks] into [form-params (src-coerce! g :form-params :string)])
446+
(update-in [:info :public :parameters :formData] #(if % (list `st/merge % g) g))
420447
(assoc-in [:info :public :consumes] ["application/x-www-form-urlencoded"]))))
421448

422449
;;
@@ -438,10 +465,12 @@
438465
" (ok (dissoc foo :tempfile)))")))
439466

440467
(defmethod restructure-param :multipart-params [_ params acc]
441-
(let [schema (strict (fnk-schema params))]
468+
(let [schema (strict (fnk-schema params))
469+
g (gensym 'multipart-params-schema)]
442470
(-> acc
443-
(update-in [:letks] into [params (src-coerce! schema :multipart-params :string)])
444-
(update-in [:info :public :parameters :formData] st/merge schema)
471+
(update :outer-lets into [g schema])
472+
(update-in [:letks] into [params (src-coerce! g :multipart-params :string)])
473+
(update-in [:info :public :parameters :formData] #(if % (list `st/merge % g) g))
445474
(assoc-in [:info :public :consumes] ["multipart/form-data"]))))
446475

447476
;;
@@ -458,10 +487,12 @@
458487
" (ok {:total (+ x y)}))")))
459488

460489
(defmethod restructure-param :header-params [_ header-params acc]
461-
(let [schema (fnk-schema header-params)]
490+
(let [schema (fnk-schema header-params)
491+
g (gensym 'multipart-params-schema)]
462492
(-> acc
463-
(update-in [:letks] into [header-params (src-coerce! schema :headers :string)])
464-
(assoc-in [:info :public :parameters :header] schema))))
493+
(update :outer-lets into [g schema])
494+
(update-in [:letks] into [header-params (src-coerce! g :headers :string)])
495+
(assoc-in [:info :public :parameters :header] g))))
465496

466497
;;
467498
;; :query-params
@@ -477,10 +508,12 @@
477508
" (ok {:total (+ x y)}))")))
478509

479510
(defmethod restructure-param :query-params [_ query-params acc]
480-
(let [schema (fnk-schema query-params)]
511+
(let [schema (fnk-schema query-params)
512+
g (gensym 'multipart-params-schema)]
481513
(-> acc
482-
(update-in [:letks] into [query-params (src-coerce! schema :query-params :string)])
483-
(assoc-in [:info :public :parameters :query] schema))))
514+
(update :outer-lets into [g schema])
515+
(update-in [:letks] into [query-params (src-coerce! g :query-params :string)])
516+
(assoc-in [:info :public :parameters :query] g))))
484517

485518
;;
486519
;; path-params
@@ -496,10 +529,12 @@
496529
" (ok {:total (+ x y)}))")))
497530

498531
(defmethod restructure-param :path-params [_ path-params acc]
499-
(let [schema (fnk-schema path-params)]
532+
(let [schema (fnk-schema path-params)
533+
g (gensym 'form-params-schema)]
500534
(-> acc
501-
(update-in [:letks] into [path-params (src-coerce! schema :route-params :string)])
502-
(assoc-in [:info :public :parameters :path] schema))))
535+
(update :outer-lets into [g schema])
536+
(update-in [:letks] into [path-params (src-coerce! g :route-params :string)])
537+
(assoc-in [:info :public :parameters :path] g))))
503538

504539
;;
505540
;; middleware
@@ -568,7 +603,7 @@
568603

569604
(defmethod help/help-for [:meta :coercion] [_ _]
570605
(help/text
571-
"Route-spesific overrides for coercion. See more on wiki:"
606+
"Route-specific overrides for coercion. See more on wiki:"
572607
"https://github.com/metosin/compojure-api/wiki/Validation-and-coercion\n"
573608
(help/code
574609
"(POST \"/user\" []"
@@ -577,22 +612,24 @@
577612
" (ok user))")))
578613

579614
(defmethod restructure-param :coercion [_ coercion acc]
580-
(-> acc
581-
(assoc-in [:info :coercion] coercion)
582-
(update-in [:middleware] conj [`mw/wrap-coercion coercion])))
615+
(let [g (gensym 'coercion)]
616+
(-> acc
617+
(update :outer-lets into [g coercion])
618+
(assoc-in [:info :coercion] g)
619+
(update-in [:middleware] conj [`mw/wrap-coercion g]))))
583620

584621
;;
585622
;; Impl
586623
;;
587624

588625
(defmacro dummy-let
589-
"Dummy let-macro used in resolving route-docs. not part of normal invokation chain."
626+
"Dummy let-macro used in resolving route-docs. not part of normal invocation chain."
590627
[bindings & body]
591628
(let [bind-form (vec (apply concat (for [n (take-nth 2 bindings)] [n nil])))]
592629
`(let ~bind-form ~@body)))
593630

594631
(defmacro dummy-letk
595-
"Dummy letk-macro used in resolving route-docs. not part of normal invokation chain."
632+
"Dummy letk-macro used in resolving route-docs. not part of normal invocation chain."
596633
[bindings & body]
597634
(reduce
598635
(fn [cur-body-form [bind-form]]
@@ -612,7 +649,8 @@
612649
[path route]
613650
`(compojure.api.compojure-compat/make-context
614651
~(#'compojure.core/context-route path)
615-
(constantly ~route)))
652+
(let [r# ~route]
653+
(fn [_#] r#))))
616654

617655
(defn routing [handlers]
618656
(if-let [handlers (seq (keep identity (flatten handlers)))]
@@ -731,24 +769,7 @@
731769
(when (var? v)
732770
(when (middleware-vars (symbol v))
733771
(let [[_ path route-arg & args] body
734-
[options body] (extract-parameters args true)
735-
[path-string lets arg-with-request] (destructure-compojure-api-request path route-arg)
736-
{:keys [lets
737-
letks
738-
responses
739-
middleware
740-
info
741-
swagger
742-
body]} (reduce
743-
(fn [acc [k v]]
744-
(restructure-param k v (update-in acc [:parameters] dissoc k)))
745-
{:lets lets
746-
:letks []
747-
:responses nil
748-
:middleware []
749-
:info {}
750-
:body body}
751-
options)]
772+
[options body] (extract-parameters args true)]
752773
(static-body? &env body))))))))))
753774

754775
(def route-middleware-vars (into #{}
@@ -921,6 +942,7 @@
921942

922943
{:keys [lets
923944
letks
945+
outer-lets
924946
responses
925947
middleware
926948
info
@@ -930,6 +952,7 @@
930952
(restructure-param k v (update-in acc [:parameters] dissoc k)))
931953
{:lets lets
932954
:letks []
955+
:outer-lets [] ;; lets around the call to map->Route
933956
:responses nil
934957
:middleware []
935958
:info {}
@@ -942,6 +965,7 @@
942965
(-> info :public :static)))
943966
"Cannot be both a :dynamic and :static context.")
944967

968+
;; I think it's ok if we have :outer-lets
945969
bindings? (boolean (or (route-args? route-arg) (seq lets) (seq letks)))
946970

947971
_ (assert (not (and (-> info :public :static)
@@ -1064,24 +1088,26 @@
10641088
form `(compojure.core/let-request [~arg-with-request ~'+compojure-api-request+] ~form)
10651089
form `(fn [~'+compojure-api-request+] ~form)
10661090
form `(delay (flatten (~form {})))]
1067-
form)]
1068-
1069-
`(routes/map->Route
1070-
{:path ~path-string
1071-
:method ~method
1072-
:info (merge-parameters ~info)
1073-
:childs ~childs
1074-
:handler ~form}))
1091+
form)
1092+
form `(routes/map->Route
1093+
{:path ~path-string
1094+
:method ~method
1095+
:info (merge-parameters ~info)
1096+
:childs ~childs
1097+
:handler ~form})
1098+
form (if (seq outer-lets) `(let ~outer-lets ~form) form)]
1099+
form)
10751100

10761101
;; endpoints
10771102
(let [form `(do ~@body)
10781103
form (if (seq letks) `(p/letk ~letks ~form) form)
10791104
form (if (seq lets) `(let ~lets ~form) form)
10801105
form (compojure.core/compile-route method path arg-with-request (list form))
1081-
form (if (seq middleware) `(compojure.core/wrap-routes ~form (mw/compose-middleware ~middleware)) form)]
1082-
1083-
`(routes/map->Route
1084-
{:path ~path-string
1085-
:method ~method
1086-
:info (merge-parameters ~info)
1087-
:handler ~form})))))
1106+
form (if (seq middleware) `(compojure.core/wrap-routes ~form (mw/compose-middleware ~middleware)) form)
1107+
form `(routes/map->Route
1108+
{:path ~path-string
1109+
:method ~method
1110+
:info (merge-parameters ~info)
1111+
:handler ~form})
1112+
form (if (seq outer-lets) `(let ~outer-lets ~form) form)]
1113+
form))))

0 commit comments

Comments
 (0)