Merge branch 'master' of github.com:metosin/compojure-api

frenchy64 · frenchy64 · commit b10a068f3e4e · 2024-06-17T10:39:00.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,9 +1,12 @@
 See also: [compojure-api 1.1.x changelog](./CHANGELOG-1.1.x.md)
 
 ## 2.0.0-alpha34-SNAPSHOT
-* Lazily load spec and schema coercion
 * bump spec-tools to 0.10.6
   * notable changes: swagger `:name` defaults to `"body"` instead of `""` ([diff](https://github.com/metosin/spec-tools/compare/0.10.2...0.10.3))
+* Add compatibility for 1.x coercions
+  * implies Schema backend
+  * fn from request->field->schema->coercer
+  * mostly often just `(constantly nil)` or (constantly {:body matcher})
 * **BREAKING**: removed `api-defaults` vars
   * add `-v2` suffix to vars
 
diff --git a/src/compojure/api/api.clj b/src/compojure/api/api.clj
@@ -5,7 +5,6 @@
             [compojure.api.request :as request]
             [compojure.api.routes :as routes]
             [compojure.api.common :as common]
-            [compojure.api.request :as request]
             [ring.swagger.common :as rsc]
             [ring.swagger.middleware :as rsm]))
 
diff --git a/src/compojure/api/coercion.clj b/src/compojure/api/coercion.clj
@@ -4,8 +4,8 @@
             [compojure.api.request :as request]
             [compojure.api.coercion.core :as cc]
             ;; side effects
-            compojure.api.coercion.register-schema
-            compojure.api.coercion.register-spec)
+            [compojure.api.coercion.schema :as cschema]
+            compojure.api.coercion.spec)
   (:import (compojure.api.coercion.core CoercionError)))
 
 (def default-coercion :schema)
@@ -23,6 +23,7 @@
     (nil? coercion) nil
     (keyword? coercion) (cc/named-coercion coercion)
     (satisfies? cc/Coercion coercion) coercion
+    (fn? coercion) (cschema/create-coercion coercion)
     :else (throw (ex-info (str "invalid coercion " coercion) {:coercion coercion}))))
 
 (defn get-apidocs [maybe-coercion spec info]
diff --git a/src/compojure/api/coercion/register_schema.clj b/src/compojure/api/coercion/register_schema.clj
diff --git a/src/compojure/api/coercion/register_spec.clj b/src/compojure/api/coercion/register_spec.clj
diff --git a/src/compojure/api/coercion/schema.clj b/src/compojure/api/coercion/schema.clj
@@ -5,9 +5,7 @@
             [compojure.api.coercion.core :as cc]
             [clojure.walk :as walk]
             [schema.core :as s]
-            [compojure.api.common :as common]
-            ;; side effects
-            compojure.api.coercion.register-schema)
+            [compojure.api.common :as common])
   (:import (java.io File)
            (schema.core OptionalKey RequiredKey)
            (schema.utils ValidationError NamedError)))
@@ -34,7 +32,11 @@
   (common/fifo-memoize sc/coercer 1000))
 
 ;; don't use coercion for certain types
-(defmulti coerce-response? identity :default ::default)
+(defmulti coerce-response? #(if (or (class? %)
+                                    (keyword? %))
+                              %
+                              ::default)
+  :default ::default)
 (defmethod coerce-response? ::default [_] true)
 (defmethod coerce-response? File [_] false)
 
