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

Refactor readers to reduce surface area #1975

Open
wants to merge 52 commits into
base: dev
Choose a base branch
from
Open

Conversation

MaggieKimani1
Copy link
Contributor

@MaggieKimani1 MaggieKimani1 commented Nov 28, 2024

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

@MaggieKimani1 MaggieKimani1 marked this pull request as ready for review December 2, 2024 11:19
MaggieKimani1 and others added 9 commits December 11, 2024 18:10
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…quired

Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
@baywet baywet requested a review from Copilot December 11, 2024 15:25

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 no suggestions.

Files not reviewed (15)
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiCallbackTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDocumentTests.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
  • src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs: Evaluated as low risk
  • src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiEncodingTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiDiscriminatorTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiExampleTests.cs: Evaluated as low risk
  • src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs: Evaluated as low risk
  • src/Microsoft.OpenApi.Hidi/OpenApiService.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.Hidi.Tests/Services/OpenApiFilterServiceTests.cs: Evaluated as low risk
  • test/Microsoft.OpenApi.Readers.Tests/OpenApiReaderTests/OpenApiDiagnosticTests.cs: Evaluated as low risk
{
throw new InvalidOperationException($"Could not download the file at {url}", ex);
// YAML or other non-JSON format; copy remaining input to a new stream.
preparedStream = new MemoryStream();

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'MemoryStream' is created but not disposed.
{
throw new InvalidOperationException($"Could not open the file at {url}", ex);
// Buffer stream for non-JSON formats (e.g., YAML) since they require synchronous reading
preparedStream = new MemoryStream();

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'MemoryStream' is created but not disposed.
Copy link

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

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