Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add reitit-frontend support for fragment string #581

Merged
merged 2 commits into from
Feb 21, 2023
Merged

Add reitit-frontend support for fragment string #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83c31e3
Revert "Revert "Merge pull request #554 from just-sultanov/add-suppor…
Deraen Jan 18, 2023
2494f70
Read fragment string without decoding
Deraen Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion modules/reitit-core/src/reitit/coercion.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
:body (->ParameterCoercion :body-params :body false false)
:form (->ParameterCoercion :form-params :string true true)
:header (->ParameterCoercion :headers :string true true)
:path (->ParameterCoercion :path-params :string true true)})
:path (->ParameterCoercion :path-params :string true true)
:fragment (->ParameterCoercion :fragment :string true true)})

(defn ^:no-doc request-coercion-failed! [result coercion value in request serialize-failed-result]
(throw
Expand Down
9 changes: 7 additions & 2 deletions modules/reitit-frontend/src/reitit/frontend.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,17 @@
coercion/coerce!)]
(if-let [match (r/match-by-path router (.getPath uri))]
(let [q (query-params uri)
match (assoc match :query-params q)
fragment (when (.hasFragment uri)
ikitommi marked this conversation as resolved.
Show resolved Hide resolved
(.getFragment uri))
match (assoc match
:query-params q
:fragment fragment)
;; Return uncoerced values if coercion is not enabled - so
;; that tha parameters are always accessible from same property.
parameters (or (coerce! match)
{:path (:path-params match)
:query q})]
:query q
:fragment fragment})]
(assoc match :parameters parameters))))))

(defn match-by-name
Expand Down
122 changes: 111 additions & 11 deletions test/cljs/reitit/frontend/core_test.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,23 @@
[reitit.frontend :as rf]
[reitit.coercion :as rc]
[schema.core :as s]
[reitit.coercion.schema :as rsc]
[reitit.coercion.schema :as rcs]
[reitit.coercion.malli :as rcm]
[reitit.frontend.test-utils :refer [capture-console]]))

(defn m [x]
(assoc x :data nil :result nil))

(defn decode-form [s]
;; RFC 6749 4.2.2 specifies OAuth token response uses
;; form-urlencoded format to encode values in the fragment string.
;; Use built-in JS function to decode.
;; ring.util.codec/decode-form works on Clj.
(when s
(->> (.entries (js/URLSearchParams. s))
(map (fn [[k v]] [(keyword k) v]))
(into {}))))

(deftest match-by-path-test
(testing "simple"
(let [router (r/router ["/"
Expand All @@ -22,8 +33,10 @@
:path-params {}
:query-params {}
:path "/"
:fragment nil
:parameters {:query {}
:path {}}})
:path {}
:fragment nil}})
ikitommi marked this conversation as resolved.
Show resolved Hide resolved
(rf/match-by-path router "/")))

(is (= "/"
Expand All @@ -35,8 +48,10 @@
:path-params {}
:query-params {}
:path "/foo"
:fragment nil
:parameters {:query {}
:path {}}})
:path {}
:fragment nil}})
(rf/match-by-path router "/foo")))

(is (= (r/map->Match
Expand All @@ -45,8 +60,10 @@
:path-params {}
:query-params {:mode ["foo", "bar"]}
:path "/foo"
:fragment nil
:parameters {:query {:mode ["foo", "bar"]}
:path {}}})
:path {}
:fragment nil}})
(rf/match-by-path router "/foo?mode=foo&mode=bar")))

(is (= "/foo"
Expand All @@ -64,17 +81,20 @@
(let [router (r/router ["/"
[":id" {:name ::foo
:parameters {:path {:id s/Int}
:query {(s/optional-key :mode) s/Keyword}}}]]
:query {(s/optional-key :mode) s/Keyword}
:fragment (s/maybe s/Str)}}]]
{:compile rc/compile-request-coercers
:data {:coercion rsc/coercion}})]
:data {:coercion rcs/coercion}})]

(is (= (r/map->Match
{:template "/:id"
:path-params {:id "5"}
:query-params {}
:path "/5"
:fragment nil
:parameters {:query {}
:path {:id 5}}})
:path {:id 5}
:fragment nil}})
(m (rf/match-by-path router "/5"))))

(is (= "/5"
Expand All @@ -99,21 +119,25 @@
:path-params {:id "5"}
:query-params {:mode "foo"}
:path "/5"
:fragment nil
:parameters {:path {:id 5}
:query {:mode :foo}}})
:query {:mode :foo}
:fragment nil}})
(m (rf/match-by-path router "/5?mode=foo"))))

(is (= "/5?mode=foo"
(r/match->path (rf/match-by-name router ::foo {:id 5}) {:mode :foo}))))

(testing "fragment is ignored"
(testing "fragment string is read"
(is (= (r/map->Match
{:template "/:id"
:path-params {:id "5"}
:query-params {:mode "foo"}
:path "/5"
:fragment "fragment"
:parameters {:path {:id 5}
:query {:mode :foo}}})
:query {:mode :foo}
:fragment "fragment"}})
ikitommi marked this conversation as resolved.
Show resolved Hide resolved
(m (rf/match-by-path router "/5?mode=foo#fragment")))))

(testing "console warning about missing params"
Expand All @@ -126,4 +150,80 @@
(:messages
(capture-console
(fn []
(rf/match-by-name! router ::foo {}))))))))))
(rf/match-by-name! router ::foo {})))))))))

(testing "malli coercion"
(let [router (r/router ["/"
[":id" {:name ::foo
:parameters {:path [:map
[:id :int]]
:query [:map
[:mode {:optional true} :keyword]]
:fragment [:maybe
[:map
{:decode/string decode-form}
[:access_token :string]
[:refresh_token :string]
[:expires_in :int]
[:provider_token :string]
[:token_type :string]]]}}]]
{:compile rc/compile-request-coercers
:data {:coercion rcm/coercion}})]

(is (= (r/map->Match
{:template "/:id"
:path-params {:id "5"}
:query-params {}
:path "/5"
:fragment nil
:parameters {:query {}
:path {:id 5}
:fragment nil}})
(m (rf/match-by-path router "/5"))))

(is (= "/5"
(r/match->path (rf/match-by-name router ::foo {:id 5}))))

(testing "coercion error"
(testing "throws without options"
(is (thrown? js/Error (m (rf/match-by-path router "/a")))))

(testing "thows and calles on-coercion-error"
(let [exception (atom nil)
match (atom nil)]
(is (thrown? js/Error (m (rf/match-by-path router "/a" {:on-coercion-error (fn [m e]
(reset! match m)
(reset! exception e))}))))
(is (= {:id "a"} (-> @match :path-params)))
(is (= {:id "a"} (-> @exception (ex-data) :value))))))

(testing "query param is read"
(is (= (r/map->Match
{:template "/:id"
:path-params {:id "5"}
:query-params {:mode "foo"}
:path "/5"
:fragment nil
:parameters {:path {:id 5}
:query {:mode :foo}
:fragment nil}})
(m (rf/match-by-path router "/5?mode=foo"))))

(is (= "/5?mode=foo"
(r/match->path (rf/match-by-name router ::foo {:id 5}) {:mode :foo}))))

(testing "fragment string is read"
(is (= (r/map->Match
{:template "/:id"
:path-params {:id "5"}
:query-params {:mode "foo"}
:path "/5"
:fragment "access_token=foo&refresh_token=bar&provider_token=baz&token_type=bearer&expires_in=3600"
:parameters {:path {:id 5}
:query {:mode :foo}
:fragment {:access_token "foo"
:refresh_token "bar"
:provider_token "baz"
:token_type "bearer"
:expires_in 3600}}})
(m (rf/match-by-path router "/5?mode=foo#access_token=foo&refresh_token=bar&provider_token=baz&token_type=bearer&expires_in=3600"))))))))