diff options
165 files changed, 1352 insertions, 1852 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 6eca479d9d..96e2ad76d0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,5 @@ AllCops: - TargetRubyVersion: 2.4 + TargetRubyVersion: 2.5 # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop # to ignore them, so only the ones explicitly set in this file are enabled. DisabledByDefault: true @@ -187,6 +187,9 @@ Lint/UriEscapeUnescape: Style/ParenthesesAroundCondition: Enabled: true +Style/RedundantBegin: + Enabled: true + Style/RedundantReturn: Enabled: true AllowMultipleReturnValues: true diff --git a/.travis.yml b/.travis.yml index 6acba41c0e..2e00860c95 100644 --- a/.travis.yml +++ b/.travis.yml @@ -64,21 +64,11 @@ env: - "GEM=ac:integration" rvm: - - 2.4.5 - 2.5.3 - ruby-head matrix: include: - - rvm: 2.4.5 - env: "GEM=railties" - sudo: required - before_install: - - "rm ${BUNDLE_GEMFILE}.lock" - - "travis_retry gem update --system" - - "travis_retry gem install bundler" - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - rvm: 2.5.3 env: "GEM=railties" sudo: required @@ -99,19 +89,6 @@ matrix: - "sudo service postgresql restart 10" - rvm: 2.5.3 env: "GEM=av:ujs" - - rvm: 2.4.5 - sudo: required - env: "GEM=aj:integration" - services: - - memcached - - redis-server - - rabbitmq - before_install: - - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - rvm: 2.5.3 sudo: required env: "GEM=aj:integration" @@ -138,13 +115,6 @@ matrix: - "sudo service postgresql restart 10" - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - - rvm: 2.4.5 - env: "GEM=ar:mysql2" - sudo: required - before_install: - - "sudo mysql -e \"use mysql; update user set authentication_string='' where User='root'; update user set plugin='mysql_native_password';FLUSH PRIVILEGES;\"" - - "sudo mysql_upgrade" - - "sudo service mysql restart" - rvm: 2.5.3 env: "GEM=ar:mysql2" sudo: required @@ -167,12 +137,6 @@ matrix: - rvm: 2.5.3 env: - "GEM=ar:sqlite3_mem" - - rvm: 2.4.5 - env: "GEM=ar:postgresql" - sudo: required - before_install: - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - rvm: 2.5.3 env: "GEM=ar:postgresql" sudo: required diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 88a74a521f..162de0df0b 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -66,9 +66,9 @@ *Ilia Kasianenko* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/actioncable/CHANGELOG.md) for previous changes. diff --git a/actioncable/actioncable.gemspec b/actioncable/actioncable.gemspec index 31c6fb41f2..29836f012f 100644 --- a/actioncable/actioncable.gemspec +++ b/actioncable/actioncable.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "WebSocket framework for Rails." s.description = "Structure many real-time application concerns into channels over a single WebSocket connection." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/actioncable/lib/action_cable/server/worker.rb b/actioncable/lib/action_cable/server/worker.rb index c69cc4ac31..187c8f7939 100644 --- a/actioncable/lib/action_cable/server/worker.rb +++ b/actioncable/lib/action_cable/server/worker.rb @@ -56,14 +56,12 @@ module ActionCable def invoke(receiver, method, *args, connection:, &block) work(connection) do - begin - receiver.send method, *args, &block - rescue Exception => e - logger.error "There was an exception - #{e.class}(#{e.message})" - logger.error e.backtrace.join("\n") + receiver.send method, *args, &block + rescue Exception => e + logger.error "There was an exception - #{e.class}(#{e.message})" + logger.error e.backtrace.join("\n") - receiver.handle_exception if receiver.respond_to?(:handle_exception) - end + receiver.handle_exception if receiver.respond_to?(:handle_exception) end end diff --git a/actioncable/test/channel/base_test.rb b/actioncable/test/channel/base_test.rb index c2968226c7..39b5879607 100644 --- a/actioncable/test/channel/base_test.rb +++ b/actioncable/test/channel/base_test.rb @@ -190,81 +190,73 @@ class ActionCable::Channel::BaseTest < ActionCable::TestCase end test "notification for perform_action" do - begin - events = [] - ActiveSupport::Notifications.subscribe "perform_action.action_cable" do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end + events = [] + ActiveSupport::Notifications.subscribe "perform_action.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end - data = { "action" => :speak, "content" => "hello" } - @channel.perform_action data + data = { "action" => :speak, "content" => "hello" } + @channel.perform_action data - assert_equal 1, events.length - assert_equal "perform_action.action_cable", events[0].name - assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] - assert_equal :speak, events[0].payload[:action] - assert_equal data, events[0].payload[:data] - ensure - ActiveSupport::Notifications.unsubscribe "perform_action.action_cable" - end + assert_equal 1, events.length + assert_equal "perform_action.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] + assert_equal :speak, events[0].payload[:action] + assert_equal data, events[0].payload[:data] + ensure + ActiveSupport::Notifications.unsubscribe "perform_action.action_cable" end test "notification for transmit" do - begin - events = [] - ActiveSupport::Notifications.subscribe "transmit.action_cable" do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end - - @channel.perform_action "action" => :get_latest - expected_data = { data: "latest" } - - assert_equal 1, events.length - assert_equal "transmit.action_cable", events[0].name - assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] - assert_equal expected_data, events[0].payload[:data] - assert_nil events[0].payload[:via] - ensure - ActiveSupport::Notifications.unsubscribe "transmit.action_cable" + events = [] + ActiveSupport::Notifications.subscribe "transmit.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) end + + @channel.perform_action "action" => :get_latest + expected_data = { data: "latest" } + + assert_equal 1, events.length + assert_equal "transmit.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] + assert_equal expected_data, events[0].payload[:data] + assert_nil events[0].payload[:via] + ensure + ActiveSupport::Notifications.unsubscribe "transmit.action_cable" end test "notification for transmit_subscription_confirmation" do - begin - @channel.subscribe_to_channel + @channel.subscribe_to_channel - events = [] - ActiveSupport::Notifications.subscribe "transmit_subscription_confirmation.action_cable" do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end + events = [] + ActiveSupport::Notifications.subscribe "transmit_subscription_confirmation.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end - @channel.stub(:subscription_confirmation_sent?, false) do - @channel.send(:transmit_subscription_confirmation) + @channel.stub(:subscription_confirmation_sent?, false) do + @channel.send(:transmit_subscription_confirmation) - assert_equal 1, events.length - assert_equal "transmit_subscription_confirmation.action_cable", events[0].name - assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] - end - ensure - ActiveSupport::Notifications.unsubscribe "transmit_subscription_confirmation.action_cable" + assert_equal 1, events.length + assert_equal "transmit_subscription_confirmation.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] end + ensure + ActiveSupport::Notifications.unsubscribe "transmit_subscription_confirmation.action_cable" end test "notification for transmit_subscription_rejection" do - begin - events = [] - ActiveSupport::Notifications.subscribe "transmit_subscription_rejection.action_cable" do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end + events = [] + ActiveSupport::Notifications.subscribe "transmit_subscription_rejection.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end - @channel.send(:transmit_subscription_rejection) + @channel.send(:transmit_subscription_rejection) - assert_equal 1, events.length - assert_equal "transmit_subscription_rejection.action_cable", events[0].name - assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] - ensure - ActiveSupport::Notifications.unsubscribe "transmit_subscription_rejection.action_cable" - end + assert_equal 1, events.length + assert_equal "transmit_subscription_rejection.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] + ensure + ActiveSupport::Notifications.unsubscribe "transmit_subscription_rejection.action_cable" end test "behaves like rescuable" do diff --git a/actioncable/test/server/broadcasting_test.rb b/actioncable/test/server/broadcasting_test.rb index 03c900182a..860e79b821 100644 --- a/actioncable/test/server/broadcasting_test.rb +++ b/actioncable/test/server/broadcasting_test.rb @@ -13,50 +13,46 @@ class BroadcastingTest < ActionCable::TestCase end test "broadcast generates notification" do - begin - server = TestServer.new - - events = [] - ActiveSupport::Notifications.subscribe "broadcast.action_cable" do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end - - broadcasting = "test_queue" - message = { body: "test message" } - server.broadcast(broadcasting, message) - - assert_equal 1, events.length - assert_equal "broadcast.action_cable", events[0].name - assert_equal broadcasting, events[0].payload[:broadcasting] - assert_equal message, events[0].payload[:message] - assert_equal ActiveSupport::JSON, events[0].payload[:coder] - ensure - ActiveSupport::Notifications.unsubscribe "broadcast.action_cable" + server = TestServer.new + + events = [] + ActiveSupport::Notifications.subscribe "broadcast.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) end + + broadcasting = "test_queue" + message = { body: "test message" } + server.broadcast(broadcasting, message) + + assert_equal 1, events.length + assert_equal "broadcast.action_cable", events[0].name + assert_equal broadcasting, events[0].payload[:broadcasting] + assert_equal message, events[0].payload[:message] + assert_equal ActiveSupport::JSON, events[0].payload[:coder] + ensure + ActiveSupport::Notifications.unsubscribe "broadcast.action_cable" end test "broadcaster from broadcaster_for generates notification" do - begin - server = TestServer.new - - events = [] - ActiveSupport::Notifications.subscribe "broadcast.action_cable" do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end - - broadcasting = "test_queue" - message = { body: "test message" } - - broadcaster = server.broadcaster_for(broadcasting) - broadcaster.broadcast(message) - - assert_equal 1, events.length - assert_equal "broadcast.action_cable", events[0].name - assert_equal broadcasting, events[0].payload[:broadcasting] - assert_equal message, events[0].payload[:message] - assert_equal ActiveSupport::JSON, events[0].payload[:coder] - ensure - ActiveSupport::Notifications.unsubscribe "broadcast.action_cable" + server = TestServer.new + + events = [] + ActiveSupport::Notifications.subscribe "broadcast.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) end + + broadcasting = "test_queue" + message = { body: "test message" } + + broadcaster = server.broadcaster_for(broadcasting) + broadcaster.broadcast(message) + + assert_equal 1, events.length + assert_equal "broadcast.action_cable", events[0].name + assert_equal broadcasting, events[0].payload[:broadcasting] + assert_equal message, events[0].payload[:message] + assert_equal ActiveSupport::JSON, events[0].payload[:coder] + ensure + ActiveSupport::Notifications.unsubscribe "broadcast.action_cable" end end diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 2de83e611b..5f3fc44e6e 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -57,9 +57,9 @@ *Claudio Ortolina*, *Kota Miyake* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/actionmailer/CHANGELOG.md) for previous changes. diff --git a/actionmailer/actionmailer.gemspec b/actionmailer/actionmailer.gemspec index 6f8d6d0699..a6c482f1a0 100644 --- a/actionmailer/actionmailer.gemspec +++ b/actionmailer/actionmailer.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Email composition, delivery, and receiving framework (part of Rails)." s.description = "Email on Rails. Compose, deliver, receive, and test emails using the familiar controller/view pattern. First-class support for multipart email and attachments." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 86c0172772..d0c4f189fd 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -90,18 +90,18 @@ class BaseTest < ActiveSupport::TestCase test "can pass random headers in as a hash to mail" do hash = { "X-Special-Domain-Specific-Header" => "SecretValue", - "In-Reply-To" => "1234@mikel.me.com" } + "In-Reply-To" => "<1234@mikel.me.com>" } mail = BaseMailer.welcome(hash) assert_equal("SecretValue", mail["X-Special-Domain-Specific-Header"].decoded) - assert_equal("1234@mikel.me.com", mail["In-Reply-To"].decoded) + assert_equal("<1234@mikel.me.com>", mail["In-Reply-To"].decoded) end test "can pass random headers in as a hash to headers" do hash = { "X-Special-Domain-Specific-Header" => "SecretValue", - "In-Reply-To" => "1234@mikel.me.com" } + "In-Reply-To" => "<1234@mikel.me.com>" } mail = BaseMailer.welcome_with_headers(hash) assert_equal("SecretValue", mail["X-Special-Domain-Specific-Header"].decoded) - assert_equal("1234@mikel.me.com", mail["In-Reply-To"].decoded) + assert_equal("<1234@mikel.me.com>", mail["In-Reply-To"].decoded) end # Attachments @@ -897,22 +897,20 @@ class BaseTest < ActiveSupport::TestCase end test "notification for process" do - begin - events = [] - ActiveSupport::Notifications.subscribe("process.action_mailer") do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end + events = [] + ActiveSupport::Notifications.subscribe("process.action_mailer") do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end - BaseMailer.welcome(body: "Hello there").deliver_now + BaseMailer.welcome(body: "Hello there").deliver_now - assert_equal 1, events.length - assert_equal "process.action_mailer", events[0].name - assert_equal "BaseMailer", events[0].payload[:mailer] - assert_equal :welcome, events[0].payload[:action] - assert_equal [{ body: "Hello there" }], events[0].payload[:args] - ensure - ActiveSupport::Notifications.unsubscribe "process.action_mailer" - end + assert_equal 1, events.length + assert_equal "process.action_mailer", events[0].name + assert_equal "BaseMailer", events[0].payload[:mailer] + assert_equal :welcome, events[0].payload[:action] + assert_equal [{ body: "Hello there" }], events[0].payload[:args] + ensure + ActiveSupport::Notifications.unsubscribe "process.action_mailer" end private diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 13fbbafc0c..95da4265a4 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -193,9 +193,9 @@ *Derek Prior* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index ec56de18f1..c4c755b7ac 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Web-flow and rendering framework putting the VC in MVC (part of Rails)." s.description = "Web apps on Rails. Simple, battle-tested conventions for building and testing MVC web applications. Works with any Rack-compatible server." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index d6911ee2b5..29d1919ec5 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/hash/keys" - module ActionController module ConditionalGet extend ActiveSupport::Concern diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index be9449629f..51fac08749 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -30,13 +30,11 @@ module ActionController ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) ActiveSupport::Notifications.instrument("process_action.action_controller", raw_payload) do |payload| - begin - result = super + super.tap do payload[:status] = response.status - result - ensure - append_info_to_payload(payload) end + ensure + append_info_to_payload(payload) end end diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index 2b4559c760..cf8c0159e2 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/hash/keys" - module ActionController # ActionController::Renderer allows you to render arbitrary templates # without requirement of being in controller actions. diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb index 5b2ad36dd5..87fe19225b 100644 --- a/actionpack/lib/action_dispatch/middleware/callbacks.rb +++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb @@ -24,10 +24,8 @@ module ActionDispatch def call(env) error = nil result = run_callbacks :call do - begin - @app.call(env) - rescue => error - end + @app.call(env) + rescue => error end raise error if error result diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index eadb59173d..61773d97a2 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -48,11 +48,9 @@ module ActionDispatch wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) @interceptors.each do |interceptor| - begin - interceptor.call(request, exception) - rescue Exception - log_error(request, wrapper) - end + interceptor.call(request, exception) + rescue Exception + log_error(request, wrapper) end end diff --git a/actionpack/lib/action_dispatch/middleware/host_authorization.rb b/actionpack/lib/action_dispatch/middleware/host_authorization.rb index 48f7c25216..447b70112a 100644 --- a/actionpack/lib/action_dispatch/middleware/host_authorization.rb +++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb @@ -21,13 +21,11 @@ module ActionDispatch def allows?(host) @hosts.any? do |allowed| - begin - allowed === host - rescue - # IPAddr#=== raises an error if you give it a hostname instead of - # IP. Treat similar errors as blocked access. - false - end + allowed === host + rescue + # IPAddr#=== raises an error if you give it a hostname instead of + # IP. Treat similar errors as blocked access. + false end end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 35158f9062..a5667573f4 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -162,14 +162,12 @@ module ActionDispatch # Split the comma-separated list into an array of strings. ips = header.strip.split(/[,\s]+/) ips.select do |ip| - begin - # Only return IPs that are valid according to the IPAddr#new method. - range = IPAddr.new(ip).to_range - # We want to make sure nobody is sneaking a netmask in. - range.begin == range.end - rescue ArgumentError - nil - end + # Only return IPs that are valid according to the IPAddr#new method. + range = IPAddr.new(ip).to_range + # We want to make sure nobody is sneaking a netmask in. + range.begin == range.end + rescue ArgumentError + nil end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 2ae75b0da8..2966c969f6 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -90,11 +90,11 @@ module ActionDispatch def clear! @path_helpers.each do |helper| - @path_helpers_module.send :remove_method, helper + @path_helpers_module.remove_method helper end @url_helpers.each do |helper| - @url_helpers_module.send :remove_method, helper + @url_helpers_module.remove_method helper end @routes.clear @@ -108,8 +108,8 @@ module ActionDispatch url_name = :"#{name}_url" if routes.key? key - @path_helpers_module.send :undef_method, path_name - @url_helpers_module.send :undef_method, url_name + @path_helpers_module.undef_method path_name + @url_helpers_module.undef_method url_name end routes[key] = route define_url_helper @path_helpers_module, route, path_name, route.defaults, name, PATH diff --git a/actionpack/test/abstract/collector_test.rb b/actionpack/test/abstract/collector_test.rb index a4770b66e1..6db045fcd7 100644 --- a/actionpack/test/abstract/collector_test.rb +++ b/actionpack/test/abstract/collector_test.rb @@ -30,7 +30,7 @@ module AbstractController end test "register mime types on method missing" do - AbstractController::Collector.send(:remove_method, :js) + AbstractController::Collector.remove_method :js begin collector = MyCollector.new assert_not_respond_to collector, :js diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 763df3a776..ecb8c37e6b 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -276,16 +276,14 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end def test_assert_redirect_failure_message_with_protocol_relative_url - begin - process :redirect_external_protocol_relative - assert_redirected_to "/foo" - rescue ActiveSupport::TestCase::Assertion => ex - assert_no_match( - /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, - ex.message, - "protocol relative url was incorrectly normalized" - ) - end + process :redirect_external_protocol_relative + assert_redirected_to "/foo" + rescue ActiveSupport::TestCase::Assertion => ex + assert_no_match( + /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, + ex.message, + "protocol relative url was incorrectly normalized" + ) end def test_template_objects_exist diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 8e117528e2..104c9eeade 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -1000,16 +1000,12 @@ class YieldingAroundFiltersTest < ActionController::TestCase def test_nested_actions controller = ControllerWithNestedFilters assert_nothing_raised do - begin - test_process(controller, "raises_both") - rescue Before, After - end + test_process(controller, "raises_both") + rescue Before, After end assert_raise Before do - begin - test_process(controller, "raises_both") - rescue After - end + test_process(controller, "raises_both") + rescue After end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 39ede1442a..b5503a9c64 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -152,7 +152,7 @@ class IntegrationTestTest < ActiveSupport::TestCase assert_equal "pass", @test.foo ensure # leave other tests as unaffected as possible - mixin.__send__(:remove_method, :method_missing) + mixin.remove_method :method_missing end end end diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index d2fa0aa16e..fbfe24059b 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -365,17 +365,15 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "permitted takes a default value when Parameters.permit_all_parameters is set" do - begin - ActionController::Parameters.permit_all_parameters = true - params = ActionController::Parameters.new(person: { - age: "32", name: { first: "David", last: "Heinemeier Hansson" } - }) - - assert_predicate params.slice(:person), :permitted? - assert_predicate params[:person][:name], :permitted? - ensure - ActionController::Parameters.permit_all_parameters = false - end + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(person: { + age: "32", name: { first: "David", last: "Heinemeier Hansson" } + }) + + assert_predicate params.slice(:person), :permitted? + assert_predicate params[:person][:name], :permitted? + ensure + ActionController::Parameters.permit_all_parameters = false end test "permitting parameters as an array" do @@ -396,16 +394,14 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "to_h returns converted hash when .permit_all_parameters is set" do - begin - ActionController::Parameters.permit_all_parameters = true - params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") - - assert_instance_of ActiveSupport::HashWithIndifferentAccess, params.to_h - assert_not_kind_of ActionController::Parameters, params.to_h - assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h) - ensure - ActionController::Parameters.permit_all_parameters = false - end + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") + + assert_instance_of ActiveSupport::HashWithIndifferentAccess, params.to_h + assert_not_kind_of ActionController::Parameters, params.to_h + assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h) + ensure + ActionController::Parameters.permit_all_parameters = false end test "to_hash raises UnfilteredParameters on unfiltered params" do @@ -429,17 +425,15 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "to_hash returns converted hash when .permit_all_parameters is set" do - begin - ActionController::Parameters.permit_all_parameters = true - params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") - - assert_instance_of Hash, params.to_hash - assert_not_kind_of ActionController::Parameters, params.to_hash - assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_hash) - assert_equal({ "crab" => "Senjougahara Hitagi" }, params) - ensure - ActionController::Parameters.permit_all_parameters = false - end + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") + + assert_instance_of Hash, params.to_hash + assert_not_kind_of ActionController::Parameters, params.to_hash + assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_hash) + assert_equal({ "crab" => "Senjougahara Hitagi" }, params) + ensure + ActionController::Parameters.permit_all_parameters = false end test "to_unsafe_h returns unfiltered params" do diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 37399cfd07..c326f276bf 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -27,14 +27,12 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end def raise_nested_exceptions + raise "First error" + rescue begin - raise "First error" + raise "Second error" rescue - begin - raise "Second error" - rescue - raise "Third error" - end + raise "Third error" end end @@ -290,22 +288,20 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end test "rescue with JSON format as fallback if API request format is not supported" do - begin - Mime::Type.register "text/wibble", :wibble + Mime::Type.register "text/wibble", :wibble - ActionDispatch::IntegrationTest.register_encoder(:wibble, - param_encoder: -> params { params }) + ActionDispatch::IntegrationTest.register_encoder(:wibble, + param_encoder: -> params { params }) - @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) - get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble - assert_response 500 - assert_equal "application/json", response.content_type - assert_match(/RuntimeError: puke/, body) + get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble + assert_response 500 + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) - ensure - Mime::Type.unregister :wibble - end + ensure + Mime::Type.unregister :wibble end test "does not show filtered parameters" do diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index fa264417e1..45d91883c0 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -96,57 +96,47 @@ class MimeTypeTest < ActiveSupport::TestCase end test "custom type" do - begin - type = Mime::Type.register("image/foo", :foo) - assert_equal type, Mime[:foo] - ensure - Mime::Type.unregister(:foo) - end + type = Mime::Type.register("image/foo", :foo) + assert_equal type, Mime[:foo] + ensure + Mime::Type.unregister(:foo) end test "custom type with type aliases" do - begin - Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"] - %w[text/foobar text/foo text/bar].each do |type| - assert_equal Mime[:foobar], type - end - ensure - Mime::Type.unregister(:foobar) + Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"] + %w[text/foobar text/foo text/bar].each do |type| + assert_equal Mime[:foobar], type end + ensure + Mime::Type.unregister(:foobar) end test "register callbacks" do - begin - registered_mimes = [] - Mime::Type.register_callback do |mime| - registered_mimes << mime - end - - mime = Mime::Type.register("text/foo", :foo) - assert_equal [mime], registered_mimes - ensure - Mime::Type.unregister(:foo) + registered_mimes = [] + Mime::Type.register_callback do |mime| + registered_mimes << mime end + + mime = Mime::Type.register("text/foo", :foo) + assert_equal [mime], registered_mimes + ensure + Mime::Type.unregister(:foo) end test "custom type with extension aliases" do - begin - Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"] - %w[foobar foo bar].each do |extension| - assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension] - end - ensure - Mime::Type.unregister(:foobar) + Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"] + %w[foobar foo bar].each do |extension| + assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension] end + ensure + Mime::Type.unregister(:foobar) end test "register alias" do - begin - Mime::Type.register_alias "application/xhtml+xml", :foobar - assert_equal Mime[:html], Mime::EXTENSION_LOOKUP["foobar"] - ensure - Mime::Type.unregister(:foobar) - end + Mime::Type.register_alias "application/xhtml+xml", :foobar + assert_equal Mime[:html], Mime::EXTENSION_LOOKUP["foobar"] + ensure + Mime::Type.unregister(:foobar) end test "type should be equal to symbol" do diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index beab8e78b5..2a48a12497 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -74,17 +74,15 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest test "occurring a parse error if parsing unsuccessful" do with_test_routing do - begin - $stderr = StringIO.new # suppress the log - json = "[\"person]\": {\"name\": \"David\"}}" - exception = assert_raise(ActionDispatch::Http::Parameters::ParseError) do - post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false } - end - assert_equal JSON::ParserError, exception.cause.class - assert_equal exception.cause.message, exception.message - ensure - $stderr = STDERR + $stderr = StringIO.new # suppress the log + json = "[\"person]\": {\"name\": \"David\"}}" + exception = assert_raise(ActionDispatch::Http::Parameters::ParseError) do + post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false } end + assert_equal JSON::ParserError, exception.cause.class + assert_equal exception.cause.message, exception.message + ensure + $stderr = STDERR end end @@ -157,31 +155,27 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest end test "parses json params after custom json mime type registered" do - begin - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) - assert_parses( - { "user" => { "username" => "meinac" }, "username" => "meinac" }, - "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/json" - ) - ensure - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) - end + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) + assert_parses( + { "user" => { "username" => "meinac" }, "username" => "meinac" }, + "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/json" + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) end test "parses json params after custom json mime type registered with synonym" do - begin - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) - assert_parses( - { "user" => { "username" => "meinac" }, "username" => "meinac" }, - "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/vnd.rails+json" - ) - ensure - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) - end + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) + assert_parses( + { "user" => { "username" => "meinac" }, "username" => "meinac" }, + "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/vnd.rails+json" + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) end private diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 0f37d074af..60817c1c4d 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -42,7 +42,7 @@ class ResponseTest < ActiveSupport::TestCase def test_each_isnt_called_if_str_body_is_written # Controller writes and reads response body each_counter = 0 - @response.body = Object.new.tap { |o| o.singleton_class.send(:define_method, :each) { |&block| each_counter += 1; block.call "foo" } } + @response.body = Object.new.tap { |o| o.singleton_class.define_method(:each) { |&block| each_counter += 1; block.call "foo" } } @response["X-Foo"] = @response.body assert_equal 1, each_counter, "#each was not called once" diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index de79c05657..097ef8af29 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -41,22 +41,20 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end test "display_image return artifact format when specify RAILS_SYSTEM_TESTING_SCREENSHOT environment" do - begin - original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] - ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact" + original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact" - new_test = DrivenBySeleniumWithChrome.new("x") + new_test = DrivenBySeleniumWithChrome.new("x") - assert_equal "artifact", new_test.send(:output_type) + assert_equal "artifact", new_test.send(:output_type) - Rails.stub :root, Pathname.getwd do - new_test.stub :passed?, false do - assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image) - end + Rails.stub :root, Pathname.getwd do + new_test.stub :passed?, false do + assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image) end - ensure - ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = original_output_type end + ensure + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = original_output_type end test "image path returns the absolute path from root" do diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 23cbf153ce..df4036a5a7 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -175,9 +175,9 @@ *Rui Onodera* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/actionview/CHANGELOG.md) for previous changes. diff --git a/actionview/actionview.gemspec b/actionview/actionview.gemspec index 5f1e746421..d8bd233ceb 100644 --- a/actionview/actionview.gemspec +++ b/actionview/actionview.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Rendering framework putting the V in MVC (part of Rails)." s.description = "Simple, battle-tested conventions and helpers for building web pages." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index 3d378dcb2f..c282505e13 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.to_s =~ /^1(\.0+)?$/ singular else plural || singular.pluralize(locale) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index af67ffa12d..554d223c0e 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -24,7 +24,7 @@ module ActionView registered_details << name Accessors::DEFAULT_PROCS[name] = block - Accessors.send :define_method, :"default_#{name}", &block + Accessors.define_method(:"default_#{name}", &block) Accessors.module_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{name} @details.fetch(:#{name}, []) diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb index 270be0a380..7d1a6767d7 100644 --- a/actionview/lib/action_view/template/handlers/erb.rb +++ b/actionview/lib/action_view/template/handlers/erb.rb @@ -17,8 +17,8 @@ module ActionView class_attribute :escape_ignore_list, default: ["text/plain"] [self, singleton_class].each do |base| - base.send(:alias_method, :escape_whitelist, :escape_ignore_list) - base.send(:alias_method, :escape_whitelist=, :escape_ignore_list=) + base.alias_method :escape_whitelist, :escape_ignore_list + base.alias_method :escape_whitelist=, :escape_ignore_list= base.deprecate( escape_whitelist: "use #escape_ignore_list instead", diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index a2d1474a94..4ccd3ae336 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -36,6 +36,7 @@ class FormOptionsHelperTest < ActionView::TestCase module FakeZones FakeZone = Struct.new(:name) do def to_s; name; end + def =~(_re); end end module ClassMethods diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 38469cbe3d..68e151f154 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -14,14 +14,12 @@ class LookupContextTest < ActiveSupport::TestCase end test "allows to override default_formats with ActionView::Base.default_formats" do - begin - formats = ActionView::Base.default_formats - ActionView::Base.default_formats = [:foo, :bar] + formats = ActionView::Base.default_formats + ActionView::Base.default_formats = [:foo, :bar] - assert_equal [:foo, :bar], ActionView::LookupContext.new([]).default_formats - ensure - ActionView::Base.default_formats = formats - end + assert_equal [:foo, :bar], ActionView::LookupContext.new([]).default_formats + ensure + ActionView::Base.default_formats = formats end test "process view paths on initialization" do diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 4acbe9a4e9..31253855d7 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -107,9 +107,9 @@ *Andrew White* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* * Add support to define custom argument serializers. diff --git a/activejob/Rakefile b/activejob/Rakefile index 2b9f89853f..037e84fca9 100644 --- a/activejob/Rakefile +++ b/activejob/Rakefile @@ -68,11 +68,9 @@ def run_without_aborting(tasks) errors = [] tasks.each do |task| - begin - Rake::Task[task].invoke - rescue Exception - errors << task - end + Rake::Task[task].invoke + rescue Exception + errors << task end abort "Errors running #{errors.join(', ')}" if errors.any? diff --git a/activejob/activejob.gemspec b/activejob/activejob.gemspec index 20b9d4ccdd..c3c0447d8e 100644 --- a/activejob/activejob.gemspec +++ b/activejob/activejob.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Job framework with pluggable queues." s.description = "Declare job classes that can be run by a variety of queuing backends." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index e0a71f4da8..f03780b91e 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/class/subclasses" -require "active_support/core_ext/hash/keys" module ActiveJob # Provides helper methods for testing Active Job diff --git a/activejob/test/cases/callbacks_test.rb b/activejob/test/cases/callbacks_test.rb index 267d58a7c8..895edb34a5 100644 --- a/activejob/test/cases/callbacks_test.rb +++ b/activejob/test/cases/callbacks_test.rb @@ -25,26 +25,22 @@ class CallbacksTest < ActiveSupport::TestCase end test "#enqueue returns false when before_enqueue aborts callback chain and return_false_on_aborted_enqueue = true" do - begin - prev = ActiveJob::Base.return_false_on_aborted_enqueue - ActiveJob::Base.return_false_on_aborted_enqueue = true - assert_equal false, AbortBeforeEnqueueJob.new.enqueue - ensure - ActiveJob::Base.return_false_on_aborted_enqueue = prev - end + prev = ActiveJob::Base.return_false_on_aborted_enqueue + ActiveJob::Base.return_false_on_aborted_enqueue = true + assert_equal false, AbortBeforeEnqueueJob.new.enqueue + ensure + ActiveJob::Base.return_false_on_aborted_enqueue = prev end test "#enqueue returns self when before_enqueue aborts callback chain and return_false_on_aborted_enqueue = false" do - begin - prev = ActiveJob::Base.return_false_on_aborted_enqueue - ActiveJob::Base.return_false_on_aborted_enqueue = false - job = AbortBeforeEnqueueJob.new - assert_deprecated do - assert_equal job, job.enqueue - end - ensure - ActiveJob::Base.return_false_on_aborted_enqueue = prev + prev = ActiveJob::Base.return_false_on_aborted_enqueue + ActiveJob::Base.return_false_on_aborted_enqueue = false + job = AbortBeforeEnqueueJob.new + assert_deprecated do + assert_equal job, job.enqueue end + ensure + ActiveJob::Base.return_false_on_aborted_enqueue = prev end test "#enqueue returns self when the job was enqueued" do diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb index b5328b8d6a..67327e0bbd 100644 --- a/activejob/test/cases/exceptions_test.rb +++ b/activejob/test/cases/exceptions_test.rb @@ -54,23 +54,19 @@ class ExceptionsTest < ActiveJob::TestCase test "failed retry job when exception kept occurring against defaults" do perform_enqueued_jobs do - begin - RetryJob.perform_later "DefaultsError", 6 - assert_equal "Raised DefaultsError for the 5th time", JobBuffer.last_value - rescue DefaultsError - pass - end + RetryJob.perform_later "DefaultsError", 6 + assert_equal "Raised DefaultsError for the 5th time", JobBuffer.last_value + rescue DefaultsError + pass end end test "failed retry job when exception kept occurring against higher limit" do perform_enqueued_jobs do - begin - RetryJob.perform_later "ShortWaitTenAttemptsError", 11 - assert_equal "Raised ShortWaitTenAttemptsError for the 10th time", JobBuffer.last_value - rescue ShortWaitTenAttemptsError - pass - end + RetryJob.perform_later "ShortWaitTenAttemptsError", 11 + assert_equal "Raised ShortWaitTenAttemptsError for the 10th time", JobBuffer.last_value + rescue ShortWaitTenAttemptsError + pass end end diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb index 48ef39aaca..6154ba301d 100644 --- a/activejob/test/cases/logging_test.rb +++ b/activejob/test/cases/logging_test.rb @@ -187,11 +187,9 @@ class LoggingTest < ActiveSupport::TestCase def test_retry_stopped_logging_without_block perform_enqueued_jobs do - begin - RetryJob.perform_later "DefaultsError", 6 - rescue DefaultsError - assert_match(/Stopped retrying RetryJob due to a DefaultsError, which reoccurred on \d+ attempts\./, @logger.messages) - end + RetryJob.perform_later "DefaultsError", 6 + rescue DefaultsError + assert_match(/Stopped retrying RetryJob due to a DefaultsError, which reoccurred on \d+ attempts\./, @logger.messages) end end diff --git a/activejob/test/cases/queuing_test.rb b/activejob/test/cases/queuing_test.rb index 0e843b7215..e7bad83400 100644 --- a/activejob/test/cases/queuing_test.rb +++ b/activejob/test/cases/queuing_test.rb @@ -20,12 +20,10 @@ class QueuingTest < ActiveSupport::TestCase end test "run queued job later" do - begin - result = HelloJob.set(wait_until: 1.second.ago).perform_later "Jamie" - assert result - rescue NotImplementedError - skip - end + result = HelloJob.set(wait_until: 1.second.ago).perform_later "Jamie" + assert result + rescue NotImplementedError + skip end test "job returned by enqueue has the arguments available" do @@ -34,11 +32,9 @@ class QueuingTest < ActiveSupport::TestCase end test "job returned by perform_at has the timestamp available" do - begin - job = HelloJob.set(wait_until: Time.utc(2014, 1, 1)).perform_later - assert_equal Time.utc(2014, 1, 1).to_f, job.scheduled_at - rescue NotImplementedError - skip - end + job = HelloJob.set(wait_until: Time.utc(2014, 1, 1)).perform_later + assert_equal Time.utc(2014, 1, 1).to_f, job.scheduled_at + rescue NotImplementedError + skip end end diff --git a/activejob/test/integration/queuing_test.rb b/activejob/test/integration/queuing_test.rb index 96253773c7..1fa68a8ad5 100644 --- a/activejob/test/integration/queuing_test.rb +++ b/activejob/test/integration/queuing_test.rb @@ -60,25 +60,21 @@ class QueuingTest < ActiveSupport::TestCase end test "should not run job enqueued in the future" do - begin - TestJob.set(wait: 10.minutes).perform_later @id - wait_for_jobs_to_finish_for(5.seconds) - assert_not job_executed - rescue NotImplementedError - skip - end + TestJob.set(wait: 10.minutes).perform_later @id + wait_for_jobs_to_finish_for(5.seconds) + assert_not job_executed + rescue NotImplementedError + skip end test "should run job enqueued in the future at the specified time" do - begin - TestJob.set(wait: 5.seconds).perform_later @id - wait_for_jobs_to_finish_for(2.seconds) - assert_not job_executed - wait_for_jobs_to_finish_for(10.seconds) - assert job_executed - rescue NotImplementedError - skip - end + TestJob.set(wait: 5.seconds).perform_later @id + wait_for_jobs_to_finish_for(2.seconds) + assert_not job_executed + wait_for_jobs_to_finish_for(10.seconds) + assert job_executed + rescue NotImplementedError + skip end test "should supply a provider_job_id when available for immediate jobs" do diff --git a/activejob/test/support/integration/test_case_helpers.rb b/activejob/test/support/integration/test_case_helpers.rb index 3d9b265b66..973ee07764 100644 --- a/activejob/test/support/integration/test_case_helpers.rb +++ b/activejob/test/support/integration/test_case_helpers.rb @@ -33,14 +33,12 @@ module TestCaseHelpers end def wait_for_jobs_to_finish_for(seconds = 60) - begin - Timeout.timeout(seconds) do - while !job_executed do - sleep 0.25 - end + Timeout.timeout(seconds) do + while !job_executed do + sleep 0.25 end - rescue Timeout::Error end + rescue Timeout::Error end def job_file(id) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 38c4d0c3c7..84567dcc18 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -54,9 +54,9 @@ *Martin Larochelle* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activemodel/CHANGELOG.md) for previous changes. diff --git a/activemodel/activemodel.gemspec b/activemodel/activemodel.gemspec index 493fca698a..4deb76814b 100644 --- a/activemodel/activemodel.gemspec +++ b/activemodel/activemodel.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "A toolkit for building modeling frameworks (part of Rails)." s.description = "A toolkit for building modeling frameworks like Active Record. Rich support for attributes, callbacks, validations, serialization, internationalization, and testing." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index d8352343e9..5c4670f393 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -507,8 +507,8 @@ module ActiveModel temp_method_name = "__temp__#{safe_name}#{'=' if writer}" attr_name_expr = "::ActiveModel::AttributeMethods::AttrNames::#{const_name}" yield temp_method_name, attr_name_expr - mod.send(:alias_method, method_name, temp_method_name) - mod.send(:undef_method, temp_method_name) + mod.alias_method method_name, temp_method_name + mod.undef_method temp_method_name end end end diff --git a/activemodel/lib/active_model/attribute_set.rb b/activemodel/lib/active_model/attribute_set.rb index a890ee3932..4679b33852 100644 --- a/activemodel/lib/active_model/attribute_set.rb +++ b/activemodel/lib/active_model/attribute_set.rb @@ -37,16 +37,8 @@ module ActiveModel attributes.each_key.select { |name| self[name].initialized? } end - if defined?(JRUBY_VERSION) - # This form is significantly faster on JRuby, and this is one of our biggest hotspots. - # https://github.com/jruby/jruby/pull/2562 - def fetch_value(name, &block) - self[name].value(&block) - end - else - def fetch_value(name) - self[name].value { |n| yield n if block_given? } - end + def fetch_value(name, &block) + self[name].value(&block) end def write_from_database(name, value) diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 7f14d102dd..f18f9a601a 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/array/extract_options" -require "active_support/core_ext/hash/keys" -require "active_support/core_ext/hash/except" module ActiveModel # == Active \Model \Validations diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index 9ff342d72b..6fd54270f2 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -55,8 +55,8 @@ module ActiveModel attr_readers = attributes.reject { |name| klass.attribute_method?(name) } attr_writers = attributes.reject { |name| klass.attribute_method?("#{name}=") } klass.define_attribute_methods - klass.send(:attr_reader, *attr_readers) - klass.send(:attr_writer, *attr_writers) + klass.attr_reader(*attr_readers) + klass.attr_writer(*attr_writers) end private diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index 1b5d5b09ab..b549755ba4 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -19,11 +19,11 @@ module ActiveModel private def setup!(klass) - klass.send(:attr_reader, *attributes.map do |attribute| + klass.attr_reader(*attributes.map do |attribute| :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation") end.compact) - klass.send(:attr_writer, *attributes.map do |attribute| + klass.attr_writer(*attributes.map do |attribute| :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation=") end.compact) end diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index e17c3ca7b3..94d53b8dd1 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -90,7 +90,7 @@ module ActiveModel # class MyValidator < ActiveModel::Validator # def initialize(options={}) # super - # options[:class].send :attr_accessor, :custom_attribute + # options[:class].attr_accessor :custom_attribute # end # end class Validator diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb index 0cfc6f4b6b..ebb6cc542d 100644 --- a/activemodel/test/cases/attribute_methods_test.rb +++ b/activemodel/test/cases/attribute_methods_test.rb @@ -106,14 +106,12 @@ class AttributeMethodsTest < ActiveModel::TestCase end test "#define_attribute_method generates attribute method" do - begin - ModelWithAttributes.define_attribute_method(:foo) + ModelWithAttributes.define_attribute_method(:foo) - assert_respond_to ModelWithAttributes.new, :foo - assert_equal "value of foo", ModelWithAttributes.new.foo - ensure - ModelWithAttributes.undefine_attribute_methods - end + assert_respond_to ModelWithAttributes.new, :foo + assert_equal "value of foo", ModelWithAttributes.new.foo + ensure + ModelWithAttributes.undefine_attribute_methods end test "#define_attribute_method does not generate attribute method if already defined in attribute module" do @@ -140,36 +138,30 @@ class AttributeMethodsTest < ActiveModel::TestCase end test "#define_attribute_method generates attribute method with invalid identifier characters" do - begin - ModelWithWeirdNamesAttributes.define_attribute_method(:'a?b') + ModelWithWeirdNamesAttributes.define_attribute_method(:'a?b') - assert_respond_to ModelWithWeirdNamesAttributes.new, :'a?b' - assert_equal "value of a?b", ModelWithWeirdNamesAttributes.new.send("a?b") - ensure - ModelWithWeirdNamesAttributes.undefine_attribute_methods - end + assert_respond_to ModelWithWeirdNamesAttributes.new, :'a?b' + assert_equal "value of a?b", ModelWithWeirdNamesAttributes.new.send("a?b") + ensure + ModelWithWeirdNamesAttributes.undefine_attribute_methods end test "#define_attribute_methods works passing multiple arguments" do - begin - ModelWithAttributes.define_attribute_methods(:foo, :baz) + ModelWithAttributes.define_attribute_methods(:foo, :baz) - assert_equal "value of foo", ModelWithAttributes.new.foo - assert_equal "value of baz", ModelWithAttributes.new.baz - ensure - ModelWithAttributes.undefine_attribute_methods - end + assert_equal "value of foo", ModelWithAttributes.new.foo + assert_equal "value of baz", ModelWithAttributes.new.baz + ensure + ModelWithAttributes.undefine_attribute_methods end test "#define_attribute_methods generates attribute methods" do - begin - ModelWithAttributes.define_attribute_methods(:foo) + ModelWithAttributes.define_attribute_methods(:foo) - assert_respond_to ModelWithAttributes.new, :foo - assert_equal "value of foo", ModelWithAttributes.new.foo - ensure - ModelWithAttributes.undefine_attribute_methods - end + assert_respond_to ModelWithAttributes.new, :foo + assert_equal "value of foo", ModelWithAttributes.new.foo + ensure + ModelWithAttributes.undefine_attribute_methods end test "#alias_attribute generates attribute_aliases lookup hash" do @@ -182,38 +174,32 @@ class AttributeMethodsTest < ActiveModel::TestCase end test "#define_attribute_methods generates attribute methods with spaces in their names" do - begin - ModelWithAttributesWithSpaces.define_attribute_methods(:'foo bar') + ModelWithAttributesWithSpaces.define_attribute_methods(:'foo bar') - assert_respond_to ModelWithAttributesWithSpaces.new, :'foo bar' - assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.send(:'foo bar') - ensure - ModelWithAttributesWithSpaces.undefine_attribute_methods - end + assert_respond_to ModelWithAttributesWithSpaces.new, :'foo bar' + assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.send(:'foo bar') + ensure + ModelWithAttributesWithSpaces.undefine_attribute_methods end test "#alias_attribute works with attributes with spaces in their names" do - begin - ModelWithAttributesWithSpaces.define_attribute_methods(:'foo bar') - ModelWithAttributesWithSpaces.alias_attribute(:'foo_bar', :'foo bar') + ModelWithAttributesWithSpaces.define_attribute_methods(:'foo bar') + ModelWithAttributesWithSpaces.alias_attribute(:'foo_bar', :'foo bar') - assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.foo_bar - ensure - ModelWithAttributesWithSpaces.undefine_attribute_methods - end + assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.foo_bar + ensure + ModelWithAttributesWithSpaces.undefine_attribute_methods end test "#alias_attribute works with attributes named as a ruby keyword" do - begin - ModelWithRubyKeywordNamedAttributes.define_attribute_methods([:begin, :end]) - ModelWithRubyKeywordNamedAttributes.alias_attribute(:from, :begin) - ModelWithRubyKeywordNamedAttributes.alias_attribute(:to, :end) - - assert_equal "value of begin", ModelWithRubyKeywordNamedAttributes.new.from - assert_equal "value of end", ModelWithRubyKeywordNamedAttributes.new.to - ensure - ModelWithRubyKeywordNamedAttributes.undefine_attribute_methods - end + ModelWithRubyKeywordNamedAttributes.define_attribute_methods([:begin, :end]) + ModelWithRubyKeywordNamedAttributes.alias_attribute(:from, :begin) + ModelWithRubyKeywordNamedAttributes.alias_attribute(:to, :end) + + assert_equal "value of begin", ModelWithRubyKeywordNamedAttributes.new.from + assert_equal "value of end", ModelWithRubyKeywordNamedAttributes.new.to + ensure + ModelWithRubyKeywordNamedAttributes.undefine_attribute_methods end test "#undefine_attribute_methods removes attribute methods" do diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 9ef1148be8..bbf443290b 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -206,16 +206,14 @@ class SecurePasswordTest < ActiveModel::TestCase end test "Password digest cost honors bcrypt cost attribute when min_cost is false" do - begin - original_bcrypt_cost = BCrypt::Engine.cost - ActiveModel::SecurePassword.min_cost = false - BCrypt::Engine.cost = 5 - - @user.password = "secret" - assert_equal BCrypt::Engine.cost, @user.password_digest.cost - ensure - BCrypt::Engine.cost = original_bcrypt_cost - end + original_bcrypt_cost = BCrypt::Engine.cost + ActiveModel::SecurePassword.min_cost = false + BCrypt::Engine.cost = 5 + + @user.password = "secret" + assert_equal BCrypt::Engine.cost, @user.password_digest.cost + ensure + BCrypt::Engine.cost = original_bcrypt_cost end test "Password digest cost can be set to bcrypt min cost to speed up tests" do diff --git a/activemodel/test/cases/serializers/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb index 625e0a427a..84efc8de0d 100644 --- a/activemodel/test/cases/serializers/json_serialization_test.rb +++ b/activemodel/test/cases/serializers/json_serialization_test.rb @@ -26,20 +26,18 @@ class JsonSerializationTest < ActiveModel::TestCase end test "should include root in json if include_root_in_json is true" do - begin - original_include_root_in_json = Contact.include_root_in_json - Contact.include_root_in_json = true - json = @contact.to_json - - assert_match %r{^\{"contact":\{}, json - assert_match %r{"name":"Konata Izumi"}, json - assert_match %r{"age":16}, json - assert_includes json, %("created_at":#{ActiveSupport::JSON.encode(Time.utc(2006, 8, 1))}) - assert_match %r{"awesome":true}, json - assert_match %r{"preferences":\{"shows":"anime"\}}, json - ensure - Contact.include_root_in_json = original_include_root_in_json - end + original_include_root_in_json = Contact.include_root_in_json + Contact.include_root_in_json = true + json = @contact.to_json + + assert_match %r{^\{"contact":\{}, json + assert_match %r{"name":"Konata Izumi"}, json + assert_match %r{"age":16}, json + assert_includes json, %("created_at":#{ActiveSupport::JSON.encode(Time.utc(2006, 8, 1))}) + assert_match %r{"awesome":true}, json + assert_match %r{"preferences":\{"shows":"anime"\}}, json + ensure + Contact.include_root_in_json = original_include_root_in_json end test "should include root in json (option) even if the default is set to false" do @@ -134,19 +132,17 @@ class JsonSerializationTest < ActiveModel::TestCase 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 - Contact.include_root_in_json = true - json = @contact.as_json - - 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).as_json, json["contact"][field] - end - ensure - Contact.include_root_in_json = original_include_root_in_json + original_include_root_in_json = Contact.include_root_in_json + Contact.include_root_in_json = true + json = @contact.as_json + + 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).as_json, json["contact"][field] end + ensure + Contact.include_root_in_json = original_include_root_in_json end test "from_json should work without a root (class attribute)" do diff --git a/activemodel/test/cases/validations/confirmation_validation_test.rb b/activemodel/test/cases/validations/confirmation_validation_test.rb index 8603a8ac5c..7bf15e4bee 100644 --- a/activemodel/test/cases/validations/confirmation_validation_test.rb +++ b/activemodel/test/cases/validations/confirmation_validation_test.rb @@ -66,24 +66,22 @@ class ConfirmationValidationTest < ActiveModel::TestCase end def test_title_confirmation_with_i18n_attribute - begin - @old_load_path, @old_backend = I18n.load_path.dup, I18n.backend - I18n.load_path.clear - I18n.backend = I18n::Backend::Simple.new - I18n.backend.store_translations("en", - errors: { messages: { confirmation: "doesn't match %{attribute}" } }, - activemodel: { attributes: { topic: { title: "Test Title" } } }) - - Topic.validates_confirmation_of(:title) - - t = Topic.new("title" => "We should be confirmed", "title_confirmation" => "") - assert_predicate t, :invalid? - assert_equal ["doesn't match Test Title"], t.errors[:title_confirmation] - ensure - I18n.load_path.replace @old_load_path - I18n.backend = @old_backend - I18n.backend.reload! - end + @old_load_path, @old_backend = I18n.load_path.dup, I18n.backend + I18n.load_path.clear + I18n.backend = I18n::Backend::Simple.new + I18n.backend.store_translations("en", + errors: { messages: { confirmation: "doesn't match %{attribute}" } }, + activemodel: { attributes: { topic: { title: "Test Title" } } }) + + Topic.validates_confirmation_of(:title) + + t = Topic.new("title" => "We should be confirmed", "title_confirmation" => "") + assert_predicate t, :invalid? + assert_equal ["doesn't match Test Title"], t.errors[:title_confirmation] + ensure + I18n.load_path.replace @old_load_path + I18n.backend = @old_backend + I18n.backend.reload! end test "does not override confirmation reader if present" do diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index 774a2cde74..37e10f783c 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -427,7 +427,7 @@ class LengthValidationTest < ActiveModel::TestCase end def test_validates_length_of_using_proc_as_maximum_with_model_method - Topic.send(:define_method, :max_title_length, lambda { 5 }) + Topic.define_method(:max_title_length) { 5 } Topic.validates_length_of :title, maximum: Proc.new(&:max_title_length) t = Topic.new("title" => "valid", "content" => "whatever") diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index ca22e38c2d..eb4b02df93 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -66,7 +66,7 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_of_with_integer_only_and_proc_as_value - Topic.send(:define_method, :allow_only_integers?, lambda { false }) + Topic.define_method(:allow_only_integers?) { false } Topic.validates_numericality_of :approved, only_integer: Proc.new(&:allow_only_integers?) invalid!(NIL + BLANK + JUNK) @@ -214,23 +214,23 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_with_proc - Topic.send(:define_method, :min_approved, lambda { 5 }) + Topic.define_method(:min_approved) { 5 } Topic.validates_numericality_of :approved, greater_than_or_equal_to: Proc.new(&:min_approved) invalid!([3, 4]) valid!([5, 6]) ensure - Topic.send(:remove_method, :min_approved) + Topic.remove_method :min_approved end def test_validates_numericality_with_symbol - Topic.send(:define_method, :max_approved, lambda { 5 }) + Topic.define_method(:max_approved) { 5 } Topic.validates_numericality_of :approved, less_than_or_equal_to: :max_approved invalid!([6]) valid!([4, 5]) ensure - Topic.send(:remove_method, :max_approved) + Topic.remove_method :max_approved end def test_validates_numericality_with_numeric_message diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6f2b49c428..1aa756cc7e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* MySQL: `ROW_FORMAT=DYNAMIC` create table option by default. + + Since MySQL 5.7.9, the `innodb_default_row_format` option defines the default row + format for InnoDB tables. The default setting is `DYNAMIC`. + The row format is required for indexing on `varchar(255)` with `utf8mb4` columns. + + *Ryuta Kamizono* + * Fix join table column quoting with SQLite. *Gannon McGibbon* @@ -466,9 +474,9 @@ *Tan Huynh*, *Yukio Mizuta* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* * Deprecate `update_attributes`/`!` in favor of `update`/`!`. diff --git a/activerecord/Rakefile b/activerecord/Rakefile index fae56a51bb..013e81c959 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -9,11 +9,9 @@ def run_without_aborting(*tasks) errors = [] tasks.each do |task| - begin - Rake::Task[task].invoke - rescue Exception - errors << task - end + Rake::Task[task].invoke + rescue Exception + errors << task end abort "Errors running #{errors.join(', ')}" if errors.any? diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index bcdd82052c..1e198c3a55 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Object-relational mapper framework (part of Rails)." s.description = "Databases on Rails. Build a persistent domain model by mapping database tables to Ruby classes. Strong conventions for associations, validations, aggregations, migrations, and testing come baked-in." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index b6f0e18764..929045f29b 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -45,16 +45,14 @@ module ActiveRecord def execute_callstack_for_multiparameter_attributes(callstack) errors = [] callstack.each do |name, values_with_empty_parameters| - begin - if values_with_empty_parameters.each_value.all?(&:nil?) - values = nil - else - values = values_with_empty_parameters - end - send("#{name}=", values) - rescue => ex - errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name} (#{ex.message})", ex, name) + if values_with_empty_parameters.each_value.all?(&:nil?) + values = nil + else + values = values_with_empty_parameters end + send("#{name}=", values) + rescue => ex + errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name} (#{ex.message})", ex, name) end unless errors.empty? error_descriptions = errors.map(&:message).join(",") diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 6e1275e990..ffac5313ad 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -42,16 +42,8 @@ module ActiveRecord # This method exists to avoid the expensive primary_key check internally, without # breaking compatibility with the read_attribute API - if defined?(JRUBY_VERSION) - # This form is significantly faster on JRuby, and this is one of our biggest hotspots. - # https://github.com/jruby/jruby/pull/2562 - def _read_attribute(attr_name, &block) # :nodoc: - @attributes.fetch_value(attr_name.to_s, &block) - end - else - def _read_attribute(attr_name) # :nodoc: - @attributes.fetch_value(attr_name.to_s) { |n| yield n if block_given? } - end + def _read_attribute(attr_name, &block) # :nodoc + @attributes.fetch_value(attr_name.to_s, &block) end alias :attribute :_read_attribute diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 0f2b1e85ff..718910b090 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -283,26 +283,24 @@ module ActiveRecord def within_new_transaction(options = {}) @connection.lock.synchronize do - begin - transaction = begin_transaction options - yield - rescue Exception => error - if transaction + transaction = begin_transaction options + yield + rescue Exception => error + if transaction + rollback_transaction + after_failure_actions(transaction, error) + end + raise + ensure + if !error && transaction + if Thread.current.status == "aborting" rollback_transaction - after_failure_actions(transaction, error) - end - raise - ensure - unless error - if Thread.current.status == "aborting" - rollback_transaction if transaction - else - begin - commit_transaction if transaction - rescue Exception - rollback_transaction(transaction) unless transaction.state.completed? - raise - end + else + begin + commit_transaction + rescue Exception + rollback_transaction(transaction) unless transaction.state.completed? + raise end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index f7d75e6748..1243236c09 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -631,13 +631,11 @@ module ActiveRecord statement_name: statement_name, connection_id: object_id, connection: self) do - begin - @lock.synchronize do - yield - end - rescue => e - raise translate_exception_class(e, sql, binds) + @lock.synchronize do + yield end + rescue => e + raise translate_exception_class(e, sql, binds) end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 66db4e4149..47b5c4b9ec 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -77,6 +77,10 @@ module ActiveRecord super end + def create_table(table_name, options: default_row_format, **) + super + end + def internal_string_options_for_primary_key super.tap do |options| if !row_format_dynamic_by_default? && CHARSETS_OF_4BYTES_MAXLEN.include?(charset) @@ -104,6 +108,20 @@ module ActiveRecord end end + def default_row_format + return if row_format_dynamic_by_default? + + unless defined?(@default_row_format) + if query_value("SELECT @@innodb_file_per_table = 1 AND @@innodb_file_format = 'Barracuda'") == 1 + @default_row_format = "ROW_FORMAT=DYNAMIC" + else + @default_row_format = nil + end + end + + @default_row_format + end + def schema_creation MySQL::SchemaCreation.new(self) end diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 30cb0a27e7..11aed6c002 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -124,15 +124,13 @@ module ActiveRecord end def build_db_config_from_string(env_name, spec_name, config) - begin - url = config - uri = URI.parse(url) - if uri.try(:scheme) - ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url) - end - rescue URI::InvalidURIError - ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config) + url = config + uri = URI.parse(url) + if uri.try(:scheme) + ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url) end + rescue URI::InvalidURIError + ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config) end def build_db_config_from_hash(env_name, spec_name, config) diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index d7cb7691e0..e6dba66a08 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -158,7 +158,7 @@ module ActiveRecord # def self.statuses() statuses end detect_enum_conflict!(name, name.pluralize, true) - singleton_class.send(:define_method, name.pluralize) { enum_values } + singleton_class.define_method(name.pluralize) { enum_values } defined_enums[name] = enum_values detect_enum_conflict!(name, name) diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 350b044a3e..327121a2a2 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -519,11 +519,9 @@ module ActiveRecord def instantiate_fixtures(object, fixture_set, load_instances = true) return unless load_instances fixture_set.each do |fixture_name, fixture| - begin - object.instance_variable_set "@#{fixture_name}", fixture.find - rescue FixtureClassNotFound - nil - end + object.instance_variable_set "@#{fixture_name}", fixture.find + rescue FixtureClassNotFound + nil end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 475baa7559..d24324ecce 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -191,11 +191,9 @@ db_namespace = namespace :db do # desc "Retrieves the collation for the current environment's database" task collation: :load_config do - begin - puts ActiveRecord::Tasks::DatabaseTasks.collation_current - rescue NoMethodError - $stderr.puts "Sorry, your database adapter is not supported yet. Feel free to submit a patch." - end + puts ActiveRecord::Tasks::DatabaseTasks.collation_current + rescue NoMethodError + $stderr.puts "Sorry, your database adapter is not supported yet. Feel free to submit a patch." end desc "Retrieves the current schema version number" @@ -361,17 +359,15 @@ db_namespace = namespace :db do # desc "Recreate the test database from an existent schema.rb file" task load_schema: %w(db:test:purge) do - begin - should_reconnect = ActiveRecord::Base.connection_pool.active_connection? - ActiveRecord::Schema.verbose = false - ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config| - filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby) - ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, :ruby, filename, "test") - end - ensure - if should_reconnect - ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations.default_hash(ActiveRecord::Tasks::DatabaseTasks.env)) - end + should_reconnect = ActiveRecord::Base.connection_pool.active_connection? + ActiveRecord::Schema.verbose = false + ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config| + filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby) + ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, :ruby, filename, "test") + end + ensure + if should_reconnect + ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations.default_hash(ActiveRecord::Tasks::DatabaseTasks.env)) end end diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 383dc1bf4b..6f67dd3784 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -54,7 +54,7 @@ module ActiveRecord end RUBY else - generated_relation_methods.send(:define_method, method) do |*args, &block| + generated_relation_methods.define_method(method) do |*args, &block| scoping { klass.public_send(method, *args, &block) } end end diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index d5cc5db97e..5278eb29a9 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -179,13 +179,13 @@ module ActiveRecord extension = Module.new(&block) if block if body.respond_to?(:to_proc) - singleton_class.send(:define_method, name) do |*args| + singleton_class.define_method(name) do |*args| scope = all._exec_scope(*args, &body) scope = scope.extending(extension) if extension scope end else - singleton_class.send(:define_method, name) do |*args| + singleton_class.define_method(name) do |*args| scope = body.call(*args) || all scope = scope.extending(extension) if extension scope diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 3d31533ec8..66e594d771 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -132,19 +132,17 @@ module ActiveRecord end def test_not_specifying_database_name_for_cross_database_selects - begin - assert_nothing_raised do - ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations["arunit"].except(:database)) - - config = ARTest.connection_config - ActiveRecord::Base.connection.execute( - "SELECT #{config['arunit']['database']}.pirates.*, #{config['arunit2']['database']}.courses.* " \ - "FROM #{config['arunit']['database']}.pirates, #{config['arunit2']['database']}.courses" - ) - end - ensure - ActiveRecord::Base.establish_connection :arunit + assert_nothing_raised do + ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations["arunit"].except(:database)) + + config = ARTest.connection_config + ActiveRecord::Base.connection.execute( + "SELECT #{config['arunit']['database']}.pirates.*, #{config['arunit2']['database']}.courses.* " \ + "FROM #{config['arunit']['database']}.pirates, #{config['arunit2']['database']}.courses" + ) end + ensure + ActiveRecord::Base.establish_connection :arunit end end diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index 261fee13eb..2d71ee2f15 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -7,6 +7,7 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase include ConnectionHelper def setup + ActiveRecord::Base.connection.send(:default_row_format) ActiveRecord::Base.connection.singleton_class.class_eval do alias_method :execute_without_stub, :execute def execute(sql, name = nil) sql end @@ -68,18 +69,18 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase def (ActiveRecord::Base.connection).data_source_exists?(*); false; end %w(SPATIAL FULLTEXT UNIQUE).each do |type| - expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name` (`last_name`))" + expected = /\ACREATE TABLE `people` \(#{type} INDEX `index_people_on_last_name` \(`last_name`\)\)/ actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t| t.index :last_name, type: type end - assert_equal expected, actual + assert_match expected, actual end - expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10)))" + expected = /\ACREATE TABLE `people` \( INDEX `index_people_on_last_name` USING btree \(`last_name`\(10\)\)\)/ actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t| t.index :last_name, length: 10, using: :btree end - assert_equal expected, actual + assert_match expected, actual end def test_index_in_bulk_change @@ -106,7 +107,13 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase end def test_create_mysql_database_with_encoding - assert_equal "CREATE DATABASE `matt` DEFAULT CHARACTER SET `utf8mb4`", create_database(:matt) + if row_format_dynamic_by_default? + assert_equal "CREATE DATABASE `matt` DEFAULT CHARACTER SET `utf8mb4`", create_database(:matt) + else + error = assert_raises(RuntimeError) { create_database(:matt) } + expected = "Configure a supported :charset and ensure innodb_large_prefix is enabled to support indexes on varchar(255) string columns." + assert_equal expected, error.message + end assert_equal "CREATE DATABASE `aimonetti` DEFAULT CHARACTER SET `latin1`", create_database(:aimonetti, charset: "latin1") assert_equal "CREATE DATABASE `matt_aimonetti` DEFAULT COLLATE `utf8mb4_bin`", create_database(:matt_aimonetti, collation: "utf8mb4_bin") end @@ -130,29 +137,25 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase def test_add_timestamps with_real_execute do - begin - ActiveRecord::Base.connection.create_table :delete_me - ActiveRecord::Base.connection.add_timestamps :delete_me, null: true - assert column_present?("delete_me", "updated_at", "datetime") - assert column_present?("delete_me", "created_at", "datetime") - ensure - ActiveRecord::Base.connection.drop_table :delete_me rescue nil - end + ActiveRecord::Base.connection.create_table :delete_me + ActiveRecord::Base.connection.add_timestamps :delete_me, null: true + assert column_present?("delete_me", "updated_at", "datetime") + assert column_present?("delete_me", "created_at", "datetime") + ensure + ActiveRecord::Base.connection.drop_table :delete_me rescue nil end end def test_remove_timestamps with_real_execute do - begin - ActiveRecord::Base.connection.create_table :delete_me do |t| - t.timestamps null: true - end - ActiveRecord::Base.connection.remove_timestamps :delete_me, null: true - assert_not column_present?("delete_me", "updated_at", "datetime") - assert_not column_present?("delete_me", "created_at", "datetime") - ensure - ActiveRecord::Base.connection.drop_table :delete_me rescue nil + ActiveRecord::Base.connection.create_table :delete_me do |t| + t.timestamps null: true end + ActiveRecord::Base.connection.remove_timestamps :delete_me, null: true + assert_not column_present?("delete_me", "updated_at", "datetime") + assert_not column_present?("delete_me", "created_at", "datetime") + ensure + ActiveRecord::Base.connection.drop_table :delete_me rescue nil end end @@ -163,12 +166,12 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase [:temp], returns: false ) do - expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip` (`zip`)) AS SELECT id, name, zip FROM a_really_complicated_query" + expected = /\ACREATE TEMPORARY TABLE `temp` \( INDEX `index_temp_on_zip` \(`zip`\)\)(?: ROW_FORMAT=DYNAMIC)? AS SELECT id, name, zip FROM a_really_complicated_query/ actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t| t.index :zip end - assert_equal expected, actual + assert_match expected, actual end end diff --git a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb index afd422881b..62efaf3bfe 100644 --- a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb @@ -29,7 +29,7 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase def test_add_index # add_index calls index_name_exists? which can't work since execute is stubbed - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.send(:define_method, :index_name_exists?) { |*| false } + ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.define_method(:index_name_exists?) { |*| false } expected = %(CREATE UNIQUE INDEX "index_people_on_last_name" ON "people" ("last_name") WHERE state = 'active') assert_equal expected, add_index(:people, :last_name, unique: true, where: "state = 'active'") @@ -74,12 +74,12 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase add_index(:people, :last_name, algorithm: :copy) end - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.send :remove_method, :index_name_exists? + ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.remove_method :index_name_exists? end def test_remove_index # remove_index calls index_name_for_remove which can't work since execute is stubbed - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.send(:define_method, :index_name_for_remove) do |*| + ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.define_method(:index_name_for_remove) do |*| "index_people_on_last_name" end @@ -90,7 +90,7 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase add_index(:people, :last_name, algorithm: :copy) end - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.send :remove_method, :index_name_for_remove + ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.remove_method :index_name_for_remove end def test_remove_index_when_name_is_specified diff --git a/activerecord/test/cases/adapters/postgresql/infinity_test.rb b/activerecord/test/cases/adapters/postgresql/infinity_test.rb index 5e56ce8427..b1bf06d9e9 100644 --- a/activerecord/test/cases/adapters/postgresql/infinity_test.rb +++ b/activerecord/test/cases/adapters/postgresql/infinity_test.rb @@ -71,17 +71,15 @@ class PostgresqlInfinityTest < ActiveRecord::PostgreSQLTestCase end test "assigning 'infinity' on a datetime column with TZ aware attributes" do - begin - in_time_zone "Pacific Time (US & Canada)" do - record = PostgresqlInfinity.create!(datetime: "infinity") - assert_equal Float::INFINITY, record.datetime - assert_equal record.datetime, record.reload.datetime - end - ensure - # setting time_zone_aware_attributes causes the types to change. - # There is no way to do this automatically since it can be set on a superclass - PostgresqlInfinity.reset_column_information + in_time_zone "Pacific Time (US & Canada)" do + record = PostgresqlInfinity.create!(datetime: "infinity") + assert_equal Float::INFINITY, record.datetime + assert_equal record.datetime, record.reload.datetime end + ensure + # setting time_zone_aware_attributes causes the types to change. + # There is no way to do this automatically since it can be set on a superclass + PostgresqlInfinity.reset_column_information end test "where clause with infinite range on a datetime column" do diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 931a9bd0be..336cec30ca 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -108,23 +108,19 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase end def test_create_schema - begin - @connection.create_schema "test_schema3" - assert @connection.schema_names.include? "test_schema3" - ensure - @connection.drop_schema "test_schema3" - end + @connection.create_schema "test_schema3" + assert @connection.schema_names.include? "test_schema3" + ensure + @connection.drop_schema "test_schema3" end def test_raise_create_schema_with_existing_schema - begin + @connection.create_schema "test_schema3" + assert_raises(ActiveRecord::StatementInvalid) do @connection.create_schema "test_schema3" - assert_raises(ActiveRecord::StatementInvalid) do - @connection.create_schema "test_schema3" - end - ensure - @connection.drop_schema "test_schema3" end + ensure + @connection.drop_schema "test_schema3" end def test_drop_schema diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb index d70486605f..cfc9853aba 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb @@ -8,17 +8,15 @@ module ActiveRecord class SQLite3CreateFolder < ActiveRecord::SQLite3TestCase def test_sqlite_creates_directory Dir.mktmpdir do |dir| - begin - dir = Pathname.new(dir) - @conn = Base.sqlite3_connection database: dir.join("db/foo.sqlite3"), - adapter: "sqlite3", - timeout: 100 + dir = Pathname.new(dir) + @conn = Base.sqlite3_connection database: dir.join("db/foo.sqlite3"), + adapter: "sqlite3", + timeout: 100 - assert Dir.exist? dir.join("db") - assert File.exist? dir.join("db/foo.sqlite3") - ensure - @conn.disconnect! if @conn - end + assert Dir.exist? dir.join("db") + assert File.exist? dir.join("db/foo.sqlite3") + ensure + @conn.disconnect! if @conn end end end 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 515eb65d37..fe8bdd03ba 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 @@ -1007,16 +1007,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_has_and_belongs_to_many_while_partial_writes_false - begin - original_partial_writes = ActiveRecord::Base.partial_writes - ActiveRecord::Base.partial_writes = false - developer = Developer.new(name: "Mehmet Emin İNAÇ") - developer.projects << Project.new(name: "Bounty") - - assert developer.save - ensure - ActiveRecord::Base.partial_writes = original_partial_writes - end + original_partial_writes = ActiveRecord::Base.partial_writes + ActiveRecord::Base.partial_writes = false + developer = Developer.new(name: "Mehmet Emin İNAÇ") + developer.projects << Project.new(name: "Bounty") + + assert developer.save + ensure + ActiveRecord::Base.partial_writes = original_partial_writes end def test_has_and_belongs_to_many_with_belongs_to diff --git a/activerecord/test/cases/associations/required_test.rb b/activerecord/test/cases/associations/required_test.rb index 65a3bb5efe..c7a78e6bc4 100644 --- a/activerecord/test/cases/associations/required_test.rb +++ b/activerecord/test/cases/associations/required_test.rb @@ -25,20 +25,18 @@ class RequiredAssociationsTest < ActiveRecord::TestCase end test "belongs_to associations can be optional by default" do - begin - original_value = ActiveRecord::Base.belongs_to_required_by_default - ActiveRecord::Base.belongs_to_required_by_default = false + original_value = ActiveRecord::Base.belongs_to_required_by_default + ActiveRecord::Base.belongs_to_required_by_default = false - model = subclass_of(Child) do - belongs_to :parent, inverse_of: false, - class_name: "RequiredAssociationsTest::Parent" - end - - assert model.new.save - assert model.new(parent: Parent.new).save - ensure - ActiveRecord::Base.belongs_to_required_by_default = original_value + model = subclass_of(Child) do + belongs_to :parent, inverse_of: false, + class_name: "RequiredAssociationsTest::Parent" end + + assert model.new.save + assert model.new(parent: Parent.new).save + ensure + ActiveRecord::Base.belongs_to_required_by_default = original_value end test "required belongs_to associations have presence validated" do @@ -56,24 +54,22 @@ class RequiredAssociationsTest < ActiveRecord::TestCase end test "belongs_to associations can be required by default" do - begin - original_value = ActiveRecord::Base.belongs_to_required_by_default - ActiveRecord::Base.belongs_to_required_by_default = true + original_value = ActiveRecord::Base.belongs_to_required_by_default + ActiveRecord::Base.belongs_to_required_by_default = true - model = subclass_of(Child) do - belongs_to :parent, inverse_of: false, - class_name: "RequiredAssociationsTest::Parent" - end + model = subclass_of(Child) do + belongs_to :parent, inverse_of: false, + class_name: "RequiredAssociationsTest::Parent" + end - record = model.new - assert_not record.save - assert_equal ["Parent must exist"], record.errors.full_messages + record = model.new + assert_not record.save + assert_equal ["Parent must exist"], record.errors.full_messages - record.parent = Parent.new - assert record.save - ensure - ActiveRecord::Base.belongs_to_required_by_default = original_value - end + record.parent = Parent.new + assert record.save + ensure + ActiveRecord::Base.belongs_to_required_by_default = original_value end test "has_one associations are not required by default" do diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 0ea3fb86a6..4d6a112af5 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -21,7 +21,7 @@ class CallbackDeveloper < ActiveRecord::Base def callback_object(callback_method) klass = Class.new - klass.send(:define_method, callback_method) do |model| + klass.define_method(callback_method) do |model| model.history << [callback_method, :object] end klass.new diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 633d56e479..a15ad9a45b 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -567,23 +567,21 @@ module ActiveRecord def test_disconnect_and_clear_reloadable_connections_attempt_to_wait_for_threads_to_return_their_conns [:disconnect, :disconnect!, :clear_reloadable_connections, :clear_reloadable_connections!].each do |group_action_method| - begin - thread = timed_join_result = nil - @pool.with_connection do |connection| - thread = Thread.new { @pool.send(group_action_method) } - - # give the other `thread` some time to get stuck in `group_action_method` - timed_join_result = thread.join(0.3) - # thread.join # => `nil` means the other thread hasn't finished running and is still waiting for us to - # release our connection - assert_nil timed_join_result - - # assert that since this is within default timeout our connection hasn't been forcefully taken away from us - assert_predicate @pool, :active_connection? - end - ensure - thread.join if thread && !timed_join_result # clean up the other thread + thread = timed_join_result = nil + @pool.with_connection do |connection| + thread = Thread.new { @pool.send(group_action_method) } + + # give the other `thread` some time to get stuck in `group_action_method` + timed_join_result = thread.join(0.3) + # thread.join # => `nil` means the other thread hasn't finished running and is still waiting for us to + # release our connection + assert_nil timed_join_result + + # assert that since this is within default timeout our connection hasn't been forcefully taken away from us + assert_predicate @pool, :active_connection? end + ensure + thread.join if thread && !timed_join_result # clean up the other thread end end diff --git a/activerecord/test/cases/errors_test.rb b/activerecord/test/cases/errors_test.rb index b90e6a66c5..0d2be944b5 100644 --- a/activerecord/test/cases/errors_test.rb +++ b/activerecord/test/cases/errors_test.rb @@ -8,11 +8,9 @@ class ErrorsTest < ActiveRecord::TestCase error_klasses = ObjectSpace.each_object(Class).select { |klass| klass < base } (error_klasses - [ActiveRecord::AmbiguousSourceReflectionForThroughAssociation]).each do |error_klass| - begin - error_klass.new.inspect - rescue ArgumentError - raise "Instance of #{error_klass} can't be initialized with no arguments" - end + error_klass.new.inspect + rescue ArgumentError + raise "Instance of #{error_klass} can't be initialized with no arguments" end end end diff --git a/activerecord/test/cases/filter_attributes_test.rb b/activerecord/test/cases/filter_attributes_test.rb index 47161a547a..2f4c9b0ef7 100644 --- a/activerecord/test/cases/filter_attributes_test.rb +++ b/activerecord/test/cases/filter_attributes_test.rb @@ -90,15 +90,13 @@ class FilterAttributesTest < ActiveRecord::TestCase end test "filter_attributes should handle [FILTERED] value properly" do - begin - User.filter_attributes = ["auth"] - user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]") + User.filter_attributes = ["auth"] + user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]") - assert_includes user.inspect, "auth_token: [FILTERED]" - assert_includes user.inspect, 'token: "[FILTERED]"' - ensure - User.remove_instance_variable(:@filter_attributes) - end + assert_includes user.inspect, "auth_token: [FILTERED]" + assert_includes user.inspect, 'token: "[FILTERED]"' + ensure + User.remove_instance_variable(:@filter_attributes) end test "filter_attributes on pretty_print" do @@ -121,16 +119,14 @@ class FilterAttributesTest < ActiveRecord::TestCase end test "filter_attributes on pretty_print should handle [FILTERED] value properly" do - begin - User.filter_attributes = ["auth"] - user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]") - actual = "".dup - PP.pp(user, StringIO.new(actual)) + User.filter_attributes = ["auth"] + user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]") + actual = "".dup + PP.pp(user, StringIO.new(actual)) - assert_includes actual, "auth_token: [FILTERED]" - assert_includes actual, 'token: "[FILTERED]"' - ensure - User.remove_instance_variable(:@filter_attributes) - end + assert_includes actual, "auth_token: [FILTERED]" + assert_includes actual, 'token: "[FILTERED]"' + ensure + User.remove_instance_variable(:@filter_attributes) end end diff --git a/activerecord/test/cases/finder_respond_to_test.rb b/activerecord/test/cases/finder_respond_to_test.rb index e0acd30c22..66413a98e4 100644 --- a/activerecord/test/cases/finder_respond_to_test.rb +++ b/activerecord/test/cases/finder_respond_to_test.rb @@ -12,10 +12,10 @@ class FinderRespondToTest < ActiveRecord::TestCase end def test_should_preserve_normal_respond_to_behaviour_and_respond_to_newly_added_method - class << Topic; self; end.send(:define_method, :method_added_for_finder_respond_to_test) { } + Topic.singleton_class.define_method(:method_added_for_finder_respond_to_test) { } assert_respond_to Topic, :method_added_for_finder_respond_to_test ensure - class << Topic; self; end.send(:remove_method, :method_added_for_finder_respond_to_test) + Topic.singleton_class.remove_method :method_added_for_finder_respond_to_test end def test_should_preserve_normal_respond_to_behaviour_and_respond_to_standard_object_method @@ -56,6 +56,6 @@ class FinderRespondToTest < ActiveRecord::TestCase private def ensure_topic_method_is_not_cached(method_id) - class << Topic; self; end.send(:remove_method, method_id) if Topic.public_methods.include? method_id + Topic.singleton_class.remove_method method_id if Topic.public_methods.include? method_id end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 21e84d850b..961ae03a4c 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1116,7 +1116,7 @@ class FinderTest < ActiveRecord::TestCase def test_dynamic_finder_on_one_attribute_with_conditions_returns_same_results_after_caching # ensure this test can run independently of order - class << Account; self; end.send(:remove_method, :find_by_credit_limit) if Account.public_methods.include?(:find_by_credit_limit) + Account.singleton_class.remove_method :find_by_credit_limit if Account.public_methods.include?(:find_by_credit_limit) a = Account.where("firm_id = ?", 6).find_by_credit_limit(50) assert_equal a, Account.where("firm_id = ?", 6).find_by_credit_limit(50) # find_by_credit_limit has been cached end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index a5592fc86a..fe2f417a04 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -73,14 +73,12 @@ class FixturesTest < ActiveRecord::TestCase if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) def test_bulk_insert - begin - subscriber = InsertQuerySubscriber.new - subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) - create_fixtures("bulbs") - assert_equal 1, subscriber.events.size, "It takes one INSERT query to insert two fixtures" - ensure - ActiveSupport::Notifications.unsubscribe(subscription) - end + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + create_fixtures("bulbs") + assert_equal 1, subscriber.events.size, "It takes one INSERT query to insert two fixtures" + ensure + ActiveSupport::Notifications.unsubscribe(subscription) end def test_bulk_insert_multiple_table_with_a_multi_statement_query diff --git a/activerecord/test/cases/habtm_destroy_order_test.rb b/activerecord/test/cases/habtm_destroy_order_test.rb index b15e1b48c4..9dbd339fe7 100644 --- a/activerecord/test/cases/habtm_destroy_order_test.rb +++ b/activerecord/test/cases/habtm_destroy_order_test.rb @@ -30,23 +30,21 @@ class HabtmDestroyOrderTest < ActiveRecord::TestCase test "not destroying a student with lessons leaves student<=>lesson association intact" do # test a normal before_destroy doesn't destroy the habtm joins - begin - sicp = Lesson.new(name: "SICP") - ben = Student.new(name: "Ben Bitdiddle") - # add a before destroy to student - Student.class_eval do - before_destroy do - raise ActiveRecord::Rollback unless lessons.empty? - end + sicp = Lesson.new(name: "SICP") + ben = Student.new(name: "Ben Bitdiddle") + # add a before destroy to student + Student.class_eval do + before_destroy do + raise ActiveRecord::Rollback unless lessons.empty? end - ben.lessons << sicp - ben.save! - ben.destroy - assert_not_empty ben.reload.lessons - ensure - # get rid of it so Student is still like it was - Student.reset_callbacks(:destroy) end + ben.lessons << sicp + ben.save! + ben.destroy + assert_not_empty ben.reload.lessons + ensure + # get rid of it so Student is still like it was + Student.reset_callbacks(:destroy) end test "not destroying a lesson with students leaves student<=>lesson association intact" do diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index 7a092103c7..620e9ab6ca 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -152,25 +152,23 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end test "foreign key methods respect pluralize_table_names" do - begin - original_pluralize_table_names = ActiveRecord::Base.pluralize_table_names - ActiveRecord::Base.pluralize_table_names = false - @connection.create_table :testing - @connection.change_table :testing_parents do |t| - t.references :testing, foreign_key: true - end + original_pluralize_table_names = ActiveRecord::Base.pluralize_table_names + ActiveRecord::Base.pluralize_table_names = false + @connection.create_table :testing + @connection.change_table :testing_parents do |t| + t.references :testing, foreign_key: true + end - fk = @connection.foreign_keys("testing_parents").first - assert_equal "testing_parents", fk.from_table - assert_equal "testing", fk.to_table + fk = @connection.foreign_keys("testing_parents").first + assert_equal "testing_parents", fk.from_table + assert_equal "testing", fk.to_table - assert_difference "@connection.foreign_keys('testing_parents').size", -1 do - @connection.remove_reference :testing_parents, :testing, foreign_key: true - end - ensure - ActiveRecord::Base.pluralize_table_names = original_pluralize_table_names - @connection.drop_table "testing", if_exists: true + assert_difference "@connection.foreign_keys('testing_parents').size", -1 do + @connection.remove_reference :testing_parents, :testing, foreign_key: true end + ensure + ActiveRecord::Base.pluralize_table_names = original_pluralize_table_names + @connection.drop_table "testing", if_exists: true end class CreateDogsMigration < ActiveRecord::Migration::Current diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 661163b4a1..8b0ecd2516 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -576,29 +576,25 @@ class MigrationTest < ActiveRecord::TestCase # table name is 29 chars, the standard sequence name will # be 33 chars and should be shortened assert_nothing_raised do - begin - Person.connection.create_table :table_with_name_thats_just_ok do |t| - t.column :foo, :string, null: false - end - ensure - Person.connection.drop_table :table_with_name_thats_just_ok rescue nil + Person.connection.create_table :table_with_name_thats_just_ok do |t| + t.column :foo, :string, null: false end + ensure + Person.connection.drop_table :table_with_name_thats_just_ok rescue nil end # should be all good w/ a custom sequence name assert_nothing_raised do - begin - Person.connection.create_table :table_with_name_thats_just_ok, - sequence_name: "suitably_short_seq" do |t| - t.column :foo, :string, null: false - end + Person.connection.create_table :table_with_name_thats_just_ok, + sequence_name: "suitably_short_seq" do |t| + t.column :foo, :string, null: false + end - Person.connection.execute("select suitably_short_seq.nextval from dual") + Person.connection.execute("select suitably_short_seq.nextval from dual") - ensure - Person.connection.drop_table :table_with_name_thats_just_ok, - sequence_name: "suitably_short_seq" rescue nil - end + ensure + Person.connection.drop_table :table_with_name_thats_just_ok, + sequence_name: "suitably_short_seq" rescue nil end # confirm the custom sequence got dropped @@ -742,15 +738,13 @@ class MigrationTest < ActiveRecord::TestCase test_terminated = Concurrent::CountDownLatch.new other_process = Thread.new do - begin - conn = ActiveRecord::Base.connection_pool.checkout - conn.get_advisory_lock(lock_id) - thread_lock.count_down - test_terminated.wait # hold the lock open until we tested everything - ensure - conn.release_advisory_lock(lock_id) - ActiveRecord::Base.connection_pool.checkin(conn) - end + conn = ActiveRecord::Base.connection_pool.checkout + conn.get_advisory_lock(lock_id) + thread_lock.count_down + test_terminated.wait # hold the lock open until we tested everything + ensure + conn.release_advisory_lock(lock_id) + ActiveRecord::Base.connection_pool.checkin(conn) end thread_lock.wait # wait until the 'other process' has the lock diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb index 192d2f5251..f11c441c65 100644 --- a/activerecord/test/cases/multiple_db_test.rb +++ b/activerecord/test/cases/multiple_db_test.rb @@ -106,14 +106,12 @@ class MultipleDbTest < ActiveRecord::TestCase end def test_associations_should_work_when_model_has_no_connection - begin - ActiveRecord::Base.remove_connection - assert_nothing_raised do - College.first.courses.first - end - ensure - ActiveRecord::Base.establish_connection :arunit + ActiveRecord::Base.remove_connection + assert_nothing_raised do + College.first.courses.first end + ensure + ActiveRecord::Base.establish_connection :arunit end end end diff --git a/activerecord/test/cases/pooled_connections_test.rb b/activerecord/test/cases/pooled_connections_test.rb index fa7f759e51..080aeb0989 100644 --- a/activerecord/test/cases/pooled_connections_test.rb +++ b/activerecord/test/cases/pooled_connections_test.rb @@ -25,14 +25,12 @@ class PooledConnectionsTest < ActiveRecord::TestCase @timed_out = 0 threads.times do Thread.new do - begin - conn = ActiveRecord::Base.connection_pool.checkout - sleep 0.1 - ActiveRecord::Base.connection_pool.checkin conn - @connection_count += 1 - rescue ActiveRecord::ConnectionTimeoutError - @timed_out += 1 - end + conn = ActiveRecord::Base.connection_pool.checkout + sleep 0.1 + ActiveRecord::Base.connection_pool.checkin conn + @connection_count += 1 + rescue ActiveRecord::ConnectionTimeoutError + @timed_out += 1 end.join end end @@ -42,14 +40,12 @@ class PooledConnectionsTest < ActiveRecord::TestCase @connection_count = 0 @timed_out = 0 loops.times do - begin - conn = ActiveRecord::Base.connection_pool.checkout - ActiveRecord::Base.connection_pool.checkin conn - @connection_count += 1 - ActiveRecord::Base.connection.data_sources - rescue ActiveRecord::ConnectionTimeoutError - @timed_out += 1 - end + conn = ActiveRecord::Base.connection_pool.checkout + ActiveRecord::Base.connection_pool.checkin conn + @connection_count += 1 + ActiveRecord::Base.connection.data_sources + rescue ActiveRecord::ConnectionTimeoutError + @timed_out += 1 end end diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 0256a6da61..74d63c9995 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -78,76 +78,74 @@ class QueryCacheTest < ActiveRecord::TestCase def test_query_cache_across_threads with_temporary_connection_pool do - begin - if in_memory_db? - # Separate connections to an in-memory database create an entirely new database, - # with an empty schema etc, so we just stub out this schema on the fly. - ActiveRecord::Base.connection_pool.with_connection do |connection| - connection.create_table :tasks do |t| - t.datetime :starting - t.datetime :ending - end + if in_memory_db? + # Separate connections to an in-memory database create an entirely new database, + # with an empty schema etc, so we just stub out this schema on the fly. + ActiveRecord::Base.connection_pool.with_connection do |connection| + connection.create_table :tasks do |t| + t.datetime :starting + t.datetime :ending end - ActiveRecord::FixtureSet.create_fixtures(self.class.fixture_path, ["tasks"], {}, ActiveRecord::Base) end + ActiveRecord::FixtureSet.create_fixtures(self.class.fixture_path, ["tasks"], {}, ActiveRecord::Base) + end - ActiveRecord::Base.connection_pool.connections.each do |conn| - assert_cache :off, conn - end + ActiveRecord::Base.connection_pool.connections.each do |conn| + assert_cache :off, conn + end - assert_not_predicate ActiveRecord::Base.connection, :nil? - assert_cache :off + assert_not_predicate ActiveRecord::Base.connection, :nil? + assert_cache :off - middleware { - assert_cache :clean + middleware { + assert_cache :clean - Task.find 1 - assert_cache :dirty + Task.find 1 + assert_cache :dirty - thread_1_connection = ActiveRecord::Base.connection - ActiveRecord::Base.clear_active_connections! - assert_cache :off, thread_1_connection + thread_1_connection = ActiveRecord::Base.connection + ActiveRecord::Base.clear_active_connections! + assert_cache :off, thread_1_connection - started = Concurrent::Event.new - checked = Concurrent::Event.new + started = Concurrent::Event.new + checked = Concurrent::Event.new - thread_2_connection = nil - thread = Thread.new { - thread_2_connection = ActiveRecord::Base.connection + thread_2_connection = nil + thread = Thread.new { + thread_2_connection = ActiveRecord::Base.connection - assert_equal thread_2_connection, thread_1_connection - assert_cache :off + assert_equal thread_2_connection, thread_1_connection + assert_cache :off - middleware { - assert_cache :clean + middleware { + assert_cache :clean - Task.find 1 - assert_cache :dirty + Task.find 1 + assert_cache :dirty - started.set - checked.wait + started.set + checked.wait - ActiveRecord::Base.clear_active_connections! - }.call({}) - } + ActiveRecord::Base.clear_active_connections! + }.call({}) + } - started.wait + started.wait - thread_1_connection = ActiveRecord::Base.connection - assert_not_equal thread_1_connection, thread_2_connection - assert_cache :dirty, thread_2_connection - checked.set - thread.join + thread_1_connection = ActiveRecord::Base.connection + assert_not_equal thread_1_connection, thread_2_connection + assert_cache :dirty, thread_2_connection + checked.set + thread.join - assert_cache :off, thread_2_connection - }.call({}) + assert_cache :off, thread_2_connection + }.call({}) - ActiveRecord::Base.connection_pool.connections.each do |conn| - assert_cache :off, conn - end - ensure - ActiveRecord::Base.connection_pool.disconnect! + ActiveRecord::Base.connection_pool.connections.each do |conn| + assert_cache :off, conn end + ensure + ActiveRecord::Base.connection_pool.disconnect! end end @@ -374,12 +372,10 @@ class QueryCacheTest < ActiveRecord::TestCase assert_not_predicate Task, :connected? Task.cache do - begin - assert_queries(1) { Task.find(1); Task.find(1) } - ensure - ActiveRecord::Base.connection_handler.remove_connection(Task.connection_specification_name) - Task.connection_specification_name = spec_name - end + assert_queries(1) { Task.find(1); Task.find(1) } + ensure + ActiveRecord::Base.connection_handler.remove_connection(Task.connection_specification_name) + Task.connection_specification_name = spec_name end end end diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb index 09c365f31b..bb6912148c 100644 --- a/activerecord/test/cases/relation/update_all_test.rb +++ b/activerecord/test/cases/relation/update_all_test.rb @@ -198,11 +198,9 @@ class UpdateAllTest < ActiveRecord::TestCase def test_update_all_doesnt_ignore_order assert_equal authors(:david).id + 1, authors(:mary).id # make sure there is going to be a duplicate PK error test_update_with_order_succeeds = lambda do |order| - begin - Author.order(order).update_all("id = id + 1") - rescue ActiveRecord::ActiveRecordError - false - end + Author.order(order).update_all("id = id + 1") + rescue ActiveRecord::ActiveRecordError + false end if test_update_with_order_succeeds.call("id DESC") diff --git a/activerecord/test/cases/validations/absence_validation_test.rb b/activerecord/test/cases/validations/absence_validation_test.rb index 8235a54d8a..1982734f02 100644 --- a/activerecord/test/cases/validations/absence_validation_test.rb +++ b/activerecord/test/cases/validations/absence_validation_test.rb @@ -61,7 +61,7 @@ class AbsenceValidationTest < ActiveRecord::TestCase def test_validates_absence_of_virtual_attribute_on_model repair_validations(Interest) do - Interest.send(:attr_accessor, :token) + Interest.attr_accessor(:token) Interest.validates_absence_of(:token) interest = Interest.create!(topic: "Thought Leadering") diff --git a/activerecord/test/cases/validations/length_validation_test.rb b/activerecord/test/cases/validations/length_validation_test.rb index 1fbcdc271b..a7cb718043 100644 --- a/activerecord/test/cases/validations/length_validation_test.rb +++ b/activerecord/test/cases/validations/length_validation_test.rb @@ -64,7 +64,7 @@ class LengthValidationTest < ActiveRecord::TestCase def test_validates_length_of_virtual_attribute_on_model repair_validations(Pet) do - Pet.send(:attr_accessor, :nickname) + Pet.attr_accessor(:nickname) Pet.validates_length_of(:name, minimum: 1) Pet.validates_length_of(:nickname, minimum: 1) diff --git a/activerecord/test/cases/validations/presence_validation_test.rb b/activerecord/test/cases/validations/presence_validation_test.rb index 63c3f67da2..4b9cbe9098 100644 --- a/activerecord/test/cases/validations/presence_validation_test.rb +++ b/activerecord/test/cases/validations/presence_validation_test.rb @@ -69,7 +69,7 @@ class PresenceValidationTest < ActiveRecord::TestCase def test_validates_presence_of_virtual_attribute_on_model repair_validations(Interest) do - Interest.send(:attr_accessor, :abbreviation) + Interest.attr_accessor(:abbreviation) Interest.validates_presence_of(:topic) Interest.validates_presence_of(:abbreviation) diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index ea9b10add9..99f1ef9d86 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -123,9 +123,9 @@ *Janko Marohnić* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activestorage/CHANGELOG.md) for previous changes. diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 2c8816df25..dfada7054a 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Local and cloud file storage framework." s.description = "Attach cloud and local files in Rails applications." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index a2c4b4d57c..17cecd891c 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -51,12 +51,10 @@ module ActiveStorage def delete(key) instrument :delete, key: key do - begin - blobs.delete_blob(container, key) - rescue Azure::Core::Http::HTTPError => e - raise unless e.type == "BlobNotFound" - # Ignore files already deleted - end + blobs.delete_blob(container, key) + rescue Azure::Core::Http::HTTPError => e + raise unless e.type == "BlobNotFound" + # Ignore files already deleted end end diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 2588c41760..67892d43b2 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -29,35 +29,29 @@ module ActiveStorage end else instrument :download, key: key do - begin - File.binread path_for(key) - rescue Errno::ENOENT - raise ActiveStorage::FileNotFoundError - end + File.binread path_for(key) + rescue Errno::ENOENT + raise ActiveStorage::FileNotFoundError end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - begin - File.open(path_for(key), "rb") do |file| - file.seek range.begin - file.read range.size - end - rescue Errno::ENOENT - raise ActiveStorage::FileNotFoundError + File.open(path_for(key), "rb") do |file| + file.seek range.begin + file.read range.size end + rescue Errno::ENOENT + raise ActiveStorage::FileNotFoundError end end def delete(key) instrument :delete, key: key do - begin - File.delete path_for(key) - rescue Errno::ENOENT - # Ignore files already deleted - end + File.delete path_for(key) + rescue Errno::ENOENT + # Ignore files already deleted end end diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index e2590aa35d..9c20ed1d10 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -13,16 +13,14 @@ module ActiveStorage def upload(key, io, checksum: nil, content_type: nil, disposition: nil, filename: nil) instrument :upload, key: key, checksum: checksum do - begin - # GCS's signed URLs don't include params such as response-content-type response-content_disposition - # in the signature, which means an attacker can modify them and bypass our effort to force these to - # binary and attachment when the file's content type requires it. The only way to force them is to - # store them as object's metadata. - content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename - bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition) - rescue Google::Cloud::InvalidArgumentError - raise ActiveStorage::IntegrityError - end + # GCS's signed URLs don't include params such as response-content-type response-content_disposition + # in the signature, which means an attacker can modify them and bypass our effort to force these to + # binary and attachment when the file's content type requires it. The only way to force them is to + # store them as object's metadata. + content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename + bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition) + rescue Google::Cloud::InvalidArgumentError + raise ActiveStorage::IntegrityError end end @@ -33,11 +31,9 @@ module ActiveStorage end else instrument :download, key: key do - begin - file_for(key).download.string - rescue Google::Cloud::NotFoundError - raise ActiveStorage::FileNotFoundError - end + file_for(key).download.string + rescue Google::Cloud::NotFoundError + raise ActiveStorage::FileNotFoundError end end end @@ -53,32 +49,26 @@ module ActiveStorage def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - begin - file_for(key).download(range: range).string - rescue Google::Cloud::NotFoundError - raise ActiveStorage::FileNotFoundError - end + file_for(key).download(range: range).string + rescue Google::Cloud::NotFoundError + raise ActiveStorage::FileNotFoundError end end def delete(key) instrument :delete, key: key do - begin - file_for(key).delete - rescue Google::Cloud::NotFoundError - # Ignore files already deleted - end + file_for(key).delete + rescue Google::Cloud::NotFoundError + # Ignore files already deleted end end def delete_prefixed(prefix) instrument :delete_prefixed, prefix: prefix do bucket.files(prefix: prefix).all do |file| - begin - file.delete - rescue Google::Cloud::NotFoundError - # Ignore concurrently-deleted files - end + file.delete + rescue Google::Cloud::NotFoundError + # Ignore concurrently-deleted files end end end diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 90c3ae1b62..382920ef61 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -18,11 +18,9 @@ module ActiveStorage def upload(key, io, checksum: nil, **) instrument :upload, key: key, checksum: checksum do - begin - object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) - rescue Aws::S3::Errors::BadDigest - raise ActiveStorage::IntegrityError - end + object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) + rescue Aws::S3::Errors::BadDigest + raise ActiveStorage::IntegrityError end end @@ -33,22 +31,18 @@ module ActiveStorage end else instrument :download, key: key do - begin - object_for(key).get.body.string.force_encoding(Encoding::BINARY) - rescue Aws::S3::Errors::NoSuchKey - raise ActiveStorage::FileNotFoundError - end + object_for(key).get.body.string.force_encoding(Encoding::BINARY) + rescue Aws::S3::Errors::NoSuchKey + raise ActiveStorage::FileNotFoundError end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - begin - object_for(key).get(range: "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body.read.force_encoding(Encoding::BINARY) - rescue Aws::S3::Errors::NoSuchKey - raise ActiveStorage::FileNotFoundError - end + object_for(key).get(range: "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body.read.force_encoding(Encoding::BINARY) + rescue Aws::S3::Errors::NoSuchKey + raise ActiveStorage::FileNotFoundError end end diff --git a/activestorage/test/dummy/bin/yarn b/activestorage/test/dummy/bin/yarn index c9b7498378..d0dd7c27ac 100755 --- a/activestorage/test/dummy/bin/yarn +++ b/activestorage/test/dummy/bin/yarn @@ -3,11 +3,9 @@ VENDOR_PATH = File.expand_path("..", __dir__) Dir.chdir(VENDOR_PATH) do - begin - exec "yarnpkg #{ARGV.join(" ")}" - rescue Errno::ENOENT - $stderr.puts "Yarn executable was not detected in the system." - $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" - exit 1 - end + exec "yarnpkg #{ARGV.join(" ")}" +rescue Errno::ENOENT + $stderr.puts "Yarn executable was not detected in the system." + $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" + exit 1 end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 3b563b3fc8..8fede0e682 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -590,7 +590,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase assert_equal "town.jpg", @user.highlights.first.filename.to_s assert_equal "funky.jpg", @user.highlights.second.filename.to_s ensure - User.send(:remove_method, :highlights) + User.remove_method :highlights end end end diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index 561c3e9d23..7fb3262781 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -507,7 +507,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase assert_equal "gpj.yknuf", @user.avatar ensure - User.send(:remove_method, :avatar) + User.remove_method :avatar end end end diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 8552080e7b..4f88440e54 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -26,16 +26,14 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end test "monochrome with default variant_processor" do - begin - ActiveStorage.variant_processor = nil - - blob = create_file_blob(filename: "racecar.jpg") - variant = blob.variant(monochrome: true).processed - image = read_image(variant) - assert_match(/Gray/, image.colorspace) - ensure - ActiveStorage.variant_processor = :mini_magick - end + ActiveStorage.variant_processor = nil + + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(monochrome: true).processed + image = read_image(variant) + assert_match(/Gray/, image.colorspace) + ensure + ActiveStorage.variant_processor = :mini_magick end test "disabled variation of JPEG blob" do @@ -66,45 +64,41 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end test "disabled variation using :combine_options" do - begin - ActiveStorage.variant_processor = nil - blob = create_file_blob(filename: "racecar.jpg") - variant = ActiveSupport::Deprecation.silence do - blob.variant(combine_options: { - crop: "100x100+0+0", - monochrome: false - }).processed - end - assert_match(/racecar\.jpg/, variant.service_url) - - image = read_image(variant) - assert_equal 100, image.width - assert_equal 100, image.height - assert_match(/RGB/, image.colorspace) - ensure - ActiveStorage.variant_processor = :mini_magick + ActiveStorage.variant_processor = nil + blob = create_file_blob(filename: "racecar.jpg") + variant = ActiveSupport::Deprecation.silence do + blob.variant(combine_options: { + crop: "100x100+0+0", + monochrome: false + }).processed end + assert_match(/racecar\.jpg/, variant.service_url) + + image = read_image(variant) + assert_equal 100, image.width + assert_equal 100, image.height + assert_match(/RGB/, image.colorspace) + ensure + ActiveStorage.variant_processor = :mini_magick end test "center-weighted crop of JPEG blob using :combine_options" do - begin - ActiveStorage.variant_processor = nil - blob = create_file_blob(filename: "racecar.jpg") - variant = ActiveSupport::Deprecation.silence do - blob.variant(combine_options: { - gravity: "center", - resize: "100x100^", - crop: "100x100+0+0", - }).processed - end - assert_match(/racecar\.jpg/, variant.service_url) - - image = read_image(variant) - assert_equal 100, image.width - assert_equal 100, image.height - ensure - ActiveStorage.variant_processor = :mini_magick + ActiveStorage.variant_processor = nil + blob = create_file_blob(filename: "racecar.jpg") + variant = ActiveSupport::Deprecation.silence do + blob.variant(combine_options: { + gravity: "center", + resize: "100x100^", + crop: "100x100+0+0", + }).processed end + assert_match(/racecar\.jpg/, variant.service_url) + + image = read_image(variant) + assert_equal 100, image.width + assert_equal 100, image.height + ensure + ActiveStorage.variant_processor = :mini_magick end test "center-weighted crop of JPEG blob using :resize_to_fill" do @@ -160,18 +154,16 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end test "works for vips processor" do - begin - ActiveStorage.variant_processor = :vips - blob = create_file_blob(filename: "racecar.jpg") - variant = blob.variant(thumbnail_image: 100).processed - - image = read_image(variant) - assert_equal 100, image.width - assert_equal 67, image.height - rescue LoadError - # libvips not installed - ensure - ActiveStorage.variant_processor = :mini_magick - end + ActiveStorage.variant_processor = :vips + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(thumbnail_image: 100).processed + + image = read_image(variant) + assert_equal 100, image.width + assert_equal 67, image.height + rescue LoadError + # libvips not installed + ensure + ActiveStorage.variant_processor = :mini_magick end end diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb index 09c2e7f99c..2b07902d07 100644 --- a/activestorage/test/service/azure_storage_service_test.rb +++ b/activestorage/test/service/azure_storage_service_test.rb @@ -18,20 +18,18 @@ if SERVICE_CONFIGURATIONS[:azure] end test "uploading a tempfile" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - - Tempfile.open do |file| - file.write(data) - file.rewind - @service.upload(key, file) - end - - assert_equal data, @service.download(key) - ensure - @service.delete(key) + key = SecureRandom.base58(24) + data = "Something else entirely!" + + Tempfile.open do |file| + file.write(data) + file.rewind + @service.upload(key, file) end + + assert_equal data, @service.download(key) + ensure + @service.delete(key) end end else diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index af27946357..6bca428f50 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -10,74 +10,66 @@ if SERVICE_CONFIGURATIONS[:gcs] include ActiveStorage::Service::SharedServiceTests test "direct upload" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - checksum = Digest::MD5.base64digest(data) - url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) - - uri = URI.parse url - request = Net::HTTP::Put.new uri.request_uri - request.body = data - request.add_field "Content-Type", "" - request.add_field "Content-MD5", checksum - Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| - http.request request - end - - assert_equal data, @service.download(key) - ensure - @service.delete key + key = SecureRandom.base58(24) + data = "Something else entirely!" + checksum = Digest::MD5.base64digest(data) + url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) + + uri = URI.parse url + request = Net::HTTP::Put.new uri.request_uri + request.body = data + request.add_field "Content-Type", "" + request.add_field "Content-MD5", checksum + Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| + http.request request end + + assert_equal data, @service.download(key) + ensure + @service.delete key end test "upload with content_type and content_disposition" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") - - url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) - response = Net::HTTP.get_response(URI(url)) - assert_equal "text/plain", response.content_type - assert_match(/attachment;.*test.txt/, response["Content-Disposition"]) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/attachment;.*test.txt/, response["Content-Disposition"]) + ensure + @service.delete key end test "upload with content_type" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain") - - url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) - response = Net::HTTP.get_response(URI(url)) - assert_equal "text/plain", response.content_type - assert_match(/inline;.*test.html/, response["Content-Disposition"]) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain") + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/inline;.*test.html/, response["Content-Disposition"]) + ensure + @service.delete key end test "update metadata" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html") - - @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") - url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) - - response = Net::HTTP.get_response(URI(url)) - assert_equal "text/plain", response.content_type - assert_match(/inline;.*test.txt/, response["Content-Disposition"]) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html") + + @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html")) + + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.content_type + assert_match(/inline;.*test.txt/, response["Content-Disposition"]) + ensure + @service.delete key end test "signed URL generation" do diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb index bb502dde60..94c751a4ff 100644 --- a/activestorage/test/service/mirror_service_test.rb +++ b/activestorage/test/service/mirror_service_test.rb @@ -18,22 +18,20 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase include ActiveStorage::Service::SharedServiceTests test "uploading to all services" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - io = StringIO.new(data) - checksum = Digest::MD5.base64digest(data) + key = SecureRandom.base58(24) + data = "Something else entirely!" + io = StringIO.new(data) + checksum = Digest::MD5.base64digest(data) - @service.upload key, io.tap(&:read), checksum: checksum - assert_predicate io, :eof? + @service.upload key, io.tap(&:read), checksum: checksum + assert_predicate io, :eof? - assert_equal data, @service.primary.download(key) - @service.mirrors.each do |mirror| - assert_equal data, mirror.download(key) - end - ensure - @service.delete key + assert_equal data, @service.primary.download(key) + @service.mirrors.each do |mirror| + assert_equal data, mirror.download(key) end + ensure + @service.delete key end test "downloading from primary service" do diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index 0e5b06dd78..0a6004267f 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -11,25 +11,23 @@ if SERVICE_CONFIGURATIONS[:s3] include ActiveStorage::Service::SharedServiceTests test "direct upload" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - checksum = Digest::MD5.base64digest(data) - url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) - - uri = URI.parse url - request = Net::HTTP::Put.new uri.request_uri - request.body = data - request.add_field "Content-Type", "text/plain" - request.add_field "Content-MD5", checksum - Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| - http.request request - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + checksum = Digest::MD5.base64digest(data) + url = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum) - assert_equal data, @service.download(key) - ensure - @service.delete key + uri = URI.parse url + request = Net::HTTP::Put.new uri.request_uri + request.body = data + request.add_field "Content-Type", "text/plain" + request.add_field "Content-MD5", checksum + Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| + http.request request end + + assert_equal data, @service.download(key) + ensure + @service.delete key end test "upload a zero byte file" do diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index b97030b320..17f3736056 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -20,48 +20,42 @@ module ActiveStorage::Service::SharedServiceTests end test "uploading with integrity" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data)) + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data)) - assert_equal data, @service.download(key) - ensure - @service.delete key - end + assert_equal data, @service.download(key) + ensure + @service.delete key end test "uploading without integrity" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" + key = SecureRandom.base58(24) + data = "Something else entirely!" - assert_raises(ActiveStorage::IntegrityError) do - @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data")) - end - - assert_not @service.exist?(key) - ensure - @service.delete key + assert_raises(ActiveStorage::IntegrityError) do + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data")) end + + assert_not @service.exist?(key) + ensure + @service.delete key end test "uploading with integrity and multiple keys" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @service.upload( - key, - StringIO.new(data), - checksum: Digest::MD5.base64digest(data), - filename: "racecar.jpg", - content_type: "image/jpg" - ) - - assert_equal data, @service.download(key) - ensure - @service.delete key - end + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload( + key, + StringIO.new(data), + checksum: Digest::MD5.base64digest(data), + filename: "racecar.jpg", + content_type: "image/jpg" + ) + + assert_equal data, @service.download(key) + ensure + @service.delete key end test "downloading" do @@ -129,20 +123,18 @@ module ActiveStorage::Service::SharedServiceTests end test "deleting by prefix" do - begin - @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) - @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) - @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) - - @service.delete_prefixed("a/a/") - assert_not @service.exist?("a/a/a") - assert_not @service.exist?("a/a/b") - assert @service.exist?("a/b/a") - ensure - @service.delete("a/a/a") - @service.delete("a/a/b") - @service.delete("a/b/a") - end + @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) + @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) + @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) + + @service.delete_prefixed("a/a/") + assert_not @service.exist?("a/a/a") + assert_not @service.exist?("a/a/b") + assert @service.exist?("a/b/a") + ensure + @service.delete("a/a/a") + @service.delete("a/a/b") + @service.delete("a/b/a") end end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index ba8aaf47f9..e678f48244 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -282,9 +282,9 @@ *Jeremy Daer* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* * Adds parallel testing to Rails. diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index 448a2eeebb..bdd7bc70a0 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "A toolkit of support libraries and Ruby core extensions extracted from the Rails framework." s.description = "A toolkit of support libraries and Ruby core extensions extracted from the Rails framework. Rich support for multibyte strings, internationalization, time zones, and testing." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 53a2b07536..de1fb1886c 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -108,12 +108,10 @@ module ActiveSupport def lock_file(file_name, &block) if File.exist?(file_name) File.open(file_name, "r+") do |f| - begin - f.flock File::LOCK_EX - yield - ensure - f.flock File::LOCK_UN - end + f.flock File::LOCK_EX + yield + ensure + f.flock File::LOCK_UN end else yield diff --git a/activesupport/lib/active_support/core_ext/array.rb b/activesupport/lib/active_support/core_ext/array.rb index a2569c798b..88b6567712 100644 --- a/activesupport/lib/active_support/core_ext/array.rb +++ b/activesupport/lib/active_support/core_ext/array.rb @@ -6,5 +6,4 @@ 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" require "active_support/core_ext/array/inquiry" diff --git a/activesupport/lib/active_support/core_ext/array/prepend_and_append.rb b/activesupport/lib/active_support/core_ext/array/prepend_and_append.rb index 661971d7cd..ba3739f640 100644 --- a/activesupport/lib/active_support/core_ext/array/prepend_and_append.rb +++ b/activesupport/lib/active_support/core_ext/array/prepend_and_append.rb @@ -1,9 +1,5 @@ # frozen_string_literal: true -class Array - # The human way of thinking about adding stuff to the end of a list is with append. - alias_method :append, :push unless [].respond_to?(:append) +require "active_support/deprecation" - # The human way of thinking about adding stuff to the beginning of a list is with prepend. - alias_method :prepend, :unshift unless [].respond_to?(:prepend) -end +ActiveSupport::Deprecation.warn "Ruby 2.5+ (required by Rails 6) provides Array#append and Array#prepend natively, so requiring active_support/core_ext/array/prepend_and_append is no longer necessary. Requiring it will raise LoadError in Rails 6.1." diff --git a/activesupport/lib/active_support/core_ext/hash/compact.rb b/activesupport/lib/active_support/core_ext/hash/compact.rb index 28c8d86b9b..5cb858af5c 100644 --- a/activesupport/lib/active_support/core_ext/hash/compact.rb +++ b/activesupport/lib/active_support/core_ext/hash/compact.rb @@ -2,4 +2,4 @@ require "active_support/deprecation" -ActiveSupport::Deprecation.warn "Ruby 2.4+ (required by Rails 6) provides Hash#compact and Hash#compact! natively, so requiring active_support/core_ext/hash/compact is no longer necessary. Requiring it will raise LoadError in Rails 6.1." +ActiveSupport::Deprecation.warn "Ruby 2.5+ (required by Rails 6) provides Hash#compact and Hash#compact! natively, so requiring active_support/core_ext/hash/compact is no longer necessary. Requiring it will raise LoadError in Rails 6.1." diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb index bdf196ec3d..7d3495db2c 100644 --- a/activesupport/lib/active_support/core_ext/hash/keys.rb +++ b/activesupport/lib/active_support/core_ext/hash/keys.rb @@ -1,35 +1,6 @@ # frozen_string_literal: true class Hash - # Returns a new hash with all keys converted using the +block+ operation. - # - # hash = { name: 'Rob', age: '28' } - # - # hash.transform_keys { |key| key.to_s.upcase } # => {"NAME"=>"Rob", "AGE"=>"28"} - # - # If you do not provide a +block+, it will return an Enumerator - # for chaining with other methods: - # - # hash.transform_keys.with_index { |k, i| [k, i].join } # => {"name0"=>"Rob", "age1"=>"28"} - def transform_keys - return enum_for(:transform_keys) { size } unless block_given? - result = {} - each_key do |key| - result[yield(key)] = self[key] - end - result - end unless method_defined? :transform_keys - - # Destructively converts all keys using the +block+ operations. - # Same as +transform_keys+ but modifies +self+. - def transform_keys! - return enum_for(:transform_keys!) { size } unless block_given? - keys.each do |key| - self[yield(key)] = delete(key) - end - self - end unless method_defined? :transform_keys! - # Returns a new hash with all keys converted to strings. # # hash = { name: 'Rob', age: '28' } diff --git a/activesupport/lib/active_support/core_ext/hash/slice.rb b/activesupport/lib/active_support/core_ext/hash/slice.rb index e07a5498ff..3d0f8a1e62 100644 --- a/activesupport/lib/active_support/core_ext/hash/slice.rb +++ b/activesupport/lib/active_support/core_ext/hash/slice.rb @@ -1,29 +1,6 @@ # frozen_string_literal: true class Hash - # Slices a hash to include only the given keys. Returns a hash containing - # the given keys. - # - # { a: 1, b: 2, c: 3, d: 4 }.slice(:a, :b) - # # => {:a=>1, :b=>2} - # - # This is useful for limiting an options hash to valid keys before - # passing to a method: - # - # def search(criteria = {}) - # criteria.assert_valid_keys(:mass, :velocity, :time) - # end - # - # search(options.slice(:mass, :velocity, :time)) - # - # If you have an array of keys you want to limit to, you should splat them: - # - # valid_keys = [:mass, :velocity, :time] - # search(options.slice(*valid_keys)) - def slice(*keys) - keys.each_with_object(Hash.new) { |k, hash| hash[k] = self[k] if has_key?(k) } - end unless method_defined?(:slice) - # Replaces the hash with only the given keys. # Returns a hash containing the removed key/value pairs. # diff --git a/activesupport/lib/active_support/core_ext/hash/transform_values.rb b/activesupport/lib/active_support/core_ext/hash/transform_values.rb index fc15130c9e..e4aeb0e891 100644 --- a/activesupport/lib/active_support/core_ext/hash/transform_values.rb +++ b/activesupport/lib/active_support/core_ext/hash/transform_values.rb @@ -2,4 +2,4 @@ require "active_support/deprecation" -ActiveSupport::Deprecation.warn "Ruby 2.4+ (required by Rails 6) provides Hash#transform_values natively, so requiring active_support/core_ext/hash/transform_values is no longer necessary. Requiring it will raise LoadError in Rails 6.1." +ActiveSupport::Deprecation.warn "Ruby 2.5+ (required by Rails 6) provides Hash#transform_values natively, so requiring active_support/core_ext/hash/transform_values is no longer necessary. Requiring it will raise LoadError in Rails 6.1." diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 3128539112..be90390ae4 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -19,7 +19,7 @@ class Module # public methods as your own. # # ==== Options - # * <tt>:to</tt> - Specifies the target object + # * <tt>:to</tt> - Specifies the target object name as a symbol or string # * <tt>:prefix</tt> - Prefixes the new method with the target name or a custom prefix # * <tt>:allow_nil</tt> - If set to true, prevents a +Module::DelegationError+ # from being raised diff --git a/activesupport/lib/active_support/core_ext/numeric/inquiry.rb b/activesupport/lib/active_support/core_ext/numeric/inquiry.rb index e05e40825b..6b5240d051 100644 --- a/activesupport/lib/active_support/core_ext/numeric/inquiry.rb +++ b/activesupport/lib/active_support/core_ext/numeric/inquiry.rb @@ -2,4 +2,4 @@ require "active_support/deprecation" -ActiveSupport::Deprecation.warn "Ruby 2.4+ (required by Rails 6) provides Numeric#positive? and Numeric#negative? natively, so requiring active_support/core_ext/numeric/inquiry is no longer necessary. Requiring it will raise LoadError in Rails 6.1." +ActiveSupport::Deprecation.warn "Ruby 2.5+ (required by Rails 6) provides Numeric#positive? and Numeric#negative? natively, so requiring active_support/core_ext/numeric/inquiry is no longer necessary. Requiring it will raise LoadError in Rails 6.1." diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb index f0c3e37e65..d99571790f 100644 --- a/activesupport/lib/active_support/deprecation/method_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb @@ -60,13 +60,13 @@ module ActiveSupport with_method = "#{aliased_method}_with_deprecation#{punctuation}" without_method = "#{aliased_method}_without_deprecation#{punctuation}" - target_module.send(:define_method, with_method) do |*args, &block| + target_module.define_method(with_method) do |*args, &block| deprecator.deprecation_warning(method_name, options[method_name]) send(without_method, *args, &block) end - target_module.send(:alias_method, without_method, method_name) - target_module.send(:alias_method, method_name, with_method) + target_module.alias_method(without_method, method_name) + target_module.alias_method(method_name, with_method) case when target_module.protected_method_defined?(without_method) @@ -75,7 +75,7 @@ module ActiveSupport target_module.send(:private, method_name) end else - mod.send(:define_method, method_name) do |*args, &block| + mod.define_method(method_name) do |*args, &block| deprecator.deprecation_warning(method_name, options[method_name]) super(*args, &block) end diff --git a/activesupport/lib/active_support/evented_file_update_checker.rb b/activesupport/lib/active_support/evented_file_update_checker.rb index 97e982eb05..54bc5c0ae5 100644 --- a/activesupport/lib/active_support/evented_file_update_checker.rb +++ b/activesupport/lib/active_support/evented_file_update_checker.rb @@ -52,16 +52,17 @@ module ActiveSupport @pid = Process.pid @boot_mutex = Mutex.new - if (@dtw = directories_to_watch).any? + dtw = directories_to_watch + @dtw, @missing = dtw.partition(&:exist?) + + if @dtw.any? # Loading listen triggers warnings. These are originated by a legit # usage of attr_* macros for private attributes, but adds a lot of noise # to our test suite. Thus, we lazy load it and disable warnings locally. silence_warnings do - begin - require "listen" - rescue LoadError => e - raise LoadError, "Could not load the 'listen' gem. Add `gem 'listen'` to the development group of your Gemfile", e.backtrace - end + require "listen" + rescue LoadError => e + raise LoadError, "Could not load the 'listen' gem. Add `gem 'listen'` to the development group of your Gemfile", e.backtrace end end boot! @@ -75,6 +76,19 @@ module ActiveSupport @updated.make_true end end + + if @missing.any?(&:exist?) + @boot_mutex.synchronize do + appeared, @missing = @missing.partition(&:exist?) + shutdown! + + @dtw += appeared + boot! + + @updated.make_true + end + end + @updated.true? end @@ -96,6 +110,10 @@ module ActiveSupport Listen.to(*@dtw, &method(:changed)).start end + def shutdown! + Listen.stop + end + def changed(modified, added, removed) unless updated? @updated.make_true if (modified + added + removed).any? { |f| watching?(f) } @@ -123,7 +141,7 @@ module ActiveSupport end def directories_to_watch - dtw = (@files + @dirs.keys).map { |f| @ph.existing_parent(f) } + dtw = @files.map(&:dirname) + @dirs.keys dtw.compact! dtw.uniq! diff --git a/activesupport/lib/active_support/inflector/inflections.rb b/activesupport/lib/active_support/inflector/inflections.rb index 2b86d233e5..fa087c4dd6 100644 --- a/activesupport/lib/active_support/inflector/inflections.rb +++ b/activesupport/lib/active_support/inflector/inflections.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "concurrent/map" -require "active_support/core_ext/array/prepend_and_append" require "active_support/i18n" require "active_support/deprecation" diff --git a/activesupport/lib/active_support/reloader.rb b/activesupport/lib/active_support/reloader.rb index fea18e9712..2f81cd4f80 100644 --- a/activesupport/lib/active_support/reloader.rb +++ b/activesupport/lib/active_support/reloader.rb @@ -50,11 +50,9 @@ module ActiveSupport def self.reload! executor.wrap do new.tap do |instance| - begin - instance.run! - ensure - instance.complete! - end + instance.run! + ensure + instance.complete! end end prepare! diff --git a/activesupport/lib/active_support/testing/method_call_assertions.rb b/activesupport/lib/active_support/testing/method_call_assertions.rb index eba41aa907..03c38be481 100644 --- a/activesupport/lib/active_support/testing/method_call_assertions.rb +++ b/activesupport/lib/active_support/testing/method_call_assertions.rb @@ -35,18 +35,16 @@ module ActiveSupport assert_called(object, method_name, message, times: 0, &block) end - # TODO: No need to resort to #send once support for Ruby 2.4 is - # dropped. def assert_called_on_instance_of(klass, method_name, message = nil, times: 1, returns: nil) times_called = 0 - klass.send(:define_method, "stubbed_#{method_name}") do |*| + klass.define_method("stubbed_#{method_name}") do |*| times_called += 1 returns end - klass.send(:alias_method, "original_#{method_name}", method_name) - klass.send(:alias_method, method_name, "stubbed_#{method_name}") + klass.alias_method "original_#{method_name}", method_name + klass.alias_method method_name, "stubbed_#{method_name}" yield @@ -55,9 +53,9 @@ module ActiveSupport assert_equal times, times_called, error ensure - klass.send(:alias_method, method_name, "original_#{method_name}") - klass.send(:undef_method, "original_#{method_name}") - klass.send(:undef_method, "stubbed_#{method_name}") + klass.alias_method method_name, "original_#{method_name}" + klass.undef_method "original_#{method_name}" + klass.undef_method "stubbed_#{method_name}" end def assert_not_called_on_instance_of(klass, method_name, message = nil, &block) diff --git a/activesupport/lib/active_support/testing/parallelization.rb b/activesupport/lib/active_support/testing/parallelization.rb index c5d3a88131..63440069b1 100644 --- a/activesupport/lib/active_support/testing/parallelization.rb +++ b/activesupport/lib/active_support/testing/parallelization.rb @@ -69,31 +69,29 @@ module ActiveSupport def start @pool = @queue_size.times.map do |worker| fork do - begin - DRb.stop_service - - after_fork(worker) - - queue = DRbObject.new_with_uri(@url) - - while job = queue.pop - klass = job[0] - method = job[1] - reporter = job[2] - result = Minitest.run_one_method(klass, method) - - begin - queue.record(reporter, result) - rescue DRb::DRbConnError - result.failures.each do |failure| - failure.exception = DRb::DRbRemoteError.new(failure.exception) - end - queue.record(reporter, result) + DRb.stop_service + + after_fork(worker) + + queue = DRbObject.new_with_uri(@url) + + while job = queue.pop + klass = job[0] + method = job[1] + reporter = job[2] + result = Minitest.run_one_method(klass, method) + + begin + queue.record(reporter, result) + rescue DRb::DRbConnError + result.failures.each do |failure| + failure.exception = DRb::DRbRemoteError.new(failure.exception) end + queue.record(reporter, result) end - ensure - run_cleanup(worker) end + ensure + run_cleanup(worker) end end end diff --git a/activesupport/lib/active_support/testing/time_helpers.rb b/activesupport/lib/active_support/testing/time_helpers.rb index f160e66971..5a3fa9346c 100644 --- a/activesupport/lib/active_support/testing/time_helpers.rb +++ b/activesupport/lib/active_support/testing/time_helpers.rb @@ -22,7 +22,7 @@ module ActiveSupport @stubs[object.object_id][method_name] = Stub.new(object, method_name, new_name) - object.singleton_class.send :alias_method, new_name, method_name + object.singleton_class.alias_method new_name, method_name object.define_singleton_method(method_name, &block) end @@ -43,9 +43,9 @@ module ActiveSupport def unstub_object(stub) singleton_class = stub.object.singleton_class - singleton_class.send :silence_redefinition_of_method, stub.method_name - singleton_class.send :alias_method, stub.method_name, stub.original_method - singleton_class.send :undef_method, stub.original_method + singleton_class.silence_redefinition_of_method stub.method_name + singleton_class.alias_method stub.method_name, stub.original_method + singleton_class.undef_method stub.original_method end end diff --git a/activesupport/lib/active_support/xml_mini.rb b/activesupport/lib/active_support/xml_mini.rb index be298bf0a1..075cd4ed8b 100644 --- a/activesupport/lib/active_support/xml_mini.rb +++ b/activesupport/lib/active_support/xml_mini.rb @@ -3,6 +3,7 @@ require "time" require "base64" require "bigdecimal" +require "bigdecimal/util" require "active_support/core_ext/module/delegation" require "active_support/core_ext/string/inflections" require "active_support/core_ext/date_time/calculations" @@ -68,11 +69,7 @@ module ActiveSupport "float" => Proc.new { |float| float.to_f }, "decimal" => Proc.new do |number| if String === number - begin - BigDecimal(number) - rescue ArgumentError - BigDecimal(number.to_f.to_s) - end + number.to_d else BigDecimal(number) end diff --git a/activesupport/test/cache/behaviors/connection_pool_behavior.rb b/activesupport/test/cache/behaviors/connection_pool_behavior.rb index 4d1901a173..5e66e0b202 100644 --- a/activesupport/test/cache/behaviors/connection_pool_behavior.rb +++ b/activesupport/test/cache/behaviors/connection_pool_behavior.rb @@ -7,24 +7,22 @@ module ConnectionPoolBehavior threads = [] emulating_latency do - begin - cache = ActiveSupport::Cache.lookup_store(store, { pool_size: 2, pool_timeout: 1 }.merge(store_options)) - cache.clear - - assert_raises Timeout::Error do - # One of the three threads will fail in 1 second because our pool size - # is only two. - 3.times do - threads << Thread.new do - cache.read("latency") - end + cache = ActiveSupport::Cache.lookup_store(store, { pool_size: 2, pool_timeout: 1 }.merge(store_options)) + cache.clear + + assert_raises Timeout::Error do + # One of the three threads will fail in 1 second because our pool size + # is only two. + 3.times do + threads << Thread.new do + cache.read("latency") end - - threads.each(&:join) end - ensure - threads.each(&:kill) + + threads.each(&:join) end + ensure + threads.each(&:kill) end ensure Thread.report_on_exception = original_report_on_exception @@ -34,24 +32,22 @@ module ConnectionPoolBehavior threads = [] emulating_latency do - begin - cache = ActiveSupport::Cache.lookup_store(store, store_options) - cache.clear - - assert_nothing_raised do - # Default connection pool size is 5, assuming 10 will make sure that - # the connection pool isn't used at all. - 10.times do - threads << Thread.new do - cache.read("latency") - end + cache = ActiveSupport::Cache.lookup_store(store, store_options) + cache.clear + + assert_nothing_raised do + # Default connection pool size is 5, assuming 10 will make sure that + # the connection pool isn't used at all. + 10.times do + threads << Thread.new do + cache.read("latency") end - - threads.each(&:join) end - ensure - threads.each(&:kill) + + threads.each(&:join) end + ensure + threads.each(&:kill) end end diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 466b364e9d..79098b2a7d 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -31,7 +31,7 @@ module CallbacksTest def callback_object(callback_method) klass = Class.new - klass.send(:define_method, callback_method) do |model| + klass.define_method(callback_method) do |model| model.history << [:"#{callback_method}_save", :object] end klass.new diff --git a/activesupport/test/core_ext/array/prepend_append_test.rb b/activesupport/test/core_ext/array/prepend_append_test.rb index c34acd66ad..8573dbd5a6 100644 --- a/activesupport/test/core_ext/array/prepend_append_test.rb +++ b/activesupport/test/core_ext/array/prepend_append_test.rb @@ -1,14 +1,11 @@ # frozen_string_literal: true require "abstract_unit" -require "active_support/core_ext/array" class PrependAppendTest < ActiveSupport::TestCase - def test_append - assert_equal [1, 2], [1].append(2) - end - - def test_prepend - assert_equal [2, 1], [1].prepend(2) + def test_requiring_prepend_and_append_is_deprecated + assert_deprecated do + require "active_support/core_ext/array/prepend_and_append" + end end end diff --git a/activesupport/test/core_ext/hash/transform_keys_test.rb b/activesupport/test/core_ext/hash/transform_keys_test.rb deleted file mode 100644 index b9e41f7b25..0000000000 --- a/activesupport/test/core_ext/hash/transform_keys_test.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -require "abstract_unit" -require "active_support/core_ext/hash/keys" - -class TransformKeysTest < ActiveSupport::TestCase - test "transform_keys returns a new hash with the keys computed from the block" do - original = { a: "a", b: "b" } - mapped = original.transform_keys { |k| "#{k}!".to_sym } - - assert_equal({ a: "a", b: "b" }, original) - assert_equal({ a!: "a", b!: "b" }, mapped) - end - - test "transform_keys! modifies the keys of the original" do - original = { a: "a", b: "b" } - mapped = original.transform_keys! { |k| "#{k}!".to_sym } - - assert_equal({ a!: "a", b!: "b" }, original) - assert_same original, mapped - end - - test "transform_keys returns a sized Enumerator if no block is given" do - original = { a: "a", b: "b" } - enumerator = original.transform_keys - assert_equal original.size, enumerator.size - assert_equal Enumerator, enumerator.class - end - - test "transform_keys! returns a sized Enumerator if no block is given" do - original = { a: "a", b: "b" } - enumerator = original.transform_keys! - assert_equal original.size, enumerator.size - assert_equal Enumerator, enumerator.class - end - - test "transform_keys is chainable with Enumerable methods" do - original = { a: "a", b: "b" } - mapped = original.transform_keys.with_index { |k, i| [k, i].join.to_sym } - assert_equal({ a0: "a", b1: "b" }, mapped) - end - - test "transform_keys! is chainable with Enumerable methods" do - original = { a: "a", b: "b" } - original.transform_keys!.with_index { |k, i| [k, i].join.to_sym } - assert_equal({ a0: "a", b1: "b" }, original) - end - - test "transform_keys returns a Hash instance when self is inherited from Hash" do - class HashDescendant < ::Hash - def initialize(elements = nil) - super(elements) - (elements || {}).each_pair { |key, value| self[key] = value } - end - end - - original = HashDescendant.new(a: "a", b: "b") - mapped = original.transform_keys { |k| "#{k}!".to_sym } - - assert_equal({ a: "a", b: "b" }, original) - assert_equal({ a!: "a", b!: "b" }, mapped) - assert_equal(::Hash, mapped.class) - end -end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index f4f0dd6b31..95fdcb8faf 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -31,8 +31,6 @@ class HashExtTest < ActiveSupport::TestCase def test_methods h = {} - assert_respond_to h, :transform_keys - assert_respond_to h, :transform_keys! assert_respond_to h, :deep_transform_keys assert_respond_to h, :deep_transform_keys! assert_respond_to h, :symbolize_keys @@ -49,18 +47,6 @@ class HashExtTest < ActiveSupport::TestCase assert_respond_to h, :except! end - def test_transform_keys - assert_equal @upcase_strings, @strings.transform_keys { |key| key.to_s.upcase } - assert_equal @upcase_strings, @symbols.transform_keys { |key| key.to_s.upcase } - assert_equal @upcase_strings, @mixed.transform_keys { |key| key.to_s.upcase } - end - - def test_transform_keys_not_mutates - transformed_hash = @mixed.dup - transformed_hash.transform_keys { |key| key.to_s.upcase } - assert_equal @mixed, transformed_hash - end - def test_deep_transform_keys assert_equal @nested_upcase_strings, @nested_symbols.deep_transform_keys { |key| key.to_s.upcase } assert_equal @nested_upcase_strings, @nested_strings.deep_transform_keys { |key| key.to_s.upcase } @@ -76,19 +62,6 @@ class HashExtTest < ActiveSupport::TestCase assert_equal @nested_mixed, transformed_hash end - def test_transform_keys! - assert_equal @upcase_strings, @symbols.dup.transform_keys! { |key| key.to_s.upcase } - assert_equal @upcase_strings, @strings.dup.transform_keys! { |key| key.to_s.upcase } - assert_equal @upcase_strings, @mixed.dup.transform_keys! { |key| key.to_s.upcase } - end - - def test_transform_keys_with_bang_mutates - transformed_hash = @mixed.dup - transformed_hash.transform_keys! { |key| key.to_s.upcase } - assert_equal @upcase_strings, transformed_hash - assert_equal({ :a => 1, "b" => 2 }, @mixed) - end - def test_deep_transform_keys! assert_equal @nested_upcase_strings, @nested_symbols.deep_dup.deep_transform_keys! { |key| key.to_s.upcase } assert_equal @nested_upcase_strings, @nested_strings.deep_dup.deep_transform_keys! { |key| key.to_s.upcase } @@ -337,30 +310,16 @@ class HashExtTest < ActiveSupport::TestCase assert_equal expected, merged end - def test_slice - original = { a: "x", b: "y", c: 10 } - expected = { a: "x", b: "y" } - - # Should return a new hash with only the given keys. - assert_equal expected, original.slice(:a, :b) - assert_not_equal expected, original - end - def test_slice_inplace original = { a: "x", b: "y", c: 10 } - expected = { c: 10 } + expected_return = { c: 10 } + expected_original = { a: "x", b: "y" } - # Should replace the hash with only the given keys. - assert_equal expected, original.slice!(:a, :b) - end - - def test_slice_with_an_array_key - original = { :a => "x", :b => "y", :c => 10, [:a, :b] => "an array key" } - expected = { [:a, :b] => "an array key", :c => 10 } + # Should return a hash containing the removed key/value pairs. + assert_equal expected_return, original.slice!(:a, :b) - # Should return a new hash with only the given keys when given an array key. - assert_equal expected, original.slice([:a, :b], :c) - assert_not_equal expected, original + # Should replace the hash with only the given keys. + assert_equal expected_original, original end def test_slice_inplace_with_an_array_key @@ -371,14 +330,6 @@ class HashExtTest < ActiveSupport::TestCase assert_equal expected, original.slice!([:a, :b], :c) end - def test_slice_with_splatted_keys - original = { :a => "x", :b => "y", :c => 10, [:a, :b] => "an array key" } - expected = { a: "x", b: "y" } - - # Should grab each of the splatted keys. - assert_equal expected, original.slice(*[:a, :b]) - end - def test_slice_bang_does_not_override_default hash = Hash.new(0) hash.update(a: 1, b: 2) diff --git a/activesupport/test/core_ext/load_error_test.rb b/activesupport/test/core_ext/load_error_test.rb index 126aa51cb4..6d3726e407 100644 --- a/activesupport/test/core_ext/load_error_test.rb +++ b/activesupport/test/core_ext/load_error_test.rb @@ -13,10 +13,9 @@ class TestLoadError < ActiveSupport::TestCase end def test_path - begin load "nor/this/one.rb" - rescue LoadError => e - assert_equal "nor/this/one.rb", e.path - end + load "nor/this/one.rb" + rescue LoadError => e + assert_equal "nor/this/one.rb", e.path end def test_is_missing_with_nil_path diff --git a/activesupport/test/evented_file_update_checker_test.rb b/activesupport/test/evented_file_update_checker_test.rb index a557608986..b2d5eb94c2 100644 --- a/activesupport/test/evented_file_update_checker_test.rb +++ b/activesupport/test/evented_file_update_checker_test.rb @@ -76,6 +76,34 @@ class EventedFileUpdateCheckerTest < ActiveSupport::TestCase Process.wait(pid) end + + test "updated should become true when nonexistent directory is added later" do + Dir.mktmpdir do |dir| + watched_dir = File.join(dir, "app") + unwatched_dir = File.join(dir, "node_modules") + not_exist_watched_dir = File.join(dir, "test") + + Dir.mkdir(watched_dir) + Dir.mkdir(unwatched_dir) + + checker = new_checker([], watched_dir => ".rb", not_exist_watched_dir => ".rb") { } + + FileUtils.touch(File.join(watched_dir, "a.rb")) + wait + assert_predicate checker, :updated? + assert checker.execute_if_updated + + Dir.mkdir(not_exist_watched_dir) + wait + assert_predicate checker, :updated? + assert checker.execute_if_updated + + FileUtils.touch(File.join(unwatched_dir, "a.rb")) + wait + assert_not_predicate checker, :updated? + assert_not checker.execute_if_updated + end + end end class EventedFileUpdateCheckerPathHelperTest < ActiveSupport::TestCase diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 8062873386..7589ffd0ea 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -21,20 +21,18 @@ class TestJSONEncoding < ActiveSupport::TestCase JSONTest::EncodingTestCases.constants.each do |class_tests| define_method("test_#{class_tests[0..-6].underscore}") do - begin - prev = ActiveSupport.use_standard_json_time_format - - standard_class_tests = /Standard/.match?(class_tests) - - ActiveSupport.escape_html_entities_in_json = !standard_class_tests - ActiveSupport.use_standard_json_time_format = standard_class_tests - JSONTest::EncodingTestCases.const_get(class_tests).each do |pair| - assert_equal pair.last, sorted_json(ActiveSupport::JSON.encode(pair.first)) - end - ensure - ActiveSupport.escape_html_entities_in_json = false - ActiveSupport.use_standard_json_time_format = prev + prev = ActiveSupport.use_standard_json_time_format + + standard_class_tests = /Standard/.match?(class_tests) + + ActiveSupport.escape_html_entities_in_json = !standard_class_tests + ActiveSupport.use_standard_json_time_format = standard_class_tests + JSONTest::EncodingTestCases.const_get(class_tests).each do |pair| + assert_equal pair.last, sorted_json(ActiveSupport::JSON.encode(pair.first)) end + ensure + ActiveSupport.escape_html_entities_in_json = false + ActiveSupport.use_standard_json_time_format = prev end end diff --git a/activesupport/test/share_lock_test.rb b/activesupport/test/share_lock_test.rb index 34479020e1..30a1ddad3f 100644 --- a/activesupport/test/share_lock_test.rb +++ b/activesupport/test/share_lock_test.rb @@ -115,68 +115,66 @@ class ShareLockTest < ActiveSupport::TestCase def test_exclusive_conflicting_purpose [true, false].each do |use_upgrading| with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch| - begin - together = Concurrent::CyclicBarrier.new(2) - conflicting_exclusive_threads = [ - Thread.new do - @lock.send(use_upgrading ? :sharing : :tap) do - together.wait - @lock.exclusive(purpose: :red, compatible: [:green, :purple]) { } - end - end, - Thread.new do - @lock.send(use_upgrading ? :sharing : :tap) do - together.wait - @lock.exclusive(purpose: :blue, compatible: [:green]) { } - end + together = Concurrent::CyclicBarrier.new(2) + conflicting_exclusive_threads = [ + Thread.new do + @lock.send(use_upgrading ? :sharing : :tap) do + together.wait + @lock.exclusive(purpose: :red, compatible: [:green, :purple]) { } + end + end, + Thread.new do + @lock.send(use_upgrading ? :sharing : :tap) do + together.wait + @lock.exclusive(purpose: :blue, compatible: [:green]) { } end - ] - - assert_threads_stuck conflicting_exclusive_threads # wait for threads to get into their respective `exclusive {}` blocks - - # This thread will be stuck as long as any other thread is in - # a sharing block. While it's blocked, it holds no lock, so it - # doesn't interfere with any other attempts. - no_purpose_thread = Thread.new do - @lock.exclusive { } end - assert_threads_stuck no_purpose_thread + ] - # This thread is compatible with both of the "primary" - # attempts above. It's initially stuck on the outer share - # lock, but as soon as that's released, it can run -- - # regardless of whether those threads hold share locks. - compatible_thread = Thread.new do - @lock.exclusive(purpose: :green, compatible: []) { } - end - assert_threads_stuck compatible_thread + assert_threads_stuck conflicting_exclusive_threads # wait for threads to get into their respective `exclusive {}` blocks - assert_threads_stuck conflicting_exclusive_threads + # This thread will be stuck as long as any other thread is in + # a sharing block. While it's blocked, it holds no lock, so it + # doesn't interfere with any other attempts. + no_purpose_thread = Thread.new do + @lock.exclusive { } + end + assert_threads_stuck no_purpose_thread + + # This thread is compatible with both of the "primary" + # attempts above. It's initially stuck on the outer share + # lock, but as soon as that's released, it can run -- + # regardless of whether those threads hold share locks. + compatible_thread = Thread.new do + @lock.exclusive(purpose: :green, compatible: []) { } + end + assert_threads_stuck compatible_thread - sharing_thread_release_latch.count_down + assert_threads_stuck conflicting_exclusive_threads - assert_threads_not_stuck compatible_thread # compatible thread is now able to squeak through + sharing_thread_release_latch.count_down - if use_upgrading - # The "primary" threads both each hold a share lock, and are - # mutually incompatible; they're still stuck. - assert_threads_stuck conflicting_exclusive_threads + assert_threads_not_stuck compatible_thread # compatible thread is now able to squeak through - # The thread without a specified purpose is also stuck; it's - # not compatible with anything. - assert_threads_stuck no_purpose_thread - else - # As the primaries didn't hold a share lock, as soon as the - # outer one was released, all the exclusive locks are free - # to be acquired in turn. + if use_upgrading + # The "primary" threads both each hold a share lock, and are + # mutually incompatible; they're still stuck. + assert_threads_stuck conflicting_exclusive_threads - assert_threads_not_stuck conflicting_exclusive_threads - assert_threads_not_stuck no_purpose_thread - end - ensure - conflicting_exclusive_threads.each(&:kill) - no_purpose_thread.kill + # The thread without a specified purpose is also stuck; it's + # not compatible with anything. + assert_threads_stuck no_purpose_thread + else + # As the primaries didn't hold a share lock, as soon as the + # outer one was released, all the exclusive locks are free + # to be acquired in turn. + + assert_threads_not_stuck conflicting_exclusive_threads + assert_threads_not_stuck no_purpose_thread end + ensure + conflicting_exclusive_threads.each(&:kill) + no_purpose_thread.kill end end end diff --git a/activesupport/test/time_travel_test.rb b/activesupport/test/time_travel_test.rb index 8c47f2cdc7..9c61ab0ab5 100644 --- a/activesupport/test/time_travel_test.rb +++ b/activesupport/test/time_travel_test.rb @@ -3,24 +3,25 @@ require "abstract_unit" require "active_support/core_ext/date_time" require "active_support/core_ext/numeric/time" +require "time_zone_test_helpers" class TimeTravelTest < ActiveSupport::TestCase + include TimeZoneTestHelpers + class TimeSubclass < ::Time; end class DateSubclass < ::Date; end class DateTimeSubclass < ::DateTime; end def test_time_helper_travel Time.stub(:now, Time.now) do - begin - expected_time = Time.now + 1.day - travel 1.day + expected_time = Time.now + 1.day + travel 1.day - assert_equal expected_time.to_s(:db), Time.now.to_s(:db) - assert_equal expected_time.to_date, Date.today - assert_equal expected_time.to_datetime.to_s(:db), DateTime.now.to_s(:db) - ensure - travel_back - end + assert_equal expected_time.to_s(:db), Time.now.to_s(:db) + assert_equal expected_time.to_date, Date.today + assert_equal expected_time.to_datetime.to_s(:db), DateTime.now.to_s(:db) + ensure + travel_back end end @@ -42,16 +43,14 @@ class TimeTravelTest < ActiveSupport::TestCase def test_time_helper_travel_to Time.stub(:now, Time.now) do - begin - expected_time = Time.new(2004, 11, 24, 01, 04, 44) - travel_to expected_time + expected_time = Time.new(2004, 11, 24, 01, 04, 44) + travel_to expected_time - assert_equal expected_time, Time.now - assert_equal Date.new(2004, 11, 24), Date.today - assert_equal expected_time.to_datetime, DateTime.now - ensure - travel_back - end + assert_equal expected_time, Time.now + assert_equal Date.new(2004, 11, 24), Date.today + assert_equal expected_time.to_datetime, DateTime.now + ensure + travel_back end end @@ -71,23 +70,35 @@ class TimeTravelTest < ActiveSupport::TestCase end end + def test_time_helper_travel_to_with_time_zone + with_env_tz "US/Eastern" do + with_tz_default ActiveSupport::TimeZone["UTC"] do + Time.stub(:now, Time.now) do + expected_time = 5.minutes.ago + + travel_to 5.minutes.ago do + assert_equal expected_time.to_s(:db), Time.zone.now.to_s(:db) + end + end + end + end + end + def test_time_helper_travel_back Time.stub(:now, Time.now) do - begin - expected_time = Time.new(2004, 11, 24, 01, 04, 44) + expected_time = Time.new(2004, 11, 24, 01, 04, 44) - travel_to expected_time - assert_equal expected_time, Time.now - assert_equal Date.new(2004, 11, 24), Date.today - assert_equal expected_time.to_datetime, DateTime.now - travel_back + travel_to expected_time + assert_equal expected_time, Time.now + assert_equal Date.new(2004, 11, 24), Date.today + assert_equal expected_time.to_datetime, DateTime.now + travel_back - assert_not_equal expected_time, Time.now - assert_not_equal Date.new(2004, 11, 24), Date.today - assert_not_equal expected_time.to_datetime, DateTime.now - ensure - travel_back - end + assert_not_equal expected_time, Time.now + assert_not_equal Date.new(2004, 11, 24), Date.today + assert_not_equal expected_time.to_datetime, DateTime.now + ensure + travel_back end end @@ -122,20 +133,18 @@ class TimeTravelTest < ActiveSupport::TestCase def test_time_helper_travel_to_with_subsequent_calls Time.stub(:now, Time.now) do - begin - initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44) - subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44) - assert_nothing_raised do - travel_to initial_expected_time - travel_to subsequent_expected_time + initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44) + subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44) + assert_nothing_raised do + travel_to initial_expected_time + travel_to subsequent_expected_time - assert_equal subsequent_expected_time, Time.now + assert_equal subsequent_expected_time, Time.now - travel_back - end - ensure travel_back end + ensure + travel_back end end diff --git a/guides/CHANGELOG.md b/guides/CHANGELOG.md index 516b643cb8..9f95e22245 100644 --- a/guides/CHANGELOG.md +++ b/guides/CHANGELOG.md @@ -2,9 +2,9 @@ *Xavier Noria* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/guides/CHANGELOG.md) for previous changes. diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 6b0554bb5f..3db46bc42e 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -2132,30 +2132,6 @@ The methods `second`, `third`, `fourth`, and `fifth` return the corresponding el NOTE: Defined in `active_support/core_ext/array/access.rb`. -### Adding Elements - -#### `prepend` - -This method is an alias of `Array#unshift`. - -```ruby -%w(a b c d).prepend('e') # => ["e", "a", "b", "c", "d"] -[].prepend(10) # => [10] -``` - -NOTE: Defined in `active_support/core_ext/array/prepend_and_append.rb`. - -#### `append` - -This method is an alias of `Array#<<`. - -```ruby -%w(a b c d).append('e') # => ["a", "b", "c", "d", "e"] -[].append([1,2]) # => [[1, 2]] -``` - -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. @@ -2646,48 +2622,6 @@ There's also the bang variant `except!` that removes keys in the very receiver. NOTE: Defined in `active_support/core_ext/hash/except.rb`. -#### `transform_keys` and `transform_keys!` - -The method `transform_keys` accepts a block and returns a hash that has applied the block operations to each of the keys in the receiver: - -```ruby -{nil => nil, 1 => 1, a: :a}.transform_keys { |key| key.to_s.upcase } -# => {"" => nil, "1" => 1, "A" => :a} -``` - -In case of key collision, one of the values will be chosen. The chosen value may not always be the same given the same hash: - -```ruby -{"a" => 1, a: 2}.transform_keys { |key| key.to_s.upcase } -# The result could either be -# => {"A"=>2} -# or -# => {"A"=>1} -``` - -This method may be useful for example to build specialized conversions. For instance `stringify_keys` and `symbolize_keys` use `transform_keys` to perform their key conversions: - -```ruby -def stringify_keys - transform_keys { |key| key.to_s } -end -... -def symbolize_keys - transform_keys { |key| key.to_sym rescue key } -end -``` - -There's also the bang variant `transform_keys!` that applies the block operations to keys in the very receiver. - -Besides that, one can use `deep_transform_keys` and `deep_transform_keys!` to perform the block operation on all the keys in the given hash and all the hashes nested into it. An example of the result is: - -```ruby -{nil => nil, 1 => 1, nested: {a: 3, 5 => 5}}.deep_transform_keys { |key| key.to_s.upcase } -# => {""=>nil, "1"=>1, "NESTED"=>{"A"=>3, "5"=>5}} -``` - -NOTE: Defined in `active_support/core_ext/hash/keys.rb`. - #### `stringify_keys` and `stringify_keys!` The method `stringify_keys` returns a hash that has a stringified version of the keys in the receiver. It does so by sending `to_s` to them: @@ -2795,26 +2729,7 @@ NOTE: Defined in `active_support/core_ext/hash/keys.rb`. ### Slicing -Ruby has built-in support for taking slices out of strings and arrays. Active Support extends slicing to hashes: - -```ruby -{a: 1, b: 2, c: 3}.slice(:a, :c) -# => {:a=>1, :c=>3} - -{a: 1, b: 2, c: 3}.slice(:b, :X) -# => {:b=>2} # non-existing keys are ignored -``` - -If the receiver responds to `convert_key` keys are normalized: - -```ruby -{a: 1, b: 2}.with_indifferent_access.slice("a") -# => {:a=>1} -``` - -NOTE. Slicing may come in handy for sanitizing option hashes with a white list of keys. - -There's also `slice!` which in addition to perform a slice in place returns what's removed: +The method `slice!` replaces the hash with only the given keys and returns a hash containing the removed key/value pairs. ```ruby hash = {a: 1, b: 2} diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index e2f558d74c..264c94326e 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -90,7 +90,7 @@ $ ruby -v ruby 2.5.0 ``` -Rails requires Ruby version 2.4.1 or later. If the version number returned is +Rails requires Ruby version 2.5.0 or later. If the version number returned is less than that number, you'll need to install a fresh copy of Ruby. TIP: To quickly install Ruby and Ruby on Rails on your system in Windows, you can use diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index e74985c5b0..2682c6ffd7 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -35,7 +35,7 @@ You can find a list of all released Rails versions [here](https://rubygems.org/g Rails generally stays close to the latest released Ruby version when it's released: -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. * Rails 5 requires Ruby 2.2.2 or newer. * Rails 4 prefers Ruby 2.0 and requires 1.9.3 or newer. * Rails 3.2.x is the last branch to support Ruby 1.8.7. diff --git a/rails.gemspec b/rails.gemspec index 6f3d99228f..936f74c64b 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Full-stack web application framework." s.description = "Ruby on Rails is a full-stack web framework optimized for programmer happiness and sustainable productivity. It encourages beautiful code by favoring convention over configuration." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.required_rubygems_version = ">= 1.8.11" s.license = "MIT" diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 59024b13ef..8e358b4293 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -241,9 +241,9 @@ *Benoit Tigeot* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/railties/CHANGELOG.md) for previous changes. diff --git a/railties/lib/rails/all.rb b/railties/lib/rails/all.rb index f5dccd2381..b4a9c4e65f 100644 --- a/railties/lib/rails/all.rb +++ b/railties/lib/rails/all.rb @@ -13,8 +13,6 @@ require "rails" rails/test_unit/railtie sprockets/railtie ).each do |railtie| - begin - require railtie - rescue LoadError - end + require railtie +rescue LoadError end diff --git a/railties/lib/rails/command/behavior.rb b/railties/lib/rails/command/behavior.rb index 718e2d9ab2..7f32b04cf1 100644 --- a/railties/lib/rails/command/behavior.rb +++ b/railties/lib/rails/command/behavior.rb @@ -56,12 +56,10 @@ module Rails def lookup! $LOAD_PATH.each do |base| Dir[File.join(base, *file_lookup_paths)].each do |path| - begin - path = path.sub("#{base}/", "") - require path - rescue Exception - # No problem - end + path = path.sub("#{base}/", "") + require path + rescue Exception + # No problem end end end diff --git a/railties/lib/rails/ruby_version_check.rb b/railties/lib/rails/ruby_version_check.rb index b2d44d9b8e..ab5339bf24 100644 --- a/railties/lib/rails/ruby_version_check.rb +++ b/railties/lib/rails/ruby_version_check.rb @@ -1,15 +1,15 @@ # frozen_string_literal: true -if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("2.4.1") && RUBY_ENGINE == "ruby" +if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("2.5.0") && RUBY_ENGINE == "ruby" desc = defined?(RUBY_DESCRIPTION) ? RUBY_DESCRIPTION : "ruby #{RUBY_VERSION} (#{RUBY_RELEASE_DATE})" abort <<-end_message - Rails 6 requires Ruby 2.4.1 or newer. + Rails 6 requires Ruby 2.5.0 or newer. You're running #{desc} - Please upgrade to Ruby 2.4.1 or newer to continue. + Please upgrade to Ruby 2.5.0 or newer to continue. end_message end diff --git a/railties/railties.gemspec b/railties/railties.gemspec index 91407ee1ae..1bfb0dfe48 100644 --- a/railties/railties.gemspec +++ b/railties/railties.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Tools for creating, working with, and running Rails applications." s.description = "Rails internals: application bootup, plugins, generators, and rake tasks." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 9fff764eca..886fb0f843 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1493,14 +1493,12 @@ module ApplicationTests end test "config.session_store with :active_record_store with activerecord-session_store gem" do - begin - make_basic_app do |application| - ActionDispatch::Session::ActiveRecordStore = Class.new(ActionDispatch::Session::CookieStore) - application.config.session_store :active_record_store - end - ensure - ActionDispatch::Session.send :remove_const, :ActiveRecordStore + make_basic_app do |application| + ActionDispatch::Session::ActiveRecordStore = Class.new(ActionDispatch::Session::CookieStore) + application.config.session_store :active_record_store end + ensure + ActionDispatch::Session.send :remove_const, :ActiveRecordStore end test "config.session_store with :active_record_store without activerecord-session_store gem" do diff --git a/railties/test/application/console_test.rb b/railties/test/application/console_test.rb index 29f8b1e3d9..e74daccbe7 100644 --- a/railties/test/application/console_test.rb +++ b/railties/test/application/console_test.rb @@ -25,7 +25,7 @@ class ConsoleTest < ActiveSupport::TestCase end def test_app_method_should_return_integration_session - TestHelpers::Rack.send :remove_method, :app + TestHelpers::Rack.remove_method :app load_environment console_session = irb_context.app assert_instance_of ActionDispatch::Integration::Session, console_session diff --git a/railties/test/application/initializers/frameworks_test.rb b/railties/test/application/initializers/frameworks_test.rb index 1530ea82d6..3cd4b8fe33 100644 --- a/railties/test/application/initializers/frameworks_test.rb +++ b/railties/test/application/initializers/frameworks_test.rb @@ -230,35 +230,31 @@ module ApplicationTests end test "active record establish_connection uses Rails.env if DATABASE_URL is not set" do - begin - require "#{app_path}/config/environment" - orig_database_url = ENV.delete("DATABASE_URL") - orig_rails_env, Rails.env = Rails.env, "development" - ActiveRecord::Base.establish_connection - assert ActiveRecord::Base.connection - assert_match(/#{ActiveRecord::Base.configurations[Rails.env]['database']}/, ActiveRecord::Base.connection_config[:database]) - ensure - ActiveRecord::Base.remove_connection - ENV["DATABASE_URL"] = orig_database_url if orig_database_url - Rails.env = orig_rails_env if orig_rails_env - end + require "#{app_path}/config/environment" + orig_database_url = ENV.delete("DATABASE_URL") + orig_rails_env, Rails.env = Rails.env, "development" + ActiveRecord::Base.establish_connection + assert ActiveRecord::Base.connection + assert_match(/#{ActiveRecord::Base.configurations[Rails.env]['database']}/, ActiveRecord::Base.connection_config[:database]) + ensure + ActiveRecord::Base.remove_connection + ENV["DATABASE_URL"] = orig_database_url if orig_database_url + Rails.env = orig_rails_env if orig_rails_env end test "active record establish_connection uses DATABASE_URL even if Rails.env is set" do - begin - require "#{app_path}/config/environment" - orig_database_url = ENV.delete("DATABASE_URL") - orig_rails_env, Rails.env = Rails.env, "development" - database_url_db_name = "db/database_url_db.sqlite3" - ENV["DATABASE_URL"] = "sqlite3:#{database_url_db_name}" - ActiveRecord::Base.establish_connection - assert ActiveRecord::Base.connection - assert_match(/#{database_url_db_name}/, ActiveRecord::Base.connection_config[:database]) - ensure - ActiveRecord::Base.remove_connection - ENV["DATABASE_URL"] = orig_database_url if orig_database_url - Rails.env = orig_rails_env if orig_rails_env - end + require "#{app_path}/config/environment" + orig_database_url = ENV.delete("DATABASE_URL") + orig_rails_env, Rails.env = Rails.env, "development" + database_url_db_name = "db/database_url_db.sqlite3" + ENV["DATABASE_URL"] = "sqlite3:#{database_url_db_name}" + ActiveRecord::Base.establish_connection + assert ActiveRecord::Base.connection + assert_match(/#{database_url_db_name}/, ActiveRecord::Base.connection_config[:database]) + ensure + ActiveRecord::Base.remove_connection + ENV["DATABASE_URL"] = orig_database_url if orig_database_url + Rails.env = orig_rails_env if orig_rails_env end test "connections checked out during initialization are returned to the pool" do diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index 039987ac8c..84ac2f057e 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -321,13 +321,12 @@ module ApplicationTests end test "db:setup loads schema and seeds database" do - begin - @old_rails_env = ENV["RAILS_ENV"] - @old_rack_env = ENV["RACK_ENV"] - ENV.delete "RAILS_ENV" - ENV.delete "RACK_ENV" + @old_rails_env = ENV["RAILS_ENV"] + @old_rack_env = ENV["RACK_ENV"] + ENV.delete "RAILS_ENV" + ENV.delete "RACK_ENV" - app_file "db/schema.rb", <<-RUBY + app_file "db/schema.rb", <<-RUBY ActiveRecord::Schema.define(version: "1") do create_table :users do |t| t.string :name @@ -335,16 +334,15 @@ module ApplicationTests end RUBY - app_file "db/seeds.rb", <<-RUBY - puts ActiveRecord::Base.connection_config[:database] - RUBY + app_file "db/seeds.rb", <<-RUBY + puts ActiveRecord::Base.connection_config[:database] + RUBY - database_path = rails("db:setup") - assert_equal "development.sqlite3", File.basename(database_path.strip) - ensure - ENV["RAILS_ENV"] = @old_rails_env - ENV["RACK_ENV"] = @old_rack_env - end + database_path = rails("db:setup") + assert_equal "development.sqlite3", File.basename(database_path.strip) + ensure + ENV["RAILS_ENV"] = @old_rails_env + ENV["RACK_ENV"] = @old_rack_env end test "db:setup sets ar_internal_metadata" do diff --git a/railties/test/commands/dbconsole_test.rb b/railties/test/commands/dbconsole_test.rb index 5e3eca6585..ce048ac527 100644 --- a/railties/test/commands/dbconsole_test.rb +++ b/railties/test/commands/dbconsole_test.rb @@ -308,11 +308,9 @@ class Rails::DBConsoleTest < ActiveSupport::TestCase def capture_abort @aborted = false @output = capture(:stderr) do - begin - yield - rescue SystemExit - @aborted = true - end + yield + rescue SystemExit + @aborted = true end end diff --git a/railties/test/commands/routes_test.rb b/railties/test/commands/routes_test.rb index 693e532c5b..9efab27b38 100644 --- a/railties/test/commands/routes_test.rb +++ b/railties/test/commands/routes_test.rb @@ -161,19 +161,18 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase end test "rails routes with expanded option" do - begin - previous_console_winsize = IO.console.winsize - IO.console.winsize = [0, 27] + previous_console_winsize = IO.console.winsize + IO.console.winsize = [0, 27] - app_file "config/routes.rb", <<-RUBY + app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get '/cart', to: 'cart#show' end RUBY - output = run_routes_command(["--expanded"]) + output = run_routes_command(["--expanded"]) - assert_equal <<~MESSAGE, output + assert_equal <<~MESSAGE, output --[ Route 1 ]-------------- Prefix | cart Verb | GET @@ -205,9 +204,8 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase URI | /rails/active_storage/direct_uploads(.:format) Controller#Action | active_storage/direct_uploads#create MESSAGE - ensure - IO.console.winsize = previous_console_winsize - end + ensure + IO.console.winsize = previous_console_winsize end private diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb index 4f2894e71e..4b9878187b 100644 --- a/railties/test/generators/api_app_generator_test.rb +++ b/railties/test/generators/api_app_generator_test.rb @@ -14,9 +14,9 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase super Kernel.silence_warnings do - Thor::Base.shell.send(:attr_accessor, :always_force) + Thor::Base.shell.attr_accessor :always_force @shell = Thor::Base.shell.new - @shell.send(:always_force=, true) + @shell.always_force = true end end diff --git a/railties/test/generators/helper_generator_test.rb b/railties/test/generators/helper_generator_test.rb index 5d6b6f8f67..192839799e 100644 --- a/railties/test/generators/helper_generator_test.rb +++ b/railties/test/generators/helper_generator_test.rb @@ -30,12 +30,10 @@ class HelperGeneratorTest < Rails::Generators::TestCase require "#{destination_root}/app/helpers/products_helper" assert_nothing_raised do - begin - run_generator ["admin::products"] - ensure - # cleanup - Object.send(:remove_const, :ProductsHelper) - end + run_generator ["admin::products"] + ensure + # cleanup + Object.send(:remove_const, :ProductsHelper) end end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index 2dda856f25..7441ab0603 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -10,9 +10,9 @@ module SharedGeneratorTests Rails::Generators::AppGenerator.instance_variable_set("@desc", nil) Kernel.silence_warnings do - Thor::Base.shell.send(:attr_accessor, :always_force) + Thor::Base.shell.attr_accessor :always_force @shell = Thor::Base.shell.new - @shell.send(:always_force=, true) + @shell.always_force = true end end diff --git a/railties/test/railties/railtie_test.rb b/railties/test/railties/railtie_test.rb index 7c3d1e3759..b9725ca0ad 100644 --- a/railties/test/railties/railtie_test.rb +++ b/railties/test/railties/railtie_test.rb @@ -25,12 +25,10 @@ module RailtiesTest end test "Railtie provides railtie_name" do - begin - class ::FooBarBaz < Rails::Railtie ; end - assert_equal "foo_bar_baz", FooBarBaz.railtie_name - ensure - Object.send(:remove_const, :"FooBarBaz") - end + class ::FooBarBaz < Rails::Railtie ; end + assert_equal "foo_bar_baz", FooBarBaz.railtie_name + ensure + Object.send(:remove_const, :"FooBarBaz") end test "railtie_name can be set manually" do @@ -203,14 +201,12 @@ module RailtiesTest end test "we can change our environment if we want to" do - begin - original_env = Rails.env - Rails.env = "foo" - assert_equal("foo", Rails.env) - ensure - Rails.env = original_env - assert_equal(original_env, Rails.env) - end + original_env = Rails.env + Rails.env = "foo" + assert_equal("foo", Rails.env) + ensure + Rails.env = original_env + assert_equal(original_env, Rails.env) end end end diff --git a/railties/test/secrets_test.rb b/railties/test/secrets_test.rb index 06877bc76a..133d851819 100644 --- a/railties/test/secrets_test.rb +++ b/railties/test/secrets_test.rb @@ -35,15 +35,13 @@ class Rails::SecretsTest < ActiveSupport::TestCase test "reading with ENV variable" do run_secrets_generator do - begin - old_key = ENV["RAILS_MASTER_KEY"] - ENV["RAILS_MASTER_KEY"] = IO.binread("config/secrets.yml.key").strip - FileUtils.rm("config/secrets.yml.key") - - assert_match "# production:\n# external_api_key:", Rails::Secrets.read - ensure - ENV["RAILS_MASTER_KEY"] = old_key - end + old_key = ENV["RAILS_MASTER_KEY"] + ENV["RAILS_MASTER_KEY"] = IO.binread("config/secrets.yml.key").strip + FileUtils.rm("config/secrets.yml.key") + + assert_match "# production:\n# external_api_key:", Rails::Secrets.read + ensure + ENV["RAILS_MASTER_KEY"] = old_key end end |