Skip to content
Browse files

Don't proxy Duration#object_id, #send, #tap, and #try

Proxying these methods cause unexpected behavior:

  d = 1.day
  d.inspect                #=> "1 day"
  d.try(:inspect) || "N/A" #=> "86400"
  # ^^^ standard null checking goes pear-shaped

related to #320, #334
  • Loading branch information...
1 parent 8c2cf1a commit 966e55b7881864ca3400b4f90e6fe5d1f8708f79 @misfo committed Apr 29, 2012
View
8 activesupport/lib/active_support/duration.rb
@@ -8,7 +8,7 @@ module ActiveSupport
# Example:
#
# 1.month.ago # equivalent to Time.now.advance(:months => -1)
- class Duration < BasicObject
+ class Duration
attr_accessor :value, :parts
def initialize(value, parts) #:nodoc:
@@ -84,6 +84,12 @@ def as_json(options = nil) #:nodoc:
to_i
end
+ PROXIED_OBJECT_METHODS = (Object.public_instance_methods | [:duplicable?]) -
+ BasicObject.public_instance_methods -
+ public_instance_methods(false) -
+ [:object_id, :send, :tap, :try]
+ delegate(*PROXIED_OBJECT_METHODS, to: :value)
+
protected
def sum(sign, time = ::Time.current) #:nodoc:
View
2 activesupport/test/core_ext/duplicable_test.rb
@@ -18,7 +18,7 @@ class DuplicableTest < ActiveSupport::TestCase
def test_duplicable
(RAISE_DUP + NO).each do |v|
- assert !v.duplicable?
+ assert !v.duplicable?, "#{v.class} should not be duplicable"
end
YES.each do |v|
View
18 activesupport/test/core_ext/duration_test.rb
@@ -21,7 +21,6 @@ def test_threequals
assert ActiveSupport::Duration === 1.day
assert !(ActiveSupport::Duration === 1.day.to_i)
assert !(ActiveSupport::Duration === 'foo')
- assert !(ActiveSupport::Duration === ActiveSupport::BasicObject.new)
end
def test_equals
@@ -41,6 +40,21 @@ def test_inspect
assert_equal '14 days', 1.fortnight.inspect
end
+ def test_comparison
+ assert_equal(-1, 1.days <=> 1.year)
+ assert_equal 1, 3.years <=> 4.seconds
+ assert_equal 0, (60.seconds <=> 1.minute)
+ end
+
+ def test_unproxied_object_methods
+ assert 1.day.try(:is_a?, ActiveSupport::Duration),
+ "Duration#try should invoke Duration's instance methods"
+ assert 1.day.send(:is_a?, ActiveSupport::Duration),
+ "Duration#send should invoke Duration's instance methods"
+ assert 1.day.tap {|d| d }.is_a?(ActiveSupport::Duration),
+ "Duration#send should invoke Duration's instance methods"
+ end
+
def test_minus_with_duration_does_not_break_subtraction_of_date_from_date
assert_nothing_raised { Date.today - Date.today }
end
@@ -131,7 +145,7 @@ def test_adding_day_across_dst_boundary
assert_equal Time.local(2009,3,29,0,0,0) + 1.day, Time.local(2009,3,30,0,0,0)
end
end
-
+
def test_delegation_with_block_works
counter = 0
assert_nothing_raised do

0 comments on commit 966e55b

Please sign in to comment.
Something went wrong with that request. Please try again.