Move everything to initializers, and replace the email settings
interceptor by a BalancerDeliveryMethod.
It has the advantage that it can be configured entirely from the
`config/environment.rb` file, without an extra file to look at.
Before, every time a password was tested, the dictionaries were parsed
again by zxcvbn.
Parsing dictionaries is slow: it may take up to ~1s. This doesn't matter
that much in production, but it makes tests very slow (because we tend
to create a lot of User records).
With this changes, the initializer tester is shared between calls, class
instances and threads. It is lazily loaded on first use, in order not to
slow down the application boot sequence.
This uses ~20 Mo of memory (only once for all threads), but makes tests
more that twice faster.
For instance, model tests go from **8m 21s** to **3m 26s**.
NB:
An additionnal optimization could be to preload the tester on
boot, before workers are forked, to take advantage of Puma copy-on-write
mechanism. In this way all forked workers would use the same cached
instance.
But:
- We're not actually sure this would work properly. What if Ruby updates
an interval ivar on the class, and this forces the OS to copy the
whole data structure in each fork?
- Puma phased restarts are not compatible with copy-on-write anyway.
So we're avoiding this optimisation for now, and take the extra 20 Mo
per worker.
These constants were defined so that existing enqueued jobs wouldn't
fail during the app upgrade.
These jobs are long gone. Let's remove the compatibility code.
Previously Sentry reported job exceptions even if a retry
strategy was specified. So we had to ignore retried job exceptions
entirely.
Since sentry-delayed-job 0.4.4, we can instead let Sentry report
job exceptions when the retry count is exhausted. Which is
exactly the behavior we want.
This fixes an error message on app startup about autoloaded
constants:
> DEPRECATION WARNING: Initialization autoloaded the constants ActionText::ContentHelper and ActionText::TagHelper.
The reason for this error is that the Mailjet gem forces the
immediate loading of `action_mailer`. Which leads to the
following sequence of events:
On app init, when bundler requires all the gems in the Gemfile:
- The Mailjet gem is required,
- It loads `ActionMailer::Base`.
Later, when Rails initializes itself:
- `ActionText` creates an `action_text.helpers` initializer,
- This initializer register hooks to add `ActionText` helpers
when either `action_controller_base` or `action_mailer` are loaded,
- But as `action_mailer` has already been loaded, the hook is trigerred
immediately,
- ActiveSupport::Dependencies notices `ActionText` constants do not
exist yet, auto-load them, and add the constants to
`ActiveSupport::Dependencies.autoloaded_constants`.
And even later, at the end of the Rails initialization process:
- The `:warn_if_autoloaded` initializer is run,
- It notices that `autoloaded_constants` is not empty, and prints the
warning message.
See https://github.com/mailjet/mailjet-gem/issues/213
With Rails 6.1, the default queue is now the global application queue.
We want to retain our custom queues in some cases, so configure them
epxlicitely.
Follow-up of #5953.
Refactor the concerns with two goals:
- Getting closer from the way ActiveStorage adds its own hooks.
Usually ActiveStorage does this using an `Attachment#after_create`
hook, which then delegates to the blob to enqueue the job.
- Enqueuing each job only once. By hooking on `Attachment#after_create`,
we guarantee each job will be added only once.
We then let the jobs themselves check if they are relevant or not, and
retry or discard themselves if necessary.
We also need to update the tests a bit, because Rails'
`perform_enqueued_jobs(&block)` test helper doesn't honor the `retry_on`
clause of jobs. Instead it forwards the exception to the caller – which
makes the test fail.
Instead we use the inline version of `perform_enqueued_jobs()`, without
a block, which properly ignores errors catched by retry_on.
This warning re-appeared when running mailer tests:
```
DISABLE_SPRING=1 bin/rspec spec/mailers/administration_mailer_spec.rb
```
It is now fixed properly, in a way recommanded by the documentation.
Turns out we need not only to load the Job constants later, but also
not to do the same work twice – otherwise we'll get a
> ApiEntreprise::Job constant is already defined
when attempting to re-define the constant.
Fix a warning when running tests:
> DEPRECATION WARNING: Initialization autoloaded the constant DynamicSmtpSettingsInterceptor.
>
> Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.
>
> Reloading does not reboot the application, and therefore code executed during
> initialization does not run again. So, if you reload DynamicSmtpSettingsInterceptor, for example,
> the expected changes won't be reflected in that stale Class object.
>
> This autoloaded constant has been unloaded.
>
> Please, check the "Autoloading and Reloading Constants" guide for solutions.
However if we fix as recommanded, the interceptor will get added
each time the classes are reloaded. And as the actual class instance
changed after the reloading, they won't be de-duplicated – *and*
there's no way to remove the old interceptor without having a reference
to the (now-deleted) class.
Instead we load the interceptor once, and add a message about the class
not being auto-reloaded.
This removes spam in the debug console when running locally.
Removed messages look like a swarm of:
> [Tracing] Discarding <rails.request> transaction </assets/marianne.png> because it's not included in the random sample (sampling rate = 0.001)
Fixes zeitwerk complaining that the compatibility aliases loaded in an
initializer will never be reloaded.
In our case it doesn't matter that much, but it will reduce the console
spam.
As the comment states, it would be nice to load the Virus Scanner on
the Attachment (rather than the blob).
However, in order not to clobber the blob metadata, we want to run the
VirusScanner once the blob analyzer did run.
And the most direct way to detect that the blob analyzer did run is to
add an `on_update_commit` hook on the blob, as this hook will be
trigerred when saving changes to the metadata. This is what the current
solution uses.
So the current solution is almost optimal, and has a low chance of
accidentally clobbering the blob metadata – as the virus scanner is only
started when the analysis phase is finished.
ActiveStorage jobs are now moved to their own queue.
For consistency, we also move our own analysis jobs (VirusScannerJob)
on the same `:active_storage_analysis` queue.
This makes `ActionDispatch::Controller#content_type` return not only
the MIME type, but also in some circumstances the charset.
Example:
```ruby
reponse.content_type == 'text/html; charset=utf-8'
```
The MIME type-only fragment can now be accessed using `#media_type`.
Changes to the tests are not stricly necessary (because no charset is
present in the actual value), but represent the intent better.
This makes cookies more secure, by adding an automatic "purpose" field
to each cookie.
Cookies generated by Rails 5 are still forward-compatible. However
from now on the generated cookies will not be backward-compatible with
Rails 6.
When running the app using `bin/webpack-dev-server` (the external
(and fast) assets server), LiveReload is used. We need to explicitely
allow the LiveReload connections in the CSP policy.
Turns out we now need to specify the protocol explicitely.
For instance, Firefox iOS is reported as `firefox? true, version 20`.
As on iOS only the system-provided Webkit is allowed, we can instead
safely assume that all browsers on a recent iOS device are modern.
We are making these changes in order to always use DS_Proxy. Before this change DS_Proxy was not used to write files when ActiveStorage was used directly and not through “direct upload”.
