Redundant type: object emitted in 2.x preview

[bkoelman]

Describe the bug
Trying to use 2.0 preview 17, I see the following change in the output document, compared to 1.x:

It seems that whenever OpenApiSchema.Type is set to JsonSchemaType.Null from code, this results in also emitting "type": "object". This is redundant in many cases, for example when allOf is used.

While probably technically still correct, this difference makes it a pain to assess whether the new version produces a similar output document in our test suite. We have about 12,000 tests, many of which compare fragments of the OAS file, failing on changes like this. The fact that "nullable": true moved to the top doesn't help either, but I suspect that will be addressed in a future preview, tracked at #1314.

So I wonder if this change was intentional. And if not, how hard it would be to not emit "type": "object" when the schema contains allOf, oneOf, etc.

[bkoelman]

Here's another case:

The code that produces this schema:

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

[bkoelman]

I haven't investigated further, but both NSwag and Kiota (stable versions) don't seem to handle this very well, causing compile errors in existing code that uses the generated client.

Instead of the class ErrorLinks, it now generates a class named ErrorObject_links.

[MaggieKimani1]

Hi @bkoelman, thank you for raising this. Let me investigate this and get back to you.

[darrelmiller]

@bkoelman Unfortunately the specification says this about the nullable keyword:

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

https://spec.openapis.org/oas/v3.0.4.html#fixed-fields-20

We fixed this issue with non-compliance with the specification. However, apparently now we need to go fix Kiota. I have opened an issue microsoft/kiota#6510

[bkoelman]

@darrelmiller Thanks for looking into this. In that case, I suppose all consumers will need to adapt then.

[bkoelman]

I conducted further research on this topic and found the motivation at https://github.com/OAI/OpenAPI-Specification/blob/main/proposals/2019-10-31-Clarify-Nullable.md for why type must be specified with nullable: true . This spec change occurred in a 3.0.x patch version, but has significant effects on implementations. I doubt whether tools like NSwag, Kiota and Swashbuckle special-case their interpretation based on the patch version. From what I've encountered, they all consider nullable: true an expanding assertion today (which the patch no longer allows).

Based on my experiments, Microsoft.OpenApi just adds type: object if only null is present in the Type property. That's way too simplistic. As explained at OAI/OpenAPI-Specification#1900 (comment), there are two interpretations:

1. Type = JsonSchemaType.Null is meaningless (there is no type), so do not emit nullable: true (breaking change)
2. Type = JsonSchemaType.Null implicitly means all other types are permitted. I noticed that setting Type = JsonSchemaType.String | JsonSchemaType.Array | JsonSchemaType.Null leaves out both type and nullable in the output, which is correct. Microsoft.OpenApi could interpret Type = JsonSchemaType.Null as: any type (including null) is allowed (when output version is 3.0.x). This is also a breaking change.

For example, the following program (built against preview.17):

using System.Text;
using Microsoft.OpenApi;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Models.Interfaces;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Writers;

var document = CreateChoiceDocument();

var builder = new StringBuilder();
using (var writer = new StringWriter(builder))
{
    document.SerializeAs(OpenApiSpecVersion.OpenApi3_0, new OpenApiYamlWriter(writer));
}

var json = builder.ToString();
Console.WriteLine(json);

static OpenApiDocument CreateChoiceDocument()
{
    var integerSchema = new OpenApiSchema
    {
        Type = JsonSchemaType.Integer | JsonSchemaType.Null
    };

    var booleanSchema = new OpenApiSchema
    {
        Type = JsonSchemaType.Boolean
    };

    var refIntegerSchema = new OpenApiSchemaReference("integerSchema");
    var refBooleanSchema = new OpenApiSchemaReference("booleanSchema");

    var choiceSchema = new OpenApiSchema
    {
        OneOf =
        [
            refIntegerSchema, refBooleanSchema
        ],
        Type = JsonSchemaType.Null
    };

    return new OpenApiDocument
    {
        Components = new OpenApiComponents
        {
            Schemas = new Dictionary<string, IOpenApiSchema>
            {
                ["integerSchema"] = integerSchema,
                ["booleanSchema"] = booleanSchema,
                ["choiceSchema"] = choiceSchema
            }
        }
    };
}

