diff options
| -rw-r--r-- | actionpack/lib/action_controller/metal/force_ssl.rb | 3 | ||||
| -rw-r--r-- | activemodel/lib/active_model/attribute_methods.rb | 60 | ||||
| -rw-r--r-- | activerecord/lib/active_record/attribute_methods.rb | 3 | ||||
| -rw-r--r-- | activerecord/lib/active_record/core.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/cases/attribute_methods_test.rb | 7 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 7 | ||||
| -rw-r--r-- | activerecord/test/cases/serialized_attribute_test.rb | 2 | ||||
| -rw-r--r-- | activesupport/lib/active_support/core_ext/module/delegation.rb | 72 | ||||
| -rw-r--r-- | activesupport/test/core_ext/module_test.rb | 31 |
9 files changed, 123 insertions, 66 deletions
diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 7edeba6fc0..b8afce42c9 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -61,8 +61,9 @@ module ActionController # will be called only when it returns a false value. def force_ssl(options = {}) action_options = options.slice(*ACTION_OPTIONS) + redirect_options = options.except(*ACTION_OPTIONS) before_action(action_options) do - force_ssl_redirect(options.except(*ACTION_OPTIONS)) + force_ssl_redirect(redirect_options) end end end diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 6d11c0fbdc..5db898b33a 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -12,19 +12,21 @@ module ActiveModel # # => ActiveModel::MissingAttributeError: missing attribute: user_id class MissingAttributeError < NoMethodError end + # == Active \Model Attribute Methods # # <tt>ActiveModel::AttributeMethods</tt> provides a way to add prefixes and - # suffixes to your methods as well as handling the creation of Active Record - # like class methods such as +table_name+. + # suffixes to your methods as well as handling the creation of + # <tt>ActiveRecord::Base</tt>-like class methods such as +table_name+. # - # The requirements to implement ActiveModel::AttributeMethods are to: + # The requirements to implement <tt>ActiveModel::AttributeMethods</tt> are to: # - # * <tt>include ActiveModel::AttributeMethods</tt> in your object. - # * Call each Attribute Method module method you want to add, such as - # +attribute_method_suffix+ or +attribute_method_prefix+. + # * <tt>include ActiveModel::AttributeMethods</tt> in your class. + # * Call each of its method you want to add, such as +attribute_method_suffix+ + # or +attribute_method_prefix+. # * Call +define_attribute_methods+ after the other methods are called. # * Define the various generic +_attribute+ methods that you have declared. + # * Define an +attributes+ method, see below. # # A minimal implementation could be: # @@ -38,6 +40,10 @@ module ActiveModel # # attr_accessor :name # + # def attributes + # {'name' => @name} + # end + # # private # # def attribute_contrived?(attr) @@ -53,10 +59,10 @@ module ActiveModel # end # end # - # Note that whenever you include ActiveModel::AttributeMethods in your class, - # it requires you to implement an +attributes+ method which returns a hash - # with each attribute name in your model as hash key and the attribute value as - # hash value. + # Note that whenever you include <tt>ActiveModel::AttributeMethods</tt> in + # your class, it requires you to implement an +attributes+ method which + # returns a hash with each attribute name in your model as hash key and the + # attribute value as hash value. # # Hash keys must be strings. module AttributeMethods @@ -179,7 +185,6 @@ module ActiveModel undefine_attribute_methods end - # Allows you to make aliases for attributes. # # class Person @@ -413,17 +418,16 @@ module ActiveModel end end - # Allows access to the object attributes, which are held in the - # <tt>@attributes</tt> hash, as though they were first-class methods. So a - # Person class with a name attribute can use Person#name and Person#name= - # and never directly use the attributes hash -- except for multiple assigns - # with ActiveRecord#attributes=. A Milestone class can also ask - # Milestone#completed? to test that the completed attribute is not +nil+ - # or 0. + # Allows access to the object attributes, which are held in the hash + # returned by <tt>attributes</tt>, as though they were first-class + # methods. So a +Person+ class with a +name+ attribute can for example use + # <tt>Person#name</tt> and <tt>Person#name=</tt> and never directly use + # the attributes hash -- except for multiple assigns with + # <tt>ActiveRecord::Base#attributes=</tt>. # - # It's also possible to instantiate related objects, so a Client class - # belonging to the clients table with a +master_id+ foreign key can - # instantiate master through Client#master. + # It's also possible to instantiate related objects, so a <tt>Client</tt> + # class belonging to the +clients+ table with a +master_id+ foreign key + # can instantiate master through <tt>Client#master</tt>. def method_missing(method, *args, &block) if respond_to_without_attributes?(method, true) super @@ -433,17 +437,17 @@ module ActiveModel end end - # attribute_missing is like method_missing, but for attributes. When method_missing is - # called we check to see if there is a matching attribute method. If so, we call - # attribute_missing to dispatch the attribute. This method can be overloaded to - # customize the behavior. + # +attribute_missing+ is like +method_missing+, but for attributes. When + # +method_missing+ is called we check to see if there is a matching + # attribute method. If so, we tell +attribute_missing+ to dispatch the + # attribute. This method can be overloaded to customize the behavior. def attribute_missing(match, *args, &block) __send__(match.target, match.attr_name, *args, &block) end - # A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>, - # <tt>person.respond_to?(:name=)</tt>, and <tt>person.respond_to?(:name?)</tt> - # which will all return +true+. + # A +Person+ instance with a +name+ attribute can ask + # <tt>person.respond_to?(:name)</tt>, <tt>person.respond_to?(:name=)</tt>, + # and <tt>person.respond_to?(:name?)</tt> which will all return +true+. alias :respond_to_without_attributes? :respond_to? def respond_to?(method, include_private_methods = false) if super diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 2fbab97133..609c6e8cab 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -172,6 +172,8 @@ module ActiveRecord # If the result is true then check for the select case. # For queries selecting a subset of columns, return false for unselected columns. + # We check defined?(@attributes) not to issue warnings if called on objects that + # have been allocated but not yet initialized. if defined?(@attributes) && @attributes.present? && self.class.column_names.include?(name) return has_attribute?(name) end @@ -340,6 +342,7 @@ module ActiveRecord end def attribute_method?(attr_name) # :nodoc: + # We check defined? because Syck calls respond_to? before actually calling initialize. defined?(@attributes) && @attributes.include?(attr_name) end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 9e45e6e474..ba053700f2 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -352,7 +352,9 @@ module ActiveRecord # Returns the contents of the record as a nicely formatted string. def inspect - inspection = if @attributes + # We check defined?(@attributes) not to issue warnings if the object is + # allocated but not initialized. + inspection = if defined?(@attributes) && @attributes self.class.column_names.collect { |name| if has_attribute?(name) "#{name}: #{attribute_for_inspect(name)}" diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 387c741762..d9c032392d 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -141,13 +141,10 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_respond_to topic, :title end - # IRB inspects the return value of "MyModel.allocate" - # by inspecting it. + # IRB inspects the return value of "MyModel.allocate". def test_allocated_object_can_be_inspected topic = Topic.allocate - topic.instance_eval { @attributes = nil } - assert_nothing_raised { topic.inspect } - assert topic.inspect, "#<Topic not initialized>" + assert_equal "#<Topic not initialized>", topic.inspect end def test_array_content diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 0ff5be5b1f..557cc7e7a0 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -31,6 +31,13 @@ class FinderTest < ActiveRecord::TestCase assert_equal(topics(:first).title, Topic.find(1).title) end + def test_symbols_table_ref + Post.first # warm up + x = Symbol.all_symbols.count + Post.where("title" => {"xxxqqqq" => "bar"}) + assert_equal x, Symbol.all_symbols.count + end + # find should handle strings that come from URLs # (example: Category.find(params[:id])) def test_find_with_string diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index d0e012902e..765e92ccca 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -243,7 +243,7 @@ class SerializedAttributeTest < ActiveRecord::TestCase assert_equal [], light.long_state end - def test_serialized_columh_should_not_be_wrapped_twice + def test_serialized_column_should_not_be_wrapped_twice Topic.serialize(:content, MyObject) myobj = MyObject.new('value1', 'value2') diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index c0828343d8..6d42667e97 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -1,8 +1,10 @@ class Module - # Provides a delegate class method to easily expose contained objects' public methods - # as your own. Pass one or more methods (specified as symbols or strings) - # and the name of the target object via the <tt>:to</tt> option (also a symbol - # or string). At least one method and the <tt>:to</tt> option are required. + # Provides a +delegate+ class method to easily expose contained objects' + # public methods as your own. + # + # The macro receives one or more method names (specified as symbols or + # strings) and the name of the target object via the <tt>:to</tt> option + # (also a symbol or string). # # Delegation is particularly useful with Active Record associations: # @@ -89,39 +91,40 @@ class Module # invoice.customer_name # => 'John Doe' # invoice.customer_address # => 'Vimmersvej 13' # - # If the delegate object is +nil+ an exception is raised, and that happens - # no matter whether +nil+ responds to the delegated method. You can get a - # +nil+ instead with the +:allow_nil+ option. + # If the target is +nil+ and does not respond to the delegated method a + # +NoMethodError+ is raised, as with any other value. Sometimes, however, it + # makes sense to be robust to that situation and that is the purpose of the + # <tt>:allow_nil</tt> option: If the target is not +nil+, or it is and + # responds to the method, everything works as usual. But if it is +nil+ and + # does not respond to the delegated method, +nil+ is returned. # - # class Foo - # attr_accessor :bar - # def initialize(bar = nil) - # @bar = bar - # end - # delegate :zoo, to: :bar + # class User < ActiveRecord::Base + # has_one :profile + # delegate :age, to: :profile # end # - # Foo.new.zoo # raises NoMethodError exception (you called nil.zoo) + # User.new.age # raises NoMethodError: undefined method `age' # - # class Foo - # attr_accessor :bar - # def initialize(bar = nil) - # @bar = bar - # end - # delegate :zoo, to: :bar, allow_nil: true + # But if not having a profile yet is fine and should not be an error + # condition: + # + # class User < ActiveRecord::Base + # has_one :profile + # delegate :age, to: :profile, allow_nil: true # end # - # Foo.new.zoo # returns nil + # User.new.age # nil # - # If the delegate object is not +nil+ or +false+ and the object doesn't - # respond to the delegated method it will raise an exception. + # Note that if the target is not +nil+ then the call is attempted regardless of the + # <tt>:allow_nil</tt> option, and thus an exception is still raised if said object + # does not respond to the method: # # class Foo # def initialize(bar) # @bar = bar # end # - # delegate :name, to: :@bar + # delegate :name, to: :@bar, allow_nil: true # end # # Foo.new("Bar").name # raises NoMethodError: undefined method `name' @@ -156,22 +159,31 @@ class Module # methods still accept two arguments. definition = (method =~ /[^\]]=$/) ? 'arg' : '*args, &block' + # The following generated methods call the target exactly once, storing + # the returned value in a dummy variable. + # + # Reason is twofold: On one hand doing less calls is in general better. + # On the other hand it could be that the target has side-effects, + # whereas conceptualy, from the user point of view, the delegator should + # be doing one call. if allow_nil - module_eval(<<-EOS, file, line - 2) + module_eval(<<-EOS, file, line - 3) def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block) - if #{to} || #{to}.respond_to?(:#{method}) # if client || client.respond_to?(:name) - #{to}.#{method}(#{definition}) # client.name(*args, &block) + _ = #{to} # _ = client + if !_.nil? || nil.respond_to?(:#{method}) # if !_.nil? || nil.respond_to?(:name) + _.#{method}(#{definition}) # _.name(*args, &block) end # end end # end EOS else exception = %(raise "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}") - module_eval(<<-EOS, file, line - 1) + module_eval(<<-EOS, file, line - 2) def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block) - #{to}.#{method}(#{definition}) # client.name(*args, &block) + _ = #{to} # _ = client + _.#{method}(#{definition}) # _.name(*args, &block) rescue NoMethodError # rescue NoMethodError - if #{to}.nil? # if client.nil? + if _.nil? # if _.nil? #{exception} # # add helpful message to the exception else # else raise # raise diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index 9a8582075d..8872611fb1 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -82,6 +82,21 @@ class Name end end +class SideEffect + attr_reader :ints + + delegate :to_i, :to => :shift, :allow_nil => true + delegate :to_s, :to => :shift + + def initialize + @ints = [1, 2, 3] + end + + def shift + @ints.shift + end +end + class ModuleTest < ActiveSupport::TestCase def setup @david = Someone.new("David", Somewhere.new("Paulina", "Chicago")) @@ -171,6 +186,12 @@ class ModuleTest < ActiveSupport::TestCase assert_nil rails.name end + # Ensures with check for nil, not for a falseish target. + def test_delegation_with_allow_nil_and_false_value + project = Project.new(false, false) + assert_raise(NoMethodError) { project.name } + end + def test_delegation_with_allow_nil_and_invalid_value rails = Project.new("Rails", "David") assert_raise(NoMethodError) { rails.name } @@ -233,6 +254,16 @@ class ModuleTest < ActiveSupport::TestCase "[#{e.backtrace.inspect}] did not include [#{file_and_line}]" end + def test_delegation_invokes_the_target_exactly_once + se = SideEffect.new + + assert_equal 1, se.to_i + assert_equal [2, 3], se.ints + + assert_equal '2', se.to_s + assert_equal [3], se.ints + end + def test_parent assert_equal Yz::Zy, Yz::Zy::Cd.parent assert_equal Yz, Yz::Zy.parent |