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]

3 files changed, 54 insertions, 1 deletions

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