chore: adds tests on readers settings

baywet · baywet · commit 31bfaace7fa7 · 2025-02-26T08:11:46.000-05:00
chore: adds method with explicit exception when adding readers

Signed-off-by: Vincent Biret &lt;vibiret@microsoft.com&gt;
diff --git a/src/Microsoft.OpenApi.Readers/OpenApiReaderSettingsExtensions.cs b/src/Microsoft.OpenApi.Readers/OpenApiReaderSettingsExtensions.cs
@@ -16,18 +16,7 @@ public static class OpenApiReaderSettingsExtensions
     public static void AddYamlReader(this OpenApiReaderSettings settings)
     {
         var yamlReader = new OpenApiYamlReader();
-        settings.AddReaderToSettings(OpenApiConstants.Yaml, yamlReader);
-        settings.AddReaderToSettings(OpenApiConstants.Yml, yamlReader);
-    }
-    private static void AddReaderToSettings(this OpenApiReaderSettings settings, string format, IOpenApiReader reader)
-    {
-#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP || NET5_0_OR_GREATER
-        settings.Readers.TryAdd(format, reader);
-#else
-        if (!settings.Readers.ContainsKey(format))
-        {
-            settings.Readers.Add(format, reader);
-        }
-#endif
+        settings.TryAddReader(OpenApiConstants.Yaml, yamlReader);
+        settings.TryAddReader(OpenApiConstants.Yml, yamlReader);
     }
 }
