Simplify Malli coercion for query-params to only encode

metosin · Jan 28, 2025 · 7e9116f · 7e9116f
1 parent 7ae118f
commit 7e9116f
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 38 deletions.
diff --git a/modules/reitit-malli/src/reitit/coercion/malli.cljc b/modules/reitit-malli/src/reitit/coercion/malli.cljc
@@ -76,6 +76,20 @@
                      (assoc error :transformed transformed))))
                 value))))))))
 
+(defn- -query-string-coercer
+  "Create coercer for query-parameters, always allows extra params and does
+  encoding using string-transformer."
+  [schema transfomers options]
+  (let [;; Always allow extra paramaters on query-parameters encoding
+        open-schema (mu/open-schema schema)
+        ;; Do not remove extra keys
+        string-transformer (-transformer string-transformer-provider (assoc options :strip-extra-keys false))
+        encoder (m/encoder open-schema options string-transformer)]
+    (fn [value format]
+      (if encoder
+        (encoder value)
+        value))))
+
 ;;
 ;; public api
 ;;
@@ -178,12 +192,6 @@
        (-response-coercer [_ schema]
          (-coercer schema :response transformers :encode opts))
        (-query-string-coercer [_ schema]
-         ;; TODO: Create encoding function that only does encode, no decoding and validation?
-         (-coercer (mu/open-schema schema)
-                   :string
-                   ;; Tune transformer to not strip extra keys
-                   {:string {:default (-transformer string-transformer-provider (assoc opts :strip-extra-keys false))}}
-                   :encode
-                   opts))))))
+         (-query-string-coercer schema transformers opts))))))
 
 (def coercion (create default-options))
diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc
@@ -178,9 +178,7 @@
                                                     {:encode/string (fn [xs]
                                                                       (str/join "," (map name xs)))
                                                      :decode/string (fn [s]
-                                                                      (if (string? s)
-                                                                        (mapv keyword (str/split s #","))
-                                                                        s))}
+                                                                      (mapv keyword (str/split s #",")))}
                                                     :keyword]]]}}]
                            {:compile coercion/compile-request-coercers})
           match (r/match-by-name! router ::route {:a "olipa", :b "kerran"})]
@@ -211,12 +209,8 @@
                                                   [:x
                                                    [:vector
                                                     [:keyword
-                                                     {:decode/string (fn [s]
-                                                                       ;; Encoding coercer calls decoder -> validate -> encoder
-                                                                       ;; Decoder doesn't need to do anything as in this case the value is already "decoded"
-                                                                       (if (string? s)
-                                                                         (keyword (subs s 2))
-                                                                         s))
+                                                     ;; For query strings encode only calls encode, so no need to check if decode if value is encoded or not.
+                                                     {:decode/string (fn [s] (keyword (subs s 2)))
                                                       :encode/string (fn [k] (str "__" (name k)))}]]]]}}]
                            {:compile coercion/compile-request-coercers})]
       (is (= "/olipa/kerran?x=__a&x=__b"

diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs
@@ -308,12 +308,7 @@
   (is (= "foo?q=__x"
          (rf/match->path {:data {:coercion rcm/coercion
                                  :parameters {:query [[:map
-                                                       [:q {:decode/string (fn [s]
-                                                                             (if (string? s)
-                                                                               (keyword (if (str/starts-with? s "__")
-                                                                                          (subs s 2)
-                                                                                          s))
-                                                                               s))
+                                                       [:q {:decode/string (fn [s] (keyword (subs s 2)))
                                                             :encode/string (fn [k] (str "__" (name k)))}
                                                         :keyword]]]}}
                           :path "foo"}

diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs
@@ -1,6 +1,5 @@
 (ns reitit.frontend.easy-test
-  (:require [clojure.string :as str]
-            [clojure.test :refer [are async deftest is testing]]
+  (:require [clojure.test :refer [are async deftest is testing]]
             [goog.events :as gevents]
             [reitit.coercion.malli :as rcm]
             [reitit.core :as r]
@@ -18,12 +17,7 @@
                          :parameters {:query [:map
                                               [:q {:optional true}
                                                [:keyword
-                                                {:decode/string (fn [s]
-                                                                  (if (string? s)
-                                                                    (keyword (if (str/starts-with? s "__")
-                                                                               (subs s 2)
-                                                                               s))
-                                                                    s))
+                                                {:decode/string (fn [s] (keyword (subs s 2)))
                                                  :encode/string (fn [k] (str "__" (name k)))}]]]}}]]))
 
 ;; TODO: Only tests fragment history, also test HTML5?

diff --git a/test/cljs/reitit/frontend/history_test.cljs b/test/cljs/reitit/frontend/history_test.cljs
@@ -4,8 +4,7 @@
             [reitit.frontend.history :as rfh]
             [reitit.frontend.test-utils :refer [capture-console]]
             [goog.events :as gevents]
-            [reitit.coercion.malli :as rcm]
-            [clojure.string :as str]))
+            [reitit.coercion.malli :as rcm]))
 
 (def browser (exists? js/window))
 
@@ -18,12 +17,7 @@
                          :parameters {:query [:map
                                               [:q {:optional true}
                                                [:keyword
-                                                {:decode/string (fn [s]
-                                                                  (if (string? s)
-                                                                    (keyword (if (str/starts-with? s "__")
-                                                                               (subs s 2)
-                                                                               s))
-                                                                    s))
+                                                {:decode/string (fn [s] (keyword (subs s 2)))
                                                  :encode/string (fn [k] (str "__" (name k)))}]]]}}]]))
 
 (deftest fragment-history-test