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

feat(postgraphql): Provide mechanism to add data to resolver context #601

Merged

Conversation

alexFaunt
Copy link
Contributor

@alexFaunt alexFaunt commented Oct 7, 2017

Add an optional parameter called additionalGraphqlContextFromRequest to the options object supplied to the postgraphile middleware here

This should be provided an async function which resolves to an object. It will be called upon graphql requests, and provided the incoming request object. The resultant object is then applied to the context passed to all graphql resolvers.

This will allow us to add extra information from the request into the graphql resolver context, which when writing custom plugins will be useful for instances such as personalisation or auth flows outside of postgres.

Cheers for the library, think it's awesome.

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay on this! Perfect PR: small, nice feature, docs, tests 👍

I reserve the right to change or remove this until v4 is officially released.

@benjie benjie changed the base branch from postgraphile to benjie/graphql-build October 29, 2017 15:39
@benjie
Copy link
Member

benjie commented Oct 29, 2017

Changed the head to benjie/graphql-build so had to rebase to remove the postgraphile commits. (Currently master is v3; v3 gets merged into v4 periodically (which is benjie/graphql-build branch) that is then released as postgraphql@next, and then postgraphile is rebased on benjie/graphql-build and released. Fun times.)

@benjie benjie merged commit 5526c2c into graphile:benjie/graphql-build Oct 29, 2017
@benjie
Copy link
Member

benjie commented Nov 10, 2017

Thanks again for this, I'm now using it in one of my projects 👍

@alexFaunt
Copy link
Contributor Author

No problem, we're actually moving slowly away from postgraphile now, turns out our simple little back end is turning into a horrible Goliath. But i use it for my projects now 👍 it does so much so fast

@benjie
Copy link
Member

benjie commented Nov 10, 2017

I'd be interested to hear more about this if you've time to jot down some rough thoughts in an email. Are you moving from logic in the database, or keeping that and moving to a simpler custom GraphQL schema? Have you considered using PostGraphQL behind another GraphQL schema generated with something like graphql-weaver?

@alexFaunt
Copy link
Contributor Author

Yeah we've got the postgraphile running against our postgres instance, and using apollo-tools to join that with a hand written schema + resolvers for other fields we want more control over.

We're losing some more of that now as we're actually making a complex DB structure that has a recursion in it, which means graphql can't really handle it as there's no concept of infinite depth recursion (understandably). So we're going custom for that too.

TBH our team wasn't comfortable putting too much logic into the DB, so that's where we break with philosophy and limit what we achieve with postgraphile, so it's not a limitation in postgraphile, just limits we placed on it ourselves 👍

@benjie
Copy link
Member

benjie commented Nov 10, 2017

Yeah; as you say the indefinite recursion is a limitation of GraphQL itself... You could potentially work around that by using a recursive CTE and generating a path string (e.g. 1.1.1.5.4.2.1.1) so you can re-construct the structure on the client. Another way would be to use a recursive CTE and just expose the results as JSON. Neither are perfect though - are you looking at switching that endpoint to REST, or...?

I think there's a lot of resistance about putting logic in the database just because it's not been the way that we're used to building things in web-dev, but with todays microservices/etc putting all the business logic in the most central location makes a lot of sense as it effectively prevents anything from bypassing it or corrupting the data. I heard someone the other day say "only using a ORDBMS to store data is like only using a car to store petrol" - I feel there's an amount of truth to that, but if you're using constraints, foreign keys, indexes, triggers and views you're still reaping a lot of the benefits even if you don't use it for access control, business logic, computed columns, etc.

@alexFaunt
Copy link
Contributor Author

I think we're looking at the JSON solution so you still obtain the graph through graphql, but requesting a certain field just returns a JSON blob. We'd been toying with putting the JSON in the db too, but that didn't sit well either. This is all very much in flux though! Thanks for the CTE link, i'm still learning postgres, that will definitely come in handy when I want this data back...

RE: the resistance to moving logic to the DB, one simple problem we have is the competence of the team, we're all JS devs and postgres although simple, is still not something we feel confident to do anything complicated with yet. We also had some discussion around the microservices idea, we'll be a monolith for the foreseeable future, but when splitting to micro services we would probably never have two services hitting the same DB, so there'd be no sharing there. Not sure if you have a use-case in mind where you'd have that?

@benjie
Copy link
Member

benjie commented Nov 10, 2017

toying with putting the JSON in the db too, but that didn't sit well either

Postgres' JSONB handling is pretty 👌 but no doubt you've factored that in when evaluating it.

is still not something we feel confident to do anything complicated with yet

That's fair enough - generally a good idea to make things in technologies where you already know the caveats!

Not sure if you have a use-case in mind where you'd have that?

The sharing of Postgres between multiple microservices? Might be for example if a job worker needs to access the same database as the web clients (to see the tasks they have queued); or if there's additional gateways into the same data (e.g. an SMTP or chat gateway) that requires similar permissions checks - having these go through the existing web API could be a performance bottleneck and would also add to latency. That said, there's a lot more scaling options this way so it definitely makes sense in some contexts! As with everything, it depends on the needs of your stack.

Belline pushed a commit to Belline/postgraphql that referenced this pull request Dec 18, 2017
…raphile#601)

* feat(postgraphql): provide mechanism to add data to resolver context

* Convert additional context method to a promise and conform to naming convention

* Combine destructuring
benjie pushed a commit that referenced this pull request Apr 21, 2020
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

Successfully merging this pull request may close these issues.

2 participants