Regression: broken nullability after update to v3.9.0

[bkoelman]

Describe the bug
After updating from v3.7.0 to v3.9.0, the nullable property disappears when writing an OpenAPI 3.0 document.

Repro steps

using Microsoft.OpenApi;

var metaSchema = new OpenApiSchema
{
    Type = JsonSchemaType.Object,
    AdditionalProperties = new OpenApiSchema
    {
        Type = JsonSchemaType.Null
    }
};

var baseSchema = new OpenApiSchema
{
    Type = JsonSchemaType.Object,
    Id = "base",
    Properties = new Dictionary<string, IOpenApiSchema>
    {
        ["id"] = new OpenApiSchema
        {
            Type = JsonSchemaType.String
        }
    }
};

var derivedSchema = new OpenApiSchema
{
    Type = JsonSchemaType.Null,
    AllOf = new List<IOpenApiSchema>
    {
        new OpenApiSchemaReference(baseSchema.Id)
    },
    Properties = new Dictionary<string, IOpenApiSchema>
    {
        ["kind"] = new OpenApiSchema
        {
            Type = JsonSchemaType.String
        }
    }
};

var doc = new OpenApiDocument();
doc.Components ??= new OpenApiComponents();
doc.Components.Schemas ??= new Dictionary<string, IOpenApiSchema>();
doc.Components.Schemas.Add("meta", metaSchema);
doc.Components.Schemas.Add("base", baseSchema);
doc.Components.Schemas.Add("derived", derivedSchema);

var result = await doc.SerializeAsJsonAsync(OpenApiSpecVersion.OpenApi3_0);
Console.WriteLine(result);

Using v3.7.0, this prints:

{
  "openapi": "3.0.4",
  "info": { },
  "paths": { },
  "components": {
    "schemas": {
      "meta": {
        "type": "object",
        "additionalProperties": {
          "nullable": true
        }
      },
      "base": {
        "type": "object",
        "properties": {
          "id": {
            "type": "string"
          }
        }
      },
      "derived": {
        "allOf": [
          {
            "$ref": "#/components/schemas/base"
          }
        ],
        "properties": {
          "kind": {
            "type": "string"
          }
        },
        "nullable": true
      }
    }
  }
}

Using v3.9.0, this prints:

{
  "openapi": "3.0.4",
  "info": { },
  "paths": { },
  "components": {
    "schemas": {
      "meta": {
        "type": "object",
        "additionalProperties": {
          "enum": [
            null
          ]
        }
      },
      "base": {
        "type": "object",
        "properties": {
          "id": {
            "type": "string"
          }
        }
      },
      "derived": {
        "allOf": [
          {
            "$ref": "#/components/schemas/base"
          }
        ],
        "properties": {
          "kind": {
            "type": "string"
          }
        }
      }
    }
  }
}

Screenshots/Code Snippets
View of the diff:

Originally reported at: domaindrivendev/Swashbuckle.AspNetCore#4065

[martincostello]

Changes in 3.8.0 also created a similar but different effect as can be seen by the change in our snapshots here: domaindrivendev/Swashbuckle.AspNetCore@cc099e8.

[baywet]

Hey everyone,
Thank you for using the SDK and for reaching out.

This keyword only takes effect if type is explicitly defined within the same Schema Object

From v3.0 specification

In the examples shared above:

• was removed from the derived component schema (which has no type property, and makes the keyword unapplicable).
• was "transformed" to enum null on the meta component schema additionalProperties. (so it's applicable because that schema doesn't have a type)

