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

Merged
merged 55 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
a648591
Refactor readers to reduce surface area
darrelmiller Nov 26, 2024
c9b01cb
Moved load external references
darrelmiller Nov 26, 2024
634cb1c
Merge remote-tracking branch 'origin/vnext' into mk/fix-json-reader
MaggieKimani1 Nov 27, 2024
1464e2c
Move load methods to be adjacent
MaggieKimani1 Nov 27, 2024
e588377
Async over sync
MaggieKimani1 Nov 27, 2024
47e3e25
refactor code
MaggieKimani1 Nov 27, 2024
8521082
Use the provided format in hidi options
MaggieKimani1 Nov 28, 2024
99a80b9
Clean up tests; refactor to use async
MaggieKimani1 Nov 28, 2024
627d75b
Dispose stream
MaggieKimani1 Nov 28, 2024
46718f0
clean up and dispose stream if specified in the settings
MaggieKimani1 Nov 28, 2024
bd64612
Update public API
MaggieKimani1 Nov 28, 2024
a2f70ae
Leave stream open if specified in settings
MaggieKimani1 Nov 28, 2024
8bbb3dd
Guard against null references
MaggieKimani1 Nov 28, 2024
cecf011
code cleanup
MaggieKimani1 Nov 28, 2024
be3bbf5
Update public API
MaggieKimani1 Dec 2, 2024
408e793
remove extra semi-colon and commented out code
MaggieKimani1 Dec 2, 2024
7acdf9b
Remove unnecessary using
MaggieKimani1 Dec 2, 2024
dc25f9e
Inspect string input's format if not explicitly provided
MaggieKimani1 Dec 3, 2024
63ab53f
Remove unnecessary using and whitespace
MaggieKimani1 Dec 3, 2024
d022fa4
Remove unnecessary using
MaggieKimani1 Dec 5, 2024
04af1a6
Make format optional; add logic for inspecting stream format
MaggieKimani1 Dec 5, 2024
86b70c9
code cleanup
MaggieKimani1 Dec 5, 2024
dd80ab7
Update public API
MaggieKimani1 Dec 10, 2024
2c2bbe6
chore: code fixes recommended by sonarqube
baywet Dec 11, 2024
3665930
Update src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs
MaggieKimani1 Dec 11, 2024
5ca061b
Update src/Microsoft.OpenApi/Models/OpenApiDocument.cs
MaggieKimani1 Dec 11, 2024
e8c76db
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 11, 2024
a66e21b
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 11, 2024
4198c82
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 11, 2024
0836e97
chore: code linting
baywet Dec 11, 2024
a5e1057
Update src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs
MaggieKimani1 Dec 11, 2024
01e4f49
chore: adds missing defensive programming and passes settings when re…
baywet Dec 11, 2024
d1aedb4
Merge branch 'mk/fix-json-reader' of https://github.com/microsoft/Ope…
baywet Dec 11, 2024
07ab67a
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 11, 2024
3123cb7
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 11, 2024
ab2ddf0
fix: default settings in case of null value
baywet Dec 11, 2024
87ce841
Merge branch 'mk/fix-json-reader' of https://github.com/microsoft/Ope…
baywet Dec 11, 2024
3154d45
Merge branch 'vnext' into mk/fix-json-reader
MaggieKimani1 Dec 19, 2024
955f7fb
Fix issues from resolving merge conflicts
MaggieKimani1 Dec 19, 2024
a4933ef
Update input doc comments
MaggieKimani1 Dec 19, 2024
538d2eb
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 19, 2024
f879452
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 19, 2024
aac99fe
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 19, 2024
e3049a6
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 19, 2024
8a17bf2
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Dec 19, 2024
c944a31
Defensive programming; pass cancellation token
MaggieKimani1 Dec 20, 2024
66cc6e0
Merge remote-tracking branch 'origin/mk/fix-json-reader' into mk/fix-…
MaggieKimani1 Dec 20, 2024
47dca99
Code cleanup and fix tests
MaggieKimani1 Dec 20, 2024
e243f4e
simplify tuple variables
MaggieKimani1 Dec 20, 2024
28e1ecd
Update public API
MaggieKimani1 Dec 20, 2024
56742b9
Dispose stream in caller method
MaggieKimani1 Dec 20, 2024
ee63758
Abstract implementation detail from interface
MaggieKimani1 Dec 20, 2024
7270a89
Dispose preparedStream properly
MaggieKimani1 Dec 20, 2024
b033b76
Pass cancellation token
MaggieKimani1 Dec 20, 2024
c24d670
Update API
MaggieKimani1 Dec 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions src/Microsoft.OpenApi.Hidi/OpenApiService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static async Task TransformOpenApiDocumentAsync(HidiOptions options, ILog

#pragma warning restore CA1308 // Normalize strings to uppercase
options.Output = new($"./output{inputExtension}");
};
}

