v5: New method to get variables values (graphql-dotnet#3426)

sungam3r · web-flow · commit ec9cce9ef83e · 2022-11-28T18:09:15.000+03:00
New method to get variables values
diff --git a/.github/workflows/build-artifacts-code.yml b/.github/workflows/build-artifacts-code.yml
@@ -7,6 +7,7 @@ on:
     branches:
       - master
       - develop
+      - v5
     paths:
       - src/**
       - .github/workflows/**
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -19,7 +19,7 @@ jobs:
     - name: Setup .NET Core SDK
       uses: actions/setup-dotnet@v2
       with:
-         dotnet-version: '6.0.102'
+         dotnet-version: '6.0.x'
 
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v2
diff --git a/.github/workflows/test-code.yml b/.github/workflows/test-code.yml
@@ -39,7 +39,7 @@ jobs:
           dotnet-version: |
             3.1.x
             5.0.x
-            6.0.303
+            6.0.x
           source-url: https://nuget.pkg.github.com/graphql-dotnet/index.json
         env:
           NUGET_AUTH_TOKEN: ${{secrets.GITHUB_TOKEN}}
diff --git a/.github/workflows/test-documentation.yml b/.github/workflows/test-documentation.yml
@@ -5,6 +5,7 @@ on:
     branches:
       - master
       - develop
+      - v5
     paths:
       - docs2/**
       - package.json
diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -1,7 +1,7 @@
 <Project>
 
   <PropertyGroup>
-    <VersionPrefix>5.0.0-preview</VersionPrefix>
+    <VersionPrefix>5.4.0-preview</VersionPrefix>
     <NextVersion>6.0.0</NextVersion>
     <LangVersion>latest</LangVersion>
     <PackageLicenseExpression>MIT</PackageLicenseExpression>
diff --git a/src/GraphQL.ApiTests/net6/GraphQL.approved.txt b/src/GraphQL.ApiTests/net6/GraphQL.approved.txt
@@ -1151,7 +1151,6 @@ namespace GraphQL.Execution
     public class ExecutionStrategyRegistration : System.IEquatable<GraphQL.Execution.ExecutionStrategyRegistration>
     {
         public ExecutionStrategyRegistration(GraphQL.Execution.IExecutionStrategy Strategy, GraphQLParser.AST.OperationType Operation) { }
-        protected virtual System.Type EqualityContract { get; }
         public GraphQLParser.AST.OperationType Operation { get; set; }
         public GraphQL.Execution.IExecutionStrategy Strategy { get; set; }
     }
@@ -2915,9 +2914,15 @@ namespace GraphQL.Validation
         public System.Collections.Generic.List<GraphQL.Validation.VariableUsage>? GetRecursiveVariables(GraphQLParser.AST.GraphQLOperationDefinition operation) { }
         public System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedFragments(GraphQLParser.AST.GraphQLOperationDefinition operation) { }
         public System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedFragments(System.Collections.Generic.List<GraphQLParser.AST.GraphQLOperationDefinition> operations) { }
+        [System.Obsolete("This method can repeatedly add the same error into ValidationContext.Errors colle" +
+            "ction when called multiple times. Use GetVariablesValues method instead.")]
         public GraphQL.Validation.Variables GetVariableValues(GraphQL.Validation.IVariableVisitor? visitor = null) { }
         public System.Collections.Generic.List<GraphQL.Validation.VariableUsage>? GetVariables<TNode>(TNode node)
             where TNode : GraphQLParser.AST.ASTNode, GraphQLParser.AST.IHasSelectionSetNode { }
+        [return: System.Runtime.CompilerServices.TupleElementNames(new string?[]?[] {
+                "Variables",
+                "Errors"})]
+        public System.ValueTuple<GraphQL.Validation.Variables, System.Collections.Generic.List<GraphQL.Validation.ValidationError>?> GetVariablesValues(GraphQL.Validation.IVariableVisitor? visitor = null) { }
         public string? IsValidLiteralValue(GraphQL.Types.IGraphType type, GraphQLParser.AST.GraphQLValue valueAst) { }
         public void ReportError(GraphQL.Validation.ValidationError error) { }
     }
diff --git a/src/GraphQL.ApiTests/netstandard20+netstandard21/GraphQL.approved.txt b/src/GraphQL.ApiTests/netstandard20+netstandard21/GraphQL.approved.txt
@@ -1151,7 +1151,6 @@ namespace GraphQL.Execution
     public class ExecutionStrategyRegistration : System.IEquatable<GraphQL.Execution.ExecutionStrategyRegistration>
     {
         public ExecutionStrategyRegistration(GraphQL.Execution.IExecutionStrategy Strategy, GraphQLParser.AST.OperationType Operation) { }
-        protected virtual System.Type EqualityContract { get; }
         public GraphQLParser.AST.OperationType Operation { get; set; }
         public GraphQL.Execution.IExecutionStrategy Strategy { get; set; }
     }
@@ -2901,9 +2900,15 @@ namespace GraphQL.Validation
         public System.Collections.Generic.List<GraphQL.Validation.VariableUsage>? GetRecursiveVariables(GraphQLParser.AST.GraphQLOperationDefinition operation) { }
         public System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedFragments(GraphQLParser.AST.GraphQLOperationDefinition operation) { }
         public System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedFragments(System.Collections.Generic.List<GraphQLParser.AST.GraphQLOperationDefinition> operations) { }
+        [System.Obsolete("This method can repeatedly add the same error into ValidationContext.Errors colle" +
+            "ction when called multiple times. Use GetVariablesValues method instead.")]
         public GraphQL.Validation.Variables GetVariableValues(GraphQL.Validation.IVariableVisitor? visitor = null) { }
         public System.Collections.Generic.List<GraphQL.Validation.VariableUsage>? GetVariables<TNode>(TNode node)
             where TNode : GraphQLParser.AST.ASTNode, GraphQLParser.AST.IHasSelectionSetNode { }
+        [return: System.Runtime.CompilerServices.TupleElementNames(new string?[]?[] {
+                "Variables",
+                "Errors"})]
+        public System.ValueTuple<GraphQL.Validation.Variables, System.Collections.Generic.List<GraphQL.Validation.ValidationError>?> GetVariablesValues(GraphQL.Validation.IVariableVisitor? visitor = null) { }
         public string? IsValidLiteralValue(GraphQL.Types.IGraphType type, GraphQLParser.AST.GraphQLValue valueAst) { }
         public void ReportError(GraphQL.Validation.ValidationError error) { }
     }
diff --git a/src/GraphQL.Benchmarks/Benchmarks/DetailedBenchmark.cs b/src/GraphQL.Benchmarks/Benchmarks/DetailedBenchmark.cs
@@ -223,7 +223,7 @@ public Validation.Variables ParseVariables()
                 Schema = Schema,
                 Variables = Inputs ?? Inputs.Empty,
                 Operation = Operation,
-            }.GetVariableValues();
+            }.GetVariablesValues().Variables;
         }
 
         private static readonly DocumentValidator _documentValidator = new DocumentValidator();
diff --git a/src/GraphQL/Validation/DocumentValidator.cs b/src/GraphQL/Validation/DocumentValidator.cs
@@ -117,7 +117,13 @@ public class DocumentValidator : IDocumentValidator
                 }
 
                 // can report errors even without rules enabled
-                variables = context.GetVariableValues(variableVisitors == null ? null : variableVisitors.Count == 1 ? variableVisitors[0] : new CompositeVariableVisitor(variableVisitors));
+                (variables, var errors) = context.GetVariablesValues(variableVisitors == null ? null : variableVisitors.Count == 1 ? variableVisitors[0] : new CompositeVariableVisitor(variableVisitors));
+
+                if (errors != null)
+                {
+                    foreach (var error in errors)
+                        context.ReportError(error);
+                }
 
                 return context.HasErrors
                     ? (new ValidationResult(context.Errors), variables)
diff --git a/src/GraphQL/Validation/ValidationContext.cs b/src/GraphQL/Validation/ValidationContext.cs
@@ -118,16 +118,39 @@ public List<GraphQLFragmentSpread> GetFragmentSpreads(GraphQLSelectionSet node)
 
         /// <summary>
         /// Returns all of the variable values defined for the operation from the attached <see cref="Variables"/> object.
+        /// Only correctly validated variables are returned. If the variable is specified incorrectly, then an instance of
+        /// <see cref="ValidationError"/> is added to the <see cref="Errors"/>.
         /// </summary>
+        [Obsolete("This method can repeatedly add the same error into ValidationContext.Errors collection when called multiple times. Use GetVariablesValues method instead.")]
         public Variables GetVariableValues(IVariableVisitor? visitor = null)
+        {
+            (var variables, var errors) = GetVariablesValues(visitor);
+
+            if (errors != null)
+            {
+                foreach (var error in errors)
+                    ReportError(error);
+            }
+
+            return variables;
+        }
+
+
+        /// <summary>
+        /// Returns all of the variable values defined for the operation from the attached <see cref="Variables"/> object.
+        /// Only correctly validated variables are returned. If the variable is specified incorrectly, then an instance of
+        /// <see cref="ValidationError"/> is returned within the list of errors.
+        /// </summary>
+        public (Variables Variables, List<ValidationError>? Errors) GetVariablesValues(IVariableVisitor? visitor = null)
         {
             var variableDefinitions = Operation?.Variables;
 
             if ((variableDefinitions?.Count ?? 0) == 0)
             {
-                return Validation.Variables.None;
+                return (Validation.Variables.None, null);
             }
 
+            List<ValidationError>? errors = null;
             var variablesObj = new Variables(variableDefinitions!.Count);
 
             if (variableDefinitions != null)
@@ -140,7 +163,7 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)
 
                     if (graphType == null)
                     {
-                        ReportError(new InvalidVariableError(this, variableDef, variableDefName, $"Variable has unknown type '{variableDef.Type.Name()}'"));
+                        (errors ??= new()).Add(new InvalidVariableError(this, variableDef, variableDefName, $"Variable has unknown type '{variableDef.Type.Name()}'"));
                         continue;
                     }
 
@@ -157,7 +180,7 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)
                         }
                         catch (ValidationError error)
                         {
-                            ReportError(error);
+                            (errors ??= new()).Add(error);
                             continue;
                         }
                     }
@@ -172,14 +195,14 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)
                         }
                         catch (Exception ex)
                         {
-                            ReportError(new InvalidVariableError(this, variableDef, variableDefName, "Error coercing default value.", ex));
+                            (errors ??= new()).Add(new InvalidVariableError(this, variableDef, variableDefName, "Error coercing default value.", ex));
                             continue;
                         }
                         variable.IsDefault = true;
                     }
                     else if (graphType is NonNullGraphType)
                     {
-                        ReportError(new InvalidVariableError(this, variableDef, variable.Name, "No value provided for a non-null variable."));
+                        (errors ??= new()).Add(new InvalidVariableError(this, variableDef, variable.Name, "No value provided for a non-null variable."));
                         continue;
                     }
 
@@ -191,7 +214,7 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)
             }
 
             // return the list of parsed variables defined for the operation
-            return variablesObj;
+            return (variablesObj, errors);
         }
 
         /// <summary>

Original file line number	Diff line number	Diff line change
`@@ -223,7 +223,7 @@ public Validation.Variables ParseVariables()`
`223`	`223`	`Schema = Schema,`
`224`	`224`	`Variables = Inputs ?? Inputs.Empty,`
`225`	`225`	`Operation = Operation,`
`226`		`- }.GetVariableValues();`
	`226`	`+ }.GetVariablesValues().Variables;`
`227`	`227`	`}`
`228`	`228`
`229`	`229`	`private static readonly DocumentValidator _documentValidator = new DocumentValidator();`
Original file line number	Diff line number	Diff line change
`@@ -118,16 +118,39 @@ public List<GraphQLFragmentSpread> GetFragmentSpreads(GraphQLSelectionSet node)`
`118`	`118`
`119`	`119`	`/// <summary>`
`120`	`120`	`/// Returns all of the variable values defined for the operation from the attached <see cref="Variables"/> object.`
	`121`	`+ /// Only correctly validated variables are returned. If the variable is specified incorrectly, then an instance of`
	`122`	`+ /// <see cref="ValidationError"/> is added to the <see cref="Errors"/>.`
`121`	`123`	`/// </summary>`
	`124`	`+ [Obsolete("This method can repeatedly add the same error into ValidationContext.Errors collection when called multiple times. Use GetVariablesValues method instead.")]`
`122`	`125`	`public Variables GetVariableValues(IVariableVisitor? visitor = null)`
	`126`	`+ {`
	`127`	`+ (var variables, var errors) = GetVariablesValues(visitor);`
	`128`	`+`
	`129`	`+ if (errors != null)`
	`130`	`+ {`
	`131`	`+ foreach (var error in errors)`
	`132`	`+ ReportError(error);`
	`133`	`+ }`
	`134`	`+`
	`135`	`+ return variables;`
	`136`	`+ }`
	`137`	`+`
	`138`	`+`
	`139`	`+ /// <summary>`
	`140`	`+ /// Returns all of the variable values defined for the operation from the attached <see cref="Variables"/> object.`
	`141`	`+ /// Only correctly validated variables are returned. If the variable is specified incorrectly, then an instance of`
	`142`	`+ /// <see cref="ValidationError"/> is returned within the list of errors.`
	`143`	`+ /// </summary>`
	`144`	`+ public (Variables Variables, List<ValidationError>? Errors) GetVariablesValues(IVariableVisitor? visitor = null)`
`123`	`145`	`{`
`124`	`146`	`var variableDefinitions = Operation?.Variables;`
`125`	`147`
`126`	`148`	`if ((variableDefinitions?.Count ?? 0) == 0)`
`127`	`149`	`{`
`128`		`- return Validation.Variables.None;`
	`150`	`+ return (Validation.Variables.None, null);`
`129`	`151`	`}`
`130`	`152`
	`153`	`+ List<ValidationError>? errors = null;`
`131`	`154`	`var variablesObj = new Variables(variableDefinitions!.Count);`
`132`	`155`
`133`	`156`	`if (variableDefinitions != null)`
`@@ -140,7 +163,7 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)`
`140`	`163`
`141`	`164`	`if (graphType == null)`
`142`	`165`	`{`
`143`		`- ReportError(new InvalidVariableError(this, variableDef, variableDefName, $"Variable has unknown type '{variableDef.Type.Name()}'"));`
	`166`	`+ (errors ??= new()).Add(new InvalidVariableError(this, variableDef, variableDefName, $"Variable has unknown type '{variableDef.Type.Name()}'"));`
`144`	`167`	`continue;`
`145`	`168`	`}`
`146`	`169`
`@@ -157,7 +180,7 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)`
`157`	`180`	`}`
`158`	`181`	`catch (ValidationError error)`
`159`	`182`	`{`
`160`		`- ReportError(error);`
	`183`	`+ (errors ??= new()).Add(error);`
`161`	`184`	`continue;`
`162`	`185`	`}`
`163`	`186`	`}`
`@@ -172,14 +195,14 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)`
`172`	`195`	`}`
`173`	`196`	`catch (Exception ex)`
`174`	`197`	`{`
`175`		`- ReportError(new InvalidVariableError(this, variableDef, variableDefName, "Error coercing default value.", ex));`
	`198`	`+ (errors ??= new()).Add(new InvalidVariableError(this, variableDef, variableDefName, "Error coercing default value.", ex));`
`176`	`199`	`continue;`
`177`	`200`	`}`
`178`	`201`	`variable.IsDefault = true;`
`179`	`202`	`}`
`180`	`203`	`else if (graphType is NonNullGraphType)`
`181`	`204`	`{`
`182`		`- ReportError(new InvalidVariableError(this, variableDef, variable.Name, "No value provided for a non-null variable."));`
	`205`	`+ (errors ??= new()).Add(new InvalidVariableError(this, variableDef, variable.Name, "No value provided for a non-null variable."));`
`183`	`206`	`continue;`
`184`	`207`	`}`
`185`	`208`
`@@ -191,7 +214,7 @@ public Variables GetVariableValues(IVariableVisitor? visitor = null)`
`191`	`214`	`}`
`192`	`215`
`193`	`216`	`// return the list of parsed variables defined for the operation`
`194`		`- return variablesObj;`
	`217`	`+ return (variablesObj, errors);`
`195`	`218`	`}`
`196`	`219`
`197`	`220`	`/// <summary>`