Fixed issue with non-null arguments that have defaults (#2441)

src/HotChocolate/Core/src/Execution/Extensions/ExecutionRequestExecutorExtensions.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using static HotChocolate.Execution.Properties.Resources;
 
 namespace HotChocolate.Execution
 {
@@ -38,7 +39,7 @@ public static class ExecutionRequestExecutorExtensions
             if (string.IsNullOrEmpty(query))
             {
                 throw new ArgumentException(
-                    "The query mustn't be null or empty.",
+                    ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty,
                     nameof(query));
             }
 
@@ -60,7 +61,7 @@ public static class ExecutionRequestExecutorExtensions
             if (string.IsNullOrEmpty(query))
             {
                 throw new ArgumentException(
-                    "The query mustn't be null or empty.",
+                    ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty,
                     nameof(query));
             }
 
@@ -82,7 +83,7 @@ public static class ExecutionRequestExecutorExtensions
             if (string.IsNullOrEmpty(query))
             {
                 throw new ArgumentException(
-                    "The query mustn't be null or empty.",
+                    ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty,
                     nameof(query));
             }
 
@@ -113,7 +114,7 @@ public static class ExecutionRequestExecutorExtensions
             if (string.IsNullOrEmpty(query))
             {
                 throw new ArgumentException(
-                    "The query mustn't be null or empty.",
+                    ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty,
                     nameof(query));
             }
 
@@ -163,7 +164,7 @@ public static class ExecutionRequestExecutorExtensions
             if (string.IsNullOrEmpty(query))
             {
                 throw new ArgumentException(
-                    "The query mustn't be null or empty.",
+                    ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty,
                     nameof(query));
             }
 
@@ -186,7 +187,7 @@ public static class ExecutionRequestExecutorExtensions
             if (string.IsNullOrEmpty(query))
             {
                 throw new ArgumentException(
-                    "The query mustn't be null or empty.",
+                    ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty,
                     nameof(query));
             }
 

src/HotChocolate/Core/src/Execution/Processing/ArgumentNonNullValidator.cs

@@ -8,15 +8,36 @@ internal static class ArgumentNonNullValidator
     {
         public static ValidationResult Validate(IInputField field, IValueNode? value, Path path)
         {
-            if (value.IsNull())
+            // if no value was provided
+            if (value is null)
             {
+                // the type is a non-null type and no default value has been set we mark this
+                // field as violation.
                 if (field.Type.IsNonNullType() && field.DefaultValue.IsNull())
                 {
                     return new ValidationResult(field.Type, path);
                 }
+
+                // if the field has a default value or nullable everything is fine and we
+                // return success.
                 return default;
             }
 
+            // if null was explicitly set
+            if (value is NullValueNode)
+            {
+                // and the field type is a non-null type we will mark the field value
+                // as violation.
+                if (field.Type.IsNonNullType())
+                {
+                    return new ValidationResult(field.Type, path);
+                }
+
+                // if the field is nullable we will mark the field as valid.
+                return default;
+            }
+
+            // if the field has a value we traverse it and make sure the value is correct.
             return ValidateInnerType(field.Type, value, path);
         }
 
@@ -40,10 +61,8 @@ private static ValidationResult ValidateInnerType(IType type, IValueNode? value,
                 {
                     return ValidateList(listType, listValue, path);
                 }
-                else
-                {
-                    Validate(listType.ElementType, value, path);
-                }
+
+                Validate(listType.ElementType, value, path);
             }
 
             if (innerType is InputObjectType inputType && value is ObjectValueNode ov)
@@ -61,7 +80,7 @@ private static ValidationResult ValidateInnerType(IType type, IValueNode? value,
         {
             var fields = new Dictionary<NameString, IValueNode>();
 
-            for (int i = 0; i < value.Fields.Count; i++)
+            for (var i = 0; i < value.Fields.Count; i++)
             {
                 ObjectFieldNode field = value.Fields[i];
                 fields[field.Name.Value] = field.Value;
@@ -88,7 +107,7 @@ private static ValidationResult ValidateInnerType(IType type, IValueNode? value,
         private static ValidationResult ValidateList(ListType type, ListValueNode list, Path path)
         {
             IType elementType = type.ElementType();
-            int i = 0;
+            var i = 0;
 
             foreach (IValueNode element in list.Items)
             {

src/HotChocolate/Core/src/Execution/Properties/Resources.resx

@@ -237,4 +237,7 @@
   <data name="OperationCompiler_Compile_SelectionSetIsEmpty" xml:space="preserve">
     <value>The operation selection set is empty.</value>
   </data>
+  <data name="ExecutionRequestExecutorExtensions_ExecuteAsync_QueryCannotBeNullOrEmpty" xml:space="preserve">
+    <value>The query cannot be null or empty.</value>
+  </data>
 </root>

src/HotChocolate/Core/src/Execution/Serialization/JsonQueryResultSerializer.cs

@@ -194,6 +194,13 @@ private static void WritePath(Utf8JsonWriter writer, Path? path)
 
         private static void WritePathValue(Utf8JsonWriter writer, Path path)
         {
+            if (path is RootPathSegment)
+            {
+                writer.WriteStartArray();
+                writer.WriteEndArray();
+                return;
+            }
+
             writer.WriteStartArray();
 
             IReadOnlyList<object> list = path.ToList();
@@ -400,7 +407,7 @@ private static void WritePathValue(Utf8JsonWriter writer, Path path)
                     break;
 
                 case Path p:
-                    WritePath(writer, p);
+                    WritePathValue(writer, p);
                     break;
 
                 default:

src/HotChocolate/Core/test/Execution.Tests/ArgumentNonNullValidatorTests.cs

@@ -22,15 +22,16 @@ public void Validate_Input_With_Non_Null_Props_That_Have_No_Value_But_A_DefaultV
                         a: String! = ""bar""
                     }
                 ")
-                .Use(next => context => default(ValueTask))
+                .Use(next => context => default)
                 .Create();
 
             IInputField field = schema.QueryType.Fields["test"].Arguments["bar"];
 
             // act
-            var report = ArgumentNonNullValidator.Validate(
-                field,
-                new ObjectValueNode(), Path.New("root"));
+            ArgumentNonNullValidator.ValidationResult report =
+                ArgumentNonNullValidator.Validate(
+                    field,
+                    new ObjectValueNode(), Path.New("root"));
 
             // assert
             Assert.False(report.HasErrors);

src/HotChocolate/Core/test/Execution.Tests/Integration/Spec/ArgumentCoercionTests.cs

@@ -0,0 +1,112 @@
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
+using Snapshooter.Xunit;
+using Xunit;
+
+#nullable enable
+
+namespace HotChocolate.Execution.Integration.Spec
+{
+    public class ArgumentCoercionTests
+    {
+        [Fact]
+        public async Task Pass_In_Null_To_NonNullArgument_With_DefaultValue()
+        {
+            // arrange
+            IRequestExecutor executor =
+                await new ServiceCollection()
+                    .AddGraphQL()
+                    .AddQueryType<Query>()
+                    .BuildRequestExecutorAsync();
+
+            // act
+            IExecutionResult result = await executor.ExecuteAsync("{ sayHello(name: null) }");
+
+            // assert
+            result.ToJson().MatchSnapshot();
+        }
+
+        [Fact]
+        public async Task Pass_In_Nothing_To_NonNullArgument_With_DefaultValue()
+        {
+            // arrange
+            IRequestExecutor executor =
+                await new ServiceCollection()
+                    .AddGraphQL()
+                    .AddQueryType<Query>()
+                    .BuildRequestExecutorAsync();
+
+            // act
+            IExecutionResult result = await executor.ExecuteAsync("{ sayHello }");
+
+            // assert
+            result.ToJson().MatchSnapshot();
+        }
+
+        [Fact]
+        public async Task Pass_In_Nothing_To_NonNullArgument_With_DefaultValue_By_Variable()
+        {
+            // arrange
+            IRequestExecutor executor =
+                await new ServiceCollection()
+                    .AddGraphQL()
+                    .AddQueryType<Query>()
+                    .BuildRequestExecutorAsync();
+
+            // act
+            IExecutionResult result = await executor.ExecuteAsync(
+                "query ($a: String!) { sayHello(name: $a) }");
+
+            // assert
+            result.ToJson().MatchSnapshot();
+        }
+
+        [Fact]
+        public async Task Pass_In_Null_To_NonNullArgument_With_DefaultValue_By_Variable()
+        {
+            // arrange
+            IRequestExecutor executor =
+                await new ServiceCollection()
+                    .AddGraphQL()
+                    .AddQueryType<Query>()
+                    .BuildRequestExecutorAsync();
+
+            var variables = new Dictionary<string, object?> { { "a", null } };
+
+            // act
+            IExecutionResult result = await executor.ExecuteAsync(
+                "query ($a: String!) { sayHello(name: $a) }",
+                variables);
+
+            // assert
+            result.ToJson().MatchSnapshot();
+        }
+
+        [Fact]
+        public async Task Pass_In_Sydney_To_NonNullArgument_With_DefaultValue_By_Variable()
+        {
+            // arrange
+            IRequestExecutor executor =
+                await new ServiceCollection()
+                    .AddGraphQL()
+                    .AddQueryType<Query>()
+                    .BuildRequestExecutorAsync();
+
+            var variables = new Dictionary<string, object?> { { "a", "Sydney" } };
+
+            // act
+            IExecutionResult result = await executor.ExecuteAsync(
+                "query ($a: String!) { sayHello(name: $a) }",
+                variables);
+
+            // assert
+            result.ToJson().MatchSnapshot();
+        }
+
+        public class Query
+        {
+            public string SayHello(string name = "Michael") => $"Hello {name}.";
+        }
+    }
+}

src/HotChocolate/Core/test/Execution.Tests/Integration/Spec/__snapshots__/ArgumentCoercionTests.Pass_In_Nothing_To_NonNullArgument_With_DefaultValue.snap

@@ -0,0 +1,5 @@
+{
+  "data": {
+    "sayHello": "Hello Michael."
+  }
+}

src/HotChocolate/Core/test/Execution.Tests/Integration/Spec/__snapshots__/ArgumentCoercionTests.Pass_In_Nothing_To_NonNullArgument_With_DefaultValue_By_Variable.snap

@@ -0,0 +1,17 @@
+{
+  "errors": [
+    {
+      "message": "Variable \u0060a\u0060 is required.",
+      "locations": [
+        {
+          "line": 1,
+          "column": 8
+        }
+      ],
+      "extensions": {
+        "code": "EXEC_NON_NULL_VIOLATION",
+        "variable": "a"
+      }
+    }
+  ]
+}

src/HotChocolate/Core/test/Execution.Tests/Integration/Spec/__snapshots__/ArgumentCoercionTests.Pass_In_Null_To_NonNullArgument_With_DefaultValue.snap

@@ -0,0 +1,19 @@
+{
+  "errors": [
+    {
+      "message": "Detected a non-null violation in argument \u0060name\u0060.",
+      "locations": [
+        {
+          "line": 1,
+          "column": 12
+        }
+      ],
+      "extensions": {
+        "responseName": "sayHello",
+        "errorPath": [
+          "name"
+        ]
+      }
+    }
+  ]
+}

src/HotChocolate/Core/test/Execution.Tests/Integration/Spec/__snapshots__/ArgumentCoercionTests.Pass_In_Null_To_NonNullArgument_With_DefaultValue_By_Variable.snap

@@ -0,0 +1,17 @@
+{
+  "errors": [
+    {
+      "message": "Variable \u0060a\u0060 is required.",
+      "locations": [
+        {
+          "line": 1,
+          "column": 8
+        }
+      ],
+      "extensions": {
+        "code": "EXEC_NON_NULL_VIOLATION",
+        "variable": "a"
+      }
+    }
+  ]
+}

src/HotChocolate/Core/test/Execution.Tests/Integration/Spec/__snapshots__/ArgumentCoercionTests.Pass_In_Sydney_To_NonNullArgument_With_DefaultValue_By_Variable.snap

@@ -0,0 +1,5 @@
+{
+  "data": {
+    "sayHello": "Hello Sydney."
+  }
+}