Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQL Schema validation with custom scalars #984

Closed
jflevesque-genetec opened this issue Aug 1, 2023 · 27 comments
Closed

GraphQL Schema validation with custom scalars #984

jflevesque-genetec opened this issue Aug 1, 2023 · 27 comments
Assignees
Labels

Comments

@jflevesque-genetec
Copy link

I've been trying to get the validation to work when using WithGraphQLSchema and can't seem to get it to work. By debugging the lib, I can see that I get an exception because my scalar type cannot be resolved:

"Unable to resolve reference to type 'MyCustomScalar' on 'MyMutation'"

at GraphQL.Types.SchemaTypes.ConvertTypeReference(INamedType parentType, IGraphType type) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 995
at GraphQL.Types.SchemaTypes.ApplyTypeReference(IGraphType type) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 917
at GraphQL.Types.SchemaTypes.ApplyTypeReferences() in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 907
at GraphQL.Types.SchemaTypes.Initialize(ISchema schema, IServiceProvider serviceProvider, IEnumerable`1 graphTypeMappings) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 306
at GraphQL.Types.SchemaTypes..ctor(ISchema schema, IServiceProvider serviceProvider, IEnumerable`1 graphTypeMappings) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 190
at GraphQL.Types.SchemaTypes..ctor(ISchema schema, IServiceProvider serviceProvider) in /_/src/GraphQL/Types/Collections/SchemaTypes.cs:line 175
at GraphQL.Types.Schema.CreateSchemaTypes() in /_/src/GraphQL/Types/Schema.cs:line 477
at GraphQL.Types.Schema.CreateAndInitializeSchemaTypes() in /_/src/GraphQL/Types/Schema.cs:line 448
at GraphQL.Types.Schema.Initialize() in /_/src/GraphQL/Types/Schema.cs:line 196
at GraphQL.DocumentExecuter.CoreExecuteAsync(ExecutionOptions options) in /_/src/GraphQL/Execution/DocumentExecuter.cs:line 100
at GraphQL.DocumentExecuter.CoreExecuteAsync(ExecutionOptions options) in /_/src/GraphQL/Execution/DocumentExecuter.cs:line 208
at WireMock.Matchers.GraphQLMatcher.IsMatch(String input) in \WireMock.Net-1.5.32\src\WireMock.Net\Matchers\GraphQLMatcher.cs:line 83

I'm curious to know if this is something supported at the moment or if there's a way to define/specify the custom scalar inside the schema? At the moment, I had simply specified it using "scalar MyCustomScalar" inside the schema before my type, but clearly that isn't working.

Any inputs are well appreciated.

@StefH StefH self-assigned this Aug 1, 2023
@StefH
Copy link
Collaborator

StefH commented Aug 1, 2023

Can you please provide your full GraphQL schema + how those 'MyCustomScalar' and 'MyMutation' are defined?

@jflevesque-genetec
Copy link
Author

jflevesque-genetec commented Aug 1, 2023

scalar DateTime

input MessageInput {
  date: DateTime
  author: String
}

type Message {
  id: ID!
  content: String
  author: String
}

type Mutation {
  createMessage(input: MessageInput): Message
}

Schema taken from the documentation and modified slightly. Here is the query I'm sending through Postman:

{
    "query": "mutation { createMessage($date: DateTime!, $name: String!) { createMessage(input: { date: $date author: $name }) { id } } }",
    "variables": { "date": "aDate", "name": "aName" }
}

Exception: {"Unable to resolve reference to type 'DateTime' on 'MessageInput'"}

I was able to find a workaround by loading the Schema myself and manually adding class definitions implementing ScalarGraphType, something like

    internal class ScalarDateTime : ScalarGraphType
    {
        public ScalarDateTime()
        {
            Name = "DateTime";
        }

        public override object? ParseValue(object? value) => throw new NotImplementedException();
    }

and its registration:

GraphQL.Types.ISchema schema = GraphQL.Types.Schema.For(TestSchema);
schema.RegisterType(typeof(ScalarDateTime));

Since the exceptions are caught, I would always get an No mapping exists error message, which wasn't very helpful until I compiled the lib and used the pdbs to debug and looked at what was happening in the matcher. The error message at the exception level were pretty helpful to find this workaround.

@jflevesque-genetec
Copy link
Author

On a side note: even when registering scalar types, if the schema indicates a non-nullable response, it seems like the matcher will always throw. Not sure if its something that can be handled or not

@StefH
Copy link
Collaborator

StefH commented Aug 1, 2023

Thanks for the detailed description. I need some time to think on this and what to change in WireMock.

About No mapping exists : in several other places in the matchers, I catch exceptions, and depending on https://github.com/WireMock-Net/WireMock.Net/blob/master/src/WireMock.Net/Matchers/IMatcher.cs#L21 I do throw an exception.
However I think I need to change this behaviour, and just return the exception in the 404 message so that in case of an exception, the caller can figure out what could be wrong.

@jflevesque-genetec
Copy link
Author

jflevesque-genetec commented Aug 4, 2023

On that point, it becomes a bit problematic, because if you have multiple matchers, then which exception are you supposed to throw? In my case, I have a global matcher which returns Unauthorized if my authorization header was missing, then 1 matcher per specific request I was handling. Since it is mocking a GraphQL server, then 2 matchers out of the 3 would have thrown.

I just don't know the right way to have access to the right information to be able to troubleshoot.

I know the GraphQL support was added very recently, but I noticed if a schema also includes interfaces, it really doesn't like that (I couldn't get it to work at all if an interface was present). It was easy to remove the mention of the implements X, but it does make the schema differ from the source of truth. Maybe it could be something you could look at when you look at the scalar types problem, but no rush :)

@StefH
Copy link
Collaborator

StefH commented Aug 5, 2023

1️⃣

I'm currently rewriting some logic when an exception occurs in a matcher.

The path I'm following now is that I catch these exception(s) and write these exceptions to the logging like this:

5-8-2023 08:54:06 [Error] : Getting a Request MatchResult for Mapping '691b5474-0c38-45aa-bfc9-6ed8fece3412' failed. This mapping will not be evaluated. Exception: GraphQL.Execution.SyntaxError: Error parsing query: Expected Name, found $; for more information see http://spec.graphql.org/October2021/#Argument
 ---> GraphQLParser.Exceptions.GraphQLSyntaxErrorException: Syntax Error GraphQL (1:26) Expected Name, found $; for more information see http://spec.graphql.org/October2021/#Argument
1: mutation { createMessage($date: DateTime!, $name: String!) { createMessage(input: { date: $date author: $name }) { id } } }
                            ^

   at GraphQLParser.ParserContext.Throw_From_Expect(TokenKind kind, String description) in /_/src/GraphQLParser/ParserContext.cs:line 194
   at GraphQLParser.ParserContext.ParseName(String description) in /_/src/GraphQLParser/ParserContext.Parse.cs:line 853
   at GraphQLParser.ParserContext.ParseArgument() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 64
   at GraphQLParser.ParserContext.<>c.<ParseArguments>b__46_0(ParserContext& context) in /_/src/GraphQLParser/ParserContext.Parse.cs:line 83
   at GraphQLParser.ParserContext.OneOrMore[T](TokenKind open, ParseCallback`1 next, TokenKind close) in /_/src/GraphQLParser/ParserContext.cs:line 142
   at GraphQLParser.ParserContext.ParseArguments() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 83
   at GraphQLParser.ParserContext.ParseField() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 549
   at GraphQLParser.ParserContext.ParseSelection() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 1225
   at GraphQLParser.ParserContext.<>c.<ParseSelectionSet>b__102_0(ParserContext& context) in /_/src/GraphQLParser/ParserContext.Parse.cs:line 1240
   at GraphQLParser.ParserContext.OneOrMore[T](TokenKind open, ParseCallback`1 next, TokenKind close) in /_/src/GraphQLParser/ParserContext.cs:line 142
   at GraphQLParser.ParserContext.ParseSelectionSet() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 1240
   at GraphQLParser.ParserContext.ParseOperationDefinition() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 1066
   at GraphQLParser.ParserContext.ParseNamedDefinition(String[] oneOf) in /_/src/GraphQLParser/ParserContext.Parse.cs:line 866
   at GraphQLParser.ParserContext.ParseDefinition() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 199
   at GraphQLParser.ParserContext.ParseDefinitionsIfNotEOF() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 211
   at GraphQLParser.ParserContext.ParseDocument() in /_/src/GraphQLParser/ParserContext.Parse.cs:line 13
   at GraphQLParser.Parser.Parse(ROM source, ParserOptions options) in /_/src/GraphQLParser/Parser.cs:line 20
   at GraphQL.Execution.GraphQLDocumentBuilder.Build(String body) in /_/src/GraphQL/Execution/GraphQLDocumentBuilder.cs:line 40
   --- End of inner exception stack trace ---
   at GraphQL.Execution.GraphQLDocumentBuilder.Build(String body) in /_/src/GraphQL/Execution/GraphQLDocumentBuilder.cs:line 44
   at GraphQL.DocumentExecuter.CoreExecuteAsync(ExecutionOptions options) in /_/src/GraphQL/Execution/DocumentExecuter.cs:line 108
