Verified Commit 817a4083 authored by Bob Van Landuyt's avatar Bob Van Landuyt 💬 Committed by GitLab
Browse files

Merge branch 'rate-limit/stage-1c' into 'master' 

feat(rate_limit): Stage 1c — remaining/reset_at/limit fields and to_response_headers (Spec 6)

See merge request !274

Merged-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Approved-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Reviewed-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Co-authored-by: default avatarSam Wiskow <swiskow@gitlab.com>
parents 0f417f20 96aa6129

lib/labkit/rate_limit/evaluator.rb

+18 −10
Original line number Diff line number Diff line
@@ -53,14 +53,17 @@ module Labkit 
        resolved_limit = Integer(resolve_value(rule.limit))

        resolved_period = Integer(resolve_value(rule.period))



        count = incr_with_ttl(redis_key, resolved_period)

        count, ttl = incr_with_ttl(redis_key, resolved_period)

        exceeded = count > resolved_limit

        action = exceeded ? rule.action : :allow



        Result.new(

          matched: true, exceeded: exceeded, action: action, rule: rule,

          resolved_limit: resolved_limit, resolved_period: resolved_period

        info = Result::Info.new(

          resolved_limit: resolved_limit, resolved_period: resolved_period,

          count: count,

          remaining: [resolved_limit - count, 0].max,

          reset_at: Time.now.utc + (ttl >= 0 ? ttl : resolved_period)

        )



        Result.new(matched: true, exceeded: exceeded, action: action, rule: rule, info: info)

      end



      def build_redis_key(rule, identifier)
@@ -91,12 +94,17 @@ module Labkit 
        end

      end



      # Pipelines INCR and TTL so both are fetched in a single round-trip.

      # EXPIRE follows as a separate call only on first write (count == 1).

      # On first write TTL will be -1 (expiry not yet set); callers fall back to period.

      def incr_with_ttl(redis_key, period)

        @redis.with do |conn|

          count = conn.incr(redis_key)

          # Set expiry only on first write to avoid resetting TTL on each call

          count, ttl = conn.pipelined do |pipe|

            pipe.incr(redis_key)

            pipe.ttl(redis_key)

          end

          conn.expire(redis_key, period) if count == 1

          count

          [count, ttl]

        end

      end
@@ -117,11 +125,11 @@ module Labkit 
        )

        Metrics.limit_gauge.set(

          { rate_limiter: @name, rule: result.rule.name },

          result.resolved_limit

          result.info.resolved_limit

        )

        Metrics.period_gauge.set(

          { rate_limiter: @name, rule: result.rule.name },

          result.resolved_period

          result.info.resolved_period

        )

      end

lib/labkit/rate_limit/result.rb

+24 −6
Original line number Diff line number Diff line
@@ -13,12 +13,9 @@ module Labkit 
    #             The rule's configured action is available via rule.action

    # rule      - the matched Rule object (nil when matched? is false)

    # error?    - true if Redis was unavailable; result fails open (exceeded? is false)

    # resolved_limit   - the resolved limit value as Integer (nil when matched? is false or error)

    # resolved_period  - the resolved period value as Integer (nil when matched? is false or error)

    Result = Data.define(:matched, :exceeded, :action, :rule, :error, :resolved_limit, :resolved_period) do

      def initialize(

        matched:, action:, exceeded: false, rule: nil, error: false,

        resolved_limit: nil, resolved_period: nil)

    # info      - Result::Info with per-window counters; nil when matched? is false or error?

    Result = Data.define(:matched, :exceeded, :action, :rule, :error, :info) do

      def initialize(matched:, action: nil, exceeded: false, rule: nil, error: false, info: nil)

        super

      end