Prints the following:

openapi: 3.0.4
info: { }
paths: { }
components:
  schemas:
    integerSchema:
      type: integer
      nullable: true
    booleanSchema:
      type: boolean
    choiceSchema:
      nullable: true
      type: object
      oneOf:
        - $ref: '#/components/schemas/integerSchema'
        - $ref: '#/components/schemas/booleanSchema'

This is incorrect, because the type is not an object. This case reflects interpretation 1 mentioned above. But leaving out nullable: true breaks existing code generators.

For example, the program:

using System.Text;
using Microsoft.OpenApi;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Models.Interfaces;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Writers;

var document = CreateEntityDocument();

var builder = new StringBuilder();
using (var writer = new StringWriter(builder))
{
    document.SerializeAs(OpenApiSpecVersion.OpenApi3_0, new OpenApiYamlWriter(writer));
}

var json = builder.ToString();
Console.WriteLine(json);

static OpenApiDocument CreateEntityDocument()
{
    var idSchema = new OpenApiSchema
    {
        Properties = new Dictionary<string, IOpenApiSchema>
        {
            ["Id"] = new OpenApiSchema
            {
                Type = JsonSchemaType.Integer
            }
        }
    };

    var refIdSchema = new OpenApiSchemaReference("EntityOfInt");

    var requestBodySchema = new OpenApiSchema
    {
        Properties = new Dictionary<string, IOpenApiSchema>
        {
            ["Person"] = new OpenApiSchema
            {
                Type = JsonSchemaType.Null, // must add JsonSchemaType.Object to make it appear in the output
                AllOf =
                [
                    refIdSchema, new OpenApiSchema
                    {
                        Properties = new Dictionary<string, IOpenApiSchema>
                        {
                            ["Name"] = new OpenApiSchema
                            {
                                Type = JsonSchemaType.String
                            }
                        }
                    }
                ]
            }
        }
    };

    return new OpenApiDocument
    {
        Components = new OpenApiComponents
        {
            Schemas = new Dictionary<string, IOpenApiSchema>
            {
                ["EntityOfInt"] = idSchema,
                ["RequestBody"] = requestBodySchema
            }
        }
    };
}

Prints the following output:

openapi: 3.0.4
info: { }
paths: { }
components:
  schemas:
    EntityOfInt:
      properties:
        Id:
          type: integer
    RequestBody:
      properties:
        Person:
          nullable: true # should not appear, according to the latest 3.0.x spec
          type: object # should not appear, according to the latest 3.0.x spec
          allOf:
            - $ref: '#/components/schemas/EntityOfInt'
            - properties:
                Name:
                  type: string

The problem is this. How client code generators behave today is:

• When nullable: true does not appear in Person, they produce a class with a non-nullable Person property.
• When nullable: true does appear in Person, they produce a class with a nullable Person property.
• The type: object in Person is generally ignored (aside from a few bugs in NSwag, where the behavior changes if present, but they won't fix them because the bar for breaking changes is exceptionally high, due to high adoption and low test coverage)

Options:

• If Microsoft.OpenApi is fixed to interpret Type = JsonSchemaType.Null as a no-op, this means that OAS document producers need to know the actual type they are generating to get nullable: true emitted. This isn't always easy to determine.
• If Microsoft.OpenApi is fixed to interpret Type = JsonSchemaType.Null as "null with any type", then nullable: true will no longer appear in the OAS document, and then client code generators won't produce a nullable property. Because from their point of view, the absence of nullable: true means that nulls are not allowed (which isn't true anymore with the latest patch version of the spec: an empty object without a type and without nullable: true implicitly allows null).

All in all, changing the current behavior in Microsoft.OpenApi to comply with the latest patch of the spec has a considerable ripple effect. The meaning of nullable was poorly defined in the past, and tooling gave it their own interpretation. I wonder if it's wise to change the behavior of Microsoft.OpenApi 2.x at all in this aspect.

@darrelmiller What are your thoughts?