Started reworking for better perf

VarunSaiTeja · Mar 7, 2021 · 3d7d6e3 · 3d7d6e3
1 parent 37b22c8
commit 3d7d6e3
Show file tree

Hide file tree

Showing 11 changed files with 203 additions and 51 deletions.
diff --git a/.vscode/tasks.json b/.vscode/tasks.json
@@ -0,0 +1,24 @@
+{
+    // See https://go.microsoft.com/fwlink/?LinkId=733558
+    // for the documentation about the tasks.json format
+    "version": "2.0.0",
+    "tasks": [
+        {
+            "label": "build",
+            "command": "dotnet",
+            "type": "shell",
+            "args": [
+                "build",
+                // Ask dotnet build to generate full paths for file names.
+                "/property:GenerateFullPaths=true",
+                // Do not generate summary otherwise it leads to duplicate errors in Problems panel
+                "/consoleloggerparameters:NoSummary"
+            ],
+            "group": "build",
+            "presentation": {
+                "reveal": "silent"
+            },
+            "problemMatcher": "$msCompile"
+        }
+    ]
+}
diff --git a/Graph.ArgumentValidator.sln b/Graph.ArgumentValidator.sln
@@ -0,0 +1,48 @@
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio 15
+VisualStudioVersion = 15.0.26124.0
+MinimumVisualStudioVersion = 15.0.26124.0
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Graph.ArgumentValidator", "src\Graph.ArgumentValidator.csproj", "{59435C05-1155-47A5-9361-2702B320EB83}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Graph.ArgumentValidator.Tests", "test\Graph.ArgumentValidator.Tests.csproj", "{F31CE4C8-0B88-492B-9AE2-92D731D51B88}"
+EndProject
+Global
+	GlobalSection(SolutionConfigurationPlatforms) = preSolution
+		Debug|Any CPU = Debug|Any CPU
+		Debug|x64 = Debug|x64
+		Debug|x86 = Debug|x86
+		Release|Any CPU = Release|Any CPU
+		Release|x64 = Release|x64
+		Release|x86 = Release|x86
+	EndGlobalSection
+	GlobalSection(SolutionProperties) = preSolution
+		HideSolutionNode = FALSE
+	EndGlobalSection
+	GlobalSection(ProjectConfigurationPlatforms) = postSolution
+		{59435C05-1155-47A5-9361-2702B320EB83}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Debug|x64.Build.0 = Debug|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Debug|x86.Build.0 = Debug|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Release|Any CPU.Build.0 = Release|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Release|x64.ActiveCfg = Release|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Release|x64.Build.0 = Release|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Release|x86.ActiveCfg = Release|Any CPU
+		{59435C05-1155-47A5-9361-2702B320EB83}.Release|x86.Build.0 = Release|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Debug|x64.Build.0 = Debug|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Debug|x86.Build.0 = Debug|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Release|Any CPU.Build.0 = Release|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Release|x64.ActiveCfg = Release|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Release|x64.Build.0 = Release|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Release|x86.ActiveCfg = Release|Any CPU
+		{F31CE4C8-0B88-492B-9AE2-92D731D51B88}.Release|x86.Build.0 = Release|Any CPU
+	EndGlobalSection
+EndGlobal
diff --git a/src/Graph.ArgumentValidator.csproj b/src/Graph.ArgumentValidator.csproj
@@ -1,4 +1,5 @@
-<Project Sdk="Microsoft.NET.Sdk">
+
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFrameworks>$(LibraryTargetFrameworks)</TargetFrameworks>

