swap bool value, was confusing

Author: Southclaws

README.md

@@ -13,19 +13,19 @@ See the post for information about the requirements and design of the actual alg
 The rate limiter satisfies this interface:
 
 ```go
-Increment(ctx context.Context, key string, incr int) (*Status, bool, error)
+Increment(ctx context.Context, key string, incr int) (status *Status, allowed bool, err error)
 ```
 
 - Status includes information you'd want to set in [`RateLimit` headers](https://datatracker.ietf.org/doc/draft-ietf-httpapi-ratelimit-headers/).
-- Bool is whether the limit was exceeded or not, true means reject the request.
+- Bool is whether action should be allowed or not, false means reject the action.
 - Errors occur for cache issues, such as Redis connectivity or malformed data.
 
 The implementation is store agnostic, however due to the way it works, Redis is the recommended approach due to the usage of [hash sets](https://redis.io/docs/latest/develop/data-types/hashes/).
 
 The `incr` argument allows you to assign different weights to the action being rate limited. For example, a simple request may use a value of 1 and an expensive request may use a value of 10.
 
 ```go
-status, exceeded, err := m.rl.Increment(ctx, key, cost)
+status, allowed, err := m.rl.Increment(ctx, key, cost)
 if err != nil {
     http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
     return
@@ -39,7 +39,7 @@ w.Header().Set(RateLimitLimit, strconv.FormatUint(uint64(limit), 10))
 w.Header().Set(RateLimitRemaining, strconv.FormatUint(uint64(remaining), 10))
 w.Header().Set(RateLimitReset, resetTime)
 
-if exceeded {
+if !allowed {
     // you shall not pass.
     w.Header().Set(RetryAfter, resetTime)
     http.Error(w, http.StatusText(http.StatusTooManyRequests), http.StatusTooManyRequests)

swirl.go

@@ -36,7 +36,7 @@ func New(store Store, limit int, period, expiry time.Duration) *Limiter {
 	}
 }
 
-func (l *Limiter) Increment(ctx context.Context, key string, incr int) (*Status, bool, error) {
+func (l *Limiter) Increment(ctx context.Context, key string, incr int) (status *Status, allowed bool, err error) {
 	now := time.Now()
 	timestamp := fmt.Sprint(now.Truncate(l.counterWindow).Unix())
 
@@ -53,7 +53,7 @@ func (l *Limiter) Increment(ctx context.Context, key string, incr int) (*Status,
 			Limit:     l.limit,
 			Period:    l.limitPeriod,
 			Reset:     now.Add(l.limitPeriod),
-		}, true, nil
+		}, false, nil
 	}
 
 	// create or move whole limit period window expiry
@@ -76,7 +76,10 @@ func (l *Limiter) Increment(ctx context.Context, key string, incr int) (*Status,
 	total := 0
 	for k, v := range vals {
 		if k > threshold {
-			i, _ := strconv.Atoi(v)
+			i, err := strconv.Atoi(v)
+			if err != nil {
+				return nil, false, err
+			}
 			total += i
 		} else {
 			// Clear the old hash keys
@@ -86,21 +89,21 @@ func (l *Limiter) Increment(ctx context.Context, key string, incr int) (*Status,
 		}
 	}
 
-	// exceeded
+	// exceeded rate limit
 	if total >= int(l.limit) {
 		return &Status{
 			Remaining: 0,
 			Limit:     l.limit,
 			Period:    l.limitPeriod,
 			Reset:     now.Add(l.limitPeriod),
-		}, true, nil
+		}, false, nil
 	}
 
-	// not exceeded
+	// allowed
 	return &Status{
 		Remaining: int(l.limit) - total,
 		Limit:     l.limit,
 		Period:    l.limitPeriod,
 		Reset:     now.Add(l.limitPeriod),
-	}, false, nil
+	}, true, nil
 }

swirl_test.go

@@ -40,20 +40,20 @@ func TestRateLimit(t *testing.T) {
 		}
 	}
 
-	check(t, Status{Remaining: 9, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 8, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 7, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 6, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 5, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 4, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 3, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 2, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 1, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
-	check(t, Status{Remaining: 0, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 9, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 8, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 7, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 6, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 5, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 4, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 3, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 2, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 1, Limit: 10}, true, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 0, Limit: 10}, false, now.Add(period))(ratelimiter.Increment(ctx, "k", 1))
 
 	// Wait for after the length of period (~3s) to reset the rate limit
 	delay := period + time.Millisecond*10
 	time.Sleep(delay)
 
-	check(t, Status{Remaining: 10, Limit: 10}, false, time.Now().Add(period))(ratelimiter.Increment(ctx, "k", 1))
+	check(t, Status{Remaining: 10, Limit: 10}, true, time.Now().Add(period))(ratelimiter.Increment(ctx, "k", 1))
 }