From a24a888afe5652e391ce2ea682596686af5ec58b Mon Sep 17 00:00:00 2001 From: wycats Date: Sat, 27 Mar 2010 02:59:10 -0700 Subject: [PATCH] Limit Array#extract_options! to directl instances of Hash and HWIA. Add extractable_options? to Hash so that subclasses of Hash can opt-into extractable behavior. This fixes an issue where respond_with wasn't working with subclasses of Hash that were provided by other libraries (such as CouchDB or Mashie) [#4145 state:resolved] --- .../core_ext/array/extract_options.rb | 17 +++++++++- .../hash_with_indifferent_access.rb | 4 +++ activesupport/test/core_ext/array_ext_test.rb | 34 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/array/extract_options.rb b/activesupport/lib/active_support/core_ext/array/extract_options.rb index 9ca32dc7aa..40ceb3eb9e 100644 --- a/activesupport/lib/active_support/core_ext/array/extract_options.rb +++ b/activesupport/lib/active_support/core_ext/array/extract_options.rb @@ -1,3 +1,14 @@ +class Hash + # By default, only instances of Hash itself are extractable. + # Subclasses of Hash may implement this method and return + # true to declare themselves as extractable. If a Hash + # is extractable, Array#extract_options! pops it from + # the Array when it is the last element of the Array. + def extractable_options? + instance_of?(Hash) + end +end + class Array # Extracts options from a set of arguments. Removes and returns the last # element in the array if it's a hash, otherwise returns a blank hash. @@ -9,6 +20,10 @@ class Array # options(1, 2) # => {} # options(1, 2, :a => :b) # => {:a=>:b} def extract_options! - last.is_a?(::Hash) ? pop : {} + if last.is_a?(Hash) && last.extractable_options? + pop + else + {} + end end end diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 543dab4a75..8241b69c8b 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -4,6 +4,10 @@ module ActiveSupport class HashWithIndifferentAccess < Hash + def extractable_options? + true + end + def initialize(constructor = {}) if constructor.is_a?(Hash) super() diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb index b374eca370..aecc644549 100644 --- a/activesupport/test/core_ext/array_ext_test.rb +++ b/activesupport/test/core_ext/array_ext_test.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/big_decimal' require 'active_support/core_ext/object/conversions' require 'active_support/core_ext' # FIXME: pulling in all to_xml extensions +require 'active_support/hash_with_indifferent_access' class ArrayExtAccessTests < Test::Unit::TestCase def test_from @@ -294,12 +295,45 @@ class ArrayToXmlTests < Test::Unit::TestCase end class ArrayExtractOptionsTests < Test::Unit::TestCase + class HashSubclass < Hash + end + + class ExtractableHashSubclass < Hash + def extractable_options? + true + end + end + def test_extract_options assert_equal({}, [].extract_options!) assert_equal({}, [1].extract_options!) assert_equal({:a=>:b}, [{:a=>:b}].extract_options!) assert_equal({:a=>:b}, [1, {:a=>:b}].extract_options!) end + + def test_extract_options_doesnt_extract_hash_subclasses + hash = HashSubclass.new + hash[:foo] = 1 + array = [hash] + options = array.extract_options! + assert_equal({}, options) + assert_equal [hash], array + end + + def test_extract_options_extracts_extractable_subclass + hash = ExtractableHashSubclass.new + hash[:foo] = 1 + array = [hash] + options = array.extract_options! + assert_equal({:foo => 1}, options) + assert_equal [], array + end + + def test_extract_options_extracts_hwia + hash = [{:foo => 1}.with_indifferent_access] + options = hash.extract_options! + assert_equal 1, options[:foo] + end end class ArrayUniqByTests < Test::Unit::TestCase