From c081a650b26430fd54f587ea3e784d35742616ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bjergmark?= Date: Tue, 7 Jan 2020 18:55:41 +0100 Subject: [PATCH 1/4] Added Issue #91 testcase --- .../Issues/Issue91.cs | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs diff --git a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs new file mode 100644 index 0000000..dcacef4 --- /dev/null +++ b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs @@ -0,0 +1,112 @@ +using GraphQL.Types; +using SAHB.GraphQL.Client.Introspection.Validation; +using SAHB.GraphQL.Client.TestServer; +using SAHB.GraphQLClient; +using SAHB.GraphQLClient.FieldBuilder; +using SAHB.GraphQLClient.Introspection; +using System.Threading.Tasks; +using Xunit; + +namespace SAHB.GraphQL.Client.Introspection.Tests.Issues +{ + public class Issue91 + { + public class Issue91Nullable : IClassFixture> + { + private readonly GraphQLWebApplicationFactory _factory; + + public Issue91Nullable(GraphQLWebApplicationFactory factory) + { + _factory = factory; + } + + [Fact] + public async Task Validate_Query_IsValid() + { + // Arrange + var client = _factory.CreateClient(); + var graphQLClient = GraphQLHttpClient.Default(client); + + // Act + var introspectionQuery = await graphQLClient.CreateQuery("http://localhost/graphql").Execute(); + var validationOutput = introspectionQuery.ValidateGraphQLType(GraphQLOperationType.Query); + + // Assert + Assert.Empty(validationOutput); + } + } + + public class Issue91NonNullable : IClassFixture> + { + private readonly GraphQLWebApplicationFactory _factory; + + public Issue91NonNullable(GraphQLWebApplicationFactory factory) + { + _factory = factory; + } + + [Fact] + public async Task Validate_Query_IsValid() + { + // Arrange + var client = _factory.CreateClient(); + var graphQLClient = GraphQLHttpClient.Default(client); + + // Act + var introspectionQuery = await graphQLClient.CreateQuery("http://localhost/graphql").Execute(); + var validationOutput = introspectionQuery.ValidateGraphQLType(GraphQLOperationType.Query); + + // Assert + Assert.Single(validationOutput); + Assert.False(true); + } + } + public class Issue91Query + { + public decimal? lat { get; set; } + public decimal? lng { get; set; } + } + + public class Issue91NullableSchema : Schema + { + public Issue91NullableSchema() + { + Query = new GraphQLQuery(); + } + + private class GraphQLQuery : ObjectGraphType + { + public GraphQLQuery() + { + Field( + "lat", + resolve: context => 1.0m); + Field( + "lng", + resolve: context => 1.1m); + } + } + } + + public class Issue91NonNullableSchema : Schema + { + public Issue91NonNullableSchema() + { + Query = new GraphQLQuery(); + } + + private class GraphQLQuery : ObjectGraphType + { + public GraphQLQuery() + { + Field>( + "lat", + resolve: context => 1.0m); + Field>( + "lng", + resolve: context => 1.1m); + } + } + } + } +} From 92abc61246f17fcb2edb6a73f11901e4b38485a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bjergmark?= Date: Tue, 7 Jan 2020 19:02:31 +0100 Subject: [PATCH 2/4] Fixed testcase --- .../SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs index dcacef4..4cd96b1 100644 --- a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs +++ b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs @@ -4,6 +4,7 @@ using SAHB.GraphQLClient; using SAHB.GraphQLClient.FieldBuilder; using SAHB.GraphQLClient.Introspection; +using System.Linq; using System.Threading.Tasks; using Xunit; @@ -57,8 +58,8 @@ public async Task Validate_Query_IsValid() var validationOutput = introspectionQuery.ValidateGraphQLType(GraphQLOperationType.Query); // Assert - Assert.Single(validationOutput); - Assert.False(true); + Assert.Equal(2, validationOutput.Count()); + Assert.All(validationOutput, e => Assert.Equal(ValidationType.Field_Invalid_Type, e.ValidationType)); } } public class Issue91Query From 7e4f4ba1c35b1b679381d29916bdb8a2c6cad1b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bjergmark?= Date: Tue, 7 Jan 2020 20:38:24 +0100 Subject: [PATCH 3/4] Fixed non null validation Fixed #91 --- .../Validation/GraphQLValidation.cs | 97 +++++++++++++--- .../Validation/ValidationError.cs | 4 + .../Validation/ValidationType.cs | 10 ++ .../Issues/Issue91.cs | 107 ++++++++++++++++++ 4 files changed, 202 insertions(+), 16 deletions(-) diff --git a/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs b/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs index 5ceae69..5537f4c 100644 --- a/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs +++ b/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs @@ -153,13 +153,13 @@ private static IEnumerable ValidateSelectionSet(GraphQLIntrospe switch (graphQLType.Kind) { case GraphQLTypeKind.Enum: - foreach (var error in ValidateEnum(selectionFieldPath, selection, IsListType(introspectionField.Type), graphQLType)) + foreach (var error in ValidateEnum(selectionFieldPath, selection, IsListType(introspectionField.Type), IsNonNull(introspectionField.Type), graphQLType)) { yield return error; } break; case GraphQLTypeKind.Scalar: - foreach (var error in ValidateScalar(selectionFieldPath, selection, IsListType(introspectionField.Type), graphQLType)) + foreach (var error in ValidateScalar(selectionFieldPath, selection, IsListType(introspectionField.Type), IsNonNull(introspectionField.Type), graphQLType)) { yield return error; } @@ -201,7 +201,7 @@ private static IEnumerable ValidateSelectionSet(GraphQLIntrospe } } } - + private static string GetTypeName(GraphQLIntrospectionTypeRef type) { switch (type.Kind) @@ -215,7 +215,7 @@ private static string GetTypeName(GraphQLIntrospectionTypeRef type) } } - private static IEnumerable ValidateScalar(string selectionFieldPath, GraphQLField selection, bool isListType, GraphQLIntrospectionFullType graphQLType) + private static IEnumerable ValidateScalar(string selectionFieldPath, GraphQLField selection, bool isListType, bool isNonNull, GraphQLIntrospectionFullType graphQLType) { var type = selection.BaseType; @@ -227,7 +227,7 @@ private static IEnumerable ValidateScalar(string selectionField switch (graphQLType.Name) { case "String": - if (type != typeof(string)) + if (!IsValidStringType(type)) { yield return new ValidationError( selectionFieldPath, @@ -238,7 +238,7 @@ private static IEnumerable ValidateScalar(string selectionField } break; case "Boolean": - if (type != typeof(bool)) + if (!IsValidBooleanType(type, isNonNull)) { yield return new ValidationError( selectionFieldPath, @@ -250,9 +250,7 @@ private static IEnumerable ValidateScalar(string selectionField break; case "Float": case "Decimal": - if (type != typeof(float) - && type != typeof(double) - && type != typeof(decimal)) + if (!IsValidFloatOrDecimalType(type, isNonNull)) { yield return new ValidationError( selectionFieldPath, @@ -265,7 +263,28 @@ private static IEnumerable ValidateScalar(string selectionField } } - private static IEnumerable ValidateEnum(string selectionFieldPath, GraphQLField selection, bool isListType, GraphQLIntrospectionFullType graphQLType) + private static bool IsValidStringType(Type type) + { + return type == typeof(string); + } + + private static bool IsValidBooleanType(Type type, bool isNonNull) + { + return type == typeof(bool) + || (!isNonNull && type == typeof(bool?)); + } + + private static bool IsValidFloatOrDecimalType(Type type, bool isNonNull) + { + return type == typeof(float) + || type == typeof(double) + || type == typeof(decimal) + || (!isNonNull && type == typeof(float?)) + || (!isNonNull && type == typeof(double?)) + || (!isNonNull && type == typeof(decimal?)); + } + + private static IEnumerable ValidateEnum(string selectionFieldPath, GraphQLField selection, bool isListType, bool isNonNull, GraphQLIntrospectionFullType graphQLType) { var type = selection.BaseType; @@ -274,6 +293,25 @@ private static IEnumerable ValidateEnum(string selectionFieldPa type = GetIEnumerableType(type); } + var isNullable = IsNullableType(type); + if (isNullable) + { + type = type.GenericTypeArguments.First(); + } + + // Validate nullable + if (!isNullable != isNonNull) + { + if (isNonNull) + { + yield return new ValidationError(selectionFieldPath, ValidationType.Field_Should_Be_NonNull, selection); + } + else + { + yield return new ValidationError(selectionFieldPath, ValidationType.Field_Should_Be_Nullable, selection); + } + } + if (type.GetTypeInfo().IsEnum) { // Get all enum names @@ -316,24 +354,51 @@ private static IEnumerable ValidateEnum(string selectionFieldPa } } - private static bool IsListType(GraphQLIntrospectionTypeRef graphQLIntrospectionTypeRef) + private static bool IsNullableType(Type type) { - if (graphQLIntrospectionTypeRef is null) + var typeinfo = type.GetTypeInfo(); + if (typeinfo.GenericTypeArguments.Length != 1) + return false; + + var genericType = typeinfo.GetGenericTypeDefinition(); + return genericType == typeof(Nullable<>); + } + + private static bool IsListType(GraphQLIntrospectionTypeRef type) + { + if (type is null) { - throw new ArgumentNullException(nameof(graphQLIntrospectionTypeRef)); + throw new ArgumentNullException(nameof(type)); } - if (graphQLIntrospectionTypeRef.Kind == GraphQLTypeKind.List) + if (type.Kind == GraphQLTypeKind.List) return true; - if (HasSubtype(graphQLIntrospectionTypeRef.Kind)) + if (HasSubtype(type.Kind)) { - return IsListType(graphQLIntrospectionTypeRef.OfType); + return IsListType(type.OfType); } return false; } + private static bool IsNonNull(GraphQLIntrospectionTypeRef type) + { + if (type is null) + { + throw new ArgumentNullException(nameof(type)); + } + + if (IsListType(type)) + { + return IsNonNull(type.OfType); + } + else + { + return type.Kind == GraphQLTypeKind.NonNull; + } + } + private static GraphQLIntrospectionFullType GetSubtype(GraphQLIntrospectionSchema graphQLIntrospectionSchema, GraphQLIntrospectionTypeRef graphQLIntrospectionTypeRef) { if (graphQLIntrospectionSchema is null) diff --git a/src/SAHB.GraphQLClient/Introspection/Validation/ValidationError.cs b/src/SAHB.GraphQLClient/Introspection/Validation/ValidationError.cs index 795f92b..fcd5b2b 100644 --- a/src/SAHB.GraphQLClient/Introspection/Validation/ValidationError.cs +++ b/src/SAHB.GraphQLClient/Introspection/Validation/ValidationError.cs @@ -65,6 +65,10 @@ public string Message { return $"Field at {Path} was not found"; case ValidationType.Field_Should_Have_SelectionSet: return $"Field at {Path} should have a selectionSet"; + case ValidationType.Field_Should_Be_NonNull: + return $"Field at {Path} should be non nullable"; + case ValidationType.Field_Should_Be_Nullable: + return $"Field at {Path} should be nullable"; case ValidationType.Operation_Type_Not_Found: return $"OperationType {OperationType} was not found"; case ValidationType.PossibleType_Not_Found: diff --git a/src/SAHB.GraphQLClient/Introspection/Validation/ValidationType.cs b/src/SAHB.GraphQLClient/Introspection/Validation/ValidationType.cs index bc18b6c..fb24d61 100644 --- a/src/SAHB.GraphQLClient/Introspection/Validation/ValidationType.cs +++ b/src/SAHB.GraphQLClient/Introspection/Validation/ValidationType.cs @@ -45,6 +45,16 @@ public enum ValidationType /// Field_Invalid_Type, + /// + /// This validationType is used if the specified type should be nullable + /// + Field_Should_Be_Nullable, + + /// + /// This validationType is used if the specified type should be not nullable + /// + Field_Should_Be_NonNull, + /// /// This validationType is used if the specified type of the field is not an enum and therefore not valid /// diff --git a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs index 4cd96b1..5960f31 100644 --- a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs +++ b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs @@ -62,12 +62,77 @@ public async Task Validate_Query_IsValid() Assert.All(validationOutput, e => Assert.Equal(ValidationType.Field_Invalid_Type, e.ValidationType)); } } + + public class Issue91NullableEnum : IClassFixture> + { + private readonly GraphQLWebApplicationFactory _factory; + + public Issue91NullableEnum(GraphQLWebApplicationFactory factory) + { + _factory = factory; + } + + [Fact] + public async Task Validate_Query_IsValid() + { + // Arrange + var client = _factory.CreateClient(); + var graphQLClient = GraphQLHttpClient.Default(client); + + // Act + var introspectionQuery = await graphQLClient.CreateQuery("http://localhost/graphql").Execute(); + var validationOutput = introspectionQuery.ValidateGraphQLType(GraphQLOperationType.Query); + + // Assert + Assert.Empty(validationOutput); + } + } + + public class Issue91NonNullableEnum : IClassFixture> + { + private readonly GraphQLWebApplicationFactory _factory; + + public Issue91NonNullableEnum(GraphQLWebApplicationFactory factory) + { + _factory = factory; + } + + [Fact] + public async Task Validate_Query_IsValid() + { + // Arrange + var client = _factory.CreateClient(); + var graphQLClient = GraphQLHttpClient.Default(client); + + // Act + var introspectionQuery = await graphQLClient.CreateQuery("http://localhost/graphql").Execute(); + var validationOutput = introspectionQuery.ValidateGraphQLType(GraphQLOperationType.Query); + + // Assert + Assert.Equal(2, validationOutput.Count()); + Assert.All(validationOutput, e => Assert.Equal(ValidationType.Field_Should_Be_NonNull, e.ValidationType)); + } + } + + public class Issue91Query { public decimal? lat { get; set; } public decimal? lng { get; set; } } + public class Issue91QueryEnum + { + public ExampleEnum? lat { get; set; } + public ExampleEnum? lng { get; set; } + } + + public enum ExampleEnum + { + ELEMENT_1, + ELEMENT_2 + } + public class Issue91NullableSchema : Schema { public Issue91NullableSchema() @@ -109,5 +174,47 @@ public GraphQLQuery() } } } + + public class Issue91NullableSchemaEnum : Schema + { + public Issue91NullableSchemaEnum() + { + Query = new GraphQLQuery(); + } + + private class GraphQLQuery : ObjectGraphType + { + public GraphQLQuery() + { + Field>( + "lat", + resolve: context => ExampleEnum.ELEMENT_1); + Field>( + "lng", + resolve: context => ExampleEnum.ELEMENT_2); + } + } + } + + public class Issue91NonNullableSchemaEnum : Schema + { + public Issue91NonNullableSchemaEnum() + { + Query = new GraphQLQuery(); + } + + private class GraphQLQuery : ObjectGraphType + { + public GraphQLQuery() + { + Field>>( + "lat", + resolve: context => ExampleEnum.ELEMENT_1); + Field>>( + "lng", + resolve: context => ExampleEnum.ELEMENT_2); + } + } + } } } From 077ee932893a390e39c59587eeb9b891d0f31cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bjergmark?= Date: Tue, 7 Jan 2020 20:55:55 +0100 Subject: [PATCH 4/4] Refactor --- .../Validation/GraphQLValidation.cs | 94 ++++++++++++------- .../Issues/Issue91.cs | 2 +- 2 files changed, 63 insertions(+), 33 deletions(-) diff --git a/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs b/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs index 5537f4c..263a9c4 100644 --- a/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs +++ b/src/SAHB.GraphQLClient/Introspection/Validation/GraphQLValidation.cs @@ -150,6 +150,14 @@ private static IEnumerable ValidateSelectionSet(GraphQLIntrospe // Validate type if (selection.BaseType != null) { + if (graphQLType.Kind == GraphQLTypeKind.Enum || graphQLType.Kind == GraphQLTypeKind.Scalar) + { + foreach (var error in ValidateNonNullScalar(selectionFieldPath, selection, IsListType(introspectionField.Type), IsNonNull(introspectionField.Type), graphQLType)) + { + yield return error; + } + } + switch (graphQLType.Kind) { case GraphQLTypeKind.Enum: @@ -201,7 +209,7 @@ private static IEnumerable ValidateSelectionSet(GraphQLIntrospe } } } - + private static string GetTypeName(GraphQLIntrospectionTypeRef type) { switch (type.Kind) @@ -215,7 +223,7 @@ private static string GetTypeName(GraphQLIntrospectionTypeRef type) } } - private static IEnumerable ValidateScalar(string selectionFieldPath, GraphQLField selection, bool isListType, bool isNonNull, GraphQLIntrospectionFullType graphQLType) + private static Type GetSpecificType(GraphQLField selection, bool isListType) { var type = selection.BaseType; @@ -224,6 +232,19 @@ private static IEnumerable ValidateScalar(string selectionField type = GetIEnumerableType(type); } + var isNullable = IsNullableType(type); + if (isNullable) + { + type = type.GenericTypeArguments.First(); + } + + return type; + } + + private static IEnumerable ValidateScalar(string selectionFieldPath, GraphQLField selection, bool isListType, bool isNonNull, GraphQLIntrospectionFullType graphQLType) + { + var type = GetSpecificType(selection, isListType); + switch (graphQLType.Name) { case "String": @@ -286,31 +307,7 @@ private static bool IsValidFloatOrDecimalType(Type type, bool isNonNull) private static IEnumerable ValidateEnum(string selectionFieldPath, GraphQLField selection, bool isListType, bool isNonNull, GraphQLIntrospectionFullType graphQLType) { - var type = selection.BaseType; - - if (isListType) - { - type = GetIEnumerableType(type); - } - - var isNullable = IsNullableType(type); - if (isNullable) - { - type = type.GenericTypeArguments.First(); - } - - // Validate nullable - if (!isNullable != isNonNull) - { - if (isNonNull) - { - yield return new ValidationError(selectionFieldPath, ValidationType.Field_Should_Be_NonNull, selection); - } - else - { - yield return new ValidationError(selectionFieldPath, ValidationType.Field_Should_Be_Nullable, selection); - } - } + var type = GetSpecificType(selection, isListType); if (type.GetTypeInfo().IsEnum) { @@ -354,14 +351,47 @@ private static IEnumerable ValidateEnum(string selectionFieldPa } } + private static IEnumerable ValidateNonNullScalar(string selectionFieldPath, GraphQLField selection, bool isListType, bool isNonNull, GraphQLIntrospectionFullType graphQLType) + { + var type = selection.BaseType; + + if (isListType) + { + type = GetIEnumerableType(type); + } + + // If type is class it is always nullable + if (type.GetTypeInfo().IsClass) + yield break; + + var isNullable = IsNullableType(type); + + // Validate nullable + if (!isNullable != isNonNull) + { + if (isNonNull) + { + yield return new ValidationError(selectionFieldPath, ValidationType.Field_Should_Be_NonNull, selection); + } + else + { + yield return new ValidationError(selectionFieldPath, ValidationType.Field_Should_Be_Nullable, selection); + } + } + } + private static bool IsNullableType(Type type) { var typeinfo = type.GetTypeInfo(); - if (typeinfo.GenericTypeArguments.Length != 1) - return false; - - var genericType = typeinfo.GetGenericTypeDefinition(); - return genericType == typeof(Nullable<>); + if (typeinfo.GenericTypeArguments.Length == 1) + { + var genericType = typeinfo.GetGenericTypeDefinition(); + if (genericType == typeof(Nullable<>)) + { + return true; + } + } + return false; } private static bool IsListType(GraphQLIntrospectionTypeRef type) diff --git a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs index 5960f31..6ffbdaa 100644 --- a/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs +++ b/tests/SAHB.GraphQL.Client.Integration.Tests/Issues/Issue91.cs @@ -59,7 +59,7 @@ public async Task Validate_Query_IsValid() // Assert Assert.Equal(2, validationOutput.Count()); - Assert.All(validationOutput, e => Assert.Equal(ValidationType.Field_Invalid_Type, e.ValidationType)); + Assert.All(validationOutput, e => Assert.Equal(ValidationType.Field_Should_Be_NonNull, e.ValidationType)); } }