Merge pull request #39 from rrudakov/readable-errors

Add readable errors

Author: Roman Rudakov

.clj-kondo/config.edn

@@ -8,4 +8,5 @@
              (compojure.api.sweet/DELETE)
              (compojure.api.sweet/OPTIONS)
              (compojure.api.sweet/POST)
-             (compojure.api.sweet/PUT)]}}}
+             (compojure.api.sweet/PUT)
+             (phrase.alpha/defphraser)]}}}

project.clj

@@ -6,6 +6,8 @@
   :dependencies [[org.clojure/clojure "1.10.1"]
                  ;; Configuration file parsing
                  [aero "1.1.6"]
+                 ;; Human readable errors
+                 [phrase "0.3-alpha4"]
                  ;; Ring and compojure
                  [ring/ring-defaults "0.3.2"]
                  [metosin/compojure-api "2.0.0-alpha31"]

src/education/http/constants.clj

@@ -1,4 +1,7 @@
-(ns education.http.constants)
+(ns education.http.constants
+  (:require [phrase.alpha :as p]
+            [clojure.spec.alpha :as s]
+            [clojure.string :as str]))
 
 (def not-found-error-message "Resource not found")
 
@@ -21,3 +24,67 @@
 (def valid-decimal
   "Regex to check price."
   #"\d+(\.\d+)?")
+
+(def valid-username-regex
+  "Check new username using this regex."
+  #"^[a-zA-Z]+[a-zA-Z0-9]*$")
+
+(def valid-email-regex
+  "Check new email addresses using this regex."
+  #"[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?")
+
+(defn- ->prefix-or
+  [{:keys [in via]} or-value]
+  (->> (or (first in) (first via) or-value)
+       (name)
+       (str/capitalize)))
+
+;; Phrases
+(p/defphraser #(contains? % key)
+  [_ _ key]
+  (format "Field %s is mandatory" (name key)))
+
+(p/defphraser #(re-matches valid-username-regex %)
+  [_ _]
+  "Username must start from letter")
+
+(p/defphraser #(>= (count %) min-length)
+  [_ problem min-length]
+  (str (->prefix-or problem "value") " must be at least " min-length " characters"))
+
+(p/defphraser (complement str/blank?)
+  [_ problem]
+  (str (->prefix-or problem "value") " must not be empty"))
+
+(p/defphraser #(<= (count %) max-length)
+  [_ problem max-length]
+  (str (->prefix-or problem "value") " must not be longer than " max-length " characters"))
+
+(p/defphraser (partial re-matches valid-url-regex)
+  [_ problem]
+  (str (->prefix-or problem "field") " URL is not valid"))
+
+(p/defphraser (partial re-matches valid-decimal)
+  [_ problem]
+  (str (->prefix-or problem "value") " is not valid"))
+
+(p/defphraser #(re-matches valid-email-regex %)
+  [_ _]
+  "Email is not valid")
+
+(p/defphraser :default
+  [_ problem]
+  (str (->prefix-or problem "value") " is not valid"))
+
+;; Convert problems to
+(defn ->phrases
+  "Given a spec and a value x, phrases the first problem using context if any.
+
+  Returns nil if x is valid or no phraser was found. See phrase for details.
+  Use phrase directly if you want to phrase more than one problem."
+  [spec x]
+  (some->> (s/explain-data spec x)
+           ::s/problems
+           (mapv #(p/phrase {} %))
+           (distinct)
+           (filterv (complement nil?))))

src/education/http/routes.clj

@@ -11,7 +11,8 @@
             [education.http.endpoints.upload :refer [upload-routes]]
             [education.http.endpoints.users :refer [users-routes]]
             [ring.util.http-response :as status]
-            [taoensso.timbre :refer [error trace]])
+            [taoensso.timbre :refer [error trace]]
+            [clojure.spec.alpha :as s])
   (:import java.sql.SQLException))
 
 (defn sql-exception-handler
@@ -29,21 +30,26 @@
 (defn request-validation-handler
   "Verify request body and raise error."
   []
-  (fn [^Exception e _ _]
-    (trace e "Invalid request")
-    (error "Invalid request" (.getMessage e))
-    (status/bad-request {:message const/bad-request-error-message
-                         :details (.getMessage e)})))
+  (fn [^Exception e ex-data request]
+    (let [spec (:spec ex-data)
+          body (:body-params request)
+          errors (const/->phrases spec body)]
+      (trace e "Invalid request")
+      (error "Invalid request" (s/explain-str spec body))
+      (status/bad-request {:message const/bad-request-error-message
+                           :errors  errors}))))
 
 (defn response-validation-handler
   "Return error in case of invalid response."
   []
-  (fn [^Exception e _ _]
-    (trace e "Invalid response")
-    (error "Invalid response" (.getMessage e))
-    (status/internal-server-error
-     {:message const/server-error-message
-      :details (.getMessage e)})))
+  (fn [^Exception e ex-data _]
+    (let [spec (:spec ex-data)
+          body (:response ex-data)]
+      (trace e "Invalid response")
+      (error "Invalid response" (s/explain spec body))
+      (status/internal-server-error
+       {:message const/server-error-message
+        :details (s/explain-str spec body)}))))
 
 (defn api-routes
   "Define top-level API routes."

src/education/specs/articles.clj

@@ -1,17 +1,25 @@
 (ns education.specs.articles
-  (:require [clojure.spec.alpha :as s]))
+  (:require [clojure.spec.alpha :as s]
+            [clojure.string :as str]))
 
 (s/def ::id pos-int?)
 
 (s/def ::user_id pos-int?)
 
 (s/def ::user-id-param pos-int?)
 
-(s/def ::title (s/and string? not-empty #(<= (count %) 100)))
+(s/def ::title
+  (s/and string?
+         (complement str/blank?)
+         #(<= (count %) 100)))
 
-(s/def ::body (s/and string? not-empty))
+(s/def ::body
+  (s/and string?
+         (complement str/blank?)))
 
-(s/def ::featured_image (s/and string? #(<= (count %) 500)))
+(s/def ::featured_image
+  (s/and string?
+         #(<= (count %) 500)))
 
 (s/def ::is_main_featured boolean?)
 

src/education/specs/common.clj

@@ -1,19 +1,26 @@
 (ns education.specs.common
   (:require [clojure.spec.alpha :as s]
             [education.http.constants :as const]
-            [education.specs.gymnastics :as g]))
+            [education.specs.gymnastics :as g]
+            [clojure.string :as str]))
 
 (s/def ::id pos-int?)
 
 (s/def ::subtype_id pos-int?)
 
-(s/def ::title (s/and string? not-empty #(<= (count %) 500)))
+(s/def ::title
+  (s/and string?
+         (complement str/blank?)
+         #(<= (count %) 500)))
 
-(s/def ::subtitle (s/and string? not-empty #(<= (count %) 500)))
+(s/def ::subtitle
+  (s/and string?
+         (complement str/blank?)
+         #(<= (count %) 500)))
 
 (s/def ::url
   (s/and string?
-         not-empty
+         (complement str/blank?)
          (partial re-matches const/valid-url-regex)
          #(<= (count %) 1000)))
 
@@ -27,11 +34,13 @@
 (s/def ::screenshots
   (s/coll-of ::screenshot :kind vector? :distinct true :into []))
 
-(s/def ::description (s/and string? not-empty))
+(s/def ::description
+  (s/and string?
+         (complement str/blank?)))
 
 (s/def ::price
   (s/and string?
-         not-empty
+         (complement str/blank?)
          (partial re-matches const/valid-decimal)))
 
 (s/def ::size pos-int?)

src/education/specs/gymnastics.clj

@@ -1,10 +1,11 @@
 (ns education.specs.gymnastics
   (:require [clojure.spec.alpha :as s]
-            [education.http.constants :as const]))
+            [education.http.constants :as const]
+            [clojure.string :as str]))
 
 (s/def ::picture
   (s/nilable
    (s/and string?
-          not-empty
+          (complement str/blank?)
           (partial re-matches const/valid-url-regex)
           #(<= (count %) 1000))))

src/education/specs/users.clj

@@ -1,20 +1,17 @@
 (ns education.specs.users
-  (:require [clojure.spec.alpha :as s]))
-
-(def email-regex
-  "Check new email addresses using this regex."
-  #"[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?")
+  (:require [clojure.spec.alpha :as s]
+            [education.http.constants :as const]))
 
 (s/def ::id int?)
 
 (s/def ::username
   (s/and string?
-         #(re-matches #"^[a-zA-Z]+[a-zA-Z0-9]*$" %)
+         #(re-matches const/valid-username-regex %)
          #(>= (count %) 2)))
 
 (s/def ::password (s/and string? #(>= (count %) 6)))
 
-(s/def ::email (s/and string? #(re-matches email-regex %)))
+(s/def ::email (s/and string? #(re-matches const/valid-email-regex %)))
 
 (s/def ::role #{"admin" "moderator" "guest"})
 

test/education/http/constants_test.clj

@@ -0,0 +1,106 @@
+(ns education.http.constants-test
+  (:require [clojure.test :refer [deftest is testing]]
+            [education.http.constants :as sut]
+            [education.specs.common :as spec]
+            [education.specs.users :as user-spec]
+            [clojure.string :as str]))
+
+(def test-data
+  "Test cases."
+  [[["Field url is mandatory"]
+    ::spec/presentation-create-request
+    {:title       "Title"
+     :description "Description"}]
+
+   [["Username must start from letter"]
+    ::user-spec/username
+    "1asdf"]
+
+   [["Username must start from letter"]
+    ::user-spec/user-create-request
+    {:username "1asdf"
+     :password "123456"
+     :email    "email@example.com"}]
+
+   [["Username must be at least 2 characters"]
+    ::user-spec/username
+    "a"]
+
+   [["Username must be at least 2 characters"]
+    ::user-spec/user-create-request
+    {:username "a"
+     :password "123456"
+     :email    "email@example.com"}]
+
+   [["Title must not be empty"]
+    ::spec/title
+    ""]
+
+   [["Title must not be empty"]
+    ::spec/presentation-create-request
+    {:title       ""
+     :url         "https://google.com/api/presentation"
+     :description "Description"}]
+
+   [["Title must not be longer than 500 characters"]
+    ::spec/title
+    (str/join (repeat 501 "a"))]
+
+   [["Title must not be longer than 500 characters"]
+    ::spec/presentation-create-request
+    {:title       (str/join (repeat 501 "a"))
+     :url         "https://google.com/api/presentation"
+     :description "Description"}]
+
+   [["Url URL is not valid"]
+    ::spec/url
+    "invalid"]
+
+   [["Url URL is not valid"]
+    ::spec/presentation-create-request
+    {:title       "Title"
+     :url         "invalid"
+     :description "Description"}]
+
+   [["Price is not valid"]
+    ::spec/price
+    "invalid"]
+
+   [["Price is not valid"]
+    ::spec/dress-create-request
+    {:title       "title"
+     :description "description"
+     :pictures    []
+     :size        22
+     :price       "invalid"}]
+
+   [["Email is not valid"]
+    ::user-spec/email
+    "invalid@email"]
+
+   [["Email is not valid"]
+    ::user-spec/user-create-request
+    {:username "username"
+     :password "123456"
+     :email    "invalid@email"}]
+
+   [["Id is not valid"]
+    ::spec/id
+    "invalid"]
+
+   [["Username is not valid"]
+    ::user-spec/user-create-request
+    {:username 123
+     :password "123456"
+     :email    "valid@email.com"}]
+
+   [["Field description is mandatory"
+     "Title must not be longer than 500 characters"]
+    ::spec/presentation-create-request
+    {:title (str/join (repeat 501 "a"))
+     :url   "https://google.com/api/presentation"}]])
+
+(deftest ->phrases-test
+  (doseq [[errors spec val] test-data]
+    (testing "Test conversion error to phrase"
+      (is (= errors (sut/->phrases spec val))))))