diff options
16 files changed, 267 insertions, 78 deletions
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 85d9c3be00..f6f63f1f32 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -161,6 +161,26 @@ module ActionDispatch # :nodoc: def set_header(key, v); headers[key] = v; end def delete_header(key); headers.delete key; end + # Add a header that may have multiple values. + # + # Example: + # response.add_header 'Vary', 'Accept' + # response.add_header 'Vary', 'Accept-Encoding' + # response.add_header 'Vary', 'Cookie' + # + # assert_equal 'Accept,Accept-Encoding,Cookie', response.get_header 'Vary' + # + # http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 + def add_header(key, v) + if v.nil? + get_header key + elsif have_header? key + set_header key, "#{get_header key},#{v}" + else + set_header key, v + end + end + def await_commit synchronize do @cv.wait_until { @committed } diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index fa29f3a6c4..82cc21b17a 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -293,6 +293,65 @@ class ResponseTest < ActiveSupport::TestCase end end +class ResponseHeadersTest < ActiveSupport::TestCase + def setup + @response = ActionDispatch::Response.create + @response.set_header 'Foo', '1' + end + + test 'have_header?' do + assert @response.have_header? 'Foo' + assert_not @response.have_header? 'foo' + assert_not @response.have_header? nil + end + + test 'get_header' do + assert_equal '1', @response.get_header('Foo') + assert_nil @response.get_header('foo') + assert_nil @response.get_header(nil) + end + + test 'set_header' do + assert_equal '2', @response.set_header('Foo', '2') + assert @response.have_header?('Foo') + assert_equal '2', @response.get_header('Foo') + + assert_nil @response.set_header('Foo', nil) + assert @response.have_header?('Foo') + assert_nil @response.get_header('Foo') + end + + test 'delete_header' do + assert_nil @response.delete_header(nil) + + assert_nil @response.delete_header('foo') + assert @response.have_header?('Foo') + + assert_equal '1', @response.delete_header('Foo') + assert_not @response.have_header?('Foo') + end + + test 'add_header' do + # Add a value to an existing header + assert_equal '1,2', @response.add_header('Foo', '2') + assert_equal '1,2', @response.get_header('Foo') + + # Add nil to an existing header + assert_equal '1,2', @response.add_header('Foo', nil) + assert_equal '1,2', @response.get_header('Foo') + + # Add nil to a nonexistent header + assert_nil @response.add_header('Bar', nil) + assert_not @response.have_header?('Bar') + assert_nil @response.get_header('Bar') + + # Add a value to a nonexistent header + assert_equal '1', @response.add_header('Bar', '1') + assert @response.have_header?('Bar') + assert_equal '1', @response.get_header('Bar') + end +end + class ResponseIntegrationTest < ActionDispatch::IntegrationTest test "response cache control from railsish app" do @app = lambda { |env| diff --git a/activemodel/lib/active_model/forbidden_attributes_protection.rb b/activemodel/lib/active_model/forbidden_attributes_protection.rb index b4fa378601..d2c6a89cc2 100644 --- a/activemodel/lib/active_model/forbidden_attributes_protection.rb +++ b/activemodel/lib/active_model/forbidden_attributes_protection.rb @@ -17,8 +17,9 @@ module ActiveModel module ForbiddenAttributesProtection # :nodoc: protected def sanitize_for_mass_assignment(attributes) - if attributes.respond_to?(:permitted?) && !attributes.permitted? - raise ActiveModel::ForbiddenAttributesError + if attributes.respond_to?(:permitted?) + raise ActiveModel::ForbiddenAttributesError if !attributes.permitted? + attributes.to_h else attributes end diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 74b44810d4..3c4c8f10ec 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -5,8 +5,8 @@ module ActiveRecord FromDatabase.new(name, value, type) end - def from_user(name, value, type) - FromUser.new(name, value, type) + def from_user(name, value, type, original_attribute = nil) + FromUser.new(name, value, type, original_attribute) end def with_cast_value(name, value, type) @@ -26,10 +26,11 @@ module ActiveRecord # This method should not be called directly. # Use #from_database or #from_user - def initialize(name, value_before_type_cast, type) + def initialize(name, value_before_type_cast, type, original_attribute = nil) @name = name @value_before_type_cast = value_before_type_cast @type = type + @original_attribute = original_attribute end def value @@ -38,21 +39,33 @@ module ActiveRecord @value end + def original_value + if assigned? + original_attribute.original_value + else + type_cast(value_before_type_cast) + end + end + def value_for_database type.serialize(value) end - def changed_from?(old_value) - type.changed?(old_value, value, value_before_type_cast) + def changed? + changed_from_assignment? || changed_in_place? + end + + def changed_in_place? + has_been_read? && type.changed_in_place?(original_value_for_database, value) end - def changed_in_place_from?(old_value) - has_been_read? && type.changed_in_place?(old_value, value) + def forgetting_assignment + with_value_from_database(value_for_database) end def with_value_from_user(value) type.assert_valid_value(value) - self.class.from_user(name, value, type) + self.class.from_user(name, value, type, self) end def with_value_from_database(value) @@ -64,7 +77,7 @@ module ActiveRecord end def with_type(type) - self.class.new(name, value_before_type_cast, type) + self.class.new(name, value_before_type_cast, type, original_attribute) end def type_cast(*) @@ -97,16 +110,39 @@ module ActiveRecord protected + attr_reader :original_attribute + alias_method :assigned?, :original_attribute + def initialize_dup(other) if defined?(@value) && @value.duplicable? @value = @value.dup end end + def changed_from_assignment? + assigned? && type.changed?(original_value, value, value_before_type_cast) + end + + def original_value_for_database + if assigned? + original_attribute.original_value_for_database + else + _original_value_for_database + end + end + + def _original_value_for_database + value_for_database + end + class FromDatabase < Attribute # :nodoc: def type_cast(value) type.deserialize(value) end + + def _original_value_for_database + value_before_type_cast + end end class FromUser < Attribute # :nodoc: diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activerecord/lib/active_record/attribute/user_provided_default.rb index 501590cf0e..fb8ad9163e 100644 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ b/activerecord/lib/active_record/attribute/user_provided_default.rb @@ -4,8 +4,7 @@ module ActiveRecord class Attribute # :nodoc: class UserProvidedDefault < FromUser def initialize(name, value, type, database_default) - super(name, value, type) - @database_default = database_default + super(name, value, type, database_default) end def type_cast(value) @@ -16,17 +15,9 @@ module ActiveRecord end end - def changed_from?(old_value) - super || changed_from?(database_default.value) - end - def with_type(type) - self.class.new(name, value_before_type_cast, type, database_default) + self.class.new(name, value_before_type_cast, type, original_attribute) end - - protected - - attr_reader :database_default end end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 43c15841c2..e8a782ed13 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -35,23 +35,22 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - clear_changes_information + @mutation_tracker = nil + @previous_mutation_tracker = nil + @changed_attributes = HashWithIndifferentAccess.new end end - def init_internals - super - @mutation_tracker = AttributeMutationTracker.new(@attributes, @attributes.dup) - end - def initialize_dup(other) # :nodoc: super - @mutation_tracker = AttributeMutationTracker.new(@attributes, - self.class._default_attributes.deep_dup) + @attributes = self.class._default_attributes.map do |attr| + attr.with_value_from_user(@attributes.fetch_value(attr.name)) + end + @mutation_tracker = nil end def changes_applied - @previous_mutation_tracker = @mutation_tracker + @previous_mutation_tracker = mutation_tracker @changed_attributes = HashWithIndifferentAccess.new store_original_attributes end @@ -81,7 +80,7 @@ module ActiveRecord if defined?(@cached_changed_attributes) @cached_changed_attributes else - super.reverse_merge(@mutation_tracker.changed_values).freeze + super.reverse_merge(mutation_tracker.changed_values).freeze end end @@ -96,17 +95,24 @@ module ActiveRecord end def attribute_changed_in_place?(attr_name) - @mutation_tracker.changed_in_place?(attr_name) + mutation_tracker.changed_in_place?(attr_name) end private + def mutation_tracker + unless defined?(@mutation_tracker) + @mutation_tracker = nil + end + @mutation_tracker ||= AttributeMutationTracker.new(@attributes) + end + def changes_include?(attr_name) - super || @mutation_tracker.changed?(attr_name) + super || mutation_tracker.changed?(attr_name) end def clear_attribute_change(attr_name) - @mutation_tracker.forget_change(attr_name) + mutation_tracker.forget_change(attr_name) end def _update_record(*) @@ -122,7 +128,8 @@ module ActiveRecord end def store_original_attributes - @mutation_tracker = @mutation_tracker.now_tracking(@attributes) + @attributes = @attributes.map(&:forgetting_assignment) + @mutation_tracker = nil end def previous_mutation_tracker diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb index 08eb8ba7ac..ba7348918b 100644 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ b/activerecord/lib/active_record/attribute_mutation_tracker.rb @@ -1,14 +1,13 @@ module ActiveRecord class AttributeMutationTracker # :nodoc: - def initialize(attributes, original_attributes) + def initialize(attributes) @attributes = attributes - @original_attributes = original_attributes end def changed_values attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| if changed?(attr_name) - result[attr_name] = original_attributes.fetch_value(attr_name) + result[attr_name] = attributes[attr_name].original_value end end end @@ -16,49 +15,34 @@ module ActiveRecord def changes attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| if changed?(attr_name) - result[attr_name] = [original_attributes.fetch_value(attr_name), attributes.fetch_value(attr_name)] + result[attr_name] = [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] end end end def changed?(attr_name) attr_name = attr_name.to_s - modified?(attr_name) || changed_in_place?(attr_name) + attributes[attr_name].changed? end def changed_in_place?(attr_name) - original_database_value = original_attributes[attr_name].value_before_type_cast - attributes[attr_name].changed_in_place_from?(original_database_value) + attributes[attr_name].changed_in_place? end def forget_change(attr_name) attr_name = attr_name.to_s - original_attributes[attr_name] = attributes[attr_name].dup - end - - def now_tracking(new_attributes) - AttributeMutationTracker.new(new_attributes, clean_copy_of(new_attributes)) + attributes[attr_name] = attributes[attr_name].forgetting_assignment end protected - attr_reader :attributes, :original_attributes + attr_reader :attributes private def attr_names attributes.keys end - - def modified?(attr_name) - attributes[attr_name].changed_from?(original_attributes.fetch_value(attr_name)) - end - - def clean_copy_of(attributes) - attributes.map do |attr| - attr.with_value_from_database(attr.value_for_database) - end - end end class NullMutationTracker # :nodoc: diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 7b53f6e5a0..3f02f73a5a 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -211,7 +211,7 @@ module ActiveRecord def becomes(klass) became = klass.new became.instance_variable_set("@attributes", @attributes) - became.instance_variable_set("@mutation_tracker", @mutation_tracker) + became.instance_variable_set("@mutation_tracker", @mutation_tracker) if defined?(@mutation_tracker) became.instance_variable_set("@changed_attributes", attributes_changed_by_setter) became.instance_variable_set("@new_record", new_record?) became.instance_variable_set("@destroyed", destroyed?) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index eb53a18f0f..ccb0ab18ae 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -558,11 +558,8 @@ module ActiveRecord end def where!(opts, *rest) # :nodoc: - if Hash === opts - opts = sanitize_forbidden_attributes(opts) - references!(PredicateBuilder.references(opts)) - end - + opts = sanitize_forbidden_attributes(opts) + references!(PredicateBuilder.references(opts)) if Hash === opts self.where_clause += where_clause_factory.build(opts, rest) self end @@ -619,6 +616,7 @@ module ActiveRecord end def having!(opts, *rest) # :nodoc: + opts = sanitize_forbidden_attributes(opts) references!(PredicateBuilder.references(opts)) if Hash === opts self.having_clause += having_clause_factory.build(opts, rest) diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index b67201d94d..52d197718e 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -177,7 +177,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase if current_adapter?(:MysqlAdapter, :Mysql2Adapter) def test_read_attributes_before_type_cast_on_boolean - bool = Boolean.create({ "value" => false }) + bool = Boolean.create!({ "value" => false }) if RUBY_PLATFORM =~ /java/ # JRuby will return the value before typecast as string assert_equal "0", bool.reload.attributes_before_type_cast["value"] @@ -542,9 +542,6 @@ class AttributeMethodsTest < ActiveRecord::TestCase developer.save! - assert_equal "50000", developer.salary_before_type_cast - assert_equal 1337, developer.name_before_type_cast - assert_equal 50000, developer.salary assert_equal "1337", developer.name end diff --git a/activerecord/test/cases/attribute_test.rb b/activerecord/test/cases/attribute_test.rb index 0f216b7a13..a24a4fc6a4 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activerecord/test/cases/attribute_test.rb @@ -182,12 +182,49 @@ module ActiveRecord assert attribute.has_been_read? end + test "an attribute is not changed if it hasn't been assigned or mutated" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + + refute attribute.changed? + end + + test "an attribute is changed if it's been assigned a new value" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + changed = attribute.with_value_from_user(2) + + assert changed.changed? + end + + test "an attribute is not changed if it's assigned the same value" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + unchanged = attribute.with_value_from_user(1) + + refute unchanged.changed? + end + test "an attribute can not be mutated if it has not been read, and skips expensive calculations" do type_which_raises_from_all_methods = Object.new attribute = Attribute.from_database(:foo, "bar", type_which_raises_from_all_methods) - assert_not attribute.changed_in_place_from?("bar") + assert_not attribute.changed_in_place? + end + + test "an attribute is changed if it has been mutated" do + attribute = Attribute.from_database(:foo, "bar", Type::String.new) + attribute.value << "!" + + assert attribute.changed_in_place? + assert attribute.changed? + end + + test "an attribute can forget its changes" do + attribute = Attribute.from_database(:foo, "bar", Type::String.new) + changed = attribute.with_value_from_user("foo") + forgotten = changed.forgetting_assignment + + assert changed.changed? # sanity check + refute forgotten.changed? end test "with_value_from_user validates the value" do diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index aa10817527..d904b802fa 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -681,4 +681,36 @@ class CalculationsTest < ActiveRecord::TestCase end assert block_called end + + def test_having_with_strong_parameters + protected_params = Class.new do + attr_reader :permitted + alias :permitted? :permitted + + def initialize(parameters) + @parameters = parameters + @permitted = false + end + + def to_h + @parameters + end + + def permit! + @permitted = true + self + end + end + + params = protected_params.new(credit_limit: '50') + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Account.group(:id).having(params) + end + + result = Account.group(:id).having(params.permit!) + assert_equal 50, result[0].credit_limit + assert_equal 50, result[1].credit_limit + assert_equal 50, result[2].credit_limit + end end diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index 80d17b8114..9169207b0a 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -96,7 +96,9 @@ class IntegrationTest < ActiveRecord::TestCase owner.update_column :updated_at, Time.current key = owner.cache_key - assert pet.touch + travel(1.second) do + assert pet.touch + end assert_not_equal key, owner.reload.cache_key end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index 6af31017d6..c3a1471205 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -276,5 +276,31 @@ module ActiveRecord assert_equal essays(:david_modest_proposal), essay end + + def test_where_with_strong_parameters + protected_params = Class.new do + attr_reader :permitted + alias :permitted? :permitted + + def initialize(parameters) + @parameters = parameters + @permitted = false + end + + def to_h + @parameters + end + + def permit! + @permitted = true + self + end + end + + author = authors(:david) + params = protected_params.new(name: author.name) + assert_raises(ActiveModel::ForbiddenAttributesError) { Author.where(params) } + assert_equal author, Author.where(params.permit!).first + end end end diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index a82a7dfea0..3592dcba39 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -41,6 +41,9 @@ module ActiveSupport 'Time' end + PRECISIONS = Hash.new { |h, n| h[n] = "%FT%T.%#{n}N".freeze } + PRECISIONS[0] = '%FT%T'.freeze + include Comparable attr_reader :time_zone @@ -142,11 +145,7 @@ module ActiveSupport # # Time.zone.now.xmlschema # => "2014-12-04T11:02:37-05:00" def xmlschema(fraction_digits = 0) - fraction = if fraction_digits.to_i > 0 - (".%06i" % time.usec)[0, fraction_digits.to_i + 1] - end - - "#{time.strftime("%Y-%m-%dT%H:%M:%S")}#{fraction}#{formatted_offset(true, 'Z')}" + "#{time.strftime(PRECISIONS[fraction_digits.to_i])}#{formatted_offset(true, 'Z'.freeze)}" end alias_method :iso8601, :xmlschema diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index ccb7f02331..c40f0bacbf 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -110,14 +110,14 @@ class TimeWithZoneTest < ActiveSupport::TestCase @twz += 0.1234560001 # advance the time by a fraction of a second assert_equal "1999-12-31T19:00:00.123-05:00", @twz.xmlschema(3) assert_equal "1999-12-31T19:00:00.123456-05:00", @twz.xmlschema(6) - assert_equal "1999-12-31T19:00:00.123456-05:00", @twz.xmlschema(12) + assert_equal "1999-12-31T19:00:00.123456000100-05:00", @twz.xmlschema(12) end def test_xmlschema_with_fractional_seconds_lower_than_hundred_thousand @twz += 0.001234 # advance the time by a fraction assert_equal "1999-12-31T19:00:00.001-05:00", @twz.xmlschema(3) assert_equal "1999-12-31T19:00:00.001234-05:00", @twz.xmlschema(6) - assert_equal "1999-12-31T19:00:00.001234-05:00", @twz.xmlschema(12) + assert_equal "1999-12-31T19:00:00.001234000000-05:00", @twz.xmlschema(12) end def test_xmlschema_with_nil_fractional_seconds |