diff options
Diffstat (limited to 'activerecord')
16 files changed, 119 insertions, 113 deletions
diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index 4a5c821607..104de4f69d 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -4,21 +4,21 @@ module ActiveRecord module Associations # Keeps track of table aliases for ActiveRecord::Associations::JoinDependency class AliasTracker # :nodoc: - def self.create(connection, initial_table, type_caster) + def self.create(connection, initial_table) aliases = Hash.new(0) aliases[initial_table] = 1 - new connection, aliases, type_caster + new(connection, aliases) end - def self.create_with_joins(connection, initial_table, joins, type_caster) + def self.create_with_joins(connection, initial_table, joins) if joins.empty? - create(connection, initial_table, type_caster) + create(connection, initial_table) else aliases = Hash.new { |h, k| h[k] = initial_count_for(connection, k, joins) } aliases[initial_table] = 1 - new connection, aliases, type_caster + new(connection, aliases) end end @@ -51,17 +51,16 @@ module ActiveRecord end # table_joins is an array of arel joins which might conflict with the aliases we assign here - def initialize(connection, aliases, type_caster) + def initialize(connection, aliases) @aliases = aliases @connection = connection - @type_caster = type_caster end - def aliased_table_for(table_name, aliased_name) + def aliased_table_for(table_name, aliased_name, type_caster) if aliases[table_name].zero? # If it's zero, we can have our table_name aliases[table_name] = 1 - Arel::Table.new(table_name, type_caster: @type_caster) + 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) @@ -74,7 +73,7 @@ module ActiveRecord else aliased_name end - Arel::Table.new(table_name, type_caster: @type_caster).alias(table_alias) + Arel::Table.new(table_name, type_caster: type_caster).alias(table_alias) end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 1593b94f0c..1744dc785f 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -21,7 +21,7 @@ module ActiveRecord reflection = association.reflection scope = klass.unscoped owner = association.owner - alias_tracker = AliasTracker.create connection, association.klass.table_name, klass.type_caster + alias_tracker = AliasTracker.create connection, association.klass.table_name chain_head, chain_tail = get_chain(reflection, association, alias_tracker) scope.extending! reflection.extensions @@ -112,7 +112,11 @@ module ActiveRecord 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)) + alias_name = tracker.aliased_table_for( + refl.table_name, + refl.alias_candidate(name), + refl.klass.type_caster + ) proxy = ReflectionProxy.new(refl, alias_name) previous_reflection.next = proxy previous_reflection = proxy @@ -138,7 +142,7 @@ module ActiveRecord # Exclude the scope of the association itself, because that # was already merged in the #scope method. reflection.constraints.each do |scope_chain_item| - item = eval_scope(reflection.klass, table, scope_chain_item, owner) + item = eval_scope(reflection, table, scope_chain_item, owner) if scope_chain_item == refl.scope scope.merge! item.except(:where, :includes) @@ -159,9 +163,8 @@ module ActiveRecord scope end - def eval_scope(klass, table, scope, owner) - predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table)) - ActiveRecord::Relation.create(klass, table, predicate_builder).instance_exec(owner, &scope) + def eval_scope(reflection, table, scope, owner) + reflection.build_scope(table).instance_exec(owner, &scope) end end end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 8cdb508c43..d77fcaf668 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -1114,6 +1114,7 @@ module ActiveRecord end def reset_scope # :nodoc: + @offsets = {} @scope = nil self end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 83e5c23cc4..bc66194aef 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -93,7 +93,7 @@ module ActiveRecord # joins # => [] # def initialize(base, associations, joins, eager_loading: true) - @alias_tracker = AliasTracker.create_with_joins(base.connection, base.table_name, joins, base.type_caster) + @alias_tracker = AliasTracker.create_with_joins(base.connection, base.table_name, joins) @eager_loading = eager_loading tree = self.class.make_tree associations @join_root = JoinBase.new base, build(tree, base) @@ -185,7 +185,8 @@ module ActiveRecord node.reflection.chain.map { |reflection| alias_tracker.aliased_table_for( reflection.table_name, - table_alias_for(reflection, parent, reflection != node.reflection) + table_alias_for(reflection, parent, reflection != node.reflection), + reflection.klass.type_caster ) } 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 97cfec0302..005410d598 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -34,34 +34,10 @@ module ActiveRecord table = tables.shift klass = reflection.klass - join_keys = reflection.join_keys - key = join_keys.key - foreign_key = join_keys.foreign_key + join_scope = reflection.join_scope(table, foreign_table, foreign_klass) - constraint = build_constraint(klass, table, key, foreign_table, foreign_key) - - predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table)) - scope_chain_items = reflection.join_scopes(table, predicate_builder) - klass_scope = reflection.klass_join_scope(table, predicate_builder) - - scope_chain_items.concat [klass_scope].compact - - rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right| - left.merge right - end - - if rel && !rel.arel.constraints.empty? - binds += rel.bound_attributes - constraint = constraint.and rel.arel.constraints - end - - if reflection.type - value = foreign_klass.base_class.name - column = klass.columns_hash[reflection.type.to_s] - - binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name)) - constraint = constraint.and klass.arel_attribute(reflection.type, table).eq(Arel::Nodes::BindParam.new) - end + binds.concat join_scope.bound_attributes + constraint = join_scope.arel.constraints joins << table.create_join(table, table.create_on(constraint), join_type) @@ -72,34 +48,6 @@ module ActiveRecord JoinInformation.new joins, binds end - # Builds equality condition. - # - # Example: - # - # class Physician < ActiveRecord::Base - # has_many :appointments - # end - # - # If I execute `Physician.joins(:appointments).to_a` then - # klass # => Physician - # table # => #<Arel::Table @name="appointments" ...> - # key # => physician_id - # foreign_table # => #<Arel::Table @name="physicians" ...> - # foreign_key # => id - # - def build_constraint(klass, table, key, foreign_table, foreign_key) - constraint = table[key].eq(foreign_table[foreign_key]) - - if klass.finder_needs_type_condition? - constraint = table.create_and([ - constraint, - klass.send(:type_condition, table) - ]) - end - - constraint - end - def table tables.first end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 9f77f38b35..208d1b2670 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -147,7 +147,7 @@ module ActiveRecord def preloaders_for_one(association, records, scope) grouped_records(association, records).flat_map do |reflection, klasses| klasses.map do |rhs_klass, rs| - loader = preloader_for(reflection, rs, rhs_klass).new(rhs_klass, rs, reflection, scope) + loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope) loader.run self loader end @@ -159,6 +159,7 @@ module ActiveRecord records.each do |record| next unless record assoc = record.association(association) + next unless assoc.klass klasses = h[assoc.reflection] ||= {} (klasses[assoc.klass] ||= []) << record end @@ -180,20 +181,11 @@ module ActiveRecord end end - class NullPreloader # :nodoc: - def self.new(klass, owners, reflection, preload_scope); self; end - def self.run(preloader); end - def self.preloaded_records; []; end - def self.owners; []; 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 - + def preloader_for(reflection, owners) if owners.first.association(reflection.name).loaded? return AlreadyLoaded end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e8ee8279fd..3d3ec862a3 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -171,7 +171,7 @@ module ActiveRecord JoinKeys = Struct.new(:key, :foreign_key) # :nodoc: def join_keys - get_join_keys klass + @join_keys ||= get_join_keys(klass) end # Returns a list of scopes that should be applied for this Reflection @@ -185,10 +185,30 @@ module ActiveRecord end deprecate :scope_chain + def join_scope(table, foreign_table, foreign_klass) + predicate_builder = predicate_builder(table) + scope_chain_items = join_scopes(table, predicate_builder) + klass_scope = klass_join_scope(table, predicate_builder) + + key = join_keys.key + foreign_key = join_keys.foreign_key + + klass_scope.where!(table[key].eq(foreign_table[foreign_key])) + + if klass.finder_needs_type_condition? + klass_scope.where!(klass.send(:type_condition, table)) + end + + if type + klass_scope.where!(type => foreign_klass.base_class.name) + end + + scope_chain_items.inject(klass_scope, &:merge!) + end + def join_scopes(table, predicate_builder) # :nodoc: if scope - [ActiveRecord::Relation.create(klass, table, predicate_builder) - .instance_exec(&scope)] + [build_scope(table, predicate_builder).instance_exec(&scope)] else [] end @@ -200,12 +220,7 @@ module ActiveRecord scope.joins_values = scope.left_outer_joins_values = [].freeze } else - relation = ActiveRecord::Relation.create( - klass, - table, - predicate_builder, - ) - klass.send(:build_default_scope, relation) + klass.default_scoped(build_scope(table, predicate_builder)) end end @@ -287,12 +302,19 @@ module ActiveRecord JoinKeys.new(join_pk(association_klass), join_fk) end + def build_scope(table, predicate_builder = predicate_builder(table)) + Relation.create(klass, table, predicate_builder) + end + protected def actual_source_reflection # FIXME: this is a horrible name self end private + def predicate_builder(table) + PredicateBuilder.new(TableMetadata.new(klass, table)) + end def join_pk(_) foreign_key diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 1d661fa8ed..5ec2dfcfc9 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -147,8 +147,7 @@ module ActiveRecord def last(limit = nil) return find_last(limit) if loaded? || limit_value - result = limit(limit) - result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key + result = ordered_relation.limit(limit) result = result.reverse_order! limit ? result.reverse : result.first @@ -535,11 +534,7 @@ module ActiveRecord if loaded? records[index, limit] || [] else - relation = if order_values.empty? && primary_key - order(arel_attribute(primary_key).asc) - else - self - end + relation = ordered_relation if limit_value.nil? || index < limit_value relation = relation.offset(offset_index + index) unless index.zero? @@ -554,11 +549,7 @@ module ActiveRecord if loaded? records[-index] else - relation = if order_values.empty? && primary_key - order(arel_attribute(primary_key).asc) - else - self - end + relation = ordered_relation relation.to_a[-index] # TODO: can be made more performant on large result sets by @@ -572,5 +563,13 @@ module ActiveRecord def find_last(limit) limit ? records.last(limit) : records.last end + + def ordered_relation + if order_values.empty? && primary_key + order(arel_attribute(primary_key).asc) + else + self + end + end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6ccdd7adcb..d44f6fd572 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1122,7 +1122,7 @@ module ActiveRecord validate_order_args(order_args) references = order_args.grep(String) - references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact! + references.map! { |arg| arg =~ /^\W?(\w+)\W?\./ && $1 }.compact! references!(references) if references.any? # if a symbol is given we prepend the quoted table name diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index a61fdd6454..388f471bf5 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -29,8 +29,7 @@ module ActiveRecord end end - def default_scoped # :nodoc: - scope = relation + def default_scoped(scope = relation) # :nodoc: build_default_scope(scope) || scope end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 541165b3d1..c25d87dd3e 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -59,7 +59,6 @@ module ActiveRecord args.concat(["--no-data"]) args.concat(["--routines"]) args.concat(["--skip-comments"]) - args.concat(Array(extra_flags)) if extra_flags ignore_tables = ActiveRecord::SchemaDumper.ignore_tables if ignore_tables.any? @@ -67,6 +66,7 @@ module ActiveRecord end args.concat(["#{configuration['database']}"]) + args.unshift(*extra_flags) if extra_flags run_cmd("mysqldump", args, "dumping") end @@ -75,7 +75,7 @@ module ActiveRecord args = prepare_command_options args.concat(["--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}]) args.concat(["--database", "#{configuration['database']}"]) - args.concat(Array(extra_flags)) if extra_flags + args.unshift(*extra_flags) if extra_flags run_cmd("mysql", args, "loading") end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 590d3642bd..169e1a5449 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -741,6 +741,41 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal client2, firm.clients.merge!(where: ["#{QUOTED_TYPE} = :type", { type: "Client" }], order: "id").first end + def test_find_first_after_reset_scope + firm = Firm.all.merge!(order: "id").first + collection = firm.clients + + original_object_id = collection.first.object_id + assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object" + + # It should return a different object, since the association has been reloaded + assert_not_equal original_object_id, firm.clients.first.object_id, "Expected #first to return a new object" + end + + def test_find_first_after_reset + firm = Firm.all.merge!(order: "id").first + collection = firm.clients + + original_object_id = collection.first.object_id + assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object" + collection.reset + + # It should return a different object, since the association has been reloaded + assert_not_equal original_object_id, collection.first.object_id, "Expected #first after #reload to return a new object" + end + + def test_find_first_after_reload + firm = Firm.all.merge!(order: "id").first + collection = firm.clients + + original_object_id = collection.first.object_id + assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object" + collection.reset + + # It should return a different object, since the association has been reloaded + assert_not_equal original_object_id, collection.first.object_id, "Expected #first after #reload to return a new object" + end + def test_find_all_with_include_and_conditions assert_nothing_raised do Developer.all.merge!(joins: :audit_logs, where: { "audit_logs.message" => nil, :name => "Smith" }).to_a diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 86f3b0b962..dc81f13fe9 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1717,6 +1717,9 @@ class RelationTest < ActiveRecord::TestCase scope = Post.order("comments.body") assert_equal ["comments"], scope.references_values + scope = Post.order("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}") + assert_equal ["comments"], scope.references_values + scope = Post.order("comments.body", "yaks.body") assert_equal ["comments", "yaks"], scope.references_values @@ -1735,6 +1738,9 @@ class RelationTest < ActiveRecord::TestCase scope = Post.reorder("comments.body") assert_equal %w(comments), scope.references_values + scope = Post.reorder("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}") + assert_equal ["comments"], scope.references_values + scope = Post.reorder("comments.body", "yaks.body") assert_equal %w(comments yaks), scope.references_values diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index c22d974536..9c6fb14376 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -296,7 +296,7 @@ if current_adapter?(:Mysql2Adapter) def test_structure_dump_with_extra_flags filename = "awesome-file.sql" - expected_command = ["mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "--noop", "test-db"] + expected_command = ["mysqldump", "--noop", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "test-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_dump_flags(["--noop"]) do @@ -364,7 +364,7 @@ if current_adapter?(:Mysql2Adapter) def test_structure_load filename = "awesome-file.sql" - expected_command = ["mysql", "--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}, "--database", "test-db", "--noop"] + expected_command = ["mysql", "--noop", "--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}, "--database", "test-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_load_flags(["--noop"]) do diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb index 2c3ad230a7..0f8be0ad85 100644 --- a/activerecord/test/models/membership.rb +++ b/activerecord/test/models/membership.rb @@ -1,4 +1,5 @@ class Membership < ActiveRecord::Base + enum type: %i(Membership CurrentMembership SuperMembership SelectedMembership TenantMembership) belongs_to :member belongs_to :club end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 6da0ef26f6..d7ceb6d4ef 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -491,7 +491,7 @@ ActiveRecord::Schema.define do t.datetime :joined_on t.integer :club_id, :member_id t.boolean :favourite, default: false - t.string :type + t.integer :type end create_table :member_types, force: true do |t| |