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

Support decorative style limit declaration #19

Open
EPinci opened this issue Mar 30, 2017 · 6 comments
Open

Support decorative style limit declaration #19

EPinci opened this issue Mar 30, 2017 · 6 comments

Comments

@EPinci
Copy link

EPinci commented Mar 30, 2017

Hey! This is an amazing project!

Can you, please, evaluate supporting declaring limits using decorative attributes?
This would align with other decorations pattern of asp.net (such as routes or authorization) and would give a very maintainable approach at declaring limits.

Thank you!

@hypervtechnics
Copy link

Any progress on this?

@JayTechNL
Copy link

Any progress on this?

I second that. This feature would be much appreciated.

@cristipufu
Copy link
Collaborator

I tried to implement this some time ago (https://github.com/stefanprodan/AspNetCoreRateLimit/tree/feature/inline_attributes), but I couldn't easily make it work, it needed too much refactoring, maybe some breaking changes - so I abandoned it.

In the same time, I'm not sure of the actual value of this feature - having to re-deploy the code to change the limits doesn't sound so "maintainable".

@JayTechNL
Copy link

JayTechNL commented Aug 19, 2020

Too bad, but I understand. Thanks for the effort 👍

I personally don't need to update this real-time for my solution, I would just favor something like this:
[HttpThrottle(["1m", "5m"], [2, 100])]
over a long list of appsettings policies. The HttpThrottle being the pseudo code here of course.

Anyway, great project and I think I'll manage without it haha.

Cheers!

@BarsikV
Copy link

BarsikV commented Dec 20, 2021

In the same time, I'm not sure of the actual value of this feature - having to re-deploy the code to change the limits doesn't sound so "maintainable".

I think both types of configuration are very valid. I see how changing the configuration on the fly without redeploying could be valuable.
However, having all the endpoint configurations at the Controller level is much easier to read and maintain at the code level. Redeploying is very easy in a modern CI/CD scenario, unless you need to do it a lot, which is not always the case.

Would like very much to see this feature.
Looking at the code, I understand your concern about needing a major refactor. On the other hand, all the code is already there, the attribute is the same as the middleware, but with the rules passed to them directly and per endpoint.

@BarsikV
Copy link

BarsikV commented Dec 20, 2021

Another way of implementing this is creating plain attributes, which do nothing. And use reflection to get all the endpoints configured this way. Then just pass it as the configuration class.

Yes, this is worse than implementing the code directly in an action filter attribute, yet it does the job as good as the current implementation, while not requiring too much effort. And if you configure it once per app life, we don't care about reflection being slow here.

I am most likely will be going with this approach for my scenario, can do a PR here if it works.

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

No branches or pull requests

4 participants