diff options
143 files changed, 2621 insertions, 458 deletions
diff --git a/.CONTRIBUTING.md.swp b/.CONTRIBUTING.md.swp Binary files differdeleted file mode 100644 index 22230b626b..0000000000 --- a/.CONTRIBUTING.md.swp +++ /dev/null diff --git a/.rubocop.yml b/.rubocop.yml index 3e15d34dbd..a673e6ba83 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -173,6 +173,12 @@ Lint/RequireParentheses: Lint/StringConversionInInterpolation: Enabled: true +Lint/UriEscapeUnescape: + Enabled: true + +Style/ParenthesesAroundCondition: + Enabled: true + Style/RedundantReturn: Enabled: true AllowMultipleReturnValues: true @@ -13,3 +13,4 @@ brew "yarn" cask "xquartz" brew "mupdf" brew "poppler" +brew "imagemagick" @@ -47,6 +47,7 @@ gem "dalli" gem "listen", ">= 3.0.5", "< 3.2", require: false gem "libxml-ruby", platforms: :ruby gem "connection_pool", require: false +gem "i18n", "~> 1.0.1" # for railties app_generator_test gem "bootsnap", ">= 1.1.0", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 259221979e..5af1ce00e2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -276,9 +276,9 @@ GEM httpclient (2.8.3) i18n (1.0.1) concurrent-ruby (~> 1.0) - image_processing (1.2.0) + image_processing (1.6.0) mini_magick (~> 4.0) - ruby-vips (>= 2.0.10, < 3) + ruby-vips (>= 2.0.11, < 3) io-like (0.3.0) jaro_winkler (1.5.1) jaro_winkler (1.5.1-java) @@ -411,7 +411,7 @@ GEM ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.9.0) - ruby-vips (2.0.12) + ruby-vips (2.0.13) ffi (~> 1.9) ruby_dep (1.5.0) rubyzip (1.2.1) @@ -534,6 +534,7 @@ DEPENDENCIES ffi (<= 1.9.21) google-cloud-storage (~> 1.11) hiredis + i18n (~> 1.0.1) image_processing (~> 1.2) json (>= 2.0.0) kindlerb (~> 1.2.0) @@ -578,4 +579,4 @@ DEPENDENCIES websocket-client-simple! BUNDLED WITH - 1.16.2 + 1.16.3 diff --git a/actioncable/lib/action_cable.rb b/actioncable/lib/action_cable.rb index e7456e3c1b..35eacc2f4f 100644 --- a/actioncable/lib/action_cable.rb +++ b/actioncable/lib/action_cable.rb @@ -51,4 +51,6 @@ module ActionCable autoload :Channel autoload :RemoteConnections autoload :SubscriptionAdapter + autoload :TestHelper + autoload :TestCase end diff --git a/actioncable/lib/action_cable/subscription_adapter.rb b/actioncable/lib/action_cable/subscription_adapter.rb index bcece8d33b..6a9d5c2080 100644 --- a/actioncable/lib/action_cable/subscription_adapter.rb +++ b/actioncable/lib/action_cable/subscription_adapter.rb @@ -5,6 +5,7 @@ module ActionCable extend ActiveSupport::Autoload autoload :Base + autoload :Test autoload :SubscriberMap autoload :ChannelPrefix end diff --git a/actioncable/lib/action_cable/subscription_adapter/test.rb b/actioncable/lib/action_cable/subscription_adapter/test.rb new file mode 100644 index 0000000000..52226a7c71 --- /dev/null +++ b/actioncable/lib/action_cable/subscription_adapter/test.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require_relative "async" + +module ActionCable + module SubscriptionAdapter + # == Test adapter for Action Cable + # + # The test adapter should be used only in testing. Along with + # <tt>ActionCable::TestHelper</tt> it makes a great tool to test your Rails application. + # + # To use the test adapter set adapter value to +test+ in your +cable.yml+. + # + # NOTE: Test adapter extends the <tt>ActionCable::SubscriptionsAdapter::Async</tt> adapter, + # so it could be used in system tests too. + class Test < Async + def broadcast(channel, payload) + broadcasts(channel) << payload + super + end + + def broadcasts(channel) + channels_data[channel] ||= [] + end + + def clear_messages(channel) + channels_data[channel] = [] + end + + def clear + @channels_data = nil + end + + private + def channels_data + @channels_data ||= {} + end + end + end +end diff --git a/actioncable/lib/action_cable/test_case.rb b/actioncable/lib/action_cable/test_case.rb new file mode 100644 index 0000000000..d153259bf6 --- /dev/null +++ b/actioncable/lib/action_cable/test_case.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require "active_support/test_case" + +module ActionCable + class TestCase < ActiveSupport::TestCase + include ActionCable::TestHelper + + ActiveSupport.run_load_hooks(:action_cable_test_case, self) + end +end diff --git a/actioncable/lib/action_cable/test_helper.rb b/actioncable/lib/action_cable/test_helper.rb new file mode 100644 index 0000000000..dbd5ec3b16 --- /dev/null +++ b/actioncable/lib/action_cable/test_helper.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +module ActionCable + # Provides helper methods for testing Action Cable broadcasting + module TestHelper + def before_setup # :nodoc: + server = ActionCable.server + test_adapter = ActionCable::SubscriptionAdapter::Test.new(server) + + @old_pubsub_adapter = server.pubsub + + server.instance_variable_set(:@pubsub, test_adapter) + super + end + + def after_teardown # :nodoc: + super + ActionCable.server.instance_variable_set(:@pubsub, @old_pubsub_adapter) + end + + # Asserts that the number of broadcasted messages to the stream matches the given number. + # + # def test_broadcasts + # assert_broadcasts 'messages', 0 + # ActionCable.server.broadcast 'messages', { text: 'hello' } + # assert_broadcasts 'messages', 1 + # ActionCable.server.broadcast 'messages', { text: 'world' } + # assert_broadcasts 'messages', 2 + # end + # + # If a block is passed, that block should cause the specified number of + # messages to be broadcasted. + # + # def test_broadcasts_again + # assert_broadcasts('messages', 1) do + # ActionCable.server.broadcast 'messages', { text: 'hello' } + # end + # + # assert_broadcasts('messages', 2) do + # ActionCable.server.broadcast 'messages', { text: 'hi' } + # ActionCable.server.broadcast 'messages', { text: 'how are you?' } + # end + # end + # + def assert_broadcasts(stream, number) + if block_given? + original_count = broadcasts_size(stream) + yield + new_count = broadcasts_size(stream) + assert_equal number, new_count - original_count, "#{number} broadcasts to #{stream} expected, but #{new_count - original_count} were sent" + else + actual_count = broadcasts_size(stream) + assert_equal number, actual_count, "#{number} broadcasts to #{stream} expected, but #{actual_count} were sent" + end + end + + # Asserts that no messages have been sent to the stream. + # + # def test_no_broadcasts + # assert_no_broadcasts 'messages' + # ActionCable.server.broadcast 'messages', { text: 'hi' } + # assert_broadcasts 'messages', 1 + # end + # + # If a block is passed, that block should not cause any message to be sent. + # + # def test_broadcasts_again + # assert_no_broadcasts 'messages' do + # # No job messages should be sent from this block + # end + # end + # + # Note: This assertion is simply a shortcut for: + # + # assert_broadcasts 'messages', 0, &block + # + def assert_no_broadcasts(stream, &block) + assert_broadcasts stream, 0, &block + end + + # Asserts that the specified message has been sent to the stream. + # + # def test_assert_transmited_message + # ActionCable.server.broadcast 'messages', text: 'hello' + # assert_broadcast_on('messages', text: 'hello') + # end + # + # If a block is passed, that block should cause a message with the specified data to be sent. + # + # def test_assert_broadcast_on_again + # assert_broadcast_on('messages', text: 'hello') do + # ActionCable.server.broadcast 'messages', text: 'hello' + # end + # end + # + def assert_broadcast_on(stream, data) + # Encode to JSON and back–we want to use this value to compare + # with decoded JSON. + # Comparing JSON strings doesn't work due to the order if the keys. + serialized_msg = + ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(data)) + + new_messages = broadcasts(stream) + if block_given? + old_messages = new_messages + clear_messages(stream) + + yield + new_messages = broadcasts(stream) + clear_messages(stream) + + # Restore all sent messages + (old_messages + new_messages).each { |m| pubsub_adapter.broadcast(stream, m) } + end + + message = new_messages.find { |msg| ActiveSupport::JSON.decode(msg) == serialized_msg } + + assert message, "No messages sent with #{data} to #{stream}" + end + + def pubsub_adapter # :nodoc: + ActionCable.server.pubsub + end + + delegate :broadcasts, :clear_messages, to: :pubsub_adapter + + private + def broadcasts_size(channel) # :nodoc: + broadcasts(channel).size + end + end +end diff --git a/actioncable/test/subscription_adapter/test_adapter_test.rb b/actioncable/test/subscription_adapter/test_adapter_test.rb new file mode 100644 index 0000000000..3fe07adb4a --- /dev/null +++ b/actioncable/test/subscription_adapter/test_adapter_test.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "test_helper" +require_relative "common" + +class ActionCable::SubscriptionAdapter::TestTest < ActionCable::TestCase + include CommonSubscriptionAdapterTest + + def setup + super + + @tx_adapter.shutdown + @tx_adapter = @rx_adapter + end + + def cable_config + { adapter: "test" } + end + + test "#broadcast stores messages for streams" do + @tx_adapter.broadcast("channel", "payload") + @tx_adapter.broadcast("channel2", "payload2") + + assert_equal ["payload"], @tx_adapter.broadcasts("channel") + assert_equal ["payload2"], @tx_adapter.broadcasts("channel2") + end + + test "#clear_messages deletes recorded broadcasts for the channel" do + @tx_adapter.broadcast("channel", "payload") + @tx_adapter.broadcast("channel2", "payload2") + + @tx_adapter.clear_messages("channel") + + assert_equal [], @tx_adapter.broadcasts("channel") + assert_equal ["payload2"], @tx_adapter.broadcasts("channel2") + end + + test "#clear deletes all recorded broadcasts" do + @tx_adapter.broadcast("channel", "payload") + @tx_adapter.broadcast("channel2", "payload2") + + @tx_adapter.clear + + assert_equal [], @tx_adapter.broadcasts("channel") + assert_equal [], @tx_adapter.broadcasts("channel2") + end +end diff --git a/actioncable/test/test_helper.rb b/actioncable/test/test_helper.rb index ac7881c950..f5b9ebf517 100644 --- a/actioncable/test/test_helper.rb +++ b/actioncable/test/test_helper.rb @@ -15,6 +15,10 @@ end # Require all the stubs and models Dir[File.expand_path("stubs/*.rb", __dir__)].each { |file| require file } +# Set test adapter and logger +ActionCable.server.config.cable = { "adapter" => "test" } +ActionCable.server.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } + class ActionCable::TestCase < ActiveSupport::TestCase include ActiveSupport::Testing::MethodCallAssertions diff --git a/actioncable/test/test_helper_test.rb b/actioncable/test/test_helper_test.rb new file mode 100644 index 0000000000..f82adb9c8f --- /dev/null +++ b/actioncable/test/test_helper_test.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require "test_helper" + +class BroadcastChannel < ActionCable::Channel::Base +end + +class TransmissionsTest < ActionCable::TestCase + def test_assert_broadcasts + assert_nothing_raised do + assert_broadcasts("test", 1) do + ActionCable.server.broadcast "test", "message" + end + end + end + + def test_assert_broadcasts_with_no_block + assert_nothing_raised do + ActionCable.server.broadcast "test", "message" + assert_broadcasts "test", 1 + end + + assert_nothing_raised do + ActionCable.server.broadcast "test", "message 2" + ActionCable.server.broadcast "test", "message 3" + assert_broadcasts "test", 3 + end + end + + def test_assert_no_broadcasts_with_no_block + assert_nothing_raised do + assert_no_broadcasts "test" + end + end + + def test_assert_no_broadcasts + assert_nothing_raised do + assert_no_broadcasts("test") do + ActionCable.server.broadcast "test2", "message" + end + end + end + + def test_assert_broadcasts_message_too_few_sent + ActionCable.server.broadcast "test", "hello" + error = assert_raises Minitest::Assertion do + assert_broadcasts("test", 2) do + ActionCable.server.broadcast "test", "world" + end + end + + assert_match(/2 .* but 1/, error.message) + end + + def test_assert_broadcasts_message_too_many_sent + error = assert_raises Minitest::Assertion do + assert_broadcasts("test", 1) do + ActionCable.server.broadcast "test", "hello" + ActionCable.server.broadcast "test", "world" + end + end + + assert_match(/1 .* but 2/, error.message) + end +end + +class TransmitedDataTest < ActionCable::TestCase + include ActionCable::TestHelper + + def test_assert_broadcast_on + assert_nothing_raised do + assert_broadcast_on("test", "message") do + ActionCable.server.broadcast "test", "message" + end + end + end + + def test_assert_broadcast_on_with_hash + assert_nothing_raised do + assert_broadcast_on("test", text: "hello") do + ActionCable.server.broadcast "test", text: "hello" + end + end + end + + def test_assert_broadcast_on_with_no_block + assert_nothing_raised do + ActionCable.server.broadcast "test", "hello" + assert_broadcast_on "test", "hello" + end + + assert_nothing_raised do + ActionCable.server.broadcast "test", "world" + assert_broadcast_on "test", "world" + end + end + + def test_assert_broadcast_on_message + ActionCable.server.broadcast "test", "hello" + error = assert_raises Minitest::Assertion do + assert_broadcast_on("test", "world") + end + + assert_match(/No messages sent/, error.message) + end +end diff --git a/actionmailer/test/test_helper_test.rb b/actionmailer/test/test_helper_test.rb index dcea23aabb..d31170706b 100644 --- a/actionmailer/test/test_helper_test.rb +++ b/actionmailer/test/test_helper_test.rb @@ -290,7 +290,7 @@ class TestHelperMailerTest < ActionMailer::TestCase end end - def test_assert_enqueued_email_with_with_no_block_wiht_parameterized_args + def test_assert_enqueued_email_with_with_no_block_with_parameterized_args assert_nothing_raised do silence_stream($stdout) do TestHelperMailer.with(all: "good").test_parameter_args.deliver_later diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a5497aa055..a30f178190 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,19 @@ +* Purpose metadata for signed/encrypted cookies. + + Rails can now thwart attacks that attempt to copy signed/encrypted value + of a cookie and use it as the value of another cookie. + + It does so by stashing the cookie-name in the purpose field which is + then signed/encrypted along with the cookie value. Then, on a server-side + read, we verify the cookie-names and discard any attacked cookies. + + Enable `action_dispatch.use_cookies_with_metadata` to use this feature, which + writes cookies with the new purpose and expiry metadata embedded. + + Pull Request: #32937 + + *Assain Jaleel* + * Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. `respond_to` can match multiple types and lead to undefined behavior when diff --git a/actionpack/lib/abstract_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index f99b0830b2..febd8a67a6 100644 --- a/actionpack/lib/abstract_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -82,7 +82,7 @@ module AbstractController # Given a key (as described in +expire_fragment+), returns # a key array suitable for use in reading, writing, or expiring a # cached fragment. All keys begin with <tt>:views</tt>, - # followed by ENV["RAILS_CACHE_ID"] or ENV["RAILS_APP_VERSION"] if set, + # followed by <tt>ENV["RAILS_CACHE_ID"]</tt> or <tt>ENV["RAILS_APP_VERSION"]</tt> if set, # followed by any controller-wide key prefix values, ending # with the specified +key+ value. def combined_fragment_cache_key(key) diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index edfef39771..7ed7b9d546 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -17,7 +17,7 @@ module ActionController #:nodoc: # access. When a request reaches your application, \Rails verifies the received # token with the token in the session. All requests are checked except GET requests # as these should be idempotent. Keep in mind that all session-oriented requests - # should be CSRF protected, including JavaScript and HTML requests. + # are CSRF protected by default, including JavaScript and HTML requests. # # Since HTML and JavaScript requests are typically made from the browser, we # need to ensure to verify request authenticity for the web browser. We can @@ -30,16 +30,23 @@ module ActionController #:nodoc: # URL on your site. When your JavaScript response loads on their site, it executes. # With carefully crafted JavaScript on their end, sensitive data in your JavaScript # response may be extracted. To prevent this, only XmlHttpRequest (known as XHR or - # Ajax) requests are allowed to make GET requests for JavaScript responses. + # Ajax) requests are allowed to make requests for JavaScript responses. # - # It's important to remember that XML or JSON requests are also affected and if - # you're building an API you should change forgery protection method in + # It's important to remember that XML or JSON requests are also checked by default. If + # you're building an API or an SPA you could change forgery protection method in # <tt>ApplicationController</tt> (by default: <tt>:exception</tt>): # # class ApplicationController < ActionController::Base # protect_from_forgery unless: -> { request.format.json? } # end # + # It is generally safe to exclude XHR requests from CSRF protection + # (like the code snippet above does), because XHR requests can only be made from + # the same origin. Note however that any cross-origin third party domain + # allowed via {CORS}[https://en.wikipedia.org/wiki/Cross-origin_resource_sharing] + # will also be able to create XHR requests. Be sure to check your + # CORS whitelist before disabling forgery protection for XHR. + # # CSRF protection is turned on with the <tt>protect_from_forgery</tt> method. # By default <tt>protect_from_forgery</tt> protects your session with # <tt>:null_session</tt> method, which provides an empty session diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index 2d1523f0fc..2b4559c760 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -81,7 +81,7 @@ module ActionController # * <tt>:html</tt> - Renders the provided HTML safe string, otherwise # performs HTML escape on the string first. Sets the content type as <tt>text/html</tt>. # * <tt>:json</tt> - Renders the provided hash or object in JSON. You don't - # need to call <tt>.to_json<tt> on the object you want to render. + # need to call <tt>.to_json</tt> on the object you want to render. # * <tt>:body</tt> - Renders provided text and sets content type of <tt>text/plain</tt>. # # If no <tt>options</tt> hash is passed or if <tt>:update</tt> is specified, the default is diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index 1d58964862..de11939fa8 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/core_ext/object/duplicable" +require "active_support/core_ext/array/extract" module ActionDispatch module Http @@ -38,8 +39,8 @@ module ActionDispatch end end - deep_regexps, regexps = regexps.partition { |r| r.to_s.include?("\\.".freeze) } - deep_strings, strings = strings.partition { |s| s.include?("\\.".freeze) } + deep_regexps = regexps.extract! { |r| r.to_s.include?("\\.".freeze) } + deep_strings = strings.extract! { |s| s.include?("\\.".freeze) } regexps << Regexp.new(strings.join("|".freeze), true) unless strings.empty? deep_regexps << Regexp.new(deep_strings.join("|".freeze), true) unless deep_strings.empty? diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index c45d947904..34331b7e4b 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -81,6 +81,10 @@ module ActionDispatch get_header Cookies::COOKIES_ROTATIONS end + def use_cookies_with_metadata + get_header Cookies::USE_COOKIES_WITH_METADATA + end + # :startdoc: end @@ -182,6 +186,7 @@ module ActionDispatch COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze COOKIES_DIGEST = "action_dispatch.cookies_digest".freeze COOKIES_ROTATIONS = "action_dispatch.cookies_rotations".freeze + USE_COOKIES_WITH_METADATA = "action_dispatch.use_cookies_with_metadata".freeze # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -470,7 +475,7 @@ module ActionDispatch def [](name) if data = @parent_jar[name.to_s] - parse name, data + parse(name, data, purpose: "cookie.#{name}") || parse(name, data) end end @@ -481,7 +486,7 @@ module ActionDispatch options = { value: options } end - commit(options) + commit(name, options) @parent_jar[name] = options end @@ -497,13 +502,24 @@ module ActionDispatch end end - def parse(name, data); data; end - def commit(options); end + def cookie_metadata(name, options) + if request.use_cookies_with_metadata + metadata = expiry_options(options) + metadata[:purpose] = "cookie.#{name}" + + metadata + else + {} + end + end + + def parse(name, data, purpose: nil); data; end + def commit(name, options); end end class PermanentCookieJar < AbstractCookieJar # :nodoc: private - def commit(options) + def commit(name, options) options[:expires] = 20.years.from_now end end @@ -583,14 +599,14 @@ module ActionDispatch end private - def parse(name, signed_message) + def parse(name, signed_message, purpose: nil) deserialize(name) do |rotate| - @verifier.verified(signed_message, on_rotation: rotate) + @verifier.verified(signed_message, on_rotation: rotate, purpose: purpose) end end - def commit(options) - options[:value] = @verifier.generate(serialize(options[:value]), expiry_options(options)) + def commit(name, options) + options[:value] = @verifier.generate(serialize(options[:value]), cookie_metadata(name, options)) raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end @@ -631,16 +647,16 @@ module ActionDispatch end private - def parse(name, encrypted_message) + def parse(name, encrypted_message, purpose: nil) deserialize(name) do |rotate| - @encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate) + @encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate, purpose: purpose) end rescue ActiveSupport::MessageEncryptor::InvalidMessage, ActiveSupport::MessageVerifier::InvalidSignature parse_legacy_signed_message(name, encrypted_message) end - def commit(options) - options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value]), expiry_options(options)) + def commit(name, options) + options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value]), cookie_metadata(name, options)) raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index eb6fbca6ba..efc3988bc3 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -21,6 +21,7 @@ module ActionDispatch config.action_dispatch.encrypted_signed_cookie_salt = "signed encrypted cookie" config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie" config.action_dispatch.use_authenticated_cookie_encryption = false + config.action_dispatch.use_cookies_with_metadata = false config.action_dispatch.perform_deep_munge = true config.action_dispatch.default_headers = { diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index 0ae464082d..fb0efb9a58 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/hash/indifferent_access" + module ActionDispatch class Request class Utils # :nodoc: diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index cba49d1a0b..413e524ef6 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -83,7 +83,7 @@ module ActionDispatch private def normalize_filter(filter) if filter[:controller] - { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } + { controller: /#{filter[:controller].underscore.sub(/_?controller\z/, "")}/ } elsif filter[:grep] { controller: /#{filter[:grep]}/, action: /#{filter[:grep]}/, verb: /#{filter[:grep]}/, name: /#{filter[:grep]}/, path: /#{filter[:grep]}/ } diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index aba778fad6..6637c2cae9 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -289,6 +289,46 @@ class CookiesTest < ActionController::TestCase cookies[:user_name] = { value: "assain", expires: 2.hours } head :ok end + + def encrypted_discount_and_user_id_cookie + cookies.encrypted[:user_id] = { value: 50, expires: 1.hour } + cookies.encrypted[:discount_percentage] = 10 + + head :ok + end + + def signed_discount_and_user_id_cookie + cookies.signed[:user_id] = { value: 50, expires: 1.hour } + cookies.signed[:discount_percentage] = 10 + + head :ok + end + + def rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_on + # cookies.encrypted[:favorite] = { value: "5-2-Stable Chocolate Cookies", expires: 1000.years } + cookies[:favorite] = "KvH5lIHvX5vPQkLIK63r/NuIMwzWky8M0Zwk8SZ6DwUv8+srf36geR4nWq5KmhsZIYXA8NRdCZYIfxMKJsOFlz77Gf+Fq8vBBCWJTp95rx39A28TCUTJEyMhCNJO5eie7Skef76Qt5Jo/SCnIADAhzyGQkGBopKRcA==--qXZZFWGbCy6N8AGy--WswoH+xHrNh9MzSXDpB2fA==" + + head :ok + end + + def rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off + cookies[:favorite] = "Wmg4amgvcVVvWGcwK3c4WjJEbTdRQUgrWXhBdDliUTR0cVNidXpmVTMrc2RjcitwUzVsWWEwZGtuVGtFUjJwNi0tcVhVMTFMOTQ1d0hIVE1FK0pJc05SQT09--8b2a55c375049a50f7a959b9d42b31ef0b2bb594" + + head :ok + end + + def rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_on + # cookies.signed[:favorite] = { value: "5-2-Stable Choco Chip Cookie", expires: 1000.years } + cookies[:favorite] = "eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaEpJaUUxTFRJdFUzUmhZbXhsSUVOb2IyTnZJRU5vYVhBZ1EyOXZhMmxsQmpvR1JWUT0iLCJleHAiOiIzMDE4LTA3LTExVDE2OjExOjI2Ljc1M1oiLCJwdXIiOm51bGx9fQ==--7df5d885b78b70a501d6e82140ae91b24060ac00" + + head :ok + end + + def rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_off + cookies[:favorite] = "BAhJIiE1LTItU3RhYmxlIENob2NvIENoaXAgQ29va2llBjoGRVQ=--50bbdbf8d64f5a3ec3e54878f54d4f55b6cb3aff" + + head :ok + end end tests TestController @@ -1274,6 +1314,8 @@ class CookiesTest < ActionController::TestCase end def test_signed_cookie_with_expires_set_relatively + request.env["action_dispatch.use_cookies_with_metadata"] = true + cookies.signed[:user_name] = { value: "assain", expires: 2.hours } travel 1.hour @@ -1284,6 +1326,8 @@ class CookiesTest < ActionController::TestCase end def test_encrypted_cookie_with_expires_set_relatively + request.env["action_dispatch.use_cookies_with_metadata"] = true + cookies.encrypted[:user_name] = { value: "assain", expires: 2.hours } travel 1.hour @@ -1300,6 +1344,124 @@ class CookiesTest < ActionController::TestCase end end + def test_purpose_metadata_for_encrypted_cookies + get :encrypted_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_equal 50, cookies.encrypted[:discount_percentage] + + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :encrypted_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_nil cookies.encrypted[:discount_percentage] + end + + def test_purpose_metadata_for_signed_cookies + get :signed_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_equal 50, cookies.signed[:discount_percentage] + + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :signed_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_nil cookies.signed[:discount_percentage] + end + + def test_switch_off_metadata_for_encrypted_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + get :encrypted_discount_and_user_id_cookie + + travel 2.hours + assert_equal 50, cookies.encrypted[:user_id] + + cookies[:discount_percentage] = cookies[:user_id] + assert_not_equal 10, cookies.encrypted[:discount_percentage] + assert_equal 50, cookies.encrypted[:discount_percentage] + end + + def test_switch_off_metadata_for_signed_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + get :signed_discount_and_user_id_cookie + + travel 2.hours + assert_equal 50, cookies.signed[:user_id] + + cookies[:discount_percentage] = cookies[:user_id] + assert_not_equal 10, cookies.signed[:discount_percentage] + assert_equal 50, cookies.signed[:discount_percentage] + end + + def test_read_rails_5_2_stable_encrypted_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + + travel 1001.years do + assert_nil cookies.encrypted[:favorite] + end + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + end + + def test_read_rails_5_2_stable_signed_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + + travel 1001.years do + assert_nil cookies.signed[:favorite] + end + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + end + + def test_read_rails_5_2_stable_encrypted_cookies_if_use_metadata_config_is_true + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + + travel 1001.years do + assert_nil cookies.encrypted[:favorite] + end + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + end + + def test_read_rails_5_2_stable_signed_cookies_if_use_metadata_config_is_true + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + + travel 1001.years do + assert_nil cookies.signed[:favorite] + end + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + end + private def assert_cookie_header(expected) header = @response.headers["Set-Cookie"] diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index 600280d6b3..668469a01d 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -20,6 +20,7 @@ module ActionDispatch setup do @cleaner = ActiveSupport::BacktraceCleaner.new + @cleaner.remove_filters! @cleaner.add_silencer { |line| line !~ /^lib/ } end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 4c552c635a..46b3b1fa25 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,13 +1,12 @@ -* Deprecate exposing public methods in view's helpers. +* Deprecate calling private model methods from view helpers. - For example, in methods like `options_from_collection_for_select` - and `collection_select` it is possible to call private methods from - the objects used. + For example, in methods like `options_from_collection_for_select` + and `collection_select` it is possible to call private methods from + the objects used. - See [#33546](https://github.com/rails/rails/issues/33546) for details. - - *[Ana María Martínez Gómez](https://github.com/Ana06)* + Fixes #33546. + *Ana María Martínez Gómez* * Fix issue with `button_to`'s `to_form_params` diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index 14bd8ffa84..cbcce4a4dc 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -55,7 +55,7 @@ module ActionView # that path. # * <tt>:skip_pipeline</tt> - This option is used to bypass the asset pipeline # when it is set to true. - # * <tt>:nonce<tt> - When set to true, adds an automatic nonce value if + # * <tt>:nonce</tt> - When set to true, adds an automatic nonce value if # you have Content Security Policy enabled. # # ==== Examples diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 2ecba2e337..2b9d55a019 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -806,13 +806,11 @@ module ActionView end def public_or_deprecated_send(item, value) - begin - item.public_send(value) - rescue NoMethodError - raise unless item.respond_to?(value, true) && !item.respond_to?(value) - ActiveSupport::Deprecation.warn "Using private methods in view's helpers is deprecated" - item.send(value) - end + item.public_send(value) + rescue NoMethodError + raise unless item.respond_to?(value, true) && !item.respond_to?(value) + ActiveSupport::Deprecation.warn "Using private methods from view helpers is deprecated (calling private #{item.class}##{value})" + item.send(value) end def prompt_text(prompt) diff --git a/actionview/lib/action_view/helpers/number_helper.rb b/actionview/lib/action_view/helpers/number_helper.rb index 4b53b8fe6e..35206b7e48 100644 --- a/actionview/lib/action_view/helpers/number_helper.rb +++ b/actionview/lib/action_view/helpers/number_helper.rb @@ -100,6 +100,9 @@ module ActionView # absolute value of the number. # * <tt>:raise</tt> - If true, raises +InvalidNumberError+ when # the argument is invalid. + # * <tt>:strip_insignificant_zeros</tt> - If +true+ removes + # insignificant zeros after the decimal separator (defaults to + # +false+). # # ==== Examples # @@ -117,6 +120,8 @@ module ActionView # # => R$1234567890,50 # number_to_currency(1234567890.50, unit: "R$", separator: ",", delimiter: "", format: "%n %u") # # => 1234567890,50 R$ + # number_to_currency(1234567890.50, strip_insignificant_zeros: true) + # # => "$1,234,567,890.5" def number_to_currency(number, options = {}) delegate_number_helper_method(:number_to_currency, number, options) end diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index 77a1c1fed9..a338d076e4 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -228,7 +228,7 @@ module ActionView # pluralize(2, 'Person', locale: :de) # # => 2 Personen def pluralize(count, singular, plural_arg = nil, plural: plural_arg, locale: I18n.locale) - word = if (count == 1 || count =~ /^1(\.0+)?$/) + word = if count == 1 || count =~ /^1(\.0+)?$/ singular else plural || singular.pluralize(locale) diff --git a/actionview/package.json b/actionview/package.json index 624eb5de93..1f74df79d3 100644 --- a/actionview/package.json +++ b/actionview/package.json @@ -30,7 +30,7 @@ }, "homepage": "http://rubyonrails.org/", "devDependencies": { - "coffeelint": "^1.15.7", + "coffeelint": "^2.1.0", "eslint": "^2.13.1" } } diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index 7bce946d3b..8a0a706fef 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -74,11 +74,11 @@ class FormOptionsHelperTest < ActionView::TestCase end def test_collection_options_with_private_value_method - assert_deprecated(/Using private methods in view's helpers is deprecated/) { options_from_collection_for_select(dummy_posts, "secret", "title") } + assert_deprecated("Using private methods from view helpers is deprecated (calling private Struct::Post#secret)") { options_from_collection_for_select(dummy_posts, "secret", "title") } end def test_collection_options_with_private_text_method - assert_deprecated(/Using private methods in view's helpers is deprecated/) { options_from_collection_for_select(dummy_posts, "author_name", "secret") } + assert_deprecated("Using private methods from view helpers is deprecated (calling private Struct::Post#secret)") { options_from_collection_for_select(dummy_posts, "author_name", "secret") } end def test_collection_options_with_preselected_value diff --git a/actionview/test/template/text_helper_test.rb b/actionview/test/template/text_helper_test.rb index 45edfe18be..4d47706bda 100644 --- a/actionview/test/template/text_helper_test.rb +++ b/actionview/test/template/text_helper_test.rb @@ -9,7 +9,7 @@ class TextHelperTest < ActionView::TestCase super # This simulates the fact that instance variables are reset every time # a view is rendered. The cycle helper depends on this behavior. - @_cycles = nil if (defined? @_cycles) + @_cycles = nil if defined?(@_cycles) end def test_concat diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 8526741383..2417ea3a87 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,23 +1,29 @@ -* Move `enqueue`/`enqueue_at` notifications to an around callback. +* Allow `assert_performed_with` to be called without a block. - Improves timing accuracy over the old after callback by including - time spent writing to the adapter's IO implementation. + *bogdanvlviv* - *Zach Kemp* +* Execution of `assert_performed_jobs`, and `assert_no_performed_jobs` + without a block should respect passed `:except`, `:only`, and `:queue` options. -* Allow `queue` option to `assert_no_enqueued_jobs`. + *bogdanvlviv* - Example: - ``` - def test_no_logging - assert_no_enqueued_jobs queue: 'default' do - LoggingJob.set(queue: :some_queue).perform_later - end - end - ``` +* Allow `:queue` option to job assertions and helpers. *bogdanvlviv* +* Allow `perform_enqueued_jobs` to be called without a block. + + Performs all of the jobs that have been enqueued up to this point in the test. + + *Kevin Deisz* + +* Move `enqueue`/`enqueue_at` notifications to an around callback. + + Improves timing accuracy over the old after callback by including + time spent writing to the adapter's IO implementation. + + *Zach Kemp* + * Allow call `assert_enqueued_with` with no block. Example: diff --git a/activejob/lib/active_job/execution.rb b/activejob/lib/active_job/execution.rb index d75be376ec..f5a343311f 100644 --- a/activejob/lib/active_job/execution.rb +++ b/activejob/lib/active_job/execution.rb @@ -31,11 +31,11 @@ module ActiveJob # # MyJob.new(*args).perform_now def perform_now + # Guard against jobs that were persisted before we started counting executions by zeroing out nil counters + self.executions = (executions || 0) + 1 + deserialize_arguments_if_needed run_callbacks :perform do - # Guard against jobs that were persisted before we started counting executions by zeroing out nil counters - self.executions = (executions || 0) + 1 - perform(*arguments) end rescue => exception diff --git a/activejob/lib/active_job/queue_adapters/test_adapter.rb b/activejob/lib/active_job/queue_adapters/test_adapter.rb index 885f9ff01c..f73ad444ba 100644 --- a/activejob/lib/active_job/queue_adapters/test_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/test_adapter.rb @@ -12,7 +12,7 @@ module ActiveJob # # Rails.application.config.active_job.queue_adapter = :test class TestAdapter - attr_accessor(:perform_enqueued_jobs, :perform_enqueued_at_jobs, :filter, :reject) + attr_accessor(:perform_enqueued_jobs, :perform_enqueued_at_jobs, :filter, :reject, :queue) attr_writer(:enqueued_jobs, :performed_jobs) # Provides a store of all the enqueued jobs with the TestAdapter so you can check them. @@ -29,14 +29,14 @@ module ActiveJob return if filtered?(job) job_data = job_to_hash(job) - enqueue_or_perform(perform_enqueued_jobs, job, job_data) + perform_or_enqueue(perform_enqueued_jobs, job, job_data) end def enqueue_at(job, timestamp) #:nodoc: return if filtered?(job) job_data = job_to_hash(job, at: timestamp) - enqueue_or_perform(perform_enqueued_at_jobs, job, job_data) + perform_or_enqueue(perform_enqueued_at_jobs, job, job_data) end private @@ -44,7 +44,7 @@ module ActiveJob { job: job.class, args: job.serialize.fetch("arguments"), queue: job.queue_name }.merge!(extras) end - def enqueue_or_perform(perform, job, job_data) + def perform_or_enqueue(perform, job, job_data) if perform performed_jobs << job_data Base.execute job.serialize @@ -54,12 +54,20 @@ module ActiveJob end def filtered?(job) + filtered_queue?(job) || filtered_job_class?(job) + end + + def filtered_queue?(job) + if queue + job.queue_name != queue.to_s + end + end + + def filtered_job_class?(job) if filter !Array(filter).include?(job.class) elsif reject Array(reject).include?(job.class) - else - false end end end diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 04cde28a96..bb9e3e6ca4 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -52,7 +52,7 @@ module ActiveJob queue_adapter_changed_jobs.each { |klass| klass.disable_test_adapter } end - # Specifies the queue adapter to use with all active job test helpers. + # Specifies the queue adapter to use with all Active Job test helpers. # # Returns an instance of the queue adapter and defaults to # <tt>ActiveJob::QueueAdapters::TestAdapter</tt>. @@ -117,14 +117,18 @@ module ActiveJob # end def assert_enqueued_jobs(number, only: nil, except: nil, queue: nil) if block_given? - original_count = enqueued_jobs_size(only: only, except: except, queue: queue) + original_count = enqueued_jobs_with(only: only, except: except, queue: queue) + yield - new_count = enqueued_jobs_size(only: only, except: except, queue: queue) - assert_equal number, new_count - original_count, "#{number} jobs expected, but #{new_count - original_count} were enqueued" + + new_count = enqueued_jobs_with(only: only, except: except, queue: queue) + + actual_count = new_count - original_count else - actual_count = enqueued_jobs_size(only: only, except: except, queue: queue) - assert_equal number, actual_count, "#{number} jobs expected, but #{actual_count} were enqueued" + actual_count = enqueued_jobs_with(only: only, except: except, queue: queue) end + + assert_equal number, actual_count, "#{number} jobs expected, but #{actual_count} were enqueued" end # Asserts that no jobs have been enqueued. @@ -176,7 +180,7 @@ module ActiveJob # Asserts that the number of performed jobs matches the given number. # If no block is passed, <tt>perform_enqueued_jobs</tt> - # must be called around the job call. + # must be called around or after the job call. # # def test_jobs # assert_performed_jobs 0 @@ -186,10 +190,11 @@ module ActiveJob # end # assert_performed_jobs 1 # - # perform_enqueued_jobs do - # HelloJob.perform_later('yves') - # assert_performed_jobs 2 - # end + # HelloJob.perform_later('yves') + # + # perform_enqueued_jobs + # + # assert_performed_jobs 2 # end # # If a block is passed, that block should cause the specified number of @@ -206,7 +211,7 @@ module ActiveJob # end # end # - # The block form supports filtering. If the :only option is specified, + # This method also supports filtering. If the +:only+ option is specified, # then only the listed job(s) will be performed. # # def test_hello_job @@ -216,7 +221,7 @@ module ActiveJob # end # end # - # Also if the :except option is specified, + # Also if the +:except+ option is specified, # then the job(s) except specific class will be performed. # # def test_hello_job @@ -237,17 +242,30 @@ module ActiveJob # end # end # end - def assert_performed_jobs(number, only: nil, except: nil) + # + # If the +:queue+ option is specified, + # then only the job(s) enqueued to a specific queue will be performed. + # + # def test_assert_performed_jobs_with_queue_option + # assert_performed_jobs 1, queue: :some_queue do + # HelloJob.set(queue: :some_queue).perform_later("jeremy") + # HelloJob.set(queue: :other_queue).perform_later("bogdan") + # end + # end + def assert_performed_jobs(number, only: nil, except: nil, queue: nil, &block) if block_given? original_count = performed_jobs.size - perform_enqueued_jobs(only: only, except: except) { yield } + + perform_enqueued_jobs(only: only, except: except, queue: queue, &block) + new_count = performed_jobs.size - assert_equal number, new_count - original_count, - "#{number} jobs expected, but #{new_count - original_count} were performed" + + performed_jobs_size = new_count - original_count else - performed_jobs_size = performed_jobs.size - assert_equal number, performed_jobs_size, "#{number} jobs expected, but #{performed_jobs_size} were performed" + performed_jobs_size = performed_jobs_with(only: only, except: except, queue: queue) end + + assert_equal number, performed_jobs_size, "#{number} jobs expected, but #{performed_jobs_size} were performed" end # Asserts that no jobs have been performed. @@ -269,7 +287,7 @@ module ActiveJob # end # end # - # The block form supports filtering. If the :only option is specified, + # The block form supports filtering. If the +:only+ option is specified, # then only the listed job(s) will not be performed. # # def test_no_logging @@ -278,7 +296,7 @@ module ActiveJob # end # end # - # Also if the :except option is specified, + # Also if the +:except+ option is specified, # then the job(s) except specific class will not be performed. # # def test_no_logging @@ -287,11 +305,20 @@ module ActiveJob # end # end # + # If the +:queue+ option is specified, + # then only the job(s) enqueued to a specific queue will not be performed. + # + # def test_assert_no_performed_jobs_with_queue_option + # assert_no_performed_jobs queue: :some_queue do + # HelloJob.set(queue: :other_queue).perform_later("jeremy") + # end + # end + # # Note: This assertion is simply a shortcut for: # # assert_performed_jobs 0, &block - def assert_no_performed_jobs(only: nil, except: nil, &block) - assert_performed_jobs 0, only: only, except: except, &block + def assert_no_performed_jobs(only: nil, except: nil, queue: nil, &block) + assert_performed_jobs 0, only: only, except: except, queue: queue, &block end # Asserts that the job has been enqueued with the given arguments. @@ -338,7 +365,25 @@ module ActiveJob instantiate_job(matching_job) end - # Asserts that the job passed in the block has been performed with the given arguments. + # Asserts that the job has been performed with the given arguments. + # + # def test_assert_performed_with + # MyJob.perform_later(1,2,3) + # + # perform_enqueued_jobs + # + # assert_performed_with(job: MyJob, args: [1,2,3], queue: 'high') + # + # MyJob.set(wait_until: Date.tomorrow.noon).perform_later + # + # perform_enqueued_jobs + # + # assert_performed_with(job: MyJob, at: Date.tomorrow.noon) + # end + # + # If a block is passed, that block performs all of the jobs that were + # enqueued throughout the duration of the block and asserts that + # the job has been performed with the given arguments in the block. # # def test_assert_performed_with # assert_performed_with(job: MyJob, args: [1,2,3], queue: 'high') do @@ -349,20 +394,31 @@ module ActiveJob # MyJob.set(wait_until: Date.tomorrow.noon).perform_later # end # end - def assert_performed_with(job: nil, args: nil, at: nil, queue: nil) - original_performed_jobs_count = performed_jobs.count + def assert_performed_with(job: nil, args: nil, at: nil, queue: nil, &block) expected = { job: job, args: args, at: at, queue: queue }.compact serialized_args = serialize_args_for_assertion(expected) - perform_enqueued_jobs { yield } - in_block_jobs = performed_jobs.drop(original_performed_jobs_count) - matching_job = in_block_jobs.find do |in_block_job| - serialized_args.all? { |key, value| value == in_block_job[key] } + + if block_given? + original_performed_jobs_count = performed_jobs.count + + perform_enqueued_jobs(&block) + + jobs = performed_jobs.drop(original_performed_jobs_count) + else + jobs = performed_jobs end + + matching_job = jobs.find do |performed_job| + serialized_args.all? { |key, value| value == performed_job[key] } + end + assert matching_job, "No performed job found with #{expected}" instantiate_job(matching_job) end - # Performs all enqueued jobs in the duration of the block. + # Performs all enqueued jobs. If a block is given, performs all of the jobs + # that were enqueued throughout the duration of the block. If a block is + # not given, performs all of the enqueued jobs up to this point in the test. # # def test_perform_enqueued_jobs # perform_enqueued_jobs do @@ -371,6 +427,14 @@ module ActiveJob # assert_performed_jobs 1 # end # + # def test_perform_enqueued_jobs_without_block + # MyJob.perform_later(1, 2, 3) + # + # perform_enqueued_jobs + # + # assert_performed_jobs 1 + # end + # # This method also supports filtering. If the +:only+ option is specified, # then only the listed job(s) will be performed. # @@ -393,24 +457,42 @@ module ActiveJob # assert_performed_jobs 1 # end # - def perform_enqueued_jobs(only: nil, except: nil) + # If the +:queue+ option is specified, + # then only the job(s) enqueued to a specific queue will be performed. + # + # def test_perform_enqueued_jobs_with_queue + # perform_enqueued_jobs queue: :some_queue do + # MyJob.set(queue: :some_queue).perform_later(1, 2, 3) # will be performed + # HelloJob.set(queue: :other_queue).perform_later(1, 2, 3) # will not be performed + # end + # assert_performed_jobs 1 + # end + # + def perform_enqueued_jobs(only: nil, except: nil, queue: nil) + return flush_enqueued_jobs(only: only, except: except, queue: queue) unless block_given? + validate_option(only: only, except: except) + old_perform_enqueued_jobs = queue_adapter.perform_enqueued_jobs old_perform_enqueued_at_jobs = queue_adapter.perform_enqueued_at_jobs old_filter = queue_adapter.filter old_reject = queue_adapter.reject + old_queue = queue_adapter.queue begin queue_adapter.perform_enqueued_jobs = true queue_adapter.perform_enqueued_at_jobs = true queue_adapter.filter = only queue_adapter.reject = except + queue_adapter.queue = queue + yield ensure queue_adapter.perform_enqueued_jobs = old_perform_enqueued_jobs queue_adapter.perform_enqueued_at_jobs = old_perform_enqueued_at_jobs queue_adapter.filter = old_filter queue_adapter.reject = old_reject + queue_adapter.queue = old_queue end end @@ -432,22 +514,44 @@ module ActiveJob performed_jobs.clear end - def enqueued_jobs_size(only: nil, except: nil, queue: nil) + def jobs_with(jobs, only: nil, except: nil, queue: nil) validate_option(only: only, except: except) - enqueued_jobs.count do |job| + + jobs.count do |job| job_class = job.fetch(:job) + if only next false unless Array(only).include?(job_class) elsif except next false if Array(except).include?(job_class) end + if queue next false unless queue.to_s == job.fetch(:queue, job_class.queue_name) end + + yield job if block_given? + true end end + def enqueued_jobs_with(only: nil, except: nil, queue: nil, &block) + jobs_with(enqueued_jobs, only: only, except: except, queue: queue, &block) + end + + def performed_jobs_with(only: nil, except: nil, queue: nil, &block) + jobs_with(performed_jobs, only: only, except: except, queue: queue, &block) + end + + def flush_enqueued_jobs(only: nil, except: nil, queue: nil) + enqueued_jobs_with(only: only, except: except, queue: queue) do |payload| + args = ActiveJob::Arguments.deserialize(payload[:args]) + instantiate_job(payload.merge(args: args)).perform_now + queue_adapter.performed_jobs << payload + end + end + def serialize_args_for_assertion(args) args.dup.tap do |serialized_args| serialized_args[:args] = ActiveJob::Arguments.serialize(serialized_args[:args]) if serialized_args[:args] diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb index 47d4e3c0c2..37bb65538a 100644 --- a/activejob/test/cases/exceptions_test.rb +++ b/activejob/test/cases/exceptions_test.rb @@ -2,6 +2,7 @@ require "helper" require "jobs/retry_job" +require "models/person" class ExceptionsTest < ActiveJob::TestCase setup do @@ -131,4 +132,11 @@ class ExceptionsTest < ActiveJob::TestCase assert_equal [ "Raised SecondDiscardableErrorOfTwo for the 1st time" ], JobBuffer.values end end + + test "successfully retry job throwing DeserializationError" do + perform_enqueued_jobs do + RetryJob.perform_later Person.new(404), 5 + assert_equal ["Raised ActiveJob::DeserializationError for the 5 time"], JobBuffer.values + end + end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index d0a21a5da3..805dd80ad1 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -610,7 +610,7 @@ class EnqueuedJobsTest < ActiveJob::TestCase end class PerformedJobsTest < ActiveJob::TestCase - def test_performed_enqueue_jobs_with_only_option_doesnt_leak_outside_the_block + def test_perform_enqueued_jobs_with_only_option_doesnt_leak_outside_the_block assert_nil queue_adapter.filter perform_enqueued_jobs only: HelloJob do assert_equal HelloJob, queue_adapter.filter @@ -618,7 +618,13 @@ class PerformedJobsTest < ActiveJob::TestCase assert_nil queue_adapter.filter end - def test_performed_enqueue_jobs_with_except_option_doesnt_leak_outside_the_block + def test_perform_enqueued_jobs_without_block_with_only_option_doesnt_leak + perform_enqueued_jobs only: HelloJob + + assert_nil queue_adapter.filter + end + + def test_perform_enqueued_jobs_with_except_option_doesnt_leak_outside_the_block assert_nil queue_adapter.reject perform_enqueued_jobs except: HelloJob do assert_equal HelloJob, queue_adapter.reject @@ -626,6 +632,150 @@ class PerformedJobsTest < ActiveJob::TestCase assert_nil queue_adapter.reject end + def test_perform_enqueued_jobs_without_block_with_except_option_doesnt_leak + perform_enqueued_jobs except: HelloJob + + assert_nil queue_adapter.reject + end + + def test_perform_enqueued_jobs_with_queue_option_doesnt_leak_outside_the_block + assert_nil queue_adapter.queue + perform_enqueued_jobs queue: :some_queue do + assert_equal :some_queue, queue_adapter.queue + end + assert_nil queue_adapter.queue + end + + def test_perform_enqueued_jobs_without_block_with_queue_option_doesnt_leak + perform_enqueued_jobs queue: :some_queue + + assert_nil queue_adapter.reject + end + + def test_perform_enqueued_jobs_with_block + perform_enqueued_jobs do + HelloJob.perform_later("kevin") + LoggingJob.perform_later("bogdan") + end + + assert_performed_jobs 2 + end + + def test_perform_enqueued_jobs_without_block + HelloJob.perform_later("kevin") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + assert_performed_jobs 2 + end + + def test_perform_enqueued_jobs_with_block_with_only_option + perform_enqueued_jobs only: LoggingJob do + HelloJob.perform_later("kevin") + LoggingJob.perform_later("bogdan") + end + + assert_performed_jobs 1 + assert_performed_jobs 1, only: LoggingJob + end + + def test_perform_enqueued_jobs_without_block_with_only_option + HelloJob.perform_later("kevin") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs only: LoggingJob + + assert_performed_jobs 1 + assert_performed_jobs 1, only: LoggingJob + end + + def test_perform_enqueued_jobs_with_block_with_except_option + perform_enqueued_jobs except: HelloJob do + HelloJob.perform_later("kevin") + LoggingJob.perform_later("bogdan") + end + + assert_performed_jobs 1 + assert_performed_jobs 1, only: LoggingJob + end + + def test_perform_enqueued_jobs_without_block_with_except_option + HelloJob.perform_later("kevin") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs except: HelloJob + + assert_performed_jobs 1 + assert_performed_jobs 1, only: LoggingJob + end + + def test_perform_enqueued_jobs_with_block_with_queue_option + perform_enqueued_jobs queue: :some_queue do + HelloJob.set(queue: :some_queue).perform_later("kevin") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.perform_later("bogdan") + end + + assert_performed_jobs 1 + assert_performed_jobs 1, only: HelloJob, queue: :some_queue + end + + def test_perform_enqueued_jobs_without_block_with_queue_option + HelloJob.set(queue: :some_queue).perform_later("kevin") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs queue: :some_queue + + assert_performed_jobs 1 + assert_performed_jobs 1, only: HelloJob, queue: :some_queue + end + + def test_perform_enqueued_jobs_with_block_with_only_and_queue_options + perform_enqueued_jobs only: HelloJob, queue: :other_queue do + HelloJob.set(queue: :some_queue).perform_later("kevin") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("bogdan") + end + + assert_performed_jobs 1 + assert_performed_jobs 1, only: HelloJob, queue: :other_queue + end + + def test_perform_enqueued_jobs_without_block_with_only_and_queue_options + HelloJob.set(queue: :some_queue).perform_later("kevin") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("bogdan") + + perform_enqueued_jobs only: HelloJob, queue: :other_queue + + assert_performed_jobs 1 + assert_performed_jobs 1, only: HelloJob, queue: :other_queue + end + + def test_perform_enqueued_jobs_with_block_with_except_and_queue_options + perform_enqueued_jobs except: HelloJob, queue: :other_queue do + HelloJob.set(queue: :other_queue).perform_later("kevin") + LoggingJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("bogdan") + end + + assert_performed_jobs 1 + assert_performed_jobs 1, only: LoggingJob, queue: :other_queue + end + + def test_perform_enqueued_jobs_without_block_with_except_and_queue_options + HelloJob.set(queue: :other_queue).perform_later("kevin") + LoggingJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("bogdan") + + perform_enqueued_jobs except: HelloJob, queue: :other_queue + + assert_performed_jobs 1 + assert_performed_jobs 1, only: LoggingJob, queue: :other_queue + end + def test_assert_performed_jobs assert_nothing_raised do assert_performed_jobs 1 do @@ -731,6 +881,28 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_performed_jobs_without_block_with_only_option + HelloJob.perform_later("jeremy") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + assert_performed_jobs 1, only: HelloJob + end + + def test_assert_performed_jobs_without_block_with_only_option_failure + LoggingJob.perform_later("jeremy") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, only: HelloJob + end + + assert_match(/1 .* but 0/, error.message) + end + def test_assert_performed_jobs_with_except_option assert_nothing_raised do assert_performed_jobs 1, except: LoggingJob do @@ -740,6 +912,28 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_performed_jobs_without_block_with_except_option + HelloJob.perform_later("jeremy") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + assert_performed_jobs 1, except: HelloJob + end + + def test_assert_performed_jobs_without_block_with_except_option_failure + HelloJob.perform_later("jeremy") + HelloJob.perform_later("bogdan") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, except: HelloJob + end + + assert_match(/1 .* but 0/, error.message) + end + def test_assert_performed_jobs_with_only_and_except_option error = assert_raise ArgumentError do assert_performed_jobs 1, only: HelloJob, except: HelloJob do @@ -751,6 +945,19 @@ class PerformedJobsTest < ActiveJob::TestCase assert_match(/`:only` and `:except`/, error.message) end + def test_assert_performed_jobs_without_block_with_only_and_except_options + error = assert_raise ArgumentError do + HelloJob.perform_later("jeremy") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + assert_performed_jobs 1, only: HelloJob, except: HelloJob + end + + assert_match(/`:only` and `:except`/, error.message) + end + def test_assert_performed_jobs_with_only_option_as_array assert_nothing_raised do assert_performed_jobs 2, only: [HelloJob, LoggingJob] do @@ -876,6 +1083,134 @@ class PerformedJobsTest < ActiveJob::TestCase assert_match(/`:only` and `:except`/, error.message) end + def test_assert_performed_jobs_with_queue_option + assert_performed_jobs 1, queue: :some_queue do + HelloJob.set(queue: :some_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + end + end + + def test_assert_performed_jobs_with_queue_option_failure + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, queue: :some_queue do + HelloJob.set(queue: :other_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + end + end + + assert_match(/1 .* but 0/, error.message) + end + + def test_assert_performed_jobs_without_block_with_queue_option + HelloJob.set(queue: :some_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + + perform_enqueued_jobs + + assert_performed_jobs 1, queue: :some_queue + end + + def test_assert_performed_jobs_without_block_with_queue_option_failure + HelloJob.set(queue: :other_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, queue: :some_queue + end + + assert_match(/1 .* but 0/, error.message) + end + + def test_assert_performed_jobs_with_only_and_queue_options + assert_performed_jobs 1, only: HelloJob, queue: :some_queue do + HelloJob.set(queue: :some_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + def test_assert_performed_jobs_with_only_and_queue_options_failure + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, only: HelloJob, queue: :some_queue do + HelloJob.set(queue: :other_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + assert_match(/1 .* but 0/, error.message) + end + + def test_assert_performed_jobs_without_block_with_only_and_queue_options + HelloJob.set(queue: :some_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + assert_performed_jobs 1, only: HelloJob, queue: :some_queue + end + + def test_assert_performed_jobs_without_block_with_only_and_queue_options_failure + HelloJob.set(queue: :other_queue).perform_later("jeremy") + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, only: HelloJob, queue: :some_queue + end + + assert_match(/1 .* but 0/, error.message) + end + + def test_assert_performed_jobs_with_except_and_queue_options + assert_performed_jobs 1, except: HelloJob, queue: :other_queue do + HelloJob.set(queue: :other_queue).perform_later("jeremy") + LoggingJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("jeremy") + end + end + + def test_assert_performed_jobs_with_except_and_queue_options_failure + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, except: HelloJob, queue: :other_queue do + HelloJob.set(queue: :other_queue).perform_later("jeremy") + LoggingJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + assert_match(/1 .* but 0/, error.message) + end + + def test_assert_performed_jobs_without_block_with_except_and_queue_options + HelloJob.set(queue: :other_queue).perform_later("jeremy") + LoggingJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("jeremy") + + perform_enqueued_jobs + + assert_performed_jobs 1, except: HelloJob, queue: :other_queue + end + + def test_assert_performed_jobs_without_block_with_except_and_queue_options_failure + HelloJob.set(queue: :other_queue).perform_later("jeremy") + LoggingJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_jobs 1, except: HelloJob, queue: :other_queue + end + + assert_match(/1 .* but 0/, error.message) + end + def test_assert_no_performed_jobs_with_only_option assert_nothing_raised do assert_no_performed_jobs only: HelloJob do @@ -884,6 +1219,26 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_no_performed_jobs_without_block_with_only_option + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + assert_no_performed_jobs only: HelloJob + end + + def test_assert_no_performed_jobs_without_block_with_only_option_failure + HelloJob.perform_later("bogdan") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs only: HelloJob + end + + assert_match(/0 .* but 1/, error.message) + end + def test_assert_no_performed_jobs_with_except_option assert_nothing_raised do assert_no_performed_jobs except: LoggingJob do @@ -892,6 +1247,26 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_no_performed_jobs_without_block_with_except_option + HelloJob.perform_later("jeremy") + + perform_enqueued_jobs + + assert_no_performed_jobs except: HelloJob + end + + def test_assert_no_performed_jobs_without_block_with_except_option_failure + LoggingJob.perform_later("jeremy") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs except: HelloJob + end + + assert_match(/0 .* but 1/, error.message) + end + def test_assert_no_performed_jobs_with_only_and_except_option error = assert_raise ArgumentError do assert_no_performed_jobs only: HelloJob, except: HelloJob do @@ -902,6 +1277,19 @@ class PerformedJobsTest < ActiveJob::TestCase assert_match(/`:only` and `:except`/, error.message) end + def test_assert_no_performed_jobs_without_block_with_only_and_except_options + error = assert_raise ArgumentError do + HelloJob.perform_later("jeremy") + LoggingJob.perform_later("bogdan") + + perform_enqueued_jobs + + assert_no_performed_jobs only: HelloJob, except: HelloJob + end + + assert_match(/`:only` and `:except`/, error.message) + end + def test_assert_no_performed_jobs_with_only_option_as_array assert_nothing_raised do assert_no_performed_jobs only: [HelloJob, RescueJob] do @@ -962,13 +1350,141 @@ class PerformedJobsTest < ActiveJob::TestCase assert_match(/`:only` and `:except`/, error.message) end - def test_assert_performed_job + def test_assert_no_performed_jobs_with_queue_option + assert_no_performed_jobs queue: :some_queue do + HelloJob.set(queue: :other_queue).perform_later("jeremy") + end + end + + def test_assert_no_performed_jobs_with_queue_option_failure + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs queue: :some_queue do + HelloJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + assert_match(/0 .* but 1/, error.message) + end + + def test_assert_no_performed_jobs_without_block_with_queue_option + HelloJob.set(queue: :other_queue).perform_later("jeremy") + + perform_enqueued_jobs + + assert_no_performed_jobs queue: :some_queue + end + + def test_assert_no_performed_jobs_without_block_with_queue_option_failure + HelloJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs queue: :some_queue + end + + assert_match(/0 .* but 1/, error.message) + end + + def test_assert_no_performed_jobs_with_only_and_queue_options + assert_no_performed_jobs only: HelloJob, queue: :some_queue do + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + def test_assert_no_performed_jobs_with_only_and_queue_options_failure + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs only: HelloJob, queue: :some_queue do + HelloJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + assert_match(/0 .* but 1/, error.message) + end + + def test_assert_no_performed_jobs_without_block_with_only_and_queue_options + HelloJob.set(queue: :other_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + assert_no_performed_jobs only: HelloJob, queue: :some_queue + end + + def test_assert_no_performed_jobs_without_block_with_only_and_queue_options_failure + HelloJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs only: HelloJob, queue: :some_queue + end + + assert_match(/0 .* but 1/, error.message) + end + + def test_assert_no_performed_jobs_with_except_and_queue_options + assert_no_performed_jobs except: HelloJob, queue: :some_queue do + HelloJob.set(queue: :other_queue).perform_later("bogdan") + HelloJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("jeremy") + end + end + + def test_assert_no_performed_jobs_with_except_and_queue_options_failure + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs except: HelloJob, queue: :some_queue do + HelloJob.set(queue: :other_queue).perform_later("bogdan") + HelloJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + end + end + + assert_match(/0 .* but 1/, error.message) + end + + def test_assert_no_performed_jobs_without_block_with_except_and_queue_options + HelloJob.set(queue: :other_queue).perform_later("bogdan") + HelloJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :other_queue).perform_later("jeremy") + + perform_enqueued_jobs + + assert_no_performed_jobs except: HelloJob, queue: :some_queue + end + + def test_assert_no_performed_jobs_without_block_with_except_and_queue_options_failure + HelloJob.set(queue: :other_queue).perform_later("bogdan") + HelloJob.set(queue: :some_queue).perform_later("bogdan") + LoggingJob.set(queue: :some_queue).perform_later("jeremy") + + perform_enqueued_jobs + + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_no_performed_jobs except: HelloJob, queue: :some_queue + end + + assert_match(/0 .* but 1/, error.message) + end + + def test_assert_performed_with assert_performed_with(job: NestedJob, queue: "default") do NestedJob.perform_later end end - def test_assert_performed_job_returns + def test_assert_performed_with_without_block + NestedJob.perform_later + + perform_enqueued_jobs + + assert_performed_with(job: NestedJob, queue: "default") + end + + def test_assert_performed_with_returns job = assert_performed_with(job: NestedJob, queue: "default") do NestedJob.perform_later end @@ -979,7 +1495,20 @@ class PerformedJobsTest < ActiveJob::TestCase assert_equal "default", job.queue_name end - def test_assert_performed_job_failure + def test_assert_performed_with_without_block_returns + NestedJob.perform_later + + perform_enqueued_jobs + + job = assert_performed_with(job: NestedJob, queue: "default") + + assert_instance_of NestedJob, job + assert_nil job.scheduled_at + assert_equal [], job.arguments + assert_equal "default", job.queue_name + end + + def test_assert_performed_with_failure assert_raise ActiveSupport::TestCase::Assertion do assert_performed_with(job: LoggingJob) do HelloJob.perform_later @@ -993,7 +1522,23 @@ class PerformedJobsTest < ActiveJob::TestCase end end - def test_assert_performed_job_with_at_option + def test_assert_performed_with_without_block_failure + HelloJob.perform_later + + perform_enqueued_jobs + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: LoggingJob) + end + + HelloJob.set(queue: "important").perform_later + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, queue: "low") + end + end + + def test_assert_performed_with_with_at_option assert_performed_with(job: HelloJob, at: Date.tomorrow.noon) do HelloJob.set(wait_until: Date.tomorrow.noon).perform_later end @@ -1005,14 +1550,37 @@ class PerformedJobsTest < ActiveJob::TestCase end end - def test_assert_performed_job_with_global_id_args + def test_assert_performed_with_without_block_with_at_option + HelloJob.set(wait_until: Date.tomorrow.noon).perform_later + + perform_enqueued_jobs + + assert_performed_with(job: HelloJob, at: Date.tomorrow.noon) + + HelloJob.set(wait_until: Date.tomorrow.noon).perform_later + + perform_enqueued_jobs + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, at: Date.today.noon) + end + end + + def test_assert_performed_wiht_with_global_id_args ricardo = Person.new(9) assert_performed_with(job: HelloJob, args: [ricardo]) do HelloJob.perform_later(ricardo) end end - def test_assert_performed_job_failure_with_global_id_args + def test_assert_performed_with_without_bllock_with_global_id_args + ricardo = Person.new(9) + HelloJob.perform_later(ricardo) + perform_enqueued_jobs + assert_performed_with(job: HelloJob, args: [ricardo]) + end + + def test_assert_performed_with_failure_with_global_id_args ricardo = Person.new(9) wilma = Person.new(11) error = assert_raise ActiveSupport::TestCase::Assertion do @@ -1024,7 +1592,19 @@ class PerformedJobsTest < ActiveJob::TestCase assert_equal "No performed job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message end - def test_assert_performed_job_does_not_change_jobs_count + def test_assert_performed_with_without_block_failure_with_global_id_args + ricardo = Person.new(9) + wilma = Person.new(11) + HelloJob.perform_later(ricardo) + perform_enqueued_jobs + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, args: [wilma]) + end + + assert_equal "No performed job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message + end + + def test_assert_performed_with_does_not_change_jobs_count assert_performed_with(job: HelloJob) do HelloJob.perform_later end @@ -1035,6 +1615,18 @@ class PerformedJobsTest < ActiveJob::TestCase assert_equal 2, queue_adapter.performed_jobs.count end + + def test_assert_performed_with_without_block_does_not_change_jobs_count + HelloJob.perform_later + perform_enqueued_jobs + assert_performed_with(job: HelloJob) + + perform_enqueued_jobs + HelloJob.perform_later + assert_performed_with(job: HelloJob) + + assert_equal 2, queue_adapter.performed_jobs.count + end end class OverrideQueueAdapterTest < ActiveJob::TestCase diff --git a/activejob/test/jobs/retry_job.rb b/activejob/test/jobs/retry_job.rb index 1383fffd7d..68dc17e16c 100644 --- a/activejob/test/jobs/retry_job.rb +++ b/activejob/test/jobs/retry_job.rb @@ -24,6 +24,7 @@ class RetryJob < ActiveJob::Base retry_on ExponentialWaitTenAttemptsError, wait: :exponentially_longer, attempts: 10 retry_on CustomWaitTenAttemptsError, wait: ->(executions) { executions * 2 }, attempts: 10 retry_on(CustomCatchError) { |job, error| JobBuffer.add("Dealt with a job that failed to retry in a custom way after #{job.arguments.second} attempts. Message: #{error.message}") } + retry_on(ActiveJob::DeserializationError) { |job, error| JobBuffer.add("Raised #{error.class} for the #{job.executions} time") } discard_on DiscardableError discard_on FirstDiscardableErrorOfTwo, SecondDiscardableErrorOfTwo diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 8f80838a33..4bf96e11b0 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,19 @@ +* Fix `ActiveModel::Serializers::JSON#as_json` method for timestamps. + + Before: + ``` + contact = Contact.new(created_at: Time.utc(2006, 8, 1)) + contact.as_json["created_at"] # => 2006-08-01 00:00:00 UTC + ``` + + After: + ``` + contact = Contact.new(created_at: Time.utc(2006, 8, 1)) + contact.as_json["created_at"] # => "2006-08-01T00:00:00.000Z" + ``` + + *Bogdan Gusiev* + * Allows configurable attribute name for `#has_secure_password`. This still defaults to an attribute named 'password', causing no breaking change. There is a new method `#authenticate_XXX` where XXX is the diff --git a/activemodel/lib/active_model/serializers/json.rb b/activemodel/lib/active_model/serializers/json.rb index 25e1541d66..f77fb98c32 100644 --- a/activemodel/lib/active_model/serializers/json.rb +++ b/activemodel/lib/active_model/serializers/json.rb @@ -26,13 +26,13 @@ module ActiveModel # user = User.find(1) # user.as_json # # => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true} + # # "created_at" => "2006-08-01T17:27:133.000Z", "awesome" => true} # # ActiveRecord::Base.include_root_in_json = true # # user.as_json # # => { "user" => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true } } + # # "created_at" => "2006-08-01T17:27:13.000Z", "awesome" => true } } # # This behavior can also be achieved by setting the <tt>:root</tt> option # to +true+ as in: @@ -40,7 +40,7 @@ module ActiveModel # user = User.find(1) # user.as_json(root: true) # # => { "user" => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true } } + # # "created_at" => "2006-08-01T17:27:13.000Z", "awesome" => true } } # # Without any +options+, the returned Hash will include all the model's # attributes. @@ -48,7 +48,7 @@ module ActiveModel # user = User.find(1) # user.as_json # # => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true} + # # "created_at" => "2006-08-01T17:27:13.000Z", "awesome" => true} # # The <tt>:only</tt> and <tt>:except</tt> options can be used to limit # the attributes included, and work similar to the +attributes+ method. @@ -63,14 +63,14 @@ module ActiveModel # # user.as_json(methods: :permalink) # # => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true, + # # "created_at" => "2006-08-01T17:27:13.000Z", "awesome" => true, # # "permalink" => "1-konata-izumi" } # # To include associations use <tt>:include</tt>: # # user.as_json(include: :posts) # # => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true, + # # "created_at" => "2006-08-01T17:27:13.000Z", "awesome" => true, # # "posts" => [ { "id" => 1, "author_id" => 1, "title" => "Welcome to the weblog" }, # # { "id" => 2, "author_id" => 1, "title" => "So I was thinking" } ] } # @@ -81,7 +81,7 @@ module ActiveModel # only: :body } }, # only: :title } }) # # => { "id" => 1, "name" => "Konata Izumi", "age" => 16, - # # "created_at" => "2006/08/01", "awesome" => true, + # # "created_at" => "2006-08-01T17:27:13.000Z", "awesome" => true, # # "posts" => [ { "comments" => [ { "body" => "1st post!" }, { "body" => "Second!" } ], # # "title" => "Welcome to the weblog" }, # # { "comments" => [ { "body" => "Don't think too hard" } ], @@ -93,11 +93,12 @@ module ActiveModel include_root_in_json end + hash = serializable_hash(options).as_json if root root = model_name.element if root == true - { root => serializable_hash(options) } + { root => hash } else - serializable_hash(options) + hash end end diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 0478915be7..3753040316 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -23,6 +23,8 @@ module ActiveModel if record.respond_to?(came_from_user) && record.public_send(came_from_user) raw_value = record.read_attribute_before_type_cast(attr_name) + elsif record.respond_to?(:read_attribute) + raw_value = record.read_attribute(attr_name) end raw_value ||= value diff --git a/activemodel/test/cases/serializers/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb index aae98c9fe4..625e0a427a 100644 --- a/activemodel/test/cases/serializers/json_serialization_test.rb +++ b/activemodel/test/cases/serializers/json_serialization_test.rb @@ -129,6 +129,10 @@ class JsonSerializationTest < ActiveModel::TestCase assert_equal :name, options[:except] end + test "as_json should serialize timestamps" do + assert_equal "2006-08-01T00:00:00.000Z", @contact.as_json["created_at"] + end + test "as_json should return a hash if include_root_in_json is true" do begin original_include_root_in_json = Contact.include_root_in_json @@ -138,7 +142,7 @@ class JsonSerializationTest < ActiveModel::TestCase assert_kind_of Hash, json assert_kind_of Hash, json["contact"] %w(name age created_at awesome preferences).each do |field| - assert_equal @contact.send(field), json["contact"][field] + assert_equal @contact.send(field).as_json, json["contact"][field] end ensure Contact.include_root_in_json = original_include_root_in_json diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d1ae41ab97..f6a6aa05a9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,15 @@ +* SQLite3 adapter `alter_table` method restores foreign keys. + + *Yasuo Honda* + +* Allow `:to_table` option to `invert_remove_foreign_key`. + + Example: + + remove_foreign_key :accounts, to_table: :owners + + *Nikolay Epifanov*, *Rich Chen* + * Add environment & load_config dependency to `bin/rake db:seed` to enable seed load in environments without Rails and custom DB configuration diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index b5fb0092d7..0166ed98ca 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -34,7 +34,7 @@ module ActiveRecord::Associations::Builder # :nodoc: foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column - if (@_after_replace_counter_called ||= false) + if @_after_replace_counter_called ||= false @_after_replace_counter_called = false elsif association(reflection.name).target_changed? if reflection.polymorphic? 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 d2b7817b45..294a3dc32c 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -73,7 +73,7 @@ module ActiveRecord # `skip_time_zone_conversion_for_attributes` would not be picked up. subclass.class_eval do matcher = ->(name, type) { create_time_zone_conversion_attribute?(name, type) } - decorate_matching_attribute_types(matcher, :_time_zone_conversion) do |type| + decorate_matching_attribute_types(matcher, "_time_zone_conversion") do |type| TimeZoneConverter.new(type) end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 3be0906f2a..4702de1964 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -980,11 +980,18 @@ module ActiveRecord # # remove_foreign_key :accounts, column: :owner_id # + # Removes the foreign key on +accounts.owner_id+. + # + # remove_foreign_key :accounts, to_table: :owners + # # Removes the foreign key named +special_fk_name+ on the +accounts+ table. # # remove_foreign_key :accounts, name: :special_fk_name # - # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key. + # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key + # with an addition of + # [<tt>:to_table</tt>] + # The name of the table that contains the referenced primary key. def remove_foreign_key(from_table, options_or_to_table = {}) return unless supports_foreign_keys? diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb index 231278c184..79351bc3a4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/array/extract" + module ActiveRecord module ConnectionAdapters module PostgreSQL @@ -16,12 +18,12 @@ module ActiveRecord def run(records) nodes = records.reject { |row| @store.key? row["oid"].to_i } - mapped, nodes = nodes.partition { |row| @store.key? row["typname"] } - ranges, nodes = nodes.partition { |row| row["typtype"] == "r".freeze } - enums, nodes = nodes.partition { |row| row["typtype"] == "e".freeze } - domains, nodes = nodes.partition { |row| row["typtype"] == "d".freeze } - arrays, nodes = nodes.partition { |row| row["typinput"] == "array_in".freeze } - composites, nodes = nodes.partition { |row| row["typelem"].to_i != 0 } + mapped = nodes.extract! { |row| @store.key? row["typname"] } + ranges = nodes.extract! { |row| row["typtype"] == "r".freeze } + enums = nodes.extract! { |row| row["typtype"] == "e".freeze } + domains = nodes.extract! { |row| row["typtype"] == "d".freeze } + arrays = nodes.extract! { |row| row["typinput"] == "array_in".freeze } + composites = nodes.extract! { |row| row["typelem"].to_i != 0 } mapped.each { |row| register_mapped_type(row) } enums.each { |row| register_enum_type(row) } diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 26b8113b0d..00da7690a2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -700,6 +700,11 @@ module ActiveRecord sql end + def add_column_for_alter(table_name, column_name, type, options = {}) + return super unless options.key?(:comment) + [super, Proc.new { change_column_comment(table_name, column_name, options[:comment]) }] + end + def change_column_for_alter(table_name, column_name, type, options = {}) sqls = [change_column_sql(table_name, column_name, type, options)] sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) @@ -708,7 +713,6 @@ module ActiveRecord sqls end - # Changes the default value of a table column. def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc: column = column_for(table_name, column_name) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index bee74dc33d..c61e94f159 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -409,12 +409,23 @@ module ActiveRecord def alter_table(table_name, options = {}) altered_table_name = "a#{table_name}" - caller = lambda { |definition| yield definition if block_given? } + foreign_keys = foreign_keys(table_name) + + caller = lambda do |definition| + rename = options[:rename] || {} + foreign_keys.each do |fk| + if column = rename[fk.options[:column]] + fk.options[:column] = column + end + definition.foreign_key(fk.to_table, fk.options) + end + + yield definition if block_given? + end transaction do disable_referential_integrity do - move_table(table_name, altered_table_name, - options.merge(temporary: true)) + move_table(table_name, altered_table_name, options.merge(temporary: true)) move_table(altered_table_name, table_name, &caller) end end @@ -446,6 +457,7 @@ module ActiveRecord primary_key: column_name == from_primary_key ) end + yield @definition if block_given? end copy_table_indexes(from, to, options[:rename] || {}) diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 7f096bb532..1030b2368b 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -165,7 +165,7 @@ module ActiveRecord def inherited(subclass) subclass.class_eval do is_lock_column = ->(name, _) { lock_optimistically && name == locking_column } - decorate_matching_attribute_types(is_lock_column, :_optimistic_locking) do |type| + decorate_matching_attribute_types(is_lock_column, "_optimistic_locking") do |type| LockingType.new(type) end end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 1ae6840921..6b84431343 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -4,6 +4,8 @@ module ActiveRecord class LogSubscriber < ActiveSupport::LogSubscriber IGNORE_PAYLOAD_NAMES = ["SCHEMA", "EXPLAIN"] + class_attribute :backtrace_cleaner, default: ActiveSupport::BacktraceCleaner.new + def self.runtime=(value) ActiveRecord::RuntimeRegistry.sql_runtime = value end @@ -100,21 +102,15 @@ module ActiveRecord end def log_query_source - location = extract_query_source_location(caller_locations) - - if location - source = "#{location.path}:#{location.lineno}" - source = source.sub("#{::Rails.root}/", "") if defined?(::Rails.root) + source = extract_query_source_location(caller) + if source logger.debug(" ↳ #{source}") end end - RAILS_GEM_ROOT = File.expand_path("../../..", __dir__) + "/" - PATHS_TO_IGNORE = /\A(#{RAILS_GEM_ROOT}|#{RbConfig::CONFIG["rubylibdir"]})/ - def extract_query_source_location(locations) - locations.find { |line| line.absolute_path && !line.absolute_path.match?(PATHS_TO_IGNORE) } + backtrace_cleaner.clean(locations).first end end end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 087632b10f..dea6d4ec08 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -214,11 +214,24 @@ module ActiveRecord end def invert_remove_foreign_key(args) - from_table, to_table, remove_options = args - raise ActiveRecord::IrreversibleMigration, "remove_foreign_key is only reversible if given a second table" if to_table.nil? || to_table.is_a?(Hash) + from_table, options_or_to_table, options_or_nil = args + + to_table = if options_or_to_table.is_a?(Hash) + options_or_to_table[:to_table] + else + options_or_to_table + end + + remove_options = if options_or_to_table.is_a?(Hash) + options_or_to_table.except(:to_table) + else + options_or_nil + end + + raise ActiveRecord::IrreversibleMigration, "remove_foreign_key is only reversible if given a second table" if to_table.nil? reversed_args = [from_table, to_table] - reversed_args << remove_options if remove_options + reversed_args << remove_options if remove_options.present? [:add_foreign_key, reversed_args] end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 009f412234..7ece083fd4 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -77,6 +77,10 @@ module ActiveRecord ActiveSupport.on_load(:active_record) { self.logger ||= ::Rails.logger } end + initializer "active_record.backtrace_cleaner" do + ActiveSupport.on_load(:active_record) { LogSubscriber.backtrace_cleaner = ::Rails.backtrace_cleaner } + end + initializer "active_record.migration_error" do if config.active_record.delete(:migration_error) == :page_load config.app_middleware.insert_after ::ActionDispatch::Callbacks, diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 10e4779ca4..07b16a0740 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -152,10 +152,10 @@ module ActiveRecord def merge_multi_values if other.reordering_value # override any order specified in the original relation - relation.reorder! other.order_values + relation.reorder!(*other.order_values) elsif other.order_values.any? # merge in order_values from relation - relation.order! other.order_values + relation.order!(*other.order_values) end extensions = other.extensions - relation.extensions diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 64bf83e3c1..e5191fa38a 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/array/extract" + module ActiveRecord class PredicateBuilder class ArrayHandler # :nodoc: @@ -11,8 +13,8 @@ module ActiveRecord return attribute.in([]) if value.empty? values = value.map { |x| x.is_a?(Base) ? x.id : x } - nils, values = values.partition(&:nil?) - ranges, values = values.partition { |v| v.is_a?(Range) } + nils = values.extract!(&:nil?) + ranges = values.extract! { |v| v.is_a?(Range) } values_predicate = case values.length diff --git a/activerecord/test/cases/arel/attributes/math_test.rb b/activerecord/test/cases/arel/attributes/math_test.rb index f3aabe4f60..41eea217c0 100644 --- a/activerecord/test/cases/arel/attributes/math_test.rb +++ b/activerecord/test/cases/arel/attributes/math_test.rb @@ -6,35 +6,35 @@ module Arel module Attributes class MathTest < Arel::Spec %i[* /].each do |math_operator| - it "average should be compatiable with #{math_operator}" do + it "average should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].average.public_send(math_operator, 2)).to_sql.must_be_like %{ AVG("users"."id") #{math_operator} 2 } end - it "count should be compatiable with #{math_operator}" do + it "count should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].count.public_send(math_operator, 2)).to_sql.must_be_like %{ COUNT("users"."id") #{math_operator} 2 } end - it "maximum should be compatiable with #{math_operator}" do + it "maximum should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].maximum.public_send(math_operator, 2)).to_sql.must_be_like %{ MAX("users"."id") #{math_operator} 2 } end - it "minimum should be compatiable with #{math_operator}" do + it "minimum should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].minimum.public_send(math_operator, 2)).to_sql.must_be_like %{ MIN("users"."id") #{math_operator} 2 } end - it "attribute node should be compatiable with #{math_operator}" do + it "attribute node should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].public_send(math_operator, 2)).to_sql.must_be_like %{ "users"."id" #{math_operator} 2 @@ -43,35 +43,35 @@ module Arel end %i[+ - & | ^ << >>].each do |math_operator| - it "average should be compatiable with #{math_operator}" do + it "average should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].average.public_send(math_operator, 2)).to_sql.must_be_like %{ (AVG("users"."id") #{math_operator} 2) } end - it "count should be compatiable with #{math_operator}" do + it "count should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].count.public_send(math_operator, 2)).to_sql.must_be_like %{ (COUNT("users"."id") #{math_operator} 2) } end - it "maximum should be compatiable with #{math_operator}" do + it "maximum should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].maximum.public_send(math_operator, 2)).to_sql.must_be_like %{ (MAX("users"."id") #{math_operator} 2) } end - it "minimum should be compatiable with #{math_operator}" do + it "minimum should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].minimum.public_send(math_operator, 2)).to_sql.must_be_like %{ (MIN("users"."id") #{math_operator} 2) } end - it "attribute node should be compatiable with #{math_operator}" do + it "attribute node should be compatible with #{math_operator}" do table = Arel::Table.new :users (table[:id].public_send(math_operator, 2)).to_sql.must_be_like %{ ("users"."id" #{math_operator} 2) diff --git a/activerecord/test/cases/arel/select_manager_test.rb b/activerecord/test/cases/arel/select_manager_test.rb index 6b10d0b612..c17487ae88 100644 --- a/activerecord/test/cases/arel/select_manager_test.rb +++ b/activerecord/test/cases/arel/select_manager_test.rb @@ -278,7 +278,7 @@ module Arel @m2.where(table[:age].lt(99)) end - it "should interect two managers" do + it "should intersect two managers" do # FIXME should this intersect "managers" or "statements" ? # FIXME this probably shouldn't return a node node = @m1.intersect @m2 diff --git a/activerecord/test/cases/arel/visitors/postgres_test.rb b/activerecord/test/cases/arel/visitors/postgres_test.rb index ba9cfcfc64..f7f2c76b6f 100644 --- a/activerecord/test/cases/arel/visitors/postgres_test.rb +++ b/activerecord/test/cases/arel/visitors/postgres_test.rb @@ -215,7 +215,7 @@ module Arel } end - it "should know how to generate paranthesis when supplied with many Dimensions" do + it "should know how to generate parenthesis when supplied with many Dimensions" do dim1 = Arel::Nodes::GroupingElement.new(@table[:name]) dim2 = Arel::Nodes::GroupingElement.new([@table[:bool], @table[:created_at]]) node = Arel::Nodes::Cube.new([dim1, dim2]) @@ -241,7 +241,7 @@ module Arel } end - it "should know how to generate paranthesis when supplied with many Dimensions" do + it "should know how to generate parenthesis when supplied with many Dimensions" do group1 = Arel::Nodes::GroupingElement.new(@table[:name]) group2 = Arel::Nodes::GroupingElement.new([@table[:bool], @table[:created_at]]) node = Arel::Nodes::GroupingSet.new([group1, group2]) @@ -267,7 +267,7 @@ module Arel } end - it "should know how to generate paranthesis when supplied with many Dimensions" do + it "should know how to generate parenthesis when supplied with many Dimensions" do group1 = Arel::Nodes::GroupingElement.new(@table[:name]) group2 = Arel::Nodes::GroupingElement.new([@table[:bool], @table[:created_at]]) node = Arel::Nodes::RollUp.new([group1, group2]) diff --git a/activerecord/test/cases/arel/visitors/to_sql_test.rb b/activerecord/test/cases/arel/visitors/to_sql_test.rb index ce836eded7..e8ac50bfa3 100644 --- a/activerecord/test/cases/arel/visitors/to_sql_test.rb +++ b/activerecord/test/cases/arel/visitors/to_sql_test.rb @@ -427,7 +427,7 @@ module Arel compile(node).must_equal %(("products"."price" - 7)) end - it "should handle Concatination" do + it "should handle Concatenation" do table = Table.new(:users) node = table[:name].concat(table[:name]) compile(node).must_equal %("users"."name" || "users"."name") diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index f414fbf64b..482302055d 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -781,7 +781,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_polymorphic_has_manys_works - assert_equal [10, 20].to_set, pirates(:redbeard).treasure_estimates.map(&:price).to_set + assert_equal ["$10.00", "$20.00"].to_set, pirates(:redbeard).treasure_estimates.map(&:price).to_set end def test_symbols_as_keys diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index d7e898a1c0..9eea34d2b9 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -661,6 +661,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase self.table_name = "books" belongs_to :author, class_name: "SpecialAuthor" has_one :subscription, class_name: "SpecialSupscription", foreign_key: "subscriber_id" + + enum status: [:proposed, :written, :published] end class SpecialAuthor < ActiveRecord::Base @@ -678,6 +680,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase book = SpecialBook.create!(status: "published") author.book = book + assert_equal "published", book.status assert_not_equal 0, SpecialAuthor.joins(:book).where(books: { status: "published" }).count end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 9ac03629c3..6aecf5fa35 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -91,7 +91,9 @@ module ActiveRecord end def test_full_pool_exception + @pool.checkout_timeout = 0.001 # no need to delay test suite by waiting the whole full default timeout @pool.size.times { assert @pool.checkout } + assert_raises(ConnectionTimeoutError) do @pool.checkout end @@ -156,6 +158,48 @@ module ActiveRecord @pool.connections.each { |conn| conn.close if conn.in_use? } end + def test_idle_timeout_configuration + @pool.disconnect! + spec = ActiveRecord::Base.connection_pool.spec + spec.config.merge!(idle_timeout: "0.02") + @pool = ConnectionPool.new(spec) + idle_conn = @pool.checkout + @pool.checkin(idle_conn) + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 0.01 + ) + + @pool.flush + assert_equal 1, @pool.connections.length + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 0.02 + ) + + @pool.flush + assert_equal 0, @pool.connections.length + end + + def test_disable_flush + @pool.disconnect! + spec = ActiveRecord::Base.connection_pool.spec + spec.config.merge!(idle_timeout: -5) + @pool = ConnectionPool.new(spec) + idle_conn = @pool.checkout + @pool.checkin(idle_conn) + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 1 + ) + + @pool.flush + assert_equal 1, @pool.connections.length + end + def test_flush idle_conn = @pool.checkout recent_conn = @pool.checkout @@ -166,9 +210,10 @@ module ActiveRecord assert_equal 3, @pool.connections.length - def idle_conn.seconds_idle - 1000 - end + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 1000 + ) @pool.flush(30) diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 3a11bb081b..1a19b8dafd 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -329,11 +329,24 @@ module ActiveRecord assert_equal [:add_foreign_key, [:dogs, :people, primary_key: "person_id"]], enable end + def test_invert_remove_foreign_key_with_primary_key_and_to_table_in_options + enable = @recorder.inverse_of :remove_foreign_key, [:dogs, to_table: :people, primary_key: "uuid"] + assert_equal [:add_foreign_key, [:dogs, :people, primary_key: "uuid"]], enable + end + def test_invert_remove_foreign_key_with_on_delete_on_update enable = @recorder.inverse_of :remove_foreign_key, [:dogs, :people, on_delete: :nullify, on_update: :cascade] assert_equal [:add_foreign_key, [:dogs, :people, on_delete: :nullify, on_update: :cascade]], enable end + def test_invert_remove_foreign_key_with_to_table_in_options + enable = @recorder.inverse_of :remove_foreign_key, [:dogs, to_table: :people] + assert_equal [:add_foreign_key, [:dogs, :people]], enable + + enable = @recorder.inverse_of :remove_foreign_key, [:dogs, to_table: :people, column: :owner_id] + assert_equal [:add_foreign_key, [:dogs, :people, column: :owner_id]], enable + end + def test_invert_remove_foreign_key_is_irreversible_without_to_table assert_raises ActiveRecord::IrreversibleMigration do @recorder.inverse_of :remove_foreign_key, [:dogs, column: "owner_id"] diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index c471dd1106..bb233fbf74 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -53,16 +53,49 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create? end def test_change_column_of_parent_table - foreign_keys = ActiveRecord::Base.connection.foreign_keys("astronauts") rocket = Rocket.create!(name: "myrocket") rocket.astronauts << Astronaut.create! @connection.change_column_null :rockets, :name, false + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert_equal "myrocket", Rocket.first.name + assert_equal "astronauts", fk.from_table + assert_equal "rockets", fk.to_table + end + + def test_rename_column_of_child_table + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.rename_column :astronauts, :name, :astronaut_name + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert_equal "myrocket", Rocket.first.name + assert_equal "astronauts", fk.from_table + assert_equal "rockets", fk.to_table + end + + def test_rename_reference_column_of_child_table + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.rename_column :astronauts, :rocket_id, :new_rocket_id + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + fk = foreign_keys.first assert_equal "myrocket", Rocket.first.name assert_equal "astronauts", fk.from_table assert_equal "rockets", fk.to_table + assert_equal "new_rocket_id", fk.options[:column] end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index d1292dc53d..868bb40ab2 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -793,12 +793,20 @@ if ActiveRecord::Base.connection.supports_bulk_alter? end def test_adding_multiple_columns - assert_queries(1) do + classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] + expected_query_count = { + "Mysql2Adapter" => 1, + "PostgreSQLAdapter" => 2, # one for bulk change, one for comment + }.fetch(classname) { + raise "need an expected query count for #{classname}" + } + + assert_queries(expected_query_count) do with_bulk_change_table do |t| t.column :name, :string t.string :qualification, :experience t.integer :age, default: 0 - t.date :birthdate + t.date :birthdate, comment: "This is a comment" t.timestamps null: true end end @@ -806,6 +814,7 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert_equal 8, columns.size [:name, :qualification, :experience].each { |s| assert_equal :string, column(s).type } assert_equal "0", column(:age).default + assert_equal "This is a comment", column(:birthdate).comment end def test_removing_columns diff --git a/activerecord/test/cases/reaper_test.rb b/activerecord/test/cases/reaper_test.rb index b034fe3e3b..b630f782bc 100644 --- a/activerecord/test/cases/reaper_test.rb +++ b/activerecord/test/cases/reaper_test.rb @@ -61,9 +61,9 @@ module ActiveRecord def test_reaping_frequency_configuration spec = ActiveRecord::Base.connection_pool.spec.dup - spec.config[:reaping_frequency] = 100 + spec.config[:reaping_frequency] = "10.01" pool = ConnectionPool.new spec - assert_equal 100, pool.reaper.frequency + assert_equal 10.01, pool.reaper.frequency end def test_connection_pool_starts_reaper diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index f53ef1fe35..6e7998d15a 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -121,6 +121,16 @@ class RelationMergingTest < ActiveRecord::TestCase relation = relation.merge(Post.from("posts")) assert_not_empty relation.from_clause end + + def test_merging_with_order_with_binds + relation = Post.all.merge(Post.order([Arel.sql("title LIKE ?"), "%suffix"])) + assert_equal ["title LIKE '%suffix'"], relation.order_values + end + + def test_merging_with_order_without_binds + relation = Post.all.merge(Post.order(Arel.sql("title LIKE '%?'"))) + assert_equal ["title LIKE '%?'"], relation.order_values + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relation/select_test.rb b/activerecord/test/cases/relation/select_test.rb index 0577e6bfdb..dec8a6925d 100644 --- a/activerecord/test/cases/relation/select_test.rb +++ b/activerecord/test/cases/relation/select_test.rb @@ -7,7 +7,7 @@ module ActiveRecord class SelectTest < ActiveRecord::TestCase fixtures :posts - def test_select_with_nil_agrument + def test_select_with_nil_argument expected = Post.select(:title).to_sql assert_equal expected, Post.select(nil).select(:title).to_sql end diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index 4cd4515c3b..1192b30b14 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -322,7 +322,7 @@ class SerializedAttributeTest < ActiveRecord::TestCase topic = Topic.create!(content: {}) topic2 = Topic.create!(content: nil) - assert_equal [topic, topic2], Topic.where(content: nil) + assert_equal [topic, topic2], Topic.where(content: nil).sort_by(&:id) end def test_nil_is_always_persisted_as_null diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index a33877f43a..66763c727f 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -3,11 +3,11 @@ require "cases/helper" require "models/topic" require "models/reply" -require "models/person" require "models/developer" require "models/computer" require "models/parrot" require "models/company" +require "models/price_estimate" class ValidationsTest < ActiveRecord::TestCase fixtures :topics, :developers @@ -183,6 +183,22 @@ class ValidationsTest < ActiveRecord::TestCase assert_not_predicate klass.new(wibble: BigDecimal("97.179")), :valid? end + def test_numericality_validator_wont_be_affected_by_custom_getter + price_estimate = PriceEstimate.new(price: 50) + + assert_equal "$50.00", price_estimate.price + assert_equal 50, price_estimate.price_before_type_cast + assert_equal 50, price_estimate.read_attribute(:price) + + assert_predicate price_estimate, :price_came_from_user? + assert_predicate price_estimate, :valid? + + price_estimate.save! + + assert_not_predicate price_estimate, :price_came_from_user? + assert_predicate price_estimate, :valid? + end + def test_acceptance_validator_doesnt_require_db_connection klass = Class.new(ActiveRecord::Base) do self.table_name = "posts" diff --git a/activerecord/test/models/price_estimate.rb b/activerecord/test/models/price_estimate.rb index f1f88d8d8d..669d0991f7 100644 --- a/activerecord/test/models/price_estimate.rb +++ b/activerecord/test/models/price_estimate.rb @@ -1,6 +1,14 @@ # frozen_string_literal: true class PriceEstimate < ActiveRecord::Base + include ActiveSupport::NumberHelper + belongs_to :estimate_of, polymorphic: true belongs_to :thing, polymorphic: true + + validates_numericality_of :price + + def price + number_to_currency super + end end diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 0e9c2b5619..8bfda4799e 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,15 @@ +* Added the `ActiveStorage::SetCurrent` concern for custom Active Storage + controllers that can't inherit from `ActiveStorage::BaseController`. + + *George Claghorn* + +* Active Storage error classes like `ActiveStorage::IntegrityError` and + `ActiveStorage::UnrepresentableError` now inherit from `ActiveStorage::Error` + instead of `StandardError`. This permits rescuing `ActiveStorage::Error` to + handle all Active Storage errors. + + *Andrei Makarov*, *George Claghorn* + * Uploaded files assigned to a record are persisted to storage when the record is saved instead of immediately. diff --git a/activestorage/app/assets/javascripts/activestorage.js b/activestorage/app/assets/javascripts/activestorage.js index d3fd795a3a..375eb6b533 100644 --- a/activestorage/app/assets/javascripts/activestorage.js +++ b/activestorage/app/assets/javascripts/activestorage.js @@ -855,14 +855,22 @@ return DirectUploadsController; }(); var processingAttribute = "data-direct-uploads-processing"; + var submitButtonsByForm = new WeakMap(); var started = false; function start() { if (!started) { started = true; + document.addEventListener("click", didClick, true); document.addEventListener("submit", didSubmitForm); document.addEventListener("ajax:before", didSubmitRemoteElement); } } + function didClick(event) { + var target = event.target; + if (target.tagName == "INPUT" && target.type == "submit" && target.form) { + submitButtonsByForm.set(target.form, target); + } + } function didSubmitForm(event) { handleFormSubmissionEvent(event); } @@ -894,7 +902,7 @@ } } function submitForm(form) { - var button = findElement(form, "input[type=submit]"); + var button = submitButtonsByForm.get(form) || findElement(form, "input[type=submit]"); if (button) { var _button = button, disabled = _button.disabled; button.disabled = false; @@ -909,6 +917,7 @@ button.click(); form.removeChild(button); } + submitButtonsByForm.delete(form); } function disable(input) { input.disabled = true; diff --git a/activestorage/app/controllers/active_storage/base_controller.rb b/activestorage/app/controllers/active_storage/base_controller.rb index 59312ac8df..b27d2bd8aa 100644 --- a/activestorage/app/controllers/active_storage/base_controller.rb +++ b/activestorage/app/controllers/active_storage/base_controller.rb @@ -1,10 +1,8 @@ # frozen_string_literal: true -# The base controller for all ActiveStorage controllers. +# The base class for all Active Storage controllers. class ActiveStorage::BaseController < ActionController::Base - protect_from_forgery with: :exception + include ActiveStorage::SetCurrent - before_action do - ActiveStorage::Current.host = request.base_url - end + protect_from_forgery with: :exception end diff --git a/activestorage/app/controllers/concerns/active_storage/set_current.rb b/activestorage/app/controllers/concerns/active_storage/set_current.rb new file mode 100644 index 0000000000..597afe7064 --- /dev/null +++ b/activestorage/app/controllers/concerns/active_storage/set_current.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# Sets the <tt>ActiveStorage::Current.host</tt> attribute, which the disk service uses to generate URLs. +# Include this concern in custom controllers that call ActiveStorage::Blob#service_url, +# ActiveStorage::Variant#service_url, or ActiveStorage::Preview#service_url so the disk service can +# generate URLs using the same host, protocol, and base path as the current request. +module ActiveStorage::SetCurrent + extend ActiveSupport::Concern + + included do + before_action do + ActiveStorage::Current.host = request.base_url + end + end +end diff --git a/activestorage/app/javascript/activestorage/ujs.js b/activestorage/app/javascript/activestorage/ujs.js index 08c535470d..f5353389ef 100644 --- a/activestorage/app/javascript/activestorage/ujs.js +++ b/activestorage/app/javascript/activestorage/ujs.js @@ -2,16 +2,25 @@ import { DirectUploadsController } from "./direct_uploads_controller" import { findElement } from "./helpers" const processingAttribute = "data-direct-uploads-processing" +const submitButtonsByForm = new WeakMap let started = false export function start() { if (!started) { started = true + document.addEventListener("click", didClick, true) document.addEventListener("submit", didSubmitForm) document.addEventListener("ajax:before", didSubmitRemoteElement) } } +function didClick(event) { + const { target } = event + if (target.tagName == "INPUT" && target.type == "submit" && target.form) { + submitButtonsByForm.set(target.form, target) + } +} + function didSubmitForm(event) { handleFormSubmissionEvent(event) } @@ -49,7 +58,8 @@ function handleFormSubmissionEvent(event) { } function submitForm(form) { - let button = findElement(form, "input[type=submit]") + let button = submitButtonsByForm.get(form) || findElement(form, "input[type=submit]") + if (button) { const { disabled } = button button.disabled = false @@ -64,6 +74,7 @@ function submitForm(form) { button.click() form.removeChild(button) } + submitButtonsByForm.delete(form) } function disable(input) { diff --git a/activestorage/app/jobs/active_storage/analyze_job.rb b/activestorage/app/jobs/active_storage/analyze_job.rb index 2a952f9f74..804ee4557a 100644 --- a/activestorage/app/jobs/active_storage/analyze_job.rb +++ b/activestorage/app/jobs/active_storage/analyze_job.rb @@ -2,6 +2,8 @@ # Provides asynchronous analysis of ActiveStorage::Blob records via ActiveStorage::Blob#analyze_later. class ActiveStorage::AnalyzeJob < ActiveStorage::BaseJob + retry_on ActiveStorage::IntegrityError, attempts: 10, wait: :exponentially_longer + def perform(blob) blob.analyze end diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 056fd2be99..ea57fa5f78 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "ostruct" + # Image blobs can have variants that are the result of a set of transformations applied to the original. # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the # original. @@ -51,7 +53,7 @@ # * {ImageProcessing::Vips}[https://github.com/janko-m/image_processing/blob/master/doc/vips.md#methods] # * {ruby-vips reference}[http://www.rubydoc.info/gems/ruby-vips/Vips/Image] class ActiveStorage::Variant - WEB_IMAGE_CONTENT_TYPES = %w( image/png image/jpeg image/jpg image/gif ) + WEB_IMAGE_CONTENT_TYPES = %w[ image/png image/jpeg image/jpg image/gif ] attr_reader :blob, :variation delegate :service, to: :blob @@ -95,37 +97,35 @@ class ActiveStorage::Variant def process blob.open do |image| - transform image do |output| - upload output - end + transform(image) { |output| upload(output) } end end - - def filename - if WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) - blob.filename - else - ActiveStorage::Filename.new("#{blob.filename.base}.png") - end + def transform(image, &block) + variation.transform(image, format: format, &block) end - def content_type - blob.content_type.presence_in(WEB_IMAGE_CONTENT_TYPES) || "image/png" + def upload(file) + service.upload(key, file) end - def transform(image) - format = "png" unless WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) - result = variation.transform(image, format: format) - begin - yield result - ensure - result.close! - end + def specification + @specification ||= + if WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) + Specification.new \ + filename: blob.filename, + content_type: blob.content_type, + format: nil + else + Specification.new \ + filename: ActiveStorage::Filename.new("#{blob.filename.base}.png"), + content_type: "image/png", + format: "png" + end end - def upload(file) - service.upload(key, file) - end + delegate :filename, :content_type, :format, to: :specification + + class Specification < OpenStruct; end end diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index ae1376a6cf..3adc2407e5 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -47,13 +47,9 @@ class ActiveStorage::Variation # saves the transformed image into a temporary file. If +format+ is specified # it will be the format of the result image, otherwise the result image # retains the source format. - def transform(file, format: nil) + def transform(file, format: nil, &block) ActiveSupport::Notifications.instrument("transform.active_storage") do - if processor - image_processing_transform(file, format) - else - mini_magick_transform(file, format) - end + transformer.transform(file, format: format, &block) end end @@ -63,63 +59,22 @@ class ActiveStorage::Variation end private - # Applies image transformations using the ImageProcessing gem. - def image_processing_transform(file, format) - operations = transformations.each_with_object([]) do |(name, argument), list| - if name.to_s == "combine_options" - ActiveSupport::Deprecation.warn("The ImageProcessing ActiveStorage variant backend doesn't need :combine_options, as it already generates a single MiniMagick command. In Rails 6.1 :combine_options will not be supported anymore.") - list.concat argument.keep_if { |key, value| value.present? }.to_a - elsif argument.present? - list << [name, argument] + def transformer + if ActiveStorage.variant_processor + begin + require "image_processing" + rescue LoadError + ActiveSupport::Deprecation.warn <<~WARNING + Generating image variants will require the image_processing gem in Rails 6.1. + Please add `gem 'image_processing', '~> 1.2'` to your Gemfile. + WARNING + + ActiveStorage::Transformers::MiniMagickTransformer.new(transformations) + else + ActiveStorage::Transformers::ImageProcessingTransformer.new(transformations) end - end - - processor - .source(file) - .loader(page: 0) - .convert(format) - .apply(operations) - .call - end - - # Applies image transformations using the MiniMagick gem. - def mini_magick_transform(file, format) - image = MiniMagick::Image.new(file.path, file) - - transformations.each do |name, argument_or_subtransformations| - image.mogrify do |command| - if name.to_s == "combine_options" - argument_or_subtransformations.each do |subtransformation_name, subtransformation_argument| - pass_transform_argument(command, subtransformation_name, subtransformation_argument) - end - else - pass_transform_argument(command, name, argument_or_subtransformations) - end - end - end - - image.format(format) if format - - image.tempfile.tap(&:open) - end - - # Returns the ImageProcessing processor class specified by `ActiveStorage.variant_processor`. - def processor - begin - require "image_processing" - rescue LoadError - ActiveSupport::Deprecation.warn("Using mini_magick gem directly is deprecated and will be removed in Rails 6.1. Please add `gem 'image_processing', '~> 1.2'` to your Gemfile.") - return nil - end - - ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize) if ActiveStorage.variant_processor - end - - def pass_transform_argument(command, method, argument) - if argument == true - command.public_send(method) - elsif argument.present? - command.public_send(method, argument) + else + ActiveStorage::Transformers::MiniMagickTransformer.new(transformations) end end end diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 928db32691..d3e3a2f49b 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -50,4 +50,12 @@ module ActiveStorage mattr_accessor :variable_content_types, default: [] mattr_accessor :content_types_to_serve_as_binary, default: [] mattr_accessor :service_urls_expire_in, default: 5.minutes + + module Transformers + extend ActiveSupport::Autoload + + autoload :Transformer + autoload :ImageProcessingTransformer + autoload :MiniMagickTransformer + end end diff --git a/activestorage/lib/active_storage/errors.rb b/activestorage/lib/active_storage/errors.rb index bedcd080c4..f4bf66a615 100644 --- a/activestorage/lib/active_storage/errors.rb +++ b/activestorage/lib/active_storage/errors.rb @@ -1,11 +1,22 @@ # frozen_string_literal: true module ActiveStorage - class InvariableError < StandardError; end - class UnpreviewableError < StandardError; end - class UnrepresentableError < StandardError; end + # Generic base class for all Active Storage exceptions. + class Error < StandardError; end + + # Raised when ActiveStorage::Blob#variant is called on a blob that isn't variable. + # Use ActiveStorage::Blob#variable? to determine whether a blob is variable. + class InvariableError < Error; end + + # Raised when ActiveStorage::Blob#preview is called on a blob that isn't previewable. + # Use ActiveStorage::Blob#previewable? to determine whether a blob is previewable. + class UnpreviewableError < Error; end + + # Raised when ActiveStorage::Blob#representation is called on a blob that isn't representable. + # Use ActiveStorage::Blob#representable? to determine whether a blob is representable. + class UnrepresentableError < Error; end # Raised when uploaded or downloaded data does not match a precomputed checksum. # Indicates that a network error or a software bug caused data corruption. - class IntegrityError < StandardError; end + class IntegrityError < Error; end end diff --git a/activestorage/lib/active_storage/transformers/image_processing_transformer.rb b/activestorage/lib/active_storage/transformers/image_processing_transformer.rb new file mode 100644 index 0000000000..7f8685b72d --- /dev/null +++ b/activestorage/lib/active_storage/transformers/image_processing_transformer.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require "image_processing" + +module ActiveStorage + module Transformers + class ImageProcessingTransformer < Transformer + private + def process(file, format:) + processor. + source(file). + loader(page: 0). + convert(format). + apply(operations). + call + end + + def processor + ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize) + end + + def operations + transformations.each_with_object([]) do |(name, argument), list| + if name.to_s == "combine_options" + ActiveSupport::Deprecation.warn <<~WARNING + Active Storage's ImageProcessing transformer doesn't support :combine_options, + as it always generates a single ImageMagick command. Passing :combine_options will + not be supported in Rails 6.1. + WARNING + + list.concat argument.keep_if { |key, value| value.present? }.to_a + elsif argument.present? + list << [ name, argument ] + end + end + end + end + end +end diff --git a/activestorage/lib/active_storage/transformers/mini_magick_transformer.rb b/activestorage/lib/active_storage/transformers/mini_magick_transformer.rb new file mode 100644 index 0000000000..e8e99cea9e --- /dev/null +++ b/activestorage/lib/active_storage/transformers/mini_magick_transformer.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "mini_magick" + +module ActiveStorage + module Transformers + class MiniMagickTransformer < Transformer + private + def process(file, format:) + image = MiniMagick::Image.new(file.path, file) + + transformations.each do |name, argument_or_subtransformations| + image.mogrify do |command| + if name.to_s == "combine_options" + argument_or_subtransformations.each do |subtransformation_name, subtransformation_argument| + pass_transform_argument(command, subtransformation_name, subtransformation_argument) + end + else + pass_transform_argument(command, name, argument_or_subtransformations) + end + end + end + + image.format(format) if format + + image.tempfile.tap(&:open) + end + + def pass_transform_argument(command, method, argument) + if argument == true + command.public_send(method) + elsif argument.present? + command.public_send(method, argument) + end + end + end + end +end diff --git a/activestorage/lib/active_storage/transformers/transformer.rb b/activestorage/lib/active_storage/transformers/transformer.rb new file mode 100644 index 0000000000..2e21201004 --- /dev/null +++ b/activestorage/lib/active_storage/transformers/transformer.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module ActiveStorage + module Transformers + # A Transformer applies a set of transformations to an image. + # + # The following concrete subclasses are included in Active Storage: + # + # * ActiveStorage::Transformers::ImageProcessingTransformer: + # backed by ImageProcessing, a common interface for MiniMagick and ruby-vips + # + # * ActiveStorage::Transformers::MiniMagickTransformer: + # backed by MiniMagick, a wrapper around the ImageMagick CLI + class Transformer + attr_reader :transformations + + def initialize(transformations) + @transformations = transformations + end + + # Applies the transformations to the source image in +file+, producing a target image in the + # specified +format+. Yields an open Tempfile containing the target image. Closes and unlinks + # the output tempfile after yielding to the given block. Returns the result of the block. + def transform(file, format:) + output = process(file, format: format) + + begin + yield output + ensure + output.close! + end + end + + private + # Returns an open Tempfile containing a transformed image in the given +format+. + # All subclasses implement this method. + def process(file, format:) #:doc: + raise NotImplementedError + end + end + end +end diff --git a/activestorage/test/jobs/purge_job_test.rb b/activestorage/test/jobs/purge_job_test.rb index ed4100b78d..251022a96f 100644 --- a/activestorage/test/jobs/purge_job_test.rb +++ b/activestorage/test/jobs/purge_job_test.rb @@ -24,14 +24,4 @@ class ActiveStorage::PurgeJobTest < ActiveJob::TestCase end end end - - test "ignores attached blob" do - User.create! name: "DHH", avatar: @blob - - perform_enqueued_jobs do - assert_nothing_raised do - ActiveStorage::PurgeJob.perform_later @blob - end - end - end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 5423d59984..4ae02edd6a 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,14 @@ +* Add `Array#extract!`. + + The method removes and returns the elements for which the block returns a true value. + If no block is given, an Enumerator is returned instead. + + numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + odd_numbers = numbers.extract! { |number| number.odd? } # => [1, 3, 5, 7, 9] + numbers # => [0, 2, 4, 6, 8] + + *bogdanvlviv* + * Support not to cache `nil` for `ActiveSupport::Cache#fetch`. cache.fetch('bar', skip_nil: true) { nil } @@ -96,7 +107,7 @@ *Godfrey Chan* -* Fix bug where `URI.unscape` would fail with mixed Unicode/escaped character input: +* Fix bug where `URI.unescape` would fail with mixed Unicode/escaped character input: URI.unescape("\xe3\x83\x90") # => "バ" URI.unescape("%E3%83%90") # => "バ" diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb index 16dd733ddb..1796956bd7 100644 --- a/activesupport/lib/active_support/backtrace_cleaner.rb +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -31,6 +31,9 @@ module ActiveSupport class BacktraceCleaner def initialize @filters, @silencers = [], [] + add_gem_filter + add_gem_silencer + add_stdlib_silencer end # Returns the backtrace after all filters and silencers have been run @@ -82,6 +85,26 @@ module ActiveSupport end private + + FORMATTED_GEMS_PATTERN = /\A[^\/]+ \([\w.]+\) / + + def add_gem_filter + gems_paths = (Gem.path | [Gem.default_dir]).map { |p| Regexp.escape(p) } + return if gems_paths.empty? + + gems_regexp = %r{(#{gems_paths.join('|')})/(bundler/)?gems/([^/]+)-([\w.]+)/(.*)} + gems_result = '\3 (\4) \5'.freeze + add_filter { |line| line.sub(gems_regexp, gems_result) } + end + + def add_gem_silencer + add_silencer { |line| FORMATTED_GEMS_PATTERN.match?(line) } + end + + def add_stdlib_silencer + add_silencer { |line| line.start_with?(RbConfig::CONFIG["rubylibdir"]) } + end + def filter_backtrace(backtrace) @filters.each do |f| backtrace = backtrace.map { |line| f.call(line) } diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index c266b432c0..487fe79f41 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -657,9 +657,17 @@ module ActiveSupport # * <tt>:if</tt> - A symbol or an array of symbols, each naming an instance # method or a proc; the callback will be called only when they all return # a true value. + # + # If a proc is given, its body is evaluated in the context of the + # current object. It can also optionally accept the current object as + # an argument. # * <tt>:unless</tt> - A symbol or an array of symbols, each naming an # instance method or a proc; the callback will be called only when they # all return a false value. + # + # If a proc is given, its body is evaluated in the context of the + # current object. It can also optionally accept the current object as + # an argument. # * <tt>:prepend</tt> - If +true+, the callback will be prepended to the # existing chain rather than appended. def set_callback(name, *filter_list, &block) diff --git a/activesupport/lib/active_support/core_ext/array.rb b/activesupport/lib/active_support/core_ext/array.rb index 6d83b76882..a2569c798b 100644 --- a/activesupport/lib/active_support/core_ext/array.rb +++ b/activesupport/lib/active_support/core_ext/array.rb @@ -3,6 +3,7 @@ require "active_support/core_ext/array/wrap" require "active_support/core_ext/array/access" require "active_support/core_ext/array/conversions" +require "active_support/core_ext/array/extract" require "active_support/core_ext/array/extract_options" require "active_support/core_ext/array/grouping" require "active_support/core_ext/array/prepend_and_append" diff --git a/activesupport/lib/active_support/core_ext/array/extract.rb b/activesupport/lib/active_support/core_ext/array/extract.rb new file mode 100644 index 0000000000..cc5a8a3f88 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/array/extract.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class Array + # Removes and returns the elements for which the block returns a true value. + # If no block is given, an Enumerator is returned instead. + # + # numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + # odd_numbers = numbers.extract! { |number| number.odd? } # => [1, 3, 5, 7, 9] + # numbers # => [0, 2, 4, 6, 8] + def extract! + return to_enum(:extract!) { size } unless block_given? + + extracted_elements = [] + + reject! do |element| + extracted_elements << element if yield(element) + end + + extracted_elements + end +end diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 8fd6e932f1..c75ad52b0c 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -85,6 +85,9 @@ module ActiveSupport # number given by <tt>:format</tt>). Accepts the same fields # than <tt>:format</tt>, except <tt>%n</tt> is here the # absolute value of the number. + # * <tt>:strip_insignificant_zeros</tt> - If +true+ removes + # insignificant zeros after the decimal separator (defaults to + # +false+). # # ==== Examples # @@ -100,6 +103,8 @@ module ActiveSupport # # => "£1234567890,50" # number_to_currency(1234567890.50, unit: '£', separator: ',', delimiter: '', format: '%n %u') # # => "1234567890,50 £" + # number_to_currency(1234567890.50, strip_insignificant_zeros: true) + # # => "$1,234,567,890.5" def number_to_currency(number, options = {}) NumberToCurrencyConverter.convert(number, options) end diff --git a/activesupport/test/cache/behaviors/cache_store_behavior.rb b/activesupport/test/cache/behaviors/cache_store_behavior.rb index ae272e87d7..e17c09ba39 100644 --- a/activesupport/test/cache/behaviors/cache_store_behavior.rb +++ b/activesupport/test/cache/behaviors/cache_store_behavior.rb @@ -148,7 +148,7 @@ module CacheStoreBehavior end end - # Use strings that are guarenteed to compress well, so we can easily tell if + # Use strings that are guaranteed to compress well, so we can easily tell if # the compression kicked in or not. SMALL_STRING = "0" * 100 LARGE_STRING = "0" * 2.kilobytes diff --git a/activesupport/test/clean_backtrace_test.rb b/activesupport/test/clean_backtrace_test.rb index 1b44c7c9bf..a0a7056952 100644 --- a/activesupport/test/clean_backtrace_test.rb +++ b/activesupport/test/clean_backtrace_test.rb @@ -74,3 +74,43 @@ class BacktraceCleanerFilterAndSilencerTest < ActiveSupport::TestCase assert_equal [ "/class.rb" ], @bc.clean([ "/mongrel/class.rb" ]) end end + +class BacktraceCleanerDefaultFilterAndSilencerTest < ActiveSupport::TestCase + def setup + @bc = ActiveSupport::BacktraceCleaner.new + end + + test "should format installed gems correctly" do + backtrace = [ "#{Gem.default_dir}/gems/nosuchgem-1.2.3/lib/foo.rb" ] + result = @bc.clean(backtrace, :all) + assert_equal "nosuchgem (1.2.3) lib/foo.rb", result[0] + end + + test "should format installed gems not in Gem.default_dir correctly" do + target_dir = Gem.path.detect { |p| p != Gem.default_dir } + # skip this test if default_dir is the only directory on Gem.path + if target_dir + backtrace = [ "#{target_dir}/gems/nosuchgem-1.2.3/lib/foo.rb" ] + result = @bc.clean(backtrace, :all) + assert_equal "nosuchgem (1.2.3) lib/foo.rb", result[0] + end + end + + test "should format gems installed by bundler" do + backtrace = [ "#{Gem.default_dir}/bundler/gems/nosuchgem-1.2.3/lib/foo.rb" ] + result = @bc.clean(backtrace, :all) + assert_equal "nosuchgem (1.2.3) lib/foo.rb", result[0] + end + + test "should silence gems from the backtrace" do + backtrace = [ "#{Gem.path[0]}/gems/nosuchgem-1.2.3/lib/foo.rb" ] + result = @bc.clean(backtrace) + assert_empty result + end + + test "should silence stdlib" do + backtrace = ["#{RbConfig::CONFIG["rubylibdir"]}/lib/foo.rb"] + result = @bc.clean(backtrace) + assert_empty result + end +end diff --git a/activesupport/test/core_ext/array/extract_test.rb b/activesupport/test/core_ext/array/extract_test.rb new file mode 100644 index 0000000000..200727667c --- /dev/null +++ b/activesupport/test/core_ext/array/extract_test.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "active_support/core_ext/array" + +class ExtractTest < ActiveSupport::TestCase + def test_extract + numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + array_id = numbers.object_id + + odd_numbers = numbers.extract!(&:odd?) + + assert_equal [1, 3, 5, 7, 9], odd_numbers + assert_equal [0, 2, 4, 6, 8], numbers + assert_equal array_id, numbers.object_id + end + + def test_extract_without_block + numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + array_id = numbers.object_id + + extract_enumerator = numbers.extract! + + assert_instance_of Enumerator, extract_enumerator + assert_equal numbers.size, extract_enumerator.size + + odd_numbers = extract_enumerator.each(&:odd?) + + assert_equal [1, 3, 5, 7, 9], odd_numbers + assert_equal [0, 2, 4, 6, 8], numbers + assert_equal array_id, numbers.object_id + end + + def test_extract_on_empty_array + empty_array = [] + array_id = empty_array.object_id + + new_empty_array = empty_array.extract! {} + + assert_equal [], new_empty_array + assert_equal [], empty_array + assert_equal array_id, empty_array.object_id + end +end diff --git a/activesupport/test/core_ext/object/to_query_test.rb b/activesupport/test/core_ext/object/to_query_test.rb index b0b7ef0913..561dadbbcf 100644 --- a/activesupport/test/core_ext/object/to_query_test.rb +++ b/activesupport/test/core_ext/object/to_query_test.rb @@ -88,7 +88,7 @@ class ToQueryTest < ActiveSupport::TestCase } expected = "foo[contents][][name]=gorby&foo[contents][][id]=123&foo[contents][][name]=puff&foo[contents][][d]=true" - assert_equal expected, URI.decode(params.to_query) + assert_equal expected, URI.decode_www_form_component(params.to_query) end private diff --git a/activesupport/test/key_generator_test.rb b/activesupport/test/key_generator_test.rb index cdde2c573a..9dfc0b2154 100644 --- a/activesupport/test/key_generator_test.rb +++ b/activesupport/test/key_generator_test.rb @@ -9,9 +9,6 @@ rescue LoadError, NameError $stderr.puts "Skipping KeyGenerator test: broken OpenSSL install" else - require "active_support/time" - require "active_support/json" - class KeyGeneratorTest < ActiveSupport::TestCase def setup @secret = SecureRandom.hex(64) diff --git a/activesupport/test/testing/method_call_assertions_test.rb b/activesupport/test/testing/method_call_assertions_test.rb index 0500e47def..5cdeb683e3 100644 --- a/activesupport/test/testing/method_call_assertions_test.rb +++ b/activesupport/test/testing/method_call_assertions_test.rb @@ -36,6 +36,8 @@ class MethodCallAssertionsTest < ActiveSupport::TestCase assert_called(@object, :increment, returns: 10) do assert_equal 10, @object.increment end + + assert_equal 1, @object.increment end def test_assert_called_failure @@ -70,6 +72,14 @@ class MethodCallAssertionsTest < ActiveSupport::TestCase end end + def test_assert_called_with_arguments_and_returns + assert_called_with(@object, :<<, [ 2 ], returns: 10) do + assert_equal(10, @object << 2) + end + + assert_nil(@object << 2) + end + def test_assert_called_with_failure assert_raises(MockExpectationError) do assert_called_with(@object, :<<, [ 4567 ]) do diff --git a/guides/assets/stylesheets/main.css b/guides/assets/stylesheets/main.css index cd355b1d1a..2657a84a91 100644 --- a/guides/assets/stylesheets/main.css +++ b/guides/assets/stylesheets/main.css @@ -283,8 +283,12 @@ body { #header .wrapper, #topNav .wrapper, #feature .wrapper {padding-left: 1em; max-width: 960px;} #feature .wrapper {max-width: 640px; padding-right: 23em; position: relative; z-index: 0;} +@media screen and (max-width: 960px) { + #container .wrapper { padding-right: 23em; } +} + @media screen and (max-width: 800px) { - #feature .wrapper { padding-right: 0; } + #feature .wrapper, #container .wrapper { padding-right: 0; } } /* Links diff --git a/guides/rails_guides/levenshtein.rb b/guides/rails_guides/levenshtein.rb index c48af797fa..2213ef754d 100644 --- a/guides/rails_guides/levenshtein.rb +++ b/guides/rails_guides/levenshtein.rb @@ -12,8 +12,8 @@ module RailsGuides n = s.length m = t.length - return m if (0 == n) - return n if (0 == m) + return m if 0 == n + return n if 0 == m d = (0..m).to_a x = nil diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index 6c5f03ab38..37cbf3f53d 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -422,6 +422,21 @@ use the rendered text for the text part. The render command is the same one used inside of Action Controller, so you can use all the same options, such as `:text`, `:inline` etc. +If you would like to render a template located outside of the default `app/views/mailer_name/` directory, you can apply the `prepend_view_path`, like so: + +```ruby +class UserMailer < ApplicationMailer + prepend_view_path "custom/path/to/mailer/view" + + # This will try to load "custom/path/to/mailer/view/welcome_email" template + def welcome_email + # ... + end +end +``` + +You can also consider using the [append_view_path](https://guides.rubyonrails.org/action_view_overview.html#view-paths) method. + #### Caching mailer view You can perform fragment caching in mailer views like in application views using the `cache` method. diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index d90ea2e26a..fad4c19827 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -115,7 +115,7 @@ to Active Record instances: * `created_at` - Automatically gets set to the current date and time when the record is first created. * `updated_at` - Automatically gets set to the current date and time whenever - the record is updated. + the record is created or updated. * `lock_version` - Adds [optimistic locking](http://api.rubyonrails.org/classes/ActiveRecord/Locking.html) to a model. diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index 5b06ff78bb..5946acb412 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -319,6 +319,14 @@ class Order < ApplicationRecord end ``` +As the proc is evaluated in the context of the object, it is also possible to write this as: + +```ruby +class Order < ApplicationRecord + before_save :normalize_card_number, if: Proc.new { paid_with_card? } +end +``` + ### Multiple Conditions for Callbacks When writing conditional callbacks, it is possible to mix both `:if` and `:unless` in the same callback declaration: diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index cfa444fda0..4d195988f8 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -491,6 +491,9 @@ NOTE: Active Record only supports single column foreign keys. `execute` and `structure.sql` are required to use composite foreign keys. See [Schema Dumping and You](#schema-dumping-and-you). +NOTE: The SQLite3 adapter doesn't support `add_foreign_key` since SQLite supports +only [a limited subset of ALTER TABLE](https://www.sqlite.org/lang_altertable.html). + Removing a foreign key is easy as well: ```ruby @@ -923,9 +926,10 @@ your database schema. It tends to be faster and less error prone to create a new instance of your application's database by loading the schema file via `rails db:schema:load` -than it is to replay the entire migration history. Old migrations may fail to -apply correctly if those migrations use changing external dependencies or rely -on application code which evolves separately from your migrations. +than it is to replay the entire migration history. +[Old migrations](#old-migrations) may fail to apply correctly if those +migrations use changing external dependencies or rely on application code which +evolves separately from your migrations. Schema files are also useful if you want a quick look at what attributes an Active Record object has. This information is not in the model's code and is @@ -1042,3 +1046,21 @@ end This is generally a much cleaner way to set up the database of a blank application. + +Old Migrations +-------------- + +The `db/schema.rb` or `db/structure.sql` is a snapshot of the current state of your +database and is the authoritative source for rebuilding that database. This +makes it possible to delete old migration files. + +When you delete migration files in the `db/migrate/` directory, any environment +where `rails db:migrate` was run when those files still existed will hold a reference +to the migration timestamp specific to them inside an internal Rails database +table named `schema_migrations`. This table is used to keep track of whether +migrations have been executed in a specific environment. + +If you run the `rails db:migrate:status` command, which displays the status +(up or down) of each migration, you should see `********** NO FILE **********` +displayed next to any deleted migration file which was once executed on a +specific environment but can no longer be found in the `db/migrate/` directory. diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index a2890b9b7a..91cc175095 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -1277,16 +1277,6 @@ class Article < ApplicationRecord end ``` -This is exactly the same as defining a class method, and which you use is a matter of personal preference: - -```ruby -class Article < ApplicationRecord - def self.published - where(published: true) - end -end -``` - Scopes are also chainable within scopes: ```ruby diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index afe837a97c..3f13ef8d10 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -927,6 +927,13 @@ class Account < ApplicationRecord end ``` +As `Lambdas` are a type of `Proc`, they can also be used to write inline +conditions in a shorter way. + +```ruby +validates :password, confirmation: true, unless: -> { password.blank? } +``` + ### Grouping Conditional validations Sometimes it is useful to have multiple validations use one condition. It can diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index 6933717c2b..1c15d075b9 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -174,7 +174,7 @@ google: Add the [`google-cloud-storage`](https://github.com/GoogleCloudPlatform/google-cloud-ruby/tree/master/google-cloud-storage) gem to your `Gemfile`: ```ruby -gem "google-cloud-storage", "~> 1.8", require: false +gem "google-cloud-storage", "~> 1.11", require: false ``` ### Mirror Service diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index dfd21915b0..f9fc7044ba 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -2156,6 +2156,19 @@ This method is an alias of `Array#<<`. NOTE: Defined in `active_support/core_ext/array/prepend_and_append.rb`. +### Extracting + +The method `extract!` removes and returns the elements for which the block returns a true value. +If no block is given, an Enumerator is returned instead. + +```ruby +numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] +odd_numbers = numbers.extract! { |number| number.odd? } # => [1, 3, 5, 7, 9] +numbers # => [0, 2, 4, 6, 8] +``` + +NOTE: Defined in `active_support/core_ext/array/extract.rb`. + ### Options Extraction When the last argument in a method call is a hash, except perhaps for a `&block` argument, Ruby allows you to omit the brackets: diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 3147b00f3b..b5e40aa40f 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -239,7 +239,6 @@ Now get busy and add/edit code. You're on your branch now, so you can write what * Include tests that fail without your code, and pass with it. * Update the (surrounding) documentation, examples elsewhere, and the guides: whatever is affected by your contribution. - TIP: Changes that are cosmetic in nature and do not add anything substantial to the stability, functionality, or testability of Rails will generally not be accepted (read more about [our rationales behind this decision](https://github.com/rails/rails/pull/13771#issuecomment-32746700)). #### Follow the Coding Conventions @@ -254,12 +253,24 @@ Rails follows a simple set of coding style conventions: * Prefer class << self over self.method for class methods. * Use `my_method(my_arg)` not `my_method( my_arg )` or `my_method my_arg`. * Use `a = b` and not `a=b`. -* Use assert_not methods instead of refute. +* Use assert\_not methods instead of refute. * Prefer `method { do_stuff }` instead of `method{do_stuff}` for single-line blocks. * Follow the conventions in the source you see used already. The above are guidelines - please use your best judgment in using them. +Additionally, we have [RuboCop](https://www.rubocop.org/) rules defined to codify some of our coding conventions. You can run RuboCop locally against the file that you have modified before submitting a pull request: + +```bash +$ rubocop actionpack/lib/action_controller/metal/strong_parameters.rb +Inspecting 1 file +. + +1 file inspected, no offenses detected +``` + +For `rails-ujs` CoffeeScript and JavaScript files, you can run `npm run lint` in `actionview` folder. + ### Benchmark Your Code For changes that might have an impact on performance, please benchmark your diff --git a/guides/source/development_dependencies_install.md b/guides/source/development_dependencies_install.md index 057bcf2c1b..07538a1cb7 100644 --- a/guides/source/development_dependencies_install.md +++ b/guides/source/development_dependencies_install.md @@ -350,57 +350,78 @@ prerequisite for installing this package manager is that On macOS, you can run: ```bash -brew install yarn +$ brew install yarn ``` On Ubuntu, you can run: ```bash -curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add - -echo "deb https://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list +$ curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add - +$ echo "deb https://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list -sudo apt-get update && sudo apt-get install yarn +$ sudo apt-get update && sudo apt-get install yarn ``` On Fedora or CentOS, just run: ```bash -sudo wget https://dl.yarnpkg.com/rpm/yarn.repo -O /etc/yum.repos.d/yarn.repo +$ sudo wget https://dl.yarnpkg.com/rpm/yarn.repo -O /etc/yum.repos.d/yarn.repo -sudo yum install yarn +$ sudo yum install yarn ``` Finally, after installing Yarn, you will need to run the following command inside of the `activestorage` directory to install the dependencies: ```bash -yarn install +$ yarn install ``` -Extracting previews, tested in ActiveStorage's test suite requires third-party -applications, FFmpeg for video and muPDF for PDFs, and on macOS also XQuartz -and Poppler. Without these applications installed, ActiveStorage tests will +Extracting previews, tested in Active Storage's test suite requires third-party +applications, ImageMagick for images, FFmpeg for video and muPDF for PDFs, and on macOS also XQuartz +and Poppler. Without these applications installed, Active Storage tests will raise errors. On macOS you can run: ```bash -brew install ffmpeg -brew cask install xquartz -brew install mupdf-tools -brew install poppler +$ brew install ffmpeg +$ brew install imagemagick +$ brew cask install xquartz +$ brew install mupdf-tools +$ brew install poppler ``` On Ubuntu, you can run: ```bash -sudo apt-get update && install ffmpeg -sudo apt-get update && install mupdf mupdf-tools +$ sudo apt-get update +$ sudo apt-get install ffmpeg +$ sudo apt-get install imagemagick +$ sudo apt-get install mupdf mupdf-tools ``` On Fedora or CentOS, just run: ```bash -sudo yum install ffmpeg -sudo yum install mupdf +$ sudo yum install ffmpeg +$ sudo yum install imagemagick +$ sudo yum install mupdf +``` + +FreeBSD users can just run: + +```bash +# pkg install imagemagick +# pkg install ffmpeg +# pkg install mupdf +``` + +On Arch Linux, you can run: + +```bash +$ sudo pacman -S ffmpeg +$ sudo pacman -S imagemagick +$ sudo pacman -S mupdf mupdf-tools +$ sudo pacman -S poppler ``` diff --git a/guides/source/testing.md b/guides/source/testing.md index 01cda8e6e4..de93e1c653 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -1113,11 +1113,10 @@ end Now you can try running all the tests and they should pass. -NOTE: If you followed the steps in the Basic Authentication section, you'll need to add the following to the `setup` block to get all the tests passing: +NOTE: If you followed the steps in the Basic Authentication section, you'll need to add authorization to every request header to get all the tests passing: ```ruby -request.headers['Authorization'] = ActionController::HttpAuthentication::Basic. - encode_credentials('dhh', 'secret') +post articles_url, params: { article: { body: 'Rails is awesome!', title: 'Hello Rails' } }, headers: { Authorization: ActionController::HttpAuthentication::Basic.encode_credentials('dhh', 'secret') } ``` ### Available Request Types for Functional Tests diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 81cb5a6c9f..2a8882171f 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,10 +1,22 @@ +* Deprecate `rake routes` in favor of `rails routes`. + + *Yuji Yaginuma* + +* Deprecate `rake initializers` in favor of `rails initializers`. + + *Annie-Claude Côté* + +* Deprecate `rake dev:cache` in favor of `rails dev:cache`. + + *Annie-Claude Côté* + * Deprecate `rails notes` subcommands in favor of passing an `annotations` argument to `rails notes`. The following subcommands are replaced by passing `--annotations` or `-a` to `rails notes`: - - `rails notes:custom ANNOTATION=custom` is deprecated in favor of using `rails notes -a custom`. - - `rails notes:optimize` is deprecated in favor of using `rails notes -a OPTIMIZE`. - - `rails notes:todo` is deprecated in favor of using`rails notes -a TODO`. - - `rails notes:fixme` is deprecated in favor of using `rails notes -a FIXME`. + - `rails notes:custom ANNOTATION=custom` is deprecated in favor of using `rails notes -a custom`. + - `rails notes:optimize` is deprecated in favor of using `rails notes -a OPTIMIZE`. + - `rails notes:todo` is deprecated in favor of using`rails notes -a TODO`. + - `rails notes:fixme` is deprecated in favor of using `rails notes -a FIXME`. *Annie-Claude Côté* diff --git a/railties/lib/rails/api/generator.rb b/railties/lib/rails/api/generator.rb index 3405560b74..126d4d0438 100644 --- a/railties/lib/rails/api/generator.rb +++ b/railties/lib/rails/api/generator.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "sdoc" +require "active_support/core_ext/array/extract" class RDoc::Generator::API < RDoc::Generator::SDoc # :nodoc: RDoc::RDoc.add_generator self @@ -11,7 +12,7 @@ class RDoc::Generator::API < RDoc::Generator::SDoc # :nodoc: # since they aren't nested under a definition of the `ActiveStorage` module. if visited.empty? classes = classes.reject { |klass| active_storage?(klass) } - core_exts, classes = classes.partition { |klass| core_extension?(klass) } + core_exts = classes.extract! { |klass| core_extension?(klass) } super.unshift([ "Core extensions", "", "", build_core_ext_subtree(core_exts, visited) ]) else diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 31d0d65a30..99e42ebefb 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -267,6 +267,7 @@ module Rails "action_dispatch.cookies_serializer" => config.action_dispatch.cookies_serializer, "action_dispatch.cookies_digest" => config.action_dispatch.cookies_digest, "action_dispatch.cookies_rotations" => config.action_dispatch.cookies_rotations, + "action_dispatch.use_cookies_with_metadata" => config.action_dispatch.use_cookies_with_metadata, "action_dispatch.content_security_policy" => config.content_security_policy, "action_dispatch.content_security_policy_report_only" => config.content_security_policy_report_only, "action_dispatch.content_security_policy_nonce_generator" => config.content_security_policy_nonce_generator diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 9c54cc1f37..9eb07219e0 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -120,6 +120,10 @@ module Rails if respond_to?(:action_view) action_view.default_enforce_utf8 = false end + + if respond_to?(:action_dispatch) + action_dispatch.use_cookies_with_metadata = true + end else raise "Unknown version #{target_version.to_s.inspect}" end diff --git a/railties/lib/rails/backtrace_cleaner.rb b/railties/lib/rails/backtrace_cleaner.rb index 0e78959966..b1e3c923b7 100644 --- a/railties/lib/rails/backtrace_cleaner.rb +++ b/railties/lib/rails/backtrace_cleaner.rb @@ -16,19 +16,7 @@ module Rails add_filter { |line| line.sub(@root, EMPTY_STRING) } add_filter { |line| line.sub(RENDER_TEMPLATE_PATTERN, EMPTY_STRING) } add_filter { |line| line.sub(DOT_SLASH, SLASH) } # for tests - - add_gem_filters add_silencer { |line| !APP_DIRS_PATTERN.match?(line) } end - - private - def add_gem_filters - gems_paths = (Gem.path | [Gem.default_dir]).map { |p| Regexp.escape(p) } - return if gems_paths.empty? - - gems_regexp = %r{(#{gems_paths.join('|')})/gems/([^/]+)-([\w.]+)/(.*)} - gems_result = '\2 (\3) \4'.freeze - add_filter { |line| line.sub(gems_regexp, gems_result) } - end end end diff --git a/railties/lib/rails/command/spellchecker.rb b/railties/lib/rails/command/spellchecker.rb index 04485097fa..085d5b16df 100644 --- a/railties/lib/rails/command/spellchecker.rb +++ b/railties/lib/rails/command/spellchecker.rb @@ -24,8 +24,8 @@ module Rails n = s.length m = t.length - return m if (0 == n) - return n if (0 == m) + return m if 0 == n + return n if 0 == m d = (0..m).to_a x = nil diff --git a/railties/lib/rails/commands/dev/dev_command.rb b/railties/lib/rails/commands/dev/dev_command.rb new file mode 100644 index 0000000000..820dc4db9e --- /dev/null +++ b/railties/lib/rails/commands/dev/dev_command.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require "rails/dev_caching" + +module Rails + module Command + class DevCommand < Base # :nodoc: + desc "Toggle development mode caching on/off" + def cache + Rails::DevCaching.enable_by_file + end + end + end +end diff --git a/railties/lib/rails/commands/initializers/initializers_command.rb b/railties/lib/rails/commands/initializers/initializers_command.rb new file mode 100644 index 0000000000..559546acea --- /dev/null +++ b/railties/lib/rails/commands/initializers/initializers_command.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Rails + module Command + class InitializersCommand < Base # :nodoc: + desc "Print out all defined initializers in the order they are invoked by Rails." + def perform + require_application_and_environment! + + Rails.application.initializers.tsort_each do |initializer| + say "#{initializer.context_class}.#{initializer.name}" + end + end + end + end +end diff --git a/railties/lib/rails/commands/server/server_command.rb b/railties/lib/rails/commands/server/server_command.rb index 2c5440d9ec..9d517f3239 100644 --- a/railties/lib/rails/commands/server/server_command.rb +++ b/railties/lib/rails/commands/server/server_command.rb @@ -286,7 +286,7 @@ module Rails original_options.concat [ "-u", using ] else # Use positional internally to get around Thor's immutable options. - # TODO: Replace `using` occurences with `options[:using]` after deprecation removal. + # TODO: Replace `using` occurrences with `options[:using]` after deprecation removal. @using = options[:using] end end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 985e9ab263..a6dc60342c 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -458,6 +458,12 @@ module Rails end end + def generate_bundler_binstub + if bundle_install? + bundle_command("binstubs bundler") + end + end + def generate_spring_binstubs if bundle_install? && spring_install? bundle_command("exec spring binstub --all") diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 83c5c9f297..a6d160f1eb 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -469,7 +469,8 @@ module Rails end public_task :apply_rails_template, :run_bundle - public_task :run_webpack, :generate_spring_binstubs + public_task :generate_bundler_binstub, :generate_spring_binstubs + public_task :run_webpack def run_after_bundle_callbacks @after_bundle_callbacks.each(&:call) diff --git a/railties/lib/rails/generators/rails/app/templates/bin/bundle.tt b/railties/lib/rails/generators/rails/app/templates/bin/bundle.tt deleted file mode 100644 index a84f0afe47..0000000000 --- a/railties/lib/rails/generators/rails/app/templates/bin/bundle.tt +++ /dev/null @@ -1,2 +0,0 @@ -ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) -load Gem.bin_path('bundler', 'bundle') diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index d646694477..888336af92 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -69,7 +69,7 @@ Rails.application.configure do # Use a real queuing backend for Active Job (and separate queues per environment). # config.active_job.queue_adapter = :resque - # config.active_job.queue_name_prefix = "<%= app_name %>_#{Rails.env}" + # config.active_job.queue_name_prefix = "<%= app_name %>_production" <%- unless options.skip_action_mailer? -%> config.action_mailer.perform_caching = false diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt index 179b97de4a..54eb0cb1d2 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt @@ -8,3 +8,10 @@ # Don't force requests from old versions of IE to be UTF-8 encoded # Rails.application.config.action_view.default_enforce_utf8 = false + +# Embed purpose and expiry metadata inside signed and encrypted +# cookies for increased security. +# +# This option is not backwards compatible with earlier Rails versions. +# It's best enabled when your entire app is migrated and stable on 6.0. +# Rails.application.config.action_dispatch.use_cookies_with_metadata = true diff --git a/railties/lib/rails/tasks.rb b/railties/lib/rails/tasks.rb index 56f2eba312..2f644a20c9 100644 --- a/railties/lib/rails/tasks.rb +++ b/railties/lib/rails/tasks.rb @@ -12,6 +12,7 @@ require "rake" middleware misc restart + routes tmp yarn ).tap { |arr| diff --git a/railties/lib/rails/tasks/dev.rake b/railties/lib/rails/tasks/dev.rake index 5aea6f7dc5..8d75965294 100644 --- a/railties/lib/rails/tasks/dev.rake +++ b/railties/lib/rails/tasks/dev.rake @@ -1,10 +1,11 @@ # frozen_string_literal: true -require "rails/dev_caching" +require "rails/command" +require "active_support/deprecation" namespace :dev do - desc "Toggle development mode caching on/off" task :cache do - Rails::DevCaching.enable_by_file + ActiveSupport::Deprecation.warn("Using `bin/rake dev:cache` is deprecated and will be removed in Rails 6.1. Use `bin/rails dev:cache` instead.\n") + Rails::Command.invoke "dev:cache" end end diff --git a/railties/lib/rails/tasks/initializers.rake b/railties/lib/rails/tasks/initializers.rake index ae85cb0f86..7ccf7455bb 100644 --- a/railties/lib/rails/tasks/initializers.rake +++ b/railties/lib/rails/tasks/initializers.rake @@ -1,8 +1,10 @@ # frozen_string_literal: true +require "rails/command" +require "active_support/deprecation" + desc "Print out all defined initializers in the order they are invoked by Rails." -task initializers: :environment do - Rails.application.initializers.tsort_each do |initializer| - puts "#{initializer.context_class}.#{initializer.name}" - end +task :initializers do + ActiveSupport::Deprecation.warn("Using `bin/rake initializers` is deprecated and will be removed in Rails 6.1. Use `bin/rails initializers` instead.\n") + Rails::Command.invoke "initializers" end diff --git a/railties/lib/rails/tasks/routes.rake b/railties/lib/rails/tasks/routes.rake new file mode 100644 index 0000000000..21ce900a8c --- /dev/null +++ b/railties/lib/rails/tasks/routes.rake @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require "rails/command" +require "active_support/deprecation" + +task routes: :environment do + ActiveSupport::Deprecation.warn("Using `bin/rake routes` is deprecated and will be removed in Rails 6.1. Use `bin/rails routes` instead.\n") + Rails::Command.invoke "routes" +end diff --git a/railties/test/application/middleware/cookies_test.rb b/railties/test/application/middleware/cookies_test.rb index ecb4ee3446..fe48ef3f03 100644 --- a/railties/test/application/middleware/cookies_test.rb +++ b/railties/test/application/middleware/cookies_test.rb @@ -110,14 +110,14 @@ module ApplicationTests assert_equal "signed cookie".inspect, last_response.body get "/foo/read_raw_cookie" - assert_equal "signed cookie", verifier_sha512.verify(last_response.body) + assert_equal "signed cookie", verifier_sha512.verify(last_response.body, purpose: "cookie.signed_cookie") get "/foo/write_raw_cookie_sha256" get "/foo/read_signed" assert_equal "signed cookie".inspect, last_response.body get "/foo/read_raw_cookie" - assert_equal "signed cookie", verifier_sha512.verify(last_response.body) + assert_equal "signed cookie", verifier_sha512.verify(last_response.body, purpose: "cookie.signed_cookie") end test "encrypted cookies rotating multiple encryption keys" do @@ -180,14 +180,14 @@ module ApplicationTests assert_equal "encrypted cookie".inspect, last_response.body get "/foo/read_raw_cookie" - assert_equal "encrypted cookie", encryptor.decrypt_and_verify(last_response.body) + assert_equal "encrypted cookie", encryptor.decrypt_and_verify(last_response.body, purpose: "cookie.encrypted_cookie") - get "/foo/write_raw_cookie_sha256" + get "/foo/write_raw_cookie_two" get "/foo/read_encrypted" assert_equal "encrypted cookie".inspect, last_response.body get "/foo/read_raw_cookie" - assert_equal "encrypted cookie", encryptor.decrypt_and_verify(last_response.body) + assert_equal "encrypted cookie", encryptor.decrypt_and_verify(last_response.body, purpose: "cookie.encrypted_cookie") end end end diff --git a/railties/test/application/middleware/session_test.rb b/railties/test/application/middleware/session_test.rb index 9182a63ab7..b25e56b625 100644 --- a/railties/test/application/middleware/session_test.rb +++ b/railties/test/application/middleware/session_test.rb @@ -183,7 +183,7 @@ module ApplicationTests encryptor = ActiveSupport::MessageEncryptor.new(secret[0, ActiveSupport::MessageEncryptor.key_len(cipher)], cipher: cipher) get "/foo/read_raw_cookie" - assert_equal 1, encryptor.decrypt_and_verify(last_response.body)["foo"] + assert_equal 1, encryptor.decrypt_and_verify(last_response.body, purpose: "cookie._myapp_session")["foo"] end test "session upgrading signature to encryption cookie store works the same way as encrypted cookie store" do @@ -235,7 +235,7 @@ module ApplicationTests encryptor = ActiveSupport::MessageEncryptor.new(secret[0, ActiveSupport::MessageEncryptor.key_len(cipher)], cipher: cipher) get "/foo/read_raw_cookie" - assert_equal 1, encryptor.decrypt_and_verify(last_response.body)["foo"] + assert_equal 1, encryptor.decrypt_and_verify(last_response.body, purpose: "cookie._myapp_session")["foo"] end test "session upgrading signature to encryption cookie store upgrades session to encrypted mode" do @@ -297,7 +297,7 @@ module ApplicationTests encryptor = ActiveSupport::MessageEncryptor.new(secret[0, ActiveSupport::MessageEncryptor.key_len(cipher)], cipher: cipher) get "/foo/read_raw_cookie" - assert_equal 2, encryptor.decrypt_and_verify(last_response.body)["foo"] + assert_equal 2, encryptor.decrypt_and_verify(last_response.body, purpose: "cookie._myapp_session")["foo"] end test "session upgrading from AES-CBC-HMAC encryption to AES-GCM encryption" do @@ -364,7 +364,7 @@ module ApplicationTests encryptor = ActiveSupport::MessageEncryptor.new(secret[0, ActiveSupport::MessageEncryptor.key_len(cipher)], cipher: cipher) get "/foo/read_raw_cookie" - assert_equal 2, encryptor.decrypt_and_verify(last_response.body)["foo"] + assert_equal 2, encryptor.decrypt_and_verify(last_response.body, purpose: "cookie._myapp_session")["foo"] ensure ENV["RAILS_ENV"] = old_rails_env end @@ -428,7 +428,7 @@ module ApplicationTests verifier = ActiveSupport::MessageVerifier.new(app.secrets.secret_token) get "/foo/read_raw_cookie" - assert_equal 2, verifier.verify(last_response.body)["foo"] + assert_equal 2, verifier.verify(last_response.body, purpose: "cookie._myapp_session")["foo"] ensure ENV["RAILS_ENV"] = old_rails_env end diff --git a/railties/test/application/rake/dev_test.rb b/railties/test/application/rake/dev_test.rb index 66e1ac9d99..e408760ecc 100644 --- a/railties/test/application/rake/dev_test.rb +++ b/railties/test/application/rake/dev_test.rb @@ -17,33 +17,46 @@ module ApplicationTests test "dev:cache creates file and outputs message" do Dir.chdir(app_path) do - output = rails("dev:cache") - assert File.exist?("tmp/caching-dev.txt") - assert_match(/Development mode is now being cached/, output) + stderr = capture(:stderr) do + output = run_rake_dev_cache + assert File.exist?("tmp/caching-dev.txt") + assert_match(/Development mode is now being cached/, output) + end + assert_match(/DEPRECATION WARNING: Using `bin\/rake dev:cache` is deprecated and will be removed in Rails 6.1/, stderr) end end test "dev:cache deletes file and outputs message" do Dir.chdir(app_path) do - rails "dev:cache" # Create caching file. - output = rails("dev:cache") # Delete caching file. - assert_not File.exist?("tmp/caching-dev.txt") - assert_match(/Development mode is no longer being cached/, output) + stderr = capture(:stderr) do + run_rake_dev_cache # Create caching file. + output = run_rake_dev_cache # Delete caching file. + assert_not File.exist?("tmp/caching-dev.txt") + assert_match(/Development mode is no longer being cached/, output) + end + assert_match(/DEPRECATION WARNING: Using `bin\/rake dev:cache` is deprecated and will be removed in Rails 6.1/, stderr) end end test "dev:cache touches tmp/restart.txt" do Dir.chdir(app_path) do - rails "dev:cache" - assert File.exist?("tmp/restart.txt") - - prev_mtime = File.mtime("tmp/restart.txt") - sleep(1) - rails "dev:cache" - curr_mtime = File.mtime("tmp/restart.txt") - assert_not_equal prev_mtime, curr_mtime + stderr = capture(:stderr) do + run_rake_dev_cache + assert File.exist?("tmp/restart.txt") + + prev_mtime = File.mtime("tmp/restart.txt") + run_rake_dev_cache + curr_mtime = File.mtime("tmp/restart.txt") + assert_not_equal prev_mtime, curr_mtime + end + assert_match(/DEPRECATION WARNING: Using `bin\/rake dev:cache` is deprecated and will be removed in Rails 6.1/, stderr) end end + + private + def run_rake_dev_cache + `bin/rake dev:cache` + end end end end diff --git a/railties/test/application/rake/initializers_test.rb b/railties/test/application/rake/initializers_test.rb new file mode 100644 index 0000000000..fb498e28ad --- /dev/null +++ b/railties/test/application/rake/initializers_test.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" + +module ApplicationTests + module RakeTests + class RakeInitializersTest < ActiveSupport::TestCase + setup :build_app + teardown :teardown_app + + test "`rake initializers` prints out defined initializers invoked by Rails" do + capture(:stderr) do + initial_output = run_rake_initializers + initial_output_length = initial_output.split("\n").length + + assert_operator initial_output_length, :>, 0 + assert_not initial_output.include?("set_added_test_module") + + add_to_config <<-RUBY + initializer(:set_added_test_module) { } + RUBY + + final_output = run_rake_initializers + final_output_length = final_output.split("\n").length + + assert_equal 1, (final_output_length - initial_output_length) + assert final_output.include?("set_added_test_module") + end + end + + test "`rake initializers` outputs a deprecation warning" do + stderr = capture(:stderr) { run_rake_initializers } + assert_match(/DEPRECATION WARNING: Using `bin\/rake initializers` is deprecated and will be removed in Rails 6.1/, stderr) + end + + private + def run_rake_initializers + Dir.chdir(app_path) { `bin/rake initializers` } + end + end + end +end diff --git a/railties/test/application/rake/routes_test.rb b/railties/test/application/rake/routes_test.rb new file mode 100644 index 0000000000..e49ce50b69 --- /dev/null +++ b/railties/test/application/rake/routes_test.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" + +module ApplicationTests + module RakeTests + class RakeRoutesTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + setup :build_app + teardown :teardown_app + + test "`rake routes` outputs routes" do + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + get '/cart', to: 'cart#show' + end + RUBY + + assert_equal <<~MESSAGE, run_rake_routes + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show +rails_blob_representation GET /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show +update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + MESSAGE + end + + test "`rake routes` outputs a deprecation warning" do + remove_from_env_config("development", ".*config\.active_support\.deprecation.*\n") + add_to_env_config("development", "config.active_support.deprecation = :stderr") + + stderr = capture(:stderr) { run_rake_routes } + assert_match(/DEPRECATION WARNING: Using `bin\/rake routes` is deprecated and will be removed in Rails 6.1/, stderr) + end + + private + def run_rake_routes + Dir.chdir(app_path) { `bin/rake routes` } + end + end + end +end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 455dc60efd..5c34b205c9 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -525,9 +525,18 @@ module ApplicationTests def test_run_in_parallel_with_processes file_name = create_parallel_processes_test_file + app_file "db/schema.rb", <<-RUBY + ActiveRecord::Schema.define(version: 1) do + create_table :users do |t| + t.string :name + end + end + RUBY + output = run_test_command(file_name) assert_match %r{Finished in.*\n2 runs, 2 assertions}, output + assert_no_match "create_table(:users)", output end def test_run_in_parallel_with_threads @@ -539,9 +548,18 @@ module ApplicationTests file_name = create_parallel_threads_test_file + app_file "db/schema.rb", <<-RUBY + ActiveRecord::Schema.define(version: 1) do + create_table :users do |t| + t.string :name + end + end + RUBY + output = run_test_command(file_name) assert_match %r{Finished in.*\n2 runs, 2 assertions}, output + assert_no_match "create_table(:users)", output end def test_raise_error_when_specified_file_does_not_exist diff --git a/railties/test/backtrace_cleaner_test.rb b/railties/test/backtrace_cleaner_test.rb index 8490f9eb10..90e084ddca 100644 --- a/railties/test/backtrace_cleaner_test.rb +++ b/railties/test/backtrace_cleaner_test.rb @@ -8,22 +8,6 @@ class BacktraceCleanerTest < ActiveSupport::TestCase @cleaner = Rails::BacktraceCleaner.new end - test "should format installed gems correctly" do - backtrace = [ "#{Gem.path[0]}/gems/nosuchgem-1.2.3/lib/foo.rb" ] - result = @cleaner.clean(backtrace, :all) - assert_equal "nosuchgem (1.2.3) lib/foo.rb", result[0] - end - - test "should format installed gems not in Gem.default_dir correctly" do - target_dir = Gem.path.detect { |p| p != Gem.default_dir } - # skip this test if default_dir is the only directory on Gem.path - if target_dir - backtrace = [ "#{target_dir}/gems/nosuchgem-1.2.3/lib/foo.rb" ] - result = @cleaner.clean(backtrace, :all) - assert_equal "nosuchgem (1.2.3) lib/foo.rb", result[0] - end - end - test "should consider traces from irb lines as User code" do backtrace = [ "(irb):1", "/Path/to/rails/railties/lib/rails/commands/console.rb:77:in `start'", diff --git a/railties/test/commands/dev_test.rb b/railties/test/commands/dev_test.rb new file mode 100644 index 0000000000..ae8516fe9a --- /dev/null +++ b/railties/test/commands/dev_test.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" +require "rails/command" + +class Rails::Command::DevTest < ActiveSupport::TestCase + setup :build_app + teardown :teardown_app + + test "`rails dev:cache` creates both caching and restart file when restart file doesn't exist and dev caching is currently off" do + Dir.chdir(app_path) do + assert_not File.exist?("tmp/caching-dev.txt") + assert_not File.exist?("tmp/restart.txt") + + assert_equal <<~OUTPUT, run_dev_cache_command + Development mode is now being cached. + OUTPUT + + assert File.exist?("tmp/caching-dev.txt") + assert File.exist?("tmp/restart.txt") + end + end + + test "`rails dev:cache` creates caching file and touches restart file when dev caching is currently off" do + Dir.chdir(app_path) do + app_file("tmp/restart.txt", "") + + assert_not File.exist?("tmp/caching-dev.txt") + assert File.exist?("tmp/restart.txt") + restart_file_time_before = File.mtime("tmp/restart.txt") + + assert_equal <<~OUTPUT, run_dev_cache_command + Development mode is now being cached. + OUTPUT + + assert File.exist?("tmp/caching-dev.txt") + restart_file_time_after = File.mtime("tmp/restart.txt") + assert_operator restart_file_time_before, :<, restart_file_time_after + end + end + + test "`rails dev:cache` removes caching file and touches restart file when dev caching is currently on" do + Dir.chdir(app_path) do + app_file("tmp/caching-dev.txt", "") + app_file("tmp/restart.txt", "") + + assert File.exist?("tmp/caching-dev.txt") + assert File.exist?("tmp/restart.txt") + restart_file_time_before = File.mtime("tmp/restart.txt") + + assert_equal <<~OUTPUT, run_dev_cache_command + Development mode is no longer being cached. + OUTPUT + + assert_not File.exist?("tmp/caching-dev.txt") + restart_file_time_after = File.mtime("tmp/restart.txt") + assert_operator restart_file_time_before, :<, restart_file_time_after + end + end + + private + def run_dev_cache_command + rails "dev:cache" + end +end diff --git a/railties/test/commands/initializers_test.rb b/railties/test/commands/initializers_test.rb new file mode 100644 index 0000000000..bdfbb3021c --- /dev/null +++ b/railties/test/commands/initializers_test.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" +require "rails/command" + +class Rails::Command::InitializersTest < ActiveSupport::TestCase + setup :build_app + teardown :teardown_app + + test "`rails initializers` prints out defined initializers invoked by Rails" do + initial_output = run_initializers_command + initial_output_length = initial_output.split("\n").length + + assert_operator initial_output_length, :>, 0 + assert_not initial_output.include?("set_added_test_module") + + add_to_config <<-RUBY + initializer(:set_added_test_module) { } + RUBY + + final_output = run_initializers_command + final_output_length = final_output.split("\n").length + + assert_equal 1, (final_output_length - initial_output_length) + assert final_output.include?("set_added_test_module") + end + + private + def run_initializers_command + rails "initializers" + end +end diff --git a/railties/test/commands/routes_test.rb b/railties/test/commands/routes_test.rb index 77ed2bda61..693e532c5b 100644 --- a/railties/test/commands/routes_test.rb +++ b/railties/test/commands/routes_test.rb @@ -13,20 +13,33 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do resource :post + resource :user_permission end RUBY - expected_output = [" Prefix Verb URI Pattern Controller#Action", - " new_post GET /post/new(.:format) posts#new", - "edit_post GET /post/edit(.:format) posts#edit", - " post GET /post(.:format) posts#show", - " PATCH /post(.:format) posts#update", - " PUT /post(.:format) posts#update", - " DELETE /post(.:format) posts#destroy", - " POST /post(.:format) posts#create\n"].join("\n") + expected_post_output = [" Prefix Verb URI Pattern Controller#Action", + " new_post GET /post/new(.:format) posts#new", + "edit_post GET /post/edit(.:format) posts#edit", + " post GET /post(.:format) posts#show", + " PATCH /post(.:format) posts#update", + " PUT /post(.:format) posts#update", + " DELETE /post(.:format) posts#destroy", + " POST /post(.:format) posts#create\n"].join("\n") output = run_routes_command(["-c", "PostController"]) - assert_equal expected_output, output + assert_equal expected_post_output, output + + expected_perm_output = [" Prefix Verb URI Pattern Controller#Action", + " new_user_permission GET /user_permission/new(.:format) user_permissions#new", + "edit_user_permission GET /user_permission/edit(.:format) user_permissions#edit", + " user_permission GET /user_permission(.:format) user_permissions#show", + " PATCH /user_permission(.:format) user_permissions#update", + " PUT /user_permission(.:format) user_permissions#update", + " DELETE /user_permission(.:format) user_permissions#destroy", + " POST /user_permission(.:format) user_permissions#create\n"].join("\n") + + output = run_routes_command(["-c", "UserPermissionController"]) + assert_equal expected_perm_output, output end test "rails routes with global search key" do @@ -64,17 +77,30 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase Rails.application.routes.draw do get '/cart', to: 'cart#show' get '/basketball', to: 'basketball#index' + get '/user_permission', to: 'user_permission#index' end RUBY + expected_cart_output = "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n" output = run_routes_command(["-c", "cart"]) - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal expected_cart_output, output output = run_routes_command(["-c", "Cart"]) - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal expected_cart_output, output output = run_routes_command(["-c", "CartController"]) - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal expected_cart_output, output + + expected_perm_output = [" Prefix Verb URI Pattern Controller#Action", + "user_permission GET /user_permission(.:format) user_permission#index\n"].join("\n") + output = run_routes_command(["-c", "user_permission"]) + assert_equal expected_perm_output, output + + output = run_routes_command(["-c", "UserPermission"]) + assert_equal expected_perm_output, output + + output = run_routes_command(["-c", "UserPermissionController"]) + assert_equal expected_perm_output, output end test "rails routes with namespaced controller search key" do @@ -82,24 +108,40 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase Rails.application.routes.draw do namespace :admin do resource :post + resource :user_permission end end RUBY - expected_output = [" Prefix Verb URI Pattern Controller#Action", - " new_admin_post GET /admin/post/new(.:format) admin/posts#new", - "edit_admin_post GET /admin/post/edit(.:format) admin/posts#edit", - " admin_post GET /admin/post(.:format) admin/posts#show", - " PATCH /admin/post(.:format) admin/posts#update", - " PUT /admin/post(.:format) admin/posts#update", - " DELETE /admin/post(.:format) admin/posts#destroy", - " POST /admin/post(.:format) admin/posts#create\n"].join("\n") + expected_post_output = [" Prefix Verb URI Pattern Controller#Action", + " new_admin_post GET /admin/post/new(.:format) admin/posts#new", + "edit_admin_post GET /admin/post/edit(.:format) admin/posts#edit", + " admin_post GET /admin/post(.:format) admin/posts#show", + " PATCH /admin/post(.:format) admin/posts#update", + " PUT /admin/post(.:format) admin/posts#update", + " DELETE /admin/post(.:format) admin/posts#destroy", + " POST /admin/post(.:format) admin/posts#create\n"].join("\n") output = run_routes_command(["-c", "Admin::PostController"]) - assert_equal expected_output, output + assert_equal expected_post_output, output output = run_routes_command(["-c", "PostController"]) - assert_equal expected_output, output + assert_equal expected_post_output, output + + expected_perm_output = [" Prefix Verb URI Pattern Controller#Action", + " new_admin_user_permission GET /admin/user_permission/new(.:format) admin/user_permissions#new", + "edit_admin_user_permission GET /admin/user_permission/edit(.:format) admin/user_permissions#edit", + " admin_user_permission GET /admin/user_permission(.:format) admin/user_permissions#show", + " PATCH /admin/user_permission(.:format) admin/user_permissions#update", + " PUT /admin/user_permission(.:format) admin/user_permissions#update", + " DELETE /admin/user_permission(.:format) admin/user_permissions#destroy", + " POST /admin/user_permission(.:format) admin/user_permissions#create\n"].join("\n") + + output = run_routes_command(["-c", "Admin::UserPermissionController"]) + assert_equal expected_perm_output, output + + output = run_routes_command(["-c", "UserPermissionController"]) + assert_equal expected_perm_output, output end test "rails routes displays message when no routes are defined" do diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb index 9c523ad372..c2540f4091 100644 --- a/railties/test/generators/api_app_generator_test.rb +++ b/railties/test/generators/api_app_generator_test.rb @@ -118,7 +118,6 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase app/views/layouts app/views/layouts/mailer.html.erb app/views/layouts/mailer.text.erb - bin/bundle bin/rails bin/rake bin/setup diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index b0f958091c..f33a7bd99d 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -37,7 +37,6 @@ DEFAULT_APP_FILES = %w( app/views/layouts/application.html.erb app/views/layouts/mailer.html.erb app/views/layouts/mailer.text.erb - bin/bundle bin/rails bin/rake bin/setup @@ -763,17 +762,23 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_generation_runs_bundle_install - assert_generates_with_bundler + generator([destination_root], {}) + + assert_bundler_command_called("install") end def test_dev_option - assert_generates_with_bundler dev: true + generator([destination_root], dev: true) + + assert_bundler_command_called("install") rails_path = File.expand_path("../../..", Rails.root) assert_file "Gemfile", /^gem\s+["']rails["'],\s+path:\s+["']#{Regexp.escape(rails_path)}["']$/ end def test_edge_option - assert_generates_with_bundler edge: true + generator([destination_root], edge: true) + + assert_bundler_command_called("install") assert_file "Gemfile", %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["']$} end @@ -782,23 +787,14 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_gem "spring" end + def test_bundler_binstub + assert_bundler_command_called("binstubs bundler") + end + def test_spring_binstubs jruby_skip "spring doesn't run on JRuby" - command_check = -> command do - @binstub_called ||= 0 - - case command - when "install" - # Called when running bundle, we just want to stub it so nothing to do here. - when "exec spring binstub --all" - @binstub_called += 1 - assert_equal 1, @binstub_called, "exec spring binstub --all expected to be called once, but was called #{@install_called} times." - end - end - generator.stub :bundle_command, command_check do - quietly { generator.invoke_all } - end + assert_bundler_command_called("exec spring binstub --all") end def test_spring_no_fork @@ -976,7 +972,7 @@ class AppGeneratorTest < Rails::Generators::TestCase template end - sequence = ["git init", "install", "exec spring binstub --all", "echo ran after_bundle"] + sequence = ["git init", "install", "binstubs bundler", "exec spring binstub --all", "echo ran after_bundle"] @sequence_step ||= 0 ensure_bundler_first = -> command, options = nil do assert_equal sequence[@sequence_step], command, "commands should be called in sequence #{sequence}" @@ -993,7 +989,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - assert_equal 4, @sequence_step + assert_equal 5, @sequence_step end def test_gitignore @@ -1063,18 +1059,14 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def assert_generates_with_bundler(options = {}) - generator([destination_root], options) - + def assert_bundler_command_called(target_command) command_check = -> command do - @install_called ||= 0 + @command_called ||= 0 case command - when "install" - @install_called += 1 - assert_equal 1, @install_called, "install expected to be called once, but was called #{@install_called} times" - when "exec spring binstub --all" - # Called when running tests with spring, let through unscathed. + when target_command + @command_called += 1 + assert_equal 1, @command_called, "#{command} expected to be called once, but was called #{@command_called} times." end end diff --git a/tasks/release.rb b/tasks/release.rb index f13342b90c..1e83814bae 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -105,7 +105,7 @@ namespace :changelog do current_contents = File.read(fname) header = "## Rails #{version} (#{Date.today.strftime('%B %d, %Y')}) ##\n\n" - header += "* No changes.\n\n\n" if current_contents =~ /\A##/ + header += "* No changes.\n\n\n" if current_contents.start_with?("##") contents = header + current_contents File.write(fname, contents) end |