OpenAPI: document.Workspace has schemas duplicated

[desjoerd]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

While playing around hacking SchemaReferenceTransformers I got an itch around OpenApiDocumentExtensions.AddOpenApiSchemaByReference:

        document.Workspace ??= new();
        var location = document.BaseUri + "/components/schemas/" + schemaId;
        document.Workspace.RegisterComponentForDocument(document, schema, location);

So when debugging some unit tests I found that schemas are added twice. For OpenApiSchemaServiceTests.GetOpenApiRequestBody_GeneratesSchemaForPoco it adds the Todo schema 2 times:

1. https://openapi.net/d6bd3466-1e30-4009-a52a-c5e42c50c184#/components/schemas/https://openapi.net/d6bd3466-1e30-4009-a52a-c5e42c50c184/components/schemas/Todo
2. https://openapi.net/d6bd3466-1e30-4009-a52a-c5e42c50c184#/components/schemas/Todo

The first is added in the OpenApiDocumentExtensions.AddOpenApiSchemaByReference method. The second is added in the OpenApiDocumentService.GetOpenApiDocumentAsync (document.Workspace.RegisterComponents(document)). This last one scans all components and adds them to the workspace (so they are correct from a OpenAPI.NET point of view).

I tried changing OpenApiDocumentExtensions.AddOpenApiSchemaByReference to one of the following:

• document.Workspace.RegisterComponentForDocument(document, schema, schemaId);
• document.AddComponent(schemaId, schema);

Both add the schema as expected to the workspace but it fails some tests caused by circular references. This is because the moment they are added correctly to the workspace they are used when resolving references. The way how RegisterComponent works is different than just assigning the schema in document.Components.Schemas[schemaId] = schema;. RegisterComponent does not overwrite a component (TryAdd), but assigning does.

Related method:

    public static IOpenApiSchema AddOpenApiSchemaByReference(this OpenApiDocument document, string schemaId, IOpenApiSchema schema)
    {
        if(schema is OpenApiSchemaReference)
        {
            throw new ArgumentException("Schema is already a reference.", nameof(schema));
        }

        document.Components ??= new();
        document.Components.Schemas ??= new Dictionary<string, IOpenApiSchema>();
        document.Components.Schemas[schemaId] = schema;
        document.Workspace ??= new();
        var location = document.BaseUri + "/components/schemas/" + schemaId;
        document.Workspace.RegisterComponentForDocument(document, schema, location);
        
        object? description = null;
        object? example = null;
        if (schema is OpenApiSchema actualSchema)
        {
            actualSchema.Metadata?.TryGetValue(OpenApiConstants.RefDescriptionAnnotation, out description);
            actualSchema.Metadata?.TryGetValue(OpenApiConstants.RefExampleAnnotation, out example);
        }

        return new OpenApiSchemaReference(schemaId, document)
        {
            Description = description as string,
            Examples = example is JsonNode exampleJson ? [exampleJson] : null,
        };
    }

My proposed fix is to remove the whole part around the workspace in AddOpenApiSchemaByReference as it's not used anyway for resolving references. In the OpenApiDocumentService we still keep the initialization of the workspaces which gives is a correctly initialized workspace.

The below passes all tests:

    public static IOpenApiSchema AddOpenApiSchemaByReference(this OpenApiDocument document, string schemaId, IOpenApiSchema schema)
    {
        if(schema is OpenApiSchemaReference)
        {
            throw new ArgumentException("Schema is already a reference.", nameof(schema));
        }

        document.Components ??= new();
        document.Components.Schemas ??= new Dictionary<string, IOpenApiSchema>();
        document.Components.Schemas[schemaId] = schema;
        
        object? description = null;
        object? example = null;
        if (schema is OpenApiSchema actualSchema)
        {
            actualSchema.Metadata?.TryGetValue(OpenApiConstants.RefDescriptionAnnotation, out description);
            actualSchema.Metadata?.TryGetValue(OpenApiConstants.RefExampleAnnotation, out example);
        }

        return new OpenApiSchemaReference(schemaId, document)
        {
            Description = description as string,
            Examples = example is JsonNode exampleJson ? [exampleJson] : null,
        };
    }

Another fix could be just removing the initialization of the Workspace completly as it's not required for the functionality.

@captainsafia let me know what you think 😊. I can make the change, I just don't know where you would like the unit tests for the Workspace initialization (as there are none right now).

Expected Behavior

No response

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

10.0.100-rc.1.25420.111

Anything else?

It's not a breaking bug.

[MihuBot]

I'm a bot. Here are possible related and/or duplicate issues (I may be wrong):

• .NET 9 OpenAPI produces lots of duplicate schemas for the same object #58968
• [Critical] ASP.NET Core 9.0: AddOpenApi duplicates schema with a number suffix #59427
• OpenApi generate duplicate schemas #61408
• [OpenAPI] Duplicated object definitions in openapi spec #62154
• Microsoft.AspNetCore.OpenAPI duplicating schemas when returning generic wrappers and reusing model #62508

[captainsafia]

Another fix could be just removing the initialization of the Workspace completly as it's not required for the functionality.

Did you validate this? At some point in the previews, the library (Microsoft.OpenApi, here) did require you to initialize the workspace otherwise some refs would fail to resolve. There is a chance that this constraint was relaxed at some point and the change wasn't captured in any of the release notes.

[desjoerd]

I will put in a draft PR (I think this runs the CI as well) which removes it from the AddOpenApiSchemaByReference. Than we can think of a next action or adding unit tests.

I did verify it by removing it at that point and run all unit tests on main. I was surprised as well.

Removing the workspace resulted in the current existing behavior, when I updated it to be added without the prefix some unit tests started failing. My reasoning behind that is because when they are added with only the schema Id the workspace actually resolves them and because it's a TryAdd it doesn't overwrite it while being in the recursive ResolveReferences.

[desjoerd]

@captainsafia I've created a draft PR #63606.

I think it has been probably relaxed and there has been logic added for resolving references based on the document itself. AND there probably has a change around the component registration location requiring a # to be a treated as a relative reference.