From e27063af3a00d329a962764ce4a00e584694d73b Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 1 Mar 2021 20:47:23 +0900 Subject: [PATCH 01/53] Allow all tree managers' initializer takes a table Currently `SelectManager` takes a table (it was happened at 80ad95b), but other managers does not, even though it is required to generate a query. I think that all tree managers' initializer could take a table, like `SelectManager` already does. --- .../connection_adapters/abstract/database_statements.rb | 3 +-- activerecord/lib/active_record/persistence.rb | 9 +++------ activerecord/lib/arel/delete_manager.rb | 5 +++-- activerecord/lib/arel/insert_manager.rb | 5 +++-- activerecord/lib/arel/update_manager.rb | 5 +++-- 5 files changed, 13 insertions(+), 14 deletions(-) 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 14d6127bc5..52ab0759d1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -477,8 +477,7 @@ module ActiveRecord end table = Arel::Table.new(table_name) - manager = Arel::InsertManager.new - manager.into(table) + manager = Arel::InsertManager.new(table) if values_list.size == 1 values = values_list.shift diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index c713949da0..fd59c889c0 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -363,8 +363,7 @@ module ActiveRecord end end - im = Arel::InsertManager.new - im.into(arel_table) + im = Arel::InsertManager.new(arel_table) if values.empty? im.insert(connection.empty_insert_statement_value(primary_key)) @@ -386,8 +385,7 @@ module ActiveRecord constraints << current_scope.where_clause.ast end - um = Arel::UpdateManager.new - um.table(arel_table) + um = Arel::UpdateManager.new(arel_table) um.set(_substitute_values(values)) um.wheres = constraints @@ -405,8 +403,7 @@ module ActiveRecord constraints << current_scope.where_clause.ast end - dm = Arel::DeleteManager.new - dm.from(arel_table) + dm = Arel::DeleteManager.new(arel_table) dm.wheres = constraints connection.delete(dm, "#{self} Destroy") diff --git a/activerecord/lib/arel/delete_manager.rb b/activerecord/lib/arel/delete_manager.rb index fdba937d64..6872746a63 100644 --- a/activerecord/lib/arel/delete_manager.rb +++ b/activerecord/lib/arel/delete_manager.rb @@ -4,10 +4,11 @@ module Arel # :nodoc: all class DeleteManager < Arel::TreeManager include TreeManager::StatementMethods - def initialize - super + def initialize(table = nil) + super() @ast = Nodes::DeleteStatement.new @ctx = @ast + @ast.relation = table end def from(relation) diff --git a/activerecord/lib/arel/insert_manager.rb b/activerecord/lib/arel/insert_manager.rb index cb31e3060b..9ec82c960b 100644 --- a/activerecord/lib/arel/insert_manager.rb +++ b/activerecord/lib/arel/insert_manager.rb @@ -2,9 +2,10 @@ module Arel # :nodoc: all class InsertManager < Arel::TreeManager - def initialize - super + def initialize(table = nil) + super() @ast = Nodes::InsertStatement.new + @ast.relation = table end def into(table) diff --git a/activerecord/lib/arel/update_manager.rb b/activerecord/lib/arel/update_manager.rb index a809dbb307..170ea047df 100644 --- a/activerecord/lib/arel/update_manager.rb +++ b/activerecord/lib/arel/update_manager.rb @@ -4,10 +4,11 @@ module Arel # :nodoc: all class UpdateManager < Arel::TreeManager include TreeManager::StatementMethods - def initialize - super + def initialize(table = nil) + super() @ast = Nodes::UpdateStatement.new @ctx = @ast + @ast.relation = table end ### From 38715aaf9b60e4e55214f719ed10f0012179c617 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Fri, 26 Feb 2021 18:46:41 -0500 Subject: [PATCH 02/53] Remove require_dependency from controller templates --- .../rails/controller/templates/controller.rb.tt | 4 ---- .../scaffold_controller/templates/api_controller.rb.tt | 4 ---- .../rails/scaffold_controller/templates/controller.rb.tt | 4 ---- railties/test/generators/namespaced_generators_test.rb | 8 +------- 4 files changed, 1 insertion(+), 19 deletions(-) diff --git a/railties/lib/rails/generators/rails/controller/templates/controller.rb.tt b/railties/lib/rails/generators/rails/controller/templates/controller.rb.tt index 633e0b3177..52243f4a2f 100644 --- a/railties/lib/rails/generators/rails/controller/templates/controller.rb.tt +++ b/railties/lib/rails/generators/rails/controller/templates/controller.rb.tt @@ -1,7 +1,3 @@ -<% if namespaced? -%> -require_dependency "<%= namespaced_path %>/application_controller" - -<% end -%> <% module_namespacing do -%> class <%= class_name %>Controller < ApplicationController <% actions.each do |action| -%> diff --git a/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb.tt b/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb.tt index 0936314a2c..25c8be553e 100644 --- a/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb.tt +++ b/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb.tt @@ -1,7 +1,3 @@ -<% if namespaced? -%> -require_dependency "<%= namespaced_path %>/application_controller" - -<% end -%> <% module_namespacing do -%> class <%= controller_class_name %>Controller < ApplicationController before_action :set_<%= singular_table_name %>, only: %i[ show update destroy ] diff --git a/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb.tt b/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb.tt index dbc675e521..382aeecf27 100644 --- a/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb.tt +++ b/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb.tt @@ -1,7 +1,3 @@ -<% if namespaced? -%> -require_dependency "<%= namespaced_path %>/application_controller" - -<% end -%> <% module_namespacing do -%> class <%= controller_class_name %>Controller < ApplicationController before_action :set_<%= singular_table_name %>, only: %i[ show edit update destroy ] diff --git a/railties/test/generators/namespaced_generators_test.rb b/railties/test/generators/namespaced_generators_test.rb index 12ccbe6ff1..6b9a53f226 100644 --- a/railties/test/generators/namespaced_generators_test.rb +++ b/railties/test/generators/namespaced_generators_test.rb @@ -25,7 +25,6 @@ class NamespacedControllerGeneratorTest < NamespacedGeneratorTestCase def test_namespaced_controller_skeleton_is_created run_generator assert_file "app/controllers/test_app/account_controller.rb", - /require_dependency "test_app\/application_controller"/, /module TestApp/, / class AccountController < ApplicationController/ @@ -42,9 +41,7 @@ class NamespacedControllerGeneratorTest < NamespacedGeneratorTestCase def test_namespaced_controller_with_additional_namespace run_generator ["admin/account"] - assert_file "app/controllers/test_app/admin/account_controller.rb", /module TestApp/, / class Admin::AccountController < ApplicationController/ do |contents| - assert_match %r(require_dependency "test_app/application_controller"), contents - end + assert_file "app/controllers/test_app/admin/account_controller.rb", /module TestApp/, / class Admin::AccountController < ApplicationController/ end def test_helper_is_also_namespaced @@ -216,7 +213,6 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Controller assert_file "app/controllers/test_app/product_lines_controller.rb", - /require_dependency "test_app\/application_controller"/, /module TestApp/, /class ProductLinesController < ApplicationController/ @@ -284,7 +280,6 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Controller assert_file "app/controllers/test_app/admin/roles_controller.rb" do |content| assert_match(/module TestApp\n class Admin::RolesController < ApplicationController/, content) - assert_match(%r(require_dependency "test_app/application_controller"), content) end assert_file "test/controllers/test_app/admin/roles_controller_test.rb", @@ -418,7 +413,6 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Controller assert_file "app/controllers/test_app/admin/roles_controller.rb" do |content| assert_match(/module TestApp\n class Admin::RolesController < ApplicationController/, content) - assert_match(%r(require_dependency "test_app/application_controller"), content) end assert_file "test/controllers/test_app/admin/roles_controller_test.rb", /module TestApp\n class Admin::RolesControllerTest < ActionDispatch::IntegrationTest/ From 37405409e3b1bbf84a573b088ed50fec0a4bc1e9 Mon Sep 17 00:00:00 2001 From: Haroon Ahmed Date: Mon, 1 Mar 2021 21:52:59 +0000 Subject: [PATCH 03/53] Update adapter name for mysql to be mysql2 in active record multi db doc --- .../source/active_record_multiple_databases.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/guides/source/active_record_multiple_databases.md b/guides/source/active_record_multiple_databases.md index e593ff2354..ce39250be0 100644 --- a/guides/source/active_record_multiple_databases.md +++ b/guides/source/active_record_multiple_databases.md @@ -50,7 +50,7 @@ The `database.yml` looks like this: production: database: my_primary_database user: root - adapter: mysql + adapter: mysql2 ``` Let's add a replica for the first configuration, and a second database called animals and a @@ -68,21 +68,21 @@ production: primary: database: my_primary_database user: root - adapter: mysql + adapter: mysql2 primary_replica: database: my_primary_database user: root_readonly - adapter: mysql + adapter: mysql2 replica: true animals: database: my_animals_database user: animals_root - adapter: mysql + adapter: mysql2 migrations_paths: db/animals_migrate animals_replica: database: my_animals_database user: animals_readonly - adapter: mysql + adapter: mysql2 replica: true ``` @@ -338,17 +338,17 @@ Shards are declared in the three-tier config like this: production: primary: database: my_primary_database - adapter: mysql + adapter: mysql2 primary_replica: database: my_primary_database - adapter: mysql + adapter: mysql2 replica: true primary_shard_one: database: my_primary_shard_one - adapter: mysql + adapter: mysql2 primary_shard_one_replica: database: my_primary_shard_one - adapter: mysql + adapter: mysql2 replica: true ``` From d861ef6210438e37a7cde563f11fb158a13f7a4d Mon Sep 17 00:00:00 2001 From: Thais Kusuki Date: Mon, 1 Mar 2021 22:30:57 -0300 Subject: [PATCH 04/53] Fix controller name in Getting Started --- guides/source/getting_started.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 197e1c2b2b..5cb8de8fa5 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -1430,7 +1430,7 @@ Associations](association_basics.html) guide. ### Adding a Route for Comments -As with the `welcome` controller, we will need to add a route so that Rails +As with the `articles` controller, we will need to add a route so that Rails knows where we would like to navigate to see `comments`. Open up the `config/routes.rb` file again, and edit it as follows: From cc5f6457cc6b030ee89bfa050dc38227fb84b28f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 1 Mar 2021 20:16:20 +0900 Subject: [PATCH 05/53] Remove `Arel::Crud` from `Arel::Table` Originally `compile_update` and `compile_delete` doesn't work at all on `Arel::Table` since `Arel::Table` doesn't have `@ast` and `@ctx`. `compile_insert` and `create_insert` works but do not use the receiver's information at all, so just use `Arel::InsertManager.new(arel_table)` instead. --- activerecord/lib/arel/table.rb | 1 - activerecord/test/cases/arel/table_test.rb | 7 ------- 2 files changed, 8 deletions(-) diff --git a/activerecord/lib/arel/table.rb b/activerecord/lib/arel/table.rb index 6790fb6e96..01464f7239 100644 --- a/activerecord/lib/arel/table.rb +++ b/activerecord/lib/arel/table.rb @@ -2,7 +2,6 @@ module Arel # :nodoc: all class Table - include Arel::Crud include Arel::FactoryMethods include Arel::AliasPredication diff --git a/activerecord/test/cases/arel/table_test.rb b/activerecord/test/cases/arel/table_test.rb index 90c281bbd8..fab59052bc 100644 --- a/activerecord/test/cases/arel/table_test.rb +++ b/activerecord/test/cases/arel/table_test.rb @@ -42,13 +42,6 @@ module Arel assert_equal "bar", join.right end - it "should return an insert manager" do - im = @relation.compile_insert "VALUES(NULL)" - assert_kind_of Arel::InsertManager, im - im.into Table.new(:users) - assert_equal "INSERT INTO \"users\" VALUES(NULL)", im.to_sql - end - describe "skip" do it "should add an offset" do sm = @relation.skip 2 From 64ca6f608ec4456805fd7f40c8d6e0009e286415 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 1 Mar 2021 18:14:43 +0900 Subject: [PATCH 06/53] Avoid extra `BindParam` allocation to generate placeholder in queries In the Active Record usage, a `BindParam` object always has an attribute object as the value. A `BindParam` object is used to call `add_bind` to generate placeholder if `prepared_statements: true`. Since Arel is a part of Active Record now, I think that we can regard an `ActiveModel::Attribute` object as boundable without wrapping it by a `BindParam` to avoid extra `BindParam` allocation. --- activerecord/lib/active_record/persistence.rb | 10 ++-------- .../lib/active_record/relation/predicate_builder.rb | 3 +-- .../lib/active_record/relation/query_methods.rb | 3 +-- .../lib/active_record/relation/where_clause.rb | 7 +++---- activerecord/lib/arel/nodes/casted.rb | 2 +- activerecord/lib/arel/predications.rb | 2 +- activerecord/lib/arel/visitors/to_sql.rb | 8 ++++++-- 7 files changed, 15 insertions(+), 20 deletions(-) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index fd59c889c0..9968580be8 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -368,7 +368,7 @@ module ActiveRecord if values.empty? im.insert(connection.empty_insert_statement_value(primary_key)) else - im.insert(_substitute_values(values)) + im.insert(values.transform_keys { |name| arel_table[name] }) end connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) @@ -386,7 +386,7 @@ module ActiveRecord end um = Arel::UpdateManager.new(arel_table) - um.set(_substitute_values(values)) + um.set(values.transform_keys { |name| arel_table[name] }) um.wheres = constraints connection.update(um, "#{self} Update") @@ -425,12 +425,6 @@ module ActiveRecord def discriminate_class_for_record(record) self end - - def _substitute_values(values) - values.map do |name, value| - [ arel_table[name], Arel::Nodes::BindParam.new(value) ] - end - end end # Returns true if this object hasn't been saved yet -- that is, a record diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index ccf0d10d05..1a1091556e 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -65,8 +65,7 @@ module ActiveRecord end def build_bind_attribute(column_name, value) - attr = Relation::QueryAttribute.new(column_name, value, table.type(column_name)) - Arel::Nodes::BindParam.new(attr) + Relation::QueryAttribute.new(column_name, value, table.type(column_name)) end def resolve_arel_attribute(table_name, column_name, &block) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d2285257c7..60d55bcfd5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1263,8 +1263,7 @@ module ActiveRecord end def build_cast_value(name, value) - cast_value = ActiveModel::Attribute.with_cast_value(name, value, Type.default_value) - Arel::Nodes::BindParam.new(cast_value) + ActiveModel::Attribute.with_cast_value(name, value, Type.default_value) end def build_from diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 37fbb12575..97ec0cdc0c 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -224,11 +224,10 @@ module ActiveRecord end def extract_node_value(node) - case node - when Array - node.map { |v| extract_node_value(v) } - when Arel::Nodes::BindParam, Arel::Nodes::Casted, Arel::Nodes::Quoted + if node.respond_to?(:value_before_type_cast) node.value_before_type_cast + elsif Array === node + node.map { |v| extract_node_value(v) } end end end diff --git a/activerecord/lib/arel/nodes/casted.rb b/activerecord/lib/arel/nodes/casted.rb index 98a7f6d0e2..bd389f3930 100644 --- a/activerecord/lib/arel/nodes/casted.rb +++ b/activerecord/lib/arel/nodes/casted.rb @@ -47,7 +47,7 @@ module Arel # :nodoc: all def self.build_quoted(other, attribute = nil) case other - when Arel::Nodes::Node, Arel::Attributes::Attribute, Arel::Table, Arel::SelectManager, Arel::Nodes::SqlLiteral + when Arel::Nodes::Node, Arel::Attributes::Attribute, Arel::Table, Arel::SelectManager, Arel::Nodes::SqlLiteral, ActiveModel::Attribute other else case attribute diff --git a/activerecord/lib/arel/predications.rb b/activerecord/lib/arel/predications.rb index 0d2d1acc93..bbcc192d2f 100644 --- a/activerecord/lib/arel/predications.rb +++ b/activerecord/lib/arel/predications.rb @@ -52,7 +52,7 @@ module Arel # :nodoc: all else left = quoted_node(other.begin) right = quoted_node(other.end) - Nodes::Between.new(self, left.and(right)) + Nodes::Between.new(self, Nodes::And.new([left, right])) end end diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index fac8c921da..f593f4980a 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -103,7 +103,7 @@ module Arel # :nodoc: all row.each_with_index do |value, k| collector << ", " unless k == 0 case value - when Nodes::SqlLiteral, Nodes::BindParam + when Nodes::SqlLiteral, Nodes::BindParam, ActiveModel::Attribute collector = visit(value, collector) else collector << quote(value).to_s @@ -585,7 +585,7 @@ module Arel # :nodoc: all def visit_Arel_Nodes_Assignment(o, collector) case o.right - when Arel::Nodes::Node, Arel::Attributes::Attribute + when Arel::Nodes::Node, Arel::Attributes::Attribute, ActiveModel::Attribute collector = visit o.left, collector collector << " = " visit o.right, collector @@ -695,6 +695,10 @@ module Arel # :nodoc: all def bind_block; BIND_BLOCK; end + def visit_ActiveModel_Attribute(o, collector) + collector.add_bind(o, &bind_block) + end + def visit_Arel_Nodes_BindParam(o, collector) collector.add_bind(o.value, &bind_block) end From 6b9027bd1c57109e3681fbafb110f3988ea521c8 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Fri, 26 Feb 2021 13:47:39 -0500 Subject: [PATCH 07/53] Don't execute queries in background when max_threads is 0 In the multi_thread_pool executor it's possible to have a database configuration that wouldn't work well with a thread exectutor. This change checks the `@db_config.max_threads` and if it is not greater than 0 we won't create a thread pool for that configuration. Database configurations without a proper value set for `max_threads` will return a `nil` async executor and will run queries for those connection pools in the foreground. This allows applications to more easily control which database configurations in their app support async queries. In this setup one database coule be configured to run parallel queries while all other databases run queries in the foreground. --- .../abstract/connection_pool.rb | 16 +- .../connection_adapters/abstract_adapter.rb | 5 +- .../test/cases/asynchronous_queries_test.rb | 37 ++- .../test/cases/relation/load_async_test.rb | 230 ++++++++++++++++++ 4 files changed, 278 insertions(+), 10 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index f8d4f2f527..2490d196dc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -97,7 +97,7 @@ module ActiveRecord include ConnectionAdapters::AbstractPool attr_accessor :automatic_reconnect, :checkout_timeout - attr_reader :db_config, :size, :reaper, :pool_config, :connection_klass + attr_reader :db_config, :size, :reaper, :pool_config, :connection_klass, :async_executor delegate :schema_cache, :schema_cache=, to: :pool_config @@ -461,12 +461,14 @@ module ActiveRecord def build_async_executor case Base.async_query_executor when :multi_thread_pool - Concurrent::ThreadPoolExecutor.new( - min_threads: @db_config.min_threads, - max_threads: @db_config.max_threads, - max_queue: @db_config.max_queue, - fallback_policy: :caller_runs - ) + if @db_config.max_threads > 0 + Concurrent::ThreadPoolExecutor.new( + min_threads: @db_config.min_threads, + max_threads: @db_config.max_threads, + max_queue: @db_config.max_queue, + fallback_policy: :caller_runs + ) + end when :global_thread_pool Base.global_thread_pool_async_query_executor end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index bb051528bc..22768c9611 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -429,8 +429,9 @@ module ActiveRecord true end - def async_enabled? - supports_concurrent_connections? && !Base.async_query_executor.nil? + def async_enabled? # :nodoc: + supports_concurrent_connections? && + !Base.async_query_executor.nil? && !pool.async_executor.nil? end # This is meant to be implemented by the adapters that support extensions diff --git a/activerecord/test/cases/asynchronous_queries_test.rb b/activerecord/test/cases/asynchronous_queries_test.rb index cc7da91ab6..0118c94ca5 100644 --- a/activerecord/test/cases/asynchronous_queries_test.rb +++ b/activerecord/test/cases/asynchronous_queries_test.rb @@ -248,7 +248,7 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase ActiveRecord::Base.async_query_executor = old_value end - def test_one_global_thread_pool_uses_concurrency_if_set + def test_multi_thread_pool_executor_configuration old_value = ActiveRecord::Base.async_query_executor ActiveRecord::Base.async_query_executor = :multi_thread_pool @@ -282,4 +282,39 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase clean_up_connection_handler ActiveRecord::Base.async_query_executor = old_value end + + def test_multi_thread_pool_is_used_only_by_configurations_that_enable_it + old_value = ActiveRecord::Base.async_query_executor + ActiveRecord::Base.async_query_executor = :multi_thread_pool + + handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new + + config_hash1 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").configuration_hash + new_config1 = config_hash1.merge(min_threads: 0, max_threads: 10) + db_config1 = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", new_config1) + + config_hash2 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit2", name: "primary").configuration_hash + new_config2 = config_hash2.merge(min_threads: 0, max_threads: 0) + db_config2 = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit2", "primary", new_config2) + + pool1 = handler.establish_connection(db_config1) + pool2 = handler.establish_connection(db_config2, owner_name: ARUnit2Model) + + async_pool1 = pool1.instance_variable_get(:@async_executor) + async_pool2 = pool2.instance_variable_get(:@async_executor) + + assert async_pool1.is_a?(Concurrent::ThreadPoolExecutor) + assert_nil async_pool2 + + assert 0, async_pool1.min_length + assert 10, async_pool1.max_length + assert 40, async_pool1.max_queue + assert :caller_runs, async_pool1.fallback_policy + + assert_equal 2, handler.all_connection_pools.count + assert_not_equal async_pool1, async_pool2 + ensure + clean_up_connection_handler + ActiveRecord::Base.async_query_executor = old_value + end end diff --git a/activerecord/test/cases/relation/load_async_test.rb b/activerecord/test/cases/relation/load_async_test.rb index 72df32a9dc..bacde8c4ab 100644 --- a/activerecord/test/cases/relation/load_async_test.rb +++ b/activerecord/test/cases/relation/load_async_test.rb @@ -3,6 +3,7 @@ require "cases/helper" require "models/post" require "models/comment" +require "models/other_dog" module ActiveRecord class LoadAsyncTest < ActiveRecord::TestCase @@ -280,5 +281,234 @@ module ActiveRecord assert_predicate defered_posts, :loaded? end end + + class LoadAsyncMultiThreadPoolExecutorTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + fixtures :posts, :comments + + def setup + @old_config = ActiveRecord::Base.async_query_executor + ActiveRecord::Base.async_query_executor = :multi_thread_pool + + handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new + config_hash1 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").configuration_hash + new_config1 = config_hash1.merge(min_threads: 0, max_threads: 10) + db_config1 = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", new_config1) + + + config_hash2 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit2", name: "primary").configuration_hash + new_config2 = config_hash2.merge(min_threads: 0, max_threads: 10) + db_config2 = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit2", "primary", new_config2) + + handler.establish_connection(db_config1) + handler.establish_connection(db_config2, owner_name: ARUnit2Model) + end + + def teardown + ActiveRecord::Base.async_query_executor = @old_config + clean_up_connection_handler + end + + def test_scheduled? + defered_posts = Post.where(author_id: 1).load_async + assert_predicate defered_posts, :scheduled? + assert_predicate defered_posts, :loaded? + defered_posts.to_a + assert_not_predicate defered_posts, :scheduled? + end + + def test_reset + defered_posts = Post.where(author_id: 1).load_async + assert_predicate defered_posts, :scheduled? + defered_posts.reset + assert_not_predicate defered_posts, :scheduled? + end + + def test_simple_query + expected_records = Post.where(author_id: 1).to_a + + status = {} + monitor = Monitor.new + condition = monitor.new_cond + + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event| + if event.payload[:name] == "Post Load" + status[:executed] = true + status[:async] = event.payload[:async] + monitor.synchronize { condition.signal } + end + end + + defered_posts = Post.where(author_id: 1).load_async + + monitor.synchronize do + condition.wait_until { status[:executed] } + end + + assert_equal expected_records, defered_posts.to_a + assert_equal Post.connection.supports_concurrent_connections?, status[:async] + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + def test_load_async_from_transaction + posts = nil + Post.transaction do + Post.where(author_id: 1).update_all(title: "In Transaction") + posts = Post.where(author_id: 1).load_async + assert_predicate posts, :scheduled? + assert_predicate posts, :loaded? + raise ActiveRecord::Rollback + end + + assert_not_nil posts + assert_equal ["In Transaction"], posts.map(&:title).uniq + end + + def test_eager_loading_query + expected_records = Post.where(author_id: 1).eager_load(:comments).to_a + + status = {} + monitor = Monitor.new + condition = monitor.new_cond + + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event| + if event.payload[:name] == "SQL" + status[:executed] = true + status[:async] = event.payload[:async] + monitor.synchronize { condition.signal } + end + end + + defered_posts = Post.where(author_id: 1).eager_load(:comments).load_async + + assert_predicate defered_posts, :scheduled? + + monitor.synchronize do + condition.wait_until { status[:executed] } + end + + assert_equal expected_records, defered_posts.to_a + assert_queries(0) do + defered_posts.each(&:comments) + end + assert_equal Post.connection.supports_concurrent_connections?, status[:async] + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + def test_contradiction + assert_queries(0) do + assert_equal [], Post.where(id: []).load_async.to_a + end + + Post.where(id: []).load_async.reset + end + + def test_pluck + titles = Post.where(author_id: 1).pluck(:title) + assert_equal titles, Post.where(author_id: 1).load_async.pluck(:title) + end + + def test_size + expected_size = Post.where(author_id: 1).size + + defered_posts = Post.where(author_id: 1).load_async + + assert_equal expected_size, defered_posts.size + assert_predicate defered_posts, :loaded? + end + + def test_empty? + defered_posts = Post.where(author_id: 1).load_async + + assert_equal false, defered_posts.empty? + assert_predicate defered_posts, :loaded? + end + end + + class LoadAsyncMixedThreadPoolExecutorTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + fixtures :posts, :comments, :other_dogs + + def setup + @previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env" + @old_config = ActiveRecord::Base.async_query_executor + ActiveRecord::Base.async_query_executor = :multi_thread_pool + config_hash1 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").configuration_hash + config_hash2 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit2", name: "primary").configuration_hash + config = { + "default_env" => { + "animals" => config_hash2.merge({ min_threads: 0, max_threads: 0 }), + "primary" => config_hash1.merge({ min_threads: 0, max_threads: 10 }) + } + } + @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config + + ActiveRecord::Base.establish_connection(:primary) + ARUnit2Model.establish_connection(:animals) + end + + def teardown + ENV["RAILS_ENV"] = @previous_env + ActiveRecord::Base.configurations = @prev_configs + ActiveRecord::Base.async_query_executor = @old_config + clean_up_connection_handler + end + + def test_scheduled? + defered_posts = Post.where(author_id: 1).load_async + assert_predicate defered_posts, :scheduled? + assert_predicate defered_posts, :loaded? + defered_posts.to_a + assert_not_predicate defered_posts, :scheduled? + + defered_dogs = OtherDog.where(id: 1).load_async + assert_not_predicate defered_dogs, :scheduled? + assert_predicate defered_dogs, :loaded? + end + + def test_simple_query + expected_records = Post.where(author_id: 1).to_a + expected_dogs = OtherDog.where(id: 1).to_a + + status = {} + dog_status = {} + monitor = Monitor.new + condition = monitor.new_cond + + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event| + if event.payload[:name] == "Post Load" + status[:executed] = true + status[:async] = event.payload[:async] + monitor.synchronize { condition.signal } + end + + if event.payload[:name] == "OtherDog Load" + dog_status[:executed] = true + dog_status[:async] = event.payload[:async] + monitor.synchronize { condition.signal } + end + end + + defered_posts = Post.where(author_id: 1).load_async + defered_dogs = OtherDog.where(id: 1).load_async + + monitor.synchronize do + condition.wait_until { status[:executed] } + condition.wait_until { dog_status[:executed] } + end + + assert_equal expected_records, defered_posts.to_a + assert_equal expected_dogs, defered_dogs.to_a + + assert_equal Post.connection.async_enabled?, status[:async] + assert_equal OtherDog.connection.async_enabled?, dog_status[:async] + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + end end end From 7b9cfde741662603f7fdd78698190b2fdefead17 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 2 Mar 2021 22:25:48 +0900 Subject: [PATCH 08/53] Fix `method_missing` delegation to not expand positional hash argument Follow up to #41518, and similar to 3a85ced1a03c3e4fa81e316d06a65a75c760cf8c. --- .../lib/active_support/current_attributes.rb | 5 +++-- activesupport/test/current_attributes_test.rb | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/current_attributes.rb b/activesupport/lib/active_support/current_attributes.rb index 3d3031ad94..00dcfe99c4 100644 --- a/activesupport/lib/active_support/current_attributes.rb +++ b/activesupport/lib/active_support/current_attributes.rb @@ -155,14 +155,15 @@ module ActiveSupport @current_instances_key ||= name.to_sym end - def method_missing(name, *args, **kwargs, &block) + def method_missing(name, *args, &block) # Caches the method definition as a singleton method of the receiver. # # By letting #delegate handle it, we avoid an enclosure that'll capture args. singleton_class.delegate name, to: :instance - send(name, *args, **kwargs, &block) + send(name, *args, &block) end + ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true) def respond_to_missing?(name, _) super || instance.respond_to?(name) diff --git a/activesupport/test/current_attributes_test.rb b/activesupport/test/current_attributes_test.rb index 024ccec499..802fbedba9 100644 --- a/activesupport/test/current_attributes_test.rb +++ b/activesupport/test/current_attributes_test.rb @@ -36,6 +36,12 @@ class CurrentAttributesTest < ActiveSupport::TestCase self.account = account end + def get_world_and_account(hash) + hash[:world] = world + hash[:account] = account + hash + end + def respond_to_test; end def request @@ -138,6 +144,11 @@ class CurrentAttributesTest < ActiveSupport::TestCase assert_equal "world/1", Current.world assert_equal "account/1", Current.account + + hash = {} + assert_same hash, Current.get_world_and_account(hash) + assert_equal "world/1", hash[:world] + assert_equal "account/1", hash[:account] end setup { @testing_teardown = false } From d5ac941ddc3de7ad1aaff80ed67aa04fb626a263 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 2 Mar 2021 17:20:35 -0800 Subject: [PATCH 09/53] Remove special case filtering for Procs. I'm writing this patch for two purposes: 1. I want to reduce the number of times `object_id` is called. Calling `object_id` can have negative impacts on performance in Ruby 2.7+, so it would be nice to stop calling it. 2. I'm not sure why we're treating lambdas specially here. It looks like we wanted to prevent people from skipping callbacks that were defined with a lambda, but I think that is silly. If the user has a reference to a lambda, and they want to skip it, we should let them. I think this cleans up some code, helps with performance, and is a more intuitive interface. --- actionpack/test/controller/filters_test.rb | 2 +- activesupport/lib/active_support/callbacks.rb | 15 +-------------- activesupport/test/callbacks_test.rb | 9 --------- activesupport/test/test_case_test.rb | 8 ++++---- 4 files changed, 6 insertions(+), 28 deletions(-) diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index c26e09d8ac..b19d1488de 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -12,7 +12,7 @@ class ActionController::Base def before_actions filters = _process_action_callbacks.select { |c| c.kind == :before } - filters.map!(&:raw_filter) + filters.map!(&:filter) end end end diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index a86af00527..a65ff5cf72 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -289,21 +289,17 @@ module ActiveSupport end attr_accessor :kind, :name - attr_reader :chain_config + attr_reader :chain_config, :filter def initialize(name, filter, kind, options, chain_config) @chain_config = chain_config @name = name @kind = kind @filter = filter - @key = compute_identifier filter @if = check_conditionals(options[:if]) @unless = check_conditionals(options[:unless]) end - def filter; @key; end - def raw_filter; @filter; end - def merge_conditional_options(chain, if_option:, unless_option:) options = { if: @if.dup, @@ -367,15 +363,6 @@ module ActiveSupport conditionals.freeze end - def compute_identifier(filter) - case filter - when ::Proc - filter.object_id - else - filter - end - end - def conditions_lambdas @if.map { |c| CallTemplate.build(c, self).make_lambda } + @unless.map { |c| CallTemplate.build(c, self).inverted_lambda } diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 499698c1a3..a1cba3ce45 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -1129,15 +1129,6 @@ module CallbacksTest } end - def test_skip_lambda # raises error - calls = [] - callback = ->(o) { calls << o } - klass = build_class(callback) - assert_raises(ArgumentError) { klass.skip callback } - klass.new.run - assert_equal 10, calls.length - end - def test_skip_symbol # removes all calls = [] klass = build_class(:bar) diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index ac2923a313..e6342e8382 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -325,9 +325,9 @@ class SetupAndTeardownTest < ActiveSupport::TestCase teardown :foo, :sentinel def test_inherited_setup_callbacks - assert_equal [:reset_callback_record, :foo], self.class._setup_callbacks.map(&:raw_filter) + assert_equal [:reset_callback_record, :foo], self.class._setup_callbacks.map(&:filter) assert_equal [:foo], @called_back - assert_equal [:foo, :sentinel], self.class._teardown_callbacks.map(&:raw_filter) + assert_equal [:foo, :sentinel], self.class._teardown_callbacks.map(&:filter) end def setup @@ -355,9 +355,9 @@ class SubclassSetupAndTeardownTest < SetupAndTeardownTest teardown :bar def test_inherited_setup_callbacks - assert_equal [:reset_callback_record, :foo, :bar], self.class._setup_callbacks.map(&:raw_filter) + assert_equal [:reset_callback_record, :foo, :bar], self.class._setup_callbacks.map(&:filter) assert_equal [:foo, :bar], @called_back - assert_equal [:foo, :sentinel, :bar], self.class._teardown_callbacks.map(&:raw_filter) + assert_equal [:foo, :sentinel, :bar], self.class._teardown_callbacks.map(&:filter) end private From e10e6ab1d4016be40b86e98e8208f51ff5708d5d Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 3 Mar 2021 13:48:57 +0100 Subject: [PATCH 10/53] Make .excluding work when no arguments are passed The change from https://github.com/rails/rails/pull/41465 was a breaking change. Before, `.without` was delegated to `Array` and worked with empty arguments. This change restores that old behavior. --- .../lib/active_record/relation/query_methods.rb | 2 -- activerecord/test/cases/excluding_test.rb | 16 ++++++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d2285257c7..4c12794d19 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1131,8 +1131,6 @@ module ActiveRecord def excluding(*records) records.flatten!(1) - raise ArgumentError, "You must pass at least one #{klass.name} object to #excluding." if records.empty? - if records.any? { |record| !record.is_a?(klass) } raise ArgumentError, "You must only pass a single or collection of #{klass.name} objects to #excluding." end diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index 35fe26c614..1b0146fc18 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -45,18 +45,14 @@ class ExcludingTest < ActiveRecord::TestCase assert_not_includes relation.to_a, comment_more_greetings end - def test_raises_on_no_arguments - exception = assert_raises ArgumentError do - Post.excluding() - end - assert_equal "You must pass at least one Post object to #excluding.", exception.message + def test_does_not_exclude_records_when_no_arguments + assert_includes Post.excluding(), posts(:welcome) + assert_equal Post.count, Post.excluding().count end - def test_raises_on_empty_collection_argument - exception = assert_raises ArgumentError do - Post.excluding([]) - end - assert_equal "You must pass at least one Post object to #excluding.", exception.message + def test_does_not_exclude_records_with_empty_collection_argument + assert_includes Post.excluding([]), posts(:welcome) + assert_equal Post.count, Post.excluding([]).count end def test_raises_on_record_from_different_class From b19cd20f6319c019dec5285c547f815b28ef7dea Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 3 Mar 2021 15:14:34 +0100 Subject: [PATCH 11/53] Make .excluding work when a nil argument is passed The change from https://github.com/rails/rails/pull/41465 was a breaking change. Before, `.without` was delegated to `Array` and worked with nil arguments. This change restores that old behavior. --- .../lib/active_record/relation/query_methods.rb | 2 +- activerecord/test/cases/excluding_test.rb | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 4c12794d19..d0929d473c 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1131,7 +1131,7 @@ module ActiveRecord def excluding(*records) records.flatten!(1) - if records.any? { |record| !record.is_a?(klass) } + if records.compact.any? { |record| !record.is_a?(klass) } raise ArgumentError, "You must only pass a single or collection of #{klass.name} objects to #excluding." end diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index 1b0146fc18..b5fbf0e89b 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -46,13 +46,15 @@ class ExcludingTest < ActiveRecord::TestCase end def test_does_not_exclude_records_when_no_arguments - assert_includes Post.excluding(), posts(:welcome) - assert_equal Post.count, Post.excluding().count + assert_does_not_exclude_records { Post.excluding() } end def test_does_not_exclude_records_with_empty_collection_argument - assert_includes Post.excluding([]), posts(:welcome) - assert_equal Post.count, Post.excluding([]).count + assert_does_not_exclude_records { Post.excluding([]) } + end + + def test_does_not_exclude_records_with_a_nil_argument + assert_does_not_exclude_records { Post.excluding(nil) } end def test_raises_on_record_from_different_class @@ -70,4 +72,10 @@ class ExcludingTest < ActiveRecord::TestCase assert_not_includes Post.without(post).to_a, post end + + private + def assert_does_not_exclude_records + assert_includes yield, posts(:welcome) + assert_equal Post.count, yield.count + end end From 32a69da269b3fd7ac5dc4536ce19be3aca76d199 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Wed, 3 Mar 2021 15:58:27 +0100 Subject: [PATCH 12/53] Let's simplify the internal assertion here, doesn't need to a yield argument --- activerecord/test/cases/excluding_test.rb | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index b5fbf0e89b..f314b0a197 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -46,15 +46,10 @@ class ExcludingTest < ActiveRecord::TestCase end def test_does_not_exclude_records_when_no_arguments - assert_does_not_exclude_records { Post.excluding() } - end - - def test_does_not_exclude_records_with_empty_collection_argument - assert_does_not_exclude_records { Post.excluding([]) } - end - - def test_does_not_exclude_records_with_a_nil_argument - assert_does_not_exclude_records { Post.excluding(nil) } + assert_no_excludes Post.excluding + assert_no_excludes Post.excluding(nil) + assert_no_excludes Post.excluding([]) + assert_no_excludes Post.excluding([ nil ]) end def test_raises_on_record_from_different_class @@ -74,8 +69,8 @@ class ExcludingTest < ActiveRecord::TestCase end private - def assert_does_not_exclude_records - assert_includes yield, posts(:welcome) - assert_equal Post.count, yield.count + def assert_no_excludes(relation) + assert_includes relation, posts(:welcome) + assert_equal Post.count, relation.count end end From bfa1c36333c1d78c041c73cc65d23dfe7bf30e79 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Wed, 3 Mar 2021 16:02:00 +0100 Subject: [PATCH 13/53] Style --- activerecord/test/cases/excluding_test.rb | 35 ++++++----------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index f314b0a197..fd10e24d63 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -8,39 +8,29 @@ class ExcludingTest < ActiveRecord::TestCase fixtures :posts, :comments def test_result_set_does_not_include_single_excluded_record - post = posts(:welcome) - - assert_not_includes Post.excluding(post).to_a, post + assert_not_includes Post.excluding(posts(:welcome)).to_a, posts(:welcome) end def test_result_set_does_not_include_collection_of_excluded_records - post_welcome = posts(:welcome) - post_thinking = posts(:thinking) + post_welcome, post_thinking = posts(:welcome, :thinking) relation = Post.excluding(post_welcome, post_thinking) - assert_not_includes relation.to_a, post_welcome assert_not_includes relation.to_a, post_thinking end def test_result_set_through_association_does_not_include_single_excluded_record - post = posts(:welcome) - comment_greetings = comments(:greetings) - comment_more_greetings = comments(:more_greetings) - - relation = post.comments.excluding(comment_greetings) + comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings) + relation = posts(:welcome).comments.excluding(comment_greetings) assert_not_includes relation.to_a, comment_greetings assert_includes relation.to_a, comment_more_greetings end def test_result_set_through_association_does_not_include_collection_of_excluded_records - post = posts(:welcome) - comment_greetings = comments(:greetings) - comment_more_greetings = comments(:more_greetings) - - relation = post.comments.excluding([comment_greetings, comment_more_greetings]) + comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings) + relation = posts(:welcome).comments.excluding([ comment_greetings, comment_more_greetings ]) assert_not_includes relation.to_a, comment_greetings assert_not_includes relation.to_a, comment_more_greetings end @@ -53,19 +43,12 @@ class ExcludingTest < ActiveRecord::TestCase end def test_raises_on_record_from_different_class - post = posts(:welcome) - comment = comments(:greetings) - - exception = assert_raises ArgumentError do - Post.excluding(post, comment) - end - assert_equal "You must only pass a single or collection of Post objects to #excluding.", exception.message + error = assert_raises(ArgumentError) { Post.excluding(posts(:welcome), comments(:greetings)) } + assert_equal "You must only pass a single or collection of Post objects to #excluding.", error.message end def test_result_set_does_not_include_without_record - post = posts(:welcome) - - assert_not_includes Post.without(post).to_a, post + assert_not_includes Post.without(posts(:welcome)).to_a, posts(:welcome) end private From 89a213b59d89392403803dec44a713e58f428f00 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Wed, 3 Mar 2021 16:03:21 +0100 Subject: [PATCH 14/53] Clearer when this post is extracted --- activerecord/test/cases/excluding_test.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index fd10e24d63..317bbe3423 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -7,22 +7,22 @@ require "models/comment" class ExcludingTest < ActiveRecord::TestCase fixtures :posts, :comments + setup { @post = posts(:welcome) } + def test_result_set_does_not_include_single_excluded_record - assert_not_includes Post.excluding(posts(:welcome)).to_a, posts(:welcome) + assert_not_includes Post.excluding(@post).to_a, @post end def test_result_set_does_not_include_collection_of_excluded_records - post_welcome, post_thinking = posts(:welcome, :thinking) - - relation = Post.excluding(post_welcome, post_thinking) - assert_not_includes relation.to_a, post_welcome - assert_not_includes relation.to_a, post_thinking + relation = Post.excluding(@post, posts(:thinking)) + assert_not_includes relation.to_a, @post + assert_not_includes relation.to_a, posts(:thinking) end def test_result_set_through_association_does_not_include_single_excluded_record comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings) - relation = posts(:welcome).comments.excluding(comment_greetings) + relation = @post.comments.excluding(comment_greetings) assert_not_includes relation.to_a, comment_greetings assert_includes relation.to_a, comment_more_greetings end @@ -30,7 +30,7 @@ class ExcludingTest < ActiveRecord::TestCase def test_result_set_through_association_does_not_include_collection_of_excluded_records comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings) - relation = posts(:welcome).comments.excluding([ comment_greetings, comment_more_greetings ]) + relation = @post.comments.excluding([ comment_greetings, comment_more_greetings ]) assert_not_includes relation.to_a, comment_greetings assert_not_includes relation.to_a, comment_more_greetings end @@ -43,17 +43,17 @@ class ExcludingTest < ActiveRecord::TestCase end def test_raises_on_record_from_different_class - error = assert_raises(ArgumentError) { Post.excluding(posts(:welcome), comments(:greetings)) } + error = assert_raises(ArgumentError) { Post.excluding(@post, comments(:greetings)) } assert_equal "You must only pass a single or collection of Post objects to #excluding.", error.message end def test_result_set_does_not_include_without_record - assert_not_includes Post.without(posts(:welcome)).to_a, posts(:welcome) + assert_not_includes Post.without(@post).to_a, @post end private def assert_no_excludes(relation) - assert_includes relation, posts(:welcome) + assert_includes relation, @post assert_equal Post.count, relation.count end end From 17854e69b3cff5aed8b8249e3a8c22c862ce743f Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Wed, 3 Mar 2021 16:03:56 +0100 Subject: [PATCH 15/53] Can just test this inline --- activerecord/test/cases/excluding_test.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index 317bbe3423..2ff0c0fbd2 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -11,6 +11,7 @@ class ExcludingTest < ActiveRecord::TestCase def test_result_set_does_not_include_single_excluded_record assert_not_includes Post.excluding(@post).to_a, @post + assert_not_includes Post.without(@post).to_a, @post end def test_result_set_does_not_include_collection_of_excluded_records @@ -47,10 +48,6 @@ class ExcludingTest < ActiveRecord::TestCase assert_equal "You must only pass a single or collection of Post objects to #excluding.", error.message end - def test_result_set_does_not_include_without_record - assert_not_includes Post.without(@post).to_a, @post - end - private def assert_no_excludes(relation) assert_includes relation, @post From 14f35cde18de77cd7ab5d6f17d83fa8bc92ba383 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Wed, 3 Mar 2021 16:05:45 +0100 Subject: [PATCH 16/53] We don't really need all these to_a calls; fine to do a single check but rely on query otherwise --- activerecord/test/cases/excluding_test.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/activerecord/test/cases/excluding_test.rb b/activerecord/test/cases/excluding_test.rb index 2ff0c0fbd2..e2fb7d61a3 100644 --- a/activerecord/test/cases/excluding_test.rb +++ b/activerecord/test/cases/excluding_test.rb @@ -10,30 +10,32 @@ class ExcludingTest < ActiveRecord::TestCase setup { @post = posts(:welcome) } def test_result_set_does_not_include_single_excluded_record + assert_not_includes Post.excluding(@post), @post assert_not_includes Post.excluding(@post).to_a, @post - assert_not_includes Post.without(@post).to_a, @post + + assert_not_includes Post.without(@post), @post end def test_result_set_does_not_include_collection_of_excluded_records relation = Post.excluding(@post, posts(:thinking)) - assert_not_includes relation.to_a, @post - assert_not_includes relation.to_a, posts(:thinking) + assert_not_includes relation, @post + assert_not_includes relation, posts(:thinking) end def test_result_set_through_association_does_not_include_single_excluded_record comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings) relation = @post.comments.excluding(comment_greetings) - assert_not_includes relation.to_a, comment_greetings - assert_includes relation.to_a, comment_more_greetings + assert_not_includes relation, comment_greetings + assert_includes relation, comment_more_greetings end def test_result_set_through_association_does_not_include_collection_of_excluded_records comment_greetings, comment_more_greetings = comments(:greetings, :more_greetings) relation = @post.comments.excluding([ comment_greetings, comment_more_greetings ]) - assert_not_includes relation.to_a, comment_greetings - assert_not_includes relation.to_a, comment_more_greetings + assert_not_includes relation, comment_greetings + assert_not_includes relation, comment_more_greetings end def test_does_not_exclude_records_when_no_arguments From 12613accfdd5665600958c2c291f8377bc1f11e3 Mon Sep 17 00:00:00 2001 From: Aditya Pandit Date: Wed, 3 Mar 2021 22:18:31 +0530 Subject: [PATCH 17/53] Removed test for deprecated ASt combine_options transformation --- activestorage/test/models/variant_test.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 705ea89b0d..e89f075ce2 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -60,16 +60,6 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase assert_match(/RGB/, image.colorspace) end - test "variation with :combine_options is not supported" do - blob = create_file_blob(filename: "racecar.jpg") - assert_raises(ArgumentError) do - blob.variant(combine_options: { - resize: "100x100", - monochrome: false - }).processed - end - end - test "center-weighted crop of JPEG blob using :resize_to_fill" do blob = create_file_blob(filename: "racecar.jpg") variant = blob.variant(resize_to_fill: [100, 100]).processed From 93d57d5cd4a6e8fa13b271b929900c121fa67dcb Mon Sep 17 00:00:00 2001 From: Abhay Nikam Date: Thu, 4 Mar 2021 00:57:05 +0530 Subject: [PATCH 18/53] Fixes the asynchronous_queries_test.rb changed in the PR: #41586 While reading the change noticed the test cases used assert for comparsion. The problem here is assert would never raise error if the expected and actual value do not match. After changing it to the assert_equal, async_query_executor test cases started failing because the connection was already established before the spec was ran and setting global `global_executor_concurrency` did not reset the global_thread_pool_async_query_executor Changes to assert_equal also broke the async_query_executor = :multi_thread_pool specs as the max_length defaults to pool(i.e: 5) in the database configuration. PR attemps to fix all the above mentioned issues in the asynchronous_queries_test.rb --- .../test/cases/asynchronous_queries_test.rb | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/activerecord/test/cases/asynchronous_queries_test.rb b/activerecord/test/cases/asynchronous_queries_test.rb index 0118c94ca5..ba8fec4dff 100644 --- a/activerecord/test/cases/asynchronous_queries_test.rb +++ b/activerecord/test/cases/asynchronous_queries_test.rb @@ -178,15 +178,15 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase assert async_pool1.is_a?(Concurrent::ThreadPoolExecutor) assert async_pool2.is_a?(Concurrent::ThreadPoolExecutor) - assert 0, async_pool1.min_length - assert 4, async_pool1.max_length - assert 16, async_pool1.max_queue - assert :caller_runs, async_pool1.fallback_policy + assert_equal 0, async_pool1.min_length + assert_equal 4, async_pool1.max_length + assert_equal 16, async_pool1.max_queue + assert_equal :caller_runs, async_pool1.fallback_policy - assert 0, async_pool2.min_length - assert 4, async_pool2.max_length - assert 16, async_pool2.max_queue - assert :caller_runs, async_pool2.fallback_policy + assert_equal 0, async_pool2.min_length + assert_equal 4, async_pool2.max_length + assert_equal 16, async_pool2.max_queue + assert_equal :caller_runs, async_pool2.fallback_policy assert_equal 2, handler.all_connection_pools.count assert_equal async_pool1, async_pool2 @@ -199,6 +199,8 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase old_value = ActiveRecord::Base.async_query_executor ActiveRecord::Base.async_query_executor = :global_thread_pool old_concurrency = ActiveRecord::Base.global_executor_concurrency + old_global_thread_pool_async_query_executor = ActiveRecord::Core.class_variable_get(:@@global_thread_pool_async_query_executor) + ActiveRecord::Core.class_variable_set(:@@global_thread_pool_async_query_executor, nil) ActiveRecord::Base.global_executor_concurrency = 8 handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new @@ -213,15 +215,15 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase assert async_pool1.is_a?(Concurrent::ThreadPoolExecutor) assert async_pool2.is_a?(Concurrent::ThreadPoolExecutor) - assert 0, async_pool1.min_length - assert 8, async_pool1.max_length - assert 32, async_pool1.max_queue - assert :caller_runs, async_pool1.fallback_policy + assert_equal 0, async_pool1.min_length + assert_equal 8, async_pool1.max_length + assert_equal 32, async_pool1.max_queue + assert_equal :caller_runs, async_pool1.fallback_policy - assert 0, async_pool2.min_length - assert 8, async_pool2.max_length - assert 32, async_pool2.max_queue - assert :caller_runs, async_pool2.fallback_policy + assert_equal 0, async_pool2.min_length + assert_equal 8, async_pool2.max_length + assert_equal 32, async_pool2.max_queue + assert_equal :caller_runs, async_pool2.fallback_policy assert_equal 2, handler.all_connection_pools.count assert_equal async_pool1, async_pool2 @@ -229,6 +231,7 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase clean_up_connection_handler ActiveRecord::Base.global_executor_concurrency = old_concurrency ActiveRecord::Base.async_query_executor = old_value + ActiveRecord::Core.class_variable_set(:@@global_thread_pool_async_query_executor, old_global_thread_pool_async_query_executor) end def test_concurrency_cannot_be_set_with_null_executor_or_multi_thread_pool @@ -266,15 +269,15 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase assert async_pool1.is_a?(Concurrent::ThreadPoolExecutor) assert async_pool2.is_a?(Concurrent::ThreadPoolExecutor) - assert 0, async_pool1.min_length - assert 10, async_pool1.max_length - assert 40, async_pool1.max_queue - assert :caller_runs, async_pool1.fallback_policy + assert_equal 0, async_pool1.min_length + assert_equal 10, async_pool1.max_length + assert_equal 40, async_pool1.max_queue + assert_equal :caller_runs, async_pool1.fallback_policy - assert 0, async_pool2.min_length - assert 4, async_pool2.max_length - assert 16, async_pool2.max_queue - assert :caller_runs, async_pool2.fallback_policy + assert_equal 0, async_pool2.min_length + assert_equal 5, async_pool2.max_length + assert_equal 20, async_pool2.max_queue + assert_equal :caller_runs, async_pool2.fallback_policy assert_equal 2, handler.all_connection_pools.count assert_not_equal async_pool1, async_pool2 @@ -306,10 +309,10 @@ class AsynchronousExecutorTypeTest < ActiveRecord::TestCase assert async_pool1.is_a?(Concurrent::ThreadPoolExecutor) assert_nil async_pool2 - assert 0, async_pool1.min_length - assert 10, async_pool1.max_length - assert 40, async_pool1.max_queue - assert :caller_runs, async_pool1.fallback_policy + assert_equal 0, async_pool1.min_length + assert_equal 10, async_pool1.max_length + assert_equal 40, async_pool1.max_queue + assert_equal :caller_runs, async_pool1.fallback_policy assert_equal 2, handler.all_connection_pools.count assert_not_equal async_pool1, async_pool2 From 302fdb7677b95ac1c428bd5c545252bc52ac2313 Mon Sep 17 00:00:00 2001 From: Ufuk Kayserilioglu Date: Wed, 3 Mar 2021 22:48:20 +0200 Subject: [PATCH 19/53] Add missing require --- activesupport/lib/active_support/current_attributes.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activesupport/lib/active_support/current_attributes.rb b/activesupport/lib/active_support/current_attributes.rb index 00dcfe99c4..93d6131516 100644 --- a/activesupport/lib/active_support/current_attributes.rb +++ b/activesupport/lib/active_support/current_attributes.rb @@ -2,6 +2,7 @@ require "active_support/callbacks" require "active_support/core_ext/enumerable" +require "active_support/core_ext/module/delegation" module ActiveSupport # Abstract super class that provides a thread-isolated attributes singleton, which resets automatically From b68f095478fc5aaa0f77963e13d36c09e129ed60 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 3 Mar 2021 17:45:39 +0900 Subject: [PATCH 20/53] Add CHANGELOG entry for type casting enum values by the subtype Related to #35336. The notable thing about #41516 is that unknown labels will no longer match 0 on MySQL. Matching unknown labels to 0 was not by design, but rather almost like a bug, people should not rely on that behavior. Closes #41595. --- activerecord/CHANGELOG.md | 30 ++++++++++++++++++++++++++++ activerecord/test/cases/enum_test.rb | 2 ++ 2 files changed, 32 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b87f24bb6d..3e173b930c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,33 @@ +* Type cast enum values by the original attribute type. + + The notable thing about this change is that unknown labels will no longer match 0 on MySQL. + + ```ruby + class Book < ActiveRecord::Base + enum :status, { proposed: 0, written: 1, published: 2 } + end + ``` + + Before: + + ```ruby + # SELECT `books`.* FROM `books` WHERE `books`.`status` = 'prohibited' LIMIT 1 + Book.find_by(status: :prohibited) + # => # (for mysql2 adapter) + # => ActiveRecord::StatementInvalid: PG::InvalidTextRepresentation: ERROR: invalid input syntax for type integer: "prohibited" (for postgresql adapter) + # => nil (for sqlite3 adapter) + ``` + + After: + + ```ruby + # SELECT `books`.* FROM `books` WHERE `books`.`status` IS NULL LIMIT 1 + Book.find_by(status: :prohibited) + # => nil (for all adapters) + ``` + + *Ryuta Kamizono* + * Fixtures for `has_many :through` associations now load timestamps on join tables Given this fixture: diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index c50833b62f..140712a657 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -80,6 +80,7 @@ class EnumTest < ActiveRecord::TestCase assert_not_equal @book, Book.where.not(status: :published).first assert_equal @book, Book.where.not(status: :written).first assert_equal books(:ddd), Book.where(last_read: :forgotten).first + assert_nil Book.where(status: :prohibited).first end test "find via where with strings" do @@ -90,6 +91,7 @@ class EnumTest < ActiveRecord::TestCase assert_not_equal @book, Book.where.not(status: "published").first assert_equal @book, Book.where.not(status: "written").first assert_equal books(:ddd), Book.where(last_read: "forgotten").first + assert_nil Book.where(status: "prohibited").first end test "find via where should be type casted" do From ea4a5a94060be0b0e793302c9188388749ad06d1 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 4 Mar 2021 18:40:10 +0900 Subject: [PATCH 21/53] Fix aggregate attribute on Enum types To address the issues #39248 and #39271, #39255 and #39274 made aggregated results type casted by the attribute types. But I've realised that we could avoid enum mapping when implemented #41431. This change restores the expectation of #39039 especially in the Enum case. Fixes #41600. --- .../lib/active_record/relation/calculations.rb | 2 ++ activerecord/test/cases/calculations_test.rb | 18 +++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 4ff93125d7..a2f6189bc3 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -310,6 +310,7 @@ module ActiveRecord if operation != "count" type = column.try(:type_caster) || lookup_cast_type_from_join_dependencies(column_name.to_s) || Type.default_value + type = type.subtype if Enum::EnumType === type end type_cast_calculated_value(result.cast_values.first, operation, type) @@ -384,6 +385,7 @@ module ActiveRecord if operation != "count" type = column.try(:type_caster) || lookup_cast_type_from_join_dependencies(column_name.to_s) || Type.default_value + type = type.subtype if Enum::EnumType === type end hash_rows.each_with_object({}) do |row, result| diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 04bc9a95ed..7e12f2f13b 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -1162,15 +1162,15 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal({ "proposed" => 2, "published" => 2 }, Book.group(:status).count) end - def test_aggregate_attribute_on_custom_type - assert_nil Book.sum(:status) - assert_equal "medium", Book.sum(:difficulty) - assert_equal "easy", Book.minimum(:difficulty) - assert_equal "medium", Book.maximum(:difficulty) - assert_equal({ "proposed" => "proposed", "published" => nil }, Book.group(:status).sum(:status)) - assert_equal({ "proposed" => "easy", "published" => "medium" }, Book.group(:status).sum(:difficulty)) - assert_equal({ "proposed" => "easy", "published" => "easy" }, Book.group(:status).minimum(:difficulty)) - assert_equal({ "proposed" => "easy", "published" => "medium" }, Book.group(:status).maximum(:difficulty)) + def test_aggregate_attribute_on_enum_type + assert_equal 4, Book.sum(:status) + assert_equal 1, Book.sum(:difficulty) + assert_equal 0, Book.minimum(:difficulty) + assert_equal 1, Book.maximum(:difficulty) + assert_equal({ "proposed" => 0, "published" => 4 }, Book.group(:status).sum(:status)) + assert_equal({ "proposed" => 0, "published" => 1 }, Book.group(:status).sum(:difficulty)) + assert_equal({ "proposed" => 0, "published" => 0 }, Book.group(:status).minimum(:difficulty)) + assert_equal({ "proposed" => 0, "published" => 1 }, Book.group(:status).maximum(:difficulty)) end def test_minimum_and_maximum_on_non_numeric_type From 1d9da5e0f11b5240a5cfe6b51d2e4f512e0d952c Mon Sep 17 00:00:00 2001 From: Elton Goci Date: Thu, 18 Feb 2021 18:00:56 +0100 Subject: [PATCH 22/53] Fix Postgres schema creation for create index stmt Fix the CREATE INDEX statement generation for Postgresql when both `algorithm` and `if_not_exists` are specified, so that the SQL statement is generated with the right order: - Previously: `CREATE INDEX IF NOT EXISTS CONCURRENTLY ...` - Now: `CREATE INDEX CONCURRENTLY IF NOT EXISTS ...` Also update tests to account for this scenario. --- .../connection_adapters/abstract/schema_creation.rb | 2 +- .../test/cases/adapters/postgresql/active_schema_test.rb | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index 216c924a01..55cce5e501 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -94,8 +94,8 @@ module ActiveRecord sql = ["CREATE"] sql << "UNIQUE" if index.unique sql << "INDEX" - sql << "IF NOT EXISTS" if o.if_not_exists sql << o.algorithm if o.algorithm + sql << "IF NOT EXISTS" if o.if_not_exists sql << index.type if index.type sql << "#{quote_column_name(index.name)} ON #{quote_table_name(index.table)}" sql << "USING #{index.using}" if supports_index_using? && index.using diff --git a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb index b5a197cae5..0d00830fa1 100644 --- a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb @@ -40,6 +40,9 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase expected = %(CREATE INDEX CONCURRENTLY "index_people_on_last_name" ON "people" ("last_name")) assert_equal expected, add_index(:people, :last_name, algorithm: :concurrently) + expected = %(CREATE INDEX CONCURRENTLY IF NOT EXISTS "index_people_on_last_name" ON "people" ("last_name")) + assert_equal expected, add_index(:people, :last_name, if_not_exists: true, algorithm: :concurrently) + expected = %(CREATE INDEX "index_people_on_last_name_and_first_name" ON "people" ("last_name" DESC, "first_name" ASC)) assert_equal expected, add_index(:people, [:last_name, :first_name], order: { last_name: :desc, first_name: :asc }) assert_equal expected, add_index(:people, ["last_name", :first_name], order: { last_name: :desc, "first_name" => :asc }) From 333f9b32d91e1cbe8676ff9eff499c39196f772c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 5 Mar 2021 02:37:54 +0900 Subject: [PATCH 23/53] Allow statement nodes' initializer takes a table Follow up to #41579. --- activerecord/lib/arel/delete_manager.rb | 3 +-- activerecord/lib/arel/insert_manager.rb | 3 +-- activerecord/lib/arel/nodes/insert_statement.rb | 4 ++-- activerecord/lib/arel/nodes/update_statement.rb | 5 +++-- activerecord/lib/arel/update_manager.rb | 3 +-- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/activerecord/lib/arel/delete_manager.rb b/activerecord/lib/arel/delete_manager.rb index 6872746a63..707376a566 100644 --- a/activerecord/lib/arel/delete_manager.rb +++ b/activerecord/lib/arel/delete_manager.rb @@ -6,9 +6,8 @@ module Arel # :nodoc: all def initialize(table = nil) super() - @ast = Nodes::DeleteStatement.new + @ast = Nodes::DeleteStatement.new(table) @ctx = @ast - @ast.relation = table end def from(relation) diff --git a/activerecord/lib/arel/insert_manager.rb b/activerecord/lib/arel/insert_manager.rb index 9ec82c960b..1fa7612e16 100644 --- a/activerecord/lib/arel/insert_manager.rb +++ b/activerecord/lib/arel/insert_manager.rb @@ -4,8 +4,7 @@ module Arel # :nodoc: all class InsertManager < Arel::TreeManager def initialize(table = nil) super() - @ast = Nodes::InsertStatement.new - @ast.relation = table + @ast = Nodes::InsertStatement.new(table) end def into(table) diff --git a/activerecord/lib/arel/nodes/insert_statement.rb b/activerecord/lib/arel/nodes/insert_statement.rb index d28fd1f6c8..fdfc179e66 100644 --- a/activerecord/lib/arel/nodes/insert_statement.rb +++ b/activerecord/lib/arel/nodes/insert_statement.rb @@ -5,9 +5,9 @@ module Arel # :nodoc: all class InsertStatement < Arel::Nodes::Node attr_accessor :relation, :columns, :values, :select - def initialize + def initialize(relation = nil) super() - @relation = nil + @relation = relation @columns = [] @values = nil @select = nil diff --git a/activerecord/lib/arel/nodes/update_statement.rb b/activerecord/lib/arel/nodes/update_statement.rb index cfaa19e392..c7554837e7 100644 --- a/activerecord/lib/arel/nodes/update_statement.rb +++ b/activerecord/lib/arel/nodes/update_statement.rb @@ -5,8 +5,9 @@ module Arel # :nodoc: all class UpdateStatement < Arel::Nodes::Node attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key - def initialize - @relation = nil + def initialize(relation = nil) + super() + @relation = relation @wheres = [] @values = [] @orders = [] diff --git a/activerecord/lib/arel/update_manager.rb b/activerecord/lib/arel/update_manager.rb index 170ea047df..434e82a0b0 100644 --- a/activerecord/lib/arel/update_manager.rb +++ b/activerecord/lib/arel/update_manager.rb @@ -6,9 +6,8 @@ module Arel # :nodoc: all def initialize(table = nil) super() - @ast = Nodes::UpdateStatement.new + @ast = Nodes::UpdateStatement.new(table) @ctx = @ast - @ast.relation = table end ### From 27e6fba1f4a50c4b581c8f30a75b5f4066d562ce Mon Sep 17 00:00:00 2001 From: Dave Powers Date: Thu, 4 Mar 2021 14:58:37 -0500 Subject: [PATCH 24/53] Update external links in security guide This changes fixes broken links, and updates URLs to remove redirects in order to preserve longevity. It also cleans up the grammar changing instances of "an SQL" to "a SQL." --- guides/source/security.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/guides/source/security.md b/guides/source/security.md index 15f536d584..4421599000 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -64,7 +64,7 @@ Hence, the cookie serves as temporary authentication for the web application. An * Instead of stealing a cookie unknown to the attacker, they fix a user's session identifier (in the cookie) known to them. Read more about this so-called session fixation later. -The main objective of most attackers is to make money. The underground prices for stolen bank login accounts range from 0.5%-10% of account balance, $0.5-$30 for credit card numbers ($20-$60 with full details), $0.1-$1.5 for identities (Name, SSN, and DOB), $20-$50 for retailer accounts, and $6-$10 for cloud service provider accounts, according to the [Symantec Internet Security Threat Report (2017)](https://www.symantec.com/content/dam/symantec/docs/reports/istr-22-2017-en.pdf). +The main objective of most attackers is to make money. The underground prices for stolen bank login accounts range from 0.5%-10% of account balance, $0.5-$30 for credit card numbers ($20-$60 with full details), $0.1-$1.5 for identities (Name, SSN, and DOB), $20-$50 for retailer accounts, and $6-$10 for cloud service provider accounts, according to the [Symantec Internet Security Threat Report (2017)](https://docs.broadcom.com/docs/istr-22-2017-en). ### Session Storage @@ -657,7 +657,7 @@ SELECT * FROM projects WHERE (name = '') UNION The result won't be a list of projects (because there is no project with an empty name), but a list of user names and their password. So hopefully you encrypted the passwords in the database! The only problem for the attacker is, that the number of columns has to be the same in both queries. That's why the second query includes a list of ones (1), which will be always the value 1, in order to match the number of columns in the first query. -Also, the second query renames some columns with the AS statement so that the web application displays the values from the user table. Be sure to update your Rails [to at least 2.1.1](http://www.rorsecurity.info/2008/09/08/sql-injection-issue-in-limit-and-offset-parameter/). +Also, the second query renames some columns with the AS statement so that the web application displays the values from the user table. Be sure to update your Rails [to at least 2.1.1](https://rorsecurity.info/journal/2008/09/08/sql-injection-issue-in-limit-and-offset-parameter.html). #### Countermeasures @@ -669,7 +669,7 @@ Instead of passing a string to the conditions option, you can pass an array to s Model.where("login = ? AND password = ?", entered_user_name, entered_password).first ``` -As you can see, the first part of the array is an SQL fragment with question marks. The sanitized versions of the variables in the second part of the array replace the question marks. Or you can pass a hash for the same result: +As you can see, the first part of the array is a SQL fragment with question marks. The sanitized versions of the variables in the second part of the array replace the question marks. Or you can pass a hash for the same result: ```ruby Model.where(login: entered_user_name, password: entered_password).first @@ -689,7 +689,7 @@ The most common entry points are message posts, user comments, and guest books, XSS attacks work like this: An attacker injects some code, the web application saves it and displays it on a page, later presented to a victim. Most XSS examples simply display an alert box, but it is more powerful than that. XSS can steal the cookie, hijack the session, redirect the victim to a fake website, display advertisements for the benefit of the attacker, change elements on the web site to get confidential information or install malicious software through security holes in the web browser. -During the second half of 2007, there were 88 vulnerabilities reported in Mozilla browsers, 22 in Safari, 18 in IE, and 12 in Opera. The [Symantec Global Internet Security threat report](http://eval.symantec.com/mktginfo/enterprise/white_papers/b-whitepaper_internet_security_threat_report_xiii_04-2008.en-us.pdf) also documented 239 browser plug-in vulnerabilities in the last six months of 2007. [Mpack](http://pandalabs.pandasecurity.com/mpack-uncovered/) is a very active and up-to-date attack framework which exploits these vulnerabilities. For criminal hackers, it is very attractive to exploit an SQL-Injection vulnerability in a web application framework and insert malicious code in every textual table column. In April 2008 more than 510,000 sites were hacked like this, among them the British government, United Nations, and many more high profile targets. +During the second half of 2007, there were 88 vulnerabilities reported in Mozilla browsers, 22 in Safari, 18 in IE, and 12 in Opera. The Symantec Global Internet Security threat report also documented 239 browser plug-in vulnerabilities in the last six months of 2007. [Mpack](https://www.pandasecurity.com/en/mediacenter/malware/mpack-uncovered/) is a very active and up-to-date attack framework which exploits these vulnerabilities. For criminal hackers, it is very attractive to exploit a SQL-Injection vulnerability in a web application framework and insert malicious code in every textual table column. In April 2008 more than 510,000 sites were hacked like this, among them the British government, United Nations, and many more high profile targets. #### HTML/JavaScript Injection @@ -728,7 +728,7 @@ The log files on www.attacker.com will read like this: GET http://www.attacker.com/_app_session=836c1c25278e5b321d6bea4f19cb57e2 ``` -You can mitigate these attacks (in the obvious way) by adding the **httpOnly** flag to cookies, so that `document.cookie` may not be read by JavaScript. HTTP only cookies can be used from IE v6.SP1, Firefox v2.0.0.5, Opera 9.5, Safari 4, and Chrome 1.0.154 onwards. But other, older browsers (such as WebTV and IE 5.5 on Mac) can actually cause the page to fail to load. Be warned that cookies [will still be visible using Ajax](https://www.owasp.org/index.php/HTTPOnly#Browsers_Supporting_HttpOnly), though. +You can mitigate these attacks (in the obvious way) by adding the **httpOnly** flag to cookies, so that `document.cookie` may not be read by JavaScript. HTTP only cookies can be used from IE v6.SP1, Firefox v2.0.0.5, Opera 9.5, Safari 4, and Chrome 1.0.154 onwards. But other, older browsers (such as WebTV and IE 5.5 on Mac) can actually cause the page to fail to load. Be warned that cookies [will still be visible using Ajax](https://owasp.org/www-community/HttpOnly#browsers-supporting-httponly), though. ##### Defacement @@ -787,7 +787,7 @@ This example pops up a message box. It will be recognized by the above `sanitize _In order to understand today's attacks on web applications, it's best to take a look at some real-world attack vectors._ -The following is an excerpt from the [Js.Yamanner@m](http://www.symantec.com/security_response/writeup.jsp?docid=2006-061211-4111-99&tabid=1) Yahoo! Mail [worm](http://groovin.net/stuff/yammer.txt). It appeared on June 11, 2006 and was the first webmail interface worm: +The following is an excerpt from the [Js.Yamanner@m Yahoo! Mail worm](https://community.broadcom.com/symantecenterprise/communities/community-home/librarydocuments/viewdocument?DocumentKey=12d8d106-1137-4d7c-8bb4-3ea1faec83fa). It appeared on June 11, 2006 and was the first webmail interface worm: ```html test`, which makes the text italic. However, up to the current version 3.0.4, it is still vulnerable to XSS. Get the [all-new version 4](http://www.redcloth.org) that removed serious bugs. However, even that version has [some security bugs](http://www.rorsecurity.info/journal/2008/10/13/new-redcloth-security.html), so the countermeasures still apply. Here is an example for version 3.0.4: +For example, RedCloth translates `_test_` to `test`, which makes the text italic. However, up to the current version 3.0.4, it is still vulnerable to XSS. Get the [all-new version 4](http://www.redcloth.org) that removed serious bugs. However, even that version has [some security bugs](https://rorsecurity.info/journal/2008/10/13/new-redcloth-security.html), so the countermeasures still apply. Here is an example for version 3.0.4: ```ruby RedCloth.new('').to_html @@ -1027,7 +1027,7 @@ Here is a list of common headers: * **X-Frame-Options:** _`SAMEORIGIN` in Rails by default_ - allow framing on same domain. Set it to 'DENY' to deny framing at all or remove this header completely if you want to allow framing on all websites. * **X-XSS-Protection:** _`1; mode=block` in Rails by default_ - use XSS Auditor and block page if XSS attack is detected. Set it to '0;' if you want to switch XSS Auditor off(useful if response contents scripts from request parameters) * **X-Content-Type-Options:** _`nosniff` in Rails by default_ - stops the browser from guessing the MIME type of a file. -* **X-Content-Security-Policy:** [A powerful mechanism for controlling which sites certain content types can be loaded from](http://w3c.github.io/webappsec/specs/content-security-policy/csp-specification.dev.html) +* **X-Content-Security-Policy:** [A powerful mechanism for controlling which sites certain content types can be loaded from](https://w3c.github.io/webappsec-csp/) * **Access-Control-Allow-Origin:** Used to control which sites are allowed to bypass same origin policies and send cross-origin requests. * **Strict-Transport-Security:** [Used to control if the browser is allowed to only access a site over a secure connection](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) @@ -1192,4 +1192,4 @@ The security landscape shifts and it is important to keep up to date, because mi * Subscribe to the Rails security [mailing list](https://groups.google.com/forum/#!forum/rubyonrails-security). * [Brakeman - Rails Security Scanner](https://brakemanscanner.org/) - To perform static security analysis for Rails applications. * [Mozilla's Web Security Guidelines](https://infosec.mozilla.org/guidelines/web_security.html) - Recommendations on topics covering Content Security Policy, HTTP headers, Cookies, TLS configuration, etc. -* A [good security blog](https://www.owasp.org) including the [Cross-Site scripting Cheat Sheet](https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md). +* A [good security blog](https://owasp.org/) including the [Cross-Site scripting Cheat Sheet](https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md). From 08145c251427acb5d17781159162af798163e955 Mon Sep 17 00:00:00 2001 From: Nat Morcos Date: Thu, 4 Mar 2021 18:15:27 -0500 Subject: [PATCH 25/53] Fix typo in Rails autoloading guide A missing word made the sentence difficult to understand. This PR adds the missing word. --- guides/source/autoloading_and_reloading_constants.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index 8f27ead76f..04b1373d53 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -98,7 +98,7 @@ $ bin/rails runner 'p UsersHelper' UsersHelper ``` -Autoload paths automatically pick any custom directories under `app`. For example, if your application has `app/presenters`, or `app/services`, etc., they are added to autoload paths. +Autoload paths automatically pick up any custom directories under `app`. For example, if your application has `app/presenters`, or `app/services`, etc., they are added to autoload paths. The array of autoload paths can be extended by mutating `config.autoload_paths`, in `config/application.rb`, but nowadays this is discouraged. From 3da5267f1ae765a764a74b8a067a5e7a4b05e8d3 Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Fri, 5 Mar 2021 10:13:51 +0100 Subject: [PATCH 26/53] Fix ActionMailer's deliver later default queue The default queue name used by `deliver_later` is no longer `mailers`. This commit removes the misleading information from the class documentation Ref: #40848 --- actionmailer/lib/action_mailer/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index e00cd1060e..560a4deeed 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -433,7 +433,7 @@ module ActionMailer # * deliveries - Keeps an array of all the emails sent out through the Action Mailer with # delivery_method :test. Most useful for unit and functional testing. # - # * deliver_later_queue_name - The name of the queue used with deliver_later. Defaults to +mailers+. + # * deliver_later_queue_name - The name of the queue used with deliver_later. class Base < AbstractController::Base include DeliveryMethods include Rescuable From 3f3f7af9e860fa6b489454bd0b1191a2a5bf63c6 Mon Sep 17 00:00:00 2001 From: Sohaib Talaat Bhatti Date: Fri, 5 Mar 2021 17:05:24 +0500 Subject: [PATCH 27/53] Fix phone_to API documentation sample output --- actionview/lib/action_view/helpers/url_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 8a4d1e3a8f..6baeba1fbf 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -646,10 +646,10 @@ module ActionView # # => 1234567890 # # phone_to "1234567890", "Phone me" - # # => Phone me + # # => Phone me # # phone_to "1234567890", country_code: "01" - # # => 1234567890 + # # => 1234567890 # # You can use a block as well if your link target is hard to fit into the name parameter. \ERB example: # From a5b8f00903ecf9e2599bff26634ab128f4485c31 Mon Sep 17 00:00:00 2001 From: Nat Morcos Date: Thu, 4 Mar 2021 19:15:40 -0500 Subject: [PATCH 28/53] Fix typos and grammatical errors in autoloading guide --- .../autoloading_and_reloading_constants.md | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index 8f27ead76f..c0feb69e93 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -122,13 +122,13 @@ Reloading Rails automatically reloads classes and modules if application files change. -More precisely, if the web server is running and application files have been modified, Rails unloads all autoloaded constants just before the next request is processed. That way, application classes or modules used during that request are going to be autoloaded, thus picking up their current implementation in the file system. +More precisely, if the web server is running and application files have been modified, Rails unloads all autoloaded constants just before the next request is processed. That way, application classes or modules used during that request will be autoloaded again, thus picking up their current implementation in the file system. Reloading can be enabled or disabled. The setting that controls this behavior is `config.cache_classes`, which is false by default in `development` mode (reloading enabled), and true by default in `production` mode (reloading disabled). -Rails detects files have changed using an evented file monitor (default), or walking the autoload paths, depending on `config.file_watcher`. +Rails uses an evented file monitor to detect files changes by default. It can be configured instead to detect file changes by walking the autoload paths. This is controlled by the `config.file_watcher` setting. -In a Rails console there is no file watcher active regardless of the value of `config.cache_classes`. This is so because, normally, it would be confusing to have code reloaded in the middle of a console session, the same way you generally want an individual request to be served by a consistent, non-changing set of application classes and modules. +In a Rails console there is no file watcher active regardless of the value of `config.cache_classes`. This is because, normally, it would be confusing to have code reloaded in the middle of a console session. Similar to an individual request, you generally want a console session to be served by a consistent, non-changing set of application classes and modules. However, you can force a reload in the console by executing `reload!`: @@ -167,7 +167,7 @@ There are several ways to do this safely. For instance, the application could de Let's see other situations that involve stale class or module objects. -Check this Rails console session: +Check out this Rails console session: ```irb irb> joe = User.new @@ -177,7 +177,7 @@ irb> joe.class == alice.class => false ``` -`joe` is an instance of the original `User` class. When there is a reload, the `User` constant evaluates to a different, reloaded class. `alice` is an instance of the current one, but `joe` is not, his class is stale. You may define `joe` again, start an IRB subsession, or just launch a new console instead of calling `reload!`. +`joe` is an instance of the original `User` class. When there is a reload, the `User` constant then evaluates to a different, reloaded class. `alice` is an instance of the newly loaded `User`, but `joe` is not — his class is stale. You may define `joe` again, start an IRB subsession, or just launch a new console instead of calling `reload!`. Another situation in which you may find this gotcha is subclassing reloadable classes in a place that is not reloaded: @@ -205,7 +205,7 @@ That block runs when the application boots, and every time code is reloaded. NOTE: For historical reasons, this callback may run twice. The code it executes must be idempotent. -However, if you do not need to reload the class, it is easier to define it in a directory which does not belong to the autoload paths. For instance, `lib` is an idiomatic choice, it does not belong to the autoload paths by default but it belongs to `$LOAD_PATH`. Then, in the place the class is needed at boot time, just perform a regular `require` to load it. +However, if you do not need to reload the class, it is easier to define it in a directory which does not belong to the autoload paths. For instance, `lib` is an idiomatic choice. It does not belong to the autoload paths by default, but it does belong to `$LOAD_PATH`. Then, in the place the class is needed at boot time, just perform a regular `require` to load it. For example, there is no point in defining reloadable Rack middleware, because changes would not be reflected in the instance stored in the middleware stack anyway. If `lib/my_app/middleware/foo.rb` defines a middleware class, then in `config/application.rb` you write: @@ -227,17 +227,17 @@ Eager loading is controlled by the flag `config.eager_load`, which is enabled by The order in which files are eager loaded is undefined. -if the `Zeitwerk` constant is defined, Rails invokes `Zeitwerk::Loader.eager_load_all` regardless of the application autoloading mode. That ensures dependencies managed by Zeitwerk are eager loaded. +If the `Zeitwerk` constant is defined, Rails invokes `Zeitwerk::Loader.eager_load_all` regardless of the application autoloading mode. That ensures dependencies managed by Zeitwerk are eager loaded. Single Table Inheritance ------------------------ -Single Table Inheritance is a feature that doesn't play well with lazy loading. Reason is, its API generally needs to be able to enumerate the STI hierarchy to work correctly, whereas lazy loading defers loading classes until they are referenced. You can't enumerate what you haven't referenced yet. +Single Table Inheritance is a feature that doesn't play well with lazy loading. The reason is: its API generally needs to be able to enumerate the STI hierarchy to work correctly, whereas lazy loading defers loading classes until they are referenced. You can't enumerate what you haven't referenced yet. In a sense, applications need to eager load STI hierarchies regardless of the loading mode. -Of course, if the application eager loads on boot, that is already accomplished. When it does not, it is in practice enough to instantiate the existing types in the database, which in development or test modes is usually fine. One way to do that is to throw this module into the `lib` directory: +Of course, if the application eager loads on boot, that is already accomplished. When it does not, it is in practice enough to instantiate the existing types in the database, which in development or test modes is usually fine. One way to do that is to include an STI preloading module in your `lib` directory: ```ruby module StiPreload @@ -306,7 +306,7 @@ end Customizing Inflections ----------------------- -By default, Rails uses `String#camelize` to know which constant should a given file or directory name define. For example, `posts_controller.rb` should define `PostsController` because that is what `"posts_controller".camelize` returns. +By default, Rails uses `String#camelize` to know which constant a given file or directory name should define. For example, `posts_controller.rb` should define `PostsController` because that is what `"posts_controller".camelize` returns. It could be the case that some particular file or directory name does not get inflected as you want. For instance, `html_parser.rb` is expected to define `HtmlParser` by default. What if you prefer the class to be `HTMLParser`? There are a few ways to customize this. @@ -344,22 +344,22 @@ Rails.autoloaders.each do |autoloader| end ``` -There is no global configuration that can affect said instances, they are deterministic. +There is no global configuration that can affect said instances; they are deterministic. -You can even define a custom inflector for full flexibility. Please, check the [Zeitwerk documentation](https://github.com/fxn/zeitwerk#custom-inflector) for further details. +You can even define a custom inflector for full flexibility. Please check the [Zeitwerk documentation](https://github.com/fxn/zeitwerk#custom-inflector) for further details. Troubleshooting --------------- The best way to follow what the loaders are doing is to inspect their activity. -The easiest way to do that is to throw +The easiest way to do that is to include ```ruby Rails.autoloaders.log! ``` -to `config/application.rb` after loading the framework defaults. That will print traces to standard output. +in `config/application.rb` after loading the framework defaults. That will print traces to standard output. If you prefer logging to a file, configure this instead: @@ -367,7 +367,7 @@ If you prefer logging to a file, configure this instead: Rails.autoloaders.logger = Logger.new("#{Rails.root}/log/autoloading.log") ``` -The Rails logger is still not ready in `config/application.rb`, but it is in initializers: +The Rails logger is not yet available when `config/application.rb` executes. If you prefer to use the Rails logger, configure this setting in an initializer instead: ```ruby # config/initializers/log_autoloaders.rb @@ -406,7 +406,7 @@ class Admin::UsersController < ApplicationController end ``` -was not recommended because the resolution of constants inside their body was brittle. You'd better write them in this style: +was not recommended because the resolution of constants inside their body was brittle. It was better to write them in this style: ```ruby module Admin @@ -415,11 +415,11 @@ module Admin end ``` -In `zeitwerk` mode that does not matter anymore, you can pick either style. +In `zeitwerk` mode that does not matter anymore. You can pick either style. -The resolution of a constant could depend on load order, the definition of a class or module object could depend on load order, there was edge cases with singleton classes, oftentimes you had to use `require_dependency` as a workaround, .... The guide for `classic` mode documents [these issues](autoloading_and_reloading_constants_classic_mode.html#common-gotchas). +In `classic` mode, the resolution of a constant could depend on load order, the definition of a class or module object could depend on load order, there were edge cases with singleton classes, oftentimes you had to use `require_dependency` as a workaround, .... The list goes on. The guide for `classic` mode documents [these issues](autoloading_and_reloading_constants_classic_mode.html#common-gotchas). -All these problems are solved in `zeitwerk` mode, it just works as expected, and `require_dependency` should not be used anymore, it is no longer needed. +All these problems are solved in `zeitwerk` mode. It just works as expected. `require_dependency` should not be used anymore, because it is no longer needed. ### Less File Lookups @@ -437,19 +437,19 @@ While in common names these operations match, if acronyms or custom inflection r ### More Differences -There are some other subtle differences, please check [this section of _Upgrading Ruby on Rails_](upgrading_ruby_on_rails.html#autoloading) guide for details. +There are some other subtle differences. Please check the [autoloading section](upgrading_ruby_on_rails.html#autoloading) of the _Upgrading Ruby on Rails_] guide for details. Classic Mode is Deprecated -------------------------- -By now, it is still possible to use `classic` mode. However, `classic` is deprecated and will be eventually removed. +Currently, it is still possible to use `classic` mode. However, `classic` is deprecated and will be eventually removed. -New applications should use `zeitwerk` mode (which is the default), and applications being upgrade are strongly encouraged to migrate to `zeitwerk` mode. Please check the [_Upgrading Ruby on Rails_](upgrading_ruby_on_rails.html#autoloading) guide for details. +New applications should use `zeitwerk` mode (which is the default), and applications being upgraded are strongly encouraged to migrate to `zeitwerk` mode. Please check the [_Upgrading Ruby on Rails_](upgrading_ruby_on_rails.html#autoloading) guide for details. Opting Out ---------- -Applications can load Rails 6 defaults and still use the classic autoloader this way: +Applications can load Rails 6 defaults and still use the `classic` autoloader this way: ```ruby # config/application.rb @@ -457,6 +457,6 @@ config.load_defaults 6.0 config.autoloader = :classic ``` -That may be handy if upgrading to Rails 6 in different phases, but classic mode is discouraged for new applications. +That may be handy if upgrading to Rails 6 in different phases, but `classic` mode is discouraged for new applications. -`zeitwerk` mode is not available in versions of Rails previous to 6.0. +`zeitwerk` mode is not available in versions of Rails prior to 6.0. From c0140a7f89d2f061158e21a9888498adf38135f4 Mon Sep 17 00:00:00 2001 From: Henrik Nyh Date: Fri, 5 Mar 2021 17:38:52 +0000 Subject: [PATCH 29/53] CollectionProxy#any? docs: Mention "load.any?" To go with the change in #41572. --- .../lib/active_record/associations/collection_proxy.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index e06c991e0f..570d50a53c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -849,6 +849,11 @@ module ActiveRecord # person.pets.count # => 1 # person.pets.any? # => true # + # Calling it without a block when the collection is not yet + # loaded is equivalent to collection.exists?. + # If you're going to load the collection anyway, it is better + # to call collection.load.any? to avoid an extra query. + # # You can also pass a +block+ to define criteria. The behavior # is the same, it returns true if the collection based on the # criteria is not empty. From 6cd184ff0f1061141c54ee4bad960c4d1467bcf9 Mon Sep 17 00:00:00 2001 From: Christian Schmidt Date: Fri, 5 Mar 2021 19:14:11 +0100 Subject: [PATCH 30/53] Use image/jpeg instead of non-standard image/jpg --- .../mandrill/inbound_emails_controller_test.rb | 2 +- actionmailbox/test/fixtures/files/welcome.eml | 4 ++-- .../action_controller/metal/mime_responds.rb | 2 +- actionpack/test/controller/integration_test.rb | 2 +- actionpack/test/controller/test_case_test.rb | 18 +++++++++--------- .../request/multipart_params_parsing_test.rb | 2 +- actiontext/test/unit/attachment_test.rb | 2 +- actiontext/test/unit/content_test.rb | 4 ++-- actiontext/test/unit/model_test.rb | 6 +++--- activestorage/README.md | 2 +- .../lib/active_storage/attached/many.rb | 2 +- .../lib/active_storage/attached/one.rb | 2 +- .../test/controllers/disk_controller_test.rb | 8 ++++---- .../test/models/attached/many_test.rb | 12 ++++++------ activestorage/test/models/attached/one_test.rb | 14 +++++++------- .../test/service/shared_service_tests.rb | 2 +- guides/source/kindle/rails_guides.opf.erb | 2 +- 17 files changed, 43 insertions(+), 43 deletions(-) diff --git a/actionmailbox/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb index 7e73c397c4..7dc900aa68 100644 --- a/actionmailbox/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb +++ b/actionmailbox/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb @@ -19,7 +19,7 @@ class ActionMailbox::Ingresses::Mandrill::InboundEmailsControllerTest < ActionDi test "receiving an inbound email from Mandrill" do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do post rails_mandrill_inbound_emails_url, - headers: { "X-Mandrill-Signature" => "gldscd2tAb/G+DmpiLcwukkLrC4=" }, params: { mandrill_events: @events } + headers: { "X-Mandrill-Signature" => "1bNbyqkMFL4VYIT5+RQCrPs/mRY=" }, params: { mandrill_events: @events } end assert_response :ok diff --git a/actionmailbox/test/fixtures/files/welcome.eml b/actionmailbox/test/fixtures/files/welcome.eml index 5d6b3c1ea5..27fd51c58a 100644 --- a/actionmailbox/test/fixtures/files/welcome.eml +++ b/actionmailbox/test/fixtures/files/welcome.eml @@ -27,7 +27,7 @@ Content-Type: multipart/related; Content-Transfer-Encoding: base64 Content-Disposition: inline; filename=avatar1.jpeg -Content-Type: image/jpg; +Content-Type: image/jpeg; name="avatar1.jpeg" Content-Id: <7AAEB353-2341-4D46-A054-5CA5CB2363B7> @@ -397,7 +397,7 @@ mk8VWW5WRGJGAOaAP//Z Content-Transfer-Encoding: base64 Content-Disposition: inline; filename=avatar2.jpg -Content-Type: image/jpg; +Content-Type: image/jpeg; x-unix-mode=0700; name="avatar2.jpg" Content-Id: <4594E827-6E69-4329-8691-6BC35E3E73A0> diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 0a80e7e6db..fda5dbbc03 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -103,7 +103,7 @@ module ActionController #:nodoc: # If you need to use a MIME type which isn't supported by default, you can register your own handlers in # +config/initializers/mime_types.rb+ as follows. # - # Mime::Type.register "image/jpg", :jpg + # Mime::Type.register "image/jpeg", :jpg # # +respond_to+ also allows you to specify a common block for different formats by using +any+: # diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 2b73a471f4..2e1a54fbf8 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -1229,7 +1229,7 @@ class IntegrationFileUploadTest < ActionDispatch::IntegrationTest def test_fixture_file_upload post "/test_file_upload", params: { - file: fixture_file_upload("/ruby_on_rails.jpg", "image/jpg") + file: fixture_file_upload("/ruby_on_rails.jpg", "image/jpeg") } assert_equal "45142", @response.body end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 445b1c0aac..03a16e5843 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -909,7 +909,7 @@ XML def test_fixture_file_upload_with_binary filename = "ruby_on_rails.jpg" path = "#{FILES_DIR}/#{filename}" - content_type = "image/jpg" + content_type = "image/jpeg" binary_file_upload = fixture_file_upload(path, content_type, :binary) assert_equal File.open(path, READ_BINARY).read, binary_file_upload.read @@ -919,14 +919,14 @@ XML end def test_fixture_file_upload_should_be_able_access_to_tempfile - file = fixture_file_upload(FILES_DIR + "/ruby_on_rails.jpg", "image/jpg") + file = fixture_file_upload(FILES_DIR + "/ruby_on_rails.jpg", "image/jpeg") assert_respond_to file, :tempfile end def test_fixture_file_upload post :test_file_upload, params: { - file: fixture_file_upload(FILES_DIR + "/ruby_on_rails.jpg", "image/jpg") + file: fixture_file_upload(FILES_DIR + "/ruby_on_rails.jpg", "image/jpeg") } assert_equal "45142", @response.body end @@ -935,7 +935,7 @@ XML TestCaseTest.stub :fixture_path, File.expand_path("../fixtures", __dir__) do TestCaseTest.stub :file_fixture_path, nil do assert_deprecated(/In Rails 7.0, the path needs to be relative to `file_fixture_path`/) do - fixture_file_upload("multipart/ruby_on_rails.jpg", "image/jpg") + fixture_file_upload("multipart/ruby_on_rails.jpg", "image/jpeg") end end end @@ -944,7 +944,7 @@ XML def test_fixture_file_upload_does_not_output_deprecation_when_file_fixture_path_is_set TestCaseTest.stub :fixture_path, File.expand_path("../fixtures", __dir__) do assert_not_deprecated do - fixture_file_upload("ruby_on_rails.jpg", "image/jpg") + fixture_file_upload("ruby_on_rails.jpg", "image/jpeg") end end end @@ -954,7 +954,7 @@ XML expected = "`fixture_file_upload(\"multipart/ruby_on_rails.jpg\")` to `fixture_file_upload(\"ruby_on_rails.jpg\")`" assert_deprecated(expected) do - uploaded_file = fixture_file_upload("multipart/ruby_on_rails.jpg", "image/jpg") + uploaded_file = fixture_file_upload("multipart/ruby_on_rails.jpg", "image/jpeg") assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read end end @@ -975,13 +975,13 @@ XML def test_fixture_file_upload_ignores_fixture_path_given_full_path TestCaseTest.stub :fixture_path, __dir__ do - uploaded_file = fixture_file_upload("#{FILES_DIR}/ruby_on_rails.jpg", "image/jpg") + uploaded_file = fixture_file_upload("#{FILES_DIR}/ruby_on_rails.jpg", "image/jpeg") assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read end end def test_fixture_file_upload_ignores_nil_fixture_path - uploaded_file = fixture_file_upload("#{FILES_DIR}/ruby_on_rails.jpg", "image/jpg") + uploaded_file = fixture_file_upload("#{FILES_DIR}/ruby_on_rails.jpg", "image/jpeg") assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read end @@ -989,7 +989,7 @@ XML filename = "ruby_on_rails.jpg" path = "#{FILES_DIR}/#{filename}" post :test_file_upload, params: { - file: Rack::Test::UploadedFile.new(path, "image/jpg", true) + file: Rack::Test::UploadedFile.new(path, "image/jpeg", true) } assert_equal "45142", @response.body end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 7c0d2bd4b8..830920f375 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -157,7 +157,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest test "uploads and reads binary file" do with_test_routing do fixture = FIXTURE_PATH + "/ruby_on_rails.jpg" - params = { uploaded_data: fixture_file_upload(fixture, "image/jpg") } + params = { uploaded_data: fixture_file_upload(fixture, "image/jpeg") } post "/read", params: params end end diff --git a/actiontext/test/unit/attachment_test.rb b/actiontext/test/unit/attachment_test.rb index 9ed1967450..4b5b07ac7c 100644 --- a/actiontext/test/unit/attachment_test.rb +++ b/actiontext/test/unit/attachment_test.rb @@ -66,6 +66,6 @@ class ActionText::AttachmentTest < ActiveSupport::TestCase end def attachable - @attachment ||= create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + @attachment ||= create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") end end diff --git a/actiontext/test/unit/content_test.rb b/actiontext/test/unit/content_test.rb index dcedb8b1d4..dc9d4a1141 100644 --- a/actiontext/test/unit/content_test.rb +++ b/actiontext/test/unit/content_test.rb @@ -28,7 +28,7 @@ class ActionText::ContentTest < ActiveSupport::TestCase end test "extracts attachables" do - attachable = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + attachable = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") html = %Q() content = content_from_html(html) @@ -56,7 +56,7 @@ class ActionText::ContentTest < ActiveSupport::TestCase end test "identifies destroyed attachables as missing" do - attachable = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + attachable = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") html = %Q() attachable.destroy! content = content_from_html(html) diff --git a/actiontext/test/unit/model_test.rb b/actiontext/test/unit/model_test.rb index a26cebab3d..7193a82714 100644 --- a/actiontext/test/unit/model_test.rb +++ b/actiontext/test/unit/model_test.rb @@ -32,14 +32,14 @@ class ActionText::ModelTest < ActiveSupport::TestCase end test "embed extraction" do - blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") message = Message.create!(subject: "Greetings", content: ActionText::Content.new("Hello world").append_attachables(blob)) assert_equal "racecar.jpg", message.content.embeds.first.filename.to_s end test "embed extraction only extracts file attachments" do remote_image_html = '' - blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") content = ActionText::Content.new(remote_image_html).append_attachables(blob) message = Message.create!(subject: "Greetings", content: content) assert_equal [ActionText::Attachables::RemoteImage, ActiveStorage::Blob], message.content.body.attachables.map(&:class) @@ -47,7 +47,7 @@ class ActionText::ModelTest < ActiveSupport::TestCase end test "embed extraction deduplicates file attachments" do - blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") content = ActionText::Content.new("Hello world").append_attachables([ blob, blob ]) assert_nothing_raised do diff --git a/activestorage/README.md b/activestorage/README.md index b5bca0af26..6cc791c839 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -32,7 +32,7 @@ class User < ApplicationRecord end # Attach an avatar to the user. -user.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpg") +user.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpeg") # Does the user have an avatar? user.avatar.attached? # => true diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index 913b534f52..5afa3a3951 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -25,7 +25,7 @@ module ActiveStorage # # document.images.attach(params[:images]) # Array of ActionDispatch::Http::UploadedFile objects # document.images.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload - # document.images.attach(io: File.open("/path/to/racecar.jpg"), filename: "racecar.jpg", content_type: "image/jpg") + # document.images.attach(io: File.open("/path/to/racecar.jpg"), filename: "racecar.jpg", content_type: "image/jpeg") # document.images.attach([ first_blob, second_blob ]) def attach(*attachables) if record.persisted? && !record.changed? diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index 2e8b2a91cf..73527b08d2 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -25,7 +25,7 @@ module ActiveStorage # # person.avatar.attach(params[:avatar]) # ActionDispatch::Http::UploadedFile object # person.avatar.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload - # person.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpg") + # person.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpeg") # person.avatar.attach(avatar_blob) # ActiveStorage::Blob object def attach(attachable) if record.persisted? && !record.changed? diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb index f0248f6854..0183855ed3 100644 --- a/activestorage/test/controllers/disk_controller_test.rb +++ b/activestorage/test/controllers/disk_controller_test.rb @@ -5,12 +5,12 @@ require "database/setup" class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest test "showing blob inline" do - blob = create_blob(filename: "hello.jpg", content_type: "image/jpg") + blob = create_blob(filename: "hello.jpg", content_type: "image/jpeg") get blob.url assert_response :ok assert_equal "inline; filename=\"hello.jpg\"; filename*=UTF-8''hello.jpg", response.headers["Content-Disposition"] - assert_equal "image/jpg", response.headers["Content-Type"] + assert_equal "image/jpeg", response.headers["Content-Type"] assert_equal "Hello world!", response.body end @@ -46,11 +46,11 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest test "showing public blob" do with_service("local_public") do - blob = create_blob(content_type: "image/jpg") + blob = create_blob(content_type: "image/jpeg") get blob.url assert_response :ok - assert_equal "image/jpg", response.headers["Content-Type"] + assert_equal "image/jpeg", response.headers["Content-Type"] assert_equal "Hello world!", response.body end end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 1cfdd3182b..410abdac1c 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -31,7 +31,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase test "attaching new blobs from Hashes to an existing record" do @user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" }, + { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpeg" }, { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpeg" }) assert_equal "funky.jpg", @user.highlights.first.filename.to_s @@ -81,7 +81,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase assert @user.changed? @user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" }, + { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpeg" }, { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpeg" }) assert_equal "funky.jpg", @user.highlights.first.filename.to_s @@ -337,8 +337,8 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase test "attaching new blobs from Hashes to a new record" do User.new(name: "Jason").tap do |user| user.highlights.attach( - { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpg" }, - { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpg" }) + { io: StringIO.new("STUFF"), filename: "funky.jpg", content_type: "image/jpeg" }, + { io: StringIO.new("THINGS"), filename: "town.jpg", content_type: "image/jpeg" }) assert user.new_record? assert user.highlights.first.new_record? @@ -600,8 +600,8 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase test "attaching a new blob from a Hash with a custom service" do with_service("mirror") do - @user.highlights.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" - @user.vlogs.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + @user.highlights.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" + @user.vlogs.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" assert_instance_of ActiveStorage::Service::MirrorService, @user.highlights.first.service assert_instance_of ActiveStorage::Service::DiskService, @user.vlogs.first.service diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index ac42dc5c65..6c5f885bb5 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -38,12 +38,12 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase end test "attaching a new blob from a Hash to an existing record" do - @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" assert_equal "town.jpg", @user.avatar.filename.to_s end test "attaching a new blob from a Hash to an existing record passes record" do - hash = { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" } + hash = { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" } blob = ActiveStorage::Blob.build_after_unfurling(**hash) arguments = [hash.merge(record: @user, service_name: nil)] assert_called_with(ActiveStorage::Blob, :build_after_unfurling, arguments, returns: blob) do @@ -98,7 +98,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase @user.name = "Tina" assert @user.changed? - @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" assert_equal "town.jpg", @user.avatar.filename.to_s assert_not @user.avatar.persisted? assert @user.will_save_change_to_name? @@ -320,7 +320,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase end test "creating an attachment as part of an autosave association through nested attributes" do - group = Group.create!(users_attributes: [{ name: "John", avatar: { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" } }]) + group = Group.create!(users_attributes: [{ name: "John", avatar: { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" } }]) group.save! new_user = User.find_by(name: "John") assert new_user.avatar.attached? @@ -358,7 +358,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase test "attaching a new blob from a Hash to a new record" do User.new(name: "Jason").tap do |user| - user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" assert user.new_record? assert user.avatar.attachment.new_record? assert user.avatar.blob.new_record? @@ -582,8 +582,8 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase test "attaching a new blob from a Hash with a custom service" do with_service("mirror") do - @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" - @user.cover_photo.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" + @user.avatar.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" + @user.cover_photo.attach io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpeg" assert_instance_of ActiveStorage::Service::MirrorService, @user.avatar.service assert_instance_of ActiveStorage::Service::DiskService, @user.cover_photo.service diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index 3d4884b50c..c6d83a3553 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -50,7 +50,7 @@ module ActiveStorage::Service::SharedServiceTests StringIO.new(data), checksum: Digest::MD5.base64digest(data), filename: "racecar.jpg", - content_type: "image/jpg" + content_type: "image/jpeg" ) assert_equal data, @service.download(key) diff --git a/guides/source/kindle/rails_guides.opf.erb b/guides/source/kindle/rails_guides.opf.erb index 83d00cfc2e..06bfac8fc8 100644 --- a/guides/source/kindle/rails_guides.opf.erb +++ b/guides/source/kindle/rails_guides.opf.erb @@ -32,7 +32,7 @@ - + From bece535c9855d8b1f099ea7a9dd66e2c94157094 Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Fri, 5 Mar 2021 15:57:36 -0600 Subject: [PATCH 31/53] Add config.action_text.attachment_tag_name --- actiontext/CHANGELOG.md | 4 ++++ .../app/helpers/action_text/content_helper.rb | 2 +- actiontext/lib/action_text/attachment.rb | 6 ++--- .../lib/action_text/attachment_gallery.rb | 2 +- .../action_text/attachments/minification.rb | 2 +- actiontext/lib/action_text/content.rb | 4 ++-- actiontext/lib/action_text/engine.rb | 7 ++++++ actiontext/test/unit/content_test.rb | 18 ++++++++++++++ guides/source/configuring.md | 4 ++++ .../test/application/configuration_test.rb | 24 +++++++++++++++++++ 10 files changed, 65 insertions(+), 8 deletions(-) diff --git a/actiontext/CHANGELOG.md b/actiontext/CHANGELOG.md index 5853a0e4d2..8a2c1b3586 100644 --- a/actiontext/CHANGELOG.md +++ b/actiontext/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add `config.action_text.attachment_tag_name`, to specify the HTML tag that contains attachments. + + *Mark VanLandingham* + * Expose how we render the HTML _surrounding_ rich text content as an extensible `layouts/action_view/contents/_content.html.erb` template to encourage user-land customizations, while retaining private API control over how diff --git a/actiontext/app/helpers/action_text/content_helper.rb b/actiontext/app/helpers/action_text/content_helper.rb index d7c32a470d..f0f3521ccd 100644 --- a/actiontext/app/helpers/action_text/content_helper.rb +++ b/actiontext/app/helpers/action_text/content_helper.rb @@ -5,7 +5,7 @@ require "rails-html-sanitizer" module ActionText module ContentHelper mattr_accessor(:sanitizer) { Rails::Html::Sanitizer.safe_list_sanitizer.new } - mattr_accessor(:allowed_tags) { sanitizer.class.allowed_tags + [ ActionText::Attachment::TAG_NAME, "figure", "figcaption" ] } + mattr_accessor(:allowed_tags) { sanitizer.class.allowed_tags + [ ActionText::Attachment.tag_name, "figure", "figcaption" ] } mattr_accessor(:allowed_attributes) { sanitizer.class.allowed_attributes + ActionText::Attachment::ATTRIBUTES } mattr_accessor(:scrubber) diff --git a/actiontext/lib/action_text/attachment.rb b/actiontext/lib/action_text/attachment.rb index 4bd537c7c2..b91d6133bc 100644 --- a/actiontext/lib/action_text/attachment.rb +++ b/actiontext/lib/action_text/attachment.rb @@ -6,8 +6,8 @@ module ActionText class Attachment include Attachments::TrixConversion, Attachments::Minification, Attachments::Caching - TAG_NAME = "action-text-attachment" - SELECTOR = TAG_NAME + mattr_accessor :tag_name, default: "action-text-attachment" + ATTRIBUTES = %w( sgid content-type url href filename filesize width height previewable presentation caption ) class << self @@ -38,7 +38,7 @@ module ActionText private def node_from_attributes(attributes) if attributes = process_attributes(attributes).presence - ActionText::HtmlConversion.create_element(TAG_NAME, attributes) + ActionText::HtmlConversion.create_element(tag_name, attributes) end end diff --git a/actiontext/lib/action_text/attachment_gallery.rb b/actiontext/lib/action_text/attachment_gallery.rb index 48ba9f830c..a3c1c33f80 100644 --- a/actiontext/lib/action_text/attachment_gallery.rb +++ b/actiontext/lib/action_text/attachment_gallery.rb @@ -57,7 +57,7 @@ module ActionText end TAG_NAME = "div" - ATTACHMENT_SELECTOR = "#{ActionText::Attachment::SELECTOR}[presentation=gallery]" + ATTACHMENT_SELECTOR = "#{ActionText::Attachment.tag_name}[presentation=gallery]" SELECTOR = "#{TAG_NAME}:has(#{ATTACHMENT_SELECTOR} + #{ATTACHMENT_SELECTOR})" private_constant :TAG_NAME, :ATTACHMENT_SELECTOR, :SELECTOR diff --git a/actiontext/lib/action_text/attachments/minification.rb b/actiontext/lib/action_text/attachments/minification.rb index edc8f876d6..75a113163f 100644 --- a/actiontext/lib/action_text/attachments/minification.rb +++ b/actiontext/lib/action_text/attachments/minification.rb @@ -7,7 +7,7 @@ module ActionText class_methods do def fragment_by_minifying_attachments(content) - Fragment.wrap(content).replace(ActionText::Attachment::SELECTOR) do |node| + Fragment.wrap(content).replace(ActionText::Attachment.tag_name) do |node| node.tap { |n| n.inner_html = "" } end end diff --git a/actiontext/lib/action_text/content.rb b/actiontext/lib/action_text/content.rb index de43880a0c..c238d0266c 100644 --- a/actiontext/lib/action_text/content.rb +++ b/actiontext/lib/action_text/content.rb @@ -58,7 +58,7 @@ module ActionText end def render_attachments(**options, &block) - content = fragment.replace(ActionText::Attachment::SELECTOR) do |node| + content = fragment.replace(ActionText::Attachment.tag_name) do |node| block.call(attachment_for_node(node, **options)) end self.class.new(content, canonicalize: false) @@ -111,7 +111,7 @@ module ActionText private def attachment_nodes - @attachment_nodes ||= fragment.find_all(ActionText::Attachment::SELECTOR) + @attachment_nodes ||= fragment.find_all(ActionText::Attachment.tag_name) end def attachment_gallery_nodes diff --git a/actiontext/lib/action_text/engine.rb b/actiontext/lib/action_text/engine.rb index e78c95b69b..46a2a23789 100644 --- a/actiontext/lib/action_text/engine.rb +++ b/actiontext/lib/action_text/engine.rb @@ -12,6 +12,9 @@ module ActionText isolate_namespace ActionText config.eager_load_namespaces << ActionText + config.action_text = ActiveSupport::OrderedOptions.new + config.action_text.attachment_tag_name = "action-text-attachment" + initializer "action_text.attribute" do ActiveSupport.on_load(:active_record) do include ActionText::Attribute @@ -64,5 +67,9 @@ module ActionText include ActionText::SystemTestHelper end end + + initializer "action_text.configure" do |app| + ActionText::Attachment.tag_name = app.config.action_text.attachment_tag_name + end end end diff --git a/actiontext/test/unit/content_test.rb b/actiontext/test/unit/content_test.rb index dc9d4a1141..c275e523f3 100644 --- a/actiontext/test/unit/content_test.rb +++ b/actiontext/test/unit/content_test.rb @@ -78,6 +78,15 @@ class ActionText::ContentTest < ActiveSupport::TestCase assert_equal '', content.to_html end + test "converts Trix-formatted attachments with custom tag name" do + with_attachment_tag_name("arbitrary-tag") do + html = %Q(
) + content = content_from_html(html) + assert_equal 1, content.attachments.size + assert_equal '', content.to_html + end + end + test "ignores Trix-formatted attachments with malformed JSON" do html = %Q(
) content = content_from_html(html) @@ -131,4 +140,13 @@ class ActionText::ContentTest < ActiveSupport::TestCase assert_nothing_raised { content.to_s } end end + + def with_attachment_tag_name(tag_name) + previous_tag_name = ActionText::Attachment.tag_name + ActionText::Attachment.tag_name = tag_name + + yield + ensure + ActionText::Attachment.tag_name = previous_tag_name + end end diff --git a/guides/source/configuring.md b/guides/source/configuring.md index bce5bf9601..6ffe699532 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1043,6 +1043,10 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla The default is `:rails_storage_redirect`. +### Configuring Action Text + +* `config.action_text.attachment_tag_name` accepts a string for the HTML tag used to wrap attachments. Defaults to `"action-text-attachment"`. + ### Results of `config.load_defaults` `config.load_defaults` sets new defaults up to and including the version passed. Such that passing, say, `6.0` also gets the new defaults from every version before it. diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index c6f4100e9c..374edcdee0 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2760,6 +2760,30 @@ module ApplicationTests assert_kind_of ActiveSupport::HashWithIndifferentAccess, ActionCable.server.config.cable end + test "action_text.config.attachment_tag_name is 'action-text-attachment' with Rails 6 defaults" do + add_to_config 'config.load_defaults "6.1"' + + app "development" + + assert_equal "action-text-attachment", ActionText::Attachment.tag_name + end + + test "action_text.config.attachment_tag_name is 'action-text-attachment' without defaults" do + remove_from_config '.*config\.load_defaults.*\n' + + app "development" + + assert_equal "action-text-attachment", ActionText::Attachment.tag_name + end + + test "action_text.config.attachment_tag_name is can be overriden" do + add_to_config "config.action_text.attachment_tag_name = 'link'" + + app "development" + + assert_equal "link", ActionText::Attachment.tag_name + end + test "ActionMailbox.logger is Rails.logger by default" do app "development" From ddbf01c52e5094caf042f89b10bbe86d372b52e9 Mon Sep 17 00:00:00 2001 From: Abhay Nikam Date: Sun, 21 Feb 2021 12:49:11 +0530 Subject: [PATCH 32/53] @rails/actiontext: depend on released @rails/activestorage --- actiontext/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actiontext/package.json b/actiontext/package.json index 4bebab9754..aa15f02358 100644 --- a/actiontext/package.json +++ b/actiontext/package.json @@ -21,7 +21,7 @@ ], "license": "MIT", "dependencies": { - "@rails/activestorage": "^6.0.0-alpha" + "@rails/activestorage": "^6.0.0" }, "peerDependencies": { "trix": "^1.2.0" From abcb5f5050e261093833e97d1dafbf49d2a0b8f2 Mon Sep 17 00:00:00 2001 From: John Bampton Date: Sun, 7 Mar 2021 18:09:45 +1000 Subject: [PATCH 33/53] chore: fix spelling --- activerecord/lib/active_record/core.rb | 2 +- activerecord/test/cases/attribute_methods_test.rb | 4 ++-- .../test/cases/database_configurations/hash_config_test.rb | 2 +- .../test/cases/validations/numericality_validation_test.rb | 2 +- railties/test/application/configuration_test.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 405754464b..ee0cbf9d38 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -161,7 +161,7 @@ module ActiveRecord # set to +nil+ which will not run queries in the background. Applications must configure # a thread pool executor to use this feature. Options are: # - # * nil - Does not initalize a thread pool executor. Any async calls will be + # * nil - Does not initialize a thread pool executor. Any async calls will be # run in the foreground. # * :global_thread_pool - Initializes a single +Concurrent::ThreadPoolExecutor+ # that uses the +async_query_concurrency+ for the +max_threads+ value. diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 4e42cade62..0e20ceb3a1 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -430,14 +430,14 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_equal "a", topic[:title] end - test "read overriden attribute with predicate respects override" do + test "read overridden attribute with predicate respects override" do topic = Topic.new topic.approved = true def topic.approved; false; end - assert_not topic.approved?, "overriden approved should be false" + assert_not topic.approved?, "overridden approved should be false" end test "string attribute predicate" do diff --git a/activerecord/test/cases/database_configurations/hash_config_test.rb b/activerecord/test/cases/database_configurations/hash_config_test.rb index f7991d5866..c898b4bdf5 100644 --- a/activerecord/test/cases/database_configurations/hash_config_test.rb +++ b/activerecord/test/cases/database_configurations/hash_config_test.rb @@ -47,7 +47,7 @@ module ActiveRecord assert_equal 1, config.max_threads end - def test_max_queue_is_pool_multipled_by_4 + def test_max_queue_is_pool_multiplied_by_4 config = HashConfig.new("default_env", "primary", {}) assert_equal 5, config.max_threads assert_equal config.max_threads * 4, config.max_queue diff --git a/activerecord/test/cases/validations/numericality_validation_test.rb b/activerecord/test/cases/validations/numericality_validation_test.rb index 941637a860..80d3e9a9e9 100644 --- a/activerecord/test/cases/validations/numericality_validation_test.rb +++ b/activerecord/test/cases/validations/numericality_validation_test.rb @@ -102,7 +102,7 @@ class NumericalityValidationTest < ActiveRecord::TestCase if RUBY_VERSION > "3.0.0" # BigDecimal's to_d behavior changed in BigDecimal 3.0.1, see https://github.com/ruby/bigdecimal/issues/70 - # TOOD: replace this with a check against BigDecimal::VERSION + # TODO: replace this with a check against BigDecimal::VERSION assert_not_predicate subject, :valid? else assert_predicate subject, :valid? diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 374edcdee0..5aa4196180 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2776,7 +2776,7 @@ module ApplicationTests assert_equal "action-text-attachment", ActionText::Attachment.tag_name end - test "action_text.config.attachment_tag_name is can be overriden" do + test "action_text.config.attachment_tag_name is can be overridden" do add_to_config "config.action_text.attachment_tag_name = 'link'" app "development" From 433f9707a1612f973d25cd4c6d5e880f93168aef Mon Sep 17 00:00:00 2001 From: Sunny Ripert Date: Sun, 7 Mar 2021 14:57:03 +0100 Subject: [PATCH 34/53] Add pointer cursor to Exception layout --- .../lib/action_dispatch/middleware/templates/rescues/layout.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index b3a4182ac7..974aa18c05 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -153,6 +153,7 @@ font-weight: bold; margin: 0; padding: 10px 18px; + cursor: pointer; -webkit-appearance: none; } input[type="submit"]:focus, From 7c5c8f9e48207771c04708fb991e45a5f4b03286 Mon Sep 17 00:00:00 2001 From: Tsukuru Tanimichi Date: Sun, 7 Mar 2021 22:57:08 +0900 Subject: [PATCH 35/53] [ci skip] Not `masked_authenticity_token` but `form_authenticity_token` should be a public API `form_authenticity_token` would be a public API because: 1. The usage of this method [is described in the guide](https://github.com/rails/rails/blob/291a3d2ef29a3842d1156ada7526f4ee60dd2b59/guides/source/action_controller_overview.md#request-forgery-protection) and already [many Rails users depend on this method](https://stackoverflow.com/questions/941594/understanding-the-rails-authenticity-token). 2. This method [is set as helper_method](https://github.com/rails/rails/blob/291a3d2ef29a3842d1156ada7526f4ee60dd2b59/actionpack/lib/action_controller/metal/request_forgery_protection.rb#L97) and called form ActionView. Inside the Rails components, it's already used as a public interface of ActionController. --- .../action_controller/metal/request_forgery_protection.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 464cf8309e..18e72f9ad5 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -303,15 +303,15 @@ module ActionController #:nodoc: [form_authenticity_param, request.x_csrf_token] end - # Sets the token value for the current session. - def form_authenticity_token(form_options: {}) + # Creates the authenticity token for the current request. + def form_authenticity_token(form_options: {}) # :doc: masked_authenticity_token(session, form_options: form_options) end # Creates a masked version of the authenticity token that varies # on each request. The masking is used to mitigate SSL attacks # like BREACH. - def masked_authenticity_token(session, form_options: {}) # :doc: + def masked_authenticity_token(session, form_options: {}) action, method = form_options.values_at(:action, :method) raw_token = if per_form_csrf_tokens && action && method From 1c97c5b2c2b685b29ba296e7ad046b34d19d0360 Mon Sep 17 00:00:00 2001 From: Henrik Nyh Date: Sun, 7 Mar 2021 16:38:42 +0000 Subject: [PATCH 36/53] Autoloading docs: Grammar/style tweaks --- guides/source/autoloading_and_reloading_constants.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index 04db24e629..bdfd39861d 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -89,7 +89,7 @@ INFO. Autoload paths are called _root directories_ in Zeitwerk documentation, bu Within an autoload path, file names must match the constants they define as documented [here](https://github.com/fxn/zeitwerk#file-structure). -By default, the autoload paths of an application consist of all the subdirectories of `app` that exist when the application boots ---except for `assets`, `javascript`, `views`,--- plus the autoload paths of engines it might depend on. +By default, the autoload paths of an application consist of all the subdirectories of `app` that exist when the application boots ---except for `assets`, `javascript`, and `views`--- plus the autoload paths of engines it might depend on. For example, if `UsersHelper` is implemented in `app/helpers/users_helper.rb`, the module is autoloadable, you do not need (and should not write) a `require` call for it: @@ -102,13 +102,13 @@ Autoload paths automatically pick up any custom directories under `app`. For exa The array of autoload paths can be extended by mutating `config.autoload_paths`, in `config/application.rb`, but nowadays this is discouraged. -WARNING. Please, do not mutate `ActiveSupport::Dependencies.autoload_paths`, the public interface to change autoload paths is `config.autoload_paths`. +WARNING. Please do not mutate `ActiveSupport::Dependencies.autoload_paths`; the public interface to change autoload paths is `config.autoload_paths`. $LOAD_PATH ---------- -Autoload paths are added to `$LOAD_PATH` by default. However, Zeitwerk uses absolute file names internally, and your application should not issue `require` calls for autoloadable files, so those directories are actually not needed there. You can opt-out with this flag: +Autoload paths are added to `$LOAD_PATH` by default. However, Zeitwerk uses absolute file names internally, and your application should not issue `require` calls for autoloadable files, so those directories are actually not needed there. You can opt out with this flag: ```ruby config.add_autoload_paths_to_load_path = false @@ -142,7 +142,7 @@ irb(main):003:0> User.object_id => 70136284426020 ``` -as you can see, the class object stored in the `User` constant is different after reloading. +As you can see, the class object stored in the `User` constant is different after reloading. ### Reloading and Stale Objects @@ -225,9 +225,9 @@ In production-like environments it is generally better to load all the applicati Eager loading is controlled by the flag `config.eager_load`, which is enabled by default in `production` mode. -The order in which files are eager loaded is undefined. +The order in which files are eager-loaded is undefined. -If the `Zeitwerk` constant is defined, Rails invokes `Zeitwerk::Loader.eager_load_all` regardless of the application autoloading mode. That ensures dependencies managed by Zeitwerk are eager loaded. +If the `Zeitwerk` constant is defined, Rails invokes `Zeitwerk::Loader.eager_load_all` regardless of the application autoloading mode. That ensures dependencies managed by Zeitwerk are eager-loaded. Single Table Inheritance From c48539ff9446a1124731358e832b33d6001e413c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 8 Mar 2021 02:49:33 +0900 Subject: [PATCH 37/53] Allow select statement node's initializer takes a table Like other statement nodes does, related to 333f9b32. --- activerecord/lib/arel/nodes/select_core.rb | 4 ++-- activerecord/lib/arel/nodes/select_statement.rb | 4 ++-- activerecord/lib/arel/select_manager.rb | 3 +-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/arel/nodes/select_core.rb b/activerecord/lib/arel/nodes/select_core.rb index 11b4f39ece..a9bc7986d0 100644 --- a/activerecord/lib/arel/nodes/select_core.rb +++ b/activerecord/lib/arel/nodes/select_core.rb @@ -6,9 +6,9 @@ module Arel # :nodoc: all attr_accessor :projections, :wheres, :groups, :windows, :comment attr_accessor :havings, :source, :set_quantifier, :optimizer_hints - def initialize + def initialize(relation = nil) super() - @source = JoinSource.new nil + @source = JoinSource.new(relation) # https://ronsavage.github.io/SQL/sql-92.bnf.html#set%20quantifier @set_quantifier = nil diff --git a/activerecord/lib/arel/nodes/select_statement.rb b/activerecord/lib/arel/nodes/select_statement.rb index eff5dad939..ad2162b541 100644 --- a/activerecord/lib/arel/nodes/select_statement.rb +++ b/activerecord/lib/arel/nodes/select_statement.rb @@ -6,9 +6,9 @@ module Arel # :nodoc: all attr_reader :cores attr_accessor :limit, :orders, :lock, :offset, :with - def initialize(cores = [SelectCore.new]) + def initialize(relation = nil) super() - @cores = cores + @cores = [SelectCore.new(relation)] @orders = [] @limit = nil @lock = nil diff --git a/activerecord/lib/arel/select_manager.rb b/activerecord/lib/arel/select_manager.rb index dd896cefb3..5f3e39a7f1 100644 --- a/activerecord/lib/arel/select_manager.rb +++ b/activerecord/lib/arel/select_manager.rb @@ -8,9 +8,8 @@ module Arel # :nodoc: all def initialize(table = nil) super() - @ast = Nodes::SelectStatement.new + @ast = Nodes::SelectStatement.new(table) @ctx = @ast.cores.last - from table end def initialize_copy(other) From 0f917da01a49f45c696bc0e32d14e4da811d8e6d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 5 Mar 2021 23:48:19 +0900 Subject: [PATCH 38/53] Remove unused accessors `left` and `right` on `DeleteStatement` Use `relation` and `wheres` instead. --- .../lib/arel/nodes/delete_statement.rb | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/activerecord/lib/arel/nodes/delete_statement.rb b/activerecord/lib/arel/nodes/delete_statement.rb index a419975335..0a1a3b0e94 100644 --- a/activerecord/lib/arel/nodes/delete_statement.rb +++ b/activerecord/lib/arel/nodes/delete_statement.rb @@ -3,17 +3,12 @@ module Arel # :nodoc: all module Nodes class DeleteStatement < Arel::Nodes::Node - attr_accessor :left, :right, :orders, :limit, :offset, :key - - alias :relation :left - alias :relation= :left= - alias :wheres :right - alias :wheres= :right= + attr_accessor :relation, :wheres, :orders, :limit, :offset, :key def initialize(relation = nil, wheres = []) super() - @left = relation - @right = wheres + @relation = relation + @wheres = wheres @orders = [] @limit = nil @offset = nil @@ -22,18 +17,18 @@ module Arel # :nodoc: all def initialize_copy(other) super - @left = @left.clone if @left - @right = @right.clone if @right + @relation = @relation.clone if @relation + @wheres = @wheres.clone if @wheres end def hash - [self.class, @left, @right, @orders, @limit, @offset, @key].hash + [self.class, @relation, @wheres, @orders, @limit, @offset, @key].hash end def eql?(other) self.class == other.class && - self.left == other.left && - self.right == other.right && + self.relation == other.relation && + self.wheres == other.wheres && self.orders == other.orders && self.limit == other.limit && self.offset == other.offset && From b881a646da09e1646edc3de0abe6a3ef36b06a99 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 5 Mar 2021 23:50:26 +0900 Subject: [PATCH 39/53] Use `records.compact!` over `records.compact` in `excluding` `"posts"."id" IS NOT NULL` is generated if `nil` is remained in `records`. --- activerecord/lib/active_record/relation/query_methods.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d0929d473c..b71a8aefac 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1130,8 +1130,9 @@ module ActiveRecord # scoping. def excluding(*records) records.flatten!(1) + records.compact! - if records.compact.any? { |record| !record.is_a?(klass) } + unless records.all?(klass) raise ArgumentError, "You must only pass a single or collection of #{klass.name} objects to #excluding." end From c431432f93a35530cd0544192b93dabda51f2bf7 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Sun, 7 Mar 2021 22:34:54 +0100 Subject: [PATCH 40/53] Add app/{helpers,models} to autoload_once_paths The helpers of Action Text are added to a couple of non-reloadable base classes: initializer "action_text.helper" do %i[action_controller_base action_mailer].each do |abstract_controller| ActiveSupport.on_load(abstract_controller) do helper ActionText::Engine.helpers end end end Therefore, it does not make sense that they are reloadable themselves. For the same price, we can also make the models non-reloadable, thus saving parent applications from the unnecessary work of reloading this engine. We did this for turbo-rails as well. --- actiontext/lib/action_text/engine.rb | 4 ++++ .../test/application/zeitwerk_integration_test.rb | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/actiontext/lib/action_text/engine.rb b/actiontext/lib/action_text/engine.rb index 46a2a23789..b17a48d372 100644 --- a/actiontext/lib/action_text/engine.rb +++ b/actiontext/lib/action_text/engine.rb @@ -14,6 +14,10 @@ module ActionText config.action_text = ActiveSupport::OrderedOptions.new config.action_text.attachment_tag_name = "action-text-attachment" + config.autoload_once_paths = %W( + #{root}/app/helpers + #{root}/app/models + ) initializer "action_text.attribute" do ActiveSupport.on_load(:active_record) do diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index 268a7467eb..6ceb2be010 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -295,10 +295,12 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert $zeitwerk_integration_test_extras end - test "autoload_paths are set as root dirs of main, and in the same order" do + test "autoload_paths not in autoload_once_paths are set as root dirs of main, and in the same order" do boot - existing_autoload_paths = deps.autoload_paths.select { |dir| File.directory?(dir) } + existing_autoload_paths = \ + deps.autoload_paths.select { |dir| File.directory?(dir) } - + deps.autoload_once_paths assert_equal existing_autoload_paths, Rails.autoloaders.main.dirs end @@ -315,7 +317,10 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase extras.each do |extra| assert_not_includes Rails.autoloaders.main.dirs, extra end - assert_equal extras, Rails.autoloaders.once.dirs + + e1_index = Rails.autoloaders.once.dirs.index(extras.first) + assert e1_index + assert_equal extras, Rails.autoloaders.once.dirs.slice(e1_index, extras.length) end test "clear reloads the main autoloader, and does not reload the once one" do From 776277080552e110df504fe36f6ecb32a8d165de Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Sun, 7 Mar 2021 16:45:37 -0600 Subject: [PATCH 41/53] Use stable branch for --edge option when possible This alleviates the need to update the code when there is a new stable branch (for example, as done in #41454). --- railties/lib/rails/generators/app_base.rb | 3 ++- railties/test/generators/app_generator_test.rb | 16 ++++++++++++++-- .../test/generators/plugin_generator_test.rb | 16 ++++++++++++++-- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 7cbe3ae99e..6d2de46a95 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -276,8 +276,9 @@ module Rails GemfileEntry.path("rails", Rails::Generators::RAILS_DEV_PATH) ] elsif options.edge? + edge_branch = Rails.gem_version.prerelease? ? "main" : [*Rails.gem_version.segments.first(2), "stable"].join("-") [ - GemfileEntry.github("rails", "rails/rails", "main") + GemfileEntry.github("rails", "rails/rails", edge_branch) ] elsif options.main? [ diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 1317e76b51..cff9972943 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -863,8 +863,20 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_edge_option - generator([destination_root], edge: true, skip_webpack_install: true) - run_generator_instance + Rails.stub(:gem_version, Gem::Version.new("2.1.0")) do + generator([destination_root], edge: true, skip_webpack_install: true) + run_generator_instance + end + + assert_equal 1, @bundle_commands.count("install") + assert_file "Gemfile", %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["'],\s+branch:\s+["']2-1-stable["']$} + end + + def test_edge_option_during_alpha + Rails.stub(:gem_version, Gem::Version.new("2.1.0.alpha")) do + generator([destination_root], edge: true, skip_webpack_install: true) + run_generator_instance + end assert_equal 1, @bundle_commands.count("install") assert_file "Gemfile", %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["'],\s+branch:\s+["']main["']$} diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 49975403ee..97c9390907 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -250,8 +250,20 @@ class PluginGeneratorTest < Rails::Generators::TestCase end def test_edge_option - generator([destination_root], edge: true) - run_generator_instance + Rails.stub(:gem_version, Gem::Version.new("2.1.0")) do + generator([destination_root], edge: true) + run_generator_instance + end + + assert_empty @bundle_commands + assert_file "Gemfile", %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["'],\s+branch:\s+["']2-1-stable["']$} + end + + def test_edge_option_during_alpha + Rails.stub(:gem_version, Gem::Version.new("2.1.0.alpha")) do + generator([destination_root], edge: true) + run_generator_instance + end assert_empty @bundle_commands assert_file "Gemfile", %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["'],\s+branch:\s+["']main["']$} From 0d523d83657ce7066f25d87f6f094e804590e1e8 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 1 Mar 2021 22:28:35 +0100 Subject: [PATCH 42/53] Drops support for classic mode This starts a series of patches in which we drop classic mode. The final result no longer has a const_missing callback, there is no hook/unhook, and so on. So, in this patch we remove the ability of configuring classic, but some of the code that remains will be further refactored. --- .../dependencies/zeitwerk_integration.rb | 1 + .../autoloading_and_reloading_constants.md | 22 ------------------- guides/source/configuring.md | 7 ------ .../lib/rails/application/configuration.rb | 19 +--------------- railties/lib/rails/application/finisher.rb | 22 ++++++------------- railties/lib/rails/autoloaders.rb | 3 ++- .../test/application/configuration_test.rb | 19 ---------------- .../application/zeitwerk_integration_test.rb | 13 +---------- 8 files changed, 12 insertions(+), 94 deletions(-) diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index 2155fba0a9..f7aec0ffb3 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -2,6 +2,7 @@ require "set" require "active_support/core_ext/string/inflections" +require "zeitwerk" module ActiveSupport module Dependencies diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index bdfd39861d..04e5829749 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -438,25 +438,3 @@ While in common names these operations match, if acronyms or custom inflection r ### More Differences There are some other subtle differences. Please check the [autoloading section](upgrading_ruby_on_rails.html#autoloading) of the _Upgrading Ruby on Rails_] guide for details. - -Classic Mode is Deprecated --------------------------- - -Currently, it is still possible to use `classic` mode. However, `classic` is deprecated and will be eventually removed. - -New applications should use `zeitwerk` mode (which is the default), and applications being upgraded are strongly encouraged to migrate to `zeitwerk` mode. Please check the [_Upgrading Ruby on Rails_](upgrading_ruby_on_rails.html#autoloading) guide for details. - -Opting Out ----------- - -Applications can load Rails 6 defaults and still use the `classic` autoloader this way: - -```ruby -# config/application.rb -config.load_defaults 6.0 -config.autoloader = :classic -``` - -That may be handy if upgrading to Rails 6 in different phases, but `classic` mode is discouraged for new applications. - -`zeitwerk` mode is not available in versions of Rails prior to 6.0. diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 6ffe699532..842dbe57a1 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -159,13 +159,6 @@ numbers. It also filters out sensitive values of database columns when call `#in * `config.time_zone` sets the default time zone for the application and enables time zone awareness for Active Record. -* `config.autoloader` sets the autoloading mode. This option defaults to `:zeitwerk` when `config.load_defaults` is called with `6.0` or greater. Applications can still use the classic autoloader by setting this value to `:classic` after loading the framework defaults: - - ```ruby - config.load_defaults 6.0 - config.autoloader = :classic - ``` - ### Configuring Assets * `config.assets.enabled` a flag that controls whether the asset diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 32e1776bb4..5c97f4ec73 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -23,7 +23,7 @@ module Rails :require_master_key, :credentials, :disable_sandbox, :add_autoload_paths_to_load_path, :rake_eager_load - attr_reader :encoding, :api_only, :loaded_config_version, :autoloader + attr_reader :encoding, :api_only, :loaded_config_version def initialize(*) super @@ -69,7 +69,6 @@ module Rails @credentials = ActiveSupport::OrderedOptions.new @credentials.content_path = default_credentials_content_path @credentials.key_path = default_credentials_key_path - @autoloader = :classic @disable_sandbox = false @add_autoload_paths_to_load_path = true @permissions_policy = nil @@ -128,8 +127,6 @@ module Rails when "6.0" load_defaults "5.2" - self.autoloader = :zeitwerk if RUBY_ENGINE == "ruby" - if respond_to?(:action_view) action_view.default_enforce_utf8 = false end @@ -155,8 +152,6 @@ module Rails when "6.1" load_defaults "6.0" - self.autoloader = :zeitwerk if %w[ruby truffleruby].include?(RUBY_ENGINE) - if respond_to?(:active_record) active_record.has_many_inversing = true active_record.legacy_connection_handling = false @@ -365,18 +360,6 @@ module Rails end end - def autoloader=(autoloader) - case autoloader - when :classic - @autoloader = autoloader - when :zeitwerk - require "zeitwerk" - @autoloader = autoloader - else - raise ArgumentError, "config.autoloader may be :classic or :zeitwerk, got #{autoloader.inspect} instead" - end - end - def default_log_file path = paths["log"].first unless File.exist? File.dirname path diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index e4efebb615..900eb8c6e0 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -2,6 +2,7 @@ require "active_support/core_ext/string/inflections" require "active_support/core_ext/array/conversions" +require "zeitwerk" module Rails class Application @@ -39,14 +40,10 @@ module Rails example = autoloaded.first example_klass = example.constantize.class - if config.autoloader == :zeitwerk - ActiveSupport::DescendantsTracker.clear - ActiveSupport::Dependencies.clear + ActiveSupport::DescendantsTracker.clear + ActiveSupport::Dependencies.clear - unload_message = "#{these} autoloaded #{constants} #{have} been unloaded." - else - unload_message = "`config.autoloader` is set to `#{config.autoloader}`. #{these} autoloaded #{constants} would have been unloaded if `config.autoloader` had been set to `:zeitwerk`." - end + unload_message = "#{these} autoloaded #{constants} #{have} been unloaded." ActiveSupport::Deprecation.warn(<<~WARNING) Initialization autoloaded the #{constants} #{enum}. @@ -76,10 +73,8 @@ module Rails end initializer :let_zeitwerk_take_over do - if config.autoloader == :zeitwerk - require "active_support/dependencies/zeitwerk_integration" - ActiveSupport::Dependencies::ZeitwerkIntegration.take_over(enable_reloading: !config.cache_classes) - end + require "active_support/dependencies/zeitwerk_integration" + ActiveSupport::Dependencies::ZeitwerkIntegration.take_over(enable_reloading: !config.cache_classes) end # Setup default session store if not already set in config/application.rb @@ -113,10 +108,7 @@ module Rails initializer :eager_load! do if config.eager_load ActiveSupport.run_load_hooks(:before_eager_load, self) - # Checks defined?(Zeitwerk) instead of zeitwerk_enabled? because we - # want to eager load any dependency managed by Zeitwerk regardless of - # the autoloading mode of the application. - Zeitwerk::Loader.eager_load_all if defined?(Zeitwerk) + Zeitwerk::Loader.eager_load_all config.eager_load_namespaces.each(&:eager_load!) end end diff --git a/railties/lib/rails/autoloaders.rb b/railties/lib/rails/autoloaders.rb index ce3ba269fe..773d2e58e0 100644 --- a/railties/lib/rails/autoloaders.rb +++ b/railties/lib/rails/autoloaders.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/dependencies/zeitwerk_integration" +require "zeitwerk" module Rails module Autoloaders # :nodoc: @@ -41,7 +42,7 @@ module Rails end def zeitwerk_enabled? - Rails.configuration.autoloader == :zeitwerk + true end end end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 5aa4196180..473c671df3 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1219,7 +1219,6 @@ module ApplicationTests test "autoloaders" do app "development" - config = Rails.application.config assert Rails.autoloaders.zeitwerk_enabled? assert_instance_of Zeitwerk::Loader, Rails.autoloaders.main assert_equal "rails.main", Rails.autoloaders.main.tag @@ -1228,24 +1227,6 @@ module ApplicationTests assert_equal [Rails.autoloaders.main, Rails.autoloaders.once], Rails.autoloaders.to_a assert_equal ActiveSupport::Dependencies::ZeitwerkIntegration::Inflector, Rails.autoloaders.main.inflector assert_equal ActiveSupport::Dependencies::ZeitwerkIntegration::Inflector, Rails.autoloaders.once.inflector - - config.autoloader = :classic - assert_not Rails.autoloaders.zeitwerk_enabled? - assert_nil Rails.autoloaders.main - assert_nil Rails.autoloaders.once - assert_equal 0, Rails.autoloaders.count - - config.autoloader = :zeitwerk - assert Rails.autoloaders.zeitwerk_enabled? - assert_instance_of Zeitwerk::Loader, Rails.autoloaders.main - assert_equal "rails.main", Rails.autoloaders.main.tag - assert_instance_of Zeitwerk::Loader, Rails.autoloaders.once - assert_equal "rails.once", Rails.autoloaders.once.tag - assert_equal [Rails.autoloaders.main, Rails.autoloaders.once], Rails.autoloaders.to_a - assert_equal ActiveSupport::Dependencies::ZeitwerkIntegration::Inflector, Rails.autoloaders.main.inflector - assert_equal ActiveSupport::Dependencies::ZeitwerkIntegration::Inflector, Rails.autoloaders.once.inflector - - assert_raises(ArgumentError) { config.autoloader = :unknown } end test "config.action_view.cache_template_loading with cache_classes default" do diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index 6ceb2be010..6bef0be0da 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -26,7 +26,7 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase deps.singleton_class < deps::ZeitwerkIntegration::Decorations end - test "ActiveSupport::Dependencies is decorated by default" do + test "ActiveSupport::Dependencies is decorated" do boot assert decorated? @@ -36,17 +36,6 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert_equal [Rails.autoloaders.main, Rails.autoloaders.once], Rails.autoloaders.to_a end - test "ActiveSupport::Dependencies is not decorated in classic mode" do - add_to_config "config.autoloader = :classic" - boot - - assert_not decorated? - assert_not Rails.autoloaders.zeitwerk_enabled? - assert_nil Rails.autoloaders.main - assert_nil Rails.autoloaders.once - assert_equal 0, Rails.autoloaders.count - end - test "autoloaders inflect with Active Support" do app_file "config/initializers/inflections.rb", <<-RUBY ActiveSupport::Inflector.inflections(:en) do |inflect| From 14d4edd7c3b06e82e1fcef54fa0b4453315c35fd Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 8 Mar 2021 07:43:12 +0100 Subject: [PATCH 43/53] Deletes AS::Dependencies::Reference This is an internal class which is no longer needed. --- activesupport/lib/active_support/dependencies.rb | 12 ++---------- activesupport/test/dependencies_test.rb | 15 --------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 552ba7ee33..95a1747cc4 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -614,7 +614,6 @@ module ActiveSupport #:nodoc: log("removing unloadable constants") autoloaded_constants.each { |const| remove_constant const } autoloaded_constants.clear - Reference.clear! explicitly_unloadable_constants.each { |const| remove_constant const } end @@ -654,23 +653,16 @@ module ActiveSupport #:nodoc: end end - Reference = ClassCache.new - - # Store a reference to a class +klass+. - def reference(klass) - Reference.store klass - end - # Get the reference for class named +name+. # Raises an exception if referenced class does not exist. def constantize(name) - Reference.get(name) + Inflector.constantize(name) end # Get the reference for class named +name+ if one exists. # Otherwise returns +nil+. def safe_constantize(name) - Reference.safe_get(name) + Inflector.safe_constantize(name) end # Determine if the given constant has been automatically loaded. diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 6801d951e8..b459269903 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -708,21 +708,6 @@ class DependenciesTest < ActiveSupport::TestCase remove_constants(:ServiceOne) end - def test_references_should_work - with_loading "dependencies" do - c = ActiveSupport::Dependencies.reference("ServiceOne") - service_one_first = ServiceOne - assert_equal service_one_first, c.get("ServiceOne") - ActiveSupport::Dependencies.clear - assert_not defined?(ServiceOne) - service_one_second = ServiceOne - assert_not_equal service_one_first, c.get("ServiceOne") - assert_equal service_one_second, c.get("ServiceOne") - end - ensure - remove_constants(:ServiceOne) - end - def test_constantize_shortcut_for_cached_constant_lookups with_loading "dependencies" do assert_equal ServiceOne, ActiveSupport::Dependencies.constantize("ServiceOne") From 3c90308b175de54cd120f65b113ee774aec203b2 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 8 Mar 2021 07:36:44 +0100 Subject: [PATCH 44/53] Deletes AS::Dependencies::ClassCache This is an internal class which is no longer needed. --- .../lib/active_support/dependencies.rb | 36 --------- activesupport/test/class_cache_test.rb | 80 ------------------- 2 files changed, 116 deletions(-) delete mode 100644 activesupport/test/class_cache_test.rb diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 95a1747cc4..17dc8c22db 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -617,42 +617,6 @@ module ActiveSupport #:nodoc: explicitly_unloadable_constants.each { |const| remove_constant const } end - class ClassCache - def initialize - @store = Concurrent::Map.new - end - - def empty? - @store.empty? - end - - def key?(key) - @store.key?(key) - end - - def get(key) - key = key.name if key.respond_to?(:name) - @store[key] ||= Inflector.constantize(key) - end - alias :[] :get - - def safe_get(key) - key = key.name if key.respond_to?(:name) - @store[key] ||= Inflector.safe_constantize(key) - end - - def store(klass) - return self unless klass.respond_to?(:name) - raise(ArgumentError, "anonymous classes cannot be cached") if klass.name.empty? - @store[klass.name] = klass - self - end - - def clear! - @store.clear - end - end - # Get the reference for class named +name+. # Raises an exception if referenced class does not exist. def constantize(name) diff --git a/activesupport/test/class_cache_test.rb b/activesupport/test/class_cache_test.rb deleted file mode 100644 index 4c517d00b1..0000000000 --- a/activesupport/test/class_cache_test.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -require_relative "abstract_unit" -require "active_support/dependencies" - -module ActiveSupport - module Dependencies - class ClassCacheTest < ActiveSupport::TestCase - def setup - @cache = ClassCache.new - end - - def test_empty? - assert_empty @cache - @cache.store(ClassCacheTest) - assert_not_empty @cache - end - - def test_clear! - assert_empty @cache - @cache.store(ClassCacheTest) - assert_not_empty @cache - @cache.clear! - assert_empty @cache - end - - def test_set_key - @cache.store(ClassCacheTest) - assert @cache.key?(ClassCacheTest.name) - end - - def test_get_with_class - @cache.store(ClassCacheTest) - assert_equal ClassCacheTest, @cache.get(ClassCacheTest) - end - - def test_get_with_name - @cache.store(ClassCacheTest) - assert_equal ClassCacheTest, @cache.get(ClassCacheTest.name) - end - - def test_get_constantizes - assert_empty @cache - assert_equal ClassCacheTest, @cache.get(ClassCacheTest.name) - end - - def test_get_constantizes_fails_on_invalid_names - assert_empty @cache - assert_raise NameError do - @cache.get("OmgTotallyInvalidConstantName") - end - end - - def test_get_alias - assert_empty @cache - assert_equal @cache[ClassCacheTest.name], @cache.get(ClassCacheTest.name) - end - - def test_safe_get_constantizes - assert_empty @cache - assert_equal ClassCacheTest, @cache.safe_get(ClassCacheTest.name) - end - - def test_safe_get_constantizes_doesnt_fail_on_invalid_names - assert_empty @cache - assert_nil @cache.safe_get("OmgTotallyInvalidConstantName") - end - - def test_new_rejects_strings - @cache.store ClassCacheTest.name - assert_not @cache.key?(ClassCacheTest.name) - end - - def test_store_returns_self - x = @cache.store ClassCacheTest - assert_equal @cache, x - end - end - end -end From 43a7f68ae322328d0744c873ecdc2ccb65b33508 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 8 Mar 2021 10:03:33 +0100 Subject: [PATCH 45/53] Deletes AS::Dependencies::Blamable This is an internal class which is no longer needed. --- .../rescues/_request_and_response.html.erb | 7 ----- .../middleware/templates/rescues/layout.erb | 3 --- .../lib/active_support/dependencies.rb | 26 ------------------- .../raises_exception_without_blame_file.rb | 7 ----- activesupport/test/dependencies_test.rb | 7 ----- 5 files changed, 50 deletions(-) delete mode 100644 activesupport/test/dependencies/raises_exception_without_blame_file.rb diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb index 04271d8e8a..fedeafc3e7 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb @@ -1,10 +1,3 @@ -<% unless @exception.blamed_files.blank? %> - <% if (hide = @exception.blamed_files.length > 8) %> - Toggle blamed files - <% end %> - 
><%= @exception.describe_blame %>
-<% end %> -