@@ -55,9 +57,15 @@
         (update :errors stringify)))
 
   (coerce-request [_ schema value type format request]
-    (let [type-options (options type)]
-      (if-let [matcher (or (get (get type-options :formats) format)
-                           (get type-options :default))]
+    (let [legacy? (fn? options)
+          type-options (if legacy?
+                         (when-let [provider (options request)]
+                           (provider type))
+                         (options type))]
+      (if-let [matcher (if legacy?
+                         type-options
+                         (or (get (get type-options :formats) format)
+                             (get type-options :default)))]
         (let [coerce (memoized-coercer schema matcher)
               coerced (coerce value)]
           (if (su/error? coerced)
@@ -83,6 +91,10 @@
    :response {:default (constantly nil)}})
 
 (defn create-coercion [options]
+  {:pre [(or (map? options)
+             (fn? options))]}
   (->SchemaCoercion :schema options))
 
 (def default-coercion (create-coercion default-options))
+
+(defmethod cc/named-coercion :schema [_] default-coercion)
diff --git a/src/compojure/api/coercion/spec.clj b/src/compojure/api/coercion/spec.clj
@@ -6,9 +6,7 @@
             [clojure.walk :as walk]
             [compojure.api.coercion.core :as cc]
             [spec-tools.swagger.core :as swagger]
-            [compojure.api.common :as common]
-            ;; side effects
-            compojure.api.coercion.register-spec)
+            [compojure.api.common :as common])
   (:import (clojure.lang IPersistentMap)
            (schema.core RequiredKey OptionalKey)
            (spec_tools.core Spec)
@@ -151,3 +149,5 @@
   (->SpecCoercion :spec options))
 
 (def default-coercion (create-coercion default-options))
+
+(defmethod cc/named-coercion :spec [_] default-coercion)
diff --git a/src/compojure/api/middleware.clj b/src/compojure/api/middleware.clj
@@ -121,7 +121,9 @@
    :string coerce/query-schema-coercion-matcher
    :response coerce/json-schema-coercion-matcher})
 
-;; 1.1.x
+(def no-response-coercion
+  (constantly (dissoc default-coercion-matchers :response)))
+
 (defn coercion-matchers [request]
   (let [options (get-options request)]
     (if (contains? options :coercion)
diff --git a/src/compojure/api/resource.clj b/src/compojure/api/resource.clj
@@ -154,9 +154,9 @@
                         Top-level mw are applied first if route matches, method-level
                         mw are applied next if method matches
 
-  - **:coercion**       A function from request->type->coercion-matcher, used
+  - **:coercion**       A named coercion or instance of Coercion
                         in resource coercion for :body, :string and :response.
-                        Setting value to `(constantly nil)` disables both request- &
+                        Setting value to `nil` disables both request- &
                         response coercion. See tests and wiki for details.
 
   Enhancements to ring-swagger operations map:
diff --git a/test/compojure/api/coercion/schema_coercion_test.clj b/test/compojure/api/coercion/schema_coercion_test.clj
@@ -284,3 +284,10 @@
 
     (testing "generates valid swagger spec"
       (is (validator/validate app)))))
+
+(def some-spec (s/conditional
+                 identity s/Int))
+(defmethod cs/coerce-response? some-spec [_] false)
+
+(deftest coerce-response-memory-leak-test
+  (is (true? (cs/coerce-response? some-spec))))
diff --git a/test/compojure/api/coercion_test.clj b/test/compojure/api/coercion_test.clj
@@ -6,14 +6,17 @@
             [ring.util.http-response :refer :all]
             [clojure.core.async :as a]
             [schema.core :as s]
+            [compojure.api.middleware :as mw]
             [compojure.api.coercion.schema :as cs]))
 
 (defn is-has-body [expected value]
   (is (= (second value) expected)))
 
 (defn is-fails-with [expected-status [status body]]
-  (is (= status expected-status))
-  (is (every? (partial contains? body) [:type :coercion :in :value :schema :errors])))
+  (is (= status expected-status)
+      (pr-str body))
+  (is (every? (partial contains? body) [:type :coercion :in :value :schema :errors])
+      (pr-str body)))
 
 (deftest schema-coercion-test
   (testing "response schemas"
@@ -73,7 +76,15 @@
                         ping-route)]
               (let [[status body] (get* app "/ping")]
                 (is (= 200 status))
-                (is (= {:pong 123} body)))))
+                (is (= {:pong 123} body))))
+            (testing "legacy"
+              (let [app (api
+                          {:formatter :muuntaja
+                           :coercion mw/no-response-coercion}
+                          ping-route)]
+                (let [[status body] (get* app "/ping")]
+                  (is (= 200 status))
+                  (is (= {:pong 123} body))))))
           (testing "all coercion"
             (let [app (api
                         {:coercion nil}
@@ -99,7 +110,7 @@
                                (a/go (ok "foo"))))]
                 (is-fails-with 500 (get* app "/async"))))))))
 
-    (testing "body coersion"
+    (testing "body coercion"
       (let [beer-route (POST "/beer" []
                          :body [body {:beers #{(s/enum "ipa" "apa")}}]
                          (ok body))]
@@ -127,11 +138,35 @@
               (is (= 200 status))
               (is (= {:beers ["ipa" "apa" "ipa"]} body)))))
 
+        (testing "legacy body-coercion can be disabled"
+          (let [no-body-coercion (constantly (dissoc mw/default-coercion-matchers :body))
+                app (api
+                      {:formatter :muuntaja
+                       :coercion no-body-coercion}
+                      beer-route)]
+            (let [[status body] (post* app "/beer" (json-string {:beers ["ipa" "apa" "ipa"]}))]
+              (is (= 200 status))
+              (is (= {:beers ["ipa" "apa" "ipa"]} body))))
+          (let [app (api
+                      {:formatter :muuntaja
+                       :coercion nil}
+                      beer-route)]
+            (let [[status body] (post* app "/beer" (json-string {:beers ["ipa" "apa" "ipa"]}))]
+              (is (= 200 status))
+              (is (= {:beers ["ipa" "apa" "ipa"]} body)))))
+
         (testing "body-coercion can be changed"
           (let [nop-body-coercion (cs/create-coercion (assoc cs/default-options :body {:default (constantly nil)}))
                 app (api
                       {:coercion nop-body-coercion}
                       beer-route)]
+            (is-fails-with 400 (post* app "/beer" (json-string {:beers ["ipa" "apa" "ipa"]})))))
+        (testing "legacy body-coercion can be changed"
+          (let [nop-body-coercion (constantly (assoc mw/default-coercion-matchers :body (constantly nil)))
+                app (api
+                      {:formatter :muuntaja
+                       :coercion nop-body-coercion}
+                      beer-route)]
             (is-fails-with 400 (post* app "/beer" (json-string {:beers ["ipa" "apa" "ipa"]})))))))
 
     (testing "query coercion"
@@ -156,11 +191,29 @@
               (is (= 200 status))
               (is (= {:i "10"} body)))))
 