@@ -33,6 +30,27 @@ module Labkit 
      def error?

        error

      end



      # Returns RFC-compliant rate limit response headers, or {} when no rule matched or an error occurred.

      # Keys: RateLimit-Limit, RateLimit-Remaining, RateLimit-Reset (Unix timestamp).

      # reset_at is advisory only - derived from a pipelined redis.ttl call, not fully atomic.

      def to_response_headers

        return {} unless matched? && !error? && info



        {

          "RateLimit-Limit" => info.resolved_limit.to_s,

          "RateLimit-Remaining" => info.remaining.to_s,

          "RateLimit-Reset" => info.reset_at.to_i.to_s

        }

      end

    end



    # Per-window counter data attached to a matched Result.

    # resolved_limit  - the evaluated limit Integer for this rule

    # resolved_period - the evaluated period Integer (seconds) for this rule

    # count           - the raw INCR value; useful for utilization-ratio metrics

    # remaining       - requests remaining before the limit is hit (floors at 0)

    # reset_at        - best-effort UTC Time when the counter window resets

    Result::Info = Data.define(:resolved_limit, :resolved_period, :count, :remaining, :reset_at)

  end

end

spec/labkit/rate_limit/configuration_spec.rb

+4 −6
Original line number Diff line number Diff line
@@ -36,15 +36,14 @@ RSpec.describe "Labkit::RateLimit.configure (Scenario M & N)" do 
      c.logger = my_logger

    end



    allow(raw_redis).to receive(:incr).and_return(1)

    allow(raw_redis).to receive(:pipelined).and_return([1, 55])

    allow(raw_redis).to receive(:expire)



    rule = Labkit::RateLimit::Rule.new(name: "test", limit: 10, period: 60, characteristics: [:user])

    limiter = Labkit::RateLimit::Limiter.new(name: "test_limiter", rules: [rule])

    result = limiter.check({ user: 42 })



    expect(result.matched?).to be(true)

    expect(raw_redis).to have_received(:incr)

    expect(raw_redis).to have_received(:pipelined)

  end



  # Scenario N: explicit DI kwargs override configure block
@@ -59,9 +58,8 @@ RSpec.describe "Labkit::RateLimit.configure (Scenario M & N)" do 
    raw_override_redis = instance_double(Redis, "override_redis")

    override_redis = PooledRedis.new(raw_override_redis)

    override_logger = instance_double(Logger, "override_logger", info: nil, warn: nil)

    allow(raw_override_redis).to receive(:incr).and_return(1)

    allow(raw_override_redis).to receive(:pipelined).and_return([1, 55])

    allow(raw_override_redis).to receive(:expire)



    rule = Labkit::RateLimit::Rule.new(name: "test", limit: 10, period: 60, characteristics: [:user])

    limiter = Labkit::RateLimit::Limiter.new(

      name: "test_limiter", rules: [rule],
@@ -69,6 +67,6 @@ RSpec.describe "Labkit::RateLimit.configure (Scenario M & N)" do 
    )

    limiter.check({ user: 42 })



    expect(raw_override_redis).to have_received(:incr)

    expect(raw_override_redis).to have_received(:pipelined)

  end

end

spec/labkit/rate_limit/evaluator_spec.rb

