diff options
40 files changed, 751 insertions, 109 deletions
@@ -44,7 +44,9 @@ gem "libxml-ruby", platforms: :ruby gem "connection_pool", require: false # for railties app_generator_test -gem "bootsnap", ">= 1.1.0", require: false +gem "bootsnap", ">= 1.4.0", require: false + +gem "zeitwerk", ">= 1.0.0" if RUBY_ENGINE == "ruby" # Active Job group :job do diff --git a/Gemfile.lock b/Gemfile.lock index b05ae7915e..fa8094bf23 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -166,9 +166,9 @@ GEM childprocess faraday selenium-webdriver - bootsnap (1.3.2) + bootsnap (1.4.0) msgpack (~> 1.0) - bootsnap (1.3.2-java) + bootsnap (1.4.0-java) msgpack (~> 1.0) builder (3.2.3) bunny (2.13.0) @@ -324,10 +324,10 @@ GEM minitest-server (1.0.5) minitest (~> 5.0) mono_logger (1.1.0) - msgpack (1.2.4) - msgpack (1.2.4-java) - msgpack (1.2.4-x64-mingw32) - msgpack (1.2.4-x86-mingw32) + msgpack (1.2.6) + msgpack (1.2.6-java) + msgpack (1.2.6-x64-mingw32) + msgpack (1.2.6-x86-mingw32) multi_json (1.13.1) multipart-post (2.0.0) mustache (1.1.0) @@ -516,6 +516,7 @@ GEM websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) + zeitwerk (1.0.0) PLATFORMS java @@ -535,7 +536,7 @@ DEPENDENCIES benchmark-ips blade blade-sauce_labs_plugin - bootsnap (>= 1.1.0) + bootsnap (>= 1.4.0) byebug capybara (>= 2.15) chromedriver-helper @@ -588,6 +589,7 @@ DEPENDENCIES webmock webpacker (>= 4.0.0.rc.3) websocket-client-simple! + zeitwerk (>= 1.0.0) BUNDLED WITH - 1.17.2 + 1.17.3 diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index 23488db790..893a4a25b1 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -59,6 +59,14 @@ module ActionMailer end end + initializer "action_mailer.set_autoload_paths" do |app| + options = app.config.action_mailer + + if options.show_previews && options.preview_path + ActiveSupport::Dependencies.autoload_paths << options.preview_path + end + end + initializer "action_mailer.compile_config_methods" do ActiveSupport.on_load(:action_mailer) do config.compile_methods! if config.respond_to?(:compile_methods!) @@ -76,12 +84,8 @@ module ActionMailer if options.show_previews app.routes.prepend do - get "/rails/mailers" => "rails/mailers#index", internal: true - get "/rails/mailers/*path" => "rails/mailers#preview", internal: true - end - - if options.preview_path - ActiveSupport::Dependencies.autoload_paths << options.preview_path + get "/rails/mailers" => "rails/mailers#index", internal: true + get "/rails/mailers/*path" => "rails/mailers#preview", internal: true end end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 45439a3bb1..10d85037ae 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -335,7 +335,7 @@ module ActionDispatch klass = APP_SESSIONS[app] ||= Class.new(Integration::Session) { # If the app is a Rails app, make url_helpers available on the session. # This makes app.url_for and app.foo_path available in the console. - if app.respond_to?(:routes) + if app.respond_to?(:routes) && app.routes.is_a?(ActionDispatch::Routing::RouteSet) include app.routes.url_helpers include app.routes.mounted_helpers end diff --git a/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb b/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb new file mode 100644 index 0000000000..676a8c38d4 --- /dev/null +++ b/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "abstract_unit" + +module ActionDispatch + module Routing + class NonDispatchRoutedAppTest < ActionDispatch::IntegrationTest + # For example, Grape::API + class SimpleApp + def self.call(env) + [ 200, { "Content-Type" => "text/plain" }, [] ] + end + + def self.routes + [] + end + end + + setup { @app = SimpleApp } + + test "does not except" do + get "/foo" + assert_response :success + end + end + end +end diff --git a/activejob/lib/active_job/queue_name.rb b/activejob/lib/active_job/queue_name.rb index 7bb1e35181..de259261de 100644 --- a/activejob/lib/active_job/queue_name.rb +++ b/activejob/lib/active_job/queue_name.rb @@ -18,6 +18,26 @@ module ActiveJob # post.to_feed! # end # end + # + # Can be given a block that will evaluate in the context of the job + # allowing +self.arguments+ to be accessed so that a dynamic queue name + # can be applied: + # + # class PublishToFeedJob < ApplicationJob + # queue_as do + # post = self.arguments.first + # + # if post.paid? + # :paid_feeds + # else + # :feeds + # end + # end + # + # def perform(post) + # post.to_feed! + # end + # end def queue_as(part_name = nil, &block) if block_given? self.queue_name = block diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 94906a2943..abc939826b 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -77,18 +77,18 @@ module ActiveRecord class V5_1 < V5_2 def change_column(table_name, column_name, type, options = {}) - if adapter_name == "PostgreSQL" + if connection.adapter_name == "PostgreSQL" super(table_name, column_name, type, options.except(:default, :null, :comment)) - change_column_default(table_name, column_name, options[:default]) if options.key?(:default) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + connection.change_column_default(table_name, column_name, options[:default]) if options.key?(:default) + connection.change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + connection.change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) else super end end def create_table(table_name, options = {}) - if adapter_name == "Mysql2" + if connection.adapter_name == "Mysql2" super(table_name, options: "ENGINE=InnoDB", **options) else super @@ -110,13 +110,13 @@ module ActiveRecord end def create_table(table_name, options = {}) - if adapter_name == "PostgreSQL" + if connection.adapter_name == "PostgreSQL" if options[:id] == :uuid && !options.key?(:default) options[:default] = "uuid_generate_v4()" end end - unless adapter_name == "Mysql2" && options[:id] == :bigint + unless connection.adapter_name == "Mysql2" && options[:id] == :bigint if [:integer, :bigint].include?(options[:id]) && !options.key?(:default) options[:default] = nil end @@ -190,7 +190,7 @@ module ActiveRecord if options[:name].present? options[:name].to_s else - index_name(table_name, column: column_names) + connection.index_name(table_name, column: column_names) end super end @@ -210,15 +210,17 @@ module ActiveRecord end def index_name_for_remove(table_name, options = {}) - index_name = index_name(table_name, options) + index_name = connection.index_name(table_name, options) - unless index_name_exists?(table_name, index_name) + unless connection.index_name_exists?(table_name, index_name) if options.is_a?(Hash) && options.has_key?(:name) options_without_column = options.dup options_without_column.delete :column - index_name_without_column = index_name(table_name, options_without_column) + index_name_without_column = connection.index_name(table_name, options_without_column) - return index_name_without_column if index_name_exists?(table_name, index_name_without_column) + if connection.index_name_exists?(table_name, index_name_without_column) + return index_name_without_column + end end raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' does not exist" diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index cef31bea94..bdd3c540bb 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -186,11 +186,9 @@ module ActiveRecord relation = apply_join_dependency relation.pluck(*column_names) else - disallow_raw_sql!(column_names) + klass.disallow_raw_sql!(column_names) relation = spawn - relation.select_values = column_names.map { |cn| - @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn - } + relation.select_values = column_names result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } result.cast_values(klass.attribute_types) end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index eb80aab701..75976aa8fc 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1052,11 +1052,13 @@ module ActiveRecord def arel_columns(columns) columns.flat_map do |field| - if (Symbol === field || String === field) && (klass.has_attribute?(field) || klass.attribute_alias?(field)) && !from_clause.value - arel_attribute(field) - elsif Symbol === field - connection.quote_table_name(field.to_s) - elsif Proc === field + case field + when Symbol + field = field.to_s + arel_column(field) { connection.quote_table_name(field) } + when String + arel_column(field) { field } + when Proc field.call else field @@ -1064,6 +1066,16 @@ module ActiveRecord end end + def arel_column(field) + field = klass.attribute_alias(field) if klass.attribute_alias?(field) + + if klass.columns_hash.key?(field) && !from_clause.value + arel_attribute(field) + else + yield + end + end + def reverse_sql_order(order_query) if order_query.empty? return [arel_attribute(primary_key).desc] if primary_key diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index c01138c059..3525fa2ab8 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -448,8 +448,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_with_select - assert_equal 1, Company.find(2).firm_with_select.attributes.size - assert_equal 1, Company.all.merge!(includes: :firm_with_select).find(2).firm_with_select.attributes.size + assert_equal 1, Post.find(2).author_with_select.attributes.size + assert_equal 1, Post.includes(:author_with_select).find(2).author_with_select.attributes.size + end + + def test_custom_attribute_with_select + assert_equal 2, Company.find(2).firm_with_select.attributes.size + assert_equal 2, Company.includes(:firm_with_select).find(2).firm_with_select.attributes.size end def test_belongs_to_without_counter_cache_option diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 363593ca19..1b8f748bad 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1226,14 +1226,15 @@ class BasicsTest < ActiveRecord::TestCase end def test_attribute_names - assert_equal ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"], - Company.attribute_names + expected = ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description", "metadata"] + assert_equal expected, Company.attribute_names end def test_has_attribute assert Company.has_attribute?("id") assert Company.has_attribute?("type") assert Company.has_attribute?("name") + assert Company.has_attribute?("metadata") assert_not Company.has_attribute?("lastname") assert_not Company.has_attribute?("age") end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 850bc49676..b001667ac9 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -690,8 +690,9 @@ class CalculationsTest < ActiveRecord::TestCase end def test_pluck_not_auto_table_name_prefix_if_column_joined - Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)]) - assert_equal [7], Company.joins(:contracts).pluck(:developer_id) + company = Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)]) + metadata = company.contracts.first.metadata + assert_equal [metadata], Company.joins(:contracts).pluck(:metadata) end def test_pluck_with_selection_clause diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 0ab0459c38..857d743605 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -14,6 +14,7 @@ require "models/person" require "models/computer" require "models/reply" require "models/company" +require "models/contract" require "models/bird" require "models/car" require "models/engine" @@ -1815,6 +1816,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal [1, 1, 1], posts.map(&:author_address_id) end + test "joins with select custom attribute" do + contract = Company.create!(name: "test").contracts.create! + company = Company.joins(:contracts).select(:id, :metadata).find(contract.company_id) + assert_equal contract.metadata, company.metadata + end + test "delegations do not leak to other classes" do Topic.all.by_lifo assert Topic.all.class.method_defined?(:by_lifo) diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 838f515aad..a0f48d23f1 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -13,6 +13,8 @@ class Company < AbstractCompany has_many :contracts has_many :developers, through: :contracts + attribute :metadata, :json + scope :of_first_firm, lambda { joins(account: :firm). where("firms.id" => 1) diff --git a/activerecord/test/models/contract.rb b/activerecord/test/models/contract.rb index 3f663375c4..89719775c4 100644 --- a/activerecord/test/models/contract.rb +++ b/activerecord/test/models/contract.rb @@ -5,7 +5,9 @@ class Contract < ActiveRecord::Base belongs_to :developer, primary_key: :id belongs_to :firm, foreign_key: "company_id" - before_save :hi + attribute :metadata, :json + + before_save :hi, :update_metadata after_save :bye attr_accessor :hi_count, :bye_count @@ -19,6 +21,10 @@ class Contract < ActiveRecord::Base @bye_count ||= 0 @bye_count += 1 end + + def update_metadata + self.metadata = { company_id: company_id, developer_id: developer_id } + end end class NewContract < Contract diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index e32cc59399..65d1fce66d 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -31,6 +31,7 @@ class Post < ActiveRecord::Base belongs_to :author_with_posts, -> { includes(:posts) }, class_name: "Author", foreign_key: :author_id belongs_to :author_with_address, -> { includes(:author_address) }, class_name: "Author", foreign_key: :author_id + belongs_to :author_with_select, -> { select(:id) }, class_name: "Author", foreign_key: :author_id def first_comment super.body diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 86d5a67a13..349b8afc48 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -247,6 +247,7 @@ ActiveRecord::Schema.define do create_table :contracts, force: true do |t| t.references :developer, index: false t.references :company, index: false + t.string :metadata end create_table :customers, force: true do |t| diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7002474b8d..8bb531f1b8 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Revise `ActiveSupport::Notifications.unsubscribe` to correctly handle Regex or other multiple-pattern subscribers. + + *Zach Kemp* + * Add `before_reset` callback to `CurrentAttributes` and define `after_reset` as an alias of `resets` for symmetry. *Rosa Gutierrez* diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb new file mode 100644 index 0000000000..6892b31a5d --- /dev/null +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module ActiveSupport + module Dependencies + module ZeitwerkIntegration # :nodoc: all + module Decorations + def clear + Dependencies.unload_interlock do + Rails.autoloader.reload + end + end + + def constantize(cpath) + Inflector.constantize(cpath) + end + + def safe_constantize(cpath) + Inflector.safe_constantize(cpath) + end + + def autoloaded_constants + Rails.autoloaders.flat_map do |autoloader| + autoloader.loaded.to_a + end + end + + def autoloaded?(object) + cpath = object.is_a?(Module) ? object.name : object.to_s + Rails.autoloaders.any? { |autoloader| autoloader.loaded?(cpath) } + end + end + + class << self + def take_over + setup_autoloaders + freeze_autoload_paths + decorate_dependencies + end + + private + + def setup_autoloaders + Dependencies.autoload_paths.each do |autoload_path| + if File.directory?(autoload_path) + if autoload_once?(autoload_path) + Rails.once_autoloader.push_dir(autoload_path) + else + Rails.autoloader.push_dir(autoload_path) + end + end + end + + Rails.autoloaders.each(&:setup) + end + + def autoload_once?(autoload_path) + Dependencies.autoload_once_paths.include?(autoload_path) || + Gem.path.any? { |gem_path| autoload_path.to_s.start_with?(gem_path) } + end + + def freeze_autoload_paths + Dependencies.autoload_paths.freeze + Dependencies.autoload_once_paths.freeze + end + + def decorate_dependencies + Dependencies.singleton_class.prepend(Decorations) + Object.class_eval { alias_method :require_dependency, :require } + end + end + end + end +end diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb index 7ccc333463..d9e93b530c 100644 --- a/activesupport/lib/active_support/notifications.rb +++ b/activesupport/lib/active_support/notifications.rb @@ -153,6 +153,15 @@ module ActiveSupport # # ActiveSupport::Notifications.unsubscribe("render") # + # Subscribers using a regexp or other pattern-matching object will remain subscribed + # to all events that match their original pattern, unless those events match a string + # passed to `unsubscribe`: + # + # subscriber = ActiveSupport::Notifications.subscribe(/render/) { } + # ActiveSupport::Notifications.unsubscribe('render_template.action_view') + # subscriber.matches?('render_template.action_view') # => false + # subscriber.matches?('render_partial.action_view') # => true + # # == Default Queue # # Notifications ships with a queue implementation that consumes and publishes events diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 11721db103..f06504cf2c 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -2,6 +2,7 @@ require "mutex_m" require "concurrent/map" +require "set" module ActiveSupport module Notifications @@ -39,6 +40,7 @@ module ActiveSupport when String @string_subscribers[subscriber_or_name].clear @listeners_for.delete(subscriber_or_name) + @other_subscribers.each { |sub| sub.unsubscribe!(subscriber_or_name) } else pattern = subscriber_or_name.try(:pattern) if String === pattern @@ -113,11 +115,33 @@ module ActiveSupport end end + class Matcher #:nodoc: + attr_reader :pattern, :exclusions + + def self.wrap(pattern) + return pattern if String === pattern + new(pattern) + end + + def initialize(pattern) + @pattern = pattern + @exclusions = Set.new + end + + def unsubscribe!(name) + exclusions << -name if pattern === name + end + + def ===(name) + pattern === name && !exclusions.include?(name) + end + end + class Evented #:nodoc: attr_reader :pattern def initialize(pattern, delegate) - @pattern = pattern + @pattern = Matcher.wrap(pattern) @delegate = delegate @can_publish = delegate.respond_to?(:publish) end @@ -137,11 +161,15 @@ module ActiveSupport end def subscribed_to?(name) - @pattern === name + pattern === name end def matches?(name) - @pattern && @pattern === name + pattern && pattern === name + end + + def unsubscribe!(name) + pattern.unsubscribe!(name) end end @@ -204,6 +232,10 @@ module ActiveSupport true end + def unsubscribe!(*) + false + end + alias :matches? :=== end end diff --git a/activesupport/test/notifications/evented_notification_test.rb b/activesupport/test/notifications/evented_notification_test.rb index 4beb8194b9..ab2a9b8659 100644 --- a/activesupport/test/notifications/evented_notification_test.rb +++ b/activesupport/test/notifications/evented_notification_test.rb @@ -84,6 +84,39 @@ module ActiveSupport [:finish, "hi", 1, {}] ], listener.events end + + def test_listen_to_regexp + notifier = Fanout.new + listener = Listener.new + notifier.subscribe(/[a-z]*.world/, listener) + notifier.start("hi.world", 1, {}) + notifier.finish("hi.world", 2, {}) + notifier.start("hello.world", 1, {}) + notifier.finish("hello.world", 2, {}) + + assert_equal [ + [:start, "hi.world", 1, {}], + [:finish, "hi.world", 2, {}], + [:start, "hello.world", 1, {}], + [:finish, "hello.world", 2, {}] + ], listener.events + end + + def test_listen_to_regexp_with_exclusions + notifier = Fanout.new + listener = Listener.new + notifier.subscribe(/[a-z]*.world/, listener) + notifier.unsubscribe("hi.world") + notifier.start("hi.world", 1, {}) + notifier.finish("hi.world", 2, {}) + notifier.start("hello.world", 1, {}) + notifier.finish("hello.world", 2, {}) + + assert_equal [ + [:start, "hello.world", 1, {}], + [:finish, "hello.world", 2, {}] + ], listener.events + end end end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index b5d72d1a42..bb20d26a25 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -128,6 +128,25 @@ module Notifications assert_equal [["named.subscription", :foo], ["named.subscription", :foo]], @events end + def test_unsubscribing_by_name_leaves_regexp_matched_subscriptions + @matched_events = [] + @notifier.subscribe(/subscription/) { |*args| @matched_events << event(*args) } + @notifier.publish("named.subscription", :before) + @notifier.wait + [@events, @named_events, @matched_events].each do |collector| + assert_includes(collector, ["named.subscription", :before]) + end + @notifier.unsubscribe("named.subscription") + @notifier.publish("named.subscription", :after) + @notifier.publish("other.subscription", :after) + @notifier.wait + assert_includes(@events, ["named.subscription", :after]) + assert_includes(@events, ["other.subscription", :after]) + assert_includes(@matched_events, ["other.subscription", :after]) + assert_not_includes(@matched_events, ["named.subscription", :after]) + assert_not_includes(@named_events, ["named.subscription", :after]) + end + private def event(*args) args diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 19f4de8a1d..d66add2ca0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix non-symbol access to nested hashes returned from `Rails::Application.config_for` + being broken by allowing non-symbol access with a deprecation notice. + + *Ufuk Kayserilioglu* + * Fix deeply nested namespace command printing. *Gannon McGibbon* diff --git a/railties/lib/rails.rb b/railties/lib/rails.rb index 092105d502..bca2cf34e1 100644 --- a/railties/lib/rails.rb +++ b/railties/lib/rails.rb @@ -110,5 +110,25 @@ module Rails def public_path application && Pathname.new(application.paths["public"].first) end + + def autoloader + if configuration.autoloader == :zeitwerk + @autoloader ||= Zeitwerk::Loader.new + end + end + + def once_autoloader + if configuration.autoloader == :zeitwerk + @once_autoloader ||= Zeitwerk::Loader.new + end + end + + def autoloaders + if configuration.autoloader == :zeitwerk + [autoloader, once_autoloader] + else + [] + end + end end end diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 5a924ab8e6..d0417f8a49 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -7,6 +7,7 @@ require "active_support/key_generator" require "active_support/message_verifier" require "active_support/encrypted_configuration" require "active_support/deprecation" +require "active_support/hash_with_indifferent_access" require "rails/engine" require "rails/secrets" @@ -230,8 +231,8 @@ module Rails config = YAML.load(ERB.new(yaml.read).result) || {} config = (config["shared"] || {}).merge(config[env] || {}) - ActiveSupport::OrderedOptions.new.tap do |config_as_ordered_options| - config_as_ordered_options.update(config.deep_symbolize_keys) + ActiveSupport::OrderedOptions.new.tap do |options| + options.update(NonSymbolAccessDeprecatedHash.new(config)) end else raise "Could not load configuration. No such file - #{yaml}" @@ -590,5 +591,38 @@ module Rails def build_middleware config.app_middleware + super end + + class NonSymbolAccessDeprecatedHash < HashWithIndifferentAccess # :nodoc: + def initialize(value = nil) + if value.is_a?(Hash) + value.each_pair { |k, v| self[k] = v } + else + super + end + end + + def []=(key, value) + if value.is_a?(Hash) + value = self.class.new(value) + end + super(key.to_sym, value) + end + + private + + def convert_key(key) + unless key.kind_of?(Symbol) + ActiveSupport::Deprecation.warn(<<~MESSAGE.squish) + Accessing hashes returned from config_for by non-symbol keys + is deprecated and will be removed in Rails 6.1. + Use symbols for access instead. + MESSAGE + + key = key.to_sym + end + + key + end + end end end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index b7838f7e32..16fbc99e7a 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -20,7 +20,7 @@ module Rails :read_encrypted_secrets, :log_level, :content_security_policy_report_only, :content_security_policy_nonce_generator, :require_master_key, :credentials - attr_reader :encoding, :api_only, :loaded_config_version + attr_reader :encoding, :api_only, :loaded_config_version, :autoloader def initialize(*) super @@ -64,6 +64,7 @@ module Rails @credentials = ActiveSupport::OrderedOptions.new @credentials.content_path = default_credentials_content_path @credentials.key_path = default_credentials_key_path + @autoloader = :classic end def load_defaults(target_version) @@ -117,6 +118,8 @@ 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 @@ -267,6 +270,14 @@ module Rails end end + def autoloader=(autoloader) + if %i(classic zeitwerk).include?(autoloader) + @autoloader = autoloader + else + raise ArgumentError, "config.autoloader may be :classic or :zeitwerk, got #{autoloader.inspect} instead" + end + end + class Custom #:nodoc: def initialize @configurations = Hash.new diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 04aaf6dd9a..39e8ef6631 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -21,6 +21,13 @@ module Rails end end + initializer :let_zeitwerk_take_over do + if config.autoloader == :zeitwerk + require "active_support/dependencies/zeitwerk_integration" + ActiveSupport::Dependencies::ZeitwerkIntegration.take_over + end + end + initializer :add_builtin_route do |app| if Rails.env.development? app.routes.prepend do @@ -66,6 +73,7 @@ module Rails initializer :eager_load! do if config.eager_load ActiveSupport.run_load_hooks(:before_eager_load, self) + Zeitwerk::Loader.eager_load_all if defined?(Zeitwerk) config.eager_load_namespaces.each(&:eager_load!) end end diff --git a/railties/lib/rails/command/behavior.rb b/railties/lib/rails/command/behavior.rb index 7f32b04cf1..7fb2a99e67 100644 --- a/railties/lib/rails/command/behavior.rb +++ b/railties/lib/rails/command/behavior.rb @@ -71,8 +71,9 @@ module Rails paths = [] namespaces.each do |namespace| pieces = namespace.split(":") - paths << pieces.dup.push(pieces.last).join("/") - paths << pieces.join("/") + path = pieces.join("/") + paths << "#{path}/#{pieces.last}" + paths << path end paths.uniq! paths diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index d6c329b581..2485158a7b 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -472,12 +472,10 @@ module Rails # Eager load the application by loading all ruby # files inside eager_load paths. def eager_load! - config.eager_load_paths.each do |load_path| - # Starts after load_path plus a slash, ends before ".rb". - relname_range = (load_path.to_s.length + 1)...-3 - Dir.glob("#{load_path}/**/*.rb").sort.each do |file| - require_dependency file[relname_range] - end + if Rails.autoloader + eager_load_with_zeitwerk! + else + eager_load_with_dependencies! end end @@ -653,6 +651,22 @@ module Rails private + def eager_load_with_zeitwerk! + (config.eager_load_paths - Zeitwerk::Loader.all_dirs).each do |path| + Dir.glob("#{path}/**/*.rb").sort.each { |file| require file } + end + end + + def eager_load_with_dependencies! + config.eager_load_paths.each do |load_path| + # Starts after load_path plus a slash, ends before ".rb". + relname_range = (load_path.to_s.length + 1)...-3 + Dir.glob("#{load_path}/**/*.rb").sort.each do |file| + require_dependency file[relname_range] + end + end + end + def load_config_initializer(initializer) # :doc: ActiveSupport::Notifications.instrument("load_config_initializer.railties", initializer: initializer) do load(initializer) diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index d39b5d311f..a1f1224a45 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -28,7 +28,7 @@ ruby <%= "'#{RUBY_VERSION}'" -%> <% if depend_on_bootsnap? -%> # Reduces boot times through caching; required in config/boot.rb -gem 'bootsnap', '>= 1.1.0', require: false +gem 'bootsnap', '>= 1.4.0', require: false <%- end -%> <%- if options.api? -%> @@ -36,7 +36,9 @@ gem 'bootsnap', '>= 1.1.0', require: false # gem 'rack-cors' <%- end -%> -<% if RUBY_ENGINE == 'ruby' -%> +<% if RUBY_ENGINE == "ruby" -%> +gem "zeitwerk", ">= 1.0.0" + group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 37fba72ee3..960f708bdf 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1157,6 +1157,27 @@ module ApplicationTests end end + test "autoloader & autoloader=" do + app "development" + + config = Rails.application.config + assert_instance_of Zeitwerk::Loader, Rails.autoloader + assert_instance_of Zeitwerk::Loader, Rails.once_autoloader + assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + + config.autoloader = :classic + assert_nil Rails.autoloader + assert_nil Rails.once_autoloader + assert_empty Rails.autoloaders + + config.autoloader = :zeitwerk + assert_instance_of Zeitwerk::Loader, Rails.autoloader + assert_instance_of Zeitwerk::Loader, Rails.once_autoloader + assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + + assert_raises(ArgumentError) { config.autoloader = :unknown } + end + test "config.action_view.cache_template_loading with cache_classes default" do add_to_config "config.cache_classes = true" @@ -1714,7 +1735,7 @@ module ApplicationTests assert_equal true, Rails.application.config.action_mailer.show_previews end - test "config_for loads custom configuration from yaml accessible as symbol" do + test "config_for loads custom configuration from yaml accessible as symbol or string" do app_file "config/custom.yml", <<-RUBY development: foo: 'bar' @@ -1727,6 +1748,7 @@ module ApplicationTests app "development" assert_equal "bar", Rails.application.config.my_custom_config[:foo] + assert_equal "bar", Rails.application.config.my_custom_config["foo"] end test "config_for loads nested custom configuration from yaml as symbol keys" do @@ -1746,6 +1768,25 @@ module ApplicationTests assert_equal 1, Rails.application.config.my_custom_config[:foo][:bar][:baz] end + test "config_for loads nested custom configuration from yaml with deprecated non-symbol access" do + app_file "config/custom.yml", <<-RUBY + development: + foo: + bar: + baz: 1 + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + + app "development" + + assert_deprecated do + assert_equal 1, Rails.application.config.my_custom_config["foo"]["bar"]["baz"] + end + end + test "config_for makes all hash methods available" do app_file "config/custom.yml", <<-RUBY development: @@ -1762,12 +1803,93 @@ module ApplicationTests actual = Rails.application.config.my_custom_config - assert_equal actual, foo: 0, bar: { baz: 1 } - assert_equal actual.keys, [ :foo, :bar ] - assert_equal actual.values, [ 0, baz: 1] - assert_equal actual.to_h, foo: 0, bar: { baz: 1 } - assert_equal actual[:foo], 0 - assert_equal actual[:bar], baz: 1 + assert_equal({ foo: 0, bar: { baz: 1 } }, actual) + assert_equal([ :foo, :bar ], actual.keys) + assert_equal([ 0, baz: 1], actual.values) + assert_equal({ foo: 0, bar: { baz: 1 } }, actual.to_h) + assert_equal(0, actual[:foo]) + assert_equal({ baz: 1 }, actual[:bar]) + end + + test "config_for generates deprecation notice when nested hash methods are called with non-symbols" do + app_file "config/custom.yml", <<-RUBY + development: + foo: + bar: 1 + baz: 2 + qux: + boo: 3 + RUBY + + app "development" + + actual = Rails.application.config_for("custom")[:foo] + + # slice + assert_deprecated do + assert_equal({ bar: 1, baz: 2 }, actual.slice("bar", "baz")) + end + + # except + assert_deprecated do + assert_equal({ qux: { boo: 3 } }, actual.except("bar", "baz")) + end + + # dig + assert_deprecated do + assert_equal(3, actual.dig("qux", "boo")) + end + + # fetch - hit + assert_deprecated do + assert_equal(1, actual.fetch("bar", 0)) + end + + # fetch - miss + assert_deprecated do + assert_equal(0, actual.fetch("does-not-exist", 0)) + end + + # fetch_values + assert_deprecated do + assert_equal([1, 2], actual.fetch_values("bar", "baz")) + end + + # key? - hit + assert_deprecated do + assert(actual.key?("bar")) + end + + # key? - miss + assert_deprecated do + assert_not(actual.key?("does-not-exist")) + end + + # slice! + actual = Rails.application.config_for("custom")[:foo] + + assert_deprecated do + slice = actual.slice!("bar", "baz") + assert_equal({ bar: 1, baz: 2 }, actual) + assert_equal({ qux: { boo: 3 } }, slice) + end + + # extract! + actual = Rails.application.config_for("custom")[:foo] + + assert_deprecated do + extracted = actual.extract!("bar", "baz") + assert_equal({ bar: 1, baz: 2 }, extracted) + assert_equal({ qux: { boo: 3 } }, actual) + end + + # except! + actual = Rails.application.config_for("custom")[:foo] + + assert_deprecated do + actual.except!("bar", "baz") + assert_equal({ qux: { boo: 3 } }, actual) + end end test "config_for uses the Pathname object if it is provided" do diff --git a/railties/test/application/mailer_previews_test.rb b/railties/test/application/mailer_previews_test.rb index ba186bda44..fb84276b8a 100644 --- a/railties/test/application/mailer_previews_test.rb +++ b/railties/test/application/mailer_previews_test.rb @@ -85,6 +85,7 @@ module ApplicationTests end test "mailer previews are loaded from a custom preview_path" do + app_dir "lib/mailer_previews" add_to_config "config.action_mailer.preview_path = '#{app_path}/lib/mailer_previews'" mailer "notifier", <<-RUBY @@ -254,6 +255,7 @@ module ApplicationTests end test "mailer previews are reloaded from a custom preview_path" do + app_dir "lib/mailer_previews" add_to_config "config.action_mailer.preview_path = '#{app_path}/lib/mailer_previews'" app("development") @@ -818,6 +820,7 @@ module ApplicationTests def build_app super app_file "config/routes.rb", "Rails.application.routes.draw do; end" + app_dir "test/mailers/previews" end def mailer(name, contents) diff --git a/railties/test/application/multiple_applications_test.rb b/railties/test/application/multiple_applications_test.rb index 432344bccc..f0f1112f6b 100644 --- a/railties/test/application/multiple_applications_test.rb +++ b/railties/test/application/multiple_applications_test.rb @@ -100,30 +100,6 @@ module ApplicationTests assert_nothing_raised { AppTemplate::Application.new } end - def test_initializers_run_on_different_applications_go_to_the_same_class - application1 = AppTemplate::Application.new - run_count = 0 - - AppTemplate::Application.initializer :init0 do - run_count += 1 - end - - application1.initializer :init1 do - run_count += 1 - end - - AppTemplate::Application.new.initializer :init2 do - run_count += 1 - end - - assert_equal 0, run_count, "Without loading the initializers, the count should be 0" - - # Set config.eager_load to false so that an eager_load warning doesn't pop up - AppTemplate::Application.create { config.eager_load = false }.initialize! - - assert_equal 3, run_count, "There should have been three initializers that incremented the count" - end - def test_consoles_run_on_different_applications_go_to_the_same_class run_count = 0 AppTemplate::Application.console { run_count += 1 } diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 44e3b0f66b..fe56e3d076 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -145,8 +145,8 @@ module ApplicationTests # loading a specific fixture rails "db:fixtures:load", "FIXTURES=products" - assert_equal 2, ::AppTemplate::Application::Product.count - assert_equal 0, ::AppTemplate::Application::User.count + assert_equal 2, Product.count + assert_equal 0, User.count end def test_loading_only_yml_fixtures diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb new file mode 100644 index 0000000000..628a85acd8 --- /dev/null +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" +require "active_support/dependencies/zeitwerk_integration" + +class ZeitwerkIntegrationTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + end + + def boot(env = "development") + app(env) + end + + def teardown + teardown_app + end + + def deps + ActiveSupport::Dependencies + end + + def decorated? + deps.singleton_class < deps::ZeitwerkIntegration::Decorations + end + + test "ActiveSupport::Dependencies is decorated by default" do + boot + + assert decorated? + assert_instance_of Zeitwerk::Loader, Rails.autoloader + assert_instance_of Zeitwerk::Loader, Rails.once_autoloader + assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + end + + test "ActiveSupport::Dependencies is not decorated in classic mode" do + add_to_config "config.autoloader = :classic" + boot + + assert_not decorated? + assert_nil Rails.autoloader + assert_nil Rails.once_autoloader + assert_empty Rails.autoloaders + end + + test "constantize returns the value stored in the constant" do + app_file "app/models/admin/user.rb", "class Admin::User; end" + boot + + assert_same Admin::User, deps.constantize("Admin::User") + end + + test "constantize raises if the constant is unknown" do + boot + + assert_raises(NameError) { deps.constantize("Admin") } + end + + test "safe_constantize returns the value stored in the constant" do + app_file "app/models/admin/user.rb", "class Admin::User; end" + boot + + assert_same Admin::User, deps.safe_constantize("Admin::User") + end + + test "safe_constantize returns nil for unknown constants" do + boot + + assert_nil deps.safe_constantize("Admin") + end + + test "autoloaded_constants returns autoloaded constant paths" do + app_file "app/models/admin/user.rb", "class Admin::User; end" + app_file "app/models/post.rb", "class Post; end" + boot + + assert Admin::User + assert_equal ["Admin", "Admin::User"], deps.autoloaded_constants + end + + test "autoloaded? says if a constant has been autoloaded" do + app_file "app/models/user.rb", "class User; end" + app_file "app/models/post.rb", "class Post; end" + boot + + assert Post + assert deps.autoloaded?("Post") + assert deps.autoloaded?(Post) + assert_not deps.autoloaded?("User") + end + + test "eager loading loads the application code" do + $zeitwerk_integration_test_user = false + $zeitwerk_integration_test_post = false + + app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" + app_file "app/models/post.rb", "class Post; end; $zeitwerk_integration_test_post = true" + boot("production") + + assert $zeitwerk_integration_test_user + assert $zeitwerk_integration_test_post + end + + test "eager loading loads anything managed by Zeitwerk" do + $zeitwerk_integration_test_user = false + app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" + + $zeitwerk_integration_test_extras = false + app_dir "extras" + app_file "extras/webhook_hacks.rb", "WebhookHacks = 1; $zeitwerk_integration_test_extras = true" + + require "zeitwerk" + autoloader = Zeitwerk::Loader.new + autoloader.push_dir("#{app_path}/extras") + autoloader.setup + + boot("production") + + assert $zeitwerk_integration_test_user + assert $zeitwerk_integration_test_extras + end + + test "autoload paths that are below Gem.path go to the once autoloader" do + app_dir "extras" + add_to_config 'config.autoload_paths << "#{Rails.root}/extras"' + + # Mocks Gem.path to include the extras directory. + Gem.singleton_class.prepend( + Module.new do + def path + super + ["#{Rails.root}/extras"] + end + end + ) + boot + + assert_not_includes Rails.autoloader.dirs, "#{app_path}/extras" + assert_includes Rails.once_autoloader.dirs, "#{app_path}/extras" + end + + test "clear reloads the main autoloader, and does not reload the once one" do + boot + + $zeitwerk_integration_reload_test = [] + + autoloader = Rails.autoloader + def autoloader.reload + $zeitwerk_integration_reload_test << :autoloader + super + end + + once_autoloader = Rails.once_autoloader + def once_autoloader.reload + $zeitwerk_integration_reload_test << :once_autoloader + super + end + + ActiveSupport::Dependencies.clear + + assert_equal %i(autoloader), $zeitwerk_integration_reload_test + end +end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 1ee9e43e89..937b8eb427 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -660,6 +660,15 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_gem "jbuilder" end + def test_inclusion_of_zeitwerk + run_generator + if RUBY_ENGINE == "ruby" + assert_gem "zeitwerk" + else + assert_no_gem "zeitwerk" + end + end + def test_inclusion_of_a_debugger run_generator if defined?(JRUBY_VERSION) || RUBY_ENGINE == "rbx" diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 0e8e0e86ee..47d42645c6 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -421,6 +421,10 @@ module TestHelpers file_name end + def app_dir(path) + FileUtils.mkdir_p("#{app_path}/#{path}") + end + def remove_file(path) FileUtils.rm_rf "#{app_path}/#{path}" end @@ -487,7 +491,11 @@ Module.new do # Fake 'Bundler.require' -- we run using the repo's Gemfile, not an # app-specific one: we don't want to require every gem that lists. contents = File.read("#{app_template_path}/config/application.rb") - contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker).each { |r| require r }") + if RUBY_ENGINE == "ruby" + contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker zeitwerk).each { |r| require r }") + else + contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker).each { |r| require r }") + end File.write("#{app_template_path}/config/application.rb", contents) require "rails" diff --git a/railties/test/isolation/assets/package.json b/railties/test/isolation/assets/package.json index 106b1029f0..7c34450fe0 100644 --- a/railties/test/isolation/assets/package.json +++ b/railties/test/isolation/assets/package.json @@ -2,6 +2,8 @@ "name": "dummy", "private": true, "dependencies": { + "@rails/actioncable": "file:../../../../actioncable", + "@rails/activestorage": "file:../../../../activestorage", "@rails/ujs": "file:../../../../actionview", "@rails/webpacker": "https://github.com/rails/webpacker.git", "turbolinks": "^5.2.0" diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 851407dede..69f6e34d58 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -704,25 +704,27 @@ YAML RUBY @plugin.write "app/controllers/bukkits/foo_controller.rb", <<-RUBY - class Bukkits::FooController < ActionController::Base - def index - render inline: "<%= help_the_engine %>" - end + module Bukkits + class FooController < ActionController::Base + def index + render inline: "<%= help_the_engine %>" + end - def show - render plain: foo_path - end + def show + render plain: foo_path + end - def from_app - render inline: "<%= (self.respond_to?(:bar_path) || self.respond_to?(:something)) %>" - end + def from_app + render inline: "<%= (self.respond_to?(:bar_path) || self.respond_to?(:something)) %>" + end - def routes_helpers_in_view - render inline: "<%= foo_path %>, <%= main_app.bar_path %>" - end + def routes_helpers_in_view + render inline: "<%= foo_path %>, <%= main_app.bar_path %>" + end - def polymorphic_path_without_namespace - render plain: polymorphic_path(Post.new) + def polymorphic_path_without_namespace + render plain: polymorphic_path(Post.new) + end end end RUBY |