+        (testing "legacy query-coercion can be disabled"
+          (let [no-query-coercion (constantly (dissoc mw/default-coercion-matchers :string))
+                app (api
+                      {:formatter :muuntaja
+                       :coercion no-query-coercion}
+                      query-route)]
+            (let [[status body] (get* app "/query" {:i 10})]
+              (is (= 200 status))
+              (is (= {:i "10"} body)))))
+
         (testing "query-coercion can be changed"
           (let [nop-query-coercion (cs/create-coercion (assoc cs/default-options :string {:default (constantly nil)}))
                 app (api
                       {:coercion nop-query-coercion}
                       query-route)]
+            (is-fails-with 400 (get* app "/query" {:i 10}))))
+
+        (testing "legacy query-coercion can be changed"
+          (let [nop-query-coercion (constantly (assoc mw/default-coercion-matchers :string (constantly nil)))
+                app (api
+                      {:formatter :muuntaja
+                       :coercion nop-query-coercion}
+                      query-route)]
             (is-fails-with 400 (get* app "/query" {:i 10}))))))
 
     (testing "route-specific coercion"
@@ -200,6 +253,52 @@
 
         (testing "no coercion"
           (let [[status body] (get* app "/no-coercion" {:i 10})]
+            (is (= 200 status))
+            (is (= {:i "10"} body))))))
+    (testing "legacy route-specific coercion"
+      (let [app (api
+                  {:formatter :muuntaja}
+                  (GET "/default" []
+                    :query-params [i :- s/Int]
+                    (ok {:i i}))
+                  (GET "/disabled-coercion" []
+                    :coercion (constantly (assoc mw/default-coercion-matchers :string (constantly nil)))
+                    :query-params [i :- s/Int]
+                    (ok {:i i}))
+                  (GET "/no-coercion" []
+                    :coercion (constantly nil)
+                    :query-params [i :- s/Int]
+                    (ok {:i i}))
+                  (GET "/nil-coercion" []
+                    :coercion nil
+                    :query-params [i :- s/Int]
+                    (ok {:i i})))]
+
+        (testing "default coercion"
+          (let [[status body] (get* app "/default" {:i 10})]
+            (is (= 200 status))
+            (is (= {:i 10} body))))
+
+        (testing "disabled coercion"
+          (is-fails-with 400 (get* app "/disabled-coercion" {:i 10})))
+
+        (testing "exception data"
+          (let [ex (get* app "/disabled-coercion" {:i 10})]
+            (is (= 400 (first ex)))
+            (is (= {:type "compojure.api.exception/request-validation"
+                    :coercion "schema",
+                    :in ["request" "query-params"],
+                    :value {:i "10"}
+                    :schema "{Keyword Any, :i Int}",
+                    :errors {:i "(not (integer? \"10\"))"}}
+                   (select-keys (second ex)
+                                [:type :coercion :in :value :schema :errors])))))
+
+        (testing "no coercion"
+          (let [[status body] (get* app "/no-coercion" {:i 10})]
+            (is (= 200 status))
+            (is (= {:i "10"} body)))
+          (let [[status body] (get* app "/nil-coercion" {:i 10})]
             (is (= 200 status))
             (is (= {:i "10"} body)))))))
 
@@ -230,6 +329,19 @@
         (is (= ["1" 2] (:body (app {:request-method :get :uri "/api/ping" :query-params {:x "1", :y 2}}))))
         (is (thrown? Exception (app {:request-method :get :uri "/api/ping" :query-params {:x "1", :y "abba"}})))))
 
+    (testing "legacy coercion can be overridden"
+      (let [app (context "/api" []
+                  :query-params [{y :- Long 0}]
+                  (GET "/ping" []
+                    :coercion (constantly nil)
+                    :query-params [x :- Long]
+                    (ok [x y])))]
+        (is (thrown? Exception (app {:request-method :get :uri "/api/ping" :query-params {}})))
+        (is (= ["abba" 0] (:body (app {:request-method :get :uri "/api/ping" :query-params {:x "abba"}}))))
+        (is (= ["1" 0] (:body (app {:request-method :get :uri "/api/ping" :query-params {:x "1"}}))))
+        (is (= ["1" 2] (:body (app {:request-method :get :uri "/api/ping" :query-params {:x "1", :y 2}}))))
+        (is (thrown? Exception (app {:request-method :get :uri "/api/ping" :query-params {:x "1", :y "abba"}})))))
+
     (testing "context coercion is used for subroutes"
       (let [app (context "/api" []
                   :coercion nil
