Skip to content

Commit

Permalink
Merge pull request #598 from fluree/feature/format-validation-errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@mpoffald
mpoffald authored Oct 25, 2023
2 parents 7e36a02 + 80bbb5a commit 4ff41ae
Show file tree
Hide file tree
Showing 7 changed files with 422 additions and 305 deletions.
146 changes: 146 additions & 0 deletions dev/errors.clj
View file
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
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
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
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

0 comments on commit 4ff41ae

Please sign in to comment.