From e095cd2efa54ad270ca0831a9d16aa5e1299ab51 Mon Sep 17 00:00:00 2001 From: Timo Kramer Date: Fri, 13 Nov 2020 11:22:07 +0100 Subject: [PATCH 1/4] 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 "", From 1b583c1cc2f87ce958dd32cf60f6dbc3ee11c888 Mon Sep 17 00:00:00 2001 From: Miikka Koskinen Date: Fri, 23 Apr 2021 17:02:17 +0300 Subject: [PATCH 2/4] Remove operation-id uniqueness check Let's leave that for other tools for now. --- .../reitit-swagger/src/reitit/swagger.cljc | 14 ++------- test/cljc/reitit/swagger_test.clj | 29 ------------------- 2 files changed, 2 insertions(+), 41 deletions(-) diff --git a/modules/reitit-swagger/src/reitit/swagger.cljc b/modules/reitit-swagger/src/reitit/swagger.cljc index cc6f4e8e..4a2a5319 100644 --- a/modules/reitit-swagger/src/reitit/swagger.cljc +++ b/modules/reitit-swagger/src/reitit/swagger.cljc @@ -13,7 +13,6 @@ (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 ::operationId])) @@ -84,7 +83,6 @@ 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}]] @@ -96,21 +94,13 @@ (apply meta-merge (keep (comp :swagger :data) interceptors)) (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) + (select-keys data [:tags :summary :description :operationId]) (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) - _ (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"})))] + paths (->> router (r/compiled-routes) (filter accept-route) (map transform-path) map-in-order)] {: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 4adafdc9..9c1ef38a 100644 --- a/test/cljc/reitit/swagger_test.clj +++ b/test/cljc/reitit/swagger_test.clj @@ -103,32 +103,6 @@ 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" @@ -147,9 +121,6 @@ :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"})) From 1b74373911fb2e2b314f6e4128431e7a4e6b29c3 Mon Sep 17 00:00:00 2001 From: Miikka Koskinen Date: Fri, 23 Apr 2021 17:15:35 +0300 Subject: [PATCH 3/4] Use explicit :refers --- test/cljc/reitit/swagger_test.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cljc/reitit/swagger_test.clj b/test/cljc/reitit/swagger_test.clj index 9c1ef38a..c141a489 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 :all] + (:require [clojure.test :refer [deftest is testing]] [reitit.ring :as ring] [reitit.swagger :as swagger] [reitit.swagger-ui :as swagger-ui] From 38ec679207081d49df7d6995b2e3b1bf3a85c1af Mon Sep 17 00:00:00 2001 From: Timo Kramer Date: Sat, 29 May 2021 11:48:57 +0200 Subject: [PATCH 4/4] Extend the docs to mention the operationID --- doc/ring/swagger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/ring/swagger.md b/doc/ring/swagger.md index 69006a23..d5767445 100644 --- a/doc/ring/swagger.md +++ b/doc/ring/swagger.md @@ -23,6 +23,7 @@ The following route data keys contribute to the generated swagger specification: | :tags | optional set of string or keyword tags for an endpoint api docs | :summary | optional short string summary of an endpoint | :description | optional long description of an endpoint. Supports http://spec.commonmark.org/ +| :operationId | optional string specifying the unique ID of an Operation Coercion keys also contribute to the docs: