Merge pull request #186 from Microsoft/PerthCharern/FixFormData

Fix formData/body parameter and consumes handling

Fix the serialization and deserialization logic for V2 formData and body parameter.

V2 operation deserialization now respects the consumes property.

Unit tests verify that serializing, deserializing, and re-deserializing all work as expected.

Author: PerthCharern

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

@@ -92,13 +92,15 @@ internal static partial class OpenApiV2Deserializer
                 {s => s.StartsWith("x-"), (o, p, n) => o.AddExtension(p, n.CreateAny())}
             };
 
-        private static FixedFieldMap<OpenApiResponses> _responsesFixedFields = new FixedFieldMap<OpenApiResponses>();
+        private static readonly FixedFieldMap<OpenApiResponses> _responsesFixedFields =
+            new FixedFieldMap<OpenApiResponses>();
 
-        private static PatternFieldMap<OpenApiResponses> _responsesPatternFields = new PatternFieldMap<OpenApiResponses>
-        {
-            {s => !s.StartsWith("x-"), (o, p, n) => o.Add(p, LoadResponse(n))},
-            {s => s.StartsWith("x-"), (o, p, n) => o.AddExtension(p, n.CreateAny())}
-        };
+        private static readonly PatternFieldMap<OpenApiResponses> _responsesPatternFields =
+            new PatternFieldMap<OpenApiResponses>
+            {
+                {s => !s.StartsWith("x-"), (o, p, n) => o.Add(p, LoadResponse(n))},
+                {s => s.StartsWith("x-"), (o, p, n) => o.AddExtension(p, n.CreateAny())}
+            };
 
         internal static OpenApiOperation LoadOperation(ParseNode node)
         {
@@ -119,7 +121,7 @@ internal static OpenApiOperation LoadOperation(ParseNode node)
                 var formParameters = node.Context.GetFromTempStorage<List<OpenApiParameter>>("formParameters");
                 if (formParameters != null)
                 {
-                    operation.RequestBody = CreateFormBody(formParameters);
+                    operation.RequestBody = CreateFormBody(node.Context, formParameters);
                 }
             }
 
@@ -137,22 +139,33 @@ public static OpenApiResponses LoadResponses(ParseNode node)
             return domainObject;
         }
 
