Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Added Attributes.attribute!, .dangerous_attribute?

Refactored dangerous attribute detection and handling to not depend on
overriding the guts of ActiveModel::AttributeMethods
  • Loading branch information...
commit 29506c9e379f41a6d4c629141ae73084405c6589 1 parent b03e3be
@cgriego authored
View
2  CHANGELOG.md
@@ -1,6 +1,8 @@
# ActiveAttr 0.6.0 (unreleased) #
* Added AttributeDefinition#inspect
+* Added Attributes.attribute!
+* Added Attributes.dangerous_attribute?
* Added Typecasting#typecaster_for
* Added Typecasting::UnknownTypecasterError
* Changed Typecasting#typecast_attribute to take a typecaster, not a type
View
2  Gemfile
@@ -12,7 +12,7 @@ group :development do
gem "rb-fsevent"
gem "rdiscount"
gem "rdoc"
- gem "ruby-debug", :platforms => :mri_18
+ gem "ruby-debug", :platforms => :mri_18
gem "spec_coverage", :platforms => :mri_19
gem "travis-lint"
gem "yard"
View
1  lib/active_attr/attribute_definition.rb
@@ -47,6 +47,7 @@ def [](key)
# AttributeDefinition.new(:amount)
#
# @param [Symbol, String, #to_sym] name attribute name
+ # @param [Hash{Symbol => Object}] options attribute options
#
# @return [ActiveAttr::AttributeDefinition]
#
View
81 lib/active_attr/attributes.rb
@@ -120,14 +120,6 @@ def write_attribute(name, value)
end
alias_method :[]=, :write_attribute
- protected
-
- # Overrides ActiveModel::AttributeMethods
- # @private
- def attribute_method?(attr_name)
- self.class.attribute_names.include? attr_name.to_s
- end
-
private
# Read an attribute from the attributes hash
@@ -150,8 +142,9 @@ module ClassMethods
# Defines an attribute
#
# For each attribute that is defined, a getter and setter will be
- # added as an instance method to the model. An AttributeDefinition
- # instance will be added to result of the attributes class method.
+ # added as an instance method to the model. An
+ # {AttributeDefinition} instance will be added to result of the
+ # attributes class method.
#
# @example Define an attribute.
# attribute :name
@@ -161,8 +154,34 @@ module ClassMethods
# @raise [DangerousAttributeError] if the attribute name conflicts with
# existing methods
#
+ # @return [AttributeDefinition] Attribute's definition
+ #
# @since 0.2.0
def attribute(name, options={})
+ if dangerous_attribute_method_name = dangerous_attribute?(name)
+ raise DangerousAttributeError, %{an attribute method named "#{dangerous_attribute_method_name}" would conflict with an existing method}
+ else
+ attribute! name, options
+ end
+ end
+
+ # Defines an attribute without checking for conflicts
+ #
+ # Allows you to define an attribute whose methods will conflict
+ # with an existing method. For example, Ruby's Timeout library
+ # adds a timeout method to Object. Attempting to define a timeout
+ # attribute using .attribute will raise a
+ # {DangerousAttributeError}, but .attribute! will not.
+ #
+ # @example Define a dangerous attribute.
+ # attribute! :timeout
+ #
+ # @param (see AttributeDefinition#initialize)
+ #
+ # @return [AttributeDefinition] Attribute's definition
+ #
+ # @since 0.6.0
+ def attribute!(name, options={})
AttributeDefinition.new(name, options).tap do |attribute_definition|
attribute_name = attribute_definition.name.to_s
# Force active model to generate attribute methods
@@ -197,6 +216,30 @@ def attributes
@attributes ||= ActiveSupport::HashWithIndifferentAccess.new
end
+ # Determine if a given attribute name is dangerous
+ #
+ # Some attribute names can cause conflicts with existing methods
+ # on an object. For example, an attribute named "timeout" would
+ # conflict with the timeout method that Ruby's Timeout library
+ # mixes into Object.
+ #
+ # @example Testing a harmless attribute
+ # Person.dangerous_attribute? :name #=> false
+ #
+ # @example Testing a dangerous attribute
+ # Person.dangerous_attribute? :nil #=> "nil?"
+ #
+ # @param name Attribute name
+ #
+ # @return [false, String] False or the conflicting method name
+ #
+ # @since 0.6.0
+ def dangerous_attribute?(name)
+ attribute_methods(name).detect do |method_name|
+ !DEPRECATED_OBJECT_METHODS.include?(method_name.to_s) && allocate.respond_to?(method_name, true)
+ end unless attribute_names.include? name.to_s
+ end
+
# Returns the class name plus its attribute names
#
# @example Inspect the model's definition.
@@ -208,7 +251,7 @@ def attributes
def inspect
inspected_attributes = attribute_names.sort
attributes_list = "(#{inspected_attributes.join(", ")})" unless inspected_attributes.empty?
- "#{self.name}#{attributes_list}"
+ "#{name}#{attributes_list}"
end
protected
@@ -223,17 +266,15 @@ def attributes=(attributes)
@attributes = attributes
end
- # Overrides ActiveModel::AttributeMethods
- # @private
- def instance_method_already_implemented?(method_name)
- deprecated_object_method = DEPRECATED_OBJECT_METHODS.include?(method_name.to_s)
- already_implemented = !deprecated_object_method && self.allocate.respond_to?(method_name, true)
- raise DangerousAttributeError, %{an attribute method named "#{method_name}" would conflict with an existing method} if already_implemented
- false
- end
-
private
+ # Expand an attribute name into its generated methods names
+ #
+ # @since 0.6.0
+ def attribute_methods(name)
+ attribute_method_matchers.map { |matcher| matcher.method_name name }
+ end
+
# Ruby inherited hook to assign superclass attributes to subclasses
#
# @since 0.2.2
View
2  lib/active_attr/typecasted_attributes.rb
@@ -87,7 +87,7 @@ module ClassMethods
def inspect
inspected_attributes = attribute_names.sort.map { |name| "#{name}: #{_attribute_type(name)}" }
attributes_list = "(#{inspected_attributes.join(", ")})" unless inspected_attributes.empty?
- "#{self.name}#{attributes_list}"
+ "#{name}#{attributes_list}"
end
# Calculates an attribute type
View
79 spec/functional/active_attr/attributes_spec.rb
@@ -192,41 +192,78 @@ def self.name
end
context "defining dangerous attributes" do
- shared_examples "defining a dangerous attribute" do
- it "defining an attribute that conflicts with #{described_class} raises DangerousAttributeError" do
- expect { model_class.attribute(:write_attribute) }.to raise_error DangerousAttributeError, %{an attribute method named "write_attribute" would conflict with an existing method}
+ shared_examples "a dangerous attribute" do
+ it ".dangerous_attribute? is true" do
+ model_class.dangerous_attribute?(attribute_name).should be_true
end
- it "defining an attribute that conflicts with ActiveModel::AttributeMethods raises DangerousAttributeError" do
- expect { model_class.attribute(:inspect) }.to raise_error DangerousAttributeError, %{an attribute method named "inspect" would conflict with an existing method}
+ it ".attribute raises DangerousAttributeError" do
+ expect { model_class.attribute(attribute_name) }.to raise_error DangerousAttributeError, %{an attribute method named "#{attribute_name}" would conflict with an existing method}
end
- it "defining an :id attribute does not raise" do
- expect { model_class.attribute(:id) }.not_to raise_error
+ it ".attribute! does not raise" do
+ expect { model_class.attribute!(attribute_name) }.not_to raise_error
+ end
+ end
+
+ shared_examples "a whitelisted attribute" do
+ it ".dangerous_attribute? is false" do
+ model_class.dangerous_attribute?(attribute_name).should be_false
+ end
+
+ it ".attribute does not raise" do
+ expect { model_class.attribute(attribute_name) }.not_to raise_error
+ end
+
+ it ".attribute! does not raise" do
+ expect { model_class.attribute!(attribute_name) }.not_to raise_error
+ end
+ end
+
+ shared_examples "defining dangerous attributes" do
+ context "an attribute that conflicts with #{described_class}" do
+ let(:attribute_name) { :write_attribute }
+ include_examples "a dangerous attribute"
+ end
+
+ context "an attribute that conflicts with ActiveModel::AttributeMethods" do
+ let(:attribute_name) { :inspect }
+ include_examples "a dangerous attribute"
+ end
+
+ context "an attribute that conflicts with Kernel" do
+ let(:attribute_name) { :puts }
+ include_examples "a dangerous attribute"
end
- it "defining a :type attribute does not raise" do
- expect { model_class.attribute(:type) }.not_to raise_error
+ context "an attribute that conflicts with Object" do
+ let(:attribute_name) { :class }
+ include_examples "a dangerous attribute"
end
- it "defining an attribute that conflicts with Kernel raises DangerousAttributeError" do
- expect { model_class.attribute(:puts) }.to raise_error DangerousAttributeError
+ context "an attribute that conflicts with BasicObject" do
+ let(:attribute_name) { :instance_eval }
+ include_examples "a dangerous attribute"
end
- it "defining an attribute that conflicts with Object raises DangerousAttributeError" do
- expect { model_class.attribute(:class) }.to raise_error DangerousAttributeError
+ context "an attribute that conflicts with a properly implemented method_missing callback" do
+ let(:attribute_name) { :my_proper_missing_method }
+ include_examples "a dangerous attribute"
end
- it "defining an attribute that conflicts with BasicObject raises DangerousAttributeError" do
- expect { model_class.attribute(:instance_eval) }.to raise_error DangerousAttributeError
+ context "an attribute that conflicts with a less properly implemented method_missing callback" do
+ let(:attribute_name) { :my_less_proper_missing_method }
+ include_examples "a dangerous attribute"
end
- it "defining an attribute that conflicts with a properly implemented method_missing callback raises DangerousAttributeError" do
- expect { model_class.attribute(:my_proper_missing_method) }.to raise_error DangerousAttributeError
+ context "an :id attribute" do
+ let(:attribute_name) { :id }
+ include_examples "a whitelisted attribute"
end
- it "defining an attribute that conflicts with a less properly implemented method_missing callback raises DangerousAttributeError" do
- expect { model_class.attribute(:my_less_proper_missing_method) }.to raise_error DangerousAttributeError
+ context "a :type attribute" do
+ let(:attribute_name) { :type }
+ include_examples "a whitelisted attribute"
end
end
@@ -252,12 +289,12 @@ def respond_to?(method_name, include_private=false)
context "on a model class" do
let(:model_class) { dangerous_model_class }
- include_examples "defining a dangerous attribute"
+ include_examples "defining dangerous attributes"
end
context "on a child class" do
let(:model_class) { Class.new(dangerous_model_class) }
- include_examples "defining a dangerous attribute"
+ include_examples "defining dangerous attributes"
end
end
end
View
100 spec/unit/active_attr/attributes_spec.rb
@@ -51,51 +51,85 @@ def self.name
end
describe ".attribute" do
- it "creates an attribute with no options" do
- model_class.attributes.values.should include(AttributeDefinition.new(:first_name))
- end
+ context "a dangerous attribute" do
+ before { model_class.stub(:dangerous_attribute?).and_return(true) }
- it "returns the attribute definition" do
- Class.new(model_class).attribute(:address).should == AttributeDefinition.new(:address)
+ it { expect { model_class.attribute(:address) }.to raise_error DangerousAttributeError }
end
- it "defines an attribute reader that calls #attribute" do
- subject.should_receive(:attribute).with("first_name")
- subject.first_name
- end
+ context "a harmless attribute" do
+ it "creates an attribute with no options" do
+ model_class.attributes.values.should include(AttributeDefinition.new(:first_name))
+ end
- it "defines an attribute reader that can be called via super" do
- subject.should_receive(:attribute).with("amount")
- subject.amount
- end
+ it "returns the attribute definition" do
+ model_class.attribute(:address).should == AttributeDefinition.new(:address)
+ end
- it "defines an attribute writer that calls #attribute=" do
- subject.should_receive(:attribute=).with("first_name", "Ben")
- subject.first_name = "Ben"
+ it "defines an attribute reader that calls #attribute" do
+ subject.should_receive(:attribute).with("first_name")
+ subject.first_name
+ end
+
+ it "defines an attribute reader that can be called via super" do
+ subject.should_receive(:attribute).with("amount")
+ subject.amount
+ end
+
+ it "defines an attribute writer that calls #attribute=" do
+ subject.should_receive(:attribute=).with("first_name", "Ben")
+ subject.first_name = "Ben"
+ end
+
+ it "defines an attribute writer that can be called via super" do
+ subject.should_receive(:attribute=).with("amount", 1)
+ subject.amount = 1
+ end
+
+ it "defining an attribute twice does not give the class two attribute definitions" do
+ Class.new do
+ include Attributes
+ attribute :name
+ attribute :name
+ end.should have(1).attributes
+ end
+
+ it "redefining an attribute replaces the attribute definition" do
+ klass = Class.new do
+ include Attributes
+ attribute :name, :type => Symbol
+ attribute :name, :type => String
+ end
+
+ klass.should have(1).attributes
+ klass.attributes[:name].should == AttributeDefinition.new(:name, :type => String)
+ end
end
+ end
- it "defines an attribute writer that can be called via super" do
- subject.should_receive(:attribute=).with("amount", 1)
- subject.amount = 1
+ describe ".attribute!" do
+ it "can create an attribute with no options" do
+ attributeless.attribute! :first_name
+ attributeless.attributes.values.should include AttributeDefinition.new(:first_name)
end
- it "defining an attribute twice does not give the class two attribute definitions" do
- Class.new do
- include Attributes
- attribute :name
- attribute :name
- end.should have(1).attributes
+ it "returns the attribute definition" do
+ attributeless.attribute!(:address).should == AttributeDefinition.new(:address)
end
- it "redefining an attribute replaces the attribute definition" do
- klass = Class.new do
- include Attributes
- attribute :name, :type => Symbol
- attribute :name, :type => String
- end
+ it "defines an attribute reader that calls #attribute" do
+ attributeless.attribute! :first_name
+ instance = attributeless.new
+ result = mock
+ instance.should_receive(:attribute).with("first_name").and_return(result)
+ instance.first_name.should equal result
+ end
- klass.should have(1).attributes
- klass.attributes[:name].should == AttributeDefinition.new(:name, :type => String)
+ it "defines an attribute writer that calls #attribute=" do
+ attributeless.attribute! :first_name
+ instance = attributeless.new
+ instance.should_receive(:attribute=).with("first_name", "Ben")
+ instance.first_name = "Ben"
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.