feat(logging): merge logging config into one submodule

Mostly refactoring to make all logging configuration handled in a single
submodule.

BREAKING CHANGE: Change in how coercion and exception logging is
handled. Now to enable logging for exception or coercion, it should be
passed in `#duct.reitit/logging{:types []}`

See: #3

README.md

@@ -47,8 +47,9 @@ Full configuration demo:
                           :get-author {} ;; init foo.handler/get-author
                           :divide {}} ;; init foo.handler/divide
 
-  ;; Logger to be used in reitit module.
-  :duct.reitit/logger      #ig/ref :duct/logger
+  :duct.reitit/logging  {:logger (ig/ref :duct/logger)  ;; Logger to be used in reitit module.
+                         :types [:exception :coercion]
+                         :pretty? true}
 
   ;; Whether to use muuntaja for formatting. default true, can be a modified instance of muuntaja.
   :duct.reitit/muuntaja   true
@@ -60,14 +61,10 @@ Full configuration demo:
   :duct.reitit/middleware   []
 
   ;; Exception handling configuration
-  :duct.reitit/exception  {:handlers #ig/ref :foo.handler/exceptions
-                           :log? true ;; default true.
-                           :pretty? true} ;; default in dev.
+  :duct.reitit/exception  #ig/ref :foo.handler/exceptions
 
   ;; Coercion configuration
-  :duct.reitit/coercion   {:enable true
-                           :coercer 'spec ; Coercer to be used
-                           :pretty? true  ; Whether to pretty print coercion errors
+  :duct.reitit/coercion   {:coercer 'spec ; Coercer to be used
                            :formater nil} ; Function that takes spec validation error map and format it
 
   ;; Cross-origin configuration, the following defaults in for dev profile
@@ -97,9 +94,16 @@ without requiring the user to define them outside the registry.
 <project-ns>.<handler-ns>[.<result key namespace>]/<result key name>
 ```
 
-#### `:duct.reitit/logger`
+#### `:duct.reitit/logging`
 
-Logger to be used in logging stuff. e.g. `duct/logger`. default nil
+Logger configuration
+
+- `:types`: A vector of types of logs to be logged.
+    - `:exception`: whether the exception should be logged.
+    - `:coercion`: whether the coercion errors should be logged.
+    - `:requests`: whether requests to the server should be logged. Default true in development environment.
+- `:logger`: the logger would be used for logging.
+- `:pretty?`: default true in development environment. Make logs easier to read.
 
 #### `:duct.reitit/muuntaja `
 
@@ -122,21 +126,14 @@ other default configurable ones like `:duct.reitit/coercion` and `:duct.reitit/e
 coercion configuration, default nil.
 
 - `:coercer`: either 'malli 'spec 'schema or a value for custom coercor. default nil
-- `:pretty?` whether to pretty print coercion spec errors. default in dev
 - `:formater` custom function to format the return body. default nil
 
 #### `:duct.reitit/exception`
 
-Exception Handling configuration
-
-- `:handlers`: basic wrapper around [ring-reitit-exception-middleware].
-  It expects a map of exception classes or
-  `reitit.ring.middleware.exception` keys like wrap or default, and a
-  function that takes `[exception request]`.
-- `:log?` whether to log exceptions. default true. If
-  `duct.reitit/logger`, then it will be used to log exceptions,
-  otherwise it would use pretty print.
-- `:pretty?` whether to make log exceptions easier to read.
+Handlers for exceptions thrown while handling routing. It is basic wrapper
+around [ring-reitit-exception-middleware]. It expects a map of exception
+classes or `reitit.ring.middleware.exception` keys like wrap or default, and a
+function that takes `[exception request]`.
 
 [ring-reitit-exception-middleware]: https://cljdoc.org/d/metosin/reitit/0.5.15/doc/ring/exception-handling-with-ring#exceptioncreate-exception-middleware
 

src/duct/reitit.clj

@@ -8,22 +8,23 @@
 (def ^:private base-config
   {:duct.core/handler-ns 'handler
    :duct.core/middleware-ns 'middleware
-   ::exception {:log?  true}
    ::environment {}
    ::middleware []
    ::muuntaja true
-   ::coercion nil})
+   ::coercion nil
+   ::logging {:types [:exception]
+              :pretty? false :logger nil}})
    ; ::logger (m/displace (ig/ref :duct/logger))})
 
 (def ^:private configs
   {:development
-   {::exception {:pretty? true}
-    ::coercion {:pretty? true}
+   {::logging {:pretty? true
+               :types [:coercion :requests]}
     ::muuntaja true
     ::cross-origin {:origin [#".*"] :methods [:get :post :delete :options]}}
    :production
-   {::exception {:pretty? false}
-    ::coercion {:pretty? false}}})
+   {::logging {:types ^:replace [:requests]
+               :pretty? false}}})
 
 (defn- merge-to-options [configs]
   (reduce-kv

src/duct/reitit/middleware.clj

@@ -14,7 +14,7 @@
                    :id  (java.util.UUID/randomUUID)
                    :start-date (java.util.Date.)))))
 
