Merge pull request go-redis#18 from go-redis/fix/cleanup

Clenaup

Author: vmihailenco

.travis.yml

@@ -5,9 +5,9 @@ services:
   - redis-server
 
 go:
-  - 1.7
-  - 1.8
-  - 1.9
+  - 1.10.x
+  - 1.11.x
+  - 1.12.x
   - tip
 
 matrix:
@@ -17,3 +17,4 @@ matrix:
 install:
   - go get github.com/go-redis/redis
   - go get golang.org/x/time/rate
+  - go get github.com/gordonklaus/ineffassign

Makefile

@@ -1,4 +1,6 @@
 all:
 	go test ./...
 	go test ./... -short -race
+	env GOOS=linux GOARCH=386 go test ./...
 	go vet
+	ineffassign .

internal_test.go

@@ -0,0 +1,30 @@
+package redis_rate
+
+import (
+	"testing"
+	"time"
+
+	"golang.org/x/time/rate"
+)
+
+var limitPeriodTests = []struct {
+	rate   rate.Limit
+	limit  int64
+	period time.Duration
+}{
+	{rate.Every(time.Millisecond), 1000, time.Second},
+	{rate.Every(time.Second), 1, time.Second},
+	{rate.Every(time.Hour / 5), 1, 12 * time.Minute},
+}
+
+func TestLimitPeriod(t *testing.T) {
+	for i, test := range limitPeriodTests {
+		limit, period := limitPeriod(test.rate)
+		if limit != test.limit {
+			t.Fatalf("%d: got %d, wanted %d", i, limit, test.limit)
+		}
+		if period != test.period {
+			t.Fatalf("%d: got %s, wanted %s", i, period, test.period)
+		}
+	}
+}

rate.go

@@ -29,16 +29,16 @@ func NewLimiter(redis rediser) *Limiter {
 	}
 }
 
-// Reset resets the rate limit for the name in the given rate limit window.
-func (l *Limiter) Reset(name string, dur time.Duration) error {
-	udur := int64(dur / time.Second)
-	slot := time.Now().Unix() / udur
+// Reset resets the rate limit for the name in the given rate limit period.
+func (l *Limiter) Reset(name string, period time.Duration) error {
+	secs := int64(period / time.Second)
+	slot := time.Now().Unix() / secs
 
 	name = allowName(name, slot)
 	return l.redis.Del(name).Err()
 }
 
-// Reset resets the rate limit for the name and limit.
+// ResetRate resets the rate limit for the name and limit.
 func (l *Limiter) ResetRate(name string, rateLimit rate.Limit) error {
 	if rateLimit == 0 {
 		return nil
@@ -47,45 +47,40 @@ func (l *Limiter) ResetRate(name string, rateLimit rate.Limit) error {
 		return nil
 	}
 
-	dur := time.Second
-	limit := int64(rateLimit)
-	if limit == 0 {
-		limit = 1
-		dur *= time.Duration(1 / rateLimit)
-	}
-	slot := time.Now().UnixNano() / dur.Nanoseconds()
+	_, period := limitPeriod(rateLimit)
+	slot := time.Now().UnixNano() / period.Nanoseconds()
 
-	name = allowRateName(name, dur, slot)
+	name = allowRateName(name, period, slot)
 	return l.redis.Del(name).Err()
 }
 
 // AllowN reports whether an event with given name may happen at time now.
-// It allows up to maxn events within duration dur, with each interaction
+// It allows up to maxn events within period, with each interaction
 // incrementing the limit by n.
 func (l *Limiter) AllowN(
-	name string, maxn int64, dur time.Duration, n int64,
+	name string, maxn int64, period time.Duration, n int64,
 ) (count int64, delay time.Duration, allow bool) {
-	udur := int64(dur / time.Second)
+	secs := int64(period / time.Second)
 	utime := time.Now().Unix()
-	slot := utime / udur
-	delay = time.Duration((slot+1)*udur-utime) * time.Second
+	slot := utime / secs
+	delay = time.Duration((slot+1)*secs-utime) * time.Second
 
 	if l.Fallback != nil {
 		allow = l.Fallback.Allow()
 	}
 
 	name = allowName(name, slot)
-	count, err := l.incr(name, dur, n)
+	count, err := l.incr(name, period, n)
 	if err == nil {
 		allow = count <= maxn
 	}
 
 	return count, delay, allow
 }
 
-// Allow is shorthand for AllowN(name, max, dur, 1).
-func (l *Limiter) Allow(name string, maxn int64, dur time.Duration) (count int64, delay time.Duration, allow bool) {
-	return l.AllowN(name, maxn, dur, 1)
+// Allow is shorthand for AllowN(name, max, period, 1).
+func (l *Limiter) Allow(name string, maxn int64, period time.Duration) (count int64, delay time.Duration, allow bool) {
+	return l.AllowN(name, maxn, period, 1)
 }
 
 // AllowMinute is shorthand for Allow(name, maxn, time.Minute).
@@ -108,37 +103,41 @@ func (l *Limiter) AllowRate(name string, rateLimit rate.Limit) (delay time.Durat
 		return 0, true
 	}
 
-	dur := time.Second
-	limit := int64(rateLimit)
-	if limit == 0 {
-		limit = 1
-		dur *= time.Duration(1 / rateLimit)
-	}
+	limit, period := limitPeriod(rateLimit)
 	now := time.Now()
-	slot := now.UnixNano() / dur.Nanoseconds()
+	slot := now.UnixNano() / period.Nanoseconds()
 
-	if l.Fallback != nil {
-		allow = l.Fallback.Allow()
-	}
-
-	name = allowRateName(name, dur, slot)
-	count, err := l.incr(name, dur, 1)
+	name = allowRateName(name, period, slot)
+	count, err := l.incr(name, period, 1)
 	if err == nil {
 		allow = count <= limit
+	} else if l.Fallback != nil {
+		allow = l.Fallback.Allow()
 	}
 
 	if !allow {
-		delay = time.Duration(slot+1)*dur - time.Duration(now.UnixNano())
+		delay = time.Duration(slot+1)*period - time.Duration(now.UnixNano())
 	}
 
 	return delay, allow
 }
 
-func (l *Limiter) incr(name string, dur time.Duration, n int64) (int64, error) {
+func limitPeriod(rl rate.Limit) (limit int64, period time.Duration) {
+	period = time.Second
+	if rl < 1 {
+		limit = 1
+		period *= time.Duration(1 / rl)
+	} else {
+		limit = int64(rl)
+	}
+	return limit, period
+}
+
+func (l *Limiter) incr(name string, period time.Duration, n int64) (int64, error) {
 	var incr *redis.IntCmd
 	_, err := l.redis.Pipelined(func(pipe redis.Pipeliner) error {
 		incr = pipe.IncrBy(name, n)
-		pipe.Expire(name, dur)
+		pipe.Expire(name, period+30*time.Second)
 		return nil
 	})
 
@@ -150,6 +149,6 @@ func allowName(name string, slot int64) string {
 	return fmt.Sprintf("%s:%s-%d", redisPrefix, name, slot)
 }
 
-func allowRateName(name string, dur time.Duration, slot int64) string {
-	return fmt.Sprintf("%s:%s-%d-%d", redisPrefix, name, dur, slot)
+func allowRateName(name string, period time.Duration, slot int64) string {
+	return fmt.Sprintf("%s:%s-%d-%d", redisPrefix, name, period, slot)
 }