Repository: oliyh/martian
Pull request #148 (2022-02-13 23:16:49)

Handle schemas with no type.
This partially addresses #145.

If a schema has no "type" key, and ONLY has a "properties" key, then
it is a valid schema and we can reasonably assume that the type is "object".

See https://github.com/OAI/OpenAPI-Specification/issues/1657

Excerpt:
A particularly common form of this is a schema that omits type, but specifies properties.
Strictly speaking, this does not mean that the value must be an object.
It means that if the value is an object, and it includes any of those properties,
the property values must conform to the corresponding property subschemas.

In reality, this construct almost always means that the user intends type: object,
and I think it would be reasonable for a code generator to assume this,
maybe with a validation: strict|lax config option to control that behavior.

oliyh: Hello,

Thanks for your continued work on this. From reading the spec, it seems like it could be expressed as:

```clj
(s/either {(s/optional k) v ...} ;; if it's an object and it includes _any_ of these properties
          s/Any ;; does not mean the value has to be an object
          )
```

It's not pretty, but seems to closer match the intention of the spec. What do you think?
bombaywalla: What you have is more accurate with respect to the spec. I'd guess that in the particular example I am dealing with, the person writing the spec just meant for it to be an object, and didn't understand the subtleties.

Would you like me to modify the PR to make it what you suggest?

oliyh: I'm inclined to agree with you, it seems a bit rubbish that it would allow `"foo"` through, and actually I don't think schema would give the right behaviour anyway. Consider:

Properties of `foo` and `bar` as ints in the schema, and the user supplies `{:foo 123 :bar "not a number"}`. This would pass the schema I wrote above, because it would fail the map schema but then pass the Any schema. From the wording, because the keys are present, they should use the map schema only and therefore fail.

Really, the 'correct' implementation is this:
```clj
(s/conditional 
  #(some? % [:foo :bar]) 
  {(s/optional-key :foo) s/Int 
   (s/optional-key :bar) s/Int} 

  :else s/Any)
```

But this seems to be getting overly complicated now. I think, given that no one has raised an issue about this before, that we go with your implementation and we can always adjust it later if it causes problems for someone else.
bombaywalla: Made the code improvement you suggested.

Patch:
diff --git a/core/src/martian/openapi.cljc b/core/src/martian/openapi.cljc
index 57f7151..eeff720 100644
--- a/core/src/martian/openapi.cljc
+++ b/core/src/martian/openapi.cljc
@@ -53,7 +53,23 @@
               components
               (conj seen-set reference)))
      (wrap schema
-           (condp = (:type schema)
+           (condp = (if-let [typ (:type schema)]
+                      typ
+                      ;; If a schema has no :type key, and the only key it contains is a :properties key,
+                      ;; then the :type can reasonably be inferred as "object".
+                      ;;
+                      ;; See https://github.com/OAI/OpenAPI-Specification/issues/1657
+                      ;;
+                      ;; Excerpt:
+                      ;; A particularly common form of this is a schema that omits type, but specifies properties.
+                      ;; Strictly speaking, this does not mean that the value must be an object.
+                      ;; It means that if the value is an object, and it includes any of those properties,
+                      ;; the property values must conform to the corresponding property subschemas.
+                      ;;
+                      ;; In reality, this construct almost always means that the user intends type: object,
+                      ;; and I think it would be reasonable for a code generator to assume this,
+                      ;; maybe with a validation: strict|lax config option to control that behavior.
+                      (when (= #{:properties} (set (keys schema))) "object"))
              "array"   [(openapi->schema (:items schema) components seen-set)]
              "object"  (let [required? (set (:required schema))]
                          (into {}


Test patch:
diff --git a/core/test/martian/openapi_test.cljc b/core/test/martian/openapi_test.cljc
index 0d959ce..62bf512 100644
--- a/core/test/martian/openapi_test.cljc
+++ b/core/test/martian/openapi_test.cljc
@@ -160,3 +160,30 @@
             {:status (s/eq 404)
              :body {(s/optional-key :code) s/Int (s/optional-key :details) s/Str}}]
            (:response-schemas handler)))))
+
+(deftest schemas-without-type-test
+  (let [openapi-json
+        {:paths {(keyword "/models")
+                 {:get {:operationId "list-models"
+                        :summary "Lists models"
+                        :responses {:404 {:$ref "#/components/responses/NotFound"}}}}}
+         :components {:responses {:NotFound
+                                  {:description "The requested resource was not found."
+                                   :content
+                                   {:application/json
+                                    {:schema {:$ref "#/components/schemas/Error"}}}}}
+                      :schemas {:Error
+                                {:properties
+                                 {:code
+                                  {:description "An enumerated error for machine use.",
+                                   :type "integer",
+                                   :readOnly true},
+                                  :details
+                                  {:description "A human-readable description of the error.",
+                                   :type "string",
+                                   :readOnly true}}}}}}
+        [handler] (openapi->handlers openapi-json {:encodes ["application/json"]
+                                                   :decodes ["application/json"]})]
+    (is (= [{:status (s/eq 404)
+             :body {(s/optional-key :code) s/Int (s/optional-key :details) s/Str}}]
+           (:response-schemas handler)))))