diff options
61 files changed, 471 insertions, 216 deletions
@@ -46,8 +46,6 @@ gem "connection_pool", require: false # for railties app_generator_test gem "bootsnap", ">= 1.4.0", require: false -gem "zeitwerk", ">= 1.0.0" if RUBY_ENGINE == "ruby" - # Active Job group :job do gem "resque", require: false diff --git a/Gemfile.lock b/Gemfile.lock index fa8094bf23..b89c64d34e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -70,6 +70,7 @@ PATH i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) + zeitwerk (~> 1.0) rails (6.0.0.beta1) actioncable (= 6.0.0.beta1) actionmailbox (= 6.0.0.beta1) @@ -276,7 +277,7 @@ GEM hiredis (0.6.3-java) http_parser.rb (0.6.0) httpclient (2.8.3) - i18n (1.5.2) + i18n (1.5.3) concurrent-ruby (~> 1.0) image_processing (1.7.1) mini_magick (~> 4.0) @@ -589,7 +590,6 @@ DEPENDENCIES webmock webpacker (>= 4.0.0.rc.3) websocket-client-simple! - zeitwerk (>= 1.0.0) BUNDLED WITH 1.17.3 diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 3ce35edbb8..600d4e130f 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,3 +1,9 @@ +* PostgreSQL subscription adapters now support `channel_prefix` option in cable.yml + + Avoids channel name collisions when multiple apps use the same database for Action Cable. + + *Vladimir Dementyev* + * Allow passing custom configuration to `ActionCable::Server::Base`. You can now create a standalone Action Cable server with a custom configuration diff --git a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb index 50ec438c3a..1d60bed4af 100644 --- a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb +++ b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb @@ -8,6 +8,8 @@ require "digest/sha1" module ActionCable module SubscriptionAdapter class PostgreSQL < Base # :nodoc: + prepend ChannelPrefix + def initialize(*) super @listener = nil diff --git a/actioncable/test/subscription_adapter/postgresql_test.rb b/actioncable/test/subscription_adapter/postgresql_test.rb index 5fb26a8896..d262536d61 100644 --- a/actioncable/test/subscription_adapter/postgresql_test.rb +++ b/actioncable/test/subscription_adapter/postgresql_test.rb @@ -7,6 +7,7 @@ require "active_record" class PostgresqlAdapterTest < ActionCable::TestCase include CommonSubscriptionAdapterTest + include ChannelPrefixTest def setup database_config = { "adapter" => "postgresql", "database" => "activerecord_unittest" } diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 5610212fad..033a7f5b9e 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -593,6 +593,7 @@ module ActionMailer private def set_payload_for_mail(payload, mail) + payload[:mail] = mail.encoded payload[:mailer] = name payload[:message_id] = mail.message_id payload[:subject] = mail.subject @@ -601,7 +602,6 @@ module ActionMailer payload[:bcc] = mail.bcc if mail.bcc.present? payload[:cc] = mail.cc if mail.cc.present? payload[:date] = mail.date - payload[:mail] = mail.encoded payload[:perform_deliveries] = mail.perform_deliveries end diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb index 25c99342c2..26910f20f0 100644 --- a/actionmailer/lib/action_mailer/log_subscriber.rb +++ b/actionmailer/lib/action_mailer/log_subscriber.rb @@ -10,11 +10,10 @@ module ActionMailer def deliver(event) info do perform_deliveries = event.payload[:perform_deliveries] - recipients = Array(event.payload[:to]).join(", ") if perform_deliveries - "Sent mail to #{recipients} (#{event.duration.round(1)}ms)" + "Delivered mail #{event.payload[:message_id]} (#{event.duration.round(1)}ms)" else - "Skipped sending mail to #{recipients} as `perform_deliveries` is false" + "Skipped delivery of mail #{event.payload[:message_id]} as `perform_deliveries` is false" end end diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index d0c4f189fd..15613d4dce 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -913,6 +913,23 @@ class BaseTest < ActiveSupport::TestCase ActiveSupport::Notifications.unsubscribe "process.action_mailer" end + test "notification for deliver" do + begin + events = [] + ActiveSupport::Notifications.subscribe("deliver.action_mailer") do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + + BaseMailer.welcome(body: "Hello there").deliver_now + + assert_equal 1, events.length + assert_equal "deliver.action_mailer", events[0].name + assert_not_nil events[0].payload[:message_id] + ensure + ActiveSupport::Notifications.unsubscribe "deliver.action_mailer" + end + end + private # Execute the block setting the given values and restoring old values after diff --git a/actionmailer/test/log_subscriber_test.rb b/actionmailer/test/log_subscriber_test.rb index fb569ce45f..f09f1997c0 100644 --- a/actionmailer/test/log_subscriber_test.rb +++ b/actionmailer/test/log_subscriber_test.rb @@ -24,11 +24,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase end def test_deliver_is_notified - BaseMailer.welcome.deliver_now + BaseMailer.welcome(message_id: "123@abc").deliver_now wait assert_equal(1, @logger.logged(:info).size) - assert_match(/Sent mail to system@test\.lindsaar\.net/, @logger.logged(:info).first) + assert_match(/Delivered mail 123@abc/, @logger.logged(:info).first) assert_equal(2, @logger.logged(:debug).size) assert_match(/BaseMailer#welcome: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first) @@ -38,11 +38,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase end def test_deliver_message_when_perform_deliveries_is_false - BaseMailer.welcome_without_deliveries.deliver_now + BaseMailer.welcome_without_deliveries(message_id: "123@abc").deliver_now wait assert_equal(1, @logger.logged(:info).size) - assert_match("Skipped sending mail to system@test.lindsaar.net as `perform_deliveries` is false", @logger.logged(:info).first) + assert_match("Skipped delivery of mail 123@abc as `perform_deliveries` is false", @logger.logged(:info).first) assert_equal(2, @logger.logged(:debug).size) assert_match(/BaseMailer#welcome_without_deliveries: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first) diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb index c1bb48cc96..dbe1c4f0e6 100644 --- a/actionmailer/test/mailers/base_mailer.rb +++ b/actionmailer/test/mailers/base_mailer.rb @@ -21,8 +21,8 @@ class BaseMailer < ActionMailer::Base mail(template_name: "welcome", template_path: path) end - def welcome_without_deliveries - mail(template_name: "welcome") + def welcome_without_deliveries(hash = {}) + mail({ template_name: "welcome" }.merge!(hash)) mail.perform_deliveries = false end diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index f16484d1ea..43c0a84504 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -11,8 +11,8 @@ module ActionDispatch def initialize(assigns) paths = [RESCUES_TEMPLATE_PATH] - renderer = ActionView::Renderer.new ActionView::LookupContext.new(paths) - super(renderer, assigns) + lookup_context = ActionView::LookupContext.new(paths) + super(lookup_context, assigns) end def compiled_method_container diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 420136d6de..712e5d251e 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -196,10 +196,9 @@ module ActionView #:nodoc: end end - attr_reader :view_renderer + attr_reader :view_renderer, :lookup_context attr_internal :config, :assigns - delegate :lookup_context, to: :view_renderer delegate :formats, :formats=, :locale, :locale=, :view_paths, :view_paths=, to: :lookup_context def assign(new_assigns) # :nodoc: @@ -208,12 +207,17 @@ module ActionView #:nodoc: # :stopdoc: - def self.build_renderer(context, controller, formats) - lookup_context = context.is_a?(ActionView::LookupContext) ? - context : ActionView::LookupContext.new(context) - lookup_context.formats = formats if formats - lookup_context.prefixes = controller._prefixes if controller - ActionView::Renderer.new(lookup_context) + def self.build_lookup_context(context) + case context + when ActionView::Renderer + context.lookup_context + when Array + ActionView::LookupContext.new(context) + when nil + ActionView::LookupContext.new([]) + else + raise NotImplementedError, context.class.name + end end def self.empty @@ -225,34 +229,35 @@ module ActionView #:nodoc: end def self.with_context(context, assigns = {}, controller = nil) - new ActionView::Renderer.new(context), assigns, controller + new context, assigns, controller end NULL = Object.new # :startdoc: - def initialize(renderer = nil, assigns = {}, controller = nil, formats = NULL) #:nodoc: + def initialize(lookup_context = nil, assigns = {}, controller = nil, formats = NULL) #:nodoc: @_config = ActiveSupport::InheritableOptions.new - if formats == NULL - formats = nil - else + unless formats == NULL ActiveSupport::Deprecation.warn <<~eowarn Passing formats to ActionView::Base.new is deprecated eowarn end - if renderer.is_a?(ActionView::Renderer) - @view_renderer = renderer + case lookup_context + when ActionView::LookupContext + @lookup_context = lookup_context else ActiveSupport::Deprecation.warn <<~eowarn - ActionView::Base instances should be constructed with a view renderer, + ActionView::Base instances should be constructed with a lookup context, assigments, and a controller. eowarn - @view_renderer = self.class.build_renderer(renderer, controller, formats) + @lookup_context = self.class.build_lookup_context(lookup_context) end + @view_renderer = ActionView::Renderer.new @lookup_context + @cache_hit = {} assign(assigns) assign_controller(controller) @@ -275,6 +280,25 @@ module ActionView #:nodoc: msg end + def in_context(options, locals) + old_view_renderer = @view_renderer + old_lookup_context = @lookup_context + + if !lookup_context.html_fallback_for_js && options[:formats] + formats = Array(options[:formats]) + if formats == [:js] + formats << :html + end + @lookup_context = lookup_context.with_prepended_formats(formats) + @view_renderer = ActionView::Renderer.new @lookup_context + end + + yield @view_renderer + ensure + @view_renderer = old_view_renderer + @lookup_context = old_lookup_context + end + ActiveSupport.run_load_hooks(:action_view, self) end end diff --git a/actionview/lib/action_view/helpers/rendering_helper.rb b/actionview/lib/action_view/helpers/rendering_helper.rb index 1e12aa2736..7323963c72 100644 --- a/actionview/lib/action_view/helpers/rendering_helper.rb +++ b/actionview/lib/action_view/helpers/rendering_helper.rb @@ -27,10 +27,12 @@ module ActionView def render(options = {}, locals = {}, &block) case options when Hash - if block_given? - view_renderer.render_partial(self, options.merge(partial: options[:layout]), &block) - else - view_renderer.render(self, options) + in_context(options, locals) do |renderer| + if block_given? + view_renderer.render_partial(self, options.merge(partial: options[:layout]), &block) + else + view_renderer.render(self, options) + end end else view_renderer.render_partial(self, partial: options, locals: locals, &block) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 61fe11cf45..125ab4dbe3 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -260,6 +260,13 @@ module ActionView @digest_cache ||= DetailsKey.digest_cache(@details) end + def with_prepended_formats(formats) + details = @details.dup + details[:formats] = formats + + self.class.new(@view_paths, details, @prefixes) + end + def initialize_details(target, details) registered_details.each do |k| target[k] = details[k] || Accessors::DEFAULT_PROCS[k].call diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index 801916954f..f8a6f13ae9 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -308,6 +308,9 @@ module ActionView template = find_partial(@path, @template_keys) @variable ||= template.variable else + if options[:cached] + raise NotImplementedError, "render caching requires a template. Please specify a partial when rendering" + end template = nil end @@ -337,9 +340,14 @@ module ActionView spacer = find_template(@options[:spacer_template], @locals.keys).render(view, @locals) end - cache_collection_render(payload, view, template) do - template ? collection_with_template(view, template) : collection_without_template(view) - end.join(spacer).html_safe + collection_body = if template + cache_collection_render(payload, view, template) do + collection_with_template(view, template) + end + else + collection_without_template(view) + end + collection_body.join(spacer).html_safe end end @@ -379,8 +387,6 @@ module ActionView @locals = options[:locals] || {} @details = extract_details(options) - prepend_formats(options[:formats]) - partial = options[:partial] if String === partial diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index da92ce1f5e..b798e80b04 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -82,7 +82,7 @@ module ActionView # # Override this method in a module to change the default behavior. def view_context - view_context_class.new(view_renderer, view_assigns, self) + view_context_class.new(lookup_context, view_assigns, self) end # Returns an object that is able to render templates. @@ -112,7 +112,7 @@ module ActionView lookup_context.rendered_format = nil if options[:formats] lookup_context.variants = variant if variant - view_renderer.render(context, options) + context.view_renderer.render(context, options) end # Assign the rendered format to look up context. diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index 15ca202024..20510c3062 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -26,7 +26,7 @@ module ActionView view = Class.new(ActionView::Base) { include action_view_erb_handler_context._routes.url_helpers class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0) - }.with_context(action_view_erb_handler_context) + }.empty view.run(:_template, {}, ActionView::OutputBuffer.new) end diff --git a/actionview/test/fixtures/test/_first.html.erb b/actionview/test/fixtures/test/_first.html.erb new file mode 100644 index 0000000000..2e2c825acb --- /dev/null +++ b/actionview/test/fixtures/test/_first.html.erb @@ -0,0 +1 @@ +"HTML" diff --git a/actionview/test/fixtures/test/_first.xml.erb b/actionview/test/fixtures/test/_first.xml.erb new file mode 100644 index 0000000000..9cf4f4ec85 --- /dev/null +++ b/actionview/test/fixtures/test/_first.xml.erb @@ -0,0 +1 @@ +"XML" diff --git a/actionview/test/fixtures/test/_first_layer.html.erb b/actionview/test/fixtures/test/_first_layer.html.erb new file mode 100644 index 0000000000..c1f1acb410 --- /dev/null +++ b/actionview/test/fixtures/test/_first_layer.html.erb @@ -0,0 +1,4 @@ +{"format":"HTML", "children": +[ + <%= render(partial: "first").chomp.html_safe %>, +]} diff --git a/actionview/test/fixtures/test/_first_layer.xml.erb b/actionview/test/fixtures/test/_first_layer.xml.erb new file mode 100644 index 0000000000..b8581bbbfc --- /dev/null +++ b/actionview/test/fixtures/test/_first_layer.xml.erb @@ -0,0 +1,4 @@ +{"format":"XML", "children": +[ + <%= render(partial: "first").chomp.html_safe %> +]} diff --git a/actionview/test/fixtures/test/_second.html.erb b/actionview/test/fixtures/test/_second.html.erb new file mode 100644 index 0000000000..2e2c825acb --- /dev/null +++ b/actionview/test/fixtures/test/_second.html.erb @@ -0,0 +1 @@ +"HTML" diff --git a/actionview/test/fixtures/test/_second.xml.erb b/actionview/test/fixtures/test/_second.xml.erb new file mode 100644 index 0000000000..9cf4f4ec85 --- /dev/null +++ b/actionview/test/fixtures/test/_second.xml.erb @@ -0,0 +1 @@ +"XML" diff --git a/actionview/test/fixtures/test/_second_layer.html.erb b/actionview/test/fixtures/test/_second_layer.html.erb new file mode 100644 index 0000000000..307706abd2 --- /dev/null +++ b/actionview/test/fixtures/test/_second_layer.html.erb @@ -0,0 +1,4 @@ +{"format":"HTML", "children": +[ + <%= render(partial: "first").chomp.html_safe %> +]} diff --git a/actionview/test/fixtures/test/_second_layer.xml.erb b/actionview/test/fixtures/test/_second_layer.xml.erb new file mode 100644 index 0000000000..b8581bbbfc --- /dev/null +++ b/actionview/test/fixtures/test/_second_layer.xml.erb @@ -0,0 +1,4 @@ +{"format":"XML", "children": +[ + <%= render(partial: "first").chomp.html_safe %> +]} diff --git a/actionview/test/fixtures/test/mixing_formats.html.erb b/actionview/test/fixtures/test/mixing_formats.html.erb new file mode 100644 index 0000000000..c65cdd7dd4 --- /dev/null +++ b/actionview/test/fixtures/test/mixing_formats.html.erb @@ -0,0 +1,5 @@ +{"format":"HTML", "children": +[ + <%= render(partial: "first", formats: :xml).chomp.html_safe %>, + <%= render(partial: "second").chomp.html_safe %> +]} diff --git a/actionview/test/fixtures/test/mixing_formats_deep.html.erb b/actionview/test/fixtures/test/mixing_formats_deep.html.erb new file mode 100644 index 0000000000..e328887eeb --- /dev/null +++ b/actionview/test/fixtures/test/mixing_formats_deep.html.erb @@ -0,0 +1,5 @@ +{"format":"HTML", "children": +[ + <%= render(partial: "first_layer", formats: :xml).chomp.html_safe %>, + <%= render(partial: "second_layer").chomp.html_safe %> +]} diff --git a/actionview/test/template/csp_helper_test.rb b/actionview/test/template/csp_helper_test.rb new file mode 100644 index 0000000000..8bad25ba7d --- /dev/null +++ b/actionview/test/template/csp_helper_test.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class CspHelperWithCspEnabledTest < ActionView::TestCase + tests ActionView::Helpers::CspHelper + + def content_security_policy_nonce + "iyhD0Yc0W+c=" + end + + def content_security_policy? + true + end + + def test_csp_meta_tag + assert_equal "<meta name=\"csp-nonce\" content=\"iyhD0Yc0W+c=\" />", csp_meta_tag + end +end + +class CspHelperWithCspDisabledTest < ActionView::TestCase + tests ActionView::Helpers::CspHelper + + def content_security_policy? + false + end + + def test_csp_meta_tag + assert_nil csp_meta_tag + end +end diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 83bb651ea3..85735139c1 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -16,8 +16,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase view_paths = ActionController::Base.view_paths lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"]) - renderer = ActionView::Renderer.new(lookup_context) - @view = ActionView::Base.with_empty_template_cache.new(renderer, {}) + @view = ActionView::Base.with_empty_template_cache.new(lookup_context, {}) ActionView::LogSubscriber.attach_to :action_view diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 3f298d81f3..cda8c942d8 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -20,7 +20,10 @@ module RenderTestCases controller = TestController.new - @controller_view = controller.view_context_class.with_empty_template_cache.new(controller.view_renderer, controller.view_assigns, controller) + @controller_view = controller.view_context_class.with_empty_template_cache.new( + controller.lookup_context, + controller.view_assigns, + controller) # Reload and register danish language for testing I18n.backend.store_translations "da", {} @@ -30,6 +33,21 @@ module RenderTestCases assert_equal ORIGINAL_LOCALES, I18n.available_locales.map(&:to_s).sort end + def test_implicit_format_comes_from_parent_template + rendered_templates = JSON.parse(@controller_view.render(template: "test/mixing_formats")) + assert_equal({ "format" => "HTML", + "children" => ["XML", "HTML"] }, rendered_templates) + end + + def test_implicit_format_comes_from_parent_template_cascading + rendered_templates = JSON.parse(@controller_view.render(template: "test/mixing_formats_deep")) + assert_equal({ "format" => "HTML", + "children" => [ + { "format" => "XML", "children" => ["XML"] }, + { "format" => "HTML", "children" => ["HTML"] }, + ] }, rendered_templates) + end + def test_render_without_options e = assert_raises(ArgumentError) { @view.render() } assert_match(/You invoked render but did not give any of (.+) option\./, e.message) @@ -68,7 +86,7 @@ module RenderTestCases def test_render_partial_use_last_prepended_format_for_partials_with_the_same_names @view.lookup_context.formats = [:html] - assert_equal "\nHTML Template, but JSON partial", @view.render(template: "test/change_priority") + assert_equal "\nHTML Template, but HTML partial", @view.render(template: "test/change_priority") end def test_render_template_with_a_missing_partial_of_another_format @@ -752,6 +770,17 @@ class CachedCollectionViewRenderTest < ActiveSupport::TestCase @view.render(partial: "test/cached_customer", collection: [customer], cached: true) end + test "collection caching does not work on multi-partials" do + a = Object.new + b = Object.new + def a.to_partial_path; "test/partial_iteration_1"; end + def b.to_partial_path; "test/partial_iteration_2"; end + + assert_raises(NotImplementedError) do + @controller_view.render(partial: [a, b], cached: true) + end + end + private def cache_key(*names, virtual_path) digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: [] diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index 487cdd6d38..2ce008e3da 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -40,6 +40,9 @@ module ActiveJob # Timezone to be used during the job. attr_accessor :timezone + # Track when a job was enqueded + attr_accessor :enqueued_at + # These methods will be included into any Active Job object, adding # helpers for de/serialization and creation of job instances. module ClassMethods @@ -97,7 +100,8 @@ module ActiveJob "executions" => executions, "exception_executions" => exception_executions, "locale" => I18n.locale.to_s, - "timezone" => Time.zone.try(:name) + "timezone" => Time.zone.try(:name), + "enqueued_at" => Time.now.utc.iso8601 } end @@ -137,6 +141,7 @@ module ActiveJob self.exception_executions = job_data["exception_executions"] self.locale = job_data["locale"] || I18n.locale.to_s self.timezone = job_data["timezone"] || Time.zone.try(:name) + self.enqueued_at = job_data["enqueued_at"] end private diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 416be83c24..1134e718a8 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -70,7 +70,7 @@ module ActiveJob def perform_start(event) info do job = event.payload[:job] - "Performing #{job.class.name} (Job ID: #{job.job_id}) from #{queue_name(event)}" + args_info(job) + "Performing #{job.class.name} (Job ID: #{job.job_id}) from #{queue_name(event)} enqueued at #{job.enqueued_at}" + args_info(job) end end diff --git a/activejob/test/cases/job_serialization_test.rb b/activejob/test/cases/job_serialization_test.rb index 86f3651564..c1cec1f1d6 100644 --- a/activejob/test/cases/job_serialization_test.rb +++ b/activejob/test/cases/job_serialization_test.rb @@ -61,4 +61,15 @@ class JobSerializationTest < ActiveSupport::TestCase assert_equal "Hawaii", job.serialize["timezone"] end end + + test "serialize stores the enqueued_at time" do + h1 = HelloJob.new + type = h1.serialize["enqueued_at"].class + assert_equal String, type + + h2 = HelloJob.deserialize(h1.serialize) + # We should be able to parse a timestamp + type = Time.parse(h2.enqueued_at).class + assert_equal Time, type + end end diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb index 6154ba301d..acd37456c9 100644 --- a/activejob/test/cases/logging_test.rb +++ b/activejob/test/cases/logging_test.rb @@ -115,6 +115,8 @@ class LoggingTest < ActiveSupport::TestCase perform_enqueued_jobs do LoggingJob.perform_later "Dummy" assert_match(/Performing LoggingJob \(Job ID: .*?\) from .*? with arguments:.*Dummy/, @logger.messages) + + assert_match(/enqueued at /, @logger.messages) assert_match(/Dummy, here is it: Dummy/, @logger.messages) assert_match(/Performed LoggingJob \(Job ID: .*?\) from .*? in .*ms/, @logger.messages) end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f16a746b91..47ae71f2a0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -4,12 +4,6 @@ *Ryuta Kamizono* -* Chaining named scope is no longer leaking to class level querying methods. - - Fixes #14003. - - *Ryuta Kamizono* - * Allow applications to automatically switch connections. Adds a middleware and configuration options that can be used in your @@ -508,8 +502,8 @@ Iterating over the database configurations has also changed. Instead of calling hash methods on the `configurations` hash directly, a new method `configs_for` has - been provided that allows you to select the correct configuration. `env_name`, and - `spec_name` arguments are optional. For example these return an array of + been provided that allows you to select the correct configuration. `env_name` and + `spec_name` arguments are optional. For example, these return an array of database config objects for the requested environment and a single database config object will be returned for the requested environment and specification name respectively. diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 9824787658..90921dec8b 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -65,11 +65,42 @@ end task adapter => "#{adapter}:env" do adapter_short = adapter == "db2" ? adapter : adapter[/^[a-z0-9]+/] puts [adapter, adapter_short].inspect - (Dir["test/cases/**/*_test.rb"].reject { + + failing_files = [] + + test_files = (Dir["test/cases/**/*_test.rb"].reject { |x| x.include?("/adapters/") - } + Dir["test/cases/adapters/#{adapter_short}/**/*_test.rb"]).all? do |file| - sh(Gem.ruby, "-w", "-Itest", file) - end || raise("Failures") + } + Dir["test/cases/adapters/#{adapter_short}/**/*_test.rb"]).sort + + if ENV["BUILDKITE_PARALLEL_JOB_COUNT"] + n = ENV["BUILDKITE_PARALLEL_JOB"].to_i + m = ENV["BUILDKITE_PARALLEL_JOB_COUNT"].to_i + + test_files = test_files.each_slice(m).map { |slice| slice[n] }.compact + end + + test_files.each do |file| + puts "--- #{file}" + success = sh(Gem.ruby, "-w", "-Itest", file) + unless success + failing_files << file + puts "^^^ +++" + end + puts + end + + puts "--- All tests completed" + unless failing_files.empty? + puts "^^^ +++" + puts + puts "Failed in:" + failing_files.each do |file| + puts " #{file}" + end + puts + + exit 1 + end end end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index b0c0beac0e..c3d4eab562 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -209,8 +209,7 @@ module ActiveRecord # This method is abstract in the sense that it relies on # +count_records+, which is a method descendants have to provide. def size - if !find_target? - loaded! unless loaded? + if !find_target? || loaded? target.size elsif @association_ids @association_ids.size diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 6f67934a79..5972846940 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -52,7 +52,11 @@ module ActiveRecord # If the collection is empty the target is set to an empty array and # the loaded flag is set to true as well. def count_records - count = counter_cache_value || scope.count(:all) + count = if reflection.has_cached_counter? + owner._read_attribute(reflection.counter_cache_column).to_i + else + scope.count(:all) + end # If there's nothing in the database and @target has no new records # we are certain the current target is an empty array. This is a @@ -62,14 +66,6 @@ module ActiveRecord [association_scope.limit_value, count].compact.min end - def counter_cache_value - reflection.has_cached_counter? ? owner._read_attribute(reflection.counter_cache_column).to_i : nil - end - - def find_target? - super && !counter_cache_value&.zero? - end - def update_counter(difference, reflection = reflection()) if reflection.has_cached_counter? owner.increment!(reflection.counter_cache_column, difference) diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 73adf66684..a6c702cbbc 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -25,9 +25,9 @@ module ActiveRecord # # Options: # - # <tt>env_name:</tt> The environment name. Defaults to nil which will collect + # <tt>env_name:</tt> The environment name. Defaults to +nil+ which will collect # configs for all environments. - # <tt>spec_name:</tt> The specification name (ie primary, animals, etc.). Defaults + # <tt>spec_name:</tt> The specification name (i.e. primary, animals, etc.). Defaults # to +nil+. # <tt>include_replicas:</tt> Determines whether to include replicas in # the returned list. Most of the time we're only iterating over the write @@ -102,6 +102,7 @@ module ActiveRecord def build_configs(configs) return configs.configurations if configs.is_a?(DatabaseConfigurations) + return configs if configs.is_a?(Array) build_db_config = configs.each_pair.flat_map do |env_name, config| walk_configs(env_name.to_s, "primary", config) @@ -157,7 +158,7 @@ module ActiveRecord configs else configs.map do |config| - ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, config.spec_name, url, config.config) + ActiveRecord::DatabaseConfigurations::UrlConfig.new(config.env_name, config.spec_name, url, config.config) end end else @@ -166,21 +167,38 @@ module ActiveRecord end def method_missing(method, *args, &blk) - if Hash.method_defined?(method) - ActiveSupport::Deprecation.warn \ - "Returning a hash from ActiveRecord::Base.configurations is deprecated. Therefore calling `#{method}` on the hash is also deprecated. Please switch to using the `configs_for` method instead to collect and iterate over database configurations." - end - case method when :each, :first + throw_getter_deprecation(method) configurations.send(method, *args, &blk) when :fetch + throw_getter_deprecation(method) configs_for(env_name: args.first) when :values + throw_getter_deprecation(method) configurations.map(&:config) + when :[]= + throw_setter_deprecation(method) + + env_name = args[0] + config = args[1] + + remaining_configs = configurations.reject { |db_config| db_config.env_name == env_name } + new_config = build_configs(env_name => config) + new_configs = remaining_configs + new_config + + ActiveRecord::Base.configurations = new_configs else - super + raise NotImplementedError, "`ActiveRecord::Base.configurations` in Rails 6 now returns an object instead of a hash. The `#{method}` method is not supported. Please use `configs_for` or consult the documentation for supported methods." end end + + def throw_setter_deprecation(method) + ActiveSupport::Deprecation.warn("Setting `ActiveRecord::Base.configurations` with `#{method}` is deprecated. Use `ActiveRecord::Base.configurations=` directly to set the configurations instead.") + end + + def throw_getter_deprecation(method) + ActiveSupport::Deprecation.warn("`ActiveRecord::Base.configurations` no longer returns a hash. Methods that act on the hash like `#{method}` are deprecated and will be removed in Rails 6.1. Use the `configs_for` method to collect and iterate over the database configurations.") + end end end diff --git a/activerecord/lib/active_record/database_configurations/hash_config.rb b/activerecord/lib/active_record/database_configurations/hash_config.rb index c176a62458..574cb6bf15 100644 --- a/activerecord/lib/active_record/database_configurations/hash_config.rb +++ b/activerecord/lib/active_record/database_configurations/hash_config.rb @@ -16,7 +16,7 @@ module ActiveRecord # # Options are: # - # <tt>:env_name</tt> - The Rails environment, ie "development" + # <tt>:env_name</tt> - The Rails environment, i.e. "development" # <tt>:spec_name</tt> - The specification name. In a standard two-tier # database configuration this will default to "primary". In a multiple # database three-tier database configuration this corresponds to the name diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 347d745d19..f98d9bb2c0 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -321,12 +321,12 @@ module ActiveRecord # Please check unscoped if you want to remove all previous scopes (including # the default_scope) during the execution of a block. def scoping - @delegate_to_klass && klass.current_scope(true) ? yield : _scoping(self) { yield } + @delegate_to_klass ? yield : _scoping(self) { yield } end def _exec_scope(*args, &block) # :nodoc: @delegate_to_klass = true - _scoping(nil) { instance_exec(*args, &block) || self } + instance_exec(*args, &block) || self ensure @delegate_to_klass = false end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 8cf4fc469f..7874c4c35a 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -8,7 +8,7 @@ module ActiveRecord module SpawnMethods # This is overridden by Associations::CollectionProxy def spawn #:nodoc: - @delegate_to_klass && klass.current_scope(true) ? klass.all : clone + @delegate_to_klass ? klass.all : clone end # Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation. diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index bf44be1811..7d669198ca 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1230,10 +1230,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_not_predicate Ship.reflect_on_association(:treasures), :has_cached_counter? # Count should come from sql count() of treasures rather than treasures_count attribute - assert_queries(1) do - assert_equal ship.treasures.size, 0 - assert_predicate ship.treasures, :loaded? - end + assert_equal ship.treasures.size, 0 assert_no_difference lambda { ship.reload.treasures_count }, "treasures_count should not be changed" do ship.treasures.create(name: "Gold") @@ -1354,20 +1351,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase post = posts(:welcome) assert_no_queries do assert_not_empty post.comments - assert_equal 2, post.comments.size - assert_not_predicate post.comments, :loaded? - end - post = posts(:misc_by_bob) - assert_no_queries do - assert_empty post.comments - assert_predicate post.comments, :loaded? - end - end - - def test_empty_association_loading_with_counter_cache - post = posts(:misc_by_bob) - assert_no_queries do - assert_empty post.comments.to_a end end @@ -1755,6 +1738,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_nil posts.first end + def test_destroy_all_on_desynced_counter_cache_association + category = categories(:general) + assert_operator category.categorizations.count, :>, 0 + + category.categorizations.destroy_all + assert_equal 0, category.categorizations.count + end + def test_destroy_on_association_clears_scope author = Author.create!(name: "Gannon") posts = author.posts @@ -2004,6 +1995,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_not_predicate company.clients, :loaded? end + def test_counter_cache_on_unloaded_association + car = Car.create(name: "My AppliCar") + assert_equal 0, car.engines.size + end + def test_ids_reader_cache_not_used_for_size_when_association_is_dirty firm = Firm.create!(name: "Startup") assert_equal 0, firm.client_ids.size @@ -2019,24 +2015,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [3, 11], firm.client_ids end - def test_zero_counter_cache_usage_on_unloaded_association - car = Car.create!(name: "My AppliCar") - assert_no_queries do - assert_equal car.engines.size, 0 - assert_predicate car.engines, :loaded? - end - end - - def test_counter_cache_on_new_record_unloaded_association - car = Car.new(name: "My AppliCar") - # Ensure no schema queries inside assertion - Engine.primary_key - assert_no_queries do - assert_equal car.engines.size, 0 - assert_predicate car.engines, :loaded? - end - end - def test_get_ids_ignores_include_option assert_equal [readers(:michael_welcome).id], posts(:welcome).readers_with_person_ids end diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 225cccc62c..515bf5df06 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -72,6 +72,16 @@ module ActiveRecord assert_equal expected, actual end + def test_resolver_with_database_uri_and_multiple_envs + ENV["DATABASE_URL"] = "postgres://localhost" + ENV["RAILS_ENV"] = "test" + + config = { "production" => { "adapter" => "postgresql", "database" => "foo_prod" }, "test" => { "adapter" => "postgresql", "database" => "foo_test" } } + actual = resolve_spec(:test, config) + expected = { "adapter" => "postgresql", "database" => "foo_test", "host" => "localhost", "name" => "test" } + assert_equal expected, actual + end + def test_resolver_with_database_uri_and_unknown_symbol_key ENV["DATABASE_URL"] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } diff --git a/activerecord/test/cases/database_configurations_test.rb b/activerecord/test/cases/database_configurations_test.rb new file mode 100644 index 0000000000..ed8151f01a --- /dev/null +++ b/activerecord/test/cases/database_configurations_test.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require "cases/helper" + +class DatabaseConfigurationsTest < ActiveRecord::TestCase + unless in_memory_db? + def test_empty_returns_true_when_db_configs_are_empty + old_config = ActiveRecord::Base.configurations + config = {} + + ActiveRecord::Base.configurations = config + + assert_predicate ActiveRecord::Base.configurations, :empty? + assert_predicate ActiveRecord::Base.configurations, :blank? + ensure + ActiveRecord::Base.configurations = old_config + ActiveRecord::Base.establish_connection :arunit + end + end + + def test_configs_for_getter_with_env_name + configs = ActiveRecord::Base.configurations.configs_for(env_name: "arunit") + + assert_equal 1, configs.size + assert_equal ["arunit"], configs.map(&:env_name) + end + + def test_configs_for_getter_with_env_and_spec_name + config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary") + + assert_equal "arunit", config.env_name + assert_equal "primary", config.spec_name + end + + def test_default_hash_returns_config_hash_from_default_env + original_rails_env = ENV["RAILS_ENV"] + ENV["RAILS_ENV"] = "arunit" + + assert_equal ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary").config, ActiveRecord::Base.configurations.default_hash + ensure + ENV["RAILS_ENV"] = original_rails_env + end + + def test_find_db_config_returns_a_db_config_object_for_the_given_env + config = ActiveRecord::Base.configurations.find_db_config("arunit2") + + assert_equal "arunit2", config.env_name + assert_equal "primary", config.spec_name + end + + def test_to_h_turns_db_config_object_back_into_a_hash + configs = ActiveRecord::Base.configurations + assert_equal "ActiveRecord::DatabaseConfigurations", configs.class.name + assert_equal "Hash", configs.to_h.class.name + assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"], ActiveRecord::Base.configurations.to_h.keys.sort + end +end + +class LegacyDatabaseConfigurationsTest < ActiveRecord::TestCase + unless in_memory_db? + def test_setting_configurations_hash + old_config = ActiveRecord::Base.configurations + config = { "adapter" => "sqlite3" } + + assert_deprecated do + ActiveRecord::Base.configurations["readonly"] = config + end + + assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements", "readonly"], ActiveRecord::Base.configurations.configs_for.map(&:env_name).sort + ensure + ActiveRecord::Base.configurations = old_config + ActiveRecord::Base.establish_connection :arunit + end + end + + def test_can_turn_configurations_into_a_hash + assert ActiveRecord::Base.configurations.to_h.is_a?(Hash), "expected to be a hash but was not." + assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"].sort, ActiveRecord::Base.configurations.to_h.keys.sort + end + + def test_each_is_deprecated + assert_deprecated do + ActiveRecord::Base.configurations.each do |db_config| + assert_equal "primary", db_config.spec_name + end + end + end + + def test_first_is_deprecated + assert_deprecated do + db_config = ActiveRecord::Base.configurations.first + assert_equal "arunit", db_config.env_name + assert_equal "primary", db_config.spec_name + end + end + + def test_fetch_is_deprecated + assert_deprecated do + db_config = ActiveRecord::Base.configurations.fetch("arunit").first + assert_equal "arunit", db_config.env_name + assert_equal "primary", db_config.spec_name + end + end + + def test_values_are_deprecated + config_hashes = ActiveRecord::Base.configurations.configurations.map(&:config) + assert_deprecated do + assert_equal config_hashes, ActiveRecord::Base.configurations.values + end + end + + def test_unsupported_method_raises + assert_raises NotImplementedError do + ActiveRecord::Base.configurations.select { |a| a == "foo" } + end + end +end diff --git a/activerecord/test/cases/legacy_configurations_test.rb b/activerecord/test/cases/legacy_configurations_test.rb deleted file mode 100644 index c36feb5116..0000000000 --- a/activerecord/test/cases/legacy_configurations_test.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" - -module ActiveRecord - class LegacyConfigurationsTest < ActiveRecord::TestCase - def test_can_turn_configurations_into_a_hash - assert ActiveRecord::Base.configurations.to_h.is_a?(Hash), "expected to be a hash but was not." - assert_equal ["arunit", "arunit2", "arunit_without_prepared_statements"].sort, ActiveRecord::Base.configurations.to_h.keys.sort - end - - def test_each_is_deprecated - assert_deprecated do - ActiveRecord::Base.configurations.each do |db_config| - assert_equal "primary", db_config.spec_name - end - end - end - - def test_first_is_deprecated - assert_deprecated do - db_config = ActiveRecord::Base.configurations.first - assert_equal "arunit", db_config.env_name - assert_equal "primary", db_config.spec_name - end - end - - def test_fetch_is_deprecated - assert_deprecated do - db_config = ActiveRecord::Base.configurations.fetch("arunit").first - assert_equal "arunit", db_config.env_name - assert_equal "primary", db_config.spec_name - end - end - - def test_values_are_deprecated - config_hashes = ActiveRecord::Base.configurations.configurations.map(&:config) - assert_deprecated do - assert_equal config_hashes, ActiveRecord::Base.configurations.values - end - end - end -end diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index 27f9df295f..1a3a95f168 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -447,9 +447,8 @@ class NamedScopingTest < ActiveRecord::TestCase assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).to_a.uniq end - def test_chaining_doesnt_leak_conditions_to_another_scopes - expected = Topic.where(approved: false).where(id: Topic.children.select(:parent_id)) - assert_equal expected.to_a, Topic.rejected.has_children.to_a + def test_class_method_in_scope + assert_equal [topics(:second)], topics(:first).approved_replies.ordered end def test_nested_scoping diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index 0807bcf875..b35623a344 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -7,6 +7,8 @@ class Reply < Topic belongs_to :topic_with_primary_key, class_name: "Topic", primary_key: "title", foreign_key: "parent_title", counter_cache: "replies_count", touch: true has_many :replies, class_name: "SillyReply", dependent: :destroy, foreign_key: "parent_id" has_many :silly_unique_replies, dependent: :destroy, foreign_key: "parent_id" + + scope :ordered, -> { Reply.order(:id) } end class SillyReply < Topic diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index fdb461ed7f..75890c327a 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -10,9 +10,6 @@ class Topic < ActiveRecord::Base scope :approved, -> { where(approved: true) } scope :rejected, -> { where(approved: false) } - scope :children, -> { where.not(parent_id: nil) } - scope :has_children, -> { where(id: Topic.children.select(:parent_id)) } - scope :scope_with_lambda, lambda { all } scope :by_lifo, -> { where(author_name: "lifo") } diff --git a/activestorage/lib/active_storage/service/mirror_service.rb b/activestorage/lib/active_storage/service/mirror_service.rb index 75274f81b3..aa41df304e 100644 --- a/activestorage/lib/active_storage/service/mirror_service.rb +++ b/activestorage/lib/active_storage/service/mirror_service.rb @@ -9,7 +9,7 @@ module ActiveStorage class Service::MirrorService < Service attr_reader :primary, :mirrors - delegate :download, :download_chunk, :exist?, :url, to: :primary + delegate :download, :download_chunk, :exist?, :url, :path_for, to: :primary # Stitch together from named services. def self.build(primary:, mirrors:, configurator:, **options) #:nodoc: diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb index 94c751a4ff..249a5652fb 100644 --- a/activestorage/test/service/mirror_service_test.rb +++ b/activestorage/test/service/mirror_service_test.rb @@ -61,4 +61,8 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase @service.url(@key, expires_in: 2.minutes, disposition: :inline, filename: filename, content_type: "text/plain") end end + + test "path for file in primary service" do + assert_equal @service.primary.path_for(@key), @service.path_for(@key) + end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8bb531f1b8..c46b56833d 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* New autoloading based on [Zeitwerk](https://github.com/fxn/zeitwerk). + + *Xavier Noria* + * Revise `ActiveSupport::Notifications.unsubscribe` to correctly handle Regex or other multiple-pattern subscribers. *Zach Kemp* diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index bdd7bc70a0..92cdfd89fe 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -30,8 +30,9 @@ Gem::Specification.new do |s| # NOTE: Please read our dependency guidelines before updating versions: # https://edgeguides.rubyonrails.org/security.html#dependency-management-and-cves - s.add_dependency "i18n", ">= 0.7", "< 2" - s.add_dependency "tzinfo", "~> 1.1" - s.add_dependency "minitest", "~> 5.1" + s.add_dependency "i18n", ">= 0.7", "< 2" + 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.0" end diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index de1fb1886c..f43894a1ea 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -18,7 +18,6 @@ module ActiveSupport DIR_FORMATTER = "%03X" FILENAME_MAX_SIZE = 228 # max filename size on file system is 255, minus room for timestamp and random characters appended by Tempfile (used by atomic write) FILEPATH_MAX_SIZE = 900 # max is 1024, plus some room - EXCLUDED_DIRS = [".", ".."].freeze GITKEEP_FILES = [".gitkeep", ".keep"].freeze def initialize(cache_path, options = nil) @@ -35,7 +34,7 @@ module ActiveSupport # file store directory except for .keep or .gitkeep. Be careful which directory is specified in your # config file when using +FileStore+ because everything in that directory will be deleted. def clear(options = nil) - root_dirs = exclude_from(cache_path, EXCLUDED_DIRS + GITKEEP_FILES) + root_dirs = (Dir.children(cache_path) - GITKEEP_FILES) FileUtils.rm_r(root_dirs.collect { |f| File.join(cache_path, f) }) rescue Errno::ENOENT end @@ -154,7 +153,7 @@ module ActiveSupport # Delete empty directories in the cache. def delete_empty_directories(dir) return if File.realpath(dir) == File.realpath(cache_path) - if exclude_from(dir, EXCLUDED_DIRS).empty? + if Dir.children(dir).empty? Dir.delete(dir) rescue nil delete_empty_directories(File.dirname(dir)) end @@ -167,8 +166,7 @@ module ActiveSupport def search_dir(dir, &callback) return if !File.exist?(dir) - Dir.foreach(dir) do |d| - next if EXCLUDED_DIRS.include?(d) + Dir.each_child(dir) do |d| name = File.join(dir, d) if File.directory?(name) search_dir(name, &callback) @@ -193,11 +191,6 @@ module ActiveSupport end end end - - # Exclude entries from source directory - def exclude_from(source, excludes) - Dir.entries(source).reject { |f| excludes.include?(f) } - end end end end diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index 6892b31a5d..75624bae09 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -41,12 +41,14 @@ module ActiveSupport 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 + # Zeitwerk only accepts existing directories in `push_dir` to + # prevent misconfigurations. + next unless 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 diff --git a/activesupport/test/cache/stores/file_store_test.rb b/activesupport/test/cache/stores/file_store_test.rb index f6855bb308..0364d9ab64 100644 --- a/activesupport/test/cache/stores/file_store_test.rb +++ b/activesupport/test/cache/stores/file_store_test.rb @@ -101,7 +101,7 @@ class FileStoreTest < ActiveSupport::TestCase end assert File.exist?(cache_dir), "Parent of top level cache dir was deleted!" assert File.exist?(sub_cache_dir), "Top level cache dir was deleted!" - assert_empty Dir.entries(sub_cache_dir).reject { |f| ActiveSupport::Cache::FileStore::EXCLUDED_DIRS.include?(f) } + assert_empty Dir.children(sub_cache_dir) end def test_log_exception_when_cache_read_fails diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index ab3c33728d..d66add2ca0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,8 +1,3 @@ -* Fix `Rails.application.config_for` returning a HWIA when Hash were - inside Array. - - *Edouard Chin* - * Fix non-symbol access to nested hashes returned from `Rails::Application.config_for` being broken by allowing non-symbol access with a deprecation notice. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 16fbc99e7a..af3ec36064 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -271,7 +271,11 @@ module Rails end def autoloader=(autoloader) - if %i(classic zeitwerk).include?(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" diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index a1f1224a45..783254b54d 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -36,9 +36,7 @@ gem 'bootsnap', '>= 1.4.0', require: false # gem 'rack-cors' <%- end -%> -<% if RUBY_ENGINE == "ruby" -%> -gem "zeitwerk", ">= 1.0.0" - +<% if RUBY_ENGINE == 'ruby' -%> 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/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 937b8eb427..1ee9e43e89 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -660,15 +660,6 @@ 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 47d42645c6..4442cdf4bf 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -491,11 +491,7 @@ 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") - 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 + contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker).each { |r| require r }") File.write("#{app_template_path}/config/application.rb", contents) require "rails" |