weavejester / compojure

A concise routing library for Ring/Clojure
Eclipse Public License 1.0
4.08k stars 259 forks source link

parameter coercion can't work with schema validation #194

Open igotti opened 4 years ago

igotti commented 4 years ago

When dealing with an optional boolean parameter, string literals like #{"1" "y" "yes" "on" "true"}) treated as true while #{"0" "n" "no" "off" "false"}) as false and nil for others. i wrote a function named as-bool to coerce boolean literal refer to Destructuring-Syntax#parameter-coercion , while schema (s/maybe s/Bool) validates the parameter.

Failed the coercion when data binding if the boolean parameter isn't presented and route entirely as per wiki, there is no chance for schema validation comes to play.

Is it more reasonable to accept nil value at data binding and hand route flow over to schema validation at data validating?

weavejester commented 4 years ago

Sorry, how are you using Schema? Can you explain a little further?

igotti commented 4 years ago

what plumatic/schema is.

my sample code:

(api/defendpoint GET "/:card-id/alerts"
  "Getting alerts for card with specifications"
  [card-id enabled :<< as-bool :as {{:keys [page size] :or {page 1 size 20}} :params}]
  {enabled                (s/maybe s/Bool)
   page                   su/IntGreaterThanZero
   size                   su/IntGreaterThanZero}
  (do-something))

where s alias for schema.core and su things alike, api/defendpoint refers to:

(doc metabase.api.common/defendpoint)

([method route docstr? args schemas-map? & body]) Macro Define an API function. This automatically does several things:

weavejester commented 4 years ago

We can't break existing functionality in Compojure, so I see only two ways forward.

The first is to create another symbol or object to denote a missing value, and create an appropriate schema function. For example, (def missing (Object.)), or ::missing.

The second is to add in a new symbol for coercion that matches even on nil. For example, enabled :<<? as-bool.