I have so. many. regrets. about using `spec_name` for database
configurations and now I'm finally putting this mistake to an end.
Back when I started multi-db work I assumed that eventually
`connection_specification_name` (sometimes called `spec_name`) and
`spec_name` for configurations would one day be the same thing. After
2 years I no longer believe they will ever be the same thing.
This PR deprecates `spec_name` on database configurations in favor of
`name`. It's the same behavior, just a better name, or at least a
less confusing name.
`connection_specification_name` refers to the parent class name (ie
ActiveRecord::Base, AnimalsBase, etc) that holds the connection for it's
models. In some places like ConnectionHandler it shortens this to
`spec_name`, hence the major confusion.
Recently I've been working with some new folks on database stuff and
connection management and realize how confusing it was to explain that
`db_config.spec_name` was not `spec_name` and
`connection_specification_name`. Worse than that one is a symbole while
the other is a class name. This was made even more complicated by the
fact that `ActiveRecord::Base` used `primary` as the
`connection_specification_name` until #38190.
After spending 2 years with connection management I don't believe that
we can ever use the symbols from the database configs as a way to
connect the database without the class name being _somewhere_ because
a db_config does not know who it's owner class is until it's been
connected and a model has no idea what db_config belongs to it until
it's connected. The model is the only way to tie a primary/writer config
to a replica/reader config. This could change in the future but I don't
see value in adding a class name to the db_configs before connection or
telling a model what config belongs to it before connection. That would
probably break a lot of application assumptions. If we do ever end up in
that world, we can use name, because tbh `spec_name` and
`connection_specification_name` were always confusing to me.
This commit is somewhat of a bandaid fix for a bug that was revealed in #38029
and then #38151. #38151 can cause problems in certain cases when an app has
a 3-tier config, with replicas, because it reorders the configuration
and changes the implict default connection that gets picked up.
If an app calls `establish_connection` with no arguments or doesn't call
`connects_to` in `ApplicationRecord` AND uses parallel testing
databases, the application may pick up the wrong configuration.
This is because when the code in #38151 loops through the configurations
it will update the non-replica configurations and then put them at the
end of the list. If you don't specify which connection you want, Rails
will pick up the _first_ connection for that environment. So given the
following configuration:
```
test:
primary:
database: my_db
replica:
database: my_db
replica: true
```
The database configurations will get reordered to be `replica`,
`primary` and when Rails calls `establish_connection` with no arguments
it will pick up `replica` because it's first in the list.
Looking at this bug it shows that calling `establish_connection` with no
arguments (which will pick up the default env + first configuration in
the list) OR when `establish_connection` is called with an environment
like `:test` it will also pick up that env's first configuration. This
can have surprising behavior in a couple cases:
1) In the parallel testing changes we saw users getting the wrong db
configuration and hitting an `ActiveRecord::ReadOnlyError`
2) Writing a configuration that puts `replica` before `primary`, also
resulting in a `ActiveRecord::ReadOnlyError`
The real fix for this issue is to deprecate calling
`establish_connection` with an env or nothing and require an explcit
configuration (like `primary`). This would also involve blessing
`:primary` as the default connection Rails looks for on boot. In
addition, this would require us deprecating connection specification
name "primary" in favor of the class name always since that will get
mega-confusing (seriously, it's already mega-confusing).
We'll work on fixing these underlying issues, but wanted to get a fix
out that restores previous behavior.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
In 154abca we switched from using `Rails.env` to fetch the `env_name` to
`ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_sym` which
changed the type from a `String` to a `Symbol`.
This commit brings things back to the original state, so we can find the
configurations correctly!
It also modifies the configuration in the configurations array, so that
future connections can find the database with the updated keyword value.
We want to introduce an object-based DSL for building and modifying
configuration objects. As part of that we want to make sure that users
don't think they can modify configuration_hash values and have them
change the configuration. For that reason we're going to freeze the
Hash here, and have modified places in tests where we were modifying
these hashes.
The commit here also adds a test for the Test Databases and in that work
we found that we were calling `Rails.env` and Active Record doesn't load
Rails.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
