Improve testing for converters (#316)

This revealed that the ConvertTo for these methods didn't seem to work at all. (It would invariably fail when trying to validate its converted result, because it creates a string, but validation for these types fails unless the input is of the corresponding JToken type.)

Also, there were inconsistencies in how validation failures were being reported, which this fixes.

Note that this now uses the .NET 7 SDK in build pipeline (but only for access to new language features - there's no change in target runtimes).

Author: idg10

Solutions/Menes.Abstractions/Menes/Converters/BooleanConverter.cs

@@ -50,7 +50,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
         {
             string result = JsonConvert.SerializeObject(instance, this.configuration.Formatting, this.configuration.SerializerSettings);
 
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(JToken.Parse(result), schema);
 
             return result;
         }

Solutions/Menes.Abstractions/Menes/Converters/ByteArrayConverter.cs

@@ -46,7 +46,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
 
             this.validator.ValidateAndThrow(result, schema);
 
-            return result;
+            return $"\"{result}\"";
         }
     }
 }

Solutions/Menes.Abstractions/Menes/Converters/DateConverter.cs

@@ -5,9 +5,11 @@
 namespace Menes.Converters
 {
     using System;
+
     using Menes.Validation;
+
     using Microsoft.OpenApi.Models;
-    using Newtonsoft.Json;
+
     using Newtonsoft.Json.Linq;
 
     /// <summary>
@@ -16,17 +18,14 @@ namespace Menes.Converters
     public class DateConverter : IOpenApiConverter
     {
         private readonly OpenApiSchemaValidator validator;
-        private readonly IOpenApiConfiguration configuration;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DateConverter"/> class.
         /// </summary>
         /// <param name="validator">The <see cref="OpenApiSchemaValidator"/>.</param>
-        /// <param name="configuration">The OpenAPI host configuration.</param>
-        public DateConverter(OpenApiSchemaValidator validator, IOpenApiConfiguration configuration)
+        public DateConverter(OpenApiSchemaValidator validator)
         {
             this.validator = validator;
-            this.configuration = configuration;
         }
 
         /// <inheritdoc/>
@@ -39,19 +38,25 @@ public bool CanConvert(OpenApiSchema schema, bool ignoreFormat = false)
         public object ConvertFrom(string content, OpenApiSchema schema)
         {
             JToken token = content;
-            var result = (DateTimeOffset)token;
+            var result = new DateTimeOffset(DateTime.SpecifyKind((DateTime)token, DateTimeKind.Utc));
 
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(token, schema);
 
             return result;
         }
 
         /// <inheritdoc/>
         public string ConvertTo(object instance, OpenApiSchema schema)
         {
-            string result = JsonConvert.SerializeObject(instance, this.configuration.Formatting, this.configuration.SerializerSettings);
+            string result = instance switch
+            {
+                DateTimeOffset dt => $"\"{dt:yyyy-MM-dd}\"",
+                DateTime dt => $"\"{dt:yyyy-MM-dd}\"",
+                DateOnly dt => $"\"{dt:yyyy-MM-dd}\"",
+                _ => throw new ArgumentException($"Unsupported source type {instance.GetType().FullName}"),
+            };
 
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(JToken.Parse(result), schema);
 
             return result;
         }

Solutions/Menes.Abstractions/Menes/Converters/DoubleConverter.cs

@@ -49,7 +49,7 @@ public object ConvertFrom(string content, OpenApiSchema schema)
         public string ConvertTo(object instance, OpenApiSchema schema)
         {
             string result = JsonConvert.SerializeObject(instance, this.configuration.Formatting, this.configuration.SerializerSettings);
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(JToken.Parse(result), schema);
 
             return result;
         }

Solutions/Menes.Abstractions/Menes/Converters/FloatConverter.cs

@@ -51,7 +51,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
         {
             string result = JsonConvert.SerializeObject(instance, this.configuration.Formatting, this.configuration.SerializerSettings);
 
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(JToken.Parse(result), schema);
 
             return result;
         }

Solutions/Menes.Abstractions/Menes/Converters/IOpenApiConverter.cs

@@ -9,6 +9,42 @@ namespace Menes.Converters
     /// <summary>
     /// Implemented by types that can convert a value to/from an OpenAPI schema value.
     /// </summary>
+    /// <remarks>
+    /// <para>
+    /// This interface has a slight internal inconsistency: string value handling is different for
+    /// inputs (<see cref="ConvertFrom(string, OpenApiSchema)"/>) and outputs
+    /// (<see cref="ConvertTo(object, OpenApiSchema)"/>). String-typed inputs normally come from
+    /// the path, query, or sometimes a header or cookie, and in these cases, we don't wrap the
+    /// strings with double quotes. This is in contrast to JSON where you can always tell the
+    /// difference between a string value and some other value, e.g.:
+    /// </para>
+    /// <code>
+    /// {
+    ///   "booleanValue": true,
+    ///   "stringValue": "true"
+    /// </code>
+    /// <para>
+    /// If inputs were always in JSON form, you'd see a similar difference. For example, if an
+    /// input appears in a query string, <c>http://example.com/?x=true</c> would unambiguously
+    /// mean that the parameter x has the JSON boolean value <c>true</c>, whereas if we wanted a
+    /// string value, we'd use <c>http://example.com/?x=%22true%22</c>. But in practice, it's
+    /// unusual for web sites to use this convention. In practice, all query string parameters
+    /// have string values, and it's up to the web server whether it chooses to interpret them
+    /// as having a particular format.
+    /// </para>
+    /// <para>
+    /// For this reason, this interface expects incoming string values to be unquoted, because
+    /// in most cases they are. The one exception is if the entire request body is a single
+    /// string. If the request <c>Content-Type</c> is <c>application/json</c> then it must be
+    /// enclosing in double quotes. But for anything other than the body, those quotes will not
+    /// be present in the raw inputs. But outputs are always in JSON form,
+    /// <see cref="ConvertTo(object, Microsoft.OpenApi.Models.OpenApiSchema)"/> returns quoted
+    /// strings. We could have insisted on consistency here, but that would have required almost
+    /// all calls to <see cref="ConvertFrom(string, OpenApiSchema)"/> to create new strings
+    /// wrapping the existing input values in quotes. This would add noise to the code, and require
+    /// additional string allocations, so instead, we just live with asymmetry in this interface.
+    /// </para>
+    /// </remarks>
     public interface IOpenApiConverter
     {
         /// <summary>
@@ -22,7 +58,10 @@ public interface IOpenApiConverter
         /// <summary>
         /// Convert from the specified content to an object of the required type.
         /// </summary>
-        /// <param name="content">The content to convert.</param>
+        /// <param name="content">
+        /// The content to convert. If the input data is of type string, this will not be
+        /// enclosed in double quotes.
+        /// </param>
         /// <param name="schema">The schema of the content to convert.</param>
         /// <returns>An instance of the converted object.</returns>
         object ConvertFrom(string content, OpenApiSchema schema);
@@ -32,7 +71,11 @@ public interface IOpenApiConverter
         /// </summary>
         /// <param name="instance">The instance to convert.</param>
         /// <param name="schema">The schema of the content to convert.</param>
-        /// <returns>A string representation of the converted object for the output document.</returns>
+        /// <returns>
+        /// A JSON representation of the converted object for the output document. Unlike with
+        /// <see cref="ConvertFrom(string, OpenApiSchema)"/>, if the data is a JSON string,
+        /// it will be enclosed in double quotes.
+        /// </returns>
         string ConvertTo(object instance, OpenApiSchema schema);
     }
 }

Solutions/Menes.Abstractions/Menes/Converters/Integer32Converter.cs

@@ -4,6 +4,8 @@
 
 namespace Menes.Converters
 {
+    using System;
+
     using Menes.Validation;
     using Microsoft.OpenApi.Models;
     using Newtonsoft.Json;
@@ -39,7 +41,15 @@ public object ConvertFrom(string content, OpenApiSchema schema)
         {
             JToken token = content;
 
-            int result = (int)token;
+            int result;
+            try
+            {
+                result = (int)token;
+            }
+            catch (OverflowException)
+            {
+                throw new FormatException("Number was too large to parse as an Int32");
+            }
 
             this.validator.ValidateAndThrow(result, schema);
 
@@ -51,7 +61,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
         {
             string result = JsonConvert.SerializeObject(instance, this.configuration.Formatting, this.configuration.SerializerSettings);
 
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(JToken.Parse(result), schema);
 
             return result;
         }

Solutions/Menes.Abstractions/Menes/Converters/Integer64Converter.cs

@@ -4,6 +4,8 @@
 
 namespace Menes.Converters
 {
+    using System;
+
     using Menes.Validation;
     using Microsoft.OpenApi.Models;
     using Newtonsoft.Json;
@@ -39,7 +41,16 @@ public object ConvertFrom(string content, OpenApiSchema schema)
         {
             JToken token = content;
 
-            long result = (long)token;
+            long result;
+            try
+            {
+                result = (long)token;
+            }
+            catch (OverflowException)
+            {
+                throw new FormatException("Number was too large to parse as an Int64");
+            }
+
             this.validator.ValidateAndThrow(result, schema);
 
             return result;
@@ -50,7 +61,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
         {
             string result = JsonConvert.SerializeObject(instance, this.configuration.Formatting, this.configuration.SerializerSettings);
 
-            this.validator.ValidateAndThrow(result, schema);
+            this.validator.ValidateAndThrow(JToken.Parse(result), schema);
 
             return result;
         }

Solutions/Menes.Abstractions/Menes/Converters/PasswordConverter.cs

@@ -44,7 +44,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
 
             this.validator.ValidateAndThrow(result, schema);
 
-            return result;
+            return $"\"{result}\"";
         }
     }
 }

Solutions/Menes.Abstractions/Menes/Converters/StringConverter.cs

@@ -53,7 +53,7 @@ public string ConvertTo(object instance, OpenApiSchema schema)
 
             this.validator.ValidateAndThrow(result, schema);
 
-            return result;
+            return '"' + result + '"';
         }
     }
 }