diff --git a/.rubocop/rspec.yml b/.rubocop/rspec.yml index 51711080..29ae542e 100644 --- a/.rubocop/rspec.yml +++ b/.rubocop/rspec.yml @@ -1,5 +1,6 @@ RSpec/ExampleLength: CountAsOne: - array + - hash - heredoc - method_call diff --git a/Gemfile b/Gemfile index dc6363ee..9db61f1b 100644 --- a/Gemfile +++ b/Gemfile @@ -37,6 +37,7 @@ group :test do gem "rspec", "~> 3.10" gem "rspec-its" + gem "rspec-memory" gem "yardstick" end diff --git a/lib/http/headers.rb b/lib/http/headers.rb index 89ff78b2..5f285616 100644 --- a/lib/http/headers.rb +++ b/lib/http/headers.rb @@ -13,6 +13,33 @@ class Headers extend Forwardable include Enumerable + class << self + # Coerces given `object` into Headers. + # + # @raise [Error] if object can't be coerced + # @param [#to_hash, #to_h, #to_a] object + # @return [Headers] + def coerce(object) + unless object.is_a? self + object = case + when object.respond_to?(:to_hash) then object.to_hash + when object.respond_to?(:to_h) then object.to_h + when object.respond_to?(:to_a) then object.to_a + else raise Error, "Can't coerce #{object.inspect} to Headers" + end + end + + headers = new + object.each { |k, v| headers.add k, v } + headers + end + alias [] coerce + + def normalizer + @normalizer ||= Headers::Normalizer.new + end + end + # Class constructor. def initialize # The @pile stores each header value using a three element array: @@ -188,40 +215,11 @@ def merge(other) dup.tap { |dupped| dupped.merge! other } end - class << self - # Coerces given `object` into Headers. - # - # @raise [Error] if object can't be coerced - # @param [#to_hash, #to_h, #to_a] object - # @return [Headers] - def coerce(object) - unless object.is_a? self - object = case - when object.respond_to?(:to_hash) then object.to_hash - when object.respond_to?(:to_h) then object.to_h - when object.respond_to?(:to_a) then object.to_a - else raise Error, "Can't coerce #{object.inspect} to Headers" - end - end - - headers = new - object.each { |k, v| headers.add k, v } - headers - end - alias [] coerce - end - private - class << self - def header_normalizer - @header_normalizer ||= Headers::Normalizer.new - end - end - # Transforms `name` to canonical HTTP header capitalization def normalize_header(name) - self.class.header_normalizer.normalize(name) + self.class.normalizer.call(name) end # Ensures there is no new line character in the header value diff --git a/lib/http/headers/normalizer.rb b/lib/http/headers/normalizer.rb index 7c260d0d..b4a53b42 100644 --- a/lib/http/headers/normalizer.rb +++ b/lib/http/headers/normalizer.rb @@ -10,15 +10,40 @@ class Normalizer # @see http://tools.ietf.org/html/rfc7230#section-3.2 COMPLIANT_NAME_RE = /\A[A-Za-z0-9!#$%&'*+\-.^_`|~]+\z/ - MAX_CACHE_SIZE = 200 + NAME_PARTS_SEPARATOR_RE = /[\-_]/ + + # @private + # Normalized header names cache + class Cache + MAX_SIZE = 200 + + def initialize + @store = {} + end + + def get(key) + @store[key] + end + alias [] get + + def set(key, value) + # Maintain cache size + @store.delete(@store.each_key.first) while MAX_SIZE <= @store.size + + @store[key] = value + end + alias []= set + end def initialize - @cache = Cache.new(MAX_CACHE_SIZE) + @cache = Cache.new end # Transforms `name` to canonical HTTP header capitalization - def normalize(name) - @cache[name] ||= normalize_header(name) + def call(name) + name = -name.to_s + value = (@cache[name] ||= -normalize_header(name)) + value.dup end private @@ -32,51 +57,12 @@ def normalize(name) def normalize_header(name) return name if CANONICAL_NAME_RE.match?(name) - normalized = name.split(/[\-_]/).each(&:capitalize!).join("-") + normalized = name.split(NAME_PARTS_SEPARATOR_RE).each(&:capitalize!).join("-") return normalized if COMPLIANT_NAME_RE.match?(normalized) raise HeaderError, "Invalid HTTP header field name: #{name.inspect}" end - - class Cache - def initialize(max_size) - @max_size = max_size - @cache = {} - end - - def get(key) - @cache[key] - end - - def set(key, value) - @cache[key] = value - - # Maintain cache size - return unless @cache.size > @max_size - - oldest_key = @cache.keys.first - @cache.delete(oldest_key) - end - - def size - @cache.size - end - - def key?(key) - @cache.key?(key) - end - - def [](key) - get(key) - end - - def []=(key, value) - set(key, value) - end - end - - private_constant :Cache end end end diff --git a/spec/lib/http/headers/normalizer_spec.rb b/spec/lib/http/headers/normalizer_spec.rb index d18cd95e..a512af08 100644 --- a/spec/lib/http/headers/normalizer_spec.rb +++ b/spec/lib/http/headers/normalizer_spec.rb @@ -3,22 +3,58 @@ RSpec.describe HTTP::Headers::Normalizer do subject(:normalizer) { described_class.new } - describe "#normalize" do + include_context RSpec::Memory + + describe "#call" do it "normalizes the header" do - expect(normalizer.normalize("content_type")).to eq "Content-Type" + expect(normalizer.call("content_type")).to eq "Content-Type" + end + + it "returns a non-frozen string" do + expect(normalizer.call("content_type")).not_to be_frozen end - it "caches normalized headers" do - object_id = normalizer.normalize("content_type").object_id - expect(object_id).to eq normalizer.normalize("content_type").object_id + it "evicts the oldest item when cache is full" do + max_headers = (1..described_class::Cache::MAX_SIZE).map { |i| "Header#{i}" } + max_headers.each { |header| normalizer.call(header) } + normalizer.call("New-Header") + cache_store = normalizer.instance_variable_get(:@cache).instance_variable_get(:@store) + expect(cache_store.keys).to eq(max_headers[1..] + ["New-Header"]) end - it "only caches up to MAX_CACHE_SIZE headers" do - (1..described_class::MAX_CACHE_SIZE + 1).each do |i| - normalizer.normalize("header#{i}") + describe "multiple invocations with the same input" do + let(:normalized_values) { Array.new(3) { normalizer.call("content_type") } } + + it "returns the same result each time" do + expect(normalized_values.uniq.size).to eq 1 + end + + it "returns different string objects each time" do + expect(normalized_values.map(&:object_id).uniq.size).to eq normalized_values.size end + end + + it "limits allocation counts for first normalization of a header" do + expected_allocations = { + Array => 1, + described_class => 1, + Hash => 1, + described_class::Cache => 1, + MatchData => 1, + String => 6 + } + + expect do + normalizer.call("content_type") + end.to limit_allocations(**expected_allocations) + end + + it "allocates minimal memory for subsequent normalization of the same header" do + normalizer.call("content_type") - expect(normalizer.instance_variable_get(:@cache).size).to eq described_class::MAX_CACHE_SIZE + expect do + normalizer.call("content_type") + end.to limit_allocations(String => 1) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1bb40781..1bdbf00b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,7 @@ require "http" require "rspec/its" +require "rspec/memory" require "support/capture_warning" require "support/fakeio"