From 29fac13f036ebb923b46e191fb331731079bd542 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 15:22:20 +0200 Subject: [PATCH 01/30] Added lead field selection rule implementation --- .../Validation/LeafFieldSelectionsRule.cs | 25 ++++++++ .../Validation/LeafFieldSelectionsVisitor.cs | 63 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 src/Core/Validation/LeafFieldSelectionsRule.cs create mode 100644 src/Core/Validation/LeafFieldSelectionsVisitor.cs diff --git a/src/Core/Validation/LeafFieldSelectionsRule.cs b/src/Core/Validation/LeafFieldSelectionsRule.cs new file mode 100644 index 00000000000..1a55c35d8f7 --- /dev/null +++ b/src/Core/Validation/LeafFieldSelectionsRule.cs @@ -0,0 +1,25 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace HotChocolate.Validation +{ + /// + /// Field selections on scalars or enums are never allowed, + /// because they are the leaf nodes of any GraphQL query. + /// + /// Conversely the leaf field selections of GraphQL queries + /// must be of type scalar or enum. Leaf selections on objects, + /// interfaces, and unions without subfields are disallowed. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Leaf-Field-Selections + /// + internal sealed class LeafFieldSelectionsRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new LeafFieldSelectionsVisitor(schema); + } + } +} diff --git a/src/Core/Validation/LeafFieldSelectionsVisitor.cs b/src/Core/Validation/LeafFieldSelectionsVisitor.cs new file mode 100644 index 00000000000..2ca3d577c93 --- /dev/null +++ b/src/Core/Validation/LeafFieldSelectionsVisitor.cs @@ -0,0 +1,63 @@ +using System.Collections.Immutable; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class LeafFieldSelectionsVisitor + : QueryVisitorErrorBase + { + public LeafFieldSelectionsVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitField( + FieldNode field, + IType type, + ImmutableStack path) + { + if (type is IComplexOutputType t + && t.Fields.TryGetField(field.Name.Value, out IOutputField f)) + { + if (f.Type.IsScalarType() || f.Type.IsEnumType()) + { + ValidateLeafField(field, f); + } + else + { + ValidateNodeField(field, f); + base.VisitField(field, type, path); + } + } + } + + private void ValidateLeafField( + FieldNode fieldSelection, + IOutputField field) + { + if (fieldSelection.SelectionSet != null) + { + string type = field.Type.IsScalarType() ? "a scalar" : "an enum"; + Errors.Add(new ValidationError( + $"`{field.Name}` is {type} field. Selections on scalars " + + "or enums are never allowed, because they are the leaf " + + "nodes of any GraphQL query", fieldSelection)); + } + } + + private void ValidateNodeField( + FieldNode fieldSelection, + IOutputField field) + { + if (fieldSelection.SelectionSet == null) + { + Errors.Add(new ValidationError( + $"`{field.Name}` is a object, interface or union type " + + "field. Leaf selections on objects, interfaces, and " + + "unions without subfields are disallowed. ", + fieldSelection)); + } + } + } +} From 6a44a3a3a2c3a78e0f1ff837ef35e47bb8f15b5f Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 16:16:38 +0200 Subject: [PATCH 02/30] LeafFieldSelectionsRule Tests --- .../LeafFieldSelectionsRuleTests.cs | 130 ++++++++++++++++++ src/Core.Tests/Validation/Models/Query.cs | 15 ++ src/Core.Tests/Validation/Types/QueryType.cs | 3 + .../Validation/LeafFieldSelectionsVisitor.cs | 6 +- 4 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs diff --git a/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs b/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs new file mode 100644 index 00000000000..b4cf89409a8 --- /dev/null +++ b/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs @@ -0,0 +1,130 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class LeafFieldSelectionsRuleTests + : ValidationTestBase + { + public LeafFieldSelectionsRuleTests() + : base(new LeafFieldSelectionsRule()) + { + } + + [Fact] + public void ScalarSelection() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + barkVolume + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void ScalarSelectionsNotAllowedOnInt() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + barkVolume { + sinceWhen + } + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "`barkVolume` is a scalar field. Selections on scalars " + + "or enums are never allowed, because they are the leaf " + + "nodes of any GraphQL query.")); + } + + [Fact] + public void DirectQueryOnObjectWithoutSubFields() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query directQueryOnObjectWithoutSubFields { + human + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "`human` is an object, interface or union type " + + "field. Leaf selections on objects, interfaces, and " + + "unions without subfields are disallowed.")); + } + + [Fact] + public void DirectQueryOnInterfaceWithoutSubFields() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query directQueryOnInterfaceWithoutSubFields { + pet + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "`pet` is an object, interface or union type " + + "field. Leaf selections on objects, interfaces, and " + + "unions without subfields are disallowed.")); + } + + [Fact] + public void DirectQueryOnUnionWithoutSubFields() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query directQueryOnUnionWithoutSubFields { + catOrDog + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "`catOrDog` is an object, interface or union type " + + "field. Leaf selections on objects, interfaces, and " + + "unions without subfields are disallowed.")); + } + } +} diff --git a/src/Core.Tests/Validation/Models/Query.cs b/src/Core.Tests/Validation/Models/Query.cs index e6cc5760be6..c0141a9f3dd 100644 --- a/src/Core.Tests/Validation/Models/Query.cs +++ b/src/Core.Tests/Validation/Models/Query.cs @@ -16,5 +16,20 @@ public bool BooleanList(bool[] booleanListArg) { return true; } + + public Human GetHuman() + { + return null; + } + + public Human GetPet() + { + return null; + } + + public object GetCatOrDog() + { + return null; + } } } diff --git a/src/Core.Tests/Validation/Types/QueryType.cs b/src/Core.Tests/Validation/Types/QueryType.cs index 5abda2957ba..2d1230af79b 100644 --- a/src/Core.Tests/Validation/Types/QueryType.cs +++ b/src/Core.Tests/Validation/Types/QueryType.cs @@ -10,6 +10,9 @@ protected override void Configure(IObjectTypeDescriptor descriptor) descriptor.Field("arguments") .Type() .Resolver(() => null); + + descriptor.Field(t => t.GetCatOrDog()) + .Type(); } } } diff --git a/src/Core/Validation/LeafFieldSelectionsVisitor.cs b/src/Core/Validation/LeafFieldSelectionsVisitor.cs index 2ca3d577c93..7cd921e1240 100644 --- a/src/Core/Validation/LeafFieldSelectionsVisitor.cs +++ b/src/Core/Validation/LeafFieldSelectionsVisitor.cs @@ -42,7 +42,7 @@ public LeafFieldSelectionsVisitor(ISchema schema) Errors.Add(new ValidationError( $"`{field.Name}` is {type} field. Selections on scalars " + "or enums are never allowed, because they are the leaf " + - "nodes of any GraphQL query", fieldSelection)); + "nodes of any GraphQL query.", fieldSelection)); } } @@ -53,9 +53,9 @@ public LeafFieldSelectionsVisitor(ISchema schema) if (fieldSelection.SelectionSet == null) { Errors.Add(new ValidationError( - $"`{field.Name}` is a object, interface or union type " + + $"`{field.Name}` is an object, interface or union type " + "field. Leaf selections on objects, interfaces, and " + - "unions without subfields are disallowed. ", + "unions without subfields are disallowed.", fieldSelection)); } } From 91a045ec8ec7dea90320b518de7b15ce671c8f74 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 16:23:17 +0200 Subject: [PATCH 03/30] updated roadmap --- README.md | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index f35c0ec7b43..b54de93bb49 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,7 @@ dotnet new graphql-server ## Features -We currently support the following parts of the current [draft spec](http://facebook.github.io/graphql/draft/) of GraphQL. +We currently support the following parts of the current [June 2018 specification](http://facebook.github.io/graphql/June2018/) of GraphQL. ### Types @@ -210,11 +210,11 @@ We currently support the following parts of the current [draft spec](http://face - [x] Skip - [x] Continue -- [ ] Deprecated +- [ ] _Deprecated_ (in development) ### Validation -- [ ] [Validation](https://github.com/ChilliCream/hotchocolate/projects/3) +- [ ] [_Validation_](https://github.com/ChilliCream/hotchocolate/projects/3) ### Execution @@ -225,18 +225,20 @@ We currently support the following parts of the current [draft spec](http://face ### Introspection - Fields - - [x] __typename - - [x] __type - - [x] __schema -- __Schema + - [x] \_\_typename + - [x] \_\_type + - [x] \_\_schema + +- \_\_Schema + - [x] types - [x] queryType - [x] mutationType - [x] subscriptionType - [x] directives -- __Type +- \_\_Type - [x] kind - [x] name - [x] fields @@ -254,6 +256,7 @@ Moreover, we are working on the following parts that are not defined in the spec - [x] Date - [ ] Time - [ ] URL +- [ ] UUID - [x] Decimal - [x] Short (Int16) - [x] Long (Int64) @@ -264,8 +267,8 @@ Moreover, we are working on the following parts that are not defined in the spec - [ ] Export - [ ] Defer - [ ] Stream -- [ ] Custom Schema Directives -- [ ] Custom Execution Directives +- [ ] _Custom Schema Directives_ (in development) +- [ ] _Custom Execution Directives_ (in development) ### Execution Engine @@ -280,6 +283,7 @@ Moreover, we are working on the following parts that are not defined in the spec ## Supported Frameworks - [ ] ASP.NET Classic + - [ ] Get - [ ] Post From 15250fc3eb9355deae8f9951d572312d14ab64b5 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 16:24:12 +0200 Subject: [PATCH 04/30] updated roadmap --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b54de93bb49..7921bf6adc5 100644 --- a/README.md +++ b/README.md @@ -210,17 +210,17 @@ We currently support the following parts of the current [June 2018 specification - [x] Skip - [x] Continue -- [ ] _Deprecated_ (in development) +- [ ] _Deprecated_ (in development - 0.5.0) ### Validation -- [ ] [_Validation_](https://github.com/ChilliCream/hotchocolate/projects/3) +- [ ] [_Validation_](https://github.com/ChilliCream/hotchocolate/projects/3) (in development - 0.4.5) ### Execution - [x] Query - [x] Mutation -- [ ] Subscription +- [ ] _Subscription_ (in development - 0.5.0) ### Introspection @@ -267,8 +267,8 @@ Moreover, we are working on the following parts that are not defined in the spec - [ ] Export - [ ] Defer - [ ] Stream -- [ ] _Custom Schema Directives_ (in development) -- [ ] _Custom Execution Directives_ (in development) +- [ ] _Custom Schema Directives_ (in development - 0.5.0) +- [ ] _Custom Execution Directives_ (in development - 0.5.0) ### Execution Engine From d8c20ddf1bdbca8e0f7641ff29525d203d1d85ce Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 17:12:29 +0200 Subject: [PATCH 05/30] Started working on fragmnetoncompositetypesrule tests --- .../FragmentsOnCompositeTypesRule.cs | 43 ++++++++++++ .../Validation/QueryValidatorTests.cs | 31 ++++++++- .../FragmentsOnCompositeTypesRule.cs | 11 +++ .../FragmentsOnCompositeTypesVisitor.cs | 69 +++++++++++++++++++ src/Core/Validation/QueryValidator.cs | 2 + 5 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs create mode 100644 src/Core/Validation/FragmentsOnCompositeTypesRule.cs create mode 100644 src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs diff --git a/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs b/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs new file mode 100644 index 00000000000..bd9775e2449 --- /dev/null +++ b/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs @@ -0,0 +1,43 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class FragmentsOnCompositeTypesRuleTests + : ValidationTestBase + { + public FragmentsOnCompositeTypesRuleTests() + : base(new FragmentsOnCompositeTypesRule()) + { + } + + [Fact] + public void ScalarSelectionsNotAllowedOnInt() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ... fragOnObject + } + } + + fragment fragOnObject on Dog { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "`barkVolume` is a scalar field. Selections on scalars " + + "or enums are never allowed, because they are the leaf " + + "nodes of any GraphQL query.")); + } + } +} diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index ad07796ef90..0ed2a5b2794 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -496,7 +496,7 @@ public void UnusedFragment() "is not used within the current document.", t.Message)); } - [Fact] + [Fact] public void DuplicateFragments() { // arrange @@ -531,5 +531,34 @@ public void DuplicateFragments() "There are multiple fragments with the name `fragmentOne`.", t.Message)); } + + [Fact] + public void ScalarSelectionsNotAllowedOnInt() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + barkVolume { + sinceWhen + } + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "`barkVolume` is a scalar field. Selections on scalars " + + "or enums are never allowed, because they are the leaf " + + "nodes of any GraphQL query.")); + } } } diff --git a/src/Core/Validation/FragmentsOnCompositeTypesRule.cs b/src/Core/Validation/FragmentsOnCompositeTypesRule.cs new file mode 100644 index 00000000000..e872d88c24a --- /dev/null +++ b/src/Core/Validation/FragmentsOnCompositeTypesRule.cs @@ -0,0 +1,11 @@ +namespace HotChocolate.Validation +{ + internal sealed class FragmentsOnCompositeTypesRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new FragmentsOnCompositeTypesVisitor(schema); + } + } +} diff --git a/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs b/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs new file mode 100644 index 00000000000..98bb6f65303 --- /dev/null +++ b/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs @@ -0,0 +1,69 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class FragmentsOnCompositeTypesVisitor + : QueryVisitorErrorBase + { + private readonly List _fragmentErrors = + new List(); + + public FragmentsOnCompositeTypesVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitDocument( + DocumentNode document, + ImmutableStack path) + { + _fragmentErrors.Clear(); + + base.VisitDocument(document, path); + + if (_fragmentErrors.Count > 0) + { + Errors.Add(new ValidationError( + "Fragments can only be declared on unions, interfaces, " + + "and objects.", _fragmentErrors)); + } + } + + protected override void VisitFragmentDefinition( + FragmentDefinitionNode fragmentDefinition, + ImmutableStack path) + { + ValidateTypeCondition( + fragmentDefinition, + fragmentDefinition.TypeCondition.Name.Value); + + base.VisitFragmentDefinition(fragmentDefinition, path); + } + + protected override void VisitInlineFragment( + InlineFragmentNode inlineFragment, + IType type, + ImmutableStack path) + { + ValidateTypeCondition( + inlineFragment, + inlineFragment.TypeCondition.Name.Value); + + base.VisitInlineFragment(inlineFragment, type, path); + } + + private void ValidateTypeCondition( + ISyntaxNode syntaxNode, + string typeCondition) + { + if (Schema.TryGetType(typeCondition, out INamedType type) + && !type.IsCompositeType()) + { + _fragmentErrors.Add(syntaxNode); + } + } + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index 5a271777b32..de1c5ba40d3 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -26,6 +26,8 @@ public class QueryValidator new ArgumentNamesRule(), new FragmentsMustBeUsedRule(), new FragmentNameUniquenessRule(), + new LeafFieldSelectionsRule(), + new FragmentsOnCompositeTypesRule() }; private readonly ISchema _schema; From dbc3afd7617781b0535862741d94d4239d2b0c7e Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 20:50:56 +0200 Subject: [PATCH 06/30] Added fragment type tests --- .../FragmentsOnCompositeTypesRule.cs | 111 +++++++++++++++++- 1 file changed, 107 insertions(+), 4 deletions(-) diff --git a/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs b/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs index bd9775e2449..926f258d495 100644 --- a/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs +++ b/src/Core.Tests/Validation/FragmentsOnCompositeTypesRule.cs @@ -12,7 +12,7 @@ public FragmentsOnCompositeTypesRuleTests() } [Fact] - public void ScalarSelectionsNotAllowedOnInt() + public void FragOnObject() { // arrange Schema schema = ValidationUtils.CreateSchema(); @@ -31,13 +31,116 @@ ... fragOnObject // act QueryValidationResult result = Rule.Validate(schema, query); + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void FragOnInterface() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ... fragOnInterface + } + } + + fragment fragOnInterface on Pet { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void FragOnUnion() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ... fragOnUnion + } + } + + fragment fragOnUnion on CatOrDog { + ... on Dog { + name + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void FragOnScalar() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ... fragOnScalar + } + } + + fragment fragOnScalar on Int { + something + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "Fragments can only be declared on unions, interfaces, " + + "and objects.")); + } + + [Fact] + public void InlineFragOnScalar() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ... inlineFragOnScalar + } + } + + fragment inlineFragOnScalar on Dog { + ... on Boolean { + somethingElse + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + // assert Assert.True(result.HasErrors); Assert.Collection(result.Errors, t => Assert.Equal(t.Message, - "`barkVolume` is a scalar field. Selections on scalars " + - "or enums are never allowed, because they are the leaf " + - "nodes of any GraphQL query.")); + "Fragments can only be declared on unions, interfaces, " + + "and objects.")); } } } From bc3cbe23532f0fb2f48842e79d424ba7a51f212c Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 20:54:55 +0200 Subject: [PATCH 07/30] Added integration test --- .../Validation/QueryValidatorTests.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 0ed2a5b2794..66569d2dbe3 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -560,5 +560,37 @@ public void ScalarSelectionsNotAllowedOnInt() "or enums are never allowed, because they are the leaf " + "nodes of any GraphQL query.")); } + + [Fact] + public void InlineFragOnScalar() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ... inlineFragOnScalar + } + } + + fragment inlineFragOnScalar on Dog { + ... on Boolean { + somethingElse + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "Fragments can only be declared on unions, interfaces, " + + "and objects.")); + } } } From d105fabfa1b251fbdb50227e27142c26dce70f82 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 22:09:38 +0200 Subject: [PATCH 08/30] Added fragment spreads must not form cycles rule --- ...agmentSpreadsMustNotFormCyclesRuleTests.cs | 166 ++++++++++++++++++ .../Validation/QueryValidatorTests.cs | 38 ++++ .../FragmentSpreadsMustNotFormCyclesRule.cs | 11 ++ ...FragmentSpreadsMustNotFormCyclesVisitor.cs | 142 +++++++++++++++ src/Core/Validation/QueryValidator.cs | 3 +- src/Core/Validation/QueryVisitor.cs | 17 +- 6 files changed, 370 insertions(+), 7 deletions(-) create mode 100644 src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs create mode 100644 src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs create mode 100644 src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs diff --git a/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs b/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs new file mode 100644 index 00000000000..2444334cd77 --- /dev/null +++ b/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs @@ -0,0 +1,166 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class FragmentSpreadsMustNotFormCyclesRuleTests + : ValidationTestBase + { + public FragmentSpreadsMustNotFormCyclesRuleTests() + : base(new FragmentSpreadsMustNotFormCyclesRule()) + { + } + + [Fact] + public void FragmentCycle1() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...nameFragment + } + } + + fragment nameFragment on Dog { + name + ...barkVolumeFragment + } + + fragment barkVolumeFragment on Dog { + barkVolume + ...nameFragment + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The graph of fragment spreads must not form any " + + "cycles including spreading itself. Otherwise an " + + "operation could infinitely spread or infinitely " + + "execute on cycles in the underlying data.")); + } + + [Fact] + public void FragmentCycle2() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...nameFragment + } + } + + fragment nameFragment on Dog { + name + ...barkVolumeFragment + } + + fragment barkVolumeFragment on Dog { + barkVolume + ...barkVolumeFragment1 + } + + fragment barkVolumeFragment1 on Dog { + barkVolume + ...barkVolumeFragment2 + } + + fragment barkVolumeFragment2 on Dog { + barkVolume + ...nameFragment + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The graph of fragment spreads must not form any " + + "cycles including spreading itself. Otherwise an " + + "operation could infinitely spread or infinitely " + + "execute on cycles in the underlying data.")); + } + + [Fact] + public void InfiniteRecursion() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...dogFragment + } + } + + fragment dogFragment on Dog { + name + owner { + ...ownerFragment + } + } + + fragment ownerFragment on Dog { + name + pets { + ...dogFragment + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The graph of fragment spreads must not form any " + + "cycles including spreading itself. Otherwise an " + + "operation could infinitely spread or infinitely " + + "execute on cycles in the underlying data.")); + } + + [Fact] + public void QueryWithSideBySideFragSpreads() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...dogFragment + ...dogFragment + ...dogFragment + ...dogFragment + ...dogFragment + ...dogFragment + ...dogFragment + } + } + + fragment dogFragment on Dog { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + } +} diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 66569d2dbe3..0d2d209e24c 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -592,5 +592,43 @@ ... inlineFragOnScalar "Fragments can only be declared on unions, interfaces, " + "and objects.")); } + + [Fact] + public void FragmentCycle1() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...nameFragment + } + } + + fragment nameFragment on Dog { + name + ...barkVolumeFragment + } + + fragment barkVolumeFragment on Dog { + barkVolume + ...nameFragment + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The graph of fragment spreads must not form any " + + "cycles including spreading itself. Otherwise an " + + "operation could infinitely spread or infinitely " + + "execute on cycles in the underlying data.")); + } } } diff --git a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs new file mode 100644 index 00000000000..38600450feb --- /dev/null +++ b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs @@ -0,0 +1,11 @@ +namespace HotChocolate.Validation +{ + internal sealed class FragmentSpreadsMustNotFormCyclesRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new FragmentSpreadsMustNotFormCyclesVisitor(schema); + } + } +} diff --git a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs new file mode 100644 index 00000000000..bfe108c70d8 --- /dev/null +++ b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs @@ -0,0 +1,142 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class FragmentSpreadsMustNotFormCyclesVisitor + : QueryVisitorErrorBase + { + private HashSet _visited = + new HashSet(); + + private bool _cycleDetected; + + public FragmentSpreadsMustNotFormCyclesVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitOperationDefinitions( + IEnumerable oprationDefinitions, + ImmutableStack path) + { + foreach (OperationDefinitionNode operation in oprationDefinitions) + { + _visited.Clear(); + VisitOperationDefinition(operation, path); + if (_cycleDetected) + { + return; + } + } + } + + protected override void VisitFragmentDefinitions( + IEnumerable fragmentDefinitions, + ImmutableStack path) + { + } + + protected override void VisitFragmentSpread( + FragmentSpreadNode fragmentSpread, + IType type, + ImmutableStack path) + { + if (_cycleDetected) + { + return; + } + + ImmutableStack newpath = path.Push(fragmentSpread); + + if (path.Last() is DocumentNode d) + { + string fragmentName = fragmentSpread.Name.Value; + if (TryGetFragment(fragmentName, + out FragmentDefinitionNode fragment)) + { + VisitFragmentDefinition(fragment, newpath); + } + } + + VisitDirectives(fragmentSpread.Directives, newpath); + } + + protected override void VisitFragmentDefinition( + FragmentDefinitionNode fragmentDefinition, + ImmutableStack path) + { + if (_cycleDetected) + { + return; + } + + if (fragmentDefinition.TypeCondition?.Name?.Value != null + && Schema.TryGetType( + fragmentDefinition.TypeCondition.Name.Value, + out INamedOutputType typeCondition)) + { + if (_visited.Add(fragmentDefinition)) + { + ImmutableStack newpath = path + .Push(fragmentDefinition); + + VisitSelectionSet( + fragmentDefinition.SelectionSet, + typeCondition, + newpath); + + VisitDirectives( + fragmentDefinition.Directives, + newpath); + } + else + { + DetectCycle(fragmentDefinition, path); + } + } + } + + private void DetectCycle( + FragmentDefinitionNode fragmentDefinition, + ImmutableStack path) + { + ImmutableStack current = path; + + while (current.Any()) + { + current = current.Pop(out ISyntaxNode node); + + if (node == fragmentDefinition) + { + Errors.Add(new ValidationError( + "The graph of fragment spreads must not form any " + + "cycles including spreading itself. Otherwise an " + + "operation could infinitely spread or infinitely " + + "execute on cycles in the underlying data.", + GetCyclePath(path))); + return; + } + } + } + + private IEnumerable GetCyclePath( + ImmutableStack path) + { + ImmutableStack current = path; + + while (current.Any()) + { + current = current.Pop(out ISyntaxNode node); + + if (node is FragmentSpreadNode) + { + yield return node; + } + } + } + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index de1c5ba40d3..3f64853b57e 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -27,7 +27,8 @@ public class QueryValidator new FragmentsMustBeUsedRule(), new FragmentNameUniquenessRule(), new LeafFieldSelectionsRule(), - new FragmentsOnCompositeTypesRule() + new FragmentsOnCompositeTypesRule(), + new FragmentSpreadsMustNotFormCyclesRule(), }; private readonly ISchema _schema; diff --git a/src/Core/Validation/QueryVisitor.cs b/src/Core/Validation/QueryVisitor.cs index 92b17cd9e71..efe17748ade 100644 --- a/src/Core/Validation/QueryVisitor.cs +++ b/src/Core/Validation/QueryVisitor.cs @@ -39,7 +39,6 @@ public void VisitDocument(DocumentNode document) VisitDocument(document, path); } - protected virtual void VisitDocument( DocumentNode document, ImmutableStack path) @@ -94,7 +93,6 @@ public void VisitDocument(DocumentNode document) ImmutableStack newpath = path.Push(selectionSet); foreach (ISelectionNode selection in selectionSet.Selections) { - if (selection is FieldNode field) { VisitField(field, type, newpath); @@ -144,9 +142,8 @@ public void VisitDocument(DocumentNode document) if (path.Last() is DocumentNode d) { string fragmentName = fragmentSpread.Name.Value; - if (_visitedFragments.Add(fragmentName) - && _fragments.TryGetValue(fragmentName, - out FragmentDefinitionNode fragment)) + if (_fragments.TryGetValue(fragmentName, + out FragmentDefinitionNode fragment)) { VisitFragmentDefinition(fragment, newpath); } @@ -182,7 +179,8 @@ public void VisitDocument(DocumentNode document) FragmentDefinitionNode fragmentDefinition, ImmutableStack path) { - if (fragmentDefinition.TypeCondition?.Name?.Value != null + if (_visitedFragments.Add(fragmentDefinition.Name.Value) + && fragmentDefinition.TypeCondition?.Name?.Value != null && Schema.TryGetType( fragmentDefinition.TypeCondition.Name.Value, out INamedOutputType typeCondition)) @@ -222,5 +220,12 @@ protected void ClearVisitedFragments() { _visitedFragments.Clear(); } + + protected bool TryGetFragment( + string fragmentName, + out FragmentDefinitionNode fragment) + { + return _fragments.TryGetValue(fragmentName, out fragment); + } } } From 7063efb9ee2ac9433bef293569ee74d99071148f Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Fri, 24 Aug 2018 22:14:07 +0200 Subject: [PATCH 09/30] Updated Readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7921bf6adc5..6fd9b578295 100644 --- a/README.md +++ b/README.md @@ -214,7 +214,7 @@ We currently support the following parts of the current [June 2018 specification ### Validation -- [ ] [_Validation_](https://github.com/ChilliCream/hotchocolate/projects/3) (in development - 0.4.5) +- [x] [_Validation_](https://github.com/ChilliCream/hotchocolate/projects/3) ### Execution From 545997b34f07f0abbd46a435ba6ff347c53f7e29 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 09:29:10 +0200 Subject: [PATCH 10/30] Added fragment spread target defined rule --- .../FragmentSpreadTargetDefinedRuleTests.cs | 62 +++++++++++++++++++ .../Validation/QueryValidatorTests.cs | 25 ++++++++ .../FragmentSpreadTargetDefinedRule.cs | 13 ++++ .../FragmentSpreadTargetDefinedVisitor.cs | 54 ++++++++++++++++ src/Core/Validation/QueryValidator.cs | 1 + src/Core/Validation/QueryVisitor.cs | 5 ++ 6 files changed, 160 insertions(+) create mode 100644 src/Core.Tests/Validation/FragmentSpreadTargetDefinedRuleTests.cs create mode 100644 src/Core/Validation/FragmentSpreadTargetDefinedRule.cs create mode 100644 src/Core/Validation/FragmentSpreadTargetDefinedVisitor.cs diff --git a/src/Core.Tests/Validation/FragmentSpreadTargetDefinedRuleTests.cs b/src/Core.Tests/Validation/FragmentSpreadTargetDefinedRuleTests.cs new file mode 100644 index 00000000000..a1994b7ddcd --- /dev/null +++ b/src/Core.Tests/Validation/FragmentSpreadTargetDefinedRuleTests.cs @@ -0,0 +1,62 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class FragmentSpreadTargetDefinedRuleTests + : ValidationTestBase + { + public FragmentSpreadTargetDefinedRuleTests() + : base(new FragmentSpreadTargetDefinedRule()) + { + } + + [Fact] + public void UndefinedFragment() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...undefinedFragment + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The specified fragment `undefinedFragment` does not exist.")); + } + + [Fact] + public void DefinedFragment() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...definedFragment + } + } + + fragment definedFragment on Dog + { + barkVolume + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + } +} diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 0d2d209e24c..f71d2e5dbcc 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -630,5 +630,30 @@ public void FragmentCycle1() "operation could infinitely spread or infinitely " + "execute on cycles in the underlying data.")); } + + [Fact] + public void UndefinedFragment() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...undefinedFragment + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The specified fragment `undefinedFragment` does not exist.")); + } } } diff --git a/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs b/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs new file mode 100644 index 00000000000..ba1b8a83a61 --- /dev/null +++ b/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs @@ -0,0 +1,13 @@ +using System.Linq; + +namespace HotChocolate.Validation +{ + internal sealed class FragmentSpreadTargetDefinedRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new FragmentSpreadTargetDefinedVisitor(schema); + } + } +} diff --git a/src/Core/Validation/FragmentSpreadTargetDefinedVisitor.cs b/src/Core/Validation/FragmentSpreadTargetDefinedVisitor.cs new file mode 100644 index 00000000000..9ce70c7f5b3 --- /dev/null +++ b/src/Core/Validation/FragmentSpreadTargetDefinedVisitor.cs @@ -0,0 +1,54 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class FragmentSpreadTargetDefinedVisitor + : QueryVisitorErrorBase + { + private readonly Dictionary> _missingFragments = + new Dictionary>(); + + public FragmentSpreadTargetDefinedVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitDocument( + DocumentNode document, + ImmutableStack path) + { + base.VisitDocument(document, path); + + foreach (KeyValuePair> item in + _missingFragments) + { + Errors.Add(new ValidationError( + $"The specified fragment `{item.Key}` does not exist.", + item.Value)); + } + } + + protected override void VisitFragmentSpread( + FragmentSpreadNode fragmentSpread, + IType type, + ImmutableStack path) + { + if (!ContainsFragment(fragmentSpread.Name.Value)) + { + string fragmentName = fragmentSpread.Name.Value; + if (!_missingFragments.TryGetValue(fragmentName, + out List f)) + { + f = new List(); + _missingFragments[fragmentName] = f; + } + f.Add(fragmentSpread); + } + + base.VisitFragmentSpread(fragmentSpread, type, path); + } + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index 3f64853b57e..fe73a7bce59 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -29,6 +29,7 @@ public class QueryValidator new LeafFieldSelectionsRule(), new FragmentsOnCompositeTypesRule(), new FragmentSpreadsMustNotFormCyclesRule(), + new FragmentSpreadTargetDefinedRule(), }; private readonly ISchema _schema; diff --git a/src/Core/Validation/QueryVisitor.cs b/src/Core/Validation/QueryVisitor.cs index efe17748ade..604ccd806b1 100644 --- a/src/Core/Validation/QueryVisitor.cs +++ b/src/Core/Validation/QueryVisitor.cs @@ -227,5 +227,10 @@ protected void ClearVisitedFragments() { return _fragments.TryGetValue(fragmentName, out fragment); } + + protected bool ContainsFragment(string fragmentName) + { + return _fragments.ContainsKey(fragmentName); + } } } From 9e38d68a46cdb157d131914f2ec3ffd0eec8a83f Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 12:16:27 +0200 Subject: [PATCH 11/30] implemented FragmentSpreadIsPossible rule --- .../FragmentSpreadIsPossibleRuleTests.cs | 114 ++++++++++++++++++ .../Validation/QueryValidatorTests.cs | 32 ++++- .../DirectivesAreInValidLocationsRule.cs | 9 ++ .../FragmentSpreadIsPossibleRule.cs | 22 ++++ .../FragmentSpreadIsPossibleVisitor.cs | 56 +++++++++ .../FragmentSpreadTargetDefinedRule.cs | 8 ++ .../FragmentSpreadsMustNotFormCyclesRule.cs | 7 ++ .../FragmentsOnCompositeTypesRule.cs | 8 ++ src/Core/Validation/QueryValidator.cs | 1 + 9 files changed, 256 insertions(+), 1 deletion(-) create mode 100644 src/Core.Tests/Validation/FragmentSpreadIsPossibleRuleTests.cs create mode 100644 src/Core/Validation/FragmentSpreadIsPossibleRule.cs create mode 100644 src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs diff --git a/src/Core.Tests/Validation/FragmentSpreadIsPossibleRuleTests.cs b/src/Core.Tests/Validation/FragmentSpreadIsPossibleRuleTests.cs new file mode 100644 index 00000000000..5c6f16f6d6f --- /dev/null +++ b/src/Core.Tests/Validation/FragmentSpreadIsPossibleRuleTests.cs @@ -0,0 +1,114 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class FragmentSpreadIsPossibleRuleTests + : ValidationTestBase + { + public FragmentSpreadIsPossibleRuleTests() + : base(new FragmentSpreadIsPossibleRule()) + { + } + + [Fact] + public void FragmentDoesNotMatchType() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...fragmentDoesNotMatchType + } + } + + fragment fragmentDoesNotMatchType on Human { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The parent type does not match the type condition on " + + "the fragment `fragmentDoesNotMatchType`.")); + } + + [Fact] + public void InterfaceTypeDoesMatch() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...interfaceTypeDoesMatch + } + } + + fragment interfaceTypeDoesMatch on Pet { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void UnionTypeDoesMatch() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...unionTypeDoesMatch + } + } + + fragment unionTypeDoesMatch on CatOrDog { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void ObjectTypeDoesMatch() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...objectTypeDoesMatch + } + } + + fragment objectTypeDoesMatch on Dog { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + } +} diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index f71d2e5dbcc..30f203fc4ac 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -7,7 +7,7 @@ namespace HotChocolate.Validation public class QueryValidatorTests { [Fact] - public void SchemaIsNulll() + public void SchemaIsNull() { // act Action a = () => new QueryValidator(null); @@ -655,5 +655,35 @@ public void UndefinedFragment() t => Assert.Equal(t.Message, "The specified fragment `undefinedFragment` does not exist.")); } + + [Fact] + public void FragmentDoesNotMatchType() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...fragmentDoesNotMatchType + } + } + + fragment fragmentDoesNotMatchType on Human { + name + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The parent type does not match the type condition on " + + "the fragment `fragmentDoesNotMatchType`.")); + } } } diff --git a/src/Core/Validation/DirectivesAreInValidLocationsRule.cs b/src/Core/Validation/DirectivesAreInValidLocationsRule.cs index 75ec3a869a1..24909de485a 100644 --- a/src/Core/Validation/DirectivesAreInValidLocationsRule.cs +++ b/src/Core/Validation/DirectivesAreInValidLocationsRule.cs @@ -3,6 +3,15 @@ namespace HotChocolate.Validation { + /// + ///GraphQL servers define what directives they support and where they + /// support them. + /// + /// For each usage of a directive, the directive must be used in a + /// location that the server has declared support for. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Directives-Are-In-Valid-Locations + /// internal sealed class DirectivesAreInValidLocationsRule : QueryVisitorValidationErrorBase { diff --git a/src/Core/Validation/FragmentSpreadIsPossibleRule.cs b/src/Core/Validation/FragmentSpreadIsPossibleRule.cs new file mode 100644 index 00000000000..8d4f0c83390 --- /dev/null +++ b/src/Core/Validation/FragmentSpreadIsPossibleRule.cs @@ -0,0 +1,22 @@ +namespace HotChocolate.Validation +{ + /// + /// Fragments are declared on a type and will only apply when the + /// runtime object type matches the type condition. + /// + /// They also are spread within the context of a parent type. + /// + /// A fragment spread is only valid if its type condition could ever + /// apply within the parent type. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Fragment-spread-is-possible + /// + internal sealed class FragmentSpreadIsPossibleRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new FragmentSpreadIsPossibleVisitor(schema); + } + } +} diff --git a/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs b/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs new file mode 100644 index 00000000000..128989debf0 --- /dev/null +++ b/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs @@ -0,0 +1,56 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class FragmentSpreadIsPossibleVisitor + : QueryVisitorErrorBase + { + private HashSet _visited = + new HashSet(); + + private bool _cycleDetected; + + public FragmentSpreadIsPossibleVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitFragmentSpread( + FragmentSpreadNode fragmentSpread, + IType type, + ImmutableStack path) + { + if (type is INamedType parentType + && TryGetFragment(fragmentSpread.Name.Value, + out FragmentDefinitionNode fragment) + && Schema.TryGetType(fragment.TypeCondition.Name.Value, + out INamedOutputType typeCondition) + && parentType.IsCompositeType() + && typeCondition.IsCompositeType() + && !GetPossibleType(parentType) + .Intersect(GetPossibleType(typeCondition)) + .Any()) + { + Errors.Add(new ValidationError( + "The parent type does not match the type condition on " + + $"the fragment `{fragment.Name}`.", fragmentSpread)); + } + + base.VisitFragmentSpread(fragmentSpread, type, path); + } + + private IEnumerable GetPossibleType(INamedType type) + { + if (type is ObjectType ot) + { + return new[] { ot }; + } + + return Schema.GetPossibleTypes(type); + } + } +} diff --git a/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs b/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs index ba1b8a83a61..a6bebc0ed97 100644 --- a/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs +++ b/src/Core/Validation/FragmentSpreadTargetDefinedRule.cs @@ -2,6 +2,14 @@ namespace HotChocolate.Validation { + /// + /// Named fragment spreads must refer to fragments defined within the + /// document. + /// + /// It is a validation error if the target of a spread is not defined. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Fragment-spread-target-defined + /// internal sealed class FragmentSpreadTargetDefinedRule : QueryVisitorValidationErrorBase { diff --git a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs index 38600450feb..26db8b76347 100644 --- a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs +++ b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesRule.cs @@ -1,5 +1,12 @@ namespace HotChocolate.Validation { + /// + /// The graph of fragment spreads must not form any cycles including + /// spreading itself. Otherwise an operation could infinitely spread or + /// infinitely execute on cycles in the underlying data. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Fragment-spreads-must-not-form-cycles + /// internal sealed class FragmentSpreadsMustNotFormCyclesRule : QueryVisitorValidationErrorBase { diff --git a/src/Core/Validation/FragmentsOnCompositeTypesRule.cs b/src/Core/Validation/FragmentsOnCompositeTypesRule.cs index e872d88c24a..3fbaf02c66e 100644 --- a/src/Core/Validation/FragmentsOnCompositeTypesRule.cs +++ b/src/Core/Validation/FragmentsOnCompositeTypesRule.cs @@ -1,5 +1,13 @@ namespace HotChocolate.Validation { + /// + /// Fragments can only be declared on unions, interfaces, and objects. + /// They are invalid on scalars. + /// They can only be applied on non‐leaf fields. + /// This rule applies to both inline and named fragments. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Fragments-On-Composite-Types + /// internal sealed class FragmentsOnCompositeTypesRule : QueryVisitorValidationErrorBase { diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index fe73a7bce59..89582328fc0 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -30,6 +30,7 @@ public class QueryValidator new FragmentsOnCompositeTypesRule(), new FragmentSpreadsMustNotFormCyclesRule(), new FragmentSpreadTargetDefinedRule(), + new FragmentSpreadIsPossibleRule(), }; private readonly ISchema _schema; From d3d340d729f2bd6c758c3a584d8eef2053d85342 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 15:03:01 +0200 Subject: [PATCH 12/30] Fixed sonar issues --- src/Core/Validation/QueryVisitor.cs | 12 +++++------- src/Core/Validation/RequiredArgumentVisitor.cs | 14 ++++++-------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/Core/Validation/QueryVisitor.cs b/src/Core/Validation/QueryVisitor.cs index 604ccd806b1..2e59ad96bdc 100644 --- a/src/Core/Validation/QueryVisitor.cs +++ b/src/Core/Validation/QueryVisitor.cs @@ -119,14 +119,12 @@ public void VisitDocument(DocumentNode document) ImmutableStack newpath = path.Push(field); if (type is IComplexOutputType complexType - && complexType.Fields.ContainsField(field.Name.Value)) + && complexType.Fields.ContainsField(field.Name.Value) + && field.SelectionSet != null) { - if (field.SelectionSet != null) - { - VisitSelectionSet(field.SelectionSet, - complexType.Fields[field.Name.Value].Type.NamedType(), - newpath); - } + VisitSelectionSet(field.SelectionSet, + complexType.Fields[field.Name.Value].Type.NamedType(), + newpath); } VisitDirectives(field.Directives, newpath); diff --git a/src/Core/Validation/RequiredArgumentVisitor.cs b/src/Core/Validation/RequiredArgumentVisitor.cs index 2c1394ab3f5..9c9e991545b 100644 --- a/src/Core/Validation/RequiredArgumentVisitor.cs +++ b/src/Core/Validation/RequiredArgumentVisitor.cs @@ -9,8 +9,7 @@ namespace HotChocolate.Validation internal sealed class RequiredArgumentVisitor : QueryVisitorErrorBase { - private readonly Dictionary _directives = - new Dictionary(); + private readonly Dictionary _directives; public RequiredArgumentVisitor(ISchema schema) : base(schema) @@ -23,15 +22,14 @@ public RequiredArgumentVisitor(ISchema schema) IType type, ImmutableStack path) { - if (type is IComplexOutputType complexType) + if (type is IComplexOutputType complexType + && complexType.Fields.ContainsField(field.Name.Value)) { - if (complexType.Fields.ContainsField(field.Name.Value)) - { - ValidateRequiredArguments(field, field.Arguments, - complexType.Fields[field.Name.Value].Arguments); - } + ValidateRequiredArguments(field, field.Arguments, + complexType.Fields[field.Name.Value].Arguments); } + base.VisitField(field, type, path); } From 1026eab5878155a495d8224373c6d7d65633536f Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 15:58:19 +0200 Subject: [PATCH 13/30] Fixed sonar issues --- .../Validation/ValuesOfCorrectTypeRule.cs | 58 +++++++++++++++++++ src/Types/Types/Scalars/DateTimeTypeBase.cs | 2 +- src/Types/Types/TypeInitializationContext.cs | 34 ++++++----- 3 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 src/Core/Validation/ValuesOfCorrectTypeRule.cs diff --git a/src/Core/Validation/ValuesOfCorrectTypeRule.cs b/src/Core/Validation/ValuesOfCorrectTypeRule.cs new file mode 100644 index 00000000000..db958c27804 --- /dev/null +++ b/src/Core/Validation/ValuesOfCorrectTypeRule.cs @@ -0,0 +1,58 @@ +using System.Collections.Immutable; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class ValuesOfCorrectTypeVisitor + : QueryVisitorErrorBase + { + public ValuesOfCorrectTypeVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitOperationDefinition( + OperationDefinitionNode operation, + ImmutableStack path) + { + + } + + protected override void VisitField( + FieldNode field, + IType type, + ImmutableStack path) + { + IOutputField f; + f.Arguments + } + + protected override void VisitDirective( + DirectiveNode directive, + ImmutableStack path) + { + + } + + private void VisitArgument( + ISyntaxNode node, + IFieldCollection arguments, + string argumentName, + IValueNode argumentValue, + ImmutableStack path) + { + if (arguments.TryGetField(argumentName, out IInputField argument)) + { + if (argumentValue is VariableNode) + { + if() + } + else if (!argument.Type.IsInstanceOfType(argumentValue)) + { + Errors.Add(new ValidationError("", node)); + } + } + } + } +} diff --git a/src/Types/Types/Scalars/DateTimeTypeBase.cs b/src/Types/Types/Scalars/DateTimeTypeBase.cs index 9376894340f..bbcdc560369 100644 --- a/src/Types/Types/Scalars/DateTimeTypeBase.cs +++ b/src/Types/Types/Scalars/DateTimeTypeBase.cs @@ -6,7 +6,7 @@ namespace HotChocolate.Types public abstract class DateTimeTypeBase : ScalarType { - public DateTimeTypeBase(string name, string description) + protected DateTimeTypeBase(string name, string description) : base(name, description) { } diff --git a/src/Types/Types/TypeInitializationContext.cs b/src/Types/Types/TypeInitializationContext.cs index 26c10c3753f..d54104d72c0 100644 --- a/src/Types/Types/TypeInitializationContext.cs +++ b/src/Types/Types/TypeInitializationContext.cs @@ -158,26 +158,28 @@ public bool TryGetNativeType(INamedType namedType, out Type nativeType) public bool TryGetProperty(INamedType namedType, string fieldName, out T member) where T : MemberInfo { - if (namedType is ObjectType) + if (namedType is ObjectType + && _schemaContext.Types.TryGetTypeBinding(namedType, + out ObjectTypeBinding binding) + && binding.Fields.TryGetValue(fieldName, + out FieldBinding fieldBinding) + && fieldBinding.Member is T m) { - if (_schemaContext.Types.TryGetTypeBinding(namedType, out ObjectTypeBinding binding) - && binding.Fields.TryGetValue(fieldName, out FieldBinding fieldBinding) - && fieldBinding.Member is T m) - { - member = m; - return true; - } + member = m; + return true; } - if (namedType is InputObjectType) + + if (namedType is InputObjectType + && _schemaContext.Types.TryGetTypeBinding(namedType, + out InputObjectTypeBinding inputBinding) + && inputBinding.Fields.TryGetValue(fieldName, + out InputFieldBinding inputFieldBinding) + && inputFieldBinding.Property is T p) { - if (_schemaContext.Types.TryGetTypeBinding(namedType, out InputObjectTypeBinding binding) - && binding.Fields.TryGetValue(fieldName, out InputFieldBinding fieldBinding) - && fieldBinding.Property is T p) - { - member = p; - return true; - } + member = p; + return true; + } member = null; From f3100e6d6e97af1d30b1b1285082877c9a6899ae Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 16:08:50 +0200 Subject: [PATCH 14/30] Disabled ValuesOfCorrectTypeRule --- ...peRule.cs => ValuesOfCorrectTypeRule.temp} | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) rename src/Core/Validation/{ValuesOfCorrectTypeRule.cs => ValuesOfCorrectTypeRule.temp} (63%) diff --git a/src/Core/Validation/ValuesOfCorrectTypeRule.cs b/src/Core/Validation/ValuesOfCorrectTypeRule.temp similarity index 63% rename from src/Core/Validation/ValuesOfCorrectTypeRule.cs rename to src/Core/Validation/ValuesOfCorrectTypeRule.temp index db958c27804..192c3d83ac9 100644 --- a/src/Core/Validation/ValuesOfCorrectTypeRule.cs +++ b/src/Core/Validation/ValuesOfCorrectTypeRule.temp @@ -1,4 +1,5 @@ using System.Collections.Immutable; +using System.Linq; using HotChocolate.Language; using HotChocolate.Types; @@ -46,7 +47,7 @@ public ValuesOfCorrectTypeVisitor(ISchema schema) { if (argumentValue is VariableNode) { - if() + } else if (!argument.Type.IsInstanceOfType(argumentValue)) { @@ -54,5 +55,28 @@ public ValuesOfCorrectTypeVisitor(ISchema schema) } } } + + private IType GetVariableType(string variableName, ImmutableStack path) + { + ImmutableStack current = path; + + while (current.Any()) + { + current = current.Pop(out ISyntaxNode node); + if (node is OperationDefinitionNode operation) + { + VariableDefinitionNode variable = operation + .VariableDefinitions.FirstOrDefault( + t => t.Variable.Name.Value == variableName); + if (variable != null) + { + string typeName = variable.Type.NamedType().Name.Value; + return Schema.GetType(typeName); + } + } + } + + return null; + } } } From f1684673c7b3d79855e201d666dfedb7f1b74920 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 21:08:48 +0200 Subject: [PATCH 15/30] Added more tests for the lead field rule --- .../StarWarsCodeFirstTests.cs | 22 +++++++++++++ .../LeafFieldSelectionsRuleTests.cs | 18 +++++++++++ src/Core.Tests/Validation/Models/Query.cs | 5 +++ .../FragmentSpreadIsPossibleVisitor.cs | 2 -- ...FragmentSpreadsMustNotFormCyclesVisitor.cs | 1 + .../FragmentsOnCompositeTypesVisitor.cs | 26 +++++++++++++--- src/Core/Validation/QueryVisitor.cs | 31 +++++++++++++++---- 7 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs b/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs index 09a1abdf680..2e76bacd5c9 100644 --- a/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs +++ b/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs @@ -460,6 +460,28 @@ query op($ep: [Episode!]!) Assert.Equal(Snapshot.Current(), Snapshot.New(result)); } + [Fact] + public void ConditionalInlineFragment() + { + // arrange + Schema schema = CreateSchema(); + string query = @" + { + heroes(episodes: EMPIRE) { + name + ... @include(if: true) { + height + } + } + }"; + + // act + IExecutionResult result = schema.Execute(query); + + // assert + Assert.Equal(Snapshot.Current(), Snapshot.New(result)); + } + private static Schema CreateSchema() { CharacterRepository repository = new CharacterRepository(); diff --git a/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs b/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs index b4cf89409a8..bcc634d4cf1 100644 --- a/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs +++ b/src/Core.Tests/Validation/LeafFieldSelectionsRuleTests.cs @@ -31,6 +31,24 @@ public void ScalarSelection() Assert.False(result.HasErrors); } + [Fact] + public void StringList() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + stringList + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + [Fact] public void ScalarSelectionsNotAllowedOnInt() { diff --git a/src/Core.Tests/Validation/Models/Query.cs b/src/Core.Tests/Validation/Models/Query.cs index c0141a9f3dd..d31903a8a2d 100644 --- a/src/Core.Tests/Validation/Models/Query.cs +++ b/src/Core.Tests/Validation/Models/Query.cs @@ -31,5 +31,10 @@ public object GetCatOrDog() { return null; } + + public string[] GetStringList() + { + return null; + } } } diff --git a/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs b/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs index 128989debf0..16fcd8700f5 100644 --- a/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs +++ b/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs @@ -12,8 +12,6 @@ internal sealed class FragmentSpreadIsPossibleVisitor private HashSet _visited = new HashSet(); - private bool _cycleDetected; - public FragmentSpreadIsPossibleVisitor(ISchema schema) : base(schema) { diff --git a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs index bfe108c70d8..73c914a1d40 100644 --- a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs +++ b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs @@ -112,6 +112,7 @@ public FragmentSpreadsMustNotFormCyclesVisitor(ISchema schema) if (node == fragmentDefinition) { + _cycleDetected = true; Errors.Add(new ValidationError( "The graph of fragment spreads must not form any " + "cycles including spreading itself. Otherwise an " + diff --git a/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs b/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs index 98bb6f65303..ee6f81e6716 100644 --- a/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs +++ b/src/Core/Validation/FragmentsOnCompositeTypesVisitor.cs @@ -45,22 +45,38 @@ public FragmentsOnCompositeTypesVisitor(ISchema schema) protected override void VisitInlineFragment( InlineFragmentNode inlineFragment, - IType type, + IType parentType, + IType typeCondition, ImmutableStack path) { ValidateTypeCondition( inlineFragment, - inlineFragment.TypeCondition.Name.Value); + typeCondition); - base.VisitInlineFragment(inlineFragment, type, path); + base.VisitInlineFragment( + inlineFragment, + parentType, + typeCondition, + path); } private void ValidateTypeCondition( ISyntaxNode syntaxNode, string typeCondition) { - if (Schema.TryGetType(typeCondition, out INamedType type) - && !type.IsCompositeType()) + if (typeCondition != null + && Schema.TryGetType(typeCondition, out INamedType type)) + { + ValidateTypeCondition(syntaxNode, type); + } + } + + private void ValidateTypeCondition( + ISyntaxNode syntaxNode, + IType typeCondition) + { + if (typeCondition != null + && !typeCondition.IsCompositeType()) { _fragmentErrors.Add(syntaxNode); } diff --git a/src/Core/Validation/QueryVisitor.cs b/src/Core/Validation/QueryVisitor.cs index 2e59ad96bdc..dd536ad63bc 100644 --- a/src/Core/Validation/QueryVisitor.cs +++ b/src/Core/Validation/QueryVisitor.cs @@ -105,7 +105,7 @@ public void VisitDocument(DocumentNode document) if (selection is InlineFragmentNode inlineFragment) { - VisitInlineFragment(inlineFragment, type, newpath); + VisitInlineFragmentInternal(inlineFragment, type, newpath); } } } @@ -150,15 +150,34 @@ public void VisitDocument(DocumentNode document) VisitDirectives(fragmentSpread.Directives, newpath); } - protected virtual void VisitInlineFragment( + private void VisitInlineFragmentInternal( InlineFragmentNode inlineFragment, IType type, ImmutableStack path) { - if (inlineFragment.TypeCondition?.Name?.Value != null - && Schema.TryGetType( - inlineFragment.TypeCondition.Name.Value, - out INamedOutputType typeCondition)) + if (inlineFragment.TypeCondition?.Name?.Value == null) + { + VisitInlineFragment(inlineFragment, type, type, path); + } + else if (Schema.TryGetType( + inlineFragment.TypeCondition.Name.Value, + out INamedOutputType typeCondition)) + { + VisitInlineFragment(inlineFragment, type, typeCondition, path); + } + else + { + VisitInlineFragment(inlineFragment, type, null, path); + } + } + + protected virtual void VisitInlineFragment( + InlineFragmentNode inlineFragment, + IType parentType, + IType typeCondition, + ImmutableStack path) + { + if (typeCondition != null) { ImmutableStack newpath = path.Push(inlineFragment); From d560b6719dc8580776db054dbc44c888e1e27d85 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 21:46:42 +0200 Subject: [PATCH 16/30] Fixed test schema --- .../Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs | 2 +- src/Core.Tests/Validation/Models/Dog.cs | 2 -- src/Core.Tests/Validation/Types/HumanType.cs | 1 + src/Core/Validation/LeafFieldSelectionsVisitor.cs | 3 ++- src/Core/Validation/QueryVisitor.cs | 6 ++---- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs b/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs index 2444334cd77..fb357915d79 100644 --- a/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs +++ b/src/Core.Tests/Validation/FragmentSpreadsMustNotFormCyclesRuleTests.cs @@ -112,7 +112,7 @@ public void InfiniteRecursion() } } - fragment ownerFragment on Dog { + fragment ownerFragment on Human { name pets { ...dogFragment diff --git a/src/Core.Tests/Validation/Models/Dog.cs b/src/Core.Tests/Validation/Models/Dog.cs index b213e54aec9..003892f82fd 100644 --- a/src/Core.Tests/Validation/Models/Dog.cs +++ b/src/Core.Tests/Validation/Models/Dog.cs @@ -24,6 +24,4 @@ public Human GetOwner() return null; } } - - } diff --git a/src/Core.Tests/Validation/Types/HumanType.cs b/src/Core.Tests/Validation/Types/HumanType.cs index 2092df2eee8..10649b79714 100644 --- a/src/Core.Tests/Validation/Types/HumanType.cs +++ b/src/Core.Tests/Validation/Types/HumanType.cs @@ -9,6 +9,7 @@ protected override void Configure(IObjectTypeDescriptor descriptor) { descriptor.Interface(); descriptor.Field(t => t.Name).Type>(); + descriptor.Field("pets").Type>(); } } } diff --git a/src/Core/Validation/LeafFieldSelectionsVisitor.cs b/src/Core/Validation/LeafFieldSelectionsVisitor.cs index 7cd921e1240..7a91b08c627 100644 --- a/src/Core/Validation/LeafFieldSelectionsVisitor.cs +++ b/src/Core/Validation/LeafFieldSelectionsVisitor.cs @@ -20,7 +20,8 @@ public LeafFieldSelectionsVisitor(ISchema schema) if (type is IComplexOutputType t && t.Fields.TryGetField(field.Name.Value, out IOutputField f)) { - if (f.Type.IsScalarType() || f.Type.IsEnumType()) + if (f.Type.NamedType().IsScalarType() + || f.Type.NamedType().IsEnumType()) { ValidateLeafField(field, f); } diff --git a/src/Core/Validation/QueryVisitor.cs b/src/Core/Validation/QueryVisitor.cs index dd536ad63bc..bd56571ffae 100644 --- a/src/Core/Validation/QueryVisitor.cs +++ b/src/Core/Validation/QueryVisitor.cs @@ -97,13 +97,11 @@ public void VisitDocument(DocumentNode document) { VisitField(field, type, newpath); } - - if (selection is FragmentSpreadNode fragmentSpread) + else if (selection is FragmentSpreadNode fragmentSpread) { VisitFragmentSpread(fragmentSpread, type, newpath); } - - if (selection is InlineFragmentNode inlineFragment) + else if (selection is InlineFragmentNode inlineFragment) { VisitInlineFragmentInternal(inlineFragment, type, newpath); } From 6d2fdafe427ce3b98b6aef12ce8db229436fc949 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 21:50:33 +0200 Subject: [PATCH 17/30] Fixed test schema --- src/Core.Tests/Validation/Types/HumanType.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Core.Tests/Validation/Types/HumanType.cs b/src/Core.Tests/Validation/Types/HumanType.cs index 10649b79714..0f0ffa98fc8 100644 --- a/src/Core.Tests/Validation/Types/HumanType.cs +++ b/src/Core.Tests/Validation/Types/HumanType.cs @@ -9,7 +9,9 @@ protected override void Configure(IObjectTypeDescriptor descriptor) { descriptor.Interface(); descriptor.Field(t => t.Name).Type>(); - descriptor.Field("pets").Type>(); + descriptor.Field("pets") + .Type>() + .Resolver(() => ""); } } } From a992b3205cf52ffb28126d34da53d972152e97cf Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 22:09:33 +0200 Subject: [PATCH 18/30] Added support for inlinefragments without typecondition --- .../StarWarsCodeFirstTests.cs | 2 +- src/Core/Execution/FieldCollector.cs | 2 +- src/Core/Execution/FragmentCollection.cs | 34 +++++++++++++++---- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs b/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs index 2e76bacd5c9..07b4a2e0af3 100644 --- a/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs +++ b/src/Core.Tests/Integration/StarWarsCodeFirst/StarWarsCodeFirstTests.cs @@ -467,7 +467,7 @@ public void ConditionalInlineFragment() Schema schema = CreateSchema(); string query = @" { - heroes(episodes: EMPIRE) { + heroes(episodes: [EMPIRE]) { name ... @include(if: true) { height diff --git a/src/Core/Execution/FieldCollector.cs b/src/Core/Execution/FieldCollector.cs index ea5478869a9..e66058550c0 100644 --- a/src/Core/Execution/FieldCollector.cs +++ b/src/Core/Execution/FieldCollector.cs @@ -96,7 +96,7 @@ internal class FieldCollector } else if (selection is InlineFragmentNode inlineFragment) { - Fragment fragment = _fragments.GetFragment(inlineFragment); + Fragment fragment = _fragments.GetFragment(type, inlineFragment); if (DoesFragmentTypeApply(type, fragment.TypeCondition)) { CollectFields(type, fragment.SelectionSet, reportError, fields); diff --git a/src/Core/Execution/FragmentCollection.cs b/src/Core/Execution/FragmentCollection.cs index ecd2603d5cb..17bab137976 100644 --- a/src/Core/Execution/FragmentCollection.cs +++ b/src/Core/Execution/FragmentCollection.cs @@ -62,34 +62,54 @@ private IEnumerable CreateFragments(string fragmentName) } } - public Fragment GetFragment(InlineFragmentNode inlineFragment) + public Fragment GetFragment( + ObjectType parentType, + InlineFragmentNode inlineFragment) { + if (parentType == null) + { + throw new ArgumentNullException(nameof(parentType)); + } + if (inlineFragment == null) { throw new ArgumentNullException(nameof(inlineFragment)); } string fragmentName = CreateInlineFragmentName(inlineFragment); + if (!_fragments.TryGetValue(fragmentName, out List fragments)) { fragments = new List(); - fragments.Add(CreateFragment(inlineFragment)); + fragments.Add(CreateFragment(parentType, inlineFragment)); _fragments[fragmentName] = fragments; } return fragments.First(); } - private Fragment CreateFragment(InlineFragmentNode inlineFragment) + private Fragment CreateFragment( + ObjectType parentType, + InlineFragmentNode inlineFragment) { - // TODO : maybe introduce a tryget to the schema - INamedType type = _schema.GetType( - inlineFragment.TypeCondition.Name.Value); + INamedType type; + + if (inlineFragment.TypeCondition == null) + { + type = parentType; + } + else + { + type = _schema.GetType( + inlineFragment.TypeCondition.Name.Value); + } + return new Fragment(type, inlineFragment.SelectionSet); } - private string CreateInlineFragmentName(InlineFragmentNode inlineFragment) + private string CreateInlineFragmentName( + InlineFragmentNode inlineFragment) { return $"^__{inlineFragment.Location.Start}_{inlineFragment.Location.End}"; } From 3f2d07aff52054b89cee8b2f1e099fddd75c0a4c Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 22:13:02 +0200 Subject: [PATCH 19/30] Added snapshot --- .../__snapshots__/ConditionalInlineFragment.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/Core.Tests/__snapshots__/ConditionalInlineFragment.json diff --git a/src/Core.Tests/__snapshots__/ConditionalInlineFragment.json b/src/Core.Tests/__snapshots__/ConditionalInlineFragment.json new file mode 100644 index 00000000000..034542c970d --- /dev/null +++ b/src/Core.Tests/__snapshots__/ConditionalInlineFragment.json @@ -0,0 +1,11 @@ +{ + "Data": { + "heroes": [ + { + "name": "Luke Skywalker", + "height": 1.72 + } + ] + }, + "Errors": null +} From f07357503d7585b66f07fc4fc53359816f418c5c Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 23:00:44 +0200 Subject: [PATCH 20/30] [Fact] public void NotOnExistingTypeOnFragment() { // arrange Schema schema = ValidationUtils.CreateSchema(); DocumentNode query = Parser.Default.Parse(@ --- .../FragmentSpreadTypeExistenceRuleTests.cs | 148 ++++++++++++++++++ .../FragmentSpreadTypeExistenceRule.cs | 18 +++ .../FragmentSpreadTypeExistenceVisitor.cs | 61 ++++++++ src/Core/Validation/QueryVisitor.cs | 33 +++- 4 files changed, 256 insertions(+), 4 deletions(-) create mode 100644 src/Core.Tests/Validation/FragmentSpreadTypeExistenceRuleTests.cs create mode 100644 src/Core/Validation/FragmentSpreadTypeExistenceRule.cs create mode 100644 src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs diff --git a/src/Core.Tests/Validation/FragmentSpreadTypeExistenceRuleTests.cs b/src/Core.Tests/Validation/FragmentSpreadTypeExistenceRuleTests.cs new file mode 100644 index 00000000000..af25ae234dd --- /dev/null +++ b/src/Core.Tests/Validation/FragmentSpreadTypeExistenceRuleTests.cs @@ -0,0 +1,148 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class FragmentSpreadTypeExistenceRuleTests + : ValidationTestBase + { + public FragmentSpreadTypeExistenceRuleTests() + : base(new FragmentSpreadTypeExistenceRule()) + { + } + + [Fact] + public void CorrectTypeOnFragment() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...correctType + } + } + + fragment correctType on Dog { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void CorrectTypeOnInlineFragment() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...inlineFragment + } + } + + fragment inlineFragment on Dog { + ... on Dog { + name + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void CorrectTypeOnInlineFragment2() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...inlineFragment2 + } + } + + fragment inlineFragment2 on Dog { + ... @include(if: true) { + name + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void NotOnExistingTypeOnFragment() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...notOnExistingType + } + } + + fragment notOnExistingType on NotInSchema { + name + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The type of fragment `notOnExistingType` " + + "does not exist in the current schema.")); + } + + [Fact] + public void NotExistingTypeOnInlineFragment() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...inlineNotExistingType + } + } + + fragment inlineNotExistingType on Dog { + ... on NotInSchema { + name + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The specified inline fragment " + + "does not exist in the current schema.")); + } + } +} diff --git a/src/Core/Validation/FragmentSpreadTypeExistenceRule.cs b/src/Core/Validation/FragmentSpreadTypeExistenceRule.cs new file mode 100644 index 00000000000..34c725add26 --- /dev/null +++ b/src/Core/Validation/FragmentSpreadTypeExistenceRule.cs @@ -0,0 +1,18 @@ +namespace HotChocolate.Validation +{ + /// + /// Fragments must be specified on types that exist in the schema. + /// This applies for both named and inline fragments. + /// If they are not defined in the schema, the query does not validate. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Fragment-Spread-Type-Existence + /// + internal sealed class FragmentSpreadTypeExistenceRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new FragmentSpreadTypeExistenceVisitor(schema); + } + } +} diff --git a/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs b/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs new file mode 100644 index 00000000000..78aa502b4cb --- /dev/null +++ b/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs @@ -0,0 +1,61 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class FragmentSpreadTypeExistenceVisitor + : QueryVisitorErrorBase + { + private readonly Dictionary> _missingFragments = + new Dictionary>(); + + public FragmentSpreadTypeExistenceVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitFragmentDefinition( + FragmentDefinitionNode fragmentDefinition, + ImmutableStack path) + { + if (!IsFragmentVisited(fragmentDefinition)) + { + if (fragmentDefinition.TypeCondition?.Name?.Value == null + || !Schema.TryGetType( + fragmentDefinition.TypeCondition.Name.Value, + out INamedOutputType typeCondition)) + { + Errors.Add(new ValidationError( + $"The type of fragment `{fragmentDefinition.Name.Value}` " + + "does not exist in the current schema.", + fragmentDefinition)); + } + } + + base.VisitFragmentDefinition(fragmentDefinition, path); + } + + protected override void VisitInlineFragment( + InlineFragmentNode inlineFragment, + IType parentType, + IType typeCondition, + ImmutableStack path) + { + if (typeCondition == null) + { + Errors.Add(new ValidationError( + "The specified inline fragment " + + "does not exist in the current schema.", + inlineFragment)); + } + + base.VisitInlineFragment( + inlineFragment, + parentType, + typeCondition, + path); + } + } +} diff --git a/src/Core/Validation/QueryVisitor.cs b/src/Core/Validation/QueryVisitor.cs index bd56571ffae..296c9becff4 100644 --- a/src/Core/Validation/QueryVisitor.cs +++ b/src/Core/Validation/QueryVisitor.cs @@ -11,8 +11,10 @@ internal class QueryVisitor { private readonly Dictionary _fragments = new Dictionary(); - private readonly HashSet _visitedFragments = - new HashSet(); + private readonly HashSet _visitedFragments = + new HashSet(); + private readonly HashSet _touchedFragments = + new HashSet(); protected QueryVisitor(ISchema schema) { @@ -66,7 +68,9 @@ public void VisitDocument(DocumentNode document) IEnumerable fragmentDefinitions, ImmutableStack path) { - foreach (FragmentDefinitionNode fragment in fragmentDefinitions) + foreach (FragmentDefinitionNode fragment in + fragmentDefinitions.Where( + t => !_touchedFragments.Contains(t))) { VisitFragmentDefinition(fragment, path); } @@ -194,7 +198,7 @@ public void VisitDocument(DocumentNode document) FragmentDefinitionNode fragmentDefinition, ImmutableStack path) { - if (_visitedFragments.Add(fragmentDefinition.Name.Value) + if (MarkFragmentVisited(fragmentDefinition) && fragmentDefinition.TypeCondition?.Name?.Value != null && Schema.TryGetType( fragmentDefinition.TypeCondition.Name.Value, @@ -247,5 +251,26 @@ protected bool ContainsFragment(string fragmentName) { return _fragments.ContainsKey(fragmentName); } + + protected bool IsFragmentVisited(FragmentDefinitionNode fragmentDefinition) + { + if (fragmentDefinition == null) + { + throw new ArgumentNullException(nameof(fragmentDefinition)); + } + + return _visitedFragments.Contains(fragmentDefinition); + } + + protected bool MarkFragmentVisited(FragmentDefinitionNode fragmentDefinition) + { + if (fragmentDefinition == null) + { + throw new ArgumentNullException(nameof(fragmentDefinition)); + } + + _touchedFragments.Add(fragmentDefinition); + return _visitedFragments.Add(fragmentDefinition); + } } } From f04ff066977d85ad0ba104f4c7d3fb99b1876a48 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 23:08:45 +0200 Subject: [PATCH 21/30] Integrated Fragment Spread Type Existence rule into query validator class and added integration test --- .../Validation/QueryValidatorTests.cs | 32 +++++++++++++++++++ src/Core/Validation/QueryValidator.cs | 1 + 2 files changed, 33 insertions(+) diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 30f203fc4ac..63b440c5fcb 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -685,5 +685,37 @@ public void FragmentDoesNotMatchType() "The parent type does not match the type condition on " + "the fragment `fragmentDoesNotMatchType`.")); } + + [Fact] + public void NotExistingTypeOnInlineFragment() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + ...inlineNotExistingType + } + } + + fragment inlineNotExistingType on Dog { + ... on NotInSchema { + name + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal(t.Message, + "The specified inline fragment " + + "does not exist in the current schema.")); + } } } diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index 89582328fc0..bc72bdf032a 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -31,6 +31,7 @@ public class QueryValidator new FragmentSpreadsMustNotFormCyclesRule(), new FragmentSpreadTargetDefinedRule(), new FragmentSpreadIsPossibleRule(), + new FragmentSpreadTypeExistenceRule(), }; private readonly ISchema _schema; From 018183aff7a023ec880c95eef6e1322776c7a489 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 23:19:45 +0200 Subject: [PATCH 22/30] Fixed sonar issues --- .../FragmentSpreadIsPossibleVisitor.cs | 3 --- .../FragmentSpreadTypeExistenceVisitor.cs | 21 +++++++------------ ...FragmentSpreadsMustNotFormCyclesVisitor.cs | 3 ++- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs b/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs index 16fcd8700f5..afe7ac06cd9 100644 --- a/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs +++ b/src/Core/Validation/FragmentSpreadIsPossibleVisitor.cs @@ -9,9 +9,6 @@ namespace HotChocolate.Validation internal sealed class FragmentSpreadIsPossibleVisitor : QueryVisitorErrorBase { - private HashSet _visited = - new HashSet(); - public FragmentSpreadIsPossibleVisitor(ISchema schema) : base(schema) { diff --git a/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs b/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs index 78aa502b4cb..374321e7911 100644 --- a/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs +++ b/src/Core/Validation/FragmentSpreadTypeExistenceVisitor.cs @@ -8,9 +8,6 @@ namespace HotChocolate.Validation internal sealed class FragmentSpreadTypeExistenceVisitor : QueryVisitorErrorBase { - private readonly Dictionary> _missingFragments = - new Dictionary>(); - public FragmentSpreadTypeExistenceVisitor(ISchema schema) : base(schema) { @@ -20,18 +17,16 @@ public FragmentSpreadTypeExistenceVisitor(ISchema schema) FragmentDefinitionNode fragmentDefinition, ImmutableStack path) { - if (!IsFragmentVisited(fragmentDefinition)) - { - if (fragmentDefinition.TypeCondition?.Name?.Value == null + if (!IsFragmentVisited(fragmentDefinition) + && (fragmentDefinition.TypeCondition?.Name?.Value == null || !Schema.TryGetType( fragmentDefinition.TypeCondition.Name.Value, - out INamedOutputType typeCondition)) - { - Errors.Add(new ValidationError( - $"The type of fragment `{fragmentDefinition.Name.Value}` " + - "does not exist in the current schema.", - fragmentDefinition)); - } + out INamedOutputType typeCondition))) + { + Errors.Add(new ValidationError( + $"The type of fragment `{fragmentDefinition.Name.Value}` " + + "does not exist in the current schema.", + fragmentDefinition)); } base.VisitFragmentDefinition(fragmentDefinition, path); diff --git a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs index 73c914a1d40..dec92841631 100644 --- a/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs +++ b/src/Core/Validation/FragmentSpreadsMustNotFormCyclesVisitor.cs @@ -9,7 +9,7 @@ namespace HotChocolate.Validation internal sealed class FragmentSpreadsMustNotFormCyclesVisitor : QueryVisitorErrorBase { - private HashSet _visited = + private readonly HashSet _visited = new HashSet(); private bool _cycleDetected; @@ -38,6 +38,7 @@ public FragmentSpreadsMustNotFormCyclesVisitor(ISchema schema) IEnumerable fragmentDefinitions, ImmutableStack path) { + // we do not want do visit any fragments separately. } protected override void VisitFragmentSpread( From 36717986f825e192182b8c614a7be4d7a420413b Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 25 Aug 2018 23:59:25 +0200 Subject: [PATCH 23/30] Implemented Input Object Field Names rule --- .../InputObjectFieldNamesRuleTests.cs | 79 +++++++++++++++++++ .../Validation/Models/ComplexInput.cs | 2 + .../Validation/QueryValidatorTests.cs | 28 +++++++ .../Validation/InputObjectFieldNamesRule.cs | 17 ++++ .../InputObjectFieldNamesVisitor.cs | 46 +++++++++++ .../Validation/InputObjectFieldVisitorBase.cs | 62 +++++++++++++++ src/Core/Validation/QueryValidator.cs | 1 + 7 files changed, 235 insertions(+) create mode 100644 src/Core.Tests/Validation/InputObjectFieldNamesRuleTests.cs create mode 100644 src/Core/Validation/InputObjectFieldNamesRule.cs create mode 100644 src/Core/Validation/InputObjectFieldNamesVisitor.cs create mode 100644 src/Core/Validation/InputObjectFieldVisitorBase.cs diff --git a/src/Core.Tests/Validation/InputObjectFieldNamesRuleTests.cs b/src/Core.Tests/Validation/InputObjectFieldNamesRuleTests.cs new file mode 100644 index 00000000000..456147649c3 --- /dev/null +++ b/src/Core.Tests/Validation/InputObjectFieldNamesRuleTests.cs @@ -0,0 +1,79 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class InputObjectFieldNamesRuleTests + : ValidationTestBase + { + public InputObjectFieldNamesRuleTests() + : base(new InputObjectFieldNamesRule()) + { + } + + [Fact] + public void AllInputObjectFieldsExist() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { name: ""Fido"" }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + + [Fact] + public void InvalidInputObjectFieldsExist() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { favoriteCookieFlavor: ""Bacon"" }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified input object field " + + "`favoriteCookieFlavor` does not exist.", + t.Message)); + } + + [Fact] + public void InvalidNestedInputObjectFieldsExist() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { child: { favoriteCookieFlavor: ""Bacon"" } }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified input object field " + + "`favoriteCookieFlavor` does not exist.", + t.Message)); + } + } +} diff --git a/src/Core.Tests/Validation/Models/ComplexInput.cs b/src/Core.Tests/Validation/Models/ComplexInput.cs index b6a9b5ef2c6..412c1ac44aa 100644 --- a/src/Core.Tests/Validation/Models/ComplexInput.cs +++ b/src/Core.Tests/Validation/Models/ComplexInput.cs @@ -5,5 +5,7 @@ public class ComplexInput public string Name { get; set; } public string Owner { get; set; } + + public ComplexInput Child { get; set; } } } diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 63b440c5fcb..735dab6bf1a 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -717,5 +717,33 @@ public void NotExistingTypeOnInlineFragment() "The specified inline fragment " + "does not exist in the current schema.")); } + + [Fact] + public void InvalidInputObjectFieldsExist() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { favoriteCookieFlavor: ""Bacon"" }) + { + name + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified input object field " + + "`favoriteCookieFlavor` does not exist.", + t.Message)); + } } } diff --git a/src/Core/Validation/InputObjectFieldNamesRule.cs b/src/Core/Validation/InputObjectFieldNamesRule.cs new file mode 100644 index 00000000000..5a95df82c53 --- /dev/null +++ b/src/Core/Validation/InputObjectFieldNamesRule.cs @@ -0,0 +1,17 @@ +namespace HotChocolate.Validation +{ + /// + /// Every input field provided in an input object value must be defined in + /// the set of possible fields of that input object’s expected type. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Input-Object-Field-Names + /// + internal sealed class InputObjectFieldNamesRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new InputObjectFieldNamesVisitor(schema); + } + } +} diff --git a/src/Core/Validation/InputObjectFieldNamesVisitor.cs b/src/Core/Validation/InputObjectFieldNamesVisitor.cs new file mode 100644 index 00000000000..61d703c0a52 --- /dev/null +++ b/src/Core/Validation/InputObjectFieldNamesVisitor.cs @@ -0,0 +1,46 @@ +using System.Collections.Generic; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class InputObjectFieldNamesVisitor + : InputObjectFieldVisitorBase + { + private HashSet _visited = + new HashSet(); + + public InputObjectFieldNamesVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitObjectValue( + InputObjectType type, + ObjectValueNode objectValue) + { + if (_visited.Add(objectValue)) + { + foreach (ObjectFieldNode fieldValue in objectValue.Fields) + { + if (type.Fields.TryGetField(fieldValue.Name.Value, + out InputField inputField)) + { + if (inputField.Type is InputObjectType inputFieldType + && fieldValue.Value is ObjectValueNode ov) + { + VisitObjectValue(inputFieldType, ov); + } + } + else + { + Errors.Add(new ValidationError( + "The specified input object field " + + $"`{fieldValue.Name.Value}` does not exist.", + fieldValue)); + } + } + } + } + } +} diff --git a/src/Core/Validation/InputObjectFieldVisitorBase.cs b/src/Core/Validation/InputObjectFieldVisitorBase.cs new file mode 100644 index 00000000000..2f27bf036dc --- /dev/null +++ b/src/Core/Validation/InputObjectFieldVisitorBase.cs @@ -0,0 +1,62 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal abstract class InputObjectFieldVisitorBase + : QueryVisitorErrorBase + { + private readonly Dictionary _directives; + + public InputObjectFieldVisitorBase(ISchema schema) + : base(schema) + { + _directives = schema.Directives.ToDictionary(t => t.Name); + } + + protected override void VisitField( + FieldNode field, + IType type, + ImmutableStack path) + { + if (type is IComplexOutputType ct + && ct.Fields.TryGetField(field.Name.Value, out IOutputField f)) + { + VisitArguments(field.Arguments, f.Arguments); + } + } + + protected override void VisitDirective( + DirectiveNode directive, + ImmutableStack path) + { + if (_directives.TryGetValue(directive.Name.Value, out Directive d)) + { + VisitArguments(directive.Arguments, d.Arguments); + } + } + + private void VisitArguments( + IEnumerable arguments, + IFieldCollection argumentFields) + { + foreach (ArgumentNode argument in arguments) + { + if (argument.Value is ObjectValueNode ov + && argumentFields.TryGetField(argument.Name.Value, + out IInputField argumentField) + && argumentField.Type.NamedType() is InputObjectType io) + { + VisitObjectValue(io, ov); + } + } + } + + protected abstract void VisitObjectValue( + InputObjectType type, + ObjectValueNode objectValue); + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index bc72bdf032a..6132abaeaec 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -32,6 +32,7 @@ public class QueryValidator new FragmentSpreadTargetDefinedRule(), new FragmentSpreadIsPossibleRule(), new FragmentSpreadTypeExistenceRule(), + new InputObjectFieldNamesRule(), }; private readonly ISchema _schema; From 067792a4b6e627018f0da8c4b30533718e76aca9 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 00:09:24 +0200 Subject: [PATCH 24/30] Fixed sonar issue --- src/Core/Validation/InputObjectFieldNamesVisitor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Validation/InputObjectFieldNamesVisitor.cs b/src/Core/Validation/InputObjectFieldNamesVisitor.cs index 61d703c0a52..c557ed89156 100644 --- a/src/Core/Validation/InputObjectFieldNamesVisitor.cs +++ b/src/Core/Validation/InputObjectFieldNamesVisitor.cs @@ -7,7 +7,7 @@ namespace HotChocolate.Validation internal sealed class InputObjectFieldNamesVisitor : InputObjectFieldVisitorBase { - private HashSet _visited = + private readonly HashSet _visited = new HashSet(); public InputObjectFieldNamesVisitor(ISchema schema) From 52218582b1944ea238fa042a004da607475dd2c1 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 14:52:10 +0200 Subject: [PATCH 25/30] Implemented InputObject required fields rule --- .../InputObjectRequiredFieldsRuleTests.cs | 116 ++++++++++++++++++ .../Validation/Models/ComplexInput2.cs | 11 ++ src/Core.Tests/Validation/Models/Query.cs | 5 + .../Validation/QueryValidatorTests.cs | 27 ++++ .../Validation/Types/ComplexInput2Type.cs | 19 +++ src/Core.Tests/Validation/ValidationUtils.cs | 1 + .../InputObjectRequiredFieldsRule.cs | 20 +++ .../InputObjectRequiredFieldsVisitor.cs | 76 ++++++++++++ src/Core/Validation/QueryValidator.cs | 1 + 9 files changed, 276 insertions(+) create mode 100644 src/Core.Tests/Validation/InputObjectRequiredFieldsRuleTests.cs create mode 100644 src/Core.Tests/Validation/Models/ComplexInput2.cs create mode 100644 src/Core.Tests/Validation/Types/ComplexInput2Type.cs create mode 100644 src/Core/Validation/InputObjectRequiredFieldsRule.cs create mode 100644 src/Core/Validation/InputObjectRequiredFieldsVisitor.cs diff --git a/src/Core.Tests/Validation/InputObjectRequiredFieldsRuleTests.cs b/src/Core.Tests/Validation/InputObjectRequiredFieldsRuleTests.cs new file mode 100644 index 00000000000..06239187ce2 --- /dev/null +++ b/src/Core.Tests/Validation/InputObjectRequiredFieldsRuleTests.cs @@ -0,0 +1,116 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class InputObjectRequiredFieldsRuleTests + : ValidationTestBase + { + public InputObjectRequiredFieldsRuleTests() + : base(new InputObjectRequiredFieldsRule()) + { + } + + [Fact] + public void RequiredFieldsHaveValidValue() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog2(complex: { name: ""Foo"" }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void NestedRequiredFieldsHaveValidValue() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog2(complex: { name: ""Foo"" child: { name: ""123"" } }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void RequiredFieldIsNull() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog2(complex: { name: null }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "`name` is a required field and cannot be null.", + t.Message)); + } + + [Fact] + public void RequiredFieldIsNotSet() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog2(complex: { }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "`name` is a required field and cannot be null.", + t.Message)); + } + + [Fact] + public void NestedRequiredFieldIsNotSet() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog2(complex: { name: ""foo"" child: { owner: ""bar"" } }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "`name` is a required field and cannot be null.", + t.Message)); + } + } +} diff --git a/src/Core.Tests/Validation/Models/ComplexInput2.cs b/src/Core.Tests/Validation/Models/ComplexInput2.cs new file mode 100644 index 00000000000..f506d17db91 --- /dev/null +++ b/src/Core.Tests/Validation/Models/ComplexInput2.cs @@ -0,0 +1,11 @@ +namespace HotChocolate.Validation +{ + public class ComplexInput2 + { + public string Name { get; set; } + + public string Owner { get; set; } + + public ComplexInput2 Child { get; set; } + } +} diff --git a/src/Core.Tests/Validation/Models/Query.cs b/src/Core.Tests/Validation/Models/Query.cs index d31903a8a2d..00c644138dd 100644 --- a/src/Core.Tests/Validation/Models/Query.cs +++ b/src/Core.Tests/Validation/Models/Query.cs @@ -12,6 +12,11 @@ public Dog FindDog(ComplexInput complex) return null; } + public Dog FindDog2(ComplexInput2 complex) + { + return null; + } + public bool BooleanList(bool[] booleanListArg) { return true; diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 735dab6bf1a..03322e6f137 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -745,5 +745,32 @@ public void InvalidInputObjectFieldsExist() "`favoriteCookieFlavor` does not exist.", t.Message)); } + + [Fact] + public void RequiredFieldIsNull() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + findDog2(complex: { name: null }) + { + name + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "`name` is a required field and cannot be null.", + t.Message)); + } } } diff --git a/src/Core.Tests/Validation/Types/ComplexInput2Type.cs b/src/Core.Tests/Validation/Types/ComplexInput2Type.cs new file mode 100644 index 00000000000..df71cf9fffa --- /dev/null +++ b/src/Core.Tests/Validation/Types/ComplexInput2Type.cs @@ -0,0 +1,19 @@ +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + public class ComplexInput2Type + : InputObjectType + { + protected override void Configure( + IInputObjectTypeDescriptor descriptor) + { + descriptor.Field(t => t.Name) + .Type>(); + descriptor.Field(t => t.Owner) + .Type>() + .DefaultValue(new StringValueNode("1234")); + } + } +} diff --git a/src/Core.Tests/Validation/ValidationUtils.cs b/src/Core.Tests/Validation/ValidationUtils.cs index 4ca9327703d..e9115886a44 100644 --- a/src/Core.Tests/Validation/ValidationUtils.cs +++ b/src/Core.Tests/Validation/ValidationUtils.cs @@ -18,6 +18,7 @@ public static Schema CreateSchema() c.RegisterType(); c.RegisterSubscriptionType(); c.RegisterType(); + c.RegisterType(); }); } } diff --git a/src/Core/Validation/InputObjectRequiredFieldsRule.cs b/src/Core/Validation/InputObjectRequiredFieldsRule.cs new file mode 100644 index 00000000000..96b53086b77 --- /dev/null +++ b/src/Core/Validation/InputObjectRequiredFieldsRule.cs @@ -0,0 +1,20 @@ +namespace HotChocolate.Validation +{ + /// + /// Input object fields may be required. Much like a field may have + /// required arguments, an input object may have required fields. + /// + /// An input field is required if it has a non‐null type and does not have + /// a default value. Otherwise, the input object field is optional. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Input-Object-Required-Fields + /// + internal sealed class InputObjectRequiredFieldsRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new InputObjectRequiredFieldsVisitor(schema); + } + } +} diff --git a/src/Core/Validation/InputObjectRequiredFieldsVisitor.cs b/src/Core/Validation/InputObjectRequiredFieldsVisitor.cs new file mode 100644 index 00000000000..438260514f6 --- /dev/null +++ b/src/Core/Validation/InputObjectRequiredFieldsVisitor.cs @@ -0,0 +1,76 @@ + +using System.Collections.Generic; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class InputObjectRequiredFieldsVisitor + : InputObjectFieldVisitorBase + { + private readonly HashSet _visited = + new HashSet(); + + public InputObjectRequiredFieldsVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitObjectValue( + InputObjectType type, + ObjectValueNode objectValue) + { + if (_visited.Add(objectValue)) + { + Dictionary fieldValues = + CreateFieldMap(objectValue); + + foreach (InputField field in type.Fields) + { + fieldValues.TryGetValue(field.Name, + out ObjectFieldNode fieldValue); + + ValidateInputField(field, fieldValue, + (ISyntaxNode)fieldValue ?? objectValue); + + if (fieldValue?.Value is ObjectValueNode ov + && field.Type.NamedType() is InputObjectType it) + { + VisitObjectValue(it, ov); + } + } + } + } + + private void ValidateInputField( + InputField field, + ObjectFieldNode fieldValue, + ISyntaxNode node) + { + if (field.Type.IsNonNullType() + && field.DefaultValue.IsNull() + && ValueNodeExtensions.IsNull(fieldValue?.Value)) + { + Errors.Add(new ValidationError( + $"`{field.Name}` is a required field and cannot be null.", + node)); + } + } + + private Dictionary CreateFieldMap( + ObjectValueNode objectValue) + { + var fields = new Dictionary(); + + foreach (ObjectFieldNode fieldValue in objectValue.Fields) + { + if (!fields.ContainsKey(fieldValue.Name.Value)) + { + fields[fieldValue.Name.Value] = fieldValue; + } + } + + return fields; + } + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index 6132abaeaec..c8cea127a3d 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -33,6 +33,7 @@ public class QueryValidator new FragmentSpreadIsPossibleRule(), new FragmentSpreadTypeExistenceRule(), new InputObjectFieldNamesRule(), + new InputObjectRequiredFieldsRule(), }; private readonly ISchema _schema; From 19fcc06194b7e01eba6566dad079944db7178416 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 17:58:18 +0200 Subject: [PATCH 26/30] Added input object field uniqueness rule --- .../InputObjectFieldUniquenessRuleTests.cs | 55 ++++++++++++++ .../Validation/QueryValidatorTests.cs | 27 +++++++ .../InputObjectFieldUniquenessRule.cs | 18 +++++ .../InputObjectFieldUniquenessVisitor.cs | 75 +++++++++++++++++++ src/Core/Validation/QueryValidator.cs | 1 + 5 files changed, 176 insertions(+) create mode 100644 src/Core.Tests/Validation/InputObjectFieldUniquenessRuleTests.cs create mode 100644 src/Core/Validation/InputObjectFieldUniquenessRule.cs create mode 100644 src/Core/Validation/InputObjectFieldUniquenessVisitor.cs diff --git a/src/Core.Tests/Validation/InputObjectFieldUniquenessRuleTests.cs b/src/Core.Tests/Validation/InputObjectFieldUniquenessRuleTests.cs new file mode 100644 index 00000000000..c59e4e632a4 --- /dev/null +++ b/src/Core.Tests/Validation/InputObjectFieldUniquenessRuleTests.cs @@ -0,0 +1,55 @@ + +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class InputObjectFieldUniquenessRuleTests + : ValidationTestBase + { + public InputObjectFieldUniquenessRuleTests() + : base(new InputObjectFieldUniquenessRule()) + { + } + + [Fact] + public void NoFieldAmbiguity() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { name: ""A"", owner: ""B"" }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void NameFieldIsAmbiguous() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { name: ""A"", name: ""B"" }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "Field `name` is ambiguous.", + t.Message)); + } + } +} diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 03322e6f137..6fd21fccb83 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -772,5 +772,32 @@ public void RequiredFieldIsNull() "`name` is a required field and cannot be null.", t.Message)); } + + [Fact] + public void NameFieldIsAmbiguous() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + findDog(complex: { name: ""A"", name: ""B"" }) + { + name + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "Field `name` is ambiguous.", + t.Message)); + } } } diff --git a/src/Core/Validation/InputObjectFieldUniquenessRule.cs b/src/Core/Validation/InputObjectFieldUniquenessRule.cs new file mode 100644 index 00000000000..a9b7f650fdd --- /dev/null +++ b/src/Core/Validation/InputObjectFieldUniquenessRule.cs @@ -0,0 +1,18 @@ +namespace HotChocolate.Validation +{ + /// + /// Input objects must not contain more than one field of the same name, + /// otherwise an ambiguity would exist which includes an ignored portion + /// of syntax. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Input-Object-Field-Uniqueness + /// + internal sealed class InputObjectFieldUniquenessRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new InputObjectFieldUniquenessVisitor(schema); + } + } +} diff --git a/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs b/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs new file mode 100644 index 00000000000..013a60237b1 --- /dev/null +++ b/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs @@ -0,0 +1,75 @@ + +using System.Collections.Generic; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class InputObjectFieldUniquenessVisitor + : InputObjectFieldVisitorBase + { + private readonly HashSet _visited = + new HashSet(); + + public InputObjectFieldUniquenessVisitor(ISchema schema) + : base(schema) + { + } + + protected override void VisitObjectValue( + InputObjectType type, + ObjectValueNode objectValue) + { + var visitedFields = new HashSet(); + + if (_visited.Add(objectValue)) + { + foreach (ObjectFieldNode fieldValue in objectValue.Fields) + { + if (type.Fields.TryGetField(fieldValue.Name.Value, + out InputField field)) + { + VisitInputField(visitedFields, field, fieldValue); + } + } + } + } + + private void VisitInputField( + HashSet visitedFields, + InputField field, + ObjectFieldNode fieldValue) + { + if (visitedFields.Add(field.Name)) + { + if (fieldValue.Value is ObjectValueNode ov + && field.Type.NamedType() is InputObjectType it) + { + VisitObjectValue(it, ov); + } + } + else + { + Errors.Add(new ValidationError( + $"Field `{field.Name}` is ambiguous.", + fieldValue)); + } + } + + private Dictionary CreateFieldMap( + ObjectValueNode objectValue) + { + var fields = new Dictionary(); + + foreach (ObjectFieldNode fieldValue in objectValue.Fields) + { + if (!fields.ContainsKey(fieldValue.Name.Value)) + { + fields[fieldValue.Name.Value] = fieldValue; + } + } + + return fields; + } + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index c8cea127a3d..735b927340f 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -34,6 +34,7 @@ public class QueryValidator new FragmentSpreadTypeExistenceRule(), new InputObjectFieldNamesRule(), new InputObjectRequiredFieldsRule(), + new InputObjectFieldUniquenessRule(), }; private readonly ISchema _schema; From e2f77b6d991d95622d667f48e637853970e63b78 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 18:15:07 +0200 Subject: [PATCH 27/30] Implemented directives are defined rule --- .../DirectivesAreDefinedRuleTests.cs | 59 +++++++++++++++++++ .../Validation/QueryValidatorTests.cs | 27 +++++++++ .../Validation/DirectivesAreDefinedRule.cs | 20 +++++++ .../Validation/DirectivesAreDefinedVisitor.cs | 33 +++++++++++ src/Core/Validation/QueryValidator.cs | 1 + 5 files changed, 140 insertions(+) create mode 100644 src/Core.Tests/Validation/DirectivesAreDefinedRuleTests.cs create mode 100644 src/Core/Validation/DirectivesAreDefinedRule.cs create mode 100644 src/Core/Validation/DirectivesAreDefinedVisitor.cs diff --git a/src/Core.Tests/Validation/DirectivesAreDefinedRuleTests.cs b/src/Core.Tests/Validation/DirectivesAreDefinedRuleTests.cs new file mode 100644 index 00000000000..7d37d90c890 --- /dev/null +++ b/src/Core.Tests/Validation/DirectivesAreDefinedRuleTests.cs @@ -0,0 +1,59 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class DirectivesAreDefinedRuleTests + : ValidationTestBase + { + public DirectivesAreDefinedRuleTests() + : base(new DirectivesAreDefinedRule()) + { + } + + [Fact] + public void SupportedDirective() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + name @skip(if: true) + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void UnsupportedDirective() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + dog { + name @foo(bar: true) + } + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified directive `foo` " + + "is not supported by the current schema.", + t.Message)); + } + } +} diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index 6fd21fccb83..c2d751f4168 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -799,5 +799,32 @@ public void NameFieldIsAmbiguous() "Field `name` is ambiguous.", t.Message)); } + + [Fact] + public void UnsupportedDirective() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + dog { + name @foo(bar: true) + } + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified directive `foo` " + + "is not supported by the current schema.", + t.Message)); + } } } diff --git a/src/Core/Validation/DirectivesAreDefinedRule.cs b/src/Core/Validation/DirectivesAreDefinedRule.cs new file mode 100644 index 00000000000..5aab40e070a --- /dev/null +++ b/src/Core/Validation/DirectivesAreDefinedRule.cs @@ -0,0 +1,20 @@ +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + /// + /// GraphQL servers define what directives they support. + /// For each usage of a directive, the directive must be available + /// on that server. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Directives-Are-Defined + /// + internal sealed class DirectivesAreDefinedRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new DirectivesAreDefinedVisitor(schema); + } + } +} diff --git a/src/Core/Validation/DirectivesAreDefinedVisitor.cs b/src/Core/Validation/DirectivesAreDefinedVisitor.cs new file mode 100644 index 00000000000..b632e4e1fe2 --- /dev/null +++ b/src/Core/Validation/DirectivesAreDefinedVisitor.cs @@ -0,0 +1,33 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using HotChocolate.Language; + +namespace HotChocolate.Validation +{ + internal sealed class DirectivesAreDefinedVisitor + : QueryVisitorErrorBase + { + private readonly HashSet _directives; + + public DirectivesAreDefinedVisitor(ISchema schema) + : base(schema) + { + _directives = new HashSet( + schema.Directives.Select(t => t.Name)); + } + + protected override void VisitDirective( + DirectiveNode directive, + ImmutableStack path) + { + if (!_directives.Contains(directive.Name.Value)) + { + Errors.Add(new ValidationError( + $"The specified directive `{directive.Name.Value}` " + + "is not supported by the current schema.", + directive)); + } + } + } +} diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index 735b927340f..4ad779efb7f 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -35,6 +35,7 @@ public class QueryValidator new InputObjectFieldNamesRule(), new InputObjectRequiredFieldsRule(), new InputObjectFieldUniquenessRule(), + new DirectivesAreDefinedRule(), }; private readonly ISchema _schema; From b1c3f55b848f2b0450b8a49d6a97f626367e79ff Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 20:49:52 +0200 Subject: [PATCH 28/30] Implemented values of correct type rule --- .../Validation/QueryValidatorTests.cs | 31 +++ .../ValuesOfCorrectTypeRuleTests.cs | 179 ++++++++++++++++++ .../InputObjectFieldUniquenessVisitor.cs | 16 -- .../Validation/InputObjectFieldVisitorBase.cs | 2 +- src/Core/Validation/QueryValidator.cs | 1 + .../Validation/ValuesOfCorrectTypeRule.cs | 18 ++ .../Validation/ValuesOfCorrectTypeRule.temp | 82 -------- .../Validation/ValuesOfCorrectTypeVisitor.cs | 161 ++++++++++++++++ 8 files changed, 391 insertions(+), 99 deletions(-) create mode 100644 src/Core.Tests/Validation/ValuesOfCorrectTypeRuleTests.cs create mode 100644 src/Core/Validation/ValuesOfCorrectTypeRule.cs delete mode 100644 src/Core/Validation/ValuesOfCorrectTypeRule.temp create mode 100644 src/Core/Validation/ValuesOfCorrectTypeVisitor.cs diff --git a/src/Core.Tests/Validation/QueryValidatorTests.cs b/src/Core.Tests/Validation/QueryValidatorTests.cs index c2d751f4168..fa48c961d99 100644 --- a/src/Core.Tests/Validation/QueryValidatorTests.cs +++ b/src/Core.Tests/Validation/QueryValidatorTests.cs @@ -826,5 +826,36 @@ name @foo(bar: true) "is not supported by the current schema.", t.Message)); } + + [Fact] + public void StringIntoInt() + { + // arrange + DocumentNode query = Parser.Default.Parse(@" + { + arguments { + ...stringIntoInt + } + } + + fragment stringIntoInt on Arguments { + intArgField(intArg: ""123"") + } + "); + + Schema schema = ValidationUtils.CreateSchema(); + var queryValidator = new QueryValidator(schema); + + // act + QueryValidationResult result = queryValidator.Validate(query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified value type of argument `intArg` " + + "does not match the argument type.", + t.Message)); + } } } diff --git a/src/Core.Tests/Validation/ValuesOfCorrectTypeRuleTests.cs b/src/Core.Tests/Validation/ValuesOfCorrectTypeRuleTests.cs new file mode 100644 index 00000000000..492bf7329c1 --- /dev/null +++ b/src/Core.Tests/Validation/ValuesOfCorrectTypeRuleTests.cs @@ -0,0 +1,179 @@ +using HotChocolate.Language; +using Xunit; + +namespace HotChocolate.Validation +{ + public class ValuesOfCorrectTypeRuleTests + : ValidationTestBase + { + public ValuesOfCorrectTypeRuleTests() + : base(new ValuesOfCorrectTypeRule()) + { + } + + [Fact] + public void GoodBooleanArg() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + arguments { + ...goodBooleanArg + } + } + + fragment goodBooleanArg on Arguments { + booleanArgField(booleanArg: true) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void CoercedIntIntoFloatArg() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + arguments { + ...coercedIntIntoFloatArg + } + } + + fragment coercedIntIntoFloatArg on Arguments { + # Note: The input coercion rules for Float allow Int literals. + floatArgField(floatArg: 123) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void GoodComplexDefaultValue() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query goodComplexDefaultValue($search: ComplexInput = { name: ""Fido"" }) { + findDog(complex: $search) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.False(result.HasErrors); + } + + [Fact] + public void StringIntoInt() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + { + arguments { + ...stringIntoInt + } + } + + fragment stringIntoInt on Arguments { + intArgField(intArg: ""123"") + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified value type of argument `intArg` " + + "does not match the argument type.", + t.Message)); + } + + [Fact] + public void BadComplexValueArgument() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query badComplexValue { + findDog(complex: { name: 123 }) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified value type of field `name` " + + "does not match the field type.", + t.Message)); + } + + [Fact] + public void BadComplexValueVariable() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query goodComplexDefaultValue($search: ComplexInput = { name: 123 }) { + findDog(complex: $search) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified value type of field `name` " + + "does not match the field type.", + t.Message)); + } + + [Fact] + public void BadValueVariable() + { + // arrange + Schema schema = ValidationUtils.CreateSchema(); + DocumentNode query = Parser.Default.Parse(@" + query goodComplexDefaultValue($search: ComplexInput = 123) { + findDog(complex: $search) + } + "); + + // act + QueryValidationResult result = Rule.Validate(schema, query); + + // assert + Assert.True(result.HasErrors); + Assert.Collection(result.Errors, + t => Assert.Equal( + "The specified value type of variable `search` " + + "does not match the variable type.", + t.Message)); + } + } +} diff --git a/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs b/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs index 013a60237b1..349083a8114 100644 --- a/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs +++ b/src/Core/Validation/InputObjectFieldUniquenessVisitor.cs @@ -55,21 +55,5 @@ public InputObjectFieldUniquenessVisitor(ISchema schema) fieldValue)); } } - - private Dictionary CreateFieldMap( - ObjectValueNode objectValue) - { - var fields = new Dictionary(); - - foreach (ObjectFieldNode fieldValue in objectValue.Fields) - { - if (!fields.ContainsKey(fieldValue.Name.Value)) - { - fields[fieldValue.Name.Value] = fieldValue; - } - } - - return fields; - } } } diff --git a/src/Core/Validation/InputObjectFieldVisitorBase.cs b/src/Core/Validation/InputObjectFieldVisitorBase.cs index 2f27bf036dc..881825d8d30 100644 --- a/src/Core/Validation/InputObjectFieldVisitorBase.cs +++ b/src/Core/Validation/InputObjectFieldVisitorBase.cs @@ -11,7 +11,7 @@ internal abstract class InputObjectFieldVisitorBase { private readonly Dictionary _directives; - public InputObjectFieldVisitorBase(ISchema schema) + protected InputObjectFieldVisitorBase(ISchema schema) : base(schema) { _directives = schema.Directives.ToDictionary(t => t.Name); diff --git a/src/Core/Validation/QueryValidator.cs b/src/Core/Validation/QueryValidator.cs index 4ad779efb7f..3939a1ec686 100644 --- a/src/Core/Validation/QueryValidator.cs +++ b/src/Core/Validation/QueryValidator.cs @@ -36,6 +36,7 @@ public class QueryValidator new InputObjectRequiredFieldsRule(), new InputObjectFieldUniquenessRule(), new DirectivesAreDefinedRule(), + new ValuesOfCorrectTypeRule() }; private readonly ISchema _schema; diff --git a/src/Core/Validation/ValuesOfCorrectTypeRule.cs b/src/Core/Validation/ValuesOfCorrectTypeRule.cs new file mode 100644 index 00000000000..766b1ad0d3a --- /dev/null +++ b/src/Core/Validation/ValuesOfCorrectTypeRule.cs @@ -0,0 +1,18 @@ +namespace HotChocolate.Validation +{ + /// + /// Literal values must be compatible with the type expected in the position + /// they are found as per the coercion rules defined in the Type System + /// chapter. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Values-of-Correct-Type + /// + internal sealed class ValuesOfCorrectTypeRule + : QueryVisitorValidationErrorBase + { + protected override QueryVisitorErrorBase CreateVisitor(ISchema schema) + { + return new ValuesOfCorrectTypeVisitor(schema); + } + } +} diff --git a/src/Core/Validation/ValuesOfCorrectTypeRule.temp b/src/Core/Validation/ValuesOfCorrectTypeRule.temp deleted file mode 100644 index 192c3d83ac9..00000000000 --- a/src/Core/Validation/ValuesOfCorrectTypeRule.temp +++ /dev/null @@ -1,82 +0,0 @@ -using System.Collections.Immutable; -using System.Linq; -using HotChocolate.Language; -using HotChocolate.Types; - -namespace HotChocolate.Validation -{ - internal sealed class ValuesOfCorrectTypeVisitor - : QueryVisitorErrorBase - { - public ValuesOfCorrectTypeVisitor(ISchema schema) - : base(schema) - { - } - - protected override void VisitOperationDefinition( - OperationDefinitionNode operation, - ImmutableStack path) - { - - } - - protected override void VisitField( - FieldNode field, - IType type, - ImmutableStack path) - { - IOutputField f; - f.Arguments - } - - protected override void VisitDirective( - DirectiveNode directive, - ImmutableStack path) - { - - } - - private void VisitArgument( - ISyntaxNode node, - IFieldCollection arguments, - string argumentName, - IValueNode argumentValue, - ImmutableStack path) - { - if (arguments.TryGetField(argumentName, out IInputField argument)) - { - if (argumentValue is VariableNode) - { - - } - else if (!argument.Type.IsInstanceOfType(argumentValue)) - { - Errors.Add(new ValidationError("", node)); - } - } - } - - private IType GetVariableType(string variableName, ImmutableStack path) - { - ImmutableStack current = path; - - while (current.Any()) - { - current = current.Pop(out ISyntaxNode node); - if (node is OperationDefinitionNode operation) - { - VariableDefinitionNode variable = operation - .VariableDefinitions.FirstOrDefault( - t => t.Variable.Name.Value == variableName); - if (variable != null) - { - string typeName = variable.Type.NamedType().Name.Value; - return Schema.GetType(typeName); - } - } - } - - return null; - } - } -} diff --git a/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs b/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs new file mode 100644 index 00000000000..06993cccf93 --- /dev/null +++ b/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs @@ -0,0 +1,161 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using HotChocolate.Language; +using HotChocolate.Types; + +namespace HotChocolate.Validation +{ + internal sealed class ValuesOfCorrectTypeVisitor + : InputObjectFieldVisitorBase + { + private readonly HashSet _visited = + new HashSet(); + private readonly Dictionary _directives; + + public ValuesOfCorrectTypeVisitor(ISchema schema) + : base(schema) + { + _directives = schema.Directives.ToDictionary(t => t.Name); + } + + protected override void VisitOperationDefinition( + OperationDefinitionNode operation, + ImmutableStack path) + { + foreach (VariableDefinitionNode variableDefinition in + operation.VariableDefinitions) + { + if (!variableDefinition.DefaultValue.IsNull()) + { + IType type = ConvertTypeNodeToType(variableDefinition.Type); + IValueNode defaultValue = variableDefinition.DefaultValue; + + if (type is IInputType inputType + && !inputType.IsInstanceOfType(defaultValue)) + { + Errors.Add(new ValidationError( + "The specified value type of variable " + + $"`{variableDefinition.Variable.Name.Value}` " + + "does not match the variable type.", + variableDefinition)); + } + else if (defaultValue is ObjectValueNode ov + && type is InputObjectType iot) + { + VisitObjectValue(iot, ov); + } + } + } + + base.VisitOperationDefinition(operation, path); + } + + protected override void VisitField( + FieldNode field, + IType type, + ImmutableStack path) + { + if (type is IComplexOutputType ct + && ct.Fields.TryGetField(field.Name.Value, out IOutputField f)) + { + foreach (ArgumentNode argument in field.Arguments) + { + VisitArgument(f.Arguments, argument, path); + } + } + + base.VisitField(field, type, path); + } + + protected override void VisitDirective( + DirectiveNode directive, + ImmutableStack path) + { + if (_directives.TryGetValue( + directive.Name.Value, + out Directive d)) + { + foreach (ArgumentNode argument in directive.Arguments) + { + VisitArgument(d.Arguments, argument, path); + } + } + + base.VisitDirective(directive, path); + } + + protected override void VisitObjectValue( + InputObjectType type, + ObjectValueNode objectValue) + { + if (_visited.Add(objectValue)) + { + foreach (ObjectFieldNode fieldValue in objectValue.Fields) + { + if (type.Fields.TryGetField(fieldValue.Name.Value, + out InputField field)) + { + if (field.Type.IsInstanceOfType(fieldValue.Value)) + { + if (fieldValue.Value is ObjectValueNode ov + && field.Type.NamedType() is InputObjectType it) + { + VisitObjectValue(type, objectValue); + } + } + else + { + Errors.Add(new ValidationError( + "The specified value type of field " + + $"`{fieldValue.Name.Value}` " + + "does not match the field type.", + fieldValue)); + } + } + } + } + } + + private void VisitArgument( + IFieldCollection argumentFields, + ArgumentNode argument, + ImmutableStack path) + { + if (argumentFields.TryGetField(argument.Name.Value, + out IInputField argumentField)) + { + if (!(argument.Value is VariableNode) + && !argumentField.Type.IsInstanceOfType(argument.Value)) + { + Errors.Add(new ValidationError( + "The specified value type of argument " + + $"`{argument.Name.Value}` " + + "does not match the argument type.", + argument)); + } + } + } + + private IType ConvertTypeNodeToType(ITypeNode typeNode) + { + if (typeNode is NonNullTypeNode nntn) + { + return new NonNullType(ConvertTypeNodeToType(nntn.Type)); + } + + if (typeNode is ListTypeNode ltn) + { + return new ListType(ConvertTypeNodeToType(ltn.Type)); + } + + if (typeNode is NamedTypeNode ntn) + { + return Schema.GetType(ntn.Name.Value); + } + + throw new NotSupportedException(); + } + } +} From 499009f32d43912e00e12a91a8cfd7259bc183ee Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 21:19:35 +0200 Subject: [PATCH 29/30] Fixed float type --- src/Core.Tests/Types/FloatTypeTests.cs | 31 ++++++++++++++++- .../Validation/ValuesOfCorrectTypeVisitor.cs | 16 +++++++-- src/Types/Types/Scalars/FloatType.cs | 33 +++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/Core.Tests/Types/FloatTypeTests.cs b/src/Core.Tests/Types/FloatTypeTests.cs index 5fda20bc427..7a77ad29347 100644 --- a/src/Core.Tests/Types/FloatTypeTests.cs +++ b/src/Core.Tests/Types/FloatTypeTests.cs @@ -11,7 +11,7 @@ public class FloatTypeTests new FloatValueNode("1.000000E+000"); protected override IValueNode GetWrongValueNode => - new IntValueNode("1"); + new StringValueNode("1"); protected override double GetValue => 1.0d; @@ -54,5 +54,34 @@ public void ParseValue_Float_Min() // assert Assert.Equal("-3.402823E+038", literal.Value); } + + [Fact] + public void IsInstanceOfType_IntValueNode() + { + // arrange + FloatType type = new FloatType(); + IntValueNode input = new IntValueNode("123"); + + // act + bool result = type.IsInstanceOfType(input); + + // assert + Assert.True(result); + } + + [Fact] + public void ParseLiteral_IntValueNode() + { + // arrange + FloatType type = new FloatType(); + IntValueNode input = new IntValueNode("123"); + + // act + object result = type.ParseLiteral(input); + + // assert + Assert.IsType(result); + Assert.Equal(123d, result); + } } } diff --git a/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs b/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs index 06993cccf93..ac02cbc7d08 100644 --- a/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs +++ b/src/Core/Validation/ValuesOfCorrectTypeVisitor.cs @@ -33,7 +33,7 @@ public ValuesOfCorrectTypeVisitor(ISchema schema) IValueNode defaultValue = variableDefinition.DefaultValue; if (type is IInputType inputType - && !inputType.IsInstanceOfType(defaultValue)) + && !IsInstanceOfType(inputType, defaultValue)) { Errors.Add(new ValidationError( "The specified value type of variable " + @@ -97,7 +97,7 @@ public ValuesOfCorrectTypeVisitor(ISchema schema) if (type.Fields.TryGetField(fieldValue.Name.Value, out InputField field)) { - if (field.Type.IsInstanceOfType(fieldValue.Value)) + if (IsInstanceOfType(field.Type, fieldValue.Value)) { if (fieldValue.Value is ObjectValueNode ov && field.Type.NamedType() is InputObjectType it) @@ -127,7 +127,7 @@ public ValuesOfCorrectTypeVisitor(ISchema schema) out IInputField argumentField)) { if (!(argument.Value is VariableNode) - && !argumentField.Type.IsInstanceOfType(argument.Value)) + && !IsInstanceOfType(argumentField.Type, argument.Value)) { Errors.Add(new ValidationError( "The specified value type of argument " + @@ -157,5 +157,15 @@ private IType ConvertTypeNodeToType(ITypeNode typeNode) throw new NotSupportedException(); } + + private bool IsInstanceOfType(IInputType inputType, IValueNode value) + { + IInputType internalType = inputType; + if (inputType.IsNonNullType()) + { + internalType = (IInputType)inputType.InnerType(); + } + return internalType.IsInstanceOfType(value); + } } } diff --git a/src/Types/Types/Scalars/FloatType.cs b/src/Types/Types/Scalars/FloatType.cs index c3a44843db3..7287b6a186f 100644 --- a/src/Types/Types/Scalars/FloatType.cs +++ b/src/Types/Types/Scalars/FloatType.cs @@ -13,6 +13,39 @@ public FloatType() { } + public override bool IsInstanceOfType(IValueNode literal) + { + if (literal == null) + { + throw new ArgumentNullException(nameof(literal)); + } + + // Input coercion rules specify that float values can be coerced + // from IntValueNode and FloatValueNode: + // http://facebook.github.io/graphql/June2018/#sec-Float + return base.IsInstanceOfType(literal) || literal is IntValueNode; + } + + public override object ParseLiteral(IValueNode literal) + { + if (literal == null) + { + throw new ArgumentNullException(nameof(literal)); + } + + // Input coercion rules specify that float values can be coerced + // from IntValueNode and FloatValueNode: + // http://facebook.github.io/graphql/June2018/#sec-Float + if (literal is IntValueNode node) + { + return double.Parse(node.Value, + NumberStyles.Float, + CultureInfo.InvariantCulture); + } + + return base.ParseLiteral(literal); + } + protected override double OnParseLiteral(FloatValueNode node) => double.Parse(node.Value, NumberStyles.Float, CultureInfo.InvariantCulture); From df9b6a383cf57cb140f003bb098ef7832e7835d6 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sun, 26 Aug 2018 21:28:38 +0200 Subject: [PATCH 30/30] Added documentation --- src/Types/Types/Scalars/BooleanType.cs | 2 ++ src/Types/Types/Scalars/FloatType.cs | 8 ++++++++ src/Types/Types/Scalars/IdType.cs | 9 +++++++++ src/Types/Types/Scalars/IntType.cs | 7 +++++++ src/Types/Types/Scalars/StringType.cs | 6 ++++-- 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/Types/Types/Scalars/BooleanType.cs b/src/Types/Types/Scalars/BooleanType.cs index 7076e883618..f3351cabcab 100644 --- a/src/Types/Types/Scalars/BooleanType.cs +++ b/src/Types/Types/Scalars/BooleanType.cs @@ -7,6 +7,8 @@ namespace HotChocolate.Types /// The Boolean scalar type represents true or false. /// Response formats should use a built‐in boolean type if supported; /// otherwise, they should use their representation of the integers 1 and 0. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Boolean /// public sealed class BooleanType : ScalarType diff --git a/src/Types/Types/Scalars/FloatType.cs b/src/Types/Types/Scalars/FloatType.cs index 7287b6a186f..bf3c44f73fa 100644 --- a/src/Types/Types/Scalars/FloatType.cs +++ b/src/Types/Types/Scalars/FloatType.cs @@ -5,6 +5,14 @@ namespace HotChocolate.Types { + /// + /// The Float scalar type represents signed double‐precision fractional + /// values as specified by IEEE 754. Response formats that support an + /// appropriate double‐precision number type should use that type to + /// represent this scalar. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Float + /// public sealed class FloatType : NumberType { diff --git a/src/Types/Types/Scalars/IdType.cs b/src/Types/Types/Scalars/IdType.cs index f52e5f7ba94..e1a2470ac25 100644 --- a/src/Types/Types/Scalars/IdType.cs +++ b/src/Types/Types/Scalars/IdType.cs @@ -3,6 +3,15 @@ namespace HotChocolate.Types { + /// + /// The ID scalar type represents a unique identifier, often used to refetch + /// an object or as the key for a cache. The ID type is serialized in the + /// same way as a String; however, it is not intended to be human‐readable. + /// + /// While it is often numeric, it should always serialize as a String. + /// + /// http://facebook.github.io/graphql/June2018/#sec-ID + /// public sealed class IdType : StringTypeBase { diff --git a/src/Types/Types/Scalars/IntType.cs b/src/Types/Types/Scalars/IntType.cs index 0daf28cf6fc..071ebc556b6 100644 --- a/src/Types/Types/Scalars/IntType.cs +++ b/src/Types/Types/Scalars/IntType.cs @@ -3,6 +3,13 @@ namespace HotChocolate.Types { + /// + /// The Int scalar type represents a signed 32‐bit numeric non‐fractional + /// value. Response formats that support a 32‐bit integer or a number type + /// should use that type to represent this scalar. + /// + /// http://facebook.github.io/graphql/June2018/#sec-Int + /// public sealed class IntType : NumberType { diff --git a/src/Types/Types/Scalars/StringType.cs b/src/Types/Types/Scalars/StringType.cs index 69e371c234d..013fbf544d6 100644 --- a/src/Types/Types/Scalars/StringType.cs +++ b/src/Types/Types/Scalars/StringType.cs @@ -5,11 +5,13 @@ namespace HotChocolate.Types { /// /// The String scalar type represents textual data, represented as - /// UTF‐8 character sequences. The String type is most often used by GraphQL - /// to represent free‐form human‐readable text. + /// UTF‐8 character sequences. The String type is most often used + /// by GraphQL to represent free‐form human‐readable text. /// /// All response formats must support string representations, /// and that representation must be used here. + /// + /// http://facebook.github.io/graphql/June2018/#sec-String /// public sealed class StringType : StringTypeBase