Commit Graph

84598 Commits

Author SHA1 Message Date
Koichi ITO eeeb0b1b7a Enable `Minitest/AssertRaisesWithRegexpArgument` cop
This PR enables `Minitest/AssertRaisesWithRegexpArgument` cop
and it suppresses the new warning below.

```console
% bundle exec rubocop
(snip)

Offenses:

activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb:111:9: C: Minitest/AssertRaisesWithRegexpArgument:
Do not pass regular expression literals to assert_raises. Test the resulting exception.
assert_raises(ActiveRecord::StatementInvalid, /TypeError/) do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb:628:9: C: Minitest/AssertRaisesWithRegexpArgument:
Do not pass regular expression literals to assert_raises. Test the resulting exception.
assert_raises(ActiveRecord::StatementInvalid, /SQLite3::ReadOnlyException/) do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
railties/test/application/rake/multi_dbs_test.rb:307:13: C: Minitest/AssertRaisesWithRegexpArgument:
Do not pass regular expression literals to assert_raises. Test the resulting exception.
assert_raises RuntimeError, /You're using a multiple database application/ do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
railties/test/application/rake/multi_dbs_test.rb:311:13: C: Minitest/AssertRaisesWithRegexpArgument:
Do not pass regular expression literals to assert_raises. Test the resulting exception.
assert_raises RuntimeError, /You're using a multiple database application/ do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
railties/test/application/rake/multi_dbs_test.rb:336:13: C: Minitest/AssertRaisesWithRegexpArgument:
Do not pass regular expression literals to assert_raises. Test the resulting exception.
assert_raises RuntimeError, /You're using a multiple database application/ do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
railties/test/application/rake/multi_dbs_test.rb:361:13: C: Minitest/AssertRaisesWithRegexpArgument:
Do not pass regular expression literals to assert_raises. Test the resulting exception.
assert_raises RuntimeError, /You're using a multiple database application/ do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

The last argument of `assert_raises` is a custom message string to help explain failures.
So, it's not the argument that `exception.message` is compared to.
`assert_raises` returns a raised exception and can be used to match against a regular expression.

And it updates the dependency version of rubocop-minitest in the Gemfile.
Because `Minitest/AssertRaisesWithRegexpArgument` cop was introduced in minitest 0.22.
https://github.com/rubocop/rubocop-minitest/releases/tag/v0.22.0
2022-09-08 17:44:40 +09:00
Eileen M. Uchitelle 86a280a347
Merge pull request #45961 from eileencodes/make-connection_pool_list-explicit
Make connection_pool_list take an explicit argument
2022-09-07 15:15:48 -04:00
eileencodes 69396b75c9
Make connection_pool_list take an explicit argument
Following on #45924 I realized that `all_connection_pools` and
`connection_pool_list` don't make much sense as separate methods and
should follow the same deprecation as the other methods on the handler
here. So this PR deprecates `all_connection_pools` in favor of
`connection_pool_list` with an explicit argument of the role or `:all`.
Passing `nil` will throw a deprecation warning to get applications to
be explicit about behavior they expect.
2022-09-07 14:44:18 -04:00
John Hawthorn f5d676cd97
Merge pull request #45957 from jhawthorn/immutable_path_set
Make ActionView::PathSet immutable
2022-09-07 11:40:53 -07:00
Eileen M. Uchitelle cebd9ab11b
Merge pull request #45924 from eileencodes/fix-bug-in-connection-handler-methods
Fix bug in connection handler methods using all pools
2022-09-07 14:04:40 -04:00
eileencodes 74cb960e66
Fix bug in connection handler methods using all pools
Previously when I implemented multiple database roles in Rails there
were two handlers so it made sense for the methods
`active_connections?`, `clear_active_connections!`,
`clear_reloadable_connections!`, `clear_all_connections!`, and
`flush_idle_connections!` to only operate on the current (or passed)
role and not all pools regardless of role. When I removed this and moved
all the pools to the handler maintained by a pool manager, I left these
methods as-is to preserve the original behavior.

This made sense because I thought these methods were only called by
applications and not called by Rails. I realized yesterday that some of
these methods (`flush_idle_connections!`, `clear_active_connections!`,
and `clear_reloadable_connections!` are all called on boot by the
Active Record railtie.

Unfortunately this means that applications using multiple databases
aren't getting connections flushed or cleared on boot for any connection
but the writing ones.

The change here continues existing behavior if a role like reading is
passed in directly. Otherwise if the role is `nil` (which is the new
default` we fall back to all connections and issue a deprecation
warning. This will be the new default behavior in the future. In order
to easily allow turning off the deprecation warning I've added an `:all`
argument that will use all pools but no warning. The deprecation warning
will only fire if there is more than one role in the pool manager,
otherwise we assume prior behavior.

