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.

Sign up for GitHub

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

Jump to bottom

fix/example reference #2089

Merged
merged 3 commits into from
Jan 24, 2025
Merged

fix/example reference #2089

merged 3 commits into from
Jan 24, 2025

Conversation

baywet
Copy link
Member

@baywet baywet commented Jan 24, 2025

Issues

partial #1998

Commits

  • feat: makes the reference interface covariant
  • fix: visibility of serialize internal methods
  • fix: proxy design pattern implementation for OpenAPiExample

Details

This PR:

  1. clears naming confusion between IOpenAPIReferenceable and IOpenAPIReferenceHolder for who can be referenced and who references
  2. introduces a new IOpenAPIReadOnlyExtensible to enable polymorphism between references and targets
  3. introduces a new IOpenApiDescribedElement to enable polymorphism
  4. introduces a new IOpenAPIExample interface to correct the proxy implementation. To maximize reusability this interface derives from IOpenApiDescribedElement, IOpenApiSerializable and IOpenApiReadOnlyExtensible. It also defines getters only for the properties that are specific to the OpenAPIexample type. Only OpenApiExample implements the setters, this has multiple benefits including immediate feedback to the user from the compiler when they are getting confused between the target and a reference and risk mutating the target instead of the local reference
  5. updates all the DOM to return IOpenApiExample
  6. removes duplicate local fields for summary and description in the OpenApiExampleReference since those can be stored in the reference type (fields already existing)
  7. removes all virtual modifiers in OpenAPIExample
  8. makes OpenAPIExample implement IOpenAPIExample, and switches to inheritdoc comments
  9. makes the copy constructor of OpenAPIexample accept an IOpenAPIExample
  10. removes reference properties from OpenAPIExample
  11. implements IOpenAPIReferenceHolder in OpenAPIExampleReference, removing the local reference property
  12. removes setters for custom properties in OpenAPIExampleReference to avoid any confusion to what is being mutated.
  13. implements a "CopyReferenceAsTargetElementWithOverrides" to create throw away objects during serialization of references being inlined and account for overrides of description and summary

steps 4 and below need to be replicated for all other types.

@baywet baywet self-assigned this Jan 24, 2025
@baywet baywet enabled auto-merge January 24, 2025 18:20
@baywet baywet added this to the v2-Preview6 milestone Jan 24, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 24, 2025
View reviewed changes
src/Microsoft.OpenApi/Models/OpenApiComponents.cs
@@ -87,7 +88,7 @@
Schemas = components?.Schemas != null ? new Dictionary<string, OpenApiSchema>(components.Schemas) : null;
Responses = components?.Responses != null ? new Dictionary<string, OpenApiResponse>(components.Responses) : null;
Parameters = components?.Parameters != null ? new Dictionary<string, OpenApiParameter>(components.Parameters) : null;
Examples = components?.Examples != null ? new Dictionary<string, OpenApiExample>(components.Examples) : null;
Examples = components?.Examples != null ? new Dictionary<string, IOpenApiExample>(components.Examples) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one will get fixed as we update the other types

src/Microsoft.OpenApi/Models/OpenApiHeader.cs
@@ -115,7 +116,7 @@
AllowReserved = header?.AllowReserved ?? AllowReserved;
_schema = header?.Schema != null ? new(header.Schema) : null;
Example = header?.Example != null ? JsonNodeCloneHelper.Clone(header.Example) : null;
Examples = header?.Examples != null ? new Dictionary<string, OpenApiExample>(header.Examples) : null;
Examples = header?.Examples != null ? new Dictionary<string, IOpenApiExample>(header.Examples) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one will get fixed as we update the other types

src/Microsoft.OpenApi/Models/OpenApiParameter.cs
@@ -168,7 +169,7 @@
Explode = parameter?.Explode ?? Explode;
AllowReserved = parameter?.AllowReserved ?? AllowReserved;
_schema = parameter?.Schema != null ? new(parameter.Schema) : null;
Examples = parameter?.Examples != null ? new Dictionary<string, OpenApiExample>(parameter.Examples) : null;
Examples = parameter?.Examples != null ? new Dictionary<string, IOpenApiExample>(parameter.Examples) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one will get fixed as we update the other types

src/Microsoft.OpenApi/Services/OpenApiWalker.cs Dismissed Show dismissed Hide dismissed
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
74.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@baywet baywet mentioned this pull request Jan 24, 2025
Merged
@baywet baywet merged commit 079ab11 into dev Jan 24, 2025
13 of 14 checks passed
@baywet baywet deleted the fix/example-reference branch January 24, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darrelmiller darrelmiller darrelmiller approved these changes

@irvinesunday irvinesunday Awaiting requested review from irvinesunday

@gavinbarron gavinbarron Awaiting requested review from gavinbarron

@millicentachieng millicentachieng Awaiting requested review from millicentachieng

@MaggieKimani1 MaggieKimani1 Awaiting requested review from MaggieKimani1

@andrueastman andrueastman Awaiting requested review from andrueastman

Assignees

@baywet baywet

Labels
None yet
Projects
None yet
Milestone
v2-Preview6
Development

Successfully merging this pull request may close these issues.
2 participants
@baywet @darrelmiller