Commit Graph

21876 Commits

Author SHA1 Message Date
Gannon McGibbon ee81a0ed34
Merge pull request #45976 from gmcgibbon/log_subscriber_modes
Add italic and underline support to `ActiveSupport::LogSubscriber#color`
2022-09-09 23:43:50 -05:00
Gannon McGibbon 6016f9ef31 Add italic and underline support to `ActiveSupport::LogSubscriber#color`
Previously, only bold text was supported via a positional argument.
This allows for bold, italic, and underline options to be specified
for colored logs.

```ruby
info color("Hello world!", :red, bold: true, underline: true)
```
2022-09-09 22:55:33 -05:00
Rafael Mendonça França 91a2ae5e59
Remove CHANGELOG entry for change only existing in main
The issue fixed by the commit that introduced that entry only existed
in the main branch, so it isn't really a released change worthy of a
CHANGELOG entry.
2022-09-09 22:56:36 +00:00
Rafael Mendonça França 606fdfb0c6
Merge pull request #45829 from ghiculescu/nested-attribute-docs-improvements
Nested attribute docs improvements
2022-09-09 17:28:13 -04:00
Rafael Mendonça França 663e846381
Require used model to fix isolated tests 2022-09-09 21:17:59 +00:00
Rafael Mendonça França 9f372c94ea
Merge PR #44438 2022-09-09 21:14:08 +00:00
admin 62c358595c
improve "in_order_of" to allow string column name 2022-09-09 21:46:04 +03:00
Gannon McGibbon 2b42bbc8a2
Merge pull request #45975 from gmcgibbon/application_record_generator_usage
Delegate application record generator description to orm hooked generator.
2022-09-09 12:32:31 -04:00
Iliana Hadzhiatanasova a0fd15ee7e Default prepared_statements to false for mysql2 adapter 2022-09-09 16:21:33 +01:00
Jean Boussier 63be3385bd Mysql2Adapter remove reference to closed connection
If `connect` fails, we'd stick with a closed `Mysql2` connection
which means on the next try we'll try to ping it without any
chance of success.
2022-09-09 16:37:55 +02:00
Gannon McGibbon f54cbe1f4a Delegate application record generator description to orm hooked generator. 2022-09-08 18:19:51 -05:00
eileencodes 436277da88
Move SchemaMigration to an independent object
Previously, SchemaMigration inherited from ActiveRecord::Base. This is
problematic for multiple databases and resulted in building the code in
AbstractAdapter that was previously there. Rather than hacking around
the fact that SchemaMigration inherits from Base, this PR makes
SchemaMigration an independent object. Then each connection can get it's
own SchemaMigration object. This change required defining the methods
that SchemaMigration was depending on ActiveRecord::Base for (ex
create!). I reimplemented only the methods called by the framework as
this class is no-doc's so it doesn't need to implement anything beyond
that. Now each connection gets it's own SchemaMigration object which
stores the connection. I also decided to update the method names (create
-> create_version, delete_by -> delete_version, delete_all ->
delete_all_versions) to be more explicit.

This change also required adding a NullSchemaMigraiton class for cases
when we don't have a connection yet but still need to copy migrations
from the MigrationContext. Ultimately I think this is a little weird -
we need to do so much work to pick up a set of files? Maybe something to
explore in the future.

Aside from removing the hack we added back in #36439 this change will
enable my work to stop clobbering and depending directly on
Base.connection in the rake tasks. While working on this I discovered
that we always have a `ActiveRecord::SchemaMigration` because the
connection is always on `Base` in the rake tasks. This will free us up
to do less hacky stuff in the migrations and tasks.
2022-09-08 11:32:32 -04:00
Eileen M. Uchitelle e24fbf71fa
Merge pull request #45916 from eileencodes/reduce-calls-to-pool-configs
Reduce calls to pool_configs
2022-09-08 11:23:19 -04:00
eileencodes bc951b4719
Reduce allocations of pool_configs
This is a partial fix for #45906 to allocate less objects when iterating
through pool configs. My original attempt to fix this wasn't feasible
due to how the query cache works. The idea to use a dedicated `each_*`
came from jonathanhefner and I incorporated it here.

Using the following script:

```
Benchmark.memory do |x|
  x.report("all") do
    ActiveRecord::Base.connection_handler.all_connection_pools.each { }
  end

  x.report("each") do
    ActiveRecord::Base.connection_handler.each_connection_pool { }
  end

  x.compare!
end
```

We can see that less objects are allocated with the each method. This is
even more important now that we are calling `all_connection_pools` in
more placed since #45924 and #45961.

```
Calculating -------------------------------------
                 all   408.000  memsize (     0.000  retained)
                         7.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
                each   328.000  memsize (     0.000  retained)
                         3.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
                each:        328 allocated
                 all:        408 allocated - 1.24x more
```

