diff options
62 files changed, 640 insertions, 133 deletions
diff --git a/actionmailbox/CHANGELOG.md b/actionmailbox/CHANGELOG.md index bca3d1d9ed..0d4799a7b8 100644 --- a/actionmailbox/CHANGELOG.md +++ b/actionmailbox/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix Bcc header not being included with emails from `create_inbound_email_from` test helpers. + + *jduff* + * Add `ApplicationMailbox.mailbox_for` to expose mailbox routing. *James Dabbs* diff --git a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb index 8713f545f5..cd8b12dcde 100644 --- a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb +++ b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb @@ -23,7 +23,7 @@ module Rails Mail.new(params.require(:mail).permit(:from, :to, :cc, :bcc, :in_reply_to, :subject, :body).to_h).tap do |mail| mail[:bcc]&.include_in_headers = true params[:mail][:attachments].to_a.each do |attachment| - mail.add_file(filename: attachment.path, content: attachment.read) + mail.add_file(filename: attachment.original_filename, content: attachment.read) end end end diff --git a/actionmailbox/lib/action_mailbox/test_helper.rb b/actionmailbox/lib/action_mailbox/test_helper.rb index d248fa8734..dc6a0229f7 100644 --- a/actionmailbox/lib/action_mailbox/test_helper.rb +++ b/actionmailbox/lib/action_mailbox/test_helper.rb @@ -14,7 +14,11 @@ module ActionMailbox # # create_inbound_email_from_mail(from: "david@loudthinking.com", subject: "Hello!") def create_inbound_email_from_mail(status: :processing, **mail_options) - create_inbound_email_from_source Mail.new(mail_options).to_s, status: status + mail = Mail.new(mail_options) + # Bcc header is not encoded by default + mail[:bcc].include_in_headers = true if mail[:bcc] + + create_inbound_email_from_source mail.to_s, status: status end # Create an +InboundEmail+ using the raw rfc822 +source+ as text. diff --git a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb index 6fc39c2433..33282d36f7 100644 --- a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb +++ b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb @@ -30,6 +30,27 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa end end + test "create inbound email with bcc" do + with_rails_env("development") do + assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do + post rails_conductor_inbound_emails_path, params: { + mail: { + from: "Jason Fried <jason@37signals.com>", + bcc: "Replies <replies@example.com>", + subject: "Hey there", + body: "How's it going?" + } + } + end + + mail = ActionMailbox::InboundEmail.last.mail + assert_equal %w[ jason@37signals.com ], mail.from + assert_equal %w[ replies@example.com ], mail.bcc + assert_equal "Hey there", mail.subject + assert_equal "How's it going?", mail.body.decoded + end + end + test "create inbound email with attachments" do with_rails_env("development") do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do @@ -47,6 +68,7 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa mail = ActionMailbox::InboundEmail.last.mail assert_equal "Let's talk about these images:", mail.text_part.decoded assert_equal 2, mail.attachments.count + assert_equal %w[ avatar1.jpeg avatar2.jpeg ], mail.attachments.collect(&:filename) end end diff --git a/actionmailbox/test/unit/router_test.rb b/actionmailbox/test/unit/router_test.rb index 4dd3730604..7eb8e04a73 100644 --- a/actionmailbox/test/unit/router_test.rb +++ b/actionmailbox/test/unit/router_test.rb @@ -51,6 +51,15 @@ module ActionMailbox assert_equal inbound_email.mail, $processed_mail end + test "single string routing on bcc" do + @router.add_routes("first@example.com" => :first) + + inbound_email = create_inbound_email_from_mail(to: "someone@example.com", bcc: "first@example.com", subject: "This is a reply") + @router.route inbound_email + assert_equal "FirstMailbox", $processed_by + assert_equal inbound_email.mail, $processed_mail + end + test "single string routing case-insensitively" do @router.add_routes("first@example.com" => :first) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 99a4ac6845..8bb2c73e52 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,23 @@ +* Add `Vary: Accept` header when using `Accept` header for response + + For some requests like `/users/1`, Rails uses requests' `Accept` + header to determine what to return. And if we don't add `Vary` + in the response header, browsers might accidentally cache different + types of content, which would cause issues: e.g. javascript got displayed + instead of html content. This PR fixes these issues by adding `Vary: Accept` + in these types of requests. For more detailed problem description, please read: + + https://github.com/rails/rails/pull/36213 + + Fixes #25842 + + *Stan Lo* + +* Fix IntegrationTest `follow_redirect!` to follow redirection using the same HTTP verb when following + a 307 redirection. + + *Edouard Chin* + * System tests require Capybara 3.26 or newer. *George Claghorn* diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 8ba2b25552..280af50a44 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -28,6 +28,7 @@ module AbstractController else _set_rendered_content_type rendered_format end + _set_vary_header self.response_body = rendered_body end @@ -109,6 +110,9 @@ module AbstractController def _set_html_content_type # :nodoc: end + def _set_vary_header # :nodoc: + end + def _set_rendered_content_type(format) # :nodoc: end diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index efa5de313c..fd22c4fa64 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -77,6 +77,10 @@ module ActionController end end + def _set_vary_header + self.headers["Vary"] = "Accept" if request.should_apply_vary_header? + end + # Normalize arguments by catching blocks and setting them on :update. def _normalize_args(action = nil, options = {}, &blk) options = super diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 6fbd52dd51..920ae52f2b 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -749,6 +749,18 @@ module ActionController end alias_method :delete_if, :reject! + # Returns a new instance of <tt>ActionController::Parameters</tt> without the blank values. + # Uses Object#blank? for determining if a value is blank. + def compact_blank + reject { |_k, v| v.blank? } + end + + # Removes all blank values in place and returns self. + # Uses Object#blank? for determining if a value is blank. + def compact_blank! + reject! { |_k, v| v.blank? } + end + # Returns values that were assigned to the given +keys+. Note that all the # +Hash+ objects will be converted to <tt>ActionController::Parameters</tt>. def values_at(*keys) diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index a2cac49082..ac0ff133eb 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -62,13 +62,7 @@ module ActionDispatch def formats fetch_header("action_dispatch.request.formats") do |k| - params_readable = begin - parameters[:format] - rescue *RESCUABLE_MIME_FORMAT_ERRORS - false - end - - v = if params_readable + v = if params_readable? Array(Mime[parameters[:format]]) elsif use_accept_header && valid_accept_header accepts @@ -153,9 +147,19 @@ module ActionDispatch order.include?(Mime::ALL) ? format : nil end + def should_apply_vary_header? + !params_readable? && use_accept_header && valid_accept_header + end + private BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/ + def params_readable? # :doc: + parameters[:format] + rescue *RESCUABLE_MIME_FORMAT_ERRORS + false + end + def valid_accept_header # :doc: (xhr? && (accept.present? || content_mime_type)) || (accept.present? && accept !~ BROWSER_LIKE_ACCEPTS) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 642f155085..9b5a5cf2b0 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -346,28 +346,6 @@ module ActionDispatch @cookies.map { |k, v| "#{escape(k)}=#{escape(v)}" }.join "; " end - def handle_options(options) # :nodoc: - if options[:expires].respond_to?(:from_now) - options[:expires] = options[:expires].from_now - end - - options[:path] ||= "/" - - if options[:domain] == :all || options[:domain] == "all" - # If there is a provided tld length then we use it otherwise default domain regexp. - domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP - - # If host is not ip and matches domain regexp. - # (ip confirms to domain regexp so we explicitly check for ip) - options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) - ".#{$&}" - end - elsif options[:domain].is_a? Array - # If host matches one of the supplied domains without a dot in front of it. - options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } - end - end - # Sets the cookie named +name+. The second argument may be the cookie's # value or a hash of options as documented above. def []=(name, options) @@ -447,6 +425,28 @@ module ActionDispatch def write_cookie?(cookie) request.ssl? || !cookie[:secure] || always_write_cookie end + + def handle_options(options) + if options[:expires].respond_to?(:from_now) + options[:expires] = options[:expires].from_now + end + + options[:path] ||= "/" + + if options[:domain] == :all || options[:domain] == "all" + # If there is a provided tld length then we use it otherwise default domain regexp. + domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP + + # If host is not ip and matches domain regexp. + # (ip confirms to domain regexp so we explicitly check for ip) + options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) + ".#{$&}" + end + elsif options[:domain].is_a? Array + # If host matches one of the supplied domains without a dot in front of it. + options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } + end + end end class AbstractCookieJar # :nodoc: diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index c5f8b816a4..9e7b4301a8 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -49,11 +49,16 @@ module ActionDispatch # Follow a single redirect response. If the last response was not a # redirect, an exception will be raised. Otherwise, the redirect is - # performed on the location header. Any arguments are passed to the - # underlying call to `get`. + # performed on the location header. If the redirection is a 307 redirect, + # the same HTTP verb will be used when redirecting, otherwise a GET request + # will be performed. Any arguments are passed to the + # underlying request. def follow_redirect!(**args) raise "not a redirect! #{status} #{status_message}" unless redirect? - get(response.location, **args) + + method = response.status == 307 ? request.method.downcase : :get + public_send(method, response.location, **args) + status end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index cce229b30d..cb7c2467ac 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -213,6 +213,10 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest redirect_to action_url("get") end + def redirect_307 + redirect_to action_url("post"), status: 307 + end + def remove_header response.headers.delete params[:header] head :ok, "c" => "3" @@ -337,6 +341,15 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + def test_307_redirect_uses_the_same_http_verb + with_test_route_set do + post "/redirect_307" + assert_equal 307, status + follow_redirect! + assert_equal "POST", request.method + end + end + def test_redirect_reset_html_document with_test_route_set do get "/redirect" @@ -530,6 +543,32 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + def test_setting_vary_header_when_request_is_xhr_with_accept_header + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_equal "Accept", response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_format_is_provided + with_test_route_set do + get "/get", params: { format: "json" } + assert_nil response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_ignore_accept_header_is_set + original_ignore_accept_header = ActionDispatch::Request.ignore_accept_header + ActionDispatch::Request.ignore_accept_header = true + + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_nil response.headers["Vary"] + end + ensure + ActionDispatch::Request.ignore_accept_header = original_ignore_accept_header + end + private def with_default_headers(headers) original = ActionDispatch::Response.default_headers diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index 31ee7964f5..ffffd2996f 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -127,4 +127,22 @@ class ParametersMutatorsTest < ActiveSupport::TestCase test "deep_transform_keys! retains unpermitted status" do assert_not_predicate @params.deep_transform_keys! { |k| k }, :permitted? end + + test "compact_blank retains permitted status" do + @params.permit! + assert_predicate @params.compact_blank, :permitted? + end + + test "compact_blank retains unpermitted status" do + assert_not_predicate @params.compact_blank, :permitted? + end + + test "compact_blank! retains permitted status" do + @params.permit! + assert_predicate @params.compact_blank!, :permitted? + end + + test "compact_blank! retains unpermitted status" do + assert_not_predicate @params.compact_blank!, :permitted? + end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index a2a6c69dd3..dd76824413 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -4,6 +4,11 @@ require "abstract_unit" require "controller/fake_models" class TestControllerWithExtraEtags < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "test/with_implicit_template.erb" => "Hello explicitly!", + "test/hello_world.erb" => "Hello world!" + )] + def self.controller_name; "test"; end def self.controller_path; "test"; end @@ -37,6 +42,11 @@ class TestControllerWithExtraEtags < ActionController::Base end class ImplicitRenderTestController < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "implicit_render_test/hello_world.erb" => "Hello world!", + "implicit_render_test/empty_action_with_template.html.erb" => "<h1>Empty action rendered this implicitly.</h1>\n" + )] + def empty_action end @@ -46,6 +56,10 @@ end module Namespaced class ImplicitRenderTestController < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "namespaced/implicit_render_test/hello_world.erb" => "Hello world!" + )] + def hello_world fresh_when(etag: "abc") end @@ -293,13 +307,15 @@ end module TemplateModificationHelper private def modify_template(name) - path = File.expand_path("../fixtures/#{name}.erb", __dir__) - original = File.read(path) - File.write(path, "#{original} Modified!") + hash = @controller.view_paths.first.instance_variable_get(:@hash) + key = name + ".erb" + original = hash[key] + hash[key] = "#{original} Modified!" ActionView::LookupContext::DetailsKey.clear yield ensure - File.write(path, original) + hash[key] = original + ActionView::LookupContext::DetailsKey.clear end end diff --git a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee index 736cab08db..9e11bfa7ed 100644 --- a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee +++ b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee @@ -11,6 +11,7 @@ Rails.serializeElement = (element, additionalParam) -> inputs.forEach (input) -> return if !input.name || input.disabled + return if input.closest('fieldset[disabled]') if matches(input, 'select') toArray(input.options).forEach (option) -> params.push(name: input.name, value: option.value) if option.selected diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 97a6da3634..cdf436ccae 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -9,10 +9,11 @@ module ActionView class << self # Supported options: # - # * <tt>name</tt> - Template name - # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> - # * <tt>dependencies</tt> - An array of dependent views - def digest(name:, format:, finder:, dependencies: nil) + # * <tt>name</tt> - Template name + # * <tt>format</tt> - Template format + # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> + # * <tt>dependencies</tt> - An array of dependent views + def digest(name:, format: nil, finder:, dependencies: nil) if dependencies.nil? || dependencies.empty? cache_key = "#{name}.#{format}" else diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index 539bedcdf0..03eac29bb4 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -7,10 +7,15 @@ module ActionView #:nodoc: # file system. This is used internally by Rails' own test suite, and is # useful for testing extensions that have no way of knowing what the file # system will look like at runtime. - class FixtureResolver < PathResolver + class FixtureResolver < OptimizedFileSystemResolver def initialize(hash = {}, pattern = nil) - super(pattern) + super("") + if pattern + ActiveSupport::Deprecation.warn "Specifying a custom path for #{self.class} is deprecated. Implement a custom Resolver subclass instead." + @pattern = pattern + end @hash = hash + @path = "" end def data @@ -23,25 +28,32 @@ module ActionView #:nodoc: private def query(path, exts, _, locals, cache:) - query = +"" - EXTENSIONS.each do |ext, prefix| - query << "(" << exts[ext].map { |e| e && Regexp.escape("#{prefix}#{e}") }.join("|") << "|)" - end - query = /^(#{Regexp.escape(path)})#{query}$/ + regex = build_regex(path, exts) - templates = [] - @hash.each do |_path, source| - next unless query.match?(_path) + @hash.select do |_path, _| + ("/" + _path).match?(regex) + end.map do |_path, source| handler, format, variant = extract_handler_and_format_and_variant(_path) - templates << Template.new(source, _path, handler, + + Template.new(source, _path, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals ) + end.sort_by do |t| + match = ("/" + t.identifier).match(regex) + EXTENSIONS.keys.reverse.map do |ext| + if ext == :variants && exts[ext] == :any + match[ext].nil? ? 0 : 1 + elsif match[ext].nil? + exts[ext].length + else + found = match[ext].to_sym + exts[ext].index(found) + end + end end - - templates.sort_by { |t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size } end end diff --git a/actionview/test/template/testing/fixture_resolver_test.rb b/actionview/test/template/testing/fixture_resolver_test.rb index 6d0317e0c9..a6066e94c6 100644 --- a/actionview/test/template/testing/fixture_resolver_test.rb +++ b/actionview/test/template/testing/fixture_resolver_test.rb @@ -27,4 +27,26 @@ class FixtureResolverTest < ActiveSupport::TestCase assert_equal :html, templates.first.format assert_equal "variant", templates.first.variant end + + def test_should_match_locales + resolver = ActionView::FixtureResolver.new("arbitrary/path.erb" => "this text", "arbitrary/path.fr.erb" => "ce texte") + en = resolver.find_all("path", "arbitrary", false, locale: [:en], formats: [:html], variants: [], handlers: [:erb]) + fr = resolver.find_all("path", "arbitrary", false, locale: [:fr], formats: [:html], variants: [], handlers: [:erb]) + + assert_equal 1, en.size + assert_equal 2, fr.size + + assert_equal "this text", en[0].source + assert_equal "ce texte", fr[0].source + assert_equal "this text", fr[1].source + end + + def test_should_return_all_variants_for_any + resolver = ActionView::FixtureResolver.new("arbitrary/path.html.erb" => "this html", "arbitrary/path.html+varient.erb" => "this text") + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: [], handlers: [:erb]) + assert_equal 1, templates.size, "expected one template" + assert_equal "this html", templates.first.source + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: :any, handlers: [:erb]) + assert_equal 2, templates.size, "expected all templates" + end end diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index 16ea114f3b..dbbb383995 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -490,4 +490,22 @@ asyncTest('changing a select option without "data-url" attribute still fires aja setTimeout(function() { start() }, 20) }) +asyncTest('inputs inside disabled fieldset are not submited on remote forms', 3, function() { + $('form') + .append('<fieldset>\ + <input name="description" value="A wise man" />\ + </fieldset>') + .append('<fieldset disabled="disabled">\ + <input name="age" />\ + </fieldset>') + .bindNative('ajax:success', function(e, data, status, xhr) { + equal(data.params.user_name, 'john') + equal(data.params.description, 'A wise man') + equal(data.params.age, undefined) + + start() + }) + .triggerNative('submit') +}) + })() diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 2f0d72ccb9..542a4eb2db 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,6 @@ +* `assert_enqueued_with` and `assert_performed_with` can now test jobs with relative delay. + + *Vlado Cingel* Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/activejob/CHANGELOG.md) for previous changes. diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 463020a332..5b1af7da22 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -630,7 +630,7 @@ module ActiveJob def prepare_args_for_assertion(args) args.dup.tap do |arguments| - arguments[:at] = arguments[:at].to_f if arguments[:at] + arguments[:at] = round_time_arguments(arguments[:at]) if arguments[:at] arguments[:args] = round_time_arguments(arguments[:args]) if arguments[:args] end end @@ -650,6 +650,7 @@ module ActiveJob def deserialize_args_for_assertion(job) job.dup.tap do |new_job| + new_job[:at] = round_time_arguments(Time.at(new_job[:at])) if new_job[:at] new_job[:args] = ActiveJob::Arguments.deserialize(new_job[:args]) if new_job[:args] end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 32471cfacc..34dfd8eb56 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -621,6 +621,12 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end + def test_assert_enqueued_with_with_relative_at_option + assert_enqueued_with(job: HelloJob, at: 5.minutes.from_now) do + HelloJob.set(wait: 5.minutes).perform_later + end + end + def test_assert_enqueued_with_with_no_block_with_at_option HelloJob.set(wait_until: Date.tomorrow.noon).perform_later assert_enqueued_with(job: HelloJob, at: Date.tomorrow.noon) @@ -1663,6 +1669,18 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_performed_with_with_relative_at_option + assert_performed_with(job: HelloJob, at: 5.minutes.from_now) do + HelloJob.set(wait: 5.minutes).perform_later + end + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, at: 2.minutes.from_now) do + HelloJob.set(wait: 1.minute).perform_later + end + end + end + def test_assert_performed_with_without_block_with_at_option HelloJob.set(wait_until: Date.tomorrow.noon).perform_later diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 415f1f679b..1a4e0b8e59 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -352,11 +352,7 @@ module ActiveModel def attribute_method_matchers_matching(method_name) attribute_method_matchers_cache.compute_if_absent(method_name) do - # Bump plain matcher to last place so that only methods that do not - # match any other pattern match the actual attribute name. - # This is currently only needed to support legacy usage. - matchers = attribute_method_matchers.partition(&:plain?).reverse.flatten(1) - matchers.map { |matcher| matcher.match(method_name) }.compact + attribute_method_matchers.map { |matcher| matcher.match(method_name) }.compact end end @@ -406,10 +402,6 @@ module ActiveModel def method_name(attr_name) @method_name % attr_name end - - def plain? - prefix.empty? && suffix.empty? - end end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 727ddd6bb7..2af48f99db 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Add a warning for enum elements with 'not_' prefix. + + class Foo + enum status: [:sent, :not_sent] + end + + *Edu Depetris* + * Make currency symbols optional for money column type in PostgreSQL *Joel Schneider* diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index c3d4eab562..891b50d160 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -56,7 +56,7 @@ module ActiveRecord def ids_writer(ids) primary_key = reflection.association_primary_key pk_type = klass.type_for_attribute(primary_key) - ids = Array(ids).reject(&:blank?) + ids = Array(ids).compact_blank ids.map! { |i| pk_type.cast(i) } records = klass.where(primary_key => ids).index_by do |r| diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 13f94a4722..88367c79a1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -100,7 +100,7 @@ module ActiveRecord def index_exists?(table_name, column_name, options = {}) column_names = Array(column_name).map(&:to_s) checks = [] - checks << lambda { |i| i.columns == column_names } + checks << lambda { |i| Array(i.columns) == column_names } checks << lambda { |i| i.unique } if options[:unique] checks << lambda { |i| i.name == options[:name].to_s } if options[:name] diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 405fecb603..ef1eef6b69 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -476,12 +476,12 @@ module ActiveRecord # distinct queries, and requires that the ORDER BY include the distinct column. # See https://dev.mysql.com/doc/refman/5.7/en/group-by-handling.html def columns_for_distinct(columns, orders) # :nodoc: - order_columns = orders.reject(&:blank?).map { |s| + order_columns = orders.compact_blank.map { |s| # Convert Arel node to string s = s.to_sql unless s.is_a?(String) # Remove any ASC/DESC modifiers s.gsub(/\s+(?:ASC|DESC)\b/i, "") - }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" } + }.compact_blank.map.with_index { |column, i| "#{column} AS alias_#{i}" } (order_columns << super).join(", ") end diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index df26f67c6e..0732b69f81 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -50,7 +50,7 @@ module ActiveRecord # Converts the given URL to a full connection hash. def to_hash - config = raw_config.reject { |_, value| value.blank? } + config = raw_config.compact_blank config.map { |key, value| config[key] = uri_parser.unescape(value) if value.is_a? String } config end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 0062952667..628a609521 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -555,13 +555,13 @@ module ActiveRecord # PostgreSQL requires the ORDER BY columns in the select list for distinct queries, and # requires that the ORDER BY include the distinct column. def columns_for_distinct(columns, orders) #:nodoc: - order_columns = orders.reject(&:blank?).map { |s| + order_columns = orders.compact_blank.map { |s| # Convert Arel node to string s = s.to_sql unless s.is_a?(String) # Remove any ASC/DESC modifiers s.gsub(/\s+(?:ASC|DESC)\b/i, "") .gsub(/\s+NULLS\s+(?:FIRST|LAST)\b/i, "") - }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" } + }.compact_blank.map.with_index { |column, i| "#{column} AS alias_#{i}" } (order_columns << super).join(", ") end diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index e122628b05..3e387782f6 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -91,6 +91,19 @@ module ActiveRecord end alias :blank? :empty? + def each + throw_getter_deprecation(:each) + configurations.each { |config| + yield [config.env_name, config.config] + } + end + + def first + throw_getter_deprecation(:first) + config = configurations.first + [config.env_name, config.config] + end + private def env_with_configs(env = nil) if env @@ -105,18 +118,20 @@ module ActiveRecord return configs if configs.is_a?(Array) db_configs = configs.flat_map do |env_name, config| - if config.is_a?(Hash) && config.all? { |k, v| v.is_a?(Hash) } + if config.is_a?(Hash) && config.all? { |_, v| v.is_a?(Hash) } walk_configs(env_name.to_s, config) else build_db_config_from_raw_config(env_name.to_s, "primary", config) end - end.compact + end - if url = ENV["DATABASE_URL"] - merge_url_with_configs(url, db_configs) - else - db_configs + current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s + + unless db_configs.find(&:for_current_env?) + db_configs << environment_url_config(current_env, "primary", {}) end + + merge_db_environment_variables(current_env, db_configs.compact) end def walk_configs(env_name, config) @@ -156,27 +171,24 @@ module ActiveRecord end end - def merge_url_with_configs(url, configs) - env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s + def merge_db_environment_variables(current_env, configs) + configs.map do |config| + next config if config.url_config? || config.env_name != current_env - if configs.find(&:for_current_env?) - configs.map do |config| - if config.url_config? - config - else - ActiveRecord::DatabaseConfigurations::UrlConfig.new(config.env_name, config.spec_name, url, config.config) - end - end - else - configs + [ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, "primary", url)] + url_config = environment_url_config(current_env, config.spec_name, config.config) + url_config || config end end + def environment_url_config(env, spec_name, config) + url = ENV["DATABASE_URL"] + return unless url + + ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, spec_name, url, config) + end + def method_missing(method, *args, &blk) 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) diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 8077630aeb..fc49f752aa 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -200,6 +200,8 @@ module ActiveRecord # scope :active, -> { where(status: 0) } # scope :not_active, -> { where.not(status: 0) } if enum_scopes != false + klass.send(:detect_negative_condition!, value_method_name) + klass.send(:detect_enum_conflict!, name, value_method_name, true) klass.scope value_method_name, -> { where(attr => value) } @@ -261,5 +263,12 @@ module ActiveRecord source: source } end + + def detect_negative_condition!(method_name) + if method_name.start_with?("not_") && logger + logger.warn "An enum element in #{self.name} uses the prefix 'not_'." \ + " This will cause a conflict with auto generated negative scopes." + end + end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6a181882ae..6957ba052b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -138,7 +138,7 @@ module ActiveRecord end def includes!(*args) # :nodoc: - args.reject!(&:blank?) + args.compact_blank! args.flatten! self.includes_values |= args @@ -265,7 +265,7 @@ module ActiveRecord end def _select!(*fields) # :nodoc: - fields.reject!(&:blank?) + fields.compact_blank! fields.flatten! self.select_values += fields self @@ -965,7 +965,7 @@ module ActiveRecord def reverse_order! # :nodoc: orders = order_values.uniq - orders.reject!(&:blank?) + orders.compact_blank! self.order_values = reverse_sql_order(orders) self end @@ -1053,7 +1053,7 @@ module ActiveRecord ) arel.skip(Arel::Nodes::BindParam.new(offset_attribute)) end - arel.group(*arel_columns(group_values.uniq.reject(&:blank?))) unless group_values.empty? + arel.group(*arel_columns(group_values.uniq.compact_blank)) unless group_values.empty? build_order(arel) @@ -1129,7 +1129,7 @@ module ActiveRecord association_joins = buckets[:association_join] stashed_joins = buckets[:stashed_join] join_nodes = buckets[:join_node].tap(&:uniq!) - string_joins = buckets[:string_join].delete_if(&:blank?).map!(&:strip).tap(&:uniq!) + string_joins = buckets[:string_join].compact_blank!.map!(&:strip).tap(&:uniq!) string_joins.map! { |join| table.create_string_join(Arel.sql(join)) } @@ -1227,7 +1227,7 @@ module ActiveRecord def build_order(arel) orders = order_values.uniq - orders.reject!(&:blank?) + orders.compact_blank! arel.order(*orders) unless orders.empty? end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index a78bebf764..5d1ce19829 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -154,6 +154,8 @@ module ActiveRecord end def for_each(databases) + return {} unless defined?(Rails) + database_configs = ActiveRecord::DatabaseConfigurations.new(databases).configs_for(env_name: Rails.env) # if this is a single database application we don't want tasks for each primary database diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index a7e04007a9..e3efeb75b5 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -16,7 +16,7 @@ module ActiveRecord connection.create_database configuration["database"], creation_options establish_connection configuration rescue ActiveRecord::StatementInvalid => error - if error.cause.error_number == ER_DB_CREATE_EXISTS + if connection.error_number(error.cause) == ER_DB_CREATE_EXISTS raise DatabaseAlreadyExists else raise diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index d99593817a..830c0892d3 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -253,9 +253,11 @@ module ActiveRecord def test_expression_index with_example_table do - @connection.add_index "ex", "mod(id, 10), abs(number)", name: "expression" + expr = "mod(id, 10), abs(number)" + @connection.add_index "ex", expr, name: "expression" index = @connection.indexes("ex").find { |idx| idx.name == "expression" } - assert_equal "mod(id, 10), abs(number)", index.columns + assert_equal expr, index.columns + assert_equal true, @connection.index_exists?("ex", expr, name: "expression") end 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 15a045ed23..95e57f42e3 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 @@ -347,6 +347,22 @@ module ActiveRecord assert_equal expected, actual end end + + def test_does_not_change_other_environments + ENV["DATABASE_URL"] = "postgres://localhost/foo" + config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" }, "default_env" => {} } + + actual = resolve_spec(:production, config) + assert_equal config["production"].merge("name" => "production"), actual + + actual = resolve_spec(:default_env, config) + assert_equal({ + "host" => "localhost", + "database" => "foo", + "adapter" => "postgresql", + "name" => "default_env" + }, actual) + end end end end diff --git a/activerecord/test/cases/database_configurations_test.rb b/activerecord/test/cases/database_configurations_test.rb index ed8151f01a..725d1b5d1b 100644 --- a/activerecord/test/cases/database_configurations_test.rb +++ b/activerecord/test/cases/database_configurations_test.rb @@ -80,17 +80,20 @@ class LegacyDatabaseConfigurationsTest < ActiveRecord::TestCase def test_each_is_deprecated assert_deprecated do - ActiveRecord::Base.configurations.each do |db_config| - assert_equal "primary", db_config.spec_name + all_configs = ActiveRecord::Base.configurations.values + ActiveRecord::Base.configurations.each do |env_name, config| + assert_includes ["arunit", "arunit2", "arunit_without_prepared_statements"], env_name + assert_includes all_configs, config end end end def test_first_is_deprecated + first_config = ActiveRecord::Base.configurations.values.first assert_deprecated do - db_config = ActiveRecord::Base.configurations.first - assert_equal "arunit", db_config.env_name - assert_equal "primary", db_config.spec_name + env_name, config = ActiveRecord::Base.configurations.first + assert_equal "arunit", env_name + assert_equal first_config, config end end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index ae0ce195b3..8673a99c45 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -3,6 +3,7 @@ require "cases/helper" require "models/author" require "models/book" +require "active_support/log_subscriber/test_helper" class EnumTest < ActiveRecord::TestCase fixtures :books, :authors, :author_addresses @@ -565,4 +566,25 @@ class EnumTest < ActiveRecord::TestCase assert_raises(NoMethodError) { klass.proposed } end + + test "enums with a negative condition log a warning" do + old_logger = ActiveRecord::Base.logger + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + + ActiveRecord::Base.logger = logger + + expected_message = "An enum element in Book uses the prefix 'not_'."\ + " This will cause a conflict with auto generated negative scopes." + + Class.new(ActiveRecord::Base) do + def self.name + "Book" + end + enum status: [:sent, :not_sent] + end + + assert_match(expected_message, logger.logged(:warn).first) + ensure + ActiveRecord::Base.logger = old_logger + end end diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 258132835f..ac3c5bc26e 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -7,7 +7,10 @@ if current_adapter?(:Mysql2Adapter) module ActiveRecord class MysqlDBCreateTest < ActiveRecord::TestCase def setup - @connection = Class.new { def create_database(*); end }.new + @connection = Class.new do + def create_database(*); end + def error_number(_); end + end.new @configuration = { "adapter" => "mysql2", "database" => "my-app-db" @@ -90,9 +93,11 @@ if current_adapter?(:Mysql2Adapter) with_stubbed_connection_establish_connection do ActiveRecord::Base.connection.stub( :create_database, - proc { raise ActiveRecord::Tasks::DatabaseAlreadyExists } + proc { raise ActiveRecord::StatementInvalid } ) do - ActiveRecord::Tasks::DatabaseTasks.create @configuration + ActiveRecord::Base.connection.stub(:error_number, 1007) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration + end assert_equal "Database 'my-app-db' already exists\n", $stderr.string end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 5d6bb7a0b3..f20c7c92e6 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,8 @@ +* Add `compact_blank` for those times when you want to remove #blank? values from + an Enumerable (also `compact_blank!` on Hash, Array, ActionController::Parameters) + + *Dana Sherson* + * Make ActiveSupport::Logger Fiber-safe. Fixes #36752. Use `Fiber.current.__id__` in `ActiveSupport::Logger#local_level=` in order diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 4675c41936..728d332306 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -148,6 +148,41 @@ module Enumerable map { |element| element[keys.first] } end end + + # Returns a new +Array+ without the blank items. + # Uses Object#blank? for determining if an item is blank. + # + # [1, "", nil, 2, " ", [], {}, false, true].compact_blank + # # => [1, 2, true] + # + # Set.new([nil, "", 1, 2]) + # # => [2, 1] (or [1, 2]) + # + # When called on a +Hash+, returns a new +Hash+ without the blank values. + # + # { a: "", b: 1, c: nil, d: [], e: false, f: true }.compact_blank + # #=> { b: 1, f: true } + def compact_blank + reject(&:blank?) + end +end + +class Hash + # Hash#reject has its own definition, so this needs one too. + def compact_blank #:nodoc: + reject { |_k, v| v.blank? } + end + + # Removes all blank values from the +Hash+ in place and returns self. + # Uses Object#blank? for determining if a value is blank. + # + # h = { a: "", b: 1, c: nil, d: [], e: false, f: true } + # h.compact_blank! + # # => { b: 1, f: true } + def compact_blank! + # use delete_if rather than reject! because it always returns self even if nothing changed + delete_if { |_k, v| v.blank? } + end end class Range #:nodoc: @@ -185,4 +220,15 @@ class Array #:nodoc: super end end + + # Removes all blank elements from the +Array+ in place and returns self. + # Uses Object#blank? for determining if an item is blank. + # + # a = [1, "", nil, 2, " ", [], {}, false, true] + # a.compact_blank! + # # => [1, 2, true] + def compact_blank! + # use delete_if rather than reject! because it always returns self even if nothing changed + delete_if(&:blank?) + end end diff --git a/activesupport/lib/active_support/core_ext/string/access.rb b/activesupport/lib/active_support/core_ext/string/access.rb index 4ca24028b0..4894193665 100644 --- a/activesupport/lib/active_support/core_ext/string/access.rb +++ b/activesupport/lib/active_support/core_ext/string/access.rb @@ -44,7 +44,7 @@ class String # str.from(0).to(-1) # => "hello" # str.from(1).to(-2) # => "ell" def from(position) - self[position..-1] + self[position, length] end # Returns a substring from the beginning of the string to the given position. @@ -61,7 +61,8 @@ class String # str.from(0).to(-1) # => "hello" # str.from(1).to(-2) # => "ell" def to(position) - self[0..position] + position = [position + length, -1].max if position < 0 + self[0, position + 1] end # Returns the first character. If a limit is supplied, returns a substring diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 32cb3a53f4..923d6ad228 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -20,6 +20,9 @@ module ActiveSupport #:nodoc: module Dependencies #:nodoc: extend self + UNBOUND_METHOD_MODULE_NAME = Module.instance_method(:name) + private_constant :UNBOUND_METHOD_MODULE_NAME + mattr_accessor :interlock, default: Interlock.new # :doc: @@ -658,7 +661,7 @@ module ActiveSupport #:nodoc: # Determine if the given constant has been automatically loaded. def autoloaded?(desc) - return false if desc.is_a?(Module) && desc.anonymous? + return false if desc.is_a?(Module) && real_mod_name(desc).nil? name = to_constant_name desc return false unless qualified_const_defined?(name) autoloaded_constants.include?(name) @@ -714,7 +717,7 @@ module ActiveSupport #:nodoc: when String then desc.sub(/^::/, "") when Symbol then desc.to_s when Module - desc.name || + real_mod_name(desc) || raise(ArgumentError, "Anonymous modules have no name to be referenced by") else raise TypeError, "Not a valid constant descriptor: #{desc.inspect}" end @@ -788,6 +791,14 @@ module ActiveSupport #:nodoc: def log(message) logger.debug("autoloading: #{message}") if logger && verbose end + + private + + # Returns the original name of a class or module even if `name` has been + # overridden. + def real_mod_name(mod) + UNBOUND_METHOD_MODULE_NAME.bind(mod).call + end end end diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index 821e3f971e..f75083a05a 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -28,7 +28,7 @@ module ActiveSupport end def autoloaded?(object) - cpath = object.is_a?(Module) ? object.name : object.to_s + cpath = object.is_a?(Module) ? real_mod_name(object) : object.to_s Rails.autoloaders.main.unloadable_cpath?(cpath) end diff --git a/activesupport/lib/active_support/descendants_tracker.rb b/activesupport/lib/active_support/descendants_tracker.rb index 1dad4f923e..b14842bf67 100644 --- a/activesupport/lib/active_support/descendants_tracker.rb +++ b/activesupport/lib/active_support/descendants_tracker.rb @@ -77,15 +77,17 @@ module ActiveSupport end def <<(klass) - cleanup! @refs << WeakRef.new(klass) end def each - @refs.each do |ref| + @refs.reject! do |ref| yield ref.__getobj__ + false rescue WeakRef::RefError + true end + self end def refs_size diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 5981763f0e..6acf64cb39 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -367,18 +367,20 @@ module ActiveSupport key.kind_of?(Symbol) ? key.to_s : key end - def convert_value(value, options = {}) # :doc: + def convert_value(value, for: nil) # :doc: + conversion = binding.local_variable_get(:for) + if value.is_a? Hash - if options[:for] == :to_hash + if conversion == :to_hash value.to_hash else value.nested_under_indifferent_access end elsif value.is_a?(Array) - if options[:for] != :assignment || value.frozen? + if conversion != :assignment || value.frozen? value = value.dup end - value.map! { |e| convert_value(e, options) } + value.map! { |e| convert_value(e, for: conversion) } else value end diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index aa602329ec..dda71b880e 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -218,6 +218,7 @@ module ActiveSupport def finish(name, id, payload) stack = Thread.current[:_event_stack] event = stack.pop + event.payload = payload event.finish! @delegate.call event end diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index 7ab39c9bfb..24e1ab313a 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -52,7 +52,8 @@ module ActiveSupport end class Event - attr_reader :name, :time, :end, :transaction_id, :payload, :children + attr_reader :name, :time, :end, :transaction_id, :children + attr_accessor :payload def self.clock_gettime_supported? # :nodoc: defined?(Process::CLOCK_PROCESS_CPUTIME_ID) && diff --git a/activesupport/lib/active_support/secure_compare_rotator.rb b/activesupport/lib/active_support/secure_compare_rotator.rb index 14a0aee947..97110d41f7 100644 --- a/activesupport/lib/active_support/secure_compare_rotator.rb +++ b/activesupport/lib/active_support/secure_compare_rotator.rb @@ -37,7 +37,7 @@ module ActiveSupport @value = value end - def secure_compare!(other_value, on_rotation: @rotation) + def secure_compare!(other_value, on_rotation: @on_rotation) secure_compare(@value, other_value) || run_rotations(on_rotation) { |wrapper| wrapper.secure_compare!(other_value) } || raise(InvalidMatch) diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 381b5a1f32..a9bf4b82f4 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -242,4 +242,28 @@ class EnumerableTests < ActiveSupport::TestCase ]) assert_equal [[5, 99], [15, 0], [10, 50]], payments.pluck(:dollars, :cents) end + + def test_compact_blank + values = GenericEnumerable.new([1, "", nil, 2, " ", [], {}, false, true]) + + assert_equal [1, 2, true], values.compact_blank + end + + def test_array_compact_blank! + values = [1, "", nil, 2, " ", [], {}, false, true] + values.compact_blank! + + assert_equal [1, 2, true], values + end + + def test_hash_compact_blank + values = { a: "", b: 1, c: nil, d: [], e: false, f: true } + assert_equal({ b: 1, f: true }, values.compact_blank) + end + + def test_hash_compact_blank! + values = { a: "", b: 1, c: nil, d: [], e: false, f: true } + values.compact_blank! + assert_equal({ b: 1, f: true }, values) + end end diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 003a0dbccb..6bad69f7f2 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -592,6 +592,13 @@ class DependenciesTest < ActiveSupport::TestCase nil_name = Module.new def nil_name.name() nil end assert_not ActiveSupport::Dependencies.autoloaded?(nil_name) + + invalid_constant_name = Module.new do + def self.name + "primary::SchemaMigration" + end + end + assert_not ActiveSupport::Dependencies.autoloaded?(invalid_constant_name) end ensure remove_constants(:ModuleFolder) diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index c9c63680e4..08277e5436 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -41,6 +41,27 @@ module Notifications assert_operator event.duration, :>, 0 end + def test_subscribe_to_events_where_payload_is_changed_during_instrumentation + @notifier.subscribe do |event| + assert_equal "success!", event.payload[:my_key] + end + + ActiveSupport::Notifications.instrument("foo") do |payload| + payload[:my_key] = "success!" + end + end + + def test_subscribe_to_events_can_handle_nested_hashes_in_the_paylaod + @notifier.subscribe do |event| + assert_equal "success!", event.payload[:some_key][:key_one] + assert_equal "great_success!", event.payload[:some_key][:key_two] + end + + ActiveSupport::Notifications.instrument("foo", some_key: { key_one: "success!" }) do |payload| + payload[:some_key][:key_two] = "great_success!" + end + end + def test_subscribe_via_top_level_api old_notifier = ActiveSupport::Notifications.notifier ActiveSupport::Notifications.notifier = ActiveSupport::Notifications::Fanout.new diff --git a/activesupport/test/secure_compare_rotator_test.rb b/activesupport/test/secure_compare_rotator_test.rb index 8acf13e38f..d80faea128 100644 --- a/activesupport/test/secure_compare_rotator_test.rb +++ b/activesupport/test/secure_compare_rotator_test.rb @@ -41,4 +41,17 @@ class SecureCompareRotatorTest < ActiveSupport::TestCase assert_equal(true, wrapper.secure_compare!("and_another_one", on_rotation: -> { @witness = true })) end end + + test "#secure_compare! calls the on_rotation proc that given in constructor" do + @witness = nil + + wrapper = ActiveSupport::SecureCompareRotator.new("old_secret", on_rotation: -> { @witness = true }) + wrapper.rotate("new_secret") + wrapper.rotate("another_secret") + wrapper.rotate("and_another_one") + + assert_changes(:@witness, from: nil, to: true) do + assert_equal(true, wrapper.secure_compare!("and_another_one")) + end + end end diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index a1b69edd22..dda3ae0863 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -1025,6 +1025,34 @@ If `@article.author_id` is 1, this would return: <label for="article_author_id_3">M. Clark</label> ``` +Recovering some option passed (e.g. programatically checking an object from collection): + +```ruby +collection_radio_buttons(:article, :author_id, Author.all, :id, :name_with_initial, {checked: Author.last}) +``` + +In this case, the last object from the collection will be checked: + +```html +<input id="article_author_id_1" name="article[author_id]" type="radio" value="1" /> +<label for="article_author_id_1">D. Heinemeier Hansson</label> +<input id="article_author_id_2" name="article[author_id]" type="radio" value="2" /> +<label for="article_author_id_2">D. Thomas</label> +<input id="article_author_id_3" name="article[author_id]" type="radio" value="3" checked="checked" /> +<label for="article_author_id_3">M. Clark</label> +``` + +To access the passed options programatically (e.g. adding a custom class if checked): + +**Sample html.erb** + +```html+erb +<%= collection_radio_buttons(:article, :author_id, Author.all, :id, :name_with_initial, {checked: Author.last, required: true} do |rb| %> + <%= rb.label(class: "#{'my-custom-class' if rb.value == Author.last.id}") { rb.radio_button + rb.text } %> +<% end %> +``` + + #### collection_check_boxes Returns `check_box` tags for the collection of existing return values of `method` for `object`'s class. diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 8cf8efefd0..b740e933ba 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -14,6 +14,7 @@ After reading this guide, you will know: * How Rails' built-in helpers assist you. * How to handle Ajax on the server side. * The Turbolinks gem. +* How to include your Cross-Site Request Forgery token in request headers ------------------------------------------------------------------------------- @@ -524,6 +525,23 @@ For more details, including other events you can bind to, check out [the Turbolinks README](https://github.com/turbolinks/turbolinks/blob/master/README.md). +Cross-Site Request Forgery (CSRF) token in Ajax +---- + +When using another library to make Ajax calls, it is necessary to add +the security token as a default header for Ajax calls in your library. To get +the token: + +```javascript +var token = document.getElementsByName('csrf-token')[0].content +``` + +You can then submit this token as a X-CSRF-Token in your header for your +Ajax requst. You do not need to add a CSRF for GET requests, only non-GET +requests. + +You can read more about about Cross-Site Request Forgery in [Security](https://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf) + Other Resources --------------- diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index 4143b3c881..612bd170c6 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "rails/railtie/configuration" +require "yaml" module Rails class Engine @@ -40,7 +41,7 @@ module Rails paths.add "app", eager_load: true, glob: "{*,*/concerns}", - exclude: %w(assets javascript) + exclude: ["assets", webpacker_path] paths.add "app/assets", glob: "*" paths.add "app/controllers", eager_load: true paths.add "app/channels", eager_load: true, glob: "**/*_channel.rb" @@ -85,6 +86,14 @@ module Rails def autoload_paths @autoload_paths ||= paths.autoload_paths end + + def webpacker_path + if File.file?("#{Rails.root}/config/webpacker.yml") + YAML.load_file("#{Rails.root}/config/webpacker.yml")[Rails.env]["source_path"]&.gsub("app/", "") + else + "javascript" + end + end end end end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt index ffe53497bf..2510ab906f 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt @@ -38,3 +38,8 @@ # MailDeliveryJob to ensure all delivery jobs are processed properly. # Make sure your entire app is migrated and stable on 6.0 before using this setting. # Rails.application.config.action_mailer.delivery_job = "ActionMailer::MailDeliveryJob" + +# Enable the same cache key to be reused when the object being cached of type +# `ActiveRecord::Relation` changes by moving the volatile information (max updated at and count) +# of the relation's cache key into the cache version to support recycling cache key. +# Rails.application.config.active_record.collection_cache_versioning = true diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index 838fe55acc..0664338e0b 100644 --- a/railties/lib/rails/paths.rb +++ b/railties/lib/rails/paths.rb @@ -223,12 +223,10 @@ module Rails private def files_in(path) - Dir.chdir(path) do - files = Dir.glob(@glob) - files -= @exclude if @exclude - files.map! { |file| File.join(path, file) } - files.sort - end + files = Dir.glob(@glob, base: path) + files -= @exclude if @exclude + files.map! { |file| File.join(path, file) } + files.sort end end end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 96678c395c..38dda20bec 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1708,7 +1708,7 @@ module ApplicationTests app "development" ActiveSupport::Dependencies.autoload_paths.each do |path| assert_not_operator path, :ends_with?, "app/assets" - assert_not_operator path, :ends_with?, "app/javascript" + assert_not_operator path, :ends_with?, "app/#{Rails.configuration.webpacker_path}" end end diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index fe56e3d076..e8456e8c19 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -162,7 +162,6 @@ module ApplicationTests rails "generate", "scaffold", "user", "username:string", "password:string" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") @@ -194,7 +193,6 @@ module ApplicationTests rails "generate", "scaffold", "LineItems", "product:references", "cart:belongs_to" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index 9146222f73..ff8c06b479 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -98,6 +98,15 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert_nil deps.safe_constantize("Admin") end + test "autoloaded? and overridden class names" do + invalid_constant_name = Module.new do + def self.name + "primary::SchemaMigration" + end + end + assert_not deps.autoloaded?(invalid_constant_name) + end + test "unloadable constants (main)" do app_file "app/models/user.rb", "class User; end" app_file "app/models/post.rb", "class Post; end" |