+184 −67
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
  let(:redis) { PooledRedis.new(raw_redis) }

  let(:null_logger) { instance_double(Labkit::Logging::JsonLogger, warn: nil) }

  let(:identifier) { Labkit::RateLimit::Identifier.new(user: 42, ip: "1.2.3.4") }

  let(:pipe) { instance_double(Redis) }



  def make_rule(name: "default", match: {}, limit: 100, period: 60, action: :block, characteristics: [:user])

    Labkit::RateLimit::Rule.new(
@@ -24,6 +25,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 


  before do

    stub_env("RAILS_ENV", "test")

    allow(pipe).to receive_messages(incr: nil, ttl: nil)

    # Default: count=1, ttl=55; individual examples override as needed

    allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([1, 55])

    allow(raw_redis).to receive(:expire)

  end



  describe "Scenario Q: Redis key format uses rule name" do
@@ -31,11 +36,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      rule = make_rule(name: "unauthenticated_api", characteristics: [:ip])

      id = Labkit::RateLimit::Identifier.new(ip: "1.2.3.4")



      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:unauthenticated_api:ip:1.2.3.4")

        .and_return(1)

        .and_return(nil)

      expect(raw_redis).to receive(:expire).with("labkit:rl:rack_request:unauthenticated_api:ip:1.2.3.4", 60)



      evaluator(rules: [rule]).check(id)

    end
@@ -43,11 +47,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      rule = make_rule(name: "auth_api", characteristics: [:user, :ip])

      id = Labkit::RateLimit::Identifier.new(user: 42, ip: "1.2.3.4")



      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:auth_api:user:42:ip:1.2.3.4")

        .and_return(1)

        .and_return(nil)

      expect(raw_redis).to receive(:expire)



      evaluator(rules: [rule]).check(id)

    end
@@ -57,11 +60,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      rule = make_rule(name: "sha_rule", characteristics: [:user])



      expected_hash = OpenSSL::Digest::SHA256.hexdigest(long_value)

      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:sha_rule:user:#{expected_hash}")

        .and_return(1)

        .and_return(nil)

      expect(raw_redis).to receive(:expire)



      evaluator(rules: [rule]).check(id)

    end
@@ -75,17 +77,15 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
  describe "Scenario R: counter TTL matches rule period" do

    it "sets expire on first write (count == 1)" do

      rule = make_rule(name: "ttl_rule", period: 120)

      expect(raw_redis).to receive(:incr).and_return(1)

      allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([1, -1])

      expect(raw_redis).to receive(:expire).with(anything, 120)



      evaluator(rules: [rule]).check(identifier)

    end



    it "does not reset expire on subsequent writes (count > 1)" do

      rule = make_rule(name: "ttl_rule")

      expect(raw_redis).to receive(:incr).and_return(5)

      allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([5, 30])

      expect(raw_redis).not_to receive(:expire)



      evaluator(rules: [rule]).check(identifier)

    end

  end
@@ -95,11 +95,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      id = Labkit::RateLimit::Identifier.new(ip: "1.2.3.4")

      rule = make_rule(name: "auth_rule", characteristics: [:user])



      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:auth_rule:user:_unknown_")

        .and_return(1)

        .and_return(nil)

      expect(raw_redis).to receive(:expire)



      evaluator(rules: [rule]).check(id)

    end
@@ -107,11 +106,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      id = Labkit::RateLimit::Identifier.new(user: "", ip: "1.2.3.4")

      rule = make_rule(name: "auth_rule", characteristics: [:user])



      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:auth_rule:user:_unknown_")

        .and_return(1)

        .and_return(nil)

      expect(raw_redis).to receive(:expire)



      evaluator(rules: [rule]).check(id)

    end
@@ -119,11 +117,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      id = Labkit::RateLimit::Identifier.new(ip: "1.2.3.4")

      rule = make_rule(name: "compound", characteristics: [:user, :ip])



      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:compound:user:_unknown_:ip:1.2.3.4")

        .and_return(1)

        .and_return(nil)

      expect(raw_redis).to receive(:expire)



      evaluator(rules: [rule]).check(id)

    end

  end
@@ -133,19 +130,16 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      rule = make_rule(name: "custom", characteristics: [:custom_field])

      id = Labkit::RateLimit::Identifier.new(custom_field: "abc")



      expect(raw_redis).to receive(:incr)

      expect(pipe).to receive(:incr)

        .with("labkit:rl:rack_request:custom:custom_field:abc")

        .and_return(1)

      expect(raw_redis).to receive(:expire)



        .and_return(nil)

      expect { evaluator(rules: [rule]).check(id) }.not_to raise_error

    end

  end



  describe "Scenario S: Redis unavailable" do

    it "returns error Result and logs a warning when Redis is unavailable" do

      allow(raw_redis).to receive(:incr).and_raise(RuntimeError, "connection refused")



      allow(raw_redis).to receive(:pipelined).and_raise(RuntimeError, "connection refused")

      logger = instance_double(Labkit::Logging::JsonLogger)

      expect(logger).to receive(:warn).with(

        hash_including(message: "rate_limit_error", error: "RuntimeError")
@@ -163,72 +157,58 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
  describe "non-matching rules" do

    it "does not write to Redis and returns no-match Result" do

      rule = make_rule(match: { user: 999 })

      expect(raw_redis).not_to receive(:incr)



      expect(raw_redis).not_to receive(:pipelined)

      result = evaluator(rules: [rule]).check(identifier)

      expect(result.matched?).to be(false)

    end

  end



  describe "metrics emission", :with_metrics_config do

    let(:calls_total) { Labkit::RateLimit::Metrics.calls_total }

    let(:errors_total) { Labkit::RateLimit::Metrics.errors_total }

    let(:limit_gauge) { Labkit::RateLimit::Metrics.limit_gauge }

    let(:period_gauge) { Labkit::RateLimit::Metrics.period_gauge }

    let(:metrics) { Labkit::RateLimit::Metrics }



    context "when a rule matches and is not exceeded" do

      it "increments calls_total with action: allow" do

        rule = make_rule(name: "api_rule", limit: 100, period: 60)

        allow(raw_redis).to receive(:incr).and_return(1)

        allow(raw_redis).to receive(:expire)



        allow(raw_redis).to receive(:pipelined).and_return([1, 55])

        evaluator(rules: [rule]).check(identifier)



        expect(calls_total.get(rate_limiter: "rack_request", rule: "api_rule", action: "allow")).to eq(1.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "api_rule", action: "allow")).to eq(1.0)

      end



      it "sets the limit gauge with the resolved value" do

        rule = make_rule(name: "api_rule", limit: 100, period: 60)

        allow(raw_redis).to receive(:incr).and_return(1)

        allow(raw_redis).to receive(:expire)



        allow(raw_redis).to receive(:pipelined).and_return([1, 55])

        evaluator(rules: [rule]).check(identifier)



        expect(limit_gauge.get(rate_limiter: "rack_request", rule: "api_rule")).to eq(100.0)

        expect(metrics.limit_gauge.get(rate_limiter: "rack_request", rule: "api_rule")).to eq(100.0)

      end



      it "sets the period gauge with the resolved value" do

        rule = make_rule(name: "api_rule", limit: 100, period: 120)

        allow(raw_redis).to receive(:incr).and_return(1)

        allow(raw_redis).to receive(:expire)



        allow(raw_redis).to receive(:pipelined).and_return([1, 55])

        evaluator(rules: [rule]).check(identifier)



        expect(period_gauge.get(rate_limiter: "rack_request", rule: "api_rule")).to eq(120.0)

        expect(metrics.period_gauge.get(rate_limiter: "rack_request", rule: "api_rule")).to eq(120.0)

      end

    end



    context "when a rule matches and is exceeded with action: :block" do

      it "increments calls_total with action: block" do

        rule = make_rule(name: "api_rule", limit: 5, action: :block)

        allow(raw_redis).to receive(:incr).and_return(6)

        allow(raw_redis).to receive(:expire)



        allow(raw_redis).to receive(:pipelined).and_return([6, 55])

        evaluator(rules: [rule]).check(identifier)



        expect(calls_total.get(rate_limiter: "rack_request", rule: "api_rule", action: "block")).to eq(1.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "api_rule", action: "block")).to eq(1.0)

      end

    end



    context "when a rule matches and is exceeded with action: :log" do

      it "increments calls_total with action: log" do

        rule = make_rule(name: "api_rule", limit: 5, action: :log)

        allow(raw_redis).to receive(:incr).and_return(6)

        allow(raw_redis).to receive(:expire)



        allow(raw_redis).to receive(:pipelined).and_return([6, 55])

        evaluator(rules: [rule]).check(identifier)



        expect(calls_total.get(rate_limiter: "rack_request", rule: "api_rule", action: "log")).to eq(1.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "api_rule", action: "log")).to eq(1.0)

      end

    end
