diff options
Diffstat (limited to 'activerecord')
56 files changed, 1611 insertions, 911 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 29f06dc06a..3f62cea5f5 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -2,6 +2,44 @@ * Associations with a :through option can now use *any* association as the through or source association, including other associations which have a :through option and has_and_belongs_to_many associations #1812 [Jon Leighton] +* ActiveRecord::Base#dup and ActiveRecord::Base#clone semantics have changed +to closer match normal Ruby dup and clone semantics. + +* Calling ActiveRecord::Base#clone will result in a shallow copy of the record, +including copying the frozen state. No callbacks will be called. + +* Calling ActiveRecord::Base#dup will duplicate the record, including calling +after initialize hooks. Frozen state will not be copied, and all associations +will be cleared. A duped record will return true for new_record?, have a nil +id field, and is saveable. + +* Migrations can be defined as reversible, meaning that the migration system +will figure out how to reverse your migration. To use reversible migrations, +just define the "change" method. For example: + + class MyMigration < ActiveRecord::Migration + def change + create_table(:horses) do + t.column :content, :text + t.column :remind_at, :datetime + end + end + end + +Some things cannot be automatically reversed for you. If you know how to +reverse those things, you should define 'up' and 'down' in your migration. If +you define something in `change` that cannot be reversed, an +IrreversibleMigration exception will be raised when going down. + +* Migrations should use instance methods rather than class methods: + class FooMigration < ActiveRecord::Migration + def up + ... + end + end + + [Aaron Patterson] + * has_one maintains the association with separate after_create/after_update instead of a single after_save. [fxn] diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 395c72dfbc..f9b77c1799 100644..100755 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -1,4 +1,4 @@ -require 'rake' +#!/usr/bin/env rake require 'rake/testtask' require 'rake/packagetask' require 'rake/gempackagetask' diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 8e7416472f..8ca83a7e75 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -193,13 +193,17 @@ module ActiveRecord conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}" conditions << append_conditions(reflection, preload_options) - associated_records = reflection.klass.unscoped.where([conditions, ids]). + associated_records_proxy = reflection.klass.unscoped. includes(options[:include]). joins("INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}"). select("#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id"). - order(options[:order]).to_a + order(options[:order]) - set_association_collection_records(id_to_record_map, reflection.name, associated_records, 'the_parent_record_id') + all_associated_records = associated_records(ids) do |some_ids| + associated_records_proxy.where([conditions, ids]).to_a + end + + set_association_collection_records(id_to_record_map, reflection.name, all_associated_records, 'the_parent_record_id') end def preload_has_one_or_has_many_association(records, reflection, preload_options={}) @@ -376,13 +380,14 @@ module ActiveRecord find_options = { :select => preload_options[:select] || options[:select] || Arel::SqlLiteral.new("#{table_name}.*"), :include => preload_options[:include] || options[:include], - :conditions => [conditions, ids], :joins => options[:joins], :group => preload_options[:group] || options[:group], :order => preload_options[:order] || options[:order] } - reflection.klass.scoped.apply_finder_options(find_options).to_a + associated_records(ids) do |some_ids| + reflection.klass.scoped.apply_finder_options(find_options.merge(:conditions => [conditions, some_ids])).to_a + end end @@ -400,6 +405,17 @@ module ActiveRecord def in_or_equals_for_ids(ids) ids.size > 1 ? "IN (?)" : "= ?" end + + # Some databases impose a limit on the number of ids in a list (in Oracle its 1000) + # Make several smaller queries if necessary or make one query if the adapter supports it + def associated_records(ids) + in_clause_length = connection.in_clause_length || ids.size + records = [] + ids.each_slice(in_clause_length) do |some_ids| + records += yield(some_ids) + end + records + end end end end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index e82cbd0aa6..fe22fa7ca2 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -4,6 +4,8 @@ require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/string/conversions' require 'active_support/core_ext/module/remove_method' +require 'active_support/core_ext/class/attribute' +require 'active_record/associations/class_methods/join_dependency' module ActiveRecord class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc: @@ -1853,12 +1855,12 @@ module ActiveRecord callbacks.each do |callback_name| full_callback_name = "#{callback_name}_for_#{association_name}" defined_callbacks = options[callback_name.to_sym] - if options.has_key?(callback_name.to_sym) - class_inheritable_reader full_callback_name.to_sym - write_inheritable_attribute(full_callback_name.to_sym, [defined_callbacks].flatten) - else - write_inheritable_attribute(full_callback_name.to_sym, []) - end + + full_callback_value = options.has_key?(callback_name.to_sym) ? [defined_callbacks].flatten : [] + + # TODO : why do i need method_defined? I think its because of the inheritance chain + class_attribute full_callback_name.to_sym unless method_defined?(full_callback_name) + self.send("#{full_callback_name}=", full_callback_value) end end @@ -1874,552 +1876,6 @@ module ActiveRecord Array.wrap(extensions) end end - - class JoinDependency # :nodoc: - attr_reader :join_parts, :reflections, :alias_tracker - - def initialize(base, associations, joins) - @join_parts = [JoinBase.new(base, joins)] - @associations = {} - @reflections = [] - @alias_tracker = AliasTracker.new(joins) - @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1 - build(associations) - end - - def graft(*associations) - associations.each do |association| - join_associations.detect {|a| association == a} || - build(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type) - end - self - end - - def join_associations - join_parts.last(join_parts.length - 1) - end - - def join_base - join_parts.first - end - - def instantiate(rows) - primary_key = join_base.aliased_primary_key - parents = {} - - records = rows.map { |model| - primary_id = model[primary_key] - parent = parents[primary_id] ||= join_base.instantiate(model) - construct(parent, @associations, join_associations.dup, model) - parent - }.uniq - - remove_duplicate_results!(join_base.active_record, records, @associations) - records - end - - def remove_duplicate_results!(base, records, associations) - case associations - when Symbol, String - reflection = base.reflections[associations] - remove_uniq_by_reflection(reflection, records) - when Array - associations.each do |association| - remove_duplicate_results!(base, records, association) - end - when Hash - associations.keys.each do |name| - reflection = base.reflections[name] - remove_uniq_by_reflection(reflection, records) - - parent_records = [] - records.each do |record| - if descendant = record.send(reflection.name) - if reflection.collection? - parent_records.concat descendant.target.uniq - else - parent_records << descendant - end - end - end - - remove_duplicate_results!(reflection.klass, parent_records, associations[name]) unless parent_records.empty? - end - end - end - - protected - - def cache_joined_association(association) - associations = [] - parent = association.parent - while parent != join_base - associations.unshift(parent.reflection.name) - parent = parent.parent - end - ref = @associations - associations.each do |key| - ref = ref[key] - end - ref[association.reflection.name] ||= {} - end - - def build(associations, parent = nil, join_type = Arel::InnerJoin) - parent ||= join_parts.last - case associations - when Symbol, String - reflection = parent.reflections[associations.to_s.intern] or - raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" - unless join_association = find_join_association(reflection, parent) - @reflections << reflection - join_association = build_join_association(reflection, parent) - join_association.join_type = join_type - @join_parts << join_association - cache_joined_association(join_association) - end - join_association - when Array - associations.each do |association| - build(association, parent, join_type) - end - when Hash - associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| - join_association = build(name, parent, join_type) - build(associations[name], join_association, join_type) - end - else - raise ConfigurationError, associations.inspect - end - end - - def find_join_association(name_or_reflection, parent) - if String === name_or_reflection - name_or_reflection = name_or_reflection.to_sym - end - - join_associations.detect { |j| - j.reflection == name_or_reflection && j.parent == parent - } - end - - def remove_uniq_by_reflection(reflection, records) - if reflection && reflection.collection? - records.each { |record| record.send(reflection.name).target.uniq! } - end - end - - def build_join_association(reflection, parent) - JoinAssociation.new(reflection, self, parent) - end - - def construct(parent, associations, join_parts, row) - case associations - when Symbol, String - name = associations.to_s - - join_part = join_parts.detect { |j| - j.reflection.name.to_s == name && - j.parent_table_name == parent.class.table_name } - - raise(ConfigurationError, "No such association") unless join_part - - join_parts.delete(join_part) - construct_association(parent, join_part, row) - when Array - associations.each do |association| - construct(parent, association, join_parts, row) - end - when Hash - associations.sort_by { |k,_| k.to_s }.each do |name, assoc| - association = construct(parent, name, join_parts, row) - construct(association, assoc, join_parts, row) if association - end - else - raise ConfigurationError, associations.inspect - end - end - - def construct_association(record, join_part, row) - return if record.id.to_s != join_part.parent.record_id(row).to_s - - macro = join_part.reflection.macro - if macro == :has_one - return if record.instance_variable_defined?("@#{join_part.reflection.name}") - association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? - set_target_and_inverse(join_part, association, record) - else - return if row[join_part.aliased_primary_key].nil? - association = join_part.instantiate(row) - case macro - when :has_many, :has_and_belongs_to_many - collection = record.send(join_part.reflection.name) - collection.loaded - collection.target.push(association) - collection.__send__(:set_inverse_instance, association, record) - when :belongs_to - set_target_and_inverse(join_part, association, record) - else - raise ConfigurationError, "unknown macro: #{join_part.reflection.macro}" - end - end - association - end - - def set_target_and_inverse(join_part, association, record) - association_proxy = record.send("set_#{join_part.reflection.name}_target", association) - association_proxy.__send__(:set_inverse_instance, association, record) - end - - # A JoinPart represents a part of a JoinDependency. It is an abstract class, inherited - # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which - # everything else is being joined onto. A JoinAssociation represents an association which - # is joining to the base. A JoinAssociation may result in more than one actual join - # operations (for example a has_and_belongs_to_many JoinAssociation would result in - # two; one for the join table and one for the target table). - class JoinPart # :nodoc: - # The Active Record class which this join part is associated 'about'; for a JoinBase - # this is the actual base model, for a JoinAssociation this is the target model of the - # association. - attr_reader :active_record - - delegate :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :arel_engine, :to => :active_record - - def initialize(active_record) - @active_record = active_record - @cached_record = {} - end - - def ==(other) - raise NotImplementedError - end - - # An Arel::Table for the active_record - def table - raise NotImplementedError - end - - # The prefix to be used when aliasing columns in the active_record's table - def aliased_prefix - raise NotImplementedError - end - - # The alias for the active_record's table - def aliased_table_name - raise NotImplementedError - end - - # The alias for the primary key of the active_record's table - def aliased_primary_key - "#{aliased_prefix}_r0" - end - - # An array of [column_name, alias] pairs for the table - def column_names_with_alias - unless defined?(@column_names_with_alias) - @column_names_with_alias = [] - - ([primary_key] + (column_names - [primary_key])).each_with_index do |column_name, i| - @column_names_with_alias << [column_name, "#{aliased_prefix}_r#{i}"] - end - end - - @column_names_with_alias - end - - def extract_record(row) - Hash[column_names_with_alias.map{|cn, an| [cn, row[an]]}] - end - - def record_id(row) - row[aliased_primary_key] - end - - def instantiate(row) - @cached_record[record_id(row)] ||= active_record.send(:instantiate, extract_record(row)) - end - end - - class JoinBase < JoinPart # :nodoc: - # Extra joins provided when the JoinDependency was created - attr_reader :table_joins - - def initialize(active_record, joins = nil) - super(active_record) - @table_joins = joins - end - - def ==(other) - other.class == self.class && - other.active_record == active_record - end - - def aliased_prefix - "t0" - end - - def table - Arel::Table.new(table_name, :engine => arel_engine, :columns => active_record.columns) - end - - def aliased_table_name - active_record.table_name - end - end - - class JoinAssociation < JoinPart # :nodoc: - # The reflection of the association represented - attr_reader :reflection - - # The JoinDependency object which this JoinAssociation exists within. This is mainly - # relevant for generating aliases which do not conflict with other joins which are - # part of the query. - attr_reader :join_dependency - - # A JoinBase instance representing the active record we are joining onto. - # (So in Author.has_many :posts, the Author would be that base record.) - attr_reader :parent - - # What type of join will be generated, either Arel::InnerJoin (default) or Arel::OuterJoin - attr_accessor :join_type - - attr_reader :aliased_prefix - - delegate :options, :through_reflection, :source_reflection, :through_reflection_chain, :to => :reflection - delegate :table, :table_name, :to => :parent, :prefix => true - delegate :alias_tracker, :to => :join_dependency - - def initialize(reflection, join_dependency, parent = nil) - reflection.check_validity! - - if reflection.options[:polymorphic] - raise EagerLoadPolymorphicError.new(reflection) - end - - super(reflection.klass) - - @reflection = reflection - @join_dependency = join_dependency - @parent = parent - @join_type = Arel::InnerJoin - @aliased_prefix = "t#{ join_dependency.join_parts.size }" - - setup_tables - end - - def ==(other) - other.class == self.class && - other.reflection == reflection && - other.parent == parent - end - - def find_parent_in(other_join_dependency) - other_join_dependency.join_parts.detect do |join_part| - self.parent == join_part - end - end - - def join_to(relation) - # The chain starts with the target table, but we want to end with it here (makes - # more sense in this context) - chain = through_reflection_chain.reverse - - foreign_table = parent_table - index = 0 - - chain.each do |reflection| - table = @tables[index] - conditions = [] - - if reflection.source_reflection.nil? - case reflection.macro - when :belongs_to - key = reflection.association_primary_key - foreign_key = reflection.primary_key_name - when :has_many, :has_one - key = reflection.primary_key_name - foreign_key = reflection.active_record_primary_key - - conditions << polymorphic_conditions(reflection, table) - when :has_and_belongs_to_many - # For habtm, we need to deal with the join table at the same time as the - # target table (because unlike a :through association, there is no reflection - # to represent the join table) - table, join_table = table - - join_key = reflection.primary_key_name - join_foreign_key = reflection.active_record.primary_key - - relation = relation.join(join_table, join_type).on( - join_table[join_key]. - eq(foreign_table[join_foreign_key]) - ) - - # We've done the first join now, so update the foreign_table for the second - foreign_table = join_table - - key = reflection.klass.primary_key - foreign_key = reflection.association_foreign_key - end - else - case reflection.source_reflection.macro - when :belongs_to - key = reflection.association_primary_key - foreign_key = reflection.primary_key_name - - conditions << source_type_conditions(reflection, foreign_table) - when :has_many, :has_one - key = reflection.primary_key_name - foreign_key = reflection.source_reflection.active_record_primary_key - when :has_and_belongs_to_many - table, join_table = table - - join_key = reflection.primary_key_name - join_foreign_key = reflection.klass.primary_key - - relation = relation.join(join_table, join_type).on( - join_table[join_key]. - eq(foreign_table[join_foreign_key]) - ) - - foreign_table = join_table - - key = reflection.klass.primary_key - foreign_key = reflection.association_foreign_key - end - end - - conditions << table[key].eq(foreign_table[foreign_key]) - - conditions << reflection_conditions(index, table) - conditions << sti_conditions(reflection, table) - - relation = relation.join(table, join_type).on(*conditions.flatten.compact) - - # The current table in this iteration becomes the foreign table in the next - foreign_table = table - index += 1 - end - - relation - end - - def join_relation(joining_relation) - self.join_type = Arel::OuterJoin - joining_relation.joins(self) - end - - def table - if @tables.last.is_a?(Array) - @tables.last.first - else - @tables.last - end - end - - def aliased_table_name - table.table_alias || table.name - end - - protected - - def table_alias_for(reflection, join = false) - name = alias_tracker.pluralize(reflection.name) - name << "_#{parent_table_name}" - name << "_join" if join - name - end - - def interpolate_sql(sql) - instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__) - end - - private - - # Generate aliases and Arel::Table instances for each of the tables which we will - # later generate joins for. We must do this in advance in order to correctly allocate - # the proper alias. - def setup_tables - @tables = through_reflection_chain.map do |reflection| - aliased_table_name = alias_tracker.aliased_name_for( - reflection.table_name, - table_alias_for(reflection, reflection != self.reflection) - ) - - table = Arel::Table.new( - reflection.table_name, :engine => arel_engine, - :as => aliased_table_name, :columns => reflection.klass.columns - ) - - # For habtm, we have two Arel::Table instances related to a single reflection, so - # we just store them as a pair in the array. - if reflection.macro == :has_and_belongs_to_many || - (reflection.source_reflection && - reflection.source_reflection.macro == :has_and_belongs_to_many) - - join_table_name = (reflection.source_reflection || reflection).options[:join_table] - - aliased_join_table_name = alias_tracker.aliased_name_for( - join_table_name, - table_alias_for(reflection, true) - ) - - join_table = Arel::Table.new( - join_table_name, :engine => arel_engine, - :as => aliased_join_table_name - ) - - [table, join_table] - else - table - end - end - - # The joins are generated from the through_reflection_chain in reverse order, so - # reverse the tables too (but it's important to generate the aliases in the 'forward' - # order, which is why we only do the reversal now. - @tables.reverse! - - @tables - end - - def reflection_conditions(index, table) - @reflection.through_conditions.reverse[index].map do |condition| - Arel.sql(interpolate_sql(sanitize_sql( - condition, - table.table_alias || table.name - ))) - end - end - - def sti_conditions(reflection, table) - unless reflection.klass.descends_from_active_record? - sti_column = table[reflection.klass.inheritance_column] - - condition = sti_column.eq(reflection.klass.sti_name) - - reflection.klass.descendants.each do |subclass| - condition = condition.or(sti_column.eq(subclass.sti_name)) - end - - condition - end - end - - def source_type_conditions(reflection, foreign_table) - if reflection.options[:source_type] - foreign_table[reflection.source_reflection.options[:foreign_type]]. - eq(reflection.options[:source_type]) - end - end - - def polymorphic_conditions(reflection, table) - if reflection.options[:as] - table["#{reflection.options[:as]}_type"]. - eq(reflection.active_record.base_class.name) - end - end - end - end end end end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 6090376bb8..ba9373ba6a 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -235,12 +235,12 @@ module ActiveRecord # Removes all records from this association. Returns +self+ so method calls may be chained. def clear - return self if length.zero? # forces load_target if it hasn't happened already - - if @reflection.options[:dependent] && @reflection.options[:dependent] == :destroy - destroy_all - else - delete_all + unless length.zero? # forces load_target if it hasn't happened already + if @reflection.options[:dependent] == :destroy + destroy_all + else + delete_all + end end self @@ -357,8 +357,7 @@ module ActiveRecord return false unless record.is_a?(@reflection.klass) return include_in_memory?(record) unless record.persisted? load_target if @reflection.options[:finder_sql] && !loaded? - return @target.include?(record) if loaded? - exists?(record) + loaded? ? @target.include?(record) : exists?(record) end def proxy_respond_to?(method, include_private = false) @@ -530,7 +529,7 @@ module ActiveRecord def callbacks_for(callback_name) full_callback_name = "#{callback_name}_for_#{@reflection.name}" - @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || [] + @owner.class.send(full_callback_name.to_sym) || [] end def ensure_owner_is_not_new diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index b624951cd9..b438620c8f 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -61,7 +61,7 @@ module ActiveRecord set_inverse_instance(the_target, @owner) the_target end - + def construct_find_scope { :conditions => conditions } end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb new file mode 100644 index 0000000000..ce20420aad --- /dev/null +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb @@ -0,0 +1,214 @@ +require 'active_record/associations/class_methods/join_dependency/join_part' +require 'active_record/associations/class_methods/join_dependency/join_base' +require 'active_record/associations/class_methods/join_dependency/join_association' + +module ActiveRecord + module Associations + module ClassMethods + class JoinDependency # :nodoc: + attr_reader :join_parts, :reflections, :alias_tracker + + def initialize(base, associations, joins) + @join_parts = [JoinBase.new(base, joins)] + @associations = {} + @reflections = [] + @alias_tracker = AliasTracker.new(joins) + @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1 + build(associations) + end + + def graft(*associations) + associations.each do |association| + join_associations.detect {|a| association == a} || + build(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type) + end + self + end + + def join_associations + join_parts.last(join_parts.length - 1) + end + + def join_base + join_parts.first + end + + def columns + join_parts.collect { |join_part| + table = join_part.aliased_table + join_part.column_names_with_alias.collect{ |column_name, aliased_name| + table[column_name].as Arel.sql(aliased_name) + } + }.flatten + end + + def instantiate(rows) + primary_key = join_base.aliased_primary_key + parents = {} + + records = rows.map { |model| + primary_id = model[primary_key] + parent = parents[primary_id] ||= join_base.instantiate(model) + construct(parent, @associations, join_associations.dup, model) + parent + }.uniq + + remove_duplicate_results!(join_base.active_record, records, @associations) + records + end + + def remove_duplicate_results!(base, records, associations) + case associations + when Symbol, String + reflection = base.reflections[associations] + remove_uniq_by_reflection(reflection, records) + when Array + associations.each do |association| + remove_duplicate_results!(base, records, association) + end + when Hash + associations.keys.each do |name| + reflection = base.reflections[name] + remove_uniq_by_reflection(reflection, records) + + parent_records = [] + records.each do |record| + if descendant = record.send(reflection.name) + if reflection.collection? + parent_records.concat descendant.target.uniq + else + parent_records << descendant + end + end + end + + remove_duplicate_results!(reflection.klass, parent_records, associations[name]) unless parent_records.empty? + end + end + end + + protected + + def cache_joined_association(association) + associations = [] + parent = association.parent + while parent != join_base + associations.unshift(parent.reflection.name) + parent = parent.parent + end + ref = @associations + associations.each do |key| + ref = ref[key] + end + ref[association.reflection.name] ||= {} + end + + def build(associations, parent = nil, join_type = Arel::InnerJoin) + parent ||= join_parts.last + case associations + when Symbol, String + reflection = parent.reflections[associations.to_s.intern] or + raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" + unless join_association = find_join_association(reflection, parent) + @reflections << reflection + join_association = build_join_association(reflection, parent) + join_association.join_type = join_type + @join_parts << join_association + cache_joined_association(join_association) + end + join_association + when Array + associations.each do |association| + build(association, parent, join_type) + end + when Hash + associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| + join_association = build(name, parent, join_type) + build(associations[name], join_association, join_type) + end + else + raise ConfigurationError, associations.inspect + end + end + + def find_join_association(name_or_reflection, parent) + if String === name_or_reflection + name_or_reflection = name_or_reflection.to_sym + end + + join_associations.detect { |j| + j.reflection == name_or_reflection && j.parent == parent + } + end + + def remove_uniq_by_reflection(reflection, records) + if reflection && reflection.collection? + records.each { |record| record.send(reflection.name).target.uniq! } + end + end + + def build_join_association(reflection, parent) + JoinAssociation.new(reflection, self, parent) + end + + def construct(parent, associations, join_parts, row) + case associations + when Symbol, String + name = associations.to_s + + join_part = join_parts.detect { |j| + j.reflection.name.to_s == name && + j.parent_table_name == parent.class.table_name } + + raise(ConfigurationError, "No such association") unless join_part + + join_parts.delete(join_part) + construct_association(parent, join_part, row) + when Array + associations.each do |association| + construct(parent, association, join_parts, row) + end + when Hash + associations.sort_by { |k,_| k.to_s }.each do |name, assoc| + association = construct(parent, name, join_parts, row) + construct(association, assoc, join_parts, row) if association + end + else + raise ConfigurationError, associations.inspect + end + end + + def construct_association(record, join_part, row) + return if record.id.to_s != join_part.parent.record_id(row).to_s + + macro = join_part.reflection.macro + if macro == :has_one + return if record.instance_variable_defined?("@#{join_part.reflection.name}") + association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? + set_target_and_inverse(join_part, association, record) + else + return if row[join_part.aliased_primary_key].nil? + association = join_part.instantiate(row) + case macro + when :has_many, :has_and_belongs_to_many + collection = record.send(join_part.reflection.name) + collection.loaded + collection.target.push(association) + collection.__send__(:set_inverse_instance, association, record) + when :belongs_to + set_target_and_inverse(join_part, association, record) + else + raise ConfigurationError, "unknown macro: #{join_part.reflection.macro}" + end + end + association + end + + def set_target_and_inverse(join_part, association, record) + association_proxy = record.send("set_#{join_part.reflection.name}_target", association) + association_proxy.__send__(:set_inverse_instance, association, record) + end + end + end + end +end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb new file mode 100644 index 0000000000..ed2053d3df --- /dev/null +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb @@ -0,0 +1,261 @@ +module ActiveRecord + module Associations + module ClassMethods + class JoinDependency # :nodoc: + class JoinAssociation < JoinPart # :nodoc: + # The reflection of the association represented + attr_reader :reflection + + # The JoinDependency object which this JoinAssociation exists within. This is mainly + # relevant for generating aliases which do not conflict with other joins which are + # part of the query. + attr_reader :join_dependency + + # A JoinBase instance representing the active record we are joining onto. + # (So in Author.has_many :posts, the Author would be that base record.) + attr_reader :parent + + # What type of join will be generated, either Arel::InnerJoin (default) or Arel::OuterJoin + attr_accessor :join_type + + attr_reader :aliased_prefix + + delegate :options, :through_reflection, :source_reflection, :through_reflection_chain, :to => :reflection + delegate :table, :table_name, :to => :parent, :prefix => true + delegate :alias_tracker, :to => :join_dependency + + def initialize(reflection, join_dependency, parent = nil) + reflection.check_validity! + + if reflection.options[:polymorphic] + raise EagerLoadPolymorphicError.new(reflection) + end + + super(reflection.klass) + + @reflection = reflection + @join_dependency = join_dependency + @parent = parent + @join_type = Arel::InnerJoin + @aliased_prefix = "t#{ join_dependency.join_parts.size }" + + setup_tables + end + + def ==(other) + other.class == self.class && + other.reflection == reflection && + other.parent == parent + end + + def find_parent_in(other_join_dependency) + other_join_dependency.join_parts.detect do |join_part| + self.parent == join_part + end + end + + def join_to(relation) + # The chain starts with the target table, but we want to end with it here (makes + # more sense in this context) + chain = through_reflection_chain.reverse + + foreign_table = parent_table + index = 0 + + chain.each do |reflection| + table = @tables[index] + conditions = [] + + if reflection.source_reflection.nil? + case reflection.macro + when :belongs_to + key = reflection.association_primary_key + foreign_key = reflection.primary_key_name + when :has_many, :has_one + key = reflection.primary_key_name + foreign_key = reflection.active_record_primary_key + + conditions << polymorphic_conditions(reflection, table) + when :has_and_belongs_to_many + # For habtm, we need to deal with the join table at the same time as the + # target table (because unlike a :through association, there is no reflection + # to represent the join table) + table, join_table = table + + join_key = reflection.primary_key_name + join_foreign_key = reflection.active_record.primary_key + + relation = relation.join(join_table, join_type).on( + join_table[join_key]. + eq(foreign_table[join_foreign_key]) + ) + + # We've done the first join now, so update the foreign_table for the second + foreign_table = join_table + + key = reflection.klass.primary_key + foreign_key = reflection.association_foreign_key + end + else + case reflection.source_reflection.macro + when :belongs_to + key = reflection.association_primary_key + foreign_key = reflection.primary_key_name + + conditions << source_type_conditions(reflection, foreign_table) + when :has_many, :has_one + key = reflection.primary_key_name + foreign_key = reflection.source_reflection.active_record_primary_key + when :has_and_belongs_to_many + table, join_table = table + + join_key = reflection.primary_key_name + join_foreign_key = reflection.klass.primary_key + + relation = relation.join(join_table, join_type).on( + join_table[join_key]. + eq(foreign_table[join_foreign_key]) + ) + + foreign_table = join_table + + key = reflection.klass.primary_key + foreign_key = reflection.association_foreign_key + end + end + + conditions << table[key].eq(foreign_table[foreign_key]) + + conditions << reflection_conditions(index, table) + conditions << sti_conditions(reflection, table) + + relation = relation.join(table, join_type).on(*conditions.flatten.compact) + + # The current table in this iteration becomes the foreign table in the next + foreign_table = table + index += 1 + end + + relation + end + + def join_relation(joining_relation) + self.join_type = Arel::OuterJoin + joining_relation.joins(self) + end + + def table + if @tables.last.is_a?(Array) + @tables.last.first + else + @tables.last + end + end + + def aliased_table_name + table.table_alias || table.name + end + + protected + + def table_alias_for(reflection, join = false) + name = alias_tracker.pluralize(reflection.name) + name << "_#{parent_table_name}" + name << "_join" if join + name + end + + def interpolate_sql(sql) + instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__) + end + + private + + # Generate aliases and Arel::Table instances for each of the tables which we will + # later generate joins for. We must do this in advance in order to correctly allocate + # the proper alias. + def setup_tables + @tables = through_reflection_chain.map do |reflection| + aliased_table_name = alias_tracker.aliased_name_for( + reflection.table_name, + table_alias_for(reflection, reflection != self.reflection) + ) + + table = Arel::Table.new( + reflection.table_name, :engine => arel_engine, + :as => aliased_table_name, :columns => reflection.klass.columns + ) + + # For habtm, we have two Arel::Table instances related to a single reflection, so + # we just store them as a pair in the array. + if reflection.macro == :has_and_belongs_to_many || + (reflection.source_reflection && + reflection.source_reflection.macro == :has_and_belongs_to_many) + + join_table_name = (reflection.source_reflection || reflection).options[:join_table] + + aliased_join_table_name = alias_tracker.aliased_name_for( + join_table_name, + table_alias_for(reflection, true) + ) + + join_table = Arel::Table.new( + join_table_name, :engine => arel_engine, + :as => aliased_join_table_name + ) + + [table, join_table] + else + table + end + end + + # The joins are generated from the through_reflection_chain in reverse order, so + # reverse the tables too (but it's important to generate the aliases in the 'forward' + # order, which is why we only do the reversal now. + @tables.reverse! + + @tables + end + + def reflection_conditions(index, table) + @reflection.through_conditions.reverse[index].map do |condition| + Arel.sql(interpolate_sql(sanitize_sql( + condition, + table.table_alias || table.name + ))) + end + end + + def sti_conditions(reflection, table) + unless reflection.klass.descends_from_active_record? + sti_column = table[reflection.klass.inheritance_column] + + condition = sti_column.eq(reflection.klass.sti_name) + + reflection.klass.descendants.each do |subclass| + condition = condition.or(sti_column.eq(subclass.sti_name)) + end + + condition + end + end + + def source_type_conditions(reflection, foreign_table) + if reflection.options[:source_type] + foreign_table[reflection.source_reflection.options[:foreign_type]]. + eq(reflection.options[:source_type]) + end + end + + def polymorphic_conditions(reflection, table) + if reflection.options[:as] + table["#{reflection.options[:as]}_type"]. + eq(reflection.active_record.base_class.name) + end + end + end + end + end + end +end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb new file mode 100644 index 0000000000..ed05003f66 --- /dev/null +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb @@ -0,0 +1,34 @@ +module ActiveRecord + module Associations + module ClassMethods + class JoinDependency # :nodoc: + class JoinBase < JoinPart # :nodoc: + # Extra joins provided when the JoinDependency was created + attr_reader :table_joins + + def initialize(active_record, joins = nil) + super(active_record) + @table_joins = joins + end + + def ==(other) + other.class == self.class && + other.active_record == active_record + end + + def aliased_prefix + "t0" + end + + def table + Arel::Table.new(table_name, :engine => arel_engine, :columns => active_record.columns) + end + + def aliased_table_name + active_record.table_name + end + end + end + end + end +end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb new file mode 100644 index 0000000000..0b093b65e9 --- /dev/null +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb @@ -0,0 +1,80 @@ +module ActiveRecord + module Associations + module ClassMethods + class JoinDependency # :nodoc: + # A JoinPart represents a part of a JoinDependency. It is an abstract class, inherited + # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which + # everything else is being joined onto. A JoinAssociation represents an association which + # is joining to the base. A JoinAssociation may result in more than one actual join + # operations (for example a has_and_belongs_to_many JoinAssociation would result in + # two; one for the join table and one for the target table). + class JoinPart # :nodoc: + # The Active Record class which this join part is associated 'about'; for a JoinBase + # this is the actual base model, for a JoinAssociation this is the target model of the + # association. + attr_reader :active_record + + delegate :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :arel_engine, :to => :active_record + + def initialize(active_record) + @active_record = active_record + @cached_record = {} + @column_names_with_alias = nil + end + + def aliased_table + Arel::Nodes::TableAlias.new aliased_table_name, table + end + + def ==(other) + raise NotImplementedError + end + + # An Arel::Table for the active_record + def table + raise NotImplementedError + end + + # The prefix to be used when aliasing columns in the active_record's table + def aliased_prefix + raise NotImplementedError + end + + # The alias for the active_record's table + def aliased_table_name + raise NotImplementedError + end + + # The alias for the primary key of the active_record's table + def aliased_primary_key + "#{aliased_prefix}_r0" + end + + # An array of [column_name, alias] pairs for the table + def column_names_with_alias + unless @column_names_with_alias + @column_names_with_alias = [] + + ([primary_key] + (column_names - [primary_key])).each_with_index do |column_name, i| + @column_names_with_alias << [column_name, "#{aliased_prefix}_r#{i}"] + end + end + @column_names_with_alias + end + + def extract_record(row) + Hash[column_names_with_alias.map{|cn, an| [cn, row[an]]}] + end + + def record_id(row) + row[aliased_primary_key] + end + + def instantiate(row) + @cached_record[record_id(row)] ||= active_record.send(:instantiate, extract_record(row)) + end + end + end + end + end +end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index da742fa668..2c72fd0004 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -67,7 +67,7 @@ module ActiveRecord relation.insert(attributes) end - return true + true end def delete_records(records) @@ -80,7 +80,7 @@ module ActiveRecord ).delete end end - + def construct_joins "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}" end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 7eaa05ee36..bbf62796bb 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -42,11 +42,7 @@ module ActiveRecord # documented side-effect of the method that may avoid an extra SELECT. @target ||= [] and loaded if count == 0 - if @reflection.options[:limit] - count = [ @reflection.options[:limit], count ].min - end - - return count + [@reflection.options[:limit], count].compact.min end def has_cached_counter? @@ -112,8 +108,7 @@ module ActiveRecord end def we_can_set_the_inverse_on_this?(record) - inverse = @reflection.inverse_of - return !inverse.nil? + @reflection.inverse_of end end end diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb index 07ce6f1597..f6d02a215f 100644 --- a/activerecord/lib/active_record/associations/through_association_scope.rb +++ b/activerecord/lib/active_record/associations/through_association_scope.rb @@ -57,7 +57,7 @@ module ActiveRecord def construct_select(custom_select = nil) distinct = "DISTINCT " if @reflection.options[:uniq] - selected = custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*" + custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*" end def construct_joins(custom_joins = nil) diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 439880c1fa..c19a33faa8 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,3 +1,4 @@ +require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/object/blank' module ActiveRecord @@ -88,7 +89,7 @@ module ActiveRecord end def clone_with_time_zone_conversion_attribute?(attr, old) - old.class.name == "Time" && time_zone_aware_attributes && !skip_time_zone_conversion_for_attributes.include?(attr.to_sym) + old.class.name == "Time" && time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(attr.to_sym) end end end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index d640b26b74..dc2785b6bf 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/class/attribute' + module ActiveRecord module AttributeMethods module TimeZoneConversion @@ -7,7 +9,7 @@ module ActiveRecord cattr_accessor :time_zone_aware_attributes, :instance_writer => false self.time_zone_aware_attributes = false - class_inheritable_accessor :skip_time_zone_conversion_for_attributes, :instance_writer => false + class_attribute :skip_time_zone_conversion_for_attributes, :instance_writer => false self.skip_time_zone_conversion_for_attributes = [] end @@ -54,7 +56,7 @@ module ActiveRecord private def create_time_zone_conversion_attribute?(name, column) - time_zone_aware_attributes && !skip_time_zone_conversion_for_attributes.include?(name.to_sym) && [:datetime, :timestamp].include?(column.type) + time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && [:datetime, :timestamp].include?(column.type) end end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index cb5bc06580..73ac8e82c6 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -89,7 +89,7 @@ module ActiveRecord # post = Post.create(:title => 'ruby rocks') # post.comments.create(:body => 'hello world') # post.comments[0].body = 'hi everyone' - # post.save # => saves both post and comment, with 'hi everyone' as title + # post.save # => saves both post and comment, with 'hi everyone' as body # # Destroying one of the associated models as part of the parent's save action # is as simple as marking it for destruction: diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b35f59d6df..9b09b14c87 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -7,7 +7,7 @@ require 'active_support/time' require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/class/delegating_attributes' -require 'active_support/core_ext/class/inheritable_attributes' +require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/hash/deep_merge' require 'active_support/core_ext/hash/indifferent_access' @@ -412,7 +412,7 @@ module ActiveRecord #:nodoc: self.store_full_sti_class = true # Stores the default scope for the class - class_inheritable_accessor :default_scoping, :instance_writer => false + class_attribute :default_scoping, :instance_writer => false self.default_scoping = [] # Returns a hash of all the attributes that have been specified for serialization as @@ -420,6 +420,9 @@ module ActiveRecord #:nodoc: class_attribute :serialized_attributes self.serialized_attributes = {} + class_attribute :_attr_readonly, :instance_writer => false + self._attr_readonly = [] + class << self # Class methods delegate :find, :first, :last, :all, :destroy, :destroy_all, :exists?, :delete, :delete_all, :update, :update_all, :to => :scoped delegate :find_each, :find_in_batches, :to => :scoped @@ -504,12 +507,12 @@ module ActiveRecord #:nodoc: # Attributes listed as readonly will be used to create a new record but update operations will # ignore these fields. def attr_readonly(*attributes) - write_inheritable_attribute(:attr_readonly, Set.new(attributes.map { |a| a.to_s }) + (readonly_attributes || [])) + self._attr_readonly = Set.new(attributes.map { |a| a.to_s }) + (self._attr_readonly || []) end # Returns an array of all the attributes that have been specified as readonly. def readonly_attributes - read_inheritable_attribute(:attr_readonly) || [] + self._attr_readonly end # If you have an attribute that needs to be saved to the database as an object, and retrieved as the same object, @@ -724,10 +727,6 @@ module ActiveRecord #:nodoc: @arel_engine = @relation = @arel_table = nil end - def reset_column_information_and_inheritable_attributes_for_all_subclasses#:nodoc: - descendants.each { |klass| klass.reset_inheritable_attributes; klass.reset_column_information } - end - def attribute_method?(attribute) super || (table_exists? && column_names.include?(attribute.to_s.sub(/=$/, ''))) end @@ -736,15 +735,12 @@ module ActiveRecord #:nodoc: def lookup_ancestors #:nodoc: klass = self classes = [klass] + return classes if klass == ActiveRecord::Base + while klass != klass.base_class classes << klass = klass.superclass end classes - rescue - # OPTIMIZE this rescue is to fix this test: ./test/cases/reflection_test.rb:56:in `test_human_name_for_column' - # Apparently the method base_class causes some trouble. - # It now works for sure. - [self] end # Set the i18n scope to overwrite ActiveModel. @@ -1129,7 +1125,8 @@ MSG # Article.create.published # => true def default_scope(options = {}) reset_scoped_methods - self.default_scoping << construct_finder_arel(options, default_scoping.pop) + default_scoping = self.default_scoping.dup + self.default_scoping = default_scoping << construct_finder_arel(options, default_scoping.pop) end def current_scoped_methods #:nodoc: @@ -1286,7 +1283,7 @@ MSG # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" def sanitize_sql_array(ary) statement, *values = ary - if values.first.is_a?(Hash) and statement =~ /:\w+/ + if values.first.is_a?(Hash) && statement =~ /:\w+/ replace_named_bind_variables(statement, values.first) elsif statement.include?('?') replace_bind_variables(statement, values) @@ -1385,30 +1382,6 @@ MSG result end - # Cloned objects have no id assigned and are treated as new records. Note that this is a "shallow" clone - # as it copies the object's attributes only, not its associations. The extent of a "deep" clone is - # application specific and is therefore left to the application to implement according to its need. - def initialize_copy(other) - _run_after_initialize_callbacks if respond_to?(:_run_after_initialize_callbacks) - cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast) - cloned_attributes.delete(self.class.primary_key) - - @attributes = cloned_attributes - - @changed_attributes = {} - attributes_from_column_definition.each do |attr, orig_value| - @changed_attributes[attr] = orig_value if field_changed?(attr, orig_value, @attributes[attr]) - end - - clear_aggregation_cache - clear_association_cache - @attributes_cache = {} - @persisted = false - ensure_proper_type - - populate_with_current_scope_attributes - end - # Initialize an empty model object from +coder+. +coder+ must contain # the attributes necessary for initializing an empty model object. For # example: @@ -1573,8 +1546,7 @@ MSG # Returns true if the specified +attribute+ has been set by the user or by a database load and is neither # nil nor empty? (the latter only applies to objects that respond to empty?, most notably Strings). def attribute_present?(attribute) - value = read_attribute(attribute) - !value.blank? + !read_attribute(attribute).blank? end # Returns the column object for the named attribute. @@ -1582,7 +1554,7 @@ MSG self.class.columns_hash[name.to_s] end - # Returns true if +comparison_object+ is the same exact object, or +comparison_object+ + # Returns true if +comparison_object+ is the same exact object, or +comparison_object+ # is of the same type and +self+ has an ID and it is equal to +comparison_object.id+. # # Note that new records are different from any other record by definition, unless the @@ -1600,7 +1572,7 @@ MSG # Delegates to == def eql?(comparison_object) - self == (comparison_object) + self == comparison_object end # Delegates to id in order to allow two records of the same type and id to work with something like: @@ -1619,11 +1591,42 @@ MSG @attributes.frozen? end - # Returns duplicated record with unfreezed attributes. - def dup - obj = super - obj.instance_variable_set('@attributes', @attributes.dup) - obj + # Backport dup from 1.9 so that initialize_dup() gets called + unless Object.respond_to?(:initialize_dup) + def dup # :nodoc: + copy = super + copy.initialize_dup(self) + copy + end + end + + # Duped objects have no id assigned and are treated as new records. Note + # that this is a "shallow" copy as it copies the object's attributes + # only, not its associations. The extent of a "deep" copy is application + # specific and is therefore left to the application to implement according + # to its need. + # The dup method does not preserve the timestamps (created|updated)_(at|on). + def initialize_dup(other) + cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast) + cloned_attributes.delete(self.class.primary_key) + + @attributes = cloned_attributes + + _run_after_initialize_callbacks if respond_to?(:_run_after_initialize_callbacks) + + @changed_attributes = {} + attributes_from_column_definition.each do |attr, orig_value| + @changed_attributes[attr] = orig_value if field_changed?(attr, orig_value, @attributes[attr]) + end + + clear_aggregation_cache + clear_association_cache + @attributes_cache = {} + @persisted = false + + ensure_proper_type + populate_with_current_scope_attributes + clear_timestamp_attributes end # Returns +true+ if the record is read only. Records loaded through joins with piggy-back @@ -1830,6 +1833,16 @@ MSG create_with.each { |att,value| self.respond_to?(:"#{att}=") && self.send("#{att}=", value) } if create_with end end + + # Clear attributes and changed_attributes + def clear_timestamp_attributes + %w(created_at created_on updated_at updated_on).each do |attribute_name| + if has_attribute?(attribute_name) + self[attribute_name] = nil + changed_attributes.delete(attribute_name) + end + end + end end Base.class_eval do diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index ca9314ec99..cffa2387de 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1,3 +1,4 @@ +require 'thread' require 'monitor' require 'set' require 'active_support/core_ext/module/synchronization' diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index f3fba9a3a9..0282493219 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -204,8 +204,7 @@ module ActiveRecord protected - def log(sql, name) - name ||= "SQL" + def log(sql, name = "SQL") @instrumenter.instrument("sql.active_record", :sql => sql, :name => name, :connection_id => object_id) do yield diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 5ca1923d89..c2cd9e8d5e 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -40,8 +40,7 @@ module ActiveRecord if @connection.respond_to?(:encoding) @connection.encoding.to_s else - encoding = @connection.execute('PRAGMA encoding') - encoding[0]['encoding'] + @connection.execute('PRAGMA encoding')[0]['encoding'] end end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index bf626301f1..c0e1dda2bd 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -70,7 +70,7 @@ module ActiveRecord result[self.class.locking_column] ||= 0 end - return result + result end def update(attribute_names = @attributes.keys) #:nodoc: @@ -89,7 +89,7 @@ module ActiveRecord affected_rows = relation.where( relation.table[self.class.primary_key].eq(quoted_id).and( - relation.table[self.class.locking_column].eq(quote_value(previous_value)) + relation.table[lock_col].eq(quote_value(previous_value)) ) ).arel.update(arel_attributes_values(false, false, attribute_names)) @@ -110,12 +110,10 @@ module ActiveRecord return super unless locking_enabled? if persisted? - lock_col = self.class.locking_column - previous_value = send(lock_col).to_i - table = self.class.arel_table - predicate = table[self.class.primary_key].eq(id) - predicate = predicate.and(table[self.class.locking_column].eq(previous_value)) + lock_col = self.class.locking_column + predicate = table[self.class.primary_key].eq(id). + and(table[lock_col].eq(send(lock_col).to_i)) affected_rows = self.class.unscoped.where(predicate).delete_all diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 2c306d233a..f6321f1499 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1,6 +1,3 @@ -require 'active_support/core_ext/kernel/singleton_class' -require 'active_support/core_ext/module/aliasing' - module ActiveRecord # Exception that can be raised to stop migrations from going backwards. class IrreversibleMigration < ActiveRecordError @@ -43,11 +40,11 @@ module ActiveRecord # Example of a simple migration: # # class AddSsl < ActiveRecord::Migration - # def self.up + # def up # add_column :accounts, :ssl_enabled, :boolean, :default => 1 # end # - # def self.down + # def down # remove_column :accounts, :ssl_enabled # end # end @@ -63,7 +60,7 @@ module ActiveRecord # Example of a more complex migration that also needs to initialize data: # # class AddSystemSettings < ActiveRecord::Migration - # def self.up + # def up # create_table :system_settings do |t| # t.string :name # t.string :label @@ -77,7 +74,7 @@ module ActiveRecord # :value => 1 # end # - # def self.down + # def down # drop_table :system_settings # end # end @@ -138,7 +135,7 @@ module ActiveRecord # in the <tt>db/migrate/</tt> directory where <tt>timestamp</tt> is the # UTC formatted date and time that the migration was generated. # - # You may then edit the <tt>self.up</tt> and <tt>self.down</tt> methods of + # You may then edit the <tt>up</tt> and <tt>down</tt> methods of # MyNewMigration. # # There is a special syntactic shortcut to generate migrations that add fields to a table. @@ -147,11 +144,11 @@ module ActiveRecord # # This will generate the file <tt>timestamp_add_fieldname_to_tablename</tt>, which will look like this: # class AddFieldnameToTablename < ActiveRecord::Migration - # def self.up + # def up # add_column :tablenames, :fieldname, :string # end # - # def self.down + # def down # remove_column :tablenames, :fieldname # end # end @@ -179,11 +176,11 @@ module ActiveRecord # Not all migrations change the schema. Some just fix the data: # # class RemoveEmptyTags < ActiveRecord::Migration - # def self.up + # def up # Tag.find(:all).each { |tag| tag.destroy if tag.pages.empty? } # end # - # def self.down + # def down # # not much we can do to restore deleted data # raise ActiveRecord::IrreversibleMigration, "Can't recover the deleted tags" # end @@ -192,12 +189,12 @@ module ActiveRecord # Others remove columns when they migrate up instead of down: # # class RemoveUnnecessaryItemAttributes < ActiveRecord::Migration - # def self.up + # def up # remove_column :items, :incomplete_items_count # remove_column :items, :completed_items_count # end # - # def self.down + # def down # add_column :items, :incomplete_items_count # add_column :items, :completed_items_count # end @@ -206,11 +203,11 @@ module ActiveRecord # And sometimes you need to do something in SQL not abstracted directly by migrations: # # class MakeJoinUnique < ActiveRecord::Migration - # def self.up + # def up # execute "ALTER TABLE `pages_linked_pages` ADD UNIQUE `page_id_linked_page_id` (`page_id`,`linked_page_id`)" # end # - # def self.down + # def down # execute "ALTER TABLE `pages_linked_pages` DROP INDEX `page_id_linked_page_id`" # end # end @@ -223,7 +220,7 @@ module ActiveRecord # latest column data from after the new column was added. Example: # # class AddPeopleSalary < ActiveRecord::Migration - # def self.up + # def up # add_column :people, :salary, :integer # Person.reset_column_information # Person.find(:all).each do |p| @@ -243,7 +240,7 @@ module ActiveRecord # You can also insert your own messages and benchmarks by using the +say_with_time+ # method: # - # def self.up + # def up # ... # say_with_time "Updating salaries..." do # Person.find(:all).each do |p| @@ -286,144 +283,200 @@ module ActiveRecord # # In application.rb. # + # == Reversible Migrations + # + # Starting with Rails 3.1, you will be able to define reversible migrations. + # Reversible migrations are migrations that know how to go +down+ for you. + # You simply supply the +up+ logic, and the Migration system will figure out + # how to execute the down commands for you. + # + # To define a reversible migration, define the +change+ method in your + # migration like this: + # + # class TenderloveMigration < ActiveRecord::Migration + # def change + # create_table(:horses) do + # t.column :content, :text + # t.column :remind_at, :datetime + # end + # end + # end + # + # This migration will create the horses table for you on the way up, and + # automatically figure out how to drop the table on the way down. + # + # Some commands like +remove_column+ cannot be reversed. If you care to + # define how to move up and down in these cases, you should define the +up+ + # and +down+ methods as before. + # + # If a command cannot be reversed, an + # <tt>ActiveRecord::IrreversibleMigration</tt> exception will be raised when + # the migration is moving down. + # + # For a list of commands that are reversible, please see + # <tt>ActiveRecord::Migration::CommandRecorder</tt>. class Migration - @@verbose = true - cattr_accessor :verbose + autoload :CommandRecorder, 'active_record/migration/command_recorder' class << self - def up_with_benchmarks #:nodoc: - migrate(:up) - end + attr_accessor :delegate # :nodoc: + end - def down_with_benchmarks #:nodoc: - migrate(:down) - end + def self.method_missing(name, *args, &block) # :nodoc: + (delegate || superclass.delegate).send(name, *args, &block) + end - # Execute this migration in the named direction - def migrate(direction) - return unless respond_to?(direction) + cattr_accessor :verbose - case direction - when :up then announce "migrating" - when :down then announce "reverting" - end + attr_accessor :name, :version - result = nil - time = Benchmark.measure { result = send("#{direction}_without_benchmarks") } + def initialize + @name = self.class.name + @version = nil + @connection = nil + end - case direction - when :up then announce "migrated (%.4fs)" % time.real; write - when :down then announce "reverted (%.4fs)" % time.real; write - end + # instantiate the delegate object after initialize is defined + self.verbose = true + self.delegate = new - result - end + def up + self.class.delegate = self + return unless self.class.respond_to?(:up) + self.class.up + end + + def down + self.class.delegate = self + return unless self.class.respond_to?(:down) + self.class.down + end - # Because the method added may do an alias_method, it can be invoked - # recursively. We use @ignore_new_methods as a guard to indicate whether - # it is safe for the call to proceed. - def singleton_method_added(sym) #:nodoc: - return if defined?(@ignore_new_methods) && @ignore_new_methods + # Execute this migration in the named direction + def migrate(direction) + return unless respond_to?(direction) - begin - @ignore_new_methods = true + case direction + when :up then announce "migrating" + when :down then announce "reverting" + end - case sym - when :up, :down - singleton_class.send(:alias_method_chain, sym, "benchmarks") + time = nil + ActiveRecord::Base.connection_pool.with_connection do |conn| + @connection = conn + if respond_to?(:change) + if direction == :down + recorder = CommandRecorder.new(@connection) + suppress_messages do + @connection = recorder + change + end + @connection = conn + time = Benchmark.measure { + recorder.inverse.each do |cmd, args| + send(cmd, *args) + end + } + else + time = Benchmark.measure { change } end - ensure - @ignore_new_methods = false + else + time = Benchmark.measure { send(direction) } end + @connection = nil end - def write(text="") - puts(text) if verbose + case direction + when :up then announce "migrated (%.4fs)" % time.real; write + when :down then announce "reverted (%.4fs)" % time.real; write end + end - def announce(message) - version = defined?(@version) ? @version : nil + def write(text="") + puts(text) if verbose + end - text = "#{version} #{name}: #{message}" - length = [0, 75 - text.length].max - write "== %s %s" % [text, "=" * length] - end + def announce(message) + text = "#{version} #{name}: #{message}" + length = [0, 75 - text.length].max + write "== %s %s" % [text, "=" * length] + end - def say(message, subitem=false) - write "#{subitem ? " ->" : "--"} #{message}" - end + def say(message, subitem=false) + write "#{subitem ? " ->" : "--"} #{message}" + end - def say_with_time(message) - say(message) - result = nil - time = Benchmark.measure { result = yield } - say "%.4fs" % time.real, :subitem - say("#{result} rows", :subitem) if result.is_a?(Integer) - result - end + def say_with_time(message) + say(message) + result = nil + time = Benchmark.measure { result = yield } + say "%.4fs" % time.real, :subitem + say("#{result} rows", :subitem) if result.is_a?(Integer) + result + end - def suppress_messages - save, self.verbose = verbose, false - yield - ensure - self.verbose = save - end + def suppress_messages + save, self.verbose = verbose, false + yield + ensure + self.verbose = save + end - def connection - ActiveRecord::Base.connection - end + def connection + @connection || ActiveRecord::Base.connection + end - def method_missing(method, *arguments, &block) - arg_list = arguments.map{ |a| a.inspect } * ', ' + def method_missing(method, *arguments, &block) + arg_list = arguments.map{ |a| a.inspect } * ', ' - say_with_time "#{method}(#{arg_list})" do - unless arguments.empty? || method == :execute - arguments[0] = Migrator.proper_table_name(arguments.first) - end - connection.send(method, *arguments, &block) + say_with_time "#{method}(#{arg_list})" do + unless arguments.empty? || method == :execute + arguments[0] = Migrator.proper_table_name(arguments.first) end + return super unless connection.respond_to?(method) + connection.send(method, *arguments, &block) end + end - def copy(destination, sources, options = {}) - copied = [] + def copy(destination, sources, options = {}) + copied = [] - FileUtils.mkdir_p(destination) unless File.exists?(destination) + FileUtils.mkdir_p(destination) unless File.exists?(destination) - destination_migrations = ActiveRecord::Migrator.migrations(destination) - last = destination_migrations.last - sources.each do |name, path| - source_migrations = ActiveRecord::Migrator.migrations(path) + destination_migrations = ActiveRecord::Migrator.migrations(destination) + last = destination_migrations.last + sources.each do |name, path| + source_migrations = ActiveRecord::Migrator.migrations(path) - source_migrations.each do |migration| - source = File.read(migration.filename) - source = "# This migration comes from #{name} (originally #{migration.version})\n#{source}" + source_migrations.each do |migration| + source = File.read(migration.filename) + source = "# This migration comes from #{name} (originally #{migration.version})\n#{source}" - if duplicate = destination_migrations.detect { |m| m.name == migration.name } - options[:on_skip].call(name, migration) if File.read(duplicate.filename) != source && options[:on_skip] - next - end + if duplicate = destination_migrations.detect { |m| m.name == migration.name } + options[:on_skip].call(name, migration) if File.read(duplicate.filename) != source && options[:on_skip] + next + end - migration.version = next_migration_number(last ? last.version + 1 : 0).to_i - new_path = File.join(destination, "#{migration.version}_#{migration.name.underscore}.rb") - old_path, migration.filename = migration.filename, new_path - last = migration + migration.version = next_migration_number(last ? last.version + 1 : 0).to_i + new_path = File.join(destination, "#{migration.version}_#{migration.name.underscore}.rb") + old_path, migration.filename = migration.filename, new_path + last = migration - FileUtils.cp(old_path, migration.filename) - copied << migration - options[:on_copy].call(name, migration, old_path) if options[:on_copy] - destination_migrations << migration - end + FileUtils.cp(old_path, migration.filename) + copied << migration + options[:on_copy].call(name, migration, old_path) if options[:on_copy] + destination_migrations << migration end - - copied end - def next_migration_number(number) - if ActiveRecord::Base.timestamped_migrations - [Time.now.utc.strftime("%Y%m%d%H%M%S"), "%.14d" % number].max - else - "%.3d" % number - end + copied + end + + def next_migration_number(number) + if ActiveRecord::Base.timestamped_migrations + [Time.now.utc.strftime("%Y%m%d%H%M%S"), "%.14d" % number].max + else + "%.3d" % number end end end @@ -451,7 +504,7 @@ module ActiveRecord def load_migration require(File.expand_path(filename)) - name.constantize + name.constantize.new end end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb new file mode 100644 index 0000000000..d7e481905a --- /dev/null +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -0,0 +1,91 @@ +module ActiveRecord + class Migration + # ActiveRecord::Migration::CommandRecorder records commands done during + # a migration and knows how to reverse those commands. The CommandRecorder + # knows how to invert the following commands: + # + # * add_column + # * add_index + # * add_timestamp + # * create_table + # * remove_timestamps + # * rename_column + # * rename_index + # * rename_table + class CommandRecorder + attr_accessor :commands, :delegate + + def initialize(delegate = nil) + @commands = [] + @delegate = delegate + end + + # record +command+. +command+ should be a method name and arguments. + # For example: + # + # recorder.record(:method_name, [:arg1, arg2]) + def record(*command) + @commands << command + end + + # Returns a list that represents commands that are the inverse of the + # commands stored in +commands+. For example: + # + # recorder.record(:rename_table, [:old, :new]) + # recorder.inverse # => [:rename_table, [:new, :old]] + # + # This method will raise an IrreversibleMigration exception if it cannot + # invert the +commands+. + def inverse + @commands.reverse.map { |name, args| + method = :"invert_#{name}" + raise IrreversibleMigration unless respond_to?(method, true) + __send__(method, args) + } + end + + def respond_to?(*args) # :nodoc: + super || delegate.respond_to?(*args) + end + + def send(method, *args) # :nodoc: + return super unless respond_to?(method) + record(method, args) + end + + private + def invert_create_table(args) + [:drop_table, args] + end + + def invert_rename_table(args) + [:rename_table, args.reverse] + end + + def invert_add_column(args) + [:remove_column, args.first(2)] + end + + def invert_rename_index(args) + [:rename_index, args.reverse] + end + + def invert_rename_column(args) + [:rename_column, [args.first] + args.last(2).reverse] + end + + def invert_add_index(args) + table, columns, _ = *args + [:remove_index, [table, {:column => columns}]] + end + + def invert_remove_timestamps(args) + [:add_timestamps, args] + end + + def invert_add_timestamps(args) + [:remove_timestamps, args] + end + end + end +end diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 0b92ba5caa..0f421560f0 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -2,12 +2,18 @@ require 'active_support/core_ext/array' require 'active_support/core_ext/hash/except' require 'active_support/core_ext/kernel/singleton_class' require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/class/attribute' module ActiveRecord # = Active Record Named \Scopes module NamedScope extend ActiveSupport::Concern + included do + class_attribute :scopes + self.scopes = {} + end + module ClassMethods # Returns an anonymous \scope. # @@ -33,10 +39,6 @@ module ActiveRecord end end - def scopes - read_inheritable_attribute(:scopes) || write_inheritable_attribute(:scopes, {}) - end - # Adds a class method for retrieving and querying objects. A \scope represents a narrowing of a database query, # such as <tt>where(:color => :red).select('shirts.*').includes(:washing_instructions)</tt>. # diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 0c3392263a..050b521b6a 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/object/try' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/class/attribute' module ActiveRecord module NestedAttributes #:nodoc: @@ -11,7 +12,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_inheritable_accessor :nested_attributes_options, :instance_writer => false + class_attribute :nested_attributes_options, :instance_writer => false self.nested_attributes_options = {} end @@ -268,7 +269,11 @@ module ActiveRecord if reflection = reflect_on_association(association_name) reflection.options[:autosave] = true add_autosave_association_callbacks(reflection) + + nested_attributes_options = self.nested_attributes_options.dup nested_attributes_options[association_name.to_sym] = options + self.nested_attributes_options = nested_attributes_options + type = (reflection.collection? ? :collection : :one_to_one) # def pirate_attributes=(attributes) @@ -315,11 +320,10 @@ module ActiveRecord # update_only is true, and a <tt>:_destroy</tt> key set to a truthy value, # then the existing record will be marked for destruction. def assign_nested_attributes_for_one_to_one_association(association_name, attributes) - options = nested_attributes_options[association_name] + options = self.nested_attributes_options[association_name] attributes = attributes.with_indifferent_access - check_existing_record = (options[:update_only] || !attributes['id'].blank?) - if check_existing_record && (record = send(association_name)) && + if (options[:update_only] || !attributes['id'].blank?) && (record = send(association_name)) && (options[:update_only] || record.id.to_s == attributes['id'].to_s) assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes) @@ -364,7 +368,7 @@ module ActiveRecord # { :id => '2', :_destroy => true } # ]) def assign_nested_attributes_for_collection_association(association_name, attributes_collection) - options = nested_attributes_options[association_name] + options = self.nested_attributes_options[association_name] unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" @@ -413,11 +417,8 @@ module ActiveRecord # Updates a record with the +attributes+ or marks it for destruction if # +allow_destroy+ is +true+ and has_destroy_flag? returns +true+. def assign_to_or_mark_for_destruction(record, attributes, allow_destroy) - if has_destroy_flag?(attributes) && allow_destroy - record.mark_for_destruction - else - record.attributes = attributes.except(*UNASSIGNABLE_KEYS) - end + record.attributes = attributes.except(*UNASSIGNABLE_KEYS) + record.mark_for_destruction if has_destroy_flag?(attributes) && allow_destroy end # Determines if a hash contains a truthy _destroy key. @@ -433,7 +434,7 @@ module ActiveRecord end def call_reject_if(association_name, attributes) - case callback = nested_attributes_options[association_name][:reject_if] + case callback = self.nested_attributes_options[association_name][:reject_if] when Symbol method(callback).arity == 0 ? send(callback) : send(callback, attributes) when Proc @@ -442,8 +443,7 @@ module ActiveRecord end def raise_nested_attributes_record_not_found(association_name, record_id) - reflection = self.class.reflect_on_association(association_name) - raise RecordNotFound, "Couldn't find #{reflection.klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}" + raise RecordNotFound, "Couldn't find #{self.class.reflect_on_association(association_name).klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}" end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index ba37fed3c7..d1de8cefb9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1,8 +1,15 @@ +require 'active_support/core_ext/class/attribute' + module ActiveRecord # = Active Record Reflection module Reflection # :nodoc: extend ActiveSupport::Concern + included do + class_attribute :reflections + self.reflections = {} + end + # Reflection enables to interrogate Active Record classes and objects # about their associations and aggregations. This information can, # for example, be used in a form builder that takes an Active Record object @@ -20,18 +27,9 @@ module ActiveRecord when :composed_of reflection = AggregateReflection.new(macro, name, options, active_record) end - write_inheritable_hash :reflections, name => reflection - reflection - end - # Returns a hash containing all AssociationReflection objects for the current class. - # Example: - # - # Invoice.reflections - # Account.reflections - # - def reflections - read_inheritable_attribute(:reflections) || write_inheritable_attribute(:reflections, {}) + self.reflections = self.reflections.merge(name => reflection) + reflection end # Returns an array of AggregateReflection objects for all the aggregations in the class. diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 4192456447..23ae0b4325 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -202,7 +202,7 @@ module ActiveRecord end def construct_relation_for_association_find(join_dependency) - relation = except(:includes, :eager_load, :preload, :select).select(column_aliases(join_dependency)) + relation = except(:includes, :eager_load, :preload, :select).select(join_dependency.columns) apply_join_dependency(relation, join_dependency) end @@ -349,17 +349,8 @@ module ActiveRecord end end - def column_aliases(join_dependency) - join_dependency.join_parts.collect { |join_part| - join_part.column_names_with_alias.collect{ |column_name, aliased_name| - "#{connection.quote_table_name join_part.aliased_table_name}.#{connection.quote_column_name column_name} AS #{aliased_name}" - } - }.flatten.join(", ") - end - def using_limitable_reflections?(reflections) reflections.none? { |r| r.collection? } end - end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 32c7d08daa..70d84619a1 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -26,7 +26,7 @@ module ActiveRecord when Range, Arel::Relation attribute.in(value) when ActiveRecord::Base - attribute.eq(value.quoted_id) + attribute.eq(Arel.sql(value.quoted_id)) when Class # FIXME: I think we need to deprecate this behavior attribute.eq(value.name) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 9c399d3333..0a4c119849 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -13,7 +13,7 @@ module ActiveRecord def includes(*args) args.reject! {|a| a.blank? } - return clone if args.empty? + return self if args.empty? relation = clone relation.includes_values = (relation.includes_values + args).flatten.uniq @@ -21,14 +21,18 @@ module ActiveRecord end def eager_load(*args) + return self if args.blank? + relation = clone - relation.eager_load_values += args unless args.blank? + relation.eager_load_values += args relation end def preload(*args) + return self if args.blank? + relation = clone - relation.preload_values += args unless args.blank? + relation.preload_values += args relation end @@ -43,22 +47,28 @@ module ActiveRecord end def group(*args) + return self if args.blank? + relation = clone - relation.group_values += args.flatten unless args.blank? + relation.group_values += args.flatten relation end def order(*args) + return self if args.blank? + relation = clone - relation.order_values += args.flatten unless args.blank? + relation.order_values += args.flatten relation end def joins(*args) + return self if args.blank? + relation = clone args.flatten! - relation.joins_values += args unless args.blank? + relation.joins_values += args relation end @@ -70,14 +80,18 @@ module ActiveRecord end def where(opts, *rest) + return self if opts.blank? + relation = clone - relation.where_values += build_where(opts, rest) unless opts.blank? + relation.where_values += build_where(opts, rest) relation end def having(*args) + return self if args.blank? + relation = clone - relation.having_values += build_where(*args) unless args.blank? + relation.having_values += build_where(*args) relation end @@ -141,7 +155,7 @@ module ActiveRecord "#{@klass.table_name}.#{@klass.primary_key} DESC" : reverse_sql_order(order_clause).join(', ') - except(:order).order(Arel::SqlLiteral.new(order)) + except(:order).order(Arel.sql(order)) end def arel @@ -174,10 +188,7 @@ module ActiveRecord arel = build_joins(arel, @joins_values) unless @joins_values.empty? - (@where_values - ['']).uniq.each do |where| - where = Arel.sql(where) if String === where - arel = arel.where(Arel::Nodes::Grouping.new(where)) - end + arel = collapse_wheres(arel, (@where_values - ['']).uniq) arel = arel.having(*@having_values.uniq.reject{|h| h.blank?}) unless @having_values.empty? @@ -198,6 +209,27 @@ module ActiveRecord private + def collapse_wheres(arel, wheres) + equalities = wheres.grep(Arel::Nodes::Equality) + + groups = equalities.group_by do |equality| + equality.left + end + + groups.each do |_, eqls| + test = eqls.inject(eqls.shift) do |memo, expr| + memo.or(expr) + end + arel = arel.where(test) + end + + (wheres - equalities).each do |where| + where = Arel.sql(where) if String === where + arel = arel.where(Arel::Nodes::Grouping.new(where)) + end + arel + end + def build_where(opts, other = []) case opts when String, Array diff --git a/activerecord/lib/active_record/schema.rb b/activerecord/lib/active_record/schema.rb index c1bc3214ea..c6bb5c1961 100644 --- a/activerecord/lib/active_record/schema.rb +++ b/activerecord/lib/active_record/schema.rb @@ -30,9 +30,7 @@ module ActiveRecord # ActiveRecord::Schema is only supported by database adapters that also # support migrations, the two features being very similar. class Schema < Migration - private_class_method :new - - def self.migrations_path + def migrations_path ActiveRecord::Migrator.migrations_path end @@ -48,11 +46,12 @@ module ActiveRecord # ... # end def self.define(info={}, &block) - instance_eval(&block) + schema = new + schema.instance_eval(&block) unless info[:version].blank? initialize_schema_migrations_table - assume_migrated_upto_version(info[:version], migrations_path) + assume_migrated_upto_version(info[:version], schema.migrations_path) end end end diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 230adf6b2b..2ecbd906bd 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/class/attribute' + module ActiveRecord # = Active Record Timestamp # @@ -29,14 +31,14 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_inheritable_accessor :record_timestamps, :instance_writer => false + class_attribute :record_timestamps, :instance_writer => false self.record_timestamps = true end private def create #:nodoc: - if record_timestamps + if self.record_timestamps current_time = current_time_from_proper_timezone all_timestamp_attributes.each do |column| @@ -61,7 +63,7 @@ module ActiveRecord end def should_record_timestamps? - record_timestamps && (!partial_updates? || changed? || (attributes.keys & self.class.serialized_attributes.keys).present?) + self.record_timestamps && (!partial_updates? || changed? || (attributes.keys & self.class.serialized_attributes.keys).present?) end def timestamp_attributes_for_update_in_model diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 3eba7510ac..853808eebf 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -14,17 +14,14 @@ module ActiveRecord def validate_each(record, attribute, value) finder_class = find_finder_class_for(record) - table = finder_class.unscoped - - table_name = record.class.quoted_table_name if value && record.class.serialized_attributes.key?(attribute.to_s) value = YAML.dump value end - sql, params = mount_sql_and_params(finder_class, table_name, attribute, value) + sql, params = mount_sql_and_params(finder_class, record.class.quoted_table_name, attribute, value) - relation = table.where(sql, *params) + relation = finder_class.unscoped.where(sql, *params) Array.wrap(options[:scope]).each do |scope_item| scope_value = record.send(scope_item) diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb index 8ac21c1410..126b6f434b 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb @@ -1,5 +1,5 @@ class <%= migration_class_name %> < ActiveRecord::Migration - def self.up + def up <% attributes.each do |attribute| -%> <%- if migration_action -%> <%= migration_action %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'add' %>, :<%= attribute.type %><% end %> @@ -7,7 +7,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration <%- end -%> end - def self.down + def down <% attributes.reverse.each do |attribute| -%> <%- if migration_action -%> <%= migration_action == 'add' ? 'remove' : 'add' %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'remove' %>, :<%= attribute.type %><% end %> diff --git a/activerecord/lib/rails/generators/active_record/model/templates/migration.rb b/activerecord/lib/rails/generators/active_record/model/templates/migration.rb index 1f68487304..70e064be21 100644 --- a/activerecord/lib/rails/generators/active_record/model/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/model/templates/migration.rb @@ -1,5 +1,5 @@ class <%= migration_class_name %> < ActiveRecord::Migration - def self.up + def up create_table :<%= table_name %> do |t| <% for attribute in attributes -%> t.<%= attribute.type %> :<%= attribute.name %> @@ -10,7 +10,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration end end - def self.down + def down drop_table :<%= table_name %> end end diff --git a/activerecord/lib/rails/generators/active_record/session_migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/session_migration/templates/migration.rb index 919822af7b..8f0bf1ef0d 100644 --- a/activerecord/lib/rails/generators/active_record/session_migration/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/session_migration/templates/migration.rb @@ -1,5 +1,5 @@ class <%= migration_class_name %> < ActiveRecord::Migration - def self.up + def up create_table :<%= session_table_name %> do |t| t.string :session_id, :null => false t.text :data @@ -10,7 +10,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration add_index :<%= session_table_name %>, :updated_at end - def self.down + def down drop_table :<%= session_table_name %> end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 10fb80f822..1c740c4660 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -79,6 +79,58 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_preloading_has_many_in_multiple_queries_with_more_ids_than_database_can_handle + Post.connection.expects(:in_clause_length).at_least_once.returns(5) + posts = Post.find(:all, :include=>:comments) + assert_equal 11, posts.size + end + + def test_preloading_has_many_in_one_queries_when_database_has_no_limit_on_ids_it_can_handle + Post.connection.expects(:in_clause_length).at_least_once.returns(nil) + posts = Post.find(:all, :include=>:comments) + assert_equal 11, posts.size + end + + def test_preloading_habtm_in_multiple_queries_with_more_ids_than_database_can_handle + Post.connection.expects(:in_clause_length).at_least_once.returns(5) + posts = Post.find(:all, :include=>:categories) + assert_equal 11, posts.size + end + + def test_preloading_habtm_in_one_queries_when_database_has_no_limit_on_ids_it_can_handle + Post.connection.expects(:in_clause_length).at_least_once.returns(nil) + posts = Post.find(:all, :include=>:categories) + assert_equal 11, posts.size + end + + def test_load_associated_records_in_one_query_when_adapter_has_no_limit + Post.connection.expects(:in_clause_length).at_least_once.returns(nil) + Post.expects(:i_was_called).with([1,2,3,4,5,6,7]).returns([1]) + associated_records = Post.send(:associated_records, [1,2,3,4,5,6,7]) do |some_ids| + Post.i_was_called(some_ids) + end + assert_equal [1], associated_records + end + + def test_load_associated_records_in_several_queries_when_many_ids_passed + Post.connection.expects(:in_clause_length).at_least_once.returns(5) + Post.expects(:i_was_called).with([1,2,3,4,5]).returns([1]) + Post.expects(:i_was_called).with([6,7]).returns([6]) + associated_records = Post.send(:associated_records, [1,2,3,4,5,6,7]) do |some_ids| + Post.i_was_called(some_ids) + end + assert_equal [1,6], associated_records + end + + def test_load_associated_records_in_one_query_when_a_few_ids_passed + Post.connection.expects(:in_clause_length).at_least_once.returns(5) + Post.expects(:i_was_called).with([1,2,3]).returns([1]) + associated_records = Post.send(:associated_records, [1,2,3]) do |some_ids| + Post.i_was_called(some_ids) + end + assert_equal [1], associated_records + end + def test_including_duplicate_objects_from_belongs_to popular_post = Post.create!(:title => 'foo', :body => "I like cars!") comment = popular_post.comments.create!(:body => "lol") diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 33c53e695b..fb772bb8e6 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -67,8 +67,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_no_sql_should_be_fired_if_association_already_loaded - car = Car.create(:name => 'honda') - bulb = car.bulbs.create + Car.create(:name => 'honda') bulbs = Car.first.bulbs bulbs.inspect # to load all instances of bulbs assert_no_queries do 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 7c3d20e8bd..5fac52257c 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -358,7 +358,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_association_through_a_belongs_to_association_where_the_association_doesnt_exist - author = authors(:mary) post = Post.create!(:title => "TITLE", :body => "BODY") assert_equal [], post.author_favorites end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 6fbeff8aa9..64449df8f5 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -163,7 +163,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm = ExclusivelyDependentFirm.find(9) assert_not_nil firm.account - account_id = firm.account.id assert_equal [], Account.destroyed_account_ids[firm.id] firm.destroy @@ -180,7 +179,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_dependence_with_restrict firm = RestrictedFirm.new(:name => 'restrict') firm.save! - account = firm.create_account(:credit_limit => 10) + firm.create_account(:credit_limit => 10) assert_not_nil firm.account assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } end @@ -197,8 +196,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_build_association_twice_without_saving_affects_nothing count_of_account = Account.count firm = Firm.find(:first) - account1 = firm.build_account("credit_limit" => 1000) - account2 = firm.build_account("credit_limit" => 2000) + firm.build_account("credit_limit" => 1000) + firm.build_account("credit_limit" => 2000) assert_equal count_of_account, Account.count end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 081583038f..0491b2d1fa 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -484,7 +484,6 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase def test_child_instance_should_be_shared_with_replaced_via_accessor_parent face = faces(:confused) - old_man = face.polymorphic_man new_man = Man.new assert_not_nil face.polymorphic_man @@ -499,7 +498,6 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase def test_child_instance_should_be_shared_with_replaced_via_method_parent face = faces(:confused) - old_man = face.polymorphic_man new_man = Man.new assert_not_nil face.polymorphic_man diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 217d06f3d9..9964b826d4 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -634,7 +634,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_preload_nil_polymorphic_belongs_to assert_nothing_raised do - taggings = Tagging.find(:all, :include => :taggable, :conditions => ['taggable_type IS NULL']) + Tagging.find(:all, :include => :taggable, :conditions => ['taggable_type IS NULL']) end end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 93a51d3606..83c605d2bb 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -270,17 +270,17 @@ class OverridingAssociationsTest < ActiveRecord::TestCase def test_habtm_association_redefinition_callbacks_should_differ_and_not_inherited # redeclared association on AR descendant should not inherit callbacks from superclass - callbacks = PeopleList.read_inheritable_attribute(:before_add_for_has_and_belongs_to_many) + callbacks = PeopleList.before_add_for_has_and_belongs_to_many assert_equal([:enlist], callbacks) - callbacks = DifferentPeopleList.read_inheritable_attribute(:before_add_for_has_and_belongs_to_many) + callbacks = DifferentPeopleList.before_add_for_has_and_belongs_to_many assert_equal([], callbacks) end def test_has_many_association_redefinition_callbacks_should_differ_and_not_inherited # redeclared association on AR descendant should not inherit callbacks from superclass - callbacks = PeopleList.read_inheritable_attribute(:before_add_for_has_many) + callbacks = PeopleList.before_add_for_has_many assert_equal([:enlist], callbacks) - callbacks = DifferentPeopleList.read_inheritable_attribute(:before_add_for_has_many) + callbacks = DifferentPeopleList.before_add_for_has_many assert_equal([], callbacks) end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index b13cb2d7a2..fbf7121468 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -163,15 +163,15 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas firm.account = Account.find(:first) assert_queries(Firm.partial_updates? ? 0 : 1) { firm.save! } - firm = Firm.find(:first).clone + firm = Firm.find(:first).dup firm.account = Account.find(:first) assert_queries(2) { firm.save! } - firm = Firm.find(:first).clone - firm.account = Account.find(:first).clone + firm = Firm.find(:first).dup + firm.account = Account.find(:first).dup assert_queries(2) { firm.save! } end - + def test_callbacks_firing_order_on_create eye = Eye.create(:iris_attributes => {:color => 'honey'}) assert_equal [true, false], eye.after_create_callbacks_stack diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 26f388ca46..c3ba1f0c35 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -669,64 +669,65 @@ class BasicsTest < ActiveRecord::TestCase assert_equal true, Topic.find(1).persisted? end - def test_clone + def test_dup topic = Topic.find(1) - cloned_topic = nil - assert_nothing_raised { cloned_topic = topic.clone } - assert_equal topic.title, cloned_topic.title - assert !cloned_topic.persisted? + duped_topic = nil + assert_nothing_raised { duped_topic = topic.dup } + assert_equal topic.title, duped_topic.title + assert !duped_topic.persisted? - # test if the attributes have been cloned + # test if the attributes have been duped topic.title = "a" - cloned_topic.title = "b" + duped_topic.title = "b" assert_equal "a", topic.title - assert_equal "b", cloned_topic.title + assert_equal "b", duped_topic.title - # test if the attribute values have been cloned + # test if the attribute values have been duped topic.title = {"a" => "b"} - cloned_topic = topic.clone - cloned_topic.title["a"] = "c" + duped_topic = topic.dup + duped_topic.title["a"] = "c" assert_equal "b", topic.title["a"] - # test if attributes set as part of after_initialize are cloned correctly - assert_equal topic.author_email_address, cloned_topic.author_email_address + # test if attributes set as part of after_initialize are duped correctly + assert_equal topic.author_email_address, duped_topic.author_email_address # test if saved clone object differs from original - cloned_topic.save - assert cloned_topic.persisted? - assert_not_equal cloned_topic.id, topic.id + duped_topic.save + assert duped_topic.persisted? + assert_not_equal duped_topic.id, topic.id - cloned_topic.reload + duped_topic.reload # FIXME: I think this is poor behavior, and will fix it with #5686 - assert_equal({'a' => 'c'}.to_s, cloned_topic.title) + assert_equal({'a' => 'c'}.to_s, duped_topic.title) end - def test_clone_with_aggregate_of_same_name_as_attribute + def test_dup_with_aggregate_of_same_name_as_attribute dev = DeveloperWithAggregate.find(1) assert_kind_of DeveloperSalary, dev.salary - clone = nil - assert_nothing_raised { clone = dev.clone } - assert_kind_of DeveloperSalary, clone.salary - assert_equal dev.salary.amount, clone.salary.amount - assert !clone.persisted? + dup = nil + assert_nothing_raised { dup = dev.dup } + assert_kind_of DeveloperSalary, dup.salary + assert_equal dev.salary.amount, dup.salary.amount + assert !dup.persisted? - # test if the attributes have been cloned - original_amount = clone.salary.amount + # test if the attributes have been dupd + original_amount = dup.salary.amount dev.salary.amount = 1 - assert_equal original_amount, clone.salary.amount + assert_equal original_amount, dup.salary.amount - assert clone.save - assert clone.persisted? - assert_not_equal clone.id, dev.id + assert dup.save + assert dup.persisted? + assert_not_equal dup.id, dev.id end - def test_clone_does_not_clone_associations + def test_dup_does_not_copy_associations author = authors(:david) assert_not_equal [], author.posts + author.send(:clear_association_cache) - author_clone = author.clone - assert_equal [], author_clone.posts + author_dup = author.dup + assert_equal [], author_dup.posts end def test_clone_preserves_subtype @@ -765,22 +766,22 @@ class BasicsTest < ActiveRecord::TestCase assert !cloned_developer.salary_changed? # ... and cloned instance should behave same end - def test_clone_of_saved_object_marks_attributes_as_dirty + def test_dup_of_saved_object_marks_attributes_as_dirty developer = Developer.create! :name => 'Bjorn', :salary => 100000 assert !developer.name_changed? assert !developer.salary_changed? - cloned_developer = developer.clone + cloned_developer = developer.dup assert cloned_developer.name_changed? # both attributes differ from defaults assert cloned_developer.salary_changed? end - def test_clone_of_saved_object_marks_as_dirty_only_changed_attributes + def test_dup_of_saved_object_marks_as_dirty_only_changed_attributes developer = Developer.create! :name => 'Bjorn' assert !developer.name_changed? # both attributes of saved object should be threated as not changed assert !developer.salary_changed? - cloned_developer = developer.clone + cloned_developer = developer.dup assert cloned_developer.name_changed? # ... but on cloned object should be assert !cloned_developer.salary_changed? # ... BUT salary has non-nil default which should be threated as not changed on cloned instance end @@ -1443,10 +1444,6 @@ class BasicsTest < ActiveRecord::TestCase ActiveRecord::Base.logger = original_logger end - def test_dup - assert !Minimalistic.new.freeze.dup.frozen? - end - def test_compute_type_success assert_equal Author, ActiveRecord::Base.send(:compute_type, 'Author') end diff --git a/activerecord/test/cases/clone_test.rb b/activerecord/test/cases/clone_test.rb new file mode 100644 index 0000000000..d91646efca --- /dev/null +++ b/activerecord/test/cases/clone_test.rb @@ -0,0 +1,33 @@ +require "cases/helper" +require 'models/topic' + +module ActiveRecord + class CloneTest < ActiveRecord::TestCase + fixtures :topics + + def test_persisted + topic = Topic.first + cloned = topic.clone + assert topic.persisted?, 'topic persisted' + assert cloned.persisted?, 'topic persisted' + assert !cloned.new_record?, 'topic is not new' + end + + def test_stays_frozen + topic = Topic.first + topic.freeze + + cloned = topic.clone + assert cloned.persisted?, 'topic persisted' + assert !cloned.new_record?, 'topic is not new' + assert cloned.frozen?, 'topic should be frozen' + end + + def test_shallow + topic = Topic.first + cloned = topic.clone + topic.author_name = 'Aaron' + assert_equal 'Aaron', cloned.author_name + end + end +end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index b1a54af192..a6738fb654 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -338,13 +338,13 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.changed? end - def test_cloned_objects_should_not_copy_dirty_flag_from_creator + def test_dup_objects_should_not_copy_dirty_flag_from_creator pirate = Pirate.create!(:catchphrase => "shiver me timbers") - pirate_clone = pirate.clone - pirate_clone.reset_catchphrase! + pirate_dup = pirate.dup + pirate_dup.reset_catchphrase! pirate.catchphrase = "I love Rum" assert pirate.catchphrase_changed? - assert !pirate_clone.catchphrase_changed? + assert !pirate_dup.catchphrase_changed? end def test_reverted_changes_are_not_dirty diff --git a/activerecord/test/cases/dup_test.rb b/activerecord/test/cases/dup_test.rb new file mode 100644 index 0000000000..0236f9b0a1 --- /dev/null +++ b/activerecord/test/cases/dup_test.rb @@ -0,0 +1,103 @@ +require "cases/helper" +require 'models/topic' + +module ActiveRecord + class DupTest < ActiveRecord::TestCase + fixtures :topics + + def test_dup + assert !Topic.new.freeze.dup.frozen? + end + + def test_not_readonly + topic = Topic.first + + duped = topic.dup + assert !duped.readonly?, 'should not be readonly' + end + + def test_is_readonly + topic = Topic.first + topic.readonly! + + duped = topic.dup + assert duped.readonly?, 'should be readonly' + end + + def test_dup_not_persisted + topic = Topic.first + duped = topic.dup + + assert !duped.persisted?, 'topic not persisted' + assert duped.new_record?, 'topic is new' + end + + def test_dup_has_no_id + topic = Topic.first + duped = topic.dup + assert_nil duped.id + end + + def test_dup_with_modified_attributes + topic = Topic.first + topic.author_name = 'Aaron' + duped = topic.dup + assert_equal 'Aaron', duped.author_name + end + + def test_dup_with_changes + dbtopic = Topic.first + topic = Topic.new + + topic.attributes = dbtopic.attributes + + #duped has no timestamp values + duped = dbtopic.dup + + #clear topic timestamp values + topic.send(:clear_timestamp_attributes) + + assert_equal topic.changes, duped.changes + end + + def test_dup_topics_are_independent + topic = Topic.first + topic.author_name = 'Aaron' + duped = topic.dup + + duped.author_name = 'meow' + + assert_not_equal topic.changes, duped.changes + end + + def test_dup_attributes_are_independent + topic = Topic.first + duped = topic.dup + + duped.author_name = 'meow' + topic.author_name = 'Aaron' + + assert_equal 'Aaron', topic.author_name + assert_equal 'meow', duped.author_name + end + + def test_dup_timestamps_are_cleared + topic = Topic.first + assert_not_nil topic.updated_at + assert_not_nil topic.created_at + + # temporary change to the topic object + topic.updated_at -= 3.days + + #dup should not preserve the timestamps if present + new_topic = topic.dup + assert_nil new_topic.updated_at + assert_nil new_topic.created_at + + new_topic.save + assert_not_nil new_topic.updated_at + assert_not_nil new_topic.created_at + end + + end +end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 52f26b71f5..f9bbc5299b 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -13,11 +13,6 @@ require 'active_record' require 'active_support/dependencies' require 'connection' -begin - require 'ruby-debug' -rescue LoadError -end - # Show backtraces for deprecated behavior for quicker cleanup. ActiveSupport::Deprecation.debug = true diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 31679b2efe..c3da9cdf53 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -16,7 +16,7 @@ class InheritanceTest < ActiveRecord::TestCase def test_class_with_blank_sti_name company = Company.find(:first) - company = company.clone + company = company.dup company.extend(Module.new { def read_attribute(name) return ' ' if name == 'type' diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb new file mode 100644 index 0000000000..afec64750e --- /dev/null +++ b/activerecord/test/cases/invertible_migration_test.rb @@ -0,0 +1,57 @@ +require "cases/helper" + +module ActiveRecord + class InvertibleMigrationTest < ActiveRecord::TestCase + class SilentMigration < ActiveRecord::Migration + def write(text = '') + # sssshhhhh!! + end + end + + class InvertibleMigration < SilentMigration + def change + create_table("horses") do |t| + t.column :content, :text + t.column :remind_at, :datetime + end + end + end + + class NonInvertibleMigration < SilentMigration + def change + create_table("horses") do |t| + t.column :content, :text + t.column :remind_at, :datetime + end + remove_column "horses", :content + end + end + + def teardown + if ActiveRecord::Base.connection.table_exists?("horses") + ActiveRecord::Base.connection.drop_table("horses") + end + end + + def test_no_reverse + migration = NonInvertibleMigration.new + migration.migrate(:up) + assert_raises(IrreversibleMigration) do + migration.migrate(:down) + end + end + + def test_up + migration = InvertibleMigration.new + migration.migrate(:up) + assert migration.connection.table_exists?("horses"), "horses should exist" + end + + def test_down + migration = InvertibleMigration.new + migration.migrate :up + migration.migrate :down + assert !migration.connection.table_exists?("horses") + end + end +end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 4ddcdc010b..f9678cb0c5 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -1,3 +1,4 @@ +require 'thread' require "cases/helper" require 'models/person' require 'models/reader' diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb new file mode 100644 index 0000000000..ea2292dda5 --- /dev/null +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -0,0 +1,108 @@ +require "cases/helper" + +module ActiveRecord + class Migration + class CommandRecorderTest < ActiveRecord::TestCase + def setup + @recorder = CommandRecorder.new + end + + def test_respond_to_delegates + recorder = CommandRecorder.new(Class.new { + def america; end + }.new) + assert recorder.respond_to?(:america) + end + + def test_send_calls_super + assert_raises(NoMethodError) do + @recorder.send(:create_table, :horses) + end + end + + def test_send_delegates_to_record + recorder = CommandRecorder.new(Class.new { + def create_table(name); end + }.new) + assert recorder.respond_to?(:create_table), 'respond_to? create_table' + recorder.send(:create_table, :horses) + assert_equal [[:create_table, [:horses]]], recorder.commands + end + + def test_unknown_commands_raise_exception + @recorder.record :execute, ['some sql'] + assert_raises(ActiveRecord::IrreversibleMigration) do + @recorder.inverse + end + end + + def test_record + @recorder.record :create_table, [:system_settings] + assert_equal 1, @recorder.commands.length + end + + def test_inverse + @recorder.record :create_table, [:system_settings] + assert_equal 1, @recorder.inverse.length + + @recorder.record :rename_table, [:old, :new] + assert_equal 2, @recorder.inverse.length + end + + def test_inverted_commands_are_reveresed + @recorder.record :create_table, [:hello] + @recorder.record :create_table, [:world] + tables = @recorder.inverse.map(&:last) + assert_equal [[:world], [:hello]], tables + end + + def test_invert_create_table + @recorder.record :create_table, [:system_settings] + drop_table = @recorder.inverse.first + assert_equal [:drop_table, [:system_settings]], drop_table + end + + def test_invert_rename_table + @recorder.record :rename_table, [:old, :new] + rename = @recorder.inverse.first + assert_equal [:rename_table, [:new, :old]], rename + end + + def test_invert_add_column + @recorder.record :add_column, [:table, :column, :type, {}] + remove = @recorder.inverse.first + assert_equal [:remove_column, [:table, :column]], remove + end + + def test_invert_rename_column + @recorder.record :rename_column, [:table, :old, :new] + rename = @recorder.inverse.first + assert_equal [:rename_column, [:table, :new, :old]], rename + end + + def test_invert_add_index + @recorder.record :add_index, [:table, [:one, :two], {:options => true}] + remove = @recorder.inverse.first + assert_equal [:remove_index, [:table, {:column => [:one, :two]}]], remove + end + + def test_invert_rename_index + @recorder.record :rename_index, [:old, :new] + rename = @recorder.inverse.first + assert_equal [:rename_index, [:new, :old]], rename + end + + def test_invert_add_timestamps + @recorder.record :add_timestamps, [:table] + remove = @recorder.inverse.first + assert_equal [:remove_timestamps, [:table]], remove + end + + def test_invert_remove_timestamps + @recorder.record :remove_timestamps, [:table] + add = @recorder.inverse.first + assert_equal [:add_timestamps, [:table]], add + end + end + end +end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index ab9b35172b..3037d73a1b 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -18,10 +18,11 @@ if ActiveRecord::Base.connection.supports_migrations? class ActiveRecord::Migration class <<self attr_accessor :message_count - def puts(text="") - self.message_count ||= 0 - self.message_count += 1 - end + end + + def puts(text="") + self.class.message_count ||= 0 + self.class.message_count += 1 end end @@ -1165,6 +1166,44 @@ if ActiveRecord::Base.connection.supports_migrations? assert_raise(ActiveRecord::StatementInvalid) { Reminder.find(:first) } end + class MockMigration < ActiveRecord::Migration + attr_reader :went_up, :went_down + def initialize + @went_up = false + @went_down = false + end + + def up + @went_up = true + super + end + + def down + @went_down = true + super + end + end + + def test_instance_based_migration_up + migration = MockMigration.new + assert !migration.went_up, 'have not gone up' + assert !migration.went_down, 'have not gone down' + + migration.migrate :up + assert migration.went_up, 'have gone up' + assert !migration.went_down, 'have not gone down' + end + + def test_instance_based_migration_down + migration = MockMigration.new + assert !migration.went_up, 'have not gone up' + assert !migration.went_down, 'have not gone down' + + migration.migrate :down + assert !migration.went_up, 'have gone up' + assert migration.went_down, 'have not gone down' + end + def test_migrator_one_up assert !Person.column_methods_hash.include?(:last_name) assert !Reminder.table_exists? @@ -1312,20 +1351,20 @@ if ActiveRecord::Base.connection.supports_migrations? def test_migrator_verbosity ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/valid", 1) - assert PeopleHaveLastNames.message_count > 0 + assert_operator PeopleHaveLastNames.message_count, :>, 0 PeopleHaveLastNames.message_count = 0 ActiveRecord::Migrator.down(MIGRATIONS_ROOT + "/valid", 0) - assert PeopleHaveLastNames.message_count > 0 + assert_operator PeopleHaveLastNames.message_count, :>, 0 PeopleHaveLastNames.message_count = 0 end def test_migrator_verbosity_off PeopleHaveLastNames.verbose = false ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/valid", 1) - assert PeopleHaveLastNames.message_count.zero? + assert_equal 0, PeopleHaveLastNames.message_count ActiveRecord::Migrator.down(MIGRATIONS_ROOT + "/valid", 0) - assert PeopleHaveLastNames.message_count.zero? + assert_equal 0, PeopleHaveLastNames.message_count end def test_migrator_going_down_due_to_version_target diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 92af53d56f..fb6a239545 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -827,7 +827,7 @@ class TestNestedAttributesWithNonStandardPrimaryKeys < ActiveRecord::TestCase fixtures :owners, :pets def setup - Owner.accepts_nested_attributes_for :pets + Owner.accepts_nested_attributes_for :pets, :allow_destroy => true @owner = owners(:ashley) @pet1, @pet2 = pets(:chew), pets(:mochi) @@ -844,6 +844,19 @@ class TestNestedAttributesWithNonStandardPrimaryKeys < ActiveRecord::TestCase @owner.update_attributes(@params) assert_equal ['Foo', 'Bar'], @owner.pets.map(&:name) end + + def test_attr_accessor_of_child_should_be_value_provided_during_update_attributes + @owner = owners(:ashley) + @pet1 = pets(:chew) + assert_equal nil, $current_user + attributes = {:pets_attributes => { "1"=> { :id => @pet1.id, + :name => "Foo2", + :current_user => "John", + :_destroy=>true }}} + @owner.update_attributes(attributes) + assert_equal 'John', $after_destroy_callback_output + end + end class TestHasOneAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 7de7b95fa8..688fc70484 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -14,11 +14,18 @@ require 'models/bird' require 'models/car' require 'models/engine' require 'models/tyre' +require 'models/minivan' class RelationTest < ActiveRecord::TestCase fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments, - :tags, :taggings, :cars + :tags, :taggings, :cars, :minivans + + def test_do_not_double_quote_string_id + van = Minivan.last + assert van + assert_equal van.id, Minivan.where(:minivan_id => van).to_a.first.minivan_id + end def test_bind_values relation = Post.scoped @@ -451,6 +458,27 @@ class RelationTest < ActiveRecord::TestCase assert_equal author, authors end + def test_find_all_using_where_twice_should_or_the_relation + david = authors(:david) + relation = Author.unscoped + relation = relation.where(:name => david.name) + relation = relation.where(:name => 'Santiago') + relation = relation.where(:id => david.id) + assert_equal [david], relation.all + end + + def test_find_all_with_multiple_ors + david = authors(:david) + relation = [ + { :name => david.name }, + { :name => 'Santiago' }, + { :name => 'tenderlove' }, + ].inject(Author.unscoped) do |memo, param| + memo.where(param) + end + assert_equal [david], relation.all + end + def test_exists davids = Author.where(:name => 'David') assert davids.exists? diff --git a/activerecord/test/models/pet.rb b/activerecord/test/models/pet.rb index a8bf94dd86..570db4c8d5 100644 --- a/activerecord/test/models/pet.rb +++ b/activerecord/test/models/pet.rb @@ -1,5 +1,13 @@ class Pet < ActiveRecord::Base + + attr_accessor :current_user + set_primary_key :pet_id belongs_to :owner, :touch => true has_many :toys + + after_destroy do |record| + $after_destroy_callback_output = record.current_user + end + end |