Refactor LeakyBucketRateLimiter to improve parameter handling and enc…

…apsulate properties

src/MatrixTextClient/Http/LeakyBucketRateLimiter.cs

@@ -9,56 +9,56 @@ namespace MatrixTextClient.Http;
 /// </summary>
 /// <remarks>
 /// This rate limiter is based on the leaky bucket metaphor. It allows for a burst of requests up to a certain
-/// quota, and then limits the rate of requests to a certain number per hour. Imagine a leaky bucket with requests
+/// capacity, and then limits the rate of requests to a certain number per hour. Imagine a leaky bucket with requests
 /// leaking out at a constant rate. When it is empty, no more requests can be made until it is refilled.
+/// The implementation is pessimistic, meaning that it will allow less leaks than the desired maximum, to make sure
+/// we never exceed the maximum rate.
 /// </remarks>
 public class LeakyBucketRateLimiter
 {
-    private readonly int _capacity;
-    private readonly long _ticksPerHour;
-    private readonly long _ticksPerRestore; // ticks to restore one leak
+    public int Capacity { get; }
+    public int MaxLeaksPerHour { get; }
+    public int WaterLevel { get; private set; }
 
-    private readonly int _maxLeaksPerHour;
+    private readonly long _ticksPerRestore; // ticks to restore one leak
 
     private object _lock = new object();
     private long _lastRestoreTicks;
 
-    private int _waterlevel;
-
-
-    public LeakyBucketRateLimiter(int capacity, int maxLeaksPerHour)
+    public LeakyBucketRateLimiter(int capacity = 10, int maxLeaksPerHour = 600)
     {
-        if(capacity > maxLeaksPerHour)
-            throw new ArgumentException("Capacity cannot be greater than maxLeaksPerHour.");
+        if(capacity > (maxLeaksPerHour / 2))
+            throw new ArgumentException("capacity must be less than half of maxLeaksPerHour to make this reliable.");
         if(capacity < 0 || maxLeaksPerHour <= 5)
             throw new ArgumentException("capacity must be greater than 0 and maxLeaksPerHour must be greater than 5.");
 
-        _capacity = capacity;
+        Capacity = capacity;
+        MaxLeaksPerHour = maxLeaksPerHour;
+        WaterLevel = capacity;
+
         var ticksPerSecond = Stopwatch.Frequency; // Use stopwatch, it's more accurate than DateTime
-        _ticksPerHour = ticksPerSecond * 60 * 60; // Avoid using TimeSpan, its tick frequency may be different
+        var ticksPerHour = ticksPerSecond * 60 * 60; // Avoid using TimeSpan, its tick frequency may be different
         var currentTicks = Stopwatch.GetTimestamp();
         _lastRestoreTicks = currentTicks;
-        _ticksPerRestore = _ticksPerHour / (maxLeaksPerHour - capacity); // subtract capacity to make sure we never exceed maxRequestsPerHour even if the bucket is full
-        _waterlevel = capacity;
-        _maxLeaksPerHour = maxLeaksPerHour;
+        _ticksPerRestore = ticksPerHour / (maxLeaksPerHour - capacity); // subtract capacity to make sure we never exceed maxRequestsPerHour even if the bucket is full
     }
 
     public bool Leak()
     {
-        lock (_lock)
+        lock (_lock) // using lock is not the fastest, but at the rate we send requests, it does not matter
         {
             var currentTicks = Stopwatch.GetTimestamp();
             var elapsedTicks = currentTicks - _lastRestoreTicks;
             var restored = (int)(elapsedTicks / _ticksPerRestore);
             if(restored > 0)
             {
-                _lastRestoreTicks += restored * _ticksPerRestore;
-                _waterlevel = Math.Min(_waterlevel + restored, _capacity);
+                _lastRestoreTicks = currentTicks; // We may lose some ticks here, but it's less of a problem than _lastRestoreTicks never catching up
+                WaterLevel = Math.Min(WaterLevel + restored, Capacity);
             }
 
-            if(_waterlevel > 0)
+            if(WaterLevel > 0)
             {
-                _waterlevel--;
+                WaterLevel--;
                 return true;
             }