fix: handle deserializing and writing empty security requirements #1426 (#2323)

* fix!: handle deserializing and writing empty security requirements #1426

make distinction between empty security requirements and no security
requirements on an operation. empty security requirements are read
as an empty list, no security requirements are read as null for
OpenAPI v2/v3/v3.1. This is a breaking change, previously both cases
were read as an empty list.

also includes a change to OpenApiOperation.SerializeInternal so
it can serialize these two cases separately. this required a new method
OpenApiWriterExtensions.WriteOptionalOrEmptyCollection.

includes unit tests, change to PublicApi.approved.txt to include the
new method, and I removed a couple of unused usings and a typo in
test name `SerializeDocWithSecuritySchemeWithInlineReferencesWorks`.

* Review: make security property tests more explicit

* Review: remove unneeded list creation from list

Author: paulmendix

src/Microsoft.OpenApi/Models/OpenApiOperation.cs

@@ -214,8 +214,8 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
             writer.WriteProperty(OpenApiConstants.Deprecated, Deprecated, false);
 
             // security
-            writer.WriteOptionalCollection(OpenApiConstants.Security, Security, callback);
-
+            writer.WriteOptionalOrEmptyCollection(OpenApiConstants.Security, Security, callback);
+            
             // servers
             writer.WriteOptionalCollection(OpenApiConstants.Servers, Servers, callback);
 

src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs

@@ -9,6 +9,7 @@
 using Microsoft.OpenApi.Models.References;
 using Microsoft.OpenApi.Models.Interfaces;
 using System;
+using System.Text.Json.Nodes;
 
 namespace Microsoft.OpenApi.Reader.V2
 {
@@ -94,7 +95,10 @@ internal static partial class OpenApiV2Deserializer
                 },
                 {
                     "security",
-                    (o, n, t) => o.Security = n.CreateList(LoadSecurityRequirement, t)
+                    (o, n, t) => { if (n.JsonNode is JsonArray)
+                    {
+                        o.Security = n.CreateList(LoadSecurityRequirement, t); 
+                    } }
                 },
             };
 

src/Microsoft.OpenApi/Reader/V3/OpenApiOperationDeserializer.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text.Json.Nodes;
 using Microsoft.OpenApi.Extensions;
 using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Models.References;
@@ -83,7 +84,13 @@ internal static partial class OpenApiV3Deserializer
                 },
                 {
                     "security",
-                    (o, n, t) => o.Security = n.CreateList(LoadSecurityRequirement, t)
+                    (o, n, t) =>
+                    { 
+                        if (n.JsonNode is JsonArray)
+                        {
+                            o.Security = n.CreateList(LoadSecurityRequirement, t);
+                        } 
+                    }
                 },
                 {
                     "servers",

src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text.Json.Nodes;
 using Microsoft.OpenApi.Extensions;
 using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Models.References;
@@ -96,8 +97,11 @@ internal static partial class OpenApiV31Deserializer
                 },
                 {
                     "security", (o, n, t) =>
-                    {
-                        o.Security = n.CreateList(LoadSecurityRequirement, t);
+                    { 
+                        if (n.JsonNode is JsonArray)
+                        {
+                            o.Security = n.CreateList(LoadSecurityRequirement, t); 
+                        } 
                     }
                 },
                 {

src/Microsoft.OpenApi/Writers/OpenApiWriterExtensions.cs

@@ -217,6 +217,26 @@ public static void WriteOptionalCollection<T>(
                 writer.WriteCollectionInternal(name, elements, action);
             }
         }
+        
+        /// <summary>
+        /// Write the optional or empty Open API object/element collection.
+        /// </summary>
+        /// <typeparam name="T">The Open API element type. <see cref="IOpenApiElement"/></typeparam>
+        /// <param name="writer">The Open API writer.</param>
+        /// <param name="name">The property name.</param>
+        /// <param name="elements">The collection values.</param>
+        /// <param name="action">The collection element writer action.</param>
+        public static void WriteOptionalOrEmptyCollection<T>(
+            this IOpenApiWriter writer,
+            string name,
+            IEnumerable<T>? elements,
+            Action<IOpenApiWriter, T> action)
+        {
+            if (elements != null)
+            {
+                writer.WriteCollectionInternal(name, elements, action);
+            }
+        }
 
         /// <summary>
         /// Write the required Open API object/element collection.

test/Microsoft.OpenApi.Tests/Microsoft.OpenApi.Tests.csproj

@@ -56,5 +56,13 @@
     </None>
 
     <None Update="PublicApi\PublicApi.approved.txt" CopyToOutputDirectory="Always" />
+      
+    <None Update="Models\Samples\docWithoutOperationSecurity.yaml">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+      
+    <None Update="Models\Samples\docWithEmptyOperationSecurity.yaml">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
   </ItemGroup>
 </Project>

test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs

@@ -12,7 +12,6 @@
 using Microsoft.OpenApi.Models.Interfaces;
 using Microsoft.OpenApi.Models.References;
 using Microsoft.OpenApi.Writers;
-using Microsoft.VisualBasic;
 using VerifyXunit;
 using Xunit;
 
@@ -2177,7 +2176,7 @@ public void SerializeAsThrowsIfVersionIsNotSupported()
         }
 
         [Fact]
