Commit 2f4bc36a authored by Sam Wiskow's avatar Sam Wiskow
Browse files

refactor(rate_limit): rename Matcher.from -> Matcher.build 

The factory method on the private Matcher value object was named
.from(input). RuboCop's CodeReuse/ActiveRecord cop (inherited from
gitlab-styles) treats `from` as an ActiveRecord query method and flags
the call site in Rule.new, even though it is plain Hash#transform_values
calling a class method on a non-ActiveRecord object.

Renaming the factory to .build sidesteps the cop without an inline
disable. .build is the more conventional Ruby name for a factory that
normalizes/coerces an input into an instance, and it has no false
collision with ActiveRecord's query API.

No behaviour change; pure rename of an `@api private` method only
called from Rule.new and the spec suite.

Co-Authored-By: default avatarClaude Opus 4.7 (1M context) <noreply@anthropic.com>
parent 2d2b1f49

lib/labkit/rate_limit/matcher.rb

+2 −2
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@ module Labkit 


    # Matcher is the internal representation of a single key/value predicate in

    # a Rule#match hash. Rule.new normalizes every match value through

    # Matcher.from; the Evaluator calls Matcher#match? per identifier value.

    # Matcher.build; the Evaluator calls Matcher#match? per identifier value.

    #

    # Accepted input shapes (everything else raises ArgumentError):

    #   - any plain value (String, Symbol, Integer, ...) -> :equality matcher
@@ -18,7 +18,7 @@ module Labkit 
    # full contract; glob support is intentionally out of scope.

    # @api private

    Matcher = Data.define(:type, :value) do

      def self.from(input)

      def self.build(input)

        case input

        when Regexp

          new(type: :regex, value: input)

lib/labkit/rate_limit/rule.rb

