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

Fix: loading the document requires knowing the format in advance #1929

Closed
wants to merge 20 commits into from
Closed

Fix: loading the document requires knowing the format in advance #1929

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ecdf133
Make format optional; infer it from the stream or textReader input in…
MaggieKimani1 Nov 13, 2024
76286ae
Convert the methods to async to avoid deadlocks
MaggieKimani1 Nov 13, 2024
ab059fc
Align wrapped params
MaggieKimani1 Nov 13, 2024
e01db74
Pass a cancellation token to allow cancelling requests/tasks
MaggieKimani1 Nov 13, 2024
f30add8
clean up code
MaggieKimani1 Nov 13, 2024
8d83694
Return the generic output of type T as part of the read result; clean…
MaggieKimani1 Nov 13, 2024
0c05c3f
Clean up tests to reflect new LoadAsync pattern
MaggieKimani1 Nov 13, 2024
917fae3
Update doc comments and doc initialization
MaggieKimani1 Nov 14, 2024
8b07082
Update public API
MaggieKimani1 Nov 14, 2024
a83461d
Refactor code to cater for non-seekable streams
MaggieKimani1 Nov 14, 2024
7468eb2
clean up code
MaggieKimani1 Nov 14, 2024
93ba1c1
chore: use of async life time for test initialization
baywet Nov 18, 2024
47bfb6f
Move code block to a reusable method to avoid duplication
MaggieKimani1 Nov 19, 2024
49a181a
use async lifetime for test initialization
MaggieKimani1 Nov 19, 2024
ecb9d6e
code refactor to remove Task.Run on CPU bound operation
MaggieKimani1 Nov 19, 2024
ebdbbe6
remove Task.Run
MaggieKimani1 Nov 19, 2024
9389d79
pass cancellation token
MaggieKimani1 Nov 19, 2024
9a2be7f
run operation asynchronously
MaggieKimani1 Nov 19, 2024
f07ff44
Update public API
MaggieKimani1 Nov 19, 2024
afc4055
Update src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
MaggieKimani1 Nov 25, 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
6 changes: 3 additions & 3 deletions src/Microsoft.OpenApi.Hidi/OpenApiService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public static async Task TransformOpenApiDocumentAsync(HidiOptions options, ILog
}

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

if (options.FilterOptions != null)
Expand Down Expand Up @@ -586,7 +586,7 @@ private static string GetInputPathExtension(string? openapi = null, string? csdl
throw new ArgumentException("Please input a file path or URL");
}

var format = await OpenApiModelFactory.GetFormatAsync(options.OpenApi).ConfigureAwait(false);
var format = await OpenApiModelFactory.GetFormatAsync(options.OpenApi, cancellationToken).ConfigureAwait(false);
var document = await GetOpenApiAsync(options, format, logger, null, cancellationToken).ConfigureAwait(false);

using (logger.BeginScope("Creating diagram"))
Expand Down Expand Up @@ -748,7 +748,7 @@ internal static async Task PluginManifestAsync(HidiOptions options, ILogger logg
}

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

cancellationToken.ThrowIfCancellationRequested();
Expand Down
11 changes: 6 additions & 5 deletions src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,15 @@ public async Task<ReadResult> ReadAsync(TextReader input,
/// <inheritdoc/>
public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input,
OpenApiSpecVersion version,
OpenApiReaderSettings settings = null) where T : IOpenApiElement
OpenApiReaderSettings settings = null,
CancellationToken token = default) where T : IOpenApiElement
{
JsonNode jsonNode;

// Parse the YAML
try
{
jsonNode = LoadJsonNodesFromYamlDocument(input);
jsonNode = await Task.Run(() => LoadJsonNodesFromYamlDocument(input));
Copy link
Member

Choose a reason for hiding this comment

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

why are we adding the task run here?

Copy link
Contributor Author

@MaggieKimani1 MaggieKimani1 Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The caller is async here

OpenAPI.NET/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs

Line 147 in f07ff44

public static async Task<ReadFragmentResult<T>> LoadAsync<T>(Stream input,

And the interface method's equivalent in the JsonReader class is also async

OpenAPI.NET/src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs

Line 124 in f07ff44

public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input,

If I change this method's signature to be synchronous, I'll also need to update this JSON content to be read synchronously:

OpenAPI.NET/src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs

Line 189 in f07ff44

var content = await input.ReadToEndAsync();

}
catch (JsonException ex)
{
Expand All @@ -63,7 +64,7 @@ public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input,
return default;
}

return await ReadFragmentAsync<T>(jsonNode, version, settings);
return ReadFragment<T>(jsonNode, version, settings);
}

/// <summary>
Expand All @@ -86,9 +87,9 @@ public async Task<ReadResult> ReadAsync(JsonNode jsonNode, OpenApiReaderSettings
}

/// <inheritdoc/>
public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(JsonNode input, OpenApiSpecVersion version, OpenApiReaderSettings settings = null) where T : IOpenApiElement
public ReadFragmentResult<T> ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, OpenApiReaderSettings settings = null) where T : IOpenApiElement
{
return await OpenApiReaderRegistry.DefaultReader.ReadFragmentAsync<T>(input, version, settings);
return OpenApiReaderRegistry.DefaultReader.ReadFragment<T>(input, version, settings);
}
}
}
5 changes: 3 additions & 2 deletions src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ public interface IOpenApiReader
/// <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="settings">The OpenApiReader settings.</param>
/// <param name="token"></param>
/// <returns>Instance of newly created IOpenApiElement.</returns>
Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input, OpenApiSpecVersion version, OpenApiReaderSettings settings = null) where T: IOpenApiElement;
Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input, OpenApiSpecVersion version, OpenApiReaderSettings settings = null, CancellationToken token = default) where T : IOpenApiElement;

