Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement deep copy for OpenApiSchema instances #57223

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Aug 7, 2024

Closes #56990.

As the last transformation we apply to the OpenAPI document, we utilize the OpenApiSchemaReferenceTransformer to replace inlined schemas in the documents with reference schemas that reference the schema definition in the top-level document.schemas registery. As part of this process, we map schemas store in the OpenApiSchemaStore cache to elements in the OpenAPI document's components.schemas property as seen below.

var schemaStore = context.ApplicationServices.GetRequiredKeyedService<OpenApiSchemaStore>(context.DocumentName);
var schemasByReference = schemaStore.SchemasByReference;
document.Components ??= new OpenApiComponents();
document.Components.Schemas ??= new Dictionary<string, OpenApiSchema>();
foreach (var (schema, referenceId) in schemasByReference.Where(kvp => kvp.Value is not null).OrderBy(kvp => kvp.Value))
{
// Reference IDs are only set for schemas that appear more than once in the OpenAPI
// document and should be represented as references instead of inlined in the document.
if (referenceId is not null)
{
// Note: we create a copy of the schema here to avoid modifying the original schema
// so that comparisons between the original schema and the resolved schema during
// the transformation process are consistent.
document.Components.Schemas.Add(
referenceId,
ResolveReferenceForSchema(new OpenApiSchema(schema), schemasByReference, isTopLevel: true));
}
}

The implementation currently uses the copy constructor implementation on OpenApiSchema to avoid making mutations to instances in the cache that we copy from that are used later in the application.

// Note: we create a copy of the schema here to avoid modifying the original schema
// so that comparisons between the original schema and the resolved schema during
// the transformation process are consistent.
document.Components.Schemas.Add(
referenceId,
ResolveReferenceForSchema(new OpenApiSchema(schema), schemasByReference, isTopLevel: true));
}

However, the copy constructor implementation for OpenApiSchema executes a shallow copy on sub-schemas within a top-level OpenApiSchema which means that modiciations to the copy made in ResolveReferenceForSchema end up impacting the instances in the cache.

To resolve this issue, we implement a deep copy implementation for OpenApiSchema so that subschemas in the copied instance can be modified without impacting the original instance.

@captainsafia captainsafia requested a review from a team as a code owner August 7, 2024 23:55
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Aug 7, 2024
@dotnet-policy-service dotnet-policy-service bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 7, 2024
@captainsafia captainsafia added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Aug 7, 2024
@captainsafia captainsafia enabled auto-merge (squash) August 8, 2024 15:21
@captainsafia captainsafia merged commit e31445e into main Aug 8, 2024
26 checks passed
@captainsafia captainsafia deleted the safia/56990 branch August 8, 2024 20:13
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-rc1 milestone Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[9.0-preview7] OpenAPI schemas are not stable between generations
3 participants