Note that if this is run again on this branch `all_connection_pools`
will have even more allocations due to the deprecation warning that was
introduced. However removing that shows similar numbers to the benchmark
here.

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2022-09-08 10:27:15 -04:00
Gabriel Oliveira fd67de56d0 Fix typo in :active_record_suppressor_registry symbol 2022-09-08 15:48:33 +02:00
Ryuta Kamizono e88005a875
Merge pull request #45965 from koic/enable_minitest_assert_raises_with_regexp_argument_cop
Enable `Minitest/AssertRaisesWithRegexpArgument` cop
2022-09-08 18:10:07 +09:00
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
fatkodima 8f12731b90 Fix loading records with encrypted attributes defined on columns with default values 2022-09-07 23:30:35 +03: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
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
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
fatkodima 2714e21021 Do not mutate relation when implicitly selecting a primary key in `ActiveRecord.find` 2022-09-02 16:38:06 +03: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
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
Tom Hallett decafbd9cf
Docs: Example should append to ignored_columns
Updating the documentation for setting `self.ignored_columns` on an ActiveRecord model to append to the existing value instead of overwriting the existing array.

Appending to the existing value, `+=`, is helpful for the scenario when 2 developers set `ignored_columns` in different parts of the file and the second one overwrites the first one.
2022-08-30 10:23:13 -04:00
fatkodima 76a8db07b0 Fix reverting the addition of invalid foreign keys and check constraints 2022-08-28 01:43:36 +03:00
fatkodima 3158bbb9f6 Update `rubocop-performance` and enable more performance-related cops 2022-08-26 15:07:11 +03:00
Jonathan Hefner e45947328e Fix typo in CHANGELOG [ci-skip]
Follow-up to #42650.
2022-08-25 11:54:13 -05:00
Jonathan Hefner 0fbac7f409
Merge pull request #42650 from intrip/becomes_with_virtual_attrs
Normalize virtual attributes on ActiveRecord::Persistence#becomes
2022-08-25 10:48:54 -05:00
fatkodima 4c62903dfb Extend dangerous attribute methods with dangerous methods from Object 2022-08-25 03:09:36 +03:00
Jacopo 2f5136a3be Normalize virtual attributes on `ActiveRecord::Persistence#becomes`
When source and target classes have a different set of attributes adapts
attributes such that the extra attributes from target are added.

Fixes #41195

Co-authored-by: SampsonCrowley <sampsonsprojects@gmail.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2022-08-24 22:51:57 +02:00
Akshay Birajdar 6ac4ee967d Use #to_hash to serialize AS::HWIA & Hash for stored attributes 2022-08-25 00:28:48 +05:30
eileencodes 92240559f2
Clean up schema load tasks
While working on another PR I noticed that we have 6 different tasks for
loading the schema; 3 for all dbs, 3 for namespaced dbs. They are:

* `db:schema:load`
* `db:schema:load:namespace`
* `db:test:load_schema`
* `db:test:load_schema:namespace`
* `db:test:load` (calls `db:test:load_schema` only)
* `db:test:load:namespace` (calls `db:test:load_schema:namespace` only)

I've removed the last two because they didn't call anything except for
another task and they were already private. I believe the original
`db:test:load` and `db:test:load:namescace` were added to handle whether
to load sql or ruby schema fules, but we no longer need that (it's
handled by the `load_schema` method directly now).

In addition I removed `:load_config, :check_protected_environments` from
the `db:schema:load:namespace` task because `purge` already calls these
so we don't need to do it twice.

I tried to remove `db:test:load_schema:namespace` and call
`db:schema:load:namespace` instead but we can't do that because
`db:test:load_schema:namespace` needs to set the environment to "test",
not use the current environment. `db:schema:load` also can't be used
instead of `db:test:load_schema` because it tries to load both dev and
test schemas, but it's callers want just the test schema.
2022-08-22 14:35:33 -04:00
Jean Boussier 2418939007
Merge pull request #45796 from Shopify/log-subscriber-silenced
Optimize AS::LogSubscriber
2022-08-22 10:43:49 +02:00
Jean Boussier d1a9a9928d
Merge pull request #45015 from fatkodima/missing-and-associated-parent-child
Fix `where.missing` and `where.associated` for parent/child associations
2022-08-20 08:43:16 +02:00
Jean Boussier 8cd4d65fe2
Merge pull request #45808 from fatkodima/check_constraint_exists-public
Make `check_constraint_exists?` public
2022-08-19 23:20:04 +02:00
Jean Boussier 3dcd28acdd
Merge pull request #45860 from HParker/add-sqlite=binary-column-default-support
Add binary column default value support to sqlite
2022-08-19 23:00:57 +02:00
eileencodes 46e4580d39
Move InternalMetadata to migration context
This PR is similar to #36439 which moved schema migrations to the
connection and migration context. This PR does the same thing for the
internal metadata class.