-        public async Task SerializeDocWithSecuritySchemeWithInlineRefererencesWorks()
+        public async Task SerializeDocWithSecuritySchemeWithInlineReferencesWorks()
         {
             var expected = @"openapi: 3.0.4
 info:
@@ -2218,5 +2217,75 @@ public async Task SerializeDocWithSecuritySchemeWithInlineRefererencesWorks()
             var actual = stringWriter.ToString();
             Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral());
         }
+        
+        [Fact]
+        public async Task SerializeDocWithoutOperationSecurityWorks()
+        {
+            var expected = """
+                           openapi: 3.0.4
+                           info:
+                             title: Repair Service
+                             version: 1.0.0
+                           servers:
+                             - url: https://pluginrentu.azurewebsites.net/api
+                           paths:
+                             /repairs:
+                               get:
+                                 summary: List all repairs
+                                 description: Returns a list of repairs with their details and images
+                                 operationId: listRepairs
+                                 responses:
+                                   '200':
+                                     description: A list of repairs
+                                     content:
+                                       application/json:
+                                         schema:
+                                           type: object
+                           """;
+
+            var doc = (await OpenApiDocument.LoadAsync("Models/Samples/docWithoutOperationSecurity.yaml", SettingsFixture.ReaderSettings)).Document;
+            var stringWriter = new StringWriter();
+            doc!.SerializeAsV3(new OpenApiYamlWriter(stringWriter, new OpenApiWriterSettings { InlineLocalReferences = true }));
+            var actual = stringWriter.ToString();
+            Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral());
+            var actualOperation = doc.Paths["/repairs"]!.Operations![HttpMethod.Get];
+            Assert.Null(actualOperation.Security);
+        }
+        
+        [Fact]
+        public async Task SerializeDocWithEmptyOperationSecurityWorks()
+        {
+            var expected = """
+                           openapi: 3.0.4
+                           info:
+                             title: Repair Service
+                             version: 1.0.0
+                           servers:
+                             - url: https://pluginrentu.azurewebsites.net/api
+                           paths:
+                             /repairs:
+                               get:
+                                 summary: List all repairs
+                                 description: Returns a list of repairs with their details and images
+                                 operationId: listRepairs
+                                 responses:
+                                   '200':
+                                     description: A list of repairs
+                                     content:
+                                       application/json:
+                                         schema:
+                                           type: object
+                                 security: [ ]
+                           """;
+
+            var doc = (await OpenApiDocument.LoadAsync("Models/Samples/docWithEmptyOperationSecurity.yaml", SettingsFixture.ReaderSettings)).Document;
+            var stringWriter = new StringWriter();
+            doc!.SerializeAsV3(new OpenApiYamlWriter(stringWriter, new OpenApiWriterSettings { InlineLocalReferences = true }));
+            var actual = stringWriter.ToString();
+            Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), actual.MakeLineBreaksEnvironmentNeutral());
+            var actualOperation = doc.Paths["/repairs"]!.Operations![HttpMethod.Get];
+            Assert.NotNull(actualOperation.Security);
+            Assert.Empty(actualOperation.Security);
+        }
     }
 }

test/Microsoft.OpenApi.Tests/Models/Samples/docWithEmptyOperationSecurity.yaml

@@ -0,0 +1,20 @@
+openapi: 3.0.0
+info:
+  title: Repair Service
+  version: 1.0.0
+servers:
+  - url: https://pluginrentu.azurewebsites.net/api
+paths:
+  /repairs:
+    get:
+      operationId: listRepairs
+      summary: List all repairs
+      description: Returns a list of repairs with their details and images
+      responses:
+        '200':
+          description: A list of repairs
+          content:
+            application/json:
+              schema:
+                type: object
+      security: []

test/Microsoft.OpenApi.Tests/Models/Samples/docWithoutOperationSecurity.yaml

@@ -0,0 +1,19 @@
+openapi: 3.0.0
+info:
+  title: Repair Service
+  version: 1.0.0
+servers:
+  - url: https://pluginrentu.azurewebsites.net/api
+paths:
+  /repairs:
+    get:
+      operationId: listRepairs
+      summary: List all repairs
+      description: Returns a list of repairs with their details and images
+      responses:
+        '200':
+          description: A list of repairs
+          content:
+            application/json:
+              schema:
+                type: object

test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt

@@ -1967,6 +1967,7 @@ namespace Microsoft.OpenApi.Writers
         public static void WriteOptionalMap<T>(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, System.Collections.Generic.Dictionary<string, T>? elements, System.Action<Microsoft.OpenApi.Writers.IOpenApiWriter, string, T> action)
             where T : Microsoft.OpenApi.Interfaces.IOpenApiElement { }
         public static void WriteOptionalObject<T>(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, T? value, System.Action<Microsoft.OpenApi.Writers.IOpenApiWriter, T> action) { }
+        public static void WriteOptionalOrEmptyCollection<T>(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, System.Collections.Generic.IEnumerable<T>? elements, System.Action<Microsoft.OpenApi.Writers.IOpenApiWriter, T> action) { }
         public static void WriteProperty(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, string? value) { }
         public static void WriteProperty(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, bool value, bool defaultValue = false) { }
         public static void WriteProperty(this Microsoft.OpenApi.Writers.IOpenApiWriter writer, string name, bool? value, bool defaultValue = false) { }