While I understand this is disruptive to snapshot tests, I believe this is the more "correct" behaviour (least transformations applied during serialization, do not serialize a keywork that's unapplicable, try to maintain the semantics when possible). See #2358 for more context.

Let me know if you have any additional comments or questions.

CC Youssef Fahmy (@Youssef1313)

[bkoelman]

This is basically #2342 (comment) again, which I fixed in #2358. TLDR: The 3.0 spec changed in 3.0.3, focused on making validators happy, not taking into account the merging of allOf subtrees into a generated class.

The meta schema represents a free-format JSON subtree, which models JSON:API meta. Interpreting the spec as "nullable not allowed without type" means a free-format structure can no longer be expressed in OpenAPI: additionalProperties must not be constrained to objects, because meta properties can also be (nullable) strings, arrays, etc.

I suppose it would be possible to adapt Swashbuckle and other consumers of this library to explicitly set Type=Object, but that results in a different openapi.json file, which we already know NSwag can't handle. Therefore, I still believe the better choice is to remain flexible about the spec interpretation:

This keyword only takes effect if type is explicitly defined within the same Schema Object.

That doesn't mean it is forbidden to emit nullable: true; it just means that document consumers should ignore it when no type is present.

[baywet]

Thank you for the additional information.

I think this is effectively asks the following question: when should semantic transformation be applied in the context of "serializing" to older document versions (v2/v3)?

I believe the answer is increasingly so: not in the object serialization code. And here are a couple of reasons for that:

1. we want to enable symmetry as much as possible, if someone loads a v2/v3 document, makes a small change, and serializes it back, it should not have other "unrelated" changes.
2. these semantic transformations might lead to tradeoffs, that library users might want to opt-out from, or make different choices.

Based on this issue, the PR that caused this "regression" which was itself reverting some other semantic transformations, the one you pointed at, #2960, #808 (which is kind of adjacent to this discussion), I propose the library should have a couple of additional "principles":

1. serialization focuses on producing structurally valid documents. It downgrades to "similar" properties when possible (e.g. nullable), even if that's semantically meaningless. Or uses extensions when not possible (e.g. x-oai/x-jsonschema). Or throws when extensions are not usable.
2. semantic transformations should be moved to document walkers that apply mutations BEFORE the serialization
3. we can provide default transformers for each version
4. users should be able to use them, or not, or roll out their own

Using copilot to perform a high level "scan" of other instances like those outside of nullable, it came back with this

Version	Serializer	Semantic transformation
v3	OpenApiSchema	Converts JSON Schema concepts for older v3: const -> singleton enum, type: null -> enum: [null] in v3.0, type including null -> non-null type + nullable: true, numeric exclusiveMaximum/Minimum <-> older boolean form, Examples -> first example before v3.1. See OpenApiSchema.cs:431-471, 531-549, 597-627, 1009-1106.
v2	OpenApiSchema	Converts schema semantics to Swagger 2 equivalents: const -> singleton enum, nullable type -> x-nullable, numeric exclusive bounds -> maximum/minimum + boolean exclusive flag, first Examples item -> example, and derives format from composed schemas when absent. See OpenApiSchema.cs:811-821, 899-903, 968-973, 1037-1106.
v2	OpenApiDocument	Converts first server.url into Swagger 2 host, basePath, and schemes; also converts component request bodies into parameter definitions. See OpenApiDocument.cs:326-327, 383-390, 463-536.
v2	OpenApiOperation	Converts requestBody into Swagger 2 body/formData parameters; derives consumes from request body media types, produces from response media types, and operation servers into schemes. See OpenApiOperation.cs:253-317, 321-343.
v2	OpenApiRequestBody	Converts request body content into a body parameter or formData parameters; binary/base64 string form fields become Swagger 2 file type with format cleared. See OpenApiRequestBody.cs:99-120, 124-160.
v2	OpenApiParameter	Converts v3 array serialization style/explode into Swagger 2 collectionFormat values: multi, pipes, ssv. See OpenApiParameter.cs:204-269.
v2	OpenApiResponse	Converts response content into Swagger 2 schema, media-type keyed examples, and media type extensions. This preserves meaning best for single-content responses; multiple distinct media schemas are inherently lossy. See OpenApiResponse.cs:127-170.
v2	OpenApiSecurityScheme	Converts v3 security schemes into Swagger 2 forms: HTTP basic becomes type: basic; OAuth flow names map to v2 names like clientCredentials -> application, authorizationCode -> accessCode. Multiple OAuth flows are reduced to one, so that case is lossy. See OpenApiSecurityScheme.cs:177-223, 267-303.
v2/v3	BaseOpenApiReference	Rewrites reference values to the version-specific reference target path, e.g. v3 #/components/... vs v2 #/definitions, #/parameters, etc. See BaseOpenApiReference.cs:60-132, 196-237.