Move query string coercion to coercion ns from core

This commit is contained in:
Juho Teperi 2025-01-28 14:34:21 +02:00
parent ce6d9e26cd
commit 7ae118fbb5
5 changed files with 80 additions and 59 deletions

View file

@ -1,5 +1,6 @@
(ns reitit.coercion
(:require [#?(:clj reitit.walk :cljs clojure.walk) :as walk]
[reitit.core :as r]
[reitit.impl :as impl])
#?(:clj
(:import (java.io Writer))))
@ -220,3 +221,33 @@
[match]
(if-let [coercers (-> match :result :coerce)]
(coerce-request coercers match)))
(defn coerce-query-params
"Uses an input schema and coercion implementation from the given match to
encode query-parameters map.
If no match, no input schema or coercion implementation, just returns the
original parameters map."
[match query-params]
(when query-params
(let [coercion (-> match :data :coercion)
schema (when coercion
(-compile-model coercion (-> match :data :parameters :query) nil))
coercer (when (and schema coercion)
(-query-string-coercer coercion schema))]
(if coercer
(let [result (coercer query-params :default)]
(if (error? result)
(throw (ex-info (str "Query parameters coercion failed")
result))
result))
query-params))))
(defn match->path
"Create routing path from given match and optional query-parameters map.
Query-parameters are encoded using the input schema and coercion implementation."
([match]
(r/match->path match))
([match query-params]
(r/match->path match (coerce-query-params match query-params))))

View file

@ -1,9 +1,7 @@
(ns reitit.core
(:require [reitit.exception :as exception]
[reitit.impl :as impl]
[reitit.trie :as trie]
;; FIXME: Should avoid coercion require here?
[reitit.coercion :as coercion]))
[reitit.trie :as trie]))
;;
;; Expand
@ -70,25 +68,12 @@
(:template match) (:required match) path-params)))))
(defn match->path
"Create routing path from given match and optional query-parameters map."
([match]
(match->path match nil))
([match query-params]
(some-> match :path (cond-> (seq query-params)
;; TODO: Should the coercion be applied elsewhere (FE ns?) so the core ns doesn't depend
;; on the coercion?
;; NOTE: Re-creates coercer on every call, could this be pre-compiled somewhere
;; or memoized? Does it matter much?
;; TODO: query-coercer could be compiled in reitit.frontend/router, same as request coercers.
(str "?" (let [coercion (-> match :data :coercion)
schema (when coercion
(coercion/-compile-model coercion (-> match :data :parameters :query) nil))
coercer (when (and schema coercion)
(coercion/-query-string-coercer coercion schema))
query-params (if coercer
(coercer query-params :default)
query-params)]
;; Default encoding for values will handle values that aren't encoded using coercer
(impl/query-string query-params)))))))
(str "?" (impl/query-string query-params))))))
;;
;; Different routers

View file

