search

haskell-graphql / graphql-api

Write type-safe GraphQL services in Haskell
BSD 3-Clause "New" or "Revised" License
406 stars 35 forks source link

Merge selections in a set into ordered map of fields #92

Closed jml closed 7 years ago

jml commented 7 years ago

Fixes #59

This makes the job of the resolver much simpler. It doesn't have to worry about inline fragments or whatever, it only deals with ordered maps of fields.

Unfortunately, it also makes the job of the validator more complex. Recursing through the whole query to merge fields is a bit involved. Defining some generic operations on OrderedMap made this much easier to think about.

This PR also introduces the notion of types into the validator. We need to know the type of the object we're building a selection set for, and we need to know the definitions of the type conditions. This (to me) means building and providing a value that represents the whole schema, and making sure each resolver has it. This in turn means passing a new value, allTypes down through each resolver. I think this is actually a perfect use case for ReaderT, and would be happy to change it.

It also means changing the top-level methods to only accept GraphQL Objects. This is fine, because that's all their allowed to take: https://facebook.github.io/graphql/#sec-Initial-types

Something that naturally arises from this work is that the difference between leafs and non-leafs becomes really obvious. We talked in person about different approaches, and for this PR I settled on changing the type of resolver to take Maybe (SelectionSet Value), where Nothing indicates a leaf query.

Testing is pretty shallow. I added one end-to-end test that shows off what this can do, but it's kind of limited. Some of the algorithmic & traversal code could probably do with property tests. Let me know which bits you trust least.

Some things on which I'd particularly like input:

jml commented 7 years ago

This changes so much that I have trouble finding my way in. Continuing but some preliminary questions.

Thanks for making the effort. I realise it's pretty big, but (aside from the OrderedMap changes) I couldn't break it up into smaller pieces without either having chunks of unused code, or making changes that would seem to be without any justification.