From dc92f6f48e321a9eece01aef1787c62e26a21f24 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Fri, 19 Apr 2019 22:25:59 -0400 Subject: [PATCH 01/11] http-swagger++ --- examples/http-swagger/src/example/server.clj | 8 +++++++- modules/reitit-http/src/reitit/http/spec.cljc | 2 +- modules/reitit-ring/src/reitit/ring/spec.cljc | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/examples/http-swagger/src/example/server.clj b/examples/http-swagger/src/example/server.clj index 81b8ede4..7ddcd9c3 100644 --- a/examples/http-swagger/src/example/server.clj +++ b/examples/http-swagger/src/example/server.clj @@ -9,8 +9,10 @@ [reitit.http.interceptors.muuntaja :as muuntaja] [reitit.http.interceptors.exception :as exception] [reitit.http.interceptors.multipart :as multipart] + [reitit.http.spec :as spec] [reitit.http.interceptors.dev :as dev] [reitit.interceptor.sieppari :as sieppari] + [reitit.dev.pretty :as pretty] [ring.adapter.jetty :as jetty] [aleph.http :as client] [muuntaja.core :as m] @@ -110,9 +112,13 @@ :body {:total (- x y)}})}}]]] {;;:reitit.interceptor/transform dev/print-context-diffs + :validate spec/validate + :exception pretty/exception :data {:coercion spec-coercion/coercion :muuntaja m/instance - :interceptors [;; query-params & form-params + :interceptors [;; swagger feature + swagger/swagger-feature + ;; query-params & form-params (parameters/parameters-interceptor) ;; content-negotiation (muuntaja/format-negotiate-interceptor) diff --git a/modules/reitit-http/src/reitit/http/spec.cljc b/modules/reitit-http/src/reitit/http/spec.cljc index fad6ee01..d9225d38 100644 --- a/modules/reitit-http/src/reitit/http/spec.cljc +++ b/modules/reitit-http/src/reitit/http/spec.cljc @@ -22,5 +22,5 @@ [routes {:keys [spec] :or {spec ::data}}] (when-let [problems (rrs/validate-route-data routes :interceptors spec)] (exception/fail! - ::invalid-route-data + ::rs/invalid-route-data {:problems problems}))) diff --git a/modules/reitit-ring/src/reitit/ring/spec.cljc b/modules/reitit-ring/src/reitit/ring/spec.cljc index 60fb6f4f..ecdc2ff2 100644 --- a/modules/reitit-ring/src/reitit/ring/spec.cljc +++ b/modules/reitit-ring/src/reitit/ring/spec.cljc @@ -42,5 +42,5 @@ [routes {:keys [spec] :or {spec ::data}}] (when-let [problems (validate-route-data routes :middleware spec)] (exception/fail! - ::invalid-route-data + ::rs/invalid-route-data {:problems problems}))) From 674b60a12455c7fbb9b4b2a87d0fc08b46a05512 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Fri, 19 Apr 2019 22:39:45 -0400 Subject: [PATCH 02/11] spell-spec --- examples/http-swagger/src/example/server.clj | 1 + modules/reitit-core/src/reitit/spec.cljc | 19 ++++++++----- modules/reitit-dev/project.clj | 1 + modules/reitit-dev/src/reitit/dev/pretty.cljc | 20 +++++++++----- modules/reitit-http/src/reitit/http.cljc | 13 ++++----- .../reitit-http/src/reitit/http/coercion.cljc | 16 +++++++++-- modules/reitit-http/src/reitit/http/spec.cljc | 6 ++--- .../reitit/http/interceptors/multipart.clj | 4 +++ modules/reitit-ring/src/reitit/ring.cljc | 10 +++---- .../reitit-ring/src/reitit/ring/coercion.cljc | 16 +++++++++-- modules/reitit-ring/src/reitit/ring/spec.cljc | 27 ++++++++++++++----- .../reitit-swagger/src/reitit/swagger.cljc | 12 ++++----- project.clj | 2 ++ 13 files changed, 100 insertions(+), 47 deletions(-) diff --git a/examples/http-swagger/src/example/server.clj b/examples/http-swagger/src/example/server.clj index 7ddcd9c3..5453665a 100644 --- a/examples/http-swagger/src/example/server.clj +++ b/examples/http-swagger/src/example/server.clj @@ -112,6 +112,7 @@ :body {:total (- x y)}})}}]]] {;;:reitit.interceptor/transform dev/print-context-diffs + ;;:wrap-spec reitit.dev.pretty/closed-keys :validate spec/validate :exception pretty/exception :data {:coercion spec-coercion/coercion diff --git a/modules/reitit-core/src/reitit/spec.cljc b/modules/reitit-core/src/reitit/spec.cljc index fbe340d9..11081f87 100644 --- a/modules/reitit-core/src/reitit/spec.cljc +++ b/modules/reitit-core/src/reitit/spec.cljc @@ -39,7 +39,8 @@ (s/def ::name keyword?) (s/def ::handler fn?) -(s/def ::default-data (s/keys :opt-un [::name ::handler])) +(s/def ::no-doc boolean?) +(s/def ::default-data (s/keys :opt-un [::name ::handler ::no-doc])) ;; ;; router @@ -75,6 +76,8 @@ ;; coercion ;; +(s/def :reitit.core.coercion/coercion any?) + (s/def :reitit.core.coercion/model any?) (s/def :reitit.core.coercion/query :reitit.core.coercion/model) @@ -90,7 +93,8 @@ :reitit.core.coercion/path])) (s/def ::parameters - (s/keys :opt-un [:reitit.core.coercion/parameters])) + (s/keys :opt-un [:reitit.core.coercion/coercion + :reitit.core.coercion/parameters])) (s/def :reitit.core.coercion/status (s/or :number number? :default #{:default})) @@ -103,7 +107,8 @@ (s/map-of :reitit.core.coercion/status :reitit.core.coercion/response)) (s/def ::responses - (s/keys :opt-un [:reitit.core.coercion/responses])) + (s/keys :opt-un [:reitit.core.coercion/coercion + :reitit.core.coercion/responses])) ;; ;; Route data validator @@ -111,14 +116,14 @@ (defrecord Problem [path scope data spec problems]) -(defn validate-route-data [routes spec] +(defn validate-route-data [routes wrap-spec spec] (some->> (for [[p d _] routes] - (when-let [problems (and spec (s/explain-data spec d))] + (when-let [problems (and spec (s/explain-data (wrap-spec 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-spec] :or {spec ::default-data, wrap-spec identity}}] + (when-let [problems (validate-route-data routes wrap-spec 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..5b7d7d15 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,13 +223,16 @@ (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)))) +;; FIXME +(def closed-keys spec-tools.spell/closed-keys) + (defn de-expound-colors [^String s mappings] (let [s' (reduce (fn [s [from to]] @@ -316,12 +322,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..0f935966 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)) 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 d9225d38..9c54685e 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 wrap-spec] :or {spec ::data, wrap-spec identity}}] + (when-let [problems (rrs/validate-route-data routes :interceptors wrap-spec spec)] (exception/fail! ::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..104e6a7d 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)) 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 ecdc2ff2..487a2211 100644 --- a/modules/reitit-ring/src/reitit/ring/spec.cljc +++ b/modules/reitit-ring/src/reitit/ring/spec.cljc @@ -9,10 +9,25 @@ ;; (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 ::endpoint + (s/keys :req-un [::rs/handler] + :opt-un [::rs/name ::rs/no-doc ::middleware])) (s/def ::data - (s/keys :req-un [::rs/handler] - :opt-un [::rs/name ::middleware])) + (s/merge + ::endpoint + (s/map-of #{:get :head :post :put :delete :connect :options :trace :patch} map?))) ;; ;; Validator @@ -26,21 +41,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 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-spec (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 wrap-spec] :or {spec ::data, wrap-spec identity}}] + (when-let [problems (validate-route-data routes :middleware wrap-spec spec)] (exception/fail! ::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/project.clj b/project.clj index 8b2a3176..5db61e4e 100644 --- a/project.clj +++ b/project.clj @@ -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"]] @@ -80,6 +81,7 @@ [metosin/jsonista] [lambdaisland/deep-diff] [meta-merge] + [com.bhauman/spell-spec] [expound] [fipp] From dd864a01425f928c808c316994c8fdbb52d4121f Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sat, 20 Apr 2019 01:04:45 -0400 Subject: [PATCH 03/11] Set closed specs in http-swagger example --- examples/http-swagger/src/example/server.clj | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/http-swagger/src/example/server.clj b/examples/http-swagger/src/example/server.clj index 5453665a..d7083a88 100644 --- a/examples/http-swagger/src/example/server.clj +++ b/examples/http-swagger/src/example/server.clj @@ -13,6 +13,7 @@ [reitit.http.interceptors.dev :as dev] [reitit.interceptor.sieppari :as sieppari] [reitit.dev.pretty :as pretty] + [spec-tools.spell :as spell] [ring.adapter.jetty :as jetty] [aleph.http :as client] [muuntaja.core :as m] @@ -74,7 +75,7 @@ "https://randomuser.me/api/" {:query-params {:seed seed, :results results}}) :body - (partial m/decode m/instance "application/json") + (partial m/decode "application/json") :results (fn [results] {:status 200 @@ -112,7 +113,7 @@ :body {:total (- x y)}})}}]]] {;;:reitit.interceptor/transform dev/print-context-diffs - ;;:wrap-spec reitit.dev.pretty/closed-keys + :wrap-spec spell/closed-keys :validate spec/validate :exception pretty/exception :data {:coercion spec-coercion/coercion From 68d68402d90fd8257e48c975d2958cbab8958e7c Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 23 Apr 2019 07:33:41 -0400 Subject: [PATCH 04/11] Fix Java Trie example --- modules/reitit-core/java-src/reitit/Trie.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/reitit-core/java-src/reitit/Trie.java b/modules/reitit-core/java-src/reitit/Trie.java index 953a70ad..b7af4c1f 100644 --- a/modules/reitit-core/java-src/reitit/Trie.java +++ b/modules/reitit-core/java-src/reitit/Trie.java @@ -291,8 +291,8 @@ public class Trie { staticMatcher("/auth/", linearMatcher( Arrays.asList( - staticMatcher("login", dataMatcher(null, 1)), - staticMatcher("recovery", dataMatcher(null, 2))), true))), true); + staticMatcher("login", dataMatcher(PersistentArrayMap.EMPTY, 1)), + staticMatcher("recovery", dataMatcher(PersistentArrayMap.EMPTY, 2))), true))), true); System.err.println(matcher); System.out.println(lookup(matcher, "/auth/login")); System.out.println(lookup(matcher, "/auth/recovery")); From 1326d769366854569a57804671485de4d5296deb Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 23 Apr 2019 07:50:16 -0400 Subject: [PATCH 05/11] Faster params in Trie --- modules/reitit-core/java-src/reitit/Trie.java | 42 ++++++++++++------- perf-test/clj/reitit/go_perf_test.clj | 6 ++- project.clj | 2 +- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/modules/reitit-core/java-src/reitit/Trie.java b/modules/reitit-core/java-src/reitit/Trie.java index b7af4c1f..ff5eb5f5 100644 --- a/modules/reitit-core/java-src/reitit/Trie.java +++ b/modules/reitit-core/java-src/reitit/Trie.java @@ -38,20 +38,35 @@ public class Trie { return decode(new String(chars, begin, end - begin), hasPercent, hasPlus); } - public static class Match { - public IPersistentMap params; + public static final class Match { + final private Object[] params; public final Object data; + private int i = 0; - public Match(IPersistentMap params, Object data) { - this.params = params; + public Match(Integer size, Object data) { + this.params = new Object[size]; this.data = data; } + public void assoc(Keyword key, Object value) { + params[i] = key; + params[i + 1] = value; + i += 2; + } + + public IPersistentMap params() { + return new PersistentArrayMap(params); + } + + Match copy() { + return new Match(params.length, data); + } + @Override public String toString() { Map m = new HashMap<>(); m.put(Keyword.intern("data"), data); - m.put(Keyword.intern("params"), params); + m.put(Keyword.intern("params"), params()); return m.toString(); } } @@ -116,13 +131,13 @@ public class Trie { private final Match match; DataMatcher(IPersistentMap params, Object data) { - this.match = new Match(params, data); + this.match = new Match(params.count() * 2, data); } @Override public Match match(int i, int max, char[] path) { if (i == max) { - return match; + return match.copy(); } return null; } @@ -175,7 +190,7 @@ public class Trie { } final Match m = child.match(stop, max, path); if (m != null) { - m.params = m.params.assoc(key, decode(new String(path, i, stop - i), hasPercent, hasPlus)); + m.assoc(key, decode(new String(path, i, stop - i), hasPercent, hasPlus)); } return m; } @@ -204,19 +219,18 @@ public class Trie { static final class CatchAllMatcher implements Matcher { private final Keyword parameter; - private final IPersistentMap params; - private final Object data; + private final Match match; CatchAllMatcher(Keyword parameter, IPersistentMap params, Object data) { + this.match = new Match(params.count() * 2, data); this.parameter = parameter; - this.params = params; - this.data = data; } @Override public Match match(int i, int max, char[] path) { if (i <= max) { - return new Match(params.assoc(parameter, decode(path, i, max)), data); + match.copy().assoc(parameter, decode(path, i, max)); + return match; } return null; } @@ -233,7 +247,7 @@ public class Trie { @Override public String toString() { - return "[" + parameter + " " + new DataMatcher(null, data) + "]"; + return "[" + parameter + " " + new DataMatcher(null, match.data) + "]"; } } 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/project.clj b/project.clj index 5db61e4e..8b76504a 100644 --- a/project.clj +++ b/project.clj @@ -70,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 From c8eaa955c3852229161254593e118da8b0be6a6d Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 23 Apr 2019 07:50:50 -0400 Subject: [PATCH 06/11] Revert fast params in a Trie --- modules/reitit-core/java-src/reitit/Trie.java | 42 +++++++------------ 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/modules/reitit-core/java-src/reitit/Trie.java b/modules/reitit-core/java-src/reitit/Trie.java index ff5eb5f5..b7af4c1f 100644 --- a/modules/reitit-core/java-src/reitit/Trie.java +++ b/modules/reitit-core/java-src/reitit/Trie.java @@ -38,35 +38,20 @@ public class Trie { return decode(new String(chars, begin, end - begin), hasPercent, hasPlus); } - public static final class Match { - final private Object[] params; + public static class Match { + public IPersistentMap params; public final Object data; - private int i = 0; - public Match(Integer size, Object data) { - this.params = new Object[size]; + public Match(IPersistentMap params, Object data) { + this.params = params; this.data = data; } - public void assoc(Keyword key, Object value) { - params[i] = key; - params[i + 1] = value; - i += 2; - } - - public IPersistentMap params() { - return new PersistentArrayMap(params); - } - - Match copy() { - return new Match(params.length, data); - } - @Override public String toString() { Map m = new HashMap<>(); m.put(Keyword.intern("data"), data); - m.put(Keyword.intern("params"), params()); + m.put(Keyword.intern("params"), params); return m.toString(); } } @@ -131,13 +116,13 @@ public class Trie { private final Match match; DataMatcher(IPersistentMap params, Object data) { - this.match = new Match(params.count() * 2, data); + this.match = new Match(params, data); } @Override public Match match(int i, int max, char[] path) { if (i == max) { - return match.copy(); + return match; } return null; } @@ -190,7 +175,7 @@ public class Trie { } final Match m = child.match(stop, max, path); if (m != null) { - m.assoc(key, decode(new String(path, i, stop - i), hasPercent, hasPlus)); + m.params = m.params.assoc(key, decode(new String(path, i, stop - i), hasPercent, hasPlus)); } return m; } @@ -219,18 +204,19 @@ public class Trie { static final class CatchAllMatcher implements Matcher { private final Keyword parameter; - private final Match match; + private final IPersistentMap params; + private final Object data; CatchAllMatcher(Keyword parameter, IPersistentMap params, Object data) { - this.match = new Match(params.count() * 2, data); this.parameter = parameter; + this.params = params; + this.data = data; } @Override public Match match(int i, int max, char[] path) { if (i <= max) { - match.copy().assoc(parameter, decode(path, i, max)); - return match; + return new Match(params.assoc(parameter, decode(path, i, max)), data); } return null; } @@ -247,7 +233,7 @@ public class Trie { @Override public String toString() { - return "[" + parameter + " " + new DataMatcher(null, match.data) + "]"; + return "[" + parameter + " " + new DataMatcher(null, data) + "]"; } } From c3de6ff3ddc6a62d139a31a089ab1423876bbdc0 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 28 Apr 2019 17:36:43 +0300 Subject: [PATCH 07/11] Fix tests --- modules/reitit-http/src/reitit/http.cljc | 2 +- modules/reitit-ring/src/reitit/ring.cljc | 2 +- modules/reitit-ring/src/reitit/ring/spec.cljc | 8 +------- test/clj/reitit/http_test.clj | 4 ++-- test/cljc/reitit/ring_spec_test.cljc | 10 +++++----- test/cljc/reitit/ring_test.cljc | 4 ++-- 6 files changed, 12 insertions(+), 18 deletions(-) diff --git a/modules/reitit-http/src/reitit/http.cljc b/modules/reitit-http/src/reitit/http.cljc index 0f935966..e4a9a6ca 100644 --- a/modules/reitit-http/src/reitit/http.cljc +++ b/modules/reitit-http/src/reitit/http.cljc @@ -16,7 +16,7 @@ (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)) default-options-handler) + (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)) diff --git a/modules/reitit-ring/src/reitit/ring.cljc b/modules/reitit-ring/src/reitit/ring.cljc index 104e6a7d..0915151a 100644 --- a/modules/reitit-ring/src/reitit/ring.cljc +++ b/modules/reitit-ring/src/reitit/ring.cljc @@ -31,7 +31,7 @@ (defn compile-result [[path data] {:keys [::default-options-handler] :as opts}] (let [[top childs] (group-keys data) childs (cond-> childs - (and (not (:options childs)) default-options-handler) + (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) diff --git a/modules/reitit-ring/src/reitit/ring/spec.cljc b/modules/reitit-ring/src/reitit/ring/spec.cljc index 487a2211..e1af34c3 100644 --- a/modules/reitit-ring/src/reitit/ring/spec.cljc +++ b/modules/reitit-ring/src/reitit/ring/spec.cljc @@ -20,14 +20,8 @@ (s/def ::patch map?) -(s/def ::endpoint - (s/keys :req-un [::rs/handler] - :opt-un [::rs/name ::rs/no-doc ::middleware])) - (s/def ::data - (s/merge - ::endpoint - (s/map-of #{:get :head :post :put :delete :connect :options :trace :patch} map?))) + (s/keys :opt-un [::rs/handler ::rs/name ::rs/no-doc ::middleware])) ;; ;; Validator 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))) From a9bdceeeb6703af8a1ba5c0ed56da6e155840eaf Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 29 Apr 2019 00:37:28 +0300 Subject: [PATCH 08/11] reitit.spec/wrap spec-tools.spec/closed --- examples/http-swagger/src/example/server.clj | 6 +++--- modules/reitit-core/src/reitit/spec.cljc | 8 ++++---- modules/reitit-dev/src/reitit/dev/pretty.cljc | 3 --- modules/reitit-http/src/reitit/http/spec.cljc | 4 ++-- modules/reitit-ring/src/reitit/ring/spec.cljc | 8 ++++---- project.clj | 2 +- 6 files changed, 14 insertions(+), 17 deletions(-) diff --git a/examples/http-swagger/src/example/server.clj b/examples/http-swagger/src/example/server.clj index d7083a88..2de6d8fc 100644 --- a/examples/http-swagger/src/example/server.clj +++ b/examples/http-swagger/src/example/server.clj @@ -112,9 +112,9 @@ {:status 200 :body {:total (- x y)}})}}]]] - {;;:reitit.interceptor/transform dev/print-context-diffs - :wrap-spec spell/closed-keys - :validate spec/validate + {;:reitit.interceptor/transform dev/print-context-diffs ;; pretty context diffs + :validate spec/validate ;; enable spec validation for route data + :reitit.spec/wrap spell/closed ;; strict top-level validation (alpha) :exception pretty/exception :data {:coercion spec-coercion/coercion :muuntaja m/instance diff --git a/modules/reitit-core/src/reitit/spec.cljc b/modules/reitit-core/src/reitit/spec.cljc index 11081f87..18a26b72 100644 --- a/modules/reitit-core/src/reitit/spec.cljc +++ b/modules/reitit-core/src/reitit/spec.cljc @@ -116,14 +116,14 @@ (defrecord Problem [path scope data spec problems]) -(defn validate-route-data [routes wrap-spec spec] +(defn validate-route-data [routes wrap spec] (some->> (for [[p d _] routes] - (when-let [problems (and spec (s/explain-data (wrap-spec 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 wrap-spec] :or {spec ::default-data, wrap-spec identity}}] - (when-let [problems (validate-route-data routes wrap-spec 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/src/reitit/dev/pretty.cljc b/modules/reitit-dev/src/reitit/dev/pretty.cljc index 5b7d7d15..fa631723 100644 --- a/modules/reitit-dev/src/reitit/dev/pretty.cljc +++ b/modules/reitit-dev/src/reitit/dev/pretty.cljc @@ -230,9 +230,6 @@ :cljs "unknown")] (ex-info (exception-str message source (printer)) (assoc (or data {}) ::exception/cause e)))) -;; FIXME -(def closed-keys spec-tools.spell/closed-keys) - (defn de-expound-colors [^String s mappings] (let [s' (reduce (fn [s [from to]] diff --git a/modules/reitit-http/src/reitit/http/spec.cljc b/modules/reitit-http/src/reitit/http/spec.cljc index 9c54685e..f665c250 100644 --- a/modules/reitit-http/src/reitit/http/spec.cljc +++ b/modules/reitit-http/src/reitit/http/spec.cljc @@ -19,8 +19,8 @@ ;; (defn validate - [routes {:keys [spec wrap-spec] :or {spec ::data, wrap-spec identity}}] - (when-let [problems (rrs/validate-route-data routes :interceptors wrap-spec spec)] + [routes {:keys [spec ::rs/wrap] :or {spec ::data, wrap identity}}] + (when-let [problems (rrs/validate-route-data routes :interceptors wrap spec)] (exception/fail! ::rs/invalid-route-data {:problems problems}))) diff --git a/modules/reitit-ring/src/reitit/ring/spec.cljc b/modules/reitit-ring/src/reitit/ring/spec.cljc index e1af34c3..6cfaa299 100644 --- a/modules/reitit-ring/src/reitit/ring/spec.cljc +++ b/modules/reitit-ring/src/reitit/ring/spec.cljc @@ -35,21 +35,21 @@ :invalid non-specs})) (s/merge-spec-impl (vec specs) (vec specs) nil)) -(defn validate-route-data [routes key wrap-spec 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 (wrap-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 wrap-spec] :or {spec ::data, wrap-spec identity}}] - (when-let [problems (validate-route-data routes :middleware wrap-spec spec)] + [routes {:keys [spec ::rs/wrap] :or {spec ::data, wrap identity}}] + (when-let [problems (validate-route-data routes :middleware wrap spec)] (exception/fail! ::rs/invalid-route-data {:problems problems}))) diff --git a/project.clj b/project.clj index 8b76504a..e2e5bd3b 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-SNAPSHOT"] [metosin/schema-tools "0.11.0"] [metosin/muuntaja "0.6.4"] [metosin/jsonista "0.2.2"] From 791e79134e20531c419172fdf35468199c852eb0 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 1 May 2019 22:32:26 +0300 Subject: [PATCH 09/11] Update spec-tools --- CHANGELOG.md | 2 +- project.clj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8e9a034..15bc475c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ We use [Break Versioning][breakver]. The version numbers follow a `. Date: Wed, 1 May 2019 22:51:28 +0300 Subject: [PATCH 10/11] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15bc475c..fcbab7a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ We use [Break Versioning][breakver]. The version numbers follow a `. Date: Fri, 10 May 2019 08:30:44 +0300 Subject: [PATCH 11/11] Request perf test --- perf-test/clj/reitit/request_perf.cljc | 96 ++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 perf-test/clj/reitit/request_perf.cljc 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!))