5-8-2023 08:54:06 [Warn] : HttpStatusCode set to 404 : No matching mapping found

Would this be a solution for you?


2️⃣

About fixing the root-cause: I did not have time yet to investigate that. Probably a way to solve this is to expose some interface on the mapper and the json mapping to support adding extra types like you do:

GraphQL.Types.ISchema schema = GraphQL.Types.Schema.For(TestSchema);
schema.RegisterType(typeof(ScalarDateTime));

#986

@jflevesque-genetec
Copy link
Author

1- It looks very promising. What would it look like if you have multiple matchers configured?
2- It would probably be the easiest. Otherwise, any way to inject a list of types or something when you setup the schema?

@StefH
Copy link
Collaborator

StefH commented Aug 21, 2023

2]
For some reason several simple standard build-in types are not registered by default by GraphQL.NET
I don't know why.

But when I change the code to below, your example works.

private static ISchema BuildSchema(string typeDefinitions)
{
    var schema = Schema.For(typeDefinitions);

    // #984
    schema.RegisterTypes(schema.BuiltInTypeMappings.Select(x => x.graphType).ToArray());

    return schema;
}

@StefH
Copy link
Collaborator

StefH commented Oct 15, 2023

#1011

@StefH StefH added feature and removed question labels Oct 15, 2023
@StefH
Copy link
Collaborator

StefH commented Oct 15, 2023

@jflevesque-genetec

You can try preview version 1.5.39-ci-17839 which supports default scalars like DateTime.

