diff options
Diffstat (limited to 'activerecord')
103 files changed, 1464 insertions, 1968 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4aa7ecfc7e..217eada1d7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,57 @@ +* Add `#up_only` to database migrations for code that is only relevant when + migrating up, e.g. populating a new column. + + *Rich Daley* + +* Require raw SQL fragments to be explicitly marked when used in + relation query methods. + + Before: + ``` + Article.order("LENGTH(title)") + ``` + + After: + ``` + Article.order(Arel.sql("LENGTH(title)")) + ``` + + This prevents SQL injection if applications use the [strongly + discouraged] form `Article.order(params[:my_order])`, under the + mistaken belief that only column names will be accepted. + + Raw SQL strings will now cause a deprecation warning, which will + become an UnknownAttributeReference error in Rails 6.0. Applications + can opt in to the future behavior by setting `allow_unsafe_raw_sql` + to `:disabled`. + + Common and judged-safe string values (such as simple column + references) are unaffected: + ``` + Article.order("title DESC") + ``` + + *Ben Toews* + +* `update_all` will now pass its values to `Type#cast` before passing them to + `Type#serialize`. This means that `update_all(foo: 'true')` will properly + persist a boolean. + + *Sean Griffin* + +* Add new error class `StatementTimeout` which will be raised + when statement timeout exceeded. + + *Ryuta Kamizono* + +* Fix `bin/rails db:migrate` with specified `VERSION`. + `bin/rails db:migrate` with empty VERSION behaves as without `VERSION`. + Check a format of `VERSION`: Allow a migration version number + or name of a migration file. Raise error if format of `VERSION` is invalid. + Raise error if target migration doesn't exist. + + *bogdanvlviv* + * Fixed a bug where column orders for an index weren't written to db/schema.rb when using the sqlite adapter. @@ -136,8 +190,8 @@ *Jeremy Green* -* Add new error class `TransactionTimeout` for MySQL adapter which will be raised - when lock wait time expires. +* Add new error class `TransactionTimeout` which will be raised + when lock wait timeout exceeded. *Gabriel Courtemanche* diff --git a/activerecord/README.rdoc b/activerecord/README.rdoc index ae53ecd177..ba83a9adb2 100644 --- a/activerecord/README.rdoc +++ b/activerecord/README.rdoc @@ -26,7 +26,7 @@ The Product class is automatically mapped to the table named "products", which might look like this: CREATE TABLE products ( - id int NOT NULL auto_increment, + id bigint NOT NULL auto_increment, name varchar(255), PRIMARY KEY (id) ); diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 7ad06fe840..8e42a11df4 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -31,5 +31,5 @@ Gem::Specification.new do |s| s.add_dependency "activesupport", version s.add_dependency "activemodel", version - s.add_dependency "arel", "9.0.0.alpha" + s.add_dependency "arel", ">= 9.0" end diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index bf6dfd46e1..5de6503144 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -27,14 +27,14 @@ require "active_support" require "active_support/rails" require "active_model" require "arel" +require "yaml" require "active_record/version" -require "active_record/attribute_set" +require "active_model/attribute_set" module ActiveRecord extend ActiveSupport::Autoload - autoload :Attribute autoload :Base autoload :Callbacks autoload :Core @@ -104,6 +104,7 @@ module ActiveRecord autoload :Result autoload :TableMetadata + autoload :Type end module Coders @@ -181,3 +182,7 @@ end ActiveSupport.on_load(:i18n) do I18n.load_path << File.expand_path("active_record/locale/en.yml", __dir__) end + +YAML.load_tags["!ruby/object:ActiveRecord::AttributeSet"] = "ActiveModel::AttributeSet" +YAML.load_tags["!ruby/object:ActiveRecord::Attribute::FromDatabase"] = "ActiveModel::Attribute::FromDatabase" +YAML.load_tags["!ruby/object:ActiveRecord::LazyAttributeHash"] = "ActiveModel::LazyAttributeHash" diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 573f41d7ad..661605d3e5 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -140,26 +140,6 @@ module ActiveRecord class HasOneThroughCantAssociateThroughHasOneOrManyReflection < ThroughCantAssociateThroughHasOneOrManyReflection #:nodoc: end - class HasManyThroughCantAssociateNewRecords < ActiveRecordError #:nodoc: - def initialize(owner = nil, reflection = nil) - if owner && reflection - super("Cannot associate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to create the has_many :through record associating them.") - else - super("Cannot associate new records.") - end - end - end - - class HasManyThroughCantDissociateNewRecords < ActiveRecordError #:nodoc: - def initialize(owner = nil, reflection = nil) - if owner && reflection - super("Cannot dissociate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to delete the has_many :through record associating them.") - else - super("Cannot dissociate new records.") - end - end - end - class ThroughNestedAssociationsAreReadonly < ActiveRecordError #:nodoc: def initialize(owner = nil, reflection = nil) if owner && reflection @@ -189,16 +169,6 @@ module ActiveRecord end end - class ReadOnlyAssociation < ActiveRecordError #:nodoc: - def initialize(reflection = nil) - if reflection - super("Cannot add to a has_many :through association. Try adding to #{reflection.through_reflection.name.inspect}.") - else - super("Read-only reflection error.") - end - end - end - # This error is raised when trying to destroy a parent instance in N:1 or 1:1 associations # (has_many, has_one) when there is at least 1 child associated instance. # ex: if @project.tasks.size > 0, DeleteRestrictionError will be raised when trying to destroy @project @@ -483,14 +453,14 @@ module ActiveRecord # The tables for these classes could look something like: # # CREATE TABLE users ( - # id int NOT NULL auto_increment, - # account_id int default NULL, + # id bigint NOT NULL auto_increment, + # account_id bigint default NULL, # name varchar default NULL, # PRIMARY KEY (id) # ) # # CREATE TABLE accounts ( - # id int NOT NULL auto_increment, + # id bigint NOT NULL auto_increment, # name varchar default NULL, # PRIMARY KEY (id) # ) diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 07f7303f8d..8b4a48a38c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -988,6 +988,12 @@ module ActiveRecord load_target == other end + ## + # :method: to_ary + # + # :call-seq: + # to_ary() + # # Returns a new array of objects from the collection. If the collection # hasn't been loaded, it fetches the records from the database. # @@ -1021,10 +1027,6 @@ module ActiveRecord # # #<Pet id: 5, name: "Brain", person_id: 1>, # # #<Pet id: 6, name: "Boss", person_id: 1> # # ] - def to_ary - load_target.dup - end - alias_method :to_a, :to_ary def records # :nodoc: load_target diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index e1754d4a19..e1087be9b3 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -44,16 +44,8 @@ module ActiveRecord extend ActiveSupport::Autoload eager_autoload do - autoload :Association, "active_record/associations/preloader/association" - autoload :SingularAssociation, "active_record/associations/preloader/singular_association" - autoload :CollectionAssociation, "active_record/associations/preloader/collection_association" - autoload :ThroughAssociation, "active_record/associations/preloader/through_association" - - autoload :HasMany, "active_record/associations/preloader/has_many" - autoload :HasManyThrough, "active_record/associations/preloader/has_many_through" - autoload :HasOne, "active_record/associations/preloader/has_one" - autoload :HasOneThrough, "active_record/associations/preloader/has_one_through" - autoload :BelongsTo, "active_record/associations/preloader/belongs_to" + autoload :Association, "active_record/associations/preloader/association" + autoload :ThroughAssociation, "active_record/associations/preloader/through_association" end # Eager loads the named associations for the given Active Record record(s). @@ -166,8 +158,6 @@ module ActiveRecord end class AlreadyLoaded # :nodoc: - attr_reader :owners, :reflection - def initialize(klass, owners, reflection, preload_scope) @owners = owners @reflection = reflection @@ -178,11 +168,13 @@ module ActiveRecord def preloaded_records owners.flat_map { |owner| owner.association(reflection.name).target } end + + protected + attr_reader :owners, :reflection 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 + # and attach it to a relation. The class returned implements a `run` method # that accepts a preloader. def preloader_for(reflection, owners) if owners.first.association(reflection.name).loaded? @@ -190,13 +182,10 @@ module ActiveRecord end reflection.check_preloadable! - case reflection.macro - when :has_many - reflection.options[:through] ? HasManyThrough : HasMany - when :has_one - reflection.options[:through] ? HasOneThrough : HasOne - when :belongs_to - BelongsTo + if reflection.options[:through] + ThroughAssociation + else + Association end end end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 607d376a08..735da152b7 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -4,7 +4,6 @@ module ActiveRecord module Associations class Preloader class Association #:nodoc: - attr_reader :owners, :reflection, :preload_scope, :model, :klass attr_reader :preloaded_records def initialize(klass, owners, reflection, preload_scope) @@ -17,11 +16,20 @@ module ActiveRecord end def run(preloader) - associated_records_by_owner(preloader).each do |owner, records| - associate_records_to_owner(owner, records) + records = load_records do |record| + owner = owners_by_key[convert_key(record[association_key_name])] + association = owner.association(reflection.name) + association.set_inverse_instance(record) + end + + owners.each do |owner| + associate_records_to_owner(owner, records[convert_key(owner[owner_key_name])] || []) end end + protected + attr_reader :owners, :reflection, :preload_scope, :model, :klass + private # The name of the key on the associated records def association_key_name @@ -33,20 +41,14 @@ module ActiveRecord reflection.join_foreign_key end - def associated_records_by_owner(preloader) - records = load_records do |record| - owner = owners_by_key[convert_key(record[association_key_name])] - association = owner.association(reflection.name) - association.set_inverse_instance(record) - end - - owners.each_with_object({}) do |owner, result| - result[owner] = records[convert_key(owner[owner_key_name])] || [] - end - end - def associate_records_to_owner(owner, records) - raise NotImplementedError + association = owner.association(reflection.name) + if reflection.collection? + association.loaded! + association.target.concat(records) + else + association.target = records.first + end end def owner_keys diff --git a/activerecord/lib/active_record/associations/preloader/belongs_to.rb b/activerecord/lib/active_record/associations/preloader/belongs_to.rb deleted file mode 100644 index a8e3340b23..0000000000 --- a/activerecord/lib/active_record/associations/preloader/belongs_to.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class BelongsTo < SingularAssociation #:nodoc: - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb deleted file mode 100644 index fc2029f54a..0000000000 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class CollectionAssociation < Association #:nodoc: - private - def associate_records_to_owner(owner, records) - association = owner.association(reflection.name) - association.loaded! - association.target.concat(records) - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_many.rb b/activerecord/lib/active_record/associations/preloader/has_many.rb deleted file mode 100644 index 72f55bc43f..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_many.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasMany < CollectionAssociation #:nodoc: - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb deleted file mode 100644 index 0639fdca44..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasManyThrough < CollectionAssociation #:nodoc: - include ThroughAssociation - - def associated_records_by_owner(preloader) - records_by_owner = super - - if reflection_scope.distinct_value - records_by_owner.each_value(&:uniq!) - end - - records_by_owner - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_one.rb b/activerecord/lib/active_record/associations/preloader/has_one.rb deleted file mode 100644 index e339b65fb5..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_one.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasOne < SingularAssociation #:nodoc: - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_one_through.rb b/activerecord/lib/active_record/associations/preloader/has_one_through.rb deleted file mode 100644 index 17734d0257..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_one_through.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasOneThrough < SingularAssociation #:nodoc: - include ThroughAssociation - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb deleted file mode 100644 index 30a92411e3..0000000000 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class SingularAssociation < Association #:nodoc: - private - def associate_records_to_owner(owner, records) - association = owner.association(reflection.name) - association.target = records.first - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index fa32cc5553..a6b7ab80a2 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -3,78 +3,54 @@ module ActiveRecord module Associations class Preloader - module ThroughAssociation #:nodoc: - def through_reflection - reflection.through_reflection - end - - def source_reflection - reflection.source_reflection - end - - def associated_records_by_owner(preloader) - through_scope = through_scope() - - preloader.preload(owners, - through_reflection.name, - through_scope) - - through_records = owners.map do |owner| - center = owner.association(through_reflection.name).target - [owner, Array(center)] - end - - reset_association(owners, through_reflection.name, through_scope) - - middle_records = through_records.flat_map(&:last) - - reflection_scope = reflection_scope() if reflection.scope - - preloaders = preloader.preload(middle_records, - source_reflection.name, - reflection_scope) - + class ThroughAssociation < Association # :nodoc: + def run(preloader) + already_loaded = owners.first.association(through_reflection.name).loaded? + through_scope = through_scope() + reflection_scope = target_reflection_scope + through_preloaders = preloader.preload(owners, through_reflection.name, through_scope) + middle_records = through_preloaders.flat_map(&:preloaded_records) + preloaders = preloader.preload(middle_records, source_reflection.name, reflection_scope) @preloaded_records = preloaders.flat_map(&:preloaded_records) - middle_to_pl = preloaders.each_with_object({}) do |pl, h| - pl.owners.each { |middle| - h[middle] = pl - } - end - - through_records.each_with_object({}) do |(lhs, center), records_by_owner| - pl_to_middle = center.group_by { |record| middle_to_pl[record] } - - records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| - rhs_records = middles.flat_map { |r| - r.association(source_reflection.name).target - }.compact - - # Respect the order on `reflection_scope` if it exists, else use the natural order. - if reflection_scope && !reflection_scope.order_values.empty? - @id_map ||= id_to_index_map @preloaded_records - rhs_records.sort_by { |rhs| @id_map[rhs] } - else - rhs_records + owners.each do |owner| + through_records = Array(owner.association(through_reflection.name).target) + if already_loaded + if source_type = reflection.options[:source_type] + through_records = through_records.select do |record| + record[reflection.foreign_type] == source_type + end end + else + owner.association(through_reflection.name).reset if through_scope + end + result = through_records.flat_map do |record| + association = record.association(source_reflection.name) + target = association.target + association.reset if preload_scope + target end + result.compact! + if reflection_scope + result.sort_by! { |rhs| preload_index[rhs] } if reflection_scope.order_values.any? + result.uniq! if reflection_scope.distinct_value + end + associate_records_to_owner(owner, result) end end private + def through_reflection + reflection.through_reflection + end - def id_to_index_map(ids) - id_map = {} - ids.each_with_index { |id, index| id_map[id] = index } - id_map + def source_reflection + reflection.source_reflection end - def reset_association(owners, association_name, should_reset) - # Don't cache the association - we would only be caching a subset - if should_reset - owners.each { |owner| - owner.association(association_name).reset - } + def preload_index + @preload_index ||= @preloaded_records.each_with_object({}).with_index do |(id, result), index| + result[id] = index end end @@ -115,6 +91,16 @@ module ActiveRecord scope unless scope.empty_scope? end + + def target_reflection_scope + if preload_scope + reflection_scope.merge(preload_scope) + elsif reflection.scope + reflection_scope + else + nil + end + end end end end diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb deleted file mode 100644 index fc474edc15..0000000000 --- a/activerecord/lib/active_record/attribute.rb +++ /dev/null @@ -1,242 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - class Attribute # :nodoc: - class << self - def from_database(name, value, type) - FromDatabase.new(name, value, type) - end - - def from_user(name, value, type, original_attribute = nil) - FromUser.new(name, value, type, original_attribute) - end - - def with_cast_value(name, value, type) - WithCastValue.new(name, value, type) - end - - def null(name) - Null.new(name) - end - - def uninitialized(name, type) - Uninitialized.new(name, type) - end - end - - attr_reader :name, :value_before_type_cast, :type - - # This method should not be called directly. - # Use #from_database or #from_user - def initialize(name, value_before_type_cast, type, original_attribute = nil) - @name = name - @value_before_type_cast = value_before_type_cast - @type = type - @original_attribute = original_attribute - end - - def value - # `defined?` is cheaper than `||=` when we get back falsy values - @value = type_cast(value_before_type_cast) unless defined?(@value) - @value - end - - def original_value - if assigned? - original_attribute.original_value - else - type_cast(value_before_type_cast) - end - end - - def value_for_database - type.serialize(value) - end - - def changed? - changed_from_assignment? || changed_in_place? - end - - def changed_in_place? - has_been_read? && type.changed_in_place?(original_value_for_database, value) - end - - def forgetting_assignment - with_value_from_database(value_for_database) - end - - def with_value_from_user(value) - type.assert_valid_value(value) - self.class.from_user(name, value, type, original_attribute || self) - end - - def with_value_from_database(value) - self.class.from_database(name, value, type) - end - - def with_cast_value(value) - self.class.with_cast_value(name, value, type) - end - - def with_type(type) - if changed_in_place? - with_value_from_user(value).with_type(type) - else - self.class.new(name, value_before_type_cast, type, original_attribute) - end - end - - def type_cast(*) - raise NotImplementedError - end - - def initialized? - true - end - - def came_from_user? - false - end - - def has_been_read? - defined?(@value) - end - - def ==(other) - self.class == other.class && - name == other.name && - value_before_type_cast == other.value_before_type_cast && - type == other.type - end - alias eql? == - - def hash - [self.class, name, value_before_type_cast, type].hash - end - - def init_with(coder) - @name = coder["name"] - @value_before_type_cast = coder["value_before_type_cast"] - @type = coder["type"] - @original_attribute = coder["original_attribute"] - @value = coder["value"] if coder.map.key?("value") - end - - def encode_with(coder) - coder["name"] = name - coder["value_before_type_cast"] = value_before_type_cast unless value_before_type_cast.nil? - coder["type"] = type if type - coder["original_attribute"] = original_attribute if original_attribute - coder["value"] = value if defined?(@value) - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :original_attribute - alias_method :assigned?, :original_attribute - - def original_value_for_database - if assigned? - original_attribute.original_value_for_database - else - _original_value_for_database - end - end - - private - def initialize_dup(other) - if defined?(@value) && @value.duplicable? - @value = @value.dup - end - end - - def changed_from_assignment? - assigned? && type.changed?(original_value, value, value_before_type_cast) - end - - def _original_value_for_database - type.serialize(original_value) - end - - class FromDatabase < Attribute # :nodoc: - def type_cast(value) - type.deserialize(value) - end - - def _original_value_for_database - value_before_type_cast - end - end - - class FromUser < Attribute # :nodoc: - def type_cast(value) - type.cast(value) - end - - def came_from_user? - !type.value_constructed_by_mass_assignment?(value_before_type_cast) - end - end - - class WithCastValue < Attribute # :nodoc: - def type_cast(value) - value - end - - def changed_in_place? - false - end - end - - class Null < Attribute # :nodoc: - def initialize(name) - super(name, nil, Type.default_value) - end - - def type_cast(*) - nil - end - - def with_type(type) - self.class.with_cast_value(name, nil, type) - end - - def with_value_from_database(value) - raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{name}`" - end - alias_method :with_value_from_user, :with_value_from_database - end - - class Uninitialized < Attribute # :nodoc: - UNINITIALIZED_ORIGINAL_VALUE = Object.new - - def initialize(name, type) - super(name, nil, type) - end - - def value - if block_given? - yield name - end - end - - def original_value - UNINITIALIZED_ORIGINAL_VALUE - end - - def value_for_database - end - - def initialized? - false - end - - def with_type(type) - self.class.new(name, type) - end - end - private_constant :FromDatabase, :FromUser, :Null, :Uninitialized, :WithCastValue - end -end diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activerecord/lib/active_record/attribute/user_provided_default.rb deleted file mode 100644 index f746960fac..0000000000 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require "active_record/attribute" - -module ActiveRecord - class Attribute # :nodoc: - class UserProvidedDefault < FromUser # :nodoc: - def initialize(name, value, type, database_default) - @user_provided_value = value - super(name, value, type, database_default) - end - - def value_before_type_cast - if user_provided_value.is_a?(Proc) - @memoized_value_before_type_cast ||= user_provided_value.call - else - @user_provided_value - end - end - - def with_type(type) - self.class.new(name, user_provided_value, type, original_attribute) - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :user_provided_value - end - end -end diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 891e556dc4..64f81ca582 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -167,6 +167,46 @@ module ActiveRecord end end + # Regexp whitelist. Matches the following: + # "#{table_name}.#{column_name}" + # "#{column_name}" + COLUMN_NAME_WHITELIST = /\A(?:\w+\.)?\w+\z/i + + # Regexp whitelist. Matches the following: + # "#{table_name}.#{column_name}" + # "#{table_name}.#{column_name} #{direction}" + # "#{column_name}" + # "#{column_name} #{direction}" + COLUMN_NAME_ORDER_WHITELIST = /\A(?:\w+\.)?\w+(?:\s+asc|\s+desc)?\z/i + + def enforce_raw_sql_whitelist(args, whitelist: COLUMN_NAME_WHITELIST) # :nodoc: + unexpected = args.reject do |arg| + arg.kind_of?(Arel::Node) || + arg.is_a?(Arel::Nodes::SqlLiteral) || + arg.is_a?(Arel::Attributes::Attribute) || + arg.to_s.split(/\s*,\s*/).all? { |part| whitelist.match?(part) } + end + + return if unexpected.none? + + if allow_unsafe_raw_sql == :deprecated + ActiveSupport::Deprecation.warn( + "Dangerous query method (method whose arguments are used as raw " \ + "SQL) called with non-attribute argument(s): " \ + "#{unexpected.map(&:inspect).join(", ")}. Non-attribute " \ + "arguments will be disallowed in Rails 6.0. This method should " \ + "not be called with user-provided values, such as request " \ + "parameters or model attributes. Known-safe values can be passed " \ + "by wrapping them in Arel.sql()." + ) + else + raise(ActiveRecord::UnknownAttributeReference, + "Query method called with non-attribute argument(s): " + + unexpected.map(&:inspect).join(", ") + ) + end + end + # Returns true if the given attribute exists, otherwise false. # # class Person < ActiveRecord::Base @@ -236,7 +276,7 @@ module ActiveRecord return has_attribute?(name) end - return true + true end # Returns +true+ if the given attribute is in the attributes hash, otherwise +false+. diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 79110d04f4..3de6fe566d 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/module/attribute_accessors" -require "active_record/attribute_mutation_tracker" module ActiveRecord module AttributeMethods @@ -33,65 +32,13 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new @mutations_from_database = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new end end - def initialize_dup(other) # :nodoc: - super - @attributes = self.class._default_attributes.map do |attr| - attr.with_value_from_user(@attributes.fetch_value(attr.name)) - end - @mutations_from_database = nil - end - - def changes_applied # :nodoc: - @mutations_before_last_save = mutations_from_database - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_changes_information # :nodoc: - @mutations_before_last_save = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_attribute_changes(attr_names) # :nodoc: - super - attr_names.each do |attr_name| - clear_attribute_change(attr_name) - end - end - - def changed_attributes # :nodoc: - # This should only be set by methods which will call changed_attributes - # multiple times when it is known that the computed value cannot change. - if defined?(@cached_changed_attributes) - @cached_changed_attributes - else - super.reverse_merge(mutations_from_database.changed_values).freeze - end - end - - def changes # :nodoc: - cache_changed_attributes do - super - end - end - - def previous_changes # :nodoc: - mutations_before_last_save.changes - end - - def attribute_changed_in_place?(attr_name) # :nodoc: - mutations_from_database.changed_in_place?(attr_name) - end - # Did this attribute change when we last saved? This method can be invoked # as +saved_change_to_name?+ instead of <tt>saved_change_to_attribute?("name")</tt>. # Behaves similarly to +attribute_changed?+. This method is useful in @@ -182,26 +129,6 @@ module ActiveRecord result end - def mutations_from_database - unless defined?(@mutations_from_database) - @mutations_from_database = nil - end - @mutations_from_database ||= AttributeMutationTracker.new(@attributes) - end - - def changes_include?(attr_name) - super || mutations_from_database.changed?(attr_name) - end - - def clear_attribute_change(attr_name) - mutations_from_database.forget_change(attr_name) - end - - def attribute_will_change!(attr_name) - super - mutations_from_database.force_change(attr_name) - end - def _update_record(*) partial_writes? ? super(keys_for_partial_write) : super end @@ -213,25 +140,6 @@ module ActiveRecord def keys_for_partial_write changed_attribute_names_to_save & self.class.column_names end - - def forget_attribute_assignments - @attributes = @attributes.map(&:forgetting_assignment) - end - - def mutations_before_last_save - @mutations_before_last_save ||= NullMutationTracker.instance - end - - def cache_changed_attributes - @cached_changed_attributes = changed_attributes - yield - ensure - clear_changed_attributes_cache - end - - def clear_changed_attributes_cache - remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) - end end end end diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb deleted file mode 100644 index 94bf641a5d..0000000000 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ /dev/null @@ -1,111 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - class AttributeMutationTracker # :nodoc: - OPTION_NOT_GIVEN = Object.new - - def initialize(attributes) - @attributes = attributes - @forced_changes = Set.new - end - - def changed_values - attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| - if changed?(attr_name) - result[attr_name] = attributes[attr_name].original_value - end - end - end - - def changes - attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| - change = change_to_attribute(attr_name) - if change - result[attr_name] = change - end - end - end - - def change_to_attribute(attr_name) - attr_name = attr_name.to_s - if changed?(attr_name) - [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] - end - end - - def any_changes? - attr_names.any? { |attr| changed?(attr) } - end - - def changed?(attr_name, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) - attr_name = attr_name.to_s - forced_changes.include?(attr_name) || - attributes[attr_name].changed? && - (OPTION_NOT_GIVEN == from || attributes[attr_name].original_value == from) && - (OPTION_NOT_GIVEN == to || attributes[attr_name].value == to) - end - - def changed_in_place?(attr_name) - attributes[attr_name.to_s].changed_in_place? - end - - def forget_change(attr_name) - attr_name = attr_name.to_s - attributes[attr_name] = attributes[attr_name].forgetting_assignment - forced_changes.delete(attr_name) - end - - def original_value(attr_name) - attributes[attr_name.to_s].original_value - end - - def force_change(attr_name) - forced_changes << attr_name.to_s - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :attributes, :forced_changes - - private - - def attr_names - attributes.keys - end - end - - class NullMutationTracker # :nodoc: - include Singleton - - def changed_values(*) - {} - end - - def changes(*) - {} - end - - def change_to_attribute(attr_name) - end - - def any_changes?(*) - false - end - - def changed?(*) - false - end - - def changed_in_place?(*) - false - end - - def forget_change(*) - end - - def original_value(*) - end - end -end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb deleted file mode 100644 index 9785666e77..0000000000 --- a/activerecord/lib/active_record/attribute_set.rb +++ /dev/null @@ -1,113 +0,0 @@ -# frozen_string_literal: true - -require "active_record/attribute_set/builder" -require "active_record/attribute_set/yaml_encoder" - -module ActiveRecord - class AttributeSet # :nodoc: - delegate :each_value, :fetch, to: :attributes - - def initialize(attributes) - @attributes = attributes - end - - def [](name) - attributes[name] || Attribute.null(name) - end - - def []=(name, value) - attributes[name] = value - end - - def values_before_type_cast - attributes.transform_values(&:value_before_type_cast) - end - - def to_hash - initialized_attributes.transform_values(&:value) - end - alias_method :to_h, :to_hash - - def key?(name) - attributes.key?(name) && self[name].initialized? - end - - def keys - attributes.each_key.select { |name| self[name].initialized? } - end - - if defined?(JRUBY_VERSION) - # This form is significantly faster on JRuby, and this is one of our biggest hotspots. - # https://github.com/jruby/jruby/pull/2562 - def fetch_value(name, &block) - self[name].value(&block) - end - else - def fetch_value(name) - self[name].value { |n| yield n if block_given? } - end - end - - def write_from_database(name, value) - attributes[name] = self[name].with_value_from_database(value) - end - - def write_from_user(name, value) - attributes[name] = self[name].with_value_from_user(value) - end - - def write_cast_value(name, value) - attributes[name] = self[name].with_cast_value(value) - end - - def freeze - @attributes.freeze - super - end - - def deep_dup - self.class.allocate.tap do |copy| - copy.instance_variable_set(:@attributes, attributes.deep_dup) - end - end - - def initialize_dup(_) - @attributes = attributes.dup - super - end - - def initialize_clone(_) - @attributes = attributes.clone - super - end - - def reset(key) - if key?(key) - write_from_database(key, nil) - end - end - - def accessed - attributes.select { |_, attr| attr.has_been_read? }.keys - end - - def map(&block) - new_attributes = attributes.transform_values(&block) - AttributeSet.new(new_attributes) - end - - def ==(other) - attributes == other.attributes - end - - protected - - attr_reader :attributes - - private - - def initialized_attributes - attributes.select { |_, attr| attr.initialized? } - end - end -end diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb deleted file mode 100644 index 349cc7e403..0000000000 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -require "active_record/attribute" - -module ActiveRecord - class AttributeSet # :nodoc: - class Builder # :nodoc: - attr_reader :types, :always_initialized, :default - - def initialize(types, always_initialized = nil, &default) - @types = types - @always_initialized = always_initialized - @default = default - end - - def build_from_database(values = {}, additional_types = {}) - if always_initialized && !values.key?(always_initialized) - values[always_initialized] = nil - end - - attributes = LazyAttributeHash.new(types, values, additional_types, &default) - AttributeSet.new(attributes) - end - end - end - - class LazyAttributeHash # :nodoc: - delegate :transform_values, :each_key, :each_value, :fetch, to: :materialize - - def initialize(types, values, additional_types, &default) - @types = types - @values = values - @additional_types = additional_types - @materialized = false - @delegate_hash = {} - @default = default || proc {} - end - - def key?(key) - delegate_hash.key?(key) || values.key?(key) || types.key?(key) - end - - def [](key) - delegate_hash[key] || assign_default_value(key) - end - - def []=(key, value) - if frozen? - raise RuntimeError, "Can't modify frozen hash" - end - delegate_hash[key] = value - end - - def deep_dup - dup.tap do |copy| - copy.instance_variable_set(:@delegate_hash, delegate_hash.transform_values(&:dup)) - end - end - - def initialize_dup(_) - @delegate_hash = Hash[delegate_hash] - super - end - - def select - keys = types.keys | values.keys | delegate_hash.keys - keys.each_with_object({}) do |key, hash| - attribute = self[key] - if yield(key, attribute) - hash[key] = attribute - end - end - end - - def ==(other) - if other.is_a?(LazyAttributeHash) - materialize == other.materialize - else - materialize == other - end - end - - def marshal_dump - materialize - end - - def marshal_load(delegate_hash) - @delegate_hash = delegate_hash - @types = {} - @values = {} - @additional_types = {} - @materialized = true - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :types, :values, :additional_types, :delegate_hash, :default - - def materialize - unless @materialized - values.each_key { |key| self[key] } - types.each_key { |key| self[key] } - unless frozen? - @materialized = true - end - end - delegate_hash - end - - private - - def assign_default_value(name) - type = additional_types.fetch(name, types[name]) - value_present = true - value = values.fetch(name) { value_present = false } - - if value_present - delegate_hash[name] = Attribute.from_database(name, value, type) - elsif types.key?(name) - delegate_hash[name] = default.call(name) || Attribute.uninitialized(name, type) - end - end - end -end diff --git a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb b/activerecord/lib/active_record/attribute_set/yaml_encoder.rb deleted file mode 100644 index 9254ce16ab..0000000000 --- a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - class AttributeSet - # Attempts to do more intelligent YAML dumping of an - # ActiveRecord::AttributeSet to reduce the size of the resulting string - class YAMLEncoder # :nodoc: - def initialize(default_types) - @default_types = default_types - end - - def encode(attribute_set, coder) - coder["concise_attributes"] = attribute_set.each_value.map do |attr| - if attr.type.equal?(default_types[attr.name]) - attr.with_type(nil) - else - attr - end - end - end - - def decode(coder) - if coder["attributes"] - coder["attributes"] - else - attributes_hash = Hash[coder["concise_attributes"].map do |attr| - if attr.type.nil? - attr = attr.with_type(default_types[attr.name]) - end - [attr.name, attr] - end] - AttributeSet.new(attributes_hash) - end - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :default_types - end - end -end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 9224d58928..0b7c9398a8 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "active_record/attribute/user_provided_default" +require "active_model/attribute/user_provided_default" module ActiveRecord # See ActiveRecord::Attributes::ClassMethods for documentation @@ -250,14 +250,14 @@ module ActiveRecord if value == NO_DEFAULT_PROVIDED default_attribute = _default_attributes[name].with_type(type) elsif from_user - default_attribute = Attribute::UserProvidedDefault.new( + default_attribute = ActiveModel::Attribute::UserProvidedDefault.new( name, value, type, _default_attributes.fetch(name.to_s) { nil }, ) else - default_attribute = Attribute.from_database(name, value, type) + default_attribute = ActiveModel::Attribute.from_database(name, value, type) end _default_attributes[name] = default_attribute end diff --git a/activerecord/lib/active_record/collection_cache_key.rb b/activerecord/lib/active_record/collection_cache_key.rb index f3e6516414..88b398ad45 100644 --- a/activerecord/lib/active_record/collection_cache_key.rb +++ b/activerecord/lib/active_record/collection_cache_key.rb @@ -12,6 +12,9 @@ module ActiveRecord timestamp = collection.max_by(×tamp_column)._read_attribute(timestamp_column) end else + if collection.eager_loading? + collection = collection.send(:apply_join_dependency) + end column_type = type_for_attribute(timestamp_column.to_s) column = connection.column_name_from_arel_node(collection.arel_attribute(timestamp_column)) select_values = "COUNT(*) AS #{connection.quote_column_name("size")}, MAX(%s) AS timestamp" 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 0759f4d2b3..c5013dc1ee 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -82,11 +82,8 @@ module ActiveRecord # * private methods that require being called in a +synchronize+ blocks # are now explicitly documented class ConnectionPool - # Threadsafe, fair, FIFO queue. Meant to be used by ConnectionPool - # with which it shares a Monitor. But could be a generic Queue. - # - # The Queue in stdlib's 'thread' could replace this class except - # stdlib's doesn't support waiting with a timeout. + # Threadsafe, fair, LIFO queue. Meant to be used by ConnectionPool + # with which it shares a Monitor. class Queue def initialize(lock = Monitor.new) @lock = lock @@ -175,7 +172,7 @@ module ActiveRecord # Removes and returns the head of the queue if possible, or +nil+. def remove - @queue.shift + @queue.pop end # Remove and return the head the queue if the number of @@ -450,6 +447,21 @@ module ActiveRecord disconnect(false) end + # Discards all connections in the pool (even if they're currently + # leased!), along with the pool itself. Any further interaction with the + # pool (except #spec and #schema_cache) is undefined. + # + # See AbstractAdapter#discard! + def discard! # :nodoc: + synchronize do + return if @connections.nil? # already discarded + @connections.each do |conn| + conn.discard! + end + @connections = @available = @thread_cached_conns = nil + end + end + # Clears the cache which maps classes and re-connects connections that # require reloading. # @@ -866,11 +878,31 @@ module ActiveRecord # about the model. The model needs to pass a specification name to the handler, # in order to look up the correct connection pool. class ConnectionHandler + def self.unowned_pool_finalizer(pid_map) # :nodoc: + lambda do |_| + discard_unowned_pools(pid_map) + end + end + + def self.discard_unowned_pools(pid_map) # :nodoc: + pid_map.each do |pid, pools| + pools.values.compact.each(&:discard!) unless pid == Process.pid + end + end + def initialize # These caches are keyed by spec.name (ConnectionSpecification#name). @owner_to_pool = Concurrent::Map.new(initial_capacity: 2) do |h, k| + # Discard the parent's connection pools immediately; we have no need + # of them + ConnectionHandler.discard_unowned_pools(h) + h[k] = Concurrent::Map.new(initial_capacity: 2) end + + # Backup finalizer: if the forked child never needed a pool, the above + # early discard has not occurred + ObjectSpace.define_finalizer self, ConnectionHandler.unowned_pool_finalizer(@owner_to_pool) end def connection_pool_list diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 6698047ad6..9b7345f7c3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -216,7 +216,7 @@ module ActiveRecord # generates: # # CREATE TABLE suppliers ( - # id int auto_increment PRIMARY KEY + # id bigint auto_increment PRIMARY KEY # ) ENGINE=InnoDB DEFAULT CHARSET=utf8 # # ====== Rename the primary key column @@ -228,7 +228,7 @@ module ActiveRecord # generates: # # CREATE TABLE objects ( - # guid int auto_increment PRIMARY KEY, + # guid bigint auto_increment PRIMARY KEY, # name varchar(80) # ) # @@ -255,8 +255,8 @@ module ActiveRecord # generates: # # CREATE TABLE order ( - # product_id integer NOT NULL, - # client_id integer NOT NULL + # product_id bigint NOT NULL, + # client_id bigint NOT NULL # ); # # ALTER TABLE ONLY "orders" @@ -265,15 +265,15 @@ module ActiveRecord # ====== Do not add a primary key column # # create_table(:categories_suppliers, id: false) do |t| - # t.column :category_id, :integer - # t.column :supplier_id, :integer + # t.column :category_id, :bigint + # t.column :supplier_id, :bigint # end # # generates: # # CREATE TABLE categories_suppliers ( - # category_id int, - # supplier_id int + # category_id bigint, + # supplier_id bigint # ) # # ====== Create a temporary table based on a query @@ -361,8 +361,8 @@ module ActiveRecord # generates: # # CREATE TABLE assemblies_parts ( - # assembly_id int NOT NULL, - # part_id int NOT NULL, + # assembly_id bigint NOT NULL, + # part_id bigint NOT NULL, # ) ENGINE=InnoDB DEFAULT CHARSET=utf8 # def create_join_table(table_1, table_2, column_options: {}, **options) @@ -432,7 +432,7 @@ module ActiveRecord # t.references :company # end # - # Creates a <tt>company_id(integer)</tt> column. + # Creates a <tt>company_id(bigint)</tt> column. # # ====== Add a polymorphic foreign key column # @@ -440,7 +440,7 @@ module ActiveRecord # t.belongs_to :company, polymorphic: true # end # - # Creates <tt>company_type(varchar)</tt> and <tt>company_id(integer)</tt> columns. + # Creates <tt>company_type(varchar)</tt> and <tt>company_id(bigint)</tt> columns. # # ====== Remove a column # @@ -811,14 +811,14 @@ module ActiveRecord indexes(table_name).detect { |i| i.name == index_name } end - # Adds a reference. The reference column is an integer by default, + # Adds a reference. The reference column is a bigint by default, # the <tt>:type</tt> option can be used to specify a different type. # Optionally adds a +_type+ column, if <tt>:polymorphic</tt> option is provided. # #add_reference and #add_belongs_to are acceptable. # # The +options+ hash can include the following keys: # [<tt>:type</tt>] - # The reference column type. Defaults to +:integer+. + # The reference column type. Defaults to +:bigint+. # [<tt>:index</tt>] # Add an appropriate index. Defaults to true. # See #add_index for usage of this option. @@ -829,7 +829,7 @@ module ActiveRecord # [<tt>:null</tt>] # Whether the column allows nulls. Defaults to true. # - # ====== Create a user_id integer column + # ====== Create a user_id bigint column # # add_reference(:products, :user) # diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 147e16e9fa..d9ac8db6a8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -240,7 +240,7 @@ module ActiveRecord rollback_transaction if transaction else begin - commit_transaction + commit_transaction if transaction rescue Exception rollback_transaction(transaction) unless transaction.state.completed? raise diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index e3aab8dad8..5411a6a262 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -require "active_record/type" require "active_record/connection_adapters/determine_if_preparable_visitor" require "active_record/connection_adapters/schema_cache" require "active_record/connection_adapters/sql_type_metadata" require "active_record/connection_adapters/abstract/schema_dumper" require "active_record/connection_adapters/abstract/schema_creation" +require "active_support/concurrency/load_interlock_aware_monitor" require "arel/collectors/bind" require "arel/collectors/composite" require "arel/collectors/sql_string" @@ -108,7 +108,7 @@ module ActiveRecord @schema_cache = SchemaCache.new self @quoted_column_names, @quoted_table_names = {}, {} @visitor = arel_visitor - @lock = Monitor.new + @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true @@ -367,6 +367,19 @@ module ActiveRecord reset_transaction end + # Immediately forget this connection ever existed. Unlike disconnect!, + # this will not communicate with the server. + # + # After calling this method, the behavior of all other methods becomes + # undefined. This is called internally just before a forked process gets + # rid of a connection that belonged to its parent. + def discard! + # This should be overridden by concrete adapters. + # + # Prevent @connection's finalizer from touching the socket, or + # otherwise communicating with its server, when it is collected. + end + # Reset the state of this connection, directing the DBMS to clear # transactions and other connection-related server-side state. Usually a # database-dependent operation. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index bfec6fb784..ca651ef390 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -635,6 +635,7 @@ module ActiveRecord ER_CANNOT_ADD_FOREIGN = 1215 ER_CANNOT_CREATE_TABLE = 1005 ER_LOCK_WAIT_TIMEOUT = 1205 + ER_QUERY_TIMEOUT = 3024 def translate_exception(exception, message) case error_number(exception) @@ -660,6 +661,8 @@ module ActiveRecord Deadlocked.new(message) when ER_LOCK_WAIT_TIMEOUT TransactionTimeout.new(message) + when ER_QUERY_TIMEOUT + StatementTimeout.new(message) else super end diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 28d949b503..5d81de9fe1 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -11,7 +11,7 @@ module ActiveRecord # Instantiates a new column in the table. # - # +name+ is the column's name, such as <tt>supplier_id</tt> in <tt>supplier_id int</tt>. + # +name+ is the column's name, such as <tt>supplier_id</tt> in <tt>supplier_id bigint</tt>. # +default+ is the type-casted default value, such as +new+ in <tt>sales_stage varchar(20) default 'new'</tt>. # +sql_type_metadata+ is various information about the type of the column # +null+ determines if this column allows +NULL+ values. diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 8de582fee1..1d614dc8bf 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -105,6 +105,11 @@ module ActiveRecord @connection.close end + def discard! # :nodoc: + @connection.automatic_close = false + @connection = nil + end + private def connect diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 2c3c1df2a9..1739c288b6 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -273,6 +273,11 @@ module ActiveRecord end end + def discard! # :nodoc: + @connection.socket_io.reopen(IO::NULL) + @connection = nil + end + def native_database_types #:nodoc: NATIVE_DATABASE_TYPES end @@ -391,6 +396,8 @@ module ActiveRecord UNIQUE_VIOLATION = "23505" SERIALIZATION_FAILURE = "40001" DEADLOCK_DETECTED = "40P01" + LOCK_NOT_AVAILABLE = "55P03" + QUERY_CANCELED = "57014" def translate_exception(exception, message) return exception unless exception.respond_to?(:result) @@ -410,6 +417,10 @@ module ActiveRecord SerializationFailure.new(message) when DEADLOCK_DETECTED Deadlocked.new(message) + when LOCK_NOT_AVAILABLE + TransactionTimeout.new(message) + when QUERY_CANCELED + StatementTimeout.new(message) else super end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 0f7a503c90..481159e501 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -76,6 +76,14 @@ module ActiveRecord # scope being ignored is error-worthy, rather than a warning. mattr_accessor :error_on_ignored_order, instance_writer: false, default: false + # :singleton-method: + # Specify the behavior for unsafe raw query methods. Values are as follows + # deprecated - Warnings are logged when unsafe raw SQL is passed to + # query methods. + # disabled - Unsafe raw SQL passed to query methods results in + # UnknownAttributeReference exception. + mattr_accessor :allow_unsafe_raw_sql, instance_writer: false, default: :deprecated + ## # :singleton-method: # Specify whether or not to use timestamps for migration versions @@ -269,7 +277,8 @@ module ActiveRecord relation = Relation.create(self, arel_table, predicate_builder) if finder_needs_type_condition? && !ignore_default_scope? - relation.where(type_condition).create_with(inheritance_column.to_s => sti_name) + relation.where!(type_condition) + relation.create_with!(inheritance_column.to_s => sti_name) else relation end diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 5005d58f1c..ee4f818cbf 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -53,7 +53,7 @@ module ActiveRecord unscoped.where(primary_key => object.id).update_all(updates) end - return true + true end # A generic "counter updater" implementation, intended primarily to be diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 9ef3316393..7382879fce 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -335,8 +335,36 @@ module ActiveRecord class IrreversibleOrderError < ActiveRecordError end - # TransactionTimeout will be raised when lock wait timeout expires. - # Wait time value is set by innodb_lock_wait_timeout. + # TransactionTimeout will be raised when lock wait timeout exceeded. class TransactionTimeout < StatementInvalid end + + # StatementTimeout will be raised when statement timeout exceeded. + class StatementTimeout < StatementInvalid + end + + # UnknownAttributeReference is raised when an unknown and potentially unsafe + # value is passed to a query method when allow_unsafe_raw_sql is set to + # :disabled. For example, passing a non column name value to a relation's + # #order method might cause this exception. + # + # When working around this exception, caution should be taken to avoid SQL + # injection vulnerabilities when passing user-provided values to query + # methods. Known-safe values can be passed to query methods by wrapping them + # in Arel.sql. + # + # For example, with allow_unsafe_raw_sql set to :disabled, the following + # code would raise this exception: + # + # Post.order("length(title)").first + # + # The desired result can be accomplished by wrapping the known-safe string + # in Arel.sql: + # + # Post.order(Arel.sql("length(title)")).first + # + # Again, such a workaround should *not* be used when passing user-provided + # values, such as request parameters or model attributes to query methods. + class UnknownAttributeReference < ActiveRecordError + end end diff --git a/activerecord/lib/active_record/legacy_yaml_adapter.rb b/activerecord/lib/active_record/legacy_yaml_adapter.rb index 23644aab8f..ffa095dd94 100644 --- a/activerecord/lib/active_record/legacy_yaml_adapter.rb +++ b/activerecord/lib/active_record/legacy_yaml_adapter.rb @@ -8,7 +8,7 @@ module ActiveRecord case coder["active_record_yaml_version"] when 1, 2 then coder else - if coder["attributes"].is_a?(AttributeSet) + if coder["attributes"].is_a?(ActiveModel::AttributeSet) Rails420.convert(klass, coder) else Rails41.convert(klass, coder) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 1485687053..15e9c09ffb 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -581,7 +581,8 @@ module ActiveRecord def load_schema_if_pending! if ActiveRecord::Migrator.needs_migration? || !ActiveRecord::Migrator.any_migrations? # Roundtrip to Rake to allow plugins to hook into database initialization. - FileUtils.cd Rails.root do + root = defined?(ENGINE_ROOT) ? ENGINE_ROOT : Rails.root + FileUtils.cd(root) do current_config = Base.connection_config Base.clear_all_connections! system("bin/rails db:test:prepare") @@ -733,6 +734,24 @@ module ActiveRecord execute_block { yield helper } end + # Used to specify an operation that is only run when migrating up + # (for example, populating a new column with its initial values). + # + # In the following example, the new column +published+ will be given + # the value +true+ for all existing records. + # + # class AddPublishedToPosts < ActiveRecord::Migration[5.2] + # def change + # add_column :posts, :published, :boolean, default: false + # up_only do + # execute "update posts set published = 'true'" + # end + # end + # end + def up_only + execute_block { yield } unless reverting? + end + # Runs the given migration classes. # Last argument can specify options: # - :direction (default is :up) @@ -1224,7 +1243,7 @@ module ActiveRecord # Return true if a valid version is not provided. def invalid_target? - !target && @target_version && @target_version > 0 + @target_version && @target_version != 0 && !target end def execute_migration_in_transaction(migration, direction) diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index bed9400f51..12ee4a4137 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -323,7 +323,7 @@ module ActiveRecord end def attributes_builder # :nodoc: - @attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key) do |name| + @attributes_builder ||= ActiveModel::AttributeSet::Builder.new(attribute_types, primary_key) do |name| unless columns_hash.key?(name) _default_attributes[name].dup end @@ -346,7 +346,7 @@ module ActiveRecord end def yaml_encoder # :nodoc: - @yaml_encoder ||= AttributeSet::YAMLEncoder.new(attribute_types) + @yaml_encoder ||= ActiveModel::AttributeSet::YAMLEncoder.new(attribute_types) end # Returns the type of the attribute with the given name, after applying @@ -376,7 +376,7 @@ module ActiveRecord end def _default_attributes # :nodoc: - @default_attributes ||= AttributeSet.new({}) + @default_attributes ||= ActiveModel::AttributeSet.new({}) end # Returns an array of column names as strings. diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 435c81c153..fa20bce3a9 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -63,6 +63,18 @@ module ActiveRecord # member.update params[:member] # member.avatar.icon # => 'sad' # + # If you want to update the current avatar without providing the id, you must add <tt>:update_only</tt> option. + # + # class Member < ActiveRecord::Base + # has_one :avatar + # accepts_nested_attributes_for :avatar, update_only: true + # end + # + # params = { member: { avatar_attributes: { icon: 'sad' } } } + # member.update params[:member] + # member.avatar.id # => 2 + # member.avatar.icon # => 'sad' + # # By default you will only be able to set and update attributes on the # associated model. If you want to destroy the associated model through the # attributes hash, you have to enable it first using the diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index a57c60ffac..4e1b05dbf6 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -165,6 +165,38 @@ module ActiveRecord where(primary_key => id_or_array).delete_all end + def _insert_record(values) # :nodoc: + primary_key_value = nil + + if primary_key && Hash === values + arel_primary_key = arel_attribute(primary_key) + primary_key_value = values[arel_primary_key] + + if !primary_key_value && prefetch_primary_key? + primary_key_value = next_sequence_value + values[arel_primary_key] = primary_key_value + end + end + + if values.empty? + im = arel_table.compile_insert(connection.empty_insert_statement_value) + im.into arel_table + else + im = arel_table.compile_insert(_substitute_values(values)) + end + + connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) + end + + def _update_record(values, id, id_was) # :nodoc: + bind = predicate_builder.build_bind_attribute(primary_key, id_was || id) + um = arel_table.where( + arel_attribute(primary_key).eq(bind) + ).compile_update(_substitute_values(values), primary_key) + + connection.update(um, "#{self} Update") + end + private # Called by +instantiate+ to decide which class to use for a new # record instance. @@ -174,6 +206,13 @@ module ActiveRecord def discriminate_class_for_record(record) self end + + def _substitute_values(values) + values.map do |attr, value| + bind = predicate_builder.build_bind_attribute(attr.name, value) + [attr, bind] + end + end end # Returns true if this object hasn't been saved yet -- that is, a record @@ -671,7 +710,7 @@ module ActiveRecord rows_affected = 0 @_trigger_update_callback = true else - rows_affected = self.class.unscoped._update_record attributes_values, id, id_in_database + rows_affected = self.class._update_record(attributes_values, id, id_in_database) @_trigger_update_callback = rows_affected > 0 end @@ -685,7 +724,7 @@ module ActiveRecord def _create_record(attribute_names = self.attribute_names) attributes_values = arel_attributes_with_values_for_create(attribute_names) - new_id = self.class.unscoped.insert attributes_values + new_id = self.class._insert_record(attributes_values) self.id ||= new_id if self.class.primary_key @new_record = false diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index 3d5babb8b7..8e23128333 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -26,16 +26,19 @@ module ActiveRecord end def self.run - caching_pool = ActiveRecord::Base.connection_pool - caching_was_enabled = caching_pool.query_cache_enabled + ActiveRecord::Base.connection_handler.connection_pool_list.map do |pool| + caching_was_enabled = pool.query_cache_enabled - caching_pool.enable_query_cache! + pool.enable_query_cache! - [caching_pool, caching_was_enabled] + [pool, caching_was_enabled] + end end - def self.complete((caching_pool, caching_was_enabled)) - caching_pool.disable_query_cache! unless caching_was_enabled + def self.complete(caching_pools) + caching_pools.each do |pool, caching_was_enabled| + pool.disable_query_cache! unless caching_was_enabled + end ActiveRecord::Base.connection_handler.connection_pool_list.each do |pool| pool.release_connection if pool.active_connection? && !pool.connection.transaction_open? diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 723272b4b2..fce3e1c5cf 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -4,16 +4,16 @@ require "active_record" db_namespace = namespace :db do desc "Set the environment value for the database" - task "environment:set" => [:environment, :load_config] do + task "environment:set" => :load_config do ActiveRecord::InternalMetadata.create_table ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment end - task check_protected_environments: [:environment, :load_config] do + task check_protected_environments: :load_config do ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! end - task :load_config do + task load_config: :environment do ActiveRecord::Base.configurations = ActiveRecord::Tasks::DatabaseTasks.database_configuration || {} ActiveRecord::Migrator.migrations_paths = ActiveRecord::Tasks::DatabaseTasks.migrations_paths end @@ -56,7 +56,7 @@ db_namespace = namespace :db do end desc "Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog)." - task migrate: [:environment, :load_config] do + task migrate: :load_config do ActiveRecord::Tasks::DatabaseTasks.migrate db_namespace["_dump"].invoke end @@ -78,7 +78,7 @@ db_namespace = namespace :db do namespace :migrate do # desc 'Rollbacks the database one migration and re migrate up (options: STEP=x, VERSION=x).' - task redo: [:environment, :load_config] do + task redo: :load_config do raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty? if ENV["VERSION"] @@ -94,24 +94,35 @@ db_namespace = namespace :db do task reset: ["db:drop", "db:create", "db:migrate"] # desc 'Runs the "up" for a given migration VERSION.' - task up: [:environment, :load_config] do + task up: :load_config do raise "VERSION is required" if !ENV["VERSION"] || ENV["VERSION"].empty? - version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil - ActiveRecord::Migrator.run(:up, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version) + ActiveRecord::Tasks::DatabaseTasks.check_target_version + + ActiveRecord::Migrator.run( + :up, + ActiveRecord::Tasks::DatabaseTasks.migrations_paths, + ActiveRecord::Tasks::DatabaseTasks.target_version + ) db_namespace["_dump"].invoke end # desc 'Runs the "down" for a given migration VERSION.' - task down: [:environment, :load_config] do + task down: :load_config do raise "VERSION is required - To go down one migration, use db:rollback" if !ENV["VERSION"] || ENV["VERSION"].empty? - version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil - ActiveRecord::Migrator.run(:down, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version) + + ActiveRecord::Tasks::DatabaseTasks.check_target_version + + ActiveRecord::Migrator.run( + :down, + ActiveRecord::Tasks::DatabaseTasks.migrations_paths, + ActiveRecord::Tasks::DatabaseTasks.target_version + ) db_namespace["_dump"].invoke end desc "Display status of migrations" - task status: [:environment, :load_config] do + task status: :load_config do unless ActiveRecord::SchemaMigration.table_exists? abort "Schema migrations table does not exist yet." end @@ -129,14 +140,14 @@ db_namespace = namespace :db do end desc "Rolls the schema back to the previous version (specify steps w/ STEP=n)." - task rollback: [:environment, :load_config] do + task rollback: :load_config do step = ENV["STEP"] ? ENV["STEP"].to_i : 1 ActiveRecord::Migrator.rollback(ActiveRecord::Tasks::DatabaseTasks.migrations_paths, step) db_namespace["_dump"].invoke end # desc 'Pushes the schema to the next version (specify steps w/ STEP=n).' - task forward: [:environment, :load_config] do + task forward: :load_config do step = ENV["STEP"] ? ENV["STEP"].to_i : 1 ActiveRecord::Migrator.forward(ActiveRecord::Tasks::DatabaseTasks.migrations_paths, step) db_namespace["_dump"].invoke @@ -146,12 +157,12 @@ db_namespace = namespace :db do task reset: [ "db:drop", "db:setup" ] # desc "Retrieves the charset for the current environment's database" - task charset: [:environment, :load_config] do + task charset: :load_config do puts ActiveRecord::Tasks::DatabaseTasks.charset_current end # desc "Retrieves the collation for the current environment's database" - task collation: [:environment, :load_config] do + task collation: :load_config do begin puts ActiveRecord::Tasks::DatabaseTasks.collation_current rescue NoMethodError @@ -160,12 +171,12 @@ db_namespace = namespace :db do end desc "Retrieves the current schema version number" - task version: [:environment, :load_config] do + task version: :load_config do puts "Current version: #{ActiveRecord::Migrator.current_version}" end # desc "Raises an error if there are pending migrations" - task abort_if_pending_migrations: [:environment, :load_config] do + task abort_if_pending_migrations: :load_config do pending_migrations = ActiveRecord::Migrator.open(ActiveRecord::Tasks::DatabaseTasks.migrations_paths).pending_migrations if pending_migrations.any? @@ -188,7 +199,7 @@ db_namespace = namespace :db do namespace :fixtures do desc "Loads fixtures into the current environment's database. Load specific fixtures using FIXTURES=x,y. Load from subdirectory in test/fixtures using FIXTURES_DIR=z. Specify an alternative path (eg. spec/fixtures) using FIXTURES_PATH=spec/fixtures." - task load: [:environment, :load_config] do + task load: :load_config do require "active_record/fixtures" base_dir = ActiveRecord::Tasks::DatabaseTasks.fixtures_path @@ -210,7 +221,7 @@ db_namespace = namespace :db do end # desc "Search for a fixture given a LABEL or ID. Specify an alternative path (eg. spec/fixtures) using FIXTURES_PATH=spec/fixtures." - task identify: [:environment, :load_config] do + task identify: :load_config do require "active_record/fixtures" label, id = ENV["LABEL"], ENV["ID"] @@ -236,7 +247,7 @@ db_namespace = namespace :db do namespace :schema do desc "Creates a db/schema.rb file that is portable against any DB supported by Active Record" - task dump: [:environment, :load_config] do + task dump: :load_config do require "active_record/schema_dumper" filename = ENV["SCHEMA"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "schema.rb") File.open(filename, "w:utf-8") do |file| @@ -246,7 +257,7 @@ db_namespace = namespace :db do end desc "Loads a schema.rb file into the database" - task load: [:environment, :load_config, :check_protected_environments] do + task load: [:load_config, :check_protected_environments] do ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:ruby, ENV["SCHEMA"]) end @@ -256,14 +267,14 @@ db_namespace = namespace :db do namespace :cache do desc "Creates a db/schema_cache.yml file." - task dump: [:environment, :load_config] do + task dump: :load_config do conn = ActiveRecord::Base.connection filename = File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "schema_cache.yml") ActiveRecord::Tasks::DatabaseTasks.dump_schema_cache(conn, filename) end desc "Clears a db/schema_cache.yml file." - task clear: [:environment, :load_config] do + task clear: :load_config do filename = File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "schema_cache.yml") rm_f filename, verbose: false end @@ -273,7 +284,7 @@ db_namespace = namespace :db do namespace :structure do desc "Dumps the database structure to db/structure.sql. Specify another file with SCHEMA=db/my_structure.sql" - task dump: [:environment, :load_config] do + task dump: :load_config do filename = ENV["SCHEMA"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "structure.sql") current_config = ActiveRecord::Tasks::DatabaseTasks.current_config ActiveRecord::Tasks::DatabaseTasks.structure_dump(current_config, filename) @@ -288,7 +299,7 @@ db_namespace = namespace :db do end desc "Recreates the databases from the structure.sql file" - task load: [:environment, :load_config, :check_protected_environments] do + task load: [:load_config, :check_protected_environments] do ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:sql, ENV["SCHEMA"]) end @@ -327,12 +338,12 @@ db_namespace = namespace :db do end # desc "Empty the test database" - task purge: %w(environment load_config check_protected_environments) do + task purge: %w(load_config check_protected_environments) do ActiveRecord::Tasks::DatabaseTasks.purge ActiveRecord::Base.configurations["test"] end # desc 'Load the test schema' - task prepare: %w(environment load_config) do + task prepare: :load_config do unless ActiveRecord::Base.configurations.blank? db_namespace["test:load"].invoke end diff --git a/activerecord/lib/active_record/railties/jdbcmysql_error.rb b/activerecord/lib/active_record/railties/jdbcmysql_error.rb deleted file mode 100644 index 72c75ddd52..0000000000 --- a/activerecord/lib/active_record/railties/jdbcmysql_error.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -#FIXME Remove if ArJdbcMysql will give. -module ArJdbcMySQL #:nodoc: - class Error < StandardError #:nodoc: - attr_accessor :error_number, :sql_state - - def initialize(msg) - super - @error_number = nil - @sql_state = nil - end - - # Mysql gem compatibility - alias_method :errno, :error_number - alias_method :error, :message - end -end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index c18214a842..87bfd75bca 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -839,10 +839,6 @@ module ActiveRecord source_reflection.join_scopes(table, predicate_builder) + super end - def source_type_scope - through_reflection.klass.where(foreign_type => options[:source_type]) - end - def has_scope? scope || options[:source_type] || source_reflection.has_scope? || @@ -1009,22 +1005,17 @@ module ActiveRecord @previous_reflection = previous_reflection end - def scopes - scopes = @previous_reflection.scopes - scopes << @previous_reflection.source_type_scope - end - def join_scopes(table, predicate_builder) # :nodoc: scopes = @previous_reflection.join_scopes(table, predicate_builder) + super - scopes << @previous_reflection.source_type_scope + scopes << build_scope(table, predicate_builder).instance_exec(nil, &source_type_scope) end def constraints - @reflection.constraints + [source_type_info] + @reflection.constraints + [source_type_scope] end private - def source_type_info + def source_type_scope type = @previous_reflection.foreign_type source_type = @previous_reflection.options[:source_type] lambda { |object| where(type => source_type) } diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index c16d89aa00..d3b8091665 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -36,67 +36,6 @@ module ActiveRecord reset end - def insert(values) # :nodoc: - primary_key_value = nil - - if primary_key && Hash === values - primary_key_value = values[values.keys.find { |k| - k.name == primary_key - }] - - if !primary_key_value && klass.prefetch_primary_key? - primary_key_value = klass.next_sequence_value - values[arel_attribute(klass.primary_key)] = primary_key_value - end - end - - im = arel.create_insert - im.into @table - - substitutes = substitute_values values - - if values.empty? # empty insert - im.values = Arel.sql(connection.empty_insert_statement_value) - else - im.insert substitutes - end - - @klass.connection.insert( - im, - "#{@klass} Create", - primary_key || false, - primary_key_value, - nil, - ) - end - - def _update_record(values, id, id_was) # :nodoc: - substitutes = substitute_values values - - scope = @klass.unscoped - - if @klass.finder_needs_type_condition? - scope.unscope!(where: @klass.inheritance_column) - end - - relation = scope.where(@klass.primary_key => (id_was || id)) - um = relation - .arel - .compile_update(substitutes, @klass.primary_key) - - @klass.connection.update( - um, - "#{@klass} Update", - ) - end - - def substitute_values(values) # :nodoc: - values.map do |arel_attr, value| - bind = predicate_builder.build_bind_attribute(arel_attr.name, value) - [arel_attr, bind] - end - end - def arel_attribute(name) # :nodoc: klass.arel_attribute(name, table) end @@ -243,9 +182,10 @@ module ActiveRecord end # Converts relation objects to Array. - def to_a + def to_ary records.dup end + alias to_a to_ary def records # :nodoc: load @@ -359,6 +299,11 @@ module ActiveRecord def update_all(updates) raise ArgumentError, "Empty list of attributes to change" if updates.blank? + if eager_loading? + relation = apply_join_dependency + return relation.update_all(updates) + end + stmt = Arel::UpdateManager.new stmt.set Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)) @@ -423,6 +368,11 @@ module ActiveRecord raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}") end + if eager_loading? + relation = apply_join_dependency + return relation.delete_all + end + stmt = Arel::DeleteManager.new stmt.from(table) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 116bddce85..d49472fc70 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -130,7 +130,7 @@ module ActiveRecord # end def calculate(operation, column_name) if has_include?(column_name) - relation = apply_join_dependency(construct_join_dependency) + relation = apply_join_dependency relation.distinct! if operation.to_s.downcase == "count" relation.calculate(operation, column_name) @@ -180,9 +180,10 @@ module ActiveRecord end if has_include?(column_names.first) - relation = apply_join_dependency(construct_join_dependency) + relation = apply_join_dependency relation.pluck(*column_names) else + enforce_raw_sql_whitelist(column_names) relation = spawn relation.select_values = column_names.map { |cn| @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 48af777b69..4863befec8 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -38,7 +38,7 @@ module ActiveRecord # may vary depending on the klass of a relation, so we create a subclass of Relation # for each different klass, and the delegations are compiled into that subclass only. - delegate :to_xml, :encode_with, :length, :each, :uniq, :to_ary, :join, + delegate :to_xml, :encode_with, :length, :each, :uniq, :join, :[], :&, :|, :+, :-, :sample, :reverse, :rotate, :compact, :in_groups, :in_groups_of, :to_sentence, :to_formatted_s, :as_json, :shuffle, :split, :slice, :index, :rindex, to: :records diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 707245bab2..706fd57704 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -284,7 +284,7 @@ module ActiveRecord # * Hash - Finds the record that matches these +find+-style conditions # (such as <tt>{name: 'David'}</tt>). # * +false+ - Returns always +false+. - # * No args - Returns +false+ if the table is empty, +true+ otherwise. + # * No args - Returns +false+ if the relation is empty, +true+ otherwise. # # For more information about specifying conditions as a hash or array, # see the Conditions section in the introduction to ActiveRecord::Base. @@ -300,6 +300,7 @@ module ActiveRecord # Person.exists?(name: 'David') # Person.exists?(false) # Person.exists? + # Person.where(name: 'Spartacus', rating: 4).exists? def exists?(conditions = :none) if Base === conditions raise ArgumentError, <<-MSG.squish @@ -394,7 +395,7 @@ module ActiveRecord ) end - def apply_join_dependency(join_dependency) + def apply_join_dependency(join_dependency = construct_join_dependency) relation = except(:includes, :eager_load, :preload).joins!(join_dependency) if using_limitable_reflections?(join_dependency.reflections) diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index ebc72d28fd..b736b21525 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -112,22 +112,20 @@ module ActiveRecord if other.klass == relation.klass relation.joins!(*other.joins_values) else - joins_dependency, rest = other.joins_values.partition do |join| + alias_tracker = nil + joins_dependency = other.joins_values.map do |join| case join when Hash, Symbol, Array - true + alias_tracker ||= other.alias_tracker + ActiveRecord::Associations::JoinDependency.new( + other.klass, other.table, join, alias_tracker + ) else - false + join end end - join_dependency = ActiveRecord::Associations::JoinDependency.new( - other.klass, other.table, joins_dependency, other.alias_tracker - ) - - relation.joins! rest - - @relation = relation.joins join_dependency + relation.joins!(*joins_dependency) end end diff --git a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb index f51ea4fde0..c8bbfa5051 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb @@ -4,6 +4,10 @@ module ActiveRecord class PredicateBuilder class RelationHandler # :nodoc: def call(attribute, value) + if value.eager_loading? + value = value.send(:apply_join_dependency) + end + if value.select_values.empty? value = value.select(value.arel_attribute(value.klass.primary_key)) end diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index fad08e2613..3532f28858 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" module ActiveRecord class Relation - class QueryAttribute < Attribute # :nodoc: + class QueryAttribute < ActiveModel::Attribute # :nodoc: def type_cast(value) value end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 897ff5c8af..749223422f 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -295,6 +295,7 @@ module ActiveRecord spawn.order!(*args) end + # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: preprocess_order_args(args) @@ -316,6 +317,7 @@ module ActiveRecord spawn.reorder!(*args) end + # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: preprocess_order_args(args) @@ -930,7 +932,7 @@ module ActiveRecord arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? if limit_value - limit_attribute = Attribute.with_cast_value( + limit_attribute = ActiveModel::Attribute.with_cast_value( "LIMIT".freeze, connection.sanitize_limit(limit_value), Type.default_value, @@ -938,7 +940,7 @@ module ActiveRecord arel.take(Arel::Nodes::BindParam.new(limit_attribute)) end if offset_value - offset_attribute = Attribute.with_cast_value( + offset_attribute = ActiveModel::Attribute.with_cast_value( "OFFSET".freeze, offset_value.to_i, Type.default_value, @@ -963,6 +965,9 @@ module ActiveRecord name = from_clause.name case opts when Relation + if opts.eager_loading? + opts = opts.send(:apply_join_dependency) + end name ||= "subquery" opts.arel.as(name.to_s) else @@ -1035,6 +1040,8 @@ module ActiveRecord def build_select(arel) if select_values.any? arel.project(*arel_columns(select_values.uniq)) + elsif @klass.ignored_columns.any? + arel.project(*arel_columns(@klass.column_names)) else arel.project(table[Arel.star]) end @@ -1071,7 +1078,7 @@ module ActiveRecord end o.split(",").map! do |s| s.strip! - s.gsub!(/\sasc\Z/i, " DESC") || s.gsub!(/\sdesc\Z/i, " ASC") || s.concat(" DESC") + s.gsub!(/\sasc\Z/i, " DESC") || s.gsub!(/\sdesc\Z/i, " ASC") || (s << " DESC") end else o @@ -1080,6 +1087,10 @@ module ActiveRecord end def does_not_support_reverse?(order) + # Account for String subclasses like Arel::Nodes::SqlLiteral that + # override methods like #count. + order = String.new(order) unless order.instance_of?(String) + # Uses SQL function with multiple arguments. (order.include?(",") && order.split(",").find { |section| section.count("(") != section.count(")") }) || # Uses "nulls first" like construction. @@ -1113,6 +1124,12 @@ module ActiveRecord klass.send(:sanitize_sql_for_order, arg) end order_args.flatten! + + @klass.enforce_raw_sql_whitelist( + order_args.flat_map { |a| a.is_a?(Hash) ? a.keys : a }, + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST + ) + validate_order_args(order_args) references = order_args.grep(String) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 1c3099f55c..21f8bc7cb2 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -63,7 +63,17 @@ module ActiveRecord # # => "id ASC" def sanitize_sql_for_order(condition) # :doc: if condition.is_a?(Array) && condition.first.to_s.include?("?") - sanitize_sql_array(condition) + enforce_raw_sql_whitelist([condition.first], + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST + ) + + # Ensure we aren't dealing with a subclass of String that might + # override methods we use (eg. Arel::Nodes::SqlLiteral). + if condition.first.kind_of?(String) && !condition.first.instance_of?(String) + condition = [String.new(condition.first), *condition[1..-1]] + end + + Arel.sql(sanitize_sql_array(condition)) else condition end @@ -110,7 +120,8 @@ module ActiveRecord def sanitize_sql_hash_for_assignment(attrs, table) # :doc: c = connection attrs.map do |attr, value| - value = type_for_attribute(attr.to_s).serialize(value) + type = type_for_attribute(attr.to_s) + value = type.serialize(type.cast(value)) "#{c.quote_table_name_for_assignment(table, attr)} = #{c.quote(value)}" end.join(", ") end diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 86ae374318..8c612df27a 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -111,7 +111,7 @@ module ActiveRecord # The user has defined their own default scope method, so call that evaluate_default_scope do if scope = default_scope - (base_rel ||= relation).merge(scope) + (base_rel ||= relation).merge!(scope) end end elsif default_scopes.any? @@ -119,7 +119,7 @@ module ActiveRecord evaluate_default_scope do default_scopes.inject(base_rel) do |default_scope, scope| scope = scope.respond_to?(:to_proc) ? scope : scope.method(:call) - default_scope.merge(base_rel.instance_exec(&scope)) + default_scope.merge!(base_rel.instance_exec(&scope)) end end end diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 6fa096c1fe..310af72c41 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -24,8 +24,14 @@ module ActiveRecord # You can define a scope that applies to all finders using # {default_scope}[rdoc-ref:Scoping::Default::ClassMethods#default_scope]. def all + current_scope = self.current_scope + if current_scope - current_scope.clone + if self == current_scope.klass + current_scope.clone + else + relation.merge!(current_scope) + end else default_scoped end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index ff388ff1f6..4657e51e6d 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -164,13 +164,12 @@ module ActiveRecord end def migrate - raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty? + check_target_version verbose = ENV["VERBOSE"] ? ENV["VERBOSE"] != "false" : true - version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil scope = ENV["SCOPE"] verbose_was, Migration.verbose = Migration.verbose, verbose - Migrator.migrate(migrations_paths, version) do |migration| + Migrator.migrate(migrations_paths, target_version) do |migration| scope.blank? || scope == migration.scope end ActiveRecord::Base.clear_cache! @@ -178,6 +177,16 @@ module ActiveRecord Migration.verbose = verbose_was end + def check_target_version + if target_version && !(Migration::MigrationFilenameRegexp.match?(ENV["VERSION"]) || /\A\d+\z/.match?(ENV["VERSION"])) + raise "Invalid format of target version: `VERSION=#{ENV['VERSION']}`" + end + end + + def target_version + ENV["VERSION"].to_i if ENV["VERSION"] && !ENV["VERSION"].empty? + end + def charset_current(environment = env) charset ActiveRecord::Base.configurations[environment] end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 2f91884f59..e697fa6def 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -3,8 +3,6 @@ module ActiveRecord module Tasks # :nodoc: class MySQLDatabaseTasks # :nodoc: - ACCESS_DENIED_ERROR = 1045 - delegate :connection, :establish_connection, to: ActiveRecord::Base def initialize(configuration) @@ -21,20 +19,6 @@ module ActiveRecord else raise end - rescue error_class => error - if error.respond_to?(:errno) && error.errno == ACCESS_DENIED_ERROR - $stdout.print error.message - establish_connection root_configuration_without_database - connection.create_database configuration["database"], creation_options - if configuration["username"] != "root" - connection.execute grant_statement.gsub(/\s+/, " ").strip - end - establish_connection configuration - else - $stderr.puts error.inspect - $stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}" - $stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration["encoding"] - end end def drop @@ -99,37 +83,6 @@ module ActiveRecord end end - def error_class - if configuration["adapter"].include?("jdbc") - require "active_record/railties/jdbcmysql_error" - ArJdbcMySQL::Error - elsif defined?(Mysql2) - Mysql2::Error - else - StandardError - end - end - - def grant_statement - <<-SQL -GRANT ALL PRIVILEGES ON `#{configuration['database']}`.* - TO '#{configuration['username']}'@'localhost' -IDENTIFIED BY '#{configuration['password']}' WITH GRANT OPTION; - SQL - end - - def root_configuration_without_database - configuration_without_database.merge( - "username" => "root", - "password" => root_password - ) - end - - def root_password - $stdout.print "Please provide the root password for your MySQL installation\n>" - $stdin.gets.strip - end - def prepare_command_options args = { "host" => "--host", diff --git a/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb b/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb.tt index 60050e0bf8..60050e0bf8 100644 --- a/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb +++ b/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb.tt diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt index 5f7201cfe1..5f7201cfe1 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt index 481c70201b..481c70201b 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt diff --git a/activerecord/lib/rails/generators/active_record/model/templates/model.rb b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt index 55dc65c8ad..55dc65c8ad 100644 --- a/activerecord/lib/rails/generators/active_record/model/templates/model.rb +++ b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt diff --git a/activerecord/lib/rails/generators/active_record/model/templates/module.rb b/activerecord/lib/rails/generators/active_record/model/templates/module.rb.tt index a3bf1c37b6..a3bf1c37b6 100644 --- a/activerecord/lib/rails/generators/active_record/model/templates/module.rb +++ b/activerecord/lib/rails/generators/active_record/model/templates/module.rb.tt diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index a6b83ec377..4e73c557ed 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -9,7 +9,7 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase def setup ActiveRecord::Base.connection.singleton_class.class_eval do alias_method :execute_without_stub, :execute - def execute(sql, name = nil) return sql end + def execute(sql, name = nil) sql end end end diff --git a/activerecord/test/cases/adapters/mysql2/explain_test.rb b/activerecord/test/cases/adapters/mysql2/explain_test.rb index 2736f7cf0e..b8e778f0b0 100644 --- a/activerecord/test/cases/adapters/mysql2/explain_test.rb +++ b/activerecord/test/cases/adapters/mysql2/explain_test.rb @@ -1,23 +1,23 @@ # frozen_string_literal: true require "cases/helper" -require "models/developer" -require "models/computer" +require "models/author" +require "models/post" class Mysql2ExplainTest < ActiveRecord::Mysql2TestCase - fixtures :developers + fixtures :authors def test_explain_for_one_query - explain = Developer.where(id: 1).explain - assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain - assert_match %r(developers |.* const), explain + explain = Author.where(id: 1).explain + assert_match %(EXPLAIN for: SELECT `authors`.* FROM `authors` WHERE `authors`.`id` = 1), explain + assert_match %r(authors |.* const), explain end def test_explain_with_eager_loading - explain = Developer.where(id: 1).includes(:audit_logs).explain - assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain - assert_match %r(developers |.* const), explain - assert_match %(EXPLAIN for: SELECT `audit_logs`.* FROM `audit_logs` WHERE `audit_logs`.`developer_id` = 1), explain - assert_match %r(audit_logs |.* ALL), explain + explain = Author.where(id: 1).includes(:posts).explain + assert_match %(EXPLAIN for: SELECT `authors`.* FROM `authors` WHERE `authors`.`id` = 1), explain + assert_match %r(authors |.* const), explain + assert_match %(EXPLAIN for: SELECT `posts`.* FROM `posts` WHERE `posts`.`author_id` = 1), explain + assert_match %r(posts |.* ALL), explain end end diff --git a/activerecord/test/cases/adapters/mysql2/transaction_test.rb b/activerecord/test/cases/adapters/mysql2/transaction_test.rb index 25d9f69a89..4a3a4503de 100644 --- a/activerecord/test/cases/adapters/mysql2/transaction_test.rb +++ b/activerecord/test/cases/adapters/mysql2/transaction_test.rb @@ -60,9 +60,60 @@ module ActiveRecord end end - test "raises TransactionTimeout when mysql raises ER_LOCK_WAIT_TIMEOUT" do + test "raises TransactionTimeout when lock wait timeout exceeded" do assert_raises(ActiveRecord::TransactionTimeout) do - ActiveRecord::Base.connection.execute("SIGNAL SQLSTATE 'HY000' SET MESSAGE_TEXT = 'Testing error', MYSQL_ERRNO = 1205;") + s = Sample.create!(value: 1) + latch1 = Concurrent::CountDownLatch.new + latch2 = Concurrent::CountDownLatch.new + + thread = Thread.new do + Sample.transaction do + Sample.lock.find(s.id) + latch1.count_down + latch2.wait + end + end + + begin + Sample.transaction do + latch1.wait + Sample.connection.execute("SET innodb_lock_wait_timeout = 1") + Sample.lock.find(s.id) + end + ensure + Sample.connection.execute("SET innodb_lock_wait_timeout = DEFAULT") + latch2.count_down + thread.join + end + end + end + + test "raises StatementTimeout when statement timeout exceeded" do + skip unless ActiveRecord::Base.connection.show_variable("max_execution_time") + assert_raises(ActiveRecord::StatementTimeout) do + s = Sample.create!(value: 1) + latch1 = Concurrent::CountDownLatch.new + latch2 = Concurrent::CountDownLatch.new + + thread = Thread.new do + Sample.transaction do + Sample.lock.find(s.id) + latch1.count_down + latch2.wait + end + end + + begin + Sample.transaction do + latch1.wait + Sample.connection.execute("SET max_execution_time = 1") + Sample.lock.find(s.id) + end + ensure + Sample.connection.execute("SET max_execution_time = DEFAULT") + latch2.count_down + thread.join + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/explain_test.rb b/activerecord/test/cases/adapters/postgresql/explain_test.rb index 16fec94ede..be525383e9 100644 --- a/activerecord/test/cases/adapters/postgresql/explain_test.rb +++ b/activerecord/test/cases/adapters/postgresql/explain_test.rb @@ -1,22 +1,22 @@ # frozen_string_literal: true require "cases/helper" -require "models/developer" -require "models/computer" +require "models/author" +require "models/post" class PostgreSQLExplainTest < ActiveRecord::PostgreSQLTestCase - fixtures :developers + fixtures :authors def test_explain_for_one_query - explain = Developer.where(id: 1).explain - assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain + explain = Author.where(id: 1).explain + assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain assert_match %(QUERY PLAN), explain end def test_explain_with_eager_loading - explain = Developer.where(id: 1).includes(:audit_logs).explain + explain = Author.where(id: 1).includes(:posts).explain assert_match %(QUERY PLAN), explain - assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain - assert_match %r(EXPLAIN for: SELECT "audit_logs"\.\* FROM "audit_logs" WHERE "audit_logs"\."developer_id" = (?:\$1 \[\["developer_id", 1\]\]|1)), explain + assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain + assert_match %r(EXPLAIN for: SELECT "posts"\.\* FROM "posts" WHERE "posts"\."author_id" = (?:\$1 \[\["author_id", 1\]\]|1)), explain end end diff --git a/activerecord/test/cases/adapters/postgresql/transaction_test.rb b/activerecord/test/cases/adapters/postgresql/transaction_test.rb index f56adf4a5e..4d63bbce59 100644 --- a/activerecord/test/cases/adapters/postgresql/transaction_test.rb +++ b/activerecord/test/cases/adapters/postgresql/transaction_test.rb @@ -91,6 +91,63 @@ module ActiveRecord end end + test "raises TransactionTimeout when lock wait timeout exceeded" do + skip unless ActiveRecord::Base.connection.postgresql_version >= 90300 + assert_raises(ActiveRecord::TransactionTimeout) do + s = Sample.create!(value: 1) + latch1 = Concurrent::CountDownLatch.new + latch2 = Concurrent::CountDownLatch.new + + thread = Thread.new do + Sample.transaction do + Sample.lock.find(s.id) + latch1.count_down + latch2.wait + end + end + + begin + Sample.transaction do + latch1.wait + Sample.connection.execute("SET lock_timeout = 1") + Sample.lock.find(s.id) + end + ensure + Sample.connection.execute("SET lock_timeout = DEFAULT") + latch2.count_down + thread.join + end + end + end + + test "raises StatementTimeout when statement timeout exceeded" do + assert_raises(ActiveRecord::StatementTimeout) do + s = Sample.create!(value: 1) + latch1 = Concurrent::CountDownLatch.new + latch2 = Concurrent::CountDownLatch.new + + thread = Thread.new do + Sample.transaction do + Sample.lock.find(s.id) + latch1.count_down + latch2.wait + end + end + + begin + Sample.transaction do + latch1.wait + Sample.connection.execute("SET statement_timeout = 1") + Sample.lock.find(s.id) + end + ensure + Sample.connection.execute("SET statement_timeout = DEFAULT") + latch2.count_down + thread.join + end + end + end + private def with_warning_suppression diff --git a/activerecord/test/cases/adapters/sqlite3/explain_test.rb b/activerecord/test/cases/adapters/sqlite3/explain_test.rb index 3b081d34e1..b6d2ccdb53 100644 --- a/activerecord/test/cases/adapters/sqlite3/explain_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/explain_test.rb @@ -1,23 +1,23 @@ # frozen_string_literal: true require "cases/helper" -require "models/developer" -require "models/computer" +require "models/author" +require "models/post" class SQLite3ExplainTest < ActiveRecord::SQLite3TestCase - fixtures :developers + fixtures :authors def test_explain_for_one_query - explain = Developer.where(id: 1).explain - assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\? \[\["id", 1\]\]|1)), explain - assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) + explain = Author.where(id: 1).explain + assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\? \[\["id", 1\]\]|1)), explain + assert_match(/(SEARCH )?TABLE authors USING (INTEGER )?PRIMARY KEY/, explain) end def test_explain_with_eager_loading - explain = Developer.where(id: 1).includes(:audit_logs).explain - assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\? \[\["id", 1\]\]|1)), explain - assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) - assert_match %r(EXPLAIN for: SELECT "audit_logs"\.\* FROM "audit_logs" WHERE "audit_logs"\."developer_id" = (?:\? \[\["developer_id", 1\]\]|1)), explain - assert_match(/(SCAN )?TABLE audit_logs/, explain) + explain = Author.where(id: 1).includes(:posts).explain + assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\? \[\["id", 1\]\]|1)), explain + assert_match(/(SEARCH )?TABLE authors USING (INTEGER )?PRIMARY KEY/, explain) + assert_match %r(EXPLAIN for: SELECT "posts"\.\* FROM "posts" WHERE "posts"\."author_id" = (?:\? \[\["author_id", 1\]\]|1)), explain + assert_match(/(SCAN )?TABLE posts/, explain) end end diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index e69cfe5e52..829e12fbc8 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -37,7 +37,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).to_a - assert_equal 1, assert_no_queries { authors.size } + assert_equal 3, assert_no_queries { authors.size } assert_equal 10, assert_no_queries { authors[0].comments.size } end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 9afe6a893c..9a042c74db 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -427,7 +427,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_association_loading_with_belongs_to_and_order_string_with_quoted_table_name quoted_posts_id = Comment.connection.quote_table_name("posts") + "." + Comment.connection.quote_column_name("id") assert_nothing_raised do - Comment.includes(:post).references(:posts).order(quoted_posts_id) + Comment.includes(:post).references(:posts).order(Arel.sql(quoted_posts_id)) end end @@ -874,14 +874,14 @@ class EagerAssociationTest < ActiveRecord::TestCase posts(:thinking, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: "UPPER(posts.title)", limit: 2, offset: 1 + order: Arel.sql("UPPER(posts.title)"), limit: 2, offset: 1 ).to_a ) assert_equal( posts(:sti_post_and_comments, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: "UPPER(posts.title) DESC", limit: 2, offset: 1 + order: Arel.sql("UPPER(posts.title) DESC"), limit: 2, offset: 1 ).to_a ) end @@ -891,14 +891,14 @@ class EagerAssociationTest < ActiveRecord::TestCase posts(:thinking, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: ["UPPER(posts.title)", "posts.id"], limit: 2, offset: 1 + order: [Arel.sql("UPPER(posts.title)"), "posts.id"], limit: 2, offset: 1 ).to_a ) assert_equal( posts(:sti_post_and_comments, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: ["UPPER(posts.title) DESC", "posts.id"], limit: 2, offset: 1 + order: [Arel.sql("UPPER(posts.title) DESC"), "posts.id"], limit: 2, offset: 1 ).to_a ) end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 6bd11a5d81..4ca11af791 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -831,7 +831,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_find_scoped_grouped_having - assert_equal 1, authors(:david).popular_grouped_posts.length + assert_equal 2, authors(:david).popular_grouped_posts.length assert_equal 0, authors(:mary).popular_grouped_posts.length end diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index ab3bf5eb8d..65d30d011b 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -24,6 +24,11 @@ require "models/category" require "models/categorization" require "models/membership" require "models/essay" +require "models/hotel" +require "models/department" +require "models/chef" +require "models/cake_designer" +require "models/drink_designer" class NestedThroughAssociationsTest < ActiveRecord::TestCase fixtures :authors, :author_addresses, :books, :posts, :subscriptions, :subscribers, :tags, :taggings, @@ -574,6 +579,37 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase assert !c.post_taggings.empty? end + def test_polymorphic_has_many_through_when_through_association_has_not_loaded + cake_designer = CakeDesigner.create!(chef: Chef.new) + drink_designer = DrinkDesigner.create!(chef: Chef.new) + department = Department.create!(chefs: [cake_designer.chef, drink_designer.chef]) + Hotel.create!(departments: [department]) + hotel = Hotel.includes(:cake_designers, :drink_designers).take + + assert_equal [cake_designer], hotel.cake_designers + assert_equal [drink_designer], hotel.drink_designers + end + + def test_polymorphic_has_many_through_when_through_association_has_already_loaded + cake_designer = CakeDesigner.create!(chef: Chef.new) + drink_designer = DrinkDesigner.create!(chef: Chef.new) + department = Department.create!(chefs: [cake_designer.chef, drink_designer.chef]) + Hotel.create!(departments: [department]) + hotel = Hotel.includes(:chefs, :cake_designers, :drink_designers).take + + assert_equal [cake_designer], hotel.cake_designers + assert_equal [drink_designer], hotel.drink_designers + end + + def test_polymorphic_has_many_through_joined_different_table_twice + cake_designer = CakeDesigner.create!(chef: Chef.new) + drink_designer = DrinkDesigner.create!(chef: Chef.new) + department = Department.create!(chefs: [cake_designer.chef, drink_designer.chef]) + hotel = Hotel.create!(departments: [department]) + + assert_equal hotel, Hotel.joins(:cake_designers, :drink_designers).take + end + private def assert_includes_and_joins_equal(query, expected, association) diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 2f42684212..c48f7d3518 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -200,12 +200,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase if current_adapter?(:Mysql2Adapter) test "read attributes_before_type_cast on a boolean" do bool = Boolean.create!("value" => false) - if RUBY_PLATFORM.include?("java") - # JRuby will return the value before typecast as string. - assert_equal "0", bool.reload.attributes_before_type_cast["value"] - else - assert_equal 0, bool.reload.attributes_before_type_cast["value"] - end + assert_equal 0, bool.reload.attributes_before_type_cast["value"] end end diff --git a/activerecord/test/cases/attribute_set_test.rb b/activerecord/test/cases/attribute_set_test.rb deleted file mode 100644 index 8be77ed88f..0000000000 --- a/activerecord/test/cases/attribute_set_test.rb +++ /dev/null @@ -1,255 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" - -module ActiveRecord - class AttributeSetTest < ActiveRecord::TestCase - test "building a new set from raw attributes" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) - attributes = builder.build_from_database(foo: "1.1", bar: "2.2") - - assert_equal 1, attributes[:foo].value - assert_equal 2.2, attributes[:bar].value - assert_equal :foo, attributes[:foo].name - assert_equal :bar, attributes[:bar].name - end - - test "building with custom types" do - builder = AttributeSet::Builder.new(foo: Type::Float.new) - attributes = builder.build_from_database({ foo: "3.3", bar: "4.4" }, { bar: Type::Integer.new }) - - assert_equal 3.3, attributes[:foo].value - assert_equal 4, attributes[:bar].value - end - - test "[] returns a null object" do - builder = AttributeSet::Builder.new(foo: Type::Float.new) - attributes = builder.build_from_database(foo: "3.3") - - assert_equal "3.3", attributes[:foo].value_before_type_cast - assert_nil attributes[:bar].value_before_type_cast - assert_equal :bar, attributes[:bar].name - end - - test "duping creates a new hash, but does not dup the attributes" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::String.new) - attributes = builder.build_from_database(foo: 1, bar: "foo") - - # Ensure the type cast value is cached - attributes[:foo].value - attributes[:bar].value - - duped = attributes.dup - duped.write_from_database(:foo, 2) - duped[:bar].value << "bar" - - assert_equal 1, attributes[:foo].value - assert_equal 2, duped[:foo].value - assert_equal "foobar", attributes[:bar].value - assert_equal "foobar", duped[:bar].value - end - - test "deep_duping creates a new hash and dups each attribute" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::String.new) - attributes = builder.build_from_database(foo: 1, bar: "foo") - - # Ensure the type cast value is cached - attributes[:foo].value - attributes[:bar].value - - duped = attributes.deep_dup - duped.write_from_database(:foo, 2) - duped[:bar].value << "bar" - - assert_equal 1, attributes[:foo].value - assert_equal 2, duped[:foo].value - assert_equal "foo", attributes[:bar].value - assert_equal "foobar", duped[:bar].value - end - - test "freezing cloned set does not freeze original" do - attributes = AttributeSet.new({}) - clone = attributes.clone - - clone.freeze - - assert clone.frozen? - assert_not attributes.frozen? - end - - test "to_hash returns a hash of the type cast values" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) - attributes = builder.build_from_database(foo: "1.1", bar: "2.2") - - assert_equal({ foo: 1, bar: 2.2 }, attributes.to_hash) - assert_equal({ foo: 1, bar: 2.2 }, attributes.to_h) - end - - test "to_hash maintains order" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) - attributes = builder.build_from_database(foo: "2.2", bar: "3.3") - - attributes[:bar] - hash = attributes.to_h - - assert_equal [[:foo, 2], [:bar, 3.3]], hash.to_a - end - - test "values_before_type_cast" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Integer.new) - attributes = builder.build_from_database(foo: "1.1", bar: "2.2") - - assert_equal({ foo: "1.1", bar: "2.2" }, attributes.values_before_type_cast) - end - - test "known columns are built with uninitialized attributes" do - attributes = attributes_with_uninitialized_key - assert attributes[:foo].initialized? - assert_not attributes[:bar].initialized? - end - - test "uninitialized attributes are not included in the attributes hash" do - attributes = attributes_with_uninitialized_key - assert_equal({ foo: 1 }, attributes.to_hash) - end - - test "uninitialized attributes are not included in keys" do - attributes = attributes_with_uninitialized_key - assert_equal [:foo], attributes.keys - end - - test "uninitialized attributes return false for key?" do - attributes = attributes_with_uninitialized_key - assert attributes.key?(:foo) - assert_not attributes.key?(:bar) - end - - test "unknown attributes return false for key?" do - attributes = attributes_with_uninitialized_key - assert_not attributes.key?(:wibble) - end - - test "fetch_value returns the value for the given initialized attribute" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) - attributes = builder.build_from_database(foo: "1.1", bar: "2.2") - - assert_equal 1, attributes.fetch_value(:foo) - assert_equal 2.2, attributes.fetch_value(:bar) - end - - test "fetch_value returns nil for unknown attributes" do - attributes = attributes_with_uninitialized_key - assert_nil attributes.fetch_value(:wibble) { "hello" } - end - - test "fetch_value returns nil for unknown attributes when types has a default" do - types = Hash.new(Type::Value.new) - builder = AttributeSet::Builder.new(types) - attributes = builder.build_from_database - - assert_nil attributes.fetch_value(:wibble) { "hello" } - end - - test "fetch_value uses the given block for uninitialized attributes" do - attributes = attributes_with_uninitialized_key - value = attributes.fetch_value(:bar) { |n| n.to_s + "!" } - assert_equal "bar!", value - end - - test "fetch_value returns nil for uninitialized attributes if no block is given" do - attributes = attributes_with_uninitialized_key - assert_nil attributes.fetch_value(:bar) - end - - test "the primary_key is always initialized" do - builder = AttributeSet::Builder.new({ foo: Type::Integer.new }, :foo) - attributes = builder.build_from_database - - assert attributes.key?(:foo) - assert_equal [:foo], attributes.keys - assert attributes[:foo].initialized? - end - - class MyType - def cast(value) - return if value.nil? - value + " from user" - end - - def deserialize(value) - return if value.nil? - value + " from database" - end - - def assert_valid_value(*) - end - end - - test "write_from_database sets the attribute with database typecasting" do - builder = AttributeSet::Builder.new(foo: MyType.new) - attributes = builder.build_from_database - - assert_nil attributes.fetch_value(:foo) - - attributes.write_from_database(:foo, "value") - - assert_equal "value from database", attributes.fetch_value(:foo) - end - - test "write_from_user sets the attribute with user typecasting" do - builder = AttributeSet::Builder.new(foo: MyType.new) - attributes = builder.build_from_database - - assert_nil attributes.fetch_value(:foo) - - attributes.write_from_user(:foo, "value") - - assert_equal "value from user", attributes.fetch_value(:foo) - end - - def attributes_with_uninitialized_key - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) - builder.build_from_database(foo: "1.1") - end - - test "freezing doesn't prevent the set from materializing" do - builder = AttributeSet::Builder.new(foo: Type::String.new) - attributes = builder.build_from_database(foo: "1") - - attributes.freeze - assert_equal({ foo: "1" }, attributes.to_hash) - end - - test "#accessed_attributes returns only attributes which have been read" do - builder = AttributeSet::Builder.new(foo: Type::Value.new, bar: Type::Value.new) - attributes = builder.build_from_database(foo: "1", bar: "2") - - assert_equal [], attributes.accessed - - attributes.fetch_value(:foo) - - assert_equal [:foo], attributes.accessed - end - - test "#map returns a new attribute set with the changes applied" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Integer.new) - attributes = builder.build_from_database(foo: "1", bar: "2") - new_attributes = attributes.map do |attr| - attr.with_cast_value(attr.value + 1) - end - - assert_equal 2, new_attributes.fetch_value(:foo) - assert_equal 3, new_attributes.fetch_value(:bar) - end - - test "comparison for equality is correctly implemented" do - builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Integer.new) - attributes = builder.build_from_database(foo: "1", bar: "2") - attributes2 = builder.build_from_database(foo: "1", bar: "2") - attributes3 = builder.build_from_database(foo: "2", bar: "2") - - assert_equal attributes, attributes2 - assert_not_equal attributes2, attributes3 - end - end -end diff --git a/activerecord/test/cases/attribute_test.rb b/activerecord/test/cases/attribute_test.rb deleted file mode 100644 index 1731e7926e..0000000000 --- a/activerecord/test/cases/attribute_test.rb +++ /dev/null @@ -1,255 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" - -module ActiveRecord - class AttributeTest < ActiveRecord::TestCase - setup do - @type = Minitest::Mock.new - end - - teardown do - assert @type.verify - end - - test "from_database + read type casts from database" do - @type.expect(:deserialize, "type cast from database", ["a value"]) - attribute = Attribute.from_database(nil, "a value", @type) - - type_cast_value = attribute.value - - assert_equal "type cast from database", type_cast_value - end - - test "from_user + read type casts from user" do - @type.expect(:cast, "type cast from user", ["a value"]) - attribute = Attribute.from_user(nil, "a value", @type) - - type_cast_value = attribute.value - - assert_equal "type cast from user", type_cast_value - end - - test "reading memoizes the value" do - @type.expect(:deserialize, "from the database", ["whatever"]) - attribute = Attribute.from_database(nil, "whatever", @type) - - type_cast_value = attribute.value - second_read = attribute.value - - assert_equal "from the database", type_cast_value - assert_same type_cast_value, second_read - end - - test "reading memoizes falsy values" do - @type.expect(:deserialize, false, ["whatever"]) - attribute = Attribute.from_database(nil, "whatever", @type) - - attribute.value - attribute.value - end - - test "read_before_typecast returns the given value" do - attribute = Attribute.from_database(nil, "raw value", @type) - - raw_value = attribute.value_before_type_cast - - assert_equal "raw value", raw_value - end - - test "from_database + read_for_database type casts to and from database" do - @type.expect(:deserialize, "read from database", ["whatever"]) - @type.expect(:serialize, "ready for database", ["read from database"]) - attribute = Attribute.from_database(nil, "whatever", @type) - - serialize = attribute.value_for_database - - assert_equal "ready for database", serialize - end - - test "from_user + read_for_database type casts from the user to the database" do - @type.expect(:cast, "read from user", ["whatever"]) - @type.expect(:serialize, "ready for database", ["read from user"]) - attribute = Attribute.from_user(nil, "whatever", @type) - - serialize = attribute.value_for_database - - assert_equal "ready for database", serialize - end - - test "duping dups the value" do - @type.expect(:deserialize, "type cast".dup, ["a value"]) - attribute = Attribute.from_database(nil, "a value", @type) - - value_from_orig = attribute.value - value_from_clone = attribute.dup.value - value_from_orig << " foo" - - assert_equal "type cast foo", value_from_orig - assert_equal "type cast", value_from_clone - end - - test "duping does not dup the value if it is not dupable" do - @type.expect(:deserialize, false, ["a value"]) - attribute = Attribute.from_database(nil, "a value", @type) - - assert_same attribute.value, attribute.dup.value - end - - test "duping does not eagerly type cast if we have not yet type cast" do - attribute = Attribute.from_database(nil, "a value", @type) - attribute.dup - end - - class MyType - def cast(value) - value + " from user" - end - - def deserialize(value) - value + " from database" - end - - def assert_valid_value(*) - end - end - - test "with_value_from_user returns a new attribute with the value from the user" do - old = Attribute.from_database(nil, "old", MyType.new) - new = old.with_value_from_user("new") - - assert_equal "old from database", old.value - assert_equal "new from user", new.value - end - - test "with_value_from_database returns a new attribute with the value from the database" do - old = Attribute.from_user(nil, "old", MyType.new) - new = old.with_value_from_database("new") - - assert_equal "old from user", old.value - assert_equal "new from database", new.value - end - - test "uninitialized attributes yield their name if a block is given to value" do - block = proc { |name| name.to_s + "!" } - foo = Attribute.uninitialized(:foo, nil) - bar = Attribute.uninitialized(:bar, nil) - - assert_equal "foo!", foo.value(&block) - assert_equal "bar!", bar.value(&block) - end - - test "uninitialized attributes have no value" do - assert_nil Attribute.uninitialized(:foo, nil).value - end - - test "attributes equal other attributes with the same constructor arguments" do - first = Attribute.from_database(:foo, 1, Type::Integer.new) - second = Attribute.from_database(:foo, 1, Type::Integer.new) - assert_equal first, second - end - - test "attributes do not equal attributes with different names" do - first = Attribute.from_database(:foo, 1, Type::Integer.new) - second = Attribute.from_database(:bar, 1, Type::Integer.new) - assert_not_equal first, second - end - - test "attributes do not equal attributes with different types" do - first = Attribute.from_database(:foo, 1, Type::Integer.new) - second = Attribute.from_database(:foo, 1, Type::Float.new) - assert_not_equal first, second - end - - test "attributes do not equal attributes with different values" do - first = Attribute.from_database(:foo, 1, Type::Integer.new) - second = Attribute.from_database(:foo, 2, Type::Integer.new) - assert_not_equal first, second - end - - test "attributes do not equal attributes of other classes" do - first = Attribute.from_database(:foo, 1, Type::Integer.new) - second = Attribute.from_user(:foo, 1, Type::Integer.new) - assert_not_equal first, second - end - - test "an attribute has not been read by default" do - attribute = Attribute.from_database(:foo, 1, Type::Value.new) - assert_not attribute.has_been_read? - end - - test "an attribute has been read when its value is calculated" do - attribute = Attribute.from_database(:foo, 1, Type::Value.new) - attribute.value - assert attribute.has_been_read? - end - - test "an attribute is not changed if it hasn't been assigned or mutated" do - attribute = Attribute.from_database(:foo, 1, Type::Value.new) - - refute attribute.changed? - end - - test "an attribute is changed if it's been assigned a new value" do - attribute = Attribute.from_database(:foo, 1, Type::Value.new) - changed = attribute.with_value_from_user(2) - - assert changed.changed? - end - - test "an attribute is not changed if it's assigned the same value" do - attribute = Attribute.from_database(:foo, 1, Type::Value.new) - unchanged = attribute.with_value_from_user(1) - - refute unchanged.changed? - end - - test "an attribute can not be mutated if it has not been read, - and skips expensive calculations" do - type_which_raises_from_all_methods = Object.new - attribute = Attribute.from_database(:foo, "bar", type_which_raises_from_all_methods) - - assert_not attribute.changed_in_place? - end - - test "an attribute is changed if it has been mutated" do - attribute = Attribute.from_database(:foo, "bar", Type::String.new) - attribute.value << "!" - - assert attribute.changed_in_place? - assert attribute.changed? - end - - test "an attribute can forget its changes" do - attribute = Attribute.from_database(:foo, "bar", Type::String.new) - changed = attribute.with_value_from_user("foo") - forgotten = changed.forgetting_assignment - - assert changed.changed? # sanity check - refute forgotten.changed? - end - - test "with_value_from_user validates the value" do - type = Type::Value.new - type.define_singleton_method(:assert_valid_value) do |value| - if value == 1 - raise ArgumentError - end - end - - attribute = Attribute.from_database(:foo, 1, type) - assert_equal 1, attribute.value - assert_equal 2, attribute.with_value_from_user(2).value - assert_raises ArgumentError do - attribute.with_value_from_user(1) - end - end - - test "with_type preserves mutations" do - attribute = Attribute.from_database(:foo, "".dup, Type::Value.new) - attribute.value << "1" - - assert_equal 1, attribute.with_type(Type::Integer.new).value - end - end -end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index f0ef522515..d79afa2ee9 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1476,4 +1476,25 @@ class BasicsTest < ActiveRecord::TestCase assert_equal(%w(first_name last_name), Developer.ignored_columns) assert_equal(%w(first_name last_name), SymbolIgnoredDeveloper.ignored_columns) end + + test "when #reload called, ignored columns' attribute methods are not defined" do + developer = Developer.create!(name: "Developer") + refute developer.respond_to?(:first_name) + refute developer.respond_to?(:first_name=) + + developer.reload + + refute developer.respond_to?(:first_name) + refute developer.respond_to?(:first_name=) + end + + test "ignored columns not included in SELECT" do + query = Developer.all.to_sql.downcase + + # ignored column + refute query.include?("first_name") + + # regular column + assert query.include?("name") + end end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 66bc14b5ab..55b50e4f84 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -663,14 +663,14 @@ class CalculationsTest < ActiveRecord::TestCase end def test_pluck_with_selection_clause - assert_equal [50, 53, 55, 60], Account.pluck("DISTINCT credit_limit").sort - assert_equal [50, 53, 55, 60], Account.pluck("DISTINCT accounts.credit_limit").sort - assert_equal [50, 53, 55, 60], Account.pluck("DISTINCT(credit_limit)").sort + assert_equal [50, 53, 55, 60], Account.pluck(Arel.sql("DISTINCT credit_limit")).sort + assert_equal [50, 53, 55, 60], Account.pluck(Arel.sql("DISTINCT accounts.credit_limit")).sort + assert_equal [50, 53, 55, 60], Account.pluck(Arel.sql("DISTINCT(credit_limit)")).sort # MySQL returns "SUM(DISTINCT(credit_limit))" as the column name unless # an alias is provided. Without the alias, the column cannot be found # and properly typecast. - assert_equal [50 + 53 + 55 + 60], Account.pluck("SUM(DISTINCT(credit_limit)) as credit_limit") + assert_equal [50 + 53 + 55 + 60], Account.pluck(Arel.sql("SUM(DISTINCT(credit_limit)) as credit_limit")) end def test_plucks_with_ids @@ -772,7 +772,7 @@ class CalculationsTest < ActiveRecord::TestCase companies = Company.order(:name).limit(3).load assert_queries 1 do - assert_equal ["37signals", "Apex", "Ex Nihilo"], companies.pluck("DISTINCT name") + assert_equal ["37signals", "Apex", "Ex Nihilo"], companies.pluck(Arel.sql("DISTINCT name")) end end diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index c137693211..19d6464a22 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -73,6 +73,16 @@ module ActiveRecord assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 end + test "cache_key for relation with includes" do + comments = Comment.includes(:post).where("posts.type": "Post") + assert_match(/\Acomments\/query-(\h+)-(\d+)-(\d+)\z/, comments.cache_key) + end + + test "cache_key for loaded relation with includes" do + comments = Comment.includes(:post).where("posts.type": "Post").load + assert_match(/\Acomments\/query-(\h+)-(\d+)-(\d+)\z/, comments.cache_key) + end + test "it triggers at most one query" do developers = Developer.where(name: "David") diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 74d0ed348e..cae74a2b9b 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -1,10 +1,15 @@ # frozen_string_literal: true require "cases/helper" +require "models/person" module ActiveRecord module ConnectionAdapters class ConnectionHandlerTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + fixtures :people + def setup @handler = ConnectionHandler.new @spec_name = "primary" @@ -139,6 +144,33 @@ module ActiveRecord rd.close end + def test_forked_child_doesnt_mangle_parent_connection + object_id = ActiveRecord::Base.connection.object_id + assert ActiveRecord::Base.connection.active? + + rd, wr = IO.pipe + rd.binmode + wr.binmode + + pid = fork { + rd.close + if ActiveRecord::Base.connection.active? + wr.write Marshal.dump ActiveRecord::Base.connection.object_id + end + wr.close + + exit # allow finalizers to run + } + + wr.close + + Process.waitpid pid + assert_not_equal object_id, Marshal.load(rd.read) + rd.close + + assert_equal 3, ActiveRecord::Base.connection.select_value("SELECT COUNT(*) FROM people") + end + def test_retrieve_connection_pool_copies_schema_cache_from_ancestor_pool @pool.schema_cache = @pool.connection.schema_cache @pool.schema_cache.add("posts") diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 2bfe490602..cb2fefb4f6 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -205,6 +205,14 @@ module ActiveRecord end.join end + def test_checkout_order_is_lifo + conn1 = @pool.checkout + conn2 = @pool.checkout + @pool.checkin conn1 + @pool.checkin conn2 + assert_equal [conn2, conn1], 2.times.map { @pool.checkout } + end + # The connection pool is "fair" if threads waiting for # connections receive them in the order in which they began # waiting. This ensures that we don't timeout one HTTP request diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index d03af7f111..b5c9bdf3a7 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -239,7 +239,7 @@ class FinderTest < ActiveRecord::TestCase # Ensure +exists?+ runs without an error by excluding order value. def test_exists_with_order - assert_equal true, Topic.order("invalid sql here").exists? + assert_equal true, Topic.order(Arel.sql("invalid sql here")).exists? end def test_exists_with_joins @@ -652,7 +652,7 @@ class FinderTest < ActiveRecord::TestCase def test_last_with_irreversible_order assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("coalesce(author_name, title)").last + Topic.order(Arel.sql("coalesce(author_name, title)")).last end end diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb index 60c628511f..ebe0b0aa87 100644 --- a/activerecord/test/cases/invertible_migration_test.rb +++ b/activerecord/test/cases/invertible_migration_test.rb @@ -161,6 +161,15 @@ module ActiveRecord end end + class UpOnlyMigration < SilentMigration + def change + add_column :horses, :oldie, :integer, default: 0 + up_only { execute "update horses set oldie = 1" } + end + end + + self.use_transactional_tests = false + setup do @verbose_was, ActiveRecord::Migration.verbose = ActiveRecord::Migration.verbose, false end @@ -378,5 +387,23 @@ module ActiveRecord "horses_index_named index should not exist" end end + + def test_up_only + InvertibleMigration.new.migrate(:up) + horse1 = Horse.create + # populates existing horses with oldie = 1 but new ones have default 0 + UpOnlyMigration.new.migrate(:up) + Horse.reset_column_information + horse1.reload + horse2 = Horse.create + + assert 1, horse1.oldie # created before migration + assert 0, horse2.oldie # created after migration + + UpOnlyMigration.new.migrate(:down) # should be no error + connection = ActiveRecord::Base.connection + assert !connection.column_exists?(:horses, :oldie) + Horse.reset_column_information + end end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 6791d50940..e857180bd1 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -611,14 +611,12 @@ unless in_memory_db? end end - if current_adapter?(:PostgreSQLAdapter, :OracleAdapter) - def test_no_locks_no_wait - first, second = duel { Person.find 1 } - assert first.end > second.end - end - - private + def test_no_locks_no_wait + first, second = duel { Person.find 1 } + assert first.end > second.end + end + private def duel(zzz = 5) t0, t1, t2, t3 = nil, nil, nil, nil @@ -646,6 +644,5 @@ unless in_memory_db? assert t3 > t2 [t0.to_f..t1.to_f, t2.to_f..t3.to_f] end - end end end diff --git a/activerecord/test/cases/migrator_test.rb b/activerecord/test/cases/migrator_test.rb index ee10be119c..1047ba1367 100644 --- a/activerecord/test/cases/migrator_test.rb +++ b/activerecord/test/cases/migrator_test.rb @@ -66,6 +66,26 @@ class MigratorTest < ActiveRecord::TestCase list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] ActiveRecord::Migrator.new(:up, list, 3).run end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, -1).run + end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, 0).run + end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, 3).migrate + end + + assert_raises(ActiveRecord::UnknownMigrationVersionError) do + list = [ActiveRecord::Migration.new("Foo", 1), ActiveRecord::Migration.new("Bar", 2)] + ActiveRecord::Migrator.new(:up, list, -1).migrate + end end def test_finds_migrations diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 6cbe18cc8c..f088c064f5 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -94,27 +94,31 @@ class PersistenceTest < ActiveRecord::TestCase end def test_delete_all_with_joins_and_where_part_is_hash - where_args = { toys: { name: "Bone" } } - count = Pet.joins(:toys).where(where_args).count + pets = Pet.joins(:toys).where(toys: { name: "Bone" }) - assert_equal count, 1 - assert_equal count, Pet.joins(:toys).where(where_args).delete_all + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all + end + + def test_delete_all_with_joins_and_where_part_is_not_hash + pets = Pet.joins(:toys).where("toys.name = ?", "Bone") + + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all end def test_delete_all_with_left_joins - where_args = { toys: { name: "Bone" } } - count = Pet.left_joins(:toys).where(where_args).count + pets = Pet.left_joins(:toys).where(toys: { name: "Bone" }) - assert_equal count, 1 - assert_equal count, Pet.left_joins(:toys).where(where_args).delete_all + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all end - def test_delete_all_with_joins_and_where_part_is_not_hash - where_args = ["toys.name = ?", "Bone"] - count = Pet.joins(:toys).where(where_args).count + def test_delete_all_with_includes + pets = Pet.includes(:toys).where(toys: { name: "Bone" }) - assert_equal count, 1 - assert_equal count, Pet.joins(:toys).where(where_args).delete_all + assert_equal true, pets.exists? + assert_equal pets.count, pets.delete_all end def test_increment_attribute @@ -496,17 +500,24 @@ class PersistenceTest < ActiveRecord::TestCase end def test_update_all_with_joins - where_args = { toys: { name: "Bone" } } - count = Pet.left_joins(:toys).where(where_args).count + pets = Pet.joins(:toys).where(toys: { name: "Bone" }) - assert_equal count, Pet.joins(:toys).where(where_args).update_all(name: "Bob") + assert_equal true, pets.exists? + assert_equal pets.count, pets.update_all(name: "Bob") end def test_update_all_with_left_joins - where_args = { toys: { name: "Bone" } } - count = Pet.left_joins(:toys).where(where_args).count + pets = Pet.left_joins(:toys).where(toys: { name: "Bone" }) + + assert_equal true, pets.exists? + assert_equal pets.count, pets.update_all(name: "Bob") + end + + def test_update_all_with_includes + pets = Pet.includes(:toys).where(toys: { name: "Bone" }) - assert_equal count, Pet.left_joins(:toys).where(where_args).update_all(name: "Bob") + assert_equal true, pets.exists? + assert_equal pets.count, pets.update_all(name: "Bob") end def test_update_all_with_non_standard_table_name diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 5cb537b623..46f90b0bca 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -452,6 +452,15 @@ class QueryCacheTest < ActiveRecord::TestCase end end + def test_query_cache_is_enabled_on_all_connection_pools + middleware { + ActiveRecord::Base.connection_handler.connection_pool_list.each do |pool| + assert pool.query_cache_enabled + assert pool.connection.query_cache_enabled + end + }.call({}) + end + private def middleware(&app) executor = Class.new(ActiveSupport::Executor) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index fd5985ffe7..a71d8de521 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -274,18 +274,32 @@ module ActiveRecord assert_equal({ 2 => 1, 4 => 3, 5 => 1 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count) end + def test_relation_merging_keeps_joining_order + authors = Author.where(id: 1) + posts = Post.joins(:author).merge(authors) + comments = Comment.joins(:post).merge(posts) + ratings = Rating.joins(:comment).merge(comments) + + assert_equal 3, ratings.count + end + class EnsureRoundTripTypeCasting < ActiveRecord::Type::Value def type :string end + def cast(value) + raise value unless value == "value from user" + "cast value" + end + def deserialize(value) raise value unless value == "type cast for database" "type cast from database" end def serialize(value) - raise value unless value == "value from user" + raise value unless value == "cast value" "type cast for database" end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 72433d1e8e..50ad1d5b26 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -195,6 +195,18 @@ class RelationTest < ActiveRecord::TestCase assert_equal(relation.map(&:post_count).sort, subquery.values.sort) end + def test_finding_with_subquery_with_eager_loading_in_from + relation = Comment.includes(:post).where("posts.type": "Post") + assert_equal relation.to_a, Comment.select("*").from(relation).to_a + assert_equal relation.to_a, Comment.select("subquery.*").from(relation).to_a + assert_equal relation.to_a, Comment.select("a.*").from(relation, :a).to_a + end + + def test_finding_with_subquery_with_eager_loading_in_where + relation = Comment.includes(:post).where("posts.type": "Post") + assert_equal relation.sort_by(&:id), Comment.where(id: relation).sort_by(&:id) + end + def test_finding_with_conditions assert_equal ["David"], Author.where(name: "David").map(&:name) assert_equal ["Mary"], Author.where(["name = ?", "Mary"]).map(&:name) @@ -238,7 +250,7 @@ class RelationTest < ActiveRecord::TestCase end def test_reverse_order_with_function - topics = Topic.order("length(title)").reverse_order + topics = Topic.order(Arel.sql("length(title)")).reverse_order assert_equal topics(:second).title, topics.first.title end @@ -248,24 +260,24 @@ class RelationTest < ActiveRecord::TestCase end def test_reverse_order_with_function_other_predicates - topics = Topic.order("author_name, length(title), id").reverse_order + topics = Topic.order(Arel.sql("author_name, length(title), id")).reverse_order assert_equal topics(:second).title, topics.first.title - topics = Topic.order("length(author_name), id, length(title)").reverse_order + topics = Topic.order(Arel.sql("length(author_name), id, length(title)")).reverse_order assert_equal topics(:fifth).title, topics.first.title end def test_reverse_order_with_multiargument_function assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("concat(author_name, title)").reverse_order + Topic.order(Arel.sql("concat(author_name, title)")).reverse_order end assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("concat(lower(author_name), title)").reverse_order + Topic.order(Arel.sql("concat(lower(author_name), title)")).reverse_order end assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("concat(author_name, lower(title))").reverse_order + Topic.order(Arel.sql("concat(author_name, lower(title))")).reverse_order end assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("concat(lower(author_name), title, length(title)").reverse_order + Topic.order(Arel.sql("concat(lower(author_name), title, length(title)")).reverse_order end end @@ -277,10 +289,10 @@ class RelationTest < ActiveRecord::TestCase def test_reverse_order_with_nulls_first_or_last assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("title NULLS FIRST").reverse_order + Topic.order(Arel.sql("title NULLS FIRST")).reverse_order end assert_raises(ActiveRecord::IrreversibleOrderError) do - Topic.order("title nulls last").reverse_order + Topic.order(Arel.sql("title nulls last")).reverse_order end end @@ -373,29 +385,29 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_cross_table_order_and_limit tags = Tag.includes(:taggings). - order("tags.name asc", "taggings.taggable_id asc", "REPLACE('abc', taggings.taggable_type, taggings.taggable_type)"). + order("tags.name asc", "taggings.taggable_id asc", Arel.sql("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)")). limit(1).to_a assert_equal 1, tags.length end def test_finding_with_complex_order_and_limit - tags = Tag.includes(:taggings).references(:taggings).order("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)").limit(1).to_a + tags = Tag.includes(:taggings).references(:taggings).order(Arel.sql("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)")).limit(1).to_a assert_equal 1, tags.length end def test_finding_with_complex_order - tags = Tag.includes(:taggings).references(:taggings).order("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)").to_a + tags = Tag.includes(:taggings).references(:taggings).order(Arel.sql("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)")).to_a assert_equal 3, tags.length end def test_finding_with_sanitized_order - query = Tag.order(["field(id, ?)", [1, 3, 2]]).to_sql + query = Tag.order([Arel.sql("field(id, ?)"), [1, 3, 2]]).to_sql assert_match(/field\(id, 1,3,2\)/, query) - query = Tag.order(["field(id, ?)", []]).to_sql + query = Tag.order([Arel.sql("field(id, ?)"), []]).to_sql assert_match(/field\(id, NULL\)/, query) - query = Tag.order(["field(id, ?)", nil]).to_sql + query = Tag.order([Arel.sql("field(id, ?)"), nil]).to_sql assert_match(/field\(id, NULL\)/, query) end @@ -927,13 +939,13 @@ class RelationTest < ActiveRecord::TestCase assert_equal 11, posts.count(:all) assert_equal 11, posts.count(:id) - assert_equal 1, posts.where("comments_count > 1").count - assert_equal 9, posts.where(comments_count: 0).count + assert_equal 3, posts.where("comments_count > 1").count + assert_equal 6, posts.where(comments_count: 0).count end def test_count_with_block posts = Post.all - assert_equal 10, posts.count { |p| p.comments_count.even? } + assert_equal 8, posts.count { |p| p.comments_count.even? } end def test_count_on_association_relation @@ -950,10 +962,10 @@ class RelationTest < ActiveRecord::TestCase def test_count_with_distinct posts = Post.all - assert_equal 3, posts.distinct(true).count(:comments_count) + assert_equal 4, posts.distinct(true).count(:comments_count) assert_equal 11, posts.distinct(false).count(:comments_count) - assert_equal 3, posts.distinct(true).select(:comments_count).count + assert_equal 4, posts.distinct(true).select(:comments_count).count assert_equal 11, posts.distinct(false).select(:comments_count).count end @@ -992,7 +1004,7 @@ class RelationTest < ActiveRecord::TestCase best_posts = posts.where(comments_count: 0) best_posts.load # force load - assert_no_queries { assert_equal 9, best_posts.size } + assert_no_queries { assert_equal 6, best_posts.size } end def test_size_with_limit @@ -1003,7 +1015,7 @@ class RelationTest < ActiveRecord::TestCase best_posts = posts.where(comments_count: 0) best_posts.load # force load - assert_no_queries { assert_equal 9, best_posts.size } + assert_no_queries { assert_equal 6, best_posts.size } end def test_size_with_zero_limit @@ -1026,7 +1038,7 @@ class RelationTest < ActiveRecord::TestCase def test_count_complex_chained_relations posts = Post.select("comments_count").where("id is not null").group("author_id").where("comments_count > 0") - expected = { 1 => 2 } + expected = { 1 => 4, 2 => 1 } assert_equal expected, posts.count end @@ -1567,7 +1579,7 @@ 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}") + scope = Post.order(Arel.sql("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}")) if current_adapter?(:OracleAdapter) assert_equal ["COMMENTS"], scope.references_values else @@ -1584,7 +1596,7 @@ class RelationTest < ActiveRecord::TestCase scope = Post.order("comments.body asc") assert_equal ["comments"], scope.references_values - scope = Post.order("foo(comments.body)") + scope = Post.order(Arel.sql("foo(comments.body)")) assert_equal [], scope.references_values end @@ -1592,7 +1604,7 @@ 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}") + scope = Post.reorder(Arel.sql("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}")) if current_adapter?(:OracleAdapter) assert_equal ["COMMENTS"], scope.references_values else @@ -1609,7 +1621,7 @@ class RelationTest < ActiveRecord::TestCase scope = Post.reorder("comments.body asc") assert_equal %w(comments), scope.references_values - scope = Post.reorder("foo(comments.body)") + scope = Post.reorder(Arel.sql("foo(comments.body)")) assert_equal [], scope.references_values end @@ -1781,7 +1793,7 @@ class RelationTest < ActiveRecord::TestCase end test "arel_attribute respects a custom table" do - assert_equal [posts(:welcome)], custom_post_relation.ranked_by_comments.limit_by(1).to_a + assert_equal [posts(:sti_comments)], custom_post_relation.ranked_by_comments.limit_by(1).to_a end test "alias_tracker respects a custom table" do diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 716ca29eda..fdfeabaa3b 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -120,49 +120,49 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_unscope_with_where_attributes expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(name: "David").unscope(where: :name).collect(&:name) - assert_equal expected, received + assert_equal expected.sort, received.sort expected_2 = Developer.order("salary DESC").collect(&:name) received_2 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope({ where: :name }, :select).collect(&:name) - assert_equal expected_2, received_2 + assert_equal expected_2.sort, received_2.sort expected_3 = Developer.order("salary DESC").collect(&:name) received_3 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope(:select, :where).collect(&:name) - assert_equal expected_3, received_3 + assert_equal expected_3.sort, received_3.sort expected_4 = Developer.order("salary DESC").collect(&:name) received_4 = DeveloperOrderedBySalary.where.not("name" => "Jamis").unscope(where: :name).collect(&:name) - assert_equal expected_4, received_4 + assert_equal expected_4.sort, received_4.sort expected_5 = Developer.order("salary DESC").collect(&:name) received_5 = DeveloperOrderedBySalary.where.not("name" => ["Jamis", "David"]).unscope(where: :name).collect(&:name) - assert_equal expected_5, received_5 + assert_equal expected_5.sort, received_5.sort expected_6 = Developer.order("salary DESC").collect(&:name) received_6 = DeveloperOrderedBySalary.where(Developer.arel_table["name"].eq("David")).unscope(where: :name).collect(&:name) - assert_equal expected_6, received_6 + assert_equal expected_6.sort, received_6.sort expected_7 = Developer.order("salary DESC").collect(&:name) received_7 = DeveloperOrderedBySalary.where(Developer.arel_table[:name].eq("David")).unscope(where: :name).collect(&:name) - assert_equal expected_7, received_7 + assert_equal expected_7.sort, received_7.sort end def test_unscope_comparison_where_clauses # unscoped for WHERE (`developers`.`id` <= 2) expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(id: -Float::INFINITY..2).unscope(where: :id).collect { |dev| dev.name } - assert_equal expected, received + assert_equal expected.sort, received.sort # unscoped for WHERE (`developers`.`id` < 2) expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(id: -Float::INFINITY...2).unscope(where: :id).collect { |dev| dev.name } - assert_equal expected, received + assert_equal expected.sort, received.sort end def test_unscope_multiple_where_clauses expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(name: "Jamis").where(id: 1).unscope(where: [:name, :id]).collect(&:name) - assert_equal expected, received + assert_equal expected.sort, received.sort end def test_unscope_string_where_clauses_involved @@ -172,23 +172,23 @@ class DefaultScopingTest < ActiveRecord::TestCase dev_ordered_relation = DeveloperOrderedBySalary.where(name: "Jamis").where("created_at > ?", 1.year.ago) received = dev_ordered_relation.unscope(where: [:name]).collect(&:name) - assert_equal expected, received + assert_equal expected.sort, received.sort end def test_unscope_with_grouping_attributes expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.group(:name).unscope(:group).collect(&:name) - assert_equal expected, received + assert_equal expected.sort, received.sort expected_2 = Developer.order("salary DESC").collect(&:name) received_2 = DeveloperOrderedBySalary.group("name").unscope(:group).collect(&:name) - assert_equal expected_2, received_2 + assert_equal expected_2.sort, received_2.sort end def test_unscope_with_limit_in_query expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.limit(1).unscope(:limit).collect(&:name) - assert_equal expected, received + assert_equal expected.sort, received.sort end def test_order_to_unscope_reordering @@ -472,7 +472,7 @@ class DefaultScopingTest < ActiveRecord::TestCase test "a scope can remove the condition from the default scope" do scope = DeveloperCalledJamis.david2 assert_equal 1, scope.where_clause.ast.children.length - assert_equal Developer.where(name: "David"), scope + assert_equal Developer.where(name: "David").map(&:id), scope.map(&:id) end def test_with_abstract_class_where_clause_should_not_be_duplicated diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index f3b84d88c2..116f8e83aa 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -240,6 +240,20 @@ class RelationScopingTest < ActiveRecord::TestCase assert_nil SpecialComment.current_scope end + def test_scoping_respects_current_class + Comment.unscoped do + assert_equal "a comment...", Comment.all.what_are_you + assert_equal "a special comment...", SpecialComment.all.what_are_you + end + end + + def test_scoping_respects_sti_constraint + Comment.unscoped do + assert_equal comments(:greetings), Comment.find(1) + assert_raises(ActiveRecord::RecordNotFound) { SpecialComment.find(1) } + end + end + def test_circular_joins_with_scoping_does_not_crash posts = Post.joins(comments: :post).scoping do Post.first(10) diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index 1495d2ab89..5a094ead42 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -357,8 +357,15 @@ module ActiveRecord ActiveRecord::Migration.expects(:verbose=).with(ActiveRecord::Migration.verbose) ActiveRecord::Tasks::DatabaseTasks.migrate + ENV["VERBOSE"] = "" + ENV["VERSION"] = "" + ActiveRecord::Migrator.expects(:migrate).with("custom/path", nil) + ActiveRecord::Migration.expects(:verbose=).with(true) + ActiveRecord::Migration.expects(:verbose=).with(ActiveRecord::Migration.verbose) + ActiveRecord::Tasks::DatabaseTasks.migrate + ENV["VERBOSE"] = "yes" - ENV["VERSION"] = "unknown" + ENV["VERSION"] = "0" ActiveRecord::Migrator.expects(:migrate).with("custom/path", 0) ActiveRecord::Migration.expects(:verbose=).with(true) ActiveRecord::Migration.expects(:verbose=).with(ActiveRecord::Migration.verbose) @@ -367,15 +374,47 @@ module ActiveRecord ENV["VERBOSE"], ENV["VERSION"] = verbose, version end - def test_migrate_raise_error_on_empty_version + def test_migrate_raise_error_on_invalid_version_format version = ENV["VERSION"] - ENV["VERSION"] = "" + + ENV["VERSION"] = "unknown" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0 " + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1." + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_" e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } - assert_equal "Empty VERSION provided", e.message + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_name" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_match(/Invalid format of target version/, e.message) ensure ENV["VERSION"] = version end + def test_migrate_raise_error_on_failed_check_target_version + ActiveRecord::Tasks::DatabaseTasks.stubs(:check_target_version).raises("foo") + + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_equal "foo", e.message + end + def test_migrate_clears_schema_cache_afterward ActiveRecord::Base.expects(:clear_cache!) ActiveRecord::Tasks::DatabaseTasks.migrate @@ -444,6 +483,108 @@ module ActiveRecord end end + class DatabaseTaskTargetVersionTest < ActiveRecord::TestCase + def test_target_version_returns_nil_if_version_does_not_exist + version = ENV.delete("VERSION") + assert_nil ActiveRecord::Tasks::DatabaseTasks.target_version + ensure + ENV["VERSION"] = version + end + + def test_target_version_returns_nil_if_version_is_empty + version = ENV["VERSION"] + + ENV["VERSION"] = "" + assert_nil ActiveRecord::Tasks::DatabaseTasks.target_version + ensure + ENV["VERSION"] = version + end + + def test_target_version_returns_converted_to_integer_env_version_if_version_exists + version = ENV["VERSION"] + + ENV["VERSION"] = "0" + assert_equal ENV["VERSION"].to_i, ActiveRecord::Tasks::DatabaseTasks.target_version + + ENV["VERSION"] = "42" + assert_equal ENV["VERSION"].to_i, ActiveRecord::Tasks::DatabaseTasks.target_version + + ENV["VERSION"] = "042" + assert_equal ENV["VERSION"].to_i, ActiveRecord::Tasks::DatabaseTasks.target_version + ensure + ENV["VERSION"] = version + end + end + + class DatabaseTaskCheckTargetVersionTest < ActiveRecord::TestCase + def test_check_target_version_does_not_raise_error_on_empty_version + version = ENV["VERSION"] + ENV["VERSION"] = "" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + ensure + ENV["VERSION"] = version + end + + def test_check_target_version_does_not_raise_error_if_version_is_not_setted + version = ENV.delete("VERSION") + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + ensure + ENV["VERSION"] = version + end + + def test_check_target_version_raises_error_on_invalid_version_format + version = ENV["VERSION"] + + ENV["VERSION"] = "unknown" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1.1.11" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "0 " + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1." + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + + ENV["VERSION"] = "1_name" + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + assert_match(/Invalid format of target version/, e.message) + ensure + ENV["VERSION"] = version + end + + def test_check_target_version_does_not_raise_error_on_valid_version_format + version = ENV["VERSION"] + + ENV["VERSION"] = "0" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + + ENV["VERSION"] = "1" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + + ENV["VERSION"] = "001" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + + ENV["VERSION"] = "001_name.rb" + assert_nothing_raised { ActiveRecord::Tasks::DatabaseTasks.check_target_version } + ensure + ENV["VERSION"] = version + end + end + class DatabaseTasksStructureDumpTest < ActiveRecord::TestCase include DatabaseTasksSetupper diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 98fe24baa0..047153e7cc 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -75,7 +75,7 @@ if current_adapter?(:Mysql2Adapter) end end - class MysqlDBCreateAsRootTest < ActiveRecord::TestCase + class MysqlDBCreateWithInvalidPermissionsTest < ActiveRecord::TestCase def setup @connection = stub("Connection", create_database: true) @error = Mysql2::Error.new("Invalid permissions") @@ -86,13 +86,8 @@ if current_adapter?(:Mysql2Adapter) "password" => "wossname" } - $stdin.stubs(:gets).returns("secret\n") - $stdout.stubs(:print).returns(nil) - @error.stubs(:errno).returns(1045) ActiveRecord::Base.stubs(:connection).returns(@connection) - ActiveRecord::Base.stubs(:establish_connection). - raises(@error). - then.returns(true) + ActiveRecord::Base.stubs(:establish_connection).raises(@error) $stdout, @original_stdout = StringIO.new, $stdout $stderr, @original_stderr = StringIO.new, $stderr @@ -102,75 +97,11 @@ if current_adapter?(:Mysql2Adapter) $stdout, $stderr = @original_stdout, @original_stderr end - def test_root_password_is_requested - assert_permissions_granted_for("pat") - $stdin.expects(:gets).returns("secret\n") - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_connection_established_as_root - assert_permissions_granted_for("pat") - ActiveRecord::Base.expects(:establish_connection).with( - "adapter" => "mysql2", - "database" => nil, - "username" => "root", - "password" => "secret" - ) - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_database_created_by_root - assert_permissions_granted_for("pat") - @connection.expects(:create_database). - with("my-app-db", {}) - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_grant_privileges_for_normal_user - assert_permissions_granted_for("pat") - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_do_not_grant_privileges_for_root_user - @configuration["username"] = "root" - @configuration["password"] = "" - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - def test_connection_established_as_normal_user - assert_permissions_granted_for("pat") - ActiveRecord::Base.expects(:establish_connection).returns do - ActiveRecord::Base.expects(:establish_connection).with( - "adapter" => "mysql2", - "database" => "my-app-db", - "username" => "pat", - "password" => "secret" - ) - - raise @error + def test_raises_error + assert_raises(Mysql2::Error) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration end - - ActiveRecord::Tasks::DatabaseTasks.create @configuration end - - def test_sends_output_to_stderr_when_other_errors - @error.stubs(:errno).returns(42) - - $stderr.expects(:puts).at_least_once.returns(nil) - - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end - - private - - def assert_permissions_granted_for(db_user) - db_name = @configuration["database"] - db_password = @configuration["password"] - @connection.expects(:execute).with("GRANT ALL PRIVILEGES ON `#{db_name}`.* TO '#{db_user}'@'localhost' IDENTIFIED BY '#{db_password}' WITH GRANT OPTION;") - end end class MySQLDBDropTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index e57ebf56c8..06a8693a7d 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -110,7 +110,7 @@ module ActiveRecord # FIXME: this needs to be refactored so specific database can add their own # ignored SQL, or better yet, use a different notification for the queries # 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] + 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] sqlite3_ignored = [/^\s*SELECT name\b.*\bFROM sqlite_master/im, /^\s*SELECT sql\b.*\bFROM sqlite_master/im] diff --git a/activerecord/test/cases/transaction_isolation_test.rb b/activerecord/test/cases/transaction_isolation_test.rb index b1ebccdcc3..eaafd13360 100644 --- a/activerecord/test/cases/transaction_isolation_test.rb +++ b/activerecord/test/cases/transaction_isolation_test.rb @@ -15,9 +15,7 @@ unless ActiveRecord::Base.connection.supports_transaction_isolation? end end end -end - -if ActiveRecord::Base.connection.supports_transaction_isolation? +else class TransactionIsolationTest < ActiveRecord::TestCase self.use_transactional_tests = false diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 7fd125ab74..5c8ae4d3cb 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -954,27 +954,25 @@ class TransactionsWithTransactionalFixturesTest < ActiveRecord::TestCase end end if Topic.connection.supports_savepoints? -if current_adapter?(:PostgreSQLAdapter) +if ActiveRecord::Base.connection.supports_transaction_isolation? class ConcurrentTransactionTest < TransactionTest # This will cause transactions to overlap and fail unless they are performed on # separate database connections. - unless in_memory_db? - def test_transaction_per_thread - threads = 3.times.map do - Thread.new do - Topic.transaction do - topic = Topic.find(1) - topic.approved = !topic.approved? - assert topic.save! - topic.approved = !topic.approved? - assert topic.save! - end - Topic.connection.close + def test_transaction_per_thread + threads = 3.times.map do + Thread.new do + Topic.transaction do + topic = Topic.find(1) + topic.approved = !topic.approved? + assert topic.save! + topic.approved = !topic.approved? + assert topic.save! end + Topic.connection.close end - - threads.each(&:join) end + + threads.each(&:join) end # Test for dirty reads among simultaneous transactions. diff --git a/activerecord/test/cases/unsafe_raw_sql_test.rb b/activerecord/test/cases/unsafe_raw_sql_test.rb new file mode 100644 index 0000000000..72d4997d0b --- /dev/null +++ b/activerecord/test/cases/unsafe_raw_sql_test.rb @@ -0,0 +1,299 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" +require "models/comment" + +class UnsafeRawSqlTest < ActiveRecord::TestCase + fixtures :posts, :comments + + test "order: allows string column name" do + ids_expected = Post.order(Arel.sql("title")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("title").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("title").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows symbol column name" do + ids_expected = Post.order(Arel.sql("title")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(:title).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:title).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows downcase symbol direction" do + ids_expected = Post.order(Arel.sql("title") => Arel.sql("asc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(title: :asc).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: :asc).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows upcase symbol direction" do + ids_expected = Post.order(Arel.sql("title") => Arel.sql("ASC")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(title: :ASC).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: :ASC).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows string direction" do + ids_expected = Post.order(Arel.sql("title") => Arel.sql("asc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(title: "asc").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: "asc").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows multiple columns" do + ids_expected = Post.order(Arel.sql("author_id"), Arel.sql("title")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(:author_id, :title).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:author_id, :title).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows mixed" do + ids_expected = Post.order(Arel.sql("author_id"), Arel.sql("title") => Arel.sql("asc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(:author_id, title: :asc).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:author_id, title: :asc).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows table and column name" do + ids_expected = Post.order(Arel.sql("title")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("posts.title").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("posts.title").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows column name and direction in string" do + ids_expected = Post.order(Arel.sql("title desc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("title desc").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("title desc").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows table name, column name and direction in string" do + ids_expected = Post.order(Arel.sql("title desc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("posts.title desc").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("posts.title desc").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: disallows invalid column name" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.order("len(title) asc").pluck(:id) + end + end + end + + test "order: disallows invalid direction" do + with_unsafe_raw_sql_disabled do + assert_raises(ArgumentError) do + Post.order(title: :foo).pluck(:id) + end + end + end + + test "order: disallows invalid column with direction" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.order("len(title)" => :asc).pluck(:id) + end + end + end + + test "order: always allows Arel" do + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(Arel.sql("length(title)")).pluck(:title) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(Arel.sql("length(title)")).pluck(:title) } + + assert_equal ids_depr, ids_disabled + end + + test "order: allows Arel.sql with binds" do + ids_expected = Post.order(Arel.sql("REPLACE(title, 'misc', 'zzzz'), id")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order([Arel.sql("REPLACE(title, ?, ?), id"), "misc", "zzzz"]).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order([Arel.sql("REPLACE(title, ?, ?), id"), "misc", "zzzz"]).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: disallows invalid bind statement" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.order(["REPLACE(title, ?, ?), id", "misc", "zzzz"]).pluck(:id) + end + end + end + + test "order: disallows invalid Array arguments" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.order(["author_id", "length(title)"]).pluck(:id) + end + end + end + + test "order: allows valid Array arguments" do + ids_expected = Post.order(Arel.sql("author_id, length(title)")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(["author_id", Arel.sql("length(title)")]).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(["author_id", Arel.sql("length(title)")]).pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: logs deprecation warning for unrecognized column" do + with_unsafe_raw_sql_deprecated do + assert_deprecated(/Dangerous query method/) do + Post.order("length(title)") + end + end + end + + test "pluck: allows string column name" do + titles_expected = Post.pluck(Arel.sql("title")) + + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck("title") } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("title") } + + assert_equal titles_expected, titles_depr + assert_equal titles_expected, titles_disabled + end + + test "pluck: allows symbol column name" do + titles_expected = Post.pluck(Arel.sql("title")) + + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck(:title) } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:title) } + + assert_equal titles_expected, titles_depr + assert_equal titles_expected, titles_disabled + end + + test "pluck: allows multiple column names" do + values_expected = Post.pluck(Arel.sql("title"), Arel.sql("id")) + + values_depr = with_unsafe_raw_sql_deprecated { Post.pluck(:title, :id) } + values_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:title, :id) } + + assert_equal values_expected, values_depr + assert_equal values_expected, values_disabled + end + + test "pluck: allows column names with includes" do + values_expected = Post.includes(:comments).pluck(Arel.sql("title"), Arel.sql("id")) + + values_depr = with_unsafe_raw_sql_deprecated { Post.includes(:comments).pluck(:title, :id) } + values_disabled = with_unsafe_raw_sql_disabled { Post.includes(:comments).pluck(:title, :id) } + + assert_equal values_expected, values_depr + assert_equal values_expected, values_disabled + end + + test "pluck: allows auto-generated attributes" do + values_expected = Post.pluck(Arel.sql("tags_count")) + + values_depr = with_unsafe_raw_sql_deprecated { Post.pluck(:tags_count) } + values_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:tags_count) } + + assert_equal values_expected, values_depr + assert_equal values_expected, values_disabled + end + + test "pluck: allows table and column names" do + titles_expected = Post.pluck(Arel.sql("title")) + + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck("posts.title") } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("posts.title") } + + assert_equal titles_expected, titles_depr + assert_equal titles_expected, titles_disabled + end + + test "pluck: disallows invalid column name" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.pluck("length(title)") + end + end + end + + test "pluck: disallows invalid column name amongst valid names" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.pluck(:title, "length(title)") + end + end + end + + test "pluck: disallows invalid column names with includes" do + with_unsafe_raw_sql_disabled do + assert_raises(ActiveRecord::UnknownAttributeReference) do + Post.includes(:comments).pluck(:title, "length(title)") + end + end + end + + test "pluck: always allows Arel" do + values_depr = with_unsafe_raw_sql_deprecated { Post.includes(:comments).pluck(:title, Arel.sql("length(title)")) } + values_disabled = with_unsafe_raw_sql_disabled { Post.includes(:comments).pluck(:title, Arel.sql("length(title)")) } + + assert_equal values_depr, values_disabled + end + + test "pluck: logs deprecation warning" do + with_unsafe_raw_sql_deprecated do + assert_deprecated(/Dangerous query method/) do + Post.includes(:comments).pluck(:title, "length(title)") + end + end + end + + def with_unsafe_raw_sql_disabled(&blk) + with_config(:disabled, &blk) + end + + def with_unsafe_raw_sql_deprecated(&blk) + with_config(:deprecated, &blk) + end + + def with_config(new_value, &blk) + old_value = ActiveRecord::Base.allow_unsafe_raw_sql + ActiveRecord::Base.allow_unsafe_raw_sql = new_value + blk.call + ensure + ActiveRecord::Base.allow_unsafe_raw_sql = old_value + end +end diff --git a/activerecord/test/fixtures/other_posts.yml b/activerecord/test/fixtures/other_posts.yml index 39ff763547..3e11a33802 100644 --- a/activerecord/test/fixtures/other_posts.yml +++ b/activerecord/test/fixtures/other_posts.yml @@ -5,3 +5,4 @@ second_welcome: author_id: 1 title: Welcome to the another weblog body: It's really nice today + comments_count: 1 diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index 86d46f753a..8d7e1e0ae7 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -28,6 +28,7 @@ sti_comments: author_id: 1 title: sti comments body: hello + comments_count: 5 type: Post sti_post_and_comments: @@ -35,6 +36,7 @@ sti_post_and_comments: author_id: 1 title: sti me body: hello + comments_count: 2 type: StiPost sti_habtm: @@ -50,6 +52,8 @@ eager_other: title: eager loading with OR'd conditions body: hello type: Post + comments_count: 1 + tags_count: 3 misc_by_bob: id: 8 @@ -57,6 +61,7 @@ misc_by_bob: title: misc post by bob body: hello type: Post + tags_count: 1 misc_by_mary: id: 9 @@ -64,6 +69,7 @@ misc_by_mary: title: misc post by mary body: hello type: Post + tags_count: 1 other_by_bob: id: 10 @@ -71,6 +77,7 @@ other_by_bob: title: other post by bob body: hello type: Post + tags_count: 1 other_by_mary: id: 11 @@ -78,3 +85,4 @@ other_by_mary: title: other post by mary body: hello type: Post + tags_count: 1 diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 740aa593ac..5ab433f2d9 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -60,6 +60,10 @@ end class SpecialComment < Comment default_scope { where(deleted_at: nil) } + + def self.what_are_you + "a special comment..." + end end class SubSpecialComment < SpecialComment diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 7f064bf3dd..780a2c17f5 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -319,5 +319,9 @@ class FakeKlass def arel_attribute(name, table) table[name] end + + def enforce_raw_sql_whitelist(*args) + # noop + end end end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 8f872c38ba..a4505a4892 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -191,6 +191,7 @@ ActiveRecord::Schema.define do t.string :resource_id t.string :resource_type t.integer :developer_id + t.datetime :updated_at t.datetime :deleted_at t.integer :comments end |