mickhansen / dataloader-sequelize

Batching and simplification of Sequelize with facebook/dataloader
MIT License
284 stars 58 forks source link

Multi-tenancy? #17

Closed thebigredgeek closed 4 years ago

thebigredgeek commented 8 years ago

Per https://github.com/facebook/dataloader:

"In many applications, a web server using DataLoader serves requests to many different users with different access permissions. It may be dangerous to use one cache across many users, and is encouraged to create a new cache per request:".

It would be really great if I could create a new instance of this data loader for each GraphQL request, and pass the model reference to the dataloader. The dataloader could provide it's own findById and findByPrimary methods attached to itself and wrap relation getters attached to models returned by said methods.

Thoughts @mickhansen

mickhansen commented 8 years ago

It's an approach we want to take aswell, but one much slower than the current approach. I find in query ACL with graphql and dataloader to be quite tricky in any case, so i might look towards after-fetch ACL in the future.

A short term approach would be to have the dataloader support inspecting call options looking for a _loader arg or similar and delegating to that.

With that we could slowly move towards a toggleable approach atleast.

thebigredgeek commented 8 years ago

@mickhansen can you clarify what you mean by "slower" approach? In terms of performance? I was thinking that having data loader instances that are request-specific prevents a single dataloader from trying to fetch data for multiple users, keeping the load pipeline kinda clean.

mickhansen commented 8 years ago

I can't remember what i meant by slower. I suppose we want for the case that could have the highest performance impact by batching together multiple possible queries.

In any case, yes i'd like to move towards something we we can support both globals dataloader, request-local dataloader and globals with request-local caching dataloader.

thebigredgeek commented 8 years ago

Awesome. This might be worth targeting for a v2 release?

mickhansen commented 8 years ago

Should be possible to work most of it in a v1 release with a flag.

mickhansen commented 8 years ago

Firstly we'll need to figure out a way to extract the code currently placed in the shims, and then call it from the shims.

Then it should be possible to generate new loaders for each request, inject them into context and from context manually into findOptions, the shim can then look for a specific options key, call out to that or call to the global.

stale[bot] commented 4 years ago

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.