diff options
106 files changed, 867 insertions, 523 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 0618bcf7ec..db32b3748e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -376,7 +376,7 @@ GEM rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.0.4) + rails-html-sanitizer (1.1.0) loofah (~> 2.2, >= 2.2.2) rainbow (3.0.0) rake (12.3.2) diff --git a/actionmailbox/lib/action_mailbox/router.rb b/actionmailbox/lib/action_mailbox/router.rb index 54982eae21..9ac58e5b3f 100644 --- a/actionmailbox/lib/action_mailbox/router.rb +++ b/actionmailbox/lib/action_mailbox/router.rb @@ -31,7 +31,7 @@ module ActionMailbox end def mailbox_for(inbound_email) - routes.detect { |route| route.match?(inbound_email) }.try(:mailbox_class) + routes.detect { |route| route.match?(inbound_email) }&.mailbox_class end private diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index a6223964e5..6fa5be54cb 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -268,14 +268,14 @@ class BaseTest < ActiveSupport::TestCase end test "accessing inline attachments after mail was called works" do - class LateInlineAttachmentMailer < ActionMailer::Base + class LateInlineAttachmentAccessorMailer < ActionMailer::Base def welcome mail body: "yay", from: "welcome@example.com", to: "to@example.com" attachments.inline["invoice.pdf"] end end - assert_nothing_raised { LateInlineAttachmentMailer.welcome.message } + assert_nothing_raised { LateInlineAttachmentAccessorMailer.welcome.message } end test "adding inline attachments while rendering mail works" do diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 29d1919ec5..967bda38f2 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" +require "active_support/core_ext/integer/time" + module ActionController module ConditionalGet extend ActiveSupport::Concern @@ -17,7 +20,7 @@ module ActionController # of cached pages. # # class InvoicesController < ApplicationController - # etag { current_user.try :id } + # etag { current_user&.id } # # def show # # Etag will differ even for the same invoice when it's viewed by a different current_user diff --git a/actionpack/lib/action_controller/metal/content_security_policy.rb b/actionpack/lib/action_controller/metal/content_security_policy.rb index ebd90f07c8..25fc110bfe 100644 --- a/actionpack/lib/action_controller/metal/content_security_policy.rb +++ b/actionpack/lib/action_controller/metal/content_security_policy.rb @@ -45,7 +45,7 @@ module ActionController #:nodoc: end def current_content_security_policy - request.content_security_policy.try(:clone) || ActionDispatch::ContentSecurityPolicy.new + request.content_security_policy&.clone || ActionDispatch::ContentSecurityPolicy.new end end end diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index a251c29d23..660aef4106 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -163,18 +163,18 @@ module ActionController "/**/#{options[:callback]}(#{json})" else - self.content_type ||= Mime[:json] + self.content_type = Mime[:json] if media_type.nil? json end end add :js do |js, options| - self.content_type ||= Mime[:js] + self.content_type = Mime[:js] if media_type.nil? js.respond_to?(:to_js) ? js.to_js(options) : js end add :xml do |xml, options| - self.content_type ||= Mime[:xml] + self.content_type = Mime[:xml] if media_type.nil? xml.respond_to?(:to_xml) ? xml.to_xml(options) : xml end end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index e923afb17c..31df6cea0f 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -280,7 +280,7 @@ module ActionController #:nodoc: # Check for cross-origin JavaScript responses. def non_xhr_javascript_response? # :doc: - %r(\A(?:text|application)/javascript).match?(content_type) && !request.xhr? + %r(\A(?:text|application)/javascript).match?(media_type) && !request.xhr? end AUTHENTICITY_TOKEN_LENGTH = 32 diff --git a/actionpack/lib/action_dispatch/http/feature_policy.rb b/actionpack/lib/action_dispatch/http/feature_policy.rb index 78e982918d..c09690f9fc 100644 --- a/actionpack/lib/action_dispatch/http/feature_policy.rb +++ b/actionpack/lib/action_dispatch/http/feature_policy.rb @@ -42,7 +42,7 @@ module ActionDispatch #:nodoc: end def policy_empty?(policy) - policy.try(:directives) && policy.directives.empty? + policy&.directives&.empty? end end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 96bdf570af..9d94d94ffb 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -532,9 +532,13 @@ module ActionDispatch if value case when needs_migration?(value) - self[name] = Marshal.load(value) + Marshal.load(value).tap do |v| + self[name] = { value: v } + end when rotate - self[name] = serializer.load(value) + serializer.load(value).tap do |v| + self[name] = { value: v } + end else serializer.load(value) end diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb index e861e52f09..91f332be13 100644 --- a/actionpack/lib/action_dispatch/system_testing/browser.rb +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -47,14 +47,14 @@ module ActionDispatch case type when :chrome if ::Selenium::WebDriver::Service.respond_to? :driver_path= - ::Selenium::WebDriver::Chrome::Service.driver_path.try(:call) + ::Selenium::WebDriver::Chrome::Service.driver_path&.call else # Selenium <= v3.141.0 ::Selenium::WebDriver::Chrome.driver_path end when :firefox if ::Selenium::WebDriver::Service.respond_to? :driver_path= - ::Selenium::WebDriver::Firefox::Service.driver_path.try(:call) + ::Selenium::WebDriver::Firefox::Service.driver_path&.call else # Selenium <= v3.141.0 ::Selenium::WebDriver::Firefox.driver_path diff --git a/actionpack/test/controller/render_js_test.rb b/actionpack/test/controller/render_js_test.rb index da8f6e8062..02bc0c6ade 100644 --- a/actionpack/test/controller/render_js_test.rb +++ b/actionpack/test/controller/render_js_test.rb @@ -32,4 +32,13 @@ class RenderJSTest < ActionController::TestCase get :show_partial, format: "js", xhr: true assert_equal "partial js", @response.body end + + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_vanilla_js_hello, xhr: true } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end end diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index 82c6aaafe5..0045d2be34 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -133,4 +133,22 @@ class RenderJsonTest < ActionController::TestCase get :render_json_without_options assert_equal '{"a":"b"}', @response.body end + + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_json_hello_world } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end + + def test_should_not_trigger_content_type_deprecation_with_callback + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_json_hello_world_with_callback, xhr: true } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end end diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 28d8e281ab..16da41e7c7 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -98,4 +98,13 @@ class RenderXmlTest < ActionController::TestCase get :implicit_content_type, format: "atom" assert_equal Mime[:atom], @response.media_type end + + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_with_to_xml } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 01250880f5..145d04f5be 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -591,6 +591,15 @@ module RequestForgeryProtectionTests end end + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :same_origin_js, xhr: true } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end + def test_should_not_raise_error_if_token_is_not_a_string assert_blocked do patch :index, params: { custom_authenticity_token: { foo: "bar" } } diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index d129fa717d..e4d4792de6 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -893,6 +893,19 @@ class CookiesTest < ActionController::TestCase assert_equal 45, encryptor.decrypt_and_verify(@response.cookies["foo"]) end + def test_cookie_with_hash_value_not_modified_by_rotation + @request.env["action_dispatch.signed_cookie_digest"] = "SHA256" + @request.env["action_dispatch.cookies_rotations"].rotate :signed, digest: "SHA1" + + key_generator = @request.env["action_dispatch.key_generator"] + old_secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) + old_value = ActiveSupport::MessageVerifier.new(old_secret).generate(bar: "baz") + + @request.headers["Cookie"] = "foo=#{old_value}" + get :get_signed_cookie + assert_equal({ bar: "baz" }, @controller.send(:cookies).signed[:foo]) + end + def test_cookie_with_all_domain_option get :set_cookie_with_domain assert_response :success diff --git a/actiontext/app/helpers/action_text/content_helper.rb b/actiontext/app/helpers/action_text/content_helper.rb index ed2887d865..1e05f572f7 100644 --- a/actiontext/app/helpers/action_text/content_helper.rb +++ b/actiontext/app/helpers/action_text/content_helper.rb @@ -4,7 +4,7 @@ require "rails-html-sanitizer" module ActionText module ContentHelper - mattr_accessor(:sanitizer) { Rails::Html::Sanitizer.white_list_sanitizer.new } + mattr_accessor(:sanitizer) { Rails::Html::Sanitizer.safe_list_sanitizer.new } mattr_accessor(:allowed_tags) { sanitizer.class.allowed_tags + [ ActionText::Attachment::TAG_NAME, "figure", "figcaption" ] } mattr_accessor(:allowed_attributes) { sanitizer.class.allowed_attributes + ActionText::Attachment::ATTRIBUTES } mattr_accessor(:scrubber) diff --git a/actiontext/app/helpers/action_text/tag_helper.rb b/actiontext/app/helpers/action_text/tag_helper.rb index 1dc6202ae1..fe40be74f5 100644 --- a/actiontext/app/helpers/action_text/tag_helper.rb +++ b/actiontext/app/helpers/action_text/tag_helper.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" require "action_view/helpers/tags/placeholderable" module ActionText diff --git a/actiontext/lib/action_text/attachment.rb b/actiontext/lib/action_text/attachment.rb index e90a3e7d48..4bd537c7c2 100644 --- a/actiontext/lib/action_text/attachment.rb +++ b/actiontext/lib/action_text/attachment.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + module ActionText class Attachment include Attachments::TrixConversion, Attachments::Minification, Attachments::Caching diff --git a/actiontext/lib/action_text/attachments/trix_conversion.rb b/actiontext/lib/action_text/attachments/trix_conversion.rb index 24937d6c22..15319f4e37 100644 --- a/actiontext/lib/action_text/attachments/trix_conversion.rb +++ b/actiontext/lib/action_text/attachments/trix_conversion.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + module ActionText module Attachments module TrixConversion diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index ca3ce1476a..71a8b3fdcb 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,7 @@ +* ActionView::Helpers::SanitizeHelper: support rails-html-sanitizer 1.1.0. + + *Juanito Fatas* + * Added `phone_to` helper method to create a link from mobile numbers *Pietro Moro* diff --git a/actionview/lib/action_view/helpers/sanitize_helper.rb b/actionview/lib/action_view/helpers/sanitize_helper.rb index f4fa133f55..a4d796d138 100644 --- a/actionview/lib/action_view/helpers/sanitize_helper.rb +++ b/actionview/lib/action_view/helpers/sanitize_helper.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/object/try" require "rails-html-sanitizer" module ActionView @@ -17,7 +16,7 @@ module ActionView # ASCII, and hex character references to work around these protocol filters. # All special characters will be escaped. # - # The default sanitizer is Rails::Html::WhiteListSanitizer. See {Rails HTML + # The default sanitizer is Rails::Html::SafeListSanitizer. See {Rails HTML # Sanitizers}[https://github.com/rails/rails-html-sanitizer] for more information. # # Custom sanitization rules can also be provided. @@ -80,12 +79,12 @@ module ActionView # config.action_view.sanitized_allowed_tags = ['strong', 'em', 'a'] # config.action_view.sanitized_allowed_attributes = ['href', 'title'] def sanitize(html, options = {}) - self.class.white_list_sanitizer.sanitize(html, options).try(:html_safe) + self.class.safe_list_sanitizer.sanitize(html, options)&.html_safe end # Sanitizes a block of CSS code. Used by +sanitize+ when it comes across a style attribute. def sanitize_css(style) - self.class.white_list_sanitizer.sanitize_css(style) + self.class.safe_list_sanitizer.sanitize_css(style) end # Strips all HTML tags from +html+, including comments and special characters. @@ -123,20 +122,14 @@ module ActionView end module ClassMethods #:nodoc: - attr_writer :full_sanitizer, :link_sanitizer, :white_list_sanitizer - - # Vendors the full, link and white list sanitizers. - # Provided strictly for compatibility and can be removed in Rails 6. - def sanitizer_vendor - Rails::Html::Sanitizer - end + attr_writer :full_sanitizer, :link_sanitizer, :safe_list_sanitizer def sanitized_allowed_tags - sanitizer_vendor.white_list_sanitizer.allowed_tags + safe_list_sanitizer.allowed_tags end def sanitized_allowed_attributes - sanitizer_vendor.white_list_sanitizer.allowed_attributes + safe_list_sanitizer.allowed_attributes end # Gets the Rails::Html::FullSanitizer instance used by +strip_tags+. Replace with @@ -145,9 +138,8 @@ module ActionView # class Application < Rails::Application # config.action_view.full_sanitizer = MySpecialSanitizer.new # end - # def full_sanitizer - @full_sanitizer ||= sanitizer_vendor.full_sanitizer.new + @full_sanitizer ||= Rails::Html::Sanitizer.full_sanitizer.new end # Gets the Rails::Html::LinkSanitizer instance used by +strip_links+. @@ -156,20 +148,18 @@ module ActionView # class Application < Rails::Application # config.action_view.link_sanitizer = MySpecialSanitizer.new # end - # def link_sanitizer - @link_sanitizer ||= sanitizer_vendor.link_sanitizer.new + @link_sanitizer ||= Rails::Html::Sanitizer.link_sanitizer.new end - # Gets the Rails::Html::WhiteListSanitizer instance used by sanitize and +sanitize_css+. + # Gets the Rails::Html::SafeListSanitizer instance used by sanitize and +sanitize_css+. # Replace with any object that responds to +sanitize+. # # class Application < Rails::Application - # config.action_view.white_list_sanitizer = MySpecialSanitizer.new + # config.action_view.safe_list_sanitizer = MySpecialSanitizer.new # end - # - def white_list_sanitizer - @white_list_sanitizer ||= sanitizer_vendor.white_list_sanitizer.new + def safe_list_sanitizer + @safe_list_sanitizer ||= Rails::Html::Sanitizer.safe_list_sanitizer.new end end end diff --git a/actionview/lib/action_view/helpers/tags/date_field.rb b/actionview/lib/action_view/helpers/tags/date_field.rb index ceaabfa99c..9cdfc6991f 100644 --- a/actionview/lib/action_view/helpers/tags/date_field.rb +++ b/actionview/lib/action_view/helpers/tags/date_field.rb @@ -6,7 +6,7 @@ module ActionView class DateField < DatetimeField # :nodoc: private def format_date(value) - value.try(:strftime, "%Y-%m-%d") + value&.strftime("%Y-%m-%d") end end end diff --git a/actionview/lib/action_view/helpers/tags/datetime_local_field.rb b/actionview/lib/action_view/helpers/tags/datetime_local_field.rb index 8908bf9948..f0834ac6ce 100644 --- a/actionview/lib/action_view/helpers/tags/datetime_local_field.rb +++ b/actionview/lib/action_view/helpers/tags/datetime_local_field.rb @@ -12,7 +12,7 @@ module ActionView private def format_date(value) - value.try(:strftime, "%Y-%m-%dT%T") + value&.strftime("%Y-%m-%dT%T") end end end diff --git a/actionview/lib/action_view/helpers/tags/month_field.rb b/actionview/lib/action_view/helpers/tags/month_field.rb index 463866a181..b582bb4f79 100644 --- a/actionview/lib/action_view/helpers/tags/month_field.rb +++ b/actionview/lib/action_view/helpers/tags/month_field.rb @@ -6,7 +6,7 @@ module ActionView class MonthField < DatetimeField # :nodoc: private def format_date(value) - value.try(:strftime, "%Y-%m") + value&.strftime("%Y-%m") end end end diff --git a/actionview/lib/action_view/helpers/tags/time_field.rb b/actionview/lib/action_view/helpers/tags/time_field.rb index e74c578db9..e5e0b84891 100644 --- a/actionview/lib/action_view/helpers/tags/time_field.rb +++ b/actionview/lib/action_view/helpers/tags/time_field.rb @@ -6,7 +6,7 @@ module ActionView class TimeField < DatetimeField # :nodoc: private def format_date(value) - value.try(:strftime, "%T.%L") + value&.strftime("%T.%L") end end end diff --git a/actionview/lib/action_view/helpers/tags/week_field.rb b/actionview/lib/action_view/helpers/tags/week_field.rb index 5a403ed91d..7828a3149f 100644 --- a/actionview/lib/action_view/helpers/tags/week_field.rb +++ b/actionview/lib/action_view/helpers/tags/week_field.rb @@ -6,7 +6,7 @@ module ActionView class WeekField < DatetimeField # :nodoc: private def format_date(value) - value.try(:strftime, "%Y-W%V") + value&.strftime("%Y-W%V") end end end diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 1b05d4aa71..61ab3c2e13 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -45,7 +45,7 @@ module ActionView def _back_url # :nodoc: _filtered_referrer || "javascript:history.back()" end - protected :_back_url + private :_back_url def _filtered_referrer # :nodoc: if controller.respond_to?(:request) @@ -56,7 +56,7 @@ module ActionView end rescue URI::InvalidURIError end - protected :_filtered_referrer + private :_filtered_referrer # Creates an anchor element of the given +name+ using a URL created by the set of +options+. # See the valid options in the documentation for +url_for+. It's also possible to diff --git a/actionview/lib/action_view/layouts.rb b/actionview/lib/action_view/layouts.rb index 74e8e80e07..b21dc1b9b3 100644 --- a/actionview/lib/action_view/layouts.rb +++ b/actionview/lib/action_view/layouts.rb @@ -306,7 +306,7 @@ module ActionView RUBY when Proc define_method :_layout_from_proc, &_layout - protected :_layout_from_proc + private :_layout_from_proc <<-RUBY result = _layout_from_proc(#{_layout.arity == 0 ? '' : 'self'}) return #{default_behavior} if result.nil? diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index 08a280c7ee..5942717b8d 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -62,7 +62,7 @@ module ActionView output = ActionView::StreamingBuffer.new(buffer) yielder = lambda { |*name| view._layout_for(*name) } - instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do + instrument(:template, identifier: template.identifier, layout: (layout && layout.virtual_path)) do outer_config = I18n.config fiber = Fiber.new do I18n.config = outer_config diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index b2d7332572..da0b2dc0d2 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/object/try" - module ActionView class TemplateRenderer < AbstractRenderer #:nodoc: def render(context, options) @@ -54,7 +52,7 @@ module ActionView # supplied as well. def render_template(view, template, layout_name, locals) render_with_layout(view, template, layout_name, locals) do |layout| - instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do + instrument(:template, identifier: template.identifier, layout: (layout && layout.virtual_path)) do template.render(view, locals) { |*name| view._layout_for(*name) } end end diff --git a/actionview/test/template/date_helper_i18n_test.rb b/actionview/test/template/date_helper_i18n_test.rb index 60303b4c91..f100a011a8 100644 --- a/actionview/test/template/date_helper_i18n_test.rb +++ b/actionview/test/template/date_helper_i18n_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "abstract_unit" +require "active_support/core_ext/integer/time" class DateHelperDistanceOfTimeInWordsI18nTests < ActiveSupport::TestCase include ActionView::Helpers::DateHelper diff --git a/actionview/test/template/date_helper_test.rb b/actionview/test/template/date_helper_test.rb index 0a294ec674..ca904a7b1e 100644 --- a/actionview/test/template/date_helper_test.rb +++ b/actionview/test/template/date_helper_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "abstract_unit" +require "active_support/core_ext/integer/time" class DateHelperTest < ActionView::TestCase tests ActionView::Helpers::DateHelper diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index 283125698d..5b10858d80 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -100,7 +100,7 @@ module ActiveJob "executions" => executions, "exception_executions" => exception_executions, "locale" => I18n.locale.to_s, - "timezone" => Time.zone.try(:name), + "timezone" => Time.zone&.name, "enqueued_at" => Time.now.utc.iso8601 } end @@ -140,7 +140,7 @@ module ActiveJob self.executions = job_data["executions"] 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.timezone = job_data["timezone"] || Time.zone&.name self.enqueued_at = job_data["enqueued_at"] end diff --git a/activejob/test/jobs/timezone_dependent_job.rb b/activejob/test/jobs/timezone_dependent_job.rb index f63ff95b95..358c3aa859 100644 --- a/activejob/test/jobs/timezone_dependent_job.rb +++ b/activejob/test/jobs/timezone_dependent_job.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "../support/job_buffer" +require "active_support/time" class TimezoneDependentJob < ActiveJob::Base def perform(now) diff --git a/activejob/test/support/integration/dummy_app_template.rb b/activejob/test/support/integration/dummy_app_template.rb index b56dd3e591..65229ba397 100644 --- a/activejob/test/support/integration/dummy_app_template.rb +++ b/activejob/test/support/integration/dummy_app_template.rb @@ -21,7 +21,7 @@ class TestJob < ActiveJob::Base File.open(Rails.root.join("tmp/\#{x}.new"), "wb+") do |f| f.write Marshal.dump({ "locale" => I18n.locale.to_s || "en", - "timezone" => Time.zone.try(:name) || "UTC", + "timezone" => Time.zone&.name || "UTC", "executed_at" => Time.now.to_r }) end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 9d77564c61..c301f4766d 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,9 @@ +* Add *_previously_was attribute methods when dirty tracking. Example: + pirate.update(catchphrase: "Ahoy!") + pirate.previous_changes["catchphrase"] # => ["Thar She Blows!", "Ahoy!"] + pirate.catchphrase_previously_was # => "Thar She Blows!" + + *DHH* Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/activemodel/CHANGELOG.md) for previous changes. diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index aaefe00c83..245616502d 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -84,6 +84,7 @@ module ActiveModel # person.previous_changes # => {"name" => [nil, "Bill"]} # person.name_previously_changed? # => true # person.name_previous_change # => [nil, "Bill"] + # person.name_previously_was # => nil # person.reload! # person.previous_changes # => {} # @@ -122,7 +123,7 @@ module ActiveModel included do attribute_method_suffix "_changed?", "_change", "_will_change!", "_was" - attribute_method_suffix "_previously_changed?", "_previous_change" + attribute_method_suffix "_previously_changed?", "_previous_change", "_previously_was" attribute_method_affix prefix: "restore_", suffix: "!" end @@ -180,6 +181,11 @@ module ActiveModel mutations_before_last_save.changed?(attr_name.to_s) end + # Dispatch target for <tt>*_previously_was</tt> attribute methods. + def attribute_previously_was(attr_name) # :nodoc: + mutations_before_last_save.original_value(attr_name.to_s) + end + # Restore all previous data of the provided attributes. def restore_attributes(attr_names = changed) attr_names.each { |attr_name| restore_attribute!(attr_name) } diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 5f409326bd..c177d771ad 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -45,19 +45,19 @@ module ActiveModel # end # # user = User.new(name: 'david', password: '', password_confirmation: 'nomatch') - # user.save # => false, password required + # user.save # => false, password required # user.password = 'mUc3m00RsqyRe' - # user.save # => false, confirmation doesn't match + # user.save # => false, confirmation doesn't match # user.password_confirmation = 'mUc3m00RsqyRe' - # user.save # => true + # user.save # => true # user.recovery_password = "42password" - # user.recovery_password_digest # => "$2a$04$iOfhwahFymCs5weB3BNH/uXkTG65HR.qpW.bNhEjFP3ftli3o5DQC" - # user.save # => true - # user.authenticate('notright') # => false - # user.authenticate('mUc3m00RsqyRe') # => user - # user.authenticate_recovery_password('42password') # => user - # User.find_by(name: 'david').try(:authenticate, 'notright') # => false - # User.find_by(name: 'david').try(:authenticate, 'mUc3m00RsqyRe') # => user + # user.recovery_password_digest # => "$2a$04$iOfhwahFymCs5weB3BNH/uXkTG65HR.qpW.bNhEjFP3ftli3o5DQC" + # user.save # => true + # user.authenticate('notright') # => false + # user.authenticate('mUc3m00RsqyRe') # => user + # user.authenticate_recovery_password('42password') # => user + # User.find_by(name: 'david')&.authenticate('notright') # => false + # User.find_by(name: 'david')&.authenticate('mUc3m00RsqyRe') # => user def has_secure_password(attribute = :password, validations: true) # Load bcrypt gem only when has_secure_password is used. # This is to avoid ActiveModel (and by extension the entire framework) diff --git a/activemodel/lib/active_model/type/float.rb b/activemodel/lib/active_model/type/float.rb index 36c7a5103c..435e39b2c9 100644 --- a/activemodel/lib/active_model/type/float.rb +++ b/activemodel/lib/active_model/type/float.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + module ActiveModel module Type class Float < Value # :nodoc: diff --git a/activemodel/test/cases/helper.rb b/activemodel/test/cases/helper.rb index a4cb472ffc..c6335ac9e3 100644 --- a/activemodel/test/cases/helper.rb +++ b/activemodel/test/cases/helper.rb @@ -10,6 +10,7 @@ I18n.enforce_available_locales = false require "active_support/testing/autorun" require "active_support/testing/method_call_assertions" +require "active_support/core_ext/integer/time" class ActiveModel::TestCase < ActiveSupport::TestCase include ActiveSupport::Testing::MethodCallAssertions diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index daad76759f..6669d028a0 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "cases/helper" -require "active_support/all" require "models/topic" require "models/person" diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 3346725f2d..e3886f394a 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -44,7 +44,7 @@ module ActiveRecord def decrement_counters_before_last_save if reflection.polymorphic? - model_was = owner.attribute_before_last_save(reflection.foreign_type).try(:constantize) + model_was = owner.attribute_before_last_save(reflection.foreign_type)&.constantize else model_was = klass end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index fb44232dff..9a4de1a034 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + module ActiveRecord module AttributeMethods module TimeZoneConversion diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 36001efdd5..276d5a25d4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1020,8 +1020,8 @@ module ActiveRecord # In some cases you may want to prevent writes to the database # even if you are on a database that can write. `while_preventing_writes` # will prevent writes to the database for the duration of the block. - def while_preventing_writes - original, @prevent_writes = @prevent_writes, true + def while_preventing_writes(enabled = true) + original, @prevent_writes = @prevent_writes, enabled yield ensure @prevent_writes = original diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index dbd533b4b3..aedd8119ef 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -221,7 +221,7 @@ module ActiveRecord end class_methods do - private def define_column_methods(*column_types) # :nodoc: + def define_column_methods(*column_types) # :nodoc: column_types.each do |column_type| module_eval <<-RUBY, __FILE__, __LINE__ + 1 def #{column_type}(*names, **options) @@ -231,6 +231,7 @@ module ActiveRecord RUBY end end + private :define_column_methods end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb b/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb index 8df91c988b..75213e964b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb @@ -24,7 +24,7 @@ WARNING: Rails was not able to disable referential integrity. This is most likely caused due to missing permissions. Rails needs superuser privileges to disable referential integrity. - cause: #{original_exception.try(:message)} + cause: #{original_exception&.message} WARNING raise e diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index f33ba87435..d6758c9d4b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -12,6 +12,7 @@ class ::PG::Connection # :nodoc: end end +require "active_support/core_ext/object/try" require "active_record/connection_adapters/abstract_adapter" require "active_record/connection_adapters/statement_pool" require "active_record/connection_adapters/postgresql/column" diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 595ef4ee25..1b2c9c6754 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -45,7 +45,7 @@ module ActiveRecord # #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10 @env_name="development", # @spec_name="primary", @config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>, # #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbdea90 @env_name="production", - # @spec_name="primary", @config={"adapter"=>"mysql2", "database"=>"db/production.sqlite3"}> + # @spec_name="primary", @config={"adapter"=>"sqlite3", "database"=>"db/production.sqlite3"}> # ]> def self.configurations=(config) @@configurations = ActiveRecord::DatabaseConfigurations.new(config) diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index fcf13a910c..268be34b21 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -9,6 +9,8 @@ module ActiveRecord # objects (either a HashConfig or UrlConfig) that are constructed from the # application's database configuration hash or URL string. class DatabaseConfigurations + class InvalidConfigurationError < StandardError; end + attr_reader :configurations delegate :any?, to: :configurations @@ -146,17 +148,19 @@ module ActiveRecord build_db_config_from_string(env_name, spec_name, config) when Hash build_db_config_from_hash(env_name, spec_name, config.stringify_keys) + else + raise InvalidConfigurationError, "'{ #{env_name} => #{config} }' is not a valid configuration. Expected '#{config}' to be a URL string or a Hash." end end def build_db_config_from_string(env_name, spec_name, config) url = config uri = URI.parse(url) - if uri.try(:scheme) + if uri.scheme ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url) + else + raise InvalidConfigurationError, "'{ #{env_name} => #{config} }' is not a valid configuration. Expected '#{config}' to be a URL string or a Hash." end - rescue URI::InvalidURIError - ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config) end def build_db_config_from_hash(env_name, spec_name, config) diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 509f21c9a5..0f110b4536 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -106,7 +106,7 @@ module ActiveRecord # Wraps the underlying database error as +cause+. class StatementInvalid < ActiveRecordError def initialize(message = nil, sql: nil, binds: nil) - super(message || $!.try(:message)) + super(message || $!&.message) @sql = sql @binds = binds end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 3df4b3c87f..e4b958f1b9 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -553,15 +553,15 @@ module ActiveRecord end end - def create_fixtures(fixtures_directory, fixture_set_names, class_names = {}, config = ActiveRecord::Base) + def create_fixtures(fixtures_directory, fixture_set_names, class_names = {}, config = ActiveRecord::Base, &block) fixture_set_names = Array(fixture_set_names).map(&:to_s) class_names = ClassCache.new class_names, config # FIXME: Apparently JK uses this. - connection = block_given? ? yield : ActiveRecord::Base.connection + connection = block_given? ? block : lambda { ActiveRecord::Base.connection } fixture_files_to_read = fixture_set_names.reject do |fs_name| - fixture_is_cached?(connection, fs_name) + fixture_is_cached?(connection.call, fs_name) end if fixture_files_to_read.any? @@ -571,9 +571,9 @@ module ActiveRecord class_names, connection, ) - cache_fixtures(connection, fixtures_map) + cache_fixtures(connection.call, fixtures_map) end - cached_fixtures(connection, fixture_set_names) + cached_fixtures(connection.call, fixture_set_names) end # Returns a consistent, platform-independent identifier for +label+. @@ -612,7 +612,11 @@ module ActiveRecord def insert(fixture_sets, connection) # :nodoc: fixture_sets_by_connection = fixture_sets.group_by do |fixture_set| - fixture_set.model_class&.connection || connection + if fixture_set.model_class + fixture_set.model_class.connection + else + connection.call + end end fixture_sets_by_connection.each do |conn, set| @@ -623,6 +627,7 @@ module ActiveRecord table_rows_for_connection[table].unshift(*rows) end end + conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys) # Cap primary key sequences to max(pk). diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index f6577dcbc4..3863b6bfea 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -14,7 +14,7 @@ module ActiveRecord @returning = (connection.supports_insert_returning? ? primary_keys : false) if @returning.nil? @returning = false if @returning == [] - @unique_by = find_unique_index_for(unique_by) if unique_by + @unique_by = find_unique_index_for(unique_by || model.primary_key) @on_duplicate = :skip if @on_duplicate == :update && updatable_columns.empty? ensure_valid_options_for_connection! @@ -32,7 +32,7 @@ module ActiveRecord end def primary_keys - Array(model.primary_key) + Array(connection.schema_cache.primary_keys(model.table_name)) end @@ -61,6 +61,8 @@ module ActiveRecord if index = unique_indexes.find { |i| match.include?(i.name) || i.columns == match } index + elsif match == primary_keys + nil else raise ArgumentError, "No unique index found for #{unique_by}" end diff --git a/activerecord/lib/active_record/middleware/database_selector/resolver.rb b/activerecord/lib/active_record/middleware/database_selector/resolver.rb index 3eb1039c50..b2aa453a8e 100644 --- a/activerecord/lib/active_record/middleware/database_selector/resolver.rb +++ b/activerecord/lib/active_record/middleware/database_selector/resolver.rb @@ -46,7 +46,7 @@ module ActiveRecord private def read_from_primary(&blk) ActiveRecord::Base.connected_to(role: ActiveRecord::Base.writing_role) do - ActiveRecord::Base.connection_handler.while_preventing_writes do + ActiveRecord::Base.connection_handler.while_preventing_writes(true) do instrumenter.instrument("database_selector.active_record.read_from_primary") do yield end @@ -64,10 +64,12 @@ module ActiveRecord def write_to_primary(&blk) ActiveRecord::Base.connected_to(role: ActiveRecord::Base.writing_role) do - instrumenter.instrument("database_selector.active_record.wrote_to_primary") do - yield - ensure - context.update_last_write_timestamp + ActiveRecord::Base.connection_handler.while_preventing_writes(false) do + instrumenter.instrument("database_selector.active_record.wrote_to_primary") do + yield + ensure + context.update_last_write_timestamp + end end end end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index d5375390c7..f06a98bd98 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -2,6 +2,7 @@ require "active_record" require "rails" +require "active_support/core_ext/object/try" require "active_model/railtie" # For now, action_controller must always be present with diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 4d9acc911b..d699e2e21b 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -265,6 +265,8 @@ db_namespace = namespace :db do end abort %{Run `rails db:migrate` to update your database then try again.} end + ensure + ActiveRecord::Base.establish_connection(ActiveRecord::Tasks::DatabaseTasks.env.to_sym) end namespace :abort_if_pending_migrations do diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 29e2e2cedc..2876bae302 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1114,7 +1114,7 @@ module ActiveRecord else join end - end.compact_blank! + end.compact_blank!.uniq! while joins.first.is_a?(Arel::Nodes::Join) join_node = joins.shift @@ -1144,8 +1144,8 @@ module ActiveRecord def build_join_query(manager, buckets, join_type, aliases) association_joins = buckets[:association_join] stashed_joins = buckets[:stashed_join] - leading_joins = buckets[:leading_join].tap(&:uniq!) - join_nodes = buckets[:join_node].tap(&:uniq!) + leading_joins = buckets[:leading_join] + join_nodes = buckets[:join_node] join_sources = manager.join_sources join_sources.concat(leading_joins) unless leading_joins.empty? diff --git a/activerecord/lib/active_record/serialization.rb b/activerecord/lib/active_record/serialization.rb index 741fea43ce..9fc62a99f8 100644 --- a/activerecord/lib/active_record/serialization.rb +++ b/activerecord/lib/active_record/serialization.rb @@ -11,7 +11,7 @@ module ActiveRecord #:nodoc: end def serializable_hash(options = nil) - options = options.try(:dup) || {} + options = options ? options.dup : {} options[:except] = Array(options[:except]).map(&:to_s) options[:except] |= Array(self.class.inheritance_column) diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index b65af4b819..166a59ec7b 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -69,6 +69,16 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase assert_equal [expected], Person.joins(string_join).joins(agents.create_join(agents, agents.create_on(constraint))) end + def test_deduplicate_joins + posts = Post.arel_table + constraint = posts[:author_id].eq(Author.arel_attribute(:id)) + + authors = Author.joins(posts.create_join(posts, posts.create_on(constraint))) + authors = authors.joins(:author_address).merge(authors.where("posts.type": "SpecialPost")) + + assert_equal [authors(:david)], authors + end + def test_construct_finder_sql_ignores_empty_joins_hash sql = Author.joins({}).to_sql assert_no_match(/JOIN/i, sql) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index dbd1d03c4c..abce4565a4 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -952,6 +952,90 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal({ "proposed" => 2, "published" => 2 }, Book.group(:status).count) end + def test_select_avg_with_group_by_as_virtual_attribute_with_sql + rails_core = companies(:rails_core) + + sql = <<~SQL + SELECT firm_id, AVG(credit_limit) AS avg_credit_limit + FROM accounts + WHERE firm_id = ? + GROUP BY firm_id + LIMIT 1 + SQL + + account = Account.find_by_sql([sql, rails_core]).first + + # id was not selected, so it should be nil + # (cannot select id because it wasn't used in the GROUP BY clause) + assert_nil account.id + + # firm_id was explicitly selected, so it should be present + assert_equal(rails_core, account.firm) + + # avg_credit_limit should be present as a virtual attribute + assert_equal(52.5, account.avg_credit_limit) + end + + def test_select_avg_with_group_by_as_virtual_attribute_with_ar + rails_core = companies(:rails_core) + + account = Account + .select(:firm_id, "AVG(credit_limit) AS avg_credit_limit") + .where(firm: rails_core) + .group(:firm_id) + .take! + + # id was not selected, so it should be nil + # (cannot select id because it wasn't used in the GROUP BY clause) + assert_nil account.id + + # firm_id was explicitly selected, so it should be present + assert_equal(rails_core, account.firm) + + # avg_credit_limit should be present as a virtual attribute + assert_equal(52.5, account.avg_credit_limit) + end + + def test_select_avg_with_joins_and_group_by_as_virtual_attribute_with_sql + rails_core = companies(:rails_core) + + sql = <<~SQL + SELECT companies.*, AVG(accounts.credit_limit) AS avg_credit_limit + FROM companies + INNER JOIN accounts ON companies.id = accounts.firm_id + WHERE companies.id = ? + GROUP BY companies.id + LIMIT 1 + SQL + + firm = DependentFirm.find_by_sql([sql, rails_core]).first + + # all the DependentFirm attributes should be present + assert_equal rails_core, firm + assert_equal rails_core.name, firm.name + + # avg_credit_limit should be present as a virtual attribute + assert_equal(52.5, firm.avg_credit_limit) + end + + def test_select_avg_with_joins_and_group_by_as_virtual_attribute_with_ar + rails_core = companies(:rails_core) + + firm = DependentFirm + .select("companies.*", "AVG(accounts.credit_limit) AS avg_credit_limit") + .where(id: rails_core) + .joins(:account) + .group(:id) + .take! + + # all the DependentFirm attributes should be present + assert_equal rails_core, firm + assert_equal rails_core.name, firm.name + + # avg_credit_limit should be present as a virtual attribute + assert_equal(52.5, firm.avg_credit_limit) + end + def test_count_with_block_and_column_name_raises_an_error assert_raises(ArgumentError) do Account.count(:firm_id) { true } 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 ee2972101f..2ac249b478 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 @@ -28,6 +28,22 @@ module ActiveRecord resolver.resolve(spec, spec) end + def test_invalid_string_config + config = { "foo" => "bar" } + + assert_raises ActiveRecord::DatabaseConfigurations::InvalidConfigurationError do + resolve_config(config) + end + end + + def test_invalid_symbol_config + config = { "foo" => :bar } + + assert_raises ActiveRecord::DatabaseConfigurations::InvalidConfigurationError do + resolve_config(config) + end + end + def test_resolver_with_database_uri_and_current_env_symbol_key ENV["DATABASE_URL"] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } diff --git a/activerecord/test/cases/database_selector_test.rb b/activerecord/test/cases/database_selector_test.rb index fd02d2acb4..340151e6db 100644 --- a/activerecord/test/cases/database_selector_test.rb +++ b/activerecord/test/cases/database_selector_test.rb @@ -123,6 +123,40 @@ module ActiveRecord assert read end + def test_preventing_writes_turns_off_for_primary_write + resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver.new(@session, delay: 5.seconds) + + # Session should start empty + assert_nil @session_store[:last_write] + + called = false + resolver.write do + assert ActiveRecord::Base.connected_to?(role: :writing) + called = true + end + assert called + + # and be populated by the last write time + assert @session_store[:last_write] + + read = false + write = false + resolver.read do + assert ActiveRecord::Base.connected_to?(role: :writing) + assert ActiveRecord::Base.connection_handler.prevent_writes + read = true + + resolver.write do + assert ActiveRecord::Base.connected_to?(role: :writing) + assert_not ActiveRecord::Base.connection_handler.prevent_writes + write = true + end + end + + assert write + assert read + end + def test_read_from_replica_with_no_delay resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver.new(@session, delay: 0.seconds) diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index a2a501a794..38baf0509c 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -491,6 +491,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 4, pirate.previous_changes.size assert_equal [nil, "arrr"], pirate.previous_changes["catchphrase"] + assert_nil pirate.catchphrase_previously_was assert_equal [nil, pirate.id], pirate.previous_changes["id"] assert_nil pirate.previous_changes["updated_on"][0] assert_not_nil pirate.previous_changes["updated_on"][1] @@ -507,6 +508,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 4, pirate.previous_changes.size assert_equal [nil, "arrr"], pirate.previous_changes["catchphrase"] + assert_nil pirate.catchphrase_previously_was assert_equal [nil, pirate.id], pirate.previous_changes["id"] assert_includes pirate.previous_changes, "updated_on" assert_includes pirate.previous_changes, "created_on" @@ -525,6 +527,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 2, pirate.previous_changes.size assert_equal ["arrr", "Me Maties!"], pirate.previous_changes["catchphrase"] + assert_equal "arrr", pirate.catchphrase_previously_was assert_not_nil pirate.previous_changes["updated_on"][0] assert_not_nil pirate.previous_changes["updated_on"][1] assert_not pirate.previous_changes.key?("parrot_id") @@ -539,6 +542,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 2, pirate.previous_changes.size assert_equal ["Me Maties!", "Thar She Blows!"], pirate.previous_changes["catchphrase"] + assert_equal "Me Maties!", pirate.catchphrase_previously_was assert_not_nil pirate.previous_changes["updated_on"][0] assert_not_nil pirate.previous_changes["updated_on"][1] assert_not pirate.previous_changes.key?("parrot_id") @@ -551,6 +555,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 2, pirate.previous_changes.size assert_equal ["Thar She Blows!", "Ahoy!"], pirate.previous_changes["catchphrase"] + assert_equal "Thar She Blows!", pirate.catchphrase_previously_was assert_not_nil pirate.previous_changes["updated_on"][0] assert_not_nil pirate.previous_changes["updated_on"][1] assert_not pirate.previous_changes.key?("parrot_id") @@ -563,6 +568,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 2, pirate.previous_changes.size assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes["catchphrase"] + assert_equal "Ahoy!", pirate.catchphrase_previously_was assert_not_nil pirate.previous_changes["updated_on"][0] assert_not_nil pirate.previous_changes["updated_on"][1] assert_not pirate.previous_changes.key?("parrot_id") diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index d086d77081..42c623fafb 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require "models/book" +require "models/speedometer" class ReadonlyNameBook < Book attr_readonly :name @@ -225,6 +226,23 @@ class InsertAllTest < ActiveRecord::TestCase assert_equal new_name, Book.find(1).name end + def test_upsert_all_updates_existing_record_by_primary_key + skip unless supports_insert_on_duplicate_update? + + Book.upsert_all [{ id: 1, name: "New edition" }], unique_by: :id + + assert_equal "New edition", Book.find(1).name + end + + def test_upsert_all_updates_existing_record_by_configured_primary_key + skip unless supports_insert_on_duplicate_update? + + error = assert_raises ArgumentError do + Speedometer.upsert_all [{ speedometer_id: "s1", name: "New Speedometer" }] + end + assert_match "No unique index found for speedometer_id", error.message + end + def test_upsert_all_does_not_update_readonly_attributes skip unless supports_insert_on_duplicate_update? diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 1a20fe5dc2..8b3ae02947 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1932,7 +1932,7 @@ class RelationTest < ActiveRecord::TestCase assert_no_queries do result = authors_count.map do |post| - [post.num_posts, post.author.try(:name)] + [post.num_posts, post.author&.name] end expected = [[1, nil], [5, "David"], [3, "Mary"], [2, "Bob"]] diff --git a/activestorage/app/jobs/active_storage/mirror_job.rb b/activestorage/app/jobs/active_storage/mirror_job.rb index e34faedb56..e70629d6ec 100644 --- a/activestorage/app/jobs/active_storage/mirror_job.rb +++ b/activestorage/app/jobs/active_storage/mirror_job.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + # Provides asynchronous mirroring of directly-uploaded blobs. class ActiveStorage::MirrorJob < ActiveStorage::BaseJob queue_as { ActiveStorage.queues[:mirror] } diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb index 06864a846f..4bd6b56b6c 100644 --- a/activestorage/lib/active_storage/attached/model.rb +++ b/activestorage/lib/active_storage/attached/model.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/try" + module ActiveStorage # Provides the class-level DSL for declaring an Active Record model's attachments. module Attached::Model diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index 8d77e9b20f..0648da70b5 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -17,10 +17,12 @@ module ActiveStorage @container = container end - def upload(key, io, checksum: nil, content_type: nil, **) + def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, **) instrument :upload, key: key, checksum: checksum do handle_errors do - blobs.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum, content_type: content_type) + content_disposition = content_disposition_with(filename: filename, type: disposition) if disposition && filename + + blobs.create_block_blob(container, key, IO.try_convert(io) || io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition) end end end diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index e4bd57048a..a73f6ab526 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -20,12 +20,14 @@ module ActiveStorage @upload_options = upload end - def upload(key, io, checksum: nil, content_type: nil, **) + def upload(key, io, checksum: nil, filename: nil, content_type: nil, disposition: nil, **) instrument :upload, key: key, checksum: checksum do + content_disposition = content_disposition_with(filename: filename, type: disposition) if disposition && filename + if io.size < multipart_upload_threshold - upload_with_single_part key, io, checksum: checksum, content_type: content_type + upload_with_single_part key, io, checksum: checksum, content_type: content_type, content_disposition: content_disposition else - upload_with_multipart key, io, content_type: content_type + upload_with_multipart key, io, content_type: content_type, content_disposition: content_disposition end end end @@ -103,16 +105,16 @@ module ActiveStorage MAXIMUM_UPLOAD_PARTS_COUNT = 10000 MINIMUM_UPLOAD_PART_SIZE = 5.megabytes - def upload_with_single_part(key, io, checksum: nil, content_type: nil) - object_for(key).put(body: io, content_md5: checksum, content_type: content_type, **upload_options) + def upload_with_single_part(key, io, checksum: nil, content_type: nil, content_disposition: nil) + object_for(key).put(body: io, content_md5: checksum, content_type: content_type, content_disposition: content_disposition, **upload_options) rescue Aws::S3::Errors::BadDigest raise ActiveStorage::IntegrityError end - def upload_with_multipart(key, io, content_type: nil) + def upload_with_multipart(key, io, content_type: nil, content_disposition: nil) part_size = [ io.size.fdiv(MAXIMUM_UPLOAD_PARTS_COUNT).ceil, MINIMUM_UPLOAD_PART_SIZE ].max - object_for(key).upload_stream(content_type: content_type, part_size: part_size, **upload_options) do |out| + object_for(key).upload_stream(content_type: content_type, content_disposition: content_disposition, part_size: part_size, **upload_options) do |out| IO.copy_stream(io, out) end end diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index fc7b86ccb0..9eea1b94c8 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -23,6 +23,21 @@ if SERVICE_CONFIGURATIONS[:azure] @service.delete key end + test "upload with content disposition" do + key = SecureRandom.base58(24) + data = "Foobar" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), disposition: :inline) + + assert_equal("inline; filename=\"test.txt\"; filename*=UTF-8''test.txt", @service.blobs.get_blob_properties(@service.container, key).properties[:content_disposition]) + + url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: nil, filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_match(/attachment;.*test\.html/, response["Content-Disposition"]) + ensure + @service.delete key + end + test "signed URL generation" do url = @service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png") diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index b9120770e6..3dd1b59681 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -77,6 +77,23 @@ if SERVICE_CONFIGURATIONS[:s3] @service.delete key end + test "upload with content disposition" do + key = SecureRandom.base58(24) + data = "Something else entirely!" + + @service.upload( + key, + StringIO.new(data), + checksum: Digest::MD5.base64digest(data), + filename: ActiveStorage::Filename.new("cool_data.txt"), + disposition: :attachment + ) + + assert_equal("attachment; filename=\"cool_data.txt\"; filename*=UTF-8''cool_data.txt", @service.bucket.object(key).content_disposition) + ensure + @service.delete key + end + test "uploading a large object in multiple parts" do service = build_service(upload: { multipart_threshold: 5.megabytes }) diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index ac38b9362c..164b0acd96 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -6,6 +6,7 @@ require_relative "dummy/config/environment.rb" require "bundler/setup" require "active_support" require "active_support/test_case" +require "active_support/core_ext/object/try" require "active_support/testing/autorun" require "active_storage/service/mirror_service" require "image_processing/mini_magick" diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index f20c7c92e6..dad7005eb1 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,11 @@ +* Allow initializing `thread_mattr_*` attributes via `:default` option + + class Scraper + thread_mattr_reader :client, default: Api::Client.new + end + + *Guilherme Mansur* + * Add `compact_blank` for those times when you want to remove #blank? values from an Enumerable (also `compact_blank!` on Hash, Array, ActionController::Parameters) diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index a5063d0784..a42553ae3a 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -7,6 +7,7 @@ require "active_support/core_ext/module/attribute_accessors" require "active_support/core_ext/numeric/bytes" require "active_support/core_ext/numeric/time" require "active_support/core_ext/object/to_param" +require "active_support/core_ext/object/try" require "active_support/core_ext/string/inflections" module ActiveSupport diff --git a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb index c7a2378e41..27cb47eb6e 100644 --- a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/core_ext/object/try" +require "active_support/core_ext/date_time/conversions" module DateAndTime module Calculations diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 5b48254646..cc42647879 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true require "active_support/xml_mini" -require "active_support/time" require "active_support/core_ext/object/blank" require "active_support/core_ext/object/to_param" require "active_support/core_ext/object/to_query" +require "active_support/core_ext/object/try" require "active_support/core_ext/array/wrap" require "active_support/core_ext/hash/reverse_merge" require "active_support/core_ext/string/inflections" diff --git a/activesupport/lib/active_support/core_ext/marshal.rb b/activesupport/lib/active_support/core_ext/marshal.rb index 0c72cd7b47..5ff0e34d82 100644 --- a/activesupport/lib/active_support/core_ext/marshal.rb +++ b/activesupport/lib/active_support/core_ext/marshal.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/string/inflections" + module ActiveSupport module MarshalWithAutoloading # :nodoc: def load(source, proc = nil) diff --git a/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb b/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb index a6e87aeb68..ea4034303e 100644 --- a/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb +++ b/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb @@ -33,7 +33,7 @@ class Module # end # # Current.new.user # => NoMethodError - def thread_mattr_reader(*syms, instance_reader: true, instance_accessor: true) # :nodoc: + def thread_mattr_reader(*syms, instance_reader: true, instance_accessor: true, default: nil) # :nodoc: syms.each do |sym| raise NameError.new("invalid attribute name: #{sym}") unless /^[_A-Za-z]\w*$/.match?(sym) @@ -52,6 +52,8 @@ class Module end EOS end + + Thread.current["attr_" + name + "_#{sym}"] = default unless default.nil? end end alias :thread_cattr_reader :thread_mattr_reader @@ -74,7 +76,7 @@ class Module # end # # Current.new.user = "DHH" # => NoMethodError - def thread_mattr_writer(*syms, instance_writer: true, instance_accessor: true) # :nodoc: + def thread_mattr_writer(*syms, instance_writer: true, instance_accessor: true, default: nil) # :nodoc: syms.each do |sym| raise NameError.new("invalid attribute name: #{sym}") unless /^[_A-Za-z]\w*$/.match?(sym) @@ -93,6 +95,8 @@ class Module end EOS end + + public_send("#{sym}=", default) unless default.nil? end end alias :thread_cattr_writer :thread_mattr_writer @@ -136,8 +140,8 @@ class Module # # Current.new.user = "DHH" # => NoMethodError # Current.new.user # => NoMethodError - def thread_mattr_accessor(*syms, instance_reader: true, instance_writer: true, instance_accessor: true) - thread_mattr_reader(*syms, instance_reader: instance_reader, instance_accessor: instance_accessor) + def thread_mattr_accessor(*syms, instance_reader: true, instance_writer: true, instance_accessor: true, default: nil) + thread_mattr_reader(*syms, instance_reader: instance_reader, instance_accessor: instance_accessor, default: default) thread_mattr_writer(*syms, instance_writer: instance_writer, instance_accessor: instance_accessor) end alias :thread_cattr_accessor :thread_mattr_accessor diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 94d1115ccf..33a17a7741 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/inflections" +require "active_support/core_ext/object/blank" module ActiveSupport # The Inflector transforms words from singular to plural, class names to table diff --git a/activesupport/lib/active_support/inflector/transliterate.rb b/activesupport/lib/active_support/inflector/transliterate.rb index 3ba2d93ed8..1899a1212d 100644 --- a/activesupport/lib/active_support/inflector/transliterate.rb +++ b/activesupport/lib/active_support/inflector/transliterate.rb @@ -60,6 +60,7 @@ module ActiveSupport # Transliteration is restricted to UTF-8, US-ASCII and GB18030 strings # Other encodings will raise an ArgumentError. def transliterate(string, replacement = "?", locale: nil) + string = string.dup if string.frozen? raise ArgumentError, "Can only transliterate strings. Received #{string.class.name}" unless string.is_a?(String) allowed_encodings = [Encoding::UTF_8, Encoding::US_ASCII, Encoding::GB18030] diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index dda71b880e..b0f30d2995 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -3,6 +3,7 @@ require "mutex_m" require "concurrent/map" require "set" +require "active_support/core_ext/object/try" module ActiveSupport module Notifications diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 8572d56722..59d2d6f530 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -6,6 +6,7 @@ require "bigdecimal" require "active_support/core_ext/string/access" require "active_support/ordered_hash" require "active_support/core_ext/object/conversions" +require "active_support/core_ext/date/conversions" require "active_support/core_ext/object/deep_dup" require "active_support/inflections" diff --git a/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb b/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb index e0e331fc91..a2b3239884 100644 --- a/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb +++ b/activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb @@ -4,23 +4,32 @@ require "abstract_unit" require "active_support/core_ext/module/attribute_accessors_per_thread" class ModuleAttributeAccessorPerThreadTest < ActiveSupport::TestCase - def setup - @class = Class.new do - thread_mattr_accessor :foo - thread_mattr_accessor :bar, instance_writer: false - thread_mattr_reader :shaq, instance_reader: false - thread_mattr_accessor :camp, instance_accessor: false - - def self.name; "MyClass" end - end + class MyClass + thread_mattr_accessor :foo + thread_mattr_accessor :bar, instance_writer: false + thread_mattr_reader :shaq, instance_reader: false + thread_mattr_accessor :camp, instance_accessor: false + end - @subclass = Class.new(@class) do - def self.name; "SubMyClass" end - end + class SubMyClass < MyClass + end + setup do + @class = MyClass + @subclass = SubMyClass @object = @class.new end + def test_can_initialize_with_default_value + Thread.new do + @class.thread_mattr_accessor :baz, default: "default_value" + + assert_equal "default_value", @class.baz + end.join + + assert_nil @class.baz + end + def test_should_use_mattr_default Thread.new do assert_nil @class.foo @@ -66,23 +75,23 @@ class ModuleAttributeAccessorPerThreadTest < ActiveSupport::TestCase threads = [] threads << Thread.new do @class.foo = "things" - sleep 1 + Thread.pass assert_equal "things", @class.foo end threads << Thread.new do @class.foo = "other things" - sleep 1 + Thread.pass assert_equal "other things", @class.foo end threads << Thread.new do @class.foo = "really other things" - sleep 1 + Thread.pass assert_equal "really other things", @class.foo end - threads.each { |t| t.join } + threads.each(&:join) end def test_should_raise_name_error_if_attribute_name_is_invalid diff --git a/activesupport/test/core_ext/object/acts_like_test.rb b/activesupport/test/core_ext/object/acts_like_test.rb index 31241caf0a..8aa9eb036a 100644 --- a/activesupport/test/core_ext/object/acts_like_test.rb +++ b/activesupport/test/core_ext/object/acts_like_test.rb @@ -1,7 +1,10 @@ # frozen_string_literal: true require "abstract_unit" -require "active_support/core_ext/object" +require "active_support/core_ext/date/acts_like" +require "active_support/core_ext/time/acts_like" +require "active_support/core_ext/date_time/acts_like" +require "active_support/core_ext/object/acts_like" class ObjectTests < ActiveSupport::TestCase class DuckTime diff --git a/activesupport/test/core_ext/object/deep_dup_test.rb b/activesupport/test/core_ext/object/deep_dup_test.rb index 1fb26ebac7..8e5e28c513 100644 --- a/activesupport/test/core_ext/object/deep_dup_test.rb +++ b/activesupport/test/core_ext/object/deep_dup_test.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "abstract_unit" -require "active_support/core_ext/object" +require "active_support/core_ext/object/deep_dup" class DeepDupTest < ActiveSupport::TestCase def test_array_deep_dup diff --git a/activesupport/test/core_ext/object/instance_variables_test.rb b/activesupport/test/core_ext/object/instance_variables_test.rb index 9052d209a3..dd710e9349 100644 --- a/activesupport/test/core_ext/object/instance_variables_test.rb +++ b/activesupport/test/core_ext/object/instance_variables_test.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "abstract_unit" -require "active_support/core_ext/object" +require "active_support/core_ext/object/instance_variables" class ObjectInstanceVariableTest < ActiveSupport::TestCase def setup diff --git a/activesupport/test/core_ext/object/json_gem_encoding_test.rb b/activesupport/test/core_ext/object/json_gem_encoding_test.rb index 4cdb6ed09f..eef02f7458 100644 --- a/activesupport/test/core_ext/object/json_gem_encoding_test.rb +++ b/activesupport/test/core_ext/object/json_gem_encoding_test.rb @@ -22,7 +22,7 @@ class JsonGemEncodingTest < ActiveSupport::TestCase JSONTest::EncodingTestCases.constants.each_with_index do |name| JSONTest::EncodingTestCases.const_get(name).each_with_index do |(subject, _), i| - test("#{name[0..-6].underscore} #{i}") do + test("#{name[0..-6]} #{i}") do assert_same_with_or_without_active_support(subject) end end diff --git a/activesupport/test/core_ext/object/try_test.rb b/activesupport/test/core_ext/object/try_test.rb index a838334034..d501b5edce 100644 --- a/activesupport/test/core_ext/object/try_test.rb +++ b/activesupport/test/core_ext/object/try_test.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "abstract_unit" -require "active_support/core_ext/object" +require "active_support/core_ext/object/try" class ObjectTryTest < ActiveSupport::TestCase def setup diff --git a/activesupport/test/current_attributes_test.rb b/activesupport/test/current_attributes_test.rb index adbdc646bc..4cbf462da0 100644 --- a/activesupport/test/current_attributes_test.rb +++ b/activesupport/test/current_attributes_test.rb @@ -9,7 +9,7 @@ class CurrentAttributesTest < ActiveSupport::TestCase attribute :world, :account, :person, :request delegate :time_zone, to: :person - before_reset { Session.previous = person.try(:id) } + before_reset { Session.previous = person&.id } resets do Time.zone = "UTC" @@ -18,13 +18,13 @@ class CurrentAttributesTest < ActiveSupport::TestCase def account=(account) super - self.person = "#{account}'s person" + self.person = Person.new(1, "#{account}'s person") end def person=(person) super - Time.zone = person.try(:time_zone) - Session.current = person.try(:id) + Time.zone = person&.time_zone + Session.current = person&.id end def request @@ -63,7 +63,7 @@ class CurrentAttributesTest < ActiveSupport::TestCase test "set attribute via overwritten method" do Current.account = "account/1" assert_equal "account/1", Current.account - assert_equal "account/1's person", Current.person + assert_equal "account/1's person", Current.person.name end test "set auxiliary class via overwritten method" do diff --git a/activesupport/test/file_update_checker_shared_tests.rb b/activesupport/test/file_update_checker_shared_tests.rb index 84d89fa0a7..bd056a1576 100644 --- a/activesupport/test/file_update_checker_shared_tests.rb +++ b/activesupport/test/file_update_checker_shared_tests.rb @@ -3,294 +3,298 @@ require "fileutils" module FileUpdateCheckerSharedTests - extend ActiveSupport::Testing::Declarative - include FileUtils + def self.included(kls) + kls.class_eval do + extend ActiveSupport::Testing::Declarative + include FileUtils - def tmpdir - @tmpdir - end + def tmpdir + @tmpdir + end - def tmpfile(name) - File.join(tmpdir, name) - end + def tmpfile(name) + File.join(tmpdir, name) + end - def tmpfiles - @tmpfiles ||= %w(foo.rb bar.rb baz.rb).map { |f| tmpfile(f) } - end + def tmpfiles + @tmpfiles ||= %w(foo.rb bar.rb baz.rb).map { |f| tmpfile(f) } + end - def run(*args) - capture_exceptions do - Dir.mktmpdir(nil, __dir__) { |dir| @tmpdir = dir; super } - end - end + def run(*args) + capture_exceptions do + Dir.mktmpdir(nil, __dir__) { |dir| @tmpdir = dir; super } + end + end - test "should not execute the block if no paths are given" do - silence_warnings { require "listen" } - i = 0 + test "should not execute the block if no paths are given" do + silence_warnings { require "listen" } + i = 0 - checker = new_checker { i += 1 } + checker = new_checker { i += 1 } - assert_not checker.execute_if_updated - assert_equal 0, i - end + assert_not checker.execute_if_updated + assert_equal 0, i + end - test "should not execute the block if no files change" do - i = 0 + test "should not execute the block if no files change" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - checker = new_checker(tmpfiles) { i += 1 } + checker = new_checker(tmpfiles) { i += 1 } - assert_not checker.execute_if_updated - assert_equal 0, i - end + assert_not checker.execute_if_updated + assert_equal 0, i + end - test "should execute the block once when files are created" do - i = 0 + test "should execute the block once when files are created" do + i = 0 - checker = new_checker(tmpfiles) { i += 1 } + checker = new_checker(tmpfiles) { i += 1 } - touch(tmpfiles) - wait + touch(tmpfiles) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "should execute the block once when files are modified" do - i = 0 + test "should execute the block once when files are modified" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - checker = new_checker(tmpfiles) { i += 1 } + checker = new_checker(tmpfiles) { i += 1 } - touch(tmpfiles) - wait + touch(tmpfiles) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "should execute the block once when files are deleted" do - i = 0 + test "should execute the block once when files are deleted" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - checker = new_checker(tmpfiles) { i += 1 } + checker = new_checker(tmpfiles) { i += 1 } - rm_f(tmpfiles) - wait + rm_f(tmpfiles) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "updated should become true when watched files are created" do - i = 0 + test "updated should become true when watched files are created" do + i = 0 - checker = new_checker(tmpfiles) { i += 1 } - assert_not_predicate checker, :updated? + checker = new_checker(tmpfiles) { i += 1 } + assert_not_predicate checker, :updated? - touch(tmpfiles) - wait + touch(tmpfiles) + wait - assert_predicate checker, :updated? - end + assert_predicate checker, :updated? + end - test "updated should become true when watched files are modified" do - i = 0 + test "updated should become true when watched files are modified" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - checker = new_checker(tmpfiles) { i += 1 } - assert_not_predicate checker, :updated? + checker = new_checker(tmpfiles) { i += 1 } + assert_not_predicate checker, :updated? - touch(tmpfiles) - wait + touch(tmpfiles) + wait - assert_predicate checker, :updated? - end + assert_predicate checker, :updated? + end - test "updated should become true when watched files are deleted" do - i = 0 + test "updated should become true when watched files are deleted" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - checker = new_checker(tmpfiles) { i += 1 } - assert_not_predicate checker, :updated? + checker = new_checker(tmpfiles) { i += 1 } + assert_not_predicate checker, :updated? - rm_f(tmpfiles) - wait + rm_f(tmpfiles) + wait - assert_predicate checker, :updated? - end + assert_predicate checker, :updated? + end - test "should be robust to handle files with wrong modified time" do - i = 0 + test "should be robust to handle files with wrong modified time" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - now = Time.now - time = Time.mktime(now.year + 1, now.month, now.day) # wrong mtime from the future - File.utime(time, time, tmpfiles[0]) + now = Time.now + time = Time.mktime(now.year + 1, now.month, now.day) # wrong mtime from the future + File.utime(time, time, tmpfiles[0]) - checker = new_checker(tmpfiles) { i += 1 } + checker = new_checker(tmpfiles) { i += 1 } - touch(tmpfiles[1..-1]) - wait + touch(tmpfiles[1..-1]) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "should return max_time for files with mtime = Time.at(0)" do - i = 0 + test "should return max_time for files with mtime = Time.at(0)" do + i = 0 - FileUtils.touch(tmpfiles) + FileUtils.touch(tmpfiles) - time = Time.at(0) # wrong mtime from the future - File.utime(time, time, tmpfiles[0]) + time = Time.at(0) # wrong mtime from the future + File.utime(time, time, tmpfiles[0]) - checker = new_checker(tmpfiles) { i += 1 } + checker = new_checker(tmpfiles) { i += 1 } - touch(tmpfiles[1..-1]) - wait + touch(tmpfiles[1..-1]) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "should cache updated result until execute" do - i = 0 + test "should cache updated result until execute" do + i = 0 - checker = new_checker(tmpfiles) { i += 1 } - assert_not_predicate checker, :updated? + checker = new_checker(tmpfiles) { i += 1 } + assert_not_predicate checker, :updated? - touch(tmpfiles) - wait + touch(tmpfiles) + wait - assert_predicate checker, :updated? - checker.execute - assert_not_predicate checker, :updated? - end + assert_predicate checker, :updated? + checker.execute + assert_not_predicate checker, :updated? + end - test "should execute the block if files change in a watched directory one extension" do - i = 0 + test "should execute the block if files change in a watched directory one extension" do + i = 0 - checker = new_checker([], tmpdir => :rb) { i += 1 } + checker = new_checker([], tmpdir => :rb) { i += 1 } - touch(tmpfile("foo.rb")) - wait + touch(tmpfile("foo.rb")) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "should execute the block if files change in a watched directory any extensions" do - i = 0 + test "should execute the block if files change in a watched directory any extensions" do + i = 0 - checker = new_checker([], tmpdir => []) { i += 1 } + checker = new_checker([], tmpdir => []) { i += 1 } - touch(tmpfile("foo.rb")) - wait + touch(tmpfile("foo.rb")) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "should execute the block if files change in a watched directory several extensions" do - i = 0 + test "should execute the block if files change in a watched directory several extensions" do + i = 0 - checker = new_checker([], tmpdir => [:rb, :txt]) { i += 1 } + checker = new_checker([], tmpdir => [:rb, :txt]) { i += 1 } - touch(tmpfile("foo.rb")) - wait + touch(tmpfile("foo.rb")) + wait - assert checker.execute_if_updated - assert_equal 1, i + assert checker.execute_if_updated + assert_equal 1, i - touch(tmpfile("foo.txt")) - wait + touch(tmpfile("foo.txt")) + wait - assert checker.execute_if_updated - assert_equal 2, i - end + assert checker.execute_if_updated + assert_equal 2, i + end - test "should not execute the block if the file extension is not watched" do - i = 0 + test "should not execute the block if the file extension is not watched" do + i = 0 - checker = new_checker([], tmpdir => :txt) { i += 1 } + checker = new_checker([], tmpdir => :txt) { i += 1 } - touch(tmpfile("foo.rb")) - wait + touch(tmpfile("foo.rb")) + wait - assert_not checker.execute_if_updated - assert_equal 0, i - end + assert_not checker.execute_if_updated + assert_equal 0, i + end - test "does not assume files exist on instantiation" do - i = 0 + test "does not assume files exist on instantiation" do + i = 0 - non_existing = tmpfile("non_existing.rb") - checker = new_checker([non_existing]) { i += 1 } + non_existing = tmpfile("non_existing.rb") + checker = new_checker([non_existing]) { i += 1 } - touch(non_existing) - wait + touch(non_existing) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "detects files in new subdirectories" do - i = 0 + test "detects files in new subdirectories" do + i = 0 - checker = new_checker([], tmpdir => :rb) { i += 1 } + checker = new_checker([], tmpdir => :rb) { i += 1 } - subdir = tmpfile("subdir") - mkdir(subdir) - wait + subdir = tmpfile("subdir") + mkdir(subdir) + wait - assert_not checker.execute_if_updated - assert_equal 0, i + assert_not checker.execute_if_updated + assert_equal 0, i - touch(File.join(subdir, "nested.rb")) - wait + touch(File.join(subdir, "nested.rb")) + wait - assert checker.execute_if_updated - assert_equal 1, i - end + assert checker.execute_if_updated + assert_equal 1, i + end - test "looked up extensions are inherited in subdirectories not listening to them" do - i = 0 + test "looked up extensions are inherited in subdirectories not listening to them" do + i = 0 - subdir = tmpfile("subdir") - mkdir(subdir) + subdir = tmpfile("subdir") + mkdir(subdir) - checker = new_checker([], tmpdir => :rb, subdir => :txt) { i += 1 } + checker = new_checker([], tmpdir => :rb, subdir => :txt) { i += 1 } - touch(tmpfile("new.txt")) - wait + touch(tmpfile("new.txt")) + wait - assert_not checker.execute_if_updated - assert_equal 0, i + assert_not checker.execute_if_updated + assert_equal 0, i - # subdir does not look for Ruby files, but its parent tmpdir does. - touch(File.join(subdir, "nested.rb")) - wait + # subdir does not look for Ruby files, but its parent tmpdir does. + touch(File.join(subdir, "nested.rb")) + wait - assert checker.execute_if_updated - assert_equal 1, i + assert checker.execute_if_updated + assert_equal 1, i - touch(File.join(subdir, "nested.txt")) - wait + touch(File.join(subdir, "nested.txt")) + wait - assert checker.execute_if_updated - assert_equal 2, i - end + assert checker.execute_if_updated + assert_equal 2, i + end - test "initialize raises an ArgumentError if no block given" do - assert_raise ArgumentError do - new_checker([]) + test "initialize raises an ArgumentError if no block given" do + assert_raise ArgumentError do + new_checker([]) + end + end end end end diff --git a/activesupport/test/transliterate_test.rb b/activesupport/test/transliterate_test.rb index 2e02b5e938..f13c5efa47 100644 --- a/activesupport/test/transliterate_test.rb +++ b/activesupport/test/transliterate_test.rb @@ -59,19 +59,19 @@ class TransliterateTest < ActiveSupport::TestCase end def test_transliterate_handles_strings_with_valid_utf8_encodings - string = String.new("A", encoding: Encoding::UTF_8) + string = String.new("A", encoding: Encoding::UTF_8).freeze assert_equal "A", ActiveSupport::Inflector.transliterate(string) end def test_transliterate_handles_strings_with_valid_us_ascii_encodings - string = String.new("A", encoding: Encoding::US_ASCII) + string = String.new("A", encoding: Encoding::US_ASCII).freeze transcoded = ActiveSupport::Inflector.transliterate(string) assert_equal "A", transcoded assert_equal Encoding::US_ASCII, transcoded.encoding end def test_transliterate_handles_strings_with_valid_gb18030_encodings - string = String.new("A", encoding: Encoding::GB18030) + string = String.new("A", encoding: Encoding::GB18030).freeze transcoded = ActiveSupport::Inflector.transliterate(string) assert_equal "A", transcoded assert_equal Encoding::GB18030, transcoded.encoding @@ -84,7 +84,7 @@ class TransliterateTest < ActiveSupport::TestCase Encoding::GB18030 ] incompatible_encodings.each do |encoding| - string = String.new("", encoding: encoding) + string = String.new("", encoding: encoding).freeze exception = assert_raises ArgumentError do ActiveSupport::Inflector.transliterate(string) end @@ -93,17 +93,17 @@ class TransliterateTest < ActiveSupport::TestCase end def test_transliterate_handles_strings_with_invalid_utf8_bytes - string = String.new("\255", encoding: Encoding::UTF_8) + string = String.new("\255", encoding: Encoding::UTF_8).freeze assert_equal "?", ActiveSupport::Inflector.transliterate(string) end def test_transliterate_handles_strings_with_invalid_us_ascii_bytes - string = String.new("\255", encoding: Encoding::US_ASCII) + string = String.new("\255", encoding: Encoding::US_ASCII).freeze assert_equal "?", ActiveSupport::Inflector.transliterate(string) end def test_transliterate_handles_strings_with_invalid_gb18030_bytes - string = String.new("\255", encoding: Encoding::GB18030) + string = String.new("\255", encoding: Encoding::GB18030).freeze assert_equal "?", ActiveSupport::Inflector.transliterate(string) end end diff --git a/activesupport/test/xml_mini_test.rb b/activesupport/test/xml_mini_test.rb index 18a3f2ca66..73e7f40b0d 100644 --- a/activesupport/test/xml_mini_test.rb +++ b/activesupport/test/xml_mini_test.rb @@ -5,6 +5,7 @@ require "active_support/xml_mini" require "active_support/builder" require "active_support/core_ext/hash" require "active_support/core_ext/big_decimal" +require "active_support/core_ext/date/conversions" require "yaml" module XmlMiniTest diff --git a/guides/source/6_0_release_notes.md b/guides/source/6_0_release_notes.md index 36b42db6b2..78bf477594 100644 --- a/guides/source/6_0_release_notes.md +++ b/guides/source/6_0_release_notes.md @@ -430,7 +430,7 @@ Please refer to the [Changelog][active-record] for detailed changes. * Deprecate using class level querying methods if the receiver scope has leaked. ([Pull Request](https://github.com/rails/rails/pull/35280)) -* Deprecate `config.activerecord.sqlite3.represent_boolean_as_integer`. +* Deprecate `config.active_record.sqlite3.represent_boolean_as_integer`. ([Commit](https://github.com/rails/rails/commit/f59b08119bc0c01a00561d38279b124abc82561b)) * Deprecate passing `migrations_paths` to `connection.assume_migrated_upto_version`. diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index 212cbfaf43..2556119d33 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -277,7 +277,7 @@ Rails.autoloaders.main Rails.autoloaders.once ``` -The former is the main one. The latter is there mostly for backwards compatibily reasons, in case the application has something in `config.autoload_once_paths` (this is discouraged nowadays). +The former is the main one. The latter is there mostly for backwards compatibility reasons, in case the application has something in `config.autoload_once_paths` (this is discouraged nowadays). You can check if `zeitwerk` mode is enabled with diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index 6418005921..bee6755b52 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -307,7 +307,7 @@ When dealing with RESTful resources, calls to `form_with` can get significantly ## Creating a new article # long-style: form_with(model: @article, url: articles_path) -short-style: +# short-style: form_with(model: @article) ## Editing an existing article diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 05980d1614..39a291ed97 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -78,6 +78,20 @@ Upgrading from Rails 5.2 to Rails 6.0 For more information on changes made to Rails 6.0 please see the [release notes](6_0_release_notes.html). +### Using Webpacker + +[Webpacker](https://github.com/rails/webpacker) +is the default JavaScript compiler for Rails 6. But if you are upgrading the app, it is not activated by default. +If you want to use Webpacker, then include it in your Gemfile and install it: + +```ruby +gem "webpacker" +``` + +```sh +bin/rails webpacker:install +``` + ### Force SSL The `force_ssl` method on controllers has been deprecated and will be removed in diff --git a/railties/lib/rails/command/helpers/pretty_credentials.rb b/railties/lib/rails/command/helpers/pretty_credentials.rb deleted file mode 100644 index 873ed0e825..0000000000 --- a/railties/lib/rails/command/helpers/pretty_credentials.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "fileutils" - -module Rails - module Command - module Helpers - module PrettyCredentials - Error = Class.new(StandardError) - - def opt_in_pretty_credentials - unless already_answered? || already_opted_in? - answer = yes?("Would you like to make the credentials diff from git more readable in the future? [Y/n]") - end - - opt_in! if answer - FileUtils.touch(tracker) unless answer.nil? - rescue Error - say("Couldn't setup git to prettify the credentials diff") - end - - private - def already_answered? - tracker.exist? - end - - def already_opted_in? - system_call("git config --get 'diff.rails_credentials.textconv'", accepted_codes: [0, 1]) - end - - def opt_in! - system_call("git config diff.rails_credentials.textconv 'bin/rails credentials:show'", accepted_codes: [0]) - - git_attributes = Rails.root.join(".gitattributes") - File.open(git_attributes, "a+") do |file| - file.write(<<~EOM) - config/credentials/*.yml.enc diff=rails_credentials - config/credentials.yml.enc diff=rails_credentials - EOM - end - end - - def tracker - Rails.root.join("tmp", "rails_pretty_credentials") - end - - def system_call(command_line, accepted_codes:) - result = system(command_line) - raise(Error) if accepted_codes.exclude?($?.exitstatus) - result - end - end - end - end -end diff --git a/railties/lib/rails/commands/credentials/USAGE b/railties/lib/rails/commands/credentials/USAGE index c8d3fb9eda..6b896ab02a 100644 --- a/railties/lib/rails/commands/credentials/USAGE +++ b/railties/lib/rails/commands/credentials/USAGE @@ -30,6 +30,21 @@ You could prepend that to your server's start command like this: RAILS_MASTER_KEY="very-secret-and-secure" server.start +=== Set up Git to Diff Credentials + +Rails provides `rails credentials:diff --enable` to instruct Git to call `rails credentials:diff` +when `git diff` is run on a credentials file. + +Running the command enrolls the project such that all credentials files use the +"rails_credentials" diff driver in .gitattributes. + +Additionally since Git requires the driver itself to be set up in a config file +that isn't tracked Rails automatically ensures it's configured when running +`credentials:edit`. + +Otherwise each co-worker would have to run enable manually, including on each new +repo clone. + === Editing Credentials This will open a temporary file in `$EDITOR` with the decrypted contents to edit diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index 772e105007..9cde44558b 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -1,18 +1,19 @@ # frozen_string_literal: true +require "pathname" require "active_support" require "rails/command/helpers/editor" -require "rails/command/helpers/pretty_credentials" require "rails/command/environment_argument" -require "pathname" module Rails module Command class CredentialsCommand < Rails::Command::Base # :nodoc: include Helpers::Editor - include Helpers::PrettyCredentials include EnvironmentArgument + require_relative "credentials_command/diffing" + include Diffing + self.environment_desc = "Uses credentials from config/credentials/:environment.yml.enc encrypted by config/credentials/:environment.key key" no_commands do @@ -31,35 +32,44 @@ module Rails ensure_encryption_key_has_been_added if credentials.key.nil? ensure_credentials_have_been_added + ensure_rails_credentials_driver_is_set catch_editing_exceptions do change_credentials_in_system_editor end say "File encrypted and saved." - opt_in_pretty_credentials rescue ActiveSupport::MessageEncryptor::InvalidMessage say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?" end - def show(git_textconv_path = nil) - if git_textconv_path - default_environment = extract_environment_from_path(git_textconv_path) - fallback_message = File.read(git_textconv_path) - end - - extract_environment_option_from_argument(default_environment: default_environment) + def show + extract_environment_option_from_argument(default_environment: nil) require_application! - say credentials(git_textconv_path).read.presence || fallback_message || missing_credentials_message - rescue => e - raise(e) unless git_textconv_path - fallback_message + say credentials.read.presence || missing_credentials_message + end + + option :enroll, type: :boolean, default: false, + desc: "Enrolls project in credential file diffing with `git diff`" + + def diff(content_path = nil) + if @content_path = content_path + extract_environment_option_from_argument(default_environment: extract_environment_from_path(content_path)) + require_application! + + say credentials.read.presence || credentials.content_path.read + else + require_application! + enroll_project_in_credentials_diffing if options[:enroll] + end + rescue ActiveSupport::MessageEncryptor::InvalidMessage + say credentials.content_path.read end private - def credentials(content = nil) - Rails.application.encrypted(content || content_path, key_path: key_path) + def credentials + Rails.application.encrypted(content_path, key_path: key_path) end def ensure_encryption_key_has_been_added @@ -89,8 +99,9 @@ module Rails end end + def content_path - options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" + @content_path ||= options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" end def key_path @@ -98,15 +109,7 @@ module Rails end def extract_environment_from_path(path) - regex = %r{ - ([A-Za-z0-9]+) # match the environment - (?<!credentials) # don't match if file contains the word "credentials" - # in such case, the environment should be the default one - \.yml\.enc # look for `.yml.enc` file extension - }x - path.match(regex) - - Regexp.last_match(1) + available_environments.find { |env| path.include? env } if path.match?(/\.yml\.enc$/) end def encryption_key_file_generator diff --git a/railties/lib/rails/commands/credentials/credentials_command/diffing.rb b/railties/lib/rails/commands/credentials/credentials_command/diffing.rb new file mode 100644 index 0000000000..1d34c68074 --- /dev/null +++ b/railties/lib/rails/commands/credentials/credentials_command/diffing.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Rails::Command::CredentialsCommand::Diffing # :nodoc: + def enroll_project_in_credentials_diffing + if enrolled? + true + else + gitattributes.write(<<~end_of_template, mode: "a") + config/credentials/*.yml.enc diff=rails_credentials + config/credentials.yml.enc diff=rails_credentials + end_of_template + + say "Project successfully enrolled!" + say "Rails ensures the rails_credentials diff driver is set when running `credentials:edit`. See `credentials:help` for more." + end + end + + def ensure_rails_credentials_driver_is_set + set_driver if enrolled? && !driver_configured? + end + + private + def enrolled? + gitattributes.read.match?(/config\/credentials(\/\*)?\.yml\.enc diff=rails_credentials/) + rescue Errno::ENOENT + false + end + + def driver_configured? + system "git config --get diff.rails_credentials.textconv", out: File::NULL + end + + def set_driver + puts "running" + system "git config diff.rails_credentials.textconv 'bin/rails credentials:diff'" + end + + def gitattributes + Rails.root.join(".gitattributes") + end +end diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index b4c0028b1f..f8f5ff443a 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -3,6 +3,7 @@ require "rails/railtie" require "rails/engine/railties" require "active_support/core_ext/module/delegation" +require "active_support/core_ext/object/try" require "pathname" require "thread" diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 436315ce1e..aa7ef1077e 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -128,11 +128,6 @@ module Rails end end - # Remove the color from output. - def no_color! - Thor::Base.shell = Thor::Shell::Basic - end - # Returns an array of generator namespaces that are hidden. # Generator namespaces may be hidden for a variety of reasons. # Some are aliased such as "rails:migration" and can be diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index c66e349442..41dabb87df 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -3,6 +3,8 @@ # your test database is "scratch space" for the test suite and is wiped # and recreated between test runs. Don't rely on the data there! +require "active_support/core_ext/integer/time" + Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. <%# Spring executes the reloaders when files change. %> diff --git a/railties/test/application/current_attributes_integration_test.rb b/railties/test/application/current_attributes_integration_test.rb index 146e96facc..f4ff5b8077 100644 --- a/railties/test/application/current_attributes_integration_test.rb +++ b/railties/test/application/current_attributes_integration_test.rb @@ -18,7 +18,7 @@ class CurrentAttributesIntegrationTest < ActiveSupport::TestCase def customer=(customer) super - Time.zone = customer.try(:time_zone) + Time.zone = customer&.time_zone end end RUBY @@ -53,7 +53,7 @@ class CurrentAttributesIntegrationTest < ActiveSupport::TestCase RUBY app_file "app/views/customers/index.html.erb", <<-RUBY - <%= Current.customer.try(:name) || 'noone' %>,<%= Time.zone.name %> + <%= Current.customer&.name || 'noone' %>,<%= Time.zone.name %> RUBY require "#{app_path}/config/environment" diff --git a/railties/test/application/rake/multi_dbs_test.rb b/railties/test/application/rake/multi_dbs_test.rb index 2606e64424..c5f8904f4e 100644 --- a/railties/test/application/rake/multi_dbs_test.rb +++ b/railties/test/application/rake/multi_dbs_test.rb @@ -349,6 +349,25 @@ module ApplicationTests rails "db:drop" rescue nil end end + + test "db:seed uses primary database connection" do + @old_rails_env = ENV["RAILS_ENV"] + @old_rack_env = ENV["RACK_ENV"] + ENV.delete "RAILS_ENV" + ENV.delete "RACK_ENV" + + db_migrate_and_schema_dump_and_load "schema" + + app_file "db/seeds.rb", <<-RUBY + print Book.connection.pool.spec.config[:database] + RUBY + + output = rails("db:seed") + assert_equal output, "db/development.sqlite3" + ensure + ENV["RAILS_ENV"] = @old_rails_env + ENV["RACK_ENV"] = @old_rack_env + end end end end diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 3dec6fbe10..974b34836b 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -10,9 +10,8 @@ require "tempfile" class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation, EnvHelpers - setup { build_app } - - teardown { teardown_app } + setup :build_app + teardown :teardown_app test "edit without editor gives hint" do run_edit_command(editor: "").tap do |output| @@ -90,134 +89,95 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase assert_match(/secret_key_base/, output) end - test "edit ask the user to opt in to pretty credentials" do - assert_match(/Would you like to make the credentials diff from git/, run_edit_command) + + test "show credentials" do + assert_match(/access_key_id: 123/, run_show_command) end - test "edit doesn't ask the user to opt in to pretty credentials when alreasy asked" do - app_file("tmp/rails_pretty_credentials", "") + test "show command raises error when require_master_key is specified and key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = true" - assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true)) end - test "edit doesn't ask the user to opt in when user already opted in" do - content = <<~EOM - [diff "rails_credentials"] - textconv = bin/rails credentials:show - EOM - app_file(".git/config", content) + test "show command does not raise error when require_master_key is false and master key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = false" - assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command) end - test "edit ask the user to opt in to pretty credentials, user accepts" do - file = Tempfile.open("credentials_test") - file.write("y") - file.rewind + test "show command displays content specified by environment option" do + run_edit_command(environment: "production") - run_edit_command(stdin: file.path) + assert_match(/access_key_id: 123/, run_show_command(environment: "production")) + end - git_attributes = app_path(".gitattributes") - expected = <<~EOM - config/credentials/*.yml.enc diff=rails_credentials - config/credentials.yml.enc diff=rails_credentials - EOM - assert(File.exist?(git_attributes)) - assert_equal(expected, File.read(git_attributes)) - Dir.chdir(app_path) do - assert_equal("bin/rails credentials:show\n", `git config --get 'diff.rails_credentials.textconv'`) - end - ensure - file.close! + test "show command properly expands environment option" do + run_edit_command(environment: "production") + + output = run_show_command(environment: "prod") + assert_match(/access_key_id: 123/, output) + assert_no_match(/secret_key_base/, output) end - test "edit ask the user to opt in to pretty credentials, user refuses" do - file = Tempfile.open("credentials_test") - file.write("n") - file.rewind - run_edit_command(stdin: file.path) + test "diff enroll diffing" do + assert_match("successfully enrolled", run_diff_command(enroll: true)) - git_attributes = app_path(".gitattributes") - assert_not(File.exist?(git_attributes)) - ensure - file.close! + assert_equal <<~EOM, File.read(app_path(".gitattributes")) + config/credentials/*.yml.enc diff=rails_credentials + config/credentials.yml.enc diff=rails_credentials + EOM end - test "show credentials" do - assert_match(/access_key_id: 123/, run_show_command) + test "running edit after enrolling in diffing sets diff driver" do + run_diff_command(enroll: true) + run_edit_command + + Dir.chdir(app_path) do + assert_equal "bin/rails credentials:diff", `git config --get 'diff.rails_credentials.textconv'`.strip + end end - test "show command when argument is provided (from git diff left file)" do + test "diff from git diff left file" do run_edit_command(environment: "development") - assert_match(/access_key_id: 123/, run_show_command("config/credentials/development.yml.enc")) + assert_match(/access_key_id: 123/, run_diff_command("config/credentials/development.yml.enc")) end - test "show command when argument is provided (from git diff right file)" do + test "diff from git diff right file" do run_edit_command(environment: "development") - dir = Dir.mktmpdir - file_path = File.join(dir, "KnAM4a_development.yml.enc") - file_content = File.read(app_path("config", "credentials", "development.yml.enc")) - File.write(file_path, file_content) + content_path = app_path("config", "credentials", "KnAM4a_development.yml.enc") + File.write(content_path, + File.read(app_path("config", "credentials", "development.yml.enc"))) - assert_match(/access_key_id: 123/, run_show_command(file_path)) - ensure - FileUtils.rm_rf(dir) + assert_match(/access_key_id: 123/, run_diff_command(content_path)) end - test "show command when argument is provided (git diff) and filename is the master credentials" do - assert_match(/access_key_id: 123/, run_show_command("config/credentials.yml.enc")) + test "diff for main credentials" do + assert_match(/access_key_id: 123/, run_diff_command("config/credentials.yml.enc")) end - test "show command when argument is provided (git diff) and master key is not available" do + test "diff when master key is not available" do remove_file "config/master.key" raw_content = File.read(app_path("config", "credentials.yml.enc")) - assert_match(raw_content, run_show_command("config/credentials.yml.enc")) + assert_match(raw_content, run_diff_command("config/credentials.yml.enc")) end - test "show command when argument is provided (git diff) return the raw encrypted content in an error occurs" do + test "diff returns raw encrypted content when errors occur" do run_edit_command(environment: "development") - dir = Dir.mktmpdir - file_path = File.join(dir, "20190807development.yml.enc") - file_content = File.read(app_path("config", "credentials", "development.yml.enc")) - File.write(file_path, file_content) + content_path = app_path("20190807development.yml.enc") + encrypted_content = File.read(app_path("config", "credentials", "development.yml.enc")) + File.write(content_path, encrypted_content + "ruin decryption") - assert_match(file_content, run_show_command(file_path)) - ensure - FileUtils.rm_rf(dir) + assert_match(encrypted_content, run_diff_command(content_path)) end - test "show command raises error when require_master_key is specified and key does not exist" do - remove_file "config/master.key" - add_to_config "config.require_master_key = true" - - assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true)) - end - - test "show command does not raise error when require_master_key is false and master key does not exist" do - remove_file "config/master.key" - add_to_config "config.require_master_key = false" - - assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command) - end - - test "show command displays content specified by environment option" do - run_edit_command(environment: "production") - - assert_match(/access_key_id: 123/, run_show_command(environment: "production")) - end - - test "show command properly expands environment option" do - run_edit_command(environment: "production") - - output = run_show_command(environment: "prod") - assert_match(/access_key_id: 123/, output) - assert_no_match(/secret_key_base/, output) - end private def run_edit_command(editor: "cat", environment: nil, **options) @@ -227,9 +187,13 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end - def run_show_command(path = nil, environment: nil, **options) + def run_show_command(environment: nil, **options) args = environment ? ["--environment", environment] : [] - args.unshift(path) rails "credentials:show", args, **options end + + def run_diff_command(path = nil, enroll: nil, **options) + args = enroll ? ["--enroll"] : [path] + rails "credentials:diff", args, **options + end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 6077ba3ee7..0fe62df8ba 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -301,7 +301,7 @@ module TestHelpers # stderr:: true to pass STDERR output straight to the "real" STDERR. # By default, the STDERR and STDOUT of the process will be # combined in the returned string. - def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL) + def rails(*args, allow_failure: false, stderr: false) args = args.flatten fork = true @@ -328,7 +328,7 @@ module TestHelpers out_read.close err_read.close if err_read - $stdin.reopen(stdin, "r") + $stdin.reopen(File::NULL, "r") $stdout.reopen(out_write) $stderr.reopen(err_write) |