diff --git a/src/Graph.ArgumentValidator.sln b/src/Graph.ArgumentValidator.sln
diff --git a/src/Extensions.cs → src/RequestExecutorBuilderExtensions.cs b/src/Extensions.cs → src/RequestExecutorBuilderExtensions.cs
@@ -3,12 +3,12 @@
 
 namespace Graph.ArgumentValidator
 {
-    public static class Extensions
+    public static class RequestExecutorBuilderExtensions
     {
         public static IRequestExecutorBuilder AddArgumentValidator(
             this IRequestExecutorBuilder requestExecutorBuilder)
         {
-            requestExecutorBuilder.UseField<ValidationMiddleware>();
+            requestExecutorBuilder.TryAddTypeInterceptor<ValidationTypeInterceptor>();
 
             return requestExecutorBuilder;
         }

diff --git a/src/ValidatableAttribute.cs b/src/ValidatableAttribute.cs
@@ -2,7 +2,7 @@
 
 namespace Graph.ArgumentValidator
 {
-    [AttributeUsage(AttributeTargets.Class, Inherited = true, AllowMultiple = false)]
+    [AttributeUsage(AttributeTargets.Parameter, Inherited = true, AllowMultiple = false)]
     public class ValidatableAttribute : Attribute
     {
         public ValidatableAttribute()

diff --git a/src/ValidationMiddleware.cs b/src/ValidationMiddleware.cs
@@ -17,43 +17,27 @@ public ValidationMiddleware(FieldDelegate next)
             _next = next;
         }
 
+        // this middleware is ensured to only execute on fields that have arguments that need validation.
         public async Task InvokeAsync(IMiddlewareContext context)
         {
-            var arguments = context.Field.Arguments;
-
-            if (arguments.Count is 0)
-                await _next(context);
-
             var errors = new List<ValidationResult>();
 
-            foreach (var argument in arguments)
+            // we could even further optimize and aggregate this list in the interceptor and inject it into the middleware
+            foreach (var argument in context.Field.Arguments.Where(t => t.ContextData.ContainsKey(WellKnownContextData.NeedValidation)))
             {
-                if (argument == null)
-                {
-                    continue;
-                }
-
                 var input = context.ArgumentValue<object>(argument.Name);
-
-                if (input == null)
-                {
-                    continue;
-                }
-
-                if (!ValidatorSettings.ValidateAllInputs &&
-                    input.GetType().GetCustomAttribute(typeof(ValidatableAttribute)) == null)
-                    continue;
-
                 var validationContext = new ValidationContext(input);
                 Validator.TryValidateObject(input, validationContext, errors, true);
             }
 
-            if (errors.Any())
+            if (errors.Count > 0)
             {
                 foreach (var error in errors)
                 {
+                    // NOTE: it would be good to provide an error code.
                     context.ReportError(ErrorBuilder.New()
                         .SetMessage(error.ErrorMessage)
+                        // .SetCode()
                         .Build());
                 }
             }

diff --git a/src/ValidationTypeInterceptor.cs b/src/ValidationTypeInterceptor.cs
@@ -0,0 +1,57 @@
+using System.Collections.Generic;
+using HotChocolate.Configuration;
+using HotChocolate.Resolvers;
+using HotChocolate.Types.Descriptors.Definitions;
+
+namespace Graph.ArgumentValidator
+{
+    internal class ValidationTypeInterceptor : TypeInterceptor
+    {
+        private FieldMiddleware _middleware;
+
+        public override void OnBeforeCompleteType(
+            ITypeCompletionContext completionContext,
+            DefinitionBase definition,
+            IDictionary<string, object> contextData)
+        {
+            if (definition is ObjectTypeDefinition objectTypeDef)
+            {
+                foreach (var fieldDef in objectTypeDef.Fields)
+                {
+                    // most fields do not need validation.
+                    bool needValidation = false;
+
+                    foreach (var argumentDef in fieldDef.Arguments)
+                    {
+                        if (argumentDef.Parameter is not null &&
+                            argumentDef.Parameter.IsDefined(typeof(ValidatableAttribute), true))
+                        {
+                            // we will set a marker for this argument to be validated.
+                            argumentDef.ContextData[WellKnownContextData.NeedValidation] = true;
+                            needValidation = true;
+                        }
+                    }
+
+                    if (needValidation)
+                    {
+                        // if validation is needed we will ensure that a validation middleware exists.
+                        if (_middleware is null)
+                        {
+                            // if no middleware is yet created we will compile a middleware from our
+                            // ValidationMiddleware class.
+                            _middleware = FieldClassMiddlewareFactory.Create<ValidationMiddleware>();
+                        }
+
+                        // we add the validation middleware to the first spot so that validation is executed first.
+                        fieldDef.MiddlewareComponents.Insert(0, _middleware);
+                    }
+                }
+            }
+        }
+    }
+
+    internal static class WellKnownContextData
+    {
+        public const string NeedValidation = nameof(NeedValidation);
+    }
+}
diff --git a/test/Graph.ArgumentValidator.Tests.csproj b/test/Graph.ArgumentValidator.Tests.csproj
@@ -0,0 +1,27 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net5.0</TargetFramework>
+
+    <IsPackable>false</IsPackable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="../src/Graph.ArgumentValidator.csproj" />
+    <PackageReference Include="Snapshooter.Xunit" Version="0.6.1" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
+    <PackageReference Include="xunit" Version="2.4.1" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+    <PackageReference Include="coverlet.collector" Version="1.3.0">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+  </ItemGroup>
+
+</Project>
diff --git a/test/UnitTest1.cs b/test/UnitTest1.cs
@@ -0,0 +1,31 @@
+using System;
+using System.ComponentModel.DataAnnotations;
+using System.Threading.Tasks;
+using HotChocolate;
+using HotChocolate.Execution;
+using Microsoft.Extensions.DependencyInjection;
+using Snapshooter.Xunit;
+using Xunit;
+
+namespace Graph.ArgumentValidator
+{
+    public class ValidationTests
+    {
+        [Fact]
+        public async Task Ensure_Validation_Is_Executed()
+        {
+            var result =
+                await new ServiceCollection()
+                    .AddGraphQL()
+                    .AddQueryType<Query>()
+                    .AddArgumentValidator()
+                    .ExecuteRequestAsync("{ argIsEmail(email: \"abc\") }");
+
+            result.ToJson().MatchSnapshot();
+        }
+    }
+
+    public class Query {
+        public string ArgIsEmail([Validatable] [EmailAddress] string email) => email;
+    }
+}
diff --git a/test/__snapshots__/ValidationTests.Ensure_Validation_Is_Executed.snap b/test/__snapshots__/ValidationTests.Ensure_Validation_Is_Executed.snap
@@ -0,0 +1,5 @@
+{
+  "data": {
+    "argIsEmail": "abc"
+  }
+}