From a28bb2e9b6fcedfd3c45d1c2caa6d9c4029d901c Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Thu, 10 Aug 2017 09:43:50 +0300 Subject: [PATCH] Lot's of small improvements * implement by-name (fixes #5) * match-route => match * implement routes * by-name & match return Match-records (more info, faster to use) * reitit.regex => reitit.impl --- perf-test/clj/reitit/perf_test.clj | 3 +- src/reitit/core.cljc | 72 +++++++++++++++++++++------- src/reitit/{regex.cljc => impl.cljc} | 35 +++++++++++--- test/cljc/reitit/core_test.cljc | 57 ++++++++++++++++------ 4 files changed, 128 insertions(+), 39 deletions(-) rename src/reitit/{regex.cljc => impl.cljc} (76%) diff --git a/perf-test/clj/reitit/perf_test.clj b/perf-test/clj/reitit/perf_test.clj index b49c9a48..12abe959 100644 --- a/perf-test/clj/reitit/perf_test.clj +++ b/perf-test/clj/reitit/perf_test.clj @@ -96,8 +96,9 @@ (call))) ;; 1.0µs (-94%) + ;; 770ns (-95%, -23%) (title "reitit") - (let [call #(reitit/match-route reitit-routes "/workspace/1/1")] + (let [call #(reitit/match reitit-routes "/workspace/1/1")] (assert (call)) (cc/quick-bench (call)))) diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index dd03a71f..1be4a18e 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -1,6 +1,8 @@ (ns reitit.core (:require [meta-merge.core :refer [meta-merge]] - [reitit.regex :as regex])) + [reitit.impl :as impl #?@(:cljs [:refer [Route]])]) + #?(:clj + (:import (reitit.impl Route)))) (defprotocol Expand (expand [this])) @@ -57,31 +59,65 @@ (->> (walk data opts) (map-meta merge-meta))) (defprotocol Routing - (match-route [this path]) - (path-for [this name] [this name parameters])) + (routes [this]) + (match [this path]) + (by-name [this name] [this name parameters])) -(defrecord LinearRouter [routes] +(defrecord Match [template meta path params]) + +(defrecord LinearRouter [routes data lookup] Routing - (match-route [_ path] + (routes [_] + routes) + (match [_ path] (reduce - (fn [acc [p m matcher]] - (if-let [params (matcher path)] - (reduced (assoc m :route-params params)))) - nil routes))) + (fn [acc ^Route route] + (if-let [params ((:matcher route) path)] + (reduced (->Match (:path route) (:meta route) path params)))) + nil data)) + (by-name [_ name] + ((lookup name) nil)) + (by-name [_ name params] + ((lookup name) params))) -(defrecord LookupRouter [routes] +(defn linear-router [routes] + (->LinearRouter + routes + (mapv (partial apply impl/create) routes) + (->> (for [[p {:keys [name] :as meta}] routes + :when name + :let [route (impl/create p meta)]] + [name (fn [params] + (->Match p meta (impl/path-for route params) params))]) + (into {})))) + +(defrecord LookupRouter [routes data lookup] Routing - (match-route [_ path] - (routes path))) + (routes [_] + routes) + (match [_ path] + (data path)) + (by-name [_ name] + ((lookup name) nil)) + (by-name [_ name params] + ((lookup name) params))) + +(defn lookup-router [routes] + (->LookupRouter + routes + (->> (for [[p meta] routes] + [p (->Match p meta p {})]) + (into {})) + (->> (for [[p {:keys [name] :as meta}] routes + :when name] + [name (fn [params] + (->Match p meta p params))]) + (into {})))) (defn router ([data] (router data {})) ([data opts] (let [routes (resolve-routes data opts)] - (if (some regex/contains-wilds? (map first routes)) - (->LinearRouter - (for [[p m] routes] - [p m (regex/matcher p)])) - (->LookupRouter - (into {} routes)))))) + ((if (some impl/contains-wilds? (map first routes)) + linear-router lookup-router) routes)))) diff --git a/src/reitit/regex.cljc b/src/reitit/impl.cljc similarity index 76% rename from src/reitit/regex.cljc rename to src/reitit/impl.cljc index 77e11318..5114ae61 100644 --- a/src/reitit/regex.cljc +++ b/src/reitit/impl.cljc @@ -10,8 +10,9 @@ ; ; You must not remove this notice, or any other, from this software. -(ns reitit.regex - (:require [clojure.string :as str]) +(ns reitit.impl + (:require [clojure.string :as str] + [clojure.set :as set]) (:import #?(:clj (java.util.regex Pattern)))) ;; @@ -97,12 +98,34 @@ (wild? (first parts))))) ;; -;; Routing +;; Routing (c) Metosin ;; -(defn matcher [path] +(defrecord Route [path matcher parts params meta]) + +(defn create [path meta] (if (contains-wilds? path) (as-> (parse-path path) $ (assoc $ :path-re (path-regex $)) - (path-matcher $)) - #(if (= path %) {}))) + (merge $ {:path path + :matcher (path-matcher $) + :meta meta}) + (dissoc $ :path-re :path-constraints) + (update $ :path-params set) + (set/rename-keys $ {:path-parts :parts + :path-params :params}) + (map->Route $)) + (map->Route {:path path + :meta meta + :matcher #(if (= path %) {})}))) + +(defn path-for [^Route route params] + (when-not (every? #(contains? params %) (:params route)) + (let [defined (-> params keys set) + required (:params route) + missing (clojure.set/difference required defined)] + (throw + (ex-info + (str "missing path-params for route '" (:path route) "': " missing) + {:params params, :required required})))) + (str "/" (str/join \/ (map #(get params % %) (:parts route))))) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 6725fc09..e8ca8d02 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -1,25 +1,51 @@ (ns reitit.core-test (:require [clojure.test :refer [deftest testing is are]] - #?(:clj [reitit.core :as reitit] - :cljs [reitit.core :as reitit :refer [LinearRouter LookupRouter]])) + [reitit.core :as reitit #?@(:cljs [:refer [Match LinearRouter LookupRouter]])]) #?(:clj - (:import (reitit.core LinearRouter LookupRouter)))) + (:import (reitit.core Match LinearRouter LookupRouter) + (clojure.lang ExceptionInfo)))) (deftest reitit-test (testing "linear router" - (let [router (reitit/router ["/api" - ["/ipa" - ["/:size"]]])] + (let [router (reitit/router ["/api" ["/ipa" ["/:size" ::beer]]])] (is (instance? LinearRouter router)) - (is (map? (reitit/match-route router "/api/ipa/large"))))) + (is (= [["/api/ipa/:size" {:name ::beer}]] + (reitit/routes router))) + (is (= (reitit/map->Match + {:template "/api/ipa/:size" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {:size "large"}}) + (reitit/match router "/api/ipa/large"))) + (is (= (reitit/map->Match + {:template "/api/ipa/:size" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {:size "large"}}) + (reitit/by-name router ::beer {:size "large"}))) + (is (thrown-with-msg? + ExceptionInfo + #"^missing path-params for route '/api/ipa/:size': \#\{:size\}$" + (reitit/by-name router ::beer))))) (testing "lookup router" - (let [router (reitit/router ["/api" - ["/ipa" - ["/large"]]])] + (let [router (reitit/router ["/api" ["/ipa" ["/large" ::beer]]])] (is (instance? LookupRouter router)) - (is (map? (reitit/match-route router "/api/ipa/large"))))) + (is (= [["/api/ipa/large" {:name ::beer}]] + (reitit/routes router))) + (is (= (reitit/map->Match + {:template "/api/ipa/large" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {}}) + (reitit/match router "/api/ipa/large"))) + (is (= (reitit/map->Match + {:template "/api/ipa/large" + :meta {:name ::beer} + :path "/api/ipa/large" + :params {:size "large"}}) + (reitit/by-name router ::beer {:size "large"}))))) (testing "bide sample" (let [routes [["/auth/login" :auth/login] @@ -47,7 +73,10 @@ ["/api/admin/db" {:mw [:api :admin :db], :roles #{:admin}}]] router (reitit/router routes)] (is (= expected (reitit/resolve-routes routes {}))) - (is (= {:mw [:api], :parameters {:id String, :sub-id String} - :route-params {:id "1", :sub-id "2"}} - (reitit/match-route router "/api/user/1/2")))))) + (is (= (reitit/map->Match + {:template "/api/user/:id/:sub-id" + :meta {:mw [:api], :parameters {:id String, :sub-id String}} + :path "/api/user/1/2" + :params {:id "1", :sub-id "2"}}) + (reitit/match router "/api/user/1/2"))))))