Body params should be taken from supplied values and should not be named

[oliyh]

Currently martian attempts to give the body params a name and expects values in a submap under that name, e.g. (request-for m {:pet {:name "charlie" :age 2}}) where :pet is derived from the schema.

It should not do this; it is not robust and does not fit with the aim of abstracting HTTP structure from the function call. Instead, it should allow this: (request-for m {:name "charlie" :age 2}).

[markdingram]

Hi @oliyh, either I'm missing something (which is entirely possible!), but wrapping the body params still seems to be required when loading from a http swagger JSON,

Referring back to:
https://juxt.pro/blog/posts/martian.html

(-> (martian-http/bootstrap-swagger "https://pedestal-api.herokuapp.com/swagger.json")
    (martian/request-for :create-pet {:name "Doggy McDogFace" :type "Dog" :age 3}))

Gives:

ExceptionInfo Value cannot be coerced to match schema: {:pet missing-required-key}  schema.coerce/eval7131/coercer!--7136/fn--7137/fn--7138 (coerce.clj:52)```

Wrapping with the schema pet works:

(martian/request-for :create-pet {:pet {:name "Doggy McDogFace" :type "Dog" :age 3}}))

(running in a REPL from current master a6c9cc7)

I can see the test case still passes, so is there a subtle difference between data driven & http derived martian?

Thanks!

[oliyh]

Hi Mark, That behaviour changed in order to accommodate vector bodies in #26, as it was essentially destructuring the body param as a map which is not valid for arrays. The blog post is out of date, and actually the readme is too, thanks for pointing it out. You should be using `:pet` as the parameter as you've already worked out. You can inspect individual routes by using the `explore` function or just directly inspecting the Martian record if you think something has been interpreted strangely.

…

On 8 Dec 2016 12:41 p.m., "Mark Ingram" ***@***.***> wrote: Hi @oliyh <https://github.com/oliyh>, either I'm missing something (which is entirely possible!), but wrapping the body params still seems to be required when loading from a http swagger JSON, Referring back to: https://juxt.pro/blog/posts/martian.html (-> (martian-http/bootstrap-swagger "https://pedestal-api.herokuapp.com/swagger.json") (martian/request-for :create-pet {:name "Doggy McDogFace" :type "Dog" :age 3})) Gives: ExceptionInfo Value cannot be coerced to match schema: {:pet missing-required-key} schema.coerce/eval7131/coercer!--7136/fn--7137/fn--7138 (coerce.clj:52)``` Wrapping with the schema pet works: (martian/request-for :create-pet {:pet {:name "Doggy McDogFace" :type "Dog" :age 3}})) (running in a REPL from current master a6c9cc7 <a6c9cc7> ) I can see the test case still passes, so is there a subtle difference between data driven & http derived martian? Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB9S46FqE-TSHAtZNnUov4o9auUinfwAks5rF_sMgaJpZM4I1p2V> .

[markdingram]

Ahh thanks.

The underlying reason I was looking here was that in my compojure-api derived swagger there are some anonymous body schemas that end up being named some random variant of Body12345, Body45245 etc.

Like you mention it is no problem to pull out the name from Martian e.g.:

(-> (martian/explore m :create-pet) (:parameters) (first) (key))

...but feels a little clunky.

I wonder if a :martian.core/body key could be added that always resolves to the body schema? If you like the approach or suggest something better I can do the PR.

(martian/request-for :create-pet {::martian/body {:name "Doggy McDogFace" :type "Dog" :age 3}}))

[oliyh]

Hi @markdingram, I've gone back and forth on this a couple of times (as you can tell) without a satisfactory solution. I think I prefer the way it currently works but as you point out, things are not always named properly which can be a pain. I think your proposal of a namespaced keyword is reasonable as an alternative though, perhaps taking precedence over the schema name rather than replacing it. By the way have you checked you are all up to date on compojure-api and its dependencies? I raised the naming issue in frankiesardo/route-swagger#7 where @ikitommi suggested things would be named sensibly, perhaps you need to def them separately for naming rather than inline in your compojure routes?

[markdingram]

I created a PR #30 with the option where the namespaced keyword takes precedence, falling back to the named schema otherwise. This seems to be a nice simple path forward?

I am using a pretty recent compojure-api 1.1.8, which pulls ring-swagger 0.22.10 - these include any changes made by the issue you mentioned above.