ring-swagger & compojure-api
mgrbyte 2019-03-19T10:03:04.034800Z

I've run into a case where it's not set (in a custom exception handler for the :compojure.api.exception/request-validation error). I'll try and find time to knock up an example repo with minimal steps to reproduce, but am vacation from tomorrow, so might not be for a week or so.

slipset 2019-03-19T11:32:43.036100Z

It’s that time of year again, so I’m playing with compojure-api 🙂

slipset 2019-03-19T11:32:54.036400Z

And I have this route:

slipset 2019-03-19T11:32:56.036700Z

(GET "/" {system :system :as request}
      :coercion :spec 
      :summary "Returns all the things"
      :return (spec-tools/merge ::specs/identifiable
                                (spec/keys :opt-un [::specs/description]))

ikitommi 2019-03-19T11:33:29.037200Z

e.g. the exception handler happens before the request-injection. I presume it’s safe to reverse the order. PR welcome, after the vacations 🙂

slipset 2019-03-19T11:33:57.037800Z

Which shows up in the swagger ui, which is great, but it doesn’t show anything useful in for the return value, it only shows {}

slipset 2019-03-19T11:34:53.038700Z

If I change to

(GET "/" {system :system :as request}
      :coercion :spec 
      :summary "Returns all the things"
      :return (spec/merge ::specs/identifiable
                                (spec/keys :opt-un [::specs/description]))

slipset 2019-03-19T11:35:08.039200Z

I get

  "_id": "string",
  "name": "string",
  "description": "string"

slipset 2019-03-19T11:35:11.039500Z

as expected.

ikitommi 2019-03-19T11:35:28.039800Z

You can run the transformation from repl, something like:

(require '[spec-tools.swagger.core :as swagger])
(swagger/transform (spec-tools/merge ::specs/identifiable
                                (spec/keys :opt-un [::specs/description])))

ikitommi 2019-03-19T11:35:34.040100Z

… to see what pops up.

slipset 2019-03-19T11:37:00.041300Z

ardoq.api.crud-api-v2> (swagger/transform (spec/merge ::specs/identifiable
                                (spec/keys :opt-un [::specs/description])))

;; => {:type "object",
  {:type "string",
   :x-allOf [{:type "string"} {}],
   :title "ardoq.specs/_id"},
  "name" {:type "string"},
  "description" {:type "string", :x-nullable true}},
 :required ["_id" "name"]}
ardoq.api.crud-api-v2> (swagger/transform (spec-tools/merge ::specs/identifiable
                                (spec/keys :opt-un [::specs/description])))
;; => {}

ikitommi 2019-03-19T11:37:41.041600Z

oh, and no tests for st/merge either: https://github.com/metosin/spec-tools/blob/master/test/cljc/spec_tools/swagger/core_test.cljc

slipset 2019-03-19T11:38:10.042400Z

and somewhat involved, IIRC 🙂

ikitommi 2019-03-19T11:38:19.042500Z

now that you know it’s not working properly, would you have time to make a PR? 😉

slipset 2019-03-19T11:38:45.043100Z

I have time to make an issue, but I don’t know if I’m capable of creating a PR.

ikitommi 2019-03-19T11:39:42.044Z

Issue is good, I should have time to check if that’s a small fix or needs more time. most likely just missing a multimethod dispatch for that.

ikitommi 2019-03-19T11:39:53.044200Z

but now just now (the time)

slipset 2019-03-19T11:45:11.044400Z

user> (defmethod swagger/accept-spec 'spec-tools.core/merge [_ _ children _]
  ;; Use x-anyOf and x-allOf instead of normal versions
  {:type "object"
   :properties (->> (concat children
                            (mapcat :x-anyOf children)
                            (mapcat :x-allOf children))
                    (map :properties)
                    (reduce merge {}))
   ;; Don't include top schema from s/or.
   :required (->> (concat (remove :x-anyOf children)
                          (mapcat :x-allOf children))
                  (map :required)
                  (reduce into (sorted-set))
                  (into []))})
;; => #multifn[accept-spec 0x3f8c13a8]
user> (swagger/transform (spec-tools/merge ::has-string ::has-int))
;; => {:type "object", :properties {}, :required []}

slipset 2019-03-19T11:45:35.044700Z

Closer, but still no cigar 🙂

ikitommi 2019-03-19T11:47:00.045800Z

you have mad skills at extending libraries 🙂

slipset 2019-03-19T11:47:25.046200Z

children is nil for spec-tools.core/merge

ikitommi 2019-03-19T11:48:23.046800Z

you need to check also from the visitor ns that there is a mm dispatch to walk the childs.

ikitommi 2019-03-19T11:49:19.048Z

it goes: 1) visitor walks the specs (need an mm for all form symbols) 2) json-schema is used as a base transfromation 3) swagger overrides the stuff that it can’t handle

slipset 2019-03-19T11:55:01.048500Z

You Rock!!

slipset 2019-03-19T11:55:51.048700Z

user> (swagger/transform (spec-tools/merge ::has-string ::has-int))
;; => {:type "object",
 {"string" {:type "string"},
  "int" {:type "integer", :format "int64"}},
 :required ["int" "string"]}

slipset 2019-03-19T11:56:51.049Z

I might just have time to make a pr after all 🙂

ikitommi 2019-03-19T12:17:53.049200Z

awesome! 🙂

slipset 2019-03-19T13:20:10.049500Z

seems to be the same case for spec/coll-of

slipset 2019-03-19T16:05:22.050Z

I’ve got another sort of weird case:

slipset 2019-03-19T16:05:50.050200Z

(POST "/" {{document :json-params} :params
               system :system :as request}
      :coercion :no-coercion
      :summary "Create one thing"
      :body [document spec]
      :return  (spec/merge spec ::specs/metadata)
      (create* (service-kw system) (request-util/->context request) spec document)))))

slipset 2019-03-19T16:07:37.051200Z

only returns the first of the merged specs. It works as expected if :return only contains spec (which is also a result of a merged spec)

slipset 2019-03-19T16:08:21.051900Z

I’ll be happy to file an issue for this if it’s supposed to be supported like this.

slipset 2019-03-19T16:59:15.052300Z

Here’s the issue for coll-of https://github.com/metosin/spec-tools/issues/175