Handle free form objects

[omartell]

At the moment if you have a swagger property like this :

{:Pet {:type  "object" :properties {}}}

The Plumatic schema that you get is:

{:Pet {}}

This only allows an empty map as a posible value, which is quite restrictive and probably not very useful.

So, this PR changes the existing code so that if you have no explicit properties, it will produce the following to allow any value:

{:Pet s/Any}

Happy to hear your thoughts on the approach and implementation. Relevant swagger docs for free form objects https://swagger.io/docs/specification/data-models/data-types#free-form.

Oliver

[oliyh]

Hi Oliver,

Thanks for the PR. Interesting, my initial thought was that it's done the right thing - the swagger definition is saying that it's an object (map) with no keys.

I would imagine Swagger has a more explicit way of describing a blob, I would want to find out what that was first and making sure Martian supported it before accepting this change. I'll try to find that out soon and reply, unless you have time first.

[oliyh]

By the way how much control over this do you have on the server side, are you using pedestal-api or compojure-api or something else that uses metosin's swagger generation?

[omartell]

Thanks for the reply Oliy.

The server is using compojure-api. Basically the schema on the server looks like this:

{:conversion {s/Any s/Any}
(s/optional-key :contact) {s/Any s/Any}
(s/optional-key :account) {s/Any s/Any}}

And the only way I can make martian interact with the endpoint is by submitting:

{:conversion {}}

[omartell]

The only relevant swagger information I could find was the free form object section on here https://swagger.io/docs/specification/data-models/data-types#free-form. Maybe that helps.

[oliyh]

Hi,

Thanks for the research. Yes, additionalProperties seems to be the key which describes what you want, I think it makes sense to get that behaviour into the swagger generation library and support it in Martian.

In the meantime all this is captured as data in Martian so it is possible to reach in and override it. I'll think a bit more about this.

[omartell]

For more context this is an example swagger schema with those properties:

{:swagger     "2.0",
 :info        {:title "Swagger API", :version "0.0.1"},
 :produces    ["application/json"
               "application/transit+msgpack"
               "application/transit+json"
               "application/edn"],
 :consumes    ["application/json"
               "application/transit+msgpack"
               "application/transit+json"
               "application/edn"],
 :basePath    "/",
 :paths       {:api/create-pet {:post {:operationId "create-pet",
                                       :parameters  [{:in          "body",
                                                      :name        "Body50766",
                                                      :description "",
                                                      :required    true,
                                                      :schema      {:$ref "#/definitions/Body50766"}}],
                                       :responses   {:default {:description ""}}}}},
 :definitions {:Body50766      {:type                 "object",
                                :properties           {:name  {:$ref "#/definitions/Body50766Name"},
                                                       :owner {:$ref "#/definitions/Body50766Owner"},
                                                       :age   {:$ref "#/definitions/Body50766Age"}},
                                :additionalProperties false,
                                :required             ["name"]},
               :Body50766Name  {:type "object", :additionalProperties {}},
               :Body50766Owner {:type "object", :additionalProperties {}},
               :Body50766Age   {:type "object", :additionalProperties {}}}}

My thinking was that the issue was within martian as the previous swagger schema works fine through the swagger UI. Swagger is basically allowing to add any kv to name, owner and age, but martian is more restrictive by only allowing the empty map {}. Perhaps the schema I suggested before s/Any is not right, and it should be {s/Any s/Any} to model an object as the swagger spec does. Ideally we should look at additionalProperties to support the free form object.

[oliyh]

Yes, your proposal sounds good, and the swagger schema looks as I expected it to as well. Also, we should merge {s/Any s/Any} with whatever keys are defined.

[omartell]

Hey @oliyh, sorry for the delay. I just pushed an update, let me know what you think of the code and if you think there's anything missing/could be improved.

[oliyh]

Hi @omartell

Thanks for this, it looks great - just one small point I have questioned, could you check if my assumption is correct?

Cheers

[omartell]

@oliyh just pushed an update removing the or.

[oliyh]

Thanks for this, it's now on clojars 0.1.11-SNAPSHOT

[omartell]

Thanks for merging @oliyh