/// <summary>
/// Reads the JsonNode input and parses the fragment of an OpenAPI description into an Open API Element.
Expand All @@ -48,6 +49,6 @@ public interface IOpenApiReader
/// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
/// <param name="settings">The OpenApiReader settings.</param>
/// <returns>Instance of newly created IOpenApiElement.</returns>
Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(JsonNode input, OpenApiSpecVersion version, OpenApiReaderSettings settings = null) where T: IOpenApiElement;
ReadFragmentResult<T> ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, OpenApiReaderSettings settings = null) where T: IOpenApiElement;
}
}
23 changes: 14 additions & 9 deletions src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public async Task<ReadResult> ReadAsync(TextReader input,
// Parse the JSON text in the TextReader into JsonNodes
try
{
jsonNode = await LoadJsonNodesAsync(input);
jsonNode = await LoadJsonNodesAsync(input, cancellationToken);
}
catch (JsonException ex)
{
Expand Down Expand Up @@ -123,14 +123,15 @@ public async Task<ReadResult> ReadAsync(JsonNode jsonNode,
/// <inheritdoc/>
public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input,
OpenApiSpecVersion version,
OpenApiReaderSettings settings = null) where T: IOpenApiElement
OpenApiReaderSettings settings = null,
CancellationToken token = default) where T: IOpenApiElement
{
JsonNode jsonNode;

// Parse the JSON
try
{
jsonNode = await LoadJsonNodesAsync(input);
jsonNode = await LoadJsonNodesAsync(input, token);
}
catch (JsonException ex)
{
Expand All @@ -139,13 +140,13 @@ public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(TextReader input,
return default;
}

return await ReadFragmentAsync<T>(jsonNode, version, settings);
return ReadFragment<T>(jsonNode, version, settings);
}

/// <inheritdoc/>
public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(JsonNode input,
OpenApiSpecVersion version,
OpenApiReaderSettings settings = null) where T : IOpenApiElement
public ReadFragmentResult<T> ReadFragment<T>(JsonNode input,
OpenApiSpecVersion version,
OpenApiReaderSettings settings = null) where T : IOpenApiElement
{
var diagnostic = new OpenApiDiagnostic();
settings ??= new OpenApiReaderSettings();
Expand All @@ -158,7 +159,7 @@ public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(JsonNode input,
try
{
// Parse the OpenAPI element asynchronously
element = await Task.Run(() => context.ParseFragment<T>(input, version));
element = context.ParseFragment<T>(input, version);
}
catch (OpenApiException ex)
{
Expand All @@ -182,9 +183,13 @@ public async Task<ReadFragmentResult<T>> ReadFragmentAsync<T>(JsonNode input,
};
}

private async Task<JsonNode> LoadJsonNodesAsync(TextReader input)
private async Task<JsonNode> LoadJsonNodesAsync(TextReader input, CancellationToken token = default)
{
#if NETSTANDARD2_0
var content = await input.ReadToEndAsync();
MaggieKimani1 marked this conversation as resolved.
Show resolved Hide resolved
#else
var content = await input.ReadToEndAsync(token);
#endif
return JsonNode.Parse(content);
}

Expand Down
15 changes: 10 additions & 5 deletions src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static OpenApiModelFactory()
/// <returns></returns>
public static async Task<ReadResult> LoadAsync(string url, OpenApiReaderSettings settings = null, CancellationToken cancellationToken = default)
{
var format = await GetFormatAsync(url);
var format = await GetFormatAsync(url, cancellationToken);
var stream = await GetStreamAsync(url);
return await LoadAsync(stream, format, settings);
}
Expand Down Expand Up @@ -129,7 +129,7 @@ public static async Task<ReadFragmentResult<T>> LoadAsync<T>(string url,
OpenApiReaderSettings settings = null,
CancellationToken cancellationToken = default) where T : IOpenApiElement
{
var format = await GetFormatAsync(url);
var format = await GetFormatAsync(url, cancellationToken);
settings ??= new OpenApiReaderSettings();
var stream = await GetStreamAsync(url);
return await LoadAsync<T>(stream, version, format, settings);
Expand Down Expand Up @@ -172,11 +172,15 @@ public static async Task<ReadFragmentResult<T>> LoadAsync<T>(TextReader input,
return await OpenApiReaderRegistry.GetReader(format).ReadFragmentAsync<T>(input, version, settings);
}

private static async Task<string> GetContentTypeAsync(string url)
private static async Task<string> GetContentTypeAsync(string url, CancellationToken token = default)
{
if (!string.IsNullOrEmpty(url))
{
#if NETSTANDARD2_0
var response = await _httpClient.GetAsync(url);
MaggieKimani1 marked this conversation as resolved.
Show resolved Hide resolved
#else
var response = await _httpClient.GetAsync(url, token);
#endif
var mediaType = response.Content.Headers.ContentType.MediaType;
var contentType = mediaType.Split(";".ToCharArray(), StringSplitOptions.RemoveEmptyEntries).First();
return contentType;
Expand All @@ -189,8 +193,9 @@ private static async Task<string> GetContentTypeAsync(string url)
/// Infers the OpenAPI format from the input URL.
/// </summary>
/// <param name="url">The input URL.</param>
/// <param name="token"></param>
/// <returns>The OpenAPI format.</returns>
public static async Task<string> GetFormatAsync(string url)
public static async Task<string> GetFormatAsync(string url, CancellationToken token = default)
{
if (!string.IsNullOrEmpty(url))
{
Expand All @@ -207,7 +212,7 @@ public static async Task<string> GetFormatAsync(string url)
}
else
{
var contentType = await GetContentTypeAsync(url);
var contentType = await GetContentTypeAsync(url, token);
return contentType.Split('/').LastOrDefault();
}
}
Expand Down
10 changes: 5 additions & 5 deletions test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,9 @@ namespace Microsoft.OpenApi.Interfaces
{
System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> ReadAsync(System.IO.TextReader input, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken cancellationToken = default);
System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> ReadAsync(System.Text.Json.Nodes.JsonNode jsonNode, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings, System.Threading.CancellationToken cancellationToken = default);
System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadFragmentResult<T>> ReadFragmentAsync<T>(System.IO.TextReader input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null)
Microsoft.OpenApi.Reader.ReadFragmentResult<T> ReadFragment<T>(System.Text.Json.Nodes.JsonNode input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null)
where T : Microsoft.OpenApi.Interfaces.IOpenApiElement;
System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadFragmentResult<T>> ReadFragmentAsync<T>(System.Text.Json.Nodes.JsonNode input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null)
System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadFragmentResult<T>> ReadFragmentAsync<T>(System.IO.TextReader input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken token = default)
where T : Microsoft.OpenApi.Interfaces.IOpenApiElement;
}
public interface IOpenApiReferenceable : Microsoft.OpenApi.Interfaces.IOpenApiElement, Microsoft.OpenApi.Interfaces.IOpenApiSerializable
Expand Down Expand Up @@ -1310,14 +1310,14 @@ namespace Microsoft.OpenApi.Reader
public OpenApiJsonReader() { }
public System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> ReadAsync(System.IO.TextReader input, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken cancellationToken = default) { }
public System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> ReadAsync(System.Text.Json.Nodes.JsonNode jsonNode, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings, System.Threading.CancellationToken cancellationToken = default) { }
public System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadFragmentResult<T>> ReadFragmentAsync<T>(System.IO.TextReader input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null)
public Microsoft.OpenApi.Reader.ReadFragmentResult<T> ReadFragment<T>(System.Text.Json.Nodes.JsonNode input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null)
where T : Microsoft.OpenApi.Interfaces.IOpenApiElement { }
public System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadFragmentResult<T>> ReadFragmentAsync<T>(System.Text.Json.Nodes.JsonNode input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null)
public System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadFragmentResult<T>> ReadFragmentAsync<T>(System.IO.TextReader input, Microsoft.OpenApi.OpenApiSpecVersion version, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken token = default)
where T : Microsoft.OpenApi.Interfaces.IOpenApiElement { }
}
public static class OpenApiModelFactory
{
public static System.Threading.Tasks.Task<string> GetFormatAsync(string url) { }
public static System.Threading.Tasks.Task<string> GetFormatAsync(string url, System.Threading.CancellationToken token = default) { }
public static System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> LoadAsync(string url, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken cancellationToken = default) { }
public static System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> LoadAsync(System.IO.Stream input, string format = null, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken cancellationToken = default) { }
public static System.Threading.Tasks.Task<Microsoft.OpenApi.Reader.ReadResult> LoadAsync(System.IO.TextReader input, string format = null, Microsoft.OpenApi.Reader.OpenApiReaderSettings settings = null, System.Threading.CancellationToken cancellationToken = default) { }
Expand Down
Loading