Merge pull request #1279 from mwpastore/full-indifference

Rage Against the Params

lib/sinatra/base.rb

@@ -15,6 +15,7 @@ require 'time'
 require 'uri'
 
 # other files we need
+require 'sinatra/indifferent_hash'
 require 'sinatra/show_exceptions'
 require 'sinatra/version'
 
@@ -240,18 +241,6 @@ module Sinatra
     def http_status; 404 end
   end
 
-  class IndifferentHash < Hash
-    def [](key)
-      value = super(key)
-      return super(key.to_s) if value.nil? && Symbol === key
-      value
-    end
-
-    def has_key?(key)
-      super(key) || (Symbol === key && super(key.to_s))
-    end
-  end
-
   # Methods available to routes, before/after filters, and views.
   module Helpers
     # Set or retrieve the response status code.
@@ -1078,20 +1067,6 @@ module Sinatra
       send_file path, options.merge(:disposition => nil)
     end
 
-    # Enable string or symbol key access to the nested params hash.
-    def indifferent_params(object)
-      case object
-      when Hash
-        new_hash = IndifferentHash.new
-        object.each { |key, value| new_hash[key] = indifferent_params(value) }
-        new_hash
-      when Array
-        object.map { |item| indifferent_params(item) }
-      else
-        object
-      end
-    end
-
     # Run the block with 'throw :halt' support and apply result to the response.
     def invoke
       res = catch(:halt) { yield }
@@ -1110,8 +1085,7 @@ module Sinatra
 
     # Dispatch a request with error handling.
     def dispatch!
-      @params = indifferent_params(@request.params)
-      force_encoding(@params)
+      force_encoding(@params = IndifferentHash[@request.params])
 
       invoke do
         static! if settings.static? && (request.get? || request.head?)

lib/sinatra/indifferent_hash.rb

