diff --git a/CHANGELOG.md b/CHANGELOG.md index 18a8162f..492f454b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,15 @@ We use [Break Versioning][breakver]. The version numbers follow a `.> (for [[p d _] routes] - (when-let [problems (and spec (s/explain-data spec d))] + (when-let [problems (and spec (s/explain-data (wrap spec) d))] (->Problem p nil d spec problems))) (keep identity) (seq) (vec))) -(defn validate [routes {:keys [spec] :or {spec ::default-data}}] - (when-let [problems (validate-route-data routes spec)] +(defn validate [routes {:keys [spec ::wrap] :or {spec ::default-data, wrap identity}}] + (when-let [problems (validate-route-data routes wrap spec)] (exception/fail! ::invalid-route-data {:problems problems}))) diff --git a/modules/reitit-dev/project.clj b/modules/reitit-dev/project.clj index 444a2201..15aa954e 100644 --- a/modules/reitit-dev/project.clj +++ b/modules/reitit-dev/project.clj @@ -9,5 +9,6 @@ :parent-project {:path "../../project.clj" :inherit [:deploy-repositories :managed-dependencies]} :dependencies [[metosin/reitit-core] + [com.bhauman/spell-spec] [expound] [fipp]]) diff --git a/modules/reitit-dev/src/reitit/dev/pretty.cljc b/modules/reitit-dev/src/reitit/dev/pretty.cljc index fe7763ae..fa631723 100644 --- a/modules/reitit-dev/src/reitit/dev/pretty.cljc +++ b/modules/reitit-dev/src/reitit/dev/pretty.cljc @@ -3,6 +3,9 @@ [clojure.spec.alpha :as s] [reitit.exception :as exception] [arrangement.core] + ;; spell-spec + [spec-tools.spell :as spell] + [spell-spec.expound] ;; expound [expound.ansi] [expound.alpha] @@ -178,7 +181,7 @@ (if (and (not= 1 line)) (let [file-name (str/replace file #"(.*?)\.\S[^\.]+" "$1") target-name (name target) - ns (str (subs target-name 0 (str/index-of target-name (str "user" "$"))) file-name)] + ns (str (subs target-name 0 (or (str/index-of target-name (str file-name "$")) 0)) file-name)] (str ns ":" line)) "repl") (catch #?(:clj Exception, :cljs js/Error) _ @@ -220,10 +223,10 @@ (defn exception [e] (let [data (-> e ex-data :data) message (format-exception (-> e ex-data :type) #?(:clj (.getMessage ^Exception e) :cljs (ex-message e)) data) - source #?(:clj (->> e Throwable->map :trace - (drop-while #(not= (name (first %)) "reitit.core$router")) - (drop-while #(= (name (first %)) "reitit.core$router")) - next first source-str) + source #?(:clj (->> e Throwable->map :trace + (drop-while #(not= (name (first %)) "reitit.core$router")) + (drop-while #(= (name (first %)) "reitit.core$router")) + next first source-str) :cljs "unknown")] (ex-info (exception-str message source (printer)) (assoc (or data {}) ::exception/cause e)))) @@ -316,12 +319,12 @@ (into [:group] (map - (fn [{:keys [data path spec]}] + (fn [{:keys [data path spec scope]}] [:group [:span (color :grey "-- On route -----------------------")] [:break] [:break] - (text path) + (text path) (if scope [:span " " (text scope)]) [:break] [:break] (-> (s/explain-data spec data) diff --git a/modules/reitit-http/src/reitit/http.cljc b/modules/reitit-http/src/reitit/http.cljc index ecb28fca..e4a9a6ca 100644 --- a/modules/reitit-http/src/reitit/http.cljc +++ b/modules/reitit-http/src/reitit/http.cljc @@ -2,8 +2,7 @@ (:require [meta-merge.core :refer [meta-merge]] [reitit.interceptor :as interceptor] [reitit.ring :as ring] - [reitit.core :as r] - [reitit.impl :as impl])) + [reitit.core :as r])) (defrecord Endpoint [data interceptors queue handler path method]) @@ -16,6 +15,9 @@ (defn compile-result [[path data] {:keys [::default-options-handler] :as opts}] (let [[top childs] (ring/group-keys data) + childs (cond-> childs + (and (not (:options childs)) (not (:handler top)) default-options-handler) + (assoc :options {:no-doc true, :handler default-options-handler})) compile (fn [[path data] opts scope] (interceptor/compile-result [path data] opts scope)) ->endpoint (fn [p d m s] @@ -29,12 +31,7 @@ (fn [acc method] (cond-> acc any? (assoc method (->endpoint path data method nil)))) - (ring/map->Methods - {:options - (if default-options-handler - (->endpoint path (assoc data - :handler default-options-handler - :no-doc true) :options nil))}) + (ring/map->Methods {}) ring/http-methods))] (if-not (seq childs) (->methods true top) diff --git a/modules/reitit-http/src/reitit/http/coercion.cljc b/modules/reitit-http/src/reitit/http/coercion.cljc index 45e62fd1..459aaadb 100644 --- a/modules/reitit-http/src/reitit/http/coercion.cljc +++ b/modules/reitit-http/src/reitit/http/coercion.cljc @@ -11,7 +11,13 @@ {:name ::coerce-request :spec ::rs/parameters :compile (fn [{:keys [coercion parameters]} opts] - (if (and coercion parameters) + (cond + ;; no coercion, skip + (not coercion) nil + ;; just coercion, don't mount + (not parameters) {} + ;; mount + :else (let [coercers (coercion/request-coercers coercion parameters opts)] {:enter (fn [ctx] (let [request (:request ctx) @@ -27,7 +33,13 @@ {:name ::coerce-response :spec ::rs/responses :compile (fn [{:keys [coercion responses]} opts] - (if (and coercion responses) + (cond + ;; no coercion, skip + (not coercion) nil + ;; just coercion, don't mount + (not responses) {} + ;; mount + :else (let [coercers (coercion/response-coercers coercion responses opts)] {:leave (fn [ctx] (let [request (:request ctx) diff --git a/modules/reitit-http/src/reitit/http/spec.cljc b/modules/reitit-http/src/reitit/http/spec.cljc index fad6ee01..f665c250 100644 --- a/modules/reitit-http/src/reitit/http/spec.cljc +++ b/modules/reitit-http/src/reitit/http/spec.cljc @@ -12,15 +12,15 @@ (s/def ::interceptors (s/coll-of (partial satisfies? interceptor/IntoInterceptor))) (s/def ::data - (s/keys :opt-un [::rs/handler ::rs/name ::interceptors])) + (s/keys :opt-un [::rs/handler ::rs/name ::rs/no-doc ::interceptors])) ;; ;; Validator ;; (defn validate - [routes {:keys [spec] :or {spec ::data}}] - (when-let [problems (rrs/validate-route-data routes :interceptors spec)] + [routes {:keys [spec ::rs/wrap] :or {spec ::data, wrap identity}}] + (when-let [problems (rrs/validate-route-data routes :interceptors wrap spec)] (exception/fail! - ::invalid-route-data + ::rs/invalid-route-data {:problems problems}))) diff --git a/modules/reitit-interceptors/src/reitit/http/interceptors/multipart.clj b/modules/reitit-interceptors/src/reitit/http/interceptors/multipart.clj index 12bf779e..cb767c0d 100644 --- a/modules/reitit-interceptors/src/reitit/http/interceptors/multipart.clj +++ b/modules/reitit-interceptors/src/reitit/http/interceptors/multipart.clj @@ -11,6 +11,9 @@ (s/def ::bytes bytes?) (s/def ::size int?) +(s/def ::multipart :reitit.core.coercion/model) +(s/def ::parameters (s/keys :opt-un [::multipart])) + (def temp-file-part "Spec for file param created by ring.middleware.multipart-params.temp-file store." (st/spec @@ -41,6 +44,7 @@ (multipart-interceptor nil)) ([options] {:name ::multipart + :spec ::parameters :compile (fn [{:keys [parameters coercion]} opts] (if-let [multipart (:multipart parameters)] (let [parameter-coercion {:multipart (coercion/->ParameterCoercion diff --git a/modules/reitit-ring/src/reitit/ring.cljc b/modules/reitit-ring/src/reitit/ring.cljc index 4f2abef5..0915151a 100644 --- a/modules/reitit-ring/src/reitit/ring.cljc +++ b/modules/reitit-ring/src/reitit/ring.cljc @@ -30,6 +30,9 @@ (defn compile-result [[path data] {:keys [::default-options-handler] :as opts}] (let [[top childs] (group-keys data) + childs (cond-> childs + (and (not (:options childs)) (not (:handler top)) default-options-handler) + (assoc :options {:no-doc true, :handler default-options-handler})) ->endpoint (fn [p d m s] (-> (middleware/compile-result [p d] opts s) (map->Endpoint) @@ -40,12 +43,7 @@ (fn [acc method] (cond-> acc any? (assoc method (->endpoint path data method nil)))) - (map->Methods - {:options - (if default-options-handler - (->endpoint path (assoc data - :handler default-options-handler - :no-doc true) :options nil))}) + (map->Methods {}) http-methods))] (if-not (seq childs) (->methods true top) diff --git a/modules/reitit-ring/src/reitit/ring/coercion.cljc b/modules/reitit-ring/src/reitit/ring/coercion.cljc index 489c058e..3ee5676f 100644 --- a/modules/reitit-ring/src/reitit/ring/coercion.cljc +++ b/modules/reitit-ring/src/reitit/ring/coercion.cljc @@ -25,7 +25,13 @@ {:name ::coerce-request :spec ::rs/parameters :compile (fn [{:keys [coercion parameters]} opts] - (if (and coercion parameters) + (cond + ;; no coercion, skip + (not coercion) nil + ;; just coercion, don't mount + (not parameters) {} + ;; mount + :else (let [coercers (coercion/request-coercers coercion parameters opts)] (fn [handler] (fn @@ -43,7 +49,13 @@ {:name ::coerce-response :spec ::rs/responses :compile (fn [{:keys [coercion responses]} opts] - (if (and coercion responses) + (cond + ;; no coercion, skip + (not coercion) nil + ;; just coercion, don't mount + (not responses) {} + ;; mount + :else (let [coercers (coercion/response-coercers coercion responses opts)] (fn [handler] (fn diff --git a/modules/reitit-ring/src/reitit/ring/spec.cljc b/modules/reitit-ring/src/reitit/ring/spec.cljc index 60fb6f4f..6cfaa299 100644 --- a/modules/reitit-ring/src/reitit/ring/spec.cljc +++ b/modules/reitit-ring/src/reitit/ring/spec.cljc @@ -9,10 +9,19 @@ ;; (s/def ::middleware (s/coll-of #(satisfies? middleware/IntoMiddleware %))) +(s/def ::get map?) +(s/def ::head map?) +(s/def ::post map?) +(s/def ::put map?) +(s/def ::delete map?) +(s/def ::connect map?) +(s/def ::options map?) +(s/def ::trace map?) +(s/def ::patch map?) + (s/def ::data - (s/keys :req-un [::rs/handler] - :opt-un [::rs/name ::middleware])) + (s/keys :opt-un [::rs/handler ::rs/name ::rs/no-doc ::middleware])) ;; ;; Validator @@ -26,21 +35,21 @@ :invalid non-specs})) (s/merge-spec-impl (vec specs) (vec specs) nil)) -(defn validate-route-data [routes key spec] +(defn validate-route-data [routes key wrap spec] (->> (for [[p _ c] routes [method {:keys [data] :as endpoint}] c :when endpoint :let [target (key endpoint) component-specs (seq (keep :spec target)) specs (keep identity (into [spec] component-specs)) - spec (merge-specs specs)]] + spec (wrap (merge-specs specs))]] (when-let [problems (and spec (s/explain-data spec data))] (rs/->Problem p method data spec problems))) (keep identity) (seq))) (defn validate - [routes {:keys [spec] :or {spec ::data}}] - (when-let [problems (validate-route-data routes :middleware spec)] + [routes {:keys [spec ::rs/wrap] :or {spec ::data, wrap identity}}] + (when-let [problems (validate-route-data routes :middleware wrap spec)] (exception/fail! - ::invalid-route-data + ::rs/invalid-route-data {:problems problems}))) diff --git a/modules/reitit-swagger/src/reitit/swagger.cljc b/modules/reitit-swagger/src/reitit/swagger.cljc index eff32865..44255238 100644 --- a/modules/reitit-swagger/src/reitit/swagger.cljc +++ b/modules/reitit-swagger/src/reitit/swagger.cljc @@ -89,12 +89,12 @@ (if (and data (not no-doc)) [method (meta-merge - (apply meta-merge (keep (comp :swagger :data) middleware)) - (apply meta-merge (keep (comp :swagger :data) interceptors)) - (if coercion - (coercion/get-apidocs coercion :swagger data)) - (select-keys data [:tags :summary :description]) - (strip-top-level-keys swagger))])) + (apply meta-merge (keep (comp :swagger :data) middleware)) + (apply meta-merge (keep (comp :swagger :data) interceptors)) + (if coercion + (coercion/get-apidocs coercion :swagger data)) + (select-keys data [:tags :summary :description]) + (strip-top-level-keys swagger))])) transform-path (fn [[p _ c]] (if-let [endpoint (some->> c (keep transform-endpoint) (seq) (into {}))] [(swagger-path p) endpoint]))] diff --git a/perf-test/clj/reitit/go_perf_test.clj b/perf-test/clj/reitit/go_perf_test.clj index 4e432c2e..ba03b753 100644 --- a/perf-test/clj/reitit/go_perf_test.clj +++ b/perf-test/clj/reitit/go_perf_test.clj @@ -296,8 +296,7 @@ (def app (ring/ring-handler (ring/router - (reduce (partial add h) [] routes) - {::trie/parameters trie/record-parameters}) + (reduce (partial add h) [] routes)) (ring/create-default-handler) {:inject-match? false, :inject-router? false})) @@ -319,6 +318,7 @@ ;; 140µs (java-segment-router) ;; 60ns (java-segment-router, no injects) ;; 55ns (trie-router, no injects) + ;; 54µs (trie-router, quick-pam) ;; 54ns (trie-router, no injects, optimized) (let [req (map->Req {:request-method :get, :uri "/user/repos"})] (title "static") @@ -337,6 +337,7 @@ ;; 273ns (trie-router, no injects, direct-data) ;; 256ns (trie-router, pre-defined parameters) ;; 237ns (trie-router, single-sweep wild-params) + ;; 226µs (trie-router, quick-pam) ;; 191ns (trie-router, record parameters) (let [req (map->Req {:request-method :get, :uri "/repos/julienschmidt/httprouter/stargazers"})] (title "param") @@ -354,6 +355,7 @@ ;; 63µs (trie-router, no injects, switch-case) - 124µs (clojure) ;; 63µs (trie-router, no injects, direct-data) ;; 54µs (trie-router, non-transient params) + ;; 50µs (trie-router, quick-pam) ;; 49µs (trie-router, pre-defined parameters) (let [requests (mapv route->req routes)] (title "all") diff --git a/perf-test/clj/reitit/request_perf.cljc b/perf-test/clj/reitit/request_perf.cljc new file mode 100644 index 00000000..eb552d7f --- /dev/null +++ b/perf-test/clj/reitit/request_perf.cljc @@ -0,0 +1,96 @@ +(ns reitit.request-perf + (:require [criterium.core :as cc] + [reitit.perf-utils :refer :all] + [potemkin :as p])) + +(set! *warn-on-reflection* true) + +;; +;; start repl with `lein perf repl` +;; perf measured with the following setup: +;; +;; Model Name: MacBook Pro +;; Model Identifier: MacBookPro113 +;; Processor Name: Intel Core i7 +;; Processor Speed: 2,5 GHz +;; Number of Processors: 1 +;; Total Number of Cores: 4 +;; L2 Cache (per Core): 256 KB +;; L3 Cache: 6 MB +;; Memory: 16 GB +;; + +(defprotocol RawRequest + (-uri [this]) + (-request-method [this]) + (-path-params [this])) + +(p/def-derived-map + ZeroCopyRequest + [raw] + :uri (-uri raw) + :request-method (-request-method raw) + :path-params (-path-params raw)) + +(defprotocol RingRequest + (get-uri [this]) + (get-request-method [this]) + (get-path-params [this])) + +(defn ring-request [raw] + {:uri (-uri raw) + :request-method (-request-method raw) + :path-params (-path-params raw)}) + +(defn record-request [raw] + (->RecordRequest (-uri raw) (-request-method raw) (-path-params raw))) + +(defrecord RawRingRequest [raw] + RingRequest + (get-uri [_] (-uri raw)) + (get-request-method [_] (-request-method raw)) + (get-path-params [_] (-path-params raw))) + +(def raw + (reify + RawRequest + (-uri [_] "/ping") + (-request-method [_] :get) + (-path-params [_] {:a 1}))) + +(defn bench-all! [] + + ;; 530ns + (title "potemkin zero-copy") + (assert (= :get (:request-method (->ZeroCopyRequest raw)))) + (cc/quick-bench + (let [req (->ZeroCopyRequest raw)] + (dotimes [_ 10] + (:request-method req)))) + + ;; 73ns + (title "map copy-request") + (assert (= :get (:request-method (ring-request raw)))) + (cc/quick-bench + (let [req (ring-request raw)] + (dotimes [_ 10] + (:request-method req)))) + + ;; 7ns + (title "record copy-request") + (assert (= :get (:request-method (record-request raw)))) + (cc/quick-bench + (let [req (record-request raw)] + (dotimes [_ 10] + (:request-method req)))) + + ;; 7ns + (title "request protocols") + (assert (= :get (get-request-method (->RawRingRequest raw)))) + (cc/quick-bench + (let [req (->RawRingRequest raw)] + (dotimes [_ 10] + (get-request-method req))))) + +(comment + (bench-all!)) diff --git a/project.clj b/project.clj index e00a4cd2..e8ba1f70 100644 --- a/project.clj +++ b/project.clj @@ -27,7 +27,7 @@ [metosin/reitit-sieppari "0.3.1"] [metosin/reitit-pedestal "0.3.1"] [metosin/ring-swagger-ui "2.2.10"] - [metosin/spec-tools "0.9.1"] + [metosin/spec-tools "0.9.2-alpha1"] [metosin/schema-tools "0.11.0"] [metosin/muuntaja "0.6.4"] [metosin/jsonista "0.2.2"] @@ -37,6 +37,7 @@ [fipp "0.6.17" :exclusions [org.clojure/core.rrb-vector]] [expound "0.7.2"] [lambdaisland/deep-diff "0.0-47"] + [com.bhauman/spell-spec "0.1.1"] [ring/ring-core "1.7.1"] [io.pedestal/pedestal.service "0.5.5"]] @@ -69,7 +70,7 @@ :java-source-paths ["modules/reitit-core/java-src"] - :dependencies [[org.clojure/clojure "1.10.0"] + :dependencies [[org.clojure/clojure "1.10.1-beta2"] [org.clojure/clojurescript "1.10.520"] ;; modules dependencies @@ -80,6 +81,7 @@ [metosin/jsonista] [lambdaisland/deep-diff] [meta-merge] + [com.bhauman/spell-spec] [expound] [fipp] diff --git a/test/clj/reitit/http_test.clj b/test/clj/reitit/http_test.clj index f18bdf36..934ef9bc 100644 --- a/test/clj/reitit/http_test.clj +++ b/test/clj/reitit/http_test.clj @@ -495,11 +495,11 @@ (fn [{:keys [::r/router]}] (is router)) {:executor sieppari/executor}) - {})) + {})) (testing "3-arity" ((http/ring-handler (http/router []) (fn [{:keys [::r/router]}] (is router)) {:executor sieppari/executor}) - {} ::respond ::raise))) + {} ::respond ::raise))) diff --git a/test/cljc/reitit/ring_spec_test.cljc b/test/cljc/reitit/ring_spec_test.cljc index ae99764d..6e2d7c6d 100644 --- a/test/cljc/reitit/ring_spec_test.cljc +++ b/test/cljc/reitit/ring_spec_test.cljc @@ -21,13 +21,13 @@ (testing "with default spec validates :name, :handler and :middleware" (is (thrown-with-msg? ExceptionInfo - #":reitit.ring.spec/invalid-route-data" + #"Invalid route data" (ring/router ["/api" {:handler "identity"}] {:validate rrs/validate}))) (is (thrown-with-msg? ExceptionInfo - #":reitit.ring.spec/invalid-route-data" + #"Invalid route data" (ring/router ["/api" {:handler identity :name "kikka"}] @@ -36,7 +36,7 @@ (testing "all endpoints are validated" (is (thrown-with-msg? ExceptionInfo - #":reitit.ring.spec/invalid-route-data" + #"Invalid route data" (ring/router ["/api" {:patch {:handler "identity"}}] {:validate rrs/validate})))) @@ -69,7 +69,7 @@ (handler request)))}]}}))) (is (thrown-with-msg? ExceptionInfo - #":reitit.ring.spec/invalid-route-data" + #"Invalid route data" (ring/router ["/api" {:get {:handler identity :roles #{:adminz}}}] @@ -113,7 +113,7 @@ (is (thrown-with-msg? ExceptionInfo - #":reitit.ring.spec/invalid-route-data" + #"Invalid route data" (ring/router ["/api" ["/plus/:e" diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index bce7314d..b23d41b7 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -570,10 +570,10 @@ (ring/router []) (fn [{:keys [::r/router]}] (is router))) - {})) + {})) (testing "3-arity" ((ring/ring-handler (ring/router []) (fn [{:keys [::r/router]} _ _] (is router))) - {} ::respond ::raise))) + {} ::respond ::raise)))