Merge branch 'main' into drop-rack-1

.github/workflows/build.yml

@@ -17,12 +17,12 @@ jobs:
     strategy:
       matrix:
         ruby:
-          - 3.2.2
-          - 3.1.4
-          - 3.0.6
-          - 2.7.8
-          - 2.6.10
-          - 2.5.8
+          - '3.2'
+          - '3.1'
+          - '3.0'
+          - '2.7'
+          - '2.6'
+          - '2.5'
         gemfile:
           - rack_3
           - rack_2
@@ -31,7 +31,6 @@ jobs:
           - rails_6_1
           - rails_6_0
           - rails_5_2
-          - rails_4_2
           - dalli3
           - dalli2
           - redis_5
@@ -48,61 +47,53 @@ jobs:
           - redis_store
         exclude:
           - gemfile: rails_5_2
-            ruby: 3.2.2
+            ruby: '3.2'
           - gemfile: active_support_5_redis_cache_store
-            ruby: 3.2.2
+            ruby: '3.2'
           - gemfile: active_support_5_redis_cache_store_pooled
-            ruby: 3.2.2
-          - gemfile: rails_4_2
-            ruby: 3.2.2
+            ruby: '3.2'
           - gemfile: dalli2
-            ruby: 3.2.2
+            ruby: '3.2'
           - gemfile: rails_5_2
-            ruby: 3.1.4
+            ruby: '3.1'
           - gemfile: active_support_5_redis_cache_store
-            ruby: 3.1.4
+            ruby: '3.1'
           - gemfile: active_support_5_redis_cache_store_pooled
-            ruby: 3.1.4
-          - gemfile: rails_4_2
-            ruby: 3.1.4
+            ruby: '3.1'
           - gemfile: dalli2
-            ruby: 3.1.4
+            ruby: '3.1'
           - gemfile: rails_5_2
-            ruby: 3.0.6
+            ruby: '3.0'
           - gemfile: active_support_5_redis_cache_store
-            ruby: 3.0.6
+            ruby: '3.0'
           - gemfile: active_support_5_redis_cache_store_pooled
-            ruby: 3.0.6
-          - gemfile: rails_4_2
-            ruby: 3.0.6
+            ruby: '3.0'
           - gemfile: dalli2
-            ruby: 3.0.6
-          - gemfile: rails_4_2
-            ruby: 2.7.8
+            ruby: '3.0'
           - gemfile: rails_7_0
-            ruby: 2.6.10
+            ruby: '2.6'
           - gemfile: rails_7_0
-            ruby: 2.5.8
+            ruby: '2.5'
           - gemfile: active_support_7_0_redis_cache_store
-            ruby: 2.5.8
+            ruby: '2.6'
           - gemfile: active_support_7_0_redis_cache_store
-            ruby: 2.6.10
+            ruby: '2.5'
           - gemfile: active_support_7_0_redis_cache_store_pooled
-            ruby: 2.5.8
+            ruby: '2.6'
           - gemfile: active_support_7_0_redis_cache_store_pooled
-            ruby: 2.6.10
+            ruby: '2.5'
           - gemfile: rails_7_1
-            ruby: 2.6.10
+            ruby: '2.6'
           - gemfile: rails_7_1
-            ruby: 2.5.8
+            ruby: '2.5'
           - gemfile: active_support_7_1_redis_cache_store
-            ruby: 2.5.8
+            ruby: '2.6'
           - gemfile: active_support_7_1_redis_cache_store
-            ruby: 2.6.10
+            ruby: '2.5'
           - gemfile: active_support_7_1_redis_cache_store_pooled
-            ruby: 2.5.8
+            ruby: '2.6'
           - gemfile: active_support_7_1_redis_cache_store_pooled
-            ruby: 2.6.10
+            ruby: '2.5'
     env:
       BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile
     steps:

Appraisals

@@ -28,14 +28,6 @@ appraise 'rails_5-2' do
   gem 'railties', '~> 5.2.0'
 end
 
-appraise 'rails_4-2' do
-  gem 'railties', '~> 4.2.0'
-
-  # Override rack-test version constraint by making it more loose
-  # so it's compatible with actionpack 4.2.x
-  gem "rack-test", ">= 0.6"
-end
-
 appraise 'dalli2' do
   gem 'dalli', '~> 2.0'
 end

lib/rack/attack/throttle.rb

@@ -38,8 +38,9 @@ def matched_by?(request)
           epoch_time: cache.last_epoch_time
         }
 
+        annotate_request_with_throttle_data(request, data)
+
         (count > current_limit).tap do |throttled|
-          annotate_request_with_throttle_data(request, data)
           if throttled
             annotate_request_with_matched_data(request, data)
             Rack::Attack.instrument(request)

spec/rack_attack_instrumentation_spec.rb

@@ -2,42 +2,38 @@
 
 require_relative "spec_helper"
 require 'active_support'
+require 'active_support/subscriber'
 
-# ActiveSupport::Subscribers added in ~> 4.0.2.0
-if ActiveSupport::VERSION::MAJOR > 3
-  require_relative 'spec_helper'
-  require 'active_support/subscriber'
-  class CustomSubscriber < ActiveSupport::Subscriber
-    @notification_count = 0
+class CustomSubscriber < ActiveSupport::Subscriber
+  @notification_count = 0
 
-    class << self
-      attr_accessor :notification_count
-    end
+  class << self
+    attr_accessor :notification_count
+  end
 
-    def throttle(_event)
-      self.class.notification_count += 1
-    end
+  def throttle(_event)
+    self.class.notification_count += 1
   end
+end
 
-  describe 'Rack::Attack.instrument' do
-    before do
-      @period = 60 # Use a long period; failures due to cache key rotation less likely
-      Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-      Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip }
-    end
+describe 'Rack::Attack.instrument' do
+  before do
+    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
+    Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip }
+  end
 
-    describe "with throttling" do
-      before do
-        ActiveSupport::Notifications.stub(:notifier, ActiveSupport::Notifications::Fanout.new) do
-          CustomSubscriber.attach_to("rack_attack")
-          2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-        end
+  describe "with throttling" do
+    before do
+      ActiveSupport::Notifications.stub(:notifier, ActiveSupport::Notifications::Fanout.new) do
+        CustomSubscriber.attach_to("rack_attack")
+        2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
       end
+    end
 
-      it 'should instrument without error' do
-        _(last_response.status).must_equal 429
-        assert_equal 1, CustomSubscriber.notification_count
-      end
+    it 'should instrument without error' do
+      _(last_response.status).must_equal 429
+      assert_equal 1, CustomSubscriber.notification_count
     end
   end
 end