From 1f0412676978690e42f50bd1ab2514ffc20ebd23 Mon Sep 17 00:00:00 2001 From: Ambrose Bonnaire-Sergeant Date: Mon, 22 Apr 2024 02:56:38 -0500 Subject: [PATCH] wip static only if certain --- CHANGELOG.md | 5 + src/compojure/api/core.clj | 2 +- src/compojure/api/meta.clj | 214 ++++++++++++++++-- src/compojure/api/resource.clj | 2 +- test/compojure/api/integration_test.clj | 9 +- test/compojure/api/perf_test.clj | 2 +- .../compojure/api/coercion/issue336_test.clj | 2 + 7 files changed, 216 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 891f8e38..c10479db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ See also: [compojure-api 1.1.x changelog](./CHANGELOG-1.1.x.md) * Remove potemkin [#445](https://github.com/metosin/compojure-api/issues/445) * Add back `compojure.api.routes/create` * Add back `middleware` (and deprecate it) +* Make `context` :dynamic by default +* Add `:static true` option to `context` +* Add static context optimization coach + * `-Dcompojure.api.meta.static-context-coach=print` to print hints + * `-Dcompojure.api.meta.static-context-coach=assert` to assert hints ## 2.0.0-alpha31 (2019-12-20) diff --git a/src/compojure/api/core.clj b/src/compojure/api/core.clj index 6bf3bb61..0c9cb97e 100644 --- a/src/compojure/api/core.clj +++ b/src/compojure/api/core.clj @@ -74,7 +74,7 @@ {:childs [handler] :handler x-handler}))) -(defmacro context {:style/indent 2} [& args] (meta/restructure nil args {:context? true})) +(defmacro context {:style/indent 2} [& args] (meta/restructure nil args {:context? true :&form &form :&env &env})) (defmacro GET {:style/indent 2} [& args] (meta/restructure :get args nil)) (defmacro ANY {:style/indent 2} [& args] (meta/restructure nil args nil)) diff --git a/src/compojure/api/meta.clj b/src/compojure/api/meta.clj index c6331f42..e9d86452 100644 --- a/src/compojure/api/meta.clj +++ b/src/compojure/api/meta.clj @@ -75,27 +75,57 @@ (defmethod help/help-for [:meta :dynamic] [_ _] (help/text "If set to to `true`, makes a `context` dynamic," - "e.g. body is evaluated on each request. NOTE:" - "Vanilla Compojure has this enabled by default" - "while compojure-api default to `false`, being" - "much faster. For details, see:\n" - "https://github.com/weavejester/compojure/issues/148\n" + "which wraps the body in a closure that is evaluated on each request." + "This is the default behavior in vanilla compojure. In compojure-api," + "this is also the usual behavior, except:" + "If the context does not bind any variables and its body contains" + "just top-level calls to compojure.api endpoint macros like GET," + "then the body will be cached for each request." (help/code "(context \"/static\" []" - " (if (= 0 (random-int 2))" + " :static true" + " (when (= 0 (random-int 2))" + " (println 'ping!) ;; never printed during request" " ;; mounting decided once" - " (GET \"/ping\" [] (ok \"pong\")))" + " (GET \"/ping\" [] (ok \"pong\"))))" "" "(context \"/dynamic\" []" " :dynamic true" - " (if (= 0 (random-int 2))" + " (when (= 0 (random-int 2))" + " (println 'ping!) ;; printed 50% of requests" " ;; mounted for 50% of requests" - " (GET \"/ping\" [] (ok \"pong\")))"))) + " (GET \"/ping\" [] (ok \"pong\"))))"))) (defmethod restructure-param :dynamic [k v acc] (update-in acc [:info :public] assoc k v)) +(defmethod help/help-for [:meta :static] [_ _] + (help/text + "If set to to `true`, makes a `context` static," + "which resolves the body before processing requests." + "This is much faster than :dynamic contexts at the" + "cost of expressivity: routes under a static context" + "cannot change based on the request." + + (help/code + "(context \"/static\" []" + " :static true" + " (when (= 0 (random-int 2))" + " (println 'ping!) ;; never printed during request" + " ;; mounting decided once" + " (GET \"/ping\" [] (ok \"pong\"))))" + "" + "(context \"/dynamic\" []" + " :dynamic true" + " (when (= 0 (random-int 2))" + " (println 'ping!) ;; printed 50% of requests" + " ;; mounted for 50% of requests" + " (GET \"/ping\" [] (ok \"pong\")))"))) + +(defmethod restructure-param :static [k v acc] + (update-in acc [:info :public] assoc k v)) + ;; ;; summary ;; @@ -579,7 +609,7 @@ (defmacro static-context [path route] - `(#'compojure.api.compojure-compat/make-context + `(compojure.api.compojure-compat/make-context ~(#'compojure.core/context-route path) (constantly ~route))) @@ -628,7 +658,145 @@ (defn- route-args? [arg] (not= arg [])) -(defn restructure [method [path route-arg & args] {:keys [context?]}] +(def endpoint-vars (conj (into #{} + (mapcat (fn [n] + (map #(symbol (name %) (name n)) + '[compojure.api.core + compojure.api.sweet]))) + '[GET ANY HEAD PATCH DELETE OPTIONS POST PUT]) + 'compojure.api.sweet/resource + 'compojure.api.resource/resource)) + +(defn- static-endpoint? [&env body] + (and (seq? body) + (boolean + (let [sym (first body)] + (when (symbol? sym) + (when-some [v (resolve &env sym)] + (when (var? v) + (endpoint-vars + (symbol v))))))))) + +(declare static-body?) + +(def context-vars (into #{} + (mapcat (fn [n] + (map #(symbol (name %) (name n)) + '[compojure.api.core + compojure.api.sweet]))) + '[context])) + +(defn- static-context? [&env body] + (and (seq? body) + (boolean + (let [sym (first body)] + (when (symbol? sym) + (when-some [v (resolve &env sym)] + (when (var? v) + (when (context-vars (symbol v)) + (let [[_ path route-arg & args] body + [options body] (extract-parameters args true) + [path-string lets arg-with-request] (destructure-compojure-api-request path route-arg) + {:keys [lets + letks + responses + middleware + info + swagger + body]} (reduce + (fn [acc [k v]] + (restructure-param k v (update-in acc [:parameters] dissoc k))) + {:lets lets + :letks [] + :responses nil + :middleware [] + :info {} + :body body} + options) + static? (not (or (-> info :public :dynamic) + (route-args? route-arg) (seq lets) (seq letks))) + safely-static (or (-> info :public :static) (static-body? &env body))] + safely-static))))))))) + +(def middleware-vars (into #{} + (mapcat (fn [n] + (map #(symbol (name %) (name n)) + '[compojure.api.core + compojure.api.sweet]))) + '[middleware])) + +(defn- static-middleware? [&env body] + (and (seq? body) + (boolean + (let [sym (first body)] + (when (symbol? sym) + (when-some [v (resolve &env sym)] + (when (var? v) + (when (middleware-vars (symbol v)) + (let [[_ path route-arg & args] body + [options body] (extract-parameters args true) + [path-string lets arg-with-request] (destructure-compojure-api-request path route-arg) + {:keys [lets + letks + responses + middleware + info + swagger + body]} (reduce + (fn [acc [k v]] + (restructure-param k v (update-in acc [:parameters] dissoc k))) + {:lets lets + :letks [] + :responses nil + :middleware [] + :info {} + :body body} + options) + safely-static (static-body? &env body)] + safely-static))))))))) + +(def route-middleware-vars (into #{} + (mapcat (fn [n] + (map #(symbol (name %) (name n)) + '[compojure.api.core + compojure.api.sweet]))) + '[route-middleware])) + +(defn- static-route-middleware? [&env body] + (and (seq? body) + (boolean + (let [sym (first body)] + (when (symbol? sym) + (when-some [v (resolve &env sym)] + (when (var? v) + (when (route-middleware-vars (symbol v)) + (let [[_ mids & body] body] + (and (some? mids) + (static-body? &env body))))))))))) + +(defn- static-cond? [&env form] + (and (seq? form) + (boolean + (let [sym (first form)] + (when (symbol? sym) + (let [v (resolve &env sym)] + (when (or (= #'when v) + (= #'= v) + (= #'not= v) + (= sym 'if)) + (static-body? &env (next form))))))))) + +(defn- static-body? [&env body] + (every? #(or (static-endpoint? &env %) + (contains? &env %) ;;local + ((some-fn keyword? number? boolean?) %) + (static-cond? &env %) + (static-context? &env %) + (static-middleware? &env %) + (static-route-middleware? &env %)) + body)) + +(defn restructure [method [path route-arg & args] {:keys [context? &form &env]}] (let [[options body] (extract-parameters args true) [path-string lets arg-with-request] (destructure-compojure-api-request path route-arg) @@ -651,10 +819,30 @@ coercion (:coercion info) + _ (assert (not (and (-> info :public :dynamic) + (-> info :public :static))) + "Cannot be both a :dynamic and :static context.") + static? (not (or (-> info :public :dynamic) (route-args? route-arg) (seq lets) (seq letks))) + safely-static (or (-> info :public :static) (static-body? &env body)) + + _ (when context? + (when-not safely-static + (when-some [coach (or (System/getProperty "compojure.api.meta.static-context-coach") + "assert")] + (when (and static? (not (-> info :public :static))) + (case coach + "off" nil + "print" (println "This looks like it could be a static context: " (pr-str {:form &form :meta (meta &form)})) + "assert" (throw (ex-info "This looks like it could be a static context" + {:form &form + :meta (meta &form)})) + (throw (ex-info "compojure.api.meta.static-context-coach must be either print or assert" {:provided coach}))))))) + + ;; :dynamic by default + static-context? (and static? context? safely-static) - static-context? (and static? context?) info (cond-> info static-context? (assoc :static-context? static-context?)) @@ -676,7 +864,7 @@ ~form) form) form (if (seq middleware) `((mw/compose-middleware ~middleware) ~form) form) - form (if static? + form (if static-context? `(static-context ~path ~form) `(compojure.core/context ~path ~arg-with-request ~form)) diff --git a/src/compojure/api/resource.clj b/src/compojure/api/resource.clj index 5a032eca..ec74a061 100644 --- a/src/compojure/api/resource.clj +++ b/src/compojure/api/resource.clj @@ -159,7 +159,7 @@ Setting value to `(constantly nil)` disables both request- & response coercion. See tests and wiki for details. - Enchancements to ring-swagger operations map: + Enhancements to ring-swagger operations map: 1) :parameters use ring request keys (query-params, path-params, ...) instead of swagger-params (query, path, ...). This keeps things simple as ring keys are used in diff --git a/test/compojure/api/integration_test.clj b/test/compojure/api/integration_test.clj index ad0a2818..98d18675 100644 --- a/test/compojure/api/integration_test.clj +++ b/test/compojure/api/integration_test.clj @@ -1206,7 +1206,7 @@ (GET "/route" [q] :swagger {:x-name :boolean :operationId "echoBoolean" - :description "Ehcoes a boolean" + :description "Echoes a boolean" :parameters {:query {:q s/Bool}}} (ok {:q q})))] @@ -1220,7 +1220,7 @@ => (contains {:x-name "boolean" :operationId "echoBoolean" - :description "Ehcoes a boolean" + :description "Echoes a boolean" :parameters [{:description "" :in "query" :name "q" @@ -1229,7 +1229,7 @@ (fact "run-time values" (let [runtime-data {:x-name :boolean :operationId "echoBoolean" - :description "Ehcoes a boolean" + :description "Echoes a boolean" :parameters {:query {:q s/Bool}}} app (api (swagger-routes) @@ -1247,7 +1247,7 @@ => (contains {:x-name "boolean" :operationId "echoBoolean" - :description "Ehcoes a boolean" + :description "Echoes a boolean" :parameters [{:description "" :in "query" :name "q" @@ -1705,6 +1705,7 @@ (fact "context" (let [app (api (context "/api" [] + :static true (for [path ["/ping" "/pong"]] (GET path [] (ok {:path path})))))] diff --git a/test/compojure/api/perf_test.clj b/test/compojure/api/perf_test.clj index 2e81d2c6..30c204bd 100644 --- a/test/compojure/api/perf_test.clj +++ b/test/compojure/api/perf_test.clj @@ -336,4 +336,4 @@ (time (dotimes [_ count] (call3))) - #_(cc/quick-bench (call3)))) \ No newline at end of file + #_(cc/quick-bench (call3)))) diff --git a/test19/compojure/api/coercion/issue336_test.clj b/test19/compojure/api/coercion/issue336_test.clj index 07e390fc..b01b0f5c 100644 --- a/test19/compojure/api/coercion/issue336_test.clj +++ b/test19/compojure/api/coercion/issue336_test.clj @@ -39,8 +39,10 @@ (context "/api" [] :tags ["api"] :coercion :spec + :static true (context "/jr1" [] + :static true (resource {:get {:summary "Number of successful full-text article requests by month and journal"