[OpenAPI] Improve missing query parameter error (#9492)

Author: tobias-tengler

src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/DynamicEndpointMiddleware.cs

@@ -333,13 +333,19 @@ private static bool TryGetValueForParameter(
                     return false;
                 }
 
+                if (leaf.IsNonNullType)
+                {
+                    throw new BadRequestException(
+                        $"Required route parameter '{leaf.ParameterKey}' is missing");
+                }
+
                 parameterValue = s_nullValueNode;
                 return true;
             }
 
             try
             {
-                parameterValue = ParseValueNode(value, leaf.Type);
+                parameterValue = ParseValueNode(value, leaf.NamedType);
                 return true;
             }
             catch (InvalidFormatException)
@@ -350,20 +356,32 @@ private static bool TryGetValueForParameter(
 
         if (leaf.ParameterType is OpenApiEndpointParameterType.Query)
         {
-            if (!query.TryGetValue(leaf.ParameterKey, out var values) || values is not [{ } value])
+            if (!query.TryGetValue(leaf.ParameterKey, out var values))
             {
                 if (leaf.HasDefaultValue)
                 {
                     return false;
                 }
 
+                if (leaf.IsNonNullType)
+                {
+                    throw new BadRequestException(
+                        $"Required query parameter '{leaf.ParameterKey}' is missing");
+                }
+
                 parameterValue = s_nullValueNode;
                 return true;
             }
 
+            if (values is not [{ } value])
+            {
+                throw new BadRequestException(
+                    $"Query parameter '{leaf.ParameterKey}' can only be specified once.");
+            }
+
             try
             {
-                parameterValue = ParseValueNode(value, leaf.Type);
+                parameterValue = ParseValueNode(value, leaf.NamedType);
                 return true;
             }
             catch (InvalidFormatException)

src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointDescriptor.cs

@@ -21,9 +21,10 @@ internal sealed class VariableValueInsertionTrie
 
 internal sealed record VariableValueInsertionTrieLeaf(
     string ParameterKey,
-    ITypeDefinition Type,
+    ITypeDefinition NamedType,
     OpenApiEndpointParameterType ParameterType,
-    bool HasDefaultValue) : IVariableValueInsertionTrieSegment;
+    bool HasDefaultValue,
+    bool IsNonNullType) : IVariableValueInsertionTrieSegment;
 
 internal enum OpenApiEndpointParameterType
 {

src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointFactory.cs

@@ -96,7 +96,7 @@ private static OpenApiEndpointDescriptor CreateEndpointDescriptor(
 
         var responseNameToExtract = rootField.Alias?.Value ?? rootField.Name.Value;
 
-        var route = CreateRoutePattern(endpointDefinition.Route);
+        var route = RoutePatternFactory.Parse(endpointDefinition.Route);
 
         var parameterTrie = new VariableValueInsertionTrie();
 
@@ -123,7 +123,7 @@ void InsertParametersIntoTrie(
         {
             foreach (var parameter in parameters)
             {
-                var (inputType, hasDefaultValue) = GetParameterDetails(
+                var (inputType, hasDefaultValue, isNonNullType) = GetParameterDetails(
                     parameter,
                     endpointDefinition.OperationDefinition,
                     schema);
@@ -132,7 +132,8 @@ void InsertParametersIntoTrie(
                     parameter.Key,
                     inputType,
                     parameterType,
-                    hasDefaultValue);
+                    hasDefaultValue,
+                    isNonNullType);
 
                 var inputObjectPath = parameter.InputObjectPath;
 
@@ -185,7 +186,7 @@ void InsertParametersIntoTrie(
         }
     }
 
-    private static (ITypeDefinition Type, bool HasDefaultValue) GetParameterDetails(
+    private static (ITypeDefinition Type, bool HasDefaultValue, bool IsNonNullType) GetParameterDetails(
         OpenApiEndpointDefinitionParameter parameter,
         OperationDefinitionNode operation,
         ISchemaDefinition schema)
@@ -195,6 +196,7 @@ private static (ITypeDefinition Type, bool HasDefaultValue) GetParameterDetails(
 
         var currentType = schema.Types[variable.Type.NamedType().Name.Value];
         var hasDefaultValue = variable.DefaultValue is not null;
+        var isNonNullType = variable.Type.IsNonNullType();
 
         if (parameter.InputObjectPath is { Length: > 0 })
         {
@@ -209,35 +211,10 @@ private static (ITypeDefinition Type, bool HasDefaultValue) GetParameterDetails(
 
                 currentType = field.Type.NamedType();
                 hasDefaultValue = field.DefaultValue is not null;
+                isNonNullType = field.Type.IsNonNullType();
             }
         }
 
-        return (currentType, hasDefaultValue);
-    }
-
-    private static RoutePattern CreateRoutePattern(string route)
-    {
-        return RoutePatternFactory.Parse(route);
-        // var segments = new List<RoutePatternPathSegment>();
-        //
-        // foreach (var segment in route.Segments)
-        // {
-        //     if (segment is OpenApiRouteSegmentLiteral stringSegment)
-        //     {
-        //         segments.Add(
-        //             RoutePatternFactory.Segment(
-        //                 RoutePatternFactory.LiteralPart(stringSegment.Value)));
-        //     }
-        //     else if (segment is OpenApiRouteSegmentParameter mapSegment)
-        //     {
-        //         // We do not apply route constraints here, as they are not meant for validation but to disambiguate routes:
-        //         // https://learn.microsoft.com/en-us/aspnet/core/fundamentals/routing#route-constraints
-        //         segments.Add(
-        //             RoutePatternFactory.Segment(
-        //                 RoutePatternFactory.ParameterPart(mapSegment.Key)));
-        //     }
-        // }
-        //
-        // return RoutePatternFactory.Pattern(segments);
+        return (currentType, hasDefaultValue, isNonNullType);
     }
 }

src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/Endpoints/HttpEndpointIntegrationTestBase.cs

@@ -84,6 +84,27 @@ public async Task Http_Get_With_Query_Parameter()
         response.MatchSnapshot();
     }
 
+    [Fact]
+    public async Task Http_Get_With_Missing_Required_Query_Parameter()
+    {
+        // arrange
+        var storage = new TestOpenApiDefinitionStorage(
+            """
+            query SearchProducts($text: String, $first: Int!)
+              @http(method: GET, route: "/products/search", queryParameters: ["text", "first"]) {
+              searchProductsPaginated(text: $text, first: $first)
+            }
+            """);
+        var server = CreateTestServer(storage);
+        var client = server.CreateClient();
+
+        // act
+        var response = await client.GetAsync("/products/search?text=Chair");
+
+        // assert
+        response.MatchSnapshot();
+    }
+
     [Fact]
     public async Task Http_Get_Without_Query_Parameter_That_Has_Default_Value()
     {

src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/Endpoints/__snapshots__/HttpEndpointIntegrationTestBase.Http_Get_With_Missing_Required_Query_Parameter.snap

@@ -0,0 +1,6 @@
+Headers:
+Content-Type: application/problem+json
+-------------------------->
+Status Code: BadRequest
+-------------------------->
+{"type":"https://tools.ietf.org/html/rfc9110#section-15.5.1","title":"Bad Request","status":400,"detail":"Required query parameter 'first' is missing"}

src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/TestSchema.cs

@@ -94,6 +94,9 @@ public string SearchProducts(string? text, float? minPrice)
             var formattedMinPrice = minPrice?.ToString(CultureInfo.InvariantCulture);
             return $"Searched for: {text ?? "all"}, minPrice: {formattedMinPrice}";
         }
+
+        public string SearchProductsPaginated(string? text, int first)
+            => $"Searched for: {text ?? "all"}, first: {first}";
     }
 
     public class Mutation