diff --git a/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs b/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
@@ -66,7 +66,7 @@ public static T Load<T>(MemoryStream input, OpenApiSpecVersion version, string f
         {
             format ??= InspectStreamFormat(input);
             settings ??= DefaultReaderSettings.Value;
-            return settings.Readers[format].ReadFragment<T>(input, version, openApiDocument, out diagnostic, settings);
+            return settings.GetReader(format).ReadFragment<T>(input, version, openApiDocument, out diagnostic, settings);
         }
 
         /// <summary>
@@ -239,7 +239,7 @@ public static T Parse<T>(string input,
         private static async Task<ReadResult> InternalLoadAsync(Stream input, string format, OpenApiReaderSettings settings, CancellationToken cancellationToken = default)
         {
             settings ??= DefaultReaderSettings.Value;
-            var reader = settings.Readers[format];
+            var reader = settings.GetReader(format);
             var readResult = await reader.ReadAsync(input, settings, cancellationToken).ConfigureAwait(false);
 
             if (settings.LoadExternalRefs)
@@ -280,7 +280,7 @@ private static ReadResult InternalLoad(MemoryStream input, string format, OpenAp
                 throw new ArgumentException($"Cannot parse the stream: {nameof(input)} is empty or contains no elements.");
             }
 
-            var reader = settings.Readers[format];
+            var reader = settings.GetReader(format);
             var readResult = reader.Read(input, settings);
             return readResult;
         }
diff --git a/src/Microsoft.OpenApi/Reader/OpenApiReaderSettings.cs b/src/Microsoft.OpenApi/Reader/OpenApiReaderSettings.cs
@@ -40,23 +40,66 @@ internal get
         /// </summary>
         public void AddJsonReader()
         {
+            TryAddReader(OpenApiConstants.Json, new OpenApiJsonReader());
+        }
+        /// <summary>
+        /// Gets the reader for the specified format
+        /// </summary>
+        /// <param name="format">Format to fetch the reader for</param>
+        /// <returns>The retrieved reader</returns>
+        /// <exception cref="NotSupportedException">When no reader is registered for that format</exception>
+        internal IOpenApiReader GetReader(string format)
+        {
+            Utils.CheckArgumentNullOrEmpty(format);
+            if (Readers.TryGetValue(format, out var reader))
+            {
+                return reader;
+            }
+
+            throw new NotSupportedException($"Format '{format}' is not supported.");
+        }
+        /// <summary>
+        /// Adds a reader for the specified format.
+        /// This method is a no-op if the reader already exists.
+        /// This method is equivalent to TryAdd, is provided for compatibility reasons and TryAdd should be used instead when available.
+        /// </summary>
+        /// <param name="format">Format to add a reader for</param>
+        /// <param name="reader">Reader to add</param>
+        /// <returns>True if the reader was added, false if it already existed</returns>
+        public bool TryAddReader(string format, IOpenApiReader reader)
+        {
+            Utils.CheckArgumentNullOrEmpty(format);
+            Utils.CheckArgumentNull(reader);
 #if NETSTANDARD2_1_OR_GREATER || NETCOREAPP || NET5_0_OR_GREATER
-            Readers.TryAdd(OpenApiConstants.Json, new OpenApiJsonReader());
+            return Readers.TryAdd(format, reader);
 #else
-            if (!Readers.ContainsKey(OpenApiConstants.Json))
+            if (!Readers.ContainsKey(format))
             {
-                Readers.Add(OpenApiConstants.Json, new OpenApiJsonReader());
+                Readers.Add(format, reader);
+                return true;
             }
+            return false;
 #endif
         }
-        /// <summary>
-        /// Readers to use to parse the OpenAPI document
-        /// </summary>
-        public Dictionary<string, IOpenApiReader> Readers { get; init; } = new Dictionary<string, IOpenApiReader>(StringComparer.OrdinalIgnoreCase)
+        private Dictionary<string, IOpenApiReader> _readers = new(StringComparer.OrdinalIgnoreCase)
         {
             { OpenApiConstants.Json, new OpenApiJsonReader() }
         };
         /// <summary>
+        /// Readers to use to parse the OpenAPI document
+        /// </summary>
+        public Dictionary<string, IOpenApiReader> Readers
+        { 
+            get => _readers;
+            init
+            {
+                Utils.CheckArgumentNull(value);
+                _readers = value.Comparer == StringComparer.OrdinalIgnoreCase ?
+                    value :
+                    new Dictionary<string, IOpenApiReader>(value, StringComparer.OrdinalIgnoreCase);
+            }
+        }
+        /// <summary>
         /// When external references are found, load them into a shared workspace
         /// </summary>
         public bool LoadExternalRefs { get; set; } = false;
@@ -107,18 +150,21 @@ public void AddJsonReader()
         /// </summary>
         public void AddMicrosoftExtensionParsers()
         {
-            if (!ExtensionParsers.ContainsKey(OpenApiPagingExtension.Name))
-                ExtensionParsers.Add(OpenApiPagingExtension.Name, static (i, _) => OpenApiPagingExtension.Parse(i));
-            if (!ExtensionParsers.ContainsKey(OpenApiEnumValuesDescriptionExtension.Name))
-                ExtensionParsers.Add(OpenApiEnumValuesDescriptionExtension.Name, static (i, _ ) => OpenApiEnumValuesDescriptionExtension.Parse(i));
-            if (!ExtensionParsers.ContainsKey(OpenApiPrimaryErrorMessageExtension.Name))
-                ExtensionParsers.Add(OpenApiPrimaryErrorMessageExtension.Name, static (i, _ ) => OpenApiPrimaryErrorMessageExtension.Parse(i));
-            if (!ExtensionParsers.ContainsKey(OpenApiDeprecationExtension.Name))
-                ExtensionParsers.Add(OpenApiDeprecationExtension.Name, static (i, _ ) => OpenApiDeprecationExtension.Parse(i));
-            if (!ExtensionParsers.ContainsKey(OpenApiReservedParameterExtension.Name))
-                ExtensionParsers.Add(OpenApiReservedParameterExtension.Name, static (i, _ ) => OpenApiReservedParameterExtension.Parse(i));
-            if (!ExtensionParsers.ContainsKey(OpenApiEnumFlagsExtension.Name))
-                ExtensionParsers.Add(OpenApiEnumFlagsExtension.Name, static (i, _ ) => OpenApiEnumFlagsExtension.Parse(i));
+            TryAddExtensionParser(OpenApiPagingExtension.Name, static (i, _) => OpenApiPagingExtension.Parse(i));
+            TryAddExtensionParser(OpenApiEnumValuesDescriptionExtension.Name, static (i, _ ) => OpenApiEnumValuesDescriptionExtension.Parse(i));
+            TryAddExtensionParser(OpenApiPrimaryErrorMessageExtension.Name, static (i, _ ) => OpenApiPrimaryErrorMessageExtension.Parse(i));
+            TryAddExtensionParser(OpenApiDeprecationExtension.Name, static (i, _ ) => OpenApiDeprecationExtension.Parse(i));
+            TryAddExtensionParser(OpenApiReservedParameterExtension.Name, static (i, _ ) => OpenApiReservedParameterExtension.Parse(i));
+            TryAddExtensionParser(OpenApiEnumFlagsExtension.Name, static (i, _ ) => OpenApiEnumFlagsExtension.Parse(i));
+        }
+        private void TryAddExtensionParser(string name, Func<JsonNode, OpenApiSpecVersion, IOpenApiExtension> parser)
+        {
+#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP || NET5_0_OR_GREATER
+            ExtensionParsers.TryAdd(name, parser);
+#else
+            if (!ExtensionParsers.ContainsKey(name))
+                ExtensionParsers.Add(name, parser);
+#endif
         }
     }
 }