+1 −1
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ module Labkit 


        super(

          name: name_str.freeze,

          match: match.transform_keys(&:to_sym).transform_values { |v| Matcher.from(v) }.freeze, # rubocop:disable CodeReuse/ActiveRecord

          match: match.transform_keys(&:to_sym).transform_values { |v| Matcher.build(v) }.freeze,

          limit: limit,

          period: period,

          action: action_sym,

spec/labkit/rate_limit/matcher_spec.rb

+28 −28
Original line number Diff line number Diff line
@@ -3,40 +3,40 @@ 
require "spec_helper"



RSpec.describe Labkit::RateLimit::Matcher do

  describe ".from" do

  describe ".build" do

    context "with plain values (equality matchers)" do

      it "wraps a String as :equality" do

        m = described_class.from("/api/v4/projects")

        m = described_class.build("/api/v4/projects")

        expect(m.type).to eq(:equality)

        expect(m.value).to eq("/api/v4/projects")

      end



      it "wraps a Symbol as :equality" do

        m = described_class.from(:api)

        m = described_class.build(:api)

        expect(m.type).to eq(:equality)

        expect(m.value).to eq(:api)

      end



      it "wraps an Integer as :equality" do

        m = described_class.from(42)

        m = described_class.build(42)

        expect(m.type).to eq(:equality)

        expect(m.value).to eq(42)

      end



      it "wraps booleans as :equality" do

        expect(described_class.from(true)).to have_attributes(type: :equality, value: true)

        expect(described_class.from(false)).to have_attributes(type: :equality, value: false)

        expect(described_class.build(true)).to have_attributes(type: :equality, value: true)

        expect(described_class.build(false)).to have_attributes(type: :equality, value: false)

      end



      it "wraps nil as :equality" do

        expect(described_class.from(nil)).to have_attributes(type: :equality, value: nil)

        expect(described_class.build(nil)).to have_attributes(type: :equality, value: nil)

      end

    end



    context "with a bare Regexp (Ruby convenience)" do

      it "stores the Regexp directly as :regex (no recompilation)" do

        re = %r{^/api/v\d+}

        m = described_class.from(re)

        m = described_class.build(re)

        expect(m.type).to eq(:regex)

        expect(m.value).to equal(re) # same object, not a re-compiled copy

      end
@@ -44,62 +44,62 @@ RSpec.describe Labkit::RateLimit::Matcher do 


    context "with the canonical { regex: \"...\" } shape" do

      it "compiles the source into a Regexp at construction time" do

        m = described_class.from(regex: "^/api/v\\d+/projects")

        m = described_class.build(regex: "^/api/v\\d+/projects")

        expect(m.type).to eq(:regex)

        expect(m.value).to be_a(Regexp)

        expect(m.value.source).to eq("^/api/v\\d+/projects")

      end



      it "accepts a string-keyed hash too (mirrors YAML.safe_load output)" do

        m = described_class.from("regex" => "^/api/v\\d+/projects")

        m = described_class.build("regex" => "^/api/v\\d+/projects")

        expect(m.type).to eq(:regex)

        expect(m.value.source).to eq("^/api/v\\d+/projects")

      end



      it "raises ArgumentError when the regex source is not a String" do

        expect { described_class.from(regex: 42) }

        expect { described_class.build(regex: 42) }

          .to raise_error(ArgumentError, /requires a String source/)

      end



      it "raises ArgumentError when the regex source is nil" do

        expect { described_class.from(regex: nil) }

        expect { described_class.build(regex: nil) }

          .to raise_error(ArgumentError, /requires a String source/)

      end



      it "raises ArgumentError on invalid regex syntax (RegexpError wrapped)" do

        expect { described_class.from(regex: "[unclosed") }

        expect { described_class.build(regex: "[unclosed") }

          .to raise_error(ArgumentError, /failed to compile/)

      end



      it "raises ArgumentError when the regex source exceeds 200 characters" do

        expect { described_class.from(regex: "a" * 201) }

        expect { described_class.build(regex: "a" * 201) }

          .to raise_error(ArgumentError, /exceeds 200/)

      end

    end



    context "with unsupported shapes (Scenario H)" do

      it "rejects an Array" do

        expect { described_class.from(["/a", "/b"]) }

        expect { described_class.build(["/a", "/b"]) }

          .to raise_error(ArgumentError, /single-key Hash/)

      end



      it "rejects an empty Hash (no type key)" do

        expect { described_class.from({}) }

        expect { described_class.build({}) }

          .to raise_error(ArgumentError, /single-key Hash/)

      end



      it "rejects a multi-key Hash" do

        expect { described_class.from(regex: "^/a", other: "/a") }

        expect { described_class.build(regex: "^/a", other: "/a") }

          .to raise_error(ArgumentError, /single-key Hash/)

      end



      it "rejects { glob: \"...\" } as an unknown type key" do

        expect { described_class.from(glob: "/api/v*/projects") }

        expect { described_class.build(glob: "/api/v*/projects") }

          .to raise_error(ArgumentError, /unknown type key/)

      end



      it "rejects { prefix: \"...\" } as an unknown type key" do

        expect { described_class.from(prefix: "/a") }

        expect { described_class.build(prefix: "/a") }

          .to raise_error(ArgumentError, /unknown type key/)

      end

    end
@@ -107,7 +107,7 @@ RSpec.describe Labkit::RateLimit::Matcher do 


  describe "#match?" do

    context "with an :equality matcher" do

      let(:matcher) { described_class.from("api") }

      let(:matcher) { described_class.build("api") }



      it "returns true for an equal value" do

        expect(matcher.match?("api")).to be(true)
@@ -122,14 +122,14 @@ RSpec.describe Labkit::RateLimit::Matcher do 
      end



      it "supports non-String identifier values (Integer, boolean, ...)" do

        m = described_class.from(42)

        m = described_class.build(42)

        expect(m.match?(42)).to be(true)

        expect(m.match?(43)).to be(false)

      end

    end



    context "with a :regex matcher" do

      let(:matcher) { described_class.from(regex: "^/api/v\\d+/projects") }

      let(:matcher) { described_class.build(regex: "^/api/v\\d+/projects") }



      it "returns true when the identifier value matches" do

        expect(matcher.match?("/api/v4/projects/42")).to be(true)
@@ -156,8 +156,8 @@ RSpec.describe Labkit::RateLimit::Matcher do 


    context "with a :regex matcher built from a bare Regexp" do

      it "matches identically to the canonical { regex: \"...\" } form" do

        from_hash = described_class.from(regex: "^/api/v\\d+/projects")

        from_bare = described_class.from(%r{^/api/v\d+/projects})

        from_hash = described_class.build(regex: "^/api/v\\d+/projects")

        from_bare = described_class.build(%r{^/api/v\d+/projects})



        ["/api/v4/projects/42", "/admin", "no slashes"].each do |v|

          expect(from_hash.match?(v)).to eq(from_bare.match?(v)), "diverged on #{v.inspect}"
@@ -168,14 +168,14 @@ RSpec.describe Labkit::RateLimit::Matcher do 


  describe "value equality (Data.define)" do

    it "treats two equality matchers with the same value as equal" do

      first = described_class.from("api")

      second = described_class.from("api")

      first = described_class.build("api")

      second = described_class.build("api")

      expect(first).to eq(second)

    end



    it "treats two regex matchers with the same compiled source as equal (Scenario I parity)" do

      ruby_form = described_class.from(regex: "^/api/v\\d+")

      yaml_form = described_class.from("regex" => "^/api/v\\d+")

      ruby_form = described_class.build(regex: "^/api/v\\d+")

      yaml_form = described_class.build("regex" => "^/api/v\\d+")

      expect(ruby_form).to eq(yaml_form)

    end

  end

spec/labkit/rate_limit/rule_spec.rb

+1 −1
Original line number Diff line number Diff line
@@ -104,7 +104,7 @@ RSpec.describe Labkit::RateLimit::Rule do 
    it "symbolizes match keys and wraps values in Matcher" do

      rule = valid_rule(match: { "endpoint" => "/api/v4" })

      expect(rule.match.keys).to eq([:endpoint])

      expect(rule.match[:endpoint]).to eq(Labkit::RateLimit::Matcher.from("/api/v4"))

      expect(rule.match[:endpoint]).to eq(Labkit::RateLimit::Matcher.build("/api/v4"))

    end



    it "symbolizes characteristics" do