Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

allow spaces and other characters in attribute names [#4725 state:res…

…olved]

* define the dynamically defined methods with
  'define_method' instead of def
* wrap some string injected method names in quotes

Signed-off-by: Santiago Pastorino <santiago@wyeworks.com>
  • Loading branch information...
commit bca070ef2ddbbe7e093c340ec7722e4dca0f37a5 1 parent d65e3b4
Caleb Land authored spastorino committed
View
8 activemodel/lib/active_model/attribute_methods.rb
@@ -230,8 +230,8 @@ def attribute_method_affix(*affixes)
def alias_attribute(new_name, old_name)
attribute_method_matchers.each do |matcher|
module_eval <<-STR, __FILE__, __LINE__ + 1
- def #{matcher.method_name(new_name)}(*args)
- send(:#{matcher.method_name(old_name)}, *args)
+ define_method(:'#{matcher.method_name(new_name)}') do |*args|
+ send(:'#{matcher.method_name(old_name)}', *args)
end
STR
end
@@ -277,8 +277,8 @@ def define_attribute_methods(attr_names)
if method_defined?(:'#{method_name}')
undef :'#{method_name}'
end
- def #{method_name}(*args)
- send(:#{matcher.method_missing_target}, '#{attr_name}', *args)
+ define_method(:'#{method_name}') do |*args|
+ send(:'#{matcher.method_missing_target}', '#{attr_name}', *args)
end
STR
end
View
30 activemodel/test/cases/attribute_methods_test.rb
@@ -21,6 +21,21 @@ class ModelWithAttributes2
attribute_method_suffix '_test'
end
+class ModelWithAttributesWithSpaces
+ include ActiveModel::AttributeMethods
+
+ attribute_method_suffix ''
+
+ def attributes
+ { :'foo bar' => 'value of foo bar'}
+ 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),
@@ -35,6 +50,21 @@ class AttributeMethodsTest < ActiveModel::TestCase
assert_equal "value of foo", ModelWithAttributes.new.foo
end
+ test '#define_attribute_methods generates attribute methods with spaces in their names' do
+ ModelWithAttributesWithSpaces.define_attribute_methods([:'foo bar'])
+
+ assert ModelWithAttributesWithSpaces.attribute_methods_generated?
+ assert_respond_to ModelWithAttributesWithSpaces.new, :'foo bar'
+ assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.send(:'foo bar')
+ 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
+
test '#undefine_attribute_methods removes attribute methods' do
ModelWithAttributes.define_attribute_methods([:foo])
ModelWithAttributes.undefine_attribute_methods

4 comments on commit bca070e

@fxn
Owner

Note that #define_method and #send accept strings, while send(:foo) reads great, in metaprogramming strings are a bit simpler.

@spastorino
Owner

Xavier also we should remove module_eval too

@fxn
Owner

@spastorino you're right, let me revise those.

@spastorino
Owner

Pushed 6f4d998, and also method_defined?.
Sorry Xavier I haven't seen you were going to do it, go ahead with the module_eval part :).

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