Browse files

Fixed ActiveSupport::OrderedHash #delete_if, #reject!, and #reject, w…

…hich did not sync the @keys after the operation.

This probably holds true for other mutating methods as well.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
1 parent 69387ce commit 7394d12dc7c4bd6c1604e482042efab23f455794 @alloy alloy committed with NZKoz Dec 11, 2008
Showing with 46 additions and 1 deletion.
  1. +25 −0 activesupport/lib/active_support/ordered_hash.rb
  2. +21 −1 activesupport/test/ordered_hash_test.rb
View
25 activesupport/lib/active_support/ordered_hash.rb
@@ -25,6 +25,25 @@ def delete(key)
super
end
+ def delete_if
+ super
+ sync_keys!
+ self
+ end
+
+ def reject!
+ super
+ sync_keys!
+ self
+ end
+
+ def reject(&block)
+ dup.reject!(&block)
+ end
+
+ alias_method :super_keys, :keys
+ private :super_keys
+
def keys
@keys
end
@@ -48,6 +67,12 @@ def each_value
def each
keys.each {|key| yield [key, self[key]]}
end
+
+ private
+
+ def sync_keys!
+ (@keys - super_keys).each { |k| @keys.delete(k) }
+ end
end
end
end
View
22 activesupport/test/ordered_hash_test.rb
@@ -83,4 +83,24 @@ def test_each
def test_each_with_index
@ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair}
end
-end
+
+ def test_delete_if
+ (copy = @ordered_hash.dup).delete('pink')
+ assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' }
+ assert !@ordered_hash.keys.include?('pink')
+ end
+
+ def test_reject!
+ (copy = @ordered_hash.dup).delete('pink')
+ @ordered_hash.reject! { |k, _| k == 'pink' }
+ assert_equal copy, @ordered_hash
+ assert !@ordered_hash.keys.include?('pink')
+ end
+
+ def test_reject
+ copy = @ordered_hash.dup
+ new_ordered_hash = @ordered_hash.reject { |k, _| k == 'pink' }
+ assert_equal copy, @ordered_hash
+ assert !new_ordered_hash.keys.include?('pink')
+ end
+end

0 comments on commit 7394d12

Please sign in to comment.