Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/format validation errors #598

Merged
merged 13 commits into from
Oct 25, 2023
Merged

Feature/format validation errors #598

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a7708a3
Filter and format malli error messages
mpoffald Oct 5, 2023
ac1fc9f
cleanup, rename fns, add docstrings and comments
mpoffald Oct 23, 2023
8b5cbe7
fix `or-parent` loop
mpoffald Oct 24, 2023
c5fe903
sprinkle some more `:error/message`s around, add some doc links
mpoffald Oct 24, 2023
3d693cc
simplify error message in schema
mpoffald Oct 24, 2023
b944281
use more general/robust calculation of limit failures
mpoffald Oct 24, 2023
2a6fc52
better wording for fallthrough invalid-type case
mpoffald Oct 24, 2023
8a2fc35
add pointers to docs
mpoffald Oct 24, 2023
c602f8c
use same default-error-overrides universally in absence of opts, add …
mpoffald Oct 24, 2023
0d86363
better docstring
mpoffald Oct 24, 2023
48ac67a
add errors repl file
mpoffald Oct 24, 2023
6eddc33
normalize kebab vs camel-case keys in docs msg
mpoffald Oct 24, 2023
80bbb5a
fix typo, use consistent terminology
mpoffald Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions dev/errors.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
(ns errors
(:require [fluree.db.json-ld.api :as fluree]
[clojure.java.io :as io]
[clojure.tools.namespace.repl :as tn :refer [refresh refresh-all]]
[clojure.core.async :as async]
[malli.util :as mu]
[malli.error :as me]
[fluree.db.validation :as v]
[malli.core :as m]
[fluree.db.did :as did]
[fluree.db.util.async :refer [<? <?? go-try merge-into?]]
[fluree.db.flake :as flake]
[fluree.db.util.json :as json]
[fluree.db.serde.json :as serdejson]
[fluree.db.storage :as storage]
[fluree.db.query.fql :as fql]
[fluree.db.query.range :as query-range]
[fluree.db.dbproto :as dbproto]
[fluree.db.constants :as const]
[fluree.json-ld :as json-ld]
[clojure.string :as str]
[criterium.core :refer [bench]]))


(comment
;;history errors
(do
(require '[fluree.db.test-utils :as test-utils])
(require '[fluree.db.query.history :as history])
(def conn (test-utils/create-conn))
(def ledger @(fluree/create conn "historytest" {:defaultContext ["" {:ex "http://example.org/ns/"}]}))
(def db1 @(test-utils/transact ledger [{:id :ex/dan
:ex/x "foo-1"
:ex/y "bar-1"}
{:id :ex/cat
:ex/x "foo-1"
:ex/y "bar-1"}
{:id :ex/dog
:ex/x "foo-1"
:ex/y "bar-1"}])))


;;empty request
(ex-message @(fluree/history ledger {}))

;;invalid commit-details
(ex-message @(fluree/history ledger {:history [:ex/cat] :commit-details "I like cats"
:t {:at :latest}}))

;;missing subject
(ex-message @(fluree/history ledger {:history nil}))

;;missing subject
(ex-message @(fluree/history ledger {:history []}))

;;invalid flake
(ex-message @(fluree/history ledger {:history [1 2] :t {:from 1}}))

;;invalid t
(ex-message @(fluree/history ledger {:history [:ex/cat] :t {:from 2 :to 0}}))

;;invalid t
(ex-message @(fluree/history ledger {:history [:ex/cat] :t {:from -2 :to -1}}))

;;missing t values
(ex-message @(fluree/history ledger {:history [:ex/cat] :t {}}))

;;invalid t values
(ex-message @(fluree/history ledger {:history [:ex/cat] :t {:at 1 :from 1}}))


;;fql

;;missing select
(ex-message @(fluree/query {} '{:where [[?s ?p ?o ]]}))

;;multiple selects
(ex-message @(fluree/query {} '{:select [?s]
:selectOne [?s ?p ]
:where [[?s ?p ?o ]]}))
;;invalid select var
(ex-message @(fluree/query {} '{:select [+]
:where [[?s ?p ?o ]]}))

;;unknown key
(ex-message @(fluree/query {} '{:select [?s]
:where [[?s ?p ?o ]]
:foo [?o]}))

;;extra k/v in where map
(ex-message @(fluree/query {} '{:select ['?name '?email]
:where [['?s :type :ex/User]
['?s :schema/age '?age]
['?s :schema/name '?name]
{:union [[['?s :ex/email '?email]]
[['?s :schema/email '?email]]]
:filter ["(> ?age 30)"]}]}))

;;unrecognized op in where map
(ex-message @(fluree/query {} {:select ['?name '?age]
:where [['?s :type :ex/User]
['?s :schema/age '?age]
['?s :schema/name '?name]
{:foo "(> ?age 45)"}]}))

;;invalid where
(ex-message @(fluree/query {} '{:select [?s ?o]
:where ?s}))

;;invalid where
(ex-message @(fluree/query {} '{:select [?s ?o]
:where [?s ?p ?o]}))

;;invalid group-by
(ex-message @(fluree/query {} '{:select [?s]
:where [[?s ?p ?o]]
:group-by {}}))