@@ -238,7 +218,7 @@ RSpec.describe Labkit::RateLimit::Evaluator do 


        evaluator(rules: [rule]).check(identifier)



        expect(calls_total.get(rate_limiter: "rack_request", rule: "unmatched", action: "allow")).to eq(1.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "unmatched", action: "allow")).to eq(1.0)

      end



      it "does not set the limit or period gauges" do
@@ -246,33 +226,30 @@ RSpec.describe Labkit::RateLimit::Evaluator do 


        evaluator(rules: [rule]).check(identifier)



        expect(limit_gauge.get(rate_limiter: "rack_request", rule: "default")).to eq(0.0)

        expect(metrics.limit_gauge.get(rate_limiter: "rack_request", rule: "default")).to eq(0.0)

      end

    end



    context "when Redis fails" do

      it "increments errors_total and does not increment calls_total" do

        rule = make_rule(name: "err_rule")

        allow(raw_redis).to receive(:incr).and_raise(RuntimeError, "connection refused")



        allow(raw_redis).to receive(:pipelined).and_raise(RuntimeError, "connection refused")

        evaluator(rules: [rule]).check(identifier)



        expect(errors_total.get(rate_limiter: "rack_request")).to eq(1.0)

        expect(calls_total.get(rate_limiter: "rack_request", rule: "err_rule", action: "allow")).to eq(0.0)

        expect(calls_total.get(rate_limiter: "rack_request", rule: "err_rule", action: "block")).to eq(0.0)

        expect(metrics.errors_total.get(rate_limiter: "rack_request")).to eq(1.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "err_rule", action: "allow")).to eq(0.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "err_rule", action: "block")).to eq(0.0)

      end

    end



    context "with callable limit and period" do

      it "sets gauges with the resolved integer values" do

        rule = make_rule(name: "callable_rule", limit: -> { 42 }, period: -> { 300 })

        allow(raw_redis).to receive(:incr).and_return(1)

        allow(raw_redis).to receive(:expire)



        allow(raw_redis).to receive(:pipelined).and_return([1, 55])

        evaluator(rules: [rule]).check(identifier)



        expect(limit_gauge.get(rate_limiter: "rack_request", rule: "callable_rule")).to eq(42.0)

        expect(period_gauge.get(rate_limiter: "rack_request", rule: "callable_rule")).to eq(300.0)

        expect(metrics.limit_gauge.get(rate_limiter: "rack_request", rule: "callable_rule")).to eq(42.0)

        expect(metrics.period_gauge.get(rate_limiter: "rack_request", rule: "callable_rule")).to eq(300.0)

      end

    end
