Downcast oneOf null to nullable with OpenAPI 3.0

[desjoerd]

ASP.NET Core 10 OpenApi generates "oneOf" schemas for nullable references for OpenApi 3.1 which is structurely different than what would be idiomatic in OpenAPI 3.0

OpenAPI 3.1

"contact": 
{
  "oneOf": 
  [
    {
      "type": "null"
    },
    {
      "$ref": "#/components/schemas/Person"
    }
  ]
}

When downcasting this to OpenAPI 3.0 we get

"contact": 
{
  "oneOf": 
  [
    {
      "nullable": true
    },
    {
      "$ref": "#/components/schemas/Person"
    }
  ]
}

This is incorrect as { "nullable": true } does not refer to a base schema.

The pattern of "oneOf" with OpenAPI 3.1 should be downcasted to the following:

"contact":
{
  "allOf":
  {
    "$ref": "#/components/schemas/Person"
  },
  "nullable": true
}

Remarks:

• Probably a pattern of "anyOf" with a { "type": "null" } should also be recognized.
• OpenAPI 3.1 "oneOf" with multiple schemas AND a null schema would be required to be downcasted to a "oneOf" without the { "type": "null" } and the nullable: true

[martincostello]

We recently had a very similar issue in Swashbuckle: domaindrivendev/Swashbuckle.AspNetCore#3683

[baywet]

Hi @desjoerd
Thank you for using the SDK and for reaching out.

Can you please provide more context on why you think that having the nullable:true on the "root schema" is better in any way than having it as a oneOf entry?

[desjoerd]

According to the openapi 3.0.4 specification https://spec.openapis.org/oas/v3.0.4.html#fixed-fields-20 nullable: true only takes into effect when type is specified. Which makes it do nothing in the current situation.

Edit:
You would expect that the below is invalid as well:

"contact":
{
  "allOf":
  {
    "$ref": "#/components/schemas/Person"
  },
  "nullable": true
}

But $ref is a full replacement in OpenAPI 3.0.4 and allOf merges all schemas and makes the contact schema be something like:

"contact":
{
  "allOf":
  {
    "$ref": "#/components/schemas/Person"
  },
  "nullable": true
},
"Person": {
  "type": "object",
  "properties": {
    "firstName": { "type": "string" }
  }
}

Becomes

"contact":
{
  "allOf":
  {
    "type": "object",
    "properties": {
      "firstName": { "type": "string" }
    },
  },
  "nullable": true
},
...

Becomes

"contact":
{
  "type": "object",
  "properties": {
    "firstName": { "type": "string" }
  },
  "nullable": true
},
...

[baywet]

Thank you for the additional information.

Right, but this has nothing to do with the location (root or anyOf entry) of the value, but rather the fact that the type keyword is missing when downcasting.

And no, even though semantically it "looks like" we can do permutations like you just shared in your latest edit, this is not how the JSON schema specification works. Each keyword is evaluated independently.

The downcast should set a type when inserting a nullable:true schema. Where it becomes tricky is that the type that must be set depends on the other entry (object, number, etc...)

Thoughts?

[desjoerd]

The moment we add "type": "object" to the { nullable: true } an empty object { } becomes valid as well, as it would match the object without properties in the oneOf.

"contact": 
{
  "oneOf": 
  [
    {
      "type": "object",
      "nullable": true
    },
    {
      "$ref": "#/components/schemas/Person"
    }
  ]
}
...

---

On the $ref resolving and composition, I think it depends on how you interpret the following

The OpenAPI Specification allows combining and extending model definitions using the allOf keyword of JSON Schema, in effect offering model composition. allOf takes an array of object definitions that are validated independently but together compose a single object.
From https://spec.openapis.org/oas/v3.0.4.html#composition-and-inheritance-polymorphism

And

Any sibling elements of a $ref are ignored. This is because $ref works by replacing itself and everything on its level with the definition it is pointing at.
From https://swagger.io/docs/specification/v3_0/using-ref/#considerations

[baywet]

The moment we add "type": "object" to the { nullable: true } an empty object { } becomes valid as well

This is already the case because of the type: object in the person schema in your example. It only becomes a problem when properties are required.

I think it depends on how you interpret the following

allOf takes an array of object definitions that are validated independently

This clearly calls out that each entry is evaluated independently.

but together compose a single object.

This calls out the JSON instance (not the JSON schema, the payload object in memory) is ultimately a single object to which all the validations are applied.

I don't think there's any room for other interpretation, like "we can now do schema transformation for what we think is semantically equivalent schemas". Case in point there was an attempt to normalize such transformation at some point (as a separate specification).

As for the swagger quote, this IS NOT the specification, and it's not normative (but it can sometimes be useful).

In 3.X, all references are OpenAPI references, and do not define any additional fields/overridable keywords.
From 3.1 and onwards, JSON schema references are defined from the JSON schema specification and other references are OpenAPI references, which support summary, description, and extension fields (x-). The consequence of that is that JSON Schema references support overriding keywords in the reference (but nullable does not exist in that version). I'm mentioning all of that to clarify defining nullable:true in the same schema object would never be a valid option.

Ultimately, downcasting is going to be lossy in a number of different subtle ways. Which is why using the latest version of the specification is always going to guarantee a greater fidelity with the object model (at least, this is how this library is designed).
Since using nullable: true is not valid without the type keyword, my suggestion here is to "pick" the type of one of the sister entries as an "improvement" over the current downcast behaviour. And yet, that might still be a bit leaky in the context of required properties, but it'll be better than emitting a document that is semantically invalid.
Thoughts?