Use coercion to encode query-string values in match->path

modules/reitit-core/src/reitit/coercion.cljc

@@ -19,7 +19,8 @@
   (-open-model [this model] "Returns a new model which allows extra keys in maps")
   (-encode-error [this error] "Converts error in to a serializable format")
   (-request-coercer [this type model] "Returns a `value format => value` request coercion function")
-  (-response-coercer [this model] "Returns a `value format => value` response coercion function"))
+  (-response-coercer [this model] "Returns a `value format => value` response coercion function")
+  (-query-string-coercer [this model] "Returns a `value => value` query string coercion function"))
 
 #?(:clj
    (defmethod print-method ::coercion [coercion ^Writer w]

modules/reitit-core/src/reitit/core.cljc

@@ -1,7 +1,9 @@
 (ns reitit.core
   (:require [reitit.exception :as exception]
             [reitit.impl :as impl]
-            [reitit.trie :as trie]))
+            [reitit.trie :as trie]
+            ;; FIXME: Should avoid coercion require here?
+            [reitit.coercion :as coercion]))
 
 ;;
 ;; Expand
@@ -71,7 +73,22 @@
   ([match]
    (match->path match nil))
   ([match query-params]
-   (some-> match :path (cond-> (seq query-params) (str "?" (impl/query-string 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?
+                         (str "?" (let [schema (-> match :data :parameters :query
+                                                   ;; FIXME: Why?
+                                                   first)
+                                        coercion (-> match :data :coercion)
+                                        coercer (when (and schema coercion)
+                                                  (coercion/-query-string-coercer coercion schema))
+                                        query-params (or (when 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)))))))
 
 ;;
 ;; Different routers

modules/reitit-malli/src/reitit/coercion/malli.cljc

@@ -176,6 +176,8 @@
        (-request-coercer [_ type schema]
          (-coercer schema type transformers :decode opts))
        (-response-coercer [_ schema]
-         (-coercer schema :response transformers :encode opts))))))
+         (-coercer schema :response transformers :encode opts))
+       (-query-string-coercer [_ schema]
+         (-coercer schema :string transformers :encode opts))))))
 
 (def coercion (create default-options))

modules/reitit-schema/src/reitit/coercion/schema.cljc

@@ -101,6 +101,9 @@
             value))))
     (-response-coercer [this schema]
       (if (coerce-response? schema)
-        (coercion/-request-coercer this :response schema)))))
+        (coercion/-request-coercer this :response schema)))
+    (-query-string-coercer [this schema]
+      ;; TODO: Can this be implemented?
+      nil)))
 
 (def coercion (create default-options))

modules/reitit-spec/src/reitit/coercion/spec.cljc

@@ -148,6 +148,9 @@
             value))))
     (-response-coercer [this spec]
       (if (coerce-response? spec)
-        (coercion/-request-coercer this :response spec)))))
+        (coercion/-request-coercer this :response spec)))
+    (-query-string-coercer [this spec]
+      ;; TODO: Can this be implemented?
+      nil)))
 
 (def coercion (create default-options))

test/cljc/reitit/coercion_test.cljc

@@ -1,5 +1,8 @@
 (ns reitit.coercion-test
-  (:require [clojure.test :refer [deftest is testing]]
+  (:require [clojure.spec.alpha :as cs]
+            [clojure.string :as str]
+            [clojure.test :refer [deftest is testing]]
+            [malli.core :as m]
             [malli.experimental.lite :as l]
             [reitit.coercion :as coercion]
             [reitit.coercion.malli]
@@ -7,8 +10,8 @@
             [reitit.coercion.spec]
             [reitit.core :as r]
             [schema.core :as s]
-            [clojure.spec.alpha :as cs]
+            [spec-tools.data-spec :as ds]
-            [spec-tools.data-spec :as ds])
+            [malli.transform :as mt])
   #?(:clj
      (:import (clojure.lang ExceptionInfo))))
 
@@ -150,3 +153,37 @@
                 {:compile coercion/compile-request-coercers})]
     (is (= {:path {:user-id 123, :company "metosin"}}
            (:parameters (match-by-path-and-coerce! router "/metosin/users/123"))))))
+
+(deftest match->path-parameter-coercion-test
+  (testing "default handling for query-string collection"
+    (let [router (r/router ["/:a/:b" ::route])]
+      (is (= "/olipa/kerran?x=a&x=b"
+             (-> router
+                 (r/match-by-name! ::route {:a "olipa", :b "kerran"})
+                 (r/match->path {:x [:a :b]}))))))
+
+  (testing "custom encode/string for a collection"
+    (let [router (r/router ["/:a/:b"
+                            {:name ::route
+                             :coercion reitit.coercion.malli/coercion
+                             :parameters {:query [:map
+                                                  [:x
+                                                   [:vector
+                                                    {:encode/string (fn [xs]
+                                                                      (str/join "," (map name xs)))
+                                                     :decode/string (fn [s]
+                                                                      (if (string? s)
+                                                                        (mapv keyword (str/split s #","))
+                                                                        s))}
+                                                    :keyword]]]}}]
+                           {:compile coercion/compile-request-coercers})]
+      ;; NOTE: "," is urlencoded by the impl/query-string step, is that ok?
+      (is (= "/olipa/kerran?x=a%2Cb"
+             (-> router
+                 (r/match-by-name! ::route {:a "olipa", :b "kerran"})
+                 (r/match->path {:x [:a :b]}))))
+
+      (is (= {:query {:x [:a :b]}}
+             (-> (r/match-by-path router "/olipa/kerran")
+                 (assoc :query-params {:x "a,b"})
+                 (coercion/coerce!)))))))