Commit 752be81b authored by Sam Wiskow's avatar Sam Wiskow
Browse files

fix: resolve rubocop violations in spec files 

- Move FakeRedis class outside RSpec.describe block
  (Lint/ConstantDefinitionInBlock, RSpec/LeakyConstantDeclaration)
- Place required keyword param (rules:) before optional params in check()
  (Style/KeywordParametersOrder)
- Expand single-line before hooks to multi-line (RSpec/SingleLineHook)
- Replace ASCII arrows in comments (Style/AsciiComments)
- Replace multi-line {..} blocks with do..end (Style/BlockDelimiters)
- Replace string concatenation with interpolation (Style/StringConcatenation)
- Use single-quoted strings inside interpolations
  (Style/StringLiteralsInInterpolation)
- Use be(true) instead of eq(true) (RSpec/BeEq)
parent 6d96a062

spec/labkit/rate_limit/evaluator_spec.rb

+3 −3
Original line number Diff line number Diff line
@@ -58,8 +58,8 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
    end



    it "produces different keys for two distinct long values sharing a prefix" do

      val_a = "a" + "x" * 200

      val_b = "b" + "x" * 200

      val_a = "a#{'x' * 200}"

      val_b = "b#{'x' * 200}"



      hash_a = OpenSSL::Digest::SHA256.hexdigest(val_a)

      hash_b = OpenSSL::Digest::SHA256.hexdigest(val_b)
@@ -161,7 +161,7 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
        expect(data["limit"]).to eq(100)

        expect(data["period"]).to eq(60)

        expect(data["count"]).to eq(42)

        expect(data["matched"]).to eq(true)

        expect(data["matched"]).to be(true)

        expect(data).to have_key("exceeded")

        expect(data["identifier"]).to be_a(Hash)

        expect(data["redis_key"]).to eq("labkit:rl:rack_request:0:user:42")

spec/labkit/rate_limit_spec.rb

+38 −31
Original line number Diff line number Diff line
@@ -3,10 +3,8 @@ 
require "spec_helper"

require "redis"



RSpec.describe Labkit::RateLimit do

  include StubENV



  # A minimal in-memory Redis fake for integration tests

# A minimal in-memory Redis fake for integration tests.

# Defined at top level to avoid RSpec/LeakyConstantDeclaration.

class FakeRedis

  def initialize

    @store = Hash.new(0)
@@ -25,10 +23,14 @@ RSpec.describe Labkit::RateLimit do 
  end

end



RSpec.describe Labkit::RateLimit do

  include StubENV



  let(:redis) { FakeRedis.new }

  let(:logger) { instance_double(Logger, info: nil, warn: nil) }



  def check(call_site: "rack_request", identifier: { user: 42, ip: "1.2.3.4" }, rules:)

  # required keyword params first, optional last

  def check(rules:, call_site: "rack_request", identifier: { user: 42, ip: "1.2.3.4" })

    described_class.check(

      call_site: call_site,

      identifier: identifier,
@@ -45,7 +47,9 @@ RSpec.describe Labkit::RateLimit do 
    )

  end



  before { stub_env("LABKIT_ENV", "test") }

  before do

    stub_env("LABKIT_ENV", "test")

  end



  # Scenario 1: Identifier round-trips

  describe "scenario 1: Identifier round-trip" do
@@ -79,7 +83,7 @@ RSpec.describe Labkit::RateLimit do 
    end

  end



  # Scenario 4: Exceeded :block rule → :block; :log-action counted independently

  # Scenario 4: Exceeded :block rule -> :block; :log-action counted independently

  describe "scenario 4: exceeded :block rule" do

    it "returns :block when a block-action rule is exceeded" do

      # Pre-fill the counter above the limit
@@ -95,7 +99,7 @@ RSpec.describe Labkit::RateLimit do 
        rule(action: :block, limit: 100),

        rule(action: :log, limit: 50, characteristics: [:ip])

      ]

      result = check(identifier: { user: 42, ip: "1.2.3.4" }, rules: rules)

      result = check(rules: rules, identifier: { user: 42, ip: "1.2.3.4" })

      expect(result).to eq(:allow)

      expect(redis.get("labkit:rl:rack_request:0:user:42")).to eq(1)

      expect(redis.get("labkit:rl:rack_request:1:ip:1.2.3.4")).to eq(1)
@@ -105,7 +109,7 @@ RSpec.describe Labkit::RateLimit do 
      # Pre-fill :log rule's counter above its limit

      51.times { redis.incr("labkit:rl:rack_request:0:ip:1.2.3.4") }

      rules = [rule(action: :log, limit: 50, characteristics: [:ip])]

      result = check(identifier: { user: 42, ip: "1.2.3.4" }, rules: rules)

      result = check(rules: rules, identifier: { user: 42, ip: "1.2.3.4" })

      expect(result).to eq(:allow)

    end
@@ -113,26 +117,26 @@ RSpec.describe Labkit::RateLimit do 
      # Only :ip counter is pre-filled above limit; :user is within limit

      51.times { redis.incr("labkit:rl:rack_request:0:ip:1.2.3.4") }

      rules = [rule(action: :block, limit: 50, characteristics: [:user, :ip])]

      result = check(identifier: { user: 42, ip: "1.2.3.4" }, rules: rules)

      result = check(rules: rules, identifier: { user: 42, ip: "1.2.3.4" })

      expect(result).to eq(:block)

    end

  end



  # Scenario 5: All rules within limit → :allow; all counters incremented

  # Scenario 5: All rules within limit -> :allow; all counters incremented

  describe "scenario 5: all rules within limit" do

    it "returns :allow and increments all counters" do

      rules = [

        rule(match: {}, limit: 100, characteristics: [:user]),

        rule(match: {}, limit: 100, characteristics: [:ip])

      ]

      result = check(identifier: { user: 42, ip: "1.2.3.4" }, rules: rules)

      result = check(rules: rules, identifier: { user: 42, ip: "1.2.3.4" })

      expect(result).to eq(:allow)

      expect(redis.get("labkit:rl:rack_request:0:user:42")).to eq(1)

      expect(redis.get("labkit:rl:rack_request:1:ip:1.2.3.4")).to eq(1)

    end

  end



  # Scenario 6: Redis unavailable → :allow; no exception; WARN log

  # Scenario 6: Redis unavailable -> :allow; no exception; WARN log

  describe "scenario 6: Redis unavailable" do

    it "returns :allow and logs a warning when Redis is unavailable" do

      broken_redis = instance_double(Redis)