-(defn- get-coercion-middleware [{:keys [pretty?] :as coercion}]
+(defn- get-coercion-middleware [coercion {:keys [pretty?] :as _logging}]
   (when coercion
     {:coerce-exceptions (when-not pretty? rcc/coerce-exceptions-middleware)
      :coerce-request rcc/coerce-request-middleware
@@ -27,8 +27,8 @@
   (->>  defaults (conj (or middleware [])) (apply concat) vec compact))
 
 (defmethod init-key :duct.reitit/middleware [_ options]
-  (let [{:keys [muuntaja middleware coercion]} options
-        {:keys [coerce-response coerce-request coerce-exceptions]} (get-coercion-middleware coercion)
+  (let [{:keys [muuntaja middleware coercion logging]} options
+        {:keys [coerce-response coerce-request coerce-exceptions]} (get-coercion-middleware coercion logging)
         format-middleware    (get-format-middleware muuntaja)
         exception-middleware (get-exception-middleware options)
         create-middleware (partial extend-middleware middleware)]

src/duct/reitit/middleware/exception.clj

@@ -1,10 +1,11 @@
 (ns duct.reitit.middleware.exception
-  (:require [duct.reitit.util :refer [try-resolve-sym spy compact]]
+  (:require [duct.reitit.util :refer [try-resolve-sym spy compact member?]]
             [reitit.ring.middleware.exception :as exception :refer [default-handlers create-exception-middleware]]
             [duct.logger :as logger]
-            [clojure.pprint :as pprint]))
+            [clojure.pprint :as pprint]
+            [duct.reitit.util :as util]))
 
-(defn coercion-error-handler [status expound-printer _formatter]
+(defn get-coercion-error-handler [status expound-printer _formatter]
   (let [printer (expound-printer {:theme :figwheel-theme, :print-specs? false})
         handler (exception/create-coercion-handler status)]
     (if printer
@@ -14,12 +15,13 @@
       (fn [exception request] ;; TODO: format
         (handler exception request)))))
 
-(defn coercion-handlers [{:keys [pretty? formatter]}]
-  (let [printer (when pretty? (try-resolve-sym 'expound.alpha/custom-printer))]
+(defn coercion-handlers [{:keys [formatter]} {:keys [pretty? types]}]
+  (let [printer (when (and pretty? (member? types :coercion))
+                  (try-resolve-sym 'expound.alpha/custom-printer))]
     (when (or printer formatter)
       #:reitit.coercion
-       {:request-coercion (coercion-error-handler 400 printer formatter)
-        :response-coercion (coercion-error-handler 500 printer formatter)})))
+       {:request-coercion (get-coercion-error-handler 400 printer formatter)
+        :response-coercion (get-coercion-error-handler 500 printer formatter)})))
 
 (defn- with-default-exceptions [& handlers]
   (->> (cons default-handlers handlers)
@@ -40,19 +42,20 @@
                          (vec))}]
     (if pretty? (str "\n" (with-out-str (pprint/pprint log))) log)))
 
+(defn- get-exception-wrapper [{:keys [logger pretty?]}]
+  (fn [handler exception request]
+    (if logger
+      (logger/log logger :error (format-exception-log exception request pretty?))
+      (pprint/pprint (format-exception-log exception request false)))
+    (handler exception request)))
+
 (defn get-exception-middleware
   "Create custom exception middleware."
-  [{:keys [coercion exception logger]}]
-  (let [coercion-handlers (coercion-handlers coercion)]
+  [{:keys [coercion exception logging]}]
+  (let [coercion-handlers (coercion-handlers coercion logging)
+        exception-wrapper (when (member? (:types logging) :exception)
+                            {::exception/wrap (get-exception-wrapper logging)})]
     (with-default-exceptions
-      (:handlers exception)
+      exception
       coercion-handlers
-      (when (:log? exception)
-        {::exception/wrap
-         (fn [handler e request]
-           (if logger
-             (logger/log logger :error (format-exception-log e request (:pretty? exception)))
-             (pprint/pprint (format-exception-log e request false)))
-           (handler e request))}))))
-
-
+      exception-wrapper)))

test/duct/reitit/middleware_test.clj

@@ -120,31 +120,41 @@
           (is (not (str/includes? (:body requestc) "github"))))))
 
     (testing "Coercion Pretty Exception"
-      (let [middleware (new-middleware {:munntaja false :coercion {:pretty? true}})
+      (let [middleware (new-middleware {:munntaja false :coercion {} :logging {:pretty? true
+                                                                               :types [:coercion]}})
             app  (->> {:middleware middleware :environment environment}
                       (new-router routes)
                       (ring/ring-handler))
             request {:request-method :get :uri (str "/identity/company/users/tami")}]
+
+        (is (= [:reitit.ring.middleware.parameters/parameters
+                :duct.reitit.middleware/environment-middleware
+                :reitit.ring.middleware.exception/exception
+                :reitit.ring.coercion/coerce-request
+                :reitit.ring.coercion/coerce-response]
+               (mapv :name middleware))
+            "Shouldn't include coerce-exceptions")
+
         (is (str/includes? (with-out-str (app request)) "-- Spec failed --------------------")
             "Should only print to stdout and not return it")))))
 
 (deftest custom-error-handling
-  (let [exception {java.lang.NullPointerException (fn [_ r]
-                                                    {:status 500
-                                                     :cause "No parameters received"
-                                                     :uri (:uri r)})
-                   java.lang.ArithmeticException (fn [e r]
-                                                   {:status 500
-                                                    :cause (ex-message e)
-                                                    :data (:body-params r)
-                                                    :uri (:uri r)})}
-        middleware (new-middleware {:munntaja false :coercion nil :exception {:handlers exception}})
+  (let [exception {java.lang.NullPointerException
+                   (fn [_ r]
+                     {:status 500
+                      :cause "No parameters received"
+                      :uri (:uri r)})
+                   java.lang.ArithmeticException
+                   (fn [e r]
+                     {:status 500
+                      :cause (ex-message e)
+                      :data (:body-params r)
+                      :uri (:uri r)})}
+        middleware (new-middleware {:munntaja false :coercion nil :exception exception})
         router (new-router [["/math" (fn [{{:keys [lhs rhs]} :body-params}] (/ lhs rhs))]] {:middleware middleware})
         handler (ring/ring-handler router)
         math-response (handler {:request-method :get :uri "/math" :body-params {:lhs 5 :rhs 0}})
         no-params-response (handler {:request-method :get :uri "/math"})]
     (is (= "Divide by zero" (:cause math-response)))
     (is (= {:lhs 5, :rhs 0} (:data math-response)))
     (is (= "No parameters received" (:cause no-params-response)))))
-
-

test/duct/reitit_test.clj

@@ -8,7 +8,8 @@
             [reitit.ring :as ring]
             [reitit.core :as r]
             [fipp.clojure :refer [pprint]]
-            [duct.reitit.util :refer [to-edn spy]]))
+            [duct.reitit.util :refer [to-edn spy]]
+            [clojure.string :as str]))
 
 (core/load-hierarchy)
 
@@ -55,11 +56,13 @@
                   :duct.module/reitit {}}
             [_ in-options] (new-config-handling base)]
         (are [path value] (-> path in-options (= value))
-          [:exception :log?] true ;; log exception enabled by default
-          :muuntaja true          ;; Muuntaja formatting is enabled by default
-          :environment {}         ;; Empty Environment
-          :middleware []          ;; Empty Middleware
-          :coercion nil)))        ;; no :coercion configuration
+          [:logging :types] [:exception] ;; default types supported by default
+          [:logging :pretty?] false      ;; No pretty logging by default.
+          [:logging :logger] nil         ;; No logger by default.
+          :muuntaja true                 ;; Muuntaja formatting is enabled by default
+          :environment {}                ;; Empty Environment
+          :middleware []                 ;; Empty Middleware
+          :coercion nil)))               ;; no :coercion configuration
 
     (testing "Default Development Environment Options"
       (let [base {:duct.profile/dev {}
@@ -68,9 +71,9 @@
                   :duct.module/reitit {}}
             [_ in-options] (new-config-handling base [:duct.profile/dev])]
         (are [path value] (-> path in-options (= value))
-          [:exception :log?] true          ;; log exception enabled by default
-          [:exception :pretty?] true       ;; log exception enabled by default
-          [:coercion :pretty?] true        ;; log exception enabled by default
+          [:logging :types] [:exception :coercion :requests] ;; default types supported by default
+          [:logging :pretty?] true         ;; pretty logging by default.
+          [:logging :logger] nil           ;; No logger by default.
           :muuntaja true                   ;; Muuntaja formatting is enabled by default
           :environment {}                  ;; Empty Environment
           :middleware []                   ;; Empty Middleware
@@ -84,13 +87,13 @@
                   :duct.module/reitit {}}
             [_ in-options] (new-config-handling base [:duct.profile/prod])]
         (are [path value] (-> path in-options (= value))
-          [:exception :log?] true          ;; log exception enabled by default
-          [:exception :pretty?] false      ;; No pretty exceptions
-          [:coercion :pretty?] false       ;; log exception enabled by default
-          :muuntaja true                   ;; Muuntaja formatting is enabled by default
-          :environment {}                  ;; Empty Environment
-          :middleware []                   ;; Empty Middleware
-          :cross-origin nil)))))              ;; No Cross-origin
+          [:logging :types] [:requests] ;; default types supported by default
+          [:logging :pretty?] false     ;; No pretty logging by default.
+          [:logging :logger] nil        ;; No logger by default.
+          :muuntaja true                ;; Muuntaja formatting is enabled by default
+          :environment {}               ;; Empty Environment
+          :middleware []                ;; Empty Middleware
+          :cross-origin nil)))))        ;; No Cross-origin
 
 (derive :foo/database :duct/const)
 (derive :foo/index-path :duct/const)