;;invalid order-by
(ex-message @(fluree/query {} '{:select ['?name '?favNums]
:where [['?s :schema/name '?name]
['?s :schema/age '?age]
['?s :ex/favNums '?favNums]]
:orderBy [(foo ?favNums)]}))

;;invalid bind
(ex-message @(fluree/query {} '{:select [?firstLetterOfName ?name ?canVote]
:where [[?s :schema/age ?age]
[?s :schema/name ?name]
{:bind [?canVote (>= ?age 18)]}]}))

;;invalid filter
(ex-message @(fluree/query {} '{:select ['?name '?age]
:where [['?s :type :ex/User]
['?s :schema/age '?age]
['?s :schema/name '?name]
{:filter "(> ?age 45)"}]}))

;;invalid filter
(ex-message @(fluree/query {} '{:select ['?name '?age]
:where [['?s :type :ex/User]
['?s :schema/age '?age]
['?s :schema/name '?name]
{:filter :foo}]}))

)
6 changes: 3 additions & 3 deletions src/fluree/db/api/query.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@
(catch* e
(throw
(ex-info
(str "History query not properly formatted. Provided "
(pr-str query-map))
(-> e
v/explain-error
(v/format-explained-errors nil))
{:status 400
:message (v/humanize-error e)
:error :db/invalid-query}))))
history-query (cond-> coerced-query did (assoc-in [:opts :did] did))]
(<? (history* db history-query)))))
Expand Down
74 changes: 26 additions & 48 deletions src/fluree/db/query/fql/syntax.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,34 +114,38 @@
::var ::v/var
::iri ::v/iri
::subject ::v/subject
::subselect-map [:map-of ::iri [:ref ::subselection]]
::subselection [:sequential [:orn
[:wildcard ::wildcard]
[:predicate ::iri]
[:subselect-map [:ref ::subselect-map]]]]
::select-map [:map-of {:max 1}
::subselect-map [:map-of {:error/message "must be map from iri to subselection"}
::iri [:ref ::subselection]]
::subselection [:sequential {:error/message
"subselection must be a vector"}
[:orn {:error/message "subselection must be a wildcard (\"*\") or subselection map"}
[:wildcard ::wildcard]
[:predicate ::iri]
[:subselect-map [:ref ::subselect-map]]]]
::select-map [:map-of {:max 1
:error/message "Only one key/val for select-map"}
::var ::subselection]
::selector [:orn
::selector [:orn {:error/message "selector must be either a variable, wildcard symbol (`*`), iri, function application, or select map"}
[:wildcard ::wildcard]
[:var ::var]
[:aggregate ::function]
[:select-map ::select-map]]
::select [:orn
::select [:orn {:error/message "Select must be a valid selector or vector of selectors"}
[:selector ::selector]
[:collection [:sequential ::selector]]]
::direction [:orn
::direction [:orn {:error/message "Direction must be ASC or DESC"}
[:asc [:fn asc?]]
[:desc [:fn desc?]]]
::ordering [:orn
::ordering [:orn {:error/message "Ordering must be a var or two-tuple formatted ['ASC' or 'DESC', var]"}
[:scalar ::var]
[:vector [:and list?
[:catn
[:direction ::direction]
[:dimension ::var]]]]]
::order-by [:orn
::order-by [:orn {:error/message "orderBy clause must be variable or two-tuple formatted ['ASC' or 'DESC', var]"}
[:clause ::ordering]
[:collection [:sequential ::ordering]]]
::group-by [:orn
::group-by [:orn {:error/message "groupBy clause must be a variable or a vector of variables"}
[:clause ::var]
[:collection [:sequential ::var]]]
::triple ::v/triple
Expand All @@ -164,52 +168,26 @@
(def coerce-query*
(m/coercer ::query (mt/transformer {:name :fql}) {:registry registry}))



(defn humanize-error
[error]
(let [explain-data (-> error ex-data :data :explain)]
(let [explain-data (v/explain-error error)]
(log/trace "query validation error:"
(update explain-data :errors
(fn [errors] (map #(dissoc % :schema) errors))))
(-> explain-data
(me/humanize
{:errors
(-> me/default-errors
(assoc
::m/missing-key
{:error/fn
(fn [{:keys [in]} _]
(let [k (-> in last name)]
(str "Query is missing a '" k "' clause. "
"'" k "' is required in queries. "
"See documentation here for details: "
docs/error-codes-page "#query-missing-" k)))}
::m/extra-key
{:error/fn
(fn [{:keys [in]} _]
(let [k (-> in last name)]
(str "Query contains an unknown key: '" k "'. "
"See documentation here for more information on allowed query keys: "
docs/error-codes-page "#query-unknown-key")))}))}))))

(defn coalesce-query-errors
[humanized-errors qry]
(str "Invalid query: " (pr-str qry) " - "
(str/join "; "
(if (map? humanized-errors)
(map (fn [[k v]]
(str (name k) ": "
(str/join " " v))) humanized-errors)
humanized-errors))))
(v/format-explained-errors nil))))


(defn coerce-query
[qry]
(try*
(coerce-query* qry)
(catch* e
(let [he (humanize-error e)
_ (log/trace "humanized errors:" he)
error-msg (coalesce-query-errors he qry)]
(throw (ex-info error-msg {:status 400, :error :db/invalid-query}))))))
(coerce-query* qry)
(catch* e
(let [error-msg (humanize-error e)
_ (log/trace "humanized errors:" error-msg)]
(throw (ex-info error-msg {:status 400, :error :db/invalid-query}))))))

(def parse-selector
(m/parser ::selector {:registry registry}))
Expand Down
59 changes: 35 additions & 24 deletions src/fluree/db/query/history.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(ns fluree.db.query.history
(:require [clojure.core.async :as async :refer [go >! <!]]
[clojure.string :as str]
[malli.core :as m]
[fluree.json-ld :as json-ld]
[fluree.db.constants :as const]
Expand All @@ -10,6 +11,7 @@
[fluree.db.query.json-ld.response :as json-ld-resp]
[fluree.db.util.async :refer [<? go-try]]
[fluree.db.util.core :as util #?(:clj :refer :cljs :refer-macros) [try* catch*]]
[fluree.db.util.docs :as docs]
[fluree.db.util.log :as log]
[fluree.db.query.range :as query-range]
[fluree.db.db.json-ld :as jld-db]
Expand All @@ -24,14 +26,18 @@
it wants to require, which are not required/supported here in the db library."
[extra-kvs]
[:and
[:map-of ::json-ld-keyword :any]
[:map-of ::json-ld-keyword :any]
[:fn {:error/message "Must supply a value for either \"history\" or \"commit-details\""}
(fn [{:keys [history commit-details t]}]
(or (string? history) (keyword? history) (seq history) commit-details))]
(into
[:map
[:history {:optional true}
[:orn
[:subject ::iri]
[:orn {:error/message
"Value of \"history\" must be a subject, or a vector containing one or more of subject, predicate, object"}
[:subject {:error/message "Invalid iri"} ::iri]
[:flake
[:or
[:or {:error/message "Must provide a tuple of one more more iris"}
[:catn
[:s ::iri]]
[:catn
Expand All @@ -41,39 +47,44 @@
[:s [:maybe ::iri]]
[:p ::iri]
[:o [:not :nil]]]]]]]
[:commit-details {:optional true} :boolean]
[:commit-details {:optional true
:error/message "Invalid value of \"commit-details\" key"} :boolean]
[:context {:optional true} ::context]
[:opts {:optional true} [:map-of :keyword :any]]
[:t
[:and
[:map-of :keyword :any]
[:map-of {:error/message "Value of \"t\" must be a map"} :keyword :any]
[:map
[:from {:optional true} [:or
[:= :latest]
[:int {:min 0}]
[:re datatype/iso8601-datetime-re]]]
[:to {:optional true} [:or
[:= :latest]
[:int {:min 0}]
[:re datatype/iso8601-datetime-re]]]
[:at {:optional true} [:or
[:= :latest]
[:int {:min 0}]
[:re datatype/iso8601-datetime-re]]]]
[:fn {:error/message "Either \"from\" or \"to\" `t` keys must be provided."}
[:from {:optional true}
[:or {:error/message "Value of \"from\" must be one of: the key latest, an integer > 0, or an iso-8601 datetime value"}
[:= :latest]
[:int {:min 0
:error/message "Must be a positive value"} ]
[:re datatype/iso8601-datetime-re]]]
[:to {:optional true}
[:or {:error/message "Value of \"to\" must be one of: the key latest, an integer > 0, or an iso-8601 datetime value"}
[:= :latest]
[:int {:min 0
:error/message "Must be a positive value"}]
[:re datatype/iso8601-datetime-re]]]
[:at {:optional true}
[:or {:error/message "Value of \"at\" must be one of: the key latest, an integer > 0, or an iso-8601 datetime value"}
[:= :latest]
[:int {:min 0
:error/message "Must be a positive value"} ]
[:re datatype/iso8601-datetime-re]]]]
[:fn {:error/message "Must provide: either \"from\" or \"to\", or the key \"at\" "}
(fn [{:keys [from to at]}]
;; if you have :at, you cannot have :from or :to
(if at
(not (or from to))
(or from to)))]
[:fn {:error/message "\"from\" value must be less than or equal to \"to\" value."}
[:fn {:error/message "\"from\" value must be less than or equal to \"to\" value,"}
(fn [{:keys [from to]}] (if (and (number? from) (number? to))
(<= from to)
true))]]]]
extra-kvs)
[:fn {:error/message "Must supply either a :history or :commit-details key."}
(fn [{:keys [history commit-details t]}]
(or history commit-details))]])
extra-kvs)])


(def registry
(merge
Expand Down
Loading