Skip to content

Commit

Permalink
avoid nil.dup
Browse files Browse the repository at this point in the history
Signed-off-by: Santiago Pastorino <santiago@wyeworks.com>
  • Loading branch information
amatsuda authored and spastorino committed Feb 7, 2011
1 parent 1fd9d97 commit 40aefb9
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
Expand Up @@ -41,7 +41,7 @@ def define_method_attribute=(attr_name)
if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name])
method_body, line = <<-EOV, __LINE__ + 1
def #{attr_name}=(original_time)
time = original_time.dup
time = original_time.dup unless original_time.nil?

This comment has been minimized.

Copy link
@adzap

adzap Feb 7, 2011

Contributor

You also can't dup a Fixnum.

This comment has been minimized.

Copy link
@spastorino

spastorino Feb 7, 2011

Contributor

Please provide a patch using respond_to?

This comment has been minimized.

Copy link
@brainopia

brainopia Feb 7, 2011

Contributor

Won't work, dup is defined via Kernel module for every object.

This comment has been minimized.

Copy link
@spastorino

spastorino Feb 7, 2011

Contributor

@brianopia yes you're right

This comment has been minimized.

Copy link
@tenderlove

tenderlove Feb 7, 2011

Member

Do people pass Fixnums to this method?

This comment has been minimized.

Copy link
@adzap

adzap Feb 8, 2011

Contributor

Apparently you can use fixnum in seconds as a value for the Mysql duration data type.

This comment has been minimized.

Copy link
@brianmario

brianmario Feb 8, 2011

Contributor

Is there a duration data type in Mysql?

This comment has been minimized.

Copy link
@tenderlove

tenderlove Feb 8, 2011

Member

@brianmario I don't see one. @adzap, can you show us?

unless time.acts_like?(:time)
time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time
end
Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/attribute_methods_test.rb
Expand Up @@ -460,6 +460,14 @@ def test_accessing_cached_attributes_caches_the_converted_values_and_nothing_els
end
end

def test_write_nil_to_time_attributes
in_time_zone "Pacific Time (US & Canada)" do
record = @target.new
record.written_on = nil
assert_nil record.written_on
end
end

def test_time_attributes_are_retrieved_in_current_time_zone
in_time_zone "Pacific Time (US & Canada)" do
utc_time = Time.utc(2008, 1, 1)
Expand Down

2 comments on commit 40aefb9

@adzap
Copy link
Contributor

@adzap adzap commented on 40aefb9 Feb 8, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tenderlove @brianmario upon searching, I can't find one. I was told about it in a bug report elsewhere. Perhaps they used 'data type' too liberally. I am not up to date with Mysql features so I didn't question it.

Perhaps we can ignore that case.

@francocatena
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error also affects 3-0-stable, I create a patch in lighthouse for the same problem: https://rails.lighthouseapp.com/projects/8994/tickets/6367-timezone-aware-attributes-fail-with-nil-assignments

Please sign in to comment.