diff options
Diffstat (limited to 'activerecord')
35 files changed, 392 insertions, 153 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 3f1908ec18..52c38edf81 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,60 @@ +* Take into account association conditions when deleting through records. + + Fixes #18424. + + *Piotr Jakubowski* + +* Fix nested `has_many :through` associations on unpersisted parent instances. + + For example, if you have + + class Post < ActiveRecord::Base + belongs_to :author + has_many :books, through: :author + has_many :subscriptions, through: :books + end + + class Author < ActiveRecord::Base + has_one :post + has_many :books + has_many :subscriptions, through: :books + end + + class Book < ActiveRecord::Base + belongs_to :author + has_many :subscriptions + end + + class Subscription < ActiveRecord::Base + belongs_to :book + end + + Before: + + If `post` is not persisted, then `post.subscriptions` will be empty. + + After: + + If `post` is not persisted, then `post.subscriptions` can be set and used + just like it would if `post` were persisted. + + Fixes #16313. + + *Zoltan Kiss* + +* Fixed inconsistency with `first(n)` when used with `limit()`. + The `first(n)` finder now respects the `limit()`, making it consistent + with `relation.to_a.first(n)`, and also with the behavior of `last(n)`. + + Fixes #23979. + + *Brian Christian* + +* Use `count(:all)` in `HasManyAssociation#count_records` to prevent invalid + SQL queries for association counting. + + *Klas Eskilson* + * Fix to invoke callbacks when using `update_attribute`. *Mike Busch* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 661605d3e5..b1cad0d0a4 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1061,12 +1061,6 @@ module ActiveRecord # belongs_to :dungeon, inverse_of: :evil_wizard # end # - # There are limitations to <tt>:inverse_of</tt> support: - # - # * does not work with <tt>:through</tt> associations. - # * does not work with <tt>:polymorphic</tt> associations. - # * inverse associations for #belongs_to associations #has_many are ignored. - # # For more information, see the documentation for the +:inverse_of+ option. # # == Deleting from associations @@ -1279,6 +1273,9 @@ module ActiveRecord # Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a #has_many # association will use "person_id" as the default <tt>:foreign_key</tt>. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option. # [:foreign_type] # Specify the column used to store the associated object's type, if this is a polymorphic # association. By default this is guessed to be the name of the polymorphic association @@ -1352,8 +1349,7 @@ module ActiveRecord # <tt>:autosave</tt> to <tt>true</tt>. # [:inverse_of] # Specifies the name of the #belongs_to association on the associated object - # that is the inverse of this #has_many association. Does not work in combination - # with <tt>:through</tt> or <tt>:as</tt> options. + # that is the inverse of this #has_many association. # See ActiveRecord::Associations::ClassMethods's overview on Bi-directional associations for more detail. # [:extend] # Specifies a module or array of modules that will be extended into the association object returned. @@ -1449,6 +1445,9 @@ module ActiveRecord # Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a #has_one association # will use "person_id" as the default <tt>:foreign_key</tt>. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option. # [:foreign_type] # Specify the column used to store the associated object's type, if this is a polymorphic # association. By default this is guessed to be the name of the polymorphic association @@ -1464,6 +1463,9 @@ module ActiveRecord # <tt>:primary_key</tt>, and <tt>:foreign_key</tt> are ignored, as the association uses the # source reflection. You can only use a <tt>:through</tt> query through a #has_one # or #belongs_to association on the join model. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option. # [:source] # Specifies the source association name used by #has_one <tt>:through</tt> queries. # Only use it if the name cannot be inferred from the association. @@ -1484,8 +1486,7 @@ module ActiveRecord # <tt>:autosave</tt> to <tt>true</tt>. # [:inverse_of] # Specifies the name of the #belongs_to association on the associated object - # that is the inverse of this #has_one association. Does not work in combination - # with <tt>:through</tt> or <tt>:as</tt> options. + # that is the inverse of this #has_one association. # See ActiveRecord::Associations::ClassMethods's overview on Bi-directional associations for more detail. # [:required] # When set to +true+, the association will also have its presence validated. @@ -1570,6 +1571,9 @@ module ActiveRecord # association will use "person_id" as the default <tt>:foreign_key</tt>. Similarly, # <tt>belongs_to :favorite_person, class_name: "Person"</tt> will use a foreign key # of "favorite_person_id". + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option. # [:foreign_type] # Specify the column used to store the associated object's type, if this is a polymorphic # association. By default this is guessed to be the name of the association with a "_type" @@ -1619,8 +1623,7 @@ module ActiveRecord # +after_commit+ and +after_rollback+ callbacks are executed. # [:inverse_of] # Specifies the name of the #has_one or #has_many association on the associated - # object that is the inverse of this #belongs_to association. Does not work in - # combination with the <tt>:polymorphic</tt> options. + # object that is the inverse of this #belongs_to association. # See ActiveRecord::Associations::ClassMethods's overview on Bi-directional associations for more detail. # [:optional] # When set to +true+, the association will not have its presence validated. @@ -1789,6 +1792,9 @@ module ActiveRecord # of this class in lower-case and "_id" suffixed. So a Person class that makes # a #has_and_belongs_to_many association to Project will use "person_id" as the # default <tt>:foreign_key</tt>. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option. # [:association_foreign_key] # Specify the foreign key used for the association on the receiving side of the association. # By default this is guessed to be the name of the associated class in lower-case and "_id" suffixed. diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index ca3032d967..7c69cd65ee 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -104,8 +104,8 @@ module ActiveRecord::Associations::Builder # :nodoc: def self.define_readers(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}(*args) - association(:#{name}).reader(*args) + def #{name} + association(:#{name}).reader end CODE end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 07c7f28d2d..cf85a87fa7 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -63,7 +63,7 @@ module ActiveRecord count = if reflection.has_cached_counter? owner._read_attribute(reflection.counter_cache_column).to_i else - scope.count + scope.count(:all) end # If there's nothing in the database and @target has no new records 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 adbf52b87c..7a3bef969b 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -140,6 +140,7 @@ module ActiveRecord scope = through_association.scope scope.where! construct_join_attributes(*records) + scope = scope.where(through_scope_attributes) case method when :destroy @@ -147,14 +148,7 @@ module ActiveRecord count = scope.destroy_all.length else scope.each(&:_run_destroy_callbacks) - - arel = scope.arel - - stmt = Arel::DeleteManager.new - stmt.from scope.klass.arel_table - stmt.wheres = arel.constraints - - count = scope.klass.connection.delete(stmt, "SQL") + count = scope.delete_all end when :nullify count = scope.update_all(source_reflection.foreign_key => nil) diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index e1087be9b3..59320431ee 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -83,7 +83,7 @@ module ActiveRecord # { author: :avatar } # [ :books, { author: :avatar } ] def preload(records, associations, preload_scope = nil) - records = records.compact + records = Array.wrap(records).compact if records.empty? [] diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index bce2a95ce1..54673b74f7 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -4,9 +4,20 @@ module ActiveRecord module Associations # = Active Record Through Association module ThroughAssociation #:nodoc: - delegate :source_reflection, :through_reflection, to: :reflection + delegate :source_reflection, to: :reflection private + def through_reflection + @through_reflection ||= begin + refl = reflection.through_reflection + + while refl.through_reflection? + refl = refl.through_reflection + end + + refl + end + end # We merge in these scopes for two reasons: # @@ -38,24 +49,22 @@ module ActiveRecord def construct_join_attributes(*records) ensure_mutable - if source_reflection.association_primary_key(reflection.klass) == reflection.klass.primary_key + association_primary_key = source_reflection.association_primary_key(reflection.klass) + + if association_primary_key == reflection.klass.primary_key && !options[:source_type] join_attributes = { source_reflection.name => records } else join_attributes = { - source_reflection.foreign_key => - records.map { |record| - record.send(source_reflection.association_primary_key(reflection.klass)) - } + source_reflection.foreign_key => records.map(&association_primary_key.to_sym) } end if options[:source_type] - join_attributes[source_reflection.foreign_type] = - records.map { |record| record.class.base_class.name } + join_attributes[source_reflection.foreign_type] = [ options[:source_type] ] end if records.count == 1 - Hash[join_attributes.map { |k, v| [k, v.first] }] + join_attributes.transform_values!(&:first) else join_attributes end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 9849f9d5d7..c730584902 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1005,9 +1005,7 @@ module ActiveRecord def retrieve_connection(spec_name) #:nodoc: pool = retrieve_connection_pool(spec_name) raise ConnectionNotEstablished, "No connection pool with '#{spec_name}' found." unless pool - conn = pool.connection - raise ConnectionNotEstablished, "No connection for '#{spec_name}' in connection pool" unless conn - conn + pool.connection end # Returns true if a connection that's accessible to this class has diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index fc80d332f9..2655407d75 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -537,12 +537,7 @@ module ActiveRecord end def extract_limit(sql_type) - case sql_type - when /^bigint/i - 8 - when /\((.*)\)/ - $1.to_i - end + $1.to_i if sql_type =~ /\((.*)\)/ end def translate_exception_class(e, sql) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb index a89aa5ea09..6edb7cfd3c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb @@ -60,7 +60,7 @@ module ActiveRecord end def type_cast_single_for_database(value) - infinity?(value) ? "" : @subtype.serialize(value) + infinity?(value) ? value : @subtype.serialize(value) end def extract_bounds(value) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 9fdeab06c1..e75202b0be 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -138,7 +138,7 @@ module ActiveRecord end def encode_range(range) - "[#{type_cast(range.first)},#{type_cast(range.last)}#{range.exclude_end? ? ')' : ']'}" + "[#{type_cast_range_value(range.first)},#{type_cast_range_value(range.last)}#{range.exclude_end? ? ')' : ']'}" end def determine_encoding_of_strings_in_array(value) @@ -154,6 +154,14 @@ module ActiveRecord else _type_cast(values) end end + + def type_cast_range_value(value) + infinity?(value) ? "" : type_cast(value) + end + + def infinity?(value) + value.respond_to?(:infinite?) && value.infinite? + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index bf5fbb30e1..a8895f8606 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -364,6 +364,31 @@ module ActiveRecord SQL end + def bulk_change_table(table_name, operations) + sql_fragments = [] + non_combinable_operations = [] + + operations.each do |command, args| + table, arguments = args.shift, args + method = :"#{command}_for_alter" + + if respond_to?(method, true) + sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) } + sql_fragments << sqls + non_combinable_operations << procs if procs.present? + else + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? + non_combinable_operations.each(&:call) + sql_fragments = [] + non_combinable_operations = [] + send(command, table, *arguments) + end + end + + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? + non_combinable_operations.each(&:call) + end + # Renames a table. # Also renames a table's primary key sequence if the sequence name exists and # matches the Active Record default. @@ -394,7 +419,7 @@ module ActiveRecord def change_column(table_name, column_name, type, options = {}) #:nodoc: clear_cache! - sqls, procs = change_column_for_alter(table_name, column_name, type, options) + sqls, procs = Array(change_column_for_alter(table_name, column_name, type, options)).partition { |v| v.is_a?(String) } execute "ALTER TABLE #{quote_table_name(table_name)} #{sqls.join(", ")}" procs.each(&:call) end @@ -665,12 +690,10 @@ module ActiveRecord def change_column_for_alter(table_name, column_name, type, options = {}) sqls = [change_column_sql(table_name, column_name, type, options)] - procs = [] sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) sqls << change_column_null_for_alter(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - procs << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment) - - [sqls, procs] + sqls << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment) + sqls end @@ -694,6 +717,14 @@ module ActiveRecord "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL" end + def add_timestamps_for_alter(table_name, options = {}) + [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)] + end + + def remove_timestamps_for_alter(table_name, options = {}) + [remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)] + end + def add_index_opclass(quoted_columns, **options) opclasses = options_for_index_columns(options[:opclass]) quoted_columns.each do |name, column| diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 23fc69d649..441be47fa1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -122,6 +122,10 @@ module ActiveRecord include PostgreSQL::SchemaStatements include PostgreSQL::DatabaseStatements + def supports_bulk_alter? + true + end + def supports_index_sort_order? true end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 441c7cd28f..d4f5bd16ac 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -290,19 +290,18 @@ module ActiveRecord rename_table_indexes(table_name, new_name) end - # See: https://www.sqlite.org/lang_altertable.html - # SQLite has an additional restriction on the ALTER TABLE statement - def valid_alter_table_type?(type) - type.to_sym != :primary_key + def valid_alter_table_type?(type, options = {}) + !invalid_alter_table_type?(type, options) end + deprecate :valid_alter_table_type? def add_column(table_name, column_name, type, options = {}) #:nodoc: - if valid_alter_table_type?(type) && !options[:primary_key] - super(table_name, column_name, type, options) - else + if invalid_alter_table_type?(type, options) alter_table(table_name) do |definition| definition.column(column_name, type, options) end + else + super end end @@ -386,6 +385,12 @@ module ActiveRecord end alias column_definitions table_structure + # See: https://www.sqlite.org/lang_altertable.html + # SQLite has an additional restriction on the ALTER TABLE statement + def invalid_alter_table_type?(type, options) + type.to_sym == :primary_key || options[:primary_key] + end + def alter_table(table_name, options = {}) altered_table_name = "a#{table_name}" caller = lambda { |definition| yield definition if block_given? } diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index a3f8bfd1f1..c28e31a3da 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -34,7 +34,8 @@ module ActiveRecord def self.add_reflection(ar, name, reflection) ar.clear_reflections_cache - ar._reflections = ar._reflections.merge(name.to_s => reflection) + name = name.to_s + ar._reflections = ar._reflections.except(name).merge!(name => reflection) end def self.add_aggregate_reflection(ar, name, reflection) @@ -563,7 +564,7 @@ module ActiveRecord end VALID_AUTOMATIC_INVERSE_MACROS = [:has_many, :has_one, :belongs_to] - INVALID_AUTOMATIC_INVERSE_OPTIONS = [:conditions, :through, :foreign_key] + INVALID_AUTOMATIC_INVERSE_OPTIONS = [:through, :foreign_key] def add_as_source(seed) seed diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4df3864d07..10be583ef4 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -430,7 +430,7 @@ module ActiveRecord relation = self if eager_loading? - find_with_associations { |rel, _| relation = rel } + apply_join_dependency { |rel, _| relation = rel } end conn = klass.connection @@ -533,7 +533,7 @@ module ActiveRecord skip_query_cache_if_necessary do @records = if eager_loading? - find_with_associations do |relation, join_dependency| + apply_join_dependency do |relation, join_dependency| if ActiveRecord::NullRelation === relation [] else diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ff06ecbee1..98992caa0c 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -148,7 +148,7 @@ module ActiveRecord # # [#<Person id:4>, #<Person id:3>, #<Person id:2>] def last(limit = nil) - return find_last(limit) if loaded? || limit_value + return find_last(limit) if loaded? || has_limit_or_offset? result = ordered_relation.limit(limit) result = result.reverse_order! @@ -313,7 +313,7 @@ module ActiveRecord return false if !conditions || limit_value == 0 if eager_loading? - relation = apply_join_dependency(construct_join_dependency(eager_loading: false)) + relation = apply_join_dependency(eager_loading: false) return relation.exists?(conditions) end @@ -358,24 +358,6 @@ module ActiveRecord offset_value || 0 end - def find_with_associations - # NOTE: the JoinDependency constructed here needs to know about - # any joins already present in `self`, so pass them in - # - # failing to do so means that in cases like activerecord/test/cases/associations/inner_join_association_test.rb:136 - # incorrect SQL is generated. In that case, the join dependency for - # SpecialCategorizations is constructed without knowledge of the - # preexisting join in joins_values to categorizations (by way of - # the `has_many :through` for categories). - # - join_dependency = construct_join_dependency - - relation = apply_join_dependency(join_dependency) - relation._select!(join_dependency.aliases.columns) - - yield relation, join_dependency - end - def construct_relation_for_exists(conditions) relation = except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) @@ -396,17 +378,23 @@ module ActiveRecord ) end - def apply_join_dependency(join_dependency = construct_join_dependency) + def apply_join_dependency(eager_loading: true) + join_dependency = construct_join_dependency(eager_loading: eager_loading) relation = except(:includes, :eager_load, :preload).joins!(join_dependency) - if using_limitable_reflections?(join_dependency.reflections) - relation - else - if relation.limit_value + if eager_loading && !using_limitable_reflections?(join_dependency.reflections) + if has_limit_or_offset? limited_ids = limited_ids_for(relation) limited_ids.empty? ? relation.none! : relation.where!(primary_key => limited_ids) end - relation.except(:limit, :offset) + relation.limit_value = relation.offset_value = nil + end + + if block_given? + relation._select!(join_dependency.aliases.columns) + yield relation, join_dependency + else + relation end end @@ -532,7 +520,11 @@ module ActiveRecord else relation = ordered_relation - if limit_value.nil? || index < limit_value + if limit_value + limit = [limit_value - index, limit].min + end + + if limit > 0 relation = relation.offset(offset_index + index) unless index.zero? relation.limit(limit).to_a else @@ -547,12 +539,11 @@ module ActiveRecord else relation = ordered_relation - relation.to_a[-index] - # TODO: can be made more performant on large result sets by - # for instance, last(index)[-index] (which would require - # refactoring the last(n) finder method to make test suite pass), - # or by using a combination of reverse_order, limit, and offset, - # e.g., reverse_order.offset(index-1).first + if equal?(relation) || has_limit_or_offset? + relation.records[-index] + else + relation.last(index)[-index] + end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 0296101f81..009624e194 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -378,7 +378,7 @@ module ActiveRecord if !VALID_UNSCOPING_VALUES.include?(scope) raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." end - set_value(scope, nil) + set_value(scope, DEFAULT_VALUES[scope]) when Hash scope.each do |key, target_value| if key != :where @@ -905,7 +905,7 @@ module ActiveRecord protected # Returns a relation value with a given name def get_value(name) # :nodoc: - @values[name] || default_value_for(name) + @values.fetch(name, DEFAULT_VALUES[name]) end # Sets the relation value with the given name @@ -1190,7 +1190,6 @@ module ActiveRecord DEFAULT_VALUES = { create_with: FROZEN_EMPTY_HASH, - readonly: false, where: Relation::WhereClause.empty, having: Relation::WhereClause.empty, from: Relation::FromClause.empty @@ -1199,15 +1198,5 @@ module ActiveRecord Relation::MULTI_VALUE_METHODS.each do |value| DEFAULT_VALUES[value] ||= FROZEN_EMPTY_ARRAY end - - Relation::SINGLE_VALUE_METHODS.each do |value| - DEFAULT_VALUES[value] = nil if DEFAULT_VALUES[value].nil? - end - - def default_value_for(name) - DEFAULT_VALUES.fetch(name) do - raise ArgumentError, "unknown relation value #{name.inspect}" - end - end end end diff --git a/activerecord/test/cases/adapters/postgresql/range_test.rb b/activerecord/test/cases/adapters/postgresql/range_test.rb index 813a8721a2..261c24634e 100644 --- a/activerecord/test/cases/adapters/postgresql/range_test.rb +++ b/activerecord/test/cases/adapters/postgresql/range_test.rb @@ -358,6 +358,18 @@ _SQL end end + def test_infinity_values + PostgresqlRange.create!(int4_range: 1..Float::INFINITY, + int8_range: -Float::INFINITY..0, + float_range: -Float::INFINITY..Float::INFINITY) + + record = PostgresqlRange.first + + assert_equal(1...Float::INFINITY, record.int4_range) + assert_equal(-Float::INFINITY...1, record.int8_range) + assert_equal(-Float::INFINITY...Float::INFINITY, record.float_range) + end + private def assert_equal_round_trip(range, attribute, value) round_trip(range, attribute, value) diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index cd5d6f17d8..20579c6476 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -500,6 +500,10 @@ module ActiveRecord end end + def test_deprecate_valid_alter_table_type + assert_deprecated { @conn.valid_alter_table_type?(:string) } + end + private def assert_logged(logs) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 2649dc010f..9830917bc3 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -787,7 +787,7 @@ class EagerAssociationTest < ActiveRecord::TestCase Tagging.create!(taggable_type: "Post", taggable_id: post2.id, tag: tag) tag_with_includes = OrderedTag.includes(:tagged_posts).find(tag.id) - assert_equal(tag_with_includes.taggings.map(&:taggable).map(&:title), tag_with_includes.tagged_posts.map(&:title)) + assert_equal tag_with_includes.ordered_taggings.map(&:taggable).map(&:title), tag_with_includes.tagged_posts.map(&:title) end def test_eager_has_many_through_multiple_with_order diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 0eafd3a4e2..18548f8516 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2529,6 +2529,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [first_bulb, second_bulb], car.bulbs end + test "association size calculation works with default scoped selects when not previously fetched" do + firm = Firm.create!(name: "Firm") + 5.times { firm.developers_with_select << Developer.create!(name: "Developer") } + + same_firm = Firm.find(firm.id) + assert_equal 5, same_firm.developers_with_select.size + end + test "prevent double insertion of new object when the parent association loaded in the after save callback" do reset_callbacks(:save, Bulb) do Bulb.after_save { |record| record.car.bulbs.load } diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 7c9c9e81ab..56a4b7c4d1 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1308,6 +1308,70 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end end + def test_has_many_through_update_ids_with_conditions + author = Author.create!(name: "Bill") + category = categories(:general) + + author.update( + special_categories_with_condition_ids: [category.id], + nonspecial_categories_with_condition_ids: [category.id] + ) + + assert_equal [category.id], author.special_categories_with_condition_ids + assert_equal [category.id], author.nonspecial_categories_with_condition_ids + + author.update(nonspecial_categories_with_condition_ids: []) + author.reload + + assert_equal [category.id], author.special_categories_with_condition_ids + assert_equal [], author.nonspecial_categories_with_condition_ids + end + + def test_single_has_many_through_association_with_unpersisted_parent_instance + post_with_single_has_many_through = Class.new(Post) do + def self.name; "PostWithSingleHasManyThrough"; end + has_many :subscriptions, through: :author + end + post = post_with_single_has_many_through.new + + post.author = authors(:mary) + book1 = Book.create!(name: "essays on single has many through associations 1") + post.author.books << book1 + subscription1 = Subscription.first + book1.subscriptions << subscription1 + assert_equal [subscription1], post.subscriptions.to_a + + post.author = authors(:bob) + book2 = Book.create!(name: "essays on single has many through associations 2") + post.author.books << book2 + subscription2 = Subscription.second + book2.subscriptions << subscription2 + assert_equal [subscription2], post.subscriptions.to_a + end + + def test_nested_has_many_through_association_with_unpersisted_parent_instance + post_with_nested_has_many_through = Class.new(Post) do + def self.name; "PostWithNestedHasManyThrough"; end + has_many :books, through: :author + has_many :subscriptions, through: :books + end + post = post_with_nested_has_many_through.new + + post.author = authors(:mary) + book1 = Book.create!(name: "essays on nested has many through associations 1") + post.author.books << book1 + subscription1 = Subscription.first + book1.subscriptions << subscription1 + assert_equal [subscription1], post.subscriptions.to_a + + post.author = authors(:bob) + book2 = Book.create!(name: "essays on nested has many through associations 2") + post.author.books << book2 + subscription2 = Subscription.second + book2.subscriptions << subscription2 + assert_equal [subscription2], post.subscriptions.to_a + end + private def make_model(name) Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 5eda39a0c7..e682da6fed 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -688,6 +688,11 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [], Topic.includes(:replies).limit(1).where("0 = 1").pluck(:id) end + def test_pluck_with_includes_offset + assert_equal [5], Topic.includes(:replies).order(:id).offset(4).pluck(:id) + assert_equal [], Topic.includes(:replies).order(:id).offset(5).pluck(:id) + end + def test_pluck_not_auto_table_name_prefix_if_column_included Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)]) ids = Company.includes(:contracts).pluck(:developer_id) diff --git a/activerecord/test/cases/connection_adapters/type_lookup_test.rb b/activerecord/test/cases/connection_adapters/type_lookup_test.rb index 917a04ebc3..1c79d776f0 100644 --- a/activerecord/test/cases/connection_adapters/type_lookup_test.rb +++ b/activerecord/test/cases/connection_adapters/type_lookup_test.rb @@ -82,11 +82,11 @@ unless current_adapter?(:PostgreSQLAdapter) # PostgreSQL does not use type strin end def test_bigint_limit - cast_type = @connection.send(:type_map).lookup("bigint") + limit = @connection.send(:type_map).lookup("bigint").send(:_limit) if current_adapter?(:OracleAdapter) - assert_equal 19, cast_type.limit + assert_equal 19, limit else - assert_equal 8, cast_type.limit + assert_equal 8, limit end end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 70c0ffb3bf..cb29c578b7 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -91,7 +91,7 @@ module ActiveRecord end def test_full_pool_exception - @pool.size.times { @pool.checkout } + @pool.size.times { assert @pool.checkout } assert_raises(ConnectionTimeoutError) do @pool.checkout end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index a602f83d8c..d4408776d3 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -771,6 +771,13 @@ class DirtyTest < ActiveRecord::TestCase assert person.changed? end + test "attributes not selected are still missing after save" do + person = Person.select(:id).first + assert_raises(ActiveModel::MissingAttributeError) { person.first_name } + assert person.save # calls forget_attribute_assignments + assert_raises(ActiveModel::MissingAttributeError) { person.first_name } + end + test "saved_change_to_attribute? returns whether a change occurred in the last save" do person = Person.create!(first_name: "Sean") diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 8369a10b5a..4769ffd64d 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -270,25 +270,27 @@ class FinderTest < ActiveRecord::TestCase end def test_exists_with_includes_limit_and_empty_result - assert_equal false, Topic.includes(:replies).limit(0).exists? - assert_equal false, Topic.includes(:replies).limit(1).where("0 = 1").exists? + assert_no_queries { assert_equal false, Topic.includes(:replies).limit(0).exists? } + assert_queries(1) { assert_equal false, Topic.includes(:replies).limit(1).where("0 = 1").exists? } end def test_exists_with_distinct_association_includes_and_limit author = Author.first - assert_equal false, author.unique_categorized_posts.includes(:special_comments).limit(0).exists? - assert_equal true, author.unique_categorized_posts.includes(:special_comments).limit(1).exists? + unique_categorized_posts = author.unique_categorized_posts.includes(:special_comments) + assert_no_queries { assert_equal false, unique_categorized_posts.limit(0).exists? } + assert_queries(1) { assert_equal true, unique_categorized_posts.limit(1).exists? } end def test_exists_with_distinct_association_includes_limit_and_order author = Author.first - assert_equal false, author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC").limit(0).exists? - assert_equal true, author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC").limit(1).exists? + unique_categorized_posts = author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC") + assert_no_queries { assert_equal false, unique_categorized_posts.limit(0).exists? } + assert_queries(1) { assert_equal true, unique_categorized_posts.limit(1).exists? } end def test_exists_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association - developer = developers(:david) - assert_not_predicate developer.ratings.includes(comment: :post).where(posts: { id: 1 }), :exists? + ratings = developers(:david).ratings.includes(comment: :post).where(posts: { id: 1 }) + assert_queries(1) { assert_not_predicate ratings.limit(1), :exists? } end def test_exists_with_empty_table_and_no_args_given @@ -677,12 +679,34 @@ class FinderTest < ActiveRecord::TestCase assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2) assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) + assert_equal comments.offset(2).to_a.last, comments.offset(2).last + assert_equal comments.offset(2).to_a.last(2), comments.offset(2).last(2) + assert_equal comments.offset(2).to_a.last(3), comments.offset(2).last(3) + comments = comments.offset(1) assert_equal comments.limit(2).to_a.last, comments.limit(2).last assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2) assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) end + def test_first_on_relation_with_limit_and_offset + post = posts("sti_comments") + + comments = post.comments.order(id: :asc) + assert_equal comments.limit(2).to_a.first, comments.limit(2).first + assert_equal comments.limit(2).to_a.first(2), comments.limit(2).first(2) + assert_equal comments.limit(2).to_a.first(3), comments.limit(2).first(3) + + assert_equal comments.offset(2).to_a.first, comments.offset(2).first + assert_equal comments.offset(2).to_a.first(2), comments.offset(2).first(2) + assert_equal comments.offset(2).to_a.first(3), comments.offset(2).first(3) + + comments = comments.offset(1) + assert_equal comments.limit(2).to_a.first, comments.limit(2).first + assert_equal comments.limit(2).to_a.first(2), comments.limit(2).first(2) + assert_equal comments.limit(2).to_a.first(3), comments.limit(2).first(3) + end + def test_take_and_first_and_last_with_integer_should_return_an_array assert_kind_of Array, Topic.take(5) assert_kind_of Array, Topic.first(5) @@ -1049,14 +1073,6 @@ class FinderTest < ActiveRecord::TestCase assert_raise(ArgumentError) { Topic.find_by_title_and_author_name("The First Topic") } end - def test_find_last_with_offset - devs = Developer.order("id") - - assert_equal devs[2], Developer.offset(2).first - assert_equal devs[-3], Developer.offset(2).last - assert_equal devs[-3], Developer.offset(2).order("id DESC").first - end - def test_find_by_nil_attribute topic = Topic.find_by_last_read nil assert_not_nil topic @@ -1301,12 +1317,12 @@ class FinderTest < ActiveRecord::TestCase test "#skip_query_cache! for #exists? with a limited eager load" do Topic.cache do - assert_queries(2) do + assert_queries(1) do Topic.eager_load(:replies).limit(1).exists? Topic.eager_load(:replies).limit(1).exists? end - assert_queries(4) do + assert_queries(2) do Topic.eager_load(:replies).limit(1).skip_query_cache!.exists? Topic.eager_load(:replies).limit(1).skip_query_cache!.exists? end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index b18af2ab55..a3ebc8070a 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -801,8 +801,15 @@ if ActiveRecord::Base.connection.supports_bulk_alter? t.integer :age end - # Adding an index fires a query every time to check if an index already exists or not - assert_queries(3) do + classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] + expected_query_count = { + "Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not + "PostgreSQLAdapter" => 2, + }.fetch(classname) { + raise "need an expected query count for #{classname}" + } + + assert_queries(expected_query_count) do with_bulk_change_table do |t| t.index :username, unique: true, name: :awesome_username_index t.index [:name, :age] @@ -826,7 +833,15 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert index(:index_delete_me_on_name) - assert_queries(3) do + classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] + expected_query_count = { + "Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not + "PostgreSQLAdapter" => 2, + }.fetch(classname) { + raise "need an expected query count for #{classname}" + } + + assert_queries(expected_query_count) do with_bulk_change_table do |t| t.remove_index :name t.index :name, name: :new_name_index, unique: true @@ -848,10 +863,15 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert ! column(:name).default assert_equal :date, column(:birthdate).type - # One query for columns (delete_me table) - # One query for primary key (delete_me table) - # One query to do the bulk change - assert_queries(3, ignore_none: true) do + classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] + expected_query_count = { + "Mysql2Adapter" => 3, # one query for columns, one query for primary key, one query to do the bulk change + "PostgreSQLAdapter" => 2, # one query for columns, one for bulk change + }.fetch(classname) { + raise "need an expected query count for #{classname}" + } + + assert_queries(expected_query_count, ignore_none: true) do with_bulk_change_table do |t| t.change :name, :string, default: "NONAME" t.change :birthdate, :datetime diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 44055e5ab6..e78c3cee8a 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -254,23 +254,34 @@ class ReflectionTest < ActiveRecord::TestCase end def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case - @hotel = Hotel.create! - @department = @hotel.departments.create! - @department.chefs.create!(employable: CakeDesigner.create!) - @department.chefs.create!(employable: DrinkDesigner.create!) + hotel = Hotel.create! + department = hotel.departments.create! + department.chefs.create!(employable: CakeDesigner.create!) + department.chefs.create!(employable: DrinkDesigner.create!) - assert_equal 1, @hotel.cake_designers.size - assert_equal 1, @hotel.drink_designers.size - assert_equal 2, @hotel.chefs.size + assert_equal 1, hotel.cake_designers.size + assert_equal 1, hotel.cake_designers.count + assert_equal 1, hotel.drink_designers.size + assert_equal 1, hotel.drink_designers.count + assert_equal 2, hotel.chefs.size + assert_equal 2, hotel.chefs.count end def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case_and_sti - @hotel = Hotel.create! - @hotel.mocktail_designers << MocktailDesigner.create! + hotel = Hotel.create! + hotel.mocktail_designers << MocktailDesigner.create! + + assert_equal 1, hotel.mocktail_designers.size + assert_equal 1, hotel.mocktail_designers.count + assert_equal 1, hotel.chef_lists.size + assert_equal 1, hotel.chef_lists.count + + hotel.mocktail_designers = [] - assert_equal 1, @hotel.mocktail_designers.size - assert_equal 1, @hotel.mocktail_designers.count - assert_equal 1, @hotel.chef_lists.size + assert_equal 0, hotel.mocktail_designers.size + assert_equal 0, hotel.mocktail_designers.count + assert_equal 0, hotel.chef_lists.size + assert_equal 0, hotel.chef_lists.count end def test_scope_chain_of_polymorphic_association_does_not_leak_into_other_hmt_associations diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index b424ca91de..dbf3389774 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -24,10 +24,9 @@ module ActiveRecord def test_initialize_single_values relation = Relation.new(FakeKlass, :b, nil) - (Relation::SINGLE_VALUE_METHODS - [:create_with, :readonly]).each do |method| + (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end - assert_equal false, relation.readonly_value value = relation.create_with_value assert_equal({}, value) assert_predicate value, :frozen? diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index d8c96316ed..024b5bd8a1 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -116,7 +116,7 @@ module ActiveRecord # instead examining the SQL content. oracle_ignored = [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im, /^\s*select .* from all_constraints/im, /^\s*select .* from all_tab_cols/im, /^\s*select .* from all_sequences/im] mysql_ignored = [/^SHOW FULL TABLES/i, /^SHOW FULL FIELDS/, /^SHOW CREATE TABLE /i, /^SHOW VARIABLES /, /^\s*SELECT (?:column_name|table_name)\b.*\bFROM information_schema\.(?:key_column_usage|tables)\b/im] - postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i] + postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i, /^\s*SELECT\b.*::regtype::oid\b/im] sqlite3_ignored = [/^\s*SELECT name\b.*\bFROM sqlite_master/im, /^\s*SELECT sql\b.*\bFROM sqlite_master/im] [oracle_ignored, mysql_ignored, postgresql_ignored, sqlite3_ignored].each do |db_ignored_sql| diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index cb8686f315..27da886e1c 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -88,6 +88,9 @@ class Author < ActiveRecord::Base has_many :special_categories, through: :special_categorizations, source: :category has_one :special_category, through: :special_categorizations, source: :category + has_many :special_categories_with_conditions, -> { where(categorizations: { special: true }) }, through: :categorizations, source: :category + has_many :nonspecial_categories_with_conditions, -> { where(categorizations: { special: false }) }, through: :categorizations, source: :category + has_many :categories_like_general, -> { where(name: "General") }, through: :categorizations, source: :category, class_name: "Category" has_many :categorized_posts, through: :categorizations, source: :post diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index bbc5fc2b2d..fc6488f729 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -87,6 +87,8 @@ class Firm < Company has_many :association_with_references, -> { references(:foo) }, class_name: "Client" + has_many :developers_with_select, -> { select("id, name, first_name") }, class_name: "Developer" + has_one :lead_developer, class_name: "Developer" has_many :projects diff --git a/activerecord/test/models/tag.rb b/activerecord/test/models/tag.rb index bc13c3a42d..c1a8890a8a 100644 --- a/activerecord/test/models/tag.rb +++ b/activerecord/test/models/tag.rb @@ -11,6 +11,6 @@ end class OrderedTag < Tag self.table_name = "tags" - has_many :taggings, -> { order("taggings.id DESC") }, foreign_key: "tag_id" - has_many :tagged_posts, through: :taggings, source: "taggable", source_type: "Post" + has_many :ordered_taggings, -> { order("taggings.id DESC") }, foreign_key: "tag_id", class_name: "Tagging" + has_many :tagged_posts, through: :ordered_taggings, source: "taggable", source_type: "Post" end |