Previously, it was very easy to accidentally leak a database password in
production logs if an error ends up calling inspect on a ConnectionPool
or an individual connection (Adapter). This is due to the default
`#inspect` output for Pools and Adapters being unnecessarily large, and
both currently including passwords (through the DatabaseConfig of a
Pool, and the internal configuration of an Adapter).
This commit addresses these issues by defining a custom `#inspect` for
ConnectionPool, AbstractAdapter, and DatabaseConfig. The condensed
`#inspect` only includes a few valuable fields instead of all of the
internals, which prevents both the large output and passwords from being
included.
Currently, there is no (simple) way to ask a model if it connects to a
single database or to multiple shards. Furthermore, without looping
through a model's connections, I don't believe there's an easy way to
return a list of shards a model can connect to.
This commit adds a `@shard_keys` ivar that's set whenever `.connects_to`
is called. It sets the ivar to the result of `shards.keys`. `shards` in
`.connects_to` defaults to an empty hash and therefore when calling
`connects_to database: {...}` `@shard_keys` will be set to an empty array.
`@shard_keys` is set _before_ the following lines:
```
if shards.empty?
shards[:default] = database
end
```
This conditional sets the one and only shard (`:default`) to the value of `database`
that we pass to `.connects_to`. This allows for calling
`connected_to(shard: :default)` on models configured to only connect to
a database e.g.:
```ruby
class UnshardedBase < ActiveRecord::Base
self.abstract_class = true
connects_to database: { writing: :primary }
end
class UnshardedModel < UnshardedBase
end
UnshardedBase.connected_to(shard: :default) {
UnshardedBase.connection_pool.db_config.name } => primary
```
This is ultimately still an _unsharded_ model which is why `@shard_keys`
gets set before the conditional.
With the new `@shard_keys` ivar we need a way for descendants of the
abstract AR model to return that same value. For that we leverage the
existing `.connection_class_for_self` method. That method returns the
ancestor of the model where `.connects_to` was called, or returns self if
it's the connection class:
```ruby
class UnshardedBase < ActiveRecord::Base
self.abstract_class = true
connects_to database: { writing: :primary }
end
class UnshardedModel < UnshardedBase
end
ActiveRecord::Base.connection_class_for_self => ActiveRecord::Base
UnshardedBase.connection_class_for_self => UnshardedBase(abstract)
UnshardedModel.connection_class_for_self => UnshardedBase(abstract)
```
The new `.shard_keys` method is a getter which returns the value of
`@shard_keys` from the connection class or it returns an empty array.
The empty array is necessary in cases where `connects_to` was never
called.
Finally, I've added an `.connected_to_all_shards` method which takes all of the
arguments for `.connected_to` except for `shard`. Instead, it loops through
every shard key and then delegates everything else to `.connected_to`. I've
used `.map` instead of `.each` so that we can collect the results of each block.
If a developer has neglected to use a structured column type (hstore
or json) or to declare a serializer with `ActiveRecord.store`:
```ruby
class User < ActiveRecord::Base
store_accessor :settings, :notifications
end
```
then a `ConfigurationError` will now be raised with a descriptive
error message when the accessor is read or written:
```ruby
puts user.notifications
# ActiveRecord::ConfigurationError: the column 'settings' has not
# been configured as a store. Please make sure the column is
# declared serializable via 'ActiveRecord.store' or, if your
# database supports it, use a structured column type like hstore or
# json.
```
Previously, in this situation, a `NoMethodError` was raised when the
accessor was read or written:
```ruby
puts user.notifications
# NoMethodError: undefined method `accessor' for an instance of ActiveRecord::Type::Text
```
Raising a descriptive exception should help developers understand more
quickly what's wrong and how to fix it.
Closes#51699
Previously we only provided a method to set the ignored schema cache
tables, but there was no way to ask if a table was ignored by the schema
cache. Applications may want to implement their own schema cache, or at
least run this check. Rather than forcing them to implement an internal
method, this adds a way to ask whether a table is ignored by the schema
cache code.
Usage:
```ruby
ActiveRecord.schema_cache_ignored_tables = ["developers"]
ActiveRecord.schema_cache_ignored_tables?("developers")
```
which respects reject_if and is in nested_attributes order.
When in default index_errors:true mode,
fix#24390 and return index based on full association order.
This commit adds a deprecation warning for the `query_constraints:`
association option. This option will change behavior in the future versions
of Rails and applications are encouraged to switch to `foreign_key:` to preserve the
current behavior.
```ruby
Post.with_recursive(
post_and_replies: [
Post.where(id: 42),
Post.joins('JOIN post_and_replies ON posts.in_reply_to_id = post_and_replies.id'),
]
)
```
Generates the following SQL:
```sql
WITH RECURSIVE "post_and_replies" AS (
(SELECT "posts".* FROM "posts" WHERE "posts"."id" = 42)
UNION ALL
(SELECT "posts".* FROM "posts" JOIN post_and_replies ON posts.in_reply_to_id = post_and_replies.id)
)
SELECT "posts".* FROM "posts"
```
at the connection level
Fix#51448
Type cast columns of type `date` to ruby `Date` when running a raw
query through `ActiveRecord::Base.connection.select_all`.
Before:
```ruby
person = Person.find(1)
person.strict_loading!(mode: :n_plus_one_only)
person.posts.first
# SELECT * FROM posts WHERE person_id = 1; -- non-deterministic order
```
After:
```ruby
person = Person.find(1)
person.strict_loading!(mode: :n_plus_one_only)
person.posts.first # this is 1+1, not N+1
# SELECT * FROM posts WHERE person_id = 1 ORDER BY id LIMIT 1;
```
Strict loading in `:n_plus_one_only` mode is designed to prevent performance issues when
deeply traversing associations. It allows `Person.find(1).posts`, but _not_
`Person.find(1).posts.map(&:category)`. With this change, child associations are no
longer eagerly loaded, to match intended behavior and to prevent non-deterministic
order issues caused by calling methods like `first` or `last`.
Fixes#49473.
Ref: https://github.com/rails/rails/pull/26103
Ref: https://github.com/rails/rails/pull/51426
A fairly common mistake with Rails is to enqueue a job from inside a
transaction, and a record as argumemnt, which then lead to a RecordNotFound
error when picked up by the queue.
This is even one of the arguments advanced for job runners backed by the
database such as `solid_queue`, `delayed_job` or `good_job`. But relying
on this is undesirable iin my opinion as it makes the Active Job abstraction
leaky, and if in the future you need to migrate to another backend or even
just move the queue to a separate database, you may experience a lot of race
conditions of the sort.
But more generally, being able to defer work to after the current transaction
has been a missing feature of Active Record. Right now the only way to do it
is from a model callback, and this forces moving things in Active Record
models that sometimes are better done elsewhere. Even as a self-proclaimed
"service object skeptic", I often wanted this capability over the last decade,
and I'm sure it got asked or desired by many more people.
Also there's some 3rd party gems adding this capability using monkey patches.
It's not a reason to upstream the capability, but it's a proof that there is
demand for it.
Implementation wise, this proof of concept shows that it's not really hard to
implement, even with nested multi-db transactions support.
Co-Authored-By: Cristian Bica <cristian.bica@gmail.com>
This commit makes two types of queries retry-able by opting into our `allow_retry` flag:
1) SELECT queries we construct by walking the Arel tree via `#to_sql_and_binds`. We use a
new `retryable` attribute on collector classes, which defaults to true for most node types,
but will be set to false for non-idempotent node types (functions, SQL literals, etc). The
`retryable` value is returned from `#to_sql_and_binds` and used by `#select_all` and
passed down the call stack, eventually reaching the adapter's `#internal_exec_query` method.
Internally-generated SQL literals are marked as retryable via a new `retryable` attribute on
`Arel::Nodes::SqlLiteral`.
2) `#find` and `#find_by` queries with known attributes. We set `allow_retry: true` in `#cached_find_by`,
and pass this down to `#find_by_sql` and `#_query_by_sql`.
These changes ensure that queries we know are safe to retry can be retried automatically.
Controls whether `ActiveRecord::Base.connection` raises an error, emits a deprecation warning, or neither.
`ActiveRecord::Base.connection` checkouts a database connection from the pool and keep it leased until the end of
the request or job. This behavior can be undesirable in environments that use many more threads or fibers than there
is available connections.
This configuration can be used to track down and eliminate code that calls `ActiveRecord::Base.connection` and
migrate it to use `ActiveRecord::Base.with_connection` instead.
The default behavior remains unchanged, and there is currently no plans to change the default.
This adds a `dirties` option to `ActiveRecord::Base.uncached` and
`ActiveRecord::ConnectionAdapters::ConnectionPool#uncached`.
Setting `dirties` to `false`, means database writes to the connection
pool will not mark any query caches as dirty.
The option defaults to `true` which retains the existing behaviour and
clears query caches on all connection pools used by the current thread.
Co-authored-by: Jeremy Daer <jeremy@rubyonrails.org>
Replaced by `#lease_connection` to better reflect what it does.
`ActiveRecord::Base#connection` is deprecated in the same way
but without a removal timeline nor a deprecation warning.
Inside the Active Record test suite, we do remove `Base.connection`
to ensure it's not used internally.
Some callsites have been converted to use `with_connection`,
some other have been more simply migrated to `lease_connection`
and will serve as a list of callsites to convert for
https://github.com/rails/rails/pull/50793
```ruby
assert_equal "Ruby on Rails", web_sites(:rubyonrails).name
assert_equal "Ruby on Rails", fixture(:web_sites, :rubyonrails).name
```
This was brought to me by someone with a `Metadata` model. The fixtures
accessor being `metadata` which conflicts with the `metadata` method
recently added in `Minitest`.
