jkyberneees / fastify-gateway

A Node.js API gateway that just works!
https://www.npmjs.com/package/k-fastify-gateway
MIT License
107 stars 14 forks source link

feature : #20

Closed x-077 closed 5 years ago

x-077 commented 5 years ago

Hello,

it could be good if we could declare the "rate-limit" directly from the route declaration.

it could a optional parameter set to false by default if not use.

Thanks

jkyberneees commented 5 years ago

Hi @matth-c3, thanks for open this discussion. As documented here: https://github.com/fastify/fastify/blob/master/docs/Routes.md, there is no built-in mechanism that allows to setup 'rate-limits' on the routes configuration.

However, this feature can be easily added using custom middlewares, for example: https://www.npmjs.com/package/express-rate-limit in combination with https://www.npmjs.com/package/middleware-if-unless

Would you still desire this feature as being part of the core? Thanks

x-077 commented 5 years ago

You have a simpler solution, https://github.com/fastify/fastify-rate-limit fastify already have that plugin.

x-077 commented 5 years ago

If I may suggest it could be good to have a function "setRouter" where you parse each options and assign the values to the appropriate plugins directly in the middleware.

The problem with the method that you use ( for ) on each key of the json, you will be limited in the future.

However I really like it, nice project.

jkyberneees commented 5 years ago

Hi @matth-c3 thanks for your kind words. Regarding rate-limits, I know the fastify plugin exists, the problem would be the registration for a certain set of routes only. As far as I know the only way to do this would be using: https://github.com/fastify/fastify/blob/master/docs/Plugins.md#route-prefixing-option... However, as you can see in the source code, we don't use this features in the gateway.

For this purposes, we use middie, the fastify middleware engine, so developers can connect specific middlewares at global and route levels. Of course they would need to use connect-like middlewares.

Could you please also elaborate more on the "setRouter" suggestion, maybe a code snippet would help here...

Thanks

x-077 commented 5 years ago

Hello @jkyberneees ,

Currently, to use the plugin you are requiring fastify and register the plugin. like :

const fastify = require('fastify')({})

// required plugin for HTTP requests proxy
fastify.register(require('fastify-reply-from'))

// gateway plugin
fastify.register(require('k-fastify-gateway'), {

  middlewares: [
    require('co.....

more code....

As it's a suppose to be a fastify gateway module, I could expect to just provide my "map" via a JSON model and let the fastify magic happen in your module.

Meaning that on your side you can easily manipulate/prepare the model for each part ( route, middleware, config, plugins ) in the module before injecting/setting that in fastify.

Also you could easily integrate the new fastify plugins by creating a mini json model for each of them.

I think you could gain in flexibility.

Thanks.

jkyberneees commented 5 years ago

Hi @matth-c3, I understand your request. I will provide and optional factory mechanism that encapsulates all the bootstrap process and therefore improve usability. Thanks. Details to come...

x-077 commented 5 years ago

Hello @jkyberneees ,

I can help if you need, feel free to ask.

Thanks.

jkyberneees commented 5 years ago

Then, more than happy to see a MR from you ;) Thanks for your collaboration!

x-077 commented 5 years ago

MR ? Modification Request :D ?

Do you plan to open a gitter or an irc ?

jkyberneees commented 5 years ago

Merge Request = Pull Request

On Wed, Apr 3, 2019, 9:38 PM matth-c3 notifications@github.com wrote:

MR ? Modification Request :D ?

Do you plan to open a gitter or an irc ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jkyberneees/fastify-gateway/issues/20#issuecomment-479630010, or mute the thread https://github.com/notifications/unsubscribe-auth/AD6DXC0wvV1BB0GUMIpLumGU9yBHYncRks5vdQNLgaJpZM4cWbCG .

jkyberneees commented 5 years ago

Hi @matth-c3, any updates here? Thanks

x-077 commented 5 years ago

Hello @jkyberneees ,

As it's break the mechanism of your project, I started something different and I don't think that submitting a PR with 100% of new code would have value for you.

Feel free to close the issue.

jkyberneees commented 5 years ago

Thanks, looking forward to hear from you again.