diff options
Diffstat (limited to 'activerecord/lib/active_record/associations')
26 files changed, 655 insertions, 595 deletions
diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index a6a1947148..021bc32237 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -2,23 +2,25 @@ require 'active_support/core_ext/string/conversions' module ActiveRecord module Associations - # Keeps track of table aliases for ActiveRecord::Associations::ClassMethods::JoinDependency and - # ActiveRecord::Associations::ThroughAssociationScope + # Keeps track of table aliases for ActiveRecord::Associations::JoinDependency class AliasTracker # :nodoc: - attr_reader :aliases, :connection + attr_reader :aliases - def self.empty(connection) - new connection, Hash.new(0) + def self.create(connection, initial_table, type_caster) + aliases = Hash.new(0) + aliases[initial_table] = 1 + new connection, aliases, type_caster end - def self.create(connection, table_joins) - if table_joins.empty? - empty connection + def self.create_with_joins(connection, initial_table, joins, type_caster) + if joins.empty? + create(connection, initial_table, type_caster) else - aliases = Hash.new { |h,k| - h[k] = initial_count_for(connection, k, table_joins) + aliases = Hash.new { |h, k| + h[k] = initial_count_for(connection, k, joins) } - new connection, aliases + aliases[initial_table] = 1 + new connection, aliases, type_caster end end @@ -51,45 +53,37 @@ module ActiveRecord end # table_joins is an array of arel joins which might conflict with the aliases we assign here - def initialize(connection, aliases) + def initialize(connection, aliases, type_caster) @aliases = aliases @connection = connection + @type_caster = type_caster end def aliased_table_for(table_name, aliased_name) - table_alias = aliased_name_for(table_name, aliased_name) - - if table_alias == table_name - Arel::Table.new(table_name) - else - Arel::Table.new(table_name).alias(table_alias) - end - end - - def aliased_name_for(table_name, aliased_name) if aliases[table_name].zero? # If it's zero, we can have our table_name aliases[table_name] = 1 - table_name + Arel::Table.new(table_name, type_caster: @type_caster) else # Otherwise, we need to use an alias - aliased_name = connection.table_alias_for(aliased_name) + aliased_name = @connection.table_alias_for(aliased_name) # Update the count aliases[aliased_name] += 1 - if aliases[aliased_name] > 1 + table_alias = if aliases[aliased_name] > 1 "#{truncate(aliased_name)}_#{aliases[aliased_name]}" else aliased_name end + Arel::Table.new(table_name, type_caster: @type_caster).alias(table_alias) end end private def truncate(name) - name.slice(0, connection.table_alias_length - 2) + name.slice(0, @connection.table_alias_length - 2) end end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index f1c36cd047..c7b396f3d4 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -8,12 +8,12 @@ module ActiveRecord # # Association # SingularAssociation - # HasOneAssociation + # HasOneAssociation + ForeignAssociation # HasOneThroughAssociation + ThroughAssociation # BelongsToAssociation # BelongsToPolymorphicAssociation # CollectionAssociation - # HasManyAssociation + # HasManyAssociation + ForeignAssociation # HasManyThroughAssociation + ThroughAssociation class Association #:nodoc: attr_reader :owner, :target, :reflection @@ -121,7 +121,7 @@ module ActiveRecord # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the # through association's scope) def target_scope - AssociationRelation.create(klass, klass.arel_table, self).merge!(klass.all) + AssociationRelation.create(klass, klass.arel_table, klass.predicate_builder, self).merge!(klass.all) end # Loads the \target if needed and returns it. @@ -211,9 +211,12 @@ module ActiveRecord # the kind of the class of the associated objects. Meant to be used as # a sanity check when you are about to assign an associated record. def raise_on_type_mismatch!(record) - unless record.is_a?(reflection.klass) || record.is_a?(reflection.class_name.constantize) - message = "#{reflection.class_name}(##{reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" - raise ActiveRecord::AssociationTypeMismatch, message + unless record.is_a?(reflection.klass) + fresh_class = reflection.class_name.safe_constantize + unless fresh_class && record.is_a?(fresh_class) + message = "#{reflection.class_name}(##{reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" + raise ActiveRecord::AssociationTypeMismatch, message + end end end @@ -248,6 +251,14 @@ module ActiveRecord initialize_attributes(record) end end + + # Returns true if statement cache should be skipped on the association reader. + def skip_statement_cache? + reflection.scope_chain.any?(&:any?) || + scope.eager_loading? || + klass.scope_attributes? || + reflection.source_reflection.active_record.default_scopes.any? + end end end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 519d4d8651..48437a1c9e 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -2,42 +2,30 @@ module ActiveRecord module Associations class AssociationScope #:nodoc: def self.scope(association, connection) - INSTANCE.scope association, connection - end - - class BindSubstitution - def initialize(block) - @block = block - end - - def bind_value(scope, column, value, alias_tracker) - substitute = alias_tracker.connection.substitute_at( - column, scope.bind_values.length) - scope.bind_values += [[column, @block.call(value)]] - substitute - end + INSTANCE.scope(association, connection) end def self.create(&block) - block = block ? block : lambda { |val| val } - new BindSubstitution.new(block) + block ||= lambda { |val| val } + new(block) end - def initialize(bind_substitution) - @bind_substitution = bind_substitution + def initialize(value_transformation) + @value_transformation = value_transformation end INSTANCE = create def scope(association, connection) - klass = association.klass - reflection = association.reflection - scope = klass.unscoped - owner = association.owner - alias_tracker = AliasTracker.empty connection + klass = association.klass + reflection = association.reflection + scope = klass.unscoped + owner = association.owner + alias_tracker = AliasTracker.create connection, association.klass.table_name, klass.type_caster + chain_head, chain_tail = get_chain(reflection, association, alias_tracker) scope.extending! Array(reflection.options[:extend]) - add_constraints(scope, owner, klass, reflection, alias_tracker) + add_constraints(scope, owner, klass, reflection, chain_head, chain_tail) end def join_type @@ -45,137 +33,133 @@ module ActiveRecord end def self.get_bind_values(owner, chain) - bvs = [] - chain.each_with_index do |reflection, i| - if reflection == chain.last - bvs << reflection.join_id_for(owner) - if reflection.type - bvs << owner.class.base_class.name - end - else - if reflection.type - bvs << chain[i + 1].klass.base_class.name - end - end - end - bvs - end + binds = [] + last_reflection = chain.last - private + binds << last_reflection.join_id_for(owner) + if last_reflection.type + binds << owner.class.base_class.name + end - def construct_tables(chain, klass, refl, alias_tracker) - chain.map do |reflection| - alias_tracker.aliased_table_for( - table_name_for(reflection, klass, refl), - table_alias_for(reflection, refl, reflection != refl) - ) + chain.each_cons(2).each do |reflection, next_reflection| + if reflection.type + binds << next_reflection.klass.base_class.name + end end + binds end - def table_alias_for(reflection, refl, join = false) - name = "#{reflection.plural_name}_#{alias_suffix(refl)}" - name << "_join" if join - name - end + protected + + attr_reader :value_transformation + private def join(table, constraint) table.create_join(table, table.create_on(constraint), join_type) end - def column_for(table_name, column_name, alias_tracker) - columns = alias_tracker.connection.schema_cache.columns_hash(table_name) - columns[column_name] - end + def last_chain_scope(scope, table, reflection, owner, association_klass) + join_keys = reflection.join_keys(association_klass) + key = join_keys.key + foreign_key = join_keys.foreign_key + + value = transform_value(owner[foreign_key]) + scope = scope.where(table.name => { key => value }) + + if reflection.type + polymorphic_type = transform_value(owner.class.base_class.name) + scope = scope.where(table.name => { reflection.type => polymorphic_type }) + end - def bind_value(scope, column, value, alias_tracker) - @bind_substitution.bind_value scope, column, value, alias_tracker + scope end - def bind(scope, table_name, column_name, value, tracker) - column = column_for table_name, column_name, tracker - bind_value scope, column, value, tracker + def transform_value(value) + value_transformation.call(value) end - def add_constraints(scope, owner, assoc_klass, refl, tracker) - chain = refl.chain - scope_chain = refl.scope_chain + def next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection) + join_keys = reflection.join_keys(association_klass) + key = join_keys.key + foreign_key = join_keys.foreign_key - tables = construct_tables(chain, assoc_klass, refl, tracker) + constraint = table[key].eq(foreign_table[foreign_key]) - chain.each_with_index do |reflection, i| - table, foreign_table = tables.shift, tables.first + if reflection.type + value = transform_value(next_reflection.klass.base_class.name) + scope = scope.where(table.name => { reflection.type => value }) + end - join_keys = reflection.join_keys(assoc_klass) - key = join_keys.key - foreign_key = join_keys.foreign_key + scope = scope.joins(join(foreign_table, constraint)) + end - if reflection == chain.last - bind_val = bind scope, table.table_name, key.to_s, owner[foreign_key], tracker - scope = scope.where(table[key].eq(bind_val)) + class ReflectionProxy < SimpleDelegator # :nodoc: + attr_accessor :next + attr_reader :alias_name - if reflection.type - value = owner.class.base_class.name - bind_val = bind scope, table.table_name, reflection.type.to_s, value, tracker - scope = scope.where(table[reflection.type].eq(bind_val)) - end - else - constraint = table[key].eq(foreign_table[foreign_key]) + def initialize(reflection, alias_name) + super(reflection) + @alias_name = alias_name + end - if reflection.type - value = chain[i + 1].klass.base_class.name - bind_val = bind scope, table.table_name, reflection.type.to_s, value, tracker - scope = scope.where(table[reflection.type].eq(bind_val)) - end + def all_includes; nil; end + end - scope = scope.joins(join(foreign_table, constraint)) - end + def get_chain(reflection, association, tracker) + name = reflection.name + runtime_reflection = Reflection::RuntimeReflection.new(reflection, association) + previous_reflection = runtime_reflection + reflection.chain.drop(1).each do |refl| + alias_name = tracker.aliased_table_for(refl.table_name, refl.alias_candidate(name)) + proxy = ReflectionProxy.new(refl, alias_name) + previous_reflection.next = proxy + previous_reflection = proxy + end + [runtime_reflection, previous_reflection] + end - is_first_chain = i == 0 - klass = is_first_chain ? assoc_klass : reflection.klass + def add_constraints(scope, owner, association_klass, refl, chain_head, chain_tail) + owner_reflection = chain_tail + table = owner_reflection.alias_name + scope = last_chain_scope(scope, table, owner_reflection, owner, association_klass) + + reflection = chain_head + loop do + break unless reflection + table = reflection.alias_name + + unless reflection == chain_tail + next_reflection = reflection.next + foreign_table = next_reflection.alias_name + scope = next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection) + end # Exclude the scope of the association itself, because that # was already merged in the #scope method. - scope_chain[i].each do |scope_chain_item| - item = eval_scope(klass, scope_chain_item, owner) + reflection.constraints.each do |scope_chain_item| + item = eval_scope(reflection.klass, scope_chain_item, owner) if scope_chain_item == refl.scope - scope.merge! item.except(:where, :includes, :bind) + scope.merge! item.except(:where, :includes) end - if is_first_chain + reflection.all_includes do scope.includes! item.includes_values end - scope.where_values += item.where_values - scope.bind_values += item.bind_values + scope.unscope!(*item.unscope_values) + scope.where_clause += item.where_clause scope.order_values |= item.order_values end + + reflection = reflection.next end scope end - def alias_suffix(refl) - refl.name - end - - def table_name_for(reflection, klass, refl) - if reflection == refl - # If this is a polymorphic belongs_to, we want to get the klass from the - # association because it depends on the polymorphic_type attribute of - # the owner - klass.table_name - else - reflection.table_name - end - end - def eval_scope(klass, scope, owner) - if scope.is_a?(Relation) - scope - else - klass.unscoped.instance_exec(owner, &scope) - end + klass.unscoped.instance_exec(owner, &scope) end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 81fdd681de..41698c5360 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -10,7 +10,7 @@ module ActiveRecord def replace(record) if record raise_on_type_mismatch!(record) - update_counters(record) + update_counters_on_replace(record) replace_keys(record) set_inverse_instance(record) @updated = true @@ -32,52 +32,47 @@ module ActiveRecord end def decrement_counters # :nodoc: - with_cache_name { |name| decrement_counter name } + update_counters(-1) end def increment_counters # :nodoc: - with_cache_name { |name| increment_counter name } + update_counters(1) end private - def find_target? - !loaded? && foreign_key_present? && klass - end - - def with_cache_name - counter_cache_name = reflection.counter_cache_column - return unless counter_cache_name && owner.persisted? - yield counter_cache_name + def update_counters(by) + if require_counter_update? && foreign_key_present? + if target && !stale_target? + target.increment!(reflection.counter_cache_column, by) + else + klass.update_counters(target_id, reflection.counter_cache_column => by) + end + end end - def update_counters(record) - with_cache_name do |name| - return unless different_target? record - record.class.increment_counter(name, record.id) - decrement_counter name - end + def find_target? + !loaded? && foreign_key_present? && klass end - def decrement_counter(counter_cache_name) - if foreign_key_present? - klass.decrement_counter(counter_cache_name, target_id) - end + def require_counter_update? + reflection.counter_cache_column && owner.persisted? end - def increment_counter(counter_cache_name) - if foreign_key_present? - klass.increment_counter(counter_cache_name, target_id) + def update_counters_on_replace(record) + if require_counter_update? && different_target?(record) + record.increment!(reflection.counter_cache_column) + decrement_counters end end # Checks whether record is different to the current target, without loading it def different_target?(record) - record.id != owner[reflection.foreign_key] + record.id != owner._read_attribute(reflection.foreign_key) end def replace_keys(record) - owner[reflection.foreign_key] = record[reflection.association_primary_key(record.class)] + owner[reflection.foreign_key] = record._read_attribute(reflection.association_primary_key(record.class)) end def remove_keys @@ -85,7 +80,7 @@ module ActiveRecord end def foreign_key_present? - owner[reflection.foreign_key] + owner._read_attribute(reflection.foreign_key) end # NOTE - for now, we're only supporting inverse setting from belongs_to back onto @@ -99,12 +94,13 @@ module ActiveRecord if options[:primary_key] owner.send(reflection.name).try(:id) else - owner[reflection.foreign_key] + owner._read_attribute(reflection.foreign_key) end end def stale_state - owner[reflection.foreign_key] && owner[reflection.foreign_key].to_s + result = owner._read_attribute(reflection.foreign_key) { |n| owner.send(:missing_attribute, n, caller) } + result && result.to_s end end end diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 947d61ee7b..d0534056d9 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -1,5 +1,3 @@ -require 'active_support/core_ext/module/attribute_accessors' - # This is the parent Association class which defines the variables # used by all associations. # @@ -11,19 +9,14 @@ require 'active_support/core_ext/module/attribute_accessors' # - CollectionAssociation # - HasManyAssociation -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class Association #:nodoc: class << self attr_accessor :extensions - # TODO: This class accessor is needed to make activerecord-deprecated_finders work. - # We can move it to a constant in 5.0. - attr_accessor :valid_options end self.extensions = [] - self.valid_options = [:class_name, :class, :foreign_key, :validate] - - attr_reader :name, :scope, :options + VALID_OPTIONS = [:class_name, :anonymous_class, :foreign_key, :validate] # :nodoc: def self.build(model, name, scope, options, &block) if model.dangerous_attribute_method?(name) @@ -32,57 +25,60 @@ module ActiveRecord::Associations::Builder "Please choose a different association name." end - builder = create_builder model, name, scope, options, &block - reflection = builder.build(model) + extension = define_extensions model, name, &block + reflection = create_reflection model, name, scope, options, extension define_accessors model, reflection define_callbacks model, reflection define_validations model, reflection - builder.define_extensions model reflection end - def self.create_builder(model, name, scope, options, &block) + def self.create_reflection(model, name, scope, options, extension = nil) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) - new(model, name, scope, options, &block) - end - - def initialize(model, name, scope, options) - # TODO: Move this to create_builder as soon we drop support to activerecord-deprecated_finders. if scope.is_a?(Hash) options = scope scope = nil end - # TODO: Remove this model argument as soon we drop support to activerecord-deprecated_finders. - @name = name - @scope = scope - @options = options + validate_options(options) - validate_options + scope = build_scope(scope, extension) + + ActiveRecord::Reflection.create(macro, name, scope, options, model) + end + + def self.build_scope(scope, extension) + new_scope = scope if scope && scope.arity == 0 - @scope = proc { instance_exec(&scope) } + new_scope = proc { instance_exec(&scope) } + end + + if extension + new_scope = wrap_scope new_scope, extension end + + new_scope end - def build(model) - ActiveRecord::Reflection.create(macro, name, scope, options, model) + def self.wrap_scope(scope, extension) + scope end - def macro + def self.macro raise NotImplementedError end - def valid_options - Association.valid_options + Association.extensions.flat_map(&:valid_options) + def self.valid_options(options) + VALID_OPTIONS + Association.extensions.flat_map(&:valid_options) end - def validate_options - options.assert_valid_keys(valid_options) + def self.validate_options(options) + options.assert_valid_keys(valid_options(options)) end - def define_extensions(model) + def self.define_extensions(model, name) end def self.define_callbacks(model, reflection) @@ -133,8 +129,6 @@ module ActiveRecord::Associations::Builder raise NotImplementedError end - private - def self.check_dependent_options(dependent) unless valid_dependent_options.include? dependent raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{dependent}" diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 954ea3878a..dae468ba54 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,11 +1,11 @@ -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class BelongsTo < SingularAssociation #:nodoc: - def macro + def self.macro :belongs_to end - def valid_options - super + [:foreign_type, :polymorphic, :touch, :counter_cache] + def self.valid_options(options) + super + [:foreign_type, :polymorphic, :touch, :counter_cache, :optional] end def self.valid_dependent_options @@ -23,8 +23,6 @@ module ActiveRecord::Associations::Builder add_counter_cache_methods mixin end - private - def self.add_counter_cache_methods(mixin) return if mixin.method_defined? :belongs_to_counter_cache_after_update @@ -35,16 +33,24 @@ module ActiveRecord::Associations::Builder if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif attribute_changed?(foreign_key) && !new_record? && reflection.constructable? - model = reflection.klass + elsif attribute_changed?(foreign_key) && !new_record? + if reflection.polymorphic? + model = attribute(reflection.foreign_type).try(:constantize) + model_was = attribute_was(reflection.foreign_type).try(:constantize) + else + model = reflection.klass + model_was = reflection.klass + end + foreign_key_was = attribute_was foreign_key foreign_key = attribute foreign_key if foreign_key && model.respond_to?(:increment_counter) model.increment_counter(cache_column, foreign_key) end - if foreign_key_was && model.respond_to?(:decrement_counter) - model.decrement_counter(cache_column, foreign_key_was) + + if foreign_key_was && model_was.respond_to?(:decrement_counter) + model_was.decrement_counter(cache_column, foreign_key_was) end end end @@ -62,7 +68,7 @@ module ActiveRecord::Associations::Builder klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly) end - def self.touch_record(o, foreign_key, name, touch) # :nodoc: + def self.touch_record(o, foreign_key, name, touch, touch_method) # :nodoc: old_foreign_id = o.changed_attributes[foreign_key] if old_foreign_id @@ -77,9 +83,9 @@ module ActiveRecord::Associations::Builder if old_record if touch != true - old_record.touch touch + old_record.send(touch_method, touch) else - old_record.touch + old_record.send(touch_method) end end end @@ -87,9 +93,9 @@ module ActiveRecord::Associations::Builder record = o.send name if record && record.persisted? if touch != true - record.touch touch + record.send(touch_method, touch) else - record.touch + record.send(touch_method) end end end @@ -100,7 +106,8 @@ module ActiveRecord::Associations::Builder touch = reflection.options[:touch] callback = lambda { |record| - BelongsTo.touch_record(record, foreign_key, n, touch) + touch_method = touching_delayed_records? ? :touch : :touch_later + BelongsTo.touch_record(record, foreign_key, n, touch, touch_method) } model.after_save callback, if: :changed? @@ -112,5 +119,23 @@ module ActiveRecord::Associations::Builder name = reflection.name model.after_destroy lambda { |o| o.association(name).handle_dependency } end + + def self.define_validations(model, reflection) + if reflection.options.key?(:required) + reflection.options[:optional] = !reflection.options.delete(:required) + end + + if reflection.options[:optional].nil? + required = model.belongs_to_required_by_default + else + required = !reflection.options[:optional] + end + + super + + if required + model.validates_presence_of reflection.name, message: :required + end + end end end diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index bc15a49996..56a8dc4e18 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -2,27 +2,16 @@ require 'active_record/associations' -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class CollectionAssociation < Association #:nodoc: CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] - def valid_options + def self.valid_options(options) super + [:table_name, :before_add, :after_add, :before_remove, :after_remove, :extend] end - attr_reader :block_extension - - def initialize(model, name, scope, options) - super - @mod = nil - if block_given? - @mod = Module.new(&Proc.new) - @scope = wrap_scope @scope, @mod - end - end - def self.define_callbacks(model, reflection) super name = reflection.name @@ -32,10 +21,11 @@ module ActiveRecord::Associations::Builder } end - def define_extensions(model) - if @mod + def self.define_extensions(model, name) + if block_given? extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" - model.parent.const_set(extension_module_name, @mod) + extension = Module.new(&Proc.new) + model.parent.const_set(extension_module_name, extension) end end @@ -78,9 +68,7 @@ module ActiveRecord::Associations::Builder CODE end - private - - def wrap_scope(scope, mod) + def self.wrap_scope(scope, mod) if scope proc { |owner| instance_exec(owner, &scope).extending(mod) } else diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 34a555dfd4..a5c9f1666e 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -1,9 +1,9 @@ -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class HasAndBelongsToMany # :nodoc: - class JoinTableResolver + class JoinTableResolver # :nodoc: KnownTable = Struct.new :join_table - class KnownClass + class KnownClass # :nodoc: def initialize(lhs_class, rhs_class_name) @lhs_class = lhs_class @rhs_class_name = rhs_class_name @@ -11,7 +11,7 @@ module ActiveRecord::Associations::Builder end def join_table - @join_table ||= [@lhs_class.table_name, klass.table_name].sort.join("\0").gsub(/^(.*[._])(.+)\0\1(.+)/, '\1\2_\3').gsub("\0", "_") + @join_table ||= [@lhs_class.table_name, klass.table_name].sort.join("\0").gsub(/^(.*[._])(.+)\0\1(.+)/, '\1\2_\3').tr("\0", "_") end private @@ -46,7 +46,7 @@ module ActiveRecord::Associations::Builder join_model = Class.new(ActiveRecord::Base) { class << self; - attr_accessor :class_resolver + attr_accessor :left_model attr_accessor :name attr_accessor :table_name_resolver attr_accessor :left_reflection @@ -58,7 +58,7 @@ module ActiveRecord::Associations::Builder end def self.compute_type(class_name) - class_resolver.compute_type class_name + left_model.compute_type class_name end def self.add_left_association(name, options) @@ -72,33 +72,37 @@ module ActiveRecord::Associations::Builder self.right_reflection = _reflect_on_association(rhs_name) end + def self.retrieve_connection + left_model.retrieve_connection + end + } join_model.name = "HABTM_#{association_name.to_s.camelize}" join_model.table_name_resolver = habtm - join_model.class_resolver = lhs_model + join_model.left_model = lhs_model - join_model.add_left_association :left_side, class: lhs_model + join_model.add_left_association :left_side, anonymous_class: lhs_model join_model.add_right_association association_name, belongs_to_options(options) join_model end def middle_reflection(join_model) middle_name = [lhs_model.name.downcase.pluralize, - association_name].join('_').gsub(/::/, '_').to_sym + association_name].join('_'.freeze).gsub('::'.freeze, '_'.freeze).to_sym middle_options = middle_options join_model - hm_builder = HasMany.create_builder(lhs_model, - middle_name, - nil, - middle_options) - hm_builder.build lhs_model + + HasMany.create_reflection(lhs_model, + middle_name, + nil, + middle_options) end private def middle_options(join_model) middle_options = {} - middle_options[:class] = join_model + middle_options[:class_name] = "#{lhs_model.name}::#{join_model.name}" middle_options[:source] = join_model.left_reflection.name if options.key? :foreign_key middle_options[:foreign_key] = options[:foreign_key] @@ -110,7 +114,7 @@ module ActiveRecord::Associations::Builder rhs_options = {} if options.key? :class_name - rhs_options[:foreign_key] = options[:class_name].foreign_key + rhs_options[:foreign_key] = options[:class_name].to_s.foreign_key rhs_options[:class_name] = options[:class_name] end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 4c8c826f76..7864d4c536 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,11 +1,11 @@ -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class HasMany < CollectionAssociation #:nodoc: - def macro + def self.macro :has_many end - def valid_options - super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache, :join_table] + def self.valid_options(options) + super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache, :join_table, :foreign_type, :index_errors] end def self.valid_dependent_options diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index c194c8ae9a..9d64ae877b 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,11 +1,11 @@ -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class HasOne < SingularAssociation #:nodoc: - def macro + def self.macro :has_one end - def valid_options - valid = super + [:as] + def self.valid_options(options) + valid = super + [:as, :foreign_type] valid += [:through, :source, :source_type] if options[:through] valid end @@ -14,10 +14,15 @@ module ActiveRecord::Associations::Builder [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception] end - private - def self.add_destroy_callbacks(model, reflection) super unless reflection.options[:through] end + + def self.define_validations(model, reflection) + super + if reflection.options[:required] + model.validates_presence_of reflection.name, message: :required + end + end end end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index 6e6dd7204c..58a9c8ff24 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -1,8 +1,8 @@ # This class is inherited by the has_one and belongs_to association classes -module ActiveRecord::Associations::Builder +module ActiveRecord::Associations::Builder # :nodoc: class SingularAssociation < Association #:nodoc: - def valid_options + def self.valid_options(options) super + [:dependent, :primary_key, :inverse_of, :required] end @@ -27,12 +27,5 @@ module ActiveRecord::Associations::Builder end CODE end - - def self.define_validations(model, reflection) - super - if reflection.options[:required] - model.validates_presence_of reflection.name - end - end end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 065a2cff01..f32dddb8f0 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -28,12 +28,24 @@ module ActiveRecord # Implements the reader method, e.g. foo.items for Foo.has_many :items def reader(force_reload = false) if force_reload + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing an argument to force an association to reload is now + deprecated and will be removed in Rails 5.1. Please call `reload` + on the result collection proxy instead. + MSG + klass.uncached { reload } elsif stale_target? reload end - @proxy ||= CollectionProxy.create(klass, self) + if null_scope? + # Cache the proxy separately before the owner has an id + # or else a post-save proxy will still lack the id + @null_proxy ||= CollectionProxy.create(klass, self) + else + @proxy ||= CollectionProxy.create(klass, self) + end end # Implements the writer method, e.g. foo.items= for Foo.has_many :items @@ -48,17 +60,19 @@ module ActiveRecord record.send(reflection.association_primary_key) end else - column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}" - scope.pluck(column) + @association_ids ||= ( + column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}" + scope.pluck(column) + ) end end # Implements the ids writer method, e.g. foo.item_ids= for Foo.has_many :items def ids_writer(ids) pk_type = reflection.primary_key_type - ids = Array(ids).reject { |id| id.blank? } - ids.map! { |i| pk_type.type_cast_from_user(i) } - replace(klass.find(ids).index_by { |r| r.id }.values_at(*ids)) + ids = Array(ids).reject(&:blank?) + ids.map! { |i| pk_type.cast(i) } + replace(klass.find(ids).index_by(&:id).values_at(*ids)) end def reset @@ -123,6 +137,16 @@ module ActiveRecord first_nth_or_last(:last, *args) end + def take(n = nil) + if loaded? + n ? target.take(n) : target.first + else + scope.take(n).tap do |record| + set_inverse_instance record if record.is_a? ActiveRecord::Base + end + end + end + def build(attributes = {}, &block) if attributes.is_a?(Array) attributes.collect { |attr| build(attr, &block) } @@ -145,6 +169,7 @@ module ActiveRecord # be chained. Since << flattens its argument list and inserts each record, # +push+ and +concat+ behave identically. def concat(*records) + records = records.flatten if owner.new_record? load_target concat_records(records) @@ -169,8 +194,8 @@ module ActiveRecord end # Removes all records from the association without calling callbacks - # on the associated records. It honors the `:dependent` option. However - # if the `:dependent` value is `:destroy` then in that case the `:delete_all` + # on the associated records. It honors the +:dependent+ option. However + # if the +:dependent+ value is +:destroy+ then in that case the +:delete_all+ # deletion strategy for the association is applied. # # You can force a particular deletion strategy by passing a parameter. @@ -212,11 +237,7 @@ module ActiveRecord # Count all records using SQL. Construct options and pass them with # scope to the target class's +count+. - def count(column_name = nil, count_options = {}) - # TODO: Remove count_options argument as soon we remove support to - # activerecord-deprecated_finders. - column_name, count_options = nil, column_name if column_name.is_a?(Hash) - + def count(column_name = nil) relation = scope if association_scope.distinct_value # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. @@ -283,7 +304,7 @@ module ActiveRecord elsif !loaded? && !association_scope.group_values.empty? load_target.size elsif !loaded? && !association_scope.distinct_value && target.is_a?(Array) - unsaved_records = target.select { |r| r.new_record? } + unsaved_records = target.select(&:new_record?) unsaved_records.size + count_records else count_records @@ -316,7 +337,8 @@ module ActiveRecord end # Returns true if the collections is not empty. - # Equivalent to +!collection.empty?+. + # If block given, loads all records and checks for one or more matches. + # Otherwise, equivalent to +!collection.empty?+. def any? if block_given? load_target.any? { |*block_args| yield(*block_args) } @@ -326,7 +348,8 @@ module ActiveRecord end # Returns true if the collection has more than 1 record. - # Equivalent to +collection.size > 1+. + # If block given, loads all records and checks for two or more matches. + # Otherwise, equivalent to +collection.size > 1+. def many? if block_given? load_target.many? { |*block_args| yield(*block_args) } @@ -352,8 +375,11 @@ module ActiveRecord if owner.new_record? replace_records(other_array, original_target) else + replace_common_records_in_memory(other_array, original_target) if other_array != original_target transaction { replace_records(other_array, original_target) } + else + other_array end end end @@ -379,11 +405,18 @@ module ActiveRecord target end - def add_to_target(record, skip_callbacks = false) + def add_to_target(record, skip_callbacks = false, &block) + if association_scope.distinct_value + index = @target.index(record) + end + replace_on_target(record, index, skip_callbacks, &block) + end + + def replace_on_target(record, index, skip_callbacks) callback(:before_add, record) unless skip_callbacks yield(record) if block_given? - if association_scope.distinct_value && index = @target.index(record) + if index @target[index] = record else @target << record @@ -407,7 +440,7 @@ module ActiveRecord private def get_records - return scope.to_a if reflection.scope_chain.any?(&:any?) + return scope.to_a if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do @@ -486,7 +519,7 @@ module ActiveRecord def delete_or_destroy(records, method) records = records.flatten records.each { |record| raise_on_type_mismatch!(record) } - existing_records = records.reject { |r| r.new_record? } + existing_records = records.reject(&:new_record?) if existing_records.empty? remove_records(existing_records, records, method) @@ -522,10 +555,18 @@ module ActiveRecord target end + def replace_common_records_in_memory(new_target, original_target) + common_records = new_target & original_target + common_records.each do |record| + skip_callbacks = true + replace_on_target(record, @target.index(record), skip_callbacks) + end + end + def concat_records(records, should_raise = false) result = true - records.flatten.each do |record| + records.each do |record| raise_on_type_mismatch!(record) add_to_target(record) do |rec| result &&= insert_record(rec, true, should_raise) unless owner.new_record? @@ -569,8 +610,8 @@ module ActiveRecord if reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) assoc = owner.association(reflection.through_reflection.name) assoc.reader.any? { |source| - target = source.send(reflection.source_reflection.name) - target.respond_to?(:include?) ? target.include?(record) : target == record + target_reflection = source.send(reflection.source_reflection.name) + target_reflection.respond_to?(:include?) ? target_reflection.include?(record) : target_reflection == record } || target.include?(record) else target.include?(record) @@ -581,7 +622,7 @@ module ActiveRecord # specified, then #find scans the entire collection. def find_by_scan(*args) expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.map{ |arg| arg.to_s }.uniq + ids = args.flatten.compact.map(&:to_s).uniq if ids.size == 1 id = ids.first diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 84c8cfe72b..fe693cfbb6 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -29,10 +29,11 @@ module ActiveRecord # instantiation of the actual post records. class CollectionProxy < Relation delegate(*(ActiveRecord::Calculations.public_instance_methods - [:count]), to: :scope) + delegate :find_nth, to: :scope def initialize(klass, association) #:nodoc: @association = association - super klass, klass.arel_table + super klass, klass.arel_table, klass.predicate_builder merge! association.scope(nullify: false) end @@ -111,7 +112,7 @@ module ActiveRecord end # Finds an object in the collection responding to the +id+. Uses the same - # rules as <tt>ActiveRecord::Base.find</tt>. Returns <tt>ActiveRecord::RecordNotFound</tt> + # rules as ActiveRecord::Base.find. Returns ActiveRecord::RecordNotFound # error if the object cannot be found. # # class Person < ActiveRecord::Base @@ -126,7 +127,7 @@ module ActiveRecord # # ] # # person.pets.find(1) # => #<Pet id: 1, name: "Fancy-Fancy", person_id: 1> - # person.pets.find(4) # => ActiveRecord::RecordNotFound: Couldn't find Pet with id=4 + # person.pets.find(4) # => ActiveRecord::RecordNotFound: Couldn't find Pet with 'id'=4 # # person.pets.find(2) { |pet| pet.name.downcase! } # # => #<Pet id: 2, name: "fancy-fancy", person_id: 1> @@ -170,27 +171,27 @@ module ActiveRecord @association.first(*args) end - # Same as +first+ except returns only the second record. + # Same as #first except returns only the second record. def second(*args) @association.second(*args) end - # Same as +first+ except returns only the third record. + # Same as #first except returns only the third record. def third(*args) @association.third(*args) end - # Same as +first+ except returns only the fourth record. + # Same as #first except returns only the fourth record. def fourth(*args) @association.fourth(*args) end - # Same as +first+ except returns only the fifth record. + # Same as #first except returns only the fifth record. def fifth(*args) @association.fifth(*args) end - # Same as +first+ except returns only the forty second record. + # Same as #first except returns only the forty second record. # Also known as accessing "the reddit". def forty_two(*args) @association.forty_two(*args) @@ -226,6 +227,35 @@ module ActiveRecord @association.last(*args) end + # Gives a record (or N records if a parameter is supplied) from the collection + # using the same rules as <tt>ActiveRecord::Base.take</tt>. + # + # class Person < ActiveRecord::Base + # has_many :pets + # end + # + # person.pets + # # => [ + # # #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>, + # # #<Pet id: 2, name: "Spook", person_id: 1>, + # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> + # # ] + # + # person.pets.take # => #<Pet id: 1, name: "Fancy-Fancy", person_id: 1> + # + # person.pets.take(2) + # # => [ + # # #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>, + # # #<Pet id: 2, name: "Spook", person_id: 1> + # # ] + # + # another_person_without.pets # => [] + # another_person_without.pets.take # => nil + # another_person_without.pets.take(2) # => [] + def take(n = nil) + @association.take(n) + end + # Returns a new object of the collection type that has been instantiated # with +attributes+ and linked to this object, but have not yet been saved. # You can pass an array of attributes hashes, this will return an array @@ -285,7 +315,7 @@ module ActiveRecord @association.create(attributes, &block) end - # Like +create+, except that if the record is invalid, raises an exception. + # Like #create, except that if the record is invalid, raises an exception. # # class Person # has_many :pets @@ -302,8 +332,8 @@ module ActiveRecord end # Add one or more records to the collection by setting their foreign keys - # to the association's primary key. Since << flattens its argument list and - # inserts each record, +push+ and +concat+ behave identically. Returns +self+ + # to the association's primary key. Since #<< flattens its argument list and + # inserts each record, +push+ and #concat behave identically. Returns +self+ # so method calls may be chained. # # class Person < ActiveRecord::Base @@ -355,14 +385,15 @@ module ActiveRecord @association.replace(other_array) end - # Deletes all the records from the collection. For +has_many+ associations, - # the deletion is done according to the strategy specified by the <tt>:dependent</tt> - # option. + # Deletes all the records from the collection according to the strategy + # specified by the +:dependent+ option. If no +:dependent+ option is given, + # then it will follow the default strategy. # - # If no <tt>:dependent</tt> option is given, then it will follow the - # default strategy. The default strategy is <tt>:nullify</tt>. This - # sets the foreign keys to <tt>NULL</tt>. For, +has_many+ <tt>:through</tt>, - # the default strategy is +delete_all+. + # For <tt>has_many :through</tt> associations, the default deletion strategy is + # +:delete_all+. + # + # For +has_many+ associations, the default deletion strategy is +:nullify+. + # This sets the foreign keys to +NULL+. # # class Person < ActiveRecord::Base # has_many :pets # dependent: :nullify option by default @@ -393,9 +424,9 @@ module ActiveRecord # # #<Pet id: 3, name: "Choo-Choo", person_id: nil> # # ] # - # If it is set to <tt>:destroy</tt> all the objects from the collection - # are removed by calling their +destroy+ method. See +destroy+ for more - # information. + # Both +has_many+ and <tt>has_many :through</tt> dependencies default to the + # +:delete_all+ strategy if the +:dependent+ option is set to +:destroy+. + # Records are not instantiated and callbacks will not be fired. # # class Person < ActiveRecord::Base # has_many :pets, dependent: :destroy @@ -410,14 +441,9 @@ module ActiveRecord # # ] # # person.pets.delete_all - # # => [ - # # #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>, - # # #<Pet id: 2, name: "Spook", person_id: 1>, - # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> - # # ] # # Pet.find(1, 2, 3) - # # => ActiveRecord::RecordNotFound + # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) # # If it is set to <tt>:delete_all</tt>, all the objects are deleted # *without* calling their +destroy+ method. @@ -437,14 +463,15 @@ module ActiveRecord # person.pets.delete_all # # Pet.find(1, 2, 3) - # # => ActiveRecord::RecordNotFound + # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) def delete_all(dependent = nil) @association.delete_all(dependent) end # Deletes the records of the collection directly from the database - # ignoring the +:dependent+ option. It invokes +before_remove+, - # +after_remove+ , +before_destroy+ and +after_destroy+ callbacks. + # ignoring the +:dependent+ option. Records are instantiated and it + # invokes +before_remove+, +after_remove+ , +before_destroy+ and + # +after_destroy+ callbacks. # # class Person < ActiveRecord::Base # has_many :pets @@ -468,15 +495,16 @@ module ActiveRecord @association.destroy_all end - # Deletes the +records+ supplied and removes them from the collection. For - # +has_many+ associations, the deletion is done according to the strategy - # specified by the <tt>:dependent</tt> option. Returns an array with the + # Deletes the +records+ supplied from the collection according to the strategy + # specified by the +:dependent+ option. If no +:dependent+ option is given, + # then it will follow the default strategy. Returns an array with the # deleted records. # - # If no <tt>:dependent</tt> option is given, then it will follow the default - # strategy. The default strategy is <tt>:nullify</tt>. This sets the foreign - # keys to <tt>NULL</tt>. For, +has_many+ <tt>:through</tt>, the default - # strategy is +delete_all+. + # For <tt>has_many :through</tt> associations, the default deletion strategy is + # +:delete_all+. + # + # For +has_many+ associations, the default deletion strategy is +:nullify+. + # This sets the foreign keys to +NULL+. # # class Person < ActiveRecord::Base # has_many :pets # dependent: :nullify option by default @@ -529,7 +557,7 @@ module ActiveRecord # # => [#<Pet id: 2, name: "Spook", person_id: 1>] # # Pet.find(1, 3) - # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with IDs (1, 3) + # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 3) # # If it is set to <tt>:delete_all</tt>, all the +records+ are deleted # *without* calling their +destroy+ method. @@ -557,7 +585,7 @@ module ActiveRecord # # ] # # Pet.find(1) - # # => ActiveRecord::RecordNotFound: Couldn't find Pet with id=1 + # # => ActiveRecord::RecordNotFound: Couldn't find Pet with 'id'=1 # # You can pass +Fixnum+ or +String+ values, it finds the records # responding to the +id+ and executes delete on them. @@ -621,7 +649,7 @@ module ActiveRecord # person.pets.size # => 0 # person.pets # => [] # - # Pet.find(1, 2, 3) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with IDs (1, 2, 3) + # Pet.find(1, 2, 3) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) # # You can pass +Fixnum+ or +String+ values, it finds the records # responding to the +id+ and then deletes them from the database. @@ -653,7 +681,7 @@ module ActiveRecord # person.pets.size # => 0 # person.pets # => [] # - # Pet.find(4, 5, 6) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with IDs (4, 5, 6) + # Pet.find(4, 5, 6) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (4, 5, 6) def destroy(*records) @association.destroy(*records) end @@ -690,10 +718,8 @@ module ActiveRecord # # #<Pet id: 2, name: "Spook", person_id: 1>, # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> # # ] - def count(column_name = nil, options = {}) - # TODO: Remove options argument as soon we remove support to - # activerecord-deprecated_finders. - @association.count(column_name, options) + def count(column_name = nil) + @association.count(column_name) end # Returns the size of the collection. If the collection hasn't been loaded, @@ -780,10 +806,10 @@ module ActiveRecord # person.pets.any? # => false # # person.pets << Pet.new(name: 'Snoop') - # person.pets.count # => 0 + # person.pets.count # => 1 # person.pets.any? # => true # - # You can also pass a block to define criteria. The behavior + # You can also pass a +block+ to define criteria. The behavior # is the same, it returns true if the collection based on the # criteria is not empty. # @@ -817,7 +843,7 @@ module ActiveRecord # person.pets.count # => 2 # person.pets.many? # => true # - # You can also pass a block to define criteria. The + # You can also pass a +block+ to define criteria. The # behavior is the same, it returns true if the collection # based on the criteria has more than one record. # @@ -841,7 +867,7 @@ module ActiveRecord @association.many?(&block) end - # Returns +true+ if the given object is present in the collection. + # Returns +true+ if the given +record+ is present in the collection. # # class Person < ActiveRecord::Base # has_many :pets @@ -855,7 +881,7 @@ module ActiveRecord !!@association.include?(record) end - def arel + def arel #:nodoc: scope.arel end @@ -879,7 +905,7 @@ module ActiveRecord # Equivalent to <tt>Array#==</tt>. Returns +true+ if the two arrays # contain the same number of elements and if each element is equal - # to the corresponding element in the other array, otherwise returns + # to the corresponding element in the +other+ array, otherwise returns # +false+. # # class Person < ActiveRecord::Base @@ -970,12 +996,15 @@ module ActiveRecord alias_method :append, :<< def prepend(*args) - raise NoMethodError, "prepend on association is not defined. Please use << or append" + raise NoMethodError, "prepend on association is not defined. Please use <<, push or append" end # Equivalent to +delete_all+. The difference is that returns +self+, instead # of an array with the deleted objects, so methods can be chained. See # +delete_all+ for more information. + # Note that because +delete_all+ removes records by directly + # running an SQL query into the database, the +updated_at+ column of + # the object is not changed. def clear delete_all self diff --git a/activerecord/lib/active_record/associations/foreign_association.rb b/activerecord/lib/active_record/associations/foreign_association.rb new file mode 100644 index 0000000000..3ceec0ee46 --- /dev/null +++ b/activerecord/lib/active_record/associations/foreign_association.rb @@ -0,0 +1,11 @@ +module ActiveRecord::Associations + module ForeignAssociation # :nodoc: + def foreign_key_present? + if reflection.klass.primary_key + owner.attribute_present?(reflection.active_record_primary_key) + else + false + end + end + end +end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 2a97d0ed31..a9f6aaafef 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -6,6 +6,7 @@ module ActiveRecord # If the association has a <tt>:through</tt> option further specialization # is provided by its child HasManyThroughAssociation. class HasManyAssociation < CollectionAssociation #:nodoc: + include ForeignAssociation def handle_dependency case options[:dependent] @@ -14,9 +15,16 @@ module ActiveRecord when :restrict_with_error unless empty? - record = klass.human_attribute_name(reflection.name).downcase - owner.errors.add(:base, :"restrict_dependent_destroy.many", record: record) - false + record = owner.class.human_attribute_name(reflection.name).downcase + message = owner.errors.generate_message(:base, :'restrict_dependent_destroy.many', record: record, raise: true) rescue nil + if message + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + The error key `:'restrict_dependent_destroy.many'` has been deprecated and will be removed in Rails 5.1. + Please use `:'restrict_dependent_destroy.has_many'` instead. + MESSAGE + end + owner.errors.add(:base, message || :'restrict_dependent_destroy.has_many', record: record) + throw(:abort) end else @@ -42,7 +50,7 @@ module ActiveRecord end def empty? - if has_cached_counter? + if reflection.has_cached_counter? size.zero? else super @@ -65,8 +73,8 @@ module ActiveRecord # If the collection is empty the target is set to an empty array and # the loaded flag is set to true as well. def count_records - count = if has_cached_counter? - owner.read_attribute cached_counter_attribute_name + count = if reflection.has_cached_counter? + owner._read_attribute reflection.counter_cache_column else scope.count end @@ -79,56 +87,20 @@ module ActiveRecord [association_scope.limit_value, count].compact.min end - def has_cached_counter?(reflection = reflection()) - owner.attribute_present?(cached_counter_attribute_name(reflection)) - end - - def cached_counter_attribute_name(reflection = reflection()) - options[:counter_cache] || "#{reflection.name}_count" - end - def update_counter(difference, reflection = reflection()) - update_counter_in_database(difference, reflection) - update_counter_in_memory(difference, reflection) - end - - def update_counter_in_database(difference, reflection = reflection()) - if has_cached_counter?(reflection) - counter = cached_counter_attribute_name(reflection) - owner.class.update_counters(owner.id, counter => difference) + if reflection.has_cached_counter? + owner.increment!(reflection.counter_cache_column, difference) end end def update_counter_in_memory(difference, reflection = reflection()) - if has_cached_counter?(reflection) - counter = cached_counter_attribute_name(reflection) - owner[counter] += difference - owner.changed_attributes.delete(counter) # eww + if reflection.counter_must_be_updated_by_has_many? + counter = reflection.counter_cache_column + owner.increment(counter, difference) + owner.send(:clear_attribute_change, counter) # eww end end - # This shit is nasty. We need to avoid the following situation: - # - # * An associated record is deleted via record.destroy - # * Hence the callbacks run, and they find a belongs_to on the record with a - # :counter_cache options which points back at our owner. So they update the - # counter cache. - # * In which case, we must make sure to *not* update the counter cache, or else - # it will be decremented twice. - # - # Hence this method. - def inverse_updates_counter_cache?(reflection = reflection()) - counter_name = cached_counter_attribute_name(reflection) - inverse_updates_counter_named?(counter_name, reflection) - end - - def inverse_updates_counter_named?(counter_name, reflection = reflection()) - reflection.klass._reflections.values.any? { |inverse_reflection| - :belongs_to == inverse_reflection.macro && - inverse_reflection.counter_cache_column == counter_name - } - end - def delete_count(method, scope) if method == :delete_all scope.delete_all @@ -146,21 +118,13 @@ module ActiveRecord def delete_records(records, method) if method == :destroy records.each(&:destroy!) - update_counter(-records.length) unless inverse_updates_counter_cache? + update_counter(-records.length) unless reflection.inverse_updates_counter_cache? else scope = self.scope.where(reflection.klass.primary_key => records) update_counter(-delete_count(method, scope)) end end - def foreign_key_present? - if reflection.klass.primary_key - owner.attribute_present?(reflection.association_primary_key) - else - false - end - end - def concat_records(records, *) update_counter_if_success(super, records.length) end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 44c4436e95..deb0f8c9f5 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -11,21 +11,6 @@ module ActiveRecord @through_association = nil end - # Returns the size of the collection by executing a SELECT COUNT(*) query - # if the collection hasn't been loaded, and by calling collection.size if - # it has. If the collection will likely have a size greater than zero, - # and if fetching the collection will be needed afterwards, one less - # SELECT query will be generated by using #length instead. - def size - if has_cached_counter? - owner.read_attribute cached_counter_attribute_name(reflection) - elsif loaded? - target.size - else - super - end - end - def concat(*records) unless owner.new_record? records.flatten.each do |record| @@ -53,24 +38,14 @@ module ActiveRecord def insert_record(record, validate = true, raise = false) ensure_not_nested - if record.new_record? - if raise - record.save!(:validate => validate) - else - return unless record.save(:validate => validate) - end + if raise + record.save!(:validate => validate) + else + return unless record.save(:validate => validate) end save_through_record(record) - if has_cached_counter? && !through_reflection_updates_counter_cache? - ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) - Automatic updating of counter caches on through associations has been - deprecated, and will be removed in Rails 5.0. Instead, please set the - appropriate counter_cache options on the has_many and belongs_to for - your associations to #{through_reflection.name}. - MESSAGE - update_counter_in_database(1) - end + record end @@ -135,7 +110,7 @@ module ActiveRecord def update_through_counter?(method) case method when :destroy - !inverse_updates_counter_cache?(through_reflection) + !through_reflection.inverse_updates_counter_cache? when :nullify false else @@ -158,17 +133,15 @@ module ActiveRecord if scope.klass.primary_key count = scope.destroy_all.length else - scope.to_a.each do |record| - record.run_callbacks :destroy - end + scope.each(&:_run_destroy_callbacks) arel = scope.arel - stmt = Arel::DeleteManager.new arel.engine + stmt = Arel::DeleteManager.new stmt.from scope.klass.arel_table stmt.wheres = arel.constraints - count = scope.klass.connection.delete(stmt, 'SQL', scope.bind_values) + count = scope.klass.connection.delete(stmt, 'SQL', scope.bound_attributes) end when :nullify count = scope.update_all(source_reflection.foreign_key => nil) @@ -185,9 +158,9 @@ module ActiveRecord if through_reflection.collection? && update_through_counter?(method) update_counter(-count, through_reflection) + else + update_counter(-count) end - - update_counter(-count) end def through_records_for(record) @@ -225,11 +198,6 @@ module ActiveRecord def invertible_for?(record) false end - - def through_reflection_updates_counter_cache? - counter_name = cached_counter_attribute_name - inverse_updates_counter_named?(counter_name, through_reflection) - end end end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index e6095d84dc..0fe9b2e81b 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -1,7 +1,8 @@ module ActiveRecord - # = Active Record Belongs To Has One Association + # = Active Record Has One Association module Associations class HasOneAssociation < SingularAssociation #:nodoc: + include ForeignAssociation def handle_dependency case options[:dependent] @@ -10,9 +11,16 @@ module ActiveRecord when :restrict_with_error if load_target - record = klass.human_attribute_name(reflection.name).downcase - owner.errors.add(:base, :"restrict_dependent_destroy.one", record: record) - false + record = owner.class.human_attribute_name(reflection.name).downcase + message = owner.errors.generate_message(:base, :'restrict_dependent_destroy.one', record: record, raise: true) rescue nil + if message + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + The error key `:'restrict_dependent_destroy.one'` has been deprecated and will be removed in Rails 5.1. + Please use `:'restrict_dependent_destroy.has_one'` instead. + MESSAGE + end + owner.errors.add(:base, message || :'restrict_dependent_destroy.has_one', record: record) + throw(:abort) end else @@ -57,7 +65,7 @@ module ActiveRecord when :destroy target.destroy when :nullify - target.update_columns(reflection.foreign_key => nil) + target.update_columns(reflection.foreign_key => nil) if target.persisted? end end end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index ec5c189cd3..0e98a3b3a4 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -20,7 +20,7 @@ module ActiveRecord end def columns - @tables.flat_map { |t| t.column_aliases } + @tables.flat_map(&:column_aliases) end # An array of [column_name, alias] pairs for the table @@ -93,8 +93,7 @@ module ActiveRecord # joins # => [] # def initialize(base, associations, joins) - @alias_tracker = AliasTracker.create(base.connection, joins) - @alias_tracker.aliased_name_for(base.table_name, base.table_name) # Updates the count for base.table_name to 1 + @alias_tracker = AliasTracker.create_with_joins(base.connection, base.table_name, joins, base.type_caster) tree = self.class.make_tree associations @join_root = JoinBase.new base, build(tree, base) @join_root.children.each { |child| construct_tables! @join_root, child } @@ -132,9 +131,9 @@ module ActiveRecord def instantiate(result_set, aliases) primary_key = aliases.column_alias(join_root, join_root.primary_key) - seen = Hash.new { |h,parent_klass| - h[parent_klass] = Hash.new { |i,parent_id| - i[parent_id] = Hash.new { |j,child_klass| j[child_klass] = {} } + seen = Hash.new { |i, object_id| + i[object_id] = Hash.new { |j, child_class| + j[child_class] = {} } } @@ -142,11 +141,21 @@ module ActiveRecord parents = model_cache[join_root] column_aliases = aliases.column_aliases join_root - result_set.each { |row_hash| - parent = parents[row_hash[primary_key]] ||= join_root.instantiate(row_hash, column_aliases) - construct(parent, join_root, row_hash, result_set, seen, model_cache, aliases) + message_bus = ActiveSupport::Notifications.instrumenter + + payload = { + record_count: result_set.length, + class_name: join_root.base_klass.name } + message_bus.instrument('instantiation.active_record', payload) do + result_set.each { |row_hash| + parent_key = primary_key ? row_hash[primary_key] : row_hash + parent = parents[parent_key] ||= join_root.instantiate(row_hash, column_aliases) + construct(parent, join_root, row_hash, result_set, seen, model_cache, aliases) + } + end + parents.values end @@ -224,31 +233,34 @@ module ActiveRecord end def construct(ar_parent, parent, row, rs, seen, model_cache, aliases) - primary_id = ar_parent.id + return if ar_parent.nil? parent.children.each do |node| if node.reflection.collection? other = ar_parent.association(node.reflection.name) other.loaded! - else - if ar_parent.association_cache.key?(node.reflection.name) - model = ar_parent.association(node.reflection.name).target - construct(model, node, row, rs, seen, model_cache, aliases) - next - end + elsif ar_parent.association_cached?(node.reflection.name) + model = ar_parent.association(node.reflection.name).target + construct(model, node, row, rs, seen, model_cache, aliases) + next end key = aliases.column_alias(node, node.primary_key) id = row[key] - next if id.nil? + if id.nil? + nil_association = ar_parent.association(node.reflection.name) + nil_association.loaded! + next + end - model = seen[parent.base_klass][primary_id][node.base_klass][id] + model = seen[ar_parent.object_id][node.base_klass][id] if model construct(model, node, row, rs, seen, model_cache, aliases) else model = construct_model(ar_parent, node, row, model_cache, id, aliases) - seen[parent.base_klass][primary_id][node.base_klass][id] = model + model.readonly! + seen[ar_parent.object_id][node.base_klass][id] = model construct(model, node, row, rs, seen, model_cache, aliases) end end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 719eff9acc..a6ad09a38a 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -25,7 +25,7 @@ module ActiveRecord def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scope_chain, chain) joins = [] - bind_values = [] + binds = [] tables = tables.reverse scope_chain_index = 0 @@ -37,43 +37,45 @@ module ActiveRecord table = tables.shift klass = reflection.klass - case reflection.source_macro - when :belongs_to - key = reflection.association_primary_key - foreign_key = reflection.foreign_key - else - key = reflection.foreign_key - foreign_key = reflection.active_record_primary_key - end + join_keys = reflection.join_keys(klass) + key = join_keys.key + foreign_key = join_keys.foreign_key constraint = build_constraint(klass, table, key, foreign_table, foreign_key) + predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table)) scope_chain_items = scope_chain[scope_chain_index].map do |item| if item.is_a?(Relation) item else - ActiveRecord::Relation.create(klass, table).instance_exec(node, &item) + ActiveRecord::Relation.create(klass, table, predicate_builder) + .instance_exec(node, &item) end end scope_chain_index += 1 - scope_chain_items.concat [klass.send(:build_default_scope, ActiveRecord::Relation.create(klass, table))].compact + relation = ActiveRecord::Relation.create( + klass, + table, + predicate_builder, + ) + scope_chain_items.concat [klass.send(:build_default_scope, relation)].compact rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right| left.merge right end if rel && !rel.arel.constraints.empty? - bind_values.concat rel.bind_values + binds += rel.bound_attributes constraint = constraint.and rel.arel.constraints end if reflection.type value = foreign_klass.base_class.name - column = klass.columns_hash[column.to_s] + column = klass.columns_hash[reflection.type.to_s] - substitute = klass.connection.substitute_at(column, bind_values.length) - bind_values.push [column, value] + substitute = klass.connection.substitute_at(column) + binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name)) constraint = constraint.and table[reflection.type].eq substitute end @@ -83,7 +85,7 @@ module ActiveRecord foreign_table, foreign_klass = table, klass end - JoinInformation.new joins, bind_values + JoinInformation.new joins, binds end # Builds equality condition. @@ -95,7 +97,7 @@ module ActiveRecord # end # # If I execute `Physician.joins(:appointments).to_a` then - # reflection # => #<ActiveRecord::Reflection::HasManyReflection ...> + # klass # => Physician # table # => #<Arel::Table @name="appointments" ...> # key # => physician_id # foreign_table # => #<Arel::Table @name="physicians" ...> diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index 91e1c6a9d7..9c6573f913 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -19,7 +19,6 @@ module ActiveRecord def initialize(base_klass, children) @base_klass = base_klass - @column_names_with_alias = nil @children = children end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 7519fec10a..ecf6fb8643 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -2,33 +2,42 @@ module ActiveRecord module Associations # Implements the details of eager loading of Active Record associations. # - # Note that 'eager loading' and 'preloading' are actually the same thing. - # However, there are two different eager loading strategies. + # Suppose that you have the following two Active Record models: # - # The first one is by using table joins. This was only strategy available - # prior to Rails 2.1. Suppose that you have an Author model with columns - # 'name' and 'age', and a Book model with columns 'name' and 'sales'. Using - # this strategy, Active Record would try to retrieve all data for an author - # and all of its books via a single query: + # class Author < ActiveRecord::Base + # # columns: name, age + # has_many :books + # end # - # SELECT * FROM authors - # LEFT OUTER JOIN books ON authors.id = books.author_id - # WHERE authors.name = 'Ken Akamatsu' + # class Book < ActiveRecord::Base + # # columns: title, sales, author_id + # end # - # However, this could result in many rows that contain redundant data. After - # having received the first row, we already have enough data to instantiate - # the Author object. In all subsequent rows, only the data for the joined - # 'books' table is useful; the joined 'authors' data is just redundant, and - # processing this redundant data takes memory and CPU time. The problem - # quickly becomes worse and worse as the level of eager loading increases - # (i.e. if Active Record is to eager load the associations' associations as - # well). + # When you load an author with all associated books Active Record will make + # multiple queries like this: + # + # Author.includes(:books).where(name: ['bell hooks', 'Homer']).to_a + # + # => SELECT `authors`.* FROM `authors` WHERE `name` IN ('bell hooks', 'Homer') + # => SELECT `books`.* FROM `books` WHERE `author_id` IN (2, 5) + # + # Active Record saves the ids of the records from the first query to use in + # the second. Depending on the number of associations involved there can be + # arbitrarily many SQL queries made. + # + # However, if there is a WHERE clause that spans across tables Active + # Record will fall back to a slightly more resource-intensive single query: + # + # Author.includes(:books).where(books: {title: 'Illiad'}).to_a + # => SELECT `authors`.`id` AS t0_r0, `authors`.`name` AS t0_r1, `authors`.`age` AS t0_r2, + # `books`.`id` AS t1_r0, `books`.`title` AS t1_r1, `books`.`sales` AS t1_r2 + # FROM `authors` + # LEFT OUTER JOIN `books` ON `authors`.`id` = `books`.`author_id` + # WHERE `books`.`title` = 'Illiad' + # + # This could result in many rows that contain redundant data and it performs poorly at scale + # and is therefore only used when necessary. # - # The second strategy is to use multiple database queries, one for each - # level of association. Since Rails 2.1, this is the default strategy. In - # situations where a table join is necessary (e.g. when the +:conditions+ - # option references an association's column), it will fallback to the table - # join strategy. class Preloader #:nodoc: extend ActiveSupport::Autoload @@ -45,6 +54,8 @@ module ActiveRecord autoload :BelongsTo, 'active_record/associations/preloader/belongs_to' end + NULL_RELATION = Struct.new(:values, :where_clause, :joins_values).new({}, Relation::WhereClause.empty, []) + # Eager loads the named associations for the given Active Record record(s). # # In this description, 'association name' shall refer to the name passed @@ -79,9 +90,6 @@ module ActiveRecord # [ :books, :author ] # { author: :avatar } # [ :books, { author: :avatar } ] - - NULL_RELATION = Struct.new(:values, :bind_values).new({}, []) - def preload(records, associations, preload_scope = nil) records = Array.wrap(records).compact.uniq associations = Array.wrap(associations) @@ -98,6 +106,7 @@ module ActiveRecord private + # Loads all the given data into +records+ for the +association+. def preloaders_on(association, records, scope) case association when Hash @@ -107,7 +116,7 @@ module ActiveRecord when String preloaders_for_one(association.to_sym, records, scope) else - raise ArgumentError, "#{association.inspect} was not recognised for preload" + raise ArgumentError, "#{association.inspect} was not recognized for preload" end end @@ -123,6 +132,11 @@ module ActiveRecord } end + # Loads all the given data into +records+ for a singular +association+. + # + # Functions by instantiating a preloader class such as Preloader::HasManyThrough and + # call the +run+ method for each passed in class in the +records+ argument. + # # Not all records have the same class, so group then preload group on the reflection # itself so that if various subclass share the same association then we do not split # them unnecessarily @@ -151,7 +165,7 @@ module ActiveRecord h end - class AlreadyLoaded + class AlreadyLoaded # :nodoc: attr_reader :owners, :reflection def initialize(klass, owners, reflection, preload_scope) @@ -166,11 +180,16 @@ module ActiveRecord end end - class NullPreloader + class NullPreloader # :nodoc: def self.new(klass, owners, reflection, preload_scope); self; end def self.run(preloader); end + def self.preloaded_records; []; end end + # Returns a class containing the logic needed to load preload the data + # and attach it to a relation. For example +Preloader::Association+ or + # +Preloader::HasManyThrough+. The class returned implements a `run` method + # that accepts a preloader. def preloader_for(reflection, owners, rhs_klass) return NullPreloader unless rhs_klass diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index c0639742be..c43f13f3c4 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -12,7 +12,6 @@ module ActiveRecord @preload_scope = preload_scope @model = owners.first && owners.first.class @scope = nil - @owners_by_key = nil @preloaded_records = [] end @@ -33,7 +32,7 @@ module ActiveRecord end def query_scope(ids) - scope.where(association_key.in(ids)) + scope.where(association_key_name => ids) end def table @@ -56,18 +55,6 @@ module ActiveRecord raise NotImplementedError end - def owners_by_key - @owners_by_key ||= if key_conversion_required? - owners.group_by do |owner| - owner[owner_key_name].to_s - end - else - owners.group_by do |owner| - owner[owner_key_name] - end - end - end - def options reflection.options end @@ -75,32 +62,33 @@ module ActiveRecord private def associated_records_by_owner(preloader) - owners_map = owners_by_key - owner_keys = owners_map.keys.compact - - # Each record may have multiple owners, and vice-versa - records_by_owner = owners.each_with_object({}) do |owner,h| - h[owner] = [] + records = load_records + owners.each_with_object({}) do |owner, result| + result[owner] = records[convert_key(owner[owner_key_name])] || [] end + end - if owner_keys.any? - # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) - # Make several smaller queries if necessary or make one query if the adapter supports it - sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - - records = load_slices sliced - records.each do |record, owner_key| - owners_map[owner_key].each do |owner| - records_by_owner[owner] << record - end + def owner_keys + unless defined?(@owner_keys) + @owner_keys = owners.map do |owner| + owner[owner_key_name] end + @owner_keys.uniq! + @owner_keys.compact! end - - records_by_owner + @owner_keys end def key_conversion_required? - association_key_type != owner_key_type + @key_conversion_required ||= association_key_type != owner_key_type + end + + def convert_key(key) + if key_conversion_required? + key.to_s + else + key + end end def association_key_type @@ -111,17 +99,17 @@ module ActiveRecord @model.type_for_attribute(owner_key_name.to_s).type end - def load_slices(slices) - @preloaded_records = slices.flat_map { |slice| + def load_records + return {} if owner_keys.empty? + # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) + # Make several smaller queries if necessary or make one query if the adapter supports it + slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) + @preloaded_records = slices.flat_map do |slice| records_for(slice) - } - - @preloaded_records.map { |record| - key = record[association_key_name] - key = key.to_s if key_conversion_required? - - [record, key] - } + end + @preloaded_records.group_by do |record| + convert_key(record[association_key_name]) + end end def reflection_scope @@ -131,24 +119,25 @@ module ActiveRecord def build_scope scope = klass.unscoped - values = reflection_scope.values - reflection_binds = reflection_scope.bind_values + values = reflection_scope.values preload_values = preload_scope.values - preload_binds = preload_scope.bind_values - scope.where_values = Array(values[:where]) + Array(preload_values[:where]) + scope.where_clause = reflection_scope.where_clause + preload_scope.where_clause scope.references_values = Array(values[:references]) + Array(preload_values[:references]) - scope.bind_values = (reflection_binds + preload_binds) - scope._select! preload_values[:select] || values[:select] || table[Arel.star] + if preload_values[:select] || values[:select] + scope._select!(preload_values[:select] || values[:select]) + end scope.includes! preload_values[:includes] || values[:includes] - - if preload_values.key? :order - scope.order! preload_values[:order] + if preload_scope.joins_values.any? + scope.joins!(preload_scope.joins_values) else - if values.key? :order - scope.order! values[:order] - end + scope.joins!(reflection_scope.joins_values) + end + scope.order! preload_values[:order] || values[:order] + + if preload_values[:reordering] || values[:reordering] + scope.reordering_value = true end if preload_values[:readonly] || values[:readonly] @@ -159,6 +148,7 @@ module ActiveRecord scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name }) end + scope.unscope_values = Array(values[:unscope]) + Array(preload_values[:unscope]) klass.default_scoped.merge(scope) end end diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb index 7b37b5942d..2029871f39 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb @@ -8,7 +8,7 @@ module ActiveRecord records_by_owner = super if reflection_scope.distinct_value - records_by_owner.each_value { |records| records.uniq! } + records_by_owner.each_value(&:uniq!) end records_by_owner diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 1fed7f74e7..56aa23b173 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -63,7 +63,7 @@ module ActiveRecord should_reset = (through_scope != through_reflection.klass.unscoped) || (reflection.options[:source_type] && through_reflection.collection?) - # Dont cache the association - we would only be caching a subset + # Don't cache the association - we would only be caching a subset if should_reset owners.each { |owner| owner.association(association_name).reset @@ -78,9 +78,9 @@ module ActiveRecord if options[:source_type] scope.where! reflection.foreign_type => options[:source_type] else - unless reflection_scope.where_values.empty? + unless reflection_scope.where_clause.empty? scope.includes_values = Array(reflection_scope.values[:includes] || options[:source]) - scope.where_values = reflection_scope.values[:where] + scope.where_clause = reflection_scope.where_clause end scope.references! reflection_scope.values[:references] diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index f2e3a4e40f..c7cc48ba16 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -3,7 +3,13 @@ module ActiveRecord class SingularAssociation < Association #:nodoc: # Implements the reader method, e.g. foo.bar for Foo.has_one :bar def reader(force_reload = false) - if force_reload + if force_reload && klass + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing an argument to force an association to reload is now + deprecated and will be removed in Rails 5.1. Please call `reload` + on the parent object instead. + MSG + klass.uncached { reload } elsif !loaded? || stale_target? reload @@ -39,7 +45,7 @@ module ActiveRecord end def get_records - return scope.limit(1).to_a if reflection.scope_chain.any?(&:any?) + return scope.limit(1).to_a if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index f00fef8b9e..d0ec3e8015 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -3,7 +3,7 @@ module ActiveRecord module Associations module ThroughAssociation #:nodoc: - delegate :source_reflection, :through_reflection, :chain, :to => :reflection + delegate :source_reflection, :through_reflection, :to => :reflection protected @@ -13,10 +13,8 @@ module ActiveRecord # 2. To get the type conditions for any STI models in the chain def target_scope scope = super - chain.drop(1).each do |reflection| + reflection.chain.drop(1).each do |reflection| relation = reflection.klass.all - relation.merge!(reflection.scope) if reflection.scope - scope.merge!( relation.except(:select, :create_with, :includes, :preload, :joins, :eager_load) ) @@ -29,7 +27,7 @@ module ActiveRecord # Construct attributes for :through pointing to owner and associate. This is used by the # methods which create and delete records on the association. # - # We only support indirectly modifying through associations which has a belongs_to source. + # We only support indirectly modifying through associations which have a belongs_to source. # This is the "has_many :tags, through: :taggings" situation, where the join model # typically has a belongs_to on both side. In other words, associations which could also # be represented as has_and_belongs_to_many associations. @@ -77,15 +75,34 @@ module ActiveRecord end def ensure_mutable - if source_reflection.macro != :belongs_to - raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) + unless source_reflection.belongs_to? + if reflection.has_one? + raise HasOneThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) + else + raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) + end end end def ensure_not_nested if reflection.nested? - raise HasManyThroughNestedAssociationsAreReadonly.new(owner, reflection) + if reflection.has_one? + raise HasOneThroughNestedAssociationsAreReadonly.new(owner, reflection) + else + raise HasManyThroughNestedAssociationsAreReadonly.new(owner, reflection) + end + end + end + + def build_record(attributes) + inverse = source_reflection.inverse_of + target = through_association.target + + if inverse && target && !target.is_a?(Array) + attributes[inverse.foreign_key] = target.id end + + super(attributes) end end end |