This bug would have only affected applications with more than one role
and only when these methods are called outside the context of a
`connected_to` block. These methods no longer consider the set
`current_role` and applications need to be explicit if they don't want
these methods to operate on all pools.
2022-09-07 13:42:23 -04:00
Jean Boussier 049ca510c1
Merge pull request #45953 from byroot/bump-resque-redis-5.0
Update redis and resque
2022-09-07 19:35:34 +02:00
Jonathan Hefner c3ee7c5a3b
Merge pull request #45948 from jonathanhefner/word_wrap-performance
Improve `word_wrap` performance
2022-09-07 12:04:58 -05:00
John Hawthorn 0371317e25 Make ActionView::PathSet immutable
Previously we would mutate the PathSet in order to implement
{append,prepend}_view_path.

This removes the mutation methods we had and instead constructs a new
PathSet whenever we are modifying it. This should allow flexibility in
the classes holding a view_path to know that their view paths isn't
modified (for example, to allow caching).
2022-09-07 09:40:52 -07:00
Jean Boussier 545482650b Update redis and resque
Fix: https://github.com/rails/rails/issues/45913
2022-09-07 15:10:43 +02:00
John Hawthorn e65d41a47a View path strict type casting 2022-09-06 19:07:53 -07:00
Jonathan Hefner 8265d68796 Improve word_wrap performance
This improves `word_wrap` performance and reduces allocations.

Benchmark:

  ```ruby
  # frozen_string_literal: true
  require "benchmark/ips"
  require "benchmark/memory"

  def before(text, line_width: 80, break_sequence: "\n")
    text.split("\n").collect! do |line|
      line.length > line_width ? line.gsub(/(.{1,#{line_width}})(\s+|$)/, "\\1#{break_sequence}").chomp!(break_sequence) : line
    end * break_sequence
  end

  def after(text, line_width: 80, break_sequence: "\n")
    pattern = /(.{1,#{line_width}})(?:[^\S\n]+\n?|\n*\Z|\n)|\n/
    text.gsub(pattern, "\\1#{break_sequence}").chomp!(break_sequence)
  end

  TEXT = <<~LOREM
    Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
    eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad
    minim veniam, quis nostrud exercitation ullamco laboris nisi ut
    aliquip ex ea commodo consequat. Duis aute irure dolor in
    reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
    pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
    culpa qui officia deserunt mollit anim id est laborum.
  LOREM

  Benchmark.ips do |x|
    x.report("before") { before(TEXT, line_width: 50) }
    x.report("after") { after(TEXT, line_width: 50) }
    x.compare!
  end

  Benchmark.memory do |x|
    x.report("before") { before(TEXT, line_width: 50) }
    x.report("after") { after(TEXT, line_width: 50) }
    x.compare!
  end
  ```

Results:

  ```
  Warming up --------------------------------------
                before   858.000  i/100ms
                 after     2.215k i/100ms
  Calculating -------------------------------------
                before      8.540k (± 1.3%) i/s -     42.900k in   5.024157s
                 after     22.370k (± 1.1%) i/s -    112.965k in   5.050565s

  Comparison:
                 after:    22369.8 i/s
                before:     8540.2 i/s - 2.62x  (± 0.00) slower
  ```

  ```
  Calculating -------------------------------------
                before    13.993k memsize (     0.000  retained)
                         122.000  objects (     0.000  retained)
                          34.000  strings (     0.000  retained)
                 after     5.423k memsize (     0.000  retained)
                          41.000  objects (     0.000  retained)
                          20.000  strings (     0.000  retained)

  Comparison:
                 after:       5423 allocated
                before:      13993 allocated - 2.58x more
  ```
2022-09-06 16:10:31 -05:00
Jeremy Daer 01d345c4d6
Merge pull request #45739 from basecamp/content-attachment-update
update ContentAttachment so that it works with "content" attributes
2022-09-06 11:15:10 -07:00
Jonathan Hefner 9a439f6332
Merge pull request #45938 from ttanimichi/patch-2
The description of `--api` option doesn't make sense
2022-09-05 17:26:09 -05:00
Jonathan Hefner 1f246c6cb2
Merge pull request #45936 from ttanimichi/desc-force-plural
The description of `force_plural` doesn't make sense
2022-09-05 17:07:31 -05:00
Tsukuru Tanimichi f0b087efb0 Clarify description of --api option
"Indicates when to generate api" doesn't make sense:

```
$ bin/rails g scaffold | grep api
      [--api], [--no-api]                                    # Indicates when to generate api
```

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2022-09-05 16:55:36 -05:00
Tsukuru Tanimichi 3ee754e102 Clarify description of --force-plural option
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2022-09-05 16:34:42 -05:00
Jonathan Hefner f72a900c83
Merge pull request #45947 from jonathanhefner/word_wrap-test-coverage
Expand `word_wrap` test coverage
2022-09-05 16:26:41 -05:00
Jonathan Hefner 5e71ca3c21 Expand word_wrap test coverage
This rewrites the `word_wrap` tests to clarify what behavior is tested
and to expand test coverage.
2022-09-05 16:05:08 -05:00
Jonathan Hefner 23c3202de1
Merge pull request #45942 from maxim/word_wrap_chomp
Replace rstrip with chomp!(break_sequence) in word_wrap
2022-09-05 16:02:49 -05:00
Max Chernyak fe554a3daf Chomp the break_sequence in word_wrap
When word_wrap's break_sequence contains non-rstrippable
characters, word_wrap fails to strip the extra break_sequence at the
end properly. Using chomp! helps cut exactly what was specified in the
argument.
2022-09-05 15:38:23 -04:00
Jonathan Hefner 546f8a6be9 Remove obsolete word_wrap test
`word_wrap` was changed to use kwargs in #21086, so it cannot modify the
options hash.
2022-09-05 12:17:56 -05:00
John Hawthorn 9ff7c9c023
Merge pull request #45934 from jhawthorn/arel_visitor_intern
Speed up arel visitor dispatch
2022-09-02 13:35:14 -07:00
John Hawthorn 80a0eca40b Speed up arel visitor dispatch
This commit makes two performance tweaks to the dispatch_cache in
Arel::Visitors::Visitor.

First, it interns the method name into a symbol rather than a string.
send expects a symbol, so this avoids Ruby needing to intern that string
itself.

Second, it changes the dispatch cache to an identity hash. Classes are
already compared by identity, but Ruby is able to perform the lookup
more quickly if it is explicit on the hash.
2022-09-02 12:33:20 -07:00
Jean Boussier fa50226fcf
Merge pull request #45931 from fatkodima/follow-up-45695
Do not mutate relation when implicitly selecting a primary key in `ActiveRecord.find`
2022-09-02 16:05:57 +02:00
fatkodima 2714e21021 Do not mutate relation when implicitly selecting a primary key in `ActiveRecord.find` 2022-09-02 16:38:06 +03:00
Jean Boussier 4d4f0b2abd
Merge pull request #45695 from fatkodima/fix-find-select-except-id
Fix `ActiveRecord::FinderMethods.find` when passing multiple ids and primary key is not selected
2022-09-02 15:14:22 +02:00
fatkodima b55f0c54df Fix `ActiveRecord::FinderMethods.find` when passing multiple ids and primary key is not selected 2022-09-02 15:22:36 +03:00
Jean Boussier 2b01f332c6
Merge pull request #45612 from alextrueman/extend_select_with_hashes
Allow ActiveRecord::QueryMethods#select to accept a hash
2022-09-02 11:33:56 +02:00
alextrueman f4cfc2acdd Allow AR::QueryMethods#select to accept a hash
Add ability to use hash with columns and aliases inside #select method.

    Post
      .joins(:comments)
      .select(
        posts: { id: :post_id, title: :post_title },
        comments: { id: :comment_id, body: :comment_body}
      )

    instead

    Post
      .joins(:comments)
      .select(
        "posts.id as post_id, posts.title as post_title,
        comments.id as comment_id, comments.body as comment_body"
      )

Co-authored-by: Josef Šimánek <193936+simi@users.noreply.github.com>
Co-authored-by: Jean byroot Boussier <19192189+casperisfine@users.noreply.github.com>
2022-09-02 11:00:58 +02:00
Jean Boussier 8214a16b4e
Merge pull request #45928 from Shopify/rails-error-doc
Add missing documentation for Rails.error
2022-09-02 09:50:09 +02:00
Jean Boussier e4c237e014 Add missing documentation for Rails.error
The functionality itself is documented, but not the accessor
2022-09-02 09:15:58 +02:00
Jean Boussier d96612d3ca
Merge pull request #45911 from fatkodima/invalid-range-disk-controller
[ActiveStorage] Return "Range Not Satisfiable" return code for partial downloads with incorrect range
2022-09-02 08:29:52 +02:00
Rafael Mendonça França ba36ae0584
Merge pull request #45927 from r7kamura/feature/and-return
Avoid using `render(...) and return` in example code
2022-09-01 18:09:49 -04:00
Rafael Mendonça França f4124f7763
Merge pull request #45923 from compeak/main
Change ActiveStorage migration version from 5.2 to 7.0
2022-09-01 17:52:28 -04:00
Rafael Mendonça França 5ef8306e63
Merge pull request #45925 from HParker/avoid-module-less-than-in-value_for
Avoid using Module#<= in value_for
2022-09-01 17:33:57 -04:00
Ryo Nakamura a5065d4abc Avoid using `render(...) and return` in example code 2022-09-02 06:33:43 +09:00
Jonathan Hefner 4ab4144d26
Merge pull request #45918 from jonathanhefner/thread_mattr_accessor-default-require-frozen
Freeze `thread_mattr_accessor` default values
2022-09-01 16:03:18 -05:00
Jonathan Hefner 06a69e4fa8
Merge pull request #45926 from jonathanhefner/thread_mattr_accessor-anonymous-classes
Support `thread_mattr_accessor` in anonymous classes
2022-09-01 15:47:12 -05:00
Jonathan Hefner 38a2cfe9fe Freeze thread_mattr_accessor default values
This provides a basic level of protection against different threads
trying to mutate a shared default object.  It is not a bulletproof
solution, because the default may contain nested non-frozen objects, but
it should cover common cases.
2022-09-01 15:41:32 -05:00
Rafael Mendonça França 478f85c378
Merge pull request #45917 from jonathanhefner/redirect_action_dispatch-payload-include-request
Add `:request` to `redirect.action_dispatch` payload
2022-09-01 16:21:32 -04:00
Rafael Mendonça França 1e6ea56966
Merge pull request #45891 from Cofense/active-record-validations-guide-internal-link-errors
Make internal links to `errors` in the Active Record Validations guide
2022-09-01 16:17:06 -04:00
Eddie Lebow bc5561db23
Formatting typo
on-behalf-of: @Cofense <oss@cofense.com>
2022-09-01 16:09:37 -04:00
Jonathan Hefner 410c6144c6 Support thread_mattr_accessor in anonymous classes
This allows `thread_mattr_accessor` to be used with anonymous classes
when e.g. testing.
2022-09-01 14:37:23 -05:00
HParker ce6a50c101 Avoid using Module#<= in value_for
During where clause generation we search for scope type for the model.
We can do this with Module#!= instead as long as we grab the final scope type after the loop.

This is a small, but significant performance improvement.

Co-authored-by: John Hawthorn <jhawthorn@github.com>
2022-09-01 12:18:50 -07:00
John Hawthorn 44dced857f Update stackprof 2022-09-01 11:43:55 -07:00
compeak 506e357f00
update migration version from 5.2 to 7.0 2022-09-01 16:54:42 +02:00
Jonathan Hefner 40dc22f715 Add :request to redirect.action_dispatch payload
Follow-up to #43755.

This adds the request object to the `redirect.action_dispatch` payload,
for parity with `redirect_to.action_controller`.
2022-08-31 13:53:48 -05:00
Yasuo Honda b8fc7898fe
Merge pull request #45915 from yahonda/follow_up_45892
Address `NameError: uninitialized constant ActiveSupport::ErrorReporter::TestHelper`
2022-08-31 17:41:37 +09:00
Yasuo Honda b9b9e1abee Address `NameError: uninitialized constant ActiveSupport::ErrorReporter::TestHelper`
This commit addresses the isolated test failure at https://buildkite.com/rails/rails/builds/89113#0182ef62-c513-4f55-bbca-459385762ec4
Follow up #45892

```ruby
$ ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]
$ ruby -w test/cache/stores/file_store_test.rb
Running 230 tests in parallel using 8 processes
Run options: --seed 12562

......................................................................E

Error:
FileStoreTest#test_invalid_expiration_time_reports_and_logs_when_raise_on_invalid_cache_expiration_time_is_false:
NameError: uninitialized constant ActiveSupport::ErrorReporter::TestHelper
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:802:in `with_error_subscriber_and_log'
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:609:in `block in test_invalid_expiration_time_reports_and_logs_when_raise_on_invalid_cache_expiration_time_is_false'
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:792:in `with_raise_on_invalid_cache_expiration_time'
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:608:in `test_invalid_expiration_time_reports_and_logs_when_raise_on_invalid_cache_expiration_time_is_false'

rails test home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:607

...................................................................................................E

Error:
OptimizedFileStoreTest#test_invalid_expiration_time_reports_and_logs_when_raise_on_invalid_cache_expiration_time_is_false:
NameError: uninitialized constant ActiveSupport::ErrorReporter::TestHelper
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:802:in `with_error_subscriber_and_log'
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:609:in `block in test_invalid_expiration_time_reports_and_logs_when_raise_on_invalid_cache_expiration_time_is_false'
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:792:in `with_raise_on_invalid_cache_expiration_time'
    /home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:608:in `test_invalid_expiration_time_reports_and_logs_when_raise_on_invalid_cache_expiration_time_is_false'

rails test home/yahonda/src/github.com/rails/rails/activesupport/test/cache/behaviors/cache_store_behavior.rb:607

...........................................................

Finished in 0.303590s, 757.6008 runs/s, 2048.8161 assertions/s.
230 runs, 622 assertions, 0 failures, 2 errors, 0 skips
$
```
2022-08-31 17:05:59 +09:00