diff options
Diffstat (limited to 'activerecord')
23 files changed, 214 insertions, 55 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index efa4abfa4d..b3df23c623 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,14 @@ +* Ensure both parent IDs are set on join records when both sides of a + through association are new. + + *Sean Griffin* + +* `ActiveRecord::Dirty` now detects in-place changes to mutable values. + Serialized attributes on ActiveRecord models will no longer save when + unchanged. Fixes #8328. + + Sean Griffin + * Fixed automatic maintaining test schema to properly handle sql structure schema format. diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index da9b125fd6..1713ab7ed3 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -180,7 +180,11 @@ module ActiveRecord def through_records_for(record) attributes = construct_join_attributes(record) candidates = Array.wrap(through_association.target) - candidates.find_all { |c| c.attributes.slice(*attributes.keys) == attributes } + candidates.find_all do |c| + attributes.all? do |key, value| + c.public_send(key) == value + end + end end def delete_through_records(records) diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index fcf3b219d4..f00fef8b9e 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -41,12 +41,16 @@ module ActiveRecord def construct_join_attributes(*records) ensure_mutable - join_attributes = { - source_reflection.foreign_key => - records.map { |record| - record.send(source_reflection.association_primary_key(reflection.klass)) - } - } + if source_reflection.association_primary_key(reflection.klass) == reflection.klass.primary_key + join_attributes = { source_reflection.name => records } + else + join_attributes = { + source_reflection.foreign_key => + records.map { |record| + record.send(source_reflection.association_primary_key(reflection.klass)) + } + } + end if options[:source_type] join_attributes[source_reflection.foreign_type] = diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index be438aba95..6a5c057384 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -38,12 +38,39 @@ module ActiveRecord end end - def initialize_dup(other) # :nodoc: - super - init_changed_attributes - end + def initialize_dup(other) # :nodoc: + super + init_changed_attributes + end + + def changed? + super || changed_in_place.any? + end + + def changed + super | changed_in_place + end + + def attribute_changed?(attr_name, options = {}) + result = super + # We can't change "from" something in place. Only setters can define + # "from" and "to" + result ||= changed_in_place?(attr_name) unless options.key?(:from) + result + end + + def changes_applied + super + store_original_raw_attributes + end + + def reset_changes + super + original_raw_attributes.clear + end + + private - private def initialize_internals_callback super init_changed_attributes @@ -65,11 +92,20 @@ module ActiveRecord old_value = old_attribute_value(attr) - result = super(attr, value) + result = super + store_original_raw_attribute(attr) save_changed_attribute(attr, old_value) result end + def raw_write_attribute(attr, value) + attr = attr.to_s + + result = super + original_raw_attributes[attr] = value + result + end + def save_changed_attribute(attr, old_value) if attribute_changed?(attr) changed_attributes.delete(attr) unless _field_changed?(attr, old_value) @@ -105,6 +141,41 @@ module ActiveRecord raw_value = read_attribute_before_type_cast(attr) column_for_attribute(attr).changed?(old_value, new_value, raw_value) end + + def changed_in_place + self.class.attribute_names.select do |attr_name| + changed_in_place?(attr_name) + end + end + + def changed_in_place?(attr_name) + type = type_for_attribute(attr_name) + old_value = original_raw_attribute(attr_name) + value = read_attribute(attr_name) + type.changed_in_place?(old_value, value) + end + + def original_raw_attribute(attr_name) + original_raw_attributes.fetch(attr_name) do + read_attribute_before_type_cast(attr_name) + end + end + + def original_raw_attributes + @original_raw_attributes ||= {} + end + + def store_original_raw_attribute(attr_name) + type = type_for_attribute(attr_name) + value = type.type_cast_for_database(read_attribute(attr_name)) + original_raw_attributes[attr_name] = value + end + + def store_original_raw_attributes + attribute_names.each do |attr| + store_original_raw_attribute(attr) + end + end end end end diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index 60debb7d18..cec50f62a3 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -50,8 +50,6 @@ module ActiveRecord # serialize :preferences, Hash # end def serialize(attr_name, class_name_or_coder = Object) - include Behavior - coder = if [:load, :dump].all? { |x| class_name_or_coder.respond_to?(x) } class_name_or_coder else @@ -67,21 +65,6 @@ module ActiveRecord self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder) end end - - - # This is only added to the model when serialize is called, which - # ensures we do not make things slower when serialization is not used. - module Behavior # :nodoc: - extend ActiveSupport::Concern - - def should_record_timestamps? - super || (self.record_timestamps && (attributes.keys & self.class.serialized_attributes.keys).present?) - end - - def keys_for_partial_write - super | (attributes.keys & self.class.serialized_attributes.keys) - end - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb index 88de816d4f..0dd4b65333 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb @@ -3,14 +3,12 @@ module ActiveRecord module PostgreSQL module OID # :nodoc: class Hstore < Type::Value + include Type::Mutable + def type :hstore end - def type_cast_from_user(value) - type_cast_from_database(type_cast_for_database(value)) - end - def type_cast_from_database(value) ConnectionAdapters::PostgreSQLColumn.string_to_hstore(value) end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb index b4fed1bcab..d1347c7bb5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb @@ -3,14 +3,12 @@ module ActiveRecord module PostgreSQL module OID # :nodoc: class Json < Type::Value + include Type::Mutable + def type :json end - def type_cast_from_user(value) - type_cast_from_database(type_cast_for_database(value)) - end - def type_cast_from_database(value) ConnectionAdapters::PostgreSQLColumn.string_to_json(value) end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 6c089b243e..d39e5fddfe 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -256,7 +256,6 @@ module ActiveRecord end @attributes = defaults - @column_types_override = nil @column_types = self.class.column_types init_internals @@ -283,7 +282,6 @@ module ActiveRecord # post.title # => 'hello world' def init_with(coder) @attributes = coder['attributes'] - @column_types_override = coder['column_types'] @column_types = self.class.column_types init_internals @@ -357,7 +355,6 @@ module ActiveRecord # FIXME: Remove this when we better serialize attributes coder['raw_attributes'] = attributes_before_type_cast coder['attributes'] = @attributes - coder['column_types'] = @column_types_override coder['new_record'] = new_record? end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index c69a83c125..5c744762d9 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -53,12 +53,7 @@ module ActiveRecord type = column_types.fetch(name) { klass.type_for_attribute(name) } h[name] = Attribute.from_database(value, type) end - - klass.allocate.init_with( - 'attributes' => attributes, - 'column_types' => column_types, - 'new_record' => false, - ) + klass.allocate.init_with('attributes' => attributes, 'new_record' => false) end private @@ -406,8 +401,7 @@ module ActiveRecord @attributes.update(fresh_object.instance_variable_get('@attributes')) - @column_types = self.class.column_types - @column_types_override = fresh_object.instance_variable_get('@column_types_override') + @column_types = self.class.column_types self end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 95a3c70c93..51c96373ee 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -487,7 +487,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi # returns either nil or the inverse association name that it finds. def automatic_inverse_of if can_find_inverse_of_automatically?(self) - inverse_name = ActiveSupport::Inflector.underscore(active_record.name).to_sym + inverse_name = ActiveSupport::Inflector.underscore(options[:as] || active_record.name).to_sym begin reflection = klass._reflect_on_association(inverse_name) diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index e9b827886a..f1384e0bb2 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -1,3 +1,4 @@ +require 'active_record/type/mutable' require 'active_record/type/numeric' require 'active_record/type/time_value' require 'active_record/type/value' diff --git a/activerecord/lib/active_record/type/mutable.rb b/activerecord/lib/active_record/type/mutable.rb new file mode 100644 index 0000000000..64cf4b9b93 --- /dev/null +++ b/activerecord/lib/active_record/type/mutable.rb @@ -0,0 +1,16 @@ +module ActiveRecord + module Type + module Mutable + def type_cast_from_user(value) + type_cast_from_database(type_cast_for_database(value)) + end + + # +raw_old_value+ will be the `_before_type_cast` version of the + # value (likely a string). +new_value+ will be the current, type + # cast value. + def changed_in_place?(raw_old_value, new_value) # :nodoc: + raw_old_value != type_cast_for_database(new_value) + end + end + end +end diff --git a/activerecord/lib/active_record/type/serialized.rb b/activerecord/lib/active_record/type/serialized.rb index e6d84eadc0..ebde14634c 100644 --- a/activerecord/lib/active_record/type/serialized.rb +++ b/activerecord/lib/active_record/type/serialized.rb @@ -1,6 +1,8 @@ module ActiveRecord module Type class Serialized < SimpleDelegator # :nodoc: + include Mutable + attr_reader :subtype, :coder def initialize(subtype, coder) @@ -17,10 +19,6 @@ module ActiveRecord end end - def type_cast_from_user(value) - type_cast_from_database(type_cast_for_database(value)) - end - def type_cast_for_database(value) return if value.nil? unless is_default_value?(value) diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb index efcdab1c0e..b34d1697cd 100644 --- a/activerecord/lib/active_record/type/value.rb +++ b/activerecord/lib/active_record/type/value.rb @@ -60,6 +60,10 @@ module ActiveRecord old_value != new_value end + def changed_in_place?(*) # :nodoc: + false + end + private # Takes an input from the database, or from attribute setters, # and casts it to a type appropriate for this object. This method diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index a25c9cb5e4..83b495d600 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -163,6 +163,15 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase assert_equal "GMT", y.timezone end + def test_changes_in_place + hstore = Hstore.create!(settings: { 'one' => 'two' }) + hstore.settings['three'] = 'four' + hstore.save! + hstore.reload + + assert_equal 'four', hstore.settings['three'] + end + def test_gen1 assert_equal(%q(" "=>""), @column.class.hstore_to_string({' '=>''})) end diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index 3ee8839823..a3400a5a19 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -165,4 +165,24 @@ class PostgresqlJSONTest < ActiveRecord::TestCase JsonDataType.update_all payload: { } assert_equal({ }, json.reload.payload) end + + def test_changes_in_place + json = JsonDataType.new + assert_not json.changed? + + json.payload = { 'one' => 'two' } + assert json.changed? + assert json.payload_changed? + + json.save! + assert_not json.changed? + + json.payload['three'] = 'four' + assert json.payload_changed? + + json.save! + json.reload + + assert json.payload['three'] = 'four' + end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 2e62189e7a..8641584c0c 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -330,6 +330,19 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert post.single_people.include?(person) end + def test_both_parent_ids_set_when_saving_new + post = Post.new(title: 'Hello', body: 'world') + person = Person.new(first_name: 'Sean') + + post.people = [person] + post.save + + assert post.id + assert person.id + assert_equal post.id, post.readers.first.post_id + assert_equal person.id, post.readers.first.person_id + end + def test_delete_association assert_queries(2){posts(:welcome);people(:michael); } diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index a2725441b3..089cb0a3a2 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -45,6 +45,20 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_equal clubs(:moustache_club), new_member.club end + def test_creating_association_sets_both_parent_ids_for_new + member = Member.new(name: 'Sean Griffin') + club = Club.new(name: 'Da Club') + + member.club = club + + member.save! + + assert member.id + assert club.id + assert_equal member.id, member.current_membership.member_id + assert_equal club.id, member.current_membership.club_id + end + def test_replace_target_record new_club = Club.create(:name => "Marx Bros") @member.club = new_club diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index a674a39d65..60df4e14dd 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -100,6 +100,17 @@ class AutomaticInverseFindingTests < ActiveRecord::TestCase assert_respond_to club_reflection, :has_inverse? assert !club_reflection.has_inverse?, "A has_many_through association should not find an inverse automatically" end + + def test_polymorphic_relationships_should_still_not_have_inverses_when_non_polymorphic_relationship_has_the_same_name + man_reflection = Man.reflect_on_association(:polymorphic_face_without_inverse) + face_reflection = Face.reflect_on_association(:man) + + assert_respond_to face_reflection, :has_inverse? + assert face_reflection.has_inverse?, "For this test, the non-polymorphic association must have an inverse" + + assert_respond_to man_reflection, :has_inverse? + assert !man_reflection.has_inverse?, "The target of a polymorphic association should not find an inverse automatically" + end end class InverseAssociationTests < ActiveRecord::TestCase diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 87f24e32b2..5d6601a881 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -445,11 +445,20 @@ class DirtyTest < ActiveRecord::TestCase def test_save_should_store_serialized_attributes_even_with_partial_writes with_partial_writes(Topic) do topic = Topic.create!(:content => {:a => "a"}) + + assert_not topic.changed? + topic.content[:b] = "b" - #assert topic.changed? # Known bug, will fail + + assert topic.changed? + topic.save! + + assert_not topic.changed? assert_equal "b", topic.content[:b] + topic.reload + assert_equal "b", topic.content[:b] end end diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb index edb75d333f..3d7f0626e2 100644 --- a/activerecord/test/models/face.rb +++ b/activerecord/test/models/face.rb @@ -1,6 +1,7 @@ class Face < ActiveRecord::Base belongs_to :man, :inverse_of => :face belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_face + belongs_to :polymorphic_man_without_inverse, :polymorphic => true # These is a "broken" inverse_of for the purposes of testing belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face belongs_to :horrible_polymorphic_man, :polymorphic => true, :inverse_of => :horrible_polymorphic_face diff --git a/activerecord/test/models/man.rb b/activerecord/test/models/man.rb index f4d127730c..a26491ce61 100644 --- a/activerecord/test/models/man.rb +++ b/activerecord/test/models/man.rb @@ -1,6 +1,7 @@ class Man < ActiveRecord::Base has_one :face, :inverse_of => :man has_one :polymorphic_face, :class_name => 'Face', :as => :polymorphic_man, :inverse_of => :polymorphic_man + has_one :polymorphic_face_without_inverse, :class_name => 'Face', :as => :polymorphic_man_without_inverse has_many :interests, :inverse_of => :man has_many :polymorphic_interests, :class_name => 'Interest', :as => :polymorphic_man, :inverse_of => :polymorphic_man # These are "broken" inverse_of associations for the purposes of testing diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 98fd0b6803..a7c7fc70bc 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -775,6 +775,8 @@ ActiveRecord::Schema.define do t.integer :man_id t.integer :polymorphic_man_id t.string :polymorphic_man_type + t.integer :polymorphic_man_without_inverse_id + t.string :polymorphic_man_without_inverse_type t.integer :horrible_polymorphic_man_id t.string :horrible_polymorphic_man_type end |