jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
57 stars 59 forks source link

Fix multitype enum validation inside an allOf #96

Closed ugexe closed 6 years ago

ugexe commented 6 years ago

Fixes regression starting in v2.00 where an enum containing a type constraint of ["string","null"] would not accept return values ( from Mojolicious::Plugin::OpenAPI ) of undef as valid if inside an allOf.

ugexe commented 6 years ago

There is probably a better fix than this PR, but hopefully this helps demonstrate its goal. Maybe the solution even belongs in Mojolicious::Plugin::OpenAPI?

Note I have not had time to construct a test, although this PR does pass current tests. Below is an example of the OpenAPI spec which allows a return value of attributes : 'null' in 1.08 but not in 2.00 or 2.01.

{
  "definitions": {
    "stipulation" : {
      "title" : "Data",
      "type" : "object",
      "allOf" : [
        {
          "$ref" : "#/definitions/metadata"
        },
        {
          "type" : "object",
          "properties" : {
            "name" : {
              "type" : "string"
            }
          }
        }
      ]
    },
    "metadata" : {
      "title" : "Metadata",
      "type" : "object",
      "properties" : {
        "attributes" : {
          "type" : ["string","null"],
          "enum" : ["foo", "bar","null"]
        }
      }
    }
  }
}
jhthorsen commented 6 years ago

Are you sure "enum":["foo","bar","null"] is right? I thought you had to have "enum":["foo","bar",null]

I don't get why you would have both enum and type...

ugexe commented 6 years ago

Having both enum and type has ( or was ) the only way to get an enumeration to accept both string values or null. Additionally it had to be a quoted null for the app to work properly ( writen approximately 1.5 years ago ), at least up to JV@1.08

jhthorsen commented 6 years ago

But "null" isn't the same as null, don't you agree..?

ugexe commented 6 years ago

https://github.com/jhthorsen/mojolicious-plugin-openapi/blob/5a0f0a2538b79e752f6d8db2b4ed55b7c0aedf31/t/empty-string.t#L43

"schema": {"type": ["null", "string"]}

jhthorsen commented 6 years ago

type values needs to be quoted, enum values does not. Pretty sure the old code was simply broken, and your code is "confused" now that it is fixed.

ugexe commented 6 years ago

So I just applied your advice and removed the type and changed "null" to null inside the enum, and the app works with the newer JSON::Validator.

I don't remember why we had to do that odd type/enum combo originally ( I remember wondering why we had to do it that way, but without time to investigate ). If I re-encounter the situation I will revisit this with additional info, but hopefully the cause was simply inadvertently fixed by other updates somewhere.

jhthorsen commented 6 years ago

Glad it worked out for you 👍

ugexe commented 6 years ago

So the reason we had quoted nulls and the extra type info was because swagger-ui 2.x does not work otherwise. It does work with swagger-ui 3.x though, so all is good.