Previously the metadata class was only accessible on Base which makes it
a little weird when switching connections. This didn't really cause any
issues but I noticed it when working on a PR to change how connections
work in the database rake tasks.

In a multiple database environment it makes sense to access the
`ar_internal_metadata` on the existing connection rather than on
`Base.connection`. I previously didn't implement this because I felt we
wanted only one place to ask for the metadata information but recently
realized that we were still recording metadata on all the databases, so
we might as well record it correctly.

Applications should notice no change in behavior because this is only
accessed through the rake tasks. Additionally behavior between schema
migrations and internal metadata are now more similar. Eventually I'd
like neither of these to inherit from or rely on Base, but we have a
lot more work to do before that's possible.
2022-08-19 16:05:15 -04:00
HParker e1bf63fca9 Add binary column default value support to sqlite
This adds support for reading binary column default values before the column data is read from the database.
This makes binary columns behave more like other column types with default values
2022-08-19 11:27:21 -07:00
eileencodes 43a6b4fc0f
Use Time instead of Date for comparison
In Ruby trunk date changed to use the gregorian calendar in
https://github.com/ruby/date/pull/73.

This had the result that dates not in the gregorian calendar can't be
compared to times not in the gregorian calendar. Since this test isn't
testing datetime comparison but rather that bc timestamps can be stored
in the database, we can compare with `Time.new(0) - 1.week` rather than
`Date.new(0) - 1.week`. I think this might be a bug in Ruby but I'm not
sure there's a great fix. For now however, let's get this test passing.
2022-08-18 14:48:53 -04:00
Alex Ghiculescu 2220b55c3b
Update activerecord/lib/active_record/nested_attributes.rb
Co-authored-by: Petrik de Heus <petrik@deheus.net>
2022-08-17 18:12:33 -05:00
Jonathan Hefner 18f8c6ac9b Add CHANGELOG entry for #45670 [ci-skip] 2022-08-17 11:51:31 -05:00
eileencodes 2efb06d1dd
Define and raise error if `legacy_connection_handling=` is called from an app
I deprecated `legacy_connection_handling` setter in 7.0 but then removed
it without setting it to false by default upgraded apps might have this
set to false in their config and still be getting an error. This
confusing behavior came up with one of our apps at work.

To make it less confusing I've redefined `legacy_connection_handling`
setter and am raising an argument error if called. I didn't redefine the
getter because that shouldn't be in a config, it's not useful in that
context.
2022-08-16 16:28:00 -04:00
Alex Ghiculescu 10a83207e6 Nested attribute docs improvements
2 small improvemennts to the docs for `ActiveRecord::NestedAttributes`.

- Include a link to `ActionView::Helpers::FormHelper::fields_for`, since you would typically use these tools in tandem I think it makes sense to point you to it.
- Show an example of how to write an integration test when using `ActionView::Helpers::FormHelper::fields_for`. You could argue that this example should also go in Action View, but I think it makes more sense here as the "parent" doc for the Nested Attributes feature.
2022-08-15 12:26:33 -05:00
fatkodima 885c024f70 Fix `ActiveRecord::QueryMethods#in_order_of` to work with nils
When passing `nil` to `in_order_of`, the invalid SQL query is generated (because `NULL != NULL`):

```ruby
Book.in_order_of(:format, ["hardcover", "ebook", nil]).to_sql
```

```sql
SELECT "books".* FROM "books" WHERE "books"."format" IN ('hardcover', 'ebook', NULL)
ORDER BY CASE "books"."format" WHEN 'hardcover' THEN 1
WHEN 'ebook' THEN 2 WHEN NULL THEN 3 ELSE 4 END ASC
```

This PR also removes the special order field generation (`ORDER BY FIELD`) for MYSQL, because it is
unable to work with `NULL`s and the default `ORDER BY CASE` works for it (tested on MariaDB and MySQL 5.7+).
2022-08-15 19:07:55 +03:00
John Hawthorn 08af60e01d
Merge pull request #45807 from ahoglund/binary-collation-fix-2
Run DROP DEFAULT for NULL value; Add text type checks
2022-08-12 13:55:55 -07:00
Eileen M. Uchitelle 18da7b6ba7
Merge pull request #45806 from adrianna-chang-shopify/ac-expose-schema-creation-objects
Expose `#schema_creation` methods
2022-08-12 12:06:23 -04:00
Jean Boussier bd19d1baf1 Optimize AS::LogSubscriber
The various LogSubscriber subclasses tend to subscribe to events
but then end up doing nothing if the log level is high enough.

But even if we end up not logging, we have to go through the
entire notification path, record timing etc.

By allowing subscribers to dynamically bail out early, we can
save a lot of work if all subscribers are silenced.
2022-08-12 09:58:17 +02:00