@@ -0,0 +1,150 @@
+# frozen_string_literal: true
+module Sinatra
+  # A poor man's ActiveSupport::HashWithIndifferentAccess, with all the Rails-y
+  # stuff removed.
+  #
+  # Implements a hash where keys <tt>:foo</tt> and <tt>"foo"</tt> are
+  # considered to be the same.
+  #
+  #   rgb = Sinatra::IndifferentHash.new
+  #
+  #   rgb[:black]    =  '#000000' # symbol assignment
+  #   rgb[:black]  # => '#000000' # symbol retrieval
+  #   rgb['black'] # => '#000000' # string retrieval
+  #
+  #   rgb['white']   =  '#FFFFFF' # string assignment
+  #   rgb[:white]  # => '#FFFFFF' # symbol retrieval
+  #   rgb['white'] # => '#FFFFFF' # string retrieval
+  #
+  # Internally, symbols are mapped to strings when used as keys in the entire
+  # writing interface (calling e.g. <tt>[]=</tt>, <tt>merge</tt>). This mapping
+  # belongs to the public interface. For example, given:
+  #
+  #   hash = Sinatra::IndifferentHash.new(:a=>1)
+  #
+  # You are guaranteed that the key is returned as a string:
+  #
+  #   hash.keys # => ["a"]
+  #
+  # Technically other types of keys are accepted:
+  #
+  #   hash = Sinatra::IndifferentHash.new(:a=>1)
+  #   hash[0] = 0
+  #   hash # => { "a"=>1, 0=>0 }
+  #
+  # But this class is intended for use cases where strings or symbols are the
+  # expected keys and it is convenient to understand both as the same. For
+  # example the +params+ hash in Sinatra.
+  class IndifferentHash < Hash
+    def self.[](*args)
+      new.merge!(Hash[*args])
+    end
+
+    def initialize(*args)
+      super(*args.map(&method(:convert_value)))
+    end
+
+    def default(*args)
+      super(*args.map(&method(:convert_key)))
+    end
+
+    def default=(value)
+      super(convert_value(value))
+    end
+
+    def assoc(key)
+      super(convert_key(key))
+    end
+
+    def rassoc(value)
+      super(convert_value(value))
+    end
+
+    def fetch(key, *args)
+      super(convert_key(key), *args.map(&method(:convert_value)))
+    end
+
+    def [](key)
+      super(convert_key(key))
+    end
+
+    def []=(key, value)
+      super(convert_key(key), convert_value(value))
+    end
+
+    alias_method :store, :[]=
+
+    def key(value)
+      super(convert_value(value))
+    end
+
+    def key?(key)
+      super(convert_key(key))
+    end
+
+    alias_method :has_key?, :key?
+    alias_method :include?, :key?
+    alias_method :member?, :key?
+
+    def value?(value)
+      super(convert_value(value))
+    end
+
+    alias_method :has_value?, :value?
+
+    def delete(key)
+      super(convert_key(key))
+    end
+
+    def dig(key, *other_keys)
+      super(convert_key(key), *other_keys)
+    end if method_defined?(:dig) # Added in Ruby 2.3
+
+    def fetch_values(*keys)
+      super(*keys.map(&method(:convert_key)))
+    end if method_defined?(:fetch_values) # Added in Ruby 2.3
+
+    def values_at(*keys)
+      super(*keys.map(&method(:convert_key)))
+    end
+
+    def merge!(other_hash)
+      return super if other_hash.is_a?(self.class)
+
+      other_hash.each_pair do |key, value|
+        key = convert_key(key)
+        value = yield(key, self[key], value) if block_given? && key?(key)
+        self[key] = convert_value(value)
+      end
+
+      self
+    end
+
+    alias_method :update, :merge!
+
+    def merge(other_hash, &block)
+      dup.merge!(other_hash, &block)
+    end
+
+    def replace(other_hash)
+      super(other_hash.is_a?(self.class) ? other_hash : self.class[other_hash])
+    end
+
+    private
+
+    def convert_key(key)
+      key.is_a?(Symbol) ? key.to_s : key
+    end
+
+    def convert_value(value)
+      case value
+      when Hash
+        value.is_a?(self.class) ? value : self.class[value]
+      when Array
+        value.map(&method(:convert_value))
+      else
+        value
+      end
+    end
+  end
+end

sinatra-contrib/lib/sinatra/config_file.rb

@@ -156,13 +156,12 @@ module Sinatra
     # returned config is a indifferently accessible Hash, which means that you
     # can get its values using Strings or Symbols as keys.
     def config_for_env(hash)
-      if hash.respond_to? :keys and hash.keys.all? { |k| environments.include? k.to_s }
+      if hash.respond_to?(:keys) && hash.keys.all? { |k| environments.include?(k.to_s) }
         hash = hash[environment.to_s] || hash[environment.to_sym]
       end
 
-      if hash.respond_to? :to_hash
-        indifferent_hash = Hash.new { |hash, key| hash[key.to_s] if Symbol === key }
-        indifferent_hash.merge hash.to_hash
+      if hash.respond_to?(:to_hash)
+        IndifferentHash[hash.to_hash]
       else
         hash
       end

test/indifferent_hash_test.rb

