diff options
Diffstat (limited to 'activerecord/lib/active_record/associations')
18 files changed, 295 insertions, 294 deletions
diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index 4f3893588e..272eede824 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -33,7 +33,7 @@ module ActiveRecord elsif join.is_a?(Arel::Nodes::Join) join.left.name == name ? 1 : 0 elsif join.is_a?(Hash) - join.fetch(name, 0) + join[name] else raise ArgumentError, "joins list should be initialized by list of Arel::Nodes::Join" end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index ca8c7794e0..bf4942aac8 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -19,7 +19,6 @@ module ActiveRecord # HasManyThroughAssociation + ThroughAssociation class Association #:nodoc: attr_reader :owner, :target, :reflection - attr_accessor :inversed delegate :options, to: :reflection @@ -41,7 +40,9 @@ module ActiveRecord end # Reloads the \target and returns +self+ on success. - def reload + # The QueryCache is cleared if +force+ is true. + def reload(force = false) + klass.connection.clear_query_cache if force && klass reset reset_scope load_target @@ -67,7 +68,7 @@ module ActiveRecord # # Note that if the target has not been loaded, it is not considered stale. def stale_target? - !inversed && loaded? && @stale_state != stale_state + !@inversed && loaded? && @stale_state != stale_state end # Sets the target of this association to <tt>\target</tt>, and the \loaded flag to +true+. @@ -80,53 +81,44 @@ module ActiveRecord target_scope.merge!(association_scope) end - # The scope for this association. - # - # Note that the association_scope is merged into the target_scope only when the - # scope method is called. This is because at that point the call may be surrounded - # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which - # actually gets built. - def association_scope - if klass - @association_scope ||= AssociationScope.scope(self) - end - end - def reset_scope @association_scope = nil end # Set the inverse association, if possible def set_inverse_instance(record) - if invertible_for?(record) - inverse = record.association(inverse_reflection_for(record).name) - inverse.target = owner - inverse.inversed = true + if inverse = inverse_association_for(record) + inverse.inversed_from(owner) + end + record + end + + def set_inverse_instance_from_queries(record) + if inverse = inverse_association_for(record) + inverse.inversed_from_queries(owner) end record end # Remove the inverse association, if possible def remove_inverse_instance(record) - if invertible_for?(record) - inverse = record.association(inverse_reflection_for(record).name) - inverse.target = nil - inverse.inversed = false + if inverse = inverse_association_for(record) + inverse.inversed_from(nil) end end + def inversed_from(record) + self.target = record + @inversed = !!record + end + alias :inversed_from_queries :inversed_from + # Returns the class of the target. belongs_to polymorphic overrides this to look at the # polymorphic_type field on the owner. def klass reflection.klass end - # 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, self).merge!(klass.all) - end - def extensions extensions = klass.default_extensions | reflection.extensions @@ -187,6 +179,24 @@ module ActiveRecord end private + # The scope for this association. + # + # Note that the association_scope is merged into the target_scope only when the + # scope method is called. This is because at that point the call may be surrounded + # by scope.scoping { ... } or unscoped { ... } etc, which affects the scope which + # actually gets built. + def association_scope + if klass + @association_scope ||= AssociationScope.scope(self) + end + end + + # 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, self).merge!(klass.all) + end + def scope_for_create scope.scope_for_create end @@ -240,6 +250,12 @@ module ActiveRecord end end + def inverse_association_for(record) + if invertible_for?(record) + record.association(inverse_reflection_for(record).name) + end + end + # Can be redefined by subclasses, notably polymorphic belongs_to # The record parameter is necessary to support polymorphic inverses as we must check for # the association in the specific class of the record. @@ -269,6 +285,7 @@ module ActiveRecord def build_record(attributes) reflection.build_association(attributes) do |record| initialize_attributes(record, attributes) + yield(record) if block_given? 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 c8716741b0..3346725f2d 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -16,7 +16,7 @@ module ActiveRecord end end - def target=(record) + def inversed_from(record) replace_keys(record) super end @@ -34,25 +34,42 @@ module ActiveRecord @updated end - def decrement_counters # :nodoc: + def decrement_counters update_counters(-1) end - def increment_counters # :nodoc: + def increment_counters update_counters(1) end + def decrement_counters_before_last_save + if reflection.polymorphic? + model_was = owner.attribute_before_last_save(reflection.foreign_type).try(:constantize) + else + model_was = klass + end + + foreign_key_was = owner.attribute_before_last_save(reflection.foreign_key) + + if foreign_key_was && model_was < ActiveRecord::Base + update_counters_via_scope(model_was, foreign_key_was, -1) + end + end + + def target_changed? + owner.saved_change_to_attribute?(reflection.foreign_key) + end + private def replace(record) if record raise_on_type_mismatch!(record) - update_counters_on_replace(record) set_inverse_instance(record) @updated = true - else - decrement_counters end + replace_keys(record) + self.target = record end @@ -61,11 +78,16 @@ module ActiveRecord if target && !stale_target? target.increment!(reflection.counter_cache_column, by, touch: reflection.options[:touch]) else - klass.update_counters(target_id, reflection.counter_cache_column => by, touch: reflection.options[:touch]) + update_counters_via_scope(klass, owner._read_attribute(reflection.foreign_key), by) end end end + def update_counters_via_scope(klass, foreign_key, by) + scope = klass.unscoped.where!(primary_key(klass) => foreign_key) + scope.update_counters(reflection.counter_cache_column => by, touch: reflection.options[:touch]) + end + def find_target? !loaded? && foreign_key_present? && klass end @@ -74,22 +96,12 @@ module ActiveRecord reflection.counter_cache_column && owner.persisted? end - def update_counters_on_replace(record) - if require_counter_update? && different_target?(record) - owner.instance_variable_set :@_after_replace_counter_called, true - 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._read_attribute(reflection.foreign_key) + def replace_keys(record) + owner[reflection.foreign_key] = record ? record._read_attribute(primary_key(record.class)) : nil end - def replace_keys(record) - owner[reflection.foreign_key] = record ? - record._read_attribute(reflection.association_primary_key(record.class)) : nil + def primary_key(klass) + reflection.association_primary_key(klass) end def foreign_key_present? @@ -103,14 +115,6 @@ module ActiveRecord inverse && inverse.has_one? end - def target_id - if options[:primary_key] - owner.send(reflection.name).try(:id) - else - owner._read_attribute(reflection.foreign_key) - end - end - def stale_state result = owner._read_attribute(reflection.foreign_key) { |n| owner.send(:missing_attribute, n, caller) } result && result.to_s diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 75b4c4481a..9ae452e7a1 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -9,17 +9,16 @@ module ActiveRecord type.presence && type.constantize end - private + def target_changed? + super || owner.saved_change_to_attribute?(reflection.foreign_type) + end + private def replace_keys(record) super owner[reflection.foreign_type] = record ? record.class.polymorphic_name : nil end - def different_target?(record) - super || record.class != klass - end - def inverse_reflection_for(record) reflection.polymorphic_inverse_of(record.class) end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index c161454c1a..fc00f1e900 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -21,50 +21,16 @@ module ActiveRecord::Associations::Builder # :nodoc: add_default_callbacks(model, reflection) if reflection.options[:default] end - def self.define_accessors(mixin, reflection) - super - add_counter_cache_methods mixin - end - - def self.add_counter_cache_methods(mixin) - return if mixin.method_defined? :belongs_to_counter_cache_after_update - - mixin.class_eval do - def belongs_to_counter_cache_after_update(reflection) - foreign_key = reflection.foreign_key - cache_column = reflection.counter_cache_column - - if (@_after_replace_counter_called ||= false) - @_after_replace_counter_called = false - elsif saved_change_to_attribute?(foreign_key) && !new_record? - if reflection.polymorphic? - model = attribute_in_database(reflection.foreign_type).try(:constantize) - model_was = attribute_before_last_save(reflection.foreign_type).try(:constantize) - else - model = reflection.klass - model_was = reflection.klass - end - - foreign_key_was = attribute_before_last_save foreign_key - foreign_key = attribute_in_database foreign_key - - if foreign_key && model.respond_to?(:increment_counter) - model.increment_counter(cache_column, foreign_key) - end - - if foreign_key_was && model_was.respond_to?(:decrement_counter) - model_was.decrement_counter(cache_column, foreign_key_was) - end - end - end - end - end - def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column model.after_update lambda { |record| - record.belongs_to_counter_cache_after_update(reflection) + association = association(reflection.name) + + if association.target_changed? + association.increment_counters + association.decrement_counters_before_last_save + end } klass = reflection.class_name.safe_constantize @@ -84,7 +50,8 @@ module ActiveRecord::Associations::Builder # :nodoc: else klass = association.klass end - old_record = klass.find_by(klass.primary_key => old_foreign_id) + primary_key = reflection.association_primary_key(klass) + old_record = klass.find_by(primary_key => old_foreign_id) if old_record if touch != true @@ -114,12 +81,18 @@ module ActiveRecord::Associations::Builder # :nodoc: BelongsTo.touch_record(record, record.send(changes_method), foreign_key, n, touch, belongs_to_touch_method) }} - unless reflection.counter_cache_column + if reflection.counter_cache_column + touch_callback = callback.(:saved_changes) + update_callback = lambda { |record| + instance_exec(record, &touch_callback) unless association(reflection.name).target_changed? + } + model.after_update update_callback, if: :saved_changes? + else model.after_create callback.(:saved_changes), if: :saved_changes? + model.after_update callback.(:saved_changes), if: :saved_changes? model.after_destroy callback.(:changes_to_save) end - model.after_update callback.(:saved_changes), if: :saved_changes? model.after_touch callback.(:changes_to_save) end diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 35a72c3850..ff57c40121 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -24,7 +24,7 @@ module ActiveRecord::Associations::Builder # :nodoc: if block_given? extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" extension = Module.new(&Proc.new) - model.parent.const_set(extension_module_name, extension) + model.module_parent.const_set(extension_module_name, extension) end end 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 1981da11a2..0140aa15c8 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 @@ -2,39 +2,6 @@ module ActiveRecord::Associations::Builder # :nodoc: class HasAndBelongsToMany # :nodoc: - class JoinTableResolver # :nodoc: - KnownTable = Struct.new :join_table - - class KnownClass # :nodoc: - def initialize(lhs_class, rhs_class_name) - @lhs_class = lhs_class - @rhs_class_name = rhs_class_name - @join_table = nil - end - - def join_table - @join_table ||= [@lhs_class.table_name, klass.table_name].sort.join("\0").gsub(/^(.*[._])(.+)\0\1(.+)/, '\1\2_\3').tr("\0", "_") - end - - private - - def klass - @lhs_class.send(:compute_type, @rhs_class_name) - end - end - - def self.build(lhs_class, name, options) - if options[:join_table] - KnownTable.new options[:join_table].to_s - else - class_name = options.fetch(:class_name) { - name.to_s.camelize.singularize - } - KnownClass.new lhs_class, class_name.to_s - end - end - end - attr_reader :lhs_model, :association_name, :options def initialize(association_name, lhs_model, options) @@ -44,8 +11,6 @@ module ActiveRecord::Associations::Builder # :nodoc: end def through_model - habtm = JoinTableResolver.build lhs_model, association_name, options - join_model = Class.new(ActiveRecord::Base) { class << self attr_accessor :left_model @@ -56,7 +21,9 @@ module ActiveRecord::Associations::Builder # :nodoc: end def self.table_name - table_name_resolver.join_table + # Table name needs to be resolved lazily + # because RHS class might not have been loaded + @table_name ||= table_name_resolver.call end def self.compute_type(class_name) @@ -86,7 +53,7 @@ module ActiveRecord::Associations::Builder # :nodoc: } join_model.name = "HABTM_#{association_name.to_s.camelize}" - join_model.table_name_resolver = habtm + join_model.table_name_resolver = -> { table_name } join_model.left_model = lhs_model join_model.add_left_association :left_side, anonymous_class: lhs_model @@ -96,7 +63,7 @@ module ActiveRecord::Associations::Builder # :nodoc: def middle_reflection(join_model) middle_name = [lhs_model.name.downcase.pluralize, - association_name].join("_".freeze).gsub("::".freeze, "_".freeze).to_sym + association_name].join("_").gsub("::", "_").to_sym middle_options = middle_options join_model HasMany.create_reflection(lhs_model, @@ -117,6 +84,18 @@ module ActiveRecord::Associations::Builder # :nodoc: middle_options end + def table_name + if options[:join_table] + options[:join_table].to_s + else + class_name = options.fetch(:class_name) { + association_name.to_s.camelize.singularize + } + klass = lhs_model.send(:compute_type, class_name.to_s) + [lhs_model.table_name, klass.table_name].sort.join("\0").gsub(/^(.*[._])(.+)\0\1(.+)/, '\1\2_\3').tr("\0", "_") + end + end + def belongs_to_options(options) rhs_options = {} diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index d61d105544..c4741c9fe6 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -105,9 +105,7 @@ module ActiveRecord if attributes.is_a?(Array) attributes.collect { |attr| build(attr, &block) } else - add_to_target(build_record(attributes)) do |record| - yield(record) if block_given? - end + add_to_target(build_record(attributes, &block)) end end @@ -305,7 +303,6 @@ module ActiveRecord end private - def find_target scope = self.scope return scope.to_a if skip_statement_cache?(scope) @@ -360,15 +357,18 @@ module ActiveRecord if attributes.is_a?(Array) attributes.collect { |attr| _create_record(attr, raise, &block) } else + record = build_record(attributes, &block) transaction do - add_to_target(build_record(attributes)) do |record| - yield(record) if block_given? - insert_record(record, true, raise) { + result = nil + add_to_target(record) do + result = insert_record(record, true, raise) { @_was_loaded = loaded? @association_ids = nil } end + raise ActiveRecord::Rollback unless result end + record end end @@ -399,7 +399,7 @@ module ActiveRecord records.each { |record| callback(:before_remove, record) } delete_records(existing_records, method) if existing_records.any? - records.each { |record| target.delete(record) } + @target -= records records.each { |record| callback(:after_remove, record) } end @@ -412,9 +412,9 @@ module ActiveRecord end def replace_records(new_target, original_target) - delete(target - new_target) + delete(difference(target, new_target)) - unless concat(new_target - target) + unless concat(difference(new_target, target)) @target = original_target raise RecordNotSaved, "Failed to replace #{reflection.name} because one or more of the " \ "new records could not be saved." @@ -424,7 +424,7 @@ module ActiveRecord end def replace_common_records_in_memory(new_target, original_target) - common_records = new_target & original_target + common_records = intersection(new_target, original_target) common_records.each do |record| skip_callbacks = true replace_on_target(record, @target.index(record), skip_callbacks) @@ -446,7 +446,9 @@ module ActiveRecord end end - result && records + raise ActiveRecord::Rollback unless result + + records end def replace_on_target(record, index, skip_callbacks) diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 9a30198b95..4fbbc713e4 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -500,7 +500,7 @@ module ActiveRecord # Pet.find(1, 2, 3) # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) def delete_all(dependent = nil) - @association.delete_all(dependent) + @association.delete_all(dependent).tap { reset_scope } end # Deletes the records of the collection directly from the database @@ -527,7 +527,7 @@ module ActiveRecord # # Pet.find(1) # => Couldn't find Pet with id=1 def destroy_all - @association.destroy_all + @association.destroy_all.tap { reset_scope } end # Deletes the +records+ supplied from the collection according to the strategy @@ -646,7 +646,7 @@ module ActiveRecord # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> # # ] def delete(*records) - @association.delete(*records) + @association.delete(*records).tap { reset_scope } end # Destroys the +records+ supplied and removes them from the collection. @@ -718,7 +718,7 @@ module ActiveRecord # # Pet.find(4, 5, 6) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (4, 5, 6) def destroy(*records) - @association.destroy(*records) + @association.destroy(*records).tap { reset_scope } end ## @@ -1088,7 +1088,7 @@ module ActiveRecord # person.pets.reload # fetches pets from the database # # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>] def reload - proxy_association.reload + proxy_association.reload(true) reset_scope end @@ -1125,7 +1125,7 @@ module ActiveRecord SpawnMethods, ].flat_map { |klass| klass.public_instance_methods(false) - } - self.public_instance_methods(false) - [:select] + [:scoping] + } - self.public_instance_methods(false) - [:select] + [:scoping, :values] delegate(*delegate_methods, to: :scope) diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cf85a87fa7..f6fdbcde54 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -99,6 +99,7 @@ module ActiveRecord def delete_or_nullify_all_records(method) count = delete_count(method, scope) update_counter(-count) + count end # Deletes the records according to the <tt>:dependent</tt> option. @@ -130,6 +131,14 @@ module ActiveRecord end saved_successfully end + + def difference(a, b) + a - b + end + + def intersection(a, b) + a & b + end end end 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 59929b8c4e..84a9797aa5 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -21,20 +21,6 @@ module ActiveRecord super end - def concat_records(records) - ensure_not_nested - - records = super(records, true) - - if owner.new_record? && records - records.flatten.each do |record| - build_through_record(record) - end - end - - records - end - def insert_record(record, validate = true, raise = false) ensure_not_nested @@ -48,6 +34,20 @@ module ActiveRecord end private + def concat_records(records) + ensure_not_nested + + records = super(records, true) + + if owner.new_record? && records + records.flatten.each do |record| + build_through_record(record) + end + end + + records + end + # The through record (built with build_record) is temporarily cached # so that it may be reused if insert_record is subsequently called. # @@ -90,7 +90,7 @@ module ActiveRecord def build_record(attributes) ensure_not_nested - record = super(attributes) + record = super inverse = source_reflection.inverse_of if inverse @@ -161,6 +161,30 @@ module ActiveRecord else update_counter(-count) end + + count + end + + def difference(a, b) + distribution = distribution(b) + + a.reject { |record| mark_occurrence(distribution, record) } + end + + def intersection(a, b) + distribution = distribution(b) + + a.select { |record| mark_occurrence(distribution, record) } + end + + def mark_occurrence(distribution, record) + distribution[record] > 0 && distribution[record] -= 1 + end + + def distribution(array) + array.each_with_object(Hash.new(0)) do |record, distribution| + distribution[record] += 1 + end end def through_records_for(record) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index d211884135..390bfd8b08 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -106,7 +106,7 @@ module ActiveRecord end end - def _create_record(attributes, raise_error = false) + def _create_record(attributes, raise_error = false, &block) unless owner.persisted? raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index f88e383fe0..b76005b587 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -14,10 +14,8 @@ module ActiveRecord i[column.name] = column.alias } } - @name_and_alias_cache = tables.each_with_object({}) { |table, h| - h[table.node] = table.columns.map { |column| - [column.name, column.alias] - } + @columns_cache = tables.each_with_object({}) { |table, h| + h[table.node] = table.columns } end @@ -25,9 +23,8 @@ module ActiveRecord @tables.flat_map(&:column_aliases) end - # An array of [column_name, alias] pairs for the table def column_aliases(node) - @name_and_alias_cache[node] + @columns_cache[node] end def column_alias(node, column) @@ -67,42 +64,31 @@ module ActiveRecord end end - def initialize(base, table, associations, alias_tracker) - @alias_tracker = alias_tracker + def initialize(base, table, associations) tree = self.class.make_tree associations @join_root = JoinBase.new(base, table, build(tree, base)) - @join_root.children.each { |child| construct_tables! @join_root, child } end def reflections join_root.drop(1).map!(&:reflection) end - def join_constraints(joins_to_add, join_type) - joins = join_root.children.flat_map { |child| - make_join_constraints(join_root, child, join_type) - } + def join_constraints(joins_to_add, join_type, alias_tracker) + @alias_tracker = alias_tracker + + construct_tables!(join_root) + joins = make_join_constraints(join_root, join_type) joins.concat joins_to_add.flat_map { |oj| + construct_tables!(oj.join_root) if join_root.match? oj.join_root walk join_root, oj.join_root else - oj.join_root.children.flat_map { |child| - make_join_constraints(oj.join_root, child, join_type) - } + make_join_constraints(oj.join_root, join_type) end } end - def aliases - @aliases ||= Aliases.new join_root.each_with_index.map { |join_part, i| - columns = join_part.column_names.each_with_index.map { |column_name, j| - Aliases::Column.new column_name, "t#{i}_r#{j}" - } - Aliases::Table.new(join_part, columns) - } - end - def instantiate(result_set, &block) primary_key = aliases.column_alias(join_root, join_root.primary_key) @@ -127,35 +113,49 @@ module ActiveRecord 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, &block) - construct(parent, join_root, row_hash, result_set, seen, model_cache, aliases) + construct(parent, join_root, row_hash, seen, model_cache) } end parents.values end + def apply_column_aliases(relation) + relation._select!(-> { aliases.columns }) + end + protected - attr_reader :alias_tracker, :base_klass, :join_root + attr_reader :join_root private + attr_reader :alias_tracker - def make_constraints(parent, child, tables, join_type) - chain = child.reflection.chain - foreign_table = parent.table - foreign_klass = parent.base_klass - child.join_constraints(foreign_table, foreign_klass, join_type, tables, chain) + def aliases + @aliases ||= Aliases.new join_root.each_with_index.map { |join_part, i| + columns = join_part.column_names.each_with_index.map { |column_name, j| + Aliases::Column.new column_name, "t#{i}_r#{j}" + } + Aliases::Table.new(join_part, columns) + } end - def make_outer_joins(parent, child) - join_type = Arel::Nodes::OuterJoin - make_join_constraints(parent, child, join_type, true) + def construct_tables!(join_root) + join_root.each_children do |parent, child| + child.tables = table_aliases_for(parent, child) + end end - def make_join_constraints(parent, child, join_type, aliasing = false) - tables = aliasing ? table_aliases_for(parent, child) : child.tables - joins = make_constraints(parent, child, tables, join_type) + def make_join_constraints(join_root, join_type) + join_root.children.flat_map do |child| + make_constraints(join_root, child, join_type) + end + end - joins.concat child.children.flat_map { |c| make_join_constraints(child, c, join_type, aliasing) } + def make_constraints(parent, child, join_type = Arel::Nodes::OuterJoin) + foreign_table = parent.table + foreign_klass = parent.base_klass + joins = child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) + joins.concat child.children.flat_map { |c| make_constraints(child, c, join_type) } end def table_aliases_for(parent, node) @@ -168,13 +168,8 @@ module ActiveRecord } end - def construct_tables!(parent, node) - node.tables = table_aliases_for(parent, node) - node.children.each { |child| construct_tables! node, child } - end - def table_alias_for(reflection, parent, join) - name = "#{reflection.plural_name}_#{parent.table_name}" + name = reflection.alias_candidate(parent.table_name) join ? "#{name}_join" : name end @@ -183,8 +178,8 @@ module ActiveRecord [left.children.find { |node2| node1.match? node2 }, node1] }.partition(&:first) - ojs = missing.flat_map { |_, n| make_outer_joins left, n } - intersection.flat_map { |l, r| walk l, r }.concat ojs + joins = intersection.flat_map { |l, r| r.table = l.table; walk(l, r) } + joins.concat missing.flat_map { |_, n| make_constraints(left, n) } end def find_reflection(klass, name) @@ -202,11 +197,11 @@ module ActiveRecord raise EagerLoadPolymorphicError.new(reflection) end - JoinAssociation.new(reflection, build(right, reflection.klass), alias_tracker) + JoinAssociation.new(reflection, build(right, reflection.klass)) end end - def construct(ar_parent, parent, row, rs, seen, model_cache, aliases) + def construct(ar_parent, parent, row, seen, model_cache) return if ar_parent.nil? parent.children.each do |node| @@ -215,7 +210,7 @@ module ActiveRecord other.loaded! 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) + construct(model, node, row, seen, model_cache) next end @@ -227,25 +222,20 @@ module ActiveRecord next end - model = seen[ar_parent.object_id][node.base_klass][id] + model = seen[ar_parent.object_id][node][id] if model - construct(model, node, row, rs, seen, model_cache, aliases) + construct(model, node, row, seen, model_cache) else - model = construct_model(ar_parent, node, row, model_cache, id, aliases) - - if node.reflection.scope && - node.reflection.scope_for(node.base_klass.unscoped).readonly_value - model.readonly! - end + model = construct_model(ar_parent, node, row, model_cache, id) - seen[ar_parent.object_id][node.base_klass][id] = model - construct(model, node, row, rs, seen, model_cache, aliases) + seen[ar_parent.object_id][node][id] = model + construct(model, node, row, seen, model_cache) end end end - def construct_model(record, node, row, model_cache, id, aliases) + def construct_model(record, node, row, model_cache, id) other = record.association(node.reflection.name) model = model_cache[node][id] ||= @@ -259,6 +249,7 @@ module ActiveRecord other.target = model end + model.readonly! if node.readonly? model 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 c36386ec7e..4583d89cba 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -6,17 +6,14 @@ module ActiveRecord module Associations class JoinDependency # :nodoc: class JoinAssociation < JoinPart # :nodoc: - # The reflection of the association represented - attr_reader :reflection + attr_reader :reflection, :tables + attr_accessor :table - attr_accessor :tables - - def initialize(reflection, children, alias_tracker) + def initialize(reflection, children) super(reflection.klass, children) - @alias_tracker = alias_tracker - @reflection = reflection - @tables = nil + @reflection = reflection + @tables = nil end def match?(other) @@ -24,14 +21,13 @@ module ActiveRecord super && reflection == other.reflection end - def join_constraints(foreign_table, foreign_klass, join_type, tables, chain) - joins = [] - tables = tables.reverse + def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) + joins = [] # The chain starts with the target table, but we want to end with it here (makes # more sense in this context), so we reverse - chain.reverse_each do |reflection| - table = tables.shift + reflection.chain.reverse_each.with_index(1) do |reflection, i| + table = tables[-i] klass = reflection.klass constraint = reflection.build_join_constraint(table, foreign_table) @@ -54,12 +50,16 @@ module ActiveRecord joins end - def table - tables.first + def tables=(tables) + @tables = tables + @table = tables.first end - private - attr_reader :alias_tracker + def readonly? + return @readonly if defined?(@readonly) + + @readonly = reflection.scope && reflection.scope_for(base_klass.unscoped).readonly_value + end end end end 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 2181f308bf..3ad72a3646 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -33,6 +33,13 @@ module ActiveRecord children.each { |child| child.each(&block) } end + def each_children(&block) + children.each do |child| + yield self, child + child.each_children(&block) + end + end + # An Arel::Table for the active_record def table raise NotImplementedError @@ -47,8 +54,8 @@ module ActiveRecord length = column_names_with_alias.length while index < length - column_name, alias_name = column_names_with_alias[index] - hash[column_name] = row[alias_name] + column = column_names_with_alias[index] + hash[column.name] = row[column.alias] index += 1 end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 5c2ac5b374..8997579527 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -88,7 +88,6 @@ module ActiveRecord if records.empty? [] else - records.uniq! Array.wrap(associations).flat_map { |association| preloaders_on association, records, preload_scope } @@ -102,10 +101,8 @@ module ActiveRecord case association when Hash preloaders_for_hash(association, records, scope, polymorphic_parent) - when Symbol + when Symbol, String preloaders_for_one(association, records, scope, polymorphic_parent) - when String - preloaders_for_one(association.to_sym, records, scope, polymorphic_parent) else raise ArgumentError, "#{association.inspect} was not recognized for preload" end @@ -113,23 +110,21 @@ module ActiveRecord def preloaders_for_hash(association, records, scope, polymorphic_parent) association.flat_map { |parent, child| - loaders = preloaders_for_one parent, records, scope, polymorphic_parent - - recs = loaders.flat_map(&:preloaded_records).uniq - - reflection = records.first.class._reflect_on_association(parent) - polymorphic_parent = reflection && reflection.options[:polymorphic] - - loaders.concat Array.wrap(child).flat_map { |assoc| - preloaders_on assoc, recs, scope, polymorphic_parent - } - loaders + grouped_records(parent, records, polymorphic_parent).flat_map do |reflection, reflection_records| + loaders = preloaders_for_reflection(reflection, reflection_records, scope) + recs = loaders.flat_map(&:preloaded_records) + child_polymorphic_parent = reflection && reflection.options[:polymorphic] + loaders.concat Array.wrap(child).flat_map { |assoc| + preloaders_on assoc, recs, scope, child_polymorphic_parent + } + loaders + end } end # Loads all the given data into +records+ for a singular +association+. # - # Functions by instantiating a preloader class such as Preloader::HasManyThrough and + # Functions by instantiating a preloader class such as Preloader::Association 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 @@ -140,12 +135,17 @@ module ActiveRecord # classes, depending on the polymorphic_type field. So we group by the classes as # well. def preloaders_for_one(association, records, scope, polymorphic_parent) - grouped_records(association, records, polymorphic_parent).flat_map do |reflection, klasses| - klasses.map do |rhs_klass, rs| - loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope) - loader.run self - loader + grouped_records(association, records, polymorphic_parent) + .flat_map do |reflection, reflection_records| + preloaders_for_reflection reflection, reflection_records, scope end + end + + def preloaders_for_reflection(reflection, records, scope) + records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs| + loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope) + loader.run self + loader end end @@ -153,11 +153,9 @@ module ActiveRecord h = {} records.each do |record| next unless record - next if polymorphic_parent && !record.class._reflect_on_association(association) - assoc = record.association(association) - next unless assoc.klass - klasses = h[assoc.reflection] ||= {} - (klasses[assoc.klass] ||= []) << record + reflection = record.class._reflect_on_association(association) + next if polymorphic_parent && !reflection || !record.association(association).klass + (h[reflection] ||= []) << record end h end diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index ead89bfe6c..8e50cce102 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -17,9 +17,8 @@ module ActiveRecord replace(record) end - def build(attributes = {}) - record = build_record(attributes) - yield(record) if block_given? + def build(attributes = {}, &block) + record = build_record(attributes, &block) set_new_record(record) record end @@ -27,7 +26,7 @@ module ActiveRecord # Implements the reload reader method, e.g. foo.reload_bar for # Foo.has_one :bar def force_reload_reader - klass.uncached { reload } + reload(true) target end @@ -62,9 +61,8 @@ module ActiveRecord replace(record) end - def _create_record(attributes, raise_error = false) - record = build_record(attributes) - yield(record) if block_given? + def _create_record(attributes, raise_error = false, &block) + record = build_record(attributes, &block) saved = record.save set_new_record(record) raise RecordInvalid.new(record) if !saved && raise_error diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 5afb0bc068..15e6565e69 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -114,7 +114,7 @@ module ActiveRecord attributes[inverse.foreign_key] = target.id end - super(attributes) + super end end end |