servicestack-ratelimit-redis
servicestack-ratelimit-redis copied to clipboard
Update build
Update the build config
Still to update Appveyor config
Want a hand?
Well I'm done for the night but you should see that the Feature tests now have an authenticated and unauthenticated client.
Rather than mocking everything, the goal is to create real services/dto's using both LimitProviders
(I've combined them so that both can be used but the redis script key should prob be refactoring out into a single instance away from limit providers.)
Then testing is just sending dto's via the clients to verify everything works and limits are hit, status codes returned as expected etc.
I'll pick it up tomorrow night but feel free to checkout dev and work on the above if you want to. 👍
OK, will have a look. It a.m. here in ole antipodea, so will let you know something before you wake up. 👍
Ideally, that pattern you would want to setup, is having a base test class that has all the tests in it that verifies that any LimitProvider works the way you (the interface designer) says it should work.
That way, a new LimitProvider can provide its own test class that inherits, and does specifically what it needs to provide the limits for the tests.
Hey Scott, sorry was not able to help out much today.
I think I will wait until you have things squared away to your own satisfaction first, before adding to it.
I am happy to contribute the integration tests for LimitRateAttribute
if you need me to. Or perhaps you want to do those, so you get to understand how it works better? your call?
I am however, very keen to use this from nuget now that we have the attribute. Can we soon get a new nuget release out there?
@jezzsantos I'll put in an example of a attribute based integration test and leave you to flesh out more test cases. I'll also update the readme for config and leave a placeholder for describing attribute usage if you want a crask at that too.
btw, The plugin is called 'RateLimit', any objection to renaming the attribute to match so there is consistency when typing so both RateLimit and LimitRate are not both used?
Will release on nuget once the tests and docs have been updated and api has settled.
Sweet, Ill have a crack today. Thanks.
As far as the name goes, if you look at it from the consumers point fo view. I.e the person who just installed the Plugin and now wants to apply the attribute to their DTO's.
They will be applying it to "limit the rate, of their API call (rather than rate the limit :) ). So I feel that [LimitRate]
is more discoverable than [RateLimit]
. However, it is just my opinion.
I disagree on the name. If I've just installed a plugin called RateLimit, with RateLimitFeature and RateLimitHeaders etc, I would find RateLimitAttribute more discoverable because of it's consistency...
Granted, so lets apply the maintainer test then. What about when you are looking at your DTO code in 6 weeks time? You have several hundred of these API's to scan. Which one says what it means - better than the other?
[RateLimit(1, PerSecond)]
public MyDtoResponse Get(MyDto request)
{
....
}
OR
[LimitRate(1, PerSecond]
public MyDtoResponse Get(MyDto request)
{
...
}
When I am RateLimiting my API, I think in terms of it's RateLimit. not it's Limit Rate so to me that makes the most sense.
Righto, @wwwlicious you go for it.
I am looking at repo right now. Did you put in those integration tests and placeholder in readme yet?
Looks like there is a bug running the lua script with the version of redis-inside. Will need to figure out if I can get v4 running some other way. Won't be tonight though.
In the develop
branch I see 5 broken tests
I'm confused @wwwlicious, where do you want me to go and add new tests?
@jezzsantos Broken tests will be rewritten to use client but the lua script will need to fix the lua script/redis embedded instance first.
Not gotten to the other bits yet which is why you can't find them... haha!