Skip to content

Commit 80164af

Browse files
baywetbaywet
authored
Merge pull request #1975 from microsoft/mk/fix-json-reader
Refactor readers to reduce surface area
2 parents fd202c7 + c24d670 commit 80164af

30 files changed

+616
-577
lines changed

‎src/Microsoft.OpenApi.Hidi/OpenApiService.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public static async Task TransformOpenApiDocumentAsync(HidiOptions options, ILog
6767

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

7272
if (options.CleanOutput && options.Output.Exists)
7373
{
@@ -98,8 +98,7 @@ public static async Task TransformOpenApiDocumentAsync(HidiOptions options, ILog
9898
}
9999

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

104103
if (options.FilterOptions != null)
105104
{
@@ -255,7 +254,7 @@ private static async Task<OpenApiDocument> GetOpenApiAsync(HidiOptions options,
255254
else if (!string.IsNullOrEmpty(options.OpenApi))
256255
{
257256
stream = await GetStreamAsync(options.OpenApi, logger, cancellationToken).ConfigureAwait(false);
258-
var result = await ParseOpenApiAsync(options.OpenApi, options.InlineExternal, logger, stream, cancellationToken).ConfigureAwait(false);
257+
var result = await ParseOpenApiAsync(options.OpenApi, format, options.InlineExternal, logger, stream, cancellationToken).ConfigureAwait(false);
259258
document = result.Document;
260259
}
261260
else throw new InvalidOperationException("No input file path or URL provided");
@@ -352,8 +351,8 @@ private static MemoryStream ApplyFilterToCsdl(Stream csdlStream, string entitySe
352351
try
353352
{
354353
using var stream = await GetStreamAsync(openApi, logger, cancellationToken).ConfigureAwait(false);
355-
356-
result = await ParseOpenApiAsync(openApi, false, logger, stream, cancellationToken).ConfigureAwait(false);
354+
var openApiFormat = !string.IsNullOrEmpty(openApi) ? GetOpenApiFormat(openApi, logger) : OpenApiFormat.Yaml;
355+
result = await ParseOpenApiAsync(openApi, openApiFormat.GetDisplayName(),false, logger, stream, cancellationToken).ConfigureAwait(false);
357356

358357
using (logger.BeginScope("Calculating statistics"))
359358
{
@@ -381,7 +380,7 @@ private static MemoryStream ApplyFilterToCsdl(Stream csdlStream, string entitySe
381380
return result.Diagnostic.Errors.Count == 0;
382381
}
383382

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

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

403401
logger.LogTrace("{Timestamp}ms: Completed parsing.", stopwatch.ElapsedMilliseconds);
@@ -588,8 +586,8 @@ private static string GetInputPathExtension(string? openapi = null, string? csdl
588586
throw new ArgumentException("Please input a file path or URL");
589587
}
590588

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

594592
using (logger.BeginScope("Creating diagram"))
595593
{
@@ -749,9 +747,11 @@ internal static async Task PluginManifestAsync(HidiOptions options, ILogger logg
749747
options.OpenApi = apiDependency.ApiDescriptionUrl;
750748
}
751749

750+
var openApiFormat = options.OpenApiFormat ?? (!string.IsNullOrEmpty(options.OpenApi)
751+
? GetOpenApiFormat(options.OpenApi, logger) : OpenApiFormat.Yaml);
752+
752753
// Load OpenAPI document
753-
var format = OpenApiModelFactory.GetFormat(options.OpenApi);
754-
var document = await GetOpenApiAsync(options, format, logger, options.MetadataVersion, cancellationToken).ConfigureAwait(false);
754+
var document = await GetOpenApiAsync(options, openApiFormat.GetDisplayName(), logger, options.MetadataVersion, cancellationToken).ConfigureAwait(false);
755755

756756
cancellationToken.ThrowIfCancellationRequested();
757757

‎src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using SharpYaml.Serialization;
1212
using System.Linq;
1313
using Microsoft.OpenApi.Models;
14+
using System;
1415

1516
namespace Microsoft.OpenApi.Readers
1617
{
@@ -19,17 +20,41 @@ namespace Microsoft.OpenApi.Readers
1920
/// </summary>
2021
public class OpenApiYamlReader : IOpenApiReader
2122
{
23+
private const int copyBufferSize = 4096;
24+
private static readonly OpenApiJsonReader _jsonReader = new();
25+
2226
/// <inheritdoc/>
23-
public async Task<ReadResult> ReadAsync(TextReader input,
24-
OpenApiReaderSettings settings = null,
27+
public async Task<ReadResult> ReadAsync(Stream input,
28+
OpenApiReaderSettings settings,
2529
CancellationToken cancellationToken = default)
2630
{
31+
if (input is null) throw new ArgumentNullException(nameof(input));
32+
if (input is MemoryStream memoryStream)
33+
{
34+
return Read(memoryStream, settings);
35+
}
36+
else
37+
{
38+
using var preparedStream = new MemoryStream();
39+
await input.CopyToAsync(preparedStream, copyBufferSize, cancellationToken).ConfigureAwait(false);
40+
preparedStream.Position = 0;
41+
return Read(preparedStream, settings);
42+
}
43+
}
44+
45+
/// <inheritdoc/>
46+
public ReadResult Read(MemoryStream input,
47+
OpenApiReaderSettings settings)
48+
{
49+
if (input is null) throw new ArgumentNullException(nameof(input));
50+
if (settings is null) throw new ArgumentNullException(nameof(settings));
2751
JsonNode jsonNode;
2852

29-
// Parse the YAML text in the TextReader into a sequence of JsonNodes
53+
// Parse the YAML text in the stream into a sequence of JsonNodes
3054
try
3155
{
32-
jsonNode = LoadJsonNodesFromYamlDocument(input);
56+
using var stream = new StreamReader(input, default, true, -1, settings.LeaveStreamOpen);
57+
jsonNode = LoadJsonNodesFromYamlDocument(stream);
3358
}
3459
catch (JsonException ex)
3560
{
@@ -42,21 +67,29 @@ public async Task<ReadResult> ReadAsync(TextReader input,
4267
};
4368
}
4469

45-
return await ReadAsync(jsonNode, settings, cancellationToken: cancellationToken);
70+
return Read(jsonNode, settings);
71+
}
72+
73+
/// <inheritdoc/>
74+
public static ReadResult Read(JsonNode jsonNode, OpenApiReaderSettings settings, string format = null)
75+
{
76+
return _jsonReader.Read(jsonNode, settings, OpenApiConstants.Yaml);
4677
}
4778

4879
/// <inheritdoc/>
49-
public T ReadFragment<T>(TextReader input,
80+
public T ReadFragment<T>(MemoryStream input,
5081
OpenApiSpecVersion version,
5182
out OpenApiDiagnostic diagnostic,
5283
OpenApiReaderSettings settings = null) where T : IOpenApiElement
5384
{
85+
if (input is null) throw new ArgumentNullException(nameof(input));
5486
JsonNode jsonNode;
5587

5688
// Parse the YAML
5789
try
5890
{
59-
jsonNode = LoadJsonNodesFromYamlDocument(input);
91+
using var stream = new StreamReader(input);
92+
jsonNode = LoadJsonNodesFromYamlDocument(stream);
6093
}
6194
catch (JsonException ex)
6295
{
@@ -65,7 +98,13 @@ public T ReadFragment<T>(TextReader input,
6598
return default;
6699
}
67100

68-
return ReadFragment<T>(jsonNode, version, out diagnostic);
101+
return ReadFragment<T>(jsonNode, version, out diagnostic, settings);
102+
}
103+
104+
/// <inheritdoc/>
105+
public static T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
106+
{
107+
return _jsonReader.ReadFragment<T>(input, version, out diagnostic, settings);
69108
}
70109

71110
/// <summary>
@@ -80,17 +119,5 @@ static JsonNode LoadJsonNodesFromYamlDocument(TextReader input)
80119
var yamlDocument = yamlStream.Documents[0];
81120
return yamlDocument.ToJsonNode();
82121
}
83-
84-
/// <inheritdoc/>
85-
public async Task<ReadResult> ReadAsync(JsonNode jsonNode, OpenApiReaderSettings settings, string format = null, CancellationToken cancellationToken = default)
86-
{
87-
return await OpenApiReaderRegistry.DefaultReader.ReadAsync(jsonNode, settings, OpenApiConstants.Yaml, cancellationToken);
88-
}
89-
90-
/// <inheritdoc/>
91-
public T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
92-
{
93-
return OpenApiReaderRegistry.DefaultReader.ReadFragment<T>(input, version, out diagnostic);
94-
}
95122
}
96123
}

‎src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,30 @@ namespace Microsoft.OpenApi.Interfaces
1515
public interface IOpenApiReader
1616
{
1717
/// <summary>
18-
/// Reads the TextReader input and parses it into an Open API document.
18+
/// Async method to reads the stream and parse it into an Open API document.
1919
/// </summary>
20-
/// <param name="input">The TextReader input.</param>
20+
/// <param name="input">The stream input.</param>
2121
/// <param name="settings"> The OpenApi reader settings.</param>
2222
/// <param name="cancellationToken">Propagates notification that an operation should be cancelled.</param>
2323
/// <returns></returns>
24-
Task<ReadResult> ReadAsync(TextReader input, OpenApiReaderSettings settings = null, CancellationToken cancellationToken = default);
24+
Task<ReadResult> ReadAsync(Stream input, OpenApiReaderSettings settings, CancellationToken cancellationToken = default);
2525

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

3634
/// <summary>
37-
/// Reads the TextReader input and parses the fragment of an OpenAPI description into an Open API Element.
35+
/// Reads the MemoryStream and parses the fragment of an OpenAPI description into an Open API Element.
3836
/// </summary>
39-
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
37+
/// <param name="input">Memory stream containing OpenAPI description to parse.</param>
4038
/// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
4139
/// <param name="diagnostic">Returns diagnostic object containing errors detected during parsing.</param>
4240
/// <param name="settings">The OpenApiReader settings.</param>
4341
/// <returns>Instance of newly created IOpenApiElement.</returns>
44-
T ReadFragment<T>(TextReader input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
45-
46-
/// <summary>
47-
/// Reads the JsonNode input and parses the fragment of an OpenAPI description into an Open API Element.
48-
/// </summary>
49-
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
50-
/// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
51-
/// <param name="diagnostic">Returns diagnostic object containing errors detected during parsing.</param>
52-
/// <param name="settings">The OpenApiReader settings.</param>
53-
/// <returns>Instance of newly created IOpenApiElement.</returns>
54-
T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
42+
T ReadFragment<T>(MemoryStream input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
5543
}
5644
}

‎src/Microsoft.OpenApi/Models/OpenApiDocument.cs‎

Lines changed: 6 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public void SerializeAsV31(IOpenApiWriter writer)
137137

138138
writer.WriteStartObject();
139139

140-
// openApi;
140+
// openApi
141141
writer.WriteProperty(OpenApiConstants.OpenApi, "3.1.1");
142142

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

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

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

538-
/// <summary>
539-
/// Parses a local file path or Url into an Open API document.
540-
/// </summary>
541-
/// <param name="url"> The path to the OpenAPI file.</param>
542-
/// <param name="settings"></param>
543-
/// <returns></returns>
544-
public static ReadResult Load(string url, OpenApiReaderSettings? settings = null)
545-
{
546-
return OpenApiModelFactory.Load(url, settings);
547-
}
548-
549538
/// <summary>
550539
/// Reads the stream input and parses it into an Open API document.
551540
/// </summary>
552541
/// <param name="stream">Stream containing OpenAPI description to parse.</param>
553542
/// <param name="format">The OpenAPI format to use during parsing.</param>
554543
/// <param name="settings">The OpenApi reader settings.</param>
555544
/// <returns></returns>
556-
public static ReadResult Load(Stream stream,
557-
string format,
545+
public static ReadResult Load(MemoryStream stream,
546+
string? format = null,
558547
OpenApiReaderSettings? settings = null)
559548
{
560549
return OpenApiModelFactory.Load(stream, format, settings);
561550
}
562551

563-
/// <summary>
564-
/// Reads the text reader content and parses it into an Open API document.
565-
/// </summary>
566-
/// <param name="input">TextReader containing OpenAPI description to parse.</param>
567-
/// <param name="format"> The OpenAPI format to use during parsing.</param>
568-
/// <param name="settings">The OpenApi reader settings.</param>
569-
/// <returns></returns>
570-
public static ReadResult Load(TextReader input,
571-
string format,
572-
OpenApiReaderSettings? settings = null)
573-
{
574-
return OpenApiModelFactory.Load(input, format, settings);
575-
}
576-
577552
/// <summary>
578553
/// Parses a local file path or Url into an Open API document.
579554
/// </summary>
@@ -593,22 +568,11 @@ public static async Task<ReadResult> LoadAsync(string url, OpenApiReaderSettings
593568
/// <param name="settings">The OpenApi reader settings.</param>
594569
/// <param name="cancellationToken">Propagates information about operation cancelling.</param>
595570
/// <returns></returns>
596-
public static async Task<ReadResult> LoadAsync(Stream stream, string format, OpenApiReaderSettings? settings = null, CancellationToken cancellationToken = default)
571+
public static async Task<ReadResult> LoadAsync(Stream stream, string? format = null, OpenApiReaderSettings? settings = null, CancellationToken cancellationToken = default)
597572
{
598-
return await OpenApiModelFactory.LoadAsync(stream, format, settings, cancellationToken);
573+
return await OpenApiModelFactory.LoadAsync(stream, format, settings, cancellationToken).ConfigureAwait(false);
599574
}
600575

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

613577
/// <summary>
614578
/// Parses a string into a <see cref="OpenApiDocument"/> object.

0 commit comments

Comments
 (0)