diff --git a/test/Microsoft.OpenApi.Readers.Tests/OpenApiReaderSettingsExtensionsTests.cs b/test/Microsoft.OpenApi.Readers.Tests/OpenApiReaderSettingsExtensionsTests.cs
@@ -0,0 +1,32 @@
+using System;
+using Microsoft.OpenApi.Models;
+using Microsoft.OpenApi.Reader;
+using Xunit;
+
+namespace Microsoft.OpenApi.Readers.Tests;
+
+public class OpenApiReaderSettingsExtensionsTests
+{
+    [Fact]
+    public void AddsYamlReader()
+    {
+        var settings = new OpenApiReaderSettings();
+        Assert.Single(settings.Readers);
+        Assert.DoesNotContain(OpenApiConstants.Yaml, settings.Readers.Keys);
+        Assert.DoesNotContain(OpenApiConstants.Yml, settings.Readers.Keys);
+
+        settings.AddYamlReader();
+        Assert.Equal(3, settings.Readers.Count);
+        Assert.Contains(OpenApiConstants.Yaml, settings.Readers.Keys);
+        Assert.Contains(OpenApiConstants.Yml, settings.Readers.Keys);
+        Assert.IsType<OpenApiYamlReader>(settings.GetReader(OpenApiConstants.Yaml));
+        Assert.IsType<OpenApiYamlReader>(settings.GetReader(OpenApiConstants.Yml));
+    }
+    [Fact]
+    public void IsAvailableOnSameNamespace()
+    {
+        var settingsNS = typeof(OpenApiReaderSettings).Namespace;
+        var extensionsNS = typeof(OpenApiReaderSettingsExtensions).Namespace;
+        Assert.Equal(settingsNS, extensionsNS, StringComparer.Ordinal);
+    }
+}
diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
@@ -1504,6 +1504,7 @@ namespace Microsoft.OpenApi.Reader
         public Microsoft.OpenApi.Validations.ValidationRuleSet RuleSet { get; set; }
         public void AddJsonReader() { }
         public void AddMicrosoftExtensionParsers() { }
