From 6d263ae1b68cc57de8b8bdad5388607677527bca Mon Sep 17 00:00:00 2001
From: Tommi Reiman <tommi@iki.fi>
Date: Sat, 11 May 2019 10:25:09 +0300
Subject: [PATCH] Test Pedestal error mapping, add dev helpers

---
 .../pedestal-swagger/src/example/server.clj   | 15 +++++++-
 .../reitit-pedestal/src/reitit/pedestal.clj   | 38 +++++++++++--------
 test/clj/reitit/pedestal_test.clj             | 23 +++++++++++
 3 files changed, 59 insertions(+), 17 deletions(-)
 create mode 100644 test/clj/reitit/pedestal_test.clj

diff --git a/examples/pedestal-swagger/src/example/server.clj b/examples/pedestal-swagger/src/example/server.clj
index 51e16059..c5f18623 100644
--- a/examples/pedestal-swagger/src/example/server.clj
+++ b/examples/pedestal-swagger/src/example/server.clj
@@ -9,8 +9,12 @@
             [reitit.coercion.spec :as spec-coercion]
             [reitit.http.interceptors.parameters :as parameters]
             [reitit.http.interceptors.muuntaja :as muuntaja]
+            [reitit.http.interceptors.exception :as exception]
             [reitit.http.interceptors.multipart :as multipart]
             [reitit.http.interceptors.dev :as dev]
+            [reitit.dev.pretty :as pretty]
+            [spec-tools.spell :as spell]
+            [reitit.http.spec :as spec]
             [clojure.core.async :as a]
             [clojure.java.io :as io]
             [muuntaja.core :as m]))
@@ -76,15 +80,22 @@
                             {:status 200
                              :body {:total (+ x y)}})}}]]]
 
-      {;;:reitit.interceptor/transform dev/print-context-diffs
+      {;:reitit.interceptor/transform dev/print-context-diffs ;; pretty context diffs
+       ;;:validate spec/validate ;; enable spec validation for route data
+       ;;:reitit.spec/wrap spell/closed ;; strict top-level validation (alpha)
+       :exception pretty/exception
        :data {:coercion spec-coercion/coercion
               :muuntaja m/instance
-              :interceptors [;; query-params & form-params
+              :interceptors [;; swagger feature
+                             swagger/swagger-feature
+                             ;; query-params & form-params
                              (parameters/parameters-interceptor)
                              ;; content-negotiation
                              (muuntaja/format-negotiate-interceptor)
                              ;; encoding response body
                              (muuntaja/format-response-interceptor)
+                             ;; exception handling
+                             (exception/exception-interceptor)
                              ;; decoding request body
                              (muuntaja/format-request-interceptor)
                              ;; coercing response bodys
diff --git a/modules/reitit-pedestal/src/reitit/pedestal.clj b/modules/reitit-pedestal/src/reitit/pedestal.clj
index 28c21e0a..51c9a45b 100644
--- a/modules/reitit-pedestal/src/reitit/pedestal.clj
+++ b/modules/reitit-pedestal/src/reitit/pedestal.clj
@@ -4,18 +4,18 @@
             [io.pedestal.http :as http]
             [reitit.interceptor]
             [reitit.http])
-  (:import (reitit.interceptor Executor)))
+  (:import (reitit.interceptor Executor)
+           (java.lang.reflect Method)))
 
-(defn- arity [f]
+(defn- arities [f]
   (->> (class f)
        .getDeclaredMethods
        (filter #(= "invoke" (.getName %)))
-       first
-       .getParameterTypes
-       alength))
+       (map #(alength (.getParameterTypes ^Method %)))
+       (set)))
 
-(defn- error-with-arity-1? [{error-fn :error}]
-  (and error-fn (= 1 (arity error-fn))))
+(defn- error-without-arity-2? [{error-fn :error}]
+  (and error-fn (not (contains? (arities error-fn) 2))))
 
 (defn- error-arity-2->1 [error]
   (fn [context ex]
@@ -26,9 +26,23 @@
             (dissoc :error))
         context))))
 
-(defn wrap-error-arity-2->1 [interceptor]
+(defn- wrap-error-arity-2->1 [interceptor]
   (update interceptor :error error-arity-2->1))
 
+(defn ->interceptor [interceptor]
+  (cond
+    (interceptor/interceptor? interceptor)
+    interceptor
+    (seq (select-keys interceptor [:enter :leave :error]))
+    (interceptor/interceptor
+      (if (error-without-arity-2? interceptor)
+        (wrap-error-arity-2->1 interceptor)
+        interceptor))))
+
+;;
+;; Public API
+;;
+
 (def pedestal-executor
   (reify
     Executor
@@ -36,13 +50,7 @@
       (->> interceptors
            (map (fn [{:keys [::interceptor/handler] :as interceptor}]
                   (or handler interceptor)))
-           (map (fn [interceptor]
-                  (if (interceptor/interceptor? interceptor)
-                    interceptor
-                    (interceptor/interceptor
-                     (if (error-with-arity-1? interceptor) 
-                       (wrap-error-arity-2->1 interceptor)
-                       interceptor)))))))
+           (keep ->interceptor)))
     (enqueue [_ context interceptors]
       (chain/enqueue context interceptors))))
 
diff --git a/test/clj/reitit/pedestal_test.clj b/test/clj/reitit/pedestal_test.clj
new file mode 100644
index 00000000..f1942fe9
--- /dev/null
+++ b/test/clj/reitit/pedestal_test.clj
@@ -0,0 +1,23 @@
+(ns reitit.pedestal-test
+  (:require [clojure.test :refer [deftest testing is]]
+            [reitit.pedestal :as pedestal]))
+
+(deftest arities-test
+  (is (= #{0} (#'pedestal/arities (fn []))))
+  (is (= #{1} (#'pedestal/arities (fn [_]))))
+  (is (= #{0 1 2} (#'pedestal/arities (fn ([]) ([_]) ([_ _]))))))
+
+(deftest interceptor-test
+  (testing "wihtout :enter, :leave or :error are stripped"
+    (is (nil? (pedestal/->interceptor {:name ::kikka}))))
+  (testing ":error arities are wrapped"
+    (let [has-2-arity-error? (fn [interceptor]
+                               (-> interceptor
+                                   (pedestal/->interceptor)
+                                   (:error)
+                                   (#'pedestal/arities)
+                                   (contains? 2)))]
+      (is (has-2-arity-error? {:error (fn [_])}))
+      (is (has-2-arity-error? {:error (fn [_ _])}))
+      (is (has-2-arity-error? {:error (fn [_ _ _])}))
+      (is (has-2-arity-error? {:error (fn ([_]) ([_ _]))})))))