This commit just ensures we're green with the main branch of rack test.
The changes are things we should have done anyway, and are backwards
compatible with older versions of rack test
with some additional changes made later:
1. Flip the condition for backward compatibility
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2. Improve custom behavior test
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
3. Fix indentation
This commit improves handling of implied options for `AppGenerator` and
`PluginGenerator` in a few different ways.
Implied options are now reported:
```console
$ rails new my_cool_app --skip-active-job
Based on the specified options, the following options will also be activated:
--skip-action-mailer [due to --skip-active-job]
--skip-active-storage [due to --skip-active-job]
--skip-action-mailbox [due to --skip-active-storage]
--skip-action-text [due to --skip-active-storage]
...
```
Option conflicts raise an error:
```console
$ rails new my_cool_app --skip-active-job --no-skip-active-storage
Based on the specified options, the following options will also be activated:
--skip-action-mailer [due to --skip-active-job]
--skip-active-storage [due to --skip-active-job]
ERROR: Conflicts with --no-skip-active-storage
--skip-action-mailbox [due to --skip-active-storage]
--skip-action-text [due to --skip-active-storage]
railties/lib/rails/generators/app_base.rb:206:in `report_implied_options': Cannot proceed due to conflicting options (RuntimeError)
```
Meta option (e.g. `--minimal`) implications are also reported:
```console
$ rails new my_cool_app --minimal
Based on the specified options, the following options will also be activated:
--skip-active-job [due to --minimal]
--skip-action-mailer [due to --skip-active-job, --minimal]
--skip-active-storage [due to --skip-active-job, --minimal]
--skip-action-mailbox [due to --skip-active-storage, --minimal]
--skip-action-text [due to --skip-active-storage, --minimal]
--skip-javascript [due to --minimal]
--skip-hotwire [due to --skip-javascript, --minimal]
--skip-action-cable [due to --minimal]
--skip-bootsnap [due to --minimal]
--skip-dev-gems [due to --minimal]
--skip-system-test [due to --minimal]
...
```
`--no-*` options now work with meta options, and are both comprehensive
and precise:
```console
$ rails new my_cool_app --minimal --no-skip-active-storage
Based on the specified options, the following options will also be activated:
--skip-action-mailer [due to --minimal]
--skip-action-mailbox [due to --minimal]
--skip-action-text [due to --minimal]
--skip-javascript [due to --minimal]
--skip-hotwire [due to --skip-javascript, --minimal]
--skip-action-cable [due to --minimal]
--skip-bootsnap [due to --minimal]
--skip-dev-gems [due to --minimal]
--skip-system-test [due to --minimal]
...
```
Closes#45223.
Closes#45205.
Co-authored-by: Brad Trick <btrick@appacademy.io>
The majority of where this functionality was removed in #38672.
`owner_name` was never used publicaly in the db_config and was at a time
used for identifying connection owners. Now the connection owner is
always stored in the PoolConfig (same place as the db_config) so the
db_config doesn't need this information.
Since 7ccaa125ba it's not been possible to not include `SameSite` on your cookies. `SameSite` is recommended, but it's not a required field, and you should be able to opt out of it.
This PR introduces that ability: you can opt out of `SameSite` by passing `same_site: false`.
```ruby
cookies[:foo] = { value: "bar", same_site: false }
```
Previously, this incorrectly set the `SameSite` attribute to the value of the `cookies_same_site_protection` setting. https://github.com/rails/rails/pull/44934 added docs saying that you could pass `nil` as a value, but that also would fall back to the default (`:lax`).
Accessing form's method by getAttribute('method') instead of form.method as this property may return named form element if there is any named "method".
In the prior code we were getting the pool manager a bunch of times -
once in the retrieve_connection_pool, once in remove_connection_pool,
and once in the else conditional after setting it.
This change ensures we're only getting the pool manager once. If there
is no pool manager for the given key then we create a new one in the new
`set_pool_manager` method and use that.
The refactor also has a change in that we no longer instrument if a new
connection was not established. This instrumentation is private though
(denoted by the !) so it's safe to make this change.
Followup to #45450
This adds a `:urlsafe` option to the `MessageEncryptor` constructor.
When enabled, this option ensures that messages use a URL-safe encoding.
This matches the `MessageVerifier` `:urlsafe` option added in #45419.
Fixes https://github.com/rails/rails/issues/45489
- Adds `anchor: true` to the Action Cable server mount, so that it only strictly matches `/cable` rather than anything that starts with that.
- Uses `reverse_merge` instead of `merge` in `Mapper#mount`, so that you can override these options if you need to.
Previously Rails would always remove the connection if it found a
matching class in the pool manager. Therefore if
`ActiveRecord::Base.establish_connection` was called with the same
config, each time it was called it would be clobbered, even though the
config hasn't changed and the existing connection is prefectly fine. As
far as I can tell from conversations and reading the history this
functionality was added for ActiveRecord tests to be able to clobber the
connection and use a new config, then re-establish the old connection.
Essentially outside Rake tasks and AR tests, this functionality doesn't
have a ton of value.
On top of not adding a ton of value, this has resulted in a few bugs. In
Rails 6.0 I made it so that if you established a connection on
`ApplicationRecord` Rails would treat that connection the same as
`ActiveRecord::Base.` The reason for this is that the Railtie
establishes a connection on boot to the first database, but then if
you're using multiple databases you're calling `connects_to` in your
`ApplicationRecord` or primary abstract class which essentially doubles
your connections to the same database. To avoid opening 2 connections to
the same database, Rails treats them the same.
However, because we have this code that removes existing connections,
when an application boots, `ApplicationRecord` will clobber the
connection that the Railtie established even though the connection
configs are the same.
This removal of the connection caused bugs in migrations that load up a
model connected to `ApplicationRecord` (ex `Post.first`) and then calls
`execute("SELECT 1")` (obviously a simplified example). When `execute`
runs the connection is different from the one opened to run the
migration and essentially it is lost when the `remove_connection` code
is called.
To fix this I've updated the code to only remove the connection if the
database config is different. Ultimately I'd like to remove this code
altogether but to do that we first need to stop using
`Base.establish_connection` in the rake tasks and tests. This will fix
the major bugs until I come up with a solution for the areas that
currently need to call `establish_connection` on Base.
The added benefit of this change is that if your app is calling
`establish_connection` multiple times with the same config, it is now
3x faster than the previous implementation because we can return the
found pool instead of setting it up again. To benchmark this I
duplicated the `establish_connection` method to use the new behavior
with a new name.
Benchmark script:
```ruby
require "active_record"
require "logger"
require "benchmark/ips"
config_hash = { "development" => { "primary" => { "adapter" => "mysql2", "username" => "rails", "database" => "activerecord_unittest"}}}
ActiveRecord::Base.configurations = config_hash
db_config = ActiveRecord::Base.configurations.configs_for(env_name: "development", name: "primary")
p "Same model same config"
ActiveRecord::Base.connected_to(role: :writing, prevent_writes: true) do
Benchmark.ips do |x|
x.report "establish_connection with remove" do
ActiveRecord::Base.establish_connection(db_config)
end
x.report "establish_connection without remove" do
ActiveRecord::Base.establish_connection_no_remove(db_config)
end
x.compare!
end
end
```
Benchmark results:
```
Warming up --------------------------------------
establish_connection with remove
4.677k i/100ms
establish_connection without remove
19.501k i/100ms
Calculating -------------------------------------
establish_connection with remove
41.252k (±11.3%) i/s - 205.788k in 5.075525s
establish_connection without remove
179.205k (± 6.9%) i/s - 897.046k in 5.029742s
Comparison:
establish_connection without remove: 179205.1 i/s
establish_connection with remove: 41252.3 i/s - 4.34x (± 0.00) slower
```
Other changes:
1) sqlite3 now disconnects and reconnects the connection when `purge` is
called. This is necessary now that a new connection isn't created
everyt time `establish_connection` is called. Without this change to
purge the new database is left in an inaccessible state causing a
readonly error from the sqlite3 client. This wasn't happening in mysql
or postgres because they were already reconnecting the db connection.
2) I added `remove_connection` to tests that use `ApplicationRecord`.
This is required because `ApplicationRecord` or any class that is a
`primary_abstract_class` will be treated the same as
`ActiveRecord::Base`. This is fine in applications because they are
shared connections, but in the AR test environment, we don't want those
connnections to stick around (we want AR::Base back).
3) In the async tests I removed 2 calls to `establish_connection`. These
were causing sqlite3 tests to leak the state of async_executor because
it's stored on the connection. I'm not sure why these were calling
`establish_connection` but it's not necessary and was leaking state when
now that we are no longer removing the connection.
Fixes: #41855Fixes: #41876Fixes: #42873Fixes: #43004