+        public bool TryAddReader(string format, Microsoft.OpenApi.Interfaces.IOpenApiReader reader) { }
     }
     public static class OpenApiVersionExtensionMethods
     {
diff --git a/test/Microsoft.OpenApi.Tests/Reader/OpenApiReaderSettingsTests.cs b/test/Microsoft.OpenApi.Tests/Reader/OpenApiReaderSettingsTests.cs
@@ -0,0 +1,78 @@
+using System;
+using Microsoft.OpenApi.MicrosoftExtensions;
+using Microsoft.OpenApi.Models;
+using Xunit;
+
+namespace Microsoft.OpenApi.Reader.Tests;
+
+public class OpenApiReaderSettingsTests
+{
+    [Fact]
+    public void Defensive()
+    {
+        var settings = new OpenApiReaderSettings();
+        Assert.Throws<ArgumentNullException>(() => settings.GetReader(null));
+        Assert.Throws<ArgumentNullException>(() => settings.GetReader(string.Empty));
+
+        Assert.Throws<ArgumentNullException>(() => settings.TryAddReader(null, null));
+        Assert.Throws<ArgumentNullException>(() => settings.TryAddReader(string.Empty, null));
+        Assert.Throws<ArgumentNullException>(() => settings.TryAddReader(null, new OpenApiJsonReader()));
+        Assert.Throws<ArgumentNullException>(() => settings.TryAddReader(string.Empty, new OpenApiJsonReader()));
+        Assert.Throws<ArgumentNullException>(() => settings.TryAddReader("json", null));
+    }
+
+    [Fact]
+    public void Defaults()
+    {
+        var settings = new OpenApiReaderSettings();
+        Assert.NotNull(settings.HttpClient);
+
+        Assert.IsType<OpenApiJsonReader>(settings.GetReader(OpenApiConstants.Json));
+        Assert.Throws<NotSupportedException>(() =>settings.GetReader(OpenApiConstants.Yaml));
+        Assert.Single(settings.Readers);
+
+        Assert.Equal(StringComparer.OrdinalIgnoreCase, settings.Readers.Comparer);
+
+        Assert.False(settings.TryAddReader("json", new OpenApiJsonReader()));
+        Assert.Empty(settings.ExtensionParsers);
+    }
+    [Fact]
+    public void InitializesReadersWithComparer()
+    {
+        var settings = new OpenApiReaderSettings
+        {
+            Readers = []
+        };
+
+        Assert.Equal(StringComparer.OrdinalIgnoreCase, settings.Readers.Comparer);
+    }
+    [Fact]
+    public void AddsMicrosoftExtensions()
+    {
+        var settings = new OpenApiReaderSettings();
+        Assert.Empty(settings.ExtensionParsers);
+        settings.AddMicrosoftExtensionParsers();
+
+        Assert.NotEmpty(settings.ExtensionParsers);
+        Assert.Contains(OpenApiPagingExtension.Name, settings.ExtensionParsers.Keys);
+        Assert.Contains(OpenApiEnumValuesDescriptionExtension.Name, settings.ExtensionParsers.Keys);
+        Assert.Contains(OpenApiPrimaryErrorMessageExtension.Name, settings.ExtensionParsers.Keys);
+        Assert.Contains(OpenApiDeprecationExtension.Name, settings.ExtensionParsers.Keys);
+        Assert.Contains(OpenApiReservedParameterExtension.Name, settings.ExtensionParsers.Keys);
+        Assert.Contains(OpenApiEnumFlagsExtension.Name, settings.ExtensionParsers.Keys);
+    }
+    [Fact]
+    public void AddsJsonReader()
+    {
+        var settings = new OpenApiReaderSettings()
+        {
+            Readers = []
+        };
+
+        Assert.Empty(settings.Readers);
+
+        settings.AddJsonReader();
+        Assert.Single(settings.Readers);
+        Assert.IsType<OpenApiJsonReader>(settings.GetReader(OpenApiConstants.Json));
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ public static T Load<T>(MemoryStream input, OpenApiSpecVersion version, string f`
`66`	`66`	`{`
`67`	`67`	`format ??= InspectStreamFormat(input);`
`68`	`68`	`settings ??= DefaultReaderSettings.Value;`
`69`		`- return settings.Readers[format].ReadFragment<T>(input, version, openApiDocument, out diagnostic, settings);`
	`69`	`+ return settings.GetReader(format).ReadFragment<T>(input, version, openApiDocument, out diagnostic, settings);`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`/// <summary>`
`@@ -239,7 +239,7 @@ public static T Parse<T>(string input,`
`239`	`239`	`private static async Task<ReadResult> InternalLoadAsync(Stream input, string format, OpenApiReaderSettings settings, CancellationToken cancellationToken = default)`
`240`	`240`	`{`
`241`	`241`	`settings ??= DefaultReaderSettings.Value;`
`242`		`- var reader = settings.Readers[format];`
	`242`	`+ var reader = settings.GetReader(format);`
`243`	`243`	`var readResult = await reader.ReadAsync(input, settings, cancellationToken).ConfigureAwait(false);`
`244`	`244`
`245`	`245`	`if (settings.LoadExternalRefs)`
`@@ -280,7 +280,7 @@ private static ReadResult InternalLoad(MemoryStream input, string format, OpenAp`
`280`	`280`	`throw new ArgumentException($"Cannot parse the stream: {nameof(input)} is empty or contains no elements.");`
`281`	`281`	`}`
`282`	`282`
`283`		`- var reader = settings.Readers[format];`
	`283`	`+ var reader = settings.GetReader(format);`
`284`	`284`	`var readResult = reader.Read(input, settings);`
`285`	`285`	`return readResult;`
`286`	`286`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1504,6 +1504,7 @@ namespace Microsoft.OpenApi.Reader`
`1504`	`1504`	`public Microsoft.OpenApi.Validations.ValidationRuleSet RuleSet { get; set; }`
`1505`	`1505`	`public void AddJsonReader() { }`
`1506`	`1506`	`public void AddMicrosoftExtensionParsers() { }`
	`1507`	`+ public bool TryAddReader(string format, Microsoft.OpenApi.Interfaces.IOpenApiReader reader) { }`
`1507`	`1508`	`}`
`1508`	`1509`	`public static class OpenApiVersionExtensionMethods`
`1509`	`1510`	`{`