menes-dotnet/Menes

Multi-typed properties cause problems

idg10 opened this issue · 6 comments

idg10 commented

In the YAML for an OpenAPI service, we would like to be able to use JSON Schema's ability to define a property thus:

value: 
  type: [string, object, array, boolean, integer, number]

Unfortunately, the OpenApi library seems to choke when reading schemas that do this—it seems to expect a scalar. And (editing after @mwadams pointed out that my original presumption that this should work was wrong) this is apparently because although this is valid in JSON Schema, it's not supported in OpenAPI.

However, in theory this should be an acceptable alternative:

value: 
  anyOf:
    - type: string
    - type: object
    - type: array
    - type: boolean
    - type: integer
    - type: number

This gets through OpenApi's parsing, but it runs into a problem in Menes.

Expected behaviour

Either of the above should work. And in either case, both of the following should validate:

{ "value": { "iAmAn": "object" } }

and

{ "value": "I am a string" }

Observed behaviour

Only the first of the JSON files above will validate. The second will incorrectly report that value is not acceptable because it is not set to an object.

The OpenApiSchemaValidator has a method called ToJsonObjectType that includes this:

case "object":
case null: // Lazy people don't say that they are an object
    return JsonObjectType.Object;

The effect of this is to treat the preceding YAML snippet as though it were:

value:
  type: object
  anyOf:
    - type: string
    - type: object
    - type: array
    - type: boolean
    - type: integer
    - type: number

This is a problem because the type: object that Menes presumes is implied causes validation failure if the property is any of the other types.

Workaround

It is possible to work around this by explicitly setting type: none but I don't think that should be necessary.

The laziness detection needs to work harder. It should only presume that the type is implicitly object if the schema has nothing else to say.

The "in theory" is not correct. OpenAPI schema does not support an array

Mixed Types
type takes a single value. type as a list is not valid in OpenAPI (even though it is valid in JSON Schema):

# Incorrect
type:

  • string
  • integer
    Mixed types can be described using oneOf and anyOf, which specify a list of alternate types:

# Correct
oneOf:

  • type: string
  • type: integer

However, the actual bug, is correct for the anyOf, oneOf cases.

idg10 commented

What's interesting is the code in Menes does actually handle the multiple type list case explicitly. But it looks like it will never run.

Yes - that's because it has been modified from a JsonSchema validator, and I clearly didn't remove that! We should have a separate bug for this.

On the other hand, we should also look at updating it to the latest version of OpenAPI schema which is more compatible between the two.

idg10 commented

I've edited the description to take into account OpenAPI's lack of support for this Json Schema feature.

Resolved by #38