Skip to content

Commit d839ddb

Browse files
committed
Refactor ActionController::RateLimiting to use AS::Cache
Given that the limiter implementation provided by Kredis is a simple increment with a limit, all `ActiveSupport::Cache` already provide that same capability, with a wide range of backing stores, and not just Redis. This even allow to use SolidCache has a backend if you so desire. If we feel particularly fancy, we could also accept a more generic limiter interface to better allow users to swap the implementation for better algorithms such as leaky-bucket etc.
1 parent b54a287 commit d839ddb

File tree

5 files changed

+23
-46
lines changed

5 files changed

+23
-46
lines changed

Gemfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ else
8787
gem "rack", git: "https://github.com/rack/rack.git", branch: "main"
8888
end
8989

90-
gem "kredis", ">= 1.7.0", require: false
9190
gem "useragent", require: false
9291

9392
# Active Job

Gemfile.lock

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,6 @@ GEM
299299
rexml
300300
kramdown-parser-gfm (1.1.0)
301301
kramdown (~> 2.0)
302-
kredis (1.7.0)
303-
activemodel (>= 6.0.0)
304-
activesupport (>= 6.0.0)
305-
redis (>= 4.2, < 6)
306302
language_server-protocol (3.17.0.3)
307303
libxml-ruby (5.0.0)
308304
listen (3.8.0)
@@ -611,7 +607,6 @@ DEPENDENCIES
611607
jbuilder
612608
jsbundling-rails
613609
json (>= 2.0.0, != 2.7.0)
614-
kredis (>= 1.7.0)
615610
libxml-ruby
616611
listen (~> 3.3)
617612
mdl (!= 0.13.0)

actionpack/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
*DHH*
2323

24-
* Add rate limiting API using Redis and the [Kredis limiter type](https://github.com/rails/kredis/blob/main/lib/kredis/types/limiter.rb).
24+
* Add rate limiting API.
2525

2626
```ruby
2727
class SessionsController < ApplicationController
@@ -34,7 +34,7 @@
3434
end
3535
```
3636

37-
*DHH*
37+
*DHH*, *Jean Boussier*
3838

3939
* Add `image/svg+xml` to the compressible content types of ActionDispatch::Static
4040

actionpack/lib/action_controller/metal/rate_limiting.rb

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ module ClassMethods
1515
# Requests that exceed the rate limit are refused with a <tt>429 Too Many Requests</tt> response. You can specialize this by passing a callable
1616
# in the <tt>with:</tt> parameter. It's evaluated within the context of the controller processing the request.
1717
#
18+
# Rate limiting relies on a backing <tt>ActiveSupport::Cache</tt> store and defaults to <tt>config.action_controller.cache_store</tt>, which
19+
# itself default to the global `config.cache_store`. If you don't want to store rate limits in the same datastore than your general caches
20+
# you can pass a custom store in the <tt>store</tt> parameter.
21+
#
1822
# Examples:
1923
#
2024
# class SessionsController < ApplicationController
@@ -26,38 +30,24 @@ module ClassMethods
2630
# by: -> { request.domain }, with: -> { redirect_to busy_controller_url, alert: "Too many signups on domain!" }, only: :new
2731
# end
2832
#
29-
# Note: Rate limiting relies on the application having an accessible Redis server and on Kredis 1.7.0+ being available in the bundle.
30-
# This uses the Kredis limiter type underneath, which is failsafe, so in case Redis is inaccessible, the rate limit will not refuse action execution.
31-
def rate_limit(to:, within:, by: -> { request.remote_ip }, with: -> { head :too_many_requests }, **options)
32-
require_compatible_kredis
33-
before_action -> { rate_limiting(to: to, within: within, by: by, with: with) }, **options
33+
# class APIController < ApplicationController
34+
# RATE_LIMIT_STORE = ActiveSupport::Cache::RedisCacheStore.new(url: ENV["REDIS_URL"])
35+
# rate_limit to: 10, within: 3.minutes, store: RATE_LIMIT_STORE
36+
# end
37+
#
38+
# TODO Note
39+
def rate_limit(to:, within:, by: -> { request.remote_ip }, with: -> { head :too_many_requests }, store: cache_store, **options)
40+
before_action -> { rate_limiting(to: to, within: within, by: by, with: with, store: store) }, **options
3441
end
35-
36-
private
37-
def require_compatible_kredis
38-
require "kredis"
39-
40-
if Kredis::VERSION < "1.7.0"
41-
raise StandardError, \
42-
"Rate limiting requires Kredis 1.7.0+. Please update by calling `bundle update kredis`."
43-
end
44-
rescue LoadError
45-
raise LoadError, \
46-
"Rate limiting requires Redis and Kredis. " +
47-
"Please ensure you have Redis installed on your system and the Kredis gem in your Gemfile."
48-
end
4942
end
5043

5144
private
52-
def rate_limiting(to:, within:, by:, with:)
53-
limiter = Kredis.limiter "rate-limit:#{controller_path}:#{instance_exec(&by)}", limit: to, expires_in: within
54-
55-
if limiter.exceeded?
45+
def rate_limiting(to:, within:, by:, with:, store:)
46+
count = store.increment("rate-limit:#{controller_path}:#{instance_exec(&by)}", 1, expires_in: within)
47+
if count && count > to
5648
ActiveSupport::Notifications.instrument("rate_limit.action_controller", request: request) do
5749
instance_exec(&with)
5850
end
59-
else
60-
limiter.poke
6151
end
6252
end
6353
end

actionpack/test/controller/rate_limiting_test.rb

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,9 @@
11
# frozen_string_literal: true
22

33
require "abstract_unit"
4-
require "kredis"
5-
6-
Kredis.configurator = Class.new do
7-
def config_for(name) { db: "2" } end
8-
def root() Pathname.new(Dir.pwd) end
9-
end.new
10-
11-
# Enable Kredis logging
12-
# ActiveSupport::LogSubscriber.logger = ActiveSupport::Logger.new(STDOUT)
134

145
class RateLimitedController < ActionController::Base
6+
self.cache_store = ActiveSupport::Cache::MemoryStore.new
157
rate_limit to: 2, within: 2.seconds, by: -> { Thread.current[:redis_test_seggregation] }, only: :limited_to_two
168

179
def limited_to_two
@@ -29,7 +21,7 @@ class RateLimitingTest < ActionController::TestCase
2921

3022
setup do
3123
Thread.current[:redis_test_seggregation] = Random.hex(10)
32-
Kredis.counter("rate-limit:rate_limited:#{Thread.current[:redis_test_seggregation]}").del
24+
RateLimitedController.cache_store.clear
3325
end
3426

3527
test "exceeding basic limit" do
@@ -46,9 +38,10 @@ class RateLimitingTest < ActionController::TestCase
4638
get :limited_to_two
4739
assert_response :ok
4840

49-
sleep 3
50-
get :limited_to_two
51-
assert_response :ok
41+
travel_to Time.now + 3.seconds do
42+
get :limited_to_two
43+
assert_response :ok
44+
end
5245
end
5346

5447
test "limited with" do

0 commit comments

Comments
 (0)