if (options.CleanOutput && options.Output.Exists)
{
Expand Down Expand Up @@ -97,8 +97,7 @@ public static async Task TransformOpenApiDocumentAsync(HidiOptions options, ILog
}

// Load OpenAPI document
var format = OpenApiModelFactory.GetFormat(options.OpenApi);
var document = await GetOpenApiAsync(options, format, logger, options.MetadataVersion, cancellationToken).ConfigureAwait(false);
var document = await GetOpenApiAsync(options, openApiFormat.GetDisplayName(), logger, options.MetadataVersion, cancellationToken).ConfigureAwait(false);

if (options.FilterOptions != null)
{
Expand Down Expand Up @@ -254,7 +253,7 @@ private static async Task<OpenApiDocument> GetOpenApiAsync(HidiOptions options,
else if (!string.IsNullOrEmpty(options.OpenApi))
{
stream = await GetStreamAsync(options.OpenApi, logger, cancellationToken).ConfigureAwait(false);
var result = await ParseOpenApiAsync(options.OpenApi, options.InlineExternal, logger, stream, cancellationToken).ConfigureAwait(false);
var result = await ParseOpenApiAsync(options.OpenApi, format, options.InlineExternal, logger, stream, cancellationToken).ConfigureAwait(false);
document = result.Document;
}
else throw new InvalidOperationException("No input file path or URL provided");
Expand Down Expand Up @@ -351,8 +350,8 @@ private static MemoryStream ApplyFilterToCsdl(Stream csdlStream, string entitySe
try
{
using var stream = await GetStreamAsync(openApi, logger, cancellationToken).ConfigureAwait(false);

result = await ParseOpenApiAsync(openApi, false, logger, stream, cancellationToken).ConfigureAwait(false);
var openApiFormat = !string.IsNullOrEmpty(openApi) ? GetOpenApiFormat(openApi, logger) : OpenApiFormat.Yaml;
result = await ParseOpenApiAsync(openApi, openApiFormat.GetDisplayName(),false, logger, stream, cancellationToken).ConfigureAwait(false);

using (logger.BeginScope("Calculating statistics"))
{
Expand Down Expand Up @@ -380,7 +379,7 @@ private static MemoryStream ApplyFilterToCsdl(Stream csdlStream, string entitySe
return result.Diagnostic.Errors.Count == 0;
}

private static async Task<ReadResult> ParseOpenApiAsync(string openApiFile, bool inlineExternal, ILogger logger, Stream stream, CancellationToken cancellationToken = default)
private static async Task<ReadResult> ParseOpenApiAsync(string openApiFile, string format, bool inlineExternal, ILogger logger, Stream stream, CancellationToken cancellationToken = default)
{
ReadResult result;
var stopwatch = Stopwatch.StartNew();
Expand All @@ -396,7 +395,6 @@ private static async Task<ReadResult> ParseOpenApiAsync(string openApiFile, bool
new Uri("file://" + new FileInfo(openApiFile).DirectoryName + Path.DirectorySeparatorChar)
};

var format = OpenApiModelFactory.GetFormat(openApiFile);
result = await OpenApiDocument.LoadAsync(stream, format, settings, cancellationToken).ConfigureAwait(false);

logger.LogTrace("{Timestamp}ms: Completed parsing.", stopwatch.ElapsedMilliseconds);
Expand Down Expand Up @@ -587,8 +585,8 @@ private static string GetInputPathExtension(string? openapi = null, string? csdl
throw new ArgumentException("Please input a file path or URL");
}

var format = OpenApiModelFactory.GetFormat(options.OpenApi);
var document = await GetOpenApiAsync(options, format, logger, null, cancellationToken).ConfigureAwait(false);
var openApiFormat = options.OpenApiFormat ?? (!string.IsNullOrEmpty(options.OpenApi) ? GetOpenApiFormat(options.OpenApi, logger) : OpenApiFormat.Yaml);
var document = await GetOpenApiAsync(options, openApiFormat.GetDisplayName(), logger, null, cancellationToken).ConfigureAwait(false);

using (logger.BeginScope("Creating diagram"))
{
Expand Down Expand Up @@ -748,9 +746,11 @@ internal static async Task PluginManifestAsync(HidiOptions options, ILogger logg
options.OpenApi = apiDependency.ApiDescripionUrl;
}

var openApiFormat = options.OpenApiFormat ?? (!string.IsNullOrEmpty(options.OpenApi)
? GetOpenApiFormat(options.OpenApi, logger) : OpenApiFormat.Yaml);

// Load OpenAPI document
var format = OpenApiModelFactory.GetFormat(options.OpenApi);
var document = await GetOpenApiAsync(options, format, logger, options.MetadataVersion, cancellationToken).ConfigureAwait(false);
var document = await GetOpenApiAsync(options, openApiFormat.GetDisplayName(), logger, options.MetadataVersion, cancellationToken).ConfigureAwait(false);

cancellationToken.ThrowIfCancellationRequested();

Expand Down
67 changes: 47 additions & 20 deletions src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using SharpYaml.Serialization;
using System.Linq;
using Microsoft.OpenApi.Models;
using System;

namespace Microsoft.OpenApi.Readers
{
Expand All @@ -19,17 +20,41 @@ namespace Microsoft.OpenApi.Readers
/// </summary>
public class OpenApiYamlReader : IOpenApiReader
{
private const int copyBufferSize = 4096;
private static readonly OpenApiJsonReader _jsonReader = new();

/// <inheritdoc/>
public async Task<ReadResult> ReadAsync(TextReader input,
OpenApiReaderSettings settings = null,
public async Task<ReadResult> ReadAsync(Stream input,
OpenApiReaderSettings settings,
CancellationToken cancellationToken = default)
{
if (input is null) throw new ArgumentNullException(nameof(input));
if (input is MemoryStream memoryStream)
{
return Read(memoryStream, settings);
}
else
{
using var preparedStream = new MemoryStream();
await input.CopyToAsync(preparedStream, copyBufferSize, cancellationToken).ConfigureAwait(false);
preparedStream.Position = 0;
return Read(preparedStream, settings);
}
}

/// <inheritdoc/>
public ReadResult Read(MemoryStream input,
OpenApiReaderSettings settings)
{
if (input is null) throw new ArgumentNullException(nameof(input));
if (settings is null) throw new ArgumentNullException(nameof(settings));
JsonNode jsonNode;

// Parse the YAML text in the TextReader into a sequence of JsonNodes
// Parse the YAML text in the stream into a sequence of JsonNodes
try
{
jsonNode = LoadJsonNodesFromYamlDocument(input);
using var stream = new StreamReader(input, default, true, -1, settings.LeaveStreamOpen);
Fixed Show fixed Hide fixed
jsonNode = LoadJsonNodesFromYamlDocument(stream);
}
catch (JsonException ex)
{
Expand All @@ -42,21 +67,29 @@ public async Task<ReadResult> ReadAsync(TextReader input,
};
}

return await ReadAsync(jsonNode, settings, cancellationToken: cancellationToken);
return Read(jsonNode, settings);
}

/// <inheritdoc/>
public static ReadResult Read(JsonNode jsonNode, OpenApiReaderSettings settings, string format = null)
{
return _jsonReader.Read(jsonNode, settings, OpenApiConstants.Yaml);
}

/// <inheritdoc/>
public T ReadFragment<T>(TextReader input,
public T ReadFragment<T>(MemoryStream input,
OpenApiSpecVersion version,
out OpenApiDiagnostic diagnostic,
OpenApiReaderSettings settings = null) where T : IOpenApiElement
{
if (input is null) throw new ArgumentNullException(nameof(input));
JsonNode jsonNode;

// Parse the YAML
try
{
jsonNode = LoadJsonNodesFromYamlDocument(input);
using var stream = new StreamReader(input);
jsonNode = LoadJsonNodesFromYamlDocument(stream);
}
catch (JsonException ex)
{
Expand All @@ -65,7 +98,13 @@ public T ReadFragment<T>(TextReader input,
return default;
}

return ReadFragment<T>(jsonNode, version, out diagnostic);
return ReadFragment<T>(jsonNode, version, out diagnostic, settings);
}

/// <inheritdoc/>
public static T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
{
return _jsonReader.ReadFragment<T>(input, version, out diagnostic, settings);
}

/// <summary>
Expand All @@ -80,17 +119,5 @@ static JsonNode LoadJsonNodesFromYamlDocument(TextReader input)
var yamlDocument = yamlStream.Documents[0];
return yamlDocument.ToJsonNode();
}

/// <inheritdoc/>
public async Task<ReadResult> ReadAsync(JsonNode jsonNode, OpenApiReaderSettings settings, string format = null, CancellationToken cancellationToken = default)
{
return await OpenApiReaderRegistry.DefaultReader.ReadAsync(jsonNode, settings, OpenApiConstants.Yaml, cancellationToken);
}

/// <inheritdoc/>
public T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
{
return OpenApiReaderRegistry.DefaultReader.ReadFragment<T>(input, version, out diagnostic);
}
}
}
32 changes: 10 additions & 22 deletions src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,30 @@ namespace Microsoft.OpenApi.Interfaces
public interface IOpenApiReader
{
/// <summary>
/// Reads the TextReader input and parses it into an Open API document.
/// Async method to reads the stream and parse it into an Open API document.
/// </summary>
/// <param name="input">The TextReader input.</param>
/// <param name="input">The stream input.</param>
/// <param name="settings"> The OpenApi reader settings.</param>
/// <param name="cancellationToken">Propagates notification that an operation should be cancelled.</param>
/// <returns></returns>
Task<ReadResult> ReadAsync(TextReader input, OpenApiReaderSettings settings = null, CancellationToken cancellationToken = default);
Task<ReadResult> ReadAsync(Stream input, OpenApiReaderSettings settings, CancellationToken cancellationToken = default);

/// <summary>
/// Parses the JsonNode input into an Open API document.
/// Provides a synchronous method to read the input memory stream and parse it into an Open API document.
/// </summary>
/// <param name="jsonNode">The JsonNode input.</param>
/// <param name="settings">The Reader settings to be used during parsing.</param>
/// <param name="cancellationToken">Propagates notifications that operations should be cancelled.</param>
/// <param name="format">The OpenAPI format.</param>
/// <param name="input"></param>
/// <param name="settings"></param>
/// <returns></returns>
Task<ReadResult> ReadAsync(JsonNode jsonNode, OpenApiReaderSettings settings, string format = null, CancellationToken cancellationToken = default);
ReadResult Read(MemoryStream input, OpenApiReaderSettings settings);

/// <summary>
/// Reads the TextReader input and parses the fragment of an OpenAPI description into an Open API Element.
/// Reads the MemoryStream and parses the fragment of an OpenAPI description into an Open API Element.
/// </summary>
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
/// <param name="input">Memory stream containing OpenAPI description to parse.</param>
/// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
/// <param name="diagnostic">Returns diagnostic object containing errors detected during parsing.</param>
/// <param name="settings">The OpenApiReader settings.</param>
/// <returns>Instance of newly created IOpenApiElement.</returns>
T ReadFragment<T>(TextReader input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;

/// <summary>
/// Reads the JsonNode input and parses the fragment of an OpenAPI description into an Open API Element.
/// </summary>
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
/// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
/// <param name="diagnostic">Returns diagnostic object containing errors detected during parsing.</param>
/// <param name="settings">The OpenApiReader settings.</param>
/// <returns>Instance of newly created IOpenApiElement.</returns>
T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
T ReadFragment<T>(MemoryStream input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
}
}
48 changes: 6 additions & 42 deletions src/Microsoft.OpenApi/Models/OpenApiDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public void SerializeAsV31(IOpenApiWriter writer)

writer.WriteStartObject();

// openApi;
// openApi
writer.WriteProperty(OpenApiConstants.OpenApi, "3.1.1");

// jsonSchemaDialect
Expand Down Expand Up @@ -371,7 +371,7 @@ private static void WriteHostInfoV2(IOpenApiWriter writer, IList<OpenApiServer>?

// Arbitrarily choose the first server given that V2 only allows
// one host, port, and base path.
var serverUrl = ParseServerUrl(servers.First());
var serverUrl = ParseServerUrl(servers[0]);

// Divide the URL in the Url property into host and basePath required in OpenAPI V2
// The Url property cannot contain path templating to be valid for V2 serialization.
Expand Down Expand Up @@ -535,45 +535,20 @@ private static string ConvertByteArrayToString(byte[] hash)
return Workspace?.ResolveReference<IOpenApiReferenceable>(uriLocation);
}

/// <summary>
/// Parses a local file path or Url into an Open API document.
/// </summary>
/// <param name="url"> The path to the OpenAPI file.</param>
/// <param name="settings"></param>
/// <returns></returns>
public static ReadResult Load(string url, OpenApiReaderSettings? settings = null)
{
return OpenApiModelFactory.Load(url, settings);
}

/// <summary>
/// Reads the stream input and parses it into an Open API document.
/// </summary>
/// <param name="stream">Stream containing OpenAPI description to parse.</param>
/// <param name="format">The OpenAPI format to use during parsing.</param>
/// <param name="settings">The OpenApi reader settings.</param>
/// <returns></returns>
public static ReadResult Load(Stream stream,
string format,
public static ReadResult Load(MemoryStream stream,
string? format = null,
OpenApiReaderSettings? settings = null)
{
return OpenApiModelFactory.Load(stream, format, settings);
}

/// <summary>
/// Reads the text reader content and parses it into an Open API document.
/// </summary>
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
/// <param name="format"> The OpenAPI format to use during parsing.</param>
/// <param name="settings">The OpenApi reader settings.</param>
/// <returns></returns>
public static ReadResult Load(TextReader input,
string format,
OpenApiReaderSettings? settings = null)
{
return OpenApiModelFactory.Load(input, format, settings);
}

/// <summary>
/// Parses a local file path or Url into an Open API document.
/// </summary>
Expand All @@ -593,22 +568,11 @@ public static async Task<ReadResult> LoadAsync(string url, OpenApiReaderSettings
/// <param name="settings">The OpenApi reader settings.</param>
/// <param name="cancellationToken">Propagates information about operation cancelling.</param>
/// <returns></returns>
public static async Task<ReadResult> LoadAsync(Stream stream, string format, OpenApiReaderSettings? settings = null, CancellationToken cancellationToken = default)
public static async Task<ReadResult> LoadAsync(Stream stream, string? format = null, OpenApiReaderSettings? settings = null, CancellationToken cancellationToken = default)
{
return await OpenApiModelFactory.LoadAsync(stream, format, settings, cancellationToken);
return await OpenApiModelFactory.LoadAsync(stream, format, settings, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Reads the text reader content and parses it into an Open API document.
/// </summary>
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
/// <param name="format"> The OpenAPI format to use during parsing.</param>
/// <param name="settings">The OpenApi reader settings.</param>
/// <returns></returns>
public static async Task<ReadResult> LoadAsync(TextReader input, string format, OpenApiReaderSettings? settings = null)
{
return await OpenApiModelFactory.LoadAsync(input, format, settings);
}

/// <summary>
/// Parses a string into a <see cref="OpenApiDocument"/> object.
Expand Down
Loading
Loading