Support deeper types in SchemaInspector

[jorrit]

This allows reading types like [Type!]!, which are 4 levels deep. Previously a Reached the limit of nesting in type info exception would be thrown.

[mghoneimy]

Hello @jorrit ,

First of all, thank you for your contribution.

I want to understand:

1. How to reproduce the exception you're reporting here
2. How do your code changes fix this exception
3. How can we add a test to make sure we don't break this again

Also, I thought that your code was breaking the test cases, but it turns out that it's actually the new php-graphql-client release that broke one of the test cases. I will release a test for that, then I will need you to rebase your branch on master.

Thank you!

[mghoneimy]

@jorrit Thank you for rebasing your branch. I'm still waiting for your response to my three questions posted ealier to move forward with reviewing this pull request.

Thank you!

[jorrit]

Hi Mostafa, I am currently working on them! I'm on Windows, so I had to do some work to get the tests working. If I have addressed your points I could separate those improvements to another PR, perhaps.

[jorrit]

1. My schema has a field that is of the type [Int!]!. This means it is a int, non nullable, wrapped in a list, which is also non nullable. This is 4 layers of types. SchemaInspector::TYPE_SUB_QUERY has three levels of ofType subqueries. This means that schema queries fetch one ofType level to little.
2. The change adds one level of ofType {} subqueries so the innermost type is also fetched (Int in my case).
3. I would love to add some tests but it seems that all tests in SchemaClassGeneratorTest use a mock client that never reaches any actual GraphQL servers. As such, the addition to SchemaInspector::TYPE_SUB_QUERY is never sent to a server and the difference between the old and new code can't be tested.

[jorrit]

(btw: I only now see PR #2 that also addresses running tests on Windows. My approach is different, as it was for me just a matter of adding a few more PHP_EOL's instead of \n).

[mghoneimy]

@jorrit Thank you for your effort here!
I was confused on how changing only the query would lead to generating the type correctly for you until I went back to the implementation and realized it was using recursion to resolve the types, so it should be working now. Things look good and we can merge your changes now!

I had two questions though:

1. Based on this PR and on the other, I now do realize that I need to run my tests on Windows and OSX before declaring that they actually pass. This will need to be added to the travis.yml file. Would you like to do that on this PR or should I do it?
2. Do you know if there can be there more than 4 levels of nesting in GraphQL query types? If yes, do you have an idea how we can handle that in this library?

Again, thank you for your effort, I'm ready to merge your changes as soon as I hear from you on the two questions I had.

[mghoneimy]

@jorrit For some reason Travis stopped posting the results on the pull request. Here's a link to the build https://travis-ci.org/mghoneimy/php-graphql-oqm/builds/649426386?utm_medium=notification&utm_source=github_status

[jorrit]

I think travis is annoyed with all my attempts :) I have it working, somewhat. For some reason two tests fail on Windows + PHP 7.1/7.2, so I disabled them. Installing PHP 7.1 with brew on OS X is impossible, so I removed that one as well. If you want to remove or add PHP versions, let me know.

I don't know of any use cases for deeper structures, but I guess it is technically possible to have lists-of-lists like [[[[Int!]!]!]!]!. If someone has a need for that, they may post another PR, perhaps.

[mghoneimy]

@jorrit Thank you so much for your effort and significant contribution to this library!