Feature/srvdsk 17348

[raghav39]

No description provided.

[Copilot]

Pull request overview

This PR updates the rate-limiting implementation to support variable window sizes correctly in the Redis Lua script, exports several previously-internal fields/scripts for broader use, and adds additional Redis-backed tests (notably around a rate=10 bucket and window boundary behavior).

Changes:

• Fix token-bucket Lua logic to use the configured window and to persist the correct timestamp (ts) value.
• Export Bucket/SlidingWindow/RedigoStore fields and export the embedded Lua scripts (TokenBucketScript, SlidingWindowScript).
• Add several Redis integration tests covering rate=10 behavior and window boundary precision.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
token_bucket_script.go	Exports Lua script strings and updates Lua logic for ts + window.
token_bucket.lua	Mirrors the token-bucket Lua fixes (ts and window).
token_bucket.go	Exports Bucket fields and updates references accordingly.
sliding_window.go	Exports SlidingWindow fields and updates references accordingly.
redis_store.go	Updates store to use exported script variables and exports internal fields.
token_bucket_test.go	Updates Bucket construction and adds a new rate=10 test.
sliding_window_test.go	Updates SlidingWindow construction to use exported fields.
redis_store_test.go	Adds multiple new Redis-backed tests for token bucket behaviors.
go.mod / go.sum	Introduces module metadata and dependency checksums.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test overrides the package-level timeNow variable but never restores it, which can break other tests in the package depending on execution order. Save the previous value and defer restoring it (and consider applying the same pattern anywhere else timeNow is overridden).

[Copilot]

The cleanup uses pool.Get().Do(...) without closing the connection (and ignores any returned error). Please Get() the conn into a variable, defer conn.Close(), and check the Do error to avoid leaking conns in the pool.

Suggested change

	pool.Get().Do("DEL", "rate10_test:"+key)
	conn := pool.Get()
	defer conn.Close()
	if _, err := conn.Do("DEL", "rate10_test:"+key); err != nil {
	t.Errorf("cleanup failed: %v", err)
	}

[Copilot]

Outdated comment: b.windowSize no longer exists after exporting fields on Bucket. Update this comment (or remove it) to refer to b.WindowSize if still needed.

Suggested change

	// nextRefresh := time.Duration(b.windowSize) * time.Millisecond
	// nextRefresh := time.Duration(b.WindowSize) * time.Millisecond

[Copilot]

This cleanup uses pool.Get().Do(...) without closing the connection (and ignores any returned error). Use an explicit conn with defer conn.Close() to avoid leaking Redis connections during the test run.

Suggested change

	pool.Get().Do("DEL", key)
	conn := pool.Get()
	defer conn.Close()
	if _, err := conn.Do("DEL", key); err != nil {
	t.Logf("failed to delete key %q during cleanup: %v", key, err)
	}

[Copilot]

This cleanup uses pool.Get().Do(...) without closing the connection (and ignores any returned error). Use an explicit conn with defer conn.Close() to avoid leaking Redis connections during the test run.

[Copilot]

This cleanup uses pool.Get().Do(...) without closing the connection (and ignores any returned error). Use an explicit conn with defer conn.Close() to avoid leaking Redis connections during the test run.

[Copilot]

Outdated comment: b.windowSize no longer exists after exporting fields on SlidingWindow. Update this comment (or remove it) to refer to sw.WindowSize if still needed.

Suggested change

	// nextRefresh := time.Duration(b.windowSize) * time.Millisecond
	// nextRefresh := time.Duration(sw.WindowSize) * time.Millisecond

[Copilot]

These Lua scripts are static strings; declaring them as var allows accidental mutation at runtime. Consider changing TokenBucketScript to a const to prevent modification and make intent clear (same applies to SlidingWindowScript).

[Copilot]

This cleanup uses pool.Get().Do(...) without closing the connection (and ignores any returned error). Use an explicit conn with defer conn.Close() to avoid leaking Redis connections during the test run.

Suggested change

	pool.Get().Do("DEL", key)
	conn := pool.Get()
	defer conn.Close()
	if _, err := conn.Do("DEL", key); err != nil {
	t.Fatalf("failed to clean up redis key %s: %v", key, err)
	}

[Copilot]

This cleanup uses pool.Get().Do(...) without closing the connection (and ignores any returned error). Use an explicit conn with defer conn.Close() to avoid leaking Redis connections during the test run.

Suggested change

	pool.Get().Do("DEL", key)
	conn := pool.Get()
	defer conn.Close()
	if _, err := conn.Do("DEL", key); err != nil {
	t.Logf("failed to clean up key %q: %v", key, err)
	}