Revert "PERF: improve speed of rate limiter"

This reverts commit a9bcdd7f27.

spec/components/auth/default_current_user_provider_spec.rb

@@ -172,47 +172,38 @@ describe Auth::DefaultCurrentUserProvider do
 
   end
 
-  context "rate limiting" do
+  it "can only try 10 bad cookies a minute" do
+    user = Fabricate(:user)
+    token = UserAuthToken.generate!(user_id: user.id)
 
-    before do
-      RateLimiter.enable
+    provider('/').log_on_user(user, {}, {})
+
+    RateLimiter.stubs(:disabled?).returns(false)
+
+    RateLimiter.new(nil, "cookie_auth_10.0.0.1", 10, 60).clear!
+    RateLimiter.new(nil, "cookie_auth_10.0.0.2", 10, 60).clear!
+
+    ip = "10.0.0.1"
+    env = { "HTTP_COOKIE" => "_t=#{SecureRandom.hex}", "REMOTE_ADDR" => ip }
+
+    10.times do
+      provider('/', env).current_user
     end
 
-    after do
-      RateLimiter.disable
-    end
+    expect {
+      provider('/', env).current_user
+    }.to raise_error(Discourse::InvalidAccess)
 
-    it "can only try 10 bad cookies a minute" do
-      user = Fabricate(:user)
-      token = UserAuthToken.generate!(user_id: user.id)
+    expect {
+      env["HTTP_COOKIE"] = "_t=#{token.unhashed_auth_token}"
+      provider("/", env).current_user
+    }.to raise_error(Discourse::InvalidAccess)
 
-      provider('/').log_on_user(user, {}, {})
+    env["REMOTE_ADDR"] = "10.0.0.2"
 
-      RateLimiter.new(nil, "cookie_auth_10.0.0.1", 10, 60).clear!
-      RateLimiter.new(nil, "cookie_auth_10.0.0.2", 10, 60).clear!
-
-      ip = "10.0.0.1"
-      env = { "HTTP_COOKIE" => "_t=#{SecureRandom.hex}", "REMOTE_ADDR" => ip }
-
-      10.times do
-        provider('/', env).current_user
-      end
-
-      expect {
-        provider('/', env).current_user
-      }.to raise_error(Discourse::InvalidAccess)
-
-      expect {
-        env["HTTP_COOKIE"] = "_t=#{token.unhashed_auth_token}"
-        provider("/", env).current_user
-      }.to raise_error(Discourse::InvalidAccess)
-
-      env["REMOTE_ADDR"] = "10.0.0.2"
-
-      expect {
-        provider('/', env).current_user
-      }.not_to raise_error
-    end
+    expect {
+      provider('/', env).current_user
+    }.not_to raise_error
   end
 
   it "correctly removes invalid cookies" do
@@ -304,53 +295,44 @@ describe Auth::DefaultCurrentUserProvider do
 
     end
 
-    context "rate limiting" do
+    it "rate limits api usage" do
 
-      before do
-        RateLimiter.enable
+      RateLimiter.stubs(:disabled?).returns(false)
+      limiter1 = RateLimiter.new(nil, "user_api_day_#{api_key.key}", 10, 60)
+      limiter2 = RateLimiter.new(nil, "user_api_min_#{api_key.key}", 10, 60)
+      limiter1.clear!
+      limiter2.clear!
+
+      SiteSetting.max_user_api_reqs_per_day = 3
+      SiteSetting.max_user_api_reqs_per_minute = 4
+
+      params = {
+        "REQUEST_METHOD" => "GET",
+        "HTTP_USER_API_KEY" => api_key.key,
+      }
+
+      3.times do
+        provider("/", params).current_user
       end
 
-      after do
-        RateLimiter.disable
+      expect {
+        provider("/", params).current_user
+      }.to raise_error(RateLimiter::LimitExceeded)
+
+      SiteSetting.max_user_api_reqs_per_day = 4
+      SiteSetting.max_user_api_reqs_per_minute = 3
+
+      limiter1.clear!
+      limiter2.clear!
+
+      3.times do
+        provider("/", params).current_user
       end
 
-      it "rate limits api usage" do
-        limiter1 = RateLimiter.new(nil, "user_api_day_#{api_key.key}", 10, 60)
-        limiter2 = RateLimiter.new(nil, "user_api_min_#{api_key.key}", 10, 60)
-        limiter1.clear!
-        limiter2.clear!
+      expect {
+        provider("/", params).current_user
+      }.to raise_error(RateLimiter::LimitExceeded)
 
-        SiteSetting.max_user_api_reqs_per_day = 3
-        SiteSetting.max_user_api_reqs_per_minute = 4
-
-        params = {
-          "REQUEST_METHOD" => "GET",
-          "HTTP_USER_API_KEY" => api_key.key,
-        }
-
-        3.times do
-          provider("/", params).current_user
-        end
-
-        expect {
-          provider("/", params).current_user
-        }.to raise_error(RateLimiter::LimitExceeded)
-
-        SiteSetting.max_user_api_reqs_per_day = 4
-        SiteSetting.max_user_api_reqs_per_minute = 3
-
-        limiter1.clear!
-        limiter2.clear!
-
-        3.times do
-          provider("/", params).current_user
-        end
-
-        expect {
-          provider("/", params).current_user
-        }.to raise_error(RateLimiter::LimitExceeded)
-
-      end
     end
   end
 end

spec/components/rate_limiter_spec.rb

@@ -8,14 +8,11 @@ describe RateLimiter do
 
   context 'disabled' do
     before do
+      RateLimiter.stubs(:disabled?).returns(true)
       rate_limiter.performed!
       rate_limiter.performed!
     end
 
-    it "should be disabled" do
-      expect(RateLimiter.disabled?).to eq(true)
-    end
-
     it "returns true for can_perform?" do
       expect(rate_limiter.can_perform?).to eq(true)
     end
@@ -28,41 +25,10 @@ describe RateLimiter do
 
   context 'enabled' do
     before do
-      RateLimiter.enable
+      RateLimiter.stubs(:disabled?).returns(false)
       rate_limiter.clear!
     end
 
-    after do
-      RateLimiter.disable
-    end
-
-    context 'global rate limiter' do
-
-      it 'can operate in global mode' do
-        limiter = RateLimiter.new(nil, "test", 2, 10, global: true)
-        limiter.clear!
-
-        limiter.performed!
-        limiter.performed!
-        expect { limiter.performed! }.to raise_error(RateLimiter::LimitExceeded)
-      end
-
-    end
-
-    context 'handles readonly' do
-      before do
-        $redis.without_namespace.slaveof '10.0.0.1', '99999'
-      end
-
-      after do
-        $redis.without_namespace.slaveof 'no', 'one'
-      end
-
-      it 'does not explode' do
-        expect { rate_limiter.performed! }.not_to raise_error
-      end
-    end
-
     context 'never done' do
       it "should perform right away" do
         expect(rate_limiter.can_perform?).to eq(true)

spec/controllers/session_controller_spec.rb

@@ -756,7 +756,7 @@ describe SessionController do
     context 'rate limited' do
       it 'rate limits login' do
         SiteSetting.max_logins_per_ip_per_hour = 2
-        RateLimiter.enable
+        RateLimiter.stubs(:disabled?).returns(false)
         RateLimiter.clear_all!
 
         2.times do

spec/models/post_action_spec.rb

@@ -20,7 +20,7 @@ describe PostAction do
 
     it "limits redo/undo" do
 
-      RateLimiter.enable
+      RateLimiter.stubs(:disabled?).returns(false)
 
       PostAction.act(eviltrout, post, PostActionType.types[:like])
       PostAction.remove_act(eviltrout, post, PostActionType.types[:like])

spec/models/topic_spec.rb

@@ -579,7 +579,7 @@ describe Topic do
 
     it "rate limits topic invitations" do
       SiteSetting.max_topic_invitations_per_day = 2
-      RateLimiter.enable
+      RateLimiter.stubs(:disabled?).returns(false)
       RateLimiter.clear_all!
 
       start = Time.now.tomorrow.beginning_of_day
@@ -1694,7 +1694,7 @@ describe Topic do
       SiteSetting.max_replies_in_first_day = 1
       SiteSetting.stubs(:client_settings_json).returns(SiteSetting.client_settings_json_uncached)
       RateLimiter.stubs(:rate_limit_create_topic).returns(100)
-      RateLimiter.enable
+      RateLimiter.stubs(:disabled?).returns(false)
       RateLimiter.clear_all!
     end
 

spec/rails_helper.rb

@@ -106,8 +106,7 @@ RSpec.configure do |config|
     #   perf benefit seems low (shaves 20 secs off a 4 minute test suite)
     #
     # $redis = DiscourseMockRedis.new
-
-    RateLimiter.disable
+    #
     PostActionNotifier.disable
     SearchIndexer.disable
     UserActionCreator.disable
@@ -203,7 +202,6 @@ def freeze_time(now = Time.now)
   Time.stubs(:now).returns(time)
   Date.stubs(:today).returns(datetime.to_date)
   TrackTimeStub.stubs(:stubbed).returns(true)
-  Process.stubs(:clock_gettime).with(Process::CLOCK_MONOTONIC).returns(datetime.to_f)
 
   if block_given?
     begin
@@ -219,7 +217,6 @@ def unfreeze_time
   Time.unstub(:now)
   Date.unstub(:today)
   TrackTimeStub.unstub(:stubbed)
-  Process.unstub(:clock_gettime)
 end
 
 def file_from_fixtures(filename, directory = "images")