@@ -152,7 +156,7 @@ RSpec.describe Labkit::RateLimit do 
    end

  end



  # Scenario 7: Unknown characteristic in test env → ArgumentError

  # Scenario 7: Unknown characteristic in test env -> ArgumentError

  describe "scenario 7: unknown characteristic in test env" do

    it "raises ArgumentError with the characteristic name" do

      rules = [rule(characteristics: [:unknown_key])]
@@ -160,15 +164,17 @@ RSpec.describe Labkit::RateLimit do 
    end

  end



  # Scenario 8: Unknown characteristic in production → WARN; sentinel in key

  # Scenario 8: Unknown characteristic in production -> WARN; sentinel in key

  describe "scenario 8: unknown characteristic in production" do

    before { stub_env("LABKIT_ENV", "production") }

    before do

      stub_env("LABKIT_ENV", "production")

    end



    it "does not raise; logs WARN; uses sentinel in Redis key" do

      real_redis = FakeRedis.new

      rules = [rule(characteristics: [:unknown_key])]



      expect {

      expect do

        described_class.check(

          call_site: "rack_request",

          identifier: { user: 42 },
@@ -176,14 +182,14 @@ RSpec.describe Labkit::RateLimit do 
          redis: real_redis,

          logger: logger

        )

      }.not_to raise_error

      end.not_to raise_error



      expect(logger).to have_received(:warn).with(a_string_including("rate_limit_unknown_characteristic"))

      expect(real_redis.get("labkit:rl:rack_request:0:unknown_key:unknown_characteristic")).to eq(1)

    end

  end



  # Scenario 9: Two rules with same characteristics → distinct Redis keys

  # Scenario 9: Two rules with same characteristics -> distinct Redis keys

  describe "scenario 9: two rules with same characteristics" do

    it "uses distinct rule_index in keys so counters are independent" do

      rules = [
@@ -197,7 +203,7 @@ RSpec.describe Labkit::RateLimit do 
    end

  end



  # Scenario 10: empty match + limit 0 + action :block → :block on any call

  # Scenario 10: empty match + limit 0 + action :block -> :block on any call

  describe "scenario 10: limit 0 blocks immediately" do

    it "returns :block on the first call" do

      rules = [rule(match: {}, limit: 0, action: :block)]
@@ -222,23 +228,25 @@ RSpec.describe Labkit::RateLimit do 
    end

  end



  # Scenario 12: Invalid call_site in test env → ArgumentError

  # Scenario 12: Invalid call_site in test env -> ArgumentError

  describe "scenario 12: invalid call_site in test env" do

    it "raises ArgumentError" do

      expect { check(call_site: "bad:site", rules: [rule]) }

      expect { check(rules: [rule], call_site: "bad:site") }

        .to raise_error(ArgumentError, /Invalid call_site/)

    end

  end



  # Scenario 13: Invalid call_site in production → sanitized; WARN log

  # Scenario 13: Invalid call_site in production -> sanitized; WARN log

  describe "scenario 13: invalid call_site in production sanitized" do

    before { stub_env("LABKIT_ENV", "production") }

    before do

      stub_env("LABKIT_ENV", "production")

    end



    it "sanitizes and WARNs without raising" do

      real_redis = FakeRedis.new

      rules = [rule(match: {}, limit: 100, characteristics: [:user])]



      expect {

      expect do

        described_class.check(

          call_site: "bad:site",

          identifier: { user: 42 },
@@ -246,30 +254,29 @@ RSpec.describe Labkit::RateLimit do 
          redis: real_redis,

          logger: logger

        )

      }.not_to raise_error

      end.not_to raise_error



      expect(logger).to have_received(:warn).with(a_string_including("rate_limit_invalid_call_site"))

      # The Redis key should use the sanitized call_site

      expect(real_redis.get("labkit:rl:bad_site:0:user:42")).to eq(1)

    end

  end



  # Scenario 14: char_value > 200 chars → SHA-256 used; distinct long values → different keys

  # Scenario 14: char_value > 200 chars -> SHA-256 used; distinct long values -> different keys

  describe "scenario 14: long char values use SHA-256" do

    it "uses SHA-256 digest for values longer than 200 chars" do

      long_val = "v" * 201

      id = { user: long_val }

      rules = [rule(match: {}, limit: 100, characteristics: [:user])]



      check(identifier: id, rules: rules)

      check(rules: rules, identifier: id)



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

      expect(redis.get("labkit:rl:rack_request:0:user:#{expected_hash}")).to eq(1)

    end



    it "produces different Redis keys for two long values with shared prefix" do

      val_a = "prefix_" + "a" * 200

      val_b = "prefix_" + "b" * 200

      val_a = "prefix_#{'a' * 200}"

      val_b = "prefix_#{'b' * 200}"



      hash_a = OpenSSL::Digest::SHA256.hexdigest(val_a)

      hash_b = OpenSSL::Digest::SHA256.hexdigest(val_b)