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

issue with remote schema #19

Closed
jrmdayn opened this issue Jul 12, 2018 · 10 comments
Closed

issue with remote schema #19

jrmdayn opened this issue Jul 12, 2018 · 10 comments

Comments

@jrmdayn
Copy link

jrmdayn commented Jul 12, 2018

Let me first thank you for this great lib!

I came across a use case where I would like to use a middleware to change the parent argument of a resolver:

const typeDefs = `
 type Query {
   hello(name: String): String
 }
`

const myMiddleware = {
 Query: {
   hello: (resolve, parent, args, context, info) => {
     const newParent = { foo: 'bar' };
     return resolve(newParent);
   },
 },
}

But in my resolver the parent is always null... Any reason why we do not allow modifying the parent object for a root query?

Thanks!

@maticzav
Copy link
Owner

Hey @jeremie-stratumn 👋,

Thank you for such a nice feedback. I am having a hard time replicating this. Would you mind sharing a bit more of your code? What's the version of graphql-middleware that you're using?

Let me know and I'll help you out!

@jrmdayn jrmdayn changed the title cannot modify the parent of a query type issue with remote schema Jul 14, 2018
@jrmdayn
Copy link
Author

jrmdayn commented Jul 14, 2018

Thanks for your reply @maticzav . After digging a bit more it turns out that my issue is related to schema stitching.. I have put an example together here:
https://github.com/jeremie-stratumn/graphql-middleware/tree/jd/remote/examples/remote
The remote schema has 2 queries: hello and world. On the remote server, I apply a middleware to modify the parent arg of the world query. On the main server, I apply the same middleware to modify the parent arg of the hello query. In the end, only the world query has the right parent arg. The hello one has an undefined arg..

I wonder if there is a solution for this.. Let me know what you think. Thanks!

PS: I took the liberty to rename the issue now that I understand a bit more where it's coming from..

@maticzav
Copy link
Owner

@jeremie-stratumn this is perfect! Your code example made things a lot easier to understand. So, let me get right into unravelling the mysteries of your question.

  1. You should know that two schemas merged remain separate even after stitching them together.
  2. Each GraphQL server has its own "runtime".

I'll explain these in more detail, but first, I want to make sure we understand all the parameters resolver gets.

  • parent is the result of the previous function. GraphQL works recursively by calling successive types determined by the schema. It starts off with undefined and continues execution down the chain forwarding the result of the previous resolver to the new one.
  • arguments parameter is a collection of inputs user included in the request. It is field-bound and doesn't get forwarded.
  • context is a global melting point of all resolvers that get executed.
  • info is an AST representation of your query.

Because we've built remarkably numerous tools for making GraphQL server creation easier, it might be hard to see pure GraphQL under all the abstractions these tools create.

Let's refocus on our issue; we are modifying the parent type, and we expect it to be equal across two remote schemas. Since we now know that each server has its own runtime and parent is only the result of a previous query on our local machine, we can see why middleware doesn't get forwarded - we are just changing parent locally.

Since there's no way, at least that I know of, which would give you such functionality using parent property, I recommend you make use of arg instead. Arguments, compared to parent, luckily get forwarded across remote schemas.

You can probably sort the missing pieces out yourself. Do tell me if you still need any help with that.

I hope this gives you a better picture of what's going on internally. 🙂

@jrmdayn
Copy link
Author

jrmdayn commented Jul 16, 2018

Thanks for your response and the clear explanation. It did seem like a long shot to be able to modify the parent of a remote schema.. I have thought of using arg instead, but I face a similar issue it seems: I cannot modify the remote arg from the main server. Have a look at my last commit. When I override arg from the main server on the remote server, I get an empty {} arg, but when I do the same on the remote schema directly, it's ok. If I change the remote schema so that hello query accepts an argument, then I can override that argument, but cannot create a new one.. I hope this is clear

@maticzav
Copy link
Owner

Hey hey! Yes, of course! My bad on this one. The problem is that graphql-middleware doesn't change AST at all - that's why it's forwarding the "wrong" set of information - I doesn't forward the information at all. I am not sure whether this is as needed as it might seem at the moment, but I can certainly see how this could benefit your problem.

Do you maybe see how this could be solved differently? It would also be precious if you could explain the actual problem you're having a bit since I suppose the example is only a representation of your issue.

@jrmdayn
Copy link
Author

jrmdayn commented Jul 19, 2018

Some background on this. I have a main yoga server that handles subscriptions (so long lived) and multiple graphql lambdas that handle different business logics. I do schema stitching on the main server. I have implemented an ACL (Access Control) layer that controls when a request comes in that the user is logged in and is authorized to do the action. Ideally I would want this layer to be executed on the main server, via a middleware (graphql-shield), as if a user is not authorized it would not even wake up a lambda for nothing.. This patterns works for ACL questions like: is the user authenticated? or is the user authorized to read ressource X? or is the user authorized to modify resource Y?.

But I am struggling when I have a query that returns a collection of resources that the user has access to. For example:

query {
  books: [Books!]!
}

In this case I would like my ACL to run first on the main server, gather all the book ids the user is authorized to read, and pass that to the lambda either via the parent or the args argument of the resolver. This would allow me to nicely separate what uses the ACL layer (only the main server in this case) and what does not. This is why I started this issue.

What I did is, I moved the ACL for this specific case to the resolver on the lambda itself. I make an explicit call to the ACL first, get a list of ids and the resolve the books. I guess one solution could be to have an optional argument on the books query:

query {
  books(ids:[ID!]): [Books!]!
}

that the middleware could populate, but I don't like that I have to modify the schema to solve a technical limitation of the tools I use.

Hope this is more clear now?

@maticzav
Copy link
Owner

maticzav commented Aug 5, 2018

Hey @jeremie-stratumn, thank you for being so patient with my reply. Last two weeks were hectic, and your response was sitting in the leftmost corner of my Chrome all the time. Finally, I get to answer.

If I understand you correctly, you would like to use graphql-shield to filter the list of results. I have a strong opinion about this. My take on the problem is that business logic, in this particular case filtering, should be separated from access policy - should not interfere with the actual result. Therefore, I believe that adding the ids property to books is a necessary step towards making your results more transparent. I believe in this because mixing business logic with permissions makes things less scalable since you rely on two layers instead of one - more edge cases.

I reread your question, took another perspective, and I had an idea. I love your approach to filtering ids in parent and forwarding it to the lambdas. schemaStitching is not a "default" in GraphQL yet, and I believe implementing this in graphql-middleware with the Apollo approach could make this package opinionated. Since you're using schema stitching, I thought that you could modify the request itself and get the desired outcome. I am not sure how this would work, digging through GraphQL core's info I am not even sure it's possible since the execution of each resolver seems more self-contained than I thought it was.

If I draw a quick diagram of how I understand your idea;

const Query = {
  books: (parent, args, context, info) => {
    return [1,2,3,4,5]
  }
}

// Middleware somehow filters [1,2,3,4,5] and delegate somewhere in between [1,2,3], right?

const Book = {
  name: (parent, args, context, info) => {
    return name
  }
}

// Get's executed for each id.

I am not sure this could even work, to be honest. delegation, filtering and search all have to happen at the same time - in the same resolver. I think you could achieve the desired outcome manually or by moving the book resolver to lambda entirely. The second case, though, would probably resolve in errors, since you would try to execute on nulls.

As I said, this is only my thinking. If you get an idea of how this could generally work in graphql-middleware, I would love to hear it since your last case proves excellent value in having this functionality.

I hope I helped you somehow with my response.

@jrmdayn
Copy link
Author

jrmdayn commented Aug 6, 2018

Hey. So it's actually the other way around. before running the query, the middleware would filter what I am allowed to read:

// middleware: [1,2,3,4,5] => [1,2,3]

then:

const Query = {
  books: (parent, args, context, info) => {
    // read [1,2,3] from parent or args
  }
}

@stale
Copy link

stale bot commented Sep 27, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 27, 2018
@stale stale bot closed this as completed Oct 7, 2018
@ChopperLee2011
Copy link

ChopperLee2011 commented Jan 13, 2020

sorry, I think I have some issue, the args to a remote query cannot be editable in the main server.

arguments parameter is a collection of inputs user included in the request. It is field-bound and doesn't get forwarded.

@maticzav from your words, I think using middleware to update remote query's input is impossible, right ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants