Moves the part of `compile!` that compiles the template source into it's
own method. We need this for future work in improving exceptions for ERB
templates to pass to ErrorHighlight.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
* Use storage/ instead of db/ for sqlite3 db files
db/ should be for configuration only, not data. This will make it easier to mount a single volume into a container for testing, development, and even sqlite3 in production.
This change makes date/time options (value, min, max) in `time_field`, `date_field`, `datetime_field`, `week_field`, `month_field` form helpers behave in a unified way.
When System Tests call `fill_in_rich_text_area`, they interact with
`<trix-editor>` elements by changing the contents programmatically.
This is unlike how end-users will interact with the element. Overhauling
the test helper to more accurately reflect Real World usage would
require a sizable effort.
With that being said, leaving the `<trix-editor>` with focus after
populating its contents is a minor change that makes it a more genuine
recreation.
This PR aims to contain calls to `Base.connection` and
`Base.establish_connection` in active record rake tasks and the
migration code. In a follow up PR I will swap out the `Base` class for a
temporary class which will allow us to stop clobbering `Base` in the
active record rails tasks.
This work is an important step to achieve moving away from Active
Record's dependence on `Base.connection` and
`Base.establish_connection`. The reliance on `Base.connection` is
problematic for sharding support and calling `Base.establish_connection`
in the Rake tasks (without any warning message) indicates it's ok for
applications to do the same (outside these tasks, it's not).
I've vetted the approach of swapping out `Base` for a temporary class in
another branch but decided that it would be easier to demonstrate and
contain the changes if I first contained these calls. The major changes
in this PR are:
* Contain `Base.connection` in `Tasks.migration_connection` and replace
calls to each
* Contain `Base.establish_connection` in `Tasks.establish_connection`
and replace calls to each
* Add a `with_temporary_connection_for_each` method for cases where we
need to loop over each config and set the connection back afterwards
* Add a `with_temporary_connection(db_config)` method for cases where we
have one config but need to establish a new connection and set the old
one back.
* Update every place we were looping through configs to establish a
connection with the new temporary connection methods.
There are a lot of changes here but I've pulled out everything that
didn't need to be in this commit into other PRs.
Once this is merged, I'll create the next PR that replaces `Base` the
new methods in `Tasks` with a temporary connection and then we will
officially be no longer clobbering `Base` in these tasks. This also
reduces complexity because we won't need to ensure we set
`Base.connection` back at the end. Once that is working and all internal
methods are using the new temp class I'll deprecate calls on
`Base.connection` in these methods. Most applications just override the
task but not the actual methods in `Tasks` so my hope is this will be
smooth-ish. However, nothing will stop applications from still using
`Base.connection` for a very long time if they still want to clobber.
Followup: https://github.com/rails/rails/pull/46519
Followup: https://github.com/rails/rails/pull/46553
Fix: https://github.com/rails/rails/issues/45994
When using multiple database, `clear_query_caches_for_current_thread` goes
over all connections pool and have to acquire locks one by one to clear each
query cache.
If two threads do this in a different order they might deadlock.
By using a single lock for all connections we avoid this problem entirely
and properly prevent the puma thread from competing with the main thread.
As for previous PRs, it would be cleaner to have this lock around a Rack
middleware, and to release it when calling Capybara primitives like `visit`.
But we need a good chokepoint in Capybara for that, I need to explore that upstream.
The Mail gem changed format of the delivery method arguments for
sendmail from a string to an array of strings in this commit
7e1196bd29
As the settings are now a sendmail delivery method produces the
following error:
```
.../mail-2.8.0/lib/mail/network/delivery_methods/sendmail.rb:53:in `initialize': :arguments expected to be an Array of individual string args (ArgumentError)
```
This also updates the mail dependency since the new default won't work
with the older versions.
Previously, this code was using `ActiveRecord::Base.connection` for the
internal metadata. However, if a MigrationContext was initialized with a
different connection's internal metadata we should use that. This fixes
the `record_environment` method to use the correct connection for
storing the environment. We noticed this in flaky tests at Shopify
causing internal metadata to get written to the wrong readonly
connection. I have added a test so we don't miss this in the future.
