From e095cd2efa54ad270ca0831a9d16aa5e1299ab51 Mon Sep 17 00:00:00 2001 From: Timo Kramer Date: Fri, 13 Nov 2020 11:22:07 +0100 Subject: [PATCH] Support operationId in reitit-swagger OpenAPI Specification allows the operationId to be added to the "Operation Object" alongside e.g. summary and description. This commit introduces the support of this element in the reitit-swagger module and extends the tests. One test shows the correct use of operationId where both are distinct and one shows the failing of the swagger creation when the IDs are not distinct. - Spec: https://swagger.io/specification/#operation-object - Adds the support for operationId - Adds operationId in two places of the swagger test - Adds a test that checks exception on duplicate IDs - Closes #451 --- .../reitit-swagger/src/reitit/swagger.cljc | 17 +++++++-- test/cljc/reitit/swagger_test.clj | 38 ++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/modules/reitit-swagger/src/reitit/swagger.cljc b/modules/reitit-swagger/src/reitit/swagger.cljc index 998457ac..cc6f4e8e 100644 --- a/modules/reitit-swagger/src/reitit/swagger.cljc +++ b/modules/reitit-swagger/src/reitit/swagger.cljc @@ -12,9 +12,11 @@ (s/def ::tags (s/coll-of (s/or :keyword keyword? :string string?) :kind #{})) (s/def ::summary string?) (s/def ::description string?) +(s/def ::operationId string?) +(s/def ::operationIds (s/coll-of ::operationId :distinct true)) (s/def ::swagger (s/keys :opt-un [::id])) -(s/def ::spec (s/keys :opt-un [::swagger ::no-doc ::tags ::summary ::description])) +(s/def ::spec (s/keys :opt-un [::swagger ::no-doc ::tags ::summary ::description ::operationId])) (def swagger-feature "Feature for handling swagger-documentation for routes. @@ -75,13 +77,14 @@ (let [{:keys [id] :or {id ::default} :as swagger} (-> match :result request-method :data :swagger) ids (trie/into-set id) strip-top-level-keys #(dissoc % :id :info :host :basePath :definitions :securityDefinitions) - strip-endpoint-keys #(dissoc % :id :parameters :responses :summary :description) + strip-endpoint-keys #(dissoc % :id :parameters :responses :summary :description :operationId) swagger (->> (strip-endpoint-keys swagger) (merge {:swagger "2.0" :x-id ids})) accept-route (fn [route] (-> route second :swagger :id (or ::default) (trie/into-set) (set/intersection ids) seq)) base-swagger-spec {:responses ^:displace {:default {:description ""}}} + oid-acc (atom []) transform-endpoint (fn [[method {{:keys [coercion no-doc swagger] :as data} :data middleware :middleware interceptors :interceptors}]] @@ -94,12 +97,20 @@ (if coercion (coercion/get-apidocs coercion :swagger data)) (select-keys data [:tags :summary :description]) + (let [oid (select-keys data [:operationId]) + oid-val (:operationId oid) + _ (when (not (nil? oid-val)) + (reset! oid-acc (conj @oid-acc oid-val)))] + oid) (strip-top-level-keys swagger))])) transform-path (fn [[p _ c]] (if-let [endpoint (some->> c (keep transform-endpoint) (seq) (into {}))] [(swagger-path p (r/options router)) endpoint])) map-in-order #(->> % (apply concat) (apply array-map)) - paths (->> router (r/compiled-routes) (filter accept-route) (map transform-path) map-in-order)] + paths (->> router (r/compiled-routes) (filter accept-route) (map transform-path) map-in-order) + _ (when (not (s/valid? ::operationIds @oid-acc)) + (throw (ex-info (s/explain-str ::operationIds @oid-acc) {:operation-ids @oid-acc + :error "operationIds are not distinct"})))] {:status 200 :body (meta-merge swagger {:paths paths})})) ([req res raise] diff --git a/test/cljc/reitit/swagger_test.clj b/test/cljc/reitit/swagger_test.clj index b87c695c..4adafdc9 100644 --- a/test/cljc/reitit/swagger_test.clj +++ b/test/cljc/reitit/swagger_test.clj @@ -1,5 +1,5 @@ (ns reitit.swagger-test - (:require [clojure.test :refer [deftest is testing]] + (:require [clojure.test :refer :all] [reitit.ring :as ring] [reitit.swagger :as swagger] [reitit.swagger-ui :as swagger-ui] @@ -25,11 +25,13 @@ ["/spec" {:coercion spec/coercion} ["/plus/:z" {:patch {:summary "patch" + :operationId "Patch" :handler (constantly {:status 200})} :options {:summary "options" :middleware [{:data {:swagger {:responses {200 {:description "200"}}}}}] :handler (constantly {:status 200})} :get {:summary "plus" + :operationId "GetPlus" :parameters {:query {:x int?, :y int?} :path {:z int?}} :swagger {:responses {400 {:schema {:type "string"} @@ -101,6 +103,32 @@ rrc/coerce-request-middleware rrc/coerce-response-middleware]}}))) +(def failing-app + (ring/ring-handler + (ring/router + ["/api" + {:swagger {:id ::math}} + + ["/swagger.json" + {:get {:no-doc true + :swagger {:info {:title "my-api"}} + :handler (swagger/create-swagger-handler)}}] + + ["/spec" {:coercion spec/coercion} + ["/plus/:z" + {:patch {:summary "patch" + :operationId "Patch" + :handler (constantly {:status 200})} + :options {:summary "options" + :operationId "Patch" + :middleware [{:data {:swagger {:responses {200 {:description "200"}}}}}] + :handler (constantly {:status 200})}}]]] + + {:data {:middleware [swagger/swagger-feature + rrc/coerce-exceptions-middleware + rrc/coerce-request-middleware + rrc/coerce-response-middleware]}}))) + (require '[fipp.edn]) (deftest swagger-test (testing "endpoints work" @@ -118,6 +146,10 @@ (app {:request-method :get :uri "/api/schema/plus/3" :query-params {:x "2", :y "1"}}))))) + + (testing "failing swagger-spec" + (is (thrown? clojure.lang.ExceptionInfo (:body (failing-app {:request-method :get + :uri "/api/swagger.json"}))))) (testing "swagger-spec" (let [spec (:body (app {:request-method :get :uri "/api/swagger.json"})) @@ -126,6 +158,7 @@ :info {:title "my-api"} :paths {"/api/spec/plus/{z}" {:patch {:parameters [] :summary "patch" + :operationId "Patch" :responses {:default {:description ""}}} :options {:parameters [] :summary "options" @@ -156,7 +189,8 @@ 400 {:schema {:type "string"} :description "kosh"} 500 {:description "fail"}} - :summary "plus"} + :summary "plus" + :operationId "GetPlus"} :post {:parameters [{:in "body", :name "body", :description "",