diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/inspector.rb | 2 | ||||
| -rw-r--r-- | actionview/lib/action_view/helpers/number_helper.rb | 5 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/merger.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/cases/connection_pool_test.rb | 51 | ||||
| -rw-r--r-- | activerecord/test/cases/reaper_test.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/merging_test.rb | 10 | ||||
| -rw-r--r-- | activesupport/lib/active_support/number_helper.rb | 5 | ||||
| -rw-r--r-- | guides/source/contributing_to_ruby_on_rails.md | 15 | ||||
| -rw-r--r-- | railties/test/commands/routes_test.rb | 86 |
9 files changed, 150 insertions, 32 deletions
diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index cba49d1a0b..413e524ef6 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -83,7 +83,7 @@ module ActionDispatch private def normalize_filter(filter) if filter[:controller] - { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } + { controller: /#{filter[:controller].underscore.sub(/_?controller\z/, "")}/ } elsif filter[:grep] { controller: /#{filter[:grep]}/, action: /#{filter[:grep]}/, verb: /#{filter[:grep]}/, name: /#{filter[:grep]}/, path: /#{filter[:grep]}/ } diff --git a/actionview/lib/action_view/helpers/number_helper.rb b/actionview/lib/action_view/helpers/number_helper.rb index 4b53b8fe6e..35206b7e48 100644 --- a/actionview/lib/action_view/helpers/number_helper.rb +++ b/actionview/lib/action_view/helpers/number_helper.rb @@ -100,6 +100,9 @@ module ActionView # absolute value of the number. # * <tt>:raise</tt> - If true, raises +InvalidNumberError+ when # the argument is invalid. + # * <tt>:strip_insignificant_zeros</tt> - If +true+ removes + # insignificant zeros after the decimal separator (defaults to + # +false+). # # ==== Examples # @@ -117,6 +120,8 @@ module ActionView # # => R$1234567890,50 # number_to_currency(1234567890.50, unit: "R$", separator: ",", delimiter: "", format: "%n %u") # # => 1234567890,50 R$ + # number_to_currency(1234567890.50, strip_insignificant_zeros: true) + # # => "$1,234,567,890.5" def number_to_currency(number, options = {}) delegate_number_helper_method(:number_to_currency, number, options) end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 10e4779ca4..07b16a0740 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -152,10 +152,10 @@ module ActiveRecord def merge_multi_values if other.reordering_value # override any order specified in the original relation - relation.reorder! other.order_values + relation.reorder!(*other.order_values) elsif other.order_values.any? # merge in order_values from relation - relation.order! other.order_values + relation.order!(*other.order_values) end extensions = other.extensions - relation.extensions diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 9ac03629c3..6aecf5fa35 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -91,7 +91,9 @@ module ActiveRecord end def test_full_pool_exception + @pool.checkout_timeout = 0.001 # no need to delay test suite by waiting the whole full default timeout @pool.size.times { assert @pool.checkout } + assert_raises(ConnectionTimeoutError) do @pool.checkout end @@ -156,6 +158,48 @@ module ActiveRecord @pool.connections.each { |conn| conn.close if conn.in_use? } end + def test_idle_timeout_configuration + @pool.disconnect! + spec = ActiveRecord::Base.connection_pool.spec + spec.config.merge!(idle_timeout: "0.02") + @pool = ConnectionPool.new(spec) + idle_conn = @pool.checkout + @pool.checkin(idle_conn) + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 0.01 + ) + + @pool.flush + assert_equal 1, @pool.connections.length + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 0.02 + ) + + @pool.flush + assert_equal 0, @pool.connections.length + end + + def test_disable_flush + @pool.disconnect! + spec = ActiveRecord::Base.connection_pool.spec + spec.config.merge!(idle_timeout: -5) + @pool = ConnectionPool.new(spec) + idle_conn = @pool.checkout + @pool.checkin(idle_conn) + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 1 + ) + + @pool.flush + assert_equal 1, @pool.connections.length + end + def test_flush idle_conn = @pool.checkout recent_conn = @pool.checkout @@ -166,9 +210,10 @@ module ActiveRecord assert_equal 3, @pool.connections.length - def idle_conn.seconds_idle - 1000 - end + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 1000 + ) @pool.flush(30) diff --git a/activerecord/test/cases/reaper_test.rb b/activerecord/test/cases/reaper_test.rb index b034fe3e3b..b630f782bc 100644 --- a/activerecord/test/cases/reaper_test.rb +++ b/activerecord/test/cases/reaper_test.rb @@ -61,9 +61,9 @@ module ActiveRecord def test_reaping_frequency_configuration spec = ActiveRecord::Base.connection_pool.spec.dup - spec.config[:reaping_frequency] = 100 + spec.config[:reaping_frequency] = "10.01" pool = ConnectionPool.new spec - assert_equal 100, pool.reaper.frequency + assert_equal 10.01, pool.reaper.frequency end def test_connection_pool_starts_reaper diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index f53ef1fe35..6e7998d15a 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -121,6 +121,16 @@ class RelationMergingTest < ActiveRecord::TestCase relation = relation.merge(Post.from("posts")) assert_not_empty relation.from_clause end + + def test_merging_with_order_with_binds + relation = Post.all.merge(Post.order([Arel.sql("title LIKE ?"), "%suffix"])) + assert_equal ["title LIKE '%suffix'"], relation.order_values + end + + def test_merging_with_order_without_binds + relation = Post.all.merge(Post.order(Arel.sql("title LIKE '%?'"))) + assert_equal ["title LIKE '%?'"], relation.order_values + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 8fd6e932f1..c75ad52b0c 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -85,6 +85,9 @@ module ActiveSupport # number given by <tt>:format</tt>). Accepts the same fields # than <tt>:format</tt>, except <tt>%n</tt> is here the # absolute value of the number. + # * <tt>:strip_insignificant_zeros</tt> - If +true+ removes + # insignificant zeros after the decimal separator (defaults to + # +false+). # # ==== Examples # @@ -100,6 +103,8 @@ module ActiveSupport # # => "£1234567890,50" # number_to_currency(1234567890.50, unit: '£', separator: ',', delimiter: '', format: '%n %u') # # => "1234567890,50 £" + # number_to_currency(1234567890.50, strip_insignificant_zeros: true) + # # => "$1,234,567,890.5" def number_to_currency(number, options = {}) NumberToCurrencyConverter.convert(number, options) end diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 3147b00f3b..b5e40aa40f 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -239,7 +239,6 @@ Now get busy and add/edit code. You're on your branch now, so you can write what * Include tests that fail without your code, and pass with it. * Update the (surrounding) documentation, examples elsewhere, and the guides: whatever is affected by your contribution. - TIP: Changes that are cosmetic in nature and do not add anything substantial to the stability, functionality, or testability of Rails will generally not be accepted (read more about [our rationales behind this decision](https://github.com/rails/rails/pull/13771#issuecomment-32746700)). #### Follow the Coding Conventions @@ -254,12 +253,24 @@ Rails follows a simple set of coding style conventions: * Prefer class << self over self.method for class methods. * Use `my_method(my_arg)` not `my_method( my_arg )` or `my_method my_arg`. * Use `a = b` and not `a=b`. -* Use assert_not methods instead of refute. +* Use assert\_not methods instead of refute. * Prefer `method { do_stuff }` instead of `method{do_stuff}` for single-line blocks. * Follow the conventions in the source you see used already. The above are guidelines - please use your best judgment in using them. +Additionally, we have [RuboCop](https://www.rubocop.org/) rules defined to codify some of our coding conventions. You can run RuboCop locally against the file that you have modified before submitting a pull request: + +```bash +$ rubocop actionpack/lib/action_controller/metal/strong_parameters.rb +Inspecting 1 file +. + +1 file inspected, no offenses detected +``` + +For `rails-ujs` CoffeeScript and JavaScript files, you can run `npm run lint` in `actionview` folder. + ### Benchmark Your Code For changes that might have an impact on performance, please benchmark your diff --git a/railties/test/commands/routes_test.rb b/railties/test/commands/routes_test.rb index 77ed2bda61..693e532c5b 100644 --- a/railties/test/commands/routes_test.rb +++ b/railties/test/commands/routes_test.rb @@ -13,20 +13,33 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do resource :post + resource :user_permission end RUBY - expected_output = [" Prefix Verb URI Pattern Controller#Action", - " new_post GET /post/new(.:format) posts#new", - "edit_post GET /post/edit(.:format) posts#edit", - " post GET /post(.:format) posts#show", - " PATCH /post(.:format) posts#update", - " PUT /post(.:format) posts#update", - " DELETE /post(.:format) posts#destroy", - " POST /post(.:format) posts#create\n"].join("\n") + expected_post_output = [" Prefix Verb URI Pattern Controller#Action", + " new_post GET /post/new(.:format) posts#new", + "edit_post GET /post/edit(.:format) posts#edit", + " post GET /post(.:format) posts#show", + " PATCH /post(.:format) posts#update", + " PUT /post(.:format) posts#update", + " DELETE /post(.:format) posts#destroy", + " POST /post(.:format) posts#create\n"].join("\n") output = run_routes_command(["-c", "PostController"]) - assert_equal expected_output, output + assert_equal expected_post_output, output + + expected_perm_output = [" Prefix Verb URI Pattern Controller#Action", + " new_user_permission GET /user_permission/new(.:format) user_permissions#new", + "edit_user_permission GET /user_permission/edit(.:format) user_permissions#edit", + " user_permission GET /user_permission(.:format) user_permissions#show", + " PATCH /user_permission(.:format) user_permissions#update", + " PUT /user_permission(.:format) user_permissions#update", + " DELETE /user_permission(.:format) user_permissions#destroy", + " POST /user_permission(.:format) user_permissions#create\n"].join("\n") + + output = run_routes_command(["-c", "UserPermissionController"]) + assert_equal expected_perm_output, output end test "rails routes with global search key" do @@ -64,17 +77,30 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase Rails.application.routes.draw do get '/cart', to: 'cart#show' get '/basketball', to: 'basketball#index' + get '/user_permission', to: 'user_permission#index' end RUBY + expected_cart_output = "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n" output = run_routes_command(["-c", "cart"]) - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal expected_cart_output, output output = run_routes_command(["-c", "Cart"]) - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal expected_cart_output, output output = run_routes_command(["-c", "CartController"]) - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal expected_cart_output, output + + expected_perm_output = [" Prefix Verb URI Pattern Controller#Action", + "user_permission GET /user_permission(.:format) user_permission#index\n"].join("\n") + output = run_routes_command(["-c", "user_permission"]) + assert_equal expected_perm_output, output + + output = run_routes_command(["-c", "UserPermission"]) + assert_equal expected_perm_output, output + + output = run_routes_command(["-c", "UserPermissionController"]) + assert_equal expected_perm_output, output end test "rails routes with namespaced controller search key" do @@ -82,24 +108,40 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase Rails.application.routes.draw do namespace :admin do resource :post + resource :user_permission end end RUBY - expected_output = [" Prefix Verb URI Pattern Controller#Action", - " new_admin_post GET /admin/post/new(.:format) admin/posts#new", - "edit_admin_post GET /admin/post/edit(.:format) admin/posts#edit", - " admin_post GET /admin/post(.:format) admin/posts#show", - " PATCH /admin/post(.:format) admin/posts#update", - " PUT /admin/post(.:format) admin/posts#update", - " DELETE /admin/post(.:format) admin/posts#destroy", - " POST /admin/post(.:format) admin/posts#create\n"].join("\n") + expected_post_output = [" Prefix Verb URI Pattern Controller#Action", + " new_admin_post GET /admin/post/new(.:format) admin/posts#new", + "edit_admin_post GET /admin/post/edit(.:format) admin/posts#edit", + " admin_post GET /admin/post(.:format) admin/posts#show", + " PATCH /admin/post(.:format) admin/posts#update", + " PUT /admin/post(.:format) admin/posts#update", + " DELETE /admin/post(.:format) admin/posts#destroy", + " POST /admin/post(.:format) admin/posts#create\n"].join("\n") output = run_routes_command(["-c", "Admin::PostController"]) - assert_equal expected_output, output + assert_equal expected_post_output, output output = run_routes_command(["-c", "PostController"]) - assert_equal expected_output, output + assert_equal expected_post_output, output + + expected_perm_output = [" Prefix Verb URI Pattern Controller#Action", + " new_admin_user_permission GET /admin/user_permission/new(.:format) admin/user_permissions#new", + "edit_admin_user_permission GET /admin/user_permission/edit(.:format) admin/user_permissions#edit", + " admin_user_permission GET /admin/user_permission(.:format) admin/user_permissions#show", + " PATCH /admin/user_permission(.:format) admin/user_permissions#update", + " PUT /admin/user_permission(.:format) admin/user_permissions#update", + " DELETE /admin/user_permission(.:format) admin/user_permissions#destroy", + " POST /admin/user_permission(.:format) admin/user_permissions#create\n"].join("\n") + + output = run_routes_command(["-c", "Admin::UserPermissionController"]) + assert_equal expected_perm_output, output + + output = run_routes_command(["-c", "UserPermissionController"]) + assert_equal expected_perm_output, output end test "rails routes displays message when no routes are defined" do |