-
-
Notifications
You must be signed in to change notification settings - Fork 2k
🔥 feat(limiter): Add ExpirationFunc for dynamic expiration #3984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5c23d3d
8dc5341
65dab38
4cc236e
8dd54e8
cdedcf1
6505229
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,11 +19,8 @@ func (FixedWindow) New(cfg *Config) fiber.Handler { | |||||||||||||||||||
| cfg = &defaultCfg | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| var ( | ||||||||||||||||||||
| // Limiter variables | ||||||||||||||||||||
| mux = &sync.RWMutex{} | ||||||||||||||||||||
| expiration = uint64(cfg.Expiration.Seconds()) | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| // Limiter variables | ||||||||||||||||||||
| mux := &sync.RWMutex{} | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Create manager to simplify storage operations ( see manager.go ) | ||||||||||||||||||||
| manager := newManager(cfg.Storage, !cfg.DisableValueRedaction) | ||||||||||||||||||||
|
|
@@ -41,6 +38,13 @@ func (FixedWindow) New(cfg *Config) fiber.Handler { | |||||||||||||||||||
| return c.Next() | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Generate expiration from generator | ||||||||||||||||||||
| expirationDuration := cfg.ExpirationFunc(c) | ||||||||||||||||||||
| if expirationDuration <= 0 { | ||||||||||||||||||||
| expirationDuration = ConfigDefault.Expiration | ||||||||||||||||||||
| } | ||||||||||||||||||||
| expiration := uint64(expirationDuration.Seconds()) | ||||||||||||||||||||
|
Comment on lines
41
to
46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Same duration validation needed here. The same conversion safety concerns from 🔎 Suggested validation // Generate expiration from generator
expirationDuration := cfg.ExpirationFunc(c)
+if expirationDuration <= 0 {
+ return fmt.Errorf("limiter: ExpirationFunc must return a positive duration, got %v", expirationDuration)
+}
expiration := uint64(expirationDuration.Seconds())📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| // Get key from request | ||||||||||||||||||||
| key := cfg.KeyGenerator(c) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -78,7 +82,7 @@ func (FixedWindow) New(cfg *Config) fiber.Handler { | |||||||||||||||||||
| remaining := maxRequests - e.currHits | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Update storage | ||||||||||||||||||||
| if setErr := manager.set(reqCtx, key, e, cfg.Expiration); setErr != nil { | ||||||||||||||||||||
| if setErr := manager.set(reqCtx, key, e, expirationDuration); setErr != nil { | ||||||||||||||||||||
| mux.Unlock() | ||||||||||||||||||||
| return fmt.Errorf("limiter: failed to persist state: %w", setErr) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
@@ -118,7 +122,7 @@ func (FixedWindow) New(cfg *Config) fiber.Handler { | |||||||||||||||||||
| e = entry | ||||||||||||||||||||
| e.currHits-- | ||||||||||||||||||||
| remaining++ | ||||||||||||||||||||
| if setErr := manager.set(reqCtx, key, e, cfg.Expiration); setErr != nil { | ||||||||||||||||||||
| if setErr := manager.set(reqCtx, key, e, expirationDuration); setErr != nil { | ||||||||||||||||||||
| mux.Unlock() | ||||||||||||||||||||
| return fmt.Errorf("limiter: failed to persist state: %w", setErr) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ExpirationFunc in this example is redundant because it returns the same value as the static Expiration field above it (30 * time.Second). For a more useful demonstration, consider showing either a different value or a dynamic calculation based on the request context, similar to the dynamic expiration example below.