Request

<% if params_valid? %>

Parameters:

<%= debug_params(@request.filtered_parameters) %>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 974aa18c05..f49bee3437 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -239,9 +239,6 @@ var hide = function(id) { document.getElementById(id).style.display = 'none'; } - var toggleTrace = function() { - return toggle('blame_trace'); - } var toggleSessionDump = function() { return toggle('session_dump'); } diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 17dc8c22db..9c4c7cd6f1 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -298,9 +298,6 @@ module ActiveSupport #:nodoc: else yield end - rescue Exception => exception # errors from loading file - exception.blame_file! file if exception.respond_to? :blame_file! - raise end # Mark the given constant as unloadable. Unloadable constants are removed @@ -334,31 +331,9 @@ module ActiveSupport #:nodoc: end end - # Exception file-blaming. - module Blamable #:nodoc: - def blame_file!(file) - (@blamed_files ||= []).unshift file - end - - def blamed_files - @blamed_files ||= [] - end - - def describe_blame - return nil if blamed_files.empty? - "This error occurred while loading the following files:\n #{blamed_files.join "\n "}" - end - - def copy_blame!(exc) - @blamed_files = exc.blamed_files.clone - self - end - end - def hook! Loadable.include_into(Object) ModuleConstMissing.include_into(Module) - Exception.include(Blamable) end def unhook! @@ -381,7 +356,6 @@ module ActiveSupport #:nodoc: load_error.message end load_error_message.replace(message % file_name) - load_error.copy_blame!(load_error) end raise end diff --git a/activesupport/test/dependencies/raises_exception_without_blame_file.rb b/activesupport/test/dependencies/raises_exception_without_blame_file.rb deleted file mode 100644 index 3a6533cd3a..0000000000 --- a/activesupport/test/dependencies/raises_exception_without_blame_file.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -exception = Exception.new("I am not blamable!") -class << exception - undef_method(:blame_file!) -end -raise exception diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index b459269903..0090066151 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -104,13 +104,6 @@ class DependenciesTest < ActiveSupport::TestCase end end - def test_dependency_which_raises_doesnt_blindly_call_blame_file! - with_loading do - filename = "dependencies/raises_exception_without_blame_file" - assert_raises(Exception) { require_dependency filename } - end - end - def test_warnings_should_be_enabled_on_first_load with_loading "dependencies" do old_warnings, ActiveSupport::Dependencies.warnings_on_first_load = ActiveSupport::Dependencies.warnings_on_first_load, true From 08f3dfeaee84dd14399427abb1e44d8a391893ed Mon Sep 17 00:00:00 2001 From: Alex Ghiculescu Date: Mon, 8 Mar 2021 10:52:04 -0600 Subject: [PATCH 46/53] Add docs for `insert_all` with scopes Adds docs for https://github.com/rails/rails/pull/38899 cc @kamipo --- activerecord/lib/active_record/persistence.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 9968580be8..9406b84274 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -120,6 +120,14 @@ module ActiveRecord # { id: 1, title: "Rework", author: "David" }, # { id: 1, title: "Eloquent Ruby", author: "Russ" } # ]) + # + # # insert_all works on chained scopes, and you can use create_with + # # to set default attributes for all inserted records. + # + # author.books.create_with(created_at: Time.now).insert_all([ + # { id: 1, title: "Rework" }, + # { id: 2, title: "Eloquent Ruby" } + # ]) def insert_all(attributes, returning: nil, unique_by: nil) InsertAll.new(self, attributes, on_duplicate: :skip, returning: returning, unique_by: unique_by).execute end From 16e45de59e0980aa82abdf6bae6131dee276cfe7 Mon Sep 17 00:00:00 2001 From: Bradley Priest Date: Sat, 6 Mar 2021 13:41:32 +1300 Subject: [PATCH 47/53] Quote the arguments passed to the new Contains/Overlaps node types to align with the behaviour of existing predicates --- activerecord/lib/arel/predications.rb | 4 +-- .../cases/arel/attributes/attribute_test.rb | 29 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/activerecord/lib/arel/predications.rb b/activerecord/lib/arel/predications.rb index 0d2d1acc93..cdf10d6549 100644 --- a/activerecord/lib/arel/predications.rb +++ b/activerecord/lib/arel/predications.rb @@ -207,11 +207,11 @@ module Arel # :nodoc: all end def contains(other) - Arel::Nodes::Contains.new(self, other) + Arel::Nodes::Contains.new self, quoted_node(other) end def overlaps(other) - Arel::Nodes::Overlaps.new(self, other) + Arel::Nodes::Overlaps.new self, quoted_node(other) end def quoted_array(others) diff --git a/activerecord/test/cases/arel/attributes/attribute_test.rb b/activerecord/test/cases/arel/attributes/attribute_test.rb index cb3dd47c84..456fca2749 100644 --- a/activerecord/test/cases/arel/attributes/attribute_test.rb +++ b/activerecord/test/cases/arel/attributes/attribute_test.rb @@ -1038,15 +1038,13 @@ module Arel describe "#contains" do it "should create a Contains node" do relation = Table.new(:products) - query = Nodes.build_quoted("{foo,bar}") - _(relation[:tags].contains(query)).must_be_kind_of Nodes::Contains + _(relation[:tags].contains(["foo", "bar"])).must_be_kind_of Nodes::Contains end it "should generate @> in sql" do - relation = Table.new(:products) + relation = Table.new(:products, type_caster: fake_pg_caster) mgr = relation.project relation[:id] - query = Nodes.build_quoted("{foo,bar}") - mgr.where relation[:tags].contains(query) + mgr.where relation[:tags].contains(["foo", "bar"]) _(mgr.to_sql).must_be_like %{ SELECT "products"."id" FROM "products" WHERE "products"."tags" @> '{foo,bar}' } end end @@ -1054,15 +1052,13 @@ module Arel describe "#overlaps" do it "should create an Overlaps node" do relation = Table.new(:products) - query = Nodes.build_quoted("{foo,bar}") - _(relation[:tags].overlaps(query)).must_be_kind_of Nodes::Overlaps + _(relation[:tags].overlaps(["foo", "bar"])).must_be_kind_of Nodes::Overlaps end it "should generate && in sql" do - relation = Table.new(:products) + relation = Table.new(:products, type_caster: fake_pg_caster) mgr = relation.project relation[:id] - query = Nodes.build_quoted("{foo,bar}") - mgr.where relation[:tags].overlaps(query) + mgr.where relation[:tags].overlaps(["foo", "bar"]) _(mgr.to_sql).must_be_like %{ SELECT "products"."id" FROM "products" WHERE "products"."tags" && '{foo,bar}' } end end @@ -1123,6 +1119,19 @@ module Arel exclude_end?: exclude, ) end + + # Mimic PG::TextDecoder::Array casting + def fake_pg_caster + Object.new.tap do |caster| + def caster.type_cast_for_database(attr_name, value) + if attr_name == "tags" + "{#{value.join(",")}}" + else + value + end + end + end + end end end end From ac6c4bde86e8a95dfa0e95cc6422859e0b514bf6 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 8 Mar 2021 06:33:31 +0900 Subject: [PATCH 48/53] Fix merging select values from a different model's relation Unlike order values, where/having clause, etc, select values lazily evaluates the attribute name resolution until building arel. If select values are merged from a different model's relation, those may be incorrectly resolved as a column in a wrong table. In that case, the attribute name resolution should be done before merging those. --- .../lib/active_record/relation/merger.rb | 33 +++++++++++-------- .../test/cases/relation/select_test.rb | 14 ++++++++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index e436373012..42bbeb1938 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -51,30 +51,25 @@ module ActiveRecord @rewhere = rewhere end - NORMAL_VALUES = Relation::VALUE_METHODS - - Relation::CLAUSE_METHODS - - [:includes, :preload, :joins, :left_outer_joins, :order, :reverse_order, :lock, :create_with, :reordering] # :nodoc: - - def normal_values - NORMAL_VALUES - end + NORMAL_VALUES = Relation::VALUE_METHODS - Relation::CLAUSE_METHODS - + [ + :select, :includes, :preload, :joins, :left_outer_joins, + :order, :reverse_order, :lock, :create_with, :reordering + ] def merge - normal_values.each do |name| + NORMAL_VALUES.each do |name| value = values[name] # The unless clause is here mostly for performance reasons (since the `send` call might be moderately # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values # don't fall through the cracks. unless value.nil? || (value.blank? && false != value) - if name == :select - relation._select!(*value) - else - relation.public_send("#{name}!", *value) - end + relation.public_send(:"#{name}!", *value) end end + merge_select_values merge_multi_values merge_single_values merge_clauses @@ -86,6 +81,18 @@ module ActiveRecord end private + def merge_select_values + return if other.select_values.empty? + + if other.klass == relation.klass + relation.select_values |= other.select_values + else + relation.select_values |= other.instance_eval do + arel_columns(select_values) + end + end + end + def merge_preloads return if other.preload_values.empty? && other.includes_values.empty? diff --git a/activerecord/test/cases/relation/select_test.rb b/activerecord/test/cases/relation/select_test.rb index 7c98df8424..7a7ee9299a 100644 --- a/activerecord/test/cases/relation/select_test.rb +++ b/activerecord/test/cases/relation/select_test.rb @@ -41,6 +41,20 @@ module ActiveRecord end private :assert_non_select_columns_wont_be_loaded + def test_merging_select_from_different_model + posts = Post.select(:id, :title).joins(:comments) + comments = Comment.where(body: "Thank you for the welcome") + + [ + posts.merge(comments.select(:body)).first, + posts.merge(comments.select("comments.body")).first, + ].each do |post| + assert_equal 1, post.id + assert_equal "Welcome to the weblog", post.title + assert_equal "Thank you for the welcome", post.body + end + end + def test_type_casted_extra_select_with_eager_loading posts = Post.select("posts.id * 1.1 AS foo").eager_load(:comments) assert_equal 1.1, posts.first.foo From dc96825d44aa16754cfc4ae46b49a68c0f22e379 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 9 Mar 2021 08:45:35 +0900 Subject: [PATCH 49/53] Fix `number_to_human_size`'s result [ci skip] --- guides/source/action_view_helpers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guides/source/action_view_helpers.md b/guides/source/action_view_helpers.md index 304ca6d02b..332b36ebcc 100644 --- a/guides/source/action_view_helpers.md +++ b/guides/source/action_view_helpers.md @@ -347,8 +347,8 @@ number_to_human(1234567) # => 1.23 Million Formats the bytes in size into a more understandable representation; useful for reporting file sizes to users. ```ruby -number_to_human_size(1234) # => 1.2 KB -number_to_human_size(1234567) # => 1.2 MB +number_to_human_size(1234) # => 1.21 KB +number_to_human_size(1234567) # => 1.18 MB ``` #### number_to_percentage From 6efe2b490083c90550786ab70957e8dc548baf23 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 9 Mar 2021 09:19:35 +0900 Subject: [PATCH 50/53] Use `type.serializable?` in `unboundable?` --- .../lib/active_record/relation/query_attribute.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index cd18f27330..fdd69a806e 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -31,14 +31,10 @@ module ActiveRecord end def unboundable? - if defined?(@_unboundable) - @_unboundable - else - value_for_database unless value_before_type_cast.is_a?(StatementCache::Substitute) - @_unboundable = nil + unless defined?(@_unboundable) + @_unboundable = !type.serializable?(value) && type.cast(value) <=> 0 end - rescue ::RangeError - @_unboundable = type.cast(value_before_type_cast) <=> 0 + @_unboundable end private From bf57e7ff9d7cc4b12bec4a62f66ada8f08e46c2b Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Fri, 26 Feb 2021 19:55:38 -0500 Subject: [PATCH 51/53] Add app concern and test keepfiles to generated engine plugins --- railties/CHANGELOG.md | 4 ++ .../rails/plugin/plugin_generator.rb | 22 ++++++-- .../test/generators/plugin_generator_test.rb | 51 +++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index c388528a04..3408052fb9 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add app concern and test keepfiles to generated engine plugins. + + *Gannon McGibbon* + * Stop generating a license for in-app plugins. *Gannon McGibbon* diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index b8c0e62db4..acdd5c6f28 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -26,11 +26,15 @@ module Rails empty_directory_with_keep_file "app/assets/images/#{namespaced_name}" end + empty_directory_with_keep_file "app/models/concerns" + empty_directory_with_keep_file "app/controllers/concerns" remove_dir "app/mailers" if options[:skip_action_mailer] remove_dir "app/jobs" if options[:skip_active_job] elsif full? empty_directory_with_keep_file "app/models" empty_directory_with_keep_file "app/controllers" + empty_directory_with_keep_file "app/models/concerns" + empty_directory_with_keep_file "app/controllers/concerns" empty_directory_with_keep_file "app/mailers" unless options[:skip_action_mailer] empty_directory_with_keep_file "app/jobs" unless options[:skip_active_job] @@ -90,12 +94,22 @@ module Rails def test template "test/test_helper.rb" template "test/%namespaced_name%_test.rb" - append_file "Rakefile", <<-EOF - -#{rakefile_test_tasks} -task default: :test + append_file "Rakefile", <<~EOF + #{rakefile_test_tasks} + task default: :test EOF + if engine? + empty_directory_with_keep_file "test/fixtures/files" + empty_directory_with_keep_file "test/controllers" + empty_directory_with_keep_file "test/mailers" + empty_directory_with_keep_file "test/models" + empty_directory_with_keep_file "test/integration" + + unless api? + empty_directory_with_keep_file "test/helpers" + end + template "test/integration/navigation_test.rb" end end diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 97c9390907..5a10cf1e32 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -481,6 +481,57 @@ class PluginGeneratorTest < Rails::Generators::TestCase end end + def test_creating_full_engine_mode_adds_keeps + run_generator [destination_root, "--full"] + folders_with_keep = %w( + app/models/concerns + app/controllers/concerns + test/fixtures/files + test/controllers + test/mailers + test/models + test/helpers + test/integration + ) + folders_with_keep.each do |folder| + assert_file("#{folder}/.keep") + end + end + + def test_creating_full_api_engine_adds_keeps + run_generator [destination_root, "--full", "--api"] + folders_with_keep = %w( + app/models/concerns + app/controllers/concerns + test/fixtures/files + test/controllers + test/mailers + test/models + test/integration + ) + folders_with_keep.each do |folder| + assert_file("#{folder}/.keep") + end + assert_no_file("test/helpers/.keep") + end + + def test_creating_mountable_engine_mode_adds_keeps + run_generator [destination_root, "--mountable"] + folders_with_keep = %w( + app/models/concerns + app/controllers/concerns + test/fixtures/files + test/controllers + test/mailers + test/models + test/helpers + test/integration + ) + folders_with_keep.each do |folder| + assert_file("#{folder}/.keep") + end + end + def test_creating_gemspec run_generator assert_file "bukkits.gemspec", /spec\.name\s+= "bukkits"/ From 7f3ea2181e454ede12a6207816ff21d24b33a2fe Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 9 Mar 2021 16:28:47 +0900 Subject: [PATCH 52/53] Delegate `serializable?` to `subtype` on Enum Since #41516, values on Enum are not always serializable. --- activerecord/lib/active_record/enum.rb | 6 ++---- activerecord/test/cases/enum_test.rb | 4 ++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 1c5743e571..6ba12c32a2 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -115,7 +115,7 @@ module ActiveRecord end class EnumType < Type::Value # :nodoc: - delegate :type, to: :subtype + delegate :type, :serializable?, to: :subtype def initialize(name, mapping, subtype) @name = name @@ -128,10 +128,8 @@ module ActiveRecord value.to_s elsif mapping.has_value?(value) mapping.key(value) - elsif value.blank? - nil else - assert_valid_value(value) + value.presence end end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 140712a657..dcdfe2dd89 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -94,6 +94,10 @@ class EnumTest < ActiveRecord::TestCase assert_nil Book.where(status: "prohibited").first end + test "find via where with large number" do + assert_equal @book, Book.where(status: [2, 9223372036854775808]).first + end + test "find via where should be type casted" do book = Book.enabled.create! assert_predicate book, :enabled? From dc7817cb42768ad01d69d5f9b0023f556760cb06 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 9 Mar 2021 10:02:03 +0100 Subject: [PATCH 53/53] Deletes logging from AS::Dependencies --- .../lib/active_support/dependencies.rb | 13 ----- activesupport/test/dependencies_test.rb | 49 ------------------- .../application/zeitwerk_integration_test.rb | 36 -------------- 3 files changed, 98 deletions(-) diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 9c4c7cd6f1..de124a3c81 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -86,12 +86,6 @@ module ActiveSupport #:nodoc: # to allow arbitrary constants to be marked for unloading. mattr_accessor :explicitly_unloadable_constants, default: [] - # The logger used when tracing autoloads. - mattr_accessor :logger - - # If true, trace autoloads with +logger.debug+. - mattr_accessor :verbose, default: false - # The WatchStack keeps a stack of the modules being watched as files are # loaded. If a file in the process of being loaded (parent.rb) triggers the # load of another file (child.rb) the stack will ensure that child.rb @@ -474,7 +468,6 @@ module ActiveSupport #:nodoc: return nil unless base_path = autoloadable_module?(path_suffix) mod = Module.new into.const_set const_name, mod - log("constant #{qualified_name} autoloaded (module autovivified from #{File.join(base_path, path_suffix)})") autoloaded_constants << qualified_name unless autoload_once_paths.include?(base_path) autoloaded_constants.uniq! mod @@ -532,7 +525,6 @@ module ActiveSupport #:nodoc: require_or_load(expanded, qualified_name) if from_mod.const_defined?(const_name, false) - log("constant #{qualified_name} autoloaded from #{expanded}.rb") return from_mod.const_get(const_name) else raise LoadError, "Unable to autoload constant #{qualified_name}, expected #{file_path} to define it" @@ -585,7 +577,6 @@ module ActiveSupport #:nodoc: # as the environment will be in an inconsistent state, e.g. other constants # may have already been unloaded and not accessible. def remove_unloadable_constants! - log("removing unloadable constants") autoloaded_constants.each { |const| remove_constant const } autoloaded_constants.clear explicitly_unloadable_constants.each { |const| remove_constant const } @@ -732,10 +723,6 @@ module ActiveSupport #:nodoc: end end - def log(message) - logger.debug("autoloading: #{message}") if logger && verbose - end - private def uninitialized_constant(qualified_name, const_name, receiver:) NameError.new("uninitialized constant #{qualified_name}", const_name, receiver: receiver) diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 0090066151..25b7d0d9c4 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -1147,52 +1147,3 @@ class DependenciesTest < ActiveSupport::TestCase ActiveSupport::Dependencies.hook! end end - -class DependenciesLogging < ActiveSupport::TestCase - MESSAGE = "message" - - def with_settings(logger, verbose) - original_logger = ActiveSupport::Dependencies.logger - original_verbose = ActiveSupport::Dependencies.verbose - - ActiveSupport::Dependencies.logger = logger - ActiveSupport::Dependencies.verbose = verbose - - yield - ensure - ActiveSupport::Dependencies.logger = original_logger - ActiveSupport::Dependencies.verbose = original_verbose - end - - def fake_logger - Class.new do - def self.debug(message) - message - end - end - end - - test "does not log if the logger is nil and verbose is false" do - with_settings(nil, false) do - assert_nil ActiveSupport::Dependencies.log(MESSAGE) - end - end - - test "does not log if the logger is nil and verbose is true" do - with_settings(nil, true) do - assert_nil ActiveSupport::Dependencies.log(MESSAGE) - end - end - - test "does not log if the logger is set and verbose is false" do - with_settings(fake_logger, false) do - assert_nil ActiveSupport::Dependencies.log(MESSAGE) - end - end - - test "logs if the logger is set and verbose is true" do - with_settings(fake_logger, true) do - assert_equal "autoloading: #{MESSAGE}", ActiveSupport::Dependencies.log(MESSAGE) - end - end -end diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index 6bef0be0da..95778ecfd0 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -334,42 +334,6 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert_equal %i(main_autoloader), $zeitwerk_integration_reload_test end - test "verbose = true sets the dependencies logger if present" do - boot - - logger = Logger.new(File::NULL) - ActiveSupport::Dependencies.logger = logger - ActiveSupport::Dependencies.verbose = true - - Rails.autoloaders.each do |autoloader| - assert_same logger, autoloader.logger - end - end - - test "verbose = true sets the Rails logger as fallback" do - boot - - ActiveSupport::Dependencies.verbose = true - - Rails.autoloaders.each do |autoloader| - assert_same Rails.logger, autoloader.logger - end - end - - test "verbose = false sets loggers to nil" do - boot - - ActiveSupport::Dependencies.verbose = true - Rails.autoloaders.each do |autoloader| - assert autoloader.logger - end - - ActiveSupport::Dependencies.verbose = false - Rails.autoloaders.each do |autoloader| - assert_nil autoloader.logger - end - end - test "unhooks" do boot