Repository: oliyh/martian
Pull request #219 (2025-05-06 14:19:49)

Fix how default values interplay with map schemas coercion
This fixes up the issue with [Missing-key / disallowed key on path-params when setting defaults in query-schema #215](https://github.com/oliyh/martian/issues/215).

```clojure
(require
  '[martian.core :as martian]
  '[spec-tools.core :as st])

(def m (martian/bootstrap
         "http://example.com"
         [{:query-schema {:version (st/default s/Int 70)},
           :path-schema  {:id s/Str}
           :route-name   :test/route
           :method       :get,
           :path-parts   ["/part/" :id]}]
         {:use-defaults? true}))
=> #'user/m

(martian/request-for m :test/route {:id "honestly-i-am-a-string"})
=> {:method :get, :url "http://example.com/part/honestly-i-am-a-string", :query-params {:version 70}}

(martian/request-for m :test/route {})
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:id missing-required-key}
```

All existing tests pass locally (Clojure). I've also added a new one for this issue, `martian.core-test/use-defaults-test`. It goes with all 4/4 cases that were mentioned in the issue, i.e. it provides a complete coverage:

```clojure
(require
  '[martian.schema :as schema]
  '[spec-tools.core :as st])

(schema/coerce-data
  {:version (st/default s/Int 70)}
  {}
  nil true)
=> {:version 70}

(schema/coerce-data
  {:version (st/default s/Int 70)}
  {:version 100}
  nil true)
=> {:version 100}

;; This produces "Value cannot be coerced to match schema: {:id disallowed-key}" w/o the fix.
(schema/coerce-data
  {:version (st/default s/Int 70)}
  {:id "id"}
  nil true)
=> {:version 70}

;; This produces "Value cannot be coerced to match schema: {:id disallowed-key}" w/o the fix.
(schema/coerce-data
  {:version (st/default s/Int 70)}
  {:id "id" :version 100}
  nil true)
=> {:version 100}
```

marksto: @oliyh Hi Oliver! The fix is ready for your review. It would be nice to incorporate this into the next Martian release.

Patch:
diff --git a/core/src/martian/schema.cljc b/core/src/martian/schema.cljc
index d59d605..ccadcb2 100644
--- a/core/src/martian/schema.cljc
+++ b/core/src/martian/schema.cljc
@@ -42,6 +42,18 @@
           (coercion-matchers schema)))
     coercion-matchers))
 
+(defn ->map-matcher
+  "Builds a version of `stc/map-filter-matcher` that is optional and takes
+   into account custom `coercion-matchers` that may/not happen afterwards."
+  [coercion-matchers]
+  (fn [schema]
+    (let [f (stc/map-filter-matcher schema)]
+      (fn [x]
+        (let [x' (if (some? f) (f x) x)]
+          (if-some [g (coercion-matchers schema)]
+            (g x')
+            x'))))))
+
 (defn- from-maybe [s]
   (if (instance? Maybe s)
     (:schema s)
@@ -53,12 +65,12 @@
   (let [coercion-matchers (build-coercion-matchers use-defaults?)]
     (when-let [s (from-maybe schema)]
       (cond
-        (or (coercion-matchers schema)
-            (instance? AnythingSchema s))
+        (instance? AnythingSchema s)
         ((sc/coercer! schema coercion-matchers) data)
 
         (map? s)
-        (stc/coerce (unalias-data parameter-aliases data) s (stc/forwarding-matcher coercion-matchers stc/map-filter-matcher))
+        (let [map-matcher (->map-matcher coercion-matchers)]
+          (stc/coerce (unalias-data parameter-aliases data) s map-matcher))
 
         (coll? s) ;; primitives, arrays, arrays of maps
         ((sc/coercer! schema coercion-matchers)


Test patch:
diff --git a/core/test/martian/core_test.cljc b/core/test/martian/core_test.cljc
index f653094..80aed4c 100644
--- a/core/test/martian/core_test.cljc
+++ b/core/test/martian/core_test.cljc
@@ -1,6 +1,7 @@
 (ns martian.core-test
   (:require [martian.core :as martian]
             [schema.core :as s]
+            [schema-tools.core :as st]
             [clojure.spec.test.alpha :as stest]
             #?(:clj [clojure.test :refer [deftest testing is]]
                :cljs [cljs.test :refer-macros [deftest testing is]]))
@@ -318,6 +319,61 @@
       (catch Throwable e
         (is (= :missing-route (-> e ex-data :route-name)))))))
 
+(deftest use-defaults-test
+  (testing "in isolation"
+    (let [m (martian/bootstrap
+              "http://example.com"
+              [{:query-schema {:version (st/default s/Int 70)}
+                :route-name   :test-route
+                :method       :get
+                :path-parts   ["/some"]}]
+              {:use-defaults? true})]
+
+      (testing "coerces data using default values"
+
+        (testing "adds missing values when there are defaults"
+          (is (= {:method       :get
+                  :url          "http://example.com/some"
+                  :query-params {:version 70}}
+                 (martian/request-for m :test-route {}))))
+
+        (testing "does nothing if values are present"
+          (is (= {:method       :get
+                  :url          "http://example.com/some"
+                  :query-params {:version 100}}
+                 (martian/request-for m :test-route {:version 100})))))))
+
+  (testing "interplay with other params"
+    (let [m (martian/bootstrap
+              "http://example.com"
+              [{:query-schema {:version (st/default s/Int 70)}
+                :path-schema  {:id s/Str}
+                :route-name   :test-route
+                :method       :get
+                :path-parts   ["/some/" :id]}]
+              {:use-defaults? true})]
+
+      (is (thrown-with-msg? Throwable cannot-coerce-pattern
+                            (martian/request-for m :test-route {}))
+          "Could not coerce value to schema: {:id missing-required-key}")
+
+      (testing "coerces data using default values"
+        ;; NOTE: There must be no error of the following kind (after #215 fix):
+        ;;       Value cannot be coerced to match schema: {:id disallowed-key}
+
+        (testing "adds missing values when there are defaults"
+          (is (= {:method       :get
+                  :url          "http://example.com/some/id"
+                  :query-params {:version 70}}
+                 (martian/request-for m :test-route {:id "id"}))))
+
+        (testing "does nothing if values are present"
+          (is (= {:method       :get
+                  :url          "http://example.com/some/id"
+                  :query-params {:version 100}}
+                 (martian/request-for m :test-route {:id      "id"
+                                                     :version 100}))))))))
+
 (deftest kebab-mapping-test
   (let [m (martian/bootstrap "https://camels.org"
                              [{:route-name :create-camel
diff --git a/core/test/martian/schema_test.cljc b/core/test/martian/schema_test.cljc
index 71c09c9..5871b93 100644
--- a/core/test/martian/schema_test.cljc
+++ b/core/test/martian/schema_test.cljc
@@ -2,7 +2,6 @@
   (:require [martian.schema :as schema]
             [matcher-combinators.test]
             [schema.core :as s]
-            [schema.coerce :as sc]
             [schema-tools.core :as st]
             #?(:clj [clojure.test :refer [deftest testing is]]
                :cljs [cljs.test :refer-macros [deftest testing is]]))
@@ -176,6 +175,7 @@
                                           :tags [{:k nil}]}
                                          nil
                                          true)))))))))
+
 ;; "int-or-string" (s/cond-pre s/Str s/Int)
 (deftest int-or-string-test
   (is (= (s/cond-pre s/Str s/Int)