diff options
Diffstat (limited to 'activerecord')
23 files changed, 176 insertions, 85 deletions
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 53fa132219..ab85414277 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -32,6 +32,7 @@ module ActiveRecord extend ActiveSupport::Autoload autoload :Attribute + autoload :AttributeSet autoload :Base autoload :Callbacks autoload :Core diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 0ad5206980..34a555dfd4 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -15,7 +15,10 @@ module ActiveRecord::Associations::Builder end private - def klass; @rhs_class_name.constantize; end + + def klass + @lhs_class.send(:compute_type, @rhs_class_name) + end end def self.build(lhs_class, name, options) @@ -23,13 +26,7 @@ module ActiveRecord::Associations::Builder KnownTable.new options[:join_table].to_s else class_name = options.fetch(:class_name) { - model_name = name.to_s.camelize.singularize - - if lhs_class.parent_name - model_name.prepend("#{lhs_class.parent_name}::") - end - - model_name + name.to_s.camelize.singularize } KnownClass.new lhs_class, class_name end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 306588ac66..065a2cff01 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -55,9 +55,9 @@ module ActiveRecord # Implements the ids writer method, e.g. foo.item_ids= for Foo.has_many :items def ids_writer(ids) - pk_column = reflection.primary_key_column + pk_type = reflection.primary_key_type ids = Array(ids).reject { |id| id.blank? } - ids.map! { |i| pk_column.type_cast_from_user(i) } + ids.map! { |i| pk_type.type_cast_from_user(i) } replace(klass.find(ids).index_by { |r| r.id }.values_at(*ids)) end 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 175019a72b..35e176622d 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -93,7 +93,9 @@ module ActiveRecord end def through_scope_attributes - scope.where_values_hash(through_association.reflection.name.to_s).except!(through_association.reflection.foreign_key) + scope.where_values_hash(through_association.reflection.name.to_s). + except!(through_association.reflection.foreign_key, + through_association.reflection.klass.inheritance_column) end def save_through_record(record) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index b4d75d6556..268cec6160 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -230,7 +230,7 @@ module ActiveRecord # 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.any? && self.class.column_names.include?(name) + if defined?(@attributes) && self.class.column_names.include?(name) return has_attribute?(name) end @@ -247,7 +247,7 @@ module ActiveRecord # person.has_attribute?('age') # => true # person.has_attribute?(:nothing) # => false def has_attribute?(attr_name) - @attributes.has_key?(attr_name.to_s) + @attributes.include?(attr_name.to_s) end # Returns an array of names for the attributes available on this object. @@ -367,12 +367,6 @@ module ActiveRecord protected - def clone_attributes # :nodoc: - @attributes.each_with_object({}) do |(name, attr), h| - h[name] = attr.dup - end - end - def clone_attribute_value(reader_method, attribute_name) # :nodoc: value = send(reader_method, attribute_name) value.duplicable? ? value.clone : value diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb new file mode 100644 index 0000000000..102ef17e16 --- /dev/null +++ b/activerecord/lib/active_record/attribute_set.rb @@ -0,0 +1,51 @@ +module ActiveRecord + class AttributeSet # :nodoc: + delegate :[], :[]=, :fetch, :include?, :keys, :each_with_object, to: :attributes + + def initialize(attributes) + @attributes = attributes + end + + def update(other) + attributes.update(other.attributes) + end + + def freeze + @attributes.freeze + super + end + + def initialize_dup(_) + @attributes = attributes.dup + attributes.each do |key, attr| + attributes[key] = attr.dup + end + + super + end + + def initialize_clone(_) + @attributes = attributes.clone + super + end + + class Builder + def initialize(types) + @types = types + end + + def build_from_database(values, additional_types = {}) + attributes = values.each_with_object({}) do |(name, value), hash| + type = additional_types.fetch(name, @types[name]) + hash[name] = Attribute.from_database(value, type) + end + AttributeSet.new(attributes) + end + end + + protected + + attr_reader :attributes + + end +end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 9c80121d70..d0287140c8 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -109,13 +109,14 @@ module ActiveRecord end def clear_caches_calculated_from_columns - @columns = nil - @columns_hash = nil - @column_types = nil + @attributes_builder = nil @column_defaults = nil - @raw_column_defaults = nil @column_names = nil + @column_types = nil + @columns = nil + @columns_hash = nil @content_columns = nil + @raw_column_defaults = nil end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 04ae67234f..ff92375820 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -14,8 +14,8 @@ module ActiveRecord # value. Is this really the only case? Are we missing tests for other types? # We should have a real column object passed (or nil) here, and check for that # instead - if column.respond_to?(:type_cast_for_database) - value = column.type_cast_for_database(value) + if column.respond_to?(:cast_type) + value = column.cast_type.type_cast_for_database(value) end _quote(value) @@ -34,8 +34,8 @@ module ActiveRecord # value. Is this really the only case? Are we missing tests for other types? # We should have a real column object passed (or nil) here, and check for that # instead - if column.respond_to?(:type_cast_for_database) - value = column.type_cast_for_database(value) + if column.respond_to?(:cast_type) + value = column.cast_type.type_cast_for_database(value) end _type_cast(value) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 17fabe5af6..3fea8f490d 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -179,8 +179,8 @@ module ActiveRecord end def array_column(column) - if column.array && !column.respond_to?(:type_cast_for_database) - OID::Array.new(AdapterProxyType.new(column, self)) + if column.array && !column.respond_to?(:cast_type) + Column.new('', nil, OID::Array.new(AdapterProxyType.new(column, self))) else column end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 71b05cdbae..6d5151dfe4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -306,10 +306,6 @@ module ActiveRecord self.client_min_messages = old end - def supports_insert_with_returning? - true - end - def supports_ddl_transactions? true end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index d39e5fddfe..e8e165a7c8 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -251,11 +251,10 @@ module ActiveRecord def initialize(attributes = nil, options = {}) defaults = {} self.class.raw_column_defaults.each do |k, v| - default = v.duplicable? ? v.dup : v - defaults[k] = Attribute.from_database(default, type_for_attribute(k)) + defaults[k] = v.duplicable? ? v.dup : v end - @attributes = defaults + @attributes = self.class.attributes_builder.build_from_database(defaults) @column_types = self.class.column_types init_internals @@ -325,7 +324,7 @@ module ActiveRecord ## def initialize_dup(other) # :nodoc: pk = self.class.primary_key - @attributes = other.clone_attributes + @attributes = @attributes.dup @attributes[pk] = Attribute.from_database(nil, type_for_attribute(pk)) run_callbacks(:initialize) unless _initialize_callbacks.empty? diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 7c794377e0..099042cab2 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -219,12 +219,18 @@ module ActiveRecord connection.schema_cache.table_exists?(table_name) end + def attributes_builder # :nodoc: + @attributes_builder ||= AttributeSet::Builder.new(column_types) + end + def column_types # :nodoc: - @column_types ||= Hash[columns.map { |column| [column.name, column.cast_type] }] + @column_types ||= Hash.new(Type::Value.new).tap do |column_types| + columns.each { |column| column_types[column.name] = column.cast_type } + end end def type_for_attribute(attr_name) # :nodoc: - column_types.fetch(attr_name) { Type::Value.new } + column_types[attr_name] end # Returns a hash where the keys are column names and the values are diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 5c744762d9..6707f12489 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -48,11 +48,7 @@ module ActiveRecord # how this "single-table" inheritance mapping is implemented. def instantiate(attributes, column_types = {}) klass = discriminate_class_for_record(attributes) - - attributes = attributes.each_with_object({}) do |(name, value), h| - type = column_types.fetch(name) { klass.type_for_attribute(name) } - h[name] = Attribute.from_database(value, type) - end + attributes = klass.attributes_builder.build_from_database(attributes, column_types) klass.allocate.init_with('attributes' => attributes, 'new_record' => false) end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 51c96373ee..28c39bdd5c 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -290,8 +290,8 @@ module ActiveRecord @foreign_key ||= options[:foreign_key] || derive_foreign_key end - def primary_key_column - klass.columns_hash[klass.primary_key] + def primary_key_type + klass.type_for_attribute(klass.primary_key) end def association_foreign_key diff --git a/activerecord/lib/active_record/type/numeric.rb b/activerecord/lib/active_record/type/numeric.rb index 137c9e4c99..a7bf0657b9 100644 --- a/activerecord/lib/active_record/type/numeric.rb +++ b/activerecord/lib/active_record/type/numeric.rb @@ -16,26 +16,20 @@ module ActiveRecord end def changed?(old_value, _new_value, new_value_before_type_cast) # :nodoc: - # 0 => 'wibble' should mark as changed so numericality validations run - if nil_or_zero?(old_value) && non_numeric_string?(new_value_before_type_cast) - # nil => '' should not mark as changed - old_value != new_value_before_type_cast.presence - else - super - end + super || zero_to_non_number?(old_value, new_value_before_type_cast) end private + def zero_to_non_number?(old_value, new_value_before_type_cast) + old_value == 0 && non_numeric_string?(new_value_before_type_cast) + end + def non_numeric_string?(value) # 'wibble'.to_i will give zero, we want to make sure # that we aren't marking int zero to string zero as # changed. - value !~ /\A\d+\.?\d*\z/ - end - - def nil_or_zero?(value) - value.nil? || value == 0 + value.to_s !~ /\A\d+\.?\d*\z/ end end end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 2e7b1d7206..04e28a0cfe 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -48,7 +48,7 @@ module ActiveRecord def build_relation(klass, table, attribute, value) #:nodoc: if reflection = klass._reflect_on_association(attribute) attribute = reflection.foreign_key - value = value.attributes[reflection.primary_key_column.name] unless value.nil? + value = value.attributes[reflection.klass.primary_key] unless value.nil? end attribute_name = attribute.to_s diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 080c499444..cc58a4a1a2 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -860,7 +860,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 'edges', Vertex.reflect_on_association(:sources).join_table end - def test_namespaced_habtm + def test_has_and_belongs_to_many_in_a_namespaced_model_pointing_to_a_namespaced_model magazine = Publisher::Magazine.create article = Publisher::Article.create magazine.articles << article @@ -869,6 +869,15 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_includes magazine.articles, article end + def test_has_and_belongs_to_many_in_a_namespaced_model_pointing_to_a_non_namespaced_model + article = Publisher::Article.create + tag = Tag.create + article.tags << tag + article.save + + assert_includes article.tags, tag + end + def test_redefine_habtm child = SubDeveloper.new("name" => "Aredridel") child.special_projects << SpecialProject.new("name" => "Special Project") 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 8641584c0c..ad2aa99af5 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1139,4 +1139,12 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal 2, post.lazy_readers_unscope_skimmers.to_a.size assert_equal 2, post.lazy_people_unscope_skimmers.to_a.size end + + def test_has_many_through_add_with_sti_middle_relation + club = SuperClub.create!(name: 'Fight Club') + member = Member.create!(name: 'Tyler Durden') + + club.members << member + assert_equal 1, SuperMembership.where(member_id: member.id, club_id: club.id).count + end end diff --git a/activerecord/test/cases/attribute_set_test.rb b/activerecord/test/cases/attribute_set_test.rb new file mode 100644 index 0000000000..091f7e396a --- /dev/null +++ b/activerecord/test/cases/attribute_set_test.rb @@ -0,0 +1,49 @@ +require 'cases/helper' + +module ActiveRecord + class AttributeSetTest < ActiveRecord::TestCase + test "building a new set from raw attributes" do + builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) + attributes = builder.build_from_database(foo: '1.1', bar: '2.2') + + assert_equal 1, attributes[:foo].value + assert_equal 2.2, attributes[:bar].value + end + + test "building with custom types" do + builder = AttributeSet::Builder.new(foo: Type::Float.new) + attributes = builder.build_from_database({ foo: '3.3', bar: '4.4' }, { bar: Type::Integer.new }) + + assert_equal 3.3, attributes[:foo].value + assert_equal 4, attributes[:bar].value + end + + test "duping creates a new hash and dups each attribute" do + builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::String.new) + attributes = builder.build_from_database(foo: 1, bar: 'foo') + + # Ensure the type cast value is cached + attributes[:foo].value + attributes[:bar].value + + duped = attributes.dup + duped[:foo] = Attribute.from_database(2, Type::Integer.new) + duped[:bar].value << 'bar' + + assert_equal 1, attributes[:foo].value + assert_equal 2, duped[:foo].value + assert_equal 'foo', attributes[:bar].value + assert_equal 'foobar', duped[:bar].value + end + + test "freezing cloned set does not freeze original" do + attributes = AttributeSet.new({}) + clone = attributes.clone + + clone.freeze + + assert clone.frozen? + assert_not attributes.frozen? + end + end +end diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index bbd5298da1..70d9b9dbf5 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -93,12 +93,10 @@ module ActiveRecord def test_quote_true assert_equal @quoter.quoted_true, @quoter.quote(true, nil) - assert_equal '1', @quoter.quote(true, Type::Integer.new) end def test_quote_false assert_equal @quoter.quoted_false, @quoter.quote(false, nil) - assert_equal '0', @quoter.quote(false, Type::Integer.new) end def test_quote_float @@ -157,25 +155,6 @@ module ActiveRecord assert_equal "'lo\\\\l'", @quoter.quote(string, nil) end - def test_quote_string_int_column - assert_equal "1", @quoter.quote('1', Type::Integer.new) - assert_equal "1", @quoter.quote('1.2', Type::Integer.new) - end - - def test_quote_string_float_column - assert_equal "1.0", @quoter.quote('1', Type::Float.new) - assert_equal "1.2", @quoter.quote('1.2', Type::Float.new) - end - - def test_quote_as_mb_chars_binary_column - string = ActiveSupport::Multibyte::Chars.new('lo\l') - assert_equal "'lo\\\\l'", @quoter.quote(string, Type::Binary.new) - end - - def test_quote_binary_without_string_to_binary - assert_equal "'lo\\\\l'", @quoter.quote('lo\l', Type::Binary.new) - end - def test_string_with_crazy_column assert_equal "'lo\\\\l'", @quoter.quote('lo\l') end @@ -183,10 +162,6 @@ module ActiveRecord def test_quote_duration assert_equal "1800", @quoter.quote(30.minutes) end - - def test_quote_duration_int_column - assert_equal "7200", @quoter.quote(2.hours, Type::Integer.new) - end end end end diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb index a762ad4bb5..6ceafe5858 100644 --- a/activerecord/test/models/club.rb +++ b/activerecord/test/models/club.rb @@ -14,3 +14,10 @@ class Club < ActiveRecord::Base "I'm sorry sir, this is a *private* club, not a *pirate* club" end end + +class SuperClub < ActiveRecord::Base + self.table_name = "clubs" + + has_many :memberships, class_name: 'SuperMembership', foreign_key: 'club_id' + has_many :members, through: :memberships +end diff --git a/activerecord/test/models/publisher/article.rb b/activerecord/test/models/publisher/article.rb index 03a277bbdd..d73a8eb936 100644 --- a/activerecord/test/models/publisher/article.rb +++ b/activerecord/test/models/publisher/article.rb @@ -1,3 +1,4 @@ class Publisher::Article < ActiveRecord::Base has_and_belongs_to_many :magazines + has_and_belongs_to_many :tags end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index a7c7fc70bc..fd85050dd4 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -62,6 +62,11 @@ ActiveRecord::Schema.define do t.references :magazine end + create_table :articles_tags, force: true do |t| + t.references :article + t.references :tag + end + create_table :audit_logs, force: true do |t| t.column :message, :string, null: false t.column :developer_id, :integer, null: false |