-        private static OpenApiRequestBody CreateFormBody(List<OpenApiParameter> formParameters)
+        private static OpenApiRequestBody CreateFormBody(ParsingContext context, List<OpenApiParameter> formParameters)
         {
             var mediaType = new OpenApiMediaType
             {
                 Schema = new OpenApiSchema
                 {
-                    Properties = formParameters.ToDictionary(k => k.Name, v => v.Schema)
+                    Properties = formParameters.ToDictionary(
+                        k => k.Name,
+                        v =>
+                        {
+                            var schema = v.Schema;
+                            schema.Description = v.Description;
+                            return schema;
+                        }),
+                    Required = formParameters.Where(p => p.Required).Select(p => p.Name).ToList()
                 }
             };
 
+            var consumes = context.GetFromTempStorage<List<string>>("operationconsumes") ??
+                context.GetFromTempStorage<List<string>>("globalconsumes") ??
+                new List<string> {"application/x-www-form-urlencoded"};
+
             var formBody = new OpenApiRequestBody
             {
-                Content = new Dictionary<string, OpenApiMediaType>
-                {
-                    {"application/x-www-form-urlencoded", mediaType}
-                }
+                Content = consumes.ToDictionary(
+                    k => k,
+                    v => mediaType)
             };
 
             return formBody;
@@ -162,8 +175,8 @@ private static OpenApiRequestBody CreateRequestBody(
             ParsingContext context,
             OpenApiParameter bodyParameter)
         {
-            var consumes = context.GetFromTempStorage<List<string>>("operationproduces") ??
-                context.GetFromTempStorage<List<string>>("globalproduces") ?? new List<string> {"application/json"};
+            var consumes = context.GetFromTempStorage<List<string>>("operationconsumes") ??
+                context.GetFromTempStorage<List<string>>("globalconsumes") ?? new List<string> {"application/json"};
 
             var requestBody = new OpenApiRequestBody
             {

src/Microsoft.OpenApi/Models/OpenApiOperation.cs

@@ -222,16 +222,36 @@ public void SerializeAsV2(IOpenApiWriter writer)
 
                 writer.WriteEndArray();
 
-                // Create a parameter as BodyParameter type and add to Parameters.
-                // This type will be used to populate the In property as "body" when Parameters is serialized.
-                var bodyParameter = new BodyParameter
+                // This is form data. We need to split the request body into multiple parameters.
+                if (consumes.Contains("application/x-www-form-urlencoded") ||
+                    consumes.Contains("multipart/form-data"))
                 {
-                    Description = RequestBody.Description,
-                    Schema = RequestBody.Content.First().Value.Schema,
-                    Format = new List<string>(consumes)
-                };
-
-                parameters.Add(bodyParameter);
+                    foreach (var property in RequestBody.Content.First().Value.Schema.Properties)
+                    {
+                        parameters.Add(
+                            new OpenApiFormDataParameter
+                            {
+                                Description = property.Value.Description,
+                                Name = property.Key,
+                                Schema = property.Value,
+                                Required = RequestBody.Content.First().Value.Schema.Required.Contains(property.Key)
+                            });
+                    }
+                }
+                else
+                {
+                    var bodyParameter = new OpenApiBodyParameter
+                    {
+                        Description = RequestBody.Description,
+                        // V2 spec actually allows the body to have custom name.
+                        // Our library does not support this at the moment.
+                        Name = "body",
+                        Schema = RequestBody.Content.First().Value.Schema,
+                        Required = RequestBody.Required
+                    };
+
+                    parameters.Add(bodyParameter);
+                }
             }
 
             if (Responses != null)

src/Microsoft.OpenApi/Models/OpenApiParameter.cs

@@ -221,24 +221,23 @@ public void SerializeAsV2WithoutReference(IOpenApiWriter writer)
         {
             writer.WriteStartObject();
 
-            // name
             // in
-            if (IsFormDataParameter())
+            if (this is OpenApiFormDataParameter)
             {
-                writer.WriteProperty(OpenApiConstants.Name, "formData");
                 writer.WriteProperty(OpenApiConstants.In, "formData");
             }
-            else if (IsBodyParameter())
+            else if (this is OpenApiBodyParameter)
             {
-                writer.WriteProperty(OpenApiConstants.Name, "body");
                 writer.WriteProperty(OpenApiConstants.In, "body");
             }
             else
             {
-                writer.WriteProperty(OpenApiConstants.Name, Name);
                 writer.WriteProperty(OpenApiConstants.In, In.GetDisplayName());
             }
 
+            // name
+            writer.WriteProperty(OpenApiConstants.Name, Name);
+
             // description
             writer.WriteProperty(OpenApiConstants.Description, Description);
 
@@ -249,7 +248,7 @@ public void SerializeAsV2WithoutReference(IOpenApiWriter writer)
             writer.WriteProperty(OpenApiConstants.Deprecated, Deprecated, false);
 
             // schema
-            if (IsBodyParameter())
+            if (this is OpenApiBodyParameter)
             {
                 writer.WriteOptionalObject(OpenApiConstants.Schema, Schema, (w, s) => s.SerializeAsV2(w));
             }
@@ -283,44 +282,19 @@ public void SerializeAsV2WithoutReference(IOpenApiWriter writer)
 
             writer.WriteEndObject();
         }
-
-        private bool IsBodyParameter()
-        {
-            if (this is BodyParameter)
-            {
-                var parameter = (BodyParameter)this;
-
-                return !(
-                    parameter.Format.Contains("application/x-www-form-urlencoded") ||
-                    parameter.Format.Contains("multipart/form-data"));
-            }
-
-            return false;
-        }
-
-        private bool IsFormDataParameter()
-        {
-            if (this is BodyParameter)
-            {
-                var parameter = (BodyParameter)this;
-
-                return
-                    parameter.Format.Contains("application/x-www-form-urlencoded") ||
-                    parameter.Format.Contains("multipart/form-data");
-            }
-
-            return false;
-        }
     }
 
     /// <summary>
     /// Body parameter class to propagate information needed for <see cref="OpenApiParameter.SerializeAsV2"/>
     /// </summary>
-    internal class BodyParameter : OpenApiParameter
+    internal class OpenApiBodyParameter : OpenApiParameter
+    {
+    }
+
+    /// <summary>
+    /// Form parameter class to propagate information needed for <see cref="OpenApiParameter.SerializeAsV2"/>
+    /// </summary>
+    internal class OpenApiFormDataParameter : OpenApiParameter
     {
-        /// <summary>
-        /// Format of the parameter. This should be the same as the "consumes" property in Operation.
-        /// </summary>
-        public IList<string> Format { get; set; }
     }
 }

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

@@ -31,6 +31,15 @@
       <EmbeddedResource Include="V2Tests\Samples\minimal.v3.yaml">
         <CopyToOutputDirectory>Never</CopyToOutputDirectory>
       </EmbeddedResource>
+      <EmbeddedResource Include="V2Tests\Samples\OpenApiOperation\basicOperation.yaml">
+        <CopyToOutputDirectory>Never</CopyToOutputDirectory>
+      </EmbeddedResource>
+        <EmbeddedResource Include="V2Tests\Samples\OpenApiOperation\operationWithBody.yaml">
+            <CopyToOutputDirectory>Never</CopyToOutputDirectory>
+        </EmbeddedResource>
+        <EmbeddedResource Include="V2Tests\Samples\OpenApiOperation\operationWithFormData.yaml">
+            <CopyToOutputDirectory>Never</CopyToOutputDirectory>
+        </EmbeddedResource>
       <EmbeddedResource Include="V2Tests\Samples\OpenApiParameter\bodyParameter.yaml">
         <CopyToOutputDirectory>Never</CopyToOutputDirectory>
       </EmbeddedResource>