metosin / reitit

A fast data-driven routing library for Clojure/Script
https://cljdoc.org/d/metosin/reitit/
Eclipse Public License 1.0
1.4k stars 254 forks source link

Single item lists in query params with spec coercion #298

Open kalekale opened 5 years ago

kalekale commented 5 years ago

When defining a route with an array value in the query params like so:

{:get {:parameters {:query {:strings [string?]}}
           :handler (fn [{{:keys [query]} :parameters}]
                            {:status 200
                             :body query})}}

the query string is expected to look something like ?strings=hello&strings=world. However when only one is sent the query string will be ?strings=hello. Causing reitit to spit out

{
  "spec": "(spec-tools.core/spec {:spec (clojure.spec.alpha/keys :req-un [:spec$367119/strings]), :type :map, :leaf? false})",
  "problems": [
    {
      "path": [
        "strings"
      ],
      "pred": "clojure.core/coll?",
      "val": "hello",
      "via": [
        "spec$367119/strings"
      ],
      "in": [
        "strings"
      ]
    }
  ],
  "type": "reitit.coercion/request-coercion",
  "coercion": "spec",
  "value": {
    "strings": "hello"
  },
  "in": [
    "request",
    "query-params"
  ]
}

Maybe reitit should be able to coerce ?strings=hello to ["hello"] if the spec is (coll-of string?)?

ikitommi commented 5 years ago

Indeed. PR welcome (to https://github.com/metosin/spec-tools)!

miikka commented 5 years ago

Fixed this in our project by creating a custom coercion with a custom transformer:

(defn singleton->vector [_ x] (if (vector? x) x [x]))

(def string-type-decoders
 (assoc stt/string-type-decoders :vector singleton->vector))

(def string-transformer
 (st/type-transformer
   st/strip-extra-keys-transformer
   (st/type-transformer
     {:name            ::string
      :decoders        string-type-decoders
      :encoders        stt/string-type-encoders
      :default-encoder stt/any->any})))

(def spec-coercion
  (let [options {:transformers {:string {:default string-transformer}}}]
    (reitit.coercion.spec/create (potpuri.core/deep-merge reitit.coercion.spec/default-options options))))

;; use spec-coercion as the :coercion metadata for your routes

Is there a simpler way to add one decoder to the string-transformer? There's a lot of boilerplate here.

ikitommi commented 5 years ago

Solutions looks clean and the boilerplate is evident. Downside of explicit options and clear separation of layers.. Ideas welcome on easier config. Anyway, would be great addition to the docs how it can be done.

andrewsuzuki commented 4 years ago

Alternatively, could [:parameters :query] just draw from :params instead of :query-params? The former is likely already passed through ring middleware like wrap-nested-params.

MokkeMeguru commented 3 years ago

I faced on this problem in multipart. I updated multipart intercepter's code like this https://github.com/MokkeMeguru/reitit-with-multiple-input-in-formdata/blob/main/src/reitit_swagger_playground/multipart.clj#L43-L96

(defn multipart-interceptor
  "Creates a Interceptor to handle the multipart params, based on
  ring.middleware.multipart-params, taking same options. Mounts only
  if endpoint has `[:parameters :multipart]` defined. Publishes coerced
  parameters into `[:parameters :multipart]` under request.
  options:
  - :force-vectorize-keys ... vector of vectorize key
    if you have the parameter gets multiple inputs like :files gets some image files,
    you can use this option like [:files]
  "
  ([]
   (multipart-interceptor nil))
  ([options]
   {:name ::multipart
    :spec ::parameters
    :compile (fn [{:keys [parameters coercion]} opts]
               (if-let [multipart (:multipart parameters)]
                 (let [parameter-coercion {:multipart (coercion/->ParameterCoercion
                                                       :multipart-params :string true true)}
                       opts (assoc opts ::coercion/parameter-coercion parameter-coercion)
                       coercers (if multipart (coercion/request-coercers coercion parameters opts))
                       force-vectorize-keys (map name (:force-vectorize-keys options))]
                   {:data {:swagger {:consumes ^:replace #{"multipart/form-data"}}}
                    :enter (fn [ctx]
                             (let [raw-request (:request ctx)
                                   parsed-request (multipart-params/multipart-params-request raw-request options)
                                   parsed-request (if-let [{:keys [multipart-params]} parsed-request]
                                                    (assoc parsed-request
                                                           :multipart-params
                                                           (loop [mp  multipart-params
                                                                  fvk force-vectorize-keys]
                                                             (if (zero? (count fvk))
                                                               mp
                                                               (recur
                                                                (apply-singleton->vector mp (first fvk))
                                                                (rest fvk)))))
                                                    parsed-request)
                                   request (coerced-request parsed-request coercers)]
                               (assoc ctx :request request)))})))}))

In my opinion, the parameters after disassembly need to be modified based on the specifications. so that, I think you will need to send detailed specs to the interceptor. However, we need to support schema, spec, spec-tools etc...

iarenaza commented 2 years ago

Based on @miikka solution for spec-tools-based custom coercion, we have built the following solution for a malli`-based custom coercion, in case it's useful for others:

(defn singleton->vector
  [x]
  (if (string? x)
    (if (vector? x) x [x])
    x))

(def custom-string-type-decoders
  (assoc (mt/-string-decoders) :vector singleton->vector))

(def custom-string-transformer
  (mt/transformer
   {:name            :string
    :decoders        custom-string-type-decoders
    :encoders        mt/-string-decoders}))

(def custom-malli-coercion
  (rcm/create (assoc-in rcm/default-options
                        [:transformers :string :default]
                        custom-string-transformer)))
olymk2 commented 4 months ago

I have hit something similar to this when setting a query param as :vector or :sequence it works in swagger sending the values in this comma delimited format, ?key=value1,value2 but it returns invalid value error on these values using malli in this situation to validate the data.

SevereOverfl0w commented 1 month ago

I ended up with these router options to make this work with malli, and only apply to query parameters, avoiding the pitfall of also applying to headers, etc.

{:data {:coercion (reitit.c.malli/create
                    (-> reitit.c.malli/default-options
                        (assoc-in [:transformers :query :default]
                                  (malli.transform/transformer
                                    {:name :->collection
                                     :decoders {:sequential (fn [x] (if (sequential? x) x [x]))}
                                     :encoders {}}
                                    malli.transform/string-transformer))))
::reitit.c/parameter-coercion
(assoc reitit.c/default-parameter-coercion
       :query (reitit.c/->ParameterCoercion :query-params :query true true))})