@@ -280,8 +257,148 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      it "increments calls_total with unmatched" do

        evaluator(rules: []).check(identifier)



        expect(calls_total.get(rate_limiter: "rack_request", rule: "unmatched", action: "allow")).to eq(1.0)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "unmatched", action: "allow")).to eq(1.0)

      end

    end

  end



  describe "Stage 1c: remaining / reset_at / limit fields" do

    it "populates remaining as limit minus count (Scenario A)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      allow(raw_redis).to receive(:pipelined).and_return([3, 55])



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.remaining).to eq(7)

      expect(result.info.resolved_limit).to eq(10)

    end



    it "floors remaining at 0 when count exceeds limit (Scenario B)" do

      rule = make_rule(name: "r", limit: 5, period: 60)

      allow(raw_redis).to receive(:pipelined).and_return([8, 55])



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.remaining).to eq(0)

    end



    it "sets remaining to 0 and exceeded to false when count equals limit (boundary)" do

      rule = make_rule(name: "r", limit: 5, period: 60)

      allow(raw_redis).to receive(:pipelined).and_return([5, 55])



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.remaining).to eq(0)

      expect(result.exceeded?).to be(false)

    end



    it "derives reset_at from pipelined ttl when TTL > 0 (Scenario F)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      allow(raw_redis).to receive(:pipelined).and_return([1, 45])



      freeze_time = Time.now.utc

      allow(Time).to receive(:now).and_return(freeze_time)



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.reset_at).to eq(freeze_time.utc + 45)

    end



    it "falls back to period when pipelined ttl returns -1 (no expiry) (Scenario G)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([1, -1])



      freeze_time = Time.now.utc

      allow(Time).to receive(:now).and_return(freeze_time)



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.reset_at).to eq(freeze_time.utc + 60)

    end



    it "falls back to period when pipelined ttl returns -2 (key missing) (Scenario H)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([1, -2])



      freeze_time = Time.now.utc

      allow(Time).to receive(:now).and_return(freeze_time)



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.reset_at).to eq(freeze_time.utc + 60)

    end



    it "sets reset_at to Time.now when pipelined ttl returns 0 (Scenario I)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([1, 0])



      freeze_time = Time.now.utc

      allow(Time).to receive(:now).and_return(freeze_time)



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info.reset_at).to be_within(1).of(freeze_time)

    end



    it "still calls redis.expire on first write (Scenario J)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      expect(raw_redis).to receive(:expire).with(anything, 60)



      evaluator(rules: [rule]).check(identifier)

    end



    it "fails open and logs when pipelined call raises after incr (Scenario N)" do

      rule = make_rule(name: "r", limit: 10, period: 60)

      allow(raw_redis).to receive(:pipelined).and_raise(RuntimeError, "connection lost")



      logger = instance_double(Labkit::Logging::JsonLogger)

      expect(logger).to receive(:warn).with(hash_including(message: "rate_limit_error"))



      ev = described_class.new(name: "rack_request", rules: [rule], redis: redis, logger: logger)

      result = ev.check(identifier)



      expect(result.error?).to be(true)

      expect(result.matched?).to be(false)

    end



    it "returns nil for new fields when no rules match (Scenario L)" do

      rule = make_rule(match: { user: 999 })



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info).to be_nil

    end



    it "returns nil for new fields on Redis error (Scenario M)" do

      rule = make_rule(name: "r")

      allow(raw_redis).to receive(:pipelined).and_raise(RuntimeError, "down")



      result = evaluator(rules: [rule]).check(identifier)



      expect(result.info).to be_nil

      expect(result.error?).to be(true)

    end

  end



  describe "Scenario Q: no per-request logging in the success path" do

    it "does not call the logger when a rule matches and Redis is healthy" do

      rule = make_rule(name: "r", limit: 10, period: 60)



      logger = instance_double(Labkit::Logging::JsonLogger)

      expect(logger).not_to receive(:warn)



      ev = described_class.new(name: "rack_request", rules: [rule], redis: redis, logger: logger)

      ev.check(identifier)

    end



    it "does not call the logger when no rules match" do

      rule = make_rule(match: { user: 999 })



      logger = instance_double(Labkit::Logging::JsonLogger)

      expect(logger).not_to receive(:warn)



      ev = described_class.new(name: "rack_request", rules: [rule], redis: redis, logger: logger)

      ev.check(identifier)

    end

  end