@@ -0,0 +1,197 @@
+# frozen_string_literal: true
+#
+# We don't need the full test helper for this standalone class.
+#
+require 'bundler/setup'
+require 'minitest/autorun' unless defined?(Minitest)
+
+require_relative '../lib/sinatra/indifferent_hash'
+
+class TestIndifferentHashBasics < Minitest::Test
+  def test_flattened_constructor
+    hash = Sinatra::IndifferentHash[:a, 1, ?b, 2]
+    assert_equal 1, hash[?a]
+    assert_equal 2, hash[?b]
+  end
+
+  def test_pairs_constructor
+    hash = Sinatra::IndifferentHash[[[:a, 1], [?b, 2]]]
+    assert_equal 1, hash[?a]
+    assert_equal 2, hash[?b]
+  end
+
+  def test_default_block
+    hash = Sinatra::IndifferentHash.new { |h, k| h[k] = k.upcase }
+    assert_nil hash.default
+    assert_equal ?A, hash.default(:a)
+  end
+
+  def test_default_object
+    hash = Sinatra::IndifferentHash.new(:a=>1, ?b=>2)
+    assert_equal({ ?a=>1, ?b=>2 }, hash.default)
+    assert_equal({ ?a=>1, ?b=>2 }, hash[:a])
+  end
+
+  def test_default_assignment
+    hash = Sinatra::IndifferentHash.new
+    hash.default = { :a=>1, ?b=>2 }
+    assert_equal({ ?a=>1, ?b=>2 }, hash.default)
+    assert_equal({ ?a=>1, ?b=>2 }, hash[:a])
+  end
+
+  def test_assignment
+    hash = Sinatra::IndifferentHash.new
+    hash[:a] = :a
+    hash[?b] = :b
+    hash[3] = 3
+    hash[:simple_nested] = { :a=>:a, ?b=>:b }
+
+    assert_equal :a, hash[?a]
+    assert_equal :b, hash[?b]
+    assert_equal 3, hash[3]
+    assert_equal({ ?a=>:a, ?b=>:b }, hash['simple_nested'])
+    assert_nil hash[?d]
+  end
+
+  def test_merge!
+    # merge! is already mostly tested by the different constructors, so we
+    # really just need to test the block form here
+    hash = Sinatra::IndifferentHash[:a=>'a', ?b=>'b', 3=>3]
+    hash.merge!(?a=>'A', :b=>'B', :d=>'D') do |key, oldval, newval|
+      "#{oldval}*#{key}*#{newval}"
+    end
+
+    assert_equal({ ?a=>'a*a*A', ?b=>'b*b*B', 3=>3, ?d=>'D' }, hash)
+  end
+end
+
+class TestIndifferentHash < Minitest::Test
+  def skip_if_lacking(meth)
+    skip "Hash##{meth} not supported on this Ruby" unless Hash.method_defined?(meth)
+  end
+
+  def setup
+    @hash = Sinatra::IndifferentHash[:a=>:a, ?b=>:b, 3=>3,
+      :simple_nested=>{ :a=>:a, ?b=>:b },
+      :nested=>{ :a=>[{ :a=>:a, ?b=>:b }, :c, 4], ?f=>:f, 7=>7 }
+    ]
+  end
+
+  def test_hash_constructor
+    assert_equal :a, @hash[?a]
+    assert_equal :b, @hash[?b]
+    assert_equal 3, @hash[3]
+    assert_equal({ ?a=>:a, ?b=>:b }, @hash['nested'][?a][0])
+    assert_equal :c, @hash['nested'][?a][1]
+    assert_equal 4, @hash['nested'][?a][2]
+    assert_equal :f, @hash['nested'][?f]
+    assert_equal 7, @hash['nested'][7]
+    assert_equal :a, @hash['simple_nested'][?a]
+    assert_equal :b, @hash['simple_nested'][?b]
+    assert_nil @hash[?d]
+  end
+
+  def test_assoc
+    assert_nil @hash.assoc(:d)
+    assert_equal [?a, :a], @hash.assoc(:a)
+    assert_equal [?b, :b], @hash.assoc(:b)
+  end
+
+  def test_rassoc
+    assert_nil @hash.rassoc(:d)
+    assert_equal [?a, :a], @hash.rassoc(:a)
+    assert_equal [?b, :b], @hash.rassoc(:b)
+    assert_equal ['simple_nested', { ?a=>:a, ?b=>:b }], @hash.rassoc(:a=>:a, ?b=>:b)
+  end
+
+  def test_fetch
+    assert_raises(KeyError) { @hash.fetch(:d) }
+    assert_equal 1, @hash.fetch(:d, 1)
+    assert_equal 2, @hash.fetch(:d) { 2 }
+    assert_equal ?d, @hash.fetch(:d) { |k| k }
+    assert_equal :a, @hash.fetch(:a, 1)
+    assert_equal :a, @hash.fetch(:a) { 2 }
+  end
+
+  def test_symbolic_retrieval
+    assert_equal :a, @hash[:a]
+    assert_equal :b, @hash[:b]
+    assert_equal({ ?a=>:a, ?b=>:b }, @hash[:nested][:a][0])
+    assert_equal :c, @hash[:nested][:a][1]
+    assert_equal 4, @hash[:nested][:a][2]
+    assert_equal :f, @hash[:nested][:f]
+    assert_equal 7, @hash[:nested][7]
+    assert_equal :a, @hash[:simple_nested][:a]
+    assert_equal :b, @hash[:simple_nested][:b]
+    assert_nil @hash[:d]
+  end
+
+  def test_key
+    assert_nil @hash.key(:d)
+    assert_equal ?a, @hash.key(:a)
+    assert_equal 'simple_nested', @hash.key(:a=>:a, ?b=>:b)
+  end
+
+  def test_key?
+    assert_operator @hash, :key?, :a
+    assert_operator @hash, :key?, ?b
+    assert_operator @hash, :key?, 3
+    refute_operator @hash, :key?, :d
+  end
+
+  def test_value?
+    assert_operator @hash, :value?, :a
+    assert_operator @hash, :value?, :b
+    assert_operator @hash, :value?, 3
+    assert_operator @hash, :value?, { :a=>:a, ?b=>:b }
+    refute_operator @hash, :value?, :d
+  end
+
+  def test_delete
+    @hash.delete(:a)
+    @hash.delete(?b)
+    assert_nil @hash[:a]
+    assert_nil @hash[?b]
+  end
+
+  def test_dig
+    skip_if_lacking :dig
+
+    assert_equal :a, @hash.dig(:a)
+    assert_equal :b, @hash.dig(?b)
+    assert_nil @hash.dig(:d)
+
+    assert_equal :a, @hash.dig(:simple_nested, :a)
+    assert_equal :b, @hash.dig('simple_nested', ?b)
+    assert_nil @hash.dig('simple_nested', :d)
+
+    assert_equal :a, @hash.dig(:nested, :a, 0, :a)
+    assert_equal :b, @hash.dig('nested', ?a, 0, ?b)
+    assert_nil @hash.dig('nested', ?a, 0, :d)
+  end
+
+  def test_fetch_values
+    skip_if_lacking :fetch_values
+
+    assert_raises(KeyError) { @hash.fetch_values(3, :d) }
+    assert_equal [:a, :b, 3, ?D], @hash.fetch_values(:a, ?b, 3, :d) { |k| k.upcase }
+  end
+
+  def test_values_at
+    assert_equal [:a, :b, 3, nil], @hash.values_at(:a, ?b, 3, :d)
+  end
+
+  def test_merge
+    # merge just calls merge!, which is already thoroughly tested
+    hash2 = @hash.merge(?a=>1, :q=>2) { |key, oldval, newval| "#{oldval}*#{key}*#{newval}" }
+
+    refute_equal @hash, hash2
+    assert_equal 'a*a*1', hash2[:a]
+    assert_equal 2, hash2[?q]
+  end
+
+  def test_replace
+    @hash.replace(?a=>1, :q=>2)
+    assert_equal({ ?a=>1, ?q=>2 }, @hash)
+  end
+end

test/request_test.rb

@@ -38,7 +38,7 @@ class RequestTest < Minitest::Test
       'CONTENT_TYPE' => 'application/x-www-form-urlencoded',
       'rack.input' => StringIO.new('foo=bar')
     )
-    Sinatra::IndifferentHash.new.replace(request.params)
+    Sinatra::IndifferentHash[request.params]
     dumped = Marshal.dump(request.params)
     assert_equal 'bar', Marshal.load(dumped)['foo']
   end