Repository: oliyh/martian
Pull request #87 (2020-05-23 15:44:10)

Implement support for parameter refs
Implement support for parameter refs

oliyh: Hi,

Thanks so much for this contribution. You are on the right lines, I just left some feedback that would help simplify it. Some unit tests would be great, and then I'd be happy to merge it.

Cheers
Oliy
oliyh: Hi,

This looks really good now, I just left one last minor comment.

Thanks

Patch:
diff --git a/core/src/martian/schema.cljc b/core/src/martian/schema.cljc
index e52f190..9f0c868 100644
--- a/core/src/martian/schema.cljc
+++ b/core/src/martian/schema.cljc
@@ -79,27 +79,24 @@
 
 (defn schemas-for-parameters
   "Given a collection of swagger parameters returns a schema map"
-  [definitions parameters]
+  [ref-lookup parameters]
   (->> parameters
        (map (fn [{:keys [name required] :as param}]
               {(cond-> (keyword name)
                  (not required)
                  s/optional-key)
-               (make-schema definitions param)}))
+               (make-schema ref-lookup param)}))
        (into {})))
 
-(defn- resolve-ref [definitions ref]
-  (some->> ref
-           (re-find #"#/definitions/(.*)")
-           second
-           keyword
-           definitions))
+(defn- resolve-ref [ref-lookup ref]
+  (let [[_ category k] (re-find #"#/(definitions|parameters)/(.*)" ref)]
+    (get-in ref-lookup [(keyword category) (keyword k)])))
 
 (def URI
   #?(:clj java.net.URI
      :cljs goog.Uri))
 
-(defn- schema-type [definitions {:keys [type enum format $ref] :as param}]
+(defn- schema-type [ref-lookup {:keys [type enum format $ref] :as param}]
   (cond
     enum (apply s/enum enum)
     (= "string" type) (case format
@@ -108,7 +105,7 @@
                         s/Str)
     (= "integer" type) s/Int
     (= "boolean" type) s/Bool
-    (or (= "object" type) $ref) (make-schema definitions param)
+    (or (= "object" type) $ref) (make-schema ref-lookup param)
 
     :else
     s/Any))
@@ -117,7 +114,7 @@
 
 (defn make-schema
   "Takes a swagger parameter and returns a schema"
-  [definitions {:keys [name required type enum schema properties $ref items additionalProperties] :as param}]
+  [ref-lookup {:keys [name required type enum schema properties $ref items additionalProperties] :as param}]
 
   (if (let [ref (or $ref (:$ref schema))]
         (and ref (contains? *visited-refs* ref)))
@@ -126,27 +123,27 @@
     (cond
       $ref
       (binding [*visited-refs* (conj *visited-refs* $ref)]
-        (make-schema definitions (-> (dissoc param :$ref)
-                                     (merge (resolve-ref definitions $ref)))))
+        (make-schema ref-lookup (-> (dissoc param :$ref)
+                                    (merge (resolve-ref ref-lookup $ref)))))
 
       (:$ref schema)
       (binding [*visited-refs* (conj *visited-refs* (:$ref schema))]
-        (make-schema definitions (-> (dissoc param :schema)
-                                     (merge (resolve-ref definitions (:$ref schema))))))
+        (make-schema ref-lookup (-> (dissoc param :schema)
+                                    (merge (resolve-ref ref-lookup (:$ref schema))))))
 
       :else
       (cond-> (cond
                 (= "array" type)
-                [(schema-type definitions (assoc items :required true))]
+                [(schema-type ref-lookup (assoc items :required true))]
 
                 (= "array" (:type schema))
-                [(schema-type definitions (assoc (:items schema) :required true))]
+                [(schema-type ref-lookup (assoc (:items schema) :required true))]
 
                 (= "object" type)
-                (cond-> (schemas-for-parameters definitions (map (fn [[name p]] (assoc p :name name)) properties))
+                (cond-> (schemas-for-parameters ref-lookup (map (fn [[name p]] (assoc p :name name)) properties))
                   additionalProperties (assoc s/Any s/Any))
 
                 :else
-                (schema-type definitions param))
+                (schema-type ref-lookup param))
         (and (not required) (not= "array" type) (not= "array" (:type schema)))
         s/maybe))))
diff --git a/core/src/martian/swagger.cljc b/core/src/martian/swagger.cljc
index f14897f..a98acbe 100644
--- a/core/src/martian/swagger.cljc
+++ b/core/src/martian/swagger.cljc
@@ -6,31 +6,31 @@
             [schema.core :as s]
             #?(:cljs [cljs.reader :refer [read-string]])))
 
-(defn- body-schema [definitions swagger-params]
+(defn- body-schema [ref-lookup swagger-params]
   (when-let [body-params (not-empty (filter #(= "body" (:in %)) swagger-params))]
-    (schema/schemas-for-parameters definitions body-params)))
+    (schema/schemas-for-parameters ref-lookup body-params)))
 
-(defn- form-schema [definitions swagger-params]
+(defn- form-schema [ref-lookup swagger-params]
   (when-let [form-params (not-empty (filter #(= "formData" (:in %)) swagger-params))]
-    (schema/schemas-for-parameters definitions form-params)))
+    (schema/schemas-for-parameters ref-lookup form-params)))
 
-(defn- path-schema [definitions swagger-params]
+(defn- path-schema [ref-lookup swagger-params]
   (when-let [path-params (not-empty (filter #(= "path" (:in %)) swagger-params))]
-    (schema/schemas-for-parameters definitions path-params)))
+    (schema/schemas-for-parameters ref-lookup path-params)))
 
-(defn- query-schema [definitions swagger-params]
+(defn- query-schema [ref-lookup swagger-params]
   (when-let [query-params (not-empty (filter #(= "query" (:in %)) swagger-params))]
-    (schema/schemas-for-parameters definitions query-params)))
+    (schema/schemas-for-parameters ref-lookup query-params)))
 
-(defn- headers-schema [definitions swagger-params]
+(defn- headers-schema [ref-lookup swagger-params]
   (when-let [header-params (not-empty (filter #(= "header" (:in %)) swagger-params))]
-    (schema/schemas-for-parameters definitions header-params)))
+    (schema/schemas-for-parameters ref-lookup header-params)))
 
-(defn- response-schemas [definitions swagger-responses]
+(defn- response-schemas [ref-lookup swagger-responses]
   (for [[status response] swagger-responses
         :let [status-code (if (number? status) status (read-string (name status)))]]
     {:status (s/eq status-code)
-     :body (schema/make-schema definitions (assoc (:schema response) :required true))}))
+     :body (schema/make-schema ref-lookup (assoc (:schema response) :required true))}))
 
 (defn- sanitise [x]
   (if (string? x)
@@ -47,23 +47,24 @@
             %) parts)))
 
 (defn- ->handler
-  [{:keys [definitions] :as swagger-map}
+  [swagger-map
    path-item-parameters
    url-pattern
    [method swagger-definition]]
   (when-let [route-name (some-> (:operationId swagger-definition) ->kebab-case-keyword)]
-    (let [path-parts (tokenise-path url-pattern)
+    (let [ref-lookup (select-keys swagger-map [:definitions :parameters])
+          path-parts (tokenise-path url-pattern)
           uri (string/join (map str path-parts))
           parameters (concat path-item-parameters (:parameters swagger-definition))]
       {:path uri
        :path-parts path-parts
        :method method
-       :path-schema (path-schema definitions parameters)
-       :query-schema (query-schema definitions parameters)
-       :body-schema (body-schema definitions parameters)
-       :form-schema (form-schema definitions parameters)
-       :headers-schema (headers-schema definitions parameters)
-       :response-schemas (response-schemas definitions (:responses swagger-definition))
+       :path-schema (path-schema ref-lookup parameters)
+       :query-schema (query-schema ref-lookup parameters)
+       :body-schema (body-schema ref-lookup parameters)
+       :form-schema (form-schema ref-lookup parameters)
+       :headers-schema (headers-schema ref-lookup parameters)
+       :response-schemas (response-schemas ref-lookup (:responses swagger-definition))
        :produces (some :produces [swagger-definition swagger-map])
        :consumes (some :consumes [swagger-definition swagger-map])
        :summary (:summary swagger-definition)


Test patch:
diff --git a/core/test/martian/schema_test.cljc b/core/test/martian/schema_test.cljc
index 2cd2f47..6db518b 100644
--- a/core/test/martian/schema_test.cljc
+++ b/core/test/martian/schema_test.cljc
@@ -6,15 +6,16 @@
                :cljs [cljs.test :refer-macros [deftest testing is run-tests]])))
 
 (deftest free-form-object-test
-  (let [schema (schema/make-schema {:Body55523       {:type                 "object"
-                                                      :properties           {:age    {:$ref "#/definitions/Body55523Age"}
-                                                                             :name   {:$ref "#/definitions/Body55523Name"}
-                                                                             :colour {:$ref "#/definitions/Body55523Colour"}}
-                                                      :additionalProperties false
-                                                      :required             ["name"]}
-                                    :Body55523Age    {:type "object" :additionalProperties {}}
-                                    :Body55523Name   {:type "object" :additionalProperties {}}
-                                    :Body55523Colour {:type "object" :additionalProperties {}}}
+  (let [schema (schema/make-schema {:definitions
+                                    {:Body55523       {:type                 "object"
+                                                       :properties           {:age    {:$ref "#/definitions/Body55523Age"}
+                                                                              :name   {:$ref "#/definitions/Body55523Name"}
+                                                                              :colour {:$ref "#/definitions/Body55523Colour"}}
+                                                       :additionalProperties false
+                                                       :required             ["name"]}
+                                     :Body55523Age    {:type "object" :additionalProperties {}}
+                                     :Body55523Name   {:type "object" :additionalProperties {}}
+                                     :Body55523Colour {:type "object" :additionalProperties {}}}}
                                    {:in          "body"
                                     :name        "Body55523"
                                     :description ""
@@ -27,10 +28,10 @@
 
 (deftest enum-test
   (is (= (s/enum "desc" "asc")
-         (schema/make-schema {} {:name "sort"
-                                 :in "query"
-                                 :enum ["desc","asc"]
-                                 :required true}))))
+         (schema/make-schema {:definitions {}} {:name "sort"
+                                                :in "query"
+                                                :enum ["desc","asc"]
+                                                :required true}))))
 
 (deftest primitives-test
   (is (= {:id s/Int
@@ -56,27 +57,27 @@
 
 (deftest uuid-test
   (is (= (s/cond-pre s/Str s/Uuid)
-         (schema/make-schema {} {:name "uuid"
-                                 :in "path"
-                                 :required true
-                                 :type "string"
-                                 :format "uuid"}))))
+         (schema/make-schema {:definitions {}} {:name "uuid"
+                                                :in "path"
+                                                :required true
+                                                :type "string"
+                                                :format "uuid"}))))
 
 (deftest uri-test
   (is (= (s/cond-pre s/Str schema/URI)
-         (schema/make-schema {} {:name "uri"
-                                 :in "path"
-                                 :required true
-                                 :type "string"
-                                 :format "uri"}))))
+         (schema/make-schema {:definitions {}} {:name "uri"
+                                                :in "path"
+                                                :required true
+                                                :type "string"
+                                                :format "uri"}))))
 
 (deftest arrays-test
   (is (= [s/Str]
-         (schema/make-schema {} {:name "tags"
-                                 :in "body"
-                                 :required true
-                                 :type "array"
-                                 :items {:type "string"}}))))
+         (schema/make-schema {:definitions {}} {:name "tags"
+                                                :in "body"
+                                                :required true
+                                                :type "array"
+                                                :items {:type "string"}}))))
 
 (deftest objects-test
   (let [body-param {:name "Pet"
@@ -95,7 +96,26 @@
     (is (= {:id s/Int
             :name s/Str
             :tags [s/Str]}
-           (schema/make-schema definitions body-param)))))
+           (schema/make-schema {:definitions definitions} body-param)))))
+
+(deftest objects-test-with-parameter-ref
+  (let [body-param {:name "Pet"
+                    :in "body"
+                    :required true
+                    :schema {:$ref "#/parameters/Pet"}}
+        parameters {:Pet {:type "object"
+                           :properties {:id {:type "integer"
+                                             :required true}
+                                        :name {:type "string"
+                                               :required true}
+                                        :tags {:type "array"
+                                               :required true
+                                               :items {:type "string"}}}}}]
+
+    (is (= {:id s/Int
+            :name s/Str
+            :tags [s/Str]}
+           (schema/make-schema {:parameters parameters} body-param)))))
 
 (deftest optionality-test
   (is (= {(s/optional-key :id) (s/maybe s/Int)}
@@ -121,7 +141,7 @@
             (s/maybe {(s/optional-key :id) (s/maybe s/Int)
                       :name s/Str
                       (s/optional-key :tags) [s/Str]})}
-           (schema/schemas-for-parameters definitions [body-param])))))
+           (schema/schemas-for-parameters {:definitions definitions} [body-param])))))
 
 (deftest keys-test
   (testing "default params"
@@ -134,14 +154,15 @@
               :type "integer"}]))))
   (testing "object with additional properties"
     (let [schema (:Pet (schema/schemas-for-parameters
+                        {:definitions
                          {:Pet {:type                 "object"
                                 :additionalProperties true
                                 :properties           {:CamelBodyKey {:type     "integer"
-                                                                      :required true}}}}
-                         [{:name     "Pet"
-                           :in       "path"
-                           :required true
-                           :schema   {:$ref "#/definitions/Pet"}}]))]
+                                                                      :required true}}}}}
+                        [{:name     "Pet"
+                          :in       "path"
+                          :required true
+                          :schema   {:$ref "#/definitions/Pet"}}]))]
       (is (= {:CamelBodyKey s/Int
               s/Any         s/Any} schema))
       (is (s/validate schema {:CamelBodyKey 42
@@ -149,19 +170,20 @@
   (testing "object with empty properties"
     (is (= {:Pet {}}
            (schema/schemas-for-parameters
-            {:Pet {:type       "object"
-                   :properties {}}}
+            {:definitions
+             {:Pet {:type       "object"
+                    :properties {}}}}
             [{:name     "Pet"
               :in       "path"
               :required true
               :schema   {:$ref "#/definitions/Pet"}}]))))
   (testing "body params"
     (is (= {:Pet {:CamelBodyKey s/Int}}
-
            (schema/schemas-for-parameters
-            {:Pet {:type "object"
-                   :properties {:CamelBodyKey {:type "integer"
-                                               :required true}}}}
+            {:definitions
+             {:Pet {:type "object"
+                    :properties {:CamelBodyKey {:type "integer"
+                                                :required true}}}}}
             [{:name "Pet"
               :in "path"
               :required true
@@ -225,10 +247,10 @@
                                  {:fizz [{:buzz s/Str}]}]))))
 
 (deftest recursive-schema-test
-  (let [schema (schema/make-schema {:A {:type       "object"
-                                        :properties {:b {:$ref "#/definitions/B"}}}
-                                    :B {:type "object"
-                                        :properties {:a {:$ref "#/definitions/A"}}}}
+  (let [schema (schema/make-schema {:definitions {:A {:type       "object"
+                                                      :properties {:b {:$ref "#/definitions/B"}}}
+                                                  :B {:type "object"
+                                                      :properties {:a {:$ref "#/definitions/A"}}}}}
                                    {:in          "body"
                                     :name        "A"
                                     :description "A -> B -> A -> B"