@ -40,14 +40,16 @@
(defn
^{:see-also ["reitit.core/match->path"]}
match->path
"Create routing path from given match and optional query-string map and
optional fragment string."
"Create routing path from given match and optional query-parameters map and
optional fragment string.
Query-parameters are encoded using the input schema and coercion implementation."
([match]
(match->path match nil nil))
([match query-params]
(match->path match query-params nil))
([match query-params fragment]
(when-let [path (r/match->path match query-params)]
(when-let [path (coercion/match->path match query-params)]
(cond-> path
(and fragment (seq fragment)) (str "#" (impl/form-encode fragment))))))

View file

@ -160,13 +160,13 @@
(is (= "/olipa/kerran?x=a&x=b"
(-> router
(r/match-by-name! ::route {:a "olipa", :b "kerran"})
(r/match->path {:x [:a :b]}))))
(coercion/match->path {:x [:a :b]}))))
(is (= "/olipa/kerran?x=a&x=b&extra=extra-param"
(-> router
(r/match-by-name! ::route {:a "olipa", :b "kerran"})
(r/match->path {:x [:a :b]
:extra "extra-param"}))))))
(coercion/match->path {:x [:a :b]
:extra "extra-param"}))))))
(testing "custom encode/string for a collection"
(let [router (r/router ["/:a/:b"
@ -182,18 +182,20 @@
(mapv keyword (str/split s #","))
s))}
:keyword]]]}}]
{:compile coercion/compile-request-coercers})]
;; NOTE: "," is urlencoded by the impl/query-string step, is that ok?
{:compile coercion/compile-request-coercers})
match (r/match-by-name! router ::route {:a "olipa", :b "kerran"})]
(is (= {:x "a,b"}
(coercion/coerce-query-params match {:x [:a :b]})))
;; NOTE: "," is urlencoded by the impl/query-string step
(is (= "/olipa/kerran?x=a%2Cb"
(-> router
(r/match-by-name! ::route {:a "olipa", :b "kerran"})
(r/match->path {:x [:a :b]}))))
(coercion/match->path match {:x [:a :b]})))
(testing "extra query-string parameters aren't removed by coercion"
(is (= "/olipa/kerran?x=a%2Cb&extra=extra-param"
(-> router
(r/match-by-name! ::route {:a "olipa", :b "kerran"})
(r/match->path {:x [:a :b]
(coercion/match->path {:x [:a :b]
:extra "extra-param"})))))
(is (= {:query {:x [:a :b]}}
@ -220,7 +222,7 @@
(is (= "/olipa/kerran?x=__a&x=__b"
(-> router
(r/match-by-name! ::route {:a "olipa", :b "kerran"})
(r/match->path {:x [:a :b]}))))
(coercion/match->path {:x [:a :b]}))))
(is (= {:query {:x [:a :b]}}
(-> (r/match-by-path router "/olipa/kerran")

View file

@ -40,74 +40,75 @@
(fn on-navigate [match history]
(let [url (rfh/-get-path history)]
(case (swap! n inc)
1 (do (is (some? (:popstate-listener history)))
1 (rfh/push-state history ::frontpage)
2 (do (is (some? (:popstate-listener history)))
(is (= "/" url)
"start at root")
(rfe/push-state ::foo nil {:a 1} "foo bar"))
;; 0. /
;; 1. /foo?a=1#foo+bar
2 (do (is (= "/foo?a=1#foo+bar" url)
3 (do (is (= "/foo?a=1#foo+bar" url)
"push-state")
(.back js/window.history))
;; 0. /
3 (do (is (= "/" url)
4 (do (is (= "/" url)
"go back")
(rfe/navigate ::bar {:path-params {:id 1}
:query-params {:q "x"}}))
;; 0. /
;; 1. /bar/1
4 (do (is (= "/bar/1?q=__x" url)
5 (do (is (= "/bar/1?q=__x" url)
"push-state 2")
(rfe/replace-state ::bar {:id 2}))
;; 0. /
;; 1. /bar/2
5 (do (is (= "/bar/2" url)
6 (do (is (= "/bar/2" url)
"replace-state")
(rfe/set-query {:a 1}))
;; 0. /
;; 1. /bar/2
;; 2. /bar/2?a=1
6 (do (is (= "/bar/2?a=1" url)
7 (do (is (= "/bar/2?a=1" url)
"update-query with map")
(rfe/set-query #(assoc % :q "x") {:replace true}))
;; 0. /
;; 1. /bar/2
;; 2. /bar/2?a=1&b=foo
7 (do (is (= "/bar/2?a=1&q=__x" url)
8 (do (is (= "/bar/2?a=1&q=__x" url)
"update-query with fn")
(.go js/window.history -2))
;; Go to non-matching path and check set-query works
;; (without coercion) without a match
8 (do (is (= "/" url) "go back two events")
9 (do (is (= "/" url) "go back two events")
(.pushState js/window.history nil "" "#/non-matching-path"))
9 (do (is (= "/non-matching-path" url))
10 (do (is (= "/non-matching-path" url))
(rfe/set-query #(assoc % :q "x")))
10 (do (is (= "/non-matching-path?q=x" url))
11 (do (is (= "/non-matching-path?q=x" url))
(.go js/window.history -2))
;; 0. /
11 (do (is (= "/" url)
"go back two events")
12 (do (is (= "/" url)
"go back two events")
;; Reset to ensure old event listeners aren't called
(rfe/start! router
(fn on-navigate [match history]
(let [url (rfh/-get-path history)]
(case (swap! n inc)
12 (do (is (= "/" url)
"start at root")
(rfe/push-state ::foo))
13 (do (is (= "/foo" url)
"push-state")
(rfh/stop! @rfe/history)
(done))
(do
(is false (str "extra event 2" {:n @n, :url url}))
(done)))))
{:use-fragment true}))
;; Reset to ensure old event listeners aren't called
(rfe/start! router
(fn on-navigate [match history]
(let [url (rfh/-get-path history)]
(case (swap! n inc)
13 (do (is (= "/" url)
"start at root")
(rfe/push-state ::foo))
14 (do (is (= "/foo" url)
"push-state")
(rfh/stop! @rfe/history)
(done))
(do
(is false (str "extra event 2" {:n @n, :url url}))
(done)))))
{:use-fragment true}))
(do
(is false (str "extra event 1" {:n @n, :url url}))
(done)))))