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).

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 + '"';
         }
     }
 }

Solutions/Menes.Abstractions/Menes/Converters/UriConverter.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/Exceptions/OpenApiAccessControlPolicyEvaluationFailedException.cs

@@ -6,7 +6,6 @@ namespace Menes.Exceptions
 {
     using System;
     using System.Linq;
-    using System.Runtime.Serialization;
 
     /// <summary>
     /// An exception that is thrown by an <see cref="IOpenApiAccessControlPolicy"/> if it fails

Solutions/Menes.Abstractions/Menes/Exceptions/OpenApiBadRequestException.cs

@@ -94,6 +94,15 @@ public OpenApiBadRequestException(
             }
         }
 
+        /// <summary>
+        /// Creates a <see cref="OpenApiBadRequestException"/> from an <see cref="OpenApiBadRequestException"/>.
+        /// </summary>
+        /// <param name="formatException">The <see cref="OpenApiBadRequestException"/>.</param>
+        public OpenApiBadRequestException(OpenApiInvalidFormatException formatException)
+            : this(formatException.Message, formatException.Explanation!, formatException.DetailsType, formatException.DetailsInstance)
+        {
+        }
+
         /// <summary>
         /// Gets the detailed explanation of the problem, if present.
         /// </summary>

Solutions/Menes.Abstractions/Menes/Exceptions/OpenApiInvalidFormatException.cs

@@ -0,0 +1,82 @@
+// <copyright file="OpenApiInvalidFormatException.cs" company="Endjin Limited">
+// Copyright (c) Endjin Limited. All rights reserved.
+// </copyright>
+
+namespace Menes.Exceptions
+{
+    using System;
+
+    /// <summary>
+    /// An exception that indicates that data has been found not to be in the format required
+    /// by an OpenAPI specification.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// We share validation code across multiple contexts (validation of inputs, default values
+    /// in an Open API spec, and outputs) and the required externally visible behaviour depends on
+    /// the context in which the problem is detected. The common layer needs to be able to throw
+    /// an exception indicating only that a problem has been detected, and this then needs to be
+    /// turned into a more suitable exception by the higher layers of code.
+    /// </para>
+    /// </remarks>
+    public class OpenApiInvalidFormatException : Exception
+    {
+        /// <summary>
+        /// Creates an <see cref="OpenApiInvalidFormatException"/>.
+        /// </summary>
+        /// <param name="message">
+        /// The value for this exception's <see cref="Exception.Message"/> property.
+        /// </param>
+        public OpenApiInvalidFormatException(string message = "Bad request")
+            : base(message)
+        {
+        }
+
+        /// <summary>
+        /// Creates an <see cref="OpenApiInvalidFormatException"/>.
+        /// </summary>
+        /// <param name="title">
+        /// A short description of the error. This is used as the <see cref="Exception.Message"/>
+        /// property, and will also be returned in the Problem Details in the response body.
+        /// </param>
+        /// <param name="explanation">
+        /// A longer description of the problem. This is returned in the Problem Details in the
+        /// response body.
+        /// </param>
+        /// <param name="detailsType">
+        /// An optional URI identifying the problem type. If specified, this is returned in the
+        /// Problem Details in the response body.
+        /// response body.
+        /// </param>
+        /// <param name="detailsInstance">
+        /// An optional URI identifying the problem instance. If specified, this is returned in the
+        /// Problem Details in the response body.
+        /// </param>
+        public OpenApiInvalidFormatException(
+            string title,
+            string explanation,
+            string? detailsType = null,
+            string? detailsInstance = null)
+            : base(title)
+        {
+            this.Explanation = explanation;
+            this.DetailsType = detailsType;
+            this.DetailsInstance = detailsInstance;
+        }
+
+        /// <summary>
+        /// Gets the detailed explanation of the problem, if present.
+        /// </summary>
+        public string? Explanation { get; }
+
+        /// <summary>
+        /// Gets the URI identifying the problem type, if present.
+        /// </summary>
+        public string? DetailsType { get; }
+
+        /// <summary>
+        /// Gets the URI identifying the problem instance, if present.
+        /// </summary>
+        public string? DetailsInstance { get; }
+    }
+}