diff options
26 files changed, 241 insertions, 142 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index 908a988e69..3c6e743df6 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -23,7 +23,7 @@ checks: engines: rubocop: enabled: true - channel: rubocop-0-63 + channel: rubocop-0-66 ratings: paths: @@ -44,7 +44,7 @@ gem "libxml-ruby", platforms: :ruby gem "connection_pool", require: false # for railties app_generator_test -gem "bootsnap", ">= 1.4.0", require: false +gem "bootsnap", ">= 1.4.2", require: false # Active Job group :job do diff --git a/Gemfile.lock b/Gemfile.lock index d4c2808b38..62cab7261b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -70,7 +70,7 @@ PATH i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) - zeitwerk (~> 1.4, >= 1.4.2) + zeitwerk (~> 1.4, >= 1.4.3) rails (6.0.0.beta3) actioncable (= 6.0.0.beta3) actionmailbox (= 6.0.0.beta3) @@ -165,9 +165,9 @@ GEM childprocess faraday selenium-webdriver - bootsnap (1.4.0) + bootsnap (1.4.2) msgpack (~> 1.0) - bootsnap (1.4.0-java) + bootsnap (1.4.2-java) msgpack (~> 1.0) builder (3.2.3) bunny (2.13.0) @@ -319,10 +319,10 @@ GEM minitest-server (1.0.5) minitest (~> 5.0) mono_logger (1.1.0) - msgpack (1.2.6) - msgpack (1.2.6-java) - msgpack (1.2.6-x64-mingw32) - msgpack (1.2.6-x86-mingw32) + msgpack (1.2.9) + msgpack (1.2.9-java) + msgpack (1.2.9-x64-mingw32) + msgpack (1.2.9-x86-mingw32) multi_json (1.13.1) multipart-post (2.0.0) mustache (1.1.0) @@ -342,14 +342,17 @@ GEM mini_portile2 (~> 2.4.0) os (1.0.0) parallel (1.13.0) - parser (2.6.0.0) + parser (2.6.2.0) ast (~> 2.4.0) path_expander (1.0.3) pg (1.1.3) pg (1.1.3-x64-mingw32) pg (1.1.3-x86-mingw32) - powerpack (0.1.2) - psych (3.0.3) + psych (3.1.0) + psych (3.1.0-java) + jar-dependencies (>= 0.1.7) + psych (3.1.0-x64-mingw32) + psych (3.1.0-x86-mingw32) public_suffix (3.0.3) puma (3.12.1) puma (3.12.1-java) @@ -400,14 +403,14 @@ GEM resque (~> 1.26) rufus-scheduler (~> 3.2) retriable (3.1.2) - rubocop (0.63.0) + rubocop (0.66.0) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) - powerpack (~> 0.1) + psych (>= 3.1.0) rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) - unicode-display_width (~> 1.4.0) + unicode-display_width (>= 1.4.0, < 1.6) ruby-progressbar (1.10.0) ruby-vips (2.0.13) ffi (~> 1.9) @@ -488,7 +491,7 @@ GEM uber (0.1.0) uglifier (4.1.19) execjs (>= 0.3.0, < 3) - unicode-display_width (1.4.0) + unicode-display_width (1.5.0) useragent (0.16.10) vegas (0.1.11) rack (>= 1.0.0) @@ -517,7 +520,7 @@ GEM websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (1.4.2) + zeitwerk (1.4.3) PLATFORMS java @@ -537,7 +540,7 @@ DEPENDENCIES benchmark-ips blade blade-sauce_labs_plugin - bootsnap (>= 1.4.0) + bootsnap (>= 1.4.2) byebug capybara (>= 2.15) connection_pool diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 61773d97a2..bb49bc4dda 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -60,7 +60,11 @@ module ActionDispatch log_error(request, wrapper) if request.get_header("action_dispatch.show_detailed_exceptions") - content_type = request.formats.first + begin + content_type = request.formats.first + rescue Mime::Type::InvalidMimeType + render_for_api_request(Mime[:text], wrapper) + end if api_request?(content_type) render_for_api_request(content_type, wrapper) diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 1fb3e9db00..0cc56f5013 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -12,6 +12,7 @@ module ActionDispatch "ActionController::UnknownHttpMethod" => :method_not_allowed, "ActionController::NotImplemented" => :not_implemented, "ActionController::UnknownFormat" => :not_acceptable, + "Mime::Type::InvalidMimeType" => :not_acceptable, "ActionController::MissingExactTemplate" => :not_acceptable, "ActionController::InvalidAuthenticityToken" => :unprocessable_entity, "ActionController::InvalidCrossOriginRequest" => :unprocessable_entity, diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb index 3feb3a19f3..a88ad40f21 100644 --- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb @@ -21,8 +21,12 @@ module ActionDispatch def call(env) request = ActionDispatch::Request.new(env) status = request.path_info[1..-1].to_i - content_type = request.formats.first - body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) } + begin + content_type = request.formats.first + rescue Mime::Type::InvalidMimeType + content_type = Mime[:text] + end + body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) } render(status, content_type, body) end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index c85476fa38..8b1b3c0277 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -58,6 +58,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest raise ActionController::NotImplemented when "/unprocessable_entity" raise ActionController::InvalidAuthenticityToken + when "/invalid_mimetype" + raise Mime::Type::InvalidMimeType when "/not_found_original_exception" begin raise AbstractController::ActionNotFound.new @@ -178,6 +180,10 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/parameter_missing", headers: { "action_dispatch.show_exceptions" => true } assert_response 400 assert_match(/ActionController::ParameterMissing/, body) + + get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => true } + assert_response 406 + assert_match(/Mime::Type::InvalidMimeType/, body) end test "rescue with text error for xhr request" do diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index f802abc653..6fafa4e426 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -9,6 +9,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest case req.path when "/not_found" raise AbstractController::ActionNotFound + when "/invalid_mimetype" + raise Mime::Type::InvalidMimeType when "/bad_params", "/bad_params.json" begin raise StandardError.new @@ -62,6 +64,10 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest get "/unknown_http_method", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body + + get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => true } + assert_response 406 + assert_equal "", body end test "localize rescue error page" do diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index 83b990b081..9548fe12c4 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -49,14 +49,14 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. def render_template(view, template, layout_name, locals) - render_with_layout(view, layout_name, template, locals) do |layout| + render_with_layout(view, template, layout_name, locals) do |layout| instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do template.render(view, locals) { |*name| view._layout_for(*name) } end end end - def render_with_layout(view, path, template, locals) + def render_with_layout(view, template, path, locals) layout = path && find_layout(path, locals.keys, [formats.first]) content = yield(layout) diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 1c577348e5..3b4c8a94bc 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -168,7 +168,12 @@ module ActionView DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}" def initialize(pattern = nil) - @pattern = pattern || DEFAULT_PATTERN + if pattern + ActiveSupport::Deprecation.warn "Specifying a custom path for #{self.class} is deprecated. Implement a custom Resolver subclass instead." + @pattern = pattern + else + @pattern = DEFAULT_PATTERN + end super() end @@ -273,44 +278,7 @@ module ActionView end end - # A resolver that loads files from the filesystem. It allows setting your own - # resolving pattern. Such pattern can be a glob string supported by some variables. - # - # ==== Examples - # - # Default pattern, loads views the same way as previous versions of rails, eg. when you're - # looking for <tt>users/new</tt> it will produce query glob: <tt>users/new{.{en},}{.{html,js},}{.{erb,haml},}</tt> - # - # FileSystemResolver.new("/path/to/views", ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}") - # - # This one allows you to keep files with different formats in separate subdirectories, - # eg. <tt>users/new.html</tt> will be loaded from <tt>users/html/new.erb</tt> or <tt>users/new.html.erb</tt>, - # <tt>users/new.js</tt> from <tt>users/js/new.erb</tt> or <tt>users/new.js.erb</tt>, etc. - # - # FileSystemResolver.new("/path/to/views", ":prefix/{:formats/,}:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}") - # - # If you don't specify a pattern then the default will be used. - # - # In order to use any of the customized resolvers above in a Rails application, you just need - # to configure ActionController::Base.view_paths in an initializer, for example: - # - # ActionController::Base.view_paths = FileSystemResolver.new( - # Rails.root.join("app/views"), - # ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}", - # ) - # - # ==== Pattern format and variables - # - # Pattern has to be a valid glob string, and it allows you to use the - # following variables: - # - # * <tt>:prefix</tt> - usually the controller path - # * <tt>:action</tt> - name of the action - # * <tt>:locale</tt> - possible locale versions - # * <tt>:formats</tt> - possible request formats (for example html, json, xml...) - # * <tt>:variants</tt> - possible request variants (for example phone, tablet...) - # * <tt>:handlers</tt> - possible handlers (for example erb, haml, builder...) - # + # A resolver that loads files from the filesystem. class FileSystemResolver < PathResolver def initialize(path, pattern = nil) raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver) @@ -331,6 +299,10 @@ module ActionView # An Optimized resolver for Rails' most common case. class OptimizedFileSystemResolver < FileSystemResolver #:nodoc: + def initialize(path) + super(path) + end + private def find_template_paths_from_details(path, details) diff --git a/actionview/test/actionpack/abstract/abstract_controller_test.rb b/actionview/test/actionpack/abstract/abstract_controller_test.rb index 4d4e2b8ef2..eecc19d413 100644 --- a/actionview/test/actionpack/abstract/abstract_controller_test.rb +++ b/actionview/test/actionpack/abstract/abstract_controller_test.rb @@ -229,11 +229,11 @@ module AbstractController end class ActionMissingRespondToActionController < AbstractController::Base - # No actions - private - def action_missing(action_name) - self.response_body = "success" - end + # No actions + private + def action_missing(action_name) + self.response_body = "success" + end end class RespondToActionController < AbstractController::Base diff --git a/actionview/test/template/resolver_patterns_test.rb b/actionview/test/template/resolver_patterns_test.rb index 8122de779f..22815c8dbe 100644 --- a/actionview/test/template/resolver_patterns_test.rb +++ b/actionview/test/template/resolver_patterns_test.rb @@ -6,7 +6,10 @@ class ResolverPatternsTest < ActiveSupport::TestCase def setup path = File.expand_path("../fixtures", __dir__) pattern = ":prefix/{:formats/,}:action{.:formats,}{+:variants,}{.:handlers,}" - @resolver = ActionView::FileSystemResolver.new(path, pattern) + + assert_deprecated do + @resolver = ActionView::FileSystemResolver.new(path, pattern) + end end def test_should_return_empty_list_for_unknown_path diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 8997579527..c7cd87f9d4 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -143,9 +143,7 @@ module ActiveRecord def preloaders_for_reflection(reflection, records, scope) records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs| - loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope) - loader.run self - loader + preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope).run end end @@ -166,10 +164,18 @@ module ActiveRecord @reflection = reflection end - def run(preloader); end + def run + self + end def preloaded_records - owners.flat_map { |owner| owner.association(reflection.name).target } + @preloaded_records ||= records_by_owner.flat_map(&:last) + end + + def records_by_owner + @records_by_owner ||= owners.each_with_object({}) do |owner, result| + result[owner] = Array(owner.association(reflection.name).target) + end end private diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 7048ff43b8..46532f651e 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -4,26 +4,44 @@ module ActiveRecord module Associations class Preloader class Association #:nodoc: - attr_reader :preloaded_records - def initialize(klass, owners, reflection, preload_scope) @klass = klass @owners = owners @reflection = reflection @preload_scope = preload_scope @model = owners.first && owners.first.class - @preloaded_records = [] end - def run(preloader) - records = load_records do |record| - owner = owners_by_key[convert_key(record[association_key_name])] - association = owner.association(reflection.name) - association.set_inverse_instance(record) + def run + if !preload_scope || preload_scope.empty_scope? + owners.each do |owner| + associate_records_to_owner(owner, records_by_owner[owner] || []) + end + else + # Custom preload scope is used and + # the association can not be marked as loaded + # Loading into a Hash instead + records_by_owner end + self + end - owners.each do |owner| - associate_records_to_owner(owner, records[convert_key(owner[owner_key_name])] || []) + def records_by_owner + @records_by_owner ||= preloaded_records.each_with_object({}) do |record, result| + owners_by_key[convert_key(record[association_key_name])].each do |owner| + (result[owner] ||= []) << record + end + end + end + + def preloaded_records + return @preloaded_records if defined?(@preloaded_records) + return [] if owner_keys.empty? + # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) + # Make several smaller queries if necessary or make one query if the adapter supports it + slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) + @preloaded_records = slices.flat_map do |slice| + records_for(slice) end end @@ -54,13 +72,10 @@ module ActiveRecord end def owners_by_key - unless defined?(@owners_by_key) - @owners_by_key = owners.each_with_object({}) do |owner, h| - key = convert_key(owner[owner_key_name]) - h[key] = owner if key - end + @owners_by_key ||= owners.each_with_object({}) do |owner, result| + key = convert_key(owner[owner_key_name]) + (result[key] ||= []) << owner if key end - @owners_by_key end def key_conversion_required? @@ -87,23 +102,16 @@ module ActiveRecord @model.type_for_attribute(owner_key_name).type end - def load_records(&block) - return {} if owner_keys.empty? - # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) - # Make several smaller queries if necessary or make one query if the adapter supports it - slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - @preloaded_records = slices.flat_map do |slice| - records_for(slice, &block) - end - @preloaded_records.group_by do |record| - convert_key(record[association_key_name]) + def records_for(ids) + scope.where(association_key_name => ids).load do |record| + # Processing only the first owner + # because the record is modified but not an owner + owner = owners_by_key[convert_key(record[association_key_name])].first + association = owner.association(reflection.name) + association.set_inverse_instance(record) end end - def records_for(ids, &block) - scope.where(association_key_name => ids).load(&block) - end - def scope @scope ||= build_scope end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index db89b77629..bec1c4c94a 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -4,45 +4,57 @@ module ActiveRecord module Associations class Preloader class ThroughAssociation < Association # :nodoc: - def run(preloader) - already_loaded = owners.first.association(through_reflection.name).loaded? - through_scope = through_scope() - through_preloaders = preloader.preload(owners, through_reflection.name, through_scope) - middle_records = through_preloaders.flat_map(&:preloaded_records) - preloaders = preloader.preload(middle_records, source_reflection.name, scope) - @preloaded_records = preloaders.flat_map(&:preloaded_records) - - owners.each do |owner| - through_records = Array(owner.association(through_reflection.name).target) - - if already_loaded + PRELOADER = ActiveRecord::Associations::Preloader.new + + def initialize(*) + super + @already_loaded = owners.first.association(through_reflection.name).loaded? + end + + def preloaded_records + @preloaded_records ||= source_preloaders.flat_map(&:preloaded_records) + end + + def records_by_owner + return @records_by_owner if defined?(@records_by_owner) + source_records_by_owner = source_preloaders.map(&:records_by_owner).reduce(:merge) + through_records_by_owner = through_preloaders.map(&:records_by_owner).reduce(:merge) + + @records_by_owner = owners.each_with_object({}) do |owner, result| + through_records = through_records_by_owner[owner] || [] + + if @already_loaded if source_type = reflection.options[:source_type] through_records = through_records.select do |record| record[reflection.foreign_type] == source_type end end - else - owner.association(through_reflection.name).reset if through_scope end - result = through_records.flat_map do |record| - record.association(source_reflection.name).target + records = through_records.flat_map do |record| + source_records_by_owner[record] end - result.compact! - result.sort_by! { |rhs| preload_index[rhs] } if scope.order_values.any? - result.uniq! if scope.distinct_value - associate_records_to_owner(owner, result) - end - - unless scope.empty_scope? - middle_records.each do |owner| - owner.association(source_reflection.name).reset if owner - end + records.compact! + records.sort_by! { |rhs| preload_index[rhs] } if scope.order_values.any? + records.uniq! if scope.distinct_value + result[owner] = records end end private + def source_preloaders + @source_preloaders ||= PRELOADER.preload(middle_records, source_reflection.name, scope) + end + + def middle_records + through_preloaders.flat_map(&:preloaded_records) + end + + def through_preloaders + @through_preloaders ||= PRELOADER.preload(owners, through_reflection.name, through_scope) + end + def through_reflection reflection.through_reflection end @@ -52,8 +64,8 @@ module ActiveRecord end def preload_index - @preload_index ||= @preloaded_records.each_with_object({}).with_index do |(id, result), index| - result[id] = index + @preload_index ||= preloaded_records.each_with_object({}).with_index do |(record, result), index| + result[record] = index end end @@ -96,7 +108,7 @@ module ActiveRecord end end - scope unless scope.empty_scope? + scope end end end diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb index a5b53f76b4..6ade2eec24 100644 --- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb @@ -200,7 +200,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase def test_doesnt_error_when_a_set_query_is_called_while_preventing_writes @conn.while_preventing_writes do - assert_nil @conn.execute("SET NAMES utf8") + assert_nil @conn.execute("SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci") end end diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index 0b83fd8421..35da74102d 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -548,6 +548,15 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end end + def test_through_association_preload_doesnt_reset_source_association_if_already_preloaded + blue = tags(:blue) + authors = Author.preload(posts: :first_blue_tags_2, misc_post_first_blue_tags_2: {}).to_a.sort_by(&:id) + + assert_no_queries do + assert_equal [blue], authors[2].posts.first.first_blue_tags_2 + end + end + def test_nested_has_many_through_with_conditions_on_source_associations_preload_via_joins # Pointless condition to force single-query loading assert_includes_and_joins_equal( diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 03c9dfc546..0551f0781f 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,8 @@ +* Use weak references in descendants tracker to allow anonymous subclasses to + be garbage collected. + + *Edgars Beigarts* + * Update `ActiveSupport::Notifications::Instrumenter#instrument` to make passing a block optional. This will let users use `ActiveSupport::Notifications` messaging features outside of diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index be041944f6..bf0fe0f76d 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -34,5 +34,5 @@ Gem::Specification.new do |s| s.add_dependency "tzinfo", "~> 1.1" s.add_dependency "minitest", "~> 5.1" s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2" - s.add_dependency "zeitwerk", "~> 1.4", ">= 1.4.2" + s.add_dependency "zeitwerk", "~> 1.4", ">= 1.4.3" end diff --git a/activesupport/lib/active_support/descendants_tracker.rb b/activesupport/lib/active_support/descendants_tracker.rb index 05236d3162..2dca990712 100644 --- a/activesupport/lib/active_support/descendants_tracker.rb +++ b/activesupport/lib/active_support/descendants_tracker.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "weakref" + module ActiveSupport # This module provides an internal implementation to track descendants # which is faster than iterating through ObjectSpace. @@ -8,7 +10,8 @@ module ActiveSupport class << self def direct_descendants(klass) - @@direct_descendants[klass] || [] + descendants = @@direct_descendants[klass] + descendants ? descendants.to_a : [] end def descendants(klass) @@ -34,15 +37,17 @@ module ActiveSupport # This is the only method that is not thread safe, but is only ever called # during the eager loading phase. def store_inherited(klass, descendant) - (@@direct_descendants[klass] ||= []) << descendant + (@@direct_descendants[klass] ||= DescendantsArray.new) << descendant end private def accumulate_descendants(klass, acc) if direct_descendants = @@direct_descendants[klass] - acc.concat(direct_descendants) - direct_descendants.each { |direct_descendant| accumulate_descendants(direct_descendant, acc) } + direct_descendants.each do |direct_descendant| + acc << direct_descendant + accumulate_descendants(direct_descendant, acc) + end end end end @@ -59,5 +64,46 @@ module ActiveSupport def descendants DescendantsTracker.descendants(self) end + + # DescendantsArray is an array that contains weak references to classes. + class DescendantsArray # :nodoc: + include Enumerable + + def initialize + @refs = [] + end + + def initialize_copy(orig) + @refs = @refs.dup + end + + def <<(klass) + cleanup! + @refs << WeakRef.new(klass) + end + + def each + @refs.each do |ref| + yield ref.__getobj__ + rescue WeakRef::RefError + end + end + + def refs_size + @refs.size + end + + def cleanup! + @refs.delete_if { |ref| !ref.weakref_alive? } + end + + def reject! + @refs.reject! do |ref| + yield ref.__getobj__ + rescue WeakRef::RefError + true + end + end + end end end diff --git a/activesupport/lib/active_support/security_utils.rb b/activesupport/lib/active_support/security_utils.rb index 20b6b9cd3f..5e455fca57 100644 --- a/activesupport/lib/active_support/security_utils.rb +++ b/activesupport/lib/active_support/security_utils.rb @@ -24,7 +24,7 @@ module ActiveSupport # The values are first processed by SHA256, so that we don't leak length info # via timing attacks. def secure_compare(a, b) - fixed_length_secure_compare(::Digest::SHA256.hexdigest(a), ::Digest::SHA256.hexdigest(b)) && a == b + fixed_length_secure_compare(::Digest::SHA256.digest(a), ::Digest::SHA256.digest(b)) && a == b end module_function :secure_compare end diff --git a/activesupport/test/descendants_tracker_test_cases.rb b/activesupport/test/descendants_tracker_test_cases.rb index 2c94c3c56c..f8752688d2 100644 --- a/activesupport/test/descendants_tracker_test_cases.rb +++ b/activesupport/test/descendants_tracker_test_cases.rb @@ -27,6 +27,15 @@ module DescendantsTrackerTestCases assert_equal_sets [], Child2.descendants end + def test_descendants_with_garbage_collected_classes + 1.times do + child_klass = Class.new(Parent) + assert_equal_sets [Child1, Grandchild1, Grandchild2, Child2, child_klass], Parent.descendants + end + GC.start + assert_equal_sets [Child1, Grandchild1, Grandchild2, Child2], Parent.descendants + end + def test_direct_descendants assert_equal_sets [Child1, Child2], Parent.direct_descendants assert_equal_sets [Grandchild1, Grandchild2], Child1.direct_descendants diff --git a/guides/source/6_0_release_notes.md b/guides/source/6_0_release_notes.md index 0cf9ca09c7..04a02259e9 100644 --- a/guides/source/6_0_release_notes.md +++ b/guides/source/6_0_release_notes.md @@ -138,7 +138,7 @@ Please refer to the [Changelog][railties] for detailed changes. the generators. ([Pull Request](https://github.com/rails/rails/pull/34021)) -* Add support for multi environment credentials=. +* Add support for multi environment credentials. ([Pull Request](https://github.com/rails/rails/pull/33521)) * Make `null_store` as default cache store in test environment. diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index be0bc495f7..50d4a4c57d 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -948,7 +948,7 @@ If `:ruby` is selected, then the schema is stored in `db/schema.rb`. If you look at this file you'll find that it looks an awful lot like one very big migration: ```ruby -ActiveRecord::Schema.define(version: 20080906171750) do +ActiveRecord::Schema.define(version: 2008_09_06_171750) do create_table "authors", force: true do |t| t.string "name" t.datetime "created_at" diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 9d026ad4fd..df0d6d4fa0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Applications running in `:zeitwerk` mode that use `bootsnap` need + to upgrade `bootsnap` to at least 1.4.2. + + *Xavier Noria* + * Add `config.disable_sandbox` option to Rails console. This setting will disable `rails console --sandbox` mode, preventing diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index 88cd502b53..d7221453e7 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.4.1', require: false +gem 'bootsnap', '>= 1.4.2', require: false <%- end -%> <%- if options.api? -%> |