-
Notifications
You must be signed in to change notification settings - Fork 97
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
Input: Cached type in registry is not the type returned by type mapper #531
Comments
@NormySan what cache adapter are you using? I'm not familiar with this error. It looks like you're doing things correctly. |
@oojacoboo I'm using a cache pool from Symfony. Only inputs causing this problem so far, all other types seems to be working properly. |
From the cached type:
From the type mapper:
What's the stack-trace look like? How is the comparison being made? |
@oojacoboo This is the stack trace: https://gist.github.com/NormySan/d505993a1817843f7c23aa2f5a39acc0 The exception is thrown here in the recursive type mapper:
|
@NormySan do you use this same input type in multiple mutations? Can you confirm that the cached type and the type from the type mapper are effectively the same object? I think the issue here is that it's using |
@oojacoboo Yeah, I'm using the input type in multiple mutations, one is for updating and one is for creating. I tried changing the comparison to just using |
@NormySan if you're certain in your testing that it was changed and executed (not cached), and that comparison is failing, can you identify what's different about the two objects and what's causing it to fail that comparison? |
@oojacoboo I think it's failing because they are entirely different objects, I'm going to set up a basic reproduction of my codebase, maybe that will help with identifying the underlying issue. |
Here is a simple reproduction of the exception in a simple Symfony application. https://github.com/NormySan/graphqlite-input-error It does not use the Symfony bundle but instead uses a custom implementation. The GraphQL request is handled by the |
So far I've narrowed it down to the following:
I'm not sure what the expected type in the cache should be but I'm guessing that it should be the |
@NormySan so, I think the issue here is that you're using the same Input type for multiple operations. I'm not going to assume this is an incorrect design decision on your part, although it very well could be. It's mostly recommended to have a unique Input type per operation. I know this is how our API is designed and likely the reason we haven't run into this error - same for most others. That said, looking at the code, it seems clear that it's doing a direct comparison of a cached version of the Input type and the one currently being processed by the type mapper. I don't have enough insight or feedback on the overall use of this. But, it seems like a fresh look at this caching logic needs to be had.
I don't have any answers to these questions, but it seems this is the path forward. Then we can assess a refactor of the design and PR. |
Tried using two different input types and I'm still getting the same error so must be something else. I did try to set up a very minimal test based on the no-framework instructions and having the same name for the input type worked well in this case so it must be something else in Symfony or my setup causing the problem. If I can locate the underlying cause and if it's in this package I will try to make a PR. |
Similar issue in EnumTypeMapper.php. Need to add |
@NormySan did you try ommiting the Input suffix on the PHP Class? |
@Lappihuan That solved it, I would really like my inputs to be named like that though since it creates a lot of ambiguity if the input is named the same as the entities. Going to have a look at what part of the library that might be causing that issue. |
I've been trying to find a cause as to why some inputs are created as I've also been trying to find out why the name of the input might be causing issues but I can't find any reason for this either. The naming strategy looks to be working fine, this was the only place I could find where a name is being decided. |
@NormySan I agree it can be confusing. Have a look at this doc page for some insight into the type mapper https://graphqlite.thecodingmachine.io/docs/internals Also, regarding the Input name, have you tried using the following? #[Input(name: 'FacilityInput')]
class FacilityInput |
Yeah, I read the docs and it helped a bit to get started on where to look. Tried assigning a name to the input also but didn't work. I've come to the conclusion that none of the input types in my API are working. |
we use input on entities, but that might now work for you |
We just have our inputs separate from our models, entirely, and within our GraphQL namespace along with our operations. |
I do the same, everything related to the GraphQL API:s is separate from the rest of the code to create a clear boundary between API and "Domain". |
I think I might have found something related to why the types are not the same:
https://github.com/thecodingmachine/graphqlite/blob/master/src/Mappers/RecursiveTypeMapper.php#L487
https://github.com/thecodingmachine/graphqlite/blob/master/src/TypeGenerator.php#L43
https://github.com/thecodingmachine/graphqlite/blob/master/src/Mappers/RecursiveTypeMapper.php#L393
https://github.com/thecodingmachine/graphqlite/blob/master/src/Mappers/AbstractTypeMapper.php#L296
My question here then is why do the |
A change to the Edit: Looks like there is a failing test, will have a look at that. |
@NormySan instead of re-ordering the operations. I think we should focus on the checks that determine which type is being mapped where. Are you using the |
@oojacoboo I'm using the Here is another example repo that uses no framework that also has the same problem with the inputs as the one I linked to previously. |
@NormySan iirc there is semthing odd in nested input types i recently encountered which required me to "manualy" set inputType propery on Field/Input/Type attribute. I did not have time to investigate the issue in depth but i suspect there is inconsitent name conversion for PHP->GQL. |
@Lappihuan Might be, I don't really have any more time to investigate the issue either, unfortunately. |
Encountered this as well. And the issue was also mentioned in #248. |
I did some digging and the issue was introduced as part of #458 since now the If we revert this change from $type = $this->getClassAnnotation($refClass, Type::class)
?? $this->getClassAnnotation($refClass, Input::class); back to $type = $this->getClassAnnotation($refClass, Type::class); everything works as expected. Change: 83357f0#diff-b4cb271cb57cae8e3c9c55d3590bab4ea0781066f699d94ac970f89f8b7e06b4R269 |
#532 has been merged. Please confirm that this issue is now resolved. |
Hey,
I'm having a weird issue with input types where I get the exception in the title "Cached type in the registry is not the type returned by type mapper".
For some reason, the type stored in the cache is not the same as the one from the type mapper even if the input class they both reference is the same.
The mutation that I try to run only fails when I use variables to run the mutation, If I run the mutation without variables and provide the values directly it all works but this is not possible to do in the frontend since everything is automatically generated and requires the type to be provided.
Failing mutation
Working mutation
Input type
Controller/Resolver
The cached type
The type from the type mapper
The text was updated successfully, but these errors were encountered: