diff options
Diffstat (limited to 'activerecord')
23 files changed, 406 insertions, 199 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 89a12d4223..ceef240257 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,86 @@ +* Fix conflicts `counter_cache` with `touch: true` by optimistic locking. + + ``` + # create_table :posts do |t| + # t.integer :comments_count, default: 0 + # t.integer :lock_version + # t.timestamps + # end + class Post < ApplicationRecord + end + + # create_table :comments do |t| + # t.belongs_to :post + # end + class Comment < ApplicationRecord + belongs_to :post, touch: true, counter_cache: true + end + ``` + + Before: + ``` + post = Post.create! + # => begin transaction + INSERT INTO "posts" ("created_at", "updated_at", "lock_version") + VALUES ("2017-12-11 21:27:11.387397", "2017-12-11 21:27:11.387397", 0) + commit transaction + + comment = Comment.create!(post: post) + # => begin transaction + INSERT INTO "comments" ("post_id") VALUES (1) + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) + 1, + "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "posts"."id" = 1 + + UPDATE "posts" SET "updated_at" = '2017-12-11 21:27:11.398330', + "lock_version" = 1 WHERE "posts"."id" = 1 AND "posts"."lock_version" = 0 + rollback transaction + # => ActiveRecord::StaleObjectError: Attempted to touch a stale object: Post. + + Comment.take.destroy! + # => begin transaction + DELETE FROM "comments" WHERE "comments"."id" = 1 + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) - 1, + "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "posts"."id" = 1 + + UPDATE "posts" SET "updated_at" = '2017-12-11 21:42:47.785901', + "lock_version" = 1 WHERE "posts"."id" = 1 AND "posts"."lock_version" = 0 + rollback transaction + # => ActiveRecord::StaleObjectError: Attempted to touch a stale object: Post. + ``` + + After: + ``` + post = Post.create! + # => begin transaction + INSERT INTO "posts" ("created_at", "updated_at", "lock_version") + VALUES ("2017-12-11 21:27:11.387397", "2017-12-11 21:27:11.387397", 0) + commit transaction + + comment = Comment.create!(post: post) + # => begin transaction + INSERT INTO "comments" ("post_id") VALUES (1) + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) + 1, + "lock_version" = COALESCE("lock_version", 0) + 1, + "updated_at" = '2017-12-11 21:37:09.802642' WHERE "posts"."id" = 1 + commit transaction + + comment.destroy! + # => begin transaction + DELETE FROM "comments" WHERE "comments"."id" = 1 + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) - 1, + "lock_version" = COALESCE("lock_version", 0) + 1, + "updated_at" = '2017-12-11 21:39:02.685520' WHERE "posts"."id" = 1 + commit transaction + ``` + + Fixes #31199. + + *bogdanvlviv* + * Add support for PostgreSQL operator classes to `add_index`. Example: diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 57c82bf469..591c451da5 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -68,7 +68,7 @@ end (Dir["test/cases/**/*_test.rb"].reject { |x| x.include?("/adapters/") } + Dir["test/cases/adapters/#{adapter_short}/**/*_test.rb"]).all? do |file| - sh(Gem.ruby, "-w" , "-Itest", file) + sh(Gem.ruby, "-w", "-Itest", file) end || raise("Failures") end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index ba54cd8f49..68c608df13 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -49,9 +49,9 @@ module ActiveRecord def update_counters(by) if require_counter_update? && foreign_key_present? if target && !stale_target? - target.increment!(reflection.counter_cache_column, by) + target.increment!(reflection.counter_cache_column, by, touch: reflection.options[:touch]) else - klass.update_counters(target_id, reflection.counter_cache_column => by) + klass.update_counters(target_id, reflection.counter_cache_column => by, touch: reflection.options[:touch]) end end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 9904ee4bed..c161454c1a 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -114,9 +114,13 @@ module ActiveRecord::Associations::Builder # :nodoc: BelongsTo.touch_record(record, record.send(changes_method), foreign_key, n, touch, belongs_to_touch_method) }} - model.after_save callback.(:saved_changes), if: :saved_changes? - model.after_touch callback.(:changes_to_save) - model.after_destroy callback.(:changes_to_save) + unless reflection.counter_cache_column + model.after_create callback.(:saved_changes), if: :saved_changes? + model.after_destroy callback.(:changes_to_save) + end + + model.after_update callback.(:saved_changes), if: :saved_changes? + model.after_touch callback.(:changes_to_save) end def self.add_default_callbacks(model, reflection) 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 479131caad..8569c76317 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -403,12 +403,13 @@ module ActiveRecord fk.constraint_name AS 'name', rc.update_rule AS 'on_update', rc.delete_rule AS 'on_delete' - FROM information_schema.key_column_usage fk - JOIN information_schema.referential_constraints rc + FROM information_schema.referential_constraints rc + JOIN information_schema.key_column_usage fk USING (constraint_schema, constraint_name) WHERE fk.referenced_column_name IS NOT NULL AND fk.table_schema = #{scope[:schema]} AND fk.table_name = #{scope[:name]} + AND rc.constraint_schema = #{scope[:schema]} AND rc.table_name = #{scope[:name]} SQL diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 0715c11473..f3fe610c09 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -47,14 +47,13 @@ module ActiveRecord # Determines if one of the attributes passed in is the inheritance column, # and if the inheritance column is attr accessible, it initializes an # instance of the given subclass instead of the base class. - def new(*args, &block) + def new(attributes = nil, &block) if abstract_class? || self == Base raise NotImplementedError, "#{self} is an abstract class and cannot be instantiated." end - attrs = args.first if has_attribute?(inheritance_column) - subclass = subclass_from_attributes(attrs) + subclass = subclass_from_attributes(attributes) if subclass.nil? && base_class == self subclass = subclass_from_attributes(column_defaults) @@ -62,7 +61,7 @@ module ActiveRecord end if subclass && subclass != self - subclass.new(*args, &block) + subclass.new(attributes, &block) else super end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 5c10d4ff24..f6648a4e3d 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1277,10 +1277,10 @@ module ActiveRecord end def validate(migrations) - name , = migrations.group_by(&:name).find { |_, v| v.length > 1 } + name, = migrations.group_by(&:name).find { |_, v| v.length > 1 } raise DuplicateMigrationNameError.new(name) if name - version , = migrations.group_by(&:version).find { |_, v| v.length > 1 } + version, = migrations.group_by(&:version).find { |_, v| v.length > 1 } raise DuplicateMigrationVersionError.new(version) if version end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 1941d3d5ea..773b0f6cde 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -87,19 +87,6 @@ module ActiveRecord # Sets the name of the internal metadata table. ## - # :singleton-method: protected_environments - # :call-seq: protected_environments - # - # The array of names of environments where destructive actions should be prohibited. By default, - # the value is <tt>["production"]</tt>. - - ## - # :singleton-method: protected_environments= - # :call-seq: protected_environments=(environments) - # - # Sets an array of names of environments where destructive actions should be prohibited. - - ## # :singleton-method: pluralize_table_names # :call-seq: pluralize_table_names # @@ -122,9 +109,9 @@ module ActiveRecord class_attribute :table_name_suffix, instance_writer: false, default: "" class_attribute :schema_migrations_table_name, instance_accessor: false, default: "schema_migrations" class_attribute :internal_metadata_table_name, instance_accessor: false, default: "ar_internal_metadata" - class_attribute :protected_environments, instance_accessor: false, default: [ "production" ] class_attribute :pluralize_table_names, instance_writer: false, default: true + self.protected_environments = ["production"] self.inheritance_column = "type" self.ignored_columns = [].freeze @@ -238,6 +225,21 @@ module ActiveRecord (parents.detect { |p| p.respond_to?(:table_name_suffix) } || self).table_name_suffix end + # The array of names of environments where destructive actions should be prohibited. By default, + # the value is <tt>["production"]</tt>. + def protected_environments + if defined?(@protected_environments) + @protected_environments + else + superclass.protected_environments + end + end + + # Sets an array of names of environments where destructive actions should be prohibited. + def protected_environments=(environments) + @protected_environments = environments.map(&:to_s) + end + # Defines the name of the table column which will store the class name on single-table # inheritance situations. # diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 081ef5771f..dd0d52ad1c 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -52,8 +52,8 @@ module ActiveRecord # # user = users.new { |user| user.name = 'Oscar' } # user.name # => Oscar - def new(*args, &block) - scoping { @klass.new(*args, &block) } + def new(attributes = nil, &block) + scoping { klass.new(scope_for_create(attributes), &block) } end alias build new @@ -77,8 +77,12 @@ module ActiveRecord # # users.create(name: nil) # validation on name # # => #<User id: nil, name: nil, ...> - def create(*args, &block) - scoping { @klass.create(*args, &block) } + def create(attributes = nil, &block) + if attributes.is_a?(Array) + attributes.collect { |attr| create(attr, &block) } + else + scoping { klass.create(scope_for_create(attributes), &block) } + end end # Similar to #create, but calls @@ -87,8 +91,12 @@ module ActiveRecord # # Expects arguments in the same format as # {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!]. - def create!(*args, &block) - scoping { @klass.create!(*args, &block) } + def create!(attributes = nil, &block) + if attributes.is_a?(Array) + attributes.collect { |attr| create!(attr, &block) } + else + scoping { klass.create!(scope_for_create(attributes), &block) } + end end def first_or_create(attributes = nil, &block) # :nodoc: @@ -306,7 +314,7 @@ module ActiveRecord stmt = Arel::UpdateManager.new - stmt.set Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)) + stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates)) stmt.table(table) if has_join_values? @@ -440,8 +448,10 @@ module ActiveRecord where_clause.to_h(relation_table_name) end - def scope_for_create - where_values_hash.merge!(create_with_value.stringify_keys) + def scope_for_create(attributes = nil) + scope = where_values_hash.merge!(create_with_value.stringify_keys) + scope.merge!(attributes) if attributes + scope end # Returns true if relation needs eager loading. diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 749223422f..6b3ff3d610 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1041,7 +1041,7 @@ module ActiveRecord if select_values.any? arel.project(*arel_columns(select_values.uniq)) elsif @klass.ignored_columns.any? - arel.project(*arel_columns(@klass.column_names)) + arel.project(*arel_columns(@klass.column_names.map(&:to_sym))) else arel.project(table[Arel.star]) end @@ -1121,7 +1121,7 @@ module ActiveRecord def preprocess_order_args(order_args) order_args.map! do |arg| - klass.send(:sanitize_sql_for_order, arg) + klass.sanitize_sql_for_order(arg) end order_args.flatten! diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb index 1374785354..4ae94f4bfe 100644 --- a/activerecord/lib/active_record/relation/where_clause_factory.rb +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -11,7 +11,7 @@ module ActiveRecord def build(opts, other) case opts when String, Array - parts = [klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] + parts = [klass.sanitize_sql(other.empty? ? opts : ([opts] + other))] when Hash attributes = predicate_builder.resolve_column_aliases(opts) attributes = klass.send(:expand_hash_conditions_for_aggregates, attributes) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 21f8bc7cb2..58da106092 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -5,80 +5,135 @@ module ActiveRecord extend ActiveSupport::Concern module ClassMethods - private - - # Accepts an array or string of SQL conditions and sanitizes - # them into a valid SQL fragment for a WHERE clause. - # - # sanitize_sql_for_conditions(["name=? and group_id=?", "foo'bar", 4]) - # # => "name='foo''bar' and group_id=4" - # - # sanitize_sql_for_conditions(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) - # # => "name='foo''bar' and group_id='4'" - # - # sanitize_sql_for_conditions(["name='%s' and group_id='%s'", "foo'bar", 4]) - # # => "name='foo''bar' and group_id='4'" - # - # sanitize_sql_for_conditions("name='foo''bar' and group_id='4'") - # # => "name='foo''bar' and group_id='4'" - def sanitize_sql_for_conditions(condition) # :doc: - return nil if condition.blank? - - case condition - when Array; sanitize_sql_array(condition) - else condition - end + # Accepts an array or string of SQL conditions and sanitizes + # them into a valid SQL fragment for a WHERE clause. + # + # sanitize_sql_for_conditions(["name=? and group_id=?", "foo'bar", 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_for_conditions(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) + # # => "name='foo''bar' and group_id='4'" + # + # sanitize_sql_for_conditions(["name='%s' and group_id='%s'", "foo'bar", 4]) + # # => "name='foo''bar' and group_id='4'" + # + # sanitize_sql_for_conditions("name='foo''bar' and group_id='4'") + # # => "name='foo''bar' and group_id='4'" + def sanitize_sql_for_conditions(condition) + return nil if condition.blank? + + case condition + when Array; sanitize_sql_array(condition) + else condition end - alias :sanitize_sql :sanitize_sql_for_conditions - - # Accepts an array, hash, or string of SQL conditions and sanitizes - # them into a valid SQL fragment for a SET clause. - # - # sanitize_sql_for_assignment(["name=? and group_id=?", nil, 4]) - # # => "name=NULL and group_id=4" - # - # sanitize_sql_for_assignment(["name=:name and group_id=:group_id", name: nil, group_id: 4]) - # # => "name=NULL and group_id=4" - # - # Post.send(:sanitize_sql_for_assignment, { name: nil, group_id: 4 }) - # # => "`posts`.`name` = NULL, `posts`.`group_id` = 4" - # - # sanitize_sql_for_assignment("name=NULL and group_id='4'") - # # => "name=NULL and group_id='4'" - def sanitize_sql_for_assignment(assignments, default_table_name = table_name) # :doc: - case assignments - when Array; sanitize_sql_array(assignments) - when Hash; sanitize_sql_hash_for_assignment(assignments, default_table_name) - else assignments - end + end + alias :sanitize_sql :sanitize_sql_for_conditions + + # Accepts an array, hash, or string of SQL conditions and sanitizes + # them into a valid SQL fragment for a SET clause. + # + # sanitize_sql_for_assignment(["name=? and group_id=?", nil, 4]) + # # => "name=NULL and group_id=4" + # + # sanitize_sql_for_assignment(["name=:name and group_id=:group_id", name: nil, group_id: 4]) + # # => "name=NULL and group_id=4" + # + # Post.sanitize_sql_for_assignment({ name: nil, group_id: 4 }) + # # => "`posts`.`name` = NULL, `posts`.`group_id` = 4" + # + # sanitize_sql_for_assignment("name=NULL and group_id='4'") + # # => "name=NULL and group_id='4'" + def sanitize_sql_for_assignment(assignments, default_table_name = table_name) + case assignments + when Array; sanitize_sql_array(assignments) + when Hash; sanitize_sql_hash_for_assignment(assignments, default_table_name) + else assignments end - - # Accepts an array, or string of SQL conditions and sanitizes - # them into a valid SQL fragment for an ORDER clause. - # - # sanitize_sql_for_order(["field(id, ?)", [1,3,2]]) - # # => "field(id, 1,3,2)" - # - # sanitize_sql_for_order("id ASC") - # # => "id ASC" - def sanitize_sql_for_order(condition) # :doc: - if condition.is_a?(Array) && condition.first.to_s.include?("?") - 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 + + # Accepts an array, or string of SQL conditions and sanitizes + # them into a valid SQL fragment for an ORDER clause. + # + # sanitize_sql_for_order(["field(id, ?)", [1,3,2]]) + # # => "field(id, 1,3,2)" + # + # sanitize_sql_for_order("id ASC") + # # => "id ASC" + def sanitize_sql_for_order(condition) + if condition.is_a?(Array) && condition.first.to_s.include?("?") + 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 + end + + # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause. + # + # sanitize_sql_hash_for_assignment({ status: nil, group_id: 1 }, "posts") + # # => "`posts`.`status` = NULL, `posts`.`group_id` = 1" + def sanitize_sql_hash_for_assignment(attrs, table) + c = connection + attrs.map do |attr, 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 + + # Sanitizes a +string+ so that it is safe to use within an SQL + # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%". + # + # sanitize_sql_like("100%") + # # => "100\\%" + # + # sanitize_sql_like("snake_cased_string") + # # => "snake\\_cased\\_string" + # + # sanitize_sql_like("100%", "!") + # # => "100!%" + # + # sanitize_sql_like("snake_cased_string", "!") + # # => "snake!_cased!_string" + def sanitize_sql_like(string, escape_character = "\\") + pattern = Regexp.union(escape_character, "%", "_") + string.gsub(pattern) { |x| [escape_character, x].join } + end + + # Accepts an array of conditions. The array has each value + # sanitized and interpolated into the SQL statement. + # + # sanitize_sql_array(["name=? and group_id=?", "foo'bar", 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_array(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_array(["name='%s' and group_id='%s'", "foo'bar", 4]) + # # => "name='foo''bar' and group_id='4'" + def sanitize_sql_array(ary) + statement, *values = ary + if values.first.is_a?(Hash) && /:\w+/.match?(statement) + replace_named_bind_variables(statement, values.first) + elsif statement.include?("?") + replace_bind_variables(statement, values) + elsif statement.blank? + statement + else + statement % values.collect { |value| connection.quote_string(value.to_s) } + end + end + private # Accepts a hash of SQL conditions and replaces those attributes # that correspond to a {#composed_of}[rdoc-ref:Aggregations::ClassMethods#composed_of] # relationship with their expanded aggregate attribute values. @@ -113,62 +168,6 @@ module ActiveRecord expanded_attrs end - # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause. - # - # sanitize_sql_hash_for_assignment({ status: nil, group_id: 1 }, "posts") - # # => "`posts`.`status` = NULL, `posts`.`group_id` = 1" - def sanitize_sql_hash_for_assignment(attrs, table) # :doc: - c = connection - attrs.map do |attr, 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 - - # Sanitizes a +string+ so that it is safe to use within an SQL - # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%". - # - # sanitize_sql_like("100%") - # # => "100\\%" - # - # sanitize_sql_like("snake_cased_string") - # # => "snake\\_cased\\_string" - # - # sanitize_sql_like("100%", "!") - # # => "100!%" - # - # sanitize_sql_like("snake_cased_string", "!") - # # => "snake!_cased!_string" - def sanitize_sql_like(string, escape_character = "\\") # :doc: - pattern = Regexp.union(escape_character, "%", "_") - string.gsub(pattern) { |x| [escape_character, x].join } - end - - # Accepts an array of conditions. The array has each value - # sanitized and interpolated into the SQL statement. - # - # sanitize_sql_array(["name=? and group_id=?", "foo'bar", 4]) - # # => "name='foo''bar' and group_id=4" - # - # sanitize_sql_array(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) - # # => "name='foo''bar' and group_id=4" - # - # sanitize_sql_array(["name='%s' and group_id='%s'", "foo'bar", 4]) - # # => "name='foo''bar' and group_id='4'" - def sanitize_sql_array(ary) # :doc: - statement, *values = ary - if values.first.is_a?(Hash) && /:\w+/.match?(statement) - replace_named_bind_variables(statement, values.first) - elsif statement.include?("?") - replace_bind_variables(statement, values) - elsif statement.blank? - statement - else - statement % values.collect { |value| connection.quote_string(value.to_s) } - end - end - def replace_bind_variables(statement, values) raise_if_bind_arity_mismatch(statement, statement.count("?"), values.size) bound = values.dup diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 829e12fbc8..e717621928 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -136,7 +136,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_multiple_stis_and_order - author = Author.all.merge!(includes: { posts: [ :special_comments , :very_special_comment ] }, order: ["authors.name", "comments.body", "very_special_comments_posts.body"], where: "posts.id = 4").first + author = Author.all.merge!(includes: { posts: [ :special_comments, :very_special_comment ] }, order: ["authors.name", "comments.body", "very_special_comments_posts.body"], where: "posts.id = 4").first assert_equal authors(:david), author assert_no_queries do author.posts.first.special_comments diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 4ca11af791..5ed8d0ee81 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2350,7 +2350,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase test "does not duplicate associations when used with natural primary keys" do speedometer = Speedometer.create!(id: "4") - speedometer.minivans.create!(minivan_id: "a-van-red" , name: "a van", color: "red") + speedometer.minivans.create!(minivan_id: "a-van-red", name: "a van", color: "red") assert_equal 1, speedometer.minivans.to_a.size, "Only one association should be present:\n#{speedometer.minivans.to_a}" assert_equal 1, speedometer.reload.minivans.to_a.size diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 3497f6aae4..7fc4a396e4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -629,7 +629,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_readonly_attributes - assert_equal Set.new([ "title" , "comments_count" ]), ReadonlyTitlePost.readonly_attributes + assert_equal Set.new([ "title", "comments_count" ]), ReadonlyTitlePost.readonly_attributes post = ReadonlyTitlePost.create(title: "cannot change this", body: "changeable") post.reload @@ -1495,4 +1495,24 @@ class BasicsTest < ActiveRecord::TestCase # regular column assert query.include?("name") end + + test "column names are quoted when using #from clause and model has ignored columns" do + refute_empty Developer.ignored_columns + query = Developer.from("`developers`").to_sql + quoted_id = Developer.connection.quote_table_name("id") + + assert_match(/SELECT #{quoted_id}.* FROM `developers`/, query) + end + + test "protected environments by default is an array with production" do + assert_equal ["production"], ActiveRecord::Base.protected_environments + end + + def test_protected_environments_are_stored_as_an_array_of_string + previous_protected_environments = ActiveRecord::Base.protected_environments + ActiveRecord::Base.protected_environments = [:staging, "production"] + assert_equal ["staging", "production"], ActiveRecord::Base.protected_environments + ensure + ActiveRecord::Base.protected_environments = previous_protected_environments + end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index e936c56ab8..2076b57aa8 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1238,7 +1238,7 @@ class FinderTest < ActiveRecord::TestCase test "find_by with associations" do assert_equal authors(:david), Post.find_by(author: authors(:david)).author - assert_equal authors(:mary) , Post.find_by(author: authors(:mary)).author + assert_equal authors(:mary), Post.find_by(author: authors(:mary)).author end test "find_by doesn't have implicit ordering" do diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index c931f7d21c..ff4385c8b4 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -280,6 +280,21 @@ class InheritanceTest < ActiveRecord::TestCase assert_equal Firm, firm.class end + def test_where_new_with_subclass + firm = Company.where(type: "Firm").new + assert_equal Firm, firm.class + end + + def test_where_create_with_subclass + firm = Company.where(type: "Firm").create(name: "Basecamp") + assert_equal Firm, firm.class + end + + def test_where_create_bang_with_subclass + firm = Company.where(type: "Firm").create!(name: "Basecamp") + assert_equal Firm, firm.class + end + def test_new_with_abstract_class e = assert_raises(NotImplementedError) do AbstractCompany.new @@ -302,6 +317,30 @@ class InheritanceTest < ActiveRecord::TestCase assert_raise(ActiveRecord::SubclassNotFound) { Company.new(type: "Account") } end + def test_where_new_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "InvalidType").new } + end + + def test_where_new_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "Account").new } + end + + def test_where_create_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "InvalidType").create } + end + + def test_where_create_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "Account").create } + end + + def test_where_create_bang_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "InvalidType").create! } + end + + def test_where_create_bang_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "Account").create! } + end + def test_new_with_unrelated_namespaced_type without_store_full_sti_class do e = assert_raises ActiveRecord::SubclassNotFound do diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index e857180bd1..437a5a38a3 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -399,11 +399,43 @@ class OptimisticLockingTest < ActiveRecord::TestCase end end + def test_counter_cache_with_touch_and_lock_version + car = Car.create! + + assert_equal 0, car.wheels_count + assert_equal 0, car.lock_version + + previously_car_updated_at = car.updated_at + travel(1.second) do + Wheel.create!(wheelable: car) + end + + assert_equal 1, car.reload.wheels_count + assert_not_equal previously_car_updated_at, car.updated_at + assert_equal 1, car.lock_version + + previously_car_updated_at = car.updated_at + car.wheels.first.update(size: 42) + + assert_equal 1, car.reload.wheels_count + assert_not_equal previously_car_updated_at, car.updated_at + assert_equal 2, car.lock_version + + previously_car_updated_at = car.updated_at + travel(1.second) do + car.wheels.first.destroy! + end + + assert_equal 0, car.reload.wheels_count + assert_not_equal previously_car_updated_at, car.updated_at + assert_equal 3, car.lock_version + end + def test_polymorphic_destroy_with_dependencies_and_lock_version car = Car.create! assert_difference "car.wheels.count" do - car.wheels << Wheel.create! + car.wheels.create end assert_difference "car.wheels.count", -1 do car.reload.destroy diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 85a555fe35..86e45b3cbd 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -11,30 +11,30 @@ class SanitizeTest < ActiveRecord::TestCase def test_sanitize_sql_array_handles_string_interpolation quoted_bambi = ActiveRecord::Base.connection.quote_string("Bambi") - assert_equal "name='#{quoted_bambi}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi"]) - assert_equal "name='#{quoted_bambi}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi".mb_chars]) + assert_equal "name='#{quoted_bambi}'", Binary.sanitize_sql_array(["name='%s'", "Bambi"]) + assert_equal "name='#{quoted_bambi}'", Binary.sanitize_sql_array(["name='%s'", "Bambi".mb_chars]) quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote_string("Bambi\nand\nThumper") - assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi\nand\nThumper"]) - assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi\nand\nThumper".mb_chars]) + assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.sanitize_sql_array(["name='%s'", "Bambi\nand\nThumper"]) + assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.sanitize_sql_array(["name='%s'", "Bambi\nand\nThumper".mb_chars]) end def test_sanitize_sql_array_handles_bind_variables quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") - assert_equal "name=#{quoted_bambi}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi"]) - assert_equal "name=#{quoted_bambi}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi".mb_chars]) + assert_equal "name=#{quoted_bambi}", Binary.sanitize_sql_array(["name=?", "Bambi"]) + assert_equal "name=#{quoted_bambi}", Binary.sanitize_sql_array(["name=?", "Bambi".mb_chars]) quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote("Bambi\nand\nThumper") - assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper"]) - assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper".mb_chars]) + assert_equal "name=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=?", "Bambi\nand\nThumper"]) + assert_equal "name=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=?", "Bambi\nand\nThumper".mb_chars]) end def test_sanitize_sql_array_handles_named_bind_variables quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") - assert_equal "name=#{quoted_bambi}", Binary.send(:sanitize_sql_array, ["name=:name", name: "Bambi"]) - assert_equal "name=#{quoted_bambi} AND id=1", Binary.send(:sanitize_sql_array, ["name=:name AND id=:id", name: "Bambi", id: 1]) + assert_equal "name=#{quoted_bambi}", Binary.sanitize_sql_array(["name=:name", name: "Bambi"]) + assert_equal "name=#{quoted_bambi} AND id=1", Binary.sanitize_sql_array(["name=:name AND id=:id", name: "Bambi", id: 1]) quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote("Bambi\nand\nThumper") - assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=:name", name: "Bambi\nand\nThumper"]) - assert_equal "name=#{quoted_bambi_and_thumper} AND name2=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=:name AND name2=:name", name: "Bambi\nand\nThumper"]) + assert_equal "name=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=:name", name: "Bambi\nand\nThumper"]) + assert_equal "name=#{quoted_bambi_and_thumper} AND name2=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=:name AND name2=:name", name: "Bambi\nand\nThumper"]) end def test_sanitize_sql_array_handles_relations @@ -43,31 +43,31 @@ class SanitizeTest < ActiveRecord::TestCase sub_query_pattern = /\(\bselect\b.*?\bwhere\b.*?\)/i - select_author_sql = Post.send(:sanitize_sql_array, ["id in (?)", david_posts]) + select_author_sql = Post.sanitize_sql_array(["id in (?)", david_posts]) assert_match(sub_query_pattern, select_author_sql, "should sanitize `Relation` as subquery for bind variables") - select_author_sql = Post.send(:sanitize_sql_array, ["id in (:post_ids)", post_ids: david_posts]) + select_author_sql = Post.sanitize_sql_array(["id in (:post_ids)", post_ids: david_posts]) assert_match(sub_query_pattern, select_author_sql, "should sanitize `Relation` as subquery for named bind variables") end def test_sanitize_sql_array_handles_empty_statement - select_author_sql = Post.send(:sanitize_sql_array, [""]) + select_author_sql = Post.sanitize_sql_array([""]) assert_equal("", select_author_sql) end def test_sanitize_sql_like - assert_equal '100\%', Binary.send(:sanitize_sql_like, "100%") - assert_equal 'snake\_cased\_string', Binary.send(:sanitize_sql_like, "snake_cased_string") - assert_equal 'C:\\\\Programs\\\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint') - assert_equal "normal string 42", Binary.send(:sanitize_sql_like, "normal string 42") + assert_equal '100\%', Binary.sanitize_sql_like("100%") + assert_equal 'snake\_cased\_string', Binary.sanitize_sql_like("snake_cased_string") + assert_equal 'C:\\\\Programs\\\\MsPaint', Binary.sanitize_sql_like('C:\\Programs\\MsPaint') + assert_equal "normal string 42", Binary.sanitize_sql_like("normal string 42") end def test_sanitize_sql_like_with_custom_escape_character - assert_equal "100!%", Binary.send(:sanitize_sql_like, "100%", "!") - assert_equal "snake!_cased!_string", Binary.send(:sanitize_sql_like, "snake_cased_string", "!") - assert_equal "great!!", Binary.send(:sanitize_sql_like, "great!", "!") - assert_equal 'C:\\Programs\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint', "!") - assert_equal "normal string 42", Binary.send(:sanitize_sql_like, "normal string 42", "!") + assert_equal "100!%", Binary.sanitize_sql_like("100%", "!") + assert_equal "snake!_cased!_string", Binary.sanitize_sql_like("snake_cased_string", "!") + assert_equal "great!!", Binary.sanitize_sql_like("great!", "!") + assert_equal 'C:\\Programs\\MsPaint', Binary.sanitize_sql_like('C:\\Programs\\MsPaint', "!") + assert_equal "normal string 42", Binary.sanitize_sql_like("normal string 42", "!") end def test_sanitize_sql_like_example_use_case diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index 5a094ead42..fd381f229f 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -30,13 +30,30 @@ module ActiveRecord def test_raises_an_error_when_called_with_protected_environment ActiveRecord::Migrator.stubs(:current_version).returns(1) - protected_environments = ActiveRecord::Base.protected_environments.dup + protected_environments = ActiveRecord::Base.protected_environments current_env = ActiveRecord::Migrator.current_environment assert_not_includes protected_environments, current_env # Assert no error ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! - ActiveRecord::Base.protected_environments << current_env + ActiveRecord::Base.protected_environments = [current_env] + assert_raise(ActiveRecord::ProtectedEnvironmentError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end + ensure + ActiveRecord::Base.protected_environments = protected_environments + end + + def test_raises_an_error_when_called_with_protected_environment_which_name_is_a_symbol + ActiveRecord::Migrator.stubs(:current_version).returns(1) + + protected_environments = ActiveRecord::Base.protected_environments + current_env = ActiveRecord::Migrator.current_environment + assert_not_includes protected_environments, current_env + # Assert no error + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + + ActiveRecord::Base.protected_environments = [current_env.to_sym] assert_raise(ActiveRecord::ProtectedEnvironmentError) do ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! end diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 5d3e2a175c..54e3f47e16 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -90,7 +90,7 @@ class TimestampTest < ActiveRecord::TestCase @developer.touch(:created_at) end - assert !@developer.created_at_changed? , "created_at should not be changed" + assert !@developer.created_at_changed?, "created_at should not be changed" assert !@developer.changed?, "record should not be changed" assert_not_equal previously_created_at, @developer.created_at assert_not_equal @previously_updated_at, @developer.updated_at diff --git a/activerecord/test/models/wheel.rb b/activerecord/test/models/wheel.rb index e05fb64477..8db57d181e 100644 --- a/activerecord/test/models/wheel.rb +++ b/activerecord/test/models/wheel.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true class Wheel < ActiveRecord::Base - belongs_to :wheelable, polymorphic: true, counter_cache: true + belongs_to :wheelable, polymorphic: true, counter_cache: true, touch: true end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index bf66846840..3205c4c20a 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -123,7 +123,7 @@ ActiveRecord::Schema.define do create_table :cars, force: true do |t| t.string :name t.integer :engines_count - t.integer :wheels_count + t.integer :wheels_count, default: 0 t.column :lock_version, :integer, null: false, default: 0 t.timestamps null: false end @@ -964,6 +964,7 @@ ActiveRecord::Schema.define do end create_table :wheels, force: true do |t| + t.integer :size t.references :wheelable, polymorphic: true end |