From 86e74a083a2412676eb18b634d4623b0b76d1d79 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 27 Apr 2017 15:44:25 +0900 Subject: Restore `fixtures :author_addresses` This change reverted in eac6f369 but it is needed for data integrity. See #25328. --- activerecord/test/cases/associations/callbacks_test.rb | 2 +- activerecord/test/cases/associations/cascaded_eager_loading_test.rb | 2 +- activerecord/test/cases/associations/has_many_associations_test.rb | 6 +++--- .../test/cases/associations/has_one_through_associations_test.rb | 2 +- activerecord/test/cases/associations/inner_join_association_test.rb | 2 +- activerecord/test/cases/associations/join_model_test.rb | 2 +- .../test/cases/associations/left_outer_join_association_test.rb | 2 +- .../test/cases/associations/nested_through_associations_test.rb | 2 +- activerecord/test/cases/associations_test.rb | 4 ++-- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/bind_parameter_test.rb | 2 +- activerecord/test/cases/fixtures_test.rb | 2 +- activerecord/test/cases/json_serialization_test.rb | 2 +- activerecord/test/cases/readonly_test.rb | 2 +- activerecord/test/cases/relation/merging_test.rb | 4 ++-- activerecord/test/cases/relation/where_test.rb | 2 +- activerecord/test/cases/relation_test.rb | 2 +- activerecord/test/cases/relations_test.rb | 2 +- activerecord/test/cases/scoping/relation_scoping_test.rb | 4 ++-- activerecord/test/cases/transactions_test.rb | 2 +- activerecord/test/cases/yaml_serialization_test.rb | 2 +- 21 files changed, 26 insertions(+), 26 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/callbacks_test.rb b/activerecord/test/cases/associations/callbacks_test.rb index 5fd2411f6f..7721bd5cd9 100644 --- a/activerecord/test/cases/associations/callbacks_test.rb +++ b/activerecord/test/cases/associations/callbacks_test.rb @@ -7,7 +7,7 @@ require "models/computer" require "models/company" class AssociationCallbacksTest < ActiveRecord::TestCase - fixtures :posts, :authors, :projects, :developers + fixtures :posts, :authors, :author_addresses, :projects, :developers def setup @david = authors(:david) diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index ddb5c7a4aa..3638c87968 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -12,7 +12,7 @@ require "models/vertex" require "models/edge" class CascadedEagerLoadingTest < ActiveRecord::TestCase - fixtures :authors, :mixins, :companies, :posts, :topics, :accounts, :comments, + fixtures :authors, :author_addresses, :mixins, :companies, :posts, :topics, :accounts, :comments, :categorizations, :people, :categories, :edges, :vertices def test_eager_association_loading_with_cascaded_two_levels diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 1eb10c1dbe..6a479a344c 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -40,7 +40,7 @@ require "models/zine" require "models/interest" class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase - fixtures :authors, :posts, :comments + fixtures :authors, :author_addresses, :posts, :comments def test_should_generate_valid_sql author = authors(:david) @@ -51,7 +51,7 @@ class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCa end class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase - fixtures :authors, :essays, :subscribers, :subscriptions, :people + fixtures :authors, :author_addresses, :essays, :subscribers, :subscriptions, :people def test_custom_primary_key_on_new_record_should_fetch_with_query subscriber = Subscriber.new(nick: "webster132") @@ -100,7 +100,7 @@ end class HasManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :categories, :companies, :developers, :projects, - :developers_projects, :topics, :authors, :comments, + :developers_projects, :topics, :authors, :author_addresses, :comments, :posts, :readers, :taggings, :cars, :jobs, :tags, :categorizations, :zines, :interests 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 38a729d2d4..28b883586d 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -23,7 +23,7 @@ require "models/customer_carrier" class HasOneThroughAssociationsTest < ActiveRecord::TestCase fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans, - :dashboards, :speedometers, :authors, :posts, :comments, :categories, :essays, :owners + :dashboards, :speedometers, :authors, :author_addresses, :posts, :comments, :categories, :essays, :owners def setup @member = members(:groucho) diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index 7414869c8f..ddf5bc6f0b 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -10,7 +10,7 @@ require "models/tagging" require "models/tag" class InnerJoinAssociationTest < ActiveRecord::TestCase - fixtures :authors, :essays, :posts, :comments, :categories, :categories_posts, :categorizations, + fixtures :authors, :author_addresses, :essays, :posts, :comments, :categories, :categories_posts, :categorizations, :taggings, :tags def test_construct_finder_sql_applies_aliases_tables_on_association_conditions diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index a4345f3857..c078cef064 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -19,7 +19,7 @@ require "models/car" class AssociationsJoinModelTest < ActiveRecord::TestCase self.use_transactional_tests = false unless supports_savepoints? - fixtures :posts, :authors, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books, + fixtures :posts, :authors, :author_addresses, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books, # Reload edges table from fixtures as otherwise repeated test was failing :edges diff --git a/activerecord/test/cases/associations/left_outer_join_association_test.rb b/activerecord/test/cases/associations/left_outer_join_association_test.rb index 42dbbad1c8..6d3757f467 100644 --- a/activerecord/test/cases/associations/left_outer_join_association_test.rb +++ b/activerecord/test/cases/associations/left_outer_join_association_test.rb @@ -7,7 +7,7 @@ require "models/categorization" require "models/person" class LeftOuterJoinAssociationTest < ActiveRecord::TestCase - fixtures :authors, :essays, :posts, :comments, :categorizations, :people + fixtures :authors, :author_addresses, :essays, :posts, :comments, :categorizations, :people def test_construct_finder_sql_applies_aliases_tables_on_association_conditions result = Author.left_outer_joins(:thinking_posts, :welcome_posts).to_a diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index dc26f6a383..67ff7355b3 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -24,7 +24,7 @@ require "models/membership" require "models/essay" class NestedThroughAssociationsTest < ActiveRecord::TestCase - fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings, + fixtures :authors, :author_addresses, :books, :posts, :subscriptions, :subscribers, :tags, :taggings, :people, :readers, :references, :jobs, :ratings, :comments, :members, :member_details, :member_types, :sponsors, :clubs, :organizations, :categories, :categories_posts, :categorizations, :memberships, :essays diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 26056f6f63..4ab690bfc6 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -22,7 +22,7 @@ require "models/interest" class AssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, - :computers, :people, :readers, :authors, :author_favorites + :computers, :people, :readers, :authors, :author_addresses, :author_favorites def test_eager_loading_should_not_change_count_of_children liquid = Liquid.create(name: "salty") @@ -111,7 +111,7 @@ class AssociationsTest < ActiveRecord::TestCase end class AssociationProxyTest < ActiveRecord::TestCase - fixtures :authors, :posts, :categorizations, :categories, :developers, :projects, :developers_projects + fixtures :authors, :author_addresses, :posts, :categorizations, :categories, :developers, :projects, :developers_projects def test_push_does_not_load_target david = authors(:david) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 8efbc41da9..15c253890b 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -64,7 +64,7 @@ class LintTest < ActiveRecord::TestCase end class BasicsTest < ActiveRecord::TestCase - fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, "warehouse-things", :authors, :categorizations, :categories, :posts + fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, "warehouse-things", :authors, :author_addresses, :categorizations, :categories, :posts def test_column_names_are_escaped conn = ActiveRecord::Base.connection diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index a0f83df80b..6032aa9250 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -7,7 +7,7 @@ if ActiveRecord::Base.connection.supports_statement_cache? && ActiveRecord::Base.connection.prepared_statements module ActiveRecord class BindParameterTest < ActiveRecord::TestCase - fixtures :topics, :authors, :posts + fixtures :topics, :authors, :author_addresses, :posts class LogListener attr_accessor :calls diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 4137a1e955..a0a6d3c7ef 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -784,7 +784,7 @@ end class FasterFixturesTest < ActiveRecord::TestCase self.use_transactional_tests = false - fixtures :categories, :authors + fixtures :categories, :authors, :author_addresses def load_extra_fixture(name) fixture = create_fixtures(name).first diff --git a/activerecord/test/cases/json_serialization_test.rb b/activerecord/test/cases/json_serialization_test.rb index 5a1d066aef..9b4b61b16e 100644 --- a/activerecord/test/cases/json_serialization_test.rb +++ b/activerecord/test/cases/json_serialization_test.rb @@ -168,7 +168,7 @@ class JsonSerializationTest < ActiveRecord::TestCase end class DatabaseConnectedJsonEncodingTest < ActiveRecord::TestCase - fixtures :authors, :posts, :comments, :tags, :taggings + fixtures :authors, :author_addresses, :posts, :comments, :tags, :taggings include JsonSerializationHelpers diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb index a93061b516..24b678310d 100644 --- a/activerecord/test/cases/readonly_test.rb +++ b/activerecord/test/cases/readonly_test.rb @@ -10,7 +10,7 @@ require "models/person" require "models/ship" class ReadOnlyTest < ActiveRecord::TestCase - fixtures :authors, :posts, :comments, :developers, :projects, :developers_projects, :people, :readers + fixtures :authors, :author_addresses, :posts, :comments, :developers, :projects, :developers_projects, :people, :readers def test_cant_save_readonly_record dev = Developer.find(1) diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 67d76de798..64866eaf2d 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -8,7 +8,7 @@ require "models/project" require "models/rating" class RelationMergingTest < ActiveRecord::TestCase - fixtures :developers, :comments, :authors, :posts + fixtures :developers, :comments, :authors, :author_addresses, :posts def test_relation_merging devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order("id ASC").where("id < 3")) @@ -114,7 +114,7 @@ class RelationMergingTest < ActiveRecord::TestCase end class MergingDifferentRelationsTest < ActiveRecord::TestCase - fixtures :posts, :authors, :developers + fixtures :posts, :authors, :author_addresses, :developers test "merging where relations" do hello_by_bob = Post.where(body: "hello").joins(:author). diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index 9a333e0d5a..cbc466d6b8 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -15,7 +15,7 @@ require "models/vertex" module ActiveRecord class WhereTest < ActiveRecord::TestCase - fixtures :posts, :edges, :authors, :binaries, :essays, :cars, :treasures, :price_estimates + fixtures :posts, :edges, :authors, :author_addresses, :binaries, :essays, :cars, :treasures, :price_estimates def test_where_copies_bind_params author = authors(:david) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 1241bb54a4..5fb32270b7 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -6,7 +6,7 @@ require "models/rating" module ActiveRecord class RelationTest < ActiveRecord::TestCase - fixtures :posts, :comments, :authors + fixtures :posts, :comments, :authors, :author_addresses FakeKlass = Struct.new(:table_name, :name) do extend ActiveRecord::Delegation::DelegateCache diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index d32db0558f..7a710f1004 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -22,7 +22,7 @@ require "models/categorization" require "models/edge" class RelationTest < ActiveRecord::TestCase - fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments, + fixtures :authors, :author_addresses, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments, :tags, :taggings, :cars, :minivans class TopicWithCallbacks < ActiveRecord::Base diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index a1ae57fdbb..3fbff7664b 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -10,7 +10,7 @@ require "models/person" require "models/reference" class RelationScopingTest < ActiveRecord::TestCase - fixtures :authors, :developers, :projects, :comments, :posts, :developers_projects + fixtures :authors, :author_addresses, :developers, :projects, :comments, :posts, :developers_projects setup do developers(:david) @@ -238,7 +238,7 @@ class RelationScopingTest < ActiveRecord::TestCase end class NestedRelationScopingTest < ActiveRecord::TestCase - fixtures :authors, :developers, :projects, :comments, :posts + fixtures :authors, :author_addresses, :developers, :projects, :comments, :posts def test_merge_options Developer.where("salary = 80000").scoping do diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 111495c481..5c6d78b574 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -10,7 +10,7 @@ require "models/movie" class TransactionTest < ActiveRecord::TestCase self.use_transactional_tests = false - fixtures :topics, :developers, :authors, :posts + fixtures :topics, :developers, :authors, :author_addresses, :posts def setup @first, @second = Topic.find(1, 2).sort_by(&:id) diff --git a/activerecord/test/cases/yaml_serialization_test.rb b/activerecord/test/cases/yaml_serialization_test.rb index 1571b31329..ab0e67cd9d 100644 --- a/activerecord/test/cases/yaml_serialization_test.rb +++ b/activerecord/test/cases/yaml_serialization_test.rb @@ -5,7 +5,7 @@ require "models/post" require "models/author" class YamlSerializationTest < ActiveRecord::TestCase - fixtures :topics, :authors, :posts + fixtures :topics, :authors, :author_addresses, :posts def test_to_yaml_with_time_with_zone_should_not_raise_exception with_timezone_config aware_attributes: true, zone: "Pacific Time (US & Canada)" do -- cgit v1.2.3 From 4f8f5f59fc37a6adb71979eec66488f4ad08bffa Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Thu, 27 Apr 2017 12:35:48 -0400 Subject: Evaluate belongs_to :default option against the owner, not the association --- .../associations/belongs_to_association.rb | 2 +- .../cases/associations/belongs_to_associations_test.rb | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index a2432e389a..0e61dbfb00 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -22,7 +22,7 @@ module ActiveRecord end def default(&block) - writer(instance_exec(&block)) if reader.nil? + writer(owner.instance_exec(&block)) if reader.nil? end def reset diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 5b08ba1358..c8b26232b6 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -136,6 +136,24 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal david, ship.developer end + def test_default_with_lambda + model = Class.new(ActiveRecord::Base) do + self.table_name = "ships" + def self.name; "Temp"; end + belongs_to :developer, default: -> { default_developer } + + def default_developer + Developer.first + end + end + + ship = model.create! + assert_equal developers(:david), ship.developer + + ship = model.create!(developer: developers(:jamis)) + assert_equal developers(:jamis), ship.developer + end + def test_default_scope_on_relations_is_not_cached counter = 0 -- cgit v1.2.3 From 15a4d3c383e6120bb0670fe2f9f07f72a5825027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 27 Apr 2017 10:21:15 -0700 Subject: Also raise error when VERSION is nil Fix #28905 --- activerecord/lib/active_record/railties/databases.rake | 4 ++-- activerecord/test/cases/tasks/database_tasks_test.rb | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index f3e2df8786..711099e9e1 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -93,7 +93,7 @@ db_namespace = namespace :db do # desc 'Runs the "up" for a given migration VERSION.' task up: [:environment, :load_config] do - raise "VERSION is required" if ENV["VERSION"] && ENV["VERSION"].empty? + raise "VERSION is required" if !ENV["VERSION"] || ENV["VERSION"].empty? version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil ActiveRecord::Migrator.run(:up, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version) @@ -102,7 +102,7 @@ db_namespace = namespace :db do # desc 'Runs the "down" for a given migration VERSION.' task down: [:environment, :load_config] do - raise "VERSION is required - To go down one migration, use db:rollback" if ENV["VERSION"] && ENV["VERSION"].empty? + raise "VERSION is required - To go down one migration, use db:rollback" if !ENV["VERSION"] || ENV["VERSION"].empty? version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil ActiveRecord::Migrator.run(:down, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version) db_namespace["_dump"].invoke diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index b4983624ba..c47c97e9d9 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -368,7 +368,8 @@ module ActiveRecord def test_migrate_raise_error_on_empty_version version = ENV["VERSION"] ENV["VERSION"] = "" - assert_raise(RuntimeError, "Empty VERSION provided") { ActiveRecord::Tasks::DatabaseTasks.migrate } + e = assert_raise(RuntimeError) { ActiveRecord::Tasks::DatabaseTasks.migrate } + assert_equal "Empty VERSION provided", e.message ensure ENV["VERSION"] = version end -- cgit v1.2.3 From e82d2251e3fc64b415513904b014aa84d5ac6103 Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Fri, 28 Apr 2017 21:31:36 -0400 Subject: Stop creating duplicate Struct instances Just use one `Event` class. Reduces duplication, makes the tests easier to read. It might seem like each tests needs a different kind of Struct, since we make a new one for each test case. --- activerecord/test/cases/log_subscriber_test.rb | 38 ++++++++++---------------- 1 file changed, 15 insertions(+), 23 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 90ad970e16..8c67aa8746 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -21,6 +21,7 @@ class LogSubscriberTest < ActiveRecord::TestCase TRANSACTION: REGEXP_CYAN, OTHER: REGEXP_MAGENTA } + Event = Struct.new(:duration, :payload) class TestDebugLogSubscriber < ActiveRecord::LogSubscriber attr_reader :debugs @@ -55,25 +56,22 @@ class LogSubscriberTest < ActiveRecord::TestCase end def test_schema_statements_are_ignored - event = Struct.new(:duration, :payload) - logger = TestDebugLogSubscriber.new assert_equal 0, logger.debugs.length - logger.sql(event.new(0, sql: "hi mom!")) + logger.sql(Event.new(0, sql: "hi mom!")) assert_equal 1, logger.debugs.length - logger.sql(event.new(0, sql: "hi mom!", name: "foo")) + logger.sql(Event.new(0, sql: "hi mom!", name: "foo")) assert_equal 2, logger.debugs.length - logger.sql(event.new(0, sql: "hi mom!", name: "SCHEMA")) + logger.sql(Event.new(0, sql: "hi mom!", name: "SCHEMA")) assert_equal 2, logger.debugs.length end def test_sql_statements_are_not_squeezed - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new - logger.sql(event.new(0, sql: "ruby rails")) + logger.sql(Event.new(0, sql: "ruby rails")) assert_match(/ruby rails/, logger.debugs.first) end @@ -86,56 +84,51 @@ class LogSubscriberTest < ActiveRecord::TestCase end def test_basic_query_logging_coloration - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.each do |verb, color_regex| - logger.sql(event.new(0, sql: verb.to_s)) + logger.sql(Event.new(0, sql: verb.to_s)) assert_match(/#{REGEXP_BOLD}#{color_regex}#{verb}#{REGEXP_CLEAR}/i, logger.debugs.last) end end def test_basic_payload_name_logging_coloration_generic_sql - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.each do |verb, _| - logger.sql(event.new(0, sql: verb.to_s)) + logger.sql(Event.new(0, sql: verb.to_s)) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) - logger.sql(event.new(0, sql: verb.to_s, name: "SQL")) + logger.sql(Event.new(0, sql: verb.to_s, name: "SQL")) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA}SQL \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) end end def test_basic_payload_name_logging_coloration_named_sql - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.each do |verb, _| - logger.sql(event.new(0, sql: verb.to_s, name: "Model Load")) + logger.sql(Event.new(0, sql: verb.to_s, name: "Model Load")) assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Load \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) - logger.sql(event.new(0, sql: verb.to_s, name: "Model Exists")) + logger.sql(Event.new(0, sql: verb.to_s, name: "Model Exists")) assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Exists \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) - logger.sql(event.new(0, sql: verb.to_s, name: "ANY SPECIFIC NAME")) + logger.sql(Event.new(0, sql: verb.to_s, name: "ANY SPECIFIC NAME")) assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}ANY SPECIFIC NAME \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) end end def test_query_logging_coloration_with_nested_select - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.slice(:SELECT, :INSERT, :UPDATE, :DELETE).each do |verb, color_regex| - logger.sql(event.new(0, sql: "#{verb} WHERE ID IN SELECT")) + logger.sql(Event.new(0, sql: "#{verb} WHERE ID IN SELECT")) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}#{verb} WHERE ID IN SELECT#{REGEXP_CLEAR}/i, logger.debugs.last) end end def test_query_logging_coloration_with_multi_line_nested_select - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.slice(:SELECT, :INSERT, :UPDATE, :DELETE).each do |verb, color_regex| @@ -145,13 +138,12 @@ class LogSubscriberTest < ActiveRecord::TestCase SELECT ID FROM THINGS ) EOS - logger.sql(event.new(0, sql: sql)) + logger.sql(Event.new(0, sql: sql)) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}.*#{verb}.*#{REGEXP_CLEAR}/mi, logger.debugs.last) end end def test_query_logging_coloration_with_lock - event = Struct.new(:duration, :payload) logger = TestDebugLogSubscriber.new logger.colorize_logging = true sql = <<-EOS @@ -159,13 +151,13 @@ class LogSubscriberTest < ActiveRecord::TestCase (SELECT * FROM mytable FOR UPDATE) ss WHERE col1 = 5; EOS - logger.sql(event.new(0, sql: sql)) + logger.sql(Event.new(0, sql: sql)) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*FOR UPDATE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) sql = <<-EOS LOCK TABLE films IN SHARE MODE; EOS - logger.sql(event.new(0, sql: sql)) + logger.sql(Event.new(0, sql: sql)) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*LOCK TABLE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) end -- cgit v1.2.3 From 4e6d84f5553421ad1d24d4d137d46c85d4e1881c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 29 Apr 2017 23:30:38 +0900 Subject: Use `flat_map` rather than `map(&:...).flatten` --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 24ca8b0be4..5ce134d6b7 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -212,7 +212,7 @@ module ActiveRecord end def constraints - chain.map(&:scopes).flatten + chain.flat_map(&:scopes) end def counter_cache_column -- cgit v1.2.3 From 89389428b5d04e14f09009ac9af593d7c3da709f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 30 Apr 2017 02:41:44 +0900 Subject: Cleanup CHANGELOGs [ci skip] * Remove trailing spaces. * Add backticks around method and command. * Fix indentation. --- activerecord/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a229b4eaa2..68f0638e32 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,4 @@ -* Quote database name in db:create grant statement (when database_user does not have access to create the database). +* Quote database name in `db:create` grant statement (when database user does not have access to create the database). *Rune Philosof* -- cgit v1.2.3 From 9caa0cf57d2f73b3ad6c34f00c34b5fdf75fe8fd Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 5 Apr 2017 11:35:47 +0900 Subject: Deprecate `supports_statement_cache?` `supports_statement_cache?` was introduced in 3.1.0.beta1 (104d0b2) for bind parameter substitution, but it is no longer used in 3.1.0.rc1 (73ff679). Originally it should respect `prepared_statements` rather than `supports_statement_cache?` (fd39847). One more thing, named `supports_statement_cache?` is pretty misreading. We have `StatementCache` and `StatementPool`. However, `supports_statement_cache?` doesn't mean `StatementCache`, but `StatementPool` unlike its name. https://github.com/rails/rails/blob/v5.1.0/activerecord/lib/active_record/statement_cache.rb https://github.com/rails/rails/blob/v5.1.0/activerecord/lib/active_record/connection_adapters/statement_pool.rb --- activerecord/CHANGELOG.md | 4 ++++ .../connection_adapters/abstract/database_statements.rb | 5 +++-- .../active_record/connection_adapters/abstract_mysql_adapter.rb | 6 ------ .../lib/active_record/connection_adapters/postgresql_adapter.rb | 6 ------ .../lib/active_record/connection_adapters/sqlite3_adapter.rb | 6 ------ activerecord/test/cases/bind_parameter_test.rb | 7 +++++-- 6 files changed, 12 insertions(+), 22 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 68f0638e32..37d1a173e0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Deprecate `supports_statement_cache?`. + + *Ryuta Kamizono* + * Quote database name in `db:create` grant statement (when database user does not have access to create the database). *Rune Philosof* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 769f488469..c6811a4802 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -137,9 +137,10 @@ module ActiveRecord # Returns +true+ when the connection adapter supports prepared statement # caching, otherwise returns +false+ - def supports_statement_cache? - false + def supports_statement_cache? # :nodoc: + true end + deprecate :supports_statement_cache? # Runs the given block in a database transaction, and returns the result # of the block. 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 c742799aab..31cf2b4dbf 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -76,12 +76,6 @@ module ActiveRecord true end - # Returns true, since this connection adapter supports prepared statement - # caching. - def supports_statement_cache? - true - end - def supports_index_sort_order? !mariadb? && version >= "8.0.1" end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 033d81916e..f74f966fd9 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -121,12 +121,6 @@ module ActiveRecord include PostgreSQL::DatabaseStatements include PostgreSQL::ColumnDumper - # Returns true, since this connection adapter supports prepared statement - # caching. - def supports_statement_cache? - true - end - def supports_index_sort_order? true end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index e2c05ccc4e..7233325d5a 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -105,12 +105,6 @@ module ActiveRecord sqlite_version >= "3.8.0" end - # Returns true, since this connection adapter supports prepared statement - # caching. - def supports_statement_cache? - true - end - def requires_reloading? true end diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 6032aa9250..49fe219563 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -3,8 +3,7 @@ require "models/topic" require "models/author" require "models/post" -if ActiveRecord::Base.connection.supports_statement_cache? && - ActiveRecord::Base.connection.prepared_statements +if ActiveRecord::Base.connection.prepared_statements module ActiveRecord class BindParameterTest < ActiveRecord::TestCase fixtures :topics, :authors, :author_addresses, :posts @@ -66,6 +65,10 @@ if ActiveRecord::Base.connection.supports_statement_cache? && assert_logs_binds(binds) end + def test_deprecate_supports_statement_cache + assert_deprecated { ActiveRecord::Base.connection.supports_statement_cache? } + end + private def assert_logs_binds(binds) payload = { -- cgit v1.2.3 From 8d64cd86db1bc5e1ad119d13eb14f94726a0765f Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Tue, 2 May 2017 15:37:06 -0400 Subject: Add type caster to `RuntimeReflection#alias_name` Since we have been using this `Arel::Table` since 111ccc832bc977b15af12c14e7ca078dad2d4373, in order to properly handle queries, it's important that we properly type cast arguments. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/reflection.rb | 2 +- activerecord/test/cases/enum_test.rb | 5 ++++- activerecord/test/models/author.rb | 1 + activerecord/test/models/book.rb | 2 +- 5 files changed, 13 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 37d1a173e0..2e059805a2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Add type caster to `RuntimeReflection#alias_name` + + Fixes #28959. + + *Jon Moss* + * Deprecate `supports_statement_cache?`. *Ryuta Kamizono* diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 5ce134d6b7..1a9e0a4a40 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1105,7 +1105,7 @@ module ActiveRecord end def alias_name - Arel::Table.new(table_name) + Arel::Table.new(table_name, type_caster: klass.type_caster) end def all_includes; yield; end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index b7641fcf32..7f63bb2473 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -1,10 +1,12 @@ require "cases/helper" +require "models/author" require "models/book" class EnumTest < ActiveRecord::TestCase - fixtures :books + fixtures :books, :authors setup do + @author = authors(:david) @book = books(:awdr) end @@ -55,6 +57,7 @@ class EnumTest < ActiveRecord::TestCase assert_not_equal @book, Book.where(status: :written).first assert_equal @book, Book.where(status: [:published]).first assert_not_equal @book, Book.where(status: [:written]).first + assert_not @author.unpublished_books.include?(@book) assert_not_equal @book, Book.where.not(status: :published).first assert_equal @book, Book.where.not(status: :written).first end diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index fab613afd1..2d9cba77e0 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -106,6 +106,7 @@ class Author < ActiveRecord::Base has_many :tags_with_primary_key, through: :posts has_many :books + has_many :unpublished_books, -> { where(status: [:proposed, :written]) }, class_name: "Book" has_many :subscriptions, through: :books has_many :subscribers, -> { order("subscribers.nick") }, through: :subscriptions has_many :distinct_subscribers, -> { select("DISTINCT subscribers.*").order("subscribers.nick") }, through: :subscriptions, source: :subscriber diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb index 17bf3fbcb4..5f8a8a96dd 100644 --- a/activerecord/test/models/book.rb +++ b/activerecord/test/models/book.rb @@ -1,5 +1,5 @@ class Book < ActiveRecord::Base - has_many :authors + belongs_to :author has_many :citations, foreign_key: "book1_id" has_many :references, -> { distinct }, through: :citations, source: :reference_of -- cgit v1.2.3 From b6ad4052d18e4b29b8a092526c2beef013e2bf4f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 4 May 2017 03:11:33 +0900 Subject: Extract `bind_param` and `bind_attribute` into `ActiveRecord::TestCase` These are used in tests from anywhere. --- activerecord/test/cases/adapter_test.rb | 2 +- .../cases/adapters/postgresql/connection_test.rb | 4 ++-- .../adapters/postgresql/postgresql_adapter_test.rb | 8 +++---- .../postgresql/schema_authorization_test.rb | 6 +---- .../test/cases/adapters/postgresql/schema_test.rb | 10 +++------ .../cases/adapters/sqlite3/sqlite3_adapter_test.rb | 10 ++++----- activerecord/test/cases/bind_parameter_test.rb | 7 +++--- activerecord/test/cases/explain_test.rb | 6 +---- .../test/cases/relation/where_chain_test.rb | 2 +- .../test/cases/relation/where_clause_test.rb | 26 ++++++++-------------- activerecord/test/cases/test_case.rb | 8 +++++++ 11 files changed, 38 insertions(+), 51 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 601d575c0e..a1fb6427f9 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -220,7 +220,7 @@ module ActiveRecord def test_select_all_with_legacy_binds post = Post.create!(title: "foo", body: "bar") expected = @connection.select_all("SELECT * FROM posts WHERE id = #{post.id}") - result = @connection.select_all("SELECT * FROM posts WHERE id = #{Arel::Nodes::BindParam.new.to_sql}", nil, [[nil, post.id]]) + result = @connection.select_all("SELECT * FROM posts WHERE id = #{bind_param.to_sql}", nil, [[nil, post.id]]) assert_equal expected.to_hash, result.to_hash end end diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index c52d9e37cc..3deb007513 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -127,8 +127,8 @@ module ActiveRecord if ActiveRecord::Base.connection.prepared_statements def test_statement_key_is_logged - bind = Relation::QueryAttribute.new(nil, 1, Type::Value.new) - @connection.exec_query("SELECT $1::integer", "SQL", [bind], prepare: true) + binds = [bind_attribute(nil, 1)] + @connection.exec_query("SELECT $1::integer", "SQL", binds, prepare: true) name = @subscriber.payloads.last[:statement_name] assert name res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(1)") diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 003e6e62e7..434129ba73 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -202,8 +202,8 @@ module ActiveRecord string = @connection.quote("foo") @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - bind = Relation::QueryAttribute.new("id", 1, Type::Value.new) - result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, [bind]) + binds = [bind_attribute("id", 1)] + result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, binds) assert_equal 1, result.rows.length assert_equal 2, result.columns.length @@ -217,8 +217,8 @@ module ActiveRecord string = @connection.quote("foo") @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - bind = Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new) - result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, [bind]) + binds = [bind_attribute("id", "1-fuu", Type::Integer.new)] + result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, binds) assert_equal 1, result.rows.length assert_equal 2, result.columns.length diff --git a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb index f6a07da85f..bf570176f4 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb @@ -68,7 +68,7 @@ class SchemaAuthorizationTest < ActiveRecord::PostgreSQLTestCase USERS.each do |u| @connection.clear_cache! set_session_auth u - assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = $1", "SQL", [bind_param(1)]) + assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = $1", "SQL", [bind_attribute("id", 1)]) set_session_auth end end @@ -112,8 +112,4 @@ class SchemaAuthorizationTest < ActiveRecord::PostgreSQLTestCase def set_session_auth(auth = nil) @connection.session_auth = auth || "default" end - - def bind_param(value) - ActiveRecord::Relation::QueryAttribute.new(nil, value, ActiveRecord::Type::Value.new) - end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 75e30e4fe9..f6b957476b 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -169,17 +169,17 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase def test_raise_wrapped_exception_on_bad_prepare assert_raises(ActiveRecord::StatementInvalid) do - @connection.exec_query "select * from developers where id = ?", "sql", [bind_param(1)] + @connection.exec_query "select * from developers where id = ?", "sql", [bind_attribute("id", 1)] end end if ActiveRecord::Base.connection.prepared_statements def test_schema_change_with_prepared_stmt altered = false - @connection.exec_query "select * from developers where id = $1", "sql", [bind_param(1)] + @connection.exec_query "select * from developers where id = $1", "sql", [bind_attribute("id", 1)] @connection.exec_query "alter table developers add column zomg int", "sql", [] altered = true - @connection.exec_query "select * from developers where id = $1", "sql", [bind_param(1)] + @connection.exec_query "select * from developers where id = $1", "sql", [bind_attribute("id", 1)] ensure # We are not using DROP COLUMN IF EXISTS because that syntax is only # supported by pg 9.X @@ -467,10 +467,6 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase assert_equal this_index_column, this_index.columns[0] assert_equal this_index_name, this_index.name end - - def bind_param(value) - ActiveRecord::Relation::QueryAttribute.new(nil, value, ActiveRecord::Type::Value.new) - end end class SchemaForeignKeyTest < ActiveRecord::PostgreSQLTestCase diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 2179d1294c..9a812e325e 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -66,11 +66,11 @@ module ActiveRecord def test_exec_insert with_example_table do - vals = [Relation::QueryAttribute.new("number", 10, Type::Value.new)] - @conn.exec_insert("insert into ex (number) VALUES (?)", "SQL", vals) + binds = [bind_attribute("number", 10)] + @conn.exec_insert("insert into ex (number) VALUES (?)", "SQL", binds) result = @conn.exec_query( - "select number from ex where number = ?", "SQL", vals) + "select number from ex where number = ?", "SQL", binds) assert_equal 1, result.rows.length assert_equal 10, result.rows.first.first @@ -134,7 +134,7 @@ module ActiveRecord with_example_table "id int, data string" do @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') result = @conn.exec_query( - "SELECT id, data FROM ex WHERE id = ?", nil, [Relation::QueryAttribute.new(nil, 1, Type::Value.new)]) + "SELECT id, data FROM ex WHERE id = ?", nil, [bind_attribute("id", 1)]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length @@ -148,7 +148,7 @@ module ActiveRecord @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') result = @conn.exec_query( - "SELECT id, data FROM ex WHERE id = ?", nil, [Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new)]) + "SELECT id, data FROM ex WHERE id = ?", nil, [bind_attribute("id", "1-fuu", Type::Integer.new)]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 49fe219563..5af44c27eb 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -39,9 +39,8 @@ if ActiveRecord::Base.connection.prepared_statements end def test_binds_are_logged - sub = Arel::Nodes::BindParam.new - binds = [Relation::QueryAttribute.new("id", 1, Type::Value.new)] - sql = "select * from topics where id = #{sub.to_sql}" + binds = [bind_attribute("id", 1)] + sql = "select * from topics where id = #{bind_param.to_sql}" @connection.exec_query(sql, "SQL", binds) @@ -56,7 +55,7 @@ if ActiveRecord::Base.connection.prepared_statements end def test_logs_binds_after_type_cast - binds = [Relation::QueryAttribute.new("id", "10", Type::Integer.new)] + binds = [bind_attribute("id", "10", Type::Integer.new)] assert_logs_binds(binds) end diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index 86fe90ae51..4f6bd9327c 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -47,7 +47,7 @@ if ActiveRecord::Base.connection.supports_explain? def test_exec_explain_with_binds sqls = %w(foo bar) - binds = [[bind_param("wadus", 1)], [bind_param("chaflan", 2)]] + binds = [[bind_attribute("wadus", 1)], [bind_attribute("chaflan", 2)]] queries = sqls.zip(binds) stub_explain_for_query_plans(["query plan foo\n", "query plan bar\n"]) do @@ -79,9 +79,5 @@ if ActiveRecord::Base.connection.supports_explain? yield end end - - def bind_param(name, value) - ActiveRecord::Relation::QueryAttribute.new(name, value, ActiveRecord::Type::Value.new) - end end end diff --git a/activerecord/test/cases/relation/where_chain_test.rb b/activerecord/test/cases/relation/where_chain_test.rb index a96d1ae5b5..86e150ed79 100644 --- a/activerecord/test/cases/relation/where_chain_test.rb +++ b/activerecord/test/cases/relation/where_chain_test.rb @@ -25,7 +25,7 @@ module ActiveRecord end def test_association_not_eq - expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new)) + expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(bind_param)) relation = Post.joins(:comments).where.not(comments: { title: "hello" }) assert_equal(expected.to_sql, relation.where_clause.ast.to_sql) end diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb index d8e4c304f0..f8eb0dee91 100644 --- a/activerecord/test/cases/relation/where_clause_test.rb +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -47,15 +47,15 @@ class ActiveRecord::Relation test "merge removes bind parameters matching overlapping equality clauses" do a = WhereClause.new( [table["id"].eq(bind_param), table["name"].eq(bind_param)], - [attribute("id", 1), attribute("name", "Sean")], + [bind_attribute("id", 1), bind_attribute("name", "Sean")], ) b = WhereClause.new( [table["name"].eq(bind_param)], - [attribute("name", "Jim")] + [bind_attribute("name", "Jim")] ) expected = WhereClause.new( [table["id"].eq(bind_param), table["name"].eq(bind_param)], - [attribute("id", 1), attribute("name", "Jim")], + [bind_attribute("id", 1), bind_attribute("name", "Jim")], ) assert_equal expected, a.merge(b) @@ -103,10 +103,10 @@ class ActiveRecord::Relation table["name"].eq(bind_param), table["age"].gteq(bind_param), ], [ - attribute("name", "Sean"), - attribute("age", 30), + bind_attribute("name", "Sean"), + bind_attribute("age", 30), ]) - expected = WhereClause.new([table["age"].gteq(bind_param)], [attribute("age", 30)]) + expected = WhereClause.new([table["age"].gteq(bind_param)], [bind_attribute("age", 30)]) assert_equal expected, where_clause.except("id", "name") end @@ -146,8 +146,8 @@ class ActiveRecord::Relation end test "or joins the two clauses using OR" do - where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)]) - other_clause = WhereClause.new([table["name"].eq(bind_param)], [attribute("name", "Sean")]) + where_clause = WhereClause.new([table["id"].eq(bind_param)], [bind_attribute("id", 1)]) + other_clause = WhereClause.new([table["name"].eq(bind_param)], [bind_attribute("name", "Sean")]) expected_ast = Arel::Nodes::Grouping.new( Arel::Nodes::Or.new(table["id"].eq(bind_param), table["name"].eq(bind_param)) @@ -159,7 +159,7 @@ class ActiveRecord::Relation end test "or returns an empty where clause when either side is empty" do - where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)]) + where_clause = WhereClause.new([table["id"].eq(bind_param)], [bind_attribute("id", 1)]) assert_equal WhereClause.empty, where_clause.or(WhereClause.empty) assert_equal WhereClause.empty, WhereClause.empty.or(where_clause) @@ -170,13 +170,5 @@ class ActiveRecord::Relation def table Arel::Table.new("table") end - - def bind_param - Arel::Nodes::BindParam.new - end - - def attribute(name, value) - ActiveRecord::Attribute.with_cast_value(name, value, ActiveRecord::Type::Value.new) - end end end diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 31b11c19f7..9f594fef85 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -75,6 +75,14 @@ module ActiveRecord model.reset_column_information model.column_names.include?(column_name.to_s) end + + def bind_param + Arel::Nodes::BindParam.new + end + + def bind_attribute(name, value, type = ActiveRecord::Type.default_value) + ActiveRecord::Relation::QueryAttribute.new(name, value, type) + end end class PostgreSQLTestCase < TestCase -- cgit v1.2.3 From 0461f5c1e80dea174b9253c11462429f4531e716 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 4 May 2017 03:34:06 +0900 Subject: Remove duplicated `test_merging_reorders_bind_params` The same test exists in `test/cases/relation/merging_test.rb`. https://github.com/rails/rails/blob/v5.1.0/activerecord/test/cases/relation/merging_test.rb#L94-L101 --- activerecord/test/cases/relations_test.rb | 9 --------- 1 file changed, 9 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 7a710f1004..e72e9f62ef 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -2007,15 +2007,6 @@ class RelationTest < ActiveRecord::TestCase assert_equal binds, merged.bound_attributes end - def test_merging_reorders_bind_params - post = Post.first - right = Post.where(id: post.id) - left = Post.where(title: post.title) - - merged = left.merge(right) - assert_equal post, merged.first - end - def test_relation_join_method assert_equal "Thank you for the welcome,Thank you again for the welcome", Post.first.comments.join(",") end -- cgit v1.2.3 From a568a820d23543defe3c8a1e28c9aac97ca9daae Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 4 May 2017 05:14:02 +0900 Subject: Should test against `Relation#bound_attributes` Since legacy `Relation#bind_values` was removed in b06f64c. --- activerecord/test/cases/relations_test.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index e72e9f62ef..81173945a3 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1981,24 +1981,28 @@ class RelationTest < ActiveRecord::TestCase end def test_unscope_removes_binds - left = Post.where(id: Arel::Nodes::BindParam.new) - column = Post.columns_hash["id"] - left.bind_values += [[column, 20]] + left = Post.where(id: 20) + + binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))] + assert_equal binds, left.bound_attributes relation = left.unscope(where: :id) - assert_equal [], relation.bind_values + assert_equal [], relation.bound_attributes end - def test_merging_removes_rhs_bind_parameters + def test_merging_removes_rhs_binds left = Post.where(id: 20) right = Post.where(id: [1, 2, 3, 4]) + binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))] + assert_equal binds, left.bound_attributes + merged = left.merge(right) - assert_equal [], merged.bind_values + assert_equal [], merged.bound_attributes end - def test_merging_keeps_lhs_bind_parameters - binds = [ActiveRecord::Relation::QueryAttribute.new("id", 20, Post.type_for_attribute("id"))] + def test_merging_keeps_lhs_binds + binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))] right = Post.where(id: 20) left = Post.where(id: 10) -- cgit v1.2.3 From 38a7d189d285a5954946780fdd7fc42e824beb99 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 4 May 2017 20:33:15 +0900 Subject: Remove useless `target_records_from_association` Since through association is always loaded by `preloader.preload`. --- .../associations/preloader/through_association.rb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 9d44a02021..d53d3f777b 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -16,15 +16,13 @@ module ActiveRecord through_scope) through_records = owners.map do |owner| - association = owner.association through_reflection.name - - center = target_records_from_association(association) + center = owner.association(through_reflection.name).target [owner, Array(center)] end reset_association owners, through_reflection.name - middle_records = through_records.flat_map { |(_, rec)| rec } + middle_records = through_records.flat_map(&:last) preloaders = preloader.preload(middle_records, source_reflection.name, @@ -43,9 +41,7 @@ module ActiveRecord records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| rhs_records = middles.flat_map { |r| - association = r.association source_reflection.name - - target_records_from_association(association) + r.association(source_reflection.name).target }.compact # Respect the order on `reflection_scope` if it exists, else use the natural order. @@ -98,10 +94,6 @@ module ActiveRecord scope end - - def target_records_from_association(association) - association.loaded? ? association.target : association.reader - end end end end -- cgit v1.2.3 From 626f3d608868afed3aa725e3b6e0a5ce82b5e968 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 4 May 2017 17:14:22 +0900 Subject: Don't pass `arel.engine` to `Arel::SelectManager.new` The argument of `Arel::SelectManager.new` is `table`, not `engine`. https://github.com/rails/arel/blob/v8.0.0/lib/arel/select_manager.rb#L10 --- activerecord/lib/active_record/collection_cache_key.rb | 2 +- .../connection_adapters/abstract_mysql_adapter.rb | 10 ++++------ activerecord/lib/active_record/relation/calculations.rb | 3 +-- 3 files changed, 6 insertions(+), 9 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 1e1de1863a..8b937b6703 100644 --- a/activerecord/lib/active_record/collection_cache_key.rb +++ b/activerecord/lib/active_record/collection_cache_key.rb @@ -20,7 +20,7 @@ module ActiveRecord subquery_alias = "subquery_for_cache_key" subquery_column = "#{subquery_alias}.#{timestamp_column}" subquery = query.arel.as(subquery_alias) - arel = Arel::SelectManager.new(query.engine).project(select_values % subquery_column).from(subquery) + arel = Arel::SelectManager.new(subquery).project(select_values % subquery_column) else query = collection.unscope(:order) query.select_values = [select_values % column] 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 31cf2b4dbf..abc15f595f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -714,16 +714,14 @@ module ActiveRecord # MySQL is too stupid to create a temporary table for use subquery, so we have # to give it some prompting in the form of a subsubquery. Ugh! def subquery_for(key, select) - subsubselect = select.clone - subsubselect.projections = [key] + subselect = select.clone + subselect.projections = [key] # Materialize subquery by adding distinct # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on' - subsubselect.distinct unless select.limit || select.offset || select.orders.any? + subselect.distinct unless select.limit || select.offset || select.orders.any? - subselect = Arel::SelectManager.new(select.engine) - subselect.project Arel.sql(key.name) - subselect.from subsubselect.as("__active_record_temp") + Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key.name)) end def supports_rename_index? diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 9cabd1af13..861b2125d4 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -368,9 +368,8 @@ module ActiveRecord relation.select_values = [aliased_column] subquery = relation.arel.as(subquery_alias) - sm = Arel::SelectManager.new relation.engine select_value = operation_over_aggregate_column(column_alias, "count", distinct) - sm.project(select_value).from(subquery) + Arel::SelectManager.new(subquery).project(select_value) end end end -- cgit v1.2.3 From ae73637e35db20d0c4e14959d8c1274e5399278e Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 5 May 2017 18:07:12 +0930 Subject: Clarify deprecation message for #quoted_id In this case, it's the method definition that's more at fault, rather than the current caller. --- .../active_record/connection_adapters/abstract/quoting.rb | 9 ++++++++- activerecord/test/cases/quoting_test.rb | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index f0c0fbab6c..61233dcc51 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -10,8 +10,15 @@ module ActiveRecord value = id_value_for_database(value) if value.is_a?(Base) if value.respond_to?(:quoted_id) + at = value.method(:quoted_id).source_location + at &&= " at %s:%d" % at + + owner = value.method(:quoted_id).owner.to_s + klass = value.class.to_s + klass += "(#{owner})" unless owner == klass + ActiveSupport::Deprecation.warn \ - "Using #quoted_id is deprecated and will be removed in Rails 5.2." + "Defining #quoted_id is deprecated and will be ignored in Rails 5.2. (defined on #{klass}#{at})" return value.quoted_id end diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index f260d043e4..f647526fbc 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -81,8 +81,21 @@ module ActiveRecord end end + class QuotedOne + def quoted_id + 1 + end + end + class SubQuotedOne < QuotedOne + end def test_quote_with_quoted_id - assert_deprecated { assert_equal 1, @quoter.quote(Struct.new(:quoted_id).new(1)) } + assert_deprecated /defined on \S+::QuotedOne at .*quoting_test\.rb:[0-9]/ do + assert_equal 1, @quoter.quote(QuotedOne.new) + end + + assert_deprecated /defined on \S+::SubQuotedOne\(\S+::QuotedOne\) at .*quoting_test\.rb:[0-9]/ do + assert_equal 1, @quoter.quote(SubQuotedOne.new) + end end def test_quote_nil -- cgit v1.2.3 From a925295658457571d3474cabf1117f4aa85a1869 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 6 May 2017 04:52:59 +0900 Subject: Remove unused `DelegationTest#call_method` `DelegationTest#call_method` is no longer used since 9d79334a. --- .../test/cases/relation/delegation_test.rb | 34 +++++----------------- 1 file changed, 7 insertions(+), 27 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb index 8cb7b82015..cb6e4d76d3 100644 --- a/activerecord/test/cases/relation/delegation_test.rb +++ b/activerecord/test/cases/relation/delegation_test.rb @@ -3,29 +3,7 @@ require "models/post" require "models/comment" module ActiveRecord - class DelegationTest < ActiveRecord::TestCase - fixtures :posts - - def call_method(target, method) - method_arity = target.to_a.method(method).arity - - if method_arity.zero? - target.public_send(method) - elsif method_arity < 0 - if method == :shuffle! - target.public_send(method) - else - target.public_send(method, 1) - end - elsif method_arity == 1 - target.public_send(method, 1) - else - raise NotImplementedError - end - end - end - - module DelegationWhitelistBlacklistTests + module DelegationWhitelistTests ARRAY_DELEGATES = [ :+, :-, :|, :&, :[], :shuffle, :all?, :collect, :compact, :detect, :each, :each_cons, :each_with_index, @@ -43,16 +21,18 @@ module ActiveRecord end end - class DelegationAssociationTest < DelegationTest - include DelegationWhitelistBlacklistTests + class DelegationAssociationTest < ActiveRecord::TestCase + include DelegationWhitelistTests + + fixtures :posts def target Post.first.comments end end - class DelegationRelationTest < DelegationTest - include DelegationWhitelistBlacklistTests + class DelegationRelationTest < ActiveRecord::TestCase + include DelegationWhitelistTests fixtures :comments -- cgit v1.2.3 From 99b48a5aebc0ff159a8102f2782eaee8ab5a32cd Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 6 May 2017 21:23:58 +0900 Subject: Fix `warning: ambiguous first argument` ``` % ARCONN=sqlite3 be ruby -w -Itest test/cases/quoting_test.rb test/cases/quoting_test.rb:92: warning: ambiguous first argument; put parentheses or a space even after `/' operator test/cases/quoting_test.rb:96: warning: ambiguous first argument; put parentheses or a space even after `/' operator Using sqlite3 Run options: --seed 9495 ..................................... Finished in 0.046403s, 797.3622 runs/s, 1120.6172 assertions/s. 37 runs, 52 assertions, 0 failures, 0 errors, 0 skips ``` --- activerecord/test/cases/quoting_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index f647526fbc..0819776fbf 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -89,11 +89,11 @@ module ActiveRecord class SubQuotedOne < QuotedOne end def test_quote_with_quoted_id - assert_deprecated /defined on \S+::QuotedOne at .*quoting_test\.rb:[0-9]/ do + assert_deprecated(/defined on \S+::QuotedOne at .*quoting_test\.rb:[0-9]/) do assert_equal 1, @quoter.quote(QuotedOne.new) end - assert_deprecated /defined on \S+::SubQuotedOne\(\S+::QuotedOne\) at .*quoting_test\.rb:[0-9]/ do + assert_deprecated(/defined on \S+::SubQuotedOne\(\S+::QuotedOne\) at .*quoting_test\.rb:[0-9]/) do assert_equal 1, @quoter.quote(SubQuotedOne.new) end end -- cgit v1.2.3 From 2ee96632b640da0d09b164ed67b1a9f6b96594b3 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 6 May 2017 22:21:38 +0900 Subject: Delegate `ast` and `locked` to `arel` explicitly Currently `ast` and `locked` are used in the internal but delegating to `arel` is depend on `method_missing`. If a model class is defined these methods, `select_all` will be broken. It should be delegated to `arel` explicitly. --- activerecord/lib/active_record/relation.rb | 1 + activerecord/lib/active_record/relation/delegation.rb | 2 ++ activerecord/test/cases/relation/merging_test.rb | 2 +- activerecord/test/models/category.rb | 9 +++++++++ 4 files changed, 13 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 5775eda5a5..b61ac84155 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -18,6 +18,7 @@ module ActiveRecord attr_reader :table, :klass, :loaded, :predicate_builder alias :model :klass alias :loaded? :loaded + alias :locked? :locked def initialize(klass, table, predicate_builder, values = {}) @klass = klass diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 257ae04ff4..4f739a0415 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -46,6 +46,8 @@ module ActiveRecord delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, to: :klass + delegate :ast, :locked, to: :arel + module ClassSpecificRelation # :nodoc: extend ActiveSupport::Concern diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 64866eaf2d..c3b39a9295 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -56,7 +56,7 @@ class RelationMergingTest < ActiveRecord::TestCase def test_relation_merging_with_locks devs = Developer.lock.where("salary >= 80000").order("id DESC").merge(Developer.limit(2)) - assert devs.locked.present? + assert devs.locked? end def test_relation_merging_with_preload diff --git a/activerecord/test/models/category.rb b/activerecord/test/models/category.rb index e8654dca01..4b2840c653 100644 --- a/activerecord/test/models/category.rb +++ b/activerecord/test/models/category.rb @@ -29,6 +29,15 @@ class Category < ActiveRecord::Base has_many :authors_with_select, -> { select "authors.*, categorizations.post_id" }, through: :categorizations, source: :author scope :general, -> { where(name: "General") } + + # Should be delegated `ast` and `locked` to `arel`. + def self.ast + raise + end + + def self.locked + raise + end end class SpecialCategory < Category -- cgit v1.2.3 From b201474756a2ee493406ad0cb49f49c6873bdc28 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 7 May 2017 04:08:58 +0900 Subject: Should escape meta characters in regexp --- .../test/cases/adapters/mysql2/charset_collation_test.rb | 4 ++-- .../test/cases/adapters/mysql2/unsigned_type_test.rb | 8 ++++---- .../test/cases/adapters/postgresql/collation_test.rb | 4 ++-- .../test/cases/adapters/postgresql/explain_test.rb | 4 ++-- .../cases/adapters/postgresql/postgresql_adapter_test.rb | 2 +- .../test/cases/adapters/sqlite3/collation_test.rb | 4 ++-- activerecord/test/cases/adapters/sqlite3/explain_test.rb | 4 ++-- activerecord/test/cases/batches_test.rb | 4 ++-- activerecord/test/cases/inheritance_test.rb | 2 +- activerecord/test/cases/log_subscriber_test.rb | 16 ++++++++-------- 10 files changed, 26 insertions(+), 26 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb b/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb index 8826ad7fd1..e4a6ed5482 100644 --- a/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb +++ b/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb @@ -48,7 +48,7 @@ class Mysql2CharsetCollationTest < ActiveRecord::Mysql2TestCase test "schema dump includes collation" do output = dump_table_schema("charset_collations") - assert_match %r{t.string\s+"string_ascii_bin",\s+collation: "ascii_bin"$}, output - assert_match %r{t.text\s+"text_ucs2_unicode_ci",\s+collation: "ucs2_unicode_ci"$}, output + assert_match %r{t\.string\s+"string_ascii_bin",\s+collation: "ascii_bin"$}, output + assert_match %r{t\.text\s+"text_ucs2_unicode_ci",\s+collation: "ucs2_unicode_ci"$}, output end end diff --git a/activerecord/test/cases/adapters/mysql2/unsigned_type_test.rb b/activerecord/test/cases/adapters/mysql2/unsigned_type_test.rb index a0823be143..71dcfaa241 100644 --- a/activerecord/test/cases/adapters/mysql2/unsigned_type_test.rb +++ b/activerecord/test/cases/adapters/mysql2/unsigned_type_test.rb @@ -58,9 +58,9 @@ class Mysql2UnsignedTypeTest < ActiveRecord::Mysql2TestCase test "schema dump includes unsigned option" do schema = dump_table_schema "unsigned_types" - assert_match %r{t.integer\s+"unsigned_integer",\s+unsigned: true$}, schema - assert_match %r{t.bigint\s+"unsigned_bigint",\s+unsigned: true$}, schema - assert_match %r{t.float\s+"unsigned_float",\s+limit: 24,\s+unsigned: true$}, schema - assert_match %r{t.decimal\s+"unsigned_decimal",\s+precision: 10,\s+scale: 2,\s+unsigned: true$}, schema + assert_match %r{t\.integer\s+"unsigned_integer",\s+unsigned: true$}, schema + assert_match %r{t\.bigint\s+"unsigned_bigint",\s+unsigned: true$}, schema + assert_match %r{t\.float\s+"unsigned_float",\s+limit: 24,\s+unsigned: true$}, schema + assert_match %r{t\.decimal\s+"unsigned_decimal",\s+precision: 10,\s+scale: 2,\s+unsigned: true$}, schema end end diff --git a/activerecord/test/cases/adapters/postgresql/collation_test.rb b/activerecord/test/cases/adapters/postgresql/collation_test.rb index b39e298a5d..a603221d8f 100644 --- a/activerecord/test/cases/adapters/postgresql/collation_test.rb +++ b/activerecord/test/cases/adapters/postgresql/collation_test.rb @@ -47,7 +47,7 @@ class PostgresqlCollationTest < ActiveRecord::PostgreSQLTestCase test "schema dump includes collation" do output = dump_table_schema("postgresql_collations") - assert_match %r{t.string\s+"string_c",\s+collation: "C"$}, output - assert_match %r{t.text\s+"text_posix",\s+collation: "POSIX"$}, output + assert_match %r{t\.string\s+"string_c",\s+collation: "C"$}, output + assert_match %r{t\.text\s+"text_posix",\s+collation: "POSIX"$}, output end end diff --git a/activerecord/test/cases/adapters/postgresql/explain_test.rb b/activerecord/test/cases/adapters/postgresql/explain_test.rb index 7493bce4fb..d79fbccf47 100644 --- a/activerecord/test/cases/adapters/postgresql/explain_test.rb +++ b/activerecord/test/cases/adapters/postgresql/explain_test.rb @@ -7,14 +7,14 @@ class PostgreSQLExplainTest < ActiveRecord::PostgreSQLTestCase def test_explain_for_one_query explain = Developer.where(id: 1).explain - assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\$1 \[\["id", 1\]\]|1)), explain + assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain assert_match %(QUERY PLAN), explain end def test_explain_with_eager_loading explain = Developer.where(id: 1).includes(:audit_logs).explain assert_match %(QUERY PLAN), explain - assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\$1 \[\["id", 1\]\]|1)), explain + assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain end end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 434129ba73..bfc763e1ef 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -346,7 +346,7 @@ module ActiveRecord @connection.select_all "SELECT NULL::anyelement" @connection.select_all "SELECT NULL::anyelement" } - assert_match(/\Aunknown OID \d+: failed to recognize type of 'anyelement'. It will be treated as String.\n\z/, warning) + assert_match(/\Aunknown OID \d+: failed to recognize type of 'anyelement'\. It will be treated as String\.\n\z/, warning) ensure reset_connection end diff --git a/activerecord/test/cases/adapters/sqlite3/collation_test.rb b/activerecord/test/cases/adapters/sqlite3/collation_test.rb index 28e8f12c18..dd88ed3656 100644 --- a/activerecord/test/cases/adapters/sqlite3/collation_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/collation_test.rb @@ -47,7 +47,7 @@ class SQLite3CollationTest < ActiveRecord::SQLite3TestCase test "schema dump includes collation" do output = dump_table_schema("collation_table_sqlite3") - assert_match %r{t.string\s+"string_nocase",\s+collation: "NOCASE"$}, output - assert_match %r{t.text\s+"text_rtrim",\s+collation: "RTRIM"$}, output + assert_match %r{t\.string\s+"string_nocase",\s+collation: "NOCASE"$}, output + assert_match %r{t\.text\s+"text_rtrim",\s+collation: "RTRIM"$}, output end end diff --git a/activerecord/test/cases/adapters/sqlite3/explain_test.rb b/activerecord/test/cases/adapters/sqlite3/explain_test.rb index 128acb79cf..29d97ae78c 100644 --- a/activerecord/test/cases/adapters/sqlite3/explain_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/explain_test.rb @@ -7,13 +7,13 @@ class SQLite3ExplainTest < ActiveRecord::SQLite3TestCase def test_explain_for_one_query explain = Developer.where(id: 1).explain - assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\? \[\["id", 1\]\]|1)), explain + assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\? \[\["id", 1\]\]|1)), explain assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) end def test_explain_with_eager_loading explain = Developer.where(id: 1).includes(:audit_logs).explain - assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\? \[\["id", 1\]\]|1)), explain + assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\? \[\["id", 1\]\]|1)), explain assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain assert_match(/(SCAN )?TABLE audit_logs/, explain) diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 1a54c02fac..fbc3fbb44f 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -143,7 +143,7 @@ class EachTest < ActiveRecord::TestCase def test_find_in_batches_should_quote_batch_order c = Post.connection - assert_sql(/ORDER BY #{c.quote_table_name('posts')}.#{c.quote_column_name('id')}/) do + assert_sql(/ORDER BY #{c.quote_table_name('posts')}\.#{c.quote_column_name('id')}/) do Post.find_in_batches(batch_size: 1) do |batch| assert_kind_of Array, batch assert_kind_of Post, batch.first @@ -408,7 +408,7 @@ class EachTest < ActiveRecord::TestCase def test_in_batches_should_quote_batch_order c = Post.connection - assert_sql(/ORDER BY #{c.quote_table_name('posts')}.#{c.quote_column_name('id')}/) do + assert_sql(/ORDER BY #{c.quote_table_name('posts')}\.#{c.quote_column_name('id')}/) do Post.in_batches(of: 1) do |relation| assert_kind_of ActiveRecord::Relation, relation assert_kind_of Post, relation.first diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index e570e9ac1d..d70572d6eb 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -417,7 +417,7 @@ class InheritanceTest < ActiveRecord::TestCase def test_eager_load_belongs_to_primary_key_quoting con = Account.connection - assert_sql(/#{con.quote_table_name('companies')}.#{con.quote_column_name('id')} = 1/) do + assert_sql(/#{con.quote_table_name('companies')}\.#{con.quote_column_name('id')} = 1/) do Account.all.merge!(includes: :firm).find(1) end end diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 8c67aa8746..f426333aa5 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -97,7 +97,7 @@ class LogSubscriberTest < ActiveRecord::TestCase logger.colorize_logging = true SQL_COLORINGS.each do |verb, _| logger.sql(Event.new(0, sql: verb.to_s)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) logger.sql(Event.new(0, sql: verb.to_s, name: "SQL")) assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA}SQL \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) @@ -109,13 +109,13 @@ class LogSubscriberTest < ActiveRecord::TestCase logger.colorize_logging = true SQL_COLORINGS.each do |verb, _| logger.sql(Event.new(0, sql: verb.to_s, name: "Model Load")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Load \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Load \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) logger.sql(Event.new(0, sql: verb.to_s, name: "Model Exists")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Exists \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Exists \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) logger.sql(Event.new(0, sql: verb.to_s, name: "ANY SPECIFIC NAME")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}ANY SPECIFIC NAME \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}ANY SPECIFIC NAME \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) end end @@ -124,7 +124,7 @@ class LogSubscriberTest < ActiveRecord::TestCase logger.colorize_logging = true SQL_COLORINGS.slice(:SELECT, :INSERT, :UPDATE, :DELETE).each do |verb, color_regex| logger.sql(Event.new(0, sql: "#{verb} WHERE ID IN SELECT")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}#{verb} WHERE ID IN SELECT#{REGEXP_CLEAR}/i, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}#{verb} WHERE ID IN SELECT#{REGEXP_CLEAR}/i, logger.debugs.last) end end @@ -139,7 +139,7 @@ class LogSubscriberTest < ActiveRecord::TestCase ) EOS logger.sql(Event.new(0, sql: sql)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}.*#{verb}.*#{REGEXP_CLEAR}/mi, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}.*#{verb}.*#{REGEXP_CLEAR}/mi, logger.debugs.last) end end @@ -152,13 +152,13 @@ class LogSubscriberTest < ActiveRecord::TestCase WHERE col1 = 5; EOS logger.sql(Event.new(0, sql: sql)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*FOR UPDATE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*FOR UPDATE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) sql = <<-EOS LOCK TABLE films IN SHARE MODE; EOS logger.sql(Event.new(0, sql: sql)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*LOCK TABLE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*LOCK TABLE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) end def test_exists_query_logging -- cgit v1.2.3 From 76e34405abc072d778f716ce97991e91ad336030 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 3 May 2017 18:35:45 +0900 Subject: Refactor enum to use `value` instead of `label` in the scope --- activerecord/lib/active_record/enum.rb | 23 ++++++++++++----------- activerecord/test/cases/enum_test.rb | 4 ++-- activerecord/test/fixtures/books.yml | 1 + activerecord/test/models/book.rb | 2 +- 4 files changed, 16 insertions(+), 14 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 0ab03b2ab3..496abfc5d9 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -154,11 +154,12 @@ module ActiveRecord definitions.each do |name, values| # statuses = { } enum_values = ActiveSupport::HashWithIndifferentAccess.new - name = name.to_sym + name = name.to_s # def self.statuses() statuses end - detect_enum_conflict!(name, name.to_s.pluralize, true) - klass.singleton_class.send(:define_method, name.to_s.pluralize) { enum_values } + detect_enum_conflict!(name, name.pluralize, true) + singleton_class.send(:define_method, name.pluralize) { enum_values } + defined_enums[name] = enum_values detect_enum_conflict!(name, name) detect_enum_conflict!(name, "#{name}=") @@ -170,7 +171,7 @@ module ActiveRecord _enum_methods_module.module_eval do pairs = values.respond_to?(:each_pair) ? values.each_pair : values.each_with_index - pairs.each do |value, i| + pairs.each do |label, value| if enum_prefix == true prefix = "#{name}_" elsif enum_prefix @@ -182,23 +183,23 @@ module ActiveRecord suffix = "_#{enum_suffix}" end - value_method_name = "#{prefix}#{value}#{suffix}" - enum_values[value] = i + value_method_name = "#{prefix}#{label}#{suffix}" + enum_values[label] = value + label = label.to_s - # def active?() status == 0 end + # def active?() status == "active" end klass.send(:detect_enum_conflict!, name, "#{value_method_name}?") - define_method("#{value_method_name}?") { self[attr] == value.to_s } + define_method("#{value_method_name}?") { self[attr] == label } - # def active!() update! status: :active end + # def active!() update!(status: 0) end klass.send(:detect_enum_conflict!, name, "#{value_method_name}!") define_method("#{value_method_name}!") { update!(attr => value) } - # scope :active, -> { where status: 0 } + # scope :active, -> { where(status: 0) } klass.send(:detect_enum_conflict!, name, value_method_name, true) klass.scope value_method_name, -> { where(attr => value) } end end - defined_enums[name.to_s] = enum_values end end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 7f63bb2473..db3da53487 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -6,7 +6,6 @@ class EnumTest < ActiveRecord::TestCase fixtures :books, :authors setup do - @author = authors(:david) @book = books(:awdr) end @@ -39,6 +38,8 @@ class EnumTest < ActiveRecord::TestCase assert_equal @book, Book.author_visibility_visible.first assert_equal @book, Book.illustrator_visibility_visible.first assert_equal @book, Book.medium_to_read.first + assert_equal books(:ddd), Book.forgotten.first + assert_equal books(:rfr), authors(:david).unpublished_books.first end test "find via where with values" do @@ -57,7 +58,6 @@ class EnumTest < ActiveRecord::TestCase assert_not_equal @book, Book.where(status: :written).first assert_equal @book, Book.where(status: [:published]).first assert_not_equal @book, Book.where(status: [:written]).first - assert_not @author.unpublished_books.include?(@book) assert_not_equal @book, Book.where.not(status: :published).first assert_equal @book, Book.where.not(status: :written).first end diff --git a/activerecord/test/fixtures/books.yml b/activerecord/test/fixtures/books.yml index b3625ee72e..699623a6f9 100644 --- a/activerecord/test/fixtures/books.yml +++ b/activerecord/test/fixtures/books.yml @@ -25,6 +25,7 @@ ddd: name: "Domain-Driven Design" format: "hardcover" status: 2 + read_status: "forgotten" tlg: author_id: 1 diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb index 5f8a8a96dd..6466e1b341 100644 --- a/activerecord/test/models/book.rb +++ b/activerecord/test/models/book.rb @@ -8,7 +8,7 @@ class Book < ActiveRecord::Base has_many :subscribers, through: :subscriptions enum status: [:proposed, :written, :published] - enum read_status: { unread: 0, reading: 2, read: 3 } + enum read_status: { unread: 0, reading: 2, read: 3, forgotten: nil } enum nullable_status: [:single, :married] enum language: [:english, :spanish, :french], _prefix: :in enum author_visibility: [:visible, :invisible], _prefix: true -- cgit v1.2.3 From 0f5325a625e757d2e4374ce41cabad63f087c0b5 Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Fri, 28 Apr 2017 11:44:52 +0200 Subject: Fix ambigious error message of select query method --- activerecord/lib/active_record/relation/query_methods.rb | 2 +- 1 file changed, 1 insertion(+), 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 1178dec706..76e529f2de 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -248,7 +248,7 @@ module ActiveRecord return super() end - raise ArgumentError, "Call this with at least one field" if fields.empty? + raise ArgumentError, "Call `select' with at least one field" if fields.empty? spawn._select!(*fields) end -- cgit v1.2.3 From 7bebeaf0c1c359a115f3052b9488ab3838299f63 Mon Sep 17 00:00:00 2001 From: Mohit Natoo Date: Wed, 10 May 2017 16:04:13 +0700 Subject: [Foreign Key] Don't worry about the building identifier if name is already present. --- .../active_record/connection_adapters/abstract/schema_statements.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 13629dee7f..16a398f631 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1280,9 +1280,10 @@ module ActiveRecord end def foreign_key_name(table_name, options) - identifier = "#{table_name}_#{options.fetch(:column)}_fk" - hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10) options.fetch(:name) do + identifier = "#{table_name}_#{options.fetch(:column)}_fk" + hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10) + "fk_rails_#{hashed_identifier}" end end -- cgit v1.2.3 From 07a611e02351b32100f19495a34ed46d01f195c1 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 11 May 2017 12:44:05 +0900 Subject: Don't eager loading if unneeded for `FinderMethods#exists?` Fixes #29025. --- activerecord/lib/active_record/relation.rb | 3 +-- activerecord/lib/active_record/relation/finder_methods.rb | 6 ++++-- activerecord/test/cases/associations/eager_test.rb | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 5775eda5a5..333ad16e11 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -269,8 +269,7 @@ module ActiveRecord # Returns true if there are no records. def empty? return @records.empty? if loaded? - - limit_value == 0 || !exists? + !exists? end # Returns true if there are no records. diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index a1459c87c6..1d661fa8ed 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -307,9 +307,11 @@ module ActiveRecord MSG end - return false if !conditions + return false if !conditions || limit_value == 0 + + relation = self unless eager_loading? + relation ||= apply_join_dependency(self, construct_join_dependency(eager_loading: false)) - relation = apply_join_dependency(self, construct_join_dependency(eager_loading: false)) return false if ActiveRecord::NullRelation === relation relation = construct_relation_for_exists(relation, conditions) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 11f4aae5b3..36238dd088 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1363,6 +1363,7 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_nothing_raised do authors(:david).essays.includes(:writer).any? authors(:david).essays.includes(:writer).exists? + authors(:david).essays.includes(:owner).where("name IS NOT NULL").exists? end end -- cgit v1.2.3 From f73b845610931ddf03c756d804982776a3cc69a4 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Thu, 11 May 2017 17:40:41 +0900 Subject: Defer loading each DB Tasks class from AR DatabaseTasks Because we don't need to load tasks for DBs that we don't use for the current app. Also, these Tasks classes load AR::Base in their class level, and so immediately kick :active_record on_load hooks. This used to happen when we were loading tasks, not when we run a task. --- activerecord/lib/active_record/tasks/database_tasks.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 45110a79cd..c53dee43d7 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -71,9 +71,9 @@ module ActiveRecord @tasks[pattern] = task end - register_task(/mysql/, ActiveRecord::Tasks::MySQLDatabaseTasks) - register_task(/postgresql/, ActiveRecord::Tasks::PostgreSQLDatabaseTasks) - register_task(/sqlite/, ActiveRecord::Tasks::SQLiteDatabaseTasks) + register_task(/mysql/, 'ActiveRecord::Tasks::MySQLDatabaseTasks') + register_task(/postgresql/, 'ActiveRecord::Tasks::PostgreSQLDatabaseTasks') + register_task(/sqlite/, 'ActiveRecord::Tasks::SQLiteDatabaseTasks') def db_dir @db_dir ||= Rails.application.config.paths["db"].first @@ -288,11 +288,11 @@ module ActiveRecord private def class_for_adapter(adapter) - key = @tasks.keys.detect { |pattern| adapter[pattern] } - unless key + _key, task = @tasks.each_pair.detect { |pattern, _task| adapter[pattern] } + unless task raise DatabaseNotSupported, "Rake tasks not supported by '#{adapter}' adapter" end - @tasks[key] + task.is_a?(String) ? task.constantize : task end def each_current_configuration(environment) -- cgit v1.2.3 From 0584e21ac03a7abba100a8820aeab8de32facc2d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 14 May 2017 02:43:02 +0900 Subject: Remove returning true in internal callbacks `display_deprecation_warning_for_false_terminator` was removed since 3a25cdc. --- activerecord/lib/active_record/attribute_methods.rb | 1 - activerecord/lib/active_record/autosave_association.rb | 9 +-------- 2 files changed, 1 insertion(+), 9 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index ebe06566cc..83c61fad19 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -62,7 +62,6 @@ module ActiveRecord super(attribute_names) @attribute_methods_generated = true end - true end def undefine_attribute_methods # :nodoc: diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 607c54e481..f2391ba715 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -216,13 +216,7 @@ module ActiveRecord method = :validate_single_association end - define_non_cyclic_method(validation_method) do - send(method, reflection) - # TODO: remove the following line as soon as the return value of - # callbacks is ignored, that is, returning `false` does not - # display a deprecation warning or halts the callback chain. - true - end + define_non_cyclic_method(validation_method) { send(method, reflection) } validate validation_method after_validation :_ensure_no_duplicate_errors end @@ -369,7 +363,6 @@ module ActiveRecord # association whether or not the parent was a new record before saving. def before_save_collection_association @new_record_before_save = new_record? - true end def after_save_collection_association -- cgit v1.2.3 From be1dd45200b1f21aec5b151d78ef402df9fa28e4 Mon Sep 17 00:00:00 2001 From: Rusty Geldmacher Date: Fri, 12 May 2017 11:40:31 -0400 Subject: Respect `ignore_tables` in Postgres structure dump When using `sql` as the schema format, or even just doing `rake db:structure:dump`, it would be good to respect the list of ignored tables that has been configured. --- activerecord/lib/active_record/tasks/postgresql_database_tasks.rb | 6 ++++++ activerecord/test/cases/tasks/postgresql_rake_test.rb | 8 ++++++++ 2 files changed, 14 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index f1af90c1e8..7f1a768d8b 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -66,6 +66,12 @@ module ActiveRecord "--schema=#{part.strip}" end end + + ignore_tables = ActiveRecord::SchemaDumper.ignore_tables + if ignore_tables.any? + args += ignore_tables.flat_map { |table| ["-T", table] } + end + args << configuration["database"] run_cmd("pg_dump", args, "dumping") remove_sql_header_comments(filename) diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index 512388af6b..a2e968aedf 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -259,6 +259,14 @@ if current_adapter?(:PostgreSQLAdapter) end end + def test_structure_dump_with_ignore_tables + ActiveRecord::SchemaDumper.expects(:ignore_tables).returns(["foo", "bar"]) + + Kernel.expects(:system).with("pg_dump", "-s", "-x", "-O", "-f", @filename, "-T", "foo", "-T", "bar", "my-app-db").returns(true) + + ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) + end + def test_structure_dump_with_schema_search_path @configuration["schema_search_path"] = "foo,bar" -- cgit v1.2.3 From 486562fa955f2cbd574e7f4099fdf69a49a8ce20 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sat, 13 May 2017 21:35:22 -0500 Subject: Respect 'ignore_tables' in MySQL structure dump --- .../lib/active_record/tasks/mysql_database_tasks.rb | 6 ++++++ activerecord/test/cases/tasks/mysql_rake_test.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index c05f0a8fbb..541165b3d1 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -60,6 +60,12 @@ module ActiveRecord args.concat(["--routines"]) args.concat(["--skip-comments"]) args.concat(Array(extra_flags)) if extra_flags + + ignore_tables = ActiveRecord::SchemaDumper.ignore_tables + if ignore_tables.any? + args += ignore_tables.map { |table| "--ignore-table=#{configuration['database']}.#{table}" } + end + args.concat(["#{configuration['database']}"]) run_cmd("mysqldump", args, "dumping") diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index b85d303a91..33da3d11fc 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -294,6 +294,13 @@ if current_adapter?(:Mysql2Adapter) ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) end + def test_structure_dump + filename = "awesome-file.sql" + Kernel.expects(:system).with("mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "test-db").returns(true) + + ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) + end + def test_structure_dump_with_extra_flags filename = "awesome-file.sql" expected_command = ["mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "--noop", "test-db"] @@ -305,6 +312,15 @@ if current_adapter?(:Mysql2Adapter) end end + def test_structure_dump_with_ignore_tables + filename = "awesome-file.sql" + ActiveRecord::SchemaDumper.expects(:ignore_tables).returns(["foo", "bar"]) + + Kernel.expects(:system).with("mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "--ignore-table=test-db.foo", "--ignore-table=test-db.bar", "test-db").returns(true) + + ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) + end + def test_warn_when_external_structure_dump_command_execution_fails filename = "awesome-file.sql" Kernel.expects(:system) -- cgit v1.2.3 From 4b49ab66425283f4b79907873b72236c7ebec0be Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sat, 13 May 2017 23:01:13 -0500 Subject: Respect 'ignore_tables' in SQLite structure dump --- .../lib/active_record/tasks/sqlite_database_tasks.rb | 11 ++++++++++- activerecord/test/cases/tasks/sqlite_rake_test.rb | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb index 1f756c2979..5542e5585c 100644 --- a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb @@ -38,7 +38,16 @@ module ActiveRecord def structure_dump(filename, extra_flags) dbfile = configuration["database"] flags = extra_flags.join(" ") if extra_flags - `sqlite3 #{flags} #{dbfile} .schema > #{filename}` + + ignore_tables = ActiveRecord::SchemaDumper.ignore_tables + if ignore_tables.any? + tables = `sqlite3 #{dbfile} .tables`.split - ignore_tables + condition = tables.map { |table| "tbl_name = '#{table}'" }.join(" OR ") + statement = "SELECT sql FROM sqlite_master WHERE #{condition} ORDER BY tbl_name, type DESC, name" + `sqlite3 #{flags} #{dbfile} "#{statement}" > #{filename}` + else + `sqlite3 #{flags} #{dbfile} .schema > #{filename}` + end end def structure_load(filename, extra_flags) diff --git a/activerecord/test/cases/tasks/sqlite_rake_test.rb b/activerecord/test/cases/tasks/sqlite_rake_test.rb index 0d917f3f6c..ccb3834fee 100644 --- a/activerecord/test/cases/tasks/sqlite_rake_test.rb +++ b/activerecord/test/cases/tasks/sqlite_rake_test.rb @@ -180,6 +180,9 @@ if current_adapter?(:SQLite3Adapter) "adapter" => "sqlite3", "database" => @database } + + `sqlite3 #{@database} 'CREATE TABLE bar(id INTEGER)'` + `sqlite3 #{@database} 'CREATE TABLE foo(id INTEGER)'` end def test_structure_dump @@ -189,6 +192,23 @@ if current_adapter?(:SQLite3Adapter) ActiveRecord::Tasks::DatabaseTasks.structure_dump @configuration, filename, "/rails/root" assert File.exist?(dbfile) assert File.exist?(filename) + assert_match(/CREATE TABLE foo/, File.read(filename)) + assert_match(/CREATE TABLE bar/, File.read(filename)) + ensure + FileUtils.rm_f(filename) + FileUtils.rm_f(dbfile) + end + + def test_structure_dump_with_ignore_tables + dbfile = @database + filename = "awesome-file.sql" + ActiveRecord::SchemaDumper.expects(:ignore_tables).returns(["foo"]) + + ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename, "/rails/root") + assert File.exist?(dbfile) + assert File.exist?(filename) + assert_match(/bar/, File.read(filename)) + assert_no_match(/foo/, File.read(filename)) ensure FileUtils.rm_f(filename) FileUtils.rm_f(dbfile) -- cgit v1.2.3 From 0b921f5d3ebaed95dcc0d8304f054a9ccf672141 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sat, 13 May 2017 23:02:08 -0500 Subject: Update SchemaDumper.ignore_tables docs --- activerecord/lib/active_record/schema_dumper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 94d63765c9..24b81aabc8 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -11,8 +11,8 @@ module ActiveRecord ## # :singleton-method: # A list of tables which should not be dumped to the schema. - # Acceptable values are strings as well as regexp. - # This setting is only used if ActiveRecord::Base.schema_format == :ruby + # Acceptable values are strings as well as regexp if ActiveRecord::Base.schema_format == :ruby. + # Only strings are accepted if ActiveRecord::Base.schema_format == :sql. cattr_accessor :ignore_tables @@ignore_tables = [] -- cgit v1.2.3 From f98c97d37996b648a27c80166f0fc60ac94f76e2 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sun, 14 May 2017 12:15:35 -0500 Subject: Improvements for SQLite rake task. * Use NOT IN in SQL query * Quote table names propertly * Use array form of command invocation --- .../active_record/tasks/sqlite_database_tasks.rb | 25 ++++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb index 5542e5585c..7043d2f0c8 100644 --- a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb @@ -36,18 +36,18 @@ module ActiveRecord end def structure_dump(filename, extra_flags) - dbfile = configuration["database"] - flags = extra_flags.join(" ") if extra_flags + args = [] + args.concat(Array(extra_flags)) if extra_flags + args << configuration["database"] ignore_tables = ActiveRecord::SchemaDumper.ignore_tables if ignore_tables.any? - tables = `sqlite3 #{dbfile} .tables`.split - ignore_tables - condition = tables.map { |table| "tbl_name = '#{table}'" }.join(" OR ") - statement = "SELECT sql FROM sqlite_master WHERE #{condition} ORDER BY tbl_name, type DESC, name" - `sqlite3 #{flags} #{dbfile} "#{statement}" > #{filename}` + condition = ignore_tables.map { |table| connection.quote_table_name(table) }.join(", ") + args << "SELECT sql FROM sqlite_master WHERE tbl_name NOT IN (#{condition}) ORDER BY tbl_name, type DESC, name" else - `sqlite3 #{flags} #{dbfile} .schema > #{filename}` + args << ".schema" end + run_cmd("sqlite3", args, filename) end def structure_load(filename, extra_flags) @@ -65,6 +65,17 @@ module ActiveRecord def root @root end + + def run_cmd(cmd, args, out) + fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, out: out) + end + + def run_cmd_error(cmd, args) + msg = "failed to execute:\n" + msg << "#{cmd} #{args.join(' ')}\n\n" + msg << "Please check the output above for any errors and make sure that `#{cmd}` is installed in your PATH and has proper permissions.\n\n" + msg + end end end end -- cgit v1.2.3 From 701663e18c856d182265867fb50854d58a8b79d0 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Mon, 15 May 2017 18:24:33 -0500 Subject: Changelog entry [ci-skip] --- activerecord/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 2e059805a2..4e264f5f2a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Respect 'SchemaDumper.ignore_tables' in rake tasks for databases structure dump + + *Rusty Geldmacher*, *Guillermo Iguaran* + * Add type caster to `RuntimeReflection#alias_name` Fixes #28959. -- cgit v1.2.3 From 0714251b2d0d422dc3ffc852eeb604281195b0c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 16 May 2017 17:50:04 -0400 Subject: Fix association documentation The object inside the scope is of the class that define the association not the associated class. [ci skip] --- activerecord/lib/active_record/associations.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index e52c2004f3..7c37132d3a 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1276,7 +1276,7 @@ module ActiveRecord # Scope examples: # has_many :comments, -> { where(author_id: 1) } # has_many :employees, -> { joins(:address) } - # has_many :posts, ->(post) { where("max_post_length > ?", post.length) } + # has_many :posts, ->(blog) { where("max_post_length > ?", blog.max_post_length) } # # === Extensions # @@ -1443,7 +1443,7 @@ module ActiveRecord # Scope examples: # has_one :author, -> { where(comment_id: 1) } # has_one :employer, -> { joins(:company) } - # has_one :dob, ->(dob) { where("Date.new(2000, 01, 01) > ?", dob) } + # has_one :latest_post, ->(blog) { where("created_at > ?", blog.enabled_at) } # # === Options # @@ -1573,7 +1573,7 @@ module ActiveRecord # Scope examples: # belongs_to :firm, -> { where(id: 2) } # belongs_to :user, -> { joins(:friends) } - # belongs_to :level, ->(level) { where("game_level > ?", level.current) } + # belongs_to :level, ->(game) { where("game_level > ?", game.current_level) } # # === Options # @@ -1769,9 +1769,8 @@ module ActiveRecord # # Scope examples: # has_and_belongs_to_many :projects, -> { includes(:milestones, :manager) } - # has_and_belongs_to_many :categories, ->(category) { - # where("default_category = ?", category.name) - # } + # has_and_belongs_to_many :categories, ->(post) { + # where("default_category = ?", post.default_category) # # === Extensions # -- cgit v1.2.3 From 3556ee59e2466bd91fec5841b7723995fec881ba Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Wed, 17 May 2017 01:56:41 +0000 Subject: Set non 0 value to logger not to be affected by Ruby versions Addresses #29021 --- activerecord/test/cases/log_subscriber_test.rb | 46 +++++++++++++------------- 1 file changed, 23 insertions(+), 23 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index f426333aa5..b80257962c 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -59,19 +59,19 @@ class LogSubscriberTest < ActiveRecord::TestCase logger = TestDebugLogSubscriber.new assert_equal 0, logger.debugs.length - logger.sql(Event.new(0, sql: "hi mom!")) + logger.sql(Event.new(0.9, sql: "hi mom!")) assert_equal 1, logger.debugs.length - logger.sql(Event.new(0, sql: "hi mom!", name: "foo")) + logger.sql(Event.new(0.9, sql: "hi mom!", name: "foo")) assert_equal 2, logger.debugs.length - logger.sql(Event.new(0, sql: "hi mom!", name: "SCHEMA")) + logger.sql(Event.new(0.9, sql: "hi mom!", name: "SCHEMA")) assert_equal 2, logger.debugs.length end def test_sql_statements_are_not_squeezed logger = TestDebugLogSubscriber.new - logger.sql(Event.new(0, sql: "ruby rails")) + logger.sql(Event.new(0.9, sql: "ruby rails")) assert_match(/ruby rails/, logger.debugs.first) end @@ -87,7 +87,7 @@ class LogSubscriberTest < ActiveRecord::TestCase logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.each do |verb, color_regex| - logger.sql(Event.new(0, sql: verb.to_s)) + logger.sql(Event.new(0.9, sql: verb.to_s)) assert_match(/#{REGEXP_BOLD}#{color_regex}#{verb}#{REGEXP_CLEAR}/i, logger.debugs.last) end end @@ -96,11 +96,11 @@ class LogSubscriberTest < ActiveRecord::TestCase logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.each do |verb, _| - logger.sql(Event.new(0, sql: verb.to_s)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + logger.sql(Event.new(0.9, sql: verb.to_s)) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.9ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) - logger.sql(Event.new(0, sql: verb.to_s, name: "SQL")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA}SQL \(0.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + logger.sql(Event.new(0.9, sql: verb.to_s, name: "SQL")) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA}SQL \(0\.9ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) end end @@ -108,14 +108,14 @@ class LogSubscriberTest < ActiveRecord::TestCase logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.each do |verb, _| - logger.sql(Event.new(0, sql: verb.to_s, name: "Model Load")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Load \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + logger.sql(Event.new(0.9, sql: verb.to_s, name: "Model Load")) + assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Load \(0\.9ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) - logger.sql(Event.new(0, sql: verb.to_s, name: "Model Exists")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Exists \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + logger.sql(Event.new(0.9, sql: verb.to_s, name: "Model Exists")) + assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}Model Exists \(0\.9ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) - logger.sql(Event.new(0, sql: verb.to_s, name: "ANY SPECIFIC NAME")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}ANY SPECIFIC NAME \(0\.0ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) + logger.sql(Event.new(0.9, sql: verb.to_s, name: "ANY SPECIFIC NAME")) + assert_match(/#{REGEXP_BOLD}#{REGEXP_CYAN}ANY SPECIFIC NAME \(0\.9ms\)#{REGEXP_CLEAR}/i, logger.debugs.last) end end @@ -123,8 +123,8 @@ class LogSubscriberTest < ActiveRecord::TestCase logger = TestDebugLogSubscriber.new logger.colorize_logging = true SQL_COLORINGS.slice(:SELECT, :INSERT, :UPDATE, :DELETE).each do |verb, color_regex| - logger.sql(Event.new(0, sql: "#{verb} WHERE ID IN SELECT")) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}#{verb} WHERE ID IN SELECT#{REGEXP_CLEAR}/i, logger.debugs.last) + logger.sql(Event.new(0.9, sql: "#{verb} WHERE ID IN SELECT")) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.9ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}#{verb} WHERE ID IN SELECT#{REGEXP_CLEAR}/i, logger.debugs.last) end end @@ -138,8 +138,8 @@ class LogSubscriberTest < ActiveRecord::TestCase SELECT ID FROM THINGS ) EOS - logger.sql(Event.new(0, sql: sql)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}.*#{verb}.*#{REGEXP_CLEAR}/mi, logger.debugs.last) + logger.sql(Event.new(0.9, sql: sql)) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.9ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{color_regex}.*#{verb}.*#{REGEXP_CLEAR}/mi, logger.debugs.last) end end @@ -151,14 +151,14 @@ class LogSubscriberTest < ActiveRecord::TestCase (SELECT * FROM mytable FOR UPDATE) ss WHERE col1 = 5; EOS - logger.sql(Event.new(0, sql: sql)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*FOR UPDATE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) + logger.sql(Event.new(0.9, sql: sql)) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.9ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*FOR UPDATE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) sql = <<-EOS LOCK TABLE films IN SHARE MODE; EOS - logger.sql(Event.new(0, sql: sql)) - assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.0ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*LOCK TABLE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) + logger.sql(Event.new(0.9, sql: sql)) + assert_match(/#{REGEXP_BOLD}#{REGEXP_MAGENTA} \(0\.9ms\)#{REGEXP_CLEAR} #{REGEXP_BOLD}#{SQL_COLORINGS[:LOCK]}.*LOCK TABLE.*#{REGEXP_CLEAR}/mi, logger.debugs.last) end def test_exists_query_logging -- cgit v1.2.3 From 61fc1538c9f88acf07a1d937af15fc4414648853 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 17 May 2017 11:58:44 +0900 Subject: Remove unneeded `association.respond_to?(:reset_scope)` Since 86390c3 all associations have `reset_scope` so `respond_to?` is unneeded. --- activerecord/lib/active_record/autosave_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 607c54e481..ebe92e7878 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -389,7 +389,7 @@ module ActiveRecord autosave = reflection.options[:autosave] # reconstruct the scope now that we know the owner's id - association.reset_scope if association.respond_to?(:reset_scope) + association.reset_scope if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) if autosave -- cgit v1.2.3 From 75fa8dd309a84e125b59d01bf182d88419631eaa Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 18 May 2017 18:12:32 +0200 Subject: Use recyclable cache keys (#29092) --- activerecord/CHANGELOG.md | 9 +++++ activerecord/lib/active_record/integration.rb | 49 ++++++++++++++++++++++----- activerecord/test/cases/cache_key_test.rb | 22 ++++++++++-- activerecord/test/cases/integration_test.rb | 48 ++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4e264f5f2a..beead181f3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Add ActiveRecord::Base#cache_version to support recyclable cache keys via the new versioned entries + in ActiveSupport::Cache. This also means that ActiveRecord::Base#cache_key will now return a stable key + that does not include a timestamp any more. + + NOTE: This feature is turned off by default, and #cache_key will still return cache keys with timestamps + until you set ActiveRecord::Base.cache_versioning = true. That's the setting for all new apps on Rails 5.2+ + + *DHH* + * Respect 'SchemaDumper.ignore_tables' in rake tasks for databases structure dump *Rusty Geldmacher*, *Guillermo Iguaran* diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 8e71b60b29..ed652e26aa 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -13,6 +13,15 @@ module ActiveRecord # This is +:usec+, by default. class_attribute :cache_timestamp_format, instance_writer: false self.cache_timestamp_format = :usec + + ## + # :singleton-method: + # Indicates whether to use a stable #cache_key method that is accompanied + # by a changing version in the #cache_version method. + # + # This is +false+, by default until Rails 6.0. + class_attribute :cache_versioning, instance_writer: false + self.cache_versioning = false end # Returns a +String+, which Action Pack uses for constructing a URL to this @@ -52,25 +61,47 @@ module ActiveRecord # used to generate the key: # # Person.find(5).cache_key(:updated_at, :last_reviewed_at) + # + # If ActiveRecord::Base.cache_versioning is turned on, no version will be included + # in the cache key. The version will instead be supplied by #cache_version. This + # separation enables recycling of cache keys. + # + # Product.cache_versioning = true + # Product.new.cache_key # => "products/new" + # Person.find(5).cache_key # => "people/5" (even if updated_at available) def cache_key(*timestamp_names) if new_record? "#{model_name.cache_key}/new" else - timestamp = if timestamp_names.any? - max_updated_column_timestamp(timestamp_names) + if cache_version && timestamp_names.none? + "#{model_name.cache_key}/#{id}" else - max_updated_column_timestamp - end + timestamp = if timestamp_names.any? + max_updated_column_timestamp(timestamp_names) + else + max_updated_column_timestamp + end - if timestamp - timestamp = timestamp.utc.to_s(cache_timestamp_format) - "#{model_name.cache_key}/#{id}-#{timestamp}" - else - "#{model_name.cache_key}/#{id}" + if timestamp + timestamp = timestamp.utc.to_s(cache_timestamp_format) + "#{model_name.cache_key}/#{id}-#{timestamp}" + else + "#{model_name.cache_key}/#{id}" + end end end end + # Returns a cache version that can be used together with the cache key to form + # a recyclable caching scheme. By default, the #updated_at column is used for the + # cache_version, but this method can be overwritten to return something else. + # + # Note, this method will return nil if ActiveRecord::Base.cache_versioning is set to + # +false+ (which it is by default until Rails 6.0). + def cache_version + try(:updated_at).try(:to_i) if cache_versioning + end + module ClassMethods # Defines your model's +to_param+ method to generate "pretty" URLs # using +method_name+, which can be any attribute or method that diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb index 2c6a38ec35..f74cb18244 100644 --- a/activerecord/test/cases/cache_key_test.rb +++ b/activerecord/test/cases/cache_key_test.rb @@ -4,15 +4,23 @@ module ActiveRecord class CacheKeyTest < ActiveRecord::TestCase self.use_transactional_tests = false - class CacheMe < ActiveRecord::Base; end + class CacheMe < ActiveRecord::Base + self.cache_versioning = false + end + + class CacheMeWithVersion < ActiveRecord::Base + self.cache_versioning = true + end setup do @connection = ActiveRecord::Base.connection - @connection.create_table(:cache_mes) { |t| t.timestamps } + @connection.create_table(:cache_mes, force: true) { |t| t.timestamps } + @connection.create_table(:cache_me_with_versions, force: true) { |t| t.timestamps } end teardown do @connection.drop_table :cache_mes, if_exists: true + @connection.drop_table :cache_me_with_versions, if_exists: true end test "cache_key format is not too precise" do @@ -21,5 +29,15 @@ module ActiveRecord assert_equal key, record.reload.cache_key end + + test "cache_key has no version when versioning is on" do + record = CacheMeWithVersion.create + assert_equal "active_record/cache_key_test/cache_me_with_versions/#{record.id}", record.cache_key + end + + test "cache_version is only there when versioning is on" do + assert CacheMeWithVersion.create.cache_version.present? + assert_not CacheMe.create.cache_version.present? + end end end diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index 0678bb714f..7ffa86c42f 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -177,4 +177,52 @@ class IntegrationTest < ActiveRecord::TestCase owner.happy_at = nil assert_equal "owners/#{owner.id}", owner.cache_key(:happy_at) end + + def test_cache_key_is_stable_with_versioning_on + Developer.cache_versioning = true + + developer = Developer.first + first_key = developer.cache_key + + developer.touch + second_key = developer.cache_key + + assert_equal first_key, second_key + ensure + Developer.cache_versioning = false + end + + def test_cache_version_changes_with_versioning_on + Developer.cache_versioning = true + + developer = Developer.first + first_version = developer.cache_version + + travel 10.seconds do + developer.touch + end + + second_version = developer.cache_version + + assert_not_equal first_version, second_version + ensure + Developer.cache_versioning = false + end + + def test_cache_key_retains_version_when_custom_timestamp_is_used + Developer.cache_versioning = true + + developer = Developer.first + first_key = developer.cache_key(:updated_at) + + travel 10.seconds do + developer.touch + end + + second_key = developer.cache_key(:updated_at) + + assert_not_equal first_key, second_key + ensure + Developer.cache_versioning = false + end end -- cgit v1.2.3 From b9b4fa9154e7c81ddb2bac4c5d53a9cb98c3351e Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 19 May 2017 08:28:15 +0900 Subject: Cleanup CHANGELOGs [ci skip] * Fix indentation. * Add backticks. --- activerecord/CHANGELOG.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index beead181f3..907dd894cd 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,13 +1,13 @@ -* Add ActiveRecord::Base#cache_version to support recyclable cache keys via the new versioned entries - in ActiveSupport::Cache. This also means that ActiveRecord::Base#cache_key will now return a stable key +* Add `ActiveRecord::Base#cache_version` to support recyclable cache keys via the new versioned entries + in `ActiveSupport::Cache`. This also means that `ActiveRecord::Base#cache_key` will now return a stable key that does not include a timestamp any more. - - NOTE: This feature is turned off by default, and #cache_key will still return cache keys with timestamps - until you set ActiveRecord::Base.cache_versioning = true. That's the setting for all new apps on Rails 5.2+ - + + NOTE: This feature is turned off by default, and `#cache_key` will still return cache keys with timestamps + until you set `ActiveRecord::Base.cache_versioning = true`. That's the setting for all new apps on Rails 5.2+ + *DHH* -* Respect 'SchemaDumper.ignore_tables' in rake tasks for databases structure dump +* Respect `SchemaDumper.ignore_tables` in rake tasks for databases structure dump *Rusty Geldmacher*, *Guillermo Iguaran* -- cgit v1.2.3 From 62b10bfc445837c3e6e466449743605d6fca3811 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 19 May 2017 08:51:50 +0900 Subject: Make helper methods in tests to private `make_model` and `make_no_pk_hm_t` in `HasManyThroughAssociationsTest` are not a test case. it should be private. --- .../has_many_through_associations_test.rb | 37 +++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ea52fb5a67..9156f6d57a 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -64,10 +64,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase club1.members.sort_by(&:id) end - def make_model(name) - Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } - end - def test_ordered_has_many_through person_prime = Class.new(ActiveRecord::Base) do def self.name; "Person"; end @@ -152,20 +148,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert after_destroy_called, "after destroy should be called" end - def make_no_pk_hm_t - lesson = make_model "Lesson" - student = make_model "Student" - - lesson_student = make_model "LessonStudent" - lesson_student.table_name = "lessons_students" - - lesson_student.belongs_to :lesson, anonymous_class: lesson - lesson_student.belongs_to :student, anonymous_class: student - lesson.has_many :lesson_students, anonymous_class: lesson_student - lesson.has_many :students, through: :lesson_students, anonymous_class: student - [lesson, lesson_student, student] - end - def test_pk_is_not_required_for_join post = Post.includes(:scategories).first post2 = Post.includes(:categories).first @@ -1252,4 +1234,23 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase ) end end + + private + def make_model(name) + Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } + end + + def make_no_pk_hm_t + lesson = make_model "Lesson" + student = make_model "Student" + + lesson_student = make_model "LessonStudent" + lesson_student.table_name = "lessons_students" + + lesson_student.belongs_to :lesson, anonymous_class: lesson + lesson_student.belongs_to :student, anonymous_class: student + lesson.has_many :lesson_students, anonymous_class: lesson_student + lesson.has_many :students, through: :lesson_students, anonymous_class: student + [lesson, lesson_student, student] + end end -- cgit v1.2.3 From 1aad9f6b5b610a487f248268b464fbbd74c82531 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 19 May 2017 13:28:30 +0900 Subject: Both reference id and type should be `NOT NULL` if `null: false` is specified This is a regression due to #28282. Fixes #29136. --- .../connection_adapters/abstract/schema_definitions.rb | 2 +- activerecord/test/cases/migration/references_statements_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 46d7f84efd..a30fbe0e05 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -146,7 +146,7 @@ module ActiveRecord end def polymorphic_options - as_options(polymorphic) + as_options(polymorphic).merge(null: options[:null]) end def index_options diff --git a/activerecord/test/cases/migration/references_statements_test.rb b/activerecord/test/cases/migration/references_statements_test.rb index 06c44c8c52..e9eb9968cb 100644 --- a/activerecord/test/cases/migration/references_statements_test.rb +++ b/activerecord/test/cases/migration/references_statements_test.rb @@ -50,6 +50,14 @@ module ActiveRecord assert column_exists?(table_name, :taggable_type, :string, default: "Photo") end + def test_creates_reference_type_column_with_not_null + connection.create_table table_name, force: true do |t| + t.references :taggable, null: false, polymorphic: true + end + assert column_exists?(table_name, :taggable_id, :integer, null: false) + assert column_exists?(table_name, :taggable_type, :string, null: false) + end + def test_does_not_share_options_with_reference_type_column add_reference table_name, :taggable, type: :integer, limit: 2, polymorphic: true assert column_exists?(table_name, :taggable_id, :integer, limit: 2) -- cgit v1.2.3 From aa8749eb52d7919a438940c9218cad98d892f9ad Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 19 May 2017 14:09:09 +0200 Subject: Add cache_key_with_version and use it in ActiveSupport::Cache.expand_cache_key This retains the existing behavior of ActiveSupport::Cache.expand_cache_key (as used by etaging) where the cache key includes the version. --- activerecord/lib/active_record/integration.rb | 43 ++++++++++++++++----------- activerecord/test/cases/cache_key_test.rb | 8 +++++ activerecord/test/cases/integration_test.rb | 18 ++++++----- 3 files changed, 45 insertions(+), 24 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index ed652e26aa..36ce49d518 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -7,8 +7,8 @@ module ActiveRecord included do ## # :singleton-method: - # Indicates the format used to generate the timestamp in the cache key. - # Accepts any of the symbols in Time::DATE_FORMATS. + # Indicates the format used to generate the timestamp in the cache key, if + # versioning is off. Accepts any of the symbols in Time::DATE_FORMATS. # # This is +:usec+, by default. class_attribute :cache_timestamp_format, instance_writer: false @@ -51,24 +51,16 @@ module ActiveRecord id && id.to_s # Be sure to stringify the id for routes end - # Returns a cache key that can be used to identify this record. + # Returns a stable cache key that can be used to identify this record. # # Product.new.cache_key # => "products/new" - # Product.find(5).cache_key # => "products/5" (updated_at not available) - # Person.find(5).cache_key # => "people/5-20071224150000" (updated_at available) - # - # You can also pass a list of named timestamps, and the newest in the list will be - # used to generate the key: - # - # Person.find(5).cache_key(:updated_at, :last_reviewed_at) + # Product.find(5).cache_key # => "products/5" # - # If ActiveRecord::Base.cache_versioning is turned on, no version will be included - # in the cache key. The version will instead be supplied by #cache_version. This - # separation enables recycling of cache keys. + # If ActiveRecord::Base.cache_versioning is turned off, as it was in Rails 5.1 and earlier, + # the cache key will also include a version. # - # Product.cache_versioning = true - # Product.new.cache_key # => "products/new" - # Person.find(5).cache_key # => "people/5" (even if updated_at available) + # Product.cache_versioning = false + # Person.find(5).cache_key # => "people/5-20071224150000" (updated_at available) def cache_key(*timestamp_names) if new_record? "#{model_name.cache_key}/new" @@ -77,6 +69,11 @@ module ActiveRecord "#{model_name.cache_key}/#{id}" else timestamp = if timestamp_names.any? + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Specifying a timestamp name for #cache_key has been deprecated in favor of + the explicit #cache_version method that can be overwritten. + MSG + max_updated_column_timestamp(timestamp_names) else max_updated_column_timestamp @@ -99,9 +96,21 @@ module ActiveRecord # Note, this method will return nil if ActiveRecord::Base.cache_versioning is set to # +false+ (which it is by default until Rails 6.0). def cache_version - try(:updated_at).try(:to_i) if cache_versioning + if cache_versioning && timestamp = try(:updated_at) + updated_at.utc.to_s(:usec) + end + end + + # Returns a cache key along with the version. + def cache_key_with_version + if version = cache_version + "#{cache_key}-#{version}" + else + cache_key + end end + module ClassMethods # Defines your model's +to_param+ method to generate "pretty" URLs # using +method_name+, which can be any attribute or method that diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb index f74cb18244..7b8264e6e8 100644 --- a/activerecord/test/cases/cache_key_test.rb +++ b/activerecord/test/cases/cache_key_test.rb @@ -39,5 +39,13 @@ module ActiveRecord assert CacheMeWithVersion.create.cache_version.present? assert_not CacheMe.create.cache_version.present? end + + test "cache_key_with_version always has both key and version" do + r1 = CacheMeWithVersion.create + assert_equal "active_record/cache_key_test/cache_me_with_versions/#{r1.id}-#{r1.updated_at.to_s(:usec)}", r1.cache_key_with_version + + r2 = CacheMe.create + assert_equal "active_record/cache_key_test/cache_mes/#{r2.id}-#{r2.updated_at.to_s(:usec)}", r2.cache_key_with_version + end end end diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index 7ffa86c42f..9104976126 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -168,14 +168,18 @@ class IntegrationTest < ActiveRecord::TestCase end def test_named_timestamps_for_cache_key - owner = owners(:blackbeard) - assert_equal "owners/#{owner.id}-#{owner.happy_at.utc.to_s(:usec)}", owner.cache_key(:updated_at, :happy_at) + assert_deprecated do + owner = owners(:blackbeard) + assert_equal "owners/#{owner.id}-#{owner.happy_at.utc.to_s(:usec)}", owner.cache_key(:updated_at, :happy_at) + end end def test_cache_key_when_named_timestamp_is_nil - owner = owners(:blackbeard) - owner.happy_at = nil - assert_equal "owners/#{owner.id}", owner.cache_key(:happy_at) + assert_deprecated do + owner = owners(:blackbeard) + owner.happy_at = nil + assert_equal "owners/#{owner.id}", owner.cache_key(:happy_at) + end end def test_cache_key_is_stable_with_versioning_on @@ -213,13 +217,13 @@ class IntegrationTest < ActiveRecord::TestCase Developer.cache_versioning = true developer = Developer.first - first_key = developer.cache_key(:updated_at) + first_key = developer.cache_key_with_version travel 10.seconds do developer.touch end - second_key = developer.cache_key(:updated_at) + second_key = developer.cache_key_with_version assert_not_equal first_key, second_key ensure -- cgit v1.2.3 From edc0ffc38dc212aedeba3b92531552830569e322 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 20 May 2017 06:59:17 +0900 Subject: Fix `warning: assigned but unused variable - timestamp` --- activerecord/lib/active_record/integration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 36ce49d518..441237da1e 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -97,7 +97,7 @@ module ActiveRecord # +false+ (which it is by default until Rails 6.0). def cache_version if cache_versioning && timestamp = try(:updated_at) - updated_at.utc.to_s(:usec) + timestamp.utc.to_s(:usec) end end -- cgit v1.2.3 From 0d874b4a5a39646bbfc3e062db62a3b3ab68b47c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 20 May 2017 12:36:16 +0900 Subject: Make `VALID_DIRECTIONS` to `Set` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ```ruby require "benchmark/ips" require "set" array = [:asc, :desc, :ASC, :DESC, "asc", "desc", "ASC", "DESC"] set = array.to_set item = "DESC" Benchmark.ips do |x| x.report "array" do array.include?(item) end x.report "set" do set.include?(item) end end ``` ``` % ruby array_vs_set.rb Warming up -------------------------------------- array 188.441k i/100ms set 229.531k i/100ms Calculating ------------------------------------- array 3.508M (± 9.0%) i/s - 17.525M in 5.043058s set 5.134M (± 7.6%) i/s - 25.707M in 5.038921s ``` --- activerecord/lib/active_record/relation/query_methods.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 76e529f2de..79e65baae5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1100,14 +1100,16 @@ module ActiveRecord end VALID_DIRECTIONS = [:asc, :desc, :ASC, :DESC, - "asc", "desc", "ASC", "DESC"] # :nodoc: + "asc", "desc", "ASC", "DESC"].to_set # :nodoc: def validate_order_args(args) args.each do |arg| next unless arg.is_a?(Hash) arg.each do |_key, value| - raise ArgumentError, "Direction \"#{value}\" is invalid. Valid " \ - "directions are: #{VALID_DIRECTIONS.inspect}" unless VALID_DIRECTIONS.include?(value) + unless VALID_DIRECTIONS.include?(value) + raise ArgumentError, + "Direction \"#{value}\" is invalid. Valid directions are: #{VALID_DIRECTIONS.to_a.inspect}" + end end end end -- cgit v1.2.3 From bf14291a00478bbb9c65c3abe41f0b336236b6e5 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 21 May 2017 19:50:19 +0900 Subject: Remove a duplicate test of inverse_associations_test in AR --- .../test/cases/associations/inverse_associations_test.rb | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 287b3e9ebc..467cc73ecd 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -651,20 +651,6 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance" end - def test_child_instance_should_be_shared_with_replaced_via_method_parent - face = faces(:confused) - new_man = Man.new - - assert_not_nil face.polymorphic_man - face.polymorphic_man = new_man - - assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance" - face.description = "Bongo" - assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to parent instance" - new_man.polymorphic_face.description = "Mungo" - assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance" - end - def test_inversed_instance_should_not_be_reloaded_after_stale_state_changed new_man = Man.new face = Face.new -- cgit v1.2.3 From 16bc5fd0ed3892bec0560fdbc93953f8b742bdb8 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 22 May 2017 09:29:01 +0900 Subject: Remove unused `JoinPart#name` --- .../lib/active_record/associations/join_dependency/join_part.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index 61cec5403a..80c9fde5d1 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -22,10 +22,6 @@ module ActiveRecord @children = children end - def name - reflection.name - end - def match?(other) self.class == other.class end -- cgit v1.2.3 From 656c0a4da0a24db1e5be65971f36662e64f616b5 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 22 May 2017 10:33:04 +0900 Subject: Extract `JSONSharedTestCases` Both `mysql2/json_test.rb` and `postgresql/json_test.rb` have same test cases. --- .../test/cases/adapters/mysql2/json_test.rb | 176 +------------------- .../test/cases/adapters/postgresql/json_test.rb | 182 +-------------------- activerecord/test/cases/json_shared_test_cases.rb | 177 ++++++++++++++++++++ 3 files changed, 184 insertions(+), 351 deletions(-) create mode 100644 activerecord/test/cases/json_shared_test_cases.rb (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql2/json_test.rb b/activerecord/test/cases/adapters/mysql2/json_test.rb index 6954006003..d311ffb703 100644 --- a/activerecord/test/cases/adapters/mysql2/json_test.rb +++ b/activerecord/test/cases/adapters/mysql2/json_test.rb @@ -1,17 +1,11 @@ require "cases/helper" -require "support/schema_dumping_helper" +require "cases/json_shared_test_cases" if ActiveRecord::Base.connection.supports_json? class Mysql2JSONTest < ActiveRecord::Mysql2TestCase - include SchemaDumpingHelper + include JSONSharedTestCases self.use_transactional_tests = false - class JsonDataType < ActiveRecord::Base - self.table_name = "json_data_type" - - store_accessor :settings, :resolution - end - def setup @connection = ActiveRecord::Base.connection begin @@ -27,169 +21,9 @@ if ActiveRecord::Base.connection.supports_json? JsonDataType.reset_column_information end - def test_column - column = JsonDataType.columns_hash["payload"] - assert_equal :json, column.type - assert_equal "json", column.sql_type - - type = JsonDataType.type_for_attribute("payload") - assert_not type.binary? - end - - def test_change_table_supports_json - @connection.change_table("json_data_type") do |t| - t.json "users" + private + def column_type + :json end - JsonDataType.reset_column_information - column = JsonDataType.columns_hash["users"] - assert_equal :json, column.type - end - - def test_schema_dumping - output = dump_table_schema("json_data_type") - assert_match(/t\.json\s+"settings"/, output) - end - - def test_cast_value_on_write - x = JsonDataType.new payload: { "string" => "foo", :symbol => :bar } - assert_equal({ "string" => "foo", :symbol => :bar }, x.payload_before_type_cast) - assert_equal({ "string" => "foo", "symbol" => "bar" }, x.payload) - x.save - assert_equal({ "string" => "foo", "symbol" => "bar" }, x.reload.payload) - end - - def test_type_cast_json - type = JsonDataType.type_for_attribute("payload") - - data = "{\"a_key\":\"a_value\"}" - hash = type.deserialize(data) - assert_equal({ "a_key" => "a_value" }, hash) - assert_equal({ "a_key" => "a_value" }, type.deserialize(data)) - - assert_equal({}, type.deserialize("{}")) - assert_equal({ "key" => nil }, type.deserialize('{"key": null}')) - assert_equal({ "c" => "}", '"a"' => 'b "a b' }, type.deserialize(%q({"c":"}", "\"a\"":"b \"a b"}))) - end - - def test_rewrite - @connection.execute "insert into json_data_type (payload) VALUES ('{\"k\":\"v\"}')" - x = JsonDataType.first - x.payload = { '"a\'' => "b" } - assert x.save! - end - - def test_select - @connection.execute "insert into json_data_type (payload) VALUES ('{\"k\":\"v\"}')" - x = JsonDataType.first - assert_equal({ "k" => "v" }, x.payload) - end - - def test_select_multikey - @connection.execute %q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')| - x = JsonDataType.first - assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload) - end - - def test_null_json - @connection.execute "insert into json_data_type (payload) VALUES(null)" - x = JsonDataType.first - assert_nil(x.payload) - end - - def test_select_array_json_value - @connection.execute %q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')| - x = JsonDataType.first - assert_equal(["v0", { "k1" => "v1" }], x.payload) - end - - def test_select_nil_json_after_create - json = JsonDataType.create(payload: nil) - x = JsonDataType.where(payload: nil).first - assert_equal(json, x) - end - - def test_select_nil_json_after_update - json = JsonDataType.create(payload: "foo") - x = JsonDataType.where(payload: nil).first - assert_nil(x) - - json.update_attributes payload: nil - x = JsonDataType.where(payload: nil).first - assert_equal(json.reload, x) - end - - def test_rewrite_array_json_value - @connection.execute %q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')| - x = JsonDataType.first - x.payload = ["v1", { "k2" => "v2" }, "v3"] - assert x.save! - end - - def test_with_store_accessors - x = JsonDataType.new(resolution: "320×480") - assert_equal "320×480", x.resolution - - x.save! - x = JsonDataType.first - assert_equal "320×480", x.resolution - - x.resolution = "640×1136" - x.save! - - x = JsonDataType.first - assert_equal "640×1136", x.resolution - end - - def test_duplication_with_store_accessors - x = JsonDataType.new(resolution: "320×480") - assert_equal "320×480", x.resolution - - y = x.dup - assert_equal "320×480", y.resolution - end - - def test_yaml_round_trip_with_store_accessors - x = JsonDataType.new(resolution: "320×480") - assert_equal "320×480", x.resolution - - y = YAML.load(YAML.dump(x)) - assert_equal "320×480", y.resolution - end - - def test_changes_in_place - json = JsonDataType.new - assert_not json.changed? - - json.payload = { "one" => "two" } - assert json.changed? - assert json.payload_changed? - - json.save! - assert_not json.changed? - - json.payload["three"] = "four" - assert json.payload_changed? - - json.save! - json.reload - - assert_equal({ "one" => "two", "three" => "four" }, json.payload) - assert_not json.changed? - end - - def test_assigning_string_literal - json = JsonDataType.create(payload: "foo") - assert_equal "foo", json.payload - end - - def test_assigning_number - json = JsonDataType.create(payload: 1.234) - assert_equal 1.234, json.payload - end - - def test_assigning_boolean - json = JsonDataType.create(payload: true) - assert_equal true, json.payload - end end end diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index d4e627001c..4eeb563781 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -1,14 +1,8 @@ require "cases/helper" -require "support/schema_dumping_helper" +require "cases/json_shared_test_cases" module PostgresqlJSONSharedTestCases - include SchemaDumpingHelper - - class JsonDataType < ActiveRecord::Base - self.table_name = "json_data_type" - - store_accessor :settings, :resolution - end + include JSONSharedTestCases def setup @connection = ActiveRecord::Base.connection @@ -28,16 +22,6 @@ module PostgresqlJSONSharedTestCases JsonDataType.reset_column_information end - def test_column - column = JsonDataType.columns_hash["payload"] - assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type - assert_not column.array? - - type = JsonDataType.type_for_attribute("payload") - assert_not type.binary? - end - def test_default @connection.add_column "json_data_type", "permissions", column_type, default: { "users": "read", "posts": ["read", "write"] } JsonDataType.reset_column_information @@ -48,34 +32,6 @@ module PostgresqlJSONSharedTestCases JsonDataType.reset_column_information end - def test_change_table_supports_json - @connection.transaction do - @connection.change_table("json_data_type") do |t| - t.public_send column_type, "users", default: "{}" # t.json 'users', default: '{}' - end - JsonDataType.reset_column_information - column = JsonDataType.columns_hash["users"] - assert_equal column_type, column.type - - raise ActiveRecord::Rollback # reset the schema change - end - ensure - JsonDataType.reset_column_information - end - - def test_schema_dumping - output = dump_table_schema("json_data_type") - assert_match(/t\.#{column_type.to_s}\s+"payload",\s+default: {}/, output) - end - - def test_cast_value_on_write - x = JsonDataType.new payload: { "string" => "foo", :symbol => :bar } - assert_equal({ "string" => "foo", :symbol => :bar }, x.payload_before_type_cast) - assert_equal({ "string" => "foo", "symbol" => "bar" }, x.payload) - x.save - assert_equal({ "string" => "foo", "symbol" => "bar" }, x.reload.payload) - end - def test_deserialize_with_array x = JsonDataType.new(objects: ["foo" => "bar"]) assert_equal ["foo" => "bar"], x.objects @@ -84,140 +40,6 @@ module PostgresqlJSONSharedTestCases x.reload assert_equal ["foo" => "bar"], x.objects end - - def test_type_cast_json - type = JsonDataType.type_for_attribute("payload") - - data = "{\"a_key\":\"a_value\"}" - hash = type.deserialize(data) - assert_equal({ "a_key" => "a_value" }, hash) - assert_equal({ "a_key" => "a_value" }, type.deserialize(data)) - - assert_equal({}, type.deserialize("{}")) - assert_equal({ "key" => nil }, type.deserialize('{"key": null}')) - assert_equal({ "c" => "}", '"a"' => 'b "a b' }, type.deserialize(%q({"c":"}", "\"a\"":"b \"a b"}))) - end - - def test_rewrite - @connection.execute "insert into json_data_type (payload) VALUES ('{\"k\":\"v\"}')" - x = JsonDataType.first - x.payload = { '"a\'' => "b" } - assert x.save! - end - - def test_select - @connection.execute "insert into json_data_type (payload) VALUES ('{\"k\":\"v\"}')" - x = JsonDataType.first - assert_equal({ "k" => "v" }, x.payload) - end - - def test_select_multikey - @connection.execute %q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')| - x = JsonDataType.first - assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload) - end - - def test_null_json - @connection.execute "insert into json_data_type (payload) VALUES(null)" - x = JsonDataType.first - assert_nil(x.payload) - end - - def test_select_nil_json_after_create - json = JsonDataType.create(payload: nil) - x = JsonDataType.where(payload: nil).first - assert_equal(json, x) - end - - def test_select_nil_json_after_update - json = JsonDataType.create(payload: "foo") - x = JsonDataType.where(payload: nil).first - assert_nil(x) - - json.update_attributes payload: nil - x = JsonDataType.where(payload: nil).first - assert_equal(json.reload, x) - end - - def test_select_array_json_value - @connection.execute %q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')| - x = JsonDataType.first - assert_equal(["v0", { "k1" => "v1" }], x.payload) - end - - def test_rewrite_array_json_value - @connection.execute %q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')| - x = JsonDataType.first - x.payload = ["v1", { "k2" => "v2" }, "v3"] - assert x.save! - end - - def test_with_store_accessors - x = JsonDataType.new(resolution: "320×480") - assert_equal "320×480", x.resolution - - x.save! - x = JsonDataType.first - assert_equal "320×480", x.resolution - - x.resolution = "640×1136" - x.save! - - x = JsonDataType.first - assert_equal "640×1136", x.resolution - end - - def test_duplication_with_store_accessors - x = JsonDataType.new(resolution: "320×480") - assert_equal "320×480", x.resolution - - y = x.dup - assert_equal "320×480", y.resolution - end - - def test_yaml_round_trip_with_store_accessors - x = JsonDataType.new(resolution: "320×480") - assert_equal "320×480", x.resolution - - y = YAML.load(YAML.dump(x)) - assert_equal "320×480", y.resolution - end - - def test_changes_in_place - json = JsonDataType.new - assert_not json.changed? - - json.payload = { "one" => "two" } - assert json.changed? - assert json.payload_changed? - - json.save! - assert_not json.changed? - - json.payload["three"] = "four" - assert json.payload_changed? - - json.save! - json.reload - - assert_equal({ "one" => "two", "three" => "four" }, json.payload) - assert_not json.changed? - end - - def test_assigning_string_literal - json = JsonDataType.create(payload: "foo") - assert_equal "foo", json.payload - end - - def test_assigning_number - json = JsonDataType.create(payload: 1.234) - assert_equal 1.234, json.payload - end - - def test_assigning_boolean - json = JsonDataType.create(payload: true) - assert_equal true, json.payload - end end class PostgresqlJSONTest < ActiveRecord::PostgreSQLTestCase diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb new file mode 100644 index 0000000000..d190b027bf --- /dev/null +++ b/activerecord/test/cases/json_shared_test_cases.rb @@ -0,0 +1,177 @@ +require "support/schema_dumping_helper" + +module JSONSharedTestCases + include SchemaDumpingHelper + + class JsonDataType < ActiveRecord::Base + self.table_name = "json_data_type" + + store_accessor :settings, :resolution + end + + def test_column + column = JsonDataType.columns_hash["payload"] + assert_equal column_type, column.type + assert_equal column_type.to_s, column.sql_type + + type = JsonDataType.type_for_attribute("payload") + assert_not type.binary? + end + + def test_change_table_supports_json + @connection.change_table("json_data_type") do |t| + t.public_send column_type, "users" + end + JsonDataType.reset_column_information + column = JsonDataType.columns_hash["users"] + assert_equal column_type, column.type + assert_equal column_type.to_s, column.sql_type + end + + def test_schema_dumping + output = dump_table_schema("json_data_type") + assert_match(/t\.#{column_type}\s+"settings"/, output) + end + + def test_cast_value_on_write + x = JsonDataType.new(payload: { "string" => "foo", :symbol => :bar }) + assert_equal({ "string" => "foo", :symbol => :bar }, x.payload_before_type_cast) + assert_equal({ "string" => "foo", "symbol" => "bar" }, x.payload) + x.save! + assert_equal({ "string" => "foo", "symbol" => "bar" }, x.reload.payload) + end + + def test_type_cast_json + type = JsonDataType.type_for_attribute("payload") + + data = '{"a_key":"a_value"}' + hash = type.deserialize(data) + assert_equal({ "a_key" => "a_value" }, hash) + assert_equal({ "a_key" => "a_value" }, type.deserialize(data)) + + assert_equal({}, type.deserialize("{}")) + assert_equal({ "key" => nil }, type.deserialize('{"key": null}')) + assert_equal({ "c" => "}", '"a"' => 'b "a b' }, type.deserialize(%q({"c":"}", "\"a\"":"b \"a b"}))) + end + + def test_rewrite + @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + x = JsonDataType.first + x.payload = { '"a\'' => "b" } + assert x.save! + end + + def test_select + @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + x = JsonDataType.first + assert_equal({ "k" => "v" }, x.payload) + end + + def test_select_multikey + @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')|) + x = JsonDataType.first + assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload) + end + + def test_null_json + @connection.execute("insert into json_data_type (payload) VALUES(null)") + x = JsonDataType.first + assert_nil(x.payload) + end + + def test_select_nil_json_after_create + json = JsonDataType.create!(payload: nil) + x = JsonDataType.where(payload: nil).first + assert_equal(json, x) + end + + def test_select_nil_json_after_update + json = JsonDataType.create!(payload: "foo") + x = JsonDataType.where(payload: nil).first + assert_nil(x) + + json.update_attributes(payload: nil) + x = JsonDataType.where(payload: nil).first + assert_equal(json.reload, x) + end + + def test_select_array_json_value + @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + x = JsonDataType.first + assert_equal(["v0", { "k1" => "v1" }], x.payload) + end + + def test_rewrite_array_json_value + @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + x = JsonDataType.first + x.payload = ["v1", { "k2" => "v2" }, "v3"] + assert x.save! + end + + def test_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + x.save! + x = JsonDataType.first + assert_equal "320×480", x.resolution + + x.resolution = "640×1136" + x.save! + + x = JsonDataType.first + assert_equal "640×1136", x.resolution + end + + def test_duplication_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + y = x.dup + assert_equal "320×480", y.resolution + end + + def test_yaml_round_trip_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + y = YAML.load(YAML.dump(x)) + assert_equal "320×480", y.resolution + end + + def test_changes_in_place + json = JsonDataType.new + assert_not json.changed? + + json.payload = { "one" => "two" } + assert json.changed? + assert json.payload_changed? + + json.save! + assert_not json.changed? + + json.payload["three"] = "four" + assert json.payload_changed? + + json.save! + json.reload + + assert_equal({ "one" => "two", "three" => "four" }, json.payload) + assert_not json.changed? + end + + def test_assigning_string_literal + json = JsonDataType.create!(payload: "foo") + assert_equal "foo", json.payload + end + + def test_assigning_number + json = JsonDataType.create!(payload: 1.234) + assert_equal 1.234, json.payload + end + + def test_assigning_boolean + json = JsonDataType.create!(payload: true) + assert_equal true, json.payload + end +end -- cgit v1.2.3 From d3c56fce94d93d42ba2a4a537075e7ba3cd0f9f3 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 23 May 2017 00:22:47 +0900 Subject: Remove unused `left_joins_values` generation This was added at #22125 but `left_joins_values` is never used. --- activerecord/lib/active_record/relation.rb | 2 +- activerecord/test/cases/relation/mutation_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 333ad16e11..2d5be32266 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -2,7 +2,7 @@ module ActiveRecord # = Active Record \Relation class Relation MULTI_VALUE_METHODS = [:includes, :eager_load, :preload, :select, :group, - :order, :joins, :left_joins, :left_outer_joins, :references, + :order, :joins, :left_outer_joins, :references, :extending, :unscope] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering, diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index 11ef0d8743..dea787c07f 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -36,7 +36,7 @@ module ActiveRecord @relation ||= Relation.new FakeKlass.new("posts"), Post.arel_table, Post.predicate_builder end - (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select, :left_joins]).each do |method| + (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select]).each do |method| test "##{method}!" do assert relation.public_send("#{method}!", :foo).equal?(relation) assert_equal [:foo], relation.public_send("#{method}_values") -- cgit v1.2.3 From 54a030fb9ad705a0c2cfcb9e839eea0d1923b6ac Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 23 May 2017 00:58:18 +0900 Subject: Refactor making join constraints The only difference between `make_inner_joins` and `make_left_outer_joins` is the `join_type`. --- .../active_record/associations/join_dependency.rb | 29 +++++----------------- 1 file changed, 6 insertions(+), 23 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 8995b1e352..643226267c 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -106,12 +106,7 @@ module ActiveRecord def join_constraints(outer_joins, join_type) joins = join_root.children.flat_map { |child| - - if join_type == Arel::Nodes::OuterJoin - make_left_outer_joins join_root, child - else - make_inner_joins join_root, child - end + make_join_constraints(join_root, child, join_type) } joins.concat outer_joins.flat_map { |oj| @@ -175,27 +170,15 @@ module ActiveRecord end def make_outer_joins(parent, child) - tables = table_aliases_for(parent, child) - join_type = Arel::Nodes::OuterJoin - info = make_constraints parent, child, tables, join_type - - [info] + child.children.flat_map { |c| make_outer_joins(child, c) } - end - - def make_left_outer_joins(parent, child) - tables = child.tables join_type = Arel::Nodes::OuterJoin - info = make_constraints parent, child, tables, join_type - - [info] + child.children.flat_map { |c| make_left_outer_joins(child, c) } + make_join_constraints(parent, child, join_type, true) end - def make_inner_joins(parent, child) - tables = child.tables - join_type = Arel::Nodes::InnerJoin - info = make_constraints parent, child, tables, join_type + def make_join_constraints(parent, child, join_type, aliasing = false) + tables = aliasing ? table_aliases_for(parent, child) : child.tables + info = make_constraints(parent, child, tables, join_type) - [info] + child.children.flat_map { |c| make_inner_joins(child, c) } + [info] + child.children.flat_map { |c| make_join_constraints(child, c, join_type, aliasing) } end def table_aliases_for(parent, node) -- cgit v1.2.3 From 78ad8098ba46ec59eb0d6e9d1280339e1294e093 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Mon, 22 May 2017 17:39:14 +0100 Subject: More friendly exception in nested attributes --- activerecord/lib/active_record/nested_attributes.rb | 2 +- activerecord/test/cases/nested_attributes_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 01ecd79b8f..3f39fb84e8 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -458,7 +458,7 @@ module ActiveRecord end unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) - raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" + raise ArgumentError, "Hash or Array expected for attribute `#{association_name}`, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" end check_record_limit!(options[:limit], attributes_collection) diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index b87419d203..5a62cbd3a6 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -752,7 +752,7 @@ module NestedAttributesOnACollectionAssociationTests exception = assert_raise ArgumentError do @pirate.send(association_setter, "foo") end - assert_equal 'Hash or Array expected, got String ("foo")', exception.message + assert_equal %{Hash or Array expected for attribute `#{@association_name}`, got String ("foo")}, exception.message end def test_should_work_with_update_as_well -- cgit v1.2.3 From 40bdbce191ad90dfea43dad51fac5c4726b89392 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Mon, 15 May 2017 14:17:28 +0000 Subject: Define path with __dir__ ".. with __dir__ we can restore order in the Universe." - by @fxn Related to 5b8738c2df003a96f0e490c43559747618d10f5f --- activerecord/Rakefile | 6 +++--- activerecord/activerecord.gemspec | 2 +- activerecord/lib/active_record.rb | 2 +- activerecord/lib/rails/generators/active_record.rb | 2 +- activerecord/test/cases/adapters/postgresql/bytea_test.rb | 2 +- .../cases/associations/has_and_belongs_to_many_associations_test.rb | 2 +- activerecord/test/cases/inheritance_test.rb | 2 +- activerecord/test/cases/reload_models_test.rb | 2 +- activerecord/test/cases/yaml_serialization_test.rb | 4 ++-- activerecord/test/config.rb | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) (limited to 'activerecord') diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 7be3d851f1..2d0d5bd657 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -1,7 +1,7 @@ require "rake/testtask" -require File.expand_path(File.dirname(__FILE__)) + "/test/config" -require File.expand_path(File.dirname(__FILE__)) + "/test/support/config" +require File.expand_path("test/config", __dir__) +require File.expand_path("test/support/config", __dir__) def run_without_aborting(*tasks) errors = [] @@ -134,7 +134,7 @@ task drop_postgresql_databases: "db:postgresql:drop" task rebuild_postgresql_databases: "db:postgresql:rebuild" task :lines do - load File.expand_path("..", File.dirname(__FILE__)) + "/tools/line_statistics" + load File.expand_path("../tools/line_statistics", __dir__) files = FileList["lib/active_record/**/*.rb"] CodeTools::LineStatistics.new(files).print_loc end diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 0b37e9076c..450ec0bba9 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -1,4 +1,4 @@ -version = File.read(File.expand_path("../../RAILS_VERSION", __FILE__)).strip +version = File.read(File.expand_path("../RAILS_VERSION", __dir__)).strip Gem::Specification.new do |s| s.platform = Gem::Platform::RUBY diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 96b8545dfc..29f49c6195 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -177,5 +177,5 @@ ActiveSupport.on_load(:active_record) do end ActiveSupport.on_load(:i18n) do - I18n.load_path << File.dirname(__FILE__) + "/active_record/locale/en.yml" + I18n.load_path << File.expand_path("active_record/locale/en.yml", __dir__) end diff --git a/activerecord/lib/rails/generators/active_record.rb b/activerecord/lib/rails/generators/active_record.rb index 68fca44e3b..a79b8eafea 100644 --- a/activerecord/lib/rails/generators/active_record.rb +++ b/activerecord/lib/rails/generators/active_record.rb @@ -10,7 +10,7 @@ module ActiveRecord # Set the current directory as base for the inherited generators. def self.base_root - File.dirname(__FILE__) + __dir__ end end end diff --git a/activerecord/test/cases/adapters/postgresql/bytea_test.rb b/activerecord/test/cases/adapters/postgresql/bytea_test.rb index 99175e8091..539c90f0bc 100644 --- a/activerecord/test/cases/adapters/postgresql/bytea_test.rb +++ b/activerecord/test/cases/adapters/postgresql/bytea_test.rb @@ -96,7 +96,7 @@ class PostgresqlByteaTest < ActiveRecord::PostgreSQLTestCase end def test_write_binary - data = File.read(File.join(File.dirname(__FILE__), "..", "..", "..", "assets", "example.log")) + data = File.read(File.join(__dir__, "..", "..", "..", "assets", "example.log")) assert(data.size > 1) record = ByteaDataType.create(payload: data) assert_not record.new_record? diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 8060790594..4bf1b5bcd5 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -954,7 +954,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_not_nil Developer._reflections["shared_computers"] # Checking the fixture for named association is important here, because it's the only way # we've been able to reproduce this bug - assert_not_nil File.read(File.expand_path("../../../fixtures/developers.yml", __FILE__)).index("shared_computers") + assert_not_nil File.read(File.expand_path("../../fixtures/developers.yml", __dir__)).index("shared_computers") assert_equal developers(:david).shared_computers.first, computers(:laptop) end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index d70572d6eb..b4bbdc6dad 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -316,7 +316,7 @@ class InheritanceTest < ActiveRecord::TestCase end def test_new_with_autoload_paths - path = File.expand_path("../../models/autoloadable", __FILE__) + path = File.expand_path("../models/autoloadable", __dir__) ActiveSupport::Dependencies.autoload_paths << path firm = Company.new(type: "ExtraFirm") diff --git a/activerecord/test/cases/reload_models_test.rb b/activerecord/test/cases/reload_models_test.rb index 5dc9d6d8b7..3f4c0c03e3 100644 --- a/activerecord/test/cases/reload_models_test.rb +++ b/activerecord/test/cases/reload_models_test.rb @@ -13,7 +13,7 @@ class ReloadModelsTest < ActiveRecord::TestCase # development environment. Note that meanwhile the class Pet is not # reloaded, simulating a class that is present in a plugin. Object.class_eval { remove_const :Owner } - Kernel.load(File.expand_path(File.join(File.dirname(__FILE__), "../models/owner.rb"))) + Kernel.load(File.expand_path("../models/owner.rb", __dir__)) pet = Pet.find_by_name("parrot") pet.owner = Owner.find_by_name("ashley") diff --git a/activerecord/test/cases/yaml_serialization_test.rb b/activerecord/test/cases/yaml_serialization_test.rb index ab0e67cd9d..bfc13d683d 100644 --- a/activerecord/test/cases/yaml_serialization_test.rb +++ b/activerecord/test/cases/yaml_serialization_test.rb @@ -123,8 +123,8 @@ class YamlSerializationTest < ActiveRecord::TestCase def yaml_fixture(file_name) path = File.expand_path( - "../../support/yaml_compatibility_fixtures/#{file_name}.yml", - __FILE__ + "../support/yaml_compatibility_fixtures/#{file_name}.yml", + __dir__ ) File.read(path) end diff --git a/activerecord/test/config.rb b/activerecord/test/config.rb index 6e2e8b2145..a65e6ff776 100644 --- a/activerecord/test/config.rb +++ b/activerecord/test/config.rb @@ -1,4 +1,4 @@ -TEST_ROOT = File.expand_path(File.dirname(__FILE__)) +TEST_ROOT = __dir__ ASSETS_ROOT = TEST_ROOT + "/assets" FIXTURES_ROOT = TEST_ROOT + "/fixtures" MIGRATIONS_ROOT = TEST_ROOT + "/migrations" -- cgit v1.2.3 From 694900ec1611fb792d611f4260d94c0e060627a3 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 24 May 2017 06:25:09 +0900 Subject: Enable extending even if scope returns nil --- activerecord/lib/active_record/scoping/named.rb | 12 ++++++------ activerecord/test/models/topic.rb | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 27cdf8cb7e..029156189d 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -156,17 +156,17 @@ module ActiveRecord if body.respond_to?(:to_proc) singleton_class.send(:define_method, name) do |*args| - scope = all.scoping { instance_exec(*args, &body) } + scope = all + scope = scope.scoping { instance_exec(*args, &body) || scope } scope = scope.extending(extension) if extension - - scope || all + scope end else singleton_class.send(:define_method, name) do |*args| - scope = all.scoping { body.call(*args) } + scope = all + scope = scope.scoping { body.call(*args) || scope } scope = scope.extending(extension) if extension - - scope || all + scope end end end diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 0420e2d15c..d9381ac9cf 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -14,7 +14,7 @@ class Topic < ActiveRecord::Base scope :replied, -> { where "replies_count > 0" } scope "approved_as_string", -> { where(approved: true) } - scope :anonymous_extension, -> { all } do + scope :anonymous_extension, -> {} do def one 1 end -- cgit v1.2.3 From bef95d372f9104a321cf17ac9f45316e9caac920 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 24 May 2017 15:21:02 +0900 Subject: Fix a RuboCop offences using `rubocop -a` --- activerecord/lib/active_record/integration.rb | 1 - activerecord/lib/active_record/tasks/database_tasks.rb | 6 +++--- activerecord/test/cases/associations/callbacks_test.rb | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 441237da1e..32362fa86f 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -110,7 +110,6 @@ module ActiveRecord end end - module ClassMethods # Defines your model's +to_param+ method to generate "pretty" URLs # using +method_name+, which can be any attribute or method that diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index c53dee43d7..ba686fc562 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -71,9 +71,9 @@ module ActiveRecord @tasks[pattern] = task end - register_task(/mysql/, 'ActiveRecord::Tasks::MySQLDatabaseTasks') - register_task(/postgresql/, 'ActiveRecord::Tasks::PostgreSQLDatabaseTasks') - register_task(/sqlite/, 'ActiveRecord::Tasks::SQLiteDatabaseTasks') + register_task(/mysql/, "ActiveRecord::Tasks::MySQLDatabaseTasks") + register_task(/postgresql/, "ActiveRecord::Tasks::PostgreSQLDatabaseTasks") + register_task(/sqlite/, "ActiveRecord::Tasks::SQLiteDatabaseTasks") def db_dir @db_dir ||= Rails.application.config.paths["db"].first diff --git a/activerecord/test/cases/associations/callbacks_test.rb b/activerecord/test/cases/associations/callbacks_test.rb index 7721bd5cd9..f9d1e44595 100644 --- a/activerecord/test/cases/associations/callbacks_test.rb +++ b/activerecord/test/cases/associations/callbacks_test.rb @@ -128,7 +128,7 @@ class AssociationCallbacksTest < ActiveRecord::TestCase assert ar.developers_log.empty? alice = Developer.new(name: "alice") ar.developers_with_callbacks << alice - assert_equal"after_adding#{alice.id}", ar.developers_log.last + assert_equal "after_adding#{alice.id}", ar.developers_log.last bob = ar.developers_with_callbacks.create(name: "bob") assert_equal "after_adding#{bob.id}", ar.developers_log.last -- cgit v1.2.3 From f3f652827f1db772741d56664cd3fb583873d0cd Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 23 May 2017 00:11:52 +0900 Subject: Fix crashing on circular left join references with scoping Follow up of #25702. --- activerecord/lib/active_record/reflection.rb | 2 +- activerecord/test/cases/scoping/relation_scoping_test.rb | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 1a9e0a4a40..65fdbc2fe4 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -199,7 +199,7 @@ module ActiveRecord def klass_join_scope(table, predicate_builder) # :nodoc: if klass.current_scope klass.current_scope.clone.tap { |scope| - scope.joins_values = [] + scope.joins_values = scope.left_outer_joins_values = [].freeze } else relation = ActiveRecord::Relation.create( diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index 3fbff7664b..8535be8402 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -229,12 +229,19 @@ class RelationScopingTest < ActiveRecord::TestCase end end - def test_circular_joins_with_current_scope_does_not_crash + def test_circular_joins_with_scoping_does_not_crash posts = Post.joins(comments: :post).scoping do - Post.current_scope.first(10) + Post.first(10) end assert_equal posts, Post.joins(comments: :post).first(10) end + + def test_circular_left_joins_with_scoping_does_not_crash + posts = Post.left_joins(comments: :post).scoping do + Post.first(10) + end + assert_equal posts, Post.left_joins(comments: :post).first(10) + end end class NestedRelationScopingTest < ActiveRecord::TestCase -- cgit v1.2.3 From 8ace8c76a4d90cca29ede40dbc7f81351eb2e137 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 25 May 2017 00:00:08 +0900 Subject: `DEFAULT_ENV` falls back to `default_env` when `RAILS_ENV` or `RACK_ENV` is an empty string Follow up of #27399. --- activerecord/lib/active_record/connection_handling.rb | 2 +- .../test/cases/connection_adapters/connection_handler_test.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 2ede92feff..b8fbb489b6 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -1,6 +1,6 @@ module ActiveRecord module ConnectionHandling - RAILS_ENV = -> { (Rails.env if defined?(Rails.env)) || ENV["RAILS_ENV"] || ENV["RACK_ENV"] } + RAILS_ENV = -> { (Rails.env if defined?(Rails.env)) || ENV["RAILS_ENV"].presence || ENV["RACK_ENV"].presence } DEFAULT_ENV = -> { RAILS_ENV.call || "default_env" } # Establishes the connection to the database. Accepts a hash as input where diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 681399c8bb..2a71f08d90 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -9,6 +9,17 @@ module ActiveRecord @pool = @handler.establish_connection(ActiveRecord::Base.configurations["arunit"]) end + def test_default_env_fall_back_to_default_env_when_rails_env_or_rack_env_is_empty_string + original_rails_env = ENV["RAILS_ENV"] + original_rack_env = ENV["RACK_ENV"] + ENV["RAILS_ENV"] = ENV["RACK_ENV"] = "" + + assert_equal "default_env", ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + ensure + ENV["RAILS_ENV"] = original_rails_env + ENV["RACK_ENV"] = original_rack_env + end + def test_establish_connection_uses_spec_name config = { "readonly" => { "adapter" => "sqlite3" } } resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(config) -- cgit v1.2.3 From a24912cb1d34912a16aa27d952beff825e558f1f Mon Sep 17 00:00:00 2001 From: Michael Lovitt Date: Thu, 18 May 2017 15:38:30 -0500 Subject: Performance optimization for AttributeSet#deep_dup Skip the call to #dup, since it does a shallow copy of attributes, which is wasted effort, since #deep_dup then replaces that shallow copy with a #deep_dup of the given attributes. This change addresses slowness in ActiveRecord initialization introduced starting in Rails 5.0. --- activerecord/lib/active_record/attribute_set.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index 66b278219a..abe8e14f26 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -64,9 +64,7 @@ module ActiveRecord end def deep_dup - dup.tap do |copy| - copy.instance_variable_set(:@attributes, attributes.deep_dup) - end + self.class.new(attributes.deep_dup) end def initialize_dup(_) -- cgit v1.2.3 From bfc62febac905412cdbcb7698d5a3b3ea5167af3 Mon Sep 17 00:00:00 2001 From: Michael Lovitt Date: Thu, 18 May 2017 15:43:49 -0500 Subject: Performance optimization for ActiveRecord#subclass_from_attributes This change addresses slowness in ActiveRecord initialization introduced starting in Rails 5.0. --- activerecord/lib/active_record/inheritance.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index fbdaeaae51..236a65eba7 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -217,7 +217,7 @@ module ActiveRecord def subclass_from_attributes(attrs) attrs = attrs.to_h if attrs.respond_to?(:permitted?) if attrs.is_a?(Hash) - subclass_name = attrs.with_indifferent_access[inheritance_column] + subclass_name = attrs[inheritance_column] || attrs[inheritance_column.to_sym] if subclass_name.present? find_sti_class(subclass_name) -- cgit v1.2.3 From 63dd12b7b83541d8a469a8e6aed1607d77f0d994 Mon Sep 17 00:00:00 2001 From: Michael Lovitt Date: Thu, 18 May 2017 15:52:45 -0500 Subject: Performance optimization for ActiveRecord#column_defaults Memoize the #column_defaults class property, as ActiveRecord does for other properties in this module. This change addresses slowness in ActiveRecord initialization introduced starting in Rails 5.0. This method's performance has not changed with Rails 5, but it is now called much more frequently than before: every time an STI model is instantiated. --- activerecord/lib/active_record/model_schema.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 54216caaaf..aa9087fae3 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -377,7 +377,7 @@ module ActiveRecord # default values when instantiating the Active Record object for this table. def column_defaults load_schema - _default_attributes.to_hash + @column_defaults ||= _default_attributes.to_hash end def _default_attributes # :nodoc: @@ -466,6 +466,7 @@ module ActiveRecord @attribute_types = nil @content_columns = nil @default_attributes = nil + @column_defaults = nil @inheritance_column = nil unless defined?(@explicit_inheritance_column) && @explicit_inheritance_column @attributes_builder = nil @columns = nil -- cgit v1.2.3 From 6a5a814eaa0d4f98d6151df5bd96cc8ec1ae5675 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Thu, 25 May 2017 09:08:45 +0930 Subject: Add a Monitor to ModelSchema#load_schema [Vikrant Chaudhary, David Abdemoulaie, Matthew Draper] --- activerecord/CHANGELOG.md | 6 +++++ activerecord/lib/active_record/model_schema.rb | 25 ++++++++++++++++--- .../test/cases/serialized_attribute_test.rb | 28 ++++++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 907dd894cd..d17bbf80ca 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Loading model schema from database is now thread-safe. + + Fixes #28589. + + *Vikrant Chaudhary*, *David Abdemoulaie* + * Add `ActiveRecord::Base#cache_version` to support recyclable cache keys via the new versioned entries in `ActiveSupport::Cache`. This also means that `ActiveRecord::Base#cache_key` will now return a stable key that does not include a timestamp any more. diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 54216caaaf..5095cdfe9f 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -1,3 +1,5 @@ +require "monitor" + module ActiveRecord module ModelSchema extend ActiveSupport::Concern @@ -152,6 +154,8 @@ module ActiveRecord self.inheritance_column = "type" delegate :type_for_attribute, to: :class + + initialize_load_schema_monitor end # Derives the join table name for +first_table+ and +second_table+. The @@ -435,15 +439,27 @@ module ActiveRecord initialize_find_by_cache end + protected + + def initialize_load_schema_monitor + @load_schema_monitor = Monitor.new + end + private + def inherited(child_class) + super + child_class.initialize_load_schema_monitor + end + def schema_loaded? - defined?(@columns_hash) && @columns_hash + defined?(@schema_loaded) && @schema_loaded end def load_schema - unless schema_loaded? - load_schema! + return if schema_loaded? + @load_schema_monitor.synchronize do + load_schema! unless defined?(@columns_hash) && @columns_hash end end @@ -457,6 +473,8 @@ module ActiveRecord user_provided_default: false ) end + + @schema_loaded = true end def reload_schema_from_cache @@ -470,6 +488,7 @@ module ActiveRecord @attributes_builder = nil @columns = nil @columns_hash = nil + @schema_loaded = false @attribute_names = nil @yaml_encoder = nil direct_descendants.each do |descendant| diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index 673392b4c4..e1bdaab5cf 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -349,4 +349,32 @@ class SerializedAttributeTest < ActiveRecord::TestCase topic.foo refute topic.changed? end + + def test_serialized_attribute_works_under_concurrent_initial_access + model = Topic.dup + + topic = model.last + topic.update group: "1" + + model.serialize :group, JSON + model.reset_column_information + + # This isn't strictly necessary for the test, but a little bit of + # knowledge of internals allows us to make failures far more likely. + model.define_singleton_method(:define_attribute) do |*args| + Thread.pass + super(*args) + end + + threads = 4.times.map do + Thread.new do + topic.reload.group + end + end + + # All the threads should retrieve the value knowing it is JSON, and + # thus decode it. If this fails, some threads will instead see the + # raw string ("1"), or raise an exception. + assert_equal [1] * threads.size, threads.map(&:value) + end end -- cgit v1.2.3 From f3d3f5e9e774e5c75e97a099676e35942747a7fd Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 25 May 2017 18:22:09 +0900 Subject: Remove a duplicate test of migration_test in AR --- activerecord/test/cases/migration_test.rb | 27 --------------------------- 1 file changed, 27 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index da7875187a..57f94950f9 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -402,33 +402,6 @@ class MigrationTest < ActiveRecord::TestCase ActiveRecord::Migrator.up(migrations_path) end - def test_migration_sets_internal_metadata_even_when_fully_migrated - current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call - migrations_path = MIGRATIONS_ROOT + "/valid" - old_path = ActiveRecord::Migrator.migrations_paths - ActiveRecord::Migrator.migrations_paths = migrations_path - - ActiveRecord::Migrator.up(migrations_path) - assert_equal current_env, ActiveRecord::InternalMetadata[:environment] - - original_rails_env = ENV["RAILS_ENV"] - original_rack_env = ENV["RACK_ENV"] - ENV["RAILS_ENV"] = ENV["RACK_ENV"] = "foofoo" - new_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call - - refute_equal current_env, new_env - - sleep 1 # mysql by default does not store fractional seconds in the database - - ActiveRecord::Migrator.up(migrations_path) - assert_equal new_env, ActiveRecord::InternalMetadata[:environment] - ensure - ActiveRecord::Migrator.migrations_paths = old_path - ENV["RAILS_ENV"] = original_rails_env - ENV["RACK_ENV"] = original_rack_env - ActiveRecord::Migrator.up(migrations_path) - end - def test_internal_metadata_stores_environment_when_other_data_exists ActiveRecord::InternalMetadata.delete_all ActiveRecord::InternalMetadata[:foo] = "bar" -- cgit v1.2.3 From 3b86a59b26b9fed14025a41d10453fae9059f3f0 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 25 May 2017 21:54:18 +0900 Subject: Remove a duplicate test of schema_authorization_test in AR --- .../cases/adapters/postgresql/schema_authorization_test.rb | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb index bf570176f4..f86a76e08a 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb @@ -75,17 +75,6 @@ class SchemaAuthorizationTest < ActiveRecord::PostgreSQLTestCase end end - def test_schema_uniqueness - assert_nothing_raised do - set_session_auth - USERS.each do |u| - set_session_auth u - assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = 1") - set_session_auth - end - end - end - def test_sequence_schema_caching assert_nothing_raised do USERS.each do |u| -- cgit v1.2.3 From 9862b3bf1e443703df4472a5c098aaeb205096d7 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 25 May 2017 22:18:31 +0900 Subject: Remove a duplicate test of mysql_rake_test --- activerecord/test/cases/tasks/mysql_rake_test.rb | 7 ------- 1 file changed, 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 33da3d11fc..c22d974536 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -294,13 +294,6 @@ if current_adapter?(:Mysql2Adapter) ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) end - def test_structure_dump - filename = "awesome-file.sql" - Kernel.expects(:system).with("mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "test-db").returns(true) - - ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) - end - def test_structure_dump_with_extra_flags filename = "awesome-file.sql" expected_command = ["mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "--noop", "test-db"] -- cgit v1.2.3 From fb6ccce80663dad034d86c472024076a8348a12f Mon Sep 17 00:00:00 2001 From: Michael Lovitt Date: Thu, 25 May 2017 17:14:33 -0500 Subject: Make #deep_dup use #allocate instead of #new This change preserves the speedup made in a24912cb1d3 (by avoiding the wasted shallow dup of @attributes) while ensuring that the performance of #deep_dup won't be tied to the performance of #initialize --- activerecord/lib/active_record/attribute_set.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index abe8e14f26..01f9d815d5 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -64,7 +64,9 @@ module ActiveRecord end def deep_dup - self.class.new(attributes.deep_dup) + self.class.allocate.tap do |copy| + copy.instance_variable_set(:@attributes, attributes.deep_dup) + end end def initialize_dup(_) -- cgit v1.2.3 From 7a2fd70fc46ea7537a814172677adea86baf3747 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 26 May 2017 15:23:37 +0930 Subject: Avoid circular require due to autoload --- .../lib/active_record/relation/predicate_builder.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 183fe91c05..a6309e0b5c 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -1,12 +1,3 @@ -require "active_record/relation/predicate_builder/array_handler" -require "active_record/relation/predicate_builder/base_handler" -require "active_record/relation/predicate_builder/basic_object_handler" -require "active_record/relation/predicate_builder/range_handler" -require "active_record/relation/predicate_builder/relation_handler" - -require "active_record/relation/predicate_builder/association_query_value" -require "active_record/relation/predicate_builder/polymorphic_array_value" - module ActiveRecord class PredicateBuilder # :nodoc: delegate :resolve_column_aliases, to: :table @@ -178,3 +169,12 @@ module ActiveRecord end end end + +require "active_record/relation/predicate_builder/array_handler" +require "active_record/relation/predicate_builder/base_handler" +require "active_record/relation/predicate_builder/basic_object_handler" +require "active_record/relation/predicate_builder/range_handler" +require "active_record/relation/predicate_builder/relation_handler" + +require "active_record/relation/predicate_builder/association_query_value" +require "active_record/relation/predicate_builder/polymorphic_array_value" -- cgit v1.2.3 From 235836ae58605931f72ecaef95363aa6784eb977 Mon Sep 17 00:00:00 2001 From: Mohit Natoo Date: Fri, 26 May 2017 18:30:21 +0700 Subject: [ci skip] Changed sentence formation for ActiveRecordRelation#update --- activerecord/lib/active_record/relation.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 5775eda5a5..af9be9875e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -404,9 +404,9 @@ module ActiveRecord # # Note: Updating a large number of records will run an # UPDATE query for each record, which may cause a performance - # issue. So if it is not needed to run callbacks for each update, it is - # preferred to use #update_all for updating all records using - # a single query. + # issue. When running callbacks is not needed for each record update, + # it is preferred to use #update_all for updating all records + # in a single query. def update(id = :all, attributes) if id.is_a?(Array) id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) } -- cgit v1.2.3 From 7a80ab54393b500ae08cc1cf93c93ece89097112 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 27 May 2017 01:58:34 +0900 Subject: Remove a redundant test assertion --- activerecord/test/cases/finder_test.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index a7b6333010..4837a169fa 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -743,7 +743,6 @@ class FinderTest < ActiveRecord::TestCase assert Topic.where(author_name: "David", title: "The First Topic", replies_count: 1, approved: false).find(1) assert_raise(ActiveRecord::RecordNotFound) { Topic.where(author_name: "David", title: "The First Topic", replies_count: 1, approved: true).find(1) } assert_raise(ActiveRecord::RecordNotFound) { Topic.where(author_name: "David", title: "HHC", replies_count: 1, approved: false).find(1) } - assert_raise(ActiveRecord::RecordNotFound) { Topic.where(author_name: "David", title: "The First Topic", replies_count: 1, approved: true).find(1) } end def test_condition_interpolation -- cgit v1.2.3 From 1fb48e4d9908a17664470ed56919d9f87fa3dfe8 Mon Sep 17 00:00:00 2001 From: "T.J. Schuck" Date: Fri, 26 May 2017 18:12:26 -0400 Subject: Doc updates for ActiveRecord::Batches [ci skip] --- activerecord/lib/active_record/relation/batches.rb | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 76031515fd..13a2c3f511 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -30,14 +30,14 @@ module ActiveRecord # end # # ==== Options - # * :batch_size - Specifies the size of the batch. Default to 1000. + # * :batch_size - Specifies the size of the batch. Defaults to 1000. # * :start - Specifies the primary key value to start from, inclusive of the value. # * :finish - Specifies the primary key value to end at, inclusive of the value. # * :error_on_ignore - Overrides the application config to specify if an error should be raised when - # an order is present in the relation. + # an order is present in the relation. # # Limits are honored, and if present there is no requirement for the batch - # size, it can be less than, equal, or greater than the limit. + # size: it can be less than, equal to, or greater than the limit. # # The options +start+ and +finish+ are especially useful if you want # multiple workers dealing with the same processing queue. You can make @@ -89,14 +89,14 @@ module ActiveRecord # To be yielded each record one by one, use #find_each instead. # # ==== Options - # * :batch_size - Specifies the size of the batch. Default to 1000. + # * :batch_size - Specifies the size of the batch. Defaults to 1000. # * :start - Specifies the primary key value to start from, inclusive of the value. # * :finish - Specifies the primary key value to end at, inclusive of the value. # * :error_on_ignore - Overrides the application config to specify if an error should be raised when - # an order is present in the relation. + # an order is present in the relation. # # Limits are honored, and if present there is no requirement for the batch - # size, it can be less than, equal, or greater than the limit. + # size: it can be less than, equal to, or greater than the limit. # # The options +start+ and +finish+ are especially useful if you want # multiple workers dealing with the same processing queue. You can make @@ -140,9 +140,9 @@ module ActiveRecord # If you do not provide a block to #in_batches, it will return a # BatchEnumerator which is enumerable. # - # Person.in_batches.with_index do |relation, batch_index| + # Person.in_batches.each_with_index do |relation, batch_index| # puts "Processing relation ##{batch_index}" - # relation.each { |relation| relation.delete_all } + # relation.delete_all # end # # Examples of calling methods on the returned BatchEnumerator object: @@ -152,12 +152,12 @@ module ActiveRecord # Person.in_batches.each_record(&:party_all_night!) # # ==== Options - # * :of - Specifies the size of the batch. Default to 1000. - # * :load - Specifies if the relation should be loaded. Default to false. + # * :of - Specifies the size of the batch. Defaults to 1000. + # * :load - Specifies if the relation should be loaded. Defaults to false. # * :start - Specifies the primary key value to start from, inclusive of the value. # * :finish - Specifies the primary key value to end at, inclusive of the value. # * :error_on_ignore - Overrides the application config to specify if an error should be raised when - # an order is present in the relation. + # an order is present in the relation. # # Limits are honored, and if present there is no requirement for the batch # size, it can be less than, equal, or greater than the limit. @@ -186,7 +186,7 @@ module ActiveRecord # # NOTE: It's not possible to set the order. That is automatically set to # ascending on the primary key ("id ASC") to make the batch ordering - # consistent. Therefore the primary key must be orderable, e.g an integer + # consistent. Therefore the primary key must be orderable, e.g. an integer # or a string. # # NOTE: By its nature, batch processing is subject to race conditions if -- cgit v1.2.3 From d83b8e65102d625c9024cd9a2727a10b0ef83b79 Mon Sep 17 00:00:00 2001 From: Bradley Priest Date: Sat, 27 May 2017 21:47:07 +0800 Subject: Fix regression in Numericality validator where extra decimal places on a user input for a decimal column were ignored by numerically validations --- activerecord/test/cases/validations_test.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 5d9aa99497..a305aa295a 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -167,6 +167,20 @@ class ValidationsTest < ActiveRecord::TestCase assert topic.valid? end + def test_numericality_validation_checks_against_raw_value + klass = Class.new(Topic) do + def self.model_name + 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") + 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? + end + def test_acceptance_validator_doesnt_require_db_connection klass = Class.new(ActiveRecord::Base) do self.table_name = "posts" -- cgit v1.2.3 From 69dc57cd18bdda82e599de316b080dfc676e0987 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 28 May 2017 02:05:57 +0900 Subject: Remove unused `Association#interpolate` Using `Association#interpolate` was removed since #11251. --- activerecord/lib/active_record/associations/association.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 1cb2b2d7c6..6b13e6936f 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -152,14 +152,6 @@ module ActiveRecord reset end - def interpolate(sql, record = nil) - if sql.respond_to?(:to_proc) - owner.instance_exec(record, &sql) - else - sql - end - end - # We can't dump @reflection since it contains the scope proc def marshal_dump ivars = (instance_variables - [:@reflection]).map { |name| [name, instance_variable_get(name)] } -- cgit v1.2.3 From b731f598fc076acbcaeb273636e646dfebeef796 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 28 May 2017 13:27:41 +0900 Subject: Prevent extra `current_database` query for `encoding`/`collation`/`ctype` --- .../connection_adapters/postgresql/schema_statements.rb | 6 +++--- .../test/cases/adapters/postgresql/connection_test.rb | 12 +++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 5b483ad4ab..3eff9e2f83 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -186,17 +186,17 @@ module ActiveRecord # Returns the current database encoding format. def encoding - select_value("SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname LIKE '#{current_database}'", "SCHEMA") + select_value("SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = current_database()", "SCHEMA") end # Returns the current database collation. def collation - select_value("SELECT datcollate FROM pg_database WHERE datname LIKE '#{current_database}'", "SCHEMA") + select_value("SELECT datcollate FROM pg_database WHERE datname = current_database()", "SCHEMA") end # Returns the current database ctype. def ctype - select_value("SELECT datctype FROM pg_database WHERE datname LIKE '#{current_database}'", "SCHEMA") + select_value("SELECT datctype FROM pg_database WHERE datname = current_database()", "SCHEMA") end # Returns an array of schema names. diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index 3deb007513..32afe331fa 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -31,15 +31,21 @@ module ActiveRecord end def test_encoding - assert_not_nil @connection.encoding + assert_queries(1) do + assert_not_nil @connection.encoding + end end def test_collation - assert_not_nil @connection.collation + assert_queries(1) do + assert_not_nil @connection.collation + end end def test_ctype - assert_not_nil @connection.ctype + assert_queries(1) do + assert_not_nil @connection.ctype + end end def test_default_client_min_messages -- cgit v1.2.3 From 13515637aa0d6ea16e805e4475d6544918c57e68 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 27 May 2017 02:34:38 +0900 Subject: Remove a redundant test case of command_recorder_test --- activerecord/test/cases/migration/command_recorder_test.rb | 5 ----- 1 file changed, 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 802a969cb7..007926f1b9 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -211,11 +211,6 @@ module ActiveRecord assert_equal [:remove_index, [:table, { name: "new_index" }]], remove end - def test_invert_add_index_with_no_options - remove = @recorder.inverse_of :add_index, [:table, [:one, :two]] - assert_equal [:remove_index, [:table, { column: [:one, :two] }]], remove - end - def test_invert_remove_index add = @recorder.inverse_of :remove_index, [:table, :one] assert_equal [:add_index, [:table, :one]], add -- cgit v1.2.3 From 36417cf07790b645b3f3c0fde095d1059bcf0ea9 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 29 May 2017 09:02:45 +0900 Subject: Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations` `select`, `count`, and `sum` in `Relation` are also `Enumerable` method that can be passed block. `select` with block already doesn't take arguments since 4fc3366. This is follow up of that. --- activerecord/CHANGELOG.md | 4 ++++ .../lib/active_record/relation/calculations.rb | 22 ++++++++++++++++++++-- activerecord/test/cases/calculations_test.rb | 12 ++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d17bbf80ca..1f8163db12 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations`. + + *Ryuta Kamizono* + * Loading model schema from database is now thread-safe. Fixes #28589. diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 861b2125d4..c562f214c9 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -37,7 +37,16 @@ module ActiveRecord # Note: not all valid {Relation#select}[rdoc-ref:QueryMethods#select] expressions are valid #count expressions. The specifics differ # between databases. In invalid cases, an error from the database is thrown. def count(column_name = nil) - return super() if block_given? + if block_given? + unless column_name.nil? + ActiveSupport::Deprecation.warn \ + "When `count' is called with a block, it ignores other arguments. " \ + "This behavior is now deprecated and will result in an ArgumentError in Rails 5.3." + end + + return super() + end + calculate(:count, column_name) end @@ -73,7 +82,16 @@ module ActiveRecord # # Person.sum(:age) # => 4562 def sum(column_name = nil) - return super() if block_given? + if block_given? + unless column_name.nil? + ActiveSupport::Deprecation.warn \ + "When `sum' is called with a block, it ignores other arguments. " \ + "This behavior is now deprecated and will result in an ArgumentError in Rails 5.3." + end + + return super() + end + calculate(:sum, column_name) end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 3214d778d4..93f8ab18c2 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -809,4 +809,16 @@ class CalculationsTest < ActiveRecord::TestCase def test_group_by_attribute_with_custom_type assert_equal({ "proposed" => 2, "published" => 2 }, Book.group(:status).count) end + + def test_deprecate_count_with_block_and_column_name + assert_deprecated do + assert_equal 6, Account.count(:firm_id) { true } + end + end + + def test_deprecate_sum_with_block_and_column_name + assert_deprecated do + assert_equal 6, Account.sum(:firm_id) { 1 } + end + end end -- cgit v1.2.3 From de387ea482bd72bab9e81db47cf7a5991f25117c Mon Sep 17 00:00:00 2001 From: Yaw Boakye Date: Sat, 27 May 2017 13:01:03 +0000 Subject: `rename_table` renames primary key index name Formerly, `rename_table` only renamed primary key index name if the column's data type was sequential (serial, etc in PostgreSQL). The problem with that is tables whose primary keys had other data types (e.g. UUID) maintained the old primary key name. So for example, if the `cats` table has a UUID primary key, and the table is renamed to `felines`, the primary key index will still be called `cats_pkey` instead of `felines_pkey`. This PR corrects it. --- .../postgresql/schema_statements.rb | 9 +++++---- .../test/cases/migration/rename_table_test.rb | 23 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 3eff9e2f83..55566f1e34 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -377,14 +377,15 @@ module ActiveRecord clear_cache! execute "ALTER TABLE #{quote_table_name(table_name)} RENAME TO #{quote_table_name(new_name)}" pk, seq = pk_and_sequence_for(new_name) - if seq && seq.identifier == "#{table_name}_#{pk}_seq" - new_seq = "#{new_name}_#{pk}_seq" + if pk idx = "#{table_name}_pkey" new_idx = "#{new_name}_pkey" - execute "ALTER TABLE #{seq.quoted} RENAME TO #{quote_table_name(new_seq)}" execute "ALTER INDEX #{quote_table_name(idx)} RENAME TO #{quote_table_name(new_idx)}" + if seq && seq.identifier == "#{table_name}_#{pk}_seq" + new_seq = "#{new_name}_#{pk}_seq" + execute "ALTER TABLE #{seq.quoted} RENAME TO #{quote_table_name(new_seq)}" + end end - rename_table_indexes(table_name, new_name) end diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb index 7bcabd0cc6..5da3ad33a3 100644 --- a/activerecord/test/cases/migration/rename_table_test.rb +++ b/activerecord/test/cases/migration/rename_table_test.rb @@ -79,10 +79,33 @@ module ActiveRecord assert_equal ConnectionAdapters::PostgreSQL::Name.new("public", "octopi_#{pk}_seq"), seq end + def test_renaming_table_renames_primary_key + connection.create_table :cats, id: :uuid, default: "uuid_generate_v4()" + rename_table :cats, :felines + + assert connection.table_exists? :felines + refute connection.table_exists? :cats + + primary_key_name = connection.select_values(<<-SQL.strip_heredoc, "SCHEMA")[0] + SELECT c.relname + FROM pg_class c + JOIN pg_index i + ON c.oid = i.indexrelid + WHERE i.indisprimary + AND i.indrelid = 'felines'::regclass + SQL + + assert_equal "felines_pkey", primary_key_name + ensure + connection.drop_table :cats, if_exists: true + connection.drop_table :felines, if_exists: true + end + def test_renaming_table_doesnt_attempt_to_rename_non_existent_sequences connection.create_table :cats, id: :uuid, default: "uuid_generate_v4()" assert_nothing_raised { rename_table :cats, :felines } assert connection.table_exists? :felines + refute connection.table_exists? :cats ensure connection.drop_table :cats, if_exists: true connection.drop_table :felines, if_exists: true -- cgit v1.2.3 From e49b7dea48e3c5011c8a5cd0256e96251e5798f0 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 29 May 2017 20:04:24 +0900 Subject: Remove a redundant test assertion --- activerecord/test/cases/associations/eager_test.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 36238dd088..4271a09c9b 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1093,12 +1093,6 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author } - posts = assert_queries(2) do - Post.all.merge!(select: "distinct posts.*", includes: :author, joins: [:comments], where: "comments.body like 'Thank you%'", order: "posts.id").to_a - end - assert_equal [posts(:welcome)], posts - assert_equal authors(:david), assert_no_queries { posts[0].author } - posts = assert_queries(2) do Post.all.merge!(includes: :author, joins: { taggings: :tag }, where: "tags.name = 'General'", order: "posts.id").to_a end -- cgit v1.2.3 From 1c275d812f35f53f93cd96184a4f319983766cc5 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 29 May 2017 18:01:50 +0200 Subject: Add option for class_attribute default (#29270) * Allow a default value to be declared for class_attribute * Convert to using class_attribute default rather than explicit setter * Removed instance_accessor option by mistake * False is a valid default value * Documentation --- .../lib/active_record/attribute_decorators.rb | 3 +-- .../lib/active_record/attribute_methods/dirty.rb | 3 +-- .../attribute_methods/time_zone_conversion.rb | 7 ++---- activerecord/lib/active_record/attributes.rb | 3 +-- .../connection_adapters/abstract_mysql_adapter.rb | 3 +-- activerecord/lib/active_record/enum.rb | 3 +-- activerecord/lib/active_record/fixtures.rb | 20 +++++----------- activerecord/lib/active_record/inheritance.rb | 3 +-- activerecord/lib/active_record/integration.rb | 6 ++--- .../lib/active_record/locking/optimistic.rb | 3 +-- activerecord/lib/active_record/model_schema.rb | 27 ++++++---------------- .../lib/active_record/nested_attributes.rb | 3 +-- .../lib/active_record/readonly_attributes.rb | 3 +-- activerecord/lib/active_record/reflection.rb | 6 ++--- activerecord/lib/active_record/scoping/default.rb | 7 ++---- activerecord/lib/active_record/timestamp.rb | 3 +-- 16 files changed, 31 insertions(+), 72 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_decorators.rb b/activerecord/lib/active_record/attribute_decorators.rb index c39e9ce4c5..5bc8527745 100644 --- a/activerecord/lib/active_record/attribute_decorators.rb +++ b/activerecord/lib/active_record/attribute_decorators.rb @@ -3,8 +3,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :attribute_type_decorations, instance_accessor: false # :internal: - self.attribute_type_decorations = TypeDecorator.new + class_attribute :attribute_type_decorations, instance_accessor: false, default: TypeDecorator.new # :internal: end module ClassMethods # :nodoc: diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index bd5003d63a..76987fb8f4 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -14,8 +14,7 @@ module ActiveRecord raise "You cannot include Dirty after Timestamp" end - class_attribute :partial_writes, instance_writer: false - self.partial_writes = true + class_attribute :partial_writes, instance_writer: false, default: true after_create { changes_internally_applied } after_update { changes_internally_applied } diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 321d039ed4..4a8d231503 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -57,11 +57,8 @@ module ActiveRecord mattr_accessor :time_zone_aware_attributes, instance_writer: false self.time_zone_aware_attributes = false - class_attribute :skip_time_zone_conversion_for_attributes, instance_writer: false - self.skip_time_zone_conversion_for_attributes = [] - - class_attribute :time_zone_aware_types, instance_writer: false - self.time_zone_aware_types = [:datetime, :time] + class_attribute :skip_time_zone_conversion_for_attributes, instance_writer: false, default: [] + class_attribute :time_zone_aware_types, instance_writer: false, default: [ :datetime, :time ] end module ClassMethods diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 75f5ba3a96..475b9beec4 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -6,8 +6,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :attributes_to_define_after_schema_loads, instance_accessor: false # :internal: - self.attributes_to_define_after_schema_loads = {} + class_attribute :attributes_to_define_after_schema_loads, instance_accessor: false, default: {} # :internal: end module ClassMethods 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 abc15f595f..9cb7c46df5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -29,8 +29,7 @@ module ActiveRecord # to your application.rb file: # # ActiveRecord::ConnectionAdapters::Mysql2Adapter.emulate_booleans = false - class_attribute :emulate_booleans - self.emulate_booleans = true + class_attribute :emulate_booleans, default: true NATIVE_DATABASE_TYPES = { primary_key: "bigint auto_increment PRIMARY KEY", diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 496abfc5d9..12ef58a941 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -95,8 +95,7 @@ module ActiveRecord module Enum def self.extended(base) # :nodoc: - base.class_attribute(:defined_enums, instance_writer: false) - base.defined_enums = {} + base.class_attribute(:defined_enums, instance_writer: false, default: {}) end def inherited(base) # :nodoc: diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index c19216702c..bad6542be2 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -878,20 +878,12 @@ module ActiveRecord included do class_attribute :fixture_path, instance_writer: false - class_attribute :fixture_table_names - class_attribute :fixture_class_names - class_attribute :use_transactional_tests - class_attribute :use_instantiated_fixtures # true, false, or :no_instances - class_attribute :pre_loaded_fixtures - class_attribute :config - - self.fixture_table_names = [] - self.use_instantiated_fixtures = false - self.pre_loaded_fixtures = false - self.config = ActiveRecord::Base - - self.fixture_class_names = {} - self.use_transactional_tests = true + class_attribute :fixture_table_names, default: [] + class_attribute :fixture_class_names, default: {} + class_attribute :use_transactional_tests, default: true + class_attribute :use_instantiated_fixtures, default: false # true, false, or :no_instances + class_attribute :pre_loaded_fixtures, default: false + class_attribute :config, default: ActiveRecord::Base end module ClassMethods diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 236a65eba7..5776807507 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -38,8 +38,7 @@ module ActiveRecord included do # Determines whether to store the full constant name including namespace when using STI. # This is true, by default. - class_attribute :store_full_sti_class, instance_writer: false - self.store_full_sti_class = true + class_attribute :store_full_sti_class, instance_writer: false, default: true end module ClassMethods diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 32362fa86f..cf954852bc 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -11,8 +11,7 @@ module ActiveRecord # versioning is off. Accepts any of the symbols in Time::DATE_FORMATS. # # This is +:usec+, by default. - class_attribute :cache_timestamp_format, instance_writer: false - self.cache_timestamp_format = :usec + class_attribute :cache_timestamp_format, instance_writer: false, default: :usec ## # :singleton-method: @@ -20,8 +19,7 @@ module ActiveRecord # by a changing version in the #cache_version method. # # This is +false+, by default until Rails 6.0. - class_attribute :cache_versioning, instance_writer: false - self.cache_versioning = false + class_attribute :cache_versioning, instance_writer: false, default: false end # Returns a +String+, which Action Pack uses for constructing a URL to this diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 78ce9f8291..3c7110369b 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -51,8 +51,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :lock_optimistically, instance_writer: false - self.lock_optimistically = true + class_attribute :lock_optimistically, instance_writer: false, default: true end def locking_enabled? #:nodoc: diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 013562708c..1179a60e9b 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -130,26 +130,13 @@ module ActiveRecord included do mattr_accessor :primary_key_prefix_type, instance_writer: false - class_attribute :table_name_prefix, instance_writer: false - self.table_name_prefix = "" - - class_attribute :table_name_suffix, instance_writer: false - self.table_name_suffix = "" - - class_attribute :schema_migrations_table_name, instance_accessor: false - self.schema_migrations_table_name = "schema_migrations" - - class_attribute :internal_metadata_table_name, instance_accessor: false - self.internal_metadata_table_name = "ar_internal_metadata" - - class_attribute :protected_environments, instance_accessor: false - self.protected_environments = ["production"] - - class_attribute :pluralize_table_names, instance_writer: false - self.pluralize_table_names = true - - class_attribute :ignored_columns, instance_accessor: false - self.ignored_columns = [].freeze + class_attribute :table_name_prefix, instance_writer: false, default: "" + 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 + class_attribute :ignored_columns, instance_accessor: false, default: [].freeze self.inheritance_column = "type" diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 3f39fb84e8..917bc76993 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -10,8 +10,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :nested_attributes_options, instance_writer: false - self.nested_attributes_options = {} + class_attribute :nested_attributes_options, instance_writer: false, default: {} end # = Active Record Nested Attributes diff --git a/activerecord/lib/active_record/readonly_attributes.rb b/activerecord/lib/active_record/readonly_attributes.rb index 6274996ab8..af6473d250 100644 --- a/activerecord/lib/active_record/readonly_attributes.rb +++ b/activerecord/lib/active_record/readonly_attributes.rb @@ -3,8 +3,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :_attr_readonly, instance_accessor: false - self._attr_readonly = [] + class_attribute :_attr_readonly, instance_accessor: false, default: [] end module ClassMethods diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 65fdbc2fe4..d6154c9a38 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -8,10 +8,8 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :_reflections, instance_writer: false - class_attribute :aggregate_reflections, instance_writer: false - self._reflections = {} - self.aggregate_reflections = {} + class_attribute :_reflections, instance_writer: false, default: {} + class_attribute :aggregate_reflections, instance_writer: false, default: {} end def self.create(macro, name, scope, options, ar) diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 2daa48859a..ba5cc29ac1 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -5,11 +5,8 @@ module ActiveRecord included do # Stores the default scope for the class. - class_attribute :default_scopes, instance_writer: false, instance_predicate: false - class_attribute :default_scope_override, instance_writer: false, instance_predicate: false - - self.default_scopes = [] - self.default_scope_override = nil + class_attribute :default_scopes, instance_writer: false, instance_predicate: false, default: [] + class_attribute :default_scope_override, instance_writer: false, instance_predicate: false, default: nil end module ClassMethods diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 09d8d1cdd4..55f3a194a9 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -43,8 +43,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :record_timestamps - self.record_timestamps = true + class_attribute :record_timestamps, default: true end def initialize_dup(other) # :nodoc: -- cgit v1.2.3