Exception on duplicate queries

[realFlowControl]

This pull request adds an exception in case there are duplicate queries defined in your controller as specified in #175

• write unit test
• make unit tests green again

[realFlowControl]

Looks like i found a bug in the testing suite:

    /** 
     * @Query(name="arrayObject")
     * @return ArrayObject|TestObject[]
     */
    public function testArrayObject(): ArrayObject                                                                
    {   
        return new ArrayObject([]);
    }   

    /** 
     * @Query(name="arrayObject")
     * @return iterable|TestObject[]
     */
    public function testIterable(): iterable
    {   
        return array();
    }   

from tests/Fixtures/TestController.php:88-104

Same query name here, i will fix this with the next commit

[realFlowControl]

The GraphQLite bundle tests fails in Travis, i'll have a look later

[realFlowControl]

Hey @moufmouf,
all tests are green 😄
Any remarks on this?
/Flo

[moufmouf]

Hey @flow-control ,

Thanks a lot for the PR!
I've had a quick look at it and it looks good. I'll merge it later if you don't mind. I'm trying to think about ways to improve it.

Right now, this PR can detect naming conflicts inside a class.
I was wondering how we could improve it to detect naming conflicts in a directory or set of directories (at the GlobTypeMapper level).

I'm still trying to figure out the simplest way to do this.

[realFlowControl]

I hear you, and I thought about it and I had a total different idea.

Why do we see this as a problem at all? Possibly only because if you do so (using @Query(name="foo")) in more than on occurrence, you can not predict which resolver will be used, am I right?

But we could go another way an document this as a feature. If you are using GraphQLite in a project as we do, that may be extended by plugins, this "feature" allows to override queries and fields from the core, by just annotating it with the same name. The only problem left is to make this predictable because the default namespace mapper should scan in alphabetical order - which is neither explicit nor predictable. But because the NamespaceMapper is injectable, one could explicitly set the order and therefore make this explicit and predictable.

Just my thoughts about this, but what do you think about seeing it from this point of view?

If you like the idea, i would be willing to try this out and write tests for this case :-)

Hope this all makes sense ;-)

[moufmouf]

Hey @flow-control ,

Sorry for the long delay in the answer, I've had a couple of hectic weeks.

Why do we see this as a problem at all? Possibly only because if you do so (using @query(name="foo")) in more than on occurrence, you can not predict which resolver will be used, am I right?

Well, I like the idea that one can "override" the queries provided by someone else. This is particularly important if at some point, we have queries / mutations provided by third party packages (in the "vendor/" directory).

However, I feel that if 2 controllers are next to each other, in the same directory, there is no real reason why I would favor one query over the other. So, what we are saying here is that the detection of duplicates should be also performed at the GlobControllerQueryProvider level.

Anyway, your PR, is it stands is already a good improvement. I'll merge it as-is and open a new issue for the GlobControllerQueryProvider part.

Thanks a lot for this PR!