end

spec/labkit/rate_limit/limiter_spec.rb

+5 −4
Original line number Diff line number Diff line
@@ -6,7 +6,7 @@ require "redis" 
RSpec.describe Labkit::RateLimit::Limiter do

  include StubENV



  let(:raw_redis) { instance_double(Redis, incr: 1, expire: true) }

  let(:raw_redis) { instance_double(Redis) }

  let(:redis) { PooledRedis.new(raw_redis) }

  let(:logger) { instance_double(Logger, info: nil, warn: nil) }
@@ -23,6 +23,8 @@ RSpec.describe Labkit::RateLimit::Limiter do 


  before do

    stub_env("RAILS_ENV", "test")

    allow(raw_redis).to receive(:pipelined).and_return([1, 55])

    allow(raw_redis).to receive(:expire)

  end



  describe "Scenario U: name validation" do
@@ -75,14 +77,13 @@ RSpec.describe Labkit::RateLimit::Limiter do 


    it "uses the lambda return value as the operative limit" do

      r = rule(limit: -> { 5 }, action: :block)

      allow(raw_redis).to receive(:incr).and_return(5)



      allow(raw_redis).to receive(:pipelined).and_return([5, 55])

      result = described_class.new(name: "rack_request", rules: [r], redis: redis, logger: logger)

        .check({ user: 1 })



      expect(result.exceeded?).to be(false)



      allow(raw_redis).to receive(:incr).and_return(6)

      allow(raw_redis).to receive(:pipelined).and_return([6, 55])

      result2 = described_class.new(name: "rack_request", rules: [r], redis: redis, logger: logger)

        .check({ user: 1 })