Skip to content

Conversation

MaggieKimani1
Copy link
Contributor

@MaggieKimani1 MaggieKimani1 commented Nov 28, 2024

Fixes #1954
fixes #1951
Fixes #1964
Fixes #1917
fixes #1918
closes #1958
closes #1929

@MaggieKimani1 MaggieKimani1 marked this pull request as ready for review December 2, 2024 11:19
@baywet baywet requested a review from Copilot December 20, 2024 13:52
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 15 out of 30 changed files in this pull request and generated 2 comments.

Files not reviewed (15)
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.cs: Evaluated as low risk
  • src/Microsoft.OpenApi.Hidi/OpenApiService.cs: Evaluated as low risk
  • src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs: Evaluated as low risk
  • src/Microsoft.OpenApi/Models/OpenApiDocument.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDiscriminatorTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiEncodingTests.cs: Evaluated as low risk
  • src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiCallbackTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiExampleTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDocumentTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiDocumentTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Hidi.Tests/Services/OpenApiFilterServiceTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/OpenApiReaderTests/OpenApiDiagnosticTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/OpenApiReaderTests/UnsupportedSpecVersionTests.cs: Evaluated as low risk
Comments suppressed due to low confidence (3)

src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs:332

  • The method PrepareStreamForReadingAsync might read the entire stream into memory, which could be inefficient for very large streams. Consider adding a check to handle large streams more efficiently.
private static async Task<(Stream, string)> PrepareStreamForReadingAsync(Stream input, string format, CancellationToken token = default)

src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs:79

  • The method ReadAsync(JsonNode, OpenApiReaderSettings, string, CancellationToken) has been removed. Replace it with Read(JsonNode, OpenApiReaderSettings, string).
return await ReadAsync(jsonNode, settings, cancellationToken: cancellationToken);

src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs:145

  • Ensure that the external references loading functionality is still covered elsewhere in the codebase, as the LoadExternalRefsAsync method has been removed.
return Read(jsonNode, settings);

Copy link

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes!

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

(clicked the wrong button 🤦)

@baywet baywet merged commit 80164af into dev Dec 20, 2024
9 of 12 checks passed
@baywet baywet deleted the mk/fix-json-reader branch December 20, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants