From cf0fcbe7f6fd52b3455af99b61ef2c40e6a0062e Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sun, 10 Dec 2017 11:58:41 +0900 Subject: Reset schema cache after test Currently, `test_copy_table_with_composite_primary_keys` test fails depending on execution order. The reproduction step is as follows. ``` $ ARCONN=sqlite3_mem bin/test -w -n "/^(?:CalculationsTest#(?:test_#skip_query_cache\\!_for_a_simple_calculation)|PrimaryKeyAnyTypeTest#(?:test_any_type_primary_key)|ActiveRecord::ConnectionAdapters::SQLite3AdapterTest#(?:test_copy_table_with_composite_primary_keys))$/" --seed 41545 ``` The column info is cached by `PrimaryKeyAnyTypeTest#test_any_type_primary_key`, and the test seems to have failed due to the influence. So clear cache after testing so as not to affect other tests. Related: https://travis-ci.org/rails/rails/jobs/313730163#L1788 --- activerecord/test/cases/primary_keys_test.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 7f6c2382ca..80016fc19d 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -298,6 +298,8 @@ class PrimaryKeyAnyTypeTest < ActiveRecord::TestCase assert_not column.null assert_equal :string, column.type assert_equal 42, column.limit + ensure + Barcode.reset_column_information end test "schema dump primary key includes type and options" do -- cgit v1.2.3 From 2d78c66cd918b2cfde0308c0e4327943c30894f8 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Mon, 11 Dec 2017 13:53:23 -0500 Subject: Quote colum_names when building select: - #30980 introcuded a change to not use `Arel.star` when model have ignored columns, a query used to look like `SELECT *. FROM developers` whereas now it would like `SELECT column1, column2 FROM developers` - If a column has the same name has a reserved database specific keyword (such as key, where ...) then the query would fail because the names aren't quoted - Quoting almost always happen unless we use a `from` clause in the query https://github.com/rails/rails/blob/9965b98dc0d58a86e10b4343bb6e15e01661a8c3/activerecord/lib/active_record/relation/query_methods.rb#L1052 - This PR cast all columns name to symbols in order for the quoting logic to be picked up https://github.com/rails/rails/blob/9965b98dc0d58a86e10b4343bb6e15e01661a8c3/activerecord/lib/active_record/relation/query_methods.rb#L1054-L1055 - A reproduction script can be found here https://gist.github.com/Edouard-chin/f56d464a0adcb76962afc1a9134a1536 --- activerecord/lib/active_record/relation/query_methods.rb | 2 +- activerecord/test/cases/base_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 749223422f..808f796abe 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 diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 3497f6aae4..e0c0bff86e 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1495,4 +1495,12 @@ 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 end -- cgit v1.2.3 From 6ddba9b46c1d307ea7e03d753d0988183b9cb80c Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Mon, 11 Dec 2017 23:56:07 +0200 Subject: 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. --- activerecord/CHANGELOG.md | 83 ++++++++++++++++++++++ .../associations/belongs_to_association.rb | 4 +- .../associations/builder/belongs_to.rb | 10 ++- activerecord/test/cases/locking_test.rb | 34 ++++++++- activerecord/test/models/wheel.rb | 2 +- activerecord/test/schema/schema.rb | 3 +- 6 files changed, 128 insertions(+), 8 deletions(-) (limited to 'activerecord') 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/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/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/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 -- cgit v1.2.3 From 2b35826389005bdb0af85a4ebb1f0ec213174d13 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 12 Dec 2017 20:00:50 +0900 Subject: Enable `Layout/SpaceBeforeComma` rubocop rule, and fixed more Follow up of #31390. --- activerecord/Rakefile | 2 +- activerecord/lib/active_record/migration.rb | 4 ++-- activerecord/test/cases/associations/cascaded_eager_loading_test.rb | 2 +- activerecord/test/cases/associations/has_many_associations_test.rb | 2 +- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/finder_test.rb | 2 +- activerecord/test/cases/timestamp_test.rb | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) (limited to 'activerecord') 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/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/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..9e1b858bda 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 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/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 -- cgit v1.2.3 From 82822a34217503336d51b7baab82cd18cf71e435 Mon Sep 17 00:00:00 2001 From: Dmitri Dolguikh Date: Wed, 29 Nov 2017 16:27:27 -0800 Subject: Introduced `ActiveSupport::Digest` that allows to specify hash function implementation and defaults to `Digest::MD5`. Replaced calls to `::Digest::MD5.hexdigest` with calls to `ActiveSupport::Digest.hexdigest`. --- activerecord/lib/active_record/collection_cache_key.rb | 2 +- activerecord/test/cases/collection_cache_key_test.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/collection_cache_key.rb b/activerecord/lib/active_record/collection_cache_key.rb index 88b398ad45..023d144693 100644 --- a/activerecord/lib/active_record/collection_cache_key.rb +++ b/activerecord/lib/active_record/collection_cache_key.rb @@ -3,7 +3,7 @@ module ActiveRecord module CollectionCacheKey def collection_cache_key(collection = all, timestamp_column = :updated_at) # :nodoc: - query_signature = Digest::MD5.hexdigest(collection.to_sql) + query_signature = ActiveSupport::Digest.hexdigest(collection.to_sql) key = "#{collection.model_name.cache_key}/query-#{query_signature}" if collection.loaded? diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index 19d6464a22..479c9e03a5 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -24,7 +24,7 @@ module ActiveRecord /\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/ =~ developers.cache_key - assert_equal Digest::MD5.hexdigest(developers.to_sql), $1 + assert_equal ActiveSupport::Digest.hexdigest(developers.to_sql), $1 assert_equal developers.count.to_s, $2 assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 end @@ -37,7 +37,7 @@ module ActiveRecord /\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/ =~ developers.cache_key - assert_equal Digest::MD5.hexdigest(developers.to_sql), $1 + assert_equal ActiveSupport::Digest.hexdigest(developers.to_sql), $1 assert_equal developers.count.to_s, $2 assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 end @@ -50,7 +50,7 @@ module ActiveRecord /\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/ =~ developers.cache_key - assert_equal Digest::MD5.hexdigest(developers.to_sql), $1 + assert_equal ActiveSupport::Digest.hexdigest(developers.to_sql), $1 assert_equal developers.count.to_s, $2 assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 end @@ -68,7 +68,7 @@ module ActiveRecord /\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/ =~ developers.cache_key - assert_equal Digest::MD5.hexdigest(developers.to_sql), $1 + assert_equal ActiveSupport::Digest.hexdigest(developers.to_sql), $1 assert_equal developers.count.to_s, $2 assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $3 end -- cgit v1.2.3 From 9d43a84f73c1b3853a91d052a462ee60eccaf957 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Mon, 11 Dec 2017 11:41:27 +0900 Subject: Make `sanitize_sql_` methods public Currently, sanitize methods are private. So need `send` to use from outside class. However, sometimes want to use sanitize methods from outside Class when want to generate SQL including multiple tables like search. In order to avoid using `send` in such a case, changed methods to public. --- activerecord/lib/active_record/relation.rb | 2 +- .../lib/active_record/relation/query_methods.rb | 2 +- .../active_record/relation/where_clause_factory.rb | 2 +- activerecord/lib/active_record/sanitization.rb | 249 ++++++++++----------- activerecord/test/cases/sanitize_test.rb | 48 ++-- 5 files changed, 151 insertions(+), 152 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 081ef5771f..9ae40e4f4d 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -306,7 +306,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? diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 808f796abe..6b3ff3d610 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -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/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 -- cgit v1.2.3 From 1a411d4b7f34df2471a93517ad15cc2682bbdbe2 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 12 Dec 2017 23:04:27 +0000 Subject: Convert protected_environments to an array of strings These changes prevent ignoring environments name of which is a `Symbol` ``` config.active_record.protected_environments = ['staging', :production] ``` --- activerecord/lib/active_record/model_schema.rb | 30 ++++++++++++---------- activerecord/test/cases/base_test.rb | 12 +++++++++ .../test/cases/tasks/database_tasks_test.rb | 21 +++++++++++++-- 3 files changed, 47 insertions(+), 16 deletions(-) (limited to 'activerecord') 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 @@ -86,19 +86,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 ["production"]. - - ## - # :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 ["production"]. + 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/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 875b98f0b8..7fc4a396e4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1503,4 +1503,16 @@ class BasicsTest < ActiveRecord::TestCase 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/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 -- cgit v1.2.3 From 3499d320d2e616a044b74db86fe4126de783174a Mon Sep 17 00:00:00 2001 From: Hiroyuki Morita Date: Wed, 13 Dec 2017 10:25:59 +0900 Subject: Optimizing information_schema query for `foreign_keys` Use CONSTRAINT_SCHEMA key for information_schema.referential_constraints. See https://dev.mysql.com/doc/refman/5.7/en/information-schema-optimization.html. ``` > EXPLAIN SELECT fk.referenced_table_name AS 'to_table', fk.referenced_column_name AS 'primary_key', fk.column_name AS 'column', fk.constraint_name AS 'name', rc.update_rule AS 'on_update', rc.delete_rule AS 'on_delete' 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 = 'activerecord_unittest' AND fk.table_name = 'fk_test_has_pk' AND rc.constraint_schema = 'activerecord_unittest' AND rc.table_name = 'fk_test_has_pk'\G *************************** 1. row *************************** id: 1 select_type: SIMPLE table: rc partitions: NULL type: ALL possible_keys: NULL key: CONSTRAINT_SCHEMA,TABLE_NAME key_len: NULL ref: NULL rows: NULL filtered: NULL Extra: Using where; Open_full_table; Scanned 0 databases *************************** 2. row *************************** id: 1 select_type: SIMPLE table: fk partitions: NULL type: ALL possible_keys: NULL key: TABLE_SCHEMA,TABLE_NAME key_len: NULL ref: NULL rows: NULL filtered: NULL Extra: Using where; Open_full_table; Scanned 0 databases; Using join buffer (Block Nested Loop) 2 rows in set, 1 warning (0.00 sec) ``` --- .../lib/active_record/connection_adapters/abstract_mysql_adapter.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From d4007d5a54b60bd6924eeffb52c126ed32e9f31f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 11 Dec 2017 14:53:35 +0900 Subject: Fix inheritance object creation from relation We need to pass scope attributes to `klass.new` to detect subclass. Otherwise `subclass_from_attributes` can't detect subclass which is had in scope attributes. Fixes #18062. Closes #18227. Closes #30720. --- activerecord/lib/active_record/inheritance.rb | 7 +++-- activerecord/lib/active_record/relation.rb | 26 ++++++++++++------ activerecord/test/cases/inheritance_test.rb | 39 +++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 12 deletions(-) (limited to 'activerecord') 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/relation.rb b/activerecord/lib/active_record/relation.rb index 9ae40e4f4d..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 # # => # - 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: @@ -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/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 -- cgit v1.2.3 From 1d3fe75649e9e5dd9efacb7a6a0d9e9d12b3df34 Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Sun, 10 Dec 2017 23:32:00 +0000 Subject: Suppress expected exceptions by `report_on_exception` = `false` in Ruby 2.5 * Ruby 2.4 introduces `report_on_exception` to control if it reports exceptions in thread, this default value has been `false` in Ruby 2.4. Refer https://www.ruby-lang.org/en/news/2016/11/09/ruby-2-4-0-preview3-released/ * Ruby 2.5 changes `report_on_exception` default value to `true` since this commit https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?revision=61183&view=revision This pull request suppresses expected exceptions by setting `report_on_exception` = `false` it also supports Ruby 2.3 which does not have`report_on_exception`. --- activerecord/test/cases/adapters/postgresql/transaction_test.rb | 2 ++ activerecord/test/cases/connection_pool_test.rb | 3 +++ 2 files changed, 5 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/postgresql/transaction_test.rb b/activerecord/test/cases/adapters/postgresql/transaction_test.rb index c24dfeb345..9821b103df 100644 --- a/activerecord/test/cases/adapters/postgresql/transaction_test.rb +++ b/activerecord/test/cases/adapters/postgresql/transaction_test.rb @@ -14,6 +14,7 @@ module ActiveRecord setup do @abort, Thread.abort_on_exception = Thread.abort_on_exception, false + Thread.report_on_exception, @original_report_on_exception = false, Thread.report_on_exception if Thread.respond_to?(:report_on_exception) @connection = ActiveRecord::Base.connection @@ -31,6 +32,7 @@ module ActiveRecord @connection.drop_table "samples", if_exists: true Thread.abort_on_exception = @abort + Thread.report_on_exception = @original_report_on_exception if Thread.respond_to?(:report_on_exception) end test "raises SerializationFailure when a serialization failure occurs" do diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 1e08cc74dc..70c0ffb3bf 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -469,6 +469,7 @@ module ActiveRecord end def test_non_bang_disconnect_and_clear_reloadable_connections_throw_exception_if_threads_dont_return_their_conns + Thread.report_on_exception, original_report_on_exception = false, Thread.report_on_exception if Thread.respond_to?(:report_on_exception) @pool.checkout_timeout = 0.001 # no need to delay test suite by waiting the whole full default timeout [:disconnect, :clear_reloadable_connections].each do |group_action_method| @pool.with_connection do |connection| @@ -477,6 +478,8 @@ module ActiveRecord end end end + ensure + Thread.report_on_exception = original_report_on_exception if Thread.respond_to?(:report_on_exception) end def test_disconnect_and_clear_reloadable_connections_attempt_to_wait_for_threads_to_return_their_conns -- cgit v1.2.3 From 6c6c3fa166975e5aebbe444605d736909e9eb75b Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Wed, 13 Dec 2017 19:46:46 +0000 Subject: Suppress `warning: BigDecimal.new is deprecated` in activerecord `BigDecimal.new` has been deprecated in BigDecimal 1.3.3 which will be a default for Ruby 2.5. Refer https://github.com/ruby/bigdecimal/commit/533737338db915b00dc7168c3602e4b462b23503 ``` $ cd rails/activerecord/ $ git grep -l BigDecimal.new | grep \.rb | xargs sed -i -e "s/BigDecimal.new/BigDecimal/g" ``` - Changes made only to Active Record. Will apply the same change to other module once this commit is merged. - The following deprecation has not been addressed because it has been reported at `ActiveRecord::Result.new`. `ActiveRecord::Result.ancestors` did not show `BigDecimal`. * Not addressed ```ruby /path/to/rails/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb:34: warning: BigDecimal.new is deprecated ``` * database_statements.rb:34 ```ruby ActiveRecord::Result.new(result.fields, result.to_a) if result ``` * ActiveRecord::Result.ancestors ```ruby [ActiveRecord::Result, Enumerable, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, Metaclass::ObjectMethods, Mocha::ObjectMethods, PP::ObjectMixin, ActiveSupport::Dependencies::Loadable, ActiveSupport::Tryable, JSON::Ext::Generator::GeneratorMethods::Object, Kernel, BasicObject] ``` This commit has been tested with these Ruby and BigDecimal versions - ruby 2.5 and bigdecimal 1.3.3 ``` $ ruby -v ruby 2.5.0dev (2017-12-14 trunk 61217) [x86_64-linux] $ gem list |grep bigdecimal bigdecimal (default: 1.3.3, default: 1.3.2) ``` - ruby 2.4 and bigdecimal 1.3.0 ``` $ ruby -v ruby 2.4.2p198 (2017-09-14 revision 59899) [x86_64-linux-gnu] $ gem list |grep bigdecimal bigdecimal (default: 1.3.0) ``` - ruby 2.3 and bigdecimal 1.2.8 ``` $ ruby -v ruby 2.3.5p376 (2017-09-14 revision 59905) [x86_64-linux] $ gem list |grep -i bigdecimal bigdecimal (1.2.8) ``` - ruby 2.2 and bigdecimal 1.2.6 ``` $ ruby -v ruby 2.2.8p477 (2017-09-14 revision 59906) [x86_64-linux] $ gem list |grep bigdecimal bigdecimal (1.2.6) ``` --- activerecord/lib/active_record/attributes.rb | 2 +- .../connection_adapters/postgresql/oid/decimal.rb | 2 +- activerecord/test/cases/adapters/postgresql/domain_test.rb | 2 +- activerecord/test/cases/adapters/postgresql/money_test.rb | 6 +++--- activerecord/test/cases/adapters/postgresql/range_test.rb | 14 +++++++------- activerecord/test/cases/adapters/postgresql/schema_test.rb | 2 +- activerecord/test/cases/adapters/sqlite3/quoting_test.rb | 2 +- activerecord/test/cases/attributes_test.rb | 2 +- activerecord/test/cases/defaults_test.rb | 2 +- .../test/cases/migration/column_attributes_test.rb | 6 +++--- activerecord/test/cases/quoting_test.rb | 2 +- activerecord/test/cases/relation/where_test.rb | 2 +- activerecord/test/cases/validations_test.rb | 4 ++-- 13 files changed, 24 insertions(+), 24 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 0b7c9398a8..35150889d9 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -57,7 +57,7 @@ module ActiveRecord # store_listing = StoreListing.new(price_in_cents: '10.1') # # # before - # store_listing.price_in_cents # => BigDecimal.new(10.1) + # store_listing.price_in_cents # => BigDecimal(10.1) # # class StoreListing < ActiveRecord::Base # attribute :price_in_cents, :integer diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb index 879dba7afd..e7d33855c4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb @@ -6,7 +6,7 @@ module ActiveRecord module OID # :nodoc: class Decimal < Type::Decimal # :nodoc: def infinity(options = {}) - BigDecimal.new("Infinity") * (options[:negative] ? -1 : 1) + BigDecimal("Infinity") * (options[:negative] ? -1 : 1) end end end diff --git a/activerecord/test/cases/adapters/postgresql/domain_test.rb b/activerecord/test/cases/adapters/postgresql/domain_test.rb index 9c3817e2ad..dafbc0a3db 100644 --- a/activerecord/test/cases/adapters/postgresql/domain_test.rb +++ b/activerecord/test/cases/adapters/postgresql/domain_test.rb @@ -44,6 +44,6 @@ class PostgresqlDomainTest < ActiveRecord::PostgreSQLTestCase record.price = "34.15" record.save! - assert_equal BigDecimal.new("34.15"), record.reload.price + assert_equal BigDecimal("34.15"), record.reload.price end end diff --git a/activerecord/test/cases/adapters/postgresql/money_test.rb b/activerecord/test/cases/adapters/postgresql/money_test.rb index 563f0bbfae..cc10890fa8 100644 --- a/activerecord/test/cases/adapters/postgresql/money_test.rb +++ b/activerecord/test/cases/adapters/postgresql/money_test.rb @@ -33,8 +33,8 @@ class PostgresqlMoneyTest < ActiveRecord::PostgreSQLTestCase end def test_default - assert_equal BigDecimal.new("150.55"), PostgresqlMoney.column_defaults["depth"] - assert_equal BigDecimal.new("150.55"), PostgresqlMoney.new.depth + assert_equal BigDecimal("150.55"), PostgresqlMoney.column_defaults["depth"] + assert_equal BigDecimal("150.55"), PostgresqlMoney.new.depth end def test_money_values @@ -65,7 +65,7 @@ class PostgresqlMoneyTest < ActiveRecord::PostgreSQLTestCase money = PostgresqlMoney.create(wealth: "987.65".dup) assert_equal 987.65, money.wealth - new_value = BigDecimal.new("123.45") + new_value = BigDecimal("123.45") money.wealth = new_value money.save! money.reload diff --git a/activerecord/test/cases/adapters/postgresql/range_test.rb b/activerecord/test/cases/adapters/postgresql/range_test.rb index a75fdef698..813a8721a2 100644 --- a/activerecord/test/cases/adapters/postgresql/range_test.rb +++ b/activerecord/test/cases/adapters/postgresql/range_test.rb @@ -134,10 +134,10 @@ _SQL end def test_numrange_values - assert_equal BigDecimal.new("0.1")..BigDecimal.new("0.2"), @first_range.num_range - assert_equal BigDecimal.new("0.1")...BigDecimal.new("0.2"), @second_range.num_range - assert_equal BigDecimal.new("0.1")...BigDecimal.new("Infinity"), @third_range.num_range - assert_equal BigDecimal.new("-Infinity")...BigDecimal.new("Infinity"), @fourth_range.num_range + assert_equal BigDecimal("0.1")..BigDecimal("0.2"), @first_range.num_range + assert_equal BigDecimal("0.1")...BigDecimal("0.2"), @second_range.num_range + assert_equal BigDecimal("0.1")...BigDecimal("Infinity"), @third_range.num_range + assert_equal BigDecimal("-Infinity")...BigDecimal("Infinity"), @fourth_range.num_range assert_nil @empty_range.num_range end @@ -285,14 +285,14 @@ _SQL def test_create_numrange assert_equal_round_trip(@new_range, :num_range, - BigDecimal.new("0.5")...BigDecimal.new("1")) + BigDecimal("0.5")...BigDecimal("1")) end def test_update_numrange assert_equal_round_trip(@first_range, :num_range, - BigDecimal.new("0.5")...BigDecimal.new("1")) + BigDecimal("0.5")...BigDecimal("1")) assert_nil_round_trip(@first_range, :num_range, - BigDecimal.new("0.5")...BigDecimal.new("0.5")) + BigDecimal("0.5")...BigDecimal("0.5")) end def test_create_daterange diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 1126908761..2c99fa78bd 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -566,7 +566,7 @@ class DefaultsUsingMultipleSchemasAndDomainTest < ActiveRecord::PostgreSQLTestCa end def test_decimal_defaults_in_new_schema_when_overriding_domain - assert_equal BigDecimal.new("3.14159265358979323846"), Default.new.decimal_col, "Default of decimal column was not correctly parsed" + assert_equal BigDecimal("3.14159265358979323846"), Default.new.decimal_col, "Default of decimal column was not correctly parsed" end def test_bpchar_defaults_in_new_schema_when_overriding_domain diff --git a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb index de422fad23..6fdb353368 100644 --- a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -38,7 +38,7 @@ class SQLite3QuotingTest < ActiveRecord::SQLite3TestCase end def test_type_cast_bigdecimal - bd = BigDecimal.new "10.0" + bd = BigDecimal "10.0" assert_equal bd.to_f, @conn.type_cast(bd) end diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb index 2caf2a63d4..8ebfee61ff 100644 --- a/activerecord/test/cases/attributes_test.rb +++ b/activerecord/test/cases/attributes_test.rb @@ -59,7 +59,7 @@ module ActiveRecord test "nonexistent attribute" do data = OverloadedType.new(non_existent_decimal: 1) - assert_equal BigDecimal.new(1), data.non_existent_decimal + assert_equal BigDecimal(1), data.non_existent_decimal assert_raise ActiveRecord::UnknownAttributeError do UnoverloadedType.new(non_existent_decimal: 1) end diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index 4690682cd8..3d11b573f1 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -53,7 +53,7 @@ class DefaultNumbersTest < ActiveRecord::TestCase def test_default_decimal_number record = DefaultNumber.new - assert_equal BigDecimal.new("2.78"), record.decimal_number + assert_equal BigDecimal("2.78"), record.decimal_number assert_equal "2.78", record.decimal_number_before_type_cast end end diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb index be6dc2acb1..3022121f4c 100644 --- a/activerecord/test/cases/migration/column_attributes_test.rb +++ b/activerecord/test/cases/migration/column_attributes_test.rb @@ -80,7 +80,7 @@ module ActiveRecord TestModel.delete_all # Now use the Rails insertion - TestModel.create wealth: BigDecimal.new("12345678901234567890.0123456789") + TestModel.create wealth: BigDecimal("12345678901234567890.0123456789") # SELECT row = TestModel.first @@ -146,7 +146,7 @@ module ActiveRecord TestModel.create first_name: "bob", last_name: "bobsen", bio: "I was born ....", age: 18, height: 1.78, - wealth: BigDecimal.new("12345678901234567890.0123456789"), + wealth: BigDecimal("12345678901234567890.0123456789"), birthday: 18.years.ago, favorite_day: 10.days.ago, moment_of_truth: "1782-10-10 21:40:18", male: true @@ -159,7 +159,7 @@ module ActiveRecord # Test for 30 significant digits (beyond the 16 of float), 10 of them # after the decimal place. - assert_equal BigDecimal.new("0012345678901234567890.0123456789"), bob.wealth + assert_equal BigDecimal("0012345678901234567890.0123456789"), bob.wealth assert_equal true, bob.male? diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index 897d252cf8..6534770c57 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -111,7 +111,7 @@ module ActiveRecord end def test_quote_bigdecimal - bigdec = BigDecimal.new((1 << 100).to_s) + bigdec = BigDecimal((1 << 100).to_s) assert_equal bigdec.to_s("F"), @quoter.quote(bigdec) end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index d95a54a2fe..99797528b2 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -265,7 +265,7 @@ module ActiveRecord end def test_where_with_decimal_for_string_column - count = Post.where(title: BigDecimal.new(0)).count + count = Post.where(title: BigDecimal(0)).count assert_equal 0, count end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 7f84939027..14623c43d2 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -175,12 +175,12 @@ class ValidationsTest < ActiveRecord::TestCase ActiveModel::Name.new(self, nil, "Topic") end attribute :wibble, :decimal, scale: 2, precision: 9 - validates_numericality_of :wibble, greater_than_or_equal_to: BigDecimal.new("97.18") + validates_numericality_of :wibble, greater_than_or_equal_to: BigDecimal("97.18") end assert_not klass.new(wibble: "97.179").valid? assert_not klass.new(wibble: 97.179).valid? - assert_not klass.new(wibble: BigDecimal.new("97.179")).valid? + assert_not klass.new(wibble: BigDecimal("97.179")).valid? end def test_acceptance_validator_doesnt_require_db_connection -- cgit v1.2.3 From 3876defd7c951335129fa92c573c7b8fce085aac Mon Sep 17 00:00:00 2001 From: Olivier Lacan Date: Wed, 19 Oct 2016 00:59:21 +0200 Subject: Log call site for all queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This new ActiveRecord configuration option allows you to easily pinpoint what line of application code is triggering SQL queries in the development log by appending below each SQL statement log the line of Ruby code that triggered it. It’s useful with N+1 issues, and to locate stray queries. By default this new option ignores Rails and Ruby code in order to surface only callers from your application Ruby code or your gems. It is enabled on newly generated Rails 5.2 applications and can be enabled on existing Rails applications: ```ruby Rails.application.configure do # ... config.active_record.verbose_query_logs = true end ``` The `rails app:upgrade` task will also add it to `config/development.rb`. This feature purposely avoids coupling with ActiveSupport::BacktraceCleaner since ActiveRecord can be used without ActiveRecord. This decision can be reverted in the future to allow more configurable backtraces (the exclusion of gem callers for example). --- activerecord/CHANGELOG.md | 13 +++++++- activerecord/lib/active_record/core.rb | 7 ++++ activerecord/lib/active_record/log_subscriber.rb | 42 ++++++++++++++++++++++++ activerecord/test/cases/log_subscriber_test.rb | 21 ++++++++++-- 4 files changed, 80 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ceef240257..c88a4e7695 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,15 @@ +* Log database query callers + + Add `verbose_query_logs` configuration option to display the caller + of database queries in the log to facilitate N+1 query resolution + and other debugging. Excludes Ruby and Rails callers but not gems. + + Enabled in development only for new and upgraded applications. Not + recommended for use in the production environment since it relies + on Ruby's `Kernel#caller` which is fairly slow. + + *Olivier Lacan* + * Fix conflicts `counter_cache` with `touch: true` by optimistic locking. ``` @@ -536,5 +548,4 @@ *Kevin McPhillips* - Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 481159e501..88810cb328 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -17,6 +17,13 @@ module ActiveRecord # retrieved on both a class and instance level by calling +logger+. mattr_accessor :logger, instance_writer: false + ## + # :singleton-method: + # + # Specifies if the methods calling database queries should be logged below + # their relevant queries. Defaults to false. + mattr_accessor :verbose_query_logs, instance_writer: false, default: false + ## # Contains the database configuration - as is typically stored in config/database.yml - # as a Hash. diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 405f3a30c6..ba3419be6c 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -90,6 +90,48 @@ module ActiveRecord def logger ActiveRecord::Base.logger end + + def debug(progname = nil, &block) + return unless super + + if ActiveRecord::Base.verbose_query_logs + log_query_source + end + end + + def log_query_source + source_line, line_number = extract_callstack(caller_locations) + + if source_line + if defined?(::Rails.root) + app_root = "#{::Rails.root.to_s}/".freeze + source_line = source_line.sub(app_root, "") + end + + logger.debug(" ↳ #{ source_line }:#{ line_number }") + end + end + + def extract_callstack(callstack) + line = callstack.find do |frame| + frame.absolute_path && !ignored_callstack(frame.absolute_path) + end + + offending_line = line || callstack.first + + [ + offending_line.path, + offending_line.lineno, + offending_line.label + ] + end + + RAILS_GEM_ROOT = File.expand_path("../../../..", __FILE__) + "/" + + def ignored_callstack(path) + path.start_with?(RAILS_GEM_ROOT) || + path.start_with?(RbConfig::CONFIG["rubylibdir"]) + end end end diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 208e54ed0b..e2742ed33e 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -33,8 +33,9 @@ class LogSubscriberTest < ActiveRecord::TestCase super end - def debug(message) - @debugs << message + def debug(progname = nil, &block) + @debugs << progname + super end end @@ -171,6 +172,22 @@ class LogSubscriberTest < ActiveRecord::TestCase assert_match(/SELECT .*?FROM .?developers.?/i, @logger.logged(:debug).last) end + def test_vebose_query_logs + ActiveRecord::Base.verbose_query_logs = true + + logger = TestDebugLogSubscriber.new + logger.sql(Event.new(0, sql: "hi mom!")) + assert_match(/↳/, @logger.logged(:debug).last) + ensure + ActiveRecord::Base.verbose_query_logs = false + end + + def test_verbose_query_logs_disabled_by_default + logger = TestDebugLogSubscriber.new + logger.sql(Event.new(0, sql: "hi mom!")) + assert_no_match(/↳/, @logger.logged(:debug).last) + end + def test_cached_queries ActiveRecord::Base.cache do Developer.all.load -- cgit v1.2.3 From 245c1dafa8bab409fbcd780a996c619240df7143 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 14 Dec 2017 17:30:54 +0900 Subject: Enable `Layout/LeadingCommentSpace` to not allow cosmetic changes in the future Follow up of #31432. --- activerecord/test/cases/associations/eager_test.rb | 2 +- .../cases/associations/has_one_through_associations_test.rb | 10 +++++----- activerecord/test/cases/associations/join_model_test.rb | 2 +- activerecord/test/cases/dup_test.rb | 6 +++--- activerecord/test/cases/finder_test.rb | 2 +- activerecord/test/cases/fixtures_test.rb | 2 +- activerecord/test/cases/statement_cache_test.rb | 3 --- 7 files changed, 12 insertions(+), 15 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 9a042c74db..2649dc010f 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1073,7 +1073,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_load_with_sti_sharing_association - assert_queries(2) do #should not do 1 query per subclass + assert_queries(2) do # should not do 1 query per subclass Comment.includes(:post).to_a end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index fe24c465b2..1d37457464 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -100,7 +100,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end def test_has_one_through_eager_loading - members = assert_queries(3) do #base table, through table, clubs table + members = assert_queries(3) do # base table, through table, clubs table Member.all.merge!(includes: :club, where: ["name = ?", "Groucho Marx"]).to_a end assert_equal 1, members.size @@ -108,7 +108,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end def test_has_one_through_eager_loading_through_polymorphic - members = assert_queries(3) do #base table, through table, clubs table + members = assert_queries(3) do # base table, through table, clubs table Member.all.merge!(includes: :sponsor_club, where: ["name = ?", "Groucho Marx"]).to_a end assert_equal 1, members.size @@ -139,7 +139,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_nonpreload_eagerloading members = assert_queries(1) do - Member.all.merge!(includes: :club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a #force fallback + Member.all.merge!(includes: :club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a # force fallback end assert_equal 1, members.size assert_not_nil assert_no_queries { members[0].club } @@ -147,7 +147,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_nonpreload_eager_loading_through_polymorphic members = assert_queries(1) do - Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a #force fallback + Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a # force fallback end assert_equal 1, members.size assert_not_nil assert_no_queries { members[0].sponsor_club } @@ -156,7 +156,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_nonpreload_eager_loading_through_polymorphic_with_more_than_one_through_record Sponsor.new(sponsor_club: clubs(:crazy_club), sponsorable: members(:groucho)).save! members = assert_queries(1) do - Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name DESC").to_a #force fallback + Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name DESC").to_a # force fallback end assert_equal 1, members.size assert_not_nil assert_no_queries { members[0].sponsor_club } diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 87694b0788..5d83c9435b 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -467,7 +467,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase new_tag = Tag.new(name: "new") saved_post.tags << new_tag - assert new_tag.persisted? #consistent with habtm! + assert new_tag.persisted? # consistent with habtm! assert saved_post.persisted? assert_includes saved_post.tags, new_tag diff --git a/activerecord/test/cases/dup_test.rb b/activerecord/test/cases/dup_test.rb index 2fefdbf204..73da31996e 100644 --- a/activerecord/test/cases/dup_test.rb +++ b/activerecord/test/cases/dup_test.rb @@ -62,10 +62,10 @@ module ActiveRecord topic.attributes = dbtopic.attributes.except("id") - #duped has no timestamp values + # duped has no timestamp values duped = dbtopic.dup - #clear topic timestamp values + # clear topic timestamp values topic.send(:clear_timestamp_attributes) assert_equal topic.changes, duped.changes @@ -100,7 +100,7 @@ module ActiveRecord # temporary change to the topic object topic.updated_at -= 3.days - #dup should not preserve the timestamps if present + # dup should not preserve the timestamps if present new_topic = topic.dup assert_nil new_topic.updated_at assert_nil new_topic.created_at diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 2076b57aa8..62d5d88fcc 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -565,7 +565,7 @@ class FinderTest < ActiveRecord::TestCase assert_nil Topic.offset(4).second_to_last assert_nil Topic.offset(5).second_to_last - #test with limit + # test with limit assert_nil Topic.limit(1).second assert_nil Topic.limit(1).second_to_last end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index b0b63f5203..8e8a49af8e 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -247,7 +247,7 @@ class FixturesTest < ActiveRecord::TestCase def test_nonexistent_fixture_file nonexistent_fixture_path = FIXTURES_ROOT + "/imnothere" - #sanity check to make sure that this file never exists + # sanity check to make sure that this file never exists assert Dir[nonexistent_fixture_path + "*"].empty? assert_raise(Errno::ENOENT) do diff --git a/activerecord/test/cases/statement_cache_test.rb b/activerecord/test/cases/statement_cache_test.rb index 1f715e41a6..ad6cd198e2 100644 --- a/activerecord/test/cases/statement_cache_test.rb +++ b/activerecord/test/cases/statement_cache_test.rb @@ -12,7 +12,6 @@ module ActiveRecord @connection = ActiveRecord::Base.connection end - #Cache v 1.1 tests def test_statement_cache Book.create(name: "my book") Book.create(name: "my other book") @@ -51,8 +50,6 @@ module ActiveRecord assert_equal("my other book", b.name) end - #End - def test_statement_cache_with_simple_statement cache = ActiveRecord::StatementCache.create(Book.connection) do |params| Book.where(name: "my book").where("author_id > 3") -- cgit v1.2.3 From 521c9468e252129770813fa110ca7272c0096aaa Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Thu, 14 Dec 2017 19:30:50 +0000 Subject: Suppress expected exceptions by `report_on_exception` = `false` Follow up #31428 to address similar exceptions with mysql2 adapter --- activerecord/test/cases/adapters/mysql2/transaction_test.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql2/transaction_test.rb b/activerecord/test/cases/adapters/mysql2/transaction_test.rb index cb183cc54c..f921515c10 100644 --- a/activerecord/test/cases/adapters/mysql2/transaction_test.rb +++ b/activerecord/test/cases/adapters/mysql2/transaction_test.rb @@ -13,6 +13,7 @@ module ActiveRecord setup do @abort, Thread.abort_on_exception = Thread.abort_on_exception, false + Thread.report_on_exception, @original_report_on_exception = false, Thread.report_on_exception if Thread.respond_to?(:report_on_exception) @connection = ActiveRecord::Base.connection @connection.clear_cache! @@ -31,6 +32,7 @@ module ActiveRecord @connection.drop_table "samples", if_exists: true Thread.abort_on_exception = @abort + Thread.report_on_exception = @original_report_on_exception if Thread.respond_to?(:report_on_exception) end test "raises Deadlocked when a deadlock is encountered" do -- cgit v1.2.3