Merge branch 'master' into nested_has_many_through

2 files changed, 69 insertions, 8 deletions

diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb
index 2a99450a3d..73468afe55 100644
--- a/activemodel/lib/active_model/attribute_methods.rb
+++ b/activemodel/lib/active_model/attribute_methods.rb
@@ -106,11 +106,8 @@ module ActiveModel
         if block_given?
           sing.send :define_method, name, &block
         else
-          # use eval instead of a block to work around a memory leak in dev
-          # mode in fcgi
-          sing.class_eval <<-eorb, __FILE__, __LINE__ + 1
-            def #{name}; #{value.nil? ? 'nil' : value.to_s.inspect}; end
-          eorb
+          value = value.to_s if value
+          sing.send(:define_method, name) { value && value.dup }
         end
       end
 
diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb
index b001adb35a..e814666d99 100644
--- a/activemodel/test/cases/attribute_methods_test.rb
+++ b/activemodel/test/cases/attribute_methods_test.rb
@@ -5,6 +5,16 @@ class ModelWithAttributes
 
   attribute_method_suffix ''
 
+  class << self
+    define_method(:bar) do
+      'original bar'
+    end
+
+    define_method(:zomg) do
+      'original zomg'
+    end
+  end
+
   def attributes
     { :foo => 'value of foo' }
   end
@@ -36,6 +46,27 @@ private
   end
 end
 
+class ModelWithWeirdNamesAttributes
+  include ActiveModel::AttributeMethods
+
+  attribute_method_suffix ''
+
+  class << self
+    define_method(:'c?d') do
+      'original c?d'
+    end
+  end
+
+  def attributes
+    { :'a?b' => 'value of a?b' }
+  end
+
+private
+  def attribute(name)
+    attributes[name.to_sym]
+  end
+end
+
 class AttributeMethodsTest < ActiveModel::TestCase
   test 'unrelated classes should not share attribute method matchers' do
     assert_not_equal ModelWithAttributes.send(:attribute_method_matchers),
@@ -49,6 +80,14 @@ class AttributeMethodsTest < ActiveModel::TestCase
     assert_equal "value of foo", ModelWithAttributes.new.foo
   end
 
+  test '#define_attribute_method generates attribute method with invalid identifier characters' do
+    ModelWithWeirdNamesAttributes.define_attribute_method(:'a?b')
+    ModelWithWeirdNamesAttributes.define_attribute_method(:'a?b')
+
+    assert_respond_to ModelWithWeirdNamesAttributes.new, :'a?b'
+    assert_equal "value of a?b", ModelWithWeirdNamesAttributes.new.send('a?b')
+  end
+
   test '#define_attribute_methods generates attribute methods' do
     ModelWithAttributes.define_attribute_methods([:foo])
 
@@ -58,15 +97,40 @@ class AttributeMethodsTest < ActiveModel::TestCase
 
   test '#define_attribute_methods generates attribute methods with spaces in their names' do
     ModelWithAttributesWithSpaces.define_attribute_methods([:'foo bar'])
-  
+
     assert_respond_to ModelWithAttributesWithSpaces.new, :'foo bar'
     assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.send(:'foo bar')
   end
-  
+
+  def test_defined_methods_always_return_duped_string
+    ModelWithAttributes.define_attr_method(:zomg, 'lol')
+    assert_equal 'lol', ModelWithAttributes.zomg
+    ModelWithAttributes.zomg << 'bbq'
+    assert_equal 'lol', ModelWithAttributes.zomg
+  end
+
+  test '#define_attr_method generates attribute method' do
+    ModelWithAttributes.define_attr_method(:bar, 'bar')
+
+    assert_respond_to ModelWithAttributes, :bar
+    assert_equal "original bar", ModelWithAttributes.original_bar
+    assert_equal "bar", ModelWithAttributes.bar
+    ModelWithAttributes.define_attr_method(:bar)
+    assert !ModelWithAttributes.bar
+  end
+
+  test '#define_attr_method generates attribute method with invalid identifier characters' do
+    ModelWithWeirdNamesAttributes.define_attr_method(:'c?d', 'c?d')
+
+    assert_respond_to ModelWithWeirdNamesAttributes, :'c?d'
+    assert_equal "original c?d", ModelWithWeirdNamesAttributes.send('original_c?d')
+    assert_equal "c?d", ModelWithWeirdNamesAttributes.send('c?d')
+  end
+
   test '#alias_attribute works with attributes with spaces in their names' do
     ModelWithAttributesWithSpaces.define_attribute_methods([:'foo bar'])
     ModelWithAttributesWithSpaces.alias_attribute(:'foo_bar', :'foo bar')
-  
+
     assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.foo_bar
   end