@@ -123,8 +126,10 @@
                             :get-author {} ;; init foo.handler/get-author
                             :divide {}} ;; init foo.handler/divide
 
-    ;; Logger to be used in reitit module.
-    :duct.reitit/logger      (ig/ref :duct/logger)
+    ;; Logging Configuration
+    :duct.reitit/logging  {:logger (ig/ref :duct/logger)  ;; Logger to be used in reitit module.
+                           :types [:exception :coercion]
+                           :pretty? true}
 
     ;; Whether to use muuntaja for formatting. default true, can be a modified instance of muuntaja.
     :duct.reitit/muuntaja   true
@@ -136,14 +141,10 @@
     :duct.reitit/middleware   []
 
     ;; Exception handling configuration
-    :duct.reitit/exception  {:handlers (ig/ref :foo.handler/exceptions)
-                             :log? true ;; default true.
-                             :pretty? true} ;; default in dev.
+    :duct.reitit/exception  (ig/ref :foo.handler/exceptions)
 
     ;; Coercion configuration
-    :duct.reitit/coercion   {:enable true
-                             :coercer 'spec ; Coercer to be used
-                             :pretty? true ; Whether to pretty print coercion errors
+    :duct.reitit/coercion   {:coercer 'spec ; Coercer to be used
                              :formater nil} ; Function that takes spec validation error map and format it
 
     ;; Cross-origin configuration, the following defaults in for dev and local profile
@@ -180,10 +181,22 @@
     (testing "reitit ring handler"
       (let [handler (:duct.handler/root config)]
         (is (fn? handler))
-        (is (nil? (handler {:request-method :get :uri "/not-a-route"})))
-        (is (string? (:body (handler {:request-method :get :uri "/"}))))
-        (is (= "pong" (-> {:request-method :get :uri "/ping"} handler to-edn :message)))
-        (is (= 9 (-> {:request-method :post :uri "/plus" :body-params {:y 3 :x 6}} handler to-edn :total)))
-        (is (= 9 (-> {:request-method :get :uri "/plus" :query-params {:y 3 :x 6}} handler to-edn :total)))
-        (is (= "tami5" (-> {:request-method :get :uri "/author"} handler to-edn :author)))
-        (is (= "Divide by zero" (-> {:request-method :get :uri "/divide" :body-params {:y 0 :x 0}} handler to-edn :cause)))))))
+
+        (testing "Ring Routing"
+          (is (nil? (handler {:request-method :get :uri "/not-a-route"})))
+          (is (string? (:body (handler {:request-method :get :uri "/"}))))
+          (is (= "pong" (-> {:request-method :get :uri "/ping"} handler to-edn :message)))
+          (is (= 9 (-> {:request-method :post :uri "/plus" :body-params {:y 3 :x 6}} handler to-edn :total)))
+          (is (= 9 (-> {:request-method :get :uri "/plus" :query-params {:y 3 :x 6}} handler to-edn :total))))
+
+        (testing "Environment-Keys-Access"
+          (is (= "tami5" (-> {:request-method :get :uri "/author"} handler to-edn :author))))
+
+        (testing "Custom-Exception-handling"
+          ;; TODO: find away to test if logger logged :)
+          (is (= "Divide by zero" (-> {:request-method :get :uri "/divide" :body-params {:y 0 :x 0}} handler to-edn :cause))))
+
+        (testing "Coercion-Logging"
+          (let [request {:request-method :get :uri "/plus" :query-params {:y "str" :x 6}}]
+            (is (str/includes? (with-out-str (handler request)) "-- Spec failed --------------------")
+                "Should only print to stdout and not return it")))))))