Merge pull request #1975 from microsoft/mk/fix-json-reader

Refactor readers to reduce surface area

Author: baywet

src/Microsoft.OpenApi.Hidi/OpenApiService.cs

@@ -67,7 +67,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)
                 {
@@ -98,8 +98,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)
                 {
@@ -255,7 +254,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");
@@ -352,8 +351,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"))
                 {
@@ -381,7 +380,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();
@@ -397,7 +396,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);
@@ -588,8 +586,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"))
                 {
@@ -749,9 +747,11 @@ internal static async Task PluginManifestAsync(HidiOptions options, ILogger logg
                 options.OpenApi = apiDependency.ApiDescriptionUrl;
             }
 
+            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();
 

src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs

@@ -11,6 +11,7 @@
 using SharpYaml.Serialization;
 using System.Linq;
 using Microsoft.OpenApi.Models;
+using System;
 
 namespace Microsoft.OpenApi.Readers
 {
@@ -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);
+                jsonNode = LoadJsonNodesFromYamlDocument(stream);
             }
             catch (JsonException ex)
             {
@@ -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)
             {
@@ -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>
@@ -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);
-        }
     }
 }

src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs

@@ -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;
     }
 }

src/Microsoft.OpenApi/Models/OpenApiDocument.cs

@@ -137,7 +137,7 @@ public void SerializeAsV31(IOpenApiWriter writer)
 
             writer.WriteStartObject();
 
-            // openApi;
+            // openApi
             writer.WriteProperty(OpenApiConstants.OpenApi, "3.1.1");
 
             // jsonSchemaDialect
@@ -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.
@@ -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>
@@ -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.