diff options
author | willnet <netwillnet@gmail.com> | 2016-07-04 18:17:20 +0900 |
---|---|---|
committer | willnet <netwillnet@gmail.com> | 2016-08-04 10:11:06 +0900 |
commit | 3529e58efd21697cb5559fe622df8013179625a2 (patch) | |
tree | 600c752143d384b0eef3a534959af5bd6f222efe | |
parent | 5b469da6ec482414c5f59762ae8e82de7e07c365 (diff) | |
download | rails-3529e58efd21697cb5559fe622df8013179625a2.tar.gz rails-3529e58efd21697cb5559fe622df8013179625a2.tar.bz2 rails-3529e58efd21697cb5559fe622df8013179625a2.zip |
Fix `thread_mattr_accessor` share variable superclass with subclass
The current implementation of `thread_mattr_accessor` set variable
sharing superclass with subclass. So the method doesn't work as documented.
Precondition
class Account
thread_mattr_accessor :user
end
class Customer < Account
end
Account.user = "DHH"
Account.user #=> "DHH"
Customer.user = "Rafael"
Customer.user # => "Rafael"
Documented behavior
Account.user # => "DHH"
Actual behavior
Account.user # => "Rafael"
Current implementation set variable statically likes `Thread[:attr_Account_user]`,
and customer also use it.
Make variable name dynamic to use own thread-local variable.
3 files changed, 35 insertions, 8 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 047688aba8..f7561a1b34 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix `thread_mattr_accessor` share variable superclass with subclass + + The current implementation of `thread_mattr_accessor` set variable + sharing superclass with subclass. So the method doesn't work as documented. + + *Shinichi Maeshima* + * Since weeks are no longer converted to days, add `:weeks` to the list of parts that `ActiveSupport::TimeWithZone` will recognize as possibly being of variable duration to take account of DST transitions. diff --git a/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb b/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb index 7015333f7c..946be64f1c 100644 --- a/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb +++ b/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb @@ -41,14 +41,14 @@ class Module raise NameError.new("invalid attribute name: #{sym}") unless /^[_A-Za-z]\w*$/.match?(sym) class_eval(<<-EOS, __FILE__, __LINE__ + 1) def self.#{sym} - Thread.current[:"attr_#{name}_#{sym}"] + Thread.current["attr_"+ name + "_#{sym}"] end EOS unless options[:instance_reader] == false || options[:instance_accessor] == false class_eval(<<-EOS, __FILE__, __LINE__ + 1) def #{sym} - Thread.current[:"attr_#{name}_#{sym}"] + Thread.current["attr_"+ self.class.name + "_#{sym}"] end EOS end @@ -80,14 +80,14 @@ class Module raise NameError.new("invalid attribute name: #{sym}") unless /^[_A-Za-z]\w*$/.match?(sym) class_eval(<<-EOS, __FILE__, __LINE__ + 1) def self.#{sym}=(obj) - Thread.current[:"attr_#{name}_#{sym}"] = obj + Thread.current["attr_"+ name + "_#{sym}"] = obj end EOS unless options[:instance_writer] == false || options[:instance_accessor] == false class_eval(<<-EOS, __FILE__, __LINE__ + 1) def #{sym}=(obj) - Thread.current[:"attr_#{name}_#{sym}"] = obj + Thread.current["attr_"+ self.class.name + "_#{sym}"] = obj end EOS end diff --git a/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb b/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb index a9fd878b80..cdde6434f2 100644 --- a/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb +++ b/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb @@ -8,6 +8,16 @@ class ModuleAttributeAccessorPerThreadTest < ActiveSupport::TestCase thread_mattr_accessor :bar, instance_writer: false thread_mattr_reader :shaq, instance_reader: false thread_mattr_accessor :camp, instance_accessor: false + + def self.name + 'MyClass' + end + end + + @subclass = Class.new(@class) do + def self.name + 'SubMyClass' + end end @object = @class.new @@ -65,15 +75,15 @@ class ModuleAttributeAccessorPerThreadTest < ActiveSupport::TestCase threads << Thread.new do @class.foo = 'other things' sleep 1 - assert_equal 'other things', @class.foo + assert_equal 'other things', @class.foo end - + threads << Thread.new do @class.foo = 'really other things' sleep 1 - assert_equal 'really other things', @class.foo + assert_equal 'really other things', @class.foo end - + threads.each { |t| t.join } end @@ -112,4 +122,14 @@ class ModuleAttributeAccessorPerThreadTest < ActiveSupport::TestCase assert_equal @class.foo, @object.foo end + + def test_should_not_affect_superclass_if_subclass_set_value + @class.foo = 'super' + assert_equal @class.foo, 'super' + assert_nil @subclass.foo + + @subclass.foo = 'sub' + assert_equal @class.foo, 'super' + assert_equal @subclass.foo, 'sub' + end end |