aspnetcore-redis-rate-limiting icon indicating copy to clipboard operation
aspnetcore-redis-rate-limiting copied to clipboard

permitCount paramater values larger than 1 are currently not supported

Open syntx opened this issue 2 years ago • 4 comments

The parameter permitCount is passed to both AttemptAcquire and AcquireAsync in the RateLimiter abstract base class that all rate limiters in this library are are implementing.

The definition for this paramater is as follows: <param name="permitCount">Number of permits to try and acquire.</param> (See for instance here: https://github.com/dotnet/runtime/blob/43a60c8ed073a4c6134facadd01c9c1c2643e41a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiter.cs#L60)

Yet, all the provided classes disregard this parameter value, as in here:

https://github.com/cristipufu/aspnetcore-redis-rate-limiting/blob/808c7d8b3eaa18a12fafcb97514c769ed459362a/src/RedisRateLimiting/SlidingWindow/RedisSlidingWindowRateLimiter.cs#L51-L59

In some cases, a hard coded value of 1D is then passed on instead of the parameter, as in here:

https://github.com/cristipufu/aspnetcore-redis-rate-limiting/blob/bdca17fbc032f0d37eea05d51601c5e8b768180f/src/RedisRateLimiting/FixedWindow/RedisFixedWindowManager.cs#L56-L67

Are there plans to solve this? Also, If this is currently a known limitation of this library (fair), please provide a warning in the documentation.

Thanks.

syntx avatar Feb 08 '23 13:02 syntx

An additional missing feature resulting from the same problem is as follows: According to the definitions of AttemptAcquire and AcquireAsync, when given permitCount = 0, it is possible to check if the permits are exhausted or wait until the permits are replenished, respectively.

I would be happy to know if there is any workaround to check the actual state of the permits without acquiring a lease.

guynoga47 avatar Feb 15 '23 09:02 guynoga47

@guynoga47 please open a separate issue for your scenario and link the definitions

cristipufu avatar Feb 15 '23 13:02 cristipufu

@cristipufu do you plan to support this? Are you willing to accept a pull request?

manuelspezzani avatar Apr 06 '23 12:04 manuelspezzani

@manuelspezzani yes of course

cristipufu avatar Apr 06 '23 12:04 cristipufu