(https://github.com/WireMock-Net/WireMock.Net/wiki/MyGet-preview-versions)

@jflevesque-genetec
Copy link
Author

Hi Stef, I'm currently testing out the CI build and wanted to confirm with you which behavior this addresses. Does this fix only target built-in scalar types like DateTime from the GraphQL lib or custom scalar types as well defined directly in the schema string? If its also the latter, it does not seem like it is working.

@StefH
Copy link
Collaborator

StefH commented Oct 16, 2023

Currently only scalars in the schema.

Custom scalars will be implemented in another PR.

@jflevesque-genetec
Copy link
Author

In that case, everything still seems to be working fine with the CI build

@StefH
Copy link
Collaborator

StefH commented Oct 16, 2023

@jflevesque-genetec

I've built some code to get all custom scalars from a schema.

However, I also need to know what the C# Type is for that custom scalar. Else there is no check if a value is having the correct type which means that the matcher will not work as intended.


So a mapping must be provided like:

  • MyCustomScalar : int
  • AnotherScalar : string

With this mapping I can dynamically create these MyCustomScalar and AnotherScalar and provide these to the schema so that resolving works.

Also the ParseValue should be implemented so that when you pass a value "true" to the MyCustomScalar, an exception should be thrown.

@StefH
Copy link
Collaborator

StefH commented Oct 19, 2023

@jflevesque-genetec
In preview 1.5.39-ci-17870 i've added code use custom scalars.

like:

var customScalars = new Dictionary<string, Type> { { "MyCustomScalar", typeof(int) } };
var matcher = new GraphQLMatcher(testSchema, customScalars);

Can you test this?

@jflevesque-genetec
Copy link
Author

jflevesque-genetec commented Oct 19, 2023

Hi Stef,
Sorry for the delay in testing. I kind of understand how to create the matcher, but it seems to lose a lot of the convenience of the IRequestBuilder. Is there a plan to update the builder to be able to inject the types with the string schema or would it be necessary to go with the manually built matcher?

Side note: there isn't a way at the moment to add a matcher to the IRequestBuilder.

So I'm a bit unsure how I'm supposed to test this.

@StefH
Copy link
Collaborator

StefH commented Oct 26, 2023

Hello @jflevesque-genetec ,

I missed your update in your last comment.

But using the RequestBuilder can be done like this (if this is what you mean)...

private const string TestSchema = @"
  scalar DateTime
  scalar MyCustomScalar

  input MessageInput {
    content: String
    author: String
  }

  type Message {
    id: ID!
    content: String
    author: String
  }

  type Mutation {
    createMessage(input: MessageInput): Message
    createAnotherMessage(x: MyCustomScalar, dt: DateTime): Message
    updateMessage(id: ID!, input: MessageInput): Message
  }

  type Query {
   greeting:String
   students:[Student]
   studentById(id:ID!):Student
  }

  type Student {
   id:ID!
   firstName:String
   lastName:String
   fullName:String 
  }";

var customScalars = new Dictionary<string, Type> { { "MyCustomScalar", typeof(int) } };
            server
                .Given(Request.Create()
                    .WithPath("/graphql")
                    .UsingPost()
                    .WithGraphQLSchema(TestSchema, customScalars)
                )
                .RespondWith(Response.Create()
                    .WithBody("GraphQL is ok")
                );

@jflevesque-genetec
Copy link
Author

That would actually be great. Easy to add the custom scalar types for the schema using the existing request builder. It would fill my needs perfectly.

@StefH
Copy link
Collaborator

StefH commented Oct 26, 2023

If you have time. Please test it

@StefH
Copy link
Collaborator

StefH commented Nov 5, 2023

@jflevesque-genetec
Did you have time yet to test this?

@StefH
Copy link
Collaborator

StefH commented Nov 18, 2023

@jflevesque-genetec
Did you have time yet to test this?

@jflevesque-genetec
Copy link
Author

Hi @StefH,
Sorry, I left on a business trip and then completely forgot. I'll test it this week.

@StefH
Copy link
Collaborator

StefH commented Nov 29, 2023

@jflevesque-genetec
Could you test it?

@jflevesque-genetec
Copy link
Author

Hi Stef,
In 1.5.39-ci-17870, the code does not compile when I try to use .WithGraphQLSchema(TestSchema, customScalars). I've updated to 1.5.40-ci-17986 to see if it works in that version, which also does not work. There simply isn't any overload in IGraphQLRequestBuilder taking a dictionary to do as suggested.

@StefH
Copy link
Collaborator

StefH commented Dec 4, 2023

@jflevesque-genetec
Maybe the preview version was automatically removed from MyGet.

I've triggered a new build for that branch, and the preview version number is 1.5.40-ci-17988.

See also here:
https://github.com/WireMock-Net/WireMock.Net/pull/1012/files#diff-7c5e38fc36bbca0209ec077c0230d1c55fe0a40380398013cdea36bd689dcaa4R33

@jflevesque-genetec
Copy link
Author

Hi Stef,
I've just tried with 17988 and everything works

@StefH
Copy link
Collaborator

StefH commented Dec 4, 2023

Thanks.

I'll merge the branch to main and a new official version will be released shortly.

@StefH StefH closed this as completed Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants