Skip to content

feat(postgraphql): inject custom settings into the database #399

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

Merged
merged 4 commits into from
Mar 21, 2017

Conversation

mlegenhausen
Copy link

This pull request adds the pgSettings configuration parameter to postgraphql middleware function. With this parameter it is possible to set custom config values to the postgres connection. These settings can then be accessed via current_setting('my.custom.setting') in all stored procedures. This is analog to the usage of the jwt.claims settings but can be defined at middleware initialisation time.

I hope you like it.

Copy link
Contributor

@langpavel langpavel left a comment

Choose a reason for hiding this comment

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

Really, this looks good. Can you provide any use case? I'm just curious..

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.

Looks good to me, just one nit 👍

// this prevents an accidentional overwriting
if (typeof pgSettings === 'object') {
for (const key of Object.keys(pgSettings)) {
localSettings.set(key, pgSettings[key])
Copy link
Member

Choose a reason for hiding this comment

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

Please wrap the value with String constructor: localSettings.set(key, String(pgSettings[key]))

Copy link
Author

Choose a reason for hiding this comment

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

I will. Any thoughts about objects that can be serialized to a json string and injected in postgres?

Copy link
Member

Choose a reason for hiding this comment

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

I think the user should handle that themself with JSON.serialize. I just want to ensure that the types are clear so that set_config(text, text, bool) isn't mistaken for a potential different user-defined function such as set_config(test, json, bool) etc.

@mlegenhausen
Copy link
Author

mlegenhausen commented Mar 18, 2017

A very simple use case is to inject node process related information in your postgres database like the current process id. My specific use case is to make the configuration of my postgres functions configurable from the node process like the configuration of an url prefix which gets concatinated via a postgres function which gets mapped to a graphql field.

This is the first preparation for a second pull request I maybe come up to allow the generation of the settings via a middleware like function. Example:

pgSettingsMiddleware: (req, defaultPgSettings) => ({ ...defaultPgSettings, 'req.ip': req.ip })

which can then be accessed via current_setting('req.ip').

Or another approach I have in mind is to open up the middleware stack of postgraphql to allow this modification in a more generell way and to allow a much broader modification of the postgraphql data. I am thinking about a multer integration in postgraphql to allow file uploads. But that is for the future ;)

@benjie benjie changed the title (feat) Allow the injection of custom postgres settings feat(postgraphql): inject custom settings into the database Mar 21, 2017
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.

LGTM 👍

@benjie benjie merged commit 9d10389 into graphile:master Mar 21, 2017
@benjie
Copy link
Member

benjie commented Mar 21, 2017

Merged 🎉 - thanks @mlegenhausen

@calebmer
Copy link
Collaborator

Released in 3.1.0 🎉

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

* (feat) Allow the injection of custom postgres settings

* Making sure the value that gets set is a string

* Linting errors fixed
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.

4 participants