Repository: oliyh/martian
Pull request #197 (2024-01-27 14:54:01)

Fix required nested objects


Patch:
diff --git a/core/bb.edn b/core/bb.edn
index e9f274b..7ff328e 100644
--- a/core/bb.edn
+++ b/core/bb.edn
@@ -2,7 +2,8 @@
  :tasks
  {test:bb {:extra-paths ["test" "test-resources" "../test-common"]
            :extra-deps {io.github.cognitect-labs/test-runner
-                        {:git/tag "v0.5.0" :git/sha "b3fd0d2"}}
+                        {:git/tag "v0.5.0" :git/sha "b3fd0d2"}
+                        nubank/matcher-combinators {:mvn/version "3.8.5"}}
            :requires ([martian.core-test]
                       [martian.interceptors-test]
                       [martian.openapi-test])
diff --git a/core/project.clj b/core/project.clj
index cf5fe79..433f0c4 100644
--- a/core/project.clj
+++ b/core/project.clj
@@ -27,7 +27,8 @@
                                   [com.bhauman/figwheel-main "0.2.13"]
                                   [org.clojure/tools.reader "1.3.5"]
                                   [cider/piggieback "0.5.2"]
-                                  [org.clojure/tools.nrepl "0.2.13"]]
+                                  [org.clojure/tools.nrepl "0.2.13"]
+                                  [nubank/matcher-combinators "3.8.5"]]
                    :repl-options {:nrepl-middleware [cider.piggieback/wrap-cljs-repl]}}}
   :aliases {"fig"       ["trampoline" "run" "-m" "figwheel.main"]
             "fig:build" ["trampoline" "run" "-m" "figwheel.main" "-b" "dev" "-r"]
diff --git a/core/src/martian/schema.cljc b/core/src/martian/schema.cljc
index 3b01928..5236dad 100644
--- a/core/src/martian/schema.cljc
+++ b/core/src/martian/schema.cljc
@@ -7,7 +7,7 @@
             [schema-tools.coerce :as stc]
             [clojure.string :as string]
             [martian.parameter-aliases :refer [unalias-data]])
-  #?(:clj (:import [schema.core AnythingSchema Maybe EnumSchema EqSchema])))
+  #?(:clj (:import [schema.core AnythingSchema Maybe EnumSchema EqSchema OptionalKey])))
 
 (defn- keyword->string [s]
   (if (keyword? s) (name s) s))
@@ -65,9 +65,11 @@
   "Given a collection of swagger parameters returns a schema map"
   [ref-lookup parameters]
   (->> parameters
-       (map (fn [{:keys [name required] :as param}]
+       (map (fn [{:keys [name required required?] :as param}]
               {(cond-> (keyword name)
-                 (not required)
+                 (not (or required?
+                          (and (boolean? required) required)
+                          (and (string? required) (= "true" required))))
                  s/optional-key)
                (make-schema ref-lookup param)}))
        (into {})))
@@ -140,15 +142,12 @@
 
 (defn- denormalise-object-properties [{:keys [required properties] :as s}]
   (map (fn [[parameter-name param]]
-         (assoc (if (= "object" (:type param))
-                  (assoc param :properties (into {} (map (juxt :name identity)
-                                                         (denormalise-object-properties param))))
-                  param)
+         (assoc param
                 :name parameter-name
-                :required (or (when-not (= "object" (:type param))
-                                (:required param))
-                              (and (coll? required)
-                                   (contains? (set required) (name parameter-name))))))
+                :required? (or (when-not (= "object" (:type param))
+                                 (:required param))
+                               (and (coll? required)
+                                    (contains? (set required) (name parameter-name))))))
        properties))
 
 (defn- make-object-schema [ref-lookup {:keys [additionalProperties] :as schema}]
@@ -162,7 +161,7 @@
 
 (defn make-schema
   "Takes a swagger parameter and returns a schema"
-  [ref-lookup {:keys [required type schema $ref items] :as param}]
+  [ref-lookup {:keys [required required? type schema $ref items] :as param}]
   (if (let [ref (or $ref (:$ref schema))]
         (and ref (contains? *visited-refs* ref)))
     s/Any ;; avoid potential recursive loops
@@ -194,6 +193,9 @@
 
                 :else
                 (schema-type ref-lookup param))
-        (and (not required)
+        (and (or (not required)
+                 ;;(= "object" type) ;; todo work out something better here
+                 )
+             (not required?)
              (not= "array" type) (not= "array" (:type schema)))
         s/maybe))))


Test patch:
diff --git a/core/test/martian/schema_test.cljc b/core/test/martian/schema_test.cljc
index ad11f3b..4792f48 100644
--- a/core/test/martian/schema_test.cljc
+++ b/core/test/martian/schema_test.cljc
@@ -1,5 +1,6 @@
 (ns martian.schema-test
   (:require [martian.schema :as schema]
+            [matcher-combinators.test]
             [schema.core :as s]
             [schema-tools.core :as st]
             #?(:clj [clojure.test :refer [deftest testing is]]
@@ -135,45 +136,45 @@
                                        nil
                                        true))))
 
-         (testing "adds missing values when there are defaults"
-           (is (= {:name "Brachiosaurus"
+          (testing "adds missing values when there are defaults"
+            (is (= {:name "Brachiosaurus"
+                    :address {:city "trondheim"}}
+                   (schema/coerce-data schema
+                                       {:name "Brachiosaurus"
+                                        :address {:city nil}}
+                                       nil
+                                       true))))
+
+          (testing "adds missing keys"
+            (is (= {:name "Brachiosaurus"
                     :address {:city "trondheim"}}
-                  (schema/coerce-data schema
-                                      {:name "Brachiosaurus"
-                                       :address {:city nil}}
-                                      nil
+                   (schema/coerce-data schema
+                                       {:name "Brachiosaurus"
+                                        :address {}}
+                                       nil
                                        true))))
 
-         (testing "adds missing keys"
-           (is (= {:name "Brachiosaurus"
-                   :address {:city "trondheim"}}
-                  (schema/coerce-data schema
-                                      {:name "Brachiosaurus"
-                                       :address {}}
-                                      nil
-                                      true))))
-
-         (testing "removing extra keys"
-           (is (= {:name "Brachiosaurus"
-                   :address {:city "trondheim"}}
-                  (schema/coerce-data schema
-                                      {:name "Brachiosaurus"
-                                       :extra "key"
-                                       :address {}}
-                                      nil
-                                      true))))
-
-         ;; doesn't work - a limitation of spec-tools?
-         #_(testing "works inside arrays"
-           (is (= {:name "Brachiosaurus"
-                   :address {:city "stavanger"}
-                   :tags [{}]}
-                  (schema/coerce-data schema
-                                      {:name "Brachiosaurus"
-                                       :address {:city "stavanger"}
-                                       :tags [{:k nil}]}
-                                      nil
-                                      true)))))))))
+          (testing "removing extra keys"
+            (is (= {:name "Brachiosaurus"
+                    :address {:city "trondheim"}}
+                   (schema/coerce-data schema
+                                       {:name "Brachiosaurus"
+                                        :extra "key"
+                                        :address {}}
+                                       nil
+                                       true))))
+
+          ;; doesn't work - a limitation of spec-tools?
+          #_(testing "works inside arrays"
+              (is (= {:name "Brachiosaurus"
+                      :address {:city "stavanger"}
+                      :tags [{}]}
+                     (schema/coerce-data schema
+                                         {:name "Brachiosaurus"
+                                          :address {:city "stavanger"}
+                                          :tags [{:k nil}]}
+                                         nil
+                                         true)))))))))
 ;; "int-or-string" (s/cond-pre s/Str s/Int)
 (deftest int-or-string-test
   (is (= (s/cond-pre s/Str s/Int)
@@ -239,13 +240,13 @@
                     :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"}}}}}]
+                          :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
@@ -402,8 +403,6 @@
     (is (= (s/maybe {(s/optional-key :b) (s/maybe {(s/optional-key :a) s/Any})})
            schema))))
 
-
-
 (deftest resolve-ref-object-test
   (let [params       {:idParam {:name "id" :in "query"}}
         swagger-spec {:definitions params}
@@ -416,8 +415,7 @@
       (is (= {:name "id" :in "query"} (schema/resolve-ref-object {:$ref "#/definitions/allId"}
                                                                  {:definitions (assoc params :allId {:$ref "#/definitions/idParam"})})))
       (is (= {:name "id" :in "query"} (schema/resolve-ref-object {:$ref "#/components/parameters/allId"}
-                                                                 {:components {:parameters (assoc params :allId {:$ref "#/components/parameters/idParam"})}})))
-      )
+                                                                 {:components {:parameters (assoc params :allId {:$ref "#/components/parameters/idParam"})}}))))
     (testing "throws non local references"
       (is (thrown-with-msg? ExceptionInfo #"^Non-local references are not supported yet.*"
                             (schema/resolve-ref-object {:$ref "parameters.json#/definitions/idParam"} swagger-spec)))
@@ -427,8 +425,55 @@
       (let [cyclic-params {:idParam {:$ref "#/definitions/id"}
                            :id {:$ref "#/definitions/idParam"}}]
         (is (thrown-with-msg? ExceptionInfo #"^Cyclic reference.*"
-                              (schema/resolve-ref-object {:$ref "#/definitions/idParam"} {:definitions cyclic-params})))
-        ))
+                              (schema/resolve-ref-object {:$ref "#/definitions/idParam"} {:definitions cyclic-params})))))
     (testing "throws when does not find reference"
-      (is (thrown-with-msg? ExceptionInfo #"^Cannot find reference.*" (schema/resolve-ref-object {:$ref "#/definitions/otherParam"} swagger-spec)))
-      )))
+      (is (thrown-with-msg? ExceptionInfo #"^Cannot find reference.*" (schema/resolve-ref-object {:$ref "#/definitions/otherParam"} swagger-spec))))))
+
+(def definition
+  {:org.chaos-mesh.v1alpha1.NetworkChaos
+   {:description "NetworkChaos is the Schema for the networkchaos API"
+    :type "object"
+    :required ["spec"]
+    :properties
+    {:spec {:type "object"
+            :required ["action" "mode" "selector"]
+            :properties
+            {:selector {:type "object"
+                        :required ["namespaces" "fieldSelectors"]
+                        :properties
+                        {:namespaces {:type "array"
+                                      :items {:type "string"}}
+                         :fieldSelectors {:type "object"
+                                          :additionalProperties {:type "string"}}
+                         :pods {:type "object"
+                                :additionalProperties {:type "array"
+                                                       :items {:type "string"}}}}}
+             :mode {:type "string"
+                    :enum ["one"
+                           "all"
+                           "fixed"
+                           "fixed-percent"
+                           "random-max-percent"]}
+             :duration {:type "string"}
+             :action {:type "string"
+                      :enum ["netem"
+                             "delay"
+                             "loss"
+                             "duplicate"
+                             "corrupt"
+                             "partition"
+                             "bandwidth"]}}}}}})
+
+(deftest require-nested-objects
+  (let [schema (schema/make-schema {:definitions definition}
+                                   {:name "body",
+                                    :in "body",
+                                    :required true,
+                                    :schema {:$ref "#/definitions/org.chaos-mesh.v1alpha1.NetworkChaos"}})]
+    (is (match? {:spec {:mode any?
+                        :action any?
+                        :selector {:namespaces any?
+                                   :fieldSelectors any?
+                                   (s/optional-key :pods) any?}
+                        (s/optional-key :duration) any?}}
+                schema))))
diff --git a/core/test/martian/swagger_test.cljc b/core/test/martian/swagger_test.cljc
index 8c10701..65dd597 100644
--- a/core/test/martian/swagger_test.cljc
+++ b/core/test/martian/swagger_test.cljc
@@ -143,14 +143,14 @@
           :body-schema
           {:body
            {(s/optional-key :phone)
-            (s/maybe {:operation (st/default (s/enum "=" "contains" "like" "startswith") "=")
-                      :value s/Str}),
+            {:operation (st/default (s/enum "=" "contains" "like" "startswith") "=")
+             :value s/Str},
             (s/optional-key :gender)
-            (s/maybe {:operation (st/default (s/enum "=" "contains" "like" "startswith") "=")
-                      :value s/Str}),
+            {:operation (st/default (s/enum "=" "contains" "like" "startswith") "=")
+             :value s/Str},
             (s/optional-key :age)
-            (s/maybe {:operation (st/default (s/enum "=" "contains" "like" "startswith") "=")
-                      :value s/Str})}},
+            {:operation (st/default (s/enum "=" "contains" "like" "startswith") "=")
+             :value s/Str}}},
           :route-name :phones,
           :response-schemas [